From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua0-f169.google.com (mail-ua0-f169.google.com [209.85.217.169]) by dpdk.org (Postfix) with ESMTP id 63BE0F72 for ; Mon, 8 May 2017 19:59:11 +0200 (CEST) Received: by mail-ua0-f169.google.com with SMTP id e55so48717404uaa.2 for ; Mon, 08 May 2017 10:59:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=netronome-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=gJT3CaZMUbCt2opWkbDEW4vjxCLb36A95xvX5kRN+3s=; b=Zs+b7nQayhByEO5/zTehH2UHCdq6H+VeKbrok/ORRVSFlLg2zMPN5RceiStjnKuzZb tSVlNrzP3gC4dftVUBb7RLetdZRdrY8NlJkmZexo/Gn74QqiaDabJuEML4sUmIBjvSkt bboL5eakWAK9m8mY62anCGgwEfLnyOVmuLqh470U9Y0ocMLTMwyvkkPMnQcdZhYkU6rP XpcjahDJtdu651NNwA7ICH+fVzdbKeZHp1Qzi6gtV5jhcRFEZ4LDUi34tdNFFPtfmy6B TFivfexUaOThGSNoRoaKRCjhJX0TMVU6bD62zN0q5+pWZPmbAKH/7XQ9mC03vaPzQpJe 1CGw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=gJT3CaZMUbCt2opWkbDEW4vjxCLb36A95xvX5kRN+3s=; b=horOxGGq+tuiCoqk3XAXZm3Hh4QgHkX0pLqAKV2M+V9Nw5n5sIm0X52Wl4eYrcQFHk /NWSW9i0s539kfCJISZdEzPlobrgMYRDSfwfdbY33fctYLCn9RGK0zCfEP+70Nu/S+in HpqxTqsOO7dRKBtI2NTbFnzEh+O9oakvLF+F+OTN+zuIdniWjDYkFPpLR6mK2gmE2z2/ x29WK5VwWIxTcVB5RR54VXDdecC5x+XlDSKIDzylT6RJjtHdO4ADt+KK3qbPZzXviks0 CdRth5n5nN3+rfkT/jo0YdOPuK56/SKvjNT4wzciqETM3NnIBWIq/9mKhKZCVLRaZrhV Cqog== X-Gm-Message-State: AODbwcB3jWDmDKNdV4FKbK2rcv/yKGNW2zFhZnVwgaAPa8rjUmi1cUSU JBpnxFLobEp04u9zA+PIc2wOOY5Qmuxi X-Received: by 10.31.33.3 with SMTP id h3mr7968506vkh.29.1494266350534; Mon, 08 May 2017 10:59:10 -0700 (PDT) MIME-Version: 1.0 Received: by 10.103.14.69 with HTTP; Mon, 8 May 2017 10:59:10 -0700 (PDT) In-Reply-To: References: <1493203787-25555-1-git-send-email-alejandro.lucero@netronome.com> <1528500.cNnDXbOJL1@xps> <20170508152006.GA28180@jerin> From: Alejandro Lucero Date: Mon, 8 May 2017 18:59:10 +0100 Message-ID: To: Jerin Jacob Cc: Thomas Monjalon , dev , "Burakov, Anatoly" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH] vfio: fix device unplug when several devices per vfio group X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 May 2017 17:59:11 -0000 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 m> wrote: > >> -----Original Message----- >> > Date: Sun, 30 Apr 2017 19:29:49 +0200 >> > From: Thomas Monjalon >> > To: Alejandro Lucero >> > Cc: dev@dpdk.org, "Burakov, Anatoly" >> > 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 remaini= ng >> > > > devices is the same group for avoiding to close such a group. >> > > > >> > > > Fixes: 94c0776b1bad ("vfio: support hotplug") >> > > > >> > > > Signed-off-by: Alejandro Lucero >> > > >> > > 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 >> > >> > 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 bu= t > I can just test it against a system with the "normal" case for VFIO devic= e > groups. Maybe Jerin or/and Anatoly can test it against the other case. > > >> =E2=9E=9C [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 =3D 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 !=3D 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=3D%d\n", >> iommu_group_no, vfio_group_fd); >> + >> /* if group_fd =3D=3D 0, that means the device isn't managed by = VFIO >> * */ >> if (vfio_group_fd =3D=3D 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=3D44 >> 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=3D47 >> 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=3D50 >> 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=3D53 >> 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=3D56 >> 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=3D59 >> 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=3D62 >> 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=3D65 >> 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=3D68 >> 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=3D71 >> 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=3D74 >> 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=3D77 >> 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=3D80 >> 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=3D83 >> 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=3D86 >> vfio_group_fd 86 device 1 >> Segmentation fault >> >> >> >