On Fri Jan 30, 2026 at 3:24 PM CET, Alice Ryhl wrote:
> +/// 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.
I think this is still a bit confusing, I think we should just rename GpuVmCore
to UniqueGpuVm and rewrite this to something like:
"The driver specific data of of `GpuVm` is only accessible through a
[`UniqueGpuVm`], which guarantees exclusive access."
> +/// # 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,
Let's be flexibile and also accept an impl PinInit<T, E> instead.
> + ) -> 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) }
I'd pull the Opaque::cast_from() call out of the unsafe block.
> + }
> +
> + /// 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.
This description seems a bit odd. In the end, the trait makes GPUVM aware of
other driver specific types. So, maybe a better name would be
gpuvm::DriverAttributes, gpuvm::DriverTypes, gpuvm::DriverInfo or just
gpuvm::Driver. My favorite is gpuvm::DriverInfo.
We should also change the doc-comment accordingly. Maybe somthing like: "This
trait make the [`GpuVm`] aware of the other driver specific DRM types."
> +pub trait DriverGpuVm: Sized {
> + /// Parent `Driver` for this object.
> + type Driver: drm::Driver<Object = Self::Object>;
> +
> + /// The kind of GEM object stored in this GPUVM.
> + type Object: IntoGEMObject;
> +}
