On Fri Jan 23, 2026 at 12:12 PM GMT, Eliot Courtney wrote:
> The current code indexes into `after_rx` using `tx` which is an index
> for the whole buffer, not the split buffer `after_rx`.
>
> Also add more rigorous no-panic proofs.
>
> Fixes: 75f6b1de8133 ("gpu: nova-core: gsp: Add GSP command queue bindings and
> handling")
> Signed-off-by: Eliot Courtney <[email protected]>
> ---
> drivers/gpu/nova-core/gsp/cmdq.rs | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs
> b/drivers/gpu/nova-core/gsp/cmdq.rs
> index aa8758fc7723..c26396fda29c 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -1,7 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0
>
> use core::{
> - cmp,
> mem,
> sync::atomic::{
> fence,
> @@ -267,10 +266,20 @@ fn new(dev: &device::Device<device::Bound>) ->
> Result<Self> {
> // PANIC: per the invariant of `cpu_read_ptr`, `rx` is `<
> MSGQ_NUM_PAGES`.
> let (before_rx, after_rx) = gsp_mem.gspq.msgq.data.split_at(rx);
This code doesn't need splitting as it doesn't have the uniqueness issue that
mutable references have. While you're at it, probably it's chance to simplify
the code.
>
> - match tx.cmp(&rx) {
> - cmp::Ordering::Equal => (&after_rx[0..0], &after_rx[0..0]),
> - cmp::Ordering::Greater => (&after_rx[..tx], &before_rx[0..0]),
> - cmp::Ordering::Less => (after_rx, &before_rx[..tx]),
> + // The area starting at `rx` and ending at `tx - 1` modulo
> MSGQ_NUM_PAGES, inclusive,
> + // belongs to the driver for reading.
> + if rx <= tx {
> + // The area is contiguous.
> + // PANIC:
> + // - The index `tx - rx` is non-negative because `rx <= tx` in
> this branch.
> + // - The index does not exceed `after_rx.len()` (which is
> `MSGQ_NUM_PAGES - rx`)
> + // because `tx < MSGQ_NUM_PAGES` by the `gsp_write_ptr`
> invariant.
> + (&after_rx[..(tx - rx)], &after_rx[0..0])
This can be just `(&data[rx..tx], &[])` without the split.
> + } else {
> + // The area is discontiguous.
> + // PANIC: `tx` does not exceed `before_rx.len()` (which equals
> `rx`) because
> + // `tx < rx` in this branch.
> + (after_rx, &before_rx[..tx])
This can be just `(&data[rx..], &data[..tx])` without the split.
Best,
Gary
> }
> }
>
