soc: qcom: pdr: Fix the potential deadlock

When some client process A call pdr_add_lookup() to add the look up for
the service and does schedule locator work, later a process B got a new
server pack

soc: qcom: pdr: Fix the potential deadlock

When some client process A call pdr_add_lookup() to add the look up for
the service and does schedule locator work, later a process B got a new
server packet indicating locator is up and call pdr_locator_new_server()
which eventually sets pdr->locator_init_complete to true which process A
sees and takes list lock and queries domain list but it will timeout due
to deadlock as the response will queued to the same qmi->wq and it is
ordered workqueue and process B is not able to complete new server
request work due to deadlock on list lock.

Fix it by removing the unnecessary list iteration as the list iteration
is already being done inside locator work, so avoid it here and just
call schedule_work() here.

Process A Process B

process_scheduled_works()
pdr_add_lookup() qmi_data_ready_work()
process_scheduled_works() pdr_locator_new_server()
pdr->locator_init_complete=true;
pdr_locator_work()
mutex_lock(&pdr->list_lock);

pdr_locate_service() mutex_lock(&pdr->list_lock);

pdr_get_domain_list()
pr_err("PDR: %s get domain list
txn wait failed: %d\n",
req->service_name,
ret);

Timeout error log due to deadlock:

"
PDR: tms/servreg get domain list txn wait failed: -110
PDR: service lookup for msm/adsp/sensor_pd:tms/servreg failed: -110
"

Thanks to Bjorn and Johan for letting me know that this commit also fixes
an audio regression when using the in-kernel pd-mapper as that makes it
easier to hit this race. [1]

Link: https://lore.kernel.org/lkml/[email protected]/ # [1]
Fixes: fbe639b44a82 ("soc: qcom: Introduce Protection Domain Restart helpers")
CC: [email protected]
Reviewed-by: Bjorn Andersson <[email protected]>
Tested-by: Bjorn Andersson <[email protected]>
Tested-by: Johan Hovold <[email protected]>
Signed-off-by: Saranya R <[email protected]>
Co-developed-by: Mukesh Ojha <[email protected]>
Signed-off-by: Mukesh Ojha <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

soc: qcom: pdr: simplify with cleanup.h

Allocate the memory with scoped/cleanup.h to reduce error handling (less
error paths) and make the code a bit simpler.

Signed-off-by: Krzysztof Kozlowski <kr

soc: qcom: pdr: simplify with cleanup.h

Allocate the memory with scoped/cleanup.h to reduce error handling (less
error paths) and make the code a bit simpler.

Signed-off-by: Krzysztof Kozlowski <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

soc: qcom: pdr: fix parsing of domains lists

While parsing the domains list, start offsets from 0 rather than from
domains_read. The domains_read is equal to the total count of the
domains we have s

soc: qcom: pdr: fix parsing of domains lists

While parsing the domains list, start offsets from 0 rather than from
domains_read. The domains_read is equal to the total count of the
domains we have seen, while the domains list in the message starts from
offset 0.

Fixes: fbe639b44a82 ("soc: qcom: Introduce Protection Domain Restart helpers")
Tested-by: Steev Klimaszewski <[email protected]>
Tested-by: Alexey Minnekhanov <[email protected]>
Reviewed-by: Chris Lew <[email protected]>
Tested-by: Neil Armstrong <[email protected]> # on SM8550-QRD
Signed-off-by: Dmitry Baryshkov <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

soc: qcom: pdr: protect locator_addr with the main mutex

If the service locator server is restarted fast enough, the PDR can
rewrite locator_addr fields concurrently. Protect them by placing
modific

soc: qcom: pdr: protect locator_addr with the main mutex

If the service locator server is restarted fast enough, the PDR can
rewrite locator_addr fields concurrently. Protect them by placing
modification of those fields under the main pdr->lock.

Fixes: fbe639b44a82 ("soc: qcom: Introduce Protection Domain Restart helpers")
Tested-by: Neil Armstrong <[email protected]> # on SM8550-QRD
Tested-by: Steev Klimaszewski <[email protected]>
Tested-by: Alexey Minnekhanov <[email protected]>
Signed-off-by: Dmitry Baryshkov <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

soc: qcom: Switch to EXPORT_SYMBOL_GPL()

