Hello,
Please find my comments below:
On 10/16/25 10:12 AM, Mathieu Poirier wrote:
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?
Here, "third-party" could be a bootloader or another core in a
heterogeneous system. In my-case it is a platform management controller.
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.
As per my understanding, rproc_reset_rsc_table_on_detach() will setup
clean resource table, that sets vring addresses to 0xffffffff. Please
let me know if this understanding is not correct.
If we do not, call rproc_attach(), then correct vring addresses are not
setup in the resource table for next attach to work. Because,
rproc_handle_resources() and rproc_alloc_registered_carveouts() are not
called as part __rproc_attach().
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.
I agree to this. I can find a way to call rproc_detach() and
rproc_attach() sequentially, instead of __rproc_detach() and
rproc_attach() calls. I might have to remove
rproc_trigger_attach_recovery completely, but that is implementation
details. We can work it out later, once we agree to the current problem
& solution.
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?
The main motivation is to make the attach operation works during
attach_recovery(). The __rproc_detach() works as expected, but attach
doesn't work. After recovery, I am not able to strat RPMsg communication.
Please let me know if I am missing something.
Thanks,
Tanmay
Thanks,
Mathieu
Signed-off-by: Tanmay Shah <[email protected]>
---
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
