[v5,5/6] eal/interrupts: make interrupt handle structure opaque

Message ID 20211022204934.132186-6-hkalra@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series make rte_intr_handle internal |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Harman Kalra Oct. 22, 2021, 8:49 p.m. UTC
  Moving interrupt handle structure definition inside the c file
to make its fields totally opaque to the outside world.

Dynamically allocating the efds and elist array os intr_handle
structure, based on size provided by user. Eg size can be
MSIX interrupts supported by a PCI device.

Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
 drivers/bus/pci/linux/pci_vfio.c       |   7 +
 lib/eal/common/eal_common_interrupts.c | 190 +++++++++++++++++++++++--
 lib/eal/include/meson.build            |   1 -
 lib/eal/include/rte_eal_interrupts.h   |  72 ----------
 lib/eal/include/rte_interrupts.h       |  22 ++-
 5 files changed, 209 insertions(+), 83 deletions(-)
 delete mode 100644 lib/eal/include/rte_eal_interrupts.h
  

Comments

Dmitry Kozlyuk Oct. 22, 2021, 11:33 p.m. UTC | #1
2021-10-23 02:19 (UTC+0530), Harman Kalra:
> Moving interrupt handle structure definition inside the c file
> to make its fields totally opaque to the outside world.
> 
> Dynamically allocating the efds and elist array os intr_handle
> structure, based on size provided by user. Eg size can be
> MSIX interrupts supported by a PCI device.
> 
> Signed-off-by: Harman Kalra <hkalra@marvell.com>
> ---
>  drivers/bus/pci/linux/pci_vfio.c       |   7 +
>  lib/eal/common/eal_common_interrupts.c | 190 +++++++++++++++++++++++--
>  lib/eal/include/meson.build            |   1 -
>  lib/eal/include/rte_eal_interrupts.h   |  72 ----------
>  lib/eal/include/rte_interrupts.h       |  22 ++-
>  5 files changed, 209 insertions(+), 83 deletions(-)
>  delete mode 100644 lib/eal/include/rte_eal_interrupts.h

> diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
> [...]
> diff --git a/lib/eal/include/rte_interrupts.h b/lib/eal/include/rte_interrupts.h
> index a29232e16a..fc6b2d1210 100644
> --- a/lib/eal/include/rte_interrupts.h
> +++ b/lib/eal/include/rte_interrupts.h
> @@ -33,7 +33,27 @@ struct rte_intr_handle;
>  /** Interrupt instance could be shared within primary secondary process. */
>  #define RTE_INTR_INSTANCE_F_SHARED	0x00000002
>  
> -#include "rte_eal_interrupts.h"
> +#define RTE_MAX_RXTX_INTR_VEC_ID      512
> +#define RTE_INTR_VEC_ZERO_OFFSET      0
> +#define RTE_INTR_VEC_RXTX_OFFSET      1
> +
> +/**
> + * The interrupt source type, e.g. UIO, VFIO, ALARM etc.
> + */
> +enum rte_intr_handle_type {
> +	RTE_INTR_HANDLE_UNKNOWN = 0,  /**< generic unknown handle */
> +	RTE_INTR_HANDLE_UIO,          /**< uio device handle */
> +	RTE_INTR_HANDLE_UIO_INTX,     /**< uio generic handle */
> +	RTE_INTR_HANDLE_VFIO_LEGACY,  /**< vfio device handle (legacy) */
> +	RTE_INTR_HANDLE_VFIO_MSI,     /**< vfio device handle (MSI) */
> +	RTE_INTR_HANDLE_VFIO_MSIX,    /**< vfio device handle (MSIX) */
> +	RTE_INTR_HANDLE_ALARM,        /**< alarm handle */
> +	RTE_INTR_HANDLE_EXT,          /**< external handler */
> +	RTE_INTR_HANDLE_VDEV,         /**< virtual device */
> +	RTE_INTR_HANDLE_DEV_EVENT,    /**< device event handle */
> +	RTE_INTR_HANDLE_VFIO_REQ,     /**< VFIO request handle */
> +	RTE_INTR_HANDLE_MAX           /**< count of elements */

Wasn't this member going to be removed since v1?

Ray, do you agree?
MAX enum members have been scheduled for removal long ago.
This one even seems unlikely to be used as array size and break anything.

> +};
>  
>  /** Function to be registered for the specific interrupt */
>  typedef void (*rte_intr_callback_fn)(void *cb_arg);
  

Patch

diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
index c8da3e2fe8..f274aa4aab 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -266,6 +266,13 @@  pci_vfio_setup_interrupts(struct rte_pci_device *dev, int vfio_dev_fd)
 			return -1;
 		}
 
+		/* Reallocate the efds and elist fields of intr_handle based
+		 * on PCI device MSIX size.
+		 */
+		if (rte_intr_event_list_update(dev->intr_handle,
+						      irq.count))
+			return -1;
+
 		/* if this vector cannot be used with eventfd, fail if we explicitly
 		 * specified interrupt type, otherwise continue */
 		if ((irq.flags & VFIO_IRQ_INFO_EVENTFD) == 0) {
diff --git a/lib/eal/common/eal_common_interrupts.c b/lib/eal/common/eal_common_interrupts.c
index 618782e9cc..ea98058ac4 100644
--- a/lib/eal/common/eal_common_interrupts.c
+++ b/lib/eal/common/eal_common_interrupts.c
@@ -26,6 +26,29 @@ 
 #define IS_RTE_MEMORY(intr_handle)		\
 		!!(intr_handle->alloc_flag & RTE_INTR_INSTANCE_F_SHARED)
 
+struct rte_intr_handle {
+	RTE_STD_C11
+	union {
+		struct {
+			/** VFIO/UIO cfg device file descriptor */
+			int dev_fd;
+			int fd;	/**< interrupt event file descriptor */
+		};
+		void *windows_handle; /**< device driver handle */
+	};
+	uint32_t alloc_flag;	/** Interrupt instance alloc flag */
+	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 efd_counter_size;     /**< size of efd counter, used for vdev */
+	uint16_t nb_intr;
+		/**< Max vector count, default RTE_MAX_RXTX_INTR_VEC_ID */
+	int *efds;  /**< intr vectors/efds mapping */
+	struct rte_epoll_event *elist; /**< intr vector epoll event */
+	uint16_t vec_list_size;
+	int *intr_vec;                 /**< intr vector number array */
+};
+
 struct rte_intr_handle *rte_intr_instance_alloc(uint32_t flags)
 {
 	struct rte_intr_handle *intr_handle;
@@ -52,16 +75,52 @@  struct rte_intr_handle *rte_intr_instance_alloc(uint32_t flags)
 		return NULL;
 	}
 
+	if (is_rte_memory)
+		intr_handle->efds = rte_zmalloc(NULL,
+						RTE_MAX_RXTX_INTR_VEC_ID *
+						sizeof(uint32_t), 0);
+	else
+		intr_handle->efds = calloc(RTE_MAX_RXTX_INTR_VEC_ID,
+					   sizeof(uint32_t));
+	if (!intr_handle->efds) {
+		RTE_LOG(ERR, EAL, "Fail to allocate event fd list\n");
+		rte_errno = ENOMEM;
+		goto fail;
+	}
+
+	if (is_rte_memory)
+		intr_handle->elist =
+			rte_zmalloc(NULL, RTE_MAX_RXTX_INTR_VEC_ID *
+				    sizeof(struct rte_epoll_event), 0);
+	else
+		intr_handle->elist = calloc(RTE_MAX_RXTX_INTR_VEC_ID,
+					    sizeof(struct rte_epoll_event));
+	if (!intr_handle->elist) {
+		RTE_LOG(ERR, EAL, "fail to allocate event fd list\n");
+		rte_errno = ENOMEM;
+		goto fail;
+	}
+
 	intr_handle->nb_intr = RTE_MAX_RXTX_INTR_VEC_ID;
 	intr_handle->alloc_flag = flags;
 
 	return intr_handle;
+fail:
+	if (is_rte_memory) {
+		rte_free(intr_handle->efds);
+		rte_free(intr_handle);
+	} else {
+		free(intr_handle->efds);
+		free(intr_handle);
+	}
+	return NULL;
 }
 
 int rte_intr_instance_copy(struct rte_intr_handle *intr_handle,
 			   const struct rte_intr_handle *src)
 {
-	uint16_t nb_intr;
+	struct rte_epoll_event *tmp_elist;
+	int *tmp_efds;
 
 	CHECK_VALID_INTR_HANDLE(intr_handle);
 
@@ -72,17 +131,51 @@  int rte_intr_instance_copy(struct rte_intr_handle *intr_handle,
 	}
 
 	intr_handle->fd = src->fd;
-	intr_handle->vfio_dev_fd = src->vfio_dev_fd;
+	intr_handle->dev_fd = src->dev_fd;
 	intr_handle->type = src->type;
+	intr_handle->alloc_flag = src->alloc_flag;
 	intr_handle->max_intr = src->max_intr;
 	intr_handle->nb_efd = src->nb_efd;
 	intr_handle->efd_counter_size = src->efd_counter_size;
 
-	nb_intr = RTE_MIN(src->nb_intr, intr_handle->nb_intr);
-	memcpy(intr_handle->efds, src->efds, nb_intr);
-	memcpy(intr_handle->elist, src->elist, nb_intr);
+	if (intr_handle->nb_intr != src->nb_intr) {
+		if (IS_RTE_MEMORY(src))
+			tmp_efds = rte_realloc(intr_handle->efds, src->nb_intr *
+					       sizeof(uint32_t), 0);
+		else
+			tmp_efds = realloc(intr_handle->efds, src->nb_intr *
+					   sizeof(uint32_t));
+		if (tmp_efds == NULL) {
+			RTE_LOG(ERR, EAL, "Failed to realloc the efds list");
+			rte_errno = ENOMEM;
+			goto fail;
+		}
+
+		if (IS_RTE_MEMORY(src))
+			tmp_elist = rte_realloc(intr_handle->elist,
+						src->nb_intr *
+						sizeof(struct rte_epoll_event),
+						0);
+		else
+			tmp_elist = realloc(intr_handle->elist,	src->nb_intr *
+					    sizeof(struct rte_epoll_event));
+		if (tmp_elist == NULL) {
+			RTE_LOG(ERR, EAL, "Failed to realloc the event list");
+			rte_errno = ENOMEM;
+			goto up_efds;
+		}
+
+		intr_handle->efds = tmp_efds;
+		intr_handle->elist = tmp_elist;
+		intr_handle->nb_intr = src->nb_intr;
+	}
+
+	memcpy(intr_handle->efds, src->efds, src->nb_intr);
+	memcpy(intr_handle->elist, src->elist, src->nb_intr);
 
 	return 0;
+up_efds:
+	intr_handle->efds = tmp_efds;
 fail:
 	return -rte_errno;
 }
@@ -96,13 +189,68 @@  int rte_intr_instance_alloc_flag_get(const struct rte_intr_handle *intr_handle)
 	return -rte_errno;
 }
 
+int rte_intr_event_list_update(struct rte_intr_handle *intr_handle,
+				      int size)
+{
+	struct rte_epoll_event *tmp_elist;
+	int *tmp_efds;
+
+	CHECK_VALID_INTR_HANDLE(intr_handle);
+
+	if (size == 0) {
+		RTE_LOG(ERR, EAL, "Size can't be zero\n");
+		rte_errno = EINVAL;
+		goto fail;
+	}
+
+	if (IS_RTE_MEMORY(intr_handle))
+		tmp_efds = rte_realloc(intr_handle->efds, size *
+				       sizeof(uint32_t), 0);
+	else
+		tmp_efds = realloc(intr_handle->efds, size *
+				   sizeof(uint32_t));
+	if (tmp_efds == NULL) {
+		RTE_LOG(ERR, EAL, "Failed to realloc the efds list");
+		rte_errno = ENOMEM;
+		goto fail;
+	}
+
+	if (IS_RTE_MEMORY(intr_handle))
+		tmp_elist = rte_realloc(intr_handle->elist, size *
+					sizeof(struct rte_epoll_event),
+					0);
+	else
+		tmp_elist = realloc(intr_handle->elist,	size *
+				    sizeof(struct rte_epoll_event));
+	if (tmp_elist == NULL) {
+		RTE_LOG(ERR, EAL, "Failed to realloc the event list");
+		rte_errno = ENOMEM;
+		goto up_efds;
+	}
+
+	intr_handle->efds = tmp_efds;
+	intr_handle->elist = tmp_elist;
+	intr_handle->nb_intr = size;
+
+	return 0;
+up_efds:
+	intr_handle->efds = tmp_efds;
+fail:
+	return -rte_errno;
+}
+
 void rte_intr_instance_free(struct rte_intr_handle *intr_handle)
 {
 	if (intr_handle != NULL) {
-		if (IS_RTE_MEMORY(intr_handle) != 0)
+		if (IS_RTE_MEMORY(intr_handle)) {
+			rte_free(intr_handle->efds);
+			rte_free(intr_handle->elist);
 			rte_free(intr_handle);
-		else
+		} else {
+			free(intr_handle->efds);
+			free(intr_handle->elist);
 			free(intr_handle);
+		}
 	}
 }
 
