remoteproc_virtio: optimize the remoteproc virtio transport layer #489

CV-Bowen · 2023-05-12T05:03:45Z

remoteproc_virtio: optimize the remoteproc virtio transport layer

Implement the rproc_virtio_create_virtqueues() and rproc_virtio_delete_virtqueues()
Becase item 1, modified the rpmsg_init_vdev_with_config() also

arnopo · 2023-05-12T08:29:28Z

Could you details what was the issue you are facing that needs to move the virtio_create_virtqueues to the transport layer?
that would help to understand your work

CV-Bowen · 2023-05-12T11:24:06Z

@arnopo Hi, we want to support different transport layer (remoteproc, mmio) for virtio drivers. After this PR, we can implement the mmio tranport layer base on OpenAmp.
I think this patch is also helpful for virtio-mmio framework in openamp/virtio-exp branch to support remoteproc transport layer.

lib/include/openamp/virtio.h

arnopo · 2023-05-15T08:32:37Z

@arnopo Hi, we want to support different transport layer (remoteproc, mmio) for virtio drivers. After this PR, we can implement the mmio tranport layer base on OpenAmp. I think this patch is also helpful for virtio-mmio framework in openamp/virtio-exp branch to support remoteproc transport layer.

I understand the objective., what is not clear to me as a first review is why you need to move virtio_create_virtqueues this should be independent from the transport layer, no?

lib/include/openamp/virtio.h

lib/remoteproc/remoteproc_virtio.c

CV-Bowen · 2023-05-15T10:27:05Z

@arnopo Hi, we want to support different transport layer (remoteproc, mmio) for virtio drivers. After this PR, we can implement the mmio tranport layer base on OpenAmp. I think this patch is also helpful for virtio-mmio framework in openamp/virtio-exp branch to support remoteproc transport layer.

I understand the objective., what is not clear to me as a first review is why you need to move virtio_create_virtqueues this should be independent from the transport layer, no?

I found only rpmsg_virtio_create_virtqueues call virtio_create_virtqueues in OpenAmp, so I delete virtio_create_virtqueues after moving it's implementation to rproc_virtio_create_virtqueues. I do not consider that some other codes use it, I will restore this api and call it directly in rproc_virtio_create_virtqueues().

CV-Bowen · 2023-06-04T09:49:48Z

@arnopo @edmooring
Commit 1: virtio: follow virtio 1.2 spec, add more virtio status and device id
Seems commit 1 can be an independent PR. I will upload it later, and this PR will be only for Commit 2.

lib/include/openamp/rpmsg_virtio.h

lib/include/openamp/virtio.h

arnopo · 2023-06-13T07:41:49Z

@CV-Bowen
seems that you remove the Commit 2: virtio: decoupling the transport layer and virtio device layer

Please notice also #494, we should find a common solution...

CV-Bowen · 2023-06-13T08:12:18Z

@arnopo Sorry, I has fixed this and I will look #494.

lib/virtio/virtio.c

xiaoxiang781216 · 2023-06-17T08:25:13Z

@CV-Bowen seems that you remove the Commit 2: virtio: decoupling the transport layer and virtio device layer

Please notice also #494, we should find a common solution...

@CV-Bowen 's change extract and generalize from #494, after this change get merged:

The new mmio transport addition don't need modify rest code in OpenAMP
All code manipulate virtio device can work with both transport(mmio v.s. remoteproc)

arnopo

This PR probably breaks a lot of implementations.
I haven't done a test, but I'm fairly convinced that it breaks all Zephyr and ST projects, for example.

One reason is that you introduce a dependency between remoteproc and remoteproc_virtio, using struct remoteproc
Another is that some projects don't use the remoteproc virtio at all but static vrings.

Could you also split the commit in 3 ( as described in your commit message) ?
This would help for the review...

lib/include/openamp/virtio.h

lib/remoteproc/remoteproc_virtio.c

lib/rpmsg/rpmsg_virtio.c

lib/include/openamp/rpmsg_virtio.h

CV-Bowen · 2023-10-16T14:46:26Z

All the comments should have been addressed. Could you take a look again? @arnopo

lib/remoteproc/remoteproc_virtio.c

arnopo

This PR seems Ok in its design but will require some tests to ensure that it does not introduce regression in legacy.
Please find some comments on code

