Re: [PATCH v3 08/33] gpu: nova-core: gsp: compute the queue regions from a count and a slot

["Gary Guo" <gary@garyguo.net>]

On Wed Sep 23, 2026 at 2:20 PM BST, Alexandre Courbot wrote:
> On Wed Sep 23, 2026 at 8:30 PM JST, Gary Guo wrote:
>> On Wed Sep 23, 2026 at 5:55 AM BST, Alexandre Courbot wrote:
>>> On Fri Sep 18, 2026 at 10:06 AM JST, John Hubbard wrote:
>>>>      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 = num::u32_as_usize(self.free_slots());
>>>> +        let w_slot = num::u32_as_usize(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]);
>>>>  
>>>> -        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)
>>>> -        };
>>>> -
>>>>          // 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)),
>>>> -            )
>>>> -        }
>>>> +        // - `data` points to the `MSGQ_NUM_PAGES` initialized entries of the CPU message queue.
>>>> +        // - The returned slices cover the `avail` free slots from the write pointer on, which the
>>>> +        //   GSP does not read until `advance_cpu_write_ptr` publishes them.
>>>> +        // - `split_at_mut` gives two non-overlapping halves, and the `&mut self` borrow lasts as
>>>> +        //   long as the returned slices, so that no other call hands out the same region while
>>>> +        //   they live.
>>>> +        let data =
>>>> +            unsafe { core::slice::from_raw_parts_mut(data, num::u32_as_usize(MSGQ_NUM_PAGES)) };
>>>> +        let (before_w, after_w) = data.split_at_mut(w_slot);
>>>
>>> This creates a reference over the whole ring, including the parts owned
>>> by the GSP, which breaks the `Coherent` safety contract that the device
>>> must not be able to read or write to a live slice. So we'll need to call
>>> `from_raw_parts_mut` twice, with the correct sizes, instead of
>>> splitting.
>>>
>>> (also `split_at_mut` is panicking and should have a `PANIC:` comment
>>> justifying why it cannot, but once the point above is addressed that
>>> call will go away).
>>>
>>> I wanted to try it locally and ended up with something that seems to
>>> work, so let me share it to save some time:
>>>
>>>     fn driver_write_area(&mut self) -> (&mut [[u8; GSP_PAGE_SIZE]], &mut [[u8; GSP_PAGE_SIZE]]) {
>>>         let avail = self.free_slots();
>>>         let w_slot = 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]);
>>>
>>>         let in_after = avail.min(MSGQ_NUM_PAGES - w_slot);
>>>         let in_before = avail - in_after;
>>>
>>>         // SAFETY:
>>>         // - `data` was created from a valid pointer of `MSGQ_NUM_PAGES` entries.
>>>         // - The `in_after` entries after `w_slot` belong to the `avail` entries that the driver is
>>>         //   currently allowed to write.
>>>         // - The `in_before` first entries belong to the `avail` entries that the driver is
>>>         //   currently allowed to write.
>>>         // - The slices do not overlap.
>>>         unsafe {
>>>             (
>>>                 core::slice::from_raw_parts_mut(
>>>                     data.add(num::u32_as_usize(w_slot)),
>>>                     num::u32_as_usize(in_after),
>>>                 ),
>>>                 core::slice::from_raw_parts_mut(data, num::u32_as_usize(in_before)),
>>>             )
>>>         }
>>>     }
>>>
>>> It has turned out quite short, which I like! I also opted to work with
>>> the original `u32` until the very end, as it results in less conversions
>>> overall.
>>
>> Possibly take some thing from the old projection syntax rework series?
>>
>> https://lore.kernel.org/rust-for-linux/20260415-projection-syntax-rework-v1-4-450723cb3727@garyguo.net/
>
> Oh yes, I forgot about this patch. Do you mean using `ptr::project` to
> create the final sub-slices, or am I missing something else?

I think it's possible to use I/O projection on `CoherentView` and then you
assert that it's not concurrently accessed by turning them from `CoherentView`
to `&mut []` using `CoherentView::as_mut`.

Best,
Gary