[PATCH v3 0/2] rust: drm: gpuvm: implement Send and Sync for refcounted handles

[PATCH v3 0/2] rust: drm: gpuvm: implement Send and Sync for refcounted handles

[Sami Tolvanen]

Hi folks,

GpuVaAlloc and GpuVmBo implement neither Send or Sync, so drivers
that move them between threads need their own unsafe impls. GpuVm
implements both unconditionally, which Sashiko points out is unsound.

Patch 1 adds Send and Sync for GpuVaAlloc (unconditionally) and
GpuVmBo (similarly to Arc). Patch 2 gives GpuVm the same bounds as
it can alias types using obtain() and drop them across the threads
via deferred_cleanup().

Tested with downstream Tyr as there don't appear to be any in-tree
users yet.

	Sami

---
v3:
- Added a patch for GpuVm, and changed GpuVmBo bounds based on
  another Sashiko analysis.

v2:
- Added a missing T::Object: Send + Sync bound pointed out by
  Sashiko.

---
Sami Tolvanen (2):
  rust: drm: gpuvm: implement Send and Sync for GpuVaAlloc and GpuVmBo
  rust: drm: gpuvm: require Send and Sync for GpuVm's shared data

 rust/kernel/drm/gpuvm/mod.rs   | 22 +++++++++++++++++++---
 rust/kernel/drm/gpuvm/va.rs    |  8 ++++++++
 rust/kernel/drm/gpuvm/vm_bo.rs | 22 ++++++++++++++++++++++
 3 files changed, 49 insertions(+), 3 deletions(-)


base-commit: fea3a2dd7d3fc1936211ced5f84420e610435730
-- 
2.54.0.1099.g489fc7bff1-goog

[PATCH v3 1/2] rust: drm: gpuvm: implement Send and Sync for GpuVaAlloc and GpuVmBo

[Sami Tolvanen]

Moving a GpuVaAlloc or GpuVmBo between threads currently forces drivers
to write their own unsafe Send and Sync impls. Provide the markers in
the abstraction instead.

GpuVaAlloc wraps only uninitialised memory and exposes none of it, so it
is unconditionally Send and Sync. GpuVmBo is an atomically refcounted
handle whose accessors hand out the driver data and GEM object by shared
reference and whose deferred put drops them, so its Send and Sync impls
are bounded on T::VmBoData and T::Object.

Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 rust/kernel/drm/gpuvm/va.rs    |  8 ++++++++
 rust/kernel/drm/gpuvm/vm_bo.rs | 22 ++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/rust/kernel/drm/gpuvm/va.rs b/rust/kernel/drm/gpuvm/va.rs
