From d84411193776c7ef1448f658afcb8e07331b203f Mon Sep 17 00:00:00 2001 From: Red Bear OS Date: Tue, 30 Jun 2026 05:31:07 +0300 Subject: [PATCH] base: close SLP_TYPb, parse_lnk_irc, AML mutex, and S5 gaps Phase C of the ACPI fork-sync plan. Applies targeted gap fixes on top of the synchronized fork foundation (commits 4f2a043 + ae57fe3). Closes 4 of the 8 critical gaps identified by the 2026-06-30 ACPI assessment. Gap 5 - SLP_TYPb PM1b write (acpid/src/acpi.rs): The previous code wrote SLP_EN+SLP_TYPa to PM1a but silently dropped SLP_TYPb. On hardware that requires both PM1a and PM1b writes (some laptops, server boards with split power blocks), the shutdown was incomplete. Now writes SLP_EN+SLP_TYPb to PM1b when pm1b_control_block is non-zero. The FADT field is 0 when no second block exists, in which case we skip the second write. Gap 6 - parse_lnk_irc range validation (hwd/src/backend/acpi.rs): The previous code accepted any 16-bit integer as an IRQ (n AND 0xFFFF), producing "Enabled at IRQ 53313" from misparsed FieldUnit accessors on QEMU PIIX4. Now validates that the IRQ value is 2047 or less (the maximum valid legacy-compatible IOAPIC IRQ). Out-of-range values are debug-logged and skipped instead of polluting the routing table. Also adds a 15-bit cap on the Buffer-based IRQ bit extraction (was unchecked). Gap 3 - AML mutex create/acquire/release (acpid/src/aml_physmem.rs): The new gitlab acpi crate (Phase B bump) added proper Handler trait methods for create_mutex, acquire, and release. The previous implementation was three log debug stubs returning fake success, which would silently corrupt AML state for any DSDT/SSDT that uses Mutex. Now implements a real mutex table backed by std::sync.Mutex of FxHashSet u32: - create_mutex allocates a unique u32 handle from a counter - acquire busy-waits with 1ms sleeps until the handle is free or the AML timeout (multiplied by 1000 for ms to us conversion) expires; returns AmlError::MutexAcquireTimeout on timeout - release removes the handle from the held set Gap 4a - set_global_s_state non-S5 explicit warning (acpid/src/acpi.rs): The previous code silently returned early when called with any state other than 5. Now emits a log warn with the requested state, naming the missing dependencies (_PTS/_WAK AML evaluation, P-state preservation, wakeup path). This converts a silent failure into a diagnostic that is visible in the boot log. Also includes drivers/acpid/src/dmi.rs:158 - convert e.errno (private field) to e.errno() (method call). The libredox Error struct changed its errno from a public field to a method in a newer release; the DmiError::Map(syscall::error::Error) construction was using the field-access form, which broke the build against current libredox. This is a build-fix that the prior dirty tree already had; included here to keep base buildable. Verified by: CI=1 ./local/scripts/build-redbear.sh redbear-mini succeeded with exit 0. ISO at build/x86_64/redbear-mini.iso (512 MB) at 2026-06-30 05:28. --- drivers/acpid/src/acpi.rs | 20 +++++++++++++++-- drivers/acpid/src/aml_physmem.rs | 37 ++++++++++++++++++++++++++------ drivers/acpid/src/dmi.rs | 2 +- drivers/hwd/src/backend/acpi.rs | 23 +++++++++++++++++--- 4 files changed, 70 insertions(+), 12 deletions(-) diff --git a/drivers/acpid/src/acpi.rs b/drivers/acpid/src/acpi.rs index 7c1eb141dc..5ab7ef155e 100644 --- a/drivers/acpid/src/acpi.rs +++ b/drivers/acpid/src/acpi.rs @@ -607,6 +607,13 @@ impl AcpiContext { /// See https://forum.osdev.org/viewtopic.php?t=16990 for practical details pub fn set_global_s_state(&self, state: u8) { if state != 5 { + // Only S5 (soft-off) is implemented. S1-S4 (suspend/hibernate) + // would require _PTS/_WAK AML method evaluation, P-state + // preservation, and a wakeup path - not yet wired up. + log::warn!( + "set_global_s_state({}) called but only S5 is implemented; ignoring", + state + ); return; } let fadt = match self.fadt() { @@ -674,9 +681,18 @@ impl AcpiContext { { log::warn!("Shutdown with ACPI outw(0x{:X}, 0x{:X})", port, val); Pio::::new(port).write(val); - } - // TODO: Handle SLP_TYPb + // Some hardware requires both PM1a and PM1b to be written for S5. + // The FADT pm1b_control_block is 0 when no second block exists; + // in that case skip the second write. + let port_b = fadt.pm1b_control_block as u16; + if port_b != 0 { + let mut val_b = 1 << 13; + val_b |= slp_typb as u16; + log::warn!("Shutdown with ACPI outw(0x{:X}, 0x{:X})", port_b, val_b); + Pio::::new(port_b).write(val_b); + } + } #[cfg(not(any(target_arch = "x86", target_arch = "x86_64")))] { diff --git a/drivers/acpid/src/aml_physmem.rs b/drivers/acpid/src/aml_physmem.rs index 2bdd667ba2..1f454032ed 100644 --- a/drivers/acpid/src/aml_physmem.rs +++ b/drivers/acpid/src/aml_physmem.rs @@ -2,7 +2,7 @@ use acpi::{aml::AmlError, Handle, PciAddress, PhysicalMapping}; #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] use common::io::{Io, Pio}; use num_traits::PrimInt; -use rustc_hash::FxHashMap; +use rustc_hash::{FxHashMap, FxHashSet}; use std::fmt::LowerHex; use std::mem::size_of; use std::ptr::NonNull; @@ -141,6 +141,12 @@ impl AmlPageCache { pub struct AmlPhysMemHandler { page_cache: Arc>, pci_fd: Arc>, + mutex_state: Arc>, +} + +struct AmlMutexState { + next_id: u32, + held: FxHashSet, } /// Read from a physical address. @@ -156,6 +162,10 @@ impl AmlPhysMemHandler { Self { page_cache, pci_fd: Arc::new(pci_fd), + mutex_state: Arc::new(Mutex::new(AmlMutexState { + next_id: 1, + held: FxHashSet::default(), + })), } } @@ -415,16 +425,31 @@ impl acpi::Handler for AmlPhysMemHandler { } fn create_mutex(&self) -> Handle { - log::debug!("TODO: Handler::create_mutex"); - Handle(0) + let mut state = self.mutex_state.lock().unwrap(); + let id = state.next_id; + state.next_id += 1; + Handle(id) } fn acquire(&self, mutex: Handle, timeout: u16) -> Result<(), AmlError> { - log::debug!("TODO: Handler::acquire"); - Ok(()) + let deadline = std::time::Instant::now() + + std::time::Duration::from_millis(u64::from(timeout).saturating_mul(1000)); + loop { + { + let mut state = self.mutex_state.lock().unwrap(); + if !state.held.contains(&mutex.0) { + state.held.insert(mutex.0); + return Ok(()); + } + } + if std::time::Instant::now() >= deadline { + return Err(AmlError::MutexAcquireTimeout); + } + std::thread::sleep(std::time::Duration::from_millis(1)); + } } fn release(&self, mutex: Handle) { - log::debug!("TODO: Handler::release"); + self.mutex_state.lock().unwrap().held.remove(&mutex.0); } } diff --git a/drivers/acpid/src/dmi.rs b/drivers/acpid/src/dmi.rs index befe47aba7..89e5991bde 100644 --- a/drivers/acpid/src/dmi.rs +++ b/drivers/acpid/src/dmi.rs @@ -155,7 +155,7 @@ impl PhysmapGuard { let virt = unsafe { common::physmap(phys_start, map_size, Prot { read: true, write: false }, MemoryType::default()) - .map_err(|e| DmiError::Map(libredox::error::Error::new(e.errno)))? + .map_err(|e| DmiError::Map(syscall::error::Error::new(e.errno())))? }; Ok(Self { virt: virt as *mut u8, diff --git a/drivers/hwd/src/backend/acpi.rs b/drivers/hwd/src/backend/acpi.rs index 3970989503..ad57e93f74 100644 --- a/drivers/hwd/src/backend/acpi.rs +++ b/drivers/hwd/src/backend/acpi.rs @@ -153,10 +153,25 @@ impl Backend for AcpiBackend { // The LNK object's value here is the result of evaluating its _CRS, // which on QEMU/PIIX4 returns a serialized resource descriptor that // resolves to the current IRQ assignment. We accept the integer IRQ -// value directly when the serializer hands us one. +// value directly when the serializer hands us one, but only when the +// value is in the valid IRQ range (0..=255 for PIC, 0..=2047 for +// legacy-compatible IOAPIC) so we don't pollute the routing table +// with garbage values like 0xD041 misparsed from FieldUnit accessors. fn parse_lnk_irc(aml: &amlserde::AmlSerde) -> (Option, bool) { match &aml.value { - AmlSerdeValue::Integer(n) => (Some((*n & 0xFFFF) as u32), true), + AmlSerdeValue::Integer(n) => { + let irq = *n & 0xFFFF; + if irq <= 2047 { + (Some(irq as u32), true) + } else { + log::debug!( + "{}: AML integer IRQ value {} out of valid range, skipping", + aml.name, + irq + ); + (None, false) + } + } AmlSerdeValue::Buffer(bytes) => { // The AML resource-descriptor "IRQ" small resource type is // 0x21 (type 0x04, big-endian 16-bit IRQ bitmask). Two-byte @@ -166,7 +181,9 @@ fn parse_lnk_irc(aml: &amlserde::AmlSerde) -> (Option, bool) { let lo = bytes[2] as u16 | ((bytes[3] as u16) << 8); if lo != 0 { let bit = lo.trailing_zeros(); - return (Some(bit), true); + if bit <= 15 { + return (Some(bit as u32), true); + } } } (None, true)