public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/6] Rust GPUVM immediate mode
@ 2026-01-21 11:31 Alice Ryhl
  2026-01-21 11:31 ` [PATCH v3 1/6] rust: drm: add base GPUVM immediate mode abstraction Alice Ryhl
                   ` (5 more replies)
  0 siblings, 6 replies; 22+ messages in thread
From: Alice Ryhl @ 2026-01-21 11:31 UTC (permalink / raw)
  To: Danilo Krummrich, Daniel Almeida
  Cc: Boris Brezillon, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux, Alice Ryhl

This series provides an immediate mode GPUVM implementation.

Only immediate mode is provided for Rust code, as all planned Rust
drivers intend to use GPUVM in immediate mode.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Changes in v3:
- C prerequisites have landed, so only Rust part is present.
- The logic for drm_exec was removed, and is for a follow-up.
- Split up into patches.
- Add lifetime to SmStepContext.
- Docs filled out.
- Mutex abstractions used for GEM gpuva lock.
- Drop 'shared data' concept for now. (Can be added back later if required.)
- Rename 'core' field to 'data'.
- GpuVmCore<T> now derefs to GpuVm<T> instead of T.
- Renamed GpuVmBoObtain to GpuVmBoResident.
- Probably more changes I forgot.
- Link to v2: https://lore.kernel.org/r/20260108-gpuvm-rust-v2-0-dbd014005a0b@google.com

Changes in v2:
- For this version, only the C prerequisites are included. Rust will be
  sent as follow-up.
- Add comment to drm_gpuvm_bo_destroy_not_in_lists()
- Add Fixes: tag.
- Pick up Reviewed-by tags.
- Link to v1: https://lore.kernel.org/r/20251128-gpuvm-rust-v1-0-ebf66bf234e0@google.com

---
Alice Ryhl (4):
      rust: gpuvm: add GpuVm::obtain()
      rust: gpuvm: add GpuVa struct
      rust: gpuvm: add GpuVmCore::sm_unmap()
      rust: gpuvm: add GpuVmCore::sm_map()

Asahi Lina (2):
      rust: drm: add base GPUVM immediate mode abstraction
      rust: helpers: Add bindings/wrappers for dma_resv_lock

 MAINTAINERS                     |   2 +
 rust/bindings/bindings_helper.h |   2 +
 rust/helpers/dma-resv.c         |  13 ++
 rust/helpers/drm_gpuvm.c        |  18 ++
 rust/helpers/helpers.c          |   2 +
 rust/kernel/drm/gpuvm/mod.rs    | 299 +++++++++++++++++++++++++++++
 rust/kernel/drm/gpuvm/sm_ops.rs | 408 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/drm/gpuvm/va.rs     | 148 +++++++++++++++
 rust/kernel/drm/gpuvm/vm_bo.rs  | 227 ++++++++++++++++++++++
 rust/kernel/drm/mod.rs          |   1 +
 10 files changed, 1120 insertions(+)
---
base-commit: 263e9ef3f5736697483af66babb8bc72f145b3f4
change-id: 20251128-gpuvm-rust-b719cac27ad6

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 1/6] rust: drm: add base GPUVM immediate mode abstraction
  2026-01-21 11:31 [PATCH v3 0/6] Rust GPUVM immediate mode Alice Ryhl
@ 2026-01-21 11:31 ` Alice Ryhl
  2026-01-21 17:04   ` Daniel Almeida
  2026-01-21 11:31 ` [PATCH v3 2/6] rust: helpers: Add bindings/wrappers for dma_resv_lock Alice Ryhl
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 22+ messages in thread
From: Alice Ryhl @ 2026-01-21 11:31 UTC (permalink / raw)
  To: Danilo Krummrich, Daniel Almeida
  Cc: Boris Brezillon, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux, Alice Ryhl

From: Asahi Lina <lina+kernel@asahilina.net>

Add a GPUVM abstraction to be used by Rust GPU drivers.

GPUVM keeps track of a GPU's virtual address (VA) space and manages the
corresponding virtual mappings represented by "GPU VA" objects. It also
keeps track of the gem::Object<T> used to back the mappings through
GpuVmBo<T>.

This abstraction is only usable by drivers that wish to use GPUVM in
immediate mode. This allows us to build the locking scheme into the API
design. It means that the GEM mutex is used for the GEM gpuva list, and
that the resv lock is used for the extobj list. The evicted list is not
yet used in this version.

This abstraction provides a special handle called the GpuVmCore, which
is a wrapper around ARef<GpuVm> that provides access to the interval
tree. Generally, all changes to the address space requires mutable
access to this unique handle.

Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
Co-developed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
Co-developed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 MAINTAINERS                     |   2 +
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/drm_gpuvm.c        |  18 ++++
 rust/helpers/helpers.c          |   1 +
 rust/kernel/drm/gpuvm/mod.rs    | 231 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/drm/mod.rs          |   1 +
 6 files changed, 254 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3b84ad595e226f231b256d24f0da6bac459e93a8..618becae72985b9dfdca8469ee48d4752fd0ca41 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8720,6 +8720,8 @@ S:	Supported
 T:	git https://gitlab.freedesktop.org/drm/misc/kernel.git
 F:	drivers/gpu/drm/drm_gpuvm.c
 F:	include/drm/drm_gpuvm.h
+F:	rust/helpers/drm_gpuvm.c
+F:	rust/kernel/drm/gpuvm/
 
 DRM LOG
 M:	Jocelyn Falempe <jfalempe@redhat.com>
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index a067038b4b422b4256f4a2b75fe644d47e6e82c8..dd60a5c6b142ec2c5fd6df80279ab6813163791c 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -33,6 +33,7 @@
 #include <drm/drm_drv.h>
 #include <drm/drm_file.h>
 #include <drm/drm_gem.h>
+#include <drm/drm_gpuvm.h>
 #include <drm/drm_ioctl.h>
 #include <kunit/test.h>
 #include <linux/auxiliary_bus.h>
diff --git a/rust/helpers/drm_gpuvm.c b/rust/helpers/drm_gpuvm.c
new file mode 100644
index 0000000000000000000000000000000000000000..d1471e5844ec81f994af9252d9054053ab13f352
--- /dev/null
+++ b/rust/helpers/drm_gpuvm.c
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0 or MIT
+
+#ifdef CONFIG_DRM_GPUVM
+
+#include <drm/drm_gpuvm.h>
+
+struct drm_gpuvm *rust_helper_drm_gpuvm_get(struct drm_gpuvm *obj)
+{
+	return drm_gpuvm_get(obj);
+}
+
+bool rust_helper_drm_gpuvm_is_extobj(struct drm_gpuvm *gpuvm,
+				     struct drm_gem_object *obj)
+{
+	return drm_gpuvm_is_extobj(gpuvm, obj);
+}
+
+#endif // CONFIG_DRM_GPUVM
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 79c72762ad9c4b473971e6210c9577860d2e2b08..0943d589b7578d3c0e207937f63a5e02719c6146 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -26,6 +26,7 @@
 #include "device.c"
 #include "dma.c"
 #include "drm.c"
+#include "drm_gpuvm.c"
 #include "err.c"
 #include "irq.c"
 #include "fs.c"
diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs
new file mode 100644
index 0000000000000000000000000000000000000000..81b5e767885d8258c44086444b153c91961ffabc
--- /dev/null
+++ b/rust/kernel/drm/gpuvm/mod.rs
@@ -0,0 +1,231 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+
+#![cfg(CONFIG_DRM_GPUVM = "y")]
+
+//! DRM GPUVM in immediate mode
+//!
+//! Rust abstractions for using GPUVM in immediate mode. This is when the GPUVM state is updated
+//! during `run_job()`, i.e., in the DMA fence signalling critical path, to ensure that the GPUVM
+//! and the GPU's virtual address space has the same state at all times.
+//!
+//! C header: [`include/drm/drm_gpuvm.h`](srctree/include/drm/drm_gpuvm.h)
+
+use kernel::{
+    alloc::AllocError,
+    bindings,
+    drm,
+    drm::gem::IntoGEMObject,
+    prelude::*,
+    sync::aref::{
+        ARef,
+        AlwaysRefCounted, //
+    },
+    types::Opaque, //
+};
+
+use core::{
+    cell::UnsafeCell,
+    ops::{
+        Deref,
+        Range, //
+    },
+    ptr::NonNull, //
+};
+
+/// A DRM GPU VA manager.
+///
+/// This object is refcounted, but the "core" is only accessible using a special unique handle. The
+/// core consists of the `core` field and the GPUVM's interval tree.
+///
+/// # Invariants
+///
+/// * Stored in an allocation managed by the refcount in `self.vm`.
+/// * Access to `data` and the gpuvm interval tree is controlled via the [`GpuVmCore`] type.
+#[pin_data]
+pub struct GpuVm<T: DriverGpuVm> {
+    #[pin]
+    vm: Opaque<bindings::drm_gpuvm>,
+    /// Accessed only through the [`GpuVmCore`] reference.
+    data: UnsafeCell<T>,
+}
+
+// SAFETY: By type invariants, the allocation is managed by the refcount in `self.vm`.
+unsafe impl<T: DriverGpuVm> AlwaysRefCounted for GpuVm<T> {
+    fn inc_ref(&self) {
+        // SAFETY: By type invariants, the allocation is managed by the refcount in `self.vm`.
+        unsafe { bindings::drm_gpuvm_get(self.vm.get()) };
+    }
+
+    unsafe fn dec_ref(obj: NonNull<Self>) {
+        // SAFETY: By type invariants, the allocation is managed by the refcount in `self.vm`.
+        unsafe { bindings::drm_gpuvm_put((*obj.as_ptr()).vm.get()) };
+    }
+}
+
+impl<T: DriverGpuVm> GpuVm<T> {
+    const fn vtable() -> &'static bindings::drm_gpuvm_ops {
+        &bindings::drm_gpuvm_ops {
+            vm_free: Some(Self::vm_free),
+            op_alloc: None,
+            op_free: None,
+            vm_bo_alloc: None,
+            vm_bo_free: None,
+            vm_bo_validate: None,
+            sm_step_map: None,
+            sm_step_unmap: None,
+            sm_step_remap: None,
+        }
+    }
+
+    /// Creates a GPUVM instance.
+    #[expect(clippy::new_ret_no_self)]
+    pub fn new<E>(
+        name: &'static CStr,
+        dev: &drm::Device<T::Driver>,
+        r_obj: &T::Object,
+        range: Range<u64>,
+        reserve_range: Range<u64>,
+        data: T,
+    ) -> Result<GpuVmCore<T>, E>
+    where
+        E: From<AllocError>,
+        E: From<core::convert::Infallible>,
+    {
+        let obj = KBox::try_pin_init::<E>(
+            try_pin_init!(Self {
+                data: UnsafeCell::new(data),
+                vm <- Opaque::ffi_init(|vm| {
+                    // SAFETY: These arguments are valid. `vm` is valid until refcount drops to
+                    // zero.
+                    unsafe {
+                        bindings::drm_gpuvm_init(
+                            vm,
+                            name.as_char_ptr(),
+                            bindings::drm_gpuvm_flags_DRM_GPUVM_IMMEDIATE_MODE
+                                | bindings::drm_gpuvm_flags_DRM_GPUVM_RESV_PROTECTED,
+                            dev.as_raw(),
+                            r_obj.as_raw(),
+                            range.start,
+                            range.end - range.start,
+                            reserve_range.start,
+                            reserve_range.end - reserve_range.start,
+                            const { Self::vtable() },
+                        )
+                    }
+                }),
+            }? E),
+            GFP_KERNEL,
+        )?;
+        // SAFETY: This transfers the initial refcount to the ARef.
+        Ok(GpuVmCore(unsafe {
+            ARef::from_raw(NonNull::new_unchecked(KBox::into_raw(
+                Pin::into_inner_unchecked(obj),
+            )))
+        }))
+    }
+
+    /// Access this [`GpuVm`] from a raw pointer.
+    ///
+    /// # Safety
+    ///
+    /// The pointer must reference the `struct drm_gpuvm` in a valid [`GpuVm<T>`] that remains
+    /// valid for at least `'a`.
+    #[inline]
+    pub unsafe fn from_raw<'a>(ptr: *mut bindings::drm_gpuvm) -> &'a Self {
+        // SAFETY: Caller passes a pointer to the `drm_gpuvm` in a `GpuVm<T>`. Caller ensures the
+        // pointer is valid for 'a.
+        unsafe { &*kernel::container_of!(Opaque::cast_from(ptr), Self, vm) }
+    }
+
+    /// Returns a raw pointer to the embedded `struct drm_gpuvm`.
+    #[inline]
+    pub fn as_raw(&self) -> *mut bindings::drm_gpuvm {
+        self.vm.get()
+    }
+
+    /// The start of the VA space.
+    #[inline]
+    pub fn va_start(&self) -> u64 {
+        // SAFETY: The `mm_start` field is immutable.
+        unsafe { (*self.as_raw()).mm_start }
+    }
+
+    /// The length of the GPU's virtual address space.
+    #[inline]
+    pub fn va_length(&self) -> u64 {
+        // SAFETY: The `mm_range` field is immutable.
+        unsafe { (*self.as_raw()).mm_range }
+    }
+
+    /// Returns the range of the GPU virtual address space.
+    #[inline]
+    pub fn va_range(&self) -> Range<u64> {
+        let start = self.va_start();
+        // OVERFLOW: This reconstructs the Range<u64> passed to the constructor, so it won't fail.
+        let end = start + self.va_length();
+        Range { start, end }
+    }
+
+    /// Clean up buffer objects that are no longer used.
+    #[inline]
+    pub fn deferred_cleanup(&self) {
+        // SAFETY: This GPUVM uses immediate mode.
+        unsafe { bindings::drm_gpuvm_bo_deferred_cleanup(self.as_raw()) }
+    }
+
+    /// Check if this GEM object is an external object for this GPUVM.
+    #[inline]
+    pub fn is_extobj(&self, obj: &T::Object) -> bool {
+        // SAFETY: We may call this with any GPUVM and GEM object.
+        unsafe { bindings::drm_gpuvm_is_extobj(self.as_raw(), obj.as_raw()) }
+    }
+
+    /// Free this GPUVM.
+    ///
+    /// # Safety
+    ///
+    /// Called when refcount hits zero.
+    unsafe extern "C" fn vm_free(me: *mut bindings::drm_gpuvm) {
+        // SAFETY: Caller passes a pointer to the `drm_gpuvm` in a `GpuVm<T>`.
+        let me = unsafe { kernel::container_of!(Opaque::cast_from(me), Self, vm).cast_mut() };
+        // SAFETY: By type invariants we can free it when refcount hits zero.
+        drop(unsafe { KBox::from_raw(me) })
+    }
+}
+
+/// The manager for a GPUVM.
+pub trait DriverGpuVm: Sized {
+    /// Parent `Driver` for this object.
+    type Driver: drm::Driver;
+
+    /// The kind of GEM object stored in this GPUVM.
+    type Object: IntoGEMObject;
+}
+
+/// The core of the DRM GPU VA manager.
+///
+/// This object is a unique reference to the VM that can access the interval tree and the Rust
+/// `data` field.
+///
+/// # Invariants
+///
+/// Each `GpuVm` instance has at most one `GpuVmCore` reference.
+pub struct GpuVmCore<T: DriverGpuVm>(ARef<GpuVm<T>>);
+
+impl<T: DriverGpuVm> GpuVmCore<T> {
+    /// Access the core data of this GPUVM.
+    #[inline]
+    pub fn data(&mut self) -> &mut T {
+        // SAFETY: By the type invariants we may access `core`.
+        unsafe { &mut *self.0.data.get() }
+    }
+}
+
+impl<T: DriverGpuVm> Deref for GpuVmCore<T> {
+    type Target = GpuVm<T>;
+
+    #[inline]
+    fn deref(&self) -> &GpuVm<T> {
+        &self.0
+    }
+}
diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
index 1b82b6945edf25b947afc08300e211bd97150d6b..a4b6c5430198571ec701af2ef452cc9ac55870e6 100644
--- a/rust/kernel/drm/mod.rs
+++ b/rust/kernel/drm/mod.rs
@@ -6,6 +6,7 @@
 pub mod driver;
 pub mod file;
 pub mod gem;
+pub mod gpuvm;
 pub mod ioctl;
 
 pub use self::device::Device;

