[Qemu-devel] [PATCH 0/4] ioeventfd support for virtio-mmio

[Qemu-devel] [PATCH 0/4] ioeventfd support for virtio-mmio

[Ying-Shiuan Pan]

I added host/guest notifiers support for virtio-mmio, so that qemu can
enable vhost-net for kvm-arm. Sorry for that I cannot measure the performance
difference because I have only Exynos-5250 arndale boards which only provide
a 100 Mbps Ethernet. But in our previous work in kvm-arm (refer: Evaluation of
a Server-Grade Software-Only ARM Hypervisor), the vhost-net increased at least
30% in throughput comparing to virtio-net.

Ying-Shiuan Pan (4):
  virtio-mmio: introduce set_host_notifier()
  virtio-mmio: introduce set_guest_notifiers
  virtio-mmio: start ioeventfd when status gets DRIVER_OK
  virtio-mmio: add a new property for ioeventfd

 hw/block/virtio-blk.c      |   1 +
 hw/net/virtio-net.c        |   1 +
 hw/virtio/virtio-mmio.c    | 176 +++++++++++++++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio.h |   1 +
 4 files changed, 179 insertions(+)

-- 
1.8.1.2

[Qemu-devel] [PATCH 1/4] virtio-mmio: introduce set_host_notifier()

[Ying-Shiuan Pan]

set_host_notifier() is introduced into virtio-mmio now. Most of codes came
from virtio-pci.

Signed-off-by: Ying-Shiuan Pan <yspan@itri.org.tw>
---
 hw/virtio/virtio-mmio.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 8829eb0..558fd2f 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -23,6 +23,7 @@
 #include "hw/virtio/virtio.h"
 #include "qemu/host-utils.h"
 #include "hw/virtio/virtio-bus.h"
+#include "qemu/error-report.h"
 
 /* #define DEBUG_VIRTIO_MMIO */
 
@@ -87,11 +88,61 @@ typedef struct {
     uint32_t guest_page_shift;
     /* virtio-bus */
     VirtioBusState bus;
+    bool ioeventfd_disabled;
+    bool ioeventfd_started;
 } VirtIOMMIOProxy;
 
 static void virtio_mmio_bus_new(VirtioBusState *bus, size_t bus_size,
                                 VirtIOMMIOProxy *dev);
 
+static int virtio_mmio_set_host_notifier_internal(VirtIOMMIOProxy *proxy,
+                                                 int n, bool assign, bool set_handler)
+{
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    VirtQueue *vq = virtio_get_queue(vdev, n);
+    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
+    int r = 0;
+
+    if (assign) {
+        r = event_notifier_init(notifier, 1);
+        if (r < 0) {
+            error_report("%s: unable to init event notifier: %d",
+                         __func__, r);
+            return r;
+        }
+        virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
+        memory_region_add_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
+                                  true, n, notifier);
+    } else {
+        memory_region_del_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
+                                  true, n, notifier);
+        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
+        event_notifier_cleanup(notifier);
+    }
+    return r;
+}
+
+static void virtio_mmio_stop_ioeventfd(VirtIOMMIOProxy *proxy)
+{
+    int r;
+    int n;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+
+    if (!proxy->ioeventfd_started) {
+        return;
+    }
+
+    for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
+        if (!virtio_queue_get_num(vdev, n)) {
+            continue;
+        }
+
+        r = virtio_mmio_set_host_notifier_internal(proxy, n, false, false);
+        assert(r >= 0);
+    }
+    proxy->ioeventfd_started = false;
+}
+
 static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size)
 {
     VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
@@ -345,6 +396,24 @@ static void virtio_mmio_reset(DeviceState *d)
     proxy->guest_page_shift = 0;
 }
 
+static int virtio_mmio_set_host_notifier(DeviceState *opaque, int n, bool assign)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
+
+    /* Stop using ioeventfd for virtqueue kick if the device starts using host
+     * notifiers.  This makes it easy to avoid stepping on each others' toes.
+     */
+    proxy->ioeventfd_disabled = assign;
+    if (assign) {
+        virtio_mmio_stop_ioeventfd(proxy);
+    }
+    /* We don't need to start here: it's not needed because backend
+     * currently only stops on status change away from ok,
+     * reset, vmstop and such. If we do add code to start here,
+     * need to check vmstate, device state etc. */
+    return virtio_mmio_set_host_notifier_internal(proxy, n, assign, false);
+}
+
 /* virtio-mmio device */
 
 /* This is called by virtio-bus just after the device is plugged. */
