Re: [PATCH v5 12/19] rust: drm: Add RegistrationGuard for drm_dev_enter/exit critical sections

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Using `drm_dev_unplug()` instead of `drm_dev_unregister()` breaks normal hardware shutdown for display drivers on module unload.
--

commit f4a9e1c28c30b5e7af0c36efbe74ccb2e5047e20
Author: Danilo Krummrich <dakr@kernel.org>
rust: drm: Add RegistrationGuard for drm_dev_enter/exit critical sections

This commit adds a RegistrationGuard to represent a drm_dev_enter/exit SRCU
protection area. It also changes Registration::drop to use drm_dev_unplug()
instead of drm_dev_unregister() to provide an SRCU barrier.

> diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs
> index 5152a18a8312b..3cda8dceb498d 100644
> --- a/rust/kernel/drm/driver.rs
> +++ b/rust/kernel/drm/driver.rs
[ ... ]
> @@ -199,8 +199,14 @@ unsafe impl<T: Driver> Send for Registration<T> {}
>  
>  impl<T: Driver> Drop for Registration<T> {
>      fn drop(&mut self) {
> +        // Use `drm_dev_unplug` rather than `drm_dev_unregister` to ensure that existing
> +        // `drm_dev_enter()` critical sections complete before unregistration proceeds. This
> +        // is required for the safety of `RegistrationGuard`, which relies on the SRCU barrier in
> +        // `drm_dev_unplug()` to guarantee that the parent device is still bound within the
> +        // critical section.
> +        //
>          // SAFETY: Safe by the invariant of `ARef<drm::Device<T>>`. The existence of this
> -        // `Registration` also guarantees the this `drm::Device` is actually registered.
> -        unsafe { bindings::drm_dev_unregister(self.0.as_raw()) };
> +        // `Registration` also guarantees that this `drm::Device` is actually registered.
> +        unsafe { bindings::drm_dev_unplug(self.0.as_raw()) };

[Severity: High]
Will replacing drm_dev_unregister() with drm_dev_unplug() here break normal
hardware shutdown for display drivers on module unload?

When drm_dev_unplug() is called, it sets dev->unplugged = true, which causes
any subsequent calls to drm_dev_enter() to return false. 

The standard pattern for DRM driver removal is to unregister the device, which
now calls drm_dev_unplug() through this Drop implementation. The driver then
typically calls drm_atomic_helper_shutdown() to gracefully disable the
hardware.

Since dev->unplugged is now true, the KMS disable code paths (which rely on
drm_dev_enter() to prevent accessing hot-unplugged hardware) will immediately
abort.

Does this cause the shutdown sequence to silently fail and leave the display
hardware enabled, potentially continuing to scan out from freed memory or
trigger IOMMU faults?

>      }
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260628145406.2107056-1-dakr@kernel.org?part=12