Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 5e1fdf8

Browse files
Remove unsafe from certain IRQ related code
While turning IRQs on or off is something that "sounds critical", it is not unsafe in these sense of compromising memory safety. Rust's unsafe should be about memory safety only, hence removing it from certain functions.
1 parent f222d73 commit 5e1fdf8

File tree

30 files changed

+246
-370
lines changed

30 files changed

+246
-370
lines changed

‎13_exceptions_part2_peripheral_IRQs/README.md

Lines changed: 35 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -421,13 +421,9 @@ core are masked before the `f(data)` is being executed, and restored afterwards:
421421
/// previous state before returning, so this is deemed safe.
422422
#[inline(always)]
423423
pub fn exec_with_irq_masked<T>(f: impl FnOnce() -> T) -> T {
424-
let ret: T;
425-
426-
unsafe {
427-
let saved = local_irq_mask_save();
428-
ret = f();
429-
local_irq_restore(saved);
430-
}
424+
let saved = local_irq_mask_save();
425+
let ret = f();
426+
local_irq_restore(saved);
431427

432428
ret
433429
}
@@ -814,7 +810,7 @@ diff -uNr 12_integrated_testing/kernel/src/_arch/aarch64/exception/asynchronous.
814810
trait DaifField {
815811
fn daif_field() -> tock_registers::fields::Field<u64, DAIF::Register>;
816812
}
817-
@@ -66,6 +71,71 @@
813+
@@ -66,6 +71,60 @@
818814
// Public Code
819815
//--------------------------------------------------------------------------------------------------
820816

