Hi Alex,
On 11/05/2026 21:09, Alex Williamson wrote:
On Mon, 11 May 2026 16:30:39 +0100
Matt Evans <[email protected]> wrote:
Hi Alex, Leon,
On 27/04/2026 15:36, Alex Williamson wrote:
On Sun, 26 Apr 2026 13:52:15 +0300
Leon Romanovsky <[email protected]> wrote:
On Fri, Apr 24, 2026 at 03:31:53PM -0300, Jason Gunthorpe wrote:
On Thu, Apr 16, 2026 at 06:17:52AM -0700, Matt Evans wrote:
A new field is reserved in vfio_device_feature_dma_buf.flags to
request CPU-facing memory type attributes for mmap()s of the buffer.
Add a flag VFIO_DEVICE_FEATURE_DMA_BUF_ATTR_WC, which results in WC
PTEs for the DMABUF's BAR region.
Signed-off-by: Matt Evans <[email protected]>
---
drivers/vfio/pci/vfio_pci_dmabuf.c | 15 +++++++++++++--
drivers/vfio/pci/vfio_pci_priv.h | 1 +
include/uapi/linux/vfio.h | 12 +++++++++---
3 files changed, 23 insertions(+), 5 deletions(-)
Nice and simple
Reviewed-by: Jason Gunthorpe <[email protected]>
@@ -1549,8 +1551,12 @@ struct vfio_region_dma_range {
struct vfio_device_feature_dma_buf {
__u32 region_index;
__u32 open_flags;
- __u32 flags;
- __u32 nr_ranges;
+ __u32 flags;
+ /* Flags sub-field reserved for attribute enum */
+#define VFIO_DEVICE_FEATURE_DMA_BUF_ATTR_MASK (0xfU << 28)
+#define VFIO_DEVICE_FEATURE_DMA_BUF_ATTR_UC (0 << 28)
+#define VFIO_DEVICE_FEATURE_DMA_BUF_ATTR_WC (1 << 28)
+ __u32 nr_ranges;
Alex,
The TPH proposal extends the flags field in a similar way, but I suggested
a different approach to conserve bits. At the moment, we spend three bits
on a single feature, which feels wasteful.
What do you think?
https://lore.kernel.org/all/20260409120415.GF86584@unreal/
I already proposed a very different interface for TPH that decouples
the dma-buf creation from setting the TPH values:
https://lore.kernel.org/all/[email protected]/
This is overall less intrusive than the TPH change proposed, but it
could still make sense to align this as an operation on the dma-buf,
that can be probed as a separate feature. Thanks,
I'll add a VFIO_DEVICE_FEATURE_DMA_BUF_ATTRS in a v2 instead to get in
line with the TPH work, no worries.
For the benefit of future hackers, how would you describe the criteria
for adding flags to this existing field? What hypothetical feature
characteristics would be appropriate? (Maybe it's that these attrs &
TPH add scalar fields in several bits rather than a simple boolean.)
Two of us have independently added something that's turned out to be
inapproriate so some guidance would be good.
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.)
Cheers,
Matt
