[AMD Official Use Only - General]
> -----Original Message----- > From: Burakov, Anatoly <anatoly.bura...@intel.com> > Sent: Wednesday, April 5, 2023 7:46 PM > To: Gupta, Nipun <nipun.gu...@amd.com>; David Marchand > <david.march...@redhat.com> > Cc: dev@dpdk.org; tho...@monjalon.net; Yigit, Ferruh > <ferruh.yi...@amd.com>; Agarwal, Nikhil <nikhil.agar...@amd.com> > Subject: Re: [PATCH v2] vfio: do not coalesce DMA mappings > > Caution: This message originated from an External Source. Use proper caution > when opening attachments, clicking links, or responding. > > > On 4/4/2023 5:32 PM, Nipun Gupta wrote: > > > > > > 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.gu...@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, I can reproduce this issue with regular devices too (run testpmd > with no devices, bind a NIC to VFIO, attach it, then quit). You're > correct in that since the initial mapping was done with mapping large > contiguous zones (such as when mempools are created before attach), any > subsequent freeing of memory will cause these errors to happen. Thanks for trying on other devices too, and good to know that this is also reproduced with them :) Regards, Nipun > > I don't think this can be fixed by anything other than not doing the > contiguous mapping thing, so provisionally, I think this patch should be > accepted. I'll play around with it some more and get back to you :) > > >> > >> -- > >> Thanks, > >> Anatoly > >> > > -- > Thanks, > Anatoly
