[PULL v3 12/55] virtio-pci: add support for configure interrupt

Michael S. Tsirkin posted 55 patches 4 years, 1 month ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Jason Wang <jasowang@redhat.com>, Shannon Zhao <shannon.zhaosl@gmail.com>, Peter Maydell <peter.maydell@linaro.org>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, Ani Sinha <ani@anisinha.ca>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Michael S. Tsirkin" <mst@redhat.com>, Raphael Norwitz <raphael.norwitz@nutanix.com>, Igor Mammedov <imammedo@redhat.com>, Peter Xu <peterx@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Kevin Wolf <kwolf@redhat.com>, David Hildenbrand <david@redhat.com>, "Gonglei (Arei)" <arei.gonglei@huawei.com>, Hanna Reitz <hreitz@redhat.com>, Fam Zheng <fam@euphon.net>
[PULL v3 12/55] virtio-pci: add support for configure interrupt
Posted by Michael S. Tsirkin 4 years, 1 month ago
From: Cindy Lu <lulu@redhat.com>

Add support for configure interrupt, The process is used kvm_irqfd_assign
to set the gsi to kernel. When the configure notifier was signal by
host, qemu will inject a msix interrupt to guest

Signed-off-by: Cindy Lu <lulu@redhat.com>
Message-Id: <20211104164827.21911-11-lulu@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio-pci.h |  4 +-
 hw/virtio/virtio-pci.c | 92 ++++++++++++++++++++++++++++++++++++------
 2 files changed, 83 insertions(+), 13 deletions(-)

diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 2446dcd9ae..b704acc5a8 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -251,5 +251,7 @@ void virtio_pci_types_register(const VirtioPCIDeviceTypeInfo *t);
  * @fixed_queues.
  */
 unsigned virtio_pci_optimal_num_queues(unsigned fixed_queues);
-
+void virtio_pci_set_guest_notifier_fd_handler(VirtIODevice *vdev, VirtQueue *vq,
+                                              int n, bool assign,
+                                              bool with_irqfd);
 #endif
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 7201cf3dc1..98fb5493ae 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -727,7 +727,8 @@ static int virtio_pci_get_notifier(VirtIOPCIProxy *proxy, int queue_no,
     VirtQueue *vq;
 
     if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
-        return -1;
+        *n = virtio_config_get_guest_notifier(vdev);
+        *vector = vdev->config_vector;
     } else {
         if (!virtio_queue_get_num(vdev, queue_no)) {
             return -1;
@@ -802,6 +803,10 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
     return ret;
 }
 
+static int kvm_virtio_pci_vector_config_use(VirtIOPCIProxy *proxy)
+{
+    return kvm_virtio_pci_vector_use_one(proxy, VIRTIO_CONFIG_IRQ_IDX);
+}
 
 static void kvm_virtio_pci_vector_release_one(VirtIOPCIProxy *proxy,
                                               int queue_no)
@@ -839,6 +844,11 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
     }
 }
 
+static void kvm_virtio_pci_vector_config_release(VirtIOPCIProxy *proxy)
+{
+    kvm_virtio_pci_vector_release_one(proxy, VIRTIO_CONFIG_IRQ_IDX);
+}
+
 static int virtio_pci_one_vector_unmask(VirtIOPCIProxy *proxy,
                                        unsigned int queue_no,
                                        unsigned int vector,
@@ -920,9 +930,17 @@ static int virtio_pci_vector_unmask(PCIDevice *dev, unsigned vector,
         }
         vq = virtio_vector_next_queue(vq);
     }
-
+    /* unmask config intr */
+    n = virtio_config_get_guest_notifier(vdev);
+    ret = virtio_pci_one_vector_unmask(proxy, VIRTIO_CONFIG_IRQ_IDX, vector,
+                                       msg, n);
+    if (ret < 0) {
+        goto undo_config;
+    }
     return 0;
-
+undo_config:
+    n = virtio_config_get_guest_notifier(vdev);
+    virtio_pci_one_vector_mask(proxy, VIRTIO_CONFIG_IRQ_IDX, vector, n);
 undo:
     vq = virtio_vector_first_queue(vdev, vector);
     while (vq && unmasked >= 0) {
@@ -956,6 +974,8 @@ static void virtio_pci_vector_mask(PCIDevice *dev, unsigned vector)
         }
         vq = virtio_vector_next_queue(vq);
     }
+    n = virtio_config_get_guest_notifier(vdev);
+    virtio_pci_one_vector_mask(proxy, VIRTIO_CONFIG_IRQ_IDX, vector, n);
 }
 
 static void virtio_pci_vector_poll(PCIDevice *dev,
@@ -987,6 +1007,34 @@ static void virtio_pci_vector_poll(PCIDevice *dev,
             msix_set_pending(dev, vector);
         }
     }
+    /* poll the config intr */
+    ret = virtio_pci_get_notifier(proxy, VIRTIO_CONFIG_IRQ_IDX, &notifier,
+                                  &vector);
+    if (ret < 0) {
+        return;
+    }
+    if (vector < vector_start || vector >= vector_end ||
+        !msix_is_masked(dev, vector)) {
+        return;
+    }
+    if (k->guest_notifier_pending) {
+        if (k->guest_notifier_pending(vdev, VIRTIO_CONFIG_IRQ_IDX)) {
+            msix_set_pending(dev, vector);
+        }
+    } else if (event_notifier_test_and_clear(notifier)) {
+        msix_set_pending(dev, vector);
+    }
+}
+
+void virtio_pci_set_guest_notifier_fd_handler(VirtIODevice *vdev, VirtQueue *vq,
+                                              int n, bool assign,
+                                              bool with_irqfd)
+{
+    if (n == VIRTIO_CONFIG_IRQ_IDX) {
+        virtio_config_set_guest_notifier_fd_handler(vdev, assign, with_irqfd);
+    } else {
+        virtio_queue_set_guest_notifier_fd_handler(vq, assign, with_irqfd);
+    }
 }
 
 static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
@@ -995,17 +1043,25 @@ static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
     VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
     VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
-    VirtQueue *vq = virtio_get_queue(vdev, n);
-    EventNotifier *notifier = virtio_queue_get_guest_notifier(vq);
+    VirtQueue *vq = NULL;
+    EventNotifier *notifier = NULL;
+
+    if (n == VIRTIO_CONFIG_IRQ_IDX) {
+        notifier = virtio_config_get_guest_notifier(vdev);
+    } else {
+        vq = virtio_get_queue(vdev, n);
+        notifier = virtio_queue_get_guest_notifier(vq);
+    }
 
     if (assign) {
         int r = event_notifier_init(notifier, 0);
         if (r < 0) {
             return r;
         }
-        virtio_queue_set_guest_notifier_fd_handler(vq, true, with_irqfd);
+        virtio_pci_set_guest_notifier_fd_handler(vdev, vq, n, true, with_irqfd);
     } else {
-        virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd);
+        virtio_pci_set_guest_notifier_fd_handler(vdev, vq, n, false,
+                                                 with_irqfd);
         event_notifier_cleanup(notifier);
     }
 
@@ -1047,6 +1103,7 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
         msix_unset_vector_notifiers(&proxy->pci_dev);
         if (proxy->vector_irqfd) {
             kvm_virtio_pci_vector_release(proxy, nvqs);
+            kvm_virtio_pci_vector_config_release(proxy);
             g_free(proxy->vector_irqfd);
             proxy->vector_irqfd = NULL;
         }
@@ -1062,7 +1119,11 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
             goto assign_error;
         }
     }
