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

[Albert Esteve]

On Thu, Oct 8, 2026 at 9:03 PM Mukesh Ojha <mukesh.ojha@xxxxxxxxxxxxxxxx> wrote:
>
> On Tue, Oct 06, 2026 at 02:57:45PM +0530, Pavan Kondeti wrote:
> > On Thu, Oct 01, 2026 at 06:16:48PM +0200, Albert Esteve wrote:
> > > From: Ninad Naik <quic_ninanaik@xxxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxxx>
> > > Co-developed-by: Guru Das Srinagesh <quic_gurus@xxxxxxxxxxx>
> > > Signed-off-by: Guru Das Srinagesh <quic_gurus@xxxxxxxxxxx>
> > > Signed-off-by: Venkatakrishnaiah Pari <quic_vpari@xxxxxxxxxxx>
> > > Signed-off-by: Jian Shu <quic_jianshu@xxxxxxxxxxx>
> > > Signed-off-by: Ninad Naik <quic_ninanaik@xxxxxxxxxxx>
> > > Signed-off-by: Albert Esteve <aesteve@xxxxxxxxxx>
> > > ---
> > >  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?
>
> In the downstream implementation, there can be calls that occupy waitq
> contexts and sleep waiting for events from other VMs.  However, there can
> be multiple such calls and we may run out of waitq contexts. Normally,
> these calls are SMCinvoke calls or qcom_scm_qtee_invoke_smc() users. In
> those scenarios, even normal slow SMC calls will wait for a context and may
> wait forever for one. For this reason, one waitq context is always reserved
> for slow calls to ensure forward progression. So, slow calls
> still need mutex for such cases.. However, I doubt if we have
> sleeping SMCinvoke in upstream ..

Thanks for the context. That's correct, SMCInvoke isn't upstream, so
the starvation scenario described in the commit message cannot happen
today. The semaphore here just replaces the mutex that was
unnecessarily serializing all calls. I can rephrase the commit message
to avoid mentioning SMCInvoke as motivation (that mention comes from
the downstream commit). And if SMCInvoke eventually lands upstream, a
reservation could be added.

BR,
Albert.

>
> --
> -Mukesh Ojha
>