Received-SPF: pass (google.com: domain of linux-kernel+bounces-49213-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Reinette Chatre <reinette.chatre@intel.com>
To: jgg@nvidia.com,
	yishaih@nvidia.com,
	shameerali.kolothum.thodi@huawei.com,
	kevin.tian@intel.com,
	alex.williamson@redhat.com
Cc: kvm@vger.kernel.org,
	dave.jiang@intel.com,
	ashok.raj@intel.com,
	reinette.chatre@intel.com,
	linux-kernel@vger.kernel.org,
	patches@lists.linux.dev
Subject: [PATCH 08/17] vfio/pci: Extract MSI/MSI-X specific code from common flow
Date: Thu,  1 Feb 2024 20:57:02 -0800
Message-Id: <79aec9a35a494dcd11be059021a3c4c3f4f74b40.1706849424.git.reinette.chatre@intel.com>
In-Reply-To: <cover.1706849424.git.reinette.chatre@intel.com>
References: <cover.1706849424.git.reinette.chatre@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

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@intel.com>
---
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@intel.com

 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