Give each Rust DRM device its own driver and file-operations tables so file_operations::owner can identify the module that owns the implementation. Open DRM file descriptors then hold the same module reference that C DRM drivers receive through DEFINE_DRM_GEM_*_FOPS().
Pass the owning module to UnregisteredDevice::new() and use the built-in null module for the shmem KUnit device. Assisted-by: Claude:claude-opus-5 Signed-off-by: Mike Lothian <[email protected]> --- rust/kernel/drm/device.rs | 46 ++++++++++++++++++++++++++++++++++-- rust/kernel/drm/gem/shmem.rs | 8 ++++++- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index bc2cdcd2b695..efbec3f42bda 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -208,9 +208,14 @@ const fn compute_features() -> u32 { /// Create a new `UnregisteredDevice` for a `drm::Driver`. /// /// This can be used to create a [`Registration`](kernel::drm::Registration). + /// + /// `module` must be the module that owns the driver implementation, i.e. `&THIS_MODULE`. It is + /// stamped into this device's `file_operations::owner` so that an open `/dev/dri/cardN` file + /// descriptor pins the module, exactly as `DEFINE_DRM_GEM_*_FOPS()` does in C. pub fn new( dev: &T::ParentDevice<device::Bound>, data: impl PinInit<T::Data, Error>, + module: &'static ThisModule, ) -> Result<Self> { // `__drm_dev_alloc` uses `kmalloc()` to allocate memory, hence ensure a `kmalloc()` // compatible `Layout`. @@ -253,8 +258,37 @@ pub fn new( unsafe { bindings::drm_dev_put(drm_dev) }; })?; - // SAFETY: `drm_dev` is still private to this function. - unsafe { (*drm_dev).driver = const { &Self::VTABLE } }; + // Give this device its own `file_operations`/`drm_driver` pair so that the owning module + // can be stamped into the fops. `fops->owner` is what makes `fops_get()` in + // `drm_stub_open()` take a module reference for every open DRM file: without it nothing + // pins the module, and unloading the driver while a compositor still has + // `/dev/dri/cardN` in a poll set frees the `file_operations` out from under + // `do_sys_poll()`, which then faults on `f_op->poll`. + // + // SAFETY: `raw_drm` is a valid pointer to `Self`, still private to this function, and + // both fields are plain data that need no drop. + let raw_fops = unsafe { Opaque::cast_into(ptr::addr_of!((*raw_drm.as_ptr()).fops)) }; + // SAFETY: `raw_fops` is valid, aligned and points at uninitialized memory we own. + unsafe { + raw_fops.write(bindings::file_operations { + owner: module.as_ptr(), + ..Self::GEM_FOPS + }) + }; + + // SAFETY: as above, for the per-device `drm_driver` copy. + let raw_vtable = unsafe { Opaque::cast_into(ptr::addr_of!((*raw_drm.as_ptr()).vtable)) }; + // SAFETY: `raw_vtable` is valid, aligned and points at uninitialized memory we own. + unsafe { + raw_vtable.write(bindings::drm_driver { + fops: raw_fops, + ..Self::VTABLE + }) + }; + + // SAFETY: `drm_dev` is still private to this function; `raw_vtable` lives inside the DRM + // device allocation and so outlives every use of `drm_device::driver`. + unsafe { (*drm_dev).driver = raw_vtable }; // SAFETY: `raw_drm` is valid; no concurrent access before registration. unsafe { (*raw_drm.as_ptr()).registration_data = UnsafeCell::new(NonNull::dangling()) }; @@ -277,12 +311,20 @@ pub fn new( /// /// * `self.dev` is a valid instance of a `struct device`. /// * The data layout of `Self` remains the same across all implementations of `C`. +/// * `self.vtable` and `self.fops` are initialized before the device is registered and are never +/// mutated afterwards; `self.dev.driver` points at `self.vtable`, whose `fops` points at +/// `self.fops`. /// * Any invariants for `C` also apply. #[repr(C)] pub struct Device<T: drm::Driver, C: DeviceContext = Normal> { dev: Opaque<bindings::drm_device>, data: T::Data, pub(super) registration_data: UnsafeCell<NonNull<T::RegistrationData<'static>>>, + /// Per-device copy of the driver vtable, so that `fops` below can be referenced from it. + vtable: Opaque<bindings::drm_driver>, + /// Per-device copy of the DRM file operations, carrying the owning module in `owner` so that + /// an open DRM file descriptor pins the module. + fops: Opaque<bindings::file_operations>, _ctx: PhantomData<C>, } diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs index 86797ab39ffd..8751000c92bb 100644 --- a/rust/kernel/drm/gem/shmem.rs +++ b/rust/kernel/drm/gem/shmem.rs @@ -642,12 +642,18 @@ impl drm::Driver for KunitDriver { const IOCTLS: &'static [drm::ioctl::DrmIoctlDescriptor] = &[]; } + // These tests only ever build into the kernel image, so there is no module to pin. A null + // `file_operations::owner` is exactly what a built-in driver uses. + // + // SAFETY: `NULL` is the correct `THIS_MODULE` for built-in code. + static KUNIT_MODULE: ThisModule = unsafe { ThisModule::from_ptr(ptr::null_mut()) }; + fn create_drm_dev() -> Result<(faux::Registration, UnregisteredDevice<KunitDriver>)> { // Create a faux DRM device so we can test gem object creation. let data = try_pin_init!(KunitData {}); let reg = faux::Registration::new(c"Kunit", None)?; let fdev = reg.as_ref(); - let drm = UnregisteredDevice::new(fdev, data)?; + let drm = UnregisteredDevice::new(fdev, data, &KUNIT_MODULE)?; Ok((reg, drm)) }
