On 4/26/20 4:55 AM, 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.w...@intel.com>
> Acked-by: Vamsi Attunuru <vattun...@marvell.com>
> Tested-by: Vamsi Attunuru <vattun...@marvell.com>
Few points below, anyway:
Acked-by: Andrew Rybchenko <arybche...@solarflare.com>
[snip]
> diff --git a/doc/guides/linux_gsg/linux_drivers.rst
> b/doc/guides/linux_gsg/linux_drivers.rst
> index 238f3e900..b42fd708b 100644
> --- a/doc/guides/linux_gsg/linux_drivers.rst
> +++ b/doc/guides/linux_gsg/linux_drivers.rst
> @@ -72,11 +72,50 @@ 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.
If token is write-only in kernel sysfs, shouldn't we make it
invisible in ps output (procfs)? I.e. substitute with something
like xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx.
However, it does not look easy. Just a thought.
[snip]
> diff --git a/drivers/bus/pci/linux/pci_vfio.c
> b/drivers/bus/pci/linux/pci_vfio.c
> index 64cd84a68..efb64e2ba 100644
> --- a/drivers/bus/pci/linux/pci_vfio.c
> +++ b/drivers/bus/pci/linux/pci_vfio.c
> @@ -11,6 +11,7 @@
> #include <sys/mman.h>
> #include <stdbool.h>
>
> +#include <rte_devargs.h>
> #include <rte_log.h>
> #include <rte_pci.h>
> #include <rte_bus_pci.h>
> @@ -644,12 +645,72 @@ pci_vfio_msix_is_mappable(int vfio_dev_fd, int
> msix_region)
> return ret;
> }
>
> +static int
> +vfio_pci_vf_token_arg(struct rte_devargs *devargs, rte_uuid_t uuid)
> +{
> +#define VF_TOKEN_ARG "vf_token="
> + char c, *p, *vf_token;
> +
> + memset(uuid, 0, sizeof(rte_uuid_t));
> +
> + if (devargs == NULL)
> + return 0;
> +
> + p = strstr(devargs->args, VF_TOKEN_ARG);
> + if (!p)
Please, p == NULL as you do few lines above.
[snip]
