From 337a2d1245d44e8e9c96d53d2ff89b63d7fe039a Mon Sep 17 00:00:00 2001 From: David Brown Date: Thu, 22 Aug 2024 15:15:48 -0600 Subject: [PATCH 01/12] rust: Bring in fugit library The fugit crate implements tick-based timers, providing a `Duration` and `Instant` type that we can use for timeouts. This is an external crate, covered under the Apache-2.0 or MIT license. This will need to be vendored due to Rust policies. Signed-off-by: David Brown --- lib/rust/zephyr/Cargo.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/rust/zephyr/Cargo.toml b/lib/rust/zephyr/Cargo.toml index 7005f52ab55b6..977aea210e343 100644 --- a/lib/rust/zephyr/Cargo.toml +++ b/lib/rust/zephyr/Cargo.toml @@ -12,6 +12,9 @@ Functionality for Rust-based applications that run on Zephyr. [dependencies] zephyr-sys = { version = "0.1.0", path = "../zephyr-sys" } +[dependencies.fugit] +version = "0.3.7" + # These are needed at build time. # Whether these need to be vendored is an open question. They are not # used by the core Zephyr tree, but are needed by zephyr applications. From 213032f14514757a6a9788c9d52b5b943baa3baf Mon Sep 17 00:00:00 2001 From: David Brown Date: Wed, 21 Aug 2024 15:20:11 -0600 Subject: [PATCH 02/12] rust: Duration and Timeout types Use specific instances of `fugit::Duration` and `fugit::Instant`, with the clock parameter based on the configured clock rate. As the fugit times are fixed, this precludes using these time types with dynamically changing clocks. With this, is a local Timeout type that simply wraps `k_timeout_t`. Having this wrapper allows us to implement `From` from each of the timeout types. The zephyr timeout type can support either durations or instants, as well as two magic times, forever and no wait. Forever and NoWait are implemented as singleton types that also have conversions. The `zephyr::time::sleep` function then can take anything that can be converted to this wrapped Timeout type, including Forever, and NoWait. This template will be used for future API calls that need a timeout. Signed-off-by: David Brown --- lib/rust/zephyr-sys/build.rs | 3 + lib/rust/zephyr/src/lib.rs | 2 + lib/rust/zephyr/src/time.rs | 104 +++++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+) create mode 100644 lib/rust/zephyr/src/time.rs diff --git a/lib/rust/zephyr-sys/build.rs b/lib/rust/zephyr-sys/build.rs index 19714ae985dac..3d987a12581f8 100644 --- a/lib/rust/zephyr-sys/build.rs +++ b/lib/rust/zephyr-sys/build.rs @@ -67,6 +67,9 @@ fn main() -> Result<()> { .derive_copy(false) .allowlist_function("k_.*") .allowlist_function("gpio_.*") + .allowlist_function("sys_.*") + // Deprecated + .blocklist_function("sys_clock_timeout_end_calc") .parse_callbacks(Box::new(bindgen::CargoCallbacks::new())) .generate() .expect("Unable to generate bindings"); diff --git a/lib/rust/zephyr/src/lib.rs b/lib/rust/zephyr/src/lib.rs index 1440213ec9576..84233d052da09 100644 --- a/lib/rust/zephyr/src/lib.rs +++ b/lib/rust/zephyr/src/lib.rs @@ -9,6 +9,8 @@ #![no_std] #![allow(unexpected_cfgs)] +pub mod time; + // Bring in the generated kconfig module include!(concat!(env!("OUT_DIR"), "/kconfig.rs")); diff --git a/lib/rust/zephyr/src/time.rs b/lib/rust/zephyr/src/time.rs new file mode 100644 index 0000000000000..5db2b733061f2 --- /dev/null +++ b/lib/rust/zephyr/src/time.rs @@ -0,0 +1,104 @@ +//! Time types similar to `std::time` types. +//! +//! However, the rust-embedded world tends to use `fugit` for time. This has a Duration and +//! Instant type, but they are parameterized by the slice used, and try hard to do the conversion +//! at compile time. +//! +//! std has two time types, a Duration which represents an elapsed amount of time, and Instant, +//! which represents a specific instance in time. +//! +//! Zephyr typically coordinates time in terms of a system tick interval, which comes from +//! `sys_clock_hw_cycles_per_sec()`. +//! +//! The Rust/std semantics require Instant to be monotonically increasing. +//! +//! Zephyr's `sys/time_units.h` header contains numerous optimized macros for manipulating time in +//! these units, specifically for converting between human time units and ticks, trying to avoid +//! division, especially division by non-constants. + +use zephyr_sys::k_timeout_t; + +// The system ticks, is mostly a constant, but there are some boards that use a dynamic tick +// frequency, and thus need to read this at runtime. +#[cfg(CONFIG_TIMER_READS_ITS_FREQUENCY_AT_RUNTIME)] +compile_error!("Rust does not (yet) support dynamic frequency timer"); + +// Given the above not defined, the system time base comes from a kconfig. +/// The system time base. The system clock has this many ticks per second. +pub const SYS_FREQUENCY: u32 = crate::kconfig::CONFIG_SYS_CLOCK_TICKS_PER_SEC as u32; + +/// Zephyr can be configured for either 64-bit or 32-bit time values. Use the appropriate type +/// internally to match. This should end up the same size as `k_ticks_t`, but unsigned instead of +/// signed. +#[cfg(CONFIG_TIMEOUT_64BIT)] +pub type Tick = u64; +#[cfg(not(CONFIG_TIMEOUT_64BIT))] +pub type Tick = u32; + +/// Duration appropraite for Zephyr calls that expect `k_timeout_t`. The result will be a time +/// interval from "now" (when the call is made). +pub type Duration = fugit::Duration; + +/// An Instant appropriate for Zephyr calls that expect a `k_timeout_t`. The result will be an +/// absolute time in terms of system ticks. +#[cfg(CONFIG_TIMEOUT_64BIT)] +pub type Instant = fugit::Instant; + +// The zephry k_timeout_t represents several different types of intervals, based on the range of +// the value. It is a signed number of the same size as the Tick here, which effectively means it +// is one bit less. +// +// 0: K_NO_WAIT: indicates the operation should not wait. +// 1 .. Max: indicates a duration in ticks of a delay from "now". +// -1: K_FOREVER: a time that never expires. +// MIN .. -2: A wait for an absolute amount of ticks from the start of the system. +// +// The absolute time offset is only implemented when time is a 64 bit value. This also means that +// "Instant" isn't available when time is defined as a 32-bit value. + +// Wrapper around the timeout type, so we can implement From/Info. +pub struct Timeout(pub k_timeout_t); + +// From allows methods to take a time of various types and convert it into a Zephyr timeout. +impl From for Timeout { + fn from(value: Duration) -> Timeout { + Timeout(k_timeout_t { ticks: value.ticks() as i64 }) + } +} + +#[cfg(CONFIG_TIMEOUT_64BIT)] +impl From for Timeout { + fn from(value: Instant) -> Timeout { + Timeout(k_timeout_t { ticks: -1 - 1 - (value.ticks() as i64) }) + } +} + +/// A sleep that waits forever. This is its own type, that is `Into` and can be used +/// anywhere a timeout is needed. +pub struct Forever; + +impl From for Timeout { + fn from(_value: Forever) -> Timeout { + Timeout(crate::sys::K_FOREVER) + } +} + +/// A sleep that doesn't ever wait. This is its own type, that is `Info` and can be used +/// anywhere a timeout is needed. +pub struct NoWait; + +impl From for Timeout { + fn from(_valued: NoWait) -> Timeout { + Timeout(crate::sys::K_NO_WAIT) + } +} + +/// Put the current thread to sleep, for the given duration. Uses `k_sleep` for the actual sleep. +/// Returns a duration roughly representing the remaining amount of time if the sleep was woken. +pub fn sleep(timeout: T) -> Duration + where T: Into, +{ + let timeout: Timeout = timeout.into(); + let rest = unsafe { crate::raw::k_sleep(timeout.0) }; + Duration::millis(rest as Tick) +} From b5b8d922bcd24f1b0e8ab763b45c9350259b3686 Mon Sep 17 00:00:00 2001 From: David Brown Date: Mon, 9 Sep 2024 15:26:37 -0600 Subject: [PATCH 03/12] Rust: Add SPDX to time.rs Ensure this file is properly marked. Signed-off-by: David Brown --- lib/rust/zephyr/src/time.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/rust/zephyr/src/time.rs b/lib/rust/zephyr/src/time.rs index 5db2b733061f2..db900a2b3a559 100644 --- a/lib/rust/zephyr/src/time.rs +++ b/lib/rust/zephyr/src/time.rs @@ -1,3 +1,6 @@ +// Copyright (c) 2024 Linaro LTD +// SPDX-License-Identifier: Apache-2.0 + //! Time types similar to `std::time` types. //! //! However, the rust-embedded world tends to use `fugit` for time. This has a Duration and From e823c228f15870dd895f7584cb3e8a419284da1a Mon Sep 17 00:00:00 2001 From: David Brown Date: Tue, 10 Sep 2024 09:28:37 -0600 Subject: [PATCH 04/12] rust: Create 'sys' module The `zephyr-sys` crate contains all of the generated bindings to the C API in Zephyr. The `zephyr::sys` module wraps these abstractions in as thin of a way as possible, but still allowing them to be used without 'unsafe'. Although the interfaces are "safe", because they are just wrappers around C calls, most aren't directly useful without unsafe, and this crate provides higher-level abstractions around these underlying primitives. This commit adds a definition for `K_FOREVER`, and `K_NO_WAIT`, which are too complicated for bindgen to autogenerate bindings for. We will rely on a test to ensure that the values match those in the C headers. Signed-off-by: David Brown --- lib/rust/zephyr/src/lib.rs | 1 + lib/rust/zephyr/src/sys.rs | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 lib/rust/zephyr/src/sys.rs diff --git a/lib/rust/zephyr/src/lib.rs b/lib/rust/zephyr/src/lib.rs index 84233d052da09..9a02e897b97fe 100644 --- a/lib/rust/zephyr/src/lib.rs +++ b/lib/rust/zephyr/src/lib.rs @@ -9,6 +9,7 @@ #![no_std] #![allow(unexpected_cfgs)] +pub mod sys; pub mod time; // Bring in the generated kconfig module diff --git a/lib/rust/zephyr/src/sys.rs b/lib/rust/zephyr/src/sys.rs new file mode 100644 index 0000000000000..d7342a6d9eea1 --- /dev/null +++ b/lib/rust/zephyr/src/sys.rs @@ -0,0 +1,18 @@ +// Copyright (c) 2024 Linaro LTD +// SPDX-License-Identifier: Apache-2.0 + +//! Zephyr 'sys' module. +//! +//! The `zephyr-sys` crate contains the direct C bindings to the Zephyr API. All of these are +//! unsafe. +//! +//! This module `zephyr::sys` contains thin wrappers to these C bindings, that can be used without +//! unsafe, but as unchanged as possible. + +use zephyr_sys::k_timeout_t; + +// These two constants are not able to be captured by bindgen. It is unlikely that these values +// would change in the Zephyr headers, but there will be an explicit test to make sure they are +// correct. +pub const K_FOREVER: k_timeout_t = k_timeout_t { ticks: -1 }; +pub const K_NO_WAIT: k_timeout_t = k_timeout_t { ticks: 0 }; From e9b1585752c1ca98449c04a44344e27061040903 Mon Sep 17 00:00:00 2001 From: David Brown Date: Tue, 10 Sep 2024 09:34:28 -0600 Subject: [PATCH 05/12] rust: zephyr: time: Clarify docs Rewrite the doc comment for `zephyr::time`. A little bit of LLM help and this is much clearer. Signed-off-by: David Brown --- lib/rust/zephyr/src/time.rs | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/lib/rust/zephyr/src/time.rs b/lib/rust/zephyr/src/time.rs index db900a2b3a559..12e520b10b26e 100644 --- a/lib/rust/zephyr/src/time.rs +++ b/lib/rust/zephyr/src/time.rs @@ -1,23 +1,28 @@ // Copyright (c) 2024 Linaro LTD // SPDX-License-Identifier: Apache-2.0 -//! Time types similar to `std::time` types. +//! Time types designed for Zephyr, inspired by `std::time`. //! -//! However, the rust-embedded world tends to use `fugit` for time. This has a Duration and -//! Instant type, but they are parameterized by the slice used, and try hard to do the conversion -//! at compile time. +//! In `std`, there are two primary time types: `Duration`, representing a span of time, and +//! `Instant`, representing a specific point in time. Both have nanosecond precision, which is +//! well-suited to more powerful machines. However, on embedded systems like Zephyr, this precision +//! can lead to performance issues, often requiring divisions whenever timeouts are used. //! -//! std has two time types, a Duration which represents an elapsed amount of time, and Instant, -//! which represents a specific instance in time. +//! In the Rust embedded ecosystem, the `fugit` crate is commonly used for handling time. It +//! provides both `Duration` and `Instant` types, but with parameters that allow the representation +//! to match the time slice used, enabling compile-time conversion and storing time directly as +//! tick counts. //! -//! Zephyr typically coordinates time in terms of a system tick interval, which comes from -//! `sys_clock_hw_cycles_per_sec()`. +//! Zephyr manages time in terms of system tick intervals, derived from +//! `sys_clock_hw_cycles_per_sec()`. This model aligns well with `fugit`, especially when the +//! types are properly parameterized. //! -//! The Rust/std semantics require Instant to be monotonically increasing. +//! It's important to note that Rust’s `std::Instant` requires time to be monotonically increasing. //! -//! Zephyr's `sys/time_units.h` header contains numerous optimized macros for manipulating time in -//! these units, specifically for converting between human time units and ticks, trying to avoid -//! division, especially division by non-constants. +//! Zephyr’s `sys/time_units.h` provides a variety of optimized macros for manipulating time +//! values, converting between human-readable units and ticks, and minimizing divisions (especially +//! by non-constant values). Similarly, the `fugit` crate offers constructors that aim to result +//! in constants when possible, avoiding costly division operations. use zephyr_sys::k_timeout_t; From f8e33c5d4295f6647fe5d02dc943f355c8b7cff0 Mon Sep 17 00:00:00 2001 From: David Brown Date: Tue, 10 Sep 2024 13:22:12 -0600 Subject: [PATCH 06/12] tests: lib: rust: Add 'time' test Add a test to test that the time values in Rust (Duration and Instance) and up calculating the same results as the various macros used in the C code. Signed-off-by: David Brown --- tests/lib/rust/time/CMakeLists.txt | 10 ++++ tests/lib/rust/time/Cargo.toml | 16 +++++ tests/lib/rust/time/prj.conf | 5 ++ tests/lib/rust/time/src/lib.rs | 86 +++++++++++++++++++++++++++ tests/lib/rust/time/src/times.c | 94 ++++++++++++++++++++++++++++++ tests/lib/rust/time/testcase.yaml | 9 +++ 6 files changed, 220 insertions(+) create mode 100644 tests/lib/rust/time/CMakeLists.txt create mode 100644 tests/lib/rust/time/Cargo.toml create mode 100644 tests/lib/rust/time/prj.conf create mode 100644 tests/lib/rust/time/src/lib.rs create mode 100644 tests/lib/rust/time/src/times.c create mode 100644 tests/lib/rust/time/testcase.yaml diff --git a/tests/lib/rust/time/CMakeLists.txt b/tests/lib/rust/time/CMakeLists.txt new file mode 100644 index 0000000000000..ae269f27eb701 --- /dev/null +++ b/tests/lib/rust/time/CMakeLists.txt @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: Apache-2.0 + +cmake_minimum_required(VERSION 3.20.0) + +find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) +project(time_rust) + +target_sources(app PRIVATE src/times.c) + +rust_cargo_application() diff --git a/tests/lib/rust/time/Cargo.toml b/tests/lib/rust/time/Cargo.toml new file mode 100644 index 0000000000000..360788e3ec63b --- /dev/null +++ b/tests/lib/rust/time/Cargo.toml @@ -0,0 +1,16 @@ +# Copyright (c) 2024 Linaro LTD +# SPDX-License-Identifier: Apache-2.0 + +[package] +# This must be rustapp for now. +name = "rustapp" +version = "0.1.0" +edition = "2021" +description = "Tests of time" +license = "Apache-2.0 or MIT" + +[lib] +crate-type = ["staticlib"] + +[dependencies] +zephyr = "0.1.0" diff --git a/tests/lib/rust/time/prj.conf b/tests/lib/rust/time/prj.conf new file mode 100644 index 0000000000000..d9bbcfe972f87 --- /dev/null +++ b/tests/lib/rust/time/prj.conf @@ -0,0 +1,5 @@ +# Copyright (c) 2024 Linaro LTD +# SPDX-License-Identifier: Apache-2.0 + +CONFIG_RUST=y +CONFIG_MAIN_STACK_SIZE=2048 diff --git a/tests/lib/rust/time/src/lib.rs b/tests/lib/rust/time/src/lib.rs new file mode 100644 index 0000000000000..e8d1602d338d8 --- /dev/null +++ b/tests/lib/rust/time/src/lib.rs @@ -0,0 +1,86 @@ +// Copyright (c) 2024 Linaro LTD +// SPDX-License-Identifier: Apache-2.0 + +#![no_std] + +use core::ffi::{ + c_char, + CStr, +}; + +use zephyr::printkln; +use zephyr::time::{Duration, Instant, Tick, Timeout}; +use zephyr::raw::k_timeout_t; + +#[no_mangle] +extern "C" fn rust_main() { + printkln!("Tick frequency: {}", zephyr::time::SYS_FREQUENCY); + check_conversions(); + printkln!("All tests passed"); +} + +/// Verify that the conversions are correct. +fn check_conversions() { + let mut index = 0; + loop { + // The entry returns is always valid, so is a valid reference. + let entry = unsafe { &*get_time_entry(index) }; + if entry.name.is_null() { + break; + } + let name = unsafe { + CStr::from_ptr(entry.name).to_str().expect("Invalid C string") + }; + printkln!("Testing: {}", name); + + // The units must match the enum in the C code. + match entry.units { + // UNIT_FOREVER + 0 => { + assert_eq!(entry.value.ticks, zephyr::sys::K_FOREVER.ticks); + } + // UNIT_NO_WAIT + 1 => { + assert_eq!(entry.value.ticks, zephyr::sys::K_NO_WAIT.ticks); + } + // UNIT_DUR_MS + 2 => { + let value = Duration::millis_at_least(entry.uvalue as Tick); + let value: Timeout = value.into(); + assert_eq!(entry.value.ticks, value.0.ticks); + } + // UNIT_INST_MS + 3 => { + let base = Instant::from_ticks(0); + let value = Duration::millis_at_least(entry.uvalue as Tick); + let value = base + value; + let value: Timeout = value.into(); + let c_value = unsafe { ms_to_abs_timeout(entry.uvalue) }; + if c_value.ticks != value.0.ticks { + printkln!("Mismatch C: {}, Rust: {}", + c_value.ticks, value.0.ticks); + } + assert_eq!(c_value.ticks, value.0.ticks); + } + _ => { + panic!("Invalid unit enum"); + } + } + + index += 1; + } +} + +/// The time entry information. +#[repr(C)] +struct TimeEntry { + name: *const c_char, + units: u32, + uvalue: i64, + value: k_timeout_t, +} + +extern "C" { + fn get_time_entry(index: usize) -> *const TimeEntry; + fn ms_to_abs_timeout(ms: i64) -> k_timeout_t; +} diff --git a/tests/lib/rust/time/src/times.c b/tests/lib/rust/time/src/times.c new file mode 100644 index 0000000000000..7660c28557a01 --- /dev/null +++ b/tests/lib/rust/time/src/times.c @@ -0,0 +1,94 @@ +/* Copyright (c) 2024 Linaro LTD */ +/* SPDX-License-Identifier: Apache-2.0 */ + +#include + +/* Rather than trying to get C enums to match in size with Rust ones, just use a known integet type. + */ +enum units { + UNIT_FOREVER, + UNIT_NO_WAIT, + UNIT_DUR_MSEC, + UNIT_INST_MSEC, +}; + +/* Data handed back from C containing processed time constant values. + */ +struct time_entry { + const char *name; + + uint32_t units; + + /* Value in the given units. */ + int64_t uvalue; + + /* Value in ticks. */ + k_timeout_t value; +}; + +const struct time_entry time_entries[] = { + { + .name = "K_FOREVER", + .units = UNIT_FOREVER, + .uvalue = 0, + .value = K_FOREVER, + }, + { + .name = "K_NO_WAIT", + .units = UNIT_NO_WAIT, + .uvalue = 0, + .value = K_NO_WAIT, + }, +#define DUR_TEST(unit, n) \ + { \ + .name = "Duration " #unit " " #n, \ + .units = UNIT_DUR_ ## unit, \ + .uvalue = n, \ + .value = K_ ## unit(n) , \ + } + // Test various values near typical clock boundaries. + DUR_TEST(MSEC, 1), + DUR_TEST(MSEC, 2), + DUR_TEST(MSEC, 99), + DUR_TEST(MSEC, 100), + DUR_TEST(MSEC, 101), + DUR_TEST(MSEC, 999), + DUR_TEST(MSEC, 1000), + DUR_TEST(MSEC, 1001), + DUR_TEST(MSEC, 32767), + DUR_TEST(MSEC, 32768), + DUR_TEST(MSEC, 32769), +#define INST_TEST(unit, n) \ + { \ + .name = "Instant " #unit " " #n, \ + .units = UNIT_INST_ ## unit, \ + .uvalue = n, \ + } + INST_TEST(MSEC, 1), + INST_TEST(MSEC, 2), + INST_TEST(MSEC, 99), + INST_TEST(MSEC, 100), + INST_TEST(MSEC, 101), + INST_TEST(MSEC, 999), + INST_TEST(MSEC, 1000), + INST_TEST(MSEC, 1001), + INST_TEST(MSEC, 32767), + INST_TEST(MSEC, 32768), + INST_TEST(MSEC, 32769), + { + .name = 0, + }, +}; + +/* Return the indexed time entry. It is up to the Rust code to detect the null name, and handle it + * properly. + */ +const struct time_entry *get_time_entry(uintptr_t index) { + return &time_entries[index]; +} + +/* The abs timeout is not constant, so provide this wrapper function. + */ +const k_timeout_t ms_to_abs_timeout(int64_t ms) { + return K_TIMEOUT_ABS_MS(ms); +} diff --git a/tests/lib/rust/time/testcase.yaml b/tests/lib/rust/time/testcase.yaml new file mode 100644 index 0000000000000..b28c72a89c88b --- /dev/null +++ b/tests/lib/rust/time/testcase.yaml @@ -0,0 +1,9 @@ +common: + filter: CONFIG_RUST_SUPPORTED +tests: + test.rust.time: + harness: console + harness_config: + type: one_line + regex: + - "All tests passed" From ea375ac2a790a602a1f80114a64b664fd7326b00 Mon Sep 17 00:00:00 2001 From: David Brown Date: Tue, 10 Sep 2024 13:27:26 -0600 Subject: [PATCH 07/12] rust: zephyr: use printkln for panic messages If we have printk enabled, use that to output the desperation panic message when panic is called from Rust code. Signed-off-by: David Brown --- lib/rust/zephyr/src/lib.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/rust/zephyr/src/lib.rs b/lib/rust/zephyr/src/lib.rs index 9a02e897b97fe..80bf18a05902a 100644 --- a/lib/rust/zephyr/src/lib.rs +++ b/lib/rust/zephyr/src/lib.rs @@ -27,7 +27,12 @@ use core::panic::PanicInfo; /// Override rust's panic. This simplistic initial version just hangs in a loop. #[panic_handler] -fn panic(_ :&PanicInfo) -> ! { +fn panic(info :&PanicInfo) -> ! { + #[cfg(CONFIG_PRINTK)] + { + printkln!("panic: {}", info); + } + let _ = info; loop { } } From e1aa3466339656cb96c0a0bd253f1561c4564660 Mon Sep 17 00:00:00 2001 From: David Brown Date: Tue, 10 Sep 2024 13:41:03 -0600 Subject: [PATCH 08/12] tests: rust: time: Fix various C formatting errors Too much rust code formatting leaks into my C. Fix these various whitespace errors. Signed-off-by: David Brown --- tests/lib/rust/time/src/times.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/lib/rust/time/src/times.c b/tests/lib/rust/time/src/times.c index 7660c28557a01..443587c7d9fa2 100644 --- a/tests/lib/rust/time/src/times.c +++ b/tests/lib/rust/time/src/times.c @@ -44,9 +44,9 @@ const struct time_entry time_entries[] = { .name = "Duration " #unit " " #n, \ .units = UNIT_DUR_ ## unit, \ .uvalue = n, \ - .value = K_ ## unit(n) , \ + .value = K_ ## unit(n), \ } - // Test various values near typical clock boundaries. + /* Test various values near typical clock boundaries. */ DUR_TEST(MSEC, 1), DUR_TEST(MSEC, 2), DUR_TEST(MSEC, 99), @@ -83,12 +83,14 @@ const struct time_entry time_entries[] = { /* Return the indexed time entry. It is up to the Rust code to detect the null name, and handle it * properly. */ -const struct time_entry *get_time_entry(uintptr_t index) { +const struct time_entry *get_time_entry(uintptr_t index) +{ return &time_entries[index]; } /* The abs timeout is not constant, so provide this wrapper function. */ -const k_timeout_t ms_to_abs_timeout(int64_t ms) { +const k_timeout_t ms_to_abs_timeout(int64_t ms) +{ return K_TIMEOUT_ABS_MS(ms); } From 9afdb7749728594e31ddba6af35e109d6bf113c6 Mon Sep 17 00:00:00 2001 From: David Brown Date: Tue, 10 Sep 2024 15:55:29 -0600 Subject: [PATCH 09/12] rust: panic: Call into arch panic handler When the panic happens, call into the Zephyr `k_panic()` handler instead of just spinning. This should halt the whole system, not just the current thread. Signed-off-by: David Brown --- lib/rust/main.c | 8 ++++++++ lib/rust/zephyr/src/lib.rs | 8 +++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/rust/main.c b/lib/rust/main.c index a5fa648cbd1dc..ba75f92ed4de3 100644 --- a/lib/rust/main.c +++ b/lib/rust/main.c @@ -16,4 +16,12 @@ int main(void) return 0; } +/* On most arches, panic is entirely macros resulting in some kind of inline assembly. Create this + * wrapper so the Rust panic handler can call the same kind of panic. + */ +void rust_panic_wrap(void) +{ + k_panic(); +} + #endif diff --git a/lib/rust/zephyr/src/lib.rs b/lib/rust/zephyr/src/lib.rs index 80bf18a05902a..ce71e46f93aae 100644 --- a/lib/rust/zephyr/src/lib.rs +++ b/lib/rust/zephyr/src/lib.rs @@ -33,7 +33,13 @@ fn panic(info :&PanicInfo) -> ! { printkln!("panic: {}", info); } let _ = info; - loop { + + // Call into the wrapper for the system panic function. + unsafe { + extern "C" { + fn rust_panic_wrap() -> !; + } + rust_panic_wrap(); } } From f5a4ad92cbe24b2a50140dbc1a9b405129706886 Mon Sep 17 00:00:00 2001 From: David Brown Date: Tue, 10 Sep 2024 16:07:26 -0600 Subject: [PATCH 10/12] tests: lib: rust: Clarify some comments Add some additional comments to clarify that not all of the fields are used for all of the test types. Signed-off-by: David Brown --- tests/lib/rust/time/src/times.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/lib/rust/time/src/times.c b/tests/lib/rust/time/src/times.c index 443587c7d9fa2..ac23d9e7ead67 100644 --- a/tests/lib/rust/time/src/times.c +++ b/tests/lib/rust/time/src/times.c @@ -27,16 +27,15 @@ struct time_entry { }; const struct time_entry time_entries[] = { + /* For the constants, only the `.value` gets used by the test. */ { .name = "K_FOREVER", .units = UNIT_FOREVER, - .uvalue = 0, .value = K_FOREVER, }, { .name = "K_NO_WAIT", .units = UNIT_NO_WAIT, - .uvalue = 0, .value = K_NO_WAIT, }, #define DUR_TEST(unit, n) \ @@ -58,6 +57,9 @@ const struct time_entry time_entries[] = { DUR_TEST(MSEC, 32767), DUR_TEST(MSEC, 32768), DUR_TEST(MSEC, 32769), + /* The Instance tests don't set the `.value` because it isn't constant, and the test code + * will calculate the value at runtime, using the conversion functions below. + */ #define INST_TEST(unit, n) \ { \ .name = "Instant " #unit " " #n, \ From ec1c9f699eb829f2b8f3a0a0c386b9f39125ac26 Mon Sep 17 00:00:00 2001 From: David Brown Date: Tue, 10 Sep 2024 16:30:26 -0600 Subject: [PATCH 11/12] rust: time: Check various time conversion errors When debug assertions are enabled, ensure that the time conversions are sensible. If there is an overflow on time with the assertions disabled, just use Default, which will be zero. Signed-off-by: David Brown --- lib/rust/zephyr/src/time.rs | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/lib/rust/zephyr/src/time.rs b/lib/rust/zephyr/src/time.rs index 12e520b10b26e..4610449d34258 100644 --- a/lib/rust/zephyr/src/time.rs +++ b/lib/rust/zephyr/src/time.rs @@ -24,7 +24,9 @@ //! by non-constant values). Similarly, the `fugit` crate offers constructors that aim to result //! in constants when possible, avoiding costly division operations. -use zephyr_sys::k_timeout_t; +use zephyr_sys::{k_timeout_t, k_ticks_t}; + +use core::fmt::Debug; // The system ticks, is mostly a constant, but there are some boards that use a dynamic tick // frequency, and thus need to read this at runtime. @@ -70,14 +72,20 @@ pub struct Timeout(pub k_timeout_t); // From allows methods to take a time of various types and convert it into a Zephyr timeout. impl From for Timeout { fn from(value: Duration) -> Timeout { - Timeout(k_timeout_t { ticks: value.ticks() as i64 }) + let ticks: k_ticks_t = checked_cast(value.ticks()); + debug_assert_ne!(ticks, crate::sys::K_FOREVER.ticks); + debug_assert_ne!(ticks, crate::sys::K_NO_WAIT.ticks); + Timeout(k_timeout_t { ticks }) } } #[cfg(CONFIG_TIMEOUT_64BIT)] impl From for Timeout { fn from(value: Instant) -> Timeout { - Timeout(k_timeout_t { ticks: -1 - 1 - (value.ticks() as i64) }) + let ticks: k_ticks_t = checked_cast(value.ticks()); + debug_assert_ne!(ticks, crate::sys::K_FOREVER.ticks); + debug_assert_ne!(ticks, crate::sys::K_NO_WAIT.ticks); + Timeout(k_timeout_t { ticks: -1 - 1 - ticks }) } } @@ -110,3 +118,18 @@ pub fn sleep(timeout: T) -> Duration let rest = unsafe { crate::raw::k_sleep(timeout.0) }; Duration::millis(rest as Tick) } + +/// Convert from the Tick time type, which is unsigned, to the `k_ticks_t` type. When debug +/// assertions are enabled, it will panic on overflow. +fn checked_cast(tick: I) -> O +where + I: TryInto, + I::Error: Debug, + O: Default, +{ + if cfg!(debug_assertions) { + tick.try_into().expect("Overflow in time conversion") + } else { + tick.try_into().unwrap_or(O::default()) + } +} From 7b168d6918cfc23819dcd263d31daca54bef73d4 Mon Sep 17 00:00:00 2001 From: David Brown Date: Tue, 10 Sep 2024 23:05:45 -0600 Subject: [PATCH 12/12] rust: Comment cleanup Comment cleanups. Fix several minor typos in the comments. Signed-off-by: David Brown --- lib/rust/zephyr/src/time.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/rust/zephyr/src/time.rs b/lib/rust/zephyr/src/time.rs index 4610449d34258..7e6228e7421c8 100644 --- a/lib/rust/zephyr/src/time.rs +++ b/lib/rust/zephyr/src/time.rs @@ -45,7 +45,7 @@ pub type Tick = u64; #[cfg(not(CONFIG_TIMEOUT_64BIT))] pub type Tick = u32; -/// Duration appropraite for Zephyr calls that expect `k_timeout_t`. The result will be a time +/// Duration appropriate for Zephyr calls that expect `k_timeout_t`. The result will be a time /// interval from "now" (when the call is made). pub type Duration = fugit::Duration; @@ -54,7 +54,7 @@ pub type Duration = fugit::Duration; #[cfg(CONFIG_TIMEOUT_64BIT)] pub type Instant = fugit::Instant; -// The zephry k_timeout_t represents several different types of intervals, based on the range of +// The Zephyr `k_timeout_t` represents several different types of intervals, based on the range of // the value. It is a signed number of the same size as the Tick here, which effectively means it // is one bit less. // @@ -63,13 +63,13 @@ pub type Instant = fugit::Instant; // -1: K_FOREVER: a time that never expires. // MIN .. -2: A wait for an absolute amount of ticks from the start of the system. // -// The absolute time offset is only implemented when time is a 64 bit value. This also means that +// The absolute time offset is only implemented when time is a 64-bit value. This also means that // "Instant" isn't available when time is defined as a 32-bit value. // Wrapper around the timeout type, so we can implement From/Info. pub struct Timeout(pub k_timeout_t); -// From allows methods to take a time of various types and convert it into a Zephyr timeout. +// `From` allows methods to take a time of various types and convert it into a Zephyr timeout. impl From for Timeout { fn from(value: Duration) -> Timeout { let ticks: k_ticks_t = checked_cast(value.ticks());