[Qemu-devel] [PATCH v2 2/3] virtio-mmio: introduce set_guest_notifiers

[Qemu-devel] [PATCH v2 2/3] virtio-mmio: introduce set_guest_notifiers

[Pavel Fedin]

Same as host notifier of virtio-mmio, most of codes came from virtio-pci.
The kvm-arm does not yet support irqfd, need to fix the hard-coded part
after
kvm-arm gets irqfd support.

Signed-off-by: Ying-Shiuan Pan <address@hidden>
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 hw/virtio/virtio-mmio.c | 61
+++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 6b40d56..8756240 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -394,6 +394,66 @@ static void virtio_mmio_reset(DeviceState *d)
     proxy->guest_page_shift = 0;
 }
 
+static int virtio_mmio_set_guest_notifier(DeviceState *d, int n, bool
assign,
+                                          bool with_irqfd)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(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);
+
+    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);
+    } else {
+        virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd);
+        event_notifier_cleanup(notifier);
+    }
+
+    if (vdc->guest_notifier_mask) {
+        vdc->guest_notifier_mask(vdev, n, !assign);
+    }
+
+    return 0;
+}
+
+static int virtio_mmio_set_guest_notifiers(DeviceState *d, int nvqs,
+                                           bool assign)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    /* TODO: need to check if kvm-arm supports irqfd */
+    bool with_irqfd = false;
+    int r, n;
+
+    nvqs = MIN(nvqs, VIRTIO_PCI_QUEUE_MAX);
+
+    for (n = 0; n < nvqs; n++) {
+        if (!virtio_queue_get_num(vdev, n)) {
+            break;
+        }
+
+        r = virtio_mmio_set_guest_notifier(d, n, assign, with_irqfd);
+        if (r < 0) {
+            goto assign_error;
+        }
+    }
+
+    return 0;
+
+assign_error:
+    /* We get here on assignment failure. Recover by undoing for VQs 0 ..
n. */
+    assert(assign);
+    while (--n >= 0) {
+        virtio_mmio_set_guest_notifier(d, n, !assign, false);
+    }
+    return r;
+}
+
 static int virtio_mmio_set_host_notifier(DeviceState *opaque, int n,
                                          bool assign)
 {
@@ -471,6 +531,7 @@ static void virtio_mmio_bus_class_init(ObjectClass
*klass, void *data)
     k->save_config = virtio_mmio_save_config;
     k->load_config = virtio_mmio_load_config;
     k->set_host_notifier = virtio_mmio_set_host_notifier;
+    k->set_guest_notifiers = virtio_mmio_set_guest_notifiers;
     k->get_features = virtio_mmio_get_features;
     k->device_plugged = virtio_mmio_device_plugged;
     k->has_variable_vring_alignment = true;
-- 
1.9.5.msysgit.0

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] [PATCH v2 2/3] virtio-mmio: introduce set_guest_notifiers

[Cornelia Huck]

On Thu, 07 May 2015 13:49:54 +0300
Pavel Fedin <p.fedin@samsung.com> wrote:

> Same as host notifier of virtio-mmio, most of codes came from virtio-pci.

Possibly another case for common helper functions, as I also copied a
lot of stuff from pci for ccw :)

> The kvm-arm does not yet support irqfd, need to fix the hard-coded part
> after
> kvm-arm gets irqfd support.

Hm, weren't there some patches for irqfd on arm?

> 
> Signed-off-by: Ying-Shiuan Pan <address@hidden>
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> ---
>  hw/virtio/virtio-mmio.c | 61
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)

Re: [Qemu-devel] [PATCH v2 2/3] virtio-mmio: introduce set_guest_notifiers

[Pavel Fedin]

 Hello!

> Hm, weren't there some patches for irqfd on arm?

 Yes, there were. However, they had a design problem by breaking backwards compatibility
with unmodified virtio. Their idea was to set up one more shared memory area between
virtio and vhost-net and use it to pass ISR value, which helps to distinguish, which event
took place (queue update or config change). So, this idea was rejected.
 http://lists.gnu.org/archive/html/qemu-devel/2014-10/msg03056.html

 I thought about it, and technically, i think, this can be done in better way. Actually,
as far as i understood, all we need is mechanism for distinguishing between these two
events. On PCI we do this by using multiple IRQs via MSI-X, and one IRQ signals exactly
one type of event. MSI-X code also has "two IRQs" mode as a failsafe, where one IRQ
signals config change and another IRQ signals queues update (and all queues are polled in
turn). I think a similar thing could be done for virtio-mmio. It could allocate two IRQs
instead of one and describe both of them in the device tree. Guest side, upon seeing that,
could make use of those two IRQs and acknowledge to the host side that "yes, i am new
version and use new mode".
 But, sorry, i will unlikely implement this, because we already have PCI with MSI-X (i
hope this is going to be published soon), so my project can use PCI emulation. So
implementing irqfds for virtio-mmio is a bit out of my scope.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] [PATCH v2 2/3] virtio-mmio: introduce set_guest_notifiers

[Cornelia Huck]

On Fri, 08 May 2015 09:45:00 +0300
Pavel Fedin <p.fedin@samsung.com> wrote:

