[PATCH 19.11 0/2] backport ifpga patches to stable release 19.11.12

[PATCH 19.11 0/2] backport ifpga patches to stable release 19.11.12

[Wei Huang]

The patches to be backported are
20659eb380  raw/ifpga: fix interrupt handle allocation
2479a1e9a8  raw/ifpga: fix monitor thread

Wei Huang (2):
  raw/ifpga: fix interrupt handle allocation
  raw/ifpga: fix monitor thread

 drivers/raw/ifpga/ifpga_rawdev.c | 168 ++++++++++++++++++++++++---------------
 drivers/raw/ifpga/ifpga_rawdev.h |   9 ++-
 2 files changed, 113 insertions(+), 64 deletions(-)

-- 
1.8.3.1

[PATCH 19.11 1/2] raw/ifpga: fix interrupt handle allocation

[Wei Huang]

[ upstream commit 20659eb38099033b9aa4f7476a7ba7b5d3258569 ]

Allocate FPGA interrupt handle instance for each card.

Fixes: e0a1aaf ("raw/ifpga: introduce IRQ functions")
Cc: stable@dpdk.org

Signed-off-by: Wei Huang <wei.huang@intel.com>
Acked-by: Tianfei Zhang <tianfei.zhang@intel.com>
---
 drivers/raw/ifpga/ifpga_rawdev.c | 88 ++++++++++++++++++++++++++++------------
 drivers/raw/ifpga/ifpga_rawdev.h |  7 +++-
 2 files changed, 67 insertions(+), 28 deletions(-)

diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index 7380433..dfebb7e 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -79,10 +79,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(
@@ -126,6 +122,7 @@ struct ifpga_rawdev *
 {
 	struct ifpga_rawdev *dev;
 	uint16_t dev_id;
+	int i = 0;
 
 	dev = ifpga_rawdev_get(rawdev);
 	if (dev != NULL) {
@@ -142,6 +139,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;
 }
@@ -1335,36 +1334,63 @@ 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;
+	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;
+	}
+
+	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_free(*intr_handle);
+		*intr_handle = NULL;
+	}
 
-	return rte_intr_callback_unregister(intr_handle, handler, arg);
+	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 i;
 
-	adapter = ifpga_rawdev_get_priv(dev);
+	if (!dev || !dev->rawdev)
+		return -ENODEV;
+
+	adapter = ifpga_rawdev_get_priv(dev->rawdev);
 	if (!adapter)
 		return -ENODEV;
 
@@ -1373,29 +1399,37 @@ 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;
 	}
 
-	intr_handle->type = RTE_INTR_HANDLE_VFIO_MSIX;
+	if (*intr_handle)
+		return -EBUSY;
 
-	ret = rte_intr_efd_enable(intr_handle, count);
+	*intr_handle = rte_zmalloc(NULL, sizeof(struct rte_intr_handle), 0);
+	if (!(*intr_handle))
+		return -ENOMEM;
+
+	(*intr_handle)->type = RTE_INTR_HANDLE_VFIO_MSIX;
+
+	ret = rte_intr_efd_enable(*intr_handle, count);
 	if (ret)
 		return -ENODEV;
 
