Re: [PATCH v4 1/6] rust: drm: add base GPUVM immediate mode abstraction

[Alice Ryhl <aliceryhl@google.com>]

On Thu, Feb 19, 2026 at 03:36:20PM +0100, Danilo Krummrich wrote:
> 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."

But it's not really the same as e.g. UniqueArc<_>, which implies that
there are no normal Arc<_>s whatsoever. This one is just a special ref,
but there may also be normal refs at the same time.

> > +    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.

I think that's a bit verbose, and all existing uses do it inside the
`container_of!`.

> > +/// 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."

I mean, it doesn't just do that. The type implementing this is the type
stored in the `core` field, so you really do get more than just some
type relationships from it.

Alice

> > +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;
> > +}