Re: [RFC PATCH] remoteproc: core: full attach detach during recovery

[Mathieu Poirier <mathieu.poirier@linaro.org>]

Good morning,

On Thu, Oct 02, 2025 at 08:33:46AM -0700, Tanmay Shah wrote:
> Current recovery operation does only virtio device reset, but do not
> free and re-allocate all the resources. As third-party is booting the
> remote processor during attach-detach, it is better to free and
> re-allocate resoruces as resource table state might be unknown to linux
> when remote processor boots and reports crash.

1) When referring to "third-party", should I assume boot loader?
2) Function rproc_attach_recovery() calls __rproc_detach(), which in turn calls
rproc_reset_rsc_table_on_detach().  That function deals explicitly with the
resource table.
3) The code in this patch mixes __rproc_detach() with rproc_attach(), something
that is likely not a good idea.  We either do __rproc_detach/__rproc_attach or
rproc_detach/rproc_attach but I'd like to avoid the mix-and-match to keep the
amount of possible states to a minimum.

If I understand correctly, the main motivation for this patch is the management
of the resource table.  But as noted in (2), this should be taken care of.  Am I
missing some information?

Thanks,
Mathieu

> 
> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> ---
> 
> Note: RFC patch for design discussion. Please do not merge. 
> 
>  drivers/remoteproc/remoteproc_core.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 825672100528..4971508bc5b2 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1786,7 +1786,20 @@ static int rproc_attach_recovery(struct rproc *rproc)
>  	if (ret)
>  		return ret;
>  
> -	return __rproc_attach(rproc);
> +	/* clean up all acquired resources */
> +	rproc_resource_cleanup(rproc);
> +
> +	/* release HW resources if needed */
> +	rproc_unprepare_device(rproc);
> +
> +	rproc_disable_iommu(rproc);
> +
> +	/* Free the copy of the resource table */
> +	kfree(rproc->cached_table);
> +	rproc->cached_table = NULL;
> +	rproc->table_ptr = NULL;
> +
> +	return rproc_attach(rproc);
>  }
>  
>  static int rproc_boot_recovery(struct rproc *rproc)
> 
> base-commit: 56d030ea3330ab737fe6c05f89d52f56208b07ac
> -- 
> 2.34.1
>