[PATCH v5 0/4] Introduce DeviceContext

[PATCH v5 0/4] Introduce DeviceContext

[Lyude Paul]

Previous version of this patch series:
  https://patchwork.freedesktop.org/series/160217/#rev3

One of the unsolved issues we still have with the rust DRM bindings is
the ability to limit certain Device operations to contexts where we can
guarantee that a Device has been fully initialized and registered with
userspace, or vice-versa (e.g. must be unregistered).

While the previous solution for this that I had was simply not exposing
drm::Device at all until the device has been registered with userspace,
unfortunately this isn't enough since:

* As we found out with Tyr, drivers occasionally need to be able to
  create GEM objects before device registration
* We would still need to be able to handle KMS callbacks which could be
  invoked after KMS init but before userspace registration (not handled
  in this series specifically, but DeviceContext will be required for
  handling this).

This patch series provides a pretty nice solution to this, by
implementing a very similar solution to kernel::device::DeviceContext:
introducing our own DeviceContext type state.

This patch series depends on the two pre-requisite patch series:

* https://patchwork.freedesktop.org/series/160523/

                          Series-wide changes

V2:
* s/DeviceCtx/DeviceContext/ for consistency
* Move private driver-data availability to the Registration
  DeviceContext
* s/AnyCtx/Init/
V4:
* Split out DriverAllocImpl into it's own patch

More changes described in each patch description.

Lyude Paul (3):
  rust/drm: Introduce DeviceContext
  rust/drm: Don't setup private driver data until registration
  rust/drm/gem: Use DeviceContext with GEM objects

Lyude Paul (4):
  rust/drm: Introduce DeviceContext
  rust/drm: Don't setup private driver data until registration
  rust/drm/gem: Add DriverAllocImpl type alias
  rust/drm/gem: Use DeviceContext with GEM objects

 drivers/gpu/drm/nova/driver.rs |  10 +-
 drivers/gpu/drm/nova/gem.rs    |  11 +-
 drivers/gpu/drm/tyr/driver.rs  |  12 +-
 drivers/gpu/drm/tyr/gem.rs     |   3 +-
 rust/kernel/drm/device.rs      | 250 +++++++++++++++++++++++++--------
 rust/kernel/drm/driver.rs      |  52 +++++--
 rust/kernel/drm/gem/mod.rs     |  64 ++++++---
 rust/kernel/drm/mod.rs         |   4 +
 8 files changed, 305 insertions(+), 101 deletions(-)


base-commit: cea7b66a80412e2a5b74627b89ae25f1d0110a4b
prerequisite-patch-id: ddb20be75c8fc92ba2d48a5c4aa8e724202de1ff
prerequisite-patch-id: 87e69cd89a806392d3275b19fbfb160d44d957c9
-- 
2.52.0

[PATCH v5 1/4] rust/drm: Introduce DeviceContext

[Lyude Paul]

One of the tricky things about DRM bindings in Rust is the fact that
initialization of a DRM device is a multi-step process. It's quite normal
for a device driver to start making use of its DRM device for tasks like
creating GEM objects before userspace registration happens. This is an
issue in rust though, since prior to userspace registration the device is
only partly initialized. This means there's a plethora of DRM device
operations we can't yet expose without opening up the door to UB if the DRM
device in question isn't yet registered.

Additionally, this isn't something we can reliably check at runtime. And
even if we could, performing an operation which requires the device be
registered when the device isn't actually registered is a programmer bug,
meaning there's no real way to gracefully handle such a mistake at runtime.
And even if that wasn't the case, it would be horrendously annoying and
noisy to have to check if a device is registered constantly throughout a
driver.

In order to solve this, we first take inspiration from
`kernel::device::DeviceContext` and introduce `kernel::drm::DeviceContext`.
This provides us with a ZST type that we can generalize over to represent
contexts where a device is known to have been registered with userspace at
some point in time (`Registered`), along with contexts where we can't make
such a guarantee (`Uninit`).

It's important to note we intentionally do not provide a `DeviceContext`
which represents an unregistered device. This is because there's no
reasonable way to guarantee that a device with long-living references to
itself will not be registered eventually with userspace. Instead, we
provide a new-type for this: `UnregisteredDevice` which can
provide a guarantee that the `Device` has never been registered with
userspace. To ensure this, we modify `Registration` so that creating a new
`Registration` requires passing ownership of an `UnregisteredDevice`.

Signed-off-by: Lyude Paul <lyude@redhat.com>

---
V2:
* Make sure that `UnregisteredDevice` is not thread-safe (since DRM device
  initialization is also not thread-safe)
* Rename from AnyCtx to Uninit, I think this name actually makes a bit more
  sense.
* Change assume_registered() to assume_ctx()
  Since it looks like in some situations, we'll want to update the
  DeviceContext of a object to the latest DeviceContext we know the Device
  to be in.
* Rename Init to Uninit
  When we eventually add KMS support, we're going to have 3 different
  DeviceContexts - Uninit, Init, Registered. Additionally, aside from not
  being registered there are a number of portions of the rest of the Device
  which also aren't usable before at least the Init context - so the naming
  of Uninit makes this a little clearer.
* s/DeviceContext/DeviceContext/
  For consistency with the rest of the kernel
* Drop as_ref::<Device<T, Uninit>>() for now since I don't actually think
  we need this quite yet
V3:
* Get rid of drm_dev_ctx!, as we don't actually need to implement Send or
  Sync ourselves
* Remove mention of C function in drm::device::Registration rustdoc
* Add more documentation to the DeviceContext trait, go into detail about
  the various setup phases and such.
* Add missing period to comment in `UnregisteredDevice::new()`.
V4:
* Address some comments from Danilo I missed last round:
  * Remove leftover rebase detritus from new_foreign_owned()
    (the seemingly useless cast)
  * Remove no-op mention in Registered device context

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/nova/driver.rs |   8 +-
 drivers/gpu/drm/tyr/driver.rs  |  10 +-
 rust/kernel/drm/device.rs      | 178 +++++++++++++++++++++++++++------
 rust/kernel/drm/driver.rs      |  35 +++++--
 rust/kernel/drm/mod.rs         |   4 +
 5 files changed, 190 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index b1af0a099551d..99d6841b69cbc 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -21,7 +21,7 @@ pub(crate) struct NovaDriver {
 }
 
 /// Convienence type alias for the DRM device type for this driver