-- 
2.52.0.457.g6b5491de43-goog


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 2/6] rust: helpers: Add bindings/wrappers for dma_resv_lock
  2026-01-21 11:31 [PATCH v3 0/6] Rust GPUVM immediate mode Alice Ryhl
  2026-01-21 11:31 ` [PATCH v3 1/6] rust: drm: add base GPUVM immediate mode abstraction Alice Ryhl
@ 2026-01-21 11:31 ` Alice Ryhl
  2026-01-21 12:39   ` Daniel Almeida
  2026-01-21 15:44   ` Gary Guo
  2026-01-21 11:31 ` [PATCH v3 3/6] rust: gpuvm: add GpuVm::obtain() Alice Ryhl
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 22+ messages in thread
From: Alice Ryhl @ 2026-01-21 11:31 UTC (permalink / raw)
  To: Danilo Krummrich, Daniel Almeida
  Cc: Boris Brezillon, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux, Alice Ryhl

From: Asahi Lina <lina+kernel@asahilina.net>

This is just for basic usage in the DRM shmem abstractions for implied
locking, not intended as a full DMA Reservation abstraction yet.

Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Janne Grunau <j@jannau.net>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Taken from:
https://lore.kernel.org/all/20251202220924.520644-3-lyude@redhat.com/
---
 rust/bindings/bindings_helper.h |  1 +
 rust/helpers/dma-resv.c         | 13 +++++++++++++
 rust/helpers/helpers.c          |  1 +
 3 files changed, 15 insertions(+)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index dd60a5c6b142ec2c5fd6df80279ab6813163791c..ed80dd8088bc60c67b02f7666e602e33158bb962 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -52,6 +52,7 @@
 #include <linux/device/faux.h>
 #include <linux/dma-direction.h>
 #include <linux/dma-mapping.h>
+#include <linux/dma-resv.h>
 #include <linux/errname.h>
 #include <linux/ethtool.h>
 #include <linux/fdtable.h>
diff --git a/rust/helpers/dma-resv.c b/rust/helpers/dma-resv.c
new file mode 100644
index 0000000000000000000000000000000000000000..05501cb814513b483afd0b7f220230d867863c2f
--- /dev/null
+++ b/rust/helpers/dma-resv.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/dma-resv.h>
+
+int rust_helper_dma_resv_lock(struct dma_resv *obj, struct ww_acquire_ctx *ctx)
+{
+	return dma_resv_lock(obj, ctx);
+}
+
+void rust_helper_dma_resv_unlock(struct dma_resv *obj)
+{
+	dma_resv_unlock(obj);
+}
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 0943d589b7578d3c0e207937f63a5e02719c6146..aae78c938b0b5848b1740fb3f2dc4b7f93b1a760 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -25,6 +25,7 @@
 #include "cred.c"
 #include "device.c"
 #include "dma.c"
+#include "dma-resv.c"
 #include "drm.c"
 #include "drm_gpuvm.c"
 #include "err.c"

-- 
2.52.0.457.g6b5491de43-goog


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 3/6] rust: gpuvm: add GpuVm::obtain()
  2026-01-21 11:31 [PATCH v3 0/6] Rust GPUVM immediate mode Alice Ryhl
  2026-01-21 11:31 ` [PATCH v3 1/6] rust: drm: add base GPUVM immediate mode abstraction Alice Ryhl
  2026-01-21 11:31 ` [PATCH v3 2/6] rust: helpers: Add bindings/wrappers for dma_resv_lock Alice Ryhl
@ 2026-01-21 11:31 ` Alice Ryhl
  2026-01-21 17:31   ` Daniel Almeida
  2026-01-26 15:00   ` Boris Brezillon
  2026-01-21 11:31 ` [PATCH v3 4/6] rust: gpuvm: add GpuVa struct Alice Ryhl
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 22+ messages in thread
From: Alice Ryhl @ 2026-01-21 11:31 UTC (permalink / raw)
  To: Danilo Krummrich, Daniel Almeida
  Cc: Boris Brezillon, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux, Alice Ryhl

This provides a mechanism to create (or look up) VMBO instances, which
represent the mapping between GPUVM and GEM objects.

The GpuVmBoResident<T> type can be considered like ARef<GpuVm<T>>,
except that no way to increment the refcount is provided.

The GpuVmBoAlloc<T> type is more akin to a pre-allocated GpuVm<T>, so
it's not really a GpuVm<T> yet. Its destructor could call
drm_gpuvm_bo_destroy_not_in_lists(), but as the type is currently
private and never called anywhere, this perf optimization does not need
to happen now.

Pre-allocating and obtaining the gpuvm_bo object is exposed as a single
step. This could theoretically be a problem if one wanted to call
drm_gpuvm_bo_obtain_prealloc() during the fence signalling critical
path, but that's not a possibility because:

1. Adding the BO to the extobj list requires the resv lock, so it cannot
   happen during the fence signalling critical path.
2. obtain() requires that the BO is not in the extobj list, so obtain()
   must be called before adding the BO to the extobj list.

Thus, drm_gpuvm_bo_obtain_prealloc() cannot be called during the fence
signalling critical path. (For extobjs at least.)

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/drm/gpuvm/mod.rs   |  32 +++++-
 rust/kernel/drm/gpuvm/vm_bo.rs | 219 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 248 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs
index 81b5e767885d8258c44086444b153c91961ffabc..cb576a7ffa07bc108704e008b7f87de52a837930 100644
--- a/rust/kernel/drm/gpuvm/mod.rs
+++ b/rust/kernel/drm/gpuvm/mod.rs
@@ -25,13 +25,20 @@
 
 use core::{
     cell::UnsafeCell,
+    mem::ManuallyDrop,
     ops::{
         Deref,
         Range, //
     },
-    ptr::NonNull, //
+    ptr::{
+        self,
+        NonNull, //
+    }, //
 };
 
+mod vm_bo;
+pub use self::vm_bo::*;
+
 /// A DRM GPU VA manager.
 ///
 /// This object is refcounted, but the "core" is only accessible using a special unique handle. The
@@ -68,8 +75,8 @@ const fn vtable() -> &'static bindings::drm_gpuvm_ops {
             vm_free: Some(Self::vm_free),
             op_alloc: None,
             op_free: None,
-            vm_bo_alloc: None,
-            vm_bo_free: None,
+            vm_bo_alloc: GpuVmBo::<T>::ALLOC_FN,
+            vm_bo_free: GpuVmBo::<T>::FREE_FN,
             vm_bo_validate: None,
             sm_step_map: None,
             sm_step_unmap: None,
@@ -166,6 +173,16 @@ pub fn va_range(&self) -> Range<u64> {
         Range { start, end }
     }
 
+    /// Get or create the [`GpuVmBo`] for this gem object.
+    #[inline]
+    pub fn obtain(
+        &self,
+        obj: &T::Object,
+        data: impl PinInit<T::VmBoData>,
+    ) -> Result<GpuVmBoResident<T>, AllocError> {
+        Ok(GpuVmBoAlloc::new(self, obj, data)?.obtain())
+    }
+
     /// Clean up buffer objects that are no longer used.
     #[inline]
     pub fn deferred_cleanup(&self) {
@@ -191,6 +208,12 @@ pub fn is_extobj(&self, obj: &T::Object) -> bool {
         // SAFETY: By type invariants we can free it when refcount hits zero.
         drop(unsafe { KBox::from_raw(me) })
     }
+
+    #[inline]
+    fn raw_resv_lock(&self) -> *mut bindings::dma_resv {
+        // SAFETY: `r_obj` is immutable and valid for duration of GPUVM.
+        unsafe { (*(*self.as_raw()).r_obj).resv }
+    }
 }
 
 /// The manager for a GPUVM.
@@ -200,6 +223,9 @@ pub trait DriverGpuVm: Sized {
 
     /// The kind of GEM object stored in this GPUVM.
     type Object: IntoGEMObject;
+
+    /// Data stored with each `struct drm_gpuvm_bo`.
+    type VmBoData;
 }
 
 /// The core of the DRM GPU VA manager.
diff --git a/rust/kernel/drm/gpuvm/vm_bo.rs b/rust/kernel/drm/gpuvm/vm_bo.rs
new file mode 100644
index 0000000000000000000000000000000000000000..310fa569b5bd43f0f872ff859b3624377b93d651
--- /dev/null
+++ b/rust/kernel/drm/gpuvm/vm_bo.rs
@@ -0,0 +1,219 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+
+use super::*;
+
+/// Represents that a given GEM object has at least one mapping on this [`GpuVm`] instance.
+///
+/// Does not assume that GEM lock is held.
+#[repr(C)]
+#[pin_data]
+pub struct GpuVmBo<T: DriverGpuVm> {
+    #[pin]
+    inner: Opaque<bindings::drm_gpuvm_bo>,
+    #[pin]
+    data: T::VmBoData,
+}
+
+impl<T: DriverGpuVm> GpuVmBo<T> {
+    pub(super) const ALLOC_FN: Option<unsafe extern "C" fn() -> *mut bindings::drm_gpuvm_bo> = {
+        use core::alloc::Layout;
+        let base = Layout::new::<bindings::drm_gpuvm_bo>();
+        let rust = Layout::new::<Self>();
+        assert!(base.size() <= rust.size());
+        if base.size() != rust.size() || base.align() != rust.align() {
+            Some(Self::vm_bo_alloc)
+        } else {
+            // This causes GPUVM to allocate a `GpuVmBo<T>` with `kzalloc(sizeof(drm_gpuvm_bo))`.
+            None
+        }
+    };
+
+    pub(super) const FREE_FN: Option<unsafe extern "C" fn(*mut bindings::drm_gpuvm_bo)> = {
+        if core::mem::needs_drop::<Self>() {
+            Some(Self::vm_bo_free)
+        } else {
+            // This causes GPUVM to free a `GpuVmBo<T>` with `kfree`.
+            None
+        }
+    };
+
+    /// Custom function for allocating a `drm_gpuvm_bo`.
+    ///
+    /// # Safety
+    ///
+    /// Always safe to call.
+    // Unsafe to match function pointer type in C struct.
+    unsafe extern "C" fn vm_bo_alloc() -> *mut bindings::drm_gpuvm_bo {
+        KBox::<Self>::new_uninit(GFP_KERNEL | __GFP_ZERO)
+            .map(KBox::into_raw)
+            .unwrap_or(ptr::null_mut())
+            .cast()
+    }
+
+    /// Custom function for freeing a `drm_gpuvm_bo`.
+    ///
+    /// # Safety
+    ///
+    /// The pointer must have been allocated with [`GpuVmBo::ALLOC_FN`], and must not be used after
+    /// this call.
+    unsafe extern "C" fn vm_bo_free(ptr: *mut bindings::drm_gpuvm_bo) {
+        // SAFETY:
+        // * The ptr was allocated from kmalloc with the layout of `GpuVmBo<T>`.
+        // * `ptr->inner` has no destructor.
+        // * `ptr->data` contains a valid `T::VmBoData` that we can drop.
+        drop(unsafe { KBox::<Self>::from_raw(ptr.cast()) });
+    }
+
+    /// Access this [`GpuVmBo`] from a raw pointer.
+    ///
+    /// # Safety
+    ///
+    /// For the duration of `'a`, the pointer must reference a valid `drm_gpuvm_bo` associated with
+    /// a [`GpuVm<T>`].
+    #[inline]
+    pub unsafe fn from_raw<'a>(ptr: *mut bindings::drm_gpuvm_bo) -> &'a Self {
+        // SAFETY: `drm_gpuvm_bo` is first field and `repr(C)`.
+        unsafe { &*ptr.cast() }
+    }
+
+    /// Returns a raw pointer to underlying C value.
+    #[inline]
+    pub fn as_raw(&self) -> *mut bindings::drm_gpuvm_bo {
+        self.inner.get()
+    }
+
+    /// The [`GpuVm`] that this GEM object is mapped in.
+    #[inline]
+    pub fn gpuvm(&self) -> &GpuVm<T> {
+        // SAFETY: The `obj` pointer is guaranteed to be valid.
+        unsafe { GpuVm::<T>::from_raw((*self.inner.get()).vm) }
+    }
+
+    /// The [`drm_gem_object`](crate::gem::Object) for these mappings.
+    #[inline]
+    pub fn obj(&self) -> &T::Object {
+        // SAFETY: The `obj` pointer is guaranteed to be valid.
+        unsafe { <T::Object as IntoGEMObject>::from_raw((*self.inner.get()).obj) }
+    }
+
+    /// The driver data with this buffer object.
+    #[inline]
+    pub fn data(&self) -> &T::VmBoData {
+        &self.data
+    }
+}
+
+/// A pre-allocated [`GpuVmBo`] object.
+///
+/// # Invariants
+///
+/// Points at a `drm_gpuvm_bo` that contains a valid `T::VmBoData`, has a refcount of one, and is
+/// absent from any gem, extobj, or evict lists.
+pub(super) struct GpuVmBoAlloc<T: DriverGpuVm>(NonNull<GpuVmBo<T>>);
+
+impl<T: DriverGpuVm> GpuVmBoAlloc<T> {
+    /// Create a new pre-allocated [`GpuVmBo`].
+    ///
+    /// It's intentional that the initializer is infallible because `drm_gpuvm_bo_put` will call
+    /// drop on the data, so we don't have a way to free it when the data is missing.
+    #[inline]
+    pub(super) fn new(
+        gpuvm: &GpuVm<T>,
+        gem: &T::Object,
+        value: impl PinInit<T::VmBoData>,
+    ) -> Result<GpuVmBoAlloc<T>, AllocError> {
+        // CAST: `GpuVmBoAlloc::vm_bo_alloc` ensures that this memory was allocated with the layout
+        // of `GpuVmBo<T>`. The type is repr(C), so `container_of` is not required.
+        // SAFETY: The provided gpuvm and gem ptrs are valid for the duration of this call.
+        let raw_ptr = unsafe {
+            bindings::drm_gpuvm_bo_create(gpuvm.as_raw(), gem.as_raw()).cast::<GpuVmBo<T>>()
+        };
+        let ptr = NonNull::new(raw_ptr).ok_or(AllocError)?;
+        // SAFETY: `ptr->data` is a valid pinned location.
+        let Ok(()) = unsafe { value.__pinned_init(&raw mut (*raw_ptr).data) };
+        // INVARIANTS: We just created the vm_bo so it's absent from lists, and the data is valid
+        // as we just initialized it.
+        Ok(GpuVmBoAlloc(ptr))
+    }
+
+    /// Returns a raw pointer to underlying C value.
+    #[inline]
+    pub(super) fn as_raw(&self) -> *mut bindings::drm_gpuvm_bo {
+        // SAFETY: The pointer references a valid `drm_gpuvm_bo`.
+        unsafe { (*self.0.as_ptr()).inner.get() }
+    }
+
+    /// Look up whether there is an existing [`GpuVmBo`] for this gem object.
+    #[inline]
+    pub(super) fn obtain(self) -> GpuVmBoResident<T> {
+        let me = ManuallyDrop::new(self);
+        // SAFETY: Valid `drm_gpuvm_bo` not already in the lists.
+        let ptr = unsafe { bindings::drm_gpuvm_bo_obtain_prealloc(me.as_raw()) };
+
+        // If the vm_bo does not already exist, ensure that it's in the extobj list.
+        if ptr::eq(ptr, me.as_raw()) && me.gpuvm().is_extobj(me.obj()) {
+            let resv_lock = me.gpuvm().raw_resv_lock();
+            // SAFETY: The GPUVM is still alive, so its resv lock is too.
+            unsafe { bindings::dma_resv_lock(resv_lock, ptr::null_mut()) };
+            // SAFETY: We hold the GPUVMs resv lock.
+            unsafe { bindings::drm_gpuvm_bo_extobj_add(ptr) };
+            // SAFETY: We took the lock, so we can unlock it.
+            unsafe { bindings::dma_resv_unlock(resv_lock) };
+        }
+
+        // INVARIANTS: Valid `drm_gpuvm_bo` in the GEM list.
+        // SAFETY: `drm_gpuvm_bo_obtain_prealloc` always returns a non-null ptr
+        GpuVmBoResident(unsafe { NonNull::new_unchecked(ptr.cast()) })
+    }
+}
+
+impl<T: DriverGpuVm> Deref for GpuVmBoAlloc<T> {
+    type Target = GpuVmBo<T>;
+    #[inline]
+    fn deref(&self) -> &GpuVmBo<T> {
+        // SAFETY: By the type invariants we may deref while `Self` exists.
+        unsafe { self.0.as_ref() }
+    }
+}
+
+impl<T: DriverGpuVm> Drop for GpuVmBoAlloc<T> {
+    #[inline]
+    fn drop(&mut self) {
+        // TODO: Call drm_gpuvm_bo_destroy_not_in_lists() directly.
+        // SAFETY: It's safe to perform a deferred put in any context.
+        unsafe { bindings::drm_gpuvm_bo_put_deferred(self.as_raw()) };
+    }
+}
+
+/// A [`GpuVmBo`] object in the GEM list.
+///
+/// # Invariants
+///
+/// Points at a `drm_gpuvm_bo` that contains a valid `T::VmBoData` and is present in the gem list.
+pub struct GpuVmBoResident<T: DriverGpuVm>(NonNull<GpuVmBo<T>>);
+
+impl<T: DriverGpuVm> GpuVmBoResident<T> {
+    /// Returns a raw pointer to underlying C value.
+    #[inline]
+    pub fn as_raw(&self) -> *mut bindings::drm_gpuvm_bo {
+        // SAFETY: The pointer references a valid `drm_gpuvm_bo`.
+        unsafe { (*self.0.as_ptr()).inner.get() }
+    }
+}
+
+impl<T: DriverGpuVm> Deref for GpuVmBoResident<T> {
+    type Target = GpuVmBo<T>;
+    #[inline]
+    fn deref(&self) -> &GpuVmBo<T> {
+        // SAFETY: By the type invariants we may deref while `Self` exists.
+        unsafe { self.0.as_ref() }
+    }
+}
+
+impl<T: DriverGpuVm> Drop for GpuVmBoResident<T> {
+    #[inline]
+    fn drop(&mut self) {
+        // SAFETY: It's safe to perform a deferred put in any context.
+        unsafe { bindings::drm_gpuvm_bo_put_deferred(self.as_raw()) };
+    }
+}

-- 
2.52.0.457.g6b5491de43-goog


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 4/6] rust: gpuvm: add GpuVa struct
  2026-01-21 11:31 [PATCH v3 0/6] Rust GPUVM immediate mode Alice Ryhl
                   ` (2 preceding siblings ...)
  2026-01-21 11:31 ` [PATCH v3 3/6] rust: gpuvm: add GpuVm::obtain() Alice Ryhl
@ 2026-01-21 11:31 ` Alice Ryhl
  2026-01-22 22:09   ` Daniel Almeida
  2026-01-21 11:31 ` [PATCH v3 5/6] rust: gpuvm: add GpuVmCore::sm_unmap() Alice Ryhl
  2026-01-21 11:31 ` [PATCH v3 6/6] rust: gpuvm: add GpuVmCore::sm_map() Alice Ryhl
  5 siblings, 1 reply; 22+ messages in thread
From: Alice Ryhl @ 2026-01-21 11:31 UTC (permalink / raw)
  To: Danilo Krummrich, Daniel Almeida
  Cc: Boris Brezillon, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux, Alice Ryhl

This struct will be used to keep track of individual mapped ranges in
the GPU's virtual memory.

Co-developed-by: Asahi Lina <lina+kernel@asahilina.net>
Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
Co-developed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/drm/gpuvm/mod.rs |  17 ++++-
 rust/kernel/drm/gpuvm/va.rs  | 149 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 164 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs
index cb576a7ffa07bc108704e008b7f87de52a837930..2179ddd717d8728bbe231bd94ea7b5d1e2652543 100644
--- a/rust/kernel/drm/gpuvm/mod.rs
+++ b/rust/kernel/drm/gpuvm/mod.rs
@@ -11,7 +11,10 @@
 //! C header: [`include/drm/drm_gpuvm.h`](srctree/include/drm/drm_gpuvm.h)
 
 use kernel::{
-    alloc::AllocError,
+    alloc::{
+        AllocError,
+        Flags as AllocFlags, //
+    },
     bindings,
     drm,
     drm::gem::IntoGEMObject,
@@ -25,9 +28,13 @@
 
 use core::{
     cell::UnsafeCell,
-    mem::ManuallyDrop,
+    mem::{
+        ManuallyDrop,
+        MaybeUninit, //
+    },
     ops::{
         Deref,
+        DerefMut,
         Range, //
     },
     ptr::{
@@ -36,6 +43,9 @@
     }, //
 };
 
+mod va;
+pub use self::va::*;
+
 mod vm_bo;
 pub use self::vm_bo::*;
 
@@ -224,6 +234,9 @@ pub trait DriverGpuVm: Sized {
     /// The kind of GEM object stored in this GPUVM.
     type Object: IntoGEMObject;
 
+    /// Data stored with each `struct drm_gpuva`.
+    type VaData;
+
     /// Data stored with each `struct drm_gpuvm_bo`.
     type VmBoData;
 }
diff --git a/rust/kernel/drm/gpuvm/va.rs b/rust/kernel/drm/gpuvm/va.rs
new file mode 100644
index 0000000000000000000000000000000000000000..c96796a6b2c8c7c4b5475324562968ca0f07fd09
--- /dev/null
+++ b/rust/kernel/drm/gpuvm/va.rs
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+
+#![expect(dead_code)]
+use super::*;
+
+/// Represents that a range of a GEM object is mapped in this [`GpuVm`] instance.
+///
+/// Does not assume that GEM lock is held.
+///
+/// # Invariants
+///
+/// This is a valid `drm_gpuva` that is resident in the [`GpuVm`] instance.
+#[repr(C)]
+#[pin_data]
+pub struct GpuVa<T: DriverGpuVm> {
+    #[pin]
+    inner: Opaque<bindings::drm_gpuva>,
+    #[pin]
+    data: T::VaData,
+}
+
+impl<T: DriverGpuVm> GpuVa<T> {
+    /// Access this [`GpuVa`] from a raw pointer.
+    ///
+    /// # Safety
+    ///
+    /// For the duration of `'a`, the pointer must reference a valid `drm_gpuva` associated with a
+    /// [`GpuVm<T>`].
+    #[inline]
+    pub unsafe fn from_raw<'a>(ptr: *mut bindings::drm_gpuva) -> &'a Self {
+        // SAFETY: `drm_gpuva` is first field and `repr(C)`.
+        unsafe { &*ptr.cast() }
+    }
+
+    /// Returns a raw pointer to underlying C value.
+    #[inline]
+    pub fn as_raw(&self) -> *mut bindings::drm_gpuva {
+        self.inner.get()
+    }
+
+    /// Returns the address of this mapping in the GPU virtual address space.
+    #[inline]
+    pub fn addr(&self) -> u64 {
+        // SAFETY: The `va.addr` field of `drm_gpuva` is immutable.
+        unsafe { (*self.as_raw()).va.addr }
+    }
+
+    /// Returns the length of this mapping.
+    #[inline]
+    pub fn length(&self) -> u64 {
+        // SAFETY: The `va.range` field of `drm_gpuva` is immutable.
+        unsafe { (*self.as_raw()).va.range }
+    }
+
+    /// Returns `addr..addr+length`.
+    #[inline]
+    pub fn range(&self) -> Range<u64> {
+        let addr = self.addr();
+        addr..addr + self.length()
+    }
+
+    /// Returns the offset within the GEM object.
+    #[inline]
+    pub fn gem_offset(&self) -> u64 {
+        // SAFETY: The `gem.offset` field of `drm_gpuva` is immutable.
+        unsafe { (*self.as_raw()).gem.offset }
+    }
+
+    /// Returns the GEM object.
+    #[inline]
+    pub fn obj(&self) -> &T::Object {
+        // SAFETY: The `gem.offset` field of `drm_gpuva` is immutable.
+        unsafe { <T::Object as IntoGEMObject>::from_raw((*self.as_raw()).gem.obj) }
+    }
+
+    /// Returns the underlying [`GpuVmBo`] object that backs this [`GpuVa`].
+    #[inline]
+    pub fn vm_bo(&self) -> &GpuVmBo<T> {
+        // SAFETY: The `vm_bo` field has been set and is immutable for the duration in which this
+        // `drm_gpuva` is resident in the VM.
+        unsafe { GpuVmBo::from_raw((*self.as_raw()).vm_bo) }
+    }
+}
+
+/// A pre-allocated [`GpuVa`] object.
+///
+/// # Invariants
+///
+/// The memory is zeroed.
+pub struct GpuVaAlloc<T: DriverGpuVm>(KBox<MaybeUninit<GpuVa<T>>>);
+
+impl<T: DriverGpuVm> GpuVaAlloc<T> {
+    /// Pre-allocate a [`GpuVa`] object.
+    pub fn new(flags: AllocFlags) -> Result<GpuVaAlloc<T>, AllocError> {
+        // INVARIANTS: Memory allocated with __GFP_ZERO.
+        Ok(GpuVaAlloc(KBox::new_uninit(flags | __GFP_ZERO)?))
+    }
+
+    /// Prepare this `drm_gpuva` for insertion into the GPUVM.
+    pub(super) fn prepare(mut self, va_data: impl PinInit<T::VaData>) -> *mut bindings::drm_gpuva {
+        let va_ptr = MaybeUninit::as_mut_ptr(&mut self.0);
+        // SAFETY: The `data` field is pinned.
+        let Ok(()) = unsafe { va_data.__pinned_init(&raw mut (*va_ptr).data) };
+        KBox::into_raw(self.0).cast()
+    }
+}
+
+/// A [`GpuVa`] object that has been removed.
+///
+/// # Invariants
+///
+/// The `drm_gpuva` is not resident in the [`GpuVm`].
+pub struct GpuVaRemoved<T: DriverGpuVm>(KBox<GpuVa<T>>);
+
+impl<T: DriverGpuVm> GpuVaRemoved<T> {
+    /// Convert a raw pointer into a [`GpuVaRemoved`].
+    ///
+    /// # Safety
+    ///
+    /// Must have been removed from a [`GpuVm<T>`].
+    pub(super) unsafe fn from_raw(ptr: *mut bindings::drm_gpuva) -> Self {
+        // SAFETY: Since it has been removed we can take ownership of allocation.
+        GpuVaRemoved(unsafe { KBox::from_raw(ptr.cast()) })
+    }
+
+    /// Take ownership of the VA data.
+    pub fn into_inner(self) -> T::VaData
+    where
+        T::VaData: Unpin,
+    {
+        KBox::into_inner(self.0).data
+    }
+}
+
+impl<T: DriverGpuVm> Deref for GpuVaRemoved<T> {
+    type Target = T::VaData;
+    fn deref(&self) -> &T::VaData {
+        &self.0.data
+    }
+}
+
+impl<T: DriverGpuVm> DerefMut for GpuVaRemoved<T>
+where
+    T::VaData: Unpin,
+{
+    fn deref_mut(&mut self) -> &mut T::VaData {
+        &mut self.0.data
+    }
+}

-- 
2.52.0.457.g6b5491de43-goog


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 5/6] rust: gpuvm: add GpuVmCore::sm_unmap()
  2026-01-21 11:31 [PATCH v3 0/6] Rust GPUVM immediate mode Alice Ryhl
                   ` (3 preceding siblings ...)
  2026-01-21 11:31 ` [PATCH v3 4/6] rust: gpuvm: add GpuVa struct Alice Ryhl
