[dpdk-dev,v2] vfio: fix pci_vfio_map_resource
Commit Message
The offset of the 2nd mmap() when mapping the region after msix_bar
needs to take region address into consideration as mmap() takes
address that is resource-relative instead of bar-relative. This is
exposed when binding vmxnet3 to vfio-pci.
Fixes: 90a1633b2347 ("eal/linux: allow to map BARs with MSI-X tables")
Signed-off-by: Yong Wang <yongwang@vmware.com>
Signed-off-by: Ronghua Zhang <rzhang@vmware.com>
---
v2:
* Addressed review comment from Dan
lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Comments
> The offset of the 2nd mmap() when mapping the region after msix_bar
> needs to take region address into consideration as mmap() takes address
> that is resource-relative instead of bar-relative. This is exposed when
> binding vmxnet3 to vfio-pci.
>
> Fixes: 90a1633b2347 ("eal/linux: allow to map BARs with MSI-X tables")
>
> Signed-off-by: Yong Wang <yongwang@vmware.com>
> Signed-off-by: Ronghua Zhang <rzhang@vmware.com>
> ---
> v2:
> * Addressed review comment from Dan
>
> lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 5 +++--
> 1 file changed, 3 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 46cd683..bf7cf61 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> @@ -431,7 +431,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
> } else {
> memreg[0].offset = reg.offset;
> memreg[0].size = table_start;
> - memreg[1].offset = table_end;
> + memreg[1].offset = reg.offset + table_end;
> memreg[1].size = reg.size - table_end;
>
> RTE_LOG(DEBUG, EAL,
> @@ -474,7 +474,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
> /* if there's a second part, try to map it */
> if (map_addr != MAP_FAILED
> && memreg[1].offset && memreg[1].size) {
> - void *second_addr =
> RTE_PTR_ADD(bar_addr, memreg[1].offset);
> + void *second_addr =
> RTE_PTR_ADD(bar_addr,
> +
> memreg[1].offset - reg.offset);
> map_addr =
> pci_map_resource(second_addr,
> vfio_dev_fd,
> memreg[1].offset,
> memreg[1].size,
> --
> 1.9.1
Thomas, I guess we can put it in? It doesn't affect anything outside the stated use case, and code wise there's nothing preventing this from being merged either. I've done cursory testing with an ixgbe device just in case, nothing exploded. So,
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
2016-07-14 17:15, Yong Wang:
> - void *second_addr = RTE_PTR_ADD(bar_addr, memreg[1].offset);
> + void *second_addr = RTE_PTR_ADD(bar_addr,
> + memreg[1].offset - reg.offset);
There is an error for 32-bit:
error: cast to pointer from integer of different size
note: in expansion of macro ‘RTE_PTR_ADD’
2016-07-15 17:32, Thomas Monjalon:
> 2016-07-14 17:15, Yong Wang:
> > - void *second_addr = RTE_PTR_ADD(bar_addr, memreg[1].offset);
> > + void *second_addr = RTE_PTR_ADD(bar_addr,
> > + memreg[1].offset - reg.offset);
>
> There is an error for 32-bit:
> error: cast to pointer from integer of different size
> note: in expansion of macro ‘RTE_PTR_ADD’
It can fixed like this:
- memreg[1].offset - reg.offset);
+ memreg[1].offset -
+ (uintptr_t)reg.offset);
2016-07-15 18:42, Thomas Monjalon:
> 2016-07-15 17:32, Thomas Monjalon:
> > 2016-07-14 17:15, Yong Wang:
> > > - void *second_addr = RTE_PTR_ADD(bar_addr, memreg[1].offset);
> > > + void *second_addr = RTE_PTR_ADD(bar_addr,
> > > + memreg[1].offset - reg.offset);
> >
> > There is an error for 32-bit:
> > error: cast to pointer from integer of different size
> > note: in expansion of macro ‘RTE_PTR_ADD’
>
> It can fixed like this:
> - memreg[1].offset - reg.offset);
> + memreg[1].offset -
> + (uintptr_t)reg.offset);
>
Applied with above change, thanks
@@ -431,7 +431,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
} else {
memreg[0].offset = reg.offset;
memreg[0].size = table_start;
- memreg[1].offset = table_end;
+ memreg[1].offset = reg.offset + table_end;
memreg[1].size = reg.size - table_end;
RTE_LOG(DEBUG, EAL,
@@ -474,7 +474,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
/* if there's a second part, try to map it */
if (map_addr != MAP_FAILED
&& memreg[1].offset && memreg[1].size) {
- void *second_addr = RTE_PTR_ADD(bar_addr, memreg[1].offset);
+ void *second_addr = RTE_PTR_ADD(bar_addr,
+ memreg[1].offset - reg.offset);
map_addr = pci_map_resource(second_addr,
vfio_dev_fd, memreg[1].offset,
memreg[1].size,