@@ -406,6 +475,7 @@ static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data)
     k->notify = virtio_mmio_update_irq;
     k->save_config = virtio_mmio_save_config;
     k->load_config = virtio_mmio_load_config;
+    k->set_host_notifier = virtio_mmio_set_host_notifier;
     k->get_features = virtio_mmio_get_features;
     k->device_plugged = virtio_mmio_device_plugged;
     k->has_variable_vring_alignment = true;
-- 
1.8.1.2

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

[Ying-Shiuan Pan]

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 <yspan@itri.org.tw>
---
 hw/virtio/virtio-mmio.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 558fd2f..f95b7dd 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -396,6 +396,65 @@ 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)
 {
     VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
@@ -476,6 +535,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.8.1.2

[Qemu-devel] [PATCH 3/4] virtio-mmio: start ioeventfd when status gets DRIVER_OK

[Ying-Shiuan Pan]

Signed-off-by: Ying-Shiuan Pan <yspan@itri.org.tw>
---
 hw/virtio/virtio-mmio.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index f95b7dd..11964ee 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -122,6 +122,42 @@ static int virtio_mmio_set_host_notifier_internal(VirtIOMMIOProxy *proxy,
     return r;
 }
 
+static void virtio_mmio_start_ioeventfd(VirtIOMMIOProxy *proxy)
+{
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    int n, r;
+
+    if (proxy->ioeventfd_disabled ||
+        proxy->ioeventfd_started) {
+        return;
+    }
+
+    for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
+        if (!virtio_queue_get_num(vdev, n)) {
+            continue;
+        }
+
+        r = virtio_mmio_set_host_notifier_internal(proxy, n, true, true);
+        if (r < 0) {
+            goto assign_error;
+        }
+    }
+    proxy->ioeventfd_started = true;
+    return;
+
+assign_error:
+    while (--n >= 0) {
+        if (!virtio_queue_get_num(vdev, n)) {
+            continue;
+        }
+
+        r = virtio_mmio_set_host_notifier_internal(proxy, n, false, false);
+        assert(r >= 0);
+    }
+    proxy->ioeventfd_started = false;
+    error_report("%s: failed. Fallback to a userspace (slower).", __func__);
+}
+
 static void virtio_mmio_stop_ioeventfd(VirtIOMMIOProxy *proxy)
 {
     int r;
@@ -320,7 +356,16 @@ static void virtio_mmio_write(void *opaque, hwaddr offset, uint64_t value,
         virtio_update_irq(vdev);
         break;
     case VIRTIO_MMIO_STATUS:
+        if (!(value & VIRTIO_CONFIG_S_DRIVER_OK)) {
+            virtio_mmio_stop_ioeventfd(proxy);
+        }
+
         virtio_set_status(vdev, value & 0xff);
+
+        if (value & VIRTIO_CONFIG_S_DRIVER_OK) {
+            virtio_mmio_start_ioeventfd(proxy);
+        }
+
         if (vdev->status == 0) {
             virtio_reset(vdev);
         }
-- 
1.8.1.2

[Qemu-devel] [PATCH 4/4] virtio-mmio: add a new property for ioeventfd

[Ying-Shiuan Pan]

Make ioeventfd could be enabled or disabled (default). Since ioeventfd
is not yet ready in kvm-arm, without this option, qemu will get a
problem if it attempts to initialize ioeventfd.

To coincide this option of virtio-mmio with virtio-pci, I would like to
also move VIRTIO_PCI_FLAG_USE_IOEVENTFD to 'bool ioeventfd' of VirtIODevice.
Any better suggestion?

Signed-off-by: Ying-Shiuan Pan <yspan@itri.org.tw>
---
 hw/block/virtio-blk.c      | 1 +
 hw/net/virtio-net.c        | 1 +
 hw/virtio/virtio-mmio.c    | 3 ++-
 include/hw/virtio/virtio.h | 1 +
 4 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 19d0961..befdfdf 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -756,6 +756,7 @@ static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp)
 
 static Property virtio_blk_properties[] = {
     DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlock, blk),
+    DEFINE_PROP_BOOL("ioeventfd", VirtIOBlock, parent_obj.use_ioeventfd, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 3626608..9b75e61 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1631,6 +1631,7 @@ static Property virtio_net_properties[] = {
                                                TX_TIMER_INTERVAL),
     DEFINE_PROP_INT32("x-txburst", VirtIONet, net_conf.txburst, TX_BURST),
     DEFINE_PROP_STRING("tx", VirtIONet, net_conf.tx),
+    DEFINE_PROP_BOOL("ioeventfd", VirtIONet, parent_obj.use_ioeventfd, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 11964ee..3f7a341 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -127,7 +127,8 @@ static void virtio_mmio_start_ioeventfd(VirtIOMMIOProxy *proxy)
     VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
     int n, r;
 
-    if (proxy->ioeventfd_disabled ||
+    if (!vdev->use_ioeventfd ||
+        proxy->ioeventfd_disabled ||
         proxy->ioeventfd_started) {
         return;
     }
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 3e54e90..d4cb3bf 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -121,6 +121,7 @@ struct VirtIODevice
     bool vm_running;
     VMChangeStateEntry *vmstate;
     char *bus_name;
+    bool use_ioeventfd;
 };
 
 typedef struct VirtioDeviceClass {
-- 
1.8.1.2

[Qemu-devel] [PATCH 0/4] ioeventfd support for virtio-mmio

[Shannon Zhao]

Add host/guest notifiers support for virtio-mmio, so that qemu can
enable vhost-net for kvm-arm.

Refer to the patches from Ying-Shiuan Pan
https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg00715.html

As vhost-net can improve the net performance by about 30%,
so I think it's necessary to make virtio-mmio work with vhost-net
on arm/arm64.

Shannon Zhao (4):
  virtio-mmio: introduce set_host_notifier()
  virtio-mmio: introduce set_guest_notifiers
  virtio-mmio: start ioeventfd when status gets DRIVER_OK
  virtio-mmio: add a new property for ioeventfd

 hw/net/virtio-net.c        |    1 +
 hw/virtio/virtio-mmio.c    |  176 ++++++++++++++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio.h |    1 +
 3 files changed, 178 insertions(+), 0 deletions(-)

[Qemu-devel] [PATCH 1/4] virtio-mmio: introduce set_host_notifier()

[Shannon Zhao]

set_host_notifier() is introduced into virtio-mmio now. Most of codes came
from virtio-pci.

Signed-off-by: Ying-Shiuan Pan <yingshiuan.pan@gmail.com>
Signed-off-by: Li Liu <john.liuli@huawei.com>
Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
---
 hw/virtio/virtio-mmio.c |   70 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 2450c13..d8ec2d1 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -23,6 +23,7 @@
 #include "hw/virtio/virtio.h"
 #include "qemu/host-utils.h"
 #include "hw/virtio/virtio-bus.h"
+#include "qemu/error-report.h"
 
 /* #define DEBUG_VIRTIO_MMIO */
 
@@ -87,8 +88,58 @@ typedef struct {
     uint32_t guest_page_shift;
     /* virtio-bus */
     VirtioBusState bus;
+    bool ioeventfd_disabled;
+    bool ioeventfd_started;
 } VirtIOMMIOProxy;
 
+static int virtio_mmio_set_host_notifier_internal(VirtIOMMIOProxy *proxy,
+                                                 int n, bool assign, bool set_handler)
+{
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    VirtQueue *vq = virtio_get_queue(vdev, n);
+    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
+    int r = 0;
+
+    if (assign) {
+        r = event_notifier_init(notifier, 1);
+        if (r < 0) {
+            error_report("%s: unable to init event notifier: %d",
+                         __func__, r);
+            return r;
+        }
+        virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
+        memory_region_add_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
+                                  true, n, notifier);
+    } else {
+        memory_region_del_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
+                                  true, n, notifier);
+        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
+        event_notifier_cleanup(notifier);
+    }
+    return r;
+}
+
+static void virtio_mmio_stop_ioeventfd(VirtIOMMIOProxy *proxy)
+{
+    int r;
+    int n;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+
+    if (!proxy->ioeventfd_started) {
+        return;
+    }
+
+    for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
+        if (!virtio_queue_get_num(vdev, n)) {
+            continue;
+        }
+
+        r = virtio_mmio_set_host_notifier_internal(proxy, n, false, false);
+        assert(r >= 0);
+    }
+    proxy->ioeventfd_started = false;
+}
+
 static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size)
 {
     VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
@@ -342,6 +393,24 @@ static void virtio_mmio_reset(DeviceState *d)
     proxy->guest_page_shift = 0;
 }
 
