Hi all,
This series is based on previous RFCs/discussions:
Tech topic: https://lore.kernel.org/linux-iommu/20250918214425.2677057-1-amastro@fb.com/ RFCv1: https://lore.kernel.org/all/20260226202211.929005-1-mattev@meta.com/ RFCv2: https://lore.kernel.org/kvm/20260312184613.3710705-1-mattev@meta.com/
The background/rationale is covered in more detail in the RFC cover letters. The TL;DR is:
The goal is to enable userspace driver designs that use VFIO to export DMABUFs representing subsets of PCI device BARs, and "vend" those buffers from a primary process to other subordinate processes by fd. These processes then mmap() the buffers and their access to the device is isolated to the exported ranges. This is an improvement on sharing the VFIO device fd to subordinate processes, which would allow unfettered access.
This is achieved by enabling mmap() of vfio-pci DMABUFs, passed by fd to subordinate processes. Second, a new ioctl()-based revocation mechanism is added to allow the primary process to forcibly revoke access to previously-shared BAR spans, even if the subordinate processes haven't cleanly exited.
(The related topic of safe delegation of iommufd control to the subordinate processes is not addressed here, and is follow-up work.)
As well as isolation and revocation, another advantage to accessing a BAR through a VMA backed by a DMABUF is that it's straightforward to mmap() the buffer with access attributes, such as write-combining.
Feedback from the RFCs requested that, instead of creating DMABUF-specific vm_ops and .fault paths, to go the whole way and migrate the existing VFIO PCI BAR mmap() to be backed by a DMABUF too, resulting in a common vm_ops and fault handler for mmap()s of both the VFIO device and explicitly-exported DMABUFs. This will help future iommufd emulation of VFIO Type1 peer-to-peer, making it easier to get a DMABUF for a VFIO BAR as a DMA target.
mmap() conversion to use DMABUF underneath has been done for vfio-pci, but not sub-drivers:
nvgrace-gpu's mmap() override path is unchanged; I kept this out of scope for now not least because I don't have a thorough test setup for this system. I would prefer to help the nvgrace-gpu maintainers enable BAR mmap() DMABUFs themselves.
Notes on patches ================
PCI/P2PDMA: Add CONFIG_PCI_P2PDMA_CORE
Later in the series, vfio-pci's mmap() is going to depend on pcim_p2pdma_provider() which depended on CONFIG_PCI_P2PDMA, which in turn depended on ZONE_DEVICE (which isn't available on 32-bit and some archs, because they lack MEMORY_HOTPLUG and friends). VFIO does _not_ require actual P2P to be present for basic mmap() functionality, only for the optional CONFIG_DMA_SHARED_BUFFER feature.
This splits P2PDMA into a CONFIG_PCI_P2PDMA_CORE (which currently contains pcim_p2pdma_provider()) and an optional CONFIG_PCI_P2PDMA (which depends on ZONE_DEVICE etc., and provides P2P functionality).
vfio/pci: Add a helper to look up PFNs for DMABUFs vfio/pci: Add a helper to create a DMABUF for a BAR-map VMA
The first is for a DMABUF VMA fault handler to determine arbitrary-sized PFNs from ranges in DMABUF. Secondly, refactor DMABUF export for use by the existing export feature and add a new helper that creates a DMABUF corresponding to a VFIO BAR mmap() request.
vfio/pci: Convert BAR mmap() to use a DMABUF
The vfio-pci core mmap() creates a DMABUF with the helper, and the vm_ops fault handler uses the other helper to resolve the fault. Because this depends on DMABUF structs/code, CONFIG_VFIO_PCI_CORE needs to depend on CONFIG_DMA_SHARED_BUFFER. The CONFIG_VFIO_PCI_DMABUF still conditionally enables the export support code.
NOTE: The user mmap()s a device fd, but the resulting VMA's vm_file becomes that of the DMABUF which takes ownership of the device and puts it on release. This maintains the existing behaviour of a VMA keeping the VFIO device open.
BAR zapping then happens via the existing vfio_pci_dma_buf_move() path, which now needs to unmap PTEs in the DMABUF's address_space.
vfio/pci: Provide a user-facing name for BAR mappings
There was a request for decent debug naming in /proc/<pid>/maps etc. comparable to the existing VFIO names: since the VMAs are DMABUFs, they have a "dmabuf:" prefix and can't be 100% identical to before. This is a user-visible change, but this patch at least now gives us extra info on the BDF & BAR being mapped.
vfio/pci: Clean up BAR zap and revocation
In general (see NOTE!) the vfio_pci_zap_bars() is now obsolete, since it unmaps PTEs in the VFIO device address_space which is now unused. This consolidates all calls (e.g. around reset) with the neighbouring vfio_pci_dma_buf_move()s into new functions, to revoke-zap/unrevoke.
!!! NOTE: the nvgrace-gpu driver continues to use its own private vm_ops, fault handler, etc. for its special memregions, and these DO still add PTEs to the VFIO device address_space. So, a temporary flag, vdev->bar_needs_zap, maintains the old behaviour for this use. At least this patch's consolidation makes it easy to remove the remaining zap when this need goes away; a FIXME reminds that this can be removed when nvgrace-gpu is converted.
vfio/pci: Support mmap() of a VFIO DMABUF
Adds mmap() for a DMABUF fd exported from vfio-pci.
It was a goal to keep the VFIO device fd lifetime behaviour unchanged with respect to the DMABUFs. An application can close all device fds, and this will revoke/clean up all DMABUFs; no mappings or other access can be performed now. When enabling mmap() of the DMABUFs, this means access through the VMA is also revoked. This complicates the fault handler because whilst the DMABUF exists, it has no guarantee that the corresponding VFIO device is still alive. Adds synchronisation ensuring the vdev is available before vdev->memory_lock is touched; this holds the device registration so that even if the buffer has been cleaned up, vdev hasn't been freed and so the lock can be safely taken.
(I decided against the alternative of preventing cleanup by holding the VFIO device open if any DMABUFs exist, because it's both a change of behaviour and less clean overall.)
I've added a chonky comment in place, happy to clarify more if you have ideas.
This commit makes VFIO_PCI_CORE depend on PCI_P2PDMA_CORE (commit 1) to bring in (only) the P2PDMA provider code.
vfio/pci: Permanently revoke a DMABUF on request
By weight, this is mostly a rename of revoked to an enum, status. There are now 3 states for a buffer, usable and revoked temporary/permanent. A new VFIO device ioctl is added, VFIO_DEVICE_PCI_DMABUF_REVOKE, which passes a DMABUF (exported from that device) and permanently revokes it. Thus a userspace driver can guarantee any downstream consumers of a shared fd are prevented from accessing a BAR range, and that range can be reused.
The code doing revocation in vfio_pci_dma_buf_move() is moved, unchanged, to a common function for use by _move() and the new ioctl path.
Q: I can't think of a good reason to temporarily revoke/unrevoke buffers from userspace, so didn't add a 'flags' field to the ioctl struct. Easy to add if people think it's worthwhile for future use.
vfio/pci: Add mmap() attributes to DMABUF feature
Adds a new VFIO feature, VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR. After a DMABUF is exported, this feature ioctl() isused to set a memory attribute that will be used by future mmap()s of the DMABUF fd (i.e. it does nothing for any existing maps).
The default is UC, and via the feature one can specify CPU access as WC. The attribute is an enum/scalar rather than bitmap/cumulative. The attributes follow a "try-fail" model where a client can request an attribute and either succeed or fail with ENOTSUPP if it's unknown; if future attributes are platform-specific then their support can be probed.
(Since it's just UC/WC for now, there is no reservation or numeric structure to the namespace yet, but we could support system/arch-specific values in future by carving out base + arch-specific + IMPDEF ranges.)
Testing =======
(The [RFC ONLY] userspace test program, for QEMU edu-plus, has been dropped from the series, but can be found in the GitHub branch below. It at least illustrates the export, map, revoke, attribute, and close semantics interoperate.)
This code has been tested in mapping DMABUFs of single/multiple ranges, aliasing mmap()s, aliasing ranges across DMABUFs, vm_pgoff > 0, revocation, shutdown/cleanup scenarios, and hugepage mappings seem to work correctly. I've lightly tested WC mappings also (by observing resulting PTEs as having the correct attributes...). No regressions observed on the VFIO selftests, or on our internal vfio-pci applications.
End ===
This is based on VFIO next (e.g. at b9285405c5f6).
These commits are on GitHub for easier browsing, along with "[RFC ONLY] selftests: vfio: Add standalone vfio_dmabuf_mmap_test":
https://github.com/metamev/linux/compare/b9285405c5f6...metamev:linux:dev/me...
Thanks for reading,
Matt
================================================================================ Change log:
v2:
- Rebase on VFIO next, picking up Alex's vfio_pci_dma_buf_move()/vfio_pci_dma_buf_cleanup() fixes, and dropping "vfio/pci: Fix vfio_pci_dma_buf_cleanup() double-put"
- Added "PCI/P2PDMA: Add CONFIG_PCI_P2PDMA_CORE" so that the newly-added vfio-pci hard dependency on the P2PDMA provider instead pulls in the _CORE variant and not the full-fat CONFIG_PCI_P2PDMA. This means that the core of vfio-pci does not need ZONE_DEVICE, but if it's available then enabling P2PDMA in turn enables DMABUF export. Fixes basic VFIO operation on 32b or other platforms without ZONE_DEVICE.
- Fixed comment inaccuracy in vfio_pci_dma_buf_revoke() and cleaned up vdev validity test.
- vfio_pci_dma_buf_find_pfn(): use PAGE_ALIGN(), better span variable naming, OVF check
- Made vm_pgoffs use consistent (keeping the resource index at the top and masking where offset is used). For BAR mmap, use new vma_pgoff_adjust to create the DMABUF with the exact mmap()ed span instead of from the start of the BAR with an invisible portion before the mapping.
- Added VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR to set memory attributes, instead of using the export `flags` field.
- vfio_pci_ioctl_reset: Moved vfio_pci_zap_revoke_bars() (effectively, vfio_pci_dma_buf_move()) back after D0 transition. Note, if a BAR zap is needed, it's done in this function so now happens after this D0 transition with the _move; it was done before it at the time of the memory_lock taking.
- Minimised vfio_pci_dma_buf_mmap() (removed redundant span check), added READ_ONCE for memattr
- Misc fixes: comment in DMABUF name generation, removed superfluous READ_ONCE from faulthandler
v1: https://lore.kernel.org/kvm/20260416131815.2729131-1-mattev@meta.com/
- Cleanup of the common DMABUF-aware VMA vm_ops fault handler and export code. - Fixed a lot of races, particularly faults racing with DMABUF cleanup (if the VFIO device fds close, for example). - Added nicer human-readable names for VFIO mmap() VMAs
RFCv2: Respin based on the feedback/suggestions: https://lore.kernel.org/kvm/20260312184613.3710705-1-mattev@meta.com/
- Transform the existing VFIO BAR mmap path to also use DMABUFs behind the scenes, and then simply share that code for explicitly-mapped DMABUFs. Jason wanted to go that direction to enable iommufd VFIO type 1 emulation to pick up a DMABUF for an IO mapping.
- Revoke buffers using a VFIO device fd ioctl
RFCv1: https://lore.kernel.org/all/20260226202211.929005-1-mattev@meta.com/
Matt Evans (9): PCI/P2PDMA: Add CONFIG_PCI_P2PDMA_CORE vfio/pci: Add a helper to look up PFNs for DMABUFs vfio/pci: Add a helper to create a DMABUF for a BAR-map VMA vfio/pci: Convert BAR mmap() to use a DMABUF vfio/pci: Provide a user-facing name for BAR mappings vfio/pci: Clean up BAR zap and revocation vfio/pci: Support mmap() of a VFIO DMABUF vfio/pci: Permanently revoke a DMABUF on request vfio/pci: Add mmap() attributes to DMABUF feature
drivers/pci/Kconfig | 10 +- drivers/pci/Makefile | 2 +- drivers/pci/p2pdma.c | 16 + drivers/vfio/pci/Kconfig | 4 +- drivers/vfio/pci/Makefile | 3 +- drivers/vfio/pci/nvgrace-gpu/main.c | 5 + drivers/vfio/pci/vfio_pci_config.c | 30 +- drivers/vfio/pci/vfio_pci_core.c | 225 +++++++++--- drivers/vfio/pci/vfio_pci_dmabuf.c | 548 ++++++++++++++++++++++++---- drivers/vfio/pci/vfio_pci_priv.h | 57 ++- include/linux/pci-p2pdma.h | 24 +- include/linux/pci.h | 2 +- include/linux/vfio_pci_core.h | 1 + include/uapi/linux/vfio.h | 57 +++ 14 files changed, 815 insertions(+), 169 deletions(-)
The P2PDMA code currently provides two features under the same CONFIG_PCI_P2PDMA option:
1. Locate providers via pcim_p2pdma_provider() 2. Manage actual P2P DMA
Other code (such as vfio-pci) depends on 1, without having a hard dependency on 2.
A future commit expands the use of DMABUF in vfio-pci for non-P2P scenarios, relying on pcim_p2pdma_provider() always being present. If that depended on CONFIG_PCI_P2PDMA, it would make vfio-pci only available if CONFIG_ZONE_DEVICE is present (e.g. 64-bit systems), even when P2P is not needed.
To resolve this, introduce CONFIG_PCI_P2PDMA_CORE which contains the basic provider functionality to make it available even if the CONFIG_PCI_P2PDMA feature is disabled or unavailable due to !CONFIG_ZONE_DEVICE. Users such as vfio-pci can enable their own P2P features based off the original CONFIG_PCI_P2PDMA (available when CONFIG_ZONE_DEVICE is set).
Signed-off-by: Matt Evans mattev@meta.com --- drivers/pci/Kconfig | 10 +++++----- drivers/pci/Makefile | 2 +- drivers/pci/p2pdma.c | 16 ++++++++++++++++ include/linux/pci-p2pdma.h | 24 ++++++++++++++---------- include/linux/pci.h | 2 +- 5 files changed, 37 insertions(+), 17 deletions(-)
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 33c88432b728..59d70bc84cc9 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -206,11 +206,7 @@ config PCIE_TPH config PCI_P2PDMA bool "PCI peer-to-peer transfer support" depends on ZONE_DEVICE - # - # The need for the scatterlist DMA bus address flag means PCI P2PDMA - # requires 64bit - # - depends on 64BIT + select PCI_P2PDMA_CORE select GENERIC_ALLOCATOR select NEED_SG_DMA_FLAGS help @@ -226,6 +222,10 @@ config PCI_P2PDMA
If unsure, say N.
+config PCI_P2PDMA_CORE + default n + bool + config PCI_LABEL def_bool y if (DMI || ACPI) select NLS diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 41ebc3b9a518..419b646a301d 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -30,7 +30,7 @@ obj-$(CONFIG_PCI_SYSCALL) += syscall.o obj-$(CONFIG_PCI_STUB) += pci-stub.o obj-$(CONFIG_PCI_PF_STUB) += pci-pf-stub.o obj-$(CONFIG_PCI_ECAM) += ecam.o -obj-$(CONFIG_PCI_P2PDMA) += p2pdma.o +obj-$(CONFIG_PCI_P2PDMA_CORE) += p2pdma.o obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o obj-$(CONFIG_VGA_ARB) += vgaarb.o obj-$(CONFIG_PCI_DOE) += doe.o diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c index 7c898542af8d..619d46c652b8 100644 --- a/drivers/pci/p2pdma.c +++ b/drivers/pci/p2pdma.c @@ -28,6 +28,14 @@ struct pci_p2pdma { struct p2pdma_provider mem[PCI_STD_NUM_BARS]; };
+/* + * CONFIG_PCI_P2PDMA_CORE provides just a bare-bones init and + * pcim_p2pdma_provider() interface (used by things like VFIO even if + * full P2PDMA isn't present). The full P2PDMA feature is under the + * CONFIG_PCI_P2PDMA option. + */ +#ifdef CONFIG_PCI_P2PDMA + struct pci_p2pdma_pagemap { struct dev_pagemap pgmap; struct p2pdma_provider *mem; @@ -226,6 +234,8 @@ static const struct dev_pagemap_ops p2pdma_pgmap_ops = { .folio_free = p2pdma_folio_free, };
+#endif /* CONFIG_PCI_P2PDMA */ + static void pci_p2pdma_release(void *data) { struct pci_dev *pdev = data; @@ -241,11 +251,13 @@ static void pci_p2pdma_release(void *data) synchronize_rcu(); xa_destroy(&p2pdma->map_types);
+#ifdef CONFIG_PCI_P2PDMA if (!p2pdma->pool) return;
gen_pool_destroy(p2pdma->pool); sysfs_remove_group(&pdev->dev.kobj, &p2pmem_group); +#endif }
/** @@ -330,6 +342,8 @@ struct p2pdma_provider *pcim_p2pdma_provider(struct pci_dev *pdev, int bar) } EXPORT_SYMBOL_GPL(pcim_p2pdma_provider);
+#ifdef CONFIG_PCI_P2PDMA + static int pci_p2pdma_setup_pool(struct pci_dev *pdev) { struct pci_p2pdma *p2pdma; @@ -1207,3 +1221,5 @@ ssize_t pci_p2pdma_enable_show(char *page, struct pci_dev *p2p_dev, return sprintf(page, "%s\n", pci_name(p2p_dev)); } EXPORT_SYMBOL_GPL(pci_p2pdma_enable_show); + +#endif diff --git a/include/linux/pci-p2pdma.h b/include/linux/pci-p2pdma.h index 873de20a2247..4c42a7b2ee85 100644 --- a/include/linux/pci-p2pdma.h +++ b/include/linux/pci-p2pdma.h @@ -67,9 +67,22 @@ enum pci_p2pdma_map_type { PCI_P2PDMA_MAP_THRU_HOST_BRIDGE, };
-#ifdef CONFIG_PCI_P2PDMA +#ifdef CONFIG_PCI_P2PDMA_CORE int pcim_p2pdma_init(struct pci_dev *pdev); struct p2pdma_provider *pcim_p2pdma_provider(struct pci_dev *pdev, int bar); +#else +static inline int pcim_p2pdma_init(struct pci_dev *pdev) +{ + return -EOPNOTSUPP; +} +static inline struct p2pdma_provider *pcim_p2pdma_provider(struct pci_dev *pdev, + int bar) +{ + return NULL; +} +#endif + +#ifdef CONFIG_PCI_P2PDMA int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, size_t size, u64 offset); int pci_p2pdma_distance_many(struct pci_dev *provider, struct device **clients, @@ -89,15 +102,6 @@ ssize_t pci_p2pdma_enable_show(char *page, struct pci_dev *p2p_dev, enum pci_p2pdma_map_type pci_p2pdma_map_type(struct p2pdma_provider *provider, struct device *dev); #else /* CONFIG_PCI_P2PDMA */ -static inline int pcim_p2pdma_init(struct pci_dev *pdev) -{ - return -EOPNOTSUPP; -} -static inline struct p2pdma_provider *pcim_p2pdma_provider(struct pci_dev *pdev, - int bar) -{ - return NULL; -} static inline int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, size_t size, u64 offset) { diff --git a/include/linux/pci.h b/include/linux/pci.h index 2c4454583c11..531aec355686 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -557,7 +557,7 @@ struct pci_dev { u16 pasid_cap; /* PASID Capability offset */ u16 pasid_features; #endif -#ifdef CONFIG_PCI_P2PDMA +#ifdef CONFIG_PCI_P2PDMA_CORE struct pci_p2pdma __rcu *p2pdma; #endif #ifdef CONFIG_PCI_DOE
On 2026-05-27 04:23, Matt Evans wrote:
The P2PDMA code currently provides two features under the same CONFIG_PCI_P2PDMA option:
- Locate providers via pcim_p2pdma_provider()
- Manage actual P2P DMA
Other code (such as vfio-pci) depends on 1, without having a hard dependency on 2.
A future commit expands the use of DMABUF in vfio-pci for non-P2P scenarios, relying on pcim_p2pdma_provider() always being present. If that depended on CONFIG_PCI_P2PDMA, it would make vfio-pci only available if CONFIG_ZONE_DEVICE is present (e.g. 64-bit systems), even when P2P is not needed.
To resolve this, introduce CONFIG_PCI_P2PDMA_CORE which contains the basic provider functionality to make it available even if the CONFIG_PCI_P2PDMA feature is disabled or unavailable due to !CONFIG_ZONE_DEVICE. Users such as vfio-pci can enable their own P2P features based off the original CONFIG_PCI_P2PDMA (available when CONFIG_ZONE_DEVICE is set).
Signed-off-by: Matt Evans mattev@meta.com
Largely this looks good to me. I have one minor nit below that you can apply or not. Either way, feel free to add:
Reviewed-by: Logan Gunthorpe logang@deltatee.com
static void pci_p2pdma_release(void *data) { struct pci_dev *pdev = data; @@ -241,11 +251,13 @@ static void pci_p2pdma_release(void *data) synchronize_rcu(); xa_destroy(&p2pdma->map_types); +#ifdef CONFIG_PCI_P2PDMA if (!p2pdma->pool) return; gen_pool_destroy(p2pdma->pool); sysfs_remove_group(&pdev->dev.kobj, &p2pmem_group); +#endif }
I'm personally not a fan of adding #ifdefs inside functions like this. This instance is small and easy to understand, but it can quickly become a bit of a mess if we start adding more features. I probably would have created a pci_p2pdma_release_pool() helper which does the inverse of pci_p2pdma_setup_pool(), it would be called in pci_p2pdma_release() and an empty implementation would be provided in the case where CONFIG_PCI_P2PDMA is not set.
Logan
Hi Logan,
On 27/05/2026 17:07, Logan Gunthorpe wrote:
On 2026-05-27 04:23, Matt Evans wrote:
The P2PDMA code currently provides two features under the same CONFIG_PCI_P2PDMA option:
- Locate providers via pcim_p2pdma_provider()
- Manage actual P2P DMA
Other code (such as vfio-pci) depends on 1, without having a hard dependency on 2.
A future commit expands the use of DMABUF in vfio-pci for non-P2P scenarios, relying on pcim_p2pdma_provider() always being present. If that depended on CONFIG_PCI_P2PDMA, it would make vfio-pci only available if CONFIG_ZONE_DEVICE is present (e.g. 64-bit systems), even when P2P is not needed.
To resolve this, introduce CONFIG_PCI_P2PDMA_CORE which contains the basic provider functionality to make it available even if the CONFIG_PCI_P2PDMA feature is disabled or unavailable due to !CONFIG_ZONE_DEVICE. Users such as vfio-pci can enable their own P2P features based off the original CONFIG_PCI_P2PDMA (available when CONFIG_ZONE_DEVICE is set).
Signed-off-by: Matt Evans mattev@meta.com
Largely this looks good to me. I have one minor nit below that you can apply or not. Either way, feel free to add:
Reviewed-by: Logan Gunthorpe logang@deltatee.com
static void pci_p2pdma_release(void *data) { struct pci_dev *pdev = data; @@ -241,11 +251,13 @@ static void pci_p2pdma_release(void *data) synchronize_rcu(); xa_destroy(&p2pdma->map_types); +#ifdef CONFIG_PCI_P2PDMA if (!p2pdma->pool) return; gen_pool_destroy(p2pdma->pool); sysfs_remove_group(&pdev->dev.kobj, &p2pmem_group); +#endif }
I'm personally not a fan of adding #ifdefs inside functions like this. This instance is small and easy to understand, but it can quickly become a bit of a mess if we start adding more features. I probably would have created a pci_p2pdma_release_pool() helper which does the inverse of pci_p2pdma_setup_pool(), it would be called in pci_p2pdma_release() and an empty implementation would be provided in the case where CONFIG_PCI_P2PDMA is not set.
That's cleaner, I'll do that. Thanks for the review.
Matt
On Wed, 27 May 2026 03:23:04 -0700 Matt Evans mattev@meta.com wrote:
The P2PDMA code currently provides two features under the same CONFIG_PCI_P2PDMA option:
- Locate providers via pcim_p2pdma_provider()
- Manage actual P2P DMA
Other code (such as vfio-pci) depends on 1, without having a hard dependency on 2.
A future commit expands the use of DMABUF in vfio-pci for non-P2P scenarios, relying on pcim_p2pdma_provider() always being present. If that depended on CONFIG_PCI_P2PDMA, it would make vfio-pci only available if CONFIG_ZONE_DEVICE is present (e.g. 64-bit systems), even when P2P is not needed.
To resolve this, introduce CONFIG_PCI_P2PDMA_CORE which contains the basic provider functionality to make it available even if the CONFIG_PCI_P2PDMA feature is disabled or unavailable due to !CONFIG_ZONE_DEVICE. Users such as vfio-pci can enable their own P2P features based off the original CONFIG_PCI_P2PDMA (available when CONFIG_ZONE_DEVICE is set).
Signed-off-by: Matt Evans mattev@meta.com
drivers/pci/Kconfig | 10 +++++----- drivers/pci/Makefile | 2 +- drivers/pci/p2pdma.c | 16 ++++++++++++++++ include/linux/pci-p2pdma.h | 24 ++++++++++++++---------- include/linux/pci.h | 2 +- 5 files changed, 37 insertions(+), 17 deletions(-)
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 33c88432b728..59d70bc84cc9 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -206,11 +206,7 @@ config PCIE_TPH config PCI_P2PDMA bool "PCI peer-to-peer transfer support" depends on ZONE_DEVICE
- #
- # The need for the scatterlist DMA bus address flag means PCI P2PDMA
- # requires 64bit
- #
- depends on 64BIT
- select PCI_P2PDMA_CORE select GENERIC_ALLOCATOR select NEED_SG_DMA_FLAGS help
@@ -226,6 +222,10 @@ config PCI_P2PDMA If unsure, say N. +config PCI_P2PDMA_CORE
- default n
- bool
config PCI_LABEL def_bool y if (DMI || ACPI) select NLS diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 41ebc3b9a518..419b646a301d 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -30,7 +30,7 @@ obj-$(CONFIG_PCI_SYSCALL) += syscall.o obj-$(CONFIG_PCI_STUB) += pci-stub.o obj-$(CONFIG_PCI_PF_STUB) += pci-pf-stub.o obj-$(CONFIG_PCI_ECAM) += ecam.o -obj-$(CONFIG_PCI_P2PDMA) += p2pdma.o +obj-$(CONFIG_PCI_P2PDMA_CORE) += p2pdma.o obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o obj-$(CONFIG_VGA_ARB) += vgaarb.o obj-$(CONFIG_PCI_DOE) += doe.o diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c index 7c898542af8d..619d46c652b8 100644 --- a/drivers/pci/p2pdma.c +++ b/drivers/pci/p2pdma.c @@ -28,6 +28,14 @@ struct pci_p2pdma { struct p2pdma_provider mem[PCI_STD_NUM_BARS]; }; +/*
- CONFIG_PCI_P2PDMA_CORE provides just a bare-bones init and
- pcim_p2pdma_provider() interface (used by things like VFIO even if
- full P2PDMA isn't present). The full P2PDMA feature is under the
- CONFIG_PCI_P2PDMA option.
- */
+#ifdef CONFIG_PCI_P2PDMA
struct pci_p2pdma_pagemap { struct dev_pagemap pgmap; struct p2pdma_provider *mem; @@ -226,6 +234,8 @@ static const struct dev_pagemap_ops p2pdma_pgmap_ops = { .folio_free = p2pdma_folio_free, }; +#endif /* CONFIG_PCI_P2PDMA */
static void pci_p2pdma_release(void *data) { struct pci_dev *pdev = data; @@ -241,11 +251,13 @@ static void pci_p2pdma_release(void *data) synchronize_rcu(); xa_destroy(&p2pdma->map_types); +#ifdef CONFIG_PCI_P2PDMA if (!p2pdma->pool) return; gen_pool_destroy(p2pdma->pool); sysfs_remove_group(&pdev->dev.kobj, &p2pmem_group); +#endif } /** @@ -330,6 +342,8 @@ struct p2pdma_provider *pcim_p2pdma_provider(struct pci_dev *pdev, int bar) } EXPORT_SYMBOL_GPL(pcim_p2pdma_provider); +#ifdef CONFIG_PCI_P2PDMA
static int pci_p2pdma_setup_pool(struct pci_dev *pdev) { struct pci_p2pdma *p2pdma; @@ -1207,3 +1221,5 @@ ssize_t pci_p2pdma_enable_show(char *page, struct pci_dev *p2p_dev, return sprintf(page, "%s\n", pci_name(p2p_dev)); } EXPORT_SYMBOL_GPL(pci_p2pdma_enable_show);
+#endif
This deserves a /* CONFIG_PCI_P2PDMA */ reference like the previous long span, though potentially a better solution would be to split the core code out to a separate file with shared header. I asked opus to see what this would look like and it generated the patch below (unverified, compile tested only). Bigger diff, harder to follow blame, but the shape of the core is much more apparent. You're welcome to take or leave it. Thanks,
Alex
drivers/pci/Kconfig | 12 ++-- drivers/pci/Makefile | 1 drivers/pci/p2pdma.c | 115 ++++----------------------------------- drivers/pci/p2pdma.h | 30 ++++++++++ drivers/pci/p2pdma_core.c | 119 +++++++++++++++++++++++++++++++++++++++++ include/linux/pci-p2pdma.h | 24 ++++---- include/linux/pci.h | 2 7 files changed, 184 insertions(+), 119 deletions(-)
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 33c88432b728..2e5af8c576f5 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -206,11 +206,7 @@ config PCIE_TPH config PCI_P2PDMA bool "PCI peer-to-peer transfer support" depends on ZONE_DEVICE - # - # The need for the scatterlist DMA bus address flag means PCI P2PDMA - # requires 64bit - # - depends on 64BIT + select PCI_P2PDMA_CORE select GENERIC_ALLOCATOR select NEED_SG_DMA_FLAGS help @@ -226,6 +222,12 @@ config PCI_P2PDMA
If unsure, say N.
+# Selected by features that only need the pcim_p2pdma_provider() interface +# (e.g. vfio-pci's DMABUF export of BAR ranges) without depending on the +# full ZONE_DEVICE-backed allocator that PCI_P2PDMA pulls in. +config PCI_P2PDMA_CORE + bool + config PCI_LABEL def_bool y if (DMI || ACPI) select NLS diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 41ebc3b9a518..0b32572d57a1 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -30,6 +30,7 @@ obj-$(CONFIG_PCI_SYSCALL) += syscall.o obj-$(CONFIG_PCI_STUB) += pci-stub.o obj-$(CONFIG_PCI_PF_STUB) += pci-pf-stub.o obj-$(CONFIG_PCI_ECAM) += ecam.o +obj-$(CONFIG_PCI_P2PDMA_CORE) += p2pdma_core.o obj-$(CONFIG_PCI_P2PDMA) += p2pdma.o obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o obj-$(CONFIG_VGA_ARB) += vgaarb.o diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c index 7c898542af8d..158c274fda18 100644 --- a/drivers/pci/p2pdma.c +++ b/drivers/pci/p2pdma.c @@ -21,12 +21,7 @@ #include <linux/seq_buf.h> #include <linux/xarray.h>
-struct pci_p2pdma { - struct gen_pool *pool; - bool p2pmem_published; - struct xarray map_types; - struct p2pdma_provider mem[PCI_STD_NUM_BARS]; -}; +#include "p2pdma.h"
struct pci_p2pdma_pagemap { struct dev_pagemap pgmap; @@ -226,110 +221,24 @@ static const struct dev_pagemap_ops p2pdma_pgmap_ops = { .folio_free = p2pdma_folio_free, };
-static void pci_p2pdma_release(void *data) +/* + * Tear down the allocator pool side of pci_p2pdma. Called from the core + * release path (pci_p2pdma_release in p2pdma_core.c) while pdev->p2pdma + * has already been cleared but the structure itself has not yet been + * freed. Pairs with the rcu_dereference()s in pci_alloc_p2pmem() and + * friends, so that no allocator readers are in flight when we destroy + * the pool. + */ +void pci_p2pdma_release_pool(struct pci_p2pdma *p2pdma, struct pci_dev *pdev) { - struct pci_dev *pdev = data; - struct pci_p2pdma *p2pdma; - - p2pdma = rcu_dereference_protected(pdev->p2pdma, 1); - if (!p2pdma) - return; - - /* Flush and disable pci_alloc_p2p_mem() */ - pdev->p2pdma = NULL; - if (p2pdma->pool) - synchronize_rcu(); - xa_destroy(&p2pdma->map_types); - if (!p2pdma->pool) return;
+ synchronize_rcu(); gen_pool_destroy(p2pdma->pool); sysfs_remove_group(&pdev->dev.kobj, &p2pmem_group); }
-/** - * pcim_p2pdma_init - Initialise peer-to-peer DMA providers - * @pdev: The PCI device to enable P2PDMA for - * - * This function initializes the peer-to-peer DMA infrastructure - * for a PCI device. It allocates and sets up the necessary data - * structures to support P2PDMA operations, including mapping type - * tracking. - */ -int pcim_p2pdma_init(struct pci_dev *pdev) -{ - struct pci_p2pdma *p2p; - int i, ret; - - p2p = rcu_dereference_protected(pdev->p2pdma, 1); - if (p2p) - return 0; - - p2p = devm_kzalloc(&pdev->dev, sizeof(*p2p), GFP_KERNEL); - if (!p2p) - return -ENOMEM; - - xa_init(&p2p->map_types); - /* - * Iterate over all standard PCI BARs and record only those that - * correspond to MMIO regions. Skip non-memory resources (e.g. I/O - * port BARs) since they cannot be used for peer-to-peer (P2P) - * transactions. - */ - for (i = 0; i < PCI_STD_NUM_BARS; i++) { - if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM)) - continue; - - p2p->mem[i].owner = &pdev->dev; - p2p->mem[i].bus_offset = - pci_bus_address(pdev, i) - pci_resource_start(pdev, i); - } - - ret = devm_add_action_or_reset(&pdev->dev, pci_p2pdma_release, pdev); - if (ret) - goto out_p2p; - - rcu_assign_pointer(pdev->p2pdma, p2p); - return 0; - -out_p2p: - devm_kfree(&pdev->dev, p2p); - return ret; -} -EXPORT_SYMBOL_GPL(pcim_p2pdma_init); - -/** - * pcim_p2pdma_provider - Get peer-to-peer DMA provider - * @pdev: The PCI device to enable P2PDMA for - * @bar: BAR index to get provider - * - * This function gets peer-to-peer DMA provider for a PCI device. The lifetime - * of the provider (and of course the MMIO) is bound to the lifetime of the - * driver. A driver calling this function must ensure that all references to the - * provider, and any DMA mappings created for any MMIO, are all cleaned up - * before the driver remove() completes. - * - * Since P2P is almost always shared with a second driver this means some system - * to notify, invalidate and revoke the MMIO's DMA must be in place to use this - * function. For example a revoke can be built using DMABUF. - */ -struct p2pdma_provider *pcim_p2pdma_provider(struct pci_dev *pdev, int bar) -{ - struct pci_p2pdma *p2p; - - if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM)) - return NULL; - - p2p = rcu_dereference_protected(pdev->p2pdma, 1); - if (WARN_ON(!p2p)) - /* Someone forgot to call to pcim_p2pdma_init() before */ - return NULL; - - return &p2p->mem[bar]; -} -EXPORT_SYMBOL_GPL(pcim_p2pdma_provider); - static int pci_p2pdma_setup_pool(struct pci_dev *pdev) { struct pci_p2pdma *p2pdma; @@ -932,7 +841,7 @@ void *pci_alloc_p2pmem(struct pci_dev *pdev, size_t size) struct pci_p2pdma *p2pdma;
/* - * Pairs with synchronize_rcu() in pci_p2pdma_release() to + * Pairs with synchronize_rcu() in pci_p2pdma_release_pool() to * ensure pdev->p2pdma is non-NULL for the duration of the * read-lock. */ diff --git a/drivers/pci/p2pdma.h b/drivers/pci/p2pdma.h new file mode 100644 index 000000000000..790e1df8b609 --- /dev/null +++ b/drivers/pci/p2pdma.h @@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * PCI Peer-to-Peer DMA internal definitions, shared between p2pdma_core.c + * (always built when CONFIG_PCI_P2PDMA_CORE is set) and p2pdma.c (built + * only when the full CONFIG_PCI_P2PDMA feature is enabled). + */ + +#ifndef DRIVERS_PCI_P2PDMA_H +#define DRIVERS_PCI_P2PDMA_H + +#include <linux/genalloc.h> +#include <linux/pci.h> +#include <linux/pci-p2pdma.h> +#include <linux/xarray.h> + +struct pci_p2pdma { + struct gen_pool *pool; + bool p2pmem_published; + struct xarray map_types; + struct p2pdma_provider mem[PCI_STD_NUM_BARS]; +}; + +#ifdef CONFIG_PCI_P2PDMA +void pci_p2pdma_release_pool(struct pci_p2pdma *p2pdma, struct pci_dev *pdev); +#else +static inline void pci_p2pdma_release_pool(struct pci_p2pdma *p2pdma, + struct pci_dev *pdev) {} +#endif + +#endif /* DRIVERS_PCI_P2PDMA_H */ diff --git a/drivers/pci/p2pdma_core.c b/drivers/pci/p2pdma_core.c new file mode 100644 index 000000000000..c7622613f03a --- /dev/null +++ b/drivers/pci/p2pdma_core.c @@ -0,0 +1,119 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * PCI Peer-to-Peer DMA core: provider infrastructure shared by drivers + * that only need pcim_p2pdma_provider() (e.g. vfio-pci) without depending + * on the full CONFIG_PCI_P2PDMA allocator/pool feature. + * + * Copyright (c) 2016-2018, Logan Gunthorpe + * Copyright (c) 2016-2017, Microsemi Corporation + * Copyright (c) 2017, Christoph Hellwig + * Copyright (c) 2018, Eideticom Inc. + */ + +#define pr_fmt(fmt) "pci-p2pdma: " fmt + +#include <linux/device.h> +#include <linux/pci.h> +#include <linux/pci-p2pdma.h> +#include <linux/rcupdate.h> +#include <linux/slab.h> +#include <linux/xarray.h> + +#include "p2pdma.h" + +static void pci_p2pdma_release(void *data) +{ + struct pci_dev *pdev = data; + struct pci_p2pdma *p2pdma; + + p2pdma = rcu_dereference_protected(pdev->p2pdma, 1); + if (!p2pdma) + return; + + /* Flush and disable pci_alloc_p2p_mem() */ + pdev->p2pdma = NULL; + pci_p2pdma_release_pool(p2pdma, pdev); + xa_destroy(&p2pdma->map_types); +} + +/** + * pcim_p2pdma_init - Initialise peer-to-peer DMA providers + * @pdev: The PCI device to enable P2PDMA for + * + * This function initializes the peer-to-peer DMA infrastructure + * for a PCI device. It allocates and sets up the necessary data + * structures to support P2PDMA operations, including mapping type + * tracking. + */ +int pcim_p2pdma_init(struct pci_dev *pdev) +{ + struct pci_p2pdma *p2p; + int i, ret; + + p2p = rcu_dereference_protected(pdev->p2pdma, 1); + if (p2p) + return 0; + + p2p = devm_kzalloc(&pdev->dev, sizeof(*p2p), GFP_KERNEL); + if (!p2p) + return -ENOMEM; + + xa_init(&p2p->map_types); + /* + * Iterate over all standard PCI BARs and record only those that + * correspond to MMIO regions. Skip non-memory resources (e.g. I/O + * port BARs) since they cannot be used for peer-to-peer (P2P) + * transactions. + */ + for (i = 0; i < PCI_STD_NUM_BARS; i++) { + if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM)) + continue; + + p2p->mem[i].owner = &pdev->dev; + p2p->mem[i].bus_offset = + pci_bus_address(pdev, i) - pci_resource_start(pdev, i); + } + + ret = devm_add_action_or_reset(&pdev->dev, pci_p2pdma_release, pdev); + if (ret) + goto out_p2p; + + rcu_assign_pointer(pdev->p2pdma, p2p); + return 0; + +out_p2p: + devm_kfree(&pdev->dev, p2p); + return ret; +} +EXPORT_SYMBOL_GPL(pcim_p2pdma_init); + +/** + * pcim_p2pdma_provider - Get peer-to-peer DMA provider + * @pdev: The PCI device to enable P2PDMA for + * @bar: BAR index to get provider + * + * This function gets peer-to-peer DMA provider for a PCI device. The lifetime + * of the provider (and of course the MMIO) is bound to the lifetime of the + * driver. A driver calling this function must ensure that all references to the + * provider, and any DMA mappings created for any MMIO, are all cleaned up + * before the driver remove() completes. + * + * Since P2P is almost always shared with a second driver this means some system + * to notify, invalidate and revoke the MMIO's DMA must be in place to use this + * function. For example a revoke can be built using DMABUF. + */ +struct p2pdma_provider *pcim_p2pdma_provider(struct pci_dev *pdev, int bar) +{ + struct pci_p2pdma *p2p; + + if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM)) + return NULL; + + p2p = rcu_dereference_protected(pdev->p2pdma, 1); + if (WARN_ON(!p2p)) + /* Someone forgot to call to pcim_p2pdma_init() before */ + return NULL; + + return &p2p->mem[bar]; +} +EXPORT_SYMBOL_GPL(pcim_p2pdma_provider); diff --git a/include/linux/pci-p2pdma.h b/include/linux/pci-p2pdma.h index 873de20a2247..4c42a7b2ee85 100644 --- a/include/linux/pci-p2pdma.h +++ b/include/linux/pci-p2pdma.h @@ -67,9 +67,22 @@ enum pci_p2pdma_map_type { PCI_P2PDMA_MAP_THRU_HOST_BRIDGE, };
-#ifdef CONFIG_PCI_P2PDMA +#ifdef CONFIG_PCI_P2PDMA_CORE int pcim_p2pdma_init(struct pci_dev *pdev); struct p2pdma_provider *pcim_p2pdma_provider(struct pci_dev *pdev, int bar); +#else +static inline int pcim_p2pdma_init(struct pci_dev *pdev) +{ + return -EOPNOTSUPP; +} +static inline struct p2pdma_provider *pcim_p2pdma_provider(struct pci_dev *pdev, + int bar) +{ + return NULL; +} +#endif + +#ifdef CONFIG_PCI_P2PDMA int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, size_t size, u64 offset); int pci_p2pdma_distance_many(struct pci_dev *provider, struct device **clients, @@ -89,15 +102,6 @@ ssize_t pci_p2pdma_enable_show(char *page, struct pci_dev *p2p_dev, enum pci_p2pdma_map_type pci_p2pdma_map_type(struct p2pdma_provider *provider, struct device *dev); #else /* CONFIG_PCI_P2PDMA */ -static inline int pcim_p2pdma_init(struct pci_dev *pdev) -{ - return -EOPNOTSUPP; -} -static inline struct p2pdma_provider *pcim_p2pdma_provider(struct pci_dev *pdev, - int bar) -{ - return NULL; -} static inline int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, size_t size, u64 offset) { diff --git a/include/linux/pci.h b/include/linux/pci.h index 2c4454583c11..531aec355686 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -557,7 +557,7 @@ struct pci_dev { u16 pasid_cap; /* PASID Capability offset */ u16 pasid_features; #endif -#ifdef CONFIG_PCI_P2PDMA +#ifdef CONFIG_PCI_P2PDMA_CORE struct pci_p2pdma __rcu *p2pdma; #endif #ifdef CONFIG_PCI_DOE
On Wed, May 27, 2026 at 03:09:46PM -0600, Alex Williamson wrote:
This deserves a /* CONFIG_PCI_P2PDMA */ reference like the previous long span, though potentially a better solution would be to split the core code out to a separate file with shared header. I asked opus to see what this would look like and it generated the patch below (unverified, compile tested only). Bigger diff, harder to follow blame, but the shape of the core is much more apparent. You're welcome to take or leave it.
I think it is a good idea, it will make it easier to add more stuff to these different aspects.
Really they are now two different things with one layered on top of the other.
Jason
On Fri, May 29, 2026 at 08:05:53PM -0300, Jason Gunthorpe wrote:
On Wed, May 27, 2026 at 03:09:46PM -0600, Alex Williamson wrote:
This deserves a /* CONFIG_PCI_P2PDMA */ reference like the previous long span, though potentially a better solution would be to split the core code out to a separate file with shared header. I asked opus to see what this would look like and it generated the patch below (unverified, compile tested only). Bigger diff, harder to follow blame, but the shape of the core is much more apparent. You're welcome to take or leave it.
I think it is a good idea, it will make it easier to add more stuff to these different aspects.
Really they are now two different things with one layered on top of the other.
Sounds good to me too.
On Fri, May 29, 2026 at 08:05:53PM -0300, Jason Gunthorpe wrote:
On Wed, May 27, 2026 at 03:09:46PM -0600, Alex Williamson wrote:
This deserves a /* CONFIG_PCI_P2PDMA */ reference like the previous long span, though potentially a better solution would be to split the core code out to a separate file with shared header. I asked opus to see what this would look like and it generated the patch below (unverified, compile tested only). Bigger diff, harder to follow blame, but the shape of the core is much more apparent. You're welcome to take or leave it.
I think it is a good idea, it will make it easier to add more stuff to these different aspects.
Really they are now two different things with one layered on top of the other.
A little late to this party. I agree with this split per status quo. Although, one observation from me, I've posted a concurrent RFC [1] that implements ZONE_DEVICE P2P registration for VFIO. That work relies on pci_p2pdma_add_resource(), which requires ZONE_DEVICE and the full CONFIG_PCI_P2PDMA stack rather than just the CORE part.
I guess I can be careful to guard the ZONE_DEVICE P2P registration feature in VFIO with the full PCI_P2PDMA config. Apart from that, this split provides a clean foundation for both series to coexist.
Thanks, Praan
[1] https://lore.kernel.org/all/20260610151853.3608948-1-praan@google.com/
From: Matt Evans mattev@meta.com Sent: Wednesday, May 27, 2026 6:23 PM
The P2PDMA code currently provides two features under the same CONFIG_PCI_P2PDMA option:
- Locate providers via pcim_p2pdma_provider()
- Manage actual P2P DMA
Other code (such as vfio-pci) depends on 1, without having a hard dependency on 2.
A future commit expands the use of DMABUF in vfio-pci for non-P2P scenarios, relying on pcim_p2pdma_provider() always being present. If that depended on CONFIG_PCI_P2PDMA, it would make vfio-pci only available if CONFIG_ZONE_DEVICE is present (e.g. 64-bit systems), even when P2P is not needed.
To resolve this, introduce CONFIG_PCI_P2PDMA_CORE which contains the basic provider functionality to make it available even if the CONFIG_PCI_P2PDMA feature is disabled or unavailable due to !CONFIG_ZONE_DEVICE. Users such as vfio-pci can enable their own P2P features based off the original CONFIG_PCI_P2PDMA (available when CONFIG_ZONE_DEVICE is set).
Signed-off-by: Matt Evans mattev@meta.com
Reviewed-by: Kevin Tian kevin.tian@intel.com
Add vfio_pci_dma_buf_find_pfn(), which a VMA fault handler can use to find a PFN.
This supports multi-range DMABUFs, which typically would be used to represent scattered spans but might even represent overlapping or aliasing spans of PFNs.
Because this is intended to be used in vfio_pci_core.c, we also need to expose the struct vfio_pci_dma_buf in the vfio_pci_priv.h header.
Signed-off-by: Matt Evans mattev@meta.com --- drivers/vfio/pci/vfio_pci_dmabuf.c | 142 ++++++++++++++++++++++++++--- drivers/vfio/pci/vfio_pci_priv.h | 20 ++++ 2 files changed, 149 insertions(+), 13 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index c16f460c01d6..0d132c4ca95f 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -9,19 +9,6 @@
MODULE_IMPORT_NS("DMA_BUF");
-struct vfio_pci_dma_buf { - struct dma_buf *dmabuf; - struct vfio_pci_core_device *vdev; - struct list_head dmabufs_elm; - size_t size; - struct phys_vec *phys_vec; - struct p2pdma_provider *provider; - u32 nr_ranges; - struct kref kref; - struct completion comp; - u8 revoked : 1; -}; - static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf, struct dma_buf_attachment *attachment) { @@ -106,6 +93,135 @@ static const struct dma_buf_ops vfio_pci_dmabuf_ops = { .release = vfio_pci_dma_buf_release, };
+int vfio_pci_dma_buf_find_pfn(struct vfio_pci_dma_buf *vpdmabuf, + struct vm_area_struct *vma, + unsigned long address, + unsigned int order, + unsigned long *out_pfn) +{ + /* + * Given a VMA (start, end, pgoffs) and a fault address, + * search the corresponding DMABUF's phys_vec[] to find the + * range representing the address's offset into the VMA, and + * its PFN. + * + * The phys_vec[] ranges represent contiguous spans of VAs + * upwards from the buffer offset 0; the actual PFNs might be + * in any order, overlap/alias, etc. Calculate an offset of + * the desired page given VMA start/pgoff and address, then + * search upwards from 0 to find which span contains it. + * + * On success, a valid PFN for a page sized by 'order' is + * returned into out_pfn. + * + * Failure occurs if: + * - The page would cross the edge of the VMA + * - The page isn't entirely contained within a range + * - We find a range, but the final PFN isn't aligned to the + * requested order. + * + * (Upon failure, the caller is expected to try again with a + * smaller order; the tests above will always succeed for + * order=0 as the limit case.) + * + * It's suboptimal if DMABUFs are created with neigbouring + * ranges that are physically contiguous, since hugepages + * can't straddle range boundaries. (The construction of the + * ranges vector should merge such ranges.) + * + * Finally, vma_pgoff_adjust is used for a DMABUF representing + * a VFIO BAR mmap, which is created from the start of the + * offset region. It should be zero, or equal vm_pgoff. + */ + + const unsigned long pagesize = PAGE_SIZE << order; + unsigned long vma_off = ((vma->vm_pgoff - vpdmabuf->vma_pgoff_adjust) << + PAGE_SHIFT) & VFIO_PCI_OFFSET_MASK; + unsigned long rounded_page_addr = ALIGN_DOWN(address, pagesize); + unsigned long rounded_page_end = rounded_page_addr + pagesize; + unsigned long page_buf_offset; + unsigned long range_buf_offset = 0; + unsigned int i; + + if (rounded_page_addr < vma->vm_start || rounded_page_end > vma->vm_end) { + if (order > 0) + return -EAGAIN; + + /* A fault address outside of the VMA is absurd. */ + WARN(1, "Fault addr 0x%lx outside VMA 0x%lx-0x%lx\n", + address, vma->vm_start, vma->vm_end); + return -EFAULT; + } + + if (vpdmabuf->vma_pgoff_adjust != 0 && + vpdmabuf->vma_pgoff_adjust != (vma->vm_pgoff & + (VFIO_PCI_OFFSET_MASK >> PAGE_SHIFT))) { + WARN(1, "Unexpected vma_pgoff_adjust 0x%lx (vm_pgoff 0x%lx)\n", + vpdmabuf->vma_pgoff_adjust, vma->vm_pgoff); + return -EFAULT; + } + + if (unlikely(check_add_overflow(rounded_page_addr - vma->vm_start, + vma_off, &page_buf_offset))) + return -EFAULT; + + for (i = 0; i < vpdmabuf->nr_ranges; i++) { + unsigned long page_buf_offset_end; + size_t range_len = vpdmabuf->phys_vec[i].len; + phys_addr_t range_start = vpdmabuf->phys_vec[i].paddr; + + if (unlikely(check_add_overflow(page_buf_offset, pagesize, + &page_buf_offset_end))) + return -EFAULT; + /* + * If the current range starts after the page's span, + * this and any future range won't match. Bail early. + */ + if (page_buf_offset_end <= range_buf_offset) + break; + + if (page_buf_offset >= range_buf_offset && + page_buf_offset_end <= range_buf_offset + range_len) { + /* + * The faulting page is wholly contained + * within the span represented by the range. + * Validate PFN alignment for the order: + */ + unsigned long pfn = (range_start + page_buf_offset - + range_buf_offset) / PAGE_SIZE; + + if (IS_ALIGNED(pfn, 1 << order)) { + *out_pfn = pfn; + return 0; + } + /* Retry with smaller order */ + return -EAGAIN; + } + range_buf_offset += range_len; + } + + /* + * A hugepage straddling a range boundary will fail to match a + * range, but the address will (eventually) match when retried + * with a smaller page. + */ + if (order > 0) + return -EAGAIN; + + /* + * If we get here, the address fell outside of the span + * represented by the (concatenated) ranges. Setup of a + * mapping must ensure that the VMA is <= the total size of + * the ranges, so this should never happen. But, if it does, + * force SIGBUS for the access and warn. + */ + WARN_ONCE(1, "No range for addr 0x%lx, order %d: VMA 0x%lx-0x%lx pgoff 0x%lx, %u ranges, size 0x%zx\n", + address, order, vma->vm_start, vma->vm_end, vma->vm_pgoff, + vpdmabuf->nr_ranges, vpdmabuf->size); + + return -EFAULT; +} + /* * This is a temporary "private interconnect" between VFIO DMABUF and iommufd. * It allows the two co-operating drivers to exchange the physical address of diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index fca9d0dfac90..c8f6f959056a 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -23,6 +23,20 @@ struct vfio_pci_ioeventfd { bool test_mem; };
+struct vfio_pci_dma_buf { + struct dma_buf *dmabuf; + struct vfio_pci_core_device *vdev; + struct list_head dmabufs_elm; + size_t size; + struct phys_vec *phys_vec; + struct p2pdma_provider *provider; + u32 nr_ranges; + struct kref kref; + struct completion comp; + unsigned long vma_pgoff_adjust; + u8 revoked : 1; +}; + bool vfio_pci_intx_mask(struct vfio_pci_core_device *vdev); void vfio_pci_intx_unmask(struct vfio_pci_core_device *vdev);
@@ -114,6 +128,12 @@ static inline bool vfio_pci_is_vga(struct pci_dev *pdev) return (pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA; }
+int vfio_pci_dma_buf_find_pfn(struct vfio_pci_dma_buf *vpdmabuf, + struct vm_area_struct *vma, + unsigned long address, + unsigned int order, + unsigned long *out_pfn); + #ifdef CONFIG_VFIO_PCI_DMABUF int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, struct vfio_device_feature_dma_buf __user *arg,
On Wed, 27 May 2026 03:23:05 -0700 Matt Evans mattev@meta.com wrote:
Add vfio_pci_dma_buf_find_pfn(), which a VMA fault handler can use to find a PFN.
This supports multi-range DMABUFs, which typically would be used to represent scattered spans but might even represent overlapping or aliasing spans of PFNs.
Because this is intended to be used in vfio_pci_core.c, we also need to expose the struct vfio_pci_dma_buf in the vfio_pci_priv.h header.
Signed-off-by: Matt Evans mattev@meta.com
drivers/vfio/pci/vfio_pci_dmabuf.c | 142 ++++++++++++++++++++++++++--- drivers/vfio/pci/vfio_pci_priv.h | 20 ++++ 2 files changed, 149 insertions(+), 13 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index c16f460c01d6..0d132c4ca95f 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -9,19 +9,6 @@ MODULE_IMPORT_NS("DMA_BUF"); -struct vfio_pci_dma_buf {
- struct dma_buf *dmabuf;
- struct vfio_pci_core_device *vdev;
- struct list_head dmabufs_elm;
- size_t size;
- struct phys_vec *phys_vec;
- struct p2pdma_provider *provider;
- u32 nr_ranges;
- struct kref kref;
- struct completion comp;
- u8 revoked : 1;
-};
static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf, struct dma_buf_attachment *attachment) { @@ -106,6 +93,135 @@ static const struct dma_buf_ops vfio_pci_dmabuf_ops = { .release = vfio_pci_dma_buf_release, }; +int vfio_pci_dma_buf_find_pfn(struct vfio_pci_dma_buf *vpdmabuf,
This private dmabuf object pointer is named priv pretty consistently throughout the rest of the file.
struct vm_area_struct *vma,unsigned long address,unsigned int order,unsigned long *out_pfn)+{
- /*
* Given a VMA (start, end, pgoffs) and a fault address,* search the corresponding DMABUF's phys_vec[] to find the* range representing the address's offset into the VMA, and* its PFN.** The phys_vec[] ranges represent contiguous spans of VAs* upwards from the buffer offset 0; the actual PFNs might be* in any order, overlap/alias, etc. Calculate an offset of* the desired page given VMA start/pgoff and address, then* search upwards from 0 to find which span contains it.** On success, a valid PFN for a page sized by 'order' is* returned into out_pfn.** Failure occurs if:* - The page would cross the edge of the VMA* - The page isn't entirely contained within a range* - We find a range, but the final PFN isn't aligned to the* requested order.** (Upon failure, the caller is expected to try again with a* smaller order; the tests above will always succeed for* order=0 as the limit case.)** It's suboptimal if DMABUFs are created with neigbouring* ranges that are physically contiguous, since hugepages* can't straddle range boundaries. (The construction of the* ranges vector should merge such ranges.)** Finally, vma_pgoff_adjust is used for a DMABUF representing* a VFIO BAR mmap, which is created from the start of the* offset region. It should be zero, or equal vm_pgoff.*/- const unsigned long pagesize = PAGE_SIZE << order;
- unsigned long vma_off = ((vma->vm_pgoff - vpdmabuf->vma_pgoff_adjust) <<
PAGE_SHIFT) & VFIO_PCI_OFFSET_MASK;- unsigned long rounded_page_addr = ALIGN_DOWN(address, pagesize);
- unsigned long rounded_page_end = rounded_page_addr + pagesize;
- unsigned long page_buf_offset;
- unsigned long range_buf_offset = 0;
- unsigned int i;
- if (rounded_page_addr < vma->vm_start || rounded_page_end > vma->vm_end) {
if (order > 0)return -EAGAIN;/* A fault address outside of the VMA is absurd. */WARN(1, "Fault addr 0x%lx outside VMA 0x%lx-0x%lx\n",address, vma->vm_start, vma->vm_end);return -EFAULT;- }
- if (vpdmabuf->vma_pgoff_adjust != 0 &&
vpdmabuf->vma_pgoff_adjust != (vma->vm_pgoff &(VFIO_PCI_OFFSET_MASK >> PAGE_SHIFT))) {WARN(1, "Unexpected vma_pgoff_adjust 0x%lx (vm_pgoff 0x%lx)\n",vpdmabuf->vma_pgoff_adjust, vma->vm_pgoff);return -EFAULT;- }
This appears to be user trigger'able, by causing a vma to be split, and also unnecessary as it seems the math works out correctly for the split case.
- if (unlikely(check_add_overflow(rounded_page_addr - vma->vm_start,
vma_off, &page_buf_offset)))return -EFAULT;- for (i = 0; i < vpdmabuf->nr_ranges; i++) {
unsigned long page_buf_offset_end;size_t range_len = vpdmabuf->phys_vec[i].len;phys_addr_t range_start = vpdmabuf->phys_vec[i].paddr;if (unlikely(check_add_overflow(page_buf_offset, pagesize,&page_buf_offset_end)))return -EFAULT;
Why is this inside the loop, the args are invariant of anything modified in the loop. Thanks,
Alex
/** If the current range starts after the page's span,* this and any future range won't match. Bail early.*/if (page_buf_offset_end <= range_buf_offset)break;if (page_buf_offset >= range_buf_offset &&page_buf_offset_end <= range_buf_offset + range_len) {/** The faulting page is wholly contained* within the span represented by the range.* Validate PFN alignment for the order:*/unsigned long pfn = (range_start + page_buf_offset -range_buf_offset) / PAGE_SIZE;if (IS_ALIGNED(pfn, 1 << order)) {*out_pfn = pfn;return 0;}/* Retry with smaller order */return -EAGAIN;}range_buf_offset += range_len;- }
- /*
* A hugepage straddling a range boundary will fail to match a* range, but the address will (eventually) match when retried* with a smaller page.*/- if (order > 0)
return -EAGAIN;- /*
* If we get here, the address fell outside of the span* represented by the (concatenated) ranges. Setup of a* mapping must ensure that the VMA is <= the total size of* the ranges, so this should never happen. But, if it does,* force SIGBUS for the access and warn.*/- WARN_ONCE(1, "No range for addr 0x%lx, order %d: VMA 0x%lx-0x%lx pgoff 0x%lx, %u ranges, size 0x%zx\n",
address, order, vma->vm_start, vma->vm_end, vma->vm_pgoff,vpdmabuf->nr_ranges, vpdmabuf->size);- return -EFAULT;
+}
/*
- This is a temporary "private interconnect" between VFIO DMABUF and iommufd.
- It allows the two co-operating drivers to exchange the physical address of
diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index fca9d0dfac90..c8f6f959056a 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -23,6 +23,20 @@ struct vfio_pci_ioeventfd { bool test_mem; }; +struct vfio_pci_dma_buf {
- struct dma_buf *dmabuf;
- struct vfio_pci_core_device *vdev;
- struct list_head dmabufs_elm;
- size_t size;
- struct phys_vec *phys_vec;
- struct p2pdma_provider *provider;
- u32 nr_ranges;
- struct kref kref;
- struct completion comp;
- unsigned long vma_pgoff_adjust;
- u8 revoked : 1;
+};
bool vfio_pci_intx_mask(struct vfio_pci_core_device *vdev); void vfio_pci_intx_unmask(struct vfio_pci_core_device *vdev); @@ -114,6 +128,12 @@ static inline bool vfio_pci_is_vga(struct pci_dev *pdev) return (pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA; } +int vfio_pci_dma_buf_find_pfn(struct vfio_pci_dma_buf *vpdmabuf,
struct vm_area_struct *vma,unsigned long address,unsigned int order,unsigned long *out_pfn);#ifdef CONFIG_VFIO_PCI_DMABUF int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, struct vfio_device_feature_dma_buf __user *arg,
Hi Alex,
On 27/05/2026 23:38, Alex Williamson wrote:
On Wed, 27 May 2026 03:23:05 -0700 Matt Evans mattev@meta.com wrote:
Add vfio_pci_dma_buf_find_pfn(), which a VMA fault handler can use to find a PFN.
This supports multi-range DMABUFs, which typically would be used to represent scattered spans but might even represent overlapping or aliasing spans of PFNs.
Because this is intended to be used in vfio_pci_core.c, we also need to expose the struct vfio_pci_dma_buf in the vfio_pci_priv.h header.
Signed-off-by: Matt Evans mattev@meta.com
drivers/vfio/pci/vfio_pci_dmabuf.c | 142 ++++++++++++++++++++++++++--- drivers/vfio/pci/vfio_pci_priv.h | 20 ++++ 2 files changed, 149 insertions(+), 13 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index c16f460c01d6..0d132c4ca95f 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -9,19 +9,6 @@ MODULE_IMPORT_NS("DMA_BUF"); -struct vfio_pci_dma_buf {
- struct dma_buf *dmabuf;
- struct vfio_pci_core_device *vdev;
- struct list_head dmabufs_elm;
- size_t size;
- struct phys_vec *phys_vec;
- struct p2pdma_provider *provider;
- u32 nr_ranges;
- struct kref kref;
- struct completion comp;
- u8 revoked : 1;
-};
- static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf, struct dma_buf_attachment *attachment) {
@@ -106,6 +93,135 @@ static const struct dma_buf_ops vfio_pci_dmabuf_ops = { .release = vfio_pci_dma_buf_release, }; +int vfio_pci_dma_buf_find_pfn(struct vfio_pci_dma_buf *vpdmabuf,
This private dmabuf object pointer is named priv pretty consistently throughout the rest of the file.
Right you are, fixed.
struct vm_area_struct *vma,unsigned long address,unsigned int order,unsigned long *out_pfn)+{
- /*
* Given a VMA (start, end, pgoffs) and a fault address,* search the corresponding DMABUF's phys_vec[] to find the* range representing the address's offset into the VMA, and* its PFN.** The phys_vec[] ranges represent contiguous spans of VAs* upwards from the buffer offset 0; the actual PFNs might be* in any order, overlap/alias, etc. Calculate an offset of* the desired page given VMA start/pgoff and address, then* search upwards from 0 to find which span contains it.** On success, a valid PFN for a page sized by 'order' is* returned into out_pfn.** Failure occurs if:* - The page would cross the edge of the VMA* - The page isn't entirely contained within a range* - We find a range, but the final PFN isn't aligned to the* requested order.** (Upon failure, the caller is expected to try again with a* smaller order; the tests above will always succeed for* order=0 as the limit case.)** It's suboptimal if DMABUFs are created with neigbouring* ranges that are physically contiguous, since hugepages* can't straddle range boundaries. (The construction of the* ranges vector should merge such ranges.)** Finally, vma_pgoff_adjust is used for a DMABUF representing* a VFIO BAR mmap, which is created from the start of the* offset region. It should be zero, or equal vm_pgoff.*/- const unsigned long pagesize = PAGE_SIZE << order;
- unsigned long vma_off = ((vma->vm_pgoff - vpdmabuf->vma_pgoff_adjust) <<
PAGE_SHIFT) & VFIO_PCI_OFFSET_MASK;- unsigned long rounded_page_addr = ALIGN_DOWN(address, pagesize);
- unsigned long rounded_page_end = rounded_page_addr + pagesize;
- unsigned long page_buf_offset;
- unsigned long range_buf_offset = 0;
- unsigned int i;
- if (rounded_page_addr < vma->vm_start || rounded_page_end > vma->vm_end) {
if (order > 0)return -EAGAIN;/* A fault address outside of the VMA is absurd. */WARN(1, "Fault addr 0x%lx outside VMA 0x%lx-0x%lx\n",address, vma->vm_start, vma->vm_end);return -EFAULT;- }
- if (vpdmabuf->vma_pgoff_adjust != 0 &&
vpdmabuf->vma_pgoff_adjust != (vma->vm_pgoff &(VFIO_PCI_OFFSET_MASK >> PAGE_SHIFT))) {WARN(1, "Unexpected vma_pgoff_adjust 0x%lx (vm_pgoff 0x%lx)\n",vpdmabuf->vma_pgoff_adjust, vma->vm_pgoff);return -EFAULT;- }
This appears to be user trigger'able, by causing a vma to be split, and also unnecessary as it seems the math works out correctly for the split case.
Good spot, fixed. I added a VMA split case to the [RFC] vfio_dmabuf_mmap_test, thanks for spotting this.
- if (unlikely(check_add_overflow(rounded_page_addr - vma->vm_start,
vma_off, &page_buf_offset)))return -EFAULT;- for (i = 0; i < vpdmabuf->nr_ranges; i++) {
unsigned long page_buf_offset_end;size_t range_len = vpdmabuf->phys_vec[i].len;phys_addr_t range_start = vpdmabuf->phys_vec[i].paddr;if (unlikely(check_add_overflow(page_buf_offset, pagesize,&page_buf_offset_end)))return -EFAULT;Why is this inside the loop, the args are invariant of anything modified in the loop. Thanks,
Right; I slipped up when renaming/refactoring the additions, that definitely goes above. Fixed!
Thanks,
Matt
Alex
/** If the current range starts after the page's span,* this and any future range won't match. Bail early.*/if (page_buf_offset_end <= range_buf_offset)break;if (page_buf_offset >= range_buf_offset &&page_buf_offset_end <= range_buf_offset + range_len) {/** The faulting page is wholly contained* within the span represented by the range.* Validate PFN alignment for the order:*/unsigned long pfn = (range_start + page_buf_offset -range_buf_offset) / PAGE_SIZE;if (IS_ALIGNED(pfn, 1 << order)) {*out_pfn = pfn;return 0;}/* Retry with smaller order */return -EAGAIN;}range_buf_offset += range_len;- }
- /*
* A hugepage straddling a range boundary will fail to match a* range, but the address will (eventually) match when retried* with a smaller page.*/- if (order > 0)
return -EAGAIN;- /*
* If we get here, the address fell outside of the span* represented by the (concatenated) ranges. Setup of a* mapping must ensure that the VMA is <= the total size of* the ranges, so this should never happen. But, if it does,* force SIGBUS for the access and warn.*/- WARN_ONCE(1, "No range for addr 0x%lx, order %d: VMA 0x%lx-0x%lx pgoff 0x%lx, %u ranges, size 0x%zx\n",
address, order, vma->vm_start, vma->vm_end, vma->vm_pgoff,vpdmabuf->nr_ranges, vpdmabuf->size);- return -EFAULT;
+}
- /*
- This is a temporary "private interconnect" between VFIO DMABUF and iommufd.
- It allows the two co-operating drivers to exchange the physical address of
diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index fca9d0dfac90..c8f6f959056a 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -23,6 +23,20 @@ struct vfio_pci_ioeventfd { bool test_mem; }; +struct vfio_pci_dma_buf {
- struct dma_buf *dmabuf;
- struct vfio_pci_core_device *vdev;
- struct list_head dmabufs_elm;
- size_t size;
- struct phys_vec *phys_vec;
- struct p2pdma_provider *provider;
- u32 nr_ranges;
- struct kref kref;
- struct completion comp;
- unsigned long vma_pgoff_adjust;
- u8 revoked : 1;
+};
- bool vfio_pci_intx_mask(struct vfio_pci_core_device *vdev); void vfio_pci_intx_unmask(struct vfio_pci_core_device *vdev);
@@ -114,6 +128,12 @@ static inline bool vfio_pci_is_vga(struct pci_dev *pdev) return (pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA; } +int vfio_pci_dma_buf_find_pfn(struct vfio_pci_dma_buf *vpdmabuf,
struct vm_area_struct *vma,unsigned long address,unsigned int order,unsigned long *out_pfn);- #ifdef CONFIG_VFIO_PCI_DMABUF int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, struct vfio_device_feature_dma_buf __user *arg,
This helper, vfio_pci_core_mmap_prep_dmabuf(), creates a single-range DMABUF for the purpose of mapping a PCI BAR. This is used in a future commit by VFIO's ordinary mmap() path.
This function transfers ownership of the VFIO device fd to the DMABUF, which fput()s when it's released.
Refactor the existing vfio_pci_core_feature_dma_buf() to split out export code common to the two paths, VFIO_DEVICE_FEATURE_DMA_BUF and this new VFIO_BAR mmap().
Signed-off-by: Matt Evans mattev@meta.com --- drivers/vfio/pci/vfio_pci_dmabuf.c | 140 ++++++++++++++++++++++------- drivers/vfio/pci/vfio_pci_priv.h | 5 ++ 2 files changed, 115 insertions(+), 30 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index 0d132c4ca95f..782408c08a5e 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -82,6 +82,8 @@ static void vfio_pci_dma_buf_release(struct dma_buf *dmabuf) up_write(&priv->vdev->memory_lock); vfio_device_put_registration(&priv->vdev->vdev); } + if (priv->vfile) + fput(priv->vfile); kfree(priv->phys_vec); kfree(priv); } @@ -222,6 +224,45 @@ int vfio_pci_dma_buf_find_pfn(struct vfio_pci_dma_buf *vpdmabuf, return -EFAULT; }
+/* + * Create a DMABUF corresponding to priv, add it to vdev->dmabufs list + * for tracking (meaning cleanup or revocation will zap it), and take + * a vfio_device registration. + */ +static int vfio_pci_dmabuf_export(struct vfio_pci_core_device *vdev, + struct vfio_pci_dma_buf *priv, uint32_t flags) +{ + DEFINE_DMA_BUF_EXPORT_INFO(exp_info); + + if (!vfio_device_try_get_registration(&vdev->vdev)) + return -ENODEV; + + exp_info.ops = &vfio_pci_dmabuf_ops; + exp_info.size = priv->size; + exp_info.flags = flags; + exp_info.priv = priv; + + priv->dmabuf = dma_buf_export(&exp_info); + if (IS_ERR(priv->dmabuf)) { + vfio_device_put_registration(&vdev->vdev); + return PTR_ERR(priv->dmabuf); + } + + kref_init(&priv->kref); + init_completion(&priv->comp); + + /* dma_buf_put() now frees priv */ + INIT_LIST_HEAD(&priv->dmabufs_elm); + down_write(&vdev->memory_lock); + dma_resv_lock(priv->dmabuf->resv, NULL); + priv->revoked = !__vfio_pci_memory_enabled(vdev); + list_add_tail(&priv->dmabufs_elm, &vdev->dmabufs); + dma_resv_unlock(priv->dmabuf->resv); + up_write(&vdev->memory_lock); + + return 0; +} + /* * This is a temporary "private interconnect" between VFIO DMABUF and iommufd. * It allows the two co-operating drivers to exchange the physical address of @@ -340,7 +381,6 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, { struct vfio_device_feature_dma_buf get_dma_buf = {}; struct vfio_region_dma_range *dma_ranges; - DEFINE_DMA_BUF_EXPORT_INFO(exp_info); struct vfio_pci_dma_buf *priv; size_t length; int ret; @@ -400,34 +440,9 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, kfree(dma_ranges); dma_ranges = NULL;
- if (!vfio_device_try_get_registration(&vdev->vdev)) { - ret = -ENODEV; + ret = vfio_pci_dmabuf_export(vdev, priv, get_dma_buf.open_flags); + if (ret) goto err_free_phys; - } - - exp_info.ops = &vfio_pci_dmabuf_ops; - exp_info.size = priv->size; - exp_info.flags = get_dma_buf.open_flags; - exp_info.priv = priv; - - priv->dmabuf = dma_buf_export(&exp_info); - if (IS_ERR(priv->dmabuf)) { - ret = PTR_ERR(priv->dmabuf); - goto err_dev_put; - } - - kref_init(&priv->kref); - init_completion(&priv->comp); - - /* dma_buf_put() now frees priv */ - INIT_LIST_HEAD(&priv->dmabufs_elm); - down_write(&vdev->memory_lock); - dma_resv_lock(priv->dmabuf->resv, NULL); - priv->revoked = !__vfio_pci_memory_enabled(vdev); - list_add_tail(&priv->dmabufs_elm, &vdev->dmabufs); - dma_resv_unlock(priv->dmabuf->resv); - up_write(&vdev->memory_lock); - /* * dma_buf_fd() consumes the reference, when the file closes the dmabuf * will be released. @@ -438,8 +453,6 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
return ret;
-err_dev_put: - vfio_device_put_registration(&vdev->vdev); err_free_phys: kfree(priv->phys_vec); err_free_priv: @@ -449,6 +462,73 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, return ret; }
+int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev, + struct vm_area_struct *vma, + u64 phys_start, u64 req_len, + unsigned int res_index) +{ + struct vfio_pci_dma_buf *priv; + const unsigned int nr_ranges = 1; + unsigned long vma_pgoff = vma->vm_pgoff & (VFIO_PCI_OFFSET_MASK >> PAGE_SHIFT); + int ret; + + priv = kzalloc_obj(*priv); + if (!priv) + return -ENOMEM; + + priv->phys_vec = kzalloc_obj(*priv->phys_vec); + if (!priv->phys_vec) { + ret = -ENOMEM; + goto err_free_priv; + } + + /* + * The DMABUF begins from the mmap()'s BAR offset, i.e. the + * start of the VMA corresponds to byte 0 of the DMABUF and + * byte (vma_pgoff << PAGE_SHIFT) of the BAR. + * + * vfio_pci_dma_buf_find_pfn() reverses this offset using + * vma_pgoff_adjust, so that ultimately a fault's offset from + * the start of the _VMA_ has a consistent usage whether the + * VMA originates from an mmap() of the VFIO device here or a + * direct DMABUF mmap(). + */ + priv->vdev = vdev; + priv->size = req_len; + priv->nr_ranges = nr_ranges; + priv->vma_pgoff_adjust = vma_pgoff; + priv->provider = pcim_p2pdma_provider(vdev->pdev, res_index); + if (!priv->provider) { + ret = -EINVAL; + goto err_free_phys; + } + + priv->phys_vec[0].paddr = phys_start + ((u64)vma_pgoff << PAGE_SHIFT); + priv->phys_vec[0].len = priv->size; + + ret = vfio_pci_dmabuf_export(vdev, priv, O_CLOEXEC | O_RDWR); + if (ret) + goto err_free_phys; + + /* + * The VMA gets the DMABUF file so that other users can locate + * the DMABUF via a VA. Ownership of the original VFIO device + * file being mmap()ed transfers to priv, and is put when the + * DMABUF is released. + */ + priv->vfile = vma->vm_file; + vma->vm_file = priv->dmabuf->file; + vma->vm_private_data = priv; + + return 0; + +err_free_phys: + kfree(priv->phys_vec); +err_free_priv: + kfree(priv); + return ret; +} + void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked) { struct vfio_pci_dma_buf *priv; diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index c8f6f959056a..06dc0fd3e230 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -30,6 +30,7 @@ struct vfio_pci_dma_buf { size_t size; struct phys_vec *phys_vec; struct p2pdma_provider *provider; + struct file *vfile; u32 nr_ranges; struct kref kref; struct completion comp; @@ -133,6 +134,10 @@ int vfio_pci_dma_buf_find_pfn(struct vfio_pci_dma_buf *vpdmabuf, unsigned long address, unsigned int order, unsigned long *out_pfn); +int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev, + struct vm_area_struct *vma, + u64 phys_start, u64 req_len, + unsigned int res_index);
#ifdef CONFIG_VFIO_PCI_DMABUF int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
On Wed, 27 May 2026 03:23:06 -0700 Matt Evans mattev@meta.com wrote:
This helper, vfio_pci_core_mmap_prep_dmabuf(), creates a single-range DMABUF for the purpose of mapping a PCI BAR. This is used in a future commit by VFIO's ordinary mmap() path.
This function transfers ownership of the VFIO device fd to the DMABUF, which fput()s when it's released.
Refactor the existing vfio_pci_core_feature_dma_buf() to split out export code common to the two paths, VFIO_DEVICE_FEATURE_DMA_BUF and this new VFIO_BAR mmap().
Signed-off-by: Matt Evans mattev@meta.com
drivers/vfio/pci/vfio_pci_dmabuf.c | 140 ++++++++++++++++++++++------- drivers/vfio/pci/vfio_pci_priv.h | 5 ++ 2 files changed, 115 insertions(+), 30 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index 0d132c4ca95f..782408c08a5e 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -82,6 +82,8 @@ static void vfio_pci_dma_buf_release(struct dma_buf *dmabuf) up_write(&priv->vdev->memory_lock); vfio_device_put_registration(&priv->vdev->vdev); }
- if (priv->vfile)
fput(priv->vfile);} @@ -222,6 +224,45 @@ int vfio_pci_dma_buf_find_pfn(struct vfio_pci_dma_buf *vpdmabuf, return -EFAULT; } +/*
- Create a DMABUF corresponding to priv, add it to vdev->dmabufs list
- for tracking (meaning cleanup or revocation will zap it), and take
- a vfio_device registration.
- */
+static int vfio_pci_dmabuf_export(struct vfio_pci_core_device *vdev,
struct vfio_pci_dma_buf *priv, uint32_t flags)
s/uint32_t/u32/?
+{
- DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
- if (!vfio_device_try_get_registration(&vdev->vdev))
return -ENODEV;- exp_info.ops = &vfio_pci_dmabuf_ops;
- exp_info.size = priv->size;
- exp_info.flags = flags;
- exp_info.priv = priv;
- priv->dmabuf = dma_buf_export(&exp_info);
- if (IS_ERR(priv->dmabuf)) {
vfio_device_put_registration(&vdev->vdev);return PTR_ERR(priv->dmabuf);- }
- kref_init(&priv->kref);
- init_completion(&priv->comp);
- /* dma_buf_put() now frees priv */
- INIT_LIST_HEAD(&priv->dmabufs_elm);
- down_write(&vdev->memory_lock);
- dma_resv_lock(priv->dmabuf->resv, NULL);
- priv->revoked = !__vfio_pci_memory_enabled(vdev);
- list_add_tail(&priv->dmabufs_elm, &vdev->dmabufs);
- dma_resv_unlock(priv->dmabuf->resv);
- up_write(&vdev->memory_lock);
- return 0;
+}
/*
- This is a temporary "private interconnect" between VFIO DMABUF and iommufd.
- It allows the two co-operating drivers to exchange the physical address of
@@ -340,7 +381,6 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, { struct vfio_device_feature_dma_buf get_dma_buf = {}; struct vfio_region_dma_range *dma_ranges;
- DEFINE_DMA_BUF_EXPORT_INFO(exp_info); struct vfio_pci_dma_buf *priv; size_t length; int ret;
@@ -400,34 +440,9 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, kfree(dma_ranges); dma_ranges = NULL;
- if (!vfio_device_try_get_registration(&vdev->vdev)) {
ret = -ENODEV;
- ret = vfio_pci_dmabuf_export(vdev, priv, get_dma_buf.open_flags);
- if (ret) goto err_free_phys;
- }
- exp_info.ops = &vfio_pci_dmabuf_ops;
- exp_info.size = priv->size;
- exp_info.flags = get_dma_buf.open_flags;
- exp_info.priv = priv;
- priv->dmabuf = dma_buf_export(&exp_info);
- if (IS_ERR(priv->dmabuf)) {
ret = PTR_ERR(priv->dmabuf);goto err_dev_put;- }
- kref_init(&priv->kref);
- init_completion(&priv->comp);
- /* dma_buf_put() now frees priv */
- INIT_LIST_HEAD(&priv->dmabufs_elm);
- down_write(&vdev->memory_lock);
- dma_resv_lock(priv->dmabuf->resv, NULL);
- priv->revoked = !__vfio_pci_memory_enabled(vdev);
- list_add_tail(&priv->dmabufs_elm, &vdev->dmabufs);
- dma_resv_unlock(priv->dmabuf->resv);
- up_write(&vdev->memory_lock);
- /*
- dma_buf_fd() consumes the reference, when the file closes the dmabuf
- will be released.
@@ -438,8 +453,6 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, return ret; -err_dev_put:
- vfio_device_put_registration(&vdev->vdev);
err_free_phys: kfree(priv->phys_vec); err_free_priv: @@ -449,6 +462,73 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, return ret; } +int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev,
struct vm_area_struct *vma,u64 phys_start, u64 req_len,unsigned int res_index)+{
- struct vfio_pci_dma_buf *priv;
- const unsigned int nr_ranges = 1;
Why, versus priv->nr_ranges = 1; below? Thanks,
Alex
- unsigned long vma_pgoff = vma->vm_pgoff & (VFIO_PCI_OFFSET_MASK >> PAGE_SHIFT);
- int ret;
- priv = kzalloc_obj(*priv);
- if (!priv)
return -ENOMEM;- priv->phys_vec = kzalloc_obj(*priv->phys_vec);
- if (!priv->phys_vec) {
ret = -ENOMEM;goto err_free_priv;- }
- /*
* The DMABUF begins from the mmap()'s BAR offset, i.e. the* start of the VMA corresponds to byte 0 of the DMABUF and* byte (vma_pgoff << PAGE_SHIFT) of the BAR.** vfio_pci_dma_buf_find_pfn() reverses this offset using* vma_pgoff_adjust, so that ultimately a fault's offset from* the start of the _VMA_ has a consistent usage whether the* VMA originates from an mmap() of the VFIO device here or a* direct DMABUF mmap().*/- priv->vdev = vdev;
- priv->size = req_len;
- priv->nr_ranges = nr_ranges;
- priv->vma_pgoff_adjust = vma_pgoff;
- priv->provider = pcim_p2pdma_provider(vdev->pdev, res_index);
- if (!priv->provider) {
ret = -EINVAL;goto err_free_phys;- }
- priv->phys_vec[0].paddr = phys_start + ((u64)vma_pgoff << PAGE_SHIFT);
- priv->phys_vec[0].len = priv->size;
- ret = vfio_pci_dmabuf_export(vdev, priv, O_CLOEXEC | O_RDWR);
- if (ret)
goto err_free_phys;- /*
* The VMA gets the DMABUF file so that other users can locate* the DMABUF via a VA. Ownership of the original VFIO device* file being mmap()ed transfers to priv, and is put when the* DMABUF is released.*/- priv->vfile = vma->vm_file;
- vma->vm_file = priv->dmabuf->file;
- vma->vm_private_data = priv;
- return 0;
+err_free_phys:
- kfree(priv->phys_vec);
+err_free_priv:
- kfree(priv);
- return ret;
+}
void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked) { struct vfio_pci_dma_buf *priv; diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index c8f6f959056a..06dc0fd3e230 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -30,6 +30,7 @@ struct vfio_pci_dma_buf { size_t size; struct phys_vec *phys_vec; struct p2pdma_provider *provider;
- struct file *vfile; u32 nr_ranges; struct kref kref; struct completion comp;
@@ -133,6 +134,10 @@ int vfio_pci_dma_buf_find_pfn(struct vfio_pci_dma_buf *vpdmabuf, unsigned long address, unsigned int order, unsigned long *out_pfn); +int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev,
struct vm_area_struct *vma,u64 phys_start, u64 req_len,unsigned int res_index);#ifdef CONFIG_VFIO_PCI_DMABUF int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
Hi Alex,
On 27/05/2026 23:59, Alex Williamson wrote:
On Wed, 27 May 2026 03:23:06 -0700 Matt Evans mattev@meta.com wrote:
This helper, vfio_pci_core_mmap_prep_dmabuf(), creates a single-range DMABUF for the purpose of mapping a PCI BAR. This is used in a future commit by VFIO's ordinary mmap() path.
This function transfers ownership of the VFIO device fd to the DMABUF, which fput()s when it's released.
Refactor the existing vfio_pci_core_feature_dma_buf() to split out export code common to the two paths, VFIO_DEVICE_FEATURE_DMA_BUF and this new VFIO_BAR mmap().
Signed-off-by: Matt Evans mattev@meta.com
drivers/vfio/pci/vfio_pci_dmabuf.c | 140 ++++++++++++++++++++++------- drivers/vfio/pci/vfio_pci_priv.h | 5 ++ 2 files changed, 115 insertions(+), 30 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index 0d132c4ca95f..782408c08a5e 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -82,6 +82,8 @@ static void vfio_pci_dma_buf_release(struct dma_buf *dmabuf) up_write(&priv->vdev->memory_lock); vfio_device_put_registration(&priv->vdev->vdev); }
- if (priv->vfile)
fput(priv->vfile);@@ -222,6 +224,45 @@ int vfio_pci_dma_buf_find_pfn(struct vfio_pci_dma_buf *vpdmabuf, return -EFAULT; } +/*
- Create a DMABUF corresponding to priv, add it to vdev->dmabufs list
- for tracking (meaning cleanup or revocation will zap it), and take
- a vfio_device registration.
- */
+static int vfio_pci_dmabuf_export(struct vfio_pci_core_device *vdev,
struct vfio_pci_dma_buf *priv, uint32_t flags)s/uint32_t/u32/?
Fixed.
+{
- DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
- if (!vfio_device_try_get_registration(&vdev->vdev))
return -ENODEV;- exp_info.ops = &vfio_pci_dmabuf_ops;
- exp_info.size = priv->size;
- exp_info.flags = flags;
- exp_info.priv = priv;
- priv->dmabuf = dma_buf_export(&exp_info);
- if (IS_ERR(priv->dmabuf)) {
vfio_device_put_registration(&vdev->vdev);return PTR_ERR(priv->dmabuf);- }
- kref_init(&priv->kref);
- init_completion(&priv->comp);
- /* dma_buf_put() now frees priv */
- INIT_LIST_HEAD(&priv->dmabufs_elm);
- down_write(&vdev->memory_lock);
- dma_resv_lock(priv->dmabuf->resv, NULL);
- priv->revoked = !__vfio_pci_memory_enabled(vdev);
- list_add_tail(&priv->dmabufs_elm, &vdev->dmabufs);
- dma_resv_unlock(priv->dmabuf->resv);
- up_write(&vdev->memory_lock);
- return 0;
+}
- /*
- This is a temporary "private interconnect" between VFIO DMABUF and iommufd.
- It allows the two co-operating drivers to exchange the physical address of
@@ -340,7 +381,6 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, { struct vfio_device_feature_dma_buf get_dma_buf = {}; struct vfio_region_dma_range *dma_ranges;
- DEFINE_DMA_BUF_EXPORT_INFO(exp_info); struct vfio_pci_dma_buf *priv; size_t length; int ret;
@@ -400,34 +440,9 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, kfree(dma_ranges); dma_ranges = NULL;
- if (!vfio_device_try_get_registration(&vdev->vdev)) {
ret = -ENODEV;
- ret = vfio_pci_dmabuf_export(vdev, priv, get_dma_buf.open_flags);
- if (ret) goto err_free_phys;
- }
- exp_info.ops = &vfio_pci_dmabuf_ops;
- exp_info.size = priv->size;
- exp_info.flags = get_dma_buf.open_flags;
- exp_info.priv = priv;
- priv->dmabuf = dma_buf_export(&exp_info);
- if (IS_ERR(priv->dmabuf)) {
ret = PTR_ERR(priv->dmabuf);goto err_dev_put;- }
- kref_init(&priv->kref);
- init_completion(&priv->comp);
- /* dma_buf_put() now frees priv */
- INIT_LIST_HEAD(&priv->dmabufs_elm);
- down_write(&vdev->memory_lock);
- dma_resv_lock(priv->dmabuf->resv, NULL);
- priv->revoked = !__vfio_pci_memory_enabled(vdev);
- list_add_tail(&priv->dmabufs_elm, &vdev->dmabufs);
- dma_resv_unlock(priv->dmabuf->resv);
- up_write(&vdev->memory_lock);
- /*
- dma_buf_fd() consumes the reference, when the file closes the dmabuf
- will be released.
@@ -438,8 +453,6 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, return ret; -err_dev_put:
- vfio_device_put_registration(&vdev->vdev); err_free_phys: kfree(priv->phys_vec); err_free_priv:
@@ -449,6 +462,73 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, return ret; } +int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev,
struct vm_area_struct *vma,u64 phys_start, u64 req_len,unsigned int res_index)+{
- struct vfio_pci_dma_buf *priv;
- const unsigned int nr_ranges = 1;
Why, versus priv->nr_ranges = 1; below? Thanks,
Hm, a vestige from a simpler time when it was a different shape and unnecessary now, fixed as per suggestion.
Thanks,
Matt
Alex
- unsigned long vma_pgoff = vma->vm_pgoff & (VFIO_PCI_OFFSET_MASK >> PAGE_SHIFT);
- int ret;
- priv = kzalloc_obj(*priv);
- if (!priv)
return -ENOMEM;- priv->phys_vec = kzalloc_obj(*priv->phys_vec);
- if (!priv->phys_vec) {
ret = -ENOMEM;goto err_free_priv;- }
- /*
* The DMABUF begins from the mmap()'s BAR offset, i.e. the* start of the VMA corresponds to byte 0 of the DMABUF and* byte (vma_pgoff << PAGE_SHIFT) of the BAR.** vfio_pci_dma_buf_find_pfn() reverses this offset using* vma_pgoff_adjust, so that ultimately a fault's offset from* the start of the _VMA_ has a consistent usage whether the* VMA originates from an mmap() of the VFIO device here or a* direct DMABUF mmap().*/- priv->vdev = vdev;
- priv->size = req_len;
- priv->nr_ranges = nr_ranges;
- priv->vma_pgoff_adjust = vma_pgoff;
- priv->provider = pcim_p2pdma_provider(vdev->pdev, res_index);
- if (!priv->provider) {
ret = -EINVAL;goto err_free_phys;- }
- priv->phys_vec[0].paddr = phys_start + ((u64)vma_pgoff << PAGE_SHIFT);
- priv->phys_vec[0].len = priv->size;
- ret = vfio_pci_dmabuf_export(vdev, priv, O_CLOEXEC | O_RDWR);
- if (ret)
goto err_free_phys;- /*
* The VMA gets the DMABUF file so that other users can locate* the DMABUF via a VA. Ownership of the original VFIO device* file being mmap()ed transfers to priv, and is put when the* DMABUF is released.*/- priv->vfile = vma->vm_file;
- vma->vm_file = priv->dmabuf->file;
- vma->vm_private_data = priv;
- return 0;
+err_free_phys:
- kfree(priv->phys_vec);
+err_free_priv:
- kfree(priv);
- return ret;
+}
- void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked) { struct vfio_pci_dma_buf *priv;
diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index c8f6f959056a..06dc0fd3e230 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -30,6 +30,7 @@ struct vfio_pci_dma_buf { size_t size; struct phys_vec *phys_vec; struct p2pdma_provider *provider;
- struct file *vfile; u32 nr_ranges; struct kref kref; struct completion comp;
@@ -133,6 +134,10 @@ int vfio_pci_dma_buf_find_pfn(struct vfio_pci_dma_buf *vpdmabuf, unsigned long address, unsigned int order, unsigned long *out_pfn); +int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev,
struct vm_area_struct *vma,u64 phys_start, u64 req_len,unsigned int res_index);#ifdef CONFIG_VFIO_PCI_DMABUF int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags,
Convert the VFIO device fd fops->mmap to create a DMABUF representing the BAR mapping, and make the VMA fault handler look up PFNs from the corresponding DMABUF. This supports future code mmap()ing BAR DMABUFs, and iommufd work to support Type1 P2P.
First, vfio_pci_core_mmap() uses the new vfio_pci_core_mmap_prep_dmabuf() helper to export a DMABUF representing a single BAR range. Then, the vfio_pci_mmap_huge_fault() callback is updated to understand revoked buffers, and uses the new vfio_pci_dma_buf_find_pfn() helper to determine the PFN for a given fault address.
Now that the VFIO DMABUFs can be mmap()ed, vfio_pci_dma_buf_move() zaps PTEs (used on the revocation and cleanup paths).
CONFIG_VFIO_PCI_CORE now unconditionally depends on CONFIG_DMA_SHARED_BUFFER and CONFIG_PCI_P2PDMA_CORE. The CONFIG_VFIO_PCI_DMABUF feature conditionally includes support for VFIO_DEVICE_FEATURE_DMA_BUF, depending on the availability of CONFIG_PCI_P2PDMA.
Signed-off-by: Matt Evans mattev@meta.com --- drivers/vfio/pci/Kconfig | 4 +- drivers/vfio/pci/Makefile | 3 +- drivers/vfio/pci/vfio_pci_core.c | 79 +++++++++++++++++++----------- drivers/vfio/pci/vfio_pci_dmabuf.c | 12 +++++ drivers/vfio/pci/vfio_pci_priv.h | 11 +---- 5 files changed, 68 insertions(+), 41 deletions(-)
diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig index 296bf01e185e..9197343a7301 100644 --- a/drivers/vfio/pci/Kconfig +++ b/drivers/vfio/pci/Kconfig @@ -6,6 +6,8 @@ config VFIO_PCI_CORE tristate select VFIO_VIRQFD select IRQ_BYPASS_MANAGER + select PCI_P2PDMA_CORE + select DMA_SHARED_BUFFER
config VFIO_PCI_INTX def_bool y if !S390 @@ -56,7 +58,7 @@ config VFIO_PCI_ZDEV_KVM To enable s390x KVM vfio-pci extensions, say Y.
config VFIO_PCI_DMABUF - def_bool y if VFIO_PCI_CORE && PCI_P2PDMA && DMA_SHARED_BUFFER + def_bool y if PCI_P2PDMA
source "drivers/vfio/pci/mlx5/Kconfig"
diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile index 6138f1bf241d..881452ea89be 100644 --- a/drivers/vfio/pci/Makefile +++ b/drivers/vfio/pci/Makefile @@ -1,8 +1,7 @@ # SPDX-License-Identifier: GPL-2.0-only
-vfio-pci-core-y := vfio_pci_core.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o +vfio-pci-core-y := vfio_pci_core.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o vfio_pci_dmabuf.o vfio-pci-core-$(CONFIG_VFIO_PCI_ZDEV_KVM) += vfio_pci_zdev.o -vfio-pci-core-$(CONFIG_VFIO_PCI_DMABUF) += vfio_pci_dmabuf.o obj-$(CONFIG_VFIO_PCI_CORE) += vfio-pci-core.o
vfio-pci-y := vfio_pci.o diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index 041243a84d81..c5f934905ce0 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -1683,18 +1683,6 @@ void vfio_pci_memory_unlock_and_restore(struct vfio_pci_core_device *vdev, u16 c up_write(&vdev->memory_lock); }
-static unsigned long vma_to_pfn(struct vm_area_struct *vma) -{ - struct vfio_pci_core_device *vdev = vma->vm_private_data; - int index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT); - u64 pgoff; - - pgoff = vma->vm_pgoff & - ((1U << (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT)) - 1); - - return (pci_resource_start(vdev->pdev, index) >> PAGE_SHIFT) + pgoff; -} - vm_fault_t vfio_pci_vmf_insert_pfn(struct vfio_pci_core_device *vdev, struct vm_fault *vmf, unsigned long pfn, @@ -1722,23 +1710,42 @@ static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf, unsigned int order) { struct vm_area_struct *vma = vmf->vma; - struct vfio_pci_core_device *vdev = vma->vm_private_data; - unsigned long addr = vmf->address & ~((PAGE_SIZE << order) - 1); - unsigned long pgoff = (addr - vma->vm_start) >> PAGE_SHIFT; - unsigned long pfn = vma_to_pfn(vma) + pgoff; - vm_fault_t ret = VM_FAULT_FALLBACK; - - if (is_aligned_for_order(vma, addr, pfn, order)) { - scoped_guard(rwsem_read, &vdev->memory_lock) - ret = vfio_pci_vmf_insert_pfn(vdev, vmf, pfn, order); - } + struct vfio_pci_dma_buf *priv = vma->vm_private_data; + struct vfio_pci_core_device *vdev; + unsigned long pfn = 0; + vm_fault_t ret = VM_FAULT_SIGBUS;
- dev_dbg_ratelimited(&vdev->pdev->dev, - "%s(,order = %d) BAR %ld page offset 0x%lx: 0x%x\n", - __func__, order, - vma->vm_pgoff >> - (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT), - pgoff, (unsigned int)ret); + /* + * We can rely on the existence of both a DMABUF (priv) and + * the VFIO device it was exported from (vdev). This fault's + * VMA was established using vfio_pci_core_mmap_prep_dmabuf() + * which transfers ownership of the VFIO device fd to the + * DMABUF, and so the VFIO device is held open because the + * VMA's vm_file (DMABUF) is open. + * + * Since vfio_pci_dma_buf_cleanup() cannot have happened, + * vdev must be valid; we can take memory_lock. + */ + vdev = READ_ONCE(priv->vdev); + + scoped_guard(rwsem_read, &vdev->memory_lock) { + if (!priv->revoked) { + int pres = vfio_pci_dma_buf_find_pfn(priv, vma, + vmf->address, + order, &pfn); + + if (pres == 0) + ret = vfio_pci_vmf_insert_pfn(vdev, vmf, + pfn, order); + else if (pres == -EAGAIN) + ret = VM_FAULT_FALLBACK; + } + + dev_dbg_ratelimited(&vdev->pdev->dev, + "%s(order = %d) PFN 0x%lx, VA 0x%lx, pgoff 0x%lx: 0x%x\n", + __func__, order, pfn, vmf->address, + vma->vm_pgoff, (unsigned int)ret); + }
return ret; } @@ -1763,6 +1770,7 @@ int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma unsigned int index; u64 phys_len, req_len, pgoff, req_start; void __iomem *bar_io; + int ret;
index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT);
@@ -1802,7 +1810,20 @@ int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma if (IS_ERR(bar_io)) return PTR_ERR(bar_io);
- vma->vm_private_data = vdev; + /* + * Create a DMABUF with a single range corresponding to this + * mapping, and wire it into vma->vm_private_data. The VMA's + * vm_file becomes that of the DMABUF, and the DMABUF takes + * ownership of the VFIO device file (put upon DMABUF + * release). This maintains the behaviour of a live VMA + * mapping holding the VFIO device file open. + */ + ret = vfio_pci_core_mmap_prep_dmabuf(vdev, vma, + pci_resource_start(pdev, index), + req_len, index); + if (ret) + return ret; + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index 782408c08a5e..f7797f58d44b 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -9,6 +9,7 @@
MODULE_IMPORT_NS("DMA_BUF");
+#ifdef CONFIG_VFIO_PCI_DMABUF static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf, struct dma_buf_attachment *attachment) { @@ -25,6 +26,7 @@ static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf,
return 0; } +#endif /* CONFIG_VFIO_PCI_DMABUF */
static void vfio_pci_dma_buf_done(struct kref *kref) { @@ -89,7 +91,9 @@ static void vfio_pci_dma_buf_release(struct dma_buf *dmabuf) }
static const struct dma_buf_ops vfio_pci_dmabuf_ops = { +#ifdef CONFIG_VFIO_PCI_DMABUF .attach = vfio_pci_dma_buf_attach, +#endif .map_dma_buf = vfio_pci_dma_buf_map, .unmap_dma_buf = vfio_pci_dma_buf_unmap, .release = vfio_pci_dma_buf_release, @@ -263,6 +267,7 @@ static int vfio_pci_dmabuf_export(struct vfio_pci_core_device *vdev, return 0; }
+#ifdef CONFIG_VFIO_PCI_DMABUF /* * This is a temporary "private interconnect" between VFIO DMABUF and iommufd. * It allows the two co-operating drivers to exchange the physical address of @@ -461,6 +466,7 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, kfree(dma_ranges); return ret; } +#endif /* CONFIG_VFIO_PCI_DMABUF */
int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev, struct vm_area_struct *vma, @@ -535,6 +541,10 @@ void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked) struct vfio_pci_dma_buf *tmp;
lockdep_assert_held_write(&vdev->memory_lock); + /* + * Holding memory_lock ensures a racing VMA fault observes + * priv->revoked properly. + */
list_for_each_entry_safe(priv, tmp, &vdev->dmabufs, dmabufs_elm) { if (!get_file_active(&priv->dmabuf->file)) @@ -552,6 +562,8 @@ void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked) if (revoked) { kref_put(&priv->kref, vfio_pci_dma_buf_done); wait_for_completion(&priv->comp); + unmap_mapping_range(priv->dmabuf->file->f_mapping, + 0, priv->size, 1); /* * Re-arm the registered kref reference and the * completion so the post-revoke state matches the diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index 06dc0fd3e230..d38e1b98b2e9 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -138,13 +138,13 @@ int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev, struct vm_area_struct *vma, u64 phys_start, u64 req_len, unsigned int res_index); +void vfio_pci_dma_buf_cleanup(struct vfio_pci_core_device *vdev); +void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked);
#ifdef CONFIG_VFIO_PCI_DMABUF int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, struct vfio_device_feature_dma_buf __user *arg, size_t argsz); -void vfio_pci_dma_buf_cleanup(struct vfio_pci_core_device *vdev); -void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked); #else static inline int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, @@ -153,13 +153,6 @@ vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, { return -ENOTTY; } -static inline void vfio_pci_dma_buf_cleanup(struct vfio_pci_core_device *vdev) -{ -} -static inline void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, - bool revoked) -{ -} #endif
#endif
On Wed, 27 May 2026 03:23:07 -0700 Matt Evans mattev@meta.com wrote:
Convert the VFIO device fd fops->mmap to create a DMABUF representing the BAR mapping, and make the VMA fault handler look up PFNs from the corresponding DMABUF. This supports future code mmap()ing BAR DMABUFs, and iommufd work to support Type1 P2P.
First, vfio_pci_core_mmap() uses the new vfio_pci_core_mmap_prep_dmabuf() helper to export a DMABUF representing a single BAR range. Then, the vfio_pci_mmap_huge_fault() callback is updated to understand revoked buffers, and uses the new vfio_pci_dma_buf_find_pfn() helper to determine the PFN for a given fault address.
Now that the VFIO DMABUFs can be mmap()ed, vfio_pci_dma_buf_move() zaps PTEs (used on the revocation and cleanup paths).
CONFIG_VFIO_PCI_CORE now unconditionally depends on CONFIG_DMA_SHARED_BUFFER and CONFIG_PCI_P2PDMA_CORE. The CONFIG_VFIO_PCI_DMABUF feature conditionally includes support for VFIO_DEVICE_FEATURE_DMA_BUF, depending on the availability of CONFIG_PCI_P2PDMA.
Signed-off-by: Matt Evans mattev@meta.com
drivers/vfio/pci/Kconfig | 4 +- drivers/vfio/pci/Makefile | 3 +- drivers/vfio/pci/vfio_pci_core.c | 79 +++++++++++++++++++----------- drivers/vfio/pci/vfio_pci_dmabuf.c | 12 +++++ drivers/vfio/pci/vfio_pci_priv.h | 11 +---- 5 files changed, 68 insertions(+), 41 deletions(-)
diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig index 296bf01e185e..9197343a7301 100644 --- a/drivers/vfio/pci/Kconfig +++ b/drivers/vfio/pci/Kconfig @@ -6,6 +6,8 @@ config VFIO_PCI_CORE tristate select VFIO_VIRQFD select IRQ_BYPASS_MANAGER
- select PCI_P2PDMA_CORE
- select DMA_SHARED_BUFFER
config VFIO_PCI_INTX def_bool y if !S390 @@ -56,7 +58,7 @@ config VFIO_PCI_ZDEV_KVM To enable s390x KVM vfio-pci extensions, say Y. config VFIO_PCI_DMABUF
- def_bool y if VFIO_PCI_CORE && PCI_P2PDMA && DMA_SHARED_BUFFER
- def_bool y if PCI_P2PDMA
This largely only breaks consistency, but should VFIO_PCI_CORE become a 'depends on' rather than dropped entirely?
source "drivers/vfio/pci/mlx5/Kconfig" diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile index 6138f1bf241d..881452ea89be 100644 --- a/drivers/vfio/pci/Makefile +++ b/drivers/vfio/pci/Makefile @@ -1,8 +1,7 @@ # SPDX-License-Identifier: GPL-2.0-only -vfio-pci-core-y := vfio_pci_core.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o +vfio-pci-core-y := vfio_pci_core.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o vfio_pci_dmabuf.o vfio-pci-core-$(CONFIG_VFIO_PCI_ZDEV_KVM) += vfio_pci_zdev.o -vfio-pci-core-$(CONFIG_VFIO_PCI_DMABUF) += vfio_pci_dmabuf.o obj-$(CONFIG_VFIO_PCI_CORE) += vfio-pci-core.o vfio-pci-y := vfio_pci.o diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index 041243a84d81..c5f934905ce0 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -1683,18 +1683,6 @@ void vfio_pci_memory_unlock_and_restore(struct vfio_pci_core_device *vdev, u16 c up_write(&vdev->memory_lock); } -static unsigned long vma_to_pfn(struct vm_area_struct *vma) -{
- struct vfio_pci_core_device *vdev = vma->vm_private_data;
- int index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT);
- u64 pgoff;
- pgoff = vma->vm_pgoff &
((1U << (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT)) - 1);- return (pci_resource_start(vdev->pdev, index) >> PAGE_SHIFT) + pgoff;
-}
vm_fault_t vfio_pci_vmf_insert_pfn(struct vfio_pci_core_device *vdev, struct vm_fault *vmf, unsigned long pfn, @@ -1722,23 +1710,42 @@ static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf, unsigned int order) { struct vm_area_struct *vma = vmf->vma;
- struct vfio_pci_core_device *vdev = vma->vm_private_data;
- unsigned long addr = vmf->address & ~((PAGE_SIZE << order) - 1);
- unsigned long pgoff = (addr - vma->vm_start) >> PAGE_SHIFT;
- unsigned long pfn = vma_to_pfn(vma) + pgoff;
- vm_fault_t ret = VM_FAULT_FALLBACK;
- if (is_aligned_for_order(vma, addr, pfn, order)) {
scoped_guard(rwsem_read, &vdev->memory_lock)ret = vfio_pci_vmf_insert_pfn(vdev, vmf, pfn, order);- }
- struct vfio_pci_dma_buf *priv = vma->vm_private_data;
- struct vfio_pci_core_device *vdev;
- unsigned long pfn = 0;
- vm_fault_t ret = VM_FAULT_SIGBUS;
- dev_dbg_ratelimited(&vdev->pdev->dev,
"%s(,order = %d) BAR %ld page offset 0x%lx: 0x%x\n",__func__, order,vma->vm_pgoff >>(VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT),pgoff, (unsigned int)ret);
- /*
* We can rely on the existence of both a DMABUF (priv) and* the VFIO device it was exported from (vdev). This fault's* VMA was established using vfio_pci_core_mmap_prep_dmabuf()* which transfers ownership of the VFIO device fd to the* DMABUF, and so the VFIO device is held open because the* VMA's vm_file (DMABUF) is open.** Since vfio_pci_dma_buf_cleanup() cannot have happened,* vdev must be valid; we can take memory_lock.*/- vdev = READ_ONCE(priv->vdev);
The above comment argues that vdev is stable, so why do we need to access it with READ_ONCE()?
- scoped_guard(rwsem_read, &vdev->memory_lock) {
if (!priv->revoked) {int pres = vfio_pci_dma_buf_find_pfn(priv, vma,vmf->address,order, &pfn);if (pres == 0)ret = vfio_pci_vmf_insert_pfn(vdev, vmf,pfn, order);else if (pres == -EAGAIN)ret = VM_FAULT_FALLBACK;}dev_dbg_ratelimited(&vdev->pdev->dev,"%s(order = %d) PFN 0x%lx, VA 0x%lx, pgoff 0x%lx: 0x%x\n",__func__, order, pfn, vmf->address,vma->vm_pgoff, (unsigned int)ret);
Looks like this should still be outside the scope of the memory_lock. Thanks,
Alex
- }
return ret; } @@ -1763,6 +1770,7 @@ int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma unsigned int index; u64 phys_len, req_len, pgoff, req_start; void __iomem *bar_io;
- int ret;
index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT); @@ -1802,7 +1810,20 @@ int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma if (IS_ERR(bar_io)) return PTR_ERR(bar_io);
- vma->vm_private_data = vdev;
- /*
* Create a DMABUF with a single range corresponding to this* mapping, and wire it into vma->vm_private_data. The VMA's* vm_file becomes that of the DMABUF, and the DMABUF takes* ownership of the VFIO device file (put upon DMABUF* release). This maintains the behaviour of a live VMA* mapping holding the VFIO device file open.*/- ret = vfio_pci_core_mmap_prep_dmabuf(vdev, vma,
pci_resource_start(pdev, index),req_len, index);- if (ret)
return ret;- vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index 782408c08a5e..f7797f58d44b 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -9,6 +9,7 @@ MODULE_IMPORT_NS("DMA_BUF"); +#ifdef CONFIG_VFIO_PCI_DMABUF static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf, struct dma_buf_attachment *attachment) { @@ -25,6 +26,7 @@ static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf, return 0; } +#endif /* CONFIG_VFIO_PCI_DMABUF */ static void vfio_pci_dma_buf_done(struct kref *kref) { @@ -89,7 +91,9 @@ static void vfio_pci_dma_buf_release(struct dma_buf *dmabuf) } static const struct dma_buf_ops vfio_pci_dmabuf_ops = { +#ifdef CONFIG_VFIO_PCI_DMABUF .attach = vfio_pci_dma_buf_attach, +#endif .map_dma_buf = vfio_pci_dma_buf_map, .unmap_dma_buf = vfio_pci_dma_buf_unmap, .release = vfio_pci_dma_buf_release, @@ -263,6 +267,7 @@ static int vfio_pci_dmabuf_export(struct vfio_pci_core_device *vdev, return 0; } +#ifdef CONFIG_VFIO_PCI_DMABUF /*
- This is a temporary "private interconnect" between VFIO DMABUF and iommufd.
- It allows the two co-operating drivers to exchange the physical address of
@@ -461,6 +466,7 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, kfree(dma_ranges); return ret; } +#endif /* CONFIG_VFIO_PCI_DMABUF */ int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev, struct vm_area_struct *vma, @@ -535,6 +541,10 @@ void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked) struct vfio_pci_dma_buf *tmp; lockdep_assert_held_write(&vdev->memory_lock);
- /*
* Holding memory_lock ensures a racing VMA fault observes* priv->revoked properly.*/list_for_each_entry_safe(priv, tmp, &vdev->dmabufs, dmabufs_elm) { if (!get_file_active(&priv->dmabuf->file)) @@ -552,6 +562,8 @@ void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked) if (revoked) { kref_put(&priv->kref, vfio_pci_dma_buf_done); wait_for_completion(&priv->comp);
unmap_mapping_range(priv->dmabuf->file->f_mapping,0, priv->size, 1); /* * Re-arm the registered kref reference and the * completion so the post-revoke state matches thediff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index 06dc0fd3e230..d38e1b98b2e9 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -138,13 +138,13 @@ int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev, struct vm_area_struct *vma, u64 phys_start, u64 req_len, unsigned int res_index); +void vfio_pci_dma_buf_cleanup(struct vfio_pci_core_device *vdev); +void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked); #ifdef CONFIG_VFIO_PCI_DMABUF int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, struct vfio_device_feature_dma_buf __user *arg, size_t argsz); -void vfio_pci_dma_buf_cleanup(struct vfio_pci_core_device *vdev); -void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked); #else static inline int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, @@ -153,13 +153,6 @@ vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, { return -ENOTTY; } -static inline void vfio_pci_dma_buf_cleanup(struct vfio_pci_core_device *vdev) -{ -} -static inline void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev,
bool revoked)-{ -} #endif #endif
Hi Alex,
On 29/05/2026 00:15, Alex Williamson wrote:
On Wed, 27 May 2026 03:23:07 -0700 Matt Evans mattev@meta.com wrote:
Convert the VFIO device fd fops->mmap to create a DMABUF representing the BAR mapping, and make the VMA fault handler look up PFNs from the corresponding DMABUF. This supports future code mmap()ing BAR DMABUFs, and iommufd work to support Type1 P2P.
First, vfio_pci_core_mmap() uses the new vfio_pci_core_mmap_prep_dmabuf() helper to export a DMABUF representing a single BAR range. Then, the vfio_pci_mmap_huge_fault() callback is updated to understand revoked buffers, and uses the new vfio_pci_dma_buf_find_pfn() helper to determine the PFN for a given fault address.
Now that the VFIO DMABUFs can be mmap()ed, vfio_pci_dma_buf_move() zaps PTEs (used on the revocation and cleanup paths).
CONFIG_VFIO_PCI_CORE now unconditionally depends on CONFIG_DMA_SHARED_BUFFER and CONFIG_PCI_P2PDMA_CORE. The CONFIG_VFIO_PCI_DMABUF feature conditionally includes support for VFIO_DEVICE_FEATURE_DMA_BUF, depending on the availability of CONFIG_PCI_P2PDMA.
Signed-off-by: Matt Evans mattev@meta.com
drivers/vfio/pci/Kconfig | 4 +- drivers/vfio/pci/Makefile | 3 +- drivers/vfio/pci/vfio_pci_core.c | 79 +++++++++++++++++++----------- drivers/vfio/pci/vfio_pci_dmabuf.c | 12 +++++ drivers/vfio/pci/vfio_pci_priv.h | 11 +---- 5 files changed, 68 insertions(+), 41 deletions(-)
diff --git a/drivers/vfio/pci/Kconfig b/drivers/vfio/pci/Kconfig index 296bf01e185e..9197343a7301 100644 --- a/drivers/vfio/pci/Kconfig +++ b/drivers/vfio/pci/Kconfig @@ -6,6 +6,8 @@ config VFIO_PCI_CORE tristate select VFIO_VIRQFD select IRQ_BYPASS_MANAGER
- select PCI_P2PDMA_CORE
- select DMA_SHARED_BUFFER
config VFIO_PCI_INTX def_bool y if !S390 @@ -56,7 +58,7 @@ config VFIO_PCI_ZDEV_KVM To enable s390x KVM vfio-pci extensions, say Y. config VFIO_PCI_DMABUF
- def_bool y if VFIO_PCI_CORE && PCI_P2PDMA && DMA_SHARED_BUFFER
- def_bool y if PCI_P2PDMA
This largely only breaks consistency, but should VFIO_PCI_CORE become a 'depends on' rather than dropped entirely?
That makes more sense, avoids the file being built if !VFIO_PCI... Fixed.
source "drivers/vfio/pci/mlx5/Kconfig" diff --git a/drivers/vfio/pci/Makefile b/drivers/vfio/pci/Makefile index 6138f1bf241d..881452ea89be 100644 --- a/drivers/vfio/pci/Makefile +++ b/drivers/vfio/pci/Makefile @@ -1,8 +1,7 @@ # SPDX-License-Identifier: GPL-2.0-only -vfio-pci-core-y := vfio_pci_core.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o +vfio-pci-core-y := vfio_pci_core.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o vfio_pci_dmabuf.o vfio-pci-core-$(CONFIG_VFIO_PCI_ZDEV_KVM) += vfio_pci_zdev.o -vfio-pci-core-$(CONFIG_VFIO_PCI_DMABUF) += vfio_pci_dmabuf.o obj-$(CONFIG_VFIO_PCI_CORE) += vfio-pci-core.o vfio-pci-y := vfio_pci.o diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index 041243a84d81..c5f934905ce0 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -1683,18 +1683,6 @@ void vfio_pci_memory_unlock_and_restore(struct vfio_pci_core_device *vdev, u16 c up_write(&vdev->memory_lock); } -static unsigned long vma_to_pfn(struct vm_area_struct *vma) -{
- struct vfio_pci_core_device *vdev = vma->vm_private_data;
- int index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT);
- u64 pgoff;
- pgoff = vma->vm_pgoff &
((1U << (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT)) - 1);- return (pci_resource_start(vdev->pdev, index) >> PAGE_SHIFT) + pgoff;
-}
- vm_fault_t vfio_pci_vmf_insert_pfn(struct vfio_pci_core_device *vdev, struct vm_fault *vmf, unsigned long pfn,
@@ -1722,23 +1710,42 @@ static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf, unsigned int order) { struct vm_area_struct *vma = vmf->vma;
- struct vfio_pci_core_device *vdev = vma->vm_private_data;
- unsigned long addr = vmf->address & ~((PAGE_SIZE << order) - 1);
- unsigned long pgoff = (addr - vma->vm_start) >> PAGE_SHIFT;
- unsigned long pfn = vma_to_pfn(vma) + pgoff;
- vm_fault_t ret = VM_FAULT_FALLBACK;
- if (is_aligned_for_order(vma, addr, pfn, order)) {
scoped_guard(rwsem_read, &vdev->memory_lock)ret = vfio_pci_vmf_insert_pfn(vdev, vmf, pfn, order);- }
- struct vfio_pci_dma_buf *priv = vma->vm_private_data;
- struct vfio_pci_core_device *vdev;
- unsigned long pfn = 0;
- vm_fault_t ret = VM_FAULT_SIGBUS;
- dev_dbg_ratelimited(&vdev->pdev->dev,
"%s(,order = %d) BAR %ld page offset 0x%lx: 0x%x\n",__func__, order,vma->vm_pgoff >>(VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT),pgoff, (unsigned int)ret);
- /*
* We can rely on the existence of both a DMABUF (priv) and* the VFIO device it was exported from (vdev). This fault's* VMA was established using vfio_pci_core_mmap_prep_dmabuf()* which transfers ownership of the VFIO device fd to the* DMABUF, and so the VFIO device is held open because the* VMA's vm_file (DMABUF) is open.** Since vfio_pci_dma_buf_cleanup() cannot have happened,* vdev must be valid; we can take memory_lock.*/- vdev = READ_ONCE(priv->vdev);
The above comment argues that vdev is stable, so why do we need to access it with READ_ONCE()?
Fixed as of my reply to your review of [7/8].
- scoped_guard(rwsem_read, &vdev->memory_lock) {
if (!priv->revoked) {int pres = vfio_pci_dma_buf_find_pfn(priv, vma,vmf->address,order, &pfn);if (pres == 0)ret = vfio_pci_vmf_insert_pfn(vdev, vmf,pfn, order);else if (pres == -EAGAIN)ret = VM_FAULT_FALLBACK;}dev_dbg_ratelimited(&vdev->pdev->dev,"%s(order = %d) PFN 0x%lx, VA 0x%lx, pgoff 0x%lx: 0x%x\n",__func__, order, pfn, vmf->address,vma->vm_pgoff, (unsigned int)ret);Looks like this should still be outside the scope of the memory_lock.
Argh, I think I stuffed a rebase there, thanks for that. I should've caught that in my local review. Fixed!
(Reply flurry pausing now; still working on implementing the suggestion for "[PATCH v2 6/9] vfio/pci: Clean up BAR zap and revocation" and refactoring for "[PATCH v2 1/9] PCI/P2PDMA: Add CONFIG_PCI_P2PDMA_CORE", but will post a v3 in a day or two. As ever, thanks for the reviews.)
Matt
Thanks,
Alex
- }
return ret; } @@ -1763,6 +1770,7 @@ int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma unsigned int index; u64 phys_len, req_len, pgoff, req_start; void __iomem *bar_io;
- int ret;
index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT); @@ -1802,7 +1810,20 @@ int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct *vma if (IS_ERR(bar_io)) return PTR_ERR(bar_io);
- vma->vm_private_data = vdev;
- /*
* Create a DMABUF with a single range corresponding to this* mapping, and wire it into vma->vm_private_data. The VMA's* vm_file becomes that of the DMABUF, and the DMABUF takes* ownership of the VFIO device file (put upon DMABUF* release). This maintains the behaviour of a live VMA* mapping holding the VFIO device file open.*/- ret = vfio_pci_core_mmap_prep_dmabuf(vdev, vma,
pci_resource_start(pdev, index),req_len, index);- if (ret)
return ret;- vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index 782408c08a5e..f7797f58d44b 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -9,6 +9,7 @@ MODULE_IMPORT_NS("DMA_BUF"); +#ifdef CONFIG_VFIO_PCI_DMABUF static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf, struct dma_buf_attachment *attachment) { @@ -25,6 +26,7 @@ static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf, return 0; } +#endif /* CONFIG_VFIO_PCI_DMABUF */ static void vfio_pci_dma_buf_done(struct kref *kref) { @@ -89,7 +91,9 @@ static void vfio_pci_dma_buf_release(struct dma_buf *dmabuf) } static const struct dma_buf_ops vfio_pci_dmabuf_ops = { +#ifdef CONFIG_VFIO_PCI_DMABUF .attach = vfio_pci_dma_buf_attach, +#endif .map_dma_buf = vfio_pci_dma_buf_map, .unmap_dma_buf = vfio_pci_dma_buf_unmap, .release = vfio_pci_dma_buf_release, @@ -263,6 +267,7 @@ static int vfio_pci_dmabuf_export(struct vfio_pci_core_device *vdev, return 0; } +#ifdef CONFIG_VFIO_PCI_DMABUF /*
- This is a temporary "private interconnect" between VFIO DMABUF and iommufd.
- It allows the two co-operating drivers to exchange the physical address of
@@ -461,6 +466,7 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, kfree(dma_ranges); return ret; } +#endif /* CONFIG_VFIO_PCI_DMABUF */ int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev, struct vm_area_struct *vma, @@ -535,6 +541,10 @@ void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked) struct vfio_pci_dma_buf *tmp; lockdep_assert_held_write(&vdev->memory_lock);
- /*
* Holding memory_lock ensures a racing VMA fault observes* priv->revoked properly.*/list_for_each_entry_safe(priv, tmp, &vdev->dmabufs, dmabufs_elm) { if (!get_file_active(&priv->dmabuf->file)) @@ -552,6 +562,8 @@ void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked) if (revoked) { kref_put(&priv->kref, vfio_pci_dma_buf_done); wait_for_completion(&priv->comp);
unmap_mapping_range(priv->dmabuf->file->f_mapping,0, priv->size, 1); /* * Re-arm the registered kref reference and the * completion so the post-revoke state matches thediff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index 06dc0fd3e230..d38e1b98b2e9 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -138,13 +138,13 @@ int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev, struct vm_area_struct *vma, u64 phys_start, u64 req_len, unsigned int res_index); +void vfio_pci_dma_buf_cleanup(struct vfio_pci_core_device *vdev); +void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked); #ifdef CONFIG_VFIO_PCI_DMABUF int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, struct vfio_device_feature_dma_buf __user *arg, size_t argsz); -void vfio_pci_dma_buf_cleanup(struct vfio_pci_core_device *vdev); -void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked); #else static inline int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, @@ -153,13 +153,6 @@ vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, { return -ENOTTY; } -static inline void vfio_pci_dma_buf_cleanup(struct vfio_pci_core_device *vdev) -{ -} -static inline void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev,
bool revoked)-{ -} #endif #endif
Since converting BAR mmap()s to using DMABUFs, we lose the original device path in /proc/<pid>/maps, lsof, etc. Generate a debug-oriented synthetic 'filename' based on the cdev, plus BDF, plus resource index.
This applies only to BAR mappings via the VFIO device fd, as explicitly-exported DMABUFs are named by userspace via the DMA_BUF_SET_NAME ioctl.
Signed-off-by: Matt Evans mattev@meta.com --- drivers/vfio/pci/vfio_pci_dmabuf.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index f7797f58d44b..733607371082 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -4,6 +4,7 @@ #include <linux/dma-buf-mapping.h> #include <linux/pci-p2pdma.h> #include <linux/dma-resv.h> +#include <uapi/linux/dma-buf.h>
#include "vfio_pci_priv.h"
@@ -476,6 +477,7 @@ int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev, struct vfio_pci_dma_buf *priv; const unsigned int nr_ranges = 1; unsigned long vma_pgoff = vma->vm_pgoff & (VFIO_PCI_OFFSET_MASK >> PAGE_SHIFT); + char *bufname; int ret;
priv = kzalloc_obj(*priv); @@ -488,6 +490,20 @@ int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev, goto err_free_priv; }
+ bufname = kzalloc(DMA_BUF_NAME_LEN, GFP_KERNEL); + if (!bufname) { + ret = -ENOMEM; + goto err_free_phys; + } + + /* + * Maximum size of the friendly debug name is + * vfio1234567890:ffff:ff:3f.7/5 = 30, which fits within + * DMA_BUF_NAME_LEN. + */ + snprintf(bufname, DMA_BUF_NAME_LEN, "%s:%s/%x", + dev_name(&vdev->vdev.device), pci_name(vdev->pdev), res_index); + /* * The DMABUF begins from the mmap()'s BAR offset, i.e. the * start of the VMA corresponds to byte 0 of the DMABUF and @@ -506,7 +522,7 @@ int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev, priv->provider = pcim_p2pdma_provider(vdev->pdev, res_index); if (!priv->provider) { ret = -EINVAL; - goto err_free_phys; + goto err_free_name; }
priv->phys_vec[0].paddr = phys_start + ((u64)vma_pgoff << PAGE_SHIFT); @@ -514,7 +530,7 @@ int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev,
ret = vfio_pci_dmabuf_export(vdev, priv, O_CLOEXEC | O_RDWR); if (ret) - goto err_free_phys; + goto err_free_name;
/* * The VMA gets the DMABUF file so that other users can locate @@ -526,8 +542,15 @@ int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev, vma->vm_file = priv->dmabuf->file; vma->vm_private_data = priv;
+ spin_lock(&priv->dmabuf->name_lock); + kfree(priv->dmabuf->name); + priv->dmabuf->name = bufname; + spin_unlock(&priv->dmabuf->name_lock); + return 0;
+err_free_name: + kfree(bufname); err_free_phys: kfree(priv->phys_vec); err_free_priv:
Previously, vfio_pci_zap_bars() (and the wrapper vfio_pci_zap_and_down_write_memory_lock()) calls were paired with calls of vfio_pci_dma_buf_move().
This commit replaces them a unified new function, vfio_pci_zap_revoke_bars() containing both the vfio_pci_dma_buf_move() and the unmap_mapping_range(), making it harder for callers to omit one. It adds a wrapper, vfio_pci_lock_zap_revoke_bars(), which takes the write memory_lock before zapping, and adds a new vfio_pci_unrevoke_bars() for the re-enable path.
However, as of "vfio/pci: Convert BAR mmap() to use a DMABUF" the unmap_mapping_range() to zap is entirely redundant for plain vfio-pci, since the DMABUFs used for BAR mappings already zap PTEs when the vfio_pci_dma_buf_move() occurs.
One exception remains as a FIXME: in nvgrace-gpu, some BAR VMAs conditionally use custom vm_ops, which have not moved to be backed by DMABUFs. If these BARs are mmap()ed, the vdev enables the existing behaviour of unmap_mapping_range() for the device fd address space.
Signed-off-by: Matt Evans mattev@meta.com --- drivers/vfio/pci/nvgrace-gpu/main.c | 5 +++ drivers/vfio/pci/vfio_pci_config.c | 30 ++++++------- drivers/vfio/pci/vfio_pci_core.c | 66 ++++++++++++++++++++--------- drivers/vfio/pci/vfio_pci_priv.h | 3 +- include/linux/vfio_pci_core.h | 1 + 5 files changed, 66 insertions(+), 39 deletions(-)
diff --git a/drivers/vfio/pci/nvgrace-gpu/main.c b/drivers/vfio/pci/nvgrace-gpu/main.c index 15e2f03c6cd4..cfa649200a7f 100644 --- a/drivers/vfio/pci/nvgrace-gpu/main.c +++ b/drivers/vfio/pci/nvgrace-gpu/main.c @@ -364,6 +364,8 @@ static int nvgrace_gpu_mmap(struct vfio_device *core_vdev, struct nvgrace_gpu_pci_core_device *nvdev = container_of(core_vdev, struct nvgrace_gpu_pci_core_device, core_device.vdev); + struct vfio_pci_core_device *vdev = + container_of(core_vdev, struct vfio_pci_core_device, vdev); struct mem_region *memregion; u64 req_len, pgoff, end; unsigned int index; @@ -374,6 +376,9 @@ static int nvgrace_gpu_mmap(struct vfio_device *core_vdev, if (!memregion) return vfio_pci_core_mmap(core_vdev, vma);
+ /* Non-DMABUF BAR mappings need an extra zap */ + vdev->bar_needs_zap = true; + /* * Request to mmap the BAR. Map to the CPU accessible memory on the * GPU using the memory information gathered from the system ACPI diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c index a10ed733f0e3..8bfab0da481c 100644 --- a/drivers/vfio/pci/vfio_pci_config.c +++ b/drivers/vfio/pci/vfio_pci_config.c @@ -590,12 +590,10 @@ static int vfio_basic_config_write(struct vfio_pci_core_device *vdev, int pos, virt_mem = !!(le16_to_cpu(*virt_cmd) & PCI_COMMAND_MEMORY); new_mem = !!(new_cmd & PCI_COMMAND_MEMORY);
- if (!new_mem) { - vfio_pci_zap_and_down_write_memory_lock(vdev); - vfio_pci_dma_buf_move(vdev, true); - } else { + if (!new_mem) + vfio_pci_lock_zap_revoke_bars(vdev); + else down_write(&vdev->memory_lock); - }
/* * If the user is writing mem/io enable (new_mem/io) and we @@ -631,7 +629,7 @@ static int vfio_basic_config_write(struct vfio_pci_core_device *vdev, int pos, *virt_cmd |= cpu_to_le16(new_cmd & mask);
if (__vfio_pci_memory_enabled(vdev)) - vfio_pci_dma_buf_move(vdev, false); + vfio_pci_unrevoke_bars(vdev); up_write(&vdev->memory_lock); }
@@ -712,16 +710,14 @@ static int __init init_pci_cap_basic_perm(struct perm_bits *perm) static void vfio_lock_and_set_power_state(struct vfio_pci_core_device *vdev, pci_power_t state) { - if (state >= PCI_D3hot) { - vfio_pci_zap_and_down_write_memory_lock(vdev); - vfio_pci_dma_buf_move(vdev, true); - } else { + if (state >= PCI_D3hot) + vfio_pci_lock_zap_revoke_bars(vdev); + else down_write(&vdev->memory_lock); - }
vfio_pci_set_power_state(vdev, state); if (__vfio_pci_memory_enabled(vdev)) - vfio_pci_dma_buf_move(vdev, false); + vfio_pci_unrevoke_bars(vdev); up_write(&vdev->memory_lock); }
@@ -908,11 +904,10 @@ static int vfio_exp_config_write(struct vfio_pci_core_device *vdev, int pos, &cap);
if (!ret && (cap & PCI_EXP_DEVCAP_FLR)) { - vfio_pci_zap_and_down_write_memory_lock(vdev); - vfio_pci_dma_buf_move(vdev, true); + vfio_pci_lock_zap_revoke_bars(vdev); pci_try_reset_function(vdev->pdev); if (__vfio_pci_memory_enabled(vdev)) - vfio_pci_dma_buf_move(vdev, false); + vfio_pci_unrevoke_bars(vdev); up_write(&vdev->memory_lock); } } @@ -993,11 +988,10 @@ static int vfio_af_config_write(struct vfio_pci_core_device *vdev, int pos, &cap);
if (!ret && (cap & PCI_AF_CAP_FLR) && (cap & PCI_AF_CAP_TP)) { - vfio_pci_zap_and_down_write_memory_lock(vdev); - vfio_pci_dma_buf_move(vdev, true); + vfio_pci_lock_zap_revoke_bars(vdev); pci_try_reset_function(vdev->pdev); if (__vfio_pci_memory_enabled(vdev)) - vfio_pci_dma_buf_move(vdev, false); + vfio_pci_unrevoke_bars(vdev); up_write(&vdev->memory_lock); } } diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index c5f934905ce0..cfea59806a4f 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -319,8 +319,7 @@ static int vfio_pci_runtime_pm_entry(struct vfio_pci_core_device *vdev, * The vdev power related flags are protected with 'memory_lock' * semaphore. */ - vfio_pci_zap_and_down_write_memory_lock(vdev); - vfio_pci_dma_buf_move(vdev, true); + vfio_pci_lock_zap_revoke_bars(vdev);
if (vdev->pm_runtime_engaged) { up_write(&vdev->memory_lock); @@ -406,7 +405,7 @@ static void vfio_pci_runtime_pm_exit(struct vfio_pci_core_device *vdev) down_write(&vdev->memory_lock); __vfio_pci_runtime_pm_exit(vdev); if (__vfio_pci_memory_enabled(vdev)) - vfio_pci_dma_buf_move(vdev, false); + vfio_pci_unrevoke_bars(vdev); up_write(&vdev->memory_lock); }
@@ -1256,6 +1255,8 @@ static int vfio_pci_ioctl_set_irqs(struct vfio_pci_core_device *vdev, return ret; }
+static void vfio_pci_zap_revoke_bars(struct vfio_pci_core_device *vdev); + static int vfio_pci_ioctl_reset(struct vfio_pci_core_device *vdev, void __user *arg) { @@ -1264,7 +1265,7 @@ static int vfio_pci_ioctl_reset(struct vfio_pci_core_device *vdev, if (!vdev->reset_works) return -EINVAL;
- vfio_pci_zap_and_down_write_memory_lock(vdev); + down_write(&vdev->memory_lock);
/* * This function can be invoked while the power state is non-D0. If @@ -1277,10 +1278,11 @@ static int vfio_pci_ioctl_reset(struct vfio_pci_core_device *vdev, */ vfio_pci_set_power_state(vdev, PCI_D0);
- vfio_pci_dma_buf_move(vdev, true); + vfio_pci_zap_revoke_bars(vdev); + ret = pci_try_reset_function(vdev->pdev); if (__vfio_pci_memory_enabled(vdev)) - vfio_pci_dma_buf_move(vdev, false); + vfio_pci_unrevoke_bars(vdev); up_write(&vdev->memory_lock);
return ret; @@ -1648,20 +1650,44 @@ ssize_t vfio_pci_core_write(struct vfio_device *core_vdev, const char __user *bu } EXPORT_SYMBOL_GPL(vfio_pci_core_write);
-static void vfio_pci_zap_bars(struct vfio_pci_core_device *vdev) +static void vfio_pci_zap_revoke_bars(struct vfio_pci_core_device *vdev) { - struct vfio_device *core_vdev = &vdev->vdev; - loff_t start = VFIO_PCI_INDEX_TO_OFFSET(VFIO_PCI_BAR0_REGION_INDEX); - loff_t end = VFIO_PCI_INDEX_TO_OFFSET(VFIO_PCI_ROM_REGION_INDEX); - loff_t len = end - start; + lockdep_assert_held_write(&vdev->memory_lock); + vfio_pci_dma_buf_move(vdev, true);
- unmap_mapping_range(core_vdev->inode->i_mapping, start, len, true); + /* + * All VFIO PCI BARs are backed by DMABUFs, with the current + * exception of the nvgrace-gpu device which uses its own + * vm_ops for a subset of BARs. For this, BAR mappings are + * still made in the vdev's address_space, and a zap is + * required. The tracking is crude, and will (harmlessly) + * continue to zap if the special BAR is unmapped, but that + * behaviour isn't the common case. + * + * FIXME: This can go away if the special nvgrace-gpu mapping + * is converted to use DMABUF. + */ + if (vdev->bar_needs_zap) { + struct vfio_device *core_vdev = &vdev->vdev; + loff_t start = VFIO_PCI_INDEX_TO_OFFSET(VFIO_PCI_BAR0_REGION_INDEX); + loff_t end = VFIO_PCI_INDEX_TO_OFFSET(VFIO_PCI_ROM_REGION_INDEX); + loff_t len = end - start; + + unmap_mapping_range(core_vdev->inode->i_mapping, + start, len, true); + } }
-void vfio_pci_zap_and_down_write_memory_lock(struct vfio_pci_core_device *vdev) +void vfio_pci_lock_zap_revoke_bars(struct vfio_pci_core_device *vdev) { down_write(&vdev->memory_lock); - vfio_pci_zap_bars(vdev); + vfio_pci_zap_revoke_bars(vdev); +} + +void vfio_pci_unrevoke_bars(struct vfio_pci_core_device *vdev) +{ + lockdep_assert_held_write(&vdev->memory_lock); + vfio_pci_dma_buf_move(vdev, false); }
u16 vfio_pci_memory_lock_and_enable(struct vfio_pci_core_device *vdev) @@ -2517,9 +2543,10 @@ static int vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set, }
/* - * Take the memory write lock for each device and zap BAR - * mappings to prevent the user accessing the device while in - * reset. Locking multiple devices is prone to deadlock, + * Take the memory write lock for each device and + * zap/revoke BAR mappings to prevent the user (or + * peers) accessing the device while in reset. + * Locking multiple devices is prone to deadlock, * runaway and unwind if we hit contention. */ if (!down_write_trylock(&vdev->memory_lock)) { @@ -2527,8 +2554,7 @@ static int vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set, break; }
- vfio_pci_dma_buf_move(vdev, true); - vfio_pci_zap_bars(vdev); + vfio_pci_zap_revoke_bars(vdev); }
if (!list_entry_is_head(vdev, @@ -2558,7 +2584,7 @@ static int vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set, list_for_each_entry_from_reverse(vdev, &dev_set->device_list, vdev.dev_set_list) { if (vdev->vdev.open_count && __vfio_pci_memory_enabled(vdev)) - vfio_pci_dma_buf_move(vdev, false); + vfio_pci_unrevoke_bars(vdev); up_write(&vdev->memory_lock); }
diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index d38e1b98b2e9..10833aabd7fb 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -83,7 +83,8 @@ void vfio_config_free(struct vfio_pci_core_device *vdev); int vfio_pci_set_power_state(struct vfio_pci_core_device *vdev, pci_power_t state);
-void vfio_pci_zap_and_down_write_memory_lock(struct vfio_pci_core_device *vdev); +void vfio_pci_lock_zap_revoke_bars(struct vfio_pci_core_device *vdev); +void vfio_pci_unrevoke_bars(struct vfio_pci_core_device *vdev); u16 vfio_pci_memory_lock_and_enable(struct vfio_pci_core_device *vdev); void vfio_pci_memory_unlock_and_restore(struct vfio_pci_core_device *vdev, u16 cmd); diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h index 7accd0eac457..e35e82c24c8c 100644 --- a/include/linux/vfio_pci_core.h +++ b/include/linux/vfio_pci_core.h @@ -127,6 +127,7 @@ struct vfio_pci_core_device { bool needs_pm_restore:1; bool pm_intx_masked:1; bool pm_runtime_engaged:1; + bool bar_needs_zap:1; struct pci_saved_state *pci_saved_state; struct pci_saved_state *pm_save; int ioeventfds_nr;
On Wed, 27 May 2026 03:23:09 -0700 Matt Evans mattev@meta.com wrote:
Previously, vfio_pci_zap_bars() (and the wrapper vfio_pci_zap_and_down_write_memory_lock()) calls were paired with calls of vfio_pci_dma_buf_move().
This commit replaces them a unified new function, vfio_pci_zap_revoke_bars() containing both the vfio_pci_dma_buf_move() and the unmap_mapping_range(), making it harder for callers to omit one. It adds a wrapper, vfio_pci_lock_zap_revoke_bars(), which takes the write memory_lock before zapping, and adds a new vfio_pci_unrevoke_bars() for the re-enable path.
However, as of "vfio/pci: Convert BAR mmap() to use a DMABUF" the unmap_mapping_range() to zap is entirely redundant for plain vfio-pci, since the DMABUFs used for BAR mappings already zap PTEs when the vfio_pci_dma_buf_move() occurs.
One exception remains as a FIXME: in nvgrace-gpu, some BAR VMAs conditionally use custom vm_ops, which have not moved to be backed by DMABUFs. If these BARs are mmap()ed, the vdev enables the existing behaviour of unmap_mapping_range() for the device fd address space.
Signed-off-by: Matt Evans mattev@meta.com
drivers/vfio/pci/nvgrace-gpu/main.c | 5 +++ drivers/vfio/pci/vfio_pci_config.c | 30 ++++++------- drivers/vfio/pci/vfio_pci_core.c | 66 ++++++++++++++++++++--------- drivers/vfio/pci/vfio_pci_priv.h | 3 +- include/linux/vfio_pci_core.h | 1 + 5 files changed, 66 insertions(+), 39 deletions(-)
diff --git a/drivers/vfio/pci/nvgrace-gpu/main.c b/drivers/vfio/pci/nvgrace-gpu/main.c index 15e2f03c6cd4..cfa649200a7f 100644 --- a/drivers/vfio/pci/nvgrace-gpu/main.c +++ b/drivers/vfio/pci/nvgrace-gpu/main.c @@ -364,6 +364,8 @@ static int nvgrace_gpu_mmap(struct vfio_device *core_vdev, struct nvgrace_gpu_pci_core_device *nvdev = container_of(core_vdev, struct nvgrace_gpu_pci_core_device, core_device.vdev);
- struct vfio_pci_core_device *vdev =
container_of(core_vdev, struct vfio_pci_core_device, vdev);@@ -374,6 +376,9 @@ static int nvgrace_gpu_mmap(struct vfio_device *core_vdev, if (!memregion) return vfio_pci_core_mmap(core_vdev, vma);
- /* Non-DMABUF BAR mappings need an extra zap */
- vdev->bar_needs_zap = true;
This works, but it's subtle, and failing to opt-in is dangerous. The name, to me, also suggests some transient state of the device rather than an ongoing property. What if we used "zap_bars_on_revoke" and made vfio_pci_core_register_device() set this to true when:
if (vdev->vdev.ops->mmap != vfio_pci_core_mmap) vdev->zap_bars_on_revoke = true;
That gives us automatic opt-in to the safe default. The hisi_acc driver .mmap is just a wrapper to vfio_pci_core_mmap(), so it could manually opt-out:
ret = vfio_pci_core_register_device(&hisi_acc_vdev->core_device); if (ret) return ret;
/* mmaps are to the dmabuf, not the region */ hisi_acc_vdev->core_device.zap_bars_on_revoke = false;
We can't have the opt-out in vfio_pci_core_mmap() as drivers like nvgrace-gpu can use the core function for some BARs and their own handling for others. Thanks,
Alex
/* * Request to mmap the BAR. Map to the CPU accessible memory on the * GPU using the memory information gathered from the system ACPI diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c index a10ed733f0e3..8bfab0da481c 100644 --- a/drivers/vfio/pci/vfio_pci_config.c +++ b/drivers/vfio/pci/vfio_pci_config.c @@ -590,12 +590,10 @@ static int vfio_basic_config_write(struct vfio_pci_core_device *vdev, int pos, virt_mem = !!(le16_to_cpu(*virt_cmd) & PCI_COMMAND_MEMORY); new_mem = !!(new_cmd & PCI_COMMAND_MEMORY);
if (!new_mem) {vfio_pci_zap_and_down_write_memory_lock(vdev);vfio_pci_dma_buf_move(vdev, true);} else {
if (!new_mem)vfio_pci_lock_zap_revoke_bars(vdev);else down_write(&vdev->memory_lock);
}/* * If the user is writing mem/io enable (new_mem/io) and we @@ -631,7 +629,7 @@ static int vfio_basic_config_write(struct vfio_pci_core_device *vdev, int pos, *virt_cmd |= cpu_to_le16(new_cmd & mask); if (__vfio_pci_memory_enabled(vdev))
vfio_pci_dma_buf_move(vdev, false);
vfio_pci_unrevoke_bars(vdev);@@ -712,16 +710,14 @@ static int __init init_pci_cap_basic_perm(struct perm_bits *perm) static void vfio_lock_and_set_power_state(struct vfio_pci_core_device *vdev, pci_power_t state) {
- if (state >= PCI_D3hot) {
vfio_pci_zap_and_down_write_memory_lock(vdev);vfio_pci_dma_buf_move(vdev, true);- } else {
- if (state >= PCI_D3hot)
vfio_pci_lock_zap_revoke_bars(vdev);- else down_write(&vdev->memory_lock);
- }
vfio_pci_set_power_state(vdev, state); if (__vfio_pci_memory_enabled(vdev))
vfio_pci_dma_buf_move(vdev, false);
vfio_pci_unrevoke_bars(vdev);} @@ -908,11 +904,10 @@ static int vfio_exp_config_write(struct vfio_pci_core_device *vdev, int pos, &cap); if (!ret && (cap & PCI_EXP_DEVCAP_FLR)) {
vfio_pci_zap_and_down_write_memory_lock(vdev);vfio_pci_dma_buf_move(vdev, true);
vfio_pci_lock_zap_revoke_bars(vdev); pci_try_reset_function(vdev->pdev); if (__vfio_pci_memory_enabled(vdev))
vfio_pci_dma_buf_move(vdev, false);
vfio_pci_unrevoke_bars(vdev); up_write(&vdev->memory_lock);@@ -993,11 +988,10 @@ static int vfio_af_config_write(struct vfio_pci_core_device *vdev, int pos, &cap); if (!ret && (cap & PCI_AF_CAP_FLR) && (cap & PCI_AF_CAP_TP)) {
vfio_pci_zap_and_down_write_memory_lock(vdev);vfio_pci_dma_buf_move(vdev, true);
vfio_pci_lock_zap_revoke_bars(vdev); pci_try_reset_function(vdev->pdev); if (__vfio_pci_memory_enabled(vdev))
vfio_pci_dma_buf_move(vdev, false);
vfio_pci_unrevoke_bars(vdev); up_write(&vdev->memory_lock);diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index c5f934905ce0..cfea59806a4f 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -319,8 +319,7 @@ static int vfio_pci_runtime_pm_entry(struct vfio_pci_core_device *vdev, * The vdev power related flags are protected with 'memory_lock' * semaphore. */
- vfio_pci_zap_and_down_write_memory_lock(vdev);
- vfio_pci_dma_buf_move(vdev, true);
- vfio_pci_lock_zap_revoke_bars(vdev);
if (vdev->pm_runtime_engaged) { up_write(&vdev->memory_lock); @@ -406,7 +405,7 @@ static void vfio_pci_runtime_pm_exit(struct vfio_pci_core_device *vdev) down_write(&vdev->memory_lock); __vfio_pci_runtime_pm_exit(vdev); if (__vfio_pci_memory_enabled(vdev))
vfio_pci_dma_buf_move(vdev, false);
vfio_pci_unrevoke_bars(vdev);} @@ -1256,6 +1255,8 @@ static int vfio_pci_ioctl_set_irqs(struct vfio_pci_core_device *vdev, return ret; } +static void vfio_pci_zap_revoke_bars(struct vfio_pci_core_device *vdev);
static int vfio_pci_ioctl_reset(struct vfio_pci_core_device *vdev, void __user *arg) { @@ -1264,7 +1265,7 @@ static int vfio_pci_ioctl_reset(struct vfio_pci_core_device *vdev, if (!vdev->reset_works) return -EINVAL;
- vfio_pci_zap_and_down_write_memory_lock(vdev);
- down_write(&vdev->memory_lock);
/* * This function can be invoked while the power state is non-D0. If @@ -1277,10 +1278,11 @@ static int vfio_pci_ioctl_reset(struct vfio_pci_core_device *vdev, */ vfio_pci_set_power_state(vdev, PCI_D0);
- vfio_pci_dma_buf_move(vdev, true);
- vfio_pci_zap_revoke_bars(vdev);
- ret = pci_try_reset_function(vdev->pdev); if (__vfio_pci_memory_enabled(vdev))
vfio_pci_dma_buf_move(vdev, false);
vfio_pci_unrevoke_bars(vdev);return ret; @@ -1648,20 +1650,44 @@ ssize_t vfio_pci_core_write(struct vfio_device *core_vdev, const char __user *bu } EXPORT_SYMBOL_GPL(vfio_pci_core_write); -static void vfio_pci_zap_bars(struct vfio_pci_core_device *vdev) +static void vfio_pci_zap_revoke_bars(struct vfio_pci_core_device *vdev) {
- struct vfio_device *core_vdev = &vdev->vdev;
- loff_t start = VFIO_PCI_INDEX_TO_OFFSET(VFIO_PCI_BAR0_REGION_INDEX);
- loff_t end = VFIO_PCI_INDEX_TO_OFFSET(VFIO_PCI_ROM_REGION_INDEX);
- loff_t len = end - start;
- lockdep_assert_held_write(&vdev->memory_lock);
- vfio_pci_dma_buf_move(vdev, true);
- unmap_mapping_range(core_vdev->inode->i_mapping, start, len, true);
- /*
* All VFIO PCI BARs are backed by DMABUFs, with the current* exception of the nvgrace-gpu device which uses its own* vm_ops for a subset of BARs. For this, BAR mappings are* still made in the vdev's address_space, and a zap is* required. The tracking is crude, and will (harmlessly)* continue to zap if the special BAR is unmapped, but that* behaviour isn't the common case.** FIXME: This can go away if the special nvgrace-gpu mapping* is converted to use DMABUF.*/- if (vdev->bar_needs_zap) {
struct vfio_device *core_vdev = &vdev->vdev;loff_t start = VFIO_PCI_INDEX_TO_OFFSET(VFIO_PCI_BAR0_REGION_INDEX);loff_t end = VFIO_PCI_INDEX_TO_OFFSET(VFIO_PCI_ROM_REGION_INDEX);loff_t len = end - start;unmap_mapping_range(core_vdev->inode->i_mapping,start, len, true);- }
} -void vfio_pci_zap_and_down_write_memory_lock(struct vfio_pci_core_device *vdev) +void vfio_pci_lock_zap_revoke_bars(struct vfio_pci_core_device *vdev) { down_write(&vdev->memory_lock);
- vfio_pci_zap_bars(vdev);
- vfio_pci_zap_revoke_bars(vdev);
+}
+void vfio_pci_unrevoke_bars(struct vfio_pci_core_device *vdev) +{
- lockdep_assert_held_write(&vdev->memory_lock);
- vfio_pci_dma_buf_move(vdev, false);
} u16 vfio_pci_memory_lock_and_enable(struct vfio_pci_core_device *vdev) @@ -2517,9 +2543,10 @@ static int vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set, } /*
* Take the memory write lock for each device and zap BAR* mappings to prevent the user accessing the device while in* reset. Locking multiple devices is prone to deadlock,
* Take the memory write lock for each device and* zap/revoke BAR mappings to prevent the user (or* peers) accessing the device while in reset.* Locking multiple devices is prone to deadlock,*/ if (!down_write_trylock(&vdev->memory_lock)) {
- runaway and unwind if we hit contention.
@@ -2527,8 +2554,7 @@ static int vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set, break; }
vfio_pci_dma_buf_move(vdev, true);vfio_pci_zap_bars(vdev);
vfio_pci_zap_revoke_bars(vdev);if (!list_entry_is_head(vdev, @@ -2558,7 +2584,7 @@ static int vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set, list_for_each_entry_from_reverse(vdev, &dev_set->device_list, vdev.dev_set_list) { if (vdev->vdev.open_count && __vfio_pci_memory_enabled(vdev))
vfio_pci_dma_buf_move(vdev, false);
vfio_pci_unrevoke_bars(vdev);diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index d38e1b98b2e9..10833aabd7fb 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -83,7 +83,8 @@ void vfio_config_free(struct vfio_pci_core_device *vdev); int vfio_pci_set_power_state(struct vfio_pci_core_device *vdev, pci_power_t state); -void vfio_pci_zap_and_down_write_memory_lock(struct vfio_pci_core_device *vdev); +void vfio_pci_lock_zap_revoke_bars(struct vfio_pci_core_device *vdev); +void vfio_pci_unrevoke_bars(struct vfio_pci_core_device *vdev); u16 vfio_pci_memory_lock_and_enable(struct vfio_pci_core_device *vdev); void vfio_pci_memory_unlock_and_restore(struct vfio_pci_core_device *vdev, u16 cmd); diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h index 7accd0eac457..e35e82c24c8c 100644 --- a/include/linux/vfio_pci_core.h +++ b/include/linux/vfio_pci_core.h @@ -127,6 +127,7 @@ struct vfio_pci_core_device { bool needs_pm_restore:1; bool pm_intx_masked:1; bool pm_runtime_engaged:1;
- bool bar_needs_zap:1; struct pci_saved_state *pci_saved_state; struct pci_saved_state *pm_save; int ioeventfds_nr;
A VFIO DMABUF can export a subset of a BAR to userspace by fd; add support for mmap() of this fd. This provides another route for a process to map BARs, except one where the process can only map a specific subset of a BAR represented by the exported DMABUF.
mmap() support enables userspace driver designs that safely delegate access to BAR sub-ranges to other client processes by sharing a DMABUF fd, without having to share the (omnipotent) VFIO device fd with them.
Since the main VFIO BAR mmap() is now DMABUF-aware, this path reuses the existing vm_ops. But, since the lifecycle of an exported DMABUF is still decoupled from that of the device fd it came from, the device fd might now be closed concurrent with a VMA fault.
Extra synchronisation is added to deal with the possibility of a fault racing with the DMABUF cleanup path. (Note that this differs to a DMABUF implicitly created on the mmap() path, which holds ownership of the device fd and so prevents close-during-fault scenarios in order to maintain the same user-facing behaviour on close.) It does this by temporarily taking a VFIO device registration to ensure vdev remains valid, then vdev->memory_lock can be taken.
Signed-off-by: Matt Evans mattev@meta.com --- drivers/vfio/pci/vfio_pci_core.c | 79 ++++++++++++++++++++++++++---- drivers/vfio/pci/vfio_pci_dmabuf.c | 27 ++++++++++ drivers/vfio/pci/vfio_pci_priv.h | 2 + 3 files changed, 99 insertions(+), 9 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index cfea59806a4f..41e049fa9a8a 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -12,6 +12,8 @@
#include <linux/aperture.h> #include <linux/device.h> +#include <linux/dma-buf.h> +#include <linux/dma-resv.h> #include <linux/eventfd.h> #include <linux/file.h> #include <linux/interrupt.h> @@ -1742,19 +1744,77 @@ static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf, vm_fault_t ret = VM_FAULT_SIGBUS;
/* - * We can rely on the existence of both a DMABUF (priv) and - * the VFIO device it was exported from (vdev). This fault's - * VMA was established using vfio_pci_core_mmap_prep_dmabuf() - * which transfers ownership of the VFIO device fd to the - * DMABUF, and so the VFIO device is held open because the - * VMA's vm_file (DMABUF) is open. + * The only thing this can rely on is that the DMABUF relating + * to the VMA's vm_file exists (priv). * - * Since vfio_pci_dma_buf_cleanup() cannot have happened, - * vdev must be valid; we can take memory_lock. + * A DMABUF for a VFIO device fd mmap() holds a reference to + * the original VFIO device fd, but an explicitly-exported + * DMABUF does not. The original fd might have closed, + * meaning this fault can race with + * vfio_pci_dma_buf_cleanup(), meaning priv->vdev might be + * NULL, and the VFIO device registration might have been + * dropped. + * + * With the goal of taking vdev->memory_lock in a world where + * vdev might not still exist: + * + * 1. Take the resv lock on the DMABUF: + * - If racing cleanup got in first, the buffer is revoked; + * stop/exit if so. + * - If we got in first, the buffer is not revoked so vdev is + * non-NULL, accessible, and cleanup _has not yet put the + * VFIO device registration_. So, the device refcount must + * be >0. + * + * 2. Take vfio_device registration (refcount guaranteed >0 + * hereafter). + * + * 3. Unlock the DMABUF's resv lock: + * - A racing cleanup can now complete. + * - But, the device refcount >0, meaning the vfio_device + * (and vfio_pcie_core device vdev) have not yet been + * freed. vdev is accessible, even if the DMABUF has been + * revoked or cleanup has happened, because + * vfio_unregister_group_dev() can't complete. + * + * 4. Take the vdev->memory_lock + * - Either the DMABUF is usable, or has been cleaned up. + * Whichever, it can no longer change under us. + * - Test the DMABUF revocation status again: if it was + * revoked between 1 and 4 return a SIGBUS. Otherwise, + * return a PFN. + * - It's not necessary to also take the resv lock, because + * the status/vdev can't change while memory_lock is held. + * + * 5. Unlock, done. */ + + dma_resv_lock(priv->dmabuf->resv, NULL); vdev = READ_ONCE(priv->vdev);
+ if (priv->revoked || !vdev) { + pr_debug_ratelimited("%s VA 0x%lx, pgoff 0x%lx: DMABUF revoked/cleaned up\n", + __func__, vmf->address, vma->vm_pgoff); + dma_resv_unlock(priv->dmabuf->resv); + return VM_FAULT_SIGBUS; + } + /* vdev is usable */ + + if (!vfio_device_try_get_registration(&vdev->vdev)) { + /* + * If vdev != NULL (above), the registration should + * already be >0 and so this try_get should never + * fail. + */ + dev_warn(&vdev->pdev->dev, "%s: Unexpected registration failure\n", + __func__); + dma_resv_unlock(priv->dmabuf->resv); + return VM_FAULT_SIGBUS; + } + dma_resv_unlock(priv->dmabuf->resv); + scoped_guard(rwsem_read, &vdev->memory_lock) { + /* Revocation status must be re-read, under memory_lock */ if (!priv->revoked) { int pres = vfio_pci_dma_buf_find_pfn(priv, vma, vmf->address, @@ -1773,6 +1833,7 @@ static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf, vma->vm_pgoff, (unsigned int)ret); }
+ vfio_device_put_registration(&vdev->vdev); return ret; }
@@ -1781,7 +1842,7 @@ static vm_fault_t vfio_pci_mmap_page_fault(struct vm_fault *vmf) return vfio_pci_mmap_huge_fault(vmf, 0); }
-static const struct vm_operations_struct vfio_pci_mmap_ops = { +const struct vm_operations_struct vfio_pci_mmap_ops = { .fault = vfio_pci_mmap_page_fault, #ifdef CONFIG_ARCH_SUPPORTS_HUGE_PFNMAP .huge_fault = vfio_pci_mmap_huge_fault, diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index 733607371082..4b3b15655f1d 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -27,6 +27,32 @@ static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf,
return 0; } + +static int vfio_pci_dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma) +{ + struct vfio_pci_dma_buf *priv = dmabuf->priv; + + if (priv->revoked) + return -ENODEV; + if ((vma->vm_flags & VM_SHARED) == 0) + return -EINVAL; + + /* + * dma_buf_mmap_internal() has asserted that the VMA is + * contained within the DMABUF size before calling this. + */ + + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); + vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot); + + /* See comments in vfio_pci_core_mmap() re VM_ALLOW_ANY_UNCACHED. */ + vm_flags_set(vma, VM_ALLOW_ANY_UNCACHED | VM_IO | VM_PFNMAP | + VM_DONTEXPAND | VM_DONTDUMP); + vma->vm_private_data = priv; + vma->vm_ops = &vfio_pci_mmap_ops; + + return 0; +} #endif /* CONFIG_VFIO_PCI_DMABUF */
static void vfio_pci_dma_buf_done(struct kref *kref) @@ -94,6 +120,7 @@ static void vfio_pci_dma_buf_release(struct dma_buf *dmabuf) static const struct dma_buf_ops vfio_pci_dmabuf_ops = { #ifdef CONFIG_VFIO_PCI_DMABUF .attach = vfio_pci_dma_buf_attach, + .mmap = vfio_pci_dma_buf_mmap, #endif .map_dma_buf = vfio_pci_dma_buf_map, .unmap_dma_buf = vfio_pci_dma_buf_unmap, diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index 10833aabd7fb..db2e2aeae88f 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -38,6 +38,8 @@ struct vfio_pci_dma_buf { u8 revoked : 1; };
+extern const struct vm_operations_struct vfio_pci_mmap_ops; + bool vfio_pci_intx_mask(struct vfio_pci_core_device *vdev); void vfio_pci_intx_unmask(struct vfio_pci_core_device *vdev);
On Wed, 27 May 2026 03:23:10 -0700 Matt Evans mattev@meta.com wrote:
A VFIO DMABUF can export a subset of a BAR to userspace by fd; add support for mmap() of this fd. This provides another route for a process to map BARs, except one where the process can only map a specific subset of a BAR represented by the exported DMABUF.
mmap() support enables userspace driver designs that safely delegate access to BAR sub-ranges to other client processes by sharing a DMABUF fd, without having to share the (omnipotent) VFIO device fd with them.
Since the main VFIO BAR mmap() is now DMABUF-aware, this path reuses the existing vm_ops. But, since the lifecycle of an exported DMABUF is still decoupled from that of the device fd it came from, the device fd might now be closed concurrent with a VMA fault.
Extra synchronisation is added to deal with the possibility of a fault racing with the DMABUF cleanup path. (Note that this differs to a DMABUF implicitly created on the mmap() path, which holds ownership of the device fd and so prevents close-during-fault scenarios in order to maintain the same user-facing behaviour on close.) It does this by temporarily taking a VFIO device registration to ensure vdev remains valid, then vdev->memory_lock can be taken.
Suggest some general rewording of the commit log here, quite confusing.
Signed-off-by: Matt Evans mattev@meta.com
drivers/vfio/pci/vfio_pci_core.c | 79 ++++++++++++++++++++++++++---- drivers/vfio/pci/vfio_pci_dmabuf.c | 27 ++++++++++ drivers/vfio/pci/vfio_pci_priv.h | 2 + 3 files changed, 99 insertions(+), 9 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index cfea59806a4f..41e049fa9a8a 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -12,6 +12,8 @@ #include <linux/aperture.h> #include <linux/device.h> +#include <linux/dma-buf.h> +#include <linux/dma-resv.h> #include <linux/eventfd.h> #include <linux/file.h> #include <linux/interrupt.h> @@ -1742,19 +1744,77 @@ static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf, vm_fault_t ret = VM_FAULT_SIGBUS; /*
* We can rely on the existence of both a DMABUF (priv) and* the VFIO device it was exported from (vdev). This fault's* VMA was established using vfio_pci_core_mmap_prep_dmabuf()* which transfers ownership of the VFIO device fd to the* DMABUF, and so the VFIO device is held open because the* VMA's vm_file (DMABUF) is open.
* The only thing this can rely on is that the DMABUF relating* to the VMA's vm_file exists (priv).
* Since vfio_pci_dma_buf_cleanup() cannot have happened,* vdev must be valid; we can take memory_lock.
* A DMABUF for a VFIO device fd mmap() holds a reference to* the original VFIO device fd, but an explicitly-exported* DMABUF does not. The original fd might have closed,* meaning this fault can race with* vfio_pci_dma_buf_cleanup(), meaning priv->vdev might be* NULL, and the VFIO device registration might have been* dropped.** With the goal of taking vdev->memory_lock in a world where* vdev might not still exist:** 1. Take the resv lock on the DMABUF:* - If racing cleanup got in first, the buffer is revoked;* stop/exit if so.* - If we got in first, the buffer is not revoked so vdev is* non-NULL, accessible, and cleanup _has not yet put the* VFIO device registration_. So, the device refcount must* be >0.** 2. Take vfio_device registration (refcount guaranteed >0* hereafter).** 3. Unlock the DMABUF's resv lock:* - A racing cleanup can now complete.* - But, the device refcount >0, meaning the vfio_device* (and vfio_pcie_core device vdev) have not yet been* freed. vdev is accessible, even if the DMABUF has been* revoked or cleanup has happened, because* vfio_unregister_group_dev() can't complete.** 4. Take the vdev->memory_lock* - Either the DMABUF is usable, or has been cleaned up.* Whichever, it can no longer change under us.* - Test the DMABUF revocation status again: if it was* revoked between 1 and 4 return a SIGBUS. Otherwise,* return a PFN.* - It's not necessary to also take the resv lock, because* the status/vdev can't change while memory_lock is held.** 5. Unlock, done.- dma_resv_lock(priv->dmabuf->resv, NULL); vdev = READ_ONCE(priv->vdev);
I think you've again avoided the need for the READ_ONCE() by getting it under dma_resv_lock(), so it's still unnecessary.
- if (priv->revoked || !vdev) {
pr_debug_ratelimited("%s VA 0x%lx, pgoff 0x%lx: DMABUF revoked/cleaned up\n",__func__, vmf->address, vma->vm_pgoff);dma_resv_unlock(priv->dmabuf->resv);return VM_FAULT_SIGBUS;- }
- /* vdev is usable */
- if (!vfio_device_try_get_registration(&vdev->vdev)) {
/** If vdev != NULL (above), the registration should* already be >0 and so this try_get should never* fail.*/dev_warn(&vdev->pdev->dev, "%s: Unexpected registration failure\n",__func__);dma_resv_unlock(priv->dmabuf->resv);return VM_FAULT_SIGBUS;- }
- dma_resv_unlock(priv->dmabuf->resv);
- scoped_guard(rwsem_read, &vdev->memory_lock) {
/* Revocation status must be re-read, under memory_lock */@@ -1773,6 +1833,7 @@ static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf, vma->vm_pgoff, (unsigned int)ret); }
- vfio_device_put_registration(&vdev->vdev); return ret;
} @@ -1781,7 +1842,7 @@ static vm_fault_t vfio_pci_mmap_page_fault(struct vm_fault *vmf) return vfio_pci_mmap_huge_fault(vmf, 0); } -static const struct vm_operations_struct vfio_pci_mmap_ops = { +const struct vm_operations_struct vfio_pci_mmap_ops = { .fault = vfio_pci_mmap_page_fault, #ifdef CONFIG_ARCH_SUPPORTS_HUGE_PFNMAP .huge_fault = vfio_pci_mmap_huge_fault, diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index 733607371082..4b3b15655f1d 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -27,6 +27,32 @@ static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf, return 0; }
+static int vfio_pci_dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma) +{
- struct vfio_pci_dma_buf *priv = dmabuf->priv;
- if (priv->revoked)
return -ENODEV;
Questionable validity to testing revoked without a lock, but doesn't this also fail to follow the "map regardless, sort it out on fault" paradigm used elsewhere in vfio-pci? Thanks,
Alex
- if ((vma->vm_flags & VM_SHARED) == 0)
return -EINVAL;- /*
* dma_buf_mmap_internal() has asserted that the VMA is* contained within the DMABUF size before calling this.*/- vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
- vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
- /* See comments in vfio_pci_core_mmap() re VM_ALLOW_ANY_UNCACHED. */
- vm_flags_set(vma, VM_ALLOW_ANY_UNCACHED | VM_IO | VM_PFNMAP |
VM_DONTEXPAND | VM_DONTDUMP);- vma->vm_private_data = priv;
- vma->vm_ops = &vfio_pci_mmap_ops;
- return 0;
+} #endif /* CONFIG_VFIO_PCI_DMABUF */ static void vfio_pci_dma_buf_done(struct kref *kref) @@ -94,6 +120,7 @@ static void vfio_pci_dma_buf_release(struct dma_buf *dmabuf) static const struct dma_buf_ops vfio_pci_dmabuf_ops = { #ifdef CONFIG_VFIO_PCI_DMABUF .attach = vfio_pci_dma_buf_attach,
- .mmap = vfio_pci_dma_buf_mmap,
#endif .map_dma_buf = vfio_pci_dma_buf_map, .unmap_dma_buf = vfio_pci_dma_buf_unmap, diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index 10833aabd7fb..db2e2aeae88f 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -38,6 +38,8 @@ struct vfio_pci_dma_buf { u8 revoked : 1; }; +extern const struct vm_operations_struct vfio_pci_mmap_ops;
bool vfio_pci_intx_mask(struct vfio_pci_core_device *vdev); void vfio_pci_intx_unmask(struct vfio_pci_core_device *vdev);
Hi Alex,
On 29/05/2026 00:15, Alex Williamson wrote:
On Wed, 27 May 2026 03:23:10 -0700 Matt Evans mattev@meta.com wrote:
A VFIO DMABUF can export a subset of a BAR to userspace by fd; add support for mmap() of this fd. This provides another route for a process to map BARs, except one where the process can only map a specific subset of a BAR represented by the exported DMABUF.
mmap() support enables userspace driver designs that safely delegate access to BAR sub-ranges to other client processes by sharing a DMABUF fd, without having to share the (omnipotent) VFIO device fd with them.
Since the main VFIO BAR mmap() is now DMABUF-aware, this path reuses the existing vm_ops. But, since the lifecycle of an exported DMABUF is still decoupled from that of the device fd it came from, the device fd might now be closed concurrent with a VMA fault.
Extra synchronisation is added to deal with the possibility of a fault racing with the DMABUF cleanup path. (Note that this differs to a DMABUF implicitly created on the mmap() path, which holds ownership of the device fd and so prevents close-during-fault scenarios in order to maintain the same user-facing behaviour on close.) It does this by temporarily taking a VFIO device registration to ensure vdev remains valid, then vdev->memory_lock can be taken.
Suggest some general rewording of the commit log here, quite confusing.
OK!
Signed-off-by: Matt Evans mattev@meta.com
drivers/vfio/pci/vfio_pci_core.c | 79 ++++++++++++++++++++++++++---- drivers/vfio/pci/vfio_pci_dmabuf.c | 27 ++++++++++ drivers/vfio/pci/vfio_pci_priv.h | 2 + 3 files changed, 99 insertions(+), 9 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index cfea59806a4f..41e049fa9a8a 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -12,6 +12,8 @@ #include <linux/aperture.h> #include <linux/device.h> +#include <linux/dma-buf.h> +#include <linux/dma-resv.h> #include <linux/eventfd.h> #include <linux/file.h> #include <linux/interrupt.h> @@ -1742,19 +1744,77 @@ static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf, vm_fault_t ret = VM_FAULT_SIGBUS; /*
* We can rely on the existence of both a DMABUF (priv) and* the VFIO device it was exported from (vdev). This fault's* VMA was established using vfio_pci_core_mmap_prep_dmabuf()* which transfers ownership of the VFIO device fd to the* DMABUF, and so the VFIO device is held open because the* VMA's vm_file (DMABUF) is open.
* The only thing this can rely on is that the DMABUF relating* to the VMA's vm_file exists (priv).
* Since vfio_pci_dma_buf_cleanup() cannot have happened,* vdev must be valid; we can take memory_lock.
* A DMABUF for a VFIO device fd mmap() holds a reference to* the original VFIO device fd, but an explicitly-exported* DMABUF does not. The original fd might have closed,* meaning this fault can race with* vfio_pci_dma_buf_cleanup(), meaning priv->vdev might be* NULL, and the VFIO device registration might have been* dropped.** With the goal of taking vdev->memory_lock in a world where* vdev might not still exist:** 1. Take the resv lock on the DMABUF:* - If racing cleanup got in first, the buffer is revoked;* stop/exit if so.* - If we got in first, the buffer is not revoked so vdev is* non-NULL, accessible, and cleanup _has not yet put the* VFIO device registration_. So, the device refcount must* be >0.** 2. Take vfio_device registration (refcount guaranteed >0* hereafter).** 3. Unlock the DMABUF's resv lock:* - A racing cleanup can now complete.* - But, the device refcount >0, meaning the vfio_device* (and vfio_pcie_core device vdev) have not yet been* freed. vdev is accessible, even if the DMABUF has been* revoked or cleanup has happened, because* vfio_unregister_group_dev() can't complete.** 4. Take the vdev->memory_lock* - Either the DMABUF is usable, or has been cleaned up.* Whichever, it can no longer change under us.* - Test the DMABUF revocation status again: if it was* revoked between 1 and 4 return a SIGBUS. Otherwise,* return a PFN.* - It's not necessary to also take the resv lock, because* the status/vdev can't change while memory_lock is held.** 5. Unlock, done.- dma_resv_lock(priv->dmabuf->resv, NULL); vdev = READ_ONCE(priv->vdev);
I think you've again avoided the need for the READ_ONCE() by getting it under dma_resv_lock(), so it's still unnecessary.
Reviewed, you're right ofc. This originally went in when I was using a different approach to resolve the race. I've tweaked the comment and actually it can be further simplified as this !vdev test can be removed:
- if (priv->revoked || !vdev) {
pr_debug_ratelimited("%s VA 0x%lx, pgoff 0x%lx: DMABUF revoked/cleaned up\n",__func__, vmf->address, vma->vm_pgoff);dma_resv_unlock(priv->dmabuf->resv);return VM_FAULT_SIGBUS;- }
- /* vdev is usable */
...and (plain) vdev read moved here. If (holding the dma_resv_lock()) it's not revoked then vdev is usable/valid.
- if (!vfio_device_try_get_registration(&vdev->vdev)) {
/** If vdev != NULL (above), the registration should* already be >0 and so this try_get should never* fail.*/dev_warn(&vdev->pdev->dev, "%s: Unexpected registration failure\n",__func__);dma_resv_unlock(priv->dmabuf->resv);return VM_FAULT_SIGBUS;- }
- dma_resv_unlock(priv->dmabuf->resv);
- scoped_guard(rwsem_read, &vdev->memory_lock) {
/* Revocation status must be re-read, under memory_lock */@@ -1773,6 +1833,7 @@ static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf, vma->vm_pgoff, (unsigned int)ret); }
- vfio_device_put_registration(&vdev->vdev); return ret; }
@@ -1781,7 +1842,7 @@ static vm_fault_t vfio_pci_mmap_page_fault(struct vm_fault *vmf) return vfio_pci_mmap_huge_fault(vmf, 0); } -static const struct vm_operations_struct vfio_pci_mmap_ops = { +const struct vm_operations_struct vfio_pci_mmap_ops = { .fault = vfio_pci_mmap_page_fault, #ifdef CONFIG_ARCH_SUPPORTS_HUGE_PFNMAP .huge_fault = vfio_pci_mmap_huge_fault, diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index 733607371082..4b3b15655f1d 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -27,6 +27,32 @@ static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf, return 0; }
+static int vfio_pci_dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma) +{
- struct vfio_pci_dma_buf *priv = dmabuf->priv;
- if (priv->revoked)
return -ENODEV;Questionable validity to testing revoked without a lock, but doesn't this also fail to follow the "map regardless, sort it out on fault" paradigm used elsewhere in vfio-pci? Thanks,
Kind of, though this was intentional. (I had removed a crappy comment here from the RFC, but should re-add something better...)
You're right that, if we don't test this here, the fault handler ensures safety.
In the revoke ioctl^Wfeature header UAPI comment I'd suggested a revocation prevents a new map (in addition to prevents access to an existing map) with the intention of making userspace easier to debug:
- I'd rather trace an mmap() failure than a SIGBUS (eg. if another process has revoked my DMABUF and I didn't get the memo). For a prior revoke this is a reliable & useful test.
- Even if there's an active race such as userspace doing an ioctl(RESET) at the same time as an mmap() (!) then obviously this check won't always catch it, but when it does then that's useful.
I'd prefer to keep this belt-and-braces check to early-fail (with comment), if you don't mind much. It _is_ testing revoked outside of any synchronisation as anything we can lock against here doesn't have a useful effect AFAICT. (Taking the example of a concurrent ioctl(RESET), a revoke still manifests "before" (caught here) or "after" (caught in fault handler).) Maybe READ_ONCE() wouldn't hurt, if only to indicate this.
With thanks,
Matt
Alex
- if ((vma->vm_flags & VM_SHARED) == 0)
return -EINVAL;- /*
* dma_buf_mmap_internal() has asserted that the VMA is* contained within the DMABUF size before calling this.*/- vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
- vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
- /* See comments in vfio_pci_core_mmap() re VM_ALLOW_ANY_UNCACHED. */
- vm_flags_set(vma, VM_ALLOW_ANY_UNCACHED | VM_IO | VM_PFNMAP |
VM_DONTEXPAND | VM_DONTDUMP);- vma->vm_private_data = priv;
- vma->vm_ops = &vfio_pci_mmap_ops;
- return 0;
+} #endif /* CONFIG_VFIO_PCI_DMABUF */ static void vfio_pci_dma_buf_done(struct kref *kref) @@ -94,6 +120,7 @@ static void vfio_pci_dma_buf_release(struct dma_buf *dmabuf) static const struct dma_buf_ops vfio_pci_dmabuf_ops = { #ifdef CONFIG_VFIO_PCI_DMABUF .attach = vfio_pci_dma_buf_attach,
- .mmap = vfio_pci_dma_buf_mmap, #endif .map_dma_buf = vfio_pci_dma_buf_map, .unmap_dma_buf = vfio_pci_dma_buf_unmap,
diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index 10833aabd7fb..db2e2aeae88f 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -38,6 +38,8 @@ struct vfio_pci_dma_buf { u8 revoked : 1; }; +extern const struct vm_operations_struct vfio_pci_mmap_ops;
- bool vfio_pci_intx_mask(struct vfio_pci_core_device *vdev); void vfio_pci_intx_unmask(struct vfio_pci_core_device *vdev);
On Tue, 2 Jun 2026 18:35:25 +0100 Matt Evans mattev@meta.com wrote:
On 29/05/2026 00:15, Alex Williamson wrote:
On Wed, 27 May 2026 03:23:10 -0700 Matt Evans mattev@meta.com wrote:
diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index 733607371082..4b3b15655f1d 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -27,6 +27,32 @@ static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf, return 0; }
+static int vfio_pci_dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma) +{
- struct vfio_pci_dma_buf *priv = dmabuf->priv;
- if (priv->revoked)
return -ENODEV;Questionable validity to testing revoked without a lock, but doesn't this also fail to follow the "map regardless, sort it out on fault" paradigm used elsewhere in vfio-pci? Thanks,
Kind of, though this was intentional. (I had removed a crappy comment here from the RFC, but should re-add something better...)
You're right that, if we don't test this here, the fault handler ensures safety.
In the revoke ioctl^Wfeature header UAPI comment I'd suggested a revocation prevents a new map (in addition to prevents access to an existing map) with the intention of making userspace easier to debug:
I'd rather trace an mmap() failure than a SIGBUS (eg. if another process has revoked my DMABUF and I didn't get the memo). For a prior revoke this is a reliable & useful test.
Even if there's an active race such as userspace doing an ioctl(RESET) at the same time as an mmap() (!) then obviously this check won't always catch it, but when it does then that's useful.
I'd prefer to keep this belt-and-braces check to early-fail (with comment), if you don't mind much. It _is_ testing revoked outside of any synchronisation as anything we can lock against here doesn't have a useful effect AFAICT. (Taking the example of a concurrent ioctl(RESET), a revoke still manifests "before" (caught here) or "after" (caught in fault handler).) Maybe READ_ONCE() wouldn't hurt, if only to indicate this.
Hey Matt,
I think only the permanently revoked semantics are up for interpretation here, mmap with memory disabled or across reset is currently allowed and better debug'ability is not sufficient to risk breaking compatibility. Thanks,
Alex
Expand the VFIO DMABUF revocation state to three states: Not revoked, temporarily revoked, and permanently revoked.
The first two are for existing transient revocation, e.g. across a function reset, and the DMABUF is put into the last in response to a new ioctl(VFIO_DEVICE_PCI_DMABUF_REVOKE) request.
This VFIO device fd ioctl() passes a DMABUF by fd and requests that the DMABUF is permanently revoked. On success, it's guaranteed that the buffer can never be imported/attached/mmap()ed in future, that dynamic imports have been cleanly detached, and that all mappings have been made inaccessible/PTEs zapped.
This is useful for lifecycle management, to reclaim VFIO PCI BAR ranges previously delegated to a subordinate client process: The driver process can ensure that the loaned resources are revoked when the client is deemed "done", and exported ranges can be safely re-used elsewhere.
Refactor the revocation code out of vfio_pci_dma_buf_move() to a function common to move and the new ioctl path.
Signed-off-by: Matt Evans mattev@meta.com --- drivers/vfio/pci/vfio_pci_core.c | 21 ++++- drivers/vfio/pci/vfio_pci_dmabuf.c | 146 +++++++++++++++++++++-------- drivers/vfio/pci/vfio_pci_priv.h | 14 ++- include/uapi/linux/vfio.h | 30 ++++++ 4 files changed, 170 insertions(+), 41 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index 41e049fa9a8a..5184b3cac160 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -1500,6 +1500,21 @@ static int vfio_pci_ioctl_ioeventfd(struct vfio_pci_core_device *vdev, ioeventfd.fd); }
+static int vfio_pci_ioctl_dmabuf_revoke(struct vfio_pci_core_device *vdev, + struct vfio_pci_dmabuf_revoke __user *arg) +{ + unsigned long minsz = offsetofend(struct vfio_pci_dmabuf_revoke, dmabuf_fd); + struct vfio_pci_dmabuf_revoke revoke; + + if (copy_from_user(&revoke, arg, minsz)) + return -EFAULT; + + if (revoke.argsz < minsz) + return -EINVAL; + + return vfio_pci_dma_buf_revoke(vdev, revoke.dmabuf_fd); +} + long vfio_pci_core_ioctl(struct vfio_device *core_vdev, unsigned int cmd, unsigned long arg) { @@ -1522,6 +1537,8 @@ long vfio_pci_core_ioctl(struct vfio_device *core_vdev, unsigned int cmd, return vfio_pci_ioctl_reset(vdev, uarg); case VFIO_DEVICE_SET_IRQS: return vfio_pci_ioctl_set_irqs(vdev, uarg); + case VFIO_DEVICE_PCI_DMABUF_REVOKE: + return vfio_pci_ioctl_dmabuf_revoke(vdev, uarg); default: return -ENOTTY; } @@ -1792,7 +1809,7 @@ static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf, dma_resv_lock(priv->dmabuf->resv, NULL); vdev = READ_ONCE(priv->vdev);
- if (priv->revoked || !vdev) { + if (priv->status != VFIO_PCI_DMABUF_OK || !vdev) { pr_debug_ratelimited("%s VA 0x%lx, pgoff 0x%lx: DMABUF revoked/cleaned up\n", __func__, vmf->address, vma->vm_pgoff); dma_resv_unlock(priv->dmabuf->resv); @@ -1815,7 +1832,7 @@ static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf,
scoped_guard(rwsem_read, &vdev->memory_lock) { /* Revocation status must be re-read, under memory_lock */ - if (!priv->revoked) { + if (priv->status == VFIO_PCI_DMABUF_OK) { int pres = vfio_pci_dma_buf_find_pfn(priv, vma, vmf->address, order, &pfn); diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index 4b3b15655f1d..3fa14760898f 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -19,7 +19,7 @@ static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf, if (!attachment->peer2peer) return -EOPNOTSUPP;
- if (priv->revoked) + if (priv->status != VFIO_PCI_DMABUF_OK) return -ENODEV;
if (!dma_buf_attach_revocable(attachment)) @@ -32,7 +32,7 @@ static int vfio_pci_dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct * { struct vfio_pci_dma_buf *priv = dmabuf->priv;
- if (priv->revoked) + if (priv->status != VFIO_PCI_DMABUF_OK) return -ENODEV; if ((vma->vm_flags & VM_SHARED) == 0) return -EINVAL; @@ -72,7 +72,7 @@ vfio_pci_dma_buf_map(struct dma_buf_attachment *attachment,
dma_resv_assert_held(priv->dmabuf->resv);
- if (priv->revoked) + if (priv->status != VFIO_PCI_DMABUF_OK) return ERR_PTR(-ENODEV);
ret = dma_buf_phys_vec_to_sgt(attachment, priv->provider, @@ -287,7 +287,8 @@ static int vfio_pci_dmabuf_export(struct vfio_pci_core_device *vdev, INIT_LIST_HEAD(&priv->dmabufs_elm); down_write(&vdev->memory_lock); dma_resv_lock(priv->dmabuf->resv, NULL); - priv->revoked = !__vfio_pci_memory_enabled(vdev); + priv->status = __vfio_pci_memory_enabled(vdev) ? VFIO_PCI_DMABUF_OK : + VFIO_PCI_DMABUF_TEMP_REVOKED; list_add_tail(&priv->dmabufs_elm, &vdev->dmabufs); dma_resv_unlock(priv->dmabuf->resv); up_write(&vdev->memory_lock); @@ -318,7 +319,7 @@ int vfio_pci_dma_buf_iommufd_map(struct dma_buf_attachment *attachment, return -EOPNOTSUPP;
priv = attachment->dmabuf->priv; - if (priv->revoked) + if (priv->status != VFIO_PCI_DMABUF_OK) return -ENODEV;
/* More than one range to iommufd will require proper DMABUF support */ @@ -585,6 +586,63 @@ int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev, return ret; }
+static void __vfio_pci_dma_buf_revoke(struct vfio_pci_dma_buf *priv, bool revoked, + bool permanently) +{ + bool was_revoked; + + lockdep_assert_held_write(&priv->vdev->memory_lock); + + if ((priv->status == VFIO_PCI_DMABUF_PERM_REVOKED) || + (priv->status == VFIO_PCI_DMABUF_OK && !revoked) || + (priv->status == VFIO_PCI_DMABUF_TEMP_REVOKED && revoked && !permanently)) { + return; + } + + dma_resv_lock(priv->dmabuf->resv, NULL); + was_revoked = priv->status != VFIO_PCI_DMABUF_OK; + + if (revoked) + priv->status = permanently ? VFIO_PCI_DMABUF_PERM_REVOKED : + VFIO_PCI_DMABUF_TEMP_REVOKED; + + /* + * If TEMP_REVOKED is being upgraded to PERM_REVOKED, the + * buffer is already gone. Don't wait on it again. + */ + if (was_revoked && revoked) { + dma_resv_unlock(priv->dmabuf->resv); + return; + } + + dma_buf_invalidate_mappings(priv->dmabuf); + dma_resv_wait_timeout(priv->dmabuf->resv, + DMA_RESV_USAGE_BOOKKEEP, false, + MAX_SCHEDULE_TIMEOUT); + dma_resv_unlock(priv->dmabuf->resv); + if (revoked) { + kref_put(&priv->kref, vfio_pci_dma_buf_done); + wait_for_completion(&priv->comp); + unmap_mapping_range(priv->dmabuf->file->f_mapping, + 0, priv->size, 1); + /* + * Re-arm the registered kref reference and the + * completion so the post-revoke state matches the + * post-creation state. An un-revoke followed by a + * new mapping needs the kref to be non-zero before + * kref_get(), and vfio_pci_dma_buf_cleanup() + * delegates its drain back through this revoke + * path on a possibly-already-revoked dma-buf. + */ + kref_init(&priv->kref); + reinit_completion(&priv->comp); + } else { + dma_resv_lock(priv->dmabuf->resv, NULL); + priv->status = VFIO_PCI_DMABUF_OK; + dma_resv_unlock(priv->dmabuf->resv); + } +} + void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked) { struct vfio_pci_dma_buf *priv; @@ -593,44 +651,13 @@ void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked) lockdep_assert_held_write(&vdev->memory_lock); /* * Holding memory_lock ensures a racing VMA fault observes - * priv->revoked properly. + * priv->status properly. */
list_for_each_entry_safe(priv, tmp, &vdev->dmabufs, dmabufs_elm) { if (!get_file_active(&priv->dmabuf->file)) continue; - - if (priv->revoked != revoked) { - dma_resv_lock(priv->dmabuf->resv, NULL); - if (revoked) - priv->revoked = true; - dma_buf_invalidate_mappings(priv->dmabuf); - dma_resv_wait_timeout(priv->dmabuf->resv, - DMA_RESV_USAGE_BOOKKEEP, false, - MAX_SCHEDULE_TIMEOUT); - dma_resv_unlock(priv->dmabuf->resv); - if (revoked) { - kref_put(&priv->kref, vfio_pci_dma_buf_done); - wait_for_completion(&priv->comp); - unmap_mapping_range(priv->dmabuf->file->f_mapping, - 0, priv->size, 1); - /* - * Re-arm the registered kref reference and the - * completion so the post-revoke state matches the - * post-creation state. An un-revoke followed by a - * new mapping needs the kref to be non-zero before - * kref_get(), and vfio_pci_dma_buf_cleanup() - * delegates its drain back through this revoke - * path on a possibly-already-revoked dma-buf. - */ - kref_init(&priv->kref); - reinit_completion(&priv->comp); - } else { - dma_resv_lock(priv->dmabuf->resv, NULL); - priv->revoked = false; - dma_resv_unlock(priv->dmabuf->resv); - } - } + __vfio_pci_dma_buf_revoke(priv, revoked, false); fput(priv->dmabuf->file); } } @@ -662,3 +689,46 @@ void vfio_pci_dma_buf_cleanup(struct vfio_pci_core_device *vdev) } up_write(&vdev->memory_lock); } + +#ifdef CONFIG_VFIO_PCI_DMABUF +int vfio_pci_dma_buf_revoke(struct vfio_pci_core_device *vdev, int dmabuf_fd) +{ + struct vfio_pci_dma_buf *priv; + struct dma_buf *dmabuf; + int ret = 0; + + dmabuf = dma_buf_get(dmabuf_fd); + if (IS_ERR(dmabuf)) + return PTR_ERR(dmabuf); + + priv = dmabuf->priv; + /* + * Sanity-check the DMABUF is really a vfio_pci_dma_buf _and_ + * relates to the VFIO device it was provided with. + * + * If the DMABUF relates to this vdev then priv->vdev is + * stable because this open fd prevents cleanup. + * + * If it relates to a different vdev, reading priv->vdev might + * race with a concurrent cleanup on that device. But if so, + * it points to a non-matching vdev or NULL and is unusable + * either way. + */ + if (dmabuf->ops != &vfio_pci_dmabuf_ops || priv->vdev != vdev) { + ret = -ENODEV; + goto out_put_buf; + } + + scoped_guard(rwsem_write, &vdev->memory_lock) { + if (priv->status == VFIO_PCI_DMABUF_PERM_REVOKED) + ret = -EBADFD; + else + __vfio_pci_dma_buf_revoke(priv, true, true); + } + + out_put_buf: + dma_buf_put(dmabuf); + + return ret; +} +#endif /* CONFIG_VFIO_PCI_DMABUF */ diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index db2e2aeae88f..a1e0f4fcb1dc 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -23,6 +23,12 @@ struct vfio_pci_ioeventfd { bool test_mem; };
+enum vfio_pci_dma_buf_status { + VFIO_PCI_DMABUF_OK = 0, + VFIO_PCI_DMABUF_TEMP_REVOKED = 1, + VFIO_PCI_DMABUF_PERM_REVOKED = 2, +}; + struct vfio_pci_dma_buf { struct dma_buf *dmabuf; struct vfio_pci_core_device *vdev; @@ -35,7 +41,7 @@ struct vfio_pci_dma_buf { struct kref kref; struct completion comp; unsigned long vma_pgoff_adjust; - u8 revoked : 1; + enum vfio_pci_dma_buf_status status; };
extern const struct vm_operations_struct vfio_pci_mmap_ops; @@ -148,6 +154,7 @@ void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked); int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, struct vfio_device_feature_dma_buf __user *arg, size_t argsz); +int vfio_pci_dma_buf_revoke(struct vfio_pci_core_device *vdev, int dmabuf_fd); #else static inline int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, @@ -156,6 +163,11 @@ vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, { return -ENOTTY; } +static inline int vfio_pci_dma_buf_revoke(struct vfio_pci_core_device *vdev, + int dmabuf_fd) +{ + return -ENODEV; +} #endif
#endif diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index 5de618a3a5ee..02366e9f8e16 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -1321,6 +1321,36 @@ struct vfio_precopy_info {
#define VFIO_MIG_GET_PRECOPY_INFO _IO(VFIO_TYPE, VFIO_BASE + 21)
+/** + * VFIO_DEVICE_PCI_DMABUF_REVOKE - _IO(VFIO_TYPE, VFIO_BASE + 22) + * + * This ioctl is used on the device FD, and requests that access to + * the buffer corresponding to the DMABUF FD parameter is immediately + * and permanently revoked. On successful return, the buffer is not + * accessible through any mmap() or dma-buf import. The request fails + * if the buffer is pinned; otherwise, the exporter marks the buffer + * as inaccessible and uses the move_notify callback to inform + * importers of the change. The buffer is permanently disabled, and + * VFIO refuses all map, mmap, attach, etc. requests. + * + * Returns: + * + * Return: 0 on success, -1 and errno set on failure: + * + * ENODEV if the associated dmabuf FD no longer exists/is closed, + * or is not a DMABUF created for this device. + * EINVAL if the dmabuf_fd parameter isn't a DMABUF. + * EBADF if the dmabuf_fd parameter isn't a valid file number. + * EBADFD if the buffer has already been revoked. + * + */ +struct vfio_pci_dmabuf_revoke { + __u32 argsz; + __s32 dmabuf_fd; +}; + +#define VFIO_DEVICE_PCI_DMABUF_REVOKE _IO(VFIO_TYPE, VFIO_BASE + 22) + /* * Upon VFIO_DEVICE_FEATURE_SET, allow the device to be moved into a low power * state with the platform-based power management. Device use of lower power
On Wed, 27 May 2026 03:23:11 -0700 Matt Evans mattev@meta.com wrote:
Expand the VFIO DMABUF revocation state to three states: Not revoked, temporarily revoked, and permanently revoked.
The first two are for existing transient revocation, e.g. across a function reset, and the DMABUF is put into the last in response to a new ioctl(VFIO_DEVICE_PCI_DMABUF_REVOKE) request.
The DMABUF is created via a VFIO_DEVICE_FEATURE ioctl and you next patch is setting attributes via another VFIO_DEVICE_FEATURE, why would the REVOKE operation not also be a VFIO_DEVICE_FEATURE?
This VFIO device fd ioctl() passes a DMABUF by fd and requests that the DMABUF is permanently revoked. On success, it's guaranteed that the buffer can never be imported/attached/mmap()ed in future, that dynamic imports have been cleanly detached, and that all mappings have been made inaccessible/PTEs zapped.
This is useful for lifecycle management, to reclaim VFIO PCI BAR ranges previously delegated to a subordinate client process: The driver process can ensure that the loaned resources are revoked when the client is deemed "done", and exported ranges can be safely re-used elsewhere.
Refactor the revocation code out of vfio_pci_dma_buf_move() to a function common to move and the new ioctl path.
Signed-off-by: Matt Evans mattev@meta.com
drivers/vfio/pci/vfio_pci_core.c | 21 ++++- drivers/vfio/pci/vfio_pci_dmabuf.c | 146 +++++++++++++++++++++-------- drivers/vfio/pci/vfio_pci_priv.h | 14 ++- include/uapi/linux/vfio.h | 30 ++++++ 4 files changed, 170 insertions(+), 41 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index 41e049fa9a8a..5184b3cac160 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -1500,6 +1500,21 @@ static int vfio_pci_ioctl_ioeventfd(struct vfio_pci_core_device *vdev, ioeventfd.fd); } +static int vfio_pci_ioctl_dmabuf_revoke(struct vfio_pci_core_device *vdev,
struct vfio_pci_dmabuf_revoke __user *arg)+{
- unsigned long minsz = offsetofend(struct vfio_pci_dmabuf_revoke, dmabuf_fd);
- struct vfio_pci_dmabuf_revoke revoke;
- if (copy_from_user(&revoke, arg, minsz))
return -EFAULT;- if (revoke.argsz < minsz)
return -EINVAL;- return vfio_pci_dma_buf_revoke(vdev, revoke.dmabuf_fd);
+}
long vfio_pci_core_ioctl(struct vfio_device *core_vdev, unsigned int cmd, unsigned long arg) { @@ -1522,6 +1537,8 @@ long vfio_pci_core_ioctl(struct vfio_device *core_vdev, unsigned int cmd, return vfio_pci_ioctl_reset(vdev, uarg); case VFIO_DEVICE_SET_IRQS: return vfio_pci_ioctl_set_irqs(vdev, uarg);
- case VFIO_DEVICE_PCI_DMABUF_REVOKE:
return vfio_pci_ioctl_dmabuf_revoke(vdev, uarg);@@ -1792,7 +1809,7 @@ static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf, dma_resv_lock(priv->dmabuf->resv, NULL); vdev = READ_ONCE(priv->vdev);
- if (priv->revoked || !vdev) {
- if (priv->status != VFIO_PCI_DMABUF_OK || !vdev) { pr_debug_ratelimited("%s VA 0x%lx, pgoff 0x%lx: DMABUF revoked/cleaned up\n", __func__, vmf->address, vma->vm_pgoff); dma_resv_unlock(priv->dmabuf->resv);
@@ -1815,7 +1832,7 @@ static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf, scoped_guard(rwsem_read, &vdev->memory_lock) { /* Revocation status must be re-read, under memory_lock */
if (!priv->revoked) {
if (priv->status == VFIO_PCI_DMABUF_OK) { int pres = vfio_pci_dma_buf_find_pfn(priv, vma, vmf->address, order, &pfn);diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index 4b3b15655f1d..3fa14760898f 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -19,7 +19,7 @@ static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf, if (!attachment->peer2peer) return -EOPNOTSUPP;
- if (priv->revoked)
- if (priv->status != VFIO_PCI_DMABUF_OK) return -ENODEV;
if (!dma_buf_attach_revocable(attachment)) @@ -32,7 +32,7 @@ static int vfio_pci_dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct * { struct vfio_pci_dma_buf *priv = dmabuf->priv;
- if (priv->revoked)
- if (priv->status != VFIO_PCI_DMABUF_OK) return -ENODEV; if ((vma->vm_flags & VM_SHARED) == 0) return -EINVAL;
@@ -72,7 +72,7 @@ vfio_pci_dma_buf_map(struct dma_buf_attachment *attachment, dma_resv_assert_held(priv->dmabuf->resv);
- if (priv->revoked)
- if (priv->status != VFIO_PCI_DMABUF_OK) return ERR_PTR(-ENODEV);
ret = dma_buf_phys_vec_to_sgt(attachment, priv->provider, @@ -287,7 +287,8 @@ static int vfio_pci_dmabuf_export(struct vfio_pci_core_device *vdev, INIT_LIST_HEAD(&priv->dmabufs_elm); down_write(&vdev->memory_lock); dma_resv_lock(priv->dmabuf->resv, NULL);
- priv->revoked = !__vfio_pci_memory_enabled(vdev);
- priv->status = __vfio_pci_memory_enabled(vdev) ? VFIO_PCI_DMABUF_OK :
VFIO_PCI_DMABUF_TEMP_REVOKED;@@ -318,7 +319,7 @@ int vfio_pci_dma_buf_iommufd_map(struct dma_buf_attachment *attachment, return -EOPNOTSUPP; priv = attachment->dmabuf->priv;
- if (priv->revoked)
- if (priv->status != VFIO_PCI_DMABUF_OK) return -ENODEV;
/* More than one range to iommufd will require proper DMABUF support */ @@ -585,6 +586,63 @@ int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev, return ret; } +static void __vfio_pci_dma_buf_revoke(struct vfio_pci_dma_buf *priv, bool revoked,
bool permanently)
If the underscore prefix is mean to imply the lock semantics, that's explicit with the annotation below and can be dropped.
The double bool args are not very intuitive to use and the [false, true] combination is rather invalid. Why not an enum:
enum vfio_pci_dma_buf_revoke_action { VFIO_PCI_DMABUF_REVOKE_RESTORE, VFIO_PCI_DMABUF_REVOKE_TEMPORARY, VFIO_PCI_DMABUF_REVOKE_PERMANENT, };
+{
- bool was_revoked;
- lockdep_assert_held_write(&priv->vdev->memory_lock);
- if ((priv->status == VFIO_PCI_DMABUF_PERM_REVOKED) ||
(priv->status == VFIO_PCI_DMABUF_OK && !revoked) ||(priv->status == VFIO_PCI_DMABUF_TEMP_REVOKED && revoked && !permanently)) {return;- }
- dma_resv_lock(priv->dmabuf->resv, NULL);
- was_revoked = priv->status != VFIO_PCI_DMABUF_OK;
- if (revoked)
priv->status = permanently ? VFIO_PCI_DMABUF_PERM_REVOKED :VFIO_PCI_DMABUF_TEMP_REVOKED;- /*
* If TEMP_REVOKED is being upgraded to PERM_REVOKED, the* buffer is already gone. Don't wait on it again.*/- if (was_revoked && revoked) {
dma_resv_unlock(priv->dmabuf->resv);return;- }
- dma_buf_invalidate_mappings(priv->dmabuf);
- dma_resv_wait_timeout(priv->dmabuf->resv,
DMA_RESV_USAGE_BOOKKEEP, false,MAX_SCHEDULE_TIMEOUT);- dma_resv_unlock(priv->dmabuf->resv);
- if (revoked) {
kref_put(&priv->kref, vfio_pci_dma_buf_done);wait_for_completion(&priv->comp);unmap_mapping_range(priv->dmabuf->file->f_mapping,0, priv->size, 1);/** Re-arm the registered kref reference and the* completion so the post-revoke state matches the* post-creation state. An un-revoke followed by a* new mapping needs the kref to be non-zero before* kref_get(), and vfio_pci_dma_buf_cleanup()* delegates its drain back through this revoke* path on a possibly-already-revoked dma-buf.*/kref_init(&priv->kref);reinit_completion(&priv->comp);- } else {
dma_resv_lock(priv->dmabuf->resv, NULL);priv->status = VFIO_PCI_DMABUF_OK;dma_resv_unlock(priv->dmabuf->resv);- }
+}
void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked) { struct vfio_pci_dma_buf *priv; @@ -593,44 +651,13 @@ void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked) lockdep_assert_held_write(&vdev->memory_lock); /* * Holding memory_lock ensures a racing VMA fault observes
* priv->revoked properly.
* priv->status properly.list_for_each_entry_safe(priv, tmp, &vdev->dmabufs, dmabufs_elm) { if (!get_file_active(&priv->dmabuf->file)) continue;
if (priv->revoked != revoked) {dma_resv_lock(priv->dmabuf->resv, NULL);if (revoked)priv->revoked = true;dma_buf_invalidate_mappings(priv->dmabuf);dma_resv_wait_timeout(priv->dmabuf->resv,DMA_RESV_USAGE_BOOKKEEP, false,MAX_SCHEDULE_TIMEOUT);dma_resv_unlock(priv->dmabuf->resv);if (revoked) {kref_put(&priv->kref, vfio_pci_dma_buf_done);wait_for_completion(&priv->comp);unmap_mapping_range(priv->dmabuf->file->f_mapping,0, priv->size, 1);/** Re-arm the registered kref reference and the* completion so the post-revoke state matches the* post-creation state. An un-revoke followed by a* new mapping needs the kref to be non-zero before* kref_get(), and vfio_pci_dma_buf_cleanup()* delegates its drain back through this revoke* path on a possibly-already-revoked dma-buf.*/kref_init(&priv->kref);reinit_completion(&priv->comp);} else {dma_resv_lock(priv->dmabuf->resv, NULL);priv->revoked = false;dma_resv_unlock(priv->dmabuf->resv);}}
__vfio_pci_dma_buf_revoke(priv, revoked, false);} @@ -662,3 +689,46 @@ void vfio_pci_dma_buf_cleanup(struct vfio_pci_core_device *vdev) } up_write(&vdev->memory_lock); }
+#ifdef CONFIG_VFIO_PCI_DMABUF +int vfio_pci_dma_buf_revoke(struct vfio_pci_core_device *vdev, int dmabuf_fd) +{
- struct vfio_pci_dma_buf *priv;
- struct dma_buf *dmabuf;
- int ret = 0;
- dmabuf = dma_buf_get(dmabuf_fd);
- if (IS_ERR(dmabuf))
return PTR_ERR(dmabuf);- priv = dmabuf->priv;
- /*
* Sanity-check the DMABUF is really a vfio_pci_dma_buf _and_* relates to the VFIO device it was provided with.** If the DMABUF relates to this vdev then priv->vdev is* stable because this open fd prevents cleanup.** If it relates to a different vdev, reading priv->vdev might* race with a concurrent cleanup on that device. But if so,* it points to a non-matching vdev or NULL and is unusable* either way.*/- if (dmabuf->ops != &vfio_pci_dmabuf_ops || priv->vdev != vdev) {
ret = -ENODEV;goto out_put_buf;- }
- scoped_guard(rwsem_write, &vdev->memory_lock) {
if (priv->status == VFIO_PCI_DMABUF_PERM_REVOKED)ret = -EBADFD;else__vfio_pci_dma_buf_revoke(priv, true, true);- }
- out_put_buf:
- dma_buf_put(dmabuf);
- return ret;
+} +#endif /* CONFIG_VFIO_PCI_DMABUF */ diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index db2e2aeae88f..a1e0f4fcb1dc 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -23,6 +23,12 @@ struct vfio_pci_ioeventfd { bool test_mem; }; +enum vfio_pci_dma_buf_status {
- VFIO_PCI_DMABUF_OK = 0,
- VFIO_PCI_DMABUF_TEMP_REVOKED = 1,
- VFIO_PCI_DMABUF_PERM_REVOKED = 2,
+};
struct vfio_pci_dma_buf { struct dma_buf *dmabuf; struct vfio_pci_core_device *vdev; @@ -35,7 +41,7 @@ struct vfio_pci_dma_buf { struct kref kref; struct completion comp; unsigned long vma_pgoff_adjust;
- u8 revoked : 1;
- enum vfio_pci_dma_buf_status status;
}; extern const struct vm_operations_struct vfio_pci_mmap_ops; @@ -148,6 +154,7 @@ void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked); int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, struct vfio_device_feature_dma_buf __user *arg, size_t argsz); +int vfio_pci_dma_buf_revoke(struct vfio_pci_core_device *vdev, int dmabuf_fd); #else static inline int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, @@ -156,6 +163,11 @@ vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, { return -ENOTTY; } +static inline int vfio_pci_dma_buf_revoke(struct vfio_pci_core_device *vdev,
int dmabuf_fd)+{
- return -ENODEV;
+} #endif #endif diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index 5de618a3a5ee..02366e9f8e16 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -1321,6 +1321,36 @@ struct vfio_precopy_info { #define VFIO_MIG_GET_PRECOPY_INFO _IO(VFIO_TYPE, VFIO_BASE + 21) +/**
- VFIO_DEVICE_PCI_DMABUF_REVOKE - _IO(VFIO_TYPE, VFIO_BASE + 22)
- This ioctl is used on the device FD, and requests that access to
- the buffer corresponding to the DMABUF FD parameter is immediately
- and permanently revoked. On successful return, the buffer is not
- accessible through any mmap() or dma-buf import. The request fails
- if the buffer is pinned; otherwise, the exporter marks the buffer
- as inaccessible and uses the move_notify callback to inform
- importers of the change. The buffer is permanently disabled, and
- VFIO refuses all map, mmap, attach, etc. requests.
- Returns:
- Return: 0 on success, -1 and errno set on failure:
- ENODEV if the associated dmabuf FD no longer exists/is closed,
These actually seem to map to EBADF/EINVAL. Thanks,
Alex
or is not a DMABUF created for this device.
- EINVAL if the dmabuf_fd parameter isn't a DMABUF.
- EBADF if the dmabuf_fd parameter isn't a valid file number.
- EBADFD if the buffer has already been revoked.
- */
+struct vfio_pci_dmabuf_revoke {
- __u32 argsz;
- __s32 dmabuf_fd;
+};
+#define VFIO_DEVICE_PCI_DMABUF_REVOKE _IO(VFIO_TYPE, VFIO_BASE + 22)
/*
- Upon VFIO_DEVICE_FEATURE_SET, allow the device to be moved into a low power
- state with the platform-based power management. Device use of lower power
Hi Alex,
On 29/05/2026 00:14, Alex Williamson wrote:
On Wed, 27 May 2026 03:23:11 -0700 Matt Evans mattev@meta.com wrote:
Expand the VFIO DMABUF revocation state to three states: Not revoked, temporarily revoked, and permanently revoked.
The first two are for existing transient revocation, e.g. across a function reset, and the DMABUF is put into the last in response to a new ioctl(VFIO_DEVICE_PCI_DMABUF_REVOKE) request.
The DMABUF is created via a VFIO_DEVICE_FEATURE ioctl and you next patch is setting attributes via another VFIO_DEVICE_FEATURE, why would the REVOKE operation not also be a VFIO_DEVICE_FEATURE?
I was possibly reading too much into VFIO features tending to have GET/SET semantics (accessor-like) rather than DO_THING. But sure, a revocation SETs a new buffer state so it makes sense.
I've converted the revoke operation to a SET on a new VFIO_DEVICE_FEATURE_DMA_BUF_REVOKE, and looks more nicely contained.
This VFIO device fd ioctl() passes a DMABUF by fd and requests that the DMABUF is permanently revoked. On success, it's guaranteed that the buffer can never be imported/attached/mmap()ed in future, that dynamic imports have been cleanly detached, and that all mappings have been made inaccessible/PTEs zapped.
This is useful for lifecycle management, to reclaim VFIO PCI BAR ranges previously delegated to a subordinate client process: The driver process can ensure that the loaned resources are revoked when the client is deemed "done", and exported ranges can be safely re-used elsewhere.
Refactor the revocation code out of vfio_pci_dma_buf_move() to a function common to move and the new ioctl path.
Signed-off-by: Matt Evans mattev@meta.com
drivers/vfio/pci/vfio_pci_core.c | 21 ++++- drivers/vfio/pci/vfio_pci_dmabuf.c | 146 +++++++++++++++++++++-------- drivers/vfio/pci/vfio_pci_priv.h | 14 ++- include/uapi/linux/vfio.h | 30 ++++++ 4 files changed, 170 insertions(+), 41 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index 41e049fa9a8a..5184b3cac160 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -1500,6 +1500,21 @@ static int vfio_pci_ioctl_ioeventfd(struct vfio_pci_core_device *vdev, ioeventfd.fd); } +static int vfio_pci_ioctl_dmabuf_revoke(struct vfio_pci_core_device *vdev,
struct vfio_pci_dmabuf_revoke __user *arg)+{
- unsigned long minsz = offsetofend(struct vfio_pci_dmabuf_revoke, dmabuf_fd);
- struct vfio_pci_dmabuf_revoke revoke;
- if (copy_from_user(&revoke, arg, minsz))
return -EFAULT;- if (revoke.argsz < minsz)
return -EINVAL;- return vfio_pci_dma_buf_revoke(vdev, revoke.dmabuf_fd);
+}
- long vfio_pci_core_ioctl(struct vfio_device *core_vdev, unsigned int cmd, unsigned long arg) {
@@ -1522,6 +1537,8 @@ long vfio_pci_core_ioctl(struct vfio_device *core_vdev, unsigned int cmd, return vfio_pci_ioctl_reset(vdev, uarg); case VFIO_DEVICE_SET_IRQS: return vfio_pci_ioctl_set_irqs(vdev, uarg);
- case VFIO_DEVICE_PCI_DMABUF_REVOKE:
return vfio_pci_ioctl_dmabuf_revoke(vdev, uarg);@@ -1792,7 +1809,7 @@ static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf, dma_resv_lock(priv->dmabuf->resv, NULL); vdev = READ_ONCE(priv->vdev);
- if (priv->revoked || !vdev) {
- if (priv->status != VFIO_PCI_DMABUF_OK || !vdev) { pr_debug_ratelimited("%s VA 0x%lx, pgoff 0x%lx: DMABUF revoked/cleaned up\n", __func__, vmf->address, vma->vm_pgoff); dma_resv_unlock(priv->dmabuf->resv);
@@ -1815,7 +1832,7 @@ static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf, scoped_guard(rwsem_read, &vdev->memory_lock) { /* Revocation status must be re-read, under memory_lock */
if (!priv->revoked) {
if (priv->status == VFIO_PCI_DMABUF_OK) { int pres = vfio_pci_dma_buf_find_pfn(priv, vma, vmf->address, order, &pfn);diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index 4b3b15655f1d..3fa14760898f 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -19,7 +19,7 @@ static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf, if (!attachment->peer2peer) return -EOPNOTSUPP;
- if (priv->revoked)
- if (priv->status != VFIO_PCI_DMABUF_OK) return -ENODEV;
if (!dma_buf_attach_revocable(attachment)) @@ -32,7 +32,7 @@ static int vfio_pci_dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct * { struct vfio_pci_dma_buf *priv = dmabuf->priv;
- if (priv->revoked)
- if (priv->status != VFIO_PCI_DMABUF_OK) return -ENODEV; if ((vma->vm_flags & VM_SHARED) == 0) return -EINVAL;
@@ -72,7 +72,7 @@ vfio_pci_dma_buf_map(struct dma_buf_attachment *attachment, dma_resv_assert_held(priv->dmabuf->resv);
- if (priv->revoked)
- if (priv->status != VFIO_PCI_DMABUF_OK) return ERR_PTR(-ENODEV);
ret = dma_buf_phys_vec_to_sgt(attachment, priv->provider, @@ -287,7 +287,8 @@ static int vfio_pci_dmabuf_export(struct vfio_pci_core_device *vdev, INIT_LIST_HEAD(&priv->dmabufs_elm); down_write(&vdev->memory_lock); dma_resv_lock(priv->dmabuf->resv, NULL);
- priv->revoked = !__vfio_pci_memory_enabled(vdev);
- priv->status = __vfio_pci_memory_enabled(vdev) ? VFIO_PCI_DMABUF_OK :
VFIO_PCI_DMABUF_TEMP_REVOKED;@@ -318,7 +319,7 @@ int vfio_pci_dma_buf_iommufd_map(struct dma_buf_attachment *attachment, return -EOPNOTSUPP; priv = attachment->dmabuf->priv;
- if (priv->revoked)
- if (priv->status != VFIO_PCI_DMABUF_OK) return -ENODEV;
/* More than one range to iommufd will require proper DMABUF support */ @@ -585,6 +586,63 @@ int vfio_pci_core_mmap_prep_dmabuf(struct vfio_pci_core_device *vdev, return ret; } +static void __vfio_pci_dma_buf_revoke(struct vfio_pci_dma_buf *priv, bool revoked,
bool permanently)If the underscore prefix is mean to imply the lock semantics, that's explicit with the annotation below and can be dropped.
The double bool args are not very intuitive to use and the [false, true] combination is rather invalid. Why not an enum:
enum vfio_pci_dma_buf_revoke_action { VFIO_PCI_DMABUF_REVOKE_RESTORE, VFIO_PCI_DMABUF_REVOKE_TEMPORARY, VFIO_PCI_DMABUF_REVOKE_PERMANENT, };
That's a fair comment. I've reframed this helper as setting the revocation state, and callers pass in one of the existing enum vfio_pci_dma_buf_status values which neatens it up a lot. (See '^^^' below.)
+{
- bool was_revoked;
- lockdep_assert_held_write(&priv->vdev->memory_lock);
- if ((priv->status == VFIO_PCI_DMABUF_PERM_REVOKED) ||
(priv->status == VFIO_PCI_DMABUF_OK && !revoked) ||(priv->status == VFIO_PCI_DMABUF_TEMP_REVOKED && revoked && !permanently)) {return;- }
- dma_resv_lock(priv->dmabuf->resv, NULL);
- was_revoked = priv->status != VFIO_PCI_DMABUF_OK;
- if (revoked)
priv->status = permanently ? VFIO_PCI_DMABUF_PERM_REVOKED :VFIO_PCI_DMABUF_TEMP_REVOKED;- /*
* If TEMP_REVOKED is being upgraded to PERM_REVOKED, the* buffer is already gone. Don't wait on it again.*/- if (was_revoked && revoked) {
dma_resv_unlock(priv->dmabuf->resv);return;- }
- dma_buf_invalidate_mappings(priv->dmabuf);
- dma_resv_wait_timeout(priv->dmabuf->resv,
DMA_RESV_USAGE_BOOKKEEP, false,MAX_SCHEDULE_TIMEOUT);- dma_resv_unlock(priv->dmabuf->resv);
- if (revoked) {
kref_put(&priv->kref, vfio_pci_dma_buf_done);wait_for_completion(&priv->comp);unmap_mapping_range(priv->dmabuf->file->f_mapping,0, priv->size, 1);/** Re-arm the registered kref reference and the* completion so the post-revoke state matches the* post-creation state. An un-revoke followed by a* new mapping needs the kref to be non-zero before* kref_get(), and vfio_pci_dma_buf_cleanup()* delegates its drain back through this revoke* path on a possibly-already-revoked dma-buf.*/kref_init(&priv->kref);reinit_completion(&priv->comp);- } else {
dma_resv_lock(priv->dmabuf->resv, NULL);priv->status = VFIO_PCI_DMABUF_OK;dma_resv_unlock(priv->dmabuf->resv);- }
+}
- void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked) { struct vfio_pci_dma_buf *priv;
@@ -593,44 +651,13 @@ void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked) lockdep_assert_held_write(&vdev->memory_lock); /* * Holding memory_lock ensures a racing VMA fault observes
* priv->revoked properly.
* priv->status properly.list_for_each_entry_safe(priv, tmp, &vdev->dmabufs, dmabufs_elm) { if (!get_file_active(&priv->dmabuf->file)) continue;
if (priv->revoked != revoked) {dma_resv_lock(priv->dmabuf->resv, NULL);if (revoked)priv->revoked = true;dma_buf_invalidate_mappings(priv->dmabuf);dma_resv_wait_timeout(priv->dmabuf->resv,DMA_RESV_USAGE_BOOKKEEP, false,MAX_SCHEDULE_TIMEOUT);dma_resv_unlock(priv->dmabuf->resv);if (revoked) {kref_put(&priv->kref, vfio_pci_dma_buf_done);wait_for_completion(&priv->comp);unmap_mapping_range(priv->dmabuf->file->f_mapping,0, priv->size, 1);/** Re-arm the registered kref reference and the* completion so the post-revoke state matches the* post-creation state. An un-revoke followed by a* new mapping needs the kref to be non-zero before* kref_get(), and vfio_pci_dma_buf_cleanup()* delegates its drain back through this revoke* path on a possibly-already-revoked dma-buf.*/kref_init(&priv->kref);reinit_completion(&priv->comp);} else {dma_resv_lock(priv->dmabuf->resv, NULL);priv->revoked = false;dma_resv_unlock(priv->dmabuf->resv);}}
__vfio_pci_dma_buf_revoke(priv, revoked, false);@@ -662,3 +689,46 @@ void vfio_pci_dma_buf_cleanup(struct vfio_pci_core_device *vdev) } up_write(&vdev->memory_lock); }
+#ifdef CONFIG_VFIO_PCI_DMABUF +int vfio_pci_dma_buf_revoke(struct vfio_pci_core_device *vdev, int dmabuf_fd) +{
- struct vfio_pci_dma_buf *priv;
- struct dma_buf *dmabuf;
- int ret = 0;
- dmabuf = dma_buf_get(dmabuf_fd);
- if (IS_ERR(dmabuf))
return PTR_ERR(dmabuf);- priv = dmabuf->priv;
- /*
* Sanity-check the DMABUF is really a vfio_pci_dma_buf _and_* relates to the VFIO device it was provided with.** If the DMABUF relates to this vdev then priv->vdev is* stable because this open fd prevents cleanup.** If it relates to a different vdev, reading priv->vdev might* race with a concurrent cleanup on that device. But if so,* it points to a non-matching vdev or NULL and is unusable* either way.*/- if (dmabuf->ops != &vfio_pci_dmabuf_ops || priv->vdev != vdev) {
ret = -ENODEV;goto out_put_buf;- }
- scoped_guard(rwsem_write, &vdev->memory_lock) {
if (priv->status == VFIO_PCI_DMABUF_PERM_REVOKED)ret = -EBADFD;else__vfio_pci_dma_buf_revoke(priv, true, true);- }
- out_put_buf:
- dma_buf_put(dmabuf);
- return ret;
+} +#endif /* CONFIG_VFIO_PCI_DMABUF */ diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index db2e2aeae88f..a1e0f4fcb1dc 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -23,6 +23,12 @@ struct vfio_pci_ioeventfd { bool test_mem; }; +enum vfio_pci_dma_buf_status {
- VFIO_PCI_DMABUF_OK = 0,
- VFIO_PCI_DMABUF_TEMP_REVOKED = 1,
- VFIO_PCI_DMABUF_PERM_REVOKED = 2,
+};
^^^ Here ^^^
- struct vfio_pci_dma_buf { struct dma_buf *dmabuf; struct vfio_pci_core_device *vdev;
@@ -35,7 +41,7 @@ struct vfio_pci_dma_buf { struct kref kref; struct completion comp; unsigned long vma_pgoff_adjust;
- u8 revoked : 1;
- enum vfio_pci_dma_buf_status status; };
extern const struct vm_operations_struct vfio_pci_mmap_ops; @@ -148,6 +154,7 @@ void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked); int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, struct vfio_device_feature_dma_buf __user *arg, size_t argsz); +int vfio_pci_dma_buf_revoke(struct vfio_pci_core_device *vdev, int dmabuf_fd); #else static inline int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, @@ -156,6 +163,11 @@ vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, { return -ENOTTY; } +static inline int vfio_pci_dma_buf_revoke(struct vfio_pci_core_device *vdev,
int dmabuf_fd)+{
- return -ENODEV;
+} #endif #endif diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index 5de618a3a5ee..02366e9f8e16 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -1321,6 +1321,36 @@ struct vfio_precopy_info { #define VFIO_MIG_GET_PRECOPY_INFO _IO(VFIO_TYPE, VFIO_BASE + 21) +/**
- VFIO_DEVICE_PCI_DMABUF_REVOKE - _IO(VFIO_TYPE, VFIO_BASE + 22)
- This ioctl is used on the device FD, and requests that access to
- the buffer corresponding to the DMABUF FD parameter is immediately
- and permanently revoked. On successful return, the buffer is not
- accessible through any mmap() or dma-buf import. The request fails
- if the buffer is pinned; otherwise, the exporter marks the buffer
- as inaccessible and uses the move_notify callback to inform
- importers of the change. The buffer is permanently disabled, and
- VFIO refuses all map, mmap, attach, etc. requests.
- Returns:
- Return: 0 on success, -1 and errno set on failure:
- ENODEV if the associated dmabuf FD no longer exists/is closed,
These actually seem to map to EBADF/EINVAL. Thanks,
OK, these go away when moving to use a VFIO feature for revocation.
Thanks,
Matt
Alex
or is not a DMABUF created for this device.
- EINVAL if the dmabuf_fd parameter isn't a DMABUF.
- EBADF if the dmabuf_fd parameter isn't a valid file number.
- EBADFD if the buffer has already been revoked.
- */
+struct vfio_pci_dmabuf_revoke {
- __u32 argsz;
- __s32 dmabuf_fd;
+};
+#define VFIO_DEVICE_PCI_DMABUF_REVOKE _IO(VFIO_TYPE, VFIO_BASE + 22)
- /*
- Upon VFIO_DEVICE_FEATURE_SET, allow the device to be moved into a low power
- state with the platform-based power management. Device use of lower power
A new VFIO feature, VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR, is added to set (and get) 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_UC - VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_WC, which results in WC PTEs for the DMABUF's BAR region.
Signed-off-by: Matt Evans mattev@meta.com --- drivers/vfio/pci/vfio_pci_core.c | 2 + drivers/vfio/pci/vfio_pci_dmabuf.c | 70 +++++++++++++++++++++++++++++- drivers/vfio/pci/vfio_pci_priv.h | 12 +++++ include/uapi/linux/vfio.h | 27 ++++++++++++ 4 files changed, 110 insertions(+), 1 deletion(-)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index 5184b3cac160..e256a925e7ce 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -1590,6 +1590,8 @@ int vfio_pci_core_ioctl_feature(struct vfio_device *device, u32 flags, return vfio_pci_core_feature_token(vdev, flags, arg, argsz); case VFIO_DEVICE_FEATURE_DMA_BUF: return vfio_pci_core_feature_dma_buf(vdev, flags, arg, argsz); + case VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR: + return vfio_pci_core_feature_dma_buf_memattr(vdev, flags, arg, argsz); default: return -ENOTTY; } diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index 3fa14760898f..db8b95ddbe18 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -42,7 +42,10 @@ static int vfio_pci_dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct * * contained within the DMABUF size before calling this. */
- vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); + if (READ_ONCE(priv->memattr) == VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_WC) + vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot); + else + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
/* See comments in vfio_pci_core_mmap() re VM_ALLOW_ANY_UNCACHED. */ @@ -464,6 +467,7 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, priv->vdev = vdev; priv->nr_ranges = get_dma_buf.nr_ranges; priv->size = length; + priv->memattr = VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_NC; ret = vdev->pci_ops->get_dmabuf_phys(vdev, &priv->provider, get_dma_buf.region_index, priv->phys_vec, dma_ranges, @@ -731,4 +735,68 @@ int vfio_pci_dma_buf_revoke(struct vfio_pci_core_device *vdev, int dmabuf_fd)
return ret; } + +int vfio_pci_core_feature_dma_buf_memattr( + struct vfio_pci_core_device *vdev, u32 flags, + struct vfio_device_feature_dma_buf_memattr __user *arg, + size_t argsz) +{ + struct vfio_device_feature_dma_buf_memattr db_attr; + struct vfio_pci_dma_buf *priv; + struct dma_buf *dmabuf; + int ret; + + if (!vdev->pci_ops || !vdev->pci_ops->get_dmabuf_phys) + return -EOPNOTSUPP; + + ret = vfio_check_feature(flags, argsz, + VFIO_DEVICE_FEATURE_GET | + VFIO_DEVICE_FEATURE_SET, + sizeof(db_attr)); + if (ret != 1) + return ret; + + if (copy_from_user(&db_attr, arg, sizeof(db_attr))) + return -EFAULT; + + dmabuf = dma_buf_get(db_attr.dmabuf_fd); + if (IS_ERR(dmabuf)) + return PTR_ERR(dmabuf); + + /* Verify DMABUF: see comments in vfio_pci_dma_buf_revoke() */ + priv = dmabuf->priv; + if (dmabuf->ops != &vfio_pci_dmabuf_ops || priv->vdev != vdev) { + ret = -ENODEV; + goto out_put_buf; + } + + ret = 0; + scoped_guard(rwsem_write, &vdev->memory_lock) { + uint32_t old_attr = priv->memattr; + + if (flags & VFIO_DEVICE_FEATURE_SET) { + switch(db_attr.memattr) { + case VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_NC: + case VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_WC: + priv->memattr = db_attr.memattr; + break; + + default: + ret = -ENOTSUPP; + } + } + db_attr.memattr = old_attr; + } + + if (!ret && (flags & VFIO_DEVICE_FEATURE_GET)) { + if (copy_to_user(arg, &db_attr, sizeof(db_attr))) + ret = -EFAULT; + } + + out_put_buf: + dma_buf_put(dmabuf); + + return ret; + +} #endif /* CONFIG_VFIO_PCI_DMABUF */ diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index a1e0f4fcb1dc..8067be45beb0 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -41,6 +41,7 @@ struct vfio_pci_dma_buf { struct kref kref; struct completion comp; unsigned long vma_pgoff_adjust; + u32 memattr; enum vfio_pci_dma_buf_status status; };
@@ -154,6 +155,10 @@ void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked); int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, struct vfio_device_feature_dma_buf __user *arg, size_t argsz); +int vfio_pci_core_feature_dma_buf_memattr( + struct vfio_pci_core_device *vdev, u32 flags, + struct vfio_device_feature_dma_buf_memattr __user *arg, + size_t argsz); int vfio_pci_dma_buf_revoke(struct vfio_pci_core_device *vdev, int dmabuf_fd); #else static inline int @@ -163,6 +168,13 @@ vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, { return -ENOTTY; } +static inline int vfio_pci_core_feature_dma_buf_memattr( + struct vfio_pci_core_device *vdev, u32 flags, + struct vfio_device_feature_dma_buf_memattr __user *arg, + size_t argsz) +{ + return -ENODEV; +} static inline int vfio_pci_dma_buf_revoke(struct vfio_pci_core_device *vdev, int dmabuf_fd) { diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index 02366e9f8e16..9b0b68f8a1ef 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -1564,6 +1564,33 @@ struct vfio_device_feature_dma_buf { */ #define VFIO_DEVICE_FEATURE_MIG_PRECOPY_INFOv2 12
+/** + * Given a dma_buf fd previously created by + * VFIO_DEVICE_FEATURE_DMA_BUF, GET or SET the memory attribute that + * will be used by future mmap()s of that fd. SETting a new attribute + * does not affect existing VMAs. + * + * The default, if no previous SET has been performed, is NC. + * + * Return: 0 on success, -1 and errno is set on failure: + * + * ENOTSUPP: The given memattr is not supported. + * EBADF, EINVAL: dmabuf_fd is not a DMABUF fd. + * ENODEV: The dmabuf_fd does not match this VFIO device. + */ +#define VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR 13 + +/* Valid memory attributes for the memattr field */ +enum vfio_device_dma_buf_memattr { + VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_NC = 0, /* pgprot_noncached */ + VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_WC = 1, /* pgprot_writecombine */ +}; + +struct vfio_device_feature_dma_buf_memattr { + __s32 dmabuf_fd; + __u32 memattr; +}; + /* -------- API for Type1 VFIO IOMMU -------- */
/**
On Wed, 27 May 2026 03:23:12 -0700 Matt Evans mattev@meta.com wrote:
A new VFIO feature, VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR, is added to set (and get) 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_UC
- VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_WC, which results in WC PTEs for the DMABUF's BAR region.
Signed-off-by: Matt Evans mattev@meta.com
drivers/vfio/pci/vfio_pci_core.c | 2 + drivers/vfio/pci/vfio_pci_dmabuf.c | 70 +++++++++++++++++++++++++++++- drivers/vfio/pci/vfio_pci_priv.h | 12 +++++ include/uapi/linux/vfio.h | 27 ++++++++++++ 4 files changed, 110 insertions(+), 1 deletion(-)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index 5184b3cac160..e256a925e7ce 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -1590,6 +1590,8 @@ int vfio_pci_core_ioctl_feature(struct vfio_device *device, u32 flags, return vfio_pci_core_feature_token(vdev, flags, arg, argsz); case VFIO_DEVICE_FEATURE_DMA_BUF: return vfio_pci_core_feature_dma_buf(vdev, flags, arg, argsz);
- case VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR:
return vfio_pci_core_feature_dma_buf_memattr(vdev, flags, arg, argsz);diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index 3fa14760898f..db8b95ddbe18 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -42,7 +42,10 @@ static int vfio_pci_dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct * * contained within the DMABUF size before calling this. */
- vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
- if (READ_ONCE(priv->memattr) == VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_WC)
vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);- else
vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);/* See comments in vfio_pci_core_mmap() re VM_ALLOW_ANY_UNCACHED. */ @@ -464,6 +467,7 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, priv->vdev = vdev; priv->nr_ranges = get_dma_buf.nr_ranges; priv->size = length;
- priv->memattr = VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_NC; ret = vdev->pci_ops->get_dmabuf_phys(vdev, &priv->provider, get_dma_buf.region_index, priv->phys_vec, dma_ranges,
@@ -731,4 +735,68 @@ int vfio_pci_dma_buf_revoke(struct vfio_pci_core_device *vdev, int dmabuf_fd) return ret; }
+int vfio_pci_core_feature_dma_buf_memattr(
- struct vfio_pci_core_device *vdev, u32 flags,
- struct vfio_device_feature_dma_buf_memattr __user *arg,
- size_t argsz)
+{
- struct vfio_device_feature_dma_buf_memattr db_attr;
- struct vfio_pci_dma_buf *priv;
- struct dma_buf *dmabuf;
- int ret;
- if (!vdev->pci_ops || !vdev->pci_ops->get_dmabuf_phys)
return -EOPNOTSUPP;- ret = vfio_check_feature(flags, argsz,
VFIO_DEVICE_FEATURE_GET |VFIO_DEVICE_FEATURE_SET,sizeof(db_attr));
I don't see why this needs to support GET. Are we solving a userspace problem that doesn't exist?
- if (ret != 1)
return ret;- if (copy_from_user(&db_attr, arg, sizeof(db_attr)))
return -EFAULT;- dmabuf = dma_buf_get(db_attr.dmabuf_fd);
- if (IS_ERR(dmabuf))
return PTR_ERR(dmabuf);- /* Verify DMABUF: see comments in vfio_pci_dma_buf_revoke() */
- priv = dmabuf->priv;
- if (dmabuf->ops != &vfio_pci_dmabuf_ops || priv->vdev != vdev) {
ret = -ENODEV;goto out_put_buf;- }
- ret = 0;
- scoped_guard(rwsem_write, &vdev->memory_lock) {
Why? This doesn't serialize against mmap. Just use a WRITE_ONCE() to match the READ_ONCE() on mmap?
uint32_t old_attr = priv->memattr;if (flags & VFIO_DEVICE_FEATURE_SET) {switch(db_attr.memattr) {case VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_NC:case VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_WC:priv->memattr = db_attr.memattr;break;default:ret = -ENOTSUPP;
-EINVAL
}}db_attr.memattr = old_attr;- }
- if (!ret && (flags & VFIO_DEVICE_FEATURE_GET)) {
if (copy_to_user(arg, &db_attr, sizeof(db_attr)))ret = -EFAULT;- }
- out_put_buf:
- dma_buf_put(dmabuf);
- return ret;
+} #endif /* CONFIG_VFIO_PCI_DMABUF */ diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index a1e0f4fcb1dc..8067be45beb0 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -41,6 +41,7 @@ struct vfio_pci_dma_buf { struct kref kref; struct completion comp; unsigned long vma_pgoff_adjust;
- u32 memattr; enum vfio_pci_dma_buf_status status;
}; @@ -154,6 +155,10 @@ void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked); int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, struct vfio_device_feature_dma_buf __user *arg, size_t argsz); +int vfio_pci_core_feature_dma_buf_memattr(
- struct vfio_pci_core_device *vdev, u32 flags,
- struct vfio_device_feature_dma_buf_memattr __user *arg,
- size_t argsz);
int vfio_pci_dma_buf_revoke(struct vfio_pci_core_device *vdev, int dmabuf_fd); #else static inline int @@ -163,6 +168,13 @@ vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, { return -ENOTTY; } +static inline int vfio_pci_core_feature_dma_buf_memattr(
- struct vfio_pci_core_device *vdev, u32 flags,
- struct vfio_device_feature_dma_buf_memattr __user *arg,
- size_t argsz)
+{
- return -ENODEV;
-ENOTTY
Thanks, Alex
+} static inline int vfio_pci_dma_buf_revoke(struct vfio_pci_core_device *vdev, int dmabuf_fd) { diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index 02366e9f8e16..9b0b68f8a1ef 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -1564,6 +1564,33 @@ struct vfio_device_feature_dma_buf { */ #define VFIO_DEVICE_FEATURE_MIG_PRECOPY_INFOv2 12 +/**
- Given a dma_buf fd previously created by
- VFIO_DEVICE_FEATURE_DMA_BUF, GET or SET the memory attribute that
- will be used by future mmap()s of that fd. SETting a new attribute
- does not affect existing VMAs.
- The default, if no previous SET has been performed, is NC.
- Return: 0 on success, -1 and errno is set on failure:
- ENOTSUPP: The given memattr is not supported.
- EBADF, EINVAL: dmabuf_fd is not a DMABUF fd.
- ENODEV: The dmabuf_fd does not match this VFIO device.
- */
+#define VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR 13
+/* Valid memory attributes for the memattr field */ +enum vfio_device_dma_buf_memattr {
- VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_NC = 0, /* pgprot_noncached */
- VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_WC = 1, /* pgprot_writecombine */
+};
+struct vfio_device_feature_dma_buf_memattr {
- __s32 dmabuf_fd;
- __u32 memattr;
+};
/* -------- API for Type1 VFIO IOMMU -------- */ /**
Hi Alex,
On 29/05/2026 00:14, Alex Williamson wrote:
On Wed, 27 May 2026 03:23:12 -0700 Matt Evans mattev@meta.com wrote:
A new VFIO feature, VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR, is added to set (and get) 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_UC
- VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_WC, which results in WC PTEs for the DMABUF's BAR region.
Signed-off-by: Matt Evans mattev@meta.com
drivers/vfio/pci/vfio_pci_core.c | 2 + drivers/vfio/pci/vfio_pci_dmabuf.c | 70 +++++++++++++++++++++++++++++- drivers/vfio/pci/vfio_pci_priv.h | 12 +++++ include/uapi/linux/vfio.h | 27 ++++++++++++ 4 files changed, 110 insertions(+), 1 deletion(-)
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index 5184b3cac160..e256a925e7ce 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -1590,6 +1590,8 @@ int vfio_pci_core_ioctl_feature(struct vfio_device *device, u32 flags, return vfio_pci_core_feature_token(vdev, flags, arg, argsz); case VFIO_DEVICE_FEATURE_DMA_BUF: return vfio_pci_core_feature_dma_buf(vdev, flags, arg, argsz);
- case VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR:
return vfio_pci_core_feature_dma_buf_memattr(vdev, flags, arg, argsz);diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c index 3fa14760898f..db8b95ddbe18 100644 --- a/drivers/vfio/pci/vfio_pci_dmabuf.c +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c @@ -42,7 +42,10 @@ static int vfio_pci_dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct * * contained within the DMABUF size before calling this. */
- vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
- if (READ_ONCE(priv->memattr) == VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_WC)
vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);- else
vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);/* See comments in vfio_pci_core_mmap() re VM_ALLOW_ANY_UNCACHED. */ @@ -464,6 +467,7 @@ int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, priv->vdev = vdev; priv->nr_ranges = get_dma_buf.nr_ranges; priv->size = length;
- priv->memattr = VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_NC; ret = vdev->pci_ops->get_dmabuf_phys(vdev, &priv->provider, get_dma_buf.region_index, priv->phys_vec, dma_ranges,
@@ -731,4 +735,68 @@ int vfio_pci_dma_buf_revoke(struct vfio_pci_core_device *vdev, int dmabuf_fd) return ret; }
+int vfio_pci_core_feature_dma_buf_memattr(
- struct vfio_pci_core_device *vdev, u32 flags,
- struct vfio_device_feature_dma_buf_memattr __user *arg,
- size_t argsz)
+{
- struct vfio_device_feature_dma_buf_memattr db_attr;
- struct vfio_pci_dma_buf *priv;
- struct dma_buf *dmabuf;
- int ret;
- if (!vdev->pci_ops || !vdev->pci_ops->get_dmabuf_phys)
return -EOPNOTSUPP;- ret = vfio_check_feature(flags, argsz,
VFIO_DEVICE_FEATURE_GET |VFIO_DEVICE_FEATURE_SET,sizeof(db_attr));I don't see why this needs to support GET. Are we solving a userspace problem that doesn't exist?
Possibly, I'm a bit twitchy about unobservable/write-only internal state so added GET out of habit. However, no worries, removed it and this function is now much lighter.
- if (ret != 1)
return ret;- if (copy_from_user(&db_attr, arg, sizeof(db_attr)))
return -EFAULT;- dmabuf = dma_buf_get(db_attr.dmabuf_fd);
- if (IS_ERR(dmabuf))
return PTR_ERR(dmabuf);- /* Verify DMABUF: see comments in vfio_pci_dma_buf_revoke() */
- priv = dmabuf->priv;
- if (dmabuf->ops != &vfio_pci_dmabuf_ops || priv->vdev != vdev) {
ret = -ENODEV;goto out_put_buf;- }
- ret = 0;
- scoped_guard(rwsem_write, &vdev->memory_lock) {
Why? This doesn't serialize against mmap. Just use a WRITE_ONCE() to match the READ_ONCE() on mmap?
Ooops, good point. Fixed.
uint32_t old_attr = priv->memattr;if (flags & VFIO_DEVICE_FEATURE_SET) {switch(db_attr.memattr) {case VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_NC:case VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_WC:priv->memattr = db_attr.memattr;break;default:ret = -ENOTSUPP;-EINVAL
I'd like to push back on this one; ENOTSUPP distinguishes trying to use a wild attribute value from all manner of other screwups such as trying to GET (with it no longer supported), passing an fd that isn't a DMABUF, etc.
I'm not wed to the value ENOTSUPP, just something distinctive versus the other errors userspace might see, and that value seems appropriate.
}}db_attr.memattr = old_attr;- }
- if (!ret && (flags & VFIO_DEVICE_FEATURE_GET)) {
if (copy_to_user(arg, &db_attr, sizeof(db_attr)))ret = -EFAULT;- }
- out_put_buf:
- dma_buf_put(dmabuf);
- return ret;
+} #endif /* CONFIG_VFIO_PCI_DMABUF */ diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index a1e0f4fcb1dc..8067be45beb0 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -41,6 +41,7 @@ struct vfio_pci_dma_buf { struct kref kref; struct completion comp; unsigned long vma_pgoff_adjust;
- u32 memattr; enum vfio_pci_dma_buf_status status; };
@@ -154,6 +155,10 @@ void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked); int vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, struct vfio_device_feature_dma_buf __user *arg, size_t argsz); +int vfio_pci_core_feature_dma_buf_memattr(
- struct vfio_pci_core_device *vdev, u32 flags,
- struct vfio_device_feature_dma_buf_memattr __user *arg,
- size_t argsz); int vfio_pci_dma_buf_revoke(struct vfio_pci_core_device *vdev, int dmabuf_fd); #else static inline int
@@ -163,6 +168,13 @@ vfio_pci_core_feature_dma_buf(struct vfio_pci_core_device *vdev, u32 flags, { return -ENOTTY; } +static inline int vfio_pci_core_feature_dma_buf_memattr(
- struct vfio_pci_core_device *vdev, u32 flags,
- struct vfio_device_feature_dma_buf_memattr __user *arg,
- size_t argsz)
+{
- return -ENODEV;
-ENOTTY
Gotcha, fixed. Yes, that's quite conspicuous versus the chunk above, unsure where I got that from.
Thank you,
Matt
Thanks, Alex
+} static inline int vfio_pci_dma_buf_revoke(struct vfio_pci_core_device *vdev, int dmabuf_fd) { diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index 02366e9f8e16..9b0b68f8a1ef 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -1564,6 +1564,33 @@ struct vfio_device_feature_dma_buf { */ #define VFIO_DEVICE_FEATURE_MIG_PRECOPY_INFOv2 12 +/**
- Given a dma_buf fd previously created by
- VFIO_DEVICE_FEATURE_DMA_BUF, GET or SET the memory attribute that
- will be used by future mmap()s of that fd. SETting a new attribute
- does not affect existing VMAs.
- The default, if no previous SET has been performed, is NC.
- Return: 0 on success, -1 and errno is set on failure:
- ENOTSUPP: The given memattr is not supported.
- EBADF, EINVAL: dmabuf_fd is not a DMABUF fd.
- ENODEV: The dmabuf_fd does not match this VFIO device.
- */
+#define VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR 13
+/* Valid memory attributes for the memattr field */ +enum vfio_device_dma_buf_memattr {
- VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_NC = 0, /* pgprot_noncached */
- VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_WC = 1, /* pgprot_writecombine */
+};
+struct vfio_device_feature_dma_buf_memattr {
- __s32 dmabuf_fd;
- __u32 memattr;
+};
- /* -------- API for Type1 VFIO IOMMU -------- */
/**
On Tue, 2 Jun 2026 17:50:04 +0100 Matt Evans mattev@meta.com wrote:
On 29/05/2026 00:14, Alex Williamson wrote:
On Wed, 27 May 2026 03:23:12 -0700 Matt Evans mattev@meta.com wrote:
uint32_t old_attr = priv->memattr;if (flags & VFIO_DEVICE_FEATURE_SET) {switch(db_attr.memattr) {case VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_NC:case VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_WC:priv->memattr = db_attr.memattr;break;default:ret = -ENOTSUPP;-EINVAL
I'd like to push back on this one; ENOTSUPP distinguishes trying to use a wild attribute value from all manner of other screwups such as trying to GET (with it no longer supported), passing an fd that isn't a DMABUF, etc.
I'm not wed to the value ENOTSUPP, just something distinctive versus the other errors userspace might see, and that value seems appropriate.
It needs to be a uAPI errno though, aiui ENOTSUPP is an internal NFS errno. How about EOPNOTSUPP? Thanks,
Alex
Hi Alex,
On 02/06/2026 20:14, Alex Williamson wrote:
On Tue, 2 Jun 2026 17:50:04 +0100 Matt Evans mattev@meta.com wrote:
On 29/05/2026 00:14, Alex Williamson wrote:
On Wed, 27 May 2026 03:23:12 -0700 Matt Evans mattev@meta.com wrote:
uint32_t old_attr = priv->memattr;if (flags & VFIO_DEVICE_FEATURE_SET) {switch(db_attr.memattr) {case VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_NC:case VFIO_DEVICE_FEATURE_DMA_BUF_MEMATTR_WC:priv->memattr = db_attr.memattr;break;default:ret = -ENOTSUPP;-EINVAL
I'd like to push back on this one; ENOTSUPP distinguishes trying to use a wild attribute value from all manner of other screwups such as trying to GET (with it no longer supported), passing an fd that isn't a DMABUF, etc.
I'm not wed to the value ENOTSUPP, just something distinctive versus the other errors userspace might see, and that value seems appropriate.
It needs to be a uAPI errno though, aiui ENOTSUPP is an internal NFS errno. How about EOPNOTSUPP? Thanks,
Ew, right! TIL, and it's caught by checkpatch too (which I fastidiously run but still missed this somehow...) :( "ENOTSUPP is not a standard error code and should be avoided in new patches. EOPNOTSUPP should be used instead."
Fixed.
Thanks,
Matt
linaro-mm-sig@lists.linaro.org
-
Alex Williamson -
Alex Williamson -
Bjorn Helgaas -
Jason Gunthorpe -
Logan Gunthorpe -
Matt Evans -
Pranjal Shrivastava -
Tian, Kevin