diff options
| author | Danilo Krummrich <dakr@kernel.org> | 2026-07-12 16:59:55 +0200 |
|---|---|---|
| committer | Danilo Krummrich <dakr@kernel.org> | 2026-07-12 16:59:55 +0200 |
| commit | aea0aeeff1502c8dfffc52fc1f3db706388c948c (patch) | |
| tree | e8fca5d59125aa6f7730c3c2119cfe1feb7e74af | |
| parent | d85845b64c0020b2812243a22fa79d57cc1c1e38 (diff) | |
| parent | 354a8f8b098b29d7c6064a12958cb289421a09d1 (diff) | |
| download | linux-aea0aeeff1502c8dfffc52fc1f3db706388c948c.tar.gz linux-aea0aeeff1502c8dfffc52fc1f3db706388c948c.zip | |
Merge patch series "rust: drm: Higher-Ranked Lifetime private data"
Danilo Krummrich <dakr@kernel.org> says:
DRM ioctls run in process context without any guarantee that the parent
bus device is still bound. This series solves the problem by introducing
RegistrationGuard -- a guard representing a drm_dev_enter/exit SRCU
critical section that proves the parent bus device is bound for the
lifetime of the guard.
As initial plumbing for this, the DRM DeviceContext typestates are
reworked: Uninit is renamed to Normal, defaults are adjusted,
AlwaysRefCounted is restricted to Normal, and a Deref chain from
Device<T, Registered> to Device<T, Normal> is established. This gives
Device<T, Registered> the semantic that the device is currently
registered and the parent bus device is bound, which makes the
RegistrationGuard and ioctl dispatch much cleaner. An Ioctl context
restricts registration_guard() to ioctl dispatch, where the DRM core
guarantees prior registration.
On top of that, add RegistrationData as a GAT (Generic Associated Type)
on drm::Driver, allowing drivers to store data whose lifetime is tied to
the parent bus device binding scope. The data is allocated in
Registration::new(), lifetime-erased to 'static for storage, and made
accessible through Device<T, Registered>::registration_data_with(). The
closure's HRTB ties the lifetime to the closure scope; internally the
'static pointer is cast back to the closure-scoped lifetime. The
reference is valid for the duration of the drm_dev_enter/exit critical
section held by RegistrationGuard.
Also update the ioctl dispatch macro to wrap every handler in a
RegistrationGuard, returning ENODEV if the device has been unplugged,
and pass the registration data to handlers.
Link: https://patch.msgid.link/20260628145406.2107056-1-dakr@kernel.org
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
| -rw-r--r-- | drivers/gpu/drm/drm_drv.c | 34 | ||||
| -rw-r--r-- | drivers/gpu/drm/nova/driver.rs | 38 | ||||
| -rw-r--r-- | drivers/gpu/drm/nova/file.rs | 22 | ||||
| -rw-r--r-- | drivers/gpu/drm/nova/gem.rs | 18 | ||||
| -rw-r--r-- | drivers/gpu/drm/tyr/driver.rs | 25 | ||||
| -rw-r--r-- | drivers/gpu/drm/tyr/file.rs | 8 | ||||
| -rw-r--r-- | drivers/gpu/drm/tyr/gem.rs | 11 | ||||
| -rw-r--r-- | rust/kernel/drm/device.rs | 289 | ||||
| -rw-r--r-- | rust/kernel/drm/driver.rs | 111 | ||||
| -rw-r--r-- | rust/kernel/drm/gem/mod.rs | 98 | ||||
| -rw-r--r-- | rust/kernel/drm/gem/shmem.rs | 257 | ||||
| -rw-r--r-- | rust/kernel/drm/ioctl.rs | 56 | ||||
| -rw-r--r-- | rust/kernel/drm/mod.rs | 4 | ||||
| -rw-r--r-- | rust/kernel/faux.rs | 69 | ||||
| -rw-r--r-- | samples/rust/rust_driver_faux.rs | 3 |
15 files changed, 656 insertions, 387 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); diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs index 48933d86ddda..739690bc2db5 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, @@ -17,18 +20,14 @@ use crate::gem::NovaObject; pub(crate) struct NovaDriver; -pub(crate) struct Nova { +pub(crate) struct Nova<'bound> { #[expect(unused)] drm: ARef<drm::Device<NovaDriver>>, + _reg: drm::Registration<'bound, NovaDriver>, } /// Convienence type alias for the DRM device type for this driver -pub(crate) type NovaDevice<Ctx = drm::Registered> = drm::Device<NovaDriver, Ctx>; - -#[pin_data] -pub(crate) struct NovaData { - pub(crate) adev: ARef<auxiliary::Device>, -} +pub(crate) type NovaDevice<Ctx = drm::Normal> = drm::Device<NovaDriver, Ctx>; const INFO: drm::DriverInfo = drm::DriverInfo { major: 0, @@ -53,27 +52,32 @@ 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<Self::IdInfo> = &AUX_TABLE; fn probe<'bound>( adev: &'bound auxiliary::Device<Core<'_>>, _info: &'bound Self::IdInfo, ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound { - let data = try_pin_init!(NovaData { adev: adev.into() }); - - let drm = drm::UnregisteredDevice::<Self>::new(adev.as_ref(), data)?; - let drm = drm::Registration::new_foreign_owned(drm, adev.as_ref(), 0)?; - - Ok(Nova { drm: drm.into() }) + let drm = drm::UnregisteredDevice::<Self>::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)? }; + + Ok(Nova { + drm: reg.device().into(), + _reg: reg, + }) } } #[vtable] impl drm::Driver for NovaDriver { - type Data = NovaData; + type Data = (); + type RegistrationData<'a> = (); type File = File; - type Object<Ctx: drm::DeviceContext> = gem::Object<NovaObject, Ctx>; + type Object = gem::Object<NovaObject>; + type ParentDevice<Ctx: DeviceContext> = auxiliary::Device<Ctx>; const INFO: drm::DriverInfo = INFO; diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs index a3b7bd36792c..298c02bacb4b 100644 --- a/drivers/gpu/drm/nova/file.rs +++ b/drivers/gpu/drm/nova/file.rs @@ -4,7 +4,13 @@ use crate::driver::{NovaDevice, NovaDriver}; use crate::gem::NovaObject; use kernel::{ alloc::flags::*, - drm::{self, gem::BaseObject}, + auxiliary, + device::Bound, + drm::{ + self, + gem::BaseObject, + Registered, // + }, pci, prelude::*, uapi, @@ -23,13 +29,13 @@ impl drm::file::DriverFile for File { impl File { /// IOCTL: get_param: Query GPU / driver metadata. pub(crate) fn get_param( - dev: &NovaDevice, + dev: &NovaDevice<Registered>, + _reg_data: &(), getparam: &mut uapi::drm_nova_getparam, _file: &drm::File<File>, ) -> Result<u32> { - let adev = &dev.adev; - let parent = adev.parent(); - let pdev: &pci::Device = parent.try_into()?; + let adev: &auxiliary::Device<Bound> = dev.as_ref(); + let pdev: &pci::Device<Bound> = adev.parent().try_into()?; let value = match getparam.param as u32 { uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => pdev.resource_len(1)?, @@ -43,7 +49,8 @@ impl File { /// IOCTL: gem_create: Create a new DRM GEM object. pub(crate) fn gem_create( - dev: &NovaDevice, + dev: &NovaDevice<Registered>, + _reg_data: &(), req: &mut uapi::drm_nova_gem_create, file: &drm::File<File>, ) -> Result<u32> { @@ -56,7 +63,8 @@ impl File { /// IOCTL: gem_info: Query GEM metadata. pub(crate) fn gem_info( - _dev: &NovaDevice, + _dev: &NovaDevice<Registered>, + _reg_data: &(), req: &mut uapi::drm_nova_gem_info, file: &drm::File<File>, ) -> Result<u32> { 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<Ctx: DeviceContext>( - _dev: &NovaDevice<Ctx>, - _size: usize, - _args: Self::Args, - ) -> impl PinInit<Self, Error> { + fn new(_dev: &NovaDevice, _size: usize, _args: Self::Args) -> impl PinInit<Self, Error> { try_pin_init!(NovaObject {}) } } impl NovaObject { /// Create a new DRM GEM object. - pub(crate) fn new<Ctx: DeviceContext>( - dev: &NovaDevice<Ctx>, - size: usize, - ) -> Result<ARef<gem::Object<Self, Ctx>>> { + pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result<ARef<gem::Object<Self>>> { if size == 0 { return Err(EINVAL); } let aligned_size = page::page_align(size).ok_or(EINVAL)?; - gem::Object::<Self, Ctx>::new(dev, aligned_size, ()) + gem::Object::<Self>::new(dev, aligned_size, ()) } /// Look up a GEM object handle for a `File` and return an `ObjectRef` for it. diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs index d063bc664cc1..8348c6cd3929 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, @@ -46,13 +47,14 @@ 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<Ctx = drm::Registered> = drm::Device<TyrDrmDriver, Ctx>; +pub(crate) type TyrDrmDevice<Ctx = drm::Normal> = drm::Device<TyrDrmDriver, Ctx>; pub(crate) struct TyrPlatformDriver; #[pin_data(PinnedDrop)] -pub(crate) struct TyrPlatformDriverData { +pub(crate) struct TyrPlatformDriverData<'bound> { _device: ARef<TyrDrmDevice>, + _reg: drm::Registration<'bound, TyrDrmDriver>, } #[pin_data] @@ -97,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<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE); fn probe<'bound>( @@ -148,11 +150,14 @@ impl platform::Driver for TyrPlatformDriver { gpu_info, }); - let tdev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev.as_ref(), data)?; - let tdev = drm::driver::Registration::new_foreign_owned(tdev, pdev.as_ref(), 0)?; + let tdev = drm::UnregisteredDevice::<TyrDrmDriver>::new(pdev, data)?; + // 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 @@ -163,7 +168,7 @@ impl platform::Driver for TyrPlatformDriver { } #[pinned_drop] -impl PinnedDrop for TyrPlatformDriverData { +impl PinnedDrop for TyrPlatformDriverData<'_> { fn drop(self: Pin<&mut Self>) {} } @@ -180,8 +185,10 @@ const INFO: drm::DriverInfo = drm::DriverInfo { #[vtable] impl drm::Driver for TyrDrmDriver { type Data = TyrDrmDeviceData; + type RegistrationData<'a> = (); type File = TyrDrmFileData; - type Object<R: drm::DeviceContext> = drm::gem::shmem::Object<BoData, R>; + type Object = drm::gem::shmem::Object<BoData>; + type ParentDevice<Ctx: DeviceContext> = platform::Device<Ctx>; const INFO: drm::DriverInfo = INFO; const FEAT_RENDER: bool = true; diff --git a/drivers/gpu/drm/tyr/file.rs b/drivers/gpu/drm/tyr/file.rs index 31411da203c5..b686041d5d6b 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,8 @@ impl drm::file::DriverFile for TyrDrmFileData { impl TyrDrmFileData { pub(crate) fn dev_query( - ddev: &TyrDrmDevice, + ddev: &TyrDrmDevice<Registered>, + _reg_data: &(), devquery: &mut uapi::drm_panthor_dev_query, _file: &TyrDrmFile, ) -> Result<u32> { 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<Ctx: DeviceContext>( - _dev: &TyrDrmDevice<Ctx>, - _size: usize, - args: BoCreateArgs, - ) -> impl PinInit<Self, Error> { + fn new(_dev: &TyrDrmDevice, _size: usize, args: BoCreateArgs) -> impl PinInit<Self, Error> { try_pin_init!(Self { flags: args.flags }) } } diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 7ad124327a83..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, @@ -74,66 +75,59 @@ macro_rules! drm_legacy_fields { /// A trait implemented by all possible contexts a [`Device`] can be used in. /// -/// Setting up a new [`Device`] is a multi-stage process. Each step of the process that a user -/// interacts with in Rust has a respective [`DeviceContext`] typestate. For example, -/// `Device<T, Registered>` would be a [`Device`] that reached the [`Registered`] [`DeviceContext`]. +/// A [`Device`] can be in one of the following contexts: /// -/// Each stage of this process is described below: +/// - [`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 is currently registered with userspace and the parent bus device +/// is bound. /// -/// ```text -/// 1 2 3 -/// +--------------+ +------------------+ +-----------------------+ -/// |Device created| → |Device initialized| → |Registered w/ userspace| -/// +--------------+ +------------------+ +-----------------------+ -/// (Uninit) (Registered) -/// ``` -/// -/// 1. The [`Device`] is in the [`Uninit`] context and is not guaranteed to be initialized or -/// registered with userspace. Only a limited subset of DRM core functionality is available. -/// 2. The [`Device`] is guaranteed to be fully initialized, but is not guaranteed to be registered -/// with userspace. All DRM core functionality which doesn't interact with userspace is -/// available. We currently don't have a context for representing this. -/// 3. The [`Device`] is guaranteed to be fully initialized, and is guaranteed to have been -/// registered with userspace at some point - thus putting it in the [`Registered`] context. -/// -/// An important caveat of [`DeviceContext`] which must be kept in mind: when used as a typestate -/// for a reference type, it can only guarantee that a [`Device`] reached a particular stage in the -/// initialization process _at the time the reference was taken_. No guarantee is made in regards to -/// what stage of the process the [`Device`] is currently in. This means for instance that a -/// `&Device<T, Uninit>` may actually be registered with userspace, it just wasn't known to be -/// registered at the time the reference was taken. +/// Both `Device<T, Ioctl>` and `Device<T, Registered>` dereference to `Device<T>` ([`Normal`]), +/// so any method available on a [`Normal`] device is also available in the other contexts. pub trait DeviceContext: Sealed + Send + Sync + 'static {} -/// The [`DeviceContext`] of a [`Device`] that was registered with userspace at some point. +/// The general-purpose, reference-counted [`DeviceContext`]. /// -/// 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. +/// A [`Device`] in this context may or may not be registered with userspace. This context is used +/// for reference-counted device handles and during device setup via [`UnregisteredDevice`]. +/// +/// [`AlwaysRefCounted`] is only implemented for `Device<T, Normal>`, making this the required +/// context for [`ARef`]-based device handles. +pub struct Normal; + +impl Sealed for Normal {} +impl DeviceContext for Normal {} + +/// The [`DeviceContext`] of a [`Device`] that is currently registered with userspace. /// -/// Note: A device in this context is not guaranteed to remain registered with userspace for its -/// entire lifetime, as this is impossible to guarantee at compile-time. +/// A [`Device`] in this context is guaranteed to be registered and its parent bus device is +/// guaranteed to be bound. This is enforced at runtime by [`RegistrationGuard`], which holds a +/// `drm_dev_enter()` / `drm_dev_exit()` SRCU critical section. /// /// # Invariants /// -/// A [`Device`] in this [`DeviceContext`] is guaranteed to have been registered with userspace -/// at some point in time. +/// The parent bus device is bound for the duration of any reference to a `Device<T, Registered>`. pub struct Registered; impl Sealed for Registered {} impl DeviceContext for Registered {} -/// The [`DeviceContext`] of a [`Device`] that may be unregistered and partly uninitialized. +/// 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. /// -/// 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`]. +/// # Invariants /// -/// A [`Device`] in this context can be used to create a -/// [`Registration`](drm::driver::Registration). -pub struct Uninit; +/// A [`Device`] in this context has been registered with userspace via `drm_dev_register()` at +/// some point. +pub struct Ioctl; -impl Sealed for Uninit {} -impl DeviceContext for Uninit {} +impl Sealed for Ioctl {} +impl DeviceContext for Ioctl {} /// A [`Device`] which is known at compile-time to be unregistered with userspace. /// @@ -147,10 +141,10 @@ impl DeviceContext for Uninit {} /// /// The device in `self.0` is guaranteed to be a newly created [`Device`] that has not yet been /// registered with userspace until this type is dropped. -pub struct UnregisteredDevice<T: drm::Driver>(ARef<Device<T, Uninit>>, NotThreadSafe); +pub struct UnregisteredDevice<T: drm::Driver>(ARef<Device<T, Normal>>, NotThreadSafe); impl<T: drm::Driver> Deref for UnregisteredDevice<T> { - type Target = Device<T, Uninit>; + type Target = Device<T, Normal>; fn deref(&self) -> &Self::Target { &self.0 @@ -178,15 +172,13 @@ impl<T: drm::Driver> UnregisteredDevice<T> { master_drop: None, debugfs_init: None, - // Ignore the Uninit DeviceContext below. It is only provided because it is required by the - // compiler, and it is not actually used by these functions. - gem_create_object: T::Object::<Uninit>::ALLOC_OPS.gem_create_object, - prime_handle_to_fd: T::Object::<Uninit>::ALLOC_OPS.prime_handle_to_fd, - prime_fd_to_handle: T::Object::<Uninit>::ALLOC_OPS.prime_fd_to_handle, - gem_prime_import: T::Object::<Uninit>::ALLOC_OPS.gem_prime_import, - gem_prime_import_sg_table: T::Object::<Uninit>::ALLOC_OPS.gem_prime_import_sg_table, - dumb_create: T::Object::<Uninit>::ALLOC_OPS.dumb_create, - dumb_map_offset: T::Object::<Uninit>::ALLOC_OPS.dumb_map_offset, + gem_create_object: T::Object::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, @@ -208,10 +200,13 @@ impl<T: drm::Driver> UnregisteredDevice<T> { /// Create a new `UnregisteredDevice` for a `drm::Driver`. /// /// This can be used to create a [`Registration`](kernel::drm::Registration). - pub fn new(dev: &device::Device, data: impl PinInit<T::Data, Error>) -> Result<Self> { + pub fn new( + dev: &T::ParentDevice<device::Bound>, + data: impl PinInit<T::Data, Error>, + ) -> Result<Self> { // `__drm_dev_alloc` uses `kmalloc()` to allocate memory, hence ensure a `kmalloc()` // compatible `Layout`. - let layout = Kmalloc::aligned_layout(Layout::new::<Device<T, Uninit>>()); + let layout = Kmalloc::aligned_layout(Layout::new::<Device<T, Normal>>()); // Use a temporary vtable without a `release` callback until `data` is initialized, so // init failure can release the DRM device without dropping uninitialized fields. @@ -223,12 +218,12 @@ impl<T: drm::Driver> UnregisteredDevice<T> { // SAFETY: // - `alloc_vtable` reference remains valid until no longer used, // - `dev` is valid by its type invarants, - let raw_drm: *mut Device<T, Uninit> = unsafe { + let raw_drm: *mut Device<T, Normal> = unsafe { bindings::__drm_dev_alloc( - dev.as_raw(), + dev.as_ref().as_raw(), &alloc_vtable, layout.size(), - mem::offset_of!(Device<T, Uninit>, dev), + mem::offset_of!(Device<T, Normal>, dev), ) } .cast(); @@ -253,6 +248,9 @@ impl<T: drm::Driver> UnregisteredDevice<T> { // 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`. @@ -264,16 +262,8 @@ impl<T: drm::Driver> UnregisteredDevice<T> { /// 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 currently registered with userspace and its parent +/// bus device is bound. The [`Normal`] context is the general-purpose, reference-counted context. /// /// # Invariants /// @@ -281,9 +271,10 @@ impl<T: drm::Driver> UnregisteredDevice<T> { /// * The data layout of `Self` remains the same across all implementations of `C`. /// * Any invariants for `C` also apply. #[repr(C)] -pub struct Device<T: drm::Driver, C: DeviceContext = Registered> { +pub struct Device<T: drm::Driver, C: DeviceContext = Normal> { dev: Opaque<bindings::drm_device>, data: T::Data, + pub(super) registration_data: UnsafeCell<NonNull<T::RegistrationData<'static>>>, _ctx: PhantomData<C>, } @@ -352,7 +343,111 @@ impl<T: drm::Driver, C: DeviceContext> Device<T, C> { } } -impl<T: drm::Driver, C: DeviceContext> Deref for Device<T, C> { +impl<T: drm::Driver> Device<T, Ioctl> { + /// Guard against the parent bus device being unbound. + /// + /// Returns a [`RegistrationGuard`] if the device has not been unplugged, [`None`] otherwise. + /// + /// While [`RegistrationGuard`] is held the parent device is guaranteed to be bound. + #[must_use] + pub fn registration_guard(&self) -> Option<RegistrationGuard<'_, T>> { + let mut idx: i32 = 0; + // SAFETY: `self.as_raw()` is a valid pointer to a `struct drm_device`. + if unsafe { bindings::drm_dev_enter(self.as_raw(), &mut idx) } { + // INVARIANT: + // - `idx` is the SRCU index from the successful `drm_dev_enter()` above. + // - The parent bus device is bound: `drm_dev_enter()` succeeded, meaning + // `drm_dev_unplug()` has not completed; since it is only called from + // `Registration::drop()` during parent unbind, the parent is still bound. + Some(RegistrationGuard { + // SAFETY: See INVARIANT above; the `Registered` context invariant holds. + dev: unsafe { self.assume_ctx() }, + idx, + _not_send: NotThreadSafe, + }) + } else { + None + } + } +} + +/// A guard proving the DRM device is registered and the parent bus device is bound. +/// +/// The guard dereferences to [`Device<T, Registered>`], providing access to the DRM device with +/// the guarantee that the parent bus device is bound for the entire duration of the critical +/// section. +/// +/// Internally this is backed by a `drm_dev_enter()` / `drm_dev_exit()` SRCU critical section. +/// +/// # Invariants +/// +/// - `idx` is the SRCU read lock index returned by a successful `drm_dev_enter()` call. +/// - The parent bus device of `dev` is bound for the lifetime of this guard. +#[must_use] +pub struct RegistrationGuard<'a, T: drm::Driver> { + dev: &'a Device<T, Registered>, + idx: i32, + _not_send: NotThreadSafe, +} + +impl<T: drm::Driver> Device<T, Registered> { + /// 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<R, F>(&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<T: drm::Driver> Deref for RegistrationGuard<'_, T> { + type Target = Device<T, Registered>; + + #[inline] + fn deref(&self) -> &Self::Target { + self.dev + } +} + +impl<T: drm::Driver> Drop for RegistrationGuard<'_, T> { + #[inline] + fn drop(&mut self) { + // SAFETY: `self.idx` was returned by a successful `drm_dev_enter()` call, as guaranteed + // by the type invariants of `RegistrationGuard`. + unsafe { bindings::drm_dev_exit(self.idx) }; + } +} + +impl<T: drm::Driver> Deref for Device<T> { type Target = T::Data; fn deref(&self) -> &Self::Target { @@ -360,9 +455,31 @@ impl<T: drm::Driver, C: DeviceContext> Deref for Device<T, C> { } } +impl<T: drm::Driver> Deref for Device<T, Registered> { + type Target = Device<T>; + + #[inline] + fn deref(&self) -> &Self::Target { + // SAFETY: The caller holds a `Device<T, Registered>`, which guarantees all invariants + // of the weaker `Normal` context. + unsafe { self.assume_ctx() } + } +} + +impl<T: drm::Driver> Deref for Device<T, Ioctl> { + type Target = Device<T>; + + #[inline] + fn deref(&self) -> &Self::Target { + // SAFETY: The caller holds a `Device<T, Ioctl>`, which guarantees all invariants + // of the weaker `Normal` context. + unsafe { self.assume_ctx() } + } +} + // SAFETY: DRM device objects are always reference counted and the get/put functions // satisfy the requirements. -unsafe impl<T: drm::Driver, C: DeviceContext> AlwaysRefCounted for Device<T, C> { +unsafe impl<T: drm::Driver> AlwaysRefCounted for Device<T> { fn inc_ref(&self) { // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. unsafe { bindings::drm_dev_get(self.as_raw()) }; @@ -377,11 +494,29 @@ unsafe impl<T: drm::Driver, C: DeviceContext> AlwaysRefCounted for Device<T, C> } } -impl<T: drm::Driver, C: DeviceContext> AsRef<device::Device> for Device<T, C> { - fn as_ref(&self) -> &device::Device { +impl<T: drm::Driver> AsRef<T::ParentDevice<device::Normal>> for Device<T> { + fn as_ref(&self) -> &T::ParentDevice<device::Normal> { // 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) } + } +} + +impl<T: drm::Driver> AsRef<T::ParentDevice<device::Bound>> for Device<T, Registered> { + #[inline] + fn as_ref(&self) -> &T::ParentDevice<device::Bound> { + let dev = (**self).as_ref().as_ref(); + + // SAFETY: A `Device<T, Registered>` 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) } } } @@ -392,12 +527,10 @@ unsafe impl<T: drm::Driver, C: DeviceContext> Send for Device<T, C> {} // by the synchronization in `struct drm_device`. unsafe impl<T: drm::Driver, C: DeviceContext> Sync for Device<T, C> {} -impl<T, C, const ID: u64> WorkItem<ID> for Device<T, C> +impl<T: drm::Driver, const ID: u64> WorkItem<ID> for Device<T> where - T: drm::Driver, T::Data: WorkItem<ID, Pointer = ARef<Self>>, T::Data: HasWork<Self, ID>, - C: DeviceContext, { type Pointer = ARef<Self>; diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index 25f7e233884d..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,12 +106,23 @@ 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<Ctx: drm::DeviceContext>: AllocImpl; + type Object: AllocImpl; /// The type used to represent a DRM File (client) type File: drm::file::DriverFile; + /// The bus device type of the parent device that the DRM device is associated with. + type ParentDevice<Ctx: device::DeviceContext>: device::AsBusDevice<Ctx>; + /// Driver metadata const INFO: DriverInfo; @@ -136,68 +143,84 @@ pub trait Driver { /// The registration type of a `drm::Device`. /// /// Once the `Registration` structure is dropped, the device is unregistered. -pub struct Registration<T: Driver>(ARef<drm::Device<T>>); - -impl<T: Driver> Registration<T> { - fn new(drm: drm::UnregisteredDevice<T>, flags: usize) -> Result<Self> { - // SAFETY: `drm.as_raw()` is valid by the invariants of `drm::Device`. - to_result(unsafe { bindings::drm_dev_register(drm.as_raw(), flags) })?; - - // 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<drm::Device<T>>, + _reg_data: Pin<KBox<T::RegistrationData<'a>>>, +} - /// 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<T>, + /// # 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<E>( dev: &'a device::Device<device::Bound>, + drm: drm::UnregisteredDevice<T>, + reg_data: impl PinInit<T::RegistrationData<'a>, E>, flags: usize, - ) -> Result<&'a drm::Device<T>> + ) -> Result<Self> where - T: 'static, + Error: From<E>, { - 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); } - let reg = Registration::<T>::new(drm, flags)?; - let drm = NonNull::from(reg.device()); + let reg_data: Pin<KBox<T::RegistrationData<'a>>> = 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<T::RegistrationData<'static>> = + 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<T> { - &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<T: Driver> Sync for Registration<T> {} +unsafe impl<T: Driver> Sync for Registration<'_, T> {} // SAFETY: Registration with and unregistration from the DRM subsystem can happen from any thread. -unsafe impl<T: Driver> Send for Registration<T> {} +unsafe impl<T: Driver> Send for Registration<'_, T> {} -impl<T: Driver> Drop for Registration<T> { +impl<T: Driver> Drop for Registration<'_, T> { fn drop(&mut self) { + // Use `drm_dev_unplug` rather than `drm_dev_unregister` to ensure that existing + // `drm_dev_enter()` critical sections complete before unregistration proceeds. This + // is required for the safety of `RegistrationGuard`, which relies on the SRCU barrier in + // `drm_dev_unplug()` to guarantee that the parent device is still bound within the + // critical section. + // // SAFETY: Safe by the invariant of `ARef<drm::Device<T>>`. The existence of this - // `Registration` also guarantees the this `drm::Device` is actually registered. - unsafe { bindings::drm_dev_unregister(self.0.as_raw()) }; + // `Registration` also guarantees that this `drm::Device` is actually registered. + unsafe { bindings::drm_dev_unplug(self.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/mod.rs b/rust/kernel/drm/gem/mod.rs index 48fa6e96dfe7..b03b5f9ccd7e 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -10,7 +10,7 @@ use crate::{ self, device::{ DeviceContext, - Registered, // + Normal, // }, driver::{ AllocImpl, @@ -81,8 +81,7 @@ pub type DriverFile<T> = drm::File<<<T as DriverObject>::Driver as drm::Driver>: /// A type alias for retrieving the current [`AllocImpl`] for a given [`DriverObject`]. /// /// [`Driver`]: drm::Driver -pub type DriverAllocImpl<T, Ctx = Registered> = - <<T as DriverObject>::Driver as drm::Driver>::Object<Ctx>; +pub type DriverAllocImpl<T> = <<T as DriverObject>::Driver as drm::Driver>::Object; /// GEM object functions, which must be implemented by drivers. pub trait DriverObject: Sync + Send + Sized + 'static { @@ -93,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<Ctx: DeviceContext>( - dev: &drm::Device<Self::Driver, Ctx>, + fn new( + dev: &drm::Device<Self::Driver>, size: usize, args: Self::Args, ) -> impl PinInit<Self, Error>; @@ -109,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; @@ -183,7 +182,7 @@ pub trait BaseObject: IntoGEMObject { fn create_handle<D, F>(&self, file: &drm::File<F>) -> Result<u32> where Self: AllocImpl<Driver = D>, - D: drm::Driver<Object<Registered> = Self, File = F>, + D: drm::Driver<Object = Self, File = F>, F: drm::file::DriverFile<Driver = D>, { let mut handle: u32 = 0; @@ -197,8 +196,8 @@ pub trait BaseObject: IntoGEMObject { /// Looks up an object by its handle for a given `File`. fn lookup_handle<D, F>(file: &drm::File<F>, handle: u32) -> Result<ARef<Self>> where - Self: AllocImpl<Driver = D>, - D: drm::Driver<Object<Registered> = Self, File = F>, + Self: AllocImpl<Driver = D> + AlwaysRefCounted, + D: drm::Driver<Object = Self, File = F>, F: drm::file::DriverFile<Driver = D>, { // SAFETY: The arguments are all valid per the type invariants. @@ -254,7 +253,7 @@ impl<T: IntoGEMObject> BaseObjectPrivate for T {} /// * Any type invariants of `Ctx` apply to the parent DRM device for this GEM object. #[repr(C)] #[pin_data] -pub struct Object<T: DriverObject + Send + Sync, Ctx: DeviceContext = Registered> { +pub struct Object<T: DriverObject + Send + Sync, Ctx: DeviceContext = Normal> { obj: Opaque<bindings::drm_gem_object>, #[pin] data: T, @@ -280,12 +279,43 @@ impl<T: DriverObject, Ctx: DeviceContext> Object<T, Ctx> { rss: None, }; + /// Returns the `Device` that owns this GEM object. + pub fn dev(&self) -> &drm::Device<T::Driver, Ctx> { + // SAFETY: + // - `struct drm_gem_object.dev` is initialized and valid for as long as the GEM + // object lives. + // - The device we used for creating the gem object is passed as &drm::Device<T::Driver> to + // Object::<T>::new(), so we know that `T::Driver` is the right generic parameter to use + // here. + // - Any type invariants of `Ctx` are upheld by using the same `Ctx` for the `Device` we + // return. + unsafe { drm::Device::from_raw((*self.as_raw()).dev) } + } + + 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<bindings::drm_gem_object> = obj.cast(); + + // SAFETY: All of our objects are of type `Object<T>`. + 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<T: DriverObject> Object<T> { /// Create a new GEM object. - pub fn new( - dev: &drm::Device<T::Driver, Ctx>, - size: usize, - args: T::Args, - ) -> Result<ARef<Self>> { + pub fn new(dev: &drm::Device<T::Driver>, size: usize, args: T::Args) -> Result<ARef<Self>> { let obj: Pin<KBox<Self>> = KBox::pin_init( try_pin_init!(Self { obj: Opaque::new(bindings::drm_gem_object::default()), @@ -321,46 +351,12 @@ impl<T: DriverObject, Ctx: DeviceContext> Object<T, Ctx> { // 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<T::Driver, Ctx> { - // SAFETY: - // - `struct drm_gem_object.dev` is initialized and valid for as long as the GEM - // object lives. - // - The device we used for creating the gem object is passed as &drm::Device<T::Driver> to - // Object::<T>::new(), so we know that `T::Driver` is the right generic parameter to use - // here. - // - Any type invariants of `Ctx` are upheld by using the same `Ctx` for the `Device` we - // return. - unsafe { drm::Device::from_raw((*self.as_raw()).dev) } - } - - 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<bindings::drm_gem_object> = obj.cast(); - - // SAFETY: All of our objects are of type `Object<T>`. - 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<T, C> for Object<T, C> + impl<T> for Object<T> where - T: DriverObject, - C: DeviceContext + T: DriverObject } impl<T: DriverObject, Ctx: DeviceContext> super::private::Sealed for Object<T, Ctx> {} diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index 3ee19ef6264e..60dca8871b87 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, - Registered, // + Device, // }, error::{ from_err_ptr, @@ -48,7 +46,6 @@ use crate::{ }; use core::{ ffi::c_void, - marker::PhantomData, mem::{ ManuallyDrop, MaybeUninit, // @@ -73,17 +70,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 = Registered> { +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<T, C>>, + pub parent_resv_obj: Option<&'a Object<T>>, } -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 { @@ -99,38 +96,35 @@ impl<'a, T: DriverObject, C: DeviceContext> Default for ObjectConfig<'a, T, C> { /// /// - `obj` contains a valid initialized `struct drm_gem_shmem_object` for the lifetime of this /// object. -/// - Any type invariants of `C` apply to the parent DRM device for this GEM object. #[repr(C)] #[pin_data] -pub struct Object<T: DriverObject, C: DeviceContext = Registered> { +pub struct Object<T: DriverObject> { #[pin] obj: Opaque<bindings::drm_gem_shmem_object>, /// Parent object that owns this object's DMA reservation object. - parent_resv_obj: Option<ARef<Object<T, C>>>, + parent_resv_obj: Option<ARef<Object<T>>>, /// Devres object for unmapping any SGTable on driver-unbind. - sgt_res: ManuallyDrop<SetOnce<Devres<SGTableMap<T, C>>>>, + sgt_res: ManuallyDrop<SetOnce<Devres<SGTableMap<T>>>>, #[pin] /// Lock for protecting initialization of `sgt_res`. sgt_lock: Mutex<()>, #[pin] inner: T, - _ctx: PhantomData<C>, } super::impl_aref_for_gem_obj! { - impl<T, C> for Object<T, C> + impl<T> for Object<T> where - T: DriverObject, - C: DeviceContext + T: DriverObject } // SAFETY: All GEM objects are thread-safe. -unsafe impl<T: DriverObject, C: DeviceContext> Send for Object<T, C> {} +unsafe impl<T: DriverObject> Send for Object<T> {} // SAFETY: All GEM objects are thread-safe. -unsafe impl<T: DriverObject, C: DeviceContext> Sync for Object<T, C> {} +unsafe impl<T: DriverObject> Sync for Object<T> {} -impl<T: DriverObject, C: DeviceContext> Object<T, C> { +impl<T: DriverObject> Object<T> { /// `drm_gem_object_funcs` vtable suitable for GEM shmem objects. const VTABLE: bindings::drm_gem_object_funcs = bindings::drm_gem_object_funcs { free: Some(Self::free_callback), @@ -157,56 +151,8 @@ impl<T: DriverObject, C: DeviceContext> Object<T, C> { 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<T::Driver, C>, - size: usize, - config: ObjectConfig<'_, T, C>, - args: T::Args, - ) -> Result<ARef<Self>> { - let new: Pin<KBox<Self>> = 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::<C>, - }), - 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<T::Driver, C> { + pub fn dev(&self) -> &Device<T::Driver> { // SAFETY: `dev` will have been initialized in `Self::new()` by `drm_gem_shmem_init()`. unsafe { Device::from_raw((*self.as_raw()).dev) } } @@ -220,8 +166,8 @@ impl<T: DriverObject, C: DeviceContext> Object<T, C> { // SAFETY: // - We verified above that `obj` is valid, which makes `this` valid - // - This function is set in AllocOps, so we know that `this` is contained within a - // `Object<T, C>` + // - This function is set in AllocOps, so we know that `this` is contained within an + // `Object<T>` let this = unsafe { container_of!(Opaque::cast_from(base), Self, obj) }.cast_mut(); // We need to drop `sgt_res` first, since doing so requires that the GEM object is still @@ -242,7 +188,7 @@ impl<T: DriverObject, C: DeviceContext> Object<T, C> { } /// Attempt to create a vmap from the gem object, and confirm the size of said vmap. - fn make_vmap<'a, R, const SIZE: usize>(&'a self) -> Result<VMap<T, R, C, SIZE>> + fn make_vmap<'a, R, const SIZE: usize>(&'a self) -> Result<VMap<T, R, SIZE>> where R: Deref<Target = Self> + From<&'a Self>, { @@ -304,13 +250,7 @@ impl<T: DriverObject, C: DeviceContext> Object<T, C> { /// Creates and returns a virtual kernel memory mapping for this object. #[inline] - pub fn vmap<const SIZE: usize>(&self) -> Result<VMapRef<'_, T, C, SIZE>> { - self.make_vmap() - } - - /// Creates and returns an owned reference to a virtual kernel memory mapping for this object. - #[inline] - pub fn owned_vmap<const SIZE: usize>(&self) -> Result<VMapOwned<T, C, SIZE>> { + pub fn vmap<const SIZE: usize>(&self) -> Result<VMapRef<'_, T, SIZE>> { self.make_vmap() } @@ -324,7 +264,8 @@ impl<T: DriverObject, C: DeviceContext> Object<T, C> { &'a self, dev: &'a device::Device<Bound>, ) -> 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); } @@ -353,9 +294,62 @@ impl<T: DriverObject, C: DeviceContext> Object<T, C> { Ok(sgt_res.access(dev)?) } + + /// Create a new shmem-backed DRM object of the given size. + /// + /// Additional config options can be specified using `config`. + pub fn new( + dev: &Device<T::Driver>, + size: usize, + config: ObjectConfig<'_, T>, + args: T::Args, + ) -> Result<ARef<Self>> { + let new: Pin<KBox<Self>> = 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), + }), + 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<const SIZE: usize>(&self) -> Result<VMapOwned<T, SIZE>> { + self.make_vmap() + } } -impl<T: DriverObject, C: DeviceContext> Deref for Object<T, C> { +impl<T: DriverObject> Deref for Object<T> { type Target = T; fn deref(&self) -> &Self::Target { @@ -363,15 +357,15 @@ impl<T: DriverObject, C: DeviceContext> Deref for Object<T, C> { } } -impl<T: DriverObject, C: DeviceContext> DerefMut for Object<T, C> { +impl<T: DriverObject> DerefMut for Object<T> { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.inner } } -impl<T: DriverObject, C: DeviceContext> Sealed for Object<T, C> {} +impl<T: DriverObject> Sealed for Object<T> {} -impl<T: DriverObject, C: DeviceContext> gem::IntoGEMObject for Object<T, C> { +impl<T: DriverObject> gem::IntoGEMObject for Object<T> { fn as_raw(&self) -> *mut bindings::drm_gem_object { // SAFETY: // - Our immutable reference is proof that this is safe to dereference. @@ -390,7 +384,7 @@ impl<T: DriverObject, C: DeviceContext> gem::IntoGEMObject for Object<T, C> { } } -impl<T: DriverObject, C: DeviceContext> driver::AllocImpl for Object<T, C> { +impl<T: DriverObject> driver::AllocImpl for Object<T> { type Driver = T::Driver; const ALLOC_OPS: driver::AllocOps = driver::AllocOps { @@ -409,14 +403,11 @@ impl<T: DriverObject, C: DeviceContext> driver::AllocImpl for Object<T, C> { /// When this is dropped, the `dma_resv` lock is dropped as well. /// // TODO: This should be replace with a WwMutex equivalent once we have such bindings in the kernel. -struct DmaResvGuard<'a, T: DriverObject, C: DeviceContext = Registered>( - &'a Object<T, C>, - NotThreadSafe, -); +struct DmaResvGuard<'a, T: DriverObject>(&'a Object<T>, NotThreadSafe); -impl<'a, T: DriverObject, C: DeviceContext> DmaResvGuard<'a, T, C> { +impl<'a, T: DriverObject> DmaResvGuard<'a, T> { #[inline] - fn new(obj: &'a Object<T, C>) -> Self { + fn new(obj: &'a Object<T>) -> Self { // SAFETY: This lock is initialized throughout the lifetime of `object`. unsafe { bindings::dma_resv_lock(obj.raw_dma_resv(), ptr::null_mut()) }; @@ -424,7 +415,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. @@ -438,40 +429,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<D, R, C = Registered, const SIZE: usize = 0> +pub struct VMap<D, R, const SIZE: usize = 0> where D: DriverObject, - C: DeviceContext, - R: Deref<Target = Object<D, C>>, + R: Deref<Target = Object<D>>, { addr: *mut c_void, owner: R, } /// An alias type for a reference to a shmem-based GEM object's VMap. -pub type VMapRef<'a, D, C, const SIZE: usize = 0> = VMap<D, &'a Object<D, C>, C, SIZE>; +pub type VMapRef<'a, D, const SIZE: usize = 0> = VMap<D, &'a Object<D>, SIZE>; /// An alias type for an owned reference to a shmem-based GEM object's VMap. -pub type VMapOwned<D, C, const SIZE: usize = 0> = VMap<D, ARef<Object<D, C>>, C, SIZE>; +pub type VMapOwned<D, const SIZE: usize = 0> = VMap<D, ARef<Object<D>>, SIZE>; -impl<D, R, C, const SIZE: usize> VMap<D, R, C, SIZE> +impl<D, R, const SIZE: usize> VMap<D, R, SIZE> where D: DriverObject, - C: DeviceContext, - R: Deref<Target = Object<D, C>>, + R: Deref<Target = Object<D>>, { /// Borrows a reference to the object that owns this virtual mapping. #[inline] - pub fn owner(&self) -> &Object<D, C> { + pub fn owner(&self) -> &Object<D> { &self.owner } } -impl<D, R, C, const SIZE: usize> Drop for VMap<D, R, C, SIZE> +impl<D, R, const SIZE: usize> Drop for VMap<D, R, SIZE> where D: DriverObject, - C: DeviceContext, - R: Deref<Target = Object<D, C>>, + R: Deref<Target = Object<D>>, { #[inline] fn drop(&mut self) { @@ -490,29 +478,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<D, R, C, const SIZE: usize> Send for VMap<D, R, C, SIZE> +unsafe impl<D, R, const SIZE: usize> Send for VMap<D, R, SIZE> where D: DriverObject, - C: DeviceContext, - R: Deref<Target = Object<D, C>> + Send, + R: Deref<Target = Object<D>> + Send, { } // SAFETY: `addr` points to a valid memory address for as long as `owner` exists, meaning that so // long as `owner` is `Sync` so is `VMap`. -unsafe impl<D, R, C, const SIZE: usize> Sync for VMap<D, R, C, SIZE> +unsafe impl<D, R, const SIZE: usize> Sync for VMap<D, R, SIZE> where D: DriverObject, - C: DeviceContext, - R: Deref<Target = Object<D, C>> + Sync, + R: Deref<Target = Object<D>> + Sync, { } -impl<D, R, C, const SIZE: usize> Io for VMap<D, R, C, SIZE> +impl<D, R, const SIZE: usize> Io for VMap<D, R, SIZE> where D: DriverObject, - C: DeviceContext, - R: Deref<Target = Object<D, C>>, + R: Deref<Target = Object<D>>, { #[inline] fn addr(&self) -> usize { @@ -525,22 +510,20 @@ where } } -impl<D, R, C, const SIZE: usize> IoKnownSize for VMap<D, R, C, SIZE> +impl<D, R, const SIZE: usize> IoKnownSize for VMap<D, R, SIZE> where D: DriverObject, - C: DeviceContext, - R: Deref<Target = Object<D, C>>, + R: Deref<Target = Object<D>>, { const MIN_SIZE: usize = SIZE; } macro_rules! impl_vmap_io_capable { ($ty:ty) => { - impl<D, R, C, const SIZE: usize> IoCapable<$ty> for VMap<D, R, C, SIZE> + impl<D, R, const SIZE: usize> IoCapable<$ty> for VMap<D, R, SIZE> where D: DriverObject, - C: DeviceContext, - R: Deref<Target = Object<D, C>>, + R: Deref<Target = Object<D>>, { #[inline] unsafe fn io_read(&self, address: usize) -> $ty { @@ -583,11 +566,11 @@ impl_vmap_io_capable!(u64); /// [`SGTable`]. /// /// [`SGTable`]: scatterlist::SGTable -pub struct SGTableMap<T: DriverObject, C: DeviceContext> { - obj: NonNull<Object<T, C>>, +pub struct SGTableMap<T: DriverObject> { + obj: NonNull<Object<T>>, } -impl<T: DriverObject, C: DeviceContext> Deref for SGTableMap<T, C> { +impl<T: DriverObject> Deref for SGTableMap<T> { type Target = scatterlist::SGTable; fn deref(&self) -> &Self::Target { @@ -598,7 +581,7 @@ impl<T: DriverObject, C: DeviceContext> Deref for SGTableMap<T, C> { } } -impl<T: DriverObject, C: DeviceContext> Drop for SGTableMap<T, C> { +impl<T: DriverObject> Drop for SGTableMap<T> { fn drop(&mut self) { // SAFETY: `obj` is always valid via our type invariants let obj = unsafe { self.obj.as_ref() }; @@ -609,8 +592,8 @@ impl<T: DriverObject, C: DeviceContext> Drop for SGTableMap<T, C> { } } -impl<T: DriverObject, C: DeviceContext> SGTableMap<T, C> { - fn new(obj: &Object<T, C>) -> impl Init<Self, Error> { +impl<T: DriverObject> SGTableMap<T> { + fn new(obj: &Object<T>) -> impl Init<Self, Error> { // INVARIANT: // - We call drm_gem_shmem_get_pages_sgt below and check whether or not it succeeds, // fulfilling the invariant of SGTableMap that the object's `sgt` field is initialized. @@ -624,10 +607,10 @@ impl<T: DriverObject, C: DeviceContext> SGTableMap<T, C> { // SAFETY: The NonNull in SGTableMap is guaranteed valid by our type invariants, and the GEM object // it points to is guaranteed to be thread-safe. -unsafe impl<T: DriverObject, C: DeviceContext> Send for SGTableMap<T, C> {} +unsafe impl<T: DriverObject> Send for SGTableMap<T> {} // SAFETY: The NonNull in SGTableMap is guaranteed valid by our type invariants, and the GEM object // it points to is guaranteed to be thread-safe. -unsafe impl<T: DriverObject, C: DeviceContext> Sync for SGTableMap<T, C> {} +unsafe impl<T: DriverObject> Sync for SGTableMap<T> {} #[kunit_tests(rust_drm_gem_shmem)] mod tests { @@ -670,8 +653,8 @@ mod tests { type Driver = KunitDriver; type Args = (); - fn new<C: DeviceContext>( - _dev: &drm::Device<KunitDriver, C>, + fn new( + _dev: &drm::Device<KunitDriver>, _size: usize, _args: Self::Args, ) -> impl PinInit<Self, Error> { @@ -682,8 +665,10 @@ mod tests { #[vtable] impl drm::Driver for KunitDriver { type Data = KunitData; + type RegistrationData<'a> = (); type File = KunitFile; - type Object<Ctx: DeviceContext> = Object<KunitObject, Ctx>; + type Object = Object<KunitObject>; + type ParentDevice<Ctx: device::DeviceContext> = faux::Device<Ctx>; const INFO: drm::DriverInfo = INFO; const IOCTLS: &'static [drm::ioctl::DrmIoctlDescriptor] = &[]; @@ -692,17 +677,18 @@ mod tests { fn create_drm_dev() -> Result<(faux::Registration, UnregisteredDevice<KunitDriver>)> { // Create a faux DRM device so we can test gem object creation. let data = try_pin_init!(KunitData {}); - let dev = faux::Registration::new(c"Kunit", None)?; - let drm = UnregisteredDevice::new(dev.as_ref(), data)?; + let reg = faux::Registration::new(c"Kunit", None)?; + let fdev = reg.as_ref(); + let drm = UnregisteredDevice::new(fdev, data)?; - Ok((dev, drm)) + Ok((reg, drm)) } #[test] fn compile_time_vmap_sizes() -> Result { let (_dev, drm) = create_drm_dev()?; - let obj = Object::<KunitObject, _>::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; + let obj = Object::<KunitObject>::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; // Try creating a normal vmap obj.vmap::<PAGE_SIZE>()?; @@ -726,7 +712,7 @@ mod tests { fn vmap_io() -> Result { let (_dev, drm) = create_drm_dev()?; - let obj = Object::<KunitObject, _>::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; + let obj = Object::<KunitObject>::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; let vmap = obj.vmap::<PAGE_SIZE>()?; @@ -755,9 +741,10 @@ 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::<KunitObject, _>::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; + let obj = Object::<KunitObject>::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; assert_eq!(obj.sg_table(wrong_dev.as_ref()).err().unwrap(), EINVAL); diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs index cf328101dde4..64af9eacc306 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<T: crate::drm::Driver>( + ptr: *const crate::drm::Device<T, crate::drm::Ioctl>, + ) -> *const crate::drm::Device<T, crate::drm::Registered> { + ptr.cast() + } } /// Declare the DRM ioctls for a driver. @@ -82,7 +94,8 @@ pub mod internal { /// `user_callback` should have the following prototype: /// /// ```ignore -/// fn foo(device: &kernel::drm::Device<Self>, +/// fn foo(device: &kernel::drm::Device<Self, kernel::drm::Registered>, +/// reg_data: &Self::RegistrationData<'_>, /// data: &mut uapi::argument_type, /// file: &kernel::drm::File<Self::File>, /// ) -> Result<u32> @@ -131,10 +144,45 @@ 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::from_raw(raw_dev); + 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!(), + 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 _, &'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. @@ -147,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(dev, 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()), diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs index a66e7166f66b..fd6ed35bc35a 100644 --- a/rust/kernel/drm/mod.rs +++ b/rust/kernel/drm/mod.rs @@ -11,8 +11,10 @@ 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::Uninit; +pub use self::device::RegistrationGuard; pub use self::device::UnregisteredDevice; pub use self::driver::Driver; pub use self::driver::DriverInfo; 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<Ctx: device::DeviceContext = device::Normal>( + Opaque<bindings::faux_device>, + PhantomData<Ctx>, +); + +impl<Ctx: device::DeviceContext> Device<Ctx> { + #[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<bindings::faux_device>`. + unsafe { &*ptr.cast() } + } +} + +impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> { + #[inline] + fn as_ref(&self) -> &device::Device<Ctx> { + // 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<Ctx: device::DeviceContext> device::AsBusDevice<Ctx> for Device<Ctx> { + 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<device::Device<device::Bound>> for Registration { - fn as_ref(&self) -> &device::Device<device::Bound> { +impl AsRef<Device<device::Bound>> for Registration { + #[inline] + fn as_ref(&self) -> &Device<device::Bound> { // 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<device::Device<device::Bound>> 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 }) } |