-
+    r = virtio_pci_set_guest_notifier(d, VIRTIO_CONFIG_IRQ_IDX, assign,
+                                      with_irqfd);
+    if (r < 0) {
+        goto config_assign_error;
+    }
     /* Must set vector notifier after guest notifier has been assigned */
     if ((with_irqfd || k->guest_notifier_mask) && assign) {
         if (with_irqfd) {
@@ -1071,11 +1132,14 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
                           msix_nr_vectors_allocated(&proxy->pci_dev));
             r = kvm_virtio_pci_vector_use(proxy, nvqs);
             if (r < 0) {
-                goto assign_error;
+                goto config_assign_error;
             }
         }
-        r = msix_set_vector_notifiers(&proxy->pci_dev,
-                                      virtio_pci_vector_unmask,
+        r = kvm_virtio_pci_vector_config_use(proxy);
+        if (r < 0) {
+            goto config_error;
+        }
+        r = msix_set_vector_notifiers(&proxy->pci_dev, virtio_pci_vector_unmask,
                                       virtio_pci_vector_mask,
                                       virtio_pci_vector_poll);
         if (r < 0) {
@@ -1090,7 +1154,11 @@ notifiers_error:
         assert(assign);
         kvm_virtio_pci_vector_release(proxy, nvqs);
     }
-
+config_error:
+    kvm_virtio_pci_vector_config_release(proxy);
+config_assign_error:
+    virtio_pci_set_guest_notifier(d, VIRTIO_CONFIG_IRQ_IDX, !assign,
+                                  with_irqfd);
 assign_error:
     /* We get here on assignment failure. Recover by undoing for VQs 0 .. n. */
     assert(assign);
-- 
MST


Re: [PULL v3 12/55] virtio-pci: add support for configure interrupt
Posted by Volker Rümelin 4 years, 1 month ago
Hi,

> From: Cindy Lu <lulu@redhat.com>
>
> Add support for configure interrupt, The process is used kvm_irqfd_assign
> to set the gsi to kernel. When the configure notifier was signal by
> host, qemu will inject a msix interrupt to guest
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> Message-Id: <20211104164827.21911-11-lulu@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>   hw/virtio/virtio-pci.h |  4 +-
>   hw/virtio/virtio-pci.c | 92 ++++++++++++++++++++++++++++++++++++------
>   2 files changed, 83 insertions(+), 13 deletions(-)
>

Since this commit I see the following warnings.

With -drive 
if=virtio,id=disk1,file=/srv/cdimg/Linux/images/opensuse.qcow2,discard=unmap

qemu-system-x86_64: virtio-blk failed to set guest notifier (-16), 
ensure -accel kvm is set.
qemu-system-x86_64: virtio_bus_start_ioeventfd: failed. Fallback to 
userspace (slower).

With libvirt
     <controller type='pci' index='1' model='pcie-root-port'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x1c' 
function='0'
        multifunction='on'/>
     </controller>
     <controller type='pci' index='2' model='pcie-root-port'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x1c' 
function='1'/>
     </controller>
     <controller type='scsi' index='0' model='virtio-scsi'>
       <driver queues='4'/>
       <address type='pci' domain='0x0000' bus='0x01' slot='0x00' 
function='0'/>
     </controller>
     <disk type='block' device='disk'>
       <driver name='qemu' type='raw' cache='none' discard='unmap' 
io='io_uring'/>
       <source dev='/dev/vgtmp/lnxpowerm1'/>
       <target dev='sda' bus='scsi'/>
       <address type='drive' controller='0' bus='0' unit='0'/>
       <boot order='1'/>
     </disk>

2022-01-08T17:45:26.911491Z qemu-system-x86_64: virtio-scsi: Failed to 
set guest notifiers (-16), ensure -accel kvm is set.
2022-01-08T17:45:26.911505Z qemu-system-x86_64: 
virtio_bus_start_ioeventfd: failed. Fallback to userspace (slower).

The messages appear around the time the Linux guest initializes the drivers.

With best regards,
Volker

Re: [PULL v3 12/55] virtio-pci: add support for configure interrupt
Posted by Michael S. Tsirkin 4 years ago
On Sun, Jan 09, 2022 at 07:17:30AM +0100, Volker Rümelin wrote:
> Hi,
> 
> > From: Cindy Lu <lulu@redhat.com>
> > 
> > Add support for configure interrupt, The process is used kvm_irqfd_assign
> > to set the gsi to kernel. When the configure notifier was signal by
> > host, qemu will inject a msix interrupt to guest
> > 
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > Message-Id: <20211104164827.21911-11-lulu@redhat.com>
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >   hw/virtio/virtio-pci.h |  4 +-
> >   hw/virtio/virtio-pci.c | 92 ++++++++++++++++++++++++++++++++++++------
> >   2 files changed, 83 insertions(+), 13 deletions(-)
> > 
> 
> Since this commit I see the following warnings.
> 
> With -drive
> if=virtio,id=disk1,file=/srv/cdimg/Linux/images/opensuse.qcow2,discard=unmap
> 
> qemu-system-x86_64: virtio-blk failed to set guest notifier (-16), ensure
> -accel kvm is set.
> qemu-system-x86_64: virtio_bus_start_ioeventfd: failed. Fallback to
> userspace (slower).
> 
> With libvirt
>     <controller type='pci' index='1' model='pcie-root-port'>
>       <address type='pci' domain='0x0000' bus='0x00' slot='0x1c'
> function='0'
>        multifunction='on'/>
>     </controller>
>     <controller type='pci' index='2' model='pcie-root-port'>
>       <address type='pci' domain='0x0000' bus='0x00' slot='0x1c'
> function='1'/>
>     </controller>
>     <controller type='scsi' index='0' model='virtio-scsi'>
>       <driver queues='4'/>
>       <address type='pci' domain='0x0000' bus='0x01' slot='0x00'
> function='0'/>
>     </controller>
>     <disk type='block' device='disk'>
>       <driver name='qemu' type='raw' cache='none' discard='unmap'
> io='io_uring'/>
>       <source dev='/dev/vgtmp/lnxpowerm1'/>
>       <target dev='sda' bus='scsi'/>
>       <address type='drive' controller='0' bus='0' unit='0'/>
>       <boot order='1'/>
>     </disk>
> 
> 2022-01-08T17:45:26.911491Z qemu-system-x86_64: virtio-scsi: Failed to set
> guest notifiers (-16), ensure -accel kvm is set.
> 2022-01-08T17:45:26.911505Z qemu-system-x86_64: virtio_bus_start_ioeventfd:
> failed. Fallback to userspace (slower).
> 
> The messages appear around the time the Linux guest initializes the drivers.
> 
> With best regards,
> Volker

I guess it's a host that has an oldish kernel?
Does the following help?


diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 98fb5493ae..b77cd69f97 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1130,15 +1130,15 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
             proxy->vector_irqfd =
                 g_malloc0(sizeof(*proxy->vector_irqfd) *
                           msix_nr_vectors_allocated(&proxy->pci_dev));
+            r = kvm_virtio_pci_vector_config_use(proxy);
+            if (r < 0) {
+                goto config_error;
+            }
             r = kvm_virtio_pci_vector_use(proxy, nvqs);
             if (r < 0) {
                 goto config_assign_error;
             }
         }
-        r = kvm_virtio_pci_vector_config_use(proxy);
-        if (r < 0) {
-            goto config_error;
-        }
         r = msix_set_vector_notifiers(&proxy->pci_dev, virtio_pci_vector_unmask,
                                       virtio_pci_vector_mask,
                                       virtio_pci_vector_poll);
@@ -1155,7 +1155,9 @@ notifiers_error:
         kvm_virtio_pci_vector_release(proxy, nvqs);
     }
 config_error:
