[PATCH v3 0/2] firmware: qcom: scm locking improvements

[PATCH v3 0/2] firmware: qcom: scm locking improvements

[Albert Esteve]

The qcom_scm driver serializes Secure Channel Manager calls
to Qualcomm firmware. This series ports two related fixes from
Qualcomm's kernel trees [1][2] adapted for the current upstream
driver.

Series summary:

 Patch 1: Replace SCM global mutex with a counting semaphore.
 Patch 2: Allow SCM waitqueue completions to freeze and be killed
 cleanly on suspend and shutdown.

Adaptations:

- Size the semaphore from scm->wq_cnt (already queried via
  WAITQ_GET_INFO during probe) rather than adding call_ctx_cnt
  and extending skip_mutex helper
- Do not port skip_mutex unlock/relock around WAITQ_SLEEP; that
  path does not exist in master
- Initialize the semaphore before publishing __scm
- Drop TASK_KILLABLE: returning early on a fatal signal skips the
  QCOM_SCM_WAITQ_RESUME SMC, leaking the TrustZone execution context

Tested on Qualcomm's SA8775P Ride V3, kernel 7.2:
- qcom_scm probes successfully
- qcomtee platform device registered
- SMC calls complete without error

[1] https://git.codelinaro.org/clo/le/meta-qti-auto-kernel/-/raw/LY.AU.0.1.0.r1-16800-gen4meta.0/recipes-kernel/linux/files/scm_adci/0008-PENDING-firmware-qcom-scm-Introduce-new-locking-mech.patch
[2] https://git.codelinaro.org/clo/le/meta-qti-auto-kernel/-/commit/9319960e593c88e6d88bfaf0f6ed3e1235926ea3

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
Changes in v3:
- Drop TASK_KILLABLE from patch 2 (raised by Sashiko)
- Rebase to v7.3-rc5 kernel
- Link to v2: https://lore.kernel.org/r/20260831-port-scm-patches-v2-0-bab5595e77b3@redhat.com

Changes in v2:
- Improved issue explanation in commit bodies
  and added Fixes: tags
- Treat 0 wq_cnt responses as invalid
- Forward wait_for_completion_state() return value
- Link to v1: https://lore.kernel.org/r/20260824-port-scm-patches-v1-0-1dfd69374402@redhat.com

---
Ninad Naik (1):
      firmware: qcom: scm: Introduce new locking mechanism for SCM driver

Yuvaraj Ranganathan (1):
      firmware: qcom: scm: Allow the SMC request to freeze

 drivers/firmware/qcom/qcom_scm-legacy.c |  8 ++------
 drivers/firmware/qcom/qcom_scm-smc.c    |  7 ++-----
 drivers/firmware/qcom/qcom_scm.c        | 10 ++++++----
 drivers/firmware/qcom/qcom_scm.h        |  3 +++
 4 files changed, 13 insertions(+), 15 deletions(-)
---
base-commit: 551c722f40809618230001baccf219193e22fc5a
change-id: 20260820-port-scm-patches-bf58a5a1b56e

Best regards,
-- 
Albert Esteve <aesteve@redhat.com>

[PATCH v3 1/2] firmware: qcom: scm: Introduce new locking mechanism for SCM driver

[Albert Esteve]

From: Ninad Naik <quic_ninanaik@quicinc.com>

qcom_scm holds its global mutex across WAITQ_SLEEP and
wait_for_completion(). Firmware waitqs allow multiple SMCs in
flight (wq_cnt). If one call is parked on a waitq while holding
the mutex, a second call (e.g., SMCInvoke) cannot enter firmware
and both stall on the waitq.

Replace the global mutex with a counting semaphore sized from wq_cnt,
with at least 1 wait queue.

