Hi Jonah QE tested this series v1 with a tap device with vhost=off with regression tests, everything works fine. And QE also add "notification_data=true" to the qemu command line then got "1" when performing the command [1] inside the guest. [1] cut -c39 /sys/devices/pci0000:00/0000:00:01.3/0000:05:00.0/virtio1/features
Tested-by: Lei Yang <leiy...@redhat.com> On Thu, Mar 7, 2024 at 7:18 PM Eugenio Perez Martin <epere...@redhat.com> wrote: > > On Wed, Mar 6, 2024 at 8:34 AM Michael S. Tsirkin <m...@redhat.com> wrote: > > > > On Wed, Mar 06, 2024 at 08:07:31AM +0100, Eugenio Perez Martin wrote: > > > On Wed, Mar 6, 2024 at 6:34 AM Jason Wang <jasow...@redhat.com> wrote: > > > > > > > > On Tue, Mar 5, 2024 at 3:46 AM Jonah Palmer <jonah.pal...@oracle.com> > > > > wrote: > > > > > > > > > > The goal of these patches are to add support to a variety of virtio > > > > > and > > > > > vhost devices for the VIRTIO_F_NOTIFICATION_DATA transport feature. > > > > > This > > > > > feature indicates that a driver will pass extra data (instead of just > > > > > a > > > > > virtqueue's index) when notifying the corresponding device. > > > > > > > > > > The data passed in by the driver when this feature is enabled varies > > > > > in > > > > > format depending on if the device is using a split or packed virtqueue > > > > > layout: > > > > > > > > > > Split VQ > > > > > - Upper 16 bits: shadow_avail_idx > > > > > - Lower 16 bits: virtqueue index > > > > > > > > > > Packed VQ > > > > > - Upper 16 bits: 1-bit wrap counter & 15-bit shadow_avail_idx > > > > > - Lower 16 bits: virtqueue index > > > > > > > > > > Also, due to the limitations of ioeventfd not being able to carry the > > > > > extra provided by the driver, ioeventfd is left disabled for any > > > > > devices > > > > > using this feature. > > > > > > > > Is there any method to overcome this? This might help for vhost. > > > > > > > > > > As a half-baked idea, read(2)ing an eventfd descriptor returns an > > > 8-byte integer already. The returned value of read depends on eventfd > > > flags, but both have to do with the number of writes of the other end. > > > > > > My proposal is to replace this value with the last value written by > > > the guest, so we can extract the virtio notification data from there. > > > The behavior of read is similar to not-EFD_SEMAPHORE, reading a value > > > and then blocking if read again without writes. The behavior of KVM > > > writes is different, as it is not a counter anymore. > > > > > > Thanks! > > > > > > I doubt you will be able to support this in ioeventfd... > > I agree. > > > But vhost does not really need the value at all. > > So why mask out ioeventfd with vhost? > > The interface should not be able to start with vhost-kernel because > the feature is not offered by the vhost-kernel device. So ioeventfd is > always enabled with vhost-kernel. > > Or do you mean we should allow it and let vhost-kernel fetch data from > the avail ring as usual? I'm ok with that but then the guest can place > any value to it, so the driver cannot be properly "validated by > software" that way. > > > vhost-vdpa is probably the only one that might need it... > > Right, but vhost-vdpa already supports doorbell memory regions so I > guess it has little use, isn't it? > > Thanks! > > > > > > > > > > > Thanks > > > > > > > > > > > > > > A significant aspect of this effort has been to maintain compatibility > > > > > across different backends. As such, the feature is offered by backend > > > > > devices only when supported, with fallback mechanisms where backend > > > > > support is absent. > > > > > > > > > > Jonah Palmer (8): > > > > > virtio/virtio-pci: Handle extra notification data > > > > > virtio-pci: Lock ioeventfd state with VIRTIO_F_NOTIFICATION_DATA > > > > > virtio-mmio: Handle extra notification data > > > > > virtio-mmio: Lock ioeventfd state with VIRTIO_F_NOTIFICATION_DATA > > > > > virtio-ccw: Handle extra notification data > > > > > virtio-ccw: Lock ioeventfd state with VIRTIO_F_NOTIFICATION_DATA > > > > > vhost/vhost-user: Add VIRTIO_F_NOTIFICATION_DATA to vhost feature > > > > > bits > > > > > virtio: Add VIRTIO_F_NOTIFICATION_DATA property definition > > > > > > > > > > hw/block/vhost-user-blk.c | 1 + > > > > > hw/net/vhost_net.c | 2 ++ > > > > > hw/s390x/s390-virtio-ccw.c | 16 ++++++++++++---- > > > > > hw/s390x/virtio-ccw.c | 6 ++++-- > > > > > hw/scsi/vhost-scsi.c | 1 + > > > > > hw/scsi/vhost-user-scsi.c | 1 + > > > > > hw/virtio/vhost-user-fs.c | 2 +- > > > > > hw/virtio/vhost-user-vsock.c | 1 + > > > > > hw/virtio/virtio-mmio.c | 15 +++++++++++---- > > > > > hw/virtio/virtio-pci.c | 16 +++++++++++----- > > > > > hw/virtio/virtio.c | 18 ++++++++++++++++++ > > > > > include/hw/virtio/virtio.h | 5 ++++- > > > > > net/vhost-vdpa.c | 1 + > > > > > 13 files changed, 68 insertions(+), 17 deletions(-) > > > > > > > > > > -- > > > > > 2.39.3 > > > > > > > > > > > > >