[PATCH] gpu: nova-core: use pin-init self-referential support

[PATCH] gpu: nova-core: use pin-init self-referential support

[Danilo Krummrich]

Express all self-referential borrows through pin-init's self-referential
support [1], replacing unsafely derived references.

Cc: Gary Guo <gary@garyguo.net>
Link: https://lore.kernel.org/all/20261008-dev-selfref-v2-0-e280b3c8fba5@garyguo.net/ [1]
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/gpu/nova-core/api.rs     | 24 ++++++++++++---
 drivers/gpu/nova-core/driver.rs  | 53 +++++++++++---------------------
 drivers/gpu/nova-core/gpu.rs     | 32 ++++++++-----------
 drivers/gpu/nova-core/irq/gsp.rs | 17 +++++-----
 4 files changed, 59 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
index eec69aace3b2..94808af3886d 100644
--- a/drivers/gpu/nova-core/api.rs
+++ b/drivers/gpu/nova-core/api.rs
@@ -22,11 +22,27 @@
 };
 
 /// API handle for the auxiliary bus child drivers to interact with nova-core.
-pub struct NovaCoreApi<'bound> {
-    pub(crate) gpu: Pin<&'bound Gpu<'bound>>,
+#[pin_data]
+pub struct NovaCoreApi<'bound>
+where
+    // `Gpu` is invariant in its resource lifetime, which outlives this API borrow.
+    exists<'gpu_resources>: 'bound,
+{
+    #[uses('gpu_resources: invariant)]
+    gpu: Pin<&'bound Gpu<'gpu_resources>>,
     pub(crate) pdev: &'bound pci::Device<Bound>,
 }
 
+impl<'bound> NovaCoreApi<'bound> {
+    /// Creates an API handle.
+    pub(crate) fn new<'gpu: 'bound>(
+        pdev: &'bound pci::Device<Bound>,
+        gpu: Pin<&'bound Gpu<'gpu>>,
+    ) -> impl PinInit<Self> + use<'bound, 'gpu> {
+        pin_init!(Self { gpu, pdev })
+    }
+}
+
 impl NovaCoreApi<'_> {
     /// Obtain a [`NovaCoreApi`] handle from an auxiliary device registered
     /// by nova-core.
@@ -36,12 +52,12 @@ pub fn of(adev: &auxiliary::Device<Bound>) -> Result<NovaCoreApiHandle<'_>> {
 
     /// Returns the GPU [`GetGspStaticInfoReply`].
     pub fn gsp_static_info(&self) -> &GetGspStaticInfoReply {
-        &self.gpu.gsp_static_info
+        self.with_gpu(|gpu| &gpu.gsp_static_info)
     }
 
     /// Returns the GPU [`Spec`].
     pub fn spec(&self) -> &Spec {
-        &self.gpu.spec
+        self.with_gpu(|gpu| &gpu.spec)
     }
 
     /// Returns the size of the PCIe BAR used for accessing VRAM, typically
diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index 560a70c4f5a7..ac5a9d9f8331 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -33,9 +33,11 @@
 #[pin_data]
 pub(crate) struct NovaCore<'bound> {
     #[allow(clippy::type_complexity)]
-    _reg: auxiliary::Registration<'bound, ForLt!(NovaCoreApi<'_>)>,
+    #[uses('gpu: invariant)]
+    _reg: auxiliary::Registration<'gpu, ForLt!(NovaCoreApi<'_>)>,
     #[pin]
-    pub(crate) gpu: Gpu<'bound>,
+    #[uses('bar: invariant, 'bar1)]
+    pub(crate) gpu: Gpu<'bar>,
     bar: pci::Bar<'bound, BAR0_SIZE>,
     bar1: pci::Bar<'bound>,
 }
@@ -129,44 +131,25 @@ fn probe<'bound>(
                 crate::irq::doorbell_test::run_selftest(pdev, bar, spec.chipset)?;
             },
 