-pub(crate) type NovaDevice = drm::Device<NovaDriver>;
+pub(crate) type NovaDevice<Ctx = drm::Registered> = drm::Device<NovaDriver, Ctx>;
 
 #[pin_data]
 pub(crate) struct NovaData {
@@ -56,10 +56,10 @@ impl auxiliary::Driver for NovaDriver {
     fn probe(adev: &auxiliary::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> {
         let data = try_pin_init!(NovaData { adev: adev.into() });
 
-        let drm = drm::Device::<Self>::new(adev.as_ref(), data)?;
-        drm::Registration::new_foreign_owned(&drm, adev.as_ref(), 0)?;
+        let drm = drm::UnregisteredDevice::<Self>::new(adev.as_ref(), data)?;
+        let drm = drm::Registration::new_foreign_owned(drm, adev.as_ref(), 0)?;
 
-        Ok(Self { drm })
+        Ok(Self { drm: drm.into() })
     }
 }
 
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index 568cb89aaed86..0d479b5d7bd62 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -30,7 +30,7 @@
 pub(crate) type IoMem = kernel::io::mem::IoMem<SZ_2M>;
 
 /// Convenience type alias for the DRM device type for this driver.
-pub(crate) type TyrDevice = drm::Device<TyrDriver>;
+pub(crate) type TyrDevice<Ctx = drm::Registered> = drm::Device<TyrDriver, Ctx>;
 
 #[pin_data(PinnedDrop)]
 pub(crate) struct TyrDriver {
@@ -133,10 +133,12 @@ fn probe(
                 gpu_info,
         });
 
-        let tdev: ARef<TyrDevice> = drm::Device::new(pdev.as_ref(), data)?;
-        drm::driver::Registration::new_foreign_owned(&tdev, pdev.as_ref(), 0)?;
+        let tdev = drm::UnregisteredDevice::<Self>::new(pdev.as_ref(), data)?;
+        let tdev = drm::driver::Registration::new_foreign_owned(tdev, pdev.as_ref(), 0)?;
 
-        let driver = TyrDriver { _device: tdev };
+        let driver = TyrDriver {
+            _device: tdev.into(),
+        };
 
         // We need this to be dev_info!() because dev_dbg!() does not work at
         // all in Rust for now, and we need to see whether probe succeeded.
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 629ef0bd1188e..8f1780b8f0196 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -10,7 +10,8 @@
     device,
     drm::{
         self,
-        driver::AllocImpl, //
+        driver::AllocImpl,
+        private::Sealed, //
     },
     error::from_err_ptr,
     prelude::*,
@@ -18,10 +19,14 @@
         ARef,
         AlwaysRefCounted, //
     },
-    types::Opaque, //
+    types::{
+        NotThreadSafe,
+        Opaque, //
+    }, //
 };
 use core::{
     alloc::Layout,
+    marker::PhantomData,
     mem,
     ops::Deref,
     ptr::{
@@ -61,26 +66,98 @@ macro_rules! drm_legacy_fields {
     }
 }
 
-/// A typed DRM device with a specific `drm::Driver` implementation.
+/// A trait implemented by all possible contexts a [`Device`] can be used in.
 ///
-/// The device is always reference-counted.
+/// Setting up a new [`Device`] is a multi-stage process. Each step of the process that a user
+/// interacts with in Rust has a respective [`DeviceContext`] typestate. For example,
+/// `Device<T, Registered>` would be a [`Device`] that reached the [`Registered`] [`DeviceContext`].
+///
+/// Each stage of this process is described below:
+///
+/// ```text
+///        1                     2                         3
+/// +--------------+   +------------------+   +-----------------------+
+/// |Device created| → |Device initialized| → |Registered w/ userspace|
+/// +--------------+   +------------------+   +-----------------------+
+///    (Uninit)                                      (Registered)
+/// ```
+///
+/// 1. The [`Device`] is in the [`Uninit`] context and is not guaranteed to be initialized or
+///    registered with userspace. Only a limited subset of DRM core functionality is available.
+/// 2. The [`Device`] is guaranteed to be fully initialized, but is not guaranteed to be registered
+///    with userspace. All DRM core functionality which doesn't interact with userspace is
+///    available. We currently don't have a context for representing this.
+/// 3. The [`Device`] is guaranteed to be fully initialized, and is guaranteed to have been
+///    registered with userspace at some point - thus putting it in the [`Registered`] context.
+///
+/// An important caveat of [`DeviceContext`] which must be kept in mind: when used as a typestate
+/// for a reference type, it can only guarantee that a [`Device`] reached a particular stage in the
+/// initialization process _at the time the reference was taken_. No guarantee is made in regards to
+/// what stage of the process the [`Device`] is currently in. This means for instance that a
+/// `&Device<T, Uninit>` may actually be registered with userspace, it just wasn't known to be
+/// registered at the time the reference was taken.
+pub trait DeviceContext: Sealed + Send + Sync {}
+
+/// The [`DeviceContext`] of a [`Device`] that was registered with userspace at some point.
+///
+/// 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.
 ///
 /// # Invariants
 ///
-/// `self.dev` is a valid instance of a `struct device`.
-#[repr(C)]
-pub struct Device<T: drm::Driver> {
-    dev: Opaque<bindings::drm_device>,
-    data: T::Data,
+/// A [`Device`] in this [`DeviceContext`] is guaranteed to have been registered with userspace
+/// at some point in time.
+pub struct Registered;
+
+impl Sealed for Registered {}
+impl DeviceContext for Registered {}
+
+/// The [`DeviceContext`] of a [`Device`] that may be unregistered and partly uninitialized.
+///
+/// A [`Device`] in this context is only guaranteed to be partly initialized, and may or may not
+/// be registered with userspace. Thus operations which depend on the [`Device`] being fully
+/// initialized, or which depend on the [`Device`] being registered with userspace are not
+/// available through this [`DeviceContext`].
+///
+/// A [`Device`] in this context can be used to create a
+/// [`Registration`](drm::driver::Registration).
+pub struct Uninit;
+
+impl Sealed for Uninit {}
+impl DeviceContext for Uninit {}
+
+/// 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,
+/// and can be used to create a [`Registration`](drm::driver::Registration) once the driver is ready
+/// to register the device with userspace.
+///
+/// Since DRM device initialization must be single-threaded, this object is not thread-safe.
+///
+/// # Invariants
+///
+/// The device in `self.0` is guaranteed to be a newly created [`Device`] that has not yet been
+/// registered with userspace until this type is dropped.
+pub struct UnregisteredDevice<T: drm::Driver>(ARef<Device<T, Uninit>>, NotThreadSafe);
+
+impl<T: drm::Driver> Deref for UnregisteredDevice<T> {
+    type Target = Device<T, Uninit>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
 }
 
-impl<T: drm::Driver> Device<T> {
+impl<T: drm::Driver> UnregisteredDevice<T> {
     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),
         unload: None,
-        release: Some(Self::release),
+        release: Some(Device::<T>::release),
         master_set: None,
         master_drop: None,
         debugfs_init: None,
@@ -108,8 +185,10 @@ impl<T: drm::Driver> Device<T> {
 
     const GEM_FOPS: bindings::file_operations = drm::gem::create_fops();
 
-    /// Create a new `drm::Device` for a `drm::Driver`.
-    pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<ARef<Self>> {
+    /// 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> {
         // `__drm_dev_alloc` uses `kmalloc()` to allocate memory, hence ensure a `kmalloc()`
         // compatible `Layout`.
         let layout = Kmalloc::aligned_layout(Layout::new::<Self>());
@@ -117,12 +196,12 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<A
         // SAFETY:
         // - `VTABLE`, as a `const` is pinned to the read-only section of the compilation,
         // - `dev` is valid by its type invarants,
-        let raw_drm: *mut Self = unsafe {
+        let raw_drm: *mut Device<T, Uninit> = unsafe {
             bindings::__drm_dev_alloc(
                 dev.as_raw(),
                 &Self::VTABLE,
                 layout.size(),
-                mem::offset_of!(Self, dev),
+                mem::offset_of!(Device<T, Uninit>, dev),
             )
         }
         .cast();
@@ -137,7 +216,7 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<A
         unsafe { data.__pinned_init(raw_data) }.inspect_err(|_| {
             // SAFETY: `raw_drm` is a valid pointer to `Self`, given that `__drm_dev_alloc` was
             // successful.
-            let drm_dev = unsafe { Self::into_drm_device(raw_drm) };
+            let drm_dev = unsafe { Device::into_drm_device(raw_drm) };
 
             // SAFETY: `__drm_dev_alloc()` was successful, hence `drm_dev` must be valid and the
             // refcount must be non-zero.
@@ -146,9 +225,39 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<A
 
         // SAFETY: The reference count is one, and now we take ownership of that reference as a
         // `drm::Device`.
-        Ok(unsafe { ARef::from_raw(raw_drm) })
+        // INVARIANT: We just created the device above, but have yet to call `drm_dev_register`.
+        // `Self` cannot be copied or sent to another thread - ensuring that `drm_dev_register`
+        // won't be called during its lifetime and that the device is unregistered.
+        Ok(Self(unsafe { ARef::from_raw(raw_drm) }, NotThreadSafe))
     }
+}
 
+/// A typed DRM device with a specific [`drm::Driver`] implementation and [`DeviceContext`].
+///
+/// Since DRM devices can be used before being fully initialized and registered with userspace, `C`
+/// represents the furthest [`DeviceContext`] we can guarantee that this [`Device`] has reached.
+///
+/// Keep in mind: this means that an unregistered device can still have the registration state
+/// [`Registered`] as long as it was registered with userspace once in the past, and that the
+/// behavior of such a device is still well-defined. Additionally, a device with the registration
+/// state [`Uninit`] simply does not have a guaranteed registration state at compile time, and could
+/// be either registered or unregistered. Since there is no way to guarantee a long-lived reference
+/// to an unregistered device would remain unregistered, we do not provide a [`DeviceContext`] for
+/// this.
+///
+/// # Invariants
+///
+/// * `self.dev` is a valid instance of a `struct device`.
+/// * The data layout of `Self` remains the same across all implementations of `C`.
+/// * Any invariants for `C` also apply.
+#[repr(C)]
+pub struct Device<T: drm::Driver, C: DeviceContext = Registered> {
+    dev: Opaque<bindings::drm_device>,
+    data: T::Data,
+    _ctx: PhantomData<C>,
+}
+
+impl<T: drm::Driver, C: DeviceContext> Device<T, C> {
     pub(crate) fn as_raw(&self) -> *mut bindings::drm_device {
         self.dev.get()
     }
@@ -174,13 +283,13 @@ unsafe fn into_drm_device(ptr: NonNull<Self>) -> *mut bindings::drm_device {
     ///
     /// # Safety
     ///
-    /// Callers must ensure that `ptr` is valid, non-null, and has a non-zero reference count,
-    /// i.e. it must be ensured that the reference count of the C `struct drm_device` `ptr` points
-    /// to can't drop to zero, for the duration of this function call and the entire duration when
-    /// the returned reference exists.
-    ///
-    /// Additionally, callers must ensure that the `struct device`, `ptr` is pointing to, is
-    /// embedded in `Self`.
+    /// * Callers must ensure that `ptr` is valid, non-null, and has a non-zero reference count,
+    ///   i.e. it must be ensured that the reference count of the C `struct drm_device` `ptr` points
+    ///   to can't drop to zero, for the duration of this function call and the entire duration when
+    ///   the returned reference exists.
+    /// * Additionally, callers must ensure that the `struct device`, `ptr` is pointing to, is
+    ///   embedded in `Self`.
+    /// * Callers promise that any type invariants of `C` will be upheld.
     #[doc(hidden)]
     pub unsafe fn from_raw<'a>(ptr: *const bindings::drm_device) -> &'a Self {
         // SAFETY: By the safety requirements of this function `ptr` is a valid pointer to a
@@ -200,9 +309,20 @@ extern "C" fn release(ptr: *mut bindings::drm_device) {
         // - `this` is valid for dropping.
         unsafe { core::ptr::drop_in_place(this) };
     }
+
+    /// Change the [`DeviceContext`] for a [`Device`].
+    ///
+    /// # Safety
+    ///
+    /// The caller promises that `self` fulfills all of the guarantees provided by the given
+    /// [`DeviceContext`].
+    pub(crate) unsafe fn assume_ctx<NewCtx: DeviceContext>(&self) -> &Device<T, NewCtx> {
+        // SAFETY: The data layout is identical via our type invariants.
+        unsafe { mem::transmute(self) }
+    }
 }
 
-impl<T: drm::Driver> Deref for Device<T> {
+impl<T: drm::Driver, C: DeviceContext> Deref for Device<T, C> {
     type Target = T::Data;
 
     fn deref(&self) -> &Self::Target {
@@ -212,7 +332,7 @@ fn deref(&self) -> &Self::Target {
 
 // SAFETY: DRM device objects are always reference counted and the get/put functions
 // satisfy the requirements.
-unsafe impl<T: drm::Driver> AlwaysRefCounted for Device<T> {
+unsafe impl<T: drm::Driver, C: DeviceContext> AlwaysRefCounted for Device<T, C> {
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
         unsafe { bindings::drm_dev_get(self.as_raw()) };
@@ -227,7 +347,7 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
     }
 }
 
-impl<T: drm::Driver> AsRef<device::Device> for Device<T> {
+impl<T: drm::Driver, C: DeviceContext> AsRef<device::Device> for Device<T, C> {
     fn as_ref(&self) -> &device::Device {
         // SAFETY: `bindings::drm_device::dev` is valid as long as the DRM device itself is valid,
         // which is guaranteed by the type invariant.
@@ -236,8 +356,8 @@ fn as_ref(&self) -> &device::Device {
 }
 
 // SAFETY: A `drm::Device` can be released from any thread.
-unsafe impl<T: drm::Driver> Send for Device<T> {}
+unsafe impl<T: drm::Driver, C: DeviceContext> Send for Device<T, C> {}
 
 // SAFETY: A `drm::Device` can be shared among threads because all immutable methods are protected
 // by the synchronization in `struct drm_device`.
-unsafe impl<T: drm::Driver> Sync for Device<T> {}
+unsafe impl<T: drm::Driver, C: DeviceContext> Sync for Device<T, C> {}
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index 5233bdebc9fcd..55b01ee088548 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -13,6 +13,10 @@
     prelude::*,
     sync::aref::ARef, //
 };
+use core::{
+    mem,
+    ptr::NonNull, //
+};
 
 /// Driver use the GEM memory manager. This should be set for all modern drivers.
 pub(crate) const FEAT_GEM: u32 = bindings::drm_driver_feature_DRIVER_GEM;
@@ -123,21 +127,31 @@ pub trait Driver {
 pub struct Registration<T: Driver>(ARef<drm::Device<T>>);
 
 impl<T: Driver> Registration<T> {
-    fn new(drm: &drm::Device<T>, flags: usize) -> Result<Self> {
+    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) })?;
 
-        Ok(Self(drm.into()))
+        // SAFETY: We just called `drm_dev_register` above
+        let new = NonNull::from(unsafe { drm.assume_ctx() });
+
+        // Leak the ARef from UnregisteredDevice in preparation for transferring its ownership.
+        mem::forget(drm);
+
+        // SAFETY: `drm`'s `Drop` constructor was never called, ensuring that there remains at least
+        // one reference to the device - which we take ownership over here.
+        let new = unsafe { ARef::from_raw(new) };
+
+        Ok(Self(new))
     }
 
-    /// Registers a new [`Device`](drm::Device) with userspace.
+    /// Registers a new [`UnregisteredDevice`](drm::UnregisteredDevice) with userspace.
     ///
     /// Ownership of the [`Registration`] object is passed to [`devres::register`].
-    pub fn new_foreign_owned(
-        drm: &drm::Device<T>,
-        dev: &device::Device<device::Bound>,
+    pub fn new_foreign_owned<'a>(
+        drm: drm::UnregisteredDevice<T>,
+        dev: &'a device::Device<device::Bound>,
         flags: usize,
-    ) -> Result
+    ) -> Result<&'a drm::Device<T>>
     where
         T: 'static,
     {
@@ -146,8 +160,13 @@ pub fn new_foreign_owned(
         }
 
         let reg = Registration::<T>::new(drm, flags)?;
+        let drm = NonNull::from(reg.device());
+
+        devres::register(dev, reg, GFP_KERNEL)?;
 
-        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() })
     }
 
     /// Returns a reference to the `Device` instance for this registration.
diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
index 1b82b6945edf2..64a43cb0fe57c 100644
--- a/rust/kernel/drm/mod.rs
+++ b/rust/kernel/drm/mod.rs
@@ -9,6 +9,10 @@
 pub mod ioctl;
 
 pub use self::device::Device;
+pub use self::device::DeviceContext;
+pub use self::device::Registered;
+pub use self::device::Uninit;
+pub use self::device::UnregisteredDevice;
 pub use self::driver::Driver;
 pub use self::driver::DriverInfo;
 pub use self::driver::Registration;
-- 
2.52.0

[PATCH v5 2/4] rust/drm: Don't setup private driver data until registration

[Lyude Paul]

Now that we have a DeviceContext that we can use to represent whether a
Device is known to have been registered, we can make it so that drivers can
create their Devices but wait until the registration phase to assign their
private data to the Device. This is desirable as some drivers need to make
use of the DRM device early on before finalizing their private driver data.

As such, this change makes it so that the driver's private data can
currently only be accessed through Device<T, Registered> types and not
Device<T, Uninit>.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
V4:
* Remove accidental double-aliasing &mut in Device::release()
---
 drivers/gpu/drm/nova/driver.rs |  4 +--
 drivers/gpu/drm/tyr/driver.rs  |  4 +--
 rust/kernel/drm/device.rs      | 64 +++++++++++++++++++++-------------
 rust/kernel/drm/driver.rs      | 19 ++++++++--
 4 files changed, 59 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index 99d6841b69cbc..8cea5f68c3b04 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -56,8 +56,8 @@ impl auxiliary::Driver for NovaDriver {
     fn probe(adev: &auxiliary::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> {
         let data = try_pin_init!(NovaData { adev: adev.into() });
 
-        let drm = drm::UnregisteredDevice::<Self>::new(adev.as_ref(), data)?;
-        let drm = drm::Registration::new_foreign_owned(drm, adev.as_ref(), 0)?;
+        let drm = drm::UnregisteredDevice::<Self>::new(adev.as_ref())?;
+        let drm = drm::Registration::new_foreign_owned(drm, adev.as_ref(), data, 0)?;
 
         Ok(Self { drm: drm.into() })
     }
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index 0d479b5d7bd62..b574ee1c24587 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -133,8 +133,8 @@ fn probe(
                 gpu_info,
         });
 
-        let tdev = drm::UnregisteredDevice::<Self>::new(pdev.as_ref(), data)?;
-        let tdev = drm::driver::Registration::new_foreign_owned(tdev, pdev.as_ref(), 0)?;
+        let tdev = drm::UnregisteredDevice::<Self>::new(pdev.as_ref())?;
+        let tdev = drm::driver::Registration::new_foreign_owned(tdev, pdev.as_ref(), data, 0)?;
 
         let driver = TyrDriver {
             _device: tdev.into(),
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 8f1780b8f0196..a1d9b0e92f3fd 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -26,13 +26,15 @@
 };
 use core::{
     alloc::Layout,
+    cell::UnsafeCell,
     marker::PhantomData,
-    mem,
-    ops::Deref,
-    ptr::{
+    mem::{
         self,
-        NonNull, //
+        MaybeUninit, //
     },
+    ops::Deref,
+    ptr::NonNull,
+    sync::atomic::*,
 };
 
 #[cfg(CONFIG_DRM_LEGACY)]
@@ -141,7 +143,7 @@ impl DeviceContext for Uninit {}
 ///
 /// The device in `self.0` is guaranteed to be a newly created [`Device`] that has not yet been
 /// registered with userspace until this type is dropped.
-pub struct UnregisteredDevice<T: drm::Driver>(ARef<Device<T, Uninit>>, NotThreadSafe);
+pub struct UnregisteredDevice<T: drm::Driver>(pub(crate) ARef<Device<T, Uninit>>, NotThreadSafe);
 
 impl<T: drm::Driver> Deref for UnregisteredDevice<T> {
     type Target = Device<T, Uninit>;
@@ -188,7 +190,7 @@ impl<T: drm::Driver> UnregisteredDevice<T> {
     /// 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: &device::Device) -> Result<Self> {
         // `__drm_dev_alloc` uses `kmalloc()` to allocate memory, hence ensure a `kmalloc()`
         // compatible `Layout`.
         let layout = Kmalloc::aligned_layout(Layout::new::<Self>());
@@ -207,22 +209,6 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<S
         .cast();
         let raw_drm = NonNull::new(from_err_ptr(raw_drm)?).ok_or(ENOMEM)?;
 
-        // SAFETY: `raw_drm` is a valid pointer to `Self`.
-        let raw_data = unsafe { ptr::addr_of_mut!((*raw_drm.as_ptr()).data) };
-
-        // SAFETY:
-        // - `raw_data` is a valid pointer to uninitialized memory.
-        // - `raw_data` will not move until it is dropped.
-        unsafe { data.__pinned_init(raw_data) }.inspect_err(|_| {
-            // SAFETY: `raw_drm` is a valid pointer to `Self`, given that `__drm_dev_alloc` was
-            // successful.
-            let drm_dev = unsafe { Device::into_drm_device(raw_drm) };
-
-            // SAFETY: `__drm_dev_alloc()` was successful, hence `drm_dev` must be valid and the
-            // refcount must be non-zero.
-            unsafe { bindings::drm_dev_put(drm_dev) };
-        })?;
-
         // 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`.
@@ -253,7 +239,15 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<S
 #[repr(C)]
 pub struct Device<T: drm::Driver, C: DeviceContext = Registered> {
     dev: Opaque<bindings::drm_device>,
-    data: T::Data,
+
+    /// Keeps track of whether we've initialized the device data yet.
+    pub(crate) data_is_init: AtomicBool,
+
+    /// The Driver's private data.
+    ///
+    /// This must only be written to from [`drm::Registration::new`].
+    pub(crate) data: UnsafeCell<MaybeUninit<T::Data>>,
+
     _ctx: PhantomData<C>,
 }
 
@@ -304,6 +298,22 @@ extern "C" fn release(ptr: *mut bindings::drm_device) {
         // SAFETY: `ptr` is a valid pointer to a `struct drm_device` and embedded in `Self`.
         let this = unsafe { Self::from_drm_device(ptr) };
 
+        // SAFETY:
+        // - Since we are in release(), we are guaranteed that no one else has access to `this`.
+        // - We confirmed above that `this` is a valid pointer to an initialized `Self`.
+        let is_init = unsafe { &*this }.data_is_init.load(Ordering::Relaxed);
+        if is_init {
+            // SAFETY:
+            // - We confirmed we have unique access to this above.
+            // - We confirmed that `data` is initialized above.
+            let data_ptr = unsafe { &mut (*this).data };
+
+            // SAFETY:
+            // - We checked that the data is initialized above.
+            // - We do not use `data` any point after calling this function.
+            unsafe { data_ptr.get_mut().assume_init_drop() };
+        }
+
         // SAFETY:
         // - When `release` runs it is guaranteed that there is no further access to `this`.
         // - `this` is valid for dropping.
@@ -322,11 +332,15 @@ pub(crate) unsafe fn assume_ctx<NewCtx: DeviceContext>(&self) -> &Device<T, NewC
     }
 }
 
-impl<T: drm::Driver, C: DeviceContext> Deref for Device<T, C> {
+impl<T: drm::Driver> Deref for Device<T, Registered> {
     type Target = T::Data;
 
     fn deref(&self) -> &Self::Target {
-        &self.data
+        // SAFETY:
+        // - `data` is initialized before any `Device`s with the `Registered` context are available
+        //   to the user.
+        // - `data` is only written to once in `Registration::new()`, so this read will never race.
+        unsafe { (&*self.data.get()).assume_init_ref() }
     }
 }
 
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index 55b01ee088548..cfb8884ece700 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -15,7 +15,8 @@
 };
 use core::{
     mem,
-    ptr::NonNull, //
+    ptr::NonNull,
+    sync::atomic::*, //
 };
 
 /// Driver use the GEM memory manager. This should be set for all modern drivers.
@@ -127,7 +128,18 @@ pub trait Driver {
 pub struct Registration<T: Driver>(ARef<drm::Device<T>>);
 
 impl<T: Driver> Registration<T> {
-    fn new(drm: drm::UnregisteredDevice<T>, flags: usize) -> Result<Self> {
+    fn new(
+        drm: drm::UnregisteredDevice<T>,
+        data: impl PinInit<T::Data, Error>,
+        flags: usize,
+    ) -> Result<Self> {
+        // SAFETY:
+        // - `raw_data` is a valid pointer to uninitialized memory.
+        // - `raw_data` will not move until it is dropped.
+        unsafe { data.__pinned_init(drm.0.data.get().cast()) }?;
+
+        drm.data_is_init.store(true, Ordering::Relaxed);
+
         // SAFETY: `drm.as_raw()` is valid by the invariants of `drm::Device`.
         to_result(unsafe { bindings::drm_dev_register(drm.as_raw(), flags) })?;
 
@@ -150,6 +162,7 @@ fn new(drm: drm::UnregisteredDevice<T>, flags: usize) -> Result<Self> {
     pub fn new_foreign_owned<'a>(
         drm: drm::UnregisteredDevice<T>,
         dev: &'a device::Device<device::Bound>,
+        data: impl PinInit<T::Data, Error>,
         flags: usize,
     ) -> Result<&'a drm::Device<T>>
     where
@@ -159,7 +172,7 @@ pub fn new_foreign_owned<'a>(
             return Err(EINVAL);
         }
 
-        let reg = Registration::<T>::new(drm, flags)?;
+        let reg = Registration::<T>::new(drm, data, flags)?;
         let drm = NonNull::from(reg.device());
 
         devres::register(dev, reg, GFP_KERNEL)?;
-- 
2.52.0

[PATCH v5 3/4] rust/drm/gem: Add DriverAllocImpl type alias

[Lyude Paul]

This is just a type alias that resolves into the AllocImpl for a given
  T: drm::gem::DriverObject

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 rust/kernel/drm/gem/mod.rs | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index b4199945db378..ad74c5159f725 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -33,6 +33,11 @@
 /// [`DriverFile`]: drm::file::DriverFile
 pub type DriverFile<T> = drm::File<<<T as DriverObject>::Driver as drm::Driver>::File>;
 
+/// A type alias for retrieving the current [`AllocImpl`] for a given [`DriverObject`].
+///
+/// [`Driver`]: drm::Driver
+pub type DriverAllocImpl<T> = <<T as DriverObject>::Driver as drm::Driver>::Object;
+
 /// GEM object functions, which must be implemented by drivers.
 pub trait DriverObject: Sync + Send + Sized {
     /// Parent `Driver` for this object.
@@ -42,12 +47,12 @@ pub trait DriverObject: Sync + Send + Sized {
     fn new(dev: &drm::Device<Self::Driver>, size: usize) -> impl PinInit<Self, Error>;
 
     /// Open a new handle to an existing object, associated with a File.
-    fn open(_obj: &<Self::Driver as drm::Driver>::Object, _file: &DriverFile<Self>) -> Result {
+    fn open(_obj: &DriverAllocImpl<Self>, _file: &DriverFile<Self>) -> Result {
         Ok(())
     }
 
     /// Close a handle to an existing object, associated with a File.
-    fn close(_obj: &<Self::Driver as drm::Driver>::Object, _file: &DriverFile<Self>) {}
+    fn close(_obj: &DriverAllocImpl<Self>, _file: &DriverFile<Self>) {}
 }
 
 /// Trait that represents a GEM object subtype
-- 
2.52.0

[PATCH v5 4/4] rust/drm/gem: Use DeviceContext with GEM objects

[Lyude Paul]

Now that we have the ability to represent the context in which a DRM device
is in at compile-time, we can start carrying around this context with GEM
object types in order to allow a driver to safely create GEM objects before
a DRM device has registered with userspace.

Signed-off-by: Lyude Paul <lyude@redhat.com>

---
V4:
* Add a comment to explain the Uninit DeviceContext usage in the GEM object
  vtable (tl;dr: the DeviceContext is meaningless here)

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/nova/driver.rs |  2 +-
 drivers/gpu/drm/nova/gem.rs    | 11 ++++---
 drivers/gpu/drm/tyr/driver.rs  |  2 +-
 drivers/gpu/drm/tyr/gem.rs     |  3 +-
 rust/kernel/drm/device.rs      | 18 ++++++-----
 rust/kernel/drm/driver.rs      |  2 +-
 rust/kernel/drm/gem/mod.rs     | 57 ++++++++++++++++++++++------------
 7 files changed, 61 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index 8cea5f68c3b04..2c13261450406 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -67,7 +67,7 @@ fn probe(adev: &auxiliary::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<S
 impl drm::Driver for NovaDriver {
     type Data = NovaData;
     type File = File;
-    type Object = gem::Object<NovaObject>;
+    type Object<Ctx: drm::DeviceContext> = gem::Object<NovaObject, Ctx>;
 
     const INFO: drm::DriverInfo = INFO;
 
diff --git a/drivers/gpu/drm/nova/gem.rs b/drivers/gpu/drm/nova/gem.rs
index 6ccfa5da57617..f6e98b9db58d8 100644
--- a/drivers/gpu/drm/nova/gem.rs
+++ b/drivers/gpu/drm/nova/gem.rs
@@ -2,7 +2,7 @@
 
 use kernel::{
     drm,
-    drm::{gem, gem::BaseObject},
+    drm::{gem, gem::BaseObject, DeviceContext},
     page,
     prelude::*,
     sync::aref::ARef,
@@ -20,20 +20,23 @@ pub(crate) struct NovaObject {}
 impl gem::DriverObject for NovaObject {
     type Driver = NovaDriver;
 
-    fn new(_dev: &NovaDevice, _size: usize) -> impl PinInit<Self, Error> {
+    fn new<Ctx: DeviceContext>(_dev: &NovaDevice<Ctx>, _size: usize) -> impl PinInit<Self, Error> {
         try_pin_init!(NovaObject {})
     }
 }
 
 impl NovaObject {
     /// Create a new DRM GEM object.
-    pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result<ARef<gem::Object<Self>>> {
+    pub(crate) fn new<Ctx: DeviceContext>(
+        dev: &NovaDevice<Ctx>,
+        size: usize,
+    ) -> Result<ARef<gem::Object<Self, Ctx>>> {
         if size == 0 {
             return Err(EINVAL);
         }
         let aligned_size = page::page_align(size).ok_or(EINVAL)?;
 
-        gem::Object::new(dev, aligned_size)
+        gem::Object::<Self, Ctx>::new(dev, aligned_size)
     }
 
     /// Look up a GEM object handle for a `File` and return an `ObjectRef` for it.
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index b574ee1c24587..170ffdfd92a18 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -177,7 +177,7 @@ fn drop(self: Pin<&mut Self>) {
 impl drm::Driver for TyrDriver {
     type Data = TyrData;
     type File = File;
-    type Object = drm::gem::Object<TyrObject>;
+    type Object<R: drm::DeviceContext> = drm::gem::Object<TyrObject, R>;
 
     const INFO: drm::DriverInfo = INFO;
 
diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs
index 1273bf89dbd5d..00804f8c14bd4 100644
--- a/drivers/gpu/drm/tyr/gem.rs
+++ b/drivers/gpu/drm/tyr/gem.rs
@@ -3,6 +3,7 @@
 use crate::driver::TyrDevice;
 use crate::driver::TyrDriver;
 use kernel::drm::gem;
+use kernel::drm::DeviceContext;
 use kernel::prelude::*;
 
 /// GEM Object inner driver data
@@ -12,7 +13,7 @@ pub(crate) struct TyrObject {}
 impl gem::DriverObject for TyrObject {
     type Driver = TyrDriver;
 
-    fn new(_dev: &TyrDevice, _size: usize) -> impl PinInit<Self, Error> {
+    fn new<Ctx: DeviceContext>(_dev: &TyrDevice<Ctx>, _size: usize) -> impl PinInit<Self, Error> {
         try_pin_init!(TyrObject {})
     }
 }
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index a1d9b0e92f3fd..8dd1508c51cec 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -163,13 +163,17 @@ impl<T: drm::Driver> UnregisteredDevice<T> {
         master_set: None,
         master_drop: None,
         debugfs_init: None,
-        gem_create_object: T::Object::ALLOC_OPS.gem_create_object,
-        prime_handle_to_fd: T::Object::ALLOC_OPS.prime_handle_to_fd,
-        prime_fd_to_handle: T::Object::ALLOC_OPS.prime_fd_to_handle,
-        gem_prime_import: T::Object::ALLOC_OPS.gem_prime_import,
-        gem_prime_import_sg_table: T::Object::ALLOC_OPS.gem_prime_import_sg_table,
-        dumb_create: T::Object::ALLOC_OPS.dumb_create,
-        dumb_map_offset: T::Object::ALLOC_OPS.dumb_map_offset,
+
+        // Ignore the Uninit DeviceContext below. It is only provided because it is required by the
+        // compiler, and it is not actually used by these functions.
+        gem_create_object: T::Object::<Uninit>::ALLOC_OPS.gem_create_object,
+        prime_handle_to_fd: T::Object::<Uninit>::ALLOC_OPS.prime_handle_to_fd,
+        prime_fd_to_handle: T::Object::<Uninit>::ALLOC_OPS.prime_fd_to_handle,
+        gem_prime_import: T::Object::<Uninit>::ALLOC_OPS.gem_prime_import,
+        gem_prime_import_sg_table: T::Object::<Uninit>::ALLOC_OPS.gem_prime_import_sg_table,
+        dumb_create: T::Object::<Uninit>::ALLOC_OPS.dumb_create,
+        dumb_map_offset: T::Object::<Uninit>::ALLOC_OPS.dumb_map_offset,
+
         show_fdinfo: None,
         fbdev_probe: None,
 
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index cfb8884ece700..e6893f089733d 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -110,7 +110,7 @@ pub trait Driver {
     type Data: Sync + Send;
 
     /// The type used to manage memory for this driver.
-    type Object: AllocImpl;
+    type Object<Ctx: drm::DeviceContext>: AllocImpl;
 
     /// The type used to represent a DRM File (client)
     type File: drm::file::DriverFile;
diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
index ad74c5159f725..3af9f52f8eda4 100644
--- a/rust/kernel/drm/gem/mod.rs
+++ b/rust/kernel/drm/gem/mod.rs
@@ -8,6 +8,10 @@
     bindings,
     drm::{
         self,
+        device::{
+            DeviceContext,
+            Registered, //
+        },
         driver::{
             AllocImpl,
             AllocOps, //
@@ -22,6 +26,7 @@
     types::Opaque,
 };
 use core::{
+    marker::PhantomData,
     ops::Deref,
     ptr::NonNull, //
 };
@@ -36,7 +41,8 @@
 /// A type alias for retrieving the current [`AllocImpl`] for a given [`DriverObject`].
 ///
 /// [`Driver`]: drm::Driver
-pub type DriverAllocImpl<T> = <<T as DriverObject>::Driver as drm::Driver>::Object;
+pub type DriverAllocImpl<T, Ctx = Registered> =
+    <<T as DriverObject>::Driver as drm::Driver>::Object<Ctx>;
 
 /// GEM object functions, which must be implemented by drivers.
 pub trait DriverObject: Sync + Send + Sized {
@@ -44,7 +50,10 @@ pub trait DriverObject: Sync + Send + Sized {
     type Driver: drm::Driver;
 
     /// Create a new driver data object for a GEM object of a given size.
-    fn new(dev: &drm::Device<Self::Driver>, size: usize) -> impl PinInit<Self, Error>;
+    fn new<Ctx: DeviceContext>(
+        dev: &drm::Device<Self::Driver, Ctx>,
+        size: usize,
+    ) -> impl PinInit<Self, Error>;
 
     /// Open a new handle to an existing object, associated with a File.
     fn open(_obj: &DriverAllocImpl<Self>, _file: &DriverFile<Self>) -> Result {
@@ -78,9 +87,12 @@ extern "C" fn open_callback<T: DriverObject>(
     // 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) };
 
-    // SAFETY: `open_callback` is specified in the AllocOps structure for `DriverObject<T>`,
-    // ensuring that `raw_obj` is contained within a `DriverObject<T>`
-    let obj = unsafe { <<T::Driver as drm::Driver>::Object as IntoGEMObject>::from_raw(raw_obj) };
+    // SAFETY:
+    // * `open_callback` is specified in the AllocOps structure for `DriverObject`, ensuring that
+    //   `raw_obj` is contained within a `DriverAllocImpl<T>`
+    // * It is only possible for `open_callback` to be called after device registration, ensuring
+    //   that the object's device is in the `Registered` state.
+    let obj: &DriverAllocImpl<T> = unsafe { IntoGEMObject::from_raw(raw_obj) };
 
     match T::open(obj, file) {
         Err(e) => e.to_errno(),
@@ -97,12 +109,12 @@ extern "C" fn close_callback<T: DriverObject>(
 
     // SAFETY: `close_callback` is specified in the AllocOps structure for `Object<T>`, ensuring
     // that `raw_obj` is indeed contained within a `Object<T>`.
-    let obj = unsafe { <<T::Driver as drm::Driver>::Object as IntoGEMObject>::from_raw(raw_obj) };
+    let obj: &DriverAllocImpl<T> = unsafe { IntoGEMObject::from_raw(raw_obj) };
 
     T::close(obj, file);
 }
 
-impl<T: DriverObject> IntoGEMObject for Object<T> {
+impl<T: DriverObject, Ctx: DeviceContext> IntoGEMObject for Object<T, Ctx> {
     fn as_raw(&self) -> *mut bindings::drm_gem_object {
         self.obj.get()
     }
@@ -110,7 +122,7 @@ fn as_raw(&self) -> *mut bindings::drm_gem_object {
     unsafe fn from_raw<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a Self {
         // SAFETY: `obj` is guaranteed to be in an `Object<T>` via the safety contract of this
         // function
-        unsafe { &*crate::container_of!(Opaque::cast_from(self_ptr), Object<T>, obj) }
+        unsafe { &*crate::container_of!(Opaque::cast_from(self_ptr), Object<T, Ctx>, obj) }
     }
 }
 
@@ -127,7 +139,7 @@ fn size(&self) -> usize {
     fn create_handle<D, F>(&self, file: &drm::File<F>) -> Result<u32>
     where
         Self: AllocImpl<Driver = D>,
-        D: drm::Driver<Object = Self, File = F>,
+        D: drm::Driver<Object<Registered> = Self, File = F>,
         F: drm::file::DriverFile<Driver = D>,
     {
         let mut handle: u32 = 0;
@@ -142,7 +154,7 @@ fn create_handle<D, F>(&self, file: &drm::File<F>) -> Result<u32>
     fn lookup_handle<D, F>(file: &drm::File<F>, handle: u32) -> Result<ARef<Self>>
     where
         Self: AllocImpl<Driver = D>,
-        D: drm::Driver<Object = Self, File = F>,
+        D: drm::Driver<Object<Registered> = Self, File = F>,
         F: drm::file::DriverFile<Driver = D>,
     {
         // SAFETY: The arguments are all valid per the type invariants.
@@ -182,16 +194,18 @@ impl<T: IntoGEMObject> BaseObject for T {}
 ///
 /// # Invariants
 ///
-/// - `self.obj` is a valid instance of a `struct drm_gem_object`.
+/// * `self.obj` is a valid instance of a `struct drm_gem_object`.
+/// * Any type invariants of `Ctx` apply to the parent DRM device for this GEM object.
 #[repr(C)]
 #[pin_data]
-pub struct Object<T: DriverObject + Send + Sync> {
+pub struct Object<T: DriverObject + Send + Sync, Ctx: DeviceContext = Registered> {
     obj: Opaque<bindings::drm_gem_object>,
     #[pin]
     data: T,
+    _ctx: PhantomData<Ctx>,
 }
 
-impl<T: DriverObject> Object<T> {
+impl<T: DriverObject, Ctx: DeviceContext> Object<T, Ctx> {
     const OBJECT_FUNCS: bindings::drm_gem_object_funcs = bindings::drm_gem_object_funcs {
         free: Some(Self::free_callback),
         open: Some(open_callback::<T>),
@@ -211,11 +225,12 @@ impl<T: DriverObject> Object<T> {
     };
 
     /// Create a new GEM object.
-    pub fn new(dev: &drm::Device<T::Driver>, size: usize) -> Result<ARef<Self>> {
+    pub fn new(dev: &drm::Device<T::Driver, Ctx>, size: usize) -> Result<ARef<Self>> {
         let obj: Pin<KBox<Self>> = KBox::pin_init(
             try_pin_init!(Self {
                 obj: Opaque::new(bindings::drm_gem_object::default()),
                 data <- T::new(dev, size),
+                _ctx: PhantomData,
             }),
             GFP_KERNEL,
         )?;
@@ -224,6 +239,8 @@ pub fn new(dev: &drm::Device<T::Driver>, size: usize) -> Result<ARef<Self>> {
         unsafe { (*obj.as_raw()).funcs = &Self::OBJECT_FUNCS };
 
         // SAFETY: The arguments are all valid per the type invariants.
+        // INVARIANT: We use `dev` for creating the GEM object, which is known to be in state `Ctx` -
+        // ensuring that the GEM object's pointer to the DRM device is always in the same state.
         to_result(unsafe { bindings::drm_gem_object_init(dev.as_raw(), obj.obj.get(), size) })?;
 
         // SAFETY: We will never move out of `Self` as `ARef<Self>` is always treated as pinned.
@@ -237,13 +254,15 @@ pub fn new(dev: &drm::Device<T::Driver>, size: usize) -> Result<ARef<Self>> {
     }
 
     /// Returns the `Device` that owns this GEM object.
-    pub fn dev(&self) -> &drm::Device<T::Driver> {
+    pub fn dev(&self) -> &drm::Device<T::Driver, Ctx> {
         // SAFETY:
         // - `struct drm_gem_object.dev` is initialized and valid for as long as the GEM
         //   object lives.
         // - The device we used for creating the gem object is passed as &drm::Device<T::Driver> to
         //   Object::<T>::new(), so we know that `T::Driver` is the right generic parameter to use
         //   here.
+        // - Any type invariants of `Ctx` are upheld by using the same `Ctx` for the `Device` we
+        //   return.
         unsafe { drm::Device::from_raw((*self.as_raw()).dev) }
     }
 
@@ -269,7 +288,7 @@ extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) {
 }
 
 // SAFETY: Instances of `Object<T>` are always reference-counted.
-unsafe impl<T: DriverObject> crate::sync::aref::AlwaysRefCounted for Object<T> {
+unsafe impl<T: DriverObject, Ctx: DeviceContext> AlwaysRefCounted for Object<T, Ctx> {
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
         unsafe { bindings::drm_gem_object_get(self.as_raw()) };
@@ -284,9 +303,9 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
     }
 }
 
-impl<T: DriverObject> super::private::Sealed for Object<T> {}
+impl<T: DriverObject, Ctx: DeviceContext> super::private::Sealed for Object<T, Ctx> {}
 
-impl<T: DriverObject> Deref for Object<T> {
+impl<T: DriverObject, Ctx: DeviceContext> Deref for Object<T, Ctx> {
     type Target = T;
 
     fn deref(&self) -> &Self::Target {
@@ -294,7 +313,7 @@ fn deref(&self) -> &Self::Target {
     }
 }
 
-impl<T: DriverObject> AllocImpl for Object<T> {
+impl<T: DriverObject, Ctx: DeviceContext> AllocImpl for Object<T, Ctx> {
     type Driver = T::Driver;
 
     const ALLOC_OPS: AllocOps = AllocOps {
-- 
2.52.0

Re: [PATCH v5 2/4] rust/drm: Don't setup private driver data until registration

[Daniel Almeida]

Hi Lyude,

> On 30 Jan 2026, at 21:13, Lyude Paul <lyude@redhat.com> wrote:
> 
> Now that we have a DeviceContext that we can use to represent whether a
> Device is known to have been registered, we can make it so that drivers can
> create their Devices but wait until the registration phase to assign their
> private data to the Device. This is desirable as some drivers need to make
> use of the DRM device early on before finalizing their private driver data.
> 
> As such, this change makes it so that the driver's private data can
> currently only be accessed through Device<T, Registered> types and not
> Device<T, Uninit>.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---
> V4:
> * Remove accidental double-aliasing &mut in Device::release()
> ---
> drivers/gpu/drm/nova/driver.rs |  4 +--
> drivers/gpu/drm/tyr/driver.rs  |  4 +--
> rust/kernel/drm/device.rs      | 64 +++++++++++++++++++++-------------
> rust/kernel/drm/driver.rs      | 19 ++++++++--
> 4 files changed, 59 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
> index 99d6841b69cbc..8cea5f68c3b04 100644
> --- a/drivers/gpu/drm/nova/driver.rs
> +++ b/drivers/gpu/drm/nova/driver.rs
> @@ -56,8 +56,8 @@ impl auxiliary::Driver for NovaDriver {
>     fn probe(adev: &auxiliary::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> {
>         let data = try_pin_init!(NovaData { adev: adev.into() });
> 
> -        let drm = drm::UnregisteredDevice::<Self>::new(adev.as_ref(), data)?;
> -        let drm = drm::Registration::new_foreign_owned(drm, adev.as_ref(), 0)?;
> +        let drm = drm::UnregisteredDevice::<Self>::new(adev.as_ref())?;
> +        let drm = drm::Registration::new_foreign_owned(drm, adev.as_ref(), data, 0)?;
> 
>         Ok(Self { drm: drm.into() })
>     }
> diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
> index 0d479b5d7bd62..b574ee1c24587 100644
> --- a/drivers/gpu/drm/tyr/driver.rs
> +++ b/drivers/gpu/drm/tyr/driver.rs
> @@ -133,8 +133,8 @@ fn probe(
>                 gpu_info,
>         });
> 
> -        let tdev = drm::UnregisteredDevice::<Self>::new(pdev.as_ref(), data)?;
> -        let tdev = drm::driver::Registration::new_foreign_owned(tdev, pdev.as_ref(), 0)?;
> +        let tdev = drm::UnregisteredDevice::<Self>::new(pdev.as_ref())?;
> +        let tdev = drm::driver::Registration::new_foreign_owned(tdev, pdev.as_ref(), data, 0)?;
> 
>         let driver = TyrDriver {
>             _device: tdev.into(),
> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> index 8f1780b8f0196..a1d9b0e92f3fd 100644
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs
> @@ -26,13 +26,15 @@
> };
> use core::{
>     alloc::Layout,
> +    cell::UnsafeCell,
>     marker::PhantomData,
> -    mem,
> -    ops::Deref,
> -    ptr::{
> +    mem::{
>         self,
> -        NonNull, //
> +        MaybeUninit, //
>     },
> +    ops::Deref,
> +    ptr::NonNull,
> +    sync::atomic::*,
> };
> 
> #[cfg(CONFIG_DRM_LEGACY)]
> @@ -141,7 +143,7 @@ impl DeviceContext for Uninit {}
> ///
> /// The device in `self.0` is guaranteed to be a newly created [`Device`] that has not yet been
> /// registered with userspace until this type is dropped.
> -pub struct UnregisteredDevice<T: drm::Driver>(ARef<Device<T, Uninit>>, NotThreadSafe);
> +pub struct UnregisteredDevice<T: drm::Driver>(pub(crate) ARef<Device<T, Uninit>>, NotThreadSafe);
> 
> impl<T: drm::Driver> Deref for UnregisteredDevice<T> {
>     type Target = Device<T, Uninit>;
> @@ -188,7 +190,7 @@ impl<T: drm::Driver> UnregisteredDevice<T> {
>     /// 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: &device::Device) -> Result<Self> {
>         // `__drm_dev_alloc` uses `kmalloc()` to allocate memory, hence ensure a `kmalloc()`
>         // compatible `Layout`.
>         let layout = Kmalloc::aligned_layout(Layout::new::<Self>());
> @@ -207,22 +209,6 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<S
>         .cast();
>         let raw_drm = NonNull::new(from_err_ptr(raw_drm)?).ok_or(ENOMEM)?;
> 
> -        // SAFETY: `raw_drm` is a valid pointer to `Self`.
> -        let raw_data = unsafe { ptr::addr_of_mut!((*raw_drm.as_ptr()).data) };
> -
> -        // SAFETY:
> -        // - `raw_data` is a valid pointer to uninitialized memory.
> -        // - `raw_data` will not move until it is dropped.
> -        unsafe { data.__pinned_init(raw_data) }.inspect_err(|_| {
> -            // SAFETY: `raw_drm` is a valid pointer to `Self`, given that `__drm_dev_alloc` was
> -            // successful.
> -            let drm_dev = unsafe { Device::into_drm_device(raw_drm) };
> -
> -            // SAFETY: `__drm_dev_alloc()` was successful, hence `drm_dev` must be valid and the
> -            // refcount must be non-zero.
> -            unsafe { bindings::drm_dev_put(drm_dev) };
> -        })?;
> -
>         // 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`.
> @@ -253,7 +239,15 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<S
> #[repr(C)]
> pub struct Device<T: drm::Driver, C: DeviceContext = Registered> {
>     dev: Opaque<bindings::drm_device>,
> -    data: T::Data,
> +
> +    /// Keeps track of whether we've initialized the device data yet.
> +    pub(crate) data_is_init: AtomicBool,

What about this?

	/**
	 * @registered:
	 *
	 * Internally used by drm_dev_register() and drm_connector_register().
	 */
	bool registered;

Can’t we use this in lieu of this flag you’ve added?

> +
> +    /// The Driver's private data.
> +    ///
> +    /// This must only be written to from [`drm::Registration::new`].
> +    pub(crate) data: UnsafeCell<MaybeUninit<T::Data>>,
> +
>     _ctx: PhantomData<C>,
> }
> 

— Daniel

Re: [PATCH v5 2/4] rust/drm: Don't setup private driver data until registration

[lyude]

On Wed, 2026-02-04 at 15:28 -0300, Daniel Almeida wrote:
> > +    pub(crate) data_is_init: AtomicBool,
> 
> What about this?
> 
> 	/**
> 	 * @registered:
> 	 *
> 	 * Internally used by drm_dev_register() and
> drm_connector_register().
> 	 */
> 	bool registered;
> 
> Can’t we use this in lieu of this flag you’ve added?

No we couldn't unfortunately. As I said before: a lot of KMS setup has
to happen prior to registration, and most of those callbacks
(atomic_check, atomic_commit, etc.) can both happen before registration
and after. And that's a lot more difficult to deal with if we don't
have access to the private driver data for any of those callbacks.

So - it really does truly need to be tracked separately.
> 
> > +

Re: [PATCH v5 1/4] rust/drm: Introduce DeviceContext

[Daniel Almeida]

Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

Re: [PATCH v5 2/4] rust/drm: Don't setup private driver data until registration

[Daniel Almeida]

> +
> +    /// Keeps track of whether we've initialized the device data yet.
> +    pub(crate) data_is_init: AtomicBool,

Oh well, I was unable to think of a better plan than this ^

Nit: you’ve added a few pub(crate), consider whether these could
be pub(in path) or, better yet, pub(super).


Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

Re: [PATCH v5 3/4] rust/drm/gem: Add DriverAllocImpl type alias

[Daniel Almeida]

> On 30 Jan 2026, at 21:13, Lyude Paul <lyude@redhat.com> wrote:
> 
> This is just a type alias that resolves into the AllocImpl for a given
>  T: drm::gem::DriverObject
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---
> rust/kernel/drm/gem/mod.rs | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
> index b4199945db378..ad74c5159f725 100644
> --- a/rust/kernel/drm/gem/mod.rs
> +++ b/rust/kernel/drm/gem/mod.rs
> @@ -33,6 +33,11 @@
> /// [`DriverFile`]: drm::file::DriverFile
> pub type DriverFile<T> = drm::File<<<T as DriverObject>::Driver as drm::Driver>::File>;
> 
> +/// A type alias for retrieving the current [`AllocImpl`] for a given [`DriverObject`].
> +///
> +/// [`Driver`]: drm::Driver
> +pub type DriverAllocImpl<T> = <<T as DriverObject>::Driver as drm::Driver>::Object;
> +
> /// GEM object functions, which must be implemented by drivers.
> pub trait DriverObject: Sync + Send + Sized {
>     /// Parent `Driver` for this object.
> @@ -42,12 +47,12 @@ pub trait DriverObject: Sync + Send + Sized {
>     fn new(dev: &drm::Device<Self::Driver>, size: usize) -> impl PinInit<Self, Error>;
> 
>     /// Open a new handle to an existing object, associated with a File.
> -    fn open(_obj: &<Self::Driver as drm::Driver>::Object, _file: &DriverFile<Self>) -> Result {
> +    fn open(_obj: &DriverAllocImpl<Self>, _file: &DriverFile<Self>) -> Result {
>         Ok(())
>     }
> 
>     /// Close a handle to an existing object, associated with a File.
> -    fn close(_obj: &<Self::Driver as drm::Driver>::Object, _file: &DriverFile<Self>) {}
> +    fn close(_obj: &DriverAllocImpl<Self>, _file: &DriverFile<Self>) {}
> }
> 
> /// Trait that represents a GEM object subtype
> -- 
> 2.52.0
> 

Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

Re: [PATCH v5 4/4] rust/drm/gem: Use DeviceContext with GEM objects

[Daniel Almeida]

> On 30 Jan 2026, at 21:13, Lyude Paul <lyude@redhat.com> wrote:
> 
> Now that we have the ability to represent the context in which a DRM device
> is in at compile-time, we can start carrying around this context with GEM
> object types in order to allow a driver to safely create GEM objects before
> a DRM device has registered with userspace.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> 
> ---
> V4:
> * Add a comment to explain the Uninit DeviceContext usage in the GEM object
>  vtable (tl;dr: the DeviceContext is meaningless here)
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---
> drivers/gpu/drm/nova/driver.rs |  2 +-
> drivers/gpu/drm/nova/gem.rs    | 11 ++++---
> drivers/gpu/drm/tyr/driver.rs  |  2 +-
> drivers/gpu/drm/tyr/gem.rs     |  3 +-
> rust/kernel/drm/device.rs      | 18 ++++++-----
> rust/kernel/drm/driver.rs      |  2 +-
> rust/kernel/drm/gem/mod.rs     | 57 ++++++++++++++++++++++------------
> 7 files changed, 61 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
> index 8cea5f68c3b04..2c13261450406 100644
> --- a/drivers/gpu/drm/nova/driver.rs
> +++ b/drivers/gpu/drm/nova/driver.rs
> @@ -67,7 +67,7 @@ fn probe(adev: &auxiliary::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<S
> impl drm::Driver for NovaDriver {
>     type Data = NovaData;
>     type File = File;
> -    type Object = gem::Object<NovaObject>;
> +    type Object<Ctx: drm::DeviceContext> = gem::Object<NovaObject, Ctx>;
> 
>     const INFO: drm::DriverInfo = INFO;
> 
> diff --git a/drivers/gpu/drm/nova/gem.rs b/drivers/gpu/drm/nova/gem.rs
> index 6ccfa5da57617..f6e98b9db58d8 100644
> --- a/drivers/gpu/drm/nova/gem.rs
> +++ b/drivers/gpu/drm/nova/gem.rs
> @@ -2,7 +2,7 @@
> 
> use kernel::{
>     drm,
> -    drm::{gem, gem::BaseObject},
> +    drm::{gem, gem::BaseObject, DeviceContext},
>     page,
>     prelude::*,
>     sync::aref::ARef,
> @@ -20,20 +20,23 @@ pub(crate) struct NovaObject {}
> impl gem::DriverObject for NovaObject {
>     type Driver = NovaDriver;
> 
> -    fn new(_dev: &NovaDevice, _size: usize) -> impl PinInit<Self, Error> {
> +    fn new<Ctx: DeviceContext>(_dev: &NovaDevice<Ctx>, _size: usize) -> impl PinInit<Self, Error> {
>         try_pin_init!(NovaObject {})
>     }
> }
> 
> impl NovaObject {
>     /// Create a new DRM GEM object.
> -    pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result<ARef<gem::Object<Self>>> {
> +    pub(crate) fn new<Ctx: DeviceContext>(
> +        dev: &NovaDevice<Ctx>,
> +        size: usize,
> +    ) -> Result<ARef<gem::Object<Self, Ctx>>> {
>         if size == 0 {
>             return Err(EINVAL);
>         }
>         let aligned_size = page::page_align(size).ok_or(EINVAL)?;
> 
> -        gem::Object::new(dev, aligned_size)
> +        gem::Object::<Self, Ctx>::new(dev, aligned_size)
>     }
> 
>     /// Look up a GEM object handle for a `File` and return an `ObjectRef` for it.
> diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
> index b574ee1c24587..170ffdfd92a18 100644
> --- a/drivers/gpu/drm/tyr/driver.rs
> +++ b/drivers/gpu/drm/tyr/driver.rs
> @@ -177,7 +177,7 @@ fn drop(self: Pin<&mut Self>) {
> impl drm::Driver for TyrDriver {
>     type Data = TyrData;
>     type File = File;
> -    type Object = drm::gem::Object<TyrObject>;
> +    type Object<R: drm::DeviceContext> = drm::gem::Object<TyrObject, R>;
> 
>     const INFO: drm::DriverInfo = INFO;
> 
> diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs
> index 1273bf89dbd5d..00804f8c14bd4 100644
> --- a/drivers/gpu/drm/tyr/gem.rs
> +++ b/drivers/gpu/drm/tyr/gem.rs
> @@ -3,6 +3,7 @@
> use crate::driver::TyrDevice;
> use crate::driver::TyrDriver;
> use kernel::drm::gem;
> +use kernel::drm::DeviceContext;
> use kernel::prelude::*;
> 
> /// GEM Object inner driver data
> @@ -12,7 +13,7 @@ pub(crate) struct TyrObject {}
> impl gem::DriverObject for TyrObject {
>     type Driver = TyrDriver;
> 
> -    fn new(_dev: &TyrDevice, _size: usize) -> impl PinInit<Self, Error> {
> +    fn new<Ctx: DeviceContext>(_dev: &TyrDevice<Ctx>, _size: usize) -> impl PinInit<Self, Error> {
>         try_pin_init!(TyrObject {})
>     }
> }
> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> index a1d9b0e92f3fd..8dd1508c51cec 100644
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs
> @@ -163,13 +163,17 @@ impl<T: drm::Driver> UnregisteredDevice<T> {
>         master_set: None,
>         master_drop: None,
>         debugfs_init: None,
> -        gem_create_object: T::Object::ALLOC_OPS.gem_create_object,
> -        prime_handle_to_fd: T::Object::ALLOC_OPS.prime_handle_to_fd,
> -        prime_fd_to_handle: T::Object::ALLOC_OPS.prime_fd_to_handle,
> -        gem_prime_import: T::Object::ALLOC_OPS.gem_prime_import,
> -        gem_prime_import_sg_table: T::Object::ALLOC_OPS.gem_prime_import_sg_table,
> -        dumb_create: T::Object::ALLOC_OPS.dumb_create,
> -        dumb_map_offset: T::Object::ALLOC_OPS.dumb_map_offset,
> +
> +        // Ignore the Uninit DeviceContext below. It is only provided because it is required by the
> +        // compiler, and it is not actually used by these functions.
> +        gem_create_object: T::Object::<Uninit>::ALLOC_OPS.gem_create_object,
> +        prime_handle_to_fd: T::Object::<Uninit>::ALLOC_OPS.prime_handle_to_fd,
> +        prime_fd_to_handle: T::Object::<Uninit>::ALLOC_OPS.prime_fd_to_handle,
> +        gem_prime_import: T::Object::<Uninit>::ALLOC_OPS.gem_prime_import,
> +        gem_prime_import_sg_table: T::Object::<Uninit>::ALLOC_OPS.gem_prime_import_sg_table,
> +        dumb_create: T::Object::<Uninit>::ALLOC_OPS.dumb_create,
> +        dumb_map_offset: T::Object::<Uninit>::ALLOC_OPS.dumb_map_offset,
> +
>         show_fdinfo: None,
>         fbdev_probe: None,
> 
> diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
> index cfb8884ece700..e6893f089733d 100644
> --- a/rust/kernel/drm/driver.rs
> +++ b/rust/kernel/drm/driver.rs
> @@ -110,7 +110,7 @@ pub trait Driver {
>     type Data: Sync + Send;
> 
>     /// The type used to manage memory for this driver.
> -    type Object: AllocImpl;
> +    type Object<Ctx: drm::DeviceContext>: AllocImpl;
> 
>     /// The type used to represent a DRM File (client)
>     type File: drm::file::DriverFile;
> diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
> index ad74c5159f725..3af9f52f8eda4 100644
> --- a/rust/kernel/drm/gem/mod.rs
> +++ b/rust/kernel/drm/gem/mod.rs
> @@ -8,6 +8,10 @@
>     bindings,
>     drm::{
>         self,
> +        device::{
> +            DeviceContext,
> +            Registered, //
> +        },
>         driver::{
>             AllocImpl,
>             AllocOps, //
> @@ -22,6 +26,7 @@
>     types::Opaque,
> };
> use core::{
> +    marker::PhantomData,
>     ops::Deref,
>     ptr::NonNull, //
> };
> @@ -36,7 +41,8 @@
> /// A type alias for retrieving the current [`AllocImpl`] for a given [`DriverObject`].
> ///
> /// [`Driver`]: drm::Driver
> -pub type DriverAllocImpl<T> = <<T as DriverObject>::Driver as drm::Driver>::Object;
> +pub type DriverAllocImpl<T, Ctx = Registered> =
> +    <<T as DriverObject>::Driver as drm::Driver>::Object<Ctx>;
> 
> /// GEM object functions, which must be implemented by drivers.
> pub trait DriverObject: Sync + Send + Sized {
> @@ -44,7 +50,10 @@ pub trait DriverObject: Sync + Send + Sized {
>     type Driver: drm::Driver;
> 
>     /// Create a new driver data object for a GEM object of a given size.
> -    fn new(dev: &drm::Device<Self::Driver>, size: usize) -> impl PinInit<Self, Error>;
> +    fn new<Ctx: DeviceContext>(
> +        dev: &drm::Device<Self::Driver, Ctx>,
> +        size: usize,
> +    ) -> impl PinInit<Self, Error>;
> 
>     /// Open a new handle to an existing object, associated with a File.
>     fn open(_obj: &DriverAllocImpl<Self>, _file: &DriverFile<Self>) -> Result {
> @@ -78,9 +87,12 @@ extern "C" fn open_callback<T: DriverObject>(
>     // 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) };
> 
> -    // SAFETY: `open_callback` is specified in the AllocOps structure for `DriverObject<T>`,
> -    // ensuring that `raw_obj` is contained within a `DriverObject<T>`
> -    let obj = unsafe { <<T::Driver as drm::Driver>::Object as IntoGEMObject>::from_raw(raw_obj) };
> +    // SAFETY:
> +    // * `open_callback` is specified in the AllocOps structure for `DriverObject`, ensuring that
> +    //   `raw_obj` is contained within a `DriverAllocImpl<T>`
> +    // * It is only possible for `open_callback` to be called after device registration, ensuring
> +    //   that the object's device is in the `Registered` state.
> +    let obj: &DriverAllocImpl<T> = unsafe { IntoGEMObject::from_raw(raw_obj) };
> 
>     match T::open(obj, file) {
>         Err(e) => e.to_errno(),
> @@ -97,12 +109,12 @@ extern "C" fn close_callback<T: DriverObject>(
> 
>     // SAFETY: `close_callback` is specified in the AllocOps structure for `Object<T>`, ensuring
>     // that `raw_obj` is indeed contained within a `Object<T>`.
> -    let obj = unsafe { <<T::Driver as drm::Driver>::Object as IntoGEMObject>::from_raw(raw_obj) };
> +    let obj: &DriverAllocImpl<T> = unsafe { IntoGEMObject::from_raw(raw_obj) };
> 
>     T::close(obj, file);
> }
> 
> -impl<T: DriverObject> IntoGEMObject for Object<T> {
> +impl<T: DriverObject, Ctx: DeviceContext> IntoGEMObject for Object<T, Ctx> {
>     fn as_raw(&self) -> *mut bindings::drm_gem_object {
>         self.obj.get()
>     }
> @@ -110,7 +122,7 @@ fn as_raw(&self) -> *mut bindings::drm_gem_object {
>     unsafe fn from_raw<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a Self {
>         // SAFETY: `obj` is guaranteed to be in an `Object<T>` via the safety contract of this
>         // function
> -        unsafe { &*crate::container_of!(Opaque::cast_from(self_ptr), Object<T>, obj) }
> +        unsafe { &*crate::container_of!(Opaque::cast_from(self_ptr), Object<T, Ctx>, obj) }
>     }
> }
> 
> @@ -127,7 +139,7 @@ fn size(&self) -> usize {
>     fn create_handle<D, F>(&self, file: &drm::File<F>) -> Result<u32>
>     where
>         Self: AllocImpl<Driver = D>,
> -        D: drm::Driver<Object = Self, File = F>,
> +        D: drm::Driver<Object<Registered> = Self, File = F>,
>         F: drm::file::DriverFile<Driver = D>,
>     {
>         let mut handle: u32 = 0;
> @@ -142,7 +154,7 @@ fn create_handle<D, F>(&self, file: &drm::File<F>) -> Result<u32>
>     fn lookup_handle<D, F>(file: &drm::File<F>, handle: u32) -> Result<ARef<Self>>
>     where
>         Self: AllocImpl<Driver = D>,
> -        D: drm::Driver<Object = Self, File = F>,
> +        D: drm::Driver<Object<Registered> = Self, File = F>,
>         F: drm::file::DriverFile<Driver = D>,
>     {
>         // SAFETY: The arguments are all valid per the type invariants.
> @@ -182,16 +194,18 @@ impl<T: IntoGEMObject> BaseObject for T {}
> ///
> /// # Invariants
> ///
> -/// - `self.obj` is a valid instance of a `struct drm_gem_object`.
> +/// * `self.obj` is a valid instance of a `struct drm_gem_object`.
> +/// * Any type invariants of `Ctx` apply to the parent DRM device for this GEM object.
> #[repr(C)]
> #[pin_data]
> -pub struct Object<T: DriverObject + Send + Sync> {
> +pub struct Object<T: DriverObject + Send + Sync, Ctx: DeviceContext = Registered> {
>     obj: Opaque<bindings::drm_gem_object>,
>     #[pin]
>     data: T,
> +    _ctx: PhantomData<Ctx>,
> }
> 
> -impl<T: DriverObject> Object<T> {
> +impl<T: DriverObject, Ctx: DeviceContext> Object<T, Ctx> {
>     const OBJECT_FUNCS: bindings::drm_gem_object_funcs = bindings::drm_gem_object_funcs {
>         free: Some(Self::free_callback),
>         open: Some(open_callback::<T>),
> @@ -211,11 +225,12 @@ impl<T: DriverObject> Object<T> {
>     };
> 
>     /// Create a new GEM object.
> -    pub fn new(dev: &drm::Device<T::Driver>, size: usize) -> Result<ARef<Self>> {
> +    pub fn new(dev: &drm::Device<T::Driver, Ctx>, size: usize) -> Result<ARef<Self>> {
>         let obj: Pin<KBox<Self>> = KBox::pin_init(
>             try_pin_init!(Self {
>                 obj: Opaque::new(bindings::drm_gem_object::default()),
>                 data <- T::new(dev, size),
> +                _ctx: PhantomData,
>             }),
>             GFP_KERNEL,
>         )?;
> @@ -224,6 +239,8 @@ pub fn new(dev: &drm::Device<T::Driver>, size: usize) -> Result<ARef<Self>> {
>         unsafe { (*obj.as_raw()).funcs = &Self::OBJECT_FUNCS };
> 
>         // SAFETY: The arguments are all valid per the type invariants.
> +        // INVARIANT: We use `dev` for creating the GEM object, which is known to be in state `Ctx` -
> +        // ensuring that the GEM object's pointer to the DRM device is always in the same state.

I’d rewrite this, i.e.: they are in the same state at this moment, because
upgrading the typestate in drm::Device will not carry over to the object.

>         to_result(unsafe { bindings::drm_gem_object_init(dev.as_raw(), obj.obj.get(), size) })?;
> 
>         // SAFETY: We will never move out of `Self` as `ARef<Self>` is always treated as pinned.
> @@ -237,13 +254,15 @@ pub fn new(dev: &drm::Device<T::Driver>, size: usize) -> Result<ARef<Self>> {
>     }
> 
>     /// Returns the `Device` that owns this GEM object.
> -    pub fn dev(&self) -> &drm::Device<T::Driver> {
> +    pub fn dev(&self) -> &drm::Device<T::Driver, Ctx> {
>         // SAFETY:
>         // - `struct drm_gem_object.dev` is initialized and valid for as long as the GEM
>         //   object lives.
>         // - The device we used for creating the gem object is passed as &drm::Device<T::Driver> to
>         //   Object::<T>::new(), so we know that `T::Driver` is the right generic parameter to use
>         //   here.
> +        // - Any type invariants of `Ctx` are upheld by using the same `Ctx` for the `Device` we
> +        //   return.
>         unsafe { drm::Device::from_raw((*self.as_raw()).dev) }
>     }
> 
> @@ -269,7 +288,7 @@ extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) {
> }
> 
> // SAFETY: Instances of `Object<T>` are always reference-counted.
> -unsafe impl<T: DriverObject> crate::sync::aref::AlwaysRefCounted for Object<T> {
> +unsafe impl<T: DriverObject, Ctx: DeviceContext> AlwaysRefCounted for Object<T, Ctx> {
>     fn inc_ref(&self) {
>         // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
>         unsafe { bindings::drm_gem_object_get(self.as_raw()) };
> @@ -284,9 +303,9 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
>     }
> }
> 
> -impl<T: DriverObject> super::private::Sealed for Object<T> {}
> +impl<T: DriverObject, Ctx: DeviceContext> super::private::Sealed for Object<T, Ctx> {}
> 
> -impl<T: DriverObject> Deref for Object<T> {
> +impl<T: DriverObject, Ctx: DeviceContext> Deref for Object<T, Ctx> {
>     type Target = T;
> 
>     fn deref(&self) -> &Self::Target {
> @@ -294,7 +313,7 @@ fn deref(&self) -> &Self::Target {
>     }
> }
> 
> -impl<T: DriverObject> AllocImpl for Object<T> {
> +impl<T: DriverObject, Ctx: DeviceContext> AllocImpl for Object<T, Ctx> {
>     type Driver = T::Driver;
> 
>     const ALLOC_OPS: AllocOps = AllocOps {
> -- 
> 2.52.0
> 

Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

Re: [PATCH v5 1/4] rust/drm: Introduce DeviceContext

[Deborah Brouwer]

Hi Lyude,

On Fri, Jan 30, 2026 at 07:13:30PM -0500, Lyude Paul wrote:
> One of the tricky things about DRM bindings in Rust is the fact that
> initialization of a DRM device is a multi-step process. It's quite normal
> for a device driver to start making use of its DRM device for tasks like
> creating GEM objects before userspace registration happens. This is an
> issue in rust though, since prior to userspace registration the device is
> only partly initialized. This means there's a plethora of DRM device
> operations we can't yet expose without opening up the door to UB if the DRM
> device in question isn't yet registered.
> 
> Additionally, this isn't something we can reliably check at runtime. And
> even if we could, performing an operation which requires the device be
> registered when the device isn't actually registered is a programmer bug,
> meaning there's no real way to gracefully handle such a mistake at runtime.
> And even if that wasn't the case, it would be horrendously annoying and
> noisy to have to check if a device is registered constantly throughout a
> driver.
> 
> In order to solve this, we first take inspiration from
> `kernel::device::DeviceContext` and introduce `kernel::drm::DeviceContext`.
> This provides us with a ZST type that we can generalize over to represent
> contexts where a device is known to have been registered with userspace at
> some point in time (`Registered`), along with contexts where we can't make
> such a guarantee (`Uninit`).
> 
> It's important to note we intentionally do not provide a `DeviceContext`
> which represents an unregistered device. This is because there's no
> reasonable way to guarantee that a device with long-living references to
> itself will not be registered eventually with userspace. Instead, we
> provide a new-type for this: `UnregisteredDevice` which can
> provide a guarantee that the `Device` has never been registered with
> userspace. To ensure this, we modify `Registration` so that creating a new
> `Registration` requires passing ownership of an `UnregisteredDevice`.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> 
> ---
> V2:
> * Make sure that `UnregisteredDevice` is not thread-safe (since DRM device
>   initialization is also not thread-safe)
> * Rename from AnyCtx to Uninit, I think this name actually makes a bit more
>   sense.
> * Change assume_registered() to assume_ctx()
>   Since it looks like in some situations, we'll want to update the
>   DeviceContext of a object to the latest DeviceContext we know the Device
>   to be in.
> * Rename Init to Uninit
>   When we eventually add KMS support, we're going to have 3 different
>   DeviceContexts - Uninit, Init, Registered. Additionally, aside from not
>   being registered there are a number of portions of the rest of the Device
>   which also aren't usable before at least the Init context - so the naming
>   of Uninit makes this a little clearer.
> * s/DeviceContext/DeviceContext/
>   For consistency with the rest of the kernel
> * Drop as_ref::<Device<T, Uninit>>() for now since I don't actually think
>   we need this quite yet
> V3:
> * Get rid of drm_dev_ctx!, as we don't actually need to implement Send or
>   Sync ourselves
> * Remove mention of C function in drm::device::Registration rustdoc
> * Add more documentation to the DeviceContext trait, go into detail about
>   the various setup phases and such.
> * Add missing period to comment in `UnregisteredDevice::new()`.
> V4:
> * Address some comments from Danilo I missed last round:
>   * Remove leftover rebase detritus from new_foreign_owned()
>     (the seemingly useless cast)
>   * Remove no-op mention in Registered device context
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---
>  drivers/gpu/drm/nova/driver.rs |   8 +-
>  drivers/gpu/drm/tyr/driver.rs  |  10 +-
>  rust/kernel/drm/device.rs      | 178 +++++++++++++++++++++++++++------
>  rust/kernel/drm/driver.rs      |  35 +++++--
>  rust/kernel/drm/mod.rs         |   4 +
>  5 files changed, 190 insertions(+), 45 deletions(-)
> 
...
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs
...

> +pub struct UnregisteredDevice<T: drm::Driver>(ARef<Device<T, Uninit>>, NotThreadSafe);
> +
> +impl<T: drm::Driver> Deref for UnregisteredDevice<T> {
> +    type Target = Device<T, Uninit>;
> +
> +    fn deref(&self) -> &Self::Target {
> +        &self.0
> +    }
>  }
>  
> -impl<T: drm::Driver> Device<T> {
> +impl<T: drm::Driver> UnregisteredDevice<T> {
>      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),
>          unload: None,
> -        release: Some(Self::release),
> +        release: Some(Device::<T>::release),
>          master_set: None,
>          master_drop: None,
>          debugfs_init: None,
> @@ -108,8 +185,10 @@ impl<T: drm::Driver> Device<T> {
>  
>      const GEM_FOPS: bindings::file_operations = drm::gem::create_fops();
>  
> -    /// Create a new `drm::Device` for a `drm::Driver`.
> -    pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<ARef<Self>> {
> +    /// 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> {
>          // `__drm_dev_alloc` uses `kmalloc()` to allocate memory, hence ensure a `kmalloc()`
>          // compatible `Layout`.
>          let layout = Kmalloc::aligned_layout(Layout::new::<Self>());
> @@ -117,12 +196,12 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<A
>          // SAFETY:
>          // - `VTABLE`, as a `const` is pinned to the read-only section of the compilation,
>          // - `dev` is valid by its type invarants,
> -        let raw_drm: *mut Self = unsafe {
> +        let raw_drm: *mut Device<T, Uninit> = unsafe {
>              bindings::__drm_dev_alloc(
>                  dev.as_raw(),
>                  &Self::VTABLE,
>                  layout.size(),
> -                mem::offset_of!(Self, dev),
> +                mem::offset_of!(Device<T, Uninit>, dev),
>              )
>          }
>          .cast();

When I was testing this series with Tyr, I got this kernel Oops:

[   31.351423] tyr fb000000.gpu: mali-unknown id 0xa867 major 0x67 minor 0x0 status 0x5
[   31.352324] tyr fb000000.gpu: Features: L2:0x7120306 Tiler:0x809 Mem:0x301 MMU:0x2830 AS:0xff
[   31.353299] tyr fb000000.gpu: shader_present=0x0000000000050005 l2_present=0x0000000000000001 tiler_present=0x0000000000000001
[   31.357585] Unable to handle kernel paging request at virtual address ffff003064726553
[   31.358324] Mem abort info:
[   31.358579]   ESR = 0x0000000096000005
[   31.358956]   EC = 0x25: DABT (current EL), IL = 32 bits
[   31.359592]   SET = 0, FnV = 0
[   31.359898]   EA = 0, S1PTW = 0
[   31.360185]   FSC = 0x05: level 1 translation fault
[   31.360620] Data abort info:
[   31.360882]   ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000
[   31.361370]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[   31.361832]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[   31.361839] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000002926000
[   31.361847] [ffff003064726553] pgd=0000000000000000, p4d=18000002fffff403, pud=0000000000000000

[   31.364156] Internal error: Oops: 0000000096000005 [#1]  SMP
[   31.365050] Modules linked in: tyr(+) soundcore sha256 cfg80211 rfkill pci_endpoint_test fuse dm_mod ipv6
[   31.366212] CPU: 3 UID: 0 PID: 254 Comm: (udev-worker) Not tainted 6.19.0-rc5 #281 PREEMPT
[   31.366942] Hardware name: Radxa ROCK 5B (DT)
[   31.367325] pstate: 00400009 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   31.367935] pc : drmm_kmalloc+0x34/0x128
[   31.368286] lr : drm_gem_init+0x70/0xb8
[   31.368627] sp : ffff800081ec3250
[   31.368918] x29: ffff800081ec3250 x28: ffff800082c01000 x27: ffff800081ec3420
[   31.369549] x26: ffff000100dc8a00 x25: ffff0001009ab960 x24: ffff000107a14840
[   31.370179] x23: ffff0001009ab7e0 x22: 0000000000000138 x21: 0000000000000dc0
[   31.370808] x20: ffff000100a801d0 x19: ffff000100a801d0 x18: 0000000000000003
[   31.371438] x17: 0000000000000000 x16: ffffc82e1355a228 x15: ffff000107c08978
[   31.372068] x14: ffffffffffffffff x13: 0000000000000044 x12: 0000000000000000
[   31.372697] x11: 0000000000000030 x10: ffffc82e12bd2db0 x9 : ffff003064726163
[   31.373327] x8 : 00000000000001b8 x7 : ffffc82e107c8dc8 x6 : 0000000000000000
[   31.373956] x5 : 0000000000000000 x4 : 0000000000000003 x3 : 0000000000000000
[   31.374591] x2 : 0000000000000dc0 x1 : 0000000000000dc0 x0 : 00000000000001b8
[   31.375221] Call trace:
[  OK  ] Listening on systemd-hostnamed.sock[   31.375438]  drmm_kmalloc+0x34/0x128 (P)
et - Hostname Service Socket.
[   31.376264]  drm_gem_init+0x70/0xb8
[   31.376838]  drm_dev_init+0x27c/0x30c
[   31.377164]  __drm_dev_alloc+0x44/0x88
[   31.377496]  _RNvMs3_NtNtCs8pcx3nCgX4X_6kernel3drm6deviceINtB5_18UnregisteredDeviceNtNtCs5CCNNSUyUK_3tyr6driver12TyrDrmDriverE3newB19_+0x50/0xe4 [tyr]
[   31.378690]  _RNvXs0_NtCs5CCNNSUyUK_3tyr6driverNtB5_21TyrPlatformDeviceDataNtNtCs8pcx3nCgX4X_6kernel8platform6Driver5probe+0x30c/0x4ec [tyr]
[   31.379794]  _RNvMs0_NtCs8pcx3nCgX4X_6kernel8platformINtB5_7AdapterNtNtCs5CCNNSUyUK_3tyr6driver21TyrPlatformDeviceDataE14probe_callbackBT_+0x60/0x220 [tyr]
[   31.381010]  platform_probe+0x5c/0x9c
[   31.381337]  really_probe+0x154/0x43c
[   31.381665]  __driver_probe_device+0xa4/0x118
[   31.382052]  driver_probe_device+0x40/0x230
[   31.382423]  __driver_attach+0xf8/0x288
[   31.382764]  bus_for_each_dev+0xec/0x144
[   31.383112]  driver_attach+0x24/0x30
[   31.383430]  bus_add_driver+0x14c/0x2a0
[   31.383771]  driver_register+0x68/0x100
[   31.384109]  __platform_driver_register+0x20/0x2c
[   31.384524]  init_module+0x84/0xfa0 [tyr]
[   31.384885]  do_one_initcall+0x104/0x35c
[   31.385235]  do_init_module+0x58/0x228
[   31.385569]  load_module+0x16f4/0x18f8
[   31.385901]  __arm64_sys_finit_module+0x24c/0x368
[   31.386317]  invoke_syscall+0x40/0xcc
[   31.386646]  el0_svc_common+0x80/0xd8
[   31.386652]  do_el0_svc+0x1c/0x28
[  OK  ] Reached target sockets.target -[   31.387270]  el0_svc+0x54/0x1d4
 Socket Units.
[   31.388011]  el0t_64_sync_handler+0x84/0x12c
[   31.388509]  el0t_64_sync+0x198/0x19c
[   31.388838] Code: 54000682 f9400689 aa0803e0 2a1503e1 (b943f122)
[   31.389372] ---[ end trace 0000000000000000 ]---
[   31.396698] Unable to handle kernel paging request at virtual address 0030303030306266

I just fixed it by changing the layout size from Self -> Device<T, Uninit>

After this fix, it's working great to load the firmware with the
Unregistered Device.

thanks,
Deborah

[PATCH v5 1/4] rust/drm: Introduce DeviceContext

[Lyude Paul]

One of the tricky things about DRM bindings in Rust is the fact that
initialization of a DRM device is a multi-step process. It's quite normal
for a device driver to start making use of its DRM device for tasks like
creating GEM objects before userspace registration happens. This is an
issue in rust though, since prior to userspace registration the device is
only partly initialized. This means there's a plethora of DRM device
operations we can't yet expose without opening up the door to UB if the DRM
device in question isn't yet registered.

Additionally, this isn't something we can reliably check at runtime. And
even if we could, performing an operation which requires the device be
registered when the device isn't actually registered is a programmer bug,
meaning there's no real way to gracefully handle such a mistake at runtime.
And even if that wasn't the case, it would be horrendously annoying and
noisy to have to check if a device is registered constantly throughout a
driver.

In order to solve this, we first take inspiration from
`kernel::device::DeviceContext` and introduce `kernel::drm::DeviceContext`.
This provides us with a ZST type that we can generalize over to represent
contexts where a device is known to have been registered with userspace at
some point in time (`Registered`), along with contexts where we can't make
such a guarantee (`Uninit`).

It's important to note we intentionally do not provide a `DeviceContext`
which represents an unregistered device. This is because there's no
reasonable way to guarantee that a device with long-living references to
itself will not be registered eventually with userspace. Instead, we
provide a new-type for this: `UnregisteredDevice` which can
provide a guarantee that the `Device` has never been registered with
userspace. To ensure this, we modify `Registration` so that creating a new
`Registration` requires passing ownership of an `UnregisteredDevice`.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

---
V2:
* Make sure that `UnregisteredDevice` is not thread-safe (since DRM device
  initialization is also not thread-safe)
* Rename from AnyCtx to Uninit, I think this name actually makes a bit more
  sense.
* Change assume_registered() to assume_ctx()
  Since it looks like in some situations, we'll want to update the
  DeviceContext of a object to the latest DeviceContext we know the Device
  to be in.
* Rename Init to Uninit
  When we eventually add KMS support, we're going to have 3 different
  DeviceContexts - Uninit, Init, Registered. Additionally, aside from not
  being registered there are a number of portions of the rest of the Device
  which also aren't usable before at least the Init context - so the naming
  of Uninit makes this a little clearer.
* s/DeviceContext/DeviceContext/
  For consistency with the rest of the kernel
* Drop as_ref::<Device<T, Uninit>>() for now since I don't actually think
  we need this quite yet
V3:
* Get rid of drm_dev_ctx!, as we don't actually need to implement Send or
  Sync ourselves
* Remove mention of C function in drm::device::Registration rustdoc
* Add more documentation to the DeviceContext trait, go into detail about
  the various setup phases and such.
* Add missing period to comment in `UnregisteredDevice::new()`.
V4:
* Address some comments from Danilo I missed last round:
  * Remove leftover rebase detritus from new_foreign_owned()
    (the seemingly useless cast)
  * Remove no-op mention in Registered device context
V5:
* Fix incorrect size on Kmalloc (Deborah)

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 drivers/gpu/drm/nova/driver.rs |   8 +-
 drivers/gpu/drm/tyr/driver.rs  |  10 +-
 rust/kernel/drm/device.rs      | 180 +++++++++++++++++++++++++++------
 rust/kernel/drm/driver.rs      |  35 +++++--
 rust/kernel/drm/mod.rs         |   4 +
 5 files changed, 191 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index b1af0a099551d..99d6841b69cbc 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -21,7 +21,7 @@ pub(crate) struct NovaDriver {
 }
 
 /// Convienence type alias for the DRM device type for this driver
-pub(crate) type NovaDevice = drm::Device<NovaDriver>;
+pub(crate) type NovaDevice<Ctx = drm::Registered> = drm::Device<NovaDriver, Ctx>;
 
 #[pin_data]
 pub(crate) struct NovaData {
@@ -56,10 +56,10 @@ impl auxiliary::Driver for NovaDriver {
     fn probe(adev: &auxiliary::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> {
         let data = try_pin_init!(NovaData { adev: adev.into() });
 
-        let drm = drm::Device::<Self>::new(adev.as_ref(), data)?;
-        drm::Registration::new_foreign_owned(&drm, adev.as_ref(), 0)?;
+        let drm = drm::UnregisteredDevice::<Self>::new(adev.as_ref(), data)?;
+        let drm = drm::Registration::new_foreign_owned(drm, adev.as_ref(), 0)?;
 
-        Ok(Self { drm })
+        Ok(Self { drm: drm.into() })
     }
 }
 
diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index 6114346415805..6238f6e2b3bd2 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -42,7 +42,7 @@
 pub(crate) struct TyrDrmDriver;
 
 /// Convenience type alias for the DRM device type for this driver.
-pub(crate) type TyrDrmDevice = drm::Device<TyrDrmDriver>;
+pub(crate) type TyrDrmDevice<Ctx = drm::Registered> = drm::Device<TyrDrmDriver, Ctx>;
 
 #[pin_data(PinnedDrop)]
 pub(crate) struct TyrPlatformDriverData {
@@ -145,10 +145,12 @@ fn probe(
                 gpu_info,
         });
 
-        let ddev: ARef<TyrDrmDevice> = drm::Device::new(pdev.as_ref(), data)?;
-        drm::driver::Registration::new_foreign_owned(&ddev, pdev.as_ref(), 0)?;
+        let tdev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev.as_ref(), data)?;
+        let tdev = drm::driver::Registration::new_foreign_owned(tdev, pdev.as_ref(), 0)?;
 
-        let driver = TyrPlatformDriverData { _device: ddev };
+        let driver = TyrPlatformDriverData {
+            _device: tdev.into(),
+        };
 
         // We need this to be dev_info!() because dev_dbg!() does not work at
         // all in Rust for now, and we need to see whether probe succeeded.
diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
index 629ef0bd1188e..b47dd9c29d578 100644
--- a/rust/kernel/drm/device.rs
+++ b/rust/kernel/drm/device.rs
@@ -10,7 +10,8 @@
     device,
     drm::{
         self,
-        driver::AllocImpl, //
+        driver::AllocImpl,
+        private::Sealed, //
     },
     error::from_err_ptr,
     prelude::*,
@@ -18,10 +19,14 @@
         ARef,
         AlwaysRefCounted, //
     },
-    types::Opaque, //
+    types::{
+        NotThreadSafe,
+        Opaque, //
+    }, //
 };
 use core::{
     alloc::Layout,
+    marker::PhantomData,
     mem,
     ops::Deref,
     ptr::{
@@ -61,26 +66,98 @@ macro_rules! drm_legacy_fields {
     }
 }
 
-/// A typed DRM device with a specific `drm::Driver` implementation.
+/// A trait implemented by all possible contexts a [`Device`] can be used in.
 ///
-/// The device is always reference-counted.
+/// Setting up a new [`Device`] is a multi-stage process. Each step of the process that a user
+/// interacts with in Rust has a respective [`DeviceContext`] typestate. For example,
+/// `Device<T, Registered>` would be a [`Device`] that reached the [`Registered`] [`DeviceContext`].
+///
+/// Each stage of this process is described below:
+///
+/// ```text
+///        1                     2                         3
+/// +--------------+   +------------------+   +-----------------------+
+/// |Device created| → |Device initialized| → |Registered w/ userspace|
+/// +--------------+   +------------------+   +-----------------------+
+///    (Uninit)                                      (Registered)
+/// ```
+///
+/// 1. The [`Device`] is in the [`Uninit`] context and is not guaranteed to be initialized or
+///    registered with userspace. Only a limited subset of DRM core functionality is available.
+/// 2. The [`Device`] is guaranteed to be fully initialized, but is not guaranteed to be registered
+///    with userspace. All DRM core functionality which doesn't interact with userspace is
+///    available. We currently don't have a context for representing this.
+/// 3. The [`Device`] is guaranteed to be fully initialized, and is guaranteed to have been
+///    registered with userspace at some point - thus putting it in the [`Registered`] context.
+///
+/// An important caveat of [`DeviceContext`] which must be kept in mind: when used as a typestate
+/// for a reference type, it can only guarantee that a [`Device`] reached a particular stage in the
+/// initialization process _at the time the reference was taken_. No guarantee is made in regards to
+/// what stage of the process the [`Device`] is currently in. This means for instance that a
+/// `&Device<T, Uninit>` may actually be registered with userspace, it just wasn't known to be
+/// registered at the time the reference was taken.
+pub trait DeviceContext: Sealed + Send + Sync {}
+
+/// The [`DeviceContext`] of a [`Device`] that was registered with userspace at some point.
+///
+/// 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.
 ///
 /// # Invariants
 ///
-/// `self.dev` is a valid instance of a `struct device`.
-#[repr(C)]
-pub struct Device<T: drm::Driver> {
-    dev: Opaque<bindings::drm_device>,
-    data: T::Data,
+/// A [`Device`] in this [`DeviceContext`] is guaranteed to have been registered with userspace
+/// at some point in time.
+pub struct Registered;
+
+impl Sealed for Registered {}
+impl DeviceContext for Registered {}
+
+/// The [`DeviceContext`] of a [`Device`] that may be unregistered and partly uninitialized.
+///
+/// A [`Device`] in this context is only guaranteed to be partly initialized, and may or may not
+/// be registered with userspace. Thus operations which depend on the [`Device`] being fully
+/// initialized, or which depend on the [`Device`] being registered with userspace are not
+/// available through this [`DeviceContext`].
+///
+/// A [`Device`] in this context can be used to create a
+/// [`Registration`](drm::driver::Registration).
+pub struct Uninit;
+
+impl Sealed for Uninit {}
+impl DeviceContext for Uninit {}
+
+/// 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,
+/// and can be used to create a [`Registration`](drm::driver::Registration) once the driver is ready
+/// to register the device with userspace.
+///
+/// Since DRM device initialization must be single-threaded, this object is not thread-safe.
+///
+/// # Invariants
+///
+/// The device in `self.0` is guaranteed to be a newly created [`Device`] that has not yet been
+/// registered with userspace until this type is dropped.
+pub struct UnregisteredDevice<T: drm::Driver>(ARef<Device<T, Uninit>>, NotThreadSafe);
+
+impl<T: drm::Driver> Deref for UnregisteredDevice<T> {
+    type Target = Device<T, Uninit>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
 }
 
-impl<T: drm::Driver> Device<T> {
+impl<T: drm::Driver> UnregisteredDevice<T> {
     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),
         unload: None,
-        release: Some(Self::release),
+        release: Some(Device::<T>::release),
         master_set: None,
         master_drop: None,
         debugfs_init: None,
@@ -108,21 +185,23 @@ impl<T: drm::Driver> Device<T> {
 
     const GEM_FOPS: bindings::file_operations = drm::gem::create_fops();
 
-    /// Create a new `drm::Device` for a `drm::Driver`.
-    pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<ARef<Self>> {
+    /// 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> {
         // `__drm_dev_alloc` uses `kmalloc()` to allocate memory, hence ensure a `kmalloc()`
         // compatible `Layout`.
-        let layout = Kmalloc::aligned_layout(Layout::new::<Self>());
+        let layout = Kmalloc::aligned_layout(Layout::new::<Device<T, Uninit>>());
 
         // SAFETY:
         // - `VTABLE`, as a `const` is pinned to the read-only section of the compilation,
         // - `dev` is valid by its type invarants,
-        let raw_drm: *mut Self = unsafe {
+        let raw_drm: *mut Device<T, Uninit> = unsafe {
             bindings::__drm_dev_alloc(
                 dev.as_raw(),
                 &Self::VTABLE,
                 layout.size(),
-                mem::offset_of!(Self, dev),
+                mem::offset_of!(Device<T, Uninit>, dev),
             )
         }
         .cast();
@@ -137,7 +216,7 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<A
         unsafe { data.__pinned_init(raw_data) }.inspect_err(|_| {
             // SAFETY: `raw_drm` is a valid pointer to `Self`, given that `__drm_dev_alloc` was
             // successful.
-            let drm_dev = unsafe { Self::into_drm_device(raw_drm) };
+            let drm_dev = unsafe { Device::into_drm_device(raw_drm) };
 
             // SAFETY: `__drm_dev_alloc()` was successful, hence `drm_dev` must be valid and the
             // refcount must be non-zero.
@@ -146,9 +225,39 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<A
 
         // SAFETY: The reference count is one, and now we take ownership of that reference as a
         // `drm::Device`.
-        Ok(unsafe { ARef::from_raw(raw_drm) })
+        // INVARIANT: We just created the device above, but have yet to call `drm_dev_register`.
+        // `Self` cannot be copied or sent to another thread - ensuring that `drm_dev_register`
+        // won't be called during its lifetime and that the device is unregistered.
+        Ok(Self(unsafe { ARef::from_raw(raw_drm) }, NotThreadSafe))
     }
+}
 
+/// A typed DRM device with a specific [`drm::Driver`] implementation and [`DeviceContext`].
+///
+/// Since DRM devices can be used before being fully initialized and registered with userspace, `C`
+/// represents the furthest [`DeviceContext`] we can guarantee that this [`Device`] has reached.
+///
+/// Keep in mind: this means that an unregistered device can still have the registration state
+/// [`Registered`] as long as it was registered with userspace once in the past, and that the
+/// behavior of such a device is still well-defined. Additionally, a device with the registration
+/// state [`Uninit`] simply does not have a guaranteed registration state at compile time, and could
+/// be either registered or unregistered. Since there is no way to guarantee a long-lived reference
+/// to an unregistered device would remain unregistered, we do not provide a [`DeviceContext`] for
+/// this.
+///
+/// # Invariants
+///
+/// * `self.dev` is a valid instance of a `struct device`.
+/// * The data layout of `Self` remains the same across all implementations of `C`.
+/// * Any invariants for `C` also apply.
+#[repr(C)]
+pub struct Device<T: drm::Driver, C: DeviceContext = Registered> {
+    dev: Opaque<bindings::drm_device>,
+    data: T::Data,
+    _ctx: PhantomData<C>,
+}
+
+impl<T: drm::Driver, C: DeviceContext> Device<T, C> {
     pub(crate) fn as_raw(&self) -> *mut bindings::drm_device {
         self.dev.get()
     }
@@ -174,13 +283,13 @@ unsafe fn into_drm_device(ptr: NonNull<Self>) -> *mut bindings::drm_device {
     ///
     /// # Safety
     ///
-    /// Callers must ensure that `ptr` is valid, non-null, and has a non-zero reference count,
-    /// i.e. it must be ensured that the reference count of the C `struct drm_device` `ptr` points
-    /// to can't drop to zero, for the duration of this function call and the entire duration when
-    /// the returned reference exists.
-    ///
-    /// Additionally, callers must ensure that the `struct device`, `ptr` is pointing to, is
-    /// embedded in `Self`.
+    /// * Callers must ensure that `ptr` is valid, non-null, and has a non-zero reference count,
+    ///   i.e. it must be ensured that the reference count of the C `struct drm_device` `ptr` points
+    ///   to can't drop to zero, for the duration of this function call and the entire duration when
+    ///   the returned reference exists.
+    /// * Additionally, callers must ensure that the `struct device`, `ptr` is pointing to, is
+    ///   embedded in `Self`.
+    /// * Callers promise that any type invariants of `C` will be upheld.
     #[doc(hidden)]
     pub unsafe fn from_raw<'a>(ptr: *const bindings::drm_device) -> &'a Self {
         // SAFETY: By the safety requirements of this function `ptr` is a valid pointer to a
@@ -200,9 +309,20 @@ extern "C" fn release(ptr: *mut bindings::drm_device) {
         // - `this` is valid for dropping.
         unsafe { core::ptr::drop_in_place(this) };
     }
+
+    /// Change the [`DeviceContext`] for a [`Device`].
+    ///
+    /// # Safety
+    ///
+    /// The caller promises that `self` fulfills all of the guarantees provided by the given
+    /// [`DeviceContext`].
+    pub(crate) unsafe fn assume_ctx<NewCtx: DeviceContext>(&self) -> &Device<T, NewCtx> {
+        // SAFETY: The data layout is identical via our type invariants.
+        unsafe { mem::transmute(self) }
+    }
 }
 
-impl<T: drm::Driver> Deref for Device<T> {
+impl<T: drm::Driver, C: DeviceContext> Deref for Device<T, C> {
     type Target = T::Data;
 
     fn deref(&self) -> &Self::Target {
@@ -212,7 +332,7 @@ fn deref(&self) -> &Self::Target {
 
 // SAFETY: DRM device objects are always reference counted and the get/put functions
 // satisfy the requirements.
-unsafe impl<T: drm::Driver> AlwaysRefCounted for Device<T> {
+unsafe impl<T: drm::Driver, C: DeviceContext> AlwaysRefCounted for Device<T, C> {
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
         unsafe { bindings::drm_dev_get(self.as_raw()) };
@@ -227,7 +347,7 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
     }
 }
 
-impl<T: drm::Driver> AsRef<device::Device> for Device<T> {
+impl<T: drm::Driver, C: DeviceContext> AsRef<device::Device> for Device<T, C> {
     fn as_ref(&self) -> &device::Device {
         // SAFETY: `bindings::drm_device::dev` is valid as long as the DRM device itself is valid,
         // which is guaranteed by the type invariant.
@@ -236,8 +356,8 @@ fn as_ref(&self) -> &device::Device {
 }
 
 // SAFETY: A `drm::Device` can be released from any thread.
-unsafe impl<T: drm::Driver> Send for Device<T> {}
+unsafe impl<T: drm::Driver, C: DeviceContext> Send for Device<T, C> {}
 
 // SAFETY: A `drm::Device` can be shared among threads because all immutable methods are protected
 // by the synchronization in `struct drm_device`.
-unsafe impl<T: drm::Driver> Sync for Device<T> {}
+unsafe impl<T: drm::Driver, C: DeviceContext> Sync for Device<T, C> {}
diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
index 5233bdebc9fcd..55b01ee088548 100644
--- a/rust/kernel/drm/driver.rs
+++ b/rust/kernel/drm/driver.rs
@@ -13,6 +13,10 @@
     prelude::*,
     sync::aref::ARef, //
 };
+use core::{
+    mem,
+    ptr::NonNull, //
+};
 
 /// Driver use the GEM memory manager. This should be set for all modern drivers.
 pub(crate) const FEAT_GEM: u32 = bindings::drm_driver_feature_DRIVER_GEM;
@@ -123,21 +127,31 @@ pub trait Driver {
 pub struct Registration<T: Driver>(ARef<drm::Device<T>>);
 
 impl<T: Driver> Registration<T> {
-    fn new(drm: &drm::Device<T>, flags: usize) -> Result<Self> {
+    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) })?;
 
-        Ok(Self(drm.into()))
+        // SAFETY: We just called `drm_dev_register` above
+        let new = NonNull::from(unsafe { drm.assume_ctx() });
+
+        // Leak the ARef from UnregisteredDevice in preparation for transferring its ownership.
+        mem::forget(drm);
+
+        // SAFETY: `drm`'s `Drop` constructor was never called, ensuring that there remains at least
+        // one reference to the device - which we take ownership over here.
+        let new = unsafe { ARef::from_raw(new) };
+
+        Ok(Self(new))
     }
 
-    /// Registers a new [`Device`](drm::Device) with userspace.
+    /// Registers a new [`UnregisteredDevice`](drm::UnregisteredDevice) with userspace.
     ///
     /// Ownership of the [`Registration`] object is passed to [`devres::register`].
-    pub fn new_foreign_owned(
-        drm: &drm::Device<T>,
-        dev: &device::Device<device::Bound>,
+    pub fn new_foreign_owned<'a>(
+        drm: drm::UnregisteredDevice<T>,
+        dev: &'a device::Device<device::Bound>,
         flags: usize,
-    ) -> Result
+    ) -> Result<&'a drm::Device<T>>
     where
         T: 'static,
     {
@@ -146,8 +160,13 @@ pub fn new_foreign_owned(
         }
 
         let reg = Registration::<T>::new(drm, flags)?;
+        let drm = NonNull::from(reg.device());
+
+        devres::register(dev, reg, GFP_KERNEL)?;
 
-        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() })
     }
 
     /// Returns a reference to the `Device` instance for this registration.
diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
index 1b82b6945edf2..64a43cb0fe57c 100644
--- a/rust/kernel/drm/mod.rs
+++ b/rust/kernel/drm/mod.rs
@@ -9,6 +9,10 @@
 pub mod ioctl;
 
 pub use self::device::Device;
+pub use self::device::DeviceContext;
+pub use self::device::Registered;
+pub use self::device::Uninit;
+pub use self::device::UnregisteredDevice;
 pub use self::driver::Driver;
 pub use self::driver::DriverInfo;
 pub use self::driver::Registration;
-- 
2.53.0