lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <DD7GCYCZU3P3.1KK174S7MQ5BW@nvidia.com>
Date: Thu, 02 Oct 2025 10:41:22 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Joel Fernandes" <joelagnelf@...dia.com>,
 <linux-kernel@...r.kernel.org>, <rust-for-linux@...r.kernel.org>,
 <dri-devel@...ts.freedesktop.org>, <dakr@...nel.org>, <acourbot@...dia.com>
Cc: "Alistair Popple" <apopple@...dia.com>, "Miguel Ojeda"
 <ojeda@...nel.org>, "Alex Gaynor" <alex.gaynor@...il.com>, "Boqun Feng"
 <boqun.feng@...il.com>, "Gary Guo" <gary@...yguo.net>,
 <bjorn3_gh@...tonmail.com>, "Benno Lossin" <lossin@...nel.org>, "Andreas
 Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>,
 "Trevor Gross" <tmgross@...ch.edu>, "David Airlie" <airlied@...il.com>,
 "Simona Vetter" <simona@...ll.ch>, "Maarten Lankhorst"
 <maarten.lankhorst@...ux.intel.com>, "Maxime Ripard" <mripard@...nel.org>,
 "Thomas Zimmermann" <tzimmermann@...e.de>, "John Hubbard"
 <jhubbard@...dia.com>, "Timur Tabi" <ttabi@...dia.com>,
 <joel@...lfernandes.org>, "Elle Rhumsaa" <elle@...thered-steel.dev>, "Yury
 Norov" <yury.norov@...il.com>, "Daniel Almeida"
 <daniel.almeida@...labora.com>, "Andrea Righi" <arighi@...dia.com>,
 <nouveau@...ts.freedesktop.org>
Subject: Re: [PATCH v5 6/9] rust: bitfield: Add KUNIT tests for bitfield

On Tue Sep 30, 2025 at 11:45 PM JST, Joel Fernandes wrote:
> Add KUNIT tests to make sure the macro is working correctly.
>
> Signed-off-by: Joel Fernandes <joelagnelf@...dia.com>
> ---
>  rust/kernel/bitfield.rs | 321 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 321 insertions(+)
>
> diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
> index fed19918c3b9..9a20bcd2eb60 100644
> --- a/rust/kernel/bitfield.rs
> +++ b/rust/kernel/bitfield.rs
> @@ -402,3 +402,324 @@ fn default() -> Self {
>          }
>      };
>  }
> +
> +#[::kernel::macros::kunit_tests(kernel_bitfield)]
> +mod tests {
> +    use core::convert::TryFrom;
> +
> +    // Enum types for testing => and ?=> conversions
> +    #[derive(Debug, Clone, Copy, PartialEq)]
> +    enum MemoryType {
> +        Unmapped = 0,
> +        Normal = 1,
> +        Device = 2,
> +        Reserved = 3,
> +    }
> +
> +    impl Default for MemoryType {
> +        fn default() -> Self {
> +            MemoryType::Unmapped
> +        }
> +    }

Tip: you can add `Default` to the `#[derive]` marker of `MemoryType` and
mark the variant you want as default with `#[default]` instead of
providing a full impl block:

    #[derive(Debug, Default, Clone, Copy, PartialEq)]
    enum MemoryType {
        #[default]
        Unmapped = 0,
        Normal = 1,
        Device = 2,
        Reserved = 3,
    }

> +
> +    impl TryFrom<u8> for MemoryType {
> +        type Error = u8;
> +        fn try_from(value: u8) -> Result<Self, Self::Error> {
> +            match value {
> +                0 => Ok(MemoryType::Unmapped),
> +                1 => Ok(MemoryType::Normal),
> +                2 => Ok(MemoryType::Device),
> +                3 => Ok(MemoryType::Reserved),
> +                _ => Err(value),
> +            }
> +        }
> +    }
> +
> +    impl From<MemoryType> for u64 {
> +        fn from(mt: MemoryType) -> u64 {
> +            mt as u64
> +        }
> +    }
> +
> +    #[derive(Debug, Clone, Copy, PartialEq)]
> +    enum Priority {
> +        Low = 0,
> +        Medium = 1,
> +        High = 2,
> +        Critical = 3,
> +    }
> +
> +    impl Default for Priority {
> +        fn default() -> Self {
> +            Priority::Low
> +        }
> +    }
> +
> +    impl From<u8> for Priority {
> +        fn from(value: u8) -> Self {
> +            match value & 0x3 {
> +                0 => Priority::Low,
> +                1 => Priority::Medium,
> +                2 => Priority::High,
> +                _ => Priority::Critical,
> +            }
> +        }
> +    }
> +
> +    impl From<Priority> for u16 {
> +        fn from(p: Priority) -> u16 {
> +            p as u16
> +        }
> +    }
> +
> +    bitfield! {
> +        struct TestPageTableEntry(u64) {
> +            0:0       present     as bool;
> +            1:1       writable    as bool;
> +            11:9      available   as u8;
> +            13:12     mem_type    as u8 ?=> MemoryType;
> +            17:14     extended_type as u8 ?=> MemoryType;  // For testing failures
> +            51:12     pfn         as u64;
> +            51:12     pfn_overlap as u64;
> +            61:52     available2  as u16;
> +        }
> +    }
> +
> +    bitfield! {
> +        struct TestControlRegister(u16) {
> +            0:0       enable      as bool;
> +            3:1       mode        as u8;
> +            5:4       priority    as u8 => Priority;
> +            7:4       priority_nibble as u8;
> +            15:8      channel     as u8;
> +        }
> +    }
> +
> +    bitfield! {
> +        struct TestStatusRegister(u8) {
> +            0:0       ready       as bool;
> +            1:1       error       as bool;
> +            3:2       state       as u8;
> +            7:4       reserved    as u8;
> +            7:0       full_byte   as u8;  // For entire register
> +        }
> +    }
> +
> +    #[test]
> +    fn test_single_bits() {
> +        let mut pte = TestPageTableEntry::default();
> +
> +        assert!(!pte.present());
> +        assert!(!pte.writable());
> +
> +        pte = pte.set_present(true);
> +        assert!(pte.present());
> +
> +        pte = pte.set_writable(true);
> +        assert!(pte.writable());
> +
> +        pte = pte.set_writable(false);
> +        assert!(!pte.writable());
> +
> +        assert_eq!(pte.available(), 0);
> +        pte = pte.set_available(0x5);
> +        assert_eq!(pte.available(), 0x5);

I'd suggest testing the actual raw value of the register on top of
invoking the getter. That way you also test that:

- The right field is actually written (i.e. if the offset is off by one,
  the getter will return the expected result even though the bitfield
  has the wrong value),
- No other field has been affected.

So something like:

    pte = pte.set_present(true);
    assert!(pte.present());
    assert(pte.into(), 0x1u64);

    pte = pte.set_writable(true);
    assert!(pte.writable());
    assert(pte.into(), 0x3u64);

It might look a bit gross, but it is ok since these are not doctests
that users are going to take as a reference, so we case improve test
coverage at the detriment of readability.


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