On Wed, Feb 13, 2019 at 11:10:22AM +0200, 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 first implementation returns EBUSY
> and since kernel 3.11 returns EEXISTS. For spapr mapping EBUSY is returned
> since kernel 4.10.
>
What is the earliest version supported by DPDK? I thought 3.10 was
dropped, should we care about the 3.11 return value?
> Signed-off-by: Shahaf Shuler <shah...@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 48ca9465d4..2a2d655b37 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 != -EEXIST) && (ret != -EBUSY)) {
Won't a ret == 0 trigger the error then?
It seems ifdef about linux versions are not common in vfio code, but
bar that I think it would be cleaner to restrict the acceptable
error to it.
When a version will be dropped it will be much easier to remove the
related code instead of digging in the commit logs, and leaving both
would not be clean.
> 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 != -EBUSY) {
> RTE_LOG(ERR, EAL, " cannot set up DMA remapping, error
> %i (%s)\n",
> errno, strerror(errno));
> return -1;
> --
> 2.12.0
>
--
Gaëtan Rivet
6WIND
