[v16,2/2] eal: support for VFIO-PCI VF token

  The kernel module vfio-pci introduces the VF token to enable SR-IOV
support since 5.7.

The VF token can be set by a vfio-pci based PF driver and must be known
by the vfio-pci based VF driver in order to gain access to the device.

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 doc/guides/linux_gsg/linux_drivers.rst        | 35 ++++++++++++++++++-
 doc/guides/linux_gsg/linux_eal_parameters.rst |  4 +++
 doc/guides/rel_notes/release_20_08.rst        |  5 +++
 lib/librte_eal/common/eal_common_options.c    |  2 ++
 lib/librte_eal/common/eal_internal_cfg.h      |  2 ++
 lib/librte_eal/common/eal_options.h           |  2 ++
 lib/librte_eal/freebsd/eal.c                  |  4 +++
 lib/librte_eal/include/rte_eal.h              | 15 ++++++++
 lib/librte_eal/linux/eal.c                    | 29 +++++++++++++++
 lib/librte_eal/linux/eal_vfio.c               | 19 ++++++++++
 lib/librte_eal/rte_eal_version.map            |  1 +
 11 files changed, 117 insertions(+), 1 deletion(-)
  

On Wed, Jun 17, 2020 at 02:33:21PM +0800, Haiyue Wang wrote:
> The kernel module vfio-pci introduces the VF token to enable SR-IOV
> support since 5.7.
> 
> The VF token can be set by a vfio-pci based PF driver and must be known
> by the vfio-pci based VF driver in order to gain access to the device.
> 
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---

Tested-by: Harman Kalra <hkalra@marvell.com>