Fixes: ccd207ec848e ("firmware: qcom_scm: Support multiple waitq contexts")
Signed-off-by: Murali Nalajala <quic_mnalajal@quicinc.com>
Co-developed-by: Guru Das Srinagesh <quic_gurus@quicinc.com>
Signed-off-by: Guru Das Srinagesh <quic_gurus@quicinc.com>
Signed-off-by: Venkatakrishnaiah Pari <quic_vpari@quicinc.com>
Signed-off-by: Jian Shu <quic_jianshu@quicinc.com>
Signed-off-by: Ninad Naik <quic_ninanaik@quicinc.com>
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 drivers/firmware/qcom/qcom_scm-legacy.c | 8 ++------
 drivers/firmware/qcom/qcom_scm-smc.c    | 7 ++-----
 drivers/firmware/qcom/qcom_scm.c        | 6 +++++-
 drivers/firmware/qcom/qcom_scm.h        | 3 +++
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/firmware/qcom/qcom_scm-legacy.c b/drivers/firmware/qcom/qcom_scm-legacy.c
index 029e6d117cb8..6afcbe6f7e5e 100644
--- a/drivers/firmware/qcom/qcom_scm-legacy.c
+++ b/drivers/firmware/qcom/qcom_scm-legacy.c
@@ -6,7 +6,6 @@
 #include <linux/slab.h>
 #include <linux/io.h>
 #include <linux/module.h>
-#include <linux/mutex.h>
 #include <linux/errno.h>
 #include <linux/err.h>
 #include <linux/firmware/qcom/qcom_scm.h>
@@ -15,9 +14,6 @@
 
 #include "qcom_scm.h"
 
