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 10:45:05 +0100 Message-ID: <20190213094505.d7ly2sundvmffiny@bidouze.vm.6wind.com> References: <0f98e0a06c535d58ac8d2c39645b07eb966ec5ec.1550048187.git.shahafs@mellanox.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 943501B137 for ; Wed, 13 Feb 2019 10:45:29 +0100 (CET) Received: by mail-wr1-f66.google.com with SMTP id c8so1704637wrs.4 for ; Wed, 13 Feb 2019 01:45:29 -0800 (PST) Content-Disposition: inline In-Reply-To: <0f98e0a06c535d58ac8d2c39645b07eb966ec5ec.1550048187.git.shahafs@mellanox.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" 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); > 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