-	intr_handle->fd = intr_handle->efds[0];
+	(*intr_handle)->fd = (*intr_handle)->efds[0];
 
 	IFPGA_RAWDEV_PMD_DEBUG("register %s irq, vfio_fd=%d, fd=%d\n",
-			name, intr_handle->vfio_dev_fd,
-			intr_handle->fd);
+			name, (*intr_handle)->vfio_dev_fd,
+			(*intr_handle)->fd);
 
 	if (type == IFPGA_FME_IRQ) {
 		struct fpga_fme_err_irq_set err_irq_set;
-		err_irq_set.evtfd = intr_handle->efds[0];
+		err_irq_set.evtfd = (*intr_handle)->efds[0];
 
 		ret = opae_manager_ifpga_set_err_irq(mgr, &err_irq_set);
 		if (ret)
@@ -1406,13 +1440,13 @@ static int fme_clean_fme_error(struct opae_manager *mgr)
 			return -EINVAL;
 
 		ret = opae_acc_set_irq(acc, vec_start, count,
-				intr_handle->efds);
+				(*intr_handle)->efds);
 		if (ret)
 			return -EINVAL;
 	}
 
 	/* 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)
 		return -EINVAL;
@@ -1511,7 +1545,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;
@@ -1536,6 +1570,7 @@ static int fme_clean_fme_error(struct opae_manager *mgr)
 	char name[RTE_RAWDEV_NAME_MAX_LEN];
 	struct opae_adapter *adapter;
 	struct opae_manager *mgr;
+	struct ifpga_rawdev *dev;
 
 	if (!pci_dev) {
 		IFPGA_RAWDEV_PMD_ERR("Invalid pci_dev of the device!");
@@ -1555,6 +1590,7 @@ static int fme_clean_fme_error(struct opae_manager *mgr)
 		IFPGA_RAWDEV_PMD_ERR("Invalid device name (%s)", name);
 		return -EINVAL;
 	}
+	dev = ifpga_rawdev_get(rawdev);
 
 	adapter = ifpga_rawdev_get_priv(rawdev);
 	if (!adapter)
@@ -1564,7 +1600,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 7754beb..d82b887 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.h
+++ b/drivers/raw/ifpga/ifpga_rawdev.h
@@ -48,6 +48,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;
@@ -57,6 +58,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 *
@@ -68,12 +71,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);
 
 #endif /* _IFPGA_RAWDEV_H_ */
-- 
1.8.3.1

[PATCH 19.11 2/2] raw/ifpga: fix monitor thread

[Wei Huang]

[ upstream commit 2479a1e9a8886c93ba4c8a7aa2a35a04302f1aae ]

Monitor thread handles graceful shutdown according to the value of
specific sensors in device, two issues are found below.
1. Thread is not created when card is probed.
2. Thread is canceled without checking presence of other cards.
To fix them, thread is created in pci device probe function, a reference
count is checked before canceling the thread.

Fixes: 9c006c4 ("raw/ifpga: scan PCIe BDF device tree")
Cc: stable@dpdk.org

Signed-off-by: Wei Huang <wei.huang@intel.com>
Acked-by: Tianfei Zhang <tianfei.zhang@intel.com>
---
 drivers/raw/ifpga/ifpga_rawdev.c | 80 ++++++++++++++++++++++------------------
 drivers/raw/ifpga/ifpga_rawdev.h |  2 +
 2 files changed, 46 insertions(+), 36 deletions(-)

diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index dfebb7e..b385e17 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -76,7 +76,7 @@
 
 static struct ifpga_rawdev ifpga_rawdevices[IFPGA_RAWDEV_NUM];
 
-static int ifpga_monitor_start;
+static int ifpga_monitor_refcnt;
 static pthread_t ifpga_monitor_start_thread;
 
 static struct ifpga_rawdev *
@@ -141,6 +141,7 @@ struct ifpga_rawdev *
 	dev->dev_id = dev_id;
 	for (i = 0; i < IFPGA_MAX_IRQ; i++)
 		dev->intr_handle[i] = NULL;
+	dev->poll_enabled = 0;
 
 	return dev;
 }
@@ -214,10 +215,10 @@ static int ifpga_get_dev_vendor_id(const char *bdf,
 
 	return 0;
 }
