Re: [PATCH v2 05/11] rust: io: restrict untyped IO access and `register!` to `Region`

[Andreas Hindborg <a.hindborg@kernel.org>]

"Gary Guo" <gary@garyguo.net> writes:

> On Tue Apr 28, 2026 at 1:08 PM BST, Andreas Hindborg wrote:
>> "Gary Guo" <gary@garyguo.net> writes:
>>
>>> On Tue Apr 28, 2026 at 10:02 AM BST, Andreas Hindborg wrote:
>>>> Gary Guo <gary@garyguo.net> 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 :)

That is probably a good call 😅 Going over the pieces again, I am not
sure when and where I decided this was an index rather than a byte
offset.


Best regards,
Andreas Hindborg