On Fri, Mar 8, 2024 at 6:01 PM Michael S. Tsirkin <m...@redhat.com> wrote:
>
> On Mon, Mar 04, 2024 at 02:46:06PM -0500, Jonah Palmer wrote:
> > Prevent ioeventfd from being enabled/disabled when a virtio-pci
> > device has negotiated the VIRTIO_F_NOTIFICATION_DATA transport
> > feature.
> >
> > Due to ioeventfd not being able to carry the extra data associated with
> > this feature, the ioeventfd should be left in a disabled state for
> > emulated virtio-pci devices using this feature.
> >
> > Reviewed-by: Eugenio Pérez <epere...@redhat.com>
> > Signed-off-by: Jonah Palmer <jonah.pal...@oracle.com>
>
> I thought hard about this. I propose that for now,
> instead of disabling ioevetfd silently we error out unless
> user disabled it for us.
> WDYT?
>
Yes, error is a better plan than silently disabling it. In the
(unlikely?) case we are able to make notification data work with
eventfd in the future, it makes the change more evident.
>
> > ---
> > hw/virtio/virtio-pci.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index d12edc567f..287b8f7720 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -417,13 +417,15 @@ static void virtio_ioport_write(void *opaque,
> > uint32_t addr, uint32_t val)
> > }
> > break;
> > case VIRTIO_PCI_STATUS:
> > - if (!(val & VIRTIO_CONFIG_S_DRIVER_OK)) {
> > + if (!(val & VIRTIO_CONFIG_S_DRIVER_OK) &&
> > + !virtio_vdev_has_feature(vdev, VIRTIO_F_NOTIFICATION_DATA)) {
> > virtio_pci_stop_ioeventfd(proxy);
> > }
> >
> > virtio_set_status(vdev, val & 0xFF);
> >
> > - if (val & VIRTIO_CONFIG_S_DRIVER_OK) {
> > + if ((val & VIRTIO_CONFIG_S_DRIVER_OK) &&
> > + !virtio_vdev_has_feature(vdev, VIRTIO_F_NOTIFICATION_DATA)) {
> > virtio_pci_start_ioeventfd(proxy);
> > }
> >
> > --
> > 2.39.3
>
