Re: [PATCH v4 7/7] rust: bitfield: Add KUnit tests for bitfield

[Yury Norov <ynorov@nvidia.com>]

On Wed, May 27, 2026 at 09:52:01PM +0900, Alexandre Courbot wrote:
> From: Joel Fernandes <joelagnelf@nvidia.com>
> 
> Add KUNIT tests to make sure the macro is working correctly. The unit
> tests are put behind the new `RUST_BITFIELD_KUNIT_TEST` Kconfig option.
> 
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> Co-developed-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  rust/kernel/Kconfig.test |  10 ++
>  rust/kernel/bitfield.rs  | 315 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 325 insertions(+)
> 
> diff --git a/rust/kernel/Kconfig.test b/rust/kernel/Kconfig.test
> index fc47614e6ec9..4fc6dc978101 100644
> --- a/rust/kernel/Kconfig.test
> +++ b/rust/kernel/Kconfig.test
> @@ -73,4 +73,14 @@ config RUST_ATOMICS_KUNIT_TEST
>  
>  	  If unsure, say N.
>  
> +config RUST_BITFIELD_KUNIT_TEST
> +	bool "KUnit tests for the Rust `bitfield!` macro" if !KUNIT_ALL_TESTS
> +	default KUNIT_ALL_TESTS
> +	help
> +	  This option enables KUnit tests for the Rust `bitfield!` macro.
> +	  These are only for development and testing, not for regular
> +	  kernel use cases.
> +
> +	  If unsure, say N.
> +
>  endif
> diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
> index 2498107979dc..4720cdd23c74 100644
> --- a/rust/kernel/bitfield.rs
> +++ b/rust/kernel/bitfield.rs
> @@ -546,3 +546,318 @@ fn fmt(&self, f: &mut ::kernel::fmt::Formatter<'_>) -> ::kernel::fmt::Result {
>          }
>      };
>  }
> +
> +#[cfg(CONFIG_RUST_BITFIELD_KUNIT_TEST)]

Here, can you also add a comment describing limitations, like
unsigned base types only, prohibited names, rules for unallocated
bits.

The motivation is simple: you demonstrate which behavior is intentional,
and which is implementation detail. So that, if someone changes the code,
he'd have to walk through the test to ensure consistency, and if that
doesn't hold - inspect the codebase for the corner cases.

