[PATCH v8 00/14] Peripheral Image Loader support for Qualcomm SoCs running Linux host at EL2

[PATCH v8 00/14] Peripheral Image Loader support for Qualcomm SoCs running Linux host at EL2

[Mukesh Ojha]

In May 2025, we discussed the challenges at Linaro Connect 2025 [1] 
related to Secure PAS remoteproc enablement when Linux is running at EL2
for Qualcomm SoCs.

[1] https://resources.linaro.org/en/resource/sF8jXifdb9V1mUefdbfafa

Below, is the summary of the discussion.

Qualcomm is working to enable remote processors on the SA8775p SoC with
a Linux host running at EL2. In doing so, it has encountered several
challenges related to how the remoteproc framework is handled when Linux
runs at EL1.

One of the main challenges arises from differences in how IOMMU
translation is currently managed on SoCs running the Qualcomm EL2
hypervisor (QHEE), where IOMMU translation for any device is entirely
owned by the hypervisor. Additionally, the firmware for remote
processors does not contain a resource table, which would typically
include the necessary IOMMU configuration settings.

Qualcomm SoCs running with QHEE (EL2) have been utilizing the Peripheral
Authentication Service (PAS) from TrustZone (TZ) firmware to securely
authenticate and reset remote processors via a single SMC call,
_auth_and_reset_. This call is first trapped by QHEE, which then invokes
TZ for authentication. Once authentication is complete, the call returns
to QHEE, which sets up the IOMMU translation scheme for the remote
processors and subsequently brings them out of reset. The design of the
Qualcomm EL2 hypervisor dictates that the Linux host OS running at EL1
is not permitted to configure IOMMU translation for remote processors,
and only a single-stage translation is configured.

To make the remote processor bring-up (PAS) sequence
hypervisor-independent, the auth_and_reset SMC call is now handled
entirely by TZ. However, the issue of IOMMU configuration remains
unresolved, for example a scenario, when KVM host at EL2 has no
knowledge of the remote processors’ IOMMU settings.  This is being
addressed by overlaying the IOMMU properties when the SoC runs a Linux
host at EL2. SMC call is being provided from the TrustZone firmware to
retrieve the resource table for a given subsystem.

There are also remote processors such as those for video, camera, and
graphics that do not use the remoteproc framework to manage their
lifecycle. Instead, they rely on the Qualcomm PAS service to
authenticate their firmware. These processors also need to be brought
out of reset when Linux is running at EL2. The client drivers for these
processors use the MDT loader function to load and authenticate
firmware. Similar to the Qualcomm remoteproc PAS driver, they also need
to retrieve the resource table, create a shared memory bridge
(shmbridge), and map the resources before bringing the processors out of
reset.

It is based on next-20251120 and tested on SA8775p which is now called
Lemans IOT platform and does not addresses DMA problem discussed at
[1] which is future scope of the series.

Changes in v8: https://lore.kernel.org/lkml/20251113-kvm-rproc-v7-v7-0-df4910b7c20a@oss.qualcomm.com/
 - Addressed suggestion from Stephen which was regarding commit message(9/14),
   debug log(12/14) suggestion, and return type change(4/14).
 - Added R-b tag on 10/14 .

Changes in v7: https://lore.kernel.org/lkml/20251104-kvm_rproc_v6-v6-0-7017b0adc24e@oss.qualcomm.com/
 - Addressed Bryan suggestion to modify commit message on 2/14 .
 - Repharsed commit message based on Krzysztof comment made on 1/14.
 - Addressed Konrad suggestion 
	o To remove pas metadata data structure.
	o Added his suggestion on adding retry in rsc_table SCM call.
	o Added his rephrased version of comment made in 12/14 

Changes in v6: https://lore.kernel.org/lkml/20251013-kvm_rprocv5-v5-0-d609ed766061@oss.qualcomm.com/
 - Added comments made by Bryan to save some cycles and added in 2/14
   which could change patch order.
 - Renamed qcom_scm_pas_context_init to devm_qcom_scm_pas_context_init()
 - Replaced devm_kzalloc with kzalloc for output_rt in scm function as
   remoteproc framework does not usage devm_ api for resource table
   pointer which is cause mem-abort issue start/stop test.
 - Removed union usage and redundant code from qcom_scm_pas_prep_and_init_image().
   
Changes in v5: https://lore.kernel.org/lkml/20251007-kvm_rprocv4_next-20251007-v4-0-de841623af3c@oss.qualcomm.com/
 - Replaced minitems with maxitems.
 - Addressed comments made by Bryan, Mani and Konrad.
 - Fixed some of highlighted issues in v4.
 - Added a new patch which removes qcom_mdt_pas_init() from exported
   symbol list.
 - Slight change in  v4's 5/12, so that it does use qcom_mdt_pas_load()
   directly while it should use in the commit where it gets introduced.
   Hence, reordered the patches a bit like v4 5/12 comes early before
   4/12.

Changes in v4: https://lore.kernel.org/lkml/20250921-kvm_rproc_pas-v3-0-458f09647920@oss.qualcomm.com/
 - Fixed kernel robot warning/errors.
 - Reworded some of the commit log, code comment as per suggestion from Bryan.
 - Added support of gpdsp0 and gpdsp1 and disabled iris node.
 - Add R-b tag to some of the reviewed patches.
 - Rename struct qcom_scm_pas_cxt to qcom_scm_pas_context.

Changes in v3: https://lore.kernel.org/lkml/20250819165447.4149674-1-mukesh.ojha@oss.qualcomm.com/
 - Dropped video subsystem enablement for now, could add it in future
   or on a separate series.
 - Addressed most of the suggestion from Stephen and Bryan like some
   remoteproc code checking resource table presence or right error
   code propagation above the layer.
 - Added leman-el2 overlay file.
 - Added missed iommus binding which was missed last series.
 - Separated qcom_mdt_pas_load() patch and its usage.
 - Patch numbering got changed compared to last version

Changes in v2: https://lore.kernel.org/lkml/20241004212359.2263502-1-quic_mojha@quicinc.com/
 - A lot has changed from the V1 and a fresh look would be preferred.
 - Removed approach where device tree contain devmem resources in
   remoteproc node.
 - SHMbridge need to created for both carveout and metadata memory
   shared to TZ in a new way.
 - Now, resource table would be given by SMC call which need to mapped
   along with carveout before triggering _auth_and_reset_.
 - IOMMU properties need to be added to firmware devices tree node when Linux
   control IOMMU.

---
Mukesh Ojha (14):
      dt-bindings: remoteproc: qcom,pas: Add iommus property
      firmware: qcom_scm: Remove redundant piece of code
      firmware: qcom_scm: Rename peripheral as pas_id
      firmware: qcom_scm: Introduce PAS context initialization helper function
      remoteproc: pas: Replace metadata context with PAS context structure
      soc: qcom: mdtloader: Add PAS context aware qcom_mdt_pas_load() function
      soc: qcom: mdtloader: Remove qcom_mdt_pas_init() from exported symbols
      firmware: qcom_scm: Add a prep version of auth_and_reset function
      firmware: qcom_scm: Refactor qcom_scm_pas_init_image()
      firmware: qcom_scm: Add SHM bridge handling for PAS when running without QHEE
      firmware: qcom_scm: Add qcom_scm_pas_get_rsc_table() to get resource table
      remoteproc: pas: Extend parse_fw callback to fetch resources via SMC call
      remoteproc: qcom: pas: Enable Secure PAS support with IOMMU managed by Linux
      arm64: dts: qcom: Add EL2 overlay for Lemans

 .../bindings/remoteproc/qcom,pas-common.yaml       |   3 +
 arch/arm64/boot/dts/qcom/Makefile                  |  10 +
 arch/arm64/boot/dts/qcom/lemans-el2.dtso           |  41 +++
 drivers/firmware/qcom/qcom_scm.c                   | 368 ++++++++++++++++++---
 drivers/firmware/qcom/qcom_scm.h                   |   1 +
 drivers/remoteproc/qcom_q6v5_pas.c                 | 166 ++++++++--
 drivers/soc/qcom/mdt_loader.c                      |  42 ++-
 include/linux/firmware/qcom/qcom_scm.h             |  30 +-
 include/linux/soc/qcom/mdt_loader.h                |  22 +-
 9 files changed, 577 insertions(+), 106 deletions(-)
---
base-commit: 88cbd8ac379cf5ce68b7efcfd4d1484a6871ee0b
change-id: 20251121-kvm_rproc_v8-5618000321fb

Best regards,
-- 
-Mukesh Ojha

[PATCH v8 01/14] dt-bindings: remoteproc: qcom,pas: Add iommus property

[Mukesh Ojha]

On most Qualcomm platforms, the Qualcomm Hypervisor Execution
Environment (QHEE) is either used as a standalone hypervisor or
co-exists as a module with the Gunyah hypervisor. When QHEE is
present, it configures firmware streams for remote processors.
If QHEE is not available, the operating system must perform
these configurations instead and for that remote processor
firmware stream should be presented to the operating system.
To support this, add the iommus property as an optional property
for PAS-supported devices.

Acked-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
index 63a82e7a8bf8..68c17bf18987 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
@@ -44,6 +44,9 @@ properties:
       - const: stop-ack
       - const: shutdown-ack
 
+  iommus:
+    maxItems: 1
+
   power-domains:
     minItems: 1
     maxItems: 3

-- 
2.50.1

[PATCH v8 02/14] firmware: qcom_scm: Remove redundant piece of code

[Mukesh Ojha]

The qcom_scm_pas_metadata_release() function already frees the allocated
memory and sets ctx->ptr to NULL. Resetting ctx->phys and ctx->size to
zero is unnecessary because the context is expected to be discarded
after release.

Lets removes redundant assignments.

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index e777b7cb9b12..26969bcd763c 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -650,8 +650,6 @@ void qcom_scm_pas_metadata_release(struct qcom_scm_pas_metadata *ctx)
 	dma_free_coherent(__scm->dev, ctx->size, ctx->ptr, ctx->phys);
 
 	ctx->ptr = NULL;
-	ctx->phys = 0;
-	ctx->size = 0;
 }
 EXPORT_SYMBOL_GPL(qcom_scm_pas_metadata_release);
 

-- 
2.50.1

[PATCH v8 03/14] firmware: qcom_scm: Rename peripheral as pas_id

[Mukesh Ojha]

Peripheral and pas_id refers to unique id for a subsystem and used only
when peripheral authentication service from secure world is utilized.

Lets rename peripheral to pas_id to reflect closer to its meaning.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm.c       | 30 +++++++++++++++---------------
 include/linux/firmware/qcom/qcom_scm.h | 10 +++++-----
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 26969bcd763c..0a0c48fca7cf 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -562,7 +562,7 @@ static void qcom_scm_set_download_mode(u32 dload_mode)
  * qcom_scm_pas_init_image() - Initialize peripheral authentication service
  *			       state machine for a given peripheral, using the
  *			       metadata
- * @peripheral: peripheral id
+ * @pas_id:	peripheral authentication service id
  * @metadata:	pointer to memory containing ELF header, program header table
  *		and optional blob of data used for authenticating the metadata
  *		and the rest of the firmware
@@ -575,7 +575,7 @@ static void qcom_scm_set_download_mode(u32 dload_mode)
  * track the metadata allocation, this needs to be released by invoking
  * qcom_scm_pas_metadata_release() by the caller.
  */
-int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size,
+int qcom_scm_pas_init_image(u32 pas_id, const void *metadata, size_t size,
 			    struct qcom_scm_pas_metadata *ctx)
 {
 	dma_addr_t mdata_phys;
@@ -585,7 +585,7 @@ int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size,
 		.svc = QCOM_SCM_SVC_PIL,
 		.cmd = QCOM_SCM_PIL_PAS_INIT_IMAGE,
 		.arginfo = QCOM_SCM_ARGS(2, QCOM_SCM_VAL, QCOM_SCM_RW),
-		.args[0] = peripheral,
+		.args[0] = pas_id,
 		.owner = ARM_SMCCC_OWNER_SIP,
 	};
 	struct qcom_scm_res res;
@@ -656,20 +656,20 @@ EXPORT_SYMBOL_GPL(qcom_scm_pas_metadata_release);
 /**
  * qcom_scm_pas_mem_setup() - Prepare the memory related to a given peripheral
  *			      for firmware loading
- * @peripheral:	peripheral id
+ * @pas_id:	peripheral authentication service id
  * @addr:	start address of memory area to prepare
  * @size:	size of the memory area to prepare
  *
  * Returns 0 on success.
  */
