[PATCH v4 03/12] firmware: qcom_scm: Introduce PAS context initialization and destroy helper

Mukesh Ojha posted 12 patches 4 months ago
There is a newer version of this series
[PATCH v4 03/12] firmware: qcom_scm: Introduce PAS context initialization and destroy helper
Posted by Mukesh Ojha 4 months ago
When the Peripheral Authentication Service (PAS) method runs on a SoC
where Linux operates at EL2 (i.e., without the Gunyah hypervisor), the
reset sequences are handled by TrustZone. In such cases, Linux must
perform additional steps before invoking PAS SMC calls, such as creating
a SHM bridge. Therefore, PAS SMC calls require awareness and handling of
these additional steps when Linux runs at EL2.

To support this, there is a need for a data structure that can be
initialized prior to invoking any SMC or MDT functions. This structure
allows those functions to determine whether they are operating in the
presence or absence of the Gunyah hypervisor and behave accordingly.

Currently, remoteproc and non-remoteproc subsystems use different
variants of the MDT loader helper API, primarily due to differences in
metadata context handling. Remoteproc subsystems retain the metadata
context until authentication and reset are completed, while
non-remoteproc subsystems (e.g., video, graphics, IPA, etc.) do not
retain the metadata context and can free it within the
qcom_scm_pas_init() call by passing a NULL context parameter and due to
these differences, it is not possible to extend metadata context
handling to support remoteproc and non remoteproc subsystem use PAS
operations, when Linux operates at EL2.

Add PAS context data structure and helper functions to initialize and
destroy it.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm.c       | 54 ++++++++++++++++++++++++++++++++++
 include/linux/firmware/qcom/qcom_scm.h | 11 +++++++
 2 files changed, 65 insertions(+)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 3379607eaf94..b8ce4fc34dbe 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -558,6 +558,60 @@ static void qcom_scm_set_download_mode(u32 dload_mode)
 		dev_err(__scm->dev, "failed to set download mode: %d\n", ret);
 }
 
+/**
+ * qcom_scm_pas_context_init() - Initialize peripheral authentication service
+ *				 context for a given peripheral and it can be
+ *				 destroyed with qcom_scm_pas_context_destroy()
+ *				 to release the context
+ *
+ * @dev:	  PAS firmware device
+ * @pas_id:	  peripheral authentication service id
+ * @mem_phys:	  Subsystem reserve memory start address
+ * @mem_size:	  Subsystem reserve memory size
+ *
+ * Upon successful, returns the PAS context or ERR_PTR() of the error otherwise.
+ */
+void *qcom_scm_pas_context_init(struct device *dev, u32 pas_id, phys_addr_t mem_phys,
+				size_t mem_size)
+{
+	struct qcom_scm_pas_context *ctx;
+
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return ERR_PTR(-ENOMEM);
+
+	ctx->dev = dev;
+	ctx->pas_id = pas_id;
+	ctx->mem_phys = mem_phys;
+	ctx->mem_size = mem_size;
+
+	ctx->metadata = kzalloc(sizeof(*ctx->metadata), GFP_KERNEL);
+	if (!ctx->metadata) {
+		kfree(ctx);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	return ctx;
+}
+EXPORT_SYMBOL_GPL(qcom_scm_pas_context_init);
+
+/**
+ * qcom_scm_pas_context_destroy() - release PAS context
+ *
+ * @ctx:	PAS context
+ */
+void qcom_scm_pas_context_destroy(struct qcom_scm_pas_context *ctx)
+{
+	kfree(ctx->metadata);
+	ctx->metadata = NULL;
+	ctx->dev = NULL;
+	ctx->pas_id = 0;
+	ctx->mem_phys = 0;
+	ctx->mem_size = 0;
+	kfree(ctx);
+}
+EXPORT_SYMBOL_GPL(qcom_scm_pas_context_destroy);
+
 /**
  * qcom_scm_pas_init_image() - Initialize peripheral authentication service
  *			       state machine for a given peripheral, using the
diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
index a13f703b16cd..e82fdc200df7 100644
--- a/include/linux/firmware/qcom/qcom_scm.h
+++ b/include/linux/firmware/qcom/qcom_scm.h
@@ -72,6 +72,17 @@ struct qcom_scm_pas_metadata {
 	ssize_t size;
 };
 
+struct qcom_scm_pas_context {
+	struct device *dev;
+	u32 pas_id;
+	phys_addr_t mem_phys;
+	size_t mem_size;
+	struct qcom_scm_pas_metadata *metadata;
+};
+
+void *qcom_scm_pas_context_init(struct device *dev, u32 pas_id, phys_addr_t mem_phys,
+				size_t mem_size);
+void qcom_scm_pas_context_destroy(struct qcom_scm_pas_context *ctx);
 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);

-- 
2.50.1
Re: [PATCH v4 03/12] firmware: qcom_scm: Introduce PAS context initialization and destroy helper
Posted by Manivannan Sadhasivam 4 months ago
On Tue, Oct 07, 2025 at 10:18:48PM +0530, Mukesh Ojha wrote:
> When the Peripheral Authentication Service (PAS) method runs on a SoC
> where Linux operates at EL2 (i.e., without the Gunyah hypervisor), the
> reset sequences are handled by TrustZone. In such cases, Linux must
> perform additional steps before invoking PAS SMC calls, such as creating
> a SHM bridge. Therefore, PAS SMC calls require awareness and handling of
> these additional steps when Linux runs at EL2.
> 
> To support this, there is a need for a data structure that can be
> initialized prior to invoking any SMC or MDT functions. This structure
> allows those functions to determine whether they are operating in the
> presence or absence of the Gunyah hypervisor and behave accordingly.
> 
> Currently, remoteproc and non-remoteproc subsystems use different
> variants of the MDT loader helper API, primarily due to differences in
> metadata context handling. Remoteproc subsystems retain the metadata
> context until authentication and reset are completed, while
> non-remoteproc subsystems (e.g., video, graphics, IPA, etc.) do not
> retain the metadata context and can free it within the
> qcom_scm_pas_init() call by passing a NULL context parameter and due to
> these differences, it is not possible to extend metadata context
> handling to support remoteproc and non remoteproc subsystem use PAS
> operations, when Linux operates at EL2.
> 
> Add PAS context data structure and helper functions to initialize and
> destroy it.
> 
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
>  drivers/firmware/qcom/qcom_scm.c       | 54 ++++++++++++++++++++++++++++++++++
>  include/linux/firmware/qcom/qcom_scm.h | 11 +++++++
>  2 files changed, 65 insertions(+)
> 
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 3379607eaf94..b8ce4fc34dbe 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -558,6 +558,60 @@ static void qcom_scm_set_download_mode(u32 dload_mode)
>  		dev_err(__scm->dev, "failed to set download mode: %d\n", ret);
>  }
>  
> +/**
> + * qcom_scm_pas_context_init() - Initialize peripheral authentication service
> + *				 context for a given peripheral and it can be
> + *				 destroyed with qcom_scm_pas_context_destroy()
> + *				 to release the context
> + *
> + * @dev:	  PAS firmware device
> + * @pas_id:	  peripheral authentication service id
> + * @mem_phys:	  Subsystem reserve memory start address
> + * @mem_size:	  Subsystem reserve memory size
> + *
> + * Upon successful, returns the PAS context or ERR_PTR() of the error otherwise.
> + */
> +void *qcom_scm_pas_context_init(struct device *dev, u32 pas_id, phys_addr_t mem_phys,
> +				size_t mem_size)
> +{
> +	struct qcom_scm_pas_context *ctx;
> +
> +	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return ERR_PTR(-ENOMEM);
> +
> +	ctx->dev = dev;
> +	ctx->pas_id = pas_id;
> +	ctx->mem_phys = mem_phys;
> +	ctx->mem_size = mem_size;
> +
> +	ctx->metadata = kzalloc(sizeof(*ctx->metadata), GFP_KERNEL);
> +	if (!ctx->metadata) {
> +		kfree(ctx);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	return ctx;
> +}
> +EXPORT_SYMBOL_GPL(qcom_scm_pas_context_init);
> +
> +/**
> + * qcom_scm_pas_context_destroy() - release PAS context
> + *
> + * @ctx:	PAS context
> + */
> +void qcom_scm_pas_context_destroy(struct qcom_scm_pas_context *ctx)
> +{
> +	kfree(ctx->metadata);
> +	ctx->metadata = NULL;
> +	ctx->dev = NULL;
> +	ctx->pas_id = 0;
> +	ctx->mem_phys = 0;
> +	ctx->mem_size = 0;

Why do you need to zero initialize these fields before freeing? Are they
carrying any sensitive data that warrants zero initialization?

- Mani

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH v4 03/12] firmware: qcom_scm: Introduce PAS context initialization and destroy helper
Posted by Bryan O'Donoghue 4 months ago
On 07/10/2025 22:23, Manivannan Sadhasivam wrote:
>> +void qcom_scm_pas_context_destroy(struct qcom_scm_pas_context *ctx)
>> +{
>> +	kfree(ctx->metadata);
>> +	ctx->metadata = NULL;
>> +	ctx->dev = NULL;
>> +	ctx->pas_id = 0;
>> +	ctx->mem_phys = 0;
>> +	ctx->mem_size = 0;
> Why do you need to zero initialize these fields before freeing? Are they
> carrying any sensitive data that warrants zero initialization?

Mukesh, have to say I don't think adding my RB to this patch is really 
warranted.

I gave review feedback that the above looked odd.

https://lore.kernel.org/linux-arm-msm/9139706a-708c-4be6-a994-120cce0cd0e6@linaro.org

Could you please drop my RB here, and fix the above in your next version.

Also please add me to the cc list for the whole series.

---
bod
Re: [PATCH v4 03/12] firmware: qcom_scm: Introduce PAS context initialization and destroy helper
Posted by Mukesh Ojha 4 months ago
On Tue, Oct 07, 2025 at 11:16:28PM +0100, Bryan O'Donoghue wrote:
> On 07/10/2025 22:23, Manivannan Sadhasivam wrote:
> > > +void qcom_scm_pas_context_destroy(struct qcom_scm_pas_context *ctx)
> > > +{
> > > +	kfree(ctx->metadata);
> > > +	ctx->metadata = NULL;
> > > +	ctx->dev = NULL;
> > > +	ctx->pas_id = 0;
> > > +	ctx->mem_phys = 0;
> > > +	ctx->mem_size = 0;
> > Why do you need to zero initialize these fields before freeing? Are they
> > carrying any sensitive data that warrants zero initialization?

Nothing special about the data.

> 
> Mukesh, have to say I don't think adding my RB to this patch is really
> warranted.
> 
> I gave review feedback that the above looked odd.
> 
> https://lore.kernel.org/linux-arm-msm/9139706a-708c-4be6-a994-120cce0cd0e6@linaro.org
> 
> Could you please drop my RB here, and fix the above in your next version.

Sorry if I misunderstood your comment on this particular patch.

I assumed your concern was regarding the manual destroy call, and I
responded to that point. Since I didn’t receive a follow-up on that, I
proceeded to address all other comments and added your Reviewed-by tag.

However, since we are revisiting this discussion, it seems appropriate
to remove the destroy function altogether and switch to using devm_
APIs.

> 
> Also please add me to the cc list for the whole series.

Will surely do it, thanks.

> 
> ---
> bod

-- 
-Mukesh Ojha