| Message ID | 20101006145650.GA10968@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, 2010-10-06 at 16:56 +0200, Michael S. Tsirkin wrote: > vhost irqfd support: case where many vqs are > mapped to a single msix vector is currently broken. > Fix it up. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > > This is on top of the qemu patchset, which is unchanged. > Fixes from v1: > correct error handling > > hw/msix.c | 68 ++++++++++++++++++++++++++++++++++++++----------------- > hw/msix.h | 4 +- > hw/pci.h | 3 +- > hw/virtio-pci.c | 56 ++++++++++++++++++++++++++++++++++++++------- > 4 files changed, 97 insertions(+), 34 deletions(-) > > diff --git a/hw/msix.c b/hw/msix.c > index 3dd0456..3d4dd61 100644 > --- a/hw/msix.c > +++ b/hw/msix.c > @@ -300,10 +300,8 @@ static void msix_mmio_writel(void *opaque, target_phys_addr_t addr, > if (kvm_enabled() && kvm_irqchip_in_kernel()) { > kvm_msix_update(dev, vector, was_masked, msix_is_masked(dev, vector)); > } > - if (was_masked != msix_is_masked(dev, vector) && > - dev->msix_mask_notifier && dev->msix_mask_notifier_opaque[vector]) { > + if (was_masked != msix_is_masked(dev, vector) && dev->msix_mask_notifier) { > int r = dev->msix_mask_notifier(dev, vector, > - dev->msix_mask_notifier_opaque[vector], > msix_is_masked(dev, vector)); > assert(r >= 0); > } > @@ -351,9 +349,8 @@ static void msix_mask_all(struct PCIDevice *dev, unsigned nentries) > int was_masked = msix_is_masked(dev, vector); > dev->msix_table_page[offset] |= MSIX_VECTOR_MASK; > if (was_masked != msix_is_masked(dev, vector) && > - dev->msix_mask_notifier && dev->msix_mask_notifier_opaque[vector]) { > + dev->msix_mask_notifier) { > r = dev->msix_mask_notifier(dev, vector, > - dev->msix_mask_notifier_opaque[vector], > msix_is_masked(dev, vector)); > assert(r >= 0); > } > @@ -379,8 +376,6 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries, > sizeof *dev->msix_irq_entries); > } > #endif > - dev->msix_mask_notifier_opaque = > - qemu_mallocz(nentries * sizeof *dev->msix_mask_notifier_opaque); > dev->msix_mask_notifier = NULL; > dev->msix_entry_used = qemu_mallocz(MSIX_MAX_ENTRIES * > sizeof *dev->msix_entry_used); > @@ -444,8 +439,6 @@ int msix_uninit(PCIDevice *dev) > dev->msix_entry_used = NULL; > qemu_free(dev->msix_irq_entries); > dev->msix_irq_entries = NULL; > - qemu_free(dev->msix_mask_notifier_opaque); > - dev->msix_mask_notifier_opaque = NULL; > dev->cap_present &= ~QEMU_PCI_CAP_MSIX; > return 0; > } > @@ -590,46 +583,79 @@ void msix_unuse_all_vectors(PCIDevice *dev) > msix_free_irq_entries(dev); > } > > -int msix_set_mask_notifier(PCIDevice *dev, unsigned vector, void *opaque) > +static int msix_set_mask_notifier_for_vector(PCIDevice *dev, unsigned vector) > { > int r = 0; > if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector]) > return 0; > > assert(dev->msix_mask_notifier); > - assert(opaque); > - assert(!dev->msix_mask_notifier_opaque[vector]); > > /* Unmask the new notifier unless vector is masked. */ > if (!msix_is_masked(dev, vector)) { > - r = dev->msix_mask_notifier(dev, vector, opaque, false); > + r = dev->msix_mask_notifier(dev, vector, false); > if (r < 0) { > return r; > } > } > - dev->msix_mask_notifier_opaque[vector] = opaque; > return r; > } > > -int msix_unset_mask_notifier(PCIDevice *dev, unsigned vector) > +static int msix_unset_mask_notifier_for_vector(PCIDevice *dev, unsigned vector) > { > int r = 0; > - void *opaque; > if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector]) > return 0; > > - opaque = dev->msix_mask_notifier_opaque[vector]; > - > assert(dev->msix_mask_notifier); > - assert(opaque); > > /* Mask the old notifier unless it is already masked. */ > if (!msix_is_masked(dev, vector)) { > - r = dev->msix_mask_notifier(dev, vector, opaque, true); > + r = dev->msix_mask_notifier(dev, vector, true); > if (r < 0) { > return r; > } > } > - dev->msix_mask_notifier_opaque[vector] = NULL; > + return r; > +} The above need to be combined to a single function now since the only difference is s/true/false. Alex > + > +int msix_set_mask_notifier(PCIDevice *dev, msix_mask_notifier_func f) > +{ > + int r, n; > + assert(!dev->msix_mask_notifier); > + dev->msix_mask_notifier = f; > + for (n = 0; n < dev->msix_entries_nr; ++n) { > + r = msix_set_mask_notifier_for_vector(dev, n); > + if (r < 0) { > + goto undo; > + } > + } > + return 0; > + > +undo: > + while (--n >= 0) { > + msix_unset_mask_notifier_for_vector(dev, n); > + } > + dev->msix_mask_notifier = NULL; > + return r; > +} > + > +int msix_unset_mask_notifier(PCIDevice *dev) > +{ > + int r, n; > + assert(dev->msix_mask_notifier); > + for (n = 0; n < dev->msix_entries_nr; ++n) { > + r = msix_unset_mask_notifier_for_vector(dev, n); > + if (r < 0) { > + goto undo; > + } > + } > + dev->msix_mask_notifier = NULL; > + return 0; > + > +undo: > + while (--n >= 0) { > + msix_set_mask_notifier_for_vector(dev, n); > + } > return r; > } > diff --git a/hw/msix.h b/hw/msix.h > index 6b21ffb..5a81df5 100644 > --- a/hw/msix.h > +++ b/hw/msix.h > @@ -33,6 +33,6 @@ void msix_reset(PCIDevice *dev); > > extern int msix_supported; > > -int msix_set_mask_notifier(PCIDevice *dev, unsigned vector, void *opaque); > -int msix_unset_mask_notifier(PCIDevice *dev, unsigned vector); > +int msix_set_mask_notifier(PCIDevice *dev, msix_mask_notifier_func); > +int msix_unset_mask_notifier(PCIDevice *dev); > #endif > diff --git a/hw/pci.h b/hw/pci.h > index ccb99d0..a40dc14 100644 > --- a/hw/pci.h > +++ b/hw/pci.h > @@ -131,7 +131,7 @@ enum { > #define PCI_CAPABILITY_CONFIG_MSIX_LENGTH 0x10 > > typedef int (*msix_mask_notifier_func)(PCIDevice *, unsigned vector, > - void *opaque, int masked); > + int masked); > > struct PCIDevice { > DeviceState qdev; > @@ -198,7 +198,6 @@ struct PCIDevice { > > struct kvm_irq_routing_entry *msix_irq_entries; > > - void **msix_mask_notifier_opaque; > msix_mask_notifier_func msix_mask_notifier; > > /* Device capability configuration space */ > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > index 232f943..c1204ce 100644 > --- a/hw/virtio-pci.c > +++ b/hw/virtio-pci.c > @@ -428,11 +428,10 @@ static void virtio_pci_guest_notifier_read(void *opaque) > } > } > > -static int virtio_pci_mask_notifier(PCIDevice *dev, unsigned vector, > - void *opaque, int masked) > +static int virtio_pci_mask_vq(PCIDevice *dev, unsigned vector, > + VirtQueue *vq, int masked) > { > #ifdef CONFIG_KVM > - VirtQueue *vq = opaque; > EventNotifier *notifier = virtio_queue_get_guest_notifier(vq); > int r = kvm_set_irqfd(dev->msix_irq_entries[vector].gsi, > event_notifier_get_fd(notifier), > @@ -453,6 +452,34 @@ static int virtio_pci_mask_notifier(PCIDevice *dev, unsigned vector, > #endif > } > > +static int virtio_pci_mask_notifier(PCIDevice *dev, unsigned vector, > + int masked) > +{ > + VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev); > + VirtIODevice *vdev = proxy->vdev; > + int r, n; > + > + for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) { > + if (!virtio_queue_get_num(vdev, n)) { > + break; > + } > + if (virtio_queue_vector(vdev, n) != vector) { > + continue; > + } > + r = virtio_pci_mask_vq(dev, vector, virtio_get_queue(vdev, n), masked); > + if (r < 0) { > + goto undo; > + } > + } > + return 0; > +undo: > + while (--n >= 0) { > + virtio_pci_mask_vq(dev, vector, virtio_get_queue(vdev, n), !masked); > + } > + return r; > +} > + > + > static int virtio_pci_set_guest_notifier(void *opaque, int n, bool assign) > { > VirtIOPCIProxy *proxy = opaque; > @@ -466,11 +493,7 @@ static int virtio_pci_set_guest_notifier(void *opaque, int n, bool assign) > } > qemu_set_fd_handler(event_notifier_get_fd(notifier), > virtio_pci_guest_notifier_read, NULL, vq); > - msix_set_mask_notifier(&proxy->pci_dev, > - virtio_queue_vector(proxy->vdev, n), vq); > } else { > - msix_unset_mask_notifier(&proxy->pci_dev, > - virtio_queue_vector(proxy->vdev, n)); > qemu_set_fd_handler(event_notifier_get_fd(notifier), > NULL, NULL, NULL); > /* Test and clear notifier before closing it, > @@ -488,6 +511,13 @@ static int virtio_pci_set_guest_notifiers(void *opaque, bool assign) > VirtIODevice *vdev = proxy->vdev; > int r, n; > > + /* Must unset mask notifier while guest notifier > + * is still assigned */ > + if (!assign) { > + r = msix_unset_mask_notifier(&proxy->pci_dev); > + assert(r >= 0); > + } > + > for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) { > if (!virtio_queue_get_num(vdev, n)) { > break; > @@ -499,6 +529,16 @@ static int virtio_pci_set_guest_notifiers(void *opaque, bool assign) > } > } > > + /* Must set mask notifier after guest notifier > + * has been assigned */ > + if (assign) { > + r = msix_set_mask_notifier(&proxy->pci_dev, > + virtio_pci_mask_notifier); > + if (r < 0) { > + goto assign_error; > + } > + } > + > return 0; > > assign_error: > @@ -584,8 +624,6 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, > > proxy->pci_dev.config_write = virtio_write_config; > > - proxy->pci_dev.msix_mask_notifier = virtio_pci_mask_notifier; > - > size = VIRTIO_PCI_REGION_SIZE(&proxy->pci_dev) + vdev->config_len; > if (size & (size-1)) > size = 1 << qemu_fls(size);
On Wed, Oct 06, 2010 at 10:48:44AM -0600, Alex Williamson wrote: > > -int msix_unset_mask_notifier(PCIDevice *dev, unsigned vector) > > +static int msix_unset_mask_notifier_for_vector(PCIDevice *dev, unsigned vector) > > { > > int r = 0; > > - void *opaque; > > if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector]) > > return 0; > > > > - opaque = dev->msix_mask_notifier_opaque[vector]; > > - > > assert(dev->msix_mask_notifier); > > - assert(opaque); > > > > /* Mask the old notifier unless it is already masked. */ > > if (!msix_is_masked(dev, vector)) { > > - r = dev->msix_mask_notifier(dev, vector, opaque, true); > > + r = dev->msix_mask_notifier(dev, vector, true); > > if (r < 0) { > > return r; > > } > > } > > - dev->msix_mask_notifier_opaque[vector] = NULL; > > + return r; > > +} > > The above need to be combined to a single function now since the only > difference is s/true/false. > > Alex This is the way it was in the past, and it turned out to be very confusing to read since both variables: mask and assign are bool but polarity is reversed. Unrolled it seems easier to grok.
On Wed, 2010-10-06 at 19:02 +0200, Michael S. Tsirkin wrote: > On Wed, Oct 06, 2010 at 10:48:44AM -0600, Alex Williamson wrote: > > > -int msix_unset_mask_notifier(PCIDevice *dev, unsigned vector) > > > +static int msix_unset_mask_notifier_for_vector(PCIDevice *dev, unsigned vector) > > > { > > > int r = 0; > > > - void *opaque; > > > if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector]) > > > return 0; > > > > > > - opaque = dev->msix_mask_notifier_opaque[vector]; > > > - > > > assert(dev->msix_mask_notifier); > > > - assert(opaque); > > > > > > /* Mask the old notifier unless it is already masked. */ > > > if (!msix_is_masked(dev, vector)) { > > > - r = dev->msix_mask_notifier(dev, vector, opaque, true); > > > + r = dev->msix_mask_notifier(dev, vector, true); > > > if (r < 0) { > > > return r; > > > } > > > } > > > - dev->msix_mask_notifier_opaque[vector] = NULL; > > > + return r; > > > +} > > > > The above need to be combined to a single function now since the only > > difference is s/true/false. > > > > Alex > > This is the way it was in the past, and it turned out to be very > confusing to read since both variables: mask and assign are bool but > polarity is reversed. > > Unrolled it seems easier to grok. You could always keep the functions as separate wrapper callers of the common function so you only need to keep true = unset, false = set straight in one place. Thanks, Alex
On Wed, Oct 06, 2010 at 11:24:24AM -0600, Alex Williamson wrote: > On Wed, 2010-10-06 at 19:02 +0200, Michael S. Tsirkin wrote: > > On Wed, Oct 06, 2010 at 10:48:44AM -0600, Alex Williamson wrote: > > > > -int msix_unset_mask_notifier(PCIDevice *dev, unsigned vector) > > > > +static int msix_unset_mask_notifier_for_vector(PCIDevice *dev, unsigned vector) > > > > { > > > > int r = 0; > > > > - void *opaque; > > > > if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector]) > > > > return 0; > > > > > > > > - opaque = dev->msix_mask_notifier_opaque[vector]; > > > > - > > > > assert(dev->msix_mask_notifier); > > > > - assert(opaque); > > > > > > > > /* Mask the old notifier unless it is already masked. */ > > > > if (!msix_is_masked(dev, vector)) { > > > > - r = dev->msix_mask_notifier(dev, vector, opaque, true); > > > > + r = dev->msix_mask_notifier(dev, vector, true); > > > > if (r < 0) { > > > > return r; > > > > } > > > > } > > > > - dev->msix_mask_notifier_opaque[vector] = NULL; > > > > + return r; > > > > +} > > > > > > The above need to be combined to a single function now since the only > > > difference is s/true/false. > > > > > > Alex > > > > This is the way it was in the past, and it turned out to be very > > confusing to read since both variables: mask and assign are bool but > > polarity is reversed. > > > > Unrolled it seems easier to grok. > > You could always keep the functions as separate wrapper callers of the > common function so you only need to keep true = unset, false = set > straight in one place. Thanks, > > Alex wrappers still make this confusing. we had so many bugs here, I feel minor duplication is worth it.
diff --git a/hw/msix.c b/hw/msix.c index 3dd0456..3d4dd61 100644 --- a/hw/msix.c +++ b/hw/msix.c @@ -300,10 +300,8 @@ static void msix_mmio_writel(void *opaque, target_phys_addr_t addr, if (kvm_enabled() && kvm_irqchip_in_kernel()) { kvm_msix_update(dev, vector, was_masked, msix_is_masked(dev, vector)); } - if (was_masked != msix_is_masked(dev, vector) && - dev->msix_mask_notifier && dev->msix_mask_notifier_opaque[vector]) { + if (was_masked != msix_is_masked(dev, vector) && dev->msix_mask_notifier) { int r = dev->msix_mask_notifier(dev, vector, - dev->msix_mask_notifier_opaque[vector], msix_is_masked(dev, vector)); assert(r >= 0); } @@ -351,9 +349,8 @@ static void msix_mask_all(struct PCIDevice *dev, unsigned nentries) int was_masked = msix_is_masked(dev, vector); dev->msix_table_page[offset] |= MSIX_VECTOR_MASK; if (was_masked != msix_is_masked(dev, vector) && - dev->msix_mask_notifier && dev->msix_mask_notifier_opaque[vector]) { + dev->msix_mask_notifier) { r = dev->msix_mask_notifier(dev, vector, - dev->msix_mask_notifier_opaque[vector], msix_is_masked(dev, vector)); assert(r >= 0); } @@ -379,8 +376,6 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries, sizeof *dev->msix_irq_entries); } #endif - dev->msix_mask_notifier_opaque = - qemu_mallocz(nentries * sizeof *dev->msix_mask_notifier_opaque); dev->msix_mask_notifier = NULL; dev->msix_entry_used = qemu_mallocz(MSIX_MAX_ENTRIES * sizeof *dev->msix_entry_used); @@ -444,8 +439,6 @@ int msix_uninit(PCIDevice *dev) dev->msix_entry_used = NULL; qemu_free(dev->msix_irq_entries); dev->msix_irq_entries = NULL; - qemu_free(dev->msix_mask_notifier_opaque); - dev->msix_mask_notifier_opaque = NULL; dev->cap_present &= ~QEMU_PCI_CAP_MSIX; return 0; } @@ -590,46 +583,79 @@ void msix_unuse_all_vectors(PCIDevice *dev) msix_free_irq_entries(dev); } -int msix_set_mask_notifier(PCIDevice *dev, unsigned vector, void *opaque) +static int msix_set_mask_notifier_for_vector(PCIDevice *dev, unsigned vector) { int r = 0; if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector]) return 0; assert(dev->msix_mask_notifier); - assert(opaque); - assert(!dev->msix_mask_notifier_opaque[vector]); /* Unmask the new notifier unless vector is masked. */ if (!msix_is_masked(dev, vector)) { - r = dev->msix_mask_notifier(dev, vector, opaque, false); + r = dev->msix_mask_notifier(dev, vector, false); if (r < 0) { return r; } } - dev->msix_mask_notifier_opaque[vector] = opaque; return r; } -int msix_unset_mask_notifier(PCIDevice *dev, unsigned vector) +static int msix_unset_mask_notifier_for_vector(PCIDevice *dev, unsigned vector) { int r = 0; - void *opaque; if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector]) return 0; - opaque = dev->msix_mask_notifier_opaque[vector]; - assert(dev->msix_mask_notifier); - assert(opaque); /* Mask the old notifier unless it is already masked. */ if (!msix_is_masked(dev, vector)) { - r = dev->msix_mask_notifier(dev, vector, opaque, true); + r = dev->msix_mask_notifier(dev, vector, true); if (r < 0) { return r; } } - dev->msix_mask_notifier_opaque[vector] = NULL; + return r; +} + +int msix_set_mask_notifier(PCIDevice *dev, msix_mask_notifier_func f) +{ + int r, n; + assert(!dev->msix_mask_notifier); + dev->msix_mask_notifier = f; + for (n = 0; n < dev->msix_entries_nr; ++n) { + r = msix_set_mask_notifier_for_vector(dev, n); + if (r < 0) { + goto undo; + } + } + return 0; + +undo: + while (--n >= 0) { + msix_unset_mask_notifier_for_vector(dev, n); + } + dev->msix_mask_notifier = NULL; + return r; +} + +int msix_unset_mask_notifier(PCIDevice *dev) +{ + int r, n; + assert(dev->msix_mask_notifier); + for (n = 0; n < dev->msix_entries_nr; ++n) { + r = msix_unset_mask_notifier_for_vector(dev, n); + if (r < 0) { + goto undo; + } + } + dev->msix_mask_notifier = NULL; + return 0; + +undo: + while (--n >= 0) { + msix_set_mask_notifier_for_vector(dev, n); + } return r; } diff --git a/hw/msix.h b/hw/msix.h index 6b21ffb..5a81df5 100644 --- a/hw/msix.h +++ b/hw/msix.h @@ -33,6 +33,6 @@ void msix_reset(PCIDevice *dev); extern int msix_supported; -int msix_set_mask_notifier(PCIDevice *dev, unsigned vector, void *opaque); -int msix_unset_mask_notifier(PCIDevice *dev, unsigned vector); +int msix_set_mask_notifier(PCIDevice *dev, msix_mask_notifier_func); +int msix_unset_mask_notifier(PCIDevice *dev); #endif diff --git a/hw/pci.h b/hw/pci.h index ccb99d0..a40dc14 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -131,7 +131,7 @@ enum { #define PCI_CAPABILITY_CONFIG_MSIX_LENGTH 0x10 typedef int (*msix_mask_notifier_func)(PCIDevice *, unsigned vector, - void *opaque, int masked); + int masked); struct PCIDevice { DeviceState qdev; @@ -198,7 +198,6 @@ struct PCIDevice { struct kvm_irq_routing_entry *msix_irq_entries; - void **msix_mask_notifier_opaque; msix_mask_notifier_func msix_mask_notifier; /* Device capability configuration space */ diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 232f943..c1204ce 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -428,11 +428,10 @@ static void virtio_pci_guest_notifier_read(void *opaque) } } -static int virtio_pci_mask_notifier(PCIDevice *dev, unsigned vector, - void *opaque, int masked) +static int virtio_pci_mask_vq(PCIDevice *dev, unsigned vector, + VirtQueue *vq, int masked) { #ifdef CONFIG_KVM - VirtQueue *vq = opaque; EventNotifier *notifier = virtio_queue_get_guest_notifier(vq); int r = kvm_set_irqfd(dev->msix_irq_entries[vector].gsi, event_notifier_get_fd(notifier), @@ -453,6 +452,34 @@ static int virtio_pci_mask_notifier(PCIDevice *dev, unsigned vector, #endif } +static int virtio_pci_mask_notifier(PCIDevice *dev, unsigned vector, + int masked) +{ + VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev); + VirtIODevice *vdev = proxy->vdev; + int r, n; + + for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) { + if (!virtio_queue_get_num(vdev, n)) { + break; + } + if (virtio_queue_vector(vdev, n) != vector) { + continue; + } + r = virtio_pci_mask_vq(dev, vector, virtio_get_queue(vdev, n), masked); + if (r < 0) { + goto undo; + } + } + return 0; +undo: + while (--n >= 0) { + virtio_pci_mask_vq(dev, vector, virtio_get_queue(vdev, n), !masked); + } + return r; +} + + static int virtio_pci_set_guest_notifier(void *opaque, int n, bool assign) { VirtIOPCIProxy *proxy = opaque; @@ -466,11 +493,7 @@ static int virtio_pci_set_guest_notifier(void *opaque, int n, bool assign) } qemu_set_fd_handler(event_notifier_get_fd(notifier), virtio_pci_guest_notifier_read, NULL, vq); - msix_set_mask_notifier(&proxy->pci_dev, - virtio_queue_vector(proxy->vdev, n), vq); } else { - msix_unset_mask_notifier(&proxy->pci_dev, - virtio_queue_vector(proxy->vdev, n)); qemu_set_fd_handler(event_notifier_get_fd(notifier), NULL, NULL, NULL); /* Test and clear notifier before closing it, @@ -488,6 +511,13 @@ static int virtio_pci_set_guest_notifiers(void *opaque, bool assign) VirtIODevice *vdev = proxy->vdev; int r, n; + /* Must unset mask notifier while guest notifier + * is still assigned */ + if (!assign) { + r = msix_unset_mask_notifier(&proxy->pci_dev); + assert(r >= 0); + } + for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) { if (!virtio_queue_get_num(vdev, n)) { break; @@ -499,6 +529,16 @@ static int virtio_pci_set_guest_notifiers(void *opaque, bool assign) } } + /* Must set mask notifier after guest notifier + * has been assigned */ + if (assign) { + r = msix_set_mask_notifier(&proxy->pci_dev, + virtio_pci_mask_notifier); + if (r < 0) { + goto assign_error; + } + } + return 0; assign_error: @@ -584,8 +624,6 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, proxy->pci_dev.config_write = virtio_write_config; - proxy->pci_dev.msix_mask_notifier = virtio_pci_mask_notifier; - size = VIRTIO_PCI_REGION_SIZE(&proxy->pci_dev) + vdev->config_len; if (size & (size-1)) size = 1 << qemu_fls(size);
vhost irqfd support: case where many vqs are mapped to a single msix vector is currently broken. Fix it up. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- This is on top of the qemu patchset, which is unchanged. Fixes from v1: correct error handling hw/msix.c | 68 ++++++++++++++++++++++++++++++++++++++----------------- hw/msix.h | 4 +- hw/pci.h | 3 +- hw/virtio-pci.c | 56 ++++++++++++++++++++++++++++++++++++++------- 4 files changed, 97 insertions(+), 34 deletions(-)