Re: [dpdk-dev] [RFC v1 1/1] vfio: set vf token and gain vf device access

["Wang, Haiyue" <haiyue.wang@intel.com>]

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 <vattunuru@marvell.com>
> Sent: Friday, April 10, 2020 15:28
> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; alex.williamson@redhat.com; thomas@monjalon.net;
> david.marchand@redhat.com
> Subject: RE: [dpdk-dev] [RFC v1 1/1] vfio: set vf token and gain vf device access
> 
> Hi Wang,
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Wang, Haiyue
> > Sent: Thursday, April 2, 2020 6:13 PM
> > To: Vamsi Krishna Attunuru <vattunuru@marvell.com>; dev@dpdk.org
> > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> > alex.williamson@redhat.com; thomas@monjalon.net;
> > david.marchand@redhat.com
> > Subject: Re: [dpdk-dev] [RFC v1 1/1] vfio: set vf token and gain vf device
> > access
> >
> > Hi A Vamsi,
> >
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of vattunuru@marvell.com
> > > Sent: Thursday, March 5, 2020 12:33
> > > To: dev@dpdk.org
> > > Cc: jerinj@marvell.com; alex.williamson@redhat.com;
> > > thomas@monjalon.net; david.marchand@redhat.com; Vamsi Attunuru
> > > <vattunuru@marvell.com>
> > > Subject: [dpdk-dev] [RFC v1 1/1] vfio: set vf token and gain vf device
> > > access
> > >
> > > From: Vamsi Attunuru <vattunuru@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.
> > > 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 <vattunuru@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://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