Re: [PATCH rfc] vfio-pci: Allow write combining

[Alex Williamson <alex.williamson@redhat.com>]

On Thu, 1 Aug 2024 14:13:55 -0300
Jason Gunthorpe <jgg@ziepe.ca> wrote:

> On Thu, Aug 01, 2024 at 10:52:18AM -0600, Alex Williamson wrote:
> > > > vfio_region_info.flags in not currently tested for input therefore this
> > > > proposal could lead to unexpected behavior for a caller that doesn't
> > > > currently zero this field.  It's intended as an output-only field.    
> > > 
> > > Perhaps a REGION_INFO2 then?
> > > 
> > > I still think per-request is better than a global flag  
> > 
> > I don't understand why we'd need a REGION_INFO2, we already have
> > support for defining new regions.  
> 
> It is not a new region, it is a modified mmap behavior for an existing
> region.

If we're returning a different offset into the vfio device file from
which to get a WC mapping, what's the difference?  A vfio "region" is
describing a region or range of the vfio device file descriptor.
Region indexes that map into the same device resource are not
fundamentally incompatible AFAICT, but it does mean that zapping user
access is not a nice contiguous single range.
 
> > We'd populate these new regions only for BARs that support prefetch and
> > mmap   
> 
> That's not the point, prefetch has nothing to do with write combining.

I was following the original proposal in this thread that added a
prefetch flag to REGION_INFO and allowed enabling WC only for
IORESOURCE_PREFETCH.

> Every BAR can be mapped writecombining, it is up to the VFIO userspace
> to understand if it can use it or not. The only use case for this
> feature would be something like in DPDK.
> 
> VM side write combining is already handled by KVM allowing the VM to
> upgrade the page attributes to WC from NC.
> 
> Doubling all the region indexes just for WC does not seem like a good
> idea to me...

Is the difference you see that in the REQ_WC proposal the user is
effectively asking vfio to pop a WC region into existence vs here
they're pre-populated?  At the limit they're the same.  We could use a
DEVICE_FEATURE to ask vfio to selectively populate WC regions after
which the user could re-enumerate additional regions, or in fact to
switch on WC for a given region if we want to go that route.  Thanks,

Alex