[PATCH 0/2] gpu: nova-core: convert to kernel bitfield macro and remove local variant

[PATCH 0/2] gpu: nova-core: convert to kernel bitfield macro and remove local variant

[Alexandre Courbot]

Now that the kernel-wide bitfield macro is available, it is time for the
Nova-local version to be removed.

The first patch converts all uses of the local macro to the global one.
This involves redefining all bitfields since the syntax has changed, and
taking into account the use of `Bounded`.

The second patch removes the now unused Nova-local bitfield macro.

This is a follow-up to [1] containing the Nova patches that have been
dropped after [2].

This series applies on top of a merge of `drm-rust-next` and today's
`master`. It is to be applied after `-rc1` is tagged and merged into
`drm-rust-next`.

[1] https://lore.kernel.org/all/20260606-bitfield-v5-0-b92188820914@nvidia.com/
[2] https://lore.kernel.org/all/20260527-bitfield-v4-0-e8821d4efbde@nvidia.com/

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
Alexandre Courbot (2):
      gpu: nova-core: convert to kernel bitfield macro
      gpu: nova-core: remove local bitfield macro

 drivers/gpu/nova-core/bitfield.rs  | 329 -------------------------------------
 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 -
 5 files changed, 56 insertions(+), 381 deletions(-)
---
base-commit: 31686e06c0342de47f40a0278bfe8126de86c3ec
change-id: 20260617-nova-bitfield-be4bbb298a00

Best regards,
--  
Alexandre Courbot <acourbot@nvidia.com>

[PATCH 1/2] gpu: nova-core: convert to kernel bitfield macro

[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

[PATCH 2/2] gpu: nova-core: remove local bitfield macro

[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

Re: [PATCH 1/2] gpu: nova-core: convert to kernel bitfield macro

[Eliot Courtney]

On Wed Jun 17, 2026 at 11:02 PM JST, Alexandre Courbot wrote:
> 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>
> ---

Modulo the comment raised by sashiko about skipping the log on error,

Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>