From 3bc05dc04ba5eb341a45802c5804ce0002227bdc Mon Sep 17 00:00:00 2001 From: Billy Price Date: Tue, 23 Sep 2025 18:18:08 -0700 Subject: [PATCH 01/14] Early implementation of ACPI time and alarm device service --- Cargo.lock | 33 + Cargo.toml | 3 + .../src/ec_type/generator/ec_memory_map.yaml | 38 -- embedded-service/src/ec_type/message.rs | 22 - embedded-service/src/ec_type/mod.rs | 321 ---------- .../src/ec_type/protocols/mctp.rs | 2 + embedded-service/src/ec_type/structure.rs | 25 +- espi-service/src/espi_service.rs | 26 - time-alarm-service/Cargo.toml | 35 + time-alarm-service/src/acpi_timestamp.rs | 217 +++++++ time-alarm-service/src/lib.rs | 600 ++++++++++++++++++ time-alarm-service/src/timer.rs | 336 ++++++++++ 12 files changed, 1227 insertions(+), 431 deletions(-) create mode 100644 time-alarm-service/Cargo.toml create mode 100644 time-alarm-service/src/acpi_timestamp.rs create mode 100644 time-alarm-service/src/lib.rs create mode 100644 time-alarm-service/src/timer.rs diff --git a/Cargo.lock b/Cargo.lock index 8b21774c..d3936e50 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -298,6 +298,20 @@ name = "bytemuck" version = "1.23.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3995eaeebcdf32f91f980d360f78732ddc061097ab4e39991ae7a6ace9194677" +dependencies = [ + "bytemuck_derive", +] + +[[package]] +name = "bytemuck_derive" +version = "1.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4f154e572231cb6ba2bd1176980827e3d5dc04cc183a75dea38109fbdd672d29" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] [[package]] name = "byteorder" @@ -1977,6 +1991,25 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "time-alarm-service" +version = "0.1.0" +dependencies = [ + "bytemuck", + "defmt 0.3.100", + "embassy-executor", + "embassy-futures", + "embassy-sync", + "embassy-time", + "embedded-hal 1.0.0", + "embedded-hal-async", + "embedded-mcu-hal", + "embedded-services", + "heapless 0.8.0", + "log", + "uuid", +] + [[package]] name = "tokio" version = "1.47.1" diff --git a/Cargo.toml b/Cargo.toml index 74e657ab..9729ec3f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,6 +13,7 @@ members = [ "platform-service", "power-button-service", "power-policy-service", + "time-alarm-service", "type-c-service", "debug-service", ] @@ -34,6 +35,7 @@ bitfield = "0.17.0" bitflags = "2.8.0" bitvec = { version = "1.0.1", default-features = false } block-device-driver = "0.2" +bytemuck = {version = "1.23.2", features=[ "derive" ]} cfg-if = "1.0.0" chrono = { version = "0.4", default-features = false } cortex-m = "0.7.6" @@ -54,6 +56,7 @@ embedded-hal-async = "1.0" embedded-hal-nb = "1.0" embedded-io = "0.6.1" embedded-io-async = "0.6.1" +embedded-mcu-hal = { git = "https://github.com/OpenDevicePartnership/embedded-mcu" } embedded-services = { path = "./embedded-service" } embedded-storage = "0.3" embedded-storage-async = "0.4.1" diff --git a/embedded-service/src/ec_type/generator/ec_memory_map.yaml b/embedded-service/src/ec_type/generator/ec_memory_map.yaml index 05677cc6..59e20535 100644 --- a/embedded-service/src/ec_type/generator/ec_memory_map.yaml +++ b/embedded-service/src/ec_type/generator/ec_memory_map.yaml @@ -39,44 +39,6 @@ Capabilities: res0: type: u16 -# Size 0x28 -TimeAlarm: - events: - type: u32 - capability: - type: u32 - year: - type: u16 - month: - type: u8 - day: - type: u8 - hour: - type: u8 - minute: - type: u8 - second: - type: u8 - valid: - type: u8 - daylight: - type: u8 - res1: - type: u8 - milli: - type: u16 - time_zone: - type: u16 - res2: - type: u16 - alarm_status: - type: u32 - ac_time_val: - type: u32 - dc_time_val: - type: u32 - - # Size 0x64 Battery: events: diff --git a/embedded-service/src/ec_type/message.rs b/embedded-service/src/ec_type/message.rs index 6e14ec2f..8cdce81a 100644 --- a/embedded-service/src/ec_type/message.rs +++ b/embedded-service/src/ec_type/message.rs @@ -14,28 +14,6 @@ pub enum CapabilitiesMessage { DebugMask(u16), } -#[allow(missing_docs)] -#[derive(Clone, Copy, Debug, PartialEq)] -pub enum TimeAlarmMessage { - Events(u32), - Capability(u32), - Year(u16), - Month(u8), - Day(u8), - Hour(u8), - Minute(u8), - Second(u8), - Valid(u8), - Daylight(u8), - Res1(u8), - Milli(u16), - TimeZone(u16), - Res2(u16), - AlarmStatus(u32), - AcTimeVal(u32), - DcTimeVal(u32), -} - #[allow(missing_docs)] #[derive(Clone, Copy, Debug, PartialEq)] pub enum BatteryMessage { diff --git a/embedded-service/src/ec_type/mod.rs b/embedded-service/src/ec_type/mod.rs index 1393a3b5..8fae0f2f 100644 --- a/embedded-service/src/ec_type/mod.rs +++ b/embedded-service/src/ec_type/mod.rs @@ -82,29 +82,6 @@ pub fn update_thermal_section(msg: &message::ThermalMessage, memory_map: &mut st } } -/// Update time alarm section of memory map based on battery message -pub fn update_time_alarm_section(msg: &message::TimeAlarmMessage, memory_map: &mut structure::ECMemory) { - match msg { - message::TimeAlarmMessage::Events(events) => memory_map.alarm.events = *events, - message::TimeAlarmMessage::Capability(capability) => memory_map.alarm.capability = *capability, - message::TimeAlarmMessage::Year(year) => memory_map.alarm.year = *year, - message::TimeAlarmMessage::Month(month) => memory_map.alarm.month = *month, - message::TimeAlarmMessage::Day(day) => memory_map.alarm.day = *day, - message::TimeAlarmMessage::Hour(hour) => memory_map.alarm.hour = *hour, - message::TimeAlarmMessage::Minute(minute) => memory_map.alarm.minute = *minute, - message::TimeAlarmMessage::Second(second) => memory_map.alarm.second = *second, - message::TimeAlarmMessage::Valid(valid) => memory_map.alarm.valid = *valid, - message::TimeAlarmMessage::Daylight(daylight) => memory_map.alarm.daylight = *daylight, - message::TimeAlarmMessage::Res1(res1) => memory_map.alarm.res1 = *res1, - message::TimeAlarmMessage::Milli(milli) => memory_map.alarm.milli = *milli, - message::TimeAlarmMessage::TimeZone(time_zone) => memory_map.alarm.time_zone = *time_zone, - message::TimeAlarmMessage::Res2(res2) => memory_map.alarm.res2 = *res2, - message::TimeAlarmMessage::AlarmStatus(alarm_status) => memory_map.alarm.alarm_status = *alarm_status, - message::TimeAlarmMessage::AcTimeVal(ac_time_val) => memory_map.alarm.ac_time_val = *ac_time_val, - message::TimeAlarmMessage::DcTimeVal(dc_time_val) => memory_map.alarm.dc_time_val = *dc_time_val, - } -} - /// Helper macro to simplify the conversion of memory map to message macro_rules! into_message { ($offset:ident, $length:ident, $member:expr, $msg:expr) => { @@ -410,99 +387,6 @@ pub fn mem_map_to_thermal_msg( } } -/// Convert from memory map offset and length to time alarm message -/// Modifies offset and length -pub fn mem_map_to_time_alarm_msg( - memory_map: &structure::ECMemory, - offset: &mut usize, - length: &mut usize, -) -> Result { - let local_offset = *offset - offset_of!(structure::ECMemory, alarm); - - if local_offset == offset_of!(structure::TimeAlarm, events) { - into_message!( - offset, - length, - memory_map.alarm.events, - message::TimeAlarmMessage::Events - ); - } else if local_offset == offset_of!(structure::TimeAlarm, capability) { - into_message!( - offset, - length, - memory_map.alarm.capability, - message::TimeAlarmMessage::Capability - ); - } else if local_offset == offset_of!(structure::TimeAlarm, year) { - into_message!(offset, length, memory_map.alarm.year, message::TimeAlarmMessage::Year); - } else if local_offset == offset_of!(structure::TimeAlarm, month) { - into_message!(offset, length, memory_map.alarm.month, message::TimeAlarmMessage::Month); - } else if local_offset == offset_of!(structure::TimeAlarm, day) { - into_message!(offset, length, memory_map.alarm.day, message::TimeAlarmMessage::Day); - } else if local_offset == offset_of!(structure::TimeAlarm, hour) { - into_message!(offset, length, memory_map.alarm.hour, message::TimeAlarmMessage::Hour); - } else if local_offset == offset_of!(structure::TimeAlarm, minute) { - into_message!( - offset, - length, - memory_map.alarm.minute, - message::TimeAlarmMessage::Minute - ); - } else if local_offset == offset_of!(structure::TimeAlarm, second) { - into_message!( - offset, - length, - memory_map.alarm.second, - message::TimeAlarmMessage::Second - ); - } else if local_offset == offset_of!(structure::TimeAlarm, valid) { - into_message!(offset, length, memory_map.alarm.valid, message::TimeAlarmMessage::Valid); - } else if local_offset == offset_of!(structure::TimeAlarm, daylight) { - into_message!( - offset, - length, - memory_map.alarm.daylight, - message::TimeAlarmMessage::Daylight - ); - } else if local_offset == offset_of!(structure::TimeAlarm, res1) { - into_message!(offset, length, memory_map.alarm.res1, message::TimeAlarmMessage::Res1); - } else if local_offset == offset_of!(structure::TimeAlarm, milli) { - into_message!(offset, length, memory_map.alarm.milli, message::TimeAlarmMessage::Milli); - } else if local_offset == offset_of!(structure::TimeAlarm, time_zone) { - into_message!( - offset, - length, - memory_map.alarm.time_zone, - message::TimeAlarmMessage::TimeZone - ); - } else if local_offset == offset_of!(structure::TimeAlarm, res2) { - into_message!(offset, length, memory_map.alarm.res2, message::TimeAlarmMessage::Res2); - } else if local_offset == offset_of!(structure::TimeAlarm, alarm_status) { - into_message!( - offset, - length, - memory_map.alarm.alarm_status, - message::TimeAlarmMessage::AlarmStatus - ); - } else if local_offset == offset_of!(structure::TimeAlarm, ac_time_val) { - into_message!( - offset, - length, - memory_map.alarm.ac_time_val, - message::TimeAlarmMessage::AcTimeVal - ); - } else if local_offset == offset_of!(structure::TimeAlarm, dc_time_val) { - into_message!( - offset, - length, - memory_map.alarm.dc_time_val, - message::TimeAlarmMessage::DcTimeVal - ); - } else { - Err(Error::InvalidLocation) - } -} - #[cfg(test)] mod tests { use super::*; @@ -998,209 +882,4 @@ mod tests { let res = mem_map_to_thermal_msg(&memory_map, &mut offset, &mut length); assert!(res.is_err() && res.unwrap_err() == Error::InvalidLocation); } - - #[test] - fn test_mem_map_to_time_alarm_msg() { - use crate::ec_type::message::TimeAlarmMessage; - use crate::ec_type::structure::{ECMemory, TimeAlarm}; - - let memory_map = ECMemory { - alarm: TimeAlarm { - events: 1, - capability: 2, - year: 2025, - month: 3, - day: 12, - hour: 10, - minute: 30, - second: 45, - valid: 1, - daylight: 0, - res1: 0, - milli: 500, - time_zone: 1, - res2: 0, - alarm_status: 1, - ac_time_val: 100, - dc_time_val: 200, - }, - ..Default::default() - }; - - let mut offset = offset_of!(ECMemory, alarm); - let mut length = size_of::(); - - test_field!( - memory_map, - offset, - length, - memory_map.alarm.events, - mem_map_to_time_alarm_msg, - TimeAlarmMessage::Events - ); - test_field!( - memory_map, - offset, - length, - memory_map.alarm.capability, - mem_map_to_time_alarm_msg, - TimeAlarmMessage::Capability - ); - test_field!( - memory_map, - offset, - length, - memory_map.alarm.year, - mem_map_to_time_alarm_msg, - TimeAlarmMessage::Year - ); - test_field!( - memory_map, - offset, - length, - memory_map.alarm.month, - mem_map_to_time_alarm_msg, - TimeAlarmMessage::Month - ); - test_field!( - memory_map, - offset, - length, - memory_map.alarm.day, - mem_map_to_time_alarm_msg, - TimeAlarmMessage::Day - ); - test_field!( - memory_map, - offset, - length, - memory_map.alarm.hour, - mem_map_to_time_alarm_msg, - TimeAlarmMessage::Hour - ); - test_field!( - memory_map, - offset, - length, - memory_map.alarm.minute, - mem_map_to_time_alarm_msg, - TimeAlarmMessage::Minute - ); - test_field!( - memory_map, - offset, - length, - memory_map.alarm.second, - mem_map_to_time_alarm_msg, - TimeAlarmMessage::Second - ); - test_field!( - memory_map, - offset, - length, - memory_map.alarm.valid, - mem_map_to_time_alarm_msg, - TimeAlarmMessage::Valid - ); - test_field!( - memory_map, - offset, - length, - memory_map.alarm.daylight, - mem_map_to_time_alarm_msg, - TimeAlarmMessage::Daylight - ); - test_field!( - memory_map, - offset, - length, - memory_map.alarm.res1, - mem_map_to_time_alarm_msg, - TimeAlarmMessage::Res1 - ); - test_field!( - memory_map, - offset, - length, - memory_map.alarm.milli, - mem_map_to_time_alarm_msg, - TimeAlarmMessage::Milli - ); - test_field!( - memory_map, - offset, - length, - memory_map.alarm.time_zone, - mem_map_to_time_alarm_msg, - TimeAlarmMessage::TimeZone - ); - test_field!( - memory_map, - offset, - length, - memory_map.alarm.res2, - mem_map_to_time_alarm_msg, - TimeAlarmMessage::Res2 - ); - test_field!( - memory_map, - offset, - length, - memory_map.alarm.alarm_status, - mem_map_to_time_alarm_msg, - TimeAlarmMessage::AlarmStatus - ); - test_field!( - memory_map, - offset, - length, - memory_map.alarm.ac_time_val, - mem_map_to_time_alarm_msg, - TimeAlarmMessage::AcTimeVal - ); - test_field!( - memory_map, - offset, - length, - memory_map.alarm.dc_time_val, - mem_map_to_time_alarm_msg, - TimeAlarmMessage::DcTimeVal - ); - - assert_eq!(length, 0); - } - - #[test] - fn test_mem_map_to_time_alarm_msg_error() { - use crate::ec_type::structure::{ECMemory, TimeAlarm}; - - let memory_map = ECMemory { - alarm: TimeAlarm { - events: 1, - capability: 2, - year: 2025, - month: 3, - day: 12, - hour: 10, - minute: 30, - second: 45, - valid: 1, - daylight: 0, - res1: 0, - milli: 500, - time_zone: 1, - res2: 0, - alarm_status: 1, - ac_time_val: 100, - dc_time_val: 200, - }, - ..Default::default() - }; - - let mut offset = offset_of!(ECMemory, alarm) + 1; - let mut length = size_of::(); - - let res = mem_map_to_time_alarm_msg(&memory_map, &mut offset, &mut length); - assert!(res.is_err() && res.unwrap_err() == Error::InvalidLocation); - } } diff --git a/embedded-service/src/ec_type/protocols/mctp.rs b/embedded-service/src/ec_type/protocols/mctp.rs index e2fac103..e672887e 100644 --- a/embedded-service/src/ec_type/protocols/mctp.rs +++ b/embedded-service/src/ec_type/protocols/mctp.rs @@ -80,6 +80,7 @@ pub fn handle_mctp_header( 2 => crate::comms::EndpointID::Internal(crate::comms::Internal::Battery), 3 => crate::comms::EndpointID::Internal(crate::comms::Internal::Thermal), 4 => crate::comms::EndpointID::Internal(crate::comms::Internal::Debug), + 5 => crate::comms::EndpointID::Internal(crate::comms::Internal::TimeAlarm), // TODO should this be an enum and/or should we implement EndpointId::from_mctp_id() or something? _ => return Err(MctpError::InvalidDestinationEndpoint), }; @@ -140,6 +141,7 @@ pub fn build_mctp_header( crate::comms::EndpointID::Internal(crate::comms::Internal::Battery) => ret[6] = 2, crate::comms::EndpointID::Internal(crate::comms::Internal::Thermal) => ret[6] = 3, crate::comms::EndpointID::Internal(crate::comms::Internal::Debug) => ret[6] = 4, + crate::comms::EndpointID::Internal(crate::comms::Internal::TimeAlarm) => ret[6] = 5, // TODO I don't love the duplication here - should we consolidate somewhere? _ => return Err(MctpError::InvalidDestinationEndpoint), } diff --git a/embedded-service/src/ec_type/structure.rs b/embedded-service/src/ec_type/structure.rs index 05b3c2e6..7d17b006 100644 --- a/embedded-service/src/ec_type/structure.rs +++ b/embedded-service/src/ec_type/structure.rs @@ -1,5 +1,6 @@ //! EC Internal Data Structures +// TODO need to rev? do we need to update yaml? #[allow(missing_docs)] pub const EC_MEMMAP_VERSION: Version = Version { major: 0, @@ -34,29 +35,6 @@ pub struct Capabilities { pub res0: u16, } -#[allow(missing_docs)] -#[repr(C, packed)] -#[derive(Clone, Copy, Debug, Default)] -pub struct TimeAlarm { - pub events: u32, - pub capability: u32, - pub year: u16, - pub month: u8, - pub day: u8, - pub hour: u8, - pub minute: u8, - pub second: u8, - pub valid: u8, - pub daylight: u8, - pub res1: u8, - pub milli: u16, - pub time_zone: u16, - pub res2: u16, - pub alarm_status: u32, - pub ac_time_val: u32, - pub dc_time_val: u32, -} - #[allow(missing_docs)] #[repr(C, packed)] #[derive(Clone, Copy, Debug, Default)] @@ -125,7 +103,6 @@ pub struct ECMemory { pub ver: Version, pub caps: Capabilities, pub notif: Notifications, - pub alarm: TimeAlarm, pub batt: Battery, pub therm: Thermal, } diff --git a/espi-service/src/espi_service.rs b/espi-service/src/espi_service.rs index d470e732..8ca6868f 100644 --- a/espi-service/src/espi_service.rs +++ b/espi-service/src/espi_service.rs @@ -66,10 +66,6 @@ impl Service<'_, '_> { && offset < offset_of!(ec_type::structure::ECMemory, therm) + size_of::() { self.route_to_thermal_service(&mut offset, &mut length).await?; - } else if offset >= offset_of!(ec_type::structure::ECMemory, alarm) - && offset < offset_of!(ec_type::structure::ECMemory, alarm) + size_of::() - { - self.route_to_time_alarm_service(&mut offset, &mut length).await?; } } @@ -116,26 +112,6 @@ impl Service<'_, '_> { Ok(()) } - async fn route_to_time_alarm_service(&self, offset: &mut usize, length: &mut usize) -> Result<(), ec_type::Error> { - let msg = { - let memory_map = self - .ec_memory - .try_lock() - .expect("Messages handled one after another, should be infallible."); - ec_type::mem_map_to_time_alarm_msg(&memory_map, offset, length)? - }; - - comms::send( - EndpointID::External(External::Host), - EndpointID::Internal(Internal::TimeAlarm), - &msg, - ) - .await - .unwrap(); - - Ok(()) - } - async fn wait_for_subsystem_msg(&self) -> HostMsgInternal<'_> { self.host_tx_queue.receive().await } @@ -245,8 +221,6 @@ impl comms::MailboxDelegate for Service<'_, '_> { ec_type::update_battery_section(msg, &mut memory_map); } else if let Some(msg) = message.data.get::() { ec_type::update_thermal_section(msg, &mut memory_map); - } else if let Some(msg) = message.data.get::() { - ec_type::update_time_alarm_section(msg, &mut memory_map); } else { return Err(comms::MailboxDelegateError::MessageNotFound); } diff --git a/time-alarm-service/Cargo.toml b/time-alarm-service/Cargo.toml new file mode 100644 index 00000000..32fef2d9 --- /dev/null +++ b/time-alarm-service/Cargo.toml @@ -0,0 +1,35 @@ +[package] +name = "time-alarm-service" +description = "Time and Alarm service implementation" +version.workspace = true +edition.workspace = true +license.workspace = true +repository.workspace = true + +[dependencies] +# TODO review these and figure out if we actually need them +bytemuck.workspace = true +defmt = { workspace = true, optional = true } +log = { workspace = true, optional = true } +embassy-executor.workspace = true +embassy-futures.workspace = true +embassy-sync.workspace = true +embassy-time.workspace = true +embedded-hal-async.workspace = true +embedded-hal.workspace = true +embedded-mcu-hal.workspace = true +embedded-services.workspace = true +heapless.workspace = true +uuid.workspace = true + +[features] +defmt = [ + "dep:defmt", + "embedded-services/defmt", + "embassy-time/defmt", + "embassy-sync/defmt", + "embassy-executor/defmt", +] + +[lints] +workspace = true diff --git a/time-alarm-service/src/acpi_timestamp.rs b/time-alarm-service/src/acpi_timestamp.rs new file mode 100644 index 00000000..89c6d9ad --- /dev/null +++ b/time-alarm-service/src/acpi_timestamp.rs @@ -0,0 +1,217 @@ +use embedded_mcu_hal::time::{Datetime, UncheckedDatetime}; + +use crate::TimeAlarmError; + +// Timestamp structure as specified in the ACPI spec. Must be exactly this layout. +// TODO are there any endianness shenanigans associated with bytemuck here? +#[repr(C)] +#[derive(bytemuck::Pod, bytemuck::Zeroable, Copy, Clone, Debug)] +struct RawAcpiTimestamp { + // Year: 1900 - 9999 + year: u16, + + // Month: 1 - 12 + month: u8, + + // Day: 1 - 31 + day: u8, + + // Hour: 0 - 23 + hour: u8, + + // Minute: 0 - 59 + minute: u8, + + // Second: 0 - 59. Leap seconds are not supported. + second: u8, + + // For _GRT, 0 = time is not valid (request failed), 1 = time is valid. For _SRT, this is padding and should be 0. + valid_or_padding: u8, + + // Millseconds: 1-1000. Leap seconds are not supported. // TODO ACPI spec says 1-1000, but surely it should be 0-999? what's up with that? We may need to do some translation if this isn't just a typo in the spec + milliseconds: u16, + + // Time zone: -1440 to 1440 in minutes from UTC, or 2047 if unspecified + time_zone: i16, + + // 1 = daylight savings time in effect, 0 = standard time + daylight: u8, + + // Reserved, must be 0 + _padding: [u8; 3] +} + +impl RawAcpiTimestamp { + // Try to interpret a byte slice as an AcpiTimestamp. The slice must be exactly 16 bytes long. + // Validity of the fields is not checked here. + pub fn try_from_bytes(bytes: &[u8]) -> Result { + bytemuck::try_pod_read_unaligned(bytes).map_err(|_| TimeAlarmError::InvalidArgument) + } + + // Get a byte slice representing this AcpiTimestamp. + pub fn as_bytes(&self) -> &[u8; core::mem::size_of::()] /* 16 */ { + bytemuck::bytes_of(self).try_into().expect("Should never fail because we know the size of AcpiTimestamp at compile time") + } + + #[allow(dead_code)] // TODO either use this or remove it. I think we only need it if we want to return an invalid timestamp to the host rather than an error code that the ASL translates into the invalid timestamp structure to report failure. + pub fn invalid() -> Self { + Self { + year: 0, + month: 0, + day: 0, + hour: 0, + minute: 0, + second: 0, + valid_or_padding: 0, // invalid + milliseconds: 0, + time_zone: 0, + daylight: 0, + _padding: [0; 3], + } + } +} + +// TODO s/AcpiTimestamp/AcpiTime/? +impl From<&AcpiTimestamp> for RawAcpiTimestamp { + fn from(ts: &AcpiTimestamp) -> Self { + Self { + year: ts.datetime.year(), + month: ts.datetime.month(), + day: ts.datetime.day(), + hour: ts.datetime.hour(), + minute: ts.datetime.minute(), + second: ts.datetime.second(), + valid_or_padding: 1, // valid + milliseconds: (ts.datetime.nanoseconds() / 1_000_000).try_into().expect("Datetime::nanoseconds() is capped at 10^9 and therefore should always divide by 10^6 into something that fits in u16"), + time_zone: ts.time_zone.into(), + daylight: ts.dst_status.into(), + _padding: [0; 3], + } + } +} + +// ------------------------------------------------- + +#[derive(Copy, Clone, Debug)] +pub enum AcpiDaylightSavingsTimeStatus { + /// Daylight savings time is not observed in this timezone. + NotObserved, + + /// Daylight savings time is observed in this timezone, but the current time has not been adjusted for it. + NotAdjusted, + + /// Daylight savings time is observed in this timezone, and the current time has been adjusted for it. + Adjusted +} + +impl TryFrom for AcpiDaylightSavingsTimeStatus { + type Error = TimeAlarmError; + + fn try_from(value: u8) -> Result { + match value { + 0 => Ok(Self::NotObserved), + 1 => Ok(Self::NotAdjusted), + 3 => Ok(Self::Adjusted), + _ => Err(TimeAlarmError::InvalidArgument) + } + } +} + +impl From for u8 { + fn from(val: AcpiDaylightSavingsTimeStatus) -> Self { + match val { + AcpiDaylightSavingsTimeStatus::NotObserved => 0, + AcpiDaylightSavingsTimeStatus::NotAdjusted => 1, + AcpiDaylightSavingsTimeStatus::Adjusted => 3, + } + } +} + +// ------------------------------------------------- + +#[derive(Copy, Clone, Debug)] +pub struct AcpiTimeZoneOffset { + minutes_from_utc: i16 // minutes from UTC +} + +impl AcpiTimeZoneOffset { + pub fn new(minutes_from_utc: i16) -> Result { + if !(-1440..=1440).contains(&minutes_from_utc) { + return Err(TimeAlarmError::InvalidArgument); + } + Ok(Self { + minutes_from_utc + }) + } + + pub fn minutes_from_utc(&self) -> i16 { + self.minutes_from_utc + } +} + +#[derive(Copy, Clone, Debug)] +pub enum AcpiTimeZone { + /// The time zone is not specified and no relation to UTC can be inferred. + Unknown, + + /// The time zone is this many minutes from UTC. + MinutesFromUtc(AcpiTimeZoneOffset) +} + +impl TryFrom for AcpiTimeZone { + type Error = TimeAlarmError; + + fn try_from(value: i16) -> Result { + if value == 2047 { + Ok(Self::Unknown) + } else { + Ok(Self::MinutesFromUtc(AcpiTimeZoneOffset::new(value)?)) + } + } +} + +impl From for i16 { + fn from(val: AcpiTimeZone) -> Self { + match val { + AcpiTimeZone::Unknown => 2047, + AcpiTimeZone::MinutesFromUtc(offset) => offset.minutes_from_utc() + } + } +} + +// ------------------------------------------------- + +pub(crate) struct AcpiTimestamp { + pub datetime: Datetime, + pub time_zone: AcpiTimeZone, + pub dst_status: AcpiDaylightSavingsTimeStatus +} + +impl AcpiTimestamp { + pub fn as_bytes(&self) -> [u8; core::mem::size_of::()] /* 16 */ { + *RawAcpiTimestamp::from(self).as_bytes() + } + + pub fn try_from_bytes(bytes: &[u8]) -> Result { + let raw = RawAcpiTimestamp::try_from_bytes(bytes)?; + + Ok(Self { + datetime: Datetime::new(UncheckedDatetime { + year: raw.year, + month: raw.month, + day: raw.day, + hour: raw.hour, + minute: raw.minute, + second: raw.second, + nanosecond: (raw.milliseconds as u32) * 1_000_000, + }).map_err(|_| TimeAlarmError::InvalidArgument)?, // TODO verify that this is sufficient validation + time_zone: raw.time_zone.try_into()?, + dst_status: raw.daylight.try_into()?, + }) + } + + #[allow(dead_code)] // TODO either use this or remove it. I think we only need it if we want to return an invalid timestamp to the host rather than an error code that the ASL translates into the invalid timestamp structure to report failure. + pub fn invalid_timestamp_bytes() -> [u8; core::mem::size_of::()] /* 16 */ { + *RawAcpiTimestamp::invalid().as_bytes() + } +} diff --git a/time-alarm-service/src/lib.rs b/time-alarm-service/src/lib.rs new file mode 100644 index 00000000..95379af6 --- /dev/null +++ b/time-alarm-service/src/lib.rs @@ -0,0 +1,600 @@ +#![no_std] +// #![allow(unused)] // TODO remove before checkin + +use core::any::Any; +use core::array::TryFromSliceError; +use core::borrow::Borrow; +use core::cell::RefCell; +use core::panic; +use embassy_futures::select::{Either, select}; +use embassy_sync::blocking_mutex::Mutex; +use embassy_sync::channel::Channel; +use embassy_sync::once_lock::OnceLock; +use embassy_sync::signal::Signal; +use embedded_services::ec_type::message::AcpiMsgComms; +use embedded_services::{GlobalRawMutex, comms::MailboxDelegateError}; + +use embedded_mcu_hal::NvramStorage; +use embedded_mcu_hal::time::{Datetime, DatetimeClock, DatetimeClockError}; +use embedded_services::{comms, error, info, trace}; + +mod acpi_timestamp; +use acpi_timestamp::{AcpiDaylightSavingsTimeStatus, AcpiTimeZone, AcpiTimestamp}; + +mod timer; +use timer::Timer; + +// ------------------------------------------------- + +#[derive(Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum TimeAlarmError { + UnknownCommand, + DoubleInitError, + MailboxFullError, + InvalidAcpiTimerId, + InvalidArgument, + ClockError(DatetimeClockError), +} + +// TODO may want to map other error types here +impl From for MailboxDelegateError { + fn from(_error: TimeAlarmError) -> Self { + MailboxDelegateError::InvalidData + } +} + +impl From for TimeAlarmError { + fn from(_error: TryFromSliceError) -> Self { + TimeAlarmError::InvalidArgument + } +} + +// TODO should an impl like this exist in the MailboxDelegateError crate? For now use map_err +// impl From> for MailboxDelegateError { +// fn from(_error: TrySendError) -> Self { +// match _error { +// TrySendError::Full(()) => MailboxDelegateError::BufferFull, +// _ => MailboxDelegateError::Other // TODO is this the right mapping? +// } +// } +// } + +// ------------------------------------------------- + +// Timer ID as defined in the ACPI spec. +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum AcpiTimerId { + AcPower, + DcPower, +} + +impl AcpiTimerId { + // Given a byte slice, attempts to parse an AcpiTimerId from the first 4 bytes. + // Returns the parsed AcpiTimerId and a slice of the remaining bytes. + fn try_from_bytes(bytes: &'_ [u8]) -> Result<(Self, &'_ [u8]), TimeAlarmError> { + const SIZE_BYTES: usize = core::mem::size_of::(); + let id = u32::from_le_bytes( + bytes + .get(0..SIZE_BYTES) + .ok_or(TimeAlarmError::InvalidArgument)? + .try_into() + .map_err(|_| TimeAlarmError::InvalidArgument)?, + ); + + Ok((AcpiTimerId::try_from(id)?, &bytes[SIZE_BYTES..])) + } + + fn get_other_timer_id(&self) -> Self { + match self { + AcpiTimerId::AcPower => AcpiTimerId::DcPower, + AcpiTimerId::DcPower => AcpiTimerId::AcPower, + } + } +} + +impl TryFrom for AcpiTimerId { + type Error = TimeAlarmError; + + fn try_from(value: u32) -> Result { + match value { + 0 => Ok(AcpiTimerId::AcPower), + 1 => Ok(AcpiTimerId::DcPower), + _ => Err(TimeAlarmError::InvalidAcpiTimerId), + } + } +} + +// ------------------------------------------------- + +// TODO is there some way to do this with an enum that's more readable? I can't figure out how to make it store in +// a u32 directly and impossible to do Some(u32::MAX) without just adding a panic or whatever. Maybe that's the +// right thing to do? but then I think we end up spending a byte on the enum discriminant, which seems wasteful? +// Is there some way to tell the compiler it can get away with inferring discriminant from the value? +// +#[derive(Clone, Copy, Debug, PartialEq)] +struct AlarmTimerSeconds(u32); +impl AlarmTimerSeconds { + pub const DISABLED: Self = Self(u32::MAX); +} + +impl Default for AlarmTimerSeconds { + fn default() -> Self { + Self::DISABLED + } +} + +#[derive(Clone, Copy, Debug, PartialEq)] +struct AlarmExpiredWakePolicy(u32); +impl AlarmExpiredWakePolicy { + #[allow(dead_code)] + pub const INSTANTLY: Self = Self(0); + pub const NEVER: Self = Self(u32::MAX); +} + +impl Default for AlarmExpiredWakePolicy { + fn default() -> Self { + Self::NEVER + } +} + +// ------------------------------------------------- + +/// Represents an ACPI Time and Alarm Device command. +/// See ACPI Specification 6.4, Section 9.18 "Time and Alarm Device" for details on semantics. +// TODO should these all take an 'endpoint to respond to' parameter? Right now we're assuming that the only thing that will send us commands is the host, +// which may not be the case? +#[rustfmt::skip] +enum AcpiTimeAlarmDeviceCommand { + GetCapabilities, // 0: _GCP --> u32 (bitmask), failure: infallible + GetRealTime, // 1: _GRT --> AcpiTimestamp, failure: valid bit = 0 in returned timestamp + SetRealTime(AcpiTimestamp), // 2: _SRT --> u32 (bool), failure: u32::MAX + GetWakeStatus(AcpiTimerId), // 3: _GWS --> u32 (bitmask), failure: infallible + ClearWakeStatus(AcpiTimerId), // 4: _CWS --> u32 (bool), failure: 1 + SetExpiredTimerPolicy(AcpiTimerId, AlarmExpiredWakePolicy), // 5: _STP --> u32 (bool), failure: 1 + SetTimerValue(AcpiTimerId, AlarmTimerSeconds), // 6: _STV --> u32 (bool), failure: 1, + GetExpiredTimerPolicy(AcpiTimerId), // 7: _TIP --> u32 (AlarmExpiredWakePolicy) failure: infallible + GetTimerValue(AcpiTimerId), // 8: _TIV --> u32 (AlarmTimerSeconds), failure: infallible, u32::MAX if disabled +} + +impl AcpiTimeAlarmDeviceCommand { + fn try_from_bytes(bytes: &[u8]) -> Result { + // TODO for now, we assume that the message structure is followed by the ACPI arguments in order as specified in + // the ACPI spec https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/09_ACPI-Defined_Devices_and_Device-Specific_Objects/ACPIdefined_Devices_and_DeviceSpecificObjects.html#tiv-timer-values + // For example, _STP will be [0x05, , ] + // We need to make sure this is actually how we implement it over eSPI, or adapt to match what eSPI actually sends us. + + const COMMAND_CODE_SIZE_BYTES: usize = core::mem::size_of::(); + let command_code = u32::from_le_bytes(bytes.try_into()?); + + let bytes = bytes.get(COMMAND_CODE_SIZE_BYTES..) + .expect("Should never fail because if there were less than 4 bytes, u32::from_le_bytes would have failed. If there were exactly four bytes, this will return an empty slice, not None."); + + // TODO I feel like these numbers should be associated with the enum somehow, maybe inferred from ordinal position in the enum? + // but I don't know if it's possible to do that in Rust. Figure out if there's a clean way to do it, maybe some to-int + // macro or something? + match command_code { + 0 => Ok(AcpiTimeAlarmDeviceCommand::GetCapabilities), // TODO there's a case to be made that this should just be a hardcoded constant in the ASL - if we do that, may want to renumber + 1 => Ok(AcpiTimeAlarmDeviceCommand::GetRealTime), + 2 => Ok(AcpiTimeAlarmDeviceCommand::SetRealTime(AcpiTimestamp::try_from_bytes( + bytes, + )?)), + _ => { + let (timer_id, bytes) = AcpiTimerId::try_from_bytes(bytes)?; + match command_code { + 3 => Ok(AcpiTimeAlarmDeviceCommand::GetWakeStatus(timer_id)), + 4 => Ok(AcpiTimeAlarmDeviceCommand::ClearWakeStatus(timer_id)), + 5 => Ok(AcpiTimeAlarmDeviceCommand::SetExpiredTimerPolicy( + timer_id, + AlarmExpiredWakePolicy(u32::from_le_bytes(bytes.try_into()?)), + )), + 6 => Ok(AcpiTimeAlarmDeviceCommand::SetTimerValue( + timer_id, + AlarmTimerSeconds(u32::from_le_bytes(bytes.try_into()?)), + )), + 7 => Ok(AcpiTimeAlarmDeviceCommand::GetExpiredTimerPolicy(timer_id)), + 8 => Ok(AcpiTimeAlarmDeviceCommand::GetTimerValue(timer_id)), + _ => Err(TimeAlarmError::UnknownCommand), + } + } + } + } +} + +// ------------------------------------------------- + +struct TimeZoneData { + // Storage used to back the timezone and DST settings. + // TODO can we achieve this without dyn? I think this would require making the service generic over the clock type, which means no static global SERVICE? + storage: &'static mut dyn NvramStorage<'static, u32>, +} + +// TODO is there a cleaner way to do this sort of bitpacking? I really just want std::bit_cast< struct { int16_t, uint8_t, uint8_t }> +impl TimeZoneData { + const TZ_MASK: u32 = 0x0000FFFF; + const DST_MASK: u32 = 0x00FF0000; + const DST_SHIFT: u32 = 16; + + pub fn new(storage: &'static mut dyn NvramStorage<'static, u32>) -> Self { + Self { storage } + } + + pub fn set_data(&mut self, tz: AcpiTimeZone, dst: AcpiDaylightSavingsTimeStatus) { + let tz_i16: i16 = tz.into(); + let tz_u32 = tz_i16 as u32; + + let dst_u8: u8 = dst.into(); + let dst_u32 = (dst_u8 as u32) << Self::DST_SHIFT; + + self.storage.write(tz_u32 | dst_u32); + } + + pub fn get_data(&self) -> (AcpiTimeZone, AcpiDaylightSavingsTimeStatus) { + let tz_u16 = (self.storage.read() & Self::TZ_MASK) as u16; + let tz_i16 = tz_u16 as i16; + let tz = AcpiTimeZone::try_from(tz_i16).unwrap_or(AcpiTimeZone::Unknown); + + let dst_u8 = ((self.storage.read() & Self::DST_MASK) >> Self::DST_SHIFT) as u8; + let dst = AcpiDaylightSavingsTimeStatus::try_from(dst_u8).unwrap_or(AcpiDaylightSavingsTimeStatus::NotObserved); + + (tz, dst) + } +} + +// ------------------------------------------------- + +struct ClockState { + // TODO can we achieve this without dyn? I think this would require making the service generic over the clock type, + // which means no static global SERVICE / memory allocation for the SERVICE would have to be done by the caller + // of init(), which might interfere with the requirement that we can pass static references to it to the comms + // system? Need to investigate options for that + // + datetime_clock: &'static mut dyn DatetimeClock, + tz_data: TimeZoneData, +} + +// TODO see if there's some sort of bitfield crate that can make this cleaner +#[derive(Copy, Clone, Debug, Default)] +struct TimerStatus { + timer_expired: bool, + timer_triggered_wake: bool, +} + +impl From for u32 { + fn from(value: TimerStatus) -> Self { + let mut result = 0u32; + if value.timer_expired { + result |= 0x1; + } + if value.timer_triggered_wake { + result |= 0x2; + } + result + } +} + +// ------------------------------------------------- + +struct Timers { + ac_timer: Timer, + dc_timer: Timer, +} + +impl Timers { + fn get_timer(&self, timer: AcpiTimerId) -> &Timer { + match timer { + AcpiTimerId::AcPower => &self.ac_timer, + AcpiTimerId::DcPower => &self.dc_timer, + } + } + + // TODO it's a bit unfortunate that we have to pass these in individually like this, but I don't see another way to get compile-time + // checking of the number of registers passed in without the experimental split_array_mut - ask if theres' a better way + fn new( + ac_expiration_storage: &'static mut dyn NvramStorage<'static, u32>, + ac_policy_storage: &'static mut dyn NvramStorage<'static, u32>, + dc_expiration_storage: &'static mut dyn NvramStorage<'static, u32>, + dc_policy_storage: &'static mut dyn NvramStorage<'static, u32>, + ) -> Self { + Self { + ac_timer: Timer::new(true, ac_expiration_storage, ac_policy_storage), + dc_timer: Timer::new(false, dc_expiration_storage, dc_policy_storage), + } + } +} + +// ------------------------------------------------- + +pub struct Service { + endpoint: comms::Endpoint, + + // ACPI messages from the host are sent through this channel. + acpi_channel: Channel, + + clock_state: Mutex>, + + power_source_signal: Signal, // TODO figure out how to feed this thing + + timers: Timers, +} + +// TODO can we template this on the clock/storage type? +impl Service { + pub fn new( + backing_clock: &'static mut impl DatetimeClock, + tz_storage: &'static mut dyn NvramStorage<'static, u32>, + ac_expiration_storage: &'static mut dyn NvramStorage<'static, u32>, + ac_policy_storage: &'static mut dyn NvramStorage<'static, u32>, + dc_expiration_storage: &'static mut dyn NvramStorage<'static, u32>, + dc_policy_storage: &'static mut dyn NvramStorage<'static, u32>, + ) -> Self { + Service { + endpoint: comms::Endpoint::uninit(comms::EndpointID::Internal(comms::Internal::TimeAlarm)), + acpi_channel: Channel::new(), + clock_state: Mutex::new(RefCell::new(ClockState { + datetime_clock: backing_clock, + tz_data: TimeZoneData::new(tz_storage), + })), + power_source_signal: Signal::new(), + timers: Timers::new( + ac_expiration_storage, + ac_policy_storage, + dc_expiration_storage, + dc_policy_storage, + ), + } + } + + pub async fn handle_requests(&self) { + loop { + let acpi_command = self.acpi_channel.receive(); + let power_source_change = self.power_source_signal.wait(); + + match select(acpi_command, power_source_change).await { + Either::First(acpi_command) => { + self.handle_acpi_command(acpi_command).await.unwrap_or_else(|e| { + error!("Error handling ACPI command: {:?}", e); + // TODO what should happen if this fails? How do we communicate failure back to the host? + }); + } + Either::Second(new_power_source) => { + info!("Power source changed to {:?}", new_power_source); + + self.timers + .get_timer(new_power_source.get_other_timer_id()) + .set_active(false); + self.timers.get_timer(new_power_source).set_active(true); + } + } + } + } + + pub async fn handle_timer(&self, timer_id: AcpiTimerId) { + let timer = self.timers.get_timer(timer_id); + loop { + timer.wait_until_wake().await; + // TODO [SPEC_QUESTION] section 9.18.7 indicates that when a timer expires, both timers have their wake policies reset, + // but I can't find any similar rule for the actual timer value - that seems odd to me, verify that's actually how + // it's supposed to work + self.timers + .get_timer(timer_id.get_other_timer_id()) + .set_timer_wake_policy(AlarmExpiredWakePolicy::NEVER); + + todo!("Figure out how to signal a wake event to the host when this timer expires"); + } + } + + async fn send_acpi_response(&self, response: &impl Any) { + // TODO right now we're hardcoded to reply to the host, but I think anyone can send us a command - do we need to track + // the source endpoint of the command so we can respond to it specifically? What do other services do? + self.endpoint + .send(comms::EndpointID::External(comms::External::Host), response) + .await + .expect("send returns Result<(), Infallible>"); + } + + async fn handle_acpi_command(&self, command: AcpiTimeAlarmDeviceCommand) -> Result<(), TimeAlarmError> { + info!("Received Time-Alarm Device command: {:?}", command); + match command { + // TODO these all need to return a buffer on error. + // The size and shape of that buffer depend on the message type, so we probably can't punt to the caller unless we want to do the translation in the ASL? That seems cleaner to me, but maybe there's some reason not to do that? + AcpiTimeAlarmDeviceCommand::GetCapabilities => { + todo!( + "implement or remove GetCapabilities - if implemented, it'd return a constant, so there's a case to be made that it belongs wholly in the ASL" + ); + } + AcpiTimeAlarmDeviceCommand::GetRealTime => { + let time = self.clock_state.lock(|clock_state| { + let clock_state = clock_state.borrow(); + match clock_state.datetime_clock.get_current_datetime() { + // TODO figure out why type inference doesn't work with map_err and the ? operator + Ok(datetime) => { + let (time_zone, dst_status) = clock_state.tz_data.get_data(); + Ok(AcpiTimestamp { + datetime, + time_zone, + dst_status, + }) + } + Err(e) => Err(TimeAlarmError::ClockError(e)), + } + })?; + + self.send_acpi_response(&time.as_bytes()).await; + // TODO is this sufficient, or do we also need a 'success' / 'EOM' packet or something? + + Ok(()) + } + AcpiTimeAlarmDeviceCommand::SetRealTime(timestamp) => { + self.clock_state.lock(|clock_state| { + let mut clock_state = clock_state.borrow_mut(); + clock_state + .datetime_clock + .set_current_datetime(×tamp.datetime) + .map_err(TimeAlarmError::ClockError)?; + clock_state.tz_data.set_data(timestamp.time_zone, timestamp.dst_status); + + // TODO do we need to return a success code over espi? + // TODO do we need to adjust the timers based on the new time? check with ACPI spec + Ok(()) + }) + } + AcpiTimeAlarmDeviceCommand::GetWakeStatus(timer_id) => { + let status = self.timers.get_timer(timer_id).get_wake_status(); + let packed_status: u32 = status.into(); + self.send_acpi_response(&packed_status.to_le_bytes()).await; + // TODO is this sufficient, or do we also need a 'success' / 'EOM' packet or something? + + Ok(()) + } + AcpiTimeAlarmDeviceCommand::ClearWakeStatus(timer_id) => { + self.timers.get_timer(timer_id).clear_wake_status(); + // TODO do we need to return a success code over espi? + Ok(()) + } + AcpiTimeAlarmDeviceCommand::SetExpiredTimerPolicy(timer_id, timer_policy) => { + self.timers.get_timer(timer_id).set_timer_wake_policy(timer_policy); + // TODO do we need to return a success code over espi? + Ok(()) + } + AcpiTimeAlarmDeviceCommand::SetTimerValue(timer_id, timer_value) => { + let new_expiration_time = match timer_value { + AlarmTimerSeconds::DISABLED => None, + AlarmTimerSeconds(secs) => { + let current_time = self.clock_state.lock(|clock_state| { + let clock_state = clock_state.borrow(); + clock_state + .datetime_clock + .get_current_datetime() + .map_err(TimeAlarmError::ClockError) + })?; + + let expiration_time = + Datetime::from_unix_time_seconds(current_time.to_unix_time_seconds() + u64::from(secs)); // TODO why doesn't into() work here? + Some(expiration_time) + } + }; + + self.timers.get_timer(timer_id).set_expiration_time(new_expiration_time); + // TODO do we need to return a success code over espi? + + Ok(()) + } + AcpiTimeAlarmDeviceCommand::GetExpiredTimerPolicy(timer_id) => { + let wake_policy = self.timers.get_timer(timer_id).get_timer_wake_policy(); + self.send_acpi_response(&wake_policy.0.to_le_bytes()).await; + // TODO is this sufficient, or do we also need a 'success' / 'EOM' packet or something? + + Ok(()) + } + AcpiTimeAlarmDeviceCommand::GetTimerValue(timer_id) => { + let expiration_time = self.timers.get_timer(timer_id).get_expiration_time(); + + const ACPI_TIMER_DISABLED: u32 = u32::MAX; + let timer_wire_format: u32 = match expiration_time { + Some(expiration_time) => { + let current_time = self.clock_state.lock(|clock_state| { + let clock_state = clock_state.borrow(); + clock_state + .datetime_clock + .get_current_datetime() + .map_err(TimeAlarmError::ClockError) + })?; + + expiration_time.to_unix_time_seconds().saturating_sub(current_time.to_unix_time_seconds()).try_into().expect("Per the ACPI spec, timers are communicated in u32 seconds, so this shouldn't be able to overflow") + } + None => ACPI_TIMER_DISABLED, + }; + + self.send_acpi_response(&timer_wire_format.to_le_bytes()).await; + // TODO is this sufficient, or do we also need a 'success' / 'EOM' packet or something? + + Ok(()) + } + } + } +} + +impl comms::MailboxDelegate for Service { + fn receive(&self, _message: &comms::Message) -> Result<(), comms::MailboxDelegateError> { + trace!("Received message at time-alarm-service"); + + if let Some(msg) = _message.data.get::() { + let buffer_access = msg.payload.borrow(); + let buffer: &[u8] = buffer_access.borrow(); + + // TODO right now, if this fails, what happens? The TAD spec has different ways of reporting failure depending on the command, do we need to handle that here or is that in the ASL? + // TODO what is supposed to happen if there's e.g. an invalid timestamp? Is returning an error here sufficient, or do we need to send some kind of error response back to the host? + self.acpi_channel + .try_send(AcpiTimeAlarmDeviceCommand::try_from_bytes(&buffer[0..msg.payload_len])?) + .map_err(|_| MailboxDelegateError::BufferFull)?; + Ok(()) + } else { + Err(comms::MailboxDelegateError::InvalidData) + } + } +} + +static SERVICE: OnceLock = OnceLock::new(); // TODO is this really what we want? I'd love to have init return an instance instead, that would let us template over the clock type... unclear how that would work with register_endpoint, though + +// TODO figure out if there's a cleaner way to pass a bunch of these storage instances in without having a ton of parameters - tried taking a slice but hit some weird type issues that I'm punting on for now +pub async fn init( + spawner: &embassy_executor::Spawner, + backing_clock: &'static mut impl DatetimeClock, + tz_storage: &'static mut dyn NvramStorage<'static, u32>, + ac_expiration_storage: &'static mut dyn NvramStorage<'static, u32>, + ac_policy_storage: &'static mut dyn NvramStorage<'static, u32>, + dc_expiration_storage: &'static mut dyn NvramStorage<'static, u32>, + dc_policy_storage: &'static mut dyn NvramStorage<'static, u32>, +) -> Result<(), TimeAlarmError> { + info!("Starting time-alarm service task"); + + let service = SERVICE.get_or_init(|| { + Service::new( + backing_clock, + tz_storage, + ac_expiration_storage, + ac_policy_storage, + dc_expiration_storage, + dc_policy_storage, + ) + }); + + comms::register_endpoint(service, &service.endpoint) + .await + .map_err(|_| { + error!("Failed to register time-alarm service endpoint"); + TimeAlarmError::DoubleInitError // TODO if we impl from on error type, tear this out + })?; + + // TODO we need to subscribe to messages that tell us if we're on AC or DC power so we can decide which alarms to trigger - how do we do that? + + spawner.must_spawn(command_handler_task()); + spawner.must_spawn(timer_task(AcpiTimerId::AcPower)); + spawner.must_spawn(timer_task(AcpiTimerId::DcPower)); + + Ok(()) +} + +#[embassy_executor::task] +async fn command_handler_task() { + info!("Starting time-alarm service task"); + let service = SERVICE.get_or_init(|| panic!("should have already been initialized by init()")); + + service.handle_requests().await; +} + +#[embassy_executor::task] +async fn timer_task(timer_id: AcpiTimerId) { + info!("Starting time-alarm timer task"); + let service = SERVICE.get_or_init(|| panic!("should have already been initialized by init()")); + + service.handle_timer(timer_id).await; +} + +pub fn get_current_datetime() -> Option { + SERVICE + .try_get()? + .clock_state + .lock(|clock_state| clock_state.borrow().datetime_clock.get_current_datetime().ok()) +} diff --git a/time-alarm-service/src/timer.rs b/time-alarm-service/src/timer.rs new file mode 100644 index 00000000..d1a8f213 --- /dev/null +++ b/time-alarm-service/src/timer.rs @@ -0,0 +1,336 @@ +use crate::{AlarmExpiredWakePolicy, TimerStatus}; +use core::cell::RefCell; +use embassy_futures::select::{Either, select}; +use embassy_sync::{blocking_mutex::Mutex, signal::Signal}; +use embedded_mcu_hal::NvramStorage; +use embedded_mcu_hal::time::Datetime; +use embedded_services::GlobalRawMutex; + +/// Represents where in the timer lifecycle the current timer is +#[derive(Copy, Clone, Debug, PartialEq)] +enum WakeState { + /// Timer is not active + Clear, + /// Timer is active and programmed with the original expiration time + Armed, + /// Timer is active but expired when on the wrong power source + /// Includes the time at which we started running down the policy delay and the number of seconds that had already elapsed on the policy delay when we started waiting + ExpiredWaitingForPolicyDelay(Datetime, u32), + /// Timer is active and waiting for power source to be consistent with the timer type. + /// Includes the number of seconds that we've spent in the ExpiredWaitingForPolicyDelay state for so far. + ExpiredWaitingForPowerSource(u32), + // Expired while the policy was set to NEVER, so the timer is effectively dead until reprogrammed + ExpiredOrphaned, +} + +// TODO do we want to do something like this to make persistent storage easier to read? +// struct PersistentAlarmExpiredWakePolicy { +// wake_policy: AlarmExpiredWakePolicy, +// storage: &'static mut dyn NvramStorage<'static, u32>, +// } +// struct PersistentExpirationTime { +// expiration_time: Option, +// storage: &'static mut dyn NvramStorage<'static, u32>, +// } + +struct TimerState { + /// When the timer is programmed to expire, or None if the timer is not set + /// This can't be part of the wake_state because we need to be able to report its value for _CWS even when the timer has expired and + /// we're handling the power source policy. + expiration_time_storage: &'static mut dyn NvramStorage<'static, u32>, + + // Persistent storage for the AlarmExpiredWakePolicy + wake_policy_storage: &'static mut dyn NvramStorage<'static, u32>, + + wake_state: WakeState, + + timer_status: TimerStatus, + + // Whether or not this timer is currently active (i.e. the system is on the power source this timer manages) + // Even if it's not active, it still counts down if it's programmed - it just won't trigger a wake event if it expires while inactive. + is_active: bool, +} + +impl TimerState { + const NO_EXPIRATION_TIME: u32 = u32::MAX; + + fn get_timer_wake_policy(&self) -> AlarmExpiredWakePolicy { + AlarmExpiredWakePolicy(self.wake_policy_storage.read()) + } + + fn set_timer_wake_policy(&mut self, wake_policy: AlarmExpiredWakePolicy) { + self.wake_policy_storage.write(wake_policy.0); + } + + fn get_expiration_time(&self) -> Option { + match self.expiration_time_storage.read() { + Self::NO_EXPIRATION_TIME => None, + secs => Some(Datetime::from_unix_time_seconds(secs.into())), + } + } + + fn set_expiration_time(&mut self, expiration_time: Option) { + match expiration_time { + Some(dt) => { + self.expiration_time_storage + .write(dt.to_unix_time_seconds().try_into().expect( + "Datetime::to_unix_timestamp() returns i64, which should always fit in u32 until the year 2106", + )); + self.wake_state = WakeState::Armed; + } + None => { + self.expiration_time_storage.write(Self::NO_EXPIRATION_TIME); + self.wake_state = WakeState::Clear; + } + } + } +} + +pub(crate) struct Timer { + timer_state: Mutex>, + + timer_signal: Signal>, +} + +impl Timer { + pub fn new( + active: bool, + expiration_time_storage: &'static mut dyn NvramStorage<'static, u32>, + wake_policy_storage: &'static mut dyn NvramStorage<'static, u32>, + ) -> Self { + let result = Self { + timer_state: Mutex::new(RefCell::new(TimerState { + expiration_time_storage, + wake_policy_storage, + + wake_state: WakeState::Clear, + + timer_status: Default::default(), + is_active: false, + })), + timer_signal: Signal::new(), + }; + + // TODO make sure there's not some weird edge case here with coming back from a power loss - we may need to suppress the wake policy timer in this case? + result.set_timer_wake_policy( + result + .timer_state + .lock(|timer_state| timer_state.borrow().get_timer_wake_policy()), + ); + + result.set_expiration_time( + result + .timer_state + .lock(|timer_state| timer_state.borrow().get_expiration_time()), + ); + + result.set_active(active); + + result + } + + pub fn get_wake_status(&self) -> TimerStatus { + self.timer_state.lock(|timer_state| { + let timer_state = timer_state.borrow(); + timer_state.timer_status + }) + } + + pub fn clear_wake_status(&self) { + self.timer_state.lock(|timer_state| { + let mut timer_state = timer_state.borrow_mut(); + timer_state.timer_status = Default::default(); + }); + } + + // TODO [SPEC_QUESTION] the spec is ambiguous on whether or not this policy should include the number of seconds that have elapsed against it + // (i.e. if the user set it to 60s and 45s have elapsed since we switched to the expired power source, should we report + // 60s or 15s?)- see if we can get a concrete answer on this. + // + pub fn get_timer_wake_policy(&self) -> AlarmExpiredWakePolicy { + self.timer_state + .lock(|timer_state| timer_state.borrow().get_timer_wake_policy()) + } + + pub fn set_timer_wake_policy(&self, wake_policy: AlarmExpiredWakePolicy) { + self.timer_state.lock(|timer_state| { + let mut timer_state = timer_state.borrow_mut(); + timer_state.set_timer_wake_policy(wake_policy); + + // TODO [SPEC_QUESTION] verify this is correct - the spec isn't particularly clear on what should happen if reprogramming the policy while it's actively ticking down, + // may need to look at the windows acpi implementation or something + // + if let WakeState::ExpiredWaitingForPolicyDelay(_, _) = timer_state.wake_state { + timer_state.wake_state = WakeState::ExpiredWaitingForPolicyDelay( + crate::get_current_datetime() + .expect("Datetime clock should have already been initialized before we were constructed"), + 0, + ); + self.timer_signal.signal(Some(wake_policy.0)); + } + }) + } + + pub fn set_expiration_time(&self, expiration_time: Option) { + self.timer_state.lock(|timer_state| { + let mut timer_state = timer_state.borrow_mut(); + + // Per ACPI 6.4 section 9.18.1: "The status of wake timers can be reset by setting the wake alarm". + timer_state.timer_status = Default::default(); + + match expiration_time { + Some(dt) => { + timer_state.set_expiration_time(expiration_time); + timer_state.wake_state = WakeState::Armed; + + // Note: If the expiration time was in the past, this will immediately trigger the timer to expire. + self.timer_signal.signal(Some( + dt + .to_unix_time_seconds() + .saturating_sub(crate::get_current_datetime().expect("datetime clocks should have been initialized before we were constructed").to_unix_time_seconds()).try_into() + .expect("Users should not have been able to program a time greater than u32::MAX seconds in the future - the ACPI spec prevents it") + )); + } + None => self.clear_expiration_time(&mut timer_state) + } + }); + } + + pub fn get_expiration_time(&self) -> Option { + self.timer_state + .lock(|timer_state| timer_state.borrow().get_expiration_time()) + } + + pub fn set_active(&self, is_active: bool) { + self.timer_state.lock(|timer_state| { + let mut timer_state = timer_state.borrow_mut(); + + let was_active = timer_state.is_active; + timer_state.is_active = is_active; + + if was_active == is_active { + return; // No change + } + + if !was_active { + if let WakeState::ExpiredWaitingForPowerSource(seconds_already_elapsed) = timer_state.wake_state { + timer_state.wake_state = WakeState::ExpiredWaitingForPolicyDelay( + crate::get_current_datetime() + .expect("datetime clock should have already been initialized before we were constructed"), + seconds_already_elapsed, + ); + self.timer_signal.signal(Some( + timer_state + .get_timer_wake_policy() + .0 + .saturating_sub(seconds_already_elapsed), + )); + } + } else if was_active { + if let WakeState::ExpiredWaitingForPolicyDelay(wait_start_time, seconds_elapsed_before_wait) = + timer_state.wake_state + { + let total_seconds_elapsed_on_policy_delay: u32 = seconds_elapsed_before_wait + + (crate::get_current_datetime() + .expect("Datetime clock should have already been initialized before we were constructed") + .to_unix_time_seconds() + .saturating_sub(wait_start_time.to_unix_time_seconds())) as u32; // TODO figure out how to make this build without the cast + + timer_state.wake_state = + WakeState::ExpiredWaitingForPowerSource(total_seconds_elapsed_on_policy_delay); + self.timer_signal.signal(None); + } + } + }); + } + + pub(crate) async fn wait_until_wake(&self) { + let mut wait_duration: Option = self.timer_signal.wait().await; + + loop { + 'waiting_for_timer: loop { + match wait_duration { + Some(seconds) => { + match select( + embassy_time::Timer::after_secs(seconds.into()), + self.timer_signal.wait(), + ) + .await + { + Either::First(()) => { + if self.process_expired_timer() { + return; + } + } + Either::Second(new_wait_duration) => { + wait_duration = new_wait_duration; + } + } + } + None => { + // Wait until a new timer command comes in + break 'waiting_for_timer; + } + } + } + } + } + + /// Handles state changes for when the timer expires (figuring out what to do based on the current power source, etc). + /// Returns true if the timer's expiry indicates that a wake event should be signaled to the host. + fn process_expired_timer(&self) -> bool { + self.timer_state.lock(|timer_state| { + let mut timer_state = timer_state.borrow_mut(); + + match timer_state.wake_state { + // Clear: timer was disarmed right as we were waking - nothing to do. + // ExpiredOrphaned: shouldn't happen, but if we're in this state the timer should be dead, so nothing to do. + // ExpiredWaitingForPowerSource: shouldn't happen, but if we're in this state the timer is still waiting for power source so nothing to do. + WakeState::Clear | WakeState::ExpiredOrphaned | WakeState::ExpiredWaitingForPowerSource(_) => return false, + + WakeState::Armed | WakeState::ExpiredWaitingForPolicyDelay(_, _) => { + let now = crate::get_current_datetime().expect("Datetime clock should have already been initialized before we were constructed"); + let expiration_time = timer_state.get_expiration_time().expect("We should never be in the Armed or ExpiredWaitingForPolicyDelay states if there's no expiration time set"); + if now.to_unix_time_seconds() < expiration_time.to_unix_time_seconds() { // TODO we should probably implement Ord for Datetime and use that + // Time hasn't actually passed the mark yet - this can happen if we were reprogrammed with a different time right as the old timer was expiring. Reset the timer. + timer_state.wake_state = WakeState::Armed; + self.timer_signal.signal(Some(expiration_time + .to_unix_time_seconds() + .saturating_sub(now.to_unix_time_seconds()) + .try_into() + .expect("Users should not have been able to program a time greater than u32::MAX seconds in the future - the ACPI spec prevents it"))); + return false; + } + + timer_state.timer_status.timer_expired = true; + if timer_state.is_active { + timer_state.timer_status.timer_triggered_wake = true; + self.clear_expiration_time(&mut timer_state); + return true; + } + else { + if timer_state.get_timer_wake_policy() == AlarmExpiredWakePolicy::NEVER { + timer_state.wake_state = WakeState::ExpiredOrphaned; + return false; + } + + if let WakeState::ExpiredWaitingForPolicyDelay(_, _) = timer_state.wake_state { + timer_state.wake_state = WakeState::ExpiredWaitingForPowerSource(timer_state.get_timer_wake_policy().0); + } else { + timer_state.wake_state = WakeState::ExpiredWaitingForPowerSource(0); + } + } + } + } + + false + }) + } + + fn clear_expiration_time(&self, timer_state: &mut TimerState) { + timer_state.set_expiration_time(None); + timer_state.set_timer_wake_policy(AlarmExpiredWakePolicy::NEVER); + timer_state.wake_state = WakeState::Clear; + self.timer_signal.signal(None); + } +} From 020cf178524f022d9fef2d60fbae58c5e7b3a504 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Fri, 26 Sep 2025 18:36:26 -0700 Subject: [PATCH 02/14] Clean up todos --- Cargo.lock | 4 - time-alarm-service/Cargo.toml | 5 - time-alarm-service/src/acpi_timestamp.rs | 30 +- time-alarm-service/src/lib.rs | 453 +++++++++++------------ time-alarm-service/src/timer.rs | 199 +++++----- 5 files changed, 329 insertions(+), 362 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d3936e50..500ddf7a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2001,13 +2001,9 @@ dependencies = [ "embassy-futures", "embassy-sync", "embassy-time", - "embedded-hal 1.0.0", - "embedded-hal-async", "embedded-mcu-hal", "embedded-services", - "heapless 0.8.0", "log", - "uuid", ] [[package]] diff --git a/time-alarm-service/Cargo.toml b/time-alarm-service/Cargo.toml index 32fef2d9..8d3eb981 100644 --- a/time-alarm-service/Cargo.toml +++ b/time-alarm-service/Cargo.toml @@ -7,7 +7,6 @@ license.workspace = true repository.workspace = true [dependencies] -# TODO review these and figure out if we actually need them bytemuck.workspace = true defmt = { workspace = true, optional = true } log = { workspace = true, optional = true } @@ -15,12 +14,8 @@ embassy-executor.workspace = true embassy-futures.workspace = true embassy-sync.workspace = true embassy-time.workspace = true -embedded-hal-async.workspace = true -embedded-hal.workspace = true embedded-mcu-hal.workspace = true embedded-services.workspace = true -heapless.workspace = true -uuid.workspace = true [features] defmt = [ diff --git a/time-alarm-service/src/acpi_timestamp.rs b/time-alarm-service/src/acpi_timestamp.rs index 89c6d9ad..5c2fa0ac 100644 --- a/time-alarm-service/src/acpi_timestamp.rs +++ b/time-alarm-service/src/acpi_timestamp.rs @@ -3,7 +3,7 @@ use embedded_mcu_hal::time::{Datetime, UncheckedDatetime}; use crate::TimeAlarmError; // Timestamp structure as specified in the ACPI spec. Must be exactly this layout. -// TODO are there any endianness shenanigans associated with bytemuck here? +// TODO [TESTING] are there any endianness shenanigans associated with bytemuck here? #[repr(C)] #[derive(bytemuck::Pod, bytemuck::Zeroable, Copy, Clone, Debug)] struct RawAcpiTimestamp { @@ -28,7 +28,8 @@ struct RawAcpiTimestamp { // For _GRT, 0 = time is not valid (request failed), 1 = time is valid. For _SRT, this is padding and should be 0. valid_or_padding: u8, - // Millseconds: 1-1000. Leap seconds are not supported. // TODO ACPI spec says 1-1000, but surely it should be 0-999? what's up with that? We may need to do some translation if this isn't just a typo in the spec + // Millseconds: 1-1000. Leap seconds are not supported. + // TODO [SPEC] The ACPI spec says 1-1000, but it seems like it should be 0-999? We may need to do some translation if this isn't just a typo in the spec. milliseconds: u16, // Time zone: -1440 to 1440 in minutes from UTC, or 2047 if unspecified @@ -52,26 +53,8 @@ impl RawAcpiTimestamp { pub fn as_bytes(&self) -> &[u8; core::mem::size_of::()] /* 16 */ { bytemuck::bytes_of(self).try_into().expect("Should never fail because we know the size of AcpiTimestamp at compile time") } - - #[allow(dead_code)] // TODO either use this or remove it. I think we only need it if we want to return an invalid timestamp to the host rather than an error code that the ASL translates into the invalid timestamp structure to report failure. - pub fn invalid() -> Self { - Self { - year: 0, - month: 0, - day: 0, - hour: 0, - minute: 0, - second: 0, - valid_or_padding: 0, // invalid - milliseconds: 0, - time_zone: 0, - daylight: 0, - _padding: [0; 3], - } - } } -// TODO s/AcpiTimestamp/AcpiTime/? impl From<&AcpiTimestamp> for RawAcpiTimestamp { fn from(ts: &AcpiTimestamp) -> Self { Self { @@ -204,14 +187,9 @@ impl AcpiTimestamp { minute: raw.minute, second: raw.second, nanosecond: (raw.milliseconds as u32) * 1_000_000, - }).map_err(|_| TimeAlarmError::InvalidArgument)?, // TODO verify that this is sufficient validation + })?, time_zone: raw.time_zone.try_into()?, dst_status: raw.daylight.try_into()?, }) } - - #[allow(dead_code)] // TODO either use this or remove it. I think we only need it if we want to return an invalid timestamp to the host rather than an error code that the ASL translates into the invalid timestamp structure to report failure. - pub fn invalid_timestamp_bytes() -> [u8; core::mem::size_of::()] /* 16 */ { - *RawAcpiTimestamp::invalid().as_bytes() - } } diff --git a/time-alarm-service/src/lib.rs b/time-alarm-service/src/lib.rs index 95379af6..d88af151 100644 --- a/time-alarm-service/src/lib.rs +++ b/time-alarm-service/src/lib.rs @@ -1,11 +1,9 @@ #![no_std] -// #![allow(unused)] // TODO remove before checkin use core::any::Any; use core::array::TryFromSliceError; use core::borrow::Borrow; use core::cell::RefCell; -use core::panic; use embassy_futures::select::{Either, select}; use embassy_sync::blocking_mutex::Mutex; use embassy_sync::channel::Channel; @@ -37,10 +35,16 @@ pub enum TimeAlarmError { ClockError(DatetimeClockError), } -// TODO may want to map other error types here impl From for MailboxDelegateError { - fn from(_error: TimeAlarmError) -> Self { - MailboxDelegateError::InvalidData + fn from(error: TimeAlarmError) -> Self { + match error { + TimeAlarmError::UnknownCommand=> MailboxDelegateError::InvalidData, + TimeAlarmError::DoubleInitError=> panic!("Should never attempt intitialization as a response to receiving a mailbox message"), + TimeAlarmError::MailboxFullError=> MailboxDelegateError::BufferFull, + TimeAlarmError::InvalidAcpiTimerId=> MailboxDelegateError::InvalidData, + TimeAlarmError::InvalidArgument=> MailboxDelegateError::InvalidData, + TimeAlarmError::ClockError(_)=> MailboxDelegateError::Other, + } } } @@ -50,15 +54,23 @@ impl From for TimeAlarmError { } } -// TODO should an impl like this exist in the MailboxDelegateError crate? For now use map_err -// impl From> for MailboxDelegateError { -// fn from(_error: TrySendError) -> Self { -// match _error { -// TrySendError::Full(()) => MailboxDelegateError::BufferFull, -// _ => MailboxDelegateError::Other // TODO is this the right mapping? -// } -// } -// } +impl From for TimeAlarmError { + fn from(e: DatetimeClockError) -> Self { + TimeAlarmError::ClockError(e) + } +} + +impl From for TimeAlarmError { + fn from(_error: embedded_services::intrusive_list::Error) -> Self { + TimeAlarmError::DoubleInitError + } +} + +impl From for TimeAlarmError { + fn from(_error: embedded_mcu_hal::time::DatetimeError) -> Self { + TimeAlarmError::InvalidArgument + } +} // ------------------------------------------------- @@ -78,8 +90,7 @@ impl AcpiTimerId { bytes .get(0..SIZE_BYTES) .ok_or(TimeAlarmError::InvalidArgument)? - .try_into() - .map_err(|_| TimeAlarmError::InvalidArgument)?, + .try_into()?, ); Ok((AcpiTimerId::try_from(id)?, &bytes[SIZE_BYTES..])) @@ -107,11 +118,6 @@ impl TryFrom for AcpiTimerId { // ------------------------------------------------- -// TODO is there some way to do this with an enum that's more readable? I can't figure out how to make it store in -// a u32 directly and impossible to do Some(u32::MAX) without just adding a panic or whatever. Maybe that's the -// right thing to do? but then I think we end up spending a byte on the enum discriminant, which seems wasteful? -// Is there some way to tell the compiler it can get away with inferring discriminant from the value? -// #[derive(Clone, Copy, Debug, PartialEq)] struct AlarmTimerSeconds(u32); impl AlarmTimerSeconds { @@ -142,11 +148,10 @@ impl Default for AlarmExpiredWakePolicy { /// Represents an ACPI Time and Alarm Device command. /// See ACPI Specification 6.4, Section 9.18 "Time and Alarm Device" for details on semantics. -// TODO should these all take an 'endpoint to respond to' parameter? Right now we're assuming that the only thing that will send us commands is the host, -// which may not be the case? #[rustfmt::skip] enum AcpiTimeAlarmDeviceCommand { - GetCapabilities, // 0: _GCP --> u32 (bitmask), failure: infallible + // Notably missing from the ACPI spec here is _GCP / 'Get Capabilities'. It just returns a constant and is expected to be implemented wholly in the ACPI ASL code. + GetRealTime, // 1: _GRT --> AcpiTimestamp, failure: valid bit = 0 in returned timestamp SetRealTime(AcpiTimestamp), // 2: _SRT --> u32 (bool), failure: u32::MAX GetWakeStatus(AcpiTimerId), // 3: _GWS --> u32 (bitmask), failure: infallible @@ -155,11 +160,13 @@ enum AcpiTimeAlarmDeviceCommand { SetTimerValue(AcpiTimerId, AlarmTimerSeconds), // 6: _STV --> u32 (bool), failure: 1, GetExpiredTimerPolicy(AcpiTimerId), // 7: _TIP --> u32 (AlarmExpiredWakePolicy) failure: infallible GetTimerValue(AcpiTimerId), // 8: _TIV --> u32 (AlarmTimerSeconds), failure: infallible, u32::MAX if disabled + + RespondToInvalidCommand // Not an ACPI method. Used internally to indicate that an invalid command was received, and we must respond with an error asynchronously. } impl AcpiTimeAlarmDeviceCommand { fn try_from_bytes(bytes: &[u8]) -> Result { - // TODO for now, we assume that the message structure is followed by the ACPI arguments in order as specified in + // TODO [COMMS] for now, we assume that the message structure is followed by the ACPI arguments in order as specified in // the ACPI spec https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/09_ACPI-Defined_Devices_and_Device-Specific_Objects/ACPIdefined_Devices_and_DeviceSpecificObjects.html#tiv-timer-values // For example, _STP will be [0x05, , ] // We need to make sure this is actually how we implement it over eSPI, or adapt to match what eSPI actually sends us. @@ -170,11 +177,7 @@ impl AcpiTimeAlarmDeviceCommand { let bytes = bytes.get(COMMAND_CODE_SIZE_BYTES..) .expect("Should never fail because if there were less than 4 bytes, u32::from_le_bytes would have failed. If there were exactly four bytes, this will return an empty slice, not None."); - // TODO I feel like these numbers should be associated with the enum somehow, maybe inferred from ordinal position in the enum? - // but I don't know if it's possible to do that in Rust. Figure out if there's a clean way to do it, maybe some to-int - // macro or something? match command_code { - 0 => Ok(AcpiTimeAlarmDeviceCommand::GetCapabilities), // TODO there's a case to be made that this should just be a hardcoded constant in the ASL - if we do that, may want to renumber 1 => Ok(AcpiTimeAlarmDeviceCommand::GetRealTime), 2 => Ok(AcpiTimeAlarmDeviceCommand::SetRealTime(AcpiTimestamp::try_from_bytes( bytes, @@ -201,54 +204,73 @@ impl AcpiTimeAlarmDeviceCommand { } } -// ------------------------------------------------- +enum AcpiTimeAlarmCommandResult { + /// Used for returning timestamps, i.e. the current time. + Timestamp(AcpiTimestamp), -struct TimeZoneData { - // Storage used to back the timezone and DST settings. - // TODO can we achieve this without dyn? I think this would require making the service generic over the clock type, which means no static global SERVICE? - storage: &'static mut dyn NvramStorage<'static, u32>, -} + /// Used for returning simple u32 values, such as timer values, wake status bitmasks, etc. + U32(u32), -// TODO is there a cleaner way to do this sort of bitpacking? I really just want std::bit_cast< struct { int16_t, uint8_t, uint8_t }> -impl TimeZoneData { - const TZ_MASK: u32 = 0x0000FFFF; - const DST_MASK: u32 = 0x00FF0000; - const DST_SHIFT: u32 = 16; + /// The operation succeeded, but there's no data to return. + Valueless, +} - pub fn new(storage: &'static mut dyn NvramStorage<'static, u32>) -> Self { - Self { storage } - } +// ------------------------------------------------- - pub fn set_data(&mut self, tz: AcpiTimeZone, dst: AcpiDaylightSavingsTimeStatus) { - let tz_i16: i16 = tz.into(); - let tz_u32 = tz_i16 as u32; +mod time_zone_data { + use crate::AcpiDaylightSavingsTimeStatus; + use crate::AcpiTimeZone; + use crate::NvramStorage; + use crate::TimeAlarmError; - let dst_u8: u8 = dst.into(); - let dst_u32 = (dst_u8 as u32) << Self::DST_SHIFT; + pub struct TimeZoneData { + // Storage used to back the timezone and DST settings. + storage: &'static mut dyn NvramStorage<'static, u32>, + } - self.storage.write(tz_u32 | dst_u32); + #[repr(C)] + #[derive(bytemuck::Pod, bytemuck::Zeroable, Copy, Clone, Debug)] + struct RawTimeZoneData { + tz: i16, + dst: u8, + _padding: u8, // padding to make the struct 4 bytes } - pub fn get_data(&self) -> (AcpiTimeZone, AcpiDaylightSavingsTimeStatus) { - let tz_u16 = (self.storage.read() & Self::TZ_MASK) as u16; - let tz_i16 = tz_u16 as i16; - let tz = AcpiTimeZone::try_from(tz_i16).unwrap_or(AcpiTimeZone::Unknown); + impl TimeZoneData { + pub fn new(storage: &'static mut dyn NvramStorage<'static, u32>) -> Self { + Self { storage } + } + + /// Writes the given time zone and daylight savings time status to NVRAM. + /// + pub fn set_data(&mut self, tz: AcpiTimeZone, dst: AcpiDaylightSavingsTimeStatus) { + let representation = RawTimeZoneData { + tz: tz.into(), + dst: dst.into(), + _padding: 0, + }; - let dst_u8 = ((self.storage.read() & Self::DST_MASK) >> Self::DST_SHIFT) as u8; - let dst = AcpiDaylightSavingsTimeStatus::try_from(dst_u8).unwrap_or(AcpiDaylightSavingsTimeStatus::NotObserved); + self.storage.write(bytemuck::cast(representation)); + } - (tz, dst) + /// Retreives the current time zone / daylight savings time. + /// If the stored data is invalid, implying that the NVRAM has never been initialized, defaults to + /// (AcpiTimeZone::Unknown, AcpiDaylightSavingsTimeStatus::NotObserved). + /// + pub fn get_data(&self) -> (AcpiTimeZone, AcpiDaylightSavingsTimeStatus) { + let representation: RawTimeZoneData = bytemuck::cast(self.storage.read()); + (|| -> Result<(AcpiTimeZone, AcpiDaylightSavingsTimeStatus), TimeAlarmError> { + Ok((representation.tz.try_into()?, representation.dst.try_into()?)) + })() + .unwrap_or_else(|_| (AcpiTimeZone::Unknown, AcpiDaylightSavingsTimeStatus::NotObserved)) + } } } +use time_zone_data::TimeZoneData; // ------------------------------------------------- struct ClockState { - // TODO can we achieve this without dyn? I think this would require making the service generic over the clock type, - // which means no static global SERVICE / memory allocation for the SERVICE would have to be done by the caller - // of init(), which might interfere with the requirement that we can pass static references to it to the comms - // system? Need to investigate options for that - // datetime_clock: &'static mut dyn DatetimeClock, tz_data: TimeZoneData, } @@ -288,8 +310,6 @@ impl Timers { } } - // TODO it's a bit unfortunate that we have to pass these in individually like this, but I don't see another way to get compile-time - // checking of the number of registers passed in without the experimental split_array_mut - ask if theres' a better way fn new( ac_expiration_storage: &'static mut dyn NvramStorage<'static, u32>, ac_policy_storage: &'static mut dyn NvramStorage<'static, u32>, @@ -297,8 +317,8 @@ impl Timers { dc_policy_storage: &'static mut dyn NvramStorage<'static, u32>, ) -> Self { Self { - ac_timer: Timer::new(true, ac_expiration_storage, ac_policy_storage), - dc_timer: Timer::new(false, dc_expiration_storage, dc_policy_storage), + ac_timer: Timer::new(ac_expiration_storage, ac_policy_storage), + dc_timer: Timer::new(dc_expiration_storage, dc_policy_storage), } } } @@ -309,183 +329,200 @@ pub struct Service { endpoint: comms::Endpoint, // ACPI messages from the host are sent through this channel. - acpi_channel: Channel, + acpi_channel: Channel, clock_state: Mutex>, - power_source_signal: Signal, // TODO figure out how to feed this thing + // TODO [POWER_SOURCE] signal this whenever the power source changes + power_source_signal: Signal, timers: Timers, } -// TODO can we template this on the clock/storage type? impl Service { - pub fn new( + // TODO [DYN] if we want to allow taking the HAL traits as concrete types rather than as dyn references, we'll likely need to make this a macro + // in order to accommodate the restriction that embassy tasks can't have generic parameters. When we do that, it may be worthwhile to + // also investigate ways to take the backing storage as a slice rather than as a bunch of individual references - currently, we can't + // take a slice of the array because that would be a slice of trait impls and we need dyn references here to accommodate the constraints + // on embassy task implementation. + // + pub async fn init( + service_storage: &'static mut OnceLock, + spawner: &embassy_executor::Spawner, backing_clock: &'static mut impl DatetimeClock, tz_storage: &'static mut dyn NvramStorage<'static, u32>, ac_expiration_storage: &'static mut dyn NvramStorage<'static, u32>, ac_policy_storage: &'static mut dyn NvramStorage<'static, u32>, dc_expiration_storage: &'static mut dyn NvramStorage<'static, u32>, dc_policy_storage: &'static mut dyn NvramStorage<'static, u32>, - ) -> Self { - Service { - endpoint: comms::Endpoint::uninit(comms::EndpointID::Internal(comms::Internal::TimeAlarm)), - acpi_channel: Channel::new(), - clock_state: Mutex::new(RefCell::new(ClockState { - datetime_clock: backing_clock, - tz_data: TimeZoneData::new(tz_storage), - })), - power_source_signal: Signal::new(), - timers: Timers::new( - ac_expiration_storage, - ac_policy_storage, - dc_expiration_storage, - dc_policy_storage, - ), - } + ) -> Result<(), TimeAlarmError> { + info!("Starting time-alarm service task"); + + let service = service_storage.get_or_init(|| { + Service { + endpoint: comms::Endpoint::uninit(comms::EndpointID::Internal(comms::Internal::TimeAlarm)), + acpi_channel: Channel::new(), + clock_state: Mutex::new(RefCell::new(ClockState { + datetime_clock: backing_clock, + tz_data: TimeZoneData::new(tz_storage), + })), + power_source_signal: Signal::new(), + timers: Timers::new( + ac_expiration_storage, + ac_policy_storage, + dc_expiration_storage, + dc_policy_storage, + ), + } + }); + + // TODO [POWER_SOURCE] we need to subscribe to messages that tell us if we're on AC or DC power so we can decide which alarms to trigger - how do we do that? + // TODO [POWER_SOURCE] if it's possible to learn which power source is active at init time, we should set that one active rather than defaulting to the AC timer. + service.timers.ac_timer.start(&service.clock_state, true); + service.timers.dc_timer.start(&service.clock_state, false); + + comms::register_endpoint(service, &service.endpoint) + .await?; + + spawner.must_spawn(command_handler_task(service)); + spawner.must_spawn(timer_task(service, AcpiTimerId::AcPower)); + spawner.must_spawn(timer_task(service, AcpiTimerId::DcPower)); + + Ok(()) } - pub async fn handle_requests(&self) { + pub async fn handle_requests(&'static self) { loop { let acpi_command = self.acpi_channel.receive(); let power_source_change = self.power_source_signal.wait(); match select(acpi_command, power_source_change).await { - Either::First(acpi_command) => { - self.handle_acpi_command(acpi_command).await.unwrap_or_else(|e| { - error!("Error handling ACPI command: {:?}", e); - // TODO what should happen if this fails? How do we communicate failure back to the host? - }); + Either::First((respond_to_endpoint, acpi_command)) => { + const COMMAND_SUCCEEDED: u32 = 1; + const COMMAND_FAILED: u32 = 0; + + let acpi_result = self.handle_acpi_command(acpi_command).await; + match acpi_result { + Ok(response_payload) => { + // TODO [COMMS] is it a problem that we're sending the response in two pieces? If yes, we may need to + // arrange it into a buffer or something. May not be worth solving if we're looking to pivot + // to using something like postcard for this, though. + // + // TODO [COMMS] it seems like we're sort of conflating wire representation with message representation here - + // is this really how we want to pass messages through the comms system? It seems like it makes it + // harder for other services to send messages to us. May change with postcard? + // + self.send_acpi_response(respond_to_endpoint, &COMMAND_SUCCEEDED).await; + match response_payload { + AcpiTimeAlarmCommandResult::Timestamp(timestamp) => { + self.send_acpi_response(respond_to_endpoint, ×tamp.as_bytes()) + .await + } + AcpiTimeAlarmCommandResult::U32(value) => { + self.send_acpi_response(respond_to_endpoint, &value).await + } + AcpiTimeAlarmCommandResult::Valueless => (), // nothing more to send + } + } + Err(e) => { + error!("Error handling ACPI command: {:?}", e); + self.send_acpi_response(respond_to_endpoint, &COMMAND_FAILED).await; + } + } } Either::Second(new_power_source) => { info!("Power source changed to {:?}", new_power_source); self.timers .get_timer(new_power_source.get_other_timer_id()) - .set_active(false); - self.timers.get_timer(new_power_source).set_active(true); + .set_active(&self.clock_state, false); + self.timers.get_timer(new_power_source).set_active(&self.clock_state, true); } } } } - pub async fn handle_timer(&self, timer_id: AcpiTimerId) { + pub async fn handle_timer(&'static self, timer_id: AcpiTimerId) { let timer = self.timers.get_timer(timer_id); loop { - timer.wait_until_wake().await; - // TODO [SPEC_QUESTION] section 9.18.7 indicates that when a timer expires, both timers have their wake policies reset, + timer.wait_until_wake(&self.clock_state).await; + // TODO [SPEC] section 9.18.7 indicates that when a timer expires, both timers have their wake policies reset, // but I can't find any similar rule for the actual timer value - that seems odd to me, verify that's actually how // it's supposed to work self.timers .get_timer(timer_id.get_other_timer_id()) - .set_timer_wake_policy(AlarmExpiredWakePolicy::NEVER); + .set_timer_wake_policy(&self.clock_state, AlarmExpiredWakePolicy::NEVER); - todo!("Figure out how to signal a wake event to the host when this timer expires"); + // TODO [COMMS] Figure out how to signal a wake event to the host and do that here } } - async fn send_acpi_response(&self, response: &impl Any) { - // TODO right now we're hardcoded to reply to the host, but I think anyone can send us a command - do we need to track - // the source endpoint of the command so we can respond to it specifically? What do other services do? + async fn send_acpi_response(&self, destination: comms::EndpointID, response: &impl Any) { self.endpoint - .send(comms::EndpointID::External(comms::External::Host), response) + .send(destination, response) .await .expect("send returns Result<(), Infallible>"); } - async fn handle_acpi_command(&self, command: AcpiTimeAlarmDeviceCommand) -> Result<(), TimeAlarmError> { + async fn handle_acpi_command( + &'static self, + command: AcpiTimeAlarmDeviceCommand, + ) -> Result { info!("Received Time-Alarm Device command: {:?}", command); match command { - // TODO these all need to return a buffer on error. - // The size and shape of that buffer depend on the message type, so we probably can't punt to the caller unless we want to do the translation in the ASL? That seems cleaner to me, but maybe there's some reason not to do that? - AcpiTimeAlarmDeviceCommand::GetCapabilities => { - todo!( - "implement or remove GetCapabilities - if implemented, it'd return a constant, so there's a case to be made that it belongs wholly in the ASL" - ); - } AcpiTimeAlarmDeviceCommand::GetRealTime => { - let time = self.clock_state.lock(|clock_state| { + self.clock_state.lock(|clock_state| { let clock_state = clock_state.borrow(); - match clock_state.datetime_clock.get_current_datetime() { - // TODO figure out why type inference doesn't work with map_err and the ? operator - Ok(datetime) => { - let (time_zone, dst_status) = clock_state.tz_data.get_data(); - Ok(AcpiTimestamp { - datetime, - time_zone, - dst_status, - }) - } - Err(e) => Err(TimeAlarmError::ClockError(e)), - } - })?; - - self.send_acpi_response(&time.as_bytes()).await; - // TODO is this sufficient, or do we also need a 'success' / 'EOM' packet or something? - - Ok(()) + let datetime = clock_state.datetime_clock.get_current_datetime()?; + let (time_zone, dst_status) = clock_state.tz_data.get_data(); + Ok(AcpiTimeAlarmCommandResult::Timestamp(AcpiTimestamp { + datetime, + time_zone, + dst_status, + })) + }) } AcpiTimeAlarmDeviceCommand::SetRealTime(timestamp) => { self.clock_state.lock(|clock_state| { let mut clock_state = clock_state.borrow_mut(); clock_state .datetime_clock - .set_current_datetime(×tamp.datetime) - .map_err(TimeAlarmError::ClockError)?; + .set_current_datetime(×tamp.datetime)?; clock_state.tz_data.set_data(timestamp.time_zone, timestamp.dst_status); - // TODO do we need to return a success code over espi? - // TODO do we need to adjust the timers based on the new time? check with ACPI spec - Ok(()) + // TODO [SPEC] the spec is ambiguous on whether or not we should adjust any outstanding timers based on the new time - see if we can find an answer elsewhere + Ok(AcpiTimeAlarmCommandResult::Valueless) }) } AcpiTimeAlarmDeviceCommand::GetWakeStatus(timer_id) => { let status = self.timers.get_timer(timer_id).get_wake_status(); - let packed_status: u32 = status.into(); - self.send_acpi_response(&packed_status.to_le_bytes()).await; - // TODO is this sufficient, or do we also need a 'success' / 'EOM' packet or something? - - Ok(()) + Ok(AcpiTimeAlarmCommandResult::U32(status.into())) } AcpiTimeAlarmDeviceCommand::ClearWakeStatus(timer_id) => { self.timers.get_timer(timer_id).clear_wake_status(); - // TODO do we need to return a success code over espi? - Ok(()) + Ok(AcpiTimeAlarmCommandResult::Valueless) } AcpiTimeAlarmDeviceCommand::SetExpiredTimerPolicy(timer_id, timer_policy) => { - self.timers.get_timer(timer_id).set_timer_wake_policy(timer_policy); - // TODO do we need to return a success code over espi? - Ok(()) + self.timers.get_timer(timer_id).set_timer_wake_policy(&self.clock_state, timer_policy); + Ok(AcpiTimeAlarmCommandResult::Valueless) } AcpiTimeAlarmDeviceCommand::SetTimerValue(timer_id, timer_value) => { let new_expiration_time = match timer_value { AlarmTimerSeconds::DISABLED => None, AlarmTimerSeconds(secs) => { - let current_time = self.clock_state.lock(|clock_state| { - let clock_state = clock_state.borrow(); - clock_state - .datetime_clock - .get_current_datetime() - .map_err(TimeAlarmError::ClockError) - })?; - - let expiration_time = - Datetime::from_unix_time_seconds(current_time.to_unix_time_seconds() + u64::from(secs)); // TODO why doesn't into() work here? - Some(expiration_time) + let current_time = self + .clock_state + .lock(|clock_state| clock_state.borrow().datetime_clock.get_current_datetime())?; + + Some(Datetime::from_unix_time_seconds(current_time.to_unix_time_seconds() + u64::from(secs))) } }; - self.timers.get_timer(timer_id).set_expiration_time(new_expiration_time); - // TODO do we need to return a success code over espi? - - Ok(()) + self.timers.get_timer(timer_id).set_expiration_time(&self.clock_state, new_expiration_time); + Ok(AcpiTimeAlarmCommandResult::Valueless) } AcpiTimeAlarmDeviceCommand::GetExpiredTimerPolicy(timer_id) => { - let wake_policy = self.timers.get_timer(timer_id).get_timer_wake_policy(); - self.send_acpi_response(&wake_policy.0.to_le_bytes()).await; - // TODO is this sufficient, or do we also need a 'success' / 'EOM' packet or something? - - Ok(()) + Ok(AcpiTimeAlarmCommandResult::U32(self.timers.get_timer(timer_id).get_timer_wake_policy().0)) } AcpiTimeAlarmDeviceCommand::GetTimerValue(timer_id) => { let expiration_time = self.timers.get_timer(timer_id).get_expiration_time(); @@ -493,41 +530,41 @@ impl Service { const ACPI_TIMER_DISABLED: u32 = u32::MAX; let timer_wire_format: u32 = match expiration_time { Some(expiration_time) => { - let current_time = self.clock_state.lock(|clock_state| { - let clock_state = clock_state.borrow(); - clock_state - .datetime_clock - .get_current_datetime() - .map_err(TimeAlarmError::ClockError) - })?; + let current_time = self + .clock_state + .lock(|clock_state| clock_state.borrow().datetime_clock.get_current_datetime())?; expiration_time.to_unix_time_seconds().saturating_sub(current_time.to_unix_time_seconds()).try_into().expect("Per the ACPI spec, timers are communicated in u32 seconds, so this shouldn't be able to overflow") } None => ACPI_TIMER_DISABLED, }; - self.send_acpi_response(&timer_wire_format.to_le_bytes()).await; - // TODO is this sufficient, or do we also need a 'success' / 'EOM' packet or something? - - Ok(()) + Ok(AcpiTimeAlarmCommandResult::U32(timer_wire_format)) } + + AcpiTimeAlarmDeviceCommand::RespondToInvalidCommand => Err(TimeAlarmError::InvalidArgument), } } } impl comms::MailboxDelegate for Service { - fn receive(&self, _message: &comms::Message) -> Result<(), comms::MailboxDelegateError> { + fn receive(&self, message: &comms::Message) -> Result<(), comms::MailboxDelegateError> { trace!("Received message at time-alarm-service"); - if let Some(msg) = _message.data.get::() { + if let Some(msg) = message.data.get::() { let buffer_access = msg.payload.borrow(); let buffer: &[u8] = buffer_access.borrow(); - // TODO right now, if this fails, what happens? The TAD spec has different ways of reporting failure depending on the command, do we need to handle that here or is that in the ASL? - // TODO what is supposed to happen if there's e.g. an invalid timestamp? Is returning an error here sufficient, or do we need to send some kind of error response back to the host? self.acpi_channel - .try_send(AcpiTimeAlarmDeviceCommand::try_from_bytes(&buffer[0..msg.payload_len])?) + .try_send(( + message.from, + AcpiTimeAlarmDeviceCommand::try_from_bytes(&buffer[0..msg.payload_len]) + .unwrap_or(AcpiTimeAlarmDeviceCommand::RespondToInvalidCommand), + )) .map_err(|_| MailboxDelegateError::BufferFull)?; + // TODO [COMMS] right now, if pushing the message to the channel fails, the error that we return this gets + // discarded by our caller and we have no opportunity to raise a failure. Fixing that probably + // requires changes in the mailbox system, so we're ignoring it for now. Ok(()) } else { Err(comms::MailboxDelegateError::InvalidData) @@ -535,66 +572,14 @@ impl comms::MailboxDelegate for Service { } } -static SERVICE: OnceLock = OnceLock::new(); // TODO is this really what we want? I'd love to have init return an instance instead, that would let us template over the clock type... unclear how that would work with register_endpoint, though - -// TODO figure out if there's a cleaner way to pass a bunch of these storage instances in without having a ton of parameters - tried taking a slice but hit some weird type issues that I'm punting on for now -pub async fn init( - spawner: &embassy_executor::Spawner, - backing_clock: &'static mut impl DatetimeClock, - tz_storage: &'static mut dyn NvramStorage<'static, u32>, - ac_expiration_storage: &'static mut dyn NvramStorage<'static, u32>, - ac_policy_storage: &'static mut dyn NvramStorage<'static, u32>, - dc_expiration_storage: &'static mut dyn NvramStorage<'static, u32>, - dc_policy_storage: &'static mut dyn NvramStorage<'static, u32>, -) -> Result<(), TimeAlarmError> { - info!("Starting time-alarm service task"); - - let service = SERVICE.get_or_init(|| { - Service::new( - backing_clock, - tz_storage, - ac_expiration_storage, - ac_policy_storage, - dc_expiration_storage, - dc_policy_storage, - ) - }); - - comms::register_endpoint(service, &service.endpoint) - .await - .map_err(|_| { - error!("Failed to register time-alarm service endpoint"); - TimeAlarmError::DoubleInitError // TODO if we impl from on error type, tear this out - })?; - - // TODO we need to subscribe to messages that tell us if we're on AC or DC power so we can decide which alarms to trigger - how do we do that? - - spawner.must_spawn(command_handler_task()); - spawner.must_spawn(timer_task(AcpiTimerId::AcPower)); - spawner.must_spawn(timer_task(AcpiTimerId::DcPower)); - - Ok(()) -} - #[embassy_executor::task] -async fn command_handler_task() { +async fn command_handler_task(service: &'static Service) { info!("Starting time-alarm service task"); - let service = SERVICE.get_or_init(|| panic!("should have already been initialized by init()")); - service.handle_requests().await; } #[embassy_executor::task] -async fn timer_task(timer_id: AcpiTimerId) { +async fn timer_task(service: &'static Service, timer_id: AcpiTimerId) { info!("Starting time-alarm timer task"); - let service = SERVICE.get_or_init(|| panic!("should have already been initialized by init()")); - service.handle_timer(timer_id).await; } - -pub fn get_current_datetime() -> Option { - SERVICE - .try_get()? - .clock_state - .lock(|clock_state| clock_state.borrow().datetime_clock.get_current_datetime().ok()) -} diff --git a/time-alarm-service/src/timer.rs b/time-alarm-service/src/timer.rs index d1a8f213..c9f3c115 100644 --- a/time-alarm-service/src/timer.rs +++ b/time-alarm-service/src/timer.rs @@ -1,4 +1,4 @@ -use crate::{AlarmExpiredWakePolicy, TimerStatus}; +use crate::{AlarmExpiredWakePolicy, ClockState, TimerStatus}; use core::cell::RefCell; use embassy_futures::select::{Either, select}; use embassy_sync::{blocking_mutex::Mutex, signal::Signal}; @@ -23,24 +23,67 @@ enum WakeState { ExpiredOrphaned, } -// TODO do we want to do something like this to make persistent storage easier to read? -// struct PersistentAlarmExpiredWakePolicy { -// wake_policy: AlarmExpiredWakePolicy, -// storage: &'static mut dyn NvramStorage<'static, u32>, -// } -// struct PersistentExpirationTime { -// expiration_time: Option, -// storage: &'static mut dyn NvramStorage<'static, u32>, -// } +mod persistent_storage { + use crate::{AlarmExpiredWakePolicy, Datetime}; + use embedded_mcu_hal::NvramStorage; -struct TimerState { - /// When the timer is programmed to expire, or None if the timer is not set - /// This can't be part of the wake_state because we need to be able to report its value for _CWS even when the timer has expired and - /// we're handling the power source policy. - expiration_time_storage: &'static mut dyn NvramStorage<'static, u32>, + pub struct PersistentStorage { + /// When the timer is programmed to expire, or None if the timer is not set + /// This can't be part of the wake_state because we need to be able to report its value for _CWS even when the timer has expired and + /// we're handling the power source policy. + expiration_time_storage: &'static mut dyn NvramStorage<'static, u32>, + + // Persistent storage for the AlarmExpiredWakePolicy + wake_policy_storage: &'static mut dyn NvramStorage<'static, u32>, + } + + impl PersistentStorage { + pub fn new( + expiration_time_storage: &'static mut dyn NvramStorage<'static, u32>, + wake_policy_storage: &'static mut dyn NvramStorage<'static, u32>, + ) -> Self { + Self { + expiration_time_storage, + wake_policy_storage, + } + } + + const NO_EXPIRATION_TIME: u32 = u32::MAX; + + pub fn get_timer_wake_policy(&self) -> AlarmExpiredWakePolicy { + AlarmExpiredWakePolicy(self.wake_policy_storage.read()) + } + + pub fn set_timer_wake_policy(&mut self, wake_policy: AlarmExpiredWakePolicy) { + self.wake_policy_storage.write(wake_policy.0); + } + + pub fn get_expiration_time(&self) -> Option { + match self.expiration_time_storage.read() { + Self::NO_EXPIRATION_TIME => None, + secs => Some(Datetime::from_unix_time_seconds(secs.into())), + } + } + + pub fn set_expiration_time(&mut self, expiration_time: Option) { + match expiration_time { + Some(dt) => { + self.expiration_time_storage + .write(dt.to_unix_time_seconds().try_into().expect( + "Datetime::to_unix_timestamp() returns i64, which should always fit in u32 until the year 2106", + )); + } + None => { + self.expiration_time_storage.write(Self::NO_EXPIRATION_TIME); + } + } + } + } +} +use persistent_storage::PersistentStorage; - // Persistent storage for the AlarmExpiredWakePolicy - wake_policy_storage: &'static mut dyn NvramStorage<'static, u32>, +struct TimerState { + persistent_storage: PersistentStorage, wake_state: WakeState, @@ -52,38 +95,7 @@ struct TimerState { } impl TimerState { - const NO_EXPIRATION_TIME: u32 = u32::MAX; - - fn get_timer_wake_policy(&self) -> AlarmExpiredWakePolicy { - AlarmExpiredWakePolicy(self.wake_policy_storage.read()) - } - - fn set_timer_wake_policy(&mut self, wake_policy: AlarmExpiredWakePolicy) { - self.wake_policy_storage.write(wake_policy.0); - } - fn get_expiration_time(&self) -> Option { - match self.expiration_time_storage.read() { - Self::NO_EXPIRATION_TIME => None, - secs => Some(Datetime::from_unix_time_seconds(secs.into())), - } - } - - fn set_expiration_time(&mut self, expiration_time: Option) { - match expiration_time { - Some(dt) => { - self.expiration_time_storage - .write(dt.to_unix_time_seconds().try_into().expect( - "Datetime::to_unix_timestamp() returns i64, which should always fit in u32 until the year 2106", - )); - self.wake_state = WakeState::Armed; - } - None => { - self.expiration_time_storage.write(Self::NO_EXPIRATION_TIME); - self.wake_state = WakeState::Clear; - } - } - } } pub(crate) struct Timer { @@ -94,39 +106,36 @@ pub(crate) struct Timer { impl Timer { pub fn new( - active: bool, expiration_time_storage: &'static mut dyn NvramStorage<'static, u32>, wake_policy_storage: &'static mut dyn NvramStorage<'static, u32>, ) -> Self { - let result = Self { + Self { timer_state: Mutex::new(RefCell::new(TimerState { - expiration_time_storage, - wake_policy_storage, - + persistent_storage: PersistentStorage::new(expiration_time_storage, wake_policy_storage), wake_state: WakeState::Clear, - timer_status: Default::default(), is_active: false, })), timer_signal: Signal::new(), - }; + } + } - // TODO make sure there's not some weird edge case here with coming back from a power loss - we may need to suppress the wake policy timer in this case? - result.set_timer_wake_policy( - result + pub fn start(&self, clock_state: &'static Mutex>, active: bool) { + self.set_timer_wake_policy( + clock_state, + self .timer_state - .lock(|timer_state| timer_state.borrow().get_timer_wake_policy()), + .lock(|timer_state| timer_state.borrow().persistent_storage.get_timer_wake_policy()), ); - result.set_expiration_time( - result + self.set_expiration_time( + clock_state, + self .timer_state - .lock(|timer_state| timer_state.borrow().get_expiration_time()), + .lock(|timer_state| timer_state.borrow().persistent_storage.get_expiration_time()), ); - result.set_active(active); - - result + self.set_active(clock_state, active); } pub fn get_wake_status(&self) -> TimerStatus { @@ -143,27 +152,26 @@ impl Timer { }); } - // TODO [SPEC_QUESTION] the spec is ambiguous on whether or not this policy should include the number of seconds that have elapsed against it + // TODO [SPEC] the spec is ambiguous on whether or not this policy should include the number of seconds that have elapsed against it // (i.e. if the user set it to 60s and 45s have elapsed since we switched to the expired power source, should we report // 60s or 15s?)- see if we can get a concrete answer on this. // pub fn get_timer_wake_policy(&self) -> AlarmExpiredWakePolicy { self.timer_state - .lock(|timer_state| timer_state.borrow().get_timer_wake_policy()) + .lock(|timer_state| timer_state.borrow().persistent_storage.get_timer_wake_policy()) } - pub fn set_timer_wake_policy(&self, wake_policy: AlarmExpiredWakePolicy) { + pub fn set_timer_wake_policy(&self, clock_state: &'static Mutex>, wake_policy: AlarmExpiredWakePolicy) { self.timer_state.lock(|timer_state| { let mut timer_state = timer_state.borrow_mut(); - timer_state.set_timer_wake_policy(wake_policy); + timer_state.persistent_storage.set_timer_wake_policy(wake_policy); - // TODO [SPEC_QUESTION] verify this is correct - the spec isn't particularly clear on what should happen if reprogramming the policy while it's actively ticking down, + // TODO [SPEC] verify this is correct - the spec isn't particularly clear on what should happen if reprogramming the policy while it's actively ticking down, // may need to look at the windows acpi implementation or something // if let WakeState::ExpiredWaitingForPolicyDelay(_, _) = timer_state.wake_state { timer_state.wake_state = WakeState::ExpiredWaitingForPolicyDelay( - crate::get_current_datetime() - .expect("Datetime clock should have already been initialized before we were constructed"), + Self::get_current_datetime(clock_state), 0, ); self.timer_signal.signal(Some(wake_policy.0)); @@ -171,7 +179,7 @@ impl Timer { }) } - pub fn set_expiration_time(&self, expiration_time: Option) { + pub fn set_expiration_time(&self, clock_state: &'static Mutex>, expiration_time: Option) { self.timer_state.lock(|timer_state| { let mut timer_state = timer_state.borrow_mut(); @@ -180,14 +188,14 @@ impl Timer { match expiration_time { Some(dt) => { - timer_state.set_expiration_time(expiration_time); + timer_state.persistent_storage.set_expiration_time(expiration_time); timer_state.wake_state = WakeState::Armed; // Note: If the expiration time was in the past, this will immediately trigger the timer to expire. self.timer_signal.signal(Some( dt .to_unix_time_seconds() - .saturating_sub(crate::get_current_datetime().expect("datetime clocks should have been initialized before we were constructed").to_unix_time_seconds()).try_into() + .saturating_sub(Self::get_current_datetime(clock_state).to_unix_time_seconds()).try_into() .expect("Users should not have been able to program a time greater than u32::MAX seconds in the future - the ACPI spec prevents it") )); } @@ -198,10 +206,10 @@ impl Timer { pub fn get_expiration_time(&self) -> Option { self.timer_state - .lock(|timer_state| timer_state.borrow().get_expiration_time()) + .lock(|timer_state| timer_state.borrow().persistent_storage.get_expiration_time()) } - pub fn set_active(&self, is_active: bool) { + pub fn set_active(&self, clock_state: &'static Mutex>, is_active: bool) { self.timer_state.lock(|timer_state| { let mut timer_state = timer_state.borrow_mut(); @@ -215,12 +223,11 @@ impl Timer { if !was_active { if let WakeState::ExpiredWaitingForPowerSource(seconds_already_elapsed) = timer_state.wake_state { timer_state.wake_state = WakeState::ExpiredWaitingForPolicyDelay( - crate::get_current_datetime() - .expect("datetime clock should have already been initialized before we were constructed"), + Self::get_current_datetime(clock_state), seconds_already_elapsed, ); self.timer_signal.signal(Some( - timer_state + timer_state.persistent_storage .get_timer_wake_policy() .0 .saturating_sub(seconds_already_elapsed), @@ -231,10 +238,10 @@ impl Timer { timer_state.wake_state { let total_seconds_elapsed_on_policy_delay: u32 = seconds_elapsed_before_wait - + (crate::get_current_datetime() - .expect("Datetime clock should have already been initialized before we were constructed") + + u32::try_from(Self::get_current_datetime(clock_state) .to_unix_time_seconds() - .saturating_sub(wait_start_time.to_unix_time_seconds())) as u32; // TODO figure out how to make this build without the cast + .saturating_sub(wait_start_time.to_unix_time_seconds())) + .expect("The ACPI spec expresses timeouts in terms of u32s - it's impossible to schedule a timer u32::MAX seconds in the future"); timer_state.wake_state = WakeState::ExpiredWaitingForPowerSource(total_seconds_elapsed_on_policy_delay); @@ -244,7 +251,7 @@ impl Timer { }); } - pub(crate) async fn wait_until_wake(&self) { + pub(crate) async fn wait_until_wake(&self, clock_state: &'static Mutex>) { let mut wait_duration: Option = self.timer_signal.wait().await; loop { @@ -258,7 +265,7 @@ impl Timer { .await { Either::First(()) => { - if self.process_expired_timer() { + if self.process_expired_timer(clock_state) { return; } } @@ -278,7 +285,7 @@ impl Timer { /// Handles state changes for when the timer expires (figuring out what to do based on the current power source, etc). /// Returns true if the timer's expiry indicates that a wake event should be signaled to the host. - fn process_expired_timer(&self) -> bool { + fn process_expired_timer(&self, clock_state: &'static Mutex>) -> bool { self.timer_state.lock(|timer_state| { let mut timer_state = timer_state.borrow_mut(); @@ -289,9 +296,9 @@ impl Timer { WakeState::Clear | WakeState::ExpiredOrphaned | WakeState::ExpiredWaitingForPowerSource(_) => return false, WakeState::Armed | WakeState::ExpiredWaitingForPolicyDelay(_, _) => { - let now = crate::get_current_datetime().expect("Datetime clock should have already been initialized before we were constructed"); - let expiration_time = timer_state.get_expiration_time().expect("We should never be in the Armed or ExpiredWaitingForPolicyDelay states if there's no expiration time set"); - if now.to_unix_time_seconds() < expiration_time.to_unix_time_seconds() { // TODO we should probably implement Ord for Datetime and use that + let now = Self::get_current_datetime(clock_state); + let expiration_time = timer_state.persistent_storage.get_expiration_time().expect("We should never be in the Armed or ExpiredWaitingForPolicyDelay states if there's no expiration time set"); + if now.to_unix_time_seconds() < expiration_time.to_unix_time_seconds() { // Time hasn't actually passed the mark yet - this can happen if we were reprogrammed with a different time right as the old timer was expiring. Reset the timer. timer_state.wake_state = WakeState::Armed; self.timer_signal.signal(Some(expiration_time @@ -305,17 +312,18 @@ impl Timer { timer_state.timer_status.timer_expired = true; if timer_state.is_active { timer_state.timer_status.timer_triggered_wake = true; + timer_state.persistent_storage.set_timer_wake_policy(AlarmExpiredWakePolicy::NEVER); self.clear_expiration_time(&mut timer_state); return true; } else { - if timer_state.get_timer_wake_policy() == AlarmExpiredWakePolicy::NEVER { + if timer_state.persistent_storage.get_timer_wake_policy() == AlarmExpiredWakePolicy::NEVER { timer_state.wake_state = WakeState::ExpiredOrphaned; return false; } if let WakeState::ExpiredWaitingForPolicyDelay(_, _) = timer_state.wake_state { - timer_state.wake_state = WakeState::ExpiredWaitingForPowerSource(timer_state.get_timer_wake_policy().0); + timer_state.wake_state = WakeState::ExpiredWaitingForPowerSource(timer_state.persistent_storage.get_timer_wake_policy().0); } else { timer_state.wake_state = WakeState::ExpiredWaitingForPowerSource(0); } @@ -328,9 +336,14 @@ impl Timer { } fn clear_expiration_time(&self, timer_state: &mut TimerState) { - timer_state.set_expiration_time(None); - timer_state.set_timer_wake_policy(AlarmExpiredWakePolicy::NEVER); + timer_state.persistent_storage.set_expiration_time(None); timer_state.wake_state = WakeState::Clear; self.timer_signal.signal(None); } + + fn get_current_datetime(clock_state: &'static Mutex>) -> Datetime { + clock_state.lock(|clock_state| clock_state.borrow().datetime_clock.get_current_datetime() + .expect("Datetime clock should have already been initialized before we were constructed")) + } + } From f3546b6c08ca881dca1a9b7102611cb4fc7c905a Mon Sep 17 00:00:00 2001 From: Billy Price Date: Tue, 30 Sep 2025 09:10:56 -0700 Subject: [PATCH 03/14] cleanup if cruft --- time-alarm-service/src/timer.rs | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/time-alarm-service/src/timer.rs b/time-alarm-service/src/timer.rs index c9f3c115..db5e7663 100644 --- a/time-alarm-service/src/timer.rs +++ b/time-alarm-service/src/timer.rs @@ -233,21 +233,20 @@ impl Timer { .saturating_sub(seconds_already_elapsed), )); } - } else if was_active { - if let WakeState::ExpiredWaitingForPolicyDelay(wait_start_time, seconds_elapsed_before_wait) = + } else if let WakeState::ExpiredWaitingForPolicyDelay(wait_start_time, seconds_elapsed_before_wait) = timer_state.wake_state - { - let total_seconds_elapsed_on_policy_delay: u32 = seconds_elapsed_before_wait - + u32::try_from(Self::get_current_datetime(clock_state) - .to_unix_time_seconds() - .saturating_sub(wait_start_time.to_unix_time_seconds())) - .expect("The ACPI spec expresses timeouts in terms of u32s - it's impossible to schedule a timer u32::MAX seconds in the future"); - - timer_state.wake_state = - WakeState::ExpiredWaitingForPowerSource(total_seconds_elapsed_on_policy_delay); - self.timer_signal.signal(None); - } + { + let total_seconds_elapsed_on_policy_delay: u32 = seconds_elapsed_before_wait + + u32::try_from(Self::get_current_datetime(clock_state) + .to_unix_time_seconds() + .saturating_sub(wait_start_time.to_unix_time_seconds())) + .expect("The ACPI spec expresses timeouts in terms of u32s - it's impossible to schedule a timer u32::MAX seconds in the future"); + + timer_state.wake_state = + WakeState::ExpiredWaitingForPowerSource(total_seconds_elapsed_on_policy_delay); + self.timer_signal.signal(None); } + }); } From 3685aedf112cc9be0f2ce515fef711d7fe6cf69c Mon Sep 17 00:00:00 2001 From: Billy Price Date: Tue, 30 Sep 2025 09:13:36 -0700 Subject: [PATCH 04/14] cargo fmt --- time-alarm-service/src/acpi_timestamp.rs | 390 ++++++------- time-alarm-service/src/lib.rs | 95 +-- time-alarm-service/src/timer.rs | 701 ++++++++++++----------- 3 files changed, 597 insertions(+), 589 deletions(-) diff --git a/time-alarm-service/src/acpi_timestamp.rs b/time-alarm-service/src/acpi_timestamp.rs index 5c2fa0ac..8456f5cb 100644 --- a/time-alarm-service/src/acpi_timestamp.rs +++ b/time-alarm-service/src/acpi_timestamp.rs @@ -1,195 +1,195 @@ -use embedded_mcu_hal::time::{Datetime, UncheckedDatetime}; - -use crate::TimeAlarmError; - -// Timestamp structure as specified in the ACPI spec. Must be exactly this layout. -// TODO [TESTING] are there any endianness shenanigans associated with bytemuck here? -#[repr(C)] -#[derive(bytemuck::Pod, bytemuck::Zeroable, Copy, Clone, Debug)] -struct RawAcpiTimestamp { - // Year: 1900 - 9999 - year: u16, - - // Month: 1 - 12 - month: u8, - - // Day: 1 - 31 - day: u8, - - // Hour: 0 - 23 - hour: u8, - - // Minute: 0 - 59 - minute: u8, - - // Second: 0 - 59. Leap seconds are not supported. - second: u8, - - // For _GRT, 0 = time is not valid (request failed), 1 = time is valid. For _SRT, this is padding and should be 0. - valid_or_padding: u8, - - // Millseconds: 1-1000. Leap seconds are not supported. - // TODO [SPEC] The ACPI spec says 1-1000, but it seems like it should be 0-999? We may need to do some translation if this isn't just a typo in the spec. - milliseconds: u16, - - // Time zone: -1440 to 1440 in minutes from UTC, or 2047 if unspecified - time_zone: i16, - - // 1 = daylight savings time in effect, 0 = standard time - daylight: u8, - - // Reserved, must be 0 - _padding: [u8; 3] -} - -impl RawAcpiTimestamp { - // Try to interpret a byte slice as an AcpiTimestamp. The slice must be exactly 16 bytes long. - // Validity of the fields is not checked here. - pub fn try_from_bytes(bytes: &[u8]) -> Result { - bytemuck::try_pod_read_unaligned(bytes).map_err(|_| TimeAlarmError::InvalidArgument) - } - - // Get a byte slice representing this AcpiTimestamp. - pub fn as_bytes(&self) -> &[u8; core::mem::size_of::()] /* 16 */ { - bytemuck::bytes_of(self).try_into().expect("Should never fail because we know the size of AcpiTimestamp at compile time") - } -} - -impl From<&AcpiTimestamp> for RawAcpiTimestamp { - fn from(ts: &AcpiTimestamp) -> Self { - Self { - year: ts.datetime.year(), - month: ts.datetime.month(), - day: ts.datetime.day(), - hour: ts.datetime.hour(), - minute: ts.datetime.minute(), - second: ts.datetime.second(), - valid_or_padding: 1, // valid - milliseconds: (ts.datetime.nanoseconds() / 1_000_000).try_into().expect("Datetime::nanoseconds() is capped at 10^9 and therefore should always divide by 10^6 into something that fits in u16"), - time_zone: ts.time_zone.into(), - daylight: ts.dst_status.into(), - _padding: [0; 3], - } - } -} - -// ------------------------------------------------- - -#[derive(Copy, Clone, Debug)] -pub enum AcpiDaylightSavingsTimeStatus { - /// Daylight savings time is not observed in this timezone. - NotObserved, - - /// Daylight savings time is observed in this timezone, but the current time has not been adjusted for it. - NotAdjusted, - - /// Daylight savings time is observed in this timezone, and the current time has been adjusted for it. - Adjusted -} - -impl TryFrom for AcpiDaylightSavingsTimeStatus { - type Error = TimeAlarmError; - - fn try_from(value: u8) -> Result { - match value { - 0 => Ok(Self::NotObserved), - 1 => Ok(Self::NotAdjusted), - 3 => Ok(Self::Adjusted), - _ => Err(TimeAlarmError::InvalidArgument) - } - } -} - -impl From for u8 { - fn from(val: AcpiDaylightSavingsTimeStatus) -> Self { - match val { - AcpiDaylightSavingsTimeStatus::NotObserved => 0, - AcpiDaylightSavingsTimeStatus::NotAdjusted => 1, - AcpiDaylightSavingsTimeStatus::Adjusted => 3, - } - } -} - -// ------------------------------------------------- - -#[derive(Copy, Clone, Debug)] -pub struct AcpiTimeZoneOffset { - minutes_from_utc: i16 // minutes from UTC -} - -impl AcpiTimeZoneOffset { - pub fn new(minutes_from_utc: i16) -> Result { - if !(-1440..=1440).contains(&minutes_from_utc) { - return Err(TimeAlarmError::InvalidArgument); - } - Ok(Self { - minutes_from_utc - }) - } - - pub fn minutes_from_utc(&self) -> i16 { - self.minutes_from_utc - } -} - -#[derive(Copy, Clone, Debug)] -pub enum AcpiTimeZone { - /// The time zone is not specified and no relation to UTC can be inferred. - Unknown, - - /// The time zone is this many minutes from UTC. - MinutesFromUtc(AcpiTimeZoneOffset) -} - -impl TryFrom for AcpiTimeZone { - type Error = TimeAlarmError; - - fn try_from(value: i16) -> Result { - if value == 2047 { - Ok(Self::Unknown) - } else { - Ok(Self::MinutesFromUtc(AcpiTimeZoneOffset::new(value)?)) - } - } -} - -impl From for i16 { - fn from(val: AcpiTimeZone) -> Self { - match val { - AcpiTimeZone::Unknown => 2047, - AcpiTimeZone::MinutesFromUtc(offset) => offset.minutes_from_utc() - } - } -} - -// ------------------------------------------------- - -pub(crate) struct AcpiTimestamp { - pub datetime: Datetime, - pub time_zone: AcpiTimeZone, - pub dst_status: AcpiDaylightSavingsTimeStatus -} - -impl AcpiTimestamp { - pub fn as_bytes(&self) -> [u8; core::mem::size_of::()] /* 16 */ { - *RawAcpiTimestamp::from(self).as_bytes() - } - - pub fn try_from_bytes(bytes: &[u8]) -> Result { - let raw = RawAcpiTimestamp::try_from_bytes(bytes)?; - - Ok(Self { - datetime: Datetime::new(UncheckedDatetime { - year: raw.year, - month: raw.month, - day: raw.day, - hour: raw.hour, - minute: raw.minute, - second: raw.second, - nanosecond: (raw.milliseconds as u32) * 1_000_000, - })?, - time_zone: raw.time_zone.try_into()?, - dst_status: raw.daylight.try_into()?, - }) - } -} +use embedded_mcu_hal::time::{Datetime, UncheckedDatetime}; + +use crate::TimeAlarmError; + +// Timestamp structure as specified in the ACPI spec. Must be exactly this layout. +// TODO [TESTING] are there any endianness shenanigans associated with bytemuck here? +#[repr(C)] +#[derive(bytemuck::Pod, bytemuck::Zeroable, Copy, Clone, Debug)] +struct RawAcpiTimestamp { + // Year: 1900 - 9999 + year: u16, + + // Month: 1 - 12 + month: u8, + + // Day: 1 - 31 + day: u8, + + // Hour: 0 - 23 + hour: u8, + + // Minute: 0 - 59 + minute: u8, + + // Second: 0 - 59. Leap seconds are not supported. + second: u8, + + // For _GRT, 0 = time is not valid (request failed), 1 = time is valid. For _SRT, this is padding and should be 0. + valid_or_padding: u8, + + // Millseconds: 1-1000. Leap seconds are not supported. + // TODO [SPEC] The ACPI spec says 1-1000, but it seems like it should be 0-999? We may need to do some translation if this isn't just a typo in the spec. + milliseconds: u16, + + // Time zone: -1440 to 1440 in minutes from UTC, or 2047 if unspecified + time_zone: i16, + + // 1 = daylight savings time in effect, 0 = standard time + daylight: u8, + + // Reserved, must be 0 + _padding: [u8; 3], +} + +impl RawAcpiTimestamp { + // Try to interpret a byte slice as an AcpiTimestamp. The slice must be exactly 16 bytes long. + // Validity of the fields is not checked here. + pub fn try_from_bytes(bytes: &[u8]) -> Result { + bytemuck::try_pod_read_unaligned(bytes).map_err(|_| TimeAlarmError::InvalidArgument) + } + + // Get a byte slice representing this AcpiTimestamp. + pub fn as_bytes(&self) -> &[u8; core::mem::size_of::()] /* 16 */ { + bytemuck::bytes_of(self) + .try_into() + .expect("Should never fail because we know the size of AcpiTimestamp at compile time") + } +} + +impl From<&AcpiTimestamp> for RawAcpiTimestamp { + fn from(ts: &AcpiTimestamp) -> Self { + Self { + year: ts.datetime.year(), + month: ts.datetime.month(), + day: ts.datetime.day(), + hour: ts.datetime.hour(), + minute: ts.datetime.minute(), + second: ts.datetime.second(), + valid_or_padding: 1, // valid + milliseconds: (ts.datetime.nanoseconds() / 1_000_000).try_into().expect("Datetime::nanoseconds() is capped at 10^9 and therefore should always divide by 10^6 into something that fits in u16"), + time_zone: ts.time_zone.into(), + daylight: ts.dst_status.into(), + _padding: [0; 3], + } + } +} + +// ------------------------------------------------- + +#[derive(Copy, Clone, Debug)] +pub enum AcpiDaylightSavingsTimeStatus { + /// Daylight savings time is not observed in this timezone. + NotObserved, + + /// Daylight savings time is observed in this timezone, but the current time has not been adjusted for it. + NotAdjusted, + + /// Daylight savings time is observed in this timezone, and the current time has been adjusted for it. + Adjusted, +} + +impl TryFrom for AcpiDaylightSavingsTimeStatus { + type Error = TimeAlarmError; + + fn try_from(value: u8) -> Result { + match value { + 0 => Ok(Self::NotObserved), + 1 => Ok(Self::NotAdjusted), + 3 => Ok(Self::Adjusted), + _ => Err(TimeAlarmError::InvalidArgument), + } + } +} + +impl From for u8 { + fn from(val: AcpiDaylightSavingsTimeStatus) -> Self { + match val { + AcpiDaylightSavingsTimeStatus::NotObserved => 0, + AcpiDaylightSavingsTimeStatus::NotAdjusted => 1, + AcpiDaylightSavingsTimeStatus::Adjusted => 3, + } + } +} + +// ------------------------------------------------- + +#[derive(Copy, Clone, Debug)] +pub struct AcpiTimeZoneOffset { + minutes_from_utc: i16, // minutes from UTC +} + +impl AcpiTimeZoneOffset { + pub fn new(minutes_from_utc: i16) -> Result { + if !(-1440..=1440).contains(&minutes_from_utc) { + return Err(TimeAlarmError::InvalidArgument); + } + Ok(Self { minutes_from_utc }) + } + + pub fn minutes_from_utc(&self) -> i16 { + self.minutes_from_utc + } +} + +#[derive(Copy, Clone, Debug)] +pub enum AcpiTimeZone { + /// The time zone is not specified and no relation to UTC can be inferred. + Unknown, + + /// The time zone is this many minutes from UTC. + MinutesFromUtc(AcpiTimeZoneOffset), +} + +impl TryFrom for AcpiTimeZone { + type Error = TimeAlarmError; + + fn try_from(value: i16) -> Result { + if value == 2047 { + Ok(Self::Unknown) + } else { + Ok(Self::MinutesFromUtc(AcpiTimeZoneOffset::new(value)?)) + } + } +} + +impl From for i16 { + fn from(val: AcpiTimeZone) -> Self { + match val { + AcpiTimeZone::Unknown => 2047, + AcpiTimeZone::MinutesFromUtc(offset) => offset.minutes_from_utc(), + } + } +} + +// ------------------------------------------------- + +pub(crate) struct AcpiTimestamp { + pub datetime: Datetime, + pub time_zone: AcpiTimeZone, + pub dst_status: AcpiDaylightSavingsTimeStatus, +} + +impl AcpiTimestamp { + pub fn as_bytes(&self) -> [u8; core::mem::size_of::()] /* 16 */ { + *RawAcpiTimestamp::from(self).as_bytes() + } + + pub fn try_from_bytes(bytes: &[u8]) -> Result { + let raw = RawAcpiTimestamp::try_from_bytes(bytes)?; + + Ok(Self { + datetime: Datetime::new(UncheckedDatetime { + year: raw.year, + month: raw.month, + day: raw.day, + hour: raw.hour, + minute: raw.minute, + second: raw.second, + nanosecond: (raw.milliseconds as u32) * 1_000_000, + })?, + time_zone: raw.time_zone.try_into()?, + dst_status: raw.daylight.try_into()?, + }) + } +} diff --git a/time-alarm-service/src/lib.rs b/time-alarm-service/src/lib.rs index d88af151..c313137f 100644 --- a/time-alarm-service/src/lib.rs +++ b/time-alarm-service/src/lib.rs @@ -38,12 +38,14 @@ pub enum TimeAlarmError { impl From for MailboxDelegateError { fn from(error: TimeAlarmError) -> Self { match error { - TimeAlarmError::UnknownCommand=> MailboxDelegateError::InvalidData, - TimeAlarmError::DoubleInitError=> panic!("Should never attempt intitialization as a response to receiving a mailbox message"), - TimeAlarmError::MailboxFullError=> MailboxDelegateError::BufferFull, - TimeAlarmError::InvalidAcpiTimerId=> MailboxDelegateError::InvalidData, - TimeAlarmError::InvalidArgument=> MailboxDelegateError::InvalidData, - TimeAlarmError::ClockError(_)=> MailboxDelegateError::Other, + TimeAlarmError::UnknownCommand => MailboxDelegateError::InvalidData, + TimeAlarmError::DoubleInitError => { + panic!("Should never attempt intitialization as a response to receiving a mailbox message") + } + TimeAlarmError::MailboxFullError => MailboxDelegateError::BufferFull, + TimeAlarmError::InvalidAcpiTimerId => MailboxDelegateError::InvalidData, + TimeAlarmError::InvalidArgument => MailboxDelegateError::InvalidData, + TimeAlarmError::ClockError(_) => MailboxDelegateError::Other, } } } @@ -358,22 +360,20 @@ impl Service { ) -> Result<(), TimeAlarmError> { info!("Starting time-alarm service task"); - let service = service_storage.get_or_init(|| { - Service { - endpoint: comms::Endpoint::uninit(comms::EndpointID::Internal(comms::Internal::TimeAlarm)), - acpi_channel: Channel::new(), - clock_state: Mutex::new(RefCell::new(ClockState { - datetime_clock: backing_clock, - tz_data: TimeZoneData::new(tz_storage), - })), - power_source_signal: Signal::new(), - timers: Timers::new( - ac_expiration_storage, - ac_policy_storage, - dc_expiration_storage, - dc_policy_storage, - ), - } + let service = service_storage.get_or_init(|| Service { + endpoint: comms::Endpoint::uninit(comms::EndpointID::Internal(comms::Internal::TimeAlarm)), + acpi_channel: Channel::new(), + clock_state: Mutex::new(RefCell::new(ClockState { + datetime_clock: backing_clock, + tz_data: TimeZoneData::new(tz_storage), + })), + power_source_signal: Signal::new(), + timers: Timers::new( + ac_expiration_storage, + ac_policy_storage, + dc_expiration_storage, + dc_policy_storage, + ), }); // TODO [POWER_SOURCE] we need to subscribe to messages that tell us if we're on AC or DC power so we can decide which alarms to trigger - how do we do that? @@ -381,8 +381,7 @@ impl Service { service.timers.ac_timer.start(&service.clock_state, true); service.timers.dc_timer.start(&service.clock_state, false); - comms::register_endpoint(service, &service.endpoint) - .await?; + comms::register_endpoint(service, &service.endpoint).await?; spawner.must_spawn(command_handler_task(service)); spawner.must_spawn(timer_task(service, AcpiTimerId::AcPower)); @@ -436,7 +435,9 @@ impl Service { self.timers .get_timer(new_power_source.get_other_timer_id()) .set_active(&self.clock_state, false); - self.timers.get_timer(new_power_source).set_active(&self.clock_state, true); + self.timers + .get_timer(new_power_source) + .set_active(&self.clock_state, true); } } } @@ -470,24 +471,20 @@ impl Service { ) -> Result { info!("Received Time-Alarm Device command: {:?}", command); match command { - AcpiTimeAlarmDeviceCommand::GetRealTime => { - self.clock_state.lock(|clock_state| { - let clock_state = clock_state.borrow(); - let datetime = clock_state.datetime_clock.get_current_datetime()?; - let (time_zone, dst_status) = clock_state.tz_data.get_data(); - Ok(AcpiTimeAlarmCommandResult::Timestamp(AcpiTimestamp { - datetime, - time_zone, - dst_status, - })) - }) - } + AcpiTimeAlarmDeviceCommand::GetRealTime => self.clock_state.lock(|clock_state| { + let clock_state = clock_state.borrow(); + let datetime = clock_state.datetime_clock.get_current_datetime()?; + let (time_zone, dst_status) = clock_state.tz_data.get_data(); + Ok(AcpiTimeAlarmCommandResult::Timestamp(AcpiTimestamp { + datetime, + time_zone, + dst_status, + })) + }), AcpiTimeAlarmDeviceCommand::SetRealTime(timestamp) => { self.clock_state.lock(|clock_state| { let mut clock_state = clock_state.borrow_mut(); - clock_state - .datetime_clock - .set_current_datetime(×tamp.datetime)?; + clock_state.datetime_clock.set_current_datetime(×tamp.datetime)?; clock_state.tz_data.set_data(timestamp.time_zone, timestamp.dst_status); // TODO [SPEC] the spec is ambiguous on whether or not we should adjust any outstanding timers based on the new time - see if we can find an answer elsewhere @@ -503,7 +500,9 @@ impl Service { Ok(AcpiTimeAlarmCommandResult::Valueless) } AcpiTimeAlarmDeviceCommand::SetExpiredTimerPolicy(timer_id, timer_policy) => { - self.timers.get_timer(timer_id).set_timer_wake_policy(&self.clock_state, timer_policy); + self.timers + .get_timer(timer_id) + .set_timer_wake_policy(&self.clock_state, timer_policy); Ok(AcpiTimeAlarmCommandResult::Valueless) } AcpiTimeAlarmDeviceCommand::SetTimerValue(timer_id, timer_value) => { @@ -514,16 +513,20 @@ impl Service { .clock_state .lock(|clock_state| clock_state.borrow().datetime_clock.get_current_datetime())?; - Some(Datetime::from_unix_time_seconds(current_time.to_unix_time_seconds() + u64::from(secs))) + Some(Datetime::from_unix_time_seconds( + current_time.to_unix_time_seconds() + u64::from(secs), + )) } }; - self.timers.get_timer(timer_id).set_expiration_time(&self.clock_state, new_expiration_time); + self.timers + .get_timer(timer_id) + .set_expiration_time(&self.clock_state, new_expiration_time); Ok(AcpiTimeAlarmCommandResult::Valueless) } - AcpiTimeAlarmDeviceCommand::GetExpiredTimerPolicy(timer_id) => { - Ok(AcpiTimeAlarmCommandResult::U32(self.timers.get_timer(timer_id).get_timer_wake_policy().0)) - } + AcpiTimeAlarmDeviceCommand::GetExpiredTimerPolicy(timer_id) => Ok(AcpiTimeAlarmCommandResult::U32( + self.timers.get_timer(timer_id).get_timer_wake_policy().0, + )), AcpiTimeAlarmDeviceCommand::GetTimerValue(timer_id) => { let expiration_time = self.timers.get_timer(timer_id).get_expiration_time(); diff --git a/time-alarm-service/src/timer.rs b/time-alarm-service/src/timer.rs index db5e7663..29172e51 100644 --- a/time-alarm-service/src/timer.rs +++ b/time-alarm-service/src/timer.rs @@ -1,348 +1,353 @@ -use crate::{AlarmExpiredWakePolicy, ClockState, TimerStatus}; -use core::cell::RefCell; -use embassy_futures::select::{Either, select}; -use embassy_sync::{blocking_mutex::Mutex, signal::Signal}; -use embedded_mcu_hal::NvramStorage; -use embedded_mcu_hal::time::Datetime; -use embedded_services::GlobalRawMutex; - -/// Represents where in the timer lifecycle the current timer is -#[derive(Copy, Clone, Debug, PartialEq)] -enum WakeState { - /// Timer is not active - Clear, - /// Timer is active and programmed with the original expiration time - Armed, - /// Timer is active but expired when on the wrong power source - /// Includes the time at which we started running down the policy delay and the number of seconds that had already elapsed on the policy delay when we started waiting - ExpiredWaitingForPolicyDelay(Datetime, u32), - /// Timer is active and waiting for power source to be consistent with the timer type. - /// Includes the number of seconds that we've spent in the ExpiredWaitingForPolicyDelay state for so far. - ExpiredWaitingForPowerSource(u32), - // Expired while the policy was set to NEVER, so the timer is effectively dead until reprogrammed - ExpiredOrphaned, -} - -mod persistent_storage { - use crate::{AlarmExpiredWakePolicy, Datetime}; - use embedded_mcu_hal::NvramStorage; - - pub struct PersistentStorage { - /// When the timer is programmed to expire, or None if the timer is not set - /// This can't be part of the wake_state because we need to be able to report its value for _CWS even when the timer has expired and - /// we're handling the power source policy. - expiration_time_storage: &'static mut dyn NvramStorage<'static, u32>, - - // Persistent storage for the AlarmExpiredWakePolicy - wake_policy_storage: &'static mut dyn NvramStorage<'static, u32>, - } - - impl PersistentStorage { - pub fn new( - expiration_time_storage: &'static mut dyn NvramStorage<'static, u32>, - wake_policy_storage: &'static mut dyn NvramStorage<'static, u32>, - ) -> Self { - Self { - expiration_time_storage, - wake_policy_storage, - } - } - - const NO_EXPIRATION_TIME: u32 = u32::MAX; - - pub fn get_timer_wake_policy(&self) -> AlarmExpiredWakePolicy { - AlarmExpiredWakePolicy(self.wake_policy_storage.read()) - } - - pub fn set_timer_wake_policy(&mut self, wake_policy: AlarmExpiredWakePolicy) { - self.wake_policy_storage.write(wake_policy.0); - } - - pub fn get_expiration_time(&self) -> Option { - match self.expiration_time_storage.read() { - Self::NO_EXPIRATION_TIME => None, - secs => Some(Datetime::from_unix_time_seconds(secs.into())), - } - } - - pub fn set_expiration_time(&mut self, expiration_time: Option) { - match expiration_time { - Some(dt) => { - self.expiration_time_storage - .write(dt.to_unix_time_seconds().try_into().expect( - "Datetime::to_unix_timestamp() returns i64, which should always fit in u32 until the year 2106", - )); - } - None => { - self.expiration_time_storage.write(Self::NO_EXPIRATION_TIME); - } - } - } - } -} -use persistent_storage::PersistentStorage; - -struct TimerState { - persistent_storage: PersistentStorage, - - wake_state: WakeState, - - timer_status: TimerStatus, - - // Whether or not this timer is currently active (i.e. the system is on the power source this timer manages) - // Even if it's not active, it still counts down if it's programmed - it just won't trigger a wake event if it expires while inactive. - is_active: bool, -} - -impl TimerState { - -} - -pub(crate) struct Timer { - timer_state: Mutex>, - - timer_signal: Signal>, -} - -impl Timer { - pub fn new( - expiration_time_storage: &'static mut dyn NvramStorage<'static, u32>, - wake_policy_storage: &'static mut dyn NvramStorage<'static, u32>, - ) -> Self { - Self { - timer_state: Mutex::new(RefCell::new(TimerState { - persistent_storage: PersistentStorage::new(expiration_time_storage, wake_policy_storage), - wake_state: WakeState::Clear, - timer_status: Default::default(), - is_active: false, - })), - timer_signal: Signal::new(), - } - } - - pub fn start(&self, clock_state: &'static Mutex>, active: bool) { - self.set_timer_wake_policy( - clock_state, - self - .timer_state - .lock(|timer_state| timer_state.borrow().persistent_storage.get_timer_wake_policy()), - ); - - self.set_expiration_time( - clock_state, - self - .timer_state - .lock(|timer_state| timer_state.borrow().persistent_storage.get_expiration_time()), - ); - - self.set_active(clock_state, active); - } - - pub fn get_wake_status(&self) -> TimerStatus { - self.timer_state.lock(|timer_state| { - let timer_state = timer_state.borrow(); - timer_state.timer_status - }) - } - - pub fn clear_wake_status(&self) { - self.timer_state.lock(|timer_state| { - let mut timer_state = timer_state.borrow_mut(); - timer_state.timer_status = Default::default(); - }); - } - - // TODO [SPEC] the spec is ambiguous on whether or not this policy should include the number of seconds that have elapsed against it - // (i.e. if the user set it to 60s and 45s have elapsed since we switched to the expired power source, should we report - // 60s or 15s?)- see if we can get a concrete answer on this. - // - pub fn get_timer_wake_policy(&self) -> AlarmExpiredWakePolicy { - self.timer_state - .lock(|timer_state| timer_state.borrow().persistent_storage.get_timer_wake_policy()) - } - - pub fn set_timer_wake_policy(&self, clock_state: &'static Mutex>, wake_policy: AlarmExpiredWakePolicy) { - self.timer_state.lock(|timer_state| { - let mut timer_state = timer_state.borrow_mut(); - timer_state.persistent_storage.set_timer_wake_policy(wake_policy); - - // TODO [SPEC] verify this is correct - the spec isn't particularly clear on what should happen if reprogramming the policy while it's actively ticking down, - // may need to look at the windows acpi implementation or something - // - if let WakeState::ExpiredWaitingForPolicyDelay(_, _) = timer_state.wake_state { - timer_state.wake_state = WakeState::ExpiredWaitingForPolicyDelay( - Self::get_current_datetime(clock_state), - 0, - ); - self.timer_signal.signal(Some(wake_policy.0)); - } - }) - } - - pub fn set_expiration_time(&self, clock_state: &'static Mutex>, expiration_time: Option) { - self.timer_state.lock(|timer_state| { - let mut timer_state = timer_state.borrow_mut(); - - // Per ACPI 6.4 section 9.18.1: "The status of wake timers can be reset by setting the wake alarm". - timer_state.timer_status = Default::default(); - - match expiration_time { - Some(dt) => { - timer_state.persistent_storage.set_expiration_time(expiration_time); - timer_state.wake_state = WakeState::Armed; - - // Note: If the expiration time was in the past, this will immediately trigger the timer to expire. - self.timer_signal.signal(Some( - dt - .to_unix_time_seconds() - .saturating_sub(Self::get_current_datetime(clock_state).to_unix_time_seconds()).try_into() - .expect("Users should not have been able to program a time greater than u32::MAX seconds in the future - the ACPI spec prevents it") - )); - } - None => self.clear_expiration_time(&mut timer_state) - } - }); - } - - pub fn get_expiration_time(&self) -> Option { - self.timer_state - .lock(|timer_state| timer_state.borrow().persistent_storage.get_expiration_time()) - } - - pub fn set_active(&self, clock_state: &'static Mutex>, is_active: bool) { - self.timer_state.lock(|timer_state| { - let mut timer_state = timer_state.borrow_mut(); - - let was_active = timer_state.is_active; - timer_state.is_active = is_active; - - if was_active == is_active { - return; // No change - } - - if !was_active { - if let WakeState::ExpiredWaitingForPowerSource(seconds_already_elapsed) = timer_state.wake_state { - timer_state.wake_state = WakeState::ExpiredWaitingForPolicyDelay( - Self::get_current_datetime(clock_state), - seconds_already_elapsed, - ); - self.timer_signal.signal(Some( - timer_state.persistent_storage - .get_timer_wake_policy() - .0 - .saturating_sub(seconds_already_elapsed), - )); - } - } else if let WakeState::ExpiredWaitingForPolicyDelay(wait_start_time, seconds_elapsed_before_wait) = - timer_state.wake_state - { - let total_seconds_elapsed_on_policy_delay: u32 = seconds_elapsed_before_wait - + u32::try_from(Self::get_current_datetime(clock_state) - .to_unix_time_seconds() - .saturating_sub(wait_start_time.to_unix_time_seconds())) - .expect("The ACPI spec expresses timeouts in terms of u32s - it's impossible to schedule a timer u32::MAX seconds in the future"); - - timer_state.wake_state = - WakeState::ExpiredWaitingForPowerSource(total_seconds_elapsed_on_policy_delay); - self.timer_signal.signal(None); - } - - }); - } - - pub(crate) async fn wait_until_wake(&self, clock_state: &'static Mutex>) { - let mut wait_duration: Option = self.timer_signal.wait().await; - - loop { - 'waiting_for_timer: loop { - match wait_duration { - Some(seconds) => { - match select( - embassy_time::Timer::after_secs(seconds.into()), - self.timer_signal.wait(), - ) - .await - { - Either::First(()) => { - if self.process_expired_timer(clock_state) { - return; - } - } - Either::Second(new_wait_duration) => { - wait_duration = new_wait_duration; - } - } - } - None => { - // Wait until a new timer command comes in - break 'waiting_for_timer; - } - } - } - } - } - - /// Handles state changes for when the timer expires (figuring out what to do based on the current power source, etc). - /// Returns true if the timer's expiry indicates that a wake event should be signaled to the host. - fn process_expired_timer(&self, clock_state: &'static Mutex>) -> bool { - self.timer_state.lock(|timer_state| { - let mut timer_state = timer_state.borrow_mut(); - - match timer_state.wake_state { - // Clear: timer was disarmed right as we were waking - nothing to do. - // ExpiredOrphaned: shouldn't happen, but if we're in this state the timer should be dead, so nothing to do. - // ExpiredWaitingForPowerSource: shouldn't happen, but if we're in this state the timer is still waiting for power source so nothing to do. - WakeState::Clear | WakeState::ExpiredOrphaned | WakeState::ExpiredWaitingForPowerSource(_) => return false, - - WakeState::Armed | WakeState::ExpiredWaitingForPolicyDelay(_, _) => { - let now = Self::get_current_datetime(clock_state); - let expiration_time = timer_state.persistent_storage.get_expiration_time().expect("We should never be in the Armed or ExpiredWaitingForPolicyDelay states if there's no expiration time set"); - if now.to_unix_time_seconds() < expiration_time.to_unix_time_seconds() { - // Time hasn't actually passed the mark yet - this can happen if we were reprogrammed with a different time right as the old timer was expiring. Reset the timer. - timer_state.wake_state = WakeState::Armed; - self.timer_signal.signal(Some(expiration_time - .to_unix_time_seconds() - .saturating_sub(now.to_unix_time_seconds()) - .try_into() - .expect("Users should not have been able to program a time greater than u32::MAX seconds in the future - the ACPI spec prevents it"))); - return false; - } - - timer_state.timer_status.timer_expired = true; - if timer_state.is_active { - timer_state.timer_status.timer_triggered_wake = true; - timer_state.persistent_storage.set_timer_wake_policy(AlarmExpiredWakePolicy::NEVER); - self.clear_expiration_time(&mut timer_state); - return true; - } - else { - if timer_state.persistent_storage.get_timer_wake_policy() == AlarmExpiredWakePolicy::NEVER { - timer_state.wake_state = WakeState::ExpiredOrphaned; - return false; - } - - if let WakeState::ExpiredWaitingForPolicyDelay(_, _) = timer_state.wake_state { - timer_state.wake_state = WakeState::ExpiredWaitingForPowerSource(timer_state.persistent_storage.get_timer_wake_policy().0); - } else { - timer_state.wake_state = WakeState::ExpiredWaitingForPowerSource(0); - } - } - } - } - - false - }) - } - - fn clear_expiration_time(&self, timer_state: &mut TimerState) { - timer_state.persistent_storage.set_expiration_time(None); - timer_state.wake_state = WakeState::Clear; - self.timer_signal.signal(None); - } - - fn get_current_datetime(clock_state: &'static Mutex>) -> Datetime { - clock_state.lock(|clock_state| clock_state.borrow().datetime_clock.get_current_datetime() - .expect("Datetime clock should have already been initialized before we were constructed")) - } - -} +use crate::{AlarmExpiredWakePolicy, ClockState, TimerStatus}; +use core::cell::RefCell; +use embassy_futures::select::{Either, select}; +use embassy_sync::{blocking_mutex::Mutex, signal::Signal}; +use embedded_mcu_hal::NvramStorage; +use embedded_mcu_hal::time::Datetime; +use embedded_services::GlobalRawMutex; + +/// Represents where in the timer lifecycle the current timer is +#[derive(Copy, Clone, Debug, PartialEq)] +enum WakeState { + /// Timer is not active + Clear, + /// Timer is active and programmed with the original expiration time + Armed, + /// Timer is active but expired when on the wrong power source + /// Includes the time at which we started running down the policy delay and the number of seconds that had already elapsed on the policy delay when we started waiting + ExpiredWaitingForPolicyDelay(Datetime, u32), + /// Timer is active and waiting for power source to be consistent with the timer type. + /// Includes the number of seconds that we've spent in the ExpiredWaitingForPolicyDelay state for so far. + ExpiredWaitingForPowerSource(u32), + // Expired while the policy was set to NEVER, so the timer is effectively dead until reprogrammed + ExpiredOrphaned, +} + +mod persistent_storage { + use crate::{AlarmExpiredWakePolicy, Datetime}; + use embedded_mcu_hal::NvramStorage; + + pub struct PersistentStorage { + /// When the timer is programmed to expire, or None if the timer is not set + /// This can't be part of the wake_state because we need to be able to report its value for _CWS even when the timer has expired and + /// we're handling the power source policy. + expiration_time_storage: &'static mut dyn NvramStorage<'static, u32>, + + // Persistent storage for the AlarmExpiredWakePolicy + wake_policy_storage: &'static mut dyn NvramStorage<'static, u32>, + } + + impl PersistentStorage { + pub fn new( + expiration_time_storage: &'static mut dyn NvramStorage<'static, u32>, + wake_policy_storage: &'static mut dyn NvramStorage<'static, u32>, + ) -> Self { + Self { + expiration_time_storage, + wake_policy_storage, + } + } + + const NO_EXPIRATION_TIME: u32 = u32::MAX; + + pub fn get_timer_wake_policy(&self) -> AlarmExpiredWakePolicy { + AlarmExpiredWakePolicy(self.wake_policy_storage.read()) + } + + pub fn set_timer_wake_policy(&mut self, wake_policy: AlarmExpiredWakePolicy) { + self.wake_policy_storage.write(wake_policy.0); + } + + pub fn get_expiration_time(&self) -> Option { + match self.expiration_time_storage.read() { + Self::NO_EXPIRATION_TIME => None, + secs => Some(Datetime::from_unix_time_seconds(secs.into())), + } + } + + pub fn set_expiration_time(&mut self, expiration_time: Option) { + match expiration_time { + Some(dt) => { + self.expiration_time_storage + .write(dt.to_unix_time_seconds().try_into().expect( + "Datetime::to_unix_timestamp() returns i64, which should always fit in u32 until the year 2106", + )); + } + None => { + self.expiration_time_storage.write(Self::NO_EXPIRATION_TIME); + } + } + } + } +} +use persistent_storage::PersistentStorage; + +struct TimerState { + persistent_storage: PersistentStorage, + + wake_state: WakeState, + + timer_status: TimerStatus, + + // Whether or not this timer is currently active (i.e. the system is on the power source this timer manages) + // Even if it's not active, it still counts down if it's programmed - it just won't trigger a wake event if it expires while inactive. + is_active: bool, +} + +impl TimerState {} + +pub(crate) struct Timer { + timer_state: Mutex>, + + timer_signal: Signal>, +} + +impl Timer { + pub fn new( + expiration_time_storage: &'static mut dyn NvramStorage<'static, u32>, + wake_policy_storage: &'static mut dyn NvramStorage<'static, u32>, + ) -> Self { + Self { + timer_state: Mutex::new(RefCell::new(TimerState { + persistent_storage: PersistentStorage::new(expiration_time_storage, wake_policy_storage), + wake_state: WakeState::Clear, + timer_status: Default::default(), + is_active: false, + })), + timer_signal: Signal::new(), + } + } + + pub fn start(&self, clock_state: &'static Mutex>, active: bool) { + self.set_timer_wake_policy( + clock_state, + self.timer_state + .lock(|timer_state| timer_state.borrow().persistent_storage.get_timer_wake_policy()), + ); + + self.set_expiration_time( + clock_state, + self.timer_state + .lock(|timer_state| timer_state.borrow().persistent_storage.get_expiration_time()), + ); + + self.set_active(clock_state, active); + } + + pub fn get_wake_status(&self) -> TimerStatus { + self.timer_state.lock(|timer_state| { + let timer_state = timer_state.borrow(); + timer_state.timer_status + }) + } + + pub fn clear_wake_status(&self) { + self.timer_state.lock(|timer_state| { + let mut timer_state = timer_state.borrow_mut(); + timer_state.timer_status = Default::default(); + }); + } + + // TODO [SPEC] the spec is ambiguous on whether or not this policy should include the number of seconds that have elapsed against it + // (i.e. if the user set it to 60s and 45s have elapsed since we switched to the expired power source, should we report + // 60s or 15s?)- see if we can get a concrete answer on this. + // + pub fn get_timer_wake_policy(&self) -> AlarmExpiredWakePolicy { + self.timer_state + .lock(|timer_state| timer_state.borrow().persistent_storage.get_timer_wake_policy()) + } + + pub fn set_timer_wake_policy( + &self, + clock_state: &'static Mutex>, + wake_policy: AlarmExpiredWakePolicy, + ) { + self.timer_state.lock(|timer_state| { + let mut timer_state = timer_state.borrow_mut(); + timer_state.persistent_storage.set_timer_wake_policy(wake_policy); + + // TODO [SPEC] verify this is correct - the spec isn't particularly clear on what should happen if reprogramming the policy while it's actively ticking down, + // may need to look at the windows acpi implementation or something + // + if let WakeState::ExpiredWaitingForPolicyDelay(_, _) = timer_state.wake_state { + timer_state.wake_state = + WakeState::ExpiredWaitingForPolicyDelay(Self::get_current_datetime(clock_state), 0); + self.timer_signal.signal(Some(wake_policy.0)); + } + }) + } + + pub fn set_expiration_time( + &self, + clock_state: &'static Mutex>, + expiration_time: Option, + ) { + self.timer_state.lock(|timer_state| { + let mut timer_state = timer_state.borrow_mut(); + + // Per ACPI 6.4 section 9.18.1: "The status of wake timers can be reset by setting the wake alarm". + timer_state.timer_status = Default::default(); + + match expiration_time { + Some(dt) => { + timer_state.persistent_storage.set_expiration_time(expiration_time); + timer_state.wake_state = WakeState::Armed; + + // Note: If the expiration time was in the past, this will immediately trigger the timer to expire. + self.timer_signal.signal(Some( + dt + .to_unix_time_seconds() + .saturating_sub(Self::get_current_datetime(clock_state).to_unix_time_seconds()).try_into() + .expect("Users should not have been able to program a time greater than u32::MAX seconds in the future - the ACPI spec prevents it") + )); + } + None => self.clear_expiration_time(&mut timer_state) + } + }); + } + + pub fn get_expiration_time(&self) -> Option { + self.timer_state + .lock(|timer_state| timer_state.borrow().persistent_storage.get_expiration_time()) + } + + pub fn set_active(&self, clock_state: &'static Mutex>, is_active: bool) { + self.timer_state.lock(|timer_state| { + let mut timer_state = timer_state.borrow_mut(); + + let was_active = timer_state.is_active; + timer_state.is_active = is_active; + + if was_active == is_active { + return; // No change + } + + if !was_active { + if let WakeState::ExpiredWaitingForPowerSource(seconds_already_elapsed) = timer_state.wake_state { + timer_state.wake_state = WakeState::ExpiredWaitingForPolicyDelay( + Self::get_current_datetime(clock_state), + seconds_already_elapsed, + ); + self.timer_signal.signal(Some( + timer_state.persistent_storage + .get_timer_wake_policy() + .0 + .saturating_sub(seconds_already_elapsed), + )); + } + } else if let WakeState::ExpiredWaitingForPolicyDelay(wait_start_time, seconds_elapsed_before_wait) = + timer_state.wake_state + { + let total_seconds_elapsed_on_policy_delay: u32 = seconds_elapsed_before_wait + + u32::try_from(Self::get_current_datetime(clock_state) + .to_unix_time_seconds() + .saturating_sub(wait_start_time.to_unix_time_seconds())) + .expect("The ACPI spec expresses timeouts in terms of u32s - it's impossible to schedule a timer u32::MAX seconds in the future"); + + timer_state.wake_state = + WakeState::ExpiredWaitingForPowerSource(total_seconds_elapsed_on_policy_delay); + self.timer_signal.signal(None); + } + }); + } + + pub(crate) async fn wait_until_wake(&self, clock_state: &'static Mutex>) { + let mut wait_duration: Option = self.timer_signal.wait().await; + + loop { + 'waiting_for_timer: loop { + match wait_duration { + Some(seconds) => { + match select( + embassy_time::Timer::after_secs(seconds.into()), + self.timer_signal.wait(), + ) + .await + { + Either::First(()) => { + if self.process_expired_timer(clock_state) { + return; + } + } + Either::Second(new_wait_duration) => { + wait_duration = new_wait_duration; + } + } + } + None => { + // Wait until a new timer command comes in + break 'waiting_for_timer; + } + } + } + } + } + + /// Handles state changes for when the timer expires (figuring out what to do based on the current power source, etc). + /// Returns true if the timer's expiry indicates that a wake event should be signaled to the host. + fn process_expired_timer(&self, clock_state: &'static Mutex>) -> bool { + self.timer_state.lock(|timer_state| { + let mut timer_state = timer_state.borrow_mut(); + + match timer_state.wake_state { + // Clear: timer was disarmed right as we were waking - nothing to do. + // ExpiredOrphaned: shouldn't happen, but if we're in this state the timer should be dead, so nothing to do. + // ExpiredWaitingForPowerSource: shouldn't happen, but if we're in this state the timer is still waiting for power source so nothing to do. + WakeState::Clear | WakeState::ExpiredOrphaned | WakeState::ExpiredWaitingForPowerSource(_) => return false, + + WakeState::Armed | WakeState::ExpiredWaitingForPolicyDelay(_, _) => { + let now = Self::get_current_datetime(clock_state); + let expiration_time = timer_state.persistent_storage.get_expiration_time().expect("We should never be in the Armed or ExpiredWaitingForPolicyDelay states if there's no expiration time set"); + if now.to_unix_time_seconds() < expiration_time.to_unix_time_seconds() { + // Time hasn't actually passed the mark yet - this can happen if we were reprogrammed with a different time right as the old timer was expiring. Reset the timer. + timer_state.wake_state = WakeState::Armed; + self.timer_signal.signal(Some(expiration_time + .to_unix_time_seconds() + .saturating_sub(now.to_unix_time_seconds()) + .try_into() + .expect("Users should not have been able to program a time greater than u32::MAX seconds in the future - the ACPI spec prevents it"))); + return false; + } + + timer_state.timer_status.timer_expired = true; + if timer_state.is_active { + timer_state.timer_status.timer_triggered_wake = true; + timer_state.persistent_storage.set_timer_wake_policy(AlarmExpiredWakePolicy::NEVER); + self.clear_expiration_time(&mut timer_state); + return true; + } + else { + if timer_state.persistent_storage.get_timer_wake_policy() == AlarmExpiredWakePolicy::NEVER { + timer_state.wake_state = WakeState::ExpiredOrphaned; + return false; + } + + if let WakeState::ExpiredWaitingForPolicyDelay(_, _) = timer_state.wake_state { + timer_state.wake_state = WakeState::ExpiredWaitingForPowerSource(timer_state.persistent_storage.get_timer_wake_policy().0); + } else { + timer_state.wake_state = WakeState::ExpiredWaitingForPowerSource(0); + } + } + } + } + + false + }) + } + + fn clear_expiration_time(&self, timer_state: &mut TimerState) { + timer_state.persistent_storage.set_expiration_time(None); + timer_state.wake_state = WakeState::Clear; + self.timer_signal.signal(None); + } + + fn get_current_datetime(clock_state: &'static Mutex>) -> Datetime { + clock_state.lock(|clock_state| { + clock_state + .borrow() + .datetime_clock + .get_current_datetime() + .expect("Datetime clock should have already been initialized before we were constructed") + }) + } +} From 262d479c4cdb65350e915af963e27d8374af89aa Mon Sep 17 00:00:00 2001 From: Billy Price Date: Tue, 30 Sep 2025 09:30:35 -0700 Subject: [PATCH 05/14] fix wait --- time-alarm-service/src/timer.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/time-alarm-service/src/timer.rs b/time-alarm-service/src/timer.rs index 29172e51..9c98a721 100644 --- a/time-alarm-service/src/timer.rs +++ b/time-alarm-service/src/timer.rs @@ -252,9 +252,8 @@ impl Timer { } pub(crate) async fn wait_until_wake(&self, clock_state: &'static Mutex>) { - let mut wait_duration: Option = self.timer_signal.wait().await; - loop { + let mut wait_duration: Option = self.timer_signal.wait().await; 'waiting_for_timer: loop { match wait_duration { Some(seconds) => { From fe89ac97489460120fd0cebbc6cdb52d7ab5d047 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Tue, 30 Sep 2025 10:12:20 -0700 Subject: [PATCH 06/14] comments --- embedded-service/src/ec_type/structure.rs | 1 - time-alarm-service/src/acpi_timestamp.rs | 1 + time-alarm-service/src/lib.rs | 10 +++++++--- time-alarm-service/src/timer.rs | 2 -- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/embedded-service/src/ec_type/structure.rs b/embedded-service/src/ec_type/structure.rs index 7d17b006..558b9d47 100644 --- a/embedded-service/src/ec_type/structure.rs +++ b/embedded-service/src/ec_type/structure.rs @@ -1,6 +1,5 @@ //! EC Internal Data Structures -// TODO need to rev? do we need to update yaml? #[allow(missing_docs)] pub const EC_MEMMAP_VERSION: Version = Version { major: 0, diff --git a/time-alarm-service/src/acpi_timestamp.rs b/time-alarm-service/src/acpi_timestamp.rs index 8456f5cb..ffc26756 100644 --- a/time-alarm-service/src/acpi_timestamp.rs +++ b/time-alarm-service/src/acpi_timestamp.rs @@ -96,6 +96,7 @@ impl TryFrom for AcpiDaylightSavingsTimeStatus { match value { 0 => Ok(Self::NotObserved), 1 => Ok(Self::NotAdjusted), + // 2 would be Adjusted but not Observed, which is nonsensical, so omitted. 3 => Ok(Self::Adjusted), _ => Err(TimeAlarmError::InvalidArgument), } diff --git a/time-alarm-service/src/lib.rs b/time-alarm-service/src/lib.rs index c313137f..b44b9360 100644 --- a/time-alarm-service/src/lib.rs +++ b/time-alarm-service/src/lib.rs @@ -174,10 +174,14 @@ impl AcpiTimeAlarmDeviceCommand { // We need to make sure this is actually how we implement it over eSPI, or adapt to match what eSPI actually sends us. const COMMAND_CODE_SIZE_BYTES: usize = core::mem::size_of::(); - let command_code = u32::from_le_bytes(bytes.try_into()?); - + let command_code = u32::from_le_bytes( + bytes + .get(..COMMAND_CODE_SIZE_BYTES) + .ok_or(TimeAlarmError::InvalidArgument)? + .try_into()?, + ); let bytes = bytes.get(COMMAND_CODE_SIZE_BYTES..) - .expect("Should never fail because if there were less than 4 bytes, u32::from_le_bytes would have failed. If there were exactly four bytes, this will return an empty slice, not None."); + .expect("Should never fail because if there were less than 4 bytes, parsing the command code would have failed. If there were exactly four bytes, this will return an empty slice, not None."); match command_code { 1 => Ok(AcpiTimeAlarmDeviceCommand::GetRealTime), diff --git a/time-alarm-service/src/timer.rs b/time-alarm-service/src/timer.rs index 9c98a721..4f38b495 100644 --- a/time-alarm-service/src/timer.rs +++ b/time-alarm-service/src/timer.rs @@ -94,8 +94,6 @@ struct TimerState { is_active: bool, } -impl TimerState {} - pub(crate) struct Timer { timer_state: Mutex>, From d851bc8e9c15a6874b68f8f3919437b4d52acf40 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Wed, 1 Oct 2025 17:04:06 -0700 Subject: [PATCH 07/14] put response in single buffer --- time-alarm-service/src/lib.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/time-alarm-service/src/lib.rs b/time-alarm-service/src/lib.rs index b44b9360..4d76830f 100644 --- a/time-alarm-service/src/lib.rs +++ b/time-alarm-service/src/lib.rs @@ -407,29 +407,34 @@ impl Service { let acpi_result = self.handle_acpi_command(acpi_command).await; match acpi_result { Ok(response_payload) => { - // TODO [COMMS] is it a problem that we're sending the response in two pieces? If yes, we may need to - // arrange it into a buffer or something. May not be worth solving if we're looking to pivot - // to using something like postcard for this, though. - // // TODO [COMMS] it seems like we're sort of conflating wire representation with message representation here - // is this really how we want to pass messages through the comms system? It seems like it makes it - // harder for other services to send messages to us. May change with postcard? + // harder for other services to send messages to us - we're obligated to serialize/deserialize messages + // whenever we send them to another subsystem on the MCU rather than just passing around strongly-typed + // objects. We may want to consider changing the comms system to allow passing strongly-typed objects and + // perhaps a trait that indicates if it's serializable for an off-system transport like eSPI? // self.send_acpi_response(respond_to_endpoint, &COMMAND_SUCCEEDED).await; match response_payload { AcpiTimeAlarmCommandResult::Timestamp(timestamp) => { - self.send_acpi_response(respond_to_endpoint, ×tamp.as_bytes()) - .await + let mut response = [0u8; 4 + core::mem::size_of::()]; + response[..4].copy_from_slice(&COMMAND_SUCCEEDED.to_le_bytes()); + response[4..].copy_from_slice(×tamp.as_bytes()); + self.send_acpi_response(respond_to_endpoint, &response).await } AcpiTimeAlarmCommandResult::U32(value) => { + let mut response = [0u8; 8]; + response[..4].copy_from_slice(&COMMAND_SUCCEEDED.to_le_bytes()); + response[4..].copy_from_slice(&value.to_le_bytes()); self.send_acpi_response(respond_to_endpoint, &value).await } - AcpiTimeAlarmCommandResult::Valueless => (), // nothing more to send + AcpiTimeAlarmCommandResult::Valueless => + self.send_acpi_response(respond_to_endpoint, &COMMAND_SUCCEEDED.to_le_bytes()).await } } Err(e) => { error!("Error handling ACPI command: {:?}", e); - self.send_acpi_response(respond_to_endpoint, &COMMAND_FAILED).await; + self.send_acpi_response(respond_to_endpoint, &COMMAND_FAILED.to_le_bytes()).await; } } } From 6ab25d670fc00b434e469d3564c6fea07307a7df Mon Sep 17 00:00:00 2001 From: Billy Price Date: Wed, 1 Oct 2025 17:24:32 -0700 Subject: [PATCH 08/14] bitfield --- Cargo.lock | 1 + time-alarm-service/Cargo.toml | 1 + time-alarm-service/src/lib.rs | 32 ++++++++++++-------------------- time-alarm-service/src/timer.rs | 4 ++-- 4 files changed, 16 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 500ddf7a..ba3bd2b7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1995,6 +1995,7 @@ dependencies = [ name = "time-alarm-service" version = "0.1.0" dependencies = [ + "bitfield 0.17.0", "bytemuck", "defmt 0.3.100", "embassy-executor", diff --git a/time-alarm-service/Cargo.toml b/time-alarm-service/Cargo.toml index 8d3eb981..256f7197 100644 --- a/time-alarm-service/Cargo.toml +++ b/time-alarm-service/Cargo.toml @@ -7,6 +7,7 @@ license.workspace = true repository.workspace = true [dependencies] +bitfield.workspace = true bytemuck.workspace = true defmt = { workspace = true, optional = true } log = { workspace = true, optional = true } diff --git a/time-alarm-service/src/lib.rs b/time-alarm-service/src/lib.rs index 4d76830f..641c4992 100644 --- a/time-alarm-service/src/lib.rs +++ b/time-alarm-service/src/lib.rs @@ -1,5 +1,6 @@ #![no_std] +use bitfield::bitfield; use core::any::Any; use core::array::TryFromSliceError; use core::borrow::Borrow; @@ -16,6 +17,7 @@ use embedded_mcu_hal::NvramStorage; use embedded_mcu_hal::time::{Datetime, DatetimeClock, DatetimeClockError}; use embedded_services::{comms, error, info, trace}; + mod acpi_timestamp; use acpi_timestamp::{AcpiDaylightSavingsTimeStatus, AcpiTimeZone, AcpiTimestamp}; @@ -281,25 +283,15 @@ struct ClockState { tz_data: TimeZoneData, } -// TODO see if there's some sort of bitfield crate that can make this cleaner -#[derive(Copy, Clone, Debug, Default)] -struct TimerStatus { - timer_expired: bool, - timer_triggered_wake: bool, -} - -impl From for u32 { - fn from(value: TimerStatus) -> Self { - let mut result = 0u32; - if value.timer_expired { - result |= 0x1; - } - if value.timer_triggered_wake { - result |= 0x2; - } - result - } -} +bitfield!( + #[derive(Copy, Clone, Default, PartialEq, Eq)] + #[cfg_attr(feature = "defmt", derive(defmt::Format))] + pub struct TimerStatus(u32); + impl Debug; + bool; + timer_expired, set_timer_expired: 0; + timer_triggered_wake, set_timer_triggered_wake: 1; +); // ------------------------------------------------- @@ -502,7 +494,7 @@ impl Service { } AcpiTimeAlarmDeviceCommand::GetWakeStatus(timer_id) => { let status = self.timers.get_timer(timer_id).get_wake_status(); - Ok(AcpiTimeAlarmCommandResult::U32(status.into())) + Ok(AcpiTimeAlarmCommandResult::U32(status.0)) } AcpiTimeAlarmDeviceCommand::ClearWakeStatus(timer_id) => { self.timers.get_timer(timer_id).clear_wake_status(); diff --git a/time-alarm-service/src/timer.rs b/time-alarm-service/src/timer.rs index 4f38b495..130691ee 100644 --- a/time-alarm-service/src/timer.rs +++ b/time-alarm-service/src/timer.rs @@ -306,9 +306,9 @@ impl Timer { return false; } - timer_state.timer_status.timer_expired = true; + timer_state.timer_status.set_timer_expired(true); if timer_state.is_active { - timer_state.timer_status.timer_triggered_wake = true; + timer_state.timer_status.set_timer_triggered_wake(true); timer_state.persistent_storage.set_timer_wake_policy(AlarmExpiredWakePolicy::NEVER); self.clear_expiration_time(&mut timer_state); return true; From 1a398d4a8c2f61d6f9705717b53c77f5b657f412 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Thu, 2 Oct 2025 14:33:45 -0700 Subject: [PATCH 09/14] Fix defmt build --- time-alarm-service/src/acpi_timestamp.rs | 5 +++++ time-alarm-service/src/lib.rs | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/time-alarm-service/src/acpi_timestamp.rs b/time-alarm-service/src/acpi_timestamp.rs index ffc26756..8aa145c3 100644 --- a/time-alarm-service/src/acpi_timestamp.rs +++ b/time-alarm-service/src/acpi_timestamp.rs @@ -6,6 +6,7 @@ use crate::TimeAlarmError; // TODO [TESTING] are there any endianness shenanigans associated with bytemuck here? #[repr(C)] #[derive(bytemuck::Pod, bytemuck::Zeroable, Copy, Clone, Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] struct RawAcpiTimestamp { // Year: 1900 - 9999 year: u16, @@ -78,6 +79,7 @@ impl From<&AcpiTimestamp> for RawAcpiTimestamp { // ------------------------------------------------- #[derive(Copy, Clone, Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum AcpiDaylightSavingsTimeStatus { /// Daylight savings time is not observed in this timezone. NotObserved, @@ -116,6 +118,7 @@ impl From for u8 { // ------------------------------------------------- #[derive(Copy, Clone, Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] pub struct AcpiTimeZoneOffset { minutes_from_utc: i16, // minutes from UTC } @@ -134,6 +137,7 @@ impl AcpiTimeZoneOffset { } #[derive(Copy, Clone, Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum AcpiTimeZone { /// The time zone is not specified and no relation to UTC can be inferred. Unknown, @@ -165,6 +169,7 @@ impl From for i16 { // ------------------------------------------------- +#[cfg_attr(feature = "defmt", derive(defmt::Format))] pub(crate) struct AcpiTimestamp { pub datetime: Datetime, pub time_zone: AcpiTimeZone, diff --git a/time-alarm-service/src/lib.rs b/time-alarm-service/src/lib.rs index 641c4992..219a7d3b 100644 --- a/time-alarm-service/src/lib.rs +++ b/time-alarm-service/src/lib.rs @@ -80,6 +80,7 @@ impl From for TimeAlarmError { // Timer ID as defined in the ACPI spec. #[derive(Debug, Clone, Copy, PartialEq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum AcpiTimerId { AcPower, DcPower, @@ -123,6 +124,7 @@ impl TryFrom for AcpiTimerId { // ------------------------------------------------- #[derive(Clone, Copy, Debug, PartialEq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] struct AlarmTimerSeconds(u32); impl AlarmTimerSeconds { pub const DISABLED: Self = Self(u32::MAX); @@ -135,6 +137,7 @@ impl Default for AlarmTimerSeconds { } #[derive(Clone, Copy, Debug, PartialEq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] struct AlarmExpiredWakePolicy(u32); impl AlarmExpiredWakePolicy { #[allow(dead_code)] @@ -153,6 +156,7 @@ impl Default for AlarmExpiredWakePolicy { /// Represents an ACPI Time and Alarm Device command. /// See ACPI Specification 6.4, Section 9.18 "Time and Alarm Device" for details on semantics. #[rustfmt::skip] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] enum AcpiTimeAlarmDeviceCommand { // Notably missing from the ACPI spec here is _GCP / 'Get Capabilities'. It just returns a constant and is expected to be implemented wholly in the ACPI ASL code. From d227646d1a052c94c1705ef9fa6da8440fefa283 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Thu, 2 Oct 2025 14:55:53 -0700 Subject: [PATCH 10/14] fix arg type for storage --- time-alarm-service/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/time-alarm-service/src/lib.rs b/time-alarm-service/src/lib.rs index 219a7d3b..f9fe4d06 100644 --- a/time-alarm-service/src/lib.rs +++ b/time-alarm-service/src/lib.rs @@ -349,7 +349,7 @@ impl Service { // on embassy task implementation. // pub async fn init( - service_storage: &'static mut OnceLock, + service_storage: &'static OnceLock, spawner: &embassy_executor::Spawner, backing_clock: &'static mut impl DatetimeClock, tz_storage: &'static mut dyn NvramStorage<'static, u32>, From d8bdced67a942dd601f56d53f6773ca2a29c73d8 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Fri, 3 Oct 2025 14:27:16 -0700 Subject: [PATCH 11/14] fix a couple bugs --- time-alarm-service/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/time-alarm-service/src/lib.rs b/time-alarm-service/src/lib.rs index f9fe4d06..256f2a54 100644 --- a/time-alarm-service/src/lib.rs +++ b/time-alarm-service/src/lib.rs @@ -410,10 +410,11 @@ impl Service { // objects. We may want to consider changing the comms system to allow passing strongly-typed objects and // perhaps a trait that indicates if it's serializable for an off-system transport like eSPI? // - self.send_acpi_response(respond_to_endpoint, &COMMAND_SUCCEEDED).await; match response_payload { AcpiTimeAlarmCommandResult::Timestamp(timestamp) => { - let mut response = [0u8; 4 + core::mem::size_of::()]; + // TODO I want to be able to say '4 + sizeof(decltype(timestamp.as_bytes()))' here but I don't know how to do that in rust - is there a cleaner way to learn the size of the return type of a function? + const ACPI_TIMESTAMP_BYTES_SIZE: usize = 16; + let mut response = [0u8; 4 + ACPI_TIMESTAMP_BYTES_SIZE]; response[..4].copy_from_slice(&COMMAND_SUCCEEDED.to_le_bytes()); response[4..].copy_from_slice(×tamp.as_bytes()); self.send_acpi_response(respond_to_endpoint, &response).await @@ -586,7 +587,7 @@ async fn command_handler_task(service: &'static Service) { service.handle_requests().await; } -#[embassy_executor::task] +#[embassy_executor::task(pool_size = 2)] async fn timer_task(service: &'static Service, timer_id: AcpiTimerId) { info!("Starting time-alarm timer task"); service.handle_timer(timer_id).await; From 6a5bd779aae7371c3c682839e583181b6c0aa4e8 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Thu, 9 Oct 2025 16:20:46 -0700 Subject: [PATCH 12/14] initial example service - needs more test coverage, but work is paused pending upcoming changes to the comms system --- examples/rt685s-evk/Cargo.lock | 31 ++++++ examples/rt685s-evk/Cargo.toml | 2 + examples/rt685s-evk/src/bin/time_alarm.rs | 128 ++++++++++++++++++++++ time-alarm-service/src/lib.rs | 82 ++++++++++---- 4 files changed, 223 insertions(+), 20 deletions(-) create mode 100644 examples/rt685s-evk/src/bin/time_alarm.rs diff --git a/examples/rt685s-evk/Cargo.lock b/examples/rt685s-evk/Cargo.lock index cc1e5ce4..33dc079e 100644 --- a/examples/rt685s-evk/Cargo.lock +++ b/examples/rt685s-evk/Cargo.lock @@ -204,6 +204,20 @@ name = "bytemuck" version = "1.23.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3995eaeebcdf32f91f980d360f78732ddc061097ab4e39991ae7a6ace9194677" +dependencies = [ + "bytemuck_derive", +] + +[[package]] +name = "bytemuck_derive" +version = "1.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4f154e572231cb6ba2bd1176980827e3d5dc04cc183a75dea38109fbdd672d29" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] [[package]] name = "byteorder" @@ -1346,6 +1360,7 @@ dependencies = [ "embedded-cfu-protocol", "embedded-hal 1.0.0", "embedded-hal-async", + "embedded-mcu-hal", "embedded-services", "embedded-usb-pd", "futures", @@ -1356,6 +1371,7 @@ dependencies = [ "power-button-service", "power-policy-service", "static_cell", + "time-alarm-service", "tps6699x", "type-c-service", ] @@ -1541,6 +1557,21 @@ dependencies = [ "syn", ] +[[package]] +name = "time-alarm-service" +version = "0.1.0" +dependencies = [ + "bitfield 0.17.0", + "bytemuck", + "defmt 0.3.100", + "embassy-executor", + "embassy-futures", + "embassy-sync", + "embassy-time", + "embedded-mcu-hal", + "embedded-services", +] + [[package]] name = "tps6699x" version = "0.1.0" diff --git a/examples/rt685s-evk/Cargo.toml b/examples/rt685s-evk/Cargo.toml index f38dcedc..94bfd4d9 100644 --- a/examples/rt685s-evk/Cargo.toml +++ b/examples/rt685s-evk/Cargo.toml @@ -69,6 +69,7 @@ embedded-usb-pd = { git = "https://github.com/OpenDevicePartnership/embedded-usb "defmt", ] } type-c-service = { path = "../../type-c-service", features = ["defmt"] } +time-alarm-service = { path = "../../time-alarm-service", features = ["defmt"] } static_cell = "2.1.0" embedded-hal = "1.0.0" @@ -78,6 +79,7 @@ platform-service = { path = "../../platform-service", features = [ "defmt", "imxrt685", ] } +embedded-mcu-hal = { git = "https://github.com/OpenDevicePartnership/embedded-mcu" } # Needed otherwise cargo will pull from git [patch."https://github.com/OpenDevicePartnership/embedded-services"] diff --git a/examples/rt685s-evk/src/bin/time_alarm.rs b/examples/rt685s-evk/src/bin/time_alarm.rs new file mode 100644 index 00000000..2e69482c --- /dev/null +++ b/examples/rt685s-evk/src/bin/time_alarm.rs @@ -0,0 +1,128 @@ +#![no_std] +#![no_main] + +use embassy_sync::once_lock::OnceLock; +use embedded_mcu_hal::Nvram; +use embedded_services::{error, info}; +use static_cell::StaticCell; +use {defmt_rtt as _, panic_probe as _}; + +mod mock_espi_service { + use crate::OnceLock; + use crate::{error, info}; + use core::borrow::{Borrow, BorrowMut}; + use embassy_time::{Duration, Ticker}; + use embedded_services::buffer::OwnedRef; + use embedded_services::comms::{self, EndpointID, External, Internal}; + use embedded_services::ec_type::message::AcpiMsgComms; + use embedded_services::ec_type::message::HostMsg; + + embedded_services::define_static_buffer!(acpi_buf, u8, [0u8; 69]); + + pub struct Service { + endpoint: comms::Endpoint, + acpi_buf_owned_ref: OwnedRef<'static, u8>, + } + + impl Service { + pub async fn init(spawner: embassy_executor::Spawner, service_storage: &'static OnceLock) { + let instance = service_storage.get_or_init(|| Service { + endpoint: comms::Endpoint::uninit(EndpointID::External(External::Host)), + acpi_buf_owned_ref: acpi_buf::get_mut().unwrap(), + }); + + comms::register_endpoint(instance, &instance.endpoint).await.unwrap(); + + spawner.must_spawn(run_mock_service(instance)); + } + } + + impl comms::MailboxDelegate for Service { + fn receive(&self, message: &comms::Message) -> Result<(), comms::MailboxDelegateError> { + info!("mock eSPI service received message from time-alarm service"); + let msg = message.data.get::().ok_or_else(|| { + error!("Mock eSPI service received unknown message type"); + comms::MailboxDelegateError::MessageNotFound + })?; + + match msg { + HostMsg::Notification(n) => { + info!("Notification: offset={}", n.offset); + } + HostMsg::Response(acpi) => { + let payload = acpi.payload.borrow(); + let payload_slice: &[u8] = payload.borrow(); + info!( + "Response: payload_len={}, payload={:?}", + acpi.payload_len, + &payload_slice[..acpi.payload_len] + ); + } + } + + Ok(()) + } + } + + // espi service that will update the memory map + #[embassy_executor::task] + async fn run_mock_service(espi_service: &'static Service) { + let mut ticker = Ticker::every(Duration::from_secs(1)); + + loop { + // let event = select(espi_service.signal.wait(), ticker.next()).await; + ticker.next().await; + + let payload_len = { + // TODO alternate between different messages. + let mut buffer_access = espi_service.acpi_buf_owned_ref.borrow_mut(); + let buffer: &mut [u8] = buffer_access.borrow_mut(); + buffer[0] = 2; + + 4 // u32 + }; + + let message = AcpiMsgComms { + payload: acpi_buf::get(), + payload_len, + }; + + espi_service + .endpoint + .send(EndpointID::Internal(Internal::TimeAlarm), &message) + .await + .unwrap(); + } + } +} + +#[embassy_executor::main] +async fn main(spawner: embassy_executor::Spawner) { + let p = embassy_imxrt::init(Default::default()); + + static RTC: StaticCell = StaticCell::new(); + let rtc = RTC.init(embassy_imxrt::rtc::Rtc::new(p.RTC)); + let (dt_clock, rtc_nvram) = rtc.split(); + + let [tz, ac_expiration, ac_policy, dc_expiration, dc_policy, ..] = rtc_nvram.storage(); + + embedded_services::init().await; + info!("services initialized"); + + static MOCK_ESPI_SERVICE: OnceLock = OnceLock::new(); + mock_espi_service::Service::init(spawner, &MOCK_ESPI_SERVICE).await; + + static TIME_ALARM_SERVICE: OnceLock = OnceLock::new(); + time_alarm_service::Service::init( + &TIME_ALARM_SERVICE, + &spawner, + dt_clock, + tz, + ac_expiration, + ac_policy, + dc_expiration, + dc_policy, + ) + .await + .unwrap(); +} diff --git a/time-alarm-service/src/lib.rs b/time-alarm-service/src/lib.rs index 256f2a54..0e4204bd 100644 --- a/time-alarm-service/src/lib.rs +++ b/time-alarm-service/src/lib.rs @@ -1,9 +1,8 @@ #![no_std] use bitfield::bitfield; -use core::any::Any; use core::array::TryFromSliceError; -use core::borrow::Borrow; +use core::borrow::{Borrow, BorrowMut}; use core::cell::RefCell; use embassy_futures::select::{Either, select}; use embassy_sync::blocking_mutex::Mutex; @@ -12,7 +11,8 @@ use embassy_sync::once_lock::OnceLock; use embassy_sync::signal::Signal; use embedded_services::ec_type::message::AcpiMsgComms; use embedded_services::{GlobalRawMutex, comms::MailboxDelegateError}; - +use embedded_services::buffer::OwnedRef; +use embedded_services::ec_type::message::HostMsg; use embedded_mcu_hal::NvramStorage; use embedded_mcu_hal::time::{Datetime, DatetimeClock, DatetimeClockError}; use embedded_services::{comms, error, info, trace}; @@ -339,8 +339,19 @@ pub struct Service { power_source_signal: Signal, timers: Timers, + + acpi_buf_owned_ref: OwnedRef<'static, u8> } +// TODO [STORAGE] This statically allocates a buffer oustide the Service struct to hold ACPI responses. This is a common pattern in this repo, +// but sort-of breaks the goal of allowing the caller to allocate storage. It's not as punishing as statically allocating +// the Service struct at this layer because it doesn't prevent us from knowing the concrete type of any generics passed in, +// but it does mean we can't have multiple instances of the service and if we do we'll be at risk of crashing due to buffer +// contention. For the clock this may not be an issue because there's typically only one clock, but we may want to explore +// options for better patterns for other service and adopt those here for uniformity. +// +embedded_services::define_static_buffer!(acpi_buf, u8, [0u8; 69]); + impl Service { // TODO [DYN] if we want to allow taking the HAL traits as concrete types rather than as dyn references, we'll likely need to make this a macro // in order to accommodate the restriction that embassy tasks can't have generic parameters. When we do that, it may be worthwhile to @@ -374,6 +385,7 @@ impl Service { dc_expiration_storage, dc_policy_storage, ), + acpi_buf_owned_ref: acpi_buf::get_mut().unwrap(), }); // TODO [POWER_SOURCE] we need to subscribe to messages that tell us if we're on AC or DC power so we can decide which alarms to trigger - how do we do that? @@ -397,9 +409,6 @@ impl Service { match select(acpi_command, power_source_change).await { Either::First((respond_to_endpoint, acpi_command)) => { - const COMMAND_SUCCEEDED: u32 = 1; - const COMMAND_FAILED: u32 = 0; - let acpi_result = self.handle_acpi_command(acpi_command).await; match acpi_result { Ok(response_payload) => { @@ -412,26 +421,18 @@ impl Service { // match response_payload { AcpiTimeAlarmCommandResult::Timestamp(timestamp) => { - // TODO I want to be able to say '4 + sizeof(decltype(timestamp.as_bytes()))' here but I don't know how to do that in rust - is there a cleaner way to learn the size of the return type of a function? - const ACPI_TIMESTAMP_BYTES_SIZE: usize = 16; - let mut response = [0u8; 4 + ACPI_TIMESTAMP_BYTES_SIZE]; - response[..4].copy_from_slice(&COMMAND_SUCCEEDED.to_le_bytes()); - response[4..].copy_from_slice(×tamp.as_bytes()); - self.send_acpi_response(respond_to_endpoint, &response).await + self.send_acpi_response(respond_to_endpoint, Some(×tamp.as_bytes())).await } AcpiTimeAlarmCommandResult::U32(value) => { - let mut response = [0u8; 8]; - response[..4].copy_from_slice(&COMMAND_SUCCEEDED.to_le_bytes()); - response[4..].copy_from_slice(&value.to_le_bytes()); - self.send_acpi_response(respond_to_endpoint, &value).await + self.send_acpi_response(respond_to_endpoint, Some(&value.to_le_bytes())).await } AcpiTimeAlarmCommandResult::Valueless => - self.send_acpi_response(respond_to_endpoint, &COMMAND_SUCCEEDED.to_le_bytes()).await + self.send_acpi_response(respond_to_endpoint, None).await } } Err(e) => { error!("Error handling ACPI command: {:?}", e); - self.send_acpi_response(respond_to_endpoint, &COMMAND_FAILED.to_le_bytes()).await; + self.send_acpi_error(respond_to_endpoint).await; } } } @@ -464,9 +465,50 @@ impl Service { } } - async fn send_acpi_response(&self, destination: comms::EndpointID, response: &impl Any) { + async fn send_acpi_response(&self, destination: comms::EndpointID, data: Option<&[u8]>) { + let mut wire_message_len = core::mem::size_of::(); + { + const COMMAND_SUCCEEDED: u32 = 1; + let mut buffer_access = self.acpi_buf_owned_ref.borrow_mut(); + let buffer: &mut [u8] = buffer_access.borrow_mut(); + + buffer[..wire_message_len].copy_from_slice(&COMMAND_SUCCEEDED.to_le_bytes()); + + if let Some(data) = data { + if data.len() > (buffer.len() - wire_message_len) { + panic!("ACPI response payload too large"); + } + buffer[wire_message_len..(wire_message_len + data.len())].copy_from_slice(data); + wire_message_len += data.len(); + } + } + + let response = HostMsg::Response(AcpiMsgComms { + payload: acpi_buf::get(), + payload_len: wire_message_len + }); + + self.endpoint + .send(destination, &response) + .await + .expect("send returns Result<(), Infallible>"); + } + + async fn send_acpi_error(&self, destination: comms::EndpointID) { + let total_wire_message_len = core::mem::size_of::(); + { + const COMMAND_FAILED: u32 = 0; + let mut buffer_access = self.acpi_buf_owned_ref.borrow_mut(); + let buffer: &mut [u8] = buffer_access.borrow_mut(); + + buffer[..total_wire_message_len].copy_from_slice(&COMMAND_FAILED.to_le_bytes()); + } + let response = HostMsg::Response(AcpiMsgComms { + payload: acpi_buf::get(), + payload_len: total_wire_message_len + }); self.endpoint - .send(destination, response) + .send(destination, &response) .await .expect("send returns Result<(), Infallible>"); } From 7c72f72fa18ed442353bb465a167191305b8eec4 Mon Sep 17 00:00:00 2001 From: Billy Price Date: Tue, 14 Oct 2025 15:18:20 -0700 Subject: [PATCH 13/14] fix numbering of events, service ID --- .../src/ec_type/protocols/mctp.rs | 4 +-- time-alarm-service/src/lib.rs | 34 +++++++++---------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/embedded-service/src/ec_type/protocols/mctp.rs b/embedded-service/src/ec_type/protocols/mctp.rs index e672887e..90c03639 100644 --- a/embedded-service/src/ec_type/protocols/mctp.rs +++ b/embedded-service/src/ec_type/protocols/mctp.rs @@ -80,7 +80,7 @@ pub fn handle_mctp_header( 2 => crate::comms::EndpointID::Internal(crate::comms::Internal::Battery), 3 => crate::comms::EndpointID::Internal(crate::comms::Internal::Thermal), 4 => crate::comms::EndpointID::Internal(crate::comms::Internal::Debug), - 5 => crate::comms::EndpointID::Internal(crate::comms::Internal::TimeAlarm), // TODO should this be an enum and/or should we implement EndpointId::from_mctp_id() or something? + 0xB => crate::comms::EndpointID::Internal(crate::comms::Internal::TimeAlarm), // TODO should this be an enum and/or should we implement EndpointId::from_mctp_id() or something? Also note, we're going to eat a merge conflict here, others have been renumbered _ => return Err(MctpError::InvalidDestinationEndpoint), }; @@ -141,7 +141,7 @@ pub fn build_mctp_header( crate::comms::EndpointID::Internal(crate::comms::Internal::Battery) => ret[6] = 2, crate::comms::EndpointID::Internal(crate::comms::Internal::Thermal) => ret[6] = 3, crate::comms::EndpointID::Internal(crate::comms::Internal::Debug) => ret[6] = 4, - crate::comms::EndpointID::Internal(crate::comms::Internal::TimeAlarm) => ret[6] = 5, // TODO I don't love the duplication here - should we consolidate somewhere? + crate::comms::EndpointID::Internal(crate::comms::Internal::TimeAlarm) => ret[6] = 0xB, // TODO I don't love the duplication here - should we consolidate somewhere? _ => return Err(MctpError::InvalidDestinationEndpoint), } diff --git a/time-alarm-service/src/lib.rs b/time-alarm-service/src/lib.rs index 0e4204bd..d6dbc08f 100644 --- a/time-alarm-service/src/lib.rs +++ b/time-alarm-service/src/lib.rs @@ -160,14 +160,14 @@ impl Default for AlarmExpiredWakePolicy { enum AcpiTimeAlarmDeviceCommand { // Notably missing from the ACPI spec here is _GCP / 'Get Capabilities'. It just returns a constant and is expected to be implemented wholly in the ACPI ASL code. - GetRealTime, // 1: _GRT --> AcpiTimestamp, failure: valid bit = 0 in returned timestamp - SetRealTime(AcpiTimestamp), // 2: _SRT --> u32 (bool), failure: u32::MAX - GetWakeStatus(AcpiTimerId), // 3: _GWS --> u32 (bitmask), failure: infallible - ClearWakeStatus(AcpiTimerId), // 4: _CWS --> u32 (bool), failure: 1 - SetExpiredTimerPolicy(AcpiTimerId, AlarmExpiredWakePolicy), // 5: _STP --> u32 (bool), failure: 1 + GetRealTime, // 2: _GRT --> AcpiTimestamp, failure: valid bit = 0 in returned timestamp + SetRealTime(AcpiTimestamp), // 3: _SRT --> u32 (bool), failure: u32::MAX + GetWakeStatus(AcpiTimerId), // 4: _GWS --> u32 (bitmask), failure: infallible + ClearWakeStatus(AcpiTimerId), // 5: _CWS --> u32 (bool), failure: 1 SetTimerValue(AcpiTimerId, AlarmTimerSeconds), // 6: _STV --> u32 (bool), failure: 1, - GetExpiredTimerPolicy(AcpiTimerId), // 7: _TIP --> u32 (AlarmExpiredWakePolicy) failure: infallible - GetTimerValue(AcpiTimerId), // 8: _TIV --> u32 (AlarmTimerSeconds), failure: infallible, u32::MAX if disabled + GetTimerValue(AcpiTimerId), // 7: _TIV --> u32 (AlarmTimerSeconds), failure: infallible, u32::MAX if disabled + SetExpiredTimerPolicy(AcpiTimerId, AlarmExpiredWakePolicy), // 8: _STP --> u32 (bool), failure: 1 + GetExpiredTimerPolicy(AcpiTimerId), // 9: _TIP --> u32 (AlarmExpiredWakePolicy) failure: infallible RespondToInvalidCommand // Not an ACPI method. Used internally to indicate that an invalid command was received, and we must respond with an error asynchronously. } @@ -190,25 +190,25 @@ impl AcpiTimeAlarmDeviceCommand { .expect("Should never fail because if there were less than 4 bytes, parsing the command code would have failed. If there were exactly four bytes, this will return an empty slice, not None."); match command_code { - 1 => Ok(AcpiTimeAlarmDeviceCommand::GetRealTime), - 2 => Ok(AcpiTimeAlarmDeviceCommand::SetRealTime(AcpiTimestamp::try_from_bytes( + 2 => Ok(AcpiTimeAlarmDeviceCommand::GetRealTime), + 3 => Ok(AcpiTimeAlarmDeviceCommand::SetRealTime(AcpiTimestamp::try_from_bytes( bytes, )?)), _ => { let (timer_id, bytes) = AcpiTimerId::try_from_bytes(bytes)?; match command_code { - 3 => Ok(AcpiTimeAlarmDeviceCommand::GetWakeStatus(timer_id)), - 4 => Ok(AcpiTimeAlarmDeviceCommand::ClearWakeStatus(timer_id)), - 5 => Ok(AcpiTimeAlarmDeviceCommand::SetExpiredTimerPolicy( - timer_id, - AlarmExpiredWakePolicy(u32::from_le_bytes(bytes.try_into()?)), - )), + 4 => Ok(AcpiTimeAlarmDeviceCommand::GetWakeStatus(timer_id)), + 5 => Ok(AcpiTimeAlarmDeviceCommand::ClearWakeStatus(timer_id)), 6 => Ok(AcpiTimeAlarmDeviceCommand::SetTimerValue( timer_id, AlarmTimerSeconds(u32::from_le_bytes(bytes.try_into()?)), )), - 7 => Ok(AcpiTimeAlarmDeviceCommand::GetExpiredTimerPolicy(timer_id)), - 8 => Ok(AcpiTimeAlarmDeviceCommand::GetTimerValue(timer_id)), + 7 => Ok(AcpiTimeAlarmDeviceCommand::GetTimerValue(timer_id)), + 8 => Ok(AcpiTimeAlarmDeviceCommand::SetExpiredTimerPolicy( + timer_id, + AlarmExpiredWakePolicy(u32::from_le_bytes(bytes.try_into()?)), + )), + 9 => Ok(AcpiTimeAlarmDeviceCommand::GetExpiredTimerPolicy(timer_id)), _ => Err(TimeAlarmError::UnknownCommand), } } From 22f4f004793b35c01885d311f6cc966ab9f199eb Mon Sep 17 00:00:00 2001 From: Billy Price Date: Wed, 15 Oct 2025 16:59:43 -0700 Subject: [PATCH 14/14] Implement GCP --- time-alarm-service/src/lib.rs | 41 ++++++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/time-alarm-service/src/lib.rs b/time-alarm-service/src/lib.rs index d6dbc08f..f309bb86 100644 --- a/time-alarm-service/src/lib.rs +++ b/time-alarm-service/src/lib.rs @@ -158,8 +158,7 @@ impl Default for AlarmExpiredWakePolicy { #[rustfmt::skip] #[cfg_attr(feature = "defmt", derive(defmt::Format))] enum AcpiTimeAlarmDeviceCommand { - // Notably missing from the ACPI spec here is _GCP / 'Get Capabilities'. It just returns a constant and is expected to be implemented wholly in the ACPI ASL code. - + GetCapabilities, // 1: _GCP --> u32 (bitmask), failure: infallible GetRealTime, // 2: _GRT --> AcpiTimestamp, failure: valid bit = 0 in returned timestamp SetRealTime(AcpiTimestamp), // 3: _SRT --> u32 (bool), failure: u32::MAX GetWakeStatus(AcpiTimerId), // 4: _GWS --> u32 (bitmask), failure: infallible @@ -190,6 +189,7 @@ impl AcpiTimeAlarmDeviceCommand { .expect("Should never fail because if there were less than 4 bytes, parsing the command code would have failed. If there were exactly four bytes, this will return an empty slice, not None."); match command_code { + 1 => Ok(AcpiTimeAlarmDeviceCommand::GetCapabilities), 2 => Ok(AcpiTimeAlarmDeviceCommand::GetRealTime), 3 => Ok(AcpiTimeAlarmDeviceCommand::SetRealTime(AcpiTimestamp::try_from_bytes( bytes, @@ -296,6 +296,24 @@ bitfield!( timer_expired, set_timer_expired: 0; timer_triggered_wake, set_timer_triggered_wake: 1; ); +// ------------------------------------------------- + +bitfield!( + #[derive(Copy, Clone, Default, PartialEq, Eq)] + #[cfg_attr(feature = "defmt", derive(defmt::Format))] + pub struct TimeAlarmDeviceCapabilities(u32); + impl Debug; + bool; + ac_wake_implemented, set_ac_wake_implemented: 0; + dc_wake_implemented, set_dc_wake_implemented: 1; + realtime_implemented, set_realtime_implemented: 2; + realtime_accuracy_in_milliseconds, set_realtime_accuracy_in_milliseconds: 3; + get_wake_status_supported, set_get_wake_status_supported: 4; + ac_s4_wake_supported, set_ac_s4_wake_supported: 5; + ac_s5_wake_supported, set_ac_s5_wake_supported: 6; + dc_s4_wake_supported, set_dc_s4_wake_supported: 7; + dc_s5_wake_supported, set_dc_s5_wake_supported: 8; +); // ------------------------------------------------- @@ -340,7 +358,9 @@ pub struct Service { timers: Timers, - acpi_buf_owned_ref: OwnedRef<'static, u8> + acpi_buf_owned_ref: OwnedRef<'static, u8>, + + capabilities: TimeAlarmDeviceCapabilities, } // TODO [STORAGE] This statically allocates a buffer oustide the Service struct to hold ACPI responses. This is a common pattern in this repo, @@ -386,6 +406,20 @@ impl Service { dc_policy_storage, ), acpi_buf_owned_ref: acpi_buf::get_mut().unwrap(), + capabilities: { + // TODO [CONFIG] We could consider making some of these user-configurable, e.g. if we want to support devices that don't have a battery + let mut caps = TimeAlarmDeviceCapabilities(0); + caps.set_ac_wake_implemented(true); + caps.set_dc_wake_implemented(true); + caps.set_realtime_implemented(true); + caps.set_realtime_accuracy_in_milliseconds(false); + caps.set_get_wake_status_supported(true); + caps.set_ac_s4_wake_supported(true); + caps.set_ac_s5_wake_supported(true); + caps.set_dc_s4_wake_supported(true); + caps.set_dc_s5_wake_supported(true); + caps + }, }); // TODO [POWER_SOURCE] we need to subscribe to messages that tell us if we're on AC or DC power so we can decide which alarms to trigger - how do we do that? @@ -519,6 +553,7 @@ impl Service { ) -> Result { info!("Received Time-Alarm Device command: {:?}", command); match command { + AcpiTimeAlarmDeviceCommand::GetCapabilities => Ok(AcpiTimeAlarmCommandResult::U32(self.capabilities.0)), AcpiTimeAlarmDeviceCommand::GetRealTime => self.clock_state.lock(|clock_state| { let clock_state = clock_state.borrow(); let datetime = clock_state.datetime_clock.get_current_datetime()?;