From: Lyude Paul Date: Thu, 7 May 2026 21:59:21 +0000 (-0400) Subject: rust/drm/gem: Use DeviceContext with GEM objects X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0023a1e8d01a9d400257d30c851bd16a29568809;p=thirdparty%2Fkernel%2Flinux.git rust/drm/gem: Use DeviceContext with GEM objects Now that we have the ability to represent the context in which a DRM device is in at compile-time, we can start carrying around this context with GEM object types in order to allow a driver to safely create GEM objects before a DRM device has registered with userspace. Signed-off-by: Lyude Paul Reviewed-by: Daniel Almeida Link: https://patch.msgid.link/20260507220044.3204919-4-lyude@redhat.com Signed-off-by: Danilo Krummrich --- diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs index 33fe088dd3b9a..48933d86ddda9 100644 --- a/drivers/gpu/drm/nova/driver.rs +++ b/drivers/gpu/drm/nova/driver.rs @@ -73,7 +73,7 @@ impl auxiliary::Driver for NovaDriver { impl drm::Driver for NovaDriver { type Data = NovaData; type File = File; - type Object = gem::Object; + type Object = gem::Object; const INFO: drm::DriverInfo = INFO; diff --git a/drivers/gpu/drm/nova/gem.rs b/drivers/gpu/drm/nova/gem.rs index e073e174e2578..9d8ff7de2c0f7 100644 --- a/drivers/gpu/drm/nova/gem.rs +++ b/drivers/gpu/drm/nova/gem.rs @@ -2,7 +2,7 @@ use kernel::{ drm, - drm::{gem, gem::BaseObject}, + drm::{gem, gem::BaseObject, DeviceContext}, page, prelude::*, sync::aref::ARef, @@ -21,20 +21,27 @@ impl gem::DriverObject for NovaObject { type Driver = NovaDriver; type Args = (); - fn new(_dev: &NovaDevice, _size: usize, _args: Self::Args) -> impl PinInit { + fn new( + _dev: &NovaDevice, + _size: usize, + _args: Self::Args, + ) -> impl PinInit { try_pin_init!(NovaObject {}) } } impl NovaObject { /// Create a new DRM GEM object. - pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result>> { + pub(crate) fn new( + dev: &NovaDevice, + size: usize, + ) -> Result>> { if size == 0 { return Err(EINVAL); } let aligned_size = page::page_align(size).ok_or(EINVAL)?; - gem::Object::new(dev, aligned_size, ()) + gem::Object::::new(dev, aligned_size, ()) } /// Look up a GEM object handle for a `File` and return an `ObjectRef` for it. diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs index 3e53ecd331433..d063bc664cc11 100644 --- a/drivers/gpu/drm/tyr/driver.rs +++ b/drivers/gpu/drm/tyr/driver.rs @@ -181,7 +181,7 @@ const INFO: drm::DriverInfo = drm::DriverInfo { impl drm::Driver for TyrDrmDriver { type Data = TyrDrmDeviceData; type File = TyrDrmFileData; - type Object = drm::gem::shmem::Object; + type Object = drm::gem::shmem::Object; const INFO: drm::DriverInfo = INFO; const FEAT_RENDER: bool = true; diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs index 1640a161754b2..c6d4d6f9bae39 100644 --- a/drivers/gpu/drm/tyr/gem.rs +++ b/drivers/gpu/drm/tyr/gem.rs @@ -5,7 +5,10 @@ //! DRM's GEM subsystem with shmem backing. use kernel::{ - drm::gem, + drm::{ + gem, + DeviceContext, // + }, prelude::*, // }; @@ -30,7 +33,11 @@ impl gem::DriverObject for BoData { type Driver = TyrDrmDriver; type Args = BoCreateArgs; - fn new(_dev: &TyrDrmDevice, _size: usize, args: BoCreateArgs) -> impl PinInit { + fn new( + _dev: &TyrDrmDevice, + _size: usize, + args: BoCreateArgs, + ) -> impl PinInit { try_pin_init!(Self { flags: args.flags }) } } diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index a47d81a267244..477cf771fb10e 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -177,13 +177,17 @@ impl UnregisteredDevice { master_set: None, master_drop: None, debugfs_init: None, - gem_create_object: T::Object::ALLOC_OPS.gem_create_object, - prime_handle_to_fd: T::Object::ALLOC_OPS.prime_handle_to_fd, - prime_fd_to_handle: T::Object::ALLOC_OPS.prime_fd_to_handle, - gem_prime_import: T::Object::ALLOC_OPS.gem_prime_import, - gem_prime_import_sg_table: T::Object::ALLOC_OPS.gem_prime_import_sg_table, - dumb_create: T::Object::ALLOC_OPS.dumb_create, - dumb_map_offset: T::Object::ALLOC_OPS.dumb_map_offset, + + // Ignore the Uninit DeviceContext below. It is only provided because it is required by the + // compiler, and it is not actually used by these functions. + gem_create_object: T::Object::::ALLOC_OPS.gem_create_object, + prime_handle_to_fd: T::Object::::ALLOC_OPS.prime_handle_to_fd, + prime_fd_to_handle: T::Object::::ALLOC_OPS.prime_fd_to_handle, + gem_prime_import: T::Object::::ALLOC_OPS.gem_prime_import, + gem_prime_import_sg_table: T::Object::::ALLOC_OPS.gem_prime_import_sg_table, + dumb_create: T::Object::::ALLOC_OPS.dumb_create, + dumb_map_offset: T::Object::::ALLOC_OPS.dumb_map_offset, + show_fdinfo: None, fbdev_probe: None, diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index 07af409959a9a..25f7e233884db 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -111,7 +111,7 @@ pub trait Driver { type Data: Sync + Send; /// The type used to manage memory for this driver. - type Object: AllocImpl; + type Object: AllocImpl; /// The type used to represent a DRM File (client) type File: drm::file::DriverFile; diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index d4b5940ec0df2..c8b66d8168719 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -8,6 +8,10 @@ use crate::{ bindings, drm::{ self, + device::{ + DeviceContext, + Registered, // + }, driver::{ AllocImpl, AllocOps, // @@ -22,6 +26,7 @@ use crate::{ types::Opaque, }; use core::{ + marker::PhantomData, ops::Deref, ptr::NonNull, // }; @@ -76,7 +81,8 @@ pub type DriverFile = drm::File<<::Driver as drm::Driver>: /// A type alias for retrieving the current [`AllocImpl`] for a given [`DriverObject`]. /// /// [`Driver`]: drm::Driver -pub type DriverAllocImpl = <::Driver as drm::Driver>::Object; +pub type DriverAllocImpl = + <::Driver as drm::Driver>::Object; /// GEM object functions, which must be implemented by drivers. pub trait DriverObject: Sync + Send + Sized { @@ -87,8 +93,8 @@ pub trait DriverObject: Sync + Send + Sized { type Args; /// Create a new driver data object for a GEM object of a given size. - fn new( - dev: &drm::Device, + fn new( + dev: &drm::Device, size: usize, args: Self::Args, ) -> impl PinInit; @@ -125,9 +131,12 @@ extern "C" fn open_callback( // SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`. let file = unsafe { DriverFile::::from_raw(raw_file) }; - // SAFETY: `open_callback` is specified in the AllocOps structure for `DriverObject`, - // ensuring that `raw_obj` is contained within a `DriverObject` - let obj = unsafe { <::Object as IntoGEMObject>::from_raw(raw_obj) }; + // SAFETY: + // * `open_callback` is specified in the AllocOps structure for `DriverObject`, ensuring that + // `raw_obj` is contained within a `DriverAllocImpl` + // * It is only possible for `open_callback` to be called after device registration, ensuring + // that the object's device is in the `Registered` state. + let obj: &DriverAllocImpl = unsafe { IntoGEMObject::from_raw(raw_obj) }; match T::open(obj, file) { Err(e) => e.to_errno(), @@ -144,12 +153,12 @@ extern "C" fn close_callback( // SAFETY: `close_callback` is specified in the AllocOps structure for `Object`, ensuring // that `raw_obj` is indeed contained within a `Object`. - let obj = unsafe { <::Object as IntoGEMObject>::from_raw(raw_obj) }; + let obj: &DriverAllocImpl = unsafe { IntoGEMObject::from_raw(raw_obj) }; T::close(obj, file); } -impl IntoGEMObject for Object { +impl IntoGEMObject for Object { fn as_raw(&self) -> *mut bindings::drm_gem_object { self.obj.get() } @@ -157,7 +166,7 @@ impl IntoGEMObject for Object { unsafe fn from_raw<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a Self { // SAFETY: `obj` is guaranteed to be in an `Object` via the safety contract of this // function - unsafe { &*crate::container_of!(Opaque::cast_from(self_ptr), Object, obj) } + unsafe { &*crate::container_of!(Opaque::cast_from(self_ptr), Object, obj) } } } @@ -174,7 +183,7 @@ pub trait BaseObject: IntoGEMObject { fn create_handle(&self, file: &drm::File) -> Result where Self: AllocImpl, - D: drm::Driver, + D: drm::Driver = Self, File = F>, F: drm::file::DriverFile, { let mut handle: u32 = 0; @@ -189,7 +198,7 @@ pub trait BaseObject: IntoGEMObject { fn lookup_handle(file: &drm::File, handle: u32) -> Result> where Self: AllocImpl, - D: drm::Driver, + D: drm::Driver = Self, File = F>, F: drm::file::DriverFile, { // SAFETY: The arguments are all valid per the type invariants. @@ -241,16 +250,18 @@ impl BaseObjectPrivate for T {} /// /// # Invariants /// -/// - `self.obj` is a valid instance of a `struct drm_gem_object`. +/// * `self.obj` is a valid instance of a `struct drm_gem_object`. +/// * Any type invariants of `Ctx` apply to the parent DRM device for this GEM object. #[repr(C)] #[pin_data] -pub struct Object { +pub struct Object { obj: Opaque, #[pin] data: T, + _ctx: PhantomData, } -impl Object { +impl Object { const OBJECT_FUNCS: bindings::drm_gem_object_funcs = bindings::drm_gem_object_funcs { free: Some(Self::free_callback), open: Some(open_callback::), @@ -270,11 +281,16 @@ impl Object { }; /// Create a new GEM object. - pub fn new(dev: &drm::Device, size: usize, args: T::Args) -> Result> { + pub fn new( + dev: &drm::Device, + size: usize, + args: T::Args, + ) -> Result> { let obj: Pin> = KBox::pin_init( try_pin_init!(Self { obj: Opaque::new(bindings::drm_gem_object::default()), data <- T::new(dev, size, args), + _ctx: PhantomData, }), GFP_KERNEL, )?; @@ -282,6 +298,8 @@ impl Object { // SAFETY: `obj.as_raw()` is guaranteed to be valid by the initialization above. unsafe { (*obj.as_raw()).funcs = &Self::OBJECT_FUNCS }; + // INVARIANT: `dev` and the GEM object are in the same state at the moment, and upgrading + // the typestate in `dev` will not carry over to the GEM object. if let Err(err) = // SAFETY: The arguments are all valid per the type invariants. to_result(unsafe { @@ -305,13 +323,15 @@ impl Object { } /// Returns the `Device` that owns this GEM object. - pub fn dev(&self) -> &drm::Device { + pub fn dev(&self) -> &drm::Device { // SAFETY: // - `struct drm_gem_object.dev` is initialized and valid for as long as the GEM // object lives. // - The device we used for creating the gem object is passed as &drm::Device to // Object::::new(), so we know that `T::Driver` is the right generic parameter to use // here. + // - Any type invariants of `Ctx` are upheld by using the same `Ctx` for the `Device` we + // return. unsafe { drm::Device::from_raw((*self.as_raw()).dev) } } @@ -336,11 +356,16 @@ impl Object { } } -impl_aref_for_gem_obj!(impl for Object where T: DriverObject); +impl_aref_for_gem_obj! { + impl for Object + where + T: DriverObject, + C: DeviceContext +} -impl super::private::Sealed for Object {} +impl super::private::Sealed for Object {} -impl Deref for Object { +impl Deref for Object { type Target = T; fn deref(&self) -> &Self::Target { @@ -348,7 +373,7 @@ impl Deref for Object { } } -impl AllocImpl for Object { +impl AllocImpl for Object { type Driver = T::Driver; const ALLOC_OPS: AllocOps = AllocOps { diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index 35d7523e164ff..34af402899a0e 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -15,7 +15,9 @@ use crate::{ driver, gem, private::Sealed, - Device, // + Device, + DeviceContext, + Registered, // }, error::to_result, prelude::*, @@ -23,11 +25,12 @@ use crate::{ types::Opaque, // }; use core::{ + marker::PhantomData, ops::{ Deref, DerefMut, // }, - ptr::NonNull, + ptr::NonNull, // }; use gem::{ BaseObjectPrivate, @@ -40,42 +43,49 @@ 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. #[derive(Default)] -pub struct ObjectConfig<'a, T: DriverObject> { +pub struct ObjectConfig<'a, T: DriverObject, C: DeviceContext = Registered> { /// Whether to set the write-combine map flag. pub map_wc: bool, /// Reuse the DMA reservation from another GEM object. /// /// The newly created [`Object`] will hold an owned refcount to `parent_resv_obj` if specified. - pub parent_resv_obj: Option<&'a Object>, + pub parent_resv_obj: Option<&'a Object>, } /// A shmem-backed GEM object. /// /// # Invariants /// -/// `obj` contains a valid initialized `struct drm_gem_shmem_object` for the lifetime of this -/// object. +/// - `obj` contains a valid initialized `struct drm_gem_shmem_object` for the lifetime of this +/// object. +/// - Any type invariants of `C` apply to the parent DRM device for this GEM object. #[repr(C)] #[pin_data] -pub struct Object { +pub struct Object { #[pin] obj: Opaque, /// Parent object that owns this object's DMA reservation object. - parent_resv_obj: Option>>, + parent_resv_obj: Option>>, #[pin] inner: T, + _ctx: PhantomData, } -super::impl_aref_for_gem_obj!(impl for Object where T: DriverObject); +super::impl_aref_for_gem_obj! { + impl for Object + where + T: DriverObject, + C: DeviceContext +} // SAFETY: All GEM objects are thread-safe. -unsafe impl Send for Object {} +unsafe impl Send for Object {} // SAFETY: All GEM objects are thread-safe. -unsafe impl Sync for Object {} +unsafe impl Sync for Object {} -impl Object { +impl Object { /// `drm_gem_object_funcs` vtable suitable for GEM shmem objects. const VTABLE: bindings::drm_gem_object_funcs = bindings::drm_gem_object_funcs { free: Some(Self::free_callback), @@ -106,9 +116,9 @@ impl Object { /// /// Additional config options can be specified using `config`. pub fn new( - dev: &Device, + dev: &Device, size: usize, - config: ObjectConfig<'_, T>, + config: ObjectConfig<'_, T, C>, args: T::Args, ) -> Result> { let new: Pin> = KBox::try_pin_init( @@ -116,6 +126,7 @@ impl Object { obj <- Opaque::init_zeroed(), parent_resv_obj: config.parent_resv_obj.map(|p| p.into()), inner <- T::new(dev, size, args), + _ctx: PhantomData::, }), GFP_KERNEL, )?; @@ -148,7 +159,7 @@ impl Object { } /// Returns the `Device` that owns this GEM object. - pub fn dev(&self) -> &Device { + pub fn dev(&self) -> &Device { // SAFETY: `dev` will have been initialized in `Self::new()` by `drm_gem_shmem_init()`. unsafe { Device::from_raw((*self.as_raw()).dev) } } @@ -168,7 +179,7 @@ impl Object { // SAFETY: // - We verified above that `obj` is valid, which makes `this` valid // - This function is set in AllocOps, so we know that `this` is contained within a - // `Object` + // `Object` let this = unsafe { container_of!(Opaque::cast_from(this), Self, obj) }.cast_mut(); // SAFETY: We're recovering the Kbox<> we created in gem_create_object() @@ -176,7 +187,7 @@ impl Object { } } -impl Deref for Object { +impl Deref for Object { type Target = T; fn deref(&self) -> &Self::Target { @@ -184,15 +195,15 @@ impl Deref for Object { } } -impl DerefMut for Object { +impl DerefMut for Object { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.inner } } -impl Sealed for Object {} +impl Sealed for Object {} -impl gem::IntoGEMObject for Object { +impl gem::IntoGEMObject for Object { fn as_raw(&self) -> *mut bindings::drm_gem_object { // SAFETY: // - Our immutable reference is proof that this is safe to dereference. @@ -200,18 +211,18 @@ impl gem::IntoGEMObject for Object { unsafe { &raw mut (*self.obj.get()).base } } - unsafe fn from_raw<'a>(obj: *mut bindings::drm_gem_object) -> &'a Object { + unsafe fn from_raw<'a>(obj: *mut bindings::drm_gem_object) -> &'a Self { // SAFETY: The safety contract of from_gem_obj() guarantees that `obj` is contained within // `Self` unsafe { let obj = Opaque::cast_from(container_of!(obj, bindings::drm_gem_shmem_object, base)); - &*container_of!(obj, Object, obj) + &*container_of!(obj, Self, obj) } } } -impl driver::AllocImpl for Object { +impl driver::AllocImpl for Object { type Driver = T::Driver; const ALLOC_OPS: driver::AllocOps = driver::AllocOps {