-static int ifpga_rawdev_fill_info(struct ifpga_rawdev *ifpga_dev,
-	const char *bdf)
+static int ifpga_rawdev_fill_info(struct ifpga_rawdev *ifpga_dev)
 {
-	char path[1024] = "/sys/bus/pci/devices/0000:";
+	struct opae_adapter *adapter = NULL;
+	char path[1024] = "/sys/bus/pci/devices/";
 	char link[1024], link1[1024];
 	char dir[1024] = "/sys/devices/";
 	char *c;
@@ -232,7 +233,11 @@ static int ifpga_rawdev_fill_info(struct ifpga_rawdev *ifpga_dev,
 	int func;
 	uint32_t dev_id, vendor_id;
 
-	strlcat(path, bdf, sizeof(path));
+	adapter = ifpga_dev ? ifpga_rawdev_get_priv(ifpga_dev->rawdev) : NULL;
+	if (!adapter)
+		return -ENODEV;
+
+	strlcat(path, adapter->name, sizeof(path));
 	memset(link, 0, sizeof(link));
 	memset(link1, 0, sizeof(link1));
 	ret = readlink(path, link, (sizeof(link)-1));
@@ -382,7 +387,7 @@ static int ifpga_rawdev_fill_info(struct ifpga_rawdev *ifpga_dev,
 		/* monitor temperature sensors */
 		if (!strcmp(sensor->name, "Board Temperature") ||
 				!strcmp(sensor->name, "FPGA Die Temperature")) {
-			IFPGA_RAWDEV_PMD_INFO("read sensor %s %d %d %d\n",
+			IFPGA_RAWDEV_PMD_DEBUG("read sensor %s %d %d %d\n",
 					sensor->name, value, sensor->high_warn,
 					sensor->high_fatal);
 
@@ -424,7 +429,7 @@ static int set_surprise_link_check_aer(
 	bool enable = 0;
 	uint32_t aer_new0, aer_new1;
 
-	if (!ifpga_rdev) {
+	if (!ifpga_rdev || !ifpga_rdev->rawdev) {
 		printf("\n device does not exist\n");
 		return -EFAULT;
 	}
@@ -503,11 +508,11 @@ static int set_surprise_link_check_aer(
 	int gsd_enable, ret;
 #define MS 1000
 
-	while (1) {
+	while (ifpga_monitor_refcnt) {
 		gsd_enable = 0;
 		for (i = 0; i < IFPGA_RAWDEV_NUM; i++) {
 			ifpga_rdev = &ifpga_rawdevices[i];
-			if (ifpga_rdev->rawdev) {
+			if (ifpga_rdev->poll_enabled) {
 				ret = set_surprise_link_check_aer(ifpga_rdev,
 					gsd_enable);
 				if (ret == 1 && !gsd_enable) {
@@ -527,30 +532,45 @@ static int set_surprise_link_check_aer(
 }
 
 static int
-ifpga_monitor_start_func(void)
+ifpga_monitor_start_func(struct ifpga_rawdev *dev)
 {
 	int ret;
 
-	if (ifpga_monitor_start == 0) {
-		ret = pthread_create(&ifpga_monitor_start_thread,
-			NULL,
+	if (!dev)
+		return -ENODEV;
+
+	ret = ifpga_rawdev_fill_info(dev);
+	if (ret)
+		return ret;
+
+	dev->poll_enabled = 1;
+
+	if (ifpga_monitor_refcnt++ == 0) {
+		ret = rte_ctrl_thread_create(&ifpga_monitor_start_thread,
+			"ifpga-monitor", NULL,
 			ifpga_rawdev_gsd_handle, NULL);
 		if (ret) {
+			ifpga_monitor_start_thread = 0;
 			IFPGA_RAWDEV_PMD_ERR(
-				"Fail to create ifpga nonitor thread");
+				"Fail to create ifpga monitor thread");
 			return -1;
 		}
-		ifpga_monitor_start = 1;
 	}
 
 	return 0;
 }
 static int
-ifpga_monitor_stop_func(void)
+ifpga_monitor_stop_func(struct ifpga_rawdev *dev)
 {
 	int ret;
 
-	if (ifpga_monitor_start == 1) {
+	if (!dev || !dev->poll_enabled)
+		return 0;
+
+	dev->poll_enabled = 0;
+
+	if ((--ifpga_monitor_refcnt == 0) &&
+		ifpga_monitor_start_thread) {
 		ret = pthread_cancel(ifpga_monitor_start_thread);
 		if (ret)
 			IFPGA_RAWDEV_PMD_ERR("Can't cancel the thread");
@@ -559,8 +579,6 @@ static int set_surprise_link_check_aer(
 		if (ret)
 			IFPGA_RAWDEV_PMD_ERR("Can't join the thread");
 
-		ifpga_monitor_start = 0;
-
 		return ret;
 	}
 
@@ -1550,6 +1568,10 @@ static int fme_clean_fme_error(struct opae_manager *mgr)
 	if (ret)
 		goto free_adapter_data;
 
+	ret = ifpga_monitor_start_func(dev);
+	if (ret)
+		goto free_adapter_data;
+
 	return ret;
 
 free_adapter_data:
@@ -1592,6 +1614,8 @@ static int fme_clean_fme_error(struct opae_manager *mgr)
 	}
 	dev = ifpga_rawdev_get(rawdev);
 
+	ifpga_monitor_stop_func(dev);
+
 	adapter = ifpga_rawdev_get_priv(rawdev);
 	if (!adapter)
 		return -ENODEV;
@@ -1626,7 +1650,7 @@ static int fme_clean_fme_error(struct opae_manager *mgr)
 static int
 ifpga_rawdev_pci_remove(struct rte_pci_device *pci_dev)
 {
-	ifpga_monitor_stop_func();
+	IFPGA_RAWDEV_PMD_INFO("remove pci_dev %s", pci_dev->device.name);
 	return ifpga_rawdev_destroy(pci_dev);
 }
 
@@ -1679,11 +1703,8 @@ static int ifpga_rawdev_get_string_arg(const char *key __rte_unused,
 {
 	struct rte_devargs *devargs;
 	struct rte_kvargs *kvlist = NULL;
-	struct rte_rawdev *rawdev = NULL;
-	struct ifpga_rawdev *ifpga_dev;
 	int port;
 	char *name = NULL;
-	const char *bdf;
 	char dev_name[RTE_RAWDEV_NAME_MAX_LEN];
 	int ret = -1;
 
@@ -1725,19 +1746,6 @@ static int ifpga_rawdev_get_string_arg(const char *key __rte_unused,
 	}
 
 	memset(dev_name, 0, sizeof(dev_name));
-	snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "IFPGA:%s", name);
-	rawdev = rte_rawdev_pmd_get_named_dev(dev_name);
-	if (!rawdev)
-		goto end;
-	ifpga_dev = ifpga_rawdev_get(rawdev);
-	if (!ifpga_dev)
-		goto end;
-	bdf = name;
-	ifpga_rawdev_fill_info(ifpga_dev, bdf);
-
-	ifpga_monitor_start_func();
-
-	memset(dev_name, 0, sizeof(dev_name));
 	snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s",
 	port, name);
 
diff --git a/drivers/raw/ifpga/ifpga_rawdev.h b/drivers/raw/ifpga/ifpga_rawdev.h
index d82b887..7be4133 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.h
+++ b/drivers/raw/ifpga/ifpga_rawdev.h
@@ -60,6 +60,8 @@ struct ifpga_rawdev {
 	char parent_bdf[16];
 	/* 0 for FME interrupt, others are reserved for AFU irq */
 	void *intr_handle[IFPGA_MAX_IRQ];
+	/* enable monitor thread poll device's sensors or not */
+	int poll_enabled;
 };
 
 struct ifpga_rawdev *
-- 
1.8.3.1

Re: [PATCH 19.11 0/2] backport ifpga patches to stable release 19.11.12

[Christian Ehrhardt]

On Fri, Mar 11, 2022 at 8:18 AM Wei Huang <wei.huang@intel.com> wrote:
>
> The patches to be backported are
> 20659eb380  raw/ifpga: fix interrupt handle allocation
> 2479a1e9a8  raw/ifpga: fix monitor thread
>
> Wei Huang (2):
>   raw/ifpga: fix interrupt handle allocation
>   raw/ifpga: fix monitor thread

Both applied, thank you


>  drivers/raw/ifpga/ifpga_rawdev.c | 168 ++++++++++++++++++++++++---------------
>  drivers/raw/ifpga/ifpga_rawdev.h |   9 ++-
>  2 files changed, 113 insertions(+), 64 deletions(-)
>
> --
> 1.8.3.1
>


--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd