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 Konrad
