From 201a733145751aa691e7e3b9c0f263f0c92db0c5 Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Thu, 13 Jun 2019 09:57:37 -0600 Subject: [PATCH] vfio/common: Introduce vfio_set_irq_signaling helper The code used to assign an interrupt index/subindex to an eventfd is duplicated many times. Let's introduce an helper that allows to set/unset the signaling for an ACTION_TRIGGER, ACTION_MASK or ACTION_UNMASK action. In the error message, we now use errno in case of any VFIO_DEVICE_SET_IRQS ioctl failure. Signed-off-by: Eric Auger Reviewed-by: Cornelia Huck Reviewed-by: Li Qiang Signed-off-by: Alex Williamson --- hw/vfio/common.c | 78 ++++++++++++ hw/vfio/pci.c | 217 ++++++++-------------------------- hw/vfio/platform.c | 62 ++++------ include/hw/vfio/vfio-common.h | 2 + 4 files changed, 151 insertions(+), 208 deletions(-) diff --git a/hw/vfio/common.c b/hw/vfio/common.c index 4374cc6176..a859298fda 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -95,6 +95,84 @@ void vfio_mask_single_irqindex(VFIODevice *vbasedev, int index) ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, &irq_set); } +static inline const char *action_to_str(int action) +{ + switch (action) { + case VFIO_IRQ_SET_ACTION_MASK: + return "MASK"; + case VFIO_IRQ_SET_ACTION_UNMASK: + return "UNMASK"; + case VFIO_IRQ_SET_ACTION_TRIGGER: + return "TRIGGER"; + default: + return "UNKNOWN ACTION"; + } +} + +static const char *index_to_str(VFIODevice *vbasedev, int index) +{ + if (vbasedev->type != VFIO_DEVICE_TYPE_PCI) { + return NULL; + } + + switch (index) { + case VFIO_PCI_INTX_IRQ_INDEX: + return "INTX"; + case VFIO_PCI_MSI_IRQ_INDEX: + return "MSI"; + case VFIO_PCI_MSIX_IRQ_INDEX: + return "MSIX"; + case VFIO_PCI_ERR_IRQ_INDEX: + return "ERR"; + case VFIO_PCI_REQ_IRQ_INDEX: + return "REQ"; + default: + return NULL; + } +} + +int vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex, + int action, int fd, Error **errp) +{ + struct vfio_irq_set *irq_set; + int argsz, ret = 0; + const char *name; + int32_t *pfd; + + argsz = sizeof(*irq_set) + sizeof(*pfd); + + irq_set = g_malloc0(argsz); + irq_set->argsz = argsz; + irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | action; + irq_set->index = index; + irq_set->start = subindex; + irq_set->count = 1; + pfd = (int32_t *)&irq_set->data; + *pfd = fd; + + if (ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) { + ret = -errno; + } + g_free(irq_set); + + if (!ret) { + return 0; + } + + error_setg_errno(errp, -ret, "VFIO_DEVICE_SET_IRQS failure"); + + name = index_to_str(vbasedev, index); + if (name) { + error_prepend(errp, "%s-%d: ", name, subindex); + } else { + error_prepend(errp, "index %d-%d: ", index, subindex); + } + error_prepend(errp, + "Failed to %s %s eventfd signaling for interrupt ", + fd < 0 ? "tear down" : "set up", action_to_str(action)); + return ret; +} + /* * IO Port/MMIO - Beware of the endians, VFIO is always little endian */ diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 6520c05dee..ce3fe96efe 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -114,9 +114,7 @@ static void vfio_intx_enable_kvm(VFIOPCIDevice *vdev, Error **errp) .gsi = vdev->intx.route.irq, .flags = KVM_IRQFD_FLAG_RESAMPLE, }; - struct vfio_irq_set *irq_set; - int ret, argsz; - int32_t *pfd; + Error *err = NULL; if (vdev->no_kvm_intx || !kvm_irqfds_enabled() || vdev->intx.route.mode != PCI_INTX_ENABLED || @@ -144,22 +142,10 @@ static void vfio_intx_enable_kvm(VFIOPCIDevice *vdev, Error **errp) goto fail_irqfd; } - argsz = sizeof(*irq_set) + sizeof(*pfd); - - irq_set = g_malloc0(argsz); - irq_set->argsz = argsz; - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | VFIO_IRQ_SET_ACTION_UNMASK; - irq_set->index = VFIO_PCI_INTX_IRQ_INDEX; - irq_set->start = 0; - irq_set->count = 1; - pfd = (int32_t *)&irq_set->data; - - *pfd = irqfd.resamplefd; - - ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set); - g_free(irq_set); - if (ret) { - error_setg_errno(errp, -ret, "failed to setup INTx unmask fd"); + if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX, 0, + VFIO_IRQ_SET_ACTION_UNMASK, + irqfd.resamplefd, &err)) { + error_propagate(errp, err); goto fail_vfio; } @@ -263,10 +249,10 @@ static void vfio_intx_update(PCIDevice *pdev) static int vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp) { uint8_t pin = vfio_pci_read_config(&vdev->pdev, PCI_INTERRUPT_PIN, 1); - int ret, argsz, retval = 0; - struct vfio_irq_set *irq_set; - int32_t *pfd; Error *err = NULL; + int32_t fd; + int ret; + if (!pin) { return 0; @@ -293,27 +279,15 @@ static int vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp) error_setg_errno(errp, -ret, "event_notifier_init failed"); return ret; } + fd = event_notifier_get_fd(&vdev->intx.interrupt); + qemu_set_fd_handler(fd, vfio_intx_interrupt, NULL, vdev); - argsz = sizeof(*irq_set) + sizeof(*pfd); - - irq_set = g_malloc0(argsz); - irq_set->argsz = argsz; - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | VFIO_IRQ_SET_ACTION_TRIGGER; - irq_set->index = VFIO_PCI_INTX_IRQ_INDEX; - irq_set->start = 0; - irq_set->count = 1; - pfd = (int32_t *)&irq_set->data; - - *pfd = event_notifier_get_fd(&vdev->intx.interrupt); - qemu_set_fd_handler(*pfd, vfio_intx_interrupt, NULL, vdev); - - ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set); - if (ret) { - error_setg_errno(errp, -ret, "failed to setup INTx fd"); - qemu_set_fd_handler(*pfd, NULL, NULL, vdev); + if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_INTX_IRQ_INDEX, 0, + VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) { + error_propagate(errp, err); + qemu_set_fd_handler(fd, NULL, NULL, vdev); event_notifier_cleanup(&vdev->intx.interrupt); - retval = -errno; - goto cleanup; + return -errno; } vfio_intx_enable_kvm(vdev, &err); @@ -324,11 +298,7 @@ static int vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp) vdev->interrupt = VFIO_INT_INTx; trace_vfio_intx_enable(vdev->vbasedev.name); - -cleanup: - g_free(irq_set); - - return retval; + return 0; } static void vfio_intx_disable(VFIOPCIDevice *vdev) @@ -531,31 +501,19 @@ static int vfio_msix_vector_do_use(PCIDevice *pdev, unsigned int nr, error_report("vfio: failed to enable vectors, %d", ret); } } else { - int argsz; - struct vfio_irq_set *irq_set; - int32_t *pfd; - - argsz = sizeof(*irq_set) + sizeof(*pfd); - - irq_set = g_malloc0(argsz); - irq_set->argsz = argsz; - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | - VFIO_IRQ_SET_ACTION_TRIGGER; - irq_set->index = VFIO_PCI_MSIX_IRQ_INDEX; - irq_set->start = nr; - irq_set->count = 1; - pfd = (int32_t *)&irq_set->data; + Error *err = NULL; + int32_t fd; if (vector->virq >= 0) { - *pfd = event_notifier_get_fd(&vector->kvm_interrupt); + fd = event_notifier_get_fd(&vector->kvm_interrupt); } else { - *pfd = event_notifier_get_fd(&vector->interrupt); + fd = event_notifier_get_fd(&vector->interrupt); } - ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set); - g_free(irq_set); - if (ret) { - error_report("vfio: failed to modify vector, %d", ret); + if (vfio_set_irq_signaling(&vdev->vbasedev, + VFIO_PCI_MSIX_IRQ_INDEX, nr, + VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) { + error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name); } } @@ -592,26 +550,10 @@ static void vfio_msix_vector_release(PCIDevice *pdev, unsigned int nr) * be re-asserted on unmask. Nothing to do if already using QEMU mode. */ if (vector->virq >= 0) { - int argsz; - struct vfio_irq_set *irq_set; - int32_t *pfd; + int32_t fd = event_notifier_get_fd(&vector->interrupt); - argsz = sizeof(*irq_set) + sizeof(*pfd); - - irq_set = g_malloc0(argsz); - irq_set->argsz = argsz; - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | - VFIO_IRQ_SET_ACTION_TRIGGER; - irq_set->index = VFIO_PCI_MSIX_IRQ_INDEX; - irq_set->start = nr; - irq_set->count = 1; - pfd = (int32_t *)&irq_set->data; - - *pfd = event_notifier_get_fd(&vector->interrupt); - - ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set); - - g_free(irq_set); + vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_MSIX_IRQ_INDEX, nr, + VFIO_IRQ_SET_ACTION_TRIGGER, fd, NULL); } } @@ -2638,10 +2580,8 @@ static void vfio_err_notifier_handler(void *opaque) */ static void vfio_register_err_notifier(VFIOPCIDevice *vdev) { - int ret; - int argsz; - struct vfio_irq_set *irq_set; - int32_t *pfd; + Error *err = NULL; + int32_t fd; if (!vdev->pci_aer) { return; @@ -2653,58 +2593,30 @@ static void vfio_register_err_notifier(VFIOPCIDevice *vdev) return; } - argsz = sizeof(*irq_set) + sizeof(*pfd); + fd = event_notifier_get_fd(&vdev->err_notifier); + qemu_set_fd_handler(fd, vfio_err_notifier_handler, NULL, vdev); - irq_set = g_malloc0(argsz); - irq_set->argsz = argsz; - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | - VFIO_IRQ_SET_ACTION_TRIGGER; - irq_set->index = VFIO_PCI_ERR_IRQ_INDEX; - irq_set->start = 0; - irq_set->count = 1; - pfd = (int32_t *)&irq_set->data; - - *pfd = event_notifier_get_fd(&vdev->err_notifier); - qemu_set_fd_handler(*pfd, vfio_err_notifier_handler, NULL, vdev); - - ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set); - if (ret) { - error_report("vfio: Failed to set up error notification"); - qemu_set_fd_handler(*pfd, NULL, NULL, vdev); + if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_ERR_IRQ_INDEX, 0, + VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) { + error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name); + qemu_set_fd_handler(fd, NULL, NULL, vdev); event_notifier_cleanup(&vdev->err_notifier); vdev->pci_aer = false; } - g_free(irq_set); } static void vfio_unregister_err_notifier(VFIOPCIDevice *vdev) { - int argsz; - struct vfio_irq_set *irq_set; - int32_t *pfd; - int ret; + Error *err = NULL; if (!vdev->pci_aer) { return; } - argsz = sizeof(*irq_set) + sizeof(*pfd); - - irq_set = g_malloc0(argsz); - irq_set->argsz = argsz; - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | - VFIO_IRQ_SET_ACTION_TRIGGER; - irq_set->index = VFIO_PCI_ERR_IRQ_INDEX; - irq_set->start = 0; - irq_set->count = 1; - pfd = (int32_t *)&irq_set->data; - *pfd = -1; - - ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set); - if (ret) { - error_report("vfio: Failed to de-assign error fd: %m"); + if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_ERR_IRQ_INDEX, 0, + VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) { + error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name); } - g_free(irq_set); qemu_set_fd_handler(event_notifier_get_fd(&vdev->err_notifier), NULL, NULL, vdev); event_notifier_cleanup(&vdev->err_notifier); @@ -2729,9 +2641,8 @@ static void vfio_register_req_notifier(VFIOPCIDevice *vdev) { struct vfio_irq_info irq_info = { .argsz = sizeof(irq_info), .index = VFIO_PCI_REQ_IRQ_INDEX }; - int argsz; - struct vfio_irq_set *irq_set; - int32_t *pfd; + Error *err = NULL; + int32_t fd; if (!(vdev->features & VFIO_FEATURE_ENABLE_REQ)) { return; @@ -2747,57 +2658,31 @@ static void vfio_register_req_notifier(VFIOPCIDevice *vdev) return; } - argsz = sizeof(*irq_set) + sizeof(*pfd); + fd = event_notifier_get_fd(&vdev->req_notifier); + qemu_set_fd_handler(fd, vfio_req_notifier_handler, NULL, vdev); - irq_set = g_malloc0(argsz); - irq_set->argsz = argsz; - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | - VFIO_IRQ_SET_ACTION_TRIGGER; - irq_set->index = VFIO_PCI_REQ_IRQ_INDEX; - irq_set->start = 0; - irq_set->count = 1; - pfd = (int32_t *)&irq_set->data; - - *pfd = event_notifier_get_fd(&vdev->req_notifier); - qemu_set_fd_handler(*pfd, vfio_req_notifier_handler, NULL, vdev); - - if (ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set)) { - error_report("vfio: Failed to set up device request notification"); - qemu_set_fd_handler(*pfd, NULL, NULL, vdev); + if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_REQ_IRQ_INDEX, 0, + VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err)) { + error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name); + qemu_set_fd_handler(fd, NULL, NULL, vdev); event_notifier_cleanup(&vdev->req_notifier); } else { vdev->req_enabled = true; } - - g_free(irq_set); } static void vfio_unregister_req_notifier(VFIOPCIDevice *vdev) { - int argsz; - struct vfio_irq_set *irq_set; - int32_t *pfd; + Error *err = NULL; if (!vdev->req_enabled) { return; } - argsz = sizeof(*irq_set) + sizeof(*pfd); - - irq_set = g_malloc0(argsz); - irq_set->argsz = argsz; - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | - VFIO_IRQ_SET_ACTION_TRIGGER; - irq_set->index = VFIO_PCI_REQ_IRQ_INDEX; - irq_set->start = 0; - irq_set->count = 1; - pfd = (int32_t *)&irq_set->data; - *pfd = -1; - - if (ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set)) { - error_report("vfio: Failed to de-assign device request fd: %m"); + if (vfio_set_irq_signaling(&vdev->vbasedev, VFIO_PCI_REQ_IRQ_INDEX, 0, + VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err)) { + error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name); } - g_free(irq_set); qemu_set_fd_handler(event_notifier_get_fd(&vdev->req_notifier), NULL, NULL, vdev); event_notifier_cleanup(&vdev->req_notifier); diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c index ad29725955..622e609fb4 100644 --- a/hw/vfio/platform.c +++ b/hw/vfio/platform.c @@ -107,26 +107,19 @@ static int vfio_set_trigger_eventfd(VFIOINTp *intp, eventfd_user_side_handler_t handler) { VFIODevice *vbasedev = &intp->vdev->vbasedev; - struct vfio_irq_set *irq_set; - int argsz, ret; - int32_t *pfd; + int32_t fd = event_notifier_get_fd(intp->interrupt); + Error *err = NULL; + int ret; - argsz = sizeof(*irq_set) + sizeof(*pfd); - irq_set = g_malloc0(argsz); - irq_set->argsz = argsz; - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | VFIO_IRQ_SET_ACTION_TRIGGER; - irq_set->index = intp->pin; - irq_set->start = 0; - irq_set->count = 1; - pfd = (int32_t *)&irq_set->data; - *pfd = event_notifier_get_fd(intp->interrupt); - qemu_set_fd_handler(*pfd, (IOHandler *)handler, NULL, intp); - ret = ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, irq_set); - if (ret < 0) { - error_report("vfio: Failed to set trigger eventfd: %m"); - qemu_set_fd_handler(*pfd, NULL, NULL, NULL); + qemu_set_fd_handler(fd, (IOHandler *)handler, NULL, intp); + + ret = vfio_set_irq_signaling(vbasedev, intp->pin, 0, + VFIO_IRQ_SET_ACTION_TRIGGER, fd, &err); + if (ret) { + error_reportf_err(err, VFIO_MSG_PREFIX, vbasedev->name); + qemu_set_fd_handler(fd, NULL, NULL, NULL); } - g_free(irq_set); + return ret; } @@ -331,7 +324,6 @@ static void vfio_platform_eoi(VFIODevice *vbasedev) static void vfio_start_eventfd_injection(SysBusDevice *sbdev, qemu_irq irq) { - int ret; VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev); VFIOINTp *intp; @@ -342,10 +334,7 @@ static void vfio_start_eventfd_injection(SysBusDevice *sbdev, qemu_irq irq) } assert(intp); - ret = vfio_set_trigger_eventfd(intp, vfio_intp_interrupt); - if (ret) { - error_report("vfio: failed to start eventfd signaling for IRQ %d: %m", - intp->pin); + if (vfio_set_trigger_eventfd(intp, vfio_intp_interrupt)) { abort(); } } @@ -362,25 +351,16 @@ static void vfio_start_eventfd_injection(SysBusDevice *sbdev, qemu_irq irq) */ static int vfio_set_resample_eventfd(VFIOINTp *intp) { + int32_t fd = event_notifier_get_fd(intp->unmask); VFIODevice *vbasedev = &intp->vdev->vbasedev; - struct vfio_irq_set *irq_set; - int argsz, ret; - int32_t *pfd; + Error *err = NULL; + int ret; - argsz = sizeof(*irq_set) + sizeof(*pfd); - irq_set = g_malloc0(argsz); - irq_set->argsz = argsz; - irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD | VFIO_IRQ_SET_ACTION_UNMASK; - irq_set->index = intp->pin; - irq_set->start = 0; - irq_set->count = 1; - pfd = (int32_t *)&irq_set->data; - *pfd = event_notifier_get_fd(intp->unmask); - qemu_set_fd_handler(*pfd, NULL, NULL, NULL); - ret = ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, irq_set); - g_free(irq_set); - if (ret < 0) { - error_report("vfio: Failed to set resample eventfd: %m"); + qemu_set_fd_handler(fd, NULL, NULL, NULL); + ret = vfio_set_irq_signaling(vbasedev, intp->pin, 0, + VFIO_IRQ_SET_ACTION_UNMASK, fd, &err); + if (ret) { + error_reportf_err(err, VFIO_MSG_PREFIX, vbasedev->name); } return ret; } @@ -436,8 +416,6 @@ static void vfio_start_irqfd_injection(SysBusDevice *sbdev, qemu_irq irq) return; fail_vfio: kvm_irqchip_remove_irqfd_notifier(kvm_state, intp->interrupt, irq); - error_report("vfio: failed to start eventfd signaling for IRQ %d: %m", - intp->pin); abort(); fail_irqfd: vfio_start_eventfd_injection(sbdev, irq); diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h index e05bc09794..a88b69b675 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -166,6 +166,8 @@ void vfio_put_base_device(VFIODevice *vbasedev); void vfio_disable_irqindex(VFIODevice *vbasedev, int index); void vfio_unmask_single_irqindex(VFIODevice *vbasedev, int index); void vfio_mask_single_irqindex(VFIODevice *vbasedev, int index); +int vfio_set_irq_signaling(VFIODevice *vbasedev, int index, int subindex, + int action, int fd, Error **errp); void vfio_region_write(void *opaque, hwaddr addr, uint64_t data, unsigned size); uint64_t vfio_region_read(void *opaque,