* [PATCH] vfio: Fix overflow while assigning vfio BAR region offset and size @ 2015-06-16 14:16 Rahul Lakkireddy 2015-06-17 12:09 ` Thomas Monjalon 2015-06-23 15:00 ` [PATCH v2] " Rahul Lakkireddy 0 siblings, 2 replies; 16+ messages in thread From: Rahul Lakkireddy @ 2015-06-16 14:16 UTC (permalink / raw) To: dev; +Cc: Felix Marti, Kumar Sanghvi, Nirranjan Kirubaharan After the commit-id 90a1633b2 (eal/linux: allow to map BARs with MSI-X tables), VFIO stopped working. On further debug, found that although BAR region offset and size from vfio are read as u64, they are truncated when assigned to uint32_t variables resulting in wrong offset being passed for mmap. The fix is to use uint64_t for offset and size. Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com> Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com> --- lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c index 426953a..d0385ff 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c @@ -728,7 +728,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev) struct vfio_region_info reg = { .argsz = sizeof(reg) }; void *bar_addr; struct memreg { - uint32_t offset, size; + uint64_t offset, size; } memreg[2] = {}; reg.index = i; @@ -771,7 +771,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev) RTE_LOG(DEBUG, EAL, "Trying to map BAR %d that contains the MSI-X " "table. Trying offsets: " - "%04x:%04x, %04x:%04x\n", i, + "%04lx:%04lx, %04lx:%04lx\n", i, memreg[0].offset, memreg[0].size, memreg[1].offset, memreg[1].size); } -- 2.4.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] vfio: Fix overflow while assigning vfio BAR region offset and size 2015-06-16 14:16 [PATCH] vfio: Fix overflow while assigning vfio BAR region offset and size Rahul Lakkireddy @ 2015-06-17 12:09 ` Thomas Monjalon 2015-06-18 14:23 ` Rahul Lakkireddy 2015-06-23 15:00 ` [PATCH v2] " Rahul Lakkireddy 1 sibling, 1 reply; 16+ messages in thread From: Thomas Monjalon @ 2015-06-17 12:09 UTC (permalink / raw) To: Rahul Lakkireddy; +Cc: dev, Felix Marti, Nirranjan Kirubaharan, Kumar Sanghvi Hi Rahul, 2015-06-16 19:46, Rahul Lakkireddy: > After the commit-id 90a1633b2 (eal/linux: allow to map BARs with MSI-X tables), Please show this information before the Signed-off-by lines: Fixes: 90a1633b2347 ("eal/linux: allow to map BARs with MSI-X tables") generated with this git alias: fixline = log -1 --abbrev=12 --format='Fixes: %h (\"%s\")' > VFIO stopped working. On further debug, found that although BAR region I suppose the whole VFIO didn't stopped working. Please precise the conditions of the failures (large offset, etc). > offset and size from vfio are read as u64, they are truncated when assigned to > uint32_t variables resulting in wrong offset being passed for mmap. > > The fix is to use uint64_t for offset and size. Unfortunately, it doesn't build for 32-bit target. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] vfio: Fix overflow while assigning vfio BAR region offset and size 2015-06-17 12:09 ` Thomas Monjalon @ 2015-06-18 14:23 ` Rahul Lakkireddy 0 siblings, 0 replies; 16+ messages in thread From: Rahul Lakkireddy @ 2015-06-18 14:23 UTC (permalink / raw) To: Thomas Monjalon; +Cc: dev, Felix Marti, Nirranjan Kirubaharan, Kumar Sanghvi Hi Thomas, On Wed, Jun 17, 2015 at 14:09:35 +0200, Thomas Monjalon wrote: > Hi Rahul, > > 2015-06-16 19:46, Rahul Lakkireddy: > > After the commit-id 90a1633b2 (eal/linux: allow to map BARs with MSI-X tables), > > Please show this information before the Signed-off-by lines: > Fixes: 90a1633b2347 ("eal/linux: allow to map BARs with MSI-X tables") > generated with this git alias: > fixline = log -1 --abbrev=12 --format='Fixes: %h (\"%s\")' > Ok. I'll add it in v2. > > VFIO stopped working. On further debug, found that although BAR region > > I suppose the whole VFIO didn't stopped working. > Please precise the conditions of the failures (large offset, etc). When using vfio, the probe fails over Chelsio T5 after commit-id 90a1633b2 (eal/linux: allow to map BARs with MSI-X tables). While debugging further, found that the region offset for BAR 2 returned by VFIO is too large to fit in uint32_t. Also, before the above commit, the region offset was passed to mmap as it is and so VFIO is working fine before the above commit. > > > offset and size from vfio are read as u64, they are truncated when assigned to > > uint32_t variables resulting in wrong offset being passed for mmap. > > > > The fix is to use uint64_t for offset and size. > > Unfortunately, it doesn't build for 32-bit target. I tried on RHEL-6.0 32-bit with T=i686-native-linuxapp-gcc and it built fine. So, am I missing something here or some option may be? I will also try out on newer distro like ubuntu 32-bit and confirm. Thanks, Rahul. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2] vfio: Fix overflow while assigning vfio BAR region offset and size 2015-06-16 14:16 [PATCH] vfio: Fix overflow while assigning vfio BAR region offset and size Rahul Lakkireddy 2015-06-17 12:09 ` Thomas Monjalon @ 2015-06-23 15:00 ` Rahul Lakkireddy 2015-06-30 21:12 ` Thomas Monjalon 2015-07-13 8:51 ` [PATCH v3] " Rahul Lakkireddy 1 sibling, 2 replies; 16+ messages in thread From: Rahul Lakkireddy @ 2015-06-23 15:00 UTC (permalink / raw) To: dev; +Cc: Felix Marti, Kumar Sanghvi, Nirranjan Kirubaharan When using vfio, the probe fails over Chelsio T5 adapters after commit-id 90a1633b2 (eal/linux: allow to map BARs with MSI-X tables). While debugging further, found that the BAR region offset and size read from vfio are u64, but are assigned to uint32_t variables. This results in the u64 value getting truncated to 0 and passing wrong offset and size to mmap for subsequent BAR regions (i.e. trying to overwrite previously allocated BAR 0 region). The fix is to use these region offset and size directly rather than assigning to uint32_t variables. Fixes: 90a1633b2347 ("eal/linux: allow to map BARs with MSI-X tables") Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com> Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com> --- v2: - For fixing 32-bit build failure, rather than converting uint32_t var to uint64_t as done in v1, taking a different approach instead to revert a part of above commit-id so as to use the original region offset and size directly. - Add the commit-id that this patch fixes and update commit log. lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c index 426953a..ff974f5 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c @@ -775,9 +775,6 @@ pci_vfio_map_resource(struct rte_pci_device *dev) memreg[0].offset, memreg[0].size, memreg[1].offset, memreg[1].size); } - } else { - memreg[0].offset = reg.offset; - memreg[0].size = reg.size; } /* try to figure out an address */ @@ -815,6 +812,15 @@ pci_vfio_map_resource(struct rte_pci_device *dev) MAP_FIXED); } + if (!map_addr) { + /* Not a BAR containing MSI-X */ + map_addr = pci_map_resource(bar_addr, + vfio_dev_fd, + reg.offset, + reg.size, + MAP_FIXED); + } + if (map_addr == MAP_FAILED || !map_addr) { munmap(bar_addr, reg.size); bar_addr = MAP_FAILED; -- 2.4.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2] vfio: Fix overflow while assigning vfio BAR region offset and size 2015-06-23 15:00 ` [PATCH v2] " Rahul Lakkireddy @ 2015-06-30 21:12 ` Thomas Monjalon 2015-07-01 8:34 ` Alejandro Lucero 2015-07-13 8:51 ` [PATCH v3] " Rahul Lakkireddy 1 sibling, 1 reply; 16+ messages in thread From: Thomas Monjalon @ 2015-06-30 21:12 UTC (permalink / raw) To: anatoly.burakov; +Cc: dev, Nirranjan Kirubaharan, Felix Marti, Kumar Sanghvi Hi Anatoly, Please could you review this fix to allow Chelsio using VFIO? Thanks 2015-06-23 20:30, Rahul Lakkireddy: > When using vfio, the probe fails over Chelsio T5 adapters after > commit-id 90a1633b2 (eal/linux: allow to map BARs with MSI-X tables). > > While debugging further, found that the BAR region offset and size read from > vfio are u64, but are assigned to uint32_t variables. This results in the u64 > value getting truncated to 0 and passing wrong offset and size to mmap for > subsequent BAR regions (i.e. trying to overwrite previously allocated BAR 0 > region). > > The fix is to use these region offset and size directly rather than assigning > to uint32_t variables. > > Fixes: 90a1633b2347 ("eal/linux: allow to map BARs with MSI-X tables") > Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com> > Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] vfio: Fix overflow while assigning vfio BAR region offset and size 2015-06-30 21:12 ` Thomas Monjalon @ 2015-07-01 8:34 ` Alejandro Lucero 2015-07-01 10:00 ` Burakov, Anatoly 0 siblings, 1 reply; 16+ messages in thread From: Alejandro Lucero @ 2015-07-01 8:34 UTC (permalink / raw) To: Thomas Monjalon; +Cc: dev, Felix Marti, Nirranjan Kirubaharan, Kumar Sanghvi I submitted a patch for fixing this issue on the 25th of June. I did not notice someone had reported this before. The last patch from Rahul does not solve the problem. For those cases where the MSI-X table is in one of the BARs to map, the memreg array is still in use. My fix was using unsigned long instead of uint32_t for the memreg array as this is used as a parameter for mmap system call which expects such a type for the offset (and size). This worked for me but I did not realize this has to be compiled for 32 bit systems as well. In that case unsigned long will work for the mmap but not for the VFIO kernel API which expects uint64_t for the offset and size inside the struct vfio_region_info. The point is, the offset param from the vfio_region_info has the index BAR to map. For this VFIO kernel code uses VFIO_PCI_INDEX_TO_OFFSET: #define VFIO_PCI_OFFSET_SHIFT <http://lxr.free-electrons.com/ident?v=3.13;i=VFIO_PCI_OFFSET_SHIFT> 40 #define VFIO_PCI_INDEX_TO_OFFSET <http://lxr.free-electrons.com/ident?v=3.13;i=VFIO_PCI_INDEX_TO_OFFSET>(index <http://lxr.free-electrons.com/ident?v=3.13;i=index>) ((u64 <http://lxr.free-electrons.com/ident?v=3.13;i=u64>)(index <http://lxr.free-electrons.com/ident?v=3.13;i=index>) << VFIO_PCI_OFFSET_SHIFT <http://lxr.free-electrons.com/ident?v=3.13;i=VFIO_PCI_OFFSET_SHIFT>) This index will be used by the VFIO mmap implementation when the DPDK code tries to map the BARs. That code does the opposite for getting the index: index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT); In this case PAGE_SHIFT needs to be used because the mmap system call modifies the offset previously. In a 32-bit system mmap system call and VFIO mmap implementation will get an unsigned long offset, as it does the struct vma_area_struct for vm_pgoff. VFIO will not be able to map the right BAR except for BAR 0. So, basically, VFIO kernel code does not work for 32 bit systems. I think we should define memreg as unsigned long and to report this problem to the VFIO kernel maintainer. On Tue, Jun 30, 2015 at 10:12 PM, Thomas Monjalon <thomas.monjalon@6wind.com > wrote: > Hi Anatoly, > Please could you review this fix to allow Chelsio using VFIO? > Thanks > > 2015-06-23 20:30, Rahul Lakkireddy: > > When using vfio, the probe fails over Chelsio T5 adapters after > > commit-id 90a1633b2 (eal/linux: allow to map BARs with MSI-X tables). > > > > While debugging further, found that the BAR region offset and size read > from > > vfio are u64, but are assigned to uint32_t variables. This results in > the u64 > > value getting truncated to 0 and passing wrong offset and size to mmap > for > > subsequent BAR regions (i.e. trying to overwrite previously allocated > BAR 0 > > region). > > > > The fix is to use these region offset and size directly rather than > assigning > > to uint32_t variables. > > > > Fixes: 90a1633b2347 ("eal/linux: allow to map BARs with MSI-X tables") > > Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com> > > Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com> > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] vfio: Fix overflow while assigning vfio BAR region offset and size 2015-07-01 8:34 ` Alejandro Lucero @ 2015-07-01 10:00 ` Burakov, Anatoly 2015-07-06 15:45 ` Alejandro Lucero 0 siblings, 1 reply; 16+ messages in thread From: Burakov, Anatoly @ 2015-07-01 10:00 UTC (permalink / raw) To: Alejandro Lucero, Thomas Monjalon Cc: dev, Felix Marti, Nirranjan Kirubaharan, Kumar Sanghvi Hi all, > The last patch from Rahul does not solve the problem. For those cases where the MSI-X table is in one of the BARs to map, the memreg array is still in use. Rahul's initial patch was pretty much what you have submitted, it just didn't build on a 32-bit system. > My fix was using unsigned long instead of uint32_t for the memreg array as this is used as a parameter for mmap system call which expects such a type for the offset (and size). Maybe use off_t? That would at least be guaranteed to compile on any system... > In a 32-bit system mmap system call and VFIO mmap implementation will get an unsigned long offset, as it does the struct vma_area_struct for vm_pgoff. > VFIO will not be able to map the right BAR except for BAR 0. > > So, basically, VFIO kernel code does not work for 32 bit systems. > > I think we should define memreg as unsigned long and to report this problem to the VFIO kernel maintainer. If that's the case, this should indeed be taken up with the kernel maintainers. I don't have a 32-bit system handy to test it, unfortunately. Thanks, Anatoly ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] vfio: Fix overflow while assigning vfio BAR region offset and size 2015-07-01 10:00 ` Burakov, Anatoly @ 2015-07-06 15:45 ` Alejandro Lucero 2015-07-07 7:56 ` Rahul Lakkireddy 2015-07-07 9:08 ` Burakov, Anatoly 0 siblings, 2 replies; 16+ messages in thread From: Alejandro Lucero @ 2015-07-06 15:45 UTC (permalink / raw) To: Burakov, Anatoly; +Cc: dev, Felix Marti, Nirranjan Kirubaharan, Kumar Sanghvi Hi all, >From the kernel VFIO maintainer: "I suppose in the short term, mmap should not be advertised as available on 32bit hosts. Thanks," So, as VFIO support for 32bit systems is broken, DPDK should not configure VFIO in that case. This is the complete email sent to the kernel maintainer and his answer: On Thu, 2015-07-02 at 14:42 +0100, Alejandro Lucero wrote: > Hi Alex, > > is VFIO expected to work in 32 bit systems? > > I know VFIO initial goal was a better control of device assignment to > virtual machines and it is based on IOMMU hardware support. I do not know > how likely is to have a 32 bit system with IOMMU but it seems to be > possible such hardware configuration. > > The problem with VFIO and 32 bit systems is the VFIO kernel code uses the > upper bits (VFIO_PCI_OFFSET_SHIFT) of a __u64 variable, offset field in > struct vfio_region_info, for saving info about the PCI BAR index to work > with. This is done inside the ioctl command VFIO_DEVICE_GET_REGION_INFO. > That vfio_region_info is got by the process doing the system call and the > offset is used as a parameter for mmap system call which expects such a > parameter as unsigned long. If I am not wrong, unsigned long in 32 bit > linux systems is a 32 bit type, so when the vfio_pci_mmap function is > executed, the index BAR to work with is obtained from the offset, which > turns to be always 0 as the value was "lost in translation". There is a > chance current implementation can work if all the PCI BARs are equal in > terms of size, but obviously this is not acceptable. > > So, if VFIO needs to work in 32 bits systems another way to map the device > PCI BARs is needed. Not necessarily, VFIO_PCI_OFFSET_SHIFT is an internal implementation detail, userspace should always use vfio_region_info.offset. We're therefore free to come up with other algorithms for handling this limitation. If we want to continue using a single device file descriptor, we could simply choose a smaller shift on 32bit systems. A 29 bit shift would give us 512MB regions support, which is sufficient for the vast majority of devices. We could also replace the macros with functions such that we pack regions as tightly as possible within the device file descriptor. It's reasonable to expect that we could support up to 2G BARs using such a method. A hybrid approach is also possible, for instance the config space region could also contain the ROM and VGA areas (or we could simply choose not to support VGA on 32bit hosts). If we need to support 4G BARs, our only choice is really to extend the vfio region support for a separate file descriptor per region. The only devices I'm aware of with 4G BARs are Nvidia Tesla. This is possible, but I would expect such devices would be extremely rare on 32bit hosts. I suppose in the short term, mmap should not be advertised as available on 32bit hosts. Thanks, Alex On Wed, Jul 1, 2015 at 11:00 AM, Burakov, Anatoly <anatoly.burakov@intel.com > wrote: > Hi all, > > > The last patch from Rahul does not solve the problem. For those cases > where the MSI-X table is in one of the BARs to map, the memreg array is > still in use. > > Rahul's initial patch was pretty much what you have submitted, it just > didn't build on a 32-bit system. > > > My fix was using unsigned long instead of uint32_t for the memreg array > as this is used as a parameter for mmap system call which expects such a > type for the offset (and size). > > Maybe use off_t? That would at least be guaranteed to compile on any > system... > > > In a 32-bit system mmap system call and VFIO mmap implementation will > get an unsigned long offset, as it does the struct vma_area_struct for > vm_pgoff. > > VFIO will not be able to map the right BAR except for BAR 0. > > > > So, basically, VFIO kernel code does not work for 32 bit systems. > > > > I think we should define memreg as unsigned long and to report this > problem to the VFIO kernel maintainer. > > If that's the case, this should indeed be taken up with the kernel > maintainers. I don't have a 32-bit system handy to test it, unfortunately. > > Thanks, > Anatoly > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] vfio: Fix overflow while assigning vfio BAR region offset and size 2015-07-06 15:45 ` Alejandro Lucero @ 2015-07-07 7:56 ` Rahul Lakkireddy 2015-07-07 9:08 ` Burakov, Anatoly 1 sibling, 0 replies; 16+ messages in thread From: Rahul Lakkireddy @ 2015-07-07 7:56 UTC (permalink / raw) To: Alejandro Lucero; +Cc: dev, Felix Marti, Nirranjan Kirubaharan, Kumar Sanghvi Hi Alejandro, On Mon, Jul 06, 2015 at 16:45:01 +0100, Alejandro Lucero wrote: > Hi all, > > From the kernel VFIO maintainer: > > "I suppose in the short term, mmap should not be advertised as available > on 32bit hosts. Thanks," > > So, as VFIO support for 32bit systems is broken, DPDK should not configure > VFIO in that case. > Thank you very much for the clarification. > > If we need to support 4G BARs, our only choice is really to extend the > vfio region support for a separate file descriptor per region. The only > devices I'm aware of with 4G BARs are Nvidia Tesla. This is possible, > but I would expect such devices would be extremely rare on 32bit hosts. > Our Chelsio T5 cards can also have 4G bar size. So, it seems this won't work on 32-bit with current state of kernel vfio driver. Nevertheless, updating your patch with below diff works fine on 64-bit for vfio testing on Chelsio T5 cards and it compiles for 32-bit targets as well. diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c index 426953a..6127f5f 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c @@ -728,7 +728,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev) struct vfio_region_info reg = { .argsz = sizeof(reg) }; void *bar_addr; struct memreg { - uint32_t offset, size; + unsigned long offset, size; } memreg[2] = {}; reg.index = i; @@ -771,7 +771,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev) RTE_LOG(DEBUG, EAL, "Trying to map BAR %d that contains the MSI-X " "table. Trying offsets: " - "%04x:%04x, %04x:%04x\n", i, + "0x%04lx:0x%04lx, 0x%04lx:0x%04lx\n", i, memreg[0].offset, memreg[0].size, memreg[1].offset, memreg[1].size); } Thanks, Rahul ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2] vfio: Fix overflow while assigning vfio BAR region offset and size 2015-07-06 15:45 ` Alejandro Lucero 2015-07-07 7:56 ` Rahul Lakkireddy @ 2015-07-07 9:08 ` Burakov, Anatoly 2015-07-07 10:40 ` Rahul Lakkireddy 1 sibling, 1 reply; 16+ messages in thread From: Burakov, Anatoly @ 2015-07-07 9:08 UTC (permalink / raw) To: Alejandro Lucero; +Cc: dev, Felix Marti, Nirranjan Kirubaharan, Kumar Sanghvi Hi all, > So, as VFIO support for 32bit systems is broken, DPDK should not configure VFIO in that case. ...Or no one should try and run VFIO on a 32-bit system, which should be noted in documentation. I'm a bit wary of adding special handling in this case. Does making the offset off_t fix the compile issues for 32 bit kernel? Thanks, Anatoly ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] vfio: Fix overflow while assigning vfio BAR region offset and size 2015-07-07 9:08 ` Burakov, Anatoly @ 2015-07-07 10:40 ` Rahul Lakkireddy 2015-07-07 10:50 ` Burakov, Anatoly 0 siblings, 1 reply; 16+ messages in thread From: Rahul Lakkireddy @ 2015-07-07 10:40 UTC (permalink / raw) To: Burakov, Anatoly; +Cc: dev, Felix Marti, Nirranjan Kirubaharan, Kumar Sanghvi Hi Anatoly, On Tue, Jul 07, 2015 at 09:08:08 +0000, Burakov, Anatoly wrote: > Hi all, > > > So, as VFIO support for 32bit systems is broken, DPDK should not configure VFIO in that case. > > ...Or no one should try and run VFIO on a 32-bit system, which should be noted in documentation. I'm a bit wary of adding special handling in this case. > > Does making the offset off_t fix the compile issues for 32 bit kernel? > Making offset off_t seems to fail for x86_x32-native-linuxapp-gcc target with the following error in 64-bit gcc, but is working fine with i686 and x86_64 targets: /home/ubuntu/dpdk/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c: In function ‘pci_vfio_map_resource’: /home/ubuntu/dpdk/x86_x32-native-linuxapp-gcc/include/rte_common.h:90:30: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] #define RTE_PTR_ADD(ptr, x) ((void*)((uintptr_t)(ptr) + (x))) ^ /home/ubuntu/dpdk/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c:816:25: note: in expansion of macro ‘RTE_PTR_ADD’ void *second_addr = RTE_PTR_ADD(bar_addr, memreg[1].offset); ^ cc1: all warnings being treated as errors make[7]: *** [eal_pci_vfio.o] Error 1 make[6]: *** [eal] Error 2 make[5]: *** [linuxapp] Error 2 make[4]: *** [librte_eal] Error 2 make[3]: *** [lib] Error 2 make[2]: *** [all] Error 2 make[1]: *** [x86_x32-native-linuxapp-gcc_install] Error 2 make: *** [install] Error 2 However, unsigned long seems to be working fine for all builds. Thanks, Rahul ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] vfio: Fix overflow while assigning vfio BAR region offset and size 2015-07-07 10:40 ` Rahul Lakkireddy @ 2015-07-07 10:50 ` Burakov, Anatoly 2015-07-10 9:54 ` Rahul Lakkireddy 0 siblings, 1 reply; 16+ messages in thread From: Burakov, Anatoly @ 2015-07-07 10:50 UTC (permalink / raw) To: Rahul Lakkireddy; +Cc: dev, Felix Marti, Nirranjan Kirubaharan, Kumar Sanghvi Hi Rahul, > However, unsigned long seems to be working fine for all builds. unsigned long it is then, if there aren't any other objections. Thanks, Anatoly ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] vfio: Fix overflow while assigning vfio BAR region offset and size 2015-07-07 10:50 ` Burakov, Anatoly @ 2015-07-10 9:54 ` Rahul Lakkireddy 2015-07-10 17:27 ` Alejandro Lucero 0 siblings, 1 reply; 16+ messages in thread From: Rahul Lakkireddy @ 2015-07-10 9:54 UTC (permalink / raw) To: Alejandro Lucero; +Cc: dev, Felix Marti, Nirranjan Kirubaharan, Kumar Sanghvi On Tue, Jul 07, 2015 at 10:50:23 +0000, Burakov, Anatoly wrote: > Hi Rahul, > > > However, unsigned long seems to be working fine for all builds. > > unsigned long it is then, if there aren't any other objections. > > Thanks, > Anatoly Hi Alejandro, Are you planning to update the original patch as per below and re-submit: http://dpdk.org/ml/archives/dev/2015-July/020963.html Or, I can also submit it if you want. Please let me know. Thanks, Rahul ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] vfio: Fix overflow while assigning vfio BAR region offset and size 2015-07-10 9:54 ` Rahul Lakkireddy @ 2015-07-10 17:27 ` Alejandro Lucero 0 siblings, 0 replies; 16+ messages in thread From: Alejandro Lucero @ 2015-07-10 17:27 UTC (permalink / raw) To: Rahul Lakkireddy; +Cc: dev, Felix Marti, Nirranjan Kirubaharan, Kumar Sanghvi Hi Rahul, Go ahead. That's fine for me. Thanks On Fri, Jul 10, 2015 at 10:54 AM, Rahul Lakkireddy < rahul.lakkireddy@chelsio.com> wrote: > On Tue, Jul 07, 2015 at 10:50:23 +0000, Burakov, Anatoly wrote: > > Hi Rahul, > > > > > However, unsigned long seems to be working fine for all builds. > > > > unsigned long it is then, if there aren't any other objections. > > > > Thanks, > > Anatoly > > Hi Alejandro, > > Are you planning to update the original patch as per below and re-submit: > http://dpdk.org/ml/archives/dev/2015-July/020963.html > > Or, I can also submit it if you want. > Please let me know. > > Thanks, > Rahul > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3] vfio: Fix overflow while assigning vfio BAR region offset and size 2015-06-23 15:00 ` [PATCH v2] " Rahul Lakkireddy 2015-06-30 21:12 ` Thomas Monjalon @ 2015-07-13 8:51 ` Rahul Lakkireddy 2015-07-14 8:56 ` Thomas Monjalon 1 sibling, 1 reply; 16+ messages in thread From: Rahul Lakkireddy @ 2015-07-13 8:51 UTC (permalink / raw) To: dev; +Cc: Felix Marti, Kumar Sanghvi, Nirranjan Kirubaharan When using vfio, the probe fails for BAR > 0 after the commit-id 90a1633b2 (eal/linux: allow to map BARs with MSI-X tables). While debugging further, found that the BAR region offset and size read from vfio are u64, but are assigned to uint32_t variables. This results in the u64 value getting truncated to 0 and passing wrong offset and size to mmap for subsequent BAR regions. The fix is to use unsigned long for the offset and size. This is based on patch by Alejandro Lucero <alejandro.lucero@netronome.com> posted at below: http://dpdk.org/ml/archives/dev/2015-June/020201.html and updated with diff from below to fix 32-bit compilation: http://dpdk.org/ml/archives/dev/2015-July/020963.html Fixes: 90a1633b2347 ("eal/linux: allow to map BARs with MSI-X tables") Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com> Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com> --- v3: - Use unsigned long instead and updated the commit log. v2: - For fixing 32-bit build failure, rather than converting uint32_t var to uint64_t as done in v1, taking a different approach instead to revert a part of above commit-id so as to use the original region offset and size directly. - Add the commit-id that this patch fixes and update commit log. lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c index 426953a..6127f5f 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c @@ -728,7 +728,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev) struct vfio_region_info reg = { .argsz = sizeof(reg) }; void *bar_addr; struct memreg { - uint32_t offset, size; + unsigned long offset, size; } memreg[2] = {}; reg.index = i; @@ -771,7 +771,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev) RTE_LOG(DEBUG, EAL, "Trying to map BAR %d that contains the MSI-X " "table. Trying offsets: " - "%04x:%04x, %04x:%04x\n", i, + "0x%04lx:0x%04lx, 0x%04lx:0x%04lx\n", i, memreg[0].offset, memreg[0].size, memreg[1].offset, memreg[1].size); } -- 2.4.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3] vfio: Fix overflow while assigning vfio BAR region offset and size 2015-07-13 8:51 ` [PATCH v3] " Rahul Lakkireddy @ 2015-07-14 8:56 ` Thomas Monjalon 0 siblings, 0 replies; 16+ messages in thread From: Thomas Monjalon @ 2015-07-14 8:56 UTC (permalink / raw) To: Rahul Lakkireddy; +Cc: dev, Felix Marti, Nirranjan Kirubaharan, Kumar Sanghvi 2015-07-13 14:21, Rahul Lakkireddy: > When using vfio, the probe fails for BAR > 0 after the > commit-id 90a1633b2 (eal/linux: allow to map BARs with MSI-X tables). > > While debugging further, found that the BAR region offset and size read from > vfio are u64, but are assigned to uint32_t variables. This results in the u64 > value getting truncated to 0 and passing wrong offset and size to mmap for > subsequent BAR regions. > > The fix is to use unsigned long for the offset and size. > > This is based on patch by Alejandro Lucero <alejandro.lucero@netronome.com> > posted at below: > > http://dpdk.org/ml/archives/dev/2015-June/020201.html > > and updated with diff from below to fix 32-bit compilation: > > http://dpdk.org/ml/archives/dev/2015-July/020963.html > > Fixes: 90a1633b2347 ("eal/linux: allow to map BARs with MSI-X tables") > Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com> > Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com> Applied, thanks ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-07-14 8:57 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-06-16 14:16 [PATCH] vfio: Fix overflow while assigning vfio BAR region offset and size Rahul Lakkireddy 2015-06-17 12:09 ` Thomas Monjalon 2015-06-18 14:23 ` Rahul Lakkireddy 2015-06-23 15:00 ` [PATCH v2] " Rahul Lakkireddy 2015-06-30 21:12 ` Thomas Monjalon 2015-07-01 8:34 ` Alejandro Lucero 2015-07-01 10:00 ` Burakov, Anatoly 2015-07-06 15:45 ` Alejandro Lucero 2015-07-07 7:56 ` Rahul Lakkireddy 2015-07-07 9:08 ` Burakov, Anatoly 2015-07-07 10:40 ` Rahul Lakkireddy 2015-07-07 10:50 ` Burakov, Anatoly 2015-07-10 9:54 ` Rahul Lakkireddy 2015-07-10 17:27 ` Alejandro Lucero 2015-07-13 8:51 ` [PATCH v3] " Rahul Lakkireddy 2015-07-14 8:56 ` Thomas Monjalon
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.