Re: [PATCH v5 11/19] rust: drm: add Ioctl device context typestate

"Alexandre Courbot" <[email protected]>
Newsgroups dev.linux.lists.nova-gpu,dev.linux.lists.driver-core,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel,org.kernel.vger.rust-for-linux
Message-ID <[email protected]>
On Sun Jun 28, 2026 at 11:53 PM JST, Danilo Krummrich wrote:
> Add the Ioctl DeviceContext for DRM devices that have been registered
> with userspace previously.
>
> A Device<T, Ioctl> has been registered at some point, but may be
> concurrently unregistering or already unregistered. drm_dev_enter() can
> guard against this, ensuring the device remains registered for the
> duration of the critical section.
>
> This typestate will be used in ioctl dispatch context where registration
> is guaranteed by the DRM core, and RegistrationGuard can safely be
> acquired.
>
> Reviewed-by: Lyude Paul <[email protected]>
> Signed-off-by: Danilo Krummrich <[email protected]>
> ---
>  rust/kernel/drm/device.rs | 34 +++++++++++++++++++++++++++++++---
>  rust/kernel/drm/mod.rs    |  1 +
>  2 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> index 86a7fca1d33f..d429b4655449 100644
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs
> @@ -74,14 +74,16 @@ macro_rules! drm_legacy_fields {
>  
>  /// A trait implemented by all possible contexts a [`Device`] can be used in.
>  ///
> -/// A [`Device`] can be in one of two contexts:
> +/// A [`Device`] can be in one of the following contexts:
>  ///
>  /// - [`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.
>  /// - [`Registered`]: The device has been registered with userspace at some point.
>  ///
> -/// `Device<T, Registered>` dereferences to `Device<T>` ([`Normal`]), so any method available on a
> -/// [`Normal`] device is also available on a [`Registered`] one.
> +/// Both `Device<T, Ioctl>` 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 {}
>  
>  /// The general-purpose, reference-counted [`DeviceContext`].
> @@ -113,6 +115,21 @@ impl DeviceContext for Normal {}
>  impl Sealed for Registered {}
>  impl DeviceContext for Registered {}
>  
> +/// The [`DeviceContext`] of a [`Device`] that has been registered with userspace previously.
> +///
> +/// A [`Device`] in this context has been registered at some point, but may be concurrently
> +/// unregistering or already unregistered. `drm_dev_enter()` can guard against this, ensuring the
> +/// device remains registered for the duration of the critical section.
> +///
> +/// # Invariants
> +///
> +/// A [`Device`] in this context has been registered with userspace via `drm_dev_register()` at
> +/// some point.
> +pub struct Ioctl;
> +
> +impl Sealed for Ioctl {}
> +impl DeviceContext for Ioctl {}
> +
>  /// A [`Device`] which is known at compile-time to be unregistered with userspace.
>  ///
>  /// This type allows performing operations which are only safe to do before userspace registration,
> @@ -342,6 +359,17 @@ fn deref(&self) -> &Self::Target {
>      }
>  }
>  
> +impl<T: drm::Driver> Deref for Device<T, Ioctl> {
> +    type Target = Device<T>;
> +
> +    #[inline]
> +    fn deref(&self) -> &Self::Target {
> +        // SAFETY: The caller holds a `Device<T, Ioctl>`, which guarantees all invariants
> +        // of the weaker `Normal` context.
> +        unsafe { self.assume_ctx() }
> +    }
> +}

Since the `Deref` implementation is the same for `Ioctl` and
`Registered`, do we want to add a private marker trait for these two so
we can factor out this `impl` block?

It wouldn't reduce the LoC count, but would make the logical connection
between the two states explicit and set an anchor for potential future
blocks with a similar relationship.

With or without this:

Reviewed-by: Alexandre Courbot <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.