Skip to content

Commit 359a834

Browse files
rmsynromancardenas
rmsyn
and
romancardenas
committed
fixup: riscv: write-only CSR compile-time asserts
Applies compile-time checks in write-only CSR macros. Authored-by: rmsyn <[email protected]> Co-authored-by: romancardenas <[email protected]>
1 parent 7c3bed7 commit 359a834

File tree

4 files changed

+40
-137
lines changed

4 files changed

+40
-137
lines changed

‎riscv/src/register/macros.rs‎

Lines changed: 34 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -749,8 +749,6 @@ macro_rules! read_write_csr_field {
749749
$field:ident,
750750
$(#[$set_field_doc:meta])+
751751
$set_field:ident,
752-
$(#[$try_set_field_doc:meta])+
753-
$try_set_field:ident,
754752
bit: $bit:literal$(,)?
755753
) => {
756754
$crate::read_only_csr_field!(
@@ -762,10 +760,7 @@ macro_rules! read_write_csr_field {
762760
$crate::write_only_csr_field!(
763761
$ty,
764762
$(#[$set_field_doc])+
765-
$set_field,
766-
$(#[$try_set_field_doc])+
767-
$try_set_field,
768-
bit: $bit,
763+
$set_field: $bit,
769764
);
770765
};
771766

@@ -804,8 +799,6 @@ macro_rules! read_write_csr_field {
804799
$field:ident,
805800
$(#[$set_field_doc:meta])+
806801
$set_field:ident,
807-
$(#[$try_set_field_doc:meta])+
808-
$try_set_field:ident,
809802
range: [$bit_start:literal : $bit_end:literal]$(,)?
810803
) => {
811804
$crate::read_only_csr_field!(
@@ -817,10 +810,7 @@ macro_rules! read_write_csr_field {
817810
$crate::write_only_csr_field!(
818811
$ty,
819812
$(#[$set_field_doc])+
820-
$set_field,
821-
$(#[$try_set_field_doc])+
822-
$try_set_field,
823-
range: [$bit_start : $bit_end],
813+
$set_field: [$bit_start : $bit_end],
824814
);
825815
};
826816

@@ -831,8 +821,6 @@ macro_rules! read_write_csr_field {
831821
$try_field:ident,
832822
$(#[$set_field_doc:meta])+
833823
$set_field:ident,
834-
$(#[$try_set_field_doc:meta])+
835-
$try_set_field:ident,
836824
$(#[$field_ty_doc:meta])+
837825
$field_ty:ident {
838826
range: [$field_start:literal : $field_end:literal],
@@ -863,8 +851,6 @@ macro_rules! read_write_csr_field {
863851
$ty,
864852
$(#[$set_field_doc])+
865853
$set_field,
866-
$(#[$try_set_field_doc])+
867-
$try_set_field,
868854
$field_ty,
869855
range: [$field_start : $field_end],
870856
);
@@ -1005,32 +991,14 @@ macro_rules! read_only_csr_field {
1005991
macro_rules! write_only_csr_field {
1006992
($ty:ident,
1007993
$(#[$field_doc:meta])+
1008-
$field:ident,
1009-
$(#[$try_field_doc:meta])+
1010-
$try_field:ident,
1011-
bit: $bit:literal$(,)?) => {
994+
$field:ident: $bit:literal$(,)?) => {
995+
const _: () = assert!($bit < usize::BITS);
996+
1012997
impl $ty {
1013998
$(#[$field_doc])+
1014999
#[inline]
10151000
pub fn $field(&mut self, $field: bool) {
1016-
self.$try_field($field).unwrap();
1017-
}
1018-
1019-
$(#[$try_field_doc])+
1020-
#[inline]
1021-
pub fn $try_field(&mut self, $field: bool) -> $crate::result::Result<()> {
1022-
let max_width = core::mem::size_of_val(&self.bits) * 8;
1023-
1024-
if $bit < max_width {
1025-
self.bits = $crate::bits::bf_insert(self.bits, $bit, 1, $field as usize);
1026-
Ok(())
1027-
} else {
1028-
Err($crate::result::Error::IndexOutOfBounds {
1029-
index: $bit,
1030-
min: 0,
1031-
max: max_width,
1032-
})
1033-
}
1001+
self.bits = $crate::bits::bf_insert(self.bits, $bit, 1, $field as usize);
10341002
}
10351003
}
10361004
};
@@ -1041,6 +1009,9 @@ macro_rules! write_only_csr_field {
10411009
$(#[$try_field_doc:meta])+
10421010
$try_field:ident,
10431011
range: $bit_start:literal..=$bit_end:literal$(,)?) => {
1012+
const _: () = assert!($bit_end < usize::BITS);
1013+
const _: () = assert!($bit_start <= $bit_end);
1014+
10441015
impl $ty {
10451016
$(#[$field_doc])+
10461017
#[inline]
@@ -1051,9 +1022,7 @@ macro_rules! write_only_csr_field {
10511022
$(#[$try_field_doc])+
10521023
#[inline]
10531024
pub fn $try_field(&mut self, index: usize, $field: bool) -> $crate::result::Result<()> {
1054-
let max_width = core::mem::size_of_val(&self.bits) * 8;
1055-
1056-
if index < max_width && ($bit_start..=$bit_end).contains(&index) {
1025+
if ($bit_start..=$bit_end).contains(&index) {
10571026
self.bits = $crate::bits::bf_insert(self.bits, index, 1, $field as usize);
10581027
Ok(())
10591028
} else {
@@ -1069,47 +1038,27 @@ macro_rules! write_only_csr_field {
10691038

10701039
($ty:ident,
10711040
$(#[$field_doc:meta])+
1072-
$field:ident,
1073-
$(#[$try_field_doc:meta])+
1074-
$try_field:ident,
1075-
range: [$bit_start:literal : $bit_end:literal]$(,)?) => {
1041+
$field:ident: [$bit_start:literal : $bit_end:literal]$(,)?) => {
1042+
const _: () = assert!($bit_end < usize::BITS);
1043+
const _: () = assert!($bit_start <= $bit_end);
1044+
10761045
impl $ty {
10771046
$(#[$field_doc])+
10781047
#[inline]
10791048
pub fn $field(&mut self, $field: usize) {
1080-
self.$try_field($field).unwrap();
1081-
}
1082-
1083-
$(#[$try_field_doc])+
1084-
#[inline]
1085-
pub fn $try_field(&mut self, $field: usize) -> $crate::result::Result<()> {
1086-
let max_width = core::mem::size_of_val(&self.bits) * 8;
1087-
1088-
if $bit_start < max_width && $bit_start <= $bit_end {
1089-
self.bits = $crate::bits::bf_insert(
1090-
self.bits,
1091-
$bit_start,
1092-
$bit_end - $bit_start + 1,
1093-
$field,
1094-
);
1095-
1096-
Ok(())
1097-
} else {
1098-
Err($crate::result::Error::IndexOutOfBounds {
1099-
index: $bit_start,
1100-
min: $bit_start,
1101-
max: $bit_end,
1102-
})
1103-
}
1049+
self.bits = $crate::bits::bf_insert(
1050+
self.bits,
1051+
$bit_start,
1052+
$bit_end - $bit_start + 1,
1053+
$field,
1054+
);
11041055
}
11051056
}
11061057
};
11071058

11081059
($ty:ident,
11091060
$(#[$field_doc:meta])+
11101061
$field:ident,
1111-
$(#[$try_field_doc:meta])+
1112-
$try_field:ident,
11131062
$(#[$field_ty_doc:meta])+
11141063
$field_ty:ident {
11151064
range: [$field_start:literal : $field_end:literal],
@@ -1130,8 +1079,6 @@ macro_rules! write_only_csr_field {
11301079
$ty,
11311080
$(#[$field_doc])+
11321081
$field,
1133-
$(#[$try_field_doc])+
1134-
$try_field,
11351082
$field_ty,
11361083
range: [$field_start : $field_end],
11371084
);
@@ -1140,40 +1087,23 @@ macro_rules! write_only_csr_field {
11401087
($ty:ident,
11411088
$(#[$field_doc:meta])+
11421089
$field:ident,
1143-
$(#[$try_field_doc:meta])+
1144-
$try_field:ident,
11451090
$field_ty:ident,
11461091
range: [$field_start:literal : $field_end:literal]$(,)?
11471092
) => {
1148-
impl $ty {
1149-
$(#[$field_doc])+
1150-
#[inline]
1151-
pub fn $field(&mut self, $field: $field_ty) {
1152-
self.$try_field($field).unwrap();
1153-
}
1154-
1155-
$(#[$try_field_doc])+
1156-
#[inline]
1157-
pub fn $try_field(&mut self, $field: $field_ty) -> $crate::result::Result<()> {
1158-
let max_width = core::mem::size_of_val(&self.bits) * 8;
1159-
1160-
if $field_end < max_width && $field_start <= $field_end {
1161-
self.bits = $crate::bits::bf_insert(
1162-
self.bits,
1163-
$field_start,
1164-
$field_end - $field_start + 1,
1165-
$field.into(),
1166-
);
1093+
const _: () = assert!($field_end < usize::BITS);
1094+
const _: () = assert!($field_start <= $field_end);
11671095

1168-
Ok(())
1169-
} else {
1170-
Err($crate::result::Error::IndexOutOfBounds {
1171-
index: $field_start,
1172-
min: $field_start,
1173-
max: $field_end,
1174-
})
1175-
}
1176-
}
1177-
}
1096+
impl $ty {
1097+
$(#[$field_doc])+
1098+
#[inline]
1099+
pub fn $field(&mut self, $field: $field_ty) {
1100+
self.bits = $crate::bits::bf_insert(
1101+
self.bits,
1102+
$field_start,
1103+
$field_end - $field_start + 1,
1104+
$field.into(),
1105+
);
1106+
}
1107+
}
11781108
};
11791109
}

‎riscv/src/register/mcountinhibit.rs‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,6 @@ read_write_csr_field! {
1919
///
2020
/// **NOTE**: only updates the in-memory value without touching the CSR.
2121
set_cy,
22-
/// Attempts to set the `cycle[h]` inhibit field value.
23-
///
24-
/// **NOTE**: only updates the in-memory value without touching the CSR.
25-
try_set_cy,
2622
bit: 0,
2723
}
2824

@@ -34,10 +30,6 @@ read_write_csr_field! {
3430
///
3531
/// **NOTE**: only updates the in-memory value without touching the CSR.
3632
set_ir,
37-
/// Attempts to set the `instret[h]` inhibit field value.
38-
///
39-
/// **NOTE**: only updates the in-memory value without touching the CSR.
40-
try_set_ir,
4133
bit: 2,
4234
}
4335

‎riscv/src/register/tests/read_write_csr.rs‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ read_write_csr_field! {
1212
single,
1313
/// setter test single-bit field
1414
set_single,
15-
/// try-setter test single-bit field
16-
try_set_single,
1715
bit: 0,
1816
}
1917

@@ -36,8 +34,6 @@ read_write_csr_field!(
3634
multi_field,
3735
/// setter multi-bit field
3836
set_multi_field,
39-
/// try-setter multi-bit field
40-
try_set_multi_field,
4137
range: [4:7],
4238
);
4339

@@ -49,11 +45,9 @@ read_write_csr_field!(
4945
try_field_enum,
5046
/// setter multi-bit field
5147
set_field_enum,
52-
/// try-setter multi-bit field
53-
try_set_field_enum,
5448
/// field enum type with valid field variants
5549
MtestFieldEnum {
56-
range: [7:11],
50+
range: [8:11],
5751
default: Field1,
5852
Field1 = 1,
5953
Field2 = 2,
@@ -127,7 +121,7 @@ fn test_mtest_read_write() {
127121
mtest.set_multi_field(0x0);
128122
assert_eq!(mtest.multi_field(), 0x0);
129123

130-
assert_eq!(mtest.try_field_enum(), Err(Error::InvalidVariant(0)),);
124+
assert_eq!(mtest.try_field_enum(), Err(Error::InvalidVariant(0)));
131125

132126
[
133127
MtestFieldEnum::Field1,
@@ -143,6 +137,6 @@ fn test_mtest_read_write() {
143137
});
144138

145139
// check that setting an invalid variant returns `None`
146-
mtest = Mtest::from_bits(0xbad << 7);
147-
assert_eq!(mtest.try_field_enum(), Err(Error::InvalidVariant(13)),);
140+
mtest = Mtest::from_bits(0xbad << 8);
141+
assert_eq!(mtest.try_field_enum(), Err(Error::InvalidVariant(13)));
148142
}

‎riscv/src/register/tests/write_only_csr.rs‎

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,7 @@ write_only_csr! {
99
write_only_csr_field! {
1010
Mtest,
1111
/// setter test single-bit field
12-
set_single,
13-
/// try-setter test single-bit field
14-
try_set_single,
15-
bit: 0,
12+
set_single: 0,
1613
}
1714

1815
write_only_csr_field! {
@@ -27,18 +24,13 @@ write_only_csr_field! {
2724
write_only_csr_field!(
2825
Mtest,
2926
/// setter multi-bit field
30-
set_multi_field,
31-
/// try-setter multi-bit field
32-
try_set_multi_field,
33-
range: [4:7],
27+
set_multi_field: [4:7],
3428
);
3529

3630
write_only_csr_field!(
3731
Mtest,
3832
/// setter multi-bit field
3933
set_field_enum,
40-
/// try-setter multi-bit field
41-
try_set_field_enum,
4234
/// field enum type with valid field variants
4335
MtestFieldEnum {
4436
range: [8:11],
@@ -113,11 +105,6 @@ fn test_mtest_write_only() {
113105
]
114106
.into_iter()
115107
.for_each(|variant| {
116-
assert_eq!(
117-
mtest.try_set_field_enum(variant),
118-
Ok(()),
119-
"field value: {variant:?}"
120-
);
121108
mtest.set_field_enum(variant);
122109
assert_eq!(MtestFieldEnum::from_usize(mtest.bits() >> 8), Ok(variant));
123110
});

0 commit comments

Comments
 (0)