All of lore.kernel.org
 help / color / mirror / Atom feed
From: Danilo Krummrich <dakr@kernel.org>
To: aliceryhl@google.com, airlied@gmail.com, simona@ffwll.ch,
	daniel.almeida@collabora.com, acourbot@nvidia.com,
	apopple@nvidia.com, ecourtney@nvidia.com,
	deborah.brouwer@collabora.com, lyude@redhat.com,
	ojeda@kernel.org, boqun@kernel.org, gary@garyguo.net,
	bjorn3_gh@protonmail.com, lossin@kernel.org,
	a.hindborg@kernel.org, tmgross@umich.edu
Cc: driver-core@lists.linux.dev, nova-gpu@lists.linux.dev,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	rust-for-linux@vger.kernel.org,
	Danilo Krummrich <dakr@kernel.org>
Subject: [PATCH 2/6] rust: drm: Add UnbindGuard for drm_dev_enter/exit critical sections
Date: Thu,  7 May 2026 00:06:00 +0200	[thread overview]
Message-ID: <20260506221027.858481-3-dakr@kernel.org> (raw)
In-Reply-To: <20260506221027.858481-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 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 | 80 ++++++++++++++++++++++++++++++++++++++-
 rust/kernel/drm/driver.rs | 10 ++++-
 2 files changed, 87 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 492c2f2c7ca4..bb685165032d 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -6,7 +6,11 @@
 
 use crate::{
     alloc::allocator::Kmalloc,
-    bindings, device,
+    bindings,
+    device::{
+        self,
+        AsBusDevice as _, //
+    },
     drm::{
         self,
         driver::AllocImpl,
@@ -334,6 +338,80 @@ 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)).
+    /// Between `drm_dev_enter()` and `drm_dev_exit()` 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>;
+
+    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> {
+    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 9d06f8c5b2da..751a68bb27e1 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -187,8 +187,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


  parent reply	other threads:[~2026-05-06 22:10 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-06 22:05 [PATCH 0/6] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
2026-05-06 22:05 ` [PATCH 1/6] rust: drm: Add Driver::ParentDevice associated type Danilo Krummrich
2026-05-08 21:49   ` lyude
2026-05-06 22:06 ` Danilo Krummrich [this message]
2026-05-06 22:06 ` [PATCH 3/6] rust: drm: Add RegistrationData to drm::Driver Danilo Krummrich
2026-05-06 22:06 ` [PATCH 4/6] rust: drm: Wrap ioctl dispatch in UnbindGuard Danilo Krummrich
2026-05-06 22:06 ` [PATCH 5/6] rust: drm: Pass registration data to ioctl handlers Danilo Krummrich
2026-05-06 22:06 ` [PATCH 6/6] rust: drm: Pass bound parent device " Danilo Krummrich
2026-05-07  9:38 ` [PATCH 0/6] rust: drm: Higher-Ranked Lifetime private data Danilo Krummrich
2026-05-14 18:59   ` Deborah Brouwer
2026-05-14 19:07     ` 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=20260506221027.858481-3-dakr@kernel.org \
    --to=dakr@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=airlied@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=apopple@nvidia.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --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=simona@ffwll.ch \
    --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.