Hi A Vamsi,

> -----Original Message-----
> From: dev <dev-boun...@dpdk.org> On Behalf Of vattun...@marvell.com
> Sent: Thursday, March 5, 2020 12:33
> To: dev@dpdk.org
> Cc: jer...@marvell.com; alex.william...@redhat.com; tho...@monjalon.net; 
> david.march...@redhat.com;
> Vamsi Attunuru <vattun...@marvell.com>
> Subject: [dpdk-dev] [RFC v1 1/1] vfio: set vf token and gain vf device access
> 
> From: Vamsi Attunuru <vattun...@marvell.com>
> 
> vfio-pci driver enables virtual function access from
> the DPDK applications when those vf device's physical
> function is also bound to vfio driver.
> 
> Patch adds the required configuration and checks to
> enable DPDK applications to access both pf and it's
> vf devices through vfio-pci driver.
> 
> See background on vf token scheme in linux vfio driver.
> http://patches.dpdk.org/cover/65915/
> 
> When a physical function is enabled with  non-zero
> virtual functions,  patch sets the UUID using
> VFIO_DEVICE_FEATURE ioctl from physical function's
> file descriptor. Same UUID is used to gain the access
> for the virtual functions on those  physical function.
> 
> Following changes required on top of this DPDK patch
> * Kernel version check for VFIO_DEVICE_FEATURE ioctl
> * Use uuid gen API to generate UUID.
> 
> Signed-off-by: Vamsi Attunuru <vattun...@marvell.com>
> 
> diff --git a/lib/librte_eal/linux/eal/eal_vfio.c 
> b/lib/librte_eal/linux/eal/eal_vfio.c
> index 01b5ef3..e2fdd35 100644
> --- a/lib/librte_eal/linux/eal/eal_vfio.c
> +++ b/lib/librte_eal/linux/eal/eal_vfio.c
> @@ -12,6 +12,7 @@
>  #include <rte_log.h>
>  #include <rte_memory.h>
>  #include <rte_eal_memconfig.h>
> +#include <rte_uuid.h>
>  #include <rte_vfio.h>
> 
>  #include "eal_filesystem.h"
> @@ -50,6 +51,9 @@ struct vfio_config {
>       struct user_mem_maps mem_maps;
>  };
> 
> +rte_uuid_t uuid_token = RTE_UUID_INIT(0xf8615163, 0xdf3e, 0x46c5,
> +                                   0x913f, 0xf2d2f965ed0eULL);
> +
>  /* per-process VFIO config */
>  static struct vfio_config vfio_cfgs[VFIO_MAX_CONTAINERS];
>  static struct vfio_config *default_vfio_cfg = &vfio_cfgs[0];
> @@ -657,6 +661,102 @@ rte_vfio_clear_group(int vfio_group_fd)
>       return 0;
>  }
> 
> +static bool
> +rte_vfio_dev_is_physfn(const char *sysfs_base, const char *dev_addr)
> +{
> +     char linkname[PATH_MAX];
> +     char filename[PATH_MAX];
> +     int ret;
> +
> +     memset(linkname, 0, sizeof(linkname));
> +     memset(filename, 0, sizeof(filename));
> +
> +     /* check if physfn directory exist for this device */
> +     snprintf(linkname, sizeof(linkname),
> +                      "%s/%s/physfn", sysfs_base, dev_addr);
> +
> +     ret = readlink(linkname, filename, sizeof(filename));
> +
> +     /* For PFs, physfn directory does not exist */
> +     if (ret < 0)
> +             return true;
> +
> +     return false;
> +}
> +
> +static int
> +is_vf_token_required(const char *sysfs_base, const char *dev_addr)
> +{
> +     char *tok[16], *physfn, *physfn_drv;
> +     char linkname[PATH_MAX];
> +     char filename[PATH_MAX];
> +     int ret;
> +
> +     memset(linkname, 0, sizeof(linkname));
> +     memset(filename, 0, sizeof(filename));
> +
> +     snprintf(linkname, sizeof(linkname),
> +                      "%s/%s/physfn", sysfs_base, dev_addr);
> +
> +     ret = readlink(linkname, filename, sizeof(filename));
> +     if (ret < 0)
> +             return -1;
> +
> +     ret = rte_strsplit(filename, sizeof(filename),
> +                     tok, RTE_DIM(tok), '/');
> +     if (ret <= 0) {
> +             RTE_LOG(ERR, EAL, " %s cannot get it's physfn\n", dev_addr);
> +             return -1;
> +     }
> +
> +     physfn = tok[ret - 1];
> +
> +     snprintf(linkname, sizeof(linkname),
> +                      "/sys/bus/pci/devices/%s/driver", physfn);
> +     ret = readlink(linkname, filename, sizeof(filename));
> +     if (ret < 0)
> +             return -1;
> +
> +     ret = rte_strsplit(filename, sizeof(filename),
> +                     tok, RTE_DIM(tok), '/');
> +     if (ret <= 0) {
> +             RTE_LOG(ERR, EAL, "  %s cannot get it's physfn driver info\n",
> +                     dev_addr);
> +             return -1;
> +     }
> +
> +     physfn_drv = tok[ret - 1];
> +
> +     if (strncmp(physfn_drv, "vfio-pci", sizeof("vfio-pci")))
> +             return 1;
> +
> +     /* physfn is bound to vfio-pci */
> +     return 0;
> +}
> +

Based on Alex's vfio for qemu patch 
(https://lore.kernel.org/lkml/20200204161737.34696...@w520.home/),
and understand the SR-IOV design by reading your RFC, I scratched a simple 
design.

1. ./usertools/dpdk-devbind.py -b vfio-pci 0000:87:00.0
2. echo 2 > /sys/bus/pci/devices/0000:87:00.0/sriov_numvfs
3. ./x86_64-native-linux-gcc/app/testpmd -l 22-25 -n 4 -w 
87:00.0,vf_token=2ab74924-c335-45f4-9b16-8569e5b08258 --file-prefix=pf -- -i

  If no token in VF:
./x86_64-native-linux-gcc/app/testpmd -l 26-29 -n 4 -w 87:02.0 
--file-prefix=vf1 -- -i
Kernel error:   vfio-pci 0000:87:02.0: VF token required to access device

./x86_64-native-linux-gcc/app/testpmd -l 26-29 -n 4 -w 
87:02.0,vf_token=2ab74924-c335-45f4-9b16-8569e5b08258 --file-prefix=vf1 -- -i


static int
vfio_pci_vf_token_arg_handler(__rte_unused const char *key,
                              const char *value, void *opaque)
{
        if (rte_uuid_parse(value, opaque)) {
                RTE_LOG(ERR, EAL,
                        "The VF token is not a valid uuid : %s\n", value);
                return -1;
        }

        return 0;
}

static int
vfio_pci_vf_token_arg(struct rte_devargs *devargs, rte_uuid_t uu)
{
        const char *key = "vf_token";
        struct rte_kvargs *kvlist;
        int ret = 0;

        if (devargs == NULL)
                return 0;

        kvlist = rte_kvargs_parse(devargs->args, NULL);
        if (kvlist == NULL)
                return 0;

        if (!rte_kvargs_count(kvlist, key))
                goto exit;

        if (rte_kvargs_process(kvlist, key,
                               vfio_pci_vf_token_arg_handler, uu) < 0)
                goto exit;

        ret = 1;

exit:
        rte_kvargs_free(kvlist);
        return ret;
}



static int
 pci_vfio_map_resource_primary(struct rte_pci_device *dev)
 {
        struct vfio_device_info device_info = { .argsz = sizeof(device_info) };
+       rte_uuid_t vf_token = RTE_UUID_INIT(0, 0, 0, 0, 0ULL);
        char pci_addr[PATH_MAX] = {0};
        int vfio_dev_fd;
        struct rte_pci_addr *loc = &dev->addr;
@@ -668,8 +712,9 @@ pci_vfio_map_resource_primary(struct rte_pci_device *dev)
        snprintf(pci_addr, sizeof(pci_addr), PCI_PRI_FMT,
                        loc->domain, loc->bus, loc->devid, loc->function);
 
+       vfio_pci_vf_token_arg(dev->device.devargs, vf_token);
        ret = rte_vfio_setup_device(rte_pci_get_sysfs_path(), pci_addr,
-                                       &vfio_dev_fd, &device_info);
+                                       &vfio_dev_fd, &device_info, vf_token);
        if (ret)
                return ret;


int
 rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
-               int *vfio_dev_fd, struct vfio_device_info *device_info)
+               int *vfio_dev_fd, struct vfio_device_info *device_info,
+               rte_uuid_t vf_token)
 {
        struct vfio_group_status group_status = {
                        .argsz = sizeof(group_status)
@@ -712,6 +713,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;
+       char dev[PATH_MAX];
        int i, ret;
 
        /* get group number */
@@ -895,8 +897,19 @@ rte_vfio_setup_device(const char *sysfs_base, const char 
*dev_addr,
                                t->type_id, t->name);
        }
 
+       if (!rte_uuid_is_null(vf_token)) {
+               char vf_token_str[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);
+       } else {
+               snprintf(dev, sizeof(dev),
+                        "%s", dev_addr);
+       }
+
        /* get a file descriptor for the device */
-       *vfio_dev_fd = ioctl(vfio_group_fd, VFIO_GROUP_GET_DEVICE_FD, dev_addr);
+       *vfio_dev_fd = ioctl(vfio_group_fd, VFIO_GROUP_GET_DEVICE_FD, dev);



> +static bool
> +rte_vfio_dev_has_nonzero_numvfs(const char *sysfs_base, const char *dev_addr)
> +{
> +     char linkname[PATH_MAX];
> +     unsigned long num_vfs;
> +     int ret;
> +
> +     if (!rte_vfio_dev_is_physfn(sysfs_base, dev_addr))
> +             return false;
> +
> +     memset(linkname, 0, sizeof(linkname));
> +
> +     snprintf(linkname, sizeof(linkname),
> +                      "%s/%s/sriov_numvfs", sysfs_base, dev_addr);
> +
> +     ret  = eal_parse_sysfs_value(linkname, &num_vfs);
> +
> +     if ((ret < 0) || (num_vfs == 0))
> +             return false;
> +
> +     return true;
> +}
> +
>  int
>  rte_vfio_setup_device(const char *sysfs_base, const char *dev_addr,
>               int *vfio_dev_fd, struct vfio_device_info *device_info)
> @@ -669,6 +769,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;
> +     char dev[PATH_MAX];
>       int i, ret;
> 
>       /* get group number */
> @@ -683,6 +784,29 @@ rte_vfio_setup_device(const char *sysfs_base, const char 
> *dev_addr,
>       if (ret < 0)
>               return -1;
> 
> +     snprintf(dev, sizeof(dev), "%s", dev_addr);
> +
> +     if (!rte_vfio_dev_is_physfn(sysfs_base, dev_addr)) {
> +             char vf_token[PATH_MAX];
> +             /*
> +              *  Check if vf_token is required or not,
> +              *  vf_token is required when the VF's physfn is
> +              *  binded with vfio-pci driver
> +              */
> +             ret = is_vf_token_required(sysfs_base, dev_addr);
> +             /* if negative, something failed */
> +             if (ret < 0)
> +                     return -1;
> +
> +             if (ret == 0) {
> +                     /* vf_token required to open device file descriptor */
> +                     rte_uuid_unparse(uuid_token,
> +                                      vf_token, sizeof(vf_token));
> +                     snprintf(dev, sizeof(dev),
> +                              "%s vf_token=%s", dev_addr, vf_token);
> +             }
> +     }
> +
>       /* get the actual group fd */
>       vfio_group_fd = rte_vfio_get_group_fd(iommu_group_num);
>       if (vfio_group_fd < 0)
> @@ -853,7 +977,7 @@ rte_vfio_setup_device(const char *sysfs_base, const char 
> *dev_addr,
>       }
> 
>       /* get a file descriptor for the device */
> -     *vfio_dev_fd = ioctl(vfio_group_fd, VFIO_GROUP_GET_DEVICE_FD, dev_addr);
> +     *vfio_dev_fd = ioctl(vfio_group_fd, VFIO_GROUP_GET_DEVICE_FD, dev);
>       if (*vfio_dev_fd < 0) {
>               /* if we cannot get a device fd, this implies a problem with
>                * the VFIO group or the container not having IOMMU configured.
> @@ -877,6 +1001,31 @@ rte_vfio_setup_device(const char *sysfs_base, const 
> char *dev_addr,
>               rte_vfio_clear_group(vfio_group_fd);
>               return -1;
>       }
> +
> +     if (rte_vfio_dev_has_nonzero_numvfs(sysfs_base, dev_addr)) {
> +#define VF_TOKEN  (sizeof(struct vfio_device_feature) + sizeof(rte_uuid_t))
> +
> +             struct vfio_device_feature *vf_token;
> +             uint8_t local[VF_TOKEN];
> +
> +             memset(local, 0, VF_TOKEN);
> +             vf_token = (struct vfio_device_feature *)local;
> +             vf_token->argsz = VF_TOKEN;
> +             vf_token->flags = VFIO_DEVICE_FEATURE_SET |
> +                                     VFIO_DEVICE_FEATURE_PCI_VF_TOKEN;
> +
> +             memcpy(local + sizeof(struct vfio_device_feature),
> +                    &uuid_token, sizeof(uuid_token));
> +
> +             ret = ioctl(*vfio_dev_fd, VFIO_DEVICE_FEATURE, vf_token);
> +             if (ret) {
> +                     RTE_LOG(ERR, EAL, " Failed to set UUID on %s "
> +                                     "error %i (%s)\n", dev_addr, errno,
> +                                     strerror(errno));
> +                     return -1;
> +             }
> +     }
> +
>       vfio_group_device_get(vfio_group_fd);
> 
>       return 0;
> --
> 2.8.4

Reply via email to