Claude review: gpu: nova-core: fb: make sure to unregister SysmemFlush on boot failure

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**The bug being fixed is real and important.** Looking at `Gpu::new` (gpu.rs:249-275), the `try_pin_init!` macro initializes fields sequentially. If any field after `sysmem_flush` fails (e.g., `Falcon::new` at line 262, `Gsp::boot` at line 272), the already-initialized `SysmemFlush` is dropped. Without a `Drop` impl, the DMA page is freed but the GPU register `NV_PFB_NISO_FLUSH_SYSMEM_ADDR` still points to it.

**Adding `Arc<Devres<Bar0>>` to `SysmemFlush`:**

```rust
pub(crate) struct SysmemFlush {
    chipset: Chipset,
    device: ARef<device::Device>,
+   bar: Arc<Devres<Bar0>>,
    page: CoherentHandle,
}
```

This is the right approach. `SysmemFlush` needs independent access to the BAR for its `Drop` impl, since it can't rely on receiving a `&Bar0` reference as a parameter during drop. The `Arc` clone is cheap and guarantees the `Devres<Bar0>` outlives the `SysmemFlush` even if `Gpu`'s own `bar` field is dropped first (Rust drops struct fields in declaration order, and `bar` is declared before `sysmem_flush` in the `Gpu` struct).

**The `Drop` implementation:**

```rust
impl Drop for SysmemFlush {
    fn drop(&mut self) {
        let _ = self.bar.try_access_with(|bar| self.unregister(bar));
    }
}
```

This is correct. `try_access_with` returns `None` if the `Devres` has been revoked (device unbound), in which case the hardware is already torn down and unregistering is unnecessary. The `let _ = ...` appropriately discards the `Option<()>` result.

**The `registered_dma_handle != 0` guard:**

```rust
-        } else {
-            // Another page has been registered after us for some reason - warn as this is a bug.
+        } else if registered_dma_handle != 0 {
```

This is a necessary change to avoid a spurious warning on the normal unbind path. After `unbind()` calls `unregister()` (writing 0 to the register), the subsequent `Drop` would read 0, which doesn't match `self.page.dma_handle()`. Without this guard, it would incorrectly warn "attempt to unregister a sysmem flush page that is not active." The new behavior correctly distinguishes three cases:
1. Register matches our page → unregister (write 0)
2. Register holds a different non-zero address → warn (genuine bug)
3. Register is 0 → silently do nothing (already cleaned up)

**Minor observation (not blocking):** The doc comment update is good but could be slightly more precise:

```rust
+/// Users should call [`Self::unregister`] before unloading to ensure unregistering is infallible.
+/// [`Drop`] performs a best-effort fallback using revocable BAR access.
```

The comment says "before unloading" but the actual call site is in `Gpu::unbind`, which happens at unbind time. "Before unloading" could be confused with module unload. A more precise wording might be "during unbind" but this is a very minor documentation nit.

**No issues with the `Gpu::new` call site change:**

```rust
-            sysmem_flush: SysmemFlush::register(pdev.as_ref(), bar, spec.chipset)?,
+            sysmem_flush: SysmemFlush::register(
+                pdev.as_ref(),
+                devres_bar.clone(),
+                bar,
+                spec.chipset,
+            )?,
```

The `devres_bar.clone()` correctly creates a new `Arc` reference for `SysmemFlush` to own. The original `devres_bar` is later moved into `bar: devres_bar` (line 274). If initialization fails before that point, both `Arc`s are dropped independently and the `Devres<Bar0>` refcount goes to zero, properly cleaning up.

**Overall: Clean, well-reasoned fix with no correctness issues.**

---
Generated by Claude Code Patch Reviewer