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

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
>

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

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

On Tue, Dec 09, 2025 at 04:15:25PM +0530, Mukesh Ojha wrote:
> 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:
[..]
> > > 
> > > 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..

It's indeed fine, much better. Thank you.

> 
> ---------------------------------------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,

This isn't an enumeration, it's the constant 20. So #define would be
more suitable...

> +};
> +
>  #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,

Lowercase 'h' in Hence, please.

> +	 * 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;

I still think it would be nice to match the prototype of
rproc_find_loaded_rsc_table() and return the pointer our resource table.

Effectively:
  return ret ? ERR_PTR(ret) : tbl_ptr;

(although return tbl_ptr; after making sure it's either the allocated
memory or ERR_PTR() would be nicer)

Should be able to make the return type struct resource_table * (even
though struct resource_table is a opaque type here).

But, my concrete concerns have been addressed, so I'm happy either way.

Regards,
Bjorn

> +}
> +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

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

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

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

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

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

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

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

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

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

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