>  Hello!
> 
> > Hm, weren't there some patches for irqfd on arm?
> 
>  Yes, there were. However, they had a design problem by breaking backwards compatibility
> with unmodified virtio. Their idea was to set up one more shared memory area between
> virtio and vhost-net and use it to pass ISR value, which helps to distinguish, which event
> took place (queue update or config change). So, this idea was rejected.
>  http://lists.gnu.org/archive/html/qemu-devel/2014-10/msg03056.html
> 
>  I thought about it, and technically, i think, this can be done in better way. Actually,
> as far as i understood, all we need is mechanism for distinguishing between these two
> events. On PCI we do this by using multiple IRQs via MSI-X, and one IRQ signals exactly
> one type of event. MSI-X code also has "two IRQs" mode as a failsafe, where one IRQ
> signals config change and another IRQ signals queues update (and all queues are polled in
> turn). I think a similar thing could be done for virtio-mmio. It could allocate two IRQs
> instead of one and describe both of them in the device tree. Guest side, upon seeing that,
> could make use of those two IRQs and acknowledge to the host side that "yes, i am new
> version and use new mode".
>  But, sorry, i will unlikely implement this, because we already have PCI with MSI-X (i
> hope this is going to be published soon), so my project can use PCI emulation. So
> implementing irqfds for virtio-mmio is a bit out of my scope.

Thanks for the explanation.

Yes, I think it makes sense to just pick the low-hanging fruit for
virtio-mmio and wait for pci.

Re: [Qemu-devel] [PATCH v2 2/3] virtio-mmio: introduce set_guest_notifiers

[Pavel Fedin]

 Hello!

> Yes, I think it makes sense to just pick the low-hanging fruit for
> virtio-mmio and wait for pci.

 Does this mean that my series can be accepted as it is? Since PCI is potentially better
solution, MMIO is a low priority in my project, and i have lots of other tasks. This means
i unfortunately don't have time for further refactor. If you ACK, i will resend the series
once again as v3, i set up git send-email and it should be working now.
 I just wanted to share this piece because it's already done, and i would not like it to
go to oblivion again.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


> -----Original Message-----
> From: qemu-devel-bounces+p.fedin=samsung.com@nongnu.org [mailto:qemu-devel-
> bounces+p.fedin=samsung.com@nongnu.org] On Behalf Of Cornelia Huck
> Sent: Friday, May 08, 2015 1:20 PM
> To: Pavel Fedin
> Cc: qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH v2 2/3] virtio-mmio: introduce set_guest_notifiers
> 
> On Fri, 08 May 2015 09:45:00 +0300
> Pavel Fedin <p.fedin@samsung.com> wrote:
> 
> >  Hello!
> >
> > > Hm, weren't there some patches for irqfd on arm?
> >
> >  Yes, there were. However, they had a design problem by breaking backwards
compatibility
> > with unmodified virtio. Their idea was to set up one more shared memory area between
> > virtio and vhost-net and use it to pass ISR value, which helps to distinguish, which
event
> > took place (queue update or config change). So, this idea was rejected.
> >  http://lists.gnu.org/archive/html/qemu-devel/2014-10/msg03056.html
> >
> >  I thought about it, and technically, i think, this can be done in better way.
Actually,
> > as far as i understood, all we need is mechanism for distinguishing between these two
> > events. On PCI we do this by using multiple IRQs via MSI-X, and one IRQ signals
exactly
> > one type of event. MSI-X code also has "two IRQs" mode as a failsafe, where one IRQ
> > signals config change and another IRQ signals queues update (and all queues are polled
in
> > turn). I think a similar thing could be done for virtio-mmio. It could allocate two
IRQs
> > instead of one and describe both of them in the device tree. Guest side, upon seeing
that,
> > could make use of those two IRQs and acknowledge to the host side that "yes, i am new
> > version and use new mode".
> >  But, sorry, i will unlikely implement this, because we already have PCI with MSI-X (i
> > hope this is going to be published soon), so my project can use PCI emulation. So
> > implementing irqfds for virtio-mmio is a bit out of my scope.
> 
> Thanks for the explanation.
> 

Re: [Qemu-devel] [PATCH v2 2/3] virtio-mmio: introduce set_guest_notifiers

[Cornelia Huck]

On Fri, 08 May 2015 13:30:18 +0300
Pavel Fedin <p.fedin@samsung.com> wrote:

>  Hello!
> 
> > Yes, I think it makes sense to just pick the low-hanging fruit for
> > virtio-mmio and wait for pci.
> 
>  Does this mean that my series can be accepted as it is? Since PCI is potentially better
> solution, MMIO is a low priority in my project, and i have lots of other tasks. This means
> i unfortunately don't have time for further refactor. If you ACK, i will resend the series
> once again as v3, i set up git send-email and it should be working now.
>  I just wanted to share this piece because it's already done, and i would not like it to
> go to oblivion again.

I think (ommitting the refactoring) you also need to care about reset.
(And maybe fold patches 1 and 3.) Should not be too much effort, I hope.

And I'd recommend to cc: MST, so that there is a chance of it not
getting lost in his after-vacation mail avalanche :)