| 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
| ||
|
Message-Id: <DDB69ONCWBEJ.1KGWC6H2VG2CE@nvidia.com>
Date: Mon, 06 Oct 2025 19:37:34 +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 v6 5/5] rust: bitfield: Add KUNIT tests for bitfield
On Sat Oct 4, 2025 at 12:47 AM JST, Joel Fernandes wrote:
> Add KUNIT tests to make sure the macro is working correctly.
>
> Reviewed-by: Alexandre Courbot <acourbot@...dia.com>
> Signed-off-by: Joel Fernandes <joelagnelf@...dia.com>
> ---
> rust/kernel/bitfield.rs | 323 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 323 insertions(+)
>
> diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
> index 09cd5741598c..f0e341a1a979 100644
> --- a/rust/kernel/bitfield.rs
> +++ b/rust/kernel/bitfield.rs
> @@ -329,3 +329,326 @@ fn default() -> Self {
> }
> };
> }
> +
> +#[::kernel::macros::kunit_tests(kernel_bitfield)]
> +mod tests {
> + use core::convert::TryFrom;
> +
> + // Enum types for testing => and ?=> conversions
> + #[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, Default, Clone, Copy, PartialEq)]
> + enum Priority {
> + #[default]
> + Low = 0,
> + Medium = 1,
> + High = 2,
> + Critical = 3,
> + }
> +
> + 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;
Is the overlap with `mem_type` and `extended_type` on purpose? It looks
strange to me that the PFN also includes the memory type.
> + 51:12 pfn_overlap as u64;
If `pfn` needs to be adjusted then I guess this one also needs to be.
> + 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());
> + assert_eq!(u64::from(pte), 0x0);
> +
> + pte = pte.set_present(true);
> + assert!(pte.present());
> + assert_eq!(u64::from(pte), 0x1);
> +
> + pte = pte.set_writable(true);
> + assert!(pte.writable());
> + assert_eq!(u64::from(pte), 0x3);
> +
> + pte = pte.set_writable(false);
> + assert!(!pte.writable());
> + assert_eq!(u64::from(pte), 0x1);
> +
> + assert_eq!(pte.available(), 0);
> + pte = pte.set_available(0x5);
> + assert_eq!(pte.available(), 0x5);
> + assert_eq!(u64::from(pte), 0xA01);
> + }
> +
> + #[test]
> + fn test_range_fields() {
> + let mut pte = TestPageTableEntry::default();
> + assert_eq!(u64::from(pte), 0x0);
> +
> + pte = pte.set_pfn(0x123456);
> + assert_eq!(pte.pfn(), 0x123456);
> + // Test overlapping field reads same value
> + assert_eq!(pte.pfn_overlap(), 0x123456);
> + assert_eq!(u64::from(pte), 0x123456000);
> +
> + pte = pte.set_available(0x7);
> + assert_eq!(pte.available(), 0x7);
> + assert_eq!(u64::from(pte), 0x123456E00);
> +
> + pte = pte.set_available2(0x3FF);
> + assert_eq!(pte.available2(), 0x3FF);
> + assert_eq!(u64::from(pte), 0x3FF0000123456E00);
> +
> + // Test TryFrom with ?=> for MemoryType
> + pte = pte.set_mem_type(MemoryType::Device);
> + assert_eq!(pte.mem_type(), Ok(MemoryType::Device));
> + assert_eq!(u64::from(pte), 0x3FF0000123456E00);
> +
> + pte = pte.set_mem_type(MemoryType::Normal);
> + assert_eq!(pte.mem_type(), Ok(MemoryType::Normal));
> + assert_eq!(u64::from(pte), 0x3FF0000123455E00);
> +
> + // Test all valid values for mem_type
> + pte = pte.set_mem_type(MemoryType::Reserved);
> + assert_eq!(pte.mem_type(), Ok(MemoryType::Reserved));
> + assert_eq!(u64::from(pte), 0x3FF0000123457E00);
> +
> + // Test failure case using extended_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();
> + // Set bits 17:14 to 7 (invalid for MemoryType)
> + raw = (raw & !::kernel::bits::genmask_u64(14..=17)) | (0x7 << 14);
> + let invalid_pte = TestPageTableEntry(raw);
> + // Should return Err with the invalid value
> + assert_eq!(invalid_pte.extended_type(), Err(0x7));
> +
> + // Test a valid value after testing invalid to ensure both cases work
> + // Set bits 17:14 to 2 (valid: Device)
> + raw = (raw & !::kernel::bits::genmask_u64(14..=17)) | (0x2 << 14);
> + let valid_pte = TestPageTableEntry(raw);
> + assert_eq!(valid_pte.extended_type(), Ok(MemoryType::Device));
> +
> + let max_pfn = ::kernel::bits::genmask_u64(0..=39);
> + pte = pte.set_pfn(max_pfn);
> + assert_eq!(pte.pfn(), max_pfn);
> + assert_eq!(pte.pfn_overlap(), max_pfn);
> + }
> +
> + #[test]
> + fn test_builder_pattern() {
> + let pte = TestPageTableEntry::default()
> + .set_present(true)
> + .set_writable(true)
> + .set_available(0x7)
> + .set_pfn(0xABCDEF)
> + .set_mem_type(MemoryType::Reserved)
> + .set_available2(0x3FF);
> +
> + assert!(pte.present());
> + assert!(pte.writable());
> + assert_eq!(pte.available(), 0x7);
> + assert_eq!(pte.pfn(), 0xABCDEF);
> + assert_eq!(pte.pfn_overlap(), 0xABCDEF);
> + assert_eq!(pte.mem_type(), Ok(MemoryType::Reserved));
> + assert_eq!(pte.available2(), 0x3FF);
Maybe check the raw value here as well, although I guess the previous
test already covered this anyway.
With these points confirmed,
Reviewed-by: Alexandre Courbot <acourbot@...dia.com>
Powered by blists - more mailing lists