-    kvm_virtio_pci_vector_config_release(proxy);
+    if (with_irqfd) {
+        kvm_virtio_pci_vector_config_release(proxy);
+    }
 config_assign_error:
     virtio_pci_set_guest_notifier(d, VIRTIO_CONFIG_IRQ_IDX, !assign,
                                   with_irqfd);
-- 
MST


Re: [PULL v3 12/55] virtio-pci: add support for configure interrupt
Posted by Volker Rümelin 4 years ago
Am 09.01.22 um 17:11 schrieb Michael S. Tsirkin:
> On Sun, Jan 09, 2022 at 07:17:30AM +0100, Volker Rümelin wrote:
>> Hi,
>>
>>> From: Cindy Lu <lulu@redhat.com>
>>>
>>> Add support for configure interrupt, The process is used kvm_irqfd_assign
>>> to set the gsi to kernel. When the configure notifier was signal by
>>> host, qemu will inject a msix interrupt to guest
>>>
>>> Signed-off-by: Cindy Lu <lulu@redhat.com>
>>> Message-Id: <20211104164827.21911-11-lulu@redhat.com>
>>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>> ---
>>>    hw/virtio/virtio-pci.h |  4 +-
>>>    hw/virtio/virtio-pci.c | 92 ++++++++++++++++++++++++++++++++++++------
>>>    2 files changed, 83 insertions(+), 13 deletions(-)
>>>
>> Since this commit I see the following warnings.
>>
>> With -drive
>> if=virtio,id=disk1,file=/srv/cdimg/Linux/images/opensuse.qcow2,discard=unmap
>>
>> qemu-system-x86_64: virtio-blk failed to set guest notifier (-16), ensure
>> -accel kvm is set.
>> qemu-system-x86_64: virtio_bus_start_ioeventfd: failed. Fallback to
>> userspace (slower).
>>
>> With libvirt
>>      <controller type='pci' index='1' model='pcie-root-port'>
>>        <address type='pci' domain='0x0000' bus='0x00' slot='0x1c'
>> function='0'
>>         multifunction='on'/>
>>      </controller>
>>      <controller type='pci' index='2' model='pcie-root-port'>
>>        <address type='pci' domain='0x0000' bus='0x00' slot='0x1c'
>> function='1'/>
>>      </controller>
>>      <controller type='scsi' index='0' model='virtio-scsi'>
>>        <driver queues='4'/>
>>        <address type='pci' domain='0x0000' bus='0x01' slot='0x00'
>> function='0'/>
>>      </controller>
>>      <disk type='block' device='disk'>
>>        <driver name='qemu' type='raw' cache='none' discard='unmap'
>> io='io_uring'/>
>>        <source dev='/dev/vgtmp/lnxpowerm1'/>
>>        <target dev='sda' bus='scsi'/>
>>        <address type='drive' controller='0' bus='0' unit='0'/>
>>        <boot order='1'/>
>>      </disk>
>>
>> 2022-01-08T17:45:26.911491Z qemu-system-x86_64: virtio-scsi: Failed to set
>> guest notifiers (-16), ensure -accel kvm is set.
>> 2022-01-08T17:45:26.911505Z qemu-system-x86_64: virtio_bus_start_ioeventfd:
>> failed. Fallback to userspace (slower).
>>
>> The messages appear around the time the Linux guest initializes the drivers.
>>
>> With best regards,
>> Volker
> I guess it's a host that has an oldish kernel?

It's an openSUSE 5.3.18 frankenstein kernel.

> Does the following help?
>

No.

> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 98fb5493ae..b77cd69f97 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1130,15 +1130,15 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
>               proxy->vector_irqfd =
>                   g_malloc0(sizeof(*proxy->vector_irqfd) *
>                             msix_nr_vectors_allocated(&proxy->pci_dev));
> +            r = kvm_virtio_pci_vector_config_use(proxy);
> +            if (r < 0) {
> +                goto config_error;
> +            }
>               r = kvm_virtio_pci_vector_use(proxy, nvqs);
>               if (r < 0) {
>                   goto config_assign_error;
>               }
>           }
> -        r = kvm_virtio_pci_vector_config_use(proxy);
> -        if (r < 0) {
> -            goto config_error;
> -        }
>           r = msix_set_vector_notifiers(&proxy->pci_dev, virtio_pci_vector_unmask,
>                                         virtio_pci_vector_mask,
>                                         virtio_pci_vector_poll);

With and without this patch msix_set_vector_notifiers() returns -16.

> @@ -1155,7 +1155,9 @@ notifiers_error:
>           kvm_virtio_pci_vector_release(proxy, nvqs);
>       }
>   config_error:
> -    kvm_virtio_pci_vector_config_release(proxy);
> +    if (with_irqfd) {
> +        kvm_virtio_pci_vector_config_release(proxy);
> +    }
>   config_assign_error:
>       virtio_pci_set_guest_notifier(d, VIRTIO_CONFIG_IRQ_IDX, !assign,
>                                     with_irqfd);


Re: [PULL v3 12/55] virtio-pci: add support for configure interrupt
Posted by Michael S. Tsirkin 4 years ago
On Sun, Jan 09, 2022 at 06:52:28PM +0100, Volker Rümelin wrote:
> Am 09.01.22 um 17:11 schrieb Michael S. Tsirkin:
> > On Sun, Jan 09, 2022 at 07:17:30AM +0100, Volker Rümelin wrote:
> > > Hi,
> > > 
> > > > From: Cindy Lu <lulu@redhat.com>
> > > > 
> > > > Add support for configure interrupt, The process is used kvm_irqfd_assign
> > > > to set the gsi to kernel. When the configure notifier was signal by
> > > > host, qemu will inject a msix interrupt to guest
> > > > 
> > > > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > > > Message-Id: <20211104164827.21911-11-lulu@redhat.com>
> > > > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > ---
> > > >    hw/virtio/virtio-pci.h |  4 +-
> > > >    hw/virtio/virtio-pci.c | 92 ++++++++++++++++++++++++++++++++++++------
> > > >    2 files changed, 83 insertions(+), 13 deletions(-)
> > > > 
> > > Since this commit I see the following warnings.
> > > 
> > > With -drive
> > > if=virtio,id=disk1,file=/srv/cdimg/Linux/images/opensuse.qcow2,discard=unmap
> > > 
> > > qemu-system-x86_64: virtio-blk failed to set guest notifier (-16), ensure
> > > -accel kvm is set.
> > > qemu-system-x86_64: virtio_bus_start_ioeventfd: failed. Fallback to
> > > userspace (slower).
> > > 
> > > With libvirt
> > >      <controller type='pci' index='1' model='pcie-root-port'>
> > >        <address type='pci' domain='0x0000' bus='0x00' slot='0x1c'
> > > function='0'
> > >         multifunction='on'/>
> > >      </controller>
> > >      <controller type='pci' index='2' model='pcie-root-port'>
> > >        <address type='pci' domain='0x0000' bus='0x00' slot='0x1c'
> > > function='1'/>
> > >      </controller>
> > >      <controller type='scsi' index='0' model='virtio-scsi'>
> > >        <driver queues='4'/>
> > >        <address type='pci' domain='0x0000' bus='0x01' slot='0x00'
> > > function='0'/>
> > >      </controller>
> > >      <disk type='block' device='disk'>
> > >        <driver name='qemu' type='raw' cache='none' discard='unmap'
> > > io='io_uring'/>
> > >        <source dev='/dev/vgtmp/lnxpowerm1'/>
> > >        <target dev='sda' bus='scsi'/>
> > >        <address type='drive' controller='0' bus='0' unit='0'/>
> > >        <boot order='1'/>
> > >      </disk>
> > > 
> > > 2022-01-08T17:45:26.911491Z qemu-system-x86_64: virtio-scsi: Failed to set
> > > guest notifiers (-16), ensure -accel kvm is set.
> > > 2022-01-08T17:45:26.911505Z qemu-system-x86_64: virtio_bus_start_ioeventfd:
> > > failed. Fallback to userspace (slower).
> > > 
> > > The messages appear around the time the Linux guest initializes the drivers.
> > > 
> > > With best regards,
> > > Volker
> > I guess it's a host that has an oldish kernel?
> 
> It's an openSUSE 5.3.18 frankenstein kernel.
> 
> > Does the following help?
> > 
> 
> No.
> 
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index 98fb5493ae..b77cd69f97 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -1130,15 +1130,15 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
> >               proxy->vector_irqfd =
> >                   g_malloc0(sizeof(*proxy->vector_irqfd) *
> >                             msix_nr_vectors_allocated(&proxy->pci_dev));
> > +            r = kvm_virtio_pci_vector_config_use(proxy);
> > +            if (r < 0) {
> > +                goto config_error;
> > +            }
> >               r = kvm_virtio_pci_vector_use(proxy, nvqs);
> >               if (r < 0) {
> >                   goto config_assign_error;
> >               }
> >           }
> > -        r = kvm_virtio_pci_vector_config_use(proxy);
> > -        if (r < 0) {
> > -            goto config_error;
> > -        }
> >           r = msix_set_vector_notifiers(&proxy->pci_dev, virtio_pci_vector_unmask,
> >                                         virtio_pci_vector_mask,
> >                                         virtio_pci_vector_poll);
> 
> With and without this patch msix_set_vector_notifiers() returns -16.


