On Wed, Jan 21, 2026 at 02:22:31PM +0000, Pranjal Shrivastava wrote:
> On Wed, Jan 21, 2026 at 09:47:12AM -0400, Jason Gunthorpe wrote:
> > On Wed, Jan 21, 2026 at 02:59:16PM +0200, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro(a)nvidia.com>
> > >
> > > Use the new dma_buf_attach_revocable() helper to restrict attachments to
> > > importers that support mapping invalidation.
> > >
> > > Signed-off-by: Leon Romanovsky <leonro(a)nvidia.com>
> > > ---
> > > drivers/vfio/pci/vfio_pci_dmabuf.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c
> > > index 5fceefc40e27..85056a5a3faf 100644
> > > --- a/drivers/vfio/pci/vfio_pci_dmabuf.c
> > > +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
> > > @@ -31,6 +31,9 @@ static int vfio_pci_dma_buf_attach(struct dma_buf *dmabuf,
> > > if (priv->revoked)
> > > return -ENODEV;
> > >
> > > + if (!dma_buf_attach_revocable(attachment))
> > > + return -EOPNOTSUPP;
> > > +
> > > return 0;
> > > }
> >
> > We need to push an urgent -rc fix to implement a pin function here
> > that always fails. That was missed and it means things like rdma can
> > import vfio when the intention was to block that. It would be bad for
> > that uAPI mistake to reach a released kernel.
> >
> > It's tricky that NULL pin ops means "I support pin" :|
> >
>
> I've been wondering about this for a while now, I've been sitting on the
> following:
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index a4d8f2ff94e4..962bce959366 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -1133,6 +1133,8 @@ int dma_buf_pin(struct dma_buf_attachment *attach)
>
> if (dmabuf->ops->pin)
> ret = dmabuf->ops->pin(attach);
> + else
> + ret = -EOPNOTSUPP;
>
> return ret;
> }
>
> But didn't get a chance to dive in the history yet. I thought there's a
> good reason we didn't have it? Would it break exisitng dmabuf users?
Probably every importer which called to dma_buf_pin() while connecting
to existing exporters as many in tree implementation don't have ->pin()
implemented.
Thanks
>
> Praan
On 1/20/26 21:44, Matthew Brost wrote:
> On Tue, Jan 20, 2026 at 04:07:06PM +0200, Leon Romanovsky wrote:
>> From: Leon Romanovsky <leonro(a)nvidia.com>
>>
>> dma-buf invalidation is performed asynchronously by hardware, so VFIO must
>> wait until all affected objects have been fully invalidated.
>>
>> Fixes: 5d74781ebc86 ("vfio/pci: Add dma-buf export support for MMIO regions")
>> Signed-off-by: Leon Romanovsky <leonro(a)nvidia.com>
>> ---
>> drivers/vfio/pci/vfio_pci_dmabuf.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c
>> index d4d0f7d08c53..33bc6a1909dd 100644
>> --- a/drivers/vfio/pci/vfio_pci_dmabuf.c
>> +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
>> @@ -321,6 +321,9 @@ void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked)
>> dma_resv_lock(priv->dmabuf->resv, NULL);
>> priv->revoked = revoked;
>> dma_buf_move_notify(priv->dmabuf);
>> + dma_resv_wait_timeout(priv->dmabuf->resv,
>> + DMA_RESV_USAGE_KERNEL, false,
>> + MAX_SCHEDULE_TIMEOUT);
>
> Should we explicitly call out in the dma_buf_move_notify() /
> invalidate_mappings kernel-doc that KERNEL slots are the mechanism
> for communicating asynchronous dma_buf_move_notify /
> invalidate_mappings events via fences?
Oh, I missed that! And no that is not correct.
This should be DMA_RESV_USAGE_BOOKKEEP so that we wait for everything.
Regards,
Christian.
>
> Yes, this is probably implied, but it wouldn’t hurt to state this
> explicitly as part of the cross-driver contract.
>
> Here is what we have now:
>
> * - Dynamic importers should set fences for any access that they can't
> * disable immediately from their &dma_buf_attach_ops.invalidate_mappings
> * callback.
>
> Matt
>
>> dma_resv_unlock(priv->dmabuf->resv);
>> }
>> fput(priv->dmabuf->file);
>> @@ -342,6 +345,8 @@ void vfio_pci_dma_buf_cleanup(struct vfio_pci_core_device *vdev)
>> priv->vdev = NULL;
>> priv->revoked = true;
>> dma_buf_move_notify(priv->dmabuf);
>> + dma_resv_wait_timeout(priv->dmabuf->resv, DMA_RESV_USAGE_KERNEL,
>> + false, MAX_SCHEDULE_TIMEOUT);
>> dma_resv_unlock(priv->dmabuf->resv);
>> vfio_device_put_registration(&vdev->vdev);
>> fput(priv->dmabuf->file);
>>
>> --
>> 2.52.0
>>
On 1/20/26 12:41, Tvrtko Ursulin wrote:
>
> On 20/01/2026 10:54, Christian König wrote:
>> Implement per-fence spinlocks, allowing implementations to not give an
>> external spinlock to protect the fence internal statei. Instead a spinlock
>> embedded into the fence structure itself is used in this case.
>>
>> Shared spinlocks have the problem that implementations need to guarantee
>> that the lock live at least as long all fences referencing them.
>>
>> Using a per-fence spinlock allows completely decoupling spinlock producer
>> and consumer life times, simplifying the handling in most use cases.
>>
>> v2: improve naming, coverage and function documentation
>> v3: fix one additional locking in the selftests
>> v4: separate out some changes to make the patch smaller,
>> fix one amdgpu crash found by CI systems
>>
>> Signed-off-by: Christian König <christian.koenig(a)amd.com>
>> ---
>> drivers/dma-buf/dma-fence.c | 25 +++++++++++++++++-------
>> drivers/dma-buf/sync_debug.h | 2 +-
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 2 +-
>> drivers/gpu/drm/drm_crtc.c | 2 +-
>> drivers/gpu/drm/drm_writeback.c | 2 +-
>> drivers/gpu/drm/nouveau/nouveau_fence.c | 3 ++-
>> drivers/gpu/drm/qxl/qxl_release.c | 3 ++-
>> drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 3 ++-
>> drivers/gpu/drm/xe/xe_hw_fence.c | 3 ++-
>
> i915 needed changes too, based on the kbuild report.
Going to take a look now.
> Have you seen my note about the RCU sparse warning as well?
Nope, I must have missed that mail.
...
>> +/**
>> + * dma_fence_spinlock - return pointer to the spinlock protecting the fence
>> + * @fence: the fence to get the lock from
>> + *
>> + * Return either the pointer to the embedded or the external spin lock.
>> + */
>> +static inline spinlock_t *dma_fence_spinlock(struct dma_fence *fence)
>> +{
>> + return test_bit(DMA_FENCE_FLAG_INLINE_LOCK_BIT, &fence->flags) ?
>> + &fence->inline_lock : fence->extern_lock;
>> +}
>
> You did not want to move this helper into "dma-buf: abstract fence locking" ?
I was avoiding that to keep the pre-requisite patch smaller, cause this change here seemed independent to that.
But thinking about it I could make a third patch which introduces dma_fence_spinlock() and changes all the container_of uses.
> I think that would have been better to keep everything mechanical in one patch, and then this patch which changes behaviour does not touch any drivers but only dma-fence core.
>
> Also, what about adding something like dma_fence_container_of() in that patch as well?
I would rather like to avoid that. Using the spinlock pointer with container_of seemed to be a bit of a hack to me in the first place and I don't want to encourage people to do that in new code as well.
Regards,
Christian.
>
> Regards,
>
> Tvrtko
>
>> +
>> /**
>> * dma_fence_lock_irqsave - irqsave lock the fence
>> * @fence: the fence to lock
>> @@ -385,7 +403,7 @@ dma_fence_get_rcu_safe(struct dma_fence __rcu **fencep)
>> * Lock the fence, preventing it from changing to the signaled state.
>> */
>> #define dma_fence_lock_irqsave(fence, flags) \
>> - spin_lock_irqsave(fence->lock, flags)
>> + spin_lock_irqsave(dma_fence_spinlock(fence), flags)
>> /**
>> * dma_fence_unlock_irqrestore - unlock the fence and irqrestore
>> @@ -395,7 +413,7 @@ dma_fence_get_rcu_safe(struct dma_fence __rcu **fencep)
>> * Unlock the fence, allowing it to change it's state to signaled again.
>> */
>> #define dma_fence_unlock_irqrestore(fence, flags) \
>> - spin_unlock_irqrestore(fence->lock, flags)
>> + spin_unlock_irqrestore(dma_fence_spinlock(fence), flags)
>> #ifdef CONFIG_LOCKDEP
>> bool dma_fence_begin_signalling(void);
>
On Tue, Jan 20, 2026 at 12:44:50PM -0800, Matthew Brost wrote:
> On Tue, Jan 20, 2026 at 04:07:06PM +0200, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro(a)nvidia.com>
> >
> > dma-buf invalidation is performed asynchronously by hardware, so VFIO must
> > wait until all affected objects have been fully invalidated.
> >
> > Fixes: 5d74781ebc86 ("vfio/pci: Add dma-buf export support for MMIO regions")
> > Signed-off-by: Leon Romanovsky <leonro(a)nvidia.com>
> > ---
> > drivers/vfio/pci/vfio_pci_dmabuf.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c b/drivers/vfio/pci/vfio_pci_dmabuf.c
> > index d4d0f7d08c53..33bc6a1909dd 100644
> > --- a/drivers/vfio/pci/vfio_pci_dmabuf.c
> > +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
> > @@ -321,6 +321,9 @@ void vfio_pci_dma_buf_move(struct vfio_pci_core_device *vdev, bool revoked)
> > dma_resv_lock(priv->dmabuf->resv, NULL);
> > priv->revoked = revoked;
> > dma_buf_move_notify(priv->dmabuf);
> > + dma_resv_wait_timeout(priv->dmabuf->resv,
> > + DMA_RESV_USAGE_KERNEL, false,
> > + MAX_SCHEDULE_TIMEOUT);
>
> Should we explicitly call out in the dma_buf_move_notify() /
> invalidate_mappings kernel-doc that KERNEL slots are the mechanism
> for communicating asynchronous dma_buf_move_notify /
> invalidate_mappings events via fences?
>
> Yes, this is probably implied, but it wouldn’t hurt to state this
> explicitly as part of the cross-driver contract.
>
> Here is what we have now:
>
> * - Dynamic importers should set fences for any access that they can't
> * disable immediately from their &dma_buf_attach_ops.invalidate_mappings
> * callback.
I believe I documented this in patch 4:
https://lore.kernel.org/all/20260120-dmabuf-revoke-v3-4-b7e0b07b8214@nvidia…"
Is there anything else that should be added?
1275 /**
1276 * dma_buf_move_notify - notify attachments that DMA-buf is moving
1277 *
1278 * @dmabuf: [in] buffer which is moving
1279 *
1280 * Informs all attachments that they need to destroy and recreate all their
1281 * mappings. If the attachment is dynamic then the dynamic importer is expected
1282 * to invalidate any caches it has of the mapping result and perform a new
1283 * mapping request before allowing HW to do any further DMA.
1284 *
1285 * If the attachment is pinned then this informs the pinned importer that
1286 * the underlying mapping is no longer available. Pinned importers may take
1287 * this is as a permanent revocation so exporters should not trigger it
1288 * lightly.
1289 *
1290 * For legacy pinned importers that cannot support invalidation this is a NOP.
1291 * Drivers can call dma_buf_attach_revocable() to determine if the importer
1292 * supports this.
1293 *
1294 * NOTE: The invalidation triggers asynchronous HW operation and the callers
1295 * need to wait for this operation to complete by calling
1296 * to dma_resv_wait_timeout().
1297 */
Thanks
>
> Matt
>
> > dma_resv_unlock(priv->dmabuf->resv);
> > }
> > fput(priv->dmabuf->file);
> > @@ -342,6 +345,8 @@ void vfio_pci_dma_buf_cleanup(struct vfio_pci_core_device *vdev)
> > priv->vdev = NULL;
> > priv->revoked = true;
> > dma_buf_move_notify(priv->dmabuf);
> > + dma_resv_wait_timeout(priv->dmabuf->resv, DMA_RESV_USAGE_KERNEL,
> > + false, MAX_SCHEDULE_TIMEOUT);
> > dma_resv_unlock(priv->dmabuf->resv);
> > vfio_device_put_registration(&vdev->vdev);
> > fput(priv->dmabuf->file);
> >
> > --
> > 2.52.0
> >
On Thu, Jan 08, 2026 at 01:11:15PM +0200, Edward Srouji wrote:
> +static int phys_addr_to_bar(struct pci_dev *pdev, phys_addr_t pa)
> +{
> + resource_size_t start, end;
> + int bar;
> +
> + for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
> + /* Skip BARs not present or not memory-mapped */
> + if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
> + continue;
> +
> + start = pci_resource_start(pdev, bar);
> + end = pci_resource_end(pdev, bar);
> +
> + if (!start || !end)
> + continue;
> +
> + if (pa >= start && pa <= end)
> + return bar;
> + }
Don't we know which of the two BARs the mmap entry came from based on
its type? This seems like overkill..
Jason
Changelog:
v2:
* Changed series to document the revoke semantics instead of
implementing it.
v1: https://patch.msgid.link/20260111-dmabuf-revoke-v1-0-fb4bcc8c259b@nvidia.com
-------------------------------------------------------------------------
This series documents a dma-buf “revoke” mechanism: to allow a dma-buf
exporter to explicitly invalidate (“kill”) a shared buffer after it has
been distributed to importers, so that further CPU and device access is
prevented and importers reliably observe failure.
The change in this series is to properly document and use existing core
“revoked” state on the dma-buf object and a corresponding exporter-triggered
revoke operation. Once a dma-buf is revoked, new access paths are blocked so
that attempts to DMA-map, vmap, or mmap the buffer fail in a consistent way.
Thanks
Cc: linux-media(a)vger.kernel.org
Cc: dri-devel(a)lists.freedesktop.org
Cc: linaro-mm-sig(a)lists.linaro.org
Cc: linux-kernel(a)vger.kernel.org
Cc: amd-gfx(a)lists.freedesktop.org
Cc: virtualization(a)lists.linux.dev
Cc: intel-xe(a)lists.freedesktop.org
Cc: linux-rdma(a)vger.kernel.org
Cc: iommu(a)lists.linux.dev
Cc: kvm(a)vger.kernel.org
To: Sumit Semwal <sumit.semwal(a)linaro.org>
To: Christian König <christian.koenig(a)amd.com>
To: Alex Deucher <alexander.deucher(a)amd.com>
To: David Airlie <airlied(a)gmail.com>
To: Simona Vetter <simona(a)ffwll.ch>
To: Gerd Hoffmann <kraxel(a)redhat.com>
To: Dmitry Osipenko <dmitry.osipenko(a)collabora.com>
To: Gurchetan Singh <gurchetansingh(a)chromium.org>
To: Chia-I Wu <olvaffe(a)gmail.com>
To: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com>
To: Maxime Ripard <mripard(a)kernel.org>
To: Thomas Zimmermann <tzimmermann(a)suse.de>
To: Lucas De Marchi <lucas.demarchi(a)intel.com>
To: Thomas Hellström <thomas.hellstrom(a)linux.intel.com>
To: Rodrigo Vivi <rodrigo.vivi(a)intel.com>
To: Jason Gunthorpe <jgg(a)ziepe.ca>
To: Leon Romanovsky <leon(a)kernel.org>
To: Kevin Tian <kevin.tian(a)intel.com>
To: Joerg Roedel <joro(a)8bytes.org>
To: Will Deacon <will(a)kernel.org>
To: Robin Murphy <robin.murphy(a)arm.com>
To: Alex Williamson <alex(a)shazbot.org>
---
Leon Romanovsky (4):
dma-buf: Rename .move_notify() callback to a clearer identifier
dma-buf: Document revoke semantics
iommufd: Require DMABUF revoke semantics
vfio: Add pinned interface to perform revoke semantics
drivers/dma-buf/dma-buf.c | 6 +++---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 4 ++--
drivers/gpu/drm/virtio/virtgpu_prime.c | 2 +-
drivers/gpu/drm/xe/tests/xe_dma_buf.c | 6 +++---
drivers/gpu/drm/xe/xe_dma_buf.c | 2 +-
drivers/infiniband/core/umem_dmabuf.c | 4 ++--
drivers/infiniband/hw/mlx5/mr.c | 2 +-
drivers/iommu/iommufd/pages.c | 11 +++++++++--
drivers/vfio/pci/vfio_pci_dmabuf.c | 16 ++++++++++++++++
include/linux/dma-buf.h | 25 ++++++++++++++++++++++---
10 files changed, 60 insertions(+), 18 deletions(-)
---
base-commit: 9ace4753a5202b02191d54e9fdf7f9e3d02b85eb
change-id: 20251221-dmabuf-revoke-b90ef16e4236
Best regards,
--
Leon Romanovsky <leonro(a)nvidia.com>
Hi everyone,
dma_fences have ever lived under the tyranny dictated by the module
lifetime of their issuer, leading to crashes should anybody still holding
a reference to a dma_fence when the module of the issuer was unloaded.
The basic problem is that when buffer are shared between drivers
dma_fence objects can leak into external drivers and stay there even
after they are signaled. The dma_resv object for example only lazy releases
dma_fences.
So what happens is that when the module who originally created the dma_fence
unloads the dma_fence_ops function table becomes unavailable as well and so
any attempt to release the fence crashes the system.
Previously various approaches have been discussed, including changing the
locking semantics of the dma_fence callbacks (by me) as well as using the
drm scheduler as intermediate layer (by Sima) to disconnect dma_fences
from their actual users, but none of them are actually solving all problems.
Tvrtko did some really nice prerequisite work by protecting the returned
strings of the dma_fence_ops by RCU. This way dma_fence creators where
able to just wait for an RCU grace period after fence signaling before
they could be save to free those data structures.
Now this patch set here goes a step further and protects the whole
dma_fence_ops structure by RCU, so that after the fence signals the
pointer to the dma_fence_ops is set to NULL when there is no wait nor
release callback given. All functionality which use the dma_fence_ops
reference are put inside an RCU critical section, except for the
deprecated issuer specific wait and of course the optional release
callback.
Additional to the RCU changes the lock protecting the dma_fence state
previously had to be allocated external. This set here now changes the
functionality to make that external lock optional and allows dma_fences
to use an inline lock and be self contained.
v4:
Rebases the whole set on upstream changes, especially the cleanup
from Philip in patch "drm/amdgpu: independence for the amdkfd_fence!".
Adding two patches which brings the DMA-fence self tests up to date.
The first selftest changes removes the mock_wait and so actually starts
testing the default behavior instead of some hacky implementation in the
test. This one got upstreamed independent of this set.
The second drops the mock_fence as well and tests the new RCU and inline
spinlock functionality.
v5:
Rebase on top of drm-misc-next instead of drm-tip, leave out all driver
changes for now since those should go through the driver specific paths
anyway.
Address a few more review comments, especially some rebase mess and
typos. And finally fix one more bug found by AMDs CI system.
v6:
Minor style changes, re-ordered patch #1, dropped the scheduler fence
change for now
Please review and comment,
Christian.