rust: drm: Higher-Ranked Lifetime private data

index cf328101dde4..ccf4150d83b6 100644
@@ -135,6 +135,12 @@ macro_rules! declare_drm_ioctls {
                             // dev/file match the current driver these ioctls are being declared
                             // for, and it's not clear how to enforce this within the type system.
                             let dev = $crate::drm::device::Device::from_raw(raw_dev);
+
+                            // Enforce that the handler accepts higher-ranked
+                            // lifetimes, preventing it from requiring 'static
+                            // references that could escape this scope.
+                            let _: for<'a> fn(&'a _, &'a mut _, &'a _) -> _ = $func;
+
                             // SAFETY: The ioctl argument has size `_IOC_SIZE(cmd)`, which we
                             // asserted above matches the size of this type, and all bit patterns of
                             // UAPI structs must be valid.

index 477cf771fb10..5e91474e6dbb 100644
@@ -74,36 +74,22 @@ macro_rules! drm_legacy_fields {
 
 /// A trait implemented by all possible contexts a [`Device`] can be used in.
 ///
-/// 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`].
+/// A [`Device`] can be in one of two contexts:
 ///
-/// 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.
+/// - [`Normal`]: The general-purpose, reference-counted context. A [`Device`] in this context may
+///   or may not be registered with userspace.
+/// - [`Registered`]: The device has been registered with userspace at some point.
 pub trait DeviceContext: Sealed + Send + Sync {}
 
+/// The general-purpose, reference-counted [`DeviceContext`].
+///
+/// A [`Device`] in this context may or may not be registered with userspace. This context is used
+/// for reference-counted device handles and during device setup via [`UnregisteredDevice`].
+pub struct Normal;
+
+impl Sealed for Normal {}
+impl DeviceContext for Normal {}
+
 /// 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

@@ -121,20 +107,6 @@ pub trait DeviceContext: Sealed + Send + Sync {}
 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,

@@ -147,10 +119,10 @@ 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>(ARef<Device<T, Normal>>, NotThreadSafe);
 
 impl<T: drm::Driver> Deref for UnregisteredDevice<T> {
-    type Target = Device<T, Uninit>;
+    type Target = Device<T, Normal>;
 
     fn deref(&self) -> &Self::Target {
         &self.0

@@ -178,15 +150,13 @@ const fn compute_features() -> u32 {
         master_drop: None,
         debugfs_init: None,
 
-        // 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,
+        gem_create_object: T::Object::<Normal>::ALLOC_OPS.gem_create_object,
+        prime_handle_to_fd: T::Object::<Normal>::ALLOC_OPS.prime_handle_to_fd,
+        prime_fd_to_handle: T::Object::<Normal>::ALLOC_OPS.prime_fd_to_handle,
+        gem_prime_import: T::Object::<Normal>::ALLOC_OPS.gem_prime_import,
+        gem_prime_import_sg_table: T::Object::<Normal>::ALLOC_OPS.gem_prime_import_sg_table,
+        dumb_create: T::Object::<Normal>::ALLOC_OPS.dumb_create,
+        dumb_map_offset: T::Object::<Normal>::ALLOC_OPS.dumb_map_offset,
 
         show_fdinfo: None,
         fbdev_probe: None,

@@ -211,7 +181,7 @@ const fn compute_features() -> u32 {
     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::<Device<T, Uninit>>());
+        let layout = Kmalloc::aligned_layout(Layout::new::<Device<T, Normal>>());
 
         // Use a temporary vtable without a `release` callback until `data` is initialized, so
         // init failure can release the DRM device without dropping uninitialized fields.

@@ -223,12 +193,12 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<S
         // SAFETY:
         // - `alloc_vtable` reference remains valid until no longer used,
         // - `dev` is valid by its type invarants,
-        let raw_drm: *mut Device<T, Uninit> = unsafe {
+        let raw_drm: *mut Device<T, Normal> = unsafe {
             bindings::__drm_dev_alloc(
                 dev.as_raw(),
                 &alloc_vtable,
                 layout.size(),
-                mem::offset_of!(Device<T, Uninit>, dev),
+                mem::offset_of!(Device<T, Normal>, dev),
             )
         }
         .cast();

@@ -264,16 +234,8 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<S
 
 /// 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.
+/// A device in the [`Registered`] context is guaranteed to have been registered with userspace
+/// at some point. The [`Normal`] context is the general-purpose, reference-counted context.
 ///
 /// # Invariants
 ///

index a66e7166f66b..e5bfaf130342 100644
@@ -11,8 +11,8 @@
 
 pub use self::device::Device;
 pub use self::device::DeviceContext;
+pub use self::device::Normal;
 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;

index 3ee19ef6264e..52de59b14dad 100644
@@ -692,10 +692,12 @@ impl drm::Driver for KunitDriver {
     fn create_drm_dev() -> Result<(faux::Registration, UnregisteredDevice<KunitDriver>)> {
         // Create a faux DRM device so we can test gem object creation.
         let data = try_pin_init!(KunitData {});
-        let dev = faux::Registration::new(c"Kunit", None)?;
-        let drm = UnregisteredDevice::new(dev.as_ref(), data)?;
+        let reg = faux::Registration::new(c"Kunit", None)?;
+        let fdev = reg.as_ref();
+        let dev = fdev.as_ref();
+        let drm = UnregisteredDevice::new(dev, data)?;
 
-        Ok((dev, drm))
+        Ok((reg, drm))
     }
 
     #[test]

@@ -755,7 +757,8 @@ fn vmap_io() -> Result {
     #[test]
     fn fail_sg_table_on_wrong_dev() -> Result {
         let (_dev, drm) = create_drm_dev()?;
-        let wrong_dev = faux::Registration::new(c"EvilKunit", None)?;
+        let reg = faux::Registration::new(c"EvilKunit", None)?;
+        let wrong_dev = reg.as_ref();
 
         let obj = Object::<KunitObject, _>::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?;
 

91 lines not on this page · raw message

index 99876c8e3743..27b6d3e2bb44 100644
@@ -25,8 +25,9 @@ fn init(_module: &'static ThisModule) -> Result<Self> {
         pr_info!("Initialising Rust Faux Device Sample\n");
 
         let reg = faux::Registration::new(c"rust-faux-sample-device", None)?;
+        let fdev = reg.as_ref();
 
-        dev_info!(reg, "Hello from faux device!\n");
+        dev_info!(fdev, "Hello from faux device!\n");
 
         Ok(Self { _reg: reg })
     }

index 48933d86ddda..c5b0313006bd 100644
@@ -2,7 +2,10 @@
 
 use kernel::{
     auxiliary,
-    device::Core,
+    device::{
+        Core,
+        DeviceContext, //
+    },
     drm::{
         self,
         gem,

@@ -62,7 +65,7 @@ fn probe<'bound>(
     ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
         let data = try_pin_init!(NovaData { adev: adev.into() });
 
-        let drm = drm::UnregisteredDevice::<Self>::new(adev.as_ref(), data)?;
+        let drm = drm::UnregisteredDevice::<Self>::new(adev, data)?;
         let drm = drm::Registration::new_foreign_owned(drm, adev.as_ref(), 0)?;
 
         Ok(Nova { drm: drm.into() })

@@ -74,6 +77,7 @@ impl drm::Driver for NovaDriver {
     type Data = NovaData;
     type File = File;
     type Object<Ctx: drm::DeviceContext> = gem::Object<NovaObject, Ctx>;
+    type ParentDevice<Ctx: DeviceContext> = auxiliary::Device<Ctx>;
 
     const INFO: drm::DriverInfo = INFO;
 

index d063bc664cc1..338c25ccc151 100644
@@ -7,7 +7,8 @@
     },
     device::{
         Core,
-        Device, //
+        Device,
+        DeviceContext, //
     },
     dma::{
         Device as DmaDevice,

@@ -148,7 +149,7 @@ fn probe<'bound>(
                 gpu_info,
         });
 
-        let tdev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev.as_ref(), data)?;
+        let tdev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev, data)?;
         let tdev = drm::driver::Registration::new_foreign_owned(tdev, pdev.as_ref(), 0)?;
 
         let driver = TyrPlatformDriverData {

@@ -182,6 +183,7 @@ impl drm::Driver for TyrDrmDriver {
     type Data = TyrDrmDeviceData;
     type File = TyrDrmFileData;
     type Object<R: drm::DeviceContext> = drm::gem::shmem::Object<BoData, R>;
+    type ParentDevice<Ctx: DeviceContext> = platform::Device<Ctx>;
 
     const INFO: drm::DriverInfo = INFO;
     const FEAT_RENDER: bool = true;

index 5e91474e6dbb..23eb4c0a65ef 100644
@@ -178,7 +178,10 @@ const fn compute_features() -> u32 {
     /// 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: &T::ParentDevice<device::Bound>,
+        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::<Device<T, Normal>>());

@@ -195,7 +198,7 @@ pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<S
         // - `dev` is valid by its type invarants,
         let raw_drm: *mut Device<T, Normal> = unsafe {
             bindings::__drm_dev_alloc(
-                dev.as_raw(),
+                dev.as_ref().as_raw(),
                 &alloc_vtable,
                 layout.size(),
                 mem::offset_of!(Device<T, Normal>, dev),

index 25f7e233884d..802e7fc13e30 100644
@@ -116,6 +116,9 @@ pub trait Driver {
     /// The type used to represent a DRM File (client)
     type File: drm::file::DriverFile;
 
+    /// The bus device type of the parent device that the DRM device is associated with.
+    type ParentDevice<Ctx: device::DeviceContext>: device::AsBusDevice<Ctx>;
+
     /// Driver metadata
     const INFO: DriverInfo;
 

index 52de59b14dad..cbcfc7e4edb6 100644
@@ -684,6 +684,7 @@ impl drm::Driver for KunitDriver {
         type Data = KunitData;
         type File = KunitFile;
         type Object<Ctx: DeviceContext> = Object<KunitObject, Ctx>;
+        type ParentDevice<Ctx: device::DeviceContext> = faux::Device<Ctx>;
 
         const INFO: drm::DriverInfo = INFO;
         const IOCTLS: &'static [drm::ioctl::DrmIoctlDescriptor] = &[];

@@ -694,8 +695,7 @@ fn create_drm_dev() -> Result<(faux::Registration, UnregisteredDevice<KunitDrive
         let data = try_pin_init!(KunitData {});
         let reg = faux::Registration::new(c"Kunit", None)?;
         let fdev = reg.as_ref();
-        let dev = fdev.as_ref();
-        let drm = UnregisteredDevice::new(dev, data)?;
+        let drm = UnregisteredDevice::new(fdev, data)?;
 
         Ok((reg, drm))
     }

index c5b0313006bd..8ddb81fd0c87 100644
@@ -26,7 +26,7 @@ pub(crate) struct Nova {
 }
 
 /// Convienence type alias for the DRM device type for this driver
-pub(crate) type NovaDevice<Ctx = drm::Registered> = drm::Device<NovaDriver, Ctx>;
+pub(crate) type NovaDevice<Ctx = drm::Normal> = drm::Device<NovaDriver, Ctx>;
 
 #[pin_data]
 pub(crate) struct NovaData {

index 338c25ccc151..180631daff02 100644
@@ -47,7 +47,7 @@
 pub(crate) struct TyrDrmDriver;
 
 /// Convenience type alias for the DRM device type for this driver.
-pub(crate) type TyrDrmDevice<Ctx = drm::Registered> = drm::Device<TyrDrmDriver, Ctx>;
+pub(crate) type TyrDrmDevice<Ctx = drm::Normal> = drm::Device<TyrDrmDriver, Ctx>;
 
 pub(crate) struct TyrPlatformDriver;
 

index 23eb4c0a65ef..d712387707d2 100644
@@ -246,7 +246,7 @@ pub fn new(
 /// * 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> {
+pub struct Device<T: drm::Driver, C: DeviceContext = Normal> {
     dev: Opaque<bindings::drm_device>,
     data: T::Data,
     _ctx: PhantomData<C>,

index c8b66d816871..1023ddccd785 100644
@@ -10,6 +10,7 @@
         self,
         device::{
             DeviceContext,
+            Normal,
             Registered, //
         },
         driver::{

@@ -183,7 +184,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<Registered> = Self, File = F>,
+        D: drm::Driver<Object<Normal> = Self, File = F>,
         F: drm::file::DriverFile<Driver = D>,
     {
         let mut handle: u32 = 0;

@@ -198,7 +199,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<Registered> = Self, File = F>,
+        D: drm::Driver<Object<Normal> = Self, File = F>,
         F: drm::file::DriverFile<Driver = D>,
     {
         // SAFETY: The arguments are all valid per the type invariants.

@@ -254,7 +255,7 @@ impl<T: IntoGEMObject> BaseObjectPrivate for T {}
 /// * 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, Ctx: DeviceContext = Registered> {
+pub struct Object<T: DriverObject + Send + Sync, Ctx: DeviceContext = Normal> {
     obj: Opaque<bindings::drm_gem_object>,
     #[pin]
     data: T,

index cbcfc7e4edb6..5ffa1355ecf2 100644
@@ -22,7 +22,7 @@
         private::Sealed,
         Device,
         DeviceContext,
-        Registered, //
+        Normal, //
     },
     error::{
         from_err_ptr,

@@ -73,7 +73,7 @@
 ///
 /// This is used with [`Object::new()`] to control various properties that can only be set when
 /// initially creating a shmem-backed GEM object.
-pub struct ObjectConfig<'a, T: DriverObject, C: DeviceContext = Registered> {
+pub struct ObjectConfig<'a, T: DriverObject, C: DeviceContext = Normal> {
     /// Whether to set the write-combine map flag.
     pub map_wc: bool,
 

@@ -102,7 +102,7 @@ fn default() -> Self {
 /// - Any type invariants of `C` apply to the parent DRM device for this GEM object.
 #[repr(C)]
 #[pin_data]
-pub struct Object<T: DriverObject, C: DeviceContext = Registered> {
+pub struct Object<T: DriverObject, C: DeviceContext = Normal> {
     #[pin]
     obj: Opaque<bindings::drm_gem_shmem_object>,
     /// Parent object that owns this object's DMA reservation object.

@@ -409,7 +409,7 @@ impl<T: DriverObject, C: DeviceContext> driver::AllocImpl for Object<T, C> {
 /// When this is dropped, the `dma_resv` lock is dropped as well.
 ///
 // TODO: This should be replace with a WwMutex equivalent once we have such bindings in the kernel.
-struct DmaResvGuard<'a, T: DriverObject, C: DeviceContext = Registered>(
+struct DmaResvGuard<'a, T: DriverObject, C: DeviceContext = Normal>(
     &'a Object<T, C>,
     NotThreadSafe,
 );

@@ -438,7 +438,7 @@ fn drop(&mut self) {
 ///
 /// - The size of `owner` is >= SIZE.
 /// - The memory pointed to by `addr` remains valid at least until this object is dropped.
-pub struct VMap<D, R, C = Registered, const SIZE: usize = 0>
+pub struct VMap<D, R, C = Normal, const SIZE: usize = 0>
 where
     D: DriverObject,
     C: DeviceContext,

index d712387707d2..9825d52832af 100644
@@ -85,6 +85,9 @@ pub trait DeviceContext: Sealed + Send + Sync {}
 ///
 /// A [`Device`] in this context may or may not be registered with userspace. This context is used
 /// for reference-counted device handles and during device setup via [`UnregisteredDevice`].
+///
+/// [`AlwaysRefCounted`] is only implemented for `Device<T, Normal>`, making this the required
+/// context for [`ARef`]-based device handles.
 pub struct Normal;
 
 impl Sealed for Normal {}

@@ -327,7 +330,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, C: DeviceContext> AlwaysRefCounted for Device<T, C> {
+unsafe impl<T: drm::Driver> AlwaysRefCounted for Device<T> {
     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()) };

@@ -357,12 +360,10 @@ unsafe impl<T: drm::Driver, C: DeviceContext> Send for Device<T, C> {}
 // by the synchronization in `struct drm_device`.
 unsafe impl<T: drm::Driver, C: DeviceContext> Sync for Device<T, C> {}
 
-impl<T, C, const ID: u64> WorkItem<ID> for Device<T, C>
+impl<T: drm::Driver, const ID: u64> WorkItem<ID> for Device<T>
 where
-    T: drm::Driver,
     T::Data: WorkItem<ID, Pointer = ARef<Self>>,
     T::Data: HasWork<Self, ID>,
-    C: DeviceContext,
 {
     type Pointer = ARef<Self>;
 

index 8ddb81fd0c87..e3c54303d70e 100644
@@ -76,7 +76,7 @@ fn probe<'bound>(
 impl drm::Driver for NovaDriver {
     type Data = NovaData;
     type File = File;
-    type Object<Ctx: drm::DeviceContext> = gem::Object<NovaObject, Ctx>;
+    type Object = gem::Object<NovaObject>;
     type ParentDevice<Ctx: DeviceContext> = auxiliary::Device<Ctx>;
 
     const INFO: drm::DriverInfo = INFO;

index 9d8ff7de2c0f..2b6fe9dc0bfa 100644
@@ -2,7 +2,10 @@
 
 use kernel::{
     drm,
-    drm::{gem, gem::BaseObject, DeviceContext},
+    drm::{
+        gem,
+        gem::BaseObject, //
+    },
     page,
     prelude::*,
     sync::aref::ARef,

@@ -21,27 +24,20 @@ impl gem::DriverObject for NovaObject {
     type Driver = NovaDriver;
     type Args = ();
 
-    fn new<Ctx: DeviceContext>(
-        _dev: &NovaDevice<Ctx>,
-        _size: usize,
-        _args: Self::Args,
-    ) -> impl PinInit<Self, Error> {
+    fn new(_dev: &NovaDevice, _size: usize, _args: Self::Args) -> impl PinInit<Self, Error> {
         try_pin_init!(NovaObject {})
     }
 }
 
 impl NovaObject {
     /// Create a new DRM GEM object.
-    pub(crate) fn new<Ctx: DeviceContext>(
-        dev: &NovaDevice<Ctx>,
-        size: usize,
-    ) -> Result<ARef<gem::Object<Self, Ctx>>> {
+    pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result<ARef<gem::Object<Self>>> {
         if size == 0 {
             return Err(EINVAL);
         }
         let aligned_size = page::page_align(size).ok_or(EINVAL)?;
 
-        gem::Object::<Self, Ctx>::new(dev, aligned_size, ())
+        gem::Object::<Self>::new(dev, aligned_size, ())
     }
 
     /// Look up a GEM object handle for a `File` and return an `ObjectRef` for it.

index 180631daff02..7f082de6d6dc 100644
@@ -182,7 +182,7 @@ fn drop(self: Pin<&mut Self>) {}
 impl drm::Driver for TyrDrmDriver {
     type Data = TyrDrmDeviceData;
     type File = TyrDrmFileData;
-    type Object<R: drm::DeviceContext> = drm::gem::shmem::Object<BoData, R>;
+    type Object = drm::gem::shmem::Object<BoData>;
     type ParentDevice<Ctx: DeviceContext> = platform::Device<Ctx>;
 
     const INFO: drm::DriverInfo = INFO;

index c6d4d6f9bae3..1640a161754b 100644
@@ -5,10 +5,7 @@
 //! DRM's GEM subsystem with shmem backing.
 
 use kernel::{
-    drm::{
-        gem,
-        DeviceContext, //
-    },
+    drm::gem,
     prelude::*, //
 };
 

@@ -33,11 +30,7 @@ impl gem::DriverObject for BoData {
     type Driver = TyrDrmDriver;
     type Args = BoCreateArgs;
 
-    fn new<Ctx: DeviceContext>(
-        _dev: &TyrDrmDevice<Ctx>,
-        _size: usize,
-        args: BoCreateArgs,
-    ) -> impl PinInit<Self, Error> {
+    fn new(_dev: &TyrDrmDevice, _size: usize, args: BoCreateArgs) -> impl PinInit<Self, Error> {
         try_pin_init!(Self { flags: args.flags })
     }
 }

index 9825d52832af..6f3af46ff647 100644
@@ -153,13 +153,13 @@ const fn compute_features() -> u32 {
         master_drop: None,
         debugfs_init: None,
 
-        gem_create_object: T::Object::<Normal>::ALLOC_OPS.gem_create_object,
-        prime_handle_to_fd: T::Object::<Normal>::ALLOC_OPS.prime_handle_to_fd,
-        prime_fd_to_handle: T::Object::<Normal>::ALLOC_OPS.prime_fd_to_handle,
-        gem_prime_import: T::Object::<Normal>::ALLOC_OPS.gem_prime_import,
-        gem_prime_import_sg_table: T::Object::<Normal>::ALLOC_OPS.gem_prime_import_sg_table,
-        dumb_create: T::Object::<Normal>::ALLOC_OPS.dumb_create,
-        dumb_map_offset: T::Object::<Normal>::ALLOC_OPS.dumb_map_offset,
+        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,
 
         show_fdinfo: None,
         fbdev_probe: None,

index 802e7fc13e30..5152a18a8312 100644
@@ -111,7 +111,7 @@ pub trait Driver {
     type Data: Sync + Send;
 
     /// The type used to manage memory for this driver.
-    type Object<Ctx: drm::DeviceContext>: AllocImpl;
+    type Object: AllocImpl;
 
     /// The type used to represent a DRM File (client)
     type File: drm::file::DriverFile;

150 lines not on this page · raw message

186 lines not on this page · raw message

index cf8410e0f228..e0ef47352e88 100644
@@ -20,9 +20,7 @@
         driver,
         gem,
         private::Sealed,
-        Device,
-        DeviceContext,
-        Normal, //
+        Device, //
     },
     error::{
         from_err_ptr,

@@ -48,7 +46,6 @@
 };
 use core::{
     ffi::c_void,
-    marker::PhantomData,
     mem::{
         ManuallyDrop,
         MaybeUninit, //

@@ -99,22 +96,20 @@ fn default() -> Self {
 ///
 /// - `obj` contains a valid initialized `struct drm_gem_shmem_object` for the lifetime of this
 ///   object.
-/// - Any type invariants of `C` apply to the parent DRM device for this GEM object.
 #[repr(C)]
 #[pin_data]
-pub struct Object<T: DriverObject, C: DeviceContext = Normal> {
+pub struct Object<T: DriverObject> {
     #[pin]
     obj: Opaque<bindings::drm_gem_shmem_object>,
     /// Parent object that owns this object's DMA reservation object.
     parent_resv_obj: Option<ARef<Object<T>>>,
     /// Devres object for unmapping any SGTable on driver-unbind.
-    sgt_res: ManuallyDrop<SetOnce<Devres<SGTableMap<T, C>>>>,
+    sgt_res: ManuallyDrop<SetOnce<Devres<SGTableMap<T>>>>,
     #[pin]
     /// Lock for protecting initialization of `sgt_res`.
     sgt_lock: Mutex<()>,
     #[pin]
     inner: T,
-    _ctx: PhantomData<C>,
 }
 
 super::impl_aref_for_gem_obj! {

@@ -124,12 +119,12 @@ impl<T> for Object<T>
 }
 
 // SAFETY: All GEM objects are thread-safe.
-unsafe impl<T: DriverObject, C: DeviceContext> Send for Object<T, C> {}
+unsafe impl<T: DriverObject> Send for Object<T> {}
 
 // SAFETY: All GEM objects are thread-safe.
-unsafe impl<T: DriverObject, C: DeviceContext> Sync for Object<T, C> {}
+unsafe impl<T: DriverObject> Sync for Object<T> {}
 
-impl<T: DriverObject, C: DeviceContext> Object<T, C> {
+impl<T: DriverObject> Object<T> {
     /// `drm_gem_object_funcs` vtable suitable for GEM shmem objects.
     const VTABLE: bindings::drm_gem_object_funcs = bindings::drm_gem_object_funcs {
         free: Some(Self::free_callback),

@@ -157,7 +152,7 @@ fn as_raw_shmem(&self) -> *mut bindings::drm_gem_shmem_object {
     }
 
     /// Returns the `Device` that owns this GEM object.
-    pub fn dev(&self) -> &Device<T::Driver, C> {
+    pub fn dev(&self) -> &Device<T::Driver> {
         // SAFETY: `dev` will have been initialized in `Self::new()` by `drm_gem_shmem_init()`.
         unsafe { Device::from_raw((*self.as_raw()).dev) }
     }

@@ -171,8 +166,8 @@ extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) {
 
         // SAFETY:
         // - We verified above that `obj` is valid, which makes `this` valid
-        // - This function is set in AllocOps, so we know that `this` is contained within a
-        //   `Object<T, C>`
+        // - This function is set in AllocOps, so we know that `this` is contained within an
+        //   `Object<T>`
         let this = unsafe { container_of!(Opaque::cast_from(base), Self, obj) }.cast_mut();
 
         // We need to drop `sgt_res` first, since doing so requires that the GEM object is still

@@ -193,7 +188,7 @@ extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) {
     }
 
     /// Attempt to create a vmap from the gem object, and confirm the size of said vmap.
-    fn make_vmap<'a, R, const SIZE: usize>(&'a self) -> Result<VMap<T, R, C, SIZE>>
+    fn make_vmap<'a, R, const SIZE: usize>(&'a self) -> Result<VMap<T, R, SIZE>>
     where
         R: Deref<Target = Self> + From<&'a Self>,
     {

@@ -255,7 +250,7 @@ unsafe fn raw_vunmap(&self, mut map: bindings::iosys_map) {
 
     /// Creates and returns a virtual kernel memory mapping for this object.
     #[inline]
-    pub fn vmap<const SIZE: usize>(&self) -> Result<VMapRef<'_, T, C, SIZE>> {
+    pub fn vmap<const SIZE: usize>(&self) -> Result<VMapRef<'_, T, SIZE>> {
         self.make_vmap()
     }
 

@@ -298,9 +293,7 @@ pub fn sg_table<'a>(
 
         Ok(sgt_res.access(dev)?)
     }
-}
 
-impl<T: DriverObject> Object<T> {
     /// Create a new shmem-backed DRM object of the given size.
     ///
     /// Additional config options can be specified using `config`.

@@ -317,7 +310,6 @@ pub fn new(
                 sgt_res: ManuallyDrop::new(SetOnce::new()),
                 sgt_lock <- new_mutex!(()),
                 inner <- T::new(dev, size, args),
-                _ctx: PhantomData,
             }),
             GFP_KERNEL,
         )?;

@@ -351,12 +343,12 @@ pub fn new(
 
     /// Creates and returns an owned reference to a virtual kernel memory mapping for this object.
     #[inline]
-    pub fn owned_vmap<const SIZE: usize>(&self) -> Result<VMapOwned<T, Normal, SIZE>> {
+    pub fn owned_vmap<const SIZE: usize>(&self) -> Result<VMapOwned<T, SIZE>> {
         self.make_vmap()
     }
 }
 
-impl<T: DriverObject, C: DeviceContext> Deref for Object<T, C> {
+impl<T: DriverObject> Deref for Object<T> {
     type Target = T;
 
     fn deref(&self) -> &Self::Target {

@@ -364,15 +356,15 @@ fn deref(&self) -> &Self::Target {
     }
 }
 
-impl<T: DriverObject, C: DeviceContext> DerefMut for Object<T, C> {
+impl<T: DriverObject> DerefMut for Object<T> {
     fn deref_mut(&mut self) -> &mut Self::Target {
         &mut self.inner
     }
 }
 
-impl<T: DriverObject, C: DeviceContext> Sealed for Object<T, C> {}
+impl<T: DriverObject> Sealed for Object<T> {}
 
-impl<T: DriverObject, C: DeviceContext> gem::IntoGEMObject for Object<T, C> {
+impl<T: DriverObject> gem::IntoGEMObject for Object<T> {
     fn as_raw(&self) -> *mut bindings::drm_gem_object {
         // SAFETY:
         // - Our immutable reference is proof that this is safe to dereference.

@@ -391,7 +383,7 @@ unsafe fn from_raw<'a>(obj: *mut bindings::drm_gem_object) -> &'a Self {
     }
 }
 
-impl<T: DriverObject, C: DeviceContext> driver::AllocImpl for Object<T, C> {
+impl<T: DriverObject> driver::AllocImpl for Object<T> {
     type Driver = T::Driver;
 
     const ALLOC_OPS: driver::AllocOps = driver::AllocOps {

@@ -410,14 +402,11 @@ impl<T: DriverObject, C: DeviceContext> driver::AllocImpl for Object<T, C> {
 /// When this is dropped, the `dma_resv` lock is dropped as well.
 ///
 // TODO: This should be replace with a WwMutex equivalent once we have such bindings in the kernel.
-struct DmaResvGuard<'a, T: DriverObject, C: DeviceContext = Normal>(
-    &'a Object<T, C>,
-    NotThreadSafe,
-);
+struct DmaResvGuard<'a, T: DriverObject>(&'a Object<T>, NotThreadSafe);
 
-impl<'a, T: DriverObject, C: DeviceContext> DmaResvGuard<'a, T, C> {
+impl<'a, T: DriverObject> DmaResvGuard<'a, T> {
     #[inline]
-    fn new(obj: &'a Object<T, C>) -> Self {
+    fn new(obj: &'a Object<T>) -> Self {
         // SAFETY: This lock is initialized throughout the lifetime of `object`.
         unsafe { bindings::dma_resv_lock(obj.raw_dma_resv(), ptr::null_mut()) };
 

@@ -425,7 +414,7 @@ fn new(obj: &'a Object<T, C>) -> Self {
     }
 }
 
-impl<'a, T: DriverObject, C: DeviceContext> Drop for DmaResvGuard<'a, T, C> {
+impl<'a, T: DriverObject> Drop for DmaResvGuard<'a, T> {
     #[inline]
     fn drop(&mut self) {
         // SAFETY: We are releasing the lock grabbed during the creation of this object.

@@ -439,40 +428,37 @@ fn drop(&mut self) {
 ///
 /// - The size of `owner` is >= SIZE.
 /// - The memory pointed to by `addr` remains valid at least until this object is dropped.
-pub struct VMap<D, R, C = Normal, const SIZE: usize = 0>
+pub struct VMap<D, R, const SIZE: usize = 0>
 where
     D: DriverObject,
-    C: DeviceContext,
-    R: Deref<Target = Object<D, C>>,
+    R: Deref<Target = Object<D>>,
 {
     addr: *mut c_void,
     owner: R,
 }
 
 /// An alias type for a reference to a shmem-based GEM object's VMap.
-pub type VMapRef<'a, D, C, const SIZE: usize = 0> = VMap<D, &'a Object<D, C>, C, SIZE>;
+pub type VMapRef<'a, D, const SIZE: usize = 0> = VMap<D, &'a Object<D>, SIZE>;
 
 /// An alias type for an owned reference to a shmem-based GEM object's VMap.
-pub type VMapOwned<D, C, const SIZE: usize = 0> = VMap<D, ARef<Object<D, C>>, C, SIZE>;
+pub type VMapOwned<D, const SIZE: usize = 0> = VMap<D, ARef<Object<D>>, SIZE>;
 
-impl<D, R, C, const SIZE: usize> VMap<D, R, C, SIZE>
+impl<D, R, const SIZE: usize> VMap<D, R, SIZE>
 where
     D: DriverObject,
-    C: DeviceContext,
-    R: Deref<Target = Object<D, C>>,
+    R: Deref<Target = Object<D>>,
 {
     /// Borrows a reference to the object that owns this virtual mapping.
     #[inline]
-    pub fn owner(&self) -> &Object<D, C> {
+    pub fn owner(&self) -> &Object<D> {
         &self.owner
     }
 }
 
-impl<D, R, C, const SIZE: usize> Drop for VMap<D, R, C, SIZE>
+impl<D, R, const SIZE: usize> Drop for VMap<D, R, SIZE>
 where
     D: DriverObject,
-    C: DeviceContext,
-    R: Deref<Target = Object<D, C>>,
+    R: Deref<Target = Object<D>>,
 {
     #[inline]
     fn drop(&mut self) {

@@ -491,29 +477,26 @@ fn drop(&mut self) {
 
 // SAFETY: `addr` points to a valid memory address for as long as `owner` exists, meaning that so
 // long as `owner` is `Send` so is `VMap`.
-unsafe impl<D, R, C, const SIZE: usize> Send for VMap<D, R, C, SIZE>
+unsafe impl<D, R, const SIZE: usize> Send for VMap<D, R, SIZE>
 where
     D: DriverObject,
-    C: DeviceContext,
-    R: Deref<Target = Object<D, C>> + Send,
+    R: Deref<Target = Object<D>> + Send,
 {
 }
 
 // SAFETY: `addr` points to a valid memory address for as long as `owner` exists, meaning that so
 // long as `owner` is `Sync` so is `VMap`.
-unsafe impl<D, R, C, const SIZE: usize> Sync for VMap<D, R, C, SIZE>
+unsafe impl<D, R, const SIZE: usize> Sync for VMap<D, R, SIZE>
 where
     D: DriverObject,
-    C: DeviceContext,
-    R: Deref<Target = Object<D, C>> + Sync,
+    R: Deref<Target = Object<D>> + Sync,
 {
 }
 
-impl<D, R, C, const SIZE: usize> Io for VMap<D, R, C, SIZE>
+impl<D, R, const SIZE: usize> Io for VMap<D, R, SIZE>
 where
     D: DriverObject,
-    C: DeviceContext,
-    R: Deref<Target = Object<D, C>>,
+    R: Deref<Target = Object<D>>,
 {
     #[inline]
     fn addr(&self) -> usize {

@@ -526,22 +509,20 @@ fn maxsize(&self) -> usize {
     }
 }
 
-impl<D, R, C, const SIZE: usize> IoKnownSize for VMap<D, R, C, SIZE>
+impl<D, R, const SIZE: usize> IoKnownSize for VMap<D, R, SIZE>
 where
     D: DriverObject,
-    C: DeviceContext,
-    R: Deref<Target = Object<D, C>>,
+    R: Deref<Target = Object<D>>,
 {
     const MIN_SIZE: usize = SIZE;
 }
 
 macro_rules! impl_vmap_io_capable {
     ($ty:ty) => {
-        impl<D, R, C, const SIZE: usize> IoCapable<$ty> for VMap<D, R, C, SIZE>
+        impl<D, R, const SIZE: usize> IoCapable<$ty> for VMap<D, R, SIZE>
         where
             D: DriverObject,
-            C: DeviceContext,
-            R: Deref<Target = Object<D, C>>,
+            R: Deref<Target = Object<D>>,
         {
             #[inline]
             unsafe fn io_read(&self, address: usize) -> $ty {

@@ -584,11 +565,11 @@ unsafe fn io_write(&self, value: $ty, address: usize) {
 ///   [`SGTable`].
 ///
 /// [`SGTable`]: scatterlist::SGTable
-pub struct SGTableMap<T: DriverObject, C: DeviceContext> {
-    obj: NonNull<Object<T, C>>,
+pub struct SGTableMap<T: DriverObject> {
+    obj: NonNull<Object<T>>,
 }
 
-impl<T: DriverObject, C: DeviceContext> Deref for SGTableMap<T, C> {
+impl<T: DriverObject> Deref for SGTableMap<T> {
     type Target = scatterlist::SGTable;
 
     fn deref(&self) -> &Self::Target {

@@ -599,7 +580,7 @@ fn deref(&self) -> &Self::Target {
     }
 }
 
-impl<T: DriverObject, C: DeviceContext> Drop for SGTableMap<T, C> {
+impl<T: DriverObject> Drop for SGTableMap<T> {
     fn drop(&mut self) {
         // SAFETY: `obj` is always valid via our type invariants
         let obj = unsafe { self.obj.as_ref() };

@@ -610,8 +591,8 @@ fn drop(&mut self) {
     }
 }
 
-impl<T: DriverObject, C: DeviceContext> SGTableMap<T, C> {
-    fn new(obj: &Object<T, C>) -> impl Init<Self, Error> {
+impl<T: DriverObject> SGTableMap<T> {
+    fn new(obj: &Object<T>) -> impl Init<Self, Error> {
         // INVARIANT:
         // - We call drm_gem_shmem_get_pages_sgt below and check whether or not it succeeds,
         //   fulfilling the invariant of SGTableMap that the object's `sgt` field is initialized.

@@ -625,10 +606,10 @@ fn new(obj: &Object<T, C>) -> impl Init<Self, Error> {
 
 // SAFETY: The NonNull in SGTableMap is guaranteed valid by our type invariants, and the GEM object
 // it points to is guaranteed to be thread-safe.
-unsafe impl<T: DriverObject, C: DeviceContext> Send for SGTableMap<T, C> {}
+unsafe impl<T: DriverObject> Send for SGTableMap<T> {}
 // SAFETY: The NonNull in SGTableMap is guaranteed valid by our type invariants, and the GEM object
 // it points to is guaranteed to be thread-safe.
-unsafe impl<T: DriverObject, C: DeviceContext> Sync for SGTableMap<T, C> {}
+unsafe impl<T: DriverObject> Sync for SGTableMap<T> {}
 
 #[kunit_tests(rust_drm_gem_shmem)]
 mod tests {

@@ -705,7 +686,7 @@ fn create_drm_dev() -> Result<(faux::Registration, UnregisteredDevice<KunitDrive
     fn compile_time_vmap_sizes() -> Result {
         let (_dev, drm) = create_drm_dev()?;
 
-        let obj = Object::<KunitObject, _>::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?;
+        let obj = Object::<KunitObject>::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?;
 
         // Try creating a normal vmap
         obj.vmap::<PAGE_SIZE>()?;

@@ -729,7 +710,7 @@ fn compile_time_vmap_sizes() -> Result {
     fn vmap_io() -> Result {
         let (_dev, drm) = create_drm_dev()?;
 
-        let obj = Object::<KunitObject, _>::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?;
+        let obj = Object::<KunitObject>::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?;
 
         let vmap = obj.vmap::<PAGE_SIZE>()?;
 

@@ -761,7 +742,7 @@ fn fail_sg_table_on_wrong_dev() -> Result {
         let reg = faux::Registration::new(c"EvilKunit", None)?;
         let wrong_dev = reg.as_ref();
 
-        let obj = Object::<KunitObject, _>::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?;
+        let obj = Object::<KunitObject>::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?;
 
         assert_eq!(obj.sg_table(wrong_dev.as_ref()).err().unwrap(), EINVAL);
 

index 6f3af46ff647..86a7fca1d33f 100644
@@ -79,6 +79,9 @@ macro_rules! drm_legacy_fields {
 /// - [`Normal`]: The general-purpose, reference-counted context. A [`Device`] in this context may
 ///   or may not be registered with userspace.
 /// - [`Registered`]: The device has been registered with userspace at some point.
+///
+/// `Device<T, Registered>` dereferences to `Device<T>` ([`Normal`]), so any method available on a
+/// [`Normal`] device is also available on a [`Registered`] one.
 pub trait DeviceContext: Sealed + Send + Sync {}
 
 /// The general-purpose, reference-counted [`DeviceContext`].

@@ -320,7 +323,7 @@ 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> {
     type Target = T::Data;
 
     fn deref(&self) -> &Self::Target {

@@ -328,6 +331,17 @@ fn deref(&self) -> &Self::Target {
     }
 }
 
+impl<T: drm::Driver> Deref for Device<T, Registered> {
+    type Target = Device<T>;
+
+    #[inline]
+    fn deref(&self) -> &Self::Target {
+        // SAFETY: The caller holds a `Device<T, Registered>`, which guarantees all invariants
+        // of the weaker `Normal` context.
+        unsafe { self.assume_ctx() }
+    }
+}
+
 // 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> {

index ccf4150d83b6..6f5a9877bdae 100644
@@ -134,7 +134,8 @@ macro_rules! declare_drm_ioctls {
                             // FIXME: Currently there is nothing enforcing that the types of the
                             // dev/file match the current driver these ioctls are being declared
                             // for, and it's not clear how to enforce this within the type system.
-                            let dev = $crate::drm::device::Device::from_raw(raw_dev);
+                            let dev: &$crate::drm::device::Device<_, $crate::drm::Normal> =
+                                $crate::drm::device::Device::from_raw(raw_dev);
 
                             // Enforce that the handler accepts higher-ranked
                             // lifetimes, preventing it from requiring 'static

index 86a7fca1d33f..d429b4655449 100644
@@ -74,14 +74,16 @@ macro_rules! drm_legacy_fields {
 
 /// A trait implemented by all possible contexts a [`Device`] can be used in.
 ///
-/// A [`Device`] can be in one of two contexts:
+/// A [`Device`] can be in one of the following contexts:
 ///
 /// - [`Normal`]: The general-purpose, reference-counted context. A [`Device`] in this context may
 ///   or may not be registered with userspace.
+/// - [`Ioctl`]: The device has been registered with userspace at some point; used in ioctl
+///   dispatch context.
 /// - [`Registered`]: The device has been registered with userspace at some point.
 ///
-/// `Device<T, Registered>` dereferences to `Device<T>` ([`Normal`]), so any method available on a
-/// [`Normal`] device is also available on a [`Registered`] one.
+/// Both `Device<T, Ioctl>` and `Device<T, Registered>` dereference to `Device<T>` ([`Normal`]),
+/// so any method available on a [`Normal`] device is also available in the other contexts.
 pub trait DeviceContext: Sealed + Send + Sync {}
 
 /// The general-purpose, reference-counted [`DeviceContext`].

@@ -113,6 +115,21 @@ impl DeviceContext for Normal {}
 impl Sealed for Registered {}
 impl DeviceContext for Registered {}
 
+/// The [`DeviceContext`] of a [`Device`] that has been registered with userspace previously.
+///
+/// A [`Device`] in this context has been registered at some point, but may be concurrently
+/// unregistering or already unregistered. `drm_dev_enter()` can guard against this, ensuring the
+/// device remains registered for the duration of the critical section.
+///
+/// # Invariants
+///
+/// A [`Device`] in this context has been registered with userspace via `drm_dev_register()` at
+/// some point.
+pub struct Ioctl;
+
+impl Sealed for Ioctl {}
+impl DeviceContext for Ioctl {}
+
 /// A [`Device`] which is known at compile-time to be unregistered with userspace.
 ///
 /// This type allows performing operations which are only safe to do before userspace registration,

@@ -342,6 +359,17 @@ fn deref(&self) -> &Self::Target {
     }
 }
 
+impl<T: drm::Driver> Deref for Device<T, Ioctl> {
+    type Target = Device<T>;
+
+    #[inline]
+    fn deref(&self) -> &Self::Target {
+        // SAFETY: The caller holds a `Device<T, Ioctl>`, which guarantees all invariants
+        // of the weaker `Normal` context.
+        unsafe { self.assume_ctx() }
+    }
+}
+
 // 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> {

index e5bfaf130342..a6693d2b84b8 100644
@@ -11,6 +11,7 @@
 
 pub use self::device::Device;
 pub use self::device::DeviceContext;
+pub use self::device::Ioctl;
 pub use self::device::Normal;
 pub use self::device::Registered;
 pub use self::device::UnregisteredDevice;

index d429b4655449..c32cc0f0eba0 100644
@@ -80,7 +80,8 @@ macro_rules! drm_legacy_fields {
 ///   or may not be registered with userspace.
 /// - [`Ioctl`]: The device has been registered with userspace at some point; used in ioctl
 ///   dispatch context.
-/// - [`Registered`]: The device has been registered with userspace at some point.
+/// - [`Registered`]: The device is currently registered with userspace and the parent bus device
+///   is bound.
 ///
 /// Both `Device<T, Ioctl>` and `Device<T, Registered>` dereference to `Device<T>` ([`Normal`]),
 /// so any method available on a [`Normal`] device is also available in the other contexts.

@@ -98,18 +99,15 @@ pub trait DeviceContext: Sealed + Send + Sync {}
 impl Sealed for Normal {}
 impl DeviceContext for Normal {}
 
-/// The [`DeviceContext`] of a [`Device`] that was registered with userspace at some point.
+/// The [`DeviceContext`] of a [`Device`] that is currently registered with userspace.
 ///
-/// This represents a [`Device`] which is guaranteed to have been registered with userspace at
-/// some point in time. Such a DRM device is guaranteed to have been fully-initialized.
-///
-/// Note: A device in this context is not guaranteed to remain registered with userspace for its
-/// entire lifetime, as this is impossible to guarantee at compile-time.
+/// A [`Device`] in this context is guaranteed to be registered and its parent bus device is
+/// guaranteed to be bound. This is enforced at runtime by [`RegistrationGuard`], which holds a
+/// `drm_dev_enter()` / `drm_dev_exit()` SRCU critical section.
 ///
 /// # Invariants
 ///
-/// A [`Device`] in this [`DeviceContext`] is guaranteed to have been registered with userspace
-/// at some point in time.
+/// The parent bus device is bound for the duration of any reference to a `Device<T, Registered>`.
 pub struct Registered;
 
 impl Sealed for Registered {}

@@ -260,8 +258,8 @@ pub fn new(
 
 /// A typed DRM device with a specific [`drm::Driver`] implementation and [`DeviceContext`].
 ///
-/// A device in the [`Registered`] context is guaranteed to have been registered with userspace
-/// at some point. The [`Normal`] context is the general-purpose, reference-counted context.
+/// A device in the [`Registered`] context is currently registered with userspace and its parent
+/// bus device is bound. The [`Normal`] context is the general-purpose, reference-counted context.
 ///
 /// # Invariants
 ///

@@ -340,6 +338,71 @@ pub(crate) unsafe fn assume_ctx<NewCtx: DeviceContext>(&self) -> &Device<T, NewC
     }
 }
 
+impl<T: drm::Driver> Device<T, Ioctl> {
+    /// Guard against the parent bus device being unbound.
+    ///
+    /// Returns a [`RegistrationGuard`] if the device has not been unplugged, [`None`] otherwise.
+    ///
+    /// While [`RegistrationGuard`] is held the parent device is guaranteed to be bound.
+    #[must_use]
+    pub fn registration_guard(&self) -> Option<RegistrationGuard<'_, T>> {
+        let mut idx: i32 = 0;
+        // SAFETY: `self.as_raw()` is a valid pointer to a `struct drm_device`.
+        if unsafe { bindings::drm_dev_enter(self.as_raw(), &mut idx) } {
+            // INVARIANT:
+            // - `idx` is the SRCU index from the successful `drm_dev_enter()` above.
+            // - The parent bus device is bound: `drm_dev_enter()` succeeded, meaning
+            //   `drm_dev_unplug()` has not completed; since it is only called from
+            //   `Registration::drop()` during parent unbind, the parent is still bound.
+            Some(RegistrationGuard {
+                // SAFETY: See INVARIANT above; the `Registered` context invariant holds.
+                dev: unsafe { self.assume_ctx() },
+                idx,
+                _not_send: NotThreadSafe,
+            })
+        } else {
+            None
+        }
+    }
+}
+
+/// A guard proving the DRM device is registered and the parent bus device is bound.
+///
+/// The guard dereferences to [`Device<T, Registered>`], providing access to the DRM device with
+/// the guarantee that the parent bus device is bound for the entire duration of the critical
+/// section.
+///
+/// Internally this is backed by a `drm_dev_enter()` / `drm_dev_exit()` SRCU critical section.
+///
+/// # Invariants
+///
+/// - `idx` is the SRCU read lock index returned by a successful `drm_dev_enter()` call.
+/// - The parent bus device of `dev` is bound for the lifetime of this guard.
+#[must_use]
+pub struct RegistrationGuard<'a, T: drm::Driver> {
+    dev: &'a Device<T, Registered>,
+    idx: i32,
+    _not_send: NotThreadSafe,
+}
+
+impl<T: drm::Driver> Deref for RegistrationGuard<'_, T> {
+    type Target = Device<T, Registered>;
+
+    #[inline]
+    fn deref(&self) -> &Self::Target {
+        self.dev
+    }
+}
+
+impl<T: drm::Driver> Drop for RegistrationGuard<'_, T> {
+    #[inline]
+    fn drop(&mut self) {
+        // SAFETY: `self.idx` was returned by a successful `drm_dev_enter()` call, as guaranteed
+        // by the type invariants of `RegistrationGuard`.
+        unsafe { bindings::drm_dev_exit(self.idx) };
+    }
+}
+
 impl<T: drm::Driver> Deref for Device<T> {
     type Target = T::Data;
 

index 5152a18a8312..3cda8dceb498 100644
@@ -199,8 +199,14 @@ unsafe impl<T: Driver> Send for Registration<T> {}
 
 impl<T: Driver> Drop for Registration<T> {
     fn drop(&mut self) {
+        // Use `drm_dev_unplug` rather than `drm_dev_unregister` to ensure that existing
+        // `drm_dev_enter()` critical sections complete before unregistration proceeds. This
+        // is required for the safety of `RegistrationGuard`, which relies on the SRCU barrier in
+        // `drm_dev_unplug()` to guarantee that the parent device is still bound within the
+        // critical section.
+        //
         // SAFETY: Safe by the invariant of `ARef<drm::Device<T>>`. The existence of this
-        // `Registration` also guarantees the this `drm::Device` is actually registered.
-        unsafe { bindings::drm_dev_unregister(self.0.as_raw()) };
+        // `Registration` also guarantees that this `drm::Device` is actually registered.
+        unsafe { bindings::drm_dev_unplug(self.0.as_raw()) };
     }
 }

index a6693d2b84b8..fd6ed35bc35a 100644
@@ -14,6 +14,7 @@
 pub use self::device::Ioctl;
 pub use self::device::Normal;
 pub use self::device::Registered;
+pub use self::device::RegistrationGuard;
 pub use self::device::UnregisteredDevice;
 pub use self::driver::Driver;
 pub use self::driver::DriverInfo;

index a3b7bd36792c..19fb89b28984 100644
@@ -4,7 +4,11 @@
 use crate::gem::NovaObject;
 use kernel::{
     alloc::flags::*,
-    drm::{self, gem::BaseObject},
+    drm::{
+        self,
+        gem::BaseObject,
+        Registered, //
+    },
     pci,
     prelude::*,
     uapi,

@@ -23,7 +27,7 @@ fn open(_dev: &NovaDevice) -> Result<Pin<KBox<Self>>> {
 impl File {
     /// IOCTL: get_param: Query GPU / driver metadata.
     pub(crate) fn get_param(
-        dev: &NovaDevice,
+        dev: &NovaDevice<Registered>,
         getparam: &mut uapi::drm_nova_getparam,
         _file: &drm::File<File>,
     ) -> Result<u32> {

@@ -43,7 +47,7 @@ pub(crate) fn get_param(
 
     /// IOCTL: gem_create: Create a new DRM GEM object.
     pub(crate) fn gem_create(
-        dev: &NovaDevice,
+        dev: &NovaDevice<Registered>,
         req: &mut uapi::drm_nova_gem_create,
         file: &drm::File<File>,
     ) -> Result<u32> {

@@ -56,7 +60,7 @@ pub(crate) fn gem_create(
 
     /// IOCTL: gem_info: Query GEM metadata.
     pub(crate) fn gem_info(
-        _dev: &NovaDevice,
+        _dev: &NovaDevice<Registered>,
         req: &mut uapi::drm_nova_gem_info,
         file: &drm::File<File>,
     ) -> Result<u32> {

index 31411da203c5..fb9233eae01c 100644
@@ -1,7 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0 or MIT
 
 use kernel::{
-    drm,
+    drm::{
+        self,
+        Registered, //
+    },
     prelude::*,
     uaccess::UserSlice,
     uapi, //

@@ -28,7 +31,7 @@ fn open(_dev: &drm::Device<Self::Driver>) -> Result<Pin<KBox<Self>>> {
 
 impl TyrDrmFileData {
     pub(crate) fn dev_query(
-        ddev: &TyrDrmDevice,
+        ddev: &TyrDrmDevice<Registered>,
         devquery: &mut uapi::drm_panthor_dev_query,
         _file: &TyrDrmFile,
     ) -> Result<u32> {

79 lines not on this page · raw message