> -----Original Message-----
> From: Huang, Wei <wei.hu...@intel.com>
> Sent: Friday, February 18, 2022 3:39 PM
> To: dev@dpdk.org; Xu, Rosen <rosen...@intel.com>; Zhang, Qi Z
> <qi.z.zh...@intel.com>; nipun.gu...@nxp.com; hemant.agra...@nxp.com
> Cc: sta...@dpdk.org; Zhang, Tianfei <tianfei.zh...@intel.com>; Yigit, Ferruh
> <ferruh.yi...@intel.com>; Huang, Wei <wei.hu...@intel.com>
> Subject: [PATCH v1] raw/ifpga: fix interrupt handle allocation
>
> Allocate FPGA interrupt handle instance for each card.
>
> Fixes: e0a1aafe2af9 ("raw/ifpga: introduce IRQ functions")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Wei Huang <wei.hu...@intel.com>
> ---
> drivers/raw/ifpga/ifpga_rawdev.c | 94 ++++++++++++++++++++++++-------------
> ---
> drivers/raw/ifpga/ifpga_rawdev.h | 7 ++-
> 2 files changed, 62 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/raw/ifpga/ifpga_rawdev.c
> b/drivers/raw/ifpga/ifpga_rawdev.c
> index fdf3c23..f341f4a 100644
> --- a/drivers/raw/ifpga/ifpga_rawdev.c
> +++ b/drivers/raw/ifpga/ifpga_rawdev.c
> @@ -71,10 +71,6 @@
> static int ifpga_monitor_start;
> static pthread_t ifpga_monitor_start_thread;
>
> -#define IFPGA_MAX_IRQ 12
> -/* 0 for FME interrupt, others are reserved for AFU irq */ -static struct
> rte_intr_handle *ifpga_irq_handle[IFPGA_MAX_IRQ];
> -
> static struct ifpga_rawdev *
> ifpga_rawdev_allocate(struct rte_rawdev *rawdev); static int
> set_surprise_link_check_aer( @@ -118,6 +114,7 @@ struct ifpga_rawdev * {
> struct ifpga_rawdev *dev;
> uint16_t dev_id;
> + int i = 0;
>
> dev = ifpga_rawdev_get(rawdev);
> if (dev != NULL) {
> @@ -134,6 +131,8 @@ struct ifpga_rawdev *
> dev = &ifpga_rawdevices[dev_id];
> dev->rawdev = rawdev;
> dev->dev_id = dev_id;
> + for (i = 0; i < IFPGA_MAX_IRQ; i++)
> + dev->intr_handle[i] = NULL;
>
> return dev;
> }
> @@ -1341,49 +1340,62 @@ static int fme_clean_fme_error(struct
> opae_manager *mgr) }
>
> int
> -ifpga_unregister_msix_irq(enum ifpga_irq_type type,
> +ifpga_unregister_msix_irq(struct ifpga_rawdev *dev, enum ifpga_irq_type
> +type,
> int vec_start, rte_intr_callback_fn handler, void *arg) {
> - struct rte_intr_handle *intr_handle;
> - int rc, i;
> + struct rte_intr_handle **intr_handle;
> + int rc = 0;
> + int i = vec_start + 1;
> +
> + if (!dev)
> + return -ENODEV;
>
> if (type == IFPGA_FME_IRQ)
> - intr_handle = ifpga_irq_handle[0];
> + intr_handle = (struct rte_intr_handle **)&dev->intr_handle[0];
> else if (type == IFPGA_AFU_IRQ)
> - intr_handle = ifpga_irq_handle[vec_start + 1];
> + intr_handle = (struct rte_intr_handle **)&dev->intr_handle[i];
> else
> - return 0;
> + return -EINVAL;
>
> - rte_intr_efd_disable(intr_handle);
> + if ((*intr_handle) == NULL) {
> + IFPGA_RAWDEV_PMD_ERR("%s interrupt %d not registered\n",
> + type == IFPGA_FME_IRQ ? "FME" : "AFU",
> + type == IFPGA_FME_IRQ ? 0 : vec_start);
> + return -ENOENT;
> + }
>
> - rc = rte_intr_callback_unregister(intr_handle, handler, arg);
> + rte_intr_efd_disable(*intr_handle);
> +
> + rc = rte_intr_callback_unregister(*intr_handle, handler, arg);
> + if (rc < 0) {
> + IFPGA_RAWDEV_PMD_ERR("Failed to unregister %s interrupt
> %d\n",
> + type == IFPGA_FME_IRQ ? "FME" : "AFU",
> + type == IFPGA_FME_IRQ ? 0 : vec_start);
> + } else {
> + rte_intr_instance_free(*intr_handle);
> + *intr_handle = NULL;
> + }
>
> - for (i = 0; i < IFPGA_MAX_IRQ; i++)
> - rte_intr_instance_free(ifpga_irq_handle[i]);
> return rc;
> }
>
> int
> -ifpga_register_msix_irq(struct rte_rawdev *dev, int port_id,
> +ifpga_register_msix_irq(struct ifpga_rawdev *dev, int port_id,
> enum ifpga_irq_type type, int vec_start, int count,
> rte_intr_callback_fn handler, const char *name,
> void *arg)
> {
> int ret;
> - struct rte_intr_handle *intr_handle;
> + struct rte_intr_handle **intr_handle;
> struct opae_adapter *adapter;
> struct opae_manager *mgr;
> struct opae_accelerator *acc;
> int *intr_efds = NULL, nb_intr, i;
>
> - for (i = 0; i < IFPGA_MAX_IRQ; i++) {
> - ifpga_irq_handle[i] =
> -
> rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
> - if (ifpga_irq_handle[i] == NULL)
> - return -ENOMEM;
> - }
> + if (!dev || !dev->rawdev)
> + return -ENODEV;
>
> - adapter = ifpga_rawdev_get_priv(dev);
> + adapter = ifpga_rawdev_get_priv(dev->rawdev);
> if (!adapter)
> return -ENODEV;
>
> @@ -1392,32 +1404,40 @@ static int fme_clean_fme_error(struct
> opae_manager *mgr)
> return -ENODEV;
>
> if (type == IFPGA_FME_IRQ) {
> - intr_handle = ifpga_irq_handle[0];
> + intr_handle = (struct rte_intr_handle **)&dev->intr_handle[0];
> count = 1;
> } else if (type == IFPGA_AFU_IRQ) {
> - intr_handle = ifpga_irq_handle[vec_start + 1];
> + i = vec_start + 1;
> + intr_handle = (struct rte_intr_handle **)&dev->intr_handle[i];
> } else {
> return -EINVAL;
> }
>
> - if (rte_intr_type_set(intr_handle, RTE_INTR_HANDLE_VFIO_MSIX))
> + if (*intr_handle)
> + return -EBUSY;
> +
> + *intr_handle =
> rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
> + if (!(*intr_handle))
> + return -ENOMEM;
> +
> + if (rte_intr_type_set(*intr_handle, RTE_INTR_HANDLE_VFIO_MSIX))
> return -rte_errno;
>
> - ret = rte_intr_efd_enable(intr_handle, count);
> + ret = rte_intr_efd_enable(*intr_handle, count);
> if (ret)
> return -ENODEV;
>
> - if (rte_intr_fd_set(intr_handle,
> - rte_intr_efds_index_get(intr_handle, 0)))
> + if (rte_intr_fd_set(*intr_handle,
> + rte_intr_efds_index_get(*intr_handle, 0)))
> return -rte_errno;
>
> IFPGA_RAWDEV_PMD_DEBUG("register %s irq, vfio_fd=%d, fd=%d\n",
> - name, rte_intr_dev_fd_get(intr_handle),
> - rte_intr_fd_get(intr_handle));
> + name, rte_intr_dev_fd_get(*intr_handle),
> + rte_intr_fd_get(*intr_handle));
>
> if (type == IFPGA_FME_IRQ) {
> struct fpga_fme_err_irq_set err_irq_set;
> - err_irq_set.evtfd = rte_intr_efds_index_get(intr_handle,
> + err_irq_set.evtfd = rte_intr_efds_index_get(*intr_handle,
> 0);
>
> ret = opae_manager_ifpga_set_err_irq(mgr, &err_irq_set); @@
> -1428,14 +1448,14 @@ static int fme_clean_fme_error(struct opae_manager
> *mgr)
> if (!acc)
> return -EINVAL;
>
> - nb_intr = rte_intr_nb_intr_get(intr_handle);
> + nb_intr = rte_intr_nb_intr_get(*intr_handle);
>
> intr_efds = calloc(nb_intr, sizeof(int));
> if (!intr_efds)
> return -ENOMEM;
>
> for (i = 0; i < nb_intr; i++)
> - intr_efds[i] = rte_intr_efds_index_get(intr_handle, i);
> + intr_efds[i] = rte_intr_efds_index_get(*intr_handle, i);
>
> ret = opae_acc_set_irq(acc, vec_start, count, intr_efds);
> if (ret) {
> @@ -1445,7 +1465,7 @@ static int fme_clean_fme_error(struct opae_manager
> *mgr)
> }
>
> /* register interrupt handler using DPDK API */
> - ret = rte_intr_callback_register(intr_handle,
> + ret = rte_intr_callback_register(*intr_handle,
> handler, (void *)arg);
> if (ret) {
> free(intr_efds);
> @@ -1547,7 +1567,7 @@ static int fme_clean_fme_error(struct opae_manager
> *mgr)
> IFPGA_RAWDEV_PMD_INFO("this is a PF function");
> }
>
> - ret = ifpga_register_msix_irq(rawdev, 0, IFPGA_FME_IRQ, 0, 0,
> + ret = ifpga_register_msix_irq(dev, 0, IFPGA_FME_IRQ, 0, 0,
> fme_interrupt_handler, "fme_irq", mgr);
> if (ret)
> goto free_adapter_data;
> @@ -1604,7 +1624,7 @@ static int fme_clean_fme_error(struct opae_manager
> *mgr)
> if (!mgr)
> return -ENODEV;
>
> - if (ifpga_unregister_msix_irq(IFPGA_FME_IRQ, 0,
> + if (ifpga_unregister_msix_irq(dev, IFPGA_FME_IRQ, 0,
> fme_interrupt_handler, mgr) < 0)
> return -EINVAL;
>
> diff --git a/drivers/raw/ifpga/ifpga_rawdev.h
> b/drivers/raw/ifpga/ifpga_rawdev.h
> index 61c8366..6e09afe 100644
> --- a/drivers/raw/ifpga/ifpga_rawdev.h
> +++ b/drivers/raw/ifpga/ifpga_rawdev.h
> @@ -50,6 +50,7 @@ enum ifpga_rawdev_device_state {
>
> #define IFPGA_RAWDEV_MSIX_IRQ_NUM 7
> #define IFPGA_RAWDEV_NUM 32
> +#define IFPGA_MAX_IRQ 12
>
> struct ifpga_rawdev {
> int dev_id;
> @@ -59,6 +60,8 @@ struct ifpga_rawdev {
> uint32_t aer_old[2];
> char fvl_bdf[8][16];
> char parent_bdf[16];
> + /* 0 for FME interrupt, others are reserved for AFU irq */
> + void *intr_handle[IFPGA_MAX_IRQ];
> };
>
> struct ifpga_rawdev *
> @@ -70,12 +73,12 @@ enum ifpga_irq_type { };
>
> int
> -ifpga_register_msix_irq(struct rte_rawdev *dev, int port_id,
> +ifpga_register_msix_irq(struct ifpga_rawdev *dev, int port_id,
> enum ifpga_irq_type type, int vec_start, int count,
> rte_intr_callback_fn handler, const char *name,
> void *arg);
> int
> -ifpga_unregister_msix_irq(enum ifpga_irq_type type,
> +ifpga_unregister_msix_irq(struct ifpga_rawdev *dev, enum ifpga_irq_type
> +type,
> int vec_start, rte_intr_callback_fn handler, void *arg);
>
> struct rte_pci_bus *ifpga_get_pci_bus(void);
> --
It looks good for me.
Acked-by: Tianfei Zhang <tianfei.zh...@intel.com>