@@ -830,42 +826,32 @@ diff -uNr 12_integrated_testing/kernel/src/_arch/aarch64/exception/asynchronous.
830826
+///
831827
+/// "Writes to PSTATE.{PAN, D, A, I, F} occur in program order without the need for additional
832828
+/// synchronization."
833-
+///
834-
+/// # Safety
835-
+///
836-
+/// - Changes the HW state of the executing core.
837829
+#[inline(always)]
838-
+pub unsafe fn local_irq_unmask() {
839-
+ #[rustfmt::skip]
840-
+ asm!(
841-
+ "msr DAIFClr, {arg}",
842-
+ arg = const daif_bits::IRQ,
843-
+ options(nomem, nostack, preserves_flags)
844-
+ );
830+
+pub fn local_irq_unmask() {
831+
+ unsafe {
832+
+ asm!(
833+
+ "msr DAIFClr, {arg}",
834+
+ arg = const daif_bits::IRQ,
835+
+ options(nomem, nostack, preserves_flags)
836+
+ );
837+
+ }
845838
+}
846839
+
847840
+/// Mask IRQs on the executing core.
848-
+///
849-
+/// # Safety
850-
+///
851-
+/// - Changes the HW state of the executing core.
852841
+#[inline(always)]
853-
+pub unsafe fn local_irq_mask() {
854-
+ #[rustfmt::skip]
855-
+ asm!(
856-
+ "msr DAIFSet, {arg}",
857-
+ arg = const daif_bits::IRQ,
858-
+ options(nomem, nostack, preserves_flags)
859-
+ );
842+
+pub fn local_irq_mask() {
843+
+ unsafe {
844+
+ asm!(
845+
+ "msr DAIFSet, {arg}",
846+
+ arg = const daif_bits::IRQ,
847+
+ options(nomem, nostack, preserves_flags)
848+
+ );
849+
+ }
860850
+}
861851
+
862852
+/// Mask IRQs on the executing core and return the previously saved interrupt mask bits (DAIF).
863-
+///
864-
+/// # Safety
865-
+///
866-
+/// - Changes the HW state of the executing core.
867853
+#[inline(always)]
868-
+pub unsafe fn local_irq_mask_save() -> u64 {
854+
+pub fn local_irq_mask_save() -> u64 {
869855
+ let saved = DAIF.get();
870856
+ local_irq_mask();
871857
+
@@ -874,12 +860,11 @@ diff -uNr 12_integrated_testing/kernel/src/_arch/aarch64/exception/asynchronous.
874860
+
875861
+/// Restore the interrupt mask bits (DAIF) using the callee's argument.
876862
+///
877-
+/// # Safety
863+
+/// # Invariant
878864
+///
879-
+/// - Changes the HW state of the executing core.
880865
+/// - No sanity checks on the input.
881866
+#[inline(always)]
882-
+pub unsafe fn local_irq_restore(saved: u64) {
867+
+pub fn local_irq_restore(saved: u64) {
883868
+ DAIF.set(saved);
884869
+}
885870
+
@@ -2245,7 +2230,7 @@ diff -uNr 12_integrated_testing/kernel/src/driver.rs 13_exceptions_part2_periphe
22452230
diff -uNr 12_integrated_testing/kernel/src/exception/asynchronous.rs 13_exceptions_part2_peripheral_IRQs/kernel/src/exception/asynchronous.rs
22462231
--- 12_integrated_testing/kernel/src/exception/asynchronous.rs
22472232
+++ 13_exceptions_part2_peripheral_IRQs/kernel/src/exception/asynchronous.rs
2248-
@@ -8,7 +8,153 @@
2233+
@@ -8,7 +8,149 @@
22492234
#[path = "../_arch/aarch64/exception/asynchronous.rs"]
22502235
mod arch_asynchronous;
22512236

@@ -2383,13 +2368,9 @@ diff -uNr 12_integrated_testing/kernel/src/exception/asynchronous.rs 13_exceptio
23832368
+/// previous state before returning, so this is deemed safe.
23842369
+#[inline(always)]
23852370
+pub fn exec_with_irq_masked<T>(f: impl FnOnce() -> T) -> T {
2386-
+ let ret: T;
2387-
+
2388-
+ unsafe {
2389-
+ let saved = local_irq_mask_save();
2390-
+ ret = f();
2391-
+ local_irq_restore(saved);
2392-
+ }
2371+
+ let saved = local_irq_mask_save();
2372+
+ let ret = f();
2373+
+ local_irq_restore(saved);
23932374
+
23942375
+ ret
23952376
+}
@@ -2497,7 +2478,7 @@ diff -uNr 12_integrated_testing/kernel/src/panic_wait.rs 13_exceptions_part2_per
24972478
fn panic(info: &PanicInfo) -> ! {
24982479
use crate::time::interface::TimeManager;
24992480

2500-
+ unsafe { exception::asynchronous::local_irq_mask() };
2481+
+ exception::asynchronous::local_irq_mask();
25012482
+
25022483
// Protect against panic infinite loops if any of the following code panics itself.
25032484
panic_prevent_reenter();
@@ -2773,21 +2754,21 @@ diff -uNr 12_integrated_testing/kernel/tests/04_exception_irq_sanity.rs 13_excep
27732754
+ // Precondition: IRQs are unmasked.
27742755
+ assert!(exception::asynchronous::is_local_irq_masked());
27752756
+
2776-
+ unsafe { exception::asynchronous::local_irq_mask() };
2757+
+ exception::asynchronous::local_irq_mask();
27772758
+ assert!(!exception::asynchronous::is_local_irq_masked());
27782759
+
27792760
+ // Restore earlier state.
2780-
+ unsafe { exception::asynchronous::local_irq_unmask() };
2761+
+ exception::asynchronous::local_irq_unmask();
27812762
+}
27822763
+
27832764
+/// Check that IRQ unmasking works.
27842765
+#[kernel_test]
27852766
+fn local_irq_unmask_works() {
27862767
+ // Precondition: IRQs are masked.
2787-
+ unsafe { exception::asynchronous::local_irq_mask() };
2768+
+ exception::asynchronous::local_irq_mask();
27882769
+ assert!(!exception::asynchronous::is_local_irq_masked());
27892770
+
2790-
+ unsafe { exception::asynchronous::local_irq_unmask() };
2771+
+ exception::asynchronous::local_irq_unmask();
27912772
+ assert!(exception::asynchronous::is_local_irq_masked());
27922773
+}
27932774
+
@@ -2797,13 +2778,13 @@ diff -uNr 12_integrated_testing/kernel/tests/04_exception_irq_sanity.rs 13_excep
27972778
+ // Precondition: IRQs are unmasked.
27982779
+ assert!(exception::asynchronous::is_local_irq_masked());
27992780
+
2800-
+ let first = unsafe { exception::asynchronous::local_irq_mask_save() };
2781+
+ let first = exception::asynchronous::local_irq_mask_save();
28012782
+ assert!(!exception::asynchronous::is_local_irq_masked());
28022783
+
2803-
+ let second = unsafe { exception::asynchronous::local_irq_mask_save() };
2784+
+ let second = exception::asynchronous::local_irq_mask_save();
28042785
+ assert_ne!(first, second);
28052786
+
2806-
+ unsafe { exception::asynchronous::local_irq_restore(first) };
2787+
+ exception::asynchronous::local_irq_restore(first);
28072788
+ assert!(exception::asynchronous::is_local_irq_masked());
28082789
+}
28092790

‎13_exceptions_part2_peripheral_IRQs/kernel/src/_arch/aarch64/exception/asynchronous.rs

Lines changed: 19 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -83,42 +83,32 @@ pub fn is_local_irq_masked() -> bool {
8383
///
8484
/// "Writes to PSTATE.{PAN, D, A, I, F} occur in program order without the need for additional
8585
/// synchronization."
86-
///
87-
/// # Safety
88-
///
89-
/// - Changes the HW state of the executing core.
9086
#[inline(always)]
91-
pub unsafe fn local_irq_unmask() {
92-
#[rustfmt::skip]
93-
asm!(
94-
"msr DAIFClr, {arg}",
95-
arg = const daif_bits::IRQ,
96-
options(nomem, nostack, preserves_flags)
97-
);
87+
pub fn local_irq_unmask() {
88+
unsafe {
89+
asm!(
90+
"msr DAIFClr, {arg}",
91+
arg = const daif_bits::IRQ,
92+
options(nomem, nostack, preserves_flags)
93+
);
94+
}
9895
}
9996

10097
/// Mask IRQs on the executing core.
101-
///
102-
/// # Safety
103-
///
104-
/// - Changes the HW state of the executing core.
10598
#[inline(always)]
106-
pub unsafe fn local_irq_mask() {
107-
#[rustfmt::skip]
108-
asm!(
109-
"msr DAIFSet, {arg}",
110-
arg = const daif_bits::IRQ,
111-
options(nomem, nostack, preserves_flags)
112-
);
99+
pub fn local_irq_mask() {
100+
unsafe {
101+
asm!(
102+
"msr DAIFSet, {arg}",
103+
arg = const daif_bits::IRQ,
104+
options(nomem, nostack, preserves_flags)
105+
);
106+
}
113107
}
114108

115109
/// Mask IRQs on the executing core and return the previously saved interrupt mask bits (DAIF).
116-
///
117-
/// # Safety
118-
///
119-
/// - Changes the HW state of the executing core.
120110
#[inline(always)]
121-
pub unsafefn local_irq_mask_save() -> u64 {
111+
pub fn local_irq_mask_save() -> u64 {
122112
let saved = DAIF.get();
123113
local_irq_mask();
124114

@@ -127,12 +117,11 @@ pub unsafe fn local_irq_mask_save() -> u64 {
127117

128118
/// Restore the interrupt mask bits (DAIF) using the callee's argument.
129119
///
130-
/// # Safety
120+
/// # Invariant
131121
///
132-
/// - Changes the HW state of the executing core.
133122
/// - No sanity checks on the input.
134123
#[inline(always)]
135-
pub unsafefn local_irq_restore(saved: u64) {
124+
pub fn local_irq_restore(saved: u64) {
136125
DAIF.set(saved);
137126
}
138127

‎13_exceptions_part2_peripheral_IRQs/kernel/src/exception/asynchronous.rs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -141,13 +141,9 @@ impl<const MAX_INCLUSIVE: usize> fmt::Display for IRQNumber<{ MAX_INCLUSIVE }> {
141141
/// previous state before returning, so this is deemed safe.
142142
#[inline(always)]
143143
pub fn exec_with_irq_masked<T>(f: impl FnOnce() -> T) -> T {
144-
let ret: T;
145-
146-
unsafe {
147-
let saved = local_irq_mask_save();
148-
ret = f();
149-
local_irq_restore(saved);
150-
}
144+
let saved = local_irq_mask_save();
145+
let ret = f();
146+
local_irq_restore(saved);
151147

152148
ret
153149
}

‎13_exceptions_part2_peripheral_IRQs/kernel/src/panic_wait.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ fn panic_prevent_reenter() {
6161
fn panic(info: &PanicInfo) -> ! {
6262
use crate::time::interface::TimeManager;
6363

64-
unsafe{exception::asynchronous::local_irq_mask()};
64+
exception::asynchronous::local_irq_mask();
6565

6666
// Protect against panic infinite loops if any of the following code panics itself.
6767
panic_prevent_reenter();

‎13_exceptions_part2_peripheral_IRQs/kernel/tests/04_exception_irq_sanity.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,21 +32,21 @@ fn local_irq_mask_works() {
3232
// Precondition: IRQs are unmasked.
3333
assert!(exception::asynchronous::is_local_irq_masked());
3434

35-
unsafe{exception::asynchronous::local_irq_mask()};
35+
exception::asynchronous::local_irq_mask();
3636
assert!(!exception::asynchronous::is_local_irq_masked());
3737

3838
// Restore earlier state.
39-
unsafe{exception::asynchronous::local_irq_unmask()};
39+
exception::asynchronous::local_irq_unmask();
4040
}
4141

4242
/// Check that IRQ unmasking works.
4343
#[kernel_test]
4444
fn local_irq_unmask_works() {
4545
// Precondition: IRQs are masked.
46-
unsafe{exception::asynchronous::local_irq_mask()};
46+
exception::asynchronous::local_irq_mask();
4747
assert!(!exception::asynchronous::is_local_irq_masked());
4848

49-
unsafe{exception::asynchronous::local_irq_unmask()};
49+
exception::asynchronous::local_irq_unmask();
5050
assert!(exception::asynchronous::is_local_irq_masked());
5151
}
5252

@@ -56,12 +56,12 @@ fn local_irq_mask_save_works() {
5656
// Precondition: IRQs are unmasked.
5757
assert!(exception::asynchronous::is_local_irq_masked());
5858

59-
let first = unsafe{exception::asynchronous::local_irq_mask_save()};
59+
let first = exception::asynchronous::local_irq_mask_save();
6060
assert!(!exception::asynchronous::is_local_irq_masked());
6161

62-
let second = unsafe{exception::asynchronous::local_irq_mask_save()};
62+
let second = exception::asynchronous::local_irq_mask_save();
6363
assert_ne!(first, second);
6464

65-
unsafe{exception::asynchronous::local_irq_restore(first)};
65+
exception::asynchronous::local_irq_restore(first);
6666
assert!(exception::asynchronous::is_local_irq_masked());
6767
}

‎14_virtual_mem_part2_mmio_remap/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2300,7 +2300,7 @@ diff -uNr 13_exceptions_part2_peripheral_IRQs/kernel/src/exception/asynchronous.
23002300

23012301
impl<'irq_context> IRQContext<'irq_context> {
23022302
/// Creates an IRQContext token.
2303-
@@ -152,9 +162,17 @@
2303+
@@ -148,9 +158,17 @@
23042304
ret
23052305
}
23062306

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /