[PATCH v2 0/5] gpu: nova-core: gsp: cmdq: preparatory changes for r000 support

[PATCH v2 0/5] gpu: nova-core: gsp: cmdq: preparatory changes for r000 support

[Alexandre Courbot]

This series contains patches 5, 6 and 8 from John's r000 series v3 [1]
that prepare the command queue so that the r000 switch applies more
cleanly. They are submitted separately to make review and merging
faster, and because they are also valuable even without the r000 switch.

Patch 1 is a drive-by fix of the `NV_PFSP` register declarations that is
included here as the `NV_PGSP` declarations mirror the `NV_PFSP` ones.
Patch 5 is another requirement for the r000 command queue.

No functional change is introduced by this series.

Applies cleanly on `drm-rust-next`, and runtime-tested successfully on
Ampere.

[1] https://lore.kernel.org/20260918010719.1176945-1-jhubbard@nvidia.com

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
Changes in v2:
- Add patch returning the sequence number of posted commands.
- Add `Fixes:` tag to patch 1. (Eliot)
- Deslop a couple of commit messages. (Eliot)
- Use I/O projections in the command queue. (Gary & Eliot)
- Link to v1: https://patch.msgid.link/20261030-r000-cmdq-prep-v1-0-f2befbcd8ba9@nvidia.com

---
Alexandre Courbot (2):
      gpu: nova-core: fix stride of NV_PFSP_* registers
      gpu: nova-core: gsp: cmdq: expose the sequence number of sent commands

John Hubbard (3):
      gpu: nova-core: regs: add msgq v2 BAR0 register declarations
      gpu: nova-core: gsp: cmdq: ring the GSP doorbell from the queue memory
      gpu: nova-core: gsp: cmdq: compute the queue regions from a count and a slot

 drivers/gpu/nova-core/gsp/cmdq.rs     | 187 ++++++++++++++++------------------
 drivers/gpu/nova-core/gsp/cmdq/rpc.rs |   4 +-
 drivers/gpu/nova-core/gsp/regs.rs     |  22 +++-
 drivers/gpu/nova-core/regs.rs         |   8 +-
 4 files changed, 118 insertions(+), 103 deletions(-)
---
base-commit: 9fe580fd90e20c8432832c5f9a7e92b049ae9af6
change-id: 20261030-r000-cmdq-prep-6c5644619c11

Best regards,
--  
Alexandre Courbot <acourbot@nvidia.com>

[PATCH v2 1/5] gpu: nova-core: fix stride of NV_PFSP_* registers

[Alexandre Courbot]

OpenRM defines these registers with a stride of 8 bytes, so reflect that
fact in our own declarations. That inconsistency went unnoticed because
nova-core only ever uses the first register of each array at the moment.

