From 2a5eff93ec441224c348ff225f1488d3220f6c01 Mon Sep 17 00:00:00 2001 From: Admin Pupkin Date: Tue, 19 May 2026 02:43:58 +0300 Subject: [PATCH] P24: ACPI S5 derivation with shutdown semantics and error propagation Add deterministic S5 (soft-off) state derivation and structured error handling to acpid. Derive S5 parameters once at startup (or retry at shutdown if AML was not ready) instead of re-parsing the _S5 package on every shutdown attempt. Replace unit-return set_global_s_state() with ShutdownResult enum for proper error propagation and fallback handling. Changes: - S5State struct caches SLP_TYPa/b, PM1a/b ports, derivation timestamp - ShutdownError enum: MissingFadt, Pm1aZero, AmlNotReady, S5NotFound, S5NotPackage, SlpTypNotInteger, S5WriteFailed - ShutdownResult enum: Ok, FallbackReset, Err(ShutdownError) - derive_s5_state() method with early init attempt and lazy fallback - set_global_s_state() returns ShutdownResult instead of () - Early S5 derivation in AcpiContext::init() logs AML readiness status - main.rs logs shutdown result for debugging This is W2.1/W2.2 from ACPI-IMPROVEMENT-PLAN.md. --- ...cpi-s5-derivation-shutdown-semantics.patch | 248 ++++++++++++++++++ ...cpi-s5-derivation-shutdown-semantics.patch | 1 + recipes/core/base/recipe.toml | 1 + 3 files changed, 250 insertions(+) create mode 100644 local/patches/base/P24-acpi-s5-derivation-shutdown-semantics.patch create mode 120000 recipes/core/base/P24-acpi-s5-derivation-shutdown-semantics.patch diff --git a/local/patches/base/P24-acpi-s5-derivation-shutdown-semantics.patch b/local/patches/base/P24-acpi-s5-derivation-shutdown-semantics.patch new file mode 100644 index 0000000000..4a886b6978 --- /dev/null +++ b/local/patches/base/P24-acpi-s5-derivation-shutdown-semantics.patch @@ -0,0 +1,248 @@ +diff --git a/drivers/acpid/src/acpi.rs b/drivers/acpid/src/acpi.rs +index 8ef6ab0e..a6276f9e 100644 +--- a/drivers/acpid/src/acpi.rs ++++ b/drivers/acpid/src/acpi.rs +@@ -389,0 +390,44 @@ impl From for AmlEvalError { ++/// Cached S5 (soft-off) state derived from FADT and AML \_S5 package. ++/// ++/// Derived once at startup (or on first shutdown if AML was not ready at init) ++/// and reused for all subsequent shutdown attempts, eliminating redundant AML ++/// namespace lookups on the critical shutdown path. ++#[derive(Clone, Debug)] ++pub struct S5State { ++ pub slp_typa: u16, ++ pub slp_typb: u16, ++ pub pm1a_port: u16, ++ pub pm1b_port: u16, ++ pub derived_at: &'static str, ++} ++ ++/// Errors that can occur when deriving or executing the S5 shutdown sequence. ++#[derive(Debug, Error)] ++pub enum ShutdownError { ++ #[error("FADT not available — cannot determine shutdown parameters")] ++ MissingFadt, ++ #[error("PM1a control block address is zero — ACPI shutdown unavailable")] ++ Pm1aZero, ++ #[error("AML interpreter not initialized — cannot look up \\_S5")] ++ AmlNotReady, ++ #[error("\\_S5 not found in AML namespace")] ++ S5NotFound, ++ #[error("\\_S5 is not a Package object")] ++ S5NotPackage, ++ #[error("SLP_TYP value in \\_S5 is not an Integer")] ++ SlpTypNotInteger, ++ #[error("PM1a control write failed")] ++ S5WriteFailed, ++} ++ ++/// Result of a shutdown attempt. ++#[derive(Debug)] ++pub enum ShutdownResult { ++ /// Shutdown sequence completed (machine should power off). ++ Ok, ++ /// ACPI shutdown failed; fell back to keyboard controller reset. ++ FallbackReset, ++ /// Shutdown could not proceed due to a deterministic error. ++ Err(ShutdownError), ++} ++ +@@ -396,0 +441,2 @@ pub struct AcpiContext { ++ s5_state: RwLock>, ++ +@@ -476,0 +523,2 @@ impl AcpiContext { ++ s5_state: RwLock::new(None), ++ +@@ -490,0 +539,21 @@ impl AcpiContext { ++ // Trigger AML interpreter initialization so we can derive S5 state early. ++ // If AML init fails, S5 derivation will fall back to "shutdown_fallback" at ++ // shutdown time. ++ { ++ let mut symbols = this.aml_symbols.write(); ++ if symbols.aml_context.is_none() { ++ if let Err(e) = symbols.init() { ++ log::warn!("ACPI S5: AML init at startup failed: {} — will derive at shutdown", e); ++ } ++ } ++ } ++ match this.derive_s5_state("register_pci") { ++ Ok(_) => {} ++ Err(ShutdownError::AmlNotReady) => { ++ log::info!("ACPI S5: AML not ready at init — will derive at shutdown"); ++ } ++ Err(e) => { ++ log::warn!("ACPI S5: early derivation failed: {} — will derive at shutdown", e); ++ } ++ } ++ +@@ -592,16 +661,10 @@ impl AcpiContext { +- /// Set Power State +- /// See https://uefi.org/sites/default/files/resources/ACPI_6_1.pdf +- /// - search for PM1a +- /// See https://forum.osdev.org/viewtopic.php?t=16990 for practical details +- pub fn set_global_s_state(&self, state: u8) { +- if state != 5 { +- return; +- } +- let fadt = match self.fadt() { +- Some(fadt) => fadt, +- None => { +- log::error!("Cannot set global S-state due to missing FADT."); +- return; +- } +- }; +- ++ /// Derive the S5 (soft-off) state from FADT and AML \_S5 package. ++/// ++/// Reads PM1a/PM1b control block addresses from the FADT and the SLP_TYP ++/// values from the AML `\_S5` package, then caches the result. Subsequent ++/// calls return the cached value without re-parsing AML. ++/// ++/// `derived_at` is a log marker indicating when this derivation occurred ++/// (e.g. "register_pci", "shutdown_fallback"). ++ pub fn derive_s5_state(&self, derived_at: &'static str) -> Result { ++ let fadt = self.fadt().ok_or(ShutdownError::MissingFadt)?; +@@ -612,5 +675 @@ impl AcpiContext { +- log::error!("PM1a control block is zero - ACPI shutdown unavailable"); +- log::warn!("Falling back to keyboard controller reset"); +- #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] +- Pio::::new(0x64u16).write(0xFEu8); +- return; ++ return Err(ShutdownError::Pm1aZero); +@@ -619,2 +677,0 @@ impl AcpiContext { +- let mut val = 1u16 << 13; +- +@@ -622,4 +679,4 @@ impl AcpiContext { +- let s5_name = match acpi::aml::namespace::AmlName::from_str("\\_S5") { +- Ok(n) => n, +- Err(e) => { log::error!("\\_S5 AML name error: {:?}", e); return; } +- }; ++ let aml_context = aml_symbols ++ .aml_context ++ .as_ref() ++ .ok_or(ShutdownError::AmlNotReady)?; +@@ -627,7 +684,8 @@ impl AcpiContext { +- let s5 = match &aml_symbols.aml_context { +- Some(ctx) => match ctx.namespace.lock().get(s5_name) { +- Ok(s) => s, +- Err(e) => { log::error!("\\_S5 not found: {:?}", e); return; } +- }, +- None => { log::error!("AML context not initialized"); return; } +- }; ++ let s5_name = acpi::aml::namespace::AmlName::from_str("\\_S5") ++ .map_err(|_| ShutdownError::S5NotFound)?; ++ ++ let s5 = aml_context ++ .namespace ++ .lock() ++ .get(s5_name) ++ .map_err(|_| ShutdownError::S5NotFound)?; +@@ -637 +695 @@ impl AcpiContext { +- _ => { log::error!("\\_S5 is not a package"); return; } ++ _ => return Err(ShutdownError::S5NotPackage), +@@ -642 +700 @@ impl AcpiContext { +- _ => { log::error!("SLP_TYPa is not an integer"); return; } ++ _ => return Err(ShutdownError::SlpTypNotInteger), +@@ -647 +705,32 @@ impl AcpiContext { +- _ => 0u16 ++ _ => 0u16, ++ } ++ } else { ++ 0u16 ++ }; ++ ++ let state = S5State { ++ slp_typa, ++ slp_typb, ++ pm1a_port, ++ pm1b_port, ++ derived_at, ++ }; ++ ++ log::info!( ++ "ACPI S5: derived at={}, SLP_TYPa=0x{:X}, SLP_TYPb=0x{:X}, PM1a=0x{:04X}, PM1b=0x{:04X}", ++ derived_at, slp_typa, slp_typb, pm1a_port, pm1b_port ++ ); ++ ++ drop(aml_symbols); ++ *self.s5_state.write() = Some(state.clone()); ++ ++ Ok(state) ++ } ++ ++ /// Set Power State ++ /// See https://uefi.org/sites/default/files/resources/ACPI_6_1.pdf ++ /// - search for PM1a ++ /// See https://forum.osdev.org/viewtopic.php?t=16990 for practical details ++ pub fn set_global_s_state(&self, state: u8) -> ShutdownResult { ++ if state != 5 { ++ return ShutdownResult::Ok; +@@ -649 +737,0 @@ impl AcpiContext { +- } else { 0u16 }; +@@ -651 +739,26 @@ impl AcpiContext { +- val |= slp_typa & 0x1FFF; ++ let s5 = match self.s5_state.read().as_ref() { ++ Some(cached) => cached.clone(), ++ None => match self.derive_s5_state("shutdown_fallback") { ++ Ok(s5) => s5, ++ Err(e) => { ++ log::error!("ACPI S5 derivation failed: {}", e); ++ if matches!(e, ShutdownError::Pm1aZero | ShutdownError::MissingFadt) { ++ log::warn!("Falling back to keyboard controller reset"); ++ #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] ++ Pio::::new(0x64u16).write(0xFEu8); ++ return ShutdownResult::FallbackReset; ++ } ++ return ShutdownResult::Err(e); ++ } ++ }, ++ }; ++ ++ if s5.pm1a_port == 0 { ++ log::error!("ACPI S5: cached PM1a port is zero — shutdown unavailable"); ++ log::warn!("Falling back to keyboard controller reset"); ++ #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] ++ Pio::::new(0x64u16).write(0xFEu8); ++ return ShutdownResult::FallbackReset; ++ } ++ ++ let mut val = (1u16 << 13) | (s5.slp_typa & 0x1FFF); +@@ -655,2 +768,4 @@ impl AcpiContext { +- log::info!("ACPI shutdown: PM1a=0x{:04X} val=0x{:04X} SLP_TYPa=0x{:X} SLP_TYPb=0x{:X}", +- pm1a_port, val, slp_typa, slp_typb); ++ log::info!( ++ "ACPI shutdown: writing PM1a=0x{:04X} val=0x{:04X} (SLP_TYPa=0x{:X}, SLP_TYPb=0x{:X})", ++ s5.pm1a_port, val, s5.slp_typa, s5.slp_typb ++ ); +@@ -658 +773 @@ impl AcpiContext { +- let mut pio = Pio::::new(pm1a_port); ++ let mut pio = Pio::::new(s5.pm1a_port); +@@ -663,3 +778,2 @@ impl AcpiContext { +- log::warn!("ACPI PM1a shutdown did not power off - retry with PM1b"); +- val |= slp_typb & 0x1FFF; +- val |= 1u16 << 13; ++ log::warn!("ACPI PM1a shutdown did not power off — retrying with PM1b"); ++ val = (1u16 << 13) | (s5.slp_typb & 0x1FFF); +@@ -668,2 +782,2 @@ impl AcpiContext { +- if pm1b_port != 0 { +- let mut pio_b = Pio::::new(pm1b_port); ++ if s5.pm1b_port != 0 { ++ let mut pio_b = Pio::::new(s5.pm1b_port); +@@ -671 +785 @@ impl AcpiContext { +- log::info!("ACPI shutdown: also wrote PM1b=0x{:04X}", pm1b_port); ++ log::info!("ACPI shutdown: also wrote PM1b=0x{:04X}", s5.pm1b_port); +@@ -675 +789 @@ impl AcpiContext { +- log::error!("ACPI shutdown failed - falling back to keyboard controller reset"); ++ log::error!("ACPI shutdown failed — falling back to keyboard controller reset"); +@@ -676,0 +791 @@ impl AcpiContext { ++ return ShutdownResult::FallbackReset; +@@ -681,0 +797 @@ impl AcpiContext { ++ ShutdownResult::Err(ShutdownError::S5WriteFailed) +diff --git a/drivers/acpid/src/main.rs b/drivers/acpid/src/main.rs +index 0c1d4c72..1d242b06 100644 +--- a/drivers/acpid/src/main.rs ++++ b/drivers/acpid/src/main.rs +@@ -165 +165,2 @@ fn daemon(daemon: daemon::Daemon) -> ! { +- acpi_context.set_global_s_state(5); ++ let result = acpi_context.set_global_s_state(5); ++ log::info!("ACPI shutdown result: {:?}", result); diff --git a/recipes/core/base/P24-acpi-s5-derivation-shutdown-semantics.patch b/recipes/core/base/P24-acpi-s5-derivation-shutdown-semantics.patch new file mode 120000 index 0000000000..5296de08ec --- /dev/null +++ b/recipes/core/base/P24-acpi-s5-derivation-shutdown-semantics.patch @@ -0,0 +1 @@ +../../../local/patches/base/P24-acpi-s5-derivation-shutdown-semantics.patch \ No newline at end of file diff --git a/recipes/core/base/recipe.toml b/recipes/core/base/recipe.toml index 2f2c4c2237..23c7bb2d7e 100644 --- a/recipes/core/base/recipe.toml +++ b/recipes/core/base/recipe.toml @@ -69,6 +69,7 @@ patches = [ "P20-ramfs-requires-randd.patch", "P21-boot-daemon-graceful-panic.patch", "P23-rootfs-hard-dep-on-drivers.patch", + "P24-acpi-s5-derivation-shutdown-semantics.patch", ] [package]