Joel Fernandes
2025-Sep-03 21:54 UTC
[PATCH v2 4/4] rust: Move register and bitstruct 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).
bitstruct - defines bitfields in Rust structs similar to C.
register - support for defining hardware registers and accessors.
Signed-off-by: Joel Fernandes <joelagnelf at nvidia.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 | 5 +-
.../nova-core => rust/kernel}/bitstruct.rs | 31 ++++---
rust/kernel/lib.rs | 2 +
rust/kernel/prelude.rs | 2 +
.../regs/macros.rs => rust/kernel/register.rs | 92 ++++++++++---------
9 files changed, 74 insertions(+), 68 deletions(-)
rename {drivers/gpu/nova-core => rust/kernel}/bitstruct.rs (92%)
rename drivers/gpu/nova-core/regs/macros.rs => rust/kernel/register.rs (90%)
diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs
index be91aac6976a..06da6ce24482 100644
--- a/drivers/gpu/nova-core/falcon.rs
+++ b/drivers/gpu/nova-core/falcon.rs
@@ -7,6 +7,7 @@
use kernel::bindings;
use kernel::device;
use kernel::prelude::*;
+use kernel::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 f17599cb49fa..9287ab148da8 100644
--- a/drivers/gpu/nova-core/falcon/gsp.rs
+++ b/drivers/gpu/nova-core/falcon/gsp.rs
@@ -3,8 +3,9 @@
use crate::{
driver::Bar0,
falcon::{Falcon, FalconEngine, PFalcon2Base, PFalconBase},
- regs::{self, macros::RegisterBase},
+ regs,
};
+use kernel::register::RegisterBase;
/// Type specifying the `Gsp` falcon engine. Cannot be instantiated.
pub(crate) struct Gsp(());
diff --git a/drivers/gpu/nova-core/falcon/sec2.rs
b/drivers/gpu/nova-core/falcon/sec2.rs
index 815786c8480d..8f7b63b6c2b2 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::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 b218a2d42573..cb2bbb30cba1 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 bitstruct;
-
mod dma;
mod driver;
mod falcon;
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 206dab2e1335..6d2f20623259 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -4,9 +4,6 @@
// 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,
@@ -331,6 +328,7 @@ pub(crate) fn mem_scrubbing_done(self) -> bool {
pub(crate) mod gm107 {
// FUSE
+ use kernel::prelude::*;
register!(NV_FUSE_STATUS_OPT_DISPLAY @ 0x00021c04 {
0:0 display_disabled as bool;
@@ -339,6 +337,7 @@ pub(crate) mod gm107 {
pub(crate) mod ga100 {
// FUSE
+ use kernel::prelude::*;
register!(NV_FUSE_STATUS_OPT_DISPLAY @ 0x00820c04 {
0:0 display_disabled as bool;
diff --git a/drivers/gpu/nova-core/bitstruct.rs b/rust/kernel/bitstruct.rs
similarity index 92%
rename from drivers/gpu/nova-core/bitstruct.rs
rename to rust/kernel/bitstruct.rs
index 1047c5c17e2d..06e5435df383 100644
--- a/drivers/gpu/nova-core/bitstruct.rs
+++ b/rust/kernel/bitstruct.rs
@@ -1,9 +1,9 @@
// SPDX-License-Identifier: GPL-2.0
-//
-// bitstruct.rs ? Bitfield library for Rust structures
-//
-// A library that provides support for defining bit fields in Rust
-// structures. Also used from things that need bitfields like register macro.
+
+//! Bitfield library for Rust structures
+//!
+//! A library that provides support for defining bit fields in Rust
+//! structures. Also used from things that need bitfields like register macro.
///
/// # Syntax
///
@@ -32,6 +32,7 @@
/// the result.
/// - `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.
+#[macro_export]
macro_rules! bitstruct {
// Main entry point - defines the bitfield struct with fields
($vis:vis struct $name:ident : $storage:ty $(, $comment:literal)? {
$($fields:tt)* }) => {
@@ -125,7 +126,7 @@ impl $name {
(@check_field_bounds $hi:tt:$lo:tt $field:ident as bool) => {
#[allow(clippy::eq_op)]
const _: () = {
- ::kernel::build_assert!(
+ build_assert!(
$hi == $lo,
concat!("boolean field `", stringify!($field),
"` covers more than one bit")
);
@@ -136,7 +137,7 @@ impl $name {
(@check_field_bounds $hi:tt:$lo:tt $field:ident as $type:tt) => {
#[allow(clippy::eq_op)]
const _: () = {
- ::kernel::build_assert!(
+ build_assert!(
$hi >= $lo,
concat!("field `", stringify!($field), "`'s
MSB is smaller than its LSB")
);
@@ -198,15 +199,15 @@ impl $name {
@leaf_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt
$field:ident
{ $process:expr } $to_type:ty => $res_type:ty $(,
$comment:literal)?;
) => {
- ::kernel::macros::paste!(
+ $crate::macros::paste!(
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,
+ 1 => $crate::bits::genmask_u8($lo..=$hi) as $storage,
+ 2 => $crate::bits::genmask_u16($lo..=$hi) as $storage,
+ 4 => $crate::bits::genmask_u32($lo..=$hi) as $storage,
+ 8 => $crate::bits::genmask_u64($lo..=$hi) as $storage,
_ => <$storage>::MAX
}
};
@@ -219,7 +220,7 @@ impl $name {
)?
#[inline(always)]
$vis fn $field(self) -> $res_type {
- ::kernel::macros::paste!(
+ $crate::macros::paste!(
const MASK: $storage = $name::[<$field:upper _MASK>];
const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
);
@@ -228,7 +229,7 @@ impl $name {
$process(field)
}
- ::kernel::macros::paste!(
+ $crate::macros::paste!(
$(
#[doc="Sets the value of this field:"]
#[doc=$comment]
@@ -267,7 +268,7 @@ fn default() -> Self {
#[allow(unused_mut)]
let mut value = Self(Default::default());
- ::kernel::macros::paste!(
+ $crate::macros::paste!(
$(
value.[<set_ $field>](Default::default());
)*
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index c859a8984bae..9c492fa10967 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -64,6 +64,7 @@
#[cfg(CONFIG_AUXILIARY_BUS)]
pub mod auxiliary;
pub mod bits;
+pub mod bitstruct;
#[cfg(CONFIG_BLOCK)]
pub mod block;
pub mod bug;
@@ -112,6 +113,7 @@
pub mod prelude;
pub mod print;
pub mod rbtree;
+pub mod register;
pub mod regulator;
pub mod revocable;
pub mod security;
diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
index 25fe97aafd02..a98c7b7ab6af 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -39,6 +39,8 @@
pub use super::static_assert;
+pub use super::{bitstruct, register};
+
pub use super::error::{code::*, Error, Result};
pub use super::{str::CStr, ThisModule};
diff --git a/drivers/gpu/nova-core/regs/macros.rs b/rust/kernel/register.rs
similarity index 90%
rename from drivers/gpu/nova-core/regs/macros.rs
rename to rust/kernel/register.rs
index 22a53a73b765..1f48c5335e70 100644
--- a/drivers/gpu/nova-core/regs/macros.rs
+++ b/rust/kernel/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;
}
@@ -281,6 +282,7 @@ 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)* } )
=> {
@@ -378,7 +380,7 @@ impl $name {
/// Read the register from its address in `io`.
#[inline(always)]
pub(crate) fn read<const SIZE: usize, T>(io: &T) ->
Self where
- T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target =
$crate::io::Io<SIZE>>,
{
Self(io.read32($offset))
}
@@ -386,7 +388,7 @@ pub(crate) fn read<const SIZE: usize, T>(io: &T)
-> Self where
/// Write the value contained in `self` to the register address in
`io`.
#[inline(always)]
pub(crate) fn write<const SIZE: usize, T>(self, io: &T)
where
- T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target =
$crate::io::Io<SIZE>>,
{
io.write32(self.0, $offset)
}
@@ -398,7 +400,7 @@ pub(crate) fn alter<const SIZE: usize, T, F>(
io: &T,
f: F,
) where
- T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target =
$crate::io::Io<SIZE>>,
F: ::core::ops::FnOnce(Self) -> Self,
{
let reg = f(Self::read(io));
@@ -421,13 +423,13 @@ pub(crate) fn read<const SIZE: usize, T, B>(
#[allow(unused_variables)]
base: &B,
) -> Self where
- T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ T: ::core::ops::Deref<Target =
$crate::io::Io<SIZE>>,
+ B: $crate::register::RegisterBase<$base>,
{
const OFFSET: usize = $name::OFFSET;
let value = io.read32(
- <B as
crate::regs::macros::RegisterBase<$base>>::BASE + OFFSET
+ <B as
$crate::register::RegisterBase<$base>>::BASE + OFFSET
);
Self(value)
@@ -442,14 +444,14 @@ pub(crate) fn write<const SIZE: usize, T, B>(
#[allow(unused_variables)]
base: &B,
) where
- T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ T: ::core::ops::Deref<Target =
$crate::io::Io<SIZE>>,
+ B: $crate::register::RegisterBase<$base>,
{
const OFFSET: usize = $name::OFFSET;
io.write32(
self.0,
- <B as
crate::regs::macros::RegisterBase<$base>>::BASE + OFFSET
+ <B as
$crate::register::RegisterBase<$base>>::BASE + OFFSET
);
}
@@ -462,8 +464,8 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>(
base: &B,
f: F,
) where
- T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ T: ::core::ops::Deref<Target =
$crate::io::Io<SIZE>>,
+ B: $crate::register::RegisterBase<$base>,
F: ::core::ops::FnOnce(Self) -> Self,
{
let reg = f(Self::read(io, base));
@@ -486,7 +488,7 @@ pub(crate) fn read<const SIZE: usize, T>(
io: &T,
idx: usize,
) -> Self where
- T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target =
$crate::io::Io<SIZE>>,
{
build_assert!(idx < Self::SIZE);
@@ -503,7 +505,7 @@ pub(crate) fn write<const SIZE: usize, T>(
io: &T,
idx: usize
) where
- T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target =
$crate::io::Io<SIZE>>,
{
build_assert!(idx < Self::SIZE);
@@ -520,7 +522,7 @@ pub(crate) fn alter<const SIZE: usize, T, F>(
idx: usize,
f: F,
) where
- T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target =
$crate::io::Io<SIZE>>,
F: ::core::ops::FnOnce(Self) -> Self,
{
let reg = f(Self::read(io, idx));
@@ -535,13 +537,13 @@ pub(crate) fn alter<const SIZE: usize, T, F>(
pub(crate) fn try_read<const SIZE: usize, T>(
io: &T,
idx: usize,
- ) -> ::kernel::error::Result<Self> where
- T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
+ ) -> $crate::error::Result<Self> where
+ T: ::core::ops::Deref<Target =
$crate::io::Io<SIZE>>,
{
if idx < Self::SIZE {
Ok(Self::read(io, idx))
} else {
- Err(EINVAL)
+ Err($crate::error::code::EINVAL)
}
}
@@ -554,13 +556,13 @@ pub(crate) fn try_write<const SIZE: usize, T>(
self,
io: &T,
idx: usize,
- ) -> ::kernel::error::Result where
- T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
+ ) -> $crate::error::Result where
+ T: ::core::ops::Deref<Target =
$crate::io::Io<SIZE>>,
{
if idx < Self::SIZE {
Ok(self.write(io, idx))
} else {
- Err(EINVAL)
+ Err($crate::error::code::EINVAL)
}
}
@@ -574,14 +576,14 @@ pub(crate) fn try_alter<const SIZE: usize, T, F>(
io: &T,
idx: usize,
f: F,
- ) -> ::kernel::error::Result where
- T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
+ ) -> $crate::error::Result where
+ T: ::core::ops::Deref<Target =
$crate::io::Io<SIZE>>,
F: ::core::ops::FnOnce(Self) -> Self,
{
if idx < Self::SIZE {
Ok(Self::alter(io, idx, f))
} else {
- Err(EINVAL)
+ Err($crate::error::code::EINVAL)
}
}
}
@@ -607,12 +609,12 @@ pub(crate) fn read<const SIZE: usize, T, B>(
base: &B,
idx: usize,
) -> Self where
- T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ T: ::core::ops::Deref<Target =
$crate::io::Io<SIZE>>,
+ B: $crate::register::RegisterBase<$base>,
{
build_assert!(idx < Self::SIZE);
- let offset = <B as
crate::regs::macros::RegisterBase<$base>>::BASE +
+ let offset = <B as
$crate::register::RegisterBase<$base>>::BASE +
Self::OFFSET + (idx * Self::STRIDE);
let value = io.read32(offset);
@@ -629,12 +631,12 @@ pub(crate) fn write<const SIZE: usize, T, B>(
base: &B,
idx: usize
) where
- T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ T: ::core::ops::Deref<Target =
$crate::io::Io<SIZE>>,
+ B: $crate::register::RegisterBase<$base>,
{
build_assert!(idx < Self::SIZE);
- let offset = <B as
crate::regs::macros::RegisterBase<$base>>::BASE +
+ let offset = <B as
$crate::register::RegisterBase<$base>>::BASE +
Self::OFFSET + (idx * Self::STRIDE);
io.write32(self.0, offset);
@@ -650,8 +652,8 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>(
idx: usize,
f: F,
) where
- T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ T: ::core::ops::Deref<Target =
$crate::io::Io<SIZE>>,
+ B: $crate::register::RegisterBase<$base>,
F: ::core::ops::FnOnce(Self) -> Self,
{
let reg = f(Self::read(io, base, idx));
@@ -668,14 +670,14 @@ pub(crate) fn try_read<const SIZE: usize, T, B>(
io: &T,
base: &B,
idx: usize,
- ) -> ::kernel::error::Result<Self> where
- T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ ) -> $crate::error::Result<Self> where
+ T: ::core::ops::Deref<Target =
$crate::io::Io<SIZE>>,
+ B: $crate::register::RegisterBase<$base>,
{
if idx < Self::SIZE {
Ok(Self::read(io, base, idx))
} else {
- Err(EINVAL)
+ Err($crate::error::code::EINVAL)
}
}
@@ -690,14 +692,14 @@ pub(crate) fn try_write<const SIZE: usize, T, B>(
io: &T,
base: &B,
idx: usize,
- ) -> ::kernel::error::Result where
- T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ ) -> $crate::error::Result where
+ T: ::core::ops::Deref<Target =
$crate::io::Io<SIZE>>,
+ B: $crate::register::RegisterBase<$base>,
{
if idx < Self::SIZE {
Ok(self.write(io, base, idx))
} else {
- Err(EINVAL)
+ Err($crate::error::code::EINVAL)
}
}
@@ -713,17 +715,19 @@ pub(crate) fn try_alter<const SIZE: usize, T, B, F>(
base: &B,
idx: usize,
f: F,
- ) -> ::kernel::error::Result where
- T: ::core::ops::Deref<Target =
::kernel::io::Io<SIZE>>,
- B: crate::regs::macros::RegisterBase<$base>,
+ ) -> $crate::error::Result where
+ T: ::core::ops::Deref<Target =
$crate::io::Io<SIZE>>,
+ B: $crate::register::RegisterBase<$base>,
F: ::core::ops::FnOnce(Self) -> Self,
{
if idx < Self::SIZE {
Ok(Self::alter(io, base, idx, f))
} else {
- Err(EINVAL)
+ Err($crate::error::code::EINVAL)
}
}
}
};
}
+
+pub use register;
--
2.34.1
Elle Rhumsaa
2025-Sep-05 22:25 UTC
[PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova
On Wed, Sep 03, 2025 at 05:54:28PM -0400, Joel Fernandes wrote:> Out of broad need for these macros in Rust, move them out. Several folks > have shown interest (Nova, Tyr GPU drivers). > > bitstruct - defines bitfields in Rust structs similar to C. > register - support for defining hardware registers and accessors. > > Signed-off-by: Joel Fernandes <joelagnelf at nvidia.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 | 5 +- > .../nova-core => rust/kernel}/bitstruct.rs | 31 ++++--- > rust/kernel/lib.rs | 2 + > rust/kernel/prelude.rs | 2 + > .../regs/macros.rs => rust/kernel/register.rs | 92 ++++++++++--------- > 9 files changed, 74 insertions(+), 68 deletions(-) > rename {drivers/gpu/nova-core => rust/kernel}/bitstruct.rs (92%) > rename drivers/gpu/nova-core/regs/macros.rs => rust/kernel/register.rs (90%) > > diff --git a/drivers/gpu/nova-core/falcon.rs b/drivers/gpu/nova-core/falcon.rs > index be91aac6976a..06da6ce24482 100644 > --- a/drivers/gpu/nova-core/falcon.rs > +++ b/drivers/gpu/nova-core/falcon.rs > @@ -7,6 +7,7 @@ > use kernel::bindings; > use kernel::device; > use kernel::prelude::*; > +use kernel::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 f17599cb49fa..9287ab148da8 100644 > --- a/drivers/gpu/nova-core/falcon/gsp.rs > +++ b/drivers/gpu/nova-core/falcon/gsp.rs > @@ -3,8 +3,9 @@ > use crate::{ > driver::Bar0, > falcon::{Falcon, FalconEngine, PFalcon2Base, PFalconBase}, > - regs::{self, macros::RegisterBase}, > + regs, > }; > +use kernel::register::RegisterBase; > > /// Type specifying the `Gsp` falcon engine. Cannot be instantiated. > pub(crate) struct Gsp(()); > diff --git a/drivers/gpu/nova-core/falcon/sec2.rs b/drivers/gpu/nova-core/falcon/sec2.rs > index 815786c8480d..8f7b63b6c2b2 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::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 b218a2d42573..cb2bbb30cba1 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 bitstruct; > - > mod dma; > mod driver; > mod falcon; > diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs > index 206dab2e1335..6d2f20623259 100644 > --- a/drivers/gpu/nova-core/regs.rs > +++ b/drivers/gpu/nova-core/regs.rs > @@ -4,9 +4,6 @@ > // 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, > @@ -331,6 +328,7 @@ pub(crate) fn mem_scrubbing_done(self) -> bool { > > pub(crate) mod gm107 { > // FUSE > + use kernel::prelude::*; > > register!(NV_FUSE_STATUS_OPT_DISPLAY @ 0x00021c04 { > 0:0 display_disabled as bool; > @@ -339,6 +337,7 @@ pub(crate) mod gm107 { > > pub(crate) mod ga100 { > // FUSE > + use kernel::prelude::*; > > register!(NV_FUSE_STATUS_OPT_DISPLAY @ 0x00820c04 { > 0:0 display_disabled as bool; > diff --git a/drivers/gpu/nova-core/bitstruct.rs b/rust/kernel/bitstruct.rs > similarity index 92% > rename from drivers/gpu/nova-core/bitstruct.rs > rename to rust/kernel/bitstruct.rs > index 1047c5c17e2d..06e5435df383 100644 > --- a/drivers/gpu/nova-core/bitstruct.rs > +++ b/rust/kernel/bitstruct.rs > @@ -1,9 +1,9 @@ > // SPDX-License-Identifier: GPL-2.0 > -// > -// bitstruct.rs ? Bitfield library for Rust structures > -// > -// A library that provides support for defining bit fields in Rust > -// structures. Also used from things that need bitfields like register macro. > + > +//! Bitfield library for Rust structures > +//! > +//! A library that provides support for defining bit fields in Rust > +//! structures. Also used from things that need bitfields like register macro. > /// > /// # Syntax > /// > @@ -32,6 +32,7 @@ > /// the result. > /// - `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. > +#[macro_export] > macro_rules! bitstruct { > // Main entry point - defines the bitfield struct with fields > ($vis:vis struct $name:ident : $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => { > @@ -125,7 +126,7 @@ impl $name { > (@check_field_bounds $hi:tt:$lo:tt $field:ident as bool) => { > #[allow(clippy::eq_op)] > const _: () = { > - ::kernel::build_assert!( > + build_assert!( > $hi == $lo, > concat!("boolean field `", stringify!($field), "` covers more than one bit") > ); > @@ -136,7 +137,7 @@ impl $name { > (@check_field_bounds $hi:tt:$lo:tt $field:ident as $type:tt) => { > #[allow(clippy::eq_op)] > const _: () = { > - ::kernel::build_assert!( > + build_assert!( > $hi >= $lo, > concat!("field `", stringify!($field), "`'s MSB is smaller than its LSB") > ); > @@ -198,15 +199,15 @@ impl $name { > @leaf_accessor $name:ident $vis:vis $storage:ty, $hi:tt:$lo:tt $field:ident > { $process:expr } $to_type:ty => $res_type:ty $(, $comment:literal)?; > ) => { > - ::kernel::macros::paste!( > + $crate::macros::paste!( > 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, > + 1 => $crate::bits::genmask_u8($lo..=$hi) as $storage, > + 2 => $crate::bits::genmask_u16($lo..=$hi) as $storage, > + 4 => $crate::bits::genmask_u32($lo..=$hi) as $storage, > + 8 => $crate::bits::genmask_u64($lo..=$hi) as $storage, > _ => <$storage>::MAX > } > }; > @@ -219,7 +220,7 @@ impl $name { > )? > #[inline(always)] > $vis fn $field(self) -> $res_type { > - ::kernel::macros::paste!( > + $crate::macros::paste!( > const MASK: $storage = $name::[<$field:upper _MASK>]; > const SHIFT: u32 = $name::[<$field:upper _SHIFT>]; > ); > @@ -228,7 +229,7 @@ impl $name { > $process(field) > } > > - ::kernel::macros::paste!( > + $crate::macros::paste!( > $( > #[doc="Sets the value of this field:"] > #[doc=$comment] > @@ -267,7 +268,7 @@ fn default() -> Self { > #[allow(unused_mut)] > let mut value = Self(Default::default()); > > - ::kernel::macros::paste!( > + $crate::macros::paste!( > $( > value.[<set_ $field>](Default::default()); > )* > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > index c859a8984bae..9c492fa10967 100644 > --- a/rust/kernel/lib.rs > +++ b/rust/kernel/lib.rs > @@ -64,6 +64,7 @@ > #[cfg(CONFIG_AUXILIARY_BUS)] > pub mod auxiliary; > pub mod bits; > +pub mod bitstruct; > #[cfg(CONFIG_BLOCK)] > pub mod block; > pub mod bug; > @@ -112,6 +113,7 @@ > pub mod prelude; > pub mod print; > pub mod rbtree; > +pub mod register; > pub mod regulator; > pub mod revocable; > pub mod security; > diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs > index 25fe97aafd02..a98c7b7ab6af 100644 > --- a/rust/kernel/prelude.rs > +++ b/rust/kernel/prelude.rs > @@ -39,6 +39,8 @@ > > pub use super::static_assert; > > +pub use super::{bitstruct, register}; > + > pub use super::error::{code::*, Error, Result}; > > pub use super::{str::CStr, ThisModule}; > diff --git a/drivers/gpu/nova-core/regs/macros.rs b/rust/kernel/register.rs > similarity index 90% > rename from drivers/gpu/nova-core/regs/macros.rs > rename to rust/kernel/register.rs > index 22a53a73b765..1f48c5335e70 100644 > --- a/drivers/gpu/nova-core/regs/macros.rs > +++ b/rust/kernel/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; > } > > @@ -281,6 +282,7 @@ 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)* } ) => { > @@ -378,7 +380,7 @@ impl $name { > /// Read the register from its address in `io`. > #[inline(always)] > pub(crate) fn read<const SIZE: usize, T>(io: &T) -> Self where > - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, > + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>, > { > Self(io.read32($offset)) > } > @@ -386,7 +388,7 @@ pub(crate) fn read<const SIZE: usize, T>(io: &T) -> Self where > /// Write the value contained in `self` to the register address in `io`. > #[inline(always)] > pub(crate) fn write<const SIZE: usize, T>(self, io: &T) where > - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, > + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>, > { > io.write32(self.0, $offset) > } > @@ -398,7 +400,7 @@ pub(crate) fn alter<const SIZE: usize, T, F>( > io: &T, > f: F, > ) where > - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, > + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>, > F: ::core::ops::FnOnce(Self) -> Self, > { > let reg = f(Self::read(io)); > @@ -421,13 +423,13 @@ pub(crate) fn read<const SIZE: usize, T, B>( > #[allow(unused_variables)] > base: &B, > ) -> Self where > - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, > - B: crate::regs::macros::RegisterBase<$base>, > + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>, > + B: $crate::register::RegisterBase<$base>, > { > const OFFSET: usize = $name::OFFSET; > > let value = io.read32( > - <B as crate::regs::macros::RegisterBase<$base>>::BASE + OFFSET > + <B as $crate::register::RegisterBase<$base>>::BASE + OFFSET > ); > > Self(value) > @@ -442,14 +444,14 @@ pub(crate) fn write<const SIZE: usize, T, B>( > #[allow(unused_variables)] > base: &B, > ) where > - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, > - B: crate::regs::macros::RegisterBase<$base>, > + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>, > + B: $crate::register::RegisterBase<$base>, > { > const OFFSET: usize = $name::OFFSET; > > io.write32( > self.0, > - <B as crate::regs::macros::RegisterBase<$base>>::BASE + OFFSET > + <B as $crate::register::RegisterBase<$base>>::BASE + OFFSET > ); > } > > @@ -462,8 +464,8 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>( > base: &B, > f: F, > ) where > - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, > - B: crate::regs::macros::RegisterBase<$base>, > + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>, > + B: $crate::register::RegisterBase<$base>, > F: ::core::ops::FnOnce(Self) -> Self, > { > let reg = f(Self::read(io, base)); > @@ -486,7 +488,7 @@ pub(crate) fn read<const SIZE: usize, T>( > io: &T, > idx: usize, > ) -> Self where > - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, > + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>, > { > build_assert!(idx < Self::SIZE); > > @@ -503,7 +505,7 @@ pub(crate) fn write<const SIZE: usize, T>( > io: &T, > idx: usize > ) where > - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, > + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>, > { > build_assert!(idx < Self::SIZE); > > @@ -520,7 +522,7 @@ pub(crate) fn alter<const SIZE: usize, T, F>( > idx: usize, > f: F, > ) where > - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, > + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>, > F: ::core::ops::FnOnce(Self) -> Self, > { > let reg = f(Self::read(io, idx)); > @@ -535,13 +537,13 @@ pub(crate) fn alter<const SIZE: usize, T, F>( > pub(crate) fn try_read<const SIZE: usize, T>( > io: &T, > idx: usize, > - ) -> ::kernel::error::Result<Self> where > - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, > + ) -> $crate::error::Result<Self> where > + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>, > { > if idx < Self::SIZE { > Ok(Self::read(io, idx)) > } else { > - Err(EINVAL) > + Err($crate::error::code::EINVAL) > } > } > > @@ -554,13 +556,13 @@ pub(crate) fn try_write<const SIZE: usize, T>( > self, > io: &T, > idx: usize, > - ) -> ::kernel::error::Result where > - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, > + ) -> $crate::error::Result where > + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>, > { > if idx < Self::SIZE { > Ok(self.write(io, idx)) > } else { > - Err(EINVAL) > + Err($crate::error::code::EINVAL) > } > } > > @@ -574,14 +576,14 @@ pub(crate) fn try_alter<const SIZE: usize, T, F>( > io: &T, > idx: usize, > f: F, > - ) -> ::kernel::error::Result where > - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, > + ) -> $crate::error::Result where > + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>, > F: ::core::ops::FnOnce(Self) -> Self, > { > if idx < Self::SIZE { > Ok(Self::alter(io, idx, f)) > } else { > - Err(EINVAL) > + Err($crate::error::code::EINVAL) > } > } > } > @@ -607,12 +609,12 @@ pub(crate) fn read<const SIZE: usize, T, B>( > base: &B, > idx: usize, > ) -> Self where > - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, > - B: crate::regs::macros::RegisterBase<$base>, > + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>, > + B: $crate::register::RegisterBase<$base>, > { > build_assert!(idx < Self::SIZE); > > - let offset = <B as crate::regs::macros::RegisterBase<$base>>::BASE + > + let offset = <B as $crate::register::RegisterBase<$base>>::BASE + > Self::OFFSET + (idx * Self::STRIDE); > let value = io.read32(offset); > > @@ -629,12 +631,12 @@ pub(crate) fn write<const SIZE: usize, T, B>( > base: &B, > idx: usize > ) where > - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, > - B: crate::regs::macros::RegisterBase<$base>, > + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>, > + B: $crate::register::RegisterBase<$base>, > { > build_assert!(idx < Self::SIZE); > > - let offset = <B as crate::regs::macros::RegisterBase<$base>>::BASE + > + let offset = <B as $crate::register::RegisterBase<$base>>::BASE + > Self::OFFSET + (idx * Self::STRIDE); > > io.write32(self.0, offset); > @@ -650,8 +652,8 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>( > idx: usize, > f: F, > ) where > - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, > - B: crate::regs::macros::RegisterBase<$base>, > + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>, > + B: $crate::register::RegisterBase<$base>, > F: ::core::ops::FnOnce(Self) -> Self, > { > let reg = f(Self::read(io, base, idx)); > @@ -668,14 +670,14 @@ pub(crate) fn try_read<const SIZE: usize, T, B>( > io: &T, > base: &B, > idx: usize, > - ) -> ::kernel::error::Result<Self> where > - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, > - B: crate::regs::macros::RegisterBase<$base>, > + ) -> $crate::error::Result<Self> where > + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>, > + B: $crate::register::RegisterBase<$base>, > { > if idx < Self::SIZE { > Ok(Self::read(io, base, idx)) > } else { > - Err(EINVAL) > + Err($crate::error::code::EINVAL) > } > } > > @@ -690,14 +692,14 @@ pub(crate) fn try_write<const SIZE: usize, T, B>( > io: &T, > base: &B, > idx: usize, > - ) -> ::kernel::error::Result where > - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, > - B: crate::regs::macros::RegisterBase<$base>, > + ) -> $crate::error::Result where > + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>, > + B: $crate::register::RegisterBase<$base>, > { > if idx < Self::SIZE { > Ok(self.write(io, base, idx)) > } else { > - Err(EINVAL) > + Err($crate::error::code::EINVAL) > } > } > > @@ -713,17 +715,19 @@ pub(crate) fn try_alter<const SIZE: usize, T, B, F>( > base: &B, > idx: usize, > f: F, > - ) -> ::kernel::error::Result where > - T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>, > - B: crate::regs::macros::RegisterBase<$base>, > + ) -> $crate::error::Result where > + T: ::core::ops::Deref<Target = $crate::io::Io<SIZE>>, > + B: $crate::register::RegisterBase<$base>, > F: ::core::ops::FnOnce(Self) -> Self, > { > if idx < Self::SIZE { > Ok(Self::alter(io, base, idx, f)) > } else { > - Err(EINVAL) > + Err($crate::error::code::EINVAL) > } > } > } > }; > } > + > +pub use register; > -- > 2.34.1 > >Reviewed-by: Elle Rhumsaa <elle at weathered-steel.dev>
Miguel Ojeda
2025-Sep-07 18:15 UTC
[PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova
Hi Joel, I didn't check the macros, but a couple nits I noticed in this patch in particular given it moved it to `kernel`... On Wed, Sep 3, 2025 at 11:54?PM Joel Fernandes <joelagnelf at nvidia.com> wrote:> > +//! A library that provides support for defining bit fields in RustWhat about just "Support for defining bit fields in ..." or similar?> +//! structures. Also used from things that need bitfields like register macro.The "register macro" part sounds like it should be formatted with Markdown plus an intra-doc link.> - ::kernel::build_assert!( > + build_assert!(Is this path unqualified for some reason? Does it mean the user needs to have imported the prelude?> +pub use super::{bitstruct, register};Please justify in the commit message why we want them in the prelude, e.g. are they expected to be common? Does it simplify some code? etc. Thanks! Cheers, Miguel
Joel Fernandes
2025-Sep-08 17:06 UTC
[PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova
On 9/7/2025 2:14 PM, Miguel Ojeda wrote:> Hi Joel, > > I didn't check the macros, but a couple nits I noticed in this patch > in particular given it moved it to `kernel`... > > On Wed, Sep 3, 2025 at 11:54?PM Joel Fernandes <joelagnelf at nvidia.com> wrote: >> >> +//! A library that provides support for defining bit fields in Rust > > What about just "Support for defining bit fields in ..." or similar? > >> +//! structures. Also used from things that need bitfields like register macro.Ok, I changed it to: "Also used from things that need bitfields like [`register!`] macro." for next revision.> > The "register macro" part sounds like it should be formatted with > Markdown plus an intra-doc link. > >> - ::kernel::build_assert!( >> + build_assert!( > > Is this path unqualified for some reason? Does it mean the user needs > to have imported the prelude?Yes, for register macro importing prelude is required (I commented more below).> >> +pub use super::{bitstruct, register}; > > Please justify in the commit message why we want them in the prelude, > e.g. are they expected to be common? Does it simplify some code? etc. >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 think the prelude-way is clean, but let me know if there's any other trick I can try. I will also add this rationale to the commit message as you suggested. Thanks! - Joel