linux-kernel - Re: [PATCH] rust/drm: tyr: Convert to the 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: <E9B69FEC-0A99-4F2C-AA59-59B8C34B4E4E@collabora.com>
Date: Fri, 16 Jan 2026 10:59:48 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Dirk Behme <Dirk.Behme@...bosch.com>
Cc: Dirk Behme <dirk.behme@...il.com>,
 Danilo Krummrich <dakr@...nel.org>,
 Alice Ryhl <aliceryhl@...gle.com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>,
 Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>,
 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>,
 Steven Price <steven.price@....com>,
 dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org,
 rust-for-linux@...r.kernel.org
Subject: Re: [PATCH] rust/drm: tyr: Convert to the register!() macro


>>> 
>>> Is there any reason why you replace the UPPERCASE register names with
>>> CamelCase ones?
>>> 
>>> I was under the impression that we want to use UPPERCASE for register
>>> names. Like in nova
>>> 
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/nova-core/regs.rs
>> Not really. UPPERCASE for non-const items will trigger the linter. The Nova
>> people chose to #[allow] this to align with OpenRM and, IIRC from the LPC
>> discussions, their registers are automatically generated from some internal
>> docs.
>> We have only a few, we can simply convert them to CamelCase.
> 
> 
> I'm under the impression that we define the "future RFL register!() style standard" here.
> 
> So we want to make the CamelCase the default? And nova is the exception?

I’m not sure I would say this. It’s just that you would hit this lint
[0]. If UPPER_CASE was the “default", we would have to have the #[allow] on
every driver. 

> 
> I'm fine with that. Just want to make sure we talked about it :)
> 
> 
> ....
>>>> pub(crate) const MCU_CONTROL_ENABLE: u32 = 1;
>>>> pub(crate) const MCU_CONTROL_AUTO: u32 = 2;
>>>> pub(crate) const MCU_CONTROL_DISABLE: u32 = 0;
>>>> 
>>>> -pub(crate) const MCU_STATUS: Register<0x704> = Register;
>>>> +register!(McuControl @ 0x700, "Controls the execution state of the MCU subsystem" {
>>>> +    1:0     req as u32, "Request state change";
>>>> +});
>>> 
>>> 
>>> Any reason why req is a u32 and not a u8? Same for some other places.
>> I tend to default to u32/i32 in general, as that’s usually the native machine integer type.
>> All we get from smaller types is a spam of `into()`, `from()` and their `try_`
>> equivalents. When stored in a struct, they usually do not save space due to
>> padding that is usually inserted to fix the alignment for the type. IMHO not
>> worth it unless it really matters. Correct me if I'm wrong, but it doesn't seem
>> to be the case here.
> 
> 
> Wouldn't using u8 prevent any accidental access to 31:8 ?

The macro is doing the appropriate masking according to the bit ranges you pass
in (i.e.: 31:8), not according to the type (u8 or u32), i.e.:

        const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi; <------
        const [<$field:upper _MASK>]: $storage = {
            // Generate mask for shifting
            match ::core::mem::size_of::<$storage>() {
                1 => ::kernel::bits::genmask_u8($lo..=$hi) as $storage,
                2 => ::kernel::bits::genmask_u16($lo..=$hi) as $storage,
                4 => ::kernel::bits::genmask_u32($lo..=$hi) as $storage,
                8 => ::kernel::bits::genmask_u64($lo..=$hi) as $storage,
                _ => ::kernel::build_error!("Unsupported storage type size")
            }
        };
        const [<$field:upper _SHIFT>]: u32 = $lo;
        );

        $(
        #[doc="Returns the value of this field:"]
        #[doc=$comment]
        )?
        #[inline(always)]
        $vis fn $field(self) -> $res_type {
            ::kernel::macros::paste!(
            const MASK: $storage = $name::[<$field:upper _MASK>];
            const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
            );
            let field = ((self.0 & MASK) >> SHIFT);

            $process(field)
        }


> 
> 
> Best regards
> 
> Dirk


[0]: https://doc.rust-lang.org/stable/nightly-rustc/rustc_lint/nonstandard_style/static.NON_CAMEL_CASE_TYPES.html