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

[Cornelia Huck]

On Thu, 07 May 2015 13:51:27 +0300
Pavel Fedin <address@hidden> wrote:

Would it make sense to merge this with the first patch, so that you
introduce ioeventfds in one go?

> Signed-off-by: Ying-Shiuan Pan <address@hidden>
> Signed-off-by: Pavel Fedin <address@hidden>
> ---
>  hw/virtio/virtio-mmio.c | 47
> +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> index 8756240..0ab270d 100644
> --- a/hw/virtio/virtio-mmio.c
> +++ b/hw/virtio/virtio-mmio.c
> @@ -22,6 +22,7 @@
>  #include "hw/sysbus.h"
>  #include "hw/virtio/virtio.h"
>  #include "qemu/host-utils.h"
> +#include "sysemu/kvm.h"
>  #include "hw/virtio/virtio-bus.h"
>  #include "qemu/error-report.h"
> 
> @@ -120,6 +121,43 @@ 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 (!kvm_eventfds_enabled() ||
> +        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__);
> +}

Again, much commonality with pci and ccw, with two differences:

- the check at the beginning of the function (easily factored out)
- ccw disables ioeventfds on failure, pci doesn't and mmio can't as
  they don't have a device where they can tack an ioeventfd property bit
  on - this means mmio always uses ioeventfds for all devices if
  supported, no?

> +
>  static void virtio_mmio_stop_ioeventfd(VirtIOMMIOProxy *proxy)
>  {
>      int r;
> @@ -318,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);
>          }

Don't you also want to stop the ioeventfd on reset?