>  doc/guides/linux_gsg/linux_drivers.rst        | 35 ++++++++++++++++++-
>  doc/guides/linux_gsg/linux_eal_parameters.rst |  4 +++
>  doc/guides/rel_notes/release_20_08.rst        |  5 +++
>  lib/librte_eal/common/eal_common_options.c    |  2 ++
>  lib/librte_eal/common/eal_internal_cfg.h      |  2 ++
>  lib/librte_eal/common/eal_options.h           |  2 ++
>  lib/librte_eal/freebsd/eal.c                  |  4 +++
>  lib/librte_eal/include/rte_eal.h              | 15 ++++++++
>  lib/librte_eal/linux/eal.c                    | 29 +++++++++++++++
>  lib/librte_eal/linux/eal_vfio.c               | 19 ++++++++++
>  lib/librte_eal/rte_eal_version.map            |  1 +
>  11 files changed, 117 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/guides/linux_gsg/linux_drivers.rst b/doc/guides/linux_gsg/linux_drivers.rst
> index d40b495c1..02fba7d1e 100644
> --- a/doc/guides/linux_gsg/linux_drivers.rst
> +++ b/doc/guides/linux_gsg/linux_drivers.rst
> @@ -78,11 +78,44 @@ Note that in order to use VFIO, your kernel must support it.
>  VFIO kernel modules have been included in the Linux kernel since version 3.6.0 and are usually present by default,
>  however please consult your distributions documentation to make sure that is the case.
>  
> +The ``vfio-pci`` module since Linux version 5.7 supports the creation of virtual
> +functions. After the PF is bound to vfio-pci module, the user can create the VFs
> +by sysfs interface, and these VFs are bound to vfio-pci module automatically.
> +
> +When the PF is bound to vfio-pci, it has initial VF token generated by random. For
> +security reason, this token is write only, the user can't read it from the kernel
> +directly. For accessing the VF, the user needs to start the PF with token parameter
> +to setup a VF token (uuid format), then the VF can be accessed with this new known
> +VF token.
> +
> +DPDK will use the EAL parameter ``--vfio-vf-token`` to specify the VF token value to
> +PF and its related VFs, this VF token will be shared in all VFIO devices, including
> +the different PFs.
> +
> +.. code-block:: console
> +
> +    1. Generate the VF token by uuid command
> +        14d63f20-8445-11ea-8900-1f9ce7d5650d
> +
> +    2. sudo modprobe vfio-pci enable_sriov=1
> +
> +    2. ./usertools/dpdk-devbind.py -b vfio-pci 0000:86:00.0
> +
> +    3. echo 2 > /sys/bus/pci/devices/0000:86:00.0/sriov_numvfs
> +
> +    4. Start the PF:
> +        ./x86_64-native-linux-gcc/app/testpmd -l 22-25 -n 4 -w 86:00.0 \
> +         --vfio-vf-token=14d63f20-8445-11ea-8900-1f9ce7d5650d --file-prefix=pf -- -i
> +
> +    5. Start the VF:
> +        ./x86_64-native-linux-gcc/app/testpmd -l 26-29 -n 4 -w 86:02.0 \
> +         --vfio-vf-token=14d63f20-8445-11ea-8900-1f9ce7d5650d --file-prefix=vf0 -- -i
> +
>  Also, to use VFIO, both kernel and BIOS must support and be configured to use IO virtualization (such as Intel® VT-d).
>  
>  .. note::
>  
> -    ``vfio-pci`` module doesn't support the creation of virtual functions.
> +    ``vfio-pci`` module doesn't support the creation of virtual functions before Linux version 5.7.
>  
>  For proper operation of VFIO when running DPDK applications as a non-privileged user, correct permissions should also be set up.
>  This can be done by using the DPDK setup script (called dpdk-setup.sh and located in the usertools directory).
> diff --git a/doc/guides/linux_gsg/linux_eal_parameters.rst b/doc/guides/linux_gsg/linux_eal_parameters.rst
> index b2cc60e44..bd3977cb3 100644
> --- a/doc/guides/linux_gsg/linux_eal_parameters.rst
> +++ b/doc/guides/linux_gsg/linux_eal_parameters.rst
> @@ -40,6 +40,10 @@ Device-related options
>  
>      Use specified interrupt mode for devices bound to VFIO kernel driver.
>  
> +*   ``--vfio-vf-token <uuid>``
> +
> +    Use specified VF token for devices bound to VFIO kernel driver.
> +
>  Multiprocessing-related options
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  
> diff --git a/doc/guides/rel_notes/release_20_08.rst b/doc/guides/rel_notes/release_20_08.rst
> index 86d240213..ca7fd521b 100644
> --- a/doc/guides/rel_notes/release_20_08.rst
> +++ b/doc/guides/rel_notes/release_20_08.rst
> @@ -62,6 +62,11 @@ New Features
>  
>    * Added new PMD devarg ``reclaim_mem_mode``.
>  
> +* **Added the support for vfio-pci new VF token interface.**
> +
> +  Since Linux version 5.7, vfio-pci supports a shared VF token (UUID) to represent
> +  the trust between SR-IOV PF and the created VFs. Update the method to gain access
> +  to the PF and VFs devices by appending the VF token parameter.
>  
>  Removed Items
>  -------------
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index 0546beb3a..fc889309c 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -94,6 +94,7 @@ eal_long_options[] = {
>  	{OPT_SYSLOG,            1, NULL, OPT_SYSLOG_NUM           },
>  	{OPT_VDEV,              1, NULL, OPT_VDEV_NUM             },
>  	{OPT_VFIO_INTR,         1, NULL, OPT_VFIO_INTR_NUM        },
> +	{OPT_VFIO_VF_TOKEN,     1, NULL, OPT_VFIO_VF_TOKEN_NUM    },
>  	{OPT_VMWARE_TSC_MAP,    0, NULL, OPT_VMWARE_TSC_MAP_NUM   },
>  	{OPT_LEGACY_MEM,        0, NULL, OPT_LEGACY_MEM_NUM       },
>  	{OPT_SINGLE_FILE_SEGMENTS, 0, NULL, OPT_SINGLE_FILE_SEGMENTS_NUM},
> @@ -301,6 +302,7 @@ eal_reset_internal_config(struct internal_config *internal_cfg)
>  
>  	/* if set to NONE, interrupt mode is determined automatically */
>  	internal_cfg->vfio_intr_mode = RTE_INTR_MODE_NONE;
> +	memset(internal_cfg->vfio_vf_token, 0, sizeof(rte_uuid_t));
>  
>  #ifdef RTE_LIBEAL_USE_HPET
>  	internal_cfg->no_hpet = 0;
> diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
> index c650bc081..43095b7a2 100644
> --- a/lib/librte_eal/common/eal_internal_cfg.h
> +++ b/lib/librte_eal/common/eal_internal_cfg.h
> @@ -72,6 +72,8 @@ struct internal_config {
>  	volatile int syslog_facility;	  /**< facility passed to openlog() */
>  	/** default interrupt mode for VFIO */
>  	volatile enum rte_intr_mode vfio_intr_mode;
> +	/** the shared VF token for VFIO-PCI bound PF and VFs devices */
> +	rte_uuid_t vfio_vf_token;
>  	char *hugefile_prefix;      /**< the base filename of hugetlbfs files */
>  	char *hugepage_dir;         /**< specific hugetlbfs directory to use */
>  	char *user_mbuf_pool_ops_name;
> diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> index 18e6da9ab..89769d48b 100644
> --- a/lib/librte_eal/common/eal_options.h
> +++ b/lib/librte_eal/common/eal_options.h
> @@ -69,6 +69,8 @@ enum {
>  	OPT_VDEV_NUM,
>  #define OPT_VFIO_INTR         "vfio-intr"
>  	OPT_VFIO_INTR_NUM,
> +#define OPT_VFIO_VF_TOKEN     "vfio-vf-token"
> +	OPT_VFIO_VF_TOKEN_NUM,
>  #define OPT_VMWARE_TSC_MAP    "vmware-tsc-map"
>  	OPT_VMWARE_TSC_MAP_NUM,
>  #define OPT_LEGACY_MEM    "legacy-mem"
> diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
> index c41f265fa..4ff5028d8 100644
> --- a/lib/librte_eal/freebsd/eal.c
> +++ b/lib/librte_eal/freebsd/eal.c
> @@ -1005,6 +1005,10 @@ rte_eal_vfio_intr_mode(void)
>  	return RTE_INTR_MODE_NONE;
>  }
>  
> +void rte_eal_vfio_get_vf_token(__rte_unused rte_uuid_t vf_token)
> +{
> +}
> +
>  int rte_vfio_setup_device(__rte_unused const char *sysfs_base,
>  		      __rte_unused const char *dev_addr,
>  		      __rte_unused int *vfio_dev_fd,
> diff --git a/lib/librte_eal/include/rte_eal.h b/lib/librte_eal/include/rte_eal.h
> index 2f9ed298d..2f47347dc 100644
> --- a/lib/librte_eal/include/rte_eal.h
> +++ b/lib/librte_eal/include/rte_eal.h
> @@ -21,6 +21,7 @@
>  #include <rte_bus.h>
>  
>  #include <rte_pci_dev_feature_defs.h>
> +#include <rte_uuid.h>
>  
>  #ifdef __cplusplus
>  extern "C" {
> @@ -438,6 +439,20 @@ int rte_eal_create_uio_dev(void);
>   */
>  enum rte_intr_mode rte_eal_vfio_intr_mode(void);
>  
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Copy the user-configured vfio VF token.
> + *
> + * @param vf_token
> + *   vfio VF token configured with the command line is copied
> + *   into this parameter, zero uuid by default.
> + */
> +__rte_experimental
> +void rte_eal_vfio_get_vf_token(rte_uuid_t vf_token);
> +
>  /**
>   * A wrap API for syscall gettid.
>   *
> diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
> index f162124a3..00f3fdced 100644
> --- a/lib/librte_eal/linux/eal.c
> +++ b/lib/librte_eal/linux/eal.c
> @@ -558,6 +558,7 @@ eal_usage(const char *prgname)
>  	       "  --"OPT_FILE_PREFIX"       Prefix for hugepage filenames\n"
>  	       "  --"OPT_CREATE_UIO_DEV"    Create /dev/uioX (usually done by hotplug)\n"
>  	       "  --"OPT_VFIO_INTR"         Interrupt mode for VFIO (legacy|msi|msix)\n"
> +	       "  --"OPT_VFIO_VF_TOKEN"     VF token (UUID) shared between SR-IOV PF and VFs\n"
>  	       "  --"OPT_LEGACY_MEM"        Legacy memory mode (no dynamic allocation, contiguous segments)\n"
>  	       "  --"OPT_SINGLE_FILE_SEGMENTS" Put all hugepage memory in single files\n"
>  	       "  --"OPT_MATCH_ALLOCATIONS" Free hugepages exactly as allocated\n"
> @@ -649,6 +650,19 @@ eal_parse_vfio_intr(const char *mode)
>  	return -1;
>  }
>  
> +static int
> +eal_parse_vfio_vf_token(const char *vf_token)
> +{
> +	rte_uuid_t uuid;
> +
> +	if (!rte_uuid_parse(vf_token, uuid)) {
> +		rte_uuid_copy(internal_config.vfio_vf_token, uuid);
> +		return 0;
> +	}
> +
> +	return -1;
> +}
> +
>  /* Parse the arguments for --log-level only */
>  static void
>  eal_log_level_parse(int argc, char **argv)
> @@ -787,6 +801,16 @@ eal_parse_args(int argc, char **argv)
>  			}
>  			break;
>  
> +		case OPT_VFIO_VF_TOKEN_NUM:
> +			if (eal_parse_vfio_vf_token(optarg) < 0) {
> +				RTE_LOG(ERR, EAL, "invalid parameters for --"
> +						OPT_VFIO_VF_TOKEN "\n");
> +				eal_usage(prgname);
> +				ret = -1;
> +				goto out;
> +			}
> +			break;
> +
>  		case OPT_CREATE_UIO_DEV_NUM:
>  			internal_config.create_uio_dev = 1;
>  			break;
> @@ -1370,6 +1394,11 @@ rte_eal_vfio_intr_mode(void)
>  	return internal_config.vfio_intr_mode;
>  }
>  
> +void rte_eal_vfio_get_vf_token(rte_uuid_t vf_token)
> +{
> +	rte_uuid_copy(vf_token, internal_config.vfio_vf_token);
> +}
> +
>  int
>  rte_eal_check_module(const char *module_name)
>  {
> diff --git a/lib/librte_eal/linux/eal_vfio.c b/lib/librte_eal/linux/eal_vfio.c
> index d26e1649a..5b2f6b305 100644
> --- a/lib/librte_eal/linux/eal_vfio.c
> +++ b/lib/librte_eal/linux/eal_vfio.c
> @@ -712,6 +712,7 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
>  	int vfio_container_fd;
>  	int vfio_group_fd;
>  	int iommu_group_num;
> +	rte_uuid_t vf_token;
>  	int i, ret;
>  
>  	/* get group number */
> @@ -895,6 +896,23 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
>  				t->type_id, t->name);
>  	}
>  
> +	rte_eal_vfio_get_vf_token(vf_token);
> +
> +	/* get a file descriptor for the device with VF token firstly */
> +	if (!rte_uuid_is_null(vf_token)) {
> +		char vf_token_str[RTE_UUID_STRLEN];
> +		char dev[PATH_MAX];
> +
> +		rte_uuid_unparse(vf_token, vf_token_str, sizeof(vf_token_str));
> +		snprintf(dev, sizeof(dev),
> +			 "%s vf_token=%s", dev_addr, vf_token_str);
> +
> +		*vfio_dev_fd = ioctl(vfio_group_fd, VFIO_GROUP_GET_DEVICE_FD,
> +				     dev);
> +		if (*vfio_dev_fd >= 0)
> +			goto dev_get_info;
> +	}
> +
>  	/* get a file descriptor for the device */
>  	*vfio_dev_fd = ioctl(vfio_group_fd, VFIO_GROUP_GET_DEVICE_FD, dev_addr);
>  	if (*vfio_dev_fd < 0) {
> @@ -909,6 +927,7 @@ rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
>  		return -1;
>  	}
>  
> +dev_get_info:
>  	/* test and setup the device */
>  	ret = ioctl(*vfio_dev_fd, VFIO_DEVICE_GET_INFO, device_info);
>  	if (ret) {
> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index 196eef5af..16c8de27e 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -372,6 +372,7 @@ EXPERIMENTAL {
>  	__rte_trace_point_register;
>  	per_lcore_trace_mem;
>  	per_lcore_trace_point_sz;
> +	rte_eal_vfio_get_vf_token;
>  	rte_log_can_log;
>  	rte_thread_getname;
>  	rte_trace_dump;
> -- 
> 2.27.0
>
  

On Mon, Jun 22, 2020 at 10:40 PM Harman Kalra <hkalra@marvell.com> wrote:
>
> On Wed, Jun 17, 2020 at 02:33:21PM +0800, Haiyue Wang wrote:
> > The kernel module vfio-pci introduces the VF token to enable SR-IOV
> > support since 5.7.
> >
> > The VF token can be set by a vfio-pci based PF driver and must be known
> > by the vfio-pci based VF driver in order to gain access to the device.
> >
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > ---
>
> Tested-by: Harman Kalra <hkalra@marvell.com>

Thanks for the test Harman.

I can see no complaint on using a single token for all devices, which
is the only concern I would have with the last revision.
If everyone is ok with this choice, I will take this for -rc1.
  

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, June 25, 2020 15:33
> To: Harman Kalra <hkalra@marvell.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Cc: Wang, Haiyue <haiyue.wang@intel.com>; dev <dev@dpdk.org>; Burakov, Anatoly
> <anatoly.burakov@intel.com>; Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
> <arybchenko@solarflare.com>; Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: Re: [dpdk-dev] [PATCH v16 2/2] eal: support for VFIO-PCI VF token
> 
> On Mon, Jun 22, 2020 at 10:40 PM Harman Kalra <hkalra@marvell.com> wrote:
> >
> > On Wed, Jun 17, 2020 at 02:33:21PM +0800, Haiyue Wang wrote:
> > > The kernel module vfio-pci introduces the VF token to enable SR-IOV
> > > support since 5.7.
> > >
> > > The VF token can be set by a vfio-pci based PF driver and must be known
> > > by the vfio-pci based VF driver in order to gain access to the device.
> > >
> > > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > > Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > > Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > > ---
> >
> > Tested-by: Harman Kalra <hkalra@marvell.com>
> 
> Thanks for the test Harman.
> 
> I can see no complaint on using a single token for all devices, which

Yeah, not the best, but may meet with the most needed: create VFs by vfio-pci ;-)

Since devarg was a private option for ALL kind of devices, and have to break the
ABI policy to implement the design, so I drop this revision, choose to use it as
a global option as "vfio-intr" option does.

> is the only concern I would have with the last revision.
> If everyone is ok with this choice, I will take this for -rc1.
> 
> 
> --
> David Marchand