* [PATCH 19.11 1/2] raw/ifpga: remove virtual devices on close
@ 2022-07-11 3:40 Wei Huang
2022-07-11 3:40 ` [PATCH 19.11 2/2] raw/ifpga: unregister interrupt " Wei Huang
2022-07-11 6:16 ` [PATCH 19.11 1/2] raw/ifpga: remove virtual devices " Christian Ehrhardt
0 siblings, 2 replies; 4+ messages in thread
From: Wei Huang @ 2022-07-11 3:40 UTC (permalink / raw)
To: stable, christian.ehrhardt; +Cc: rosen.xu, tianfei.zhang, Wei Huang
[ upstream commit ae835aba40349ee9631ef6b52e68a2893febe7e0 ]
Virtual devices created on ifpga raw device will not be removed
when ifpga device has closed. To avoid resource leak problem,
this patch introduces an ifpga virtual device remove function,
virtual devices will be destroyed after the ifpga raw device closed.
Fixes: ef1e8ede3da5 ("raw/ifpga: add Intel FPGA bus rawdev driver")
Cc: stable@dpdk.org
Signed-off-by: Wei Huang <wei.huang@intel.com>
Acked-by: Tianfei Zhang <tianfei.zhang@intel.com>
Reviewed-by: Rosen Xu <rosen.xu@intel.com>
---
drivers/raw/ifpga/base/ifpga_api.c | 12 +++
drivers/raw/ifpga/base/ifpga_enumerate.c | 16 +++
drivers/raw/ifpga/base/ifpga_enumerate.h | 1 +
drivers/raw/ifpga/ifpga_rawdev.c | 178 +++++++++++++++++++++++++------
drivers/raw/ifpga/ifpga_rawdev.h | 8 ++
5 files changed, 181 insertions(+), 34 deletions(-)
diff --git a/drivers/raw/ifpga/base/ifpga_api.c b/drivers/raw/ifpga/base/ifpga_api.c
index 6dbd715..1ff57fa 100644
--- a/drivers/raw/ifpga/base/ifpga_api.c
+++ b/drivers/raw/ifpga/base/ifpga_api.c
@@ -330,8 +330,20 @@ static int ifpga_adapter_enumerate(struct opae_adapter *adapter)
return -ENOMEM;
}
+static void ifpga_adapter_destroy(struct opae_adapter *adapter)
+{
+ struct ifpga_fme_hw *fme;
+
+ if (adapter && adapter->mgr && adapter->mgr->data) {
+ fme = (struct ifpga_fme_hw *)adapter->mgr->data;
+ if (fme->parent)
+ ifpga_bus_uinit(fme->parent);
+ }
+}
+
struct opae_adapter_ops ifpga_adapter_ops = {
.enumerate = ifpga_adapter_enumerate,
+ .destroy = ifpga_adapter_destroy,
};
/**
diff --git a/drivers/raw/ifpga/base/ifpga_enumerate.c b/drivers/raw/ifpga/base/ifpga_enumerate.c
index b8846e3..48b8af4 100644
--- a/drivers/raw/ifpga/base/ifpga_enumerate.c
+++ b/drivers/raw/ifpga/base/ifpga_enumerate.c
@@ -722,3 +722,19 @@ int ifpga_bus_init(struct ifpga_hw *hw)
return 0;
}
+
+int ifpga_bus_uinit(struct ifpga_hw *hw)
+{
+ int i;
+ struct ifpga_port_hw *port;
+
+ if (hw) {
+ fme_hw_uinit(&hw->fme);
+ for (i = 0; i < MAX_FPGA_PORT_NUM; i++) {
+ port = &hw->port[i];
+ port_hw_uinit(port);
+ }
+ }
+
+ return 0;
+}
diff --git a/drivers/raw/ifpga/base/ifpga_enumerate.h b/drivers/raw/ifpga/base/ifpga_enumerate.h
index 14131e3..95ed594 100644
--- a/drivers/raw/ifpga/base/ifpga_enumerate.h
+++ b/drivers/raw/ifpga/base/ifpga_enumerate.h
@@ -6,6 +6,7 @@
#define _IFPGA_ENUMERATE_H_
int ifpga_bus_init(struct ifpga_hw *hw);
+int ifpga_bus_uinit(struct ifpga_hw *hw);
int ifpga_bus_enumerate(struct ifpga_hw *hw);
#endif /* _IFPGA_ENUMERATE_H_ */
diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index cc23a27..7f925c3 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -142,6 +142,8 @@ struct ifpga_rawdev *
for (i = 0; i < IFPGA_MAX_IRQ; i++)
dev->intr_handle[i] = NULL;
dev->poll_enabled = 0;
+ for (i = 0; i < IFPGA_MAX_VDEV; i++)
+ dev->vdev_name[i] = NULL;
return dev;
}
@@ -738,6 +740,29 @@ static int set_surprise_link_check_aer(
static int
ifpga_rawdev_close(struct rte_rawdev *dev)
{
+ struct ifpga_rawdev *ifpga_rdev = NULL;
+ struct opae_adapter *adapter;
+ char *vdev_name = NULL;
+ int i = 0;
+
+ if (dev) {
+ ifpga_rdev = ifpga_rawdev_get(dev);
+ if (ifpga_rdev) {
+ for (i = 0; i < IFPGA_MAX_VDEV; i++) {
+ vdev_name = ifpga_rdev->vdev_name[i];
+ if (vdev_name)
+ rte_vdev_uninit(vdev_name);
+ }
+ ifpga_monitor_stop_func(ifpga_rdev);
+ ifpga_rdev->rawdev = NULL;
+ }
+ adapter = ifpga_rawdev_get_priv(dev);
+ if (adapter) {
+ opae_adapter_destroy(adapter);
+ opae_adapter_data_free(adapter->data);
+ }
+ }
+
return dev ? 0:1;
}
@@ -1615,8 +1640,6 @@ 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;
@@ -1629,9 +1652,6 @@ static int fme_clean_fme_error(struct opae_manager *mgr)
fme_interrupt_handler, mgr) < 0)
return -EINVAL;
- opae_adapter_data_free(adapter->data);
- opae_adapter_free(adapter);
-
/* rte_rawdev_close is called by pmd_release */
ret = rte_rawdev_pmd_release(rawdev);
if (ret)
@@ -1699,64 +1719,117 @@ static int ifpga_rawdev_get_string_arg(const char *key __rte_unused,
return 0;
}
+
static int
-ifpga_cfg_probe(struct rte_vdev_device *dev)
+ifpga_vdev_parse_devargs(struct rte_devargs *devargs,
+ struct ifpga_vdev_args *args)
{
- struct rte_devargs *devargs;
- struct rte_kvargs *kvlist = NULL;
- int port;
+ struct rte_kvargs *kvlist;
char *name = NULL;
- char dev_name[RTE_RAWDEV_NAME_MAX_LEN];
- int ret = -1;
+ int port = 0;
+ int ret = -EINVAL;
- devargs = dev->device.devargs;
+ if (!devargs || !args)
+ return ret;
kvlist = rte_kvargs_parse(devargs->args, valid_args);
if (!kvlist) {
- IFPGA_RAWDEV_PMD_LOG(ERR, "error when parsing param");
- goto end;
+ IFPGA_RAWDEV_PMD_ERR("error when parsing devargs");
+ return ret;
}
if (rte_kvargs_count(kvlist, IFPGA_ARG_NAME) == 1) {
if (rte_kvargs_process(kvlist, IFPGA_ARG_NAME,
- &ifpga_rawdev_get_string_arg,
- &name) < 0) {
+ &ifpga_rawdev_get_string_arg, &name) < 0) {
IFPGA_RAWDEV_PMD_ERR("error to parse %s",
- IFPGA_ARG_NAME);
+ IFPGA_ARG_NAME);
goto end;
+ } else {
+ strlcpy(args->bdf, name, sizeof(args->bdf));
+ rte_free(name);
}
} else {
IFPGA_RAWDEV_PMD_ERR("arg %s is mandatory for ifpga bus",
- IFPGA_ARG_NAME);
+ IFPGA_ARG_NAME);
goto end;
}
if (rte_kvargs_count(kvlist, IFPGA_ARG_PORT) == 1) {
- if (rte_kvargs_process(kvlist,
- IFPGA_ARG_PORT,
- &rte_ifpga_get_integer32_arg,
- &port) < 0) {
+ if (rte_kvargs_process(kvlist, IFPGA_ARG_PORT,
+ &rte_ifpga_get_integer32_arg, &port) < 0) {
IFPGA_RAWDEV_PMD_ERR("error to parse %s",
IFPGA_ARG_PORT);
goto end;
+ } else {
+ args->port = port;
}
} else {
IFPGA_RAWDEV_PMD_ERR("arg %s is mandatory for ifpga bus",
- IFPGA_ARG_PORT);
+ IFPGA_ARG_PORT);
goto end;
}
+ ret = 0;
+
+end:
+ rte_kvargs_free(kvlist);
+
+ return ret;
+}
+
+static int
+ifpga_cfg_probe(struct rte_vdev_device *vdev)
+{
+ struct rte_rawdev *rawdev = NULL;
+ struct ifpga_rawdev *ifpga_dev;
+ struct ifpga_vdev_args args;
+ char dev_name[RTE_RAWDEV_NAME_MAX_LEN];
+ const char *vdev_name = NULL;
+ int i, n, ret = 0;
+
+ vdev_name = rte_vdev_device_name(vdev);
+ if (!vdev_name)
+ return -EINVAL;
+
+ IFPGA_RAWDEV_PMD_INFO("probe ifpga virtual device %s", vdev_name);
+
+ ret = ifpga_vdev_parse_devargs(vdev->device.devargs, &args);
+ if (ret)
+ return ret;
+
memset(dev_name, 0, sizeof(dev_name));
- snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s",
- port, name);
+ snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "IFPGA:%s", args.bdf);
+ rawdev = rte_rawdev_pmd_get_named_dev(dev_name);
+ if (!rawdev)
+ return -ENODEV;
+ ifpga_dev = ifpga_rawdev_get(rawdev);
+ if (!ifpga_dev)
+ return -ENODEV;
+
+ for (i = 0; i < IFPGA_MAX_VDEV; i++) {
+ if (ifpga_dev->vdev_name[i] == NULL) {
+ n = strlen(vdev_name) + 1;
+ ifpga_dev->vdev_name[i] = rte_malloc(NULL, n, 0);
+ if (ifpga_dev->vdev_name[i] == NULL)
+ return -ENOMEM;
+ strlcpy(ifpga_dev->vdev_name[i], vdev_name, n);
+ break;
+ }
+ }
+
+ if (i >= IFPGA_MAX_VDEV) {
+ IFPGA_RAWDEV_PMD_ERR("Can't create more virtual device!");
+ return -ENOENT;
+ }
+ snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s",
+ args.port, args.bdf);
ret = rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME),
- dev_name, devargs->args);
-end:
- if (kvlist)
- rte_kvargs_free(kvlist);
- if (name)
- free(name);
+ dev_name, vdev->device.devargs->args);
+ if (ret) {
+ rte_free(ifpga_dev->vdev_name[i]);
+ ifpga_dev->vdev_name[i] = NULL;
+ }
return ret;
}
@@ -1764,10 +1837,47 @@ static int ifpga_rawdev_get_string_arg(const char *key __rte_unused,
static int
ifpga_cfg_remove(struct rte_vdev_device *vdev)
{
- IFPGA_RAWDEV_PMD_INFO("Remove ifpga_cfg %p",
- vdev);
+ struct rte_rawdev *rawdev = NULL;
+ struct ifpga_rawdev *ifpga_dev;
+ struct ifpga_vdev_args args;
+ char dev_name[RTE_RAWDEV_NAME_MAX_LEN];
+ const char *vdev_name = NULL;
+ char *tmp_vdev = NULL;
+ int i, ret = 0;
- return 0;
+ vdev_name = rte_vdev_device_name(vdev);
+ if (!vdev_name)
+ return -EINVAL;
+
+ IFPGA_RAWDEV_PMD_INFO("remove ifpga virtual device %s", vdev_name);
+
+ ret = ifpga_vdev_parse_devargs(vdev->device.devargs, &args);
+ if (ret)
+ return ret;
+
+ memset(dev_name, 0, sizeof(dev_name));
+ snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "IFPGA:%s", args.bdf);
+ rawdev = rte_rawdev_pmd_get_named_dev(dev_name);
+ if (!rawdev)
+ return -ENODEV;
+ ifpga_dev = ifpga_rawdev_get(rawdev);
+ if (!ifpga_dev)
+ return -ENODEV;
+
+ snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s",
+ args.port, args.bdf);
+ ret = rte_eal_hotplug_remove(RTE_STR(IFPGA_BUS_NAME), dev_name);
+
+ for (i = 0; i < IFPGA_MAX_VDEV; i++) {
+ tmp_vdev = ifpga_dev->vdev_name[i];
+ if (tmp_vdev && !strcmp(tmp_vdev, vdev_name)) {
+ free(tmp_vdev);
+ ifpga_dev->vdev_name[i] = NULL;
+ break;
+ }
+ }
+
+ return ret;
}
static struct rte_vdev_driver ifpga_cfg_driver = {
diff --git a/drivers/raw/ifpga/ifpga_rawdev.h b/drivers/raw/ifpga/ifpga_rawdev.h
index 7be4133..8929bd7 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_VDEV 4
#define IFPGA_MAX_IRQ 12
struct ifpga_rawdev {
@@ -62,6 +63,13 @@ struct ifpga_rawdev {
void *intr_handle[IFPGA_MAX_IRQ];
/* enable monitor thread poll device's sensors or not */
int poll_enabled;
+ /* name of virtual devices created on raw device */
+ char *vdev_name[IFPGA_MAX_VDEV];
+};
+
+struct ifpga_vdev_args {
+ char bdf[PCI_PRI_STR_SIZE];
+ int port;
};
struct ifpga_rawdev *
--
1.8.3.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 19.11 2/2] raw/ifpga: unregister interrupt on close
2022-07-11 3:40 [PATCH 19.11 1/2] raw/ifpga: remove virtual devices on close Wei Huang
@ 2022-07-11 3:40 ` Wei Huang
2022-07-11 6:16 ` Christian Ehrhardt
2022-07-11 6:16 ` [PATCH 19.11 1/2] raw/ifpga: remove virtual devices " Christian Ehrhardt
1 sibling, 1 reply; 4+ messages in thread
From: Wei Huang @ 2022-07-11 3:40 UTC (permalink / raw)
To: stable, christian.ehrhardt; +Cc: rosen.xu, tianfei.zhang, Wei Huang
[ upstream commit 2545683564aa15f36392be2b4ceead454064135b ]
There is an API rte_pmd_ifpga_cleanup provided by ifpga driver to
free the software resource used by ifpga card. The function call
of rte_pmd_ifpga_cleanup is list below.
rte_pmd_ifpga_cleanup()
ifpga_rawdev_cleanup()
rte_rawdev_pmd_release()
rte_rawdev_close()
ifpga_rawdev_close()
The interrupts are unregistered in ifpga_rawdev_destroy instead of
ifpga_rawdev_close function, so rte_pmd_ifpga_cleanup cannot free
interrupt resource as expected.
To fix such issue, interrupt unregistration is moved from
ifpga_rawdev_destroy to ifpga_rawdev_close function. The change of
function call of ifpga_rawdev_destroy is as below.
ifpga_rawdev_destroy()
ifpga_unregister_msix_irq() // removed
rte_rawdev_pmd_release()
rte_rawdev_close()
ifpga_rawdev_close()
Fixes: e0a1aafe2af9 ("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>
Reviewed-by: Rosen Xu <rosen.xu@intel.com>
---
drivers/raw/ifpga/ifpga_rawdev.c | 29 +++++++++++------------------
1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index 7f925c3..0d6119c 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -86,6 +86,7 @@ static int set_surprise_link_check_aer(
static int ifpga_pci_find_next_ext_capability(unsigned int fd,
int start, int cap);
static int ifpga_pci_find_ext_capability(unsigned int fd, int cap);
+static void fme_interrupt_handler(void *param);
struct ifpga_rawdev *
ifpga_rawdev_get(const struct rte_rawdev *rawdev)
@@ -742,8 +743,9 @@ static int set_surprise_link_check_aer(
{
struct ifpga_rawdev *ifpga_rdev = NULL;
struct opae_adapter *adapter;
+ struct opae_manager *mgr;
char *vdev_name = NULL;
- int i = 0;
+ int i, ret = 0;
if (dev) {
ifpga_rdev = ifpga_rawdev_get(dev);
@@ -758,12 +760,19 @@ static int set_surprise_link_check_aer(
}
adapter = ifpga_rawdev_get_priv(dev);
if (adapter) {
+ mgr = opae_adapter_get_mgr(adapter);
+ if (ifpga_rdev && mgr) {
+ if (ifpga_unregister_msix_irq(ifpga_rdev,
+ IFPGA_FME_IRQ, 0,
+ fme_interrupt_handler, mgr) < 0)
+ ret = -EINVAL;
+ }
opae_adapter_destroy(adapter);
opae_adapter_data_free(adapter->data);
}
}
- return dev ? 0:1;
+ return ret;
}
static int
@@ -1616,9 +1625,6 @@ static int fme_clean_fme_error(struct opae_manager *mgr)
int ret;
struct rte_rawdev *rawdev;
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!");
@@ -1638,19 +1644,6 @@ 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)
- return -ENODEV;
-
- mgr = opae_adapter_get_mgr(adapter);
- if (!mgr)
- return -ENODEV;
-
- if (ifpga_unregister_msix_irq(dev, IFPGA_FME_IRQ, 0,
- fme_interrupt_handler, mgr) < 0)
- return -EINVAL;
/* rte_rawdev_close is called by pmd_release */
ret = rte_rawdev_pmd_release(rawdev);
--
1.8.3.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 19.11 2/2] raw/ifpga: unregister interrupt on close
2022-07-11 3:40 ` [PATCH 19.11 2/2] raw/ifpga: unregister interrupt " Wei Huang
@ 2022-07-11 6:16 ` Christian Ehrhardt
0 siblings, 0 replies; 4+ messages in thread
From: Christian Ehrhardt @ 2022-07-11 6:16 UTC (permalink / raw)
To: Wei Huang; +Cc: stable, rosen.xu, tianfei.zhang
On Mon, Jul 11, 2022 at 5:32 AM Wei Huang <wei.huang@intel.com> wrote:
>
> [ upstream commit 2545683564aa15f36392be2b4ceead454064135b ]
Thanks, applied
> There is an API rte_pmd_ifpga_cleanup provided by ifpga driver to
> free the software resource used by ifpga card. The function call
> of rte_pmd_ifpga_cleanup is list below.
> rte_pmd_ifpga_cleanup()
> ifpga_rawdev_cleanup()
> rte_rawdev_pmd_release()
> rte_rawdev_close()
> ifpga_rawdev_close()
>
> The interrupts are unregistered in ifpga_rawdev_destroy instead of
> ifpga_rawdev_close function, so rte_pmd_ifpga_cleanup cannot free
> interrupt resource as expected.
>
> To fix such issue, interrupt unregistration is moved from
> ifpga_rawdev_destroy to ifpga_rawdev_close function. The change of
> function call of ifpga_rawdev_destroy is as below.
> ifpga_rawdev_destroy()
> ifpga_unregister_msix_irq() // removed
> rte_rawdev_pmd_release()
> rte_rawdev_close()
> ifpga_rawdev_close()
>
> Fixes: e0a1aafe2af9 ("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>
> Reviewed-by: Rosen Xu <rosen.xu@intel.com>
> ---
> drivers/raw/ifpga/ifpga_rawdev.c | 29 +++++++++++------------------
> 1 file changed, 11 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
> index 7f925c3..0d6119c 100644
> --- a/drivers/raw/ifpga/ifpga_rawdev.c
> +++ b/drivers/raw/ifpga/ifpga_rawdev.c
> @@ -86,6 +86,7 @@ static int set_surprise_link_check_aer(
> static int ifpga_pci_find_next_ext_capability(unsigned int fd,
> int start, int cap);
> static int ifpga_pci_find_ext_capability(unsigned int fd, int cap);
> +static void fme_interrupt_handler(void *param);
>
> struct ifpga_rawdev *
> ifpga_rawdev_get(const struct rte_rawdev *rawdev)
> @@ -742,8 +743,9 @@ static int set_surprise_link_check_aer(
> {
> struct ifpga_rawdev *ifpga_rdev = NULL;
> struct opae_adapter *adapter;
> + struct opae_manager *mgr;
> char *vdev_name = NULL;
> - int i = 0;
> + int i, ret = 0;
>
> if (dev) {
> ifpga_rdev = ifpga_rawdev_get(dev);
> @@ -758,12 +760,19 @@ static int set_surprise_link_check_aer(
> }
> adapter = ifpga_rawdev_get_priv(dev);
> if (adapter) {
> + mgr = opae_adapter_get_mgr(adapter);
> + if (ifpga_rdev && mgr) {
> + if (ifpga_unregister_msix_irq(ifpga_rdev,
> + IFPGA_FME_IRQ, 0,
> + fme_interrupt_handler, mgr) < 0)
> + ret = -EINVAL;
> + }
> opae_adapter_destroy(adapter);
> opae_adapter_data_free(adapter->data);
> }
> }
>
> - return dev ? 0:1;
> + return ret;
> }
>
> static int
> @@ -1616,9 +1625,6 @@ static int fme_clean_fme_error(struct opae_manager *mgr)
> int ret;
> struct rte_rawdev *rawdev;
> 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!");
> @@ -1638,19 +1644,6 @@ 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)
> - return -ENODEV;
> -
> - mgr = opae_adapter_get_mgr(adapter);
> - if (!mgr)
> - return -ENODEV;
> -
> - if (ifpga_unregister_msix_irq(dev, IFPGA_FME_IRQ, 0,
> - fme_interrupt_handler, mgr) < 0)
> - return -EINVAL;
>
> /* rte_rawdev_close is called by pmd_release */
> ret = rte_rawdev_pmd_release(rawdev);
> --
> 1.8.3.1
>
--
Christian Ehrhardt
Senior Staff Engineer, Ubuntu Server
Canonical Ltd
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 19.11 1/2] raw/ifpga: remove virtual devices on close
2022-07-11 3:40 [PATCH 19.11 1/2] raw/ifpga: remove virtual devices on close Wei Huang
2022-07-11 3:40 ` [PATCH 19.11 2/2] raw/ifpga: unregister interrupt " Wei Huang
@ 2022-07-11 6:16 ` Christian Ehrhardt
1 sibling, 0 replies; 4+ messages in thread
From: Christian Ehrhardt @ 2022-07-11 6:16 UTC (permalink / raw)
To: Wei Huang; +Cc: stable, rosen.xu, tianfei.zhang
On Mon, Jul 11, 2022 at 5:32 AM Wei Huang <wei.huang@intel.com> wrote:
>
> [ upstream commit ae835aba40349ee9631ef6b52e68a2893febe7e0 ]
Thanks, applied
> Virtual devices created on ifpga raw device will not be removed
> when ifpga device has closed. To avoid resource leak problem,
> this patch introduces an ifpga virtual device remove function,
> virtual devices will be destroyed after the ifpga raw device closed.
>
> Fixes: ef1e8ede3da5 ("raw/ifpga: add Intel FPGA bus rawdev driver")
> Cc: stable@dpdk.org
>
> Signed-off-by: Wei Huang <wei.huang@intel.com>
> Acked-by: Tianfei Zhang <tianfei.zhang@intel.com>
> Reviewed-by: Rosen Xu <rosen.xu@intel.com>
> ---
> drivers/raw/ifpga/base/ifpga_api.c | 12 +++
> drivers/raw/ifpga/base/ifpga_enumerate.c | 16 +++
> drivers/raw/ifpga/base/ifpga_enumerate.h | 1 +
> drivers/raw/ifpga/ifpga_rawdev.c | 178 +++++++++++++++++++++++++------
> drivers/raw/ifpga/ifpga_rawdev.h | 8 ++
> 5 files changed, 181 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/raw/ifpga/base/ifpga_api.c b/drivers/raw/ifpga/base/ifpga_api.c
> index 6dbd715..1ff57fa 100644
> --- a/drivers/raw/ifpga/base/ifpga_api.c
> +++ b/drivers/raw/ifpga/base/ifpga_api.c
> @@ -330,8 +330,20 @@ static int ifpga_adapter_enumerate(struct opae_adapter *adapter)
> return -ENOMEM;
> }
>
> +static void ifpga_adapter_destroy(struct opae_adapter *adapter)
> +{
> + struct ifpga_fme_hw *fme;
> +
> + if (adapter && adapter->mgr && adapter->mgr->data) {
> + fme = (struct ifpga_fme_hw *)adapter->mgr->data;
> + if (fme->parent)
> + ifpga_bus_uinit(fme->parent);
> + }
> +}
> +
> struct opae_adapter_ops ifpga_adapter_ops = {
> .enumerate = ifpga_adapter_enumerate,
> + .destroy = ifpga_adapter_destroy,
> };
>
> /**
> diff --git a/drivers/raw/ifpga/base/ifpga_enumerate.c b/drivers/raw/ifpga/base/ifpga_enumerate.c
> index b8846e3..48b8af4 100644
> --- a/drivers/raw/ifpga/base/ifpga_enumerate.c
> +++ b/drivers/raw/ifpga/base/ifpga_enumerate.c
> @@ -722,3 +722,19 @@ int ifpga_bus_init(struct ifpga_hw *hw)
>
> return 0;
> }
> +
> +int ifpga_bus_uinit(struct ifpga_hw *hw)
> +{
> + int i;
> + struct ifpga_port_hw *port;
> +
> + if (hw) {
> + fme_hw_uinit(&hw->fme);
> + for (i = 0; i < MAX_FPGA_PORT_NUM; i++) {
> + port = &hw->port[i];
> + port_hw_uinit(port);
> + }
> + }
> +
> + return 0;
> +}
> diff --git a/drivers/raw/ifpga/base/ifpga_enumerate.h b/drivers/raw/ifpga/base/ifpga_enumerate.h
> index 14131e3..95ed594 100644
> --- a/drivers/raw/ifpga/base/ifpga_enumerate.h
> +++ b/drivers/raw/ifpga/base/ifpga_enumerate.h
> @@ -6,6 +6,7 @@
> #define _IFPGA_ENUMERATE_H_
>
> int ifpga_bus_init(struct ifpga_hw *hw);
> +int ifpga_bus_uinit(struct ifpga_hw *hw);
> int ifpga_bus_enumerate(struct ifpga_hw *hw);
>
> #endif /* _IFPGA_ENUMERATE_H_ */
> diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
> index cc23a27..7f925c3 100644
> --- a/drivers/raw/ifpga/ifpga_rawdev.c
> +++ b/drivers/raw/ifpga/ifpga_rawdev.c
> @@ -142,6 +142,8 @@ struct ifpga_rawdev *
> for (i = 0; i < IFPGA_MAX_IRQ; i++)
> dev->intr_handle[i] = NULL;
> dev->poll_enabled = 0;
> + for (i = 0; i < IFPGA_MAX_VDEV; i++)
> + dev->vdev_name[i] = NULL;
>
> return dev;
> }
> @@ -738,6 +740,29 @@ static int set_surprise_link_check_aer(
> static int
> ifpga_rawdev_close(struct rte_rawdev *dev)
> {
> + struct ifpga_rawdev *ifpga_rdev = NULL;
> + struct opae_adapter *adapter;
> + char *vdev_name = NULL;
> + int i = 0;
> +
> + if (dev) {
> + ifpga_rdev = ifpga_rawdev_get(dev);
> + if (ifpga_rdev) {
> + for (i = 0; i < IFPGA_MAX_VDEV; i++) {
> + vdev_name = ifpga_rdev->vdev_name[i];
> + if (vdev_name)
> + rte_vdev_uninit(vdev_name);
> + }
> + ifpga_monitor_stop_func(ifpga_rdev);
> + ifpga_rdev->rawdev = NULL;
> + }
> + adapter = ifpga_rawdev_get_priv(dev);
> + if (adapter) {
> + opae_adapter_destroy(adapter);
> + opae_adapter_data_free(adapter->data);
> + }
> + }
> +
> return dev ? 0:1;
> }
>
> @@ -1615,8 +1640,6 @@ 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;
> @@ -1629,9 +1652,6 @@ static int fme_clean_fme_error(struct opae_manager *mgr)
> fme_interrupt_handler, mgr) < 0)
> return -EINVAL;
>
> - opae_adapter_data_free(adapter->data);
> - opae_adapter_free(adapter);
> -
> /* rte_rawdev_close is called by pmd_release */
> ret = rte_rawdev_pmd_release(rawdev);
> if (ret)
> @@ -1699,64 +1719,117 @@ static int ifpga_rawdev_get_string_arg(const char *key __rte_unused,
>
> return 0;
> }
> +
> static int
> -ifpga_cfg_probe(struct rte_vdev_device *dev)
> +ifpga_vdev_parse_devargs(struct rte_devargs *devargs,
> + struct ifpga_vdev_args *args)
> {
> - struct rte_devargs *devargs;
> - struct rte_kvargs *kvlist = NULL;
> - int port;
> + struct rte_kvargs *kvlist;
> char *name = NULL;
> - char dev_name[RTE_RAWDEV_NAME_MAX_LEN];
> - int ret = -1;
> + int port = 0;
> + int ret = -EINVAL;
>
> - devargs = dev->device.devargs;
> + if (!devargs || !args)
> + return ret;
>
> kvlist = rte_kvargs_parse(devargs->args, valid_args);
> if (!kvlist) {
> - IFPGA_RAWDEV_PMD_LOG(ERR, "error when parsing param");
> - goto end;
> + IFPGA_RAWDEV_PMD_ERR("error when parsing devargs");
> + return ret;
> }
>
> if (rte_kvargs_count(kvlist, IFPGA_ARG_NAME) == 1) {
> if (rte_kvargs_process(kvlist, IFPGA_ARG_NAME,
> - &ifpga_rawdev_get_string_arg,
> - &name) < 0) {
> + &ifpga_rawdev_get_string_arg, &name) < 0) {
> IFPGA_RAWDEV_PMD_ERR("error to parse %s",
> - IFPGA_ARG_NAME);
> + IFPGA_ARG_NAME);
> goto end;
> + } else {
> + strlcpy(args->bdf, name, sizeof(args->bdf));
> + rte_free(name);
> }
> } else {
> IFPGA_RAWDEV_PMD_ERR("arg %s is mandatory for ifpga bus",
> - IFPGA_ARG_NAME);
> + IFPGA_ARG_NAME);
> goto end;
> }
>
> if (rte_kvargs_count(kvlist, IFPGA_ARG_PORT) == 1) {
> - if (rte_kvargs_process(kvlist,
> - IFPGA_ARG_PORT,
> - &rte_ifpga_get_integer32_arg,
> - &port) < 0) {
> + if (rte_kvargs_process(kvlist, IFPGA_ARG_PORT,
> + &rte_ifpga_get_integer32_arg, &port) < 0) {
> IFPGA_RAWDEV_PMD_ERR("error to parse %s",
> IFPGA_ARG_PORT);
> goto end;
> + } else {
> + args->port = port;
> }
> } else {
> IFPGA_RAWDEV_PMD_ERR("arg %s is mandatory for ifpga bus",
> - IFPGA_ARG_PORT);
> + IFPGA_ARG_PORT);
> goto end;
> }
>
> + ret = 0;
> +
> +end:
> + rte_kvargs_free(kvlist);
> +
> + return ret;
> +}
> +
> +static int
> +ifpga_cfg_probe(struct rte_vdev_device *vdev)
> +{
> + struct rte_rawdev *rawdev = NULL;
> + struct ifpga_rawdev *ifpga_dev;
> + struct ifpga_vdev_args args;
> + char dev_name[RTE_RAWDEV_NAME_MAX_LEN];
> + const char *vdev_name = NULL;
> + int i, n, ret = 0;
> +
> + vdev_name = rte_vdev_device_name(vdev);
> + if (!vdev_name)
> + return -EINVAL;
> +
> + IFPGA_RAWDEV_PMD_INFO("probe ifpga virtual device %s", vdev_name);
> +
> + ret = ifpga_vdev_parse_devargs(vdev->device.devargs, &args);
> + if (ret)
> + return ret;
> +
> memset(dev_name, 0, sizeof(dev_name));
> - snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s",
> - port, name);
> + snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "IFPGA:%s", args.bdf);
> + rawdev = rte_rawdev_pmd_get_named_dev(dev_name);
> + if (!rawdev)
> + return -ENODEV;
> + ifpga_dev = ifpga_rawdev_get(rawdev);
> + if (!ifpga_dev)
> + return -ENODEV;
> +
> + for (i = 0; i < IFPGA_MAX_VDEV; i++) {
> + if (ifpga_dev->vdev_name[i] == NULL) {
> + n = strlen(vdev_name) + 1;
> + ifpga_dev->vdev_name[i] = rte_malloc(NULL, n, 0);
> + if (ifpga_dev->vdev_name[i] == NULL)
> + return -ENOMEM;
> + strlcpy(ifpga_dev->vdev_name[i], vdev_name, n);
> + break;
> + }
> + }
> +
> + if (i >= IFPGA_MAX_VDEV) {
> + IFPGA_RAWDEV_PMD_ERR("Can't create more virtual device!");
> + return -ENOENT;
> + }
>
> + snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s",
> + args.port, args.bdf);
> ret = rte_eal_hotplug_add(RTE_STR(IFPGA_BUS_NAME),
> - dev_name, devargs->args);
> -end:
> - if (kvlist)
> - rte_kvargs_free(kvlist);
> - if (name)
> - free(name);
> + dev_name, vdev->device.devargs->args);
> + if (ret) {
> + rte_free(ifpga_dev->vdev_name[i]);
> + ifpga_dev->vdev_name[i] = NULL;
> + }
>
> return ret;
> }
> @@ -1764,10 +1837,47 @@ static int ifpga_rawdev_get_string_arg(const char *key __rte_unused,
> static int
> ifpga_cfg_remove(struct rte_vdev_device *vdev)
> {
> - IFPGA_RAWDEV_PMD_INFO("Remove ifpga_cfg %p",
> - vdev);
> + struct rte_rawdev *rawdev = NULL;
> + struct ifpga_rawdev *ifpga_dev;
> + struct ifpga_vdev_args args;
> + char dev_name[RTE_RAWDEV_NAME_MAX_LEN];
> + const char *vdev_name = NULL;
> + char *tmp_vdev = NULL;
> + int i, ret = 0;
>
> - return 0;
> + vdev_name = rte_vdev_device_name(vdev);
> + if (!vdev_name)
> + return -EINVAL;
> +
> + IFPGA_RAWDEV_PMD_INFO("remove ifpga virtual device %s", vdev_name);
> +
> + ret = ifpga_vdev_parse_devargs(vdev->device.devargs, &args);
> + if (ret)
> + return ret;
> +
> + memset(dev_name, 0, sizeof(dev_name));
> + snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "IFPGA:%s", args.bdf);
> + rawdev = rte_rawdev_pmd_get_named_dev(dev_name);
> + if (!rawdev)
> + return -ENODEV;
> + ifpga_dev = ifpga_rawdev_get(rawdev);
> + if (!ifpga_dev)
> + return -ENODEV;
> +
> + snprintf(dev_name, RTE_RAWDEV_NAME_MAX_LEN, "%d|%s",
> + args.port, args.bdf);
> + ret = rte_eal_hotplug_remove(RTE_STR(IFPGA_BUS_NAME), dev_name);
> +
> + for (i = 0; i < IFPGA_MAX_VDEV; i++) {
> + tmp_vdev = ifpga_dev->vdev_name[i];
> + if (tmp_vdev && !strcmp(tmp_vdev, vdev_name)) {
> + free(tmp_vdev);
> + ifpga_dev->vdev_name[i] = NULL;
> + break;
> + }
> + }
> +
> + return ret;
> }
>
> static struct rte_vdev_driver ifpga_cfg_driver = {
> diff --git a/drivers/raw/ifpga/ifpga_rawdev.h b/drivers/raw/ifpga/ifpga_rawdev.h
> index 7be4133..8929bd7 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_VDEV 4
> #define IFPGA_MAX_IRQ 12
>
> struct ifpga_rawdev {
> @@ -62,6 +63,13 @@ struct ifpga_rawdev {
> void *intr_handle[IFPGA_MAX_IRQ];
> /* enable monitor thread poll device's sensors or not */
> int poll_enabled;
> + /* name of virtual devices created on raw device */
> + char *vdev_name[IFPGA_MAX_VDEV];
> +};
> +
> +struct ifpga_vdev_args {
> + char bdf[PCI_PRI_STR_SIZE];
> + int port;
> };
>
> struct ifpga_rawdev *
> --
> 1.8.3.1
>
--
Christian Ehrhardt
Senior Staff Engineer, Ubuntu Server
Canonical Ltd
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-07-11 6:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-11 3:40 [PATCH 19.11 1/2] raw/ifpga: remove virtual devices on close Wei Huang
2022-07-11 3:40 ` [PATCH 19.11 2/2] raw/ifpga: unregister interrupt " Wei Huang
2022-07-11 6:16 ` Christian Ehrhardt
2022-07-11 6:16 ` [PATCH 19.11 1/2] raw/ifpga: remove virtual devices " Christian Ehrhardt
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).