Claude review: gpu: nova-core: gsp: add mutex locking to Cmdq

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

Patch Review

This is the main patch. It splits `Cmdq` into an outer struct (with `dev` and the `Mutex`) and `CmdqInner` (with `seq` and `gsp_mem`).

**The split is clean:**
```rust
struct CmdqInner {
    seq: u32,
    gsp_mem: DmaGspMem,
}

pub(crate) struct Cmdq {
    dev: ARef<device::Device>,
    #[pin]
    inner: Mutex<CmdqInner>,
}
```

**Methods that need `&mut` go to `CmdqInner`, stateless helpers stay on `Cmdq`.** The `dev` reference is passed as a parameter to `CmdqInner` methods. This is a reasonable pattern to avoid putting `dev` inside the mutex.

**`send_sync_command` holds the lock across send+receive:**
```rust
pub(crate) fn send_sync_command<M>(&self, bar: &Bar0, command: M) -> Result<M::Reply> {
    let mut inner = self.inner.lock();
    inner.send_command(&self.dev, bar, command)?;
    loop {
        match inner.receive_msg::<M::Reply>(&self.dev, Delta::from_secs(10)) {
            Ok(reply) => break Ok(reply),
            Err(ERANGE) => continue,
            Err(e) => break Err(e),
        }
    }
}
```
This is the correct approach for reply integrity. As noted above, it does block other queue access while waiting. The `read_poll_timeout` inside `wait_for_msg` will sleep (via `Delta::from_millis(1)` polling interval), so at least the mutex won't be hot-spinning — the kernel `Mutex` is a sleeping lock. This is important and correct.

**`send_async_command` only holds the lock for the send:**
```rust
pub(crate) fn send_async_command<M>(&self, bar: &Bar0, command: M) -> Result {
    self.inner.lock().send_command(&self.dev, bar, command)
}
```
Good — drops the lock immediately after sending.

**`receive_msg` on outer Cmdq:**
```rust
pub(crate) fn receive_msg<M: MessageFromGsp>(&self, timeout: Delta) -> Result<M> {
    self.inner.lock().receive_msg(&self.dev, timeout)
}
```
Used by `wait_gsp_init_done` and the sequencer. The lock is held during the poll timeout, same concern as above. For the init-done case, this is a single-threaded initialization path, so it's fine.

**`dma_handle()` concern:**
```rust
pub(crate) fn dma_handle(&self) -> DmaAddress {
    self.inner.lock().gsp_mem.0.dma_handle()
}
```
As noted in the overall review, this takes the lock to read an immutable value. Minor issue — could be optimized by storing the DMA handle (or the entire `DmaGspMem`) outside the mutex, but this is not a correctness problem and can be addressed later.

**All callers updated from `&mut self` / `&mut Cmdq` to `&self` / `&Cmdq`:**
```rust
-        mut self: Pin<&mut Self>,
+        self: Pin<&mut Self>,
```
```rust
-pub(crate) fn wait_gsp_init_done(cmdq: &mut Cmdq) -> Result {
+pub(crate) fn wait_gsp_init_done(cmdq: &Cmdq) -> Result {
```
```rust
-    pub(crate) fn run(cmdq: &mut Cmdq, ...) -> Result {
+    pub(crate) fn run(cmdq: &Cmdq, ...) -> Result {
```
All consistent and correct.

**Minor formatting nit:** In the `new_mutex!` invocation, the closing `})` has inconsistent indentation:
```rust
        try_pin_init!(Self {
            inner <- new_mutex!(CmdqInner {
                gsp_mem: DmaGspMem::new(dev)?,
                seq: 0,
                }),
            dev: dev.into(),
        })
```
The `})` closing `new_mutex!` should probably be at the same level as `inner`, i.e.:
```rust
            inner <- new_mutex!(CmdqInner {
                gsp_mem: DmaGspMem::new(dev)?,
                seq: 0,
            }),
```

This is purely cosmetic.

Overall a solid patch. **Reviewed-by worthy** with the optional consideration of moving `dma_handle` out of the locked state.

---
Generated by Claude Code Patch Reviewer