Re: [PATCH v2 05/11] rust: io: restrict untyped IO access and `register!` to `Region`
From: Gary Guo
Date: Tue Apr 28 2026 - 09:10:08 EST
On Tue Apr 28, 2026 at 1:08 PM BST, Andreas Hindborg wrote:
> "Gary Guo" <gary@xxxxxxxxxxx> writes:
>
>> On Tue Apr 28, 2026 at 10:02 AM BST, Andreas Hindborg wrote:
>>> Gary Guo <gary@xxxxxxxxxxx> writes:
>>>
>>>> Currently the `Io` trait exposes a bunch of untyped IO accesses, but if the
>>>> `Io` region itself is typed, then it might be weird to have
>>>>
>>>> let io: Mmio<u32> = /* ... */;
>>>> io.read8(1);
>>>>
>>>> while not unsound, it is surely strange. Thus, restrict the untyped methods
>>>> and also the register macro to `Region` type only.
>>>>
>>>> The way it is implemented is by adding a generic type to `IoLoc`. This also
>>>> paves the way to add typed register blocks in the future; for example, we
>>>> could use this mechanism to block driver A's `register!()` generated macro
>>>> from being used on driver B's MMIO. The same mechanism could be used for
>>>> relative IO registers. These are future opoortunities, and for this patch I
>>>> just restricted everything to require `IoLoc<Region<SIZE>, _>`.
>>>
>>> Does this not prevent `usize` from being used to index anything but
>>> `Mmio<Region<_>>`?
>>>
>>> It is my understanding that the following would work before this patch:
>>>
>>> fn do_read(io: &Mmio<u32>) -> Result {
>>> let v: u32 = io.try_read(8usize)?;
>>> Ok(())
>>> }
>>>
>>> But I think this will no longer work with this patch. Is that the intention?
>>
>> Your example would always fail, as you're reading 4 bytes from offset 4 from a
>> region that is only 4 bytes in size. I suppose you're trying to say
>>
>> fn do_read(io: &Mmio<[u32]>) -> Result {
>> let v: u32 = io.try_read(8usize)?;
>> Ok(())
>> }
>
> Yep, that was my intention, with the assumption that final offset
> becomes 8*4. I think that is also what would happen here as we would
> invoke `try_read::<u32, usize>(&Mmio<[u32]>, usize) -> Result<u32>` with
>
> usize: IoLoc<u32>
> Mmio<[u32]>: IoCapable<u32>
No, these methods behave the same as the old try_read32 macros, so these are
absolute byte offsets. I think the fact you're confused is a good indication
that we probably want to remove these methods, or at least make them as
inaccessible as possible :)
>
>
>> ? In any case, it's intended to be unsupported. For types regions you can use
>> projection. So the same code can be written as
>>
>> fn do_read(io: &Mmio<[u32]>) -> Result {
>> let v: u32 = io_read!(io, [try: 2]);
>> Ok(())
>> }
>>
>> i.e. reading from index 2 instead of byte offset 8. If one cares about byte
>> offset they would probably want to use the register macro instead or having the
>> MMIO region untyped.
>
> Right, makes sense. Again, my thoughts were not around the byte-offset,
> but the `location` parameter being in count of elements of `T`.
>
> But why remove the ability to call `try_read::<u16, usize>(&Mmio<[u32]>,
> usize) -> Result<u16>`?
If you're doing that it sounds like you don't actually want typed access and
again, would want to use register macro to allow mixed register sizes.
Best,
Gary