@ 2026-01-21 11:31 ` Alice Ryhl
  2026-01-22 22:43   ` Daniel Almeida
  2026-01-21 11:31 ` [PATCH v3 6/6] rust: gpuvm: add GpuVmCore::sm_map() Alice Ryhl
  5 siblings, 1 reply; 22+ messages in thread
From: Alice Ryhl @ 2026-01-21 11:31 UTC (permalink / raw)
  To: Danilo Krummrich, Daniel Almeida
  Cc: Boris Brezillon, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux, Alice Ryhl

Add the entrypoint for unmapping ranges in the GPUVM, and provide
callbacks and VA types for the implementation.

Co-developed-by: Asahi Lina <lina+kernel@asahilina.net>
Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/drm/gpuvm/mod.rs    |  30 ++++-
 rust/kernel/drm/gpuvm/sm_ops.rs | 264 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/drm/gpuvm/va.rs     |   1 -
 rust/kernel/drm/gpuvm/vm_bo.rs  |   8 ++
 4 files changed, 298 insertions(+), 5 deletions(-)

diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs
index 2179ddd717d8728bbe231bd94ea7b5d1e2652543..165a25666ccc3d62e59b73483d4eedff044423e9 100644
--- a/rust/kernel/drm/gpuvm/mod.rs
+++ b/rust/kernel/drm/gpuvm/mod.rs
@@ -18,6 +18,7 @@
     bindings,
     drm,
     drm::gem::IntoGEMObject,
+    error::to_result,
     prelude::*,
     sync::aref::{
         ARef,
@@ -28,6 +29,7 @@
 
 use core::{
     cell::UnsafeCell,
+    marker::PhantomData,
     mem::{
         ManuallyDrop,
         MaybeUninit, //
@@ -43,12 +45,15 @@
     }, //
 };
 
-mod va;
-pub use self::va::*;
+mod sm_ops;
+pub use self::sm_ops::*;
 
 mod vm_bo;
 pub use self::vm_bo::*;
 
+mod va;
+pub use self::va::*;
+
 /// A DRM GPU VA manager.
 ///
 /// This object is refcounted, but the "core" is only accessible using a special unique handle. The
@@ -89,8 +94,8 @@ const fn vtable() -> &'static bindings::drm_gpuvm_ops {
             vm_bo_free: GpuVmBo::<T>::FREE_FN,
             vm_bo_validate: None,
             sm_step_map: None,
-            sm_step_unmap: None,
-            sm_step_remap: None,
+            sm_step_unmap: Some(Self::sm_step_unmap),
+            sm_step_remap: Some(Self::sm_step_remap),
         }
     }
 
@@ -239,6 +244,23 @@ pub trait DriverGpuVm: Sized {
 
     /// Data stored with each `struct drm_gpuvm_bo`.
     type VmBoData;
+
+    /// The private data passed to callbacks.
+    type SmContext<'ctx>;
+
+    /// Indicates that an existing mapping should be removed.
+    fn sm_step_unmap<'op, 'ctx>(
+        &mut self,
+        op: OpUnmap<'op, Self>,
+        context: &mut Self::SmContext<'ctx>,
+    ) -> Result<OpUnmapped<'op, Self>, Error>;
+
+    /// Indicates that an existing mapping should be split up.
+    fn sm_step_remap<'op, 'ctx>(
+        &mut self,
+        op: OpRemap<'op, Self>,
+        context: &mut Self::SmContext<'ctx>,
+    ) -> Result<OpRemapped<'op, Self>, Error>;
 }
 
 /// The core of the DRM GPU VA manager.
diff --git a/rust/kernel/drm/gpuvm/sm_ops.rs b/rust/kernel/drm/gpuvm/sm_ops.rs
new file mode 100644
index 0000000000000000000000000000000000000000..3c29d10d63f0b0a1976c714a86d486948ba81a15
--- /dev/null
+++ b/rust/kernel/drm/gpuvm/sm_ops.rs
@@ -0,0 +1,264 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+
+use super::*;
+
+/// The actual data that gets threaded through the callbacks.
+struct SmData<'a, 'ctx, T: DriverGpuVm> {
+    gpuvm: &'a mut GpuVmCore<T>,
+    user_context: &'a mut T::SmContext<'ctx>,
+}
+
+/// Represents an `sm_step_unmap` operation that has not yet been completed.
+pub struct OpUnmap<'op, T: DriverGpuVm> {
+    op: &'op bindings::drm_gpuva_op_unmap,
+    _invariant: PhantomData<*mut &'op mut T>,
+}
+
+impl<'op, T: DriverGpuVm> OpUnmap<'op, T> {
+    /// Indicates whether this `drm_gpuva` is physically contiguous with the
+    /// original mapping request.
+    ///
+    /// Optionally, if `keep` is set, drivers may keep the actual page table
+    /// mappings for this `drm_gpuva`, adding the missing page table entries
+    /// only and update the `drm_gpuvm` accordingly.
+    pub fn keep(&self) -> bool {
+        self.op.keep
+    }
+
+    /// The range being unmapped.
+    pub fn va(&self) -> &GpuVa<T> {
+        // SAFETY: This is a valid va.
+        unsafe { GpuVa::<T>::from_raw(self.op.va) }
+    }
+
+    /// Remove the VA.
+    pub fn remove(self) -> (OpUnmapped<'op, T>, GpuVaRemoved<T>) {
+        // SAFETY: The op references a valid drm_gpuva in the GPUVM.
+        unsafe { bindings::drm_gpuva_unmap(self.op) };
+        // SAFETY: The va is no longer in the interval tree so we may unlink it.
+        unsafe { bindings::drm_gpuva_unlink_defer(self.op.va) };
+
+        // SAFETY: We just removed this va from the `GpuVm<T>`.
+        let va = unsafe { GpuVaRemoved::from_raw(self.op.va) };
+
+        (
+            OpUnmapped {
+                _invariant: self._invariant,
+            },
+            va,
+        )
+    }
+}
+
+/// Represents a completed [`OpUnmap`] operation.
+pub struct OpUnmapped<'op, T> {
+    _invariant: PhantomData<*mut &'op mut T>,
+}
+
+/// Represents an `sm_step_remap` operation that has not yet been completed.
+pub struct OpRemap<'op, T: DriverGpuVm> {
+    op: &'op bindings::drm_gpuva_op_remap,
+    _invariant: PhantomData<*mut &'op mut T>,
+}
+
+impl<'op, T: DriverGpuVm> OpRemap<'op, T> {
+    /// The preceding part of a split mapping.
+    #[inline]
+    pub fn prev(&self) -> Option<&OpRemapMapData> {
+        // SAFETY: We checked for null, so the pointer must be valid.
+        NonNull::new(self.op.prev).map(|ptr| unsafe { OpRemapMapData::from_raw(ptr) })
+    }
+
+    /// The subsequent part of a split mapping.
+    #[inline]
+    pub fn next(&self) -> Option<&OpRemapMapData> {
+        // SAFETY: We checked for null, so the pointer must be valid.
+        NonNull::new(self.op.next).map(|ptr| unsafe { OpRemapMapData::from_raw(ptr) })
+    }
+
+    /// Indicates whether the `drm_gpuva` being removed is physically contiguous with the original
+    /// mapping request.
+    ///
+    /// Optionally, if `keep` is set, drivers may keep the actual page table mappings for this
+    /// `drm_gpuva`, adding the missing page table entries only and update the `drm_gpuvm`
+    /// accordingly.
+    #[inline]
+    pub fn keep(&self) -> bool {
+        // SAFETY: The unmap pointer is always valid.
+        unsafe { (*self.op.unmap).keep }
+    }
+
+    /// The range being unmapped.
+    #[inline]
+    pub fn va_to_unmap(&self) -> &GpuVa<T> {
+        // SAFETY: This is a valid va.
+        unsafe { GpuVa::<T>::from_raw((*self.op.unmap).va) }
+    }
+
+    /// The [`drm_gem_object`](crate::gem::Object) whose VA is being remapped.
+    #[inline]
+    pub fn obj(&self) -> &T::Object {
+        self.va_to_unmap().obj()
+    }
+
+    /// The [`GpuVmBo`] that is being remapped.
+    #[inline]
+    pub fn vm_bo(&self) -> &GpuVmBo<T> {
+        self.va_to_unmap().vm_bo()
+    }
+
+    /// Update the GPUVM to perform the remapping.
+    pub fn remap(
+        self,
+        va_alloc: [GpuVaAlloc<T>; 2],
+        prev_data: impl PinInit<T::VaData>,
+        next_data: impl PinInit<T::VaData>,
+    ) -> (OpRemapped<'op, T>, OpRemapRet<T>) {
+        let [va1, va2] = va_alloc;
+
+        let mut unused_va = None;
+        let mut prev_ptr = ptr::null_mut();
+        let mut next_ptr = ptr::null_mut();
+        if self.prev().is_some() {
+            prev_ptr = va1.prepare(prev_data);
+        } else {
+            unused_va = Some(va1);
+        }
+        if self.next().is_some() {
+            next_ptr = va2.prepare(next_data);
+        } else {
+            unused_va = Some(va2);
+        }
+
+        // SAFETY: the pointers are non-null when required
+        unsafe { bindings::drm_gpuva_remap(prev_ptr, next_ptr, self.op) };
+
+        let gpuva_guard = self.vm_bo().lock_gpuva();
+        if !prev_ptr.is_null() {
+            // SAFETY: The prev_ptr is a valid drm_gpuva prepared for insertion. The vm_bo is still
+            // valid as the not-yet-unlinked gpuva holds a refcount on the vm_bo.
+            unsafe { bindings::drm_gpuva_link(prev_ptr, self.vm_bo().as_raw()) };
+        }
+        if !next_ptr.is_null() {
+            // SAFETY: The next_ptr is a valid drm_gpuva prepared for insertion. The vm_bo is still
+            // valid as the not-yet-unlinked gpuva holds a refcount on the vm_bo.
+            unsafe { bindings::drm_gpuva_link(next_ptr, self.vm_bo().as_raw()) };
+        }
+        drop(gpuva_guard);
+
+        // SAFETY: The va is no longer in the interval tree so we may unlink it.
+        unsafe { bindings::drm_gpuva_unlink_defer((*self.op.unmap).va) };
+
+        (
+            OpRemapped {
+                _invariant: self._invariant,
+            },
+            OpRemapRet {
+                // SAFETY: We just removed this va from the `GpuVm<T>`.
+                unmapped_va: unsafe { GpuVaRemoved::from_raw((*self.op.unmap).va) },
+                unused_va,
+            },
+        )
+    }
+}
+
+/// Part of an [`OpRemap`] that represents a new mapping.
+#[repr(transparent)]
+pub struct OpRemapMapData(bindings::drm_gpuva_op_map);
+
+impl OpRemapMapData {
+    /// # Safety
+    /// Must reference a valid `drm_gpuva_op_map` for duration of `'a`.
+    unsafe fn from_raw<'a>(ptr: NonNull<bindings::drm_gpuva_op_map>) -> &'a Self {
+        // SAFETY: ok per safety requirements
+        unsafe { ptr.cast().as_ref() }
+    }
+
+    /// The base address of the new mapping.
+    pub fn addr(&self) -> u64 {
+        self.0.va.addr
+    }
+
+    /// The length of the new mapping.
+    pub fn length(&self) -> u64 {
+        self.0.va.range
+    }
+
+    /// The offset within the [`drm_gem_object`](crate::gem::Object).
+    pub fn gem_offset(&self) -> u64 {
+        self.0.gem.offset
+    }
+}
+
+/// Struct containing objects removed or not used by [`OpRemap::remap`].
+pub struct OpRemapRet<T: DriverGpuVm> {
+    /// The `drm_gpuva` that was removed.
+    pub unmapped_va: GpuVaRemoved<T>,
+    /// If the remap did not split the region into two pieces, then the unused `drm_gpuva` is
+    /// returned here.
+    pub unused_va: Option<GpuVaAlloc<T>>,
+}
+
+/// Represents a completed [`OpRemap`] operation.
+pub struct OpRemapped<'op, T> {
+    _invariant: PhantomData<*mut &'op mut T>,
+}
+
+impl<T: DriverGpuVm> GpuVmCore<T> {
+    /// Remove any mappings in the given region.
+    ///
+    /// Internally calls [`DriverGpuVm::sm_step_unmap`] for ranges entirely contained within the
+    /// given range, and [`DriverGpuVm::sm_step_remap`] for ranges that overlap with the range.
+    #[inline]
+    pub fn sm_unmap(&mut self, addr: u64, length: u64, context: &mut T::SmContext<'_>) -> Result {
+        let gpuvm = self.as_raw();
+        let mut p = SmData {
+            gpuvm: self,
+            user_context: context,
+        };
+        // SAFETY:
+        // * raw_request() creates a valid request.
+        // * The private data is valid to be interpreted as SmData.
+        to_result(unsafe { bindings::drm_gpuvm_sm_unmap(gpuvm, (&raw mut p).cast(), addr, length) })
+    }
+}
+
+impl<T: DriverGpuVm> GpuVm<T> {
+    /// # Safety
+    /// Must be called from `sm_unmap` with a pointer to `SmData`.
+    pub(super) unsafe extern "C" fn sm_step_unmap(
+        op: *mut bindings::drm_gpuva_op,
+        p: *mut c_void,
+    ) -> c_int {
+        // SAFETY: The caller provides a pointer to `SmData`.
+        let p = unsafe { &mut *p.cast::<SmData<'_, '_, T>>() };
+        let op = OpUnmap {
+            // SAFETY: sm_step_unmap is called with an unmap operation.
+            op: unsafe { &(*op).__bindgen_anon_1.unmap },
+            _invariant: PhantomData,
+        };
+        match p.gpuvm.data().sm_step_unmap(op, p.user_context) {
+            Ok(OpUnmapped { .. }) => 0,
+            Err(err) => err.to_errno(),
+        }
+    }
+
+    /// # Safety
+    /// Must be called from `sm_unmap` with a pointer to `SmData`.
+    pub(super) unsafe extern "C" fn sm_step_remap(
+        op: *mut bindings::drm_gpuva_op,
+        p: *mut c_void,
+    ) -> c_int {
+        // SAFETY: The caller provides a pointer to `SmData`.
+        let p = unsafe { &mut *p.cast::<SmData<'_, '_, T>>() };
+        let op = OpRemap {
+            // SAFETY: sm_step_remap is called with a remap operation.
+            op: unsafe { &(*op).__bindgen_anon_1.remap },
+            _invariant: PhantomData,
+        };
+        match p.gpuvm.data().sm_step_remap(op, p.user_context) {
+            Ok(OpRemapped { .. }) => 0,
+            Err(err) => err.to_errno(),
+        }
+    }
+}
diff --git a/rust/kernel/drm/gpuvm/va.rs b/rust/kernel/drm/gpuvm/va.rs
index c96796a6b2c8c7c4b5475324562968ca0f07fd09..a31122ff22282186a1d76d4bb085714f6465722b 100644
--- a/rust/kernel/drm/gpuvm/va.rs
+++ b/rust/kernel/drm/gpuvm/va.rs
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0 OR MIT
 
-#![expect(dead_code)]
 use super::*;
 
 /// Represents that a range of a GEM object is mapped in this [`GpuVm`] instance.
diff --git a/rust/kernel/drm/gpuvm/vm_bo.rs b/rust/kernel/drm/gpuvm/vm_bo.rs
index 310fa569b5bd43f0f872ff859b3624377b93d651..f600dfb15379233111b5893f6aa85a12e6c9e131 100644
--- a/rust/kernel/drm/gpuvm/vm_bo.rs
+++ b/rust/kernel/drm/gpuvm/vm_bo.rs
@@ -101,6 +101,14 @@ pub fn obj(&self) -> &T::Object {
     pub fn data(&self) -> &T::VmBoData {
         &self.data
     }
+
+    pub(super) fn lock_gpuva(&self) -> crate::sync::MutexGuard<'_, ()> {
+        // SAFETY: The GEM object is valid.
+        let ptr = unsafe { &raw mut (*self.obj().as_raw()).gpuva.lock };
+        // SAFETY: The GEM object is valid, so the mutex is properly initialized.
+        let mutex = unsafe { crate::sync::Mutex::from_raw(ptr) };
+        mutex.lock()
+    }
 }
 
 /// A pre-allocated [`GpuVmBo`] object.

-- 
2.52.0.457.g6b5491de43-goog


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 6/6] rust: gpuvm: add GpuVmCore::sm_map()
  2026-01-21 11:31 [PATCH v3 0/6] Rust GPUVM immediate mode Alice Ryhl
                   ` (4 preceding siblings ...)
  2026-01-21 11:31 ` [PATCH v3 5/6] rust: gpuvm: add GpuVmCore::sm_unmap() Alice Ryhl
@ 2026-01-21 11:31 ` Alice Ryhl
  2026-01-22 22:58   ` Daniel Almeida
  5 siblings, 1 reply; 22+ messages in thread
From: Alice Ryhl @ 2026-01-21 11:31 UTC (permalink / raw)
  To: Danilo Krummrich, Daniel Almeida
  Cc: Boris Brezillon, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux, Alice Ryhl

Finally also add the operation for creating new mappings. Mapping
operations need extra data in the context since they involve a vm_bo
coming from the outside.

Co-developed-by: Asahi Lina <lina+kernel@asahilina.net>
Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/drm/gpuvm/mod.rs    |   9 ++-
 rust/kernel/drm/gpuvm/sm_ops.rs | 154 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 157 insertions(+), 6 deletions(-)

diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs
index 165a25666ccc3d62e59b73483d4eedff044423e9..557c0d629eec912a97fc4ef18495d5bf0807db0a 100644
--- a/rust/kernel/drm/gpuvm/mod.rs
+++ b/rust/kernel/drm/gpuvm/mod.rs
@@ -93,7 +93,7 @@ const fn vtable() -> &'static bindings::drm_gpuvm_ops {
             vm_bo_alloc: GpuVmBo::<T>::ALLOC_FN,
             vm_bo_free: GpuVmBo::<T>::FREE_FN,
             vm_bo_validate: None,
-            sm_step_map: None,
+            sm_step_map: Some(Self::sm_step_map),
             sm_step_unmap: Some(Self::sm_step_unmap),
             sm_step_remap: Some(Self::sm_step_remap),
         }
