[PATCH] Add bus master enable/disable APIs for CDX devices

[PATCH] Add bus master enable/disable APIs for CDX devices

[Ferruh Yigit]

From: Shubham Rohila <shubham.rohila@amd.com>

Define rte_cdx_vfio_bm_enable and rte_cdx_vfio_bm_disable to
enable or disable bus master functionality for cdx devices.

Signed-off-by: Shubham Rohila <shubham.rohila@amd.com>
---
 drivers/bus/cdx/bus_cdx_driver.h | 25 ++++++++
 drivers/bus/cdx/cdx_vfio.c       | 97 ++++++++++++++++++++++++++++++++
 drivers/bus/cdx/version.map      |  2 +
 lib/eal/include/rte_vfio.h       | 27 +++++++++
 4 files changed, 151 insertions(+)

diff --git a/drivers/bus/cdx/bus_cdx_driver.h b/drivers/bus/cdx/bus_cdx_driver.h
index 1c9a64c87ac1..2f94a6c3afff 100644
--- a/drivers/bus/cdx/bus_cdx_driver.h
+++ b/drivers/bus/cdx/bus_cdx_driver.h
@@ -16,6 +16,7 @@ extern "C" {
 
 #include <stdlib.h>
 #include <inttypes.h>
+#include <linux/types.h>
 
 #include <bus_driver.h>
 #include <dev_driver.h>
@@ -178,6 +179,30 @@ int rte_cdx_vfio_intr_enable(const struct rte_intr_handle *intr_handle);
 __rte_internal
 int rte_cdx_vfio_intr_disable(const struct rte_intr_handle *intr_handle);
 
+/**
+ * Enable Bus Mastering for CDX bus devices.
+ *
+ * @param dev
+ *   Pointer to the cdx device.
+ *
+ *  @return
+ *  0 on success, -1 on error.
+ */
+__rte_internal
+int rte_cdx_vfio_bm_enable(struct rte_cdx_device *dev);
+
+/**
+ * Disable Bus Mastering for CDX bus devices.
+ *
+ * @param dev
+ *   Pointer to the cdx device.
+ *
+ *  @return
+ *  0 on success, -1 on error.
+ */
+__rte_internal
+int rte_cdx_vfio_bm_disable(struct rte_cdx_device *dev);
+
 /**
  * Unregister a CDX driver.
  *
diff --git a/drivers/bus/cdx/cdx_vfio.c b/drivers/bus/cdx/cdx_vfio.c
index 8a3ac0b99537..a0abf71483aa 100644
--- a/drivers/bus/cdx/cdx_vfio.c
+++ b/drivers/bus/cdx/cdx_vfio.c
@@ -256,6 +256,16 @@ cdx_vfio_setup_device(struct rte_cdx_device *dev, int vfio_dev_fd,
 		return -1;
 	}
 
+	/*
+	 * Enable Bus mastering for the device. errno is set as ENOTTY if
+	 * device does not support configuring bus master.
+	 */
+	if (rte_cdx_vfio_bm_enable(dev) && errno != -ENOTTY) {
+		CDX_BUS_ERR("Bus master enable failure! Error: %d (%s)", errno,
+			strerror(errno));
+		return -1;
+	}
+
 	return 0;
 }
 
@@ -596,3 +606,90 @@ rte_cdx_vfio_intr_disable(const struct rte_intr_handle *intr_handle)
 
 	return ret;
 }
+
+/* enable Bus Mastering */
+int
+rte_cdx_vfio_bm_enable(struct rte_cdx_device *dev)
+{
+	struct vfio_device_info device_info = { .argsz = sizeof(device_info) };
+	struct vfio_device_feature_bus_master *vfio_bm_feature;
+	struct vfio_device_feature *feature;
+	int vfio_dev_fd, ret = 0;
+	size_t argsz = 0;
+
+	vfio_dev_fd = rte_intr_dev_fd_get(dev->intr_handle);
+	if (vfio_dev_fd < 0)
+		return -1;
+
+	argsz = sizeof(struct vfio_device_feature) + sizeof(struct vfio_device_feature_bus_master);
+
+	feature = (struct vfio_device_feature *)malloc(argsz);
+	if (!feature)
+		return -ENOMEM;
+
+	vfio_bm_feature = (struct vfio_device_feature_bus_master *) feature->data;
+
+	feature->argsz = argsz;
+
+	feature->flags = RTE_VFIO_DEVICE_FEATURE_BUS_MASTER | VFIO_DEVICE_FEATURE_PROBE;
+	feature->flags |= VFIO_DEVICE_FEATURE_SET;
+	ret = ioctl(vfio_dev_fd, RTE_VFIO_DEVICE_FEATURE, feature);
+	if (ret) {
+		CDX_BUS_ERR("Bus Master configuring not supported for device: %s, error: %d (%s)\n",
+			dev->name, errno, strerror(errno));
+		free(feature);
+		return ret;
+	}
+
+	feature->flags = RTE_VFIO_DEVICE_FEATURE_BUS_MASTER | VFIO_DEVICE_FEATURE_SET;
+	vfio_bm_feature->op = VFIO_DEVICE_FEATURE_SET_MASTER;
+	ret = ioctl(vfio_dev_fd, RTE_VFIO_DEVICE_FEATURE, feature);
+	if (ret < 0)
+		CDX_BUS_ERR("BM Enable Error for device: %s, Error: %d (%s)\n",
+			dev->name, errno, strerror(errno));
+
+	free(feature);
+	return ret;
+}
+
+/* Disable Bus Mastering */
+int
+rte_cdx_vfio_bm_disable(struct rte_cdx_device *dev)
+{
+	struct vfio_device_feature_bus_master *vfio_bm_feature;
+	struct vfio_device_feature *feature;
+	int vfio_dev_fd, ret = 0;
+	size_t argsz = 0;
+
+	vfio_dev_fd = rte_intr_dev_fd_get(dev->intr_handle);
+	if (vfio_dev_fd < 0)
+		return -1;
+
+	feature = (struct vfio_device_feature *)malloc(argsz);
+	if (!feature)
+		return -ENOMEM;
+
+	vfio_bm_feature = (struct vfio_device_feature_bus_master *) feature->data;
+
+	feature->argsz = argsz;
+
+	feature->flags = RTE_VFIO_DEVICE_FEATURE_BUS_MASTER | VFIO_DEVICE_FEATURE_PROBE;
+	feature->flags |= VFIO_DEVICE_FEATURE_SET;
+	ret = ioctl(vfio_dev_fd, RTE_VFIO_DEVICE_FEATURE, feature);
+	if (ret) {
+		CDX_BUS_ERR("Bus Master configuring not supported for device: %s, Error: %d (%s)\n",
+			dev->name, errno, strerror(errno));
+		free(feature);
+		return ret;
+	}
+
+	feature->flags = RTE_VFIO_DEVICE_FEATURE_BUS_MASTER | VFIO_DEVICE_FEATURE_SET;
+	vfio_bm_feature->op = VFIO_DEVICE_FEATURE_CLEAR_MASTER;
+	ret = ioctl(vfio_dev_fd, RTE_VFIO_DEVICE_FEATURE, feature);
+	if (ret < 0)
+		CDX_BUS_ERR("BM Disable Error for device: %s, Error: %d (%s)\n",
+			dev->name, errno, strerror(errno));
+
+	free(feature);
+	return ret;
+}
diff --git a/drivers/bus/cdx/version.map b/drivers/bus/cdx/version.map
index 0a15d39ae82a..5438f76029e0 100644
--- a/drivers/bus/cdx/version.map
+++ b/drivers/bus/cdx/version.map
@@ -7,6 +7,8 @@ INTERNAL {
 	rte_cdx_unregister;
 	rte_cdx_vfio_intr_disable;
 	rte_cdx_vfio_intr_enable;
+	rte_cdx_vfio_bm_enable;
+	rte_cdx_vfio_bm_disable;
 
 	local: *;
 };
diff --git a/lib/eal/include/rte_vfio.h b/lib/eal/include/rte_vfio.h
index 3487c4f2a251..1c791b3636e9 100644
--- a/lib/eal/include/rte_vfio.h
+++ b/lib/eal/include/rte_vfio.h
@@ -73,6 +73,33 @@ struct vfio_info_cap_header {
 #define RTE_VFIO_CAP_MSIX_MAPPABLE 3
 #endif
 
+/* VFIO_DEVICE_FEATURE is defined for kernel version 5.7 and newer. */
+#ifdef VFIO_DEVICE_FEATURE
+#define RTE_VFIO_DEVICE_FEATURE VFIO_DEVICE_FEATURE
+#else
+#define RTE_VFIO_DEVICE_FEATURE             _IO(VFIO_TYPE, VFIO_BASE + 17)
+struct vfio_device_feature {
+	__u32   argsz;
+	__u32   flags;
+#define VFIO_DEVICE_FEATURE_MASK        (0xffff) /* 16-bit feature index */
+#define VFIO_DEVICE_FEATURE_GET         (1 << 16) /* Get feature into data[] */
+#define VFIO_DEVICE_FEATURE_SET         (1 << 17) /* Set feature from data[] */
+#define VFIO_DEVICE_FEATURE_PROBE       (1 << 18) /* Probe feature support */
+	__u8    data[];
+};
+#endif
+
+#ifdef VFIO_DEVICE_FEATURE_BUS_MASTER
+#define RTE_VFIO_DEVICE_FEATURE_BUS_MASTER VFIO_DEVICE_FEATURE_BUS_MASTER
+#else
+#define RTE_VFIO_DEVICE_FEATURE_BUS_MASTER  10
+struct vfio_device_feature_bus_master {
+	__u32 op;
+#define  VFIO_DEVICE_FEATURE_CLEAR_MASTER       0       /* Clear Bus Master */
+#define  VFIO_DEVICE_FEATURE_SET_MASTER         1       /* Set Bus Master */
+};
+#endif
+
 #else /* not VFIO_PRESENT */
 
 /* we don't need an actual definition, only pointer is used */
-- 
2.34.1

Re: [PATCH] Add bus master enable/disable APIs for CDX devices

[Ferruh Yigit]

On 10/27/2023 5:22 PM, Ferruh Yigit wrote:
> From: Shubham Rohila <shubham.rohila@amd.com>
> 
> Define rte_cdx_vfio_bm_enable and rte_cdx_vfio_bm_disable to
> enable or disable bus master functionality for cdx devices.
> 
> Signed-off-by: Shubham Rohila <shubham.rohila@amd.com>
> 

Note: this is re-sent on behalf of Shubham to record the patch, since
the original one is not available in the mail list somehow.

[PATCH v2 1/2] bus/cdx: add support for devices without MSI

[Shubham Rohila]

From: Nikhil Agarwal <nikhil.agarwal@amd.com>

Update the cleanup routine for cdx device to support
device without MSI. Also, set vfio_dev_fd for such devices
This fd can be used for BME reload operations.

Signed-off-by: Nikhil Agarwal <nikhil.agarwal@amd.com>
Signed-off-by: Shubham Rohila <shubham.rohila@amd.com>
---
 v2
 - New patch in the series
 drivers/bus/cdx/cdx.c      |  2 +-
 drivers/bus/cdx/cdx_vfio.c | 19 +++++++++----------
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c
index 541aae76c3..62b108e082 100644
--- a/drivers/bus/cdx/cdx.c
+++ b/drivers/bus/cdx/cdx.c
@@ -405,9 +405,9 @@ cdx_probe_one_driver(struct rte_cdx_driver *dr,
 	return ret;
 
 error_probe:
+	cdx_vfio_unmap_resource(dev);
 	rte_intr_instance_free(dev->intr_handle);
 	dev->intr_handle = NULL;
-	cdx_vfio_unmap_resource(dev);
 error_map_device:
 	return ret;
 }
diff --git a/drivers/bus/cdx/cdx_vfio.c b/drivers/bus/cdx/cdx_vfio.c
index 8a3ac0b995..8cac79782e 100644
--- a/drivers/bus/cdx/cdx_vfio.c
+++ b/drivers/bus/cdx/cdx_vfio.c
@@ -101,13 +101,12 @@ cdx_vfio_unmap_resource_primary(struct rte_cdx_device *dev)
 	struct mapped_cdx_res_list *vfio_res_list;
 	int ret, vfio_dev_fd;
 
-	if (rte_intr_fd_get(dev->intr_handle) < 0)
-		return -1;
-
-	if (close(rte_intr_fd_get(dev->intr_handle)) < 0) {
-		CDX_BUS_ERR("Error when closing eventfd file descriptor for %s",
-			dev->device.name);
-		return -1;
+	if (rte_intr_fd_get(dev->intr_handle) >= 0) {
+		if (close(rte_intr_fd_get(dev->intr_handle)) < 0) {
+			CDX_BUS_ERR("Error when closing eventfd file descriptor for %s",
+				dev->device.name);
+			return -1;
+		}
 	}
 
 	vfio_dev_fd = rte_intr_dev_fd_get(dev->intr_handle);
@@ -185,6 +184,9 @@ cdx_vfio_setup_interrupts(struct rte_cdx_device *dev, int vfio_dev_fd,
 {
 	int i, ret;
 
+	if (rte_intr_dev_fd_set(dev->intr_handle, vfio_dev_fd))
+		return -1;
+
 	if (num_irqs == 0)
 		return 0;
 
@@ -227,9 +229,6 @@ cdx_vfio_setup_interrupts(struct rte_cdx_device *dev, int vfio_dev_fd,
 		if (rte_intr_type_set(dev->intr_handle, RTE_INTR_HANDLE_VFIO_MSIX))
 			return -1;
 
-		if (rte_intr_dev_fd_set(dev->intr_handle, vfio_dev_fd))
-			return -1;
-
 		return 0;
 	}
 
-- 
2.25.1

[PATCH v2 2/2] bus/cdx: add bus master enable/disable APIs

[Shubham Rohila]

Define rte_cdx_vfio_bm_enable and rte_cdx_vfio_bm_disable to
enable or disable bus master functionality for cdx devices.

Signed-off-by: Shubham Rohila <shubham.rohila@amd.com>
---
 v2
 - Fix indentations and alphabetical orderings.
 - Wrap checks in parenthesis for readability
 - Assign argsz before using in rte_cdx_vfio_bm_disable
 - Remove unecessary variable initialisation

 drivers/bus/cdx/bus_cdx_driver.h |  25 ++++++++
 drivers/bus/cdx/cdx_vfio.c       | 103 +++++++++++++++++++++++++++++++
 drivers/bus/cdx/version.map      |   2 +
 lib/eal/include/rte_vfio.h       |  27 ++++++++
 4 files changed, 157 insertions(+)

diff --git a/drivers/bus/cdx/bus_cdx_driver.h b/drivers/bus/cdx/bus_cdx_driver.h
index 1c9a64c87a..211f8e406b 100644
--- a/drivers/bus/cdx/bus_cdx_driver.h
+++ b/drivers/bus/cdx/bus_cdx_driver.h
@@ -16,6 +16,7 @@ extern "C" {
 
 #include <stdlib.h>
 #include <inttypes.h>
+#include <linux/types.h>
 
 #include <bus_driver.h>
 #include <dev_driver.h>
@@ -178,6 +179,30 @@ int rte_cdx_vfio_intr_enable(const struct rte_intr_handle *intr_handle);
 __rte_internal
 int rte_cdx_vfio_intr_disable(const struct rte_intr_handle *intr_handle);
 
+/**
+ * Enable Bus Mastering for CDX bus devices.
+ *
+ * @param dev
+ *   Pointer to the cdx device.
+ *
+ * @return
+ *   0 on success, -1 on error.
+ */
+__rte_internal
+int rte_cdx_vfio_bm_enable(struct rte_cdx_device *dev);
+
+/**
+ * Disable Bus Mastering for CDX bus devices.
+ *
+ * @param dev
+ *   Pointer to the cdx device.
+ *
+ * @return
+ *   0 on success, -1 on error.
+ */
+__rte_internal
+int rte_cdx_vfio_bm_disable(struct rte_cdx_device *dev);
+
 /**
  * Unregister a CDX driver.
  *
diff --git a/drivers/bus/cdx/cdx_vfio.c b/drivers/bus/cdx/cdx_vfio.c
index 8cac79782e..1481022f13 100644
--- a/drivers/bus/cdx/cdx_vfio.c
+++ b/drivers/bus/cdx/cdx_vfio.c
@@ -102,6 +102,10 @@ cdx_vfio_unmap_resource_primary(struct rte_cdx_device *dev)
 	int ret, vfio_dev_fd;
 
 	if (rte_intr_fd_get(dev->intr_handle) >= 0) {
+		if (rte_cdx_vfio_bm_disable(dev) < 0)
+			CDX_BUS_ERR("Error when disabling bus master for %s",
+				    dev->device.name);
+
 		if (close(rte_intr_fd_get(dev->intr_handle)) < 0) {
 			CDX_BUS_ERR("Error when closing eventfd file descriptor for %s",
 				dev->device.name);
@@ -255,6 +259,16 @@ cdx_vfio_setup_device(struct rte_cdx_device *dev, int vfio_dev_fd,
 		return -1;
 	}
 
+	/*
+	 * Enable Bus mastering for the device. errno is set as ENOTTY if
+	 * device does not support configuring bus master.
+	 */
+	if (rte_cdx_vfio_bm_enable(dev) && (errno != -ENOTTY)) {
+		CDX_BUS_ERR("Bus master enable failure! Error: %d (%s)", errno,
+			strerror(errno));
+		return -1;
+	}
+
 	return 0;
 }
 
@@ -595,3 +609,92 @@ rte_cdx_vfio_intr_disable(const struct rte_intr_handle *intr_handle)
 
 	return ret;
 }
+
+/* enable Bus Mastering */
+int
+rte_cdx_vfio_bm_enable(struct rte_cdx_device *dev)
+{
+	struct vfio_device_info device_info = { .argsz = sizeof(device_info) };
+	struct vfio_device_feature_bus_master *vfio_bm_feature;
+	struct vfio_device_feature *feature;
+	int vfio_dev_fd, ret;
+	size_t argsz;
+
+	vfio_dev_fd = rte_intr_dev_fd_get(dev->intr_handle);
+	if (vfio_dev_fd < 0)
+		return -1;
+
+	argsz = sizeof(struct vfio_device_feature) + sizeof(struct vfio_device_feature_bus_master);
+
+	feature = (struct vfio_device_feature *)malloc(argsz);
+	if (!feature)
+		return -ENOMEM;
+
+	vfio_bm_feature = (struct vfio_device_feature_bus_master *) feature->data;
+
+	feature->argsz = argsz;
+
+	feature->flags = RTE_VFIO_DEVICE_FEATURE_BUS_MASTER | VFIO_DEVICE_FEATURE_PROBE;
+	feature->flags |= VFIO_DEVICE_FEATURE_SET;
+	ret = ioctl(vfio_dev_fd, RTE_VFIO_DEVICE_FEATURE, feature);
+	if (ret) {
+		CDX_BUS_ERR("Bus Master configuring not supported for device: %s, error: %d (%s)\n",
+			dev->name, errno, strerror(errno));
+		free(feature);
+		return ret;
+	}
+
+	feature->flags = RTE_VFIO_DEVICE_FEATURE_BUS_MASTER | VFIO_DEVICE_FEATURE_SET;
+	vfio_bm_feature->op = VFIO_DEVICE_FEATURE_SET_MASTER;
+	ret = ioctl(vfio_dev_fd, RTE_VFIO_DEVICE_FEATURE, feature);
+	if (ret < 0)
+		CDX_BUS_ERR("BM Enable Error for device: %s, Error: %d (%s)\n",
+			dev->name, errno, strerror(errno));
+
+	free(feature);
+	return ret;
+}
+
+/* Disable Bus Mastering */
+int
+rte_cdx_vfio_bm_disable(struct rte_cdx_device *dev)
+{
+	struct vfio_device_feature_bus_master *vfio_bm_feature;
+	struct vfio_device_feature *feature;
+	int vfio_dev_fd, ret;
+	size_t argsz;
+
+	vfio_dev_fd = rte_intr_dev_fd_get(dev->intr_handle);
+	if (vfio_dev_fd < 0)
+		return -1;
+
+	argsz = sizeof(struct vfio_device_feature) + sizeof(struct vfio_device_feature_bus_master);
+
+	feature = (struct vfio_device_feature *)malloc(argsz);
+	if (!feature)
+		return -ENOMEM;
+
+	vfio_bm_feature = (struct vfio_device_feature_bus_master *) feature->data;
+
+	feature->argsz = argsz;
+
+	feature->flags = RTE_VFIO_DEVICE_FEATURE_BUS_MASTER | VFIO_DEVICE_FEATURE_PROBE;
+	feature->flags |= VFIO_DEVICE_FEATURE_SET;
+	ret = ioctl(vfio_dev_fd, RTE_VFIO_DEVICE_FEATURE, feature);
+	if (ret) {
+		CDX_BUS_ERR("Bus Master configuring not supported for device: %s, Error: %d (%s)\n",
+			dev->name, errno, strerror(errno));
+		free(feature);
+		return ret;
+	}
+
+	feature->flags = RTE_VFIO_DEVICE_FEATURE_BUS_MASTER | VFIO_DEVICE_FEATURE_SET;
+	vfio_bm_feature->op = VFIO_DEVICE_FEATURE_CLEAR_MASTER;
+	ret = ioctl(vfio_dev_fd, RTE_VFIO_DEVICE_FEATURE, feature);
+	if (ret < 0)
+		CDX_BUS_ERR("BM Disable Error for device: %s, Error: %d (%s)\n",
+			dev->name, errno, strerror(errno));
+
+	free(feature);
+	return ret;
+}
diff --git a/drivers/bus/cdx/version.map b/drivers/bus/cdx/version.map
index 0a15d39ae8..7a22cf5b4b 100644
--- a/drivers/bus/cdx/version.map
+++ b/drivers/bus/cdx/version.map
@@ -5,6 +5,8 @@ INTERNAL {
 	rte_cdx_register;
 	rte_cdx_unmap_device;
 	rte_cdx_unregister;
+	rte_cdx_vfio_bm_disable;
+	rte_cdx_vfio_bm_enable;
 	rte_cdx_vfio_intr_disable;
 	rte_cdx_vfio_intr_enable;
 
diff --git a/lib/eal/include/rte_vfio.h b/lib/eal/include/rte_vfio.h
index 3487c4f2a2..22832afd0f 100644
--- a/lib/eal/include/rte_vfio.h
+++ b/lib/eal/include/rte_vfio.h
@@ -73,6 +73,33 @@ struct vfio_info_cap_header {
 #define RTE_VFIO_CAP_MSIX_MAPPABLE 3
 #endif
 
+/* VFIO_DEVICE_FEATURE is defined for kernel version 5.7 and newer. */
+#ifdef	VFIO_DEVICE_FEATURE
+#define	RTE_VFIO_DEVICE_FEATURE	VFIO_DEVICE_FEATURE
+#else
+#define	RTE_VFIO_DEVICE_FEATURE	_IO(VFIO_TYPE, VFIO_BASE + 17)
+struct vfio_device_feature {
+	__u32	argsz;
+	__u32	flags;
+#define	VFIO_DEVICE_FEATURE_MASK	(0xffff) /* 16-bit feature index */
+#define	VFIO_DEVICE_FEATURE_GET		(1 << 16) /* Get feature into data[] */
+#define	VFIO_DEVICE_FEATURE_SET		(1 << 17) /* Set feature from data[] */
+#define	VFIO_DEVICE_FEATURE_PROBE	(1 << 18) /* Probe feature support */
+	__u8	data[];
+};
+#endif
+
+#ifdef	VFIO_DEVICE_FEATURE_BUS_MASTER
+#define	RTE_VFIO_DEVICE_FEATURE_BUS_MASTER	VFIO_DEVICE_FEATURE_BUS_MASTER
+#else
+#define	RTE_VFIO_DEVICE_FEATURE_BUS_MASTER	10
+struct vfio_device_feature_bus_master {
+	__u32 op;
+#define	VFIO_DEVICE_FEATURE_CLEAR_MASTER	0	/* Clear Bus Master */
+#define	VFIO_DEVICE_FEATURE_SET_MASTER		1	/* Set Bus Master */
+};
+#endif
+
 #else /* not VFIO_PRESENT */
 
 /* we don't need an actual definition, only pointer is used */
-- 
2.25.1

Re: [PATCH v2 2/2] bus/cdx: add bus master enable/disable APIs

[Gupta, Nipun]

On 11/3/2023 4:50 PM, Shubham Rohila wrote:
> Define rte_cdx_vfio_bm_enable and rte_cdx_vfio_bm_disable to
> enable or disable bus master functionality for cdx devices.
> 
> Signed-off-by: Shubham Rohila <shubham.rohila@amd.com>
> ---
>   v2
>   - Fix indentations and alphabetical orderings.
>   - Wrap checks in parenthesis for readability
>   - Assign argsz before using in rte_cdx_vfio_bm_disable
>   - Remove unecessary variable initialisation

Acked-by: Nipun Gupta <nipun.gupta@amd.com>

Re: [PATCH v2 2/2] bus/cdx: add bus master enable/disable APIs

[Thomas Monjalon]

07/11/2023 15:25, Gupta, Nipun:
> 
> On 11/3/2023 4:50 PM, Shubham Rohila wrote:
> > Define rte_cdx_vfio_bm_enable and rte_cdx_vfio_bm_disable to
> > enable or disable bus master functionality for cdx devices.
> > 
> > Signed-off-by: Shubham Rohila <shubham.rohila@amd.com>
> > ---
> >   v2
> >   - Fix indentations and alphabetical orderings.
> >   - Wrap checks in parenthesis for readability
> >   - Assign argsz before using in rte_cdx_vfio_bm_disable
> >   - Remove unecessary variable initialisation
> 
> Acked-by: Nipun Gupta <nipun.gupta@amd.com>

Applied, thanks.

Re: [PATCH v2 1/2] bus/cdx: add support for devices without MSI

[Gupta, Nipun]

On 11/3/2023 4:50 PM, Shubham Rohila wrote:
> From: Nikhil Agarwal <nikhil.agarwal@amd.com>
> 
> Update the cleanup routine for cdx device to support
> device without MSI. Also, set vfio_dev_fd for such devices
> This fd can be used for BME reload operations.
> 
> Signed-off-by: Nikhil Agarwal <nikhil.agarwal@amd.com>
> Signed-off-by: Shubham Rohila <shubham.rohila@amd.com>
> ---
>   v2
>   - New patch in the series
>   drivers/bus/cdx/cdx.c      |  2 +-
>   drivers/bus/cdx/cdx_vfio.c | 19 +++++++++----------
>   2 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c
> index 541aae76c3..62b108e082 100644
> --- a/drivers/bus/cdx/cdx.c
> +++ b/drivers/bus/cdx/cdx.c
> @@ -405,9 +405,9 @@ cdx_probe_one_driver(struct rte_cdx_driver *dr,
>   	return ret;
>   
>   error_probe:
> +	cdx_vfio_unmap_resource(dev);
>   	rte_intr_instance_free(dev->intr_handle);
>   	dev->intr_handle = NULL;
> -	cdx_vfio_unmap_resource(dev);
>   error_map_device:
>   	return ret;
>   }
> diff --git a/drivers/bus/cdx/cdx_vfio.c b/drivers/bus/cdx/cdx_vfio.c
> index 8a3ac0b995..8cac79782e 100644
> --- a/drivers/bus/cdx/cdx_vfio.c
> +++ b/drivers/bus/cdx/cdx_vfio.c
> @@ -101,13 +101,12 @@ cdx_vfio_unmap_resource_primary(struct rte_cdx_device *dev)
>   	struct mapped_cdx_res_list *vfio_res_list;
>   	int ret, vfio_dev_fd;
>   
> -	if (rte_intr_fd_get(dev->intr_handle) < 0)
> -		return -1;

Why is this check removed? If VFIO fd is not there we may not proceed 
with other VFIO cleanup?

RE: [PATCH v2 1/2] bus/cdx: add support for devices without MSI

[Agarwal, Nikhil]

[AMD Official Use Only - General]

Hi Nipun,

> -----Original Message-----
> > diff --git a/drivers/bus/cdx/cdx_vfio.c b/drivers/bus/cdx/cdx_vfio.c
> > index 8a3ac0b995..8cac79782e 100644
> > --- a/drivers/bus/cdx/cdx_vfio.c
> > +++ b/drivers/bus/cdx/cdx_vfio.c
> > @@ -101,13 +101,12 @@ cdx_vfio_unmap_resource_primary(struct
> rte_cdx_device *dev)
> >     struct mapped_cdx_res_list *vfio_res_list;
> >     int ret, vfio_dev_fd;
> >
> > -   if (rte_intr_fd_get(dev->intr_handle) < 0)
> > -           return -1;
>
> Why is this check removed? If VFIO fd is not there we may not proceed with
> other VFIO cleanup?

This check was removed from here but added later for interrupt specific cleanup
section. It was causing other cleanup in the function to be skipped in case device
does not support MSI.

Re: [PATCH v2 1/2] bus/cdx: add support for devices without MSI

[Gupta, Nipun]

On 11/3/2023 4:50 PM, Shubham Rohila wrote:
> From: Nikhil Agarwal <nikhil.agarwal@amd.com>
> 
> Update the cleanup routine for cdx device to support
> device without MSI. Also, set vfio_dev_fd for such devices
> This fd can be used for BME reload operations.
> 
> Signed-off-by: Nikhil Agarwal <nikhil.agarwal@amd.com>
> Signed-off-by: Shubham Rohila <shubham.rohila@amd.com>

Acked-by: Nipun Gupta <nipun.gupta@amd.com>

Re: [PATCH] Add bus master enable/disable APIs for CDX devices

[Stephen Hemminger]

On Fri, 27 Oct 2023 17:22:11 +0100
Ferruh Yigit <ferruh.yigit@amd.com> wrote:

> From: Shubham Rohila <shubham.rohila@amd.com>
> 
> Define rte_cdx_vfio_bm_enable and rte_cdx_vfio_bm_disable to
> enable or disable bus master functionality for cdx devices.
> 
> Signed-off-by: Shubham Rohila <shubham.rohila@amd.com>

This version of the patch had lots test failures, could you clean
up and resubmit. 

Re: [PATCH] Add bus master enable/disable APIs for CDX devices

[Ferruh Yigit]

On 10/3/2024 11:16 PM, Stephen Hemminger wrote:
> On Fri, 27 Oct 2023 17:22:11 +0100
> Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> 
>> From: Shubham Rohila <shubham.rohila@amd.com>
>>
>> Define rte_cdx_vfio_bm_enable and rte_cdx_vfio_bm_disable to
>> enable or disable bus master functionality for cdx devices.
>>
>> Signed-off-by: Shubham Rohila <shubham.rohila@amd.com>
> 
> This version of the patch had lots test failures, could you clean
> up and resubmit. 
>

v2 of the patch already merged.
Probably missed to update the status of this patch in the patchwork, I
will update it as 'superseded'