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

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

On 4/4/2023 8:43 PM, Burakov, Anatoly wrote:
> Caution: This message originated from an External Source. Use proper 
> caution when opening attachments, clicking links, or responding.
> 
> 
> On 2/7/2023 8:56 AM, Gupta, Nipun wrote:
>> [AMD Official Use Only - General]
>>
>> Hi David,
>>
>> I agree that change is not straightforward to review, but it should 
>> not cause any functional issue as we are still creating all the memory 
>> mappings, but one by one for each segment.
>> For hot plug case this causes issue as mentioned, that VFIO does not 
>> allow unmap of the individual segments in case mapping was created of 
>> a single coalesced segment.
>>
>> But yes, I am not sure why this code was added, which Anatoly may have 
>> more understanding on.
> 
> The motivation behind this code was that Linux allows limited amount of
> page mappings, so we were trying to save on those. However, since then
> there have been a few changes related to partial unmaps that may make it
> so that this code is not only no longer necessary, but is in fact
> actively harmful. I agree that this at least warrants a second look.
> 
>>
>> Anatoly,
>>
>> Can you please provide your feedback on this change?
> 
> The patch probably shouldn't include the mailmap changes :)

Sure, will send a separate patch for it.

> 
> Could you please provide some steps to reproduce the hotplug issue
> you're having? It would be great to have a test case for this patchset
> to put it in context.

I am working on CDX bus 
(http://patchwork.dpdk.org/project/dpdk/patch/20230124140746.594066-2-nipun.gupta@amd.com/) 
and trying out some cases for plug/unplug.

The test is as follows:
   # Run testpmd application
   ./dpdk-testpmd -c 0x3 -- -i --nb-cores=1

   # Bind to VFIO
   echo "vfio-cdx" >  /sys/bus/cdx/devices/cdx-00\:00/driver_override
   echo "cdx-00:00" > /sys/bus/cdx/drivers_probe

   # Plug a device
   testpmd> port attach cdx:cdx-00:00

   #quit testpmd
   testpmd> quit

This gave error at testpmd exit that memory cannot be freed. On 
debugging I updated this code and seems it should be seen with any of 
the device.

I see similar test case (without quit) mentioned 
https://doc.dpdk.org/dts/test_plans/hotplug_test_plan.html, but the 
difference is that it is with igb_uio and issue is being observed with VFIO.

Please note the device/bus mentioned in the commands is not yet 
upstreamed in DPDK, but patches would be sent out soon.

Thanks,
Nipun

> 
> -- 
> Thanks,
> Anatoly
>