@@ -248,6 +248,13 @@ pub trait DriverGpuVm: Sized {
     /// The private data passed to callbacks.
     type SmContext<'ctx>;
 
+    /// Indicates that a new mapping should be created.
+    fn sm_step_map<'op, 'ctx>(
+        &mut self,
+        op: OpMap<'op, Self>,
+        context: &mut Self::SmContext<'ctx>,
+    ) -> Result<OpMapped<'op, Self>, Error>;
+
     /// Indicates that an existing mapping should be removed.
     fn sm_step_unmap<'op, 'ctx>(
         &mut self,
diff --git a/rust/kernel/drm/gpuvm/sm_ops.rs b/rust/kernel/drm/gpuvm/sm_ops.rs
index 3c29d10d63f0b0a1976c714a86d486948ba81a15..5f3c5d3918147a6962e5658443c343835baa10b8 100644
--- a/rust/kernel/drm/gpuvm/sm_ops.rs
+++ b/rust/kernel/drm/gpuvm/sm_ops.rs
@@ -8,6 +8,100 @@ struct SmData<'a, 'ctx, T: DriverGpuVm> {
     user_context: &'a mut T::SmContext<'ctx>,
 }
 
+#[repr(C)]
+struct SmMapData<'a, 'ctx, T: DriverGpuVm> {
+    sm_data: SmData<'a, 'ctx, T>,
+    vm_bo: GpuVmBoResident<T>,
+}
+
+/// The argument for [`GpuVmCore::sm_map`].
+pub struct OpMapRequest<'a, 'ctx, T: DriverGpuVm> {
+    /// Address in GPU virtual address space.
+    pub addr: u64,
+    /// Length of mapping to create.
+    pub range: u64,
+    /// Offset in GEM object.
+    pub offset: u64,
+    /// The GEM object to map.
+    pub vm_bo: GpuVmBoResident<T>,
+    /// The user-provided context type.
+    pub context: &'a mut T::SmContext<'ctx>,
+}
+
+impl<'a, 'ctx, T: DriverGpuVm> OpMapRequest<'a, 'ctx, T> {
+    fn raw_request(&self) -> bindings::drm_gpuvm_map_req {
+        bindings::drm_gpuvm_map_req {
+            map: bindings::drm_gpuva_op_map {
+                va: bindings::drm_gpuva_op_map__bindgen_ty_1 {
+                    addr: self.addr,
+                    range: self.range,
+                },
+                gem: bindings::drm_gpuva_op_map__bindgen_ty_2 {
+                    offset: self.offset,
+                    obj: self.vm_bo.obj().as_raw(),
+                },
+            },
+        }
+    }
+}
+
+/// Represents an `sm_step_map` operation that has not yet been completed.
+pub struct OpMap<'op, T: DriverGpuVm> {
+    op: &'op bindings::drm_gpuva_op_map,
+    // Since these abstractions are designed for immediate mode, the VM BO needs to be
+    // pre-allocated, so we always have it available when we reach this point.
+    vm_bo: &'op GpuVmBo<T>,
+    _invariant: PhantomData<*mut &'op mut T>,
+}
+
+impl<'op, T: DriverGpuVm> OpMap<'op, T> {
+    /// The base address of the new mapping.
+    pub fn addr(&self) -> u64 {
+        self.op.va.addr
+    }
+
+    /// The length of the new mapping.
+    pub fn length(&self) -> u64 {
+        self.op.va.range
+    }
+
+    /// The offset within the [`drm_gem_object`](crate::gem::Object).
+    pub fn gem_offset(&self) -> u64 {
+        self.op.gem.offset
+    }
+
+    /// The [`drm_gem_object`](crate::gem::Object) to map.
+    pub fn obj(&self) -> &T::Object {
+        // SAFETY: The `obj` pointer is guaranteed to be valid.
+        unsafe { <T::Object as IntoGEMObject>::from_raw(self.op.gem.obj) }
+    }
+
+    /// The [`GpuVmBo`] that the new VA will be associated with.
+    pub fn vm_bo(&self) -> &GpuVmBo<T> {
+        self.vm_bo
+    }
+
+    /// Use the pre-allocated VA to carry out this map operation.
+    pub fn insert(self, va: GpuVaAlloc<T>, va_data: impl PinInit<T::VaData>) -> OpMapped<'op, T> {
+        let va = va.prepare(va_data);
+        // SAFETY: By the type invariants we may access the interval tree.
+        unsafe { bindings::drm_gpuva_map(self.vm_bo.gpuvm().as_raw(), va, self.op) };
+
+        let _gpuva_guard = self.vm_bo().lock_gpuva();
+        // SAFETY: The va is prepared for insertion, and we hold the GEM lock.
+        unsafe { bindings::drm_gpuva_link(va, self.vm_bo.as_raw()) };
+
+        OpMapped {
+            _invariant: self._invariant,
+        }
+    }
+}
+
+/// Represents a completed [`OpMap`] operation.
+pub struct OpMapped<'op, T> {
+    _invariant: PhantomData<*mut &'op mut T>,
+}
+
 /// Represents an `sm_step_unmap` operation that has not yet been completed.
 pub struct OpUnmap<'op, T: DriverGpuVm> {
     op: &'op bindings::drm_gpuva_op_unmap,