+static int virtio_mmio_set_host_notifier(DeviceState *opaque, int n, bool assign)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
+
+    /* Stop using ioeventfd for virtqueue kick if the device starts using host
+     * notifiers.  This makes it easy to avoid stepping on each others' toes.
+     */
+    proxy->ioeventfd_disabled = assign;
+    if (assign) {
+        virtio_mmio_stop_ioeventfd(proxy);
+    }
+    /* We don't need to start here: it's not needed because backend
+     * currently only stops on status change away from ok,
+     * reset, vmstop and such. If we do add code to start here,
+     * need to check vmstate, device state etc. */
+    return virtio_mmio_set_host_notifier_internal(proxy, n, assign, false);
+}
+
 /* virtio-mmio device */
 
 /* This is called by virtio-bus just after the device is plugged. */
@@ -399,6 +468,7 @@ static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data)
     k->notify = virtio_mmio_update_irq;
     k->save_config = virtio_mmio_save_config;
     k->load_config = virtio_mmio_load_config;
+    k->set_host_notifier = virtio_mmio_set_host_notifier;
     k->get_features = virtio_mmio_get_features;
     k->device_plugged = virtio_mmio_device_plugged;
     k->has_variable_vring_alignment = true;
-- 
1.7.1

Re: [Qemu-devel] [PATCH 1/4] virtio-mmio: introduce set_host_notifier()

