From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: gpu: nova-core: gsp: add mechanism to wait for space on command queue
Date: Thu, 19 Feb 2026 18:43:07 +1000
Message-ID: <review-patch2-20260219-cmdq-continuation-v2-2-2e8b7615536f@nvidia.com>
In-Reply-To: <20260219-cmdq-continuation-v2-2-2e8b7615536f@nvidia.com>
References: <20260219-cmdq-continuation-v2-0-2e8b7615536f@nvidia.com> <20260219-cmdq-continuation-v2-2-2e8b7615536f@nvidia.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

This patch adds `driver_write_area_size()` and changes `allocate_command` to poll-wait for space using `read_poll_timeout` instead of returning `EAGAIN` immediately.

> +    fn driver_write_area_size(&self) -> usize {
> +        let tx = self.cpu_write_ptr();
> +        let rx = self.gsp_read_ptr();
> +
> +        // `rx` and `tx` are both in `0..MSGQ_NUM_PAGES` per the invariants of `gsp_read_ptr` and
> +        // `cpu_write_ptr`. The minimum value case is where `rx == 0` and `tx == MSGQ_NUM_PAGES -
> +        // 1`, which gives `0 + MSGQ_NUM_PAGES - (MSGQ_NUM_PAGES - 1) - 1 == 0`.
> +        let slots = (rx + MSGQ_NUM_PAGES - tx - 1) % MSGQ_NUM_PAGES;
> +        num::u32_as_usize(slots) * GSP_PAGE_SIZE
> +    }

The comment says `rx` and `tx` are both in `0..MSGQ_NUM_PAGES`, but the invariant comments on `gsp_read_ptr` and `cpu_write_ptr` say "between `0` and `MSGQ_NUM_PAGES`" which is ambiguous about whether `MSGQ_NUM_PAGES` itself is included. Looking at the code, both functions use `% MSGQ_NUM_PAGES`, so the range is `0..MSGQ_NUM_PAGES` (exclusive of `MSGQ_NUM_PAGES`). The comment here is consistent and the math checks out.

> +        read_poll_timeout(
> +            || Ok(self.driver_write_area_size()),
> +            |available_bytes| *available_bytes >= size_of::<GspMsgElement>() + size,
> +            Delta::ZERO,
> +            timeout,
> +        )?;

After `read_poll_timeout` succeeds, the code calls `self.driver_write_area()` to get the actual mutable slices. Since `gsp_read_ptr` can only advance between the two calls (the GSP consumes data), the actual area is at least as large as what was polled, so no underflow can occur. The `&mut self` receiver prevents concurrent driver-side calls. This is safe.

The 1-second timeout used in `send_command` matches the nouveau driver pattern. No issues found.

---
Generated by Claude Code Patch Reviewer