Re: [PATCH 3/3] virtio-pci: don't poll masked vectors

Asias He
2012-12-19T08:56:44Z

On 12/18/2012 08:39 PM, Michael S. Tsirkin wrote:
> At the moment, when irqfd is in use but a vector is masked,
> qemu will poll it and handle vector masks in userspace.
> Since almost no one ever looks at the pending bits,
> it is better to defer this until pending bits
> are actually read.
> Implement this optimization using the new poll notifier.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> hw/virtio-pci.c | 52 ++++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 40 insertions(+), 12 deletions(-)
>
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 1c03bb5..bc6b4e0 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -509,8 +509,6 @@ static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy *proxy,
> }
> return ret;
> }
> -
> - virtio_queue_set_guest_notifier_fd_handler(vq, true, true);
> return 0;
> }
>
> @@ -529,8 +527,6 @@ static void kvm_virtio_pci_vq_vector_release(VirtIOPCIProxy *proxy,
> if (--irqfd->users == 0) {
> kvm_irqchip_release_virq(kvm_state, irqfd->virq);
> }
> -
> - virtio_queue_set_guest_notifier_fd_handler(vq, true, false);
> }
>
> static int kvm_virtio_pci_vector_use(PCIDevice *dev, unsigned vector,
> @@ -581,7 +577,36 @@ static void kvm_virtio_pci_vector_release(PCIDevice *dev, unsigned vector)
> }
> }
>
> -static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign)
> +static void kvm_virtio_pci_vector_poll(PCIDevice *dev,
> + unsigned int vector_start,
> + unsigned int vector_end)
> +{
> + VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev);
> + VirtIODevice *vdev = proxy->vdev;
> + int queue_no;
> + unsigned int vector;
> + EventNotifier *notifier;
> + VirtQueue *vq;
> +
> + for (queue_no = 0; queue_no < VIRTIO_PCI_QUEUE_MAX; queue_no++) {
> + if (!virtio_queue_get_num(vdev, queue_no)) {
> + break;
> + }
> + vector = virtio_queue_vector(vdev, queue_no);
> + if (vector < vector_start || vector >= vector_end ||
> + !msix_is_masked(dev, vector)) {
> + continue;
> + }
> + vq = virtio_get_queue(vdev, queue_no);
> + notifier = virtio_queue_get_guest_notifier(vq);
> + if (event_notifier_test_and_clear(notifier)) {
> + msix_set_pending(dev, vector);
> + }
> + }
> +}
> +
> +static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
> + bool with_irqfd)
> {
> VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
> VirtQueue *vq = virtio_get_queue(proxy->vdev, n);
> @@ -592,9 +617,9 @@ static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign)
> if (r < 0) {
> return r;
> }
> - virtio_queue_set_guest_notifier_fd_handler(vq, true, false);
> + virtio_queue_set_guest_notifier_fd_handler(vq, true, with_irqfd);

+ virtio_queue_set_guest_notifier_fd_handler(vq, assign, with_irqfd);

> } else {
> - virtio_queue_set_guest_notifier_fd_handler(vq, false, false);
> + virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd);

+ virtio_queue_set_guest_notifier_fd_handler(vq, assign, with_irqfd);

Use the 'assign' variable instead of the constants?

> event_notifier_cleanup(notifier);
> }
>
> @@ -612,9 +637,11 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, bool assign)
> VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
> VirtIODevice *vdev = proxy->vdev;
> int r, n;
> + bool with_irqfd = msix_enabled(&proxy->pci_dev) &&
> + kvm_msi_via_irqfd_enabled();
>
> /* Must unset vector notifier while guest notifier is still assigned */
> - if (kvm_msi_via_irqfd_enabled() && !assign) {
> + if (with_irqfd && !assign) {
> msix_unset_vector_notifiers(&proxy->pci_dev);
> g_free(proxy->vector_irqfd);
> proxy->vector_irqfd = NULL;
> @@ -625,21 +652,22 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, bool assign)
> break;
> }
>
> - r = virtio_pci_set_guest_notifier(d, n, assign);
> + r = virtio_pci_set_guest_notifier(d, n, assign,
> + kvm_msi_via_irqfd_enabled());
> if (r < 0) {
> goto assign_error;
> }
> }
>
> /* Must set vector notifier after guest notifier has been assigned */
> - if (kvm_msi_via_irqfd_enabled() && assign) {
> + if (with_irqfd && assign) {
> proxy->vector_irqfd =
> g_malloc0(sizeof(*proxy->vector_irqfd) *
> msix_nr_vectors_allocated(&proxy->pci_dev));
> r = msix_set_vector_notifiers(&proxy->pci_dev,
> kvm_virtio_pci_vector_use,
> kvm_virtio_pci_vector_release,
> - NULL);
> + kvm_virtio_pci_vector_poll);
> if (r < 0) {
> goto assign_error;
> }
> @@ -651,7 +679,7 @@ assign_error:
> /* We get here on assignment failure. Recover by undoing for VQs 0 .. n. */
> assert(assign);
> while (--n >= 0) {
> - virtio_pci_set_guest_notifier(d, n, !assign);
> + virtio_pci_set_guest_notifier(d, n, !assign, with_irqfd);
> }
> return r;
> }
>

--
Asias