Claude review: gpu: nova-core: run Booter Unloader and FWSEC-SB upon unbinding

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

Patch Review

This completes the unload sequence by tearing down the WPR2 region.

1. **Removing dead_code/unused annotations** (`firmware/booter.rs`, `firmware/fwsec.rs`):
```rust
-    #[expect(unused)]
     Unloader,
```
```rust
-    #[expect(dead_code)]
     Sb,
```
These variants were pre-declared but unused until now. Clean removal.

2. **WPR2 check helper** (`regs.rs`):
```rust
+    pub(crate) fn is_wpr2_set(self) -> bool {
+        self.hi_val() != 0
+    }
```
Simple and correct — the existing comment on `higher_bound()` says "A value of zero means the WPR2 region is not set."

3. **Full unload sequence** (`gsp/boot.rs`):

The unload now does:
- Shut down GSP (from patch 4)
- Run FWSEC-SB to reset the GSP falcon to pre-libos state
- If WPR2 is still set, run Booter Unloader to remove it
- Verify WPR2 was cleared

The FWSEC-SB and Booter Unloader loading mirrors the boot-time pattern in `boot()` / `run_fwsec_frts()`, using the same `needs_fwsec_bootloader()` dispatch. This symmetry with the boot path is good.

```rust
+        let bios = Vbios::new(dev, bar)?;
+        let fwsec_sb = FwsecFirmware::new(dev, gsp_falcon, bar, &bios, FwsecCommand::Sb)?;
```

One thing to note: `Vbios::new()`, `FwsecFirmware::new()`, and `BooterFirmware::new()` all request firmware from the filesystem. If firmware loading fails during unbind (e.g., firmware files have been removed), the unload will fail with an error and the WPR2 region will remain set, preventing re-probe. The `?` propagation means these failures are reported via `warn_on_err!` in the caller. This is probably the best that can be done — you can't unload without the firmware — but it's a dependency worth documenting.

```rust
+            let _ = sec2_falcon.boot(bar, Some(0xff), Some(0xff))?;
```

The `let _ =` discards the boot return value (a mailbox read result). The `Some(0xff)` arguments for both mailbox values appear to be "don't care" sentinel values (matching the pattern used during boot in the sequencer). The `?` propagation means if SEC2 fails to boot the unloader, we bail out.

```rust
+            let wpr2_hi = bar.read(regs::NV_PFB_PRI_MMU_WPR2_ADDR_HI);
+            if wpr2_hi.is_wpr2_set() {
+                dev_err!(dev, "WPR2 region still set after Booter Unloader ran\n");
+                return Err(EBUSY);
+            }
```

Good defensive check. If the booter unloader ran but didn't actually clear WPR2, returning `EBUSY` is appropriate.

4. **Updated unload signature** (`gpu.rs`):
```rust
+        let _ = kernel::warn_on_err!(self.gsp.unload(
+            dev,
+            bar,
+            self.spec.chipset,
+            &self.gsp_falcon,
+            &self.sec2_falcon,
+        ));
```

Passing chipset and sec2_falcon through is clean — they're needed for the FWSEC-SB bootloader dispatch and Booter Unloader respectively.

**Minor observation on error handling in unbind path:** Both the FWSEC-SB and Booter Unloader steps use `?` internally, which means any failure in the unload sequence will abort the remaining steps but still continue to `sysmem_flush.unregister()` thanks to the `let _ = warn_on_err!(...)` in `unbind()`. This is reasonable best-effort teardown. If FWSEC-SB fails, attempting the Booter Unloader would likely also fail, so the early return within `unload()` makes sense.

No blocking issues.

---
Generated by Claude Code Patch Reviewer