vfio_intx_set_signal() and vfio_msi_set_signal() have similar code flow mixed with actions specific to the interrupt type. Code that is similar between MSI, MSI-X, and INTx management can be shared instead of duplicated. Start by replacing the MSI/MSI-X specific code within vfio_msi_set_signal() with functions. These functions that are specific to the management of MSI/MSI-X can later be called from a shared flow. Signed-off-by: Reinette Chatre <reinette.chatre@xxxxxxxxx> --- Note to maintainers: This is similar to "vfio/pci: Separate frontend and backend code during interrupt enable/disable" that was submitted as part of IMS changes, but is not specific to IMS. https://lore.kernel.org/lkml/cover.1696609476.git.reinette.chatre@xxxxxxxxx drivers/vfio/pci/vfio_pci_intrs.c | 134 ++++++++++++++++++++---------- 1 file changed, 89 insertions(+), 45 deletions(-) diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c index 7ca2b983b66e..70f2382c9c0c 100644 --- a/drivers/vfio/pci/vfio_pci_intrs.c +++ b/drivers/vfio/pci/vfio_pci_intrs.c @@ -414,26 +414,99 @@ static int vfio_msi_alloc_irq(struct vfio_pci_core_device *vdev, return map.index < 0 ? map.index : map.virq; } +static void vfio_msi_free_interrupt(struct vfio_pci_core_device *vdev, + struct vfio_pci_irq_ctx *ctx, + unsigned int vector) +{ + struct pci_dev *pdev = vdev->pdev; + int irq; + u16 cmd; + + irq = pci_irq_vector(pdev, vector); + cmd = vfio_pci_memory_lock_and_enable(vdev); + free_irq(irq, ctx->trigger); + vfio_pci_memory_unlock_and_restore(vdev, cmd); + /*Interrupt stays allocated, will be freed at MSI/MSI-X disable. */ +} + +static int vfio_msi_request_interrupt(struct vfio_pci_core_device *vdev, + struct vfio_pci_irq_ctx *ctx, + unsigned int vector, unsigned int index) +{ + bool msix = (index == VFIO_PCI_MSIX_IRQ_INDEX) ? true : false; + int irq, ret; + u16 cmd; + + irq = vfio_msi_alloc_irq(vdev, vector, msix); + if (irq < 0) + return irq; + + /* + * If the vector was previously allocated, refresh the on-device + * message data before enabling in case it had been cleared or + * corrupted (e.g. due to backdoor resets) since writing. + */ + cmd = vfio_pci_memory_lock_and_enable(vdev); + if (msix) { + struct msi_msg msg; + + get_cached_msi_msg(irq, &msg); + pci_write_msi_msg(irq, &msg); + } + + ret = request_irq(irq, vfio_msihandler, 0, ctx->name, ctx->trigger); + vfio_pci_memory_unlock_and_restore(vdev, cmd); + + return ret; +} + +static char *vfio_msi_device_name(struct vfio_pci_core_device *vdev, + unsigned int vector, unsigned int index) +{ + struct pci_dev *pdev = vdev->pdev; + + return kasprintf(GFP_KERNEL_ACCOUNT, "vfio-msi%s[%d](%s)", + index == VFIO_PCI_MSIX_IRQ_INDEX ? "x" : "", + vector, pci_name(pdev)); +} + +static void vfio_msi_register_producer(struct vfio_pci_core_device *vdev, + struct vfio_pci_irq_ctx *ctx, + unsigned int vector) +{ + struct pci_dev *pdev = vdev->pdev; + int ret; + + ctx->producer.token = ctx->trigger; + ctx->producer.irq = pci_irq_vector(pdev, vector); + ret = irq_bypass_register_producer(&ctx->producer); + if (unlikely(ret)) { + dev_info(&pdev->dev, + "irq bypass producer (token %p) registration fails: %d\n", + ctx->producer.token, ret); + ctx->producer.token = NULL; + ctx->producer.irq = 0; + } +} + +static void vfio_msi_unregister_producer(struct vfio_pci_irq_ctx *ctx) +{ + irq_bypass_unregister_producer(&ctx->producer); +} + static int vfio_msi_set_vector_signal(struct vfio_pci_core_device *vdev, unsigned int vector, int fd, unsigned int index) { - bool msix = (index == VFIO_PCI_MSIX_IRQ_INDEX) ? true : false; - struct pci_dev *pdev = vdev->pdev; struct vfio_pci_irq_ctx *ctx; struct eventfd_ctx *trigger; - int irq = -EINVAL, ret; - u16 cmd; + int ret; ctx = vfio_irq_ctx_get(vdev, vector); if (ctx && ctx->trigger) { - irq_bypass_unregister_producer(&ctx->producer); - irq = pci_irq_vector(pdev, vector); - cmd = vfio_pci_memory_lock_and_enable(vdev); - free_irq(irq, ctx->trigger); - vfio_pci_memory_unlock_and_restore(vdev, cmd); - /* Interrupt stays allocated, will be freed at MSI-X disable. */ + vfio_msi_unregister_producer(ctx); + vfio_msi_free_interrupt(vdev, ctx, vector); kfree(ctx->name); ctx->name = NULL; eventfd_ctx_put(ctx->trigger); @@ -443,13 +516,6 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_core_device *vdev, if (fd < 0) return 0; - if (irq == -EINVAL) { - /* Interrupt stays allocated, will be freed at MSI-X disable. */ - irq = vfio_msi_alloc_irq(vdev, vector, msix); - if (irq < 0) - return irq; - } - /* Per-interrupt context remain allocated. */ if (!ctx) { ctx = vfio_irq_ctx_alloc(vdev, vector); @@ -457,8 +523,7 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_core_device *vdev, return -ENOMEM; } - ctx->name = kasprintf(GFP_KERNEL_ACCOUNT, "vfio-msi%s[%d](%s)", - msix ? "x" : "", vector, pci_name(pdev)); + ctx->name = vfio_msi_device_name(vdev, vector, index); if (!ctx->name) return -ENOMEM; @@ -468,40 +533,19 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_core_device *vdev, goto out_free_name; } - /* - * If the vector was previously allocated, refresh the on-device - * message data before enabling in case it had been cleared or - * corrupted (e.g. due to backdoor resets) since writing. - */ - cmd = vfio_pci_memory_lock_and_enable(vdev); - if (msix) { - struct msi_msg msg; - - get_cached_msi_msg(irq, &msg); - pci_write_msi_msg(irq, &msg); - } + ctx->trigger = trigger; - ret = request_irq(irq, vfio_msihandler, 0, ctx->name, trigger); - vfio_pci_memory_unlock_and_restore(vdev, cmd); + ret = vfio_msi_request_interrupt(vdev, ctx, vector, index); if (ret) goto out_put_eventfd_ctx; - ctx->producer.token = trigger; - ctx->producer.irq = irq; - ret = irq_bypass_register_producer(&ctx->producer); - if (unlikely(ret)) { - dev_info(&pdev->dev, - "irq bypass producer (token %p) registration fails: %d\n", - ctx->producer.token, ret); - - ctx->producer.token = NULL; - } - ctx->trigger = trigger; + vfio_msi_register_producer(vdev, ctx, vector); return 0; out_put_eventfd_ctx: - eventfd_ctx_put(trigger); + eventfd_ctx_put(ctx->trigger); + ctx->trigger = NULL; out_free_name: kfree(ctx->name); ctx->name = NULL; -- 2.34.1
