diff options
Diffstat (limited to 'rust')
| -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 |
7 files changed, 562 insertions, 322 deletions
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()) } } } |