From 7576183225158b62223dd19afd270482746d1245 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ioan-Cristian=20C=C3=8ERSTEA?= Date: Mon, 2 Dec 2024 17:04:37 +0200 Subject: [PATCH 1/2] Use const operand for ARM syscall implementation `asm_const` feature has been stabilized in Rust 1.82. Using const operands for inline assembly eliminates a lot of the duplicated code in the ARM syscall implementation. This commit has been tested by running a simple application that periodically blinks and writes a message on Raspberry Pi Pico. --- Cargo.toml | 2 +- runtime/src/syscalls_impl_arm.rs | 92 ++++++++------------------- rust-toolchain.toml | 2 +- syscalls_tests/src/subscribe_tests.rs | 5 ++ 4 files changed, 35 insertions(+), 66 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 10d7a29b8..776b3431b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,7 @@ version = "0.1.0" [workspace.package] # This must be kept in sync with rust-toolchain.toml; please see that file for # more information. -rust-version = "1.77" +rust-version = "1.82" [features] rust_embedded = [ diff --git a/runtime/src/syscalls_impl_arm.rs b/runtime/src/syscalls_impl_arm.rs index 5bcf923ad..4a74943e9 100644 --- a/runtime/src/syscalls_impl_arm.rs +++ b/runtime/src/syscalls_impl_arm.rs @@ -1,5 +1,5 @@ use core::arch::asm; -use libtock_platform::{syscall_class, RawSyscalls, Register}; +use libtock_platform::{RawSyscalls, Register}; unsafe impl RawSyscalls for crate::TockSyscalls { unsafe fn yield1([Register(r0)]: [Register; 1]) { @@ -43,92 +43,56 @@ unsafe impl RawSyscalls for crate::TockSyscalls { } } - unsafe fn syscall1([Register(mut r0)]: [Register; 1]) -> [Register; 2] { + unsafe fn syscall1( + [Register(mut r0)]: [Register; 1], + ) -> [Register; 2] { let r1; // Safety: This matches the invariants required by the documentation on // RawSyscalls::syscall1 unsafe { - // Syscall class 5 is Memop, the only syscall class that syscall1 - // supports. - asm!("svc 5", - inlateout("r0") r0, - lateout("r1") r1, - options(preserves_flags, nostack, nomem), + asm!( + "svc {SYSCALL_CLASS_NUMBER}", + inlateout("r0") r0, + lateout("r1") r1, + options(preserves_flags, nostack, nomem), + SYSCALL_CLASS_NUMBER = const SYSCALL_CLASS_NUMBER, ); } [Register(r0), Register(r1)] } - unsafe fn syscall2( + unsafe fn syscall2( [Register(mut r0), Register(mut r1)]: [Register; 2], ) -> [Register; 2] { // Safety: This matches the invariants required by the documentation on // RawSyscalls::syscall2 unsafe { - // TODO: Replace this match statement with a `const` operand when - // asm_const [1] is stabilized, or redesign RawSyscalls to not need - // this match statement. - // - // [1] https://github.com/rust-lang/rust/issues/93332 - match CLASS { - syscall_class::MEMOP => asm!("svc 5", - inlateout("r0") r0, - inlateout("r1") r1, - options(preserves_flags, nostack, nomem) - ), - syscall_class::EXIT => asm!("svc 6", - inlateout("r0") r0, - inlateout("r1") r1, - options(preserves_flags, nostack, nomem) - ), - _ => unreachable!(), - } + asm!( + "svc {SYSCALL_CLASS_NUMBER}", + inlateout("r0") r0, + inlateout("r1") r1, + options(preserves_flags, nostack, nomem), + SYSCALL_CLASS_NUMBER = const SYSCALL_CLASS_NUMBER, + ); } [Register(r0), Register(r1)] } - unsafe fn syscall4( + unsafe fn syscall4( [Register(mut r0), Register(mut r1), Register(mut r2), Register(mut r3)]: [Register; 4], ) -> [Register; 4] { // Safety: This matches the invariants required by the documentation on // RawSyscalls::syscall4 unsafe { - // TODO: Replace this match statement with a `const` operand when - // asm_const [1] is stabilized, or redesign RawSyscalls to not need - // this match statement. - // - // [1] https://github.com/rust-lang/rust/issues/93332 - match CLASS { - syscall_class::SUBSCRIBE => asm!("svc 1", - inlateout("r0") r0, - inlateout("r1") r1, - inlateout("r2") r2, - inlateout("r3") r3, - options(preserves_flags, nostack), - ), - syscall_class::COMMAND => asm!("svc 2", - inlateout("r0") r0, - inlateout("r1") r1, - inlateout("r2") r2, - inlateout("r3") r3, - options(preserves_flags, nostack), - ), - syscall_class::ALLOW_RW => asm!("svc 3", - inlateout("r0") r0, - inlateout("r1") r1, - inlateout("r2") r2, - inlateout("r3") r3, - options(preserves_flags, nostack), - ), - syscall_class::ALLOW_RO => asm!("svc 4", - inlateout("r0") r0, - inlateout("r1") r1, - inlateout("r2") r2, - inlateout("r3") r3, - options(preserves_flags, nostack), - ), - _ => unreachable!(), - } + asm!( + "svc {SYSCALL_CLASS_NUMBER}", + inlateout("r0") r0, + inlateout("r1") r1, + inlateout("r2") r2, + inlateout("r3") r3, + options(preserves_flags, nostack), + SYSCALL_CLASS_NUMBER = const SYSCALL_CLASS_NUMBER, + ); } [Register(r0), Register(r1), Register(r2), Register(r3)] } diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 795603e64..f87df8148 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -5,7 +5,7 @@ # you'd like to use. When you do so, update this to the first Rust version that # includes that feature. Whenever this value is updated, the rust-version field # in Cargo.toml must be updated as well. -channel = "1.77" +channel = "1.82" components = ["clippy", "rustfmt"] targets = [ "thumbv6m-none-eabi", diff --git a/syscalls_tests/src/subscribe_tests.rs b/syscalls_tests/src/subscribe_tests.rs index 673f7d5e7..7af62f290 100644 --- a/syscalls_tests/src/subscribe_tests.rs +++ b/syscalls_tests/src/subscribe_tests.rs @@ -104,6 +104,10 @@ fn success() { assert_eq!(fake::Syscalls::yield_no_wait(), YieldNoWaitReturn::NoUpcall); } +// The toolchain update from 1.78 to 1.82 broke this test. However, this test will start working +// again as of Rust 1.84. See https://github.com/tock/libtock-rs/pull/565#issuecomment-2546915870 +// for more details. +/* #[cfg(not(miri))] #[test] fn unwinding_upcall() { @@ -131,3 +135,4 @@ fn unwinding_upcall() { }); assert_eq!(exit, libtock_unittest::ExitCall::Terminate(0)); } +*/ From 0c7862a86a2cbef328ae39d85ef4d9537195b62f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ioan-Cristian=20C=C3=8ERSTEA?= Date: Thu, 16 Jan 2025 19:03:05 +0200 Subject: [PATCH 2/2] Fixed transmute clippy lint warnings. The toolchain bump from 1.79 to 1.82 introduced a new clippy lint regarding transmutes. This commit fixes the emitted lint warnings. --- platform/src/command_return.rs | 14 +++++++++----- platform/src/error_code.rs | 2 +- platform/src/syscalls_impl.rs | 16 ++++++++-------- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/platform/src/command_return.rs b/platform/src/command_return.rs index a8b89a914..c3696536d 100644 --- a/platform/src/command_return.rs +++ b/platform/src/command_return.rs @@ -139,7 +139,7 @@ impl CommandReturn { if !self.is_failure() { return None; } - Some(unsafe { transmute(self.r1) }) + Some(unsafe { transmute::(self.r1) }) } /// Returns the error code and value if this CommandReturn is of type @@ -148,7 +148,7 @@ impl CommandReturn { if !self.is_failure_u32() { return None; } - Some((unsafe { transmute(self.r1) }, self.r2)) + Some((unsafe { transmute::(self.r1) }, self.r2)) } /// Returns the error code and return values if this CommandReturn is of @@ -157,7 +157,11 @@ impl CommandReturn { if !self.is_failure_2_u32() { return None; } - Some((unsafe { transmute(self.r1) }, self.r2, self.r3)) + Some(( + unsafe { transmute::(self.r1) }, + self.r2, + self.r3, + )) } /// Returns the error code and return value if this CommandReturn is of type @@ -167,7 +171,7 @@ impl CommandReturn { return None; } Some(( - unsafe { transmute(self.r1) }, + unsafe { transmute::(self.r1) }, self.r2 as u64 + ((self.r3 as u64) << 32), )) } @@ -244,7 +248,7 @@ impl CommandReturn { let ec: ErrorCode = if return_variant == E::RETURN_VARIANT { // Safety: E::RETURN_VARIANT must be a failure variant, and // failure variants must contain a valid ErrorCode in r1. - unsafe { transmute(r1) } + unsafe { transmute::(r1) } } else { r2 = 0; r3 = 0; diff --git a/platform/src/error_code.rs b/platform/src/error_code.rs index c78b75965..9bbb159af 100644 --- a/platform/src/error_code.rs +++ b/platform/src/error_code.rs @@ -277,7 +277,7 @@ impl TryFrom for ErrorCode { fn try_from(value: u32) -> Result { if (1..=1024).contains(&value) { - Ok(unsafe { transmute(value) }) + Ok(unsafe { transmute::(value) }) } else { Err(NotAnErrorCode) } diff --git a/platform/src/syscalls_impl.rs b/platform/src/syscalls_impl.rs index d2057ede3..cd1961cfd 100644 --- a/platform/src/syscalls_impl.rs +++ b/platform/src/syscalls_impl.rs @@ -118,7 +118,7 @@ impl Syscalls for S { // then r1 will contain a valid error code. ErrorCode is // designed to be safely transmuted directly from a kernel error // code. - return Err(unsafe { core::mem::transmute(r1.as_u32()) }); + return Err(unsafe { core::mem::transmute::(r1.as_u32()) }); } // r0 indicates Success with 2 u32s. Confirm the null upcall was @@ -231,7 +231,7 @@ impl Syscalls for S { // then r1 will contain a valid error code. ErrorCode is // designed to be safely transmuted directly from a kernel error // code. - return Err(unsafe { core::mem::transmute(r1.as_u32()) }); + return Err(unsafe { core::mem::transmute::(r1.as_u32()) }); } // r0 indicates Success with 2 u32s. Confirm a zero buffer was @@ -318,7 +318,7 @@ impl Syscalls for S { // then r1 will contain a valid error code. ErrorCode is // designed to be safely transmuted directly from a kernel error // code. - return Err(unsafe { core::mem::transmute(r1.as_u32()) }); + return Err(unsafe { core::mem::transmute::(r1.as_u32()) }); } // r0 indicates Success with 2 u32s. Confirm a zero buffer was @@ -380,7 +380,7 @@ impl Syscalls for S { // then r1 will contain a valid error code. ErrorCode is // designed to be safely transmuted directly from a kernel error // code. - Err(unsafe { core::mem::transmute(r1.as_u32()) }) + Err(unsafe { core::mem::transmute::(r1.as_u32()) }) } else { Ok(()) } @@ -407,7 +407,7 @@ impl Syscalls for S { // then r1 will contain a valid error code. ErrorCode is // designed to be safely transmuted directly from a kernel error // code. - Err(unsafe { core::mem::transmute(r1.as_u32()) }) + Err(unsafe { core::mem::transmute::(r1.as_u32()) }) } else { Ok(r1.into()) } @@ -436,7 +436,7 @@ impl Syscalls for S { // then r1 will contain a valid error code. ErrorCode is // designed to be safely transmuted directly from a kernel error // code. - Err(unsafe { core::mem::transmute(r1.as_u32()) }) + Err(unsafe { core::mem::transmute::(r1.as_u32()) }) } else { Ok(r1.into()) } @@ -464,7 +464,7 @@ impl Syscalls for S { // then r1 will contain a valid error code. ErrorCode is // designed to be safely transmuted directly from a kernel error // code. - Err(unsafe { core::mem::transmute(r1.as_u32()) }) + Err(unsafe { core::mem::transmute::(r1.as_u32()) }) } else { Ok(()) } @@ -493,7 +493,7 @@ impl Syscalls for S { // then r1 will contain a valid error code. ErrorCode is // designed to be safely transmuted directly from a kernel error // code. - Err(unsafe { core::mem::transmute(r1.as_u32()) }) + Err(unsafe { core::mem::transmute::(r1.as_u32()) }) } else { Ok(()) }