[v2,2/8] crypto/bcmfs: add vfio support
Checks
Commit Message
Add vfio support for device.
Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
Signed-off-by: Raveendra Padasalagi <raveendra.padasalagi@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
drivers/crypto/bcmfs/bcmfs_device.c | 5 ++
drivers/crypto/bcmfs/bcmfs_device.h | 6 ++
drivers/crypto/bcmfs/bcmfs_vfio.c | 107 ++++++++++++++++++++++++++++
drivers/crypto/bcmfs/bcmfs_vfio.h | 17 +++++
drivers/crypto/bcmfs/meson.build | 3 +-
5 files changed, 137 insertions(+), 1 deletion(-)
create mode 100644 drivers/crypto/bcmfs/bcmfs_vfio.c
create mode 100644 drivers/crypto/bcmfs/bcmfs_vfio.h
Comments
Hi Vikas,
> Subject: [PATCH v2 2/8] crypto/bcmfs: add vfio support
>
> Add vfio support for device.
>
> Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
> Signed-off-by: Raveendra Padasalagi <raveendra.padasalagi@broadcom.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
> drivers/crypto/bcmfs/bcmfs_device.c | 5 ++
> drivers/crypto/bcmfs/bcmfs_device.h | 6 ++
> drivers/crypto/bcmfs/bcmfs_vfio.c | 107 ++++++++++++++++++++++++++++
> drivers/crypto/bcmfs/bcmfs_vfio.h | 17 +++++
> drivers/crypto/bcmfs/meson.build | 3 +-
> 5 files changed, 137 insertions(+), 1 deletion(-)
> create mode 100644 drivers/crypto/bcmfs/bcmfs_vfio.c
> create mode 100644 drivers/crypto/bcmfs/bcmfs_vfio.h
>
> diff --git a/drivers/crypto/bcmfs/bcmfs_device.c
> b/drivers/crypto/bcmfs/bcmfs_device.c
> index 47c776de6..3b5cc9e98 100644
> --- a/drivers/crypto/bcmfs/bcmfs_device.c
> +++ b/drivers/crypto/bcmfs/bcmfs_device.c
> @@ -11,6 +11,7 @@
>
> #include "bcmfs_device.h"
> #include "bcmfs_logs.h"
> +#include "bcmfs_vfio.h"
>
> struct bcmfs_device_attr {
> const char name[BCMFS_MAX_PATH_LEN];
> @@ -71,6 +72,10 @@ fsdev_allocate_one_dev(struct rte_vdev_device *vdev,
>
> fsdev->vdev = vdev;
>
> + /* attach to VFIO */
> + if (bcmfs_attach_vfio(fsdev))
> + goto cleanup;
> +
> TAILQ_INSERT_TAIL(&fsdev_list, fsdev, next);
>
> return fsdev;
> diff --git a/drivers/crypto/bcmfs/bcmfs_device.h
> b/drivers/crypto/bcmfs/bcmfs_device.h
> index cc64a8df2..c41cc0031 100644
> --- a/drivers/crypto/bcmfs/bcmfs_device.h
> +++ b/drivers/crypto/bcmfs/bcmfs_device.h
> @@ -35,6 +35,12 @@ struct bcmfs_device {
> char name[BCMFS_DEV_NAME_LEN];
> /* Parent vdev */
> struct rte_vdev_device *vdev;
> + /* vfio handle */
> + int vfio_dev_fd;
> + /* mapped address */
> + uint8_t *mmap_addr;
> + /* mapped size */
> + uint32_t mmap_size;
> };
>
> #endif /* _BCMFS_DEV_H_ */
> diff --git a/drivers/crypto/bcmfs/bcmfs_vfio.c
> b/drivers/crypto/bcmfs/bcmfs_vfio.c
> new file mode 100644
> index 000000000..dc2def580
> --- /dev/null
> +++ b/drivers/crypto/bcmfs/bcmfs_vfio.c
> @@ -0,0 +1,107 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2020 Broadcom.
> + * All rights reserved.
> + */
> +
> +#include <errno.h>
> +#include <sys/mman.h>
> +#include <sys/ioctl.h>
> +
> +#include <rte_vfio.h>
> +
> +#include "bcmfs_device.h"
> +#include "bcmfs_logs.h"
> +#include "bcmfs_vfio.h"
> +
> +#ifdef VFIO_PRESENT
I cannot see VFIO_PRESENT flag defined in this patch.
Hence the below code is a dead code and the patch
Title is not justified as it says adding support for VFIO.
> +static int
> +vfio_map_dev_obj(const char *path, const char *dev_obj,
> + uint32_t *size, void **addr, int *dev_fd)
Regards,
Akhil
Hi Akhil,
On Tue, Sep 29, 2020 at 12:30 AM Akhil Goyal <akhil.goyal@nxp.com> wrote:
>
> Hi Vikas,
>
> > Subject: [PATCH v2 2/8] crypto/bcmfs: add vfio support
> >
> > Add vfio support for device.
> >
> > Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
> > Signed-off-by: Raveendra Padasalagi <raveendra.padasalagi@broadcom.com>
> > Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > ---
> > drivers/crypto/bcmfs/bcmfs_device.c | 5 ++
> > drivers/crypto/bcmfs/bcmfs_device.h | 6 ++
> > drivers/crypto/bcmfs/bcmfs_vfio.c | 107 ++++++++++++++++++++++++++++
> > drivers/crypto/bcmfs/bcmfs_vfio.h | 17 +++++
> > drivers/crypto/bcmfs/meson.build | 3 +-
> > 5 files changed, 137 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/crypto/bcmfs/bcmfs_vfio.c
> > create mode 100644 drivers/crypto/bcmfs/bcmfs_vfio.h
> >
> > diff --git a/drivers/crypto/bcmfs/bcmfs_device.c
> > b/drivers/crypto/bcmfs/bcmfs_device.c
> > index 47c776de6..3b5cc9e98 100644
> > --- a/drivers/crypto/bcmfs/bcmfs_device.c
> > +++ b/drivers/crypto/bcmfs/bcmfs_device.c
> > @@ -11,6 +11,7 @@
> >
> > #include "bcmfs_device.h"
> > #include "bcmfs_logs.h"
> > +#include "bcmfs_vfio.h"
> >
> > struct bcmfs_device_attr {
> > const char name[BCMFS_MAX_PATH_LEN];
> > @@ -71,6 +72,10 @@ fsdev_allocate_one_dev(struct rte_vdev_device *vdev,
> >
> > fsdev->vdev = vdev;
> >
> > + /* attach to VFIO */
> > + if (bcmfs_attach_vfio(fsdev))
> > + goto cleanup;
> > +
> > TAILQ_INSERT_TAIL(&fsdev_list, fsdev, next);
> >
> > return fsdev;
> > diff --git a/drivers/crypto/bcmfs/bcmfs_device.h
> > b/drivers/crypto/bcmfs/bcmfs_device.h
> > index cc64a8df2..c41cc0031 100644
> > --- a/drivers/crypto/bcmfs/bcmfs_device.h
> > +++ b/drivers/crypto/bcmfs/bcmfs_device.h
> > @@ -35,6 +35,12 @@ struct bcmfs_device {
> > char name[BCMFS_DEV_NAME_LEN];
> > /* Parent vdev */
> > struct rte_vdev_device *vdev;
> > + /* vfio handle */
> > + int vfio_dev_fd;
> > + /* mapped address */
> > + uint8_t *mmap_addr;
> > + /* mapped size */
> > + uint32_t mmap_size;
> > };
> >
> > #endif /* _BCMFS_DEV_H_ */
> > diff --git a/drivers/crypto/bcmfs/bcmfs_vfio.c
> > b/drivers/crypto/bcmfs/bcmfs_vfio.c
> > new file mode 100644
> > index 000000000..dc2def580
> > --- /dev/null
> > +++ b/drivers/crypto/bcmfs/bcmfs_vfio.c
> > @@ -0,0 +1,107 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(C) 2020 Broadcom.
> > + * All rights reserved.
> > + */
> > +
> > +#include <errno.h>
> > +#include <sys/mman.h>
> > +#include <sys/ioctl.h>
> > +
> > +#include <rte_vfio.h>
> > +
> > +#include "bcmfs_device.h"
> > +#include "bcmfs_logs.h"
> > +#include "bcmfs_vfio.h"
> > +
> > +#ifdef VFIO_PRESENT
>
> I cannot see VFIO_PRESENT flag defined in this patch.
> Hence the below code is a dead code and the patch
> Title is not justified as it says adding support for VFIO.
I believe VFIO_PRESENT flag is dependent on the platform who supports
VFIO and determined in rte_vfio.h.
The driver will not work without VFIO support and returns silently
(functions in #else part).
Do you mean I need to change the title?
>
> > +static int
> > +vfio_map_dev_obj(const char *path, const char *dev_obj,
> > + uint32_t *size, void **addr, int *dev_fd)
>
> Regards,
> Akhil
Thanks,
Vikas
Hi Vikas,
>
> Hi Akhil,
>
> > > +
> > > +#ifdef VFIO_PRESENT
> >
> > I cannot see VFIO_PRESENT flag defined in this patch.
> > Hence the below code is a dead code and the patch
> > Title is not justified as it says adding support for VFIO.
> I believe VFIO_PRESENT flag is dependent on the platform who supports
> VFIO and determined in rte_vfio.h.
> The driver will not work without VFIO support and returns silently
> (functions in #else part).
> Do you mean I need to change the title?
Title is OK. You can explain in the patch description and bcmfs.rst about how
it is getting enabled, which config flags need to be enabled and probably in
the bcmfs.rst as well.
Regards,
Akhil
@@ -11,6 +11,7 @@
#include "bcmfs_device.h"
#include "bcmfs_logs.h"
+#include "bcmfs_vfio.h"
struct bcmfs_device_attr {
const char name[BCMFS_MAX_PATH_LEN];
@@ -71,6 +72,10 @@ fsdev_allocate_one_dev(struct rte_vdev_device *vdev,
fsdev->vdev = vdev;
+ /* attach to VFIO */
+ if (bcmfs_attach_vfio(fsdev))
+ goto cleanup;
+
TAILQ_INSERT_TAIL(&fsdev_list, fsdev, next);
return fsdev;
@@ -35,6 +35,12 @@ struct bcmfs_device {
char name[BCMFS_DEV_NAME_LEN];
/* Parent vdev */
struct rte_vdev_device *vdev;
+ /* vfio handle */
+ int vfio_dev_fd;
+ /* mapped address */
+ uint8_t *mmap_addr;
+ /* mapped size */
+ uint32_t mmap_size;
};
#endif /* _BCMFS_DEV_H_ */
new file mode 100644
@@ -0,0 +1,107 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2020 Broadcom.
+ * All rights reserved.
+ */
+
+#include <errno.h>
+#include <sys/mman.h>
+#include <sys/ioctl.h>
+
+#include <rte_vfio.h>
+
+#include "bcmfs_device.h"
+#include "bcmfs_logs.h"
+#include "bcmfs_vfio.h"
+
+#ifdef VFIO_PRESENT
+static int
+vfio_map_dev_obj(const char *path, const char *dev_obj,
+ uint32_t *size, void **addr, int *dev_fd)
+{
+ int32_t ret;
+ struct vfio_group_status status = { .argsz = sizeof(status) };
+
+ struct vfio_device_info d_info = { .argsz = sizeof(d_info) };
+ struct vfio_region_info reg_info = { .argsz = sizeof(reg_info) };
+
+ ret = rte_vfio_setup_device(path, dev_obj, dev_fd, &d_info);
+ if (ret) {
+ BCMFS_LOG(ERR, "VFIO Setting for device failed");
+ return ret;
+ }
+
+ /* getting device region info*/
+ ret = ioctl(*dev_fd, VFIO_DEVICE_GET_REGION_INFO, ®_info);
+ if (ret < 0) {
+ BCMFS_LOG(ERR, "Error in VFIO getting REGION_INFO");
+ goto map_failed;
+ }
+
+ *addr = mmap(NULL, reg_info.size,
+ PROT_WRITE | PROT_READ, MAP_SHARED,
+ *dev_fd, reg_info.offset);
+ if (*addr == MAP_FAILED) {
+ BCMFS_LOG(ERR, "Error mapping region (errno = %d)", errno);
+ ret = errno;
+ goto map_failed;
+ }
+ *size = reg_info.size;
+
+ return 0;
+
+map_failed:
+ rte_vfio_release_device(path, dev_obj, *dev_fd);
+
+ return ret;
+}
+
+int
+bcmfs_attach_vfio(struct bcmfs_device *dev)
+{
+ int ret;
+ int vfio_dev_fd;
+ void *v_addr = NULL;
+ uint32_t size = 0;
+
+ ret = vfio_map_dev_obj(dev->dirname, dev->name,
+ &size, &v_addr, &vfio_dev_fd);
+ if (ret)
+ return -1;
+
+ dev->mmap_size = size;
+ dev->mmap_addr = v_addr;
+ dev->vfio_dev_fd = vfio_dev_fd;
+
+ return 0;
+}
+
+void
+bcmfs_release_vfio(struct bcmfs_device *dev)
+{
+ int ret;
+
+ if (dev == NULL)
+ return;
+
+ /* unmap the addr */
+ munmap(dev->mmap_addr, dev->mmap_size);
+ /* release the device */
+ ret = rte_vfio_release_device(dev->dirname, dev->name,
+ dev->vfio_dev_fd);
+ if (ret < 0) {
+ BCMFS_LOG(ERR, "cannot release device");
+ return;
+ }
+}
+#else
+int
+bcmfs_attach_vfio(struct bcmfs_device *dev __rte_unused)
+{
+ return -1;
+}
+
+void
+bcmfs_release_vfio(struct bcmfs_device *dev __rte_unused)
+{
+}
+#endif
new file mode 100644
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2020 Broadcom
+ * All rights reserved.
+ */
+
+#ifndef _BCMFS_VFIO_H_
+#define _BCMFS_VFIO_H_
+
+/* Attach the bcmfs device to vfio */
+int
+bcmfs_attach_vfio(struct bcmfs_device *dev);
+
+/* Release the bcmfs device from vfio */
+void
+bcmfs_release_vfio(struct bcmfs_device *dev);
+
+#endif /* _BCMFS_VFIO_H_ */
@@ -6,5 +6,6 @@
deps += ['eal', 'bus_vdev']
sources = files(
'bcmfs_logs.c',
- 'bcmfs_device.c'
+ 'bcmfs_device.c',
+ 'bcmfs_vfio.c'
)