On 20/09/2025 20:41, Mukesh Ojha wrote:
When Secure Peripheral Authentication Service (PAS) method runs on a
SoC where Linux runs at EL2 (Gunyah absence) where reset sequences

"i.e. runs without the Gynyah Hypervisor then, reset sequences"

move to EL3 and Linux need to do some extra stuff before calling PAS
SMC calls like creating SHMbridge. So, PAS SMC call need awareness and
need handling of things required when Linux run at EL2.

"Therefore the PAS SMC call"


Currently, remoteproc and non-remoteproc subsystems use different

"Currently remoteproc"

variants of the MDT loader helper API, primarily due to the handling of
the metadata context. Remoteproc subsystems retain metadata context
until authentication and reset is done, while non-remoteproc subsystems
(e.g., video, graphics, ipa etc.) do not need to retain it and can free

"do not need to retain metadata context"

the context right inside qcom_scm_pas_init() call based on passed context
parameter as NULL.

So, in an attempt to unify the metadata API process for both remoteproc

"In an attempt to unify"

and non-remoteproc subsystems and to make the SMC helper function
cleaner whether SoC running with Gunyah presence or absence by introducing
a dedicated PAS context initialization and destroy function. Context
initialization beforehand would help all SMC function to carry it and do
the right thing whether SoC is running with Gunyah presence or absence.

Since you need to do another version of this patch re: below, please tidy up the commit log here a bit too.

Signed-off-by: Mukesh Ojha <mukesh.o...@oss.qualcomm.com>
---
  drivers/firmware/qcom/qcom_scm.c       | 53 ++++++++++++++++++++++++++++++++++
  include/linux/firmware/qcom/qcom_scm.h | 11 +++++++
  2 files changed, 64 insertions(+)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 3379607eaf94..1c6b4c6f5513 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -558,6 +558,59 @@ 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_ctx_init() - Initialize peripheral authentication service
+ *                          context for a given peripheral and it can be
+ *                          destroyed with qcom_scm_pas_ctx_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_ctx_init(struct device *dev, u32 pas_id, phys_addr_t 
mem_phys,
+                           size_t mem_size)
+{
+       struct qcom_scm_pas_ctx *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_ctx_init);
+
+/**
+ * qcom_scm_pas_ctx_destroy() - release PAS context
+ * @ctx:       PAS context
+ */
+void qcom_scm_pas_ctx_destroy(struct qcom_scm_pas_ctx *ctx)
+{
+       kfree(ctx->metadata);
+       ctx->metadata = NULL;
+       ctx->dev = NULL;
+       ctx->pas_id = 0;
+       ctx->mem_phys = 0;
+       ctx->mem_size = 0;
+       kfree(ctx);
+}

This looks a bit strange, manually destructing an object you then free. I get the argument you might make about use-after-free but, I don't think this level of defensive coding is necessary.

+EXPORT_SYMBOL_GPL(qcom_scm_pas_ctx_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..e3e9e9e9077f 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_ctx {
+       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_ctx_init(struct device *dev, u32 pas_id, phys_addr_t 
mem_phys,
+                           size_t mem_size);
+void qcom_scm_pas_ctx_destroy(struct qcom_scm_pas_ctx *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



Once fixed.

Reviewed-by: Bryan O'Donoghue <bryan.odonog...@linaro.org>

---
bod

Reply via email to