@@ -152,7 +300,7 @@  int rte_intr_dev_fd_set(struct rte_intr_handle *intr_handle, int fd)
 {
 	CHECK_VALID_INTR_HANDLE(intr_handle);
 
-	intr_handle->vfio_dev_fd = fd;
+	intr_handle->dev_fd = fd;
 
 	return 0;
 fail:
@@ -163,7 +311,7 @@  int rte_intr_dev_fd_get(const struct rte_intr_handle *intr_handle)
 {
 	CHECK_VALID_INTR_HANDLE(intr_handle);
 
-	return intr_handle->vfio_dev_fd;
+	return intr_handle->dev_fd;
 fail:
 	return -1;
 }
@@ -253,6 +401,12 @@  int rte_intr_efds_index_get(const struct rte_intr_handle *intr_handle,
 {
 	CHECK_VALID_INTR_HANDLE(intr_handle);
 
+	if (!intr_handle->efds) {
+		RTE_LOG(ERR, EAL, "Event fd list not allocated\n");
+		rte_errno = EFAULT;
+		goto fail;
+	}
+
 	if (index >= intr_handle->nb_intr) {
 		RTE_LOG(ERR, EAL, "Invalid size %d, max limit %d\n", index,
 			intr_handle->nb_intr);
@@ -270,6 +424,12 @@  int rte_intr_efds_index_set(struct rte_intr_handle *intr_handle,
 {
 	CHECK_VALID_INTR_HANDLE(intr_handle);
 
+	if (!intr_handle->efds) {
+		RTE_LOG(ERR, EAL, "Event fd list not allocated\n");
+		rte_errno = EFAULT;
+		goto fail;
+	}
+
 	if (index >= intr_handle->nb_intr) {
 		RTE_LOG(ERR, EAL, "Invalid size %d, max limit %d\n", index,
 			intr_handle->nb_intr);
@@ -289,6 +449,12 @@  struct rte_epoll_event *rte_intr_elist_index_get(
 {
 	CHECK_VALID_INTR_HANDLE(intr_handle);
 
+	if (!intr_handle->elist) {
+		RTE_LOG(ERR, EAL, "Event list not allocated\n");
+		rte_errno = EFAULT;
+		goto fail;
+	}
+
 	if (index >= intr_handle->nb_intr) {
 		RTE_LOG(ERR, EAL, "Invalid index %d, max limit %d\n", index,
 			intr_handle->nb_intr);
@@ -306,6 +472,12 @@  int rte_intr_elist_index_set(struct rte_intr_handle *intr_handle,
 {
 	CHECK_VALID_INTR_HANDLE(intr_handle);
 
+	if (!intr_handle->elist) {
+		RTE_LOG(ERR, EAL, "Event list not allocated\n");
+		rte_errno = EFAULT;
+		goto fail;
+	}
+
 	if (index >= intr_handle->nb_intr) {
 		RTE_LOG(ERR, EAL, "Invalid index %d, max limit %d\n", index,
 			intr_handle->nb_intr);
diff --git a/lib/eal/include/meson.build b/lib/eal/include/meson.build
index 8e258607b8..86468d1a2b 100644
--- a/lib/eal/include/meson.build
+++ b/lib/eal/include/meson.build
@@ -49,7 +49,6 @@  headers += files(
         'rte_version.h',
         'rte_vfio.h',
 )
-indirect_headers += files('rte_eal_interrupts.h')
 
 # special case install the generic headers, since they go in a subdir
 generic_headers = files(
diff --git a/lib/eal/include/rte_eal_interrupts.h b/lib/eal/include/rte_eal_interrupts.h
deleted file mode 100644
index 26c6300826..0000000000
--- a/lib/eal/include/rte_eal_interrupts.h
+++ /dev/null
@@ -1,72 +0,0 @@ 
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2010-2014 Intel Corporation
- */
-
-#ifndef _RTE_INTERRUPTS_H_
-#error "don't include this file directly, please include generic <rte_interrupts.h>"
-#endif
-
-/**
- * @file rte_eal_interrupts.h
- * @internal
- *
- * Contains function prototypes exposed by the EAL for interrupt handling by
- * drivers and other DPDK internal consumers.
- */
-
-#ifndef _RTE_EAL_INTERRUPTS_H_
-#define _RTE_EAL_INTERRUPTS_H_
-
-#define RTE_MAX_RXTX_INTR_VEC_ID      512
-#define RTE_INTR_VEC_ZERO_OFFSET      0
-#define RTE_INTR_VEC_RXTX_OFFSET      1
-
-/**
- * The interrupt source type, e.g. UIO, VFIO, ALARM etc.
- */
-enum rte_intr_handle_type {
-	RTE_INTR_HANDLE_UNKNOWN = 0,  /**< generic unknown handle */
-	RTE_INTR_HANDLE_UIO,          /**< uio device handle */
-	RTE_INTR_HANDLE_UIO_INTX,     /**< uio generic handle */
-	RTE_INTR_HANDLE_VFIO_LEGACY,  /**< vfio device handle (legacy) */
-	RTE_INTR_HANDLE_VFIO_MSI,     /**< vfio device handle (MSI) */
-	RTE_INTR_HANDLE_VFIO_MSIX,    /**< vfio device handle (MSIX) */
-	RTE_INTR_HANDLE_ALARM,        /**< alarm handle */
-	RTE_INTR_HANDLE_EXT,          /**< external handler */
-	RTE_INTR_HANDLE_VDEV,         /**< virtual device */
-	RTE_INTR_HANDLE_DEV_EVENT,    /**< device event handle */
-	RTE_INTR_HANDLE_VFIO_REQ,     /**< VFIO request handle */
-	RTE_INTR_HANDLE_MAX           /**< count of elements */
-};
-
-/** Handle for interrupts. */
-struct rte_intr_handle {
-	RTE_STD_C11
-	union {
-		struct {
-			RTE_STD_C11
-			union {
-				/** VFIO device file descriptor */
-				int vfio_dev_fd;
-				/** UIO cfg file desc for uio_pci_generic */
-				int uio_cfg_fd;
-			};
-			int fd;	/**< interrupt event file descriptor */
-		};
-		void *windows_handle; /**< device driver handle */
-	};
-	uint32_t alloc_flag;	/** Interrupt Instance allocation flag */
-	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 efd_counter_size;      /**< size of efd counter, used for vdev */
-	uint16_t nb_intr;
-		/**< Max vector count, default RTE_MAX_RXTX_INTR_VEC_ID */
-	int efds[RTE_MAX_RXTX_INTR_VEC_ID];  /**< intr vectors/efds mapping */
-	struct rte_epoll_event elist[RTE_MAX_RXTX_INTR_VEC_ID];
-						/**< intr vector epoll event */
-	uint16_t vec_list_size;
-	int *intr_vec;                 /**< intr vector number array */
-};
-
-#endif /* _RTE_EAL_INTERRUPTS_H_ */
diff --git a/lib/eal/include/rte_interrupts.h b/lib/eal/include/rte_interrupts.h
index a29232e16a..fc6b2d1210 100644
--- a/lib/eal/include/rte_interrupts.h
+++ b/lib/eal/include/rte_interrupts.h
@@ -33,7 +33,27 @@  struct rte_intr_handle;
 /** Interrupt instance could be shared within primary secondary process. */
 #define RTE_INTR_INSTANCE_F_SHARED	0x00000002
 
-#include "rte_eal_interrupts.h"
+#define RTE_MAX_RXTX_INTR_VEC_ID      512
+#define RTE_INTR_VEC_ZERO_OFFSET      0
+#define RTE_INTR_VEC_RXTX_OFFSET      1
+
+/**
+ * The interrupt source type, e.g. UIO, VFIO, ALARM etc.
+ */
+enum rte_intr_handle_type {
+	RTE_INTR_HANDLE_UNKNOWN = 0,  /**< generic unknown handle */
+	RTE_INTR_HANDLE_UIO,          /**< uio device handle */
+	RTE_INTR_HANDLE_UIO_INTX,     /**< uio generic handle */
+	RTE_INTR_HANDLE_VFIO_LEGACY,  /**< vfio device handle (legacy) */
+	RTE_INTR_HANDLE_VFIO_MSI,     /**< vfio device handle (MSI) */
+	RTE_INTR_HANDLE_VFIO_MSIX,    /**< vfio device handle (MSIX) */
+	RTE_INTR_HANDLE_ALARM,        /**< alarm handle */
+	RTE_INTR_HANDLE_EXT,          /**< external handler */
+	RTE_INTR_HANDLE_VDEV,         /**< virtual device */
+	RTE_INTR_HANDLE_DEV_EVENT,    /**< device event handle */
+	RTE_INTR_HANDLE_VFIO_REQ,     /**< VFIO request handle */
+	RTE_INTR_HANDLE_MAX           /**< count of elements */
+};
 
 /** Function to be registered for the specific interrupt */
 typedef void (*rte_intr_callback_fn)(void *cb_arg);