EBUSY

strace? did a syscall return this?

> > @@ -1155,7 +1155,9 @@ notifiers_error:
> >           kvm_virtio_pci_vector_release(proxy, nvqs);
> >       }
> >   config_error:
> > -    kvm_virtio_pci_vector_config_release(proxy);
> > +    if (with_irqfd) {
> > +        kvm_virtio_pci_vector_config_release(proxy);
> > +    }
> >   config_assign_error:
> >       virtio_pci_set_guest_notifier(d, VIRTIO_CONFIG_IRQ_IDX, !assign,
> >                                     with_irqfd);


Re: [PULL v3 12/55] virtio-pci: add support for configure interrupt
Posted by Volker Rümelin 4 years ago
Am 09.01.22 um 19:01 schrieb Michael S. Tsirkin:
> On Sun, Jan 09, 2022 at 06:52:28PM +0100, Volker Rümelin wrote:
>> Am 09.01.22 um 17:11 schrieb Michael S. Tsirkin:
>>> On Sun, Jan 09, 2022 at 07:17:30AM +0100, Volker Rümelin wrote:
>>>> Hi,
>>>>
>>>>> From: Cindy Lu <lulu@redhat.com>
>>>>>
>>>>> Add support for configure interrupt, The process is used kvm_irqfd_assign
>>>>> to set the gsi to kernel. When the configure notifier was signal by
>>>>> host, qemu will inject a msix interrupt to guest
>>>>>
>>>>> Signed-off-by: Cindy Lu <lulu@redhat.com>
>>>>> Message-Id: <20211104164827.21911-11-lulu@redhat.com>
>>>>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>>>> ---
>>>>>     hw/virtio/virtio-pci.h |  4 +-
>>>>>     hw/virtio/virtio-pci.c | 92 ++++++++++++++++++++++++++++++++++++------
>>>>>     2 files changed, 83 insertions(+), 13 deletions(-)
>>>>>
>>>> Since this commit I see the following warnings.
>>>>
>>>> With -drive
>>>> if=virtio,id=disk1,file=/srv/cdimg/Linux/images/opensuse.qcow2,discard=unmap
>>>>
>>>> qemu-system-x86_64: virtio-blk failed to set guest notifier (-16), ensure
>>>> -accel kvm is set.
>>>> qemu-system-x86_64: virtio_bus_start_ioeventfd: failed. Fallback to
>>>> userspace (slower).
>>>>
>>>> With libvirt
>>>>       <controller type='pci' index='1' model='pcie-root-port'>
>>>>         <address type='pci' domain='0x0000' bus='0x00' slot='0x1c'
>>>> function='0'
>>>>          multifunction='on'/>
>>>>       </controller>
>>>>       <controller type='pci' index='2' model='pcie-root-port'>
>>>>         <address type='pci' domain='0x0000' bus='0x00' slot='0x1c'
>>>> function='1'/>
>>>>       </controller>
>>>>       <controller type='scsi' index='0' model='virtio-scsi'>
>>>>         <driver queues='4'/>
>>>>         <address type='pci' domain='0x0000' bus='0x01' slot='0x00'
>>>> function='0'/>
>>>>       </controller>
>>>>       <disk type='block' device='disk'>
>>>>         <driver name='qemu' type='raw' cache='none' discard='unmap'
>>>> io='io_uring'/>
>>>>         <source dev='/dev/vgtmp/lnxpowerm1'/>
>>>>         <target dev='sda' bus='scsi'/>
>>>>         <address type='drive' controller='0' bus='0' unit='0'/>
>>>>         <boot order='1'/>
>>>>       </disk>
>>>>
>>>> 2022-01-08T17:45:26.911491Z qemu-system-x86_64: virtio-scsi: Failed to set
>>>> guest notifiers (-16), ensure -accel kvm is set.
>>>> 2022-01-08T17:45:26.911505Z qemu-system-x86_64: virtio_bus_start_ioeventfd:
>>>> failed. Fallback to userspace (slower).
>>>>
>>>> The messages appear around the time the Linux guest initializes the drivers.
>>>>
>>>> With best regards,
>>>> Volker
>>> I guess it's a host that has an oldish kernel?
>> It's an openSUSE 5.3.18 frankenstein kernel.
>>
>>> Does the following help?
>>>
>> No.
>>
>>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>>> index 98fb5493ae..b77cd69f97 100644
>>> --- a/hw/virtio/virtio-pci.c
>>> +++ b/hw/virtio/virtio-pci.c
>>> @@ -1130,15 +1130,15 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
>>>                proxy->vector_irqfd =
>>>                    g_malloc0(sizeof(*proxy->vector_irqfd) *
>>>                              msix_nr_vectors_allocated(&proxy->pci_dev));
>>> +            r = kvm_virtio_pci_vector_config_use(proxy);
>>> +            if (r < 0) {
>>> +                goto config_error;
>>> +            }
>>>                r = kvm_virtio_pci_vector_use(proxy, nvqs);
>>>                if (r < 0) {
>>>                    goto config_assign_error;
>>>                }
>>>            }
>>> -        r = kvm_virtio_pci_vector_config_use(proxy);
>>> -        if (r < 0) {
>>> -            goto config_error;
>>> -        }
>>>            r = msix_set_vector_notifiers(&proxy->pci_dev, virtio_pci_vector_unmask,
>>>                                          virtio_pci_vector_mask,
>>>                                          virtio_pci_vector_poll);
>> With and without this patch msix_set_vector_notifiers() returns -16.
>
> EBUSY
>
> strace? did a syscall return this?

I hope I got that right. I used

strace ./qemu-system-x86_64 ... 2>strace.txt

There is no EBUSY in strace.txt.

I will sprinkle a few fprintfs to find the first function returning -16.

