* Re: [PATCH] Add bus master enable/disable APIs for CDX devices
[not found] <20231026032009.3275263-1-shubham.rohila@amd.com>
@ 2023-10-27 13:34 ` Gupta, Nipun
0 siblings, 0 replies; 3+ messages in thread
From: Gupta, Nipun @ 2023-10-27 13:34 UTC (permalink / raw)
To: Shubham Rohila, nikhil.agarwal, david.marchand, dev
Cc: thomas, ferruh.yigit, anatoly.burakov
On 10/26/2023 8:50 AM, 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>
> ---
> 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 1c9a64c87a..2f94a6c3af 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.
alignment should be similar to @param dev
> + */
> +__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.
here too
> + */
> +__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 8a3ac0b995..a0abf71483 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) {
errno != -ENOTTY can be in brackets for better readability.
As we are probing the functionality in the
rte_cdx_vfio_bm_enable/disable API, so why do we need a check for errno?
> + CDX_BUS_ERR("Bus master enable failure! Error: %d (%s)", errno,
> + strerror(errno));
> + return -1;
> + }
Disable in cdx_vfio_unmap_resource_primary?
> +
> 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;
no need to initialize argsz and ret here.
> +
> + 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;
no need to initialize argsz and ret here.
> +
> + 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);
need to assign argsz before?
> + 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..5438f76029 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;
Alphabetical order.
>
> local: *;
> };
> diff --git a/lib/eal/include/rte_vfio.h b/lib/eal/include/rte_vfio.h
> index 3487c4f2a2..1c791b3636 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)
use tabs.
> +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 */
and tabs in all above here.
Regards,
Nipun
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Add bus master enable/disable APIs for CDX devices
2023-10-27 16:22 Ferruh Yigit
@ 2023-10-27 16:24 ` Ferruh Yigit
0 siblings, 0 replies; 3+ messages in thread
From: Ferruh Yigit @ 2023-10-27 16:24 UTC (permalink / raw)
To: dev, nikhil.agarwal, nipun.gupta, david.marchand
Cc: thomas, anatoly.burakov, Shubham Rohila
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] Add bus master enable/disable APIs for CDX devices
@ 2023-10-27 16:22 Ferruh Yigit
2023-10-27 16:24 ` Ferruh Yigit
0 siblings, 1 reply; 3+ messages in thread
From: Ferruh Yigit @ 2023-10-27 16:22 UTC (permalink / raw)
To: dev, nikhil.agarwal, nipun.gupta, david.marchand
Cc: thomas, ferruh.yigit, anatoly.burakov, Shubham Rohila
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-10-27 16:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20231026032009.3275263-1-shubham.rohila@amd.com>
2023-10-27 13:34 ` [PATCH] Add bus master enable/disable APIs for CDX devices Gupta, Nipun
2023-10-27 16:22 Ferruh Yigit
2023-10-27 16:24 ` Ferruh Yigit
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).