diff mbox

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

Message ID 20170508152006.GA28180@jerin (mailing list archive)
State Not Applicable, archived
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation fail apply patch file failure

Commit Message

Jerin Jacob May 8, 2017, 3:20 p.m. UTC 
  -----Original Message-----
> Date: Sun, 30 Apr 2017 19:29:49 +0200
> From: Thomas Monjalon <thomas@monjalon.net>
> To: Alejandro Lucero <alejandro.lucero@netronome.com>
> Cc: dev@dpdk.org, "Burakov, Anatoly" <anatoly.burakov@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@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@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@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?

➜ [master]83xx [dpdk-master] $ git diff
        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

Comments

Alejandro Lucero May 8, 2017, 4:44 p.m. UTC | #1 
Hi Jerin,

On Mon, May 8, 2017 at 4:20 PM, Jerin Jacob <jerin.jacob@caviumnetworks.com>
wrote:

> -----Original Message-----
> > Date: Sun, 30 Apr 2017 19:29:49 +0200
> > From: Thomas Monjalon <thomas@monjalon.net>
> > To: Alejandro Lucero <alejandro.lucero@netronome.com>
> > Cc: dev@dpdk.org, "Burakov, Anatoly" <anatoly.burakov@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@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@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@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
>
>
>
Alejandro Lucero May 8, 2017, 5:59 p.m. UTC | #2 
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@netronome.com> wrote:

> Hi Jerin,
>
> On Mon, May 8, 2017 at 4:20 PM, Jerin Jacob <jerin.jacob@caviumnetworks.co
> m> wrote:
>
>> -----Original Message-----
>> > Date: Sun, 30 Apr 2017 19:29:49 +0200
>> > From: Thomas Monjalon <thomas@monjalon.net>
>> > To: Alejandro Lucero <alejandro.lucero@netronome.com>
>> > Cc: dev@dpdk.org, "Burakov, Anatoly" <anatoly.burakov@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@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@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@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
>>
>>
>>
>
Jerin Jacob May 9, 2017, 4:13 a.m. UTC | #3 
-----Original Message-----
> Date: Mon, 8 May 2017 17:44:37 +0100
> From: Alejandro Lucero <alejandro.lucero@netronome.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>, dev <dev@dpdk.org>, "Burakov,
>  Anatoly" <anatoly.burakov@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] vfio: fix device unplug when several
>  devices per vfio group
> 
> Hi Jerin,
> 
> On Mon, May 8, 2017 at 4:20 PM, Jerin Jacob <jerin.jacob@caviumnetworks.com>
> wrote:
> 
> > -----Original Message-----
> > > Date: Sun, 30 Apr 2017 19:29:49 +0200
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > To: Alejandro Lucero <alejandro.lucero@netronome.com>
> > > Cc: dev@dpdk.org, "Burakov, Anatoly" <anatoly.burakov@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@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@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@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.


Thanks Alejandro for the patch.
Tested your patch on failure setup, it works fine after applying your
patch.

IMO, for v17.05, this fix must go in or we need to revert the original offending
patch(a9c349e3a100 ("vfio: fix device unplug when several devices per
group")) as it breaks DPDK running on the system with few PCIe devices connected in.
diff mbox

Patch

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
 * */