* [PATCH 1/2] gpu: nova-core: convert to kernel bitfield macro
2026-06-17 14:02 [PATCH 0/2] gpu: nova-core: convert to kernel bitfield macro and remove local variant Alexandre Courbot
@ 2026-06-17 14:02 ` Alexandre Courbot
2026-06-17 14:11 ` sashiko-bot
2026-06-22 5:23 ` Eliot Courtney
2026-06-17 14:02 ` [PATCH 2/2] gpu: nova-core: remove local " Alexandre Courbot
1 sibling, 2 replies; 5+ messages in thread
From: Alexandre Courbot @ 2026-06-17 14:02 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Daniel Almeida,
Tamir Duberstein, Onur Özkan
Cc: John Hubbard, Alistair Popple, Timur Tabi, Eliot Courtney,
Zhi Wang, nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
Replace uses of the Nova-local `bitfield!` macro with the kernel one.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Acked-by: Danilo Krummrich <dakr@kernel.org>
---
drivers/gpu/nova-core/fsp.rs | 8 +++-
drivers/gpu/nova-core/gsp/fw.rs | 11 ++---
drivers/gpu/nova-core/mctp.rs | 86 +++++++++++++++++++-------------------
drivers/gpu/nova-core/nova_core.rs | 3 --
4 files changed, 56 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/nova-core/fsp.rs b/drivers/gpu/nova-core/fsp.rs
index d949c03dd304..e8becf11a321 100644
--- a/drivers/gpu/nova-core/fsp.rs
+++ b/drivers/gpu/nova-core/fsp.rs
@@ -11,6 +11,7 @@
device,
dma::Coherent,
io::poll::read_poll_timeout,
+ num::TryIntoBounded,
prelude::*,
ptr::{
Alignable,
@@ -297,7 +298,12 @@ fn send_sync_fsp<M>(&mut self, dev: &device::Device, bar: Bar0<'_>, msg: &M) ->
return Err(EIO);
}
- if command_nvdm_type != u8::from(M::NVDM_TYPE).into() {
+ if command_nvdm_type
+ .try_into_bounded()
+ .ok_or(EINVAL)
+ .and_then(NvdmType::try_from)?
+ != M::NVDM_TYPE
+ {
dev_err!(
dev,
"Expected NVDM type {:?} in reply, got {:#x}\n",
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index 4db0cfa4dc4d..5a8392b33b4a 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -10,6 +10,7 @@
use core::ops::Range;
use kernel::{
+ bitfield,
dma::Coherent,
prelude::*,
ptr::{
@@ -742,8 +743,8 @@ unsafe impl AsBytes for MsgqRxHeader {}
bitfield! {
struct MsgHeaderVersion(u32) {
- 31:24 major as u8;
- 23:16 minor as u8;
+ 31:24 major;
+ 23:16 minor;
}
}
@@ -752,9 +753,9 @@ impl MsgHeaderVersion {
const MINOR_TOT: u8 = 0;
fn new() -> Self {
- Self::default()
- .set_major(Self::MAJOR_TOT)
- .set_minor(Self::MINOR_TOT)
+ Self::zeroed()
+ .with_major(Self::MAJOR_TOT)
+ .with_minor(Self::MINOR_TOT)
}
}
diff --git a/drivers/gpu/nova-core/mctp.rs b/drivers/gpu/nova-core/mctp.rs
index 482786e07bc7..acc2abbd4b0c 100644
--- a/drivers/gpu/nova-core/mctp.rs
+++ b/drivers/gpu/nova-core/mctp.rs
@@ -7,55 +7,51 @@
//! Data Model) messages between the kernel driver and GPU firmware processors
//! such as FSP and GSP.
-use kernel::pci::Vendor;
+use kernel::{
+ bitfield,
+ pci::Vendor,
+ prelude::*, //
+};
-/// NVDM message type identifiers carried over MCTP.
-#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
-#[repr(u8)]
-pub(crate) enum NvdmType {
- #[default]
- /// Chain of Trust boot message.
- Cot = 0x14,
- /// FSP command response.
- FspResponse = 0x15,
-}
+use crate::{
+ bounded_enum,
+ num, //
+};
-impl TryFrom<u8> for NvdmType {
- type Error = u8;
-
- fn try_from(value: u8) -> Result<Self, Self::Error> {
- match value {
- x if x == u8::from(Self::Cot) => Ok(Self::Cot),
- x if x == u8::from(Self::FspResponse) => Ok(Self::FspResponse),
- _ => Err(value),
- }
- }
-}
-
-impl From<NvdmType> for u8 {
- fn from(value: NvdmType) -> Self {
- value as u8
+bounded_enum! {
+ /// NVDM message type identifiers carried over MCTP.
+ #[derive(Debug, Clone, Copy, PartialEq, Eq)]
+ pub(crate) enum NvdmType with TryFrom<Bounded<u32, 8>> {
+ /// Chain of Trust boot message.
+ Cot = 0x14,
+ /// FSP command response.
+ FspResponse = 0x15,
}
}
bitfield! {
- pub(crate) struct MctpHeader(u32), "MCTP transport header for NVIDIA firmware messages." {
- 31:31 som as bool, "Start-of-message bit.";
- 30:30 eom as bool, "End-of-message bit.";
- 29:28 seq as u8, "Packet sequence number.";
- 23:16 seid as u8, "Source endpoint ID.";
+ /// MCTP transport header for NVIDIA firmware messages.
+ pub(crate) struct MctpHeader(u32) {
+ /// Start-of-message bit.
+ 31:31 som;
+ /// End-of-message bit.
+ 30:30 eom;
+ /// Packet sequence number.
+ 29:28 seq;
+ /// Source endpoint ID.
+ 23:16 seid;
}
}
impl MctpHeader {
/// Builds a single-packet MCTP header (`SOM=1`, `EOM=1`, `SEQ=0`, `SEID=0`).
pub(crate) fn single_packet() -> Self {
- Self::default().set_som(true).set_eom(true)
+ Self::zeroed().with_som(true).with_eom(true)
}
/// Returns whether this is a complete single-packet message (`SOM=1` and `EOM=1`).
pub(crate) fn is_single_packet(self) -> bool {
- self.som() && self.eom()
+ self.som().into_bool() && self.eom().into_bool()
}
}
@@ -63,26 +59,30 @@ pub(crate) fn is_single_packet(self) -> bool {
const MSG_TYPE_VENDOR_PCI: u8 = 0x7e;
bitfield! {
- pub(crate) struct NvdmHeader(u32), "NVIDIA Vendor-Defined Message header over MCTP." {
- 31:24 nvdm_type as u8 ?=> NvdmType, "NVDM message type.";
- 23:8 vendor_id as u16, "PCI vendor ID.";
- 6:0 msg_type as u8, "MCTP vendor-defined message type.";
+ /// NVIDIA Vendor-Defined Message header over MCTP.
+ pub(crate) struct NvdmHeader(u32) {
+ /// NVDM message type.
+ 31:24 nvdm_type ?=> NvdmType;
+ /// PCI vendor ID.
+ 23:8 vendor_id;
+ /// MCTP vendor-defined message type.
+ 6:0 msg_type;
}
}
impl NvdmHeader {
/// Builds an NVDM header for the given message type.
pub(crate) fn new(nvdm_type: NvdmType) -> Self {
- Self::default()
- .set_msg_type(MSG_TYPE_VENDOR_PCI)
- .set_vendor_id(Vendor::NVIDIA.as_raw())
- .set_nvdm_type(nvdm_type)
+ Self::zeroed()
+ .with_const_msg_type::<{ num::u8_as_u32(MSG_TYPE_VENDOR_PCI) }>()
+ .with_vendor_id(Vendor::NVIDIA.as_raw())
+ .with_nvdm_type(nvdm_type)
}
/// Validates this header against the expected NVIDIA NVDM format and type.
pub(crate) fn validate(self, expected_type: NvdmType) -> bool {
- self.msg_type() == MSG_TYPE_VENDOR_PCI
- && self.vendor_id() == Vendor::NVIDIA.as_raw()
+ u8::from(self.msg_type()) == MSG_TYPE_VENDOR_PCI
+ && u16::from(self.vendor_id()) == Vendor::NVIDIA.as_raw()
&& matches!(self.nvdm_type(), Ok(nvdm_type) if nvdm_type == expected_type)
}
}
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index 735b8e17c6b6..a61406ba5c0b 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -10,9 +10,6 @@
InPlaceModule, //
};
-#[macro_use]
-mod bitfield;
-
mod driver;
mod falcon;
mod fb;
--
2.54.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] gpu: nova-core: remove local bitfield macro
2026-06-17 14:02 [PATCH 0/2] gpu: nova-core: convert to kernel bitfield macro and remove local variant Alexandre Courbot
2026-06-17 14:02 ` [PATCH 1/2] gpu: nova-core: convert to kernel bitfield macro Alexandre Courbot
@ 2026-06-17 14:02 ` Alexandre Courbot
1 sibling, 0 replies; 5+ messages in thread
From: Alexandre Courbot @ 2026-06-17 14:02 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Daniel Almeida,
Tamir Duberstein, Onur Özkan
Cc: John Hubbard, Alistair Popple, Timur Tabi, Eliot Courtney,
Zhi Wang, nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
This module is now orphaned code, superseded by a kernel-global
implementation, so remove it.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
Acked-by: Danilo Krummrich <dakr@kernel.org>
---
drivers/gpu/nova-core/bitfield.rs | 329 --------------------------------------
1 file changed, 329 deletions(-)
diff --git a/drivers/gpu/nova-core/bitfield.rs b/drivers/gpu/nova-core/bitfield.rs
deleted file mode 100644
index 660c3911402d..000000000000
--- a/drivers/gpu/nova-core/bitfield.rs
+++ /dev/null
@@ -1,329 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-
-//! Bitfield library for Rust structures
-//!
-//! Support for defining bitfields in Rust structures. Also used by the [`register!`] macro.
-
-/// Defines a struct with accessors to access bits within an inner unsigned integer.
-///
-/// # Syntax
-///
-/// ```rust
-/// use nova_core::bitfield;
-///
-/// #[derive(Debug, Clone, Copy, Default)]
-/// enum Mode {
-/// #[default]
-/// Low = 0,
-/// High = 1,
-/// Auto = 2,
-/// }
-///
-/// impl TryFrom<u8> for Mode {
-/// type Error = u8;
-/// fn try_from(value: u8) -> Result<Self, Self::Error> {
-/// match value {
-/// 0 => Ok(Mode::Low),
-/// 1 => Ok(Mode::High),
-/// 2 => Ok(Mode::Auto),
-/// _ => Err(value),
-/// }
-/// }
-/// }
-///
-/// impl From<Mode> for u8 {
-/// fn from(mode: Mode) -> u8 {
-/// mode as u8
-/// }
-/// }
-///
-/// #[derive(Debug, Clone, Copy, Default)]
-/// enum State {
-/// #[default]
-/// Inactive = 0,
-/// Active = 1,
-/// }
-///
-/// impl From<bool> for State {
-/// fn from(value: bool) -> Self {
-/// if value { State::Active } else { State::Inactive }
-/// }
-/// }
-///
-/// impl From<State> for bool {
-/// fn from(state: State) -> bool {
-/// match state {
-/// State::Inactive => false,
-/// State::Active => true,
-/// }
-/// }
-/// }
-///
-/// bitfield! {
-/// pub struct ControlReg(u32) {
-/// 7:7 state as bool => State;
-/// 3:0 mode as u8 ?=> Mode;
-/// }
-/// }
-/// ```
-///
-/// This generates a struct with:
-/// - Field accessors: `mode()`, `state()`, etc.
-/// - Field setters: `set_mode()`, `set_state()`, etc. (supports chaining with builder pattern).
-/// Note that the compiler will error out if the size of the setter's arg exceeds the
-/// struct's storage size.
-/// - Debug and Default implementations.
-///
-/// Note: Field accessors and setters inherit the same visibility as the struct itself.
-/// In the example above, both `mode()` and `set_mode()` methods will be `pub`.
-///
-/// Fields are defined as follows:
-///
-/// - `as <type>` simply returns the field value casted to <type>, typically `u32`, `u16`, `u8` or
-/// `bool`. Note that `bool` fields must have a range of 1 bit.
-/// - `as <type> => <into_type>` calls `<into_type>`'s `From::<<type>>` implementation and returns
-/// 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_rules! bitfield {
- // Main entry point - defines the bitfield struct with fields
- ($vis:vis struct $name:ident($storage:ty) $(, $comment:literal)? { $($fields:tt)* }) => {
- bitfield!(@core $vis $name $storage $(, $comment)? { $($fields)* });
- };
-
- // All rules below are helpers.
-
- // Defines the wrapper `$name` type, as well as its relevant implementations (`Debug`,
- // `Default`, and conversion to the value type) and field accessor methods.
- (@core $vis:vis $name:ident $storage:ty $(, $comment:literal)? { $($fields:tt)* }) => {
- $(
- #[doc=$comment]
- )?
- #[repr(transparent)]
- #[derive(Clone, Copy)]
- $vis struct $name($storage);
-
- impl ::core::convert::From<$name> for $storage {
- fn from(val: $name) -> $storage {
- val.0
- }
- }
-
- bitfield!(@fields_dispatcher $vis $name $storage { $($fields)* });
- };
-
- // Captures the fields and passes them to all the implementers that require field information.
- //
- // Used to simplify the matching rules for implementers, so they don't need to match the entire
- // complex fields rule even though they only make use of part of it.
- (@fields_dispatcher $vis:vis $name:ident $storage:ty {
- $($hi:tt:$lo:tt $field:ident as $type:tt
- $(?=> $try_into_type:ty)?
- $(=> $into_type:ty)?
- $(, $comment:literal)?
- ;
- )*
- }
- ) => {
- bitfield!(@field_accessors $vis $name $storage {
- $(
- $hi:$lo $field as $type
- $(?=> $try_into_type)?
- $(=> $into_type)?
- $(, $comment)?
- ;
- )*
- });
- bitfield!(@debug $name { $($field;)* });
- bitfield!(@default $name { $($field;)* });
- };
-
- // Defines all the field getter/setter methods for `$name`.
- (
- @field_accessors $vis:vis $name:ident $storage:ty {
- $($hi:tt:$lo:tt $field:ident as $type:tt
- $(?=> $try_into_type:ty)?
- $(=> $into_type:ty)?
- $(, $comment:literal)?
- ;
- )*
- }
- ) => {
- $(
- bitfield!(@check_field_bounds $hi:$lo $field as $type);
- )*
-
- #[allow(dead_code)]
- impl $name {
- $(
- bitfield!(@field_accessor $vis $name $storage, $hi:$lo $field as $type
- $(?=> $try_into_type)?
- $(=> $into_type)?
- $(, $comment)?
- ;
- );
- )*
- }
- };
-
- // Boolean fields must have `$hi == $lo`.
- (@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")
- );
- };
- };
-
- // Non-boolean fields must have `$hi >= $lo`.
- (@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")
- );
- };
- };
-
- // Catches fields defined as `bool` and convert them into a boolean value.
- (
- @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool
- => $into_type:ty $(, $comment:literal)?;
- ) => {
- bitfield!(
- @leaf_accessor $vis $name $storage, $hi:$lo $field
- { |f| <$into_type>::from(f != 0) }
- bool $into_type => $into_type $(, $comment)?;
- );
- };
-
- // Shortcut for fields defined as `bool` without the `=>` syntax.
- (
- @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as bool
- $(, $comment:literal)?;
- ) => {
- bitfield!(
- @field_accessor $vis $name $storage, $hi:$lo $field as bool => bool $(, $comment)?;
- );
- };
-
- // Catches the `?=>` syntax for non-boolean fields.
- (
- @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
- ?=> $try_into_type:ty $(, $comment:literal)?;
- ) => {
- bitfield!(@leaf_accessor $vis $name $storage, $hi:$lo $field
- { |f| <$try_into_type>::try_from(f as $type) } $type $try_into_type =>
- ::core::result::Result<
- $try_into_type,
- <$try_into_type as ::core::convert::TryFrom<$type>>::Error
- >
- $(, $comment)?;);
- };
-
- // Catches the `=>` syntax for non-boolean fields.
- (
- @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
- => $into_type:ty $(, $comment:literal)?;
- ) => {
- bitfield!(@leaf_accessor $vis $name $storage, $hi:$lo $field
- { |f| <$into_type>::from(f as $type) } $type $into_type => $into_type $(, $comment)?;);
- };
-
- // Shortcut for non-boolean fields defined without the `=>` or `?=>` syntax.
- (
- @field_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident as $type:tt
- $(, $comment:literal)?;
- ) => {
- bitfield!(
- @field_accessor $vis $name $storage, $hi:$lo $field as $type => $type $(, $comment)?;
- );
- };
-
- // Generates the accessor methods for a single field.
- (
- @leaf_accessor $vis:vis $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident
- { $process:expr } $prim_type:tt $to_type:ty => $res_type:ty $(, $comment:literal)?;
- ) => {
- ::kernel::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,
- _ => ::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)
- }
-
- ::kernel::macros::paste!(
- $(
- #[doc="Sets the value of this field:"]
- #[doc=$comment]
- )?
- #[inline(always)]
- $vis fn [<set_ $field>](mut self, value: $to_type) -> Self {
- const MASK: $storage = $name::[<$field:upper _MASK>];
- const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
- let value = ($storage::from($prim_type::from(value)) << SHIFT) & MASK;
- self.0 = (self.0 & !MASK) | value;
-
- self
- }
- );
- };
-
- // Generates the `Debug` implementation for `$name`.
- (@debug $name:ident { $($field:ident;)* }) => {
- impl ::kernel::fmt::Debug for $name {
- fn fmt(&self, f: &mut ::kernel::fmt::Formatter<'_>) -> ::kernel::fmt::Result {
- f.debug_struct(stringify!($name))
- .field("<raw>", &::kernel::prelude::fmt!("{:#x}", &self.0))
- $(
- .field(stringify!($field), &self.$field())
- )*
- .finish()
- }
- }
- };
-
- // Generates the `Default` implementation for `$name`.
- (@default $name:ident { $($field:ident;)* }) => {
- /// Returns a value for the bitfield where all fields are set to their default value.
- impl ::core::default::Default for $name {
- fn default() -> Self {
- let value = Self(Default::default());
-
- ::kernel::macros::paste!(
- $(
- let value = value.[<set_ $field>](Default::default());
- )*
- );
-
- value
- }
- }
- };
-}
--
2.54.0
^ permalink raw reply related [flat|nested] 5+ messages in thread