[Fam Zheng]

On Tue, 11/04 20:47, Shannon Zhao wrote:
> set_host_notifier() is introduced into virtio-mmio now. Most of codes came
> from virtio-pci.
> 
> Signed-off-by: Ying-Shiuan Pan <yingshiuan.pan@gmail.com>
> Signed-off-by: Li Liu <john.liuli@huawei.com>
> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> ---
>  hw/virtio/virtio-mmio.c |   70 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 70 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> index 2450c13..d8ec2d1 100644
> --- a/hw/virtio/virtio-mmio.c
> +++ b/hw/virtio/virtio-mmio.c
> @@ -23,6 +23,7 @@
>  #include "hw/virtio/virtio.h"
>  #include "qemu/host-utils.h"
>  #include "hw/virtio/virtio-bus.h"
> +#include "qemu/error-report.h"
>  
>  /* #define DEBUG_VIRTIO_MMIO */
>  
> @@ -87,8 +88,58 @@ typedef struct {
>      uint32_t guest_page_shift;
>      /* virtio-bus */
>      VirtioBusState bus;
> +    bool ioeventfd_disabled;
> +    bool ioeventfd_started;
>  } VirtIOMMIOProxy;
>  
> +static int virtio_mmio_set_host_notifier_internal(VirtIOMMIOProxy *proxy,
> +                                                 int n, bool assign, bool set_handler)

I didn't review the code, but checkpatch.pl noticed this line and one more
below [*] is too long (over 80 columes).

> +{
> +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> +    VirtQueue *vq = virtio_get_queue(vdev, n);
> +    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
> +    int r = 0;
> +
> +    if (assign) {
> +        r = event_notifier_init(notifier, 1);
> +        if (r < 0) {
> +            error_report("%s: unable to init event notifier: %d",
> +                         __func__, r);
> +            return r;
> +        }
> +        virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
> +        memory_region_add_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
> +                                  true, n, notifier);
> +    } else {
> +        memory_region_del_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
> +                                  true, n, notifier);
> +        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
> +        event_notifier_cleanup(notifier);
> +    }
> +    return r;
> +}
> +
> +static void virtio_mmio_stop_ioeventfd(VirtIOMMIOProxy *proxy)
> +{
> +    int r;
> +    int n;
> +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> +
> +    if (!proxy->ioeventfd_started) {
> +        return;
> +    }
> +
> +    for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
> +        if (!virtio_queue_get_num(vdev, n)) {
> +            continue;
> +        }
> +
> +        r = virtio_mmio_set_host_notifier_internal(proxy, n, false, false);
> +        assert(r >= 0);
> +    }
> +    proxy->ioeventfd_started = false;
> +}
> +
>  static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size)
>  {
>      VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
> @@ -342,6 +393,24 @@ static void virtio_mmio_reset(DeviceState *d)
>      proxy->guest_page_shift = 0;
>  }
>  
> +static int virtio_mmio_set_host_notifier(DeviceState *opaque, int n, bool assign)

