Re: [PATCH 05/10] gpu: nova-core: gsp: Add GSP command queue handling

["Alexandre Courbot" <acourbot@nvidia.com>]

On Fri Sep 5, 2025 at 8:50 PM JST, Alexandre Courbot wrote:
>> +
>> +        Ok(GspCmdq {
>> +            dev: dev.into(),
>> +            msg_count: MSG_COUNT,
>> +            seq: 0,
>> +            gsp_mem,
>> +            _nr_ptes: nr_ptes as u32,
>> +        })
>> +    }
>> +
>> +    fn cpu_wptr(&self) -> u32 {
>> +        // SAFETY: index `0` is valid as `gsp_mem` has been allocated accordingly, thus the access
>> +        // cannot fail.
>> +        unsafe { dma_read!(self.gsp_mem[0].cpuq.tx.write_ptr).unwrap_unchecked() }
>> +    }
>> +
>> +    fn gsp_rptr(&self) -> u32 {
>> +        // SAFETY: index `0` is valid as `gsp_mem` has been allocated accordingly, thus the access
>> +        // cannot fail.
>> +        unsafe { dma_read!(self.gsp_mem[0].gspq.rx.read_ptr).unwrap_unchecked() }
>> +    }
>> +
>> +    fn cpu_rptr(&self) -> u32 {
>> +        // SAFETY: index `0` is valid as `gsp_mem` has been allocated accordingly, thus the access
>> +        // cannot fail.
>> +        unsafe { dma_read!(self.gsp_mem[0].cpuq.rx.read_ptr).unwrap_unchecked() }
>> +    }
>> +
>> +    fn gsp_wptr(&self) -> u32 {
>> +        // SAFETY: index `0` is valid as `gsp_mem` has been allocated accordingly, thus the access
>> +        // cannot fail.
>> +        unsafe { dma_read!(self.gsp_mem[0].gspq.tx.write_ptr).unwrap_unchecked() }
>> +    }
>
> Here is an easy trick to reduce the number of unsafe statements: have a
> method that returns a reference to the `gsp_mem` (which contains the
> unsafe part), and have these 4 methods call into it. And voilà, 3
> unsafes gone. :)

That sentence of mine is so wrong that I feel obligated to come back and
fix it before someone thinks this is an acceptable pattern. >_<

A method returning a reference to `gsp_mem` would still need to be
unsafe itself. Some areas of `gsp_mem` can be modified by the GSP at any
time, so it would be the responsibility of the caller to make sure that
it doesn't access any area that is currently under GSP ownership.

A better way to do this would be a have methods (safe, this time) that
provide references to the areas that are owned by the driver at the time
of calling.