All of lore.kernel.org
 help / color / mirror / Atom feed
From: Danilo Krummrich <dakr@kernel.org>
To: dakr@kernel.org, aliceryhl@google.com,
	daniel.almeida@collabora.com, acourbot@nvidia.com,
	ecourtney@nvidia.com, ojeda@kernel.org, boqun@kernel.org,
	gary@garyguo.net, bjorn3_gh@protonmail.com, lossin@kernel.org,
	a.hindborg@kernel.org, tmgross@umich.edu,
	deborah.brouwer@collabora.com, boris.brezillon@collabora.com,
	lyude@redhat.com
Cc: driver-core@lists.linux.dev, linux-kernel@vger.kernel.org,
	nova-gpu@lists.linux.dev, dri-devel@lists.freedesktop.org,
	rust-for-linux@vger.kernel.org
Subject: [PATCH v3 09/13] rust: drm: Add RegistrationGuard for drm_dev_enter/exit critical sections
Date: Sat, 20 Jun 2026 02:51:21 +0200	[thread overview]
Message-ID: <20260620005431.1562115-10-dakr@kernel.org> (raw)
In-Reply-To: <20260620005431.1562115-1-dakr@kernel.org>

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 RegistrationGuard, a guard object representing a drm_dev_enter/exit
SRCU critical section that dereferences to &Device<T, Registered>. The
guard is obtained from Device<T> (Normal) and proves at runtime that the
device is still registered.

Switch Registration::drop from drm_dev_unregister() to drm_dev_unplug()
to provide the SRCU barrier that RegistrationGuard's safety argument
relies on.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/drm/device.rs | 76 +++++++++++++++++++++++++++++++++------
 rust/kernel/drm/driver.rs | 10 ++++--
 rust/kernel/drm/mod.rs    |  1 +
 3 files changed, 74 insertions(+), 13 deletions(-)

diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 885077e270eb..bc8cfd753596 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -78,7 +78,8 @@ 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.
-/// - [`Registered`]: The device has been registered with userspace at some point.
+/// - [`Registered`]: The device is currently registered with userspace and the parent bus device
+///   is bound.
 ///
 /// `Device<T, Registered>` dereferences to `Device<T>` ([`Normal`]), so any method available on a
 /// [`Normal`] device is also available on a [`Registered`] one.
@@ -96,18 +97,15 @@ pub trait DeviceContext: Sealed + Send + Sync {}
 impl Sealed for Normal {}
 impl DeviceContext for Normal {}
 
-/// The [`DeviceContext`] of a [`Device`] that was registered with userspace at some point.
+/// The [`DeviceContext`] of a [`Device`] that is currently registered with userspace.
 ///
-/// This represents a [`Device`] which is guaranteed to have been registered with userspace at
-/// some point in time. Such a DRM device is guaranteed to have been fully-initialized.
-///
-/// Note: A device in this context is not guaranteed to remain registered with userspace for its
-/// entire lifetime, as this is impossible to guarantee at compile-time.
+/// A [`Device`] in this context is guaranteed to be registered and its parent bus device is
+/// guaranteed to be bound. This is enforced at runtime by [`RegistrationGuard`], which holds a
+/// `drm_dev_enter()` / `drm_dev_exit()` SRCU critical section.
 ///
 /// # Invariants
 ///
-/// A [`Device`] in this [`DeviceContext`] is guaranteed to have been registered with userspace
-/// at some point in time.
+/// The parent bus device is bound for the duration of any reference to a `Device<T, Registered>`.
 pub struct Registered;
 
 impl Sealed for Registered {}
@@ -243,8 +241,8 @@ pub fn new(
 
 /// A typed DRM device with a specific [`drm::Driver`] implementation and [`DeviceContext`].
 ///
-/// A device in the [`Registered`] context is guaranteed to have been registered with userspace
-/// at some point. The [`Normal`] context is the general-purpose, reference-counted context.
+/// A device in the [`Registered`] context is currently registered with userspace and its parent
+/// bus device is bound. The [`Normal`] context is the general-purpose, reference-counted context.
 ///
 /// # Invariants
 ///
@@ -323,6 +321,62 @@ pub(crate) unsafe fn assume_ctx<NewCtx: DeviceContext>(&self) -> &Device<T, NewC
     }
 }
 