[*]

No need to respin yet just for this. Please wait for a serious review.

Thanks,

Fam

Re: [Qemu-devel] [PATCH 1/4] virtio-mmio: introduce set_host_notifier()

[Shannon Zhao]

On 2014/11/19 15:47, Fam Zheng wrote:
> On Tue, 11/04 20:47, Shannon Zhao wrote:
>> set_host_notifier() is introduced into virtio-mmio now. Most of codes came
>> from virtio-pci.
>>
>> Signed-off-by: Ying-Shiuan Pan <yingshiuan.pan@gmail.com>
>> Signed-off-by: Li Liu <john.liuli@huawei.com>
>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>> ---
>>  hw/virtio/virtio-mmio.c |   70 +++++++++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 70 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
>> index 2450c13..d8ec2d1 100644
>> --- a/hw/virtio/virtio-mmio.c
>> +++ b/hw/virtio/virtio-mmio.c
>> @@ -23,6 +23,7 @@
>>  #include "hw/virtio/virtio.h"
>>  #include "qemu/host-utils.h"
>>  #include "hw/virtio/virtio-bus.h"
>> +#include "qemu/error-report.h"
>>  
>>  /* #define DEBUG_VIRTIO_MMIO */
>>  
>> @@ -87,8 +88,58 @@ typedef struct {
>>      uint32_t guest_page_shift;
>>      /* virtio-bus */
>>      VirtioBusState bus;
>> +    bool ioeventfd_disabled;
>> +    bool ioeventfd_started;
>>  } VirtIOMMIOProxy;
>>  
>> +static int virtio_mmio_set_host_notifier_internal(VirtIOMMIOProxy *proxy,
>> +                                                 int n, bool assign, bool set_handler)
> 
> I didn't review the code, but checkpatch.pl noticed this line and one more
> below [*] is too long (over 80 columes).
> 
>> +{
>> +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
>> +    VirtQueue *vq = virtio_get_queue(vdev, n);
>> +    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
>> +    int r = 0;
>> +
>> +    if (assign) {
>> +        r = event_notifier_init(notifier, 1);
>> +        if (r < 0) {
>> +            error_report("%s: unable to init event notifier: %d",
>> +                         __func__, r);
>> +            return r;
>> +        }
>> +        virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
>> +        memory_region_add_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
>> +                                  true, n, notifier);
>> +    } else {
>> +        memory_region_del_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY, 4,
>> +                                  true, n, notifier);
>> +        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
>> +        event_notifier_cleanup(notifier);
>> +    }
>> +    return r;
>> +}
>> +
>> +static void virtio_mmio_stop_ioeventfd(VirtIOMMIOProxy *proxy)
>> +{
>> +    int r;
>> +    int n;
>> +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
>> +
>> +    if (!proxy->ioeventfd_started) {
>> +        return;
>> +    }
>> +
>> +    for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
>> +        if (!virtio_queue_get_num(vdev, n)) {
>> +            continue;
>> +        }
>> +
>> +        r = virtio_mmio_set_host_notifier_internal(proxy, n, false, false);
>> +        assert(r >= 0);
>> +    }
>> +    proxy->ioeventfd_started = false;
>> +}
>> +
>>  static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size)
>>  {
>>      VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
>> @@ -342,6 +393,24 @@ static void virtio_mmio_reset(DeviceState *d)
>>      proxy->guest_page_shift = 0;
>>  }
>>  
>> +static int virtio_mmio_set_host_notifier(DeviceState *opaque, int n, bool assign)
> 
> [*]
> 
> No need to respin yet just for this. Please wait for a serious review.
> 

Ok,thanks,
Shannon

> Thanks,
> 
> Fam
> 
> .
> 


-- 
Shannon

[Qemu-devel] [PATCH 1/4] virtio-mmio: introduce set_host_notifier()

[Pavel Fedin]

set_host_notifier() is introduced into virtio-mmio now. Most of codes came
from virtio-pci.

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

diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 10123f3..32bf240 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -23,6 +23,7 @@
 #include "hw/virtio/virtio.h"
 #include "qemu/host-utils.h"
 #include "hw/virtio/virtio-bus.h"
