Hi Alexes,
On 14/10/2026 00:13, Alex Williamson wrote:
On Tue, 6 Oct 2026 20:36:36 +0100 Matt Evans matt@ozlabs.org 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, in which the process can only map a specific subset of a BAR represented by the exported DMABUF.
mmap() support enables userspace drivers to safely delegate BAR sub-ranges to other 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, the new mmap() reuses the existing vm_ops.
The lifecycle of an exported DMABUF remains decoupled from that of the device fd it came from, i.e. the device fd could be closed with DMABUF VMAs present, meaning a fault on a VMA could happen concurrently with vfio_pci_dma_buf_cleanup().
To deal with this scenario, the fault handler now temporarily takes a VFIO device registration to ensure the vdev remains valid, and then vdev locks can be taken.
This sounds like potentially measurable overhead if something is slamming the fault handler where huge-fault is not supported or active. Have you considered a fast path for the internal dmabuf where the lifecycle is constrained?
Good question, I did think about this (rationale below) though ended up trying to keep this function simpler/easier to reason about. I can have another look.
< long info, as AlexM and I were talking about this offline >
The position of your comment sounds like the device registration was part of the concern, but can you clarify please? I think it's more that ..._fault() now takes dmabuf->resv plus vdev->dmabuf_lock (on top of the pre-existing vdev->memory_lock) rather than the device registration. The device registration refcount should be cheap (like, not block or contend beyond the atomic op), and wasn't the bit I worry about. :)
The overhead is the single thread "this function does more stuff", plus the scenario of threads both concurrently faulting.
What's the baseline/fast path? With an internal/VFIO mmap DMABUF we'd at least need vdev->dmabuf_lock for its status, then the existing memory_lock for existing reasons. But once you have memory_lock then (aside from the access cost of the atomic op) the new dmabuf_lock will be cheap/uncontended in the absence of a blitz of concurrent exports. (Export vs fault is a new performance dependency.)
[ Aside: when adding vdev->dmabuf_lock it occured that we could revisit the tests in vfio_pci_vmf_insert_pfn() that require memory_lock, and remove that lock from the fault path: e.g. if we know a DMABUF isn't revoked then that implies vdev->bars_revoked = false, and that should already be in sync with pm_runtime_engaged, __vfio_pci_memory_enabled() ... I haven't traced this all through. ]
The one I worry about more is dmabuf->resv; if there are importers and fencing then this'll now hold up behind them. (But this is only for explicit DMABUFs, and feels like the price you pay with P2P. This won't happen for internal/VFIO mmap DMABUFs.)
My gut concern is mostly the straight-line cost of Taking More Locks, rather than that they will be hugely more contended in the threaded case (where threads will already interfere via memory_lock, and taking dmabuf_lock under that is Unlikely to also contend). Well, and dmabuf->resv.
But I don't have measurements of this; more complex will obviously cost more, though we don't yet know whether it's prohibitive or noise.
I'd rather not split this function before it's a problem, so to speak, as it'll be slightly more complicated. What I'd do: introduce a flag to vfio_pci_dma_buf to indicate "was internal/external export" then use this to do extra synchronisation, or avoid it for a VFIO mmap VMA. (priv->file != NULL is more or less that, but might be cleaner with a flag.)
Hmm. I'll hack up a test to try to measure the relative overhead of VMA faults before/after (modulo access to meaningful HW).
Hopefully we can get those selftests included to avoid regressions.
We'll post more tests separately (there're some extra requirements, e.g. "real devices"/device access to work through). The existing tests go some way regarding avoiding existing VFIO fd mmap() regressions. The liability of not bringing the DMABUF-mmap tests into this series is just on that feature, at least.
If anyone has tested this series (whether to see existing VFIO works, or to try the new DMABUF-mmap/revoke) that would be great to know. AlexM and I have some ideas for further testing internally too.
Overall, I'm not finding too much to pick on in this series, but it also hasn't collected much in the way of reviews. For a fundamental change like this, I hope we can get more eyes on it. Thanks,
I'm glad, I feel it's in a good state now.
100% agree needs more eyes. FWIW Kevin and Praan reviewed ~all of v5, though I dropped their R-Bs because all of the patches have in some way changed. (Though most of the changes are very small, am a stickler for process.) Please, if they or others have time for another pass... <:-)
Many thanks,
Matt