Switch to GPL version of EXPORT_SYMBOL for Qualcomm SoC drivers.

Signed-off-by: Unnathi Chalicheemala <[email protected]>
Reviewed-by: Trilok Soni <

soc: qcom: Switch to EXPORT_SYMBOL_GPL()

Switch to GPL version of EXPORT_SYMBOL for Qualcomm SoC drivers.

Signed-off-by: Unnathi Chalicheemala <[email protected]>
Reviewed-by: Trilok Soni <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

soc: qcom: pdr: replace usage of found with dedicated list iterator variable

To move the list iterator variable into the list_for_each_entry_*()
macro in the future it should be avoided to use the l

soc: qcom: pdr: replace usage of found with dedicated list iterator variable

To move the list iterator variable into the list_for_each_entry_*()
macro in the future it should be avoided to use the list iterator
variable after the loop body.

To *never* use the list iterator variable after the loop it was
concluded to use a separate iterator variable instead of a
found boolean [1].

This removes the need to use a found variable and simply checking if
the variable was set, can determine if the break/goto was hit.

Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/
Signed-off-by: Jakob Koschel <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...

soc: qcom: pdr: Prefer strscpy over strcpy

strcpy() performs no bounds checking on the destination buffer. This
could result in linear overflows beyond the end of the buffer, leading
to all kinds of

soc: qcom: pdr: Prefer strscpy over strcpy

strcpy() performs no bounds checking on the destination buffer. This
could result in linear overflows beyond the end of the buffer, leading
to all kinds of misbehaviors. The safe replacement is strscpy().

This is a previous step in the path to remove the strcpy() function
entirely from the kernel.

Signed-off-by: Len Baker <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

show more ...

soc: qcom: pdr: Fix error return code in pdr_register_listener

Fix to return the error code -EREMOTEIO from pdr_register_listener
rather than 0.

