Re: [PATCH] vfio: do not coalesce DMA mappings

[Nipun Gupta <nipun.gupta@amd.com>]

Hi Xuan,

Thanks for pointing out the issue and figuring out the patch which 
introduced this. If you have answers to below queries, please let me know:

Is there any other test cases which tests "--no-huge" which pass?

Also, if we change the "-m" option to provide lower memory, does the 
test pass?

When you mention too many pages exceed the capability of IOMMU, you are 
referring to HW capability to create multiple pages? Here it seems in 
case of 4K page size we need 256K pages which is limiting the capacity?

Regards,
Nipun

On 6/29/2023 1:51 PM, Ding, Xuan wrote:
> Hi Nipun,
> 
> I'd like to appreciate your time reading this email.
> 
> Our QA team found that since this commit "a399d7b5a994: do not coalesce DMA mappings" is introduced,
> the dpdk testpmd start with "--no-huge" parameters will failed, and shows "EAL: Cannot set up DMA remapping, error 28 (No space left on device)".
> So they reported it on dpdk Bugzilla: https://bugs.dpdk.org/show_bug.cgi?id=1235.
> 
> I understand this feature is to keep consistent with the kernel and not allow memory segments be merged.
> The side effect is the testpmd with "--no-huge" parameters will not be able to start because the too many pages will exceed the capability of IOMMU.
> Is it expected? Should we remove the --no-huge" in our testcase?
> 
> Regards,
> Xuan
> 
>> -----Original Message-----
>> From: Nipun Gupta <nipun.gupta@amd.com>
>> Sent: Friday, December 30, 2022 5:59 PM
>> To: dev@dpdk.org; thomas@monjalon.net; Burakov, Anatoly
>> <anatoly.burakov@intel.com>; ferruh.yigit@amd.com
>> Cc: nikhil.agarwal@amd.com; Nipun Gupta <nipun.gupta@amd.com>
>> Subject: [PATCH] vfio: do not coalesce DMA mappings
>>
>> At the cleanup time when dma unmap is done, linux kernel does not allow
>> unmap of individual segments which were coalesced together while creating
>> the DMA map for type1 IOMMU mappings. So, this change updates the
>> mapping of the memory
>> segments(hugepages) on a per-page basis.
>>
>> Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
>> ---
>>
>> When hotplug of devices is used, multiple pages gets colaeced and a single
>> mapping gets created for these pages (using APIs
>> rte_memseg_contig_walk() and type1_map_contig(). On the cleanup time
>> when the memory is released, the VFIO does not cleans up that memory and
>> following error is observed in the eal for 2MB
>> hugepages:
>> EAL: Unexpected size 0 of DMA remapping cleared instead of 2097152
>>
>> This is because VFIO does not clear the DMA (refer API
>> vfio_dma_do_unmap() -
>> https://elixir.bootlin.com/linux/latest/source/drivers/vfio/vfio_iommu_type1.
>> c#L1330),
>> where it checks the dma mapping where it checks for IOVA to free:
>> https://elixir.bootlin.com/linux/latest/source/drivers/vfio/vfio_iommu_type1.
>> c#L1418.
>>
>> Thus this change updates the mapping to be created individually instead of
>> colaecing them.
>>
>>   lib/eal/linux/eal_vfio.c | 29 -----------------------------
>>   1 file changed, 29 deletions(-)
>>
>> diff --git a/lib/eal/linux/eal_vfio.c b/lib/eal/linux/eal_vfio.c index
>> 549b86ae1d..56edccb0db 100644
>> --- a/lib/eal/linux/eal_vfio.c
>> +++ b/lib/eal/linux/eal_vfio.c
>> @@ -1369,19 +1369,6 @@ rte_vfio_get_group_num(const char *sysfs_base,
>>   	return 1;
>>   }
>>
>> -static int
>> -type1_map_contig(const struct rte_memseg_list *msl, const struct
>> rte_memseg *ms,
>> -		size_t len, void *arg)
>> -{
>> -	int *vfio_container_fd = arg;
>> -
>> -	if (msl->external)
>> -		return 0;
>> -
>> -	return vfio_type1_dma_mem_map(*vfio_container_fd, ms->addr_64,
>> ms->iova,
>> -			len, 1);
>> -}
>> -
>>   static int
>>   type1_map(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
>>   		void *arg)
>> @@ -1396,10 +1383,6 @@ type1_map(const struct rte_memseg_list *msl,
>> const struct rte_memseg *ms,
>>   	if (ms->iova == RTE_BAD_IOVA)
>>   		return 0;
>>
>> -	/* if IOVA mode is VA, we've already mapped the internal segments */
>> -	if (!msl->external && rte_eal_iova_mode() == RTE_IOVA_VA)
>> -		return 0;
>> -
>>   	return vfio_type1_dma_mem_map(*vfio_container_fd, ms->addr_64,
>> ms->iova,
>>   			ms->len, 1);
>>   }
>> @@ -1464,18 +1447,6 @@ vfio_type1_dma_mem_map(int vfio_container_fd,
>> uint64_t vaddr, uint64_t iova,  static int  vfio_type1_dma_map(int
>> vfio_container_fd)  {
>> -	if (rte_eal_iova_mode() == RTE_IOVA_VA) {
>> -		/* with IOVA as VA mode, we can get away with mapping
>> contiguous
>> -		 * chunks rather than going page-by-page.
>> -		 */
>> -		int ret = rte_memseg_contig_walk(type1_map_contig,
>> -				&vfio_container_fd);
>> -		if (ret)
>> -			return ret;
>> -		/* we have to continue the walk because we've skipped the
>> -		 * external segments during the config walk.
>> -		 */
>> -	}
>>   	return rte_memseg_walk(type1_map, &vfio_container_fd);  }
>>
>> --
>> 2.25.1
>