[dpdk-dev] [PATCH v2 2/6] vfio: don't fail to DMA map if memory is already mapped

[Burakov, Anatoly]

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 <shahafs at mellanox.com>
> ---
>   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 <anatoly.burakov at intel.com>

-- 
Thanks,
Anatoly