Re: [PATCH v5 6/6] firmware: qcom: scm: introduce keep_mdt_buf flag in PAS context

On Sat, Aug 01, 2026 at 11:08:30PM -0500, Bjorn Andersson wrote:
> On Fri, Jul 24, 2026 at 11:58:58PM +0530, Mukesh Ojha wrote:
> > The PAS image initialization path always retains the metadata buffer
> > when a valid qcom_scm_pas_context is provided, even if the caller does
> > not require it. This implicit behavior leads to unclear buffer ownership
> > and forces new users of qcom_mdt_pas_load() to manually release
> > metadata, which is error‑ prone and incorrect.
> > 
> > Add a keep_mdt_buf flag to struct qcom_scm_pas_context to make metadata
> > retention explicit.  Metadata buffers are now freed by default and are
> > only preserved when this flag is set. qcom_q6v5_pas enables this during
> > probe for contexts that require retained metadata for subsequent PAS
> > operations, while existing callers continue to work unchanged.
> > 
> 
> I presume given that this was sent together with the remoteproc patches
> that this can't be merged until those other changes has made it into the
> tree.
> 
> Please confirm if there is an actual dependency here.

I had an expectation that Sumit's patches would get picked up completely
somehow, which would avoid me adding the extra variable keep_mdt_buf to the
temporary data structure qcom_scm_pas_context. That's the only dependency.

> 
> > Signed-off-by: Mukesh Ojha <[email protected]>
> > ---
> >  drivers/firmware/qcom/qcom_scm.c       | 18 +++++++++++++++---
> >  drivers/remoteproc/qcom_q6v5_pas.c     |  3 +++
> >  include/linux/firmware/qcom/qcom_pas.h |  1 +
> >  3 files changed, 19 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> > index 16ae42e6c434..54ffec97cc26 100644
> > --- a/drivers/firmware/qcom/qcom_scm.c
> > +++ b/drivers/firmware/qcom/qcom_scm.c
> > @@ -625,7 +625,7 @@ static int qcom_scm_pas_prep_and_init_image(struct device *dev,
> >  	mdata_phys = qcom_tzmem_to_phys(mdata_buf);
> >  
> >  	ret = __qcom_scm_pas_init_image(dev, ctx->pas_id, mdata_phys, &res);
> > -	if (ret < 0)
> > +	if (ret < 0 || !ctx->keep_mdt_buf)
> >  		qcom_tzmem_free(mdata_buf);
> >  	else
> >  		ctx->ptr = mdata_buf;
> > @@ -664,9 +664,21 @@ static int qcom_scm_pas_init_image(struct device *dev, u32 pas_id,
> >  	memcpy(mdata_buf, metadata, size);
> >  
> >  	ret = __qcom_scm_pas_init_image(dev, pas_id, mdata_phys, &res);
> > -	if (ret < 0 || !ctx) {
> > +
> > +	/*
> > +	 * Some clients still pass the PAS context as NULL. Until all clients
> > +	 * switch to qcom_mdt_pas_load() and provide a valid PAS context, check
> > +	 * for NULL before dereferencing it.
> 
> Why don't we fix the problematic clients? There's no point in working
> around such things.

Was waiting for Sumit's patches to land so that I could convert
every client to use PAS ctx.

> 
> > +	 *
> > +	 * When a valid context is provided, keep_mdt_buf controls whether the
> > +	 * metadata buffer is retained after PAS_INIT. PAS remoteproc subsystems
> > +	 * set this flag so metadata persists until auth_and_reset() completes,
> > +	 * as TrustZone keeps the buffers locked until then. Other callers leave
> > +	 * it unset and metadata is freed immediately after the PAS_INIT call.
> 
> Please rewrite this to make it more succinct. It seems you could say the
> same thing with just the words "free the metadata on error or if client
> didn't request us to keep it".

Sure.

-- 
-Mukesh Ojha