From: "Gary Guo" <gary@garyguo.net>
To: "Andreas Hindborg" <a.hindborg@kernel.org>,
"Gary Guo" <gary@garyguo.net>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Rafael J. Wysocki" <rafael@kernel.org>,
"Danilo Krummrich" <dakr@kernel.org>,
"Miguel Ojeda" <ojeda@kernel.org>,
"Boqun Feng" <boqun@kernel.org>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <lossin@kernel.org>,
"Alice Ryhl" <aliceryhl@google.com>,
"Trevor Gross" <tmgross@umich.edu>,
"Daniel Almeida" <daniel.almeida@collabora.com>,
"Bjorn Helgaas" <bhelgaas@google.com>,
"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
"Abdiel Janulgue" <abdiel.janulgue@gmail.com>,
"Robin Murphy" <robin.murphy@arm.com>,
"Alexandre Courbot" <acourbot@nvidia.com>,
"David Airlie" <airlied@gmail.com>,
"Simona Vetter" <simona@ffwll.ch>
Cc: <driver-core@lists.linux.dev>, <rust-for-linux@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <linux-pci@vger.kernel.org>,
<nouveau@lists.freedesktop.org>,
<dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH v2 05/11] rust: io: restrict untyped IO access and `register!` to `Region`
Date: Tue, 28 Apr 2026 13:55:00 +0100 [thread overview]
Message-ID: <DI4SY1NXU1XW.2VHPFEKIFBQVW@garyguo.net> (raw)
In-Reply-To: <87tssvp9sq.fsf@t14s.mail-host-address-is-not-set>
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 :)
>
>
>> ? 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
next prev parent reply other threads:[~2026-04-28 12:55 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-21 14:56 [PATCH v2 00/11] rust: I/O type generalization and projection Gary Guo
2026-04-21 14:56 ` [PATCH v2 01/11] rust: io: generalize `MmioRaw` to pointer to arbitrary type Gary Guo
2026-04-27 13:44 ` Andreas Hindborg
2026-04-21 14:56 ` [PATCH v2 02/11] rust: io: generalize `Mmio` " Gary Guo
2026-04-27 14:10 ` Andreas Hindborg
2026-04-21 14:56 ` [PATCH v2 03/11] rust: io: use pointer types instead of address Gary Guo
2026-04-27 14:20 ` Andreas Hindborg
2026-04-27 15:27 ` Gary Guo
2026-04-28 7:12 ` Andreas Hindborg
2026-04-21 14:56 ` [PATCH v2 04/11] rust: io: add missing safety requirement in `IoCapable` methods Gary Guo
2026-04-28 7:16 ` Andreas Hindborg
2026-04-21 14:56 ` [PATCH v2 05/11] rust: io: restrict untyped IO access and `register!` to `Region` Gary Guo
2026-04-28 9:02 ` Andreas Hindborg
2026-04-28 11:14 ` Gary Guo
2026-04-28 12:08 ` Andreas Hindborg
2026-04-28 12:55 ` Gary Guo [this message]
2026-04-28 14:41 ` Andreas Hindborg
2026-04-28 14:54 ` Danilo Krummrich
2026-04-29 8:04 ` Andreas Hindborg
2026-04-21 14:56 ` [PATCH v2 06/11] rust: io: add view type Gary Guo
2026-04-28 10:53 ` Andreas Hindborg
2026-04-28 11:20 ` Gary Guo
2026-04-21 14:56 ` [PATCH v2 07/11] rust: dma: add methods to unsafely create reference from subview Gary Guo
2026-04-28 12:10 ` Andreas Hindborg
2026-04-21 14:56 ` [PATCH v2 08/11] rust: io: add `read_val` and `write_val` function on I/O view Gary Guo
2026-04-28 12:53 ` Andreas Hindborg
2026-04-21 14:56 ` [PATCH v2 09/11] gpu: nova-core: use I/O projection for cleaner encapsulation Gary Guo
2026-04-21 14:56 ` [PATCH v2 10/11] rust: dma: drop `dma_read!` and `dma_write!` API Gary Guo
2026-04-28 11:16 ` Andreas Hindborg
2026-04-21 14:56 ` [PATCH v2 11/11] rust: io: add copying methods Gary Guo
2026-04-28 13:22 ` Andreas Hindborg
2026-04-28 14:08 ` Gary Guo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DI4SY1NXU1XW.2VHPFEKIFBQVW@garyguo.net \
--to=gary@garyguo.net \
--cc=a.hindborg@kernel.org \
--cc=abdiel.janulgue@gmail.com \
--cc=acourbot@nvidia.com \
--cc=airlied@gmail.com \
--cc=aliceryhl@google.com \
--cc=bhelgaas@google.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun@kernel.org \
--cc=dakr@kernel.org \
--cc=daniel.almeida@collabora.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=driver-core@lists.linux.dev \
--cc=gregkh@linuxfoundation.org \
--cc=kwilczynski@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=nouveau@lists.freedesktop.org \
--cc=ojeda@kernel.org \
--cc=rafael@kernel.org \
--cc=robin.murphy@arm.com \
--cc=rust-for-linux@vger.kernel.org \
--cc=simona@ffwll.ch \
--cc=tmgross@umich.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox