On Fri Mar 6, 2026 at 12:48 AM GMT, 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.
>
> This can probably be done better with I/O projections, but for now we
> have to do the read_volatile by ourselves. What makes this reference UB
> btw?
MsgqRxHeader does not have interior mutability and is not pinned. Thus data must
not change underneath a `&MsgRxHeader`, which isn't true.
To correct this you need to make all methods take a pointer rather than
reference, or wrap the raw binding data inside `Opaque<>`.
Best,
Gary
