Claude review: gpu: nova-core: gsp: run the unload bundle if Gsp::boot() fails

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

Patch Review

This patch adds a `BootUnloadGuard` — a `ScopeGuard` that automatically runs the unload bundle if boot fails partway through.

**Observations:**

1. The `BootUnloadGuard` wraps `ScopeGuard<BootUnloadArgs, fn(BootUnloadArgs)>`:
```rust
pub(super) struct BootUnloadGuard<'a> {
    guard: ScopeGuard<BootUnloadArgs<'a>, fn(BootUnloadArgs<'a>)>,
}
```
Using a concrete function pointer `fn(BootUnloadArgs)` instead of a closure avoids lifetime complications with closures capturing references. Clean approach.

2. The guard's callback calls `Gsp::unload`:
```rust
|args| {
    let _ = super::Gsp::unload(
        args.gsp,
        args.dev,
        args.bar,
        args.gsp_falcon,
        args.sec2_falcon,
        args.unload_bundle,
    );
},
```
The `let _ =` discards the result in the drop path — correct, since there's nothing to do with errors during cleanup of a failed boot.

3. The `dismiss()` method returns the unload bundle for the caller to take ownership:
```rust
pub(super) fn dismiss(self) -> Option<super::UnloadBundle> {
    self.guard.dismiss().unload_bundle
}
```
This is the standard `ScopeGuard` pattern — on the success path, dismiss the guard and hand the resource to the normal owner.

4. The lifetime threading `'a` through the HAL `boot` signature is well done:
```rust
fn boot<'a>(
    &self,
    gsp: &'a Gsp,
    dev: &'a device::Device<device::Bound>,
    bar: &'a Bar0,
    ...
    gsp_falcon: &'a Falcon<GspEngine>,
    sec2_falcon: &'a Falcon<Sec2>,
) -> Result<BootUnloadGuard<'a>>;
```
This ensures the guard borrows from the same lifetime as the falcons and device, which is correct — the guard must not outlive the hardware resources it references.

5. **One concern**: The `BootUnloadGuard`'s drop callback calls `Gsp::unload`, which includes `shutdown_gsp` (sends `UNLOADING_GUEST_DRIVER` command). If boot fails early enough that the GSP isn't actually running yet (e.g., before `gsp_falcon.boot()`), the `shutdown_gsp` command will fail (5-second timeout). The unload bundle's `run()` would then also execute. This is probably fine — `shutdown_gsp` failure is logged and continued past, and the unload bundle itself should work regardless of GSP state. But it does mean a boot failure will incur an extra 5-second timeout from the futile shutdown attempt. Not a functional issue, but worth noting for debugging (the error logs will appear).

6. In `tu102.rs`, the guard construction placement is correct — it's created after the unload bundle is built but before FWSEC-FRTS:
```rust
let unload_bundle = Sec2UnloadBundle::build(...).ok().map(crate::gsp::UnloadBundle);
let unload_guard = BootUnloadGuard::new(gsp, dev, bar, gsp_falcon, sec2_falcon, unload_bundle);

// FWSEC-FRTS, falcon reset, booter load all happen after this
// Any `?` will trigger the guard
```
This means if FWSEC-FRTS, falcon boot, or booter load fails, the guard fires and cleans up. If unload bundle prep itself failed (returned `None`), the guard fires but `unload` logs a warning and returns `EAGAIN` — the hardware still won't be cleaned up, but that was already the case.

7. The `gh100.rs` stub correctly propagates the lifetime change and still returns `Err(ENOTSUPP)` — no guard needed since boot never succeeds.

**No issues.** The drop guard pattern is well-implemented and addresses a real concern about partial boot failures.

---

**Summary**: This is a clean, well-reviewed v7 series that addresses a real operational problem (inability to rebind without GPU reset). The HAL abstraction is forward-looking without being over-engineered, the error handling is thorough (continue-on-error in teardown paths, pre-load firmware at boot time), and the drop guard in patch 4 closes the gap for partial boot failures. All four patches are ready to merge.

---
Generated by Claude Code Patch Reviewer