[PATCH 0/5] gpu: nova: adopt driver lifetime infrastructure

[PATCH 0/5] gpu: nova: adopt driver lifetime infrastructure

[Danilo Krummrich]

Adopt the driver core lifetime infrastructure for nova.

Use the lifetime-bound pci::Bar directly in NovaCore, eliminating the
Arc<Devres<Bar0>> indirection. This lets SysmemFlush borrow the Bar and
implement Drop for automatic cleanup.

Replace ARef<Device> with plain borrows in SysmemFlush and the GSP sequencer,
where the structs are already lifetime-parameterized.

Separate the driver type from the driver data to allow the private data
to be lifetime-parameterized via the Data GAT.

This patch series is based on [1] and drm-rust-next.

[1] https://lore.kernel.org/driver-core/20260525202921.124698-1-dakr@kernel.org/

Danilo Krummrich (5):
  gpu: nova-core: use lifetime for Bar
  gpu: nova-core: unregister sysmem flush page from Drop
  gpu: nova-core: replace ARef<Device> with &'bound Device in
    SysmemFlush
  gpu: nova-core: gsp: replace ARef<Device> with &'a Device in sequencer
  gpu: nova: separate driver type from driver data

 drivers/gpu/drm/nova/driver.rs         | 12 ++++----
 drivers/gpu/nova-core/driver.rs        | 32 +++++++++-------------
 drivers/gpu/nova-core/fb.rs            | 31 ++++++++++-----------
 drivers/gpu/nova-core/gpu.rs           | 38 ++++++++------------------
 drivers/gpu/nova-core/gsp/boot.rs      |  2 +-
 drivers/gpu/nova-core/gsp/sequencer.rs | 11 ++++----
 6 files changed, 52 insertions(+), 74 deletions(-)

-- 
2.54.0

[PATCH 1/5] gpu: nova-core: use lifetime for Bar

[Danilo Krummrich]

Take advantage of the lifetime-parameterized pci::Bar<'bound> to hold
the BAR mapping directly in NovaCore<'bound>, and pass a borrowed
reference to Gpu<'bound>.

This eliminates the Arc<Devres<Bar0>> indirection, removes runtime
revocation checks for BAR access, and simplifies Gpu::unbind().

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/gpu/nova-core/driver.rs | 32 +++++++++++++++-----------------
 drivers/gpu/nova-core/gpu.rs    | 33 +++++++++++++--------------------
 2 files changed, 28 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index d3f2245ba2e0..d4cf4379ee87 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -13,12 +13,9 @@
     },
     prelude::*,
     sizes::SZ_16M,
-    sync::{
-        atomic::{
-            Atomic,
-            Relaxed, //
-        },
-        Arc,
+    sync::atomic::{
+        Atomic,
+        Relaxed, //
     },
     types::ForLt,
 };
@@ -31,7 +28,8 @@
 #[pin_data]
 pub(crate) struct NovaCore<'bound> {
     #[pin]
-    pub(crate) gpu: Gpu,
+    pub(crate) gpu: Gpu<'bound>,
+    bar: pci::Bar<'bound, BAR0_SIZE>,
     #[allow(clippy::type_complexity)]
     _reg: auxiliary::Registration<'bound, ForLt!(())>,
 }
@@ -48,7 +46,7 @@ pub(crate) struct NovaCore<'bound> {
 // DMA addresses. These systems should be quite rare.
 const GPU_DMA_BITS: u32 = 47;
 
