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

[Danilo Krummrich]

On Tue Mar 10, 2026 at 3:01 AM CET, Gary Guo wrote:
>> +// TODO: Revert to private once `IoView` projections replace the `gsp_mem` module.
>> +pub(in crate::gsp) struct Msgq {
>
> These could all be `(in super)`?

Yes, or just pub(super). However, that's not the case for the functions in the
gsp_mem module, they could be pub(in super::super) though. But I think I prefer
pub(in crate::gsp) for those.

>> +    pub(in crate::gsp) fn gsp_write_ptr(qs: &CoherentAllocation<GspMem>) -> u32 {
>> +        // PANIC: A `dma::CoherentAllocation` always contains at least one element.
>> +        || -> Result<u32> { Ok(dma_read!(qs, [0]?.gspq.tx.0.writePtr) % MSGQ_NUM_PAGES) }().unwrap()
>
> I wonder if I should add a panicking variant of index projection for this case.
> Perhaps of syntax `[index]!`.
>
> We could also make the existing `[index]` becoming a panicking one instead of
> `build_error!` one. It is more consistent with Rust index operator that way.

I thought the same, as something like this `[n]?.ptes[i]` looks a bit odd.

However, I think we ideally want both variants (I like your `[i]!` proposal
above), since generally users should have the choice (as they also have with a
slice through get()). For instance, the index could come from userspace. Sure,
you can always validate the index in advance, but having a fallible variant is a
bit nicer.