On Tue, Jun 16, 2026 at 12:37:29PM +0100, Matt Evans wrote:
Hi Praan,
On 16/06/2026 09:47, Pranjal Shrivastava wrote:
On Wed, Jun 10, 2026 at 04:43:23PM +0100, Matt Evans wrote:
A new VFIO feature, VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR, is added to set CPU-facing memory type attributes for a DMABUF exported from vfio-pci. These are used for subsequent mmap()s of the buffer.
There are two attributes supported:
- The default, VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_NC
- VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_WC, which results in WC PTEs for the DMABUF's BAR region.
Signed-off-by: Matt Evans matt@ozlabs.org
drivers/vfio/pci/vfio_pci_core.c | 2 ++ drivers/vfio/pci/vfio_pci_dmabuf.c | 57 +++++++++++++++++++++++++++++- drivers/vfio/pci/vfio_pci_priv.h | 14 ++++++++ include/uapi/linux/vfio.h | 27 ++++++++++++++ 4 files changed, 99 insertions(+), 1 deletion(-)
[...]
- /* Verify DMABUF: see comments in vfio_pci_dma_buf_revoke() */
- priv = dmabuf->priv;
- if (dmabuf->ops != &vfio_pci_dmabuf_ops ||
READ_ONCE(priv->vdev) != vdev) {ret = -ENODEV;goto out_put_buf;- }
- switch (db_attr.memattr) {
- case VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_NC:
- case VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_WC:
WRITE_ONCE(priv->memattr, db_attr.memattr);ret = 0;break;- default:
ret = -ENOENT;Nit: Looks like the agreement [1] was on -EOPNOTSUPP / -EINVAL but we took -ENOENT here and in the doc string? Was that intentional?
I tend to agree with Alex's suggestion here, we'd prefer one of those two (-EINVAL / -EOPNOTSUPP) since it clearly communicates to the user that "You sent a wrong arg" or "We don't support this"
Yes, it was intentional. This was noted in the v3 changelog entry in the cover letter:
- Removed GET on vfio_pci_core_feature_dma_buf_memattr(), removed unnecessary taking of memory_lock, fixed error return values. In particular, removes ENOTSUPP, and uses ENOENT to indicate an unknown attribute enum value was passed to SET. In the discussion here, https://lore.kernel.org/all/20260602131417.41366391@shazbot.org/ we'd agreed on EOPNOTSUPP before I realised that's already used elsewhere. ENOENT uniquely indicates an unknown attribute.
Ahh okay. I missed the changelogs in the cover letter.
EINVAL/EOPNOTSUPP would indeed be semantically perfect, but after posting my reply there I remembered they are already overloaded with a load of different meanings.
I think uniqueness is important here so that memattr issues (for example any future arch-specific porting issues) show up as an immediately-understandable error value.
-ENOENT means no such file or directory [2] to the user. Users may not be kernel engineers who'd wanna peek into the code and they may simply look at the uAPI files which doesn't give them an answer as to what went wrong.
But surely when they look at the uAPI header they will then see "* ENOENT: The given memattr is not supported." and understand what went wrong.
Fair enough. Since its documented it clearly in the uAPI header.
Thanks, Praan
linaro-mm-sig@lists.linaro.org