From 68b151bc6145dea3db5598ebaf4b776cd205e395 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:21 +0200 Subject: rust: drm: ioctl: fix unbounded lifetimes in ioctl handler arguments References to dev, data, and file in the declare_drm_ioctls! macro are created via unsafe pointer dereferences, producing unbounded lifetimes. If an ioctl handler explicitly annotates its parameters with 'static, the compiler accepts this, allowing the handler to stash references that outlive the ioctl call. Fix this by adding a higher-ranked function pointer coercion that enforces the handler accepts universally quantified lifetimes: let _: for<'a> fn(&'a _, &'a mut _, &'a _) -> _ = $func; Since the handler must be coercible to a function pointer accepting any lifetime 'a, it can no longer demand 'static on any parameter. Cc: stable@vger.kernel.org Fixes: 9a69570682b1 ("rust: drm: ioctl: Add DRM ioctl abstraction") Reported-by: sashiko-bot@kernel.org Closes: https://lore.kernel.org/all/20260620011346.A47D01F000E9@smtp.kernel.org/ Suggested-by: Gary Guo Reviewed-by: Alexandre Courbot Reviewed-by: Lyude Paul Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-2-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/ioctl.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs index cf328101dde4..ccf4150d83b6 100644 --- a/rust/kernel/drm/ioctl.rs +++ b/rust/kernel/drm/ioctl.rs @@ -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. -- cgit v1.2.3 From fd0f827c532b976d38e3cbbda5d3fa60a82ce8d5 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:22 +0200 Subject: rust: drm: rename Uninit DeviceContext to Normal Rename the Uninit DeviceContext to Normal to better reflect its purpose as the general-purpose, reference-counted device context. The Uninit name was a leftover from when DRM device private data initialization was planned to split across UnregisteredDevice::new() and Registration::new(); with the subsequent introduction of RegistrationData, this distinction is no longer needed. This also simplifies the DeviceContext documentation, trimming the multi-stage initialization description that no longer applies. Subsequent patches will refine the semantics of the Registered context accordingly. No functional change. Reviewed-by: Lyude Paul Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-3-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/device.rs | 92 ++++++++++++++--------------------------------- rust/kernel/drm/mod.rs | 2 +- 2 files changed, 28 insertions(+), 66 deletions(-) diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 7ad124327a83..35ff9c6942d8 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -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` 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` 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 + 'static {} +/// 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 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, @@ -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(ARef>, NotThreadSafe); +pub struct UnregisteredDevice(ARef>, NotThreadSafe); impl Deref for UnregisteredDevice { - type Target = Device; + type Target = Device; fn deref(&self) -> &Self::Target { &self.0 @@ -178,15 +150,13 @@ impl UnregisteredDevice { 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::::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, + 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, @@ -211,7 +181,7 @@ impl UnregisteredDevice { pub fn new(dev: &device::Device, data: impl PinInit) -> Result { // `__drm_dev_alloc` uses `kmalloc()` to allocate memory, hence ensure a `kmalloc()` // compatible `Layout`. - let layout = Kmalloc::aligned_layout(Layout::new::>()); + let layout = Kmalloc::aligned_layout(Layout::new::>()); // 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 @@ impl UnregisteredDevice { // SAFETY: // - `alloc_vtable` reference remains valid until no longer used, // - `dev` is valid by its type invarants, - let raw_drm: *mut Device = unsafe { + let raw_drm: *mut Device = unsafe { bindings::__drm_dev_alloc( dev.as_raw(), &alloc_vtable, layout.size(), - mem::offset_of!(Device, dev), + mem::offset_of!(Device, dev), ) } .cast(); @@ -264,16 +234,8 @@ impl UnregisteredDevice { /// 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 /// diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs index a66e7166f66b..e5bfaf130342 100644 --- a/rust/kernel/drm/mod.rs +++ b/rust/kernel/drm/mod.rs @@ -11,8 +11,8 @@ pub mod ioctl; 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; -- cgit v1.2.3 From 893f39dadaa172c0f869e5b138f2cfdd851df781 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:23 +0200 Subject: rust: faux: add Device type with AsBusDevice support Add a faux::Device type that wraps struct faux_device and implements AsBusDevice, enabling faux devices to be used as parent devices for subsystems that require a bus device, such as DRM. Update Registration to return &faux::Device via AsRef. Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-4-dakr@kernel.org [ Drop redundant 'struct device' invariant; implied by valid struct faux_device. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/drm/gem/shmem.rs | 11 ++++--- rust/kernel/faux.rs | 69 ++++++++++++++++++++++++++++++++++------ samples/rust/rust_driver_faux.rs | 3 +- 3 files changed, 68 insertions(+), 15 deletions(-) diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index 3ee19ef6264e..52de59b14dad 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -692,10 +692,12 @@ mod tests { fn create_drm_dev() -> Result<(faux::Registration, UnregisteredDevice)> { // 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 @@ mod tests { #[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::::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; diff --git a/rust/kernel/faux.rs b/rust/kernel/faux.rs index 36c92ae2943c..cd4198fbb232 100644 --- a/rust/kernel/faux.rs +++ b/rust/kernel/faux.rs @@ -9,15 +9,63 @@ use crate::{ bindings, device, - prelude::*, // + prelude::*, + types::Opaque, // }; -use core::ptr::{ - addr_of_mut, - null, - null_mut, - NonNull, // +use core::{ + marker::PhantomData, + ptr::{ + null, + null_mut, + NonNull, // + }, }; +/// A faux device. +/// +/// A faux device is a virtual device backed by the faux bus, primarily used for scenarios where a +/// real hardware device is not available or for testing. +/// +/// # Invariants +/// +/// The underlying `struct faux_device` is valid. +#[repr(transparent)] +pub struct Device( + Opaque, + PhantomData, +); + +impl Device { + #[inline] + fn as_raw(&self) -> *mut bindings::faux_device { + self.0.get() + } + + /// # Safety + /// + /// `ptr` must be a valid pointer to a `struct faux_device`. + #[inline] + unsafe fn from_raw<'a>(ptr: *mut bindings::faux_device) -> &'a Self { + // SAFETY: `Device` is a transparent wrapper of `Opaque`. + unsafe { &*ptr.cast() } + } +} + +impl AsRef> for Device { + #[inline] + fn as_ref(&self) -> &device::Device { + // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid + // `struct faux_device`. `dev` points to a valid `struct device`. + unsafe { device::Device::from_raw(&raw mut (*self.as_raw()).dev) } + } +} + +// SAFETY: `faux::Device` is a transparent wrapper of `struct faux_device`. +// The offset is guaranteed to point to a valid device field inside `faux::Device`. +unsafe impl device::AsBusDevice for Device { + const OFFSET: usize = core::mem::offset_of!(bindings::faux_device, dev); +} + /// The registration of a faux device. /// /// This type represents the registration of a [`struct faux_device`]. When an instance of this type @@ -60,10 +108,11 @@ impl Registration { } } -impl AsRef> for Registration { - fn as_ref(&self) -> &device::Device { +impl AsRef> for Registration { + #[inline] + fn as_ref(&self) -> &Device { // SAFETY: - // - The underlying `device` in `faux_device` is guaranteed by the C API to be a valid + // - The underlying `struct faux_device` is guaranteed by the C API to be a valid // initialized `device`. // - `faux_match()` always returns 1, and probe runs synchronously // (PROBE_FORCE_SYNCHRONOUS). @@ -71,7 +120,7 @@ impl AsRef> for Registration { // sysfs. // - `mem::forget(Registration)` is not a problem; if the `Registration` is leaked, the faux // device stays bound forever. - unsafe { device::Device::from_raw(addr_of_mut!((*self.as_raw()).dev)) } + unsafe { Device::from_raw(self.as_raw()) } } } diff --git a/samples/rust/rust_driver_faux.rs b/samples/rust/rust_driver_faux.rs index 99876c8e3743..27b6d3e2bb44 100644 --- a/samples/rust/rust_driver_faux.rs +++ b/samples/rust/rust_driver_faux.rs @@ -25,8 +25,9 @@ impl Module for SampleModule { 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 }) } -- cgit v1.2.3 From 49e27d58a06d7edda4791b9fa818a536dd34b7e5 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:24 +0200 Subject: rust: drm: Add Driver::ParentDevice associated type Add a ParentDevice associated type to the Driver trait, allowing each DRM driver to declare its parent bus device type (e.g. auxiliary::Device, platform::Device). Change UnregisteredDevice::new() to take &T::ParentDevice, ensuring at the type level that the DRM device's parent matches the declared bus device type. Reviewed-by: Lyude Paul Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-5-dakr@kernel.org Signed-off-by: Danilo Krummrich --- drivers/gpu/drm/nova/driver.rs | 8 ++++++-- drivers/gpu/drm/tyr/driver.rs | 6 ++++-- rust/kernel/drm/device.rs | 7 +++++-- rust/kernel/drm/driver.rs | 3 +++ rust/kernel/drm/gem/shmem.rs | 4 ++-- 5 files changed, 20 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs index 48933d86ddda..c5b0313006bd 100644 --- a/drivers/gpu/drm/nova/driver.rs +++ b/drivers/gpu/drm/nova/driver.rs @@ -2,7 +2,10 @@ use kernel::{ auxiliary, - device::Core, + device::{ + Core, + DeviceContext, // + }, drm::{ self, gem, @@ -62,7 +65,7 @@ impl auxiliary::Driver for NovaDriver { ) -> impl PinInit, Error> + 'bound { let data = try_pin_init!(NovaData { adev: adev.into() }); - let drm = drm::UnregisteredDevice::::new(adev.as_ref(), data)?; + let drm = drm::UnregisteredDevice::::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 = gem::Object; + type ParentDevice = auxiliary::Device; const INFO: drm::DriverInfo = INFO; diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs index d063bc664cc1..338c25ccc151 100644 --- a/drivers/gpu/drm/tyr/driver.rs +++ b/drivers/gpu/drm/tyr/driver.rs @@ -7,7 +7,8 @@ use kernel::{ }, device::{ Core, - Device, // + Device, + DeviceContext, // }, dma::{ Device as DmaDevice, @@ -148,7 +149,7 @@ impl platform::Driver for TyrPlatformDriver { gpu_info, }); - let tdev = drm::UnregisteredDevice::::new(pdev.as_ref(), data)?; + let tdev = drm::UnregisteredDevice::::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 = drm::gem::shmem::Object; + type ParentDevice = platform::Device; const INFO: drm::DriverInfo = INFO; const FEAT_RENDER: bool = true; diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 35ff9c6942d8..343c0ef63c6c 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -178,7 +178,10 @@ impl UnregisteredDevice { /// 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) -> Result { + pub fn new( + dev: &T::ParentDevice, + data: impl PinInit, + ) -> Result { // `__drm_dev_alloc` uses `kmalloc()` to allocate memory, hence ensure a `kmalloc()` // compatible `Layout`. let layout = Kmalloc::aligned_layout(Layout::new::>()); @@ -195,7 +198,7 @@ impl UnregisteredDevice { // - `dev` is valid by its type invarants, let raw_drm: *mut Device = unsafe { bindings::__drm_dev_alloc( - dev.as_raw(), + dev.as_ref().as_raw(), &alloc_vtable, layout.size(), mem::offset_of!(Device, dev), diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index 25f7e233884d..802e7fc13e30 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -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: device::AsBusDevice; + /// Driver metadata const INFO: DriverInfo; diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index 52de59b14dad..cbcfc7e4edb6 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -684,6 +684,7 @@ mod tests { type Data = KunitData; type File = KunitFile; type Object = Object; + type ParentDevice = faux::Device; const INFO: drm::DriverInfo = INFO; const IOCTLS: &'static [drm::ioctl::DrmIoctlDescriptor] = &[]; @@ -694,8 +695,7 @@ mod tests { 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)) } -- cgit v1.2.3 From 9030013008743626d367ff975085b4c14e9fd86b Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:25 +0200 Subject: rust: drm: change default DeviceContext to Normal Change the default DeviceContext from Registered to Normal for drm::Device, gem::Object, gem::shmem::Object and gem::shmem::ObjectConfig. Normal is the general-purpose, reference-counted context suitable for most uses; Registered represents a device that was registered with userspace and will become a non-owning context obtained through a RegistrationGuard. Update the create_handle/lookup_handle bounds from Object to Object to match the new default context of GEM objects, and update the driver device type aliases (NovaDevice, TyrDrmDevice) to default to Normal. Reviewed-by: Lyude Paul Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-6-dakr@kernel.org Signed-off-by: Danilo Krummrich --- drivers/gpu/drm/nova/driver.rs | 2 +- drivers/gpu/drm/tyr/driver.rs | 2 +- rust/kernel/drm/device.rs | 2 +- rust/kernel/drm/gem/mod.rs | 7 ++++--- rust/kernel/drm/gem/shmem.rs | 10 +++++----- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs index c5b0313006bd..8ddb81fd0c87 100644 --- a/drivers/gpu/drm/nova/driver.rs +++ b/drivers/gpu/drm/nova/driver.rs @@ -26,7 +26,7 @@ pub(crate) struct Nova { } /// Convienence type alias for the DRM device type for this driver -pub(crate) type NovaDevice = drm::Device; +pub(crate) type NovaDevice = drm::Device; #[pin_data] pub(crate) struct NovaData { diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs index 338c25ccc151..180631daff02 100644 --- a/drivers/gpu/drm/tyr/driver.rs +++ b/drivers/gpu/drm/tyr/driver.rs @@ -47,7 +47,7 @@ pub(crate) type IoMem<'a> = kernel::io::mem::IoMem<'a, SZ_2M>; pub(crate) struct TyrDrmDriver; /// Convenience type alias for the DRM device type for this driver. -pub(crate) type TyrDrmDevice = drm::Device; +pub(crate) type TyrDrmDevice = drm::Device; pub(crate) struct TyrPlatformDriver; diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 343c0ef63c6c..458519d62a46 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -246,7 +246,7 @@ impl UnregisteredDevice { /// * The data layout of `Self` remains the same across all implementations of `C`. /// * Any invariants for `C` also apply. #[repr(C)] -pub struct Device { +pub struct Device { dev: Opaque, data: T::Data, _ctx: PhantomData, diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index 48fa6e96dfe7..6a688568afbb 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -10,6 +10,7 @@ use crate::{ self, device::{ DeviceContext, + Normal, Registered, // }, driver::{ @@ -183,7 +184,7 @@ pub trait BaseObject: IntoGEMObject { fn create_handle(&self, file: &drm::File) -> Result where Self: AllocImpl, - D: drm::Driver = Self, File = F>, + D: drm::Driver = Self, File = F>, F: drm::file::DriverFile, { let mut handle: u32 = 0; @@ -198,7 +199,7 @@ pub trait BaseObject: IntoGEMObject { fn lookup_handle(file: &drm::File, handle: u32) -> Result> where Self: AllocImpl, - D: drm::Driver = Self, File = F>, + D: drm::Driver = Self, File = F>, F: drm::file::DriverFile, { // SAFETY: The arguments are all valid per the type invariants. @@ -254,7 +255,7 @@ impl 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 { +pub struct Object { obj: Opaque, #[pin] data: T, diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index cbcfc7e4edb6..5ffa1355ecf2 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -22,7 +22,7 @@ use crate::{ private::Sealed, Device, DeviceContext, - Registered, // + Normal, // }, error::{ from_err_ptr, @@ -73,7 +73,7 @@ use gem::{ /// /// 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 @@ impl<'a, T: DriverObject, C: DeviceContext> Default for ObjectConfig<'a, T, C> { /// - Any type invariants of `C` apply to the parent DRM device for this GEM object. #[repr(C)] #[pin_data] -pub struct Object { +pub struct Object { #[pin] obj: Opaque, /// Parent object that owns this object's DMA reservation object. @@ -409,7 +409,7 @@ impl driver::AllocImpl for Object { /// 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, NotThreadSafe, ); @@ -438,7 +438,7 @@ impl<'a, T: DriverObject, C: DeviceContext> Drop for DmaResvGuard<'a, T, C> { /// /// - The size of `owner` is >= SIZE. /// - The memory pointed to by `addr` remains valid at least until this object is dropped. -pub struct VMap +pub struct VMap where D: DriverObject, C: DeviceContext, -- cgit v1.2.3 From 506a7d63dab00f0f279869c956b986749292623a Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:26 +0200 Subject: rust: drm: restrict AlwaysRefCounted to Normal Device context Restrict the AlwaysRefCounted implementation for drm::Device to the Normal context. Registered devices represent a non-owning view of a device within a RegistrationGuard scope and must not be independently reference-counted. Reviewed-by: Lyude Paul Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-7-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/device.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 458519d62a46..312850f125af 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -85,6 +85,9 @@ pub trait DeviceContext: Sealed + Send + Sync + 'static {} /// /// 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`, making this the required +/// context for [`ARef`]-based device handles. pub struct Normal; impl Sealed for Normal {} @@ -327,7 +330,7 @@ impl Deref for Device { // SAFETY: DRM device objects are always reference counted and the get/put functions // satisfy the requirements. -unsafe impl AlwaysRefCounted for Device { +unsafe impl AlwaysRefCounted for Device { 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 Send for Device {} // by the synchronization in `struct drm_device`. unsafe impl Sync for Device {} -impl WorkItem for Device +impl WorkItem for Device where - T: drm::Driver, T::Data: WorkItem>, T::Data: HasWork, - C: DeviceContext, { type Pointer = ARef; -- cgit v1.2.3 From ec8b2cc27c766fab80f70b02d196b7c3be7d10ce Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:27 +0200 Subject: rust: drm: restrict AlwaysRefCounted to Normal GEM Object context Restrict AlwaysRefCounted for gem::Object and gem::shmem::Object to the Normal context, since only Normal objects should be independently reference-counted. To avoid cascading through IntoGEMObject (which had AlwaysRefCounted as a supertrait), remove AlwaysRefCounted from IntoGEMObject's supertraits and instead add it as an explicit bound on lookup_handle(), which is the only BaseObject method that returns an ARef. Since Object::new() and shmem::Object::new() return ARef, move them to Normal-only impl blocks. Similarly, simplify ObjectConfig and shmem's parent_resv_obj field to the Normal context. Remove the DeviceContext generic from DriverObject::new() and Driver::Object, since GEM objects can only be constructed in the Normal context. Simplify DriverAllocImpl accordingly. Reviewed-by: Lyude Paul Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-8-dakr@kernel.org Signed-off-by: Danilo Krummrich --- drivers/gpu/drm/nova/driver.rs | 2 +- drivers/gpu/drm/nova/gem.rs | 18 +++--- drivers/gpu/drm/tyr/driver.rs | 2 +- drivers/gpu/drm/tyr/gem.rs | 11 +--- rust/kernel/drm/device.rs | 14 ++--- rust/kernel/drm/driver.rs | 2 +- rust/kernel/drm/gem/mod.rs | 97 +++++++++++++++---------------- rust/kernel/drm/gem/shmem.rs | 129 +++++++++++++++++++++-------------------- 8 files changed, 130 insertions(+), 145 deletions(-) diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs index 8ddb81fd0c87..e3c54303d70e 100644 --- a/drivers/gpu/drm/nova/driver.rs +++ b/drivers/gpu/drm/nova/driver.rs @@ -76,7 +76,7 @@ impl auxiliary::Driver for NovaDriver { impl drm::Driver for NovaDriver { type Data = NovaData; type File = File; - type Object = gem::Object; + type Object = gem::Object; type ParentDevice = auxiliary::Device; const INFO: drm::DriverInfo = INFO; diff --git a/drivers/gpu/drm/nova/gem.rs b/drivers/gpu/drm/nova/gem.rs index 9d8ff7de2c0f..2b6fe9dc0bfa 100644 --- a/drivers/gpu/drm/nova/gem.rs +++ b/drivers/gpu/drm/nova/gem.rs @@ -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( - _dev: &NovaDevice, - _size: usize, - _args: Self::Args, - ) -> impl PinInit { + fn new(_dev: &NovaDevice, _size: usize, _args: Self::Args) -> impl PinInit { try_pin_init!(NovaObject {}) } } impl NovaObject { /// Create a new DRM GEM object. - pub(crate) fn new( - dev: &NovaDevice, - size: usize, - ) -> Result>> { + pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result>> { if size == 0 { return Err(EINVAL); } let aligned_size = page::page_align(size).ok_or(EINVAL)?; - gem::Object::::new(dev, aligned_size, ()) + gem::Object::::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 180631daff02..7f082de6d6dc 100644 --- a/drivers/gpu/drm/tyr/driver.rs +++ b/drivers/gpu/drm/tyr/driver.rs @@ -182,7 +182,7 @@ const INFO: drm::DriverInfo = drm::DriverInfo { impl drm::Driver for TyrDrmDriver { type Data = TyrDrmDeviceData; type File = TyrDrmFileData; - type Object = drm::gem::shmem::Object; + type Object = drm::gem::shmem::Object; type ParentDevice = platform::Device; const INFO: drm::DriverInfo = INFO; diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs index c6d4d6f9bae3..1640a161754b 100644 --- a/drivers/gpu/drm/tyr/gem.rs +++ b/drivers/gpu/drm/tyr/gem.rs @@ -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( - _dev: &TyrDrmDevice, - _size: usize, - args: BoCreateArgs, - ) -> impl PinInit { + fn new(_dev: &TyrDrmDevice, _size: usize, args: BoCreateArgs) -> impl PinInit { try_pin_init!(Self { flags: args.flags }) } } diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 312850f125af..eb8146ea1c98 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -153,13 +153,13 @@ impl UnregisteredDevice { 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, + 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, diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index 802e7fc13e30..5152a18a8312 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -111,7 +111,7 @@ pub trait Driver { type Data: Sync + Send; /// The type used to manage memory for this driver. - type Object: AllocImpl; + type Object: 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 6a688568afbb..b03b5f9ccd7e 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -10,8 +10,7 @@ use crate::{ self, device::{ DeviceContext, - Normal, - Registered, // + Normal, // }, driver::{ AllocImpl, @@ -82,8 +81,7 @@ pub type DriverFile = drm::File<<::Driver as drm::Driver>: /// A type alias for retrieving the current [`AllocImpl`] for a given [`DriverObject`]. /// /// [`Driver`]: drm::Driver -pub type DriverAllocImpl = - <::Driver as drm::Driver>::Object; +pub type DriverAllocImpl = <::Driver as drm::Driver>::Object; /// GEM object functions, which must be implemented by drivers. pub trait DriverObject: Sync + Send + Sized + 'static { @@ -94,8 +92,8 @@ pub trait DriverObject: Sync + Send + Sized + 'static { type Args; /// Create a new driver data object for a GEM object of a given size. - fn new( - dev: &drm::Device, + fn new( + dev: &drm::Device, size: usize, args: Self::Args, ) -> impl PinInit; @@ -110,7 +108,7 @@ pub trait DriverObject: Sync + Send + Sized + 'static { } /// Trait that represents a GEM object subtype -pub trait IntoGEMObject: Sized + super::private::Sealed + AlwaysRefCounted { +pub trait IntoGEMObject: Sized + super::private::Sealed { /// Returns a reference to the raw `drm_gem_object` structure, which must be valid as long as /// this owning object is valid. fn as_raw(&self) -> *mut bindings::drm_gem_object; @@ -184,7 +182,7 @@ pub trait BaseObject: IntoGEMObject { fn create_handle(&self, file: &drm::File) -> Result where Self: AllocImpl, - D: drm::Driver = Self, File = F>, + D: drm::Driver, F: drm::file::DriverFile, { let mut handle: u32 = 0; @@ -198,8 +196,8 @@ pub trait BaseObject: IntoGEMObject { /// Looks up an object by its handle for a given `File`. fn lookup_handle(file: &drm::File, handle: u32) -> Result> where - Self: AllocImpl, - D: drm::Driver = Self, File = F>, + Self: AllocImpl + AlwaysRefCounted, + D: drm::Driver, F: drm::file::DriverFile, { // SAFETY: The arguments are all valid per the type invariants. @@ -281,12 +279,43 @@ impl Object { rss: None, }; + /// Returns the `Device` that owns this GEM object. + pub fn dev(&self) -> &drm::Device { + // 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 to + // Object::::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) } + } + + fn as_raw(&self) -> *mut bindings::drm_gem_object { + self.obj.get() + } + + extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) { + let ptr: *mut Opaque = obj.cast(); + + // SAFETY: All of our objects are of type `Object`. + let this = unsafe { crate::container_of!(ptr, Self, obj) }; + + // SAFETY: The C code only ever calls this callback with a valid pointer to a `struct + // drm_gem_object`. + unsafe { bindings::drm_gem_object_release(obj) }; + + // SAFETY: All of our objects are allocated via `KBox`, and we're in the + // free callback which guarantees this object has zero remaining references, + // so we can drop it. + let _ = unsafe { KBox::from_raw(this) }; + } +} + +impl Object { /// Create a new GEM object. - pub fn new( - dev: &drm::Device, - size: usize, - args: T::Args, - ) -> Result> { + pub fn new(dev: &drm::Device, size: usize, args: T::Args) -> Result> { let obj: Pin> = KBox::pin_init( try_pin_init!(Self { obj: Opaque::new(bindings::drm_gem_object::default()), @@ -322,46 +351,12 @@ impl Object { // SAFETY: We take over the initial reference count from `drm_gem_object_init()`. Ok(unsafe { ARef::from_raw(ptr) }) } - - /// Returns the `Device` that owns this GEM object. - pub fn dev(&self) -> &drm::Device { - // 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 to - // Object::::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) } - } - - fn as_raw(&self) -> *mut bindings::drm_gem_object { - self.obj.get() - } - - extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) { - let ptr: *mut Opaque = obj.cast(); - - // SAFETY: All of our objects are of type `Object`. - let this = unsafe { crate::container_of!(ptr, Self, obj) }; - - // SAFETY: The C code only ever calls this callback with a valid pointer to a `struct - // drm_gem_object`. - unsafe { bindings::drm_gem_object_release(obj) }; - - // SAFETY: All of our objects are allocated via `KBox`, and we're in the - // free callback which guarantees this object has zero remaining references, - // so we can drop it. - let _ = unsafe { KBox::from_raw(this) }; - } } impl_aref_for_gem_obj! { - impl for Object + impl for Object where - T: DriverObject, - C: DeviceContext + T: DriverObject } impl super::private::Sealed for Object {} diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index 5ffa1355ecf2..cf8410e0f228 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -73,17 +73,17 @@ use gem::{ /// /// 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 = Normal> { +pub struct ObjectConfig<'a, T: DriverObject> { /// Whether to set the write-combine map flag. pub map_wc: bool, /// Reuse the DMA reservation from another GEM object. /// /// The newly created [`Object`] will hold an owned refcount to `parent_resv_obj` if specified. - pub parent_resv_obj: Option<&'a Object>, + pub parent_resv_obj: Option<&'a Object>, } -impl<'a, T: DriverObject, C: DeviceContext> Default for ObjectConfig<'a, T, C> { +impl<'a, T: DriverObject> Default for ObjectConfig<'a, T> { #[inline(always)] fn default() -> Self { Self { @@ -106,7 +106,7 @@ pub struct Object { #[pin] obj: Opaque, /// Parent object that owns this object's DMA reservation object. - parent_resv_obj: Option>>, + parent_resv_obj: Option>>, /// Devres object for unmapping any SGTable on driver-unbind. sgt_res: ManuallyDrop>>>, #[pin] @@ -118,10 +118,9 @@ pub struct Object { } super::impl_aref_for_gem_obj! { - impl for Object + impl for Object where - T: DriverObject, - C: DeviceContext + T: DriverObject } // SAFETY: All GEM objects are thread-safe. @@ -157,54 +156,6 @@ impl Object { self.obj.get() } - /// Create a new shmem-backed DRM object of the given size. - /// - /// Additional config options can be specified using `config`. - pub fn new( - dev: &Device, - size: usize, - config: ObjectConfig<'_, T, C>, - args: T::Args, - ) -> Result> { - let new: Pin> = KBox::try_pin_init( - try_pin_init!(Self { - obj <- Opaque::init_zeroed(), - parent_resv_obj: config.parent_resv_obj.map(|p| p.into()), - sgt_res: ManuallyDrop::new(SetOnce::new()), - sgt_lock <- new_mutex!(()), - inner <- T::new(dev, size, args), - _ctx: PhantomData::, - }), - GFP_KERNEL, - )?; - - // SAFETY: `obj.as_raw()` is guaranteed to be valid by the initialization above. - unsafe { (*new.as_raw()).funcs = &Self::VTABLE }; - - // SAFETY: The arguments are all valid via the type invariants. - to_result(unsafe { bindings::drm_gem_shmem_init(dev.as_raw(), new.as_raw_shmem(), size) })?; - - // SAFETY: We never move out of `self`. - let new = KBox::into_raw(unsafe { Pin::into_inner_unchecked(new) }); - - // SAFETY: We're taking over the owned refcount from `drm_gem_shmem_init`. - let obj = unsafe { ARef::from_raw(NonNull::new_unchecked(new)) }; - - // Start filling out values from `config` - if let Some(parent_resv) = config.parent_resv_obj { - // SAFETY: We have yet to expose the new gem object outside of this function, so it is - // safe to modify this field. - unsafe { (*obj.obj.get()).base.resv = parent_resv.raw_dma_resv() }; - } - - // SAFETY: We have yet to expose this object outside of this function, so we're guaranteed - // to have exclusive access - thus making this safe to hold a mutable reference to. - let shmem = unsafe { &mut *obj.as_raw_shmem() }; - shmem.set_map_wc(config.map_wc); - - Ok(obj) - } - /// Returns the `Device` that owns this GEM object. pub fn dev(&self) -> &Device { // SAFETY: `dev` will have been initialized in `Self::new()` by `drm_gem_shmem_init()`. @@ -308,12 +259,6 @@ impl Object { self.make_vmap() } - /// Creates and returns an owned reference to a virtual kernel memory mapping for this object. - #[inline] - pub fn owned_vmap(&self) -> Result> { - self.make_vmap() - } - /// Creates (if necessary) and returns an immutable reference to a scatter-gather table of DMA /// pages for this object. /// @@ -355,6 +300,62 @@ impl Object { } } +impl Object { + /// Create a new shmem-backed DRM object of the given size. + /// + /// Additional config options can be specified using `config`. + pub fn new( + dev: &Device, + size: usize, + config: ObjectConfig<'_, T>, + args: T::Args, + ) -> Result> { + let new: Pin> = KBox::try_pin_init( + try_pin_init!(Self { + obj <- Opaque::init_zeroed(), + parent_resv_obj: config.parent_resv_obj.map(|p| p.into()), + sgt_res: ManuallyDrop::new(SetOnce::new()), + sgt_lock <- new_mutex!(()), + inner <- T::new(dev, size, args), + _ctx: PhantomData, + }), + GFP_KERNEL, + )?; + + // SAFETY: `obj.as_raw()` is guaranteed to be valid by the initialization above. + unsafe { (*new.as_raw()).funcs = &Self::VTABLE }; + + // SAFETY: The arguments are all valid via the type invariants. + to_result(unsafe { bindings::drm_gem_shmem_init(dev.as_raw(), new.as_raw_shmem(), size) })?; + + // SAFETY: We never move out of `self`. + let new = KBox::into_raw(unsafe { Pin::into_inner_unchecked(new) }); + + // SAFETY: We're taking over the owned refcount from `drm_gem_shmem_init`. + let obj = unsafe { ARef::from_raw(NonNull::new_unchecked(new)) }; + + // Start filling out values from `config` + if let Some(parent_resv) = config.parent_resv_obj { + // SAFETY: We have yet to expose the new gem object outside of this function, so it is + // safe to modify this field. + unsafe { (*obj.obj.get()).base.resv = parent_resv.raw_dma_resv() }; + } + + // SAFETY: We have yet to expose this object outside of this function, so we're guaranteed + // to have exclusive access - thus making this safe to hold a mutable reference to. + let shmem = unsafe { &mut *obj.as_raw_shmem() }; + shmem.set_map_wc(config.map_wc); + + Ok(obj) + } + + /// Creates and returns an owned reference to a virtual kernel memory mapping for this object. + #[inline] + pub fn owned_vmap(&self) -> Result> { + self.make_vmap() + } +} + impl Deref for Object { type Target = T; @@ -670,8 +671,8 @@ mod tests { type Driver = KunitDriver; type Args = (); - fn new( - _dev: &drm::Device, + fn new( + _dev: &drm::Device, _size: usize, _args: Self::Args, ) -> impl PinInit { @@ -683,7 +684,7 @@ mod tests { impl drm::Driver for KunitDriver { type Data = KunitData; type File = KunitFile; - type Object = Object; + type Object = Object; type ParentDevice = faux::Device; const INFO: drm::DriverInfo = INFO; -- cgit v1.2.3 From 7f994b8912eba190ff58e9c8a378d02d13c9eb8a Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:28 +0200 Subject: rust: drm/gem: remove DeviceContext from shmem::Object Now that AlwaysRefCounted is restricted to the Normal GEM Object context, there is no use for instantiating Object with a non-Normal context. Remove the DeviceContext generic parameter from shmem::Object and all associated types (VMap, VMapRef, VMapOwned, DmaResvGuard, SGTableMap), simplifying the API. Reviewed-by: Alexandre Courbot Reviewed-by: Lyude Paul Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-9-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/gem/shmem.rs | 121 ++++++++++++++++++------------------------- 1 file changed, 51 insertions(+), 70 deletions(-) diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index cf8410e0f228..e0ef47352e88 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -20,9 +20,7 @@ use crate::{ driver, gem, private::Sealed, - Device, - DeviceContext, - Normal, // + Device, // }, error::{ from_err_ptr, @@ -48,7 +46,6 @@ use crate::{ }; use core::{ ffi::c_void, - marker::PhantomData, mem::{ ManuallyDrop, MaybeUninit, // @@ -99,22 +96,20 @@ impl<'a, T: DriverObject> Default for ObjectConfig<'a, T> { /// /// - `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 { +pub struct Object { #[pin] obj: Opaque, /// Parent object that owns this object's DMA reservation object. parent_resv_obj: Option>>, /// Devres object for unmapping any SGTable on driver-unbind. - sgt_res: ManuallyDrop>>>, + sgt_res: ManuallyDrop>>>, #[pin] /// Lock for protecting initialization of `sgt_res`. sgt_lock: Mutex<()>, #[pin] inner: T, - _ctx: PhantomData, } super::impl_aref_for_gem_obj! { @@ -124,12 +119,12 @@ super::impl_aref_for_gem_obj! { } // SAFETY: All GEM objects are thread-safe. -unsafe impl Send for Object {} +unsafe impl Send for Object {} // SAFETY: All GEM objects are thread-safe. -unsafe impl Sync for Object {} +unsafe impl Sync for Object {} -impl Object { +impl Object { /// `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 @@ impl Object { } /// Returns the `Device` that owns this GEM object. - pub fn dev(&self) -> &Device { + pub fn dev(&self) -> &Device { // 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 @@ impl 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` + // - This function is set in AllocOps, so we know that `this` is contained within an + // `Object` 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 @@ impl 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> + fn make_vmap<'a, R, const SIZE: usize>(&'a self) -> Result> where R: Deref + From<&'a Self>, { @@ -255,7 +250,7 @@ impl Object { /// Creates and returns a virtual kernel memory mapping for this object. #[inline] - pub fn vmap(&self) -> Result> { + pub fn vmap(&self) -> Result> { self.make_vmap() } @@ -298,9 +293,7 @@ impl Object { Ok(sgt_res.access(dev)?) } -} -impl Object { /// Create a new shmem-backed DRM object of the given size. /// /// Additional config options can be specified using `config`. @@ -317,7 +310,6 @@ impl Object { sgt_res: ManuallyDrop::new(SetOnce::new()), sgt_lock <- new_mutex!(()), inner <- T::new(dev, size, args), - _ctx: PhantomData, }), GFP_KERNEL, )?; @@ -351,12 +343,12 @@ impl Object { /// Creates and returns an owned reference to a virtual kernel memory mapping for this object. #[inline] - pub fn owned_vmap(&self) -> Result> { + pub fn owned_vmap(&self) -> Result> { self.make_vmap() } } -impl Deref for Object { +impl Deref for Object { type Target = T; fn deref(&self) -> &Self::Target { @@ -364,15 +356,15 @@ impl Deref for Object { } } -impl DerefMut for Object { +impl DerefMut for Object { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.inner } } -impl Sealed for Object {} +impl Sealed for Object {} -impl gem::IntoGEMObject for Object { +impl gem::IntoGEMObject for Object { 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 @@ impl gem::IntoGEMObject for Object { } } -impl driver::AllocImpl for Object { +impl driver::AllocImpl for Object { type Driver = T::Driver; const ALLOC_OPS: driver::AllocOps = driver::AllocOps { @@ -410,14 +402,11 @@ impl driver::AllocImpl for Object { /// 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, - NotThreadSafe, -); +struct DmaResvGuard<'a, T: DriverObject>(&'a Object, NotThreadSafe); -impl<'a, T: DriverObject, C: DeviceContext> DmaResvGuard<'a, T, C> { +impl<'a, T: DriverObject> DmaResvGuard<'a, T> { #[inline] - fn new(obj: &'a Object) -> Self { + fn new(obj: &'a Object) -> 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 @@ impl<'a, T: DriverObject, C: DeviceContext> DmaResvGuard<'a, T, C> { } } -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 @@ impl<'a, T: DriverObject, C: DeviceContext> Drop for DmaResvGuard<'a, T, C> { /// /// - The size of `owner` is >= SIZE. /// - The memory pointed to by `addr` remains valid at least until this object is dropped. -pub struct VMap +pub struct VMap where D: DriverObject, - C: DeviceContext, - R: Deref>, + R: Deref>, { 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, C, SIZE>; +pub type VMapRef<'a, D, const SIZE: usize = 0> = VMap, SIZE>; /// An alias type for an owned reference to a shmem-based GEM object's VMap. -pub type VMapOwned = VMap>, C, SIZE>; +pub type VMapOwned = VMap>, SIZE>; -impl VMap +impl VMap where D: DriverObject, - C: DeviceContext, - R: Deref>, + R: Deref>, { /// Borrows a reference to the object that owns this virtual mapping. #[inline] - pub fn owner(&self) -> &Object { + pub fn owner(&self) -> &Object { &self.owner } } -impl Drop for VMap +impl Drop for VMap where D: DriverObject, - C: DeviceContext, - R: Deref>, + R: Deref>, { #[inline] fn drop(&mut self) { @@ -491,29 +477,26 @@ where // 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 Send for VMap +unsafe impl Send for VMap where D: DriverObject, - C: DeviceContext, - R: Deref> + Send, + R: Deref> + 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 Sync for VMap +unsafe impl Sync for VMap where D: DriverObject, - C: DeviceContext, - R: Deref> + Sync, + R: Deref> + Sync, { } -impl Io for VMap +impl Io for VMap where D: DriverObject, - C: DeviceContext, - R: Deref>, + R: Deref>, { #[inline] fn addr(&self) -> usize { @@ -526,22 +509,20 @@ where } } -impl IoKnownSize for VMap +impl IoKnownSize for VMap where D: DriverObject, - C: DeviceContext, - R: Deref>, + R: Deref>, { const MIN_SIZE: usize = SIZE; } macro_rules! impl_vmap_io_capable { ($ty:ty) => { - impl IoCapable<$ty> for VMap + impl IoCapable<$ty> for VMap where D: DriverObject, - C: DeviceContext, - R: Deref>, + R: Deref>, { #[inline] unsafe fn io_read(&self, address: usize) -> $ty { @@ -584,11 +565,11 @@ impl_vmap_io_capable!(u64); /// [`SGTable`]. /// /// [`SGTable`]: scatterlist::SGTable -pub struct SGTableMap { - obj: NonNull>, +pub struct SGTableMap { + obj: NonNull>, } -impl Deref for SGTableMap { +impl Deref for SGTableMap { type Target = scatterlist::SGTable; fn deref(&self) -> &Self::Target { @@ -599,7 +580,7 @@ impl Deref for SGTableMap { } } -impl Drop for SGTableMap { +impl Drop for SGTableMap { fn drop(&mut self) { // SAFETY: `obj` is always valid via our type invariants let obj = unsafe { self.obj.as_ref() }; @@ -610,8 +591,8 @@ impl Drop for SGTableMap { } } -impl SGTableMap { - fn new(obj: &Object) -> impl Init { +impl SGTableMap { + fn new(obj: &Object) -> impl Init { // 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 @@ impl SGTableMap { // 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 Send for SGTableMap {} +unsafe impl Send for SGTableMap {} // 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 Sync for SGTableMap {} +unsafe impl Sync for SGTableMap {} #[kunit_tests(rust_drm_gem_shmem)] mod tests { @@ -705,7 +686,7 @@ mod tests { fn compile_time_vmap_sizes() -> Result { let (_dev, drm) = create_drm_dev()?; - let obj = Object::::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; + let obj = Object::::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; // Try creating a normal vmap obj.vmap::()?; @@ -729,7 +710,7 @@ mod tests { fn vmap_io() -> Result { let (_dev, drm) = create_drm_dev()?; - let obj = Object::::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; + let obj = Object::::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; let vmap = obj.vmap::()?; @@ -761,7 +742,7 @@ mod tests { let reg = faux::Registration::new(c"EvilKunit", None)?; let wrong_dev = reg.as_ref(); - let obj = Object::::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; + let obj = Object::::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; assert_eq!(obj.sg_table(wrong_dev.as_ref()).err().unwrap(), EINVAL); -- cgit v1.2.3 From 1c8a1f88ac32a987643bf2d534ef4aa2d3da3aeb Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:29 +0200 Subject: rust: drm: split Deref for Device context typestates Split the Deref implementation for drm::Device by context: - Device (Normal) dereferences to T::Data. - Device dereferences to Device (Normal). Reviewed-by: Lyude Paul Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-10-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/device.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index eb8146ea1c98..f5342de190f4 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -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` dereferences to `Device` ([`Normal`]), so any method available on a +/// [`Normal`] device is also available on a [`Registered`] one. pub trait DeviceContext: Sealed + Send + Sync + 'static {} /// The general-purpose, reference-counted [`DeviceContext`]. @@ -320,7 +323,7 @@ impl Device { } } -impl Deref for Device { +impl Deref for Device { type Target = T::Data; fn deref(&self) -> &Self::Target { @@ -328,6 +331,17 @@ impl Deref for Device { } } +impl Deref for Device { + type Target = Device; + + #[inline] + fn deref(&self) -> &Self::Target { + // SAFETY: The caller holds a `Device`, 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 AlwaysRefCounted for Device { -- cgit v1.2.3 From 499eb35cd4776b7c1ac8c6bb2ea6e693b7519a11 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:30 +0200 Subject: rust: drm: pin ioctl Device reference to Normal context Explicitly annotate the Device reference produced by from_raw() in the ioctl dispatch macro as Device<_, Normal>. Without this annotation, the context is inferred from the handler's first parameter type, which would allow a handler declaring &Device to obtain a Registered reference without runtime proof via RegistrationGuard. Reviewed-by: Lyude Paul Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-11-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/ioctl.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs index ccf4150d83b6..6f5a9877bdae 100644 --- a/rust/kernel/drm/ioctl.rs +++ b/rust/kernel/drm/ioctl.rs @@ -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 -- cgit v1.2.3 From 86b20b11505dcdfa5dfe108ac57220b8e3ab9d6d Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:31 +0200 Subject: rust: drm: add Ioctl device context typestate Add the Ioctl DeviceContext for DRM devices that have been registered with userspace previously. A Device has been registered at some point, but may be concurrently unregistering or already unregistered. drm_dev_enter() can guard against this, ensuring the device remains registered for the duration of the critical section. This typestate will be used in ioctl dispatch context where registration is guaranteed by the DRM core, and RegistrationGuard can safely be acquired. Reviewed-by: Lyude Paul Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-12-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/device.rs | 34 +++++++++++++++++++++++++++++++--- rust/kernel/drm/mod.rs | 1 + 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index f5342de190f4..42a068421c27 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -74,14 +74,16 @@ macro_rules! drm_legacy_fields { /// A trait implemented by all possible contexts a [`Device`] can be used in. /// -/// A [`Device`] can be in one of two contexts: +/// A [`Device`] can be in one of the following contexts: /// /// - [`Normal`]: The general-purpose, reference-counted context. A [`Device`] in this context may /// or may not be registered with userspace. +/// - [`Ioctl`]: The device has been registered with userspace at some point; used in ioctl +/// dispatch context. /// - [`Registered`]: The device has been registered with userspace at some point. /// -/// `Device` dereferences to `Device` ([`Normal`]), so any method available on a -/// [`Normal`] device is also available on a [`Registered`] one. +/// Both `Device` and `Device` dereference to `Device` ([`Normal`]), +/// so any method available on a [`Normal`] device is also available in the other contexts. pub trait DeviceContext: Sealed + Send + Sync + 'static {} /// The general-purpose, reference-counted [`DeviceContext`]. @@ -113,6 +115,21 @@ pub struct Registered; 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 @@ impl Deref for Device { } } +impl Deref for Device { + type Target = Device; + + #[inline] + fn deref(&self) -> &Self::Target { + // SAFETY: The caller holds a `Device`, 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 AlwaysRefCounted for Device { diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs index e5bfaf130342..a6693d2b84b8 100644 --- a/rust/kernel/drm/mod.rs +++ b/rust/kernel/drm/mod.rs @@ -11,6 +11,7 @@ pub mod ioctl; 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; -- cgit v1.2.3 From 2455d5f2d5e879b42fe6ab3207b7e3f4b9c78383 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:32 +0200 Subject: rust: drm: Add RegistrationGuard for drm_dev_enter/exit critical sections DRM ioctls do not guarantee that the parent bus device is still bound. However, since DRM device registration is managed through Devres, using drm_dev_unplug() on unregistration ensures that between drm_dev_enter() and drm_dev_exit() the parent device must be bound. Add RegistrationGuard, a guard object representing a drm_dev_enter/exit SRCU critical section that dereferences to &Device. The guard is obtained from Device and proves at runtime that the device is still registered. Switch Registration::drop from drm_dev_unregister() to drm_dev_unplug() to provide the SRCU barrier that RegistrationGuard's safety argument relies on. Reviewed-by: Lyude Paul Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-13-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/device.rs | 85 +++++++++++++++++++++++++++++++++++++++++------ rust/kernel/drm/driver.rs | 10 ++++-- rust/kernel/drm/mod.rs | 1 + 3 files changed, 83 insertions(+), 13 deletions(-) diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 42a068421c27..97e2b3de78bc 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -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` and `Device` dereference to `Device` ([`Normal`]), /// so any method available on a [`Normal`] device is also available in the other contexts. @@ -98,18 +99,15 @@ pub struct Normal; 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`. pub struct Registered; impl Sealed for Registered {} @@ -260,8 +258,8 @@ impl UnregisteredDevice { /// 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 @@ impl Device { } } +impl Device { + /// 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> { + 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`], 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, + idx: i32, + _not_send: NotThreadSafe, +} + +impl Deref for RegistrationGuard<'_, T> { + type Target = Device; + + #[inline] + fn deref(&self) -> &Self::Target { + self.dev + } +} + +impl 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 Deref for Device { type Target = T::Data; diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index 5152a18a8312..3cda8dceb498 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -199,8 +199,14 @@ unsafe impl Send for Registration {} impl Drop for Registration { 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>`. The existence of this - // `Registration` also guarantees the this `drm::Device` is actually registered. - unsafe { bindings::drm_dev_unregister(self.0.as_raw()) }; + // `Registration` also guarantees that this `drm::Device` is actually registered. + unsafe { bindings::drm_dev_unplug(self.0.as_raw()) }; } } diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs index a6693d2b84b8..fd6ed35bc35a 100644 --- a/rust/kernel/drm/mod.rs +++ b/rust/kernel/drm/mod.rs @@ -14,6 +14,7 @@ pub use self::device::DeviceContext; 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; -- cgit v1.2.3 From 478da53e5b682f0c39a4c6c3eeb09c0131342e8a Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:33 +0200 Subject: rust: drm: Wrap ioctl dispatch in RegistrationGuard Make Ioctl handlers receive a &Device reference, proving at the type level that the device is registered and its parent bus device is bound. This is achieved by calling registration_guard() on the Device obtained in ioctl dispatch context. If the device has been unplugged, the ioctl returns -ENODEV without calling the handler. To resolve the driver type parameter T for type inference, which the compiler cannot propagate through method resolution and associated-type projections alone, a dead-code closure and a helper function are used as a type-inference anchor. Reviewed-by: Lyude Paul Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-14-dakr@kernel.org [ Use imperative mood in commit message; clarify __dev_ctx_cast() doc comment to reflect Ioctl-to-Registered cast. - Danilo ] Signed-off-by: Danilo Krummrich --- drivers/gpu/drm/nova/file.rs | 12 ++++++++---- drivers/gpu/drm/tyr/file.rs | 7 +++++-- rust/kernel/drm/ioctl.rs | 45 +++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 55 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs index a3b7bd36792c..19fb89b28984 100644 --- a/drivers/gpu/drm/nova/file.rs +++ b/drivers/gpu/drm/nova/file.rs @@ -4,7 +4,11 @@ use crate::driver::{NovaDevice, NovaDriver}; use crate::gem::NovaObject; use kernel::{ alloc::flags::*, - drm::{self, gem::BaseObject}, + drm::{ + self, + gem::BaseObject, + Registered, // + }, pci, prelude::*, uapi, @@ -23,7 +27,7 @@ impl drm::file::DriverFile for File { impl File { /// IOCTL: get_param: Query GPU / driver metadata. pub(crate) fn get_param( - dev: &NovaDevice, + dev: &NovaDevice, getparam: &mut uapi::drm_nova_getparam, _file: &drm::File, ) -> Result { @@ -43,7 +47,7 @@ impl File { /// IOCTL: gem_create: Create a new DRM GEM object. pub(crate) fn gem_create( - dev: &NovaDevice, + dev: &NovaDevice, req: &mut uapi::drm_nova_gem_create, file: &drm::File, ) -> Result { @@ -56,7 +60,7 @@ impl File { /// IOCTL: gem_info: Query GEM metadata. pub(crate) fn gem_info( - _dev: &NovaDevice, + _dev: &NovaDevice, req: &mut uapi::drm_nova_gem_info, file: &drm::File, ) -> Result { diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs index 31411da203c5..fb9233eae01c 100644 --- a/drivers/gpu/drm/tyr/file.rs +++ b/drivers/gpu/drm/tyr/file.rs @@ -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 @@ impl drm::file::DriverFile for TyrDrmFileData { impl TyrDrmFileData { pub(crate) fn dev_query( - ddev: &TyrDrmDevice, + ddev: &TyrDrmDevice, devquery: &mut uapi::drm_panthor_dev_query, _file: &TyrDrmFile, ) -> Result { diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs index 6f5a9877bdae..c70ad5e2e5a1 100644 --- a/rust/kernel/drm/ioctl.rs +++ b/rust/kernel/drm/ioctl.rs @@ -70,6 +70,18 @@ pub mod internal { pub use bindings::drm_device; pub use bindings::drm_file; pub use bindings::drm_ioctl_desc; + + /// Cast an [`Ioctl`] DRM device pointer to [`Registered`], preserving the driver type + /// parameter `T`. + /// + /// Used by [`declare_drm_ioctls!`] to anchor type inference. + #[doc(hidden)] + #[inline] + pub const fn __dev_ctx_cast( + ptr: *const crate::drm::Device, + ) -> *const crate::drm::Device { + ptr.cast() + } } /// Declare the DRM ioctls for a driver. @@ -82,7 +94,7 @@ pub mod internal { /// `user_callback` should have the following prototype: /// /// ```ignore -/// fn foo(device: &kernel::drm::Device, +/// fn foo(device: &kernel::drm::Device, /// data: &mut uapi::argument_type, /// file: &kernel::drm::File, /// ) -> Result @@ -131,17 +143,44 @@ macro_rules! declare_drm_ioctls { // - The DRM device must have been registered when we're called through // an IOCTL. // + // INVARIANT: The `Ioctl` context requires that the device has been + // registered via `drm_dev_register()` at some point; the DRM core + // guarantees this for ioctl dispatch callbacks. + // // 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<_, $crate::drm::Normal> = + let dev: &$crate::drm::device::Device<_, $crate::drm::Ioctl> = $crate::drm::device::Device::from_raw(raw_dev); + // Type-inference anchor: the closure is never called but ties `dev`'s + // type to `$func`'s first parameter, which the compiler cannot infer + // through method resolution and associated-type projections alone. + #[allow(unreachable_code)] + let _ = || { + let __ptr = $crate::drm::ioctl::internal::__dev_ctx_cast( + ::core::ptr::from_ref(dev), + ); + + $func( + // SAFETY: This closure is never executed; the dereference + // exists purely to unify the type parameter with `$func`. + // The pointer is valid regardless. + unsafe { &*__ptr }, + unreachable!(), + unreachable!(), + ) + }; + // 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; + let Some(guard) = dev.registration_guard() else { + return $crate::error::code::ENODEV.to_errno(); + }; + // 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. @@ -154,7 +193,7 @@ macro_rules! declare_drm_ioctls { // SAFETY: This is just the DRM file structure let file = unsafe { $crate::drm::File::from_raw(raw_file) }; - match $func(dev, data, file) { + match $func(&*guard, data, file) { Err(e) => e.to_errno(), Ok(i) => i.try_into() .unwrap_or($crate::error::code::ERANGE.to_errno()), -- cgit v1.2.3 From 47f600d40bc6bf7bcc1f28f8c7fa3e3a7aa445eb Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:34 +0200 Subject: rust: drm: return ParentDevice from Device AsRef Change AsRef for drm::Device to return &T::ParentDevice instead of &device::Device, and restrict it to the Normal context. Device still gets this through Deref coercion. This provides access to the typed parent bus device rather than the raw base device. Reviewed-by: Lyude Paul Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-15-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/device.rs | 10 +++++++--- rust/kernel/drm/driver.rs | 3 ++- rust/kernel/drm/gem/shmem.rs | 3 ++- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 97e2b3de78bc..d4521ef8ed80 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -450,11 +450,15 @@ unsafe impl AlwaysRefCounted for Device { } } -impl AsRef for Device { - fn as_ref(&self) -> &device::Device { +impl AsRef> for Device { + fn as_ref(&self) -> &T::ParentDevice { // SAFETY: `bindings::drm_device::dev` is valid as long as the DRM device itself is valid, // which is guaranteed by the type invariant. - unsafe { device::Device::from_raw((*self.as_raw()).dev) } + let dev = unsafe { device::Device::from_raw((*self.as_raw()).dev) }; + + // SAFETY: The DRM device was constructed in `UnregisteredDevice::new()` with a parent + // device of type `T::ParentDevice`, hence `dev` is contained in a `T::ParentDevice`. + unsafe { device::AsBusDevice::from_device(dev) } } } diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index 3cda8dceb498..9ba2eba84191 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -170,7 +170,8 @@ impl Registration { where T: 'static, { - if drm.as_ref().as_raw() != dev.as_raw() { + let parent = drm.as_ref(); + if parent.as_ref().as_raw() != dev.as_raw() { return Err(EINVAL); } diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index e0ef47352e88..c1d82a04878b 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -264,7 +264,8 @@ impl Object { &'a self, dev: &'a device::Device, ) -> Result<&'a scatterlist::SGTable> { - if dev.as_raw() != self.dev().as_ref().as_raw() { + let parent = self.dev().as_ref(); + if dev.as_raw() != parent.as_ref().as_raw() { return Err(EINVAL); } -- cgit v1.2.3 From 453197b7cc320c5a7fc289bafff028bf6c550ceb Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:35 +0200 Subject: rust: drm: add AsRef> for Device Implement AsRef> for Device, providing access to the bound parent bus device for registered DRM devices. Since a Device guarantees that the parent bus device is bound, the conversion to T::ParentDevice is safe. Reviewed-by: Lyude Paul Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-16-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/device.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index d4521ef8ed80..fb3724c09f27 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -462,6 +462,20 @@ impl AsRef> for Device { } } +impl AsRef> for Device { + #[inline] + fn as_ref(&self) -> &T::ParentDevice { + let dev = (**self).as_ref().as_ref(); + + // SAFETY: A `Device` guarantees that the parent device is bound. + let dev = unsafe { dev.as_bound() }; + + // SAFETY: The DRM device was constructed in `UnregisteredDevice::new()` with a parent + // device of type `T::ParentDevice`, hence `dev` is contained in a `T::ParentDevice`. + unsafe { device::AsBusDevice::from_device(dev) } + } +} + // SAFETY: A `drm::Device` can be released from any thread. unsafe impl Send for Device {} -- cgit v1.2.3 From eb197f7d60f00d0f5b1b3505dfc86a7e36045a3e Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:36 +0200 Subject: drm: fix race between partial drm_dev_register() failure and ioctl If drm_dev_register() fails after registering a minor (e.g. render minor registered, primary minor fails), userspace could have opened the first minor and entered a drm_dev_enter() critical section. Since the unplugged flag was never set, the ioctl proceeds while the error path tears down device resources. Fix this by introducing drm_dev_synchronize_unplug(), which sets the unplugged flag and waits for the SRCU barrier, ensuring all in-flight drm_dev_enter() critical sections complete before cleanup proceeds; call it on the error path of drm_dev_register(). Fixes: bee330f3d672 ("drm: Use srcu to protect drm_device.unplugged") Cc: stable@vger.kernel.org Reported-by: sashiko-bot@kernel.org Closes: https://lore.kernel.org/all/20260620190648.2E9F61F000E9@smtp.kernel.org/ Reviewed-by: Alexandre Courbot Reviewed-by: Lyude Paul Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-17-dakr@kernel.org Signed-off-by: Danilo Krummrich --- drivers/gpu/drm/drm_drv.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 675675480da4..e890052061f3 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -473,6 +473,22 @@ void drm_dev_exit(int idx) } EXPORT_SYMBOL(drm_dev_exit); +/* + * Mark the device as unplugged and wait for any in-flight drm_dev_enter() + * critical sections to complete. + */ +static void drm_dev_synchronize_unplug(struct drm_device *dev) +{ + /* + * After synchronizing any critical read section is guaranteed to see + * the new value of ->unplugged, and any critical section which might + * still have seen the old value of ->unplugged is guaranteed to have + * finished. + */ + dev->unplugged = true; + synchronize_srcu(&drm_unplug_srcu); +} + /** * drm_dev_unplug - unplug a DRM device * @dev: DRM device @@ -485,15 +501,7 @@ EXPORT_SYMBOL(drm_dev_exit); */ void drm_dev_unplug(struct drm_device *dev) { - /* - * After synchronizing any critical read section is guaranteed to see - * the new value of ->unplugged, and any critical section which might - * still have seen the old value of ->unplugged is guaranteed to have - * finished. - */ - dev->unplugged = true; - synchronize_srcu(&drm_unplug_srcu); - + drm_dev_synchronize_unplug(dev); drm_dev_unregister(dev); /* Clear all CPU mappings pointing to this device */ @@ -1091,6 +1099,7 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) goto err_minors; dev->registered = true; + dev->unplugged = false; if (driver->load) { ret = driver->load(dev, flags); @@ -1118,6 +1127,13 @@ err_unload: if (dev->driver->unload) dev->driver->unload(dev); err_minors: + /* + * If a minor was registered before the failure, userspace could have + * opened it and entered a drm_dev_enter() critical section. Ensure all + * such sections complete before we clean up. + */ + drm_dev_synchronize_unplug(dev); + remove_compat_control_link(dev); drm_minor_unregister(dev, DRM_MINOR_ACCEL); drm_minor_unregister(dev, DRM_MINOR_PRIMARY); -- cgit v1.2.3 From e15b88223dc1becdc8c0d3d88795c5eb06518347 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:37 +0200 Subject: rust: drm: Add RegistrationData to drm::Driver Add a RegistrationData GAT (Generic Associated Type) to drm::Driver. The lifetime parameter is tied to the parent bus device binding scope. Registration<'a, T> takes ownership of the data via Pin>, storing it with its real lifetime. The pointer is written to drm::Device before drm_dev_register() to ensure it is already in place when ioctls arrive. Device::registration_data_with() provides access with the lifetime shortened from 'static via a pointer cast. Since Registration::drop() calls drm_dev_unplug(), which performs an SRCU barrier waiting for all drm_dev_enter() critical sections to complete, the data is guaranteed to remain valid for the duration of any RegistrationGuard. Reviewed-by: Lyude Paul Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-18-dakr@kernel.org [ Move registration_data_unchecked() to Device impl block. - Danilo ] Signed-off-by: Danilo Krummrich --- drivers/gpu/drm/nova/driver.rs | 17 +++++--- drivers/gpu/drm/tyr/driver.rs | 15 ++++--- rust/kernel/drm/device.rs | 44 +++++++++++++++++++ rust/kernel/drm/driver.rs | 95 ++++++++++++++++++++++++------------------ rust/kernel/drm/gem/shmem.rs | 1 + 5 files changed, 121 insertions(+), 51 deletions(-) diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs index e3c54303d70e..bd2a55405db8 100644 --- a/drivers/gpu/drm/nova/driver.rs +++ b/drivers/gpu/drm/nova/driver.rs @@ -20,9 +20,10 @@ use crate::gem::NovaObject; pub(crate) struct NovaDriver; -pub(crate) struct Nova { +pub(crate) struct Nova<'bound> { #[expect(unused)] drm: ARef>, + _reg: drm::Registration<'bound, NovaDriver>, } /// Convienence type alias for the DRM device type for this driver @@ -56,7 +57,7 @@ kernel::auxiliary_device_table!( impl auxiliary::Driver for NovaDriver { type IdInfo = (); - type Data<'bound> = Nova; + type Data<'bound> = Nova<'bound>; const ID_TABLE: auxiliary::IdTable = &AUX_TABLE; fn probe<'bound>( @@ -66,15 +67,21 @@ impl auxiliary::Driver for NovaDriver { let data = try_pin_init!(NovaData { adev: adev.into() }); let drm = drm::UnregisteredDevice::::new(adev, data)?; - let drm = drm::Registration::new_foreign_owned(drm, adev.as_ref(), 0)?; - - Ok(Nova { drm: drm.into() }) + // SAFETY: `reg` is stored in `Nova` and dropped when the driver is unbound; it is + // never forgotten. + let reg = unsafe { drm::Registration::new(adev.as_ref(), drm, (), 0)? }; + + Ok(Nova { + drm: reg.device().into(), + _reg: reg, + }) } } #[vtable] impl drm::Driver for NovaDriver { type Data = NovaData; + type RegistrationData<'a> = (); type File = File; type Object = gem::Object; type ParentDevice = auxiliary::Device; diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs index 7f082de6d6dc..8348c6cd3929 100644 --- a/drivers/gpu/drm/tyr/driver.rs +++ b/drivers/gpu/drm/tyr/driver.rs @@ -52,8 +52,9 @@ pub(crate) type TyrDrmDevice = drm::Device pub(crate) struct TyrPlatformDriver; #[pin_data(PinnedDrop)] -pub(crate) struct TyrPlatformDriverData { +pub(crate) struct TyrPlatformDriverData<'bound> { _device: ARef, + _reg: drm::Registration<'bound, TyrDrmDriver>, } #[pin_data] @@ -98,7 +99,7 @@ kernel::of_device_table!( impl platform::Driver for TyrPlatformDriver { type IdInfo = (); - type Data<'bound> = TyrPlatformDriverData; + type Data<'bound> = TyrPlatformDriverData<'bound>; const OF_ID_TABLE: Option> = Some(&OF_TABLE); fn probe<'bound>( @@ -150,10 +151,13 @@ impl platform::Driver for TyrPlatformDriver { }); let tdev = drm::UnregisteredDevice::::new(pdev, data)?; - let tdev = drm::driver::Registration::new_foreign_owned(tdev, pdev.as_ref(), 0)?; + // SAFETY: `reg` is stored in `TyrPlatformDriverData` and dropped when the driver is + // unbound; it is never forgotten. + let reg = unsafe { drm::Registration::new(pdev.as_ref(), tdev, (), 0)? }; let driver = TyrPlatformDriverData { - _device: tdev.into(), + _device: reg.device().into(), + _reg: reg, }; // We need this to be dev_info!() because dev_dbg!() does not work at @@ -164,7 +168,7 @@ impl platform::Driver for TyrPlatformDriver { } #[pinned_drop] -impl PinnedDrop for TyrPlatformDriverData { +impl PinnedDrop for TyrPlatformDriverData<'_> { fn drop(self: Pin<&mut Self>) {} } @@ -181,6 +185,7 @@ const INFO: drm::DriverInfo = drm::DriverInfo { #[vtable] impl drm::Driver for TyrDrmDriver { type Data = TyrDrmDeviceData; + type RegistrationData<'a> = (); type File = TyrDrmFileData; type Object = drm::gem::shmem::Object; type ParentDevice = platform::Device; diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index fb3724c09f27..f43c6887ad23 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -32,6 +32,7 @@ use crate::{ }; use core::{ alloc::Layout, + cell::UnsafeCell, marker::PhantomData, mem, ops::Deref, @@ -247,6 +248,9 @@ impl UnregisteredDevice { // SAFETY: `drm_dev` is still private to this function. unsafe { (*drm_dev).driver = const { &Self::VTABLE } }; + // SAFETY: `raw_drm` is valid; no concurrent access before registration. + unsafe { (*raw_drm.as_ptr()).registration_data = UnsafeCell::new(NonNull::dangling()) }; + // 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`. @@ -270,6 +274,7 @@ impl UnregisteredDevice { pub struct Device { dev: Opaque, data: T::Data, + pub(super) registration_data: UnsafeCell>>, _ctx: PhantomData, } @@ -385,6 +390,45 @@ pub struct RegistrationGuard<'a, T: drm::Driver> { _not_send: NotThreadSafe, } +impl Device { + /// Returns a reference to the registration data with lifetime shortened from `'static`. + /// + /// # Safety + /// + /// The returned reference must not be exposed to code that can choose a concrete lifetime for + /// it, as that would be unsound for types that are invariant over their lifetime parameter + /// (e.g. it must be passed through an HRTB-bounded closure). + #[inline] + unsafe fn registration_data_unchecked(&self) -> &T::RegistrationData<'_> { + // SAFETY: + // - `Registered` guarantees the parent bus device is bound, hence the pointer is valid. + // - The pointer cast from `Of<'static>` to `Of<'_>` is layout-compatible since lifetimes + // are erased at runtime. + // - Caller guarantees the reference is only used behind an HRTB, making the lifetime + // shortening sound regardless of variance. + unsafe { (*self.registration_data.get()).cast::<_>().as_ref() } + } + + /// Access the registration data through a closure, with the lifetime tied to the closure + /// scope. + /// + /// The data is owned by [`Registration`](drm::Registration) and is guaranteed to remain valid + /// as long as the device is registered, since [`Registration`](drm::Registration)'s `drop` + /// calls `drm_dev_unplug()` which waits for all `drm_dev_enter()` critical sections to + /// complete. + #[inline] + pub fn registration_data_with(&self, f: F) -> R + where + F: for<'a> FnOnce(&'a T::RegistrationData<'a>) -> R, + { + // SAFETY: `Registered` guarantees the device is registered and the parent bus device is + // bound. The closure's HRTB `for<'a>` prevents the caller from smuggling in references + // with a concrete short lifetime, satisfying the lifetime requirement of + // `registration_data_unchecked`. + f(unsafe { self.registration_data_unchecked() }) + } +} + impl Deref for RegistrationGuard<'_, T> { type Target = Device; diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index 9ba2eba84191..08b2a318cf02 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -7,16 +7,12 @@ use crate::{ bindings, device, - devres, drm, error::to_result, prelude::*, sync::aref::ARef, // }; -use core::{ - mem, - ptr::NonNull, // -}; +use core::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; @@ -110,6 +106,14 @@ pub trait Driver { /// Context data associated with the DRM driver type Data: Sync + Send; + /// Data owned by the [`Registration`] and accessible within a + /// [`RegistrationGuard`](drm::RegistrationGuard) critical section via + /// [`Device::registration_data_with()`](drm::Device::registration_data_with). + /// + /// The lifetime parameter is tied to the [`Registration`] scope, which is enclosed in the + /// parent bus device binding scope but may be shorter. + type RegistrationData<'a>: Send + Sync + 'a; + /// The type used to manage memory for this driver. type Object: AllocImpl; @@ -139,66 +143,72 @@ pub trait Driver { /// The registration type of a `drm::Device`. /// /// Once the `Registration` structure is dropped, the device is unregistered. -pub struct Registration(ARef>); - -impl Registration { - fn new(drm: drm::UnregisteredDevice, flags: usize) -> Result { - // SAFETY: `drm.as_raw()` is valid by the invariants of `drm::Device`. - to_result(unsafe { bindings::drm_dev_register(drm.as_raw(), flags) })?; - - // 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)) - } +pub struct Registration<'a, T: Driver> { + drm: ARef>, + _reg_data: Pin>>, +} - /// Registers a new [`UnregisteredDevice`](drm::UnregisteredDevice) with userspace. +impl<'a, T: Driver> Registration<'a, T> { + /// Register a new [`UnregisteredDevice`](drm::UnregisteredDevice) with userspace. /// - /// Ownership of the [`Registration`] object is passed to [`devres::register`]. - pub fn new_foreign_owned<'a>( - drm: drm::UnregisteredDevice, + /// # Safety + /// + /// The caller must not `mem::forget()` the returned [`Registration`] or otherwise prevent its + /// [`Drop`] implementation from running, since the registration data may contain borrowed + /// references that become invalid after `'a` ends. + pub unsafe fn new( dev: &'a device::Device, + drm: drm::UnregisteredDevice, + reg_data: impl PinInit, E>, flags: usize, - ) -> Result<&'a drm::Device> + ) -> Result where - T: 'static, + Error: From, { let parent = drm.as_ref(); if parent.as_ref().as_raw() != dev.as_raw() { return Err(EINVAL); } - let reg = Registration::::new(drm, flags)?; - let drm = NonNull::from(reg.device()); + let reg_data: Pin>> = KBox::pin_init(reg_data, GFP_KERNEL)?; + + // Store the registration data pointer in the device before registration, so that it is + // visible once ioctls can be called. + let ptr: NonNull> = + NonNull::from(Pin::get_ref(reg_data.as_ref())).cast(); - devres::register(dev, reg, GFP_KERNEL)?; + // SAFETY: No concurrent access; the device is not yet registered. + unsafe { *drm.registration_data.get() = ptr }; + + // SAFETY: `drm` is a valid, initialized but not yet registered DRM device. + let ret = unsafe { bindings::drm_dev_register(drm.as_raw(), flags) }; + if let Err(e) = to_result(ret) { + // SAFETY: `drm_dev_register()` synchronizes SRCU on failure, so no concurrent + // access to `registration_data` is possible at this point. + unsafe { *drm.registration_data.get() = NonNull::dangling() }; + return Err(e); + } - // 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() }) + Ok(Self { + drm: (&*drm).into(), + _reg_data: reg_data, + }) } /// Returns a reference to the `Device` instance for this registration. pub fn device(&self) -> &drm::Device { - &self.0 + &self.drm } } // SAFETY: `Registration` doesn't offer any methods or access to fields when shared between // threads, hence it's safe to share it. -unsafe impl Sync for Registration {} +unsafe impl Sync for Registration<'_, T> {} // SAFETY: Registration with and unregistration from the DRM subsystem can happen from any thread. -unsafe impl Send for Registration {} +unsafe impl Send for Registration<'_, T> {} -impl Drop for Registration { +impl 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 @@ -208,6 +218,9 @@ impl Drop for Registration { // // SAFETY: Safe by the invariant of `ARef>`. The existence of this // `Registration` also guarantees that this `drm::Device` is actually registered. - unsafe { bindings::drm_dev_unplug(self.0.as_raw()) }; + unsafe { bindings::drm_dev_unplug(self.drm.as_raw()) }; + // After drm_dev_unplug(), the SRCU barrier guarantees that all RegistrationGuard critical + // sections have completed, so no one holds a reference to reg_data anymore. + // reg_data is dropped here automatically. } } diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index c1d82a04878b..60dca8871b87 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -665,6 +665,7 @@ mod tests { #[vtable] impl drm::Driver for KunitDriver { type Data = KunitData; + type RegistrationData<'a> = (); type File = KunitFile; type Object = Object; type ParentDevice = faux::Device; -- cgit v1.2.3 From 3ba210061c2960380007e6475d1c1119216f5c83 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:38 +0200 Subject: rust: drm: Pass registration data to ioctl handlers Pass registration data to ioctl handlers via drm::Device::registration_data_with(). The closure's HRTB ties the lifetime to the closure scope, and the pointer cast shortens it from 'static internally. The reference is valid for the duration of the drm_dev_enter/exit critical section held by RegistrationGuard. Reviewed-by: Lyude Paul Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-19-dakr@kernel.org Signed-off-by: Danilo Krummrich --- drivers/gpu/drm/nova/file.rs | 3 +++ drivers/gpu/drm/tyr/file.rs | 1 + rust/kernel/drm/ioctl.rs | 8 ++++++-- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs index 19fb89b28984..208be4e38188 100644 --- a/drivers/gpu/drm/nova/file.rs +++ b/drivers/gpu/drm/nova/file.rs @@ -28,6 +28,7 @@ impl File { /// IOCTL: get_param: Query GPU / driver metadata. pub(crate) fn get_param( dev: &NovaDevice, + _reg_data: &(), getparam: &mut uapi::drm_nova_getparam, _file: &drm::File, ) -> Result { @@ -48,6 +49,7 @@ impl File { /// IOCTL: gem_create: Create a new DRM GEM object. pub(crate) fn gem_create( dev: &NovaDevice, + _reg_data: &(), req: &mut uapi::drm_nova_gem_create, file: &drm::File, ) -> Result { @@ -61,6 +63,7 @@ impl File { /// IOCTL: gem_info: Query GEM metadata. pub(crate) fn gem_info( _dev: &NovaDevice, + _reg_data: &(), req: &mut uapi::drm_nova_gem_info, file: &drm::File, ) -> Result { diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs index fb9233eae01c..b686041d5d6b 100644 --- a/drivers/gpu/drm/tyr/file.rs +++ b/drivers/gpu/drm/tyr/file.rs @@ -32,6 +32,7 @@ impl drm::file::DriverFile for TyrDrmFileData { impl TyrDrmFileData { pub(crate) fn dev_query( ddev: &TyrDrmDevice, + _reg_data: &(), devquery: &mut uapi::drm_panthor_dev_query, _file: &TyrDrmFile, ) -> Result { diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs index c70ad5e2e5a1..64af9eacc306 100644 --- a/rust/kernel/drm/ioctl.rs +++ b/rust/kernel/drm/ioctl.rs @@ -95,6 +95,7 @@ pub mod internal { /// /// ```ignore /// fn foo(device: &kernel::drm::Device, +/// reg_data: &Self::RegistrationData<'_>, /// data: &mut uapi::argument_type, /// file: &kernel::drm::File, /// ) -> Result @@ -169,13 +170,14 @@ macro_rules! declare_drm_ioctls { unsafe { &*__ptr }, unreachable!(), unreachable!(), + unreachable!(), ) }; // 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; + let _: for<'a> fn(&'a _, &'a _, &'a mut _, &'a _) -> _ = $func; let Some(guard) = dev.registration_guard() else { return $crate::error::code::ENODEV.to_errno(); @@ -193,7 +195,9 @@ macro_rules! declare_drm_ioctls { // SAFETY: This is just the DRM file structure let file = unsafe { $crate::drm::File::from_raw(raw_file) }; - match $func(&*guard, data, file) { + match guard.registration_data_with(|reg_data| { + $func(&*guard, reg_data, data, file) + }) { Err(e) => e.to_errno(), Ok(i) => i.try_into() .unwrap_or($crate::error::code::ERANGE.to_errno()), -- cgit v1.2.3 From 354a8f8b098b29d7c6064a12958cb289421a09d1 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:39 +0200 Subject: drm: nova: Use drm::Device to access the parent bus device The get_param ioctl needs access to the parent auxiliary device. Since ioctl handlers run inside a RegistrationGuard, accept &NovaDevice to obtain &auxiliary::Device via as_ref() directly. This removes the need for drm::Device data, hence set it to (). Reviewed-by: Lyude Paul Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-20-dakr@kernel.org Signed-off-by: Danilo Krummrich --- drivers/gpu/drm/nova/driver.rs | 11 ++--------- drivers/gpu/drm/nova/file.rs | 7 ++++--- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs index bd2a55405db8..739690bc2db5 100644 --- a/drivers/gpu/drm/nova/driver.rs +++ b/drivers/gpu/drm/nova/driver.rs @@ -29,11 +29,6 @@ pub(crate) struct Nova<'bound> { /// Convienence type alias for the DRM device type for this driver pub(crate) type NovaDevice = drm::Device; -#[pin_data] -pub(crate) struct NovaData { - pub(crate) adev: ARef, -} - const INFO: drm::DriverInfo = drm::DriverInfo { major: 0, minor: 0, @@ -64,9 +59,7 @@ impl auxiliary::Driver for NovaDriver { adev: &'bound auxiliary::Device>, _info: &'bound Self::IdInfo, ) -> impl PinInit, Error> + 'bound { - let data = try_pin_init!(NovaData { adev: adev.into() }); - - let drm = drm::UnregisteredDevice::::new(adev, data)?; + let drm = drm::UnregisteredDevice::::new(adev, Ok(()))?; // SAFETY: `reg` is stored in `Nova` and dropped when the driver is unbound; it is // never forgotten. let reg = unsafe { drm::Registration::new(adev.as_ref(), drm, (), 0)? }; @@ -80,7 +73,7 @@ impl auxiliary::Driver for NovaDriver { #[vtable] impl drm::Driver for NovaDriver { - type Data = NovaData; + type Data = (); type RegistrationData<'a> = (); type File = File; type Object = gem::Object; diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs index 208be4e38188..298c02bacb4b 100644 --- a/drivers/gpu/drm/nova/file.rs +++ b/drivers/gpu/drm/nova/file.rs @@ -4,6 +4,8 @@ use crate::driver::{NovaDevice, NovaDriver}; use crate::gem::NovaObject; use kernel::{ alloc::flags::*, + auxiliary, + device::Bound, drm::{ self, gem::BaseObject, @@ -32,9 +34,8 @@ impl File { getparam: &mut uapi::drm_nova_getparam, _file: &drm::File, ) -> Result { - let adev = &dev.adev; - let parent = adev.parent(); - let pdev: &pci::Device = parent.try_into()?; + let adev: &auxiliary::Device = dev.as_ref(); + let pdev: &pci::Device = adev.parent().try_into()?; let value = match getparam.param as u32 { uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => pdev.resource_len(1)?, -- cgit v1.2.3