DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Zhang, Tianfei" <tianfei.zhang@intel.com>
To: "Huang, Wei" <wei.huang@intel.com>, "dev@dpdk.org" <dev@dpdk.org>,
	"Xu, Rosen" <rosen.xu@intel.com>,
	"Zhang, Qi Z" <qi.z.zhang@intel.com>,
	"nipun.gupta@nxp.com" <nipun.gupta@nxp.com>,
	"hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>
Cc: "stable@dpdk.org" <stable@dpdk.org>,
	"Yigit, Ferruh" <ferruh.yigit@intel.com>
Subject: RE: [PATCH v1] raw/ifpga: fix interrupt handle allocation
Date: Mon, 21 Feb 2022 06:51:44 +0000	[thread overview]
Message-ID: <BN9PR11MB548383CC25C3CA9E798E4EC9E33A9@BN9PR11MB5483.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20220218073848.268548-1-wei.huang@intel.com>



> -----Original Message-----
> From: Huang, Wei <wei.huang@intel.com>
> Sent: Friday, February 18, 2022 3:39 PM
> To: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; nipun.gupta@nxp.com; hemant.agrawal@nxp.com
> Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Huang, Wei <wei.huang@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: stable@dpdk.org
> 
> Signed-off-by: Wei Huang <wei.huang@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.zhang@intel.com>


  reply	other threads:[~2022-02-21  6:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-18  7:38 Wei Huang
2022-02-21  6:51 ` Zhang, Tianfei [this message]
2022-03-07 22:24   ` Thomas Monjalon
2022-03-08  1:31     ` Huang, Wei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BN9PR11MB548383CC25C3CA9E798E4EC9E33A9@BN9PR11MB5483.namprd11.prod.outlook.com \
    --to=tianfei.zhang@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=nipun.gupta@nxp.com \
    --cc=qi.z.zhang@intel.com \
    --cc=rosen.xu@intel.com \
    --cc=stable@dpdk.org \
    --cc=wei.huang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).