Re: [PATCH 9/9] vfio/pci: Add mmap() attributes to DMABUF feature

[Alex Williamson <alex@shazbot.org>]

On Tue, 12 May 2026 18:51:40 +0100
Matt Evans <mattev@meta.com> wrote:
> On 11/05/2026 21:09, Alex Williamson wrote:
> > I think the question of how we actually expand an arbitrary grab bag of
> > "ATTRS" is the central question in whether we should implement the
> > interface.   
> 
> > If we follow the direction I suggested for TPH, maybe this
> > is just a VFIO_DEVICE_FEATURE_DMA_BUF_WC, where it supports only PROBE
> > and SET, with SET taking only the dma-buf fd to implement the one-way
> > promotion from UC -> WC.
> > 
> > If we support a generic SET ATTRS feature, we really need to map out how
> > flag bits are indicated as supported and how a user untangles failures
> > from trying to set various attributes.  If we end up with a feature
> > indicating each ATTR is available, we might as well have just
> > implemented a feature for each attribute.  Thanks,  
> 
> Agreed, that's key.  Alhough, the aim of this patch is for attrs to be a 
> memory type enum rather than a bag of possibly-concurrent and 
> possibly-conflicting boolean flags.  Maybe 'memory attributes' would be 
> a better feature name.
> 
> I'm not sure about the feature-per-attribute.  Say we do a 
> VFIO_DEVICE_FEATURE_DMA_BUF_WC and then later support a second, 
> VFIO_DEVICE_FEATURE_DMA_BUF_UC_WEAK (like, say, Arm Device-nGRE).  Then 
> we have to specify that these two VFIO feature types actually 
> interact/override somehow.  I doubt we'll end up with a dozen but it's a 
> bit tiresome having a few features that interact.
> 
> At least if it's a single DMA_BUF_MEMATTR feature taking an enum, we 
> just encode the N different (mutually-exclusive!) valid states and done. 
>   I don't feel having a new feature for each keeps things simpler.
> 
> Discovery of support for a specific future attribute is OK with a single 
> ATTR too; we can take an enum attribute argument to a GET and -ENOTSUPP 
> for any we don't like.
> 
> (We could also add orthogonal DMABUF flags (can't think of a good 
> example...) but I'd suggest _those_ as semantically-grouped different 
> features, with the same issues of specifying conflicting cases versus 
> existing features.)

I think the GET behavior you're proposing is a bit counter-intuitive, if
not abusive of the interface, but I do agree that if the feature is
SET'ing a single value and not a group of independent flags, that we
can probably rely more on a try-and-fail model rather than advertising
each supported value as a separate feature.

For example, the user has some list of compatible attributes ordered
from most to least desirable, they try each in order until one works,
or none work and they decide whether that's ok.

For GET, if we implement it, I think it should report the current
attribute, mirroring SET.  We could almost get away without implementing
it, but I do worry about the case of nvgrace-gpu, where it might be
interesting for the user to see that the default attribute could be WB
rather than UC.

Where does the user derive the enum value?  Are we defining our own or
is it a system header defined enum?  I'm curious if/how we're going to
handle architecture specific attributes.  Thanks,

Alex