Hi Guennadi, > -----Original Message----- > From: Guennadi Liakhovetski <guennadi.liakhovetski@xxxxxxxxxxxxxxx> > Sent: jeudi 17 septembre 2020 07:47 > To: Arnaud POULIQUEN <arnaud.pouliquen@xxxxxx> > Cc: kvm@xxxxxxxxxxxxxxx; linux-remoteproc@xxxxxxxxxxxxxxx; > virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx; sound-open-firmware@alsa- > project.org; Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>; Liam > Girdwood <liam.r.girdwood@xxxxxxxxxxxxxxx>; Michael S. Tsirkin > <mst@xxxxxxxxxx>; Jason Wang <jasowang@xxxxxxxxxx>; Ohad Ben-Cohen > <ohad@xxxxxxxxxx>; Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>; Mathieu > Poirier <mathieu.poirier@xxxxxxxxxx>; Vincent Whitchurch > <vincent.whitchurch@xxxxxxxx> > Subject: Re: [PATCH v6 0/4] Add a vhost RPMsg API > > 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. The main point I would like to highlight here is related to the use of the name "RPMsg" more than how you implement your IPC protocol. If It is a counterpart, it probably does not respect interface for RPMsg clients. A good way to answer this, might be to respond to this question: Is the rpmsg sample client[4] can be used on top of your vhost RPMsg implementation? If the response is no, describe it as a RPMsg implementation could lead to confusion... [4] https://elixir.bootlin.com/linux/v5.9-rc5/source/samples/rpmsg/rpmsg_client_sample.c Regards, Arnaud > > > 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=338 > > 335 [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 > > >
