On Fri, Nov 07, 2025 at 11:04:56AM +0100, Konrad Dybcio wrote: > On 11/5/25 7:42 AM, Mukesh Ojha wrote: > > On Tue, Nov 04, 2025 at 06:33:49PM +0100, Konrad Dybcio wrote: > >> On 11/4/25 8:35 AM, Mukesh Ojha wrote: > >>> As a superset of the existing metadata context, the PAS context > >>> structure enables both remoteproc and non-remoteproc subsystems to > >>> better support scenarios where the SoC runs with or without the Gunyah > >>> hypervisor. To reflect this, relevant SCM and metadata functions are > >>> updated to incorporate PAS context awareness. > >>> > >>> Signed-off-by: Mukesh Ojha <[email protected]> > >>> --- > >>> drivers/firmware/qcom/qcom_scm.c | 25 +++++++++++++--------- > >>> drivers/remoteproc/qcom_q6v5_pas.c | 38 > >>> ++++++++++++++++++++++++---------- > >>> drivers/soc/qcom/mdt_loader.c | 4 ++-- > >>> include/linux/firmware/qcom/qcom_scm.h | 4 ++-- > >>> include/linux/soc/qcom/mdt_loader.h | 6 +++--- > >>> 5 files changed, 49 insertions(+), 28 deletions(-) > >>> > >>> diff --git a/drivers/firmware/qcom/qcom_scm.c > >>> b/drivers/firmware/qcom/qcom_scm.c > >>> index 5a525dbd0a2e..9cdd152da592 100644 > >>> --- a/drivers/firmware/qcom/qcom_scm.c > >>> +++ b/drivers/firmware/qcom/qcom_scm.c > >>> @@ -603,7 +603,7 @@ EXPORT_SYMBOL_GPL(devm_qcom_scm_pas_context_init); > >>> * and optional blob of data used for authenticating the > >>> metadata > >>> * and the rest of the firmware > >>> * @size: size of the metadata > >>> - * @ctx: optional metadata context > >>> + * @ctx: optional pas context > >>> * > >>> * Return: 0 on success. > >>> * > >>> @@ -612,8 +612,9 @@ EXPORT_SYMBOL_GPL(devm_qcom_scm_pas_context_init); > >>> * qcom_scm_pas_metadata_release() by the caller. > >>> */ > >>> int qcom_scm_pas_init_image(u32 pas_id, const void *metadata, size_t > >>> size, > >>> - struct qcom_scm_pas_metadata *ctx) > >>> + struct qcom_scm_pas_context *ctx) > >>> { > >>> + struct qcom_scm_pas_metadata *mdt_ctx; > >> > >> This is never initialized > >> > >>> dma_addr_t mdata_phys; > >>> void *mdata_buf; > >>> int ret; > >>> @@ -665,9 +666,10 @@ int qcom_scm_pas_init_image(u32 pas_id, const void > >>> *metadata, size_t size, > >>> if (ret < 0 || !ctx) { > >>> dma_free_coherent(__scm->dev, size, mdata_buf, mdata_phys); > >>> } else if (ctx) { > >>> - ctx->ptr = mdata_buf; > >>> - ctx->phys = mdata_phys; > >>> - ctx->size = size; > >>> + mdt_ctx = ctx->metadata; > >>> + mdt_ctx->ptr = mdata_buf; > >>> + mdt_ctx->phys = mdata_phys; > >>> + mdt_ctx->size = size; > >> > >> So this will always cause stack corruption > >> > >>> } > >>> > >>> return ret ? : res.result[0]; > >>> @@ -676,16 +678,19 @@ EXPORT_SYMBOL_GPL(qcom_scm_pas_init_image); > >>> > >>> /** > >>> * qcom_scm_pas_metadata_release() - release metadata context > >>> - * @ctx: metadata context > >>> + * @ctx: pas context > >>> */ > >>> -void qcom_scm_pas_metadata_release(struct qcom_scm_pas_metadata *ctx) > >>> +void qcom_scm_pas_metadata_release(struct qcom_scm_pas_context *ctx) > >>> { > >>> - if (!ctx->ptr) > >>> + struct qcom_scm_pas_metadata *mdt_ctx; > >> > >> Is the existence of this struct any useful after you introduced > >> pas_context? > > > > Yes, it is still useful, mdt_ctx is only relevant for remoteproc based > > subsystem like adsp, cdsp, modem while they are not required for video, > > ipa, gpu etc. but the superset which is pas_context is needed by > > whosoever need to support secure PAS method Linux at EL2. > > $ b4 shazam [email protected] > $ rg 'struct qcom_scm_pas_metadata' > > include/linux/firmware/qcom/qcom_scm.h > 69:struct qcom_scm_pas_metadata { > 80: struct qcom_scm_pas_metadata *metadata; > > drivers/firmware/qcom/qcom_scm.c > 636: struct qcom_scm_pas_metadata *mdt_ctx; > 680: struct qcom_scm_pas_metadata *mdt_ctx; > 728: struct qcom_scm_pas_metadata *mdt_ctx; > > So really it seems like it always exists as part of the pas_context.. > should we just make the larger struct integrate the smaller one and drop > the unnecessary layer? > > TBF I don't really insist on this, but it surely looks a little odd
You are right, we could remove 'struct qcom_scm_pas_metadata' completely and can add the fields of it to struct qcom_scm_pas_context. Will do it next spin. > > Konrad -- -Mukesh Ojha