Fixes: ab42c32347ef ("gpu: nova-core: Hopper/Blackwell: add FSP message infrastructure")
Reviewed-by: John Hubbard <jhubbard@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/regs.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 88662da634dc..df2eddc58efb 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -402,19 +402,19 @@ pub(crate) fn mem_scrubbing_done(self) -> bool {
 register! {
     base: NovaRegisters;
 
-    pub(crate) NV_PFSP_QUEUE_HEAD(u32)[8] @ 0x008f2c00 {
+    pub(crate) NV_PFSP_QUEUE_HEAD(u32)[8, stride = 8] @ 0x008f2c00 {
         31:0    address => u32;
     }
 
-    pub(crate) NV_PFSP_QUEUE_TAIL(u32)[8] @ 0x008f2c04 {
+    pub(crate) NV_PFSP_QUEUE_TAIL(u32)[8, stride = 8] @ 0x008f2c04 {
         31:0    address => u32;
     }
 
-    pub(crate) NV_PFSP_MSGQ_HEAD(u32)[8] @ 0x008f2c80 {
+    pub(crate) NV_PFSP_MSGQ_HEAD(u32)[8, stride = 8] @ 0x008f2c80 {
         31:0    val => u32;
     }
 
-    pub(crate) NV_PFSP_MSGQ_TAIL(u32)[8] @ 0x008f2c84 {
+    pub(crate) NV_PFSP_MSGQ_TAIL(u32)[8, stride = 8] @ 0x008f2c84 {
         31:0    val => u32;
     }
 }

-- 
2.56.0

[PATCH v2 2/5] gpu: nova-core: regs: add msgq v2 BAR0 register declarations

[Alexandre Courbot]

From: John Hubbard <jhubbard@nvidia.com>

Msgq is the pair of ring buffers in shared memory through which the
driver and GSP-RM communicate. The command queue carries commands from
the driver to GSP-RM, and the message queue carries messages from GSP-RM
to the driver. Each ring buffer has a write pointer and a read pointer.

Msgq v0, which the r570 firmware uses, keeps both pointers of each ring
buffer in that ring buffer's header, in shared memory. Msgq v2, which
the r000 firmware uses, keeps the four pointers, two per ring buffer, in
four BAR0 registers instead:

    NV_PGSP_QUEUE_HEAD    command queue write pointer
    NV_PGSP_QUEUE_TAIL    command queue read pointer
    NV_PGSP_MSGQ_HEAD     message queue write pointer
    NV_PGSP_MSGQ_TAIL     message queue read pointer

The GSP numbers its queues 0 through 7, and each numbered queue is one
such pair of ring buffers with its own four pointer registers. Nova-core
uses queue 0 only.

A write to NV_PGSP_QUEUE_HEAD also interrupts the GSP. Msgq v0 uses that
write as the doorbell, so nova-core already declares NV_PGSP_QUEUE_HEAD.

Declare the other three, NV_PGSP_QUEUE_TAIL, NV_PGSP_MSGQ_HEAD and
NV_PGSP_MSGQ_TAIL.

No functional change intended.

[ acourbot: declare the registers as arrays of 8 with a stride of 8. ]
[ acourbot: use "val" for the field name of NV_PGSP_MSGQ as per OpenRM. ]

Assisted-by: LLM
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs |  6 +++++-
 drivers/gpu/nova-core/gsp/regs.rs | 22 +++++++++++++++++++++-
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 936406004a57..07b21a6f4504 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -15,6 +15,7 @@
     io::{
         io_project,
         poll::read_poll_timeout,
+        register::Array,
         Io, //
     },
     new_mutex,
@@ -496,7 +497,10 @@ fn calculate_checksum<T: Iterator<Item = u8>>(it: T) -> u32 {
 
     /// Notifies the GSP that we have updated the command queue pointers.
     fn notify_gsp(bar: Bar0<'_>) {
-        bar.write_reg(regs::NV_PGSP_QUEUE_HEAD::zeroed().with_address(0u32));
+        bar.write(
+            Array::at(0),
+            regs::NV_PGSP_QUEUE_HEAD::zeroed().with_address(0u32),
+        );
     }
 
     /// Logs and consumes every message the GSP has already posted, and returns without waiting for
diff --git a/drivers/gpu/nova-core/gsp/regs.rs b/drivers/gpu/nova-core/gsp/regs.rs
index 3c410d65e8e4..e3fbbf364b7b 100644
--- a/drivers/gpu/nova-core/gsp/regs.rs
+++ b/drivers/gpu/nova-core/gsp/regs.rs
@@ -9,12 +9,32 @@
 
 // PGSP
 
+// The GSP has eight numbered queues, each with its own four msgq pointer registers interleaved
+// at a stride of 8 bytes. nova-core uses queue 0 only.
+
 register! {
     base: NovaRegisters;
 
-    pub(super) NV_PGSP_QUEUE_HEAD(u32) @ 0x00110c00 {
+    /// Write pointer of the CPU-to-GSP command queue, which the driver advances. A write to this
+    /// register also interrupts the GSP, which makes it the doorbell.
+    pub(super) NV_PGSP_QUEUE_HEAD(u32)[8, stride = 8] @ 0x00110c00 {
         31:0    address;
     }
+
+    /// Read pointer of the CPU-to-GSP command queue, which the GSP advances.
+    pub(super) NV_PGSP_QUEUE_TAIL(u32)[8, stride = 8] @ 0x00110c04 {
+        31:0    address;
+    }
+
+    /// Write pointer of the GSP-to-CPU message queue, which the GSP advances.
+    pub(super) NV_PGSP_MSGQ_HEAD(u32)[8, stride = 8] @ 0x00110c80 {
+        31:0    val;
+    }
+
+    /// Read pointer of the GSP-to-CPU message queue, which the driver advances.
+    pub(super) NV_PGSP_MSGQ_TAIL(u32)[8, stride = 8] @ 0x00110c84 {
+        31:0    val;
+    }
 }
 
 // PBUS

-- 
2.56.0

[PATCH v2 3/5] gpu: nova-core: gsp: cmdq: ring the GSP doorbell from the queue memory

[Alexandre Courbot]

From: John Hubbard <jhubbard@nvidia.com>

A command reaches GSP-RM in two steps. The driver advances the command
queue's write pointer, which publishes the command, and then writes
NV_PGSP_QUEUE_HEAD, which interrupts the GSP so that it reads the queue.
In msgq v0, the write pointer is in the queue's shared-memory header, so
the two steps are two writes. In msgq v2, the write pointer is
NV_PGSP_QUEUE_HEAD itself, so one register write does both.

Nova-core split the two steps across two types. The queue memory type
advanced the write pointer in shared memory, and the command queue type
then rang the doorbell through a BAR0 mapping that it held for that one
purpose.

Move the BAR0 mapping into the queue memory type, and ring the doorbell
at the end of the operation that advances the write pointer in order to
prepare for the switch to msgq v2.

No functional change intended.

[ acourbot: slightly reword and trim commit message. ]

Assisted-by: LLM
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 52 +++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 07b21a6f4504..90e26c4dcc28 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -157,11 +157,16 @@ unsafe impl FromBytes for GspMem {}
 ///   pointer and the GSP read pointer. This region is returned by [`Self::driver_write_area`].
 /// * The driver owns (i.e. can read from) the part of the GSP message queue between the CPU read
 ///   pointer and the GSP write pointer. This region is returned by [`Self::driver_read_area`].
-struct DmaGspMem<'a>(Coherent<'a, GspMem>);
+struct DmaGspMem<'a> {
+    /// The queues, mapped for the GSP.
+    mem: Coherent<'a, GspMem>,
+    /// MMIO mapping of PCI BAR0, for the doorbell register.
+    bar: Bar0<'a>,
+}
 
 impl<'a> DmaGspMem<'a> {
     /// Allocate a new instance and map it for `dev`.
-    fn new(dev: &'a device::Device<device::Bound>) -> Result<Self> {
+    fn new(dev: &'a device::Device<device::Bound>, bar: Bar0<'a>) -> Result<Self> {
         const MSGQ_SIZE: u32 = cv!(size_of::<Msgq>());
         const RX_HDR_OFF: u32 = cv!(mem::offset_of!(Msgq, rx));
 
@@ -172,7 +177,7 @@ fn new(dev: &'a device::Device<device::Bound>) -> Result<Self> {
         let gsp_mem: Coherent<'_, _> = gsp_mem.into();
         PteArray::init(io_project!(gsp_mem, .ptes), gsp_mem.dma_address())?;
 
-        Ok(Self(gsp_mem))
+        Ok(Self { mem: gsp_mem, bar })
     }
 
     /// Returns the region of the CPU message queue that the driver is currently allowed to write
@@ -185,7 +190,7 @@ fn new(dev: &'a device::Device<device::Bound>) -> Result<Self> {
         let rx = self.gsp_read_ptr();
 
         // Pointer to the first entry of the CPU message queue.
-        let data = ptr::project!(mut self.0.as_mut_ptr(), .cpuq.msgq.data[build: 0]);
+        let data = ptr::project!(mut self.mem.as_mut_ptr(), .cpuq.msgq.data[build: 0]);
 
         let (tail_end, wrap_end) = if rx == 0 {
             // The write area is non-wrapping, and stops at the second-to-last entry of the command
@@ -247,7 +252,7 @@ fn driver_write_area_size(&self) -> usize {
         let rx = self.cpu_read_ptr();
 
         // Pointer to the first entry of the GSP message queue.
-        let data = ptr::project!(self.0.as_ptr(), .gspq.msgq.data[build: 0]);
+        let data = ptr::project!(self.mem.as_ptr(), .gspq.msgq.data[build: 0]);
 
         let (tail_end, wrap_end) = if rx <= tx {
             // Read area is non-wrapping and stops right before `tx`.
@@ -331,7 +336,7 @@ fn allocate_command(&mut self, size: usize, timeout: Delta) -> Result<GspCommand
     //
     // - The returned value is within `0..MSGQ_NUM_PAGES`.
     fn gsp_write_ptr(&self) -> u32 {
-        let ptr = MsgqTxHeader::write_ptr(io_project!(self.0, .gspq.tx)) % MSGQ_NUM_PAGES;
+        let ptr = MsgqTxHeader::write_ptr(io_project!(self.mem, .gspq.tx)) % MSGQ_NUM_PAGES;
 
         // ORDERING: LOAD->LOAD ordering needed to order `gsp_write_ptr` read before data read.
         dma_mb(Read);
@@ -345,7 +350,7 @@ fn gsp_write_ptr(&self) -> u32 {
     //
     // - The returned value is within `0..MSGQ_NUM_PAGES`.
     fn gsp_read_ptr(&self) -> u32 {
-        let ptr = MsgqRxHeader::read_ptr(io_project!(self.0, .gspq.rx)) % MSGQ_NUM_PAGES;
+        let ptr = MsgqRxHeader::read_ptr(io_project!(self.mem, .gspq.rx)) % MSGQ_NUM_PAGES;
 
         // ORDERING: LOAD->STORE ordering needed to order `gsp_read_ptr` read before data write.
         dma_mb(Full);
@@ -359,7 +364,7 @@ fn gsp_read_ptr(&self) -> u32 {
     //
     // - The returned value is within `0..MSGQ_NUM_PAGES`.
     fn cpu_read_ptr(&self) -> u32 {
-        MsgqRxHeader::read_ptr(io_project!(self.0, .cpuq.rx)) % MSGQ_NUM_PAGES
+        MsgqRxHeader::read_ptr(io_project!(self.mem, .cpuq.rx)) % MSGQ_NUM_PAGES
     }
 
     // Informs the GSP that it can send `elem_count` new pages into the message queue.
@@ -367,7 +372,7 @@ fn advance_cpu_read_ptr(&mut self, elem_count: u32) {
         // ORDERING: LOAD->STORE ordering needed to order `cpu_read_ptr` write after data read.
         dma_mb(Full);
 
-        let rx = io_project!(self.0, .cpuq.rx);
+        let rx = io_project!(self.mem, .cpuq.rx);
         let rptr = MsgqRxHeader::read_ptr(rx).wrapping_add(elem_count) % MSGQ_NUM_PAGES;
         MsgqRxHeader::set_read_ptr(rx, rptr)
     }
@@ -378,17 +383,24 @@ fn advance_cpu_read_ptr(&mut self, elem_count: u32) {
     //
     // - The returned value is within `0..MSGQ_NUM_PAGES`.
     fn cpu_write_ptr(&self) -> u32 {
-        MsgqTxHeader::write_ptr(io_project!(self.0, .cpuq.tx)) % MSGQ_NUM_PAGES
+        MsgqTxHeader::write_ptr(io_project!(self.mem, .cpuq.tx)) % MSGQ_NUM_PAGES
     }
 
-    // Informs the GSP that it can process `elem_count` new pages from the command queue.
+    // Publishes `elem_count` more pages of the command queue to the GSP and rings the doorbell.
     fn advance_cpu_write_ptr(&mut self, elem_count: u32) {
         // ORDERING: STORE->STORE ordering needed to order `cpu_write_ptr` write after data write.
         dma_mb(Write);
 
-        let tx = io_project!(self.0, .cpuq.tx);
+        let tx = io_project!(self.mem, .cpuq.tx);
         let wptr = MsgqTxHeader::write_ptr(tx).wrapping_add(elem_count) % MSGQ_NUM_PAGES;
         MsgqTxHeader::set_write_ptr(tx, wptr);
+
+        // A write to the head register interrupts the GSP. The pointer itself is in the
+        // shared-memory header, so the value written does not matter.
+        self.bar.write(
+            Array::at(0),
+            regs::NV_PGSP_QUEUE_HEAD::zeroed().with_address(0u32),
+        );
     }
 }
 
@@ -468,13 +480,12 @@ pub(crate) fn new(
         bar: Bar0<'cmdq>,
     ) -> impl PinInit<Self, Error> + 'cmdq {
         pin_init_scope(move || {
-            let gsp_mem = DmaGspMem::new(dev)?;
+            let gsp_mem = DmaGspMem::new(dev, bar)?;
 
             Ok(try_pin_init!(Self {
-                dma_addr: gsp_mem.0.dma_address(),
+                dma_addr: gsp_mem.mem.dma_address(),
                 inner <- new_mutex!(CmdqInner {
                     dev,
-                    bar,
                     gsp_mem,
                     seq: 0,
                 }),
@@ -495,14 +506,6 @@ fn calculate_checksum<T: Iterator<Item = u8>>(it: T) -> u32 {
         ((sum64 >> 32) as u32) ^ (sum64 as u32)
     }
 
-    /// Notifies the GSP that we have updated the command queue pointers.
-    fn notify_gsp(bar: Bar0<'_>) {
-        bar.write(
-            Array::at(0),
-            regs::NV_PGSP_QUEUE_HEAD::zeroed().with_address(0u32),
-        );
-    }
-
     /// Logs and consumes every message the GSP has already posted, and returns without waiting for
     /// more.
     ///
@@ -522,8 +525,6 @@ pub(crate) fn drain(&self) -> Result {
 struct CmdqInner<'a> {
     /// Device this command queue belongs to.
     dev: &'a device::Device,
-    /// MMIO mapping of PCI BAR0, for writing the GSP doorbell.
-    bar: Bar0<'a>,
     /// Current command sequence number.
     seq: u32,
     /// Memory area shared with the GSP for communicating commands and messages.
@@ -571,7 +572,6 @@ fn send_command_element<E: CommandElement>(&mut self, command: E) -> Result {
         let elem_count = dst.header.element_count();
         self.seq += 1;
         self.gsp_mem.advance_cpu_write_ptr(elem_count);
-        Cmdq::notify_gsp(self.bar);
 
         Ok(())
     }

-- 
2.56.0

[PATCH v2 4/5] gpu: nova-core: gsp: cmdq: compute the queue regions from a count and a slot

[Alexandre Courbot]

From: John Hubbard <jhubbard@nvidia.com>

`driver_write_area` and `driver_read_area` both branch on the relative
position of the read and write pointers to decide where the two slices
they return end. This works for msgq v0, where the pointers are indices
into the ring, but msgq v2 (used by the r000 firmware) keeps them in
BAR0 registers as free-running counters, so the branches and their
SAFETY arguments would have to be rewritten for it.

Instead, compute the number of slots available to the driver and the
slot they start at, and derive the slices from that. The slot counts
come from one helper per queue, which are the only two places msgq v2
will need to touch.

Use I/O projections to limit the amount of unsafe code to the bare
minimum.

No functional change intended.

Assisted-by: LLM
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Co-developed-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 130 +++++++++++++++++---------------------
 1 file changed, 58 insertions(+), 72 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 90e26c4dcc28..33cb9174c1e6 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -19,8 +19,8 @@
         Io, //
     },
     new_mutex,
+    num::casts::FromSafeCast,
     prelude::*,
-    ptr,
     sync::{
         barrier::{
             dma_mb,
@@ -186,60 +186,58 @@ fn new(dev: &'a device::Device<device::Bound>, bar: Bar0<'a>) -> Result<Self> {
     /// As the message queue is a circular buffer, the region may be discontiguous in memory. In
     /// that case the second slice will have a non-zero length.
     fn driver_write_area(&mut self) -> (&mut [[u8; GSP_PAGE_SIZE]], &mut [[u8; GSP_PAGE_SIZE]]) {
-        let tx = self.cpu_write_ptr();
-        let rx = self.gsp_read_ptr();
+        let avail = usize::from_safe_cast(self.cpuq_free_slots());
+        let w_slot = usize::from_safe_cast(self.cpu_write_ptr());
 
-        // Pointer to the first entry of the CPU message queue.
-        let data = ptr::project!(mut self.mem.as_mut_ptr(), .cpuq.msgq.data[build: 0]);
+        // `w_slot` is below `MSGQ_NUM_PAGES` per the invariant of `cpu_write_ptr`, so the
+        // subtraction cannot underflow.
+        let before_wrap = avail.min(cv!(MSGQ_NUM_PAGES => usize) - w_slot);
+        // Per the line above, `before_wrap` is less or equal to `avail` so the subtraction cannot
+        // underflow.
+        let after_wrap = avail - before_wrap;
 
-        let (tail_end, wrap_end) = if rx == 0 {
-            // The write area is non-wrapping, and stops at the second-to-last entry of the command
-            // queue (to leave the last one empty).
-            (MSGQ_NUM_PAGES - 1, 0)
-        } else if rx <= tx {
-            // The write area wraps and continues until `rx - 1`.
-            (MSGQ_NUM_PAGES, rx - 1)
-        } else {
-            // The write area doesn't wrap and stops at `rx - 1`.
-            (rx - 1, 0)
-        };
+        // PANIC: `w_slot + before_wrap` is at most `MSGQ_NUM_PAGES` by construction of
+        // `before_wrap`.
+        let after = io_project!(self.mem, .cpuq.msgq.data[panic: w_slot..w_slot + before_wrap]);
+        // PANIC: `after_wrap` is at most `avail`, which is below `MSGQ_NUM_PAGES`.
+        let before = io_project!(self.mem, .cpuq.msgq.data[panic: ..after_wrap]);
 
         // SAFETY:
-        // - `data` was created from a valid pointer, and `rx` and `tx` are in the
-        //   `0..MSGQ_NUM_PAGES` range per the invariants of `cpu_write_ptr` and `gsp_read_ptr`,
-        //   thus the created slices are valid.
-        // - The area starting at `tx` and ending at `rx - 2` modulo `MSGQ_NUM_PAGES`,
-        //   inclusive, belongs to the driver for writing and is not accessed concurrently by
-        //   the GSP.
-        // - The caller holds a reference to `self` for as long as the returned slices are live,
-        //   meaning the CPU write pointer cannot be advanced and thus that the returned area
-        //   remains exclusive to the CPU for the duration of the slices.
-        // - The created slices point to non-overlapping sub-ranges of `data` in all
-        //   branches (in the `rx <= tx` case, the second slice ends at `rx - 1` which is strictly
-        //   less than `tx` where the first slice starts; in the other cases the second slice is
-        //   empty), so creating two `&mut` references from them does not violate aliasing rules.
-        unsafe {
-            (
-                core::slice::from_raw_parts_mut(
-                    data.add(num::u32_as_usize(tx)),
-                    num::u32_as_usize(tail_end - tx),
-                ),
-                core::slice::from_raw_parts_mut(data, num::u32_as_usize(wrap_end)),
-            )
-        }
+        // - Both regions are within the `avail` entries starting at `w_slot` that the driver owns,
+        //   meaning the GSP won't touch them until the write pointer is advanced.
+        // - There are no other references possible as `self` is mutably borrowed.
+        // - The regions do not overlap: `avail` is at most `MSGQ_NUM_PAGES - 1`, so `after_wrap` is
+        //   strictly less than `w_slot` whenever it is non-zero.
+        unsafe { (after.as_mut(), before.as_mut()) }
     }
 
-    /// Returns the size of the region of the CPU message queue that the driver is currently allowed
-    /// to write to, in bytes.
-    fn driver_write_area_size(&self) -> usize {
+    /// Returns the number of slots of the CPU message queue that the driver is currently allowed
+    /// to write to.
+    fn cpuq_free_slots(&self) -> u32 {
         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
+        (rx + MSGQ_NUM_PAGES - tx - 1) % MSGQ_NUM_PAGES
+    }
+
+    /// Returns the size of the region of the CPU message queue that the driver is currently allowed
+    /// to write to, in bytes.
+    fn driver_write_area_size(&self) -> usize {
+        num::u32_as_usize(self.cpuq_free_slots()) * GSP_PAGE_SIZE
+    }
+
+    /// Returns the number of slots of the GSP message queue that the driver is currently allowed
+    /// to read from.
+    fn gspq_readable_slots(&self) -> u32 {
+        let tx = self.gsp_write_ptr();
+        let rx = self.cpu_read_ptr();
+
+        // `rx` and `tx` are both in `0..MSGQ_NUM_PAGES` per the invariants of `gsp_write_ptr` and
+        // `cpu_read_ptr`, so the subtraction cannot underflow.
+        (tx + MSGQ_NUM_PAGES - rx) % MSGQ_NUM_PAGES
     }
 
     /// Returns the region of the GSP message queue that the driver is currently allowed to read
@@ -248,39 +246,27 @@ fn driver_write_area_size(&self) -> usize {
     /// As the message queue is a circular buffer, the region may be discontiguous in memory. In
     /// that case the second slice will have a non-zero length.
     fn driver_read_area(&self) -> (&[[u8; GSP_PAGE_SIZE]], &[[u8; GSP_PAGE_SIZE]]) {
-        let tx = self.gsp_write_ptr();
-        let rx = self.cpu_read_ptr();
+        let avail = usize::from_safe_cast(self.gspq_readable_slots());
+        let r_slot = usize::from_safe_cast(self.cpu_read_ptr());
 
-        // Pointer to the first entry of the GSP message queue.
-        let data = ptr::project!(self.mem.as_ptr(), .gspq.msgq.data[build: 0]);
+        // `r_slot` is below `MSGQ_NUM_PAGES` per the invariant of `cpu_read_ptr`, so the
+        // subtraction cannot underflow.
+        let before_wrap = avail.min(cv!(MSGQ_NUM_PAGES => usize) - r_slot);
+        // Per the line above, `before_wrap` is less or equal to `avail` so the subtraction cannot
+        // underflow.
+        let after_wrap = avail - before_wrap;
 
-        let (tail_end, wrap_end) = if rx <= tx {
-            // Read area is non-wrapping and stops right before `tx`.
-            (tx, 0)
-        } else {
-            // Read area is wrapping and stops right before `tx`.
-            (MSGQ_NUM_PAGES, tx)
-        };
+        // PANIC: `r_slot + before_wrap` is at most `MSGQ_NUM_PAGES` by construction of
+        // `before_wrap`.
+        let after = io_project!(self.mem, .gspq.msgq.data[panic: r_slot..r_slot + before_wrap]);
+        // PANIC: `after_wrap` is at most `avail`, which is below `MSGQ_NUM_PAGES`.
+        let before = io_project!(self.mem, .gspq.msgq.data[panic: ..after_wrap]);
 
         // SAFETY:
-        // - `data` was created from a valid pointer, and `rx` and `tx` are in the
-        //   `0..MSGQ_NUM_PAGES` range per the invariants of `gsp_write_ptr` and `cpu_read_ptr`,
-        //   thus the created slices are valid.
-        // - The area starting at `rx` and ending at `tx - 1` modulo `MSGQ_NUM_PAGES`,
-        //   inclusive, belongs to the driver for reading and is not accessed concurrently by
-        //   the GSP.
-        // - The caller holds a reference to `self` for as long as the returned slices are live,
-        //   meaning the CPU read pointer cannot be advanced and thus that the returned area
-        //   remains exclusive to the CPU for the duration of the slices.
-        unsafe {
-            (
-                core::slice::from_raw_parts(
-                    data.add(num::u32_as_usize(rx)),
-                    num::u32_as_usize(tail_end - rx),
-                ),
-                core::slice::from_raw_parts(data, num::u32_as_usize(wrap_end)),
-            )
-        }
+        // - Both regions are within the `avail` entries starting at `r_slot` that the driver owns,
+        //   meaning the GSP won't touch them until the read pointer is advanced.
+        // - There are no mutable references possible as `self` is borrowed.
+        unsafe { (after.as_ref(), before.as_ref()) }
     }
 
     /// Allocates a region on the command queue that is large enough to send a command of `size`

-- 
2.56.0

Re: [PATCH v2 4/5] gpu: nova-core: gsp: cmdq: compute the queue regions from a count and a slot

[Eliot Courtney]

On Mon Nov 2, 2026 at 9:09 PM JST, Alexandre Courbot wrote:
> From: John Hubbard <jhubbard@nvidia.com>
>
> `driver_write_area` and `driver_read_area` both branch on the relative
> position of the read and write pointers to decide where the two slices
> they return end. This works for msgq v0, where the pointers are indices
> into the ring, but msgq v2 (used by the r000 firmware) keeps them in
> BAR0 registers as free-running counters, so the branches and their
> SAFETY arguments would have to be rewritten for it.
>
> Instead, compute the number of slots available to the driver and the
> slot they start at, and derive the slices from that. The slot counts
> come from one helper per queue, which are the only two places msgq v2
> will need to touch.
>
> Use I/O projections to limit the amount of unsafe code to the bare
> minimum.
>
> No functional change intended.
>
> Assisted-by: LLM
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> Co-developed-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drivers/gpu/nova-core/gsp/cmdq.rs | 130 +++++++++++++++++---------------------
>  1 file changed, 58 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index 90e26c4dcc28..33cb9174c1e6 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -19,8 +19,8 @@
>          Io, //
>      },
>      new_mutex,
> +    num::casts::FromSafeCast,
>      prelude::*,
> -    ptr,
>      sync::{
>          barrier::{
>              dma_mb,
> @@ -186,60 +186,58 @@ fn new(dev: &'a device::Device<device::Bound>, bar: Bar0<'a>) -> Result<Self> {
>      /// As the message queue is a circular buffer, the region may be discontiguous in memory. In
>      /// that case the second slice will have a non-zero length.
>      fn driver_write_area(&mut self) -> (&mut [[u8; GSP_PAGE_SIZE]], &mut [[u8; GSP_PAGE_SIZE]]) {
> -        let tx = self.cpu_write_ptr();
> -        let rx = self.gsp_read_ptr();
> +        let avail = usize::from_safe_cast(self.cpuq_free_slots());
> +        let w_slot = usize::from_safe_cast(self.cpu_write_ptr());
>  
> -        // Pointer to the first entry of the CPU message queue.
> -        let data = ptr::project!(mut self.mem.as_mut_ptr(), .cpuq.msgq.data[build: 0]);
> +        // `w_slot` is below `MSGQ_NUM_PAGES` per the invariant of `cpu_write_ptr`, so the
> +        // subtraction cannot underflow.
> +        let before_wrap = avail.min(cv!(MSGQ_NUM_PAGES => usize) - w_slot);
> +        // Per the line above, `before_wrap` is less or equal to `avail` so the subtraction cannot
> +        // underflow.
> +        let after_wrap = avail - before_wrap;
>  
> -        let (tail_end, wrap_end) = if rx == 0 {
> -            // The write area is non-wrapping, and stops at the second-to-last entry of the command
> -            // queue (to leave the last one empty).
> -            (MSGQ_NUM_PAGES - 1, 0)
> -        } else if rx <= tx {
> -            // The write area wraps and continues until `rx - 1`.
> -            (MSGQ_NUM_PAGES, rx - 1)
> -        } else {
> -            // The write area doesn't wrap and stops at `rx - 1`.
> -            (rx - 1, 0)
> -        };
> +        // PANIC: `w_slot + before_wrap` is at most `MSGQ_NUM_PAGES` by construction of
> +        // `before_wrap`.
> +        let after = io_project!(self.mem, .cpuq.msgq.data[panic: w_slot..w_slot + before_wrap]);
> +        // PANIC: `after_wrap` is at most `avail`, which is below `MSGQ_NUM_PAGES`.
> +        let before = io_project!(self.mem, .cpuq.msgq.data[panic: ..after_wrap]);

It looks like compared to v1, this kept the `after` and `before` names I
suggested but changed the indices (e.g. in_after => before_wrap). It
means that "before" and "after" are being used in two different ways in
this function ("after wrapping" and "after write pointer"). What about
picking one usage for the entire function? Either is fine imo. Same for
the other spot (read side).

With that changed:

Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>

[PATCH v2 5/5] gpu: nova-core: gsp: cmdq: expose the sequence number of sent commands

[Alexandre Courbot]

The r000 firmware requires matching the sequence number of sent commands
with the responses received from the GSP. Make `send_command_element`
return the sequence number to that end, and ignore it for now.

No functional change intended.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs     | 11 +++++++----
 drivers/gpu/nova-core/gsp/cmdq/rpc.rs |  4 +++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 33cb9174c1e6..65f25d3c4d98 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -524,27 +524,30 @@ impl CmdqInner<'_> {
     /// Allocate enough space on the command queue to store `command`, initialize it with
     /// [`CommandElement::write`], and send the command to the GSP.
     ///
+    /// Returns the sequence number of the sent element.
+    ///
     /// # Errors
     ///
     /// - `EMSGSIZE` if the command exceeds the maximum queue element size.
     /// - `ETIMEDOUT` if space does not become available within the timeout.
     ///
     /// Error codes returned by [`CommandElement::write`] are returned as-is.
-    fn send_command_element<E: CommandElement>(&mut self, command: E) -> Result {
+    fn send_command_element<E: CommandElement>(&mut self, command: E) -> Result<u32> {
         let size_in_bytes = command.size();
+        let seq = self.seq;
         let mut dst = self
             .gsp_mem
             .allocate_command(size_in_bytes, Self::ALLOCATE_TIMEOUT)?;
 
         // Fill the header.
-        let msg_element_init = GspMsgElement::init(self.seq, size_in_bytes);
+        let msg_element_init = GspMsgElement::init(seq, size_in_bytes);
         // SAFETY: `dst.header` is a valid reference, and is not touched if the initializer fails.
         unsafe {
             pin_init::raw_try_init(core::ptr::from_mut(dst.header), msg_element_init)?;
         }
 
         // Write the message payload.
-        command.write(self.dev, self.seq, &mut dst)?;
+        command.write(self.dev, seq, &mut dst)?;
 
         // Compute checksum now that the whole message is ready.
         dst.header
@@ -559,7 +562,7 @@ fn send_command_element<E: CommandElement>(&mut self, command: E) -> Result {
         self.seq += 1;
         self.gsp_mem.advance_cpu_write_ptr(elem_count);
 
-        Ok(())
+        Ok(seq)
     }
 
     /// Wait for a message to become available on the message queue.
diff --git a/drivers/gpu/nova-core/gsp/cmdq/rpc.rs b/drivers/gpu/nova-core/gsp/cmdq/rpc.rs
index 2c52ec78871b..36cba7c28d3e 100644
--- a/drivers/gpu/nova-core/gsp/cmdq/rpc.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq/rpc.rs
@@ -354,7 +354,9 @@ fn send_rpc_command<M>(&mut self, command: M) -> Result
         Error: From<M::InitError>,
     {
         match SplitState::new(command)? {
-            SplitState::Single(command) => self.send_command_element(RpcCommandElement(command)),
+            SplitState::Single(command) => self
+                .send_command_element(RpcCommandElement(command))
+                .map(|_| ()),
             SplitState::Split(command, mut continuations) => {
                 self.send_command_element(RpcCommandElement(command))?;
 

-- 
2.56.0

Re: [PATCH v2 5/5] gpu: nova-core: gsp: cmdq: expose the sequence number of sent commands

[Eliot Courtney]

On Mon Nov 2, 2026 at 9:09 PM JST, Alexandre Courbot wrote:
> The r000 firmware requires matching the sequence number of sent commands
> with the responses received from the GSP. Make `send_command_element`
> return the sequence number to that end, and ignore it for now.
>
> No functional change intended.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---

Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>