-static DEFINE_MUTEX(qcom_scm_lock);
-
-
 /**
  * struct arm_smccc_args
  * @args:	The array of values used in registers in smc instruction
@@ -173,11 +169,11 @@ int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
 	smc.args[1] = (unsigned long)&context_id;
 	smc.args[2] = cmd_phys;
 
-	mutex_lock(&qcom_scm_lock);
+	down(&qcom_scm_sem_lock);
 	__scm_legacy_do(&smc, &smc_res);
 	if (smc_res.a0)
 		ret = qcom_scm_remap_error(smc_res.a0);
-	mutex_unlock(&qcom_scm_lock);
+	up(&qcom_scm_sem_lock);
 	if (ret)
 		goto out;
 
diff --git a/drivers/firmware/qcom/qcom_scm-smc.c b/drivers/firmware/qcom/qcom_scm-smc.c
index 127365ab11fc..1b51e0fb8292 100644
--- a/drivers/firmware/qcom/qcom_scm-smc.c
+++ b/drivers/firmware/qcom/qcom_scm-smc.c
@@ -6,7 +6,6 @@
 #include <linux/io.h>
 #include <linux/errno.h>
 #include <linux/delay.h>
-#include <linux/mutex.h>
 #include <linux/slab.h>
 #include <linux/types.h>
 #include <linux/firmware/qcom/qcom_scm.h>
@@ -27,8 +26,6 @@ struct arm_smccc_args {
 #define CREATE_TRACE_POINTS
 #include "qcom_scm_trace.h"
 
-static DEFINE_MUTEX(qcom_scm_lock);
-
 #define QCOM_SCM_EBUSY_WAIT_MS 30
 #define QCOM_SCM_EBUSY_MAX_RETRY 20
 
@@ -135,11 +132,11 @@ static int __scm_smc_do(struct device *dev, struct arm_smccc_args *smc,
 	}
 
 	do {
-		mutex_lock(&qcom_scm_lock);
+		down(&qcom_scm_sem_lock);
 
 		ret = __scm_smc_do_quirk_handle_waitq(dev, smc, res);
 
-		mutex_unlock(&qcom_scm_lock);
+		up(&qcom_scm_sem_lock);
 
 		if (ret)
 			return ret;
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 3eaa4c9ccf3c..ea4481385412 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -78,6 +78,8 @@ struct qcom_scm_mem_map_info {
 	__le64 mem_size;
 };
 
+DEFINE_SEMAPHORE(qcom_scm_sem_lock, 1);
+
 /**
  * struct qcom_scm_qseecom_resp - QSEECOM SCM call response.
  * @result:    Result or status of the SCM call. See &enum qcom_scm_qseecom_result.
@@ -2869,7 +2871,7 @@ static int qcom_scm_probe(struct platform_device *pdev)
 	}
 
 	ret = qcom_scm_query_waitq_count(scm);
-	scm->wq_cnt = ret < 0 ? QCOM_SCM_DEFAULT_WAITQ_COUNT : ret;
+	scm->wq_cnt = ret <= 0 ? QCOM_SCM_DEFAULT_WAITQ_COUNT : ret;
 	scm->waitq_comps = devm_kcalloc(&pdev->dev, scm->wq_cnt, sizeof(*scm->waitq_comps),
 					GFP_KERNEL);
 	if (!scm->waitq_comps)
@@ -2893,6 +2895,8 @@ static int qcom_scm_probe(struct platform_device *pdev)
 					     "Failed to request qcom-scm irq\n");
 	}
 
+	sema_init(&qcom_scm_sem_lock, scm->wq_cnt);
+
 	/*
 	 * Paired with smp_load_acquire() in qcom_scm_is_available().
 	 *
diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h
index cf90a565fdfb..06fdc5e56bea 100644
--- a/drivers/firmware/qcom/qcom_scm.h
+++ b/drivers/firmware/qcom/qcom_scm.h
@@ -4,6 +4,8 @@
 #ifndef __QCOM_SCM_INT_H
 #define __QCOM_SCM_INT_H
 
+#include <linux/semaphore.h>
+
 struct device;
 struct qcom_tzmem_pool;
 
@@ -15,6 +17,7 @@ enum qcom_scm_convention {
 };
 
 extern enum qcom_scm_convention qcom_scm_convention;
+extern struct semaphore qcom_scm_sem_lock;
 
 #define MAX_QCOM_SCM_ARGS 10
 #define MAX_QCOM_SCM_RETS 3

-- 
2.55.0

[PATCH v3 2/2] firmware: qcom: scm: Allow the SMC request to freeze

[Albert Esteve]

From: Yuvaraj Ranganathan <yrangana@qti.qualcomm.com>

qcom_scm_wait_for_wq_completion() waits in TASK_IDLE. That is
uninterruptible, so a thread parked on a firmware waitq cannot be
frozen or killed. A long wait then blocks suspend, and shutdown
cannot tear the task down.

Wait with TASK_IDLE | TASK_FREEZABLE so the freezer can freeze the
waiter during suspend; after resume it is still waiting
for the same waitq completion.

TASK_KILLABLE is not added. If a fatal signal aborts the wait, the
driver returns early without issuing QCOM_SCM_WAITQ_RESUME, leaking
the TrustZone execution context.

Fixes: 366f05e348b2 ("firmware: qcom_scm: Use TASK_IDLE state in wait_for_wq_completion()")
Signed-off-by: Yuvaraj Ranganathan <yrangana@qti.qualcomm.com>
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 drivers/firmware/qcom/qcom_scm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index ea4481385412..92cee2263ed4 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -2664,9 +2664,7 @@ int qcom_scm_wait_for_wq_completion(struct device *dev, u32 wq_ctx)
 	if (IS_ERR(wq))
 		return PTR_ERR(wq);
 
-	wait_for_completion_state(wq, TASK_IDLE);
-
-	return 0;
+	return wait_for_completion_state(wq, TASK_IDLE | TASK_FREEZABLE);
 }
 
 static int qcom_scm_waitq_wakeup(struct qcom_scm *scm, unsigned int wq_ctx)

-- 
2.55.0

Re: [PATCH v3 1/2] firmware: qcom: scm: Introduce new locking mechanism for SCM driver

[Pavan Kondeti]

On Thu, Oct 01, 2026 at 06:16:48PM +0200, Albert Esteve wrote:
> From: Ninad Naik <quic_ninanaik@quicinc.com>
> 
> qcom_scm holds its global mutex across WAITQ_SLEEP and
> wait_for_completion(). Firmware waitqs allow multiple SMCs in
> flight (wq_cnt). If one call is parked on a waitq while holding
> the mutex, a second call (e.g., SMCInvoke) cannot enter firmware
> and both stall on the waitq.
> 
> Replace the global mutex with a counting semaphore sized from wq_cnt,
> with at least 1 wait queue.
> 
> Fixes: ccd207ec848e ("firmware: qcom_scm: Support multiple waitq contexts")
> Signed-off-by: Murali Nalajala <quic_mnalajal@quicinc.com>
> Co-developed-by: Guru Das Srinagesh <quic_gurus@quicinc.com>
> Signed-off-by: Guru Das Srinagesh <quic_gurus@quicinc.com>
> Signed-off-by: Venkatakrishnaiah Pari <quic_vpari@quicinc.com>
> Signed-off-by: Jian Shu <quic_jianshu@quicinc.com>
> Signed-off-by: Ninad Naik <quic_ninanaik@quicinc.com>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
>  drivers/firmware/qcom/qcom_scm-legacy.c | 8 ++------
>  drivers/firmware/qcom/qcom_scm-smc.c    | 7 ++-----
>  drivers/firmware/qcom/qcom_scm.c        | 6 +++++-
>  drivers/firmware/qcom/qcom_scm.h        | 3 +++
>  4 files changed, 12 insertions(+), 12 deletions(-)
> 
I don't know all the history, but in downstream [1] selective calls only
allowed to sleep w/o mutex.

Mukesh, do you know if it is safe to allow all slow calls w/o mutex?

Thanks,
Pavan

[1] https://git.codelinaro.org/clo/la/kernel/qcom/-/blob/KERNEL.PLATFORM.5.0.r35-02100-kernel.0/drivers/firmware/qcom/qcom_scm.c?ref_type=tags#L3057

Re: [PATCH v3 2/2] firmware: qcom: scm: Allow the SMC request to freeze

[Pavan Kondeti]

On Thu, Oct 01, 2026 at 06:16:49PM +0200, Albert Esteve wrote:
> From: Yuvaraj Ranganathan <yrangana@qti.qualcomm.com>
> 
> qcom_scm_wait_for_wq_completion() waits in TASK_IDLE. That is
> uninterruptible, so a thread parked on a firmware waitq cannot be
> frozen or killed. A long wait then blocks suspend, and shutdown
> cannot tear the task down.
> 
> Wait with TASK_IDLE | TASK_FREEZABLE so the freezer can freeze the
> waiter during suspend; after resume it is still waiting
> for the same waitq completion.
> 
> TASK_KILLABLE is not added. If a fatal signal aborts the wait, the
> driver returns early without issuing QCOM_SCM_WAITQ_RESUME, leaking
> the TrustZone execution context.
> 
> Fixes: 366f05e348b2 ("firmware: qcom_scm: Use TASK_IDLE state in wait_for_wq_completion()")
> Signed-off-by: Yuvaraj Ranganathan <yrangana@qti.qualcomm.com>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
>  drivers/firmware/qcom/qcom_scm.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index ea4481385412..92cee2263ed4 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -2664,9 +2664,7 @@ int qcom_scm_wait_for_wq_completion(struct device *dev, u32 wq_ctx)
>  	if (IS_ERR(wq))
>  		return PTR_ERR(wq);
>  
> -	wait_for_completion_state(wq, TASK_IDLE);
> -
> -	return 0;
> +	return wait_for_completion_state(wq, TASK_IDLE | TASK_FREEZABLE);
>  }
>  
If the wakeup happens while the task is frozen, the task will come out
of completion only when the task is thawed. I don't know if there are
any cases where that is not acceptable now that we are making all
non-atomic calls to enter w/o mutex.

Thanks,
Pavan