On Fri Mar 6, 2026 at 1:48 AM CET, Alexandre Courbot wrote:
> On Thu Mar 5, 2026 at 8:16 PM JST, Danilo Krummrich wrote:
>> @Alex: It also seems that this is based on broken code, i.e. I noticed how
>> the
>> DMA read is done in this case in e.g. gsp_read_ptr().
>>
>> fn cpu_read_ptr(&self) -> u32 {
>> let gsp_mem = self.0.start_ptr();
>>
>> // SAFETY:
>> // - The ['CoherentAllocation'] contains at least one object.
>> // - By the invariants of CoherentAllocation the pointer is valid.
>> (unsafe { (*gsp_mem).cpuq.rx.read_ptr() } % MSGQ_NUM_PAGES)
>> }
>>
>> Why isn't this using dma_read!()? I think creating this reference is UB.
>
> We can't - technically we would have to have the `dma_read` in `cmdq.rs`
> so it can access the `CoherentAllocation` (and do an unwrap in the
> process):
>
> dma_read!(self.0, 0, .gspq.rx.0.readPtr).unwrap()
>
> ... but that cannot be done as `MsgqRxHeader` is part of the bindings
> (i.e. in `fw.rs`) and thus its internal fields are not visible to
> `cmdq.rs`, as per our policy of making the bindigns opaque.
We can have a helpers for doing such dma_read!() calls in gsp/fw.rs instead, and
just forward from the actual methods.
fn cpu_read_ptr(&self) -> u32 {
fw:gsp_mem::cpu_rx_ptr(self) % MSGQ_NUM_PAGES
}
> This can probably be done better with I/O projections, but for now we have to
> do the read_volatile by ourselves.
Not necessarily, see above.
> What makes this reference UB btw?
Gary explained this in another reply already; I think fixing this with Opaque or
passing raw pointers instead involves even more unsafe. Whereas the simple
indirection from above is fully safe and can easily replaced with I/O
projections once we have them.