>
>>> @@ -1155,7 +1155,9 @@ notifiers_error:
>>>            kvm_virtio_pci_vector_release(proxy, nvqs);
>>>        }
>>>    config_error:
>>> -    kvm_virtio_pci_vector_config_release(proxy);
>>> +    if (with_irqfd) {
>>> +        kvm_virtio_pci_vector_config_release(proxy);
>>> +    }
>>>    config_assign_error:
>>>        virtio_pci_set_guest_notifier(d, VIRTIO_CONFIG_IRQ_IDX, !assign,
>>>                                      with_irqfd);


Re: [PULL v3 12/55] virtio-pci: add support for configure interrupt
Posted by Volker Rümelin 4 years ago
Am 09.01.22 um 19:54 schrieb Volker Rümelin:
> Am 09.01.22 um 19:01 schrieb Michael S. Tsirkin:
>> On Sun, Jan 09, 2022 at 06:52:28PM +0100, Volker Rümelin wrote:
>>> Am 09.01.22 um 17:11 schrieb Michael S. Tsirkin:
>>>> On Sun, Jan 09, 2022 at 07:17:30AM +0100, Volker Rümelin wrote:
>>>>> Hi,
>>>>>
>>>>>> From: Cindy Lu <lulu@redhat.com>
>>>>>>
>>>>>> Add support for configure interrupt, The process is used 
>>>>>> kvm_irqfd_assign
>>>>>> to set the gsi to kernel. When the configure notifier was signal by
>>>>>> host, qemu will inject a msix interrupt to guest
>>>>>>
>>>>>> Signed-off-by: Cindy Lu <lulu@redhat.com>
>>>>>> Message-Id: <20211104164827.21911-11-lulu@redhat.com>
>>>>>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>>>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>>>>> ---
>>>>>>     hw/virtio/virtio-pci.h |  4 +-
>>>>>>     hw/virtio/virtio-pci.c | 92 
>>>>>> ++++++++++++++++++++++++++++++++++++------
>>>>>>     2 files changed, 83 insertions(+), 13 deletions(-)
>>>>>>
>>>>> Since this commit I see the following warnings.
>>>>>
>>>>> With -drive
>>>>> if=virtio,id=disk1,file=/srv/cdimg/Linux/images/opensuse.qcow2,discard=unmap 
>>>>>
>>>>>
>>>>> qemu-system-x86_64: virtio-blk failed to set guest notifier (-16), 
>>>>> ensure
>>>>> -accel kvm is set.
>>>>> qemu-system-x86_64: virtio_bus_start_ioeventfd: failed. Fallback to
>>>>> userspace (slower).
>>>>>
>>>>> With libvirt
>>>>>       <controller type='pci' index='1' model='pcie-root-port'>
>>>>>         <address type='pci' domain='0x0000' bus='0x00' slot='0x1c'
>>>>> function='0'
>>>>>          multifunction='on'/>
>>>>>       </controller>
>>>>>       <controller type='pci' index='2' model='pcie-root-port'>
>>>>>         <address type='pci' domain='0x0000' bus='0x00' slot='0x1c'
>>>>> function='1'/>
>>>>>       </controller>
>>>>>       <controller type='scsi' index='0' model='virtio-scsi'>
>>>>>         <driver queues='4'/>
>>>>>         <address type='pci' domain='0x0000' bus='0x01' slot='0x00'
>>>>> function='0'/>
>>>>>       </controller>
>>>>>       <disk type='block' device='disk'>
>>>>>         <driver name='qemu' type='raw' cache='none' discard='unmap'
>>>>> io='io_uring'/>
>>>>>         <source dev='/dev/vgtmp/lnxpowerm1'/>
>>>>>         <target dev='sda' bus='scsi'/>
>>>>>         <address type='drive' controller='0' bus='0' unit='0'/>
>>>>>         <boot order='1'/>
>>>>>       </disk>
>>>>>
>>>>> 2022-01-08T17:45:26.911491Z qemu-system-x86_64: virtio-scsi: 
>>>>> Failed to set
>>>>> guest notifiers (-16), ensure -accel kvm is set.
>>>>> 2022-01-08T17:45:26.911505Z qemu-system-x86_64: 
>>>>> virtio_bus_start_ioeventfd:
>>>>> failed. Fallback to userspace (slower).
>>>>>
>>>>> The messages appear around the time the Linux guest initializes 
>>>>> the drivers.
>>>>>
>>>>> With best regards,
>>>>> Volker
>>>> I guess it's a host that has an oldish kernel?
>>> It's an openSUSE 5.3.18 frankenstein kernel.
>>>
>>>> Does the following help?
>>>>
>>> No.
>>>
>>>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>>>> index 98fb5493ae..b77cd69f97 100644
>>>> --- a/hw/virtio/virtio-pci.c
>>>> +++ b/hw/virtio/virtio-pci.c
>>>> @@ -1130,15 +1130,15 @@ static int 
>>>> virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
>>>>                proxy->vector_irqfd =
>>>>                    g_malloc0(sizeof(*proxy->vector_irqfd) *
>>>> msix_nr_vectors_allocated(&proxy->pci_dev));
>>>> +            r = kvm_virtio_pci_vector_config_use(proxy);
>>>> +            if (r < 0) {
>>>> +                goto config_error;
>>>> +            }
>>>>                r = kvm_virtio_pci_vector_use(proxy, nvqs);
>>>>                if (r < 0) {
>>>>                    goto config_assign_error;
>>>>                }
>>>>            }
>>>> -        r = kvm_virtio_pci_vector_config_use(proxy);
>>>> -        if (r < 0) {
>>>> -            goto config_error;
>>>> -        }
>>>>            r = msix_set_vector_notifiers(&proxy->pci_dev, 
>>>> virtio_pci_vector_unmask,
>>>> virtio_pci_vector_mask,
>>>> virtio_pci_vector_poll);
>>> With and without this patch msix_set_vector_notifiers() returns -16.
>>
>> EBUSY
>>
>> strace? did a syscall return this?
>
> I hope I got that right. I used
>
> strace ./qemu-system-x86_64 ... 2>strace.txt
>
> There is no EBUSY in strace.txt.
>
> I will sprinkle a few fprintfs to find the first function returning -16.
>

This is the call sequence:

virtio_pci_set_guest_notifiers: call msix_set_vector_notifiers()
msix_set_vector_notifiers: call msix_set_notifier_for_vector(dev, 0);
msix_set_notifier_for_vector: call msix_vector_use_notifier(dev, 0, msg);
virtio_pci_vector_unmask: call virtio_pci_one_vector_unmask(proxy, 
VIRTIO_CONFIG_IRQ_IDX, 0, msg, n)
virtio_pci_one_vector_unmask: call kvm_irqchip_update_msi_route()
kvm_irqchip_update_msi_route ret 0
virtio_pci_one_vector_unmask: call kvm_virtio_pci_irqfd_use(proxy, n, 0)
kvm_irqchip_assign_irqfd: call kvm_vm_ioctl(s, KVM_IRQFD, &irqfd)
kvm_vm_ioctl ret 0
kvm_virtio_pci_irqfd_use ret 0
virtio_pci_one_vector_unmask ret 0
msix_vector_use_notifier ret 0
msix_set_notifier_for_vector ret 0
msix_set_vector_notifiers: call msix_set_notifier_for_vector(dev, 1);
msix_set_notifier_for_vector: call msix_vector_use_notifier(dev, 1, msg);
virtio_pci_vector_unmask: call virtio_pci_one_vector_unmask(proxy, 0, 1, 
msg, n)
virtio_pci_one_vector_unmask: call kvm_irqchip_update_msi_route()
kvm_irqchip_update_msi_route ret 0
virtio_pci_one_vector_unmask: call kvm_virtio_pci_irqfd_use(proxy, n, 1)
kvm_irqchip_assign_irqfd: call kvm_vm_ioctl(s, KVM_IRQFD, &irqfd)
kvm_vm_ioctl ret 0
kvm_virtio_pci_irqfd_use ret 0
virtio_pci_one_vector_unmask ret 0
virtio_pci_vector_unmask: call virtio_pci_one_vector_unmask(proxy, 
VIRTIO_CONFIG_IRQ_IDX, 1, msg, n)
virtio_pci_one_vector_unmask: call kvm_irqchip_update_msi_route()
kvm_irqchip_update_msi_route ret 0
virtio_pci_one_vector_unmask: call kvm_virtio_pci_irqfd_use(proxy, n, 1)
kvm_irqchip_assign_irqfd: call kvm_vm_ioctl(s, KVM_IRQFD, &irqfd)
kvm_vm_ioctl ret -16
kvm_virtio_pci_irqfd_use ret -16
virtio_pci_one_vector_unmask ret -16
kvm_irqchip_assign_irqfd: call kvm_vm_ioctl(s, KVM_IRQFD, &irqfd)
kvm_vm_ioctl ret 0
kvm_irqchip_assign_irqfd: call kvm_vm_ioctl(s, KVM_IRQFD, &irqfd)
kvm_vm_ioctl ret 0
msix_vector_use_notifier ret -16
msix_set_notifier_for_vector ret -16
kvm_irqchip_assign_irqfd: call kvm_vm_ioctl(s, KVM_IRQFD, &irqfd)
kvm_vm_ioctl ret 0
msix_set_vector_notifiers r -16
qemu-system-x86_64: virtio-blk failed to set guest notifier (-16), 
ensure -accel kvm is set.
qemu-system-x86_64: virtio_bus_start_ioeventfd: failed. Fallback to 
userspace (slower).

