linux-kernel - Re: [PATCH v7.1 2/4] gpu: nova-core: bitfield: Move bitfield-specific code from register! into new 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: <EA9E9DCA-2223-4C74-810F-1CEDCFDA4088@nvidia.com>
Date: Thu, 16 Oct 2025 19:49:43 +0000
From: Joel Fernandes <joelagnelf@...dia.com>
To: Danilo Krummrich <dakr@...nel.org>
CC: Yury Norov <yury.norov@...il.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "rust-for-linux@...r.kernel.org"
	<rust-for-linux@...r.kernel.org>, "dri-devel@...ts.freedesktop.org"
	<dri-devel@...ts.freedesktop.org>, Alexandre Courbot <acourbot@...dia.com>,
	Alistair Popple <apopple@...dia.com>, Miguel Ojeda <ojeda@...nel.org>, Alex
 Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>, Gary Guo
	<gary@...yguo.net>, "bjorn3_gh@...tonmail.com" <bjorn3_gh@...tonmail.com>,
	Benno Lossin <lossin@...nel.org>, Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, David
 Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Maarten
 Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard
	<mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, John Hubbard
	<jhubbard@...dia.com>, Timur Tabi <ttabi@...dia.com>,
	"joel@...lfernandes.org" <joel@...lfernandes.org>, Elle Rhumsaa
	<elle@...thered-steel.dev>, Daniel Almeida <daniel.almeida@...labora.com>,
	"nouveau@...ts.freedesktop.org" <nouveau@...ts.freedesktop.org>, Edwin Peer
	<epeer@...dia.com>
Subject: Re: [PATCH v7.1 2/4] gpu: nova-core: bitfield: Move bitfield-specific
 code from register! into new macro



> On Oct 16, 2025, at 3:34 PM, Danilo Krummrich <dakr@...nel.org> wrote:
> 
> On Thu Oct 16, 2025 at 9:28 PM CEST, Joel Fernandes wrote:
>>>> On Oct 16, 2025, at 1:48 PM, Yury Norov <yury.norov@...il.com> wrote:
>>> On Thu, Oct 16, 2025 at 11:13:21AM -0400, Joel Fernandes wrote:
>>>> +///
>>>> +/// bitfield! {
>>>> +///     struct ControlReg {
>>>> +///         3:0 mode as u8 ?=> Mode;
>>>> +///         7:7 state as bool => State;
>>>> +///     }
>>>> +/// }
>>> 
>>> This notation is really unwelcome this days. It may be OK for a random
>>> macro in some local driver, but doesn't really work for a global basic
>>> data type:
>>> 
>>> https://lore.kernel.org/all/CAHk-=whoOUsqPKb7OQwhQf9H_3=5sXGPJrDbfQfwLB3Bi13tcQ@mail.gmail.com/
>>> 
>>> I've already shared this link with you, and shared my concern.
>>> 
>>> I realize that rust/bitfield derives the GENMASK(hi, lo) notation here,
>>> and GENMASK() derives verilog or hardware specs popular notations. But
>>> software people prefer lo:hi. I'm probably OK if you choose C-style
>>> start:nbits, if you prefer. But let's stop this hi:lo early, please.
>>> 
>>> Let me quote Linus from the link above:
>>> 
>>> It does "high, low", which is often very unintuitive, and in fact the
>>> very commit that introduced this thing from hell had to convert the
>>> sane "low,high" cases to the other way around.
>> 
>> I agree with Linus but I disagree with comparing it with these macros.
>> I agree with Linus it is oddly unreadable when used as function parameters.
>> But that is a different syntax. Over here we are using colons with sufficient whitespace around hi:lo.
> 
> I agree with Joel here.
> 
> While I'm not super opinionated for general bitfields, for the register!()
> infrastructure I very much prefer the hi:lo notation, as this is the common
> notation in datasheets and TRMs.
> 
> However, if we use hi:lo, we should use it decending, i.e.:
> 
>    bitfield! {
>        struct ControlReg {
>            7:5 state as u8 => State;
>            3:0 mode as u8 ?=> Mode;
>        }
>    }

Makes sense. The macro currently supports both. I will fix the example docs.

thanks,

 - Joel


> 
> - Danilo