> +#[::kernel::macros::kunit_tests(kernel_bitfield)]
> +mod tests {
> +    use core::convert::TryFrom;
> +
> +    use pin_init::Zeroable;
> +
> +    use kernel::num::Bounded;
> +
> +    // Enum types for testing => and ?=> conversions
> +    #[derive(Debug, Clone, Copy, PartialEq)]
> +    enum MemoryType {
> +        Unmapped = 0,
> +        Normal = 1,
> +        Device = 2,
> +        Reserved = 3,
> +    }
> +
> +    impl TryFrom<Bounded<u64, 4>> for MemoryType {
> +        type Error = u64;
> +        fn try_from(value: Bounded<u64, 4>) -> Result<Self, Self::Error> {
> +            match value.get() {
> +                0 => Ok(MemoryType::Unmapped),
> +                1 => Ok(MemoryType::Normal),
> +                2 => Ok(MemoryType::Device),
> +                3 => Ok(MemoryType::Reserved),
> +                _ => Err(value.get()),

Does this engine supports non-sequential enumeration, like:
        
                   0 => Ok(MemoryType::Unmapped),
                   1 => Ok(MemoryType::Normal),
                   2 => Err(value.get()),
                   3 => Ok(MemoryType::Reserved),
                   _ => Err(value.get()),

If so, can you add a test for it? (This is a real example from my
working place, FWIW.)

> +            }
> +        }
> +    }
> +
> +    impl From<MemoryType> for Bounded<u64, 4> {
> +        fn from(mt: MemoryType) -> Bounded<u64, 4> {
> +            Bounded::from_expr(mt as u64)
> +        }
> +    }
> +
> +    #[derive(Debug, Clone, Copy, PartialEq)]
> +    enum Priority {
> +        Low = 0,
> +        Medium = 1,
> +        High = 2,
> +        Critical = 3,
> +    }
> +
> +    impl From<Bounded<u16, 2>> for Priority {
> +        fn from(value: Bounded<u16, 2>) -> Self {
> +            match value & 0x3 {
> +                0 => Priority::Low,
> +                1 => Priority::Medium,
> +                2 => Priority::High,
> +                _ => Priority::Critical,
> +            }
> +        }
> +    }
> +
> +    impl From<Priority> for Bounded<u16, 2> {
> +        fn from(p: Priority) -> Bounded<u16, 2> {
> +            Bounded::from_expr(p as u16)
> +        }
> +    }
> +
> +    bitfield! {
> +        struct TestPageTableEntry(u64) {

Can you add bit 63 in the list? The highest and lowest bits must be
covered in some test. Maybe create a separate subtest, if you don't
want to touch this one?

> +            61:52     available2;
> +            51:16     pfn;
> +            15:12     mem_type ?=> MemoryType;
> +            11:9      available;
> +            1:1       writable;
> +            0:0       present;
> +        }
> +    }
> +
> +    bitfield! {
> +        struct TestControlRegister(u16) {

For the purpose of testing, I think it's more clear to give names that
are meaningful in the testing context, like u16reg, u8reg, and so on.
Where is the 32-bit register by the way? Is it omitted on purpose?

> +            15:8      channel;
> +            7:4       priority_nibble;
> +            5:4       priority => Priority;

I understand, you want the test looking more realistic. But giving
'real' names may mislead and distract. Instead, can you name the
fields such that they underline the testing intention.

For example, this part may look like:

               7:4       split_field;
               7:6       split_field_hi;
               5:4       split_field_lo => Priority;

So later in the code, you may add another trivially looking consistency
check:
        assert!(split_field == (split_field_hi << 2) | split_field_lo)

> +            3:1       mode;
> +            0:0       enable;
> +        }
> +    }
> +
> +    bitfield! {
> +        struct TestStatusRegister(u8) {
> +            7:0       full_byte;  // For entire register

Don't you have the .into() for it? Do you encourage to use that
placeholder instead of the method in some cases? Can you elaborate?

> +            7:4       reserved;
> +            3:2       state;
> +            1:1       error;
> +            0:0       ready;
> +        }
> +    }
> +    #[test]
> +    fn test_single_bits() {
> +        let mut pte = TestPageTableEntry::zeroed();
> +
> +        assert!(!pte.present().into_bool());
> +        assert!(!pte.writable().into_bool());
> +        assert_eq!(u64::from(pte), 0x0);
> +
> +        pte = pte.with_present(true);
> +        assert!(pte.present().into_bool());
> +        assert_eq!(u64::from(pte), 0x1);
> +
> +        pte = pte.with_writable(true);
> +        assert!(pte.writable().into_bool());
> +        assert_eq!(u64::from(pte), 0x3);
> +
> +        pte = pte.with_writable(false);
> +        assert!(!pte.writable().into_bool());
> +        assert_eq!(u64::from(pte), 0x1);
> +
> +        assert_eq!(pte.available(), 0);
> +        pte = pte.with_const_available::<0x5>();
> +        assert_eq!(pte.available(), 0x5);
> +        assert_eq!(u64::from(pte), 0xA01);
> +    }
> +
> +    #[test]
> +    fn test_range_fields() {
> +        let mut pte = TestPageTableEntry::zeroed();
> +        assert_eq!(u64::from(pte), 0x0);
> +
> +        pte = pte.with_const_pfn::<0x123456>();
> +        assert_eq!(pte.pfn(), 0x123456);
> +        assert_eq!(u64::from(pte), 0x1234560000);
> +
> +        pte = pte.with_const_available::<0x7>();
> +        assert_eq!(pte.available(), 0x7);
> +        assert_eq!(u64::from(pte), 0x1234560E00);
> +
> +        pte = pte.with_const_available2::<0x3FF>();
> +        assert_eq!(pte.available2(), 0x3FF);
> +        assert_eq!(u64::from(pte), 0x3FF0_0012_3456_0E00u64);
> +
> +        // Test TryFrom with ?=> for MemoryType
> +        pte = pte.with_mem_type(MemoryType::Device);
> +        assert_eq!(pte.mem_type(), Ok(MemoryType::Device));
> +        assert_eq!(u64::from(pte), 0x3FF0_0012_3456_2E00u64);
> +
> +        pte = pte.with_mem_type(MemoryType::Normal);
> +        assert_eq!(pte.mem_type(), Ok(MemoryType::Normal));
> +        assert_eq!(u64::from(pte), 0x3FF0_0012_3456_1E00u64);
> +
> +        // Test all valid values for mem_type
> +        pte = pte.with_mem_type(MemoryType::Reserved);
> +        assert_eq!(pte.mem_type(), Ok(MemoryType::Reserved));
> +        assert_eq!(u64::from(pte), 0x3FF0_0012_3456_3E00u64);
> +
> +        // Test failure case using mem_type field which has 4 bits (0-15)
> +        // MemoryType only handles 0-3, so values 4-15 should return Err
> +        let mut raw = pte.into_raw();
> +        // Set bits 15:12 to 7 (invalid for MemoryType)
> +        raw = (raw & !::kernel::bits::genmask_u64(12..=15)) | (0x7 << 12);
> +        let invalid_pte = TestPageTableEntry::from_raw(raw);
> +        // Should return Err with the invalid value
> +        assert_eq!(invalid_pte.mem_type(), Err(0x7));
> +
> +        // Test a valid value after testing invalid to ensure both cases work
> +        // Set bits 15:12 to 2 (valid: Device)
> +        raw = (raw & !::kernel::bits::genmask_u64(12..=15)) | (0x2 << 12);
> +        let valid_pte = TestPageTableEntry::from_raw(raw);
> +        assert_eq!(valid_pte.mem_type(), Ok(MemoryType::Device));
> +
> +        const MAX_PFN: u64 = ::kernel::bits::genmask_u64(0..=35);
> +        pte = pte.with_const_pfn::<{ MAX_PFN }>();
> +        assert_eq!(pte.pfn(), MAX_PFN);
> +    }
> +
> +    #[test]
> +    fn test_builder_pattern() {
> +        let pte = TestPageTableEntry::zeroed()
> +            .with_present(true)
> +            .with_writable(true)
> +            .with_const_available::<0x7>()
> +            .with_const_pfn::<0xABCDEF>()
> +            .with_mem_type(MemoryType::Reserved)
> +            .with_const_available2::<0x3FF>();

I can't find try_with in the test. Can you test it too?
Can I chain it like:
        
        let bitfield = TestU64Entry::zeroes()
                .try_with_present(val1)
                .try_with_writable(val2)
        ...

Thanks,
Yury

> +
> +        assert!(pte.present().into_bool());
> +        assert!(pte.writable().into_bool());
> +        assert_eq!(pte.available(), 0x7);
> +        assert_eq!(pte.pfn(), 0xABCDEF);
> +        assert_eq!(pte.mem_type(), Ok(MemoryType::Reserved));
> +        assert_eq!(pte.available2(), 0x3FF);
> +    }
> +
> +    #[test]
> +    fn test_raw_operations() {
> +        let raw_value = 0x3FF0000031233E03u64;
> +
> +        let pte = TestPageTableEntry::from_raw(raw_value);
> +        assert_eq!(u64::from(pte), raw_value);
> +
> +        assert!(pte.present().into_bool());
> +        assert!(pte.writable().into_bool());
> +        assert_eq!(pte.available(), 0x7);
> +        assert_eq!(pte.pfn(), 0x3123);
> +        assert_eq!(pte.mem_type(), Ok(MemoryType::Reserved));
> +        assert_eq!(pte.available2(), 0x3FF);
> +    }
> +
> +    #[test]
> +    fn test_u16_bitfield() {
> +        let mut ctrl = TestControlRegister::zeroed();
> +
> +        assert!(!ctrl.enable().into_bool());
> +        assert_eq!(ctrl.mode(), 0);
> +        assert_eq!(ctrl.priority(), Priority::Low);
> +        assert_eq!(ctrl.priority_nibble(), 0);
> +        assert_eq!(ctrl.channel(), 0);
> +
> +        ctrl = ctrl.with_enable(true);
> +        assert!(ctrl.enable().into_bool());
> +
> +        ctrl = ctrl.with_const_mode::<0x5>();
> +        assert_eq!(ctrl.mode(), 0x5);
> +
> +        // Test From conversion with =>
> +        ctrl = ctrl.with_priority(Priority::High);
> +        assert_eq!(ctrl.priority(), Priority::High);
> +        assert_eq!(ctrl.priority_nibble(), 0x2); // High = 2 in bits 5:4
> +
> +        ctrl = ctrl.with_channel(0xAB);
> +        assert_eq!(ctrl.channel(), 0xAB);
> +
> +        // Test overlapping fields
> +        ctrl = ctrl.with_const_priority_nibble::<0xF>();
> +        assert_eq!(ctrl.priority_nibble(), 0xF);
> +        assert_eq!(ctrl.priority(), Priority::Critical); // bits 5:4 = 0x3
> +
> +        let ctrl2 = TestControlRegister::zeroed()
> +            .with_enable(true)
> +            .with_const_mode::<0x3>()
> +            .with_priority(Priority::Medium)
> +            .with_channel(0x42);
> +
> +        assert!(ctrl2.enable().into_bool());
> +        assert_eq!(ctrl2.mode(), 0x3);
> +        assert_eq!(ctrl2.priority(), Priority::Medium);
> +        assert_eq!(ctrl2.channel(), 0x42);
> +
> +        let raw_value: u16 = 0x4217;
> +        let ctrl3 = TestControlRegister::from_raw(raw_value);
> +        assert_eq!(u16::from(ctrl3), raw_value);
> +        assert!(ctrl3.enable().into_bool());
> +        assert_eq!(ctrl3.priority(), Priority::Medium);
> +        assert_eq!(ctrl3.priority_nibble(), 0x1);
> +        assert_eq!(ctrl3.channel(), 0x42);
> +    }
> +
> +    #[test]
> +    fn test_u8_bitfield() {
> +        let mut status = TestStatusRegister::zeroed();
> +
> +        assert!(!status.ready().into_bool());
> +        assert!(!status.error().into_bool());
> +        assert_eq!(status.state(), 0);
> +        assert_eq!(status.reserved(), 0);
> +        assert_eq!(status.full_byte(), 0);
> +
> +        status = status.with_ready(true);
> +        assert!(status.ready().into_bool());
> +        assert_eq!(status.full_byte(), 0x01);
> +
> +        status = status.with_error(true);
> +        assert!(status.error().into_bool());
> +        assert_eq!(status.full_byte(), 0x03);
> +
> +        status = status.with_const_state::<0x3>();
> +        assert_eq!(status.state(), 0x3);
> +        assert_eq!(status.full_byte(), 0x0F);
> +
> +        status = status.with_const_reserved::<0xA>();
> +        assert_eq!(status.reserved(), 0xA);
> +        assert_eq!(status.full_byte(), 0xAF);
> +
> +        // Test overlapping field
> +        status = status.with_full_byte(0x55);
> +        assert_eq!(status.full_byte(), 0x55);
> +        assert!(status.ready().into_bool());
> +        assert!(!status.error().into_bool());
> +        assert_eq!(status.state(), 0x1);
> +        assert_eq!(status.reserved(), 0x5);
> +
> +        let status2 = TestStatusRegister::zeroed()
> +            .with_ready(true)
> +            .with_const_state::<0x2>()
> +            .with_const_reserved::<0x5>();
> +
> +        assert!(status2.ready().into_bool());
> +        assert!(!status2.error().into_bool());
> +        assert_eq!(status2.state(), 0x2);
> +        assert_eq!(status2.reserved(), 0x5);
> +        assert_eq!(status2.full_byte(), 0x59);
> +
> +        let raw_value: u8 = 0x59;
> +        let status3 = TestStatusRegister::from_raw(raw_value);
> +        assert_eq!(u8::from(status3), raw_value);
> +        assert!(status3.ready().into_bool());
> +        assert!(!status3.error().into_bool());
> +        assert_eq!(status3.state(), 0x2);
> +        assert_eq!(status3.reserved(), 0x5);
> +        assert_eq!(status3.full_byte(), 0x59);
> +
> +        let status4 = TestStatusRegister::from_raw(0xFF);
> +        assert!(status4.ready().into_bool());
> +        assert!(status4.error().into_bool());
> +        assert_eq!(status4.state(), 0x3);
> +        assert_eq!(status4.reserved(), 0xF);
> +        assert_eq!(status4.full_byte(), 0xFF);
> +    }
> +}
> 
> -- 
> 2.54.0