On Tue, 12 May 2026 18:51:40 +0100 Matt Evans <[email protected]> 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
