vasilito/RedBear-OS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

P24: ACPI S5 derivation with shutdown semantics and error propagation

Browse Source 
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.
This commit is contained in:
Admin Pupkin
2026-05-19 02:43:58 +03:00
parent ba5e010bb7
commit 2a5eff93ec
3 changed files with 250 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
local/patches/base/P24-acpi-s5-derivation-shutdown-semantics.patch
+248
View File
@@ -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<AmlError> 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<Option<S5State>>,
+
@@ -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<S5State, ShutdownError> {
+ 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::<u8>::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::<u8>::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::<u8>::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::<u16>::new(pm1a_port);
+ let mut pio = Pio::<u16>::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::<u16>::new(pm1b_port);
+ if s5.pm1b_port != 0 {
+ let mut pio_b = Pio::<u16>::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);
recipes/core/base/P24-acpi-s5-derivation-shutdown-semantics.patch
Symlink
+1
View File
@@ -0,0 +1 @@
../../../local/patches/base/P24-acpi-s5-derivation-shutdown-semantics.patch
recipes/core/base/recipe.toml
+1
View File
@@ -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]