[PATCH v3 15/23] rust: drm: pin the owner while DRM files remain open

Mike Lothian <[email protected]>
Newsgroups org.kernel.vger.rust-for-linux,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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))
     }
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.