* Re: [PATCH] bus/cdx: provide driver flag for optional resource mapping
2023-06-26 10:39 [PATCH] bus/cdx: provide driver flag for optional resource mapping Abhijit Gangurde
@ 2023-07-04 11:22 ` Gupta, Nipun
2023-07-05 6:52 ` [PATCH v2] " Abhijit Gangurde
` (2 subsequent siblings)
3 siblings, 0 replies; 13+ messages in thread
From: Gupta, Nipun @ 2023-07-04 11:22 UTC (permalink / raw)
To: Abhijit Gangurde, nikhil.agarwal; +Cc: dev, david.marchand, ferruh.yigit
On 6/26/2023 4:09 PM, Abhijit Gangurde wrote:
> Provide driver flag which gives an option to map the cdx
> device resource before probing the device driver.
> Also, make rte_cdx_map_device() API as public to map
> device resource separately.
>
> Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
> ---
> drivers/bus/cdx/bus_cdx_driver.h | 26 ++---------------
> drivers/bus/cdx/cdx.c | 11 ++++---
> drivers/bus/cdx/rte_bus_cdx.h | 50 ++++++++++++++++++++++++++++++++
> drivers/bus/cdx/version.map | 11 +++++--
> 4 files changed, 69 insertions(+), 29 deletions(-)
> create mode 100644 drivers/bus/cdx/rte_bus_cdx.h
>
> diff --git a/drivers/bus/cdx/bus_cdx_driver.h b/drivers/bus/cdx/bus_cdx_driver.h
> index fcacdb5896..1571131417 100644
> --- a/drivers/bus/cdx/bus_cdx_driver.h
> +++ b/drivers/bus/cdx/bus_cdx_driver.h
> @@ -37,6 +37,9 @@ struct rte_cdx_bus;
> static const char DRV_EXP_TAG(name, cdx_tbl_export)[] __rte_used = \
> RTE_STR(table)
>
> +/** Device needs resource mapping */
> +#define RTE_CDX_DRV_NEED_MAPPING 0x0001
> +
> /**
> * A structure describing an ID for a CDX driver. Each driver provides a
> * table of these IDs for each device that it supports.
> @@ -107,29 +110,6 @@ struct rte_cdx_driver {
> uint32_t drv_flags; /**< Flags RTE_CDX_DRV_*. */
> };
>
> -/**
> - * Map the CDX device resources in user space virtual memory address.
> - *
> - * @param dev
> - * A pointer to a rte_cdx_device structure describing the device
> - * to use.
> - *
> - * @return
> - * 0 on success, <0 on error.
> - */
> -__rte_internal
> -int rte_cdx_map_device(struct rte_cdx_device *dev);
> -
> -/**
> - * Unmap this device.
> - *
> - * @param dev
> - * A pointer to a rte_cdx_device structure describing the device
> - * to use.
> - */
> -__rte_internal
> -void rte_cdx_unmap_device(struct rte_cdx_device *dev);
> -
> /**
> * Register a CDX driver.
> *
> diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c
> index 28bbf92ed5..47dc4eabe7 100644
> --- a/drivers/bus/cdx/cdx.c
> +++ b/drivers/bus/cdx/cdx.c
> @@ -74,6 +74,7 @@
> #include <rte_kvargs.h>
> #include <rte_malloc.h>
> #include <rte_vfio.h>
> +#include <rte_bus_cdx.h>
>
> #include <eal_filesystem.h>
>
> @@ -380,10 +381,12 @@ cdx_probe_one_driver(struct rte_cdx_driver *dr,
> CDX_BUS_DEBUG(" probe device %s using driver: %s", dev_name,
> dr->driver.name);
>
> - ret = cdx_vfio_map_resource(dev);
> - if (ret != 0) {
> - CDX_BUS_ERR("CDX map device failed: %d", ret);
> - goto error_map_device;
> + if (dr->drv_flags & RTE_CDX_DRV_NEED_MAPPING) {
> + ret = cdx_vfio_map_resource(dev);
> + if (ret != 0) {
> + CDX_BUS_ERR("CDX map device failed: %d", ret);
> + goto error_map_device;
> + }
> }
>
> /* call the driver probe() function */
> diff --git a/drivers/bus/cdx/rte_bus_cdx.h b/drivers/bus/cdx/rte_bus_cdx.h
> new file mode 100644
> index 0000000000..b9280b5f7f
> --- /dev/null
> +++ b/drivers/bus/cdx/rte_bus_cdx.h
> @@ -0,0 +1,50 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (C) 2023, Advanced Micro Devices, Inc.
> + */
> +
> +#ifndef _RTE_BUS_CDX_H_
> +#define _RTE_BUS_CDX_H_
nit: there are no underscores before or after in other cdx header files.
please change to RTE_BUS_CDX_H
> +
> +/**
> + * @file
> + * CDX device & driver interface
> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/* Forward declarations */
> +struct rte_cdx_device;
> +
> +/**
> + * Map the CDX device resources in user space virtual memory address
nit: all doxygen comments should end with a dot.
> + *
> + * Note that driver should not call this function when flag
> + * RTE_CDX_DRV_NEED_MAPPING is set, as EAL will do that for
> + * you when it's on.
> + *
> + * @param dev
> + * A pointer to a rte_cdx_device structure describing the device
> + * to use
> + *
> + * @return
> + * 0 on success, negative on error and positive if no driver
> + * is found for the device.
> + */
> +int rte_cdx_map_device(struct rte_cdx_device *dev);
> +
> +/**
> + * Unmap this device
> + *
> + * @param dev
> + * A pointer to a rte_cdx_device structure describing the device
> + * to use
> + */
> +void rte_cdx_unmap_device(struct rte_cdx_device *dev);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _RTE_BUS_CDX_H_ */
> diff --git a/drivers/bus/cdx/version.map b/drivers/bus/cdx/version.map
> index 0a15d39ae8..cc7b1f821b 100644
> --- a/drivers/bus/cdx/version.map
> +++ b/drivers/bus/cdx/version.map
> @@ -1,9 +1,16 @@
> -INTERNAL {
> +DPDK_23 {
> global:
>
> rte_cdx_map_device;
> - rte_cdx_register;
> rte_cdx_unmap_device;
> +
> + local: *;
> +};
> +
> +INTERNAL {
> + global:
> +
> + rte_cdx_register;
> rte_cdx_unregister;
> rte_cdx_vfio_intr_disable;
> rte_cdx_vfio_intr_enable;
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v2] bus/cdx: provide driver flag for optional resource mapping
2023-06-26 10:39 [PATCH] bus/cdx: provide driver flag for optional resource mapping Abhijit Gangurde
2023-07-04 11:22 ` Gupta, Nipun
@ 2023-07-05 6:52 ` Abhijit Gangurde
2023-07-11 5:51 ` [PATCH v3] " Abhijit Gangurde
2023-07-07 8:35 ` [PATCH] " Gupta, Nipun
2023-10-16 9:16 ` [PATCH v4 1/1] " Abhijit Gangurde
3 siblings, 1 reply; 13+ messages in thread
From: Abhijit Gangurde @ 2023-07-05 6:52 UTC (permalink / raw)
To: Nipun.Gupta, nikhil.agarwal
Cc: dev, david.marchand, Ferruh.Yigit, thomas, Abhijit Gangurde
Provide driver flag which gives an option to map the cdx
device resource before probing the device driver.
Also, make rte_cdx_map_device() API as public to map
device resource separately.
Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
---
v2:
- Corrected _RTE_BUS_CDX_H_ to RTE_BUS_CDX_H
- Improved code comments.
drivers/bus/cdx/bus_cdx_driver.h | 26 ++---------------
drivers/bus/cdx/cdx.c | 11 ++++---
drivers/bus/cdx/rte_bus_cdx.h | 50 ++++++++++++++++++++++++++++++++
drivers/bus/cdx/version.map | 11 +++++--
4 files changed, 69 insertions(+), 29 deletions(-)
create mode 100644 drivers/bus/cdx/rte_bus_cdx.h
diff --git a/drivers/bus/cdx/bus_cdx_driver.h b/drivers/bus/cdx/bus_cdx_driver.h
index fcacdb5896..1571131417 100644
--- a/drivers/bus/cdx/bus_cdx_driver.h
+++ b/drivers/bus/cdx/bus_cdx_driver.h
@@ -37,6 +37,9 @@ struct rte_cdx_bus;
static const char DRV_EXP_TAG(name, cdx_tbl_export)[] __rte_used = \
RTE_STR(table)
+/** Device needs resource mapping */
+#define RTE_CDX_DRV_NEED_MAPPING 0x0001
+
/**
* A structure describing an ID for a CDX driver. Each driver provides a
* table of these IDs for each device that it supports.
@@ -107,29 +110,6 @@ struct rte_cdx_driver {
uint32_t drv_flags; /**< Flags RTE_CDX_DRV_*. */
};
-/**
- * Map the CDX device resources in user space virtual memory address.
- *
- * @param dev
- * A pointer to a rte_cdx_device structure describing the device
- * to use.
- *
- * @return
- * 0 on success, <0 on error.
- */
-__rte_internal
-int rte_cdx_map_device(struct rte_cdx_device *dev);
-
-/**
- * Unmap this device.
- *
- * @param dev
- * A pointer to a rte_cdx_device structure describing the device
- * to use.
- */
-__rte_internal
-void rte_cdx_unmap_device(struct rte_cdx_device *dev);
-
/**
* Register a CDX driver.
*
diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c
index 28bbf92ed5..47dc4eabe7 100644
--- a/drivers/bus/cdx/cdx.c
+++ b/drivers/bus/cdx/cdx.c
@@ -74,6 +74,7 @@
#include <rte_kvargs.h>
#include <rte_malloc.h>
#include <rte_vfio.h>
+#include <rte_bus_cdx.h>
#include <eal_filesystem.h>
@@ -380,10 +381,12 @@ cdx_probe_one_driver(struct rte_cdx_driver *dr,
CDX_BUS_DEBUG(" probe device %s using driver: %s", dev_name,
dr->driver.name);
- ret = cdx_vfio_map_resource(dev);
- if (ret != 0) {
- CDX_BUS_ERR("CDX map device failed: %d", ret);
- goto error_map_device;
+ if (dr->drv_flags & RTE_CDX_DRV_NEED_MAPPING) {
+ ret = cdx_vfio_map_resource(dev);
+ if (ret != 0) {
+ CDX_BUS_ERR("CDX map device failed: %d", ret);
+ goto error_map_device;
+ }
}
/* call the driver probe() function */
diff --git a/drivers/bus/cdx/rte_bus_cdx.h b/drivers/bus/cdx/rte_bus_cdx.h
new file mode 100644
index 0000000000..80acd668c0
--- /dev/null
+++ b/drivers/bus/cdx/rte_bus_cdx.h
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (C) 2023, Advanced Micro Devices, Inc.
+ */
+
+#ifndef RTE_BUS_CDX_H
+#define RTE_BUS_CDX_H
+
+/**
+ * @file
+ * CDX device & driver interface
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/* Forward declarations */
+struct rte_cdx_device;
+
+/**
+ * Map the CDX device resources in user space virtual memory address.
+ *
+ * Note that driver should not call this function when flag
+ * RTE_CDX_DRV_NEED_MAPPING is set, as EAL will do that for
+ * you when it's on.
+ *
+ * @param dev
+ * A pointer to a rte_cdx_device structure describing the device
+ * to use.
+ *
+ * @return
+ * 0 on success, negative on error and positive if no driver
+ * is found for the device.
+ */
+int rte_cdx_map_device(struct rte_cdx_device *dev);
+
+/**
+ * Unmap this device.
+ *
+ * @param dev
+ * A pointer to a rte_cdx_device structure describing the device
+ * to use.
+ */
+void rte_cdx_unmap_device(struct rte_cdx_device *dev);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* RTE_BUS_CDX_H */
diff --git a/drivers/bus/cdx/version.map b/drivers/bus/cdx/version.map
index 0a15d39ae8..cc7b1f821b 100644
--- a/drivers/bus/cdx/version.map
+++ b/drivers/bus/cdx/version.map
@@ -1,9 +1,16 @@
-INTERNAL {
+DPDK_23 {
global:
rte_cdx_map_device;
- rte_cdx_register;
rte_cdx_unmap_device;
+
+ local: *;
+};
+
+INTERNAL {
+ global:
+
+ rte_cdx_register;
rte_cdx_unregister;
rte_cdx_vfio_intr_disable;
rte_cdx_vfio_intr_enable;
--
2.25.1
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v3] bus/cdx: provide driver flag for optional resource mapping
2023-07-05 6:52 ` [PATCH v2] " Abhijit Gangurde
@ 2023-07-11 5:51 ` Abhijit Gangurde
2023-07-11 15:05 ` Gupta, Nipun
2023-09-29 15:17 ` David Marchand
0 siblings, 2 replies; 13+ messages in thread
From: Abhijit Gangurde @ 2023-07-11 5:51 UTC (permalink / raw)
To: Nipun.Gupta, nikhil.agarwal
Cc: dev, david.marchand, Ferruh.Yigit, thomas, Abhijit Gangurde
Provide driver flag which gives an option to map the cdx
device resource before probing the device driver.
Also, make rte_cdx_map_device() API as public to map
device resource separately.
Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
---
v3:
- Changed APIs to __rte_experimental.
v2:
- Corrected _RTE_BUS_CDX_H_ to RTE_BUS_CDX_H
- Improved code comments.
drivers/bus/cdx/bus_cdx_driver.h | 26 ++--------------
drivers/bus/cdx/cdx.c | 11 ++++---
drivers/bus/cdx/rte_bus_cdx.h | 52 ++++++++++++++++++++++++++++++++
drivers/bus/cdx/version.map | 11 +++++--
4 files changed, 71 insertions(+), 29 deletions(-)
create mode 100644 drivers/bus/cdx/rte_bus_cdx.h
diff --git a/drivers/bus/cdx/bus_cdx_driver.h b/drivers/bus/cdx/bus_cdx_driver.h
index fcacdb5896..1571131417 100644
--- a/drivers/bus/cdx/bus_cdx_driver.h
+++ b/drivers/bus/cdx/bus_cdx_driver.h
@@ -37,6 +37,9 @@ struct rte_cdx_bus;
static const char DRV_EXP_TAG(name, cdx_tbl_export)[] __rte_used = \
RTE_STR(table)
+/** Device needs resource mapping */
+#define RTE_CDX_DRV_NEED_MAPPING 0x0001
+
/**
* A structure describing an ID for a CDX driver. Each driver provides a
* table of these IDs for each device that it supports.
@@ -107,29 +110,6 @@ struct rte_cdx_driver {
uint32_t drv_flags; /**< Flags RTE_CDX_DRV_*. */
};
-/**
- * Map the CDX device resources in user space virtual memory address.
- *
- * @param dev
- * A pointer to a rte_cdx_device structure describing the device
- * to use.
- *
- * @return
- * 0 on success, <0 on error.
- */
-__rte_internal
-int rte_cdx_map_device(struct rte_cdx_device *dev);
-
-/**
- * Unmap this device.
- *
- * @param dev
- * A pointer to a rte_cdx_device structure describing the device
- * to use.
- */
-__rte_internal
-void rte_cdx_unmap_device(struct rte_cdx_device *dev);
-
/**
* Register a CDX driver.
*
diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c
index f9526e08cc..fc8d3b668b 100644
--- a/drivers/bus/cdx/cdx.c
+++ b/drivers/bus/cdx/cdx.c
@@ -74,6 +74,7 @@
#include <rte_kvargs.h>
#include <rte_malloc.h>
#include <rte_vfio.h>
+#include <rte_bus_cdx.h>
#include <eal_filesystem.h>
@@ -383,10 +384,12 @@ cdx_probe_one_driver(struct rte_cdx_driver *dr,
CDX_BUS_DEBUG(" probe device %s using driver: %s", dev_name,
dr->driver.name);
- ret = cdx_vfio_map_resource(dev);
- if (ret != 0) {
- CDX_BUS_ERR("CDX map device failed: %d", ret);
- goto error_map_device;
+ if (dr->drv_flags & RTE_CDX_DRV_NEED_MAPPING) {
+ ret = cdx_vfio_map_resource(dev);
+ if (ret != 0) {
+ CDX_BUS_ERR("CDX map device failed: %d", ret);
+ goto error_map_device;
+ }
}
/* call the driver probe() function */
diff --git a/drivers/bus/cdx/rte_bus_cdx.h b/drivers/bus/cdx/rte_bus_cdx.h
new file mode 100644
index 0000000000..4ca12f90c4
--- /dev/null
+++ b/drivers/bus/cdx/rte_bus_cdx.h
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (C) 2023, Advanced Micro Devices, Inc.
+ */
+
+#ifndef RTE_BUS_CDX_H
+#define RTE_BUS_CDX_H
+
+/**
+ * @file
+ * CDX device & driver interface
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/* Forward declarations */
+struct rte_cdx_device;
+
+/**
+ * Map the CDX device resources in user space virtual memory address.
+ *
+ * Note that driver should not call this function when flag
+ * RTE_CDX_DRV_NEED_MAPPING is set, as EAL will do that for
+ * you when it's on.
+ *
+ * @param dev
+ * A pointer to a rte_cdx_device structure describing the device
+ * to use.
+ *
+ * @return
+ * 0 on success, negative on error and positive if no driver
+ * is found for the device.
+ */
+__rte_experimental
+int rte_cdx_map_device(struct rte_cdx_device *dev);
+
+/**
+ * Unmap this device.
+ *
+ * @param dev
+ * A pointer to a rte_cdx_device structure describing the device
+ * to use.
+ */
+__rte_experimental
+void rte_cdx_unmap_device(struct rte_cdx_device *dev);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* RTE_BUS_CDX_H */
diff --git a/drivers/bus/cdx/version.map b/drivers/bus/cdx/version.map
index 0a15d39ae8..4ce444572b 100644
--- a/drivers/bus/cdx/version.map
+++ b/drivers/bus/cdx/version.map
@@ -1,9 +1,16 @@
-INTERNAL {
+EXPERIMENTAL {
global:
rte_cdx_map_device;
- rte_cdx_register;
rte_cdx_unmap_device;
+
+ local: *;
+};
+
+INTERNAL {
+ global:
+
+ rte_cdx_register;
rte_cdx_unregister;
rte_cdx_vfio_intr_disable;
rte_cdx_vfio_intr_enable;
--
2.25.1
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3] bus/cdx: provide driver flag for optional resource mapping
2023-07-11 5:51 ` [PATCH v3] " Abhijit Gangurde
@ 2023-07-11 15:05 ` Gupta, Nipun
2023-09-29 15:17 ` David Marchand
1 sibling, 0 replies; 13+ messages in thread
From: Gupta, Nipun @ 2023-07-11 15:05 UTC (permalink / raw)
To: Abhijit Gangurde, nikhil.agarwal
Cc: dev, david.marchand, Ferruh.Yigit, thomas
Please use 'in-reply-to' of the first submission when sending any
subsequent patches.
On 7/11/2023 11:21 AM, Abhijit Gangurde wrote:
> Provide driver flag which gives an option to map the cdx
> device resource before probing the device driver.
> Also, make rte_cdx_map_device() API as public to map
> device resource separately.
>
> Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
Acked-by: Nipun Gupta <nipun.gupta@amd.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3] bus/cdx: provide driver flag for optional resource mapping
2023-07-11 5:51 ` [PATCH v3] " Abhijit Gangurde
2023-07-11 15:05 ` Gupta, Nipun
@ 2023-09-29 15:17 ` David Marchand
2023-10-04 10:06 ` Gangurde, Abhijit
1 sibling, 1 reply; 13+ messages in thread
From: David Marchand @ 2023-09-29 15:17 UTC (permalink / raw)
To: Abhijit Gangurde; +Cc: Nipun.Gupta, nikhil.agarwal, dev, Ferruh.Yigit, thomas
On Tue, Jul 11, 2023 at 7:52 AM Abhijit Gangurde
<abhijit.gangurde@amd.com> wrote:
> @@ -383,10 +384,12 @@ cdx_probe_one_driver(struct rte_cdx_driver *dr,
> CDX_BUS_DEBUG(" probe device %s using driver: %s", dev_name,
> dr->driver.name);
>
> - ret = cdx_vfio_map_resource(dev);
> - if (ret != 0) {
> - CDX_BUS_ERR("CDX map device failed: %d", ret);
> - goto error_map_device;
> + if (dr->drv_flags & RTE_CDX_DRV_NEED_MAPPING) {
> + ret = cdx_vfio_map_resource(dev);
> + if (ret != 0) {
> + CDX_BUS_ERR("CDX map device failed: %d", ret);
> + goto error_map_device;
> + }
> }
>
> /* call the driver probe() function */
> diff --git a/drivers/bus/cdx/rte_bus_cdx.h b/drivers/bus/cdx/rte_bus_cdx.h
> new file mode 100644
> index 0000000000..4ca12f90c4
> --- /dev/null
> +++ b/drivers/bus/cdx/rte_bus_cdx.h
> @@ -0,0 +1,52 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright (C) 2023, Advanced Micro Devices, Inc.
> + */
> +
> +#ifndef RTE_BUS_CDX_H
> +#define RTE_BUS_CDX_H
> +
> +/**
> + * @file
> + * CDX device & driver interface
> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/* Forward declarations */
> +struct rte_cdx_device;
> +
> +/**
> + * Map the CDX device resources in user space virtual memory address.
> + *
> + * Note that driver should not call this function when flag
> + * RTE_CDX_DRV_NEED_MAPPING is set, as EAL will do that for
> + * you when it's on.
Why should we export this function in the application ABI, if it is
only used by drivers?
> + *
> + * @param dev
> + * A pointer to a rte_cdx_device structure describing the device
> + * to use.
> + *
> + * @return
> + * 0 on success, negative on error and positive if no driver
> + * is found for the device.
> + */
> +__rte_experimental
> +int rte_cdx_map_device(struct rte_cdx_device *dev);
>
--
David Marchand
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [PATCH v3] bus/cdx: provide driver flag for optional resource mapping
2023-09-29 15:17 ` David Marchand
@ 2023-10-04 10:06 ` Gangurde, Abhijit
2023-10-04 12:54 ` David Marchand
0 siblings, 1 reply; 13+ messages in thread
From: Gangurde, Abhijit @ 2023-10-04 10:06 UTC (permalink / raw)
To: David Marchand; +Cc: Gupta, Nipun, Agarwal, Nikhil, dev, Yigit, Ferruh, thomas
[AMD Official Use Only - General]
> <abhijit.gangurde@amd.com> wrote:
> > @@ -383,10 +384,12 @@ cdx_probe_one_driver(struct rte_cdx_driver *dr,
> > CDX_BUS_DEBUG(" probe device %s using driver: %s", dev_name,
> > dr->driver.name);
> >
> > - ret = cdx_vfio_map_resource(dev);
> > - if (ret != 0) {
> > - CDX_BUS_ERR("CDX map device failed: %d", ret);
> > - goto error_map_device;
> > + if (dr->drv_flags & RTE_CDX_DRV_NEED_MAPPING) {
> > + ret = cdx_vfio_map_resource(dev);
> > + if (ret != 0) {
> > + CDX_BUS_ERR("CDX map device failed: %d", ret);
> > + goto error_map_device;
> > + }
> > }
> >
> > /* call the driver probe() function */
> > diff --git a/drivers/bus/cdx/rte_bus_cdx.h b/drivers/bus/cdx/rte_bus_cdx.h
> > new file mode 100644
> > index 0000000000..4ca12f90c4
> > --- /dev/null
> > +++ b/drivers/bus/cdx/rte_bus_cdx.h
> > @@ -0,0 +1,52 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright (C) 2023, Advanced Micro Devices, Inc.
> > + */
> > +
> > +#ifndef RTE_BUS_CDX_H
> > +#define RTE_BUS_CDX_H
> > +
> > +/**
> > + * @file
> > + * CDX device & driver interface
> > + */
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +/* Forward declarations */
> > +struct rte_cdx_device;
> > +
> > +/**
> > + * Map the CDX device resources in user space virtual memory address.
> > + *
> > + * Note that driver should not call this function when flag
> > + * RTE_CDX_DRV_NEED_MAPPING is set, as EAL will do that for
> > + * you when it's on.
>
> Why should we export this function in the application ABI, if it is
> only used by drivers?
This can be called from an application as well if this flag is not set hence, we need to export this function.
If needed, I can update the description of the API.
Thanks,
Abhijit
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3] bus/cdx: provide driver flag for optional resource mapping
2023-10-04 10:06 ` Gangurde, Abhijit
@ 2023-10-04 12:54 ` David Marchand
2023-10-13 11:51 ` Gangurde, Abhijit
0 siblings, 1 reply; 13+ messages in thread
From: David Marchand @ 2023-10-04 12:54 UTC (permalink / raw)
To: Gangurde, Abhijit
Cc: Gupta, Nipun, Agarwal, Nikhil, dev, Yigit, Ferruh, thomas
On Wed, Oct 4, 2023 at 12:06 PM Gangurde, Abhijit
<abhijit.gangurde@amd.com> wrote:
> > > +/**
> > > + * Map the CDX device resources in user space virtual memory address.
> > > + *
> > > + * Note that driver should not call this function when flag
> > > + * RTE_CDX_DRV_NEED_MAPPING is set, as EAL will do that for
> > > + * you when it's on.
> >
> > Why should we export this function in the application ABI, if it is
> > only used by drivers?
>
> This can be called from an application as well if this flag is not set hence, we need to export this function.
What kind of applications / in which usecase, one would need to map
the device resources?
Except a driver?
--
David Marchand
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH v3] bus/cdx: provide driver flag for optional resource mapping
2023-10-04 12:54 ` David Marchand
@ 2023-10-13 11:51 ` Gangurde, Abhijit
2023-10-13 12:15 ` David Marchand
0 siblings, 1 reply; 13+ messages in thread
From: Gangurde, Abhijit @ 2023-10-13 11:51 UTC (permalink / raw)
To: David Marchand; +Cc: Gupta, Nipun, Agarwal, Nikhil, dev, Yigit, Ferruh, thomas
> > > > +/**
> > > > + * Map the CDX device resources in user space virtual memory address.
> > > > + *
> > > > + * Note that driver should not call this function when flag
> > > > + * RTE_CDX_DRV_NEED_MAPPING is set, as EAL will do that for
> > > > + * you when it's on.
> > >
> > > Why should we export this function in the application ABI, if it is
> > > only used by drivers?
> >
> > This can be called from an application as well if this flag is not set hence, we
> need to export this function.
>
> What kind of applications / in which usecase, one would need to map
> the device resources?
> Except a driver?
I understand that it is probably not the ideal use case, but some of the customers
are using a single application which also registers itself as driver. Probably such
applications need to use internal APIs instead of making these APIs external.
Will analyze it further and send another rev of this patch.
Thanks,
Abhijit
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3] bus/cdx: provide driver flag for optional resource mapping
2023-10-13 11:51 ` Gangurde, Abhijit
@ 2023-10-13 12:15 ` David Marchand
0 siblings, 0 replies; 13+ messages in thread
From: David Marchand @ 2023-10-13 12:15 UTC (permalink / raw)
To: Gangurde, Abhijit
Cc: Gupta, Nipun, Agarwal, Nikhil, dev, Yigit, Ferruh, thomas
On Fri, Oct 13, 2023 at 1:52 PM Gangurde, Abhijit
<abhijit.gangurde@amd.com> wrote:
>
> > > > > +/**
> > > > > + * Map the CDX device resources in user space virtual memory address.
> > > > > + *
> > > > > + * Note that driver should not call this function when flag
> > > > > + * RTE_CDX_DRV_NEED_MAPPING is set, as EAL will do that for
> > > > > + * you when it's on.
> > > >
> > > > Why should we export this function in the application ABI, if it is
> > > > only used by drivers?
> > >
> > > This can be called from an application as well if this flag is not set hence, we
> > need to export this function.
> >
> > What kind of applications / in which usecase, one would need to map
> > the device resources?
> > Except a driver?
>
> I understand that it is probably not the ideal use case, but some of the customers
> are using a single application which also registers itself as driver. Probably such
> applications need to use internal APIs instead of making these APIs external.
> Will analyze it further and send another rev of this patch.
External drivers should be supported with current code.
DPDK must be built with enable_driver_sdk option, and the external
driver code can include bus_cdx_driver.h.
Possibly compiling this code with -DENABLE_INTERNAL_API cflag will be necessary.
--
David Marchand
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] bus/cdx: provide driver flag for optional resource mapping
2023-06-26 10:39 [PATCH] bus/cdx: provide driver flag for optional resource mapping Abhijit Gangurde
2023-07-04 11:22 ` Gupta, Nipun
2023-07-05 6:52 ` [PATCH v2] " Abhijit Gangurde
@ 2023-07-07 8:35 ` Gupta, Nipun
2023-10-16 9:16 ` [PATCH v4 1/1] " Abhijit Gangurde
3 siblings, 0 replies; 13+ messages in thread
From: Gupta, Nipun @ 2023-07-07 8:35 UTC (permalink / raw)
To: Gangurde, Abhijit, Agarwal, Nikhil
Cc: dev, david.marchand, Yigit, Ferruh, Gangurde, Abhijit
> -----Original Message-----
> From: Abhijit Gangurde <abhijit.gangurde@amd.com>
> Sent: Monday, June 26, 2023 4:10 PM
> To: Gupta, Nipun <Nipun.Gupta@amd.com>; Agarwal, Nikhil
> <nikhil.agarwal@amd.com>
> Cc: dev@dpdk.org; david.marchand@redhat.com; Yigit, Ferruh
> <Ferruh.Yigit@amd.com>; Gangurde, Abhijit <abhijit.gangurde@amd.com>
> Subject: [PATCH] bus/cdx: provide driver flag for optional resource mapping
>
> Provide driver flag which gives an option to map the cdx
> device resource before probing the device driver.
> Also, make rte_cdx_map_device() API as public to map
> device resource separately.
>
> Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
> ---
> drivers/bus/cdx/bus_cdx_driver.h | 26 ++---------------
> drivers/bus/cdx/cdx.c | 11 ++++---
> drivers/bus/cdx/rte_bus_cdx.h | 50 ++++++++++++++++++++++++++++++++
> drivers/bus/cdx/version.map | 11 +++++--
> 4 files changed, 69 insertions(+), 29 deletions(-)
> create mode 100644 drivers/bus/cdx/rte_bus_cdx.h
>
<snip>
> +/* Forward declarations */
> +struct rte_cdx_device;
> +
> +/**
> + * Map the CDX device resources in user space virtual memory address
> + *
> + * Note that driver should not call this function when flag
> + * RTE_CDX_DRV_NEED_MAPPING is set, as EAL will do that for
> + * you when it's on.
> + *
> + * @param dev
> + * A pointer to a rte_cdx_device structure describing the device
> + * to use
> + *
> + * @return
> + * 0 on success, negative on error and positive if no driver
> + * is found for the device.
> + */
> +int rte_cdx_map_device(struct rte_cdx_device *dev);
rte_experimental?
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 1/1] bus/cdx: provide driver flag for optional resource mapping
2023-06-26 10:39 [PATCH] bus/cdx: provide driver flag for optional resource mapping Abhijit Gangurde
` (2 preceding siblings ...)
2023-07-07 8:35 ` [PATCH] " Gupta, Nipun
@ 2023-10-16 9:16 ` Abhijit Gangurde
2023-10-16 15:21 ` Thomas Monjalon
3 siblings, 1 reply; 13+ messages in thread
From: Abhijit Gangurde @ 2023-10-16 9:16 UTC (permalink / raw)
To: david.marchand, Nipun.Gupta, nikhil.agarwal
Cc: dev, Ferruh.Yigit, thomas, Abhijit Gangurde
Provide driver flag which gives an option to map the cdx device
resource before probing the device driver. External driver can
use rte_cdx_map_device() and rte_cdx_unmap_device() APIs to map/
unmap device resource separately.
Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
---
v4:
- rte_cdx_map_device() and rte_cdx_unmap_device() APIs are kept
as internal APIs.
v3:
- Changed APIs to __rte_experimental.
v2:
- Corrected _RTE_BUS_CDX_H_ to RTE_BUS_CDX_H
- Improved code comments.
drivers/bus/cdx/bus_cdx_driver.h | 3 +++
drivers/bus/cdx/cdx.c | 10 ++++++----
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/bus/cdx/bus_cdx_driver.h b/drivers/bus/cdx/bus_cdx_driver.h
index fcacdb5896..1c9a64c87a 100644
--- a/drivers/bus/cdx/bus_cdx_driver.h
+++ b/drivers/bus/cdx/bus_cdx_driver.h
@@ -37,6 +37,9 @@ struct rte_cdx_bus;
static const char DRV_EXP_TAG(name, cdx_tbl_export)[] __rte_used = \
RTE_STR(table)
+/** Device needs resource mapping */
+#define RTE_CDX_DRV_NEED_MAPPING 0x0001
+
/**
* A structure describing an ID for a CDX driver. Each driver provides a
* table of these IDs for each device that it supports.
diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c
index f9526e08cc..541aae76c3 100644
--- a/drivers/bus/cdx/cdx.c
+++ b/drivers/bus/cdx/cdx.c
@@ -383,10 +383,12 @@ cdx_probe_one_driver(struct rte_cdx_driver *dr,
CDX_BUS_DEBUG(" probe device %s using driver: %s", dev_name,
dr->driver.name);
- ret = cdx_vfio_map_resource(dev);
- if (ret != 0) {
- CDX_BUS_ERR("CDX map device failed: %d", ret);
- goto error_map_device;
+ if (dr->drv_flags & RTE_CDX_DRV_NEED_MAPPING) {
+ ret = cdx_vfio_map_resource(dev);
+ if (ret != 0) {
+ CDX_BUS_ERR("CDX map device failed: %d", ret);
+ goto error_map_device;
+ }
}
/* call the driver probe() function */
--
2.34.1
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v4 1/1] bus/cdx: provide driver flag for optional resource mapping
2023-10-16 9:16 ` [PATCH v4 1/1] " Abhijit Gangurde
@ 2023-10-16 15:21 ` Thomas Monjalon
0 siblings, 0 replies; 13+ messages in thread
From: Thomas Monjalon @ 2023-10-16 15:21 UTC (permalink / raw)
To: Abhijit Gangurde
Cc: david.marchand, Nipun.Gupta, nikhil.agarwal, dev, Ferruh.Yigit
16/10/2023 11:16, Abhijit Gangurde:
> Provide driver flag which gives an option to map the cdx device
> resource before probing the device driver. External driver can
> use rte_cdx_map_device() and rte_cdx_unmap_device() APIs to map/
> unmap device resource separately.
>
> Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread