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@xxxxxxxxxx>
> ---
>
> 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);
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
