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: <DDJZY40SO5EF.2066SEKKQ4U8I@kernel.org>
Date: Thu, 16 Oct 2025 21:34:20 +0200
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Joel Fernandes" <joelagnelf@...dia.com>
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 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;
	    }
	}

- Danilo