-            // TODO: Use self-referential pin-init syntax once available.
-            gpu <- Gpu::new(
-                pdev,
-                // SAFETY: `bar` is initialized above, pinned, and outlives `gpu`.
-                unsafe { &*core::ptr::from_ref(bar) },
-                // SAFETY: `bar1` is initialized above, pinned, and outlives `gpu`.
-                unsafe { &*core::ptr::from_ref(bar1) },
-            ).pin_chain(|_gpu| {
+            gpu <- Gpu::new(pdev, bar, bar1).pin_chain(|_gpu| {
                 #[cfg(CONFIG_NOVA_CORE_SELFTESTS)]
                 _gpu.run_selftests(pdev);
                 Ok(())
             }),
 
-            _reg: {
-                // TODO: Use `&gpu` self-referential pin-init syntax once available.
-                //
-                // SAFETY: `gpu` is initialized before this expression is evaluated
-                // (`try_pin_init!()` initializes fields in initializer order), lives at
-                // a pinned stable address, and is dropped after `_reg` (struct field
-                // drop order).
-                let gpu = unsafe {
-                    Pin::new_unchecked(&*core::ptr::from_ref(gpu.as_ref().get_ref()))
-                };
-
-                // SAFETY: `NovaCore` is dropped when the device is unbound;
-                // i.e. `mem::forget()` is never called on it.
-                unsafe {
-                    auxiliary::Registration::new_with_lt(
-                        pdev.as_ref(),
-                        c"nova-drm",
-                        // TODO[XARR]: Use XArray or perhaps IDA for proper ID
-                        // allocation/recycling. For now, use a simple atomic counter that
-                        // never recycles IDs.
-                        AUXILIARY_ID_COUNTER.fetch_add(1, Relaxed),
-                        crate::MODULE_NAME,
-                        NovaCoreApi { gpu, pdev },
-                    )?
-                }
+            // SAFETY: `NovaCore` is dropped when the device is unbound; i.e. `mem::forget()` is
+            // never called on it.
+            _reg: unsafe {
+                auxiliary::Registration::new_with_lt(
+                    pdev.as_ref(),
+                    c"nova-drm",
+                    // TODO[XARR]: Use XArray or perhaps IDA for proper ID
+                    // allocation/recycling. For now, use a simple atomic counter that
+                    // never recycles IDs.
+                    AUXILIARY_ID_COUNTER.fetch_add(1, Relaxed),
+                    crate::MODULE_NAME,
+                    NovaCoreApi::new(pdev, gpu),
+                )?
             },
         })
     }
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index d185e927233f..cbd720a4d11f 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -342,15 +342,15 @@ struct GspResources<'gpu> {
 /// The two drop as a unit, the handler first, on the drop of [`Gpu`] and on the error path of
 /// its constructor alike, so that the subtree is disabled only after the handler is freed.
 #[pin_data]
-struct GspSubtree<'a> {
+struct GspSubtree<'a, 'gpu> {
     #[pin]
-    irq: GspIrq<'a>,
+    irq: GspIrq<'a, 'gpu>,
     /// Must be kept declared *after* `irq`. A handler still in flight enables the subtree again
     /// through its rearm.
     _top: TopEnableGuard<'a>,
 }
 
-impl<'a> GspSubtree<'a> {
+impl<'a, 'gpu: 'a> GspSubtree<'a, 'gpu> {
     /// Returns an initializer that registers the GSP event handler and then enables its subtree at
     /// `TOP`.
     ///
@@ -362,7 +362,7 @@ unsafe fn new(
         pdev: &'a pci::Device<device::Bound>,
         tree: &'a Tree<'a>,
         falcon: &'a Falcon<'a, GspFalcon>,
-        cmdq: &'a Cmdq<'a>,
+        cmdq: &'a Cmdq<'gpu>,
     ) -> impl PinInit<Self, Error> + 'a {
         try_pin_init!(Self {
             // SAFETY: this function's caller must not leak the `GspSubtree` that owns this
@@ -383,7 +383,8 @@ pub(crate) struct Gpu<'gpu> {
     /// any in-flight run of it has finished, before the command queue that it drains and the
     /// falcon that it reads are freed, and before the GSP is unloaded.
     #[pin]
-    _gsp_subtree: GspSubtree<'gpu>,
+    #[uses('tree)]
+    _gsp_subtree: GspSubtree<'gsp_resources, 'gpu>,
     /// Static GPU information as provided by the GSP.
     pub(crate) gsp_static_info: GetGspStaticInfoReply,
     /// GPU memory manager owning memory management resources.
@@ -403,9 +404,6 @@ pub(crate) struct Gpu<'gpu> {
     /// Must be kept declared *after* `gsp_resources`, as the latter's `PinnedDrop` implementation
     /// requires the sysmem flush page to be in place.
     sysmem_flush: SysmemFlush<'gpu>,
-    /// Borrow of `tree` that `_gsp_subtree` holds. A field that borrows a sibling field is
-    /// self-referential, which `pin_init` cannot express, so the borrow is taken by hand.
-    tree_ref: &'gpu Tree<'gpu>,
     /// The GIN CPU interrupt tree and the PCI vectors that deliver it.
     ///
     /// Must be kept declared *after* `_gsp_subtree`, which holds a borrow of it.
@@ -456,10 +454,6 @@ pub(crate) fn new<'a>(
 
             tree: Tree::new(pdev, bar, spec.chipset, irq::gsp::GSP_SUBTREE.into())?,
 
-            // SAFETY: `tree` is initialized above, is pinned at a stable address, and is dropped
-            // after every field that uses `tree_ref` (struct field drop order).
-            tree_ref: unsafe { &*core::ptr::from_ref(tree.as_ref().get_ref()) },
-
             _: {
                 let dma_mask = hal::gpu_hal(spec.chipset).dma_mask();
 
@@ -513,19 +507,17 @@ pub(crate) fn new<'a>(
             }),
 
             _: {
-                irq::gsp::quiesce(tree_ref, &gsp_resources.gsp_falcon);
+                irq::gsp::quiesce(tree.get_ref(), &gsp_resources.gsp_falcon);
             },
 
-            // SAFETY: the GSP falcon and the command queue are fields of `gsp_resources`, which
-            // is initialized above and pinned, so both references outlive the registration. The
-            // registration is a field of `Gpu` and is never leaked, so its `Drop` runs, and field
-            // drop order runs it before either is freed.
+            // SAFETY: The registration is owned by `Gpu`, which is dropped when the device is
+            // unbound, so the registration's `Drop` runs.
             _gsp_subtree <- unsafe {
                 GspSubtree::new(
                     pdev,
-                    tree_ref,
-                    &*core::ptr::from_ref(&gsp_resources.gsp_falcon),
-                    &*core::ptr::from_ref(&gsp_resources.gsp.cmdq),
+                    tree.get_ref(),
+                    &gsp_resources.get_ref().gsp_falcon,
+                    &gsp_resources.get_ref().gsp.cmdq,
                 )
             },
 
diff --git a/drivers/gpu/nova-core/irq/gsp.rs b/drivers/gpu/nova-core/irq/gsp.rs
index 1abde8e0a405..f4fe7b5d691e 100644
--- a/drivers/gpu/nova-core/irq/gsp.rs
+++ b/drivers/gpu/nova-core/irq/gsp.rs
@@ -50,15 +50,16 @@ pub(crate) fn quiesce(tree: &Tree<'_>, falcon: &Falcon<'_, GspFalcon>) {
 }
 
 /// Threaded IRQ handler for the GSP event.
-pub(crate) struct GspInterrupt<'a> {
+pub(crate) struct GspInterrupt<'a, 'gpu> {
     falcon: &'a Falcon<'a, GspFalcon>,
-    cmdq: &'a Cmdq<'a>,
+    // `Cmdq` is invariant in `'gpu`; the handler borrows it for the shorter `'a`.
+    cmdq: &'a Cmdq<'gpu>,
     tree: &'a Tree<'a>,
     /// For logging.
     dev: &'a device::Device,
 }
 
-impl GspInterrupt<'_> {
+impl GspInterrupt<'_, '_> {
     /// Clears the latch of every host-routed cause in `status` other than SWGEN0, and logs them.
     ///
     /// Returns the causes still set after the clear. A cause driven from outside the falcon stays
@@ -83,7 +84,7 @@ fn clear_faults(
     }
 }
 
-impl irq::ThreadedHandler for GspInterrupt<'_> {
+impl irq::ThreadedHandler for GspInterrupt<'_, '_> {
     /// Top half, in hard interrupt context. Services the GSP vector only, so another vector
     /// pending in the same leaf stays pending.
     fn handle(&self) -> irq::ThreadedIrqReturn {
@@ -145,13 +146,13 @@ fn handle_threaded(&self) -> irq::IrqReturn {
 /// The declaration order is the drop order, and it is required: the vector is disabled before
 /// `free_irq` runs. See "Enabling the GSP event" in `Documentation/gpu/nova/core/interrupts.rst`.
 #[pin_data]
-pub(crate) struct GspIrq<'a> {
+pub(crate) struct GspIrq<'a, 'gpu> {
     _leaf_guard: LeafEnableGuard<'a>,
     #[pin]
-    reg: irq::ThreadedRegistration<'a, GspInterrupt<'a>>,
+    reg: irq::ThreadedRegistration<'a, GspInterrupt<'a, 'gpu>>,
 }
 
-impl<'a> GspIrq<'a> {
+impl<'a, 'gpu: 'a> GspIrq<'a, 'gpu> {
     /// Returns an initializer that registers the threaded handler and then enables the GSP
     /// vector at its leaf.
     ///
@@ -171,7 +172,7 @@ pub(crate) unsafe fn new(
         pdev: &'a pci::Device<device::Bound>,
         tree: &'a Tree<'a>,
         falcon: &'a Falcon<'a, GspFalcon>,
-        cmdq: &'a Cmdq<'a>,
+        cmdq: &'a Cmdq<'gpu>,
     ) -> impl PinInit<Self, Error> + 'a {
         let dev = pdev.as_ref();
 

base-commit: a7cdb17ca0d7ce67e9f3a52d39edd60f03223729
-- 
2.56.0