mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] firmware: qcom: scm locking improvements
@ 2026-08-24 11:35 Albert Esteve
  2026-08-24 11:35 ` [PATCH 1/2] firmware: qcom: scm: Introduce new locking mechanism for SCM driver Albert Esteve
  2026-08-24 11:35 ` [PATCH 2/2] firmware: qcom: scm: Allow the SMC request to freeze and kill Albert Esteve
  0 siblings, 2 replies; 6+ messages in thread
From: Albert Esteve @ 2026-08-24 11:35 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio
  Cc: linux-arm-msm, linux-kernel, Albert Esteve, Ninad Naik,
	Murali Nalajala, Venkatakrishnaiah Pari, Jian Shu,
	Yuvaraj Ranganathan, Guru Das Srinagesh

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

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>
---
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 and kill

 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(-)
---
base-commit: 818bebeb63dd6bf5f4e07e145f6cdbace520a34c
change-id: 20260820-port-scm-patches-bf58a5a1b56e

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] firmware: qcom: scm: Introduce new locking mechanism for SCM driver
  2026-08-24 11:35 [PATCH 0/2] firmware: qcom: scm locking improvements Albert Esteve
@ 2026-08-24 11:35 ` Albert Esteve
  2026-08-25 10:15   ` Konrad Dybcio
  2026-08-24 11:35 ` [PATCH 2/2] firmware: qcom: scm: Allow the SMC request to freeze and kill Albert Esteve
  1 sibling, 1 reply; 6+ messages in thread
From: Albert Esteve @ 2026-08-24 11:35 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio
  Cc: linux-arm-msm, linux-kernel, Albert Esteve, Ninad Naik,
	Murali Nalajala, Venkatakrishnaiah Pari, Jian Shu,
	Guru Das Srinagesh

From: Ninad Naik <quic_ninanaik@quicinc.com>

With the existing global mutex lock, there is a possibility that it
could resulty into a deadlock when SMCInvoke and smc call go to
firmware in the same order and both wait on same WAITQ in firmware.

Replace the global mutex with a counting semaphore.

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        | 4 ++++
 drivers/firmware/qcom/qcom_scm.h        | 3 +++
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/firmware/qcom/qcom_scm-legacy.c b/drivers/firmware/qcom/qcom_scm-legacy.c
index 029e6d117cb8d..6afcbe6f7e5ed 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 127365ab11fc2..1b51e0fb82924 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 3eaa4c9ccf3cc..7a042273d6d5b 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.
@@ -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 cf90a565fdfbd..06fdc5e56bea4 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] firmware: qcom: scm: Allow the SMC request to freeze and kill
  2026-08-24 11:35 [PATCH 0/2] firmware: qcom: scm locking improvements Albert Esteve
  2026-08-24 11:35 ` [PATCH 1/2] firmware: qcom: scm: Introduce new locking mechanism for SCM driver Albert Esteve
@ 2026-08-24 11:35 ` Albert Esteve
  2026-08-25 10:21   ` Konrad Dybcio
  1 sibling, 1 reply; 6+ messages in thread
From: Albert Esteve @ 2026-08-24 11:35 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio
  Cc: linux-arm-msm, linux-kernel, Albert Esteve, Yuvaraj Ranganathan

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

SMC requests in waitQ are put into IDLE state which is not allowing
the task to freeze/kill, adding FREEZABLE & KILLABLE flag will allow the
task to freeze and restore the waitQ state on resume and kill the task
on shutdown.

Signed-off-by: Yuvaraj Ranganathan <yrangana@qti.qualcomm.com>
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 drivers/firmware/qcom/qcom_scm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 7a042273d6d5b..e4ebe4c0746bb 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -2664,7 +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);
+	wait_for_completion_state(wq, TASK_IDLE | TASK_FREEZABLE | TASK_KILLABLE);
 
 	return 0;
 }

