[PATCH v3 2/5] rust: pci: rework device enabling API

Maurice Hieronymus <[email protected]>
Newsgroups org.kernel.vger.rust-for-linux,dev.linux.lists.nova-gpu,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci
Message-ID <[email protected]>
`enable_device_mem()` wraps the unmanaged `pci_enable_device_mem()` and
has no disable counterpart, so the enable count is leaked on driver
unbind and the device does not come back up on a subsequent probe.

Replace it with `enable_device()`, which wraps `pci_enable_device()`
and returns a `DeviceEnableGuard<'a>`: dropping the guard runs
`pci_disable_device()`. The guard borrows the device's bound scope
(`&'a Device<Bound>`), so it cannot outlive the driver binding, and
since it is the only way to enable the device from safe code, the
enable count always stays balanced.

Obtaining the guard still requires a `&Device<Core>`, i.e. a bus
callback. Unlike `pci_enable_device_mem()`, `pci_enable_device()`
enables I/O and memory resources.

Convert nova-core, the only user of `enable_device_mem()`, storing the
guard as the last field of `NovaCore` so the device is disabled only
after the GPU teardown.

Link: https://lore.kernel.org/rust-for-linux/[email protected]
Suggested-by: Danilo Krummrich <[email protected]>
Signed-off-by: Maurice Hieronymus <[email protected]>
---
 drivers/gpu/nova-core/driver.rs |  5 ++++-
 rust/kernel/pci.rs              | 31 ++++++++++++++++++++++++++++---
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index 5738d4ac521b..99b15da59e81 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -30,6 +30,8 @@ pub(crate) struct NovaCore<'bound> {
     bar: pci::Bar<'bound, BAR0_SIZE>,
     #[allow(clippy::type_complexity)]
     _reg: auxiliary::Registration<'bound, ForLt!(())>,
+    // Declared last so the device stays enabled until everything above is torn down.
+    _enable: pci::DeviceEnableGuard<'bound>,
 }
 
 pub(crate) struct NovaCoreDriver;
@@ -75,7 +77,7 @@ fn probe<'bound>(
         pin_init::pin_init_scope(move || {
             dev_dbg!(pdev, "Probe Nova Core GPU driver.\n");
 
-            pdev.enable_device_mem()?;
+            let enable = pdev.enable_device()?;
             pdev.set_master();
 
             Ok(try_pin_init!(NovaCore {
@@ -95,6 +97,7 @@ fn probe<'bound>(
                     crate::MODULE_NAME,
                     (),
                 )?,
+                _enable: enable,
             }))
         })
     }
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 4def9ca1824c..bd9a8113af35 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -452,11 +452,36 @@ pub fn pci_class(&self) -> Class {
     }
 }
 
+/// A guard that keeps the device's I/O and memory resources enabled.
+///
+/// # Invariants
+///
+/// The device's enable count was incremented once for this guard; dropping the guard decrements
+/// it again.
+pub struct DeviceEnableGuard<'a> {
+    dev: &'a Device<device::Bound>,
+}
+
+impl Drop for DeviceEnableGuard<'_> {
+    fn drop(&mut self) {
+        // SAFETY: `self.dev.as_raw()` is a valid pointer to a `struct pci_dev`, and by the type
+        // invariant this guard holds one increment of the device's enable count.
+        unsafe { bindings::pci_disable_device(self.dev.as_raw()) };
+    }
+}
+
 impl<'a> Device<device::Core<'a>> {
-    /// Enable memory resources for this device.
-    pub fn enable_device_mem(&self) -> Result {
+    /// Enable I/O and memory resources for this device.
+    ///
+    /// The device stays enabled for the lifetime of the returned guard; dropping the guard
+    /// disables the device again. The guard borrows the device's bound scope, so it cannot
+    /// outlive the driver binding.
+    pub fn enable_device(&self) -> Result<DeviceEnableGuard<'_>> {
         // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
-        to_result(unsafe { bindings::pci_enable_device_mem(self.as_raw()) })
+        to_result(unsafe { bindings::pci_enable_device(self.as_raw()) })?;
+
+        // INVARIANT: `pci_enable_device()` succeeded, so the enable count was incremented once.
+        Ok(DeviceEnableGuard { dev: self })
     }
 
     /// Enable bus-mastering for this device.

-- 
2.54.0
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.