+#include "qemu/error-report.h"
 
 /* #define DEBUG_VIRTIO_MMIO */
 
@@ -87,8 +88,58 @@ typedef struct {
     uint32_t guest_page_shift;
     /* virtio-bus */
     VirtioBusState bus;
+    bool ioeventfd_disabled;
+    bool ioeventfd_started;
 } VirtIOMMIOProxy;
 
+static int virtio_mmio_set_host_notifier_internal(VirtIOMMIOProxy *proxy,
+                                                  int n, bool assign, bool
set_handler)
+{
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+    VirtQueue *vq = virtio_get_queue(vdev, n);
+    EventNotifier *notifier = virtio_queue_get_host_notifier(vq);
+    int r = 0;
+ 
+    if (assign) {
+        r = event_notifier_init(notifier, 1);
+        if (r < 0) {
+            error_report("%s: unable to init event notifier: %d",
+                         __func__, r);
+            return r;
+        }
+        virtio_queue_set_host_notifier_fd_handler(vq, true, set_handler);
+        memory_region_add_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY,
4,
+                                  true, n, notifier);
+    } else {
+        memory_region_del_eventfd(&proxy->iomem, VIRTIO_MMIO_QUEUENOTIFY,
4,
+                                  true, n, notifier);
+        virtio_queue_set_host_notifier_fd_handler(vq, false, false);
+        event_notifier_cleanup(notifier);
+    }
+    return r;
+}
+ 
+static void virtio_mmio_stop_ioeventfd(VirtIOMMIOProxy *proxy)
+{
+    int r;
+    int n;
+    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+
+    if (!proxy->ioeventfd_started) {
+        return;
+    }
+
+    for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
+        if (!virtio_queue_get_num(vdev, n)) {
+            continue;
+        }
+
+        r = virtio_mmio_set_host_notifier_internal(proxy, n, false, false);
+        assert(r >= 0);
+    }
+    proxy->ioeventfd_started = false;
+}
+
 static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned
size)
 {
     VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
@@ -342,6 +393,24 @@ static void virtio_mmio_reset(DeviceState *d)
     proxy->guest_page_shift = 0;
 }
 
+static int virtio_mmio_set_host_notifier(DeviceState *opaque, int n, bool
assign)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
+
+    /* Stop using ioeventfd for virtqueue kick if the device starts using
host
+     * notifiers.  This makes it easy to avoid stepping on each others'
toes.
+     */
+    proxy->ioeventfd_disabled = assign;
+    if (assign) {
+        virtio_mmio_stop_ioeventfd(proxy);
+    }
+    /* We don't need to start here: it's not needed because backend
+     * currently only stops on status change away from ok,
+     * reset, vmstop and such. If we do add code to start here,
+     * need to check vmstate, device state etc. */
+    return virtio_mmio_set_host_notifier_internal(proxy, n, assign, false);
+}
+
 /* virtio-mmio device */
 
 /* This is called by virtio-bus just after the device is plugged. */
@@ -399,6 +468,7 @@ static void virtio_mmio_bus_class_init(ObjectClass
*klass, void *data)
     k->notify = virtio_mmio_update_irq;
     k->save_config = virtio_mmio_save_config;
     k->load_config = virtio_mmio_load_config;
+    k->set_host_notifier = virtio_mmio_set_host_notifier;
     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

Re: [Qemu-devel] [PATCH 1/4] virtio-mmio: introduce set_host_notifier()

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 948 bytes --]

On 05/06/2015 01:52 AM, Pavel Fedin wrote:

[meta-comment]

> set_host_notifier() is introduced into virtio-mmio now. Most of codes came
> from virtio-pci.
> 

Missing an 'In-Reply-To: <000501d087d0$39c1bd90$ad4538b0$@samsung.com>'
header, which makes your series unthreaded (every message appeared as
its own thread).  It makes review easier if everything is properly
threaded to the cover letter, so you may want to send a series to
yourself first as a test case of your setup before sending to the list.

Other submission hints at: http://wiki.qemu.org/Contribute/SubmitAPatch

>  
> +static int virtio_mmio_set_host_notifier_internal(VirtIOMMIOProxy *proxy,
> +                                                  int n, bool assign, bool
> set_handler)

Long line, you might want to wrap it to fit in 80 columns.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]