From 7f994b8912eba190ff58e9c8a378d02d13c9eb8a Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:28 +0200 Subject: rust: drm/gem: remove DeviceContext from shmem::Object Now that AlwaysRefCounted is restricted to the Normal GEM Object context, there is no use for instantiating Object with a non-Normal context. Remove the DeviceContext generic parameter from shmem::Object and all associated types (VMap, VMapRef, VMapOwned, DmaResvGuard, SGTableMap), simplifying the API. Reviewed-by: Alexandre Courbot Reviewed-by: Lyude Paul Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-9-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/gem/shmem.rs | 121 ++++++++++++++++++------------------------- 1 file changed, 51 insertions(+), 70 deletions(-) (limited to 'rust/kernel/drm') diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index cf8410e0f228..e0ef47352e88 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -20,9 +20,7 @@ use crate::{ driver, gem, private::Sealed, - Device, - DeviceContext, - Normal, // + Device, // }, error::{ from_err_ptr, @@ -48,7 +46,6 @@ use crate::{ }; use core::{ ffi::c_void, - marker::PhantomData, mem::{ ManuallyDrop, MaybeUninit, // @@ -99,22 +96,20 @@ impl<'a, T: DriverObject> Default for ObjectConfig<'a, T> { /// /// - `obj` contains a valid initialized `struct drm_gem_shmem_object` for the lifetime of this /// object. -/// - Any type invariants of `C` apply to the parent DRM device for this GEM object. #[repr(C)] #[pin_data] -pub struct Object { +pub struct Object { #[pin] obj: Opaque, /// Parent object that owns this object's DMA reservation object. parent_resv_obj: Option>>, /// Devres object for unmapping any SGTable on driver-unbind. - sgt_res: ManuallyDrop>>>, + sgt_res: ManuallyDrop>>>, #[pin] /// Lock for protecting initialization of `sgt_res`. sgt_lock: Mutex<()>, #[pin] inner: T, - _ctx: PhantomData, } super::impl_aref_for_gem_obj! { @@ -124,12 +119,12 @@ super::impl_aref_for_gem_obj! { } // SAFETY: All GEM objects are thread-safe. -unsafe impl Send for Object {} +unsafe impl Send for Object {} // SAFETY: All GEM objects are thread-safe. -unsafe impl Sync for Object {} +unsafe impl Sync for Object {} -impl Object { +impl Object { /// `drm_gem_object_funcs` vtable suitable for GEM shmem objects. const VTABLE: bindings::drm_gem_object_funcs = bindings::drm_gem_object_funcs { free: Some(Self::free_callback), @@ -157,7 +152,7 @@ impl Object { } /// Returns the `Device` that owns this GEM object. - pub fn dev(&self) -> &Device { + pub fn dev(&self) -> &Device { // SAFETY: `dev` will have been initialized in `Self::new()` by `drm_gem_shmem_init()`. unsafe { Device::from_raw((*self.as_raw()).dev) } } @@ -171,8 +166,8 @@ impl Object { // SAFETY: // - We verified above that `obj` is valid, which makes `this` valid - // - This function is set in AllocOps, so we know that `this` is contained within a - // `Object` + // - This function is set in AllocOps, so we know that `this` is contained within an + // `Object` let this = unsafe { container_of!(Opaque::cast_from(base), Self, obj) }.cast_mut(); // We need to drop `sgt_res` first, since doing so requires that the GEM object is still @@ -193,7 +188,7 @@ impl Object { } /// Attempt to create a vmap from the gem object, and confirm the size of said vmap. - fn make_vmap<'a, R, const SIZE: usize>(&'a self) -> Result> + fn make_vmap<'a, R, const SIZE: usize>(&'a self) -> Result> where R: Deref + From<&'a Self>, { @@ -255,7 +250,7 @@ impl Object { /// Creates and returns a virtual kernel memory mapping for this object. #[inline] - pub fn vmap(&self) -> Result> { + pub fn vmap(&self) -> Result> { self.make_vmap() } @@ -298,9 +293,7 @@ impl Object { Ok(sgt_res.access(dev)?) } -} -impl Object { /// Create a new shmem-backed DRM object of the given size. /// /// Additional config options can be specified using `config`. @@ -317,7 +310,6 @@ impl Object { sgt_res: ManuallyDrop::new(SetOnce::new()), sgt_lock <- new_mutex!(()), inner <- T::new(dev, size, args), - _ctx: PhantomData, }), GFP_KERNEL, )?; @@ -351,12 +343,12 @@ impl Object { /// Creates and returns an owned reference to a virtual kernel memory mapping for this object. #[inline] - pub fn owned_vmap(&self) -> Result> { + pub fn owned_vmap(&self) -> Result> { self.make_vmap() } } -impl Deref for Object { +impl Deref for Object { type Target = T; fn deref(&self) -> &Self::Target { @@ -364,15 +356,15 @@ impl Deref for Object { } } -impl DerefMut for Object { +impl DerefMut for Object { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.inner } } -impl Sealed for Object {} +impl Sealed for Object {} -impl gem::IntoGEMObject for Object { +impl gem::IntoGEMObject for Object { fn as_raw(&self) -> *mut bindings::drm_gem_object { // SAFETY: // - Our immutable reference is proof that this is safe to dereference. @@ -391,7 +383,7 @@ impl gem::IntoGEMObject for Object { } } -impl driver::AllocImpl for Object { +impl driver::AllocImpl for Object { type Driver = T::Driver; const ALLOC_OPS: driver::AllocOps = driver::AllocOps { @@ -410,14 +402,11 @@ impl driver::AllocImpl for Object { /// When this is dropped, the `dma_resv` lock is dropped as well. /// // TODO: This should be replace with a WwMutex equivalent once we have such bindings in the kernel. -struct DmaResvGuard<'a, T: DriverObject, C: DeviceContext = Normal>( - &'a Object, - NotThreadSafe, -); +struct DmaResvGuard<'a, T: DriverObject>(&'a Object, NotThreadSafe); -impl<'a, T: DriverObject, C: DeviceContext> DmaResvGuard<'a, T, C> { +impl<'a, T: DriverObject> DmaResvGuard<'a, T> { #[inline] - fn new(obj: &'a Object) -> Self { + fn new(obj: &'a Object) -> Self { // SAFETY: This lock is initialized throughout the lifetime of `object`. unsafe { bindings::dma_resv_lock(obj.raw_dma_resv(), ptr::null_mut()) }; @@ -425,7 +414,7 @@ impl<'a, T: DriverObject, C: DeviceContext> DmaResvGuard<'a, T, C> { } } -impl<'a, T: DriverObject, C: DeviceContext> Drop for DmaResvGuard<'a, T, C> { +impl<'a, T: DriverObject> Drop for DmaResvGuard<'a, T> { #[inline] fn drop(&mut self) { // SAFETY: We are releasing the lock grabbed during the creation of this object. @@ -439,40 +428,37 @@ impl<'a, T: DriverObject, C: DeviceContext> Drop for DmaResvGuard<'a, T, C> { /// /// - The size of `owner` is >= SIZE. /// - The memory pointed to by `addr` remains valid at least until this object is dropped. -pub struct VMap +pub struct VMap where D: DriverObject, - C: DeviceContext, - R: Deref>, + R: Deref>, { addr: *mut c_void, owner: R, } /// An alias type for a reference to a shmem-based GEM object's VMap. -pub type VMapRef<'a, D, C, const SIZE: usize = 0> = VMap, C, SIZE>; +pub type VMapRef<'a, D, const SIZE: usize = 0> = VMap, SIZE>; /// An alias type for an owned reference to a shmem-based GEM object's VMap. -pub type VMapOwned = VMap>, C, SIZE>; +pub type VMapOwned = VMap>, SIZE>; -impl VMap +impl VMap where D: DriverObject, - C: DeviceContext, - R: Deref>, + R: Deref>, { /// Borrows a reference to the object that owns this virtual mapping. #[inline] - pub fn owner(&self) -> &Object { + pub fn owner(&self) -> &Object { &self.owner } } -impl Drop for VMap +impl Drop for VMap where D: DriverObject, - C: DeviceContext, - R: Deref>, + R: Deref>, { #[inline] fn drop(&mut self) { @@ -491,29 +477,26 @@ where // SAFETY: `addr` points to a valid memory address for as long as `owner` exists, meaning that so // long as `owner` is `Send` so is `VMap`. -unsafe impl Send for VMap +unsafe impl Send for VMap where D: DriverObject, - C: DeviceContext, - R: Deref> + Send, + R: Deref> + Send, { } // SAFETY: `addr` points to a valid memory address for as long as `owner` exists, meaning that so // long as `owner` is `Sync` so is `VMap`. -unsafe impl Sync for VMap +unsafe impl Sync for VMap where D: DriverObject, - C: DeviceContext, - R: Deref> + Sync, + R: Deref> + Sync, { } -impl Io for VMap +impl Io for VMap where D: DriverObject, - C: DeviceContext, - R: Deref>, + R: Deref>, { #[inline] fn addr(&self) -> usize { @@ -526,22 +509,20 @@ where } } -impl IoKnownSize for VMap +impl IoKnownSize for VMap where D: DriverObject, - C: DeviceContext, - R: Deref>, + R: Deref>, { const MIN_SIZE: usize = SIZE; } macro_rules! impl_vmap_io_capable { ($ty:ty) => { - impl IoCapable<$ty> for VMap + impl IoCapable<$ty> for VMap where D: DriverObject, - C: DeviceContext, - R: Deref>, + R: Deref>, { #[inline] unsafe fn io_read(&self, address: usize) -> $ty { @@ -584,11 +565,11 @@ impl_vmap_io_capable!(u64); /// [`SGTable`]. /// /// [`SGTable`]: scatterlist::SGTable -pub struct SGTableMap { - obj: NonNull>, +pub struct SGTableMap { + obj: NonNull>, } -impl Deref for SGTableMap { +impl Deref for SGTableMap { type Target = scatterlist::SGTable; fn deref(&self) -> &Self::Target { @@ -599,7 +580,7 @@ impl Deref for SGTableMap { } } -impl Drop for SGTableMap { +impl Drop for SGTableMap { fn drop(&mut self) { // SAFETY: `obj` is always valid via our type invariants let obj = unsafe { self.obj.as_ref() }; @@ -610,8 +591,8 @@ impl Drop for SGTableMap { } } -impl SGTableMap { - fn new(obj: &Object) -> impl Init { +impl SGTableMap { + fn new(obj: &Object) -> impl Init { // INVARIANT: // - We call drm_gem_shmem_get_pages_sgt below and check whether or not it succeeds, // fulfilling the invariant of SGTableMap that the object's `sgt` field is initialized. @@ -625,10 +606,10 @@ impl SGTableMap { // SAFETY: The NonNull in SGTableMap is guaranteed valid by our type invariants, and the GEM object // it points to is guaranteed to be thread-safe. -unsafe impl Send for SGTableMap {} +unsafe impl Send for SGTableMap {} // SAFETY: The NonNull in SGTableMap is guaranteed valid by our type invariants, and the GEM object // it points to is guaranteed to be thread-safe. -unsafe impl Sync for SGTableMap {} +unsafe impl Sync for SGTableMap {} #[kunit_tests(rust_drm_gem_shmem)] mod tests { @@ -705,7 +686,7 @@ mod tests { fn compile_time_vmap_sizes() -> Result { let (_dev, drm) = create_drm_dev()?; - let obj = Object::::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; + let obj = Object::::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; // Try creating a normal vmap obj.vmap::()?; @@ -729,7 +710,7 @@ mod tests { fn vmap_io() -> Result { let (_dev, drm) = create_drm_dev()?; - let obj = Object::::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; + let obj = Object::::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; let vmap = obj.vmap::()?; @@ -761,7 +742,7 @@ mod tests { let reg = faux::Registration::new(c"EvilKunit", None)?; let wrong_dev = reg.as_ref(); - let obj = Object::::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; + let obj = Object::::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; assert_eq!(obj.sg_table(wrong_dev.as_ref()).err().unwrap(), EINVAL); -- cgit v1.2.3