From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E728333FE27; Thu, 19 Feb 2026 14:36:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771511785; cv=none; b=hFHiykhw9KSjGJiAB467S4+MaPqZvPPCd/iPhzNcHfxtnBvbSPDCHtTXuqrPnsnQB9iF2vMxcKHaHWmgMNG+IHgg3yC4XsWhl742U7tMwMnOfLUxw5z+3FyikXmCY1yonz3VlctrmJOn0ksntF0dVs7VOAx0WSVErIbKN5uhdow= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771511785; c=relaxed/simple; bh=U2h2fXNq15bonU/KsOObw8GNtWEaVPPYqkxrii9vizc=; h=Mime-Version:Content-Type:Date:Message-Id:To:From:Subject:Cc: References:In-Reply-To; b=r+VscEYhXSeYNuoDm24JqAe0sqfxsmmTgZ2beYQuZub31TLP3aTCIv19YxJZdhCOgxUhUpUyzV1lTKFId5NAAXl1MLSXZ2/ucTZZj0kUoS/yORnh9RSMb77n+iY59FZ74FRU7sop3slug5/6yAlB3cVpxiexoqF5obvK34m6PQs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=XZNNiI6R; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="XZNNiI6R" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 78253C4CEF7; Thu, 19 Feb 2026 14:36:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1771511784; bh=U2h2fXNq15bonU/KsOObw8GNtWEaVPPYqkxrii9vizc=; h=Date:To:From:Subject:Cc:References:In-Reply-To:From; b=XZNNiI6Rz6imH5O12Lk7KKFRXfS2dFCRKz2nUJDHS9/3PiSgqvtlz6Pjlywmx7cgM msZhhuGCsA5jBO7aD58zJIgVRn2jFYZjm1Gu1WOtjAVc77mTdTLt6m/pZuFlI3JO2X M0QHBcExPY+g6gftvCqT0gbbrAlT2AQJe92M/tz68mpsMteBORDyzKk4bEfREmqNcB EDrt19LHu3b1/DCsgwGY8zptjGlbKJKljfidY7BZjsHK9LCaHpfr/IhFH67meJkxxU t7m2go690IBO/U1h8FPWWvh5x8jdFmlBY0OEFKavmioys8NDrI0OHGQMfbgH7o6/72 tPSChGf39Np8A== Precedence: bulk X-Mailing-List: rust-for-linux@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Thu, 19 Feb 2026 15:36:20 +0100 Message-Id: To: "Alice Ryhl" From: "Danilo Krummrich" Subject: Re: [PATCH v4 1/6] rust: drm: add base GPUVM immediate mode abstraction Cc: "Daniel Almeida" , "Boris Brezillon" , "Janne Grunau" , "Matthew Brost" , =?utf-8?q?Thomas_Hellstr=C3=B6m?= , "Lyude Paul" , "Asahi Lina" , , , References: <20260130-gpuvm-rust-v4-0-8364d104ff40@google.com> <20260130-gpuvm-rust-v4-1-8364d104ff40@google.com> In-Reply-To: <20260130-gpuvm-rust-v4-1-8364d104ff40@google.com> 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 GpuVmC= ore 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 { > + #[pin] > + vm: Opaque, > + /// Accessed only through the [`GpuVmCore`] reference. > + data: UnsafeCell, > +} > + > +// SAFETY: By type invariants, the allocation is managed by the refcount= in `self.vm`. > +unsafe impl AlwaysRefCounted for GpuVm { > + 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) { > + // 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 GpuVm { > + 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( > + name: &'static CStr, > + dev: &drm::Device, > + r_obj: &T::Object, > + range: Range, > + reserve_range: Range, > + data: T, Let's be flexibile and also accept an impl PinInit instead. > + ) -> Result, E> > + where > + E: From, > + E: From, > + { > + let obj =3D KBox::try_pin_init::( > + 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_IMMEDIAT= E_MODE > + | bindings::drm_gpuvm_flags_DRM_GPUVM_RE= SV_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 [`G= puVm`] that remains > + /// valid for at least `'a`. > + #[inline] > + pub unsafe fn from_raw<'a>(ptr: *mut bindings::drm_gpuvm) -> &'a Sel= f { > + // SAFETY: Caller passes a pointer to the `drm_gpuvm` in a `GpuV= m`. Caller ensures the > + // pointer is valid for 'a. > + unsafe { &*kernel::container_of!(Opaque::cast_from(ptr), Self, v= m) } 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 { > + let start =3D self.va_start(); > + // OVERFLOW: This reconstructs the Range passed to the cons= tructor, so it won't fail. > + let end =3D 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 `GpuV= m`. > + let me =3D 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 o= f 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: "Th= is 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; > + > + /// The kind of GEM object stored in this GPUVM. > + type Object: IntoGEMObject; > +}