[PATCH v6 05/14] remoteproc: pas: Replace metadata context with PAS context structure

Mukesh Ojha posted 14 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v6 05/14] remoteproc: pas: Replace metadata context with PAS context structure
Posted by Mukesh Ojha 1 month, 2 weeks ago
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 <mukesh.ojha@oss.qualcomm.com>
---
 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;
 	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;
 	}
 
 	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;
+
+	mdt_ctx = ctx->metadata;
+	if (!mdt_ctx->ptr)
 		return;
 
-	dma_free_coherent(__scm->dev, ctx->size, ctx->ptr, ctx->phys);
+	dma_free_coherent(__scm->dev, mdt_ctx->size, mdt_ctx->ptr, mdt_ctx->phys);
 
-	ctx->ptr = NULL;
+	mdt_ctx->ptr = NULL;
 }
 EXPORT_SYMBOL_GPL(qcom_scm_pas_metadata_release);
 
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 158bcd6cc85c..729e7b9bc183 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -117,8 +117,8 @@ struct qcom_pas {
 	struct qcom_rproc_ssr ssr_subdev;
 	struct qcom_sysmon *sysmon;
 
-	struct qcom_scm_pas_metadata pas_metadata;
-	struct qcom_scm_pas_metadata dtb_pas_metadata;
+	struct qcom_scm_pas_context *pas_ctx;
+	struct qcom_scm_pas_context *dtb_pas_ctx;
 };
 
 static void qcom_pas_segment_dump(struct rproc *rproc,
@@ -211,9 +211,9 @@ static int qcom_pas_unprepare(struct rproc *rproc)
 	 * auth_and_reset() was successful, but in other cases clean it up
 	 * here.
 	 */
-	qcom_scm_pas_metadata_release(&pas->pas_metadata);
+	qcom_scm_pas_metadata_release(pas->pas_ctx);
 	if (pas->dtb_pas_id)
-		qcom_scm_pas_metadata_release(&pas->dtb_pas_metadata);
+		qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
 
 	return 0;
 }
@@ -241,7 +241,7 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
 
 		ret = qcom_mdt_pas_init(pas->dev, pas->dtb_firmware, pas->dtb_firmware_name,
 					pas->dtb_pas_id, pas->dtb_mem_phys,
-					&pas->dtb_pas_metadata);
+					pas->dtb_pas_ctx);
 		if (ret)
 			goto release_dtb_firmware;
 
@@ -255,7 +255,7 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
 	return 0;
 
 release_dtb_metadata:
-	qcom_scm_pas_metadata_release(&pas->dtb_pas_metadata);
+	qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
 
 release_dtb_firmware:
 	release_firmware(pas->dtb_firmware);
@@ -306,7 +306,7 @@ static int qcom_pas_start(struct rproc *rproc)
 	}
 
 	ret = qcom_mdt_pas_init(pas->dev, pas->firmware, rproc->firmware, pas->pas_id,
-				pas->mem_phys, &pas->pas_metadata);
+				pas->mem_phys, pas->pas_ctx);
 	if (ret)
 		goto disable_px_supply;
 
@@ -332,9 +332,9 @@ static int qcom_pas_start(struct rproc *rproc)
 		goto release_pas_metadata;
 	}
 
-	qcom_scm_pas_metadata_release(&pas->pas_metadata);
+	qcom_scm_pas_metadata_release(pas->pas_ctx);
 	if (pas->dtb_pas_id)
-		qcom_scm_pas_metadata_release(&pas->dtb_pas_metadata);
+		qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
 
 	/* firmware is used to pass reference from qcom_pas_start(), drop it now */
 	pas->firmware = NULL;
@@ -342,9 +342,9 @@ static int qcom_pas_start(struct rproc *rproc)
 	return 0;
 
 release_pas_metadata:
-	qcom_scm_pas_metadata_release(&pas->pas_metadata);
+	qcom_scm_pas_metadata_release(pas->pas_ctx);
 	if (pas->dtb_pas_id)
-		qcom_scm_pas_metadata_release(&pas->dtb_pas_metadata);
+		qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
 disable_px_supply:
 	if (pas->px_supply)
 		regulator_disable(pas->px_supply);
@@ -779,6 +779,22 @@ static int qcom_pas_probe(struct platform_device *pdev)
 	}
 
 	qcom_add_ssr_subdev(rproc, &pas->ssr_subdev, desc->ssr_name);
+
+	pas->pas_ctx = devm_qcom_scm_pas_context_init(pas->dev, pas->pas_id,
+						      pas->mem_phys, pas->mem_size);
+	if (IS_ERR(pas->pas_ctx)) {
+		ret = PTR_ERR(pas->pas_ctx);
+		goto remove_ssr_sysmon;
+	}
+
+	pas->dtb_pas_ctx = devm_qcom_scm_pas_context_init(pas->dev, pas->dtb_pas_id,
+							  pas->dtb_mem_phys,
+							  pas->dtb_mem_size);
+	if (IS_ERR(pas->dtb_pas_ctx)) {
+		ret = PTR_ERR(pas->dtb_pas_ctx);
+		goto remove_ssr_sysmon;
+	}
+
 	ret = rproc_add(rproc);
 	if (ret)
 		goto remove_ssr_sysmon;
diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
index a5c80d4fcc36..fe35038c5342 100644
--- a/drivers/soc/qcom/mdt_loader.c
+++ b/drivers/soc/qcom/mdt_loader.c
@@ -234,13 +234,13 @@ EXPORT_SYMBOL_GPL(qcom_mdt_read_metadata);
  * @fw_name:	name of the firmware, for construction of segment file names
  * @pas_id:	PAS identifier
  * @mem_phys:	physical address of allocated memory region
- * @ctx:	PAS metadata context, to be released by caller
+ * @ctx:	PAS context, ctx->metadata to be released by caller
  *
  * Returns 0 on success, negative errno otherwise.
  */
 int qcom_mdt_pas_init(struct device *dev, const struct firmware *fw,
 		      const char *fw_name, int pas_id, phys_addr_t mem_phys,
-		      struct qcom_scm_pas_metadata *ctx)
+		      struct qcom_scm_pas_context *ctx)
 {
 	const struct elf32_phdr *phdrs;
 	const struct elf32_phdr *phdr;
diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
index f574fdf88eaa..2905587dcd17 100644
--- a/include/linux/firmware/qcom/qcom_scm.h
+++ b/include/linux/firmware/qcom/qcom_scm.h
@@ -83,8 +83,8 @@ struct qcom_scm_pas_context {
 void *devm_qcom_scm_pas_context_init(struct device *dev, u32 pas_id, phys_addr_t mem_phys,
 				     size_t mem_size);
 int qcom_scm_pas_init_image(u32 pas_id, const void *metadata, size_t size,
-			    struct qcom_scm_pas_metadata *ctx);
-void qcom_scm_pas_metadata_release(struct qcom_scm_pas_metadata *ctx);
+			    struct qcom_scm_pas_context *ctx);
+void qcom_scm_pas_metadata_release(struct qcom_scm_pas_context *ctx);
 int qcom_scm_pas_mem_setup(u32 pas_id, phys_addr_t addr, phys_addr_t size);
 int qcom_scm_pas_auth_and_reset(u32 pas_id);
 int qcom_scm_pas_shutdown(u32 pas_id);
diff --git a/include/linux/soc/qcom/mdt_loader.h b/include/linux/soc/qcom/mdt_loader.h
index 8ea8230579a2..07c278841816 100644
--- a/include/linux/soc/qcom/mdt_loader.h
+++ b/include/linux/soc/qcom/mdt_loader.h
@@ -10,14 +10,14 @@
 
 struct device;
 struct firmware;
-struct qcom_scm_pas_metadata;
+struct qcom_scm_pas_context;
 
 #if IS_ENABLED(CONFIG_QCOM_MDT_LOADER)
 
 ssize_t qcom_mdt_get_size(const struct firmware *fw);
 int qcom_mdt_pas_init(struct device *dev, const struct firmware *fw,
 		      const char *fw_name, int pas_id, phys_addr_t mem_phys,
-		      struct qcom_scm_pas_metadata *pas_metadata_ctx);
+		      struct qcom_scm_pas_context *pas_ctx);
 int qcom_mdt_load(struct device *dev, const struct firmware *fw,
 		  const char *fw_name, int pas_id, void *mem_region,
 		  phys_addr_t mem_phys, size_t mem_size,
@@ -39,7 +39,7 @@ static inline ssize_t qcom_mdt_get_size(const struct firmware *fw)
 
 static inline int qcom_mdt_pas_init(struct device *dev, const struct firmware *fw,
 				    const char *fw_name, int pas_id, phys_addr_t mem_phys,
-				    struct qcom_scm_pas_metadata *pas_metadata_ctx)
+				    struct qcom_scm_pas_context *pas_ctx)
 {
 	return -ENODEV;
 }

-- 
2.50.1
Re: [PATCH v6 05/14] remoteproc: pas: Replace metadata context with PAS context structure
Posted by Konrad Dybcio 1 month, 2 weeks ago
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 <mukesh.ojha@oss.qualcomm.com>
> ---
>  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?

Konrad
Re: [PATCH v6 05/14] remoteproc: pas: Replace metadata context with PAS context structure
Posted by Mukesh Ojha 1 month, 2 weeks ago
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 <mukesh.ojha@oss.qualcomm.com>
> > ---
> >  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.

> 
> Konrad

-- 
-Mukesh Ojha
Re: [PATCH v6 05/14] remoteproc: pas: Replace metadata context with PAS context structure
Posted by Konrad Dybcio 1 month, 1 week ago
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 <mukesh.ojha@oss.qualcomm.com>
>>> ---
>>>  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 20251104-kvm_rproc_v6-v6-0-7017b0adc24e@oss.qualcomm.com
$ 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
Re: [PATCH v6 05/14] remoteproc: pas: Replace metadata context with PAS context structure
Posted by Mukesh Ojha 1 month, 1 week ago
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 <mukesh.ojha@oss.qualcomm.com>
> >>> ---
> >>>  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 20251104-kvm_rproc_v6-v6-0-7017b0adc24e@oss.qualcomm.com
> $ 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
Re: [PATCH v6 05/14] remoteproc: pas: Replace metadata context with PAS context structure
Posted by Konrad Dybcio 1 month, 2 weeks ago
On 11/4/25 6:33 PM, 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 <mukesh.ojha@oss.qualcomm.com>
>> ---
>>  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

Pardon, I'm blind

Konrad