Re: [PATCH v3 10/16] rust: io: register: make register have a typed base

"Alexandre Courbot" <[email protected]>
Newsgroups dev.linux.lists.nova-gpu,dev.linux.lists.driver-core,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci,org.kernel.vger.rust-for-linux
Message-ID <[email protected]>
On Thu Aug 27, 2026 at 11:32 PM JST, Gary Guo wrote:
> On Thu Aug 27, 2026 at 2:42 PM BST, Alexandre Courbot wrote:
>> On Wed Aug 19, 2026 at 8:09 PM JST, Gary Guo wrote:
>>> Previously `register!` defined registers can be used on any untyped I/O
>>> regions. With all users specifying their desired register type now,
>>> propagate the specified type and restrict I/O access only when type
>>> matches.
>>>
>>> Also, add an `io_project!` example which is enabled by this change.
>>>
>>> Signed-off-by: Gary Guo <[email protected]>
>>> ---
>>>  rust/kernel/io.rs          | 13 +++++++++++
>>>  rust/kernel/io/register.rs | 55 ++++++++++++++++++++++++++++++++--------------
>>>  rust/macros/io/register.rs | 34 ++++++++++++++--------------
>>>  3 files changed, 68 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
>>> index 84dd876b3407..4542187d6b91 100644
>>> --- a/rust/kernel/io.rs
>>> +++ b/rust/kernel/io.rs
>>> @@ -1692,21 +1692,34 @@ pub fn project_loc<U, L>(self, location: L) -> <T::Backend as IoBackend>::View<'
>>>  /// The syntax is of form `io_project!(io, proj)` where `io` is an expression to a type that
>>>  /// implements [`Io`] and `proj` is a [projection specification](kernel::ptr::project!).
>>>  ///
>>> +/// `io_project!` can also project to subview of registers defined with [`register!`] macro.
>>
>> nit: "to a subview".
>>
>> <...>
>>> +/// Helper function for register alias implementation.
>>> +///
>>> +/// This is used to enforce base matching. Only called during const eval.
>>> +#[doc(hidden)]
>>> +#[inline(always)]
>>> +pub const fn alias_offset<Base: ?Sized, Alias: Register<Base = Base>>() -> usize {
>>> +    Alias::OFFSET
>>> +}
>>> +
>>> +/// Helper function for register element alias implementation.
>>> +///
>>> +/// This is used to enforce base matching and provide bounds checking. Only called during const
>>> +/// eval.
>>> +#[doc(hidden)]
>>> +#[inline(always)]
>>> +pub const fn element_alias_offset<Base: ?Sized, Alias: RegisterArray<Base = Base>>(
>>> +    idx: usize,
>>> +) -> usize {
>>> +    build_assert!(idx < Alias::SIZE);
>>> +    Alias::OFFSET + idx * Alias::STRIDE
>>> +}
>>
>> You could convert the `build_assert!` (always good to eschew) into a
>> `const_assert!` if you turn `idx` into a generic parameter:
>>
>>     pub const fn element_alias_offset<
>>         Base: ?Sized,
>>         Alias: RegisterArray<Base = Base>,
>>         const IDX: usize,
>>     >() -> usize {
>>         crate::const_assert!(IDX < Alias::SIZE);
>>         Alias::OFFSET + IDX * Alias::STRIDE
>>     }
>>
>> You get a better error message, the const parameter is used in a
>> very controlled environment well within the expressive power of const
>> generics, and that function is not public interface anyway.
>>
>> If you don't like it (recent discussions make me think you might not
>> :)), then let's remove the `build_assert!` and keep the `static_assert`
>> emitted by the macro until this patch; it's not as elegant as checking
>> the condition into the same block of code that uses it, but again we are
>> in a controlled environment and static_assert > const_assert >
>> build_assert so this may actually be my preferred solution. You can
>> document the invariant in `element_alias_offset`.
>
> The error message should be identical? This function is only called during const
> evaluation, so even if I replace this build_assert to assert it'll be fine and
> never actually cause linker error or runtime panic.

Indeed, I missed that. Even in this case, it's still better to use
something the compiler enforces can only run at const eval.

`build_assert` is something that in an ideal world shouldn't need to
exist, so let's avoid proliferating its use if we can avoid it.

>
> I wonder if I should add
>
>     assert_in_const_eval()
>
> or maybe
>
>     #[const_eval_only]
>     fn ...
>
> which expands to
>
>     #[inline(always)]
>     fn const ... {
>         assert_in_const_eval()
>     }
>
> so that it's clear to reader that this is a const-eval-only function.

If that doesn't delay the series, sure.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.