[PATCH v4 1/2] rust: drm: gpuvm: require Send + Sync for the driver's associated data

[Sami Tolvanen]

DriverGpuVm permitted !Send/!Sync associated data on an abstraction whose
handles are shared and dropped across threads: obtain() runs from many
threads and the VA API performs deferred cross-thread drops. That is
unsound.

Require Send + Sync on the trait and its associated data so the GpuVm and
UniqueRefGpuVm handle impls need no per-impl bounds.

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

diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs
index a625fcd9b5f2..70cf11346cee 100644
--- a/rust/kernel/drm/gpuvm/mod.rs
+++ b/rust/kernel/drm/gpuvm/mod.rs
@@ -72,10 +72,12 @@ pub struct GpuVm<T: DriverGpuVm> {
     data: UnsafeCell<T>,
 }
 
-// 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.
+// SAFETY: It is safe to send a `GpuVm<T>` to another thread: all data reachable through it
+// (`T`, `T::VmBoData`, and the GEM `T::Object`) is `Send` by the `DriverGpuVm` bounds.
 unsafe impl<T: DriverGpuVm> Send for GpuVm<T> {}
-// SAFETY: The GPUVM api is designed to allow &self methods to be called in parallel.
+// SAFETY: It is safe to share a `&GpuVm<T>` between threads: `&self` methods only alias data
+// that is `Sync` by the `DriverGpuVm` bounds, and any thread may drop that data, or upgrade the
+// reference and ultimately drop `T`, which the same bounds make `Send`.
 unsafe impl<T: DriverGpuVm> Sync for GpuVm<T> {}
 
 // SAFETY: By type invariants, the allocation is managed by the refcount in `self.vm`.
@@ -250,18 +252,22 @@ fn raw_resv(&self) -> *mut bindings::dma_resv {
 }
 
 /// The manager for a GPUVM.
-pub trait DriverGpuVm: Sized + Send {
+pub trait DriverGpuVm: Sized + Send + Sync {
     /// Parent `Driver` for this object.
     type Driver: drm::Driver;
 
     /// The kind of GEM object stored in this GPUVM.
-    type Object: drm::driver::AllocImpl<Driver = Self::Driver>;
+    type Object: drm::driver::AllocImpl<Driver = Self::Driver> + Send + Sync;
 
     /// Data stored with each [`struct drm_gpuva`](struct@GpuVa).
-    type VaData;
+    ///
+    /// Only `Send` is required: the data has a single owner at all times, moving
+    /// between threads by value (handed back as a [`GpuVaRemoved`]) but never
+    /// accessed by two threads concurrently.
+    type VaData: Send;
 
     /// Data stored with each [`struct drm_gpuvm_bo`](struct@GpuVmBo).
-    type VmBoData;
+    type VmBoData: Send + Sync;
 
     /// The private data passed to callbacks.
     type SmContext<'ctx>;
@@ -296,12 +302,10 @@ fn sm_step_remap<'op, 'ctx>(
 /// # Invariants
 ///
 /// Each `GpuVm` instance has at most one `UniqueRefGpuVm` reference.
+// `Send`/`Sync` derive from `ARef<GpuVm<T>>`; the trait bounds make them correct for the unique
+// handle's `&mut T` access.
 pub struct UniqueRefGpuVm<T: DriverGpuVm>(ARef<GpuVm<T>>);
 
-// SAFETY: The GPUVM api is designed to allow &self methods to be called in parallel, and
-// concurrent access to `data` is safe due to the `T: Sync` requirement.
-unsafe impl<T: DriverGpuVm + Sync> Sync for UniqueRefGpuVm<T> {}
-
 impl<T: DriverGpuVm> UniqueRefGpuVm<T> {
     /// Access the data owned by this `UniqueRefGpuVm` immutably.
     #[inline]

-- 
2.54.0.1136.gdb2ca164c4-goog