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