-- 
2.55.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] firmware: qcom: scm: Introduce new locking mechanism for SCM driver
  2026-08-24 11:35 ` [PATCH 1/2] firmware: qcom: scm: Introduce new locking mechanism for SCM driver Albert Esteve
@ 2026-08-25 10:15   ` Konrad Dybcio
  2026-08-26 11:33     ` Albert Esteve
  0 siblings, 1 reply; 6+ messages in thread
From: Konrad Dybcio @ 2026-08-25 10:15 UTC (permalink / raw)
  To: Albert Esteve, Bjorn Andersson, Konrad Dybcio
  Cc: linux-arm-msm, linux-kernel, Ninad Naik, Murali Nalajala,
	Venkatakrishnaiah Pari, Jian Shu, Guru Das Srinagesh

On 8/24/26 1:35 PM, Albert Esteve wrote:
> From: Ninad Naik <quic_ninanaik@quicinc.com>
> 
> With the existing global mutex lock, there is a possibility that it
> could resulty into a deadlock when SMCInvoke and smc call go to
> firmware in the same order and both wait on same WAITQ in firmware.

I think the commit message should be extended to better explain how
that may happen (because presumably it did in  the real world)

A Fixes tag may also be applicable here

Konrad

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] firmware: qcom: scm: Allow the SMC request to freeze and kill
  2026-08-24 11:35 ` [PATCH 2/2] firmware: qcom: scm: Allow the SMC request to freeze and kill Albert Esteve
@ 2026-08-25 10:21   ` Konrad Dybcio
  0 siblings, 0 replies; 6+ messages in thread
From: Konrad Dybcio @ 2026-08-25 10:21 UTC (permalink / raw)
  To: Albert Esteve, Bjorn Andersson, Konrad Dybcio
  Cc: linux-arm-msm, linux-kernel, Yuvaraj Ranganathan

On 8/24/26 1:35 PM, Albert Esteve wrote:
> From: Yuvaraj Ranganathan <yrangana@qti.qualcomm.com>
> 
> SMC requests in waitQ are put into IDLE state which is not allowing
> the task to freeze/kill, adding FREEZABLE & KILLABLE flag will allow the
> task to freeze and restore the waitQ state on resume and kill the task
> on shutdown.

That sounds desirable. But presumably there's an actual issue behind
it that is left undescribed and future readers will be left guessing

Konrad

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] firmware: qcom: scm: Introduce new locking mechanism for SCM driver
  2026-08-25 10:15   ` Konrad Dybcio
@ 2026-08-26 11:33     ` Albert Esteve
  0 siblings, 0 replies; 6+ messages in thread
From: Albert Esteve @ 2026-08-26 11:33 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Bjorn Andersson, Konrad Dybcio, linux-arm-msm, linux-kernel,
	Ninad Naik, Murali Nalajala, Venkatakrishnaiah Pari, Jian Shu,
	Guru Das Srinagesh

On Tue, Aug 25, 2026 at 12:15 PM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 8/24/26 1:35 PM, Albert Esteve wrote:
> > From: Ninad Naik <quic_ninanaik@quicinc.com>
> >
> > With the existing global mutex lock, there is a possibility that it
> > could resulty into a deadlock when SMCInvoke and smc call go to
> > firmware in the same order and both wait on same WAITQ in firmware.
>
> I think the commit message should be extended to better explain how
> that may happen (because presumably it did in  the real world)
>
> A Fixes tag may also be applicable here

Hi Konrad,

Sure, I will add more details for both this and the next patch in the
next version, along with the Fixes tag.

We triaged/ported the patches (see cover letter for their origin), but
I do not think we hit that specifically on our boards.

BR,
Albert

>
> Konrad
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-08-26 11:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-24 11:35 [PATCH 0/2] firmware: qcom: scm locking improvements Albert Esteve
2026-08-24 11:35 ` [PATCH 1/2] firmware: qcom: scm: Introduce new locking mechanism for SCM driver Albert Esteve
2026-08-25 10:15   ` Konrad Dybcio
2026-08-26 11:33     ` Albert Esteve
2026-08-24 11:35 ` [PATCH 2/2] firmware: qcom: scm: Allow the SMC request to freeze and kill Albert Esteve
2026-08-25 10:21   ` Konrad Dybcio

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®