index 0b09fe44ab39..b108ec7aa1bc 100644
--- a/rust/kernel/drm/gpuvm/va.rs
+++ b/rust/kernel/drm/gpuvm/va.rs
@@ -104,6 +104,14 @@ pub fn vm_bo(&self) -> &GpuVmBo<T> {
 /// The memory is zeroed.
 pub struct GpuVaAlloc<T: DriverGpuVm>(KBox<MaybeUninit<GpuVa<T>>>);
 
+// SAFETY: A `GpuVaAlloc` is an owned, uninitialised allocation with no live `T::VaData` and no
+// thread-bound state.
+unsafe impl<T: DriverGpuVm> Send for GpuVaAlloc<T> {}
+
+// SAFETY: A `GpuVaAlloc` has no `&self` method that reaches its contents, so a shared
+// `&GpuVaAlloc` cannot access the allocation.
+unsafe impl<T: DriverGpuVm> Sync for GpuVaAlloc<T> {}
+
 impl<T: DriverGpuVm> GpuVaAlloc<T> {
     /// Pre-allocate a [`GpuVa`] object.
     pub fn new(flags: AllocFlags) -> Result<GpuVaAlloc<T>, AllocError> {
diff --git a/rust/kernel/drm/gpuvm/vm_bo.rs b/rust/kernel/drm/gpuvm/vm_bo.rs
index c064ac63897b..c5e3bb44a2ee 100644
--- a/rust/kernel/drm/gpuvm/vm_bo.rs
+++ b/rust/kernel/drm/gpuvm/vm_bo.rs
@@ -19,6 +19,28 @@ pub struct GpuVmBo<T: DriverGpuVm> {
     data: T::VmBoData,
 }
 
+// SAFETY: It is safe to send a `GpuVmBo<T>` to another thread when `T::VmBoData` and `T::Object`
+// are `Sync` because `data()` and `obj()` share `&T::VmBoData` and `&T::Object`; they must also be
+// `Send` because the last reference drop runs their destructors on whichever thread drains the
+// deferred-cleanup queue.
+unsafe impl<T: DriverGpuVm> Send for GpuVmBo<T>
+where
+    T::VmBoData: Send + Sync,
+    T::Object: Send + Sync,
+{
+}
+
+// SAFETY: It is safe to send `&GpuVmBo<T>` to another thread when `T::VmBoData` and `T::Object` are
+// `Sync` because `data()` and `obj()` share `&T::VmBoData` and `&T::Object`; they must also be
+// `Send` because any thread with a `&GpuVmBo<T>` can clone it via `ARef::from`, whose last drop
+// runs their destructors on whichever thread drains the deferred-cleanup queue.
+unsafe impl<T: DriverGpuVm> Sync for GpuVmBo<T>
+where
+    T::VmBoData: Send + Sync,
+    T::Object: Send + Sync,
+{
+}
+
 // SAFETY: By type invariants, the allocation is managed by the refcount in `self.inner`.
 unsafe impl<T: DriverGpuVm> AlwaysRefCounted for GpuVmBo<T> {
     fn inc_ref(&self) {
-- 
2.54.0.1099.g489fc7bff1-goog

[PATCH v3 2/2] rust: drm: gpuvm: require Send and Sync for GpuVm's shared data

[Sami Tolvanen]

GpuVm implements Send and Sync unconditionally, but it shares and drops
T::VmBoData and T::Object across threads: obtain() can return ARefs to
the same bo on several threads, and deferred_cleanup() drops them on
whichever thread calls it. This is unsound unless both are Send + Sync,
so bound both impls accordingly.

Fixes: 82b78182eacf ("rust: drm: add base GPUVM immediate mode abstraction")
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 rust/kernel/drm/gpuvm/mod.rs | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs
index ae58f6f667c1..398068f2eb4f 100644
--- a/rust/kernel/drm/gpuvm/mod.rs
+++ b/rust/kernel/drm/gpuvm/mod.rs
@@ -74,9 +74,25 @@ pub struct GpuVm<T: DriverGpuVm> {
 
 // SAFETY: The GPUVM api does not assume that it is tied to a specific thread. The destructor will
 // drop the `data` field, which is okay because it is guaranteed `Send` by the `DriverGpuVm` trait.
-unsafe impl<T: DriverGpuVm> Send for GpuVm<T> {}
-// SAFETY: The GPUVM api is designed to allow &self methods to be called in parallel.
-unsafe impl<T: DriverGpuVm> Sync for GpuVm<T> {}
+// `obtain()` called from several threads can return `ARef`s to the same bo, aliasing `T::VmBoData`
+// and `T::Object`, and `deferred_cleanup()` can drop them from any thread, so both must be
+// `Send + Sync`.
+unsafe impl<T: DriverGpuVm> Send for GpuVm<T>
+where
+    T::VmBoData: Send + Sync,
+    T::Object: Send + Sync,
+{
+}
+// SAFETY: The GPUVM api is designed to allow &self methods to be called in parallel. `obtain()`
+// called from several threads can return `ARef`s to the same bo, aliasing `T::VmBoData` and
+// `T::Object`, and `deferred_cleanup()` can drop them from any thread, so both must be
+// `Send + Sync`.
+unsafe impl<T: DriverGpuVm> Sync for GpuVm<T>
+where
+    T::VmBoData: Send + Sync,
+    T::Object: Send + Sync,
+{
+}
 
 // SAFETY: By type invariants, the allocation is managed by the refcount in `self.vm`.
 unsafe impl<T: DriverGpuVm> AlwaysRefCounted for GpuVm<T> {
-- 
2.54.0.1099.g489fc7bff1-goog

Re: [PATCH v3 2/2] rust: drm: gpuvm: require Send and Sync for GpuVm's shared data

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] GpuVm's Send and Sync implementations omit bounds for T::VaData, allowing !Send data to be moved across threads, leading to data races.
--

commit 2dbb42abd1672fd0f3c87ba84b8650b37d9c9e84
Author: Sami Tolvanen <samitolvanen@google.com>

rust: drm: gpuvm: require Send and Sync for GpuVm's shared data

This commit adds explicit Send and Sync bounds for T::VmBoData and
T::Object to the GpuVm Send and Sync implementations. It addresses
unsoundness when obtain() aliases these types across threads.

> diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs
> index ae58f6f667c1f..398068f2eb4f7 100644
> --- a/rust/kernel/drm/gpuvm/mod.rs
> +++ b/rust/kernel/drm/gpuvm/mod.rs
> @@ -74,9 +74,25 @@ pub struct GpuVm<T: DriverGpuVm> {
>  
>  // SAFETY: The GPUVM api does not assume that it is tied to a specific thread. The destructor will
>  // drop the `data` field, which is okay because it is guaranteed `Send` by the `DriverGpuVm` trait.
> -unsafe impl<T: DriverGpuVm> Send for GpuVm<T> {}
> -// SAFETY: The GPUVM api is designed to allow &self methods to be called in parallel.
> -unsafe impl<T: DriverGpuVm> Sync for GpuVm<T> {}
> +// `obtain()` called from several threads can return `ARef`s to the same bo, aliasing `T::VmBoData`
> +// and `T::Object`, and `deferred_cleanup()` can drop them from any thread, so both must be
> +// `Send + Sync`.
> +unsafe impl<T: DriverGpuVm> Send for GpuVm<T>
> +where
> +    T::VmBoData: Send + Sync,
> +    T::Object: Send + Sync,
> +{
> +}

[Severity: Critical]
This is a pre-existing issue, but does GpuVm also need Send + Sync bounds
for T::VaData here?

T::VaData is stored within GpuVa<T>, which is logically owned by the GPUVM's
interval tree. Because GpuVm<T> does not require T::VaData to be Send + Sync,
a user could implement DriverGpuVm with a !Send type for VaData.

Since GpuVm<T> explicitly bypasses the lack of Send on VaData,
UniqueRefGpuVm<T> automatically derives Send. This allows safe code to map
a VA containing !Send data on one thread, send the UniqueRefGpuVm<T> to
another thread, and unmap the VA there.

Unmapping via sm_unmap() returns a GpuVaRemoved<T> which drops the !Send
VaData on the wrong thread, potentially causing data races.

> +// SAFETY: The GPUVM api is designed to allow &self methods to be called in parallel. `obtain()`
> +// called from several threads can return `ARef`s to the same bo, aliasing `T::VmBoData` and
> +// `T::Object`, and `deferred_cleanup()` can drop them from any thread, so both must be
> +// `Send + Sync`.
> +unsafe impl<T: DriverGpuVm> Sync for GpuVm<T>
> +where
> +    T::VmBoData: Send + Sync,
> +    T::Object: Send + Sync,
> +{
> +}

[Severity: Critical]
This is a pre-existing issue, but should the Sync implementation also include
Send + Sync bounds for T::VaData for the same reasons?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609003256.1829625-4-samitolvanen@google.com?part=2

Re: [PATCH v3 2/2] rust: drm: gpuvm: require Send and Sync for GpuVm's shared data

[Sami Tolvanen]

On Mon, Jun 8, 2026 at 5:33 PM Sami Tolvanen <samitolvanen@google.com> wrote:
>
> GpuVm implements Send and Sync unconditionally, but it shares and drops
> T::VmBoData and T::Object across threads: obtain() can return ARefs to
> the same bo on several threads, and deferred_cleanup() drops them on
> whichever thread calls it. This is unsound unless both are Send + Sync,
> so bound both impls accordingly.

Sashiko keeps finding more issues here. For the benefit of those not
subscribed to dri-devel:

> This is a pre-existing issue, but does GpuVm also need Send + Sync bounds
> for T::VaData here?
>
> Since GpuVm<T> explicitly bypasses the lack of Send on VaData,
> UniqueRefGpuVm<T> automatically derives Send. This allows safe code to map
> a VA containing !Send data on one thread, send the UniqueRefGpuVm<T> to
> another thread, and unmap the VA there.
>
> Unmapping via sm_unmap() returns a GpuVaRemoved<T> which drops the !Send
> VaData on the wrong thread, potentially causing data races.

Looking at the code, UniqueRefGpuVm seems like a better place for this
than GpuVm. However, before I go add another patch to the series, I
wonder if it would make more sense to just add these bounds to the
DriverGpuVm trait instead? I'd be happy to hear some non-AI thoughts
about this too.

Sami

Re: [PATCH v3 2/2] rust: drm: gpuvm: require Send and Sync for GpuVm's shared data

[Alice Ryhl]

On Mon, Jun 08, 2026 at 06:54:28PM -0700, Sami Tolvanen wrote:
> On Mon, Jun 8, 2026 at 5:33 PM Sami Tolvanen <samitolvanen@google.com> wrote:
> >
> > GpuVm implements Send and Sync unconditionally, but it shares and drops
> > T::VmBoData and T::Object across threads: obtain() can return ARefs to
> > the same bo on several threads, and deferred_cleanup() drops them on
> > whichever thread calls it. This is unsound unless both are Send + Sync,
> > so bound both impls accordingly.
> 
> Sashiko keeps finding more issues here. For the benefit of those not
> subscribed to dri-devel:
> 
> > This is a pre-existing issue, but does GpuVm also need Send + Sync bounds
> > for T::VaData here?
> >
> > Since GpuVm<T> explicitly bypasses the lack of Send on VaData,
> > UniqueRefGpuVm<T> automatically derives Send. This allows safe code to map
> > a VA containing !Send data on one thread, send the UniqueRefGpuVm<T> to
> > another thread, and unmap the VA there.
> >
> > Unmapping via sm_unmap() returns a GpuVaRemoved<T> which drops the !Send
> > VaData on the wrong thread, potentially causing data races.
> 
> Looking at the code, UniqueRefGpuVm seems like a better place for this
> than GpuVm. However, before I go add another patch to the series, I
> wonder if it would make more sense to just add these bounds to the
> DriverGpuVm trait instead? I'd be happy to hear some non-AI thoughts
> about this too.

Adding bounds on the trait does sound like a good idea for simplicity.
It's not like we're ever going to use GPUVM with non-thread-safe types.

If it's help, note that you can use `where GpuVm<T>: Send` bounds or
similar to "reuse" the implementations elsewhere. For example,
GpuVmBo<T> being Sync requires GpuVm<T> being Sync because of the
GpuVmBo::gpuvm() method, so you can use `where GpuVm<T>: Send` as a
bound to directly express that if you want.

But if we just add bounds to the trait, it may not be necessary.

Alice