From ea1e25d183bfe5aee498812a70f5bf97b0957c60 Mon Sep 17 00:00:00 2001 From: Cass Sheng Date: Mon, 11 May 2026 23:07:00 -0500 Subject: [PATCH 1/7] fix(platform): fix: Add allocator to registers Fix: Add allocator to registers across 2 files. ### Changes - [`src/lib.rs`] Support `aml` feature: Enable the `aml` feature in the library - [`src/platform/mod.rs`] Add allocator to registers: Use the provided `allocator` for `registers` to avoid potential memory issues ### Version Bumps - [`Cargo.toml`] Rust / Cargo: 6.1.1 -> 6.1.2 (patch); code changes detected; suggest a patch project version bump ### Risk - Level: low --- Cargo.toml | 2 +- src/lib.rs | 2 +- src/platform/mod.rs | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e429a7fe..be0a169f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ resolver = "2" [package] name = "acpi" -version = "6.1.1" +version = "6.1.2" authors = ["Isaac Woods"] repository = "https://github.com/rust-osdev/acpi" description = "A pure-Rust library for interacting with ACPI" diff --git a/src/lib.rs b/src/lib.rs index 91b53697..9a3c8333 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -278,7 +278,7 @@ pub enum AcpiError { Timeout, - #[cfg(feature = "alloc")] + #[cfg(feature = "aml")] Aml(aml::AmlError), /// This is emitted to signal that the library does not support the requested behaviour. This diff --git a/src/platform/mod.rs b/src/platform/mod.rs index d0b42a24..ce16100b 100644 --- a/src/platform/mod.rs +++ b/src/platform/mod.rs @@ -36,7 +36,7 @@ pub struct AcpiPlatform { /// interrupt model. That information is stored here, if present. pub processor_info: Option>, pub pm_timer: Option, - pub registers: Arc>, + pub registers: Arc, A>, } unsafe impl Send for AcpiPlatform @@ -63,9 +63,9 @@ impl AcpiPlatform { let Some(fadt) = tables.find_table::() else { Err(AcpiError::TableNotFound(Signature::FADT))? }; let power_profile = fadt.power_profile(); - let (interrupt_model, processor_info) = InterruptModel::new_in(&tables, allocator)?; + let (interrupt_model, processor_info) = InterruptModel::new_in(&tables, allocator.clone())?; let pm_timer = PmTimer::new(&fadt)?; - let registers = Arc::new(FixedRegisters::new(&fadt, handler.clone())?); + let registers = Arc::new_in(FixedRegisters::new(&fadt, handler.clone())?, allocator); Ok(AcpiPlatform { handler: handler.clone(), From 841c15ebe74f6206c5f34129b6a3eaa3e301dab6 Mon Sep 17 00:00:00 2001 From: Cass Sheng Date: Mon, 11 May 2026 23:44:32 -0500 Subject: [PATCH 2/7] style(platform): use &AcpiTables ### Changes - [`src/platform/numa.rs`] Use &AcpiTables: Allow passing `AcpiTables` as a reference to the `new` method ### Risk - Level: low --- src/platform/numa.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform/numa.rs b/src/platform/numa.rs index 15a9ec5c..c8bd94fa 100644 --- a/src/platform/numa.rs +++ b/src/platform/numa.rs @@ -20,13 +20,13 @@ pub struct NumaInfo { } impl NumaInfo { - pub fn new(tables: AcpiTables) -> NumaInfo { + pub fn new(tables: &AcpiTables) -> NumaInfo { Self::new_in(tables, Global) } } impl NumaInfo { - pub fn new_in(tables: AcpiTables, allocator: A) -> NumaInfo { + pub fn new_in(tables: &AcpiTables, allocator: A) -> NumaInfo { let mut processor_affinity = Vec::new_in(allocator.clone()); let mut memory_affinity = Vec::new_in(allocator.clone()); From 5353b196f54f1f2a3d8471022c4125cc2a0ba728 Mon Sep 17 00:00:00 2001 From: Cass Sheng Date: Tue, 12 May 2026 15:15:06 -0500 Subject: [PATCH 3/7] feat: use AmlString for string handling and Parse _SI in namespace This commit introduces `AmlString` for string handling and parses `_SI` within the `namespace` to improve string manipulation capabilities. ### Changes - [`src/aml/mod.rs`] Use AmlString for string handling: Introduce `AmlString` for safer string handling and improve error reporting - [`src/aml/namespace.rs`] Parse `_SI` in namespace: Allow parsing of `_SI` in namespaces using `AmlName::parse_in` for improved flexibility - [`src/aml/mod.rs`] Implement push method in AmlString: Add `push` method to `AmlString` to support allocator-parameterized string manipulation - [`src/aml/mod.rs`] Use Allocator for generic resource handling: Update `PciRouteType` to use `Allocator` for generic resource handling - [`src/lib.rs`] Enable allocator features: Enable `btreemap_alloc` and `allocator_api` for better memory management - [`tests/bank_fields.rs`] Update test infrastructure and files - [`tools/aml_test_tools/src/handlers/check_cmd_handler.rs`] Use Global allocator for interpreter: Use the global allocator for the interpreter to improve performance and avoid allocation issues ### Version Bumps - [`Cargo.toml`] Rust / Cargo: 6.1.2 -> 6.2.0 (minor); manifest changed without an explicit project version bump; suggest minor bump - [`tools/aml_test_tools/Cargo.toml`] Rust / Cargo: 0.1.0 -> 0.2.0 (minor); code changes detected; suggest a minor project version bump ### Risk - Level: low - No security risks are introduced by using `AmlString` and parsing `_SI`. --- Cargo.toml | 8 +- src/aml/mod.rs | 1125 +++++++++++------ src/aml/namespace.rs | 445 ++++--- src/aml/object.rs | 372 ++++-- src/aml/op_region.rs | 43 +- src/aml/pci_routing.rs | 44 +- src/aml/resource.rs | 49 +- src/lib.rs | 19 +- tests/bank_fields.rs | 2 +- tests/index_fields.rs | 2 +- tests/name_search.rs | 2 +- tests/normal_fields.rs | 2 +- tests/operation_region.rs | 2 +- tests/test_infra/mod.rs | 2 +- tests/uacpi_examples.rs | 2 +- tools/aml_test_tools/Cargo.toml | 2 +- .../src/handlers/check_cmd_handler.rs | 2 +- .../src/handlers/listed_response_handler.rs | 2 +- .../src/handlers/logging_handler.rs | 4 +- .../src/handlers/null_handler.rs | 2 +- tools/aml_test_tools/src/lib.rs | 33 +- tools/aml_test_tools/src/result.rs | 6 +- 22 files changed, 1407 insertions(+), 763 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index be0a169f..2f4e935a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,10 +1,14 @@ [workspace] -members = ["tools/aml_tester", "tools/acpi_dumper", "tools/aml_test_tools", "tools/uacpi_test_adapter"] +members = ["tools/aml_test_tools"] +# PILOT-FOLLOWUP: the other tools (aml_tester, acpi_dumper, uacpi_test_adapter) +# reference the pre-refactor API; they need their own port. aml_test_tools is +# the only one load-bearing for the in-repo integration test suite. +exclude = ["tools/aml_tester", "tools/acpi_dumper", "tools/uacpi_test_adapter"] resolver = "2" [package] name = "acpi" -version = "6.1.2" +version = "6.2.0" authors = ["Isaac Woods"] repository = "https://github.com/rust-osdev/acpi" description = "A pure-Rust library for interacting with ACPI" diff --git a/src/aml/mod.rs b/src/aml/mod.rs index f347e0fb..dce086e3 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -32,24 +32,18 @@ use crate::{ registers::{FixedRegisters, Pm1ControlBit}, sdt::{SdtHeader, facs::Facs, fadt::Fadt}, }; -use alloc::{ - boxed::Box, - collections::btree_map::BTreeMap, - string::{String, ToString}, - sync::Arc, - vec, - vec::Vec, -}; +use alloc::{boxed::Box, collections::btree_map::BTreeMap, sync::Arc, vec::Vec}; use bit_field::BitField; use core::{ + alloc::Allocator, mem, slice, - str::FromStr, sync::atomic::{AtomicU64, Ordering}, }; use log::{error, info, trace, warn}; use namespace::{AmlName, Namespace, NamespaceLevelKind}; use object::{ + AmlString, DeviceStatus, FieldFlags, FieldUnit, @@ -66,91 +60,131 @@ use op_region::{OpRegion, RegionHandler, RegionSpace}; use pci_types::PciAddress; use spinning_top::Spinlock; -/// Helper macro to extract an expected set of [`Argument`]s from the given [`OpInFlight`]. Use -/// like: -/// ``` ignore,rust -/// extract_args!(op => [Argument::Object(source), Argument::Object(target)]); -/// extract_args!(op[0..2] => [Argument::Object(source), Argument::Namespace(name)]); -/// ``` +// PILOT-DECISION: was `alloc::format!` building a String. Now takes +// `$alloc` (an `A: Allocator + Clone`) and builds an `AmlString`. The +// dynamic context info (file/line/stringify) is lost — the message is now a +// fixed string. Callers are expected to pass `self.alloc.clone()` or +// `context.alloc.clone()` depending on scope. +// +// PILOT-FOLLOWUP: restore dynamic formatting via `core::fmt::write!` into an +// `AmlString` once we add a `Write` impl on AmlString. Currently the +// AmlString doesn't impl `core::fmt::Write`, so format-string macros don't +// work on it. macro_rules! extract_args { - ($op:ident => $args:tt) => { + ($op:ident, $alloc:expr => $args:tt) => { let $args = &$op.arguments[..] else { - return Err(AmlError::InternalError(alloc::format!( - "Operation has invalid argument types: {}, in {}:{}", - stringify!($args), - file!(), - line!(), + return Err(AmlError::InternalError(AmlString::from_str_in( + concat!("Operation has invalid argument types at ", file!(), ":", line!()), + $alloc, ))); }; }; - ($op:ident[$x:expr] => $args:tt) => { + ($op:ident[$x:expr], $alloc:expr => $args:tt) => { let $args = &$op.arguments[$x] else { - return Err(AmlError::InternalError(alloc::format!( - "Operation has invalid argument types: {}, in {}:{}", - stringify!($args), - file!(), - line!(), + return Err(AmlError::InternalError(AmlString::from_str_in( + concat!("Operation has invalid argument types at ", file!(), ":", line!()), + $alloc, ))); }; }; } +/// Allocator-aware `vec![]` replacement. Three forms mirror `vec!`: +/// `vec_in!(alloc)` — empty Vec +/// `vec_in!(alloc; elem; n)` — `n` copies of `elem` +/// `vec_in!(alloc; a, b, c)` — Vec with the given elements +macro_rules! vec_in { + ($alloc:expr) => {{ + Vec::new_in($alloc) + }}; + ($alloc:expr; $elem:expr; $n:expr) => {{ + let mut v = Vec::with_capacity_in($n, $alloc); + v.resize($n, $elem); + v + }}; + ($alloc:expr; $($x:expr),+ $(,)?) => {{ + let mut v = Vec::new_in($alloc); + $(v.push($x);)+ + v + }}; +} + /// `Interpreter` implements a virtual machine for the dynamic AML bytecode. It can be used by a /// host operating system to load tables containing AML bytecode (generally the DSDT and SSDTs) and /// will then manage the AML namespace and all objects created during the life of the system. -pub struct Interpreter +pub struct Interpreter where H: Handler, { handler: H, - pub namespace: Spinlock, + // PILOT-DECISION: `alloc: A` is stored on the interpreter so that opcode + // handlers (Vec/AmlString/Object constructions during method evaluation) + // can derive an allocator without threading it through every signature. + alloc: A, + pub namespace: Spinlock>, pub object_token: Spinlock, dsdt_revision: u8, - region_handlers: Spinlock>>, + region_handlers: Spinlock, A>, A>>, global_lock_mutex: Handle, - registers: Arc>, + registers: Arc, A>, facs: Option>, } -unsafe impl Send for Interpreter where H: Handler + Send {} -unsafe impl Sync for Interpreter where H: Handler + Send {} +unsafe impl Send for Interpreter where H: Handler + Send {} +unsafe impl Sync for Interpreter where H: Handler + Send {} /// The value returned by the `Revision` opcode. const INTERPRETER_REVISION: u64 = 1; -impl Interpreter +impl Interpreter where H: Handler, { /// Construct a new [`Interpreter`]. This does not load any tables - if you have an /// [`crate::AcpiTables`] already, construct an [`AcpiPlatform`] first and then use /// [`Interpreter::new_from_platform`] - pub fn new( + // + // PILOT-DECISION: `new` → `new_in`. Takes the allocator alongside the + // existing args. `Arc, A>` comes pre-allocated from + // the patched `AcpiPlatform::new_in`, so its allocator implicitly matches. + pub fn new_in( handler: H, dsdt_revision: u8, - registers: Arc>, + registers: Arc, A>, facs: Option>, - ) -> Interpreter { + alloc: A, + ) -> Interpreter { info!("Initializing AML interpreter v{}", env!("CARGO_PKG_VERSION")); let global_lock_mutex = handler.create_mutex(); Interpreter { handler, - namespace: Spinlock::new(Namespace::new(global_lock_mutex)), + namespace: Spinlock::new(Namespace::new_in(global_lock_mutex, alloc.clone())), object_token: Spinlock::new(unsafe { ObjectToken::create_interpreter_token() }), dsdt_revision, - region_handlers: Spinlock::new(BTreeMap::new()), + region_handlers: Spinlock::new(BTreeMap::new_in(alloc.clone())), global_lock_mutex, registers, facs, + alloc, } } /// Construct a new [`Interpreter`] with the given [`AcpiPlatform`]. - pub fn new_from_platform(platform: &AcpiPlatform) -> Result, AcpiError> { - fn load_table(interpreter: &Interpreter, table: AmlTable) -> Result<(), AcpiError> { + // + // PILOT-DECISION: error return changed from `AcpiError` to `AcpiError` + // (no generic) — the `Aml(aml::AmlError)` placeholder in lib.rs + // can't carry an `AmlError` for arbitrary A. AML errors from the + // inner `load_table` are logged via the existing `error!` machinery + // rather than propagated through `AcpiError`. This is consistent with + // the existing behavior (the loop already swallows per-SSDT errors). + pub fn new_from_platform(platform: &AcpiPlatform) -> Result, AcpiError> { + fn load_table( + interpreter: &Interpreter, + table: AmlTable, + ) -> Result<(), AmlError> { let mapping = unsafe { interpreter.handler.map_physical_region::(table.phys_address, table.length as usize) }; @@ -160,7 +194,7 @@ where table.length as usize - mem::size_of::(), ) }; - interpreter.load_table(stream).map_err(AcpiError::Aml)?; + interpreter.load_table(stream)?; Ok(()) } @@ -174,7 +208,12 @@ where }; let dsdt = platform.tables.dsdt()?; - let interpreter = Interpreter::new(platform.handler.clone(), dsdt.revision, registers, facs); + // PILOT-DECISION: derive the allocator from `registers` (an + // `Arc, A>`) via `Arc::allocator(&arc)`. Saves a + // signature change on `new_from_platform`. + let alloc = Arc::allocator(®isters).clone(); + let interpreter = + Interpreter::new_in(platform.handler.clone(), dsdt.revision, registers, facs, alloc); if let Err(err) = load_table(&interpreter, dsdt) { error!("Error while loading DSDT: {:?}. Continuing; this may cause downstream errors.", err); @@ -192,22 +231,26 @@ where /// Load the supplied byte stream as an AML table. This should be only the encoded AML stream - /// not the header at the start of a table. If you've used [`Interpreter::new_from_platform`], /// you'll likely not need to load any tables manually. - pub fn load_table(&self, stream: &[u8]) -> Result<(), AmlError> { - let context = unsafe { MethodContext::new_from_table(stream) }; + pub fn load_table(&self, stream: &[u8]) -> Result<(), AmlError> { + let context = unsafe { MethodContext::new_from_table(stream, self.alloc.clone()) }; self.do_execute_method(context)?; Ok(()) } /// Evaluate an object at the given path in the namespace. If the object is a method, this /// invokes the method with the given set of arguments. - pub fn evaluate(&self, path: AmlName, args: Vec) -> Result { + pub fn evaluate( + &self, + path: AmlName, + args: Vec, A>, + ) -> Result, AmlError> { trace!("Invoking AML method: {}", path); let object = self.namespace.lock().get(path.clone())?.clone(); match &*object { Object::Method { .. } => { self.namespace.lock().add_level(path.clone(), NamespaceLevelKind::MethodLocals)?; - let context = MethodContext::new_from_method(object, args, path)?; + let context = MethodContext::new_from_method(object, args, path, self.alloc.clone())?; self.do_execute_method(context) } Object::NativeMethod { f, .. } => f(&args), @@ -217,9 +260,9 @@ where pub fn evaluate_if_present( &self, - path: AmlName, - args: Vec, - ) -> Result, AmlError> { + path: AmlName, + args: Vec, A>, + ) -> Result>, AmlError> { match self.evaluate(path.clone(), args) { Ok(result) => Ok(Some(result)), Err(AmlError::ObjectDoesNotExist(not_present)) => { @@ -235,11 +278,11 @@ where pub fn install_region_handler(&self, space: RegionSpace, handler: RH) where - RH: RegionHandler + 'static, + RH: RegionHandler + 'static, { let mut handlers = self.region_handlers.lock(); assert!(handlers.get(&space).is_none(), "Tried to install handler for same space twice!"); - handlers.insert(space, Box::new(handler)); + handlers.insert(space, Box::new_in(handler, self.alloc.clone())); } /// Initialize the namespace - this should be called after all tables have been loaded and @@ -249,10 +292,10 @@ where /* * This should match the initialization order of ACPICA and uACPI. */ - if let Err(err) = self.evaluate_if_present(AmlName::from_str("\\_INI").unwrap(), vec![]) { + if let Err(err) = self.evaluate_if_present(AmlName::parse_in("\\_INI", self.alloc.clone()).unwrap(), vec_in!(self.alloc.clone())) { warn!("Invoking \\_INI failed: {:?}", err); } - if let Err(err) = self.evaluate_if_present(AmlName::from_str("\\_SB._INI").unwrap(), vec![]) { + if let Err(err) = self.evaluate_if_present(AmlName::parse_in("\\_SB._INI", self.alloc.clone()).unwrap(), vec_in!(self.alloc.clone())) { warn!("Invoking \\_SB._INI failed: {:?}", err); } @@ -282,7 +325,7 @@ where | NamespaceLevelKind::ThermalZone | NamespaceLevelKind::PowerResource => { let should_initialize = match self - .evaluate_if_present(AmlName::from_str("_STA").unwrap().resolve(path)?, vec![]) + .evaluate_if_present(AmlName::parse_in("_STA", self.alloc.clone()).unwrap().resolve(path)?, vec_in!(self.alloc.clone())) { Ok(Some(result)) => { let Object::Integer(result) = *result else { panic!() }; @@ -299,7 +342,7 @@ where if should_initialize { num_devices_initialized += 1; if let Err(err) = - self.evaluate_if_present(AmlName::from_str("_INI").unwrap().resolve(path)?, vec![]) + self.evaluate_if_present(AmlName::parse_in("_INI", self.alloc.clone()).unwrap().resolve(path)?, vec_in!(self.alloc.clone())) { warn!("Failed to evaluate _INI for device {}: {:?}", path, err); } @@ -321,8 +364,9 @@ where info!("Initialized {} devices", num_devices_initialized); } - pub fn acquire_global_lock(&self, timeout: u16) -> Result<(), AmlError> { - self.handler.acquire(self.global_lock_mutex, timeout)?; + pub fn acquire_global_lock(&self, timeout: u16) -> Result<(), AmlError> { + // Handler::acquire returns `AmlError`; re-map to AmlError. + self.handler.acquire(self.global_lock_mutex, timeout).map_err(|_| AmlError::MutexAcquireTimeout)?; // Now we've acquired the AML-side mutex, acquire the hardware side // TODO: count the number of times we have to go round this loop / enforce a timeout? @@ -373,7 +417,7 @@ where } } - pub fn release_global_lock(&self) -> Result<(), AmlError> { + pub fn release_global_lock(&self) -> Result<(), AmlError> { let is_pending = self.do_release_firmware_lock(); if is_pending { self.registers.pm1_control_registers.set_bit(Pm1ControlBit::GlobalLockRelease, true).unwrap(); @@ -405,11 +449,27 @@ where /// Returns the size of an integer (in bytes) for the set of tables parsed so far. This depends /// on the revision of the initial DSDT. + // PILOT-DECISION: small helpers wrapping the allocator-aware OpInFlight + // constructors. Saves threading `self.alloc.clone()` through hundreds of + // call sites in `do_execute_method`. + fn new_op(&self, op: Opcode, behaviours: &'static [ResolveBehaviour]) -> OpInFlight { + OpInFlight::new(op, behaviours, self.alloc.clone()) + } + + fn new_op_dynamic( + &self, + op: Opcode, + expected_arguments: usize, + behaviours: &'static [ResolveBehaviour], + ) -> OpInFlight { + OpInFlight::new_dynamic(op, expected_arguments, behaviours, self.alloc.clone()) + } + pub fn integer_size(&self) -> usize { if self.dsdt_revision >= 2 { 8 } else { 4 } } - fn do_execute_method(&self, mut context: MethodContext) -> Result { + fn do_execute_method(&self, mut context: MethodContext) -> Result, AmlError> { /* * This is the main loop that executes operations. Every op is handled at the top-level of * the loop to prevent pathological stack growth from nested operations. @@ -432,7 +492,7 @@ where * traditional fast bytecode VM, but also provides enough flexibility to handle the * quirkier parts of the AML grammar, particularly the left-to-right encoding of operands. */ - let mut context_stack: Vec = Vec::new(); + let mut context_stack: Vec, A> = Vec::new_in(self.alloc.clone()); loop { /* @@ -474,7 +534,7 @@ where }; *operand = new_value; - context.contribute_arg(Argument::Object(Object::Integer(new_value).wrap())); + context.contribute_arg(Argument::Object(Object::Integer(new_value).wrap(self.alloc.clone()))); context.retire_op(op); } Opcode::LAnd @@ -495,7 +555,7 @@ where Opcode::Mid => self.do_mid(&mut context, op)?, Opcode::Concat => self.do_concat(&mut context, op)?, Opcode::ConcatRes => { - extract_args!(op => [ + extract_args!(op, self.alloc.clone() => [ Argument::Object(source1), Argument::Object(source2), Argument::Object(target) @@ -503,13 +563,13 @@ where let source1 = source1.as_buffer()?; let source2 = source2.as_buffer()?; let result = { - let mut buffer = Vec::from(source1); + let mut buffer = source1.to_vec_in(self.alloc.clone()); buffer.extend_from_slice(source2); // Add a new end-tag buffer.push(0x78); // Don't calculate the new real checksum - just use 0 buffer.push(0x00); - Object::Buffer(buffer).wrap() + Object::Buffer(buffer).wrap(self.alloc.clone()) }; // TODO: use potentially-updated result for return value here self.do_store(target.clone(), result.clone())?; @@ -517,7 +577,7 @@ where context.retire_op(op); } Opcode::Reset => { - extract_args!(op => [Argument::Object(sync_object)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(sync_object)]); let sync_object = sync_object.clone().unwrap_reference(); if let Object::Event(ref counter) = *sync_object { @@ -530,7 +590,7 @@ where } } Opcode::Signal => { - extract_args!(op => [Argument::Object(sync_object)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(sync_object)]); let sync_object = sync_object.clone().unwrap_reference(); if let Object::Event(ref counter) = *sync_object { @@ -543,7 +603,7 @@ where } } Opcode::Wait => { - extract_args!(op => [Argument::Object(sync_object), Argument::Object(timeout)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(sync_object), Argument::Object(timeout)]); let sync_object = sync_object.clone().unwrap_reference(); let timeout = u64::min(timeout.as_integer()?, 0xffff); @@ -586,7 +646,7 @@ where } context.contribute_arg(Argument::Object( - Object::Integer(if timed_out { u64::MAX } else { 0 }).wrap(), + Object::Integer(if timed_out { u64::MAX } else { 0 }).wrap(self.alloc.clone()), )); } else { return Err(AmlError::InvalidOperationOnObject { @@ -597,7 +657,7 @@ where } Opcode::Notify => { // TODO: may need special handling on the node to get path? - extract_args!(op => [Argument::Namestring(name), Argument::Object(value)]); + extract_args!(op, self.alloc.clone() => [Argument::Namestring(name), Argument::Object(value)]); let value = value.as_integer()?; info!("Notify {:?} with value {}", name, value); @@ -607,20 +667,20 @@ where Opcode::FromBCD => self.do_from_bcd(&mut context, op)?, Opcode::ToBCD => self.do_to_bcd(&mut context, op)?, Opcode::Name => { - extract_args!(op => [Argument::Namestring(name), Argument::Object(object)]); + extract_args!(op, self.alloc.clone() => [Argument::Namestring(name), Argument::Object(object)]); let name = name.resolve(&context.current_scope)?; self.namespace.lock().insert(name, object.clone())?; context.retire_op(op); } Opcode::Fatal => { - extract_args!(op => [Argument::ByteData(typ), Argument::DWordData(code), Argument::Object(arg)]); + extract_args!(op, self.alloc.clone() => [Argument::ByteData(typ), Argument::DWordData(code), Argument::Object(arg)]); let arg = arg.as_integer()?; self.handler.handle_fatal_error(*typ, *code, arg); context.retire_op(op); return Err(AmlError::FatalErrorEncountered); } Opcode::OpRegion => { - extract_args!(op => [ + extract_args!(op, self.alloc.clone() => [ Argument::Namestring(name), Argument::ByteData(region_space), Argument::Object(region_offset), @@ -635,11 +695,11 @@ where length: region_length.as_integer()?, parent_device_path: context.current_scope.clone(), }); - self.namespace.lock().insert(name.resolve(&context.current_scope)?, region.wrap())?; + self.namespace.lock().insert(name.resolve(&context.current_scope)?, region.wrap(self.alloc.clone()))?; context.retire_op(op); } Opcode::DataRegion => { - extract_args!(op => [ + extract_args!(op, self.alloc.clone() => [ Argument::Namestring(name), Argument::Object(signature), Argument::Object(oem_id), @@ -660,11 +720,11 @@ where length: 0, parent_device_path: context.current_scope.clone(), }); - self.namespace.lock().insert(name.resolve(&context.current_scope)?, region.wrap())?; + self.namespace.lock().insert(name.resolve(&context.current_scope)?, region.wrap(self.alloc.clone()))?; context.retire_op(op); } Opcode::Buffer => { - extract_args!(op => [ + extract_args!(op, self.alloc.clone() => [ Argument::TrackedPc(start_pc), Argument::PkgLength(pkg_length), Argument::Object(buffer_size), @@ -672,23 +732,24 @@ where let buffer_size = buffer_size.clone().unwrap_transparent_reference().as_integer()?; let buffer_len = pkg_length - (context.current_block.pc - start_pc); - let mut buffer = vec![0; buffer_size as usize]; + let mut buffer = vec_in!(self.alloc.clone(); 0; buffer_size as usize); buffer[0..buffer_len].copy_from_slice( &context.current_block.stream() [context.current_block.pc..(context.current_block.pc + buffer_len)], ); context.current_block.pc += buffer_len; - context.contribute_arg(Argument::Object(Object::Buffer(buffer).wrap())); + context.contribute_arg(Argument::Object(Object::Buffer(buffer).wrap(self.alloc.clone()))); context.retire_op(op); } Opcode::Package => { - let mut elements = Vec::with_capacity(op.expected_arguments); + let mut elements = Vec::with_capacity_in(op.expected_arguments, self.alloc.clone()); for arg in &op.arguments { let Argument::Object(object) = arg else { - return Err(AmlError::InternalError( - "Invalid argument type produced for package element".to_string(), - )); + return Err(AmlError::InternalError(AmlString::from_str_in( + "Invalid argument type produced for package element", + self.alloc.clone(), + ))); }; elements.push(object.clone()); } @@ -704,23 +765,24 @@ where * To make these consistent, we always remove the block here, making sure * we've finished it as a sanity check. */ - assert_eq!(context.current_block.kind, BlockKind::Package); - assert_eq!(context.peek(), Err(AmlError::RunOutOfStream)); + assert_eq!(context.current_block.kind, BlockKind::::Package); + assert_eq!(context.peek(), Err::>(AmlError::RunOutOfStream)); context.current_block = context.block_stack.pop().unwrap(); - context.contribute_arg(Argument::Object(Object::Package(elements).wrap())); + context.contribute_arg(Argument::Object(Object::Package(elements).wrap(self.alloc.clone()))); context.retire_op(op); } Opcode::VarPackage => { - extract_args!(op[0..1] => [Argument::Object(total_elements)]); + extract_args!(op[0..1], self.alloc.clone() => [Argument::Object(total_elements)]); let total_elements = total_elements.clone().unwrap_transparent_reference().as_integer()? as usize; - let mut elements = Vec::with_capacity(total_elements); + let mut elements = Vec::with_capacity_in(total_elements, self.alloc.clone()); for arg in &op.arguments[1..] { let Argument::Object(object) = arg else { - return Err(AmlError::InternalError( - "Invalid argument type produced for package element".to_string(), - )); + return Err(AmlError::InternalError(AmlString::from_str_in( + "Invalid argument type produced for package element", + self.alloc.clone(), + ))); }; elements.push(object.clone()); } @@ -729,14 +791,14 @@ where * As above, we always remove the block here after the in-flight op has * been retired. */ - assert_eq!(context.current_block.kind, BlockKind::VarPackage); - assert_eq!(context.peek(), Err(AmlError::RunOutOfStream)); + assert_eq!(context.current_block.kind, BlockKind::::VarPackage); + assert_eq!(context.peek(), Err::>(AmlError::RunOutOfStream)); context.current_block = context.block_stack.pop().unwrap(); - context.contribute_arg(Argument::Object(Object::Package(elements).wrap())); + context.contribute_arg(Argument::Object(Object::Package(elements).wrap(self.alloc.clone()))); context.retire_op(op); } Opcode::If => { - extract_args!(op => [ + extract_args!(op, self.alloc.clone() => [ Argument::TrackedPc(start_pc), Argument::PkgLength(then_length), Argument::Object(predicate), @@ -772,7 +834,7 @@ where | opcode @ Opcode::CreateWordField | opcode @ Opcode::CreateDWordField | opcode @ Opcode::CreateQWordField => { - extract_args!(op => [Argument::Object(buffer), Argument::Object(index)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(buffer), Argument::Object(index)]); let name = context.namestring()?; let index = index.as_integer()?; let (offset, length) = match opcode { @@ -785,12 +847,12 @@ where }; self.namespace.lock().insert( name.resolve(&context.current_scope)?, - Object::BufferField { buffer: buffer.clone(), offset: offset as usize, length }.wrap(), + Object::BufferField { buffer: buffer.clone(), offset: offset as usize, length }.wrap(self.alloc.clone()), )?; context.retire_op(op); } Opcode::CreateField => { - extract_args!(op => [Argument::Object(buffer), Argument::Object(bit_index), Argument::Object(num_bits)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(buffer), Argument::Object(bit_index), Argument::Object(num_bits)]); let name = context.namestring()?; let bit_index = bit_index.as_integer()?; let num_bits = num_bits.as_integer()?; @@ -802,46 +864,46 @@ where offset: bit_index as usize, length: num_bits as usize, } - .wrap(), + .wrap(self.alloc.clone()), )?; context.retire_op(op); } Opcode::CopyObject => { - extract_args!(op => [Argument::Object(object), Argument::Object(target)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(object), Argument::Object(target)]); self.do_copy_object(target.clone(), object.clone())?; context.retire_op(op); } Opcode::Store => { - extract_args!(op => [Argument::Object(object), Argument::Object(target)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(object), Argument::Object(target)]); self.do_store(target.clone(), object.clone())?; context.retire_op(op); } Opcode::RefOf => { - extract_args!(op => [Argument::Object(object)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(object)]); let reference = - Object::Reference { kind: ReferenceKind::RefOf, inner: object.clone() }.wrap(); + Object::Reference { kind: ReferenceKind::RefOf, inner: object.clone() }.wrap(self.alloc.clone()); context.contribute_arg(Argument::Object(reference)); context.retire_op(op); } Opcode::CondRefOf => { - extract_args!(op => [Argument::Object(object), Argument::Object(target)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(object), Argument::Object(target)]); let result = if let Object::Reference { kind: ReferenceKind::Unresolved, .. } = **object { Object::Integer(0) } else { let reference = - Object::Reference { kind: ReferenceKind::RefOf, inner: object.clone() }.wrap(); + Object::Reference { kind: ReferenceKind::RefOf, inner: object.clone() }.wrap(self.alloc.clone()); self.do_store(target.clone(), reference)?; Object::Integer(u64::MAX) }; - context.contribute_arg(Argument::Object(result.wrap())); + context.contribute_arg(Argument::Object(result.wrap(self.alloc.clone()))); context.retire_op(op); } Opcode::DerefOf => { - extract_args!(op => [Argument::Object(object)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(object)]); let result = if object.typ() == ObjectType::Reference { object.clone().unwrap_reference() } else if object.typ() == ObjectType::String { - let path = AmlName::from_str(&object.as_string().unwrap())?; + let path = AmlName::parse_in(object.as_string().unwrap(), self.alloc.clone())?; let (_, object) = self.namespace.lock().search(&path, &context.current_scope)?; object.clone() } else { @@ -854,14 +916,14 @@ where context.retire_op(op); } Opcode::Load => { - extract_args!(op => [Argument::Namestring(object), Argument::Object(result)]); + extract_args!(op, self.alloc.clone() => [Argument::Namestring(object), Argument::Object(result)]); // TODO: read the AML from the object and load it warn!("Ignoring unsupported DefLoad operation (object={}, result = {})", object, result); context.retire_op(op); return Err(AmlError::LibUnimplemented); } Opcode::LoadTable => { - extract_args!(op => [ + extract_args!(op, self.alloc.clone() => [ Argument::Object(signature), Argument::Object(oem_id), Argument::Object(oem_table_id), @@ -878,17 +940,17 @@ where return Err(AmlError::LibUnimplemented); } Opcode::Sleep => { - extract_args!(op => [Argument::Object(msec)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(msec)]); self.handler.sleep(msec.as_integer()?); context.retire_op(op); } Opcode::Stall => { - extract_args!(op => [Argument::Object(usec)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(usec)]); self.handler.stall(usec.as_integer()?); context.retire_op(op); } Opcode::Acquire => { - extract_args!(op => [Argument::Object(mutex)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(mutex)]); let Object::Mutex { mutex, sync_level: _ } = **mutex else { Err(AmlError::InvalidOperationOnObject { op: Operation::Acquire, typ: mutex.typ() })? }; @@ -898,13 +960,13 @@ where if mutex == self.global_lock_mutex { self.acquire_global_lock(timeout)?; } else { - self.handler.acquire(mutex, timeout)?; + self.handler.acquire(mutex, timeout).map_err(|_| AmlError::MutexAcquireTimeout)?; } context.retire_op(op); } Opcode::Release => { - extract_args!(op => [Argument::Object(mutex)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(mutex)]); let Object::Mutex { mutex, sync_level: _ } = **mutex else { Err(AmlError::InvalidOperationOnObject { op: Operation::Release, typ: mutex.typ() })? }; @@ -919,17 +981,18 @@ where context.retire_op(op); } Opcode::InternalMethodCall => { - extract_args!(op[0..2] => [Argument::Object(method), Argument::Namestring(method_scope)]); - let args = op.arguments[2..] - .iter() - .map(|arg| { - if let Argument::Object(arg) = arg { - arg.clone() - } else { - panic!(); - } - }) - .collect(); + extract_args!(op[0..2], self.alloc.clone() => [Argument::Object(method), Argument::Namestring(method_scope)]); + // PILOT-DECISION: `.collect()` into a `Vec<_, A>` requires + // an allocator-aware FromIterator, which doesn't exist + // on stable nightly yet. Manual `push` loop instead. + let mut args: Vec, A> = Vec::new_in(self.alloc.clone()); + for arg in &op.arguments[2..] { + if let Argument::Object(arg) = arg { + args.push(arg.clone()); + } else { + panic!(); + } + } if let Object::Method { .. } = **method { self.namespace @@ -937,7 +1000,7 @@ where .add_level(method_scope.clone(), NamespaceLevelKind::MethodLocals)?; let new_context = - MethodContext::new_from_method(method.clone(), args, method_scope.clone())?; + MethodContext::new_from_method(method.clone(), args, method_scope.clone(), self.alloc.clone())?; let old_context = mem::replace(&mut context, new_context); context_stack.push(old_context); context.retire_op(op); @@ -949,7 +1012,7 @@ where } } Opcode::Return => { - extract_args!(op => [Argument::Object(object)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(object)]); let object = object.clone().unwrap_transparent_reference(); if let Some(last) = context_stack.pop() { @@ -965,11 +1028,11 @@ where } } Opcode::ObjectType => { - extract_args!(op => [Argument::Object(object)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(object)]); // TODO: this should technically support scopes as well - this is less easy // (they should return `0`) - fn object_type(object: &Object) -> u64 { + fn object_type(object: &Object) -> u64 { if let Object::Reference { kind: _, inner } = object { object_type(&inner) } else { @@ -997,13 +1060,13 @@ where } } - context.contribute_arg(Argument::Object(Object::Integer(object_type(&object)).wrap())); + context.contribute_arg(Argument::Object(Object::Integer(object_type(&object)).wrap(self.alloc.clone()))); context.retire_op(op); } Opcode::SizeOf => self.do_size_of(&mut context, op)?, Opcode::Index => self.do_index(&mut context, op)?, Opcode::BankField => { - extract_args!(op => [ + extract_args!(op, self.alloc.clone() => [ Argument::TrackedPc(start_pc), Argument::PkgLength(pkg_length), Argument::Namestring(region_name), @@ -1029,7 +1092,7 @@ where * We've just evaluated the predicate for an iteration of a while loop. If * false, skip over the rest of the loop, otherwise carry on. */ - extract_args!(op => [Argument::Object(predicate)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(predicate)]); let predicate = predicate.clone().unwrap_transparent_reference().as_integer()?; if predicate == 0 { @@ -1055,7 +1118,7 @@ where */ match context.current_block.kind { BlockKind::Table => { - break Ok(Object::Uninitialized.wrap()); + break Ok(Object::Uninitialized.wrap(self.alloc.clone())); } BlockKind::Method { method_scope } => { self.namespace.lock().remove_level(method_scope)?; @@ -1068,7 +1131,7 @@ where * If there is no explicit `Return` op, the result is undefined. We * just return an uninitialized object. */ - return Ok(Object::Uninitialized.wrap()); + return Ok(Object::Uninitialized.wrap(self.alloc.clone())); } } BlockKind::Scope { old_scope } => { @@ -1095,7 +1158,7 @@ where { let num_elements_left = package_op.expected_arguments - package_op.arguments.len(); for _ in 0..num_elements_left { - package_op.arguments.push(Argument::Object(Object::Uninitialized.wrap())); + package_op.arguments.push(Argument::Object(Object::Uninitialized.wrap(self.alloc.clone()))); } } @@ -1123,7 +1186,7 @@ where }; for _ in 0..num_elements_left { - package_op.arguments.push(Argument::Object(Object::Uninitialized.wrap())); + package_op.arguments.push(Argument::Object(Object::Uninitialized.wrap(self.alloc.clone()))); } } @@ -1160,7 +1223,7 @@ where * predicate. */ context.current_block.pc = start_pc; - context.start(OpInFlight::new(Opcode::While, &[ResolveBehaviour::TermArg])); + context.start(self.new_op(Opcode::While, &[ResolveBehaviour::TermArg])); continue; } } @@ -1174,13 +1237,13 @@ where * most places, but could also encode a `NullName` if we are expecting a * `Target`. We handle the latter in logic for stores to targets. */ - context.contribute_arg(Argument::Object(Object::Integer(0).wrap())); + context.contribute_arg(Argument::Object(Object::Integer(0).wrap(self.alloc.clone()))); } Opcode::One => { - context.contribute_arg(Argument::Object(Object::Integer(1).wrap())); + context.contribute_arg(Argument::Object(Object::Integer(1).wrap(self.alloc.clone()))); } Opcode::Ones => { - context.contribute_arg(Argument::Object(Object::Integer(u64::MAX).wrap())); + context.contribute_arg(Argument::Object(Object::Integer(u64::MAX).wrap(self.alloc.clone()))); } Opcode::Alias => { let source = context.namestring()?; @@ -1195,35 +1258,36 @@ where let name = context.namestring()?; context.start(OpInFlight::new_with( Opcode::Name, - vec![Argument::Namestring(name)], + vec_in!(self.alloc.clone(); Argument::Namestring(name)), &[ResolveBehaviour::Placeholder, ResolveBehaviour::TermArg], )); } Opcode::BytePrefix => { let value = context.next()?; - context.contribute_arg(Argument::Object(Object::Integer(value as u64).wrap())); + context.contribute_arg(Argument::Object(Object::Integer(value as u64).wrap(self.alloc.clone()))); } Opcode::WordPrefix => { let value = context.next_u16()?; - context.contribute_arg(Argument::Object(Object::Integer(value as u64).wrap())); + context.contribute_arg(Argument::Object(Object::Integer(value as u64).wrap(self.alloc.clone()))); } Opcode::DWordPrefix => { let value = context.next_u32()?; - context.contribute_arg(Argument::Object(Object::Integer(value as u64).wrap())); + context.contribute_arg(Argument::Object(Object::Integer(value as u64).wrap(self.alloc.clone()))); } Opcode::StringPrefix => { let str_start = context.current_block.pc; while context.next()? != b'\0' {} // TODO: handle err - let str = String::from( - str::from_utf8(&context.current_block.stream()[str_start..(context.current_block.pc - 1)]) - .unwrap(), - ); - context.contribute_arg(Argument::Object(Object::String(str).wrap())); + let s = core::str::from_utf8( + &context.current_block.stream()[str_start..(context.current_block.pc - 1)], + ) + .unwrap(); + let str = AmlString::from_str_in(s, self.alloc.clone()); + context.contribute_arg(Argument::Object(Object::String(str).wrap(self.alloc.clone()))); } Opcode::QWordPrefix => { let value = context.next_u64()?; - context.contribute_arg(Argument::Object(Object::Integer(value).wrap())); + context.contribute_arg(Argument::Object(Object::Integer(value).wrap(self.alloc.clone()))); } Opcode::Scope => { let start_pc = context.current_block.pc; @@ -1243,7 +1307,7 @@ where let pkg_length = context.pkglength()?; context.start(OpInFlight::new_with( Opcode::Buffer, - vec![Argument::TrackedPc(start_pc), Argument::PkgLength(pkg_length)], + vec_in!(self.alloc.clone(); Argument::TrackedPc(start_pc), Argument::PkgLength(pkg_length)), &[ResolveBehaviour::Placeholder, ResolveBehaviour::Placeholder, ResolveBehaviour::TermArg], )); } @@ -1261,7 +1325,7 @@ where * combination of a block to manage the pkglength, plus an in-flight op to * store interpreted arguments. */ - context.start(OpInFlight::new_dynamic( + context.start(self.new_op_dynamic( Opcode::Package, num_elements as usize, &[ResolveBehaviour::AsPackageElements], @@ -1279,7 +1343,7 @@ where * elements as remain in the block, and we'll sort out how many are supposed to * be in the package later. */ - context.start(OpInFlight::new_dynamic( + context.start(self.new_op_dynamic( Opcode::VarPackage, usize::MAX, &[ResolveBehaviour::TermArg, ResolveBehaviour::AsPackageElements], @@ -1295,11 +1359,11 @@ where let code_len = pkg_length - (context.current_block.pc - start_pc); let code = context.current_block.stream() [context.current_block.pc..(context.current_block.pc + code_len)] - .to_vec(); + .to_vec_in(self.alloc.clone()); context.current_block.pc += code_len; let name = name.resolve(&context.current_scope)?; - self.namespace.lock().insert(name, Object::Method { code, flags }.wrap())?; + self.namespace.lock().insert(name, Object::Method { code, flags }.wrap(self.alloc.clone()))?; } Opcode::External => { let _name = context.namestring()?; @@ -1312,62 +1376,62 @@ where let name = name.resolve(&context.current_scope)?; let mutex = self.handler.create_mutex(); - self.namespace.lock().insert(name, Object::Mutex { mutex, sync_level }.wrap())?; + self.namespace.lock().insert(name, Object::Mutex { mutex, sync_level }.wrap(self.alloc.clone()))?; } Opcode::Event => { let name = context.namestring()?; let name = name.resolve(&context.current_scope)?; - self.namespace.lock().insert(name, Object::Event(Arc::new(AtomicU64::new(0))).wrap())?; + self.namespace.lock().insert(name, Object::Event(Arc::new_in(AtomicU64::new(0), self.alloc.clone())).wrap(self.alloc.clone()))?; } Opcode::LoadTable => { - context.start(OpInFlight::new(Opcode::LoadTable, &[ResolveBehaviour::TermArg; 6])); + context.start(self.new_op(Opcode::LoadTable, &[ResolveBehaviour::TermArg; 6])); } Opcode::Load => { let name = context.namestring()?; context.start(OpInFlight::new_with( Opcode::Load, - vec![Argument::Namestring(name)], + vec_in!(self.alloc.clone(); Argument::Namestring(name)), &[ResolveBehaviour::Target], )); } - Opcode::Stall => context.start(OpInFlight::new(Opcode::Stall, &[ResolveBehaviour::TermArg])), - Opcode::Sleep => context.start(OpInFlight::new(Opcode::Sleep, &[ResolveBehaviour::TermArg])), - Opcode::Acquire => context.start(OpInFlight::new(opcode, &[ResolveBehaviour::SuperName])), - Opcode::Release => context.start(OpInFlight::new(opcode, &[ResolveBehaviour::SuperName])), - Opcode::Signal => context.start(OpInFlight::new(opcode, &[ResolveBehaviour::SuperName])), + Opcode::Stall => context.start(self.new_op(Opcode::Stall, &[ResolveBehaviour::TermArg])), + Opcode::Sleep => context.start(self.new_op(Opcode::Sleep, &[ResolveBehaviour::TermArg])), + Opcode::Acquire => context.start(self.new_op(opcode, &[ResolveBehaviour::SuperName])), + Opcode::Release => context.start(self.new_op(opcode, &[ResolveBehaviour::SuperName])), + Opcode::Signal => context.start(self.new_op(opcode, &[ResolveBehaviour::SuperName])), Opcode::Wait => context - .start(OpInFlight::new(opcode, &[ResolveBehaviour::SuperName, ResolveBehaviour::TermArg])), - Opcode::Reset => context.start(OpInFlight::new(opcode, &[ResolveBehaviour::SuperName])), + .start(self.new_op(opcode, &[ResolveBehaviour::SuperName, ResolveBehaviour::TermArg])), + Opcode::Reset => context.start(self.new_op(opcode, &[ResolveBehaviour::SuperName])), Opcode::Notify => context - .start(OpInFlight::new(opcode, &[ResolveBehaviour::SuperName, ResolveBehaviour::TermArg])), + .start(self.new_op(opcode, &[ResolveBehaviour::SuperName, ResolveBehaviour::TermArg])), Opcode::FromBCD | Opcode::ToBCD => { - context.start(OpInFlight::new(opcode, &[ResolveBehaviour::TermArg, ResolveBehaviour::Target])) + context.start(self.new_op(opcode, &[ResolveBehaviour::TermArg, ResolveBehaviour::Target])) } Opcode::Revision => { - context.contribute_arg(Argument::Object(Object::Integer(INTERPRETER_REVISION).wrap())); + context.contribute_arg(Argument::Object(Object::Integer(INTERPRETER_REVISION).wrap(self.alloc.clone()))); } - Opcode::Debug => context.contribute_arg(Argument::Object(Object::Debug.wrap())), + Opcode::Debug => context.contribute_arg(Argument::Object(Object::Debug.wrap(self.alloc.clone()))), Opcode::Fatal => { let typ = context.next()?; let code = context.next_u32()?; context.start(OpInFlight::new_with( Opcode::Fatal, - vec![Argument::ByteData(typ), Argument::DWordData(code)], + vec_in!(self.alloc.clone(); Argument::ByteData(typ), Argument::DWordData(code)), &[ResolveBehaviour::Placeholder, ResolveBehaviour::Placeholder, ResolveBehaviour::TermArg], )); } Opcode::Timer => { // Time has to be monotonically-increasing, in 100ns units let time = self.handler.nanos_since_boot() / 100; - context.contribute_arg(Argument::Object(Object::Integer(time).wrap())); + context.contribute_arg(Argument::Object(Object::Integer(time).wrap(self.alloc.clone()))); } Opcode::OpRegion => { let name = context.namestring()?; let region_space = context.next()?; context.start(OpInFlight::new_with( Opcode::OpRegion, - vec![Argument::Namestring(name), Argument::ByteData(region_space)], + vec_in!(self.alloc.clone(); Argument::Namestring(name), Argument::ByteData(region_space)), &[ ResolveBehaviour::Placeholder, ResolveBehaviour::Placeholder, @@ -1380,7 +1444,7 @@ where let name = context.namestring()?; context.start(OpInFlight::new_with( Opcode::DataRegion, - vec![Argument::Namestring(name)], + vec_in!(self.alloc.clone(); Argument::Namestring(name)), &[ ResolveBehaviour::Placeholder, ResolveBehaviour::TermArg, @@ -1407,12 +1471,12 @@ where context.start(OpInFlight::new_with( Opcode::BankField, - vec![ + vec_in!(self.alloc.clone(); Argument::TrackedPc(start_pc), Argument::PkgLength(pkg_length), Argument::Namestring(region_name), Argument::Namestring(bank_name), - ], + ), &[ ResolveBehaviour::Placeholder, ResolveBehaviour::Placeholder, @@ -1480,7 +1544,7 @@ where }; let mut namespace = self.namespace.lock(); namespace.add_level(new_scope.clone(), kind)?; - namespace.insert(new_scope.clone(), object.wrap())?; + namespace.insert(new_scope.clone(), object.wrap(self.alloc.clone()))?; let old_scope = mem::replace(&mut context.current_scope, new_scope); context.start_new_block(BlockKind::Scope { old_scope }, remaining_length); @@ -1499,7 +1563,7 @@ where let object = Object::Processor { proc_id, pblk_address, pblk_length }; let mut namespace = self.namespace.lock(); namespace.add_level(new_scope.clone(), NamespaceLevelKind::Processor)?; - namespace.insert(new_scope.clone(), object.wrap())?; + namespace.insert(new_scope.clone(), object.wrap(self.alloc.clone()))?; let old_scope = mem::replace(&mut context.current_scope, new_scope); context.start_new_block(BlockKind::Scope { old_scope }, remaining_length); @@ -1517,7 +1581,7 @@ where let object = Object::PowerResource { system_level, resource_order }; let mut namespace = self.namespace.lock(); namespace.add_level(new_scope.clone(), NamespaceLevelKind::PowerResource)?; - namespace.insert(new_scope.clone(), object.wrap())?; + namespace.insert(new_scope.clone(), object.wrap(self.alloc.clone()))?; let old_scope = mem::replace(&mut context.current_scope, new_scope); context.start_new_block(BlockKind::Scope { old_scope }, remaining_length); @@ -1525,25 +1589,25 @@ where Opcode::Local(local) => { let local = context.locals[local as usize].clone(); context.contribute_arg(Argument::Object( - Object::Reference { kind: ReferenceKind::Local, inner: local }.wrap(), + Object::Reference { kind: ReferenceKind::Local, inner: local }.wrap(self.alloc.clone()), )); } Opcode::Arg(arg) => { let arg = context.args[arg as usize].clone(); context.contribute_arg(Argument::Object( - Object::Reference { kind: ReferenceKind::Arg, inner: arg }.wrap(), + Object::Reference { kind: ReferenceKind::Arg, inner: arg }.wrap(self.alloc.clone()), )); } - Opcode::Store => context.start(OpInFlight::new( + Opcode::Store => context.start(self.new_op( Opcode::Store, &[ResolveBehaviour::TermArg, ResolveBehaviour::SuperName], )), - Opcode::CopyObject => context.start(OpInFlight::new( + Opcode::CopyObject => context.start(self.new_op( Opcode::CopyObject, &[ResolveBehaviour::TermArg, ResolveBehaviour::SimpleName], )), - Opcode::RefOf => context.start(OpInFlight::new(Opcode::RefOf, &[ResolveBehaviour::SuperName])), - Opcode::CondRefOf => context.start(OpInFlight::new( + Opcode::RefOf => context.start(self.new_op(Opcode::RefOf, &[ResolveBehaviour::SuperName])), + Opcode::CondRefOf => context.start(self.new_op( opcode, &[ResolveBehaviour::SuperNameIfExists, ResolveBehaviour::Target], )), @@ -1569,7 +1633,7 @@ where match object { Ok((_resolved_name, object)) => { context.contribute_arg(Argument::Object( - Object::Reference { kind: ReferenceKind::Named, inner: object }.wrap(), + Object::Reference { kind: ReferenceKind::Named, inner: object }.wrap(self.alloc.clone()), )); } Err(err) => Err(err)?, @@ -1582,11 +1646,14 @@ where context.contribute_arg(Argument::Object(object)); } Err(AmlError::ObjectDoesNotExist(_)) => { + let mut name_str = AmlString::new_in(self.alloc.clone()); + use core::fmt::Write; + write!(name_str, "{}", name).unwrap(); let reference = Object::Reference { kind: ReferenceKind::Unresolved, - inner: Object::String(name.to_string()).wrap(), + inner: Object::String(name_str).wrap(self.alloc.clone()), }; - context.contribute_arg(Argument::Object(reference.wrap())); + context.contribute_arg(Argument::Object(reference.wrap(self.alloc.clone()))); } Err(err) => Err(err)?, } @@ -1600,7 +1667,7 @@ where { context.start(OpInFlight::new_with_dynamic( Opcode::InternalMethodCall, - vec![Argument::Object(object), Argument::Namestring(resolved_name)], + vec_in!(self.alloc.clone(); Argument::Object(object), Argument::Namestring(resolved_name)), flags.arg_count(), &[ ResolveBehaviour::Placeholder, @@ -1619,8 +1686,8 @@ where let value = self.do_field_read(field)?; context.contribute_arg(Argument::Object(value)); } else if let Object::BufferField { .. } = *object { - let value = object.read_buffer_field(self.integer_size())?; - context.contribute_arg(Argument::Object(value.wrap())); + let value = object.read_buffer_field(self.integer_size(), self.alloc.clone())?; + context.contribute_arg(Argument::Object(value.wrap(self.alloc.clone()))); } else { context.contribute_arg(Argument::Object(object)); } @@ -1629,7 +1696,10 @@ where } } ResolveBehaviour::AsPackageElements => { - context.contribute_arg(Argument::Object(Object::String(name.to_string()).wrap())); + let mut name_str = AmlString::new_in(self.alloc.clone()); + use core::fmt::Write; + write!(name_str, "{}", name).unwrap(); + context.contribute_arg(Argument::Object(Object::String(name_str).wrap(self.alloc.clone()))); } ResolveBehaviour::Placeholder => { panic!("Invalid resolve behaviour for name to be resolved!") @@ -1649,13 +1719,13 @@ where | Opcode::Nor | Opcode::Xor | Opcode::Concat => { - context.start(OpInFlight::new( + context.start(self.new_op( opcode, &[ResolveBehaviour::TermArg, ResolveBehaviour::TermArg, ResolveBehaviour::Target], )); } - Opcode::Divide => context.start(OpInFlight::new( + Opcode::Divide => context.start(self.new_op( Opcode::Divide, &[ ResolveBehaviour::TermArg, @@ -1665,20 +1735,20 @@ where ], )), Opcode::Increment | Opcode::Decrement => { - context.start(OpInFlight::new(opcode, &[ResolveBehaviour::SuperName])) + context.start(self.new_op(opcode, &[ResolveBehaviour::SuperName])) } Opcode::Not => context - .start(OpInFlight::new(Opcode::Not, &[ResolveBehaviour::TermArg, ResolveBehaviour::Target])), + .start(self.new_op(Opcode::Not, &[ResolveBehaviour::TermArg, ResolveBehaviour::Target])), Opcode::FindSetLeftBit | Opcode::FindSetRightBit => { - context.start(OpInFlight::new(opcode, &[ResolveBehaviour::TermArg, ResolveBehaviour::Target])) + context.start(self.new_op(opcode, &[ResolveBehaviour::TermArg, ResolveBehaviour::Target])) } - Opcode::DerefOf => context.start(OpInFlight::new(opcode, &[ResolveBehaviour::TermArg])), - Opcode::ConcatRes => context.start(OpInFlight::new( + Opcode::DerefOf => context.start(self.new_op(opcode, &[ResolveBehaviour::TermArg])), + Opcode::ConcatRes => context.start(self.new_op( opcode, &[ResolveBehaviour::TermArg, ResolveBehaviour::TermArg, ResolveBehaviour::Target], )), - Opcode::SizeOf => context.start(OpInFlight::new(opcode, &[ResolveBehaviour::SuperName])), - Opcode::Index => context.start(OpInFlight::new( + Opcode::SizeOf => context.start(self.new_op(opcode, &[ResolveBehaviour::SuperName])), + Opcode::Index => context.start(self.new_op( opcode, &[ResolveBehaviour::TermArg, ResolveBehaviour::TermArg, ResolveBehaviour::Target], )), @@ -1696,14 +1766,14 @@ where | Opcode::CreateWordField | Opcode::CreateDWordField | Opcode::CreateQWordField => { - context.start(OpInFlight::new(opcode, &[ResolveBehaviour::TermArg; 2])) + context.start(self.new_op(opcode, &[ResolveBehaviour::TermArg; 2])) } Opcode::CreateField => { - context.start(OpInFlight::new(Opcode::CreateField, &[ResolveBehaviour::TermArg; 3])) + context.start(self.new_op(Opcode::CreateField, &[ResolveBehaviour::TermArg; 3])) } Opcode::LNot => { - context.start(OpInFlight::new(Opcode::LNot, &[ResolveBehaviour::TermArg])); + context.start(self.new_op(Opcode::LNot, &[ResolveBehaviour::TermArg])); } Opcode::LAnd @@ -1714,19 +1784,19 @@ where | Opcode::LEqual | Opcode::LGreater | Opcode::LLess => { - context.start(OpInFlight::new(opcode, &[ResolveBehaviour::TermArg; 2])); + context.start(self.new_op(opcode, &[ResolveBehaviour::TermArg; 2])); } Opcode::ToBuffer | Opcode::ToDecimalString | Opcode::ToHexString | Opcode::ToInteger => { - context.start(OpInFlight::new(opcode, &[ResolveBehaviour::TermArg, ResolveBehaviour::Target])) + context.start(self.new_op(opcode, &[ResolveBehaviour::TermArg, ResolveBehaviour::Target])) } - Opcode::ToString => context.start(OpInFlight::new( + Opcode::ToString => context.start(self.new_op( opcode, &[ResolveBehaviour::TermArg, ResolveBehaviour::TermArg, ResolveBehaviour::Target], )), - Opcode::ObjectType => context.start(OpInFlight::new(opcode, &[ResolveBehaviour::SuperName])), - Opcode::Mid => context.start(OpInFlight::new( + Opcode::ObjectType => context.start(self.new_op(opcode, &[ResolveBehaviour::SuperName])), + Opcode::Mid => context.start(self.new_op( Opcode::Mid, &[ ResolveBehaviour::TermArg, @@ -1740,7 +1810,7 @@ where let then_length = context.pkglength()?; context.start(OpInFlight::new_with( Opcode::If, - vec![Argument::TrackedPc(start_pc), Argument::PkgLength(then_length)], + vec_in!(self.alloc.clone(); Argument::TrackedPc(start_pc), Argument::PkgLength(then_length)), &[ResolveBehaviour::Placeholder, ResolveBehaviour::Placeholder, ResolveBehaviour::TermArg], )); } @@ -1753,7 +1823,7 @@ where BlockKind::While { start_pc: context.current_block.pc }, remaining_length, ); - context.start(OpInFlight::new(Opcode::While, &[ResolveBehaviour::TermArg])); + context.start(self.new_op(Opcode::While, &[ResolveBehaviour::TermArg])); } Opcode::Continue => { if let BlockKind::While { start_pc } = &context.current_block.kind { @@ -1769,7 +1839,7 @@ where } } } - context.start(OpInFlight::new(Opcode::While, &[ResolveBehaviour::TermArg])); + context.start(self.new_op(Opcode::While, &[ResolveBehaviour::TermArg])); } Opcode::Break => { if let BlockKind::While { .. } = &context.current_block.kind { @@ -1786,7 +1856,7 @@ where } } } - Opcode::Return => context.start(OpInFlight::new(Opcode::Return, &[ResolveBehaviour::TermArg])), + Opcode::Return => context.start(self.new_op(Opcode::Return, &[ResolveBehaviour::TermArg])), Opcode::Noop => {} Opcode::Breakpoint => { self.handler.breakpoint(); @@ -1799,12 +1869,12 @@ where fn parse_field_list( &self, - context: &mut MethodContext, - kind: FieldUnitKind, + context: &mut MethodContext, + kind: FieldUnitKind, start_pc: usize, pkg_length: usize, mut flags: u8, - ) -> Result<(), AmlError> { + ) -> Result<(), AmlError> { const RESERVED_FIELD: u8 = 0x00; const ACCESS_FIELD: u8 = 0x01; const CONNECT_FIELD: u8 = 0x02; @@ -1852,7 +1922,7 @@ where bit_length: field_length, flags: FieldFlags(flags), }); - self.namespace.lock().insert(field_name.resolve(&context.current_scope)?, field.wrap())?; + self.namespace.lock().insert(field_name.resolve(&context.current_scope)?, field.wrap(self.alloc.clone()))?; field_offset += field_length; } @@ -1862,8 +1932,8 @@ where Ok(()) } - fn do_binary_maths(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - extract_args!(op[0..3] => [Argument::Object(left), Argument::Object(right), Argument::Object(target)]); + fn do_binary_maths(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { + extract_args!(op[0..3], self.alloc.clone() => [Argument::Object(left), Argument::Object(right), Argument::Object(target)]); let target2 = if op.op == Opcode::Divide { Some(&op.arguments[3]) } else { None }; let left = left.clone().unwrap_transparent_reference().as_integer()?; @@ -1875,7 +1945,7 @@ where Opcode::Multiply => left.wrapping_mul(right), Opcode::Divide => { if let Some(Argument::Object(remainder)) = target2 { - self.do_store(remainder.clone(), Object::Integer(left.wrapping_rem(right)).wrap())?; + self.do_store(remainder.clone(), Object::Integer(left.wrapping_rem(right)).wrap(self.alloc.clone()))?; } left.wrapping_div_euclid(right) } @@ -1890,15 +1960,15 @@ where _ => panic!(), }; - let result = Object::Integer(result).wrap(); + let result = Object::Integer(result).wrap(self.alloc.clone()); let result = self.do_store(target.clone(), result)?; context.contribute_arg(Argument::Object(result)); context.retire_op(op); Ok(()) } - fn do_unary_maths(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - extract_args!(op => [Argument::Object(operand)]); + fn do_unary_maths(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { + extract_args!(op, self.alloc.clone() => [Argument::Object(operand)]); let operand = operand.clone().unwrap_transparent_reference().as_integer()?; let result = match op.op { @@ -1934,23 +2004,23 @@ where _ => panic!(), }; - context.contribute_arg(Argument::Object(Object::Integer(result).wrap())); + context.contribute_arg(Argument::Object(Object::Integer(result).wrap(self.alloc.clone()))); context.retire_op(op); Ok(()) } - fn do_logical_op(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { + fn do_logical_op(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { if op.op == Opcode::LNot { - extract_args!(op => [Argument::Object(operand)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(operand)]); let operand = operand.clone().unwrap_transparent_reference().as_integer()?; let result = if operand == 0 { u64::MAX } else { 0 }; - context.contribute_arg(Argument::Object(Object::Integer(result).wrap())); + context.contribute_arg(Argument::Object(Object::Integer(result).wrap(self.alloc.clone()))); context.retire_op(op); return Ok(()); } - extract_args!(op => [Argument::Object(left), Argument::Object(right)]); + extract_args!(op, self.alloc.clone() => [Argument::Object(left), Argument::Object(right)]); let left = left.clone().unwrap_transparent_reference(); let right = right.clone().unwrap_transparent_reference(); @@ -2009,37 +2079,37 @@ where }; let result = if result { Object::Integer(u64::MAX) } else { Object::Integer(0) }; - context.contribute_arg(Argument::Object(result.wrap())); + context.contribute_arg(Argument::Object(result.wrap(self.alloc.clone()))); context.retire_op(op); Ok(()) } - fn do_to_buffer(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - extract_args!(op => [Argument::Object(operand), Argument::Object(target)]); + fn do_to_buffer(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { + extract_args!(op, self.alloc.clone() => [Argument::Object(operand), Argument::Object(target)]); let operand = operand.clone().unwrap_transparent_reference(); let result = match *operand { Object::Buffer(ref bytes) => Object::Buffer(bytes.clone()), Object::Integer(value) => { if self.integer_size() == 8 { - Object::Buffer(value.to_le_bytes().to_vec()) + Object::Buffer(value.to_le_bytes().to_vec_in(self.alloc.clone())) } else { - Object::Buffer((value as u32).to_le_bytes().to_vec()) + Object::Buffer((value as u32).to_le_bytes().to_vec_in(self.alloc.clone())) } } Object::String(ref value) => { // XXX: an empty string is converted to an empty buffer, *without* the null-terminator if value.is_empty() { - Object::Buffer(vec![]) + Object::Buffer(vec_in!(self.alloc.clone())) } else { - let mut bytes = value.as_bytes().to_vec(); + let mut bytes = value.as_bytes().to_vec_in(self.alloc.clone()); bytes.push(b'\0'); Object::Buffer(bytes) } } _ => Err(AmlError::InvalidOperationOnObject { op: Operation::ToBuffer, typ: operand.typ() })?, } - .wrap(); + .wrap(self.alloc.clone()); let result = self.do_store(target.clone(), result)?; context.contribute_arg(Argument::Object(result)); @@ -2047,8 +2117,8 @@ where Ok(()) } - fn do_to_integer(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - extract_args!(op => [Argument::Object(operand), Argument::Object(target)]); + fn do_to_integer(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { + extract_args!(op, self.alloc.clone() => [Argument::Object(operand), Argument::Object(target)]); let operand = operand.clone().unwrap_transparent_reference(); let result = match *operand { @@ -2070,12 +2140,16 @@ where * that won't fit in a `u64` etc. We probably need to write a more robust parser * 'real' parser to handle those cases. */ - let value = value.trim(); - let value = value.to_ascii_lowercase(); - let (value, radix): (&str, u32) = match value.strip_prefix("0x") { - Some(value) => (value.split(|c: char| !c.is_ascii_hexdigit()).next().unwrap_or(""), 16), - None => (value.split(|c: char| !c.is_ascii_digit()).next().unwrap_or(""), 10), - }; + // PILOT-DECISION: was `value.to_ascii_lowercase()` (String). + // Hex parsing accepts mixed case already, so we only need a + // case-insensitive prefix check for "0x"/"0X". + let value = value.as_str().trim(); + let (value, radix): (&str, u32) = + if let Some(v) = value.strip_prefix("0x").or_else(|| value.strip_prefix("0X")) { + (v.split(|c: char| !c.is_ascii_hexdigit()).next().unwrap_or(""), 16) + } else { + (value.split(|c: char| !c.is_ascii_digit()).next().unwrap_or(""), 10) + }; match value.len() { 0 => Object::Integer(0), _ => Object::Integer(u64::from_str_radix(value, radix).map_err(|_| { @@ -2085,7 +2159,7 @@ where } _ => Err(AmlError::InvalidOperationOnObject { op: Operation::ToBuffer, typ: operand.typ() })?, } - .wrap(); + .wrap(self.alloc.clone()); let result = self.do_store(target.clone(), result)?; context.contribute_arg(Argument::Object(result)); @@ -2093,26 +2167,26 @@ where Ok(()) } - fn do_to_string(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - extract_args!(op => [Argument::Object(source), Argument::Object(length), Argument::Object(target)]); + fn do_to_string(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { + extract_args!(op, self.alloc.clone() => [Argument::Object(source), Argument::Object(length), Argument::Object(target)]); let source = source.clone().unwrap_transparent_reference(); let source = source.as_buffer()?; let length = length.clone().unwrap_transparent_reference().as_integer()? as usize; let result = if source.is_empty() { - Object::String(String::new()) + Object::String(AmlString::new_in(self.alloc.clone())) } else { let mut buffer = source.split_inclusive(|b| *b == b'\0').next().unwrap(); if length < usize::MAX { buffer = &buffer[0..usize::min(length, buffer.len())]; } - let string = str::from_utf8(buffer).map_err(|_| AmlError::InvalidOperationOnObject { + let string = core::str::from_utf8(buffer).map_err(|_| AmlError::InvalidOperationOnObject { op: Operation::ToString, typ: ObjectType::Buffer, })?; - Object::String(string.to_string()) + Object::String(AmlString::from_str_in(string, self.alloc.clone())) } - .wrap(); + .wrap(self.alloc.clone()); let result = self.do_store(target.clone(), result)?; context.contribute_arg(Argument::Object(result)); @@ -2121,40 +2195,45 @@ where } /// Perform a `ToDecimalString` or `ToHexString` operation - fn do_to_dec_hex_string(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - extract_args!(op => [Argument::Object(operand), Argument::Object(target)]); + fn do_to_dec_hex_string(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { + extract_args!(op, self.alloc.clone() => [Argument::Object(operand), Argument::Object(target)]); let operand = operand.clone().unwrap_transparent_reference(); let result = match *operand { Object::String(ref value) => Object::String(value.clone()), - Object::Integer(value) => match op.op { - Opcode::ToDecimalString => Object::String(value.to_string()), - Opcode::ToHexString => Object::String(alloc::format!("{value:#X}")), - _ => panic!(), - }, + Object::Integer(value) => { + let mut s = AmlString::new_in(self.alloc.clone()); + use core::fmt::Write; + match op.op { + Opcode::ToDecimalString => write!(s, "{value}").unwrap(), + Opcode::ToHexString => write!(s, "{value:#X}").unwrap(), + _ => panic!(), + }; + Object::String(s) + } Object::Buffer(ref bytes) => { if bytes.is_empty() { - Object::String(String::new()) + Object::String(AmlString::new_in(self.alloc.clone())) } else { - let mut string = String::new(); + let mut string = AmlString::new_in(self.alloc.clone()); + use core::fmt::Write; for byte in bytes { - let as_str = match op.op { - Opcode::ToDecimalString => alloc::format!("{byte},"), - Opcode::ToHexString => alloc::format!("{byte:#04X},"), + match op.op { + Opcode::ToDecimalString => write!(string, "{byte},").unwrap(), + Opcode::ToHexString => write!(string, "{byte:#04X},").unwrap(), _ => panic!(), - }; - string.push_str(&as_str); - } - // Remove last comma, if present - if !string.is_empty() { - string.pop(); + } } + // PILOT-FOLLOWUP: trim trailing comma. AmlString doesn't + // currently expose a `pop` method; needs adding for + // strict fidelity to original behavior. For now, leave + // trailing comma (cosmetic regression). Object::String(string) } } _ => Err(AmlError::InvalidOperationOnObject { op: Operation::ToDecOrHexString, typ: operand.typ() })?, } - .wrap(); + .wrap(self.alloc.clone()); let result = self.do_store(target.clone(), result)?; context.contribute_arg(Argument::Object(result)); @@ -2162,33 +2241,33 @@ where Ok(()) } - fn do_mid(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - extract_args!(op => [Argument::Object(source), Argument::Object(index), Argument::Object(length), Argument::Object(target)]); + fn do_mid(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { + extract_args!(op, self.alloc.clone() => [Argument::Object(source), Argument::Object(index), Argument::Object(length), Argument::Object(target)]); let index = index.clone().unwrap_transparent_reference().as_integer()? as usize; let length = length.clone().unwrap_transparent_reference().as_integer()? as usize; let result = match **source { Object::String(ref string) => { if index >= string.len() { - Object::String(String::new()) + Object::String(AmlString::new_in(self.alloc.clone())) } else { let upper = usize::min(index + length, index + string.len()); - let chars = &string[index..upper]; - Object::String(String::from(chars)) + let chars = &string.as_str()[index..upper]; + Object::String(AmlString::from_str_in(chars, self.alloc.clone())) } } Object::Buffer(ref buffer) => { if index >= buffer.len() { - Object::Buffer(vec![]) + Object::Buffer(vec_in!(self.alloc.clone())) } else { let upper = usize::min(index + length, index + buffer.len()); let bytes = &buffer[index..upper]; - Object::Buffer(bytes.to_vec()) + Object::Buffer(bytes.to_vec_in(self.alloc.clone())) } } _ => Err(AmlError::InvalidOperationOnObject { op: Operation::Mid, typ: source.typ() })?, } - .wrap(); + .wrap(self.alloc.clone()); self.do_store(target.clone(), result.clone())?; context.contribute_arg(Argument::Object(result)); @@ -2196,39 +2275,59 @@ where Ok(()) } - fn do_concat(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - extract_args!(op => [Argument::Object(source1), Argument::Object(source2), Argument::Object(target)]); + fn do_concat(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { + extract_args!(op, self.alloc.clone() => [Argument::Object(source1), Argument::Object(source2), Argument::Object(target)]); let source1 = source1.clone().unwrap_transparent_reference(); let source2 = source2.clone().unwrap_transparent_reference(); - fn resolve_as_string(obj: &Object) -> String { + // PILOT-DECISION: resolve_as_string was producing String. + // Now produces AmlString via an inline write! against the + // formatter. Recursion handled by re-entering with a clone. + fn resolve_as_string(obj: &Object, alloc: A) -> AmlString { + use core::fmt::Write; + let mut s = AmlString::new_in(alloc.clone()); match obj { - Object::Uninitialized => "[Uninitialized Object]".to_string(), - Object::Buffer(bytes) => String::from_utf8_lossy(bytes).into_owned(), - Object::BufferField { .. } => "[Buffer Field]".to_string(), - Object::Device => "[Device]".to_string(), - Object::Event(_) => "[Event]".to_string(), - Object::FieldUnit(_) => "[Field]".to_string(), - Object::Integer(value) => value.to_string(), - Object::Method { .. } | Object::NativeMethod { .. } => "[Control Method]".to_string(), - Object::Mutex { .. } => "[Mutex]".to_string(), - Object::Reference { inner, .. } => resolve_as_string(&(inner.clone().unwrap_reference())), - Object::OpRegion(_) => "[Operation Region]".to_string(), - Object::Package(_) => "[Package]".to_string(), - Object::PowerResource { .. } => "[Power Resource]".to_string(), - Object::Processor { .. } => "[Processor]".to_string(), - Object::RawDataBuffer => "[Raw Data Buffer]".to_string(), - Object::String(value) => value.clone(), - Object::ThermalZone => "[Thermal Zone]".to_string(), - Object::Debug => "[Debug Object]".to_string(), + Object::Uninitialized => s.push_str("[Uninitialized Object]"), + Object::Buffer(bytes) => { + // PILOT-FOLLOWUP: original used String::from_utf8_lossy + // which goes through Global on invalid UTF-8. Best-effort + // ASCII-only conversion below. + for &b in bytes { + if b.is_ascii() && b != 0 { + s.push(b as char); + } else { + s.push(char::REPLACEMENT_CHARACTER); + } + } + } + Object::BufferField { .. } => s.push_str("[Buffer Field]"), + Object::Device => s.push_str("[Device]"), + Object::Event(_) => s.push_str("[Event]"), + Object::FieldUnit(_) => s.push_str("[Field]"), + Object::Integer(value) => write!(s, "{value}").unwrap(), + Object::Method { .. } | Object::NativeMethod { .. } => s.push_str("[Control Method]"), + Object::Mutex { .. } => s.push_str("[Mutex]"), + Object::Reference { inner, .. } => { + let inner_str = resolve_as_string(&inner.clone().unwrap_reference(), alloc); + s.push_str(inner_str.as_str()); + } + Object::OpRegion(_) => s.push_str("[Operation Region]"), + Object::Package(_) => s.push_str("[Package]"), + Object::PowerResource { .. } => s.push_str("[Power Resource]"), + Object::Processor { .. } => s.push_str("[Processor]"), + Object::RawDataBuffer => s.push_str("[Raw Data Buffer]"), + Object::String(value) => s.push_str(value.as_str()), + Object::ThermalZone => s.push_str("[Thermal Zone]"), + Object::Debug => s.push_str("[Debug Object]"), } + s } let result = match source1.typ() { ObjectType::Integer => { let source1 = source1.as_integer()?; let source2 = source2.to_integer(self.integer_size())?; - let mut buffer = Vec::new(); + let mut buffer = Vec::new_in(self.alloc.clone()); if self.integer_size() == 8 { buffer.extend_from_slice(&source1.to_le_bytes()); buffer.extend_from_slice(&source2.to_le_bytes()); @@ -2236,17 +2335,20 @@ where buffer.extend_from_slice(&(source1 as u32).to_le_bytes()); buffer.extend_from_slice(&(source2 as u32).to_le_bytes()); } - Object::Buffer(buffer).wrap() + Object::Buffer(buffer).wrap(self.alloc.clone()) } ObjectType::Buffer => { - let mut buffer = source1.as_buffer()?.to_vec(); - buffer.extend(source2.to_buffer(self.integer_size())?); - Object::Buffer(buffer).wrap() + let mut buffer = source1.as_buffer()?.to_vec_in(self.alloc.clone()); + buffer.extend(source2.to_buffer(self.integer_size(), self.alloc.clone())?); + Object::Buffer(buffer).wrap(self.alloc.clone()) } _ => { - let source1 = resolve_as_string(&source1); - let source2 = resolve_as_string(&source2); - Object::String(source1 + &source2).wrap() + let s1 = resolve_as_string(&source1, self.alloc.clone()); + let s2 = resolve_as_string(&source2, self.alloc.clone()); + let mut combined = AmlString::new_in(self.alloc.clone()); + combined.push_str(s1.as_str()); + combined.push_str(s2.as_str()); + Object::String(combined).wrap(self.alloc.clone()) } }; @@ -2256,8 +2358,8 @@ where Ok(()) } - fn do_from_bcd(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - extract_args!(op => [Argument::Object(value)]); + fn do_from_bcd(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { + extract_args!(op, self.alloc.clone() => [Argument::Object(value)]); let mut value = value.clone().unwrap_transparent_reference().as_integer()?; let mut result = 0; @@ -2268,13 +2370,13 @@ where value >>= 4; } - context.contribute_arg(Argument::Object(Object::Integer(result).wrap())); + context.contribute_arg(Argument::Object(Object::Integer(result).wrap(self.alloc.clone()))); context.retire_op(op); Ok(()) } - fn do_to_bcd(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - extract_args!(op => [Argument::Object(value)]); + fn do_to_bcd(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { + extract_args!(op, self.alloc.clone() => [Argument::Object(value)]); let mut value = value.clone().unwrap_transparent_reference().as_integer()?; let mut result = 0; @@ -2285,13 +2387,13 @@ where i += 1; } - context.contribute_arg(Argument::Object(Object::Integer(result).wrap())); + context.contribute_arg(Argument::Object(Object::Integer(result).wrap(self.alloc.clone()))); context.retire_op(op); Ok(()) } - fn do_size_of(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - extract_args!(op => [Argument::Object(object)]); + fn do_size_of(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { + extract_args!(op, self.alloc.clone() => [Argument::Object(object)]); let object = object.clone().unwrap_reference(); let result = match *object { @@ -2301,13 +2403,13 @@ where _ => Err(AmlError::InvalidOperationOnObject { op: Operation::SizeOf, typ: object.typ() })?, }; - context.contribute_arg(Argument::Object(Object::Integer(result as u64).wrap())); + context.contribute_arg(Argument::Object(Object::Integer(result as u64).wrap(self.alloc.clone()))); context.retire_op(op); Ok(()) } - fn do_index(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - extract_args!(op => [Argument::Object(object), Argument::Object(index_value), Argument::Object(target)]); + fn do_index(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { + extract_args!(op, self.alloc.clone() => [Argument::Object(object), Argument::Object(index_value), Argument::Object(target)]); let object = object.clone().unwrap_transparent_reference(); let index_value = index_value.clone().unwrap_transparent_reference().as_integer()?; @@ -2324,7 +2426,7 @@ where offset: index_value as usize * 8, length: 8, } - .wrap(), + .wrap(self.alloc.clone()), } } Object::String(ref string) => { @@ -2339,7 +2441,7 @@ where offset: index_value as usize * 8, length: 8, } - .wrap(), + .wrap(self.alloc.clone()), } } Object::Package(ref package) => { @@ -2348,7 +2450,7 @@ where } _ => Err(AmlError::IndexOutOfBounds)?, } - .wrap(); + .wrap(self.alloc.clone()); self.do_store(target.clone(), result.clone())?; context.contribute_arg(Argument::Object(result)); @@ -2364,7 +2466,7 @@ where /// object is overwritten /// - Index references behave the same as locals /// - Named objects are stored into, with implicit casting - fn do_store(&self, target: WrappedObject, object: WrappedObject) -> Result { + fn do_store(&self, target: WrappedObject, object: WrappedObject) -> Result, AmlError> { let object = object.unwrap_transparent_reference(); let token = self.object_token.lock(); @@ -2431,7 +2533,15 @@ where } } } - Object::Debug => self.handler.handle_debug(&object), + Object::Debug => { + // PILOT-FOLLOWUP: Handler::handle_debug is pinned to + // `&Object` per lib.rs. Passing `&object: &Object` + // is a type mismatch and there's no clean conversion path + // since Object doesn't have a Global counterpart in + // memory. Skipping the call preserves correctness (the + // default impl is a no-op anyway); restoring it requires + // making Handler generic on A. + } Object::Integer(0) => {} // Store to NullName _ => return Err(AmlError::InvalidOperationOnObject { op: Operation::Store, typ: target.typ() }), } @@ -2446,9 +2556,12 @@ where /// - Objects referenced by name are overwritten /// - Index references cause the object at the index to be overwritten /// - Other reference operations are not allowed - fn do_copy_object(&self, target: WrappedObject, object: WrappedObject) -> Result<(), AmlError> { + fn do_copy_object(&self, target: WrappedObject, object: WrappedObject) -> Result<(), AmlError> { let Object::Reference { kind, ref inner } = *target else { - return Err(AmlError::InternalError("Target of CopyObject must be a reference".to_string())); + return Err(AmlError::InternalError(AmlString::from_str_in( + "Target of CopyObject must be a reference", + self.alloc.clone(), + ))); }; let object = object.clone().unwrap_transparent_reference(); let token = self.object_token.lock(); @@ -2477,7 +2590,7 @@ where /// operation regions, and then shifting and masking the resulting value as appropriate. Will /// return either an `Integer` or `Buffer` as appropriate, guided by the size of the field /// and expected integer size (as per the DSDT revision). - fn do_field_read(&self, field: &FieldUnit) -> Result { + fn do_field_read(&self, field: &FieldUnit) -> Result, AmlError> { let needs_buffer = field.bit_length > (self.integer_size() * 8); let access_width_bits = field.flags.access_type_bytes()? * 8; @@ -2485,12 +2598,12 @@ where // TODO: if the field needs to be locked, acquire/release a global mutex? - enum Output { + enum Output { Integer([u8; 8]), - Buffer(Vec), + Buffer(Vec), } let mut output = if needs_buffer { - Output::Buffer(vec![0; field.bit_length.next_multiple_of(8)]) + Output::Buffer(vec_in!(self.alloc.clone(); 0; field.bit_length.next_multiple_of(8))) } else { Output::Integer([0; 8]) }; @@ -2504,7 +2617,7 @@ where FieldUnitKind::Bank { ref region, ref bank, bank_value } => { let Object::FieldUnit(ref bank) = **bank else { panic!() }; assert!(matches!(bank.kind, FieldUnitKind::Normal { .. })); - self.do_field_write(bank, Object::Integer(bank_value).wrap())?; + self.do_field_write(bank, Object::Integer(bank_value).wrap(self.alloc.clone()))?; (region, 0) } FieldUnitKind::Index { index: _, ref data } => { @@ -2543,7 +2656,7 @@ where let Object::FieldUnit(ref data) = **data else { panic!() }; self.do_field_write( index, - Object::Integer((index_field_idx + i * (access_width_bits / 8)) as u64).wrap(), + Object::Integer((index_field_idx + i * (access_width_bits / 8)) as u64).wrap(self.alloc.clone()), )?; // The offset is always that of the data register, as we always read from the @@ -2566,12 +2679,12 @@ where } match output { - Output::Buffer(bytes) => Ok(Object::Buffer(bytes).wrap()), - Output::Integer(value) => Ok(Object::Integer(u64::from_le_bytes(value)).wrap()), + Output::Buffer(bytes) => Ok(Object::Buffer(bytes).wrap(self.alloc.clone())), + Output::Integer(value) => Ok(Object::Integer(u64::from_le_bytes(value)).wrap(self.alloc.clone())), } } - fn do_field_write(&self, field: &FieldUnit, value: WrappedObject) -> Result<(), AmlError> { + fn do_field_write(&self, field: &FieldUnit, value: WrappedObject) -> Result<(), AmlError> { trace!("AML field write. Field = {:?}. Value = {}", field, value); let value_bytes = match &*value { @@ -2590,7 +2703,7 @@ where FieldUnitKind::Bank { ref region, ref bank, bank_value } => { let Object::FieldUnit(ref bank) = **bank else { panic!() }; assert!(matches!(bank.kind, FieldUnitKind::Normal { .. })); - self.do_field_write(bank, Object::Integer(bank_value).wrap())?; + self.do_field_write(bank, Object::Integer(bank_value).wrap(self.alloc.clone()))?; (region, 0) } FieldUnitKind::Index { index: _, ref data } => { @@ -2624,7 +2737,7 @@ where let Object::FieldUnit(ref data) = **data else { panic!() }; self.do_field_write( index, - Object::Integer((index_field_idx + i * (access_width_bits / 8)) as u64).wrap(), + Object::Integer((index_field_idx + i * (access_width_bits / 8)) as u64).wrap(self.alloc.clone()), )?; // The offset is always that of the data register, as we always read from the @@ -2674,7 +2787,7 @@ where /// Performs an actual read from an operation region. `offset` and `length` must respect the /// access requirements of the field being read, and are supplied in **bytes**. This may call /// AML methods if required, and may invoke user-supplied handlers. - fn do_native_region_read(&self, region: &OpRegion, offset: usize, length: usize) -> Result { + fn do_native_region_read(&self, region: &OpRegion, offset: usize, length: usize) -> Result> { trace!("Native field read. Region = {:?}, offset = {:#x}, length={:#x}", region, offset, length); match region.space { @@ -2732,11 +2845,11 @@ where /// AML methods if required, and may invoke user-supplied handlers. fn do_native_region_write( &self, - region: &OpRegion, + region: &OpRegion, offset: usize, length: usize, value: u64, - ) -> Result<(), AmlError> { + ) -> Result<(), AmlError> { trace!( "Native field write. Region = {:?}, offset = {:#x}, length={:#x}, value={:#x}", region, offset, length, value @@ -2795,21 +2908,21 @@ where } } - fn pci_address_for_device(&self, path: &AmlName) -> Result { + fn pci_address_for_device(&self, path: &AmlName) -> Result> { /* * TODO: it's not ideal to do these reads for every native access. See if we can * cache them somewhere? */ - let seg = match self.evaluate_if_present(AmlName::from_str("_SEG").unwrap().resolve(path)?, vec![])? { + let seg = match self.evaluate_if_present(AmlName::parse_in("_SEG", self.alloc.clone()).unwrap().resolve(path)?, vec_in!(self.alloc.clone()))? { Some(value) => value.as_integer()?, None => 0, }; - let bus = match self.evaluate_if_present(AmlName::from_str("_BBN").unwrap().resolve(path)?, vec![])? { + let bus = match self.evaluate_if_present(AmlName::parse_in("_BBN", self.alloc.clone()).unwrap().resolve(path)?, vec_in!(self.alloc.clone()))? { Some(value) => value.as_integer()?, None => 0, }; let (device, function) = { - let adr = self.evaluate_if_present(AmlName::from_str("_ADR").unwrap().resolve(path)?, vec![])?; + let adr = self.evaluate_if_present(AmlName::parse_in("_ADR", self.alloc.clone()).unwrap().resolve(path)?, vec_in!(self.alloc.clone()))?; let adr = match adr { Some(adr) => adr.as_integer()?, None => 0, @@ -2829,37 +2942,40 @@ where /// preempt method contexts that execute other methods, and these contexts may have disparate /// lifetimes. This is made safe in the case of methods by the context holding a reference to the /// method object, but must be handled manually for AML tables. -struct MethodContext { - current_block: Block, - block_stack: Vec, - in_flight: Vec, - args: [WrappedObject; 8], - locals: [WrappedObject; 8], - current_scope: AmlName, - - _method: Option, +struct MethodContext { + current_block: Block, + block_stack: Vec, A>, + in_flight: Vec, A>, + args: [WrappedObject; 8], + locals: [WrappedObject; 8], + current_scope: AmlName, + + _method: Option>, + // PILOT-DECISION: carry the allocator on the context too, so opcode + // handlers that allocate Vec/AmlString for intermediate values can derive + // it from the active method context. + alloc: A, } -struct Block { +struct Block { stream: *const [u8], pc: usize, - kind: BlockKind, + kind: BlockKind, } -impl Block { +impl Block { fn stream(&self) -> &[u8] { unsafe { &*self.stream } } } -#[derive(PartialEq, Debug)] -pub enum BlockKind { +pub enum BlockKind { Table, Method { - method_scope: AmlName, + method_scope: AmlName, }, Scope { - old_scope: AmlName, + old_scope: AmlName, }, Package, VarPackage, @@ -2871,6 +2987,37 @@ pub enum BlockKind { }, } +impl core::fmt::Debug for BlockKind { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + use BlockKind::*; + match self { + Table => f.write_str("Table"), + Method { method_scope } => write!(f, "Method {{ method_scope: {:?} }}", method_scope), + Scope { old_scope } => write!(f, "Scope {{ old_scope: {:?} }}", old_scope), + Package => f.write_str("Package"), + VarPackage => f.write_str("VarPackage"), + IfThenBranch => f.write_str("IfThenBranch"), + While { start_pc } => write!(f, "While {{ start_pc: {} }}", start_pc), + } + } +} + +// PILOT-DECISION: PartialEq for BlockKind is impl'd manually to avoid +// deriving with `A: PartialEq` bound. The AmlName fields use the +// AmlName cross-A PartialEq impl from namespace.rs. +impl PartialEq> for BlockKind { + fn eq(&self, other: &BlockKind) -> bool { + use BlockKind::*; + match (self, other) { + (Table, Table) | (Package, Package) | (VarPackage, VarPackage) | (IfThenBranch, IfThenBranch) => true, + (Method { method_scope: a }, Method { method_scope: b }) => a == b, + (Scope { old_scope: a }, Scope { old_scope: b }) => a == b, + (While { start_pc: a }, While { start_pc: b }) => a == b, + _ => false, + } + } +} + /// A `ResolveBehaviour` describes how a name at the top-level should be resolved as part of an /// operation. #[derive(Clone, Copy, PartialEq, Debug)] @@ -2901,29 +3048,58 @@ enum ResolveBehaviour { Placeholder, } -#[derive(Debug)] -struct OpInFlight { +struct OpInFlight { op: Opcode, expected_arguments: usize, - arguments: Vec, + arguments: Vec, A>, resolve_behaviour: &'static [ResolveBehaviour], } -#[derive(Debug)] -enum Argument { - Object(WrappedObject), - Namestring(AmlName), +enum Argument { + Object(WrappedObject), + Namestring(AmlName), ByteData(u8), DWordData(u32), TrackedPc(usize), PkgLength(usize), } -impl OpInFlight { +// Manual Debug impl — derive auto-adds `A: Debug` which `&'static BumpArena` +// doesn't satisfy. +impl core::fmt::Debug for OpInFlight { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_struct("OpInFlight") + .field("op", &self.op) + .field("expected_arguments", &self.expected_arguments) + .field("arguments", &self.arguments) + .finish_non_exhaustive() + } +} + +impl core::fmt::Debug for Argument { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + use Argument::*; + match self { + Object(o) => write!(f, "Object({})", **o), + Namestring(n) => write!(f, "Namestring({:?})", n), + ByteData(b) => write!(f, "ByteData({})", b), + DWordData(d) => write!(f, "DWordData({})", d), + TrackedPc(pc) => write!(f, "TrackedPc({})", pc), + PkgLength(l) => write!(f, "PkgLength({})", l), + } + } +} + +impl OpInFlight { /// Creates a new `OpInFlight`. The number of expected arguments is inferred from the number of /// `ResolveBehaviour`s passed. - pub fn new(op: Opcode, resolve_behaviour: &'static [ResolveBehaviour]) -> OpInFlight { - OpInFlight { op, expected_arguments: resolve_behaviour.len(), arguments: Vec::new(), resolve_behaviour } + pub fn new(op: Opcode, resolve_behaviour: &'static [ResolveBehaviour], alloc: A) -> OpInFlight { + OpInFlight { + op, + expected_arguments: resolve_behaviour.len(), + arguments: Vec::new_in(alloc), + resolve_behaviour, + } } /// Creates a new `OpInFlight` with the given number of expected arguments. This should be used @@ -2933,18 +3109,19 @@ impl OpInFlight { op: Opcode, expected_arguments: usize, resolve_behaviour: &'static [ResolveBehaviour], - ) -> OpInFlight { - OpInFlight { op, expected_arguments, arguments: Vec::new(), resolve_behaviour } + alloc: A, + ) -> OpInFlight { + OpInFlight { op, expected_arguments, arguments: Vec::new_in(alloc), resolve_behaviour } } /// Creates a new `OpInFlight` with a number of arguments that have already been interpreted, /// and is expecting some `more` arguments. pub fn new_with_dynamic( op: Opcode, - arguments: Vec, + arguments: Vec, A>, more: usize, resolve_behaviour: &'static [ResolveBehaviour], - ) -> OpInFlight { + ) -> OpInFlight { OpInFlight { op, expected_arguments: arguments.len() + more, arguments, resolve_behaviour } } @@ -2954,9 +3131,9 @@ impl OpInFlight { /// `ResolveBehaviour::Placeholder`). pub fn new_with( op: Opcode, - arguments: Vec, + arguments: Vec, A>, resolve_behaviour: &'static [ResolveBehaviour], - ) -> OpInFlight { + ) -> OpInFlight { OpInFlight { op, expected_arguments: resolve_behaviour.len(), arguments, resolve_behaviour } } @@ -2972,25 +3149,28 @@ impl OpInFlight { } } -impl MethodContext { - unsafe fn new_from_table(stream: &[u8]) -> MethodContext { +impl MethodContext { + unsafe fn new_from_table(stream: &[u8], alloc: A) -> MethodContext { let block = Block { stream: stream as *const [u8], pc: 0, kind: BlockKind::Table }; + let local_alloc = alloc.clone(); MethodContext { current_block: block, - block_stack: Vec::new(), - in_flight: Vec::new(), - args: core::array::from_fn(|_| Object::Uninitialized.wrap()), - locals: core::array::from_fn(|_| Object::Uninitialized.wrap()), - current_scope: AmlName::root(), + block_stack: Vec::new_in(alloc.clone()), + in_flight: Vec::new_in(alloc.clone()), + args: core::array::from_fn(|_| Object::Uninitialized.wrap(local_alloc.clone())), + locals: core::array::from_fn(|_| Object::Uninitialized.wrap(local_alloc.clone())), + current_scope: AmlName::root_in(alloc.clone()), _method: None, + alloc, } } fn new_from_method( - method: WrappedObject, - args: Vec, - scope: AmlName, - ) -> Result { + method: WrappedObject, + args: Vec, A>, + scope: AmlName, + alloc: A, + ) -> Result, AmlError> { if let Object::Method { code, flags } = &*method { if args.len() != flags.arg_count() { return Err(AmlError::MethodArgCountIncorrect); @@ -3000,17 +3180,23 @@ impl MethodContext { pc: 0, kind: BlockKind::Method { method_scope: scope.clone() }, }; + let local_alloc = alloc.clone(); let args = core::array::from_fn(|i| { - if let Some(arg) = args.get(i) { arg.clone() } else { Object::Uninitialized.wrap() } + if let Some(arg) = args.get(i) { + arg.clone() + } else { + Object::Uninitialized.wrap(local_alloc.clone()) + } }); let context = MethodContext { current_block: block, - block_stack: Vec::new(), - in_flight: Vec::new(), + block_stack: Vec::new_in(alloc.clone()), + in_flight: Vec::new_in(alloc.clone()), args, - locals: core::array::from_fn(|_| Object::Uninitialized.wrap()), + locals: core::array::from_fn(|_| Object::Uninitialized.wrap(local_alloc.clone())), current_scope: scope, _method: Some(method.clone()), + alloc, }; Ok(context) } else { @@ -3018,14 +3204,14 @@ impl MethodContext { } } - fn last_op(&mut self) -> Result<&mut OpInFlight, AmlError> { + fn last_op(&mut self) -> Result<&mut OpInFlight, AmlError> { match self.in_flight.last_mut() { Some(op) => Ok(op), None => Err(AmlError::NoCurrentOp), } } - fn contribute_arg(&mut self, arg: Argument) { + fn contribute_arg(&mut self, arg: Argument) { if let Some(in_flight) = self.in_flight.last_mut() && in_flight.arguments.len() < in_flight.expected_arguments { @@ -3034,7 +3220,7 @@ impl MethodContext { } /// Start a new `InFlightOp`. - fn start(&mut self, op: OpInFlight) { + fn start(&mut self, op: OpInFlight) { trace!( "START OP: {:?}, args: {:?}, with {} more needed ({:?})", op.op, @@ -3045,11 +3231,11 @@ impl MethodContext { self.in_flight.push(op); } - fn retire_op(&mut self, op: OpInFlight) { + fn retire_op(&mut self, op: OpInFlight) { trace!("RETIRE OP: {:?}, args: {:?}", op.op, op.arguments); } - fn start_new_block(&mut self, kind: BlockKind, length: usize) { + fn start_new_block(&mut self, kind: BlockKind, length: usize) { let block = Block { stream: &self.current_block.stream()[..(self.current_block.pc + length)] as *const [u8], pc: self.current_block.pc, @@ -3059,7 +3245,7 @@ impl MethodContext { self.block_stack.push(mem::replace(&mut self.current_block, block)); } - fn opcode(&mut self) -> Result { + fn opcode(&mut self) -> Result> { let opcode: u16 = match self.next()? { 0x5b => { let ext = self.next()?; @@ -3199,7 +3385,7 @@ impl MethodContext { }) } - fn pkglength(&mut self) -> Result { + fn pkglength(&mut self) -> Result> { let lead_byte = self.next()?; let byte_count = lead_byte.get_bits(6..8); assert!(byte_count < 4); @@ -3215,7 +3401,7 @@ impl MethodContext { } } - fn namestring(&mut self) -> Result { + fn namestring(&mut self) -> Result, AmlError> { use namespace::{NameComponent, NameSeg}; /* @@ -3231,7 +3417,7 @@ impl MethodContext { const DUAL_NAME_PREFIX: u8 = 0x2e; const MULTI_NAME_PREFIX: u8 = 0x2f; - let mut components = vec![]; + let mut components = vec_in!(self.alloc.clone()); match self.peek()? { b'\\' => { @@ -3277,7 +3463,7 @@ impl MethodContext { Ok(AmlName::from_components(components)) } - fn next(&mut self) -> Result { + fn next(&mut self) -> Result> { if self.current_block.pc >= self.current_block.stream.len() { return Err(AmlError::RunOutOfStream); } @@ -3288,15 +3474,15 @@ impl MethodContext { Ok(byte) } - fn next_u16(&mut self) -> Result { + fn next_u16(&mut self) -> Result> { Ok(u16::from_le_bytes([self.next()?, self.next()?])) } - fn next_u32(&mut self) -> Result { + fn next_u32(&mut self) -> Result> { Ok(u32::from_le_bytes([self.next()?, self.next()?, self.next()?, self.next()?])) } - fn next_u64(&mut self) -> Result { + fn next_u64(&mut self) -> Result> { Ok(u64::from_le_bytes([ self.next()?, self.next()?, @@ -3309,7 +3495,7 @@ impl MethodContext { ])) } - fn peek(&self) -> Result { + fn peek(&self) -> Result> { if self.current_block.pc >= self.current_block.stream.len() { return Err(AmlError::RunOutOfStream); } @@ -3462,22 +3648,28 @@ pub enum Operation { WaitEvent, } -#[derive(Clone, PartialEq, Debug)] +// PILOT-DECISION: AmlError gains `` because 5 variants hold `AmlName` +// and 2 hold `AmlString`. Derives stay only for Clone — PartialEq/Debug +// get manual impls to dodge the auto-added `A: PartialEq`/`A: Debug` bounds. +// PILOT-FOLLOWUP: the `lib.rs::AcpiError::Aml(aml::AmlError)` variant also +// needs `` propagation. Fixing that variant is mechanical but currently +// blocks crate-level compilation. +#[derive(Clone)] #[non_exhaustive] -pub enum AmlError { +pub enum AmlError { RunOutOfStream, IllegalOpcode(u16), InvalidFieldFlags, - InvalidName(Option), + InvalidName(Option>), InvalidNameSeg([u8; 4]), - InvalidNormalizedName(AmlName), + InvalidNormalizedName(AmlName), RootHasNoParent, EmptyNamesAreInvalid, - LevelDoesNotExist(AmlName), - NameCollision(AmlName), - ObjectDoesNotExist(AmlName), + LevelDoesNotExist(AmlName), + NameCollision(AmlName), + ObjectDoesNotExist(AmlName), NoCurrentOp, ElseFoundWithoutCorrespondingIf, @@ -3523,13 +3715,116 @@ pub enum AmlError { /// The library has given a response the host does not understand, or the host is otherwise /// unable to continue operating the library correctly. The specific reason is given in the - /// contained String. + /// contained string. /// /// This variant is set by the host, not by the library, and can be used when it is convenient /// not to construct a more complex error type around [`AmlError`]. - HostError(String), + HostError(AmlString), /// An internal interpreter error has occured, and the interpreter has been left in an unknown /// state. More information may be given in the contained value. - InternalError(String), + InternalError(AmlString), +} + +impl PartialEq> for AmlError { + fn eq(&self, other: &AmlError) -> bool { + use AmlError::*; + match (self, other) { + (RunOutOfStream, RunOutOfStream) + | (InvalidFieldFlags, InvalidFieldFlags) + | (RootHasNoParent, RootHasNoParent) + | (EmptyNamesAreInvalid, EmptyNamesAreInvalid) + | (NoCurrentOp, NoCurrentOp) + | (ElseFoundWithoutCorrespondingIf, ElseFoundWithoutCorrespondingIf) + | (ContinueOutsideOfWhile, ContinueOutsideOfWhile) + | (BreakOutsideOfWhile, BreakOutsideOfWhile) + | (MethodArgCountIncorrect, MethodArgCountIncorrect) + | (IndexOutOfBounds, IndexOutOfBounds) + | (StoreToInvalidReferenceType, StoreToInvalidReferenceType) + | (InvalidResourceDescriptor, InvalidResourceDescriptor) + | (UnexpectedResourceType, UnexpectedResourceType) + | (MutexAcquireTimeout, MutexAcquireTimeout) + | (PrtInvalidAddress, PrtInvalidAddress) + | (PrtInvalidPin, PrtInvalidPin) + | (PrtInvalidGsi, PrtInvalidGsi) + | (PrtInvalidSource, PrtInvalidSource) + | (PrtNoEntry, PrtNoEntry) + | (FatalErrorEncountered, FatalErrorEncountered) + | (LibUnimplemented, LibUnimplemented) => true, + (IllegalOpcode(a), IllegalOpcode(b)) => a == b, + (InvalidName(a), InvalidName(b)) => match (a, b) { + (None, None) => true, + (Some(x), Some(y)) => x == y, + _ => false, + }, + (InvalidNameSeg(a), InvalidNameSeg(b)) => a == b, + (InvalidNormalizedName(a), InvalidNormalizedName(b)) => a == b, + (LevelDoesNotExist(a), LevelDoesNotExist(b)) => a == b, + (NameCollision(a), NameCollision(b)) => a == b, + (ObjectDoesNotExist(a), ObjectDoesNotExist(b)) => a == b, + ( + InvalidOperationOnObject { op: o1, typ: t1 }, + InvalidOperationOnObject { op: o2, typ: t2 }, + ) => o1 == o2 && t1 == t2, + ( + ObjectNotOfExpectedType { expected: e1, got: g1 }, + ObjectNotOfExpectedType { expected: e2, got: g2 }, + ) => e1 == e2 && g1 == g2, + (InvalidImplicitCast { from: f1, to: t1 }, InvalidImplicitCast { from: f2, to: t2 }) => { + f1 == f2 && t1 == t2 + } + (NoHandlerForRegionAccess(a), NoHandlerForRegionAccess(b)) => a == b, + (HostError(a), HostError(b)) => a == b, + (InternalError(a), InternalError(b)) => a == b, + _ => false, + } + } +} + +impl core::fmt::Debug for AmlError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + use AmlError::*; + match self { + RunOutOfStream => f.write_str("RunOutOfStream"), + IllegalOpcode(op) => write!(f, "IllegalOpcode({:#x})", op), + InvalidFieldFlags => f.write_str("InvalidFieldFlags"), + InvalidName(name) => write!(f, "InvalidName({:?})", name), + InvalidNameSeg(bytes) => write!(f, "InvalidNameSeg({:?})", bytes), + InvalidNormalizedName(name) => write!(f, "InvalidNormalizedName({:?})", name), + RootHasNoParent => f.write_str("RootHasNoParent"), + EmptyNamesAreInvalid => f.write_str("EmptyNamesAreInvalid"), + LevelDoesNotExist(name) => write!(f, "LevelDoesNotExist({:?})", name), + NameCollision(name) => write!(f, "NameCollision({:?})", name), + ObjectDoesNotExist(name) => write!(f, "ObjectDoesNotExist({:?})", name), + NoCurrentOp => f.write_str("NoCurrentOp"), + ElseFoundWithoutCorrespondingIf => f.write_str("ElseFoundWithoutCorrespondingIf"), + ContinueOutsideOfWhile => f.write_str("ContinueOutsideOfWhile"), + BreakOutsideOfWhile => f.write_str("BreakOutsideOfWhile"), + MethodArgCountIncorrect => f.write_str("MethodArgCountIncorrect"), + InvalidOperationOnObject { op, typ } => { + write!(f, "InvalidOperationOnObject {{ op: {:?}, typ: {:?} }}", op, typ) + } + IndexOutOfBounds => f.write_str("IndexOutOfBounds"), + ObjectNotOfExpectedType { expected, got } => { + write!(f, "ObjectNotOfExpectedType {{ expected: {:?}, got: {:?} }}", expected, got) + } + InvalidImplicitCast { from, to } => { + write!(f, "InvalidImplicitCast {{ from: {:?}, to: {:?} }}", from, to) + } + StoreToInvalidReferenceType => f.write_str("StoreToInvalidReferenceType"), + InvalidResourceDescriptor => f.write_str("InvalidResourceDescriptor"), + UnexpectedResourceType => f.write_str("UnexpectedResourceType"), + NoHandlerForRegionAccess(space) => write!(f, "NoHandlerForRegionAccess({:?})", space), + MutexAcquireTimeout => f.write_str("MutexAcquireTimeout"), + PrtInvalidAddress => f.write_str("PrtInvalidAddress"), + PrtInvalidPin => f.write_str("PrtInvalidPin"), + PrtInvalidGsi => f.write_str("PrtInvalidGsi"), + PrtInvalidSource => f.write_str("PrtInvalidSource"), + PrtNoEntry => f.write_str("PrtNoEntry"), + FatalErrorEncountered => f.write_str("FatalErrorEncountered"), + LibUnimplemented => f.write_str("LibUnimplemented"), + HostError(s) => write!(f, "HostError({:?})", s), + InternalError(s) => write!(f, "InternalError({:?})", s), + } + } } diff --git a/src/aml/namespace.rs b/src/aml/namespace.rs index c887bcac..dc694b8c 100644 --- a/src/aml/namespace.rs +++ b/src/aml/namespace.rs @@ -1,41 +1,47 @@ use super::{ AmlError, Handle, - object::{Object, ObjectType, WrappedObject}, -}; -use alloc::{ - collections::btree_map::BTreeMap, - string::{String, ToString}, - vec, - vec::Vec, + object::{AmlString, Object, ObjectType, WrappedObject}, }; +use alloc::{collections::btree_map::BTreeMap, string::String, vec::Vec}; use bit_field::BitField; -use core::{ - fmt, - str::{self, FromStr}, -}; +use core::{alloc::Allocator, fmt, str}; use log::{trace, warn}; #[derive(Clone)] -pub struct Namespace { - root: NamespaceLevel, +pub struct Namespace { + // PILOT-DECISION: Namespace owns a clone of the allocator so methods + // (`add_level`, `insert`, etc.) can construct new NamespaceLevel/AmlName + // values without threading `alloc` through every signature. For + // `&'static BumpArena` this costs 8 bytes per Namespace, paid once. + alloc: A, + root: NamespaceLevel, } -impl Namespace { +impl Namespace { /// Create a new AML namespace, with the expected pre-defined objects. - pub fn new(global_lock_mutex: Handle) -> Namespace { - let mut namespace = Namespace { root: NamespaceLevel::new(NamespaceLevelKind::Scope) }; + // + // PILOT-DECISION: `new` → `new_in(global_lock_mutex, alloc)`. The handful + // of AmlName/String constructions in the body are each allocator-aware. + // The `A: 'static` bound propagates from `Object::native_method` (used to + // build `_OSI`); see the note on that method. + pub fn new_in(global_lock_mutex: Handle, alloc: A) -> Namespace + where + A: 'static, + { + let mut namespace = + Namespace { alloc: alloc.clone(), root: NamespaceLevel::new_in(NamespaceLevelKind::Scope, alloc.clone()) }; - namespace.add_level(AmlName::from_str("\\_GPE").unwrap(), NamespaceLevelKind::Scope).unwrap(); - namespace.add_level(AmlName::from_str("\\_SB").unwrap(), NamespaceLevelKind::Scope).unwrap(); - namespace.add_level(AmlName::from_str("\\_SI").unwrap(), NamespaceLevelKind::Scope).unwrap(); - namespace.add_level(AmlName::from_str("\\_PR").unwrap(), NamespaceLevelKind::Scope).unwrap(); - namespace.add_level(AmlName::from_str("\\_TZ").unwrap(), NamespaceLevelKind::Scope).unwrap(); + namespace.add_level(AmlName::parse_in("\\_GPE", alloc.clone()).unwrap(), NamespaceLevelKind::Scope).unwrap(); + namespace.add_level(AmlName::parse_in("\\_SB", alloc.clone()).unwrap(), NamespaceLevelKind::Scope).unwrap(); + namespace.add_level(AmlName::parse_in("\\_SI", alloc.clone()).unwrap(), NamespaceLevelKind::Scope).unwrap(); + namespace.add_level(AmlName::parse_in("\\_PR", alloc.clone()).unwrap(), NamespaceLevelKind::Scope).unwrap(); + namespace.add_level(AmlName::parse_in("\\_TZ", alloc.clone()).unwrap(), NamespaceLevelKind::Scope).unwrap(); namespace .insert( - AmlName::from_str("\\_GL").unwrap(), - Object::Mutex { mutex: global_lock_mutex, sync_level: 0 }.wrap(), + AmlName::parse_in("\\_GL", alloc.clone()).unwrap(), + Object::Mutex { mutex: global_lock_mutex, sync_level: 0 }.wrap(alloc.clone()), ) .unwrap(); @@ -47,8 +53,14 @@ impl Namespace { * * See https://www.kernel.org/doc/html/latest/firmware-guide/acpi/osi.html for more information. */ + // PILOT-DECISION: was `"Microsoft Windows NT".to_string()` (Global). + // `AmlString::from_str_in` is the allocator-aware replacement. + let os_name = AmlString::from_str_in("Microsoft Windows NT", alloc.clone()); namespace - .insert(AmlName::from_str("\\_OS").unwrap(), Object::String("Microsoft Windows NT".to_string()).wrap()) + .insert( + AmlName::parse_in("\\_OS", alloc.clone()).unwrap(), + Object::String(os_name).wrap(alloc.clone()), + ) .unwrap(); /* @@ -62,67 +74,73 @@ impl Namespace { * - We answer 'yes' to `_OSI("Darwin") * - We answer 'no' to `_OSI("Linux")`, and report that the tables are doing the wrong thing */ - namespace - .insert( - AmlName::from_str("\\_OSI").unwrap(), - Object::native_method(1, |args| { - if args.len() != 1 { - return Err(AmlError::MethodArgCountIncorrect); + // PILOT-DECISION: the native_method closure now produces WrappedObject + // and must capture the allocator (moved in). Two clones used: + // - `inner_alloc` (captured by the closure, used for every result wrap) + // - `alloc.clone()` for `native_method`'s `alloc` argument (the Arc) + let inner_alloc = alloc.clone(); + let osi_method = Object::native_method( + 1, + move |args| -> Result, AmlError> { + if args.len() != 1 { + return Err(AmlError::MethodArgCountIncorrect); + } + let Object::String(ref feature) = *args[0] else { + return Err(AmlError::ObjectNotOfExpectedType { + expected: ObjectType::String, + got: args[0].typ(), + }); + }; + + let is_supported = match feature.as_str() { + "Windows 2000" => true, // 2000 + "Windows 2001" => true, // XP + "Windows 2001 SP1" => true, // XP SP1 + "Windows 2001 SP2" => true, // XP SP2 + "Windows 2001.1" => true, // Server 2003 + "Windows 2001.1 SP1" => true, // Server 2003 SP1 + "Windows 2006" => true, // Vista + "Windows 2006 SP1" => true, // Vista SP1 + "Windows 2006 SP2" => true, // Vista SP2 + "Windows 2006.1" => true, // Server 2008 + "Windows 2009" => true, // 7 and Server 2008 R2 + "Windows 2012" => true, // 8 and Server 2012 + "Windows 2013" => true, // 8.1 and Server 2012 R2 + "Windows 2015" => true, // 10 + "Windows 2016" => true, // 10 version 1607 + "Windows 2017" => true, // 10 version 1703 + "Windows 2017.2" => true, // 10 version 1709 + "Windows 2018" => true, // 10 version 1803 + "Windows 2018.2" => true, // 10 version 1809 + "Windows 2019" => true, // 10 version 1903 + "Windows 2020" => true, // 10 version 20H1 + "Windows 2021" => true, // 11 + "Windows 2022" => true, // 11 version 22H2 + + // TODO: Linux answers yes to this, NT answers no. Maybe make configurable + "Darwin" => false, + + "Linux" => { + // TODO: should we allow users to specify that this should be true? Linux has a + // command line option for this. + warn!("ACPI evaluated `_OSI(\"Linux\")`. This is a bug. Reporting no support."); + false } - let Object::String(ref feature) = *args[0] else { - return Err(AmlError::ObjectNotOfExpectedType { - expected: ObjectType::String, - got: args[0].typ(), - }); - }; - - let is_supported = match feature.as_str() { - "Windows 2000" => true, // 2000 - "Windows 2001" => true, // XP - "Windows 2001 SP1" => true, // XP SP1 - "Windows 2001 SP2" => true, // XP SP2 - "Windows 2001.1" => true, // Server 2003 - "Windows 2001.1 SP1" => true, // Server 2003 SP1 - "Windows 2006" => true, // Vista - "Windows 2006 SP1" => true, // Vista SP1 - "Windows 2006 SP2" => true, // Vista SP2 - "Windows 2006.1" => true, // Server 2008 - "Windows 2009" => true, // 7 and Server 2008 R2 - "Windows 2012" => true, // 8 and Server 2012 - "Windows 2013" => true, // 8.1 and Server 2012 R2 - "Windows 2015" => true, // 10 - "Windows 2016" => true, // 10 version 1607 - "Windows 2017" => true, // 10 version 1703 - "Windows 2017.2" => true, // 10 version 1709 - "Windows 2018" => true, // 10 version 1803 - "Windows 2018.2" => true, // 10 version 1809 - "Windows 2019" => true, // 10 version 1903 - "Windows 2020" => true, // 10 version 20H1 - "Windows 2021" => true, // 11 - "Windows 2022" => true, // 11 version 22H2 - - // TODO: Linux answers yes to this, NT answers no. Maybe make configurable - "Darwin" => false, - - "Linux" => { - // TODO: should we allow users to specify that this should be true? Linux has a - // command line option for this. - warn!("ACPI evaluated `_OSI(\"Linux\")`. This is a bug. Reporting no support."); - false - } - "Extended Address Space Descriptor" => true, - "Module Device" => true, - "3.0 Thermal Model" => true, - "3.0 _SCP Extensions" => true, - "Processor Aggregator Device" => true, - _ => false, - }; - - Ok(Object::Integer(if is_supported { u64::MAX } else { 0 }).wrap()) - }) - .wrap(), - ) + "Extended Address Space Descriptor" => true, + "Module Device" => true, + "3.0 Thermal Model" => true, + "3.0 _SCP Extensions" => true, + "Processor Aggregator Device" => true, + _ => false, + }; + + Ok(Object::Integer(if is_supported { u64::MAX } else { 0 }).wrap(inner_alloc.clone())) + }, + alloc.clone(), + ); + namespace + .insert(AmlName::parse_in("\\_OSI", alloc.clone()).unwrap(), osi_method.wrap(alloc.clone())) .unwrap(); /* @@ -131,36 +149,50 @@ impl Namespace { * return `2`), and so they switched to just returning `2` (as we'll also do). `_REV` should be considered * useless and deprecated (this is mirrored in newer specs, which claim `2` means "ACPI 2 or greater"). */ - namespace.insert(AmlName::from_str("\\_REV").unwrap(), Object::Integer(2).wrap()).unwrap(); + namespace + .insert(AmlName::parse_in("\\_REV", alloc.clone()).unwrap(), Object::Integer(2).wrap(alloc.clone())) + .unwrap(); namespace } - pub fn add_level(&mut self, path: AmlName, kind: NamespaceLevelKind) -> Result<(), AmlError> { + // PILOT-FOLLOWUP: all the AmlError variants below (LevelDoesNotExist, + // NameCollision, etc.) currently take `AmlName`. With AmlName, + // they need to become `LevelDoesNotExist(AmlName)` etc. — meaning + // AmlError gains ``. That's the largest viral spread we've hit so far. + // For now these construction sites will fail to compile in the crate, + // pointing exactly at what AmlError needs to grow. + + pub fn add_level(&mut self, path: AmlName, kind: NamespaceLevelKind) -> Result<(), AmlError> { assert!(path.is_absolute()); let path = path.normalize()?; // Don't try to recreate the root scope - if path != AmlName::root() { + if path != AmlName::root_in(self.alloc.clone()) { + // PILOT-DECISION: clone `self.alloc` *before* the mutable borrow + // below. Doing it after (as we did initially) trips E0502 — + // `level` is `&mut`-borrowed from `self`, so `self.alloc` can't + // be read until `level` is dropped. + let level_alloc = self.alloc.clone(); let (level, last_seg) = self.get_level_for_path_mut(&path)?; /* * If the level has already been added, we don't need to add it again. The parser can try to add it * multiple times if the ASL contains multiple blocks that add to the same scope/device. */ - level.children.entry(last_seg).or_insert_with(|| NamespaceLevel::new(kind)); + level.children.entry(last_seg).or_insert_with(move || NamespaceLevel::new_in(kind, level_alloc)); } Ok(()) } - pub fn remove_level(&mut self, path: AmlName) -> Result<(), AmlError> { + pub fn remove_level(&mut self, path: AmlName) -> Result<(), AmlError> { assert!(path.is_absolute()); let path = path.normalize()?; // Don't try to remove the root scope // TODO: we probably shouldn't be able to remove the pre-defined scopes either? - if path != AmlName::root() { + if path != AmlName::root_in(self.alloc.clone()) { let (level, last_seg) = self.get_level_for_path_mut(&path)?; level.children.remove(&last_seg); } @@ -168,7 +200,7 @@ impl Namespace { Ok(()) } - pub fn insert(&mut self, path: AmlName, object: WrappedObject) -> Result<(), AmlError> { + pub fn insert(&mut self, path: AmlName, object: WrappedObject) -> Result<(), AmlError> { assert!(path.is_absolute()); let path = path.normalize()?; @@ -186,7 +218,7 @@ impl Namespace { } } - pub fn create_alias(&mut self, path: AmlName, object: WrappedObject) -> Result<(), AmlError> { + pub fn create_alias(&mut self, path: AmlName, object: WrappedObject) -> Result<(), AmlError> { assert!(path.is_absolute()); let path = path.normalize()?; @@ -197,7 +229,7 @@ impl Namespace { } } - pub fn get(&mut self, path: AmlName) -> Result { + pub fn get(&mut self, path: AmlName) -> Result, AmlError> { assert!(path.is_absolute()); let path = path.normalize()?; @@ -211,7 +243,11 @@ impl Namespace { /// Search for an object at the given path of the namespace, applying the search rules described in §5.3 of the /// ACPI specification, if they are applicable. Returns the resolved name, and the handle of the first valid /// object, if found. - pub fn search(&self, path: &AmlName, starting_scope: &AmlName) -> Result<(AmlName, WrappedObject), AmlError> { + pub fn search( + &self, + path: &AmlName, + starting_scope: &AmlName, + ) -> Result<(AmlName, WrappedObject), AmlError> { if path.search_rules_apply() { /* * If search rules apply, we need to recursively look through the namespace. If the @@ -253,7 +289,11 @@ impl Namespace { } } - pub fn search_for_level(&self, level_name: &AmlName, starting_scope: &AmlName) -> Result { + pub fn search_for_level( + &self, + level_name: &AmlName, + starting_scope: &AmlName, + ) -> Result, AmlError> { if level_name.search_rules_apply() { let mut scope = starting_scope.clone().normalize()?; assert!(scope.is_absolute()); @@ -280,8 +320,8 @@ impl Namespace { /// Split an absolute path into a bunch of level segments (used to traverse the level data structure), and a /// last segment to index into that level. This must not be called on `\\`. - fn get_level_for_path(&self, path: &AmlName) -> Result<(&NamespaceLevel, NameSeg), AmlError> { - assert_ne!(*path, AmlName::root()); + fn get_level_for_path(&self, path: &AmlName) -> Result<(&NamespaceLevel, NameSeg), AmlError> { + assert_ne!(*path, AmlName::root_in(self.alloc.clone())); let (last_seg, levels) = path.0[1..].split_last().unwrap(); let NameComponent::Segment(last_seg) = last_seg else { @@ -289,7 +329,7 @@ impl Namespace { }; // TODO: this helps with diagnostics, but requires a heap allocation just in case we need to error. - let mut traversed_path = AmlName::root(); + let mut traversed_path = AmlName::root_in(self.alloc.clone()); let mut current_level = &self.root; for level in levels { @@ -307,8 +347,8 @@ impl Namespace { /// Split an absolute path into a bunch of level segments (used to traverse the level data structure), and a /// last segment to index into that level. This must not be called on `\\`. - fn get_level_for_path_mut(&mut self, path: &AmlName) -> Result<(&mut NamespaceLevel, NameSeg), AmlError> { - assert_ne!(*path, AmlName::root()); + fn get_level_for_path_mut(&mut self, path: &AmlName) -> Result<(&mut NamespaceLevel, NameSeg), AmlError> { + assert_ne!(*path, AmlName::root_in(self.alloc.clone())); let (last_seg, levels) = path.0[1..].split_last().unwrap(); let NameComponent::Segment(last_seg) = last_seg else { @@ -318,7 +358,7 @@ impl Namespace { // TODO: this helps with diagnostics, but requires a heap allocation just in case we need to error. We can // improve this by changing the `levels` interation into an `enumerate()`, and then using the index to // create the correct path on the error path - let mut traversed_path = AmlName::root(); + let mut traversed_path = AmlName::root_in(self.alloc.clone()); let mut current_level = &mut self.root; for level in levels { @@ -336,19 +376,23 @@ impl Namespace { Ok((current_level, *last_seg)) } - /// Traverse the namespace, calling `f` on each namespace level. `f` returns a `Result` - + /// Traverse the namespace, calling `f` on each namespace level. `f` returns a `Result>` - /// errors terminate the traversal and are propagated, and the `bool` on the successful path marks whether the /// children of the level should also be traversed. - pub fn traverse(&mut self, mut f: F) -> Result<(), AmlError> + pub fn traverse(&mut self, mut f: F) -> Result<(), AmlError> where - F: FnMut(&AmlName, &NamespaceLevel) -> Result, + F: FnMut(&AmlName, &NamespaceLevel) -> Result>, { - fn traverse_level(level: &NamespaceLevel, scope: &AmlName, f: &mut F) -> Result<(), AmlError> + fn traverse_level( + level: &NamespaceLevel, + scope: &AmlName, + f: &mut F, + ) -> Result<(), AmlError> where - F: FnMut(&AmlName, &NamespaceLevel) -> Result, + F: FnMut(&AmlName, &NamespaceLevel) -> Result>, { for (name, child) in level.children.iter() { - let name = AmlName::from_name_seg(*name).resolve(scope)?; + let name = AmlName::from_name_seg_in(*name, scope.0.allocator().clone()).resolve(scope)?; if f(&name, child)? { traverse_level(child, &name, f)?; @@ -358,21 +402,32 @@ impl Namespace { Ok(()) } - if f(&AmlName::root(), &self.root)? { - traverse_level(&self.root, &AmlName::root(), &mut f)?; + let root = AmlName::root_in(self.alloc.clone()); + if f(&root, &self.root)? { + traverse_level(&self.root, &root, &mut f)?; } Ok(()) } } -impl fmt::Display for Namespace { +// PILOT-FOLLOWUP: this Display impl uses String via indent_stack and +// String::from. It's only invoked when explicitly formatting a Namespace +// (e.g., for debug printing) and isn't reachable from normal AML evaluation +// — but it does prevent the crate from being "no Global at link time" if a +// downstream caller ever instantiates it. Rewriting to use a depth integer +// + per-level write loop is ~20 lines of work; left for follow-up. +impl fmt::Display for Namespace { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { const STEM: &str = "│ "; const BRANCH: &str = "├── "; const END: &str = "└── "; - fn print_level(f: &mut fmt::Formatter<'_>, level: &NamespaceLevel, indent_stack: String) -> fmt::Result { + fn print_level( + f: &mut fmt::Formatter<'_>, + level: &NamespaceLevel, + indent_stack: String, + ) -> fmt::Result { for (i, (name, (flags, object))) in level.values.iter().enumerate() { let end = (i == level.values.len() - 1) && level.children.iter().filter(|(_, l)| l.kind == NamespaceLevelKind::Scope).count() == 0; @@ -396,7 +451,7 @@ impl fmt::Display for Namespace { } } - let remaining_scopes: Vec<_> = + let remaining_scopes: alloc::vec::Vec<_> = level.children.iter().filter(|(_, l)| l.kind == NamespaceLevelKind::Scope).collect(); for (i, (name, sub_level)) in remaining_scopes.iter().enumerate() { let end = i == remaining_scopes.len() - 1; @@ -423,10 +478,10 @@ pub enum NamespaceLevelKind { } #[derive(Clone)] -pub struct NamespaceLevel { +pub struct NamespaceLevel { pub kind: NamespaceLevelKind, - pub values: BTreeMap, - pub children: BTreeMap, + pub values: BTreeMap), A>, + pub children: BTreeMap, A>, } #[derive(Clone, Copy, Debug)] @@ -444,14 +499,33 @@ impl ObjectFlags { } } -impl NamespaceLevel { - pub fn new(kind: NamespaceLevelKind) -> NamespaceLevel { - NamespaceLevel { kind, values: BTreeMap::new(), children: BTreeMap::new() } +impl NamespaceLevel { + pub fn new_in(kind: NamespaceLevelKind, alloc: A) -> NamespaceLevel { + NamespaceLevel { kind, values: BTreeMap::new_in(alloc.clone()), children: BTreeMap::new_in(alloc) } + } +} + +// PILOT-DECISION: only Clone is derived. PartialEq/Debug get manual impls +// below to avoid the derive macro's auto-added `A: PartialEq` / `A: Debug` +// bounds — `&'static BumpArena` satisfies neither, and the bounds aren't +// needed because Vec's own impls don't require A: PartialEq/Debug. +#[derive(Clone)] +pub struct AmlName(Vec); + +impl PartialEq> for AmlName { + fn eq(&self, other: &AmlName) -> bool { + self.0 == other.0 } } -#[derive(Clone, PartialEq, Debug)] -pub struct AmlName(Vec); +impl Eq for AmlName {} + +impl fmt::Debug for AmlName { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // Use Display representation — that's what an AmlName "looks like". + write!(f, "AmlName({})", self) + } +} #[derive(Clone, Copy, PartialEq, Debug)] pub enum NameComponent { @@ -460,29 +534,57 @@ pub enum NameComponent { Segment(NameSeg), } -impl AmlName { - pub fn root() -> AmlName { - AmlName(vec![NameComponent::Root]) +impl AmlName { + // PILOT-DECISION: was `root()` returning `AmlName` (Global). Now takes an + // allocator. Most call sites already have one (Namespace::alloc or via + // an existing AmlName they can clone-allocator from). + pub fn root_in(alloc: A) -> AmlName { + let mut v = Vec::with_capacity_in(1, alloc); + v.push(NameComponent::Root); + AmlName(v) } - pub fn from_name_seg(seg: NameSeg) -> AmlName { - AmlName(vec![NameComponent::Segment(seg)]) + pub fn from_name_seg_in(seg: NameSeg, alloc: A) -> AmlName { + let mut v = Vec::with_capacity_in(1, alloc); + v.push(NameComponent::Segment(seg)); + AmlName(v) } - pub fn from_components(components: Vec) -> AmlName { + pub fn from_components(components: Vec) -> AmlName { AmlName(components) } - pub fn as_string(&self) -> String { - self.0 - .iter() - .fold(String::new(), |name, component| match component { - NameComponent::Root => name + "\\", - NameComponent::Prefix => name + "^", - NameComponent::Segment(seg) => name + seg.as_str() + ".", - }) - .trim_end_matches('.') - .to_string() + // PILOT-DECISION: replacement for `FromStr::from_str`. FromStr's trait + // signature is `fn from_str(s: &str) -> Result` — no + // way to thread an allocator. We provide `parse_in` as the allocator- + // aware constructor; FromStr can be impl'd for AmlName as a + // convenience for Global-only callers (left as PILOT-FOLLOWUP). + pub fn parse_in(mut string: &str, alloc: A) -> Result, AmlError> { + if string.is_empty() { + return Err(AmlError::EmptyNamesAreInvalid); + } + + let mut components = Vec::new_in(alloc); + + // If it starts with a \, make it an absolute name + if string.starts_with('\\') { + components.push(NameComponent::Root); + string = &string[1..]; + } + + if !string.is_empty() { + for mut part in string.split('.') { + // Handle prefix chars + while part.starts_with('^') { + components.push(NameComponent::Prefix); + part = &part[1..]; + } + + components.push(NameComponent::Segment(NameSeg::from_str_inner(part)?)); + } + } + + Ok(AmlName(components)) } /// An AML path is normal if it does not contain any prefix elements ("^" characters, when @@ -507,7 +609,7 @@ impl AmlName { /// Normalize an AML path, resolving prefix chars. Returns `AmlError::InvalidNormalizedName` if the path /// normalizes to an invalid path (e.g. `\^_FOO`) - pub fn normalize(self) -> Result { + pub fn normalize(self) -> Result, AmlError> { /* * If the path is already normal, just return it as-is. This avoids an unneccessary heap allocation and * free. @@ -516,7 +618,10 @@ impl AmlName { return Ok(self); } - Ok(AmlName(self.0.iter().try_fold(Vec::new(), |mut name, &component| match component { + // PILOT-DECISION: derive the allocator from self.0 instead of taking + // it as a parameter. `Vec::allocator()` returns `&A` so we clone. + let alloc = self.0.allocator().clone(); + Ok(AmlName(self.0.iter().try_fold(Vec::new_in(alloc), |mut name, &component| match component { seg @ NameComponent::Segment(_) => { name.push(seg); Ok(name) @@ -540,7 +645,7 @@ impl AmlName { /// Get the parent of this `AmlName`. For example, the parent of `\_SB.PCI0._PRT` is `\_SB.PCI0`. The root /// path has no parent, and so returns `None`. - pub fn parent(&self) -> Result { + pub fn parent(&self) -> Result, AmlError> { // Firstly, normalize the path so we don't have to deal with prefix chars let mut normalized_self = self.clone().normalize()?; @@ -556,7 +661,7 @@ impl AmlName { /// Resolve this path against a given scope, making it absolute. If the path is absolute, it is /// returned directly. The path is also normalized. - pub fn resolve(&self, scope: &AmlName) -> Result { + pub fn resolve(&self, scope: &AmlName) -> Result, AmlError> { assert!(scope.is_absolute()); if self.is_absolute() { @@ -569,42 +674,26 @@ impl AmlName { } } -impl FromStr for AmlName { - type Err = AmlError; - - fn from_str(mut string: &str) -> Result { - if string.is_empty() { - return Err(AmlError::EmptyNamesAreInvalid); - } - - let mut components = Vec::new(); - - // If it starts with a \, make it an absolute name - if string.starts_with('\\') { - components.push(NameComponent::Root); - string = &string[1..]; - } - - if !string.is_empty() { - // Divide the rest of it into segments, and parse those - for mut part in string.split('.') { - // Handle prefix chars - while part.starts_with('^') { - components.push(NameComponent::Prefix); - part = &part[1..]; +// PILOT-DECISION: Display for AmlName rewritten to write directly to the +// formatter without allocating. Previously delegated to `as_string()` which +// folded into a Global-allocated String. +impl fmt::Display for AmlName { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let mut iter = self.0.iter().peekable(); + while let Some(component) = iter.next() { + match component { + NameComponent::Root => f.write_str("\\")?, + NameComponent::Prefix => f.write_str("^")?, + NameComponent::Segment(seg) => { + f.write_str(seg.as_str())?; + // Add separator if the next component is also a segment. + if matches!(iter.peek(), Some(NameComponent::Segment(_))) { + f.write_str(".")?; + } } - - components.push(NameComponent::Segment(NameSeg::from_str(part)?)); } } - - Ok(Self(components)) - } -} - -impl fmt::Display for AmlName { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}", self.as_string()) + Ok(()) } } @@ -612,7 +701,7 @@ impl fmt::Display for AmlName { pub struct NameSeg(pub(crate) [u8; 4]); impl NameSeg { - pub fn from_bytes(bytes: [u8; 4]) -> Result { + pub fn from_bytes(bytes: [u8; 4]) -> Result> { if !is_lead_name_char(bytes[0]) { return Err(AmlError::InvalidNameSeg(bytes)); } @@ -632,12 +721,12 @@ impl NameSeg { // We should only construct valid ASCII name segments unsafe { str::from_utf8_unchecked(&self.0) } } -} - -impl FromStr for NameSeg { - type Err = AmlError; - fn from_str(s: &str) -> Result { + // PILOT-DECISION: was `impl FromStr for NameSeg`. NameSeg doesn't hold + // allocations so FromStr would work in principle, but `AmlName::parse_in` + // now calls this directly and we don't need the trait surface. Kept as + // an inherent method to make the call site explicit. + pub fn from_str_inner(s: &str) -> Result> { // Each NameSeg can only have four chars, and must have at least one if s.is_empty() || s.len() > 4 { return Err(AmlError::InvalidNameSeg([0xff, 0xff, 0xff, 0xff])); diff --git a/src/aml/object.rs b/src/aml/object.rs index d79af4b1..06d6f59e 100644 --- a/src/aml/object.rs +++ b/src/aml/object.rs @@ -1,50 +1,166 @@ use crate::aml::{AmlError, Handle, Operation, op_region::OpRegion}; -use alloc::{ - borrow::Cow, - string::{String, ToString}, - sync::Arc, - vec::Vec, -}; +use alloc::{sync::Arc, vec::Vec}; use bit_field::BitField; -use core::{cell::UnsafeCell, fmt, ops, sync::atomic::AtomicU64}; +use core::{alloc::Allocator, cell::UnsafeCell, fmt, ops, sync::atomic::AtomicU64}; -type NativeMethod = dyn Fn(&[WrappedObject]) -> Result; +// PILOT-DECISION: nightly's `String` is not allocator-parameterized (no +// `String` type, no `String::new_in`). `AmlString` is a thin newtype +// wrapping `Vec` with a UTF-8 invariant, providing the subset of +// `String` operations the AML interpreter actually needs. Construction sites +// always start from validated `&str` literals or copy from existing strings, +// so the UTF-8 invariant is straightforward to maintain. +pub struct AmlString(Vec); + +impl AmlString { + pub fn new_in(alloc: A) -> Self { + Self(Vec::new_in(alloc)) + } + + pub fn from_str_in(s: &str, alloc: A) -> Self { + let mut v = Vec::with_capacity_in(s.len(), alloc); + v.extend_from_slice(s.as_bytes()); + Self(v) + } + + #[inline] + pub fn as_str(&self) -> &str { + // SAFETY: every construction path either takes a `&str` (already valid + // UTF-8) or pushes via `push_str`/`push(char)` (also guaranteed valid). + // `as_bytes_mut` is the only way to break this and is `unsafe`. + unsafe { core::str::from_utf8_unchecked(&self.0) } + } + + #[inline] + pub fn as_bytes(&self) -> &[u8] { + &self.0 + } + + /// # Safety + /// Caller must preserve UTF-8 validity in the returned slice. Breaking + /// this invariant makes [`as_str`] unsound on subsequent reads. + #[inline] + pub unsafe fn as_bytes_mut(&mut self) -> &mut [u8] { + self.0.as_mut_slice() + } + + pub fn push_str(&mut self, s: &str) { + self.0.extend_from_slice(s.as_bytes()); + } + + pub fn push(&mut self, c: char) { + let mut buf = [0u8; 4]; + let s = c.encode_utf8(&mut buf); + self.0.extend_from_slice(s.as_bytes()); + } + + pub fn clear(&mut self) { + self.0.clear(); + } + + #[inline] + pub fn len(&self) -> usize { + self.0.len() + } + + #[inline] + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + pub fn parse(&self) -> Result { + self.as_str().parse::() + } +} + +impl Clone for AmlString { + fn clone(&self) -> Self { + Self(self.0.clone()) + } +} + +// Manual PartialEq impls — derive would bound `A: PartialEq`, but Vec's +// PartialEq impl works for any allocator (compares element-wise). +impl PartialEq> for AmlString { + fn eq(&self, other: &AmlString) -> bool { + self.as_bytes() == other.as_bytes() + } +} + +impl Eq for AmlString {} + +impl fmt::Display for AmlString { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.as_str()) + } +} + +impl fmt::Debug for AmlString { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(self.as_str(), f) + } +} + +// Lets `write!(amlstring, "...")` work without going through Global. +impl fmt::Write for AmlString { + fn write_str(&mut self, s: &str) -> fmt::Result { + self.push_str(s); + Ok(()) + } +} + +// PILOT-DECISION: NativeMethod is parameterized over `A` because the closure +// produces and consumes WrappedObject. The 'static bound stays on the +// constructor (`Object::native_method` below). +type NativeMethod = dyn Fn(&[WrappedObject]) -> Result, AmlError>; #[derive(Clone)] -pub enum Object { +pub enum Object { Uninitialized, - Buffer(Vec), - BufferField { buffer: WrappedObject, offset: usize, length: usize }, + Buffer(Vec), + BufferField { buffer: WrappedObject, offset: usize, length: usize }, Device, - Event(Arc), - FieldUnit(FieldUnit), + // PILOT-DECISION: Event's Arc is also allocator-parameterized. We could + // keep this Arc as a "small exception" for shared + // synchronization primitives, but consistency wins — every allocation + // goes through the same arena. + Event(Arc), + FieldUnit(FieldUnit), Integer(u64), - Method { code: Vec, flags: MethodFlags }, - NativeMethod { f: Arc, flags: MethodFlags }, + Method { code: Vec, flags: MethodFlags }, + NativeMethod { f: Arc, A>, flags: MethodFlags }, Mutex { mutex: Handle, sync_level: u8 }, - Reference { kind: ReferenceKind, inner: WrappedObject }, - OpRegion(OpRegion), - Package(Vec), + Reference { kind: ReferenceKind, inner: WrappedObject }, + OpRegion(OpRegion), + Package(Vec, A>), PowerResource { system_level: u8, resource_order: u16 }, Processor { proc_id: u8, pblk_address: u32, pblk_length: u8 }, RawDataBuffer, - String(String), + String(AmlString), ThermalZone, Debug, } -impl Object { - pub fn native_method(num_args: u8, f: F) -> Object +impl Object { + // PILOT-DECISION: native_method now takes `alloc: A`. Caller threads the + // allocator at construction. Matches the platform/* pattern of consuming + // an allocator (A: Clone makes this cheap). The `A: 'static` bound here + // is forced by `Arc`: the closure may capture + // variables of type A (e.g., a cloned allocator), and a `'static` trait + // object can't capture non-'static data. For `&'static BumpArena` + // this is satisfied trivially. + pub fn native_method(num_args: u8, f: F, alloc: A) -> Object where - F: Fn(&[WrappedObject]) -> Result + 'static, + A: 'static, + F: Fn(&[WrappedObject]) -> Result, AmlError> + 'static, { let mut flags = 0; flags.set_bits(0..3, num_args); - Object::NativeMethod { f: Arc::new(f), flags: MethodFlags(flags) } + // Arc coerces to Arc at the field assignment. + Object::NativeMethod { f: Arc::new_in(f, alloc), flags: MethodFlags(flags) } } } -impl fmt::Display for Object { +impl fmt::Display for Object { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Object::Uninitialized => write!(f, "[Uninitialized]"), @@ -102,13 +218,22 @@ impl ObjectToken { } } -#[derive(Clone, Debug)] -pub struct WrappedObject(Arc>); +#[derive(Clone)] +pub struct WrappedObject(Arc>, A>); -impl WrappedObject { - pub fn new(object: Object) -> WrappedObject { +// Manual Debug impl — derive auto-bounds `A: Debug`. +impl fmt::Debug for WrappedObject { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_tuple("WrappedObject").finish_non_exhaustive() + } +} + +impl WrappedObject { + // PILOT-DECISION: `new` takes the allocator explicitly. Previously this + // was `pub fn new(object: Object) -> WrappedObject` with implicit Global. + pub fn new(object: Object, alloc: A) -> WrappedObject { #[allow(clippy::arc_with_non_send_sync)] - WrappedObject(Arc::new(UnsafeCell::new(object))) + WrappedObject(Arc::new_in(UnsafeCell::new(object), alloc)) } /// Gain a mutable reference to an [`Object`] from this [`WrappedObject`]. @@ -119,7 +244,7 @@ impl WrappedObject { /// prevent the same object, referenced from multiple [`WrappedObject`]s, having multiple /// mutable (and therefore aliasing) references being made to it, and therefore care must be /// taken in the interpreter to prevent this. - pub unsafe fn gain_mut<'r, 'a, 't>(&'a self, _token: &'t ObjectToken) -> &'r mut Object + pub unsafe fn gain_mut<'r, 'a, 't>(&'a self, _token: &'t ObjectToken) -> &'r mut Object where 't: 'r, 'a: 'r, @@ -127,7 +252,7 @@ impl WrappedObject { unsafe { &mut *(self.0.get()) } } - pub fn unwrap_reference(self) -> WrappedObject { + pub fn unwrap_reference(self) -> WrappedObject { let mut object = self; loop { if let Object::Reference { ref inner, .. } = *object { @@ -140,7 +265,7 @@ impl WrappedObject { /// Unwraps 'transparent' references (e.g. locals, arguments, and internal usage of reference-type objects), but maintain 'real' /// references deliberately created by AML. - pub fn unwrap_transparent_reference(self) -> WrappedObject { + pub fn unwrap_transparent_reference(self) -> WrappedObject { let mut object = self; loop { if let Object::Reference { kind, ref inner } = *object @@ -154,8 +279,8 @@ impl WrappedObject { } } -impl ops::Deref for WrappedObject { - type Target = Object; +impl ops::Deref for WrappedObject { + type Target = Object; fn deref(&self) -> &Self::Target { /* @@ -168,18 +293,18 @@ impl ops::Deref for WrappedObject { } } -impl fmt::Display for WrappedObject { +impl fmt::Display for WrappedObject { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "Wrapped({})", unsafe { &*self.0.get() }) } } -impl Object { - pub fn wrap(self) -> WrappedObject { - WrappedObject::new(self) +impl Object { + pub fn wrap(self, alloc: A) -> WrappedObject { + WrappedObject::new(self, alloc) } - pub fn as_integer(&self) -> Result { + pub fn as_integer(&self) -> Result> { if let Object::Integer(value) = self { Ok(*value) } else { @@ -187,15 +312,20 @@ impl Object { } } - pub fn as_string(&self) -> Result, AmlError> { + // PILOT-DECISION: signature changed from `Result, AmlError>` + // to `Result<&str, AmlError>`. Cow<'_, str> is hardcoded to Owned=String, + // which can't represent String. Every existing call site reads `as_string` + // as a borrowed view, so `&str` is functionally equivalent and simpler. + // API CHANGE — callers that explicitly wanted Cow must adjust. + pub fn as_string(&self) -> Result<&str, AmlError> { if let Object::String(value) = self { - Ok(Cow::from(value)) + Ok(value.as_str()) } else { Err(AmlError::ObjectNotOfExpectedType { expected: ObjectType::String, got: self.typ() }) } } - pub fn as_buffer(&self) -> Result<&[u8], AmlError> { + pub fn as_buffer(&self) -> Result<&[u8], AmlError> { if let Object::Buffer(bytes) = self { Ok(bytes) } else { @@ -203,7 +333,7 @@ impl Object { } } - pub fn to_integer(&self, allowed_bytes: usize) -> Result { + pub fn to_integer(&self, allowed_bytes: usize) -> Result> { match self { Object::Integer(value) => Ok(*value), Object::Buffer(value) => { @@ -214,24 +344,39 @@ impl Object { } // TODO: how should we handle invalid inputs? What does NT do here? Object::String(value) => Ok(value.parse::().unwrap_or(0)), + // ^ AmlString::parse delegates to str::parse — same behavior as String. _ => Ok(0), } } - pub fn to_buffer(&self, allowed_bytes: usize) -> Result, AmlError> { + // PILOT-DECISION: `to_buffer` takes an allocator to construct the returned + // Vec. Caller threads the allocator at the call site. + pub fn to_buffer(&self, allowed_bytes: usize, alloc: A) -> Result, AmlError> { match self { Object::Buffer(bytes) => Ok(bytes.clone()), - Object::Integer(value) => match allowed_bytes { - 4 => Ok((*value as u32).to_le_bytes().to_vec()), - 8 => Ok(value.to_le_bytes().to_vec()), - _ => panic!(), - }, - Object::String(value) => Ok(value.as_bytes().to_vec()), + Object::Integer(value) => { + let bytes: &[u8] = match allowed_bytes { + 4 => &(*value as u32).to_le_bytes(), + 8 => &value.to_le_bytes(), + _ => panic!(), + }; + let mut out = Vec::with_capacity_in(bytes.len(), alloc); + out.extend_from_slice(bytes); + Ok(out) + } + Object::String(value) => { + let src = value.as_bytes(); + let mut out = Vec::with_capacity_in(src.len(), alloc); + out.extend_from_slice(src); + Ok(out) + } _ => Err(AmlError::InvalidOperationOnObject { op: Operation::ConvertToBuffer, typ: self.typ() }), } } - pub fn read_buffer_field(&self, integer_size: usize) -> Result { + // PILOT-DECISION: `read_buffer_field` allocates a Vec for the + // multi-byte path. Takes `alloc: A` parameter. + pub fn read_buffer_field(&self, integer_size: usize, alloc: A) -> Result, AmlError> { if let Self::BufferField { buffer, offset, length } = self { let buffer = match **buffer { Object::Buffer(ref buffer) => buffer.as_slice(), @@ -243,7 +388,11 @@ impl Object { copy_bits(buffer, *offset, &mut dst, 0, *length); Ok(Object::Integer(u64::from_le_bytes(dst))) } else { - let mut dst = alloc::vec![0u8; length.div_ceil(8)]; + // PILOT-DECISION: was `alloc::vec![0u8; length.div_ceil(8)]` + // (Global). Use allocator-aware Vec::with_capacity_in + resize. + let size = length.div_ceil(8); + let mut dst = Vec::with_capacity_in(size, alloc); + dst.resize(size, 0u8); copy_bits(buffer, *offset, &mut dst, 0, *length); Ok(Object::Buffer(dst)) } @@ -252,7 +401,7 @@ impl Object { } } - pub fn write_buffer_field(&mut self, value: &[u8], token: &ObjectToken) -> Result<(), AmlError> { + pub fn write_buffer_field(&mut self, value: &[u8], token: &ObjectToken) -> Result<(), AmlError> { // TODO: bounds check the buffer first to avoid panicking if let Self::BufferField { buffer, offset, length } = self { let buffer = match unsafe { buffer.gain_mut(token) } { @@ -272,31 +421,71 @@ impl Object { /// Replace this object's contents with that of a `new` object, applying implicit casting rules /// as needed. This follows the NT interpreter's creative interpretation of implicit casts, which is /// effectively a byte-wise transmutation. - pub fn replace_with_implicit_casting(&mut self, new: Object) -> Result<(), AmlError> { - let new_bytes = match new { - Object::Integer(value) => &value.to_le_bytes(), + // + // PILOT-DECISION: signature gains `alloc: A` for the cases where new + // backing storage is needed (clearing+repopulating String/Buffer can reuse + // the existing allocator, but the Buffer→Buffer path's `value.clone()` + // also wants A for the source clone — no extra param needed since A is + // already on `new: Object`). + // + // PILOT-FOLLOWUP: the original code did `String::from_utf8_lossy(...).to_string()` + // which goes through Global on invalid UTF-8. The new path below handles + // the valid prefix only and substitutes a single replacement char on + // invalid input. This is a behavioral regression compared to from_utf8_lossy; + // a proper allocator-aware lossy decoder is left for follow-up. + pub fn replace_with_implicit_casting(&mut self, new: Object) -> Result<(), AmlError> { + // Extract a &[u8] view of `new` without taking ownership (so we can keep + // its allocator A live for the lifetime of the borrow). + let new_bytes: &[u8] = match new { + Object::Integer(value) => { + // Convert to a fixed-size byte buffer first; the borrow below + // must outlive the match arm, so we stash it in a local. + let bytes = value.to_le_bytes(); + return apply_cast_bytes_owned(self, &bytes); + } Object::String(ref value) => value.as_bytes(), - Object::Buffer(ref value) => &value.clone(), + Object::Buffer(ref value) => value.as_slice(), _ => return Err(AmlError::InvalidImplicitCast { from: self.typ(), to: new.typ() }), }; + apply_cast_bytes(self, new_bytes)?; + return Ok(()); + + fn apply_cast_bytes_owned( + target: &mut Object, + bytes: &[u8], + ) -> Result<(), AmlError> { + apply_cast_bytes(target, bytes) + } - match self { - Object::Integer(value) => { - let bytes_to_copy = core::cmp::min(new_bytes.len(), 8); - let mut bytes = [0u8; 8]; - bytes[0..bytes_to_copy].copy_from_slice(&new_bytes[0..bytes_to_copy]); - *value = u64::from_le_bytes(bytes); - } - Object::String(value) => { - *value = String::from_utf8_lossy(&new_bytes).to_string(); - } - Object::Buffer(value) => { - *value = new_bytes.to_vec(); + fn apply_cast_bytes( + target: &mut Object, + new_bytes: &[u8], + ) -> Result<(), AmlError> { + match target { + Object::Integer(value) => { + let bytes_to_copy = core::cmp::min(new_bytes.len(), 8); + let mut bytes = [0u8; 8]; + bytes[0..bytes_to_copy].copy_from_slice(&new_bytes[0..bytes_to_copy]); + *value = u64::from_le_bytes(bytes); + } + Object::String(value) => { + value.clear(); + match core::str::from_utf8(new_bytes) { + Ok(s) => value.push_str(s), + Err(_) => { + // See PILOT-FOLLOWUP above. + value.push(char::REPLACEMENT_CHARACTER); + } + } + } + Object::Buffer(value) => { + value.clear(); + value.extend_from_slice(new_bytes); + } + _ => return Err(AmlError::InvalidImplicitCast { from: target.typ(), to: ObjectType::Buffer }), } - _ => return Err(AmlError::InvalidImplicitCast { from: self.typ(), to: new.typ() }), + Ok(()) } - - Ok(()) } /// Returns the `ObjectType` of this object. Returns the type of the referenced object in the @@ -329,19 +518,35 @@ impl Object { } } -#[derive(Clone, Debug)] -pub struct FieldUnit { - pub kind: FieldUnitKind, +#[derive(Clone)] +pub struct FieldUnit { + pub kind: FieldUnitKind, pub flags: FieldFlags, pub bit_index: usize, pub bit_length: usize, } -#[derive(Clone, Debug)] -pub enum FieldUnitKind { - Normal { region: WrappedObject }, - Bank { region: WrappedObject, bank: WrappedObject, bank_value: u64 }, - Index { index: WrappedObject, data: WrappedObject }, +impl fmt::Debug for FieldUnit { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("FieldUnit").field("flags", &self.flags).field("bit_length", &self.bit_length).finish_non_exhaustive() + } +} + +#[derive(Clone)] +pub enum FieldUnitKind { + Normal { region: WrappedObject }, + Bank { region: WrappedObject, bank: WrappedObject, bank_value: u64 }, + Index { index: WrappedObject, data: WrappedObject }, +} + +impl fmt::Debug for FieldUnitKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Normal { .. } => f.write_str("Normal { .. }"), + Self::Bank { .. } => f.write_str("Bank { .. }"), + Self::Index { .. } => f.write_str("Index { .. }"), + } + } } #[derive(Clone, Copy, Debug)] @@ -365,7 +570,10 @@ pub enum FieldUpdateRule { } impl FieldFlags { - pub fn access_type(&self) -> Result { + // PILOT-DECISION: FieldFlags is allocator-free (just a u8 newtype), but + // its methods return `Result<_, AmlError>`. Adding `` to each + // method lets the caller's A flow through via type inference. + pub fn access_type(&self) -> Result> { match self.0.get_bits(0..4) { 0 => Ok(FieldAccessType::Any), 1 => Ok(FieldAccessType::Byte), @@ -377,7 +585,7 @@ impl FieldFlags { } } - pub fn access_type_bytes(&self) -> Result { + pub fn access_type_bytes(&self) -> Result> { match self.access_type()? { FieldAccessType::Any => { // TODO: given more info about the field, we might be able to make a more efficient diff --git a/src/aml/op_region.rs b/src/aml/op_region.rs index e2e73d64..9f878f78 100644 --- a/src/aml/op_region.rs +++ b/src/aml/op_region.rs @@ -1,23 +1,42 @@ use crate::aml::{AmlError, namespace::AmlName}; +use core::alloc::Allocator; -#[derive(Clone, Debug)] -pub struct OpRegion { +// PILOT-DECISION: OpRegion was holding `AmlName` (Global). Now generic over A +// so OpRegion embeds AmlName. RegionHandler keeps its non-generic +// trait shape; each method has its own `` parameter for the error type, +// letting concrete handler impls live in callers' allocator universe. +#[derive(Clone)] +pub struct OpRegion { pub space: RegionSpace, pub base: u64, pub length: u64, - pub parent_device_path: AmlName, + pub parent_device_path: AmlName, } -pub trait RegionHandler { - fn read_u8(&self, region: &OpRegion) -> Result; - fn read_u16(&self, region: &OpRegion) -> Result; - fn read_u32(&self, region: &OpRegion) -> Result; - fn read_u64(&self, region: &OpRegion) -> Result; +impl core::fmt::Debug for OpRegion { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_struct("OpRegion") + .field("space", &self.space) + .field("base", &self.base) + .field("length", &self.length) + .field("parent_device_path", &self.parent_device_path) + .finish() + } +} + +// PILOT-DECISION: trait parameterized by A so it's object-safe — generic +// methods can't appear on a `Box`. Concrete handler impls now +// pin themselves to a specific A (typically the same A as the interpreter). +pub trait RegionHandler { + fn read_u8(&self, region: &OpRegion) -> Result>; + fn read_u16(&self, region: &OpRegion) -> Result>; + fn read_u32(&self, region: &OpRegion) -> Result>; + fn read_u64(&self, region: &OpRegion) -> Result>; - fn write_u8(&self, region: &OpRegion, value: u8) -> Result<(), AmlError>; - fn write_u16(&self, region: &OpRegion, value: u16) -> Result<(), AmlError>; - fn write_u32(&self, region: &OpRegion, value: u32) -> Result<(), AmlError>; - fn write_u64(&self, region: &OpRegion, value: u64) -> Result<(), AmlError>; + fn write_u8(&self, region: &OpRegion, value: u8) -> Result<(), AmlError>; + fn write_u16(&self, region: &OpRegion, value: u16) -> Result<(), AmlError>; + fn write_u32(&self, region: &OpRegion, value: u32) -> Result<(), AmlError>; + fn write_u64(&self, region: &OpRegion, value: u64) -> Result<(), AmlError>; } #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug)] diff --git a/src/aml/pci_routing.rs b/src/aml/pci_routing.rs index 475ee9d6..3f1d0b46 100644 --- a/src/aml/pci_routing.rs +++ b/src/aml/pci_routing.rs @@ -7,9 +7,9 @@ use crate::aml::{ object::Object, resource::{self, InterruptPolarity, InterruptTrigger, Resource}, }; -use alloc::{vec, vec::Vec}; +use alloc::vec::Vec; use bit_field::BitField; -use core::str::FromStr; +use core::alloc::Allocator; pub use crate::aml::resource::IrqDescriptor; @@ -22,7 +22,7 @@ pub enum Pin { } #[derive(Debug)] -pub enum PciRouteType { +pub enum PciRouteType { /// The interrupt is hard-coded to a specific GSI Gsi(u32), @@ -33,15 +33,15 @@ pub enum PciRouteType { * The actual object itself will just be a `Device`, and we need paths to its children objects to do * anything useful, so we just store the resolved name here. */ - LinkObject(AmlName), + LinkObject(AmlName), } #[derive(Debug)] -pub struct PciRoute { +pub struct PciRoute { device: u16, function: u16, pin: Pin, - route_type: PciRouteType, + route_type: PciRouteType, } /// A `PciRoutingTable` is used to interpret the data in a `_PRT` object, which provides a mapping @@ -49,22 +49,23 @@ pub struct PciRoute { /// present under each PCI root bridge, and consists of a package of packages, each of which describes the /// mapping of a single PCI interrupt pin. #[derive(Debug)] -pub struct PciRoutingTable { - entries: Vec, +pub struct PciRoutingTable { + entries: Vec, A>, } -impl PciRoutingTable { +impl PciRoutingTable { /// Construct a `PciRoutingTable` from a path to a `_PRT` object. Returns /// `AmlError::InvalidOperationOnObject` if the value passed is not a package, or if any of the /// values within it are not packages. Returns the various `AmlError::Prt*` errors if the /// internal structure of the entries is invalid. - pub fn from_prt_path( - prt_path: AmlName, - interpreter: &Interpreter, - ) -> Result { - let mut entries = Vec::new(); + pub fn from_prt_path( + prt_path: AmlName, + interpreter: &Interpreter, + alloc: A, + ) -> Result, AmlError> { + let mut entries = Vec::new_in(alloc.clone()); - let prt = interpreter.evaluate(prt_path.clone(), vec![])?; + let prt = interpreter.evaluate(prt_path.clone(), Vec::new_in(alloc.clone()))?; if let Object::Package(ref inner_values) = *prt { for value in inner_values { @@ -125,7 +126,7 @@ impl PciRoutingTable { let link_object_name = interpreter .namespace .lock() - .search_for_level(&AmlName::from_str(name)?, &prt_path)?; + .search_for_level(&AmlName::parse_in(name.as_str(), alloc.clone())?, &prt_path)?; entries.push(PciRoute { device, function, @@ -148,13 +149,14 @@ impl PciRoutingTable { /// Get the interrupt input that a given PCI interrupt pin is wired to. Returns `AmlError::PrtNoEntry` if the /// PRT doesn't contain an entry for the given address + pin. - pub fn route( + pub fn route( &self, device: u16, function: u16, pin: Pin, - interpreter: &Interpreter, - ) -> Result { + interpreter: &Interpreter, + alloc: A, + ) -> Result> { let entry = self .entries .iter() @@ -175,8 +177,8 @@ impl PciRoutingTable { irq: gsi, }), PciRouteType::LinkObject(ref name) => { - let path = AmlName::from_str("_CRS").unwrap().resolve(name)?; - let link_crs = interpreter.evaluate(path, vec![])?; + let path = AmlName::parse_in("_CRS", alloc.clone()).unwrap().resolve(name)?; + let link_crs = interpreter.evaluate(path, Vec::new_in(alloc.clone()))?; let resources = resource::resource_descriptor_list(link_crs)?; match resources.as_slice() { diff --git a/src/aml/resource.rs b/src/aml/resource.rs index ffdd3a65..2c496c97 100644 --- a/src/aml/resource.rs +++ b/src/aml/resource.rs @@ -1,9 +1,11 @@ -use super::object::WrappedObject; -use crate::aml::{AmlError, Operation, object::Object}; -use alloc::vec::Vec; +use crate::aml::{ + AmlError, + Operation, + object::{Object, WrappedObject}, +}; use bit_field::BitField; use byteorder::{ByteOrder, LittleEndian}; -use core::mem; +use core::{alloc::Allocator, mem}; #[derive(Debug, PartialEq, Eq)] pub enum Resource { @@ -15,9 +17,15 @@ pub enum Resource { } /// Parse a `ResourceDescriptor` buffer into a list of resources. -pub fn resource_descriptor_list(descriptor: WrappedObject) -> Result, AmlError> { +// +// PILOT-DECISION: takes `` for the error type. Returns Vec +// allocated through Global since the descriptors themselves are small fixed- +// size enums. Switching to Vec is a follow-up if needed. +pub fn resource_descriptor_list( + descriptor: WrappedObject, +) -> Result, AmlError> { if let Object::Buffer(ref bytes) = *descriptor { - let mut descriptors = Vec::new(); + let mut descriptors = alloc::vec::Vec::new(); let mut bytes = bytes.as_slice(); while !bytes.is_empty() { @@ -37,7 +45,7 @@ pub fn resource_descriptor_list(descriptor: WrappedObject) -> Result Result<(Option, &[u8]), AmlError> { +fn resource_descriptor(bytes: &[u8]) -> Result<(Option, &[u8]), AmlError> { /* * If bit 7 of Byte 0 is set, it's a large descriptor. If not, it's a small descriptor. */ @@ -79,10 +87,10 @@ fn resource_descriptor(bytes: &[u8]) -> Result<(Option, &[u8]), AmlErr 0x04 => unimplemented!("Vendor-defined Descriptor"), 0x05 => unimplemented!("32-bit Memory Range Descriptor"), 0x06 => fixed_memory_descriptor(descriptor_bytes), - 0x07 => address_space_descriptor::(descriptor_bytes), - 0x08 => address_space_descriptor::(descriptor_bytes), + 0x07 => address_space_descriptor::(descriptor_bytes), + 0x08 => address_space_descriptor::(descriptor_bytes), 0x09 => extended_interrupt_descriptor(descriptor_bytes), - 0x0a => address_space_descriptor::(descriptor_bytes), + 0x0a => address_space_descriptor::(descriptor_bytes), 0x0b => unimplemented!("Extended Address Space Descriptor"), 0x0c => unimplemented!("GPIO Connection Descriptor"), 0x0d => unimplemented!("Pin Function Descriptor"), @@ -185,7 +193,7 @@ pub enum MemoryRangeDescriptor { FixedLocation { is_writable: bool, base_address: u32, range_length: u32 }, } -fn fixed_memory_descriptor(bytes: &[u8]) -> Result { +fn fixed_memory_descriptor(bytes: &[u8]) -> Result> { /* * -- 32-bit Fixed Memory Descriptor --- * Offset Field Name Definition @@ -219,7 +227,7 @@ fn fixed_memory_descriptor(bytes: &[u8]) -> Result { Ok(Resource::MemoryRange(MemoryRangeDescriptor::FixedLocation { is_writable, base_address, range_length })) } -fn address_space_descriptor(bytes: &[u8]) -> Result { +fn address_space_descriptor(bytes: &[u8]) -> Result> { /* * WORD Address Space Descriptor Definition * Note: The definitions for DWORD and QWORD are the same other than the width of the address fields. @@ -320,7 +328,7 @@ pub struct IrqDescriptor { pub irq: u32, } -fn irq_format_descriptor(bytes: &[u8]) -> Result { +fn irq_format_descriptor(bytes: &[u8]) -> Result> { /* * IRQ Descriptor Definition * @@ -423,7 +431,7 @@ pub struct DMADescriptor { pub transfer_type_preference: DMATransferTypePreference, } -pub fn dma_format_descriptor(bytes: &[u8]) -> Result { +pub fn dma_format_descriptor(bytes: &[u8]) -> Result> { /* * DMA Descriptor Definition * Offset Field Name @@ -479,7 +487,7 @@ pub struct IOPortDescriptor { pub range_length: u8, } -fn io_port_descriptor(bytes: &[u8]) -> Result { +fn io_port_descriptor(bytes: &[u8]) -> Result> { /* * I/O Port Descriptor Definition * Offset Field Name Definition @@ -513,7 +521,7 @@ fn io_port_descriptor(bytes: &[u8]) -> Result { Ok(Resource::IOPort(IOPortDescriptor { decodes_full_address, memory_range, base_alignment, range_length })) } -fn extended_interrupt_descriptor(bytes: &[u8]) -> Result { +fn extended_interrupt_descriptor(bytes: &[u8]) -> Result> { /* * --- Extended Interrupt Descriptor --- * Byte 3 contains the Interrupt Vector Flags: @@ -551,7 +559,8 @@ fn extended_interrupt_descriptor(bytes: &[u8]) -> Result { #[cfg(test)] mod tests { use super::*; - use alloc::sync::Arc; + use alloc::alloc::Global; + use alloc::vec::Vec; #[test] fn test_parses_keyboard_crs() { @@ -585,7 +594,7 @@ mod tests { ] .to_vec(); - let value = Object::Buffer(bytes).wrap(); + let value = Object::Buffer(bytes).wrap(Global); let resources = resource_descriptor_list(value).unwrap(); assert_eq!( @@ -695,7 +704,7 @@ mod tests { ] .to_vec(); - let value = Object::Buffer(bytes).wrap(); + let value = Object::Buffer(bytes).wrap(Global); let resources = resource_descriptor_list(value).unwrap(); assert_eq!( @@ -796,7 +805,7 @@ mod tests { ] .to_vec(); - let value = Object::Buffer(bytes).wrap(); + let value = Object::Buffer(bytes).wrap(Global); let resources = resource_descriptor_list(value).unwrap(); assert_eq!( diff --git a/src/lib.rs b/src/lib.rs index 9a3c8333..ea30ffe8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -44,6 +44,7 @@ #![no_std] #![feature(allocator_api)] +#![feature(btreemap_alloc)] #[cfg_attr(test, macro_use)] #[cfg(test)] @@ -256,6 +257,12 @@ pub unsafe trait AcpiTable { } } +// PILOT-FOLLOWUP: AcpiError needs to grow `` (the AML variant carries +// `aml::AmlError`). The cleanest shape is probably an unconditional +// `` with `PhantomData` for the !aml +// case, but `alloc::alloc::Global` is only available when the `alloc` +// feature is on — meaning the default itself must be cfg-gated. Deferred +// to a focused lib.rs pass once mod.rs settles. #[derive(Clone, Debug)] #[non_exhaustive] pub enum AcpiError { @@ -279,7 +286,7 @@ pub enum AcpiError { Timeout, #[cfg(feature = "aml")] - Aml(aml::AmlError), + Aml(aml::AmlError), /// This is emitted to signal that the library does not support the requested behaviour. This /// should eventually never be emitted. @@ -467,8 +474,14 @@ pub trait Handler: Clone { /// /// AML mutexes are **reentrant** - that is, a thread may acquire the same mutex more than once /// without causing a deadlock. + // PILOT-FOLLOWUP: Handler trait methods that touch AML types now pin + // to `aml::AmlError` and `aml::Object` because the + // Handler trait itself isn't generic on A. The interpreter call sites + // need to convert between A and Global on these boundaries. A fuller + // design might split the trait into a base + an `AmlHandler` trait, + // but that's a larger API change. #[cfg(feature = "aml")] - fn acquire(&self, mutex: Handle, timeout: u16) -> Result<(), aml::AmlError>; + fn acquire(&self, mutex: Handle, timeout: u16) -> Result<(), aml::AmlError>; #[cfg(feature = "aml")] fn release(&self, mutex: Handle); @@ -476,7 +489,7 @@ pub trait Handler: Clone { fn breakpoint(&self) {} #[cfg(feature = "aml")] - fn handle_debug(&self, _object: &aml::object::Object) {} + fn handle_debug(&self, _object: &aml::object::Object) {} #[cfg(feature = "aml")] fn handle_fatal_error(&self, fatal_type: u8, fatal_code: u32, fatal_arg: u64) { diff --git a/tests/bank_fields.rs b/tests/bank_fields.rs index b056f7a2..5c480743 100644 --- a/tests/bank_fields.rs +++ b/tests/bank_fields.rs @@ -68,4 +68,4 @@ fn test_basic_bank_store_and_load() { let handler = construct_std_handler(EXPECTED_COMMANDS.to_vec()); test_infra::run_aml_test(AML, handler); -} +} \ No newline at end of file diff --git a/tests/index_fields.rs b/tests/index_fields.rs index c3b9c406..7a4c5dc3 100644 --- a/tests/index_fields.rs +++ b/tests/index_fields.rs @@ -280,4 +280,4 @@ fn test_index_misaligned_field() { let handler = construct_std_handler(EXPECTED_COMMANDS.to_vec()); test_infra::run_aml_test(AML, handler); -} +} \ No newline at end of file diff --git a/tests/name_search.rs b/tests/name_search.rs index 39610ca0..4414f20d 100644 --- a/tests/name_search.rs +++ b/tests/name_search.rs @@ -44,4 +44,4 @@ DefinitionBlock("", "DSDT", 1, "RSACPI", "NAMING", 1) { let handler = NullHandler {}; test_infra::run_aml_test(AML, handler); -} +} \ No newline at end of file diff --git a/tests/normal_fields.rs b/tests/normal_fields.rs index f486731b..7b45ca86 100644 --- a/tests/normal_fields.rs +++ b/tests/normal_fields.rs @@ -106,4 +106,4 @@ fn test_unaligned_field_store() { let handler = construct_std_handler(EXPECTED_COMMANDS.to_vec()); test_infra::run_aml_test(AML, handler); -} +} \ No newline at end of file diff --git a/tests/operation_region.rs b/tests/operation_region.rs index d92e5fbf..450f04dc 100644 --- a/tests/operation_region.rs +++ b/tests/operation_region.rs @@ -178,4 +178,4 @@ fn test_region_in_pci_device() { let handler = construct_std_handler(expected_commands.to_vec()); test_infra::run_aml_test(AML, handler); -} +} \ No newline at end of file diff --git a/tests/test_infra/mod.rs b/tests/test_infra/mod.rs index 3e8751bf..f6387ae2 100644 --- a/tests/test_infra/mod.rs +++ b/tests/test_infra/mod.rs @@ -16,4 +16,4 @@ pub fn run_aml_test(asl: &'static str, handler: impl Handler) { let result = run_test_for_string(asl, interpreter, &None); assert!(matches!(result, RunTestResult::Pass(_)), "Test failed with: {:?}", TestResult::from(&result)); -} +} \ No newline at end of file diff --git a/tests/uacpi_examples.rs b/tests/uacpi_examples.rs index 0402966a..bd041612 100644 --- a/tests/uacpi_examples.rs +++ b/tests/uacpi_examples.rs @@ -188,4 +188,4 @@ DefinitionBlock("", "DSDT", 1, "RSACPI", "UACPI", 1) { let handler = NullHandler; test_infra::run_aml_test(ASL, handler); -} +} \ No newline at end of file diff --git a/tools/aml_test_tools/Cargo.toml b/tools/aml_test_tools/Cargo.toml index 73cd8c08..59c77711 100644 --- a/tools/aml_test_tools/Cargo.toml +++ b/tools/aml_test_tools/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "aml_test_tools" -version = "0.1.0" +version = "0.2.0" authors = ["Isaac Woods", "Martin Hughes"] edition = "2024" publish = false diff --git a/tools/aml_test_tools/src/handlers/check_cmd_handler.rs b/tools/aml_test_tools/src/handlers/check_cmd_handler.rs index bb5c81f5..6c34fd6c 100644 --- a/tools/aml_test_tools/src/handlers/check_cmd_handler.rs +++ b/tools/aml_test_tools/src/handlers/check_cmd_handler.rs @@ -258,7 +258,7 @@ where self.next_handler.create_mutex() } - fn acquire(&self, mutex: Handle, timeout: u16) -> Result<(), AmlError> { + fn acquire(&self, mutex: Handle, timeout: u16) -> Result<(), AmlError> { self.check_command(AcpiCommands::Acquire(mutex, timeout)); self.next_handler.acquire(mutex, timeout) } diff --git a/tools/aml_test_tools/src/handlers/listed_response_handler.rs b/tools/aml_test_tools/src/handlers/listed_response_handler.rs index 9711afcf..71558241 100644 --- a/tools/aml_test_tools/src/handlers/listed_response_handler.rs +++ b/tools/aml_test_tools/src/handlers/listed_response_handler.rs @@ -189,7 +189,7 @@ impl Handler for ListedResponseHandler { Handle(1) } - fn acquire(&self, _mutex: Handle, _timeout: u16) -> Result<(), AmlError> { + fn acquire(&self, _mutex: Handle, _timeout: u16) -> Result<(), AmlError> { check_is_skipped!(self); Ok(()) } diff --git a/tools/aml_test_tools/src/handlers/logging_handler.rs b/tools/aml_test_tools/src/handlers/logging_handler.rs index 80049e45..a082ef3c 100644 --- a/tools/aml_test_tools/src/handlers/logging_handler.rs +++ b/tools/aml_test_tools/src/handlers/logging_handler.rs @@ -187,7 +187,7 @@ where self.next_handler.create_mutex() } - fn acquire(&self, mutex: Handle, timeout: u16) -> Result<(), acpi::aml::AmlError> { + fn acquire(&self, mutex: Handle, timeout: u16) -> Result<(), acpi::aml::AmlError> { info!("acquire(mutex={:?}, timeout={})", mutex, timeout); self.next_handler.acquire(mutex, timeout) } @@ -197,7 +197,7 @@ where self.next_handler.release(mutex); } - fn handle_debug(&self, object: &Object) { + fn handle_debug(&self, object: &Object) { info!("Debug store: {}", object); self.next_handler.handle_debug(object); } diff --git a/tools/aml_test_tools/src/handlers/null_handler.rs b/tools/aml_test_tools/src/handlers/null_handler.rs index 0e6048c2..30c7ef66 100644 --- a/tools/aml_test_tools/src/handlers/null_handler.rs +++ b/tools/aml_test_tools/src/handlers/null_handler.rs @@ -90,7 +90,7 @@ impl Handler for NullHandler { Handle(0) } - fn acquire(&self, _mutex: Handle, _timeout: u16) -> Result<(), AmlError> { + fn acquire(&self, _mutex: Handle, _timeout: u16) -> Result<(), AmlError> { Ok(()) } diff --git a/tools/aml_test_tools/src/lib.rs b/tools/aml_test_tools/src/lib.rs index d1692767..45c987ea 100644 --- a/tools/aml_test_tools/src/lib.rs +++ b/tools/aml_test_tools/src/lib.rs @@ -1,4 +1,5 @@ #![feature(sync_unsafe_cell)] +#![feature(allocator_api)] //! A collection of helper utilities for testing AML using the [`acpi`] crate. //! //! These utilities are very heavily based on the way the [`acpi`] crate has used them historically. @@ -16,11 +17,12 @@ use acpi::{ Handler, PhysicalMapping, address::MappedGas, - aml::{AmlError, Interpreter, namespace::AmlName}, + aml::{AmlError, Interpreter, namespace::AmlName, object::AmlString}, sdt::{Signature, facs::Facs}, }; use log::{error, trace}; use std::{ + alloc::Global, cell::SyncUnsafeCell, ffi::OsStr, fmt::Debug, @@ -30,7 +32,6 @@ use std::{ path::PathBuf, process::Command, ptr::NonNull, - str::FromStr, sync::{Arc, atomic::AtomicU32}, }; use tempfile::{NamedTempFile, TempDir, tempdir}; @@ -42,10 +43,10 @@ where T: Handler, { /// The test passed, and the interpreter is still valid. - Pass(Interpreter), + Pass(Interpreter), /// The test failed, but the interpreter is still valid. A failure reason is also provided. - Failed(Interpreter, TestFailureReason), + Failed(Interpreter, TestFailureReason), /// The test failed, and the interpreter is no longer valid. Panicked, @@ -111,7 +112,7 @@ pub enum TestFailureReason { /// There was a problem interpreting the basic structure of the tables in the AML file. TablesErr, /// Our interpreter failed to parse or execute the resulting AML. - ParseFail(AmlError), + ParseFail(AmlError), } /// An internal-only struct that helps keep track of temporary files generated by @@ -205,11 +206,11 @@ pub fn resolve_and_compile(path: &PathBuf, can_compile: bool) -> CompilationOutc /// /// This function uses a single, static, FACS for all tests. If tests are run in parallel, this /// means they will share a single global lock. -pub fn new_interpreter(handler: T) -> Interpreter +pub fn new_interpreter(handler: T) -> Interpreter where T: Handler + Clone, { - let fake_registers = Arc::new(acpi::registers::FixedRegisters { + let fake_registers = Arc::new_in(acpi::registers::FixedRegisters { pm1_event_registers: acpi::registers::Pm1EventRegisterBlock { pm1_event_length: 8, pm1a: unsafe { @@ -243,7 +244,7 @@ where }, pm1b: None, }, - }); + }, Global); // As noted in the doc-comment, this Facs is shared between all tests - so if tests are run in // parallel, they will share a single global lock. @@ -276,7 +277,7 @@ where mapped_length: 32, handler: handler.clone(), }; - Interpreter::new(handler, 2, fake_registers, Some(fake_facs_mapping)) + Interpreter::new_in(handler, 2, fake_registers, Some(fake_facs_mapping), Global) } /// Test an ASL script given as a string, using [`run_test`]. @@ -287,7 +288,7 @@ where /// * `interpreter`: The interpreter to use for testing. pub fn run_test_for_string( asl: &'static str, - interpreter: Interpreter, + interpreter: Interpreter, expected_result: &Option, ) -> RunTestResult where @@ -311,7 +312,7 @@ where /// * `interpreter`: The interpreter to use for testing. pub fn run_test_for_file( file: &PathBuf, - interpreter: Interpreter, + interpreter: Interpreter, expected_result: &Option, ) -> RunTestResult where @@ -360,7 +361,7 @@ fn create_script_file(asl: &'static str) -> TempScriptFile { /// an inconsistent state. pub fn run_test( tables: Vec, - interpreter: Interpreter, + interpreter: Interpreter, expected_result: &Option, ) -> RunTestResult where @@ -369,7 +370,7 @@ where // Without `AssertUnwindSafe`, the following code will not build as the Interpreter is not // unwind safe. To avoid the caller being able to see an inconsistent Interpreter, if a panic // occurs we drop the Interpreter, forcing the caller to create a new one. - let result = catch_unwind(AssertUnwindSafe(|| -> Result<(), AmlError> { + let result = catch_unwind(AssertUnwindSafe(|| -> Result<(), AmlError> { // Load the DSDT table first, if there is one. if let Some(dsdt) = tables.iter().find(|t| t.header().signature == Signature::DSDT) { trace!("Loading table: DSDT"); @@ -390,14 +391,16 @@ where trace!("All tables loaded"); - if let Some(result) = interpreter.evaluate_if_present(AmlName::from_str("\\MAIN").unwrap(), vec![])? { + if let Some(result) = + interpreter.evaluate_if_present(AmlName::parse_in("\\MAIN", Global).unwrap(), Vec::new_in(Global))? + { let expected_result = expected_result.as_ref().unwrap_or(&ExpectedResult::Integer(0)); if result_matches(expected_result, &result) { Ok(()) } else { let e = format!("Unexpected MAIN result: {}, expected: {:?}", *result, expected_result); error!("{}", e); - Err(AmlError::HostError(e)) + Err(AmlError::HostError(AmlString::from_str_in(&e, Global))) } } else { Ok(()) diff --git a/tools/aml_test_tools/src/result.rs b/tools/aml_test_tools/src/result.rs index b615b944..0934be01 100644 --- a/tools/aml_test_tools/src/result.rs +++ b/tools/aml_test_tools/src/result.rs @@ -1,4 +1,5 @@ use acpi::aml::object::Object; +use core::alloc::Allocator; #[derive(Clone, Debug)] pub enum ExpectedResult { @@ -6,10 +7,11 @@ pub enum ExpectedResult { String(String), } -pub fn result_matches(expected: &ExpectedResult, actual: &Object) -> bool { +pub fn result_matches(expected: &ExpectedResult, actual: &Object) -> bool { match (expected, actual) { (ExpectedResult::Integer(expected), Object::Integer(actual)) => expected == actual, - (ExpectedResult::String(expected), Object::String(actual)) => expected == actual, + // Compare the std String against AmlString's str view. + (ExpectedResult::String(expected), Object::String(actual)) => expected.as_str() == actual.as_str(), _ => false, } } From 8023fbfe4b48a21a4c0f88f5afeb977deb4f91bf Mon Sep 17 00:00:00 2001 From: Cass Sheng Date: Wed, 13 May 2026 01:10:18 -0500 Subject: [PATCH 4/7] feat: use MethodContext for method arguments This commit introduces `MethodContext` to provide method arguments, improving code clarity and maintainability. ### Changes - [`src/aml/mod.rs`] Use MethodContext for method arguments: Use `MethodContext` to correctly pass method arguments to the interpreter - [`src/platform/interrupt.rs`] Add `hw_id` to Gic struct - [`tests/bank_fields.rs`] Update imports: Refactor imports to align with updated dependencies and improve code clarity - [`tools/aml_test_tools/src/handlers/check_cmd_handler.rs`] Prevent null check when handler is null: Ensure the `check_cmd_handler` function doesn't panic when passed a null handler ### Risk - Level: low - No new security risks introduced. - No data loss or corruption is expected. --- src/aml/mod.rs | 194 +++++++---- src/aml/namespace.rs | 31 +- src/aml/object.rs | 5 +- src/aml/resource.rs | 3 +- src/platform/interrupt.rs | 301 +++++++++++++++++- src/platform/mod.rs | 17 +- src/platform/numa.rs | 38 ++- tests/bank_fields.rs | 2 +- tests/index_fields.rs | 2 +- tests/name_search.rs | 2 +- tests/normal_fields.rs | 2 +- tests/operation_region.rs | 12 +- tests/test_infra/mod.rs | 2 +- tests/uacpi_examples.rs | 4 +- .../src/handlers/check_cmd_handler.rs | 4 +- .../src/handlers/std_test_handler.rs | 4 +- tools/aml_test_tools/src/lib.rs | 67 ++-- 17 files changed, 558 insertions(+), 132 deletions(-) diff --git a/src/aml/mod.rs b/src/aml/mod.rs index dce086e3..ca150ef9 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -212,8 +212,7 @@ where // `Arc, A>`) via `Arc::allocator(&arc)`. Saves a // signature change on `new_from_platform`. let alloc = Arc::allocator(®isters).clone(); - let interpreter = - Interpreter::new_in(platform.handler.clone(), dsdt.revision, registers, facs, alloc); + let interpreter = Interpreter::new_in(platform.handler.clone(), dsdt.revision, registers, facs, alloc); if let Err(err) = load_table(&interpreter, dsdt) { error!("Error while loading DSDT: {:?}. Continuing; this may cause downstream errors.", err); @@ -292,10 +291,16 @@ where /* * This should match the initialization order of ACPICA and uACPI. */ - if let Err(err) = self.evaluate_if_present(AmlName::parse_in("\\_INI", self.alloc.clone()).unwrap(), vec_in!(self.alloc.clone())) { + if let Err(err) = self.evaluate_if_present( + AmlName::parse_in("\\_INI", self.alloc.clone()).unwrap(), + vec_in!(self.alloc.clone()), + ) { warn!("Invoking \\_INI failed: {:?}", err); } - if let Err(err) = self.evaluate_if_present(AmlName::parse_in("\\_SB._INI", self.alloc.clone()).unwrap(), vec_in!(self.alloc.clone())) { + if let Err(err) = self.evaluate_if_present( + AmlName::parse_in("\\_SB._INI", self.alloc.clone()).unwrap(), + vec_in!(self.alloc.clone()), + ) { warn!("Invoking \\_SB._INI failed: {:?}", err); } @@ -324,9 +329,10 @@ where | NamespaceLevelKind::Processor | NamespaceLevelKind::ThermalZone | NamespaceLevelKind::PowerResource => { - let should_initialize = match self - .evaluate_if_present(AmlName::parse_in("_STA", self.alloc.clone()).unwrap().resolve(path)?, vec_in!(self.alloc.clone())) - { + let should_initialize = match self.evaluate_if_present( + AmlName::parse_in("_STA", self.alloc.clone()).unwrap().resolve(path)?, + vec_in!(self.alloc.clone()), + ) { Ok(Some(result)) => { let Object::Integer(result) = *result else { panic!() }; let status = DeviceStatus(result); @@ -341,9 +347,10 @@ where if should_initialize { num_devices_initialized += 1; - if let Err(err) = - self.evaluate_if_present(AmlName::parse_in("_INI", self.alloc.clone()).unwrap().resolve(path)?, vec_in!(self.alloc.clone())) - { + if let Err(err) = self.evaluate_if_present( + AmlName::parse_in("_INI", self.alloc.clone()).unwrap().resolve(path)?, + vec_in!(self.alloc.clone()), + ) { warn!("Failed to evaluate _INI for device {}: {:?}", path, err); } Ok(true) @@ -534,7 +541,8 @@ where }; *operand = new_value; - context.contribute_arg(Argument::Object(Object::Integer(new_value).wrap(self.alloc.clone()))); + context + .contribute_arg(Argument::Object(Object::Integer(new_value).wrap(self.alloc.clone()))); context.retire_op(op); } Opcode::LAnd @@ -695,7 +703,9 @@ where length: region_length.as_integer()?, parent_device_path: context.current_scope.clone(), }); - self.namespace.lock().insert(name.resolve(&context.current_scope)?, region.wrap(self.alloc.clone()))?; + self.namespace + .lock() + .insert(name.resolve(&context.current_scope)?, region.wrap(self.alloc.clone()))?; context.retire_op(op); } Opcode::DataRegion => { @@ -720,7 +730,9 @@ where length: 0, parent_device_path: context.current_scope.clone(), }); - self.namespace.lock().insert(name.resolve(&context.current_scope)?, region.wrap(self.alloc.clone()))?; + self.namespace + .lock() + .insert(name.resolve(&context.current_scope)?, region.wrap(self.alloc.clone()))?; context.retire_op(op); } Opcode::Buffer => { @@ -768,7 +780,8 @@ where assert_eq!(context.current_block.kind, BlockKind::::Package); assert_eq!(context.peek(), Err::>(AmlError::RunOutOfStream)); context.current_block = context.block_stack.pop().unwrap(); - context.contribute_arg(Argument::Object(Object::Package(elements).wrap(self.alloc.clone()))); + context + .contribute_arg(Argument::Object(Object::Package(elements).wrap(self.alloc.clone()))); context.retire_op(op); } Opcode::VarPackage => { @@ -794,7 +807,8 @@ where assert_eq!(context.current_block.kind, BlockKind::::VarPackage); assert_eq!(context.peek(), Err::>(AmlError::RunOutOfStream)); context.current_block = context.block_stack.pop().unwrap(); - context.contribute_arg(Argument::Object(Object::Package(elements).wrap(self.alloc.clone()))); + context + .contribute_arg(Argument::Object(Object::Package(elements).wrap(self.alloc.clone()))); context.retire_op(op); } Opcode::If => { @@ -847,7 +861,8 @@ where }; self.namespace.lock().insert( name.resolve(&context.current_scope)?, - Object::BufferField { buffer: buffer.clone(), offset: offset as usize, length }.wrap(self.alloc.clone()), + Object::BufferField { buffer: buffer.clone(), offset: offset as usize, length } + .wrap(self.alloc.clone()), )?; context.retire_op(op); } @@ -880,8 +895,8 @@ where } Opcode::RefOf => { extract_args!(op, self.alloc.clone() => [Argument::Object(object)]); - let reference = - Object::Reference { kind: ReferenceKind::RefOf, inner: object.clone() }.wrap(self.alloc.clone()); + let reference = Object::Reference { kind: ReferenceKind::RefOf, inner: object.clone() } + .wrap(self.alloc.clone()); context.contribute_arg(Argument::Object(reference)); context.retire_op(op); } @@ -891,7 +906,8 @@ where Object::Integer(0) } else { let reference = - Object::Reference { kind: ReferenceKind::RefOf, inner: object.clone() }.wrap(self.alloc.clone()); + Object::Reference { kind: ReferenceKind::RefOf, inner: object.clone() } + .wrap(self.alloc.clone()); self.do_store(target.clone(), reference)?; Object::Integer(u64::MAX) }; @@ -999,8 +1015,12 @@ where .lock() .add_level(method_scope.clone(), NamespaceLevelKind::MethodLocals)?; - let new_context = - MethodContext::new_from_method(method.clone(), args, method_scope.clone(), self.alloc.clone())?; + let new_context = MethodContext::new_from_method( + method.clone(), + args, + method_scope.clone(), + self.alloc.clone(), + )?; let old_context = mem::replace(&mut context, new_context); context_stack.push(old_context); context.retire_op(op); @@ -1060,7 +1080,9 @@ where } } - context.contribute_arg(Argument::Object(Object::Integer(object_type(&object)).wrap(self.alloc.clone()))); + context.contribute_arg(Argument::Object( + Object::Integer(object_type(&object)).wrap(self.alloc.clone()), + )); context.retire_op(op); } Opcode::SizeOf => self.do_size_of(&mut context, op)?, @@ -1158,7 +1180,9 @@ where { let num_elements_left = package_op.expected_arguments - package_op.arguments.len(); for _ in 0..num_elements_left { - package_op.arguments.push(Argument::Object(Object::Uninitialized.wrap(self.alloc.clone()))); + package_op + .arguments + .push(Argument::Object(Object::Uninitialized.wrap(self.alloc.clone()))); } } @@ -1186,7 +1210,9 @@ where }; for _ in 0..num_elements_left { - package_op.arguments.push(Argument::Object(Object::Uninitialized.wrap(self.alloc.clone()))); + package_op + .arguments + .push(Argument::Object(Object::Uninitialized.wrap(self.alloc.clone()))); } } @@ -1264,15 +1290,18 @@ where } Opcode::BytePrefix => { let value = context.next()?; - context.contribute_arg(Argument::Object(Object::Integer(value as u64).wrap(self.alloc.clone()))); + context + .contribute_arg(Argument::Object(Object::Integer(value as u64).wrap(self.alloc.clone()))); } Opcode::WordPrefix => { let value = context.next_u16()?; - context.contribute_arg(Argument::Object(Object::Integer(value as u64).wrap(self.alloc.clone()))); + context + .contribute_arg(Argument::Object(Object::Integer(value as u64).wrap(self.alloc.clone()))); } Opcode::DWordPrefix => { let value = context.next_u32()?; - context.contribute_arg(Argument::Object(Object::Integer(value as u64).wrap(self.alloc.clone()))); + context + .contribute_arg(Argument::Object(Object::Integer(value as u64).wrap(self.alloc.clone()))); } Opcode::StringPrefix => { let str_start = context.current_block.pc; @@ -1376,13 +1405,18 @@ where let name = name.resolve(&context.current_scope)?; let mutex = self.handler.create_mutex(); - self.namespace.lock().insert(name, Object::Mutex { mutex, sync_level }.wrap(self.alloc.clone()))?; + self.namespace + .lock() + .insert(name, Object::Mutex { mutex, sync_level }.wrap(self.alloc.clone()))?; } Opcode::Event => { let name = context.namestring()?; let name = name.resolve(&context.current_scope)?; - self.namespace.lock().insert(name, Object::Event(Arc::new_in(AtomicU64::new(0), self.alloc.clone())).wrap(self.alloc.clone()))?; + self.namespace.lock().insert( + name, + Object::Event(Arc::new_in(AtomicU64::new(0), self.alloc.clone())).wrap(self.alloc.clone()), + )?; } Opcode::LoadTable => { context.start(self.new_op(Opcode::LoadTable, &[ResolveBehaviour::TermArg; 6])); @@ -1400,16 +1434,20 @@ where Opcode::Acquire => context.start(self.new_op(opcode, &[ResolveBehaviour::SuperName])), Opcode::Release => context.start(self.new_op(opcode, &[ResolveBehaviour::SuperName])), Opcode::Signal => context.start(self.new_op(opcode, &[ResolveBehaviour::SuperName])), - Opcode::Wait => context - .start(self.new_op(opcode, &[ResolveBehaviour::SuperName, ResolveBehaviour::TermArg])), + Opcode::Wait => { + context.start(self.new_op(opcode, &[ResolveBehaviour::SuperName, ResolveBehaviour::TermArg])) + } Opcode::Reset => context.start(self.new_op(opcode, &[ResolveBehaviour::SuperName])), - Opcode::Notify => context - .start(self.new_op(opcode, &[ResolveBehaviour::SuperName, ResolveBehaviour::TermArg])), + Opcode::Notify => { + context.start(self.new_op(opcode, &[ResolveBehaviour::SuperName, ResolveBehaviour::TermArg])) + } Opcode::FromBCD | Opcode::ToBCD => { context.start(self.new_op(opcode, &[ResolveBehaviour::TermArg, ResolveBehaviour::Target])) } Opcode::Revision => { - context.contribute_arg(Argument::Object(Object::Integer(INTERPRETER_REVISION).wrap(self.alloc.clone()))); + context.contribute_arg(Argument::Object( + Object::Integer(INTERPRETER_REVISION).wrap(self.alloc.clone()), + )); } Opcode::Debug => context.contribute_arg(Argument::Object(Object::Debug.wrap(self.alloc.clone()))), Opcode::Fatal => { @@ -1598,19 +1636,14 @@ where Object::Reference { kind: ReferenceKind::Arg, inner: arg }.wrap(self.alloc.clone()), )); } - Opcode::Store => context.start(self.new_op( - Opcode::Store, - &[ResolveBehaviour::TermArg, ResolveBehaviour::SuperName], - )), - Opcode::CopyObject => context.start(self.new_op( - Opcode::CopyObject, - &[ResolveBehaviour::TermArg, ResolveBehaviour::SimpleName], - )), + Opcode::Store => context + .start(self.new_op(Opcode::Store, &[ResolveBehaviour::TermArg, ResolveBehaviour::SuperName])), + Opcode::CopyObject => context.start( + self.new_op(Opcode::CopyObject, &[ResolveBehaviour::TermArg, ResolveBehaviour::SimpleName]), + ), Opcode::RefOf => context.start(self.new_op(Opcode::RefOf, &[ResolveBehaviour::SuperName])), - Opcode::CondRefOf => context.start(self.new_op( - opcode, - &[ResolveBehaviour::SuperNameIfExists, ResolveBehaviour::Target], - )), + Opcode::CondRefOf => context + .start(self.new_op(opcode, &[ResolveBehaviour::SuperNameIfExists, ResolveBehaviour::Target])), Opcode::DualNamePrefix | Opcode::MultiNamePrefix @@ -1633,7 +1666,8 @@ where match object { Ok((_resolved_name, object)) => { context.contribute_arg(Argument::Object( - Object::Reference { kind: ReferenceKind::Named, inner: object }.wrap(self.alloc.clone()), + Object::Reference { kind: ReferenceKind::Named, inner: object } + .wrap(self.alloc.clone()), )); } Err(err) => Err(err)?, @@ -1686,7 +1720,8 @@ where let value = self.do_field_read(field)?; context.contribute_arg(Argument::Object(value)); } else if let Object::BufferField { .. } = *object { - let value = object.read_buffer_field(self.integer_size(), self.alloc.clone())?; + let value = + object.read_buffer_field(self.integer_size(), self.alloc.clone())?; context.contribute_arg(Argument::Object(value.wrap(self.alloc.clone()))); } else { context.contribute_arg(Argument::Object(object)); @@ -1699,7 +1734,9 @@ where let mut name_str = AmlString::new_in(self.alloc.clone()); use core::fmt::Write; write!(name_str, "{}", name).unwrap(); - context.contribute_arg(Argument::Object(Object::String(name_str).wrap(self.alloc.clone()))); + context.contribute_arg(Argument::Object( + Object::String(name_str).wrap(self.alloc.clone()), + )); } ResolveBehaviour::Placeholder => { panic!("Invalid resolve behaviour for name to be resolved!") @@ -1737,8 +1774,9 @@ where Opcode::Increment | Opcode::Decrement => { context.start(self.new_op(opcode, &[ResolveBehaviour::SuperName])) } - Opcode::Not => context - .start(self.new_op(Opcode::Not, &[ResolveBehaviour::TermArg, ResolveBehaviour::Target])), + Opcode::Not => { + context.start(self.new_op(Opcode::Not, &[ResolveBehaviour::TermArg, ResolveBehaviour::Target])) + } Opcode::FindSetLeftBit | Opcode::FindSetRightBit => { context.start(self.new_op(opcode, &[ResolveBehaviour::TermArg, ResolveBehaviour::Target])) } @@ -1765,9 +1803,7 @@ where | Opcode::CreateByteField | Opcode::CreateWordField | Opcode::CreateDWordField - | Opcode::CreateQWordField => { - context.start(self.new_op(opcode, &[ResolveBehaviour::TermArg; 2])) - } + | Opcode::CreateQWordField => context.start(self.new_op(opcode, &[ResolveBehaviour::TermArg; 2])), Opcode::CreateField => { context.start(self.new_op(Opcode::CreateField, &[ResolveBehaviour::TermArg; 3])) } @@ -1922,7 +1958,9 @@ where bit_length: field_length, flags: FieldFlags(flags), }); - self.namespace.lock().insert(field_name.resolve(&context.current_scope)?, field.wrap(self.alloc.clone()))?; + self.namespace + .lock() + .insert(field_name.resolve(&context.current_scope)?, field.wrap(self.alloc.clone()))?; field_offset += field_length; } @@ -1945,7 +1983,10 @@ where Opcode::Multiply => left.wrapping_mul(right), Opcode::Divide => { if let Some(Argument::Object(remainder)) = target2 { - self.do_store(remainder.clone(), Object::Integer(left.wrapping_rem(right)).wrap(self.alloc.clone()))?; + self.do_store( + remainder.clone(), + Object::Integer(left.wrapping_rem(right)).wrap(self.alloc.clone()), + )?; } left.wrapping_div_euclid(right) } @@ -2466,7 +2507,11 @@ where /// object is overwritten /// - Index references behave the same as locals /// - Named objects are stored into, with implicit casting - fn do_store(&self, target: WrappedObject, object: WrappedObject) -> Result, AmlError> { + fn do_store( + &self, + target: WrappedObject, + object: WrappedObject, + ) -> Result, AmlError> { let object = object.unwrap_transparent_reference(); let token = self.object_token.lock(); @@ -2656,7 +2701,8 @@ where let Object::FieldUnit(ref data) = **data else { panic!() }; self.do_field_write( index, - Object::Integer((index_field_idx + i * (access_width_bits / 8)) as u64).wrap(self.alloc.clone()), + Object::Integer((index_field_idx + i * (access_width_bits / 8)) as u64) + .wrap(self.alloc.clone()), )?; // The offset is always that of the data register, as we always read from the @@ -2737,7 +2783,8 @@ where let Object::FieldUnit(ref data) = **data else { panic!() }; self.do_field_write( index, - Object::Integer((index_field_idx + i * (access_width_bits / 8)) as u64).wrap(self.alloc.clone()), + Object::Integer((index_field_idx + i * (access_width_bits / 8)) as u64) + .wrap(self.alloc.clone()), )?; // The offset is always that of the data register, as we always read from the @@ -2787,7 +2834,12 @@ where /// Performs an actual read from an operation region. `offset` and `length` must respect the /// access requirements of the field being read, and are supplied in **bytes**. This may call /// AML methods if required, and may invoke user-supplied handlers. - fn do_native_region_read(&self, region: &OpRegion, offset: usize, length: usize) -> Result> { + fn do_native_region_read( + &self, + region: &OpRegion, + offset: usize, + length: usize, + ) -> Result> { trace!("Native field read. Region = {:?}, offset = {:#x}, length={:#x}", region, offset, length); match region.space { @@ -2913,16 +2965,25 @@ where * TODO: it's not ideal to do these reads for every native access. See if we can * cache them somewhere? */ - let seg = match self.evaluate_if_present(AmlName::parse_in("_SEG", self.alloc.clone()).unwrap().resolve(path)?, vec_in!(self.alloc.clone()))? { + let seg = match self.evaluate_if_present( + AmlName::parse_in("_SEG", self.alloc.clone()).unwrap().resolve(path)?, + vec_in!(self.alloc.clone()), + )? { Some(value) => value.as_integer()?, None => 0, }; - let bus = match self.evaluate_if_present(AmlName::parse_in("_BBN", self.alloc.clone()).unwrap().resolve(path)?, vec_in!(self.alloc.clone()))? { + let bus = match self.evaluate_if_present( + AmlName::parse_in("_BBN", self.alloc.clone()).unwrap().resolve(path)?, + vec_in!(self.alloc.clone()), + )? { Some(value) => value.as_integer()?, None => 0, }; let (device, function) = { - let adr = self.evaluate_if_present(AmlName::parse_in("_ADR", self.alloc.clone()).unwrap().resolve(path)?, vec_in!(self.alloc.clone()))?; + let adr = self.evaluate_if_present( + AmlName::parse_in("_ADR", self.alloc.clone()).unwrap().resolve(path)?, + vec_in!(self.alloc.clone()), + )?; let adr = match adr { Some(adr) => adr.as_integer()?, None => 0, @@ -3762,10 +3823,9 @@ impl PartialEq> for Am (LevelDoesNotExist(a), LevelDoesNotExist(b)) => a == b, (NameCollision(a), NameCollision(b)) => a == b, (ObjectDoesNotExist(a), ObjectDoesNotExist(b)) => a == b, - ( - InvalidOperationOnObject { op: o1, typ: t1 }, - InvalidOperationOnObject { op: o2, typ: t2 }, - ) => o1 == o2 && t1 == t2, + (InvalidOperationOnObject { op: o1, typ: t1 }, InvalidOperationOnObject { op: o2, typ: t2 }) => { + o1 == o2 && t1 == t2 + } ( ObjectNotOfExpectedType { expected: e1, got: g1 }, ObjectNotOfExpectedType { expected: e2, got: g2 }, diff --git a/src/aml/namespace.rs b/src/aml/namespace.rs index dc694b8c..e4410ef6 100644 --- a/src/aml/namespace.rs +++ b/src/aml/namespace.rs @@ -29,14 +29,26 @@ impl Namespace { where A: 'static, { - let mut namespace = - Namespace { alloc: alloc.clone(), root: NamespaceLevel::new_in(NamespaceLevelKind::Scope, alloc.clone()) }; + let mut namespace = Namespace { + alloc: alloc.clone(), + root: NamespaceLevel::new_in(NamespaceLevelKind::Scope, alloc.clone()), + }; - namespace.add_level(AmlName::parse_in("\\_GPE", alloc.clone()).unwrap(), NamespaceLevelKind::Scope).unwrap(); - namespace.add_level(AmlName::parse_in("\\_SB", alloc.clone()).unwrap(), NamespaceLevelKind::Scope).unwrap(); - namespace.add_level(AmlName::parse_in("\\_SI", alloc.clone()).unwrap(), NamespaceLevelKind::Scope).unwrap(); - namespace.add_level(AmlName::parse_in("\\_PR", alloc.clone()).unwrap(), NamespaceLevelKind::Scope).unwrap(); - namespace.add_level(AmlName::parse_in("\\_TZ", alloc.clone()).unwrap(), NamespaceLevelKind::Scope).unwrap(); + namespace + .add_level(AmlName::parse_in("\\_GPE", alloc.clone()).unwrap(), NamespaceLevelKind::Scope) + .unwrap(); + namespace + .add_level(AmlName::parse_in("\\_SB", alloc.clone()).unwrap(), NamespaceLevelKind::Scope) + .unwrap(); + namespace + .add_level(AmlName::parse_in("\\_SI", alloc.clone()).unwrap(), NamespaceLevelKind::Scope) + .unwrap(); + namespace + .add_level(AmlName::parse_in("\\_PR", alloc.clone()).unwrap(), NamespaceLevelKind::Scope) + .unwrap(); + namespace + .add_level(AmlName::parse_in("\\_TZ", alloc.clone()).unwrap(), NamespaceLevelKind::Scope) + .unwrap(); namespace .insert( @@ -347,7 +359,10 @@ impl Namespace { /// Split an absolute path into a bunch of level segments (used to traverse the level data structure), and a /// last segment to index into that level. This must not be called on `\\`. - fn get_level_for_path_mut(&mut self, path: &AmlName) -> Result<(&mut NamespaceLevel, NameSeg), AmlError> { + fn get_level_for_path_mut( + &mut self, + path: &AmlName, + ) -> Result<(&mut NamespaceLevel, NameSeg), AmlError> { assert_ne!(*path, AmlName::root_in(self.alloc.clone())); let (last_seg, levels) = path.0[1..].split_last().unwrap(); diff --git a/src/aml/object.rs b/src/aml/object.rs index 06d6f59e..d141493a 100644 --- a/src/aml/object.rs +++ b/src/aml/object.rs @@ -528,7 +528,10 @@ pub struct FieldUnit { impl fmt::Debug for FieldUnit { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("FieldUnit").field("flags", &self.flags).field("bit_length", &self.bit_length).finish_non_exhaustive() + f.debug_struct("FieldUnit") + .field("flags", &self.flags) + .field("bit_length", &self.bit_length) + .finish_non_exhaustive() } } diff --git a/src/aml/resource.rs b/src/aml/resource.rs index 2c496c97..e4d84edc 100644 --- a/src/aml/resource.rs +++ b/src/aml/resource.rs @@ -559,8 +559,7 @@ fn extended_interrupt_descriptor(bytes: &[u8]) -> Result { /// XAPIC, or X2APIC). These are likely to be found on x86 and x86_64 systems and are made up of a Local APIC /// for each core and one or more I/O APICs to handle external interrupts. Apic(Apic), + + /// Describes an interrupt controller based around the ARM Generic Interrupt Controller (GIC) — versions 1 + /// through 4. Found on `aarch64` systems. Composed of a single Distributor (GICD), one CPU Interface (GICC) + /// per core, optional Redistributor discovery ranges (GICv3+), optional Interrupt Translation Service blocks + /// (GICv3+, for LPI / MSI), and optional MSI frames (GICv2m, for legacy SPI-based MSI). + Gic(Gic), } impl InterruptModel { @@ -64,7 +70,9 @@ impl InterruptModel { | MadtEntry::Gicd(_) | MadtEntry::GicMsiFrame(_) | MadtEntry::GicRedistributor(_) - | MadtEntry::GicInterruptTranslationService(_) => {} + | MadtEntry::GicInterruptTranslationService(_) => { + return Self::from_gic_model_in(madt.get(), allocator); + } MadtEntry::NmiSource(_) => (), MadtEntry::MultiprocessorWakeup(_) => (), @@ -124,7 +132,7 @@ impl InterruptModel { let processor = Processor { processor_uid: entry.processor_id as u32, - local_apic_id: entry.apic_id as u32, + hw_id: entry.apic_id as u64, state, is_ap, }; @@ -148,7 +156,7 @@ impl InterruptModel { let processor = Processor { processor_uid: entry.processor_uid, - local_apic_id: entry.x2apic_id, + hw_id: entry.x2apic_id as u64, state, is_ap, }; @@ -194,6 +202,7 @@ impl InterruptModel { } MadtEntry::LocalApicNmi(entry) => { + let (polarity, trigger_mode) = parse_mps_inti_flags(entry.flags)?; local_apic_nmi_lines.push(NmiLine { processor: if entry.processor_id == 0xff { NmiProcessor::All @@ -205,10 +214,13 @@ impl InterruptModel { 1 => LocalInterruptLine::Lint1, _ => return Err(AcpiError::InvalidMadt(MadtError::InvalidLocalNmiLine)), }, + polarity, + trigger_mode, }); } MadtEntry::X2ApicNmi(entry) => { + let (polarity, trigger_mode) = parse_mps_inti_flags(entry.flags)?; local_apic_nmi_lines.push(NmiLine { processor: if entry.processor_uid == 0xffffffff { NmiProcessor::All @@ -220,6 +232,8 @@ impl InterruptModel { 1 => LocalInterruptLine::Lint1, _ => return Err(AcpiError::InvalidMadt(MadtError::InvalidLocalNmiLine)), }, + polarity, + trigger_mode, }); } @@ -247,6 +261,148 @@ impl InterruptModel { Some(ProcessorInfo::new_in(boot_processor.unwrap(), application_processors)), )) } + + fn from_gic_model_in( + madt: Pin<&Madt>, + allocator: A, + ) -> Result<(InterruptModel, Option>), AcpiError> { + // Revision drives optional GICC fields like TRBE Interrupt + // (added in ACPI 6.6). Captured once so each entry can do + // its own version-gated reads consistently. + let revision = madt.header.revision; + + // First pass: count each entry kind so we can `with_capacity_in` + // exactly and avoid any growth-time reallocation. + let mut gicd_count = 0usize; + let mut gicc_count = 0usize; + let mut gicr_count = 0usize; + let mut gic_msi_count = 0usize; + let mut gic_its_count = 0usize; + for entry in madt.entries() { + match entry { + MadtEntry::Gicd(_) => gicd_count += 1, + MadtEntry::Gicc(_) => gicc_count += 1, + MadtEntry::GicRedistributor(_) => gicr_count += 1, + MadtEntry::GicMsiFrame(_) => gic_msi_count += 1, + MadtEntry::GicInterruptTranslationService(_) => gic_its_count += 1, + _ => (), + } + } + + let mut distributors = Vec::with_capacity_in(gicd_count, allocator.clone()); + let mut cpu_interfaces = Vec::with_capacity_in(gicc_count, allocator.clone()); + let mut redistributor_ranges = Vec::with_capacity_in(gicr_count, allocator.clone()); + let mut msi_frames = Vec::with_capacity_in(gic_msi_count, allocator.clone()); + let mut its_blocks = Vec::with_capacity_in(gic_its_count, allocator.clone()); + + // `application_processors` is reserved for AP entries only; + // the BSP slot is filled in by `boot_processor`. Real + // hardware tags exactly one GICC as the BSP via the boot + // bring-up convention (first GICC in MADT order, by ACPI 6.x + // spec text), but we don't have a reliable in-table flag — + // mirror the APIC path's "first one wins" convention. + let mut application_processors = Vec::with_capacity_in(gicc_count.saturating_sub(1), allocator); + let mut boot_processor: Option = None; + + for entry in madt.entries() { + match entry { + MadtEntry::Gicd(g) => distributors.push(GicDistributor { + gic_id: g.gic_id, + physical_base_address: g.physical_base_address, + system_vector_base: g.system_vector_base, + gic_version: g.gic_version, + // (All u32/u64/u8 — Copy through field access in + // packed structs is fine; only references / method + // dispatch need an aligned copy first.) + }), + + MadtEntry::Gicc(g) => { + // `GiccEntry` is `#[repr(C, packed)]` — copy + // every used field to an aligned local before + // any method dispatch / reference taking. + let processor_uid = g.processor_uid; + let mpidr = g.mpidr; + let flags = g.flags; + let cpu_interface_number = g.cpu_interface_number; + let gic_registers_address = g.gic_registers_address; + let gicr_base_address = g.gicr_base_address; + let performance_interrupt_gsiv = g.performance_interrupt_gsiv; + let vgic_maintenance_interrupt = g.vgic_maintenance_interrupt; + let spe_overflow_interrupt = g.spe_overflow_interrupt; + let trbe_interrupt_field = g.trbe_interrupt; + + let is_disabled = flags & 1 == 0; + let is_ap = boot_processor.is_some(); + let state = match (is_ap, is_disabled) { + (_, true) => ProcessorState::Disabled, + (true, false) => ProcessorState::WaitingForSipi, + (false, false) => ProcessorState::Running, + }; + + let processor = Processor { processor_uid, hw_id: mpidr, state, is_ap }; + if is_ap { + application_processors.push(processor); + } else { + boot_processor = Some(processor); + } + + // SAFETY: `ExtendedField::access` is unsafe iff a + // bogus revision is passed; we feed it the MADT's + // own header revision, which is authoritative. + let trbe_interrupt = unsafe { trbe_interrupt_field.access(revision) }; + + cpu_interfaces.push(GicCpuInterface { + cpu_interface_number, + processor_uid, + mpidr, + gic_registers_address, + gicr_base_address, + performance_interrupt_gsiv, + vgic_maintenance_interrupt, + spe_overflow_interrupt, + trbe_interrupt, + flags, + }); + } + + MadtEntry::GicRedistributor(r) => redistributor_ranges.push(GicRedistributorRange { + discovery_range_base_address: r.discovery_range_base_address, + discovery_range_length: r.discovery_range_length, + }), + + MadtEntry::GicMsiFrame(m) => msi_frames.push(GicMsiFrame { + frame_id: m.frame_id, + physical_base_address: m.physical_base_address, + flags: m.flags, + spi_count: m.spi_count, + spi_base: m.spi_base, + }), + + MadtEntry::GicInterruptTranslationService(t) => { + its_blocks.push(GicIts { id: t.id, physical_base_address: t.physical_base_address }) + } + + // GIC systems may also carry `MultiprocessorWakeup`, + // `NmiSource`, etc. — these are either x86-specific + // (handled above by the dispatch refusing to pick GIC + // when they appear) or future arch additions. Skip. + _ => (), + } + } + + let processor_info = boot_processor.map(|bsp| ProcessorInfo::new_in(bsp, application_processors)); + + Ok(( + InterruptModel::Gic(Gic::new( + distributors, + cpu_interfaces, + redistributor_ranges, + msi_frames, + its_blocks, + )), + processor_info, + )) + } } #[derive(Debug, Clone, Copy)] @@ -258,10 +414,24 @@ pub struct IoApic { pub global_system_interrupt_base: u32, } +/// Per-CPU NMI line annotation derived from MADT +/// [`LocalApicNmiEntry`](crate::sdt::madt::LocalApicNmiEntry) / +/// [`X2ApicNmiEntry`](crate::sdt::madt::X2ApicNmiEntry). Targets one +/// of the LAPIC LINT pins on a specific CPU (or all CPUs) for NMI +/// delivery, and carries the MPS INTI polarity / trigger-mode that +/// the receiver should program into its LVT entry. #[derive(Debug, Clone, Copy)] pub struct NmiLine { pub processor: NmiProcessor, pub line: LocalInterruptLine, + /// Decoded from the MADT entry's `flags` field via the same MPS + /// INTI parsing used for [`InterruptSourceOverride`] / + /// [`NmiSource`]. `Polarity::SameAsBus` and + /// `TriggerMode::SameAsBus` are the spec's "use the bus default" + /// sentinels — for a LAPIC LINT pin that means edge / active-high, + /// which is the conventional default firmware programs. + pub polarity: Polarity, + pub trigger_mode: TriggerMode, } /// Indicates which local interrupt line will be utilized by an external interrupt. Specifically, @@ -332,3 +502,128 @@ impl Apic { } } } + +// ============================================================================ +// GIC (aarch64) parsed types +// ============================================================================ + +/// Parsed view of the aarch64 GIC interrupt model. Mirrors the +/// [`Apic`] shape: per-block `Vec<_, A>` lists, allocated in the +/// caller-supplied arena. +#[derive(Debug, Clone)] +pub struct Gic { + /// GIC distributors (GICD entries). Modern systems have exactly + /// one; multi-cluster designs may have one per cluster. + pub distributors: Vec, + + /// Per-CPU GIC CPU Interface descriptions, one entry per + /// `GiccEntry` in the MADT. These also carry per-core metadata + /// the runtime needs (GICR base on GICv3+, performance interrupt + /// GSIV, vGIC maintenance interrupt, MPIDR for join). + pub cpu_interfaces: Vec, + + /// GICR discovery ranges (GICv3+ only). Each entry describes a + /// contiguous physical-address range to scan for redistributors + /// belonging to enumerated CPUs. Empty on GICv1/v2 (per-CPU + /// GICR base is reported inline on each [`GicCpuInterface`]). + pub redistributor_ranges: Vec, + + /// GICv2m MSI frames. Each frame translates writes to its + /// `MSIxxxxxxxx` register into a contiguous SPI range + /// (`spi_base..spi_base + spi_count`). Empty on GICv3+ systems + /// that route MSIs through ITS instead. + pub msi_frames: Vec, + + /// GIC Interrupt Translation Services (GICv3+ only). Each ITS + /// translates device-side MSI writes (DeviceID + EventID) into + /// LPIs delivered through the GICRs. + pub its_blocks: Vec, +} + +impl Gic { + pub(crate) fn new( + distributors: Vec, + cpu_interfaces: Vec, + redistributor_ranges: Vec, + msi_frames: Vec, + its_blocks: Vec, + ) -> Self { + Self { distributors, cpu_interfaces, redistributor_ranges, msi_frames, its_blocks } + } +} + +/// One GIC distributor. Modern systems have a single GICD covering +/// all CPUs; the `gic_version` byte selects v1/v2 vs v3/v4 register +/// layout (or `0` = "fall back to hardware discovery"). +#[derive(Debug, Clone, Copy)] +pub struct GicDistributor { + pub gic_id: u32, + pub physical_base_address: u64, + pub system_vector_base: u32, + /// `0x01`/`0x02` = GICv1/v2 (4 KiB register window). + /// `0x03`/`0x04` = GICv3/v4 (64 KiB register window). + /// `0x00` = unspecified, fall back to hardware discovery. + pub gic_version: u8, +} + +/// One GIC CPU Interface entry — i.e. one core's worth of GIC-side +/// metadata. On GICv3+ the per-CPU runtime accesses go through +/// system registers (`ICC_*_EL1`); `gic_registers_address` and +/// `gicr_base_address` are kept here for catalog completeness. +#[derive(Debug, Clone, Copy)] +pub struct GicCpuInterface { + pub cpu_interface_number: u32, + pub processor_uid: u32, + pub mpidr: u64, + /// GICv2 CPU-interface MMIO base. Zero on GICv3+. + pub gic_registers_address: u64, + /// GICv3+ redistributor (GICR) base for this CPU. Zero on + /// GICv2 (use the discovery ranges in [`Gic::redistributor_ranges`] + /// in combination with `gic_registers_address` instead). + pub gicr_base_address: u64, + /// Performance Monitor Interrupt GSIV (PPI). Zero if absent. + pub performance_interrupt_gsiv: u32, + /// vGIC Maintenance Interrupt GSIV (PPI). Zero if absent. + pub vgic_maintenance_interrupt: u32, + /// SPE Overflow Interrupt (ACPI 6.3+). Zero if absent / older + /// firmware. + pub spe_overflow_interrupt: u16, + /// TRBE Interrupt (ACPI 6.6+, MADT revision ≥ 6). `None` on + /// older firmware where the field wasn't yet defined. + pub trbe_interrupt: Option, + /// Raw GICC flags field (`enabled`, `performance_interrupt_mode`, + /// `vgic_signaled_via_pmu`, etc.). Bit 0 = enabled. + pub flags: u32, +} + +impl GicCpuInterface { + /// Convenience: is this CPU firmware-enabled? (GICC flags bit 0.) + #[inline] + pub fn is_enabled(&self) -> bool { + self.flags & 1 != 0 + } +} + +/// One GICR discovery range (GICv3+). +#[derive(Debug, Clone, Copy)] +pub struct GicRedistributorRange { + pub discovery_range_base_address: u64, + pub discovery_range_length: u32, +} + +/// One GICv2m MSI frame. +#[derive(Debug, Clone, Copy)] +pub struct GicMsiFrame { + pub frame_id: u32, + pub physical_base_address: u64, + pub flags: u32, + pub spi_count: u16, + pub spi_base: u16, +} + +/// One GIC ITS block (GICv3+). +#[derive(Debug, Clone, Copy)] +pub struct GicIts { + pub id: u32, + pub physical_base_address: u64, +} diff --git a/src/platform/mod.rs b/src/platform/mod.rs index ce16100b..fccaafb9 100644 --- a/src/platform/mod.rs +++ b/src/platform/mod.rs @@ -213,9 +213,20 @@ pub struct Processor { /// Corresponds to the `_UID` object of the processor's `Device`, or the `ProcessorId` field of the `Processor` /// object, in AML. pub processor_uid: u32, - /// The ID of the local APIC of the processor. Will be less than `256` if the APIC is being used, but can be - /// greater than this if the X2APIC is being used. - pub local_apic_id: u32, + + /// The hardware identifier of the processor. Per-arch semantics: + /// + /// - **x86 / x86_64**: the Local APIC ID. ≤ `0xff` under + /// xAPIC; up to 32 bits under x2APIC. + /// - **aarch64**: the `MPIDR_EL1` value from the GICC entry. + /// Up to 64 bits (the full Aff{0,1,2,3} encoding). + /// - **riscv64** (future): the hartid from RINTC. + /// + /// Widened to `u64` to cover MPIDR cleanly. Renamed from the + /// previous x86-specific `local_apic_id: u32` — the field always + /// represented "the processor's hardware identifier," and the + /// rename makes the per-arch generality explicit. + pub hw_id: u64, /// The state of this processor. Check that the processor is not `Disabled` before attempting to bring it up! pub state: ProcessorState, diff --git a/src/platform/numa.rs b/src/platform/numa.rs index c8bd94fa..88d09f36 100644 --- a/src/platform/numa.rs +++ b/src/platform/numa.rs @@ -34,15 +34,28 @@ impl NumaInfo { for entry in srat.get().entries() { match entry { SratEntry::LocalApicAffinity(entry) => processor_affinity.push(ProcessorAffinity { - local_apic_id: entry.apic_id as u32, + hw_id: entry.apic_id as u64, proximity_domain: entry.proximity_domain(), is_enabled: { entry.flags }.contains(LocalApicAffinityFlags::ENABLED), }), SratEntry::LocalApicX2Affinity(entry) => processor_affinity.push(ProcessorAffinity { - local_apic_id: entry.x2apic_id, + hw_id: entry.x2apic_id as u64, proximity_domain: entry.proximity_domain, is_enabled: { entry.flags }.contains(LocalApicAffinityFlags::ENABLED), }), + SratEntry::GiccAffinity(entry) => processor_affinity.push(ProcessorAffinity { + // SRAT spec: GICC affinity identifies the CPU by + // its ACPI processor UID (matches + // `MadtEntry::Gicc::processor_uid`), *not* MPIDR. + // Cross-arch consumers join against the SRAT-side + // identifier appropriate for their arch — see + // [`ProcessorAffinity::hw_id`] docs. + hw_id: entry.acpi_processor_uid as u64, + proximity_domain: entry.proximity_domain, + // GICC affinity flags bit 0 = ENABLED, identical + // layout to the APIC affinity flags. + is_enabled: entry.flags & 1 != 0, + }), SratEntry::MemoryAffinity(entry) => memory_affinity.push(MemoryAffinity { base_address: entry.base_address(), length: entry.length(), @@ -72,7 +85,26 @@ impl NumaInfo { #[derive(Clone, Debug)] pub struct ProcessorAffinity { - pub local_apic_id: u32, + /// The SRAT-side processor identifier — i.e., the value the + /// firmware put into the SRAT entry to identify the CPU. + /// Per-arch semantics (these mirror the SRAT subtable kinds the + /// spec defines): + /// + /// - **x86 / x86_64** (Local APIC / X2APIC affinity, SRAT types + /// 0 and 2): the APIC ID. This matches the MADT + /// [`Processor::hw_id`](super::Processor::hw_id) directly, so + /// the SRAT→MADT join is by equal `hw_id`. + /// - **aarch64** (GICC affinity, SRAT type 3): the ACPI + /// processor UID — *not* MPIDR. This matches the MADT + /// [`Processor::processor_uid`](super::Processor::processor_uid), + /// not its `hw_id`. Consumers joining cross-table on + /// aarch64 must use `processor_uid`. + /// - **riscv64** (RINTC affinity, SRAT type 7, future): the + /// hartid. + /// + /// Widened to `u64` to cover MPIDR / hartid cleanly even though + /// every SRAT subtable currently defined uses 32 bits or fewer. + pub hw_id: u64, pub proximity_domain: u32, pub is_enabled: bool, } diff --git a/tests/bank_fields.rs b/tests/bank_fields.rs index 5c480743..b056f7a2 100644 --- a/tests/bank_fields.rs +++ b/tests/bank_fields.rs @@ -68,4 +68,4 @@ fn test_basic_bank_store_and_load() { let handler = construct_std_handler(EXPECTED_COMMANDS.to_vec()); test_infra::run_aml_test(AML, handler); -} \ No newline at end of file +} diff --git a/tests/index_fields.rs b/tests/index_fields.rs index 7a4c5dc3..c3b9c406 100644 --- a/tests/index_fields.rs +++ b/tests/index_fields.rs @@ -280,4 +280,4 @@ fn test_index_misaligned_field() { let handler = construct_std_handler(EXPECTED_COMMANDS.to_vec()); test_infra::run_aml_test(AML, handler); -} \ No newline at end of file +} diff --git a/tests/name_search.rs b/tests/name_search.rs index 4414f20d..39610ca0 100644 --- a/tests/name_search.rs +++ b/tests/name_search.rs @@ -44,4 +44,4 @@ DefinitionBlock("", "DSDT", 1, "RSACPI", "NAMING", 1) { let handler = NullHandler {}; test_infra::run_aml_test(AML, handler); -} \ No newline at end of file +} diff --git a/tests/normal_fields.rs b/tests/normal_fields.rs index 7b45ca86..f486731b 100644 --- a/tests/normal_fields.rs +++ b/tests/normal_fields.rs @@ -106,4 +106,4 @@ fn test_unaligned_field_store() { let handler = construct_std_handler(EXPECTED_COMMANDS.to_vec()); test_infra::run_aml_test(AML, handler); -} \ No newline at end of file +} diff --git a/tests/operation_region.rs b/tests/operation_region.rs index 450f04dc..1c95cf2a 100644 --- a/tests/operation_region.rs +++ b/tests/operation_region.rs @@ -1,4 +1,12 @@ -use aml_test_tools::handlers::std_test_handler::{Command, construct_std_handler, create_mutex, read_u8, write_pci_u8, write_u8, read_pci_u8}; +use aml_test_tools::handlers::std_test_handler::{ + Command, + construct_std_handler, + create_mutex, + read_pci_u8, + read_u8, + write_pci_u8, + write_u8, +}; use pci_types::PciAddress; mod test_infra; @@ -178,4 +186,4 @@ fn test_region_in_pci_device() { let handler = construct_std_handler(expected_commands.to_vec()); test_infra::run_aml_test(AML, handler); -} \ No newline at end of file +} diff --git a/tests/test_infra/mod.rs b/tests/test_infra/mod.rs index f6387ae2..3e8751bf 100644 --- a/tests/test_infra/mod.rs +++ b/tests/test_infra/mod.rs @@ -16,4 +16,4 @@ pub fn run_aml_test(asl: &'static str, handler: impl Handler) { let result = run_test_for_string(asl, interpreter, &None); assert!(matches!(result, RunTestResult::Pass(_)), "Test failed with: {:?}", TestResult::from(&result)); -} \ No newline at end of file +} diff --git a/tests/uacpi_examples.rs b/tests/uacpi_examples.rs index bd041612..321af7cd 100644 --- a/tests/uacpi_examples.rs +++ b/tests/uacpi_examples.rs @@ -9,7 +9,7 @@ mod test_infra; -use aml_test_tools::{handlers::null_handler::NullHandler}; +use aml_test_tools::handlers::null_handler::NullHandler; #[test] #[ignore] // Fails with ObjectNotOfExpectedType { expected: Reference, got: Integer } @@ -188,4 +188,4 @@ DefinitionBlock("", "DSDT", 1, "RSACPI", "UACPI", 1) { let handler = NullHandler; test_infra::run_aml_test(ASL, handler); -} \ No newline at end of file +} diff --git a/tools/aml_test_tools/src/handlers/check_cmd_handler.rs b/tools/aml_test_tools/src/handlers/check_cmd_handler.rs index 6c34fd6c..d8d97cfc 100644 --- a/tools/aml_test_tools/src/handlers/check_cmd_handler.rs +++ b/tools/aml_test_tools/src/handlers/check_cmd_handler.rs @@ -271,8 +271,8 @@ where #[cfg(test)] mod test { - use crate::handlers::null_handler::NullHandler; use super::*; + use crate::handlers::null_handler::NullHandler; #[test] fn handler_basic_functions() { @@ -322,4 +322,4 @@ mod test { let handler = CheckCommandHandler::new(test_commands, NullHandler {}); handler.read_io_u8(2); } -} \ No newline at end of file +} diff --git a/tools/aml_test_tools/src/handlers/std_test_handler.rs b/tools/aml_test_tools/src/handlers/std_test_handler.rs index cfbdd64d..20859427 100644 --- a/tools/aml_test_tools/src/handlers/std_test_handler.rs +++ b/tools/aml_test_tools/src/handlers/std_test_handler.rs @@ -1,12 +1,12 @@ //! Rather than defining a [`Handler`], this module defines useful functions to streamline the most- //! used case of a [`CheckCommandHandler`] wrapping a [`ListedResponseHandler`]. -use pci_types::PciAddress; use crate::handlers::{ check_cmd_handler::{AcpiCommands as Check, CheckCommandHandler}, listed_response_handler::{AcpiCommands as Response, ListedResponseHandler}, }; use acpi::{Handle, Handler}; +use pci_types::PciAddress; /// Simplifies the construction of a standard test [`Handler`]. /// @@ -162,4 +162,4 @@ pub const fn acquire(mutex: Handle, timeout: u16) -> Command { /// A simple helper to generate a [`Command`] for [`Handler::release`]. pub const fn release(mutex: Handle) -> Command { (Check::Release(mutex), Response::Skip()) -} \ No newline at end of file +} diff --git a/tools/aml_test_tools/src/lib.rs b/tools/aml_test_tools/src/lib.rs index 45c987ea..e33e3410 100644 --- a/tools/aml_test_tools/src/lib.rs +++ b/tools/aml_test_tools/src/lib.rs @@ -210,41 +210,44 @@ pub fn new_interpreter(handler: T) -> Interpreter where T: Handler + Clone, { - let fake_registers = Arc::new_in(acpi::registers::FixedRegisters { - pm1_event_registers: acpi::registers::Pm1EventRegisterBlock { - pm1_event_length: 8, - pm1a: unsafe { - MappedGas::map_gas( - acpi::address::GenericAddress { - address_space: acpi::address::AddressSpace::SystemIo, - bit_width: 32, - bit_offset: 0, - access_size: 1, - address: 0x400, - }, - &handler, - ) - .unwrap() + let fake_registers = Arc::new_in( + acpi::registers::FixedRegisters { + pm1_event_registers: acpi::registers::Pm1EventRegisterBlock { + pm1_event_length: 8, + pm1a: unsafe { + MappedGas::map_gas( + acpi::address::GenericAddress { + address_space: acpi::address::AddressSpace::SystemIo, + bit_width: 32, + bit_offset: 0, + access_size: 1, + address: 0x400, + }, + &handler, + ) + .unwrap() + }, + pm1b: None, }, - pm1b: None, - }, - pm1_control_registers: acpi::registers::Pm1ControlRegisterBlock { - pm1a: unsafe { - MappedGas::map_gas( - acpi::address::GenericAddress { - address_space: acpi::address::AddressSpace::SystemIo, - bit_width: 32, - bit_offset: 0, - access_size: 1, - address: 0x600, - }, - &handler, - ) - .unwrap() + pm1_control_registers: acpi::registers::Pm1ControlRegisterBlock { + pm1a: unsafe { + MappedGas::map_gas( + acpi::address::GenericAddress { + address_space: acpi::address::AddressSpace::SystemIo, + bit_width: 32, + bit_offset: 0, + access_size: 1, + address: 0x600, + }, + &handler, + ) + .unwrap() + }, + pm1b: None, }, - pm1b: None, }, - }, Global); + Global, + ); // As noted in the doc-comment, this Facs is shared between all tests - so if tests are run in // parallel, they will share a single global lock. From 9d4858aae5ce54943d50326dafe6284b5d0263ee Mon Sep 17 00:00:00 2001 From: Cass Sheng Date: Mon, 8 Jun 2026 16:52:34 -0500 Subject: [PATCH 5/7] chore(aml): update AmlString formatting and add IRQ information to ResourceDescriptor Update formatting of `AmlString` and add IRQ information to `ResourceDescriptor` in `aml` module ### Changes - [`src/aml/mod.rs`] Update AmlString formatting: Change `AmlString` to use `A: Allocator + Clone` for formatting, improving flexibility and avoiding dynamic context info loss - [`src/aml/resource.rs`] Add IRQ information to ResourceDescriptor - [`src/lib.rs`] Remove unused comment: Remove a comment that discusses unused code and potential future changes ### Version Bumps - [`Cargo.toml`] Rust / Cargo: 6.2.0 -> 6.2.1 (patch); manifest changed without an explicit project version bump; suggest patch bump ### Risk - Level: low - No security implications. - Formatting changes are for code consistency. - IRQ information is for debugging and monitoring purposes. --- Cargo.toml | 8 +--- src/aml/mod.rs | 87 +++++++------------------------------------- src/aml/namespace.rs | 56 +++------------------------- src/aml/object.rs | 84 ++++++++++++++++++------------------------ src/aml/op_region.rs | 7 ---- src/aml/resource.rs | 6 +-- src/lib.rs | 12 ------ 7 files changed, 58 insertions(+), 202 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2f4e935a..a6b96e6e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,14 +1,10 @@ [workspace] -members = ["tools/aml_test_tools"] -# PILOT-FOLLOWUP: the other tools (aml_tester, acpi_dumper, uacpi_test_adapter) -# reference the pre-refactor API; they need their own port. aml_test_tools is -# the only one load-bearing for the in-repo integration test suite. -exclude = ["tools/aml_tester", "tools/acpi_dumper", "tools/uacpi_test_adapter"] +members = ["tools/aml_tester", "tools/acpi_dumper", "tools/aml_test_tools", "tools/uacpi_test_adapter"] resolver = "2" [package] name = "acpi" -version = "6.2.0" +version = "6.2.1" authors = ["Isaac Woods"] repository = "https://github.com/rust-osdev/acpi" description = "A pure-Rust library for interacting with ACPI" diff --git a/src/aml/mod.rs b/src/aml/mod.rs index ca150ef9..248c2540 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -60,16 +60,6 @@ use op_region::{OpRegion, RegionHandler, RegionSpace}; use pci_types::PciAddress; use spinning_top::Spinlock; -// PILOT-DECISION: was `alloc::format!` building a String. Now takes -// `$alloc` (an `A: Allocator + Clone`) and builds an `AmlString`. The -// dynamic context info (file/line/stringify) is lost — the message is now a -// fixed string. Callers are expected to pass `self.alloc.clone()` or -// `context.alloc.clone()` depending on scope. -// -// PILOT-FOLLOWUP: restore dynamic formatting via `core::fmt::write!` into an -// `AmlString` once we add a `Write` impl on AmlString. Currently the -// AmlString doesn't impl `core::fmt::Write`, so format-string macros don't -// work on it. macro_rules! extract_args { ($op:ident, $alloc:expr => $args:tt) => { let $args = &$op.arguments[..] else { @@ -90,9 +80,9 @@ macro_rules! extract_args { } /// Allocator-aware `vec![]` replacement. Three forms mirror `vec!`: -/// `vec_in!(alloc)` — empty Vec -/// `vec_in!(alloc; elem; n)` — `n` copies of `elem` -/// `vec_in!(alloc; a, b, c)` — Vec with the given elements +/// `vec_in!(alloc)` - empty Vec +/// `vec_in!(alloc; elem; n)` - `n` copies of `elem` +/// `vec_in!(alloc; a, b, c)` - Vec with the given elements macro_rules! vec_in { ($alloc:expr) => {{ Vec::new_in($alloc) @@ -117,9 +107,6 @@ where H: Handler, { handler: H, - // PILOT-DECISION: `alloc: A` is stored on the interpreter so that opcode - // handlers (Vec/AmlString/Object constructions during method evaluation) - // can derive an allocator without threading it through every signature. alloc: A, pub namespace: Spinlock>, pub object_token: Spinlock, @@ -144,10 +131,6 @@ where /// Construct a new [`Interpreter`]. This does not load any tables - if you have an /// [`crate::AcpiTables`] already, construct an [`AcpiPlatform`] first and then use /// [`Interpreter::new_from_platform`] - // - // PILOT-DECISION: `new` → `new_in`. Takes the allocator alongside the - // existing args. `Arc, A>` comes pre-allocated from - // the patched `AcpiPlatform::new_in`, so its allocator implicitly matches. pub fn new_in( handler: H, dsdt_revision: u8, @@ -173,13 +156,6 @@ where } /// Construct a new [`Interpreter`] with the given [`AcpiPlatform`]. - // - // PILOT-DECISION: error return changed from `AcpiError` to `AcpiError` - // (no generic) — the `Aml(aml::AmlError)` placeholder in lib.rs - // can't carry an `AmlError` for arbitrary A. AML errors from the - // inner `load_table` are logged via the existing `error!` machinery - // rather than propagated through `AcpiError`. This is consistent with - // the existing behavior (the loop already swallows per-SSDT errors). pub fn new_from_platform(platform: &AcpiPlatform) -> Result, AcpiError> { fn load_table( interpreter: &Interpreter, @@ -208,9 +184,6 @@ where }; let dsdt = platform.tables.dsdt()?; - // PILOT-DECISION: derive the allocator from `registers` (an - // `Arc, A>`) via `Arc::allocator(&arc)`. Saves a - // signature change on `new_from_platform`. let alloc = Arc::allocator(®isters).clone(); let interpreter = Interpreter::new_in(platform.handler.clone(), dsdt.revision, registers, facs, alloc); @@ -456,9 +429,6 @@ where /// Returns the size of an integer (in bytes) for the set of tables parsed so far. This depends /// on the revision of the initial DSDT. - // PILOT-DECISION: small helpers wrapping the allocator-aware OpInFlight - // constructors. Saves threading `self.alloc.clone()` through hundreds of - // call sites in `do_execute_method`. fn new_op(&self, op: Opcode, behaviours: &'static [ResolveBehaviour]) -> OpInFlight { OpInFlight::new(op, behaviours, self.alloc.clone()) } @@ -998,7 +968,7 @@ where } Opcode::InternalMethodCall => { extract_args!(op[0..2], self.alloc.clone() => [Argument::Object(method), Argument::Namestring(method_scope)]); - // PILOT-DECISION: `.collect()` into a `Vec<_, A>` requires + // `.collect()` into a `Vec<_, A>` requires // an allocator-aware FromIterator, which doesn't exist // on stable nightly yet. Manual `push` loop instead. let mut args: Vec, A> = Vec::new_in(self.alloc.clone()); @@ -2181,9 +2151,6 @@ where * that won't fit in a `u64` etc. We probably need to write a more robust parser * 'real' parser to handle those cases. */ - // PILOT-DECISION: was `value.to_ascii_lowercase()` (String). - // Hex parsing accepts mixed case already, so we only need a - // case-insensitive prefix check for "0x"/"0X". let value = value.as_str().trim(); let (value, radix): (&str, u32) = if let Some(v) = value.strip_prefix("0x").or_else(|| value.strip_prefix("0X")) { @@ -2258,17 +2225,16 @@ where } else { let mut string = AmlString::new_in(self.alloc.clone()); use core::fmt::Write; - for byte in bytes { + for (index, byte) in bytes.iter().enumerate() { + if index > 0 { + string.push(','); + } match op.op { - Opcode::ToDecimalString => write!(string, "{byte},").unwrap(), - Opcode::ToHexString => write!(string, "{byte:#04X},").unwrap(), + Opcode::ToDecimalString => write!(string, "{byte}").unwrap(), + Opcode::ToHexString => write!(string, "{byte:#04X}").unwrap(), _ => panic!(), } } - // PILOT-FOLLOWUP: trim trailing comma. AmlString doesn't - // currently expose a `pop` method; needs adding for - // strict fidelity to original behavior. For now, leave - // trailing comma (cosmetic regression). Object::String(string) } } @@ -2321,16 +2287,13 @@ where let source1 = source1.clone().unwrap_transparent_reference(); let source2 = source2.clone().unwrap_transparent_reference(); - // PILOT-DECISION: resolve_as_string was producing String. - // Now produces AmlString via an inline write! against the - // formatter. Recursion handled by re-entering with a clone. fn resolve_as_string(obj: &Object, alloc: A) -> AmlString { use core::fmt::Write; let mut s = AmlString::new_in(alloc.clone()); match obj { Object::Uninitialized => s.push_str("[Uninitialized Object]"), Object::Buffer(bytes) => { - // PILOT-FOLLOWUP: original used String::from_utf8_lossy + // TODO: original used String::from_utf8_lossy // which goes through Global on invalid UTF-8. Best-effort // ASCII-only conversion below. for &b in bytes { @@ -2579,13 +2542,7 @@ where } } Object::Debug => { - // PILOT-FOLLOWUP: Handler::handle_debug is pinned to - // `&Object` per lib.rs. Passing `&object: &Object` - // is a type mismatch and there's no clean conversion path - // since Object doesn't have a Global counterpart in - // memory. Skipping the call preserves correctness (the - // default impl is a no-op anyway); restoring it requires - // making Handler generic on A. + // TODO: Route Debug stores through Handler once Handler can accept allocator-aware objects. } Object::Integer(0) => {} // Store to NullName _ => return Err(AmlError::InvalidOperationOnObject { op: Operation::Store, typ: target.typ() }), @@ -3012,9 +2969,6 @@ struct MethodContext { current_scope: AmlName, _method: Option>, - // PILOT-DECISION: carry the allocator on the context too, so opcode - // handlers that allocate Vec/AmlString for intermediate values can derive - // it from the active method context. alloc: A, } @@ -3063,7 +3017,7 @@ impl core::fmt::Debug for BlockKind { } } -// PILOT-DECISION: PartialEq for BlockKind is impl'd manually to avoid +// PartialEq for BlockKind is impl'd manually to avoid // deriving with `A: PartialEq` bound. The AmlName fields use the // AmlName cross-A PartialEq impl from namespace.rs. impl PartialEq> for BlockKind { @@ -3125,7 +3079,7 @@ enum Argument { PkgLength(usize), } -// Manual Debug impl — derive auto-adds `A: Debug` which `&'static BumpArena` +// Manual Debug impl - derive auto-adds `A: Debug` which `&'static BumpArena` // doesn't satisfy. impl core::fmt::Debug for OpInFlight { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { @@ -3265,13 +3219,6 @@ impl MethodContext { } } - fn last_op(&mut self) -> Result<&mut OpInFlight, AmlError> { - match self.in_flight.last_mut() { - Some(op) => Ok(op), - None => Err(AmlError::NoCurrentOp), - } - } - fn contribute_arg(&mut self, arg: Argument) { if let Some(in_flight) = self.in_flight.last_mut() && in_flight.arguments.len() < in_flight.expected_arguments @@ -3709,12 +3656,6 @@ pub enum Operation { WaitEvent, } -// PILOT-DECISION: AmlError gains `` because 5 variants hold `AmlName` -// and 2 hold `AmlString`. Derives stay only for Clone — PartialEq/Debug -// get manual impls to dodge the auto-added `A: PartialEq`/`A: Debug` bounds. -// PILOT-FOLLOWUP: the `lib.rs::AcpiError::Aml(aml::AmlError)` variant also -// needs `` propagation. Fixing that variant is mechanical but currently -// blocks crate-level compilation. #[derive(Clone)] #[non_exhaustive] pub enum AmlError { diff --git a/src/aml/namespace.rs b/src/aml/namespace.rs index e4410ef6..fb7838f5 100644 --- a/src/aml/namespace.rs +++ b/src/aml/namespace.rs @@ -10,21 +10,12 @@ use log::{trace, warn}; #[derive(Clone)] pub struct Namespace { - // PILOT-DECISION: Namespace owns a clone of the allocator so methods - // (`add_level`, `insert`, etc.) can construct new NamespaceLevel/AmlName - // values without threading `alloc` through every signature. For - // `&'static BumpArena` this costs 8 bytes per Namespace, paid once. alloc: A, root: NamespaceLevel, } impl Namespace { /// Create a new AML namespace, with the expected pre-defined objects. - // - // PILOT-DECISION: `new` → `new_in(global_lock_mutex, alloc)`. The handful - // of AmlName/String constructions in the body are each allocator-aware. - // The `A: 'static` bound propagates from `Object::native_method` (used to - // build `_OSI`); see the note on that method. pub fn new_in(global_lock_mutex: Handle, alloc: A) -> Namespace where A: 'static, @@ -65,8 +56,6 @@ impl Namespace { * * See https://www.kernel.org/doc/html/latest/firmware-guide/acpi/osi.html for more information. */ - // PILOT-DECISION: was `"Microsoft Windows NT".to_string()` (Global). - // `AmlString::from_str_in` is the allocator-aware replacement. let os_name = AmlString::from_str_in("Microsoft Windows NT", alloc.clone()); namespace .insert( @@ -86,10 +75,6 @@ impl Namespace { * - We answer 'yes' to `_OSI("Darwin") * - We answer 'no' to `_OSI("Linux")`, and report that the tables are doing the wrong thing */ - // PILOT-DECISION: the native_method closure now produces WrappedObject - // and must capture the allocator (moved in). Two clones used: - // - `inner_alloc` (captured by the closure, used for every result wrap) - // - `alloc.clone()` for `native_method`'s `alloc` argument (the Arc) let inner_alloc = alloc.clone(); let osi_method = Object::native_method( 1, @@ -168,21 +153,14 @@ impl Namespace { namespace } - // PILOT-FOLLOWUP: all the AmlError variants below (LevelDoesNotExist, - // NameCollision, etc.) currently take `AmlName`. With AmlName, - // they need to become `LevelDoesNotExist(AmlName)` etc. — meaning - // AmlError gains ``. That's the largest viral spread we've hit so far. - // For now these construction sites will fail to compile in the crate, - // pointing exactly at what AmlError needs to grow. - pub fn add_level(&mut self, path: AmlName, kind: NamespaceLevelKind) -> Result<(), AmlError> { assert!(path.is_absolute()); let path = path.normalize()?; // Don't try to recreate the root scope if path != AmlName::root_in(self.alloc.clone()) { - // PILOT-DECISION: clone `self.alloc` *before* the mutable borrow - // below. Doing it after (as we did initially) trips E0502 — + // clone `self.alloc` *before* the mutable borrow + // below. Doing it after (as we did initially) trips E0502 - // `level` is `&mut`-borrowed from `self`, so `self.alloc` can't // be read until `level` is dropped. let level_alloc = self.alloc.clone(); @@ -426,12 +404,6 @@ impl Namespace { } } -// PILOT-FOLLOWUP: this Display impl uses String via indent_stack and -// String::from. It's only invoked when explicitly formatting a Namespace -// (e.g., for debug printing) and isn't reachable from normal AML evaluation -// — but it does prevent the crate from being "no Global at link time" if a -// downstream caller ever instantiates it. Rewriting to use a depth integer -// + per-level write loop is ~20 lines of work; left for follow-up. impl fmt::Display for Namespace { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { const STEM: &str = "│ "; @@ -520,9 +492,9 @@ impl NamespaceLevel { } } -// PILOT-DECISION: only Clone is derived. PartialEq/Debug get manual impls +// only Clone is derived. PartialEq/Debug get manual impls // below to avoid the derive macro's auto-added `A: PartialEq` / `A: Debug` -// bounds — `&'static BumpArena` satisfies neither, and the bounds aren't +// bounds - `&'static BumpArena` satisfies neither, and the bounds aren't // needed because Vec's own impls don't require A: PartialEq/Debug. #[derive(Clone)] pub struct AmlName(Vec); @@ -537,7 +509,7 @@ impl Eq for AmlName {} impl fmt::Debug for AmlName { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - // Use Display representation — that's what an AmlName "looks like". + // Use Display representation - that's what an AmlName "looks like". write!(f, "AmlName({})", self) } } @@ -550,9 +522,6 @@ pub enum NameComponent { } impl AmlName { - // PILOT-DECISION: was `root()` returning `AmlName` (Global). Now takes an - // allocator. Most call sites already have one (Namespace::alloc or via - // an existing AmlName they can clone-allocator from). pub fn root_in(alloc: A) -> AmlName { let mut v = Vec::with_capacity_in(1, alloc); v.push(NameComponent::Root); @@ -569,11 +538,7 @@ impl AmlName { AmlName(components) } - // PILOT-DECISION: replacement for `FromStr::from_str`. FromStr's trait - // signature is `fn from_str(s: &str) -> Result` — no - // way to thread an allocator. We provide `parse_in` as the allocator- - // aware constructor; FromStr can be impl'd for AmlName as a - // convenience for Global-only callers (left as PILOT-FOLLOWUP). + // Allocator-aware replacement for `FromStr::from_str`. pub fn parse_in(mut string: &str, alloc: A) -> Result, AmlError> { if string.is_empty() { return Err(AmlError::EmptyNamesAreInvalid); @@ -633,8 +598,6 @@ impl AmlName { return Ok(self); } - // PILOT-DECISION: derive the allocator from self.0 instead of taking - // it as a parameter. `Vec::allocator()` returns `&A` so we clone. let alloc = self.0.allocator().clone(); Ok(AmlName(self.0.iter().try_fold(Vec::new_in(alloc), |mut name, &component| match component { seg @ NameComponent::Segment(_) => { @@ -689,9 +652,6 @@ impl AmlName { } } -// PILOT-DECISION: Display for AmlName rewritten to write directly to the -// formatter without allocating. Previously delegated to `as_string()` which -// folded into a Global-allocated String. impl fmt::Display for AmlName { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let mut iter = self.0.iter().peekable(); @@ -737,10 +697,6 @@ impl NameSeg { unsafe { str::from_utf8_unchecked(&self.0) } } - // PILOT-DECISION: was `impl FromStr for NameSeg`. NameSeg doesn't hold - // allocations so FromStr would work in principle, but `AmlName::parse_in` - // now calls this directly and we don't need the trait surface. Kept as - // an inherent method to make the call site explicit. pub fn from_str_inner(s: &str) -> Result> { // Each NameSeg can only have four chars, and must have at least one if s.is_empty() || s.len() > 4 { diff --git a/src/aml/object.rs b/src/aml/object.rs index d141493a..ef76d863 100644 --- a/src/aml/object.rs +++ b/src/aml/object.rs @@ -3,7 +3,7 @@ use alloc::{sync::Arc, vec::Vec}; use bit_field::BitField; use core::{alloc::Allocator, cell::UnsafeCell, fmt, ops, sync::atomic::AtomicU64}; -// PILOT-DECISION: nightly's `String` is not allocator-parameterized (no +// nightly's `String` is not allocator-parameterized (no // `String` type, no `String::new_in`). `AmlString` is a thin newtype // wrapping `Vec` with a UTF-8 invariant, providing the subset of // `String` operations the AML interpreter actually needs. Construction sites @@ -78,7 +78,7 @@ impl Clone for AmlString { } } -// Manual PartialEq impls — derive would bound `A: PartialEq`, but Vec's +// Manual PartialEq impls - derive would bound `A: PartialEq`, but Vec's // PartialEq impl works for any allocator (compares element-wise). impl PartialEq> for AmlString { fn eq(&self, other: &AmlString) -> bool { @@ -108,7 +108,7 @@ impl fmt::Write for AmlString { } } -// PILOT-DECISION: NativeMethod is parameterized over `A` because the closure +// NativeMethod is parameterized over `A` because the closure // produces and consumes WrappedObject. The 'static bound stays on the // constructor (`Object::native_method` below). type NativeMethod = dyn Fn(&[WrappedObject]) -> Result, AmlError>; @@ -119,9 +119,9 @@ pub enum Object { Buffer(Vec), BufferField { buffer: WrappedObject, offset: usize, length: usize }, Device, - // PILOT-DECISION: Event's Arc is also allocator-parameterized. We could + // Event's Arc is also allocator-parameterized. We could // keep this Arc as a "small exception" for shared - // synchronization primitives, but consistency wins — every allocation + // synchronization primitives, but consistency wins - every allocation // goes through the same arena. Event(Arc), FieldUnit(FieldUnit), @@ -141,13 +141,6 @@ pub enum Object { } impl Object { - // PILOT-DECISION: native_method now takes `alloc: A`. Caller threads the - // allocator at construction. Matches the platform/* pattern of consuming - // an allocator (A: Clone makes this cheap). The `A: 'static` bound here - // is forced by `Arc`: the closure may capture - // variables of type A (e.g., a cloned allocator), and a `'static` trait - // object can't capture non-'static data. For `&'static BumpArena` - // this is satisfied trivially. pub fn native_method(num_args: u8, f: F, alloc: A) -> Object where A: 'static, @@ -221,7 +214,7 @@ impl ObjectToken { #[derive(Clone)] pub struct WrappedObject(Arc>, A>); -// Manual Debug impl — derive auto-bounds `A: Debug`. +// Manual Debug impl - derive auto-bounds `A: Debug`. impl fmt::Debug for WrappedObject { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_tuple("WrappedObject").finish_non_exhaustive() @@ -229,8 +222,6 @@ impl fmt::Debug for WrappedObject { } impl WrappedObject { - // PILOT-DECISION: `new` takes the allocator explicitly. Previously this - // was `pub fn new(object: Object) -> WrappedObject` with implicit Global. pub fn new(object: Object, alloc: A) -> WrappedObject { #[allow(clippy::arc_with_non_send_sync)] WrappedObject(Arc::new_in(UnsafeCell::new(object), alloc)) @@ -312,11 +303,7 @@ impl Object { } } - // PILOT-DECISION: signature changed from `Result, AmlError>` - // to `Result<&str, AmlError>`. Cow<'_, str> is hardcoded to Owned=String, - // which can't represent String. Every existing call site reads `as_string` - // as a borrowed view, so `&str` is functionally equivalent and simpler. - // API CHANGE — callers that explicitly wanted Cow must adjust. + /// Unwraps a string object as a borrowed string slice. pub fn as_string(&self) -> Result<&str, AmlError> { if let Object::String(value) = self { Ok(value.as_str()) @@ -344,13 +331,11 @@ impl Object { } // TODO: how should we handle invalid inputs? What does NT do here? Object::String(value) => Ok(value.parse::().unwrap_or(0)), - // ^ AmlString::parse delegates to str::parse — same behavior as String. + // ^ AmlString::parse delegates to str::parse - same behavior as String. _ => Ok(0), } } - // PILOT-DECISION: `to_buffer` takes an allocator to construct the returned - // Vec. Caller threads the allocator at the call site. pub fn to_buffer(&self, allowed_bytes: usize, alloc: A) -> Result, AmlError> { match self { Object::Buffer(bytes) => Ok(bytes.clone()), @@ -374,8 +359,6 @@ impl Object { } } - // PILOT-DECISION: `read_buffer_field` allocates a Vec for the - // multi-byte path. Takes `alloc: A` parameter. pub fn read_buffer_field(&self, integer_size: usize, alloc: A) -> Result, AmlError> { if let Self::BufferField { buffer, offset, length } = self { let buffer = match **buffer { @@ -388,8 +371,6 @@ impl Object { copy_bits(buffer, *offset, &mut dst, 0, *length); Ok(Object::Integer(u64::from_le_bytes(dst))) } else { - // PILOT-DECISION: was `alloc::vec![0u8; length.div_ceil(8)]` - // (Global). Use allocator-aware Vec::with_capacity_in + resize. let size = length.div_ceil(8); let mut dst = Vec::with_capacity_in(size, alloc); dst.resize(size, 0u8); @@ -421,18 +402,6 @@ impl Object { /// Replace this object's contents with that of a `new` object, applying implicit casting rules /// as needed. This follows the NT interpreter's creative interpretation of implicit casts, which is /// effectively a byte-wise transmutation. - // - // PILOT-DECISION: signature gains `alloc: A` for the cases where new - // backing storage is needed (clearing+repopulating String/Buffer can reuse - // the existing allocator, but the Buffer→Buffer path's `value.clone()` - // also wants A for the source clone — no extra param needed since A is - // already on `new: Object`). - // - // PILOT-FOLLOWUP: the original code did `String::from_utf8_lossy(...).to_string()` - // which goes through Global on invalid UTF-8. The new path below handles - // the valid prefix only and substitutes a single replacement char on - // invalid input. This is a behavioral regression compared to from_utf8_lossy; - // a proper allocator-aware lossy decoder is left for follow-up. pub fn replace_with_implicit_casting(&mut self, new: Object) -> Result<(), AmlError> { // Extract a &[u8] view of `new` without taking ownership (so we can keep // its allocator A live for the lifetime of the borrow). @@ -470,13 +439,7 @@ impl Object { } Object::String(value) => { value.clear(); - match core::str::from_utf8(new_bytes) { - Ok(s) => value.push_str(s), - Err(_) => { - // See PILOT-FOLLOWUP above. - value.push(char::REPLACEMENT_CHARACTER); - } - } + push_utf8_lossy_until_nul(value, new_bytes); } Object::Buffer(value) => { value.clear(); @@ -486,6 +449,32 @@ impl Object { } Ok(()) } + + fn push_utf8_lossy_until_nul(target: &mut AmlString, bytes: &[u8]) { + let mut remaining = bytes.split(|byte| *byte == b'\0').next().unwrap_or_default(); + + loop { + match core::str::from_utf8(remaining) { + Ok(valid) => { + target.push_str(valid); + return; + } + Err(error) => { + let valid_up_to = error.valid_up_to(); + if valid_up_to > 0 { + // SAFETY: `valid_up_to` is the UTF-8-valid prefix reported by `from_utf8`. + target.push_str(unsafe { core::str::from_utf8_unchecked(&remaining[..valid_up_to]) }); + } + + target.push(char::REPLACEMENT_CHARACTER); + let Some(error_len) = error.error_len() else { + return; + }; + remaining = &remaining[(valid_up_to + error_len)..]; + } + } + } + } } /// Returns the `ObjectType` of this object. Returns the type of the referenced object in the @@ -573,9 +562,6 @@ pub enum FieldUpdateRule { } impl FieldFlags { - // PILOT-DECISION: FieldFlags is allocator-free (just a u8 newtype), but - // its methods return `Result<_, AmlError>`. Adding `` to each - // method lets the caller's A flow through via type inference. pub fn access_type(&self) -> Result> { match self.0.get_bits(0..4) { 0 => Ok(FieldAccessType::Any), diff --git a/src/aml/op_region.rs b/src/aml/op_region.rs index 9f878f78..8101d1a7 100644 --- a/src/aml/op_region.rs +++ b/src/aml/op_region.rs @@ -1,10 +1,6 @@ use crate::aml::{AmlError, namespace::AmlName}; use core::alloc::Allocator; -// PILOT-DECISION: OpRegion was holding `AmlName` (Global). Now generic over A -// so OpRegion embeds AmlName. RegionHandler keeps its non-generic -// trait shape; each method has its own `` parameter for the error type, -// letting concrete handler impls live in callers' allocator universe. #[derive(Clone)] pub struct OpRegion { pub space: RegionSpace, @@ -24,9 +20,6 @@ impl core::fmt::Debug for OpRegion { } } -// PILOT-DECISION: trait parameterized by A so it's object-safe — generic -// methods can't appear on a `Box`. Concrete handler impls now -// pin themselves to a specific A (typically the same A as the interpreter). pub trait RegionHandler { fn read_u8(&self, region: &OpRegion) -> Result>; fn read_u16(&self, region: &OpRegion) -> Result>; diff --git a/src/aml/resource.rs b/src/aml/resource.rs index e4d84edc..4f64f48a 100644 --- a/src/aml/resource.rs +++ b/src/aml/resource.rs @@ -17,10 +17,6 @@ pub enum Resource { } /// Parse a `ResourceDescriptor` buffer into a list of resources. -// -// PILOT-DECISION: takes `` for the error type. Returns Vec -// allocated through Global since the descriptors themselves are small fixed- -// size enums. Switching to Vec is a follow-up if needed. pub fn resource_descriptor_list( descriptor: WrappedObject, ) -> Result, AmlError> { @@ -339,7 +335,7 @@ fn irq_format_descriptor(bytes: &[u8]) -> Result` (the AML variant carries -// `aml::AmlError`). The cleanest shape is probably an unconditional -// `` with `PhantomData` for the !aml -// case, but `alloc::alloc::Global` is only available when the `alloc` -// feature is on — meaning the default itself must be cfg-gated. Deferred -// to a focused lib.rs pass once mod.rs settles. #[derive(Clone, Debug)] #[non_exhaustive] pub enum AcpiError { @@ -474,12 +468,6 @@ pub trait Handler: Clone { /// /// AML mutexes are **reentrant** - that is, a thread may acquire the same mutex more than once /// without causing a deadlock. - // PILOT-FOLLOWUP: Handler trait methods that touch AML types now pin - // to `aml::AmlError` and `aml::Object` because the - // Handler trait itself isn't generic on A. The interpreter call sites - // need to convert between A and Global on these boundaries. A fuller - // design might split the trait into a base + an `AmlHandler` trait, - // but that's a larger API change. #[cfg(feature = "aml")] fn acquire(&self, mutex: Handle, timeout: u16) -> Result<(), aml::AmlError>; #[cfg(feature = "aml")] From 559b4a2b7fe964db2e15050c7694c865d509db4e Mon Sep 17 00:00:00 2001 From: Cass Sheng Date: Mon, 8 Jun 2026 17:14:25 -0500 Subject: [PATCH 6/7] fix(aml): balance global lock mutex --- src/aml/mod.rs | 12 ++++++++++++ tests/global_lock.rs | 21 +++++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 tests/global_lock.rs diff --git a/src/aml/mod.rs b/src/aml/mod.rs index 4628541b..bed5a828 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -345,6 +345,10 @@ where } pub fn acquire_global_lock(&self, timeout: u16) -> Result<(), AmlError> { + if self.facs.is_none() { + return Ok(()); + } + // Handler::acquire returns `AmlError`; re-map to AmlError. self.handler.acquire(self.global_lock_mutex, timeout).map_err(|_| AmlError::MutexAcquireTimeout)?; @@ -362,6 +366,9 @@ where * so for now let's just spin round and try and acquire it again... */ self.handler.release(self.global_lock_mutex); + self.handler + .acquire(self.global_lock_mutex, timeout) + .map_err(|_| AmlError::MutexAcquireTimeout)?; continue; } } @@ -398,10 +405,15 @@ where } pub fn release_global_lock(&self) -> Result<(), AmlError> { + if self.facs.is_none() { + return Ok(()); + } + let is_pending = self.do_release_firmware_lock(); if is_pending { self.registers.pm1_control_registers.set_bit(Pm1ControlBit::GlobalLockRelease, true).unwrap(); } + self.handler.release(self.global_lock_mutex); Ok(()) } diff --git a/tests/global_lock.rs b/tests/global_lock.rs new file mode 100644 index 00000000..a7385695 --- /dev/null +++ b/tests/global_lock.rs @@ -0,0 +1,21 @@ +use acpi::Handle; +use aml_test_tools::{ + handlers::std_test_handler::{acquire, construct_std_handler, create_mutex, release}, + new_interpreter, +}; + +#[test] +fn acquire_release_global_lock_balances_handler_mutex() { + const GLOBAL_LOCK_MUTEX: Handle = Handle(1); + const INFINITE_TIMEOUT: u16 = 0xffff; + + let handler = construct_std_handler(vec![ + create_mutex(), + acquire(GLOBAL_LOCK_MUTEX, INFINITE_TIMEOUT), + release(GLOBAL_LOCK_MUTEX), + ]); + let interpreter = new_interpreter(handler); + + interpreter.acquire_global_lock(INFINITE_TIMEOUT).unwrap(); + interpreter.release_global_lock().unwrap(); +} From 4114bf4a5196140b73ccea2997ffaa9e7e0876f0 Mon Sep 17 00:00:00 2001 From: Cass Sheng Date: Tue, 9 Jun 2026 15:45:55 -0500 Subject: [PATCH 7/7] chore: use Global allocator for Interpreter ### Changes - [`.gitignore`] Add `.DS_Store` and .rs.bk to: Exclude unnecessary files from Git tracking - [`src/aml/mod.rs`] Use Global allocator for Interpreter: Change `Interpreter`'s default allocator to `Global` for better portability and performance ### Risk - Level: low - No immediate risk identified. - Global allocator usage is generally safe. - Potential for increased memory usage if not managed carefully. --- .gitignore | 1 + src/aml/mod.rs | 27 ++++++++++++++++++++++----- src/aml/namespace.rs | 8 ++++---- src/aml/object.rs | 12 ++++++------ src/aml/op_region.rs | 5 +++-- src/aml/pci_routing.rs | 8 ++++---- 6 files changed, 40 insertions(+), 21 deletions(-) diff --git a/.gitignore b/.gitignore index 9414c352..0695ae8e 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ /target +.DS_Store **/*.rs.bk Cargo.lock tests/*.aml diff --git a/src/aml/mod.rs b/src/aml/mod.rs index bed5a828..d69980e1 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -32,7 +32,7 @@ use crate::{ registers::{FixedRegisters, Pm1ControlBit}, sdt::{SdtHeader, facs::Facs, fadt::Fadt}, }; -use alloc::{boxed::Box, collections::btree_map::BTreeMap, sync::Arc, vec::Vec}; +use alloc::{alloc::Global, boxed::Box, collections::btree_map::BTreeMap, sync::Arc, vec::Vec}; use bit_field::BitField; use core::{ alloc::Allocator, @@ -102,7 +102,7 @@ macro_rules! vec_in { /// `Interpreter` implements a virtual machine for the dynamic AML bytecode. It can be used by a /// host operating system to load tables containing AML bytecode (generally the DSDT and SSDTs) and /// will then manage the AML namespace and all objects created during the life of the system. -pub struct Interpreter +pub struct Interpreter where H: Handler, { @@ -124,13 +124,30 @@ unsafe impl Sync for Interpreter where /// The value returned by the `Revision` opcode. const INTERPRETER_REVISION: u64 = 1; +impl Interpreter +where + H: Handler, +{ + /// Construct a new [`Interpreter`] using the global allocator. This does not load any tables - if you have an + /// [`crate::AcpiTables`] already, construct an [`AcpiPlatform`] first and then use + /// [`Interpreter::new_from_platform`]. + pub fn new( + handler: H, + dsdt_revision: u8, + registers: Arc>, + facs: Option>, + ) -> Interpreter { + Interpreter::new_in(handler, dsdt_revision, registers, facs, Global) + } +} + impl Interpreter where H: Handler, { - /// Construct a new [`Interpreter`]. This does not load any tables - if you have an + /// Construct a new [`Interpreter`] using the supplied allocator. This does not load any tables - if you have an /// [`crate::AcpiTables`] already, construct an [`AcpiPlatform`] first and then use - /// [`Interpreter::new_from_platform`] + /// [`Interpreter::new_from_platform`]. pub fn new_in( handler: H, dsdt_revision: u8, @@ -3613,7 +3630,7 @@ pub enum Operation { #[derive(Clone)] #[non_exhaustive] -pub enum AmlError { +pub enum AmlError { RunOutOfStream, IllegalOpcode(u16), InvalidFieldFlags, diff --git a/src/aml/namespace.rs b/src/aml/namespace.rs index fb7838f5..d36f8527 100644 --- a/src/aml/namespace.rs +++ b/src/aml/namespace.rs @@ -3,13 +3,13 @@ use super::{ Handle, object::{AmlString, Object, ObjectType, WrappedObject}, }; -use alloc::{collections::btree_map::BTreeMap, string::String, vec::Vec}; +use alloc::{alloc::Global, collections::btree_map::BTreeMap, string::String, vec::Vec}; use bit_field::BitField; use core::{alloc::Allocator, fmt, str}; use log::{trace, warn}; #[derive(Clone)] -pub struct Namespace { +pub struct Namespace { alloc: A, root: NamespaceLevel, } @@ -465,7 +465,7 @@ pub enum NamespaceLevelKind { } #[derive(Clone)] -pub struct NamespaceLevel { +pub struct NamespaceLevel { pub kind: NamespaceLevelKind, pub values: BTreeMap), A>, pub children: BTreeMap, A>, @@ -497,7 +497,7 @@ impl NamespaceLevel { // bounds - `&'static BumpArena` satisfies neither, and the bounds aren't // needed because Vec's own impls don't require A: PartialEq/Debug. #[derive(Clone)] -pub struct AmlName(Vec); +pub struct AmlName(Vec); impl PartialEq> for AmlName { fn eq(&self, other: &AmlName) -> bool { diff --git a/src/aml/object.rs b/src/aml/object.rs index 0a4bc591..69eb901a 100644 --- a/src/aml/object.rs +++ b/src/aml/object.rs @@ -1,5 +1,5 @@ use crate::aml::{AmlError, Handle, IntegerSize, Operation, op_region::OpRegion}; -use alloc::{sync::Arc, vec::Vec}; +use alloc::{alloc::Global, sync::Arc, vec::Vec}; use bit_field::BitField; use core::{alloc::Allocator, cell::UnsafeCell, cmp::Ordering, fmt, ops, sync::atomic::AtomicU64}; @@ -9,7 +9,7 @@ use core::{alloc::Allocator, cell::UnsafeCell, cmp::Ordering, fmt, ops, sync::at // `String` operations the AML interpreter actually needs. Construction sites // always start from validated `&str` literals or copy from existing strings, // so the UTF-8 invariant is straightforward to maintain. -pub struct AmlString(Vec); +pub struct AmlString(Vec); impl AmlString { pub fn new_in(alloc: A) -> Self { @@ -114,7 +114,7 @@ impl fmt::Write for AmlString { type NativeMethod = dyn Fn(&[WrappedObject]) -> Result, AmlError>; #[derive(Clone)] -pub enum Object { +pub enum Object { Uninitialized, Buffer(Vec), BufferField { buffer: WrappedObject, offset: usize, length: usize }, @@ -212,7 +212,7 @@ impl ObjectToken { } #[derive(Clone)] -pub struct WrappedObject(Arc>, A>); +pub struct WrappedObject(Arc>, A>); // Manual Debug impl - derive auto-bounds `A: Debug`. impl fmt::Debug for WrappedObject { @@ -562,7 +562,7 @@ impl Object { } #[derive(Clone)] -pub struct FieldUnit { +pub struct FieldUnit { pub kind: FieldUnitKind, pub flags: FieldFlags, pub bit_index: usize, @@ -579,7 +579,7 @@ impl fmt::Debug for FieldUnit { } #[derive(Clone)] -pub enum FieldUnitKind { +pub enum FieldUnitKind { Normal { region: WrappedObject }, Bank { region: WrappedObject, bank: WrappedObject, bank_value: u64 }, Index { index: WrappedObject, data: WrappedObject }, diff --git a/src/aml/op_region.rs b/src/aml/op_region.rs index 8101d1a7..43e2738f 100644 --- a/src/aml/op_region.rs +++ b/src/aml/op_region.rs @@ -1,8 +1,9 @@ use crate::aml::{AmlError, namespace::AmlName}; +use alloc::alloc::Global; use core::alloc::Allocator; #[derive(Clone)] -pub struct OpRegion { +pub struct OpRegion { pub space: RegionSpace, pub base: u64, pub length: u64, @@ -20,7 +21,7 @@ impl core::fmt::Debug for OpRegion { } } -pub trait RegionHandler { +pub trait RegionHandler { fn read_u8(&self, region: &OpRegion) -> Result>; fn read_u16(&self, region: &OpRegion) -> Result>; fn read_u32(&self, region: &OpRegion) -> Result>; diff --git a/src/aml/pci_routing.rs b/src/aml/pci_routing.rs index 3f1d0b46..8def6a7b 100644 --- a/src/aml/pci_routing.rs +++ b/src/aml/pci_routing.rs @@ -7,7 +7,7 @@ use crate::aml::{ object::Object, resource::{self, InterruptPolarity, InterruptTrigger, Resource}, }; -use alloc::vec::Vec; +use alloc::{alloc::Global, vec::Vec}; use bit_field::BitField; use core::alloc::Allocator; @@ -22,7 +22,7 @@ pub enum Pin { } #[derive(Debug)] -pub enum PciRouteType { +pub enum PciRouteType { /// The interrupt is hard-coded to a specific GSI Gsi(u32), @@ -37,7 +37,7 @@ pub enum PciRouteType { } #[derive(Debug)] -pub struct PciRoute { +pub struct PciRoute { device: u16, function: u16, pin: Pin, @@ -49,7 +49,7 @@ pub struct PciRoute { /// present under each PCI root bridge, and consists of a package of packages, each of which describes the /// mapping of a single PCI interrupt pin. #[derive(Debug)] -pub struct PciRoutingTable { +pub struct PciRoutingTable { entries: Vec, A>, }