[dpdk-dev] [PATCH] vfio: fix device unplug when several devices per vfio group

Alejandro Lucero alejandro.lucero at netronome.com
Mon May 8 19:59:10 CEST 2017


I have just sent a fix for this.

Because this was not detected by my testing and Anatoly's either, I suspect
we do not have any automated tests related to VFIO or UIO hotplug.

With other file descriptors already opened by the app, this problem could
have been detected with far less devices than those 64 set by default with
VFIO_MAX_GROUPS. My automated tests use just up to 8VFs but incrementing
that would not help as I can not test the one-vfio-group-per-device case.

It seems there is a way for testing this with older kernels where VFs share
same VFIO group but I would prefer to have something better. I'm working on
adding some sort of test mode to the VFIO kernel code just for this kind of
things, but I would like also to have some automated tests inside DPDK
which can catch this or other similar bugs in the future.

I have not sent any new test to DPDK but I will work on adding some for
this VFIO hotplug feature.

On Mon, May 8, 2017 at 5:44 PM, Alejandro Lucero <
alejandro.lucero at netronome.com> wrote:

> Hi Jerin,
>
> On Mon, May 8, 2017 at 4:20 PM, Jerin Jacob <jerin.jacob at caviumnetworks.co
> m> wrote:
>
>> -----Original Message-----
>> > Date: Sun, 30 Apr 2017 19:29:49 +0200
>> > From: Thomas Monjalon <thomas at monjalon.net>
>> > To: Alejandro Lucero <alejandro.lucero at netronome.com>
>> > Cc: dev at dpdk.org, "Burakov, Anatoly" <anatoly.burakov at intel.com>
>> > Subject: Re: [dpdk-dev] [PATCH] vfio: fix device unplug when several
>> >  devices per vfio group
>> >
>> > 28/04/2017 15:25, Burakov, Anatoly:
>> > > From: Alejandro Lucero [mailto:alejandro.lucero at netronome.com]
>> > > > VFIO allows a secure way of assigning devices to user space and
>> those
>> > > > devices which can not be isolated from other ones are set in same
>> VFIO
>> > > > group. Releasing or unplugging a device should be aware of remaining
>> > > > devices is the same group for avoiding to close such a group.
>> > > >
>> > > > Fixes: 94c0776b1bad ("vfio: support hotplug")
>> > > >
>> > > > Signed-off-by: Alejandro Lucero <alejandro.lucero at netronome.com>
>> > >
>> > > I have tested this on my setup on an old kernel with multiple
>> attach/detaches, and it works (whereas it fails without this patch).
>> > >
>> > > Acked-by: Anatoly  Burakov <anatoly.burakov at intel.com>
>> >
>> > Applied, thanks
>>
>> This patch creates issue when large number of PCIe devices connected to
>> system.
>> Found it through git bisect.
>>
>> This issue is, vfio_group_fd goes beyond 64(VFIO_MAX_GROUPS) and writes
>> to wrong memory on following code execution and sub sequentially creates
>> issues in vfio mapping or such.
>>
> vfio_cfg.vfio_groups[vfio_group_fd].devices++;
>>
>> I can increase VFIO_MAX_GROUPS, but I think, it is not correct fix as
>> vfio_group_fd generated from open system call.
>>
>> I add some prints the code for debug. Please find below the output.
>> Any thoughts from VFIO experts?
>>
>>
> That is a silly but serious bug. We are using the file descriptor as the
> index for updating devices counter of a vfio group structure internal to
> DPDK VFIO code. We should be using the vfio_group that file descriptor is
> registered with.
>
> I will send a fix where vfio_group_device_get/put/count functions are
> implemented which take the file descriptor as a parameter and then go
> through the vfio_group array for working with the right one.
>
> Thomas, is this fix in time yet for 17.05? I will send the patch today but
> I can just test it against a system with the "normal" case for VFIO device
> groups. Maybe Jerin or/and Anatoly can test it against the other case.
>
>
>> ➜ [master]83xx [dpdk-master] $ git diff
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c
>> b/lib/librte_eal/linuxapp/eal/eal_vfio.c
>> index d3eae20..2d8ee4c 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
>> @@ -100,6 +100,7 @@ vfio_get_group_fd(int iommu_group_no)
>>                 snprintf(filename, sizeof(filename),
>>                                  VFIO_GROUP_FMT, iommu_group_no);
>>                 vfio_group_fd = open(filename, O_RDWR);
>> +               printf("###### name %s vfio_group_fd %d\n", filename,
>> vfio_group_fd);
>>                 if (vfio_group_fd < 0) {
>>                         /* if file not found, it's not an error */
>>                         if (errno != ENOENT) {
>> @@ -259,6 +260,8 @@ vfio_setup_device(const char *sysfs_base, const char
>> *dev_addr,
>>         if (vfio_group_fd < 0)
>>                 return -1;
>>
>> +       printf("#### iommu_group_fd %d vfio_group_fd=%d\n",
>> iommu_group_no, vfio_group_fd);
>> +
>>         /* if group_fd == 0, that means the device isn't managed by VFIO
>>  * */
>>         if (vfio_group_fd == 0) {
>>                 RTE_LOG(WARNING, EAL, " %s not managed by VFIO driver,
>> skipping\n",
>> @@ -266,6 +269,7 @@ vfio_setup_device(const char *sysfs_base, const char
>> *dev_addr,
>>                 return 1;
>>         }
>>         /*
>>          * at this point, we know that this group is viable (meaning,
>>          * all devices
>>          * are either bound to VFIO or not bound to anything)
>> @@ -359,6 +363,7 @@ vfio_setup_device(const char *sysfs_base, const char
>> *dev_addr,
>>                 return -1;
>>         }
>>         vfio_cfg.vfio_groups[vfio_group_fd].devices++;
>> +       printf("vfio_group_fd %d device %d\n", vfio_group_fd,
>> vfio_cfg.vfio_groups[vfio_group_fd].devices++);
>>
>>         return 0;
>>  }
>>
>>
>> output log
>> ----------
>>
>> EAL: PCI device 0000:07:00.1 on NUMA socket 0
>> EAL:   probe driver: 177d:a04b octeontx_ssovf
>> ###### name /dev/vfio/114 vfio_group_fd 44
>> #### iommu_group_fd 114 vfio_group_fd=44
>> EAL:   using IOMMU type 1 (Type 1)
>> vfio_group_fd 44 device 1
>> EAL: PCI device 0000:07:00.2 on NUMA socket 0
>> EAL:   probe driver: 177d:a04b octeontx_ssovf
>> ###### name /dev/vfio/115 vfio_group_fd 47
>> #### iommu_group_fd 115 vfio_group_fd=47
>> vfio_group_fd 47 device 1
>> EAL: PCI device 0000:07:00.3 on NUMA socket 0
>> EAL:   probe driver: 177d:a04b octeontx_ssovf
>> ###### name /dev/vfio/116 vfio_group_fd 50
>> #### iommu_group_fd 116 vfio_group_fd=50
>> vfio_group_fd 50 device 1
>> EAL: PCI device 0000:07:00.4 on NUMA socket 0
>> EAL:   probe driver: 177d:a04b octeontx_ssovf
>> ###### name /dev/vfio/117 vfio_group_fd 53
>> #### iommu_group_fd 117 vfio_group_fd=53
>> vfio_group_fd 53 device 1
>> EAL: PCI device 0000:07:00.5 on NUMA socket 0
>> EAL:   probe driver: 177d:a04b octeontx_ssovf
>> ###### name /dev/vfio/118 vfio_group_fd 56
>> #### iommu_group_fd 118 vfio_group_fd=56
>> vfio_group_fd 56 device 1
>> EAL: PCI device 0000:07:00.6 on NUMA socket 0
>> EAL:   probe driver: 177d:a04b octeontx_ssovf
>> ###### name /dev/vfio/119 vfio_group_fd 59
>> #### iommu_group_fd 119 vfio_group_fd=59
>> vfio_group_fd 59 device 1
>> EAL: PCI device 0000:07:00.7 on NUMA socket 0
>> EAL:   probe driver: 177d:a04b octeontx_ssovf
>> ###### name /dev/vfio/120 vfio_group_fd 62
>> #### iommu_group_fd 120 vfio_group_fd=62
>> vfio_group_fd 62 device 1
>> EAL: PCI device 0000:07:01.0 on NUMA socket 0
>> EAL:   probe driver: 177d:a04b octeontx_ssovf
>> ###### name /dev/vfio/121 vfio_group_fd 65
>> #### iommu_group_fd 121 vfio_group_fd=65
>> vfio_group_fd 65 device 1632632833
>>
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^(memory corruption here)
>>
>> EAL: PCI device 0000:08:00.1 on NUMA socket 0
>> EAL:   probe driver: 177d:a04d octeontx_ssowvf
>> ###### name /dev/vfio/122 vfio_group_fd 68
>> #### iommu_group_fd 122 vfio_group_fd=68
>> vfio_group_fd 68 device 1
>> EAL: PCI device 0000:08:00.2 on NUMA socket 0
>> EAL:   probe driver: 177d:a04d octeontx_ssowvf
>> ###### name /dev/vfio/123 vfio_group_fd 71
>> #### iommu_group_fd 123 vfio_group_fd=71
>> vfio_group_fd 71 device 99999941
>> EAL: PCI device 0000:08:00.3 on NUMA socket 0
>> EAL:   probe driver: 177d:a04d octeontx_ssowvf
>> ###### name /dev/vfio/124 vfio_group_fd 74
>> #### iommu_group_fd 124 vfio_group_fd=74
>> vfio_group_fd 74 device 1
>> EAL: PCI device 0000:08:00.4 on NUMA socket 0
>> EAL:   probe driver: 177d:a04d octeontx_ssowvf
>> ###### name /dev/vfio/125 vfio_group_fd 77
>> #### iommu_group_fd 125 vfio_group_fd=77
>> vfio_group_fd 77 device 1
>> EAL: PCI device 0000:08:00.5 on NUMA socket 0
>> EAL:   probe driver: 177d:a04d octeontx_ssowvf
>> ###### name /dev/vfio/126 vfio_group_fd 80
>> #### iommu_group_fd 126 vfio_group_fd=80
>> vfio_group_fd 80 device 1
>> EAL: PCI device 0000:08:00.6 on NUMA socket 0
>> EAL:   probe driver: 177d:a04d octeontx_ssowvf
>> ###### name /dev/vfio/127 vfio_group_fd 83
>> #### iommu_group_fd 127 vfio_group_fd=83
>> vfio_group_fd 83 device 1
>> EAL: PCI device 0000:08:00.7 on NUMA socket 0
>> EAL:   probe driver: 177d:a04d octeontx_ssowvf
>> EAL: PCI device 0000:08:01.0 on NUMA socket 0
>> EAL:   probe driver: 177d:a04d octeontx_ssowvf
>> EAL: PCI device 0001:01:00.1 on NUMA socket 0
>> EAL:   probe driver: 177d:a034 net_thunderx
>> ###### name /dev/vfio/64 vfio_group_fd 86
>> #### iommu_group_fd 64 vfio_group_fd=86
>> vfio_group_fd 86 device 1
>> Segmentation fault
>>
>>
>>
>


More information about the dev mailing list