Skip to content

Commit

Permalink
fixup: riscv: write-only CSR compile-time asserts
Browse files Browse the repository at this point in the history 
Applies compile-time checks in write-only CSR macros.

Authored-by: rmsyn <[email protected]>
Co-authored-by: romancardenas <[email protected]>
  • Loading branch information
@rmsyn @romancardenas
rmsyn and romancardenas committed Sep 20, 2024
1 parent 7c3bed7 commit 359a834
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 137 deletions.
138 changes: 34 additions & 104 deletions riscv/src/register/macros.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand All @@ -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,
);
};

Expand Down Expand Up @@ -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!(
Expand All @@ -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],
);
};

Expand All @@ -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],
Expand Down Expand Up @@ -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],
);
Expand Down Expand Up @@ -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);
}
}
};
Expand All @@ -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]
Expand All @@ -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 {
Expand All @@ -1069,47 +1038,27 @@ 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,
);
}
}
};

($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],
Expand All @@ -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],
);
Expand All @@ -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(),
);
}
}
};
}
8 changes: 0 additions & 8 deletions riscv/src/register/mcountinhibit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand All @@ -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,
}

Expand Down
14 changes: 4 additions & 10 deletions riscv/src/register/tests/read_write_csr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand All @@ -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],
);

Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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)));
}
17 changes: 2 additions & 15 deletions riscv/src/register/tests/write_only_csr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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! {
Expand All @@ -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],
Expand Down Expand Up @@ -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));
});
Expand Down

0 comments on commit 359a834

Please sign in to comment.