Hello Shahaf, On Wed, Feb 13, 2019 at 11:10:21AM +0200, Shahaf Shuler wrote: > Enable users the option to call rte_vfio_dma_map with request to map > to the default vfio fd. > > Signed-off-by: Shahaf Shuler <shah...@mellanox.com> > --- > lib/librte_eal/common/include/rte_vfio.h | 6 ++++-- > lib/librte_eal/linuxapp/eal/eal_vfio.c | 14 ++++++++++++-- > 2 files changed, 16 insertions(+), 4 deletions(-) > > diff --git a/lib/librte_eal/common/include/rte_vfio.h > b/lib/librte_eal/common/include/rte_vfio.h > index cae96fab90..2a6827012f 100644 > --- a/lib/librte_eal/common/include/rte_vfio.h > +++ b/lib/librte_eal/common/include/rte_vfio.h > @@ -347,7 +347,8 @@ rte_vfio_container_group_unbind(int container_fd, int > iommu_group_num); > * Perform DMA mapping for devices in a container. > * > * @param container_fd > - * the specified container fd > + * the specified container fd. In case of -1 the default container > + * fd will be used.
I think #define RTE_VFIO_DEFAULT_CONTAINER_FD (-1) might help reading the code that will later call these functions. > * > * @param vaddr > * Starting virtual address of memory to be mapped. > @@ -370,7 +371,8 @@ rte_vfio_container_dma_map(int container_fd, uint64_t > vaddr, > * Perform DMA unmapping for devices in a container. > * > * @param container_fd > - * the specified container fd > + * the specified container fd. In case of -1 the default container > + * fd will be used. > * > * @param vaddr > * Starting virtual address of memory to be unmapped. > diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c > b/lib/librte_eal/linuxapp/eal/eal_vfio.c > index c821e83826..48ca9465d4 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c > +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c > @@ -1897,7 +1897,12 @@ rte_vfio_container_dma_map(int container_fd, uint64_t > vaddr, uint64_t iova, > return -1; > } > > - vfio_cfg = get_vfio_cfg_by_container_fd(container_fd); > + if (container_fd > 0) { Should it not be container_fd >= 0? This seems inconsistent with the doc above. Reading the code quickly, it's not clear that the container_fd==0 would be at vfio_cfgs[0], so this seems wrong. > + vfio_cfg = get_vfio_cfg_by_container_fd(container_fd); > + } else { > + /* when no fd provided use the default. */ > + vfio_cfg = &vfio_cfgs[0]; > + } Can you use: vfio_cfg = default_vfio_cfg; instead? Then the comment is redundant. Actually, to keep with my comment above, it might be better to simply have if (container_fd == RTE_VFIO_DEFAULT_CONTAINER_FD) vfio_cfg = default_vfio_cfg; else vfio_cfg = get_vfio_cfg_by_group_num(container_fd); > if (vfio_cfg == NULL) { > RTE_LOG(ERR, EAL, "Invalid container fd\n"); > return -1; > @@ -1917,7 +1922,12 @@ rte_vfio_container_dma_unmap(int container_fd, > uint64_t vaddr, uint64_t iova, > return -1; > } > > - vfio_cfg = get_vfio_cfg_by_container_fd(container_fd); > + if (container_fd > 0) { > + vfio_cfg = get_vfio_cfg_by_container_fd(container_fd); > + } else { > + /* when no fd provided use the default. */ > + vfio_cfg = &vfio_cfgs[0]; > + } > if (vfio_cfg == NULL) { > RTE_LOG(ERR, EAL, "Invalid container fd\n"); > return -1; > -- > 2.12.0 > -- Gaëtan Rivet 6WIND