+impl<T: drm::Driver> Device<T> {
+    /// Guard against the parent bus device being unbound.
+    ///
+    /// Returns a [`RegistrationGuard`] if the device has not been unplugged, [`None`] otherwise.
+    ///
+    /// While [`RegistrationGuard`] is held the parent device is guaranteed to be bound.
+    #[must_use]
+    pub fn registration_guard(&self) -> Option<RegistrationGuard<'_, T>> {
+        let mut idx: i32 = 0;
+        // SAFETY: `self.as_raw()` is a valid pointer to a `struct drm_device`.
+        if unsafe { bindings::drm_dev_enter(self.as_raw(), &mut idx) } {
+            Some(RegistrationGuard { dev: self, idx })
+        } else {
+            None
+        }
+    }
+}
+
+/// A guard proving the DRM device is registered and the parent bus device is bound.
+///
+/// The guard dereferences to [`Device<T, Registered>`], providing access to the DRM device with
+/// the guarantee that the parent bus device 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.
+///
+/// # 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 RegistrationGuard<'a, T: drm::Driver> {
+    dev: &'a Device<T>,
+    idx: i32,
+}
+
+impl<T: drm::Driver> Deref for RegistrationGuard<'_, T> {
+    type Target = Device<T, Registered>;
+
+    #[inline]
+    fn deref(&self) -> &Self::Target {
+        // SAFETY: A successful `drm_dev_enter()` guarantees that the device is registered and
+        // the parent bus device is bound.
+        unsafe { self.dev.assume_ctx() }
+    }
+}
+
+impl<T: drm::Driver> Drop for RegistrationGuard<'_, 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 `RegistrationGuard`.
+        unsafe { bindings::drm_dev_exit(self.idx) };
+    }
+}
+
 impl<T: drm::Driver> Deref for Device<T> {
     type Target = T::Data;
 
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index 9ba7dc3a7a97..ceb2829985c7 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 `RegistrationGuard`, 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()) };
     }
 }
diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
index e5bfaf130342..6cfb01eec7ee 100644
--- a/rust/kernel/drm/mod.rs
+++ b/rust/kernel/drm/mod.rs
@@ -13,6 +13,7 @@
 pub use self::device::DeviceContext;
 pub use self::device::Normal;
 pub use self::device::Registered;
+pub use self::device::RegistrationGuard;
 pub use self::device::UnregisteredDevice;
 pub use self::driver::Driver;
 pub use self::driver::DriverInfo;
-- 
2.54.0


  parent reply	other threads:[~2026-06-20  0:59 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-20  0:51 [PATCH v3 00/13] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
2026-06-20  0:51 ` [PATCH v3 01/13] rust: drm: rename Uninit DeviceContext to Normal Danilo Krummrich
2026-06-20  0:51 ` [PATCH v3 02/13] rust: drm: Add Driver::ParentDevice associated type Danilo Krummrich
2026-06-20  0:51 ` [PATCH v3 03/13] rust: drm: change default DeviceContext to Normal Danilo Krummrich
2026-06-20  0:51 ` [PATCH v3 04/13] rust: drm: restrict AlwaysRefCounted to Normal Device context Danilo Krummrich
2026-06-20  0:51 ` [PATCH v3 05/13] rust: drm: restrict AlwaysRefCounted to Normal GEM Object context Danilo Krummrich
2026-06-20  0:51 ` [PATCH v3 06/13] rust: drm: split Deref for Device context typestates Danilo Krummrich
2026-06-20  0:51 ` [PATCH v3 07/13] rust: drm: return ParentDevice from Device AsRef Danilo Krummrich
2026-06-20  0:51 ` [PATCH v3 08/13] rust: drm: add AsRef<ParentDevice<Bound>> for Device<Registered> Danilo Krummrich
2026-06-20  0:51 ` Danilo Krummrich [this message]
2026-06-20  1:12   ` [PATCH v3 09/13] rust: drm: Add RegistrationGuard for drm_dev_enter/exit critical sections sashiko-bot
2026-06-20  0:51 ` [PATCH v3 10/13] rust: drm: Add RegistrationData to drm::Driver Danilo Krummrich
2026-06-20  1:10   ` sashiko-bot
2026-06-20  0:51 ` [PATCH v3 11/13] rust: drm: Wrap ioctl dispatch in RegistrationGuard Danilo Krummrich
2026-06-20  1:13   ` sashiko-bot
2026-06-20  0:51 ` [PATCH v3 12/13] rust: drm: Pass registration data to ioctl handlers Danilo Krummrich
2026-06-20  1:16   ` sashiko-bot
2026-06-20  0:51 ` [PATCH v3 13/13] drm: nova: Use drm::Device<Registered> to access the parent bus device Danilo Krummrich
2026-06-20  1:17   ` sashiko-bot
2026-06-20 16:52 ` [PATCH v3 00/13] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260620005431.1562115-10-dakr@kernel.org \
    --to=dakr@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=boris.brezillon@collabora.com \
    --cc=daniel.almeida@collabora.com \
    --cc=deborah.brouwer@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=driver-core@lists.linux.dev \
    --cc=ecourtney@nvidia.com \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=lyude@redhat.com \
    --cc=nova-gpu@lists.linux.dev \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.