Fixes: fbe639b44a82 ("soc: qcom: Introduce Protectio

soc: qcom: pdr: Fix error return code in pdr_register_listener

Fix to return the error code -EREMOTEIO from pdr_register_listener
rather than 0.

Fixes: fbe639b44a82 ("soc: qcom: Introduce Protection Domain Restart helpers")
Reported-by: Hulk Robot <[email protected]>
Signed-off-by: Qinglang Miao <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

soc: qcom: pdr: Constify static qmi structs

Their only usage is to pass their address to qmi_handle_init() which
accepts const pointers to both qmi_ops and qmi_msg_handler. Make them
const to allow

soc: qcom: pdr: Constify static qmi structs

Their only usage is to pass their address to qmi_handle_init() which
accepts const pointers to both qmi_ops and qmi_msg_handler. Make them
const to allow the compiler to put them in read-only memory.

Signed-off-by: Rikard Falkeborn <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

soc: qcom: initialize local variable

clang static analysis reports this problem

pdr_interface.c:596:6: warning: Branch condition evaluates
to a garbage value
if (!req.service_path[0])

soc: qcom: initialize local variable

clang static analysis reports this problem

pdr_interface.c:596:6: warning: Branch condition evaluates
to a garbage value
if (!req.service_path[0])
^~~~~~~~~~~~~~~~~~~~

This check that req.service_path was set in an earlier loop.
However req is a stack variable and its initial value
is undefined.

So initialize req to 0.

Fixes: fbe639b44a82 ("soc: qcom: Introduce Protection Domain Restart helpers")
Reviewed-by: Sibi Sankar <[email protected]>
Signed-off-by: Tom Rix <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

soc: qcom: pdr: Reorder the PD state indication ack

The Protection Domains (PD) have a mechanism to keep its resources
enabled until the PD down indication is acked. Reorder the PD state
indication

soc: qcom: pdr: Reorder the PD state indication ack

The Protection Domains (PD) have a mechanism to keep its resources
enabled until the PD down indication is acked. Reorder the PD state
indication ack so that clients get to release the relevant resources
before the PD goes down.

Reviewed-by: Bjorn Andersson <[email protected]>
Reviewed-by: Rishabh Bhatnagar <[email protected]>
Fixes: fbe639b44a82 ("soc: qcom: Introduce Protection Domain Restart helpers")
Reported-by: Rishabh Bhatnagar <[email protected]>
Signed-off-by: Sibi Sankar <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

iov_iter: Move unnecessary inclusion of crypto/hash.h

The header file linux/uio.h includes crypto/hash.h which pulls in
most of the Crypto API. Since linux/uio.h is used throughout the
kernel this

iov_iter: Move unnecessary inclusion of crypto/hash.h

The header file linux/uio.h includes crypto/hash.h which pulls in
most of the Crypto API. Since linux/uio.h is used throughout the
kernel this means that every tiny bit of change to the Crypto API
causes the entire kernel to get rebuilt.

This patch fixes this by moving it into lib/iov_iter.c instead
where it is actually used.

This patch also fixes the ifdef to use CRYPTO_HASH instead of just
CRYPTO which does not guarantee the existence of ahash.

Unfortunately a number of drivers were relying on linux/uio.h to
provide access to linux/slab.h. This patch adds inclusions of
linux/slab.h as detected by build failures.

Also skbuff.h was relying on this to provide a declaration for
ahash_request. This patch adds a forward declaration instead.

Signed-off-by: Herbert Xu <[email protected]>
Signed-off-by: Al Viro <[email protected]>

show more ...

soc: qcom: pdr: Remove impossible error condition

The patch fbe639b44a82: "soc: qcom: Introduce Protection Domain
Restart helpers" leads to the following static checker warning:

drivers/soc/qcom/pd

soc: qcom: pdr: Remove impossible error condition

The patch fbe639b44a82: "soc: qcom: Introduce Protection Domain
Restart helpers" leads to the following static checker warning:

drivers/soc/qcom/pdr_interface.c:158 pdr_register_listener()
'(resp.curr_state < (-((~0 >> 1)) - 1)) => (s32min-s32max < s32min)'
These are casted to int so they can't be outside of int range.

Fixes: fbe639b44a82 ("soc: qcom: Introduce Protection Domain Restart helpers")
Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Sibi Sankar <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

soc: qcom: pdr: Avoid uninitialized use of found in pdr_indication_cb

Clang warns:

../drivers/soc/qcom/pdr_interface.c:316:2: warning: variable 'found' is
used uninitialized whenever 'for' loop exi

soc: qcom: pdr: Avoid uninitialized use of found in pdr_indication_cb

Clang warns:

../drivers/soc/qcom/pdr_interface.c:316:2: warning: variable 'found' is
used uninitialized whenever 'for' loop exits because its condition is
false [-Wsometimes-uninitialized]
list_for_each_entry(pds, &pdr->lookups, node) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/linux/list.h:624:7: note: expanded from macro
'list_for_each_entry'
&pos->member != (head);
^~~~~~~~~~~~~~~~~~~~~~
../drivers/soc/qcom/pdr_interface.c:325:7: note: uninitialized use
occurs here
if (!found)
^~~~~
../drivers/soc/qcom/pdr_interface.c:316:2: note: remove the condition if
it is always true
list_for_each_entry(pds, &pdr->lookups, node) {
^
../include/linux/list.h:624:7: note: expanded from macro
'list_for_each_entry'
&pos->member != (head);
^
../drivers/soc/qcom/pdr_interface.c:309:12: note: initialize the
variable 'found' to silence this warning
bool found;
^
= 0
1 warning generated.

Initialize found to false to fix this warning.

Reviewed-by: Nick Desaulniers <[email protected]>
Fixes: fbe639b44a82 ("soc: qcom: Introduce Protection Domain Restart helpers")
Link: https://github.com/ClangBuiltLinux/linux/issues/933
Signed-off-by: Nathan Chancellor <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...

soc: qcom: Introduce Protection Domain Restart helpers

Qualcomm SoCs (starting with MSM8998) allow for multiple protection domains
to run on the same Q6 sub-system. This allows for services like ATH

soc: qcom: Introduce Protection Domain Restart helpers

Qualcomm SoCs (starting with MSM8998) allow for multiple protection domains
to run on the same Q6 sub-system. This allows for services like ATH10K WLAN
FW to have their own separate address space and crash/recover without
disrupting the modem and other PDs running on the same sub-system. The PDR
helpers introduces an abstraction that allows for tracking/controlling the
life cycle of protection domains running on various Q6 sub-systems.

Signed-off-by: Sibi Sankar <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Andersson <[email protected]>

show more ...