* [PATCH v2 1/7] rust: drm: Add Driver::ParentDevice associated type
2026-06-03 1:15 [PATCH v2 0/7] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
@ 2026-06-03 1:15 ` Danilo Krummrich
2026-06-03 1:15 ` [PATCH v2 2/7] rust: drm: Add UnbindGuard for drm_dev_enter/exit critical sections Danilo Krummrich
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Danilo Krummrich @ 2026-06-03 1:15 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Add a ParentDevice associated type to the Driver trait, allowing each
DRM driver to declare its parent bus device type (e.g.
auxiliary::Device, platform::Device).
Change UnregisteredDevice::new() to take &T::ParentDevice<Bound>,
ensuring at the type level that the DRM device's parent matches the
declared bus device type.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
drivers/gpu/drm/nova/driver.rs | 8 ++++++--
drivers/gpu/drm/tyr/driver.rs | 6 ++++--
rust/kernel/drm/device.rs | 7 +++++--
rust/kernel/drm/driver.rs | 3 +++
4 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index 48933d86ddda..c5b0313006bd 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -2,7 +2,10 @@
use kernel::{
auxiliary,
- device::Core,
+ device::{
+ Core,
+ DeviceContext, //
+ },
drm::{
self,
gem,
@@ -62,7 +65,7 @@ fn probe<'bound>(
) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
let data = try_pin_init!(NovaData { adev: adev.into() });
- let drm = drm::UnregisteredDevice::<Self>::new(adev.as_ref(), data)?;
+ let drm = drm::UnregisteredDevice::<Self>::new(adev, data)?;
let drm = drm::Registration::new_foreign_owned(drm, adev.as_ref(), 0)?;
Ok(Nova { drm: drm.into() })
@@ -74,6 +77,7 @@ impl drm::Driver for NovaDriver {
type Data = NovaData;
type File = File;
type Object<Ctx: drm::DeviceContext> = gem::Object<NovaObject, Ctx>;
+ type ParentDevice<Ctx: DeviceContext> = auxiliary::Device<Ctx>;
const INFO: drm::DriverInfo = INFO;
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index d063bc664cc1..338c25ccc151 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -7,7 +7,8 @@
},
device::{
Core,
- Device, //
+ Device,
+ DeviceContext, //
},
dma::{
Device as DmaDevice,
@@ -148,7 +149,7 @@ fn probe<'bound>(
gpu_info,
});
- let tdev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev.as_ref(), data)?;
+ let tdev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev, data)?;
let tdev = drm::driver::Registration::new_foreign_owned(tdev, pdev.as_ref(), 0)?;
let driver = TyrPlatformDriverData {
@@ -182,6 +183,7 @@ impl drm::Driver for TyrDrmDriver {
type Data = TyrDrmDeviceData;
type File = TyrDrmFileData;
type Object<R: drm::DeviceContext> = drm::gem::shmem::Object<BoData, R>;
+ type ParentDevice<Ctx: DeviceContext> = platform::Device<Ctx>;
const INFO: drm::DriverInfo = INFO;
const FEAT_RENDER: bool = true;
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 477cf771fb10..858272fc2164 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -208,7 +208,10 @@ const fn compute_features() -> u32 {
/// Create a new `UnregisteredDevice` for a `drm::Driver`.
///
/// This can be used to create a [`Registration`](kernel::drm::Registration).
- pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<Self> {
+ pub fn new(
+ dev: &T::ParentDevice<device::Bound>,
+ data: impl PinInit<T::Data, Error>,
+ ) -> Result<Self> {
// `__drm_dev_alloc` uses `kmalloc()` to allocate memory, hence ensure a `kmalloc()`
// compatible `Layout`.
let layout = Kmalloc::aligned_layout(Layout::new::<Device<T, Uninit>>());
@@ -225,7 +228,7 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<S
// - `dev` is valid by its type invarants,
let raw_drm: *mut Device<T, Uninit> = unsafe {
bindings::__drm_dev_alloc(
- dev.as_raw(),
+ dev.as_ref().as_raw(),
&alloc_vtable,
layout.size(),
mem::offset_of!(Device<T, Uninit>, dev),
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index 25f7e233884d..802e7fc13e30 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -116,6 +116,9 @@ pub trait Driver {
/// The type used to represent a DRM File (client)
type File: drm::file::DriverFile;
+ /// The bus device type of the parent device that the DRM device is associated with.
+ type ParentDevice<Ctx: device::DeviceContext>: device::AsBusDevice<Ctx>;
+
/// Driver metadata
const INFO: DriverInfo;
--
2.54.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 2/7] rust: drm: Add UnbindGuard for drm_dev_enter/exit critical sections
2026-06-03 1:15 [PATCH v2 0/7] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
2026-06-03 1:15 ` [PATCH v2 1/7] rust: drm: Add Driver::ParentDevice associated type Danilo Krummrich
@ 2026-06-03 1:15 ` Danilo Krummrich
2026-06-03 11:47 ` Gary Guo
2026-06-03 1:15 ` [PATCH v2 3/7] rust: drm: Add RegistrationData to drm::Driver Danilo Krummrich
` (4 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Danilo Krummrich @ 2026-06-03 1:15 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
DRM ioctls do not guarantee that the parent bus device is still bound.
However, since DRM device registration is managed through Devres, using
drm_dev_unplug() on unregistration ensures that between drm_dev_enter()
and drm_dev_exit() the parent device must be bound.
Add UnbindGuard, a guard object representing a drm_dev_enter/exit SRCU
critical section that dereferences to &Device<Bound> of the parent bus
device. The guard is only available on Device<T, Registered>, ensuring
it cannot be used on unregistered devices.
Also add with_unbind_guard() as a convenience helper that executes a
closure with the bound device reference.
Switch Registration::drop from drm_dev_unregister() to drm_dev_unplug()
to provide the SRCU barrier that UnbindGuard's safety argument relies on.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
rust/kernel/drm/device.rs | 82 ++++++++++++++++++++++++++++++++++++++-
rust/kernel/drm/driver.rs | 10 ++++-
2 files changed, 89 insertions(+), 3 deletions(-)
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 858272fc2164..828618ae19af 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -7,7 +7,10 @@
use crate::{
alloc::allocator::Kmalloc,
bindings,
- device,
+ device::{
+ self,
+ AsBusDevice as _, //
+ },
drm::{
self,
driver::AllocImpl,
@@ -355,6 +358,83 @@ pub(crate) unsafe fn assume_ctx<NewCtx: DeviceContext>(&self) -> &Device<T, NewC
}
}
+impl<T: drm::Driver> Device<T, Registered> {
+ /// Guard against the parent bus device being unbound.
+ ///
+ /// Returns an [`UnbindGuard`] if the device has not been unplugged, [`None`] otherwise.
+ ///
+ /// The returned guard dereferences to the parent bus device in the [`device::Bound`] context
+ /// (see [`Driver::ParentDevice`](drm::Driver::ParentDevice)).
+ ///
+ /// While [`UnbindGuard`] is held the parent device is guaranteed to be bound.
+ #[must_use]
+ pub fn unbind_guard(&self) -> Option<UnbindGuard<'_, T>> {
+ let mut idx: i32 = 0;
+ // SAFETY: `self.as_raw()` is a valid pointer to a `struct drm_device` by the type
+ // invariants of `Device<T, Registered>`.
+ if unsafe { bindings::drm_dev_enter(self.as_raw(), &mut idx) } {
+ Some(UnbindGuard { dev: self, idx })
+ } else {
+ None
+ }
+ }
+
+ /// Execute a closure while the parent bus device is guaranteed to be bound.
+ ///
+ /// Acquires the [`UnbindGuard`] and, if the device has not been unplugged, calls `f` with the
+ /// parent bus device. Returns `None` if the device has been unplugged.
+ pub fn with_unbind_guard<R>(
+ &self,
+ f: impl FnOnce(&T::ParentDevice<device::Bound>) -> R,
+ ) -> Option<R> {
+ let guard = self.unbind_guard()?;
+ Some(f(&guard))
+ }
+}
+
+/// A guard preventing the parent bus device from being unbound.
+///
+/// The guard dereferences to [`Driver::ParentDevice<Bound>`](drm::Driver::ParentDevice), providing
+/// access to the parent bus device with the guarantee that it is bound for the entire duration of
+/// the critical section.
+///
+/// Internally this is backed by a `drm_dev_enter()` / `drm_dev_exit()` SRCU critical section.
+///
+/// See [`Device::unbind_guard`] for details on the safety argument.
+///
+/// # Invariants
+///
+/// - `idx` is the SRCU read lock index returned by a successful `drm_dev_enter()` call.
+/// - The parent bus device of `dev` is bound for the lifetime of this guard.
+#[must_use]
+pub struct UnbindGuard<'a, T: drm::Driver> {
+ dev: &'a Device<T, Registered>,
+ idx: i32,
+}
+
+impl<T: drm::Driver> Deref for UnbindGuard<'_, T> {
+ type Target = T::ParentDevice<device::Bound>;
+
+ #[inline]
+ fn deref(&self) -> &Self::Target {
+ // SAFETY:
+ // - The parent `struct device` is embedded in a `T::ParentDevice`, as guaranteed by
+ // `UnregisteredDevice::new` taking a `&T::ParentDevice<device::Bound>`.
+ // - By the type invariants of `UnbindGuard`, the parent device is bound for the lifetime
+ // of this guard.
+ unsafe { T::ParentDevice::from_device(self.dev.as_ref().as_bound()) }
+ }
+}
+
+impl<T: drm::Driver> Drop for UnbindGuard<'_, T> {
+ #[inline]
+ fn drop(&mut self) {
+ // SAFETY: `self.idx` was returned by a successful `drm_dev_enter()` call, as guaranteed
+ // by the type invariants of `UnbindGuard`.
+ unsafe { bindings::drm_dev_exit(self.idx) };
+ }
+}
+
impl<T: drm::Driver, C: DeviceContext> Deref for Device<T, C> {
type Target = T::Data;
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index 802e7fc13e30..f68a17d8939d 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -199,8 +199,14 @@ unsafe impl<T: Driver> Send for Registration<T> {}
impl<T: Driver> Drop for Registration<T> {
fn drop(&mut self) {
+ // Use `drm_dev_unplug` rather than `drm_dev_unregister` to ensure that existing
+ // `drm_dev_enter()` critical sections complete before unregistration proceeds. This
+ // is required for the safety of `UnbindGuard`, which relies on the SRCU barrier in
+ // `drm_dev_unplug()` to guarantee that the parent device is still bound within the
+ // critical section.
+ //
// SAFETY: Safe by the invariant of `ARef<drm::Device<T>>`. The existence of this
- // `Registration` also guarantees the this `drm::Device` is actually registered.
- unsafe { bindings::drm_dev_unregister(self.0.as_raw()) };
+ // `Registration` also guarantees that this `drm::Device` is actually registered.
+ unsafe { bindings::drm_dev_unplug(self.0.as_raw()) };
}
}
--
2.54.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 2/7] rust: drm: Add UnbindGuard for drm_dev_enter/exit critical sections
2026-06-03 1:15 ` [PATCH v2 2/7] rust: drm: Add UnbindGuard for drm_dev_enter/exit critical sections Danilo Krummrich
@ 2026-06-03 11:47 ` Gary Guo
0 siblings, 0 replies; 13+ messages in thread
From: Gary Guo @ 2026-06-03 11:47 UTC (permalink / raw)
To: Danilo Krummrich, aliceryhl, daniel.almeida, acourbot, ecourtney,
ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
On Wed Jun 3, 2026 at 2:15 AM BST, Danilo Krummrich wrote:
> DRM ioctls do not guarantee that the parent bus device is still bound.
> However, since DRM device registration is managed through Devres, using
> drm_dev_unplug() on unregistration ensures that between drm_dev_enter()
> and drm_dev_exit() the parent device must be bound.
>
> Add UnbindGuard, a guard object representing a drm_dev_enter/exit SRCU
> critical section that dereferences to &Device<Bound> of the parent bus
> device. The guard is only available on Device<T, Registered>, ensuring
> it cannot be used on unregistered devices.
>
> Also add with_unbind_guard() as a convenience helper that executes a
> closure with the bound device reference.
>
> Switch Registration::drop from drm_dev_unregister() to drm_dev_unplug()
> to provide the SRCU barrier that UnbindGuard's safety argument relies on.
Conceptually the actual thing this is guarding against is not device unbind but
DRM unregistration. Currently we force DRM registration to use
`devres::register` so they're the same, but with patch 3 you're adding
registration data and `Registration` is now something that lives in parent
device's driver data.
This also means that it is *possible* that a `Registration` is dropped before
device unbinding, so I think the `UnbindGuard` is not the best name for this
now, something like `UnregistrationGuard` reflect what it does more.
Best,
Gary
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
> rust/kernel/drm/device.rs | 82 ++++++++++++++++++++++++++++++++++++++-
> rust/kernel/drm/driver.rs | 10 ++++-
> 2 files changed, 89 insertions(+), 3 deletions(-)
>
> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> index 858272fc2164..828618ae19af 100644
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs
> @@ -7,7 +7,10 @@
> use crate::{
> alloc::allocator::Kmalloc,
> bindings,
> - device,
> + device::{
> + self,
> + AsBusDevice as _, //
> + },
> drm::{
> self,
> driver::AllocImpl,
> @@ -355,6 +358,83 @@ pub(crate) unsafe fn assume_ctx<NewCtx: DeviceContext>(&self) -> &Device<T, NewC
> }
> }
>
> +impl<T: drm::Driver> Device<T, Registered> {
> + /// Guard against the parent bus device being unbound.
> + ///
> + /// Returns an [`UnbindGuard`] if the device has not been unplugged, [`None`] otherwise.
> + ///
> + /// The returned guard dereferences to the parent bus device in the [`device::Bound`] context
> + /// (see [`Driver::ParentDevice`](drm::Driver::ParentDevice)).
> + ///
> + /// While [`UnbindGuard`] is held the parent device is guaranteed to be bound.
> + #[must_use]
> + pub fn unbind_guard(&self) -> Option<UnbindGuard<'_, T>> {
> + let mut idx: i32 = 0;
> + // SAFETY: `self.as_raw()` is a valid pointer to a `struct drm_device` by the type
> + // invariants of `Device<T, Registered>`.
> + if unsafe { bindings::drm_dev_enter(self.as_raw(), &mut idx) } {
> + Some(UnbindGuard { dev: self, idx })
> + } else {
> + None
> + }
> + }
> +
> + /// Execute a closure while the parent bus device is guaranteed to be bound.
> + ///
> + /// Acquires the [`UnbindGuard`] and, if the device has not been unplugged, calls `f` with the
> + /// parent bus device. Returns `None` if the device has been unplugged.
> + pub fn with_unbind_guard<R>(
> + &self,
> + f: impl FnOnce(&T::ParentDevice<device::Bound>) -> R,
> + ) -> Option<R> {
> + let guard = self.unbind_guard()?;
> + Some(f(&guard))
> + }
> +}
> +
> +/// A guard preventing the parent bus device from being unbound.
> +///
> +/// The guard dereferences to [`Driver::ParentDevice<Bound>`](drm::Driver::ParentDevice), providing
> +/// access to the parent bus device with the guarantee that it is bound for the entire duration of
> +/// the critical section.
> +///
> +/// Internally this is backed by a `drm_dev_enter()` / `drm_dev_exit()` SRCU critical section.
> +///
> +/// See [`Device::unbind_guard`] for details on the safety argument.
> +///
> +/// # Invariants
> +///
> +/// - `idx` is the SRCU read lock index returned by a successful `drm_dev_enter()` call.
> +/// - The parent bus device of `dev` is bound for the lifetime of this guard.
> +#[must_use]
> +pub struct UnbindGuard<'a, T: drm::Driver> {
> + dev: &'a Device<T, Registered>,
> + idx: i32,
> +}
> +
> +impl<T: drm::Driver> Deref for UnbindGuard<'_, T> {
> + type Target = T::ParentDevice<device::Bound>;
> +
> + #[inline]
> + fn deref(&self) -> &Self::Target {
> + // SAFETY:
> + // - The parent `struct device` is embedded in a `T::ParentDevice`, as guaranteed by
> + // `UnregisteredDevice::new` taking a `&T::ParentDevice<device::Bound>`.
> + // - By the type invariants of `UnbindGuard`, the parent device is bound for the lifetime
> + // of this guard.
> + unsafe { T::ParentDevice::from_device(self.dev.as_ref().as_bound()) }
> + }
> +}
> +
> +impl<T: drm::Driver> Drop for UnbindGuard<'_, T> {
> + #[inline]
> + fn drop(&mut self) {
> + // SAFETY: `self.idx` was returned by a successful `drm_dev_enter()` call, as guaranteed
> + // by the type invariants of `UnbindGuard`.
> + unsafe { bindings::drm_dev_exit(self.idx) };
> + }
> +}
> +
> impl<T: drm::Driver, C: DeviceContext> Deref for Device<T, C> {
> type Target = T::Data;
>
> diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
> index 802e7fc13e30..f68a17d8939d 100644
> --- a/rust/kernel/drm/driver.rs
> +++ b/rust/kernel/drm/driver.rs
> @@ -199,8 +199,14 @@ unsafe impl<T: Driver> Send for Registration<T> {}
>
> impl<T: Driver> Drop for Registration<T> {
> fn drop(&mut self) {
> + // Use `drm_dev_unplug` rather than `drm_dev_unregister` to ensure that existing
> + // `drm_dev_enter()` critical sections complete before unregistration proceeds. This
> + // is required for the safety of `UnbindGuard`, which relies on the SRCU barrier in
> + // `drm_dev_unplug()` to guarantee that the parent device is still bound within the
> + // critical section.
> + //
> // SAFETY: Safe by the invariant of `ARef<drm::Device<T>>`. The existence of this
> - // `Registration` also guarantees the this `drm::Device` is actually registered.
> - unsafe { bindings::drm_dev_unregister(self.0.as_raw()) };
> + // `Registration` also guarantees that this `drm::Device` is actually registered.
> + unsafe { bindings::drm_dev_unplug(self.0.as_raw()) };
> }
> }
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 3/7] rust: drm: Add RegistrationData to drm::Driver
2026-06-03 1:15 [PATCH v2 0/7] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
2026-06-03 1:15 ` [PATCH v2 1/7] rust: drm: Add Driver::ParentDevice associated type Danilo Krummrich
2026-06-03 1:15 ` [PATCH v2 2/7] rust: drm: Add UnbindGuard for drm_dev_enter/exit critical sections Danilo Krummrich
@ 2026-06-03 1:15 ` Danilo Krummrich
2026-06-03 11:51 ` Gary Guo
2026-06-03 23:29 ` Deborah Brouwer
2026-06-03 1:15 ` [PATCH v2 4/7] rust: drm: Wrap ioctl dispatch in UnbindGuard Danilo Krummrich
` (3 subsequent siblings)
6 siblings, 2 replies; 13+ messages in thread
From: Danilo Krummrich @ 2026-06-03 1:15 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Add a RegistrationData associated type to drm::Driver. This is a ForLt
type whose lifetime is tied to the parent bus device binding scope.
Registration<'a, T> takes ownership of the data via Pin<KBox<_>>,
storing it with its real lifetime. The pointer is written to drm::Device
before drm_dev_register() to ensure it is already in place when ioctls
arrive.
UnbindGuard::registration_data_with() provides access with the lifetime
shortened from 'static via ForLt::cast_ref_unchecked. Since
Registration::drop() calls drm_dev_unplug(), which performs an SRCU
barrier waiting for all drm_dev_enter() critical sections to complete,
the data is guaranteed to remain valid for the duration of any
UnbindGuard.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
drivers/gpu/drm/nova/driver.rs | 16 +++--
drivers/gpu/drm/tyr/driver.rs | 16 +++--
rust/kernel/drm/device.rs | 49 +++++++++++++
rust/kernel/drm/driver.rs | 123 ++++++++++++++++++++++++---------
rust/kernel/drm/mod.rs | 1 +
5 files changed, 162 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index c5b0313006bd..4267e6e6dbb4 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -12,7 +12,8 @@
ioctl, //
},
prelude::*,
- sync::aref::ARef, //
+ sync::aref::ARef,
+ types::ForLt, //
};
use crate::file::File;
@@ -20,9 +21,10 @@
pub(crate) struct NovaDriver;
-pub(crate) struct Nova {
+pub(crate) struct Nova<'bound> {
#[expect(unused)]
drm: ARef<drm::Device<NovaDriver>>,
+ _reg: drm::Registration<'bound, NovaDriver>,
}
/// Convienence type alias for the DRM device type for this driver
@@ -56,7 +58,7 @@ pub(crate) struct NovaData {
impl auxiliary::Driver for NovaDriver {
type IdInfo = ();
- type Data<'bound> = Nova;
+ type Data<'bound> = Nova<'bound>;
const ID_TABLE: auxiliary::IdTable<Self::IdInfo> = &AUX_TABLE;
fn probe<'bound>(
@@ -66,15 +68,19 @@ fn probe<'bound>(
let data = try_pin_init!(NovaData { adev: adev.into() });
let drm = drm::UnregisteredDevice::<Self>::new(adev, data)?;
- let drm = drm::Registration::new_foreign_owned(drm, adev.as_ref(), 0)?;
+ let reg = drm::Registration::new(adev.as_ref(), drm, (), 0)?;
- Ok(Nova { drm: drm.into() })
+ Ok(Nova {
+ drm: reg.device().into(),
+ _reg: reg,
+ })
}
}
#[vtable]
impl drm::Driver for NovaDriver {
type Data = NovaData;
+ type RegistrationData = ForLt!(());
type File = File;
type Object<Ctx: drm::DeviceContext> = gem::Object<NovaObject, Ctx>;
type ParentDevice<Ctx: DeviceContext> = auxiliary::Device<Ctx>;
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index 338c25ccc151..819f64a7573d 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -31,7 +31,8 @@
aref::ARef,
Mutex, //
},
- time, //
+ time,
+ types::ForLt, //
};
use crate::{
@@ -52,8 +53,9 @@
pub(crate) struct TyrPlatformDriver;
#[pin_data(PinnedDrop)]
-pub(crate) struct TyrPlatformDriverData {
+pub(crate) struct TyrPlatformDriverData<'bound> {
_device: ARef<TyrDrmDevice>,
+ _reg: drm::Registration<'bound, TyrDrmDriver>,
}
#[pin_data]
@@ -98,7 +100,7 @@ fn issue_soft_reset(dev: &Device, iomem: &IoMem<'_>) -> Result {
impl platform::Driver for TyrPlatformDriver {
type IdInfo = ();
- type Data<'bound> = TyrPlatformDriverData;
+ type Data<'bound> = TyrPlatformDriverData<'bound>;
const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
fn probe<'bound>(
@@ -150,10 +152,11 @@ fn probe<'bound>(
});
let tdev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev, data)?;
- let tdev = drm::driver::Registration::new_foreign_owned(tdev, pdev.as_ref(), 0)?;
+ let reg = drm::Registration::new(pdev.as_ref(), tdev, (), 0)?;
let driver = TyrPlatformDriverData {
- _device: tdev.into(),
+ _device: reg.device().into(),
+ _reg: reg,
};
// We need this to be dev_info!() because dev_dbg!() does not work at
@@ -164,7 +167,7 @@ fn probe<'bound>(
}
#[pinned_drop]
-impl PinnedDrop for TyrPlatformDriverData {
+impl PinnedDrop for TyrPlatformDriverData<'_> {
fn drop(self: Pin<&mut Self>) {}
}
@@ -181,6 +184,7 @@ fn drop(self: Pin<&mut Self>) {}
#[vtable]
impl drm::Driver for TyrDrmDriver {
type Data = TyrDrmDeviceData;
+ type RegistrationData = ForLt!(());
type File = TyrDrmFileData;
type Object<R: drm::DeviceContext> = drm::gem::shmem::Object<BoData, R>;
type ParentDevice<Ctx: DeviceContext> = platform::Device<Ctx>;
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 828618ae19af..9e5e069b5135 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -23,6 +23,7 @@
AlwaysRefCounted, //
},
types::{
+ ForLt,
NotThreadSafe,
Opaque, //
},
@@ -35,6 +36,7 @@
};
use core::{
alloc::Layout,
+ cell::UnsafeCell,
marker::PhantomData,
mem,
ops::Deref,
@@ -259,6 +261,9 @@ pub fn new(
// SAFETY: `drm_dev` is still private to this function.
unsafe { (*drm_dev).driver = const { &Self::VTABLE } };
+ // SAFETY: `raw_drm` is valid; no concurrent access before registration.
+ unsafe { (*raw_drm.as_ptr()).registration_data = UnsafeCell::new(NonNull::dangling()) };
+
// SAFETY: The reference count is one, and now we take ownership of that reference as a
// `drm::Device`.
// INVARIANT: We just created the device above, but have yet to call `drm_dev_register`.
@@ -290,6 +295,7 @@ pub fn new(
pub struct Device<T: drm::Driver, C: DeviceContext = Registered> {
dev: Opaque<bindings::drm_device>,
data: T::Data,
+ pub(super) registration_data: UnsafeCell<NonNull<<T::RegistrationData as ForLt>::Of<'static>>>,
_ctx: PhantomData<C>,
}
@@ -298,6 +304,27 @@ pub(crate) fn as_raw(&self) -> *mut bindings::drm_device {
self.dev.get()
}
+ /// Returns a reference to the registration data with lifetime shortened from `'static`.
+ ///
+ /// # Safety
+ ///
+ /// The caller must ensure that:
+ ///
+ /// - The parent bus device is bound (e.g. by holding an active `drm_dev_enter()` critical
+ /// section via [`UnbindGuard`]).
+ /// - The returned reference is not exposed to code that can choose a concrete lifetime for
+ /// it, as that would be unsound for types that are invariant over their lifetime parameter
+ /// (e.g. it must be passed through an HRTB-bounded closure).
+ unsafe fn registration_data_unchecked(&self) -> &<T::RegistrationData as ForLt>::Of<'_> {
+ // SAFETY: Caller guarantees the parent bus device is bound, hence
+ // the pointer is valid.
+ let static_ref = unsafe { (*self.registration_data.get()).as_ref() };
+
+ // SAFETY: Caller guarantees the reference is only used behind an HRTB, making the
+ // round-trip from `'static` sound regardless of variance.
+ unsafe { T::RegistrationData::cast_ref_unchecked(static_ref) }
+ }
+
/// # Safety
///
/// `ptr` must be a valid pointer to a `struct device` embedded in `Self`.
@@ -412,6 +439,28 @@ pub struct UnbindGuard<'a, T: drm::Driver> {
idx: i32,
}
+impl<T: drm::Driver> UnbindGuard<'_, T> {
+ /// Access the parent device and registration data through a closure, with both lifetimes
+ /// tied to the closure scope.
+ ///
+ /// The data is owned by [`Registration`](drm::Registration) and is guaranteed to remain valid
+ /// for the duration of this guard, since [`Registration`](drm::Registration)'s `drop` calls
+ /// `drm_dev_unplug()` which waits for all `drm_dev_enter()` critical sections to complete.
+ pub fn registration_data_with<R, F>(&self, f: F) -> R
+ where
+ F: for<'a> FnOnce(
+ &'a T::ParentDevice<device::Bound>,
+ &'a <T::RegistrationData as ForLt>::Of<'a>,
+ ) -> R,
+ {
+ // SAFETY: We hold an active `drm_dev_enter()` critical section, so the parent bus
+ // device is guaranteed to be bound. The closure's HRTB `for<'a>` prevents the caller
+ // from smuggling in references with a concrete short lifetime, satisfying the lifetime
+ // requirement of `registration_data_unchecked`.
+ f(&**self, unsafe { self.dev.registration_data_unchecked() })
+ }
+}
+
impl<T: drm::Driver> Deref for UnbindGuard<'_, T> {
type Target = T::ParentDevice<device::Bound>;
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index f68a17d8939d..6aa1cb99cc7f 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -7,11 +7,11 @@
use crate::{
bindings,
device,
- devres,
drm,
error::to_result,
prelude::*,
- sync::aref::ARef, //
+ sync::aref::ARef,
+ types::ForLt, //
};
use core::{
mem,
@@ -110,6 +110,16 @@ pub trait Driver {
/// Context data associated with the DRM driver
type Data: Sync + Send;
+ /// Data owned by the [`Registration`] and accessible through [`drm::device::UnbindGuard`].
+ ///
+ /// This is a [`ForLt`](trait@ForLt) type whose lifetime is tied to the parent bus
+ /// device binding scope.
+ /// The data is only accessible while the parent bus device is bound (i.e. within a
+ /// `drm_dev_enter/exit` critical section), and references handed out by
+ /// [`UnbindGuard::registration_data_with()`](drm::device::UnbindGuard::registration_data_with)
+ /// ties the lifetime to the closure scope.
+ type RegistrationData: ForLt;
+
/// The type used to manage memory for this driver.
type Object<Ctx: drm::DeviceContext>: AllocImpl;
@@ -139,12 +149,57 @@ pub trait Driver {
/// The registration type of a `drm::Device`.
///
/// Once the `Registration` structure is dropped, the device is unregistered.
-pub struct Registration<T: Driver>(ARef<drm::Device<T>>);
+pub struct Registration<'a, T: Driver> {
+ drm: ARef<drm::Device<T>>,
+ _reg_data: Pin<KBox<<T::RegistrationData as ForLt>::Of<'a>>>,
+}
+
+impl<'a, T: Driver> Registration<'a, T>
+where
+ for<'b> <T::RegistrationData as ForLt>::Of<'b>: Send,
+{
+ /// Register a new [`UnregisteredDevice`](drm::UnregisteredDevice) with userspace.
+ ///
+ /// # Safety
+ ///
+ /// The caller must not `mem::forget()` the returned [`Registration`] or otherwise prevent its
+ /// [`Drop`] implementation from running, since the registration data may contain borrowed
+ /// references that become invalid after `'a` ends.
+ ///
+ /// If the registration data is `'static`, use the safe [`Registration::new()`] instead.
+ pub unsafe fn new_with_lt<E>(
+ dev: &'a device::Device<device::Bound>,
+ drm: drm::UnregisteredDevice<T>,
+ reg_data: impl PinInit<<T::RegistrationData as ForLt>::Of<'a>, E>,
+ flags: usize,
+ ) -> Result<Self>
+ where
+ Error: From<E>,
+ {
+ if drm.as_ref().as_raw() != dev.as_raw() {
+ return Err(EINVAL);
+ }
-impl<T: Driver> Registration<T> {
- fn new(drm: drm::UnregisteredDevice<T>, flags: usize) -> Result<Self> {
- // SAFETY: `drm.as_raw()` is valid by the invariants of `drm::Device`.
- to_result(unsafe { bindings::drm_dev_register(drm.as_raw(), flags) })?;
+ let reg_data: Pin<KBox<<T::RegistrationData as ForLt>::Of<'a>>> =
+ KBox::pin_init(reg_data, GFP_KERNEL)?;
+
+ // Store the registration data pointer in the device before registration, so that it is
+ // visible once ioctls can be called.
+ //
+ // SAFETY: Lifetimes do not affect layout, so the pointer cast is sound.
+ let ptr: NonNull<<T::RegistrationData as ForLt>::Of<'static>> =
+ unsafe { mem::transmute(NonNull::from(Pin::get_ref(reg_data.as_ref()))) };
+
+ // SAFETY: No concurrent access; the device is not yet registered.
+ unsafe { *drm.registration_data.get() = ptr };
+
+ // SAFETY: `drm` is a valid, initialized but not yet registered DRM device.
+ let ret = unsafe { bindings::drm_dev_register(drm.as_raw(), flags) };
+ if let Err(e) = to_result(ret) {
+ // SAFETY: No concurrent access; registration failed.
+ unsafe { *drm.registration_data.get() = NonNull::dangling() };
+ return Err(e);
+ }
// SAFETY: We just called `drm_dev_register` above
let new = NonNull::from(unsafe { drm.assume_ctx() });
@@ -156,48 +211,49 @@ fn new(drm: drm::UnregisteredDevice<T>, flags: usize) -> Result<Self> {
// one reference to the device - which we take ownership over here.
let new = unsafe { ARef::from_raw(new) };
- Ok(Self(new))
+ Ok(Self {
+ drm: new,
+ _reg_data: reg_data,
+ })
}
- /// Registers a new [`UnregisteredDevice`](drm::UnregisteredDevice) with userspace.
- ///
- /// Ownership of the [`Registration`] object is passed to [`devres::register`].
- pub fn new_foreign_owned<'a>(
- drm: drm::UnregisteredDevice<T>,
+ /// Safe variant of [`Registration::new_with_lt()`] for registration data that does not contain
+ /// borrowed references.
+ pub fn new<E>(
dev: &'a device::Device<device::Bound>,
+ drm: drm::UnregisteredDevice<T>,
+ reg_data: impl PinInit<<T::RegistrationData as ForLt>::Of<'a>, E>,
flags: usize,
- ) -> Result<&'a drm::Device<T>>
+ ) -> Result<Self>
where
- T: 'static,
+ <T::RegistrationData as ForLt>::Of<'a>: 'static,
+ Error: From<E>,
{
- if drm.as_ref().as_raw() != dev.as_raw() {
- return Err(EINVAL);
- }
-
- let reg = Registration::<T>::new(drm, flags)?;
- let drm = NonNull::from(reg.device());
-
- devres::register(dev, reg, GFP_KERNEL)?;
-
- // SAFETY: Since `reg` was passed to devres::register(), the device now owns the lifetime
- // of the DRM registration - ensuring that this references lives for at least as long as 'a.
- Ok(unsafe { drm.as_ref() })
+ // SAFETY: `Of<'a>: 'static` guarantees the data contains no borrowed references,
+ // so forgetting the `Registration` cannot cause use-after-free.
+ unsafe { Self::new_with_lt(dev, drm, reg_data, flags) }
}
/// Returns a reference to the `Device` instance for this registration.
pub fn device(&self) -> &drm::Device<T> {
- &self.0
+ &self.drm
}
}
// SAFETY: `Registration` doesn't offer any methods or access to fields when shared between
// threads, hence it's safe to share it.
-unsafe impl<T: Driver> Sync for Registration<T> {}
+unsafe impl<T: Driver> Sync for Registration<'_, T> where
+ for<'a> <T::RegistrationData as ForLt>::Of<'a>: Send
+{
+}
// SAFETY: Registration with and unregistration from the DRM subsystem can happen from any thread.
-unsafe impl<T: Driver> Send for Registration<T> {}
+unsafe impl<T: Driver> Send for Registration<'_, T> where
+ for<'a> <T::RegistrationData as ForLt>::Of<'a>: Send
+{
+}
-impl<T: Driver> Drop for Registration<T> {
+impl<T: Driver> Drop for Registration<'_, T> {
fn drop(&mut self) {
// Use `drm_dev_unplug` rather than `drm_dev_unregister` to ensure that existing
// `drm_dev_enter()` critical sections complete before unregistration proceeds. This
@@ -207,6 +263,9 @@ fn drop(&mut self) {
//
// SAFETY: Safe by the invariant of `ARef<drm::Device<T>>`. The existence of this
// `Registration` also guarantees that this `drm::Device` is actually registered.
- unsafe { bindings::drm_dev_unplug(self.0.as_raw()) };
+ unsafe { bindings::drm_dev_unplug(self.drm.as_raw()) };
+ // After drm_dev_unplug(), the SRCU barrier guarantees that all UnbindGuard critical
+ // sections have completed, so no one holds a reference to reg_data anymore.
+ // reg_data is dropped here automatically.
}
}
diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
index a66e7166f66b..59870bb09de2 100644
--- a/rust/kernel/drm/mod.rs
+++ b/rust/kernel/drm/mod.rs
@@ -12,6 +12,7 @@
pub use self::device::Device;
pub use self::device::DeviceContext;
pub use self::device::Registered;
+pub use self::device::UnbindGuard;
pub use self::device::Uninit;
pub use self::device::UnregisteredDevice;
pub use self::driver::Driver;
--
2.54.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 3/7] rust: drm: Add RegistrationData to drm::Driver
2026-06-03 1:15 ` [PATCH v2 3/7] rust: drm: Add RegistrationData to drm::Driver Danilo Krummrich
@ 2026-06-03 11:51 ` Gary Guo
2026-06-03 22:24 ` Danilo Krummrich
2026-06-03 23:29 ` Deborah Brouwer
1 sibling, 1 reply; 13+ messages in thread
From: Gary Guo @ 2026-06-03 11:51 UTC (permalink / raw)
To: Danilo Krummrich, aliceryhl, daniel.almeida, acourbot, ecourtney,
ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
On Wed Jun 3, 2026 at 2:15 AM BST, Danilo Krummrich wrote:
> Add a RegistrationData associated type to drm::Driver. This is a ForLt
> type whose lifetime is tied to the parent bus device binding scope.
>
> Registration<'a, T> takes ownership of the data via Pin<KBox<_>>,
> storing it with its real lifetime. The pointer is written to drm::Device
> before drm_dev_register() to ensure it is already in place when ioctls
> arrive.
>
> UnbindGuard::registration_data_with() provides access with the lifetime
> shortened from 'static via ForLt::cast_ref_unchecked. Since
> Registration::drop() calls drm_dev_unplug(), which performs an SRCU
> barrier waiting for all drm_dev_enter() critical sections to complete,
> the data is guaranteed to remain valid for the duration of any
> UnbindGuard.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
> drivers/gpu/drm/nova/driver.rs | 16 +++--
> drivers/gpu/drm/tyr/driver.rs | 16 +++--
> rust/kernel/drm/device.rs | 49 +++++++++++++
> rust/kernel/drm/driver.rs | 123 ++++++++++++++++++++++++---------
> rust/kernel/drm/mod.rs | 1 +
> 5 files changed, 162 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
> index c5b0313006bd..4267e6e6dbb4 100644
> --- a/drivers/gpu/drm/nova/driver.rs
> +++ b/drivers/gpu/drm/nova/driver.rs
> @@ -12,7 +12,8 @@
> ioctl, //
> },
> prelude::*,
> - sync::aref::ARef, //
> + sync::aref::ARef,
> + types::ForLt, //
> };
>
> use crate::file::File;
> @@ -20,9 +21,10 @@
>
> pub(crate) struct NovaDriver;
>
> -pub(crate) struct Nova {
> +pub(crate) struct Nova<'bound> {
> #[expect(unused)]
> drm: ARef<drm::Device<NovaDriver>>,
> + _reg: drm::Registration<'bound, NovaDriver>,
> }
>
> /// Convienence type alias for the DRM device type for this driver
> @@ -56,7 +58,7 @@ pub(crate) struct NovaData {
>
> impl auxiliary::Driver for NovaDriver {
> type IdInfo = ();
> - type Data<'bound> = Nova;
> + type Data<'bound> = Nova<'bound>;
> const ID_TABLE: auxiliary::IdTable<Self::IdInfo> = &AUX_TABLE;
>
> fn probe<'bound>(
> @@ -66,15 +68,19 @@ fn probe<'bound>(
> let data = try_pin_init!(NovaData { adev: adev.into() });
>
> let drm = drm::UnregisteredDevice::<Self>::new(adev, data)?;
> - let drm = drm::Registration::new_foreign_owned(drm, adev.as_ref(), 0)?;
> + let reg = drm::Registration::new(adev.as_ref(), drm, (), 0)?;
>
> - Ok(Nova { drm: drm.into() })
> + Ok(Nova {
> + drm: reg.device().into(),
> + _reg: reg,
> + })
> }
> }
>
> #[vtable]
> impl drm::Driver for NovaDriver {
> type Data = NovaData;
> + type RegistrationData = ForLt!(());
> type File = File;
> type Object<Ctx: drm::DeviceContext> = gem::Object<NovaObject, Ctx>;
> type ParentDevice<Ctx: DeviceContext> = auxiliary::Device<Ctx>;
> diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
> index 338c25ccc151..819f64a7573d 100644
> --- a/drivers/gpu/drm/tyr/driver.rs
> +++ b/drivers/gpu/drm/tyr/driver.rs
> @@ -31,7 +31,8 @@
> aref::ARef,
> Mutex, //
> },
> - time, //
> + time,
> + types::ForLt, //
> };
>
> use crate::{
> @@ -52,8 +53,9 @@
> pub(crate) struct TyrPlatformDriver;
>
> #[pin_data(PinnedDrop)]
> -pub(crate) struct TyrPlatformDriverData {
> +pub(crate) struct TyrPlatformDriverData<'bound> {
> _device: ARef<TyrDrmDevice>,
> + _reg: drm::Registration<'bound, TyrDrmDriver>,
> }
>
> #[pin_data]
> @@ -98,7 +100,7 @@ fn issue_soft_reset(dev: &Device, iomem: &IoMem<'_>) -> Result {
>
> impl platform::Driver for TyrPlatformDriver {
> type IdInfo = ();
> - type Data<'bound> = TyrPlatformDriverData;
> + type Data<'bound> = TyrPlatformDriverData<'bound>;
> const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
>
> fn probe<'bound>(
> @@ -150,10 +152,11 @@ fn probe<'bound>(
> });
>
> let tdev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev, data)?;
> - let tdev = drm::driver::Registration::new_foreign_owned(tdev, pdev.as_ref(), 0)?;
> + let reg = drm::Registration::new(pdev.as_ref(), tdev, (), 0)?;
>
> let driver = TyrPlatformDriverData {
> - _device: tdev.into(),
> + _device: reg.device().into(),
> + _reg: reg,
> };
>
> // We need this to be dev_info!() because dev_dbg!() does not work at
> @@ -164,7 +167,7 @@ fn probe<'bound>(
> }
>
> #[pinned_drop]
> -impl PinnedDrop for TyrPlatformDriverData {
> +impl PinnedDrop for TyrPlatformDriverData<'_> {
> fn drop(self: Pin<&mut Self>) {}
> }
>
> @@ -181,6 +184,7 @@ fn drop(self: Pin<&mut Self>) {}
> #[vtable]
> impl drm::Driver for TyrDrmDriver {
> type Data = TyrDrmDeviceData;
> + type RegistrationData = ForLt!(());
> type File = TyrDrmFileData;
> type Object<R: drm::DeviceContext> = drm::gem::shmem::Object<BoData, R>;
> type ParentDevice<Ctx: DeviceContext> = platform::Device<Ctx>;
> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> index 828618ae19af..9e5e069b5135 100644
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs
> @@ -23,6 +23,7 @@
> AlwaysRefCounted, //
> },
> types::{
> + ForLt,
> NotThreadSafe,
> Opaque, //
> },
> @@ -35,6 +36,7 @@
> };
> use core::{
> alloc::Layout,
> + cell::UnsafeCell,
> marker::PhantomData,
> mem,
> ops::Deref,
> @@ -259,6 +261,9 @@ pub fn new(
> // SAFETY: `drm_dev` is still private to this function.
> unsafe { (*drm_dev).driver = const { &Self::VTABLE } };
>
> + // SAFETY: `raw_drm` is valid; no concurrent access before registration.
> + unsafe { (*raw_drm.as_ptr()).registration_data = UnsafeCell::new(NonNull::dangling()) };
> +
> // SAFETY: The reference count is one, and now we take ownership of that reference as a
> // `drm::Device`.
> // INVARIANT: We just created the device above, but have yet to call `drm_dev_register`.
> @@ -290,6 +295,7 @@ pub fn new(
> pub struct Device<T: drm::Driver, C: DeviceContext = Registered> {
> dev: Opaque<bindings::drm_device>,
> data: T::Data,
> + pub(super) registration_data: UnsafeCell<NonNull<<T::RegistrationData as ForLt>::Of<'static>>>,
> _ctx: PhantomData<C>,
> }
>
> @@ -298,6 +304,27 @@ pub(crate) fn as_raw(&self) -> *mut bindings::drm_device {
> self.dev.get()
> }
>
> + /// Returns a reference to the registration data with lifetime shortened from `'static`.
> + ///
> + /// # Safety
> + ///
> + /// The caller must ensure that:
> + ///
> + /// - The parent bus device is bound (e.g. by holding an active `drm_dev_enter()` critical
> + /// section via [`UnbindGuard`]).
> + /// - The returned reference is not exposed to code that can choose a concrete lifetime for
> + /// it, as that would be unsound for types that are invariant over their lifetime parameter
> + /// (e.g. it must be passed through an HRTB-bounded closure).
> + unsafe fn registration_data_unchecked(&self) -> &<T::RegistrationData as ForLt>::Of<'_> {
> + // SAFETY: Caller guarantees the parent bus device is bound, hence
> + // the pointer is valid.
> + let static_ref = unsafe { (*self.registration_data.get()).as_ref() };
> +
> + // SAFETY: Caller guarantees the reference is only used behind an HRTB, making the
> + // round-trip from `'static` sound regardless of variance.
> + unsafe { T::RegistrationData::cast_ref_unchecked(static_ref) }
> + }
> +
> /// # Safety
> ///
> /// `ptr` must be a valid pointer to a `struct device` embedded in `Self`.
> @@ -412,6 +439,28 @@ pub struct UnbindGuard<'a, T: drm::Driver> {
> idx: i32,
> }
>
> +impl<T: drm::Driver> UnbindGuard<'_, T> {
> + /// Access the parent device and registration data through a closure, with both lifetimes
> + /// tied to the closure scope.
> + ///
> + /// The data is owned by [`Registration`](drm::Registration) and is guaranteed to remain valid
> + /// for the duration of this guard, since [`Registration`](drm::Registration)'s `drop` calls
> + /// `drm_dev_unplug()` which waits for all `drm_dev_enter()` critical sections to complete.
> + pub fn registration_data_with<R, F>(&self, f: F) -> R
> + where
> + F: for<'a> FnOnce(
> + &'a T::ParentDevice<device::Bound>,
> + &'a <T::RegistrationData as ForLt>::Of<'a>,
> + ) -> R,
> + {
> + // SAFETY: We hold an active `drm_dev_enter()` critical section, so the parent bus
> + // device is guaranteed to be bound. The closure's HRTB `for<'a>` prevents the caller
> + // from smuggling in references with a concrete short lifetime, satisfying the lifetime
> + // requirement of `registration_data_unchecked`.
> + f(&**self, unsafe { self.dev.registration_data_unchecked() })
> + }
> +}
> +
> impl<T: drm::Driver> Deref for UnbindGuard<'_, T> {
> type Target = T::ParentDevice<device::Bound>;
>
> diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
> index f68a17d8939d..6aa1cb99cc7f 100644
> --- a/rust/kernel/drm/driver.rs
> +++ b/rust/kernel/drm/driver.rs
> @@ -7,11 +7,11 @@
> use crate::{
> bindings,
> device,
> - devres,
> drm,
> error::to_result,
> prelude::*,
> - sync::aref::ARef, //
> + sync::aref::ARef,
> + types::ForLt, //
> };
> use core::{
> mem,
> @@ -110,6 +110,16 @@ pub trait Driver {
> /// Context data associated with the DRM driver
> type Data: Sync + Send;
>
> + /// Data owned by the [`Registration`] and accessible through [`drm::device::UnbindGuard`].
> + ///
> + /// This is a [`ForLt`](trait@ForLt) type whose lifetime is tied to the parent bus
> + /// device binding scope.
> + /// The data is only accessible while the parent bus device is bound (i.e. within a
> + /// `drm_dev_enter/exit` critical section), and references handed out by
> + /// [`UnbindGuard::registration_data_with()`](drm::device::UnbindGuard::registration_data_with)
> + /// ties the lifetime to the closure scope.
> + type RegistrationData: ForLt;
> +
> /// The type used to manage memory for this driver.
> type Object<Ctx: drm::DeviceContext>: AllocImpl;
>
> @@ -139,12 +149,57 @@ pub trait Driver {
> /// The registration type of a `drm::Device`.
> ///
> /// Once the `Registration` structure is dropped, the device is unregistered.
> -pub struct Registration<T: Driver>(ARef<drm::Device<T>>);
> +pub struct Registration<'a, T: Driver> {
> + drm: ARef<drm::Device<T>>,
> + _reg_data: Pin<KBox<<T::RegistrationData as ForLt>::Of<'a>>>,
> +}
> +
> +impl<'a, T: Driver> Registration<'a, T>
> +where
> + for<'b> <T::RegistrationData as ForLt>::Of<'b>: Send,
> +{
> + /// Register a new [`UnregisteredDevice`](drm::UnregisteredDevice) with userspace.
> + ///
> + /// # Safety
> + ///
> + /// The caller must not `mem::forget()` the returned [`Registration`] or otherwise prevent its
> + /// [`Drop`] implementation from running, since the registration data may contain borrowed
> + /// references that become invalid after `'a` ends.
> + ///
> + /// If the registration data is `'static`, use the safe [`Registration::new()`] instead.
> + pub unsafe fn new_with_lt<E>(
> + dev: &'a device::Device<device::Bound>,
> + drm: drm::UnregisteredDevice<T>,
> + reg_data: impl PinInit<<T::RegistrationData as ForLt>::Of<'a>, E>,
> + flags: usize,
> + ) -> Result<Self>
> + where
> + Error: From<E>,
> + {
> + if drm.as_ref().as_raw() != dev.as_raw() {
> + return Err(EINVAL);
> + }
>
> -impl<T: Driver> Registration<T> {
> - fn new(drm: drm::UnregisteredDevice<T>, flags: usize) -> Result<Self> {
> - // SAFETY: `drm.as_raw()` is valid by the invariants of `drm::Device`.
> - to_result(unsafe { bindings::drm_dev_register(drm.as_raw(), flags) })?;
> + let reg_data: Pin<KBox<<T::RegistrationData as ForLt>::Of<'a>>> =
> + KBox::pin_init(reg_data, GFP_KERNEL)?;
> +
> + // Store the registration data pointer in the device before registration, so that it is
> + // visible once ioctls can be called.
> + //
> + // SAFETY: Lifetimes do not affect layout, so the pointer cast is sound.
> + let ptr: NonNull<<T::RegistrationData as ForLt>::Of<'static>> =
> + unsafe { mem::transmute(NonNull::from(Pin::get_ref(reg_data.as_ref()))) };
> +
> + // SAFETY: No concurrent access; the device is not yet registered.
> + unsafe { *drm.registration_data.get() = ptr };
> +
> + // SAFETY: `drm` is a valid, initialized but not yet registered DRM device.
> + let ret = unsafe { bindings::drm_dev_register(drm.as_raw(), flags) };
> + if let Err(e) = to_result(ret) {
> + // SAFETY: No concurrent access; registration failed.
> + unsafe { *drm.registration_data.get() = NonNull::dangling() };
> + return Err(e);
> + }
>
> // SAFETY: We just called `drm_dev_register` above
> let new = NonNull::from(unsafe { drm.assume_ctx() });
> @@ -156,48 +211,49 @@ fn new(drm: drm::UnregisteredDevice<T>, flags: usize) -> Result<Self> {
> // one reference to the device - which we take ownership over here.
> let new = unsafe { ARef::from_raw(new) };
>
> - Ok(Self(new))
> + Ok(Self {
> + drm: new,
> + _reg_data: reg_data,
> + })
> }
>
> - /// Registers a new [`UnregisteredDevice`](drm::UnregisteredDevice) with userspace.
> - ///
> - /// Ownership of the [`Registration`] object is passed to [`devres::register`].
> - pub fn new_foreign_owned<'a>(
> - drm: drm::UnregisteredDevice<T>,
> + /// Safe variant of [`Registration::new_with_lt()`] for registration data that does not contain
> + /// borrowed references.
> + pub fn new<E>(
This is currently unsound, as leaking the unbind guard also gives out
`&Device<Bound>` in addition to the registration data.
I think we should just remove the not pass `&Device<Bound>` to ioctl callbacks.
Giving back registration data is sufficient; if a device driver needs
`&Device<Bound>` it can just store a reference in its registration data; more
commonly I suspect it will just store whatever device resource is needed and
doesn't need `&Device<Bound>` (with the introduction of lifetime, we have much
fewer cases that we actually need `&Device<Bound>` and cannot be replaced with a
direct reference to the device resource).
Not passing this bound device allows us to make this safe, and also remove the
need of patch 1 and patch 5. Actually, your patch 7 is a good demonstration of
the pattern that I described :)
Best,
Gary
> dev: &'a device::Device<device::Bound>,
> + drm: drm::UnregisteredDevice<T>,
> + reg_data: impl PinInit<<T::RegistrationData as ForLt>::Of<'a>, E>,
> flags: usize,
> - ) -> Result<&'a drm::Device<T>>
> + ) -> Result<Self>
> where
> - T: 'static,
> + <T::RegistrationData as ForLt>::Of<'a>: 'static,
> + Error: From<E>,
> {
> - if drm.as_ref().as_raw() != dev.as_raw() {
> - return Err(EINVAL);
> - }
> -
> - let reg = Registration::<T>::new(drm, flags)?;
> - let drm = NonNull::from(reg.device());
> -
> - devres::register(dev, reg, GFP_KERNEL)?;
> -
> - // SAFETY: Since `reg` was passed to devres::register(), the device now owns the lifetime
> - // of the DRM registration - ensuring that this references lives for at least as long as 'a.
> - Ok(unsafe { drm.as_ref() })
> + // SAFETY: `Of<'a>: 'static` guarantees the data contains no borrowed references,
> + // so forgetting the `Registration` cannot cause use-after-free.
> + unsafe { Self::new_with_lt(dev, drm, reg_data, flags) }
> }
>
> /// Returns a reference to the `Device` instance for this registration.
> pub fn device(&self) -> &drm::Device<T> {
> - &self.0
> + &self.drm
> }
> }
>
> // SAFETY: `Registration` doesn't offer any methods or access to fields when shared between
> // threads, hence it's safe to share it.
> -unsafe impl<T: Driver> Sync for Registration<T> {}
> +unsafe impl<T: Driver> Sync for Registration<'_, T> where
> + for<'a> <T::RegistrationData as ForLt>::Of<'a>: Send
> +{
> +}
>
> // SAFETY: Registration with and unregistration from the DRM subsystem can happen from any thread.
> -unsafe impl<T: Driver> Send for Registration<T> {}
> +unsafe impl<T: Driver> Send for Registration<'_, T> where
> + for<'a> <T::RegistrationData as ForLt>::Of<'a>: Send
> +{
> +}
>
> -impl<T: Driver> Drop for Registration<T> {
> +impl<T: Driver> Drop for Registration<'_, T> {
> fn drop(&mut self) {
> // Use `drm_dev_unplug` rather than `drm_dev_unregister` to ensure that existing
> // `drm_dev_enter()` critical sections complete before unregistration proceeds. This
> @@ -207,6 +263,9 @@ fn drop(&mut self) {
> //
> // SAFETY: Safe by the invariant of `ARef<drm::Device<T>>`. The existence of this
> // `Registration` also guarantees that this `drm::Device` is actually registered.
> - unsafe { bindings::drm_dev_unplug(self.0.as_raw()) };
> + unsafe { bindings::drm_dev_unplug(self.drm.as_raw()) };
> + // After drm_dev_unplug(), the SRCU barrier guarantees that all UnbindGuard critical
> + // sections have completed, so no one holds a reference to reg_data anymore.
> + // reg_data is dropped here automatically.
> }
> }
> diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
> index a66e7166f66b..59870bb09de2 100644
> --- a/rust/kernel/drm/mod.rs
> +++ b/rust/kernel/drm/mod.rs
> @@ -12,6 +12,7 @@
> pub use self::device::Device;
> pub use self::device::DeviceContext;
> pub use self::device::Registered;
> +pub use self::device::UnbindGuard;
> pub use self::device::Uninit;
> pub use self::device::UnregisteredDevice;
> pub use self::driver::Driver;
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 3/7] rust: drm: Add RegistrationData to drm::Driver
2026-06-03 11:51 ` Gary Guo
@ 2026-06-03 22:24 ` Danilo Krummrich
2026-06-03 22:36 ` Gary Guo
0 siblings, 1 reply; 13+ messages in thread
From: Danilo Krummrich @ 2026-06-03 22:24 UTC (permalink / raw)
To: Gary Guo
Cc: aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda, boqun,
bjorn3_gh, lossin, a.hindborg, tmgross, deborah.brouwer,
boris.brezillon, driver-core, linux-kernel, nova-gpu, dri-devel,
rust-for-linux
On Wed Jun 3, 2026 at 1:51 PM CEST, Gary Guo wrote:
>> + /// Safe variant of [`Registration::new_with_lt()`] for registration data that does not contain
>> + /// borrowed references.
>> + pub fn new<E>(
>
> This is currently unsound, as leaking the unbind guard also gives out
> `&Device<Bound>` in addition to the registration data.
For this to be unsound someone would need to be able to move the
drm::Registration into a context where its Drop runs independently of the driver
unbind, because otherwise leaking the UnbindGuard would also block driver unbind
forever and the now unconstraint &Device<Bound> remains valid.
So, I assume you refer to the case where someone calls forget() on the
drm::Registration that was created without the promise not to do so, i.e. new().
> I think we should just remove the not pass `&Device<Bound>` to ioctl callbacks.
> Giving back registration data is sufficient; if a device driver needs
> `&Device<Bound>` it can just store a reference in its registration data; more
> commonly I suspect it will just store whatever device resource is needed and
> doesn't need `&Device<Bound>` (with the introduction of lifetime, we have much
> fewer cases that we actually need `&Device<Bound>` and cannot be replaced with a
> direct reference to the device resource).
>
> Not passing this bound device allows us to make this safe, and also remove the
> need of patch 1 and patch 5.
I follow your reasoning, but not passing T::ParentDevice<Bound> in the ioctl
makes Registration::new() rather pointless on its own; given that it takes
T: 'static you can't store &'a T::ParentDevice<Bound> in the first place.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/7] rust: drm: Add RegistrationData to drm::Driver
2026-06-03 22:24 ` Danilo Krummrich
@ 2026-06-03 22:36 ` Gary Guo
0 siblings, 0 replies; 13+ messages in thread
From: Gary Guo @ 2026-06-03 22:36 UTC (permalink / raw)
To: Danilo Krummrich, Gary Guo
Cc: aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda, boqun,
bjorn3_gh, lossin, a.hindborg, tmgross, deborah.brouwer,
boris.brezillon, driver-core, linux-kernel, nova-gpu, dri-devel,
rust-for-linux
On Wed Jun 3, 2026 at 11:24 PM BST, Danilo Krummrich wrote:
> On Wed Jun 3, 2026 at 1:51 PM CEST, Gary Guo wrote:
>>> + /// Safe variant of [`Registration::new_with_lt()`] for registration data that does not contain
>>> + /// borrowed references.
>>> + pub fn new<E>(
>>
>> This is currently unsound, as leaking the unbind guard also gives out
>> `&Device<Bound>` in addition to the registration data.
>
> For this to be unsound someone would need to be able to move the
> drm::Registration into a context where its Drop runs independently of the driver
> unbind, because otherwise leaking the UnbindGuard would also block driver unbind
> forever and the now unconstraint &Device<Bound> remains valid.
>
> So, I assume you refer to the case where someone calls forget() on the
> drm::Registration that was created without the promise not to do so, i.e. new().
Right, I was indeed thinking about this.
>
>> I think we should just remove the not pass `&Device<Bound>` to ioctl callbacks.
>> Giving back registration data is sufficient; if a device driver needs
>> `&Device<Bound>` it can just store a reference in its registration data; more
>> commonly I suspect it will just store whatever device resource is needed and
>> doesn't need `&Device<Bound>` (with the introduction of lifetime, we have much
>> fewer cases that we actually need `&Device<Bound>` and cannot be replaced with a
>> direct reference to the device resource).
>>
>> Not passing this bound device allows us to make this safe, and also remove the
>> need of patch 1 and patch 5.
>
> I follow your reasoning, but not passing T::ParentDevice<Bound> in the ioctl
> makes Registration::new() rather pointless on its own; given that it takes
> T: 'static you can't store &'a T::ParentDevice<Bound> in the first place.
I suppose you could create a DRM device without it backed by any real hardware
resoruces :)
Joke aside, I think my actual point is that with the current approach, and the
fact that `UnbindGuard` serializes with drop of `drm::Registration`,
`&Device<Bound>` is not really inherent to the design anymore. This can follow
the same pattern as other registration data and there's no need to bring
`&Device<Bound>` into the mix.
You might be right the `new` might not be very useful without storing device
resources; in that case I think you could just remove `new` and have the
`new_with_lt` be the only API.
Best,
Gary
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/7] rust: drm: Add RegistrationData to drm::Driver
2026-06-03 1:15 ` [PATCH v2 3/7] rust: drm: Add RegistrationData to drm::Driver Danilo Krummrich
2026-06-03 11:51 ` Gary Guo
@ 2026-06-03 23:29 ` Deborah Brouwer
1 sibling, 0 replies; 13+ messages in thread
From: Deborah Brouwer @ 2026-06-03 23:29 UTC (permalink / raw)
To: Danilo Krummrich
Cc: aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda, boqun,
gary, bjorn3_gh, lossin, a.hindborg, tmgross, boris.brezillon,
driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
On Wed, Jun 03, 2026 at 03:15:45AM +0200, Danilo Krummrich wrote:
> Add a RegistrationData associated type to drm::Driver. This is a ForLt
> type whose lifetime is tied to the parent bus device binding scope.
>
> Registration<'a, T> takes ownership of the data via Pin<KBox<_>>,
> storing it with its real lifetime. The pointer is written to drm::Device
> before drm_dev_register() to ensure it is already in place when ioctls
> arrive.
>
> UnbindGuard::registration_data_with() provides access with the lifetime
> shortened from 'static via ForLt::cast_ref_unchecked. Since
> Registration::drop() calls drm_dev_unplug(), which performs an SRCU
> barrier waiting for all drm_dev_enter() critical sections to complete,
> the data is guaranteed to remain valid for the duration of any
> UnbindGuard.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
> drivers/gpu/drm/nova/driver.rs | 16 +++--
> drivers/gpu/drm/tyr/driver.rs | 16 +++--
> rust/kernel/drm/device.rs | 49 +++++++++++++
> rust/kernel/drm/driver.rs | 123 ++++++++++++++++++++++++---------
> rust/kernel/drm/mod.rs | 1 +
> 5 files changed, 162 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
> index c5b0313006bd..4267e6e6dbb4 100644
> --- a/drivers/gpu/drm/nova/driver.rs
> +++ b/drivers/gpu/drm/nova/driver.rs
> @@ -12,7 +12,8 @@
> ioctl, //
> },
> prelude::*,
> - sync::aref::ARef, //
> + sync::aref::ARef,
> + types::ForLt, //
> };
>
> use crate::file::File;
> @@ -20,9 +21,10 @@
>
> pub(crate) struct NovaDriver;
>
> -pub(crate) struct Nova {
> +pub(crate) struct Nova<'bound> {
> #[expect(unused)]
> drm: ARef<drm::Device<NovaDriver>>,
> + _reg: drm::Registration<'bound, NovaDriver>,
> }
>
> /// Convienence type alias for the DRM device type for this driver
> @@ -56,7 +58,7 @@ pub(crate) struct NovaData {
>
> impl auxiliary::Driver for NovaDriver {
> type IdInfo = ();
> - type Data<'bound> = Nova;
> + type Data<'bound> = Nova<'bound>;
> const ID_TABLE: auxiliary::IdTable<Self::IdInfo> = &AUX_TABLE;
>
> fn probe<'bound>(
> @@ -66,15 +68,19 @@ fn probe<'bound>(
> let data = try_pin_init!(NovaData { adev: adev.into() });
>
> let drm = drm::UnregisteredDevice::<Self>::new(adev, data)?;
> - let drm = drm::Registration::new_foreign_owned(drm, adev.as_ref(), 0)?;
> + let reg = drm::Registration::new(adev.as_ref(), drm, (), 0)?;
>
> - Ok(Nova { drm: drm.into() })
> + Ok(Nova {
> + drm: reg.device().into(),
> + _reg: reg,
> + })
> }
> }
>
> #[vtable]
> impl drm::Driver for NovaDriver {
> type Data = NovaData;
> + type RegistrationData = ForLt!(());
> type File = File;
> type Object<Ctx: drm::DeviceContext> = gem::Object<NovaObject, Ctx>;
> type ParentDevice<Ctx: DeviceContext> = auxiliary::Device<Ctx>;
> diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
> index 338c25ccc151..819f64a7573d 100644
> --- a/drivers/gpu/drm/tyr/driver.rs
> +++ b/drivers/gpu/drm/tyr/driver.rs
> @@ -31,7 +31,8 @@
> aref::ARef,
> Mutex, //
> },
> - time, //
> + time,
> + types::ForLt, //
> };
>
> use crate::{
> @@ -52,8 +53,9 @@
> pub(crate) struct TyrPlatformDriver;
>
> #[pin_data(PinnedDrop)]
> -pub(crate) struct TyrPlatformDriverData {
> +pub(crate) struct TyrPlatformDriverData<'bound> {
> _device: ARef<TyrDrmDevice>,
> + _reg: drm::Registration<'bound, TyrDrmDriver>,
> }
>
> #[pin_data]
> @@ -98,7 +100,7 @@ fn issue_soft_reset(dev: &Device, iomem: &IoMem<'_>) -> Result {
>
> impl platform::Driver for TyrPlatformDriver {
> type IdInfo = ();
> - type Data<'bound> = TyrPlatformDriverData;
> + type Data<'bound> = TyrPlatformDriverData<'bound>;
> const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
>
> fn probe<'bound>(
> @@ -150,10 +152,11 @@ fn probe<'bound>(
> });
>
> let tdev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev, data)?;
> - let tdev = drm::driver::Registration::new_foreign_owned(tdev, pdev.as_ref(), 0)?;
> + let reg = drm::Registration::new(pdev.as_ref(), tdev, (), 0)?;
Hi Danilo, could we use separate data arguments in UnregisteredDevice
vs in the Registration? Basically we want to use the UnregisteredDevice
to initialize and boot the firmware which we then store as registration
data.
Could you have a look at this patch, it applies on top of this series:
https://lore.kernel.org/rust-for-linux/20260603-use_tyr_reg_data-v1-1-97f64e951cf6@collabora.com/
Deborah
>
> let driver = TyrPlatformDriverData {
> - _device: tdev.into(),
> + _device: reg.device().into(),
> + _reg: reg,
> };
>
> // We need this to be dev_info!() because dev_dbg!() does not work at
> @@ -164,7 +167,7 @@ fn probe<'bound>(
> }
>
> #[pinned_drop]
> -impl PinnedDrop for TyrPlatformDriverData {
> +impl PinnedDrop for TyrPlatformDriverData<'_> {
> fn drop(self: Pin<&mut Self>) {}
> }
>
> @@ -181,6 +184,7 @@ fn drop(self: Pin<&mut Self>) {}
> #[vtable]
> impl drm::Driver for TyrDrmDriver {
> type Data = TyrDrmDeviceData;
> + type RegistrationData = ForLt!(());
> type File = TyrDrmFileData;
> type Object<R: drm::DeviceContext> = drm::gem::shmem::Object<BoData, R>;
> type ParentDevice<Ctx: DeviceContext> = platform::Device<Ctx>;
> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> index 828618ae19af..9e5e069b5135 100644
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs
> @@ -23,6 +23,7 @@
> AlwaysRefCounted, //
> },
> types::{
> + ForLt,
> NotThreadSafe,
> Opaque, //
> },
> @@ -35,6 +36,7 @@
> };
> use core::{
> alloc::Layout,
> + cell::UnsafeCell,
> marker::PhantomData,
> mem,
> ops::Deref,
> @@ -259,6 +261,9 @@ pub fn new(
> // SAFETY: `drm_dev` is still private to this function.
> unsafe { (*drm_dev).driver = const { &Self::VTABLE } };
>
> + // SAFETY: `raw_drm` is valid; no concurrent access before registration.
> + unsafe { (*raw_drm.as_ptr()).registration_data = UnsafeCell::new(NonNull::dangling()) };
> +
> // SAFETY: The reference count is one, and now we take ownership of that reference as a
> // `drm::Device`.
> // INVARIANT: We just created the device above, but have yet to call `drm_dev_register`.
> @@ -290,6 +295,7 @@ pub fn new(
> pub struct Device<T: drm::Driver, C: DeviceContext = Registered> {
> dev: Opaque<bindings::drm_device>,
> data: T::Data,
> + pub(super) registration_data: UnsafeCell<NonNull<<T::RegistrationData as ForLt>::Of<'static>>>,
> _ctx: PhantomData<C>,
> }
>
> @@ -298,6 +304,27 @@ pub(crate) fn as_raw(&self) -> *mut bindings::drm_device {
> self.dev.get()
> }
>
> + /// Returns a reference to the registration data with lifetime shortened from `'static`.
> + ///
> + /// # Safety
> + ///
> + /// The caller must ensure that:
> + ///
> + /// - The parent bus device is bound (e.g. by holding an active `drm_dev_enter()` critical
> + /// section via [`UnbindGuard`]).
> + /// - The returned reference is not exposed to code that can choose a concrete lifetime for
> + /// it, as that would be unsound for types that are invariant over their lifetime parameter
> + /// (e.g. it must be passed through an HRTB-bounded closure).
> + unsafe fn registration_data_unchecked(&self) -> &<T::RegistrationData as ForLt>::Of<'_> {
> + // SAFETY: Caller guarantees the parent bus device is bound, hence
> + // the pointer is valid.
> + let static_ref = unsafe { (*self.registration_data.get()).as_ref() };
> +
> + // SAFETY: Caller guarantees the reference is only used behind an HRTB, making the
> + // round-trip from `'static` sound regardless of variance.
> + unsafe { T::RegistrationData::cast_ref_unchecked(static_ref) }
> + }
> +
> /// # Safety
> ///
> /// `ptr` must be a valid pointer to a `struct device` embedded in `Self`.
> @@ -412,6 +439,28 @@ pub struct UnbindGuard<'a, T: drm::Driver> {
> idx: i32,
> }
>
> +impl<T: drm::Driver> UnbindGuard<'_, T> {
> + /// Access the parent device and registration data through a closure, with both lifetimes
> + /// tied to the closure scope.
> + ///
> + /// The data is owned by [`Registration`](drm::Registration) and is guaranteed to remain valid
> + /// for the duration of this guard, since [`Registration`](drm::Registration)'s `drop` calls
> + /// `drm_dev_unplug()` which waits for all `drm_dev_enter()` critical sections to complete.
> + pub fn registration_data_with<R, F>(&self, f: F) -> R
> + where
> + F: for<'a> FnOnce(
> + &'a T::ParentDevice<device::Bound>,
> + &'a <T::RegistrationData as ForLt>::Of<'a>,
> + ) -> R,
> + {
> + // SAFETY: We hold an active `drm_dev_enter()` critical section, so the parent bus
> + // device is guaranteed to be bound. The closure's HRTB `for<'a>` prevents the caller
> + // from smuggling in references with a concrete short lifetime, satisfying the lifetime
> + // requirement of `registration_data_unchecked`.
> + f(&**self, unsafe { self.dev.registration_data_unchecked() })
> + }
> +}
> +
> impl<T: drm::Driver> Deref for UnbindGuard<'_, T> {
> type Target = T::ParentDevice<device::Bound>;
>
> diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
> index f68a17d8939d..6aa1cb99cc7f 100644
> --- a/rust/kernel/drm/driver.rs
> +++ b/rust/kernel/drm/driver.rs
> @@ -7,11 +7,11 @@
> use crate::{
> bindings,
> device,
> - devres,
> drm,
> error::to_result,
> prelude::*,
> - sync::aref::ARef, //
> + sync::aref::ARef,
> + types::ForLt, //
> };
> use core::{
> mem,
> @@ -110,6 +110,16 @@ pub trait Driver {
> /// Context data associated with the DRM driver
> type Data: Sync + Send;
>
> + /// Data owned by the [`Registration`] and accessible through [`drm::device::UnbindGuard`].
> + ///
> + /// This is a [`ForLt`](trait@ForLt) type whose lifetime is tied to the parent bus
> + /// device binding scope.
> + /// The data is only accessible while the parent bus device is bound (i.e. within a
> + /// `drm_dev_enter/exit` critical section), and references handed out by
> + /// [`UnbindGuard::registration_data_with()`](drm::device::UnbindGuard::registration_data_with)
> + /// ties the lifetime to the closure scope.
> + type RegistrationData: ForLt;
> +
> /// The type used to manage memory for this driver.
> type Object<Ctx: drm::DeviceContext>: AllocImpl;
>
> @@ -139,12 +149,57 @@ pub trait Driver {
> /// The registration type of a `drm::Device`.
> ///
> /// Once the `Registration` structure is dropped, the device is unregistered.
> -pub struct Registration<T: Driver>(ARef<drm::Device<T>>);
> +pub struct Registration<'a, T: Driver> {
> + drm: ARef<drm::Device<T>>,
> + _reg_data: Pin<KBox<<T::RegistrationData as ForLt>::Of<'a>>>,
> +}
> +
> +impl<'a, T: Driver> Registration<'a, T>
> +where
> + for<'b> <T::RegistrationData as ForLt>::Of<'b>: Send,
> +{
> + /// Register a new [`UnregisteredDevice`](drm::UnregisteredDevice) with userspace.
> + ///
> + /// # Safety
> + ///
> + /// The caller must not `mem::forget()` the returned [`Registration`] or otherwise prevent its
> + /// [`Drop`] implementation from running, since the registration data may contain borrowed
> + /// references that become invalid after `'a` ends.
> + ///
> + /// If the registration data is `'static`, use the safe [`Registration::new()`] instead.
> + pub unsafe fn new_with_lt<E>(
> + dev: &'a device::Device<device::Bound>,
> + drm: drm::UnregisteredDevice<T>,
> + reg_data: impl PinInit<<T::RegistrationData as ForLt>::Of<'a>, E>,
> + flags: usize,
> + ) -> Result<Self>
> + where
> + Error: From<E>,
> + {
> + if drm.as_ref().as_raw() != dev.as_raw() {
> + return Err(EINVAL);
> + }
>
> -impl<T: Driver> Registration<T> {
> - fn new(drm: drm::UnregisteredDevice<T>, flags: usize) -> Result<Self> {
> - // SAFETY: `drm.as_raw()` is valid by the invariants of `drm::Device`.
> - to_result(unsafe { bindings::drm_dev_register(drm.as_raw(), flags) })?;
> + let reg_data: Pin<KBox<<T::RegistrationData as ForLt>::Of<'a>>> =
> + KBox::pin_init(reg_data, GFP_KERNEL)?;
> +
> + // Store the registration data pointer in the device before registration, so that it is
> + // visible once ioctls can be called.
> + //
> + // SAFETY: Lifetimes do not affect layout, so the pointer cast is sound.
> + let ptr: NonNull<<T::RegistrationData as ForLt>::Of<'static>> =
> + unsafe { mem::transmute(NonNull::from(Pin::get_ref(reg_data.as_ref()))) };
> +
> + // SAFETY: No concurrent access; the device is not yet registered.
> + unsafe { *drm.registration_data.get() = ptr };
> +
> + // SAFETY: `drm` is a valid, initialized but not yet registered DRM device.
> + let ret = unsafe { bindings::drm_dev_register(drm.as_raw(), flags) };
> + if let Err(e) = to_result(ret) {
> + // SAFETY: No concurrent access; registration failed.
> + unsafe { *drm.registration_data.get() = NonNull::dangling() };
> + return Err(e);
> + }
>
> // SAFETY: We just called `drm_dev_register` above
> let new = NonNull::from(unsafe { drm.assume_ctx() });
> @@ -156,48 +211,49 @@ fn new(drm: drm::UnregisteredDevice<T>, flags: usize) -> Result<Self> {
> // one reference to the device - which we take ownership over here.
> let new = unsafe { ARef::from_raw(new) };
>
> - Ok(Self(new))
> + Ok(Self {
> + drm: new,
> + _reg_data: reg_data,
> + })
> }
>
> - /// Registers a new [`UnregisteredDevice`](drm::UnregisteredDevice) with userspace.
> - ///
> - /// Ownership of the [`Registration`] object is passed to [`devres::register`].
> - pub fn new_foreign_owned<'a>(
> - drm: drm::UnregisteredDevice<T>,
> + /// Safe variant of [`Registration::new_with_lt()`] for registration data that does not contain
> + /// borrowed references.
> + pub fn new<E>(
> dev: &'a device::Device<device::Bound>,
> + drm: drm::UnregisteredDevice<T>,
> + reg_data: impl PinInit<<T::RegistrationData as ForLt>::Of<'a>, E>,
> flags: usize,
> - ) -> Result<&'a drm::Device<T>>
> + ) -> Result<Self>
> where
> - T: 'static,
> + <T::RegistrationData as ForLt>::Of<'a>: 'static,
> + Error: From<E>,
> {
> - if drm.as_ref().as_raw() != dev.as_raw() {
> - return Err(EINVAL);
> - }
> -
> - let reg = Registration::<T>::new(drm, flags)?;
> - let drm = NonNull::from(reg.device());
> -
> - devres::register(dev, reg, GFP_KERNEL)?;
> -
> - // SAFETY: Since `reg` was passed to devres::register(), the device now owns the lifetime
> - // of the DRM registration - ensuring that this references lives for at least as long as 'a.
> - Ok(unsafe { drm.as_ref() })
> + // SAFETY: `Of<'a>: 'static` guarantees the data contains no borrowed references,
> + // so forgetting the `Registration` cannot cause use-after-free.
> + unsafe { Self::new_with_lt(dev, drm, reg_data, flags) }
> }
>
> /// Returns a reference to the `Device` instance for this registration.
> pub fn device(&self) -> &drm::Device<T> {
> - &self.0
> + &self.drm
> }
> }
>
> // SAFETY: `Registration` doesn't offer any methods or access to fields when shared between
> // threads, hence it's safe to share it.
> -unsafe impl<T: Driver> Sync for Registration<T> {}
> +unsafe impl<T: Driver> Sync for Registration<'_, T> where
> + for<'a> <T::RegistrationData as ForLt>::Of<'a>: Send
> +{
> +}
>
> // SAFETY: Registration with and unregistration from the DRM subsystem can happen from any thread.
> -unsafe impl<T: Driver> Send for Registration<T> {}
> +unsafe impl<T: Driver> Send for Registration<'_, T> where
> + for<'a> <T::RegistrationData as ForLt>::Of<'a>: Send
> +{
> +}
>
> -impl<T: Driver> Drop for Registration<T> {
> +impl<T: Driver> Drop for Registration<'_, T> {
> fn drop(&mut self) {
> // Use `drm_dev_unplug` rather than `drm_dev_unregister` to ensure that existing
> // `drm_dev_enter()` critical sections complete before unregistration proceeds. This
> @@ -207,6 +263,9 @@ fn drop(&mut self) {
> //
> // SAFETY: Safe by the invariant of `ARef<drm::Device<T>>`. The existence of this
> // `Registration` also guarantees that this `drm::Device` is actually registered.
> - unsafe { bindings::drm_dev_unplug(self.0.as_raw()) };
> + unsafe { bindings::drm_dev_unplug(self.drm.as_raw()) };
> + // After drm_dev_unplug(), the SRCU barrier guarantees that all UnbindGuard critical
> + // sections have completed, so no one holds a reference to reg_data anymore.
> + // reg_data is dropped here automatically.
> }
> }
> diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
> index a66e7166f66b..59870bb09de2 100644
> --- a/rust/kernel/drm/mod.rs
> +++ b/rust/kernel/drm/mod.rs
> @@ -12,6 +12,7 @@
> pub use self::device::Device;
> pub use self::device::DeviceContext;
> pub use self::device::Registered;
> +pub use self::device::UnbindGuard;
> pub use self::device::Uninit;
> pub use self::device::UnregisteredDevice;
> pub use self::driver::Driver;
> --
> 2.54.0
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 4/7] rust: drm: Wrap ioctl dispatch in UnbindGuard
2026-06-03 1:15 [PATCH v2 0/7] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
` (2 preceding siblings ...)
2026-06-03 1:15 ` [PATCH v2 3/7] rust: drm: Add RegistrationData to drm::Driver Danilo Krummrich
@ 2026-06-03 1:15 ` Danilo Krummrich
2026-06-03 1:15 ` [PATCH v2 5/7] rust: drm: Pass bound parent device to ioctl handlers Danilo Krummrich
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Danilo Krummrich @ 2026-06-03 1:15 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Run every ioctl handler inside a drm_dev_enter/exit critical section via
UnbindGuard. If the device has been unplugged, the ioctl returns ENODEV
without calling the handler.
A never-called closure anchors the driver type for the compiler by tying
dev's type to the handler's first parameter, which the compiler cannot
infer through method resolution and associated-type projections alone.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
rust/kernel/drm/ioctl.rs | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
index cf328101dde4..8ee6e8ea0ff6 100644
--- a/rust/kernel/drm/ioctl.rs
+++ b/rust/kernel/drm/ioctl.rs
@@ -87,7 +87,10 @@ pub mod internal {
/// file: &kernel::drm::File<Self::File>,
/// ) -> Result<u32>
/// ```
-/// where `Self` is the drm::drv::Driver implementation these ioctls are being declared within.
+/// where `Self` is the `drm::Driver` implementation these ioctls are being declared within.
+///
+/// The ioctl runs inside a `drm_dev_enter/exit` critical section. If the device has been
+/// unplugged, the ioctl returns `ENODEV` without calling the handler.
///
/// # Examples
///
@@ -134,7 +137,19 @@ macro_rules! declare_drm_ioctls {
// FIXME: Currently there is nothing enforcing that the types of the
// dev/file match the current driver these ioctls are being declared
// for, and it's not clear how to enforce this within the type system.
- let dev = $crate::drm::device::Device::from_raw(raw_dev);
+ let dev = unsafe {
+ $crate::drm::device::Device::from_raw(raw_dev)
+ };
+
+ // Type-inference anchor: the closure is never called but ties `dev`'s
+ // type to `$func`'s first parameter, which the compiler cannot infer
+ // through method resolution and associated-type projections alone.
+ #[allow(unreachable_code)]
+ let _ = || $func(dev, unreachable!(), unreachable!());
+
+ let Some(_guard) = dev.unbind_guard() else {
+ return $crate::error::code::ENODEV.to_errno();
+ };
// SAFETY: The ioctl argument has size `_IOC_SIZE(cmd)`, which we
// asserted above matches the size of this type, and all bit patterns of
// UAPI structs must be valid.
--
2.54.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 5/7] rust: drm: Pass bound parent device to ioctl handlers
2026-06-03 1:15 [PATCH v2 0/7] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
` (3 preceding siblings ...)
2026-06-03 1:15 ` [PATCH v2 4/7] rust: drm: Wrap ioctl dispatch in UnbindGuard Danilo Krummrich
@ 2026-06-03 1:15 ` Danilo Krummrich
2026-06-03 1:15 ` [PATCH v2 6/7] rust: drm: Pass registration data " Danilo Krummrich
2026-06-03 1:15 ` [PATCH v2 7/7] drm: nova: convert to use DRM registration data Danilo Krummrich
6 siblings, 0 replies; 13+ messages in thread
From: Danilo Krummrich @ 2026-06-03 1:15 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Pass &Self::ParentDevice<Bound> as the second argument to ioctl
handlers. This gives ioctl handlers direct access to the bound parent
bus device.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
drivers/gpu/drm/nova/file.rs | 5 +++++
drivers/gpu/drm/tyr/file.rs | 3 +++
rust/kernel/drm/ioctl.rs | 7 ++++---
3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index a3b7bd36792c..d43f92ee181d 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -4,6 +4,8 @@
use crate::gem::NovaObject;
use kernel::{
alloc::flags::*,
+ auxiliary,
+ device::Bound,
drm::{self, gem::BaseObject},
pci,
prelude::*,
@@ -24,6 +26,7 @@ impl File {
/// IOCTL: get_param: Query GPU / driver metadata.
pub(crate) fn get_param(
dev: &NovaDevice,
+ _adev: &auxiliary::Device<Bound>,
getparam: &mut uapi::drm_nova_getparam,
_file: &drm::File<File>,
) -> Result<u32> {
@@ -44,6 +47,7 @@ pub(crate) fn get_param(
/// IOCTL: gem_create: Create a new DRM GEM object.
pub(crate) fn gem_create(
dev: &NovaDevice,
+ _adev: &auxiliary::Device<Bound>,
req: &mut uapi::drm_nova_gem_create,
file: &drm::File<File>,
) -> Result<u32> {
@@ -57,6 +61,7 @@ pub(crate) fn gem_create(
/// IOCTL: gem_info: Query GEM metadata.
pub(crate) fn gem_info(
_dev: &NovaDevice,
+ _adev: &auxiliary::Device<Bound>,
req: &mut uapi::drm_nova_gem_info,
file: &drm::File<File>,
) -> Result<u32> {
diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
index 31411da203c5..181475f18c18 100644
--- a/drivers/gpu/drm/tyr/file.rs
+++ b/drivers/gpu/drm/tyr/file.rs
@@ -1,7 +1,9 @@
// SPDX-License-Identifier: GPL-2.0 or MIT
use kernel::{
+ device,
drm,
+ platform,
prelude::*,
uaccess::UserSlice,
uapi, //
@@ -29,6 +31,7 @@ fn open(_dev: &drm::Device<Self::Driver>) -> Result<Pin<KBox<Self>>> {
impl TyrDrmFileData {
pub(crate) fn dev_query(
ddev: &TyrDrmDevice,
+ _pdev: &platform::Device<device::Bound>,
devquery: &mut uapi::drm_panthor_dev_query,
_file: &TyrDrmFile,
) -> Result<u32> {
diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
index 8ee6e8ea0ff6..2229dc55764b 100644
--- a/rust/kernel/drm/ioctl.rs
+++ b/rust/kernel/drm/ioctl.rs
@@ -83,6 +83,7 @@ pub mod internal {
///
/// ```ignore
/// fn foo(device: &kernel::drm::Device<Self>,
+/// parent: &Self::ParentDevice<kernel::device::Bound>,
/// data: &mut uapi::argument_type,
/// file: &kernel::drm::File<Self::File>,
/// ) -> Result<u32>
@@ -145,9 +146,9 @@ macro_rules! declare_drm_ioctls {
// type to `$func`'s first parameter, which the compiler cannot infer
// through method resolution and associated-type projections alone.
#[allow(unreachable_code)]
- let _ = || $func(dev, unreachable!(), unreachable!());
+ let _ = || $func(dev, unreachable!(), unreachable!(), unreachable!());
- let Some(_guard) = dev.unbind_guard() else {
+ let Some(guard) = dev.unbind_guard() else {
return $crate::error::code::ENODEV.to_errno();
};
// SAFETY: The ioctl argument has size `_IOC_SIZE(cmd)`, which we
@@ -162,7 +163,7 @@ macro_rules! declare_drm_ioctls {
// SAFETY: This is just the DRM file structure
let file = unsafe { $crate::drm::File::from_raw(raw_file) };
- match $func(dev, data, file) {
+ match $func(dev, &*guard, data, file) {
Err(e) => e.to_errno(),
Ok(i) => i.try_into()
.unwrap_or($crate::error::code::ERANGE.to_errno()),
--
2.54.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 6/7] rust: drm: Pass registration data to ioctl handlers
2026-06-03 1:15 [PATCH v2 0/7] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
` (4 preceding siblings ...)
2026-06-03 1:15 ` [PATCH v2 5/7] rust: drm: Pass bound parent device to ioctl handlers Danilo Krummrich
@ 2026-06-03 1:15 ` Danilo Krummrich
2026-06-03 1:15 ` [PATCH v2 7/7] drm: nova: convert to use DRM registration data Danilo Krummrich
6 siblings, 0 replies; 13+ messages in thread
From: Danilo Krummrich @ 2026-06-03 1:15 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Pass registration data to ioctl handlers via
UnbindGuard::registration_data_with(). The closure's HRTB ties the
lifetime to the closure scope, and ForLt::cast_ref_unchecked shortens it
from 'static internally. The reference is valid for the duration of the
drm_dev_enter/exit critical section held by UnbindGuard.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
drivers/gpu/drm/nova/file.rs | 3 +++
drivers/gpu/drm/tyr/file.rs | 1 +
rust/kernel/drm/ioctl.rs | 15 +++++++++++++--
3 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index d43f92ee181d..df9760947d58 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -27,6 +27,7 @@ impl File {
pub(crate) fn get_param(
dev: &NovaDevice,
_adev: &auxiliary::Device<Bound>,
+ _reg_data: &(),
getparam: &mut uapi::drm_nova_getparam,
_file: &drm::File<File>,
) -> Result<u32> {
@@ -48,6 +49,7 @@ pub(crate) fn get_param(
pub(crate) fn gem_create(
dev: &NovaDevice,
_adev: &auxiliary::Device<Bound>,
+ _reg_data: &(),
req: &mut uapi::drm_nova_gem_create,
file: &drm::File<File>,
) -> Result<u32> {
@@ -62,6 +64,7 @@ pub(crate) fn gem_create(
pub(crate) fn gem_info(
_dev: &NovaDevice,
_adev: &auxiliary::Device<Bound>,
+ _reg_data: &(),
req: &mut uapi::drm_nova_gem_info,
file: &drm::File<File>,
) -> Result<u32> {
diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
index 181475f18c18..9f53da7633ab 100644
--- a/drivers/gpu/drm/tyr/file.rs
+++ b/drivers/gpu/drm/tyr/file.rs
@@ -32,6 +32,7 @@ impl TyrDrmFileData {
pub(crate) fn dev_query(
ddev: &TyrDrmDevice,
_pdev: &platform::Device<device::Bound>,
+ _reg_data: &(),
devquery: &mut uapi::drm_panthor_dev_query,
_file: &TyrDrmFile,
) -> Result<u32> {
diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
index 2229dc55764b..773683da7d6a 100644
--- a/rust/kernel/drm/ioctl.rs
+++ b/rust/kernel/drm/ioctl.rs
@@ -84,6 +84,7 @@ pub mod internal {
/// ```ignore
/// fn foo(device: &kernel::drm::Device<Self>,
/// parent: &Self::ParentDevice<kernel::device::Bound>,
+/// reg_data: &<Self::RegistrationData as kernel::types::ForLt>::Of<'_>,
/// data: &mut uapi::argument_type,
/// file: &kernel::drm::File<Self::File>,
/// ) -> Result<u32>
@@ -146,7 +147,15 @@ macro_rules! declare_drm_ioctls {
// type to `$func`'s first parameter, which the compiler cannot infer
// through method resolution and associated-type projections alone.
#[allow(unreachable_code)]
- let _ = || $func(dev, unreachable!(), unreachable!(), unreachable!());
+ let _ = || {
+ $func(
+ dev,
+ unreachable!(),
+ unreachable!(),
+ unreachable!(),
+ unreachable!(),
+ )
+ };
let Some(guard) = dev.unbind_guard() else {
return $crate::error::code::ENODEV.to_errno();
@@ -163,7 +172,9 @@ macro_rules! declare_drm_ioctls {
// SAFETY: This is just the DRM file structure
let file = unsafe { $crate::drm::File::from_raw(raw_file) };
- match $func(dev, &*guard, data, file) {
+ match guard.registration_data_with(|parent, reg_data| {
+ $func(dev, parent, reg_data, data, file)
+ }) {
Err(e) => e.to_errno(),
Ok(i) => i.try_into()
.unwrap_or($crate::error::code::ERANGE.to_errno()),
--
2.54.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 7/7] drm: nova: convert to use DRM registration data
2026-06-03 1:15 [PATCH v2 0/7] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
` (5 preceding siblings ...)
2026-06-03 1:15 ` [PATCH v2 6/7] rust: drm: Pass registration data " Danilo Krummrich
@ 2026-06-03 1:15 ` Danilo Krummrich
6 siblings, 0 replies; 13+ messages in thread
From: Danilo Krummrich @ 2026-06-03 1:15 UTC (permalink / raw)
To: dakr, aliceryhl, daniel.almeida, acourbot, ecourtney, ojeda,
boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross,
deborah.brouwer, boris.brezillon
Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux
Move the auxiliary device reference from drm::Device data into
RegistrationData, replacing the ARef<auxiliary::Device> with a borrowed
&'bound auxiliary::Device<Bound>. This makes the data lifetime-aware and
exercises the registration data path through the ioctl handlers.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
drivers/gpu/drm/nova/driver.rs | 20 ++++++++++----------
drivers/gpu/drm/nova/file.rs | 19 +++++++++++--------
2 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index 4267e6e6dbb4..f46d7ff6fee3 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -3,6 +3,7 @@
use kernel::{
auxiliary,
device::{
+ Bound,
Core,
DeviceContext, //
},
@@ -13,7 +14,7 @@
},
prelude::*,
sync::aref::ARef,
- types::ForLt, //
+ types::CovariantForLt, //
};
use crate::file::File;
@@ -30,9 +31,8 @@ pub(crate) struct Nova<'bound> {
/// Convienence type alias for the DRM device type for this driver
pub(crate) type NovaDevice<Ctx = drm::Registered> = drm::Device<NovaDriver, Ctx>;
-#[pin_data]
-pub(crate) struct NovaData {
- pub(crate) adev: ARef<auxiliary::Device>,
+pub(crate) struct NovaData<'bound> {
+ pub(crate) adev: &'bound auxiliary::Device<Bound>,
}
const INFO: drm::DriverInfo = drm::DriverInfo {
@@ -65,10 +65,10 @@ fn probe<'bound>(
adev: &'bound auxiliary::Device<Core<'_>>,
_info: &'bound Self::IdInfo,
) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
- let data = try_pin_init!(NovaData { adev: adev.into() });
-
- let drm = drm::UnregisteredDevice::<Self>::new(adev, data)?;
- let reg = drm::Registration::new(adev.as_ref(), drm, (), 0)?;
+ let drm = drm::UnregisteredDevice::<Self>::new(adev, Ok(()))?;
+ let reg_data = NovaData { adev };
+ // SAFETY: We never bypass the destructor of `reg`.
+ let reg = unsafe { drm::Registration::new_with_lt(adev.as_ref(), drm, reg_data, 0)? };
Ok(Nova {
drm: reg.device().into(),
@@ -79,8 +79,8 @@ fn probe<'bound>(
#[vtable]
impl drm::Driver for NovaDriver {
- type Data = NovaData;
- type RegistrationData = ForLt!(());
+ type Data = ();
+ type RegistrationData = CovariantForLt!(NovaData<'_>);
type File = File;
type Object<Ctx: drm::DeviceContext> = gem::Object<NovaObject, Ctx>;
type ParentDevice<Ctx: DeviceContext> = auxiliary::Device<Ctx>;
diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index df9760947d58..cd6c37b01e22 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -1,6 +1,10 @@
// SPDX-License-Identifier: GPL-2.0
-use crate::driver::{NovaDevice, NovaDriver};
+use crate::driver::{
+ NovaData,
+ NovaDevice,
+ NovaDriver, //
+};
use crate::gem::NovaObject;
use kernel::{
alloc::flags::*,
@@ -25,15 +29,14 @@ fn open(_dev: &NovaDevice) -> Result<Pin<KBox<Self>>> {
impl File {
/// IOCTL: get_param: Query GPU / driver metadata.
pub(crate) fn get_param(
- dev: &NovaDevice,
+ _dev: &NovaDevice,
_adev: &auxiliary::Device<Bound>,
- _reg_data: &(),
+ reg_data: &NovaData<'_>,
getparam: &mut uapi::drm_nova_getparam,
_file: &drm::File<File>,
) -> Result<u32> {
- let adev = &dev.adev;
- let parent = adev.parent();
- let pdev: &pci::Device = parent.try_into()?;
+ let parent = reg_data.adev.parent();
+ let pdev: &pci::Device<Bound> = parent.try_into()?;
let value = match getparam.param as u32 {
uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => pdev.resource_len(1)?,
@@ -49,7 +52,7 @@ pub(crate) fn get_param(
pub(crate) fn gem_create(
dev: &NovaDevice,
_adev: &auxiliary::Device<Bound>,
- _reg_data: &(),
+ _reg_data: &NovaData<'_>,
req: &mut uapi::drm_nova_gem_create,
file: &drm::File<File>,
) -> Result<u32> {
@@ -64,7 +67,7 @@ pub(crate) fn gem_create(
pub(crate) fn gem_info(
_dev: &NovaDevice,
_adev: &auxiliary::Device<Bound>,
- _reg_data: &(),
+ _reg_data: &NovaData<'_>,
req: &mut uapi::drm_nova_gem_info,
file: &drm::File<File>,
) -> Result<u32> {
--
2.54.0
^ permalink raw reply related [flat|nested] 13+ messages in thread