-int qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size)
+int qcom_scm_pas_mem_setup(u32 pas_id, phys_addr_t addr, phys_addr_t size)
 {
 	int ret;
 	struct qcom_scm_desc desc = {
 		.svc = QCOM_SCM_SVC_PIL,
 		.cmd = QCOM_SCM_PIL_PAS_MEM_SETUP,
 		.arginfo = QCOM_SCM_ARGS(3),
-		.args[0] = peripheral,
+		.args[0] = pas_id,
 		.args[1] = addr,
 		.args[2] = size,
 		.owner = ARM_SMCCC_OWNER_SIP,
@@ -697,18 +697,18 @@ EXPORT_SYMBOL_GPL(qcom_scm_pas_mem_setup);
 /**
  * qcom_scm_pas_auth_and_reset() - Authenticate the given peripheral firmware
  *				   and reset the remote processor
- * @peripheral:	peripheral id
+ * @pas_id:	peripheral authentication service id
  *
  * Return 0 on success.
  */
-int qcom_scm_pas_auth_and_reset(u32 peripheral)
+int qcom_scm_pas_auth_and_reset(u32 pas_id)
 {
 	int ret;
 	struct qcom_scm_desc desc = {
 		.svc = QCOM_SCM_SVC_PIL,
 		.cmd = QCOM_SCM_PIL_PAS_AUTH_AND_RESET,
 		.arginfo = QCOM_SCM_ARGS(1),
-		.args[0] = peripheral,
+		.args[0] = pas_id,
 		.owner = ARM_SMCCC_OWNER_SIP,
 	};
 	struct qcom_scm_res res;
@@ -733,18 +733,18 @@ EXPORT_SYMBOL_GPL(qcom_scm_pas_auth_and_reset);
 
 /**
  * qcom_scm_pas_shutdown() - Shut down the remote processor
- * @peripheral: peripheral id
+ * @pas_id:	peripheral authentication service id
  *
  * Returns 0 on success.
  */
-int qcom_scm_pas_shutdown(u32 peripheral)
+int qcom_scm_pas_shutdown(u32 pas_id)
 {
 	int ret;
 	struct qcom_scm_desc desc = {
 		.svc = QCOM_SCM_SVC_PIL,
 		.cmd = QCOM_SCM_PIL_PAS_SHUTDOWN,
 		.arginfo = QCOM_SCM_ARGS(1),
-		.args[0] = peripheral,
+		.args[0] = pas_id,
 		.owner = ARM_SMCCC_OWNER_SIP,
 	};
 	struct qcom_scm_res res;
@@ -770,18 +770,18 @@ EXPORT_SYMBOL_GPL(qcom_scm_pas_shutdown);
 /**
  * qcom_scm_pas_supported() - Check if the peripheral authentication service is
  *			      available for the given peripherial
- * @peripheral:	peripheral id
+ * @pas_id:	peripheral authentication service id
  *
  * Returns true if PAS is supported for this peripheral, otherwise false.
  */
-bool qcom_scm_pas_supported(u32 peripheral)
+bool qcom_scm_pas_supported(u32 pas_id)
 {
 	int ret;
 	struct qcom_scm_desc desc = {
 		.svc = QCOM_SCM_SVC_PIL,
 		.cmd = QCOM_SCM_PIL_PAS_IS_SUPPORTED,
 		.arginfo = QCOM_SCM_ARGS(1),
-		.args[0] = peripheral,
+		.args[0] = pas_id,
 		.owner = ARM_SMCCC_OWNER_SIP,
 	};
 	struct qcom_scm_res res;
diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
index a55ca771286b..a13f703b16cd 100644
--- a/include/linux/firmware/qcom/qcom_scm.h
+++ b/include/linux/firmware/qcom/qcom_scm.h
@@ -72,13 +72,13 @@ struct qcom_scm_pas_metadata {
 	ssize_t size;
 };
 
-int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t 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);
-int qcom_scm_pas_mem_setup(u32 peripheral, phys_addr_t addr, phys_addr_t size);
-int qcom_scm_pas_auth_and_reset(u32 peripheral);
-int qcom_scm_pas_shutdown(u32 peripheral);
-bool qcom_scm_pas_supported(u32 peripheral);
+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);
+bool qcom_scm_pas_supported(u32 pas_id);
 
 int qcom_scm_io_readl(phys_addr_t addr, unsigned int *val);
 int qcom_scm_io_writel(phys_addr_t addr, unsigned int val);

-- 
2.50.1

[PATCH v8 04/14] firmware: qcom_scm: Introduce PAS context initialization helper function

[Mukesh Ojha]

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 initialization helper function.

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

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 0a0c48fca7cf..e9e9d03ea36a 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -558,6 +558,40 @@ static void qcom_scm_set_download_mode(u32 dload_mode)
 		dev_err(__scm->dev, "failed to set download mode: %d\n", ret);
 }
 
+/**
+ * devm_qcom_scm_pas_context_init() - Initialize peripheral authentication service
+ *				      context for a given peripheral
+ *
+ * PAS context is device-resource managed, so the caller does not need
+ * to worry about freeing the context memory.
+ *
+ * @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.
+ */
+struct qcom_scm_pas_context *devm_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 = devm_kzalloc(dev, 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;
+
+	return ctx;
+}
+EXPORT_SYMBOL_GPL(devm_qcom_scm_pas_context_init);
+
 /**
  * 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..4a57fa676513 100644
--- a/include/linux/firmware/qcom/qcom_scm.h
+++ b/include/linux/firmware/qcom/qcom_scm.h
@@ -72,6 +72,20 @@ 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;
+	void *ptr;
+	dma_addr_t phys;
+	ssize_t size;
+};
+
+struct qcom_scm_pas_context *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);

-- 
2.50.1

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

[Mukesh Ojha]

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 and remove metadata context
data structure completely.

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm.c       |  8 +++----
 drivers/remoteproc/qcom_q6v5_pas.c     | 38 ++++++++++++++++++++++++----------
 drivers/soc/qcom/mdt_loader.c          |  4 ++--
 include/linux/firmware/qcom/qcom_scm.h | 10 ++-------
 include/linux/soc/qcom/mdt_loader.h    |  6 +++---
 5 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index e9e9d03ea36a..5fa974683ee0 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -601,7 +601,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.
  *
@@ -610,7 +610,7 @@ 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)
 {
 	dma_addr_t mdata_phys;
 	void *mdata_buf;
@@ -674,9 +674,9 @@ 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)
 		return;
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 c239107cb930..b125140100db 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 4a57fa676513..b10b1aeb32c6 100644
--- a/include/linux/firmware/qcom/qcom_scm.h
+++ b/include/linux/firmware/qcom/qcom_scm.h
@@ -66,12 +66,6 @@ int qcom_scm_set_warm_boot_addr(void *entry);
 void qcom_scm_cpu_power_down(u32 flags);
 int qcom_scm_set_remote_state(u32 state, u32 id);
 
-struct qcom_scm_pas_metadata {
-	void *ptr;
-	dma_addr_t phys;
-	ssize_t size;
-};
-
 struct qcom_scm_pas_context {
 	struct device *dev;
 	u32 pas_id;
@@ -87,8 +81,8 @@ struct qcom_scm_pas_context *devm_qcom_scm_pas_context_init(struct device *dev,
 							    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

[PATCH v8 06/14] soc: qcom: mdtloader: Add PAS context aware qcom_mdt_pas_load() function

[Mukesh Ojha]

Introduce a new PAS context-aware function, qcom_mdt_pas_load(), for
remote processor drivers. This function utilizes the PAS context
pointer returned from qcom_scm_pas_ctx_init() to perform firmware
metadata verification and memory setup via SMC calls.

The qcom_mdt_pas_load() and qcom_mdt_load() functions are largely
similar, but the former is designed for clients using the PAS
context-based data structure. Over time, all users of qcom_mdt_load()
can be migrated to use qcom_mdt_pas_load() for consistency and
improved abstraction.

As the remoteproc PAS driver (qcom_q6v5_pas) has already adopted the
PAS context-based approach, update it to use qcom_mdt_pas_load().

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c  | 24 +++++-------------------
 drivers/soc/qcom/mdt_loader.c       | 31 +++++++++++++++++++++++++++++++
 include/linux/soc/qcom/mdt_loader.h | 10 ++++++++++
 3 files changed, 46 insertions(+), 19 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 729e7b9bc183..9feee2cb1883 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -239,15 +239,9 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
 			return ret;
 		}
 
-		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_ctx);
-		if (ret)
-			goto release_dtb_firmware;
-
-		ret = qcom_mdt_load_no_init(pas->dev, pas->dtb_firmware, pas->dtb_firmware_name,
-					    pas->dtb_mem_region, pas->dtb_mem_phys,
-					    pas->dtb_mem_size, &pas->dtb_mem_reloc);
+		ret = qcom_mdt_pas_load(pas->dtb_pas_ctx, pas->dtb_firmware,
+					pas->dtb_firmware_name, pas->dtb_mem_region,
+					&pas->dtb_mem_reloc);
 		if (ret)
 			goto release_dtb_metadata;
 	}
@@ -256,8 +250,6 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
 
 release_dtb_metadata:
 	qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
-
-release_dtb_firmware:
 	release_firmware(pas->dtb_firmware);
 
 	return ret;
@@ -305,14 +297,8 @@ 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_ctx);
-	if (ret)
-		goto disable_px_supply;
-
-	ret = qcom_mdt_load_no_init(pas->dev, pas->firmware, rproc->firmware,
-				    pas->mem_region, pas->mem_phys, pas->mem_size,
-				    &pas->mem_reloc);
+	ret = qcom_mdt_pas_load(pas->pas_ctx, pas->firmware, rproc->firmware,
+				pas->mem_region, &pas->mem_reloc);
 	if (ret)
 		goto release_pas_metadata;
 
diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
index b125140100db..50c6a3c6b2a3 100644
--- a/drivers/soc/qcom/mdt_loader.c
+++ b/drivers/soc/qcom/mdt_loader.c
@@ -478,5 +478,36 @@ int qcom_mdt_load(struct device *dev, const struct firmware *fw,
 }
 EXPORT_SYMBOL_GPL(qcom_mdt_load);
 
+/**
+ * qcom_mdt_pas_load - Loads and authenticates the metadata of the firmware
+ * (typically contained in the .mdt file), followed by loading the actual
+ * firmware segments (e.g., .bXX files). Authentication of the segments done
+ * by a separate call.
+ *
+ * The PAS context must be initialized using qcom_scm_pas_context_init()
+ * prior to invoking this function.
+ *
+ * @ctx:        Pointer to the PAS (Peripheral Authentication Service) context
+ * @fw:         Firmware object representing the .mdt file
+ * @firmware:   Name of the firmware used to construct segment file names
+ * @mem_region: Memory region allocated for loading the firmware
+ * @reloc_base: Physical address adjusted after relocation
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+int qcom_mdt_pas_load(struct qcom_scm_pas_context *ctx, const struct firmware *fw,
+		      const char *firmware, void *mem_region, phys_addr_t *reloc_base)
+{
+	int ret;
+
+	ret = qcom_mdt_pas_init(ctx->dev, fw, firmware, ctx->pas_id, ctx->mem_phys, ctx);
+	if (ret)
+		return ret;
+
+	return qcom_mdt_load_no_init(ctx->dev, fw, firmware, mem_region, ctx->mem_phys,
+				     ctx->mem_size, reloc_base);
+}
+EXPORT_SYMBOL_GPL(qcom_mdt_pas_load);
+
 MODULE_DESCRIPTION("Firmware parser for Qualcomm MDT format");
 MODULE_LICENSE("GPL v2");
diff --git a/include/linux/soc/qcom/mdt_loader.h b/include/linux/soc/qcom/mdt_loader.h
index 07c278841816..7d57746fbbfa 100644
--- a/include/linux/soc/qcom/mdt_loader.h
+++ b/include/linux/soc/qcom/mdt_loader.h
@@ -23,6 +23,9 @@ int qcom_mdt_load(struct device *dev, const struct firmware *fw,
 		  phys_addr_t mem_phys, size_t mem_size,
 		  phys_addr_t *reloc_base);
 
+int qcom_mdt_pas_load(struct qcom_scm_pas_context *ctx, const struct firmware *fw,
+		      const char *firmware, void *mem_region, phys_addr_t *reloc_base);
+
 int qcom_mdt_load_no_init(struct device *dev, const struct firmware *fw,
 			  const char *fw_name, void *mem_region,
 			  phys_addr_t mem_phys, size_t mem_size,
@@ -52,6 +55,13 @@ static inline int qcom_mdt_load(struct device *dev, const struct firmware *fw,
 	return -ENODEV;
 }
 
+static inline int qcom_mdt_pas_load(struct qcom_scm_pas_context *ctx,
+				    const struct firmware *fw, const char *firmware,
+				    void *mem_region, phys_addr_t *reloc_base)
+{
+	return -ENODEV;
+}
+
 static inline int qcom_mdt_load_no_init(struct device *dev,
 					const struct firmware *fw,
 					const char *fw_name, void *mem_region,

-- 
2.50.1

[PATCH v8 07/14] soc: qcom: mdtloader: Remove qcom_mdt_pas_init() from exported symbols

[Mukesh Ojha]

qcom_mdt_pas_init() was previously used only by the remoteproc driver
(drivers/remoteproc/qcom_q6v5_pas.c). Since that driver has now
transitioned to using PAS context-based qcom_mdt_pas_load() function,
making qcom_mdt_pas_init() obsolete for external use.

Removes qcom_mdt_pas_init() from the list of exported symbols and make
it static to limit its scope to internal use within mdtloader.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/soc/qcom/mdt_loader.c       | 11 +++++------
 include/linux/soc/qcom/mdt_loader.h | 10 ----------
 2 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
index 50c6a3c6b2a3..054abc81e76d 100644
--- a/drivers/soc/qcom/mdt_loader.c
+++ b/drivers/soc/qcom/mdt_loader.c
@@ -238,9 +238,9 @@ EXPORT_SYMBOL_GPL(qcom_mdt_read_metadata);
  *
  * 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_context *ctx)
+static 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_context *ctx)
 {
 	const struct elf32_phdr *phdrs;
 	const struct elf32_phdr *phdr;
@@ -302,7 +302,6 @@ int qcom_mdt_pas_init(struct device *dev, const struct firmware *fw,
 out:
 	return ret;
 }
-EXPORT_SYMBOL_GPL(qcom_mdt_pas_init);
 
 static bool qcom_mdt_bins_are_split(const struct firmware *fw)
 {
@@ -469,7 +468,7 @@ int qcom_mdt_load(struct device *dev, const struct firmware *fw,
 {
 	int ret;
 
-	ret = qcom_mdt_pas_init(dev, fw, fw_name, pas_id, mem_phys, NULL);
+	ret = __qcom_mdt_pas_init(dev, fw, fw_name, pas_id, mem_phys, NULL);
 	if (ret)
 		return ret;
 
@@ -500,7 +499,7 @@ int qcom_mdt_pas_load(struct qcom_scm_pas_context *ctx, const struct firmware *f
 {
 	int ret;
 
-	ret = qcom_mdt_pas_init(ctx->dev, fw, firmware, ctx->pas_id, ctx->mem_phys, ctx);
+	ret = __qcom_mdt_pas_init(ctx->dev, fw, firmware, ctx->pas_id, ctx->mem_phys, ctx);
 	if (ret)
 		return ret;
 
diff --git a/include/linux/soc/qcom/mdt_loader.h b/include/linux/soc/qcom/mdt_loader.h
index 7d57746fbbfa..82372e0db0a1 100644
--- a/include/linux/soc/qcom/mdt_loader.h
+++ b/include/linux/soc/qcom/mdt_loader.h
@@ -15,9 +15,6 @@ 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_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,
@@ -40,13 +37,6 @@ static inline ssize_t qcom_mdt_get_size(const struct firmware *fw)
 	return -ENODEV;
 }
 
-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_context *pas_ctx)
-{
-	return -ENODEV;
-}
-
 static inline 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,

-- 
2.50.1

[PATCH v8 08/14] firmware: qcom_scm: Add a prep version of auth_and_reset function

[Mukesh Ojha]

For memory passed to TrustZone (TZ), it must either be part of a pool
registered with TZ or explicitly registered via SHMbridge SMC calls.
When Gunyah hypervisor is present, PAS SMC calls from Linux running at
EL1 are trapped by Gunyah running @ EL2, which handles SHMbridge
creation for both metadata and remoteproc carveout memory before
invoking the calls to TZ.

On SoCs running with a non-Gunyah-based hypervisor, Linux must take
responsibility for creating the SHM bridge before invoking PAS SMC
calls. For the auth_and_reset() call, the remoteproc carveout memory
must first be registered with TZ via a SHMbridge SMC call and once
authentication and reset are complete, the SHMbridge memory can be
deregistered.

Introduce qcom_scm_pas_prepare_and_auth_reset(), which sets up the SHM
bridge over the remoteproc carveout memory when Linux operates at EL2.
This behavior is indicated by a new field added to the PAS context data
structure. The function then invokes the auth_and_reset SMC call.

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

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 5fa974683ee0..fdb736d839db 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -765,6 +765,54 @@ int qcom_scm_pas_auth_and_reset(u32 pas_id)
 }
 EXPORT_SYMBOL_GPL(qcom_scm_pas_auth_and_reset);
 
+/**
+ * qcom_scm_pas_prepare_and_auth_reset() - Prepare, authenticate, and reset the
+ *					   remote processor
+ *
+ * @ctx:	Context saved during call to qcom_scm_pas_context_init()
+ *
+ * This function performs the necessary steps to prepare a PAS subsystem,
+ * authenticate it using the provided metadata, and initiate a reset sequence.
+ *
+ * It should be used when Linux is in control setting up the IOMMU hardware
+ * for remote subsystem during secure firmware loading processes. The preparation
+ * step sets up a shmbridge over the firmware memory before TrustZone accesses the
+ * firmware memory region for authentication. The authentication step verifies
+ * the integrity and authenticity of the firmware or configuration using secure
+ * metadata. Finally, the reset step ensures the subsystem starts in a clean and
+ * sane state.
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+int qcom_scm_pas_prepare_and_auth_reset(struct qcom_scm_pas_context *ctx)
+{
+	u64 handle;
+	int ret;
+
+	if (!ctx->has_iommu)
+		return qcom_scm_pas_auth_and_reset(ctx->pas_id);
+
+	/*
+	 * When Linux running @ EL1, Gunyah hypervisor running @ EL2 traps the
+	 * auth_and_reset call and create an shmbridge on the remote subsystem
+	 * memory region and then invokes a call to TrustZone to authenticate.
+	 * When Linux runs @ EL2 Linux must create the shmbridge itself and then
+	 * subsequently call TrustZone for authenticate and reset.
+	 */
+	ret = qcom_tzmem_shm_bridge_create(ctx->mem_phys, ctx->mem_size, &handle);
+	if (ret) {
+		dev_err(__scm->dev, "Failed to create shmbridge for PAS ID (%u): %d\n",
+			ctx->pas_id, ret);
+		return ret;
+	}
+
+	ret = qcom_scm_pas_auth_and_reset(ctx->pas_id);
+	qcom_tzmem_shm_bridge_delete(handle);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(qcom_scm_pas_prepare_and_auth_reset);
+
 /**
  * qcom_scm_pas_shutdown() - Shut down the remote processor
  * @pas_id:	peripheral authentication service id
diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
index b10b1aeb32c6..ccb8b2e42237 100644
--- a/include/linux/firmware/qcom/qcom_scm.h
+++ b/include/linux/firmware/qcom/qcom_scm.h
@@ -74,6 +74,7 @@ struct qcom_scm_pas_context {
 	void *ptr;
 	dma_addr_t phys;
 	ssize_t size;
+	bool has_iommu;
 };
 
 struct qcom_scm_pas_context *devm_qcom_scm_pas_context_init(struct device *dev,
@@ -87,6 +88,7 @@ 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);
 bool qcom_scm_pas_supported(u32 pas_id);
+int qcom_scm_pas_prepare_and_auth_reset(struct qcom_scm_pas_context *ctx);
 
 int qcom_scm_io_readl(phys_addr_t addr, unsigned int *val);
 int qcom_scm_io_writel(phys_addr_t addr, unsigned int val);

-- 
2.50.1

[PATCH v8 09/14] firmware: qcom_scm: Refactor qcom_scm_pas_init_image()

[Mukesh Ojha]

Refactor qcom_scm_pas_init_image() by moving the memory allocation,
copy, and free operations to a higher-level function, and isolate the
actual SMC call in a separate function. The main intention is to allow
flexibility for different allocators and to respect any constraints that
the allocator API may impose before invoking the actual SCM function.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm.c | 58 +++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 25 deletions(-)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index fdb736d839db..90649d9ade97 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -592,6 +592,37 @@ struct qcom_scm_pas_context *devm_qcom_scm_pas_context_init(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(devm_qcom_scm_pas_context_init);
 
+static int __qcom_scm_pas_init_image(u32 pas_id, dma_addr_t mdata_phys,
+				     struct qcom_scm_res *res)
+{
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_PIL,
+		.cmd = QCOM_SCM_PIL_PAS_INIT_IMAGE,
+		.arginfo = QCOM_SCM_ARGS(2, QCOM_SCM_VAL, QCOM_SCM_RW),
+		.args[0] = pas_id,
+		.owner = ARM_SMCCC_OWNER_SIP,
+	};
+	int ret;
+
+	ret = qcom_scm_clk_enable();
+	if (ret)
+		return ret;
+
+	ret = qcom_scm_bw_enable();
+	if (ret)
+		goto disable_clk;
+
+	desc.args[1] = mdata_phys;
+
+	ret = qcom_scm_call(__scm->dev, &desc, res);
+	qcom_scm_bw_disable();
+
+disable_clk:
+	qcom_scm_clk_disable();
+
+	return ret;
+}
+
 /**
  * qcom_scm_pas_init_image() - Initialize peripheral authentication service
  *			       state machine for a given peripheral, using the
@@ -612,17 +643,10 @@ EXPORT_SYMBOL_GPL(devm_qcom_scm_pas_context_init);
 int qcom_scm_pas_init_image(u32 pas_id, const void *metadata, size_t size,
 			    struct qcom_scm_pas_context *ctx)
 {
+	struct qcom_scm_res res;
 	dma_addr_t mdata_phys;
 	void *mdata_buf;
 	int ret;
-	struct qcom_scm_desc desc = {
-		.svc = QCOM_SCM_SVC_PIL,
-		.cmd = QCOM_SCM_PIL_PAS_INIT_IMAGE,
-		.arginfo = QCOM_SCM_ARGS(2, QCOM_SCM_VAL, QCOM_SCM_RW),
-		.args[0] = pas_id,
-		.owner = ARM_SMCCC_OWNER_SIP,
-	};
-	struct qcom_scm_res res;
 
 	/*
 	 * During the scm call memory protection will be enabled for the meta
@@ -643,23 +667,7 @@ int qcom_scm_pas_init_image(u32 pas_id, const void *metadata, size_t size,
 
 	memcpy(mdata_buf, metadata, size);
 
-	ret = qcom_scm_clk_enable();
-	if (ret)
-		goto out;
-
-	ret = qcom_scm_bw_enable();
-	if (ret)
-		goto disable_clk;
-
-	desc.args[1] = mdata_phys;
-
-	ret = qcom_scm_call(__scm->dev, &desc, &res);
-	qcom_scm_bw_disable();
-
-disable_clk:
-	qcom_scm_clk_disable();
-
-out:
+	ret = __qcom_scm_pas_init_image(pas_id, mdata_phys, &res);
 	if (ret < 0 || !ctx) {
 		dma_free_coherent(__scm->dev, size, mdata_buf, mdata_phys);
 	} else if (ctx) {

-- 
2.50.1

[PATCH v8 10/14] firmware: qcom_scm: Add SHM bridge handling for PAS when running without QHEE

[Mukesh Ojha]

On SoCs running with a non-Gunyah-based hypervisor, Linux must take
responsibility for creating the SHM bridge both for metadata (before
calling qcom_scm_pas_init_image()) and for remoteproc memory (before
calling qcom_scm_pas_auth_and_reset()). We have taken care the things
required for qcom_scm_pas_auth_and_reset().

Lets put these awareness of above conditions into
qcom_scm_pas_init_image() and qcom_scm_pas_metadata_release().

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 90649d9ade97..84498d0d2f0c 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -623,6 +623,30 @@ static int __qcom_scm_pas_init_image(u32 pas_id, dma_addr_t mdata_phys,
 	return ret;
 }
 
+static int qcom_scm_pas_prep_and_init_image(struct qcom_scm_pas_context *ctx,
+					    const void *metadata, size_t size)
+{
+	struct qcom_scm_res res;
+	phys_addr_t mdata_phys;
+	void *mdata_buf;
+	int ret;
+
+	mdata_buf = qcom_tzmem_alloc(__scm->mempool, size, GFP_KERNEL);
+	if (!mdata_buf)
+		return -ENOMEM;
+
+	memcpy(mdata_buf, metadata, size);
+	mdata_phys = qcom_tzmem_to_phys(mdata_buf);
+
+	ret = __qcom_scm_pas_init_image(ctx->pas_id, mdata_phys, &res);
+	if (ret < 0)
+		qcom_tzmem_free(mdata_buf);
+	else
+		ctx->ptr = mdata_buf;
+
+	return ret ? : res.result[0];
+}
+
 /**
  * qcom_scm_pas_init_image() - Initialize peripheral authentication service
  *			       state machine for a given peripheral, using the
@@ -648,6 +672,9 @@ int qcom_scm_pas_init_image(u32 pas_id, const void *metadata, size_t size,
 	void *mdata_buf;
 	int ret;
 
+	if (ctx && ctx->has_iommu)
+		return qcom_scm_pas_prep_and_init_image(ctx, metadata, size);
+
 	/*
 	 * During the scm call memory protection will be enabled for the meta
 	 * data blob, so make sure it's physically contiguous, 4K aligned and
@@ -689,7 +716,10 @@ void qcom_scm_pas_metadata_release(struct qcom_scm_pas_context *ctx)
 	if (!ctx->ptr)
 		return;
 
-	dma_free_coherent(__scm->dev, ctx->size, ctx->ptr, ctx->phys);
+	if (ctx->has_iommu)
+		qcom_tzmem_free(ctx->ptr);
+	else
+		dma_free_coherent(__scm->dev, ctx->size, ctx->ptr, ctx->phys);
 
 	ctx->ptr = NULL;
 }

-- 
2.50.1

[PATCH v8 11/14] firmware: qcom_scm: Add qcom_scm_pas_get_rsc_table() to get resource table

[Mukesh Ojha]

Qualcomm remote processor may rely on Static and Dynamic resources for
it to be functional. Static resources are fixed like for example,
memory-mapped addresses required by the subsystem and dynamic
resources, such as shared memory in DDR etc., are determined at
runtime during the boot process.

For most of the Qualcomm SoCs, when run with Gunyah or older QHEE
hypervisor, all the resources whether it is static or dynamic, is
managed by the hypervisor. Dynamic resources if it is present for a
remote processor will always be coming from secure world via SMC call
while static resources may be present in remote processor firmware
binary or it may be coming qcom_scm_pas_get_rsc_table() SMC call along
with dynamic resources.

Some of the remote processor drivers, such as video, GPU, IPA, etc., do
not check whether resources are present in their remote processor
firmware binary. In such cases, the caller of this function should set
input_rt and input_rt_size as NULL and zero respectively. Remoteproc
framework has method to check whether firmware binary contain resources
or not and they should be pass resource table pointer to input_rt and
resource table size to input_rt_size and this will be forwarded to
TrustZone for authentication. TrustZone will then append the dynamic
resources and return the complete resource table in output_rt

More about documentation on resource table format can be found in
include/linux/remoteproc.h

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm.c       | 158 +++++++++++++++++++++++++++++++++
 drivers/firmware/qcom/qcom_scm.h       |   1 +
 include/linux/firmware/qcom/qcom_scm.h |   4 +
 3 files changed, 163 insertions(+)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 84498d0d2f0c..c4420b79fb57 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -27,6 +27,7 @@
 #include <linux/of_reserved_mem.h>
 #include <linux/platform_device.h>
 #include <linux/reset-controller.h>
+#include <linux/remoteproc.h>
 #include <linux/sizes.h>
 #include <linux/types.h>
 
@@ -111,6 +112,10 @@ enum qcom_scm_qseecom_tz_cmd_info {
 	QSEECOM_TZ_CMD_INFO_VERSION		= 3,
 };
 
+enum qcom_scm_rsctable_resp_type {
+	RSCTABLE_BUFFER_NOT_SUFFICIENT		= 20,
+};
+
 #define QSEECOM_MAX_APP_NAME_SIZE		64
 #define SHMBRIDGE_RESULT_NOTSUPP		4
 
@@ -766,6 +771,159 @@ int qcom_scm_pas_mem_setup(u32 pas_id, phys_addr_t addr, phys_addr_t size)
 }
 EXPORT_SYMBOL_GPL(qcom_scm_pas_mem_setup);
 
+static int __qcom_scm_pas_get_rsc_table(u32 pas_id, void *input_rt, size_t input_rt_size,
+					void **output_rt, size_t *output_rt_size)
+{
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_PIL,
+		.cmd = QCOM_SCM_PIL_PAS_GET_RSCTABLE,
+		.arginfo = QCOM_SCM_ARGS(5, QCOM_SCM_VAL, QCOM_SCM_RO, QCOM_SCM_VAL,
+					 QCOM_SCM_RW, QCOM_SCM_VAL),
+		.args[0] = pas_id,
+		.owner = ARM_SMCCC_OWNER_SIP,
+	};
+	void *input_rt_buf, *output_rt_buf;
+	struct resource_table *rsc;
+	struct qcom_scm_res res;
+	int ret;
+
+	ret = qcom_scm_clk_enable();
+	if (ret)
+		return ret;
+
+	ret = qcom_scm_bw_enable();
+	if (ret)
+		goto disable_clk;
+
+	/*
+	 * TrustZone can not accept buffer as NULL value as argument Hence,
+	 * we need to pass a input buffer indicating that subsystem firmware
+	 * does not have resource table by filling resource table structure.
+	 */
+	if (!input_rt)
+		input_rt_size = sizeof(*rsc);
+
+	input_rt_buf = qcom_tzmem_alloc(__scm->mempool, input_rt_size, GFP_KERNEL);
+	if (!input_rt_buf) {
+		ret = -ENOMEM;
+		goto disable_scm_bw;
+	}
+
+	if (!input_rt) {
+		rsc = input_rt_buf;
+		rsc->num = 0;
+	} else {
+		memcpy(input_rt_buf, input_rt, input_rt_size);
+	}
+
+	output_rt_buf = qcom_tzmem_alloc(__scm->mempool, *output_rt_size, GFP_KERNEL);
+	if (!output_rt_buf) {
+		ret = -ENOMEM;
+		goto free_input_rt_buf;
+	}
+
+	desc.args[1] = qcom_tzmem_to_phys(input_rt_buf);
+	desc.args[2] = input_rt_size;
+	desc.args[3] = qcom_tzmem_to_phys(output_rt_buf);
+	desc.args[4] = *output_rt_size;
+
+	/*
+	 * Whether SMC fail or pass, res.result[2] will hold actual resource table
+	 * size.
+	 *
+	 * if passed 'output_rt_size' buffer size is not sufficient to hold the
+	 * resource table TrustZone sends, response code in res.result[1] as
+	 * RSCTABLE_BUFFER_NOT_SUFFICIENT so that caller can retry this SMC call with
+	 * output_rt buffer with res.result[2] size.
+	 */
+	ret = qcom_scm_call(__scm->dev, &desc, &res);
+	*output_rt_size = res.result[2];
+	if (!ret)
+		memcpy(*output_rt, output_rt_buf, *output_rt_size);
+
+	if (ret && res.result[1] == RSCTABLE_BUFFER_NOT_SUFFICIENT)
+		ret = -EAGAIN;
+
+	qcom_tzmem_free(output_rt_buf);
+
+free_input_rt_buf:
+	qcom_tzmem_free(input_rt_buf);
+
+disable_scm_bw:
+	qcom_scm_bw_disable();
+
+disable_clk:
+	qcom_scm_clk_disable();
+
+	return ret ? : res.result[0];
+}
+
+/**
+ * qcom_scm_pas_get_rsc_table() - Retrieve the resource table in passed output buffer
+ *				  for a given peripheral.
+ *
+ * Qualcomm remote processor may rely on both static and dynamic resources for
+ * its functionality. Static resources typically refer to memory-mapped addresses
+ * required by the subsystem and are often embedded within the firmware binary
+ * and dynamic resources, such as shared memory in DDR etc., are determined at
+ * runtime during the boot process.
+ *
+ * On Qualcomm Technologies devices, it's possible that static resources are not
+ * embedded in the firmware binary and instead are provided by TrustZone However,
+ * dynamic resources are always expected to come from TrustZone. This indicates
+ * that for Qualcomm devices, all resources (static and dynamic) will be provided
+ * by TrustZone via the SMC call.
+ *
+ * If the remote processor firmware binary does contain static resources, they
+ * should be passed in input_rt. These will be forwarded to TrustZone for
+ * authentication. TrustZone will then append the dynamic resources and return
+ * the complete resource table in output_rt.
+ *
+ * If the remote processor firmware binary does not include a resource table,
+ * the caller of this function should set input_rt as NULL and input_rt_size
+ * as zero respectively.
+ *
+ * More about documentation on resource table data structures can be found in
+ * include/linux/remoteproc.h
+ *
+ * @ctx:	    PAS context
+ * @pas_id:	    peripheral authentication service id
+ * @input_rt:       resource table buffer which is present in firmware binary
+ * @input_rt_size:  size of the resource table present in firmware binary
+ * @output_rt:	    buffer to which the both static and dynamic resources will
+ *		    be returned.
+ * @output_rt_size: TrustZone expects caller should pass worst case size for
+ *		    the output_rt.
+ *
+ * Return: 0 on success and nonzero on failure.
+ *
+ * Upon successful return, output_rt will have the resource table and output_rt_size
+ * will have actual resource table size,
+ */
+int qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *ctx, void *input_rt,
+			       size_t input_rt_size, void **output_rt,
+			       size_t *output_rt_size)
+{
+	unsigned int retry_num = 5;
+	int ret;
+
+	do {
+		*output_rt = kzalloc(*output_rt_size, GFP_KERNEL);
+		if (!*output_rt)
+			return -ENOMEM;
+
+		ret = __qcom_scm_pas_get_rsc_table(ctx->pas_id, input_rt,
+						   input_rt_size, output_rt,
+						   output_rt_size);
+		if (ret)
+			kfree(*output_rt);
+
+	} while (ret == -EAGAIN && --retry_num);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(qcom_scm_pas_get_rsc_table);
+
 /**
  * qcom_scm_pas_auth_and_reset() - Authenticate the given peripheral firmware
  *				   and reset the remote processor
diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h
index a56c8212cc0c..50d87c628d78 100644
--- a/drivers/firmware/qcom/qcom_scm.h
+++ b/drivers/firmware/qcom/qcom_scm.h
@@ -105,6 +105,7 @@ int qcom_scm_shm_bridge_enable(struct device *scm_dev);
 #define QCOM_SCM_PIL_PAS_SHUTDOWN	0x06
 #define QCOM_SCM_PIL_PAS_IS_SUPPORTED	0x07
 #define QCOM_SCM_PIL_PAS_MSS_RESET	0x0a
+#define QCOM_SCM_PIL_PAS_GET_RSCTABLE	0x21
 
 #define QCOM_SCM_SVC_IO			0x05
 #define QCOM_SCM_IO_READ		0x01
diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
index ccb8b2e42237..814d40d64ab0 100644
--- a/include/linux/firmware/qcom/qcom_scm.h
+++ b/include/linux/firmware/qcom/qcom_scm.h
@@ -88,6 +88,10 @@ 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);
 bool qcom_scm_pas_supported(u32 pas_id);
+int qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *ctx, void *input_rt,
+			       size_t input_rt_size, void **output_rt,
+			       size_t *output_rt_size);
+
 int qcom_scm_pas_prepare_and_auth_reset(struct qcom_scm_pas_context *ctx);
 
 int qcom_scm_io_readl(phys_addr_t addr, unsigned int *val);

-- 
2.50.1

[PATCH v8 12/14] remoteproc: pas: Extend parse_fw callback to fetch resources via SMC call

[Mukesh Ojha]

Qualcomm remote processor may rely on static and dynamic resources for
it to be functional. For most of the Qualcomm SoCs, when run with Gunyah
or older QHEE hypervisor, all the resources whether it is static or
dynamic, is managed by the hypervisor. Dynamic resources if it is
present for a remote processor will always be coming from secure world
via SMC call while static resources may be present in remote processor
firmware binary or it may be coming from SMC call along with dynamic
resources.

Remoteproc already has method like rproc_elf_load_rsc_table() to check
firmware binary has resources or not and if it is not having then we
pass NULL and zero as input resource table and its size argument
respectively to qcom_scm_pas_get_rsc_table() and while it has resource
present then it should pass the present resources to Trustzone(TZ) so that
it could authenticate the present resources and append dynamic resource
to return in output_rt argument along with authenticated resources.

Extend parse_fw callback to include SMC call to get resources from
Trustzone and to leverage resource table parsing and mapping and
unmapping code from the remoteproc framework.

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 60 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 58 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 9feee2cb1883..186a90d6c83a 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -34,6 +34,7 @@
 #define QCOM_PAS_DECRYPT_SHUTDOWN_DELAY_MS	100
 
 #define MAX_ASSIGN_COUNT 3
+#define MAX_RSCTABLE_SIZE	SZ_16K
 
 struct qcom_pas_data {
 	int crash_reason_smem;
@@ -413,6 +414,61 @@ static void *qcom_pas_da_to_va(struct rproc *rproc, u64 da, size_t len, bool *is
 	return pas->mem_region + offset;
 }
 
+static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *fw)
+{
+	size_t output_rt_size = MAX_RSCTABLE_SIZE;
+	struct qcom_pas *pas = rproc->priv;
+	struct resource_table *table = NULL;
+	void *output_rt;
+	size_t table_sz;
+	int ret;
+
+	ret = qcom_register_dump_segments(rproc, fw);
+	if (ret) {
+		dev_err(pas->dev, "Error in registering dump segments\n");
+		return ret;
+	}
+
+	if (!rproc->has_iommu)
+		return 0;
+
+	ret = rproc_elf_load_rsc_table(rproc, fw);
+	if (ret)
+		dev_dbg(&rproc->dev, "Failed to load resource table from firmware\n");
+
+	table = rproc->table_ptr;
+	table_sz = rproc->table_sz;
+
+	/*
+	 * The resources consumed by Qualcomm remote processors fall into two categories:
+	 * static (such as the memory carveouts for the rproc firmware) and dynamic (like
+	 * shared memory pools). Both are managed by a Qualcomm hypervisor (such as QHEE
+	 * or Gunyah), if one is present. Otherwise, a resource table must be retrieved
+	 * via an SCM call. That table will list all dynamic resources (if any) and possibly
+	 * the static ones. The static resources may also come from a resource table embedded
+	 * in the rproc firmware instead.
+	 *
+	 * Here, we call rproc_elf_load_rsc_table() to check firmware binary has resources
+	 * or not and if it is not having then we pass NULL and zero as input resource
+	 * table pointer and size respectively to the argument of qcom_scm_pas_get_rsc_table()
+	 * and this is even true for Qualcomm remote processor who does follow remoteproc
+	 * framework.
+	 */
+	ret = qcom_scm_pas_get_rsc_table(pas->pas_ctx, table, table_sz, &output_rt,
+					 &output_rt_size);
+	if (ret) {
+		dev_err(pas->dev, "Error in getting resource table: %d\n", ret);
+		return ret;
+	}
+
+	kfree(rproc->cached_table);
+	rproc->cached_table = output_rt;
+	rproc->table_ptr = rproc->cached_table;
+	rproc->table_sz = output_rt_size;
+
+	return ret;
+}
+
 static unsigned long qcom_pas_panic(struct rproc *rproc)
 {
 	struct qcom_pas *pas = rproc->priv;
@@ -425,7 +481,7 @@ static const struct rproc_ops qcom_pas_ops = {
 	.start = qcom_pas_start,
 	.stop = qcom_pas_stop,
 	.da_to_va = qcom_pas_da_to_va,
-	.parse_fw = qcom_register_dump_segments,
+	.parse_fw = qcom_pas_parse_firmware,
 	.load = qcom_pas_load,
 	.panic = qcom_pas_panic,
 };
@@ -435,7 +491,7 @@ static const struct rproc_ops qcom_pas_minidump_ops = {
 	.start = qcom_pas_start,
 	.stop = qcom_pas_stop,
 	.da_to_va = qcom_pas_da_to_va,
-	.parse_fw = qcom_register_dump_segments,
+	.parse_fw = qcom_pas_parse_firmware,
 	.load = qcom_pas_load,
 	.panic = qcom_pas_panic,
 	.coredump = qcom_pas_minidump,

-- 
2.50.1

[PATCH v8 13/14] remoteproc: qcom: pas: Enable Secure PAS support with IOMMU managed by Linux

[Mukesh Ojha]

Most Qualcomm platforms feature Gunyah hypervisor, which typically
handles IOMMU configuration. This includes mapping memory regions and
device memory resources for remote processors by intercepting
qcom_scm_pas_auth_and_reset() calls. These mappings are later removed
during teardown. Additionally, SHM bridge setup is required to enable
memory protection for both remoteproc metadata and its memory regions.
When the aforementioned hypervisor is absent, the operating system must
perform these configurations instead.

When Linux runs as the hypervisor (@ EL2) on a SoC, it will have its
own device tree overlay file that specifies the firmware stream ID now
managed by Linux for a particular remote processor. If the iommus
property is specified in the remoteproc device tree node, it indicates
that IOMMU configuration must be handled by Linux. In this case, the
has_iommu flag is set for the remote processor, which ensures that the
resource table, carveouts, and SHM bridge are properly configured before
memory is passed to TrustZone for authentication. Otherwise, the
has_iommu flag remains unset, which indicates default behavior.

Enables Secure PAS support for remote processors when IOMMU configuration
is managed by Linux.

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 48 ++++++++++++++++++++++++++++++++++----
 1 file changed, 43 insertions(+), 5 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 186a90d6c83a..2df3b06f8157 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -11,6 +11,7 @@
 #include <linux/delay.h>
 #include <linux/firmware.h>
 #include <linux/interrupt.h>
+#include <linux/iommu.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -256,6 +257,22 @@ static int qcom_pas_load(struct rproc *rproc, const struct firmware *fw)
 	return ret;
 }
 
+static void qcom_pas_unmap_carveout(struct rproc *rproc, phys_addr_t mem_phys, size_t size)
+{
+	if (rproc->has_iommu)
+		iommu_unmap(rproc->domain, mem_phys, size);
+}
+
+static int qcom_pas_map_carveout(struct rproc *rproc, phys_addr_t mem_phys, size_t size)
+{
+	int ret = 0;
+
+	if (rproc->has_iommu)
+		ret = iommu_map(rproc->domain, mem_phys, mem_phys, size,
+				IOMMU_READ | IOMMU_WRITE, GFP_KERNEL);
+	return ret;
+}
+
 static int qcom_pas_start(struct rproc *rproc)
 {
 	struct qcom_pas *pas = rproc->priv;
@@ -290,11 +307,15 @@ static int qcom_pas_start(struct rproc *rproc)
 	}
 
 	if (pas->dtb_pas_id) {
-		ret = qcom_scm_pas_auth_and_reset(pas->dtb_pas_id);
+		ret = qcom_pas_map_carveout(rproc, pas->dtb_mem_phys, pas->dtb_mem_size);
+		if (ret)
+			goto disable_px_supply;
+
+		ret = qcom_scm_pas_prepare_and_auth_reset(pas->dtb_pas_ctx);
 		if (ret) {
 			dev_err(pas->dev,
 				"failed to authenticate dtb image and release reset\n");
-			goto disable_px_supply;
+			goto unmap_dtb_carveout;
 		}
 	}
 
@@ -305,18 +326,22 @@ static int qcom_pas_start(struct rproc *rproc)
 
 	qcom_pil_info_store(pas->info_name, pas->mem_phys, pas->mem_size);
 
-	ret = qcom_scm_pas_auth_and_reset(pas->pas_id);
+	ret = qcom_pas_map_carveout(rproc, pas->mem_phys, pas->mem_size);
+	if (ret)
+		goto release_pas_metadata;
+
+	ret = qcom_scm_pas_prepare_and_auth_reset(pas->pas_ctx);
 	if (ret) {
 		dev_err(pas->dev,
 			"failed to authenticate image and release reset\n");
-		goto release_pas_metadata;
+		goto unmap_carveout;
 	}
 
 	ret = qcom_q6v5_wait_for_start(&pas->q6v5, msecs_to_jiffies(5000));
 	if (ret == -ETIMEDOUT) {
 		dev_err(pas->dev, "start timed out\n");
 		qcom_scm_pas_shutdown(pas->pas_id);
-		goto release_pas_metadata;
+		goto unmap_carveout;
 	}
 
 	qcom_scm_pas_metadata_release(pas->pas_ctx);
@@ -328,10 +353,16 @@ static int qcom_pas_start(struct rproc *rproc)
 
 	return 0;
 
+unmap_carveout:
+	qcom_pas_unmap_carveout(rproc, pas->mem_phys, pas->mem_size);
 release_pas_metadata:
 	qcom_scm_pas_metadata_release(pas->pas_ctx);
 	if (pas->dtb_pas_id)
 		qcom_scm_pas_metadata_release(pas->dtb_pas_ctx);
+
+unmap_dtb_carveout:
+	if (pas->dtb_pas_id)
+		qcom_pas_unmap_carveout(rproc, pas->dtb_mem_phys, pas->dtb_mem_size);
 disable_px_supply:
 	if (pas->px_supply)
 		regulator_disable(pas->px_supply);
@@ -387,8 +418,12 @@ static int qcom_pas_stop(struct rproc *rproc)
 		ret = qcom_scm_pas_shutdown(pas->dtb_pas_id);
 		if (ret)
 			dev_err(pas->dev, "failed to shutdown dtb: %d\n", ret);
+
+		qcom_pas_unmap_carveout(rproc, pas->dtb_mem_phys, pas->dtb_mem_size);
 	}
 
