linux-kernel - Re: [PATCH v4 5/7] rust: io: add `register!` macro

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DG01XSMD0VL7.2LN9LJPH57FFW@nvidia.com>
Date: Wed, 28 Jan 2026 16:42:34 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Joel Fernandes" <joelagnelf@...dia.com>
Cc: "John Hubbard" <jhubbard@...dia.com>, "Danilo Krummrich"
 <dakr@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>, "Daniel Almeida"
 <daniel.almeida@...labora.com>, "Miguel Ojeda" <ojeda@...nel.org>, "Boqun
 Feng" <boqun.feng@...il.com>, "Gary Guo" <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>, "Benno Lossin"
 <lossin@...nel.org>, "Andreas Hindborg" <a.hindborg@...nel.org>, "Trevor
 Gross" <tmgross@...ch.edu>, "Yury Norov" <yury.norov@...il.com>, "Alistair
 Popple" <apopple@...dia.com>, "Timur Tabi" <ttabi@...dia.com>, "Edwin Peer"
 <epeer@...dia.com>, "Eliot Courtney" <ecourtney@...dia.com>, "Dirk Behme"
 <dirk.behme@...bosch.com>, "Steven Price" <steven.price@....com>,
 <rust-for-linux@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 5/7] rust: io: add `register!` macro

On Wed Jan 28, 2026 at 12:47 PM JST, Joel Fernandes wrote:
> On Jan 27, 2026, at 10:02 PM, John Hubbard <jhubbard@...dia.com> wrote:
>> On 1/27/26 6:37 PM, Alexandre Courbot wrote:
>>> Add a macro for defining hardware register types with I/O accessors.
>> ...
>>> +/// Fields are instances of [`Bounded`](kernel::num::Bounded) and can be read by calling their
>>> +/// getter method, which is named after them. They also have setter methods prefixed with `set_`
>>> +/// for runtime values and `with_` for constant values. All setters return the updated register
>> OK, this still looks like a naming problem that we don't need to create.
>> Let's just pick either "set_" or "with_" as a prefix (it seems that
>> "with_" has won out, in the v3 discussion thread), and then add "const",
>> so that the name doesn't require explanation.
>>
>> ...
>>> +/// // Update some fields and write the new value back.
>>> +/// boot0
>>> +///     // Constant values.
>>> +///     .with_major_revision::<3>()
>>> +///     .with_minor_revision::<10>()
>>> +///     // Run-time value.
>>> +///     .set_vendor_id(obtain_vendor_id())
>>
>> See, that is just not obvious at all, why it's different. That's why
>> you had to write a couple of comments.
>>
>> .with_const_major_revision(), on the other hand, keeps the pattern
>> clear and obvious. No comments necessary.
>>
>> nova-core doesn't even use the const_ settors yet at all, according
>> to my quick read of the (very helpful) [PATCH FOR REFERENCE v4 7/7].
>> So clearly it is a bit rare, and the extra characters won't come into
>> play in daily life.

Great, if everyone agrees on with/with_const then this is definitely
idiomatic and clear (and not too verbose).

I'll respin tomorrow unless there is new input.

>
> I completely agree with John here, let us please not create a confusing API.
> I would much rather use a bounded! macro to create bounded values and pass
> those to a single unified setter, making it a unified API for both constants
> and runtime values. Let us not use const generics just for the sake of it or
> create special const APIs that no one but examples use!!!
>
> For example, instead of having separate set_/with_ prefixes, we could have:
>
>     boot0
>         .with_major_revision(bounded!(u8, 3))
>         .with_minor_revision(bounded!(u8, 10))
>         .with_vendor_id(obtain_vendor_id())
>
> This keeps the syntax clean while still providing compile-time bounds
> checking for constants. A single unified setter API is way cleaner and less
> confusing than having set_ for runtime and with_<N>() for compile-time
> values.
>
> Is there some reason why this will not work?

So these setters are really part of the `bitfield!` API. Bitfields are,
at the end of the day, just integers which we will very likely want to
declare constants of out of individual const field values. This can only
be done by using const methods, and a const `Bounded` needs to validate
its constraints at build time, which requires the
`Bounded::new::<VALUE>()` constructor, and by transition
similarly-shaped setters to set fields at const time.