@@ -205,6 +299,30 @@ pub struct OpRemapped<'op, T> {
 }
 
 impl<T: DriverGpuVm> GpuVmCore<T> {
+    /// Create a mapping, removing or remapping anything that overlaps.
+    ///
+    /// Internally calls the [`DriverGpuVm`] callbacks similar to [`Self::sm_unmap`], except that
+    /// the [`DriverGpuVm::sm_step_map`] is called once to create the requested mapping.
+    #[inline]
+    pub fn sm_map(&mut self, req: OpMapRequest<'_, '_, T>) -> Result {
+        let gpuvm = self.as_raw();
+        let raw_req = req.raw_request();
+        let mut p = SmMapData {
+            sm_data: SmData {
+                gpuvm: self,
+                user_context: req.context,
+            },
+            vm_bo: req.vm_bo,
+        };
+        // SAFETY:
+        // * raw_request() creates a valid request.
+        // * The private data is valid to be interpreted as both SmData and SmMapData since the
+        //   first field of SmMapData is SmData.
+        to_result(unsafe {
+            bindings::drm_gpuvm_sm_map(gpuvm, (&raw mut p).cast(), &raw const raw_req)
+        })
+    }
+
     /// Remove any mappings in the given region.
     ///
     /// Internally calls [`DriverGpuVm::sm_step_unmap`] for ranges entirely contained within the
@@ -218,19 +336,45 @@ pub fn sm_unmap(&mut self, addr: u64, length: u64, context: &mut T::SmContext<'_
         };
         // SAFETY:
         // * raw_request() creates a valid request.
-        // * The private data is valid to be interpreted as SmData.
+        // * The private data is a valid SmData.
         to_result(unsafe { bindings::drm_gpuvm_sm_unmap(gpuvm, (&raw mut p).cast(), addr, length) })
     }
 }
 
 impl<T: DriverGpuVm> GpuVm<T> {
     /// # Safety
-    /// Must be called from `sm_unmap` with a pointer to `SmData`.
+    /// Must be called from `sm_map` with a pointer to `SmMapData`.
+    pub(super) unsafe extern "C" fn sm_step_map(
+        op: *mut bindings::drm_gpuva_op,
+        p: *mut c_void,
+    ) -> c_int {
+        // SAFETY: If we reach `sm_step_map` then we were called from `sm_map` which always passes
+        // an `SmMapData` as private data.
+        let p = unsafe { &mut *p.cast::<SmMapData<'_, '_, T>>() };
+        let op = OpMap {
+            // SAFETY: sm_step_map is called with a map operation.
+            op: unsafe { &(*op).__bindgen_anon_1.map },
+            vm_bo: &p.vm_bo,
+            _invariant: PhantomData,
+        };
+        match p
+            .sm_data
+            .gpuvm
+            .data()
+            .sm_step_map(op, p.sm_data.user_context)
+        {
+            Ok(OpMapped { .. }) => 0,
+            Err(err) => err.to_errno(),
+        }
+    }
+
+    /// # Safety
+    /// Must be called from `sm_map` or `sm_unmap` with a pointer to `SmMapData` or `SmData`.
     pub(super) unsafe extern "C" fn sm_step_unmap(
         op: *mut bindings::drm_gpuva_op,
         p: *mut c_void,
     ) -> c_int {
-        // SAFETY: The caller provides a pointer to `SmData`.
+        // SAFETY: The caller provides a pointer that can be treated as `SmData`.
         let p = unsafe { &mut *p.cast::<SmData<'_, '_, T>>() };
         let op = OpUnmap {
             // SAFETY: sm_step_unmap is called with an unmap operation.
@@ -244,12 +388,12 @@ impl<T: DriverGpuVm> GpuVm<T> {
     }
 
     /// # Safety
-    /// Must be called from `sm_unmap` with a pointer to `SmData`.
+    /// Must be called from `sm_map` or `sm_unmap` with a pointer to `SmMapData` or `SmData`.
     pub(super) unsafe extern "C" fn sm_step_remap(
         op: *mut bindings::drm_gpuva_op,
         p: *mut c_void,
     ) -> c_int {
-        // SAFETY: The caller provides a pointer to `SmData`.
+        // SAFETY: The caller provides a pointer that can be treated as `SmData`.
         let p = unsafe { &mut *p.cast::<SmData<'_, '_, T>>() };
         let op = OpRemap {
             // SAFETY: sm_step_remap is called with a remap operation.

-- 
2.52.0.457.g6b5491de43-goog


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 2/6] rust: helpers: Add bindings/wrappers for dma_resv_lock
  2026-01-21 11:31 ` [PATCH v3 2/6] rust: helpers: Add bindings/wrappers for dma_resv_lock Alice Ryhl
@ 2026-01-21 12:39   ` Daniel Almeida
  2026-01-21 15:44   ` Gary Guo
  1 sibling, 0 replies; 22+ messages in thread
From: Daniel Almeida @ 2026-01-21 12:39 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Danilo Krummrich, Boris Brezillon, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux



> On 21 Jan 2026, at 08:31, Alice Ryhl <aliceryhl@google.com> wrote:
> 
> From: Asahi Lina <lina+kernel@asahilina.net>
> 
> This is just for basic usage in the DRM shmem abstractions for implied
> locking, not intended as a full DMA Reservation abstraction yet.
> 
> Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Reviewed-by: Janne Grunau <j@jannau.net>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> Taken from:
> https://lore.kernel.org/all/20251202220924.520644-3-lyude@redhat.com/
> ---
> rust/bindings/bindings_helper.h |  1 +
> rust/helpers/dma-resv.c         | 13 +++++++++++++
> rust/helpers/helpers.c          |  1 +
> 3 files changed, 15 insertions(+)
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index dd60a5c6b142ec2c5fd6df80279ab6813163791c..ed80dd8088bc60c67b02f7666e602e33158bb962 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -52,6 +52,7 @@
> #include <linux/device/faux.h>
> #include <linux/dma-direction.h>
> #include <linux/dma-mapping.h>
> +#include <linux/dma-resv.h>
> #include <linux/errname.h>
> #include <linux/ethtool.h>
> #include <linux/fdtable.h>
> diff --git a/rust/helpers/dma-resv.c b/rust/helpers/dma-resv.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..05501cb814513b483afd0b7f220230d867863c2f
> --- /dev/null
> +++ b/rust/helpers/dma-resv.c
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/dma-resv.h>
> +
> +int rust_helper_dma_resv_lock(struct dma_resv *obj, struct ww_acquire_ctx *ctx)
> +{
> + return dma_resv_lock(obj, ctx);
> +}
> +
> +void rust_helper_dma_resv_unlock(struct dma_resv *obj)
> +{
> + dma_resv_unlock(obj);
> +}
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 0943d589b7578d3c0e207937f63a5e02719c6146..aae78c938b0b5848b1740fb3f2dc4b7f93b1a760 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -25,6 +25,7 @@
> #include "cred.c"
> #include "device.c"
> #include "dma.c"
> +#include "dma-resv.c"
> #include "drm.c"
> #include "drm_gpuvm.c"
> #include "err.c"
> 
> -- 
> 2.52.0.457.g6b5491de43-goog
> 
> 

Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 2/6] rust: helpers: Add bindings/wrappers for dma_resv_lock
  2026-01-21 11:31 ` [PATCH v3 2/6] rust: helpers: Add bindings/wrappers for dma_resv_lock Alice Ryhl
  2026-01-21 12:39   ` Daniel Almeida
@ 2026-01-21 15:44   ` Gary Guo
  1 sibling, 0 replies; 22+ messages in thread
From: Gary Guo @ 2026-01-21 15:44 UTC (permalink / raw)
  To: Alice Ryhl, Danilo Krummrich, Daniel Almeida
  Cc: Boris Brezillon, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux

On Wed Jan 21, 2026 at 11:31 AM GMT, Alice Ryhl wrote:
> From: Asahi Lina <lina+kernel@asahilina.net>
>
> This is just for basic usage in the DRM shmem abstractions for implied
> locking, not intended as a full DMA Reservation abstraction yet.
>
> Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Reviewed-by: Janne Grunau <j@jannau.net>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> Taken from:
> https://lore.kernel.org/all/20251202220924.520644-3-lyude@redhat.com/
> ---
>  rust/bindings/bindings_helper.h |  1 +
>  rust/helpers/dma-resv.c         | 13 +++++++++++++
>  rust/helpers/helpers.c          |  1 +
>  3 files changed, 15 insertions(+)
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index dd60a5c6b142ec2c5fd6df80279ab6813163791c..ed80dd8088bc60c67b02f7666e602e33158bb962 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -52,6 +52,7 @@
>  #include <linux/device/faux.h>
>  #include <linux/dma-direction.h>
>  #include <linux/dma-mapping.h>
> +#include <linux/dma-resv.h>
>  #include <linux/errname.h>
>  #include <linux/ethtool.h>
>  #include <linux/fdtable.h>
> diff --git a/rust/helpers/dma-resv.c b/rust/helpers/dma-resv.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..05501cb814513b483afd0b7f220230d867863c2f
> --- /dev/null
> +++ b/rust/helpers/dma-resv.c
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/dma-resv.h>
> +
> +int rust_helper_dma_resv_lock(struct dma_resv *obj, struct ww_acquire_ctx *ctx)
> +{
> +	return dma_resv_lock(obj, ctx);
> +}
> +
> +void rust_helper_dma_resv_unlock(struct dma_resv *obj)
> +{
> +	dma_resv_unlock(obj);
> +}

This is missing __rust_helper

Best,
Gary

> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 0943d589b7578d3c0e207937f63a5e02719c6146..aae78c938b0b5848b1740fb3f2dc4b7f93b1a760 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -25,6 +25,7 @@
>  #include "cred.c"
>  #include "device.c"
>  #include "dma.c"
> +#include "dma-resv.c"
>  #include "drm.c"
>  #include "drm_gpuvm.c"
>  #include "err.c"


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 1/6] rust: drm: add base GPUVM immediate mode abstraction
  2026-01-21 11:31 ` [PATCH v3 1/6] rust: drm: add base GPUVM immediate mode abstraction Alice Ryhl
@ 2026-01-21 17:04   ` Daniel Almeida
  2026-01-22  9:23     ` Alice Ryhl
  0 siblings, 1 reply; 22+ messages in thread
From: Daniel Almeida @ 2026-01-21 17:04 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Danilo Krummrich, Boris Brezillon, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux

Hi Alice,

> On 21 Jan 2026, at 08:31, Alice Ryhl <aliceryhl@google.com> wrote:
> 
> From: Asahi Lina <lina+kernel@asahilina.net>
> 
> Add a GPUVM abstraction to be used by Rust GPU drivers.
> 
> GPUVM keeps track of a GPU's virtual address (VA) space and manages the
> corresponding virtual mappings represented by "GPU VA" objects. It also
> keeps track of the gem::Object<T> used to back the mappings through
> GpuVmBo<T>.
> 
> This abstraction is only usable by drivers that wish to use GPUVM in
> immediate mode. This allows us to build the locking scheme into the API
> design. It means that the GEM mutex is used for the GEM gpuva list, and
> that the resv lock is used for the extobj list. The evicted list is not
> yet used in this version.
> 
> This abstraction provides a special handle called the GpuVmCore, which
> is a wrapper around ARef<GpuVm> that provides access to the interval
> tree. Generally, all changes to the address space requires mutable
> access to this unique handle.
> 
> Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
> Co-developed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
> Co-developed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> MAINTAINERS                     |   2 +
> rust/bindings/bindings_helper.h |   1 +
> rust/helpers/drm_gpuvm.c        |  18 ++++
> rust/helpers/helpers.c          |   1 +
> rust/kernel/drm/gpuvm/mod.rs    | 231 ++++++++++++++++++++++++++++++++++++++++
> rust/kernel/drm/mod.rs          |   1 +
> 6 files changed, 254 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3b84ad595e226f231b256d24f0da6bac459e93a8..618becae72985b9dfdca8469ee48d4752fd0ca41 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8720,6 +8720,8 @@ S: Supported
> T: git https://gitlab.freedesktop.org/drm/misc/kernel.git
> F: drivers/gpu/drm/drm_gpuvm.c
> F: include/drm/drm_gpuvm.h
> +F: rust/helpers/drm_gpuvm.c
> +F: rust/kernel/drm/gpuvm/
> 
> DRM LOG
> M: Jocelyn Falempe <jfalempe@redhat.com>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index a067038b4b422b4256f4a2b75fe644d47e6e82c8..dd60a5c6b142ec2c5fd6df80279ab6813163791c 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -33,6 +33,7 @@
> #include <drm/drm_drv.h>
> #include <drm/drm_file.h>
> #include <drm/drm_gem.h>
> +#include <drm/drm_gpuvm.h>
> #include <drm/drm_ioctl.h>
> #include <kunit/test.h>
> #include <linux/auxiliary_bus.h>
> diff --git a/rust/helpers/drm_gpuvm.c b/rust/helpers/drm_gpuvm.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..d1471e5844ec81f994af9252d9054053ab13f352
> --- /dev/null
> +++ b/rust/helpers/drm_gpuvm.c
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: GPL-2.0 or MIT
> +
> +#ifdef CONFIG_DRM_GPUVM
> +
> +#include <drm/drm_gpuvm.h>
> +
> +struct drm_gpuvm *rust_helper_drm_gpuvm_get(struct drm_gpuvm *obj)
> +{
> + return drm_gpuvm_get(obj);
> +}
> +
> +bool rust_helper_drm_gpuvm_is_extobj(struct drm_gpuvm *gpuvm,
> +     struct drm_gem_object *obj)
> +{
> + return drm_gpuvm_is_extobj(gpuvm, obj);
> +}
> +
> +#endif // CONFIG_DRM_GPUVM
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 79c72762ad9c4b473971e6210c9577860d2e2b08..0943d589b7578d3c0e207937f63a5e02719c6146 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -26,6 +26,7 @@
> #include "device.c"
> #include "dma.c"
> #include "drm.c"
> +#include "drm_gpuvm.c"
> #include "err.c"
> #include "irq.c"
> #include "fs.c"
> diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..81b5e767885d8258c44086444b153c91961ffabc
> --- /dev/null
> +++ b/rust/kernel/drm/gpuvm/mod.rs
> @@ -0,0 +1,231 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +
> +#![cfg(CONFIG_DRM_GPUVM = "y")]
> +
> +//! DRM GPUVM in immediate mode
> +//!
> +//! Rust abstractions for using GPUVM in immediate mode. This is when the GPUVM state is updated
> +//! during `run_job()`, i.e., in the DMA fence signalling critical path, to ensure that the GPUVM
> +//! and the GPU's virtual address space has the same state at all times.
> +//!
> +//! C header: [`include/drm/drm_gpuvm.h`](srctree/include/drm/drm_gpuvm.h)
> +
> +use kernel::{
> +    alloc::AllocError,
> +    bindings,
> +    drm,
> +    drm::gem::IntoGEMObject,
> +    prelude::*,
> +    sync::aref::{
> +        ARef,
> +        AlwaysRefCounted, //
> +    },
> +    types::Opaque, //
> +};
> +
> +use core::{
> +    cell::UnsafeCell,
> +    ops::{
> +        Deref,
> +        Range, //
> +    },
> +    ptr::NonNull, //
> +};
> +
> +/// A DRM GPU VA manager.
> +///
> +/// This object is refcounted, but the "core" is only accessible using a special unique handle. The
> +/// core consists of the `core` field and the GPUVM's interval tree.
> +///
> +/// # Invariants
> +///
> +/// * Stored in an allocation managed by the refcount in `self.vm`.
> +/// * Access to `data` and the gpuvm interval tree is controlled via the [`GpuVmCore`] type.
> +#[pin_data]
> +pub struct GpuVm<T: DriverGpuVm> {
> +    #[pin]
> +    vm: Opaque<bindings::drm_gpuvm>,
> +    /// Accessed only through the [`GpuVmCore`] reference.
> +    data: UnsafeCell<T>,
> +}
> +
> +// SAFETY: By type invariants, the allocation is managed by the refcount in `self.vm`.
> +unsafe impl<T: DriverGpuVm> AlwaysRefCounted for GpuVm<T> {
> +    fn inc_ref(&self) {
> +        // SAFETY: By type invariants, the allocation is managed by the refcount in `self.vm`.
> +        unsafe { bindings::drm_gpuvm_get(self.vm.get()) };
> +    }
> +
> +    unsafe fn dec_ref(obj: NonNull<Self>) {
> +        // SAFETY: By type invariants, the allocation is managed by the refcount in `self.vm`.
> +        unsafe { bindings::drm_gpuvm_put((*obj.as_ptr()).vm.get()) };
> +    }
> +}
> +
> +impl<T: DriverGpuVm> GpuVm<T> {
> +    const fn vtable() -> &'static bindings::drm_gpuvm_ops {
> +        &bindings::drm_gpuvm_ops {
> +            vm_free: Some(Self::vm_free),
> +            op_alloc: None,
> +            op_free: None,
> +            vm_bo_alloc: None,
> +            vm_bo_free: None,
> +            vm_bo_validate: None,
> +            sm_step_map: None,
> +            sm_step_unmap: None,
> +            sm_step_remap: None,
> +        }
> +    }
> +
> +    /// Creates a GPUVM instance.
> +    #[expect(clippy::new_ret_no_self)]
> +    pub fn new<E>(
> +        name: &'static CStr,
> +        dev: &drm::Device<T::Driver>,
> +        r_obj: &T::Object,
> +        range: Range<u64>,
> +        reserve_range: Range<u64>,
> +        data: T,
> +    ) -> Result<GpuVmCore<T>, E>
> +    where
> +        E: From<AllocError>,
> +        E: From<core::convert::Infallible>,
> +    {
> +        let obj = KBox::try_pin_init::<E>(
> +            try_pin_init!(Self {
> +                data: UnsafeCell::new(data),
> +                vm <- Opaque::ffi_init(|vm| {
> +                    // SAFETY: These arguments are valid. `vm` is valid until refcount drops to
> +                    // zero.
> +                    unsafe {
> +                        bindings::drm_gpuvm_init(
> +                            vm,
> +                            name.as_char_ptr(),
> +                            bindings::drm_gpuvm_flags_DRM_GPUVM_IMMEDIATE_MODE
> +                                | bindings::drm_gpuvm_flags_DRM_GPUVM_RESV_PROTECTED,
> +                            dev.as_raw(),
> +                            r_obj.as_raw(),
> +                            range.start,
> +                            range.end - range.start,
> +                            reserve_range.start,
> +                            reserve_range.end - reserve_range.start,
> +                            const { Self::vtable() },
> +                        )
> +                    }
> +                }),
> +            }? E),
> +            GFP_KERNEL,
> +        )?;
> +        // SAFETY: This transfers the initial refcount to the ARef.
> +        Ok(GpuVmCore(unsafe {
> +            ARef::from_raw(NonNull::new_unchecked(KBox::into_raw(
> +                Pin::into_inner_unchecked(obj),
> +            )))
> +        }))
> +    }
> +
> +    /// Access this [`GpuVm`] from a raw pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The pointer must reference the `struct drm_gpuvm` in a valid [`GpuVm<T>`] that remains
> +    /// valid for at least `'a`.
> +    #[inline]
> +    pub unsafe fn from_raw<'a>(ptr: *mut bindings::drm_gpuvm) -> &'a Self {
> +        // SAFETY: Caller passes a pointer to the `drm_gpuvm` in a `GpuVm<T>`. Caller ensures the
> +        // pointer is valid for 'a.
> +        unsafe { &*kernel::container_of!(Opaque::cast_from(ptr), Self, vm) }
> +    }
> +
> +    /// Returns a raw pointer to the embedded `struct drm_gpuvm`.
> +    #[inline]
> +    pub fn as_raw(&self) -> *mut bindings::drm_gpuvm {
> +        self.vm.get()
> +    }
> +
> +    /// The start of the VA space.
> +    #[inline]
> +    pub fn va_start(&self) -> u64 {
> +        // SAFETY: The `mm_start` field is immutable.
> +        unsafe { (*self.as_raw()).mm_start }
> +    }
> +
> +    /// The length of the GPU's virtual address space.
> +    #[inline]
> +    pub fn va_length(&self) -> u64 {
> +        // SAFETY: The `mm_range` field is immutable.
> +        unsafe { (*self.as_raw()).mm_range }
> +    }
> +
> +    /// Returns the range of the GPU virtual address space.
> +    #[inline]
> +    pub fn va_range(&self) -> Range<u64> {
> +        let start = self.va_start();
> +        // OVERFLOW: This reconstructs the Range<u64> passed to the constructor, so it won't fail.
> +        let end = start + self.va_length();
> +        Range { start, end }
> +    }
> +
> +    /// Clean up buffer objects that are no longer used.
> +    #[inline]
> +    pub fn deferred_cleanup(&self) {
> +        // SAFETY: This GPUVM uses immediate mode.
> +        unsafe { bindings::drm_gpuvm_bo_deferred_cleanup(self.as_raw()) }
> +    }
> +
> +    /// Check if this GEM object is an external object for this GPUVM.
> +    #[inline]
> +    pub fn is_extobj(&self, obj: &T::Object) -> bool {
> +        // SAFETY: We may call this with any GPUVM and GEM object.
> +        unsafe { bindings::drm_gpuvm_is_extobj(self.as_raw(), obj.as_raw()) }
> +    }
> +
> +    /// Free this GPUVM.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Called when refcount hits zero.
> +    unsafe extern "C" fn vm_free(me: *mut bindings::drm_gpuvm) {
> +        // SAFETY: Caller passes a pointer to the `drm_gpuvm` in a `GpuVm<T>`.
> +        let me = unsafe { kernel::container_of!(Opaque::cast_from(me), Self, vm).cast_mut() };
> +        // SAFETY: By type invariants we can free it when refcount hits zero.
> +        drop(unsafe { KBox::from_raw(me) })
> +    }
> +}
> +
> +/// The manager for a GPUVM.
> +pub trait DriverGpuVm: Sized {
> +    /// Parent `Driver` for this object.
> +    type Driver: drm::Driver;
> +
> +    /// The kind of GEM object stored in this GPUVM.
> +    type Object: IntoGEMObject;

Hmm, can’t we derive that from Driver::AllocOps? More specifically, shouldn’t we enforce it?

> +}
> +
> +/// The core of the DRM GPU VA manager.
> +///
> +/// This object is a unique reference to the VM that can access the interval tree and the Rust
> +/// `data` field.
> +///
> +/// # Invariants
> +///
> +/// Each `GpuVm` instance has at most one `GpuVmCore` reference.
> +pub struct GpuVmCore<T: DriverGpuVm>(ARef<GpuVm<T>>);
> +
> +impl<T: DriverGpuVm> GpuVmCore<T> {
> +    /// Access the core data of this GPUVM.
> +    #[inline]
> +    pub fn data(&mut self) -> &mut T {
> +        // SAFETY: By the type invariants we may access `core`.
> +        unsafe { &mut *self.0.data.get() }
> +    }
> +}
> +
> +impl<T: DriverGpuVm> Deref for GpuVmCore<T> {
> +    type Target = GpuVm<T>;
> +
> +    #[inline]
> +    fn deref(&self) -> &GpuVm<T> {
> +        &self.0
> +    }
> +}
> diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
> index 1b82b6945edf25b947afc08300e211bd97150d6b..a4b6c5430198571ec701af2ef452cc9ac55870e6 100644
> --- a/rust/kernel/drm/mod.rs
> +++ b/rust/kernel/drm/mod.rs
> @@ -6,6 +6,7 @@
> pub mod driver;
> pub mod file;
> pub mod gem;
> +pub mod gpuvm;
> pub mod ioctl;
> 
> pub use self::device::Device;
> 
> -- 
> 2.52.0.457.g6b5491de43-goog
> 
> 

Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 3/6] rust: gpuvm: add GpuVm::obtain()
  2026-01-21 11:31 ` [PATCH v3 3/6] rust: gpuvm: add GpuVm::obtain() Alice Ryhl
@ 2026-01-21 17:31   ` Daniel Almeida
  2026-01-22  8:20     ` Alice Ryhl
  2026-01-26 15:00   ` Boris Brezillon
  1 sibling, 1 reply; 22+ messages in thread
From: Daniel Almeida @ 2026-01-21 17:31 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Danilo Krummrich, Boris Brezillon, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux

Hi Alice,

This looks good. See a few nits below:

> On 21 Jan 2026, at 08:31, Alice Ryhl <aliceryhl@google.com> wrote:
> 
> This provides a mechanism to create (or look up) VMBO instances, which
> represent the mapping between GPUVM and GEM objects.
> 
> The GpuVmBoResident<T> type can be considered like ARef<GpuVm<T>>,
> except that no way to increment the refcount is provided.

So this is like GpuVmCore, right? Since that is an ARef whose refcount cannot
be incremented.

> 
> The GpuVmBoAlloc<T> type is more akin to a pre-allocated GpuVm<T>, so
> it's not really a GpuVm<T> yet. Its destructor could call

Maybe you mean a pre-allocated GpuVmBo?

> drm_gpuvm_bo_destroy_not_in_lists(), but as the type is currently
> private and never called anywhere, this perf optimization does not need
> to happen now.
> 
> Pre-allocating and obtaining the gpuvm_bo object is exposed as a single
> step. This could theoretically be a problem if one wanted to call
> drm_gpuvm_bo_obtain_prealloc() during the fence signalling critical
> path, but that's not a possibility because:
> 
> 1. Adding the BO to the extobj list requires the resv lock, so it cannot
>   happen during the fence signalling critical path.
> 2. obtain() requires that the BO is not in the extobj list, so obtain()
>   must be called before adding the BO to the extobj list.
> 
> Thus, drm_gpuvm_bo_obtain_prealloc() cannot be called during the fence
> signalling critical path. (For extobjs at least.)

What about internal objects? This is merely a sanity check from my side.

> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> rust/kernel/drm/gpuvm/mod.rs   |  32 +++++-
> rust/kernel/drm/gpuvm/vm_bo.rs | 219 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 248 insertions(+), 3 deletions(-)
> 
> diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs
> index 81b5e767885d8258c44086444b153c91961ffabc..cb576a7ffa07bc108704e008b7f87de52a837930 100644
> --- a/rust/kernel/drm/gpuvm/mod.rs
> +++ b/rust/kernel/drm/gpuvm/mod.rs
> @@ -25,13 +25,20 @@
> 
> use core::{
>     cell::UnsafeCell,
> +    mem::ManuallyDrop,
>     ops::{
>         Deref,
>         Range, //
>     },
> -    ptr::NonNull, //
> +    ptr::{
> +        self,
> +        NonNull, //
> +    }, //
> };
> 
> +mod vm_bo;
> +pub use self::vm_bo::*;
> +
> /// A DRM GPU VA manager.
> ///
> /// This object is refcounted, but the "core" is only accessible using a special unique handle. The
> @@ -68,8 +75,8 @@ const fn vtable() -> &'static bindings::drm_gpuvm_ops {
>             vm_free: Some(Self::vm_free),
>             op_alloc: None,
>             op_free: None,
> -            vm_bo_alloc: None,
> -            vm_bo_free: None,
> +            vm_bo_alloc: GpuVmBo::<T>::ALLOC_FN,
> +            vm_bo_free: GpuVmBo::<T>::FREE_FN,
>             vm_bo_validate: None,
>             sm_step_map: None,
>             sm_step_unmap: None,
> @@ -166,6 +173,16 @@ pub fn va_range(&self) -> Range<u64> {
>         Range { start, end }
>     }
> 
> +    /// Get or create the [`GpuVmBo`] for this gem object.
> +    #[inline]
> +    pub fn obtain(
> +        &self,
> +        obj: &T::Object,
> +        data: impl PinInit<T::VmBoData>,
> +    ) -> Result<GpuVmBoResident<T>, AllocError> {
> +        Ok(GpuVmBoAlloc::new(self, obj, data)?.obtain())
> +    }
> +
>     /// Clean up buffer objects that are no longer used.
>     #[inline]
>     pub fn deferred_cleanup(&self) {
> @@ -191,6 +208,12 @@ pub fn is_extobj(&self, obj: &T::Object) -> bool {
>         // SAFETY: By type invariants we can free it when refcount hits zero.
>         drop(unsafe { KBox::from_raw(me) })
>     }
> +
> +    #[inline]
> +    fn raw_resv_lock(&self) -> *mut bindings::dma_resv {
> +        // SAFETY: `r_obj` is immutable and valid for duration of GPUVM.
> +        unsafe { (*(*self.as_raw()).r_obj).resv }
> +    }

Shouldn’t we call this raw_resv? Adding “lock” to a name may incorrectly
hint that the lock is actually taken.

> }
> 
> /// The manager for a GPUVM.
> @@ -200,6 +223,9 @@ pub trait DriverGpuVm: Sized {
> 
>     /// The kind of GEM object stored in this GPUVM.
>     type Object: IntoGEMObject;
> +
> +    /// Data stored with each `struct drm_gpuvm_bo`.
> +    type VmBoData;
> }
> 
> /// The core of the DRM GPU VA manager.
> diff --git a/rust/kernel/drm/gpuvm/vm_bo.rs b/rust/kernel/drm/gpuvm/vm_bo.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..310fa569b5bd43f0f872ff859b3624377b93d651
> --- /dev/null
> +++ b/rust/kernel/drm/gpuvm/vm_bo.rs
> @@ -0,0 +1,219 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +
> +use super::*;
> +
> +/// Represents that a given GEM object has at least one mapping on this [`GpuVm`] instance.
> +///
> +/// Does not assume that GEM lock is held.
> +#[repr(C)]
> +#[pin_data]
> +pub struct GpuVmBo<T: DriverGpuVm> {
> +    #[pin]
> +    inner: Opaque<bindings::drm_gpuvm_bo>,
> +    #[pin]
> +    data: T::VmBoData,
> +}
> +
> +impl<T: DriverGpuVm> GpuVmBo<T> {
> +    pub(super) const ALLOC_FN: Option<unsafe extern "C" fn() -> *mut bindings::drm_gpuvm_bo> = {
> +        use core::alloc::Layout;
> +        let base = Layout::new::<bindings::drm_gpuvm_bo>();
> +        let rust = Layout::new::<Self>();
> +        assert!(base.size() <= rust.size());
> +        if base.size() != rust.size() || base.align() != rust.align() {
> +            Some(Self::vm_bo_alloc)
> +        } else {
> +            // This causes GPUVM to allocate a `GpuVmBo<T>` with `kzalloc(sizeof(drm_gpuvm_bo))`.
> +            None
> +        }
> +    };
> +
> +    pub(super) const FREE_FN: Option<unsafe extern "C" fn(*mut bindings::drm_gpuvm_bo)> = {
> +        if core::mem::needs_drop::<Self>() {
> +            Some(Self::vm_bo_free)
> +        } else {
> +            // This causes GPUVM to free a `GpuVmBo<T>` with `kfree`.
> +            None
> +        }
> +    };
> +
> +    /// Custom function for allocating a `drm_gpuvm_bo`.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Always safe to call.
> +    // Unsafe to match function pointer type in C struct.

Is this missing an extra “/“ token? Also, I think this is a bit hard to parse initially.

> +    unsafe extern "C" fn vm_bo_alloc() -> *mut bindings::drm_gpuvm_bo {
> +        KBox::<Self>::new_uninit(GFP_KERNEL | __GFP_ZERO)
> +            .map(KBox::into_raw)
> +            .unwrap_or(ptr::null_mut())
> +            .cast()
> +    }
> +
> +    /// Custom function for freeing a `drm_gpuvm_bo`.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The pointer must have been allocated with [`GpuVmBo::ALLOC_FN`], and must not be used after
> +    /// this call.
> +    unsafe extern "C" fn vm_bo_free(ptr: *mut bindings::drm_gpuvm_bo) {
> +        // SAFETY:
> +        // * The ptr was allocated from kmalloc with the layout of `GpuVmBo<T>`.
> +        // * `ptr->inner` has no destructor.
> +        // * `ptr->data` contains a valid `T::VmBoData` that we can drop.
> +        drop(unsafe { KBox::<Self>::from_raw(ptr.cast()) });
> +    }
> +
> +    /// Access this [`GpuVmBo`] from a raw pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// For the duration of `'a`, the pointer must reference a valid `drm_gpuvm_bo` associated with
> +    /// a [`GpuVm<T>`].
> +    #[inline]
> +    pub unsafe fn from_raw<'a>(ptr: *mut bindings::drm_gpuvm_bo) -> &'a Self {
> +        // SAFETY: `drm_gpuvm_bo` is first field and `repr(C)`.
> +        unsafe { &*ptr.cast() }
> +    }
> +
> +    /// Returns a raw pointer to underlying C value.
> +    #[inline]
> +    pub fn as_raw(&self) -> *mut bindings::drm_gpuvm_bo {
> +        self.inner.get()
> +    }
> +
> +    /// The [`GpuVm`] that this GEM object is mapped in.
> +    #[inline]
> +    pub fn gpuvm(&self) -> &GpuVm<T> {
> +        // SAFETY: The `obj` pointer is guaranteed to be valid.
> +        unsafe { GpuVm::<T>::from_raw((*self.inner.get()).vm) }
> +    }
> +
> +    /// The [`drm_gem_object`](crate::gem::Object) for these mappings.
> +    #[inline]
> +    pub fn obj(&self) -> &T::Object {
> +        // SAFETY: The `obj` pointer is guaranteed to be valid.
> +        unsafe { <T::Object as IntoGEMObject>::from_raw((*self.inner.get()).obj) }
> +    }
> +
> +    /// The driver data with this buffer object.
> +    #[inline]
> +    pub fn data(&self) -> &T::VmBoData {
> +        &self.data
> +    }
> +}
> +
> +/// A pre-allocated [`GpuVmBo`] object.
> +///
> +/// # Invariants
> +///
> +/// Points at a `drm_gpuvm_bo` that contains a valid `T::VmBoData`, has a refcount of one, and is
> +/// absent from any gem, extobj, or evict lists.
> +pub(super) struct GpuVmBoAlloc<T: DriverGpuVm>(NonNull<GpuVmBo<T>>);
> +
> +impl<T: DriverGpuVm> GpuVmBoAlloc<T> {
> +    /// Create a new pre-allocated [`GpuVmBo`].
> +    ///
> +    /// It's intentional that the initializer is infallible because `drm_gpuvm_bo_put` will call
> +    /// drop on the data, so we don't have a way to free it when the data is missing.
> +    #[inline]
> +    pub(super) fn new(
> +        gpuvm: &GpuVm<T>,
> +        gem: &T::Object,
> +        value: impl PinInit<T::VmBoData>,
> +    ) -> Result<GpuVmBoAlloc<T>, AllocError> {
> +        // CAST: `GpuVmBoAlloc::vm_bo_alloc` ensures that this memory was allocated with the layout
> +        // of `GpuVmBo<T>`. The type is repr(C), so `container_of` is not required.
> +        // SAFETY: The provided gpuvm and gem ptrs are valid for the duration of this call.
> +        let raw_ptr = unsafe {
> +            bindings::drm_gpuvm_bo_create(gpuvm.as_raw(), gem.as_raw()).cast::<GpuVmBo<T>>()
> +        };
> +        let ptr = NonNull::new(raw_ptr).ok_or(AllocError)?;
> +        // SAFETY: `ptr->data` is a valid pinned location.
> +        let Ok(()) = unsafe { value.__pinned_init(&raw mut (*raw_ptr).data) };
> +        // INVARIANTS: We just created the vm_bo so it's absent from lists, and the data is valid
> +        // as we just initialized it.
> +        Ok(GpuVmBoAlloc(ptr))
> +    }
> +
> +    /// Returns a raw pointer to underlying C value.
> +    #[inline]
> +    pub(super) fn as_raw(&self) -> *mut bindings::drm_gpuvm_bo {
> +        // SAFETY: The pointer references a valid `drm_gpuvm_bo`.
> +        unsafe { (*self.0.as_ptr()).inner.get() }
> +    }
> +
> +    /// Look up whether there is an existing [`GpuVmBo`] for this gem object.
> +    #[inline]
> +    pub(super) fn obtain(self) -> GpuVmBoResident<T> {
> +        let me = ManuallyDrop::new(self);
> +        // SAFETY: Valid `drm_gpuvm_bo` not already in the lists.
> +        let ptr = unsafe { bindings::drm_gpuvm_bo_obtain_prealloc(me.as_raw()) };
> +
> +        // If the vm_bo does not already exist, ensure that it's in the extobj list.

Wording is a bit off. Obviously only external objects should be in the extobj
list. This is correctly captured by the check below, but the wording above
makes it sound unconditional.

> +        if ptr::eq(ptr, me.as_raw()) && me.gpuvm().is_extobj(me.obj()) {
> +            let resv_lock = me.gpuvm().raw_resv_lock();
> +            // SAFETY: The GPUVM is still alive, so its resv lock is too.
> +            unsafe { bindings::dma_resv_lock(resv_lock, ptr::null_mut()) };
> +            // SAFETY: We hold the GPUVMs resv lock.
> +            unsafe { bindings::drm_gpuvm_bo_extobj_add(ptr) };
> +            // SAFETY: We took the lock, so we can unlock it.
> +            unsafe { bindings::dma_resv_unlock(resv_lock) };
> +        }
> +
> +        // INVARIANTS: Valid `drm_gpuvm_bo` in the GEM list.
> +        // SAFETY: `drm_gpuvm_bo_obtain_prealloc` always returns a non-null ptr
> +        GpuVmBoResident(unsafe { NonNull::new_unchecked(ptr.cast()) })
> +    }
> +}
> +
> +impl<T: DriverGpuVm> Deref for GpuVmBoAlloc<T> {
> +    type Target = GpuVmBo<T>;
> +    #[inline]
> +    fn deref(&self) -> &GpuVmBo<T> {
> +        // SAFETY: By the type invariants we may deref while `Self` exists.
> +        unsafe { self.0.as_ref() }
> +    }
> +}
> +
> +impl<T: DriverGpuVm> Drop for GpuVmBoAlloc<T> {
> +    #[inline]
> +    fn drop(&mut self) {
> +        // TODO: Call drm_gpuvm_bo_destroy_not_in_lists() directly.
> +        // SAFETY: It's safe to perform a deferred put in any context.
> +        unsafe { bindings::drm_gpuvm_bo_put_deferred(self.as_raw()) };
> +    }
> +}
> +
> +/// A [`GpuVmBo`] object in the GEM list.
> +///
> +/// # Invariants
> +///
> +/// Points at a `drm_gpuvm_bo` that contains a valid `T::VmBoData` and is present in the gem list.
> +pub struct GpuVmBoResident<T: DriverGpuVm>(NonNull<GpuVmBo<T>>);
> +
> +impl<T: DriverGpuVm> GpuVmBoResident<T> {
> +    /// Returns a raw pointer to underlying C value.
> +    #[inline]
> +    pub fn as_raw(&self) -> *mut bindings::drm_gpuvm_bo {
> +        // SAFETY: The pointer references a valid `drm_gpuvm_bo`.
> +        unsafe { (*self.0.as_ptr()).inner.get() }
> +    }
> +}
> +
> +impl<T: DriverGpuVm> Deref for GpuVmBoResident<T> {
> +    type Target = GpuVmBo<T>;
> +    #[inline]
> +    fn deref(&self) -> &GpuVmBo<T> {
> +        // SAFETY: By the type invariants we may deref while `Self` exists.
> +        unsafe { self.0.as_ref() }
> +    }
> +}
> +
> +impl<T: DriverGpuVm> Drop for GpuVmBoResident<T> {
> +    #[inline]
> +    fn drop(&mut self) {
> +        // SAFETY: It's safe to perform a deferred put in any context.
> +        unsafe { bindings::drm_gpuvm_bo_put_deferred(self.as_raw()) };
> +    }
> +}
> 
> -- 
> 2.52.0.457.g6b5491de43-goog
> 
> 

Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 3/6] rust: gpuvm: add GpuVm::obtain()
  2026-01-21 17:31   ` Daniel Almeida
@ 2026-01-22  8:20     ` Alice Ryhl
  0 siblings, 0 replies; 22+ messages in thread
