Re: [PATCH 3/5] x86/mm: Introduce and export interface arch_clean_nonsnoop_dma()

[Yan Zhao]

On Sat, Jun 01, 2024 at 04:46:14PM -0300, Jason Gunthorpe wrote:
> On Mon, May 27, 2024 at 11:37:34PM -0700, Christoph Hellwig wrote:
> > On Tue, May 21, 2024 at 01:00:16PM -0300, Jason Gunthorpe wrote:
> > > > > Err, no.  There should really be no exported cache manipulation macros,
> > > > > as drivers are almost guaranteed to get this wrong.  I've added
> > > > > Russell to the Cc list who has been extremtly vocal about this at least
> > > > > for arm.
> > > > 
> > > > We could possibly move this under some IOMMU core API (ie flush and
> > > > map, unmap and flush), the iommu APIs are non-modular so this could
> > > > avoid the exported symbol.
> > > 
> > > Though this would be pretty difficult for unmap as we don't have the
> > > pfns in the core code to flush. I don't think we have alot of good
> > > options but to make iommufd & VFIO handle this directly as they have
> > > the list of pages to flush on the unmap side. Use a namespace?
> > 
> > Just have a unmap version that also takes a list of PFNs that you'd
> > need for non-coherent mappings?
> 
> VFIO has never supported that so nothing like that exists yet.. This
> is sort of the first steps to some very basic support for a
> non-coherent cache flush in a limited case of a VM that can do its own
> cache flushing through kvm.
> 
> The pfn list is needed for unpin_user_pages() and it has an ugly
> design where vfio/iommufd read back the pfns seperately from unmap,
> and they both do it differently without a common range list
> datastructure here.
> 
> So, we'd need to build some new unmap function that returns a pfn list
> that it internally fetches via the read ops. Then it can do the read,
> unmap, flush iotlb, flush cache in core code.
Would the core code flush CPU caches by providing page physical address?
If yes, do you think it's still necessary to export arch_flush_cache_phys()
(as what's implemented in this patch)?

> 
> I've been working towards this very slowly as I want to push this
> stuff down into the io page table walk and remove the significant
> inefficiency, so it is not throw away work, but it is certainly some
> notable amount of work to do.
Will VFIO also be switched to this new unmap interface? Do we need to care
about backporting?

And is it possible for VFIO alone to implement in the current proposed way
in this series as the first step for easier backport?