Re: [PATCH v2 2/3] rust: bitfield: Add KUNIT tests for bitfield

[Yury Norov <ynorov@nvidia.com>]

On Thu, Apr 09, 2026 at 11:58:48PM +0900, Alexandre Courbot wrote:
> From: Joel Fernandes <joelagnelf@nvidia.com>
> 
> Add KUNIT tests to make sure the macro is working correctly.
> 
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> [acourbot: update code to latest bitfield! macro.]
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  rust/kernel/bitfield.rs | 318 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 318 insertions(+)
> 
> diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
> index f5948eec8a76..9ab8dafff36c 100644
> --- a/rust/kernel/bitfield.rs
> +++ b/rust/kernel/bitfield.rs
> @@ -489,3 +489,321 @@ fn fmt(&self, f: &mut ::kernel::fmt::Formatter<'_>) -> ::kernel::fmt::Result {
>          }
>      };
>  }
> +
> +#[::kernel::macros::kunit_tests(kernel_bitfield)]

Reading the Documentation/rust/testing.rst, and can't understand how
should one configure it to _not_ build the tests? This should be the
default behavior.

There's half a dozen already existing unit tests, and I didn't find
a mechanism to individually control them.

> +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()),
> +            }
> +        }
> +    }
> +
> +    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) {
> +            0:0       present;
> +            1:1       writable;
> +            11:9      available;
> +            15:12     mem_type ?=> MemoryType;
> +            51:16     pfn;
> +            61:52     available2;

Nit:

In the first patch you keep the high-to-low order, can you do the same
here:

            61:52     available2;
            51:16     pfn;
            15:12     mem_type ?=> MemoryType;
            ...

> +        }
> +    }
> +
> +    bitfield! {
> +        struct TestControlRegister(u16) {
> +            0:0       enable;
> +            3:1       mode;
> +            5:4       priority => Priority;
> +            7:4       priority_nibble;
> +            15:8      channel;
> +        }
> +    }
> +
> +    bitfield! {
> +        struct TestStatusRegister(u8) {
> +            0:0       ready;
> +            1:1       error;
> +            3:2       state;
> +            7:4       reserved;
> +            7:0       full_byte;  // For entire register
> +        }
> +    }
> +
> +    #[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>();
> +
> +        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 using direct constructor syntax TestStruct(value)
> +        let pte2 = TestPageTableEntry::from_raw(raw_value);
> +        assert_eq!(u64::from(pte2), raw_value);

So what about bits not belonging to any field, like 8:2 in this case?
Are they RAZ, undef or not touched by any method except the raw
setters?

Can you discuss and add a test for it?

> +    }
> +
> +    #[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);

Does it support signed fields? I recall there was a discussion about it.
If yes, can you add a test? The underlying Bounded does, so I think the
bitfield should do so. Can I save -EFAULT in a 16-bit IMM field? This is
what EX_TYPE_EFAULT_REG does in arch/x86/include/asm/extable_fixup_types.h.

Thanks,
Yury

> +    }
> +}
> 
> -- 
> 2.53.0