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] [day] [month] [year] [list]

Message-ID: <77AAED55-B8AE-428D-AD3D-D52333DB8D8A@nvidia.com>
Date: Tue, 27 Jan 2026 22:47:09 -0500
From: Joel Fernandes <joelagnelf@...dia.com>
To: John Hubbard <jhubbard@...dia.com>
Cc: Alexandre Courbot <acourbot@...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 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.

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?

-- 
Joel Fernandes