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

Message ID 245e5f9449559d200f01ea034331ed7ba036971f.1550760029.git.shahafs@mellanox.com (mailing list archive)
State Superseded, archived
Headers
Series introduce DMA memory mapping for external memory |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Shahaf Shuler Feb. 21, 2019, 2:50 p.m. UTC
  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@mellanox.com>
---
 lib/librte_eal/linuxapp/eal/eal_vfio.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)
  

Comments

Anatoly Burakov Feb. 28, 2019, 11:58 a.m. UTC | #1
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@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@intel.com>
  

Patch

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;