On Mon, Aug 03, 2026 at 02:36:45PM +0530, Mukesh Ojha wrote: > 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. >
How on earth do you expect me to figure that out!? Regards, Bjorn > > > > > 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