lib/include/openamp/rpmsg_virtio.h

lib/remoteproc/remoteproc_virtio.c

lib/rpmsg/rpmsg_virtio.c

lib/include/openamp/rpmsg_virtio.h

lib/remoteproc/remoteproc_virtio.c

arnopo

LGTM, I approve it but will need few test before merging it

lib/remoteproc/remoteproc_virtio.c

xiaoxiang781216

LGTM.

edmooring

Looks good to go, but agree with @arnopo we need to test more. It would be nice if my comment about the comment was addressed, but it is not necessary.

lib/remoteproc/remoteproc_virtio.c

tnmysh · 2023-11-03T14:27:49Z

@arnopo, @edmooring If you are okay, we can merge the change.

arnopo · 2023-11-07T15:26:43Z

@CV-Bowen,
Please, could you rebase it to solve merge conflict, that i test it and merge it

CV-Bowen · 2023-11-08T09:02:15Z

@arnopo Rebased.

arnopo · 2023-11-10T13:01:48Z

tests Ok on my side
@edmooring , @tnmysh , could you make a short test on Xilinx platform for complementary test on remoteproc ?

github-actions · 2023-12-26T00:25:33Z

This pull request has been marked as a stale pull request because it has been open (more than) 45 days with no activity.

1. Implement the rproc_virtio_create_virtqueues() and rproc_virtio_delete_virtqueues(). 2. Because 1, also modified the rpmsg init. Signed-off-by: Bowen Wang <wangbowen6@xiaomi.com>

CV-Bowen · 2024-02-18T11:26:44Z

@arnopo Rebase to the last to fix the conflict. Could we merge this PR? There isn’t much time until release in April.

arnopo · 2024-02-19T09:53:25Z

Sorry @CV-Bowen, it go out of my radar,

@edmooring , @tnmysh , as requested could you confirm that it does not create regression on your platforms

CV-Bowen force-pushed the virtio-spec branch 2 times, most recently from ea14a9d to a94e0c6 Compare May 12, 2023 05:11

CV-Bowen changed the title ~~virtio: follow virtio 1.2 spec, add more virtio status and device~~ virtio: decoupling the transport layer and virtio device layer May 12, 2023

arnopo requested review from danmilea, edmooring, arnopo and wmamills May 12, 2023 07:26

edmooring reviewed May 12, 2023

View reviewed changes

lib/include/openamp/virtio.h Outdated Show resolved Hide resolved

CV-Bowen force-pushed the virtio-spec branch from d8ad056 to 8c60cb6 Compare May 15, 2023 06:12

arnopo requested changes May 15, 2023

View reviewed changes

CV-Bowen mentioned this pull request Jun 4, 2023

virtio: follow virtio 1.2 spec, add more virtio status and device id #493

Merged

arnopo reviewed Jun 5, 2023

View reviewed changes

lib/include/openamp/rpmsg_virtio.h Outdated Show resolved Hide resolved

lib/include/openamp/virtio.h Outdated Show resolved Hide resolved

CV-Bowen force-pushed the virtio-spec branch from 8c60cb6 to a41e87b Compare June 13, 2023 03:21

CV-Bowen force-pushed the virtio-spec branch 2 times, most recently from b9e7b3a to cda0080 Compare June 13, 2023 08:11

xiaoxiang781216 reviewed Jun 13, 2023

View reviewed changes

lib/virtio/virtio.c Outdated Show resolved Hide resolved

CV-Bowen force-pushed the virtio-spec branch 3 times, most recently from 8617243 to 4161017 Compare June 17, 2023 09:01

arnopo requested changes Jun 19, 2023

View reviewed changes

arnopo mentioned this pull request Jun 22, 2023

Virtio native support for lib openamp #494

Merged

arnopo linked an issue Jun 22, 2023 that may be closed by this pull request

what's plan to support the service/driver at the virtio level? #390

Closed

CV-Bowen changed the title ~~virtio: decoupling the transport layer and virtio device layer~~ remoteproc_virtio: optimize the remoteproc virtio transport layer Oct 16, 2023

CV-Bowen force-pushed the virtio-spec branch 2 times, most recently from 8177549 to f7e1a49 Compare October 16, 2023 14:45

xiaoxiang781216 reviewed Oct 16, 2023

View reviewed changes

