[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@xxxxxxxxxx>

`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@xxxxxxxxxx>
Co-developed-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
---
 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