* [PATCH] remoteproc: qcom: pas: pass no resource table when the firmware has none
@ 2026-10-05 12:54 Jorge Ramirez-Ortiz
2026-10-05 16:12 ` Mukesh Ojha
0 siblings, 1 reply; 8+ messages in thread
From: Jorge Ramirez-Ortiz @ 2026-10-05 12:54 UTC (permalink / raw)
To: jorge.ramirez, andersson, mathieu.poirier, konrad.dybcio,
mukesh.ojha, sumit.garg
Cc: linux-arm-msm, linux-remoteproc, linux-kernel
The firmware resource table is passed to the PAS backend even when the
firmware carries none. This only works on the first boot, while the
cached pointer and its size are both zero.
Stopping the remote processor, or failing to start it, frees the cached
table and clears the pointer but leaves the size set. The next start
then pairs a NULL table with a non-zero size.
The SCM backend substitutes an empty table and hides the problem. The
TEE backend copies from the NULL pointer:
remoteproc remoteproc2: powering up cdsp
pc : __pi_memcpy_generic+0x110/0x22c
lr : qcom_pas_tee_get_rsc_table+0xf4/0x25c
Call trace:
__pi_memcpy_generic+0x110/0x22c (P)
qcom_pas_get_rsc_table+0x38/0x60
qcom_pas_parse_firmware+0xa0/0x100
rproc_boot+0x2d4/0x380
state_store+0x40/0x100
Fixes: a4584bff63c8 ("remoteproc: pas: Extend parse_fw callback to fetch resources via SMC call")
Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
---
drivers/remoteproc/qcom_q6v5_pas.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index a005546c265d..e871e03ba794 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -463,7 +463,7 @@ static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *f
struct resource_table *table = NULL;
size_t output_rt_size;
void *output_rt;
- size_t table_sz;
+ size_t table_sz = 0;
int ret;
ret = qcom_register_dump_segments(rproc, fw);
@@ -476,11 +476,12 @@ static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *f
return 0;
ret = rproc_elf_load_rsc_table(rproc, fw);
- if (ret)
+ if (ret) {
dev_dbg(&rproc->dev, "Failed to load resource table from firmware\n");
-
- table = rproc->table_ptr;
- table_sz = rproc->table_sz;
+ } else {
+ table = rproc->table_ptr;
+ table_sz = rproc->table_sz;
+ }
/*
* The resources consumed by Qualcomm remote processors fall into two categories:
--
2.54.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] remoteproc: qcom: pas: pass no resource table when the firmware has none 2026-10-05 12:54 [PATCH] remoteproc: qcom: pas: pass no resource table when the firmware has none Jorge Ramirez-Ortiz @ 2026-10-05 16:12 ` Mukesh Ojha 2026-10-05 21:01 ` Jorge Ramirez 0 siblings, 1 reply; 8+ messages in thread From: Mukesh Ojha @ 2026-10-05 16:12 UTC (permalink / raw) To: Jorge Ramirez-Ortiz Cc: andersson, mathieu.poirier, konrad.dybcio, sumit.garg, linux-arm-msm, linux-remoteproc, linux-kernel On Mon, Oct 05, 2026 at 02:54:00PM +0200, Jorge Ramirez-Ortiz wrote: > The firmware resource table is passed to the PAS backend even when the > firmware carries none. This only works on the first boot, while the > cached pointer and its size are both zero. > > Stopping the remote processor, or failing to start it, frees the cached > table and clears the pointer but leaves the size set. The next start > then pairs a NULL table with a non-zero size. > > The SCM backend substitutes an empty table and hides the problem. The > TEE backend copies from the NULL pointer: > > remoteproc remoteproc2: powering up cdsp > pc : __pi_memcpy_generic+0x110/0x22c > lr : qcom_pas_tee_get_rsc_table+0xf4/0x25c > Call trace: > __pi_memcpy_generic+0x110/0x22c (P) > qcom_pas_get_rsc_table+0x38/0x60 > qcom_pas_parse_firmware+0xa0/0x100 > rproc_boot+0x2d4/0x380 > state_store+0x40/0x100 > > Fixes: a4584bff63c8 ("remoteproc: pas: Extend parse_fw callback to fetch resources via SMC call") > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com> > --- > drivers/remoteproc/qcom_q6v5_pas.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > index a005546c265d..e871e03ba794 100644 > --- a/drivers/remoteproc/qcom_q6v5_pas.c > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > @@ -463,7 +463,7 @@ static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *f > struct resource_table *table = NULL; > size_t output_rt_size; > void *output_rt; > - size_t table_sz; > + size_t table_sz = 0; > int ret; > > ret = qcom_register_dump_segments(rproc, fw); > @@ -476,11 +476,12 @@ static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *f > return 0; > > ret = rproc_elf_load_rsc_table(rproc, fw); > - if (ret) > + if (ret) { > dev_dbg(&rproc->dev, "Failed to load resource table from firmware\n"); > - > - table = rproc->table_ptr; > - table_sz = rproc->table_sz; > + } else { > + table = rproc->table_ptr; > + table_sz = rproc->table_sz; > + } Earlier code was intentional please read the comment below.. /**...*/ diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c index 2e1e39826ffa..5edb39ad5277 100644 --- a/drivers/remoteproc/qcom_q6v5_pas.c +++ b/drivers/remoteproc/qcom_q6v5_pas.c @@ -504,8 +504,11 @@ static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *f return 0; ret = rproc_elf_load_rsc_table(rproc, fw); - if (ret) + if (ret) { dev_dbg(&rproc->dev, "Failed to load resource table from firmware\n"); + rproc->table_ptr = NULL; + rproc->table_sz = 0; + } > > /* > * The resources consumed by Qualcomm remote processors fall into two categories: > -- > 2.54.0 > -- -Mukesh Ojha ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] remoteproc: qcom: pas: pass no resource table when the firmware has none 2026-10-05 16:12 ` Mukesh Ojha @ 2026-10-05 21:01 ` Jorge Ramirez 2026-10-06 5:23 ` Harshal Dev 2026-10-06 10:03 ` Mukesh Ojha 0 siblings, 2 replies; 8+ messages in thread From: Jorge Ramirez @ 2026-10-05 21:01 UTC (permalink / raw) To: Mukesh Ojha Cc: Jorge Ramirez-Ortiz, andersson, mathieu.poirier, konrad.dybcio, sumit.garg, linux-arm-msm, linux-remoteproc, linux-kernel On 05/10/26 21:42:24, Mukesh Ojha wrote: > On Mon, Oct 05, 2026 at 02:54:00PM +0200, Jorge Ramirez-Ortiz wrote: > > The firmware resource table is passed to the PAS backend even when the > > firmware carries none. This only works on the first boot, while the > > cached pointer and its size are both zero. > > > > Stopping the remote processor, or failing to start it, frees the cached > > table and clears the pointer but leaves the size set. The next start > > then pairs a NULL table with a non-zero size. > > > > The SCM backend substitutes an empty table and hides the problem. The > > TEE backend copies from the NULL pointer: > > > > remoteproc remoteproc2: powering up cdsp > > pc : __pi_memcpy_generic+0x110/0x22c > > lr : qcom_pas_tee_get_rsc_table+0xf4/0x25c > > Call trace: > > __pi_memcpy_generic+0x110/0x22c (P) > > qcom_pas_get_rsc_table+0x38/0x60 > > qcom_pas_parse_firmware+0xa0/0x100 > > rproc_boot+0x2d4/0x380 > > state_store+0x40/0x100 > > > > Fixes: a4584bff63c8 ("remoteproc: pas: Extend parse_fw callback to fetch resources via SMC call") > > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com> > > --- > > drivers/remoteproc/qcom_q6v5_pas.c | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > > index a005546c265d..e871e03ba794 100644 > > --- a/drivers/remoteproc/qcom_q6v5_pas.c > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > > @@ -463,7 +463,7 @@ static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *f > > struct resource_table *table = NULL; > > size_t output_rt_size; > > void *output_rt; > > - size_t table_sz; > > + size_t table_sz = 0; > > int ret; > > > > ret = qcom_register_dump_segments(rproc, fw); > > @@ -476,11 +476,12 @@ static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *f > > return 0; > > > > ret = rproc_elf_load_rsc_table(rproc, fw); > > - if (ret) > > + if (ret) { > > dev_dbg(&rproc->dev, "Failed to load resource table from firmware\n"); > > - > > - table = rproc->table_ptr; > > - table_sz = rproc->table_sz; > > + } else { > > + table = rproc->table_ptr; > > + table_sz = rproc->table_sz; > > + } > > > Earlier code was intentional please read the comment below.. /**...*/ > AFAICS the comments say that the call might pass NULL and zero as input resources. And this patch does exactly that - it doesnt allow NULL and non-zero. did I miss something? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] remoteproc: qcom: pas: pass no resource table when the firmware has none 2026-10-05 21:01 ` Jorge Ramirez @ 2026-10-06 5:23 ` Harshal Dev 2026-10-06 6:54 ` Jorge Ramirez 2026-10-06 10:03 ` Mukesh Ojha 1 sibling, 1 reply; 8+ messages in thread From: Harshal Dev @ 2026-10-06 5:23 UTC (permalink / raw) To: Jorge Ramirez, Mukesh Ojha Cc: andersson, mathieu.poirier, konrad.dybcio, sumit.garg, linux-arm-msm, linux-remoteproc, linux-kernel Hi Jorge, On 06-10-2026 02:31 am, Jorge Ramirez wrote: > On 05/10/26 21:42:24, Mukesh Ojha wrote: >> On Mon, Oct 05, 2026 at 02:54:00PM +0200, Jorge Ramirez-Ortiz wrote: >>> The firmware resource table is passed to the PAS backend even when the >>> firmware carries none. This only works on the first boot, while the >>> cached pointer and its size are both zero. >>> >>> Stopping the remote processor, or failing to start it, frees the cached >>> table and clears the pointer but leaves the size set. The next start >>> then pairs a NULL table with a non-zero size. >>> >>> The SCM backend substitutes an empty table and hides the problem. The >>> TEE backend copies from the NULL pointer: >>> >>> remoteproc remoteproc2: powering up cdsp >>> pc : __pi_memcpy_generic+0x110/0x22c >>> lr : qcom_pas_tee_get_rsc_table+0xf4/0x25c >>> Call trace: >>> __pi_memcpy_generic+0x110/0x22c (P) >>> qcom_pas_get_rsc_table+0x38/0x60 >>> qcom_pas_parse_firmware+0xa0/0x100 >>> rproc_boot+0x2d4/0x380 >>> state_store+0x40/0x100 >>> >>> Fixes: a4584bff63c8 ("remoteproc: pas: Extend parse_fw callback to fetch resources via SMC call") >>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com> Thanks for this patch, I was about to send one myself since I observed this on IQ10 as well. >>> --- >>> drivers/remoteproc/qcom_q6v5_pas.c | 11 ++++++----- >>> 1 file changed, 6 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c >>> index a005546c265d..e871e03ba794 100644 >>> --- a/drivers/remoteproc/qcom_q6v5_pas.c >>> +++ b/drivers/remoteproc/qcom_q6v5_pas.c >>> @@ -463,7 +463,7 @@ static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *f >>> struct resource_table *table = NULL; >>> size_t output_rt_size; >>> void *output_rt; >>> - size_t table_sz; >>> + size_t table_sz = 0; >>> int ret; >>> >>> ret = qcom_register_dump_segments(rproc, fw); >>> @@ -476,11 +476,12 @@ static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *f >>> return 0; >>> >>> ret = rproc_elf_load_rsc_table(rproc, fw); >>> - if (ret) >>> + if (ret) { >>> dev_dbg(&rproc->dev, "Failed to load resource table from firmware\n"); >>> - >>> - table = rproc->table_ptr; >>> - table_sz = rproc->table_sz; >>> + } else { >>> + table = rproc->table_ptr; >>> + table_sz = rproc->table_sz; >>> + } >> >> >> Earlier code was intentional please read the comment below.. /**...*/ >> > > AFAICS the comments say that the call might pass NULL and zero as input > resources. And this patch does exactly that - it doesnt allow NULL and > non-zero. > I agree with this fix, it was strange to see a NULL pointer paired with non-zero table size. As the comment itself says, a NULL pointer + 0 size should be passed when the firmware binary does not have a resource table. This is also documented here in a separate comment: https://elixir.bootlin.com/linux/v7.3-rc5/source/drivers/firmware/qcom/qcom_pas.c#L131 I am validating this fix for Nord, expect a Tested-by tag from me today. Regards, Harshal > did I miss something? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] remoteproc: qcom: pas: pass no resource table when the firmware has none 2026-10-06 5:23 ` Harshal Dev @ 2026-10-06 6:54 ` Jorge Ramirez 0 siblings, 0 replies; 8+ messages in thread From: Jorge Ramirez @ 2026-10-06 6:54 UTC (permalink / raw) To: Harshal Dev Cc: Jorge Ramirez, Mukesh Ojha, andersson, mathieu.poirier, konrad.dybcio, sumit.garg, linux-arm-msm, linux-remoteproc, linux-kernel On 06/10/26 10:53:31, Harshal Dev wrote: > Hi Jorge, > > On 06-10-2026 02:31 am, Jorge Ramirez wrote: > > On 05/10/26 21:42:24, Mukesh Ojha wrote: > >> On Mon, Oct 05, 2026 at 02:54:00PM +0200, Jorge Ramirez-Ortiz wrote: > >>> The firmware resource table is passed to the PAS backend even when the > >>> firmware carries none. This only works on the first boot, while the > >>> cached pointer and its size are both zero. > >>> > >>> Stopping the remote processor, or failing to start it, frees the cached > >>> table and clears the pointer but leaves the size set. The next start > >>> then pairs a NULL table with a non-zero size. > >>> > >>> The SCM backend substitutes an empty table and hides the problem. The > >>> TEE backend copies from the NULL pointer: > >>> > >>> remoteproc remoteproc2: powering up cdsp > >>> pc : __pi_memcpy_generic+0x110/0x22c > >>> lr : qcom_pas_tee_get_rsc_table+0xf4/0x25c > >>> Call trace: > >>> __pi_memcpy_generic+0x110/0x22c (P) > >>> qcom_pas_get_rsc_table+0x38/0x60 > >>> qcom_pas_parse_firmware+0xa0/0x100 > >>> rproc_boot+0x2d4/0x380 > >>> state_store+0x40/0x100 > >>> > >>> Fixes: a4584bff63c8 ("remoteproc: pas: Extend parse_fw callback to fetch resources via SMC call") > >>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com> > > Thanks for this patch, I was about to send one myself since I observed > this on IQ10 as well. > > >>> --- > >>> drivers/remoteproc/qcom_q6v5_pas.c | 11 ++++++----- > >>> 1 file changed, 6 insertions(+), 5 deletions(-) > >>> > >>> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > >>> index a005546c265d..e871e03ba794 100644 > >>> --- a/drivers/remoteproc/qcom_q6v5_pas.c > >>> +++ b/drivers/remoteproc/qcom_q6v5_pas.c > >>> @@ -463,7 +463,7 @@ static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *f > >>> struct resource_table *table = NULL; > >>> size_t output_rt_size; > >>> void *output_rt; > >>> - size_t table_sz; > >>> + size_t table_sz = 0; > >>> int ret; > >>> > >>> ret = qcom_register_dump_segments(rproc, fw); > >>> @@ -476,11 +476,12 @@ static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *f > >>> return 0; > >>> > >>> ret = rproc_elf_load_rsc_table(rproc, fw); > >>> - if (ret) > >>> + if (ret) { > >>> dev_dbg(&rproc->dev, "Failed to load resource table from firmware\n"); > >>> - > >>> - table = rproc->table_ptr; > >>> - table_sz = rproc->table_sz; > >>> + } else { > >>> + table = rproc->table_ptr; > >>> + table_sz = rproc->table_sz; > >>> + } > >> > >> > >> Earlier code was intentional please read the comment below.. /**...*/ > >> > > > > AFAICS the comments say that the call might pass NULL and zero as input > > resources. And this patch does exactly that - it doesnt allow NULL and > > non-zero. > > > > I agree with this fix, it was strange to see a NULL pointer paired with non-zero table size. > As the comment itself says, a NULL pointer + 0 size should be passed when the firmware binary > does not have a resource table. This is also documented here in a separate comment: > https://elixir.bootlin.com/linux/v7.3-rc5/source/drivers/firmware/qcom/qcom_pas.c#L131 > > I am validating this fix for Nord, expect a Tested-by tag from me today. hi Harshal, I have just sent a different fix that handles this case on the tee backend instead (so that SCM backend is unaffected and can continue implementing its underlying "null input pointer with a non-zero input size 'protocol'"...the fact that a comment is needed should have meant that a better more explicit abstraction should have been put in place IMO ). In any case, I have validated both patches on hardware with the TEE backend. Probably safer to drop this one in favour of the other one. Jorge ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] remoteproc: qcom: pas: pass no resource table when the firmware has none 2026-10-05 21:01 ` Jorge Ramirez 2026-10-06 5:23 ` Harshal Dev @ 2026-10-06 10:03 ` Mukesh Ojha 2026-10-10 11:25 ` Sumit Garg 1 sibling, 1 reply; 8+ messages in thread From: Mukesh Ojha @ 2026-10-06 10:03 UTC (permalink / raw) To: Jorge Ramirez Cc: andersson, mathieu.poirier, konrad.dybcio, sumit.garg, linux-arm-msm, linux-remoteproc, linux-kernel On Mon, Oct 05, 2026 at 11:01:24PM +0200, Jorge Ramirez wrote: > On 05/10/26 21:42:24, Mukesh Ojha wrote: > > On Mon, Oct 05, 2026 at 02:54:00PM +0200, Jorge Ramirez-Ortiz wrote: > > > The firmware resource table is passed to the PAS backend even when the > > > firmware carries none. This only works on the first boot, while the > > > cached pointer and its size are both zero. > > > > > > Stopping the remote processor, or failing to start it, frees the cached > > > table and clears the pointer but leaves the size set. The next start > > > then pairs a NULL table with a non-zero size. > > > > > > The SCM backend substitutes an empty table and hides the problem. The > > > TEE backend copies from the NULL pointer: > > > > > > remoteproc remoteproc2: powering up cdsp > > > pc : __pi_memcpy_generic+0x110/0x22c > > > lr : qcom_pas_tee_get_rsc_table+0xf4/0x25c > > > Call trace: > > > __pi_memcpy_generic+0x110/0x22c (P) > > > qcom_pas_get_rsc_table+0x38/0x60 > > > qcom_pas_parse_firmware+0xa0/0x100 > > > rproc_boot+0x2d4/0x380 > > > state_store+0x40/0x100 > > > > > > Fixes: a4584bff63c8 ("remoteproc: pas: Extend parse_fw callback to fetch resources via SMC call") > > > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com> > > > --- > > > drivers/remoteproc/qcom_q6v5_pas.c | 11 ++++++----- > > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > > > index a005546c265d..e871e03ba794 100644 > > > --- a/drivers/remoteproc/qcom_q6v5_pas.c > > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > > > @@ -463,7 +463,7 @@ static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *f > > > struct resource_table *table = NULL; > > > size_t output_rt_size; > > > void *output_rt; > > > - size_t table_sz; > > > + size_t table_sz = 0; > > > int ret; > > > > > > ret = qcom_register_dump_segments(rproc, fw); > > > @@ -476,11 +476,12 @@ static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *f > > > return 0; > > > > > > ret = rproc_elf_load_rsc_table(rproc, fw); > > > - if (ret) > > > + if (ret) { > > > dev_dbg(&rproc->dev, "Failed to load resource table from firmware\n"); > > > - > > > - table = rproc->table_ptr; > > > - table_sz = rproc->table_sz; > > > + } else { > > > + table = rproc->table_ptr; > > > + table_sz = rproc->table_sz; > > > + } > > > > > > Earlier code was intentional please read the comment below.. /**...*/ > > > > AFAICS the comments say that the call might pass NULL and zero as input > resources. And this patch does exactly that - it doesnt allow NULL and > non-zero. > > did I miss something? No, you did not miss anything.. I just wanted to highlight that I did not keep else statement in my initial implementation intentionally and relied table_ptr and size will be NULL on failure of rproc_elf_load_rsc_table().. -- -Mukesh Ojha ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] remoteproc: qcom: pas: pass no resource table when the firmware has none 2026-10-06 10:03 ` Mukesh Ojha @ 2026-10-10 11:25 ` Sumit Garg 2026-10-13 10:06 ` Mukesh Ojha 0 siblings, 1 reply; 8+ messages in thread From: Sumit Garg @ 2026-10-10 11:25 UTC (permalink / raw) To: Mukesh Ojha Cc: Jorge Ramirez, andersson, mathieu.poirier, konrad.dybcio, linux-arm-msm, linux-remoteproc, linux-kernel Hey Mukesh, On Tue, 06 Oct 2026 at 15:33:44 +0530, Mukesh Ojha wrote: >On Mon, Oct 05, 2026 at 11:01:24PM +0200, Jorge Ramirez wrote: >> On 05/10/26 21:42:24, Mukesh Ojha wrote: >> > On Mon, Oct 05, 2026 at 02:54:00PM +0200, Jorge Ramirez-Ortiz wrote: >> > > The firmware resource table is passed to the PAS backend even when the >> > > firmware carries none. This only works on the first boot, while the >> > > cached pointer and its size are both zero. >> > > >> > > Stopping the remote processor, or failing to start it, frees the cached >> > > table and clears the pointer but leaves the size set. The next start >> > > then pairs a NULL table with a non-zero size. >> > > >> > > The SCM backend substitutes an empty table and hides the problem. The >> > > TEE backend copies from the NULL pointer: >> > > >> > > remoteproc remoteproc2: powering up cdsp >> > > pc : __pi_memcpy_generic+0x110/0x22c >> > > lr : qcom_pas_tee_get_rsc_table+0xf4/0x25c >> > > Call trace: >> > > __pi_memcpy_generic+0x110/0x22c (P) >> > > qcom_pas_get_rsc_table+0x38/0x60 >> > > qcom_pas_parse_firmware+0xa0/0x100 >> > > rproc_boot+0x2d4/0x380 >> > > state_store+0x40/0x100 >> > > >> > > Fixes: a4584bff63c8 ("remoteproc: pas: Extend parse_fw callback to fetch resources via SMC call") >> > > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com> >> > > --- >> > > drivers/remoteproc/qcom_q6v5_pas.c | 11 ++++++----- >> > > 1 file changed, 6 insertions(+), 5 deletions(-) >> > > >> > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c >> > > index a005546c265d..e871e03ba794 100644 >> > > --- a/drivers/remoteproc/qcom_q6v5_pas.c >> > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c >> > > @@ -463,7 +463,7 @@ static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *f >> > > struct resource_table *table = NULL; >> > > size_t output_rt_size; >> > > void *output_rt; >> > > - size_t table_sz; >> > > + size_t table_sz = 0; >> > > int ret; >> > > >> > > ret = qcom_register_dump_segments(rproc, fw); >> > > @@ -476,11 +476,12 @@ static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *f >> > > return 0; >> > > >> > > ret = rproc_elf_load_rsc_table(rproc, fw); >> > > - if (ret) >> > > + if (ret) { >> > > dev_dbg(&rproc->dev, "Failed to load resource table from firmware\n"); >> > > - >> > > - table = rproc->table_ptr; >> > > - table_sz = rproc->table_sz; >> > > + } else { >> > > + table = rproc->table_ptr; >> > > + table_sz = rproc->table_sz; >> > > + } >> > >> > >> > Earlier code was intentional please read the comment below.. /**...*/ >> > >> >> AFAICS the comments say that the call might pass NULL and zero as input >> resources. And this patch does exactly that - it doesnt allow NULL and >> non-zero. >> >> did I miss something? > >No, you did not miss anything.. I just wanted to highlight that I >did not keep else statement in my initial implementation >intentionally and relied table_ptr and size will be NULL on >failure of rproc_elf_load_rsc_table().. Looks like you still haven't understood the problem this patch is trying to solve. It's the PIL restart sequence where the table_ptr is NULL and size being passed as non-zero which is incorrect from caller point of view. The backend will surely error out in this case. On a restart you again have to pass the table_ptr as NULL with size as 0 to fetch the resource table again. Surely we don't want the backend to hide errors on client part here. -Sumit ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] remoteproc: qcom: pas: pass no resource table when the firmware has none 2026-10-10 11:25 ` Sumit Garg @ 2026-10-13 10:06 ` Mukesh Ojha 0 siblings, 0 replies; 8+ messages in thread From: Mukesh Ojha @ 2026-10-13 10:06 UTC (permalink / raw) To: Sumit Garg Cc: Jorge Ramirez, andersson, mathieu.poirier, konrad.dybcio, linux-arm-msm, linux-remoteproc, linux-kernel On Sat, Oct 10, 2026 at 04:55:28PM +0530, Sumit Garg wrote: > Hey Mukesh, > > On Tue, 06 Oct 2026 at 15:33:44 +0530, Mukesh Ojha wrote: > > On Mon, Oct 05, 2026 at 11:01:24PM +0200, Jorge Ramirez wrote: > > > On 05/10/26 21:42:24, Mukesh Ojha wrote: > > > > On Mon, Oct 05, 2026 at 02:54:00PM +0200, Jorge Ramirez-Ortiz wrote: > > > > > The firmware resource table is passed to the PAS backend even when the > > > > > firmware carries none. This only works on the first boot, while the > > > > > cached pointer and its size are both zero. > > > > > > > > > > Stopping the remote processor, or failing to start it, frees the cached > > > > > table and clears the pointer but leaves the size set. The next start > > > > > then pairs a NULL table with a non-zero size. > > > > > > > > > > The SCM backend substitutes an empty table and hides the problem. The > > > > > TEE backend copies from the NULL pointer: > > > > > > > > > > remoteproc remoteproc2: powering up cdsp > > > > > pc : __pi_memcpy_generic+0x110/0x22c > > > > > lr : qcom_pas_tee_get_rsc_table+0xf4/0x25c > > > > > Call trace: > > > > > __pi_memcpy_generic+0x110/0x22c (P) > > > > > qcom_pas_get_rsc_table+0x38/0x60 > > > > > qcom_pas_parse_firmware+0xa0/0x100 > > > > > rproc_boot+0x2d4/0x380 > > > > > state_store+0x40/0x100 > > > > > > > > > > Fixes: a4584bff63c8 ("remoteproc: pas: Extend parse_fw callback to fetch resources via SMC call") > > > > > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com> > > > > > --- > > > > > drivers/remoteproc/qcom_q6v5_pas.c | 11 ++++++----- > > > > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > > > > > > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > > > > > index a005546c265d..e871e03ba794 100644 > > > > > --- a/drivers/remoteproc/qcom_q6v5_pas.c > > > > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > > > > > @@ -463,7 +463,7 @@ static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *f > > > > > struct resource_table *table = NULL; > > > > > size_t output_rt_size; > > > > > void *output_rt; > > > > > - size_t table_sz; > > > > > + size_t table_sz = 0; > > > > > int ret; > > > > > > > > > > ret = qcom_register_dump_segments(rproc, fw); > > > > > @@ -476,11 +476,12 @@ static int qcom_pas_parse_firmware(struct rproc *rproc, const struct firmware *f > > > > > return 0; > > > > > > > > > > ret = rproc_elf_load_rsc_table(rproc, fw); > > > > > - if (ret) > > > > > + if (ret) { > > > > > dev_dbg(&rproc->dev, "Failed to load resource table from firmware\n"); > > > > > - > > > > > - table = rproc->table_ptr; > > > > > - table_sz = rproc->table_sz; > > > > > + } else { > > > > > + table = rproc->table_ptr; > > > > > + table_sz = rproc->table_sz; > > > > > + } > > > > > > > > > > > > Earlier code was intentional please read the comment below.. /**...*/ > > > > > > > > > > AFAICS the comments say that the call might pass NULL and zero as input > > > resources. And this patch does exactly that - it doesnt allow NULL and > > > non-zero. > > > > > > did I miss something? > > > > No, you did not miss anything.. I just wanted to highlight that I > > did not keep else statement in my initial implementation > > intentionally and relied table_ptr and size will be NULL on > > failure of rproc_elf_load_rsc_table().. > > Looks like you still haven't understood the problem this patch is trying > to solve. It's the PIL restart sequence where the table_ptr is NULL and > size being passed as non-zero which is incorrect from caller point of > view. As per my understanding, you will only see this issue when you do a start, shutdown, and then a fresh start again. Normal remoteproc recovery is not broken? If it is indeed an issue during rproc_shutdown() where table_ptr is NULLed while table_sz is not NULLed, should that not be fixed in rproc_shutdown itself ? -- -Mukesh Ojha ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-10-13 10:06 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-05 12:54 [PATCH] remoteproc: qcom: pas: pass no resource table when the firmware has none Jorge Ramirez-Ortiz 2026-10-05 16:12 ` Mukesh Ojha 2026-10-05 21:01 ` Jorge Ramirez 2026-10-06 5:23 ` Harshal Dev 2026-10-06 6:54 ` Jorge Ramirez 2026-10-06 10:03 ` Mukesh Ojha 2026-10-10 11:25 ` Sumit Garg 2026-10-13 10:06 ` Mukesh Ojha
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.