From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Burakov, Anatoly" Subject: Re: [PATCH v2 2/6] vfio: don't fail to DMA map if memory is already mapped Date: Thu, 28 Feb 2019 11:58:57 +0000 Message-ID: <8a9d0c64-1998-a37f-9c0b-e221b84a49ac@intel.com> References: <245e5f9449559d200f01ea034331ed7ba036971f.1550760029.git.shahafs@mellanox.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org To: Shahaf Shuler , yskoh@mellanox.com, thomas@monjalon.net, ferruh.yigit@intel.com, nhorman@tuxdriver.com, gaetan.rivet@6wind.com Return-path: Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 1DA7837AF for ; Thu, 28 Feb 2019 12:59:00 +0100 (CET) In-Reply-To: <245e5f9449559d200f01ea034331ed7ba036971f.1550760029.git.shahafs@mellanox.com> Content-Language: en-US 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 21-Feb-19 2:50 PM, Shahaf Shuler wrote: > Currently vfio DMA map function will fail in case the same memory > segment is mapped twice. > > This is too strict, as this is not an error to map the same memory > twice. > > Instead, use the kernel return value to detect such state and have the > DMA function to return as successful. > > For type1 mapping the kernel driver returns EEXISTS. > For spapr mapping EBUSY is returned since kernel 4.10. > > Signed-off-by: Shahaf Shuler > --- > lib/librte_eal/linuxapp/eal/eal_vfio.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c b/lib/librte_eal/linuxapp/eal/eal_vfio.c > index 9adbda8bb7..9e8ad399f5 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c > +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c > @@ -1263,7 +1263,11 @@ vfio_type1_dma_mem_map(int vfio_container_fd, uint64_t vaddr, uint64_t iova, > VFIO_DMA_MAP_FLAG_WRITE; > > ret = ioctl(vfio_container_fd, VFIO_IOMMU_MAP_DMA, &dma_map); > - if (ret) { > + /** > + * In case the mapping was already done EEXIST will be > + * returned from kernel. > + */ > + if (ret && (errno != EEXIST)) { > RTE_LOG(ERR, EAL, " cannot set up DMA remapping, error %i (%s)\n", > errno, strerror(errno)); > return -1; > @@ -1324,7 +1328,11 @@ vfio_spapr_dma_do_map(int vfio_container_fd, uint64_t vaddr, uint64_t iova, > VFIO_DMA_MAP_FLAG_WRITE; > > ret = ioctl(vfio_container_fd, VFIO_IOMMU_MAP_DMA, &dma_map); > - if (ret) { > + /** > + * In case the mapping was already done EBUSY will be > + * returned from kernel. > + */ > + if (ret && (errno != EBUSY)) { > RTE_LOG(ERR, EAL, " cannot set up DMA remapping, error %i (%s)\n", > errno, strerror(errno)); > return -1; > Nitpicking, but maybe it would be good to throw a debug output about not attempting to map because the area is already mapped. Silently ignoring the error won't help with debugging, should any issues arise :) Otherwise LGTM Acked-by: Anatoly Burakov -- Thanks, Anatoly