Hi Vamsi, Fixed one dev_args issue, and send the whole patch to express the whole design: https://patchwork.dpdk.org/patch/68114/
BR, Haiyue > -----Original Message----- > From: Vamsi Krishna Attunuru <[email protected]> > Sent: Friday, April 10, 2020 15:28 > To: Wang, Haiyue <[email protected]>; [email protected] > Cc: Jerin Jacob Kollanukkaran <[email protected]>; > [email protected]; [email protected]; > [email protected] > Subject: RE: [dpdk-dev] [RFC v1 1/1] vfio: set vf token and gain vf device > access > > Hi Wang, > > > -----Original Message----- > > From: dev <[email protected]> On Behalf Of Wang, Haiyue > > Sent: Thursday, April 2, 2020 6:13 PM > > To: Vamsi Krishna Attunuru <[email protected]>; [email protected] > > Cc: Jerin Jacob Kollanukkaran <[email protected]>; > > [email protected]; [email protected]; > > [email protected] > > Subject: Re: [dpdk-dev] [RFC v1 1/1] vfio: set vf token and gain vf device > > access > > > > Hi A Vamsi, > > > > > -----Original Message----- > > > From: dev <[email protected]> On Behalf Of [email protected] > > > Sent: Thursday, March 5, 2020 12:33 > > > To: [email protected] > > > Cc: [email protected]; [email protected]; > > > [email protected]; [email protected]; Vamsi Attunuru > > > <[email protected]> > > > Subject: [dpdk-dev] [RFC v1 1/1] vfio: set vf token and gain vf device > > > access > > > > > > From: Vamsi Attunuru <[email protected]> > > > > > > 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. > > > https://urldefense.proofpoint.com/v2/url?u=http-3A__patches.dpdk.org_c > > > > > over_65915_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=2rpxxNF2qeP02g > > VZIWTVrW > > > -6zNZz5- > > uKt9pRqpR_M3U&m=1z1lwkWphoVghrYUNLI304mER44Vei152GYrXBwn4kk& > > s= > > > tvJ4Li8zpLMbp_p3vKQL7QV09DttNcl70YAr6BVHZm8&e= > > > > > > 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 <[email protected]> > > > > > > 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://urldefense.proofpoint.com/v2/url?u=https- > > 3A__lore.kernel.org_lkml_20200204161737.34696b91- > > 40w520.home_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=2rpxxNF2qeP0 > > 2gVZIWTVrW-6zNZz5- > > uKt9pRqpR_M3U&m=1z1lwkWphoVghrYUNLI304mER44Vei152GYrXBwn4kk& > > s=0pgRda84NrL2omZa0mn-ufSEzKksSi440O2AYImZLaI&e= ), and understand > > the SR-IOV design by reading your RFC, I scratched a simple design. > > Thanks, looks good to me, I guess now it serves the QEMU based use cases as > well. > > > > 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
