From fea3a2dd7d3fc1936211ced5f84420e610435730 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Wed, 3 Jun 2026 15:42:30 -0400 Subject: rust: drm: gem: shmem: Fix Default implementation for ObjectConfig I completely forgot when coming up with this type that #[derive(Default)] only works if all generics mentioned in the type implement Default (and T usually doesn't). This being said: We don't use `T` for anything besides using it for a reference type, so whether or not it implements `Default` shouldn't actually need to matter. So, fix this by just manually implementing Default instead of deriving it. Signed-off-by: Lyude Paul Link: https://patch.msgid.link/20260603195210.693856-2-lyude@redhat.com Signed-off-by: Alice Ryhl --- rust/kernel/drm/gem/shmem.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index 34af402899a0..084b798ce795 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -42,7 +42,6 @@ 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, C: DeviceContext = Registered> { /// Whether to set the write-combine map flag. pub map_wc: bool, @@ -53,6 +52,16 @@ pub struct ObjectConfig<'a, T: DriverObject, C: DeviceContext = Registered> { pub parent_resv_obj: Option<&'a Object>, } +impl<'a, T: DriverObject, C: DeviceContext> Default for ObjectConfig<'a, T, C> { + #[inline(always)] + fn default() -> Self { + Self { + map_wc: false, + parent_resv_obj: None, + } + } +} + /// A shmem-backed GEM object. /// /// # Invariants -- cgit v1.2.3 From 5a22c80ae8f942d4b560d6a710a037f424e33b9a Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Fri, 12 Jun 2026 15:43:33 -0400 Subject: rust: drm: gem: shmem: Add DmaResvGuard helper Just a temporary holdover to make locking/unlocking the dma_resv lock much easier. Signed-off-by: Lyude Paul Co-Authored-By: Alexandre Courbot Signed-off-by: Alexandre Courbot Reviewed-by: Alice Ryhl Acked-by: Danilo Krummrich Link: https://patch.msgid.link/20260612194436.585385-2-lyude@redhat.com --- rust/kernel/drm/gem/shmem.rs | 39 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index 084b798ce795..090c5d869fdb 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -22,7 +22,10 @@ use crate::{ error::to_result, prelude::*, sync::aref::ARef, - types::Opaque, // + types::{ + NotThreadSafe, + Opaque, // + }, }; use core::{ marker::PhantomData, @@ -30,7 +33,10 @@ use core::{ Deref, DerefMut, // }, - ptr::NonNull, // + ptr::{ + self, + NonNull, // + }, }; use gem::{ BaseObjectPrivate, @@ -244,3 +250,32 @@ impl driver::AllocImpl for Object { dumb_map_offset: None, }; } + +/// Private helper-type for holding the `dma_resv` object for a GEM shmem 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 = Registered>( + &'a Object, + NotThreadSafe, +); + +impl<'a, T: DriverObject, C: DeviceContext> DmaResvGuard<'a, T, C> { + #[inline] + #[expect(unused)] + 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()) }; + + Self(obj, NotThreadSafe) + } +} + +impl<'a, T: DriverObject, C: DeviceContext> Drop for DmaResvGuard<'a, T, C> { + #[inline] + fn drop(&mut self) { + // SAFETY: We are releasing the lock grabbed during the creation of this object. + unsafe { bindings::dma_resv_unlock(self.0.raw_dma_resv()) }; + } +} -- cgit v1.2.3 From d055768429b3a49090e5f633fa45d7b964fc23ec Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Fri, 12 Jun 2026 15:43:34 -0400 Subject: rust: drm: gem: shmem: Add vmap functions One of the more obvious use cases for gem shmem objects is the ability to create mappings into their contents. So, let's hook this up in our rust bindings. Signed-off-by: Lyude Paul Reviewed-by: Alexandre Courbot Reviewed-by: Alice Ryhl Acked-by: Danilo Krummrich Link: https://patch.msgid.link/20260612194436.585385-3-lyude@redhat.com --- rust/kernel/drm/gem/shmem.rs | 338 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 337 insertions(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index 090c5d869fdb..a38c98add3d1 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -20,6 +20,11 @@ use crate::{ Registered, // }, error::to_result, + io::{ + Io, + IoCapable, + IoKnownSize, // + }, prelude::*, sync::aref::ARef, types::{ @@ -28,7 +33,9 @@ use crate::{ }, }; use core::{ + ffi::c_void, marker::PhantomData, + mem::MaybeUninit, // ops::{ Deref, DerefMut, // @@ -39,6 +46,7 @@ use core::{ }, }; use gem::{ + BaseObject, BaseObjectPrivate, DriverObject, IntoGEMObject, // @@ -200,6 +208,79 @@ impl Object { // SAFETY: We're recovering the Kbox<> we created in gem_create_object() let _ = unsafe { KBox::from_raw(this) }; } + + /// 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> + where + R: Deref + From<&'a Self>, + { + // INVARIANT: We check here that the gem object is at least as large as `SIZE`. + if self.size() < SIZE { + return Err(ENOSPC); + } + + let mut map: MaybeUninit = MaybeUninit::uninit(); + let guard = DmaResvGuard::new(self); + + // SAFETY: `drm_gem_shmem_vmap()` can be called with the DMA reservation lock held. + to_result(unsafe { + bindings::drm_gem_shmem_vmap_locked(self.as_raw_shmem(), map.as_mut_ptr()) + })?; + + // Drop the guard explicitly here, since we may need to call `raw_vunmap()` (which + // re-acquires the lock). + drop(guard); + + // SAFETY: The call to `drm_gem_shmem_vmap_locked()` succeeded above, so we are guaranteed + // that map is properly initialized. + let map = unsafe { map.assume_init() }; + + // XXX: We don't currently support iomem allocations + if map.is_iomem { + // SAFETY: The vmap operation above succeeded, guaranteeing that `map` points to a valid + // memory mapping. + unsafe { self.raw_vunmap(map) }; + + Err(ENOTSUPP) + } else { + Ok(VMap { + // INVARIANT: `addr` remains valid for as long as `owner` does, which extends to the + // lifetime of `VMap` itself. + // SAFETY: We checked that this is not an iomem allocation, making it safe to read + // vaddr. + addr: unsafe { map.__bindgen_anon_1.vaddr }, + owner: self.into(), + }) + } + } + + /// Unmap a vmap from the gem object. + /// + /// # Safety + /// + /// - The caller promises that `map` is a valid vmap on this gem object. + /// - The caller promises that the memory pointed to by map will no longer be accesed through + /// this instance. + unsafe fn raw_vunmap(&self, mut map: bindings::iosys_map) { + let _guard = DmaResvGuard::new(self); + + // SAFETY: + // - This function is safe to call with the DMA reservation lock held. + // - The caller promises that `map` is a valid vmap on this gem object. + unsafe { bindings::drm_gem_shmem_vunmap_locked(self.as_raw_shmem(), &mut map) }; + } + + /// Creates and returns a virtual kernel memory mapping for this object. + #[inline] + pub fn vmap(&self) -> Result> { + self.make_vmap() + } + + /// Creates and returns an owned reference to a virtual kernel memory mapping for this object. + #[inline] + pub fn owned_vmap(&self) -> Result> { + self.make_vmap() + } } impl Deref for Object { @@ -263,7 +344,6 @@ struct DmaResvGuard<'a, T: DriverObject, C: DeviceContext = Registered>( impl<'a, T: DriverObject, C: DeviceContext> DmaResvGuard<'a, T, C> { #[inline] - #[expect(unused)] 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()) }; @@ -279,3 +359,259 @@ impl<'a, T: DriverObject, C: DeviceContext> Drop for DmaResvGuard<'a, T, C> { unsafe { bindings::dma_resv_unlock(self.0.raw_dma_resv()) }; } } + +/// A reference to a virtual mapping for an shmem-based GEM object in kernel address space. +/// +/// # Invariants +/// +/// - The size of `owner` is >= SIZE. +/// - The memory pointed to by `addr` remains valid at least until this object is dropped. +pub struct VMap +where + D: DriverObject, + C: DeviceContext, + 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>; + +/// An alias type for an owned reference to a shmem-based GEM object's VMap. +pub type VMapOwned = VMap>, C, SIZE>; + +impl VMap +where + D: DriverObject, + C: DeviceContext, + R: Deref>, +{ + /// Borrows a reference to the object that owns this virtual mapping. + #[inline] + pub fn owner(&self) -> &Object { + &self.owner + } +} + +impl Drop for VMap +where + D: DriverObject, + C: DeviceContext, + R: Deref>, +{ + #[inline] + fn drop(&mut self) { + // SAFETY: + // - Our existence is proof that this map was previously created using self.owner. + // - Since we are in Drop, we are guaranteed that no one will access the memory + // through this mapping after calling this. + unsafe { + self.owner.raw_vunmap(bindings::iosys_map { + is_iomem: false, + __bindgen_anon_1: bindings::iosys_map__bindgen_ty_1 { vaddr: self.addr }, + }) + }; + } +} + +// 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 +where + D: DriverObject, + C: DeviceContext, + 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 +where + D: DriverObject, + C: DeviceContext, + R: Deref> + Sync, +{ +} + +impl Io for VMap +where + D: DriverObject, + C: DeviceContext, + R: Deref>, +{ + #[inline] + fn addr(&self) -> usize { + self.addr as usize + } + + #[inline] + fn maxsize(&self) -> usize { + self.owner.size() + } +} + +impl IoKnownSize for VMap +where + D: DriverObject, + C: DeviceContext, + R: Deref>, +{ + const MIN_SIZE: usize = SIZE; +} + +macro_rules! impl_vmap_io_capable { + ($ty:ty) => { + impl IoCapable<$ty> for VMap + where + D: DriverObject, + C: DeviceContext, + R: Deref>, + { + #[inline] + unsafe fn io_read(&self, address: usize) -> $ty { + let ptr = address as *mut $ty; + + // SAFETY: The safety contract of `io_read` guarantees that address is a valid + // address within the bounds of `Self` of at least the size of $ty, and is properly + // aligned. + unsafe { ptr::read_volatile(ptr) } + } + + #[inline] + unsafe fn io_write(&self, value: $ty, address: usize) { + let ptr = address as *mut $ty; + + // SAFETY: The safety contract of `io_write` guarantees that address is a valid + // address within the bounds of `Self` of at least the size of $ty, and is properly + // aligned. + unsafe { ptr::write_volatile(ptr, value) } + } + } + }; +} + +impl_vmap_io_capable!(u8); +impl_vmap_io_capable!(u16); +impl_vmap_io_capable!(u32); +#[cfg(CONFIG_64BIT)] +impl_vmap_io_capable!(u64); + +#[kunit_tests(rust_drm_gem_shmem)] +mod tests { + use super::*; + use crate::{ + drm::{ + self, + UnregisteredDevice, // + }, + faux, + page::PAGE_SIZE, // + }; + + // The bare minimum needed to create a fake drm driver for kunit + + #[pin_data] + struct KunitData {} + struct KunitDriver; + struct KunitFile; + #[pin_data] + struct KunitObject {} + + const INFO: drm::DriverInfo = drm::DriverInfo { + major: 0, + minor: 0, + patchlevel: 0, + name: c"kunit", + desc: c"Kunit", + }; + + impl drm::file::DriverFile for KunitFile { + type Driver = KunitDriver; + + fn open(_dev: &drm::Device) -> Result>> { + Ok(KBox::new(Self, GFP_KERNEL)?.into()) + } + } + + impl gem::DriverObject for KunitObject { + type Driver = KunitDriver; + type Args = (); + + fn new( + _dev: &drm::Device, + _size: usize, + _args: Self::Args, + ) -> impl PinInit { + try_pin_init!(KunitObject {}) + } + } + + #[vtable] + impl drm::Driver for KunitDriver { + type Data = KunitData; + type File = KunitFile; + type Object = Object; + + const INFO: drm::DriverInfo = INFO; + const IOCTLS: &'static [drm::ioctl::DrmIoctlDescriptor] = &[]; + } + + fn create_drm_dev() -> Result<(faux::Registration, UnregisteredDevice)> { + // 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)?; + + Ok((dev, drm)) + } + + #[test] + fn compile_time_vmap_sizes() -> Result { + let (_dev, drm) = create_drm_dev()?; + + let obj = Object::::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; + + // Try creating a normal vmap + obj.vmap::()?; + + // Try creating a vmap that's smaller then the size we specified + let vmap = obj.vmap::<{ PAGE_SIZE - 100 }>()?; + + // Verify the owner matches + assert!(ptr::eq(vmap.owner(), obj.deref())); + + // Verify the max size matches the actual object size + assert_eq!(vmap.maxsize(), PAGE_SIZE); + + // Make sure creating a vmap that's too large fails + assert!(obj.vmap::<{ PAGE_SIZE + 200 }>().is_err()); + + Ok(()) + } + + #[test] + fn vmap_io() -> Result { + let (_dev, drm) = create_drm_dev()?; + + let obj = Object::::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; + + let vmap = obj.vmap::()?; + + vmap.write8(0xDE, 0x0); + assert_eq!(vmap.read8(0x0), 0xDE); + vmap.write32(0xFEDCBA98, 0x20); + + assert_eq!(vmap.read32(0x20), 0xFEDCBA98); + + // Ensure the ordering in memory is correct + let expected = 0xFEDCBA98_u32.to_ne_bytes().into_iter(); + for (offset, expected) in (0x20..=0x23).zip(expected) { + assert_eq!(vmap.read8(offset), expected); + } + + Ok(()) + } +} -- cgit v1.2.3 From 20003c1a1fd80ae1b1742ff54297b67f7f21b77f Mon Sep 17 00:00:00 2001 From: Deborah Brouwer Date: Wed, 10 Jun 2026 17:01:26 -0700 Subject: rust: drm: gpuvm: update DriverGpuVm for DeviceContext Since the introduction of DeviceContext, there is no longer a single driver object type to equate with the GPUVM object type. Instead of threading DeviceContext through GPUVM, remove the strict identity between DriverGpuVm::Object and drm::Driver::Object and instead tighten the requirement that the DriverGpuVm::Object be an allocatable GEM object associated with the same DRM driver. Also, make GpuVm::new() generic over DeviceContext so it can accept a drm::Device. Fixes: 0023a1e8d01a ("rust/drm/gem: Use DeviceContext with GEM objects") Signed-off-by: Deborah Brouwer Reviewed-by: Alice Ryhl Reviewed-by: Sami Tolvanen Link: https://patch.msgid.link/20260610-gpuvm_device_context_v1-v1-1-01a890b17448@collabora.com Signed-off-by: Danilo Krummrich --- rust/kernel/drm/gpuvm/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs index ae58f6f667c1..a625fcd9b5f2 100644 --- a/rust/kernel/drm/gpuvm/mod.rs +++ b/rust/kernel/drm/gpuvm/mod.rs @@ -116,9 +116,9 @@ impl GpuVm { /// Creates a GPUVM instance. #[expect(clippy::new_ret_no_self)] - pub fn new( + pub fn new( name: &'static CStr, - dev: &drm::Device, + dev: &drm::Device, r_obj: &T::Object, range: Range, reserve_range: Range, @@ -252,10 +252,10 @@ impl GpuVm { /// The manager for a GPUVM. pub trait DriverGpuVm: Sized + Send { /// Parent `Driver` for this object. - type Driver: drm::Driver; + type Driver: drm::Driver; /// The kind of GEM object stored in this GPUVM. - type Object: IntoGEMObject; + type Object: drm::driver::AllocImpl; /// Data stored with each [`struct drm_gpuva`](struct@GpuVa). type VaData; -- cgit v1.2.3 From 5f7410aa26524101d34b627fbe16670b1514962c Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Wed, 10 Jun 2026 17:04:31 -0700 Subject: rust: drm: gpuvm: add SmContext lifetime bound If a DriverGpuVm implementation is lifetime-parameterized, its SmContext<'ctx> type may depend on lifetimes carried by that driver implementation. In this case, SmContext<'ctx> is only valid if the driver implementation outlives 'ctx. Add a Self: 'ctx bound to DriverGpuVm::SmContext<'ctx> to express that requirement. Then propagate the corresponding T: 'ctx bound to the GPUVM state machine helper types that store T::SmContext<'ctx>. This allows drivers to provide lifetime-parameterized implementations of DriverGpuVm. Signed-off-by: Boris Brezillon Signed-off-by: Deborah Brouwer Reviewed-by: Alice Ryhl Link: https://patch.msgid.link/20260610-gpuvm_smcontext_lifetime_bound_v1-v1-1-531e7d2ee7b4@collabora.com Signed-off-by: Danilo Krummrich --- rust/kernel/drm/gpuvm/mod.rs | 4 +++- rust/kernel/drm/gpuvm/sm_ops.rs | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/gpuvm/mod.rs b/rust/kernel/drm/gpuvm/mod.rs index a625fcd9b5f2..20a08b3defeb 100644 --- a/rust/kernel/drm/gpuvm/mod.rs +++ b/rust/kernel/drm/gpuvm/mod.rs @@ -264,7 +264,9 @@ pub trait DriverGpuVm: Sized + Send { type VmBoData; /// The private data passed to callbacks. - type SmContext<'ctx>; + type SmContext<'ctx> + where + Self: 'ctx; /// Indicates that a new mapping should be created. fn sm_step_map<'op, 'ctx>( diff --git a/rust/kernel/drm/gpuvm/sm_ops.rs b/rust/kernel/drm/gpuvm/sm_ops.rs index 69a8e5ab2821..742c151b2540 100644 --- a/rust/kernel/drm/gpuvm/sm_ops.rs +++ b/rust/kernel/drm/gpuvm/sm_ops.rs @@ -3,7 +3,7 @@ use super::*; /// The actual data that gets threaded through the callbacks. -struct SmData<'a, 'ctx, T: DriverGpuVm> { +struct SmData<'a, 'ctx, T: DriverGpuVm + 'ctx> { gpuvm: &'a mut UniqueRefGpuVm, user_context: &'a mut T::SmContext<'ctx>, } @@ -20,7 +20,7 @@ struct SmMapData<'a, 'ctx, T: DriverGpuVm> { } /// The argument for [`UniqueRefGpuVm::sm_map`]. -pub struct OpMapRequest<'a, 'ctx, T: DriverGpuVm> { +pub struct OpMapRequest<'a, 'ctx, T: DriverGpuVm + 'ctx> { /// Address in GPU virtual address space. pub addr: u64, /// Length of mapping to create. -- cgit v1.2.3 From 616c229ab010a31c5d1f793b925c7fb2eaad664c Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Fri, 12 Jun 2026 15:43:36 -0400 Subject: rust: drm: gem: Introduce shmem::Object::sg_table() In order to do this, we need to be careful to ensure that any interface we expose for scatterlists ensures that any mappings created from one are destroyed on driver-unbind. To do this, we introduce a Devres resource into shmem::Object that we use in order to ensure that we release any SGTable mappings on driver-unbind. There's some other slightly unfortunate caveats of this: * Drivers don't have explicit control at the moment over when unmapping happens (which is exactly the same as the C side atm, so it might not be a problem). * We can't just return `SGTableMap` to the user through an Arc to attempt to fix the last caveat - because that implies the gem object would need to hold a reference count to the scatterlist mapping, which just leaves us with the same problem. Signed-off-by: Lyude Paul Reviewed-by: Alexandre Courbot Acked-by: Danilo Krummrich Link: https://patch.msgid.link/20260612194436.585385-5-lyude@redhat.com --- rust/kernel/drm/gem/shmem.rs | 174 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 164 insertions(+), 10 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index a38c98add3d1..3ee19ef6264e 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -11,6 +11,11 @@ use crate::{ container_of, + device::{ + self, + Bound, // + }, + devres::*, drm::{ driver, gem, @@ -19,14 +24,23 @@ use crate::{ DeviceContext, Registered, // }, - error::to_result, + error::{ + from_err_ptr, + to_result, // + }, io::{ Io, IoCapable, IoKnownSize, // }, prelude::*, - sync::aref::ARef, + scatterlist, + sync::{ + aref::ARef, + new_mutex, + Mutex, + SetOnce, // + }, types::{ NotThreadSafe, Opaque, // @@ -35,7 +49,10 @@ use crate::{ use core::{ ffi::c_void, marker::PhantomData, - mem::MaybeUninit, // + mem::{ + ManuallyDrop, + MaybeUninit, // + }, ops::{ Deref, DerefMut, // @@ -90,6 +107,11 @@ pub struct Object { 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>>>, + #[pin] + /// Lock for protecting initialization of `sgt_res`. + sgt_lock: Mutex<()>, #[pin] inner: T, _ctx: PhantomData, @@ -148,6 +170,8 @@ impl Object { 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::, }), @@ -192,18 +216,26 @@ impl Object { // - DRM always passes a valid gem object here // - We used drm_gem_shmem_create() in our create_gem_object callback, so we know that // `obj` is contained within a drm_gem_shmem_object - let this = unsafe { container_of!(obj, bindings::drm_gem_shmem_object, base) }; - - // SAFETY: - // - We're in free_callback - so this function is safe to call. - // - We won't be using the gem resources on `this` after this call. - unsafe { bindings::drm_gem_shmem_release(this) }; + let base = unsafe { container_of!(obj, bindings::drm_gem_shmem_object, base) }; // 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` - let this = unsafe { container_of!(Opaque::cast_from(this), Self, obj) }.cast_mut(); + 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 + // alive. + // SAFETY: + // - We verified above that `this` is valid. + // - We are in free_callback, guaranteeing we have exclusive access to `this` and that + // `sgt_res` will not be used after dropping it here. + unsafe { ManuallyDrop::drop(&mut (*this).sgt_res) }; + + // SAFETY: + // - We're in free_callback - so this function is safe to call. + // - We won't be using the gem resources on `this` after this call. + unsafe { bindings::drm_gem_shmem_release(base) }; // SAFETY: We're recovering the Kbox<> we created in gem_create_object() let _ = unsafe { KBox::from_raw(this) }; @@ -281,6 +313,46 @@ impl Object { pub fn owned_vmap(&self) -> Result> { self.make_vmap() } + + /// Creates (if necessary) and returns an immutable reference to a scatter-gather table of DMA + /// pages for this object. + /// + /// This will pin the object in memory. It is expected that `dev` should be a pointer to the + /// same [`device::Device`] which `self` belongs to, otherwise this function will return + /// `Err(EINVAL)`. + pub fn sg_table<'a>( + &'a self, + dev: &'a device::Device, + ) -> Result<&'a scatterlist::SGTable> { + if dev.as_raw() != self.dev().as_ref().as_raw() { + return Err(EINVAL); + } + + let sgt_res = 'out: { + // Fast path: sgt_res is already initialized + if let Some(sgt_res) = self.sgt_res.as_ref() { + break 'out sgt_res; + } + + // Slow path: Grab the lock and see if we need to initialize sgt_res. + let _guard = self.sgt_lock.lock(); + + // If someone initialized it while we were waiting, we can exit early. + if let Some(sgt_res) = self.sgt_res.as_ref() { + break 'out sgt_res; + } + + // If not, finish initializing and return. `populate()` cannot return false, as + // `sgt_res` must be unpopulated, and we must hold `sgt_lock` to reach this point. + self.sgt_res + .populate(Devres::new(dev, SGTableMap::new(self))?); + + // SAFETY: We just populated sgt_res above. + unsafe { self.sgt_res.as_ref().unwrap_unchecked() } + }; + + Ok(sgt_res.access(dev)?) + } } impl Deref for Object { @@ -499,6 +571,64 @@ impl_vmap_io_capable!(u32); #[cfg(CONFIG_64BIT)] impl_vmap_io_capable!(u64); +/// A reference to a GEM object that is known to have a mapped [`SGTable`]. +/// +/// This is used by the Rust bindings with [`Devres`] in order to ensure that mappings for SGTables +/// on GEM shmem objects are revoked on driver-unbind. +/// +/// # Invariants +/// +/// - `self.obj` always points to a valid GEM object. +/// - This object is proof that `self.obj.owner.sgt_res` has an initialized and valid pointer to an +/// [`SGTable`]. +/// +/// [`SGTable`]: scatterlist::SGTable +pub struct SGTableMap { + obj: NonNull>, +} + +impl Deref for SGTableMap { + type Target = scatterlist::SGTable; + + fn deref(&self) -> &Self::Target { + // SAFETY: + // - The NonNull is guaranteed to be valid via our type invariants. + // - The sgt field is guaranteed to be initialized and valid via our type invariants. + unsafe { scatterlist::SGTable::from_raw((*self.obj.as_ref().as_raw_shmem()).sgt) } + } +} + +impl Drop for SGTableMap { + fn drop(&mut self) { + // SAFETY: `obj` is always valid via our type invariants + let obj = unsafe { self.obj.as_ref() }; + let _lock = DmaResvGuard::new(obj); + + // SAFETY: We acquired the lock needed for calling this function above + unsafe { bindings::__drm_gem_shmem_free_sgt_locked(obj.as_raw_shmem()) }; + } +} + +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. + // SAFETY: + // - `obj` is fully initialized, making this function safe to call. + from_err_ptr(unsafe { bindings::drm_gem_shmem_get_pages_sgt(obj.as_raw_shmem()) })?; + + Ok(Self { obj: obj.into() }) + } +} + +// 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 {} +// 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 {} + #[kunit_tests(rust_drm_gem_shmem)] mod tests { use super::*; @@ -614,4 +744,28 @@ mod tests { Ok(()) } + + // TODO: I would love to actually test the success paths of sg_table(), but that would require + // also implementing dummy dma_ops so that trying to create a mapping doesn't explode. So, leave + // that for someone else. + + // Ensures that passing the wrong device to sg_table() fails as we expect, and also ensure it + // skips initializing `sgt_res` since we could otherwise create `sgt_res` with the wrong device + // bound to it. + #[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 obj = Object::::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; + + assert_eq!(obj.sg_table(wrong_dev.as_ref()).err().unwrap(), EINVAL); + + // If sgt_res was not initialized mistakenly with the wrong device, this should still fail. + assert_eq!(obj.sg_table(wrong_dev.as_ref()).err().unwrap(), EINVAL); + + // TODO: Someday, we should test that creating an sg_table here still succeeds. + + Ok(()) + } } -- cgit v1.2.3 From fa8cc4e3067f958ea2057f37a8a6f9c6b10a9c03 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Fri, 12 Jun 2026 15:43:35 -0400 Subject: rust: faux: Allow retrieving a bound Device When writing up some rust code that used faux devices for unit testing, I noticed that we never actually added the Bound device context to faux::Registration's AsRef implementation. This being said: the Registration object itself is proof that a driver is bound to the device - so this should be safe. Signed-off-by: Lyude Paul Reviewed-by: Alexandre Courbot Reviewed-by: Alice Ryhl Acked-by: Danilo Krummrich Link: https://patch.msgid.link/20260612194436.585385-4-lyude@redhat.com Signed-off-by: Danilo Krummrich --- rust/kernel/faux.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/faux.rs b/rust/kernel/faux.rs index 43b4974f48cd..36c92ae2943c 100644 --- a/rust/kernel/faux.rs +++ b/rust/kernel/faux.rs @@ -25,7 +25,8 @@ use core::ptr::{ /// /// # Invariants /// -/// `self.0` always holds a valid pointer to an initialized and registered [`struct faux_device`]. +/// - `self.0` always holds a valid pointer to an initialized and registered [`struct faux_device`]. +/// - This object is proof that the object described by this `Registration` is bound to a device. /// /// [`struct faux_device`]: srctree/include/linux/device/faux.h pub struct Registration(NonNull); @@ -59,10 +60,17 @@ impl Registration { } } -impl AsRef for Registration { - fn as_ref(&self) -> &device::Device { - // SAFETY: The underlying `device` in `faux_device` is guaranteed by the C API to be - // a valid initialized `device`. +impl AsRef> for Registration { + fn as_ref(&self) -> &device::Device { + // SAFETY: + // - The underlying `device` in `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). + // - `suppress_bind_attrs = true` on faux_driver prevents userspace-triggered unbind via + // 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)) } } } -- cgit v1.2.3 From 68b151bc6145dea3db5598ebaf4b776cd205e395 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:21 +0200 Subject: rust: drm: ioctl: fix unbounded lifetimes in ioctl handler arguments References to dev, data, and file in the declare_drm_ioctls! macro are created via unsafe pointer dereferences, producing unbounded lifetimes. If an ioctl handler explicitly annotates its parameters with 'static, the compiler accepts this, allowing the handler to stash references that outlive the ioctl call. Fix this by adding a higher-ranked function pointer coercion that enforces the handler accepts universally quantified lifetimes: let _: for<'a> fn(&'a _, &'a mut _, &'a _) -> _ = $func; Since the handler must be coercible to a function pointer accepting any lifetime 'a, it can no longer demand 'static on any parameter. Cc: stable@vger.kernel.org Fixes: 9a69570682b1 ("rust: drm: ioctl: Add DRM ioctl abstraction") Reported-by: sashiko-bot@kernel.org Closes: https://lore.kernel.org/all/20260620011346.A47D01F000E9@smtp.kernel.org/ Suggested-by: Gary Guo Reviewed-by: Alexandre Courbot Reviewed-by: Lyude Paul Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-2-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/ioctl.rs | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs index cf328101dde4..ccf4150d83b6 100644 --- a/rust/kernel/drm/ioctl.rs +++ b/rust/kernel/drm/ioctl.rs @@ -135,6 +135,12 @@ macro_rules! declare_drm_ioctls { // 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); + + // 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 mut _, &'a _) -> _ = $func; + // 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. -- cgit v1.2.3 From fd0f827c532b976d38e3cbbda5d3fa60a82ce8d5 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:22 +0200 Subject: rust: drm: rename Uninit DeviceContext to Normal Rename the Uninit DeviceContext to Normal to better reflect its purpose as the general-purpose, reference-counted device context. The Uninit name was a leftover from when DRM device private data initialization was planned to split across UnregisteredDevice::new() and Registration::new(); with the subsequent introduction of RegistrationData, this distinction is no longer needed. This also simplifies the DeviceContext documentation, trimming the multi-stage initialization description that no longer applies. Subsequent patches will refine the semantics of the Registered context accordingly. No functional change. Reviewed-by: Lyude Paul Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-3-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/device.rs | 92 ++++++++++++++--------------------------------- rust/kernel/drm/mod.rs | 2 +- 2 files changed, 28 insertions(+), 66 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 7ad124327a83..35ff9c6942d8 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -74,36 +74,22 @@ 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` would be a [`Device`] that reached the [`Registered`] [`DeviceContext`]. +/// A [`Device`] can be in one of two contexts: /// -/// Each stage of this process is described below: -/// -/// ```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` may actually be registered with userspace, it just wasn't known to be -/// registered at the time the reference was taken. +/// - [`Normal`]: The general-purpose, reference-counted context. A [`Device`] in this context may +/// or may not be registered with userspace. +/// - [`Registered`]: The device has been registered with userspace at some point. pub trait DeviceContext: Sealed + Send + Sync + 'static {} +/// The general-purpose, reference-counted [`DeviceContext`]. +/// +/// 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`]. +pub struct Normal; + +impl Sealed for Normal {} +impl DeviceContext for Normal {} + /// The [`DeviceContext`] of a [`Device`] that was registered with userspace at some point. /// /// This represents a [`Device`] which is guaranteed to have been registered with userspace at @@ -121,20 +107,6 @@ pub struct Registered; impl Sealed for Registered {} impl DeviceContext for Registered {} -/// The [`DeviceContext`] of a [`Device`] that may be unregistered and partly uninitialized. -/// -/// 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`]. -/// -/// A [`Device`] in this context can be used to create a -/// [`Registration`](drm::driver::Registration). -pub struct Uninit; - -impl Sealed for Uninit {} -impl DeviceContext for Uninit {} - /// A [`Device`] which is known at compile-time to be unregistered with userspace. /// /// This type allows performing operations which are only safe to do before userspace registration, @@ -147,10 +119,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(ARef>, NotThreadSafe); +pub struct UnregisteredDevice(ARef>, NotThreadSafe); impl Deref for UnregisteredDevice { - type Target = Device; + type Target = Device; fn deref(&self) -> &Self::Target { &self.0 @@ -178,15 +150,13 @@ impl UnregisteredDevice { 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::::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, + 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, @@ -211,7 +181,7 @@ impl UnregisteredDevice { pub fn new(dev: &device::Device, data: impl PinInit) -> Result { // `__drm_dev_alloc` uses `kmalloc()` to allocate memory, hence ensure a `kmalloc()` // compatible `Layout`. - let layout = Kmalloc::aligned_layout(Layout::new::>()); + let layout = Kmalloc::aligned_layout(Layout::new::>()); // 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 +193,12 @@ impl UnregisteredDevice { // SAFETY: // - `alloc_vtable` reference remains valid until no longer used, // - `dev` is valid by its type invarants, - let raw_drm: *mut Device = unsafe { + let raw_drm: *mut Device = unsafe { bindings::__drm_dev_alloc( dev.as_raw(), &alloc_vtable, layout.size(), - mem::offset_of!(Device, dev), + mem::offset_of!(Device, dev), ) } .cast(); @@ -264,16 +234,8 @@ impl UnregisteredDevice { /// 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 guaranteed to have been registered with userspace +/// at some point. The [`Normal`] context is the general-purpose, reference-counted context. /// /// # Invariants /// diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs index a66e7166f66b..e5bfaf130342 100644 --- a/rust/kernel/drm/mod.rs +++ b/rust/kernel/drm/mod.rs @@ -11,8 +11,8 @@ pub mod ioctl; pub use self::device::Device; pub use self::device::DeviceContext; +pub use self::device::Normal; pub use self::device::Registered; -pub use self::device::Uninit; pub use self::device::UnregisteredDevice; pub use self::driver::Driver; pub use self::driver::DriverInfo; -- cgit v1.2.3 From 893f39dadaa172c0f869e5b138f2cfdd851df781 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:23 +0200 Subject: rust: faux: add Device type with AsBusDevice support Add a faux::Device type that wraps struct faux_device and implements AsBusDevice, enabling faux devices to be used as parent devices for subsystems that require a bus device, such as DRM. Update Registration to return &faux::Device via AsRef. Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-4-dakr@kernel.org [ Drop redundant 'struct device' invariant; implied by valid struct faux_device. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/drm/gem/shmem.rs | 11 ++++--- rust/kernel/faux.rs | 69 +++++++++++++++++++++++++++++++++++++------- 2 files changed, 66 insertions(+), 14 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index 3ee19ef6264e..52de59b14dad 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -692,10 +692,12 @@ mod tests { fn create_drm_dev() -> Result<(faux::Registration, UnregisteredDevice)> { // 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 dev = fdev.as_ref(); + let drm = UnregisteredDevice::new(dev, data)?; - Ok((dev, drm)) + Ok((reg, drm)) } #[test] @@ -755,7 +757,8 @@ 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::::new(&drm, PAGE_SIZE, ObjectConfig::default(), ())?; 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( + Opaque, + PhantomData, +); + +impl Device { + #[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`. + unsafe { &*ptr.cast() } + } +} + +impl AsRef> for Device { + #[inline] + fn as_ref(&self) -> &device::Device { + // 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 device::AsBusDevice for Device { + 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> for Registration { - fn as_ref(&self) -> &device::Device { +impl AsRef> for Registration { + #[inline] + fn as_ref(&self) -> &Device { // 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> 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()) } } } -- cgit v1.2.3 From 49e27d58a06d7edda4791b9fa818a536dd34b7e5 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:24 +0200 Subject: rust: drm: Add Driver::ParentDevice associated type Add a ParentDevice associated type to the Driver trait, allowing each DRM driver to declare its parent bus device type (e.g. auxiliary::Device, platform::Device). Change UnregisteredDevice::new() to take &T::ParentDevice, ensuring at the type level that the DRM device's parent matches the declared bus device type. Reviewed-by: Lyude Paul Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-5-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/device.rs | 7 +++++-- rust/kernel/drm/driver.rs | 3 +++ rust/kernel/drm/gem/shmem.rs | 4 ++-- 3 files changed, 10 insertions(+), 4 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 35ff9c6942d8..343c0ef63c6c 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -178,7 +178,10 @@ impl UnregisteredDevice { /// 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) -> Result { + pub fn new( + dev: &T::ParentDevice, + data: impl PinInit, + ) -> Result { // `__drm_dev_alloc` uses `kmalloc()` to allocate memory, hence ensure a `kmalloc()` // compatible `Layout`. let layout = Kmalloc::aligned_layout(Layout::new::>()); @@ -195,7 +198,7 @@ impl UnregisteredDevice { // - `dev` is valid by its type invarants, let raw_drm: *mut Device = unsafe { bindings::__drm_dev_alloc( - dev.as_raw(), + dev.as_ref().as_raw(), &alloc_vtable, layout.size(), mem::offset_of!(Device, dev), diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index 25f7e233884d..802e7fc13e30 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -116,6 +116,9 @@ pub trait Driver { /// 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: device::AsBusDevice; + /// Driver metadata const INFO: DriverInfo; diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index 52de59b14dad..cbcfc7e4edb6 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -684,6 +684,7 @@ mod tests { type Data = KunitData; type File = KunitFile; type Object = Object; + type ParentDevice = faux::Device; const INFO: drm::DriverInfo = INFO; const IOCTLS: &'static [drm::ioctl::DrmIoctlDescriptor] = &[]; @@ -694,8 +695,7 @@ mod tests { let data = try_pin_init!(KunitData {}); let reg = faux::Registration::new(c"Kunit", None)?; let fdev = reg.as_ref(); - let dev = fdev.as_ref(); - let drm = UnregisteredDevice::new(dev, data)?; + let drm = UnregisteredDevice::new(fdev, data)?; Ok((reg, drm)) } -- cgit v1.2.3 From 9030013008743626d367ff975085b4c14e9fd86b Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:25 +0200 Subject: rust: drm: change default DeviceContext to Normal Change the default DeviceContext from Registered to Normal for drm::Device, gem::Object, gem::shmem::Object and gem::shmem::ObjectConfig. Normal is the general-purpose, reference-counted context suitable for most uses; Registered represents a device that was registered with userspace and will become a non-owning context obtained through a RegistrationGuard. Update the create_handle/lookup_handle bounds from Object to Object to match the new default context of GEM objects, and update the driver device type aliases (NovaDevice, TyrDrmDevice) to default to Normal. Reviewed-by: Lyude Paul Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-6-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/device.rs | 2 +- rust/kernel/drm/gem/mod.rs | 7 ++++--- rust/kernel/drm/gem/shmem.rs | 10 +++++----- 3 files changed, 10 insertions(+), 9 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 343c0ef63c6c..458519d62a46 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -246,7 +246,7 @@ impl UnregisteredDevice { /// * The data layout of `Self` remains the same across all implementations of `C`. /// * Any invariants for `C` also apply. #[repr(C)] -pub struct Device { +pub struct Device { dev: Opaque, data: T::Data, _ctx: PhantomData, diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index 48fa6e96dfe7..6a688568afbb 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -10,6 +10,7 @@ use crate::{ self, device::{ DeviceContext, + Normal, Registered, // }, driver::{ @@ -183,7 +184,7 @@ pub trait BaseObject: IntoGEMObject { fn create_handle(&self, file: &drm::File) -> Result where Self: AllocImpl, - D: drm::Driver = Self, File = F>, + D: drm::Driver = Self, File = F>, F: drm::file::DriverFile, { let mut handle: u32 = 0; @@ -198,7 +199,7 @@ pub trait BaseObject: IntoGEMObject { fn lookup_handle(file: &drm::File, handle: u32) -> Result> where Self: AllocImpl, - D: drm::Driver = Self, File = F>, + D: drm::Driver = Self, File = F>, F: drm::file::DriverFile, { // SAFETY: The arguments are all valid per the type invariants. @@ -254,7 +255,7 @@ impl 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 { +pub struct Object { obj: Opaque, #[pin] data: T, diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index cbcfc7e4edb6..5ffa1355ecf2 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -22,7 +22,7 @@ use crate::{ private::Sealed, Device, DeviceContext, - Registered, // + Normal, // }, error::{ from_err_ptr, @@ -73,7 +73,7 @@ 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, C: DeviceContext = Normal> { /// Whether to set the write-combine map flag. pub map_wc: bool, @@ -102,7 +102,7 @@ impl<'a, T: DriverObject, C: DeviceContext> Default for ObjectConfig<'a, T, C> { /// - 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. @@ -409,7 +409,7 @@ 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 = Registered>( +struct DmaResvGuard<'a, T: DriverObject, C: DeviceContext = Normal>( &'a Object, NotThreadSafe, ); @@ -438,7 +438,7 @@ 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, -- cgit v1.2.3 From 506a7d63dab00f0f279869c956b986749292623a Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 28 Jun 2026 16:53:26 +0200 Subject: rust: drm: restrict AlwaysRefCounted to Normal Device context Restrict the AlwaysRefCounted implementation for drm::Device to the Normal context. Registered devices represent a non-owning view of a device within a RegistrationGuard scope and must not be independently reference-counted. Reviewed-by: Lyude Paul Reviewed-by: Alexandre Courbot Tested-by: Deborah Brouwer Link: https://patch.msgid.link/20260628145406.2107056-7-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/drm/device.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 458519d62a46..312850f125af 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -85,6 +85,9 @@ pub trait DeviceContext: Sealed + Send + Sync + 'static {} /// /// 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`, making this the required +/// context fo