Re: [PATCH v6 0/4] Add a vhost RPMsg API

[Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>]

Hi Arnaud,

On Tue, Sep 15, 2020 at 02:13:23PM +0200, Arnaud POULIQUEN wrote:
> Hi  Guennadi,
> 
> On 9/1/20 5:11 PM, Guennadi Liakhovetski wrote:
> > Hi,
> > 
> > Next update:
> > 
> > v6:
> > - rename include/linux/virtio_rpmsg.h -> include/linux/rpmsg/virtio.h
> > 
> > v5:
> > - don't hard-code message layout
> > 
> > v4:
> > - add endianness conversions to comply with the VirtIO standard
> > 
> > v3:
> > - address several checkpatch warnings
> > - address comments from Mathieu Poirier
> > 
> > v2:
> > - update patch #5 with a correct vhost_dev_init() prototype
> > - drop patch #6 - it depends on a different patch, that is currently
> >   an RFC
> > - address comments from Pierre-Louis Bossart:
> >   * remove "default n" from Kconfig
> > 
> > Linux supports RPMsg over VirtIO for "remote processor" / AMP use
> > cases. It can however also be used for virtualisation scenarios,
> > e.g. when using KVM to run Linux on both the host and the guests.
> > This patch set adds a wrapper API to facilitate writing vhost
> > drivers for such RPMsg-based solutions. The first use case is an
> > audio DSP virtualisation project, currently under development, ready
> > for review and submission, available at
> > https://github.com/thesofproject/linux/pull/1501/commits
> 
> Mathieu pointed me your series. On my side i proposed the rpmsg_ns_msg
> service[1] that does not match with your implementation.
> As i come late, i hope that i did not miss something in the history...
> Don't hesitate to point me the discussions, if it is the case.

Well, as you see, this is a v6 only of this patch set, and apart from it 
there have been several side discussions and patch sets.

> Regarding your patchset, it is quite confusing for me. It seems that you
> implement your own protocol on top of vhost forked from the RPMsg one.
> But look to me that it is not the RPMsg protocol.

I'm implementing a counterpart to the rpmsg protocol over VirtIO as 
initially implemented by drivers/rpmsg/virtio_rpmsg_bus.c for the "main 
CPU" (in case of remoteproc over VirtIO) or the guest side in case of 
Linux virtualisation. Since my implementation can talk to that driver, 
I don't think, that I'm inventing a new protocol. I'm adding support 
for the same protocol for the opposite side of the VirtIO divide.

> So i would be agree with Vincent[2] which proposed to switch on a RPMsg API
> and creating a vhost rpmsg device. This is also proposed in the 
> "Enhance VHOST to enable SoC-to-SoC communication" RFC[3].
> Do you think that this alternative could match with your need?

As I replied to Vincent, I understand his proposal and the approach taken 
in the series [3], but I'm not sure I agree, that adding yet another 
virtual device / driver layer on the vhost side is a good idea. As far as 
I understand adding new completely virtual devices isn't considered to be 
a good practice in the kernel. Currently vhost is just a passive "library" 
and my vhost-rpmsg support keeps it that way. Not sure I'm in favour of 
converting vhost to a virtual device infrastructure.

Thanks for pointing me out at [3], I should have a better look at it.

Thanks
Guennadi

> [1]. https://patchwork.kernel.org/project/linux-remoteproc/list/?series=338335 
> [2]. https://www.spinics.net/lists/linux-virtualization/msg44195.html
> [3]. https://www.spinics.net/lists/linux-remoteproc/msg06634.html  
> 
> Thanks,
> Arnaud
> 
> > 
> > Thanks
> > Guennadi
> > 
> > Guennadi Liakhovetski (4):
> >   vhost: convert VHOST_VSOCK_SET_RUNNING to a generic ioctl
> >   rpmsg: move common structures and defines to headers
> >   rpmsg: update documentation
> >   vhost: add an RPMsg API
> > 
> >  Documentation/rpmsg.txt          |   6 +-
> >  drivers/rpmsg/virtio_rpmsg_bus.c |  78 +------
> >  drivers/vhost/Kconfig            |   7 +
> >  drivers/vhost/Makefile           |   3 +
> >  drivers/vhost/rpmsg.c            | 373 +++++++++++++++++++++++++++++++
> >  drivers/vhost/vhost_rpmsg.h      |  74 ++++++
> >  include/linux/rpmsg/virtio.h     |  83 +++++++
> >  include/uapi/linux/rpmsg.h       |   3 +
> >  include/uapi/linux/vhost.h       |   4 +-
> >  9 files changed, 551 insertions(+), 80 deletions(-)
> >  create mode 100644 drivers/vhost/rpmsg.c
> >  create mode 100644 drivers/vhost/vhost_rpmsg.h
> >  create mode 100644 include/linux/rpmsg/virtio.h
> > 
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization