mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/7] lifetime-parameterized DRM File private data
@ 2026-11-07  2:03 Danilo Krummrich
  2026-11-07  2:03 ` [PATCH v2 1/7] rust: drm: rename Ioctl device context to Userspace Danilo Krummrich
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-11-07  2:03 UTC (permalink / raw)
  To: dakr, aliceryhl, acourbot, daniel.almeida, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, tamird, work, brauner,
	lyude, j, alvin.sun, deborah.brouwer, laura.nao, beata.michalska
  Cc: nova-gpu, dri-devel, rust-for-linux, linux-kernel

DRM file private data is currently 'static, preventing drivers from
borrowing registration-scoped resources in their per-file state. This
series makes Driver::File lifetime-parameterized, so file data can borrow
from RegistrationData.

File data is guaranteed to be dropped before registration data, either
when the file is closed normally or when the device is unregistered,
whichever comes first. To enforce this during unbind, Registration::drop()
revokes all open file data and waits for its destruction before dropping
registration data.

fops.open is wrapped in a RegistrationGuard (drm_dev_enter/exit) so that
after drm_dev_unplug() all successfully opened files are accounted for.
GEM open/close callbacks are similarly gated to prevent access to stale
file data after unbind.

The series is based on drm-rust-next with [1] applied; a branch with the
patches is available in [2].

[1] https://lore.kernel.org/all/20261105005951.756157-1-dakr@kernel.org/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=drm/file

Changes in v2:
- Replace ForLt with Driver::File<'a>; use inner_with() for file-data access
  and allow returning borrowed fields.
- Use Opaque<bindings::drm_file> during teardown.
- Drop file data outside filelist_mutex; also clean up file data on
  registration failure.
- Reject cross-device GEM handles.
- Clarify safety documentation and add inline annotations.

Danilo Krummrich (7):
  rust: drm: rename Ioctl device context to Userspace
  rust: drm: gem: gate open/close callbacks with RegistrationGuard
  rust: drm: move file_operations from gem to device
  rust: fs: add iminor() helper
  rust: drm: wrap fops open with RegistrationGuard
  rust: drm: return impl PinInit from DriverFile::open()
  rust: drm: make Driver::File lifetime-parameterized

 drivers/gpu/drm/nova/driver.rs |   2 +-
 drivers/gpu/drm/nova/file.rs   |  16 +--
 drivers/gpu/drm/nova/gem.rs    |   8 +-
 drivers/gpu/drm/tyr/driver.rs  |   2 +-
 drivers/gpu/drm/tyr/file.rs    |  13 ++-
 rust/helpers/fs.c              |   5 +
 rust/kernel/drm/device.rs      | 141 ++++++++++++++++++++++----
 rust/kernel/drm/driver.rs      | 113 +++++++++++++++++++--
 rust/kernel/drm/file.rs        | 179 ++++++++++++++++++++++++---------
 rust/kernel/drm/gem/mod.rs     |  80 +++++++++------
 rust/kernel/drm/gem/shmem.rs   |  11 +-
 rust/kernel/drm/ioctl.rs       |  18 ++--
 rust/kernel/drm/mod.rs         |   2 +-
 13 files changed, 454 insertions(+), 136 deletions(-)


base-commit: cef0cd809e564cca9ac824a369c995ab7e0d16f3
-- 
2.56.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/7] rust: drm: rename Ioctl device context to Userspace
  2026-11-07  2:03 [PATCH v2 0/7] lifetime-parameterized DRM File private data Danilo Krummrich
@ 2026-11-07  2:03 ` Danilo Krummrich
  2026-11-07  2:03 ` [PATCH v2 2/7] rust: drm: gem: gate open/close callbacks with RegistrationGuard Danilo Krummrich
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-11-07  2:03 UTC (permalink / raw)
  To: dakr, aliceryhl, acourbot, daniel.almeida, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, tamird, work, brauner,
	lyude, j, alvin.sun, deborah.brouwer, laura.nao, beata.michalska
  Cc: nova-gpu, dri-devel, rust-for-linux, linux-kernel

The Ioctl DeviceContext typestate represents a device that has been
registered with userspace at some point. This context is not specific to
ioctl dispatch; it applies equally to GEM handle callbacks, mmap,
fdinfo, and any other operation triggered by userspace on a registered
device.

Rename it to Userspace to accurately reflect its semantics.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/drm/device.rs | 21 ++++++++++++---------
 rust/kernel/drm/ioctl.rs  | 12 ++++++------
 rust/kernel/drm/mod.rs    |  2 +-
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 6b88ade28e24..dd1c83fef323 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -79,12 +79,12 @@ macro_rules! drm_legacy_fields {
 ///
 /// - [`Normal`]: The general-purpose, reference-counted context. A [`Device`] in this context may
 ///   or may not be registered with userspace.
-/// - [`Ioctl`]: The device has been registered with userspace at some point; used in ioctl
-///   dispatch context.
+/// - [`Userspace`]: The device has been registered with userspace at some point; used in
+///   callbacks triggered by userspace operations.
 /// - [`Registered`]: The device is currently registered with userspace and the parent bus device
 ///   is bound.
 ///
-/// Both `Device<T, Ioctl>` and `Device<T, Registered>` dereference to `Device<T>` ([`Normal`]),
+/// Both `Device<T, Userspace>` and `Device<T, Registered>` dereference to `Device<T>` ([`Normal`]),
 /// so any method available on a [`Normal`] device is also available in the other contexts.
 pub trait DeviceContext: Sealed + Send + Sync + 'static {}
 
@@ -120,14 +120,17 @@ impl DeviceContext for Registered {}
 /// unregistering or already unregistered. `drm_dev_enter()` can guard against this, ensuring the
 /// device remains registered for the duration of the critical section.
 ///
+/// This context is used for all callbacks triggered by userspace operations: ioctls, GEM handle
+/// management, mmap, fdinfo, etc.
+///
 /// # Invariants
 ///
 /// A [`Device`] in this context has been registered with userspace via `drm_dev_register()` at
 /// some point.
-pub struct Ioctl;
+pub struct Userspace;
 
-impl Sealed for Ioctl {}
-impl DeviceContext for Ioctl {}
+impl Sealed for Userspace {}
+impl DeviceContext for Userspace {}
 
 /// A [`Device`] which is known at compile-time to be unregistered with userspace.
 ///
@@ -344,7 +347,7 @@ pub(crate) unsafe fn assume_ctx<NewCtx: DeviceContext>(&self) -> &Device<T, NewC
     }
 }
 
-impl<T: drm::Driver> Device<T, Ioctl> {
+impl<T: drm::Driver> Device<T, Userspace> {
     /// Guard against the parent bus device being unbound.
     ///
     /// Returns a [`RegistrationGuard`] if the device has not been unplugged, [`None`] otherwise.
@@ -467,12 +470,12 @@ fn deref(&self) -> &Self::Target {
     }
 }
 
-impl<T: drm::Driver> Deref for Device<T, Ioctl> {
+impl<T: drm::Driver> Deref for Device<T, Userspace> {
     type Target = Device<T>;
 
     #[inline]
     fn deref(&self) -> &Self::Target {
-        // SAFETY: The caller holds a `Device<T, Ioctl>`, which guarantees all invariants
+        // SAFETY: The caller holds a `Device<T, Userspace>`, which guarantees all invariants
         // of the weaker `Normal` context.
         unsafe { self.assume_ctx() }
     }
diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
index 64af9eacc306..9934b23c36eb 100644
--- a/rust/kernel/drm/ioctl.rs
+++ b/rust/kernel/drm/ioctl.rs
@@ -71,14 +71,14 @@ pub mod internal {
     pub use bindings::drm_file;
     pub use bindings::drm_ioctl_desc;
 
-    /// Cast an [`Ioctl`] DRM device pointer to [`Registered`], preserving the driver type
+    /// Cast a [`Userspace`] DRM device pointer to [`Registered`], preserving the driver type
     /// parameter `T`.
     ///
     /// Used by [`declare_drm_ioctls!`] to anchor type inference.
     #[doc(hidden)]
     #[inline]
     pub const fn __dev_ctx_cast<T: crate::drm::Driver>(
-        ptr: *const crate::drm::Device<T, crate::drm::Ioctl>,
+        ptr: *const crate::drm::Device<T, crate::drm::Userspace>,
     ) -> *const crate::drm::Device<T, crate::drm::Registered> {
         ptr.cast()
     }
@@ -144,14 +144,14 @@ macro_rules! declare_drm_ioctls {
                             // - The DRM device must have been registered when we're called through
                             //   an IOCTL.
                             //
-                            // INVARIANT: The `Ioctl` context requires that the device has been
-                            // registered via `drm_dev_register()` at some point; the DRM core
-                            // guarantees this for ioctl dispatch callbacks.
+                            // INVARIANT: The `Userspace` context requires that the device has
+                            // been registered via `drm_dev_register()` at some point; the DRM
+                            // core guarantees this for ioctl dispatch callbacks.
                             //
                             // 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<_, $crate::drm::Ioctl> =
+                            let dev: &$crate::drm::device::Device<_, $crate::drm::Userspace> =
                                 $crate::drm::device::Device::from_raw(raw_dev);
 
                             // Type-inference anchor: the closure is never called but ties `dev`'s
diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
index fd6ed35bc35a..7fcf2465a82e 100644
--- a/rust/kernel/drm/mod.rs
+++ b/rust/kernel/drm/mod.rs
@@ -11,11 +11,11 @@
 
 pub use self::device::Device;
 pub use self::device::DeviceContext;
-pub use self::device::Ioctl;
 pub use self::device::Normal;
 pub use self::device::Registered;
 pub use self::device::RegistrationGuard;
 pub use self::device::UnregisteredDevice;
+pub use self::device::Userspace;
 pub use self::driver::Driver;
 pub use self::driver::DriverInfo;
 pub use self::driver::Registration;
-- 
2.56.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 2/7] rust: drm: gem: gate open/close callbacks with RegistrationGuard
  2026-11-07  2:03 [PATCH v2 0/7] lifetime-parameterized DRM File private data Danilo Krummrich
  2026-11-07  2:03 ` [PATCH v2 1/7] rust: drm: rename Ioctl device context to Userspace Danilo Krummrich
@ 2026-11-07  2:03 ` Danilo Krummrich
  2026-11-07  2:03 ` [PATCH v2 3/7] rust: drm: move file_operations from gem to device Danilo Krummrich
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-11-07  2:03 UTC (permalink / raw)
  To: dakr, aliceryhl, acourbot, daniel.almeida, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, tamird, work, brauner,
	lyude, j, alvin.sun, deborah.brouwer, laura.nao, beata.michalska
  Cc: nova-gpu, dri-devel, rust-for-linux, linux-kernel

Wrap the GEM object open and close callbacks with a RegistrationGuard
(drm_dev_enter / drm_dev_exit) to ensure the driver callbacks only run
while the parent bus device is bound.

If the device has been unbound, open returns -ENODEV and close silently
returns. This prevents driver code from accessing device resources after
unbind and is a prerequisite for making drm::Driver::File
lifetime-parameterized, since GEM callbacks receive a &drm::File that
could otherwise be used to access invalidated file private data.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/drm/gem/mod.rs | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index 85657a39e146..94355081edab 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -128,6 +128,14 @@ extern "C" fn open_callback<T: DriverObject>(
     raw_obj: *mut bindings::drm_gem_object,
     raw_file: *mut bindings::drm_file,
 ) -> core::ffi::c_int {
+    // SAFETY: `raw_obj` is a valid pointer to a `struct drm_gem_object` with a valid `dev`.
+    let dev: &drm::Device<T::Driver, drm::Userspace> =
+        unsafe { drm::Device::from_raw((*raw_obj).dev) };
+
+    let Some(_guard) = dev.registration_guard() else {
+        return ENODEV.to_errno();
+    };
+
     // SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`.
     let file = unsafe { DriverFile::<T>::from_raw(raw_file) };
 
@@ -148,6 +156,14 @@ extern "C" fn close_callback<T: DriverObject>(
     raw_obj: *mut bindings::drm_gem_object,
     raw_file: *mut bindings::drm_file,
 ) {
+    // SAFETY: `raw_obj` is a valid pointer to a `struct drm_gem_object` with a valid `dev`.
+    let dev: &drm::Device<T::Driver, drm::Userspace> =
+        unsafe { drm::Device::from_raw((*raw_obj).dev) };
+
+    let Some(_guard) = dev.registration_guard() else {
+        return;
+    };
+
     // SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`.
     let file = unsafe { DriverFile::<T>::from_raw(raw_file) };
 
-- 
2.56.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 3/7] rust: drm: move file_operations from gem to device
  2026-11-07  2:03 [PATCH v2 0/7] lifetime-parameterized DRM File private data Danilo Krummrich
  2026-11-07  2:03 ` [PATCH v2 1/7] rust: drm: rename Ioctl device context to Userspace Danilo Krummrich
  2026-11-07  2:03 ` [PATCH v2 2/7] rust: drm: gem: gate open/close callbacks with RegistrationGuard Danilo Krummrich
@ 2026-11-07  2:03 ` Danilo Krummrich
  2026-11-07  2:03 ` [PATCH v2 4/7] rust: fs: add iminor() helper Danilo Krummrich
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-11-07  2:03 UTC (permalink / raw)
  To: dakr, aliceryhl, acourbot, daniel.almeida, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, tamird, work, brauner,
	lyude, j, alvin.sun, deborah.brouwer, laura.nao, beata.michalska
  Cc: nova-gpu, dri-devel, rust-for-linux, linux-kernel

Move the file_operations definition from drm::gem::create_fops() into
UnregisteredDevice in drm::device. This is the file_operations of the
DRM minor device, set through the drm_driver struct. It is not specific
to GEM.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/drm/device.rs  | 23 ++++++++++++++++++++---
 rust/kernel/drm/gem/mod.rs | 20 --------------------
 2 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index dd1c83fef323..3340fa0dc46b 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -195,11 +195,28 @@ const fn compute_features() -> u32 {
         driver_features: Self::compute_features(),
         ioctls: T::IOCTLS.as_ptr(),
         num_ioctls: T::IOCTLS.len() as i32,
-        fops: &Self::GEM_FOPS,
+        fops: &Self::FOPS,
     };
 
-    const GEM_FOPS: bindings::file_operations =
-        drm::gem::create_fops(crate::module::this_module::<T::OwnerModule>().as_ptr());
+    const FOPS: bindings::file_operations = {
+        let mut fops: bindings::file_operations = pin_init::zeroed();
+
+        fops.owner = crate::module::this_module::<T::OwnerModule>().as_ptr();
+        fops.open = Some(bindings::drm_open);
+        fops.release = Some(bindings::drm_release);
+        fops.unlocked_ioctl = Some(bindings::drm_ioctl);
+        #[cfg(CONFIG_COMPAT)]
+        {
+            fops.compat_ioctl = Some(bindings::drm_compat_ioctl);
+        }
+        fops.poll = Some(bindings::drm_poll);
+        fops.read = Some(bindings::drm_read);
+        fops.llseek = Some(bindings::noop_llseek);
+        fops.mmap = Some(bindings::drm_gem_mmap);
+        fops.fop_flags = bindings::FOP_UNSIGNED_OFFSET;
+
+        fops
+    };
 
     /// Create a new `UnregisteredDevice` for a `drm::Driver`.
     ///
diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index 94355081edab..260f36817f96 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -388,23 +388,3 @@ impl<T: DriverObject, Ctx: DeviceContext> AllocImpl for Object<T, Ctx> {
         dumb_map_offset: None,
     };
 }
-
-pub(super) const fn create_fops(owner: *mut bindings::module) -> bindings::file_operations {
-    let mut fops: bindings::file_operations = pin_init::zeroed();
-
-    fops.owner = owner;
-    fops.open = Some(bindings::drm_open);
-    fops.release = Some(bindings::drm_release);
-    fops.unlocked_ioctl = Some(bindings::drm_ioctl);
-    #[cfg(CONFIG_COMPAT)]
-    {
-        fops.compat_ioctl = Some(bindings::drm_compat_ioctl);
-    }
-    fops.poll = Some(bindings::drm_poll);
-    fops.read = Some(bindings::drm_read);
-    fops.llseek = Some(bindings::noop_llseek);
-    fops.mmap = Some(bindings::drm_gem_mmap);
-    fops.fop_flags = bindings::FOP_UNSIGNED_OFFSET;
-
-    fops
-}
-- 
2.56.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 4/7] rust: fs: add iminor() helper
  2026-11-07  2:03 [PATCH v2 0/7] lifetime-parameterized DRM File private data Danilo Krummrich
                   ` (2 preceding siblings ...)
  2026-11-07  2:03 ` [PATCH v2 3/7] rust: drm: move file_operations from gem to device Danilo Krummrich
@ 2026-11-07  2:03 ` Danilo Krummrich
  2026-11-07  2:03 ` [PATCH v2 5/7] rust: drm: wrap fops open with RegistrationGuard Danilo Krummrich
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-11-07  2:03 UTC (permalink / raw)
  To: dakr, aliceryhl, acourbot, daniel.almeida, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, tamird, work, brauner,
	lyude, j, alvin.sun, deborah.brouwer, laura.nao, beata.michalska
  Cc: nova-gpu, dri-devel, rust-for-linux, linux-kernel

Add a Rust helper for iminor(), which extracts the minor number from a
struct inode. This is a static inline function in the C headers, so
bindgen does not generate a binding for it.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/helpers/fs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/rust/helpers/fs.c b/rust/helpers/fs.c
index 789d60fb8908..d6f9782db178 100644
--- a/rust/helpers/fs.c
+++ b/rust/helpers/fs.c
@@ -10,3 +10,8 @@ __rust_helper struct file *rust_helper_get_file(struct file *f)
 {
 	return get_file(f);
 }
+
+__rust_helper unsigned int rust_helper_iminor(const struct inode *inode)
+{
+	return iminor(inode);
+}
-- 
2.56.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 5/7] rust: drm: wrap fops open with RegistrationGuard
  2026-11-07  2:03 [PATCH v2 0/7] lifetime-parameterized DRM File private data Danilo Krummrich
                   ` (3 preceding siblings ...)
  2026-11-07  2:03 ` [PATCH v2 4/7] rust: fs: add iminor() helper Danilo Krummrich
@ 2026-11-07  2:03 ` Danilo Krummrich
  2026-11-07  2:03 ` [PATCH v2 6/7] rust: drm: return impl PinInit from DriverFile::open() Danilo Krummrich
  2026-11-07  2:03 ` [PATCH v2 7/7] rust: drm: make Driver::File lifetime-parameterized Danilo Krummrich
  6 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-11-07  2:03 UTC (permalink / raw)
  To: dakr, aliceryhl, acourbot, daniel.almeida, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, tamird, work, brauner,
	lyude, j, alvin.sun, deborah.brouwer, laura.nao, beata.michalska
  Cc: nova-gpu, dri-devel, rust-for-linux, linux-kernel

Add a Minor abstraction with RAII release and a fops_open() wrapper that
holds a RegistrationGuard (drm_dev_enter / drm_dev_exit) across the
entire drm_open() call.

This guarantees that drm_dev_unplug() in Registration::drop() waits for
the full open sequence to complete, so all files are visible in the
filelist when iterating for cleanup.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/drm/device.rs | 63 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 62 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 3340fa0dc46b..b8444499b39c 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -42,6 +42,42 @@
     },
 };
 
+/// A reference to a `struct drm_minor` with RAII release.
+struct Minor(NonNull<bindings::drm_minor>);
+
+// Methods use `#[inline(never)]` to prevent the unexported `drm_minor_acquire()` /
+// `drm_minor_release()` symbols from being inlined into driver modules.
+impl Minor {
+    /// Acquire a minor by ID. Increments the underlying device's refcount.
+    #[inline(never)]
+    fn acquire(minor_id: u32) -> Result<Self> {
+        // SAFETY: `drm_minors_xa` is a valid global xarray; any `minor_id` is safe to
+        // look up (returns ERR_PTR on failure).
+        let ptr =
+            unsafe { bindings::drm_minor_acquire(&raw mut bindings::drm_minors_xa, minor_id) };
+        Ok(Self(NonNull::new(from_err_ptr(ptr)?).ok_or(ENODEV)?))
+    }
+
+    /// Returns a reference to the DRM device for this minor.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that the minor belongs to a `Device<T>`.
+    unsafe fn device<T: drm::Driver>(&self) -> &Device<T, Userspace> {
+        // SAFETY: The minor is valid (from `drm_minor_acquire()`) and `minor->dev`
+        // is a valid `drm_device`. The caller guarantees it is a `Device<T>`.
+        unsafe { Device::from_raw((*self.0.as_ptr()).dev) }
+    }
+}
+
+impl Drop for Minor {
+    #[inline(never)]
+    fn drop(&mut self) {
+        // SAFETY: `self.0` came from `drm_minor_acquire()` and has not been released yet.
+        unsafe { bindings::drm_minor_release(self.0.as_ptr()) }
+    }
+}
+
 #[cfg(CONFIG_DRM_LEGACY)]
 macro_rules! drm_legacy_fields {
     ( $($field:ident: $val:expr),* $(,)? ) => {
@@ -198,11 +234,36 @@ const fn compute_features() -> u32 {
         fops: &Self::FOPS,
     };
 
+    /// Wrapper for `fops.open` that holds a [`RegistrationGuard`] across the entire `drm_open()`
+    /// call. This guarantees that `drm_dev_unplug()` in `Registration::drop()` waits for the full
+    /// open sequence.
+    extern "C" fn fops_open(inode: *mut bindings::inode, filp: *mut bindings::file) -> c_int {
+        let f = || -> Result<c_int> {
+            // SAFETY: `inode` is valid.
+            let minor_id = unsafe { bindings::iminor(inode) };
+            let minor = Minor::acquire(minor_id)?;
+
+            // SAFETY: `fops_open` is only installed for devices of type `T` (via `FOPS`).
+            let _guard = (unsafe { minor.device::<T>() })
+                .registration_guard()
+                .ok_or(ENODEV)?;
+
+            // SAFETY: `inode` and `filp` are valid. The RegistrationGuard ensures the entire
+            // `drm_open()` runs within the SRCU critical section.
+            Ok(unsafe { bindings::drm_open(inode, filp) })
+        };
+
+        match f() {
+            Ok(ret) => ret,
+            Err(e) => e.to_errno(),
+        }
+    }
+
     const FOPS: bindings::file_operations = {
         let mut fops: bindings::file_operations = pin_init::zeroed();
 
         fops.owner = crate::module::this_module::<T::OwnerModule>().as_ptr();
-        fops.open = Some(bindings::drm_open);
+        fops.open = Some(Self::fops_open);
         fops.release = Some(bindings::drm_release);
         fops.unlocked_ioctl = Some(bindings::drm_ioctl);
         #[cfg(CONFIG_COMPAT)]
-- 
2.56.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 6/7] rust: drm: return impl PinInit from DriverFile::open()
  2026-11-07  2:03 [PATCH v2 0/7] lifetime-parameterized DRM File private data Danilo Krummrich
                   ` (4 preceding siblings ...)
  2026-11-07  2:03 ` [PATCH v2 5/7] rust: drm: wrap fops open with RegistrationGuard Danilo Krummrich
@ 2026-11-07  2:03 ` Danilo Krummrich
  2026-11-07  2:03 ` [PATCH v2 7/7] rust: drm: make Driver::File lifetime-parameterized Danilo Krummrich
  6 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-11-07  2:03 UTC (permalink / raw)
  To: dakr, aliceryhl, acourbot, daniel.almeida, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, tamird, work, brauner,
	lyude, j, alvin.sun, deborah.brouwer, laura.nao, beata.michalska
  Cc: nova-gpu, dri-devel, rust-for-linux, linux-kernel

Change DriverFile::open() to return impl PinInit<Self, Error> instead of
Result<Pin<KBox<Self>>>, consistent with how bus device private data
works.

Drivers no longer need to allocate a Pin<KBox<_>> themselves; they just
return an initializer and the subsystem takes care of the allocation.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/gpu/drm/nova/file.rs | 5 ++---
 drivers/gpu/drm/tyr/file.rs  | 4 ++--
 rust/kernel/drm/file.rs      | 5 +++--
 rust/kernel/drm/gem/shmem.rs | 4 ++--
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index a05264917a0b..13642f83289d 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -7,7 +7,6 @@
 };
 use crate::gem::NovaObject;
 use kernel::{
-    alloc::flags::*,
     drm::{
         self,
         gem::BaseObject,
@@ -103,8 +102,8 @@ fn write_info<T: AsBytes>(info: &mut uapi::drm_nova_info, value: T) -> Result {
 impl drm::file::DriverFile for File {
     type Driver = NovaDriver;
 
-    fn open(_dev: &NovaDevice) -> Result<Pin<KBox<Self>>> {
-        Ok(KBox::new(Self, GFP_KERNEL)?.into())
+    fn open(_dev: &NovaDevice) -> impl PinInit<Self, Error> {
+        Ok(Self)
     }
 }
 
diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
index 9f60a90d4948..6850708cfcdd 100644
--- a/drivers/gpu/drm/tyr/file.rs
+++ b/drivers/gpu/drm/tyr/file.rs
@@ -25,8 +25,8 @@ pub(crate) struct TyrDrmFileData {}
 impl drm::file::DriverFile for TyrDrmFileData {
     type Driver = TyrDrmDriver;
 
-    fn open(_dev: &drm::Device<Self::Driver>) -> Result<Pin<KBox<Self>>> {
-        KBox::try_pin_init(try_pin_init!(Self {}), GFP_KERNEL)
+    fn open(_dev: &drm::Device<Self::Driver>) -> impl PinInit<Self, Error> {
+        Ok(Self {})
     }
 }
 
diff --git a/rust/kernel/drm/file.rs b/rust/kernel/drm/file.rs
index 10160601ce5a..bd5e4ca2977f 100644
--- a/rust/kernel/drm/file.rs
+++ b/rust/kernel/drm/file.rs
@@ -18,7 +18,7 @@ pub trait DriverFile {
     type Driver: drm::Driver;
 
     /// Open a new file (called when a client opens the DRM device).
-    fn open(device: &drm::Device<Self::Driver>) -> Result<Pin<KBox<Self>>>;
+    fn open(device: &drm::Device<Self::Driver>) -> impl PinInit<Self, Error>;
 }
 
 /// An open DRM File.
@@ -70,7 +70,8 @@ pub(crate) extern "C" fn open_callback(
         // SAFETY: `raw_file` is a valid pointer to a `struct drm_file`.
         let file = unsafe { File::<T>::from_raw(raw_file) };
 
-        let inner = match T::open(drm) {
+        let init = T::open(drm);
+        let inner = match KBox::try_pin_init(init, GFP_KERNEL) {
             Err(e) => {
                 return e.to_errno();
             }
diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs
index 24cb5343e0db..1a4f81e3b5b5 100644
--- a/rust/kernel/drm/gem/shmem.rs
+++ b/rust/kernel/drm/gem/shmem.rs
@@ -604,8 +604,8 @@ struct KunitObject {}
     impl drm::file::DriverFile for KunitFile {
         type Driver = KunitDriver;
 
-        fn open(_dev: &drm::Device<KunitDriver>) -> Result<Pin<KBox<Self>>> {
-            Ok(KBox::new(Self, GFP_KERNEL)?.into())
+        fn open(_dev: &drm::Device<KunitDriver>) -> impl PinInit<Self, Error> {
+            Ok(Self)
         }
     }
 
-- 
2.56.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 7/7] rust: drm: make Driver::File lifetime-parameterized
  2026-11-07  2:03 [PATCH v2 0/7] lifetime-parameterized DRM File private data Danilo Krummrich
                   ` (5 preceding siblings ...)
  2026-11-07  2:03 ` [PATCH v2 6/7] rust: drm: return impl PinInit from DriverFile::open() Danilo Krummrich
@ 2026-11-07  2:03 ` Danilo Krummrich
  6 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-11-07  2:03 UTC (permalink / raw)
  To: dakr, aliceryhl, acourbot, daniel.almeida, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, tamird, work, brauner,
	lyude, j, alvin.sun, deborah.brouwer, laura.nao, beata.michalska
  Cc: nova-gpu, dri-devel, rust-for-linux, linux-kernel

Make the DRM file private data lifetime-parameterized, allowing drivers
to borrow from RegistrationData in their per-file data.

Introduce DriverFile<'a> as a lifetime-parameterized trait that receives
both the device and registration data in open(). Parametrize File<D> on
the driver type rather than the file data type, deriving the concrete
file type through Driver::File<'a>.

Add inner_with() to access the driver file data from a File<D> reference.

Ensure file data is always dropped before registration data:

  - In Registration::drop(), detach driver_priv for all open files under
    filelist_mutex, then drop the detached data after releasing the mutex
    and wait for file-data destruction in concurrent postclose_callback()
    calls via an open_count / WaitQueue pair on drm::Device.

  - In postclose_callback(), skip the drop if driver_priv has already
    been NULLed by the filelist iteration. The NULL write is visible
    through the filelist_mutex acquire/release chain in
    drm_close_helper().

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/gpu/drm/nova/driver.rs |   2 +-
 drivers/gpu/drm/nova/file.rs   |  13 ++-
 drivers/gpu/drm/nova/gem.rs    |   8 +-
 drivers/gpu/drm/tyr/driver.rs  |   2 +-
 drivers/gpu/drm/tyr/file.rs    |  11 +-
 rust/kernel/drm/device.rs      |  36 +++++--
 rust/kernel/drm/driver.rs      | 113 +++++++++++++++++++--
 rust/kernel/drm/file.rs        | 180 ++++++++++++++++++++++++---------
 rust/kernel/drm/gem/mod.rs     |  44 ++++++--
 rust/kernel/drm/gem/shmem.rs   |   9 +-
 rust/kernel/drm/ioctl.rs       |   6 +-
 11 files changed, 330 insertions(+), 94 deletions(-)

diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index 44a52e60b3c7..f2d40e6fab21 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -90,7 +90,7 @@ fn probe<'bound>(
 impl drm::Driver for NovaDriver {
     type Data = ();
     type RegistrationData<'a> = DrmRegData<'a>;
-    type File = File;
+    type File<'a> = File;
     type Object = gem::Object<NovaObject>;
     type ParentDevice<Ctx: DeviceContext> = auxiliary::Device<Ctx>;
 
diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs
index 13642f83289d..8b800f821e2c 100644
--- a/drivers/gpu/drm/nova/file.rs
+++ b/drivers/gpu/drm/nova/file.rs
@@ -99,10 +99,13 @@ fn write_info<T: AsBytes>(info: &mut uapi::drm_nova_info, value: T) -> Result {
     Ok(())
 }
 
-impl drm::file::DriverFile for File {
+impl drm::file::DriverFile<'_> for File {
     type Driver = NovaDriver;
 
-    fn open(_dev: &NovaDevice) -> impl PinInit<Self, Error> {
+    fn open(
+        _device: &NovaDevice<Registered>,
+        _reg_data: &DrmRegData<'_>,
+    ) -> impl PinInit<Self, Error> {
         Ok(Self)
     }
 }
@@ -113,7 +116,7 @@ pub(crate) fn gem_create(
         dev: &NovaDevice<Registered>,
         _reg_data: &DrmRegData<'_>,
         req: &mut uapi::drm_nova_gem_create,
-        file: &drm::File<File>,
+        file: &drm::File<NovaDriver>,
     ) -> Result<u32> {
         let obj = NovaObject::new(dev, req.size.try_into()?)?;
 
@@ -127,7 +130,7 @@ pub(crate) fn gem_info(
         _dev: &NovaDevice<Registered>,
         _reg_data: &DrmRegData<'_>,
         req: &mut uapi::drm_nova_gem_info,
-        file: &drm::File<File>,
+        file: &drm::File<NovaDriver>,
     ) -> Result<u32> {
         let bo = NovaObject::lookup_handle(file, req.handle)?;
 
@@ -141,7 +144,7 @@ pub(crate) fn info(
         _dev: &NovaDevice<Registered>,
         reg_data: &DrmRegData<'_>,
         info: &mut uapi::drm_nova_info,
-        _file: &drm::File<File>,
+        _file: &drm::File<NovaDriver>,
     ) -> Result<u32> {
         match info.id {
             uapi::DRM_NOVA_INFO_GPU => write_info(info, GpuInfo::new(reg_data)?)?,
diff --git a/drivers/gpu/drm/nova/gem.rs b/drivers/gpu/drm/nova/gem.rs
index 2b6fe9dc0bfa..2a21ff8ba579 100644
--- a/drivers/gpu/drm/nova/gem.rs
+++ b/drivers/gpu/drm/nova/gem.rs
@@ -11,9 +11,9 @@
     sync::aref::ARef,
 };
 
-use crate::{
-    driver::{NovaDevice, NovaDriver},
-    file::File,
+use crate::driver::{
+    NovaDevice,
+    NovaDriver, //
 };
 
 /// GEM Object inner driver data
@@ -43,7 +43,7 @@ pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result<ARef<gem::Object<Self
     /// Look up a GEM object handle for a `File` and return an `ObjectRef` for it.
     #[inline]
     pub(crate) fn lookup_handle(
-        file: &drm::File<File>,
+        file: &drm::File<NovaDriver>,
         handle: u32,
     ) -> Result<ARef<gem::Object<Self>>> {
         gem::Object::lookup_handle(file, handle)
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index c2b99d67369f..46caf7b336d9 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -209,7 +209,7 @@ fn drop(self: Pin<&mut Self>) {}
 impl drm::Driver for TyrDrmDriver {
     type Data = ();
     type RegistrationData<'drm> = TyrDrmRegistrationData<'drm>;
-    type File = TyrDrmFileData;
+    type File<'a> = TyrDrmFileData;
     type Object = Bo;
     type ParentDevice<Ctx: DeviceContext> = platform::Device<Ctx>;
 
diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs
index 6850708cfcdd..933a365cb016 100644
--- a/drivers/gpu/drm/tyr/file.rs
+++ b/drivers/gpu/drm/tyr/file.rs
@@ -19,13 +19,16 @@
 #[pin_data]
 pub(crate) struct TyrDrmFileData {}
 
-/// Convenience type alias for our DRM `File` type
-pub(crate) type TyrDrmFile = drm::file::File<TyrDrmFileData>;
+/// Convenience type alias for our DRM `File` type.
+pub(crate) type TyrDrmFile = drm::file::File<TyrDrmDriver>;
 
-impl drm::file::DriverFile for TyrDrmFileData {
+impl drm::file::DriverFile<'_> for TyrDrmFileData {
     type Driver = TyrDrmDriver;
 
-    fn open(_dev: &drm::Device<Self::Driver>) -> impl PinInit<Self, Error> {
+    fn open(
+        _device: &TyrDrmDevice<Registered>,
+        _reg_data: &TyrDrmRegistrationData<'_>,
+    ) -> impl PinInit<Self, Error> {
         Ok(Self {})
     }
 }
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index b8444499b39c..bd66e55b6e17 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -15,9 +15,13 @@
     },
     error::from_err_ptr,
     prelude::*,
-    sync::aref::{
-        ARef,
-        AlwaysRefCounted, //
+    sync::{
+        aref::{
+            ARef,
+            AlwaysRefCounted, //
+        },
+        atomic::Atomic,
+        WaitQueue,
     },
     types::{
         NotThreadSafe,
@@ -190,7 +194,10 @@ fn deref(&self) -> &Self::Target {
     }
 }
 
-impl<T: drm::Driver> UnregisteredDevice<T> {
+impl<T: drm::Driver> UnregisteredDevice<T>
+where
+    for<'a> T::File<'a>: drm::file::DriverFile<'a, Driver = T>,
+{
     const fn compute_features() -> u32 {
         let mut features = drm::driver::FEAT_GEM;
 
@@ -203,8 +210,8 @@ const fn compute_features() -> u32 {
 
     const VTABLE: bindings::drm_driver = drm_legacy_fields! {
         load: None,
-        open: Some(drm::File::<T::File>::open_callback),
-        postclose: Some(drm::File::<T::File>::postclose_callback),
+        open: Some(drm::File::<T>::open_callback),
+        postclose: Some(drm::File::<T>::postclose_callback),
         unload: None,
         release: Some(Device::<T>::release),
         master_set: None,
@@ -333,6 +340,21 @@ pub fn new(
         // SAFETY: `raw_drm` is valid; no concurrent access before registration.
         unsafe { (*raw_drm.as_ptr()).registration_data = UnsafeCell::new(NonNull::dangling()) };
 
+        // SAFETY: `raw_drm` is valid; no concurrent access before registration.
+        unsafe { (*raw_drm.as_ptr()).open_count = Atomic::new(0) };
+
+        // SAFETY:
+        // - `raw_drm` is valid and `open_count_wq` is uninitialized; no concurrent access
+        //   before registration.
+        // - The field is pinned because the Device is pinned (refcounted, allocated by
+        //   `__drm_dev_alloc()`, never moved).
+        unsafe {
+            pin_init::raw_init(
+                &raw mut (*raw_drm.as_ptr()).open_count_wq,
+                crate::new_waitqueue!("drm_open_count"),
+            )
+        };
+
         // 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`.
@@ -357,6 +379,8 @@ pub struct Device<T: drm::Driver, C: DeviceContext = Normal> {
     dev: Opaque<bindings::drm_device>,
     data: T::Data,
     pub(super) registration_data: UnsafeCell<NonNull<T::RegistrationData<'static>>>,
+    pub(super) open_count: Atomic<i32>,
+    pub(super) open_count_wq: WaitQueue,
     _ctx: PhantomData<C>,
 }
 
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index f6db91e81e79..92055c4f9e00 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -7,10 +7,22 @@
 use crate::{
     bindings,
     device,
-    drm,
+    drm::{
+        self,
+        file::FileData, //
+    },
     error::to_result,
+    interop::list::clist_create,
     prelude::*,
-    sync::aref::ARef, //
+    sync::{
+        aref::ARef,
+        atomic::{
+            Acquire,
+            Release, //
+        },
+        Mutex, //
+    },
+    types::Opaque, //
 };
 use core::{
     marker::PhantomPinned,
@@ -120,8 +132,14 @@ pub trait Driver {
     /// The type used to manage memory for this driver.
     type Object: AllocImpl;
 
-    /// The type used to represent a DRM File (client)
-    type File: drm::file::DriverFile;
+    /// The type used to represent a DRM File (client).
+    ///
+    /// File data may borrow from [`RegistrationData`](Driver::RegistrationData). File data is
+    /// guaranteed to be dropped before registration data, either when the file is closed or
+    /// when the device is unregistered, whichever comes first.
+    ///
+    /// Drivers implement [`DriverFile`](drm::file::DriverFile) for their file data type.
+    type File<'a>: 'a;
 
     /// The bus device type of the parent device that the DRM device is associated with.
     type ParentDevice<Ctx: device::DeviceContext>: device::AsBusDevice<Ctx>;
@@ -197,6 +215,13 @@ pub unsafe fn new<E>(
                 // SAFETY: `drm` is a valid, initialized but not yet registered DRM device.
                 let ret = unsafe { bindings::drm_dev_register(drm.as_raw(), 0) };
                 to_result(ret).inspect_err(|_| {
+                    // SAFETY:
+                    // - Failed registration unplugs the device and drains registration guards.
+                    // - `data` remains initialized until this closure returns.
+                    // - Failed initialization does not run `Registration::drop()`, so this is
+                    //   the only file-data cleanup for this device.
+                    unsafe { Self::drop_file_data(drm) };
+
                     // SAFETY: `drm_dev_register()` synchronizes SRCU on failure, so no
                     // concurrent access to `registration_data` is possible at this point.
                     unsafe { *drm.registration_data.get() = NonNull::dangling() };
@@ -209,6 +234,75 @@ pub unsafe fn new<E>(
     pub fn device(&self) -> &drm::Device<T> {
         &self.drm
     }
+
+    /// Detaches and drops file data, then waits for concurrent postclose destruction.
+    ///
+    /// # Safety
+    ///
+    /// - `drm` must be unplugged with all `RegistrationGuard` sections drained.
+    /// - Its registration data must remain alive until this function returns.
+    /// - This must be the only call for this device.
+    unsafe fn drop_file_data(drm: &drm::Device<T>) {
+        let raw = drm.as_raw();
+        let mut files = None;
+
+        // SAFETY: `filelist_mutex` is initialized by `drm_dev_init()` and remains valid for
+        // the lifetime of the `struct drm_device`.
+        let filelist_mutex = unsafe { Mutex::from_raw(&raw mut (*raw).filelist_mutex) };
+        {
+            let _guard = filelist_mutex.lock();
+
+            // SAFETY:
+            // - `filelist` is initialized and protected by the held mutex.
+            // - Its entries are `drm_file`s linked through `lhead`; `Opaque` has the same layout.
+            let filelist = unsafe {
+                clist_create!(
+                    &raw mut (*raw).filelist,
+                    Opaque<bindings::drm_file>,
+                    bindings::drm_file,
+                    lhead
+                )
+            };
+
+            // Detach the allocations so `postclose()` sees NULL and leaves their destruction to us.
+            // After `postclose()` returns, the DRM core can free the C file, so the detached list
+            // must not depend on it. All `RegistrationGuard` sections have already been drained.
+            for file in filelist.iter() {
+                // SAFETY: `file` is a valid `drm_file` on this device's filelist.
+                let priv_ptr = unsafe { (*file.get()).driver_priv };
+
+                // SAFETY: Setting `driver_priv` to NULL is visible to `postclose_callback()`
+                // through the `filelist_mutex` acquire/release chain in `drm_close_helper()`.
+                unsafe { (*file.get()).driver_priv = core::ptr::null_mut() };
+
+                // SAFETY:
+                // - `priv_ptr` comes from `KBox::into_raw()` in `open_callback()`.
+                // - The file is still in the list, so `postclose_callback()` has not run. Clearing
+                //   `driver_priv` transfers ownership here and makes a later postclose skip it.
+                // - Only the owning pointer is moved; the pinned allocation is dropped in place.
+                let mut data = unsafe {
+                    Pin::new_unchecked(KBox::from_raw(
+                        priv_ptr.cast::<FileData<T::File<'static>>>(),
+                    ))
+                };
+                *data.as_mut().project().next = files.take();
+                files = Some(data);
+            }
+        }
+
+        // Run driver destructors without `filelist_mutex`. Take each link before dropping its
+        // allocation to avoid recursively dropping the entire chain.
+        while let Some(mut data) = files {
+            files = data.as_mut().project().next.take();
+            drop(data);
+            drm.open_count.fetch_sub(1, Release);
+        }
+
+        // Wait for file-data destruction in concurrent `postclose_callback()` calls. No new opens
+        // can succeed after unplug, so `open_count` is monotonically decreasing.
+        drm.open_count_wq
+            .wait_event(|| drm.open_count.load(Acquire) == 0);
+    }
 }
 
 // SAFETY: `Registration` doesn't offer any methods or access to fields when shared between
@@ -230,8 +324,13 @@ fn drop(self: Pin<&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.drm.as_raw()) };
-        // After drm_dev_unplug(), the SRCU barrier guarantees that all RegistrationGuard critical
-        // sections have completed, so no one holds a reference to reg_data anymore.
-        // reg_data is dropped here automatically.
+
+        // SAFETY:
+        // - `drm_dev_unplug()` has unplugged the device and drained registration guards.
+        // - Registration data remains alive until this destructor returns.
+        // - Registration succeeded, so this destructor performs the only file-data cleanup.
+        unsafe { Self::drop_file_data(&self.drm) };
+
+        // Registration data is dropped here automatically, after all file data has been dropped.
     }
 }
diff --git a/rust/kernel/drm/file.rs b/rust/kernel/drm/file.rs
index bd5e4ca2977f..2dd0e61718fb 100644
--- a/rust/kernel/drm/file.rs
+++ b/rust/kernel/drm/file.rs
@@ -8,36 +8,66 @@
     bindings,
     drm,
     prelude::*,
+    sync::atomic::{
+        Relaxed,
+        Release, //
+    },
     types::Opaque, //
 };
 use core::marker::PhantomData;
 
 /// Trait that must be implemented by DRM drivers to represent a DRM File (a client instance).
-pub trait DriverFile {
+///
+/// The lifetime `'a` allows the file data to borrow from
+/// [`RegistrationData`](drm::Driver::RegistrationData).
+pub trait DriverFile<'a>: Sized {
     /// The parent `Driver` implementation for this `DriverFile`.
     type Driver: drm::Driver;
 
-    /// Open a new file (called when a client opens the DRM device).
-    fn open(device: &drm::Device<Self::Driver>) -> impl PinInit<Self, Error>;
+    /// Open a new DRM file, creating the per-file driver data.
+    ///
+    /// Called when a client opens the DRM device. The returned initializer may borrow from
+    /// `reg_data` with lifetime `'a`.
+    fn open(
+        device: &drm::Device<Self::Driver, drm::Registered>,
+        reg_data: &'a <Self::Driver as drm::Driver>::RegistrationData<'a>,
+    ) -> impl PinInit<Self, Error>;
+}
+
+/// File-private allocation, with a link for dropping detached data outside `filelist_mutex`.
+#[pin_data]
+pub(super) struct FileData<T> {
+    /// Pinned per-file driver data, which may borrow from registration data.
+    #[pin]
+    data: T,
+    /// Next detached allocation to drop during registration teardown; otherwise `None`.
+    pub(super) next: Option<Pin<KBox<Self>>>,
 }
 
 /// An open DRM File.
 ///
 /// # Invariants
 ///
-/// `self.0` is a valid instance of a `struct drm_file`.
+/// - `self.0` is a valid instance of a `struct drm_file` opened on a device of driver `D`.
+/// - A [`drm::RegistrationGuard`] for that device is held for the duration of any `&File<D>`
+///   reference.
 #[repr(transparent)]
-pub struct File<T: DriverFile>(Opaque<bindings::drm_file>, PhantomData<T>);
+pub struct File<D: drm::Driver>(Opaque<bindings::drm_file>, PhantomData<D>);
 
-impl<T: DriverFile> File<T> {
+impl<D: drm::Driver> File<D> {
     #[doc(hidden)]
     /// Not intended to be called externally, except via declare_drm_ioctls!()
     ///
     /// # Safety
     ///
-    /// `raw_file` must be a valid pointer to an open `struct drm_file`, opened through `T::open`.
-    pub unsafe fn from_raw<'a>(ptr: *mut bindings::drm_file) -> &'a File<T> {
-        // SAFETY: `raw_file` is valid by the safety requirements of this function.
+    /// - `ptr` must point to an open `struct drm_file` on a device of driver `D` and remain valid
+    ///   for `'a`.
+    /// - A [`drm::RegistrationGuard`] for that device must be held for all of `'a`.
+    #[inline]
+    pub unsafe fn from_raw<'a>(ptr: *mut bindings::drm_file) -> &'a File<D> {
+        // SAFETY:
+        // - The caller guarantees that `ptr` is valid for `'a`.
+        // - The caller guarantees that the type invariants hold.
         unsafe { &*ptr.cast() }
     }
 
@@ -45,60 +75,110 @@ pub(super) fn as_raw(&self) -> *mut bindings::drm_file {
         self.0.get()
     }
 
-    fn driver_priv(&self) -> *mut T {
-        // SAFETY: By the type invariants of `Self`, `self.as_raw()` is always valid.
-        unsafe { (*self.as_raw()).driver_priv }.cast()
+    /// Returns the registered DRM device this file was opened on.
+    #[inline]
+    pub fn device(&self) -> &drm::Device<D, drm::Registered> {
+        // SAFETY:
+        // - The open file keeps its minor and device alive.
+        // - By the type invariants, the device uses driver `D`.
+        // - The type invariants require a `RegistrationGuard` for the device throughout this
+        //   borrow, satisfying the `Registered` context.
+        unsafe { drm::Device::from_raw((*(*self.as_raw()).minor).dev) }
     }
 
-    /// Return a pinned reference to the driver file structure.
-    pub fn inner(&self) -> Pin<&T> {
-        // SAFETY: By the type invariant the pointer `self.as_raw()` points to a valid and opened
-        // `struct drm_file`, hence `driver_priv` has been properly initialized by `open_callback`.
-        unsafe { Pin::new_unchecked(&*(self.driver_priv())) }
+    /// Access the driver file data through a closure.
+    ///
+    /// The closure receives a pinned reference to the file data.
+    ///
+    /// The outer reference carries the `&self` lifetime while the inner type carries the HRTB
+    /// lifetime `'a`, implying `'a` outlives `&self`. This allows the closure to coerce covariant
+    /// sub-fields (e.g. `&'a T` to the caller's lifetime) and return them directly in `R`.
+    #[inline]
+    pub fn inner_with<'this, R, F>(&'this self, f: F) -> R
+    where
+        F: for<'a> FnOnce(Pin<&'this D::File<'a>>) -> R,
+    {
+        // SAFETY: By the type invariants, `self.as_raw()` points to a valid `drm_file`.
+        let ptr: *const FileData<_> = unsafe { (*self.as_raw()).driver_priv.cast_const().cast() };
+
+        // SAFETY:
+        // - The type invariants keep this file's pinned `FileData` allocation valid for `'this`.
+        // - The `data` field is structurally pinned.
+        // - The HRTB `for<'a>` prevents the caller from choosing a concrete lifetime, so shortening
+        //   is sound regardless of variance.
+        let data: Pin<&'this D::File<'this>> = unsafe { Pin::new_unchecked(&(*ptr).data) };
+        f(data)
     }
 
     /// The open callback of a `struct drm_file`.
+    ///
+    /// Called from `drm_open()`, which is itself called from `fops_open()`. The latter holds a
+    /// `RegistrationGuard`, so the device is guaranteed to be registered for the duration of this
+    /// callback.
     pub(crate) extern "C" fn open_callback(
         raw_dev: *mut bindings::drm_device,
         raw_file: *mut bindings::drm_file,
-    ) -> core::ffi::c_int {
-        // SAFETY: A callback from `struct drm_driver::open` guarantees that
-        // - `raw_dev` is valid pointer to a `struct drm_device`,
-        // - the corresponding `struct drm_device` has been registered.
-        let drm = unsafe { drm::Device::from_raw(raw_dev) };
-
-        // SAFETY: `raw_file` is a valid pointer to a `struct drm_file`.
-        let file = unsafe { File::<T>::from_raw(raw_file) };
-
-        let init = T::open(drm);
-        let inner = match KBox::try_pin_init(init, GFP_KERNEL) {
-            Err(e) => {
-                return e.to_errno();
-            }
-            Ok(i) => i,
-        };
-
-        // SAFETY: This pointer is treated as pinned, and the Drop guarantee is upheld in
-        // `postclose_callback()`.
-        let driver_priv = KBox::into_raw(unsafe { Pin::into_inner_unchecked(inner) });
-
-        // SAFETY: By the type invariants of `Self`, `self.as_raw()` is always valid.
-        unsafe { (*file.as_raw()).driver_priv = driver_priv.cast() };
-
-        0
+    ) -> core::ffi::c_int
+    where
+        for<'a> D::File<'a>: DriverFile<'a, Driver = D>,
+    {
+        // SAFETY:
+        // - The DRM core guarantees that `raw_dev` is valid.
+        // - `fops_open()` holds a `RegistrationGuard`, satisfying the `Registered` context.
+        let dev: &drm::device::Device<D, drm::Registered> =
+            unsafe { drm::device::Device::from_raw(raw_dev) };
+
+        dev.registration_data_with(|reg_data| {
+            let init = try_pin_init!(FileData {
+                next: None,
+                data <- <D::File<'_> as DriverFile<'_>>::open(dev, reg_data),
+            });
+            let inner = match KBox::try_pin_init(init, GFP_KERNEL) {
+                Err(e) => return e.to_errno(),
+                Ok(i) => i,
+            };
+
+            // SAFETY: The allocation stays pinned until `postclose_callback()` or
+            // `Registration::drop_file_data()` drops it in place.
+            let driver_priv = KBox::into_raw(unsafe { Pin::into_inner_unchecked(inner) });
+
+            dev.open_count.fetch_add(1, Relaxed);
+
+            // SAFETY: `raw_file` is a valid pointer to a `struct drm_file`.
+            unsafe { (*raw_file).driver_priv = driver_priv.cast() };
+
+            0
+        })
     }
 
     /// The postclose callback of a `struct drm_file`.
     pub(crate) extern "C" fn postclose_callback(
-        _raw_dev: *mut bindings::drm_device,
+        raw_dev: *mut bindings::drm_device,
         raw_file: *mut bindings::drm_file,
-    ) {
-        // SAFETY: This reference won't escape this function
-        let file = unsafe { File::<T>::from_raw(raw_file) };
-
-        // SAFETY: `file.driver_priv` has been created in `open_callback` through `KBox::into_raw`.
-        let _ = unsafe { KBox::from_raw(file.driver_priv()) };
+    ) where
+        for<'a> D::File<'a>: DriverFile<'a, Driver = D>,
+    {
+        // SAFETY: `raw_file` is a valid pointer to a `struct drm_file`.
+        let driver_priv = unsafe { (*raw_file).driver_priv };
+
+        if driver_priv.is_null() {
+            return;
+        }
+
+        // SAFETY:
+        // - `driver_priv` comes from `KBox::into_raw()` in `open_callback()`.
+        // - The file is not on the filelist, and the NULL check rules out ownership having been
+        //   transferred to `Registration::drop_file_data()`.
+        // - The pinned data is dropped in place while registration data is still alive, since
+        //   registration cleanup waits for `open_count` to reach zero.
+        drop(unsafe { KBox::from_raw(driver_priv.cast::<FileData<D::File<'static>>>()) });
+
+        // SAFETY: `raw_dev` is valid for the lifetime of the `struct drm_file`.
+        let dev: &drm::device::Device<D> = unsafe { drm::device::Device::from_raw(raw_dev) };
+        if dev.open_count.fetch_sub(1, Release) == 1 {
+            dev.open_count_wq.wake_up();
+        }
     }
 }
 
-impl<T: DriverFile> super::private::Sealed for File<T> {}
+impl<D: drm::Driver> super::private::Sealed for File<D> {}
diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index 260f36817f96..123a3896f2a5 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -76,7 +76,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) {
 ///
 /// [`Driver`]: drm::Driver
 /// [`DriverFile`]: drm::file::DriverFile
-pub type DriverFile<T> = drm::File<<<T as DriverObject>::Driver as drm::Driver>::File>;
+pub type DriverFile<T> = drm::File<<T as DriverObject>::Driver>;
 
 /// A type alias for retrieving the current [`AllocImpl`] for a given [`DriverObject`].
 ///
@@ -136,7 +136,9 @@ extern "C" fn open_callback<T: DriverObject>(
         return ENODEV.to_errno();
     };
 
-    // SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`.
+    // SAFETY:
+    // - Handle creation ensures the valid file belongs to the object's device (`T::Driver`).
+    // - `_guard` is held for the entire use of the file reference.
     let file = unsafe { DriverFile::<T>::from_raw(raw_file) };
 
     // SAFETY:
@@ -164,7 +166,9 @@ extern "C" fn close_callback<T: DriverObject>(
         return;
     };
 
-    // SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`.
+    // SAFETY:
+    // - Handle creation ensures the valid file belongs to the object's device (`T::Driver`).
+    // - `_guard` is held for the entire use of the file reference.
     let file = unsafe { DriverFile::<T>::from_raw(raw_file) };
 
     // SAFETY: `close_callback` is specified in the AllocOps structure for `Object<T>`, ensuring
@@ -188,6 +192,18 @@ unsafe fn from_raw<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a Self {
 
 /// Base operations shared by all GEM object classes
 pub trait BaseObject: IntoGEMObject {
+    /// Returns the DRM device that owns this GEM object.
+    #[inline]
+    fn device(&self) -> &drm::Device<Self::Driver>
+    where
+        Self: AllocImpl,
+    {
+        // SAFETY:
+        // - `self.as_raw()` points to a valid GEM object whose device is valid for this borrow.
+        // - The sealed `AllocImpl` implementations identify the device's driver as `Self::Driver`.
+        unsafe { drm::Device::from_raw((*self.as_raw()).dev) }
+    }
+
     /// Returns the size of the object in bytes.
     fn size(&self) -> usize {
         // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `struct drm_gem_object`.
@@ -196,26 +212,32 @@ fn size(&self) -> usize {
 
     /// Creates a new handle for the object associated with a given `File`
     /// (or returns an existing one).
-    fn create_handle<D, F>(&self, file: &drm::File<F>) -> Result<u32>
+    ///
+    /// Returns [`EINVAL`] if the object and file belong to different DRM devices.
+    fn create_handle<D>(&self, file: &drm::File<D>) -> Result<u32>
     where
         Self: AllocImpl<Driver = D>,
-        D: drm::Driver<Object = Self, File = F>,
-        F: drm::file::DriverFile<Driver = D>,
+        D: drm::Driver<Object = Self>,
     {
+        if self.device().as_raw() != file.device().as_raw() {
+            return Err(EINVAL);
+        }
+
         let mut handle: u32 = 0;
-        // SAFETY: The arguments are all valid per the type invariants.
+        // SAFETY:
+        // - The pointers are valid per the type invariants.
+        // - The object and file belong to the same DRM device.
         to_result(unsafe {
-            bindings::drm_gem_handle_create(file.as_raw().cast(), self.as_raw(), &mut handle)
+            bindings::drm_gem_handle_create(file.as_raw(), self.as_raw(), &mut handle)
         })?;
         Ok(handle)
     }
 
     /// Looks up an object by its handle for a given `File`.
-    fn lookup_handle<D, F>(file: &drm::File<F>, handle: u32) -> Result<ARef<Self>>
+    fn lookup_handle<D>(file: &drm::File<D>, handle: u32) -> Result<ARef<Self>>
     where
         Self: AllocImpl<Driver = D> + AlwaysRefCounted,
-        D: drm::Driver<Object = Self, File = F>,
-        F: drm::file::DriverFile<Driver = D>,
+        D: drm::Driver<Object = Self>,
     {
         // SAFETY: The arguments are all valid per the type invariants.
         let ptr = unsafe { bindings::drm_gem_object_lookup(file.as_raw().cast(), handle) };
diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs
index 1a4f81e3b5b5..1558b6079edf 100644
--- a/rust/kernel/drm/gem/shmem.rs
+++ b/rust/kernel/drm/gem/shmem.rs
@@ -601,10 +601,13 @@ struct KunitObject {}
         desc: c"Kunit",
     };
 
-    impl drm::file::DriverFile for KunitFile {
+    impl drm::file::DriverFile<'_> for KunitFile {
         type Driver = KunitDriver;
 
-        fn open(_dev: &drm::Device<KunitDriver>) -> impl PinInit<Self, Error> {
+        fn open(
+            _device: &drm::Device<KunitDriver, drm::Registered>,
+            _reg_data: &(),
+        ) -> impl PinInit<Self, Error> {
             Ok(Self)
         }
     }
@@ -626,7 +629,7 @@ fn new(
     impl drm::Driver for KunitDriver {
         type Data = KunitData;
         type RegistrationData<'a> = ();
-        type File = KunitFile;
+        type File<'a> = KunitFile;
         type Object = Object<KunitObject>;
         type ParentDevice<Ctx: device::DeviceContext> = faux::Device<Ctx>;
 
diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs
index 9934b23c36eb..65c326348c64 100644
--- a/rust/kernel/drm/ioctl.rs
+++ b/rust/kernel/drm/ioctl.rs
@@ -97,7 +97,7 @@ pub const fn __dev_ctx_cast<T: crate::drm::Driver>(
 /// fn foo(device: &kernel::drm::Device<Self, kernel::drm::Registered>,
 ///        reg_data: &Self::RegistrationData<'_>,
 ///        data: &mut uapi::argument_type,
-///        file: &kernel::drm::File<Self::File>,
+///        file: &kernel::drm::File<Self>,
 /// ) -> Result<u32>
 /// ```
 /// where `Self` is the drm::drv::Driver implementation these ioctls are being declared within.
@@ -192,7 +192,9 @@ macro_rules! declare_drm_ioctls {
                             // There is no concurrent access or aliasing; no other references
                             // to this object exist during this call.
                             let data = unsafe { &mut *(raw_data.cast::<$crate::uapi::$struct>()) };
-                            // SAFETY: This is just the DRM file structure
+                            // SAFETY:
+                            // - The DRM core provides a valid file for this device.
+                            // - `guard` is held for the entire use of the file reference.
                             let file = unsafe { $crate::drm::File::from_raw(raw_file) };
 
                             match guard.registration_data_with(|reg_data| {
-- 
2.56.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-11-07  2:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-07  2:03 [PATCH v2 0/7] lifetime-parameterized DRM File private data Danilo Krummrich
2026-11-07  2:03 ` [PATCH v2 1/7] rust: drm: rename Ioctl device context to Userspace Danilo Krummrich
2026-11-07  2:03 ` [PATCH v2 2/7] rust: drm: gem: gate open/close callbacks with RegistrationGuard Danilo Krummrich
2026-11-07  2:03 ` [PATCH v2 3/7] rust: drm: move file_operations from gem to device Danilo Krummrich
2026-11-07  2:03 ` [PATCH v2 4/7] rust: fs: add iminor() helper Danilo Krummrich
2026-11-07  2:03 ` [PATCH v2 5/7] rust: drm: wrap fops open with RegistrationGuard Danilo Krummrich
2026-11-07  2:03 ` [PATCH v2 6/7] rust: drm: return impl PinInit from DriverFile::open() Danilo Krummrich
2026-11-07  2:03 ` [PATCH v2 7/7] rust: drm: make Driver::File lifetime-parameterized Danilo Krummrich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®