From: Alice Ryhl @ 2026-01-22  8:20 UTC (permalink / raw)
  To: Daniel Almeida
  Cc: Danilo Krummrich, Boris Brezillon, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux

On Wed, Jan 21, 2026 at 02:31:26PM -0300, Daniel Almeida wrote:
> Hi Alice,
> 
> This looks good. See a few nits below:
> 
> > On 21 Jan 2026, at 08:31, Alice Ryhl <aliceryhl@google.com> wrote:
> > 
> > This provides a mechanism to create (or look up) VMBO instances, which
> > represent the mapping between GPUVM and GEM objects.
> > 
> > The GpuVmBoResident<T> type can be considered like ARef<GpuVm<T>>,
> > except that no way to increment the refcount is provided.
> 
> So this is like GpuVmCore, right? Since that is an ARef whose refcount cannot
> be incremented.

Sort of, except that GpuVmBoResident is not truly unique since you can
call obtain() twice.

> > The GpuVmBoAlloc<T> type is more akin to a pre-allocated GpuVm<T>, so
> > it's not really a GpuVm<T> yet. Its destructor could call
> 
> Maybe you mean a pre-allocated GpuVmBo?

Yes that's what I meant.

> > drm_gpuvm_bo_destroy_not_in_lists(), but as the type is currently
> > private and never called anywhere, this perf optimization does not need
> > to happen now.
> > 
> > Pre-allocating and obtaining the gpuvm_bo object is exposed as a single
> > step. This could theoretically be a problem if one wanted to call
> > drm_gpuvm_bo_obtain_prealloc() during the fence signalling critical
> > path, but that's not a possibility because:
> > 
> > 1. Adding the BO to the extobj list requires the resv lock, so it cannot
> >   happen during the fence signalling critical path.
> > 2. obtain() requires that the BO is not in the extobj list, so obtain()
> >   must be called before adding the BO to the extobj list.
> > 
> > Thus, drm_gpuvm_bo_obtain_prealloc() cannot be called during the fence
> > signalling critical path. (For extobjs at least.)
> 
> What about internal objects? This is merely a sanity check from my side.

There are only two lists: extobj and evicted.

The extobj list is used to find the dma_resv locks of external objects.
This isn't required for internal ones, as they all share the resv lock
of the GPUVM itself.

> > +    #[inline]
> > +    fn raw_resv_lock(&self) -> *mut bindings::dma_resv {
> > +        // SAFETY: `r_obj` is immutable and valid for duration of GPUVM.
> > +        unsafe { (*(*self.as_raw()).r_obj).resv }
> > +    }
> 
> Shouldn’t we call this raw_resv? Adding “lock” to a name may incorrectly
> hint that the lock is actually taken.

Good call.

> > +    /// Custom function for allocating a `drm_gpuvm_bo`.
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// Always safe to call.
> > +    // Unsafe to match function pointer type in C struct.
> 
> Is this missing an extra “/“ token? Also, I think this is a bit hard to parse initially.

Well, I didn't meant to include it in the docs. But I can reformat this
to be less confusing.

> > +    /// Look up whether there is an existing [`GpuVmBo`] for this gem object.
> > +    #[inline]
> > +    pub(super) fn obtain(self) -> GpuVmBoResident<T> {
> > +        let me = ManuallyDrop::new(self);
> > +        // SAFETY: Valid `drm_gpuvm_bo` not already in the lists.
> > +        let ptr = unsafe { bindings::drm_gpuvm_bo_obtain_prealloc(me.as_raw()) };
> > +
> > +        // If the vm_bo does not already exist, ensure that it's in the extobj list.
> 
> Wording is a bit off. Obviously only external objects should be in the extobj
> list. This is correctly captured by the check below, but the wording above
> makes it sound unconditional.

I'll update the comment. The "does not already exist" refers to the
`ptr::eq()` part of the condition, which checks whether the
pre-allocated vm_bo was created, or whether the GPUVM already has a
vm_bo for this GEM object.

> > +        if ptr::eq(ptr, me.as_raw()) && me.gpuvm().is_extobj(me.obj()) {
> > +            let resv_lock = me.gpuvm().raw_resv_lock();
> > +            // SAFETY: The GPUVM is still alive, so its resv lock is too.
> > +            unsafe { bindings::dma_resv_lock(resv_lock, ptr::null_mut()) };
> > +            // SAFETY: We hold the GPUVMs resv lock.
> > +            unsafe { bindings::drm_gpuvm_bo_extobj_add(ptr) };
> > +            // SAFETY: We took the lock, so we can unlock it.
> > +            unsafe { bindings::dma_resv_unlock(resv_lock) };
> > +        }
> > +
> > +        // INVARIANTS: Valid `drm_gpuvm_bo` in the GEM list.
> > +        // SAFETY: `drm_gpuvm_bo_obtain_prealloc` always returns a non-null ptr
> > +        GpuVmBoResident(unsafe { NonNull::new_unchecked(ptr.cast()) })
> > +    }
> > +}

> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

Thanks for the reivew!

Alice

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 1/6] rust: drm: add base GPUVM immediate mode abstraction
  2026-01-21 17:04   ` Daniel Almeida
@ 2026-01-22  9:23     ` Alice Ryhl
  0 siblings, 0 replies; 22+ messages in thread
From: Alice Ryhl @ 2026-01-22  9:23 UTC (permalink / raw)
  To: Daniel Almeida
  Cc: Danilo Krummrich, Boris Brezillon, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux

On Wed, Jan 21, 2026 at 02:04:33PM -0300, Daniel Almeida wrote:
> Hi Alice,
> 
> > On 21 Jan 2026, at 08:31, Alice Ryhl <aliceryhl@google.com> wrote:
> > +/// The manager for a GPUVM.
> > +pub trait DriverGpuVm: Sized {
> > +    /// Parent `Driver` for this object.
> > +    type Driver: drm::Driver;
> > +
> > +    /// The kind of GEM object stored in this GPUVM.
> > +    type Object: IntoGEMObject;
> 
> Hmm, can’t we derive that from Driver::AllocOps? More specifically, shouldn’t we enforce it?

Hrm, we may wish to require that Self::Object == Self::Driver::Object.

Not sure what you mean by AllocOps.

Alice

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 4/6] rust: gpuvm: add GpuVa struct
  2026-01-21 11:31 ` [PATCH v3 4/6] rust: gpuvm: add GpuVa struct Alice Ryhl
@ 2026-01-22 22:09   ` Daniel Almeida
  0 siblings, 0 replies; 22+ messages in thread
From: Daniel Almeida @ 2026-01-22 22:09 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Danilo Krummrich, Boris Brezillon, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux



> On 21 Jan 2026, at 08:31, Alice Ryhl <aliceryhl@google.com> wrote:
> 
> This struct will be used to keep track of individual mapped ranges in
> the GPU's virtual memory.
> 
> Co-developed-by: Asahi Lina <lina+kernel@asahilina.net>
> Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
> Co-developed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> rust/kernel/drm/gpuvm/mod.rs |  17 ++++-
> rust/kernel/drm/gpuvm/va.rs  | 149 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 164 insertions(+), 2 deletions(-)
> 
> diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs
> index cb576a7ffa07bc108704e008b7f87de52a837930..2179ddd717d8728bbe231bd94ea7b5d1e2652543 100644
> --- a/rust/kernel/drm/gpuvm/mod.rs
> +++ b/rust/kernel/drm/gpuvm/mod.rs
> @@ -11,7 +11,10 @@
> //! C header: [`include/drm/drm_gpuvm.h`](srctree/include/drm/drm_gpuvm.h)
> 
> use kernel::{
> -    alloc::AllocError,
> +    alloc::{
> +        AllocError,
> +        Flags as AllocFlags, //
> +    },
>     bindings,
>     drm,
>     drm::gem::IntoGEMObject,
> @@ -25,9 +28,13 @@
> 
> use core::{
>     cell::UnsafeCell,
> -    mem::ManuallyDrop,
> +    mem::{
> +        ManuallyDrop,
> +        MaybeUninit, //
> +    },
>     ops::{
>         Deref,
> +        DerefMut,
>         Range, //
>     },
>     ptr::{
> @@ -36,6 +43,9 @@
>     }, //
> };
> 
> +mod va;
> +pub use self::va::*;
> +
> mod vm_bo;
> pub use self::vm_bo::*;
> 
> @@ -224,6 +234,9 @@ pub trait DriverGpuVm: Sized {
>     /// The kind of GEM object stored in this GPUVM.
>     type Object: IntoGEMObject;
> 
> +    /// Data stored with each `struct drm_gpuva`.
> +    type VaData;
> +
>     /// Data stored with each `struct drm_gpuvm_bo`.
>     type VmBoData;
> }
> diff --git a/rust/kernel/drm/gpuvm/va.rs b/rust/kernel/drm/gpuvm/va.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..c96796a6b2c8c7c4b5475324562968ca0f07fd09
> --- /dev/null
> +++ b/rust/kernel/drm/gpuvm/va.rs
> @@ -0,0 +1,149 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +
> +#![expect(dead_code)]
> +use super::*;
> +
> +/// Represents that a range of a GEM object is mapped in this [`GpuVm`] instance.
> +///
> +/// Does not assume that GEM lock is held.
> +///
> +/// # Invariants
> +///
> +/// This is a valid `drm_gpuva` that is resident in the [`GpuVm`] instance.
> +#[repr(C)]
> +#[pin_data]
> +pub struct GpuVa<T: DriverGpuVm> {
> +    #[pin]
> +    inner: Opaque<bindings::drm_gpuva>,
> +    #[pin]
> +    data: T::VaData,
> +}
> +
> +impl<T: DriverGpuVm> GpuVa<T> {
> +    /// Access this [`GpuVa`] from a raw pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// For the duration of `'a`, the pointer must reference a valid `drm_gpuva` associated with a
> +    /// [`GpuVm<T>`].
> +    #[inline]
> +    pub unsafe fn from_raw<'a>(ptr: *mut bindings::drm_gpuva) -> &'a Self {
> +        // SAFETY: `drm_gpuva` is first field and `repr(C)`.
> +        unsafe { &*ptr.cast() }
> +    }
> +
> +    /// Returns a raw pointer to underlying C value.
> +    #[inline]
> +    pub fn as_raw(&self) -> *mut bindings::drm_gpuva {
> +        self.inner.get()
> +    }
> +
> +    /// Returns the address of this mapping in the GPU virtual address space.
> +    #[inline]
> +    pub fn addr(&self) -> u64 {
> +        // SAFETY: The `va.addr` field of `drm_gpuva` is immutable.
> +        unsafe { (*self.as_raw()).va.addr }
> +    }
> +
> +    /// Returns the length of this mapping.
> +    #[inline]
> +    pub fn length(&self) -> u64 {
> +        // SAFETY: The `va.range` field of `drm_gpuva` is immutable.
> +        unsafe { (*self.as_raw()).va.range }
> +    }
> +
> +    /// Returns `addr..addr+length`.
> +    #[inline]
> +    pub fn range(&self) -> Range<u64> {
> +        let addr = self.addr();
> +        addr..addr + self.length()
> +    }
> +
> +    /// Returns the offset within the GEM object.
> +    #[inline]
> +    pub fn gem_offset(&self) -> u64 {
> +        // SAFETY: The `gem.offset` field of `drm_gpuva` is immutable.
> +        unsafe { (*self.as_raw()).gem.offset }
> +    }
> +
> +    /// Returns the GEM object.
> +    #[inline]
> +    pub fn obj(&self) -> &T::Object {
> +        // SAFETY: The `gem.offset` field of `drm_gpuva` is immutable.
> +        unsafe { <T::Object as IntoGEMObject>::from_raw((*self.as_raw()).gem.obj) }
> +    }
> +
> +    /// Returns the underlying [`GpuVmBo`] object that backs this [`GpuVa`].
> +    #[inline]
> +    pub fn vm_bo(&self) -> &GpuVmBo<T> {
> +        // SAFETY: The `vm_bo` field has been set and is immutable for the duration in which this
> +        // `drm_gpuva` is resident in the VM.
> +        unsafe { GpuVmBo::from_raw((*self.as_raw()).vm_bo) }
> +    }
> +}
> +
> +/// A pre-allocated [`GpuVa`] object.
> +///
> +/// # Invariants
> +///
> +/// The memory is zeroed.
> +pub struct GpuVaAlloc<T: DriverGpuVm>(KBox<MaybeUninit<GpuVa<T>>>);
> +
> +impl<T: DriverGpuVm> GpuVaAlloc<T> {
> +    /// Pre-allocate a [`GpuVa`] object.
> +    pub fn new(flags: AllocFlags) -> Result<GpuVaAlloc<T>, AllocError> {
> +        // INVARIANTS: Memory allocated with __GFP_ZERO.
> +        Ok(GpuVaAlloc(KBox::new_uninit(flags | __GFP_ZERO)?))
> +    }
> +
> +    /// Prepare this `drm_gpuva` for insertion into the GPUVM.
> +    pub(super) fn prepare(mut self, va_data: impl PinInit<T::VaData>) -> *mut bindings::drm_gpuva {
> +        let va_ptr = MaybeUninit::as_mut_ptr(&mut self.0);
> +        // SAFETY: The `data` field is pinned.
> +        let Ok(()) = unsafe { va_data.__pinned_init(&raw mut (*va_ptr).data) };
> +        KBox::into_raw(self.0).cast()
> +    }
> +}
> +
> +/// A [`GpuVa`] object that has been removed.
> +///
> +/// # Invariants
> +///
> +/// The `drm_gpuva` is not resident in the [`GpuVm`].
> +pub struct GpuVaRemoved<T: DriverGpuVm>(KBox<GpuVa<T>>);
> +
> +impl<T: DriverGpuVm> GpuVaRemoved<T> {
> +    /// Convert a raw pointer into a [`GpuVaRemoved`].
> +    ///
> +    /// # Safety
> +    ///
> +    /// Must have been removed from a [`GpuVm<T>`].
> +    pub(super) unsafe fn from_raw(ptr: *mut bindings::drm_gpuva) -> Self {
> +        // SAFETY: Since it has been removed we can take ownership of allocation.
> +        GpuVaRemoved(unsafe { KBox::from_raw(ptr.cast()) })
> +    }
> +
> +    /// Take ownership of the VA data.
> +    pub fn into_inner(self) -> T::VaData
> +    where
> +        T::VaData: Unpin,
> +    {
> +        KBox::into_inner(self.0).data
> +    }
> +}
> +
> +impl<T: DriverGpuVm> Deref for GpuVaRemoved<T> {
> +    type Target = T::VaData;
> +    fn deref(&self) -> &T::VaData {
> +        &self.0.data
> +    }
> +}
> +
> +impl<T: DriverGpuVm> DerefMut for GpuVaRemoved<T>
> +where
> +    T::VaData: Unpin,
> +{
> +    fn deref_mut(&mut self) -> &mut T::VaData {
> +        &mut self.0.data
> +    }
> +}
> 
> -- 
> 2.52.0.457.g6b5491de43-goog
> 

Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 5/6] rust: gpuvm: add GpuVmCore::sm_unmap()
  2026-01-21 11:31 ` [PATCH v3 5/6] rust: gpuvm: add GpuVmCore::sm_unmap() Alice Ryhl
@ 2026-01-22 22:43   ` Daniel Almeida
  2026-01-23  9:23     ` [PATCH v3 5/6] rust: gpuvm: add GpuVmCore::sm_unmap()' Alice Ryhl
  0 siblings, 1 reply; 22+ messages in thread
From: Daniel Almeida @ 2026-01-22 22:43 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Danilo Krummrich, Boris Brezillon, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux

Hi Alice,

> On 21 Jan 2026, at 08:31, Alice Ryhl <aliceryhl@google.com> wrote:
> 
> Add the entrypoint for unmapping ranges in the GPUVM, and provide
> callbacks and VA types for the implementation.
> 
> Co-developed-by: Asahi Lina <lina+kernel@asahilina.net>
> Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> rust/kernel/drm/gpuvm/mod.rs    |  30 ++++-
> rust/kernel/drm/gpuvm/sm_ops.rs | 264 ++++++++++++++++++++++++++++++++++++++++
> rust/kernel/drm/gpuvm/va.rs     |   1 -
> rust/kernel/drm/gpuvm/vm_bo.rs  |   8 ++
> 4 files changed, 298 insertions(+), 5 deletions(-)
> 
> diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs
> index 2179ddd717d8728bbe231bd94ea7b5d1e2652543..165a25666ccc3d62e59b73483d4eedff044423e9 100644
> --- a/rust/kernel/drm/gpuvm/mod.rs
> +++ b/rust/kernel/drm/gpuvm/mod.rs
> @@ -18,6 +18,7 @@
>     bindings,
>     drm,
>     drm::gem::IntoGEMObject,
> +    error::to_result,
>     prelude::*,
>     sync::aref::{
>         ARef,
> @@ -28,6 +29,7 @@
> 
> use core::{
>     cell::UnsafeCell,
> +    marker::PhantomData,
>     mem::{
>         ManuallyDrop,
>         MaybeUninit, //
> @@ -43,12 +45,15 @@
>     }, //
> };
> 
> -mod va;
> -pub use self::va::*;
> +mod sm_ops;
> +pub use self::sm_ops::*;
> 
> mod vm_bo;
> pub use self::vm_bo::*;
> 
> +mod va;
> +pub use self::va::*;
> +
> /// A DRM GPU VA manager.
> ///
> /// This object is refcounted, but the "core" is only accessible using a special unique handle. The
> @@ -89,8 +94,8 @@ const fn vtable() -> &'static bindings::drm_gpuvm_ops {
>             vm_bo_free: GpuVmBo::<T>::FREE_FN,
>             vm_bo_validate: None,
>             sm_step_map: None,
> -            sm_step_unmap: None,
> -            sm_step_remap: None,
> +            sm_step_unmap: Some(Self::sm_step_unmap),
> +            sm_step_remap: Some(Self::sm_step_remap),
>         }
>     }
> 
> @@ -239,6 +244,23 @@ pub trait DriverGpuVm: Sized {
> 
>     /// Data stored with each `struct drm_gpuvm_bo`.
>     type VmBoData;
> +
> +    /// The private data passed to callbacks.
> +    type SmContext<'ctx>;

As I said, this lifetime fixed an issue that Deborah was having. Thanks a lot!

> +
> +    /// Indicates that an existing mapping should be removed.
> +    fn sm_step_unmap<'op, 'ctx>(
> +        &mut self,
> +        op: OpUnmap<'op, Self>,
> +        context: &mut Self::SmContext<'ctx>,
> +    ) -> Result<OpUnmapped<'op, Self>, Error>;
> +
> +    /// Indicates that an existing mapping should be split up.
> +    fn sm_step_remap<'op, 'ctx>(
> +        &mut self,
> +        op: OpRemap<'op, Self>,
> +        context: &mut Self::SmContext<'ctx>,
> +    ) -> Result<OpRemapped<'op, Self>, Error>;
> }
> 
> /// The core of the DRM GPU VA manager.
> diff --git a/rust/kernel/drm/gpuvm/sm_ops.rs b/rust/kernel/drm/gpuvm/sm_ops.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..3c29d10d63f0b0a1976c714a86d486948ba81a15
> --- /dev/null
> +++ b/rust/kernel/drm/gpuvm/sm_ops.rs
> @@ -0,0 +1,264 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +
> +use super::*;
> +
> +/// The actual data that gets threaded through the callbacks.
> +struct SmData<'a, 'ctx, T: DriverGpuVm> {
> +    gpuvm: &'a mut GpuVmCore<T>,
> +    user_context: &'a mut T::SmContext<'ctx>,
> +}
> +
> +/// Represents an `sm_step_unmap` operation that has not yet been completed.
> +pub struct OpUnmap<'op, T: DriverGpuVm> {
> +    op: &'op bindings::drm_gpuva_op_unmap,
> +    _invariant: PhantomData<*mut &'op mut T>,

Would have been cool to explain why we have a pointer in this PhantomData.

Same elsewhere, IMHO. Helps with maintainability in the future.

(To be honest, I’m not really sure what’s going on here..)


> +}
> +
> +impl<'op, T: DriverGpuVm> OpUnmap<'op, T> {
> +    /// Indicates whether this `drm_gpuva` is physically contiguous with the
> +    /// original mapping request.
> +    ///
> +    /// Optionally, if `keep` is set, drivers may keep the actual page table
> +    /// mappings for this `drm_gpuva`, adding the missing page table entries
> +    /// only and update the `drm_gpuvm` accordingly.
> +    pub fn keep(&self) -> bool {
> +        self.op.keep
> +    }
> +
> +    /// The range being unmapped.
> +    pub fn va(&self) -> &GpuVa<T> {
> +        // SAFETY: This is a valid va.
> +        unsafe { GpuVa::<T>::from_raw(self.op.va) }
> +    }
> +
> +    /// Remove the VA.
> +    pub fn remove(self) -> (OpUnmapped<'op, T>, GpuVaRemoved<T>) {
> +        // SAFETY: The op references a valid drm_gpuva in the GPUVM.
> +        unsafe { bindings::drm_gpuva_unmap(self.op) };
> +        // SAFETY: The va is no longer in the interval tree so we may unlink it.
> +        unsafe { bindings::drm_gpuva_unlink_defer(self.op.va) };
> +
> +        // SAFETY: We just removed this va from the `GpuVm<T>`.
> +        let va = unsafe { GpuVaRemoved::from_raw(self.op.va) };
> +
> +        (
> +            OpUnmapped {
> +                _invariant: self._invariant,
> +            },
> +            va,
> +        )
> +    }
> +}
> +
> +/// Represents a completed [`OpUnmap`] operation.
> +pub struct OpUnmapped<'op, T> {
> +    _invariant: PhantomData<*mut &'op mut T>,
> +}
> +
> +/// Represents an `sm_step_remap` operation that has not yet been completed.
> +pub struct OpRemap<'op, T: DriverGpuVm> {
> +    op: &'op bindings::drm_gpuva_op_remap,
> +    _invariant: PhantomData<*mut &'op mut T>,
> +}
> +
> +impl<'op, T: DriverGpuVm> OpRemap<'op, T> {
> +    /// The preceding part of a split mapping.
> +    #[inline]
> +    pub fn prev(&self) -> Option<&OpRemapMapData> {
> +        // SAFETY: We checked for null, so the pointer must be valid.
> +        NonNull::new(self.op.prev).map(|ptr| unsafe { OpRemapMapData::from_raw(ptr) })
> +    }
> +
> +    /// The subsequent part of a split mapping.
> +    #[inline]
> +    pub fn next(&self) -> Option<&OpRemapMapData> {
> +        // SAFETY: We checked for null, so the pointer must be valid.
> +        NonNull::new(self.op.next).map(|ptr| unsafe { OpRemapMapData::from_raw(ptr) })
> +    }
> +
> +    /// Indicates whether the `drm_gpuva` being removed is physically contiguous with the original
> +    /// mapping request.

We should probably link to the Rust VA type using [`GpuVa`] or something? Nbd either way.

> +    ///
> +    /// Optionally, if `keep` is set, drivers may keep the actual page table mappings for this
> +    /// `drm_gpuva`, adding the missing page table entries only and update the `drm_gpuvm`
> +    /// accordingly.
> +    #[inline]
> +    pub fn keep(&self) -> bool {
> +        // SAFETY: The unmap pointer is always valid.
> +        unsafe { (*self.op.unmap).keep }
> +    }
> +
> +    /// The range being unmapped.
> +    #[inline]
> +    pub fn va_to_unmap(&self) -> &GpuVa<T> {
> +        // SAFETY: This is a valid va.
> +        unsafe { GpuVa::<T>::from_raw((*self.op.unmap).va) }
> +    }
> +
> +    /// The [`drm_gem_object`](crate::gem::Object) whose VA is being remapped.
> +    #[inline]
> +    pub fn obj(&self) -> &T::Object {
> +        self.va_to_unmap().obj()
> +    }
> +
> +    /// The [`GpuVmBo`] that is being remapped.
> +    #[inline]
> +    pub fn vm_bo(&self) -> &GpuVmBo<T> {
> +        self.va_to_unmap().vm_bo()
> +    }
> +
> +    /// Update the GPUVM to perform the remapping.
> +    pub fn remap(
> +        self,
> +        va_alloc: [GpuVaAlloc<T>; 2],
> +        prev_data: impl PinInit<T::VaData>,
> +        next_data: impl PinInit<T::VaData>,
> +    ) -> (OpRemapped<'op, T>, OpRemapRet<T>) {
> +        let [va1, va2] = va_alloc;
> +
> +        let mut unused_va = None;
> +        let mut prev_ptr = ptr::null_mut();
> +        let mut next_ptr = ptr::null_mut();
> +        if self.prev().is_some() {
> +            prev_ptr = va1.prepare(prev_data);
> +        } else {
> +            unused_va = Some(va1);
> +        }
> +        if self.next().is_some() {
> +            next_ptr = va2.prepare(next_data);
> +        } else {
> +            unused_va = Some(va2);
> +        }
> +
> +        // SAFETY: the pointers are non-null when required
> +        unsafe { bindings::drm_gpuva_remap(prev_ptr, next_ptr, self.op) };
> +
> +        let gpuva_guard = self.vm_bo().lock_gpuva();
> +        if !prev_ptr.is_null() {
> +            // SAFETY: The prev_ptr is a valid drm_gpuva prepared for insertion. The vm_bo is still
> +            // valid as the not-yet-unlinked gpuva holds a refcount on the vm_bo.
> +            unsafe { bindings::drm_gpuva_link(prev_ptr, self.vm_bo().as_raw()) };
> +        }
> +        if !next_ptr.is_null() {
> +            // SAFETY: The next_ptr is a valid drm_gpuva prepared for insertion. The vm_bo is still
> +            // valid as the not-yet-unlinked gpuva holds a refcount on the vm_bo.
> +            unsafe { bindings::drm_gpuva_link(next_ptr, self.vm_bo().as_raw()) };
> +        }
> +        drop(gpuva_guard);
> +
> +        // SAFETY: The va is no longer in the interval tree so we may unlink it.
> +        unsafe { bindings::drm_gpuva_unlink_defer((*self.op.unmap).va) };
> +
> +        (
> +            OpRemapped {
> +                _invariant: self._invariant,
> +            },
> +            OpRemapRet {
> +                // SAFETY: We just removed this va from the `GpuVm<T>`.
> +                unmapped_va: unsafe { GpuVaRemoved::from_raw((*self.op.unmap).va) },
> +                unused_va,
> +            },
> +        )
> +    }
> +}
> +
> +/// Part of an [`OpRemap`] that represents a new mapping.
> +#[repr(transparent)]
> +pub struct OpRemapMapData(bindings::drm_gpuva_op_map);
> +
> +impl OpRemapMapData {
> +    /// # Safety
> +    /// Must reference a valid `drm_gpuva_op_map` for duration of `'a`.
> +    unsafe fn from_raw<'a>(ptr: NonNull<bindings::drm_gpuva_op_map>) -> &'a Self {
> +        // SAFETY: ok per safety requirements
> +        unsafe { ptr.cast().as_ref() }
> +    }
> +
> +    /// The base address of the new mapping.
> +    pub fn addr(&self) -> u64 {
> +        self.0.va.addr
> +    }
> +
> +    /// The length of the new mapping.
> +    pub fn length(&self) -> u64 {
> +        self.0.va.range
> +    }
> +
> +    /// The offset within the [`drm_gem_object`](crate::gem::Object).
> +    pub fn gem_offset(&self) -> u64 {
> +        self.0.gem.offset
> +    }
> +}
> +
> +/// Struct containing objects removed or not used by [`OpRemap::remap`].
> +pub struct OpRemapRet<T: DriverGpuVm> {
> +    /// The `drm_gpuva` that was removed.
> +    pub unmapped_va: GpuVaRemoved<T>,
> +    /// If the remap did not split the region into two pieces, then the unused `drm_gpuva` is
> +    /// returned here.
> +    pub unused_va: Option<GpuVaAlloc<T>>,
> +}
> +
> +/// Represents a completed [`OpRemap`] operation.
> +pub struct OpRemapped<'op, T> {
> +    _invariant: PhantomData<*mut &'op mut T>,
> +}
> +
> +impl<T: DriverGpuVm> GpuVmCore<T> {
> +    /// Remove any mappings in the given region.
> +    ///
> +    /// Internally calls [`DriverGpuVm::sm_step_unmap`] for ranges entirely contained within the
> +    /// given range, and [`DriverGpuVm::sm_step_remap`] for ranges that overlap with the range.
> +    #[inline]
> +    pub fn sm_unmap(&mut self, addr: u64, length: u64, context: &mut T::SmContext<'_>) -> Result {
> +        let gpuvm = self.as_raw();
> +        let mut p = SmData {
> +            gpuvm: self,
> +            user_context: context,
> +        };
> +        // SAFETY:
> +        // * raw_request() creates a valid request.
> +        // * The private data is valid to be interpreted as SmData.
> +        to_result(unsafe { bindings::drm_gpuvm_sm_unmap(gpuvm, (&raw mut p).cast(), addr, length) })
> +    }
> +}
> +
> +impl<T: DriverGpuVm> GpuVm<T> {
> +    /// # Safety
> +    /// Must be called from `sm_unmap` with a pointer to `SmData`.
> +    pub(super) unsafe extern "C" fn sm_step_unmap(
> +        op: *mut bindings::drm_gpuva_op,
> +        p: *mut c_void,
> +    ) -> c_int {
> +        // SAFETY: The caller provides a pointer to `SmData`.
> +        let p = unsafe { &mut *p.cast::<SmData<'_, '_, T>>() };
> +        let op = OpUnmap {
> +            // SAFETY: sm_step_unmap is called with an unmap operation.
> +            op: unsafe { &(*op).__bindgen_anon_1.unmap },
> +            _invariant: PhantomData,
> +        };
> +        match p.gpuvm.data().sm_step_unmap(op, p.user_context) {
> +            Ok(OpUnmapped { .. }) => 0,
> +            Err(err) => err.to_errno(),
> +        }
> +    }
> +
> +    /// # Safety
> +    /// Must be called from `sm_unmap` with a pointer to `SmData`.
> +    pub(super) unsafe extern "C" fn sm_step_remap(
> +        op: *mut bindings::drm_gpuva_op,
> +        p: *mut c_void,
> +    ) -> c_int {
> +        // SAFETY: The caller provides a pointer to `SmData`.
> +        let p = unsafe { &mut *p.cast::<SmData<'_, '_, T>>() };
> +        let op = OpRemap {
> +            // SAFETY: sm_step_remap is called with a remap operation.
> +            op: unsafe { &(*op).__bindgen_anon_1.remap },
> +            _invariant: PhantomData,
> +        };
> +        match p.gpuvm.data().sm_step_remap(op, p.user_context) {
> +            Ok(OpRemapped { .. }) => 0,
> +            Err(err) => err.to_errno(),
> +        }
> +    }
> +}
> diff --git a/rust/kernel/drm/gpuvm/va.rs b/rust/kernel/drm/gpuvm/va.rs
> index c96796a6b2c8c7c4b5475324562968ca0f07fd09..a31122ff22282186a1d76d4bb085714f6465722b 100644
> --- a/rust/kernel/drm/gpuvm/va.rs
> +++ b/rust/kernel/drm/gpuvm/va.rs
> @@ -1,6 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0 OR MIT
> 
> -#![expect(dead_code)]
> use super::*;
> 
> /// Represents that a range of a GEM object is mapped in this [`GpuVm`] instance.
> diff --git a/rust/kernel/drm/gpuvm/vm_bo.rs b/rust/kernel/drm/gpuvm/vm_bo.rs
> index 310fa569b5bd43f0f872ff859b3624377b93d651..f600dfb15379233111b5893f6aa85a12e6c9e131 100644
> --- a/rust/kernel/drm/gpuvm/vm_bo.rs
> +++ b/rust/kernel/drm/gpuvm/vm_bo.rs
> @@ -101,6 +101,14 @@ pub fn obj(&self) -> &T::Object {
>     pub fn data(&self) -> &T::VmBoData {
>         &self.data
>     }
> +
> +    pub(super) fn lock_gpuva(&self) -> crate::sync::MutexGuard<'_, ()> {
> +        // SAFETY: The GEM object is valid.
> +        let ptr = unsafe { &raw mut (*self.obj().as_raw()).gpuva.lock };
> +        // SAFETY: The GEM object is valid, so the mutex is properly initialized.
> +        let mutex = unsafe { crate::sync::Mutex::from_raw(ptr) };
> +        mutex.lock()
> +    }
> }
> 
> /// A pre-allocated [`GpuVmBo`] object.
> 
> -- 
> 2.52.0.457.g6b5491de43-goog
> 
> 


Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 6/6] rust: gpuvm: add GpuVmCore::sm_map()
  2026-01-21 11:31 ` [PATCH v3 6/6] rust: gpuvm: add GpuVmCore::sm_map() Alice Ryhl
@ 2026-01-22 22:58   ` Daniel Almeida
  2026-01-23  9:23     ` Alice Ryhl
  0 siblings, 1 reply; 22+ messages in thread
From: Daniel Almeida @ 2026-01-22 22:58 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Danilo Krummrich, Boris Brezillon, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux

Hi Alice,

> On 21 Jan 2026, at 08:31, Alice Ryhl <aliceryhl@google.com> wrote:
> 
> Finally also add the operation for creating new mappings. Mapping
> operations need extra data in the context since they involve a vm_bo
> coming from the outside.
> 
> Co-developed-by: Asahi Lina <lina+kernel@asahilina.net>
> Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> rust/kernel/drm/gpuvm/mod.rs    |   9 ++-
> rust/kernel/drm/gpuvm/sm_ops.rs | 154 ++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 157 insertions(+), 6 deletions(-)
> 
> diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs
> index 165a25666ccc3d62e59b73483d4eedff044423e9..557c0d629eec912a97fc4ef18495d5bf0807db0a 100644
> --- a/rust/kernel/drm/gpuvm/mod.rs
> +++ b/rust/kernel/drm/gpuvm/mod.rs
> @@ -93,7 +93,7 @@ const fn vtable() -> &'static bindings::drm_gpuvm_ops {
>             vm_bo_alloc: GpuVmBo::<T>::ALLOC_FN,
>             vm_bo_free: GpuVmBo::<T>::FREE_FN,
>             vm_bo_validate: None,
> -            sm_step_map: None,
> +            sm_step_map: Some(Self::sm_step_map),
>             sm_step_unmap: Some(Self::sm_step_unmap),
>             sm_step_remap: Some(Self::sm_step_remap),
>         }
> @@ -248,6 +248,13 @@ pub trait DriverGpuVm: Sized {
>     /// The private data passed to callbacks.
>     type SmContext<'ctx>;
> 
> +    /// Indicates that a new mapping should be created.
> +    fn sm_step_map<'op, 'ctx>(
> +        &mut self,
> +        op: OpMap<'op, Self>,
> +        context: &mut Self::SmContext<'ctx>,
> +    ) -> Result<OpMapped<'op, Self>, Error>;
> +
>     /// Indicates that an existing mapping should be removed.
>     fn sm_step_unmap<'op, 'ctx>(
>         &mut self,
> diff --git a/rust/kernel/drm/gpuvm/sm_ops.rs b/rust/kernel/drm/gpuvm/sm_ops.rs
> index 3c29d10d63f0b0a1976c714a86d486948ba81a15..5f3c5d3918147a6962e5658443c343835baa10b8 100644
> --- a/rust/kernel/drm/gpuvm/sm_ops.rs
> +++ b/rust/kernel/drm/gpuvm/sm_ops.rs
> @@ -8,6 +8,100 @@ struct SmData<'a, 'ctx, T: DriverGpuVm> {
>     user_context: &'a mut T::SmContext<'ctx>,
> }
> 
> +#[repr(C)]
> +struct SmMapData<'a, 'ctx, T: DriverGpuVm> {
> +    sm_data: SmData<'a, 'ctx, T>,
> +    vm_bo: GpuVmBoResident<T>,
> +}
> +
> +/// The argument for [`GpuVmCore::sm_map`].
> +pub struct OpMapRequest<'a, 'ctx, T: DriverGpuVm> {
> +    /// Address in GPU virtual address space.
> +    pub addr: u64,
> +    /// Length of mapping to create.
> +    pub range: u64,
> +    /// Offset in GEM object.
> +    pub offset: u64,

I’d rename this gem_offset. A bit vague/confusing otherwise.

> +    /// The GEM object to map.
> +    pub vm_bo: GpuVmBoResident<T>,
> +    /// The user-provided context type.
> +    pub context: &'a mut T::SmContext<'ctx>,
> +}
> +
> +impl<'a, 'ctx, T: DriverGpuVm> OpMapRequest<'a, 'ctx, T> {
> +    fn raw_request(&self) -> bindings::drm_gpuvm_map_req {
> +        bindings::drm_gpuvm_map_req {
> +            map: bindings::drm_gpuva_op_map {
> +                va: bindings::drm_gpuva_op_map__bindgen_ty_1 {
> +                    addr: self.addr,
> +                    range: self.range,
> +                },
> +                gem: bindings::drm_gpuva_op_map__bindgen_ty_2 {
> +                    offset: self.offset,
> +                    obj: self.vm_bo.obj().as_raw(),
> +                },
> +            },
> +        }
> +    }
> +}
> +
> +/// Represents an `sm_step_map` operation that has not yet been completed.

> +pub struct OpMap<'op, T: DriverGpuVm> {
> +    op: &'op bindings::drm_gpuva_op_map,
> +    // Since these abstractions are designed for immediate mode, the VM BO needs to be
> +    // pre-allocated, so we always have it available when we reach this point.
> +    vm_bo: &'op GpuVmBo<T>,
> +    _invariant: PhantomData<*mut &'op mut T>,
> +}
> +
> +impl<'op, T: DriverGpuVm> OpMap<'op, T> {
> +    /// The base address of the new mapping.
> +    pub fn addr(&self) -> u64 {
> +        self.op.va.addr
> +    }
> +
> +    /// The length of the new mapping.
> +    pub fn length(&self) -> u64 {
> +        self.op.va.range
> +    }
> +
> +    /// The offset within the [`drm_gem_object`](crate::gem::Object).
> +    pub fn gem_offset(&self) -> u64 {
> +        self.op.gem.offset
> +    }
> +
> +    /// The [`drm_gem_object`](crate::gem::Object) to map.
> +    pub fn obj(&self) -> &T::Object {
> +        // SAFETY: The `obj` pointer is guaranteed to be valid.
> +        unsafe { <T::Object as IntoGEMObject>::from_raw(self.op.gem.obj) }
> +    }
> +
> +    /// The [`GpuVmBo`] that the new VA will be associated with.
> +    pub fn vm_bo(&self) -> &GpuVmBo<T> {
> +        self.vm_bo
> +    }
> +
> +    /// Use the pre-allocated VA to carry out this map operation.
> +    pub fn insert(self, va: GpuVaAlloc<T>, va_data: impl PinInit<T::VaData>) -> OpMapped<'op, T> {
> +        let va = va.prepare(va_data);
> +        // SAFETY: By the type invariants we may access the interval tree.
> +        unsafe { bindings::drm_gpuva_map(self.vm_bo.gpuvm().as_raw(), va, self.op) };
> +
> +        let _gpuva_guard = self.vm_bo().lock_gpuva();
> +        // SAFETY: The va is prepared for insertion, and we hold the GEM lock.
> +        unsafe { bindings::drm_gpuva_link(va, self.vm_bo.as_raw()) };
> +
> +        OpMapped {
> +            _invariant: self._invariant,
> +        }
> +    }
> +}
> +
> +/// Represents a completed [`OpMap`] operation.
> +pub struct OpMapped<'op, T> {
> +    _invariant: PhantomData<*mut &'op mut T>,
> +}
> +
> /// Represents an `sm_step_unmap` operation that has not yet been completed.
> pub struct OpUnmap<'op, T: DriverGpuVm> {
>     op: &'op bindings::drm_gpuva_op_unmap,
> @@ -205,6 +299,30 @@ pub struct OpRemapped<'op, T> {
> }
> 
> impl<T: DriverGpuVm> GpuVmCore<T> {
> +    /// Create a mapping, removing or remapping anything that overlaps.
> +    ///
> +    /// Internally calls the [`DriverGpuVm`] callbacks similar to [`Self::sm_unmap`], except that
> +    /// the [`DriverGpuVm::sm_step_map`] is called once to create the requested mapping.
> +    #[inline]
> +    pub fn sm_map(&mut self, req: OpMapRequest<'_, '_, T>) -> Result {
> +        let gpuvm = self.as_raw();
> +        let raw_req = req.raw_request();
> +        let mut p = SmMapData {
> +            sm_data: SmData {
> +                gpuvm: self,
> +                user_context: req.context,
> +            },
> +            vm_bo: req.vm_bo,
> +        };
> +        // SAFETY:
> +        // * raw_request() creates a valid request.
> +        // * The private data is valid to be interpreted as both SmData and SmMapData since the
> +        //   first field of SmMapData is SmData.
> +        to_result(unsafe {
> +            bindings::drm_gpuvm_sm_map(gpuvm, (&raw mut p).cast(), &raw const raw_req)
> +        })
> +    }
> +
>     /// Remove any mappings in the given region.
>     ///
>     /// Internally calls [`DriverGpuVm::sm_step_unmap`] for ranges entirely contained within the
> @@ -218,19 +336,45 @@ pub fn sm_unmap(&mut self, addr: u64, length: u64, context: &mut T::SmContext<'_
>         };
>         // SAFETY:
>         // * raw_request() creates a valid request.
> -        // * The private data is valid to be interpreted as SmData.
> +        // * The private data is a valid SmData.
>         to_result(unsafe { bindings::drm_gpuvm_sm_unmap(gpuvm, (&raw mut p).cast(), addr, length) })
>     }
> }
> 
> impl<T: DriverGpuVm> GpuVm<T> {
>     /// # Safety
> -    /// Must be called from `sm_unmap` with a pointer to `SmData`.
> +    /// Must be called from `sm_map` with a pointer to `SmMapData`.
> +    pub(super) unsafe extern "C" fn sm_step_map(
> +        op: *mut bindings::drm_gpuva_op,
> +        p: *mut c_void,
> +    ) -> c_int {
> +        // SAFETY: If we reach `sm_step_map` then we were called from `sm_map` which always passes
> +        // an `SmMapData` as private data.
> +        let p = unsafe { &mut *p.cast::<SmMapData<'_, '_, T>>() };
> +        let op = OpMap {
> +            // SAFETY: sm_step_map is called with a map operation.
> +            op: unsafe { &(*op).__bindgen_anon_1.map },
> +            vm_bo: &p.vm_bo,
> +            _invariant: PhantomData,
> +        };
> +        match p
> +            .sm_data
> +            .gpuvm
> +            .data()
> +            .sm_step_map(op, p.sm_data.user_context)
> +        {
> +            Ok(OpMapped { .. }) => 0,
> +            Err(err) => err.to_errno(),
> +        }
> +    }
> +
> +    /// # Safety
> +    /// Must be called from `sm_map` or `sm_unmap` with a pointer to `SmMapData` or `SmData`.
>     pub(super) unsafe extern "C" fn sm_step_unmap(
>         op: *mut bindings::drm_gpuva_op,
>         p: *mut c_void,
>     ) -> c_int {
> -        // SAFETY: The caller provides a pointer to `SmData`.
> +        // SAFETY: The caller provides a pointer that can be treated as `SmData`.
>         let p = unsafe { &mut *p.cast::<SmData<'_, '_, T>>() };
>         let op = OpUnmap {
>             // SAFETY: sm_step_unmap is called with an unmap operation.
> @@ -244,12 +388,12 @@ impl<T: DriverGpuVm> GpuVm<T> {
>     }
> 
>     /// # Safety
> -    /// Must be called from `sm_unmap` with a pointer to `SmData`.
> +    /// Must be called from `sm_map` or `sm_unmap` with a pointer to `SmMapData` or `SmData`.
>     pub(super) unsafe extern "C" fn sm_step_remap(
>         op: *mut bindings::drm_gpuva_op,
>         p: *mut c_void,
>     ) -> c_int {
> -        // SAFETY: The caller provides a pointer to `SmData`.
> +        // SAFETY: The caller provides a pointer that can be treated as `SmData`.
>         let p = unsafe { &mut *p.cast::<SmData<'_, '_, T>>() };
>         let op = OpRemap {
>             // SAFETY: sm_step_remap is called with a remap operation.
> 
> -- 
> 2.52.0.457.g6b5491de43-goog
> 

Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 5/6] rust: gpuvm: add GpuVmCore::sm_unmap()'
  2026-01-22 22:43   ` Daniel Almeida
