On Thu, Jan 29, 2026 at 06:02:21PM +0200, Daniel Baluta wrote: > On Thu, Jan 29, 2026 at 3:45 AM Peng Fan (OSS) <[email protected]> wrote: > > > > From: Peng Fan <[email protected]> > > > > imx_rproc_elf_find_loaded_rsc_table() may incorrectly report a loaded > > resource table even when the current firmware does not provide one. > > > > When the device tree contains a "rsc-table" entry, priv->rsc_table is > > non-NULL and denotes where a resource table would be located if one is > > present in memory. However, when the current firmware has no resource > > table, rproc->table_ptr is NULL. The function still returns > > priv->rsc_table, and the remoteproc core interprets this as a valid loaded > > resource table. > > > > Fix this by returning NULL from imx_rproc_elf_find_loaded_rsc_table() when > > there is no resource table for the current firmware (i.e. when > > rproc->table_ptr is NULL). This aligns the function's semantics with the > > remoteproc core: a loaded resource table is only reported when a valid > > table_ptr exists. > > > > With this change, starting firmware without a resource table no longer > > triggers a crash. > > > > Fixes: e954a1bd1610 ("remoteproc: imx_rproc: Use imx specific hook for > > find_loaded_rsc_table") > > Cc: [email protected] > > Signed-off-by: Peng Fan <[email protected]> > > Changes looks good to me > > > > --- a/drivers/remoteproc/imx_rproc.c > > +++ b/drivers/remoteproc/imx_rproc.c > > @@ -729,6 +729,10 @@ imx_rproc_elf_find_loaded_rsc_table(struct rproc > > *rproc, const struct firmware * > > { > > struct imx_rproc *priv = rproc->priv; > > > > + /* No resource table in the firmware */ > > + if (!rproc->table_ptr) > > + return NULL; > > I wonder if we can make this change generic because it should happen > on other platforms also. > > Maybe something like this: > > remoteproc: core: Only copy loaded table when valid > > Copy resource table in memory only when: > * the current loaded firmware provides one > AND > * there is an explicit request to have the rsc table copied in memory > via rsc-table > > --- a/drivers/remoteproc/remoteproc_core.c > +++ b/drivers/remoteproc/remoteproc_core.c > @@ -1281,7 +1281,7 @@ static int rproc_start(struct rproc *rproc, > const struct firmware *fw) > * that any subsequent changes will be applied to the loaded version. > */ > loaded_table = rproc_find_loaded_rsc_table(rproc, fw); > - if (loaded_table) { > + if (rproc->cached_table && loaded_table) {
But we would be doing the check for rproc->table_ptr twice (->table_ptr and ->cached_table should be the same). The way it is currently writting forces vendor specific implementation of rproc_elf_find_loaded_rsc_table() to do the right thing. The merge window has been pushed by a week, giving me an opportunity to merge this patch. Should I do that or should we continue discussing the best approach? > memcpy(loaded_table, rproc->cached_table, rproc->table_sz); > rproc->table_ptr = loaded_table; > }
