docs/interop/vhost-user.rst | 58 ++++++ hw/virtio/vhost-user-base.c | 39 +++- hw/virtio/vhost-user-device-pci.c | 37 +++- hw/virtio/vhost-user.c | 221 ++++++++++++++++++++++ hw/virtio/virtio.c | 12 ++ include/hw/virtio/vhost-backend.h | 6 + include/hw/virtio/vhost-user.h | 1 + include/hw/virtio/virtio.h | 5 + subprojects/libvhost-user/libvhost-user.c | 149 +++++++++++++++ subprojects/libvhost-user/libvhost-user.h | 91 +++++++++ 10 files changed, 614 insertions(+), 5 deletions(-)
Hi all, v1->v2: - Corrected typos and clarifications from first review - Added SHMEM_CONFIG frontend request to query VIRTIO shared memory regions from backends - vhost-user-device to use SHMEM_CONFIG to request and initialise regions - Added MEM_READ/WRITE backend requests in case address translation fails accessing VIRTIO Shared Memory Regions with MMAPs This is an update of my attempt to have backends support dynamic fd mapping into VIRTIO Shared Memory Regions. After the first review I have added more commits and new messages to the vhost-user protocol. However, I still have some doubts as to how will this work, specially regarding the MEM_READ and MEM_WRITE commands. Thus, I am still looking for feedback, to ensure that I am going in the right direction with the implementation. The usecase for this patch is, e.g., to support vhost-user-gpu RESOURCE_BLOB operations, or DAX Window request for virtio-fs. In general, any operation where a backend need to request the frontend to mmap an fd into a VIRTIO Shared Memory Region, so that the guest can then access it. After receiving the SHMEM_MAP/UNMAP request, the frontend will perform the mmap with the instructed parameters (i.e., shmid, shm_offset, fd_offset, fd, lenght). As there are already a couple devices that could benefit of such a feature, and more could require it in the future, the goal is to make the implementation generic. To that end, the VIRTIO Shared Memory Region list is declared in the `VirtIODevice` struct. This patch also includes: SHMEM_CONFIG frontend request that is specifically meant to allow generic vhost-user-device frontend to be able to query VIRTIO Shared Memory settings from the backend (as this device is generic and agnostic of the actual backend configuration). Finally, MEM_READ/WRITE backend requests are added to deal with a potential issue when having any backend sharing a descriptor that references a mapping to another backend. The first backend will not be able to see these mappings. So these requests are a fallback for vhost-user memory translation fails. Albert Esteve (5): vhost-user: Add VIRTIO Shared Memory map request vhost_user: Add frontend command for shmem config vhost-user-dev: Add cache BAR vhost_user: Add MEM_READ/WRITE backend requests vhost_user: Implement mem_read/mem_write handlers docs/interop/vhost-user.rst | 58 ++++++ hw/virtio/vhost-user-base.c | 39 +++- hw/virtio/vhost-user-device-pci.c | 37 +++- hw/virtio/vhost-user.c | 221 ++++++++++++++++++++++ hw/virtio/virtio.c | 12 ++ include/hw/virtio/vhost-backend.h | 6 + include/hw/virtio/vhost-user.h | 1 + include/hw/virtio/virtio.h | 5 + subprojects/libvhost-user/libvhost-user.c | 149 +++++++++++++++ subprojects/libvhost-user/libvhost-user.h | 91 +++++++++ 10 files changed, 614 insertions(+), 5 deletions(-) -- 2.45.2
On Fri, Jun 28, 2024 at 04:57:05PM +0200, Albert Esteve wrote: > Hi all, > > v1->v2: > - Corrected typos and clarifications from > first review > - Added SHMEM_CONFIG frontend request to > query VIRTIO shared memory regions from > backends > - vhost-user-device to use SHMEM_CONFIG > to request and initialise regions > - Added MEM_READ/WRITE backend requests > in case address translation fails > accessing VIRTIO Shared Memory Regions > with MMAPs Hi Albert, I will be offline next week. I've posted comments. I think the hard part will be adjusting vhost-user backend code to make use of MEM_READ/MEM_WRITE when address translation fails. Ideally every guest memory access (including vring accesses) should fall back to MEM_READ/MEM_WRITE. A good test for MEM_READ/MEM_WRITE is to completely skip setting the memory table from the frontend and fall back for every guest memory access. If the vhost-user backend still works without a memory table then you know MEM_READ/MEM_WRITE is working too. The vhost-user spec should probably contain a comment explaining that MEM_READ/MEM_WRITE may be necessary when other device backends use SHMEM_MAP due to the incomplete memory table that prevents translating those memory addresses. In other words, if the guest has a device that uses SHMEM_MAP, then all other vhost-user devices should support MEM_READ/MEM_WRITE in order to ensure that DMA works with Shared Memory Regions. Stefan > > This is an update of my attempt to have > backends support dynamic fd mapping into VIRTIO > Shared Memory Regions. After the first review > I have added more commits and new messages > to the vhost-user protocol. > However, I still have some doubts as to > how will this work, specially regarding > the MEM_READ and MEM_WRITE commands. > Thus, I am still looking for feedback, > to ensure that I am going in the right > direction with the implementation. > > The usecase for this patch is, e.g., to support > vhost-user-gpu RESOURCE_BLOB operations, > or DAX Window request for virtio-fs. In > general, any operation where a backend > need to request the frontend to mmap an > fd into a VIRTIO Shared Memory Region, > so that the guest can then access it. > > After receiving the SHMEM_MAP/UNMAP request, > the frontend will perform the mmap with the > instructed parameters (i.e., shmid, shm_offset, > fd_offset, fd, lenght). > > As there are already a couple devices > that could benefit of such a feature, > and more could require it in the future, > the goal is to make the implementation > generic. > > To that end, the VIRTIO Shared Memory > Region list is declared in the `VirtIODevice` > struct. > > This patch also includes: > SHMEM_CONFIG frontend request that is > specifically meant to allow generic > vhost-user-device frontend to be able to > query VIRTIO Shared Memory settings from the > backend (as this device is generic and agnostic > of the actual backend configuration). > > Finally, MEM_READ/WRITE backend requests are > added to deal with a potential issue when having > any backend sharing a descriptor that references > a mapping to another backend. The first > backend will not be able to see these > mappings. So these requests are a fallback > for vhost-user memory translation fails. > > Albert Esteve (5): > vhost-user: Add VIRTIO Shared Memory map request > vhost_user: Add frontend command for shmem config > vhost-user-dev: Add cache BAR > vhost_user: Add MEM_READ/WRITE backend requests > vhost_user: Implement mem_read/mem_write handlers > > docs/interop/vhost-user.rst | 58 ++++++ > hw/virtio/vhost-user-base.c | 39 +++- > hw/virtio/vhost-user-device-pci.c | 37 +++- > hw/virtio/vhost-user.c | 221 ++++++++++++++++++++++ > hw/virtio/virtio.c | 12 ++ > include/hw/virtio/vhost-backend.h | 6 + > include/hw/virtio/vhost-user.h | 1 + > include/hw/virtio/virtio.h | 5 + > subprojects/libvhost-user/libvhost-user.c | 149 +++++++++++++++ > subprojects/libvhost-user/libvhost-user.h | 91 +++++++++ > 10 files changed, 614 insertions(+), 5 deletions(-) > > -- > 2.45.2 >
Adding David Stevens, who implemented SHMEM_MAP and SHMEM_UNMAP in crosvm a couple of years ago. David, I'd be particularly interested for your thoughts on the MEM_READ and MEM_WRITE commands, since as far as I know crosvm doesn't implement anything like that. The discussion leading to those being added starts here: https://lore.kernel.org/qemu-devel/20240604185416.GB90471@fedora.redhat.com/ It would be great if this could be standardised between QEMU and crosvm (and therefore have a clearer path toward being implemented in other VMMs)! Albert Esteve <aesteve@redhat.com> writes: > Hi all, > > v1->v2: > - Corrected typos and clarifications from > first review > - Added SHMEM_CONFIG frontend request to > query VIRTIO shared memory regions from > backends > - vhost-user-device to use SHMEM_CONFIG > to request and initialise regions > - Added MEM_READ/WRITE backend requests > in case address translation fails > accessing VIRTIO Shared Memory Regions > with MMAPs > > This is an update of my attempt to have > backends support dynamic fd mapping into VIRTIO > Shared Memory Regions. After the first review > I have added more commits and new messages > to the vhost-user protocol. > However, I still have some doubts as to > how will this work, specially regarding > the MEM_READ and MEM_WRITE commands. > Thus, I am still looking for feedback, > to ensure that I am going in the right > direction with the implementation. > > The usecase for this patch is, e.g., to support > vhost-user-gpu RESOURCE_BLOB operations, > or DAX Window request for virtio-fs. In > general, any operation where a backend > need to request the frontend to mmap an > fd into a VIRTIO Shared Memory Region, > so that the guest can then access it. > > After receiving the SHMEM_MAP/UNMAP request, > the frontend will perform the mmap with the > instructed parameters (i.e., shmid, shm_offset, > fd_offset, fd, lenght). > > As there are already a couple devices > that could benefit of such a feature, > and more could require it in the future, > the goal is to make the implementation > generic. > > To that end, the VIRTIO Shared Memory > Region list is declared in the `VirtIODevice` > struct. > > This patch also includes: > SHMEM_CONFIG frontend request that is > specifically meant to allow generic > vhost-user-device frontend to be able to > query VIRTIO Shared Memory settings from the > backend (as this device is generic and agnostic > of the actual backend configuration). > > Finally, MEM_READ/WRITE backend requests are > added to deal with a potential issue when having > any backend sharing a descriptor that references > a mapping to another backend. The first > backend will not be able to see these > mappings. So these requests are a fallback > for vhost-user memory translation fails. > > Albert Esteve (5): > vhost-user: Add VIRTIO Shared Memory map request > vhost_user: Add frontend command for shmem config > vhost-user-dev: Add cache BAR > vhost_user: Add MEM_READ/WRITE backend requests > vhost_user: Implement mem_read/mem_write handlers > > docs/interop/vhost-user.rst | 58 ++++++ > hw/virtio/vhost-user-base.c | 39 +++- > hw/virtio/vhost-user-device-pci.c | 37 +++- > hw/virtio/vhost-user.c | 221 ++++++++++++++++++++++ > hw/virtio/virtio.c | 12 ++ > include/hw/virtio/vhost-backend.h | 6 + > include/hw/virtio/vhost-user.h | 1 + > include/hw/virtio/virtio.h | 5 + > subprojects/libvhost-user/libvhost-user.c | 149 +++++++++++++++ > subprojects/libvhost-user/libvhost-user.h | 91 +++++++++ > 10 files changed, 614 insertions(+), 5 deletions(-) > > -- > 2.45.2
On Thu, Jul 11, 2024 at 7:56 PM Alyssa Ross <hi@alyssa.is> wrote: > > Adding David Stevens, who implemented SHMEM_MAP and SHMEM_UNMAP in > crosvm a couple of years ago. > > David, I'd be particularly interested for your thoughts on the MEM_READ > and MEM_WRITE commands, since as far as I know crosvm doesn't implement > anything like that. The discussion leading to those being added starts > here: > > https://lore.kernel.org/qemu-devel/20240604185416.GB90471@fedora.redhat.com/ > > It would be great if this could be standardised between QEMU and crosvm > (and therefore have a clearer path toward being implemented in other VMMs)! Setting aside vhost-user for a moment, the DAX example given by Stefan won't work in crosvm today. Is universal access to virtio shared memory regions actually mandated by the virtio spec? Copying from virtiofs DAX to virtiofs sharing seems reasonable enough, but what about virtio-pmem to virtio-blk? What about screenshotting a framebuffer in virtio-gpu shared memory to virtio-scsi? I guess with some plumbing in the VMM, it's solvable in a virtualized environment. But what about when you have real hardware that speaks virtio involved? That's outside my wheelhouse, but it doesn't seem like that would be easy to solve. For what it's worth, my interpretation of the target scenario: > Other backends don't see these mappings. If the guest submits a vring > descriptor referencing a mapping to another backend, then that backend > won't be able to access this memory is that it's omitting how the implementation is reconciled with section 2.10.1 of v1.3 of the virtio spec, which states that: > References into shared memory regions are represented as offsets from > the beginning of the region instead of absolute memory addresses. Offsets > are used both for references between structures stored within shared > memory and for requests placed in virtqueues that refer to shared memory. My interpretation of that statement is that putting raw guest physical addresses corresponding to virtio shared memory regions into a vring is a driver spec violation. -David
On Fri, Jul 12, 2024 at 11:06:49AM +0900, David Stevens wrote: > On Thu, Jul 11, 2024 at 7:56 PM Alyssa Ross <hi@alyssa.is> wrote: > > > > Adding David Stevens, who implemented SHMEM_MAP and SHMEM_UNMAP in > > crosvm a couple of years ago. > > > > David, I'd be particularly interested for your thoughts on the MEM_READ > > and MEM_WRITE commands, since as far as I know crosvm doesn't implement > > anything like that. The discussion leading to those being added starts > > here: > > > > https://lore.kernel.org/qemu-devel/20240604185416.GB90471@fedora.redhat.com/ > > > > It would be great if this could be standardised between QEMU and crosvm > > (and therefore have a clearer path toward being implemented in other VMMs)! > > Setting aside vhost-user for a moment, the DAX example given by Stefan > won't work in crosvm today. > > Is universal access to virtio shared memory regions actually mandated > by the virtio spec? Copying from virtiofs DAX to virtiofs sharing > seems reasonable enough, but what about virtio-pmem to virtio-blk? > What about screenshotting a framebuffer in virtio-gpu shared memory to > virtio-scsi? I guess with some plumbing in the VMM, it's solvable in a > virtualized environment. But what about when you have real hardware > that speaks virtio involved? That's outside my wheelhouse, but it > doesn't seem like that would be easy to solve. Yes, it can work for physical devices if allowed by host configuration. E.g. VFIO supports that I think. Don't think VDPA does. > For what it's worth, my interpretation of the target scenario: > > > Other backends don't see these mappings. If the guest submits a vring > > descriptor referencing a mapping to another backend, then that backend > > won't be able to access this memory > > is that it's omitting how the implementation is reconciled with > section 2.10.1 of v1.3 of the virtio spec, which states that: > > > References into shared memory regions are represented as offsets from > > the beginning of the region instead of absolute memory addresses. Offsets > > are used both for references between structures stored within shared > > memory and for requests placed in virtqueues that refer to shared memory. > > My interpretation of that statement is that putting raw guest physical > addresses corresponding to virtio shared memory regions into a vring > is a driver spec violation. > > -David This really applies within device I think. Should be clarified ... -- MST
On Fri, Jul 12, 2024 at 2:47 PM Michael S. Tsirkin <mst@redhat.com> wrote: > > On Fri, Jul 12, 2024 at 11:06:49AM +0900, David Stevens wrote: > > On Thu, Jul 11, 2024 at 7:56 PM Alyssa Ross <hi@alyssa.is> wrote: > > > > > > Adding David Stevens, who implemented SHMEM_MAP and SHMEM_UNMAP in > > > crosvm a couple of years ago. > > > > > > David, I'd be particularly interested for your thoughts on the MEM_READ > > > and MEM_WRITE commands, since as far as I know crosvm doesn't implement > > > anything like that. The discussion leading to those being added starts > > > here: > > > > > > https://lore.kernel.org/qemu-devel/20240604185416.GB90471@fedora.redhat.com/ > > > > > > It would be great if this could be standardised between QEMU and crosvm > > > (and therefore have a clearer path toward being implemented in other VMMs)! > > > > Setting aside vhost-user for a moment, the DAX example given by Stefan > > won't work in crosvm today. > > > > Is universal access to virtio shared memory regions actually mandated > > by the virtio spec? Copying from virtiofs DAX to virtiofs sharing > > seems reasonable enough, but what about virtio-pmem to virtio-blk? > > What about screenshotting a framebuffer in virtio-gpu shared memory to > > virtio-scsi? I guess with some plumbing in the VMM, it's solvable in a > > virtualized environment. But what about when you have real hardware > > that speaks virtio involved? That's outside my wheelhouse, but it > > doesn't seem like that would be easy to solve. > > Yes, it can work for physical devices if allowed by host configuration. > E.g. VFIO supports that I think. Don't think VDPA does. I'm sure it can work, but that sounds more like a SHOULD (MAY?), rather than a MUST. > > For what it's worth, my interpretation of the target scenario: > > > > > Other backends don't see these mappings. If the guest submits a vring > > > descriptor referencing a mapping to another backend, then that backend > > > won't be able to access this memory > > > > is that it's omitting how the implementation is reconciled with > > section 2.10.1 of v1.3 of the virtio spec, which states that: > > > > > References into shared memory regions are represented as offsets from > > > the beginning of the region instead of absolute memory addresses. Offsets > > > are used both for references between structures stored within shared > > > memory and for requests placed in virtqueues that refer to shared memory. > > > > My interpretation of that statement is that putting raw guest physical > > addresses corresponding to virtio shared memory regions into a vring > > is a driver spec violation. > > > > -David > > This really applies within device I think. Should be clarified ... You mean that a virtio device can use absolute memory addresses for other devices' shared memory regions, but it can't use absolute memory addresses for its own shared memory regions? That's a rather strange requirement. Or is the statement simply giving an addressing strategy that device type specifications are free to ignore? -David
On Fri, Jul 12, 2024 at 1:48 PM Michael S. Tsirkin <mst@redhat.com> wrote: > > On Fri, Jul 12, 2024 at 11:06:49AM +0900, David Stevens wrote: > > On Thu, Jul 11, 2024 at 7:56 PM Alyssa Ross <hi@alyssa.is> wrote: > > > > > > Adding David Stevens, who implemented SHMEM_MAP and SHMEM_UNMAP in > > > crosvm a couple of years ago. > > > > > > David, I'd be particularly interested for your thoughts on the MEM_READ > > > and MEM_WRITE commands, since as far as I know crosvm doesn't implement > > > anything like that. The discussion leading to those being added starts > > > here: > > > > > > https://lore.kernel.org/qemu-devel/20240604185416.GB90471@fedora.redhat.com/ > > > > > > It would be great if this could be standardised between QEMU and crosvm > > > (and therefore have a clearer path toward being implemented in other VMMs)! > > > > Setting aside vhost-user for a moment, the DAX example given by Stefan > > won't work in crosvm today. > > > > Is universal access to virtio shared memory regions actually mandated > > by the virtio spec? Copying from virtiofs DAX to virtiofs sharing > > seems reasonable enough, but what about virtio-pmem to virtio-blk? > > What about screenshotting a framebuffer in virtio-gpu shared memory to > > virtio-scsi? I guess with some plumbing in the VMM, it's solvable in a > > virtualized environment. But what about when you have real hardware > > that speaks virtio involved? That's outside my wheelhouse, but it > > doesn't seem like that would be easy to solve. > > Yes, it can work for physical devices if allowed by host configuration. > E.g. VFIO supports that I think. Don't think VDPA does. > I guess you meant iommufd support here? Thanks
© 2016 - 2024 Red Hat, Inc.