From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-106112.protonmail.ch (mail-106112.protonmail.ch [79.135.106.112]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C7E17286428; Fri, 5 Sep 2025 22:24:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=79.135.106.112 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1757111088; cv=none; b=W62+qZqNgfHHsabhVSlLRpnLeMBOIQE8Tl5BjXVDctHJO/FthBXYtVszUz97opNOUsXBhGY9mPt4ZPXE/u5I14mFwRvTtgj+cqeEDmQKJ2yMrMnBUDbkjWIzdtGeCj86ET/J3nFHjJws8RNo+VknI7V7HGEsREM/rtWouEgf+0U= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1757111088; c=relaxed/simple; bh=09vECX9RzrptiI5h7jc3i7H07wXMfDXNLBDYP2KZaYs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=E5gWLTZPlKMPAjoTvumLhD3SQ5WnlOhUKg1lhYo26zBu8L1/b4Hw4twov6PfhqIWl0KtYaQNHYBIf0hSFfboDqGUpj3iH3LfY/CTM956/kF1I2mos894nAtp5r/myrQ445eYYB3tkW5WCmdPRoRjLM9Ucx5200cfY3CPUeOMI0M= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=weathered-steel.dev; spf=pass smtp.mailfrom=weathered-steel.dev; dkim=pass (2048-bit key) header.d=weathered-steel.dev header.i=@weathered-steel.dev header.b=bsxj5Vc4; arc=none smtp.client-ip=79.135.106.112 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=weathered-steel.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=weathered-steel.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=weathered-steel.dev header.i=@weathered-steel.dev header.b="bsxj5Vc4" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=weathered-steel.dev; s=protonmail3; t=1757111078; x=1757370278; bh=i93EW/8ikm2pmoxKM3tODrWihAsKH9ShDBF4UztUg8o=; h=Date:From:To:Cc:Subject:Message-ID:References:In-Reply-To:From:To: Cc:Date:Subject:Reply-To:Feedback-ID:Message-ID:BIMI-Selector; b=bsxj5Vc4qYCuYztxM3FSUNTnDj0zYwTc635vHh6H3in39lzl1EehiB+BHRgigoJTE OCvx+MN4jXAPeuE4JRWYkEBginF/WTOe1g77p3DDcaFd9HSFJln2XzGyPPAfvA5rGQ R9vJjTPiZv9XhIrOzz/2wQt7tu3n7xvVoR9UwdSjCdRf070I+xE1debXWQw2hUU6ge 8uXD/7OCQ07uvwZaBDOgvkcEOC+bvaY/TdnzRPgT+GZejRbqkuyqWKkFnnbyPInwN0 GVGirG7jQR+csRMMwZvL+7tQQM4puti2k1xMMQDlY50s66u3Il0Dgd7HYGAzCzTzAo t+ohpbB8+RFFw== X-Pm-Submission-Id: 4cJW9b3x9tz2ScCs Date: Fri, 5 Sep 2025 22:24:32 +0000 From: Elle Rhumsaa To: Joel Fernandes Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, dakr@kernel.org, acourbot@nvidia.com, Alistair Popple , Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , bjorn3_gh@protonmail.com, Benno Lossin , Andreas Hindborg , Alice Ryhl , Trevor Gross , David Airlie , Simona Vetter , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , John Hubbard , Timur Tabi , joel@joelfernandes.org, Daniel Almeida , nouveau@lists.freedesktop.org, rust-for-linux@vger.kernel.org Subject: Re: [PATCH v2 4/4] rust: Move register and bitstruct macros out of Nova Message-ID: References: <20250903215428.1296517-1-joelagnelf@nvidia.com> <20250903215428.1296517-5-joelagnelf@nvidia.com> Precedence: bulk X-Mailing-List: rust-for-linux@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20250903215428.1296517-5-joelagnelf@nvidia.com> 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 > --- > 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 ?=> ` calls ``'s `TryFrom::<>` 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 = $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.[](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 { > +pub trait RegisterBase { > + /// The base address for the register. > const BASE: usize; > } > > @@ -281,6 +282,7 @@ pub(crate) trait RegisterBase { > /// # 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(io: &T) -> Self where > - T: ::core::ops::Deref>, > + T: ::core::ops::Deref>, > { > Self(io.read32($offset)) > } > @@ -386,7 +388,7 @@ pub(crate) fn read(io: &T) -> Self where > /// Write the value contained in `self` to the register address in `io`. > #[inline(always)] > pub(crate) fn write(self, io: &T) where > - T: ::core::ops::Deref>, > + T: ::core::ops::Deref>, > { > io.write32(self.0, $offset) > } > @@ -398,7 +400,7 @@ pub(crate) fn alter( > io: &T, > f: F, > ) where > - T: ::core::ops::Deref>, > + T: ::core::ops::Deref>, > F: ::core::ops::FnOnce(Self) -> Self, > { > let reg = f(Self::read(io)); > @@ -421,13 +423,13 @@ pub(crate) fn read( > #[allow(unused_variables)] > base: &B, > ) -> Self where > - T: ::core::ops::Deref>, > - B: crate::regs::macros::RegisterBase<$base>, > + T: ::core::ops::Deref>, > + B: $crate::register::RegisterBase<$base>, > { > const OFFSET: usize = $name::OFFSET; > > let value = io.read32( > - >::BASE + OFFSET > + >::BASE + OFFSET > ); > > Self(value) > @@ -442,14 +444,14 @@ pub(crate) fn write( > #[allow(unused_variables)] > base: &B, > ) where > - T: ::core::ops::Deref>, > - B: crate::regs::macros::RegisterBase<$base>, > + T: ::core::ops::Deref>, > + B: $crate::register::RegisterBase<$base>, > { > const OFFSET: usize = $name::OFFSET; > > io.write32( > self.0, > - >::BASE + OFFSET > + >::BASE + OFFSET > ); > } > > @@ -462,8 +464,8 @@ pub(crate) fn alter( > base: &B, > f: F, > ) where > - T: ::core::ops::Deref>, > - B: crate::regs::macros::RegisterBase<$base>, > + T: ::core::ops::Deref>, > + 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( > io: &T, > idx: usize, > ) -> Self where > - T: ::core::ops::Deref>, > + T: ::core::ops::Deref>, > { > build_assert!(idx < Self::SIZE); > > @@ -503,7 +505,7 @@ pub(crate) fn write( > io: &T, > idx: usize > ) where > - T: ::core::ops::Deref>, > + T: ::core::ops::Deref>, > { > build_assert!(idx < Self::SIZE); > > @@ -520,7 +522,7 @@ pub(crate) fn alter( > idx: usize, > f: F, > ) where > - T: ::core::ops::Deref>, > + T: ::core::ops::Deref>, > F: ::core::ops::FnOnce(Self) -> Self, > { > let reg = f(Self::read(io, idx)); > @@ -535,13 +537,13 @@ pub(crate) fn alter( > pub(crate) fn try_read( > io: &T, > idx: usize, > - ) -> ::kernel::error::Result where > - T: ::core::ops::Deref>, > + ) -> $crate::error::Result where > + T: ::core::ops::Deref>, > { > 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( > self, > io: &T, > idx: usize, > - ) -> ::kernel::error::Result where > - T: ::core::ops::Deref>, > + ) -> $crate::error::Result where > + T: ::core::ops::Deref>, > { > 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( > io: &T, > idx: usize, > f: F, > - ) -> ::kernel::error::Result where > - T: ::core::ops::Deref>, > + ) -> $crate::error::Result where > + T: ::core::ops::Deref>, > 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( > base: &B, > idx: usize, > ) -> Self where > - T: ::core::ops::Deref>, > - B: crate::regs::macros::RegisterBase<$base>, > + T: ::core::ops::Deref>, > + B: $crate::register::RegisterBase<$base>, > { > build_assert!(idx < Self::SIZE); > > - let offset = >::BASE + > + let offset = >::BASE + > Self::OFFSET + (idx * Self::STRIDE); > let value = io.read32(offset); > > @@ -629,12 +631,12 @@ pub(crate) fn write( > base: &B, > idx: usize > ) where > - T: ::core::ops::Deref>, > - B: crate::regs::macros::RegisterBase<$base>, > + T: ::core::ops::Deref>, > + B: $crate::register::RegisterBase<$base>, > { > build_assert!(idx < Self::SIZE); > > - let offset = >::BASE + > + let offset = >::BASE + > Self::OFFSET + (idx * Self::STRIDE); > > io.write32(self.0, offset); > @@ -650,8 +652,8 @@ pub(crate) fn alter( > idx: usize, > f: F, > ) where > - T: ::core::ops::Deref>, > - B: crate::regs::macros::RegisterBase<$base>, > + T: ::core::ops::Deref>, > + 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( > io: &T, > base: &B, > idx: usize, > - ) -> ::kernel::error::Result where > - T: ::core::ops::Deref>, > - B: crate::regs::macros::RegisterBase<$base>, > + ) -> $crate::error::Result where > + T: ::core::ops::Deref>, > + 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( > io: &T, > base: &B, > idx: usize, > - ) -> ::kernel::error::Result where > - T: ::core::ops::Deref>, > - B: crate::regs::macros::RegisterBase<$base>, > + ) -> $crate::error::Result where > + T: ::core::ops::Deref>, > + 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( > base: &B, > idx: usize, > f: F, > - ) -> ::kernel::error::Result where > - T: ::core::ops::Deref>, > - B: crate::regs::macros::RegisterBase<$base>, > + ) -> $crate::error::Result where > + T: ::core::ops::Deref>, > + 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