Now that AlwaysRefCounted is restricted to the Normal GEM Object context, there is no use for instantiating Object<T, C> 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.
Signed-off-by: Danilo Krummrich <[email protected]> --- rust/kernel/drm/gem/shmem.rs | 121 +++++++++++++++-------------------- 1 file changed, 51 insertions(+), 70 deletions(-) diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index cf8410e0f228..e0ef47352e88 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -20,9 +20,7 @@ driver, gem, private::Sealed, - Device, - DeviceContext, - Normal, // + Device, // }, error::{ from_err_ptr, @@ -48,7 +46,6 @@ }; use core::{ ffi::c_void, - marker::PhantomData, mem::{ ManuallyDrop, MaybeUninit, // @@ -99,22 +96,20 @@ fn default() -> Self { /// /// - `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 = Normal> { +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>>>, /// 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! { @@ -124,12 +119,12 @@ impl<T> for Object<T> } // 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,7 +152,7 @@ fn as_raw_shmem(&self) -> *mut bindings::drm_gem_shmem_object { } /// 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) } } @@ -171,8 +166,8 @@ extern "C" fn free_callback(obj: *mut bindings::drm_gem_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<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 @@ -193,7 +188,7 @@ extern "C" fn free_callback(obj: *mut bindings::drm_gem_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<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>, { @@ -255,7 +250,7 @@ unsafe fn raw_vunmap(&self, mut map: bindings::iosys_map) { /// Creates and returns a virtual kernel memory mapping for this object. #[inline] - pub fn vmap<const SIZE: usize>(&self) -> Result<VMapRef<'_, T, C, SIZE>> { + pub fn vmap<const SIZE: usize>(&self) -> Result<VMapRef<'_, T, SIZE>> { self.make_vmap() } @@ -298,9 +293,7 @@ pub fn sg_table<'a>( Ok(sgt_res.access(dev)?) } -} -impl<T: DriverObject> Object<T> { /// Create a new shmem-backed DRM object of the given size. /// /// Additional config options can be specified using `config`. @@ -317,7 +310,6 @@ pub fn new( sgt_res: ManuallyDrop::new(SetOnce::new()), sgt_lock <- new_mutex!(()), inner <- T::new(dev, size, args), - _ctx: PhantomData, }), GFP_KERNEL, )?; @@ -351,12 +343,12 @@ pub fn new( /// 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, Normal, SIZE>> { + 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 { @@ -364,15 +356,15 @@ fn deref(&self) -> &Self::Target { } } -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. @@ -391,7 +383,7 @@ unsafe fn from_raw<'a>(obj: *mut bindings::drm_gem_object) -> &'a Self { } } -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 { @@ -410,14 +402,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 = Normal>( - &'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()) }; @@ -425,7 +414,7 @@ fn new(obj: &'a Object<T, C>) -> Self { } } -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 @@ fn drop(&mut self) { /// /// - 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 = Normal, 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) { @@ -491,29 +477,26 @@ fn drop(&mut self) { // 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 { @@ -526,22 +509,20 @@ fn maxsize(&self) -> usize { } } -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 { @@ -584,11 +565,11 @@ unsafe fn io_write(&self, value: $ty, address: usize) { /// [`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 { @@ -599,7 +580,7 @@ fn deref(&self) -> &Self::Target { } } -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() }; @@ -610,8 +591,8 @@ fn drop(&mut self) { } } -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. @@ -625,10 +606,10 @@ fn new(obj: &Object<T, C>) -> impl Init<Self, Error> { // 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 { @@ -705,7 +686,7 @@ fn create_drm_dev() -> Result<(faux::Registration, UnregisteredDevice<KunitDrive 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>()?; @@ -729,7 +710,7 @@ fn compile_time_vmap_sizes() -> Result { 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>()?; @@ -761,7 +742,7 @@ fn fail_sg_table_on_wrong_dev() -> Result { 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); -- 2.54.0