@ 2026-01-23  9:23     ` Alice Ryhl
  0 siblings, 0 replies; 22+ messages in thread
From: Alice Ryhl @ 2026-01-23  9:23 UTC (permalink / raw)
  To: Daniel Almeida
  Cc: Danilo Krummrich, Boris Brezillon, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux

On Thu, Jan 22, 2026 at 07:43:11PM -0300, Daniel Almeida wrote:
> Hi Alice,
> 
> > On 21 Jan 2026, at 08:31, Alice Ryhl <aliceryhl@google.com> wrote:
> > +/// Represents an `sm_step_unmap` operation that has not yet been completed.
> > +pub struct OpUnmap<'op, T: DriverGpuVm> {
> > +    op: &'op bindings::drm_gpuva_op_unmap,
> > +    _invariant: PhantomData<*mut &'op mut T>,
> 
> Would have been cool to explain why we have a pointer in this PhantomData.
> 
> Same elsewhere, IMHO. Helps with maintainability in the future.
> 
> (To be honest, I’m not really sure what’s going on here..)

Normally, when you have an OpUnmap<'long, T> Rust will let you convert
that into an OpUnmap<'short, T>, but I don't want that in this case.
Making such coercions impossible means that callers of sm_step_unmap()
cannot return the "wrong" OpUnmapped from the closure because the only
way to get an OpUnmapped with the right lifetime is to call remove() on
the OpUnmap you received.

(Otherwise, it may be possible to return an OpUnmapped from one
sm_step_unmap() call in another sm_step_unmap() call.)

There are various different types one can place in PhantomData to have
this effect. A mutable pointer is one choice. I could also have used:

	PhantomData<fn(&'op mut T) -> &'op mut T>

or a few other options.

Alice

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 6/6] rust: gpuvm: add GpuVmCore::sm_map()
  2026-01-22 22:58   ` Daniel Almeida
@ 2026-01-23  9:23     ` Alice Ryhl
  0 siblings, 0 replies; 22+ messages in thread
From: Alice Ryhl @ 2026-01-23  9:23 UTC (permalink / raw)
  To: Daniel Almeida
  Cc: Danilo Krummrich, Boris Brezillon, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux

On Thu, Jan 22, 2026 at 07:58:34PM -0300, Daniel Almeida wrote:
> Hi Alice,
> 
> > On 21 Jan 2026, at 08:31, Alice Ryhl <aliceryhl@google.com> wrote:
> > 
> > Finally also add the operation for creating new mappings. Mapping
> > operations need extra data in the context since they involve a vm_bo
> > coming from the outside.
> > 
> > Co-developed-by: Asahi Lina <lina+kernel@asahilina.net>
> > Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> > rust/kernel/drm/gpuvm/mod.rs    |   9 ++-
> > rust/kernel/drm/gpuvm/sm_ops.rs | 154 ++++++++++++++++++++++++++++++++++++++--
> > 2 files changed, 157 insertions(+), 6 deletions(-)
> > 
> > diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs
> > index 165a25666ccc3d62e59b73483d4eedff044423e9..557c0d629eec912a97fc4ef18495d5bf0807db0a 100644
> > --- a/rust/kernel/drm/gpuvm/mod.rs
> > +++ b/rust/kernel/drm/gpuvm/mod.rs
> > @@ -93,7 +93,7 @@ const fn vtable() -> &'static bindings::drm_gpuvm_ops {
> >             vm_bo_alloc: GpuVmBo::<T>::ALLOC_FN,
> >             vm_bo_free: GpuVmBo::<T>::FREE_FN,
> >             vm_bo_validate: None,
> > -            sm_step_map: None,
> > +            sm_step_map: Some(Self::sm_step_map),
> >             sm_step_unmap: Some(Self::sm_step_unmap),
> >             sm_step_remap: Some(Self::sm_step_remap),
> >         }
> > @@ -248,6 +248,13 @@ pub trait DriverGpuVm: Sized {
> >     /// The private data passed to callbacks.
> >     type SmContext<'ctx>;
> > 
> > +    /// Indicates that a new mapping should be created.
> > +    fn sm_step_map<'op, 'ctx>(
> > +        &mut self,
> > +        op: OpMap<'op, Self>,
> > +        context: &mut Self::SmContext<'ctx>,
> > +    ) -> Result<OpMapped<'op, Self>, Error>;
> > +
> >     /// Indicates that an existing mapping should be removed.
> >     fn sm_step_unmap<'op, 'ctx>(
> >         &mut self,
> > diff --git a/rust/kernel/drm/gpuvm/sm_ops.rs b/rust/kernel/drm/gpuvm/sm_ops.rs
> > index 3c29d10d63f0b0a1976c714a86d486948ba81a15..5f3c5d3918147a6962e5658443c343835baa10b8 100644
> > --- a/rust/kernel/drm/gpuvm/sm_ops.rs
> > +++ b/rust/kernel/drm/gpuvm/sm_ops.rs
> > @@ -8,6 +8,100 @@ struct SmData<'a, 'ctx, T: DriverGpuVm> {
> >     user_context: &'a mut T::SmContext<'ctx>,
> > }
> > 
> > +#[repr(C)]
> > +struct SmMapData<'a, 'ctx, T: DriverGpuVm> {
> > +    sm_data: SmData<'a, 'ctx, T>,
> > +    vm_bo: GpuVmBoResident<T>,
> > +}
> > +
> > +/// The argument for [`GpuVmCore::sm_map`].
> > +pub struct OpMapRequest<'a, 'ctx, T: DriverGpuVm> {
> > +    /// Address in GPU virtual address space.
> > +    pub addr: u64,
> > +    /// Length of mapping to create.
> > +    pub range: u64,
> > +    /// Offset in GEM object.
> > +    pub offset: u64,
> 
> I’d rename this gem_offset. A bit vague/confusing otherwise.

Sure. I'll rename.

Alice

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 3/6] rust: gpuvm: add GpuVm::obtain()
  2026-01-21 11:31 ` [PATCH v3 3/6] rust: gpuvm: add GpuVm::obtain() Alice Ryhl
  2026-01-21 17:31   ` Daniel Almeida
@ 2026-01-26 15:00   ` Boris Brezillon
  2026-01-26 15:07     ` Alice Ryhl
  1 sibling, 1 reply; 22+ messages in thread
From: Boris Brezillon @ 2026-01-26 15:00 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Danilo Krummrich, Daniel Almeida, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux

On Wed, 21 Jan 2026 11:31:19 +0000
Alice Ryhl <aliceryhl@google.com> wrote:

> +/// A [`GpuVmBo`] object in the GEM list.
> +///
> +/// # Invariants
> +///
> +/// Points at a `drm_gpuvm_bo` that contains a valid `T::VmBoData` and is present in the gem list.
> +pub struct GpuVmBoResident<T: DriverGpuVm>(NonNull<GpuVmBo<T>>);

I find the name a bit confusing: BO residency is often used to refer to
memory backing the buffer object, and in this case, you can end up with
a GpuVmBoResident being returned for a BO that has been evicted (one
that's no longer resident).

> +
> +impl<T: DriverGpuVm> GpuVmBoResident<T> {
> +    /// Returns a raw pointer to underlying C value.
> +    #[inline]
> +    pub fn as_raw(&self) -> *mut bindings::drm_gpuvm_bo {
> +        // SAFETY: The pointer references a valid `drm_gpuvm_bo`.
> +        unsafe { (*self.0.as_ptr()).inner.get() }
> +    }
> +}

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 3/6] rust: gpuvm: add GpuVm::obtain()
  2026-01-26 15:00   ` Boris Brezillon
@ 2026-01-26 15:07     ` Alice Ryhl
  2026-01-26 15:35       ` Boris Brezillon
  0 siblings, 1 reply; 22+ messages in thread
From: Alice Ryhl @ 2026-01-26 15:07 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Danilo Krummrich, Daniel Almeida, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux

On Mon, Jan 26, 2026 at 4:00 PM Boris Brezillon
<boris.brezillon@collabora.com> wrote:
>
> On Wed, 21 Jan 2026 11:31:19 +0000
> Alice Ryhl <aliceryhl@google.com> wrote:
>
> > +/// A [`GpuVmBo`] object in the GEM list.
> > +///
> > +/// # Invariants
> > +///
> > +/// Points at a `drm_gpuvm_bo` that contains a valid `T::VmBoData` and is present in the gem list.
> > +pub struct GpuVmBoResident<T: DriverGpuVm>(NonNull<GpuVmBo<T>>);
>
> I find the name a bit confusing: BO residency is often used to refer to
> memory backing the buffer object, and in this case, you can end up with
> a GpuVmBoResident being returned for a BO that has been evicted (one
> that's no longer resident).

Good point. I meant it as "present in list" but I guess there are
other things a gpuvm may be present in.

Any naming suggestions?

Alice

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 3/6] rust: gpuvm: add GpuVm::obtain()
  2026-01-26 15:07     ` Alice Ryhl
@ 2026-01-26 15:35       ` Boris Brezillon
  2026-01-26 16:44         ` Danilo Krummrich
  0 siblings, 1 reply; 22+ messages in thread
From: Boris Brezillon @ 2026-01-26 15:35 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Danilo Krummrich, Daniel Almeida, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux

On Mon, 26 Jan 2026 16:07:30 +0100
Alice Ryhl <aliceryhl@google.com> wrote:

> On Mon, Jan 26, 2026 at 4:00 PM Boris Brezillon
> <boris.brezillon@collabora.com> wrote:
> >
> > On Wed, 21 Jan 2026 11:31:19 +0000
> > Alice Ryhl <aliceryhl@google.com> wrote:
> >  
> > > +/// A [`GpuVmBo`] object in the GEM list.
> > > +///
> > > +/// # Invariants
> > > +///
> > > +/// Points at a `drm_gpuvm_bo` that contains a valid `T::VmBoData` and is present in the gem list.
> > > +pub struct GpuVmBoResident<T: DriverGpuVm>(NonNull<GpuVmBo<T>>);  
> >
> > I find the name a bit confusing: BO residency is often used to refer to
> > memory backing the buffer object, and in this case, you can end up with
> > a GpuVmBoResident being returned for a BO that has been evicted (one
> > that's no longer resident).  
> 
> Good point. I meant it as "present in list" but I guess there are
> other things a gpuvm may be present in.
> 
> Any naming suggestions?

Valid, Bound, Present, Active?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 3/6] rust: gpuvm: add GpuVm::obtain()
  2026-01-26 15:35       ` Boris Brezillon
@ 2026-01-26 16:44         ` Danilo Krummrich
  0 siblings, 0 replies; 22+ messages in thread
From: Danilo Krummrich @ 2026-01-26 16:44 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Alice Ryhl, Daniel Almeida, Janne Grunau, Matthew Brost,
	Thomas Hellström, Lyude Paul, Asahi Lina, dri-devel,
	linux-kernel, rust-for-linux

On Mon Jan 26, 2026 at 4:35 PM CET, Boris Brezillon wrote:
> On Mon, 26 Jan 2026 16:07:30 +0100
> Alice Ryhl <aliceryhl@google.com> wrote:
>
>> On Mon, Jan 26, 2026 at 4:00 PM Boris Brezillon
>> <boris.brezillon@collabora.com> wrote:
>> >
>> > On Wed, 21 Jan 2026 11:31:19 +0000
>> > Alice Ryhl <aliceryhl@google.com> wrote:
>> >  
>> > > +/// A [`GpuVmBo`] object in the GEM list.
>> > > +///
>> > > +/// # Invariants
>> > > +///
>> > > +/// Points at a `drm_gpuvm_bo` that contains a valid `T::VmBoData` and is present in the gem list.
>> > > +pub struct GpuVmBoResident<T: DriverGpuVm>(NonNull<GpuVmBo<T>>);  
>> >
>> > I find the name a bit confusing: BO residency is often used to refer to
>> > memory backing the buffer object, and in this case, you can end up with
>> > a GpuVmBoResident being returned for a BO that has been evicted (one
>> > that's no longer resident).  
>> 
>> Good point. I meant it as "present in list" but I guess there are
>> other things a gpuvm may be present in.
>> 
>> Any naming suggestions?
>
> Valid, Bound, Present, Active?

I still have to catch up on this series, but quick drive-by comment: I'd go for
'Registered'.

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2026-01-26 16:44 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-21 11:31 [PATCH v3 0/6] Rust GPUVM immediate mode Alice Ryhl
2026-01-21 11:31 ` [PATCH v3 1/6] rust: drm: add base GPUVM immediate mode abstraction Alice Ryhl
2026-01-21 17:04   ` Daniel Almeida
2026-01-22  9:23     ` Alice Ryhl
2026-01-21 11:31 ` [PATCH v3 2/6] rust: helpers: Add bindings/wrappers for dma_resv_lock Alice Ryhl
2026-01-21 12:39   ` Daniel Almeida
2026-01-21 15:44   ` Gary Guo
2026-01-21 11:31 ` [PATCH v3 3/6] rust: gpuvm: add GpuVm::obtain() Alice Ryhl
2026-01-21 17:31   ` Daniel Almeida
2026-01-22  8:20     ` Alice Ryhl
2026-01-26 15:00   ` Boris Brezillon
2026-01-26 15:07     ` Alice Ryhl
2026-01-26 15:35       ` Boris Brezillon
2026-01-26 16:44         ` Danilo Krummrich
2026-01-21 11:31 ` [PATCH v3 4/6] rust: gpuvm: add GpuVa struct Alice Ryhl
2026-01-22 22:09   ` Daniel Almeida
2026-01-21 11:31 ` [PATCH v3 5/6] rust: gpuvm: add GpuVmCore::sm_unmap() Alice Ryhl
2026-01-22 22:43   ` Daniel Almeida
2026-01-23  9:23     ` [PATCH v3 5/6] rust: gpuvm: add GpuVmCore::sm_unmap()' Alice Ryhl
2026-01-21 11:31 ` [PATCH v3 6/6] rust: gpuvm: add GpuVmCore::sm_map() Alice Ryhl
2026-01-22 22:58   ` Daniel Almeida
2026-01-23  9:23     ` Alice Ryhl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox