[RFC] vfio: avoid re-installing irq handler

Message ID 20190715155812.10124-1-hyonkim@cisco.com (mailing list archive)
State RFC, archived
Delegated to: David Marchand
Headers
Series [RFC] vfio: avoid re-installing irq handler |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

Hyong Youb Kim (hyonkim) July 15, 2019, 3:58 p.m. UTC
  A rough patch for the approach mentioned earlier. It is only for
discussion.
http://mails.dpdk.org/archives/dev/2019-July/138113.html

To try this out, first revert the following then apply.
commit 89aac60e0be9 ("vfio: fix interrupts race condition")

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
---
 .../common/include/rte_eal_interrupts.h       |  1 +
 lib/librte_eal/linux/eal/eal_interrupts.c     | 68 ++++++++++++++++++-
 2 files changed, 67 insertions(+), 2 deletions(-)
  

Patch

diff --git a/lib/librte_eal/common/include/rte_eal_interrupts.h b/lib/librte_eal/common/include/rte_eal_interrupts.h
index b370c0d26..d509967d3 100644
--- a/lib/librte_eal/common/include/rte_eal_interrupts.h
+++ b/lib/librte_eal/common/include/rte_eal_interrupts.h
@@ -76,6 +76,7 @@  struct rte_intr_handle {
 	enum rte_intr_handle_type type;  /**< handle type */
 	uint32_t max_intr;             /**< max interrupt requested */
 	uint32_t nb_efd;               /**< number of available efd(event fd) */
+	uint8_t vfio_efd_assigned;
 	uint8_t efd_counter_size;      /**< size of efd counter, used for vdev */
 	int efds[RTE_MAX_RXTX_INTR_VEC_ID];  /**< intr vectors/efds mapping */
 	struct rte_epoll_event elist[RTE_MAX_RXTX_INTR_VEC_ID];
diff --git a/lib/librte_eal/linux/eal/eal_interrupts.c b/lib/librte_eal/linux/eal/eal_interrupts.c
index 79ad5e8d7..5b03388dd 100644
--- a/lib/librte_eal/linux/eal/eal_interrupts.c
+++ b/lib/librte_eal/linux/eal/eal_interrupts.c
@@ -107,9 +107,21 @@  static pthread_t intr_thread;
 #define MSIX_IRQ_SET_BUF_LEN (sizeof(struct vfio_irq_set) + \
 			      sizeof(int) * (RTE_MAX_RXTX_INTR_VEC_ID + 1))
 
-/* enable legacy (INTx) interrupts */
+#ifndef __INTEL_COMPILER
+#pragma GCC diagnostic ignored "-Wcast-qual"
+#endif
+
+static void
+set_vfio_efd_assigned(const struct rte_intr_handle *intr_handle, uint8_t v) {
+	((struct rte_intr_handle *)intr_handle)->vfio_efd_assigned = v;
+}
+
+#ifndef __INTEL_COMPILER
+#pragma GCC diagnostic error "-Wcast-qual"
+#endif
+
 static int
-vfio_enable_intx(const struct rte_intr_handle *intr_handle) {
+vfio_assign_efd_intx(const struct rte_intr_handle *intr_handle) {
 	struct vfio_irq_set *irq_set;
 	char irq_set_buf[IRQ_SET_BUF_LEN];
 	int len, ret;
@@ -134,8 +146,36 @@  vfio_enable_intx(const struct rte_intr_handle *intr_handle) {
 						intr_handle->fd);
 		return -1;
 	}
+	/*
+	 * In the kernel, irq has been set up, its handler installed (request_irq),
+	 * and efd is assigned to it.  Remember this so we can avoid
+	 * re-doing irq setup later.
+	 */
+	set_vfio_efd_assigned(intr_handle, 1);
+	return 0;
+}
+
+/* enable legacy (INTx) interrupts */
+static int
+vfio_enable_intx(const struct rte_intr_handle *intr_handle) {
+	struct vfio_irq_set *irq_set;
+	char irq_set_buf[IRQ_SET_BUF_LEN];
+	int len, ret;
+
+	/*
+	 * Assign efd to irq only if it's not been done, to avoid
+	 * re-installing irq handler in the kernel.
+	 */
+	if (!intr_handle->vfio_efd_assigned) {
+		ret = vfio_assign_efd_intx(intr_handle);
+		if (ret)
+			return ret;
+	}
+
+	len = sizeof(irq_set_buf);
 
 	/* unmask INTx after enabling */
+	irq_set = (struct vfio_irq_set *) irq_set_buf;
 	memset(irq_set, 0, len);
 	len = sizeof(struct vfio_irq_set);
 	irq_set->argsz = len;
@@ -194,6 +234,12 @@  vfio_disable_intx(const struct rte_intr_handle *intr_handle) {
 			"Error disabling INTx interrupts for fd %d\n", intr_handle->fd);
 		return -1;
 	}
+	/*
+	 * Irq handler's been removed in the kernel. The next time we
+	 * enable intr_handle, we need to assign efd to irq. Remember
+	 * that.
+	 */
+	set_vfio_efd_assigned(intr_handle, 0);
 	return 0;
 }
 
@@ -205,6 +251,13 @@  vfio_enable_msi(const struct rte_intr_handle *intr_handle) {
 	struct vfio_irq_set *irq_set;
 	int *fd_ptr;
 
+	/*
+	 * vfio does not support MSI masking/unmaking. So if irq has
+	 * been set up previously, nothing to do.
+	 */
+	if (intr_handle->vfio_efd_assigned)
+		return 0;
+
 	len = sizeof(irq_set_buf);
 
 	irq_set = (struct vfio_irq_set *) irq_set_buf;
@@ -223,6 +276,7 @@  vfio_enable_msi(const struct rte_intr_handle *intr_handle) {
 						intr_handle->fd);
 		return -1;
 	}
+	set_vfio_efd_assigned(intr_handle, 1);
 	return 0;
 }
 
@@ -248,6 +302,7 @@  vfio_disable_msi(const struct rte_intr_handle *intr_handle) {
 		RTE_LOG(ERR, EAL,
 			"Error disabling MSI interrupts for fd %d\n", intr_handle->fd);
 
+	set_vfio_efd_assigned(intr_handle, 0);
 	return ret;
 }
 
@@ -259,6 +314,13 @@  vfio_enable_msix(const struct rte_intr_handle *intr_handle) {
 	struct vfio_irq_set *irq_set;
 	int *fd_ptr;
 
+	/*
+	 * vfio does not support MSI-X masking/unmaking. So if irq has
+	 * been set up previously, nothing to do.
+	 */
+	if (intr_handle->vfio_efd_assigned)
+		return 0;
+
 	len = sizeof(irq_set_buf);
 
 	irq_set = (struct vfio_irq_set *) irq_set_buf;
@@ -284,6 +346,7 @@  vfio_enable_msix(const struct rte_intr_handle *intr_handle) {
 		return -1;
 	}
 
+	set_vfio_efd_assigned(intr_handle, 1);
 	return 0;
 }
 
@@ -309,6 +372,7 @@  vfio_disable_msix(const struct rte_intr_handle *intr_handle) {
 		RTE_LOG(ERR, EAL,
 			"Error disabling MSI-X interrupts for fd %d\n", intr_handle->fd);
 
+	set_vfio_efd_assigned(intr_handle, 0);
 	return ret;
 }