lib/remoteproc/remoteproc_virtio.c Outdated Show resolved Hide resolved

CV-Bowen force-pushed the virtio-spec branch from f7e1a49 to f772477 Compare October 17, 2023 01:51

arnopo requested changes Oct 18, 2023

View reviewed changes

CV-Bowen force-pushed the virtio-spec branch 2 times, most recently from a744b07 to 487fd33 Compare October 19, 2023 13:15

arnopo approved these changes Oct 20, 2023

View reviewed changes

lib/remoteproc/remoteproc_virtio.c Show resolved Hide resolved

arnopo requested review from edmooring and xiaoxiang781216 October 20, 2023 16:44

xiaoxiang781216 approved these changes Oct 20, 2023

View reviewed changes

edmooring approved these changes Oct 21, 2023

View reviewed changes

lib/remoteproc/remoteproc_virtio.c Outdated Show resolved Hide resolved

arnopo added this to the Release V2024.04 milestone Oct 24, 2023

CV-Bowen force-pushed the virtio-spec branch from 487fd33 to 9d89f95 Compare October 26, 2023 09:01

CV-Bowen force-pushed the virtio-spec branch from 9d89f95 to 741c59a Compare November 8, 2023 09:01

github-actions bot added the Stale label Dec 26, 2023

remoteproc_virtio: optimize the remoteproc virtio transport layer

f81e22f

1. Implement the rproc_virtio_create_virtqueues() and rproc_virtio_delete_virtqueues(). 2. Because 1, also modified the rpmsg init. Signed-off-by: Bowen Wang <wangbowen6@xiaomi.com>

CV-Bowen force-pushed the virtio-spec branch from 741c59a to f81e22f Compare February 18, 2024 11:22

github-actions bot removed the Stale label Feb 19, 2024

tnmysh approved these changes Feb 22, 2024

View reviewed changes

arnopo merged commit 0c7e420 into OpenAMP:main Feb 23, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remoteproc_virtio: optimize the remoteproc virtio transport layer #489

remoteproc_virtio: optimize the remoteproc virtio transport layer #489

CV-Bowen commented May 12, 2023 •

edited

arnopo commented May 12, 2023

CV-Bowen commented May 12, 2023

arnopo commented May 15, 2023

CV-Bowen commented May 15, 2023

CV-Bowen commented Jun 4, 2023

arnopo commented Jun 13, 2023

CV-Bowen commented Jun 13, 2023

xiaoxiang781216 commented Jun 17, 2023

arnopo left a comment

CV-Bowen commented Oct 16, 2023

arnopo left a comment

arnopo left a comment

xiaoxiang781216 left a comment

edmooring left a comment

tnmysh commented Nov 3, 2023

arnopo commented Nov 7, 2023

CV-Bowen commented Nov 8, 2023

arnopo commented Nov 10, 2023

github-actions bot commented Dec 26, 2023

CV-Bowen commented Feb 18, 2024

arnopo commented Feb 19, 2024 •

edited

remoteproc_virtio: optimize the remoteproc virtio transport layer #489

remoteproc_virtio: optimize the remoteproc virtio transport layer #489

Conversation

CV-Bowen commented May 12, 2023 • edited

arnopo commented May 12, 2023

CV-Bowen commented May 12, 2023

arnopo commented May 15, 2023

CV-Bowen commented May 15, 2023

CV-Bowen commented Jun 4, 2023

arnopo commented Jun 13, 2023

CV-Bowen commented Jun 13, 2023

xiaoxiang781216 commented Jun 17, 2023

arnopo left a comment

Choose a reason for hiding this comment

CV-Bowen commented Oct 16, 2023

arnopo left a comment

Choose a reason for hiding this comment

arnopo left a comment

Choose a reason for hiding this comment

xiaoxiang781216 left a comment

Choose a reason for hiding this comment

edmooring left a comment

Choose a reason for hiding this comment

tnmysh commented Nov 3, 2023

arnopo commented Nov 7, 2023

CV-Bowen commented Nov 8, 2023

arnopo commented Nov 10, 2023

github-actions bot commented Dec 26, 2023

CV-Bowen commented Feb 18, 2024

arnopo commented Feb 19, 2024 • edited

CV-Bowen commented May 12, 2023 •

edited

arnopo commented Feb 19, 2024 •

edited