On Wed, Jul 18, 2012 at 04:07:45PM +0100, Stefan Hajnoczi wrote:
> Optimize for the MSI-X enabled and vector unmasked case where it is
> possible to issue the KVM ioctl() directly instead of using irqfd.
Why? Is an ioctl faster?
> This patch introduces a new virtio binding function which tries to
> notify in a thread-safe way. If this is not possible, the function
> returns false. Virtio block then knows to use irqfd as a fallback.
> ---
> hw/msix.c | 17 +++++++++++++++++
> hw/msix.h | 1 +
> hw/virtio-blk.c | 10 ++++++++--
> hw/virtio-pci.c | 8 ++++++++
> hw/virtio.c | 9 +++++++++
> hw/virtio.h | 3 +++
> 6 files changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/hw/msix.c b/hw/msix.c
> index 7955221..3308604 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -503,6 +503,23 @@ void msix_notify(PCIDevice *dev, unsigned vector)
> stl_le_phys(address, data);
> }
>
> +bool msix_try_notify_from_thread(PCIDevice *dev, unsigned vector)
> +{
> + if (unlikely(vector >= dev->msix_entries_nr ||
> !dev->msix_entry_used[vector])) {
> + return false;
> + }
> + if (unlikely(msix_is_masked(dev, vector))) {
> + return false;
> + }
> +#ifdef KVM_CAP_IRQCHIP
> + if (likely(kvm_enabled() && kvm_irqchip_in_kernel())) {
> + kvm_set_irq(dev->msix_irq_entries[vector].gsi, 1, NULL);
> + return true;
> + }
> +#endif
> + return false;
> +}
> +
> void msix_reset(PCIDevice *dev)
> {
> if (!(dev->cap_present & QEMU_PCI_CAP_MSIX))
> diff --git a/hw/msix.h b/hw/msix.h
> index a8661e1..99fb08f 100644
> --- a/hw/msix.h
> +++ b/hw/msix.h
> @@ -26,6 +26,7 @@ void msix_vector_unuse(PCIDevice *dev, unsigned vector);
> void msix_unuse_all_vectors(PCIDevice *dev);
>
> void msix_notify(PCIDevice *dev, unsigned vector);
> +bool msix_try_notify_from_thread(PCIDevice *dev, unsigned vector);
>
> void msix_reset(PCIDevice *dev);
>
> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> index bdff68a..efeffa0 100644
> --- a/hw/virtio-blk.c
> +++ b/hw/virtio-blk.c
> @@ -82,6 +82,12 @@ static void virtio_blk_notify_guest(VirtIOBlock *s)
> !(s->vdev.guest_features & (1 << VIRTIO_F_NOTIFY_ON_EMPTY))))
> return;
>
> + /* Try to issue the ioctl() directly for speed */
> + if (likely(virtio_queue_try_notify_from_thread(s->vq))) {
> + return;
> + }
> +
> + /* If the fast path didn't work, use irqfd */
> event_notifier_set(virtio_queue_get_guest_notifier(s->vq));
> }
>
> @@ -263,7 +269,7 @@ static void data_plane_start(VirtIOBlock *s)
> vring_setup(&s->vring, &s->vdev, 0);
>
> /* Set up guest notifier (irq) */
> - if (s->vdev.binding->set_guest_notifier(s->vdev.binding_opaque, 0, true)
> != 0) {
> + if (s->vdev.binding->set_guest_notifiers(s->vdev.binding_opaque, true)
> != 0) {
> fprintf(stderr, "virtio-blk failed to set guest notifier, ensure
> -enable-kvm is set\n");
> exit(1);
> }
> @@ -315,7 +321,7 @@ static void data_plane_stop(VirtIOBlock *s)
> event_poll_cleanup(&s->event_poll);
>
> /* Clean up guest notifier (irq) */
> - s->vdev.binding->set_guest_notifier(s->vdev.binding_opaque, 0, false);
> + s->vdev.binding->set_guest_notifiers(s->vdev.binding_opaque, false);
> }
>
> static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t val)
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index f1e13af..03512b3 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -106,6 +106,13 @@ static void virtio_pci_notify(void *opaque, uint16_t
> vector)
> qemu_set_irq(proxy->pci_dev.irq[0], proxy->vdev->isr & 1);
> }
>
> +static bool virtio_pci_try_notify_from_thread(void *opaque, uint16_t vector)
> +{
> + VirtIOPCIProxy *proxy = opaque;
> + return msix_enabled(&proxy->pci_dev) &&
> + msix_try_notify_from_thread(&proxy->pci_dev, vector);
> +}
> +
> static void virtio_pci_save_config(void * opaque, QEMUFile *f)
> {
> VirtIOPCIProxy *proxy = opaque;
> @@ -707,6 +714,7 @@ static void virtio_pci_vmstate_change(void *opaque, bool
> running)
>
> static const VirtIOBindings virtio_pci_bindings = {
> .notify = virtio_pci_notify,
> + .try_notify_from_thread = virtio_pci_try_notify_from_thread,
> .save_config = virtio_pci_save_config,
> .load_config = virtio_pci_load_config,
> .save_queue = virtio_pci_save_queue,
> diff --git a/hw/virtio.c b/hw/virtio.c
> index 064aecf..a1d1a8a 100644
> --- a/hw/virtio.c
> +++ b/hw/virtio.c
> @@ -689,6 +689,15 @@ static inline int vring_need_event(uint16_t event,
> uint16_t new, uint16_t old)
> return (uint16_t)(new - event - 1) < (uint16_t)(new - old);
> }
>
> +bool virtio_queue_try_notify_from_thread(VirtQueue *vq)
> +{
> + VirtIODevice *vdev = vq->vdev;
> + if (likely(vdev->binding->try_notify_from_thread)) {
> + return vdev->binding->try_notify_from_thread(vdev->binding_opaque,
> vq->vector);
> + }
> + return false;
> +}
> +
> static bool vring_notify(VirtIODevice *vdev, VirtQueue *vq)
> {
> uint16_t old, new;
> diff --git a/hw/virtio.h b/hw/virtio.h
> index 400c092..2cdf2be 100644
> --- a/hw/virtio.h
> +++ b/hw/virtio.h
> @@ -93,6 +93,7 @@ typedef struct VirtQueueElement
>
> typedef struct {
> void (*notify)(void * opaque, uint16_t vector);
> + bool (*try_notify_from_thread)(void * opaque, uint16_t vector);
> void (*save_config)(void * opaque, QEMUFile *f);
> void (*save_queue)(void * opaque, int n, QEMUFile *f);
> int (*load_config)(void * opaque, QEMUFile *f);
> @@ -160,6 +161,8 @@ void virtio_cleanup(VirtIODevice *vdev);
>
> void virtio_notify_config(VirtIODevice *vdev);
>
> +bool virtio_queue_try_notify_from_thread(VirtQueue *vq);
> +
> void virtio_queue_set_notification(VirtQueue *vq, int enable);
>
> int virtio_queue_ready(VirtQueue *vq);
> --
> 1.7.10.4
