Re: [PATCH] remoteproc: qcom: pas: pass no resource table when the firmware has none

[Konrad Dybcio]

On 10/5/26 2:54 PM, 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@xxxxxxxxxxxxxxxx>
> ---

To my understanding, this patch is right

Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>

As an additional safeguard, we should probably drop the stale table
size whenever the pointer is reset to null:

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@
 clean_up_resources:
        rproc_resource_cleanup(rproc);
        kfree(rproc->cached_table);
        rproc->cached_table = NULL;
        rproc->table_ptr = NULL;
+       rproc->table_sz = 0;
 unprepare_rproc:
@@
        kfree(rproc->cached_table);
        rproc->cached_table = NULL;
        rproc->table_ptr = NULL;
+       rproc->table_sz = 0;

        return rproc_attach(rproc);
@@
        kfree(rproc->cached_table);
        rproc->cached_table = NULL;
        rproc->table_ptr = NULL;
+       rproc->table_sz = 0;
 out:
@@
        kfree(rproc->cached_table);
        rproc->cached_table = NULL;
        rproc->table_ptr = NULL;
+       rproc->table_sz = 0;
 out:


Konrad