-pub(crate) type Bar0 = pci::Bar<'static, BAR0_SIZE>;
+pub(crate) type Bar0 = kernel::io::Mmio<BAR0_SIZE>;
 
 kernel::pci_device_table!(
     PCI_TABLE,
@@ -95,14 +93,14 @@ fn probe<'bound>(
             // other threads of execution.
             unsafe { pdev.dma_set_mask_and_coherent(DmaMask::new::<GPU_DMA_BITS>())? };
 
-            let bar = Arc::new(
-                pdev.iomap_region_sized::<BAR0_SIZE>(0, c"nova-core/bar0")?
-                    .into_devres()?,
-                GFP_KERNEL,
-            )?;
-
             Ok(try_pin_init!(NovaCore {
-                gpu <- Gpu::new(pdev, bar.clone(), bar.access(pdev.as_ref())?),
+                bar: pdev.iomap_region_sized::<BAR0_SIZE>(0, c"nova-core/bar0")?,
+                // TODO: Use `&bar` self-referential pin-init syntax once available.
+                //
+                // SAFETY: `bar` is initialized before this expression is evaluated
+                // (`try_pin_init!()` initializes fields in declaration order), lives at a pinned
+                // stable address, and is dropped after `gpu` (struct field drop order).
+                gpu <- Gpu::new(pdev, unsafe { &*core::ptr::from_ref(bar) }),
                 _reg: auxiliary::Registration::new(
                     pdev.as_ref(),
                     c"nova-drm",
@@ -116,7 +114,7 @@ fn probe<'bound>(
         })
     }
 
-    fn unbind<'bound>(pdev: &'bound pci::Device<Core<'_>>, this: Pin<&Self::Data<'bound>>) {
-        this.gpu.unbind(pdev.as_ref());
+    fn unbind<'bound>(_pdev: &'bound pci::Device<Core<'_>>, this: Pin<&Self::Data<'bound>>) {
+        this.gpu.unbind();
     }
 }
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 78c4195e6132..108dd094e354 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -2,13 +2,11 @@
 
 use kernel::{
     device,
-    devres::Devres,
     fmt,
     io::Io,
     num::Bounded,
     pci,
-    prelude::*,
-    sync::Arc, //
+    prelude::*, //
 };
 
 use crate::{
@@ -246,10 +244,10 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
 
 /// Structure holding the resources required to operate the GPU.
 #[pin_data]
-pub(crate) struct Gpu {
+pub(crate) struct Gpu<'gpu> {
     spec: Spec,
-    /// MMIO mapping of PCI BAR 0
-    bar: Arc<Devres<Bar0>>,
+    /// MMIO mapping of PCI BAR 0.
+    bar: &'gpu Bar0,
     /// System memory page required for flushing all pending GPU-side memory writes done through
     /// PCIE into system memory, via sysmembar (A GPU-initiated HW memory-barrier operation).
     sysmem_flush: SysmemFlush,
@@ -262,12 +260,11 @@ pub(crate) struct Gpu {
     gsp: Gsp,
 }
 
-impl Gpu {
-    pub(crate) fn new<'a>(
-        pdev: &'a pci::Device<device::Bound>,
-        devres_bar: Arc<Devres<Bar0>>,
-        bar: &'a Bar0,
-    ) -> impl PinInit<Self, Error> + 'a {
+impl<'gpu> Gpu<'gpu> {
+    pub(crate) fn new(
+        pdev: &'gpu pci::Device<device::Bound>,
+        bar: &'gpu Bar0,
+    ) -> impl PinInit<Self, Error> + 'gpu {
         try_pin_init!(Self {
             spec: Spec::new(pdev.as_ref(), bar).inspect(|spec| {
                 dev_info!(pdev,"NVIDIA ({})\n", spec);
@@ -279,6 +276,8 @@ pub(crate) fn new<'a>(
                     .inspect_err(|_| dev_err!(pdev, "GFW boot did not complete\n"))?;
             },
 
+            bar,
+
             sysmem_flush: SysmemFlush::register(pdev.as_ref(), bar, spec.chipset)?,
 
             gsp_falcon: Falcon::new(
@@ -292,19 +291,13 @@ pub(crate) fn new<'a>(
             gsp <- Gsp::new(pdev),
 
             _: { gsp.boot(pdev, bar, spec.chipset, gsp_falcon, sec2_falcon)? },
-
-            bar: devres_bar,
         })
     }
 
     /// Called when the corresponding [`Device`](device::Device) is unbound.
     ///
     /// Note: This method must only be called from `Driver::unbind`.
-    pub(crate) fn unbind(&self, dev: &device::Device<device::Core<'_>>) {
-        kernel::warn_on!(self
-            .bar
-            .access(dev)
-            .inspect(|bar| self.sysmem_flush.unregister(bar))
-            .is_err());
+    pub(crate) fn unbind(&self) {
+        self.sysmem_flush.unregister(self.bar);
     }
 }
-- 
2.54.0

[PATCH 2/5] gpu: nova-core: unregister sysmem flush page from Drop

[Danilo Krummrich]

Now that SysmemFlush can borrow the Bar via HRT lifetime, store a
&'bound Bar0 reference and implement Drop to automatically unregister
the sysmem flush page. This removes the need for manual unregister()
calls and the Gpu::unbind() method.

Reported-by: Eliot Courtney <ecourtney@nvidia.com>
Closes: https://lore.kernel.org/all/20260409-fix-systemflush-v1-1-a1d6c968f17c@nvidia.com/
Fixes: 6554ad65b589 ("gpu: nova-core: register sysmem flush page")
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/gpu/nova-core/driver.rs |  4 ----
 drivers/gpu/nova-core/fb.rs     | 22 ++++++++++------------
 drivers/gpu/nova-core/gpu.rs    |  9 +--------
 3 files changed, 11 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index d4cf4379ee87..cff5034c2dcd 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -113,8 +113,4 @@ fn probe<'bound>(
             }))
         })
     }
-
-    fn unbind<'bound>(_pdev: &'bound pci::Device<Core<'_>>, this: Pin<&Self::Data<'bound>>) {
-        this.gpu.unbind();
-    }
 }
diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
index 6ee87050ce69..3b3271790cc9 100644
--- a/drivers/gpu/nova-core/fb.rs
+++ b/drivers/gpu/nova-core/fb.rs
@@ -43,21 +43,20 @@
 /// Because of this, the sysmem flush memory page must be registered as early as possible during
 /// driver initialization, and before any falcon is reset.
 ///
-/// Users are responsible for manually calling [`Self::unregister`] before dropping this object,
-/// otherwise the GPU might still use it even after it has been freed.
-pub(crate) struct SysmemFlush {
+pub(crate) struct SysmemFlush<'sys> {
     /// Chipset we are operating on.
     chipset: Chipset,
     device: ARef<device::Device>,
+    bar: &'sys Bar0,
     /// Keep the page alive as long as we need it.
     page: CoherentHandle,
 }
 
-impl SysmemFlush {
+impl<'sys> SysmemFlush<'sys> {
     /// Allocate a memory page and register it as the sysmem flush page.
     pub(crate) fn register(
         dev: &device::Device<device::Bound>,
-        bar: &Bar0,
+        bar: &'sys Bar0,
         chipset: Chipset,
     ) -> Result<Self> {
         let page = CoherentHandle::alloc(dev, kernel::page::PAGE_SIZE, GFP_KERNEL)?;
@@ -67,19 +66,18 @@ pub(crate) fn register(
         Ok(Self {
             chipset,
             device: dev.into(),
+            bar,
             page,
         })
     }
+}
 
-    /// Unregister the managed sysmem flush page.
-    ///
-    /// In order to gracefully tear down the GPU, users must make sure to call this method before
-    /// dropping the object.
-    pub(crate) fn unregister(&self, bar: &Bar0) {
+impl Drop for SysmemFlush<'_> {
+    fn drop(&mut self) {
         let hal = hal::fb_hal(self.chipset);
 
-        if hal.read_sysmem_flush_page(bar) == self.page.dma_handle() {
-            let _ = hal.write_sysmem_flush_page(bar, 0).inspect_err(|e| {
+        if hal.read_sysmem_flush_page(self.bar) == self.page.dma_handle() {
+            let _ = hal.write_sysmem_flush_page(self.bar, 0).inspect_err(|e| {
                 dev_warn!(
                     &self.device,
                     "failed to unregister sysmem flush page: {:?}\n",
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 108dd094e354..cf134cab49cd 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -250,7 +250,7 @@ pub(crate) struct Gpu<'gpu> {
     bar: &'gpu Bar0,
     /// System memory page required for flushing all pending GPU-side memory writes done through
     /// PCIE into system memory, via sysmembar (A GPU-initiated HW memory-barrier operation).
-    sysmem_flush: SysmemFlush,
+    sysmem_flush: SysmemFlush<'gpu>,
     /// GSP falcon instance, used for GSP boot up and cleanup.
     gsp_falcon: Falcon<GspFalcon>,
     /// SEC2 falcon instance, used for GSP boot up and cleanup.
@@ -293,11 +293,4 @@ pub(crate) fn new(
             _: { gsp.boot(pdev, bar, spec.chipset, gsp_falcon, sec2_falcon)? },
         })
     }
-
-    /// Called when the corresponding [`Device`](device::Device) is unbound.
-    ///
-    /// Note: This method must only be called from `Driver::unbind`.
-    pub(crate) fn unbind(&self) {
-        self.sysmem_flush.unregister(self.bar);
-    }
 }
-- 
2.54.0

[PATCH 3/5] gpu: nova-core: replace ARef<Device> with &'bound Device in SysmemFlush

[Danilo Krummrich]

Now that SysmemFlush is lifetime-parameterized, the ARef<Device> is
unnecessary -- a plain &'bound Device reference suffices.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/gpu/nova-core/fb.rs | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
index 3b3271790cc9..1fb65d4eb290 100644
--- a/drivers/gpu/nova-core/fb.rs
+++ b/drivers/gpu/nova-core/fb.rs
@@ -15,8 +15,7 @@
         Alignable,
         Alignment, //
     },
-    sizes::*,
-    sync::aref::ARef, //
+    sizes::*, //
 };
 
 use crate::{
@@ -46,7 +45,7 @@
 pub(crate) struct SysmemFlush<'sys> {
     /// Chipset we are operating on.
     chipset: Chipset,
-    device: ARef<device::Device>,
+    device: &'sys device::Device,
     bar: &'sys Bar0,
     /// Keep the page alive as long as we need it.
     page: CoherentHandle,
@@ -55,7 +54,7 @@ pub(crate) struct SysmemFlush<'sys> {
 impl<'sys> SysmemFlush<'sys> {
     /// Allocate a memory page and register it as the sysmem flush page.
     pub(crate) fn register(
-        dev: &device::Device<device::Bound>,
+        dev: &'sys device::Device<device::Bound>,
         bar: &'sys Bar0,
         chipset: Chipset,
     ) -> Result<Self> {
@@ -65,7 +64,7 @@ pub(crate) fn register(
 
         Ok(Self {
             chipset,
-            device: dev.into(),
+            device: dev,
             bar,
             page,
         })
-- 
2.54.0

[PATCH 4/5] gpu: nova-core: gsp: replace ARef<Device> with &'a Device in sequencer

[Danilo Krummrich]

GspSequencer, GspSeqIter, and GspSequencerParams are already
lifetime-parameterized; the ARef<Device> is unnecessary -- a plain
&'a Device reference suffices.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/gpu/nova-core/gsp/boot.rs      |  2 +-
 drivers/gpu/nova-core/gsp/sequencer.rs | 11 +++++------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index df105ef4b371..3e8f9611d2b3 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -234,7 +234,7 @@ pub(crate) fn boot(
             libos_dma_handle: libos_handle,
             gsp_falcon,
             sec2_falcon,
-            dev: pdev.as_ref().into(),
+            dev,
             bar,
         };
         GspSequencer::run(&self.cmdq, seq_params)?;
diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs b/drivers/gpu/nova-core/gsp/sequencer.rs
index 474e4c8021db..b3015483ed17 100644
--- a/drivers/gpu/nova-core/gsp/sequencer.rs
+++ b/drivers/gpu/nova-core/gsp/sequencer.rs
@@ -11,7 +11,6 @@
         Io, //
     },
     prelude::*,
-    sync::aref::ARef,
     time::{
         delay::fsleep,
         Delta, //
@@ -142,7 +141,7 @@ pub(crate) struct GspSequencer<'a> {
     /// Bootloader application version.
     bootloader_app_version: u32,
     /// Device for logging.
-    dev: ARef<device::Device>,
+    dev: &'a device::Device,
 }
 
 impl fw::RegWritePayload {
@@ -281,7 +280,7 @@ pub(crate) struct GspSeqIter<'a> {
     /// Number of commands processed so far.
     cmds_processed: u32,
     /// Device for logging.
-    dev: ARef<device::Device>,
+    dev: &'a device::Device,
 }
 
 impl<'a> Iterator for GspSeqIter<'a> {
@@ -309,7 +308,7 @@ fn next(&mut self) -> Option<Self::Item> {
             self.cmd_data.len() - offset
         };
         buffer[..copy_len].copy_from_slice(&self.cmd_data[offset..offset + copy_len]);
-        let cmd_result = GspSeqCmd::new(&buffer, &self.dev);
+        let cmd_result = GspSeqCmd::new(&buffer, self.dev);
 
         cmd_result.map_or_else(
             |_err| {
@@ -334,7 +333,7 @@ fn iter(&self) -> GspSeqIter<'_> {
             current_offset: 0,
             total_cmds: self.seq_info.cmd_index,
             cmds_processed: 0,
-            dev: self.dev.clone(),
+            dev: self.dev,
         }
     }
 }
@@ -350,7 +349,7 @@ pub(crate) struct GspSequencerParams<'a> {
     /// SEC2 falcon for core operations.
     pub(crate) sec2_falcon: &'a Falcon<Sec2>,
     /// Device for logging.
-    pub(crate) dev: ARef<device::Device>,
+    pub(crate) dev: &'a device::Device,
     /// BAR0 for register access.
     pub(crate) bar: &'a Bar0,
 }
-- 
2.54.0

[PATCH 5/5] gpu: nova: separate driver type from driver data

[Danilo Krummrich]

Split NovaDriver into a unit struct for trait implementations and a
separate Nova struct for the private driver data.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/gpu/drm/nova/driver.rs | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index e4765bc5b3ec..4289df7de01c 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -15,9 +15,11 @@
 use crate::file::File;
 use crate::gem::NovaObject;
 
-pub(crate) struct NovaDriver {
+pub(crate) struct NovaDriver;
+
+pub(crate) struct Nova {
     #[expect(unused)]
-    drm: ARef<drm::Device<Self>>,
+    drm: ARef<drm::Device<NovaDriver>>,
 }
 
 /// Convienence type alias for the DRM device type for this driver
@@ -51,19 +53,19 @@ pub(crate) struct NovaData {
 
 impl auxiliary::Driver for NovaDriver {
     type IdInfo = ();
-    type Data<'bound> = Self;
+    type Data<'bound> = Nova;
     const ID_TABLE: auxiliary::IdTable<Self::IdInfo> = &AUX_TABLE;
 
     fn probe<'bound>(
         adev: &'bound auxiliary::Device<Core<'_>>,
         _info: &'bound Self::IdInfo,
-    ) -> impl PinInit<Self, Error> + 'bound {
+    ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
         let data = try_pin_init!(NovaData { adev: adev.into() });
 
         let drm = drm::Device::<Self>::new(adev.as_ref(), data)?;
         drm::Registration::new_foreign_owned(&drm, adev.as_ref(), 0)?;
 
-        Ok(Self { drm })
+        Ok(Nova { drm })
     }
 }
 
-- 
2.54.0

Re: [PATCH 2/5] gpu: nova-core: unregister sysmem flush page from Drop

[Eliot Courtney]

On Tue May 26, 2026 at 7:58 AM JST, Danilo Krummrich wrote:
> Now that SysmemFlush can borrow the Bar via HRT lifetime, store a
> &'bound Bar0 reference and implement Drop to automatically unregister
> the sysmem flush page. This removes the need for manual unregister()
> calls and the Gpu::unbind() method.
>
> Reported-by: Eliot Courtney <ecourtney@nvidia.com>
> Closes: https://lore.kernel.org/all/20260409-fix-systemflush-v1-1-a1d6c968f17c@nvidia.com/
> Fixes: 6554ad65b589 ("gpu: nova-core: register sysmem flush page")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---

Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>

Re: [PATCH 3/5] gpu: nova-core: replace ARef<Device> with &'bound Device in SysmemFlush

[Eliot Courtney]

On Tue May 26, 2026 at 7:58 AM JST, Danilo Krummrich wrote:
> Now that SysmemFlush is lifetime-parameterized, the ARef<Device> is
> unnecessary -- a plain &'bound Device reference suffices.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---

Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>

Re: [PATCH 4/5] gpu: nova-core: gsp: replace ARef<Device> with &'a Device in sequencer

[Eliot Courtney]

On Tue May 26, 2026 at 7:58 AM JST, Danilo Krummrich wrote:
> GspSequencer, GspSeqIter, and GspSequencerParams are already
> lifetime-parameterized; the ARef<Device> is unnecessary -- a plain
> &'a Device reference suffices.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---

Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>

Re: [PATCH 5/5] gpu: nova: separate driver type from driver data

[Eliot Courtney]

On Tue May 26, 2026 at 7:58 AM JST, Danilo Krummrich wrote:
> Split NovaDriver into a unit struct for trait implementations and a
> separate Nova struct for the private driver data.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---

Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>

Re: [PATCH 1/5] gpu: nova-core: use lifetime for Bar

[Eliot Courtney]

On Tue May 26, 2026 at 7:58 AM JST, Danilo Krummrich wrote:
> Take advantage of the lifetime-parameterized pci::Bar<'bound> to hold
> the BAR mapping directly in NovaCore<'bound>, and pass a borrowed
> reference to Gpu<'bound>.
>
> This eliminates the Arc<Devres<Bar0>> indirection, removes runtime
> revocation checks for BAR access, and simplifies Gpu::unbind().
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---

Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>

Re: [PATCH 0/5] gpu: nova: adopt driver lifetime infrastructure

[Alexandre Courbot]

On Tue May 26, 2026 at 7:58 AM JST, Danilo Krummrich wrote:
> Adopt the driver core lifetime infrastructure for nova.
>
> Use the lifetime-bound pci::Bar directly in NovaCore, eliminating the
> Arc<Devres<Bar0>> indirection. This lets SysmemFlush borrow the Bar and
> implement Drop for automatic cleanup.
>
> Replace ARef<Device> with plain borrows in SysmemFlush and the GSP sequencer,
> where the structs are already lifetime-parameterized.
>
> Separate the driver type from the driver data to allow the private data
> to be lifetime-parameterized via the Data GAT.
>
> This patch series is based on [1] and drm-rust-next.
>
> [1] https://lore.kernel.org/driver-core/20260525202921.124698-1-dakr@kernel.org/

The series,

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
Tested-by: Alexandre Courbot <acourbot@nvidia.com>

Claude review: gpu: nova: adopt driver lifetime infrastructure

[Claude Code Review Bot]

Overall Series Review

Subject: gpu: nova: adopt driver lifetime infrastructure
Author: Danilo Krummrich <dakr@kernel.org>
Patches: 12
Reviewed: 2026-05-27T15:28:22.690399

---

This is a well-structured 5-patch series from Danilo Krummrich that adopts Rust lifetime-based resource management for the nova GPU driver, replacing runtime-checked `Arc<Devres<Bar0>>` and `ARef<Device>` with compile-time lifetime-parameterized borrows. The series is based on prerequisite driver-core lifetime infrastructure patches that introduce `'bound` lifetime support to `pci::Bar` and `pci::Driver`.

**Overall assessment: The series is clean and the direction is correct.** Moving from runtime revocation checks and reference counting to compile-time lifetimes is exactly the right approach for Rust kernel drivers. The patches are logically ordered: first establish the Bar lifetime, then leverage it for Drop-based cleanup, then replace redundant ARef wrappers, and finally separate the driver type from data.

There is one notable unsafe block that deserves scrutiny (patch 1), and patch 2 has a subtle correctness consideration around `ARef<Device>` validity in `Drop`. Otherwise the series is straightforward refactoring with clear net-positive LOC reduction (-22 lines).

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: use lifetime for Bar

[Claude Code Review Bot]

Patch Review

This is the key patch. It changes `NovaCore` to hold `pci::Bar<'bound, BAR0_SIZE>` directly instead of `Arc<Devres<Bar0>>`, and `Gpu` to borrow `&'gpu Bar0` (now aliased to `kernel::io::Mmio<BAR0_SIZE>`) instead of owning a clone of the Arc.

**The unsafe self-referential initialization:**

```rust
gpu <- Gpu::new(pdev, unsafe { &*core::ptr::from_ref(bar) }),
```

This is the most important part of the series. The safety comment explains:

```rust
// SAFETY: `bar` is initialized before this expression is evaluated
// (`try_pin_init!()` initializes fields in declaration order), lives at a pinned
// stable address, and is dropped after `gpu` (struct field drop order).
```

The safety argument has three parts:
1. **Declaration-order initialization** — `bar` is declared before `gpu` in the struct, so `try_pin_init!` initializes it first. This is correct per `pin_init` semantics.
2. **Pinned stable address** — The struct is `#[pin_data]` and the result is pinned, so `bar` won't move after initialization.
3. **Drop order** — Rust drops struct fields in declaration order, so `gpu` (declared after `bar`) is dropped before `bar`. This ensures the `&'gpu Bar0` reference in `Gpu` remains valid during `Gpu::drop`.

This reasoning is sound. The TODO comment about self-referential pin-init syntax is appropriate — this is a known pattern that lacks safe abstraction today.

**Type alias change:**

```rust
-pub(crate) type Bar0 = pci::Bar<'static, BAR0_SIZE>;
+pub(crate) type Bar0 = kernel::io::Mmio<BAR0_SIZE>;
```

This makes sense: `Gpu` and other consumers don't need the PCI bar metadata, just the MMIO mapping. The `pci::Bar<'bound>` is stored in `NovaCore`, and the borrowed `Bar0` (= `Mmio`) is what gets passed around.

**Gpu::unbind simplification:**

```rust
-    pub(crate) fn unbind(&self, dev: &device::Device<device::Core<'_>>) {
-        kernel::warn_on!(self
-            .bar
-            .access(dev)
-            .inspect(|bar| self.sysmem_flush.unregister(bar))
-            .is_err());
+    pub(crate) fn unbind(&self) {
+        self.sysmem_flush.unregister(self.bar);
     }
```

Good — the runtime `.access(dev)` check is no longer needed because the lifetime guarantees the bar is valid.

No issues found.

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: unregister sysmem flush page from Drop

[Claude Code Review Bot]

Patch Review

This patch stores `&'sys Bar0` in `SysmemFlush` and implements `Drop` to automatically unregister the flush page, removing the manual `unregister()` call and `Gpu::unbind()` entirely.

**Drop implementation:**

```rust
impl Drop for SysmemFlush<'_> {
    fn drop(&mut self) {
        let hal = hal::fb_hal(self.chipset);

        if hal.read_sysmem_flush_page(self.bar) == self.page.dma_handle() {
            let _ = hal.write_sysmem_flush_page(self.bar, 0).inspect_err(|e| {
                dev_warn!(
                    &self.device,
                    "failed to unregister sysmem flush page: {:?}\n",
                    e
                )
            });
```

This is a good pattern — the `Drop` impl reads the bar to verify the page is still the active flush page before clearing it. The error message on the else-branch (unchanged from the old `unregister()`) is retained.

**Note on `self.device` in Drop:** At this point in the series, `self.device` is still `ARef<device::Device>`, which is fine — the ARef keeps the device alive. Patch 3 changes this to a borrowed `&'sys device::Device`, which is also fine because the lifetime guarantee ensures the device outlives SysmemFlush.

**Removal of `unbind`:**

```diff
-    fn unbind<'bound>(_pdev: &'bound pci::Device<Core<'_>>, this: Pin<&Self::Data<'bound>>) {
-        this.gpu.unbind();
-    }
```

This is correct — with Drop-based cleanup, the explicit unbind callback is unnecessary. The struct drop order (gpu dropped before bar, as analyzed in patch 1) ensures the Drop runs while the bar is still valid.

The `Fixes:` and `Reported-by:` tags are appropriate — this addresses an existing bug where forgetting to call `unregister()` could leave a dangling flush page.

No issues found.

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: replace ARef<Device> with &'bound Device in SysmemFlush

[Claude Code Review Bot]

Patch Review

Straightforward simplification now that `SysmemFlush<'sys>` has a lifetime parameter.

```rust
-    device: ARef<device::Device>,
+    device: &'sys device::Device,
```

And in `register()`:

```rust
-        dev: &device::Device<device::Bound>,
+        dev: &'sys device::Device<device::Bound>,
```

With:

```rust
-            device: dev.into(),
+            device: dev,
```

**One observation:** The `device` field type is `&'sys device::Device` (unparameterized), but the `register()` function takes `&'sys device::Device<device::Bound>`. The assignment `device: dev` works because `device::Device<device::Bound>` derefs to `device::Device` (the unparameterized form). This is an implicit coercion that's standard in the kernel Rust device model — just noting it for clarity.

The `ARef` import removal is clean. No issues found.

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova-core: gsp: replace ARef<Device> with &'a Device in sequencer

[Claude Code Review Bot]

Patch Review

Same pattern as patch 3, applied to `GspSequencer`, `GspSeqIter`, and `GspSequencerParams`. All three are already lifetime-parameterized with `'a`, so this is a mechanical replacement.

In `boot.rs`:
```rust
-            dev: pdev.as_ref().into(),
+            dev,
```

This is correct — `dev` was already the right `&'a` reference from the function parameter (confirmed by reading boot.rs:220-224 in the tree, where `pdev` is a `&'a pci::Device` and `pdev.as_ref()` yields `&'a device::Device`). Wait — looking more carefully, the existing code has `dev: pdev.as_ref().into()` which converts via `ARef::from()`. The new code just uses `dev`, but the field in `GspSequencerParams` is changing from `ARef<device::Device>` to `&'a device::Device`. The `dev` variable in `boot.rs` context would need to be `pdev.as_ref()` or equivalent. Let me check what `dev` refers to in the patched boot.rs.

The diff only shows line 707 changing `dev: pdev.as_ref().into()` to `dev,`. Since `GspSequencerParams::dev` is now `&'a device::Device`, and the surrounding context has `pdev: &'a pci::Device<device::Bound>`, the field initializer `dev` must mean there's a local variable `dev` of type `&'a device::Device`. Looking at the existing boot.rs, `pdev.as_ref()` returns `&device::Device<device::Bound>`. But `GspSequencerParams::dev` is `&'a device::Device` (no type parameter). This works if there's a `Deref` or `AsRef` coercion from `Device<Bound>` to `Device`. But the shorthand `dev,` means there must be a local variable named `dev` already in scope. Without seeing the full function, this likely refers to a variable computed earlier. This appears fine given the existing code structure.

In `sequencer.rs`, the `.clone()` on ARef is replaced with a simple copy of the reference:

```rust
-            dev: self.dev.clone(),
+            dev: self.dev,
```

And the `&self.dev` dereference becomes direct:

```rust
-        let cmd_result = GspSeqCmd::new(&buffer, &self.dev);
+        let cmd_result = GspSeqCmd::new(&buffer, self.dev);
```

Both correct — `&'a device::Device` is `Copy` (it's a reference). The `sync::aref::ARef` import is properly removed.

No issues found.

---
Generated by Claude Code Patch Reviewer

Claude review: gpu: nova: separate driver type from driver data

[Claude Code Review Bot]

Patch Review

This splits `NovaDriver` into a unit struct (for trait impls) and a new `Nova` struct (for driver data), enabling the `Data` GAT to be lifetime-parameterized in the future.

```rust
-pub(crate) struct NovaDriver {
+pub(crate) struct NovaDriver;
+
+pub(crate) struct Nova {
     #[expect(unused)]
-    drm: ARef<drm::Device<Self>>,
+    drm: ARef<drm::Device<NovaDriver>>,
 }
```

The `Self` in `ARef<drm::Device<Self>>` correctly becomes `NovaDriver` since the field is now on `Nova`, not `NovaDriver`.

```rust
-    type Data<'bound> = Self;
+    type Data<'bound> = Nova;
```

And the probe return type changes:

```rust
-    ) -> impl PinInit<Self, Error> + 'bound {
+    ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
```

With:

```rust
-        Ok(Self { drm })
+        Ok(Nova { drm })
```

This is clean. Note the `Data` type uses a GAT with `'bound` which already appears in the base code (from the prerequisite series). The separation is a standard Rust pattern to allow the data type to carry lifetimes that the driver trait impl type cannot.

**Minor nit:** The existing `Convienence` typo in the doc comment (line 860/`NovaDevice` type alias) is pre-existing and not introduced by this patch.

No issues found.

---
Generated by Claude Code Patch Reviewer