| 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
| ||
|
Message-ID: <20250908212451.GA3923672@joelbox2>
Date: Mon, 8 Sep 2025 17:24:51 -0400
From: Joel Fernandes <joelagnelf@...dia.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc: linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
dakr@...nel.org, 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, 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, Elle Rhumsaa <elle@...thered-steel.dev>,
Daniel Almeida <daniel.almeida@...labora.com>,
nouveau@...ts.freedesktop.org, rust-for-linux@...r.kernel.org
Subject: Re: [PATCH v2 4/4] rust: Move register and bitstruct macros out of
Nova
On Mon, Sep 08, 2025 at 08:39:19PM +0200, Miguel Ojeda wrote:
> On Mon, Sep 8, 2025 at 7:06 PM Joel Fernandes <joelagnelf@...dia.com> wrote:
> >
> > The issue I ran into is, without adding it to prelude, the users of register!
> > macro will have to import both bitfield! and register! macros explictly, even
> > though they're only using register!. I tried to make it work without adding to
> > prelude, but couldn't:
> >
> > use kernel::{bitfield, register};
> >
> > Also not adding it to prelude, means register! macro has to invoke bitfield with
> > $crate prefixed ($crate::bitfield).
>
> I am not sure I follow -- macros should use qualified paths in general
> so that they assume as little as possible from the calling
> environment.
It works! Can you look if the below looks good to you now? No more prelude. Thanks!
---8<----
From: Joel Fernandes <joelagnelf@...dia.com>
Subject: [PATCH] rust: Move register and bitfield macros out of Nova
Out of broad need for these macros in Rust, move them out. Several folks
have shown interest (Nova, Tyr GPU drivers).
bitfield moved into bits modules - defines bitfields in Rust structs similar to C.
register moved into io module - defines hardware registers and accessors.
Signed-off-by: Joel Fernandes <joelagnelf@...dia.com>
---
drivers/gpu/nova-core/falcon.rs | 2 +-
drivers/gpu/nova-core/falcon/gsp.rs | 3 +-
drivers/gpu/nova-core/falcon/sec2.rs | 2 +-
drivers/gpu/nova-core/nova_core.rs | 3 --
drivers/gpu/nova-core/regs.rs | 6 +--
rust/kernel/bits.rs | 2 +
.../kernel/bits}/bitfield.rs | 27 ++++++-----
rust/kernel/io.rs | 1 +
.../macros.rs => rust/kernel/io/register.rs | 48 ++++++++++---------
9 files changed, 50 insertions(+), 44 deletions(-)
rename {drivers/gpu/nova-core => rust/kernel/bits}/bitfield.rs (90%)
rename drivers/gpu/nova-core/regs/macros.rs => rust/kernel/io/register.rs (93%)
diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index 938f25b556a8..c69bc2adb33c 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -7,6 +7,7 @@
use kernel::device;
use kernel::dma::DmaAddress;
use kernel::prelude::*;
+use kernel::io::register::RegisterBase;
use kernel::sync::aref::ARef;
use kernel::time::Delta;
@@ -14,7 +15,6 @@
use crate::driver::Bar0;
use crate::gpu::Chipset;
use crate::regs;
-use crate::regs::macros::RegisterBase;
use crate::util;
pub(crate) mod gsp;
diff --git a/drivers/gpu/nova-core/falcon/gsp.rs b/drivers/gpu/nova-core/falcon/gsp.rs
index c9ab375fd8a1..f315344b4256 100644
--- a/drivers/gpu/nova-core/falcon/gsp.rs
+++ b/drivers/gpu/nova-core/falcon/gsp.rs
@@ -1,12 +1,13 @@
// SPDX-License-Identifier: GPL-2.0
use kernel::prelude::*;
+use kernel::io::register::RegisterBase;
use kernel::time::Delta;
use crate::{
driver::Bar0,
falcon::{Falcon, FalconEngine, PFalcon2Base, PFalconBase},
- regs::{self, macros::RegisterBase},
+ regs,
util::wait_on,
};
diff --git a/drivers/gpu/nova-core/falcon/sec2.rs b/drivers/gpu/nova-core/falcon/sec2.rs
index 815786c8480d..81717868a8a8 100644
--- a/drivers/gpu/nova-core/falcon/sec2.rs
+++ b/drivers/gpu/nova-core/falcon/sec2.rs
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
use crate::falcon::{FalconEngine, PFalcon2Base, PFalconBase};
-use crate::regs::macros::RegisterBase;
+use kernel::io::register::RegisterBase;
/// Type specifying the `Sec2` falcon engine. Cannot be instantiated.
pub(crate) struct Sec2(());
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index eaba6ad22f7a..4dbc7e5daae3 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -2,9 +2,6 @@
//! Nova Core GPU Driver
-#[macro_use]
-mod bitfield;
-
mod dma;
mod driver;
mod falcon;
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index c214f8056d6e..07533eb6f64e 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -4,15 +4,13 @@
// but are mapped to types.
#![allow(non_camel_case_types)]
-#[macro_use]
-pub(crate) mod macros;
-
use crate::falcon::{
DmaTrfCmdSize, FalconCoreRev, FalconCoreRevSubversion, FalconFbifMemType, FalconFbifTarget,
FalconModSelAlgo, FalconSecurityModel, PFalcon2Base, PFalconBase, PeregrineCoreSelect,
};
use crate::gpu::{Architecture, Chipset};
use kernel::prelude::*;
+use kernel::register;
// PMC
@@ -352,6 +350,7 @@ pub(crate) fn mem_scrubbing_done(self) -> bool {
pub(crate) mod gm107 {
// FUSE
+ use kernel::register;
register!(NV_FUSE_STATUS_OPT_DISPLAY @ 0x00021c04 {
0:0 display_disabled as bool;
@@ -360,6 +359,7 @@ pub(crate) mod gm107 {
pub(crate) mod ga100 {
// FUSE
+ use kernel::register;
register!(NV_FUSE_STATUS_OPT_DISPLAY @ 0x00820c04 {
0:0 display_disabled as bool;
diff --git a/rust/kernel/bits.rs b/rust/kernel/bits.rs
index 553d50265883..590a77d99ad7 100644
--- a/rust/kernel/bits.rs
+++ b/rust/kernel/bits.rs
@@ -201,3 +201,5 @@ pub const fn [<genmask_ $ty>](range: RangeInclusive<u32>) -> $ty {
/// assert_eq!(genmask_u8(0..=7), u8::MAX);
/// ```
);
+
+pub mod bitfield;
diff --git a/drivers/gpu/nova-core/bitfield.rs b/rust/kernel/bits/bitfield.rs
similarity index 90%
rename from drivers/gpu/nova-core/bitfield.rs
rename to rust/kernel/bits/bitfield.rs
index 2ca5f3725246..b49bfc1abcd5 100644
--- a/drivers/gpu/nova-core/bitfield.rs
+++ b/rust/kernel/bits/bitfield.rs
@@ -33,10 +33,13 @@
//! - `as <type> ?=> <try_into_type>` calls `<try_into_type>`'s `TryFrom::<<type>>` implementation
//! and returns the result. This is useful with fields for which not all values are valid.
//!
+
+/// bitfield macro definition
+#[macro_export]
macro_rules! bitfield {
// Main entry point - defines the bitfield struct with fields
($vis:vis struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
- bitfield!(@core $name $vis $storage $(, $comment)? { $($fields)* });
+ ::kernel::bitfield!(@core $name $vis $storage $(, $comment)? { $($fields)* });
};
// All rules below are helpers.
@@ -71,7 +74,7 @@ fn from(val: $storage) -> Self {
}
}
- bitfield!(@fields_dispatcher $name $vis $storage { $($fields)* });
+ ::kernel::bitfield!(@fields_dispatcher $name $vis $storage { $($fields)* });
};
// Captures the fields and passes them to all the implementers that require field information.
@@ -87,7 +90,7 @@ fn from(val: $storage) -> Self {
)*
}
) => {
- bitfield!(@field_accessors $name $vis $storage {
+ ::kernel::bitfield!(@field_accessors $name $vis $storage {
$(
$hi:$lo $field as $type
$(?=> $try_into_type)?
@@ -96,8 +99,8 @@ fn from(val: $storage) -> Self {
;
)*
});
- bitfield!(@debug $name $storage { $($field;)* });
- bitfield!(@default $name $storage { $($field;)* });
+ ::kernel::bitfield!(@debug $name $storage { $($field;)* });
+ ::kernel::bitfield!(@default $name $storage { $($field;)* });
};
// Defines all the field getter/setter methods for `$name`.
@@ -112,7 +115,7 @@ fn from(val: $storage) -> Self {
}
) => {
$(
- bitfield!(@check_field_bounds $hi:$lo $field as $type);
+ ::kernel::bitfield!(@check_field_bounds $hi:$lo $field as $type);
)*
#[allow(dead_code)]
@@ -124,7 +127,7 @@ impl $name {
}
$(
- bitfield!(@field_accessor $name $vis $storage, $hi:$lo $field as $type
+ ::kernel::bitfield!(@field_accessor $name $vis $storage, $hi:$lo $field as $type
$(?=> $try_into_type)?
$(=> $into_type)?
$(, $comment)?
@@ -161,7 +164,7 @@ impl $name {
@field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as bool => $into_type:ty
$(, $comment:literal)?;
) => {
- bitfield!(
+ ::kernel::bitfield!(
@leaf_accessor $name $vis $storage, $hi:$lo $field
{ |f| <$into_type>::from(if f != 0 { true } else { false }) }
$into_type => $into_type $(, $comment)?;
@@ -172,7 +175,7 @@ impl $name {
(
@field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as bool $(, $comment:literal)?;
) => {
- bitfield!(@field_accessor $name $vis $storage, $hi:$lo $field as bool => bool $(, $comment)?;);
+ ::kernel::bitfield!(@field_accessor $name $vis $storage, $hi:$lo $field as bool => bool $(, $comment)?;);
};
// Catches the `?=>` syntax for non-boolean fields.
@@ -180,7 +183,7 @@ impl $name {
@field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt ?=> $try_into_type:ty
$(, $comment:literal)?;
) => {
- bitfield!(@leaf_accessor $name $vis $storage, $hi:$lo $field
+ ::kernel::bitfield!(@leaf_accessor $name $vis $storage, $hi:$lo $field
{ |f| <$try_into_type>::try_from(f as $type) } $try_into_type =>
::core::result::Result<
$try_into_type,
@@ -194,7 +197,7 @@ impl $name {
@field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt => $into_type:ty
$(, $comment:literal)?;
) => {
- bitfield!(@leaf_accessor $name $vis $storage, $hi:$lo $field
+ ::kernel::bitfield!(@leaf_accessor $name $vis $storage, $hi:$lo $field
{ |f| <$into_type>::from(f as $type) } $into_type => $into_type $(, $comment)?;);
};
@@ -203,7 +206,7 @@ impl $name {
@field_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
$(, $comment:literal)?;
) => {
- bitfield!(@field_accessor $name $vis $storage, $hi:$lo $field as $type => $type $(, $comment)?;);
+ ::kernel::bitfield!(@field_accessor $name $vis $storage, $hi:$lo $field as $type => $type $(, $comment)?;);
};
// Generates the accessor methods for a single field.
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 03b467722b86..a79b603604b1 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -8,6 +8,7 @@
use crate::{bindings, build_assert, ffi::c_void};
pub mod mem;
+pub mod register;
pub mod resource;
pub use resource::Resource;
diff --git a/drivers/gpu/nova-core/regs/macros.rs b/rust/kernel/io/register.rs
similarity index 93%
rename from drivers/gpu/nova-core/regs/macros.rs
rename to rust/kernel/io/register.rs
index fab407c50668..a212b12de0f1 100644
--- a/drivers/gpu/nova-core/regs/macros.rs
+++ b/rust/kernel/io/register.rs
@@ -16,7 +16,8 @@
/// The `T` generic argument is used to distinguish which base to use, in case a type provides
/// several bases. It is given to the `register!` macro to restrict the use of the register to
/// implementors of this particular variant.
-pub(crate) trait RegisterBase<T> {
+pub trait RegisterBase<T> {
+ /// The base address for the register.
const BASE: usize;
}
@@ -55,7 +56,7 @@ pub(crate) trait RegisterBase<T> {
/// ```
///
///
-/// Please look at the [`bitfield`] macro for the complete syntax of fields definitions.
+/// Please look at the [`bitfield`][crate::bits::bitfield::bitfield] macro for the complete syntax of fields definitions.
///
/// The documentation strings are optional. If present, they will be added to the type's
/// definition, or the field getter and setter methods they are attached to.
@@ -275,28 +276,29 @@ pub(crate) trait RegisterBase<T> {
/// # Ok(())
/// # }
/// ```
+#[macro_export]
macro_rules! register {
// Creates a register at a fixed offset of the MMIO space.
($name:ident @ $offset:literal $(, $comment:literal)? { $($fields:tt)* } ) => {
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $offset);
};
// Creates an alias register of fixed offset register `alias` with its own fields.
($name:ident => $alias:ident $(, $comment:literal)? { $($fields:tt)* } ) => {
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $alias::OFFSET);
};
// Creates a register at a relative offset from a base address provider.
($name:ident @ $base:ty [ $offset:literal ] $(, $comment:literal)? { $($fields:tt)* } ) => {
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $offset ]);
};
// Creates an alias register of relative offset register `alias` with its own fields.
($name:ident => $base:ty [ $alias:ident ] $(, $comment:literal)? { $($fields:tt)* }) => {
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $alias::OFFSET ]);
};
@@ -307,7 +309,7 @@ macro_rules! register {
}
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_array $name @ $offset [ $size ; $stride ]);
};
@@ -328,7 +330,7 @@ macro_rules! register {
$(, $comment:literal)? { $($fields:tt)* }
) => {
static_assert!(::core::mem::size_of::<u32>() <= $stride);
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative_array $name @ $base [ $offset [ $size ; $stride ] ]);
};
@@ -350,7 +352,7 @@ macro_rules! register {
}
) => {
static_assert!($idx < $alias::SIZE);
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_relative $name @ $base [ $alias::OFFSET + $idx * $alias::STRIDE ] );
};
@@ -359,7 +361,7 @@ macro_rules! register {
// to avoid it being interpreted in place of the relative register array alias rule.
($name:ident => $alias:ident [ $idx:expr ] $(, $comment:literal)? { $($fields:tt)* }) => {
static_assert!($idx < $alias::SIZE);
- bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
+ ::kernel::bitfield!(pub(crate) struct $name: u32 $(, $comment)? { $($fields)* } );
register!(@io_fixed $name @ $alias::OFFSET + $idx * $alias::STRIDE );
};
@@ -416,12 +418,12 @@ pub(crate) fn read<const SIZE: usize, T, B>(
base: &B,
) -> Self where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
{
const OFFSET: usize = $name::OFFSET;
let value = io.read32(
- <B as crate::regs::macros::RegisterBase<$base>>::BASE + OFFSET
+ <B as ::kernel::io::register::RegisterBase<$base>>::BASE + OFFSET
);
Self(value)
@@ -437,13 +439,13 @@ pub(crate) fn write<const SIZE: usize, T, B>(
base: &B,
) where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
{
const OFFSET: usize = $name::OFFSET;
io.write32(
self.0,
- <B as crate::regs::macros::RegisterBase<$base>>::BASE + OFFSET
+ <B as ::kernel::io::register::RegisterBase<$base>>::BASE + OFFSET
);
}
@@ -457,7 +459,7 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>(
f: F,
) where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
F: ::core::ops::FnOnce(Self) -> Self,
{
let reg = f(Self::read(io, base));
@@ -602,11 +604,11 @@ pub(crate) fn read<const SIZE: usize, T, B>(
idx: usize,
) -> Self where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
{
build_assert!(idx < Self::SIZE);
- let offset = <B as crate::regs::macros::RegisterBase<$base>>::BASE +
+ let offset = <B as ::kernel::io::register::RegisterBase<$base>>::BASE +
Self::OFFSET + (idx * Self::STRIDE);
let value = io.read32(offset);
@@ -624,11 +626,11 @@ pub(crate) fn write<const SIZE: usize, T, B>(
idx: usize
) where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
{
build_assert!(idx < Self::SIZE);
- let offset = <B as crate::regs::macros::RegisterBase<$base>>::BASE +
+ let offset = <B as ::kernel::io::register::RegisterBase<$base>>::BASE +
Self::OFFSET + (idx * Self::STRIDE);
io.write32(self.0, offset);
@@ -645,7 +647,7 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>(
f: F,
) where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
F: ::core::ops::FnOnce(Self) -> Self,
{
let reg = f(Self::read(io, base, idx));
@@ -664,7 +666,7 @@ pub(crate) fn try_read<const SIZE: usize, T, B>(
idx: usize,
) -> ::kernel::error::Result<Self> where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
{
if idx < Self::SIZE {
Ok(Self::read(io, base, idx))
@@ -686,7 +688,7 @@ pub(crate) fn try_write<const SIZE: usize, T, B>(
idx: usize,
) -> ::kernel::error::Result where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
{
if idx < Self::SIZE {
Ok(self.write(io, base, idx))
@@ -709,7 +711,7 @@ pub(crate) fn try_alter<const SIZE: usize, T, B, F>(
f: F,
) -> ::kernel::error::Result where
T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ B: ::kernel::io::register::RegisterBase<$base>,
F: ::core::ops::FnOnce(Self) -> Self,
{
if idx < Self::SIZE {
--
2.34.1
Powered by blists - more mailing lists