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

[Alexandre Courbot]

On Thu Apr 16, 2026 at 11:44 AM JST, Yury Norov wrote:
> On Thu, Apr 09, 2026 at 11:58:48PM +0900, Alexandre Courbot wrote:
>> From: Joel Fernandes <joelagnelf@xxxxxxxxxx>
>> 
>> Add KUNIT tests to make sure the macro is working correctly.
>> 
>> Signed-off-by: Joel Fernandes <joelagnelf@xxxxxxxxxx>
>> [acourbot: update code to latest bitfield! macro.]
>> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
>> ---
>>  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.

Adding a KConfig option as suggested by Alice.

>
>> +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;
>             ...

Indeed - done.

>
>> +        }
>> +    }
>> +
>> +    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?

Added a paragraph with a test in the doccomment.

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

Signed fields would be tricky to support properly. For now it is better
to assume that storage types and fields are all unsigned, and fields
being signed would require explicit handling, through e.g. the `=>`
conversion operator.