From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet Subject: Re: [PATCH 1/6] vfio: allow DMA map of memory for the default vfio fd Date: Wed, 13 Feb 2019 12:38:19 +0100 Message-ID: <20190213113819.xkaybql2eeqmflpf@bidouze.vm.6wind.com> References: <0f98e0a06c535d58ac8d2c39645b07eb966ec5ec.1550048187.git.shahafs@mellanox.com> <20190213094505.d7ly2sundvmffiny@bidouze.vm.6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Cc: anatoly.burakov@intel.com, yskoh@mellanox.com, thomas@monjalon.net, ferruh.yigit@intel.com, nhorman@tuxdriver.com, dev@dpdk.org To: Shahaf Shuler Return-path: Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by dpdk.org (Postfix) with ESMTP id B67CB5592 for ; Wed, 13 Feb 2019 12:38:41 +0100 (CET) Received: by mail-wr1-f66.google.com with SMTP id w17so2063035wrn.12 for ; Wed, 13 Feb 2019 03:38:41 -0800 (PST) Content-Disposition: inline In-Reply-To: <20190213094505.d7ly2sundvmffiny@bidouze.vm.6wind.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Feb 13, 2019 at 10:45:05AM +0100, Gaëtan Rivet wrote: > 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 > > --- > > 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); > Errm, copy error, this line should be vfio_cfg = get_vfio_cfg_by_container_fd(container_fd); of course. > > 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 -- Gaëtan Rivet 6WIND