+	qcom_pas_unmap_carveout(rproc, pas->mem_phys, pas->mem_size);
+
 	handover = qcom_q6v5_unprepare(&pas->q6v5);
 	if (handover)
 		qcom_pas_handover(&pas->q6v5);
@@ -758,6 +793,7 @@ static int qcom_pas_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	rproc->has_iommu = of_property_present(pdev->dev.of_node, "iommus");
 	rproc->auto_boot = desc->auto_boot;
 	rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
 
@@ -837,6 +873,8 @@ static int qcom_pas_probe(struct platform_device *pdev)
 		goto remove_ssr_sysmon;
 	}
 
+	pas->pas_ctx->has_iommu = rproc->has_iommu;
+	pas->dtb_pas_ctx->has_iommu = rproc->has_iommu;
 	ret = rproc_add(rproc);
 	if (ret)
 		goto remove_ssr_sysmon;

-- 
2.50.1

[PATCH v8 14/14] arm64: dts: qcom: Add EL2 overlay for Lemans

[Mukesh Ojha]

All the Lemans IOT variants boards are using Gunyah hypervisor which
means that, so far, Linux-based OS could only boot in EL1 on those
devices.  However, it is possible for us to boot Linux at EL2 on these
devices [1].

When running under Gunyah, remote processor firmware IOMMU streams is
controlled by the Gunyah however when Linux take ownership of it in EL2,
It need to configure it properly to use remote processor.

Add a EL2-specific DT overlay and apply it to Lemans IOT variant
devices to create -el2.dtb for each of them alongside "normal" dtb.

[1]
https://docs.qualcomm.com/bundle/publicresource/topics/80-70020-4/boot-developer-touchpoints.html#uefi

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/Makefile        | 10 ++++++++
 arch/arm64/boot/dts/qcom/lemans-el2.dtso | 41 ++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 6f34d5ed331c..56efd90b7a5e 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -37,6 +37,10 @@ lemans-evk-camera-dtbs	:= lemans-evk.dtb lemans-evk-camera.dtbo
 
 dtb-$(CONFIG_ARCH_QCOM)	+= lemans-evk-camera-csi1-imx577.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= lemans-evk-camera.dtb
+
+lemans-evk-el2-dtbs := lemans-evk.dtb lemans-el2.dtbo
+
+dtb-$(CONFIG_ARCH_QCOM)	+= lemans-evk-el2.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= monaco-evk.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8216-samsung-fortuna3g.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-acer-a1-724.dtb
@@ -142,6 +146,12 @@ dtb-$(CONFIG_ARCH_QCOM)	+= qcs8300-ride.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qcs8550-aim300-aiot.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride-r3.dtb
+
+qcs9100-ride-el2-dtbs := qcs9100-ride.dtb lemans-el2.dtbo
+qcs9100-ride-r3-el2-dtbs := qcs9100-ride-r3.dtb lemans-el2.dtbo
+
+dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride-el2.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride-r3-el2.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qdu1000-idp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qrb2210-rb1.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qrb4210-rb2.dtb
diff --git a/arch/arm64/boot/dts/qcom/lemans-el2.dtso b/arch/arm64/boot/dts/qcom/lemans-el2.dtso
new file mode 100644
index 000000000000..af35039946e3
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/lemans-el2.dtso
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+/*
+ * Lemans specific modifications required to boot in EL2.
+ */
+
+/dts-v1/;
+/plugin/;
+
+&iris {
+	/* More driver work is needed */
+	status = "disabled";
+};
+
+/*
+ * When running under Gunyah, remote processor firmware IOMMU streams is
+ * controlled by the Gunyah however when we take ownership of it in EL2,
+ * we need to configure it properly to use remote processor.
+ */
+&remoteproc_adsp {
+	iommus = <&apps_smmu 0x3000 0x0>;
+};
+
+&remoteproc_cdsp0 {
+	iommus = <&apps_smmu 0x21c0 0x0400>;
+};
+
+&remoteproc_cdsp1 {
+	iommus = <&apps_smmu 0x29c0 0x0400>;
+};
+
+&remoteproc_gpdsp0 {
+       iommus = <&apps_smmu 0x38a0 0x0>;
+};
+
+&remoteproc_gpdsp1 {
+       iommus = <&apps_smmu 0x38c0 0x0>;
+};

-- 
2.50.1

Re: [PATCH v8 00/14] Peripheral Image Loader support for Qualcomm SoCs running Linux host at EL2

[Bryan O'Donoghue]

On 21/11/2025 11:01, Mukesh Ojha wrote:
> In May 2025, we discussed the challenges at Linaro Connect 2025 [1]
> related to Secure PAS remoteproc enablement when Linux is running at EL2
> for Qualcomm SoCs.
> 
> [1] https://resources.linaro.org/en/resource/sF8jXifdb9V1mUefdbfafa
> 
> Below, is the summary of the discussion.
> 
> Qualcomm is working to enable remote processors on the SA8775p SoC with
> a Linux host running at EL2. In doing so, it has encountered several
> challenges related to how the remoteproc framework is handled when Linux
> runs at EL1.
> 
> One of the main challenges arises from differences in how IOMMU
> translation is currently managed on SoCs running the Qualcomm EL2
> hypervisor (QHEE), where IOMMU translation for any device is entirely
> owned by the hypervisor. Additionally, the firmware for remote
> processors does not contain a resource table, which would typically
> include the necessary IOMMU configuration settings.
> 
> Qualcomm SoCs running with QHEE (EL2) have been utilizing the Peripheral
> Authentication Service (PAS) from TrustZone (TZ) firmware to securely
> authenticate and reset remote processors via a single SMC call,
> _auth_and_reset_. This call is first trapped by QHEE, which then invokes
> TZ for authentication. Once authentication is complete, the call returns
> to QHEE, which sets up the IOMMU translation scheme for the remote
> processors and subsequently brings them out of reset. The design of the
> Qualcomm EL2 hypervisor dictates that the Linux host OS running at EL1
> is not permitted to configure IOMMU translation for remote processors,
> and only a single-stage translation is configured.
> 
> To make the remote processor bring-up (PAS) sequence
> hypervisor-independent, the auth_and_reset SMC call is now handled
> entirely by TZ. However, the issue of IOMMU configuration remains
> unresolved, for example a scenario, when KVM host at EL2 has no
> knowledge of the remote processors’ IOMMU settings.  This is being
> addressed by overlaying the IOMMU properties when the SoC runs a Linux
> host at EL2. SMC call is being provided from the TrustZone firmware to
> retrieve the resource table for a given subsystem.
> 
> There are also remote processors such as those for video, camera, and
> graphics that do not use the remoteproc framework to manage their
> lifecycle. Instead, they rely on the Qualcomm PAS service to
> authenticate their firmware. These processors also need to be brought
> out of reset when Linux is running at EL2. The client drivers for these
> processors use the MDT loader function to load and authenticate
> firmware. Similar to the Qualcomm remoteproc PAS driver, they also need
> to retrieve the resource table, create a shared memory bridge
> (shmbridge), and map the resources before bringing the processors out of
> reset.
> 
> It is based on next-20251120 and tested on SA8775p which is now called
> Lemans IOT platform and does not addresses DMA problem discussed at
> [1] which is future scope of the series.
> 
> Changes in v8: https://lore.kernel.org/lkml/20251113-kvm-rproc-v7-v7-0-df4910b7c20a@oss.qualcomm.com/
>   - Addressed suggestion from Stephen which was regarding commit message(9/14),
>     debug log(12/14) suggestion, and return type change(4/14).
>   - Added R-b tag on 10/14 .
Sorry.

Did we actually come up with a cogent reason to omit the video firmware 
loading here ?

AFAIU it is required for Lemans and Glymur - leaving it out is blocking 
getting video stuff done and storing up trouble.

What exactly is the blockage - is it something you want help with ?

---
bod

Re: [PATCH v8 00/14] Peripheral Image Loader support for Qualcomm SoCs running Linux host at EL2

[Mukesh Ojha]

On Fri, Nov 21, 2025 at 11:27:57AM +0000, Bryan O'Donoghue wrote:
> On 21/11/2025 11:01, Mukesh Ojha wrote:
> > In May 2025, we discussed the challenges at Linaro Connect 2025 [1]
> > related to Secure PAS remoteproc enablement when Linux is running at EL2
> > for Qualcomm SoCs.
> > 
> > [1] https://resources.linaro.org/en/resource/sF8jXifdb9V1mUefdbfafa
> > 
> > Below, is the summary of the discussion.
> > 
> > Qualcomm is working to enable remote processors on the SA8775p SoC with
> > a Linux host running at EL2. In doing so, it has encountered several
> > challenges related to how the remoteproc framework is handled when Linux
> > runs at EL1.
> > 
> > One of the main challenges arises from differences in how IOMMU
> > translation is currently managed on SoCs running the Qualcomm EL2
> > hypervisor (QHEE), where IOMMU translation for any device is entirely
> > owned by the hypervisor. Additionally, the firmware for remote
> > processors does not contain a resource table, which would typically
> > include the necessary IOMMU configuration settings.
> > 
> > Qualcomm SoCs running with QHEE (EL2) have been utilizing the Peripheral
> > Authentication Service (PAS) from TrustZone (TZ) firmware to securely
> > authenticate and reset remote processors via a single SMC call,
> > _auth_and_reset_. This call is first trapped by QHEE, which then invokes
> > TZ for authentication. Once authentication is complete, the call returns
> > to QHEE, which sets up the IOMMU translation scheme for the remote
> > processors and subsequently brings them out of reset. The design of the
> > Qualcomm EL2 hypervisor dictates that the Linux host OS running at EL1
> > is not permitted to configure IOMMU translation for remote processors,
> > and only a single-stage translation is configured.
> > 
> > To make the remote processor bring-up (PAS) sequence
> > hypervisor-independent, the auth_and_reset SMC call is now handled
> > entirely by TZ. However, the issue of IOMMU configuration remains
> > unresolved, for example a scenario, when KVM host at EL2 has no
> > knowledge of the remote processors’ IOMMU settings.  This is being
> > addressed by overlaying the IOMMU properties when the SoC runs a Linux
> > host at EL2. SMC call is being provided from the TrustZone firmware to
> > retrieve the resource table for a given subsystem.
> > 
> > There are also remote processors such as those for video, camera, and
> > graphics that do not use the remoteproc framework to manage their
> > lifecycle. Instead, they rely on the Qualcomm PAS service to
> > authenticate their firmware. These processors also need to be brought
> > out of reset when Linux is running at EL2. The client drivers for these
> > processors use the MDT loader function to load and authenticate
> > firmware. Similar to the Qualcomm remoteproc PAS driver, they also need
> > to retrieve the resource table, create a shared memory bridge
> > (shmbridge), and map the resources before bringing the processors out of
> > reset.
> > 
> > It is based on next-20251120 and tested on SA8775p which is now called
> > Lemans IOT platform and does not addresses DMA problem discussed at
> > [1] which is future scope of the series.
> > 
> > Changes in v8: https://lore.kernel.org/lkml/20251113-kvm-rproc-v7-v7-0-df4910b7c20a@oss.qualcomm.com/
> >   - Addressed suggestion from Stephen which was regarding commit message(9/14),
> >     debug log(12/14) suggestion, and return type change(4/14).
> >   - Added R-b tag on 10/14 .
> Sorry.
> 
> Did we actually come up with a cogent reason to omit the video firmware
> loading here ?
> 
> AFAIU it is required for Lemans and Glymur - leaving it out is blocking
> getting video stuff done and storing up trouble.
> 
> What exactly is the blockage - is it something you want help with ?

I replied to you here[1] and given my reason..till something concluded on
"multi-cell IOMMU[2]", I can not add video and block what is working
already.

[1]
https://lore.kernel.org/lkml/20251105081421.f6j7ks5bd4dfgr67@hu-mojha-hyd.qualcomm.com/

[2]
https://lore.kernel.org/lkml/cover.1762235099.git.charan.kalla@oss.qualcomm.com/

> 
> ---
> bod

-- 
-Mukesh Ojha

Re: [PATCH v8 00/14] Peripheral Image Loader support for Qualcomm SoCs running Linux host at EL2

[Konrad Dybcio]

On 11/21/25 12:37 PM, Mukesh Ojha wrote:
> On Fri, Nov 21, 2025 at 11:27:57AM +0000, Bryan O'Donoghue wrote:
>> On 21/11/2025 11:01, Mukesh Ojha wrote:
>>> In May 2025, we discussed the challenges at Linaro Connect 2025 [1]
>>> related to Secure PAS remoteproc enablement when Linux is running at EL2
>>> for Qualcomm SoCs.
>>>
>>> [1] https://resources.linaro.org/en/resource/sF8jXifdb9V1mUefdbfafa
>>>
>>> Below, is the summary of the discussion.
>>>
>>> Qualcomm is working to enable remote processors on the SA8775p SoC with
>>> a Linux host running at EL2. In doing so, it has encountered several
>>> challenges related to how the remoteproc framework is handled when Linux
>>> runs at EL1.
>>>
>>> One of the main challenges arises from differences in how IOMMU
>>> translation is currently managed on SoCs running the Qualcomm EL2
>>> hypervisor (QHEE), where IOMMU translation for any device is entirely
>>> owned by the hypervisor. Additionally, the firmware for remote
>>> processors does not contain a resource table, which would typically
>>> include the necessary IOMMU configuration settings.
>>>
>>> Qualcomm SoCs running with QHEE (EL2) have been utilizing the Peripheral
>>> Authentication Service (PAS) from TrustZone (TZ) firmware to securely
>>> authenticate and reset remote processors via a single SMC call,
>>> _auth_and_reset_. This call is first trapped by QHEE, which then invokes
>>> TZ for authentication. Once authentication is complete, the call returns
>>> to QHEE, which sets up the IOMMU translation scheme for the remote
>>> processors and subsequently brings them out of reset. The design of the
>>> Qualcomm EL2 hypervisor dictates that the Linux host OS running at EL1
>>> is not permitted to configure IOMMU translation for remote processors,
>>> and only a single-stage translation is configured.
>>>
>>> To make the remote processor bring-up (PAS) sequence
>>> hypervisor-independent, the auth_and_reset SMC call is now handled
>>> entirely by TZ. However, the issue of IOMMU configuration remains
>>> unresolved, for example a scenario, when KVM host at EL2 has no
>>> knowledge of the remote processors’ IOMMU settings.  This is being
>>> addressed by overlaying the IOMMU properties when the SoC runs a Linux
>>> host at EL2. SMC call is being provided from the TrustZone firmware to
>>> retrieve the resource table for a given subsystem.
>>>
>>> There are also remote processors such as those for video, camera, and
>>> graphics that do not use the remoteproc framework to manage their
>>> lifecycle. Instead, they rely on the Qualcomm PAS service to
>>> authenticate their firmware. These processors also need to be brought
>>> out of reset when Linux is running at EL2. The client drivers for these
>>> processors use the MDT loader function to load and authenticate
>>> firmware. Similar to the Qualcomm remoteproc PAS driver, they also need
>>> to retrieve the resource table, create a shared memory bridge
>>> (shmbridge), and map the resources before bringing the processors out of
>>> reset.
>>>
>>> It is based on next-20251120 and tested on SA8775p which is now called
>>> Lemans IOT platform and does not addresses DMA problem discussed at
>>> [1] which is future scope of the series.
>>>
>>> Changes in v8: https://lore.kernel.org/lkml/20251113-kvm-rproc-v7-v7-0-df4910b7c20a@oss.qualcomm.com/
>>>   - Addressed suggestion from Stephen which was regarding commit message(9/14),
>>>     debug log(12/14) suggestion, and return type change(4/14).
>>>   - Added R-b tag on 10/14 .
>> Sorry.
>>
>> Did we actually come up with a cogent reason to omit the video firmware
>> loading here ?
>>
>> AFAIU it is required for Lemans and Glymur - leaving it out is blocking
>> getting video stuff done and storing up trouble.
>>
>> What exactly is the blockage - is it something you want help with ?
> 
> I replied to you here[1] and given my reason..till something concluded on
> "multi-cell IOMMU[2]", I can not add video and block what is working
> already.
> 
> [1]
> https://lore.kernel.org/lkml/20251105081421.f6j7ks5bd4dfgr67@hu-mojha-hyd.qualcomm.com/
> 
> [2]
> https://lore.kernel.org/lkml/cover.1762235099.git.charan.kalla@oss.qualcomm.com/

I see that the following files call qcom_scm_pas_auth_.*():

drivers/firmware/qcom/qcom_scm.c
drivers/gpu/drm/msm/adreno/adreno_gpu.c
drivers/media/platform/qcom/iris/iris_firmware.c
drivers/media/platform/qcom/venus/firmware.c
drivers/net/ipa/ipa_main.c
drivers/net/wireless/ath/ath12k/ahb.c
drivers/remoteproc/qcom_q6v5_pas.c
drivers/remoteproc/qcom_wcnss.c

iris is difficult, rproc is done, adreno doesn't need it..

would ath12k_ahb or IPA be affected by this series as well?

Konrad

Re: [PATCH v8 00/14] Peripheral Image Loader support for Qualcomm SoCs running Linux host at EL2

[Mukesh Ojha]

On Fri, Nov 21, 2025 at 04:08:36PM +0100, Konrad Dybcio wrote:
> On 11/21/25 12:37 PM, Mukesh Ojha wrote:
> > On Fri, Nov 21, 2025 at 11:27:57AM +0000, Bryan O'Donoghue wrote:
> >> On 21/11/2025 11:01, Mukesh Ojha wrote:
> >>> In May 2025, we discussed the challenges at Linaro Connect 2025 [1]
> >>> related to Secure PAS remoteproc enablement when Linux is running at EL2
> >>> for Qualcomm SoCs.
> >>>
> >>> [1] https://resources.linaro.org/en/resource/sF8jXifdb9V1mUefdbfafa
> >>>
> >>> Below, is the summary of the discussion.
> >>>
> >>> Qualcomm is working to enable remote processors on the SA8775p SoC with
> >>> a Linux host running at EL2. In doing so, it has encountered several
> >>> challenges related to how the remoteproc framework is handled when Linux
> >>> runs at EL1.
> >>>
> >>> One of the main challenges arises from differences in how IOMMU
> >>> translation is currently managed on SoCs running the Qualcomm EL2
> >>> hypervisor (QHEE), where IOMMU translation for any device is entirely
> >>> owned by the hypervisor. Additionally, the firmware for remote
> >>> processors does not contain a resource table, which would typically
> >>> include the necessary IOMMU configuration settings.
> >>>
> >>> Qualcomm SoCs running with QHEE (EL2) have been utilizing the Peripheral
> >>> Authentication Service (PAS) from TrustZone (TZ) firmware to securely
> >>> authenticate and reset remote processors via a single SMC call,
> >>> _auth_and_reset_. This call is first trapped by QHEE, which then invokes
> >>> TZ for authentication. Once authentication is complete, the call returns
> >>> to QHEE, which sets up the IOMMU translation scheme for the remote
> >>> processors and subsequently brings them out of reset. The design of the
> >>> Qualcomm EL2 hypervisor dictates that the Linux host OS running at EL1
> >>> is not permitted to configure IOMMU translation for remote processors,
> >>> and only a single-stage translation is configured.
> >>>
> >>> To make the remote processor bring-up (PAS) sequence
> >>> hypervisor-independent, the auth_and_reset SMC call is now handled
> >>> entirely by TZ. However, the issue of IOMMU configuration remains
> >>> unresolved, for example a scenario, when KVM host at EL2 has no
> >>> knowledge of the remote processors’ IOMMU settings.  This is being
> >>> addressed by overlaying the IOMMU properties when the SoC runs a Linux
> >>> host at EL2. SMC call is being provided from the TrustZone firmware to
> >>> retrieve the resource table for a given subsystem.
> >>>
> >>> There are also remote processors such as those for video, camera, and
> >>> graphics that do not use the remoteproc framework to manage their
> >>> lifecycle. Instead, they rely on the Qualcomm PAS service to
> >>> authenticate their firmware. These processors also need to be brought
> >>> out of reset when Linux is running at EL2. The client drivers for these
> >>> processors use the MDT loader function to load and authenticate
> >>> firmware. Similar to the Qualcomm remoteproc PAS driver, they also need
> >>> to retrieve the resource table, create a shared memory bridge
> >>> (shmbridge), and map the resources before bringing the processors out of
> >>> reset.
> >>>
> >>> It is based on next-20251120 and tested on SA8775p which is now called
> >>> Lemans IOT platform and does not addresses DMA problem discussed at
> >>> [1] which is future scope of the series.
> >>>
> >>> Changes in v8: https://lore.kernel.org/lkml/20251113-kvm-rproc-v7-v7-0-df4910b7c20a@oss.qualcomm.com/
> >>>   - Addressed suggestion from Stephen which was regarding commit message(9/14),
> >>>     debug log(12/14) suggestion, and return type change(4/14).
> >>>   - Added R-b tag on 10/14 .
> >> Sorry.
> >>
> >> Did we actually come up with a cogent reason to omit the video firmware
> >> loading here ?
> >>
> >> AFAIU it is required for Lemans and Glymur - leaving it out is blocking
> >> getting video stuff done and storing up trouble.
> >>
> >> What exactly is the blockage - is it something you want help with ?
> > 
> > I replied to you here[1] and given my reason..till something concluded on
> > "multi-cell IOMMU[2]", I can not add video and block what is working
> > already.
> > 
> > [1]
> > https://lore.kernel.org/lkml/20251105081421.f6j7ks5bd4dfgr67@hu-mojha-hyd.qualcomm.com/
> > 
> > [2]
> > https://lore.kernel.org/lkml/cover.1762235099.git.charan.kalla@oss.qualcomm.com/
> 
> I see that the following files call qcom_scm_pas_auth_.*():
> 
> drivers/firmware/qcom/qcom_scm.c
> drivers/gpu/drm/msm/adreno/adreno_gpu.c
> drivers/media/platform/qcom/iris/iris_firmware.c
> drivers/media/platform/qcom/venus/firmware.c
> drivers/net/ipa/ipa_main.c
> drivers/net/wireless/ath/ath12k/ahb.c
> drivers/remoteproc/qcom_q6v5_pas.c
> drivers/remoteproc/qcom_wcnss.c
> 
> iris is difficult, rproc is done, adreno doesn't need it..
> 
> would ath12k_ahb or IPA be affected by this series as well?

Yes, they would be affected, and the modem as well, when Linux is
running at EL2. However, I do not see them present in any of the QLi and
targeted compute SoCs at the moment. Therefore, our firmware does not
support it yet.

> 
> Konrad

-- 
-Mukesh Ojha

Re: [PATCH v8 12/14] remoteproc: pas: Extend parse_fw callback to fetch resources via SMC call

[Konrad Dybcio]

On 11/21/25 12:01 PM, Mukesh Ojha wrote:
> Qualcomm remote processor may rely on static and dynamic resources for
> it to be functional. For most of the Qualcomm SoCs, when run with Gunyah
> or older QHEE hypervisor, all the resources whether it is static or
> dynamic, is managed by the hypervisor. Dynamic resources if it is
> present for a remote processor will always be coming from secure world
> via SMC call while static resources may be present in remote processor
> firmware binary or it may be coming from SMC call along with dynamic
> resources.
> 
> Remoteproc already has method like rproc_elf_load_rsc_table() to check
> firmware binary has resources or not and if it is not having then we
> pass NULL and zero as input resource table and its size argument
> respectively to qcom_scm_pas_get_rsc_table() and while it has resource
> present then it should pass the present resources to Trustzone(TZ) so that
> it could authenticate the present resources and append dynamic resource
> to return in output_rt argument along with authenticated resources.
> 
> Extend parse_fw callback to include SMC call to get resources from
> Trustzone and to leverage resource table parsing and mapping and
> unmapping code from the remoteproc framework.
> 
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH v8 13/14] remoteproc: qcom: pas: Enable Secure PAS support with IOMMU managed by Linux

[Konrad Dybcio]

On 11/21/25 12:01 PM, Mukesh Ojha wrote:
> Most Qualcomm platforms feature Gunyah hypervisor, which typically
> handles IOMMU configuration. This includes mapping memory regions and
> device memory resources for remote processors by intercepting
> qcom_scm_pas_auth_and_reset() calls. These mappings are later removed
> during teardown. Additionally, SHM bridge setup is required to enable
> memory protection for both remoteproc metadata and its memory regions.
> When the aforementioned hypervisor is absent, the operating system must
> perform these configurations instead.
> 
> When Linux runs as the hypervisor (@ EL2) on a SoC, it will have its
> own device tree overlay file that specifies the firmware stream ID now
> managed by Linux for a particular remote processor. If the iommus
> property is specified in the remoteproc device tree node, it indicates
> that IOMMU configuration must be handled by Linux. In this case, the
> has_iommu flag is set for the remote processor, which ensures that the
> resource table, carveouts, and SHM bridge are properly configured before
> memory is passed to TrustZone for authentication. Otherwise, the
> has_iommu flag remains unset, which indicates default behavior.
> 
> Enables Secure PAS support for remote processors when IOMMU configuration
> is managed by Linux.
> 
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---

[...]

> +	pas->pas_ctx->has_iommu = rproc->has_iommu;
> +	pas->dtb_pas_ctx->has_iommu = rproc->has_iommu;

Sorry if we've been there before, but I see that IOMMU-mapping happens
before ctx initialization.. can we drop this parameter and just use
device_iommu_mapped(ctx->dev) in qcom_scm_pas_prepare_and_auth_reset()?

Re: [PATCH v8 00/14] Peripheral Image Loader support for Qualcomm SoCs running Linux host at EL2

[Konrad Dybcio]

On 11/24/25 7:45 AM, Mukesh Ojha wrote:
> On Fri, Nov 21, 2025 at 04:08:36PM +0100, Konrad Dybcio wrote:
>> On 11/21/25 12:37 PM, Mukesh Ojha wrote:
>>> On Fri, Nov 21, 2025 at 11:27:57AM +0000, Bryan O'Donoghue wrote:
>>>> On 21/11/2025 11:01, Mukesh Ojha wrote:
>>>>> In May 2025, we discussed the challenges at Linaro Connect 2025 [1]
>>>>> related to Secure PAS remoteproc enablement when Linux is running at EL2
>>>>> for Qualcomm SoCs.
>>>>>
>>>>> [1] https://resources.linaro.org/en/resource/sF8jXifdb9V1mUefdbfafa
>>>>>
>>>>> Below, is the summary of the discussion.
>>>>>
>>>>> Qualcomm is working to enable remote processors on the SA8775p SoC with
>>>>> a Linux host running at EL2. In doing so, it has encountered several
>>>>> challenges related to how the remoteproc framework is handled when Linux
>>>>> runs at EL1.
>>>>>
>>>>> One of the main challenges arises from differences in how IOMMU
>>>>> translation is currently managed on SoCs running the Qualcomm EL2
>>>>> hypervisor (QHEE), where IOMMU translation for any device is entirely
>>>>> owned by the hypervisor. Additionally, the firmware for remote
>>>>> processors does not contain a resource table, which would typically
>>>>> include the necessary IOMMU configuration settings.
>>>>>
>>>>> Qualcomm SoCs running with QHEE (EL2) have been utilizing the Peripheral
>>>>> Authentication Service (PAS) from TrustZone (TZ) firmware to securely
>>>>> authenticate and reset remote processors via a single SMC call,
>>>>> _auth_and_reset_. This call is first trapped by QHEE, which then invokes
>>>>> TZ for authentication. Once authentication is complete, the call returns
>>>>> to QHEE, which sets up the IOMMU translation scheme for the remote
>>>>> processors and subsequently brings them out of reset. The design of the
>>>>> Qualcomm EL2 hypervisor dictates that the Linux host OS running at EL1
>>>>> is not permitted to configure IOMMU translation for remote processors,
>>>>> and only a single-stage translation is configured.
>>>>>
>>>>> To make the remote processor bring-up (PAS) sequence
>>>>> hypervisor-independent, the auth_and_reset SMC call is now handled
>>>>> entirely by TZ. However, the issue of IOMMU configuration remains
>>>>> unresolved, for example a scenario, when KVM host at EL2 has no
>>>>> knowledge of the remote processors’ IOMMU settings.  This is being
>>>>> addressed by overlaying the IOMMU properties when the SoC runs a Linux
>>>>> host at EL2. SMC call is being provided from the TrustZone firmware to
>>>>> retrieve the resource table for a given subsystem.
>>>>>
>>>>> There are also remote processors such as those for video, camera, and
>>>>> graphics that do not use the remoteproc framework to manage their
>>>>> lifecycle. Instead, they rely on the Qualcomm PAS service to
>>>>> authenticate their firmware. These processors also need to be brought
>>>>> out of reset when Linux is running at EL2. The client drivers for these
>>>>> processors use the MDT loader function to load and authenticate
>>>>> firmware. Similar to the Qualcomm remoteproc PAS driver, they also need
>>>>> to retrieve the resource table, create a shared memory bridge
>>>>> (shmbridge), and map the resources before bringing the processors out of
>>>>> reset.
>>>>>
>>>>> It is based on next-20251120 and tested on SA8775p which is now called
>>>>> Lemans IOT platform and does not addresses DMA problem discussed at
>>>>> [1] which is future scope of the series.
>>>>>
>>>>> Changes in v8: https://lore.kernel.org/lkml/20251113-kvm-rproc-v7-v7-0-df4910b7c20a@oss.qualcomm.com/
>>>>>   - Addressed suggestion from Stephen which was regarding commit message(9/14),
>>>>>     debug log(12/14) suggestion, and return type change(4/14).
>>>>>   - Added R-b tag on 10/14 .
>>>> Sorry.
>>>>
>>>> Did we actually come up with a cogent reason to omit the video firmware
>>>> loading here ?
>>>>
>>>> AFAIU it is required for Lemans and Glymur - leaving it out is blocking
>>>> getting video stuff done and storing up trouble.
>>>>
>>>> What exactly is the blockage - is it something you want help with ?
>>>
>>> I replied to you here[1] and given my reason..till something concluded on
>>> "multi-cell IOMMU[2]", I can not add video and block what is working
>>> already.
>>>
>>> [1]
>>> https://lore.kernel.org/lkml/20251105081421.f6j7ks5bd4dfgr67@hu-mojha-hyd.qualcomm.com/
>>>
>>> [2]
>>> https://lore.kernel.org/lkml/cover.1762235099.git.charan.kalla@oss.qualcomm.com/
>>
>> I see that the following files call qcom_scm_pas_auth_.*():
>>
>> drivers/firmware/qcom/qcom_scm.c
>> drivers/gpu/drm/msm/adreno/adreno_gpu.c
>> drivers/media/platform/qcom/iris/iris_firmware.c
>> drivers/media/platform/qcom/venus/firmware.c
>> drivers/net/ipa/ipa_main.c
>> drivers/net/wireless/ath/ath12k/ahb.c
>> drivers/remoteproc/qcom_q6v5_pas.c
>> drivers/remoteproc/qcom_wcnss.c
>>
>> iris is difficult, rproc is done, adreno doesn't need it..
>>
>> would ath12k_ahb or IPA be affected by this series as well?
> 
> Yes, they would be affected, and the modem as well, when Linux is
> running at EL2. However, I do not see them present in any of the QLi and
> targeted compute SoCs at the moment. Therefore, our firmware does not
> support it yet.

Hamoa's little brother should have ath12k_ahb.. and I assume IPA is
present on at least some platforms where there's a modem (QCM6490?)

Konrad

Re: [PATCH v8 11/14] firmware: qcom_scm: Add qcom_scm_pas_get_rsc_table() to get resource table

[Konrad Dybcio]

On 11/21/25 12:01 PM, Mukesh Ojha wrote:
> Qualcomm remote processor may rely on Static and Dynamic resources for
> it to be functional. Static resources are fixed like for example,
> memory-mapped addresses required by the subsystem and dynamic
> resources, such as shared memory in DDR etc., are determined at
> runtime during the boot process.
> 
> For most of the Qualcomm SoCs, when run with Gunyah or older QHEE
> hypervisor, all the resources whether it is static or dynamic, is
> managed by the hypervisor. Dynamic resources if it is present for a
> remote processor will always be coming from secure world via SMC call
> while static resources may be present in remote processor firmware
> binary or it may be coming qcom_scm_pas_get_rsc_table() SMC call along
> with dynamic resources.
> 
> Some of the remote processor drivers, such as video, GPU, IPA, etc., do
> not check whether resources are present in their remote processor
> firmware binary. In such cases, the caller of this function should set
> input_rt and input_rt_size as NULL and zero respectively. Remoteproc
> framework has method to check whether firmware binary contain resources
> or not and they should be pass resource table pointer to input_rt and
> resource table size to input_rt_size and this will be forwarded to
> TrustZone for authentication. TrustZone will then append the dynamic
> resources and return the complete resource table in output_rt
> 
> More about documentation on resource table format can be found in
> include/linux/remoteproc.h
> 
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---

[...]

> +int qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *ctx, void *input_rt,
> +			       size_t input_rt_size, void **output_rt,
> +			       size_t *output_rt_size)
> +{
> +	unsigned int retry_num = 5;
> +	int ret;
> +
> +	do {
> +		*output_rt = kzalloc(*output_rt_size, GFP_KERNEL);
> +		if (!*output_rt)
> +			return -ENOMEM;
> +
> +		ret = __qcom_scm_pas_get_rsc_table(ctx->pas_id, input_rt,
> +						   input_rt_size, output_rt,
> +						   output_rt_size);
> +		if (ret)
> +			kfree(*output_rt);
> +
> +	} while (ret == -EAGAIN && --retry_num);

Will firmware return -EAGAIN as a result, or is this to handle the
"buffer too small case"?

I think the latter should use a different errno (EOVERFLOW?) and print
a message since we decided that it's the caller that suggests a suitable
output buffer size

In case it doesn't make sense for the caller to share their expectations,
the buffer could be allocated (and perhaps resized if necessary) internally
with some validation (i.e. a rsctable likely won't be 5 GiB)

Konrad

Re: [PATCH v8 13/14] remoteproc: qcom: pas: Enable Secure PAS support with IOMMU managed by Linux

[Mukesh Ojha]

On Mon, Nov 24, 2025 at 12:31:47PM +0100, Konrad Dybcio wrote:
> On 11/21/25 12:01 PM, Mukesh Ojha wrote:
> > Most Qualcomm platforms feature Gunyah hypervisor, which typically
> > handles IOMMU configuration. This includes mapping memory regions and
> > device memory resources for remote processors by intercepting
> > qcom_scm_pas_auth_and_reset() calls. These mappings are later removed
> > during teardown. Additionally, SHM bridge setup is required to enable
> > memory protection for both remoteproc metadata and its memory regions.
> > When the aforementioned hypervisor is absent, the operating system must
> > perform these configurations instead.
> > 
> > When Linux runs as the hypervisor (@ EL2) on a SoC, it will have its
> > own device tree overlay file that specifies the firmware stream ID now
> > managed by Linux for a particular remote processor. If the iommus
> > property is specified in the remoteproc device tree node, it indicates
> > that IOMMU configuration must be handled by Linux. In this case, the
> > has_iommu flag is set for the remote processor, which ensures that the
> > resource table, carveouts, and SHM bridge are properly configured before
> > memory is passed to TrustZone for authentication. Otherwise, the
> > has_iommu flag remains unset, which indicates default behavior.
> > 
> > Enables Secure PAS support for remote processors when IOMMU configuration
> > is managed by Linux.
> > 
> > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > ---
> 
> [...]
> 
> > +	pas->pas_ctx->has_iommu = rproc->has_iommu;
> > +	pas->dtb_pas_ctx->has_iommu = rproc->has_iommu;
> 
> Sorry if we've been there before, but I see that IOMMU-mapping happens
> before ctx initialization.. can we drop this parameter and just use
> device_iommu_mapped(ctx->dev) in qcom_scm_pas_prepare_and_auth_reset()?

You are right and I am not against it, rproc already has variable `has_iommu`
which we use in framework and vendor driver too, but what I thought,
since this thing we have to do even for Iris or other drivers who are
effected, they already have device which are behind IOMMU and if wrong
device is passed in device_iommu_mapped() instead of firmware device which
could have returned true even when Gunyah is present.

If you feel, has_iommu is not correct name, I could rename it to fw_iommu ?

-- 
-Mukesh Ojha

Re: [PATCH v8 11/14] firmware: qcom_scm: Add qcom_scm_pas_get_rsc_table() to get resource table

[Mukesh Ojha]

On Mon, Nov 24, 2025 at 12:48:31PM +0100, Konrad Dybcio wrote:
> On 11/21/25 12:01 PM, Mukesh Ojha wrote:
> > Qualcomm remote processor may rely on Static and Dynamic resources for
> > it to be functional. Static resources are fixed like for example,
> > memory-mapped addresses required by the subsystem and dynamic
> > resources, such as shared memory in DDR etc., are determined at
> > runtime during the boot process.
> > 
> > For most of the Qualcomm SoCs, when run with Gunyah or older QHEE
> > hypervisor, all the resources whether it is static or dynamic, is
> > managed by the hypervisor. Dynamic resources if it is present for a
> > remote processor will always be coming from secure world via SMC call
> > while static resources may be present in remote processor firmware
> > binary or it may be coming qcom_scm_pas_get_rsc_table() SMC call along
> > with dynamic resources.
> > 
> > Some of the remote processor drivers, such as video, GPU, IPA, etc., do
> > not check whether resources are present in their remote processor
> > firmware binary. In such cases, the caller of this function should set
> > input_rt and input_rt_size as NULL and zero respectively. Remoteproc
> > framework has method to check whether firmware binary contain resources
> > or not and they should be pass resource table pointer to input_rt and
> > resource table size to input_rt_size and this will be forwarded to
> > TrustZone for authentication. TrustZone will then append the dynamic
> > resources and return the complete resource table in output_rt
> > 
> > More about documentation on resource table format can be found in
> > include/linux/remoteproc.h
> > 
> > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > ---
> 
> [...]
> 
> > +int qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *ctx, void *input_rt,
> > +			       size_t input_rt_size, void **output_rt,
> > +			       size_t *output_rt_size)
> > +{
> > +	unsigned int retry_num = 5;
> > +	int ret;
> > +
> > +	do {
> > +		*output_rt = kzalloc(*output_rt_size, GFP_KERNEL);
> > +		if (!*output_rt)
> > +			return -ENOMEM;
> > +
> > +		ret = __qcom_scm_pas_get_rsc_table(ctx->pas_id, input_rt,
> > +						   input_rt_size, output_rt,
> > +						   output_rt_size);
> > +		if (ret)
> > +			kfree(*output_rt);
> > +
> > +	} while (ret == -EAGAIN && --retry_num);
> 
> Will firmware return -EAGAIN as a result, or is this to handle the
> "buffer too small case"?

The latter one where a re-attempt could pass..

> 
> I think the latter should use a different errno (EOVERFLOW?) and print
> a message since we decided that it's the caller that suggests a suitable
> output buffer size

Agree with error code..

This is kept on the caller side keeping future in mind. where we can have
resource table coming from the client and it needs to go to TZ for
authentication.

Are you suggesting to move this retry on the caller side ?

Just for information, once video patches becomes ready, we may bring this
qcom_mdt_pas_map_devmem_rscs()[1] helper for video or any other client
should be do this here as well ?

I wanted to optimize this path, where caller is making a guess and
gets the updated output size in return.

[1]
https://lore.kernel.org/lkml/20250819165447.4149674-9-mukesh.ojha@oss.qualcomm.com/#t

> 
> In case it doesn't make sense for the caller to share their expectations,
> the buffer could be allocated (and perhaps resized if necessary) internally
> with some validation (i.e. a rsctable likely won't be 5 GiB)

Are you saying output_size as well should be checked and it should not be
greater than something like UINT_MAX or something.. ?

+	*output_rt_size = res.result[2];


> 
> Konrad

-- 
-Mukesh Ojha

Re: [PATCH v8 00/14] Peripheral Image Loader support for Qualcomm SoCs running Linux host at EL2

[Mukesh Ojha]

On Mon, Nov 24, 2025 at 12:33:47PM +0100, Konrad Dybcio wrote:
> On 11/24/25 7:45 AM, Mukesh Ojha wrote:
> > On Fri, Nov 21, 2025 at 04:08:36PM +0100, Konrad Dybcio wrote:
> >> On 11/21/25 12:37 PM, Mukesh Ojha wrote:
> >>> On Fri, Nov 21, 2025 at 11:27:57AM +0000, Bryan O'Donoghue wrote:
> >>>> On 21/11/2025 11:01, Mukesh Ojha wrote:
> >>>>> In May 2025, we discussed the challenges at Linaro Connect 2025 [1]
> >>>>> related to Secure PAS remoteproc enablement when Linux is running at EL2
> >>>>> for Qualcomm SoCs.
> >>>>>
> >>>>> [1] https://resources.linaro.org/en/resource/sF8jXifdb9V1mUefdbfafa
> >>>>>

[...]

> >>>>>
> >>>>> Changes in v8: https://lore.kernel.org/lkml/20251113-kvm-rproc-v7-v7-0-df4910b7c20a@oss.qualcomm.com/
> >>>>>   - Addressed suggestion from Stephen which was regarding commit message(9/14),
> >>>>>     debug log(12/14) suggestion, and return type change(4/14).
> >>>>>   - Added R-b tag on 10/14 .
> >>>> Sorry.
> >>>>
> >>>> Did we actually come up with a cogent reason to omit the video firmware
> >>>> loading here ?
> >>>>
> >>>> AFAIU it is required for Lemans and Glymur - leaving it out is blocking
> >>>> getting video stuff done and storing up trouble.
> >>>>
> >>>> What exactly is the blockage - is it something you want help with ?
> >>>
> >>> I replied to you here[1] and given my reason..till something concluded on
> >>> "multi-cell IOMMU[2]", I can not add video and block what is working
> >>> already.
> >>>
> >>> [1]
> >>> https://lore.kernel.org/lkml/20251105081421.f6j7ks5bd4dfgr67@hu-mojha-hyd.qualcomm.com/
> >>>
> >>> [2]
> >>> https://lore.kernel.org/lkml/cover.1762235099.git.charan.kalla@oss.qualcomm.com/
> >>
> >> I see that the following files call qcom_scm_pas_auth_.*():
> >>
> >> drivers/firmware/qcom/qcom_scm.c
> >> drivers/gpu/drm/msm/adreno/adreno_gpu.c
> >> drivers/media/platform/qcom/iris/iris_firmware.c
> >> drivers/media/platform/qcom/venus/firmware.c
> >> drivers/net/ipa/ipa_main.c
> >> drivers/net/wireless/ath/ath12k/ahb.c
> >> drivers/remoteproc/qcom_q6v5_pas.c
> >> drivers/remoteproc/qcom_wcnss.c
> >>
> >> iris is difficult, rproc is done, adreno doesn't need it..
> >>
> >> would ath12k_ahb or IPA be affected by this series as well?
> > 
> > Yes, they would be affected, and the modem as well, when Linux is
> > running at EL2. However, I do not see them present in any of the QLi and
> > targeted compute SoCs at the moment. Therefore, our firmware does not
> > support it yet.
> 
> Hamoa's little brother should have ath12k_ahb.. and I assume IPA is
> present on at least some platforms where there's a modem (QCM6490?)

Sure, We will add the support for IP's when there is a need for SoC running Linux
at EL2 (backed by firmware).

> 
> Konrad

-- 
-Mukesh Ojha

Re: [PATCH v8 13/14] remoteproc: qcom: pas: Enable Secure PAS support with IOMMU managed by Linux

[Bjorn Andersson]

On Mon, Nov 24, 2025 at 05:33:18PM +0530, Mukesh Ojha wrote:
> On Mon, Nov 24, 2025 at 12:31:47PM +0100, Konrad Dybcio wrote:
> > On 11/21/25 12:01 PM, Mukesh Ojha wrote:
> > > Most Qualcomm platforms feature Gunyah hypervisor, which typically
> > > handles IOMMU configuration. This includes mapping memory regions and
> > > device memory resources for remote processors by intercepting
> > > qcom_scm_pas_auth_and_reset() calls. These mappings are later removed
> > > during teardown. Additionally, SHM bridge setup is required to enable
> > > memory protection for both remoteproc metadata and its memory regions.
> > > When the aforementioned hypervisor is absent, the operating system must
> > > perform these configurations instead.
> > > 
> > > When Linux runs as the hypervisor (@ EL2) on a SoC, it will have its
> > > own device tree overlay file that specifies the firmware stream ID now
> > > managed by Linux for a particular remote processor. If the iommus
> > > property is specified in the remoteproc device tree node, it indicates
> > > that IOMMU configuration must be handled by Linux. In this case, the
> > > has_iommu flag is set for the remote processor, which ensures that the
> > > resource table, carveouts, and SHM bridge are properly configured before
> > > memory is passed to TrustZone for authentication. Otherwise, the
> > > has_iommu flag remains unset, which indicates default behavior.
> > > 
> > > Enables Secure PAS support for remote processors when IOMMU configuration
> > > is managed by Linux.
> > > 
> > > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > > ---
> > 
> > [...]
> > 
> > > +	pas->pas_ctx->has_iommu = rproc->has_iommu;
> > > +	pas->dtb_pas_ctx->has_iommu = rproc->has_iommu;
> > 
> > Sorry if we've been there before, but I see that IOMMU-mapping happens
> > before ctx initialization.. can we drop this parameter and just use
> > device_iommu_mapped(ctx->dev) in qcom_scm_pas_prepare_and_auth_reset()?
> 
> You are right and I am not against it, rproc already has variable `has_iommu`
> which we use in framework and vendor driver too, but what I thought,
> since this thing we have to do even for Iris or other drivers who are
> effected, they already have device which are behind IOMMU and if wrong
> device is passed in device_iommu_mapped() instead of firmware device which
> could have returned true even when Gunyah is present.
> 
> If you feel, has_iommu is not correct name, I could rename it to fw_iommu ?
> 

While this does relate to "has_iommu" and/or "fw_iommu" when it comes to
the current PAS context, the "feature flag" is "should we use tzmem or
not".

Further, in the case of the modem, we don't have an IOMMU, but we still
need to set this flag on the ctx in order to get the metadata into TZ.

So, I think this should be detached from the "iommu". How about naming
the "has_iommu" in the context to "use_tzmem"?

Regards,
Bjorn

> -- 
> -Mukesh Ojha

Re: [PATCH v8 13/14] remoteproc: qcom: pas: Enable Secure PAS support with IOMMU managed by Linux

[Mukesh Ojha]

On Wed, Nov 26, 2025 at 10:40:51AM -0600, Bjorn Andersson wrote:
> On Mon, Nov 24, 2025 at 05:33:18PM +0530, Mukesh Ojha wrote:
> > On Mon, Nov 24, 2025 at 12:31:47PM +0100, Konrad Dybcio wrote:
> > > On 11/21/25 12:01 PM, Mukesh Ojha wrote:
> > > > Most Qualcomm platforms feature Gunyah hypervisor, which typically
> > > > handles IOMMU configuration. This includes mapping memory regions and
> > > > device memory resources for remote processors by intercepting
> > > > qcom_scm_pas_auth_and_reset() calls. These mappings are later removed
> > > > during teardown. Additionally, SHM bridge setup is required to enable
> > > > memory protection for both remoteproc metadata and its memory regions.
> > > > When the aforementioned hypervisor is absent, the operating system must
> > > > perform these configurations instead.
> > > > 
> > > > When Linux runs as the hypervisor (@ EL2) on a SoC, it will have its
> > > > own device tree overlay file that specifies the firmware stream ID now
> > > > managed by Linux for a particular remote processor. If the iommus
> > > > property is specified in the remoteproc device tree node, it indicates
> > > > that IOMMU configuration must be handled by Linux. In this case, the
> > > > has_iommu flag is set for the remote processor, which ensures that the
> > > > resource table, carveouts, and SHM bridge are properly configured before
> > > > memory is passed to TrustZone for authentication. Otherwise, the
> > > > has_iommu flag remains unset, which indicates default behavior.
> > > > 
> > > > Enables Secure PAS support for remote processors when IOMMU configuration
> > > > is managed by Linux.
> > > > 
> > > > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > > > ---
> > > 
> > > [...]
> > > 
> > > > +	pas->pas_ctx->has_iommu = rproc->has_iommu;
> > > > +	pas->dtb_pas_ctx->has_iommu = rproc->has_iommu;
> > > 
> > > Sorry if we've been there before, but I see that IOMMU-mapping happens
> > > before ctx initialization.. can we drop this parameter and just use
> > > device_iommu_mapped(ctx->dev) in qcom_scm_pas_prepare_and_auth_reset()?
> > 
> > You are right and I am not against it, rproc already has variable `has_iommu`
> > which we use in framework and vendor driver too, but what I thought,
> > since this thing we have to do even for Iris or other drivers who are
> > effected, they already have device which are behind IOMMU and if wrong
> > device is passed in device_iommu_mapped() instead of firmware device which
> > could have returned true even when Gunyah is present.
> > 
> > If you feel, has_iommu is not correct name, I could rename it to fw_iommu ?
> > 
> 
> While this does relate to "has_iommu" and/or "fw_iommu" when it comes to
> the current PAS context, the "feature flag" is "should we use tzmem or
> not".
> 
> Further, in the case of the modem, we don't have an IOMMU, but we still
> need to set this flag on the ctx in order to get the metadata into TZ.
> 
> So, I think this should be detached from the "iommu". How about naming
> the "has_iommu" in the context to "use_tzmem"?

Sure, this way it gets attached to tzmem alloc/create API to be used or
not for PIL SMC calls.

> 
> Regards,
> Bjorn
> 
> > -- 
> > -Mukesh Ojha

-- 
-Mukesh Ojha

Re: [PATCH v8 00/14] Peripheral Image Loader support for Qualcomm SoCs running Linux host at EL2

[Bryan O'Donoghue]

On 21/11/2025 11:37, Mukesh Ojha wrote:
>> Sorry.
>>
>> Did we actually come up with a cogent reason to omit the video firmware
>> loading here ?
>>
>> AFAIU it is required for Lemans and Glymur - leaving it out is blocking
>> getting video stuff done and storing up trouble.
>>
>> What exactly is the blockage - is it something you want help with ?
> I replied to you here[1] and given my reason..till something concluded on
> "multi-cell IOMMU[2]", I can not add video and block what is working
> already.
> 
> [1]
> https://lore.kernel.org/lkml/20251105081421.f6j7ks5bd4dfgr67@hu-mojha- 
> hyd.qualcomm.com/

Why though ?

You are mixing together the issue of multiple SIDs and the original 
loading of firmware which could easily reuse the venus method of

&iris {
	video-firmware {
		iommus = <&apss_smmu hex>;
	};
};

That binding got dropped because it was unused in Iris.

https://lore.kernel.org/lkml/05d40a3b-cc13-b704-cac7-0ecbeea0e59d@quicinc.com/

I still fail to see why we are waiting for multi-cell IOMMU to land, 
when it is expected to and what the VPU enablement story is upstream in 
the meantime.

Blocked it seems.

---
bod

Re: [PATCH v8 00/14] Peripheral Image Loader support for Qualcomm SoCs running Linux host at EL2

[Mukesh Ojha]

On Thu, Nov 27, 2025 at 10:25:23AM +0000, Bryan O'Donoghue wrote:
> On 21/11/2025 11:37, Mukesh Ojha wrote:
> > > Sorry.
> > > 
> > > Did we actually come up with a cogent reason to omit the video firmware
> > > loading here ?
> > > 
> > > AFAIU it is required for Lemans and Glymur - leaving it out is blocking
> > > getting video stuff done and storing up trouble.
> > > 
> > > What exactly is the blockage - is it something you want help with ?
> > I replied to you here[1] and given my reason..till something concluded on
> > "multi-cell IOMMU[2]", I can not add video and block what is working
> > already.
> > 
> > [1]
> > https://lore.kernel.org/lkml/20251105081421.f6j7ks5bd4dfgr67@hu-mojha-
> > hyd.qualcomm.com/
> 
> Why though ?
> 
> You are mixing together the issue of multiple SIDs and the original loading
> of firmware which could easily reuse the venus method of
> 
> &iris {
> 	video-firmware {
> 		iommus = <&apss_smmu hex>;
> 	};
> };

I completely understand what you are saying, and it would be very easy
for me to do that if it gets accepted. However, I doubt that the people
who raised this concern would agree with the approach.

I’m not sure if the video team would like to pursue pixel/non-pixel/firmware context
banks separately. I’ll leave this to @Vikas to answer.

Also, I do not want the video PIL discussion to be part of this series, as it could
unnecessarily give the impression that this series depends on it.

> 
> That binding got dropped because it was unused in Iris.
> 
> https://lore.kernel.org/lkml/05d40a3b-cc13-b704-cac7-0ecbeea0e59d@quicinc.com/
> 
> I still fail to see why we are waiting for multi-cell IOMMU to land, when it
> is expected to and what the VPU enablement story is upstream in the
> meantime.
> 
> Blocked it seems.

No, it is ongoing, there will be next version coming.

> 
> ---
> bod

-- 
-Mukesh Ojha

Re: [PATCH v8 00/14] Peripheral Image Loader support for Qualcomm SoCs running Linux host at EL2

[Vikash Garodia]

On 12/2/2025 2:06 PM, Mukesh Ojha wrote:
> On Thu, Nov 27, 2025 at 10:25:23AM +0000, Bryan O'Donoghue wrote:
>> On 21/11/2025 11:37, Mukesh Ojha wrote:
>>>> Sorry.
>>>>
>>>> Did we actually come up with a cogent reason to omit the video firmware
>>>> loading here ?
>>>>
>>>> AFAIU it is required for Lemans and Glymur - leaving it out is blocking
>>>> getting video stuff done and storing up trouble.
>>>>
>>>> What exactly is the blockage - is it something you want help with ?
>>> I replied to you here[1] and given my reason..till something concluded on
>>> "multi-cell IOMMU[2]", I can not add video and block what is working
>>> already.
>>>
>>> [1]
>>> https://lore.kernel.org/lkml/20251105081421.f6j7ks5bd4dfgr67@hu-mojha-
>>> hyd.qualcomm.com/
>>
>> Why though ?
>>
>> You are mixing together the issue of multiple SIDs and the original loading
>> of firmware which could easily reuse the venus method of
>>
>> &iris {
>> 	video-firmware {
>> 		iommus = <&apss_smmu hex>;
>> 	};
>> };
> 
> I completely understand what you are saying, and it would be very easy
> for me to do that if it gets accepted. However, I doubt that the people
> who raised this concern would agree with the approach.
> 
> I’m not sure if the video team would like to pursue pixel/non-pixel/firmware context
> banks separately. I’ll leave this to @Vikas to answer.

Not exactly as a separate sub-node, but i do like the idea of 
introducing a simple iommu property, something like this, which Stephan 
proposed earlier in the discussion [1]

firmware-iommus = <&apps_smmu ...>;

I understand that we are doing the iommu-map thing, but a property 
exclusively for firmware like above look much simpler to me.

Dmitry/ Bryan/ Krzysztof if you are good with this, we can bring back 
video in this series. Please share your thoughts on this.

Regards,
Vikash

[1] https://lore.kernel.org/lkml/aKooCFoV3ZYwOMRx@linaro.org/

> 
> Also, I do not want the video PIL discussion to be part of this series, as it could
> unnecessarily give the impression that this series depends on it.
> 
>>
>> That binding got dropped because it was unused in Iris.
>>
>> https://lore.kernel.org/lkml/05d40a3b-cc13-b704-cac7-0ecbeea0e59d@quicinc.com/
>>
>> I still fail to see why we are waiting for multi-cell IOMMU to land, when it
>> is expected to and what the VPU enablement story is upstream in the
>> meantime.
>>
>> Blocked it seems.
> 
> No, it is ongoing, there will be next version coming.
> 
>>
>> ---
>> bod
> 

Re: [PATCH v8 00/14] Peripheral Image Loader support for Qualcomm SoCs running Linux host at EL2

[Bjorn Andersson]

On Tue, Dec 02, 2025 at 03:43:17PM +0530, Vikash Garodia wrote:
> 
> On 12/2/2025 2:06 PM, Mukesh Ojha wrote:
> > On Thu, Nov 27, 2025 at 10:25:23AM +0000, Bryan O'Donoghue wrote:
> > > On 21/11/2025 11:37, Mukesh Ojha wrote:
> > > > > Sorry.
> > > > > 
> > > > > Did we actually come up with a cogent reason to omit the video firmware
> > > > > loading here ?
> > > > > 
> > > > > AFAIU it is required for Lemans and Glymur - leaving it out is blocking
> > > > > getting video stuff done and storing up trouble.
> > > > > 
> > > > > What exactly is the blockage - is it something you want help with ?
> > > > I replied to you here[1] and given my reason..till something concluded on
> > > > "multi-cell IOMMU[2]", I can not add video and block what is working
> > > > already.
> > > > 
> > > > [1]
> > > > https://lore.kernel.org/lkml/20251105081421.f6j7ks5bd4dfgr67@hu-mojha-
> > > > hyd.qualcomm.com/
> > > 
> > > Why though ?
> > > 
> > > You are mixing together the issue of multiple SIDs and the original loading
> > > of firmware which could easily reuse the venus method of
> > > 
> > > &iris {
> > > 	video-firmware {
> > > 		iommus = <&apss_smmu hex>;
> > > 	};
> > > };
> > 
> > I completely understand what you are saying, and it would be very easy
> > for me to do that if it gets accepted. However, I doubt that the people
> > who raised this concern would agree with the approach.
> > 
> > I’m not sure if the video team would like to pursue pixel/non-pixel/firmware context
> > banks separately. I’ll leave this to @Vikas to answer.
> 
> Not exactly as a separate sub-node, but i do like the idea of introducing a
> simple iommu property, something like this, which Stephan proposed earlier
> in the discussion [1]
> 
> firmware-iommus = <&apps_smmu ...>;
> 
> I understand that we are doing the iommu-map thing, but a property
> exclusively for firmware like above look much simpler to me.
> 

"We know we need to find a generic solution to this very problem, but
while we work on that let's add this quick hack to the ABI"?

> Dmitry/ Bryan/ Krzysztof if you are good with this, we can bring back video
> in this series. Please share your thoughts on this.
> 

Please let's keep these concerns separate, so that we don't hold this
series up further. Even if we choose to go by the subnode approach, in
the same time frame, it's better to discuss it separately.

Regards,
Bjorn

> Regards,
> Vikash
> 
> [1] https://lore.kernel.org/lkml/aKooCFoV3ZYwOMRx@linaro.org/
> 
> > 
> > Also, I do not want the video PIL discussion to be part of this series, as it could
> > unnecessarily give the impression that this series depends on it.
> > 
> > > 
> > > That binding got dropped because it was unused in Iris.
> > > 
> > > https://lore.kernel.org/lkml/05d40a3b-cc13-b704-cac7-0ecbeea0e59d@quicinc.com/
> > > 
> > > I still fail to see why we are waiting for multi-cell IOMMU to land, when it
> > > is expected to and what the VPU enablement story is upstream in the
> > > meantime.
> > > 
> > > Blocked it seems.
> > 
> > No, it is ongoing, there will be next version coming.
> > 
> > > 
> > > ---
> > > bod
> > 
> 

Re: [PATCH v8 00/14] Peripheral Image Loader support for Qualcomm SoCs running Linux host at EL2

[Vikash Garodia]

On 12/3/2025 2:54 AM, Bjorn Andersson wrote:
> On Tue, Dec 02, 2025 at 03:43:17PM +0530, Vikash Garodia wrote:
>>
>> On 12/2/2025 2:06 PM, Mukesh Ojha wrote:
>>> On Thu, Nov 27, 2025 at 10:25:23AM +0000, Bryan O'Donoghue wrote:
>>>> On 21/11/2025 11:37, Mukesh Ojha wrote:
>>>>>> Sorry.
>>>>>>
>>>>>> Did we actually come up with a cogent reason to omit the video firmware
>>>>>> loading here ?
>>>>>>
>>>>>> AFAIU it is required for Lemans and Glymur - leaving it out is blocking
>>>>>> getting video stuff done and storing up trouble.
>>>>>>
>>>>>> What exactly is the blockage - is it something you want help with ?
>>>>> I replied to you here[1] and given my reason..till something concluded on
>>>>> "multi-cell IOMMU[2]", I can not add video and block what is working
>>>>> already.
>>>>>
>>>>> [1]
>>>>> https://lore.kernel.org/lkml/20251105081421.f6j7ks5bd4dfgr67@hu-mojha-
>>>>> hyd.qualcomm.com/
>>>>
>>>> Why though ?
>>>>
>>>> You are mixing together the issue of multiple SIDs and the original loading
>>>> of firmware which could easily reuse the venus method of
>>>>
>>>> &iris {
>>>> 	video-firmware {
>>>> 		iommus = <&apss_smmu hex>;
>>>> 	};
>>>> };
>>>
>>> I completely understand what you are saying, and it would be very easy
>>> for me to do that if it gets accepted. However, I doubt that the people
>>> who raised this concern would agree with the approach.
>>>
>>> I’m not sure if the video team would like to pursue pixel/non-pixel/firmware context
>>> banks separately. I’ll leave this to @Vikas to answer.
>>
>> Not exactly as a separate sub-node, but i do like the idea of introducing a
>> simple iommu property, something like this, which Stephan proposed earlier
>> in the discussion [1]
>>
>> firmware-iommus = <&apps_smmu ...>;
>>
>> I understand that we are doing the iommu-map thing, but a property
>> exclusively for firmware like above look much simpler to me.
>>
> 
> "We know we need to find a generic solution to this very problem, but
> while we work on that let's add this quick hack to the ABI"?

I would not call that as hack, rather a simpler solution instead of 
packing everything into the generic iommu-map.

"firmware-iommus" is much more readable to interpret something running 
in el2 mode, than digging into function ids inside iommu-map and then 
matching it up with specific SIDs to confirm.

>> Dmitry/ Bryan/ Krzysztof if you are good with this, we can bring back video
>> in this series. Please share your thoughts on this.
>>
> 
> Please let's keep these concerns separate, so that we don't hold this
> series up further. Even if we choose to go by the subnode approach, in
> the same time frame, it's better to discuss it separately.
> 

ACK.

> Regards,
> Bjorn
> 
>> Regards,
>> Vikash
>>
>> [1] https://lore.kernel.org/lkml/aKooCFoV3ZYwOMRx@linaro.org/
>>
>>>
>>> Also, I do not want the video PIL discussion to be part of this series, as it could
>>> unnecessarily give the impression that this series depends on it.
>>>
>>>>
>>>> That binding got dropped because it was unused in Iris.
>>>>
>>>> https://lore.kernel.org/lkml/05d40a3b-cc13-b704-cac7-0ecbeea0e59d@quicinc.com/
>>>>
>>>> I still fail to see why we are waiting for multi-cell IOMMU to land, when it
>>>> is expected to and what the VPU enablement story is upstream in the
>>>> meantime.
>>>>
>>>> Blocked it seems.
>>>
>>> No, it is ongoing, there will be next version coming.
>>>
>>>>
>>>> ---
>>>> bod
>>>
>>

Re: [PATCH v8 11/14] firmware: qcom_scm: Add qcom_scm_pas_get_rsc_table() to get resource table

[Konrad Dybcio]

On 11/24/25 4:25 PM, Mukesh Ojha wrote:
> On Mon, Nov 24, 2025 at 12:48:31PM +0100, Konrad Dybcio wrote:
>> On 11/21/25 12:01 PM, Mukesh Ojha wrote:
>>> Qualcomm remote processor may rely on Static and Dynamic resources for
>>> it to be functional. Static resources are fixed like for example,
>>> memory-mapped addresses required by the subsystem and dynamic
>>> resources, such as shared memory in DDR etc., are determined at
>>> runtime during the boot process.
>>>
>>> For most of the Qualcomm SoCs, when run with Gunyah or older QHEE
>>> hypervisor, all the resources whether it is static or dynamic, is
>>> managed by the hypervisor. Dynamic resources if it is present for a
>>> remote processor will always be coming from secure world via SMC call
>>> while static resources may be present in remote processor firmware
>>> binary or it may be coming qcom_scm_pas_get_rsc_table() SMC call along
>>> with dynamic resources.
>>>
>>> Some of the remote processor drivers, such as video, GPU, IPA, etc., do
>>> not check whether resources are present in their remote processor
>>> firmware binary. In such cases, the caller of this function should set
>>> input_rt and input_rt_size as NULL and zero respectively. Remoteproc
>>> framework has method to check whether firmware binary contain resources
>>> or not and they should be pass resource table pointer to input_rt and
>>> resource table size to input_rt_size and this will be forwarded to
>>> TrustZone for authentication. TrustZone will then append the dynamic
>>> resources and return the complete resource table in output_rt
>>>
>>> More about documentation on resource table format can be found in
>>> include/linux/remoteproc.h
>>>
>>> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
>>> ---
>>
>> [...]
>>
>>> +int qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *ctx, void *input_rt,
>>> +			       size_t input_rt_size, void **output_rt,
>>> +			       size_t *output_rt_size)
>>> +{
>>> +	unsigned int retry_num = 5;
>>> +	int ret;
>>> +
>>> +	do {
>>> +		*output_rt = kzalloc(*output_rt_size, GFP_KERNEL);
>>> +		if (!*output_rt)
>>> +			return -ENOMEM;
>>> +
>>> +		ret = __qcom_scm_pas_get_rsc_table(ctx->pas_id, input_rt,
>>> +						   input_rt_size, output_rt,
>>> +						   output_rt_size);
>>> +		if (ret)
>>> +			kfree(*output_rt);
>>> +
>>> +	} while (ret == -EAGAIN && --retry_num);
>>
>> Will firmware return -EAGAIN as a result, or is this to handle the
>> "buffer too small case"?
> 
> The latter one where a re-attempt could pass..
> 
>>
>> I think the latter should use a different errno (EOVERFLOW?) and print
>> a message since we decided that it's the caller that suggests a suitable
>> output buffer size
> 
> Agree with error code..
> 
> This is kept on the caller side keeping future in mind. where we can have
> resource table coming from the client and it needs to go to TZ for
> authentication.
> 
> Are you suggesting to move this retry on the caller side ?

I think we got confused in the review of the previous iterations and made
qcom_scm_pas_get_rsc_table() retry 5 times (on the basis that "some" error
could happen in firmware), but if it's specifically "buf too small", we should
only need to call it utmost twice - once to get the required larger size (or
succeed and exit) and another one with a now-correctly sized buffer.

Looking at it again, do we really need to be so stringent about the maximum
resource table size? Can we just push the currently defined SZ_16K inside
qcom_scm_pas_get_rsc_table() as a constant and bump it up as necessary in
the future?

> Just for information, once video patches becomes ready, we may bring this
> qcom_mdt_pas_map_devmem_rscs()[1] helper for video or any other client
> should be do this here as well ?
> 
> I wanted to optimize this path, where caller is making a guess and
> gets the updated output size in return.

We always end up allocating in __qcom_scm_pas_get_rsc_table() so I think
guessing a number like SZ_16K which is plenty for a effectively small u64[]
in this file is ok too. Perhaps we could even shrink it down a bit..

> [1]
> https://lore.kernel.org/lkml/20250819165447.4149674-9-mukesh.ojha@oss.qualcomm.com/#t
> 
>>
>> In case it doesn't make sense for the caller to share their expectations,
>> the buffer could be allocated (and perhaps resized if necessary) internally
>> with some validation (i.e. a rsctable likely won't be 5 GiB)
> 
> Are you saying output_size as well should be checked and it should not be
> greater than something like UINT_MAX or something.. ?
> 
> +	*output_rt_size = res.result[2];

Yeah we should probably make sure this doesn't exceed a large-but-not-
entirely-unreasonable value

Konrad

Re: [PATCH v8 11/14] firmware: qcom_scm: Add qcom_scm_pas_get_rsc_table() to get resource table

[Mukesh Ojha]

On Wed, Dec 03, 2025 at 01:36:32PM +0100, Konrad Dybcio wrote:
> On 11/24/25 4:25 PM, Mukesh Ojha wrote:
> > On Mon, Nov 24, 2025 at 12:48:31PM +0100, Konrad Dybcio wrote:
> >> On 11/21/25 12:01 PM, Mukesh Ojha wrote:
> >>> Qualcomm remote processor may rely on Static and Dynamic resources for
> >>> it to be functional. Static resources are fixed like for example,
> >>> memory-mapped addresses required by the subsystem and dynamic
> >>> resources, such as shared memory in DDR etc., are determined at
> >>> runtime during the boot process.
> >>>
> >>> For most of the Qualcomm SoCs, when run with Gunyah or older QHEE
> >>> hypervisor, all the resources whether it is static or dynamic, is
> >>> managed by the hypervisor. Dynamic resources if it is present for a
> >>> remote processor will always be coming from secure world via SMC call
> >>> while static resources may be present in remote processor firmware
> >>> binary or it may be coming qcom_scm_pas_get_rsc_table() SMC call along
> >>> with dynamic resources.
> >>>
> >>> Some of the remote processor drivers, such as video, GPU, IPA, etc., do
> >>> not check whether resources are present in their remote processor
> >>> firmware binary. In such cases, the caller of this function should set
> >>> input_rt and input_rt_size as NULL and zero respectively. Remoteproc
> >>> framework has method to check whether firmware binary contain resources
> >>> or not and they should be pass resource table pointer to input_rt and
> >>> resource table size to input_rt_size and this will be forwarded to
> >>> TrustZone for authentication. TrustZone will then append the dynamic
> >>> resources and return the complete resource table in output_rt
> >>>
> >>> More about documentation on resource table format can be found in
> >>> include/linux/remoteproc.h
> >>>
> >>> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> >>> ---
> >>
> >> [...]
> >>
> >>> +int qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *ctx, void *input_rt,
> >>> +			       size_t input_rt_size, void **output_rt,
> >>> +			       size_t *output_rt_size)
> >>> +{
> >>> +	unsigned int retry_num = 5;
> >>> +	int ret;
> >>> +
> >>> +	do {
> >>> +		*output_rt = kzalloc(*output_rt_size, GFP_KERNEL);
> >>> +		if (!*output_rt)
> >>> +			return -ENOMEM;
> >>> +
> >>> +		ret = __qcom_scm_pas_get_rsc_table(ctx->pas_id, input_rt,
> >>> +						   input_rt_size, output_rt,
> >>> +						   output_rt_size);
> >>> +		if (ret)
> >>> +			kfree(*output_rt);
> >>> +
> >>> +	} while (ret == -EAGAIN && --retry_num);
> >>
> >> Will firmware return -EAGAIN as a result, or is this to handle the
> >> "buffer too small case"?
> > 
> > The latter one where a re-attempt could pass..
> > 
> >>
> >> I think the latter should use a different errno (EOVERFLOW?) and print
> >> a message since we decided that it's the caller that suggests a suitable
> >> output buffer size
> > 
> > Agree with error code..
> > 
> > This is kept on the caller side keeping future in mind. where we can have
> > resource table coming from the client and it needs to go to TZ for
> > authentication.
> > 
> > Are you suggesting to move this retry on the caller side ?
> 
> I think we got confused in the review of the previous iterations and made
> qcom_scm_pas_get_rsc_table() retry 5 times (on the basis that "some" error
> could happen in firmware), but if it's specifically "buf too small", we should
> only need to call it utmost twice - once to get the required larger size (or
> succeed and exit) and another one with a now-correctly sized buffer.

Ack., thanks for clarifying.

> 
> Looking at it again, do we really need to be so stringent about the maximum
> resource table size? Can we just push the currently defined SZ_16K inside
> qcom_scm_pas_get_rsc_table() as a constant and bump it up as necessary in
> the future?

Ack.

> 
> > Just for information, once video patches becomes ready, we may bring this
> > qcom_mdt_pas_map_devmem_rscs()[1] helper for video or any other client
> > should be do this here as well ?
> > 
> > I wanted to optimize this path, where caller is making a guess and
> > gets the updated output size in return.
> 
> We always end up allocating in __qcom_scm_pas_get_rsc_table() so I think
> guessing a number like SZ_16K which is plenty for a effectively small u64[]
> in this file is ok too. Perhaps we could even shrink it down a bit..

Just to avoid iteration, are you suggesting that we can keep this
guesswork as part of __qcom_scm_pas_get_rsc_table() and start with
something smaller than SZ_16K?

I kind of agree with the first part, but SZ_16K was the recommended size
from the firmware for Lemans to start with, in order to pass the SMC
successfully on the first try. However, the same size was failing for
Glymur, and it required a second attempt with the correct size.

> 
> > [1]
> > https://lore.kernel.org/lkml/20250819165447.4149674-9-mukesh.ojha@oss.qualcomm.com/#t
> > 
> >>
> >> In case it doesn't make sense for the caller to share their expectations,
> >> the buffer could be allocated (and perhaps resized if necessary) internally
> >> with some validation (i.e. a rsctable likely won't be 5 GiB)
> > 
> > Are you saying output_size as well should be checked and it should not be
> > greater than something like UINT_MAX or something.. ?
> > 
> > +	*output_rt_size = res.result[2];
> 
> Yeah we should probably make sure this doesn't exceed a large-but-not-
> entirely-unreasonable value

Ack.

> 
> Konrad

-- 
-Mukesh Ojha

Re: [PATCH v8 11/14] firmware: qcom_scm: Add qcom_scm_pas_get_rsc_table() to get resource table

[Konrad Dybcio]

On 12/4/25 1:28 PM, Mukesh Ojha wrote:
> On Wed, Dec 03, 2025 at 01:36:32PM +0100, Konrad Dybcio wrote:
>> On 11/24/25 4:25 PM, Mukesh Ojha wrote:
>>> On Mon, Nov 24, 2025 at 12:48:31PM +0100, Konrad Dybcio wrote:
>>>> On 11/21/25 12:01 PM, Mukesh Ojha wrote:
>>>>> Qualcomm remote processor may rely on Static and Dynamic resources for
>>>>> it to be functional. Static resources are fixed like for example,
>>>>> memory-mapped addresses required by the subsystem and dynamic
>>>>> resources, such as shared memory in DDR etc., are determined at
>>>>> runtime during the boot process.
>>>>>
>>>>> For most of the Qualcomm SoCs, when run with Gunyah or older QHEE
>>>>> hypervisor, all the resources whether it is static or dynamic, is
>>>>> managed by the hypervisor. Dynamic resources if it is present for a
>>>>> remote processor will always be coming from secure world via SMC call
>>>>> while static resources may be present in remote processor firmware
>>>>> binary or it may be coming qcom_scm_pas_get_rsc_table() SMC call along
>>>>> with dynamic resources.

[...]

> Just to avoid iteration, are you suggesting that we can keep this
> guesswork as part of __qcom_scm_pas_get_rsc_table() and start with
> something smaller than SZ_16K?
> 
> I kind of agree with the first part, but SZ_16K was the recommended size
> from the firmware for Lemans to start with, in order to pass the SMC
> successfully on the first try. However, the same size was failing for
> Glymur, and it required a second attempt with the correct size.

It depends on the payload, which you're probably much more familiar with.
If 95% of them will be closer to e.g. 1K in size, it perhaps makes sense
to use up the additional few dozen cycles on our amazingly fast CPUs and
retry as necessary, instead of blindly reserving a whole bunch of memory.

Konrad

Re: [PATCH v8 00/14] Peripheral Image Loader support for Qualcomm SoCs running Linux host at EL2

[Dmitry Baryshkov]

On Wed, Dec 03, 2025 at 10:48:14AM +0530, Vikash Garodia wrote:
> 
> On 12/3/2025 2:54 AM, Bjorn Andersson wrote:
> > On Tue, Dec 02, 2025 at 03:43:17PM +0530, Vikash Garodia wrote:
> > > 
> > > On 12/2/2025 2:06 PM, Mukesh Ojha wrote:
> > > > On Thu, Nov 27, 2025 at 10:25:23AM +0000, Bryan O'Donoghue wrote:
> > > > > On 21/11/2025 11:37, Mukesh Ojha wrote:
> > > > > > > Sorry.
> > > > > > > 
> > > > > > > Did we actually come up with a cogent reason to omit the video firmware
> > > > > > > loading here ?
> > > > > > > 
> > > > > > > AFAIU it is required for Lemans and Glymur - leaving it out is blocking
> > > > > > > getting video stuff done and storing up trouble.
> > > > > > > 
> > > > > > > What exactly is the blockage - is it something you want help with ?
> > > > > > I replied to you here[1] and given my reason..till something concluded on
> > > > > > "multi-cell IOMMU[2]", I can not add video and block what is working
> > > > > > already.
> > > > > > 
> > > > > > [1]
> > > > > > https://lore.kernel.org/lkml/20251105081421.f6j7ks5bd4dfgr67@hu-mojha-
> > > > > > hyd.qualcomm.com/
> > > > > 
> > > > > Why though ?
> > > > > 
> > > > > You are mixing together the issue of multiple SIDs and the original loading
> > > > > of firmware which could easily reuse the venus method of
> > > > > 
> > > > > &iris {
> > > > > 	video-firmware {
> > > > > 		iommus = <&apss_smmu hex>;
> > > > > 	};
> > > > > };
> > > > 
> > > > I completely understand what you are saying, and it would be very easy
> > > > for me to do that if it gets accepted. However, I doubt that the people
> > > > who raised this concern would agree with the approach.
> > > > 
> > > > I’m not sure if the video team would like to pursue pixel/non-pixel/firmware context
> > > > banks separately. I’ll leave this to @Vikas to answer.
> > > 
> > > Not exactly as a separate sub-node, but i do like the idea of introducing a
> > > simple iommu property, something like this, which Stephan proposed earlier
> > > in the discussion [1]
> > > 
> > > firmware-iommus = <&apps_smmu ...>;
> > > 
> > > I understand that we are doing the iommu-map thing, but a property
> > > exclusively for firmware like above look much simpler to me.
> > > 
> > 
> > "We know we need to find a generic solution to this very problem, but
> > while we work on that let's add this quick hack to the ABI"?
> 
> I would not call that as hack, rather a simpler solution instead of packing
> everything into the generic iommu-map.
> 
> "firmware-iommus" is much more readable to interpret something running in
> el2 mode, than digging into function ids inside iommu-map and then matching
> it up with specific SIDs to confirm.

If you want it formally, NAK from my side for firmware-iommus. Either
reuse an existing approach (at least it makese sense from the historical
point of view) or introduce a generic approach, which is iommu-maps. The
proposed firmware-iommus is definitely a hack around the IOMMU
properties.

But it's really off-topic here.

> > > Dmitry/ Bryan/ Krzysztof if you are good with this, we can bring back video
> > > in this series. Please share your thoughts on this.
> > > 
> > 
> > Please let's keep these concerns separate, so that we don't hold this
> > series up further. Even if we choose to go by the subnode approach, in
> > the same time frame, it's better to discuss it separately.
> > 
> 
> ACK.
> 
> > Regards,
> > Bjorn
> > 
> > > Regards,
> > > Vikash
> > > 
> > > [1] https://lore.kernel.org/lkml/aKooCFoV3ZYwOMRx@linaro.org/
> > > 
> > > > 
> > > > Also, I do not want the video PIL discussion to be part of this series, as it could
> > > > unnecessarily give the impression that this series depends on it.
> > > > 
> > > > > 
> > > > > That binding got dropped because it was unused in Iris.
> > > > > 
> > > > > https://lore.kernel.org/lkml/05d40a3b-cc13-b704-cac7-0ecbeea0e59d@quicinc.com/
> > > > > 
> > > > > I still fail to see why we are waiting for multi-cell IOMMU to land, when it
> > > > > is expected to and what the VPU enablement story is upstream in the
> > > > > meantime.
> > > > > 
> > > > > Blocked it seems.
> > > > 
> > > > No, it is ongoing, there will be next version coming.
> > > > 
> > > > > 
> > > > > ---
> > > > > bod
> > > > 
> > > 
> 

-- 
With best wishes
Dmitry

Re: [PATCH v8 00/14] Peripheral Image Loader support for Qualcomm SoCs running Linux host at EL2

[Bjorn Andersson]

On Wed, Dec 03, 2025 at 10:48:14AM +0530, Vikash Garodia wrote:
> 
> On 12/3/2025 2:54 AM, Bjorn Andersson wrote:
> > On Tue, Dec 02, 2025 at 03:43:17PM +0530, Vikash Garodia wrote:
> > > 
> > > On 12/2/2025 2:06 PM, Mukesh Ojha wrote:
> > > > On Thu, Nov 27, 2025 at 10:25:23AM +0000, Bryan O'Donoghue wrote:
> > > > > On 21/11/2025 11:37, Mukesh Ojha wrote:
> > > > > > > Sorry.
> > > > > > > 
> > > > > > > Did we actually come up with a cogent reason to omit the video firmware
> > > > > > > loading here ?
> > > > > > > 
> > > > > > > AFAIU it is required for Lemans and Glymur - leaving it out is blocking
> > > > > > > getting video stuff done and storing up trouble.
> > > > > > > 
> > > > > > > What exactly is the blockage - is it something you want help with ?
> > > > > > I replied to you here[1] and given my reason..till something concluded on
> > > > > > "multi-cell IOMMU[2]", I can not add video and block what is working
> > > > > > already.
> > > > > > 
> > > > > > [1]
> > > > > > https://lore.kernel.org/lkml/20251105081421.f6j7ks5bd4dfgr67@hu-mojha-
> > > > > > hyd.qualcomm.com/
> > > > > 
> > > > > Why though ?
> > > > > 
> > > > > You are mixing together the issue of multiple SIDs and the original loading
> > > > > of firmware which could easily reuse the venus method of
> > > > > 
> > > > > &iris {
> > > > > 	video-firmware {
> > > > > 		iommus = <&apss_smmu hex>;
> > > > > 	};
> > > > > };
> > > > 
> > > > I completely understand what you are saying, and it would be very easy
> > > > for me to do that if it gets accepted. However, I doubt that the people
> > > > who raised this concern would agree with the approach.
> > > > 
> > > > I’m not sure if the video team would like to pursue pixel/non-pixel/firmware context
> > > > banks separately. I’ll leave this to @Vikas to answer.
> > > 
> > > Not exactly as a separate sub-node, but i do like the idea of introducing a
> > > simple iommu property, something like this, which Stephan proposed earlier
> > > in the discussion [1]
> > > 
> > > firmware-iommus = <&apps_smmu ...>;
> > > 
> > > I understand that we are doing the iommu-map thing, but a property
> > > exclusively for firmware like above look much simpler to me.
> > > 
> > 
> > "We know we need to find a generic solution to this very problem, but
> > while we work on that let's add this quick hack to the ABI"?
> 
> I would not call that as hack, rather a simpler solution instead of packing
> everything into the generic iommu-map.
> 

The solution might not be a hack, throwing it in there quickly as a
one-off is a hack.

> "firmware-iommus" is much more readable to interpret something running in
> el2 mode, than digging into function ids inside iommu-map and then matching
> it up with specific SIDs to confirm.
> 

Your argument is sensible, I would need to see (or write) the code
you're referring to, in order to be able to form an opinion. But having
two separate ways to express the "same" thing deserves more
consideration.

Looking at how the SMMU configuration will look in the next generation
might even speak in favor of what you're suggesting. Let's sync up after
Plumbers.

Regards,
Bjorn

> > > Dmitry/ Bryan/ Krzysztof if you are good with this, we can bring back video
> > > in this series. Please share your thoughts on this.
> > > 
> > 
> > Please let's keep these concerns separate, so that we don't hold this
> > series up further. Even if we choose to go by the subnode approach, in
> > the same time frame, it's better to discuss it separately.
> > 
> 
> ACK.
> 
> > Regards,
> > Bjorn
> > 
> > > Regards,
> > > Vikash
> > > 
> > > [1] https://lore.kernel.org/lkml/aKooCFoV3ZYwOMRx@linaro.org/
> > > 
> > > > 
> > > > Also, I do not want the video PIL discussion to be part of this series, as it could
> > > > unnecessarily give the impression that this series depends on it.
> > > > 
> > > > > 
> > > > > That binding got dropped because it was unused in Iris.
> > > > > 
> > > > > https://lore.kernel.org/lkml/05d40a3b-cc13-b704-cac7-0ecbeea0e59d@quicinc.com/
> > > > > 
> > > > > I still fail to see why we are waiting for multi-cell IOMMU to land, when it
> > > > > is expected to and what the VPU enablement story is upstream in the
> > > > > meantime.
> > > > > 
> > > > > Blocked it seems.
> > > > 
> > > > No, it is ongoing, there will be next version coming.
> > > > 
> > > > > 
> > > > > ---
> > > > > bod
> > > > 
> > > 
> 

Re: [PATCH v8 11/14] firmware: qcom_scm: Add qcom_scm_pas_get_rsc_table() to get resource table

[Bjorn Andersson]

On Fri, Dec 05, 2025 at 02:15:00PM +0100, Konrad Dybcio wrote:
> On 12/4/25 1:28 PM, Mukesh Ojha wrote:
> > On Wed, Dec 03, 2025 at 01:36:32PM +0100, Konrad Dybcio wrote:
> >> On 11/24/25 4:25 PM, Mukesh Ojha wrote:
> >>> On Mon, Nov 24, 2025 at 12:48:31PM +0100, Konrad Dybcio wrote:
> >>>> On 11/21/25 12:01 PM, Mukesh Ojha wrote:
> >>>>> Qualcomm remote processor may rely on Static and Dynamic resources for
> >>>>> it to be functional. Static resources are fixed like for example,
> >>>>> memory-mapped addresses required by the subsystem and dynamic
> >>>>> resources, such as shared memory in DDR etc., are determined at
> >>>>> runtime during the boot process.
> >>>>>
> >>>>> For most of the Qualcomm SoCs, when run with Gunyah or older QHEE
> >>>>> hypervisor, all the resources whether it is static or dynamic, is
> >>>>> managed by the hypervisor. Dynamic resources if it is present for a
> >>>>> remote processor will always be coming from secure world via SMC call
> >>>>> while static resources may be present in remote processor firmware
> >>>>> binary or it may be coming qcom_scm_pas_get_rsc_table() SMC call along
> >>>>> with dynamic resources.
> 
> [...]
> 
> > Just to avoid iteration, are you suggesting that we can keep this
> > guesswork as part of __qcom_scm_pas_get_rsc_table() and start with
> > something smaller than SZ_16K?
> > 
> > I kind of agree with the first part, but SZ_16K was the recommended size
> > from the firmware for Lemans to start with, in order to pass the SMC
> > successfully on the first try. However, the same size was failing for
> > Glymur, and it required a second attempt with the correct size.
> 
> It depends on the payload, which you're probably much more familiar with.
> If 95% of them will be closer to e.g. 1K in size, it perhaps makes sense
> to use up the additional few dozen cycles on our amazingly fast CPUs and
> retry as necessary, instead of blindly reserving a whole bunch of memory.
> 

Those "few dozen cycles", is tasked with sending messages to RPMh for
voting and unvoting the buses, then tzmem will hopefully hit the
genpool, twice, and then radix updates, and then more genpool updated
and more radix tree work. And then of course there's the one context
switch to secure world.

If we don't have space in the genpool, we're going to grow
dma_alloc_coherent, extend the genpool, call secure world to register
the new tzmem. And then for all those cases where the allocation wasn't
enough, the retry (with updated size) will not fit in the
PAGE_ALIGN(size) genpool that was created, so we'll do this twice.

Fortunately the tzmem growing should only happen on first remoteproc
boot, but I think it's a bit optimistic to say "a few dozen"...


The drawback with making it 16KB is that we're not going to test that
error path very often. But the more idiomatic form of first calling with
a size of 0, then allocate and pass the proper size, seems a bit
wasteful to me as well - in particular if we do it anew each subsystem
boot.

PS. 16KB is 0.03% of the ADSP carveout (or 3% of the ADSP DeviceTree
carveout...).

Regards,
Bjorn

> Konrad

Re: [PATCH v8 11/14] firmware: qcom_scm: Add qcom_scm_pas_get_rsc_table() to get resource table

[Bjorn Andersson]

On Mon, Nov 24, 2025 at 08:55:38PM +0530, Mukesh Ojha wrote:
> On Mon, Nov 24, 2025 at 12:48:31PM +0100, Konrad Dybcio wrote:
> > On 11/21/25 12:01 PM, Mukesh Ojha wrote:
> > > Qualcomm remote processor may rely on Static and Dynamic resources for
> > > it to be functional. Static resources are fixed like for example,
> > > memory-mapped addresses required by the subsystem and dynamic
> > > resources, such as shared memory in DDR etc., are determined at
> > > runtime during the boot process.
> > > 
> > > For most of the Qualcomm SoCs, when run with Gunyah or older QHEE
> > > hypervisor, all the resources whether it is static or dynamic, is
> > > managed by the hypervisor. Dynamic resources if it is present for a
> > > remote processor will always be coming from secure world via SMC call
> > > while static resources may be present in remote processor firmware
> > > binary or it may be coming qcom_scm_pas_get_rsc_table() SMC call along
> > > with dynamic resources.
> > > 
> > > Some of the remote processor drivers, such as video, GPU, IPA, etc., do
> > > not check whether resources are present in their remote processor
> > > firmware binary. In such cases, the caller of this function should set
> > > input_rt and input_rt_size as NULL and zero respectively. Remoteproc
> > > framework has method to check whether firmware binary contain resources
> > > or not and they should be pass resource table pointer to input_rt and
> > > resource table size to input_rt_size and this will be forwarded to
> > > TrustZone for authentication. TrustZone will then append the dynamic
> > > resources and return the complete resource table in output_rt
> > > 
> > > More about documentation on resource table format can be found in
> > > include/linux/remoteproc.h
> > > 
> > > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > > ---
> > 
> > [...]
> > 
> > > +int qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *ctx, void *input_rt,
> > > +			       size_t input_rt_size, void **output_rt,
> > > +			       size_t *output_rt_size)
> > > +{
> > > +	unsigned int retry_num = 5;
> > > +	int ret;
> > > +
> > > +	do {
> > > +		*output_rt = kzalloc(*output_rt_size, GFP_KERNEL);
> > > +		if (!*output_rt)
> > > +			return -ENOMEM;
> > > +
> > > +		ret = __qcom_scm_pas_get_rsc_table(ctx->pas_id, input_rt,
> > > +						   input_rt_size, output_rt,
> > > +						   output_rt_size);
> > > +		if (ret)
> > > +			kfree(*output_rt);
> > > +
> > > +	} while (ret == -EAGAIN && --retry_num);
> > 
> > Will firmware return -EAGAIN as a result, or is this to handle the
> > "buffer too small case"?
> 
> The latter one where a re-attempt could pass..
> 

But why would we need more than 1 retry here? In what cases do we expect
that the first attempt is too small, and then the next 4 attempts are
also going to be too small?

Why is 5 a good number?

Regards,
Bjorn

> > 
> > I think the latter should use a different errno (EOVERFLOW?) and print
> > a message since we decided that it's the caller that suggests a suitable
> > output buffer size
> 
> Agree with error code..
> 
> This is kept on the caller side keeping future in mind. where we can have
> resource table coming from the client and it needs to go to TZ for
> authentication.
> 
> Are you suggesting to move this retry on the caller side ?
> 
> Just for information, once video patches becomes ready, we may bring this
> qcom_mdt_pas_map_devmem_rscs()[1] helper for video or any other client
> should be do this here as well ?
> 
> I wanted to optimize this path, where caller is making a guess and
> gets the updated output size in return.
> 
> [1]
> https://lore.kernel.org/lkml/20250819165447.4149674-9-mukesh.ojha@oss.qualcomm.com/#t
> 
> > 
> > In case it doesn't make sense for the caller to share their expectations,
> > the buffer could be allocated (and perhaps resized if necessary) internally
> > with some validation (i.e. a rsctable likely won't be 5 GiB)
> 
> Are you saying output_size as well should be checked and it should not be
> greater than something like UINT_MAX or something.. ?
> 
> +	*output_rt_size = res.result[2];
> 
> 
> > 
> > Konrad
> 
> -- 
> -Mukesh Ojha

Re: [PATCH v8 11/14] firmware: qcom_scm: Add qcom_scm_pas_get_rsc_table() to get resource table

[Bjorn Andersson]

On Fri, Nov 21, 2025 at 04:31:13PM +0530, Mukesh Ojha wrote:
> Qualcomm remote processor may rely on Static and Dynamic resources for
> it to be functional. Static resources are fixed like for example,
> memory-mapped addresses required by the subsystem and dynamic
> resources, such as shared memory in DDR etc., are determined at
> runtime during the boot process.
> 
> For most of the Qualcomm SoCs, when run with Gunyah or older QHEE
> hypervisor, all the resources whether it is static or dynamic, is
> managed by the hypervisor. Dynamic resources if it is present for a
> remote processor will always be coming from secure world via SMC call
> while static resources may be present in remote processor firmware
> binary or it may be coming qcom_scm_pas_get_rsc_table() SMC call along
> with dynamic resources.
> 
> Some of the remote processor drivers, such as video, GPU, IPA, etc., do
> not check whether resources are present in their remote processor
> firmware binary. In such cases, the caller of this function should set
> input_rt and input_rt_size as NULL and zero respectively. Remoteproc
> framework has method to check whether firmware binary contain resources
> or not and they should be pass resource table pointer to input_rt and
> resource table size to input_rt_size and this will be forwarded to
> TrustZone for authentication. TrustZone will then append the dynamic
> resources and return the complete resource table in output_rt
> 
> More about documentation on resource table format can be found in
> include/linux/remoteproc.h
> 
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
>  drivers/firmware/qcom/qcom_scm.c       | 158 +++++++++++++++++++++++++++++++++
>  drivers/firmware/qcom/qcom_scm.h       |   1 +
>  include/linux/firmware/qcom/qcom_scm.h |   4 +
>  3 files changed, 163 insertions(+)
> 
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 84498d0d2f0c..c4420b79fb57 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -27,6 +27,7 @@
>  #include <linux/of_reserved_mem.h>
>  #include <linux/platform_device.h>
>  #include <linux/reset-controller.h>
> +#include <linux/remoteproc.h>
>  #include <linux/sizes.h>
>  #include <linux/types.h>
>  
> @@ -111,6 +112,10 @@ enum qcom_scm_qseecom_tz_cmd_info {
>  	QSEECOM_TZ_CMD_INFO_VERSION		= 3,
>  };
>  
> +enum qcom_scm_rsctable_resp_type {
> +	RSCTABLE_BUFFER_NOT_SUFFICIENT		= 20,
> +};
> +
>  #define QSEECOM_MAX_APP_NAME_SIZE		64
>  #define SHMBRIDGE_RESULT_NOTSUPP		4
>  
> @@ -766,6 +771,159 @@ int qcom_scm_pas_mem_setup(u32 pas_id, phys_addr_t addr, phys_addr_t size)
>  }
>  EXPORT_SYMBOL_GPL(qcom_scm_pas_mem_setup);
>  
> +static int __qcom_scm_pas_get_rsc_table(u32 pas_id, void *input_rt, size_t input_rt_size,
> +					void **output_rt, size_t *output_rt_size)

output_rt is not going to be modified, only its content, so it can be
void * (single pointer).

> +{
> +	struct qcom_scm_desc desc = {
> +		.svc = QCOM_SCM_SVC_PIL,
> +		.cmd = QCOM_SCM_PIL_PAS_GET_RSCTABLE,
> +		.arginfo = QCOM_SCM_ARGS(5, QCOM_SCM_VAL, QCOM_SCM_RO, QCOM_SCM_VAL,
> +					 QCOM_SCM_RW, QCOM_SCM_VAL),
> +		.args[0] = pas_id,
> +		.owner = ARM_SMCCC_OWNER_SIP,
> +	};
> +	void *input_rt_buf, *output_rt_buf;

I do one prefer one variable per line, preferably in reverse xmas order.

> +	struct resource_table *rsc;

Calling this "empty_rsc" will make its purpose obvious.

> +	struct qcom_scm_res res;
> +	int ret;
> +
> +	ret = qcom_scm_clk_enable();
> +	if (ret)
> +		return ret;
> +
> +	ret = qcom_scm_bw_enable();
> +	if (ret)
> +		goto disable_clk;
> +
> +	/*
> +	 * TrustZone can not accept buffer as NULL value as argument Hence,
> +	 * we need to pass a input buffer indicating that subsystem firmware
> +	 * does not have resource table by filling resource table structure.
> +	 */
> +	if (!input_rt)
> +		input_rt_size = sizeof(*rsc);

If you overwrite input_rt here, you don't need to repeat this
conditional below, like:

	struct resource_table empty_rsc = {};
	
	...
	
	if (!input_rt) {
		input_rt = &empty_rsc;
		input_rt_size = sizeof(rsc);
	}
	
	qcom_tzmem_alloc(input_rt_size)
	
	memcpy(input_rt_buf, input_rt);

> +
> +	input_rt_buf = qcom_tzmem_alloc(__scm->mempool, input_rt_size, GFP_KERNEL);
> +	if (!input_rt_buf) {
> +		ret = -ENOMEM;
> +		goto disable_scm_bw;
> +	}
> +
> +	if (!input_rt) {
> +		rsc = input_rt_buf;
> +		rsc->num = 0;
> +	} else {
> +		memcpy(input_rt_buf, input_rt, input_rt_size);
> +	}
> +

Reading this patch once more, it looks reasonable, but few of the things
in this function actually depend on *output_rt_size, yet we perform them
in the loop below.

We're expecting, with some certainty, that this sequence will be called
more than once, so I think it would be preferable to slice this
differently, and only repeat the <loop></loop> part.

<loop>

> +	output_rt_buf = qcom_tzmem_alloc(__scm->mempool, *output_rt_size, GFP_KERNEL);
> +	if (!output_rt_buf) {
> +		ret = -ENOMEM;
> +		goto free_input_rt_buf;
> +	}
> +
> +	desc.args[1] = qcom_tzmem_to_phys(input_rt_buf);
> +	desc.args[2] = input_rt_size;
> +	desc.args[3] = qcom_tzmem_to_phys(output_rt_buf);
> +	desc.args[4] = *output_rt_size;
> +
> +	/*
> +	 * Whether SMC fail or pass, res.result[2] will hold actual resource table
> +	 * size.
> +	 *
> +	 * if passed 'output_rt_size' buffer size is not sufficient to hold the
> +	 * resource table TrustZone sends, response code in res.result[1] as
> +	 * RSCTABLE_BUFFER_NOT_SUFFICIENT so that caller can retry this SMC call with
> +	 * output_rt buffer with res.result[2] size.
> +	 */
> +	ret = qcom_scm_call(__scm->dev, &desc, &res);
> +	*output_rt_size = res.result[2];
> +	if (!ret)
> +		memcpy(*output_rt, output_rt_buf, *output_rt_size);
> +
> +	if (ret && res.result[1] == RSCTABLE_BUFFER_NOT_SUFFICIENT)
> +		ret = -EAGAIN;

</loop>

> +
> +	qcom_tzmem_free(output_rt_buf);
> +
> +free_input_rt_buf:
> +	qcom_tzmem_free(input_rt_buf);
> +
> +disable_scm_bw:
> +	qcom_scm_bw_disable();
> +
> +disable_clk:
> +	qcom_scm_clk_disable();
> +
> +	return ret ? : res.result[0];

Is there a risk that res.result[0] will carry something meaningful to
the caller (which will be misinterpreted)?

> +}
> +
> +/**
> + * qcom_scm_pas_get_rsc_table() - Retrieve the resource table in passed output buffer
> + *				  for a given peripheral.
> + *
> + * Qualcomm remote processor may rely on both static and dynamic resources for
> + * its functionality. Static resources typically refer to memory-mapped addresses
> + * required by the subsystem and are often embedded within the firmware binary
> + * and dynamic resources, such as shared memory in DDR etc., are determined at
> + * runtime during the boot process.
> + *
> + * On Qualcomm Technologies devices, it's possible that static resources are not
> + * embedded in the firmware binary and instead are provided by TrustZone However,
> + * dynamic resources are always expected to come from TrustZone. This indicates
> + * that for Qualcomm devices, all resources (static and dynamic) will be provided
> + * by TrustZone via the SMC call.
> + *
> + * If the remote processor firmware binary does contain static resources, they
> + * should be passed in input_rt. These will be forwarded to TrustZone for
> + * authentication. TrustZone will then append the dynamic resources and return
> + * the complete resource table in output_rt.
> + *
> + * If the remote processor firmware binary does not include a resource table,
> + * the caller of this function should set input_rt as NULL and input_rt_size
> + * as zero respectively.
> + *
> + * More about documentation on resource table data structures can be found in
> + * include/linux/remoteproc.h
> + *
> + * @ctx:	    PAS context
> + * @pas_id:	    peripheral authentication service id
> + * @input_rt:       resource table buffer which is present in firmware binary
> + * @input_rt_size:  size of the resource table present in firmware binary
> + * @output_rt:	    buffer to which the both static and dynamic resources will
> + *		    be returned.
> + * @output_rt_size: TrustZone expects caller should pass worst case size for
> + *		    the output_rt.
> + *
> + * Return: 0 on success and nonzero on failure.
> + *
> + * Upon successful return, output_rt will have the resource table and output_rt_size
> + * will have actual resource table size,
> + */
> +int qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *ctx, void *input_rt,
> +			       size_t input_rt_size, void **output_rt,
> +			       size_t *output_rt_size)
> +{
> +	unsigned int retry_num = 5;
> +	int ret;
> +
> +	do {
> +		*output_rt = kzalloc(*output_rt_size, GFP_KERNEL);

I'd prefer the output buffer and size to be carried in a local variables
until we determine success, to avoid overwriting the caller's size with
some bogus number and return a pointer to freed memory.

Wouldn't be unreasonable to return an ERR_PTR() with the allocated
buffer, instead of returning through the reference.

Regards,
Bjorn

> +		if (!*output_rt)
> +			return -ENOMEM;
> +
> +		ret = __qcom_scm_pas_get_rsc_table(ctx->pas_id, input_rt,
> +						   input_rt_size, output_rt,
> +						   output_rt_size);
> +		if (ret)
> +			kfree(*output_rt);
> +
> +	} while (ret == -EAGAIN && --retry_num);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(qcom_scm_pas_get_rsc_table);
> +
>  /**
>   * qcom_scm_pas_auth_and_reset() - Authenticate the given peripheral firmware
>   *				   and reset the remote processor
> diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h
> index a56c8212cc0c..50d87c628d78 100644
> --- a/drivers/firmware/qcom/qcom_scm.h
> +++ b/drivers/firmware/qcom/qcom_scm.h
> @@ -105,6 +105,7 @@ int qcom_scm_shm_bridge_enable(struct device *scm_dev);
>  #define QCOM_SCM_PIL_PAS_SHUTDOWN	0x06
>  #define QCOM_SCM_PIL_PAS_IS_SUPPORTED	0x07
>  #define QCOM_SCM_PIL_PAS_MSS_RESET	0x0a
> +#define QCOM_SCM_PIL_PAS_GET_RSCTABLE	0x21
>  
>  #define QCOM_SCM_SVC_IO			0x05
>  #define QCOM_SCM_IO_READ		0x01
> diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
> index ccb8b2e42237..814d40d64ab0 100644
> --- a/include/linux/firmware/qcom/qcom_scm.h
> +++ b/include/linux/firmware/qcom/qcom_scm.h
> @@ -88,6 +88,10 @@ 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);
>  bool qcom_scm_pas_supported(u32 pas_id);
> +int qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *ctx, void *input_rt,
> +			       size_t input_rt_size, void **output_rt,
> +			       size_t *output_rt_size);
> +
>  int qcom_scm_pas_prepare_and_auth_reset(struct qcom_scm_pas_context *ctx);
>  
>  int qcom_scm_io_readl(phys_addr_t addr, unsigned int *val);
> 
> -- 
> 2.50.1
> 

Re: [PATCH v8 04/14] firmware: qcom_scm: Introduce PAS context initialization helper function

[Bjorn Andersson]

On Fri, Nov 21, 2025 at 04:31:06PM +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 initialization helper function.
> 
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
>  drivers/firmware/qcom/qcom_scm.c       | 34 ++++++++++++++++++++++++++++++++++
>  include/linux/firmware/qcom/qcom_scm.h | 14 ++++++++++++++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 0a0c48fca7cf..e9e9d03ea36a 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -558,6 +558,40 @@ static void qcom_scm_set_download_mode(u32 dload_mode)
>  		dev_err(__scm->dev, "failed to set download mode: %d\n", ret);
>  }
>  
> +/**
> + * devm_qcom_scm_pas_context_init() - Initialize peripheral authentication service
> + *				      context for a given peripheral

It doesn't just initialize, it allocates it. So I think it would be
better to name this function devm_qcom_scm_pas_context_alloc().

> + *
> + * PAS context is device-resource managed, so the caller does not need
> + * to worry about freeing the context memory.
> + *
> + * @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.

Returns: The new PAS context, or ERR_PTR() on failure.

Regards,
Bjorn

> + */
> +struct qcom_scm_pas_context *devm_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 = devm_kzalloc(dev, 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;
> +
> +	return ctx;
> +}
> +EXPORT_SYMBOL_GPL(devm_qcom_scm_pas_context_init);
> +
>  /**
>   * 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..4a57fa676513 100644
> --- a/include/linux/firmware/qcom/qcom_scm.h
> +++ b/include/linux/firmware/qcom/qcom_scm.h
> @@ -72,6 +72,20 @@ 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;
> +	void *ptr;
> +	dma_addr_t phys;
> +	ssize_t size;
> +};
> +
> +struct qcom_scm_pas_context *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);
> 
> -- 
> 2.50.1
> 

Re: [PATCH v8 08/14] firmware: qcom_scm: Add a prep version of auth_and_reset function

[Bjorn Andersson]

On Fri, Nov 21, 2025 at 04:31:10PM +0530, Mukesh Ojha wrote:
> For memory passed to TrustZone (TZ), it must either be part of a pool
> registered with TZ or explicitly registered via SHMbridge SMC calls.
> When Gunyah hypervisor is present, PAS SMC calls from Linux running at
> EL1 are trapped by Gunyah running @ EL2, which handles SHMbridge
> creation for both metadata and remoteproc carveout memory before
> invoking the calls to TZ.
> 
> On SoCs running with a non-Gunyah-based hypervisor, Linux must take
> responsibility for creating the SHM bridge before invoking PAS SMC
> calls. For the auth_and_reset() call, the remoteproc carveout memory
> must first be registered with TZ via a SHMbridge SMC call and once
> authentication and reset are complete, the SHMbridge memory can be
> deregistered.
> 
> Introduce qcom_scm_pas_prepare_and_auth_reset(), which sets up the SHM
> bridge over the remoteproc carveout memory when Linux operates at EL2.
> This behavior is indicated by a new field added to the PAS context data
> structure. The function then invokes the auth_and_reset SMC call.
> 
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
>  drivers/firmware/qcom/qcom_scm.c       | 48 ++++++++++++++++++++++++++++++++++
>  include/linux/firmware/qcom/qcom_scm.h |  2 ++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 5fa974683ee0..fdb736d839db 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -765,6 +765,54 @@ int qcom_scm_pas_auth_and_reset(u32 pas_id)
>  }
>  EXPORT_SYMBOL_GPL(qcom_scm_pas_auth_and_reset);
>  
> +/**
> + * qcom_scm_pas_prepare_and_auth_reset() - Prepare, authenticate, and reset the
> + *					   remote processor
> + *
> + * @ctx:	Context saved during call to qcom_scm_pas_context_init()
> + *
> + * This function performs the necessary steps to prepare a PAS subsystem,
> + * authenticate it using the provided metadata, and initiate a reset sequence.
> + *
> + * It should be used when Linux is in control setting up the IOMMU hardware
> + * for remote subsystem during secure firmware loading processes. The preparation
> + * step sets up a shmbridge over the firmware memory before TrustZone accesses the
> + * firmware memory region for authentication. The authentication step verifies
> + * the integrity and authenticity of the firmware or configuration using secure
> + * metadata. Finally, the reset step ensures the subsystem starts in a clean and
> + * sane state.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int qcom_scm_pas_prepare_and_auth_reset(struct qcom_scm_pas_context *ctx)
> +{
> +	u64 handle;
> +	int ret;
> +
> +	if (!ctx->has_iommu)
> +		return qcom_scm_pas_auth_and_reset(ctx->pas_id);
> +
> +	/*
> +	 * When Linux running @ EL1, Gunyah hypervisor running @ EL2 traps the
> +	 * auth_and_reset call and create an shmbridge on the remote subsystem
> +	 * memory region and then invokes a call to TrustZone to authenticate.

The above three lines explain why we take the path above, and the next
two lines explains why we didn't.

So I think it would be better to move the above 3 lines above the
!has_iommu, and just keep the two here.

> +	 * When Linux runs @ EL2 Linux must create the shmbridge itself and then
> +	 * subsequently call TrustZone for authenticate and reset.
> +	 */
> +	ret = qcom_tzmem_shm_bridge_create(ctx->mem_phys, ctx->mem_size, &handle);
> +	if (ret) {
> +		dev_err(__scm->dev, "Failed to create shmbridge for PAS ID (%u): %d\n",
> +			ctx->pas_id, ret);

We already print an error in qcom_tzmem_shm_bridge_create() and we print
another one in qcom_pas_start(), so I think we can omit this one.

Regards,
Bjorn

> +		return ret;
> +	}
> +
> +	ret = qcom_scm_pas_auth_and_reset(ctx->pas_id);
> +	qcom_tzmem_shm_bridge_delete(handle);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(qcom_scm_pas_prepare_and_auth_reset);
> +
>  /**
>   * qcom_scm_pas_shutdown() - Shut down the remote processor
>   * @pas_id:	peripheral authentication service id
> diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
> index b10b1aeb32c6..ccb8b2e42237 100644
> --- a/include/linux/firmware/qcom/qcom_scm.h
> +++ b/include/linux/firmware/qcom/qcom_scm.h
> @@ -74,6 +74,7 @@ struct qcom_scm_pas_context {
>  	void *ptr;
>  	dma_addr_t phys;
>  	ssize_t size;
> +	bool has_iommu;
>  };
>  
>  struct qcom_scm_pas_context *devm_qcom_scm_pas_context_init(struct device *dev,
> @@ -87,6 +88,7 @@ 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);
>  bool qcom_scm_pas_supported(u32 pas_id);
> +int qcom_scm_pas_prepare_and_auth_reset(struct qcom_scm_pas_context *ctx);
>  
>  int qcom_scm_io_readl(phys_addr_t addr, unsigned int *val);
>  int qcom_scm_io_writel(phys_addr_t addr, unsigned int val);
> 
> -- 
> 2.50.1
> 

Re: [PATCH v8 14/14] arm64: dts: qcom: Add EL2 overlay for Lemans

[Bjorn Andersson]

On Fri, Nov 21, 2025 at 04:31:16PM +0530, Mukesh Ojha wrote:
> All the Lemans IOT variants boards are using Gunyah hypervisor which
> means that, so far, Linux-based OS could only boot in EL1 on those
> devices.  However, it is possible for us to boot Linux at EL2 on these
> devices [1].
> 
> When running under Gunyah, remote processor firmware IOMMU streams is
> controlled by the Gunyah however when Linux take ownership of it in EL2,
> It need to configure it properly to use remote processor.
> 
> Add a EL2-specific DT overlay and apply it to Lemans IOT variant
> devices to create -el2.dtb for each of them alongside "normal" dtb.
> 
> [1]
> https://docs.qualcomm.com/bundle/publicresource/topics/80-70020-4/boot-developer-touchpoints.html#uefi
> 
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/Makefile        | 10 ++++++++
>  arch/arm64/boot/dts/qcom/lemans-el2.dtso | 41 ++++++++++++++++++++++++++++++++
>  2 files changed, 51 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 6f34d5ed331c..56efd90b7a5e 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -37,6 +37,10 @@ lemans-evk-camera-dtbs	:= lemans-evk.dtb lemans-evk-camera.dtbo
>  
>  dtb-$(CONFIG_ARCH_QCOM)	+= lemans-evk-camera-csi1-imx577.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= lemans-evk-camera.dtb
> +
> +lemans-evk-el2-dtbs := lemans-evk.dtb lemans-el2.dtbo
> +
> +dtb-$(CONFIG_ARCH_QCOM)	+= lemans-evk-el2.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= monaco-evk.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8216-samsung-fortuna3g.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-acer-a1-724.dtb
> @@ -142,6 +146,12 @@ dtb-$(CONFIG_ARCH_QCOM)	+= qcs8300-ride.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= qcs8550-aim300-aiot.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride-r3.dtb
> +
> +qcs9100-ride-el2-dtbs := qcs9100-ride.dtb lemans-el2.dtbo
> +qcs9100-ride-r3-el2-dtbs := qcs9100-ride-r3.dtb lemans-el2.dtbo
> +
> +dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride-el2.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride-r3-el2.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= qdu1000-idp.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= qrb2210-rb1.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= qrb4210-rb2.dtb
> diff --git a/arch/arm64/boot/dts/qcom/lemans-el2.dtso b/arch/arm64/boot/dts/qcom/lemans-el2.dtso
> new file mode 100644
> index 000000000000..af35039946e3
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/lemans-el2.dtso
> @@ -0,0 +1,41 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +/*
> + * Lemans specific modifications required to boot in EL2.
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +&iris {
> +	/* More driver work is needed */

You can write this comment without referring to some particular
implementation (as DeviceTree should).

/* The binding doesn't allow for describing the firmware IOMMU stream yet */

> +	status = "disabled";
> +};
> +
> +/*
> + * When running under Gunyah, remote processor firmware IOMMU streams is
> + * controlled by the Gunyah however when we take ownership of it in EL2,
> + * we need to configure it properly to use remote processor.

The comment describes how things work with Gunyah, and then from that
angle explains why we need this. I'd find it preferable to keep the
perspective of not having Gunyah. Something as simple as:

/*
 * Without Gunyah, the IOMMU is managed by the consumer of this DeviceTree,
 * so describe the firmware streams for each remoteproc.
 */

That said, once the iris binding (and others) allow us to describe the
firmware stream, this comment will be misplaced. So perhaps apply my
feedback to the commit message and omit the comment here. If someone
wonders why it looks like it does, the commit message in the git history
will tell them.

Regards,
Bjorn

> + */
> +&remoteproc_adsp {
> +	iommus = <&apps_smmu 0x3000 0x0>;
> +};
> +
> +&remoteproc_cdsp0 {
> +	iommus = <&apps_smmu 0x21c0 0x0400>;
> +};
> +
> +&remoteproc_cdsp1 {
> +	iommus = <&apps_smmu 0x29c0 0x0400>;
> +};
> +
> +&remoteproc_gpdsp0 {
> +       iommus = <&apps_smmu 0x38a0 0x0>;
> +};
> +
> +&remoteproc_gpdsp1 {
> +       iommus = <&apps_smmu 0x38c0 0x0>;
> +};
> 
> -- 
> 2.50.1
> 

Re: [PATCH v8 14/14] arm64: dts: qcom: Add EL2 overlay for Lemans

[Mukesh Ojha]

On Fri, Dec 05, 2025 at 05:00:20PM -0600, Bjorn Andersson wrote:
> On Fri, Nov 21, 2025 at 04:31:16PM +0530, Mukesh Ojha wrote:
> > All the Lemans IOT variants boards are using Gunyah hypervisor which
> > means that, so far, Linux-based OS could only boot in EL1 on those
> > devices.  However, it is possible for us to boot Linux at EL2 on these
> > devices [1].
> > 
> > When running under Gunyah, remote processor firmware IOMMU streams is
> > controlled by the Gunyah however when Linux take ownership of it in EL2,
> > It need to configure it properly to use remote processor.
> > 
> > Add a EL2-specific DT overlay and apply it to Lemans IOT variant
> > devices to create -el2.dtb for each of them alongside "normal" dtb.
> > 
> > [1]
> > https://docs.qualcomm.com/bundle/publicresource/topics/80-70020-4/boot-developer-touchpoints.html#uefi
> > 
> > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > ---
> >  arch/arm64/boot/dts/qcom/Makefile        | 10 ++++++++
> >  arch/arm64/boot/dts/qcom/lemans-el2.dtso | 41 ++++++++++++++++++++++++++++++++
> >  2 files changed, 51 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> > index 6f34d5ed331c..56efd90b7a5e 100644
> > --- a/arch/arm64/boot/dts/qcom/Makefile
> > +++ b/arch/arm64/boot/dts/qcom/Makefile
> > @@ -37,6 +37,10 @@ lemans-evk-camera-dtbs	:= lemans-evk.dtb lemans-evk-camera.dtbo
> >  
> >  dtb-$(CONFIG_ARCH_QCOM)	+= lemans-evk-camera-csi1-imx577.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= lemans-evk-camera.dtb
> > +
> > +lemans-evk-el2-dtbs := lemans-evk.dtb lemans-el2.dtbo
> > +
> > +dtb-$(CONFIG_ARCH_QCOM)	+= lemans-evk-el2.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= monaco-evk.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= msm8216-samsung-fortuna3g.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-acer-a1-724.dtb
> > @@ -142,6 +146,12 @@ dtb-$(CONFIG_ARCH_QCOM)	+= qcs8300-ride.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= qcs8550-aim300-aiot.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride-r3.dtb
> > +
> > +qcs9100-ride-el2-dtbs := qcs9100-ride.dtb lemans-el2.dtbo
> > +qcs9100-ride-r3-el2-dtbs := qcs9100-ride-r3.dtb lemans-el2.dtbo
> > +
> > +dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride-el2.dtb
> > +dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride-r3-el2.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= qdu1000-idp.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= qrb2210-rb1.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= qrb4210-rb2.dtb
> > diff --git a/arch/arm64/boot/dts/qcom/lemans-el2.dtso b/arch/arm64/boot/dts/qcom/lemans-el2.dtso
> > new file mode 100644
> > index 000000000000..af35039946e3
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/lemans-el2.dtso
> > @@ -0,0 +1,41 @@
> > +// SPDX-License-Identifier: BSD-3-Clause
> > +/*
> > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > + */
> > +
> > +/*
> > + * Lemans specific modifications required to boot in EL2.
> > + */
> > +
> > +/dts-v1/;
> > +/plugin/;
> > +
> > +&iris {
> > +	/* More driver work is needed */
> 
> You can write this comment without referring to some particular
> implementation (as DeviceTree should).
> 
> /* The binding doesn't allow for describing the firmware IOMMU stream yet */
> 
> > +	status = "disabled";
> > +};
> > +
> > +/*
> > + * When running under Gunyah, remote processor firmware IOMMU streams is
> > + * controlled by the Gunyah however when we take ownership of it in EL2,
> > + * we need to configure it properly to use remote processor.
> 
> The comment describes how things work with Gunyah, and then from that
> angle explains why we need this. I'd find it preferable to keep the
> perspective of not having Gunyah. Something as simple as:
> 
> /*
>  * Without Gunyah, the IOMMU is managed by the consumer of this DeviceTree,
>  * so describe the firmware streams for each remoteproc.
>  */
> 
> That said, once the iris binding (and others) allow us to describe the
> firmware stream, this comment will be misplaced. So perhaps apply my
> feedback to the commit message and omit the comment here. If someone
> wonders why it looks like it does, the commit message in the git history
> will tell them.

Will move all the required comments into description of commit message.

-- 
-Mukesh Ojha

Re: [PATCH v8 11/14] firmware: qcom_scm: Add qcom_scm_pas_get_rsc_table() to get resource table

[Mukesh Ojha]

On Fri, Dec 05, 2025 at 04:21:07PM -0600, Bjorn Andersson wrote:
> On Mon, Nov 24, 2025 at 08:55:38PM +0530, Mukesh Ojha wrote:
> > On Mon, Nov 24, 2025 at 12:48:31PM +0100, Konrad Dybcio wrote:
> > > On 11/21/25 12:01 PM, Mukesh Ojha wrote:
> > > > Qualcomm remote processor may rely on Static and Dynamic resources for
> > > > it to be functional. Static resources are fixed like for example,
> > > > memory-mapped addresses required by the subsystem and dynamic
> > > > resources, such as shared memory in DDR etc., are determined at
> > > > runtime during the boot process.
> > > > 
> > > > For most of the Qualcomm SoCs, when run with Gunyah or older QHEE
> > > > hypervisor, all the resources whether it is static or dynamic, is
> > > > managed by the hypervisor. Dynamic resources if it is present for a
> > > > remote processor will always be coming from secure world via SMC call
> > > > while static resources may be present in remote processor firmware
> > > > binary or it may be coming qcom_scm_pas_get_rsc_table() SMC call along
> > > > with dynamic resources.
> > > > 
> > > > Some of the remote processor drivers, such as video, GPU, IPA, etc., do
> > > > not check whether resources are present in their remote processor
> > > > firmware binary. In such cases, the caller of this function should set
> > > > input_rt and input_rt_size as NULL and zero respectively. Remoteproc
> > > > framework has method to check whether firmware binary contain resources
> > > > or not and they should be pass resource table pointer to input_rt and
> > > > resource table size to input_rt_size and this will be forwarded to
> > > > TrustZone for authentication. TrustZone will then append the dynamic
> > > > resources and return the complete resource table in output_rt
> > > > 
> > > > More about documentation on resource table format can be found in
> > > > include/linux/remoteproc.h
> > > > 
> > > > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > > > ---
> > > 
> > > [...]
> > > 
> > > > +int qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *ctx, void *input_rt,
> > > > +			       size_t input_rt_size, void **output_rt,
> > > > +			       size_t *output_rt_size)
> > > > +{
> > > > +	unsigned int retry_num = 5;
> > > > +	int ret;
> > > > +
> > > > +	do {
> > > > +		*output_rt = kzalloc(*output_rt_size, GFP_KERNEL);
> > > > +		if (!*output_rt)
> > > > +			return -ENOMEM;
> > > > +
> > > > +		ret = __qcom_scm_pas_get_rsc_table(ctx->pas_id, input_rt,
> > > > +						   input_rt_size, output_rt,
> > > > +						   output_rt_size);
> > > > +		if (ret)
> > > > +			kfree(*output_rt);
> > > > +
> > > > +	} while (ret == -EAGAIN && --retry_num);
> > > 
> > > Will firmware return -EAGAIN as a result, or is this to handle the
> > > "buffer too small case"?
> > 
> > The latter one where a re-attempt could pass..
> > 
> 
> But why would we need more than 1 retry here? In what cases do we expect
> that the first attempt is too small, and then the next 4 attempts are
> also going to be too small?
> 
> Why is 5 a good number?

This was a misunderstanding, will fix it as per latest discussion.

-- 
-Mukesh Ojha

Re: [PATCH v8 11/14] firmware: qcom_scm: Add qcom_scm_pas_get_rsc_table() to get resource table

[Mukesh Ojha]

On Fri, Dec 05, 2025 at 04:40:51PM -0600, Bjorn Andersson wrote:
> On Fri, Nov 21, 2025 at 04:31:13PM +0530, Mukesh Ojha wrote:
> > Qualcomm remote processor may rely on Static and Dynamic resources for
> > it to be functional. Static resources are fixed like for example,
> > memory-mapped addresses required by the subsystem and dynamic
> > resources, such as shared memory in DDR etc., are determined at
> > runtime during the boot process.
> > 
> > For most of the Qualcomm SoCs, when run with Gunyah or older QHEE
> > hypervisor, all the resources whether it is static or dynamic, is
> > managed by the hypervisor. Dynamic resources if it is present for a
> > remote processor will always be coming from secure world via SMC call
> > while static resources may be present in remote processor firmware
> > binary or it may be coming qcom_scm_pas_get_rsc_table() SMC call along
> > with dynamic resources.
> > 
> > Some of the remote processor drivers, such as video, GPU, IPA, etc., do
> > not check whether resources are present in their remote processor
> > firmware binary. In such cases, the caller of this function should set
> > input_rt and input_rt_size as NULL and zero respectively. Remoteproc
> > framework has method to check whether firmware binary contain resources
> > or not and they should be pass resource table pointer to input_rt and
> > resource table size to input_rt_size and this will be forwarded to
> > TrustZone for authentication. TrustZone will then append the dynamic
> > resources and return the complete resource table in output_rt
> > 
> > More about documentation on resource table format can be found in
> > include/linux/remoteproc.h
> > 
> > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > ---
> >  drivers/firmware/qcom/qcom_scm.c       | 158 +++++++++++++++++++++++++++++++++
> >  drivers/firmware/qcom/qcom_scm.h       |   1 +
> >  include/linux/firmware/qcom/qcom_scm.h |   4 +
> >  3 files changed, 163 insertions(+)
> > 
> > diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> > index 84498d0d2f0c..c4420b79fb57 100644
> > --- a/drivers/firmware/qcom/qcom_scm.c
> > +++ b/drivers/firmware/qcom/qcom_scm.c
> > @@ -27,6 +27,7 @@
> >  #include <linux/of_reserved_mem.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/reset-controller.h>
> > +#include <linux/remoteproc.h>
> >  #include <linux/sizes.h>
> >  #include <linux/types.h>
> >  
> > @@ -111,6 +112,10 @@ enum qcom_scm_qseecom_tz_cmd_info {
> >  	QSEECOM_TZ_CMD_INFO_VERSION		= 3,
> >  };
> >  
> > +enum qcom_scm_rsctable_resp_type {
> > +	RSCTABLE_BUFFER_NOT_SUFFICIENT		= 20,
> > +};
> > +
> >  #define QSEECOM_MAX_APP_NAME_SIZE		64
> >  #define SHMBRIDGE_RESULT_NOTSUPP		4
> >  
> > @@ -766,6 +771,159 @@ int qcom_scm_pas_mem_setup(u32 pas_id, phys_addr_t addr, phys_addr_t size)
> >  }
> >  EXPORT_SYMBOL_GPL(qcom_scm_pas_mem_setup);
> >  
> > +static int __qcom_scm_pas_get_rsc_table(u32 pas_id, void *input_rt, size_t input_rt_size,
> > +					void **output_rt, size_t *output_rt_size)
> 
> output_rt is not going to be modified, only its content, so it can be
> void * (single pointer).
> 
> > +{
> > +	struct qcom_scm_desc desc = {
> > +		.svc = QCOM_SCM_SVC_PIL,
> > +		.cmd = QCOM_SCM_PIL_PAS_GET_RSCTABLE,
> > +		.arginfo = QCOM_SCM_ARGS(5, QCOM_SCM_VAL, QCOM_SCM_RO, QCOM_SCM_VAL,
> > +					 QCOM_SCM_RW, QCOM_SCM_VAL),
> > +		.args[0] = pas_id,
> > +		.owner = ARM_SMCCC_OWNER_SIP,
> > +	};
> > +	void *input_rt_buf, *output_rt_buf;
> 
> I do one prefer one variable per line, preferably in reverse xmas order.

Ack.

> 
> > +	struct resource_table *rsc;
> 
> Calling this "empty_rsc" will make its purpose obvious.

Ack.

> 
> > +	struct qcom_scm_res res;
> > +	int ret;
> > +
> > +	ret = qcom_scm_clk_enable();
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = qcom_scm_bw_enable();
> > +	if (ret)
> > +		goto disable_clk;
> > +
> > +	/*
> > +	 * TrustZone can not accept buffer as NULL value as argument Hence,
> > +	 * we need to pass a input buffer indicating that subsystem firmware
> > +	 * does not have resource table by filling resource table structure.
> > +	 */
> > +	if (!input_rt)
> > +		input_rt_size = sizeof(*rsc);
> 
> If you overwrite input_rt here, you don't need to repeat this
> conditional below, like:
> 
> 	struct resource_table empty_rsc = {};
> 	
> 	...
> 	
> 	if (!input_rt) {
> 		input_rt = &empty_rsc;
> 		input_rt_size = sizeof(rsc);
> 	}
> 	
> 	qcom_tzmem_alloc(input_rt_size)
> 	
> 	memcpy(input_rt_buf, input_rt);

Ack.

> 
> > +
> > +	input_rt_buf = qcom_tzmem_alloc(__scm->mempool, input_rt_size, GFP_KERNEL);
> > +	if (!input_rt_buf) {
> > +		ret = -ENOMEM;
> > +		goto disable_scm_bw;
> > +	}
> > +
> > +	if (!input_rt) {
> > +		rsc = input_rt_buf;
> > +		rsc->num = 0;
> > +	} else {
> > +		memcpy(input_rt_buf, input_rt, input_rt_size);
> > +	}
> > +
> 
> Reading this patch once more, it looks reasonable, but few of the things
> in this function actually depend on *output_rt_size, yet we perform them
> in the loop below.
> 
> We're expecting, with some certainty, that this sequence will be called
> more than once, so I think it would be preferable to slice this
> differently, and only repeat the <loop></loop> part.
> 
> <loop>

Ack, I will move all the clock, bw voting and mentioned retry(on -EOVERFLOW) loop
to the caller of this function.

> 
> > +	output_rt_buf = qcom_tzmem_alloc(__scm->mempool, *output_rt_size, GFP_KERNEL);
> > +	if (!output_rt_buf) {
> > +		ret = -ENOMEM;
> > +		goto free_input_rt_buf;
> > +	}
> > +
> > +	desc.args[1] = qcom_tzmem_to_phys(input_rt_buf);
> > +	desc.args[2] = input_rt_size;
> > +	desc.args[3] = qcom_tzmem_to_phys(output_rt_buf);
> > +	desc.args[4] = *output_rt_size;
> > +
> > +	/*
> > +	 * Whether SMC fail or pass, res.result[2] will hold actual resource table
> > +	 * size.
> > +	 *
> > +	 * if passed 'output_rt_size' buffer size is not sufficient to hold the
> > +	 * resource table TrustZone sends, response code in res.result[1] as
> > +	 * RSCTABLE_BUFFER_NOT_SUFFICIENT so that caller can retry this SMC call with
> > +	 * output_rt buffer with res.result[2] size.
> > +	 */
> > +	ret = qcom_scm_call(__scm->dev, &desc, &res);
> > +	*output_rt_size = res.result[2];
> > +	if (!ret)
> > +		memcpy(*output_rt, output_rt_buf, *output_rt_size);
> > +
> > +	if (ret && res.result[1] == RSCTABLE_BUFFER_NOT_SUFFICIENT)
> > +		ret = -EAGAIN;
> 
> </loop>
> 
> > +
> > +	qcom_tzmem_free(output_rt_buf);
> > +
> > +free_input_rt_buf:
> > +	qcom_tzmem_free(input_rt_buf);
> > +
> > +disable_scm_bw:
> > +	qcom_scm_bw_disable();
> > +
> > +disable_clk:
> > +	qcom_scm_clk_disable();
> > +
> > +	return ret ? : res.result[0];
> 
> Is there a risk that res.result[0] will carry something meaningful to
> the caller (which will be misinterpreted)?

No, its just to align with other SMC call, it will always have 0 value
on success.

> 
> > +}
> > +
> > +/**
> > + * qcom_scm_pas_get_rsc_table() - Retrieve the resource table in passed output buffer
> > + *				  for a given peripheral.
> > + *
> > + * Qualcomm remote processor may rely on both static and dynamic resources for
> > + * its functionality. Static resources typically refer to memory-mapped addresses
> > + * required by the subsystem and are often embedded within the firmware binary
> > + * and dynamic resources, such as shared memory in DDR etc., are determined at
> > + * runtime during the boot process.
> > + *
> > + * On Qualcomm Technologies devices, it's possible that static resources are not
> > + * embedded in the firmware binary and instead are provided by TrustZone However,
> > + * dynamic resources are always expected to come from TrustZone. This indicates
> > + * that for Qualcomm devices, all resources (static and dynamic) will be provided
> > + * by TrustZone via the SMC call.
> > + *
> > + * If the remote processor firmware binary does contain static resources, they
> > + * should be passed in input_rt. These will be forwarded to TrustZone for
> > + * authentication. TrustZone will then append the dynamic resources and return
> > + * the complete resource table in output_rt.
> > + *
> > + * If the remote processor firmware binary does not include a resource table,
> > + * the caller of this function should set input_rt as NULL and input_rt_size
> > + * as zero respectively.
> > + *
> > + * More about documentation on resource table data structures can be found in
> > + * include/linux/remoteproc.h
> > + *
> > + * @ctx:	    PAS context
> > + * @pas_id:	    peripheral authentication service id
> > + * @input_rt:       resource table buffer which is present in firmware binary
> > + * @input_rt_size:  size of the resource table present in firmware binary
> > + * @output_rt:	    buffer to which the both static and dynamic resources will
> > + *		    be returned.
> > + * @output_rt_size: TrustZone expects caller should pass worst case size for
> > + *		    the output_rt.
> > + *
> > + * Return: 0 on success and nonzero on failure.
> > + *
> > + * Upon successful return, output_rt will have the resource table and output_rt_size
> > + * will have actual resource table size,
> > + */
> > +int qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *ctx, void *input_rt,
> > +			       size_t input_rt_size, void **output_rt,
> > +			       size_t *output_rt_size)
> > +{
> > +	unsigned int retry_num = 5;
> > +	int ret;
> > +
> > +	do {
> > +		*output_rt = kzalloc(*output_rt_size, GFP_KERNEL);
> 
> I'd prefer the output buffer and size to be carried in a local variables
> until we determine success, to avoid overwriting the caller's size with
> some bogus number and return a pointer to freed memory.

Sure.

> 
> Wouldn't be unreasonable to return an ERR_PTR() with the allocated
> buffer, instead of returning through the reference.

We anyway have to return the size through reference, why not do the same for 
allocated buffer as well..


-- 
-Mukesh Ojha

Re: [PATCH v8 11/14] firmware: qcom_scm: Add qcom_scm_pas_get_rsc_table() to get resource table

[Mukesh Ojha]

On Fri, Dec 05, 2025 at 04:17:56PM -0600, Bjorn Andersson wrote:
> On Fri, Dec 05, 2025 at 02:15:00PM +0100, Konrad Dybcio wrote:
> > On 12/4/25 1:28 PM, Mukesh Ojha wrote:
> > > On Wed, Dec 03, 2025 at 01:36:32PM +0100, Konrad Dybcio wrote:
> > >> On 11/24/25 4:25 PM, Mukesh Ojha wrote:
> > >>> On Mon, Nov 24, 2025 at 12:48:31PM +0100, Konrad Dybcio wrote:
> > >>>> On 11/21/25 12:01 PM, Mukesh Ojha wrote:
> > >>>>> Qualcomm remote processor may rely on Static and Dynamic resources for
> > >>>>> it to be functional. Static resources are fixed like for example,
> > >>>>> memory-mapped addresses required by the subsystem and dynamic
> > >>>>> resources, such as shared memory in DDR etc., are determined at
> > >>>>> runtime during the boot process.
> > >>>>>
> > >>>>> For most of the Qualcomm SoCs, when run with Gunyah or older QHEE
> > >>>>> hypervisor, all the resources whether it is static or dynamic, is
> > >>>>> managed by the hypervisor. Dynamic resources if it is present for a
> > >>>>> remote processor will always be coming from secure world via SMC call
> > >>>>> while static resources may be present in remote processor firmware
> > >>>>> binary or it may be coming qcom_scm_pas_get_rsc_table() SMC call along
> > >>>>> with dynamic resources.
> > 
> > [...]
> > 
> > > Just to avoid iteration, are you suggesting that we can keep this
> > > guesswork as part of __qcom_scm_pas_get_rsc_table() and start with
> > > something smaller than SZ_16K?
> > > 
> > > I kind of agree with the first part, but SZ_16K was the recommended size
> > > from the firmware for Lemans to start with, in order to pass the SMC
> > > successfully on the first try. However, the same size was failing for
> > > Glymur, and it required a second attempt with the correct size.
> > 
> > It depends on the payload, which you're probably much more familiar with.
> > If 95% of them will be closer to e.g. 1K in size, it perhaps makes sense
> > to use up the additional few dozen cycles on our amazingly fast CPUs and
> > retry as necessary, instead of blindly reserving a whole bunch of memory.
> > 
> 
> Those "few dozen cycles", is tasked with sending messages to RPMh for
> voting and unvoting the buses, then tzmem will hopefully hit the
> genpool, twice, and then radix updates, and then more genpool updated
> and more radix tree work. And then of course there's the one context
> switch to secure world.
> 
> If we don't have space in the genpool, we're going to grow
> dma_alloc_coherent, extend the genpool, call secure world to register
> the new tzmem. And then for all those cases where the allocation wasn't
> enough, the retry (with updated size) will not fit in the
> PAGE_ALIGN(size) genpool that was created, so we'll do this twice.
> 
> Fortunately the tzmem growing should only happen on first remoteproc
> boot, but I think it's a bit optimistic to say "a few dozen"...
> 
> 
> The drawback with making it 16KB is that we're not going to test that
> error path very often. But the more idiomatic form of first calling with
> a size of 0, then allocate and pass the proper size, seems a bit
> wasteful to me as well - in particular if we do it anew each subsystem
> boot.
> 
> PS. 16KB is 0.03% of the ADSP carveout (or 3% of the ADSP DeviceTree
> carveout...).

What you suggest ? 0x36c8 bytes was the size for CDSP resource table on Lemans.

-- 
-Mukesh Ojha

Re: [PATCH v8 11/14] firmware: qcom_scm: Add qcom_scm_pas_get_rsc_table() to get resource table

[Mukesh Ojha]

On Mon, Dec 08, 2025 at 10:19:26PM +0530, Mukesh Ojha wrote:
> On Fri, Dec 05, 2025 at 04:40:51PM -0600, Bjorn Andersson wrote:
> > On Fri, Nov 21, 2025 at 04:31:13PM +0530, Mukesh Ojha wrote:
> > > Qualcomm remote processor may rely on Static and Dynamic resources for
> > > it to be functional. Static resources are fixed like for example,
> > > memory-mapped addresses required by the subsystem and dynamic
> > > resources, such as shared memory in DDR etc., are determined at
> > > runtime during the boot process.
> > > 
> > > For most of the Qualcomm SoCs, when run with Gunyah or older QHEE
> > > hypervisor, all the resources whether it is static or dynamic, is
> > > managed by the hypervisor. Dynamic resources if it is present for a
> > > remote processor will always be coming from secure world via SMC call
> > > while static resources may be present in remote processor firmware
> > > binary or it may be coming qcom_scm_pas_get_rsc_table() SMC call along
> > > with dynamic resources.
> > > 
> > > Some of the remote processor drivers, such as video, GPU, IPA, etc., do
> > > not check whether resources are present in their remote processor
> > > firmware binary. In such cases, the caller of this function should set
> > > input_rt and input_rt_size as NULL and zero respectively. Remoteproc
> > > framework has method to check whether firmware binary contain resources
> > > or not and they should be pass resource table pointer to input_rt and
> > > resource table size to input_rt_size and this will be forwarded to
> > > TrustZone for authentication. TrustZone will then append the dynamic
> > > resources and return the complete resource table in output_rt
> > > 
> > > More about documentation on resource table format can be found in
> > > include/linux/remoteproc.h
> > > 
> > > Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> > > ---
> > >  drivers/firmware/qcom/qcom_scm.c       | 158 +++++++++++++++++++++++++++++++++
> > >  drivers/firmware/qcom/qcom_scm.h       |   1 +
> > >  include/linux/firmware/qcom/qcom_scm.h |   4 +
> > >  3 files changed, 163 insertions(+)
> > > 
> > > diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> > > index 84498d0d2f0c..c4420b79fb57 100644
> > > --- a/drivers/firmware/qcom/qcom_scm.c
> > > +++ b/drivers/firmware/qcom/qcom_scm.c
> > > @@ -27,6 +27,7 @@
> > >  #include <linux/of_reserved_mem.h>
> > >  #include <linux/platform_device.h>
> > >  #include <linux/reset-controller.h>
> > > +#include <linux/remoteproc.h>
> > >  #include <linux/sizes.h>
> > >  #include <linux/types.h>
> > >  
> > > @@ -111,6 +112,10 @@ enum qcom_scm_qseecom_tz_cmd_info {
> > >  	QSEECOM_TZ_CMD_INFO_VERSION		= 3,
> > >  };
> > >  
> > > +enum qcom_scm_rsctable_resp_type {
> > > +	RSCTABLE_BUFFER_NOT_SUFFICIENT		= 20,
> > > +};
> > > +
> > >  #define QSEECOM_MAX_APP_NAME_SIZE		64
> > >  #define SHMBRIDGE_RESULT_NOTSUPP		4
> > >  
> > > @@ -766,6 +771,159 @@ int qcom_scm_pas_mem_setup(u32 pas_id, phys_addr_t addr, phys_addr_t size)
> > >  }
> > >  EXPORT_SYMBOL_GPL(qcom_scm_pas_mem_setup);
> > >  
> > > +static int __qcom_scm_pas_get_rsc_table(u32 pas_id, void *input_rt, size_t input_rt_size,
> > > +					void **output_rt, size_t *output_rt_size)
> > 
> > output_rt is not going to be modified, only its content, so it can be
> > void * (single pointer).
> > 
> > > +{
> > > +	struct qcom_scm_desc desc = {
> > > +		.svc = QCOM_SCM_SVC_PIL,
> > > +		.cmd = QCOM_SCM_PIL_PAS_GET_RSCTABLE,
> > > +		.arginfo = QCOM_SCM_ARGS(5, QCOM_SCM_VAL, QCOM_SCM_RO, QCOM_SCM_VAL,
> > > +					 QCOM_SCM_RW, QCOM_SCM_VAL),
> > > +		.args[0] = pas_id,
> > > +		.owner = ARM_SMCCC_OWNER_SIP,
> > > +	};
> > > +	void *input_rt_buf, *output_rt_buf;
> > 
> > I do one prefer one variable per line, preferably in reverse xmas order.
> 
> Ack.
> 
> > 
> > > +	struct resource_table *rsc;
> > 
> > Calling this "empty_rsc" will make its purpose obvious.
> 
> Ack.
> 
> > 
> > > +	struct qcom_scm_res res;
> > > +	int ret;
> > > +
> > > +	ret = qcom_scm_clk_enable();
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	ret = qcom_scm_bw_enable();
> > > +	if (ret)
> > > +		goto disable_clk;
> > > +
> > > +	/*
> > > +	 * TrustZone can not accept buffer as NULL value as argument Hence,
> > > +	 * we need to pass a input buffer indicating that subsystem firmware
> > > +	 * does not have resource table by filling resource table structure.
> > > +	 */
> > > +	if (!input_rt)
> > > +		input_rt_size = sizeof(*rsc);
> > 
> > If you overwrite input_rt here, you don't need to repeat this
> > conditional below, like:
> > 
> > 	struct resource_table empty_rsc = {};
> > 	
> > 	...
> > 	
> > 	if (!input_rt) {
> > 		input_rt = &empty_rsc;
> > 		input_rt_size = sizeof(rsc);
> > 	}
> > 	
> > 	qcom_tzmem_alloc(input_rt_size)
> > 	
> > 	memcpy(input_rt_buf, input_rt);
> 
> Ack.
> 
> > 
> > > +
> > > +	input_rt_buf = qcom_tzmem_alloc(__scm->mempool, input_rt_size, GFP_KERNEL);
> > > +	if (!input_rt_buf) {
> > > +		ret = -ENOMEM;
> > > +		goto disable_scm_bw;
> > > +	}
> > > +
> > > +	if (!input_rt) {
> > > +		rsc = input_rt_buf;
> > > +		rsc->num = 0;
> > > +	} else {
> > > +		memcpy(input_rt_buf, input_rt, input_rt_size);
> > > +	}
> > > +
> > 
> > Reading this patch once more, it looks reasonable, but few of the things
> > in this function actually depend on *output_rt_size, yet we perform them
> > in the loop below.
> > 
> > We're expecting, with some certainty, that this sequence will be called
> > more than once, so I think it would be preferable to slice this
> > differently, and only repeat the <loop></loop> part.
> > 
> > <loop>
> 
> Ack, I will move all the clock, bw voting and mentioned retry(on -EOVERFLOW) loop
> to the caller of this function.
> 
> > 
> > > +	output_rt_buf = qcom_tzmem_alloc(__scm->mempool, *output_rt_size, GFP_KERNEL);
> > > +	if (!output_rt_buf) {
> > > +		ret = -ENOMEM;
> > > +		goto free_input_rt_buf;
> > > +	}
> > > +
> > > +	desc.args[1] = qcom_tzmem_to_phys(input_rt_buf);
> > > +	desc.args[2] = input_rt_size;
> > > +	desc.args[3] = qcom_tzmem_to_phys(output_rt_buf);
> > > +	desc.args[4] = *output_rt_size;
> > > +
> > > +	/*
> > > +	 * Whether SMC fail or pass, res.result[2] will hold actual resource table
> > > +	 * size.
> > > +	 *
> > > +	 * if passed 'output_rt_size' buffer size is not sufficient to hold the
> > > +	 * resource table TrustZone sends, response code in res.result[1] as
> > > +	 * RSCTABLE_BUFFER_NOT_SUFFICIENT so that caller can retry this SMC call with
> > > +	 * output_rt buffer with res.result[2] size.
> > > +	 */
> > > +	ret = qcom_scm_call(__scm->dev, &desc, &res);
> > > +	*output_rt_size = res.result[2];
> > > +	if (!ret)
> > > +		memcpy(*output_rt, output_rt_buf, *output_rt_size);
> > > +
> > > +	if (ret && res.result[1] == RSCTABLE_BUFFER_NOT_SUFFICIENT)
> > > +		ret = -EAGAIN;
> > 
> > </loop>
> > 
> > > +
> > > +	qcom_tzmem_free(output_rt_buf);
> > > +
> > > +free_input_rt_buf:
> > > +	qcom_tzmem_free(input_rt_buf);
> > > +
> > > +disable_scm_bw:
> > > +	qcom_scm_bw_disable();
> > > +
> > > +disable_clk:
> > > +	qcom_scm_clk_disable();
> > > +
> > > +	return ret ? : res.result[0];
> > 
> > Is there a risk that res.result[0] will carry something meaningful to
> > the caller (which will be misinterpreted)?
> 
> No, its just to align with other SMC call, it will always have 0 value
> on success.
> 
> > 
> > > +}
> > > +
> > > +/**
> > > + * qcom_scm_pas_get_rsc_table() - Retrieve the resource table in passed output buffer
> > > + *				  for a given peripheral.
> > > + *
> > > + * Qualcomm remote processor may rely on both static and dynamic resources for
> > > + * its functionality. Static resources typically refer to memory-mapped addresses
> > > + * required by the subsystem and are often embedded within the firmware binary
> > > + * and dynamic resources, such as shared memory in DDR etc., are determined at
> > > + * runtime during the boot process.
> > > + *
> > > + * On Qualcomm Technologies devices, it's possible that static resources are not
> > > + * embedded in the firmware binary and instead are provided by TrustZone However,
> > > + * dynamic resources are always expected to come from TrustZone. This indicates
> > > + * that for Qualcomm devices, all resources (static and dynamic) will be provided
> > > + * by TrustZone via the SMC call.
> > > + *
> > > + * If the remote processor firmware binary does contain static resources, they
> > > + * should be passed in input_rt. These will be forwarded to TrustZone for
> > > + * authentication. TrustZone will then append the dynamic resources and return
> > > + * the complete resource table in output_rt.
> > > + *
> > > + * If the remote processor firmware binary does not include a resource table,
> > > + * the caller of this function should set input_rt as NULL and input_rt_size
> > > + * as zero respectively.
> > > + *
> > > + * More about documentation on resource table data structures can be found in
> > > + * include/linux/remoteproc.h
> > > + *
> > > + * @ctx:	    PAS context
> > > + * @pas_id:	    peripheral authentication service id
> > > + * @input_rt:       resource table buffer which is present in firmware binary
> > > + * @input_rt_size:  size of the resource table present in firmware binary
> > > + * @output_rt:	    buffer to which the both static and dynamic resources will
> > > + *		    be returned.
> > > + * @output_rt_size: TrustZone expects caller should pass worst case size for
> > > + *		    the output_rt.
> > > + *
> > > + * Return: 0 on success and nonzero on failure.
> > > + *
> > > + * Upon successful return, output_rt will have the resource table and output_rt_size
> > > + * will have actual resource table size,
> > > + */
> > > +int qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *ctx, void *input_rt,
> > > +			       size_t input_rt_size, void **output_rt,
> > > +			       size_t *output_rt_size)
> > > +{
> > > +	unsigned int retry_num = 5;
> > > +	int ret;
> > > +
> > > +	do {
> > > +		*output_rt = kzalloc(*output_rt_size, GFP_KERNEL);
> > 
> > I'd prefer the output buffer and size to be carried in a local variables
> > until we determine success, to avoid overwriting the caller's size with
> > some bogus number and return a pointer to freed memory.
> 
> Sure.
> 
> > 
> > Wouldn't be unreasonable to return an ERR_PTR() with the allocated
> > buffer, instead of returning through the reference.
> 
> We anyway have to return the size through reference, why not do the same for 
> allocated buffer as well..


Tried to take care most of the comments apart from above one., let me know if
below is fine..

---------------------------------------o<---------------------------------------

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 4ce892d8fb25..a589961f8225 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -27,6 +27,7 @@
 #include <linux/of_reserved_mem.h>
 #include <linux/platform_device.h>
 #include <linux/reset-controller.h>
+#include <linux/remoteproc.h>
 #include <linux/sizes.h>
 #include <linux/types.h>
 
@@ -111,6 +112,10 @@ enum qcom_scm_qseecom_tz_cmd_info {
 	QSEECOM_TZ_CMD_INFO_VERSION		= 3,
 };
 
+enum qcom_scm_rsctable_resp_type {
+	RSCTABLE_BUFFER_NOT_SUFFICIENT		= 20,
+};
+
 #define QSEECOM_MAX_APP_NAME_SIZE		64
 #define SHMBRIDGE_RESULT_NOTSUPP		4
 
@@ -766,6 +771,171 @@ int qcom_scm_pas_mem_setup(u32 pas_id, phys_addr_t addr, phys_addr_t size)
 }
 EXPORT_SYMBOL_GPL(qcom_scm_pas_mem_setup);
 
+static int __qcom_scm_pas_get_rsc_table(u32 pas_id, void *input_rt,
+					size_t input_rt_size, void *output_rt,
+					size_t *output_rt_size)
+{
+	struct qcom_scm_desc desc = {
+		.svc = QCOM_SCM_SVC_PIL,
+		.cmd = QCOM_SCM_PIL_PAS_GET_RSCTABLE,
+		.arginfo = QCOM_SCM_ARGS(5, QCOM_SCM_VAL, QCOM_SCM_RO, QCOM_SCM_VAL,
+					 QCOM_SCM_RW, QCOM_SCM_VAL),
+		.args[0] = pas_id,
+		.owner = ARM_SMCCC_OWNER_SIP,
+	};
+	struct qcom_scm_res res;
+	int ret;
+
+	desc.args[1] = qcom_tzmem_to_phys(input_rt);
+	desc.args[2] = input_rt_size;
+	desc.args[3] = qcom_tzmem_to_phys(output_rt);
+	desc.args[4] = *output_rt_size;
+
+	/*
+	 * Whether SMC fail or pass, res.result[2] will hold actual resource table
+	 * size.
+	 *
+	 * If passed 'output_rt_size' buffer size is not sufficient to hold the
+	 * resource table TrustZone sends, response code in res.result[1] as
+	 * RSCTABLE_BUFFER_NOT_SUFFICIENT so that caller can retry this SMC call
+	 * with output_rt buffer with res.result[2] size however, It should not
+	 * be of unresonable size.
+	 */
+	ret = qcom_scm_call(__scm->dev, &desc, &res);
+	if (res.result[2] > SZ_1G) {
+		ret = -E2BIG;
+		return ret;
+	}
+
+	*output_rt_size = res.result[2];
+	if (ret && res.result[1] == RSCTABLE_BUFFER_NOT_SUFFICIENT)
+		ret = -EOVERFLOW;
+
+	return ret ? : res.result[0];
+}
+
+/**
+ * qcom_scm_pas_get_rsc_table() - Retrieve the resource table in passed output buffer
+ *				  for a given peripheral.
+ *
+ * Qualcomm remote processor may rely on both static and dynamic resources for
+ * its functionality. Static resources typically refer to memory-mapped addresses
+ * required by the subsystem and are often embedded within the firmware binary
+ * and dynamic resources, such as shared memory in DDR etc., are determined at
+ * runtime during the boot process.
+ *
+ * On Qualcomm Technologies devices, it's possible that static resources are not
+ * embedded in the firmware binary and instead are provided by TrustZone However,
+ * dynamic resources are always expected to come from TrustZone. This indicates
+ * that for Qualcomm devices, all resources (static and dynamic) will be provided
+ * by TrustZone via the SMC call.
+ *
+ * If the remote processor firmware binary does contain static resources, they
+ * should be passed in input_rt. These will be forwarded to TrustZone for
+ * authentication. TrustZone will then append the dynamic resources and return
+ * the complete resource table in output_rt.
+ *
+ * If the remote processor firmware binary does not include a resource table,
+ * the caller of this function should set input_rt as NULL and input_rt_size
+ * as zero respectively.
+ *
+ * More about documentation on resource table data structures can be found in
+ * include/linux/remoteproc.h
+ *
+ * @ctx:	    PAS context
+ * @pas_id:	    peripheral authentication service id
+ * @input_rt:       resource table buffer which is present in firmware binary
+ * @input_rt_size:  size of the resource table present in firmware binary
+ * @output_rt:	    buffer to which the both static and dynamic resources will
+ *		    be returned.
+ * @output_rt_size: TrustZone expects caller should pass worst case size for
+ *		    the output_rt.
+ *
+ * Return: 0 on success and nonzero on failure.
+ *
+ * Upon successful return, output_rt will have the resource table and output_rt_size
+ * will have actual resource table size,
+ */
+int qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *ctx, void *input_rt,
+			       size_t input_rt_size, void **output_rt,
+			       size_t *output_rt_size)
+{
+	struct resource_table empty_rsc = {};
+	size_t size = SZ_16K;
+	void *output_rt_tzm;
+	void *input_rt_tzm;
+	int ret;
+
+	ret = qcom_scm_clk_enable();
+	if (ret)
+		return ret;
+
+	ret = qcom_scm_bw_enable();
+	if (ret)
+		goto disable_clk;
+
+	/*
+	 * TrustZone can not accept buffer as NULL value as argument Hence,
+	 * we need to pass a input buffer indicating that subsystem firmware
+	 * does not have resource table by filling resource table structure.
+	 */
+	if (!input_rt) {
+		input_rt = &empty_rsc;
+		input_rt_size = sizeof(empty_rsc);
+	}
+
+	input_rt_tzm = qcom_tzmem_alloc(__scm->mempool, input_rt_size, GFP_KERNEL);
+	if (!input_rt_tzm) {
+		ret = -ENOMEM;
+		goto disable_scm_bw;
+	}
+
+	memcpy(input_rt_tzm, input_rt, input_rt_size);
+
+	do {
+		output_rt_tzm = qcom_tzmem_alloc(__scm->mempool, size, GFP_KERNEL);
+		if (!output_rt_tzm) {
+			ret = -ENOMEM;
+			goto free_input_rt;
+		}
+
+		ret = __qcom_scm_pas_get_rsc_table(ctx->pas_id, input_rt_tzm,
+						   input_rt_size, output_rt_tzm,
+						   &size);
+		if (ret)
+			qcom_tzmem_free(output_rt_tzm);
+
+	} while (ret == -EOVERFLOW);
+
+	if (!ret) {
+		void *tbl_ptr;
+
+		tbl_ptr = kzalloc(size, GFP_KERNEL);
+		if (!tbl_ptr)
+			goto free_output_rt;
+
+		memcpy(tbl_ptr, output_rt_tzm, size);
+		*output_rt = tbl_ptr;
+		*output_rt_size = size;
+	}
+
+free_output_rt:
+	if (!ret)
+		qcom_tzmem_free(output_rt_tzm);
+
+free_input_rt:
+	qcom_tzmem_free(input_rt_tzm);
+
+disable_scm_bw:
+	qcom_scm_bw_disable();
+
+disable_clk:
+	qcom_scm_clk_disable();
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(qcom_scm_pas_get_rsc_table);
+
 /**
  * qcom_scm_pas_auth_and_reset() - Authenticate the given peripheral firmware
  *				   and reset the remote processor
diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h
index a56c8212cc0c..50d87c628d78 100644
--- a/drivers/firmware/qcom/qcom_scm.h
+++ b/drivers/firmware/qcom/qcom_scm.h
@@ -105,6 +105,7 @@ int qcom_scm_shm_bridge_enable(struct device *scm_dev);
 #define QCOM_SCM_PIL_PAS_SHUTDOWN	0x06
 #define QCOM_SCM_PIL_PAS_IS_SUPPORTED	0x07
 #define QCOM_SCM_PIL_PAS_MSS_RESET	0x0a
+#define QCOM_SCM_PIL_PAS_GET_RSCTABLE	0x21
 
 #define QCOM_SCM_SVC_IO			0x05
 #define QCOM_SCM_IO_READ		0x01
diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
index d6d83888bb75..7c331598ea15 100644
--- a/include/linux/firmware/qcom/qcom_scm.h
+++ b/include/linux/firmware/qcom/qcom_scm.h
@@ -88,6 +88,10 @@ 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);
 bool qcom_scm_pas_supported(u32 pas_id);
+int qcom_scm_pas_get_rsc_table(struct qcom_scm_pas_context *ctx, void *input_rt,
+			       size_t input_rt_size, void **output_rt,
+			       size_t *output_rt_size);
+
 

> 
> 
> -- 
> -Mukesh Ojha

-- 
-Mukesh Ojha

Re: [PATCH v8 00/14] Peripheral Image Loader support for Qualcomm SoCs running Linux host at EL2

[Vikash Garodia]

On 12/6/2025 2:48 AM, Dmitry Baryshkov wrote:
> On Wed, Dec 03, 2025 at 10:48:14AM +0530, Vikash Garodia wrote:
>>
>> On 12/3/2025 2:54 AM, Bjorn Andersson wrote:
>>> On Tue, Dec 02, 2025 at 03:43:17PM +0530, Vikash Garodia wrote:
>>>>
>>>> On 12/2/2025 2:06 PM, Mukesh Ojha wrote:
>>>>> On Thu, Nov 27, 2025 at 10:25:23AM +0000, Bryan O'Donoghue wrote:
>>>>>> On 21/11/2025 11:37, Mukesh Ojha wrote:
>>>>>>>> Sorry.
>>>>>>>>
>>>>>>>> Did we actually come up with a cogent reason to omit the video firmware
>>>>>>>> loading here ?
>>>>>>>>
>>>>>>>> AFAIU it is required for Lemans and Glymur - leaving it out is blocking
>>>>>>>> getting video stuff done and storing up trouble.
>>>>>>>>
>>>>>>>> What exactly is the blockage - is it something you want help with ?
>>>>>>> I replied to you here[1] and given my reason..till something concluded on
>>>>>>> "multi-cell IOMMU[2]", I can not add video and block what is working
>>>>>>> already.
>>>>>>>
>>>>>>> [1]
>>>>>>> https://lore.kernel.org/lkml/20251105081421.f6j7ks5bd4dfgr67@hu-mojha-
>>>>>>> hyd.qualcomm.com/
>>>>>>
>>>>>> Why though ?
>>>>>>
>>>>>> You are mixing together the issue of multiple SIDs and the original loading
>>>>>> of firmware which could easily reuse the venus method of
>>>>>>
>>>>>> &iris {
>>>>>> 	video-firmware {
>>>>>> 		iommus = <&apss_smmu hex>;
>>>>>> 	};
>>>>>> };
>>>>>
>>>>> I completely understand what you are saying, and it would be very easy
>>>>> for me to do that if it gets accepted. However, I doubt that the people
>>>>> who raised this concern would agree with the approach.
>>>>>
>>>>> I’m not sure if the video team would like to pursue pixel/non-pixel/firmware context
>>>>> banks separately. I’ll leave this to @Vikas to answer.
>>>>
>>>> Not exactly as a separate sub-node, but i do like the idea of introducing a
>>>> simple iommu property, something like this, which Stephan proposed earlier
>>>> in the discussion [1]
>>>>
>>>> firmware-iommus = <&apps_smmu ...>;
>>>>
>>>> I understand that we are doing the iommu-map thing, but a property
>>>> exclusively for firmware like above look much simpler to me.
>>>>
>>>
>>> "We know we need to find a generic solution to this very problem, but
>>> while we work on that let's add this quick hack to the ABI"?
>>
>> I would not call that as hack, rather a simpler solution instead of packing
>> everything into the generic iommu-map.
>>
>> "firmware-iommus" is much more readable to interpret something running in
>> el2 mode, than digging into function ids inside iommu-map and then matching
>> it up with specific SIDs to confirm.
> 
> If you want it formally, NAK from my side for firmware-iommus. Either
> reuse an existing approach (at least it makese sense from the historical
> point of view) or introduce a generic approach, which is iommu-maps. The
> proposed firmware-iommus is definitely a hack around the IOMMU
> properties.
> 
> But it's really off-topic here.

Infact i see a concern with the iommu-map approach for firmware SIDs. 
Let say the hardware generates 10 SIDs, including firmware. So video 
binding should describe those 10 SIDs and the DTS should have all those 
10 SIDs as well, including firmware SID.
Given above, video driver cannot distinguish if the SOC is running in 
EL2 (KVM) mode or Gunyah mode.

Could you please suggest if we can make it work with iommu-map approach

Regards,
Vikash

Re: [PATCH v8 00/14] Peripheral Image Loader support for Qualcomm SoCs running Linux host at EL2

[Konrad Dybcio]

On 12/17/25 11:08 AM, Vikash Garodia wrote:
> 
> On 12/6/2025 2:48 AM, Dmitry Baryshkov wrote:
>> On Wed, Dec 03, 2025 at 10:48:14AM +0530, Vikash Garodia wrote:
>>>
>>> On 12/3/2025 2:54 AM, Bjorn Andersson wrote:
>>>> On Tue, Dec 02, 2025 at 03:43:17PM +0530, Vikash Garodia wrote:
>>>>>
>>>>> On 12/2/2025 2:06 PM, Mukesh Ojha wrote:
>>>>>> On Thu, Nov 27, 2025 at 10:25:23AM +0000, Bryan O'Donoghue wrote:
>>>>>>> On 21/11/2025 11:37, Mukesh Ojha wrote:
>>>>>>>>> Sorry.
>>>>>>>>>
>>>>>>>>> Did we actually come up with a cogent reason to omit the video firmware
>>>>>>>>> loading here ?
>>>>>>>>>
>>>>>>>>> AFAIU it is required for Lemans and Glymur - leaving it out is blocking
>>>>>>>>> getting video stuff done and storing up trouble.
>>>>>>>>>
>>>>>>>>> What exactly is the blockage - is it something you want help with ?
>>>>>>>> I replied to you here[1] and given my reason..till something concluded on
>>>>>>>> "multi-cell IOMMU[2]", I can not add video and block what is working
>>>>>>>> already.
>>>>>>>>
>>>>>>>> [1]
>>>>>>>> https://lore.kernel.org/lkml/20251105081421.f6j7ks5bd4dfgr67@hu-mojha-
>>>>>>>> hyd.qualcomm.com/
>>>>>>>
>>>>>>> Why though ?
>>>>>>>
>>>>>>> You are mixing together the issue of multiple SIDs and the original loading
>>>>>>> of firmware which could easily reuse the venus method of
>>>>>>>
>>>>>>> &iris {
>>>>>>>     video-firmware {
>>>>>>>         iommus = <&apss_smmu hex>;
>>>>>>>     };
>>>>>>> };
>>>>>>
>>>>>> I completely understand what you are saying, and it would be very easy
>>>>>> for me to do that if it gets accepted. However, I doubt that the people
>>>>>> who raised this concern would agree with the approach.
>>>>>>
>>>>>> I’m not sure if the video team would like to pursue pixel/non-pixel/firmware context
>>>>>> banks separately. I’ll leave this to @Vikas to answer.
>>>>>
>>>>> Not exactly as a separate sub-node, but i do like the idea of introducing a
>>>>> simple iommu property, something like this, which Stephan proposed earlier
>>>>> in the discussion [1]
>>>>>
>>>>> firmware-iommus = <&apps_smmu ...>;
>>>>>
>>>>> I understand that we are doing the iommu-map thing, but a property
>>>>> exclusively for firmware like above look much simpler to me.
>>>>>
>>>>
>>>> "We know we need to find a generic solution to this very problem, but
>>>> while we work on that let's add this quick hack to the ABI"?
>>>
>>> I would not call that as hack, rather a simpler solution instead of packing
>>> everything into the generic iommu-map.
>>>
>>> "firmware-iommus" is much more readable to interpret something running in
>>> el2 mode, than digging into function ids inside iommu-map and then matching
>>> it up with specific SIDs to confirm.
>>
>> If you want it formally, NAK from my side for firmware-iommus. Either
>> reuse an existing approach (at least it makese sense from the historical
>> point of view) or introduce a generic approach, which is iommu-maps. The
>> proposed firmware-iommus is definitely a hack around the IOMMU
>> properties.
>>
>> But it's really off-topic here.
> 
> Infact i see a concern with the iommu-map approach for firmware SIDs. Let say the hardware generates 10 SIDs, including firmware. So video binding should describe those 10 SIDs and the DTS should have all those 10 SIDs as well, including firmware SID.
> Given above, video driver cannot distinguish if the SOC is running in EL2 (KVM) mode or Gunyah mode.

EL2 vs Gunyah is not hard (something like is_hyp_mode_available()), but
again, this should all be calling some sort of is_gunyah() which would
come from the gunyah hyp drivers, which have seen no activity on lkml
for over a year..

Konrad

Re: [PATCH v8 11/14] firmware: qcom_scm: Add qcom_scm_pas_get_rsc_table() to get resource table

[Konrad Dybcio]

On 12/5/25 11:17 PM, Bjorn Andersson wrote:
> On Fri, Dec 05, 2025 at 02:15:00PM +0100, Konrad Dybcio wrote:
>> On 12/4/25 1:28 PM, Mukesh Ojha wrote:
>>> On Wed, Dec 03, 2025 at 01:36:32PM +0100, Konrad Dybcio wrote:
>>>> On 11/24/25 4:25 PM, Mukesh Ojha wrote:
>>>>> On Mon, Nov 24, 2025 at 12:48:31PM +0100, Konrad Dybcio wrote:
>>>>>> On 11/21/25 12:01 PM, Mukesh Ojha wrote:
>>>>>>> Qualcomm remote processor may rely on Static and Dynamic resources for
>>>>>>> it to be functional. Static resources are fixed like for example,
>>>>>>> memory-mapped addresses required by the subsystem and dynamic
>>>>>>> resources, such as shared memory in DDR etc., are determined at
>>>>>>> runtime during the boot process.
>>>>>>>
>>>>>>> For most of the Qualcomm SoCs, when run with Gunyah or older QHEE
>>>>>>> hypervisor, all the resources whether it is static or dynamic, is
>>>>>>> managed by the hypervisor. Dynamic resources if it is present for a
>>>>>>> remote processor will always be coming from secure world via SMC call
>>>>>>> while static resources may be present in remote processor firmware
>>>>>>> binary or it may be coming qcom_scm_pas_get_rsc_table() SMC call along
>>>>>>> with dynamic resources.
>>
>> [...]
>>
>>> Just to avoid iteration, are you suggesting that we can keep this
>>> guesswork as part of __qcom_scm_pas_get_rsc_table() and start with
>>> something smaller than SZ_16K?
>>>
>>> I kind of agree with the first part, but SZ_16K was the recommended size
>>> from the firmware for Lemans to start with, in order to pass the SMC
>>> successfully on the first try. However, the same size was failing for
>>> Glymur, and it required a second attempt with the correct size.
>>
>> It depends on the payload, which you're probably much more familiar with.
>> If 95% of them will be closer to e.g. 1K in size, it perhaps makes sense
>> to use up the additional few dozen cycles on our amazingly fast CPUs and
>> retry as necessary, instead of blindly reserving a whole bunch of memory.
>>
> 
> Those "few dozen cycles", is tasked with sending messages to RPMh for
> voting and unvoting the buses, then tzmem will hopefully hit the
> genpool, twice, and then radix updates, and then more genpool updated
> and more radix tree work. And then of course there's the one context
> switch to secure world.
> 
> If we don't have space in the genpool, we're going to grow
> dma_alloc_coherent, extend the genpool, call secure world to register
> the new tzmem. And then for all those cases where the allocation wasn't
> enough, the retry (with updated size) will not fit in the
> PAGE_ALIGN(size) genpool that was created, so we'll do this twice.
> 
> Fortunately the tzmem growing should only happen on first remoteproc
> boot, but I think it's a bit optimistic to say "a few dozen"...
> 
> 
> The drawback with making it 16KB is that we're not going to test that
> error path very often. But the more idiomatic form of first calling with
> a size of 0, then allocate and pass the proper size, seems a bit
> wasteful to me as well - in particular if we do it anew each subsystem
> boot.
> 
> PS. 16KB is 0.03% of the ADSP carveout (or 3% of the ADSP DeviceTree
> carveout...).

Hm, perhaps 16 is not a bad choice then

Konrad