[PATCH 08/17] vfio/pci: Extract MSI/MSI-X specific code from common flow

From: Reinette Chatre
Date: Fri Feb 02 2024 - 00:00:14 EST


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