From nobody Fri Dec 19 21:47:10 2025 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (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 39083302772 for ; Fri, 7 Nov 2025 19:32:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1762543950; cv=none; b=mq5UmQLCFmzkqBhEahMm0y+YLJIosipu2IQvktso16Mk1Wy0O+hdw0Vjcy+W/wQyLq1jRfi/HsXf3gUF7Z+N4aqiEcPZt1jg8/RLOmTMX1vpkvjz8gCWumPyksymOTIIKnxTd+UAvQm8l3QuRdXNcfav/H99tdgLuL99vSrNIIE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1762543950; c=relaxed/simple; bh=hpYfuChdBxZ8aIvPC01mbDDCUm6HB8Cp304N2wi/dtU=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=OOnuRHP9WAz7LYQE+KwCm6QQDl9EFCYeo8Rp2RkQmFRnoTeXXzhemVrT5T0gFjgiwdoN75KqlpxpPuN95Vq/wj3VyKj59vnOu3yFCYL+AZ4rBUXYgQ/l6nmadsCyAv72mZWTw6q+yuJiZEnAsnuouXiJyOVT0yloWXsATymoX0U= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=Vi+8WU9+; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="Vi+8WU9+" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1762543946; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=2w3HBCTCnyeGOWO+4tHcNTI3KzfddOfLp9dQjNEQkmE=; b=Vi+8WU9+MMKhlDZpUx0nWwl7oODZcgt9i048CI8grZ542VKNirc3EbaJKqn3qKyEotapkD asGOxTHCs2uHsnhvV4E9nwGnUq4UpW/r3aEvrvp7qJuzzWPJbBn5B0E7t6uQwOXGxqMJbs PrEIs5Wdb/Zcn8lGZUVNycJqESlif3k= Received: from mx-prod-mc-06.mail-002.prod.us-west-2.aws.redhat.com (ec2-35-165-154-97.us-west-2.compute.amazonaws.com [35.165.154.97]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-404-MGuTFIVJOt-aQPn_B6JBGw-1; Fri, 07 Nov 2025 14:32:23 -0500 X-MC-Unique: MGuTFIVJOt-aQPn_B6JBGw-1 X-Mimecast-MFC-AGG-ID: MGuTFIVJOt-aQPn_B6JBGw_1762543940 Received: from mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.17]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-06.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 7DCAA180028A; Fri, 7 Nov 2025 19:32:20 +0000 (UTC) Received: from chopper.lan (unknown [10.22.81.9]) by mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id B420C1945110; Fri, 7 Nov 2025 19:32:15 +0000 (UTC) From: Lyude Paul To: linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org, dri-devel@lists.freedesktop.org Cc: Danilo Krummrich , Alice Ryhl , David Airlie , Simona Vetter , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , =?UTF-8?q?Bj=C3=B6rn=20Roy=20Baron?= , Benno Lossin , Andreas Hindborg , Trevor Gross , nouveau@lists.freedesktop.org (open list:DRM DRIVER FOR NVIDIA GPUS [RUST]) Subject: [PATCH 1/2] rust: drm: Introduce DeviceCtx Date: Fri, 7 Nov 2025 14:23:53 -0500 Message-ID: <20251107193204.398657-2-lyude@redhat.com> In-Reply-To: <20251107193204.398657-1-lyude@redhat.com> References: <20251107193204.398657-1-lyude@redhat.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Scanned-By: MIMEDefang 3.0 on 10.30.177.17 Content-Type: text/plain; charset="utf-8" One of the tricky things about DRM bindings in Rust is the fact that initialization of a DRM device is a multi-step process. It's quite normal for a device driver to start making use of its DRM device for tasks like creating GEM objects before userspace registration happens. This is an issue in rust though, since prior to userspace registration the device is only partly initialized. This means there's a plethora of DRM device operations we can't yet expose without opening up the door to UB if the DRM device in question isn't yet registered. Additionally, this isn't something we can reliably check at runtime. And even if we could, performing an operation which requires the device be registered when the device isn't actually registered is a programmer bug, meaning there's no real way to gracefully handle such a mistake at runtime. And even if that wasn't the case, it would be horrendously annoying and noisy to have to check if a device is registered constantly throughout a driver. In order to solve this, we first take inspiration from `kernel::device::DeviceCtx` and introduce `kernel::drm::DeviceCtx`. This provides us with a ZST type that we can generalize over to represent contexts where a device is known to have been registered with userspace at some point in time (`Registered`), along with contexts where we can't make such a guarantee (`AnyCtx`). It's important to note we intentionally do not provide a `DeviceCtx` which represents an unregistered device. This is because there's no reasonable way to guarantee that a device with long-living references to itself will not be registered eventually with userspace. Instead, we provide a new-type for this: `UnregisteredDevice` which can provide a guarantee that the `Device` has never been registered with userspace. To ensure this, we modify `Registration` so that creating a new `Registration` requires passing ownership of an `UnregisteredDevice`. Signed-off-by: Lyude Paul --- drivers/gpu/drm/nova/driver.rs | 8 +- drivers/gpu/drm/tyr/driver.rs | 13 ++- rust/kernel/drm/device.rs | 167 +++++++++++++++++++++++++++------ rust/kernel/drm/driver.rs | 35 +++++-- rust/kernel/drm/mod.rs | 4 + 5 files changed, 181 insertions(+), 46 deletions(-) diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs index 91b7380f83ab4..c78d69d5f045a 100644 --- a/drivers/gpu/drm/nova/driver.rs +++ b/drivers/gpu/drm/nova/driver.rs @@ -13,7 +13,7 @@ pub(crate) struct NovaDriver { } =20 /// Convienence type alias for the DRM device type for this driver -pub(crate) type NovaDevice =3D drm::Device; +pub(crate) type NovaDevice =3D drm::Device; =20 #[pin_data] pub(crate) struct NovaData { @@ -48,10 +48,10 @@ impl auxiliary::Driver for NovaDriver { fn probe(adev: &auxiliary::Device, _info: &Self::IdInfo) -> Resu= lt>> { let data =3D try_pin_init!(NovaData { adev: adev.into() }); =20 - let drm =3D drm::Device::::new(adev.as_ref(), data)?; - drm::Registration::new_foreign_owned(&drm, adev.as_ref(), 0)?; + let drm =3D drm::UnregisteredDevice::::new(adev.as_ref(), da= ta)?; + let drm =3D drm::Registration::new_foreign_owned(drm, adev.as_ref(= ), 0)?; =20 - Ok(KBox::new(Self { drm }, GFP_KERNEL)?.into()) + Ok(KBox::new(Self { drm: drm.into() }, GFP_KERNEL)?.into()) } } =20 diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs index d5625dd1e41c8..e3ea5ad85f49b 100644 --- a/drivers/gpu/drm/tyr/driver.rs +++ b/drivers/gpu/drm/tyr/driver.rs @@ -30,7 +30,7 @@ pub(crate) type IoMem =3D kernel::io::mem::IoMem; =20 /// Convenience type alias for the DRM device type for this driver. -pub(crate) type TyrDevice =3D drm::Device; +pub(crate) type TyrDevice =3D drm::Device; =20 #[pin_data(PinnedDrop)] pub(crate) struct TyrDriver { @@ -140,10 +140,15 @@ fn probe( gpu_info, }); =20 - let tdev: ARef =3D drm::Device::new(pdev.as_ref(), data= )?; - drm::driver::Registration::new_foreign_owned(&tdev, pdev.as_ref(),= 0)?; + let tdev =3D drm::UnregisteredDevice::::new(pdev.as_ref(), d= ata)?; + let tdev =3D drm::driver::Registration::new_foreign_owned(tdev, pd= ev.as_ref(), 0)?; =20 - let driver =3D KBox::pin_init(try_pin_init!(TyrDriver { device: td= ev }), GFP_KERNEL)?; + let driver =3D KBox::pin_init( + try_pin_init!(TyrDriver { + device: tdev.into() + }), + GFP_KERNEL, + )?; =20 // We need this to be dev_info!() because dev_dbg!() does not work= at // all in Rust for now, and we need to see whether probe succeeded. diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 3ce8f62a00569..00072984930a3 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -7,14 +7,14 @@ use crate::{ alloc::allocator::Kmalloc, bindings, device, drm, - drm::driver::AllocImpl, + drm::{driver::AllocImpl, private::Sealed}, error::from_err_ptr, error::Result, prelude::*, sync::aref::{ARef, AlwaysRefCounted}, types::Opaque, }; -use core::{alloc::Layout, mem, ops::Deref, ptr, ptr::NonNull}; +use core::{alloc::Layout, marker::PhantomData, mem, ops::Deref, ptr, ptr::= NonNull}; =20 #[cfg(CONFIG_DRM_LEGACY)] macro_rules! drm_legacy_fields { @@ -47,26 +47,83 @@ macro_rules! drm_legacy_fields { } } =20 -/// A typed DRM device with a specific `drm::Driver` implementation. +macro_rules! drm_dev_ctx { + ( + $( #[$attrs:meta] )* + $name:ident + ) =3D> { + $( #[$attrs] )* + pub struct $name; + + impl DeviceCtx for $name {} + impl Sealed for $name {} + + // SAFETY: All registration states are free of side-effects (e.g. = no Drop) and are ZSTs, + // thus they are always thread-safe. + unsafe impl Send for $name {} + // SAFETY: All registration states are free of side-effects (e.g. = no Drop) and are ZSTs, + // thus they are always thread-safe. + unsafe impl Sync for $name {} + }; +} + +/// A trait implemented by all possible contexts a [`Device`] can be used = in. +pub trait DeviceCtx: Sealed + Send + Sync {} + +drm_dev_ctx! { + /// The [`DeviceCtx`] of a [`Device`] that was registered with userspa= ce at some point. + /// + /// This represents a [`Device`] which is guaranteed to have been regi= stered with userspace at + /// some point in time. Once a DRM device has registered itself with u= serspace, it is safe to + /// assume that the initialization process for the DRM device has comp= leted. + /// + /// Note: A device in this context is not guaranteed to remain registe= red with userspace for its + /// entire lifetime, as this is impossible to guarantee at compile-tim= e. However, any + /// userspace-dependent operations performed with an unregistered devi= ce in this [`DeviceCtx`] + /// are guaranteed to be no-ops. + /// + /// # Invariants + /// + /// A [`Device`] in this [`DeviceCtx`] is guaranteed to have called `d= rm_dev_register` once. + Registered +} + +drm_dev_ctx! { + /// The [`DeviceCtx`] of a [`Device`] that has no initialization or re= gistration guarantees. + /// + /// A [`Device`] in this context could be in any state, pre or post re= gistration, and is only + /// guaranteed to be minimally initialized. As such the operations whi= ch may be performed on + /// such [`Device`]s are more limited then in the [`Registered`] conte= xt. + AnyCtx +} + +/// A [`Device`] which is known at compile-time to be unregistered with us= erspace. /// -/// The device is always reference-counted. +/// This type allows performing operations which are only safe to do befor= e userspace registration, +/// and can be used to create a [`Registration`](drm::driver::Registration= ) once the driver is ready +/// to register the device with userspace. /// /// # Invariants /// -/// `self.dev` is a valid instance of a `struct device`. -#[repr(C)] -pub struct Device { - dev: Opaque, - data: T::Data, +/// The device in `self.0` is guaranteed to be a newly created [`Device`] = that has not yet been +/// registered with userspace until this type is dropped. +pub struct UnregisteredDevice(ARef>); + +impl Deref for UnregisteredDevice { + type Target =3D Device; + + fn deref(&self) -> &Self::Target { + &self.0 + } } =20 -impl Device { +impl UnregisteredDevice { const VTABLE: bindings::drm_driver =3D drm_legacy_fields! { load: None, open: Some(drm::File::::open_callback), postclose: Some(drm::File::::postclose_callback), unload: None, - release: Some(Self::release), + release: Some(Device::::release), master_set: None, master_drop: None, debugfs_init: None, @@ -94,8 +151,10 @@ impl Device { =20 const GEM_FOPS: bindings::file_operations =3D drm::gem::create_fops(); =20 - /// Create a new `drm::Device` for a `drm::Driver`. - pub fn new(dev: &device::Device, data: impl PinInit) -= > Result> { + /// Create a new `UnregisteredDevice` for a `drm::Driver`. + /// + /// This can be used to create a [`Registration`](kernel::drm::Registr= ation). + pub fn new(dev: &device::Device, data: impl PinInit) -= > Result { // `__drm_dev_alloc` uses `kmalloc()` to allocate memory, hence en= sure a `kmalloc()` // compatible `Layout`. let layout =3D Kmalloc::aligned_layout(Layout::new::()); @@ -103,12 +162,12 @@ pub fn new(dev: &device::Device, data: impl PinInit) -> Result =3D unsafe { bindings::__drm_dev_alloc( dev.as_raw(), &Self::VTABLE, layout.size(), - mem::offset_of!(Self, dev), + mem::offset_of!(Device, dev), ) } .cast(); @@ -123,7 +182,7 @@ pub fn new(dev: &device::Device, data: impl PinInit) -> Result) -> Result { + dev: Opaque, + data: T::Data, + _ctx: PhantomData, +} =20 +impl Device { pub(crate) fn as_raw(&self) -> *mut bindings::drm_device { self.dev.get() } @@ -160,13 +250,13 @@ unsafe fn into_drm_device(ptr: NonNull) -> *mut= bindings::drm_device { /// /// # Safety /// - /// Callers must ensure that `ptr` is valid, non-null, and has a non-z= ero reference count, - /// i.e. it must be ensured that the reference count of the C `struct = drm_device` `ptr` points - /// to can't drop to zero, for the duration of this function call and = the entire duration when - /// the returned reference exists. - /// - /// Additionally, callers must ensure that the `struct device`, `ptr` = is pointing to, is - /// embedded in `Self`. + /// * Callers must ensure that `ptr` is valid, non-null, and has a non= -zero reference count, + /// i.e. it must be ensured that the reference count of the C `struc= t drm_device` `ptr` points + /// to can't drop to zero, for the duration of this function call an= d the entire duration when + /// the returned reference exists. + /// * Additionally, callers must ensure that the `struct device`, `ptr= ` is pointing to, is + /// embedded in `Self`. + /// * Callers promise that any type invariants of `Ctx` will be upheld. #[doc(hidden)] pub unsafe fn from_raw<'a>(ptr: *const bindings::drm_device) -> &'a Se= lf { // SAFETY: By the safety requirements of this function `ptr` is a = valid pointer to a @@ -188,7 +278,26 @@ extern "C" fn release(ptr: *mut bindings::drm_device) { } } =20 -impl Deref for Device { +impl Device { + /// Assume the device has been fully initialized and registered with u= serspace. + /// + /// # Safety + /// + /// The caller promises that `drm_dev_register` has been called on thi= s device. + pub(crate) unsafe fn assume_registered(&self) -> &Device { + // SAFETY: The data layout is identical via our type invariants. + unsafe { mem::transmute(self) } + } +} + +impl AsRef> for Device { + fn as_ref(&self) -> &Device { + // SAFETY: The data layout is identical via our type invariants. + unsafe { mem::transmute(self) } + } +} + +impl Deref for Device { type Target =3D T::Data; =20 fn deref(&self) -> &Self::Target { @@ -198,7 +307,7 @@ fn deref(&self) -> &Self::Target { =20 // SAFETY: DRM device objects are always reference counted and the get/put= functions // satisfy the requirements. -unsafe impl AlwaysRefCounted for Device { +unsafe impl AlwaysRefCounted for Device { fn inc_ref(&self) { // SAFETY: The existence of a shared reference guarantees that the= refcount is non-zero. unsafe { bindings::drm_dev_get(self.as_raw()) }; @@ -213,7 +322,7 @@ unsafe fn dec_ref(obj: NonNull) { } } =20 -impl AsRef for Device { +impl AsRef for Device { fn as_ref(&self) -> &device::Device { // SAFETY: `bindings::drm_device::dev` is valid as long as the DRM= device itself is valid, // which is guaranteed by the type invariant. @@ -222,8 +331,8 @@ fn as_ref(&self) -> &device::Device { } =20 // SAFETY: A `drm::Device` can be released from any thread. -unsafe impl Send for Device {} +unsafe impl Send for Device {} =20 // SAFETY: A `drm::Device` can be shared among threads because all immutab= le methods are protected // by the synchronization in `struct drm_device`. -unsafe impl Sync for Device {} +unsafe impl Sync for Device {} diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index f30ee4c6245cd..fde0447566bef 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -10,6 +10,7 @@ prelude::*, sync::aref::ARef, }; +use core::{mem, ptr::NonNull}; use macros::vtable; =20 /// Driver use the GEM memory manager. This should be set for all modern d= rivers. @@ -122,30 +123,46 @@ pub trait Driver { =20 impl Registration { /// Creates a new [`Registration`] and registers it. - fn new(drm: &drm::Device, flags: usize) -> Result { + fn new(drm: drm::UnregisteredDevice, flags: usize) -> Result { // SAFETY: `drm.as_raw()` is valid by the invariants of `drm::Devi= ce`. to_result(unsafe { bindings::drm_dev_register(drm.as_raw(), flags)= })?; =20 - Ok(Self(drm.into())) + // SAFETY: We just called `drm_dev_register` above + let new =3D NonNull::from(unsafe { drm.assume_registered() }); + + // Leak the ARef from UnregisteredDevice in preparation for transf= erring its ownership. + mem::forget(drm); + + // SAFETY: `drm`'s `Drop` constructor was never called, ensuring t= hat there remains at least + // one reference to the device - which we take ownership over here. + let new =3D unsafe { ARef::from_raw(new) }; + + Ok(Self(new)) } =20 - /// Same as [`Registration::new`}, but transfers ownership of the [`Re= gistration`] to + /// Same as [`Registration::new`], but transfers ownership of the [`Re= gistration`] to /// [`devres::register`]. - pub fn new_foreign_owned( - drm: &drm::Device, - dev: &device::Device, + pub fn new_foreign_owned<'a>( + drm: drm::UnregisteredDevice, + dev: &'a device::Device, flags: usize, - ) -> Result + ) -> Result<&'a drm::Device> where T: 'static, { - if drm.as_ref().as_raw() !=3D dev.as_raw() { + let this_dev: &device::Device =3D drm.as_ref(); + if this_dev.as_raw() !=3D dev.as_raw() { return Err(EINVAL); } =20 let reg =3D Registration::::new(drm, flags)?; + let drm =3D NonNull::from(reg.device()); + + devres::register(dev, reg, GFP_KERNEL)?; =20 - devres::register(dev, reg, GFP_KERNEL) + // SAFETY: Since `reg` was passed to devres::register(), the devic= e now owns the lifetime + // of the DRM registration - ensuring that this references lives f= or at least as long as 'a. + Ok(unsafe { drm.as_ref() }) } =20 /// Returns a reference to the `Device` instance for this registration. diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs index 1b82b6945edf2..90018edef3236 100644 --- a/rust/kernel/drm/mod.rs +++ b/rust/kernel/drm/mod.rs @@ -8,7 +8,11 @@ pub mod gem; pub mod ioctl; =20 +pub use self::device::AnyCtx; pub use self::device::Device; +pub use self::device::DeviceCtx; +pub use self::device::Registered; +pub use self::device::UnregisteredDevice; pub use self::driver::Driver; pub use self::driver::DriverInfo; pub use self::driver::Registration; --=20 2.51.1 From nobody Fri Dec 19 21:47:10 2025 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (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 15B3A3054FA for ; Fri, 7 Nov 2025 19:32:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1762543962; cv=none; b=BPawUEhlvfVlAkCYVI6RTBKWyX1v9lpNuCwYvgAA6n7fmcOjJZAKtwzBYGU1tCGxZoWyRedj4YL+VkKiXC1UIHpklZ7S4ZHhFBi4pCNdE2rMgEMWd7dDyibYaJsilUyXBoBCaHD4+EifOyt2zkfJ2lSoufoeGdqRWob8V+1v3wA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1762543962; c=relaxed/simple; bh=vEpDgZ/NBDkSOkeh/jsY9K5RRJa6CdIyNseOkZKsDn0=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=dM1+9fPMhjz2QCZEDofIJ0oSMcnufnfN2JflgvYoImjvIlXyj8PW2hoYWgGedBaZgFic4D55akC5U0yQrI5IEqzmBYT3FbbiLRy4NpLMFfSa1piEYGjg+39IMkJ64wwsgtyIvns8rYoGdZGlad4emJHZRnwVd8HrlFohzixNHj8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=QOOqxNLd; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="QOOqxNLd" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1762543959; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ZMQ6c8QUhUu3sZavPYjgSB0Ce6fm5eV+7cMYeD6UdFs=; b=QOOqxNLdE52dfi6/gxm/+1iSz0CIKYVuWFDV9R3cxnfndfV/aMNhgCW5QALQHGvtbS8XqP V43t1PPdea72DMTZiZGfRy7KiHEXgIuE0F+uxHD9bUUgJQohs0iTLP7V1n6g+PedU+/Nw/ 8M66oCCRSAyhfjmTT1v5RvNycQbDk+c= Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-438-1QvkTD7VMnqoqtgpOjAteg-1; Fri, 07 Nov 2025 14:32:33 -0500 X-MC-Unique: 1QvkTD7VMnqoqtgpOjAteg-1 X-Mimecast-MFC-AGG-ID: 1QvkTD7VMnqoqtgpOjAteg_1762543949 Received: from mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.17]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id CF9F519560A7; Fri, 7 Nov 2025 19:32:28 +0000 (UTC) Received: from chopper.lan (unknown [10.22.81.9]) by mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id A9D3B1945110; Fri, 7 Nov 2025 19:32:23 +0000 (UTC) From: Lyude Paul To: linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org, dri-devel@lists.freedesktop.org Cc: Danilo Krummrich , Alice Ryhl , David Airlie , Simona Vetter , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , =?UTF-8?q?Bj=C3=B6rn=20Roy=20Baron?= , Benno Lossin , Andreas Hindborg , Trevor Gross , Daniel Almeida , Asahi Lina , Shankari Anand , nouveau@lists.freedesktop.org (open list:DRM DRIVER FOR NVIDIA GPUS [RUST]) Subject: [PATCH 2/2] rust/drm/gem: Use DeviceCtx with GEM objects Date: Fri, 7 Nov 2025 14:23:54 -0500 Message-ID: <20251107193204.398657-3-lyude@redhat.com> In-Reply-To: <20251107193204.398657-1-lyude@redhat.com> References: <20251107193204.398657-1-lyude@redhat.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Scanned-By: MIMEDefang 3.0 on 10.30.177.17 Content-Type: text/plain; charset="utf-8" Now that we have the ability to represent the context in which a DRM device is in at compile-time, we can start carrying around this context with GEM object types in order to allow a driver to safely create GEM objects before a DRM device has registered with userspace. Signed-off-by: Lyude Paul --- drivers/gpu/drm/nova/driver.rs | 2 +- drivers/gpu/drm/nova/gem.rs | 11 +++--- drivers/gpu/drm/tyr/driver.rs | 2 +- drivers/gpu/drm/tyr/gem.rs | 3 +- rust/kernel/drm/device.rs | 14 ++++---- rust/kernel/drm/driver.rs | 2 +- rust/kernel/drm/gem/mod.rs | 66 ++++++++++++++++++++++------------ 7 files changed, 63 insertions(+), 37 deletions(-) diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs index c78d69d5f045a..54245ee439042 100644 --- a/drivers/gpu/drm/nova/driver.rs +++ b/drivers/gpu/drm/nova/driver.rs @@ -59,7 +59,7 @@ fn probe(adev: &auxiliary::Device, _info: &Self::Id= Info) -> Result; + type Object =3D gem::Object; =20 const INFO: drm::DriverInfo =3D INFO; =20 diff --git a/drivers/gpu/drm/nova/gem.rs b/drivers/gpu/drm/nova/gem.rs index 2760ba4f3450b..14d275dc74ce7 100644 --- a/drivers/gpu/drm/nova/gem.rs +++ b/drivers/gpu/drm/nova/gem.rs @@ -2,7 +2,7 @@ =20 use kernel::{ drm, - drm::{gem, gem::BaseObject}, + drm::{gem, gem::BaseObject, DeviceCtx}, prelude::*, sync::aref::ARef, }; @@ -19,21 +19,24 @@ pub(crate) struct NovaObject {} impl gem::DriverObject for NovaObject { type Driver =3D NovaDriver; =20 - fn new(_dev: &NovaDevice, _size: usize) -> impl PinInit { + fn new(_dev: &NovaDevice, _size: usize) -> impl P= inInit { try_pin_init!(NovaObject {}) } } =20 impl NovaObject { /// Create a new DRM GEM object. - pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result>> { + pub(crate) fn new( + dev: &NovaDevice, + size: usize, + ) -> Result>> { let aligned_size =3D size.next_multiple_of(1 << 12); =20 if size =3D=3D 0 || size > aligned_size { return Err(EINVAL); } =20 - gem::Object::new(dev, aligned_size) + gem::Object::::new(dev, aligned_size) } =20 /// Look up a GEM object handle for a `File` and return an `ObjectRef`= for it. diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs index e3ea5ad85f49b..78fc08945e08e 100644 --- a/drivers/gpu/drm/tyr/driver.rs +++ b/drivers/gpu/drm/tyr/driver.rs @@ -187,7 +187,7 @@ fn drop(self: Pin<&mut Self>) { impl drm::Driver for TyrDriver { type Data =3D TyrData; type File =3D File; - type Object =3D drm::gem::Object; + type Object =3D drm::gem::Object; =20 const INFO: drm::DriverInfo =3D INFO; =20 diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs index 1273bf89dbd5d..5e1403409f468 100644 --- a/drivers/gpu/drm/tyr/gem.rs +++ b/drivers/gpu/drm/tyr/gem.rs @@ -3,6 +3,7 @@ use crate::driver::TyrDevice; use crate::driver::TyrDriver; use kernel::drm::gem; +use kernel::drm::DeviceCtx; use kernel::prelude::*; =20 /// GEM Object inner driver data @@ -12,7 +13,7 @@ pub(crate) struct TyrObject {} impl gem::DriverObject for TyrObject { type Driver =3D TyrDriver; =20 - fn new(_dev: &TyrDevice, _size: usize) -> impl PinInit { + fn new(_dev: &TyrDevice, _size: usize) -> impl Pi= nInit { try_pin_init!(TyrObject {}) } } diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 00072984930a3..227df6bf88c31 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -127,13 +127,13 @@ impl UnregisteredDevice { master_set: None, master_drop: None, debugfs_init: None, - gem_create_object: T::Object::ALLOC_OPS.gem_create_object, - prime_handle_to_fd: T::Object::ALLOC_OPS.prime_handle_to_fd, - prime_fd_to_handle: T::Object::ALLOC_OPS.prime_fd_to_handle, - gem_prime_import: T::Object::ALLOC_OPS.gem_prime_import, - gem_prime_import_sg_table: T::Object::ALLOC_OPS.gem_prime_import_s= g_table, - dumb_create: T::Object::ALLOC_OPS.dumb_create, - dumb_map_offset: T::Object::ALLOC_OPS.dumb_map_offset, + gem_create_object: T::Object::::ALLOC_OPS.gem_create_objec= t, + prime_handle_to_fd: T::Object::::ALLOC_OPS.prime_handle_to= _fd, + prime_fd_to_handle: T::Object::::ALLOC_OPS.prime_fd_to_han= dle, + gem_prime_import: T::Object::::ALLOC_OPS.gem_prime_import, + gem_prime_import_sg_table: T::Object::::ALLOC_OPS.gem_prim= e_import_sg_table, + dumb_create: T::Object::::ALLOC_OPS.dumb_create, + dumb_map_offset: T::Object::::ALLOC_OPS.dumb_map_offset, show_fdinfo: None, fbdev_probe: None, =20 diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index fde0447566bef..4e8735e9018c4 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -104,7 +104,7 @@ pub trait Driver { type Data: Sync + Send; =20 /// The type used to manage memory for this driver. - type Object: AllocImpl; + type Object: AllocImpl; =20 /// The type used to represent a DRM File (client) type File: drm::file::DriverFile; diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index eb5f3feac8907..62ca11126e1c0 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -7,13 +7,16 @@ use crate::{ alloc::flags::*, bindings, drm, - drm::driver::{AllocImpl, AllocOps}, + drm::{ + device::{DeviceCtx, Registered}, + driver::{AllocImpl, AllocOps}, + }, error::{to_result, Result}, prelude::*, sync::aref::{ARef, AlwaysRefCounted}, types::Opaque, }; -use core::{ops::Deref, ptr::NonNull}; +use core::{marker::PhantomData, ops::Deref, ptr::NonNull}; =20 /// A type alias for retrieving a [`Driver`]s [`DriverFile`] implementatio= n from its /// [`DriverObject`] implementation. @@ -22,21 +25,30 @@ /// [`DriverFile`]: drm::file::DriverFile pub type DriverFile =3D drm::File<<::Driver as drm::= Driver>::File>; =20 +/// A type alias for retrieving the current [`AllocImpl`] for a given [`Dr= iverObject`]. +/// +/// [`Driver`]: drm::Driver +pub type DriverAllocImpl =3D + <::Driver as drm::Driver>::Object; + /// GEM object functions, which must be implemented by drivers. pub trait DriverObject: Sync + Send + Sized { /// Parent `Driver` for this object. type Driver: drm::Driver; =20 /// Create a new driver data object for a GEM object of a given size. - fn new(dev: &drm::Device, size: usize) -> impl PinInit; + fn new( + dev: &drm::Device, + size: usize, + ) -> impl PinInit; =20 /// Open a new handle to an existing object, associated with a File. - fn open(_obj: &::Object, _file: &DriverFi= le) -> Result { + fn open(_obj: &DriverAllocImpl, _file: &DriverFile) -> Res= ult { Ok(()) } =20 /// Close a handle to an existing object, associated with a File. - fn close(_obj: &::Object, _file: &DriverF= ile) {} + fn close(_obj: &DriverAllocImpl, _file: &DriverFile) {} } =20 /// Trait that represents a GEM object subtype @@ -62,9 +74,12 @@ extern "C" fn open_callback( // SAFETY: `open_callback` is only ever called with a valid pointer to= a `struct drm_file`. let file =3D unsafe { DriverFile::::from_raw(raw_file) }; =20 - // SAFETY: `open_callback` is specified in the AllocOps structure for = `DriverObject`, - // ensuring that `raw_obj` is contained within a `DriverObject` - let obj =3D unsafe { <::Object as IntoGEMObj= ect>::from_raw(raw_obj) }; + // SAFETY: + // * `open_callback` is specified in the AllocOps structure for `Drive= rObject`, ensuring that + // `raw_obj` is contained within a `DriverAllocImpl` + // * It is only possible for `open_callback` to be called after device= registration, ensuring + // that the object's device is in the `Registered` state. + let obj: &DriverAllocImpl =3D unsafe { IntoGEMObject::from_raw(raw_= obj) }; =20 match T::open(obj, file) { Err(e) =3D> e.to_errno(), @@ -81,12 +96,12 @@ extern "C" fn close_callback( =20 // SAFETY: `close_callback` is specified in the AllocOps structure for= `Object`, ensuring // that `raw_obj` is indeed contained within a `Object`. - let obj =3D unsafe { <::Object as IntoGEMObj= ect>::from_raw(raw_obj) }; + let obj: &DriverAllocImpl =3D unsafe { IntoGEMObject::from_raw(raw_= obj) }; =20 T::close(obj, file); } =20 -impl IntoGEMObject for Object { +impl IntoGEMObject for Object { fn as_raw(&self) -> *mut bindings::drm_gem_object { self.obj.get() } @@ -94,7 +109,7 @@ fn as_raw(&self) -> *mut bindings::drm_gem_object { unsafe fn from_raw<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a= Self { // SAFETY: `obj` is guaranteed to be in an `Object` via the saf= ety contract of this // function - unsafe { &*crate::container_of!(Opaque::cast_from(self_ptr), Objec= t, obj) } + unsafe { &*crate::container_of!(Opaque::cast_from(self_ptr), Objec= t, obj) } } } =20 @@ -111,7 +126,7 @@ fn size(&self) -> usize { fn create_handle(&self, file: &drm::File) -> Result where Self: AllocImpl, - D: drm::Driver, + D: drm::Driver =3D Self, File =3D F>, F: drm::file::DriverFile, { let mut handle: u32 =3D 0; @@ -126,7 +141,7 @@ fn create_handle(&self, file: &drm::File) -> R= esult fn lookup_handle(file: &drm::File, handle: u32) -> Result> where Self: AllocImpl, - D: drm::Driver, + D: drm::Driver =3D Self, File =3D F>, F: drm::file::DriverFile, { // SAFETY: The arguments are all valid per the type invariants. @@ -166,16 +181,18 @@ impl BaseObject for T {} /// /// Invariants /// -/// - `self.obj` is a valid instance of a `struct drm_gem_object`. +/// * `self.obj` is a valid instance of a `struct drm_gem_object`. +/// * Any type invariants of `Ctx` apply to the parent DRM device for this= GEM object. #[repr(C)] #[pin_data] -pub struct Object { +pub struct Object { obj: Opaque, #[pin] data: T, + _ctx: PhantomData, } =20 -impl Object { +impl Object { const OBJECT_FUNCS: bindings::drm_gem_object_funcs =3D bindings::drm_g= em_object_funcs { free: Some(Self::free_callback), open: Some(open_callback::), @@ -195,11 +212,12 @@ impl Object { }; =20 /// Create a new GEM object. - pub fn new(dev: &drm::Device, size: usize) -> Result> { + pub fn new(dev: &drm::Device, size: usize) -> Result> { let obj: Pin> =3D KBox::pin_init( try_pin_init!(Self { obj: Opaque::new(bindings::drm_gem_object::default()), data <- T::new(dev, size), + _ctx: PhantomData, }), GFP_KERNEL, )?; @@ -208,6 +226,8 @@ pub fn new(dev: &drm::Device, size: usize) -= > Result> { unsafe { (*obj.as_raw()).funcs =3D &Self::OBJECT_FUNCS }; =20 // SAFETY: The arguments are all valid per the type invariants. + // INVARIANT: We use `dev` for creating the GEM object, which is k= nown to be in state `Ctx` - + // ensuring that the GEM object's pointer to the DRM device is alw= ays in the same state. to_result(unsafe { bindings::drm_gem_object_init(dev.as_raw(), obj= .obj.get(), size) })?; =20 // SAFETY: We never move out of `Self`. @@ -221,13 +241,15 @@ pub fn new(dev: &drm::Device, size: usize)= -> Result> { } =20 /// Returns the `Device` that owns this GEM object. - pub fn dev(&self) -> &drm::Device { + pub fn dev(&self) -> &drm::Device { // SAFETY: // - `struct drm_gem_object.dev` is initialized and valid for as l= ong as the GEM // object lives. // - The device we used for creating the gem object is passed as &= drm::Device to // Object::::new(), so we know that `T::Driver` is the right = generic parameter to use // here. + // - Any type invariants of `Ctx` are upheld by using the same `Ct= x` for the `Device` we + // return. unsafe { drm::Device::from_raw((*self.as_raw()).dev) } } =20 @@ -253,7 +275,7 @@ extern "C" fn free_callback(obj: *mut bindings::drm_gem= _object) { } =20 // SAFETY: Instances of `Object` are always reference-counted. -unsafe impl crate::types::AlwaysRefCounted for Object { +unsafe impl crate::types::AlwaysRefCounte= d for Object { fn inc_ref(&self) { // SAFETY: The existence of a shared reference guarantees that the= refcount is non-zero. unsafe { bindings::drm_gem_object_get(self.as_raw()) }; @@ -268,9 +290,9 @@ unsafe fn dec_ref(obj: NonNull) { } } =20 -impl super::private::Sealed for Object {} +impl super::private::Sealed for Object {} =20 -impl Deref for Object { +impl Deref for Object { type Target =3D T; =20 fn deref(&self) -> &Self::Target { @@ -278,7 +300,7 @@ fn deref(&self) -> &Self::Target { } } =20 -impl AllocImpl for Object { +impl AllocImpl for Object { type Driver =3D T::Driver; =20 const ALLOC_OPS: AllocOps =3D AllocOps { --=20 2.51.1