From 359a834554305494ea8329b3a862d6ba5cdfdd52 Mon Sep 17 00:00:00 2001 From: rmsyn Date: Fri, 20 Sep 2024 02:01:00 +0000 Subject: [PATCH] fixup: riscv: write-only CSR compile-time asserts Applies compile-time checks in write-only CSR macros. Authored-by: rmsyn Co-authored-by: romancardenas --- riscv/src/register/macros.rs | 138 +++++---------------- riscv/src/register/mcountinhibit.rs | 8 -- riscv/src/register/tests/read_write_csr.rs | 14 +-- riscv/src/register/tests/write_only_csr.rs | 17 +-- 4 files changed, 40 insertions(+), 137 deletions(-) diff --git a/riscv/src/register/macros.rs b/riscv/src/register/macros.rs index 293fd525..ff005c26 100644 --- a/riscv/src/register/macros.rs +++ b/riscv/src/register/macros.rs @@ -749,8 +749,6 @@ macro_rules! read_write_csr_field { $field:ident, $(#[$set_field_doc:meta])+ $set_field:ident, - $(#[$try_set_field_doc:meta])+ - $try_set_field:ident, bit: $bit:literal$(,)? ) => { $crate::read_only_csr_field!( @@ -762,10 +760,7 @@ macro_rules! read_write_csr_field { $crate::write_only_csr_field!( $ty, $(#[$set_field_doc])+ - $set_field, - $(#[$try_set_field_doc])+ - $try_set_field, - bit: $bit, + $set_field: $bit, ); }; @@ -804,8 +799,6 @@ macro_rules! read_write_csr_field { $field:ident, $(#[$set_field_doc:meta])+ $set_field:ident, - $(#[$try_set_field_doc:meta])+ - $try_set_field:ident, range: [$bit_start:literal : $bit_end:literal]$(,)? ) => { $crate::read_only_csr_field!( @@ -817,10 +810,7 @@ macro_rules! read_write_csr_field { $crate::write_only_csr_field!( $ty, $(#[$set_field_doc])+ - $set_field, - $(#[$try_set_field_doc])+ - $try_set_field, - range: [$bit_start : $bit_end], + $set_field: [$bit_start : $bit_end], ); }; @@ -831,8 +821,6 @@ macro_rules! read_write_csr_field { $try_field:ident, $(#[$set_field_doc:meta])+ $set_field:ident, - $(#[$try_set_field_doc:meta])+ - $try_set_field:ident, $(#[$field_ty_doc:meta])+ $field_ty:ident { range: [$field_start:literal : $field_end:literal], @@ -863,8 +851,6 @@ macro_rules! read_write_csr_field { $ty, $(#[$set_field_doc])+ $set_field, - $(#[$try_set_field_doc])+ - $try_set_field, $field_ty, range: [$field_start : $field_end], ); @@ -1005,32 +991,14 @@ macro_rules! read_only_csr_field { macro_rules! write_only_csr_field { ($ty:ident, $(#[$field_doc:meta])+ - $field:ident, - $(#[$try_field_doc:meta])+ - $try_field:ident, - bit: $bit:literal$(,)?) => { + $field:ident: $bit:literal$(,)?) => { + const _: () = assert!($bit < usize::BITS); + impl $ty { $(#[$field_doc])+ #[inline] pub fn $field(&mut self, $field: bool) { - self.$try_field($field).unwrap(); - } - - $(#[$try_field_doc])+ - #[inline] - pub fn $try_field(&mut self, $field: bool) -> $crate::result::Result<()> { - let max_width = core::mem::size_of_val(&self.bits) * 8; - - if $bit < max_width { - self.bits = $crate::bits::bf_insert(self.bits, $bit, 1, $field as usize); - Ok(()) - } else { - Err($crate::result::Error::IndexOutOfBounds { - index: $bit, - min: 0, - max: max_width, - }) - } + self.bits = $crate::bits::bf_insert(self.bits, $bit, 1, $field as usize); } } }; @@ -1041,6 +1009,9 @@ macro_rules! write_only_csr_field { $(#[$try_field_doc:meta])+ $try_field:ident, range: $bit_start:literal..=$bit_end:literal$(,)?) => { + const _: () = assert!($bit_end < usize::BITS); + const _: () = assert!($bit_start <= $bit_end); + impl $ty { $(#[$field_doc])+ #[inline] @@ -1051,9 +1022,7 @@ macro_rules! write_only_csr_field { $(#[$try_field_doc])+ #[inline] pub fn $try_field(&mut self, index: usize, $field: bool) -> $crate::result::Result<()> { - let max_width = core::mem::size_of_val(&self.bits) * 8; - - if index < max_width && ($bit_start..=$bit_end).contains(&index) { + if ($bit_start..=$bit_end).contains(&index) { self.bits = $crate::bits::bf_insert(self.bits, index, 1, $field as usize); Ok(()) } else { @@ -1069,38 +1038,20 @@ macro_rules! write_only_csr_field { ($ty:ident, $(#[$field_doc:meta])+ - $field:ident, - $(#[$try_field_doc:meta])+ - $try_field:ident, - range: [$bit_start:literal : $bit_end:literal]$(,)?) => { + $field:ident: [$bit_start:literal : $bit_end:literal]$(,)?) => { + const _: () = assert!($bit_end < usize::BITS); + const _: () = assert!($bit_start <= $bit_end); + impl $ty { $(#[$field_doc])+ #[inline] pub fn $field(&mut self, $field: usize) { - self.$try_field($field).unwrap(); - } - - $(#[$try_field_doc])+ - #[inline] - pub fn $try_field(&mut self, $field: usize) -> $crate::result::Result<()> { - let max_width = core::mem::size_of_val(&self.bits) * 8; - - if $bit_start < max_width && $bit_start <= $bit_end { - self.bits = $crate::bits::bf_insert( - self.bits, - $bit_start, - $bit_end - $bit_start + 1, - $field, - ); - - Ok(()) - } else { - Err($crate::result::Error::IndexOutOfBounds { - index: $bit_start, - min: $bit_start, - max: $bit_end, - }) - } + self.bits = $crate::bits::bf_insert( + self.bits, + $bit_start, + $bit_end - $bit_start + 1, + $field, + ); } } }; @@ -1108,8 +1059,6 @@ macro_rules! write_only_csr_field { ($ty:ident, $(#[$field_doc:meta])+ $field:ident, - $(#[$try_field_doc:meta])+ - $try_field:ident, $(#[$field_ty_doc:meta])+ $field_ty:ident { range: [$field_start:literal : $field_end:literal], @@ -1130,8 +1079,6 @@ macro_rules! write_only_csr_field { $ty, $(#[$field_doc])+ $field, - $(#[$try_field_doc])+ - $try_field, $field_ty, range: [$field_start : $field_end], ); @@ -1140,40 +1087,23 @@ macro_rules! write_only_csr_field { ($ty:ident, $(#[$field_doc:meta])+ $field:ident, - $(#[$try_field_doc:meta])+ - $try_field:ident, $field_ty:ident, range: [$field_start:literal : $field_end:literal]$(,)? ) => { - impl $ty { - $(#[$field_doc])+ - #[inline] - pub fn $field(&mut self, $field: $field_ty) { - self.$try_field($field).unwrap(); - } - - $(#[$try_field_doc])+ - #[inline] - pub fn $try_field(&mut self, $field: $field_ty) -> $crate::result::Result<()> { - let max_width = core::mem::size_of_val(&self.bits) * 8; - - if $field_end < max_width && $field_start <= $field_end { - self.bits = $crate::bits::bf_insert( - self.bits, - $field_start, - $field_end - $field_start + 1, - $field.into(), - ); + const _: () = assert!($field_end < usize::BITS); + const _: () = assert!($field_start <= $field_end); - Ok(()) - } else { - Err($crate::result::Error::IndexOutOfBounds { - index: $field_start, - min: $field_start, - max: $field_end, - }) - } - } - } + impl $ty { + $(#[$field_doc])+ + #[inline] + pub fn $field(&mut self, $field: $field_ty) { + self.bits = $crate::bits::bf_insert( + self.bits, + $field_start, + $field_end - $field_start + 1, + $field.into(), + ); + } + } }; } diff --git a/riscv/src/register/mcountinhibit.rs b/riscv/src/register/mcountinhibit.rs index 4ad8d0e0..1ec86285 100644 --- a/riscv/src/register/mcountinhibit.rs +++ b/riscv/src/register/mcountinhibit.rs @@ -19,10 +19,6 @@ read_write_csr_field! { /// /// **NOTE**: only updates the in-memory value without touching the CSR. set_cy, - /// Attempts to set the `cycle[h]` inhibit field value. - /// - /// **NOTE**: only updates the in-memory value without touching the CSR. - try_set_cy, bit: 0, } @@ -34,10 +30,6 @@ read_write_csr_field! { /// /// **NOTE**: only updates the in-memory value without touching the CSR. set_ir, - /// Attempts to set the `instret[h]` inhibit field value. - /// - /// **NOTE**: only updates the in-memory value without touching the CSR. - try_set_ir, bit: 2, } diff --git a/riscv/src/register/tests/read_write_csr.rs b/riscv/src/register/tests/read_write_csr.rs index e8b2fe56..c3981afa 100644 --- a/riscv/src/register/tests/read_write_csr.rs +++ b/riscv/src/register/tests/read_write_csr.rs @@ -12,8 +12,6 @@ read_write_csr_field! { single, /// setter test single-bit field set_single, - /// try-setter test single-bit field - try_set_single, bit: 0, } @@ -36,8 +34,6 @@ read_write_csr_field!( multi_field, /// setter multi-bit field set_multi_field, - /// try-setter multi-bit field - try_set_multi_field, range: [4:7], ); @@ -49,11 +45,9 @@ read_write_csr_field!( try_field_enum, /// setter multi-bit field set_field_enum, - /// try-setter multi-bit field - try_set_field_enum, /// field enum type with valid field variants MtestFieldEnum { - range: [7:11], + range: [8:11], default: Field1, Field1 = 1, Field2 = 2, @@ -127,7 +121,7 @@ fn test_mtest_read_write() { mtest.set_multi_field(0x0); assert_eq!(mtest.multi_field(), 0x0); - assert_eq!(mtest.try_field_enum(), Err(Error::InvalidVariant(0)),); + assert_eq!(mtest.try_field_enum(), Err(Error::InvalidVariant(0))); [ MtestFieldEnum::Field1, @@ -143,6 +137,6 @@ fn test_mtest_read_write() { }); // check that setting an invalid variant returns `None` - mtest = Mtest::from_bits(0xbad << 7); - assert_eq!(mtest.try_field_enum(), Err(Error::InvalidVariant(13)),); + mtest = Mtest::from_bits(0xbad << 8); + assert_eq!(mtest.try_field_enum(), Err(Error::InvalidVariant(13))); } diff --git a/riscv/src/register/tests/write_only_csr.rs b/riscv/src/register/tests/write_only_csr.rs index 2e7da165..73de0d0a 100644 --- a/riscv/src/register/tests/write_only_csr.rs +++ b/riscv/src/register/tests/write_only_csr.rs @@ -9,10 +9,7 @@ write_only_csr! { write_only_csr_field! { Mtest, /// setter test single-bit field - set_single, - /// try-setter test single-bit field - try_set_single, - bit: 0, + set_single: 0, } write_only_csr_field! { @@ -27,18 +24,13 @@ write_only_csr_field! { write_only_csr_field!( Mtest, /// setter multi-bit field - set_multi_field, - /// try-setter multi-bit field - try_set_multi_field, - range: [4:7], + set_multi_field: [4:7], ); write_only_csr_field!( Mtest, /// setter multi-bit field set_field_enum, - /// try-setter multi-bit field - try_set_field_enum, /// field enum type with valid field variants MtestFieldEnum { range: [8:11], @@ -113,11 +105,6 @@ fn test_mtest_write_only() { ] .into_iter() .for_each(|variant| { - assert_eq!( - mtest.try_set_field_enum(variant), - Ok(()), - "field value: {variant:?}" - ); mtest.set_field_enum(variant); assert_eq!(MtestFieldEnum::from_usize(mtest.bits() >> 8), Ok(variant)); });