>>
>>>> @@ -1155,7 +1155,9 @@ notifiers_error:
>>>>            kvm_virtio_pci_vector_release(proxy, nvqs);
>>>>        }
>>>>    config_error:
>>>> -    kvm_virtio_pci_vector_config_release(proxy);
>>>> +    if (with_irqfd) {
>>>> +        kvm_virtio_pci_vector_config_release(proxy);
>>>> +    }
>>>>    config_assign_error:
>>>>        virtio_pci_set_guest_notifier(d, VIRTIO_CONFIG_IRQ_IDX, 
>>>> !assign,
>>>>                                      with_irqfd);
>


Re: [PULL v3 12/55] virtio-pci: add support for configure interrupt
Posted by Cédric Le Goater 4 years, 1 month ago
Hello Cindy,

On 1/8/22 02:04, Michael S. Tsirkin wrote:
> From: Cindy Lu <lulu@redhat.com>
> 
> Add support for configure interrupt, The process is used kvm_irqfd_assign
> to set the gsi to kernel. When the configure notifier was signal by
> host, qemu will inject a msix interrupt to guest
> 
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> Message-Id: <20211104164827.21911-11-lulu@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>   hw/virtio/virtio-pci.h |  4 +-
>   hw/virtio/virtio-pci.c | 92 ++++++++++++++++++++++++++++++++++++------
>   2 files changed, 83 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> index 2446dcd9ae..b704acc5a8 100644
> --- a/hw/virtio/virtio-pci.h
> +++ b/hw/virtio/virtio-pci.h
> @@ -251,5 +251,7 @@ void virtio_pci_types_register(const VirtioPCIDeviceTypeInfo *t);
>    * @fixed_queues.
>    */
>   unsigned virtio_pci_optimal_num_queues(unsigned fixed_queues);
> -
> +void virtio_pci_set_guest_notifier_fd_handler(VirtIODevice *vdev, VirtQueue *vq,
> +                                              int n, bool assign,
> +                                              bool with_irqfd);
>   #endif
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 7201cf3dc1..98fb5493ae 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -727,7 +727,8 @@ static int virtio_pci_get_notifier(VirtIOPCIProxy *proxy, int queue_no,
>       VirtQueue *vq;
>   
>       if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
> -        return -1;
> +        *n = virtio_config_get_guest_notifier(vdev);
> +        *vector = vdev->config_vector;
>       } else {
>           if (!virtio_queue_get_num(vdev, queue_no)) {
>               return -1;
> @@ -802,6 +803,10 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
>       return ret;
>   }
>   
> +static int kvm_virtio_pci_vector_config_use(VirtIOPCIProxy *proxy)
> +{
> +    return kvm_virtio_pci_vector_use_one(proxy, VIRTIO_CONFIG_IRQ_IDX);
> +}
>   
>   static void kvm_virtio_pci_vector_release_one(VirtIOPCIProxy *proxy,
>                                                 int queue_no)
> @@ -839,6 +844,11 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
>       }
>   }
>   
> +static void kvm_virtio_pci_vector_config_release(VirtIOPCIProxy *proxy)
> +{
> +    kvm_virtio_pci_vector_release_one(proxy, VIRTIO_CONFIG_IRQ_IDX);
> +}
> +
>   static int virtio_pci_one_vector_unmask(VirtIOPCIProxy *proxy,
>                                          unsigned int queue_no,
>                                          unsigned int vector,
> @@ -920,9 +930,17 @@ static int virtio_pci_vector_unmask(PCIDevice *dev, unsigned vector,
>           }
>           vq = virtio_vector_next_queue(vq);
>       }
> -
> +    /* unmask config intr */
> +    n = virtio_config_get_guest_notifier(vdev);
> +    ret = virtio_pci_one_vector_unmask(proxy, VIRTIO_CONFIG_IRQ_IDX, vector,
> +                                       msg, n);
> +    if (ret < 0) {
> +        goto undo_config;
> +    }
>       return 0;
> -
> +undo_config:
> +    n = virtio_config_get_guest_notifier(vdev);
> +    virtio_pci_one_vector_mask(proxy, VIRTIO_CONFIG_IRQ_IDX, vector, n);
>   undo:
>       vq = virtio_vector_first_queue(vdev, vector);
>       while (vq && unmasked >= 0) {
> @@ -956,6 +974,8 @@ static void virtio_pci_vector_mask(PCIDevice *dev, unsigned vector)
>           }
>           vq = virtio_vector_next_queue(vq);
>       }
> +    n = virtio_config_get_guest_notifier(vdev);
> +    virtio_pci_one_vector_mask(proxy, VIRTIO_CONFIG_IRQ_IDX, vector, n);
>   }
>   
>   static void virtio_pci_vector_poll(PCIDevice *dev,
> @@ -987,6 +1007,34 @@ static void virtio_pci_vector_poll(PCIDevice *dev,
>               msix_set_pending(dev, vector);
>           }
>       }
> +    /* poll the config intr */
> +    ret = virtio_pci_get_notifier(proxy, VIRTIO_CONFIG_IRQ_IDX, &notifier,
> +                                  &vector);
> +    if (ret < 0) {
> +        return;
> +    }
> +    if (vector < vector_start || vector >= vector_end ||
> +        !msix_is_masked(dev, vector)) {
> +        return;
> +    }
> +    if (k->guest_notifier_pending) {
> +        if (k->guest_notifier_pending(vdev, VIRTIO_CONFIG_IRQ_IDX)) {
> +            msix_set_pending(dev, vector);
> +        }
> +    } else if (event_notifier_test_and_clear(notifier)) {
> +        msix_set_pending(dev, vector);
> +    }
> +}
> +
> +void virtio_pci_set_guest_notifier_fd_handler(VirtIODevice *vdev, VirtQueue *vq,
> +                                              int n, bool assign,
> +                                              bool with_irqfd)
> +{
> +    if (n == VIRTIO_CONFIG_IRQ_IDX) {
> +        virtio_config_set_guest_notifier_fd_handler(vdev, assign, with_irqfd);
> +    } else {
> +        virtio_queue_set_guest_notifier_fd_handler(vq, assign, with_irqfd);
> +    }
>   }
>   
>   static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
> @@ -995,17 +1043,25 @@ static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
>       VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
>       VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
>       VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
> -    VirtQueue *vq = virtio_get_queue(vdev, n);
> -    EventNotifier *notifier = virtio_queue_get_guest_notifier(vq);
> +    VirtQueue *vq = NULL;
> +    EventNotifier *notifier = NULL;
> +
> +    if (n == VIRTIO_CONFIG_IRQ_IDX) {
> +        notifier = virtio_config_get_guest_notifier(vdev);
> +    } else {
> +        vq = virtio_get_queue(vdev, n);
> +        notifier = virtio_queue_get_guest_notifier(vq);
> +    }
>   
>       if (assign) {
>           int r = event_notifier_init(notifier, 0);
>           if (r < 0) {
>               return r;
>           }
> -        virtio_queue_set_guest_notifier_fd_handler(vq, true, with_irqfd);
> +        virtio_pci_set_guest_notifier_fd_handler(vdev, vq, n, true, with_irqfd);
>       } else {
> -        virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd);
> +        virtio_pci_set_guest_notifier_fd_handler(vdev, vq, n, false,
> +                                                 with_irqfd);
>           event_notifier_cleanup(notifier);
>       }
>   
> @@ -1047,6 +1103,7 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
>           msix_unset_vector_notifiers(&proxy->pci_dev);
>           if (proxy->vector_irqfd) {
>               kvm_virtio_pci_vector_release(proxy, nvqs);
> +            kvm_virtio_pci_vector_config_release(proxy);
>               g_free(proxy->vector_irqfd);
>               proxy->vector_irqfd = NULL;
>           }
> @@ -1062,7 +1119,11 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
>               goto assign_error;
>           }
>       }
> -
> +    r = virtio_pci_set_guest_notifier(d, VIRTIO_CONFIG_IRQ_IDX, assign,
> +                                      with_irqfd);
> +    if (r < 0) {
> +        goto config_assign_error;
> +    }
>       /* Must set vector notifier after guest notifier has been assigned */
>       if ((with_irqfd || k->guest_notifier_mask) && assign) {
>           if (with_irqfd) {
> @@ -1071,11 +1132,14 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
>                             msix_nr_vectors_allocated(&proxy->pci_dev));
>               r = kvm_virtio_pci_vector_use(proxy, nvqs);
>               if (r < 0) {
> -                goto assign_error;
> +                goto config_assign_error;
>               }
>           }
> -        r = msix_set_vector_notifiers(&proxy->pci_dev,
> -                                      virtio_pci_vector_unmask,
> +        r = kvm_virtio_pci_vector_config_use(proxy);
> +        if (r < 0) {
> +            goto config_error;
> +        }


This is crashing a QEMU TCG machine using vhost (no irqfd). Below is a fix
but I doubt it is complete.

Thanks,

C.


 From dde4d7b21c851a33d2d03bddd18464ae4e777a3f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= <clg@kaod.org>
Date: Sun, 9 Jan 2022 10:40:47 +0100
Subject: [PATCH] vhost: Fix support for configure interrupt
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

irqfd is not necessarily  in use (TCG using vhost).

Cc: Cindy Lu <lulu@redhat.com>
Fixes: d5d24d859c39 ("virtio-pci: add support for configure interrupt")
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
  hw/virtio/virtio-pci.c | 10 ++++++----
  1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 98fb5493ae06..39ebb042860d 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1134,10 +1134,12 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
              if (r < 0) {
                  goto config_assign_error;
              }
-        }
-        r = kvm_virtio_pci_vector_config_use(proxy);
-        if (r < 0) {
-            goto config_error;
+
+            r = kvm_virtio_pci_vector_config_use(proxy);
+            if (r < 0) {
+                abort();
+                goto config_error;
+            }
          }
          r = msix_set_vector_notifiers(&proxy->pci_dev, virtio_pci_vector_unmask,
                                        virtio_pci_vector_mask,
-- 
2.31.1


Re: [PULL v3 12/55] virtio-pci: add support for configure interrupt
Posted by Michael S. Tsirkin 4 years ago
On Sun, Jan 09, 2022 at 02:33:43PM +0100, Cédric Le Goater wrote:
> Hello Cindy,
> 
> On 1/8/22 02:04, Michael S. Tsirkin wrote:
> > From: Cindy Lu <lulu@redhat.com>
> > 
> > Add support for configure interrupt, The process is used kvm_irqfd_assign
> > to set the gsi to kernel. When the configure notifier was signal by
> > host, qemu will inject a msix interrupt to guest
> > 
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > Message-Id: <20211104164827.21911-11-lulu@redhat.com>
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >   hw/virtio/virtio-pci.h |  4 +-
> >   hw/virtio/virtio-pci.c | 92 ++++++++++++++++++++++++++++++++++++------
> >   2 files changed, 83 insertions(+), 13 deletions(-)
> > 
> > diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> > index 2446dcd9ae..b704acc5a8 100644
> > --- a/hw/virtio/virtio-pci.h
> > +++ b/hw/virtio/virtio-pci.h
> > @@ -251,5 +251,7 @@ void virtio_pci_types_register(const VirtioPCIDeviceTypeInfo *t);
> >    * @fixed_queues.
> >    */
> >   unsigned virtio_pci_optimal_num_queues(unsigned fixed_queues);
> > -
> > +void virtio_pci_set_guest_notifier_fd_handler(VirtIODevice *vdev, VirtQueue *vq,
> > +                                              int n, bool assign,
> > +                                              bool with_irqfd);
> >   #endif
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index 7201cf3dc1..98fb5493ae 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -727,7 +727,8 @@ static int virtio_pci_get_notifier(VirtIOPCIProxy *proxy, int queue_no,
> >       VirtQueue *vq;
> >       if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
> > -        return -1;
> > +        *n = virtio_config_get_guest_notifier(vdev);
> > +        *vector = vdev->config_vector;
> >       } else {
> >           if (!virtio_queue_get_num(vdev, queue_no)) {
> >               return -1;
> > @@ -802,6 +803,10 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
> >       return ret;
> >   }
> > +static int kvm_virtio_pci_vector_config_use(VirtIOPCIProxy *proxy)
> > +{
> > +    return kvm_virtio_pci_vector_use_one(proxy, VIRTIO_CONFIG_IRQ_IDX);
> > +}
> >   static void kvm_virtio_pci_vector_release_one(VirtIOPCIProxy *proxy,
> >                                                 int queue_no)
> > @@ -839,6 +844,11 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
> >       }
> >   }
> > +static void kvm_virtio_pci_vector_config_release(VirtIOPCIProxy *proxy)
> > +{
> > +    kvm_virtio_pci_vector_release_one(proxy, VIRTIO_CONFIG_IRQ_IDX);
> > +}
> > +
> >   static int virtio_pci_one_vector_unmask(VirtIOPCIProxy *proxy,
> >                                          unsigned int queue_no,
> >                                          unsigned int vector,
> > @@ -920,9 +930,17 @@ static int virtio_pci_vector_unmask(PCIDevice *dev, unsigned vector,
> >           }
> >           vq = virtio_vector_next_queue(vq);
> >       }
> > -
> > +    /* unmask config intr */
> > +    n = virtio_config_get_guest_notifier(vdev);
> > +    ret = virtio_pci_one_vector_unmask(proxy, VIRTIO_CONFIG_IRQ_IDX, vector,
> > +                                       msg, n);
> > +    if (ret < 0) {
> > +        goto undo_config;
> > +    }
> >       return 0;
> > -
> > +undo_config:
> > +    n = virtio_config_get_guest_notifier(vdev);
> > +    virtio_pci_one_vector_mask(proxy, VIRTIO_CONFIG_IRQ_IDX, vector, n);
> >   undo:
> >       vq = virtio_vector_first_queue(vdev, vector);
> >       while (vq && unmasked >= 0) {
> > @@ -956,6 +974,8 @@ static void virtio_pci_vector_mask(PCIDevice *dev, unsigned vector)
> >           }
> >           vq = virtio_vector_next_queue(vq);
> >       }
> > +    n = virtio_config_get_guest_notifier(vdev);
> > +    virtio_pci_one_vector_mask(proxy, VIRTIO_CONFIG_IRQ_IDX, vector, n);
> >   }
> >   static void virtio_pci_vector_poll(PCIDevice *dev,
> > @@ -987,6 +1007,34 @@ static void virtio_pci_vector_poll(PCIDevice *dev,
> >               msix_set_pending(dev, vector);
> >           }
> >       }
> > +    /* poll the config intr */
> > +    ret = virtio_pci_get_notifier(proxy, VIRTIO_CONFIG_IRQ_IDX, &notifier,
> > +                                  &vector);
> > +    if (ret < 0) {
> > +        return;
> > +    }
> > +    if (vector < vector_start || vector >= vector_end ||
> > +        !msix_is_masked(dev, vector)) {
> > +        return;
> > +    }
> > +    if (k->guest_notifier_pending) {
> > +        if (k->guest_notifier_pending(vdev, VIRTIO_CONFIG_IRQ_IDX)) {
> > +            msix_set_pending(dev, vector);
> > +        }
> > +    } else if (event_notifier_test_and_clear(notifier)) {
> > +        msix_set_pending(dev, vector);
> > +    }
> > +}
> > +
> > +void virtio_pci_set_guest_notifier_fd_handler(VirtIODevice *vdev, VirtQueue *vq,
> > +                                              int n, bool assign,
> > +                                              bool with_irqfd)
> > +{
> > +    if (n == VIRTIO_CONFIG_IRQ_IDX) {
> > +        virtio_config_set_guest_notifier_fd_handler(vdev, assign, with_irqfd);
> > +    } else {
> > +        virtio_queue_set_guest_notifier_fd_handler(vq, assign, with_irqfd);
> > +    }
> >   }
> >   static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
> > @@ -995,17 +1043,25 @@ static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
> >       VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
> >       VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> >       VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
> > -    VirtQueue *vq = virtio_get_queue(vdev, n);
> > -    EventNotifier *notifier = virtio_queue_get_guest_notifier(vq);
> > +    VirtQueue *vq = NULL;
> > +    EventNotifier *notifier = NULL;
> > +
> > +    if (n == VIRTIO_CONFIG_IRQ_IDX) {
> > +        notifier = virtio_config_get_guest_notifier(vdev);
> > +    } else {
> > +        vq = virtio_get_queue(vdev, n);
> > +        notifier = virtio_queue_get_guest_notifier(vq);
> > +    }
> >       if (assign) {
> >           int r = event_notifier_init(notifier, 0);
> >           if (r < 0) {
> >               return r;
> >           }
> > -        virtio_queue_set_guest_notifier_fd_handler(vq, true, with_irqfd);
> > +        virtio_pci_set_guest_notifier_fd_handler(vdev, vq, n, true, with_irqfd);
> >       } else {
> > -        virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd);
> > +        virtio_pci_set_guest_notifier_fd_handler(vdev, vq, n, false,
> > +                                                 with_irqfd);
> >           event_notifier_cleanup(notifier);
> >       }
> > @@ -1047,6 +1103,7 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
> >           msix_unset_vector_notifiers(&proxy->pci_dev);
> >           if (proxy->vector_irqfd) {
> >               kvm_virtio_pci_vector_release(proxy, nvqs);
> > +            kvm_virtio_pci_vector_config_release(proxy);
> >               g_free(proxy->vector_irqfd);
> >               proxy->vector_irqfd = NULL;
> >           }
> > @@ -1062,7 +1119,11 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
> >               goto assign_error;
> >           }
> >       }
> > -
> > +    r = virtio_pci_set_guest_notifier(d, VIRTIO_CONFIG_IRQ_IDX, assign,
> > +                                      with_irqfd);
> > +    if (r < 0) {
> > +        goto config_assign_error;
> > +    }
> >       /* Must set vector notifier after guest notifier has been assigned */
> >       if ((with_irqfd || k->guest_notifier_mask) && assign) {
> >           if (with_irqfd) {
> > @@ -1071,11 +1132,14 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
> >                             msix_nr_vectors_allocated(&proxy->pci_dev));
> >               r = kvm_virtio_pci_vector_use(proxy, nvqs);
> >               if (r < 0) {
> > -                goto assign_error;
> > +                goto config_assign_error;
> >               }
> >           }
> > -        r = msix_set_vector_notifiers(&proxy->pci_dev,
> > -                                      virtio_pci_vector_unmask,
> > +        r = kvm_virtio_pci_vector_config_use(proxy);
> > +        if (r < 0) {
> > +            goto config_error;
> > +        }
> 
> 
> This is crashing a QEMU TCG machine using vhost (no irqfd). Below is a fix
> but I doubt it is complete.
> 
> Thanks,
> 
> C.
> 
> 
> From dde4d7b21c851a33d2d03bddd18464ae4e777a3f Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= <clg@kaod.org>
> Date: Sun, 9 Jan 2022 10:40:47 +0100
> Subject: [PATCH] vhost: Fix support for configure interrupt
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> irqfd is not necessarily  in use (TCG using vhost).
> 
> Cc: Cindy Lu <lulu@redhat.com>
> Fixes: d5d24d859c39 ("virtio-pci: add support for configure interrupt")
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/virtio/virtio-pci.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 98fb5493ae06..39ebb042860d 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1134,10 +1134,12 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
>              if (r < 0) {
>                  goto config_assign_error;
>              }
> -        }
> -        r = kvm_virtio_pci_vector_config_use(proxy);
> -        if (r < 0) {
> -            goto config_error;
> +
> +            r = kvm_virtio_pci_vector_config_use(proxy);
> +            if (r < 0) {
> +                abort();
> +                goto config_error;
> +            }
>          }
>          r = msix_set_vector_notifiers(&proxy->pci_dev, virtio_pci_vector_unmask,
>                                        virtio_pci_vector_mask,
> -- 
> 2.31.1


Yes, we need to fix up cleanup too, but besides that it seems good,
right?

So like this then? Cindy?

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 98fb5493ae..722ce12b45 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1134,10 +1134,10 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
             if (r < 0) {
                 goto config_assign_error;
             }
-        }
-        r = kvm_virtio_pci_vector_config_use(proxy);
-        if (r < 0) {
-            goto config_error;
+            r = kvm_virtio_pci_vector_config_use(proxy);
+            if (r < 0) {
+                goto config_error;
+            }
         }
         r = msix_set_vector_notifiers(&proxy->pci_dev, virtio_pci_vector_unmask,
                                       virtio_pci_vector_mask,
@@ -1155,7 +1155,9 @@ notifiers_error:
         kvm_virtio_pci_vector_release(proxy, nvqs);
     }
 config_error:
-    kvm_virtio_pci_vector_config_release(proxy);
+    if (with_irqfd) {
+        kvm_virtio_pci_vector_config_release(proxy);
+    }
 config_assign_error:
     virtio_pci_set_guest_notifier(d, VIRTIO_CONFIG_IRQ_IDX, !assign,
                                   with_irqfd);