Re: [PATCH] gpu: nova-core: gsp: fix UB in DmaGspMem pointer accessors

[Eliot Courtney]

On Tue Mar 10, 2026 at 11:01 AM JST, Gary Guo wrote:
>> +    pub(in crate::gsp) fn advance_cpu_read_ptr(qs: &CoherentAllocation<GspMem>, count: u32) {
>> +        let rptr = cpu_read_ptr(qs).wrapping_add(count) % MSGQ_NUM_PAGES;
>> +
>> +        // Ensure read pointer is properly ordered.
>> +        fence(Ordering::SeqCst);
>> +
>> +        // PANIC: A `dma::CoherentAllocation` always contains at least one element.
>> +        || -> Result {
>> +            dma_write!(qs, [0]?.cpuq.rx.0.readPtr, rptr);
>> +            Ok(())
>> +        }()
>> +        .unwrap()
>> +    }
>> +
>> +    pub(in crate::gsp) fn cpu_write_ptr(qs: &CoherentAllocation<GspMem>) -> u32 {
>> +        // PANIC: A `dma::CoherentAllocation` always contains at least one element.
>> +        || -> Result<u32> { Ok(dma_read!(qs, [0]?.cpuq.tx.0.writePtr) % MSGQ_NUM_PAGES) }().unwrap()
>> +    }
>> +
>> +    pub(in crate::gsp) fn advance_cpu_write_ptr(qs: &CoherentAllocation<GspMem>, count: u32) {
>> +        let wptr = cpu_write_ptr(qs).wrapping_add(count) & MSGQ_NUM_PAGES;
>
> Not really related to your change, but this `&` probably require a comment, as
> it has different behaviour compared to `%` given the `MSGQ_NUM_PAGES` is not
> power of two. I suppose this is actually intended so there's a way to
> distinguish between empty and full ring buffer?
>
> Best,
> Gary

This is actually incorrect and I have fixed it here[1]. I think it
should be merged in drm-rust-next now.

[1]: https://lore.kernel.org/all/20260129-nova-core-cmdq1-v3-0-2ede85493a27@xxxxxxxxxx/