From 804b857b15477faa160cfca86d12f53c95d3daf5 Mon Sep 17 00:00:00 2001 From: Isaac Woods Date: Sat, 14 Mar 2026 17:52:14 +0000 Subject: [PATCH 1/8] Remove panics in GAS handling code --- src/address.rs | 71 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/src/address.rs b/src/address.rs index fee34398..0bb3eeb3 100644 --- a/src/address.rs +++ b/src/address.rs @@ -2,6 +2,7 @@ //! in a wide range of address spaces. use crate::{AcpiError, Handler, PhysicalMapping}; +use core::ptr; use log::warn; /// This is the raw form of a Generic Address Structure, and follows the layout found in the ACPI tables. @@ -143,7 +144,7 @@ where } AddressSpace::SystemIo => Ok(MappedGas { gas, handler: handler.clone(), mapping: None }), other => { - warn!("Tried to map GAS of unsupported type {:?}", other); + warn!("Mapping a GAS in address space {:?} is not supported!", other); Err(AcpiError::LibUnimplemented) } } @@ -158,25 +159,24 @@ where match self.gas.address_space { AddressSpace::SystemMemory => { let mapping = self.mapping.as_ref().unwrap(); - let value = match access_size_bits { - 8 => unsafe { core::ptr::read_volatile(mapping.virtual_start.as_ptr() as *const u8) as u64 }, - 16 => unsafe { core::ptr::read_volatile(mapping.virtual_start.as_ptr() as *const u16) as u64 }, - 32 => unsafe { core::ptr::read_volatile(mapping.virtual_start.as_ptr() as *const u32) as u64 }, - 64 => unsafe { core::ptr::read_volatile(mapping.virtual_start.as_ptr() as *const u64) }, - _ => panic!(), - }; - Ok(value) + match access_size_bits { + 8 => Ok(unsafe { ptr::read_volatile(mapping.virtual_start.as_ptr() as *const u8) as u64 }), + 16 => Ok(unsafe { ptr::read_volatile(mapping.virtual_start.as_ptr() as *const u16) as u64 }), + 32 => Ok(unsafe { ptr::read_volatile(mapping.virtual_start.as_ptr() as *const u32) as u64 }), + 64 => Ok(unsafe { ptr::read_volatile(mapping.virtual_start.as_ptr() as *const u64) }), + _ => Err(AcpiError::InvalidGenericAddress), + } } - AddressSpace::SystemIo => { - let value = match access_size_bits { - 8 => self.handler.read_io_u8(self.gas.address as u16) as u64, - 16 => self.handler.read_io_u16(self.gas.address as u16) as u64, - 32 => self.handler.read_io_u32(self.gas.address as u16) as u64, - _ => panic!(), - }; - Ok(value) + AddressSpace::SystemIo => match access_size_bits { + 8 => Ok(self.handler.read_io_u8(self.gas.address as u16) as u64), + 16 => Ok(self.handler.read_io_u16(self.gas.address as u16) as u64), + 32 => Ok(self.handler.read_io_u32(self.gas.address as u16) as u64), + _ => Err(AcpiError::InvalidGenericAddress), + }, + _ => { + warn!("Read from GAS with address space {:?} is not supported. Ignored.", self.gas.address_space); + Err(AcpiError::LibUnimplemented) } - _ => unimplemented!(), } } @@ -188,16 +188,16 @@ where let mapping = self.mapping.as_ref().unwrap(); match access_size_bits { 8 => unsafe { - core::ptr::write_volatile(mapping.virtual_start.as_ptr(), value as u8); + ptr::write_volatile(mapping.virtual_start.as_ptr(), value as u8); }, 16 => unsafe { - core::ptr::write_volatile(mapping.virtual_start.as_ptr() as *mut u16, value as u16); + ptr::write_volatile(mapping.virtual_start.as_ptr() as *mut u16, value as u16); }, 32 => unsafe { - core::ptr::write_volatile(mapping.virtual_start.as_ptr() as *mut u32, value as u32); + ptr::write_volatile(mapping.virtual_start.as_ptr() as *mut u32, value as u32); }, - 64 => unsafe { core::ptr::write_volatile(mapping.virtual_start.as_ptr() as *mut u64, value) }, - _ => panic!(), + 64 => unsafe { ptr::write_volatile(mapping.virtual_start.as_ptr() as *mut u64, value) }, + _ => return Err(AcpiError::InvalidGenericAddress), } Ok(()) } @@ -206,11 +206,14 @@ where 8 => self.handler.write_io_u8(self.gas.address as u16, value as u8), 16 => self.handler.write_io_u16(self.gas.address as u16, value as u16), 32 => self.handler.write_io_u32(self.gas.address as u16, value as u32), - _ => panic!(), + _ => return Err(AcpiError::InvalidGenericAddress), } Ok(()) } - _ => unimplemented!(), + _ => { + warn!("Write to GAS with address space {:?} is not supported. Ignored.", self.gas.address_space); + Err(AcpiError::LibUnimplemented) + } } } } @@ -228,7 +231,7 @@ fn gas_decode_access_bit_width(gas: GenericAddress) -> Result { * * We use a third method, based on the alignment of the address, for registers that have * non-zero bit offsets. These are not typically encountered in normal registers - they very - * often mean the GAS has come from APEI (ACPI Platform Error Interface), and so needs speical + * often mean the GAS has come from APEI (ACPI Platform Error Interface), and so needs special * handling. */ if gas.bit_offset == 0 && [8, 16, 32, 64].contains(&gas.bit_width) { @@ -242,7 +245,19 @@ fn gas_decode_access_bit_width(gas: GenericAddress) -> Result { _ => Err(AcpiError::InvalidGenericAddress), } } else { - // TODO: work out access size based on alignment of the address - todo!() + /* + * Work out the access size based on the alignment of the address. We round up the total + * width (`bit_offset + bit_width`) to the next power-of-two, up to 64. + */ + let total_width = gas.bit_offset + gas.bit_width; + Ok(if total_width <= 8 { + 8 + } else if total_width <= 16 { + 16 + } else if total_width <= 32 { + 32 + } else { + 64 + }) } } From 8749b000dad56765162375868631078c43ac43e9 Mon Sep 17 00:00:00 2001 From: Isaac Woods Date: Sat, 14 Mar 2026 20:15:52 +0000 Subject: [PATCH 2/8] Remove some panics in AML interpreter - Do not panic by default on fatal errors - Do not panic on encountering `DefLoad` or `DefLoadTable` - Do not panic on `DefNotify` - Add correct object type for `RawDataBuffer`s - Correctly handle `AccessField` and `ExtendedAccessField` - Partially handle stores to names --- src/aml/mod.rs | 96 ++++++++++++++++++++++++++++++++++++++++++-------- src/lib.rs | 4 +-- 2 files changed, 84 insertions(+), 16 deletions(-) diff --git a/src/aml/mod.rs b/src/aml/mod.rs index dd92b876..abf88868 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -561,6 +561,17 @@ where }); } } + Opcode::Notify => { + // TODO: may need special handling on the node to get path? + let [Argument::Namestring(name), Argument::Object(value)] = &op.arguments[..] else { + panic!(); + }; + let value = value.as_integer()?; + + info!("Notify {:?} with value {}", name, value); + // TODO: support + return Err(AmlError::LibUnimplemented); + } Opcode::FromBCD => self.do_from_bcd(&mut context, op)?, Opcode::ToBCD => self.do_to_bcd(&mut context, op)?, Opcode::Name => { @@ -581,6 +592,7 @@ where let arg = arg.as_integer()?; self.handler.handle_fatal_error(*typ, *code, arg); context.retire_op(op); + return Err(AmlError::FatalErrorEncountered); } Opcode::OpRegion => { let [ @@ -822,6 +834,35 @@ where context.contribute_arg(Argument::Object(result)); context.retire_op(op); } + Opcode::Load => { + let [Argument::Namestring(object), Argument::Object(result)] = &op.arguments[..] else { + panic!(); + }; + // 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 => { + let [ + Argument::Object(signature), + Argument::Object(oem_id), + Argument::Object(oem_table_id), + Argument::Object(root_path), + Argument::Object(parameter_path), + Argument::Object(parameter_data), + ] = &op.arguments[..] + else { + panic!(); + }; + // TODO: search for the table in the RSDT/XSDT and load the contained AML + warn!( + "Ignoring unsupported DefLoadTable operation (signature = {}, oem_id = {}, oem_table_id = {}, root_path = {}, parameter_path = {}, parameter_data = {})", + signature, oem_id, oem_table_id, root_path, parameter_path, parameter_data + ); + context.retire_op(op); + return Err(AmlError::LibUnimplemented); + } Opcode::Sleep => { let [Argument::Object(msec)] = &op.arguments[..] else { panic!() }; self.handler.sleep(msec.as_integer()?); @@ -936,7 +977,7 @@ where // XXX: 15 is reserved ObjectType::Debug => 16, ObjectType::Reference => panic!(), - ObjectType::RawDataBuffer => todo!(), + ObjectType::RawDataBuffer => 17, }; context.contribute_arg(Argument::Object(Object::Integer(typ).wrap())); @@ -1266,8 +1307,17 @@ where let name = name.resolve(&context.current_scope)?; self.namespace.lock().insert(name, Object::Event(Arc::new(AtomicU64::new(0))).wrap())?; } - Opcode::LoadTable => todo!(), - Opcode::Load => todo!(), + Opcode::LoadTable => { + context.start(OpInFlight::new(Opcode::LoadTable, &[ResolveBehaviour::TermArg; 6])); + } + Opcode::Load => { + let name = context.namestring()?; + context.start(OpInFlight::new_with( + Opcode::Load, + vec![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])), @@ -1737,7 +1787,7 @@ where kind: FieldUnitKind, start_pc: usize, pkg_length: usize, - flags: u8, + mut flags: u8, ) -> Result<(), AmlError> { const RESERVED_FIELD: u8 = 0x00; const ACCESS_FIELD: u8 = 0x01; @@ -1758,18 +1808,22 @@ where * elements. They change the access type and attributes for remaining fields in * the list. */ - let _access_type = context.next()?; + let access_type = context.next()?; let _access_attrib = context.next()?; - todo!() + flags.set_bits(0..4, access_type); + } + EXTENDED_ACCESS_FIELD => { + let access_type = context.next()?; + let _extended_access_attrib = context.next()?; + let _access_length = context.next()?; + flags.set_bits(0..4, access_type); + warn!("Ignoring extended attributes and length in ExtendedAccessField"); } CONNECT_FIELD => { // TODO: either consume a namestring or `BufferData` (it's not // clear what a buffer data acc is lmao) todo!("Connect field :("); } - EXTENDED_ACCESS_FIELD => { - todo!("Extended access field :("); - } _ => { context.current_block.pc -= 1; // TODO: this should only ever be a nameseg really... @@ -2152,8 +2206,7 @@ where Object::Package(_) => "[Package]".to_string(), Object::PowerResource { .. } => "[Power Resource]".to_string(), Object::Processor { .. } => "[Processor]".to_string(), - // TODO: what even is one of these?? - Object::RawDataBuffer => todo!(), + Object::RawDataBuffer => "[Raw Data Buffer]".to_string(), Object::String(value) => value.clone(), Object::ThermalZone => "[Thermal Zone]".to_string(), Object::Debug => "[Debug Object]".to_string(), @@ -2368,10 +2421,21 @@ where _ => panic!("Stores to objects like {:?} are not yet supported", target), }, - Argument::Namestring(_) => todo!(), - Argument::ByteData(_) | Argument::DWordData(_) | Argument::TrackedPc(_) | Argument::PkgLength(_) => { - panic!() + Argument::Namestring(name) => { + let existing = self.namespace.lock().get(name.clone()); + match existing { + Ok(existing) => { + unsafe { + // TODO: this should likely be doing an implicit cast depending on object type? + *existing.gain_mut(&token) = (*object).clone(); + } + } + Err(_) => { + self.namespace.lock().insert(name.clone(), object)?; + } + } } + _ => panic!("Invalid argument type as DefStore target"), } Ok(to_return) @@ -3408,6 +3472,10 @@ pub enum AmlError { PrtInvalidSource, PrtNoEntry, + /// An OEM-defined fatal error has occured. The specification states a host should log this + /// fatal error and then shutdown in a timely fashion. + FatalErrorEncountered, + /// This is emitted to signal that the library does not support the requested behaviour. This /// should eventually never be emitted. LibUnimplemented, diff --git a/src/lib.rs b/src/lib.rs index 03e4c290..91b53697 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -72,7 +72,7 @@ use core::{ pin::Pin, ptr::NonNull, }; -use log::warn; +use log::{error, warn}; use rsdp::Rsdp; /// `AcpiTables` should be constructed after finding the RSDP or RSDT/XSDT and allows enumeration @@ -480,7 +480,7 @@ pub trait Handler: Clone { #[cfg(feature = "aml")] fn handle_fatal_error(&self, fatal_type: u8, fatal_code: u32, fatal_arg: u64) { - panic!( + error!( "Fatal error while executing AML (encountered DefFatalOp). fatal_type = {}, fatal_code = {}, fatal_arg = {}", fatal_type, fatal_code, fatal_arg ); From f24e917b3ac7152a3cd4bc52bc4e9b513915a0f9 Mon Sep 17 00:00:00 2001 From: Isaac Woods Date: Sun, 5 Apr 2026 22:49:53 +0100 Subject: [PATCH 3/8] Do not terminate on errors while populating namespace from DSDT + SSDTs --- src/aml/mod.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/aml/mod.rs b/src/aml/mod.rs index abf88868..8a3e01b6 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -47,7 +47,7 @@ use core::{ str::FromStr, sync::atomic::{AtomicU64, Ordering}, }; -use log::{info, trace, warn}; +use log::{error, info, trace, warn}; use namespace::{AmlName, Namespace, NamespaceLevelKind}; use object::{ DeviceStatus, @@ -148,10 +148,15 @@ where let dsdt = platform.tables.dsdt()?; let interpreter = Interpreter::new(platform.handler.clone(), dsdt.revision, registers, facs); - load_table(&interpreter, dsdt)?; - for ssdt in platform.tables.ssdts() { - load_table(&interpreter, ssdt)?; + if let Err(err) = load_table(&interpreter, dsdt) { + error!("Error while loading DSDT: {:?}. Continuing; this may cause downstream errors.", err); + } + + for (i, ssdt) in platform.tables.ssdts().enumerate() { + if let Err(err) = load_table(&interpreter, ssdt) { + error!("Error while loading SSDT{}: {:?}. Continuing.", i, err); + } } Ok(interpreter) @@ -2048,6 +2053,7 @@ where } Object::String(ref value) => { /* + * TODO: * This is about the same level of effort as ACPICA puts in. The uACPI test suite * has tests that this fails - namely because of support for octal, signs, strings * that won't fit in a `u64` etc. We probably need to write a more robust parser @@ -2056,9 +2062,7 @@ where 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) - } + 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), }; match value.len() { From 5cd3857cc92c3de494dee3862d91f6be088aac2b Mon Sep 17 00:00:00 2001 From: Isaac Woods Date: Wed, 8 Apr 2026 16:39:43 +0100 Subject: [PATCH 4/8] Improve correctness of `DefStore` behaviour --- src/aml/mod.rs | 256 +++++++++++++++++++++++++--------------------- src/aml/object.rs | 66 ++++++++++-- 2 files changed, 195 insertions(+), 127 deletions(-) diff --git a/src/aml/mod.rs b/src/aml/mod.rs index 8a3e01b6..0cef51ab 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -424,6 +424,7 @@ where } Opcode::Increment | Opcode::Decrement => { let [Argument::Object(operand)] = &op.arguments[..] else { panic!() }; + let operand = operand.clone().unwrap_transparent_reference(); let token = self.object_token.lock(); let Object::Integer(operand) = (unsafe { operand.gain_mut(&token) }) else { Err(AmlError::ObjectNotOfExpectedType { @@ -461,7 +462,8 @@ where Opcode::Mid => self.do_mid(&mut context, op)?, Opcode::Concat => self.do_concat(&mut context, op)?, Opcode::ConcatRes => { - let [Argument::Object(source1), Argument::Object(source2), target] = &op.arguments[..] + let [Argument::Object(source1), Argument::Object(source2), Argument::Object(target)] = + &op.arguments[..] else { panic!() }; @@ -477,7 +479,7 @@ where Object::Buffer(buffer).wrap() }; // TODO: use potentially-updated result for return value here - self.do_store(target, result.clone())?; + self.do_store(target.clone(), result.clone())?; context.contribute_arg(Argument::Object(result)); context.retire_op(op); } @@ -798,8 +800,10 @@ where context.retire_op(op); } Opcode::Store => { - let [Argument::Object(object), target] = &op.arguments[..] else { panic!() }; - self.do_store(target, object.clone())?; + let [Argument::Object(object), Argument::Object(target)] = &op.arguments[..] else { + panic!() + }; + self.do_store(target.clone(), object.clone())?; context.retire_op(op); } Opcode::RefOf => { @@ -810,13 +814,15 @@ where context.retire_op(op); } Opcode::CondRefOf => { - let [Argument::Object(object), target] = &op.arguments[..] else { panic!() }; + let [Argument::Object(object), Argument::Object(target)] = &op.arguments[..] else { + panic!() + }; 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(); - self.do_store(target, reference)?; + self.do_store(target.clone(), reference)?; Object::Integer(u64::MAX) }; context.contribute_arg(Argument::Object(result.wrap())); @@ -1022,6 +1028,7 @@ where */ let [Argument::Object(predicate)] = &op.arguments[..] else { panic!() }; let predicate = predicate.as_integer()?; + let predicate = predicate.clone().unwrap_transparent_reference().as_integer()?; if predicate == 0 { // Exit from the while loop by skipping out of the current block @@ -1161,15 +1168,11 @@ where match opcode { Opcode::Zero => { /* - * This represents a `Zero` operand that should create an `Integer` object in + * This represents a `Zero` operand that should create an `Integer` operand in * most places, but could also encode a `NullName` if we are expecting a - * `Target`. + * `Target`. We handle the latter in logic for stores to targets. */ - if context.last_op()?.resolve_behaviour() == ResolveBehaviour::Target { - context.last_op()?.arguments.push(Argument::Null); - } else { - context.last_op()?.arguments.push(Argument::Object(Object::Integer(0).wrap())); - } + context.last_op()?.arguments.push(Argument::Object(Object::Integer(0).wrap())); } Opcode::One => { context.last_op()?.arguments.push(Argument::Object(Object::Integer(1).wrap())); @@ -1519,14 +1522,15 @@ where Opcode::Local(local) => { let local = context.locals[local as usize].clone(); context.last_op()?.arguments.push(Argument::Object( - Object::Reference { kind: ReferenceKind::LocalOrArg, inner: local }.wrap(), + Object::Reference { kind: ReferenceKind::Local, inner: local }.wrap(), )); } Opcode::Arg(arg) => { let arg = context.args[arg as usize].clone(); - context.last_op()?.arguments.push(Argument::Object( - Object::Reference { kind: ReferenceKind::LocalOrArg, inner: arg }.wrap(), - )); + context + .last_op()? + .arguments + .push(Argument::Object(Object::Reference { kind: ReferenceKind::Arg, inner: arg }.wrap())); } Opcode::Store => context.start(OpInFlight::new( Opcode::Store, @@ -1554,11 +1558,13 @@ where .unwrap_or(ResolveBehaviour::TermArg); match behaviour { // XXX: `NullName` is handled separately given its ambiguity with `Zero` - ResolveBehaviour::SuperName | ResolveBehaviour::Target => { + ResolveBehaviour::SimpleName | ResolveBehaviour::SuperName | ResolveBehaviour::Target => { let object = self.namespace.lock().search(&name, &context.current_scope); match object { Ok((_resolved_name, object)) => { - context.last_op()?.arguments.push(Argument::Object(object)); + context.last_op()?.arguments.push(Argument::Object( + Object::Reference { kind: ReferenceKind::Named, inner: object }.wrap(), + )); } Err(err) => Err(err)?, } @@ -1606,6 +1612,9 @@ where } else if let Object::FieldUnit(ref field) = *object { let value = self.do_field_read(field)?; context.last_op()?.arguments.push(Argument::Object(value)); + } else if let Object::BufferField { .. } = *object { + let value = object.read_buffer_field(self.integer_size())?; + context.last_op()?.arguments.push(Argument::Object(value.wrap())); } else { context.last_op()?.arguments.push(Argument::Object(object)); } @@ -1852,7 +1861,10 @@ where } fn do_binary_maths(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - let [Argument::Object(left), Argument::Object(right), target] = &op.arguments[0..3] else { panic!() }; + let [Argument::Object(left), Argument::Object(right), Argument::Object(target)] = &op.arguments[0..3] + else { + panic!() + }; let target2 = if op.op == Opcode::Divide { Some(&op.arguments[3]) } else { None }; let left = left.clone().unwrap_transparent_reference().as_integer()?; @@ -1863,8 +1875,8 @@ where Opcode::Subtract => left.wrapping_sub(right), Opcode::Multiply => left.wrapping_mul(right), Opcode::Divide => { - if let Some(remainder) = target2 { - self.do_store(remainder, Object::Integer(left.wrapping_rem(right)).wrap())?; + if let Some(Argument::Object(remainder)) = target2 { + self.do_store(remainder.clone(), Object::Integer(left.wrapping_rem(right)).wrap())?; } left.wrapping_div_euclid(right) } @@ -1880,7 +1892,7 @@ where }; let result = Object::Integer(result).wrap(); - let result = self.do_store(target, result)?; + let result = self.do_store(target.clone(), result)?; context.contribute_arg(Argument::Object(result)); context.retire_op(op); Ok(()) @@ -2004,7 +2016,7 @@ where } fn do_to_buffer(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - let [Argument::Object(operand), target] = &op.arguments[..] else { panic!() }; + let [Argument::Object(operand), Argument::Object(target)] = &op.arguments[..] else { panic!() }; let operand = operand.clone().unwrap_transparent_reference(); let result = match *operand { @@ -2030,14 +2042,14 @@ where } .wrap(); - let result = self.do_store(target, result)?; + let result = self.do_store(target.clone(), result)?; context.contribute_arg(Argument::Object(result)); context.retire_op(op); Ok(()) } fn do_to_integer(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - let [Argument::Object(operand), target] = &op.arguments[..] else { panic!() }; + let [Argument::Object(operand), Argument::Object(target)] = &op.arguments[..] else { panic!() }; let operand = operand.clone().unwrap_transparent_reference(); let result = match *operand { @@ -2076,14 +2088,17 @@ where } .wrap(); - let result = self.do_store(target, result)?; + let result = self.do_store(target.clone(), result)?; context.contribute_arg(Argument::Object(result)); context.retire_op(op); Ok(()) } fn do_to_string(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - let [Argument::Object(source), Argument::Object(length), target] = &op.arguments[..] else { panic!() }; + let [Argument::Object(source), Argument::Object(length), Argument::Object(target)] = &op.arguments[..] + else { + panic!() + }; let source = source.clone().unwrap_transparent_reference(); let source = source.as_buffer()?; let length = length.clone().unwrap_transparent_reference().as_integer()? as usize; @@ -2103,7 +2118,7 @@ where } .wrap(); - let result = self.do_store(target, result)?; + let result = self.do_store(target.clone(), result)?; context.contribute_arg(Argument::Object(result)); context.retire_op(op); Ok(()) @@ -2111,7 +2126,7 @@ where /// Perform a `ToDecimalString` or `ToHexString` operation fn do_to_dec_hex_string(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - let [Argument::Object(operand), target] = &op.arguments[..] else { panic!() }; + let [Argument::Object(operand), Argument::Object(target)] = &op.arguments[..] else { panic!() }; let operand = operand.clone().unwrap_transparent_reference(); let result = match *operand { @@ -2145,15 +2160,19 @@ where } .wrap(); - let result = self.do_store(target, result)?; + let result = self.do_store(target.clone(), result)?; context.contribute_arg(Argument::Object(result)); context.retire_op(op); Ok(()) } fn do_mid(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - let [Argument::Object(source), Argument::Object(index), Argument::Object(length), target] = - &op.arguments[..] + let [ + Argument::Object(source), + Argument::Object(index), + Argument::Object(length), + Argument::Object(target), + ] = &op.arguments[..] else { panic!() }; @@ -2183,14 +2202,18 @@ where } .wrap(); - self.do_store(target, result.clone())?; + self.do_store(target.clone(), result.clone())?; context.contribute_arg(Argument::Object(result)); context.retire_op(op); Ok(()) } fn do_concat(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - let [Argument::Object(source1), Argument::Object(source2), target] = &op.arguments[..] else { panic!() }; + // TODO + let [Argument::Object(source1), Argument::Object(source2), Argument::Object(target)] = &op.arguments[..] + else { + panic!() + }; let source1 = source1.clone().unwrap_transparent_reference(); let source2 = source2.clone().unwrap_transparent_reference(); @@ -2243,7 +2266,7 @@ where } }; - let result = self.do_store(target, result)?; + let result = self.do_store(target.clone(), result)?; context.contribute_arg(Argument::Object(result)); context.retire_op(op); Ok(()) @@ -2300,7 +2323,10 @@ where } fn do_index(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - let [Argument::Object(object), Argument::Object(index_value), target] = &op.arguments[..] else { + // TODO + let [Argument::Object(object), Argument::Object(index_value), Argument::Object(target)] = + &op.arguments[..] + else { panic!() }; let object = object.clone().unwrap_transparent_reference(); @@ -2345,104 +2371,93 @@ where } .wrap(); - self.do_store(target, result.clone())?; + self.do_store(target.clone(), result.clone())?; context.contribute_arg(Argument::Object(result)); context.retire_op(op); Ok(()) } - fn do_store(&self, target: &Argument, object: WrappedObject) -> Result { + /// Perform a store of `object` into `target`, matching the expected behaviour of `DefStore`, + /// which depends on the target: + /// - Locals are overwritten, unless they contain a reference, in which case a store is + /// performed to the referenced object with implicit casting + /// - Args are overwritten, unless they contain a reference, in which case the referenced + /// 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 { let object = object.unwrap_transparent_reference(); let token = self.object_token.lock(); - /* - * TODO: stores should do more implicit conversion to the type of the destination in some - * cases, in line with section 19.3.5 of the spec. It's not clear what existing - * interpreters do - NT may just be memcpying objects over each other... - * - * TODO: stores to fields with `BufferAcc` can actually return a value of the store that - * differs from what was written into the field. This is used for complex field types with - * a write-then-read pattern. The return value is then used as the 'result' of the storing - * expression. - */ - let to_return = object.clone(); - - match target { - Argument::Null => {} - Argument::Object(target) => match unsafe { target.gain_mut(&token) } { - Object::Integer(target) => match unsafe { object.gain_mut(&token) } { - Object::Integer(value) => { - *target = *value; - } - Object::BufferField { .. } => { - let mut buffer = [0u8; 8]; - unsafe { object.gain_mut(&token) }.read_buffer_field(&mut buffer)?; - let value = u64::from_le_bytes(buffer); - *target = value; - } - Object::FieldUnit(field) => { - // TODO: not sure if we should convert buffers to integers if needed here? - *target = self.do_field_read(field)?.as_integer()?; + match unsafe { target.gain_mut(&token) } { + Object::Reference { kind, inner } => { + let (target_object, overwrite) = match kind { + ReferenceKind::Named => (inner.clone().unwrap_reference(), false), + ReferenceKind::Local | ReferenceKind::Index => { + if let Object::Reference { kind: _, inner: ref inner_inner } = **inner { + (inner_inner.clone(), false) + } else { + (inner.clone().unwrap_transparent_reference(), true) + } } - _ => { - let as_integer = object.to_integer(if self.dsdt_revision >= 2 { 8 } else { 4 })?; - *target = as_integer; + ReferenceKind::Arg => { + if let Object::Reference { kind: _, inner: ref inner_inner } = **inner { + (inner_inner.clone(), true) + } else { + (inner.clone().unwrap_transparent_reference(), true) + } } - }, - Object::BufferField { .. } => match unsafe { object.gain_mut(&token) } { - Object::Integer(value) => { - unsafe { target.gain_mut(&token) }.write_buffer_field(&value.to_le_bytes(), &token)?; + ReferenceKind::RefOf | ReferenceKind::Unresolved => { + return Err(AmlError::StoreToInvalidReferenceType); } - Object::Buffer(value) => { - unsafe { target.gain_mut(&token) }.write_buffer_field(value.as_slice(), &token)?; + }; + + if overwrite { + unsafe { + *target_object.gain_mut(&token) = (*object).clone(); } - _ => panic!(), - }, - Object::FieldUnit(field) => self.do_field_write(field, object)?, - Object::Reference { kind, inner } => { - match kind { - ReferenceKind::RefOf => todo!(), - ReferenceKind::LocalOrArg => { - if let Object::Reference { kind: _inner_kind, inner: inner_inner } = &**inner { - // TODO: this should store into the reference, potentially doing an - // implicit cast - unsafe { - *inner_inner.gain_mut(&token) = object.gain_mut(&token).clone(); + } else { + match &*target_object { + Object::Integer(_) | Object::String(_) | Object::Buffer(_) => { + let target_object = unsafe { target_object.gain_mut(&token) }; + target_object.replace_with_implicit_casting((*object).clone())?; + } + Object::BufferField { .. } => { + let target_object = unsafe { target_object.gain_mut(&token) }; + match unsafe { object.gain_mut(&token) } { + Object::Integer(value) => { + target_object.write_buffer_field(&value.to_le_bytes(), &token)? } - } else { - // Overwrite the value - unsafe { - *inner.gain_mut(&token) = object.gain_mut(&token).clone(); + Object::Buffer(value) => { + target_object.write_buffer_field(value.as_slice(), &token)? } + _ => panic!(), } } - ReferenceKind::Unresolved => todo!(), - } - } - Object::Debug => { - self.handler.handle_debug(&object); - } - _ => panic!("Stores to objects like {:?} are not yet supported", target), - }, - Argument::Namestring(name) => { - let existing = self.namespace.lock().get(name.clone()); - match existing { - Ok(existing) => { - unsafe { - // TODO: this should likely be doing an implicit cast depending on object type? - *existing.gain_mut(&token) = (*object).clone(); + /* + * TODO: stores to fields with BufferAcc can return a different value to the + * object stored. This is used for complex field types with a + * write-then-read pattern. We should perform a read in those cases and + * return that instead. + */ + Object::FieldUnit(field_unit) => self.do_field_write(field_unit, object.clone())?, + + _ => { + return Err(AmlError::InvalidOperationOnObject { + op: Operation::Store, + typ: target_object.typ(), + }); } } - Err(_) => { - self.namespace.lock().insert(name.clone(), object)?; - } } } - _ => panic!("Invalid argument type as DefStore target"), + Object::Debug => self.handler.handle_debug(&object), + Object::Integer(0) => {} // Store to NullName + _ => return Err(AmlError::InvalidOperationOnObject { op: Operation::Store, typ: target.typ() }), } - Ok(to_return) + Ok(object) } /// Do a read from a field by performing one or more well-formed accesses to the underlying @@ -2851,9 +2866,12 @@ enum ResolveBehaviour { /// no forward definitions in AML, so this is the usual resolution behaviour for most operands. TermArg, /// Resolve a name to reference an object. This is used when an operation needs to operate on - /// the object itself, rather than evaluate it to a value. For example, accessing a `FieldUnit` would - /// read a value from the field with `TermArg`, but resolves to the `FieldUnit` with - /// this behaviour. + /// the object itself, rather than evaluate it to a value (for example, given a `FieldUnit`, `TermArg` would read a + /// value from the field, while this would resolve to the `FieldUnit` itself. Can be a name, + /// argument, or local. + SimpleName, + /// Like a `SimpleName`, but can also be the `Debug` object or an operation that produces a + /// reference. SuperName, /// Behaves the same as `SuperName` if the object exists, but resolves successfully to an /// unresolved reference if the object does not exist. Used by `DefCondRefOf`. @@ -2880,7 +2898,6 @@ struct OpInFlight { #[derive(Debug)] enum Argument { - Null, Object(WrappedObject), Namestring(AmlName), ByteData(u8), @@ -3425,6 +3442,8 @@ pub enum Operation { DecodePrt, ParseResource, + Store, + ResetEvent, SignalEvent, WaitEvent, @@ -3463,6 +3482,11 @@ pub enum AmlError { expected: ObjectType, got: ObjectType, }, + InvalidImplicitCast { + from: ObjectType, + to: ObjectType, + }, + StoreToInvalidReferenceType, InvalidResourceDescriptor, UnexpectedResourceType, diff --git a/src/aml/object.rs b/src/aml/object.rs index 769be18f..d79af4b1 100644 --- a/src/aml/object.rs +++ b/src/aml/object.rs @@ -1,5 +1,10 @@ use crate::aml::{AmlError, Handle, Operation, op_region::OpRegion}; -use alloc::{borrow::Cow, string::String, sync::Arc, vec::Vec}; +use alloc::{ + borrow::Cow, + string::{String, ToString}, + sync::Arc, + vec::Vec, +}; use bit_field::BitField; use core::{cell::UnsafeCell, fmt, ops, sync::atomic::AtomicU64}; @@ -138,11 +143,8 @@ impl WrappedObject { pub fn unwrap_transparent_reference(self) -> WrappedObject { let mut object = self; loop { - // TODO: what should this do with unresolved namestrings? It would need namespace - // access to resolve them (and then this would probs have to move to a method on - // `Interpreter`)? if let Object::Reference { kind, ref inner } = *object - && kind == ReferenceKind::LocalOrArg + && (kind == ReferenceKind::Local || kind == ReferenceKind::Arg || kind == ReferenceKind::Named) { object = inner.clone(); } else { @@ -229,16 +231,22 @@ impl Object { } } - pub fn read_buffer_field(&self, dst: &mut [u8]) -> Result<(), AmlError> { + pub fn read_buffer_field(&self, integer_size: usize) -> Result { if let Self::BufferField { buffer, offset, length } = self { let buffer = match **buffer { Object::Buffer(ref buffer) => buffer.as_slice(), Object::String(ref string) => string.as_bytes(), _ => panic!(), }; - // TODO: bounds check the buffer first to avoid panicking - copy_bits(buffer, *offset, dst, 0, *length); - Ok(()) + if *length <= integer_size { + let mut dst = [0u8; 8]; + 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)]; + copy_bits(buffer, *offset, &mut dst, 0, *length); + Ok(Object::Buffer(dst)) + } } else { Err(AmlError::InvalidOperationOnObject { op: Operation::ReadBufferField, typ: self.typ() }) } @@ -261,6 +269,36 @@ 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(), + Object::String(ref value) => value.as_bytes(), + Object::Buffer(ref value) => &value.clone(), + _ => return Err(AmlError::InvalidImplicitCast { from: self.typ(), to: new.typ() }), + }; + + 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(); + } + _ => return Err(AmlError::InvalidImplicitCast { from: self.typ(), to: new.typ() }), + } + + Ok(()) + } + /// Returns the `ObjectType` of this object. Returns the type of the referenced object in the /// case of `Object::Reference`. pub fn typ(&self) -> ObjectType { @@ -275,7 +313,10 @@ impl Object { Object::Method { .. } => ObjectType::Method, Object::NativeMethod { .. } => ObjectType::Method, Object::Mutex { .. } => ObjectType::Mutex, - Object::Reference { inner, .. } => inner.typ(), + // Object::Reference { inner, .. } => inner.typ(), + Object::Reference { .. } => ObjectType::Reference, // TODO: maybe this should + // differentiate internal/real + // references? Object::OpRegion(_) => ObjectType::OpRegion, Object::Package(_) => ObjectType::Package, Object::PowerResource { .. } => ObjectType::PowerResource, @@ -383,8 +424,11 @@ impl MethodFlags { #[derive(Clone, Copy, PartialEq, Debug)] pub enum ReferenceKind { + Named, RefOf, - LocalOrArg, + Local, + Arg, + Index, Unresolved, } From 3a234ac266706a3e9b277e9271ddcf4179a6415d Mon Sep 17 00:00:00 2001 From: Isaac Woods Date: Wed, 8 Apr 2026 16:48:03 +0100 Subject: [PATCH 5/8] Introduce `extract_args` macro for ergonomics --- src/aml/mod.rs | 285 ++++++++++++++++++++++--------------------------- 1 file changed, 125 insertions(+), 160 deletions(-) diff --git a/src/aml/mod.rs b/src/aml/mod.rs index 0cef51ab..bedde331 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -66,6 +66,31 @@ 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)]); +/// ``` +macro_rules! extract_args { + ($op:ident => $args:tt) => { + let $args = &$op.arguments[..] else { + return Err(AmlError::InternalError(alloc::format!( + "Operation has invalid argument types: {}", + stringify!($args) + ))); + }; + }; + ($op:ident[$x:expr] => $args:tt) => { + let $args = &$op.arguments[$x] else { + return Err(AmlError::InternalError(alloc::format!( + "Operation has invalid argument types: {}", + stringify!($args) + ))); + }; + }; +} + /// `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. @@ -416,9 +441,7 @@ where | Opcode::And | Opcode::Or | Opcode::Nor - | Opcode::Xor => { - self.do_binary_maths(&mut context, op)?; - } + | Opcode::Xor => self.do_binary_maths(&mut context, op)?, Opcode::Not | Opcode::FindSetLeftBit | Opcode::FindSetRightBit => { self.do_unary_maths(&mut context, op)?; } @@ -426,6 +449,7 @@ where let [Argument::Object(operand)] = &op.arguments[..] else { panic!() }; let operand = operand.clone().unwrap_transparent_reference(); let token = self.object_token.lock(); + let Object::Integer(operand) = (unsafe { operand.gain_mut(&token) }) else { Err(AmlError::ObjectNotOfExpectedType { expected: ObjectType::Integer, @@ -450,9 +474,7 @@ where | Opcode::LGreaterEqual | Opcode::LEqual | Opcode::LGreater - | Opcode::LLess => { - self.do_logical_op(&mut context, op)?; - } + | Opcode::LLess => self.do_logical_op(&mut context, op)?, Opcode::ToBuffer => self.do_to_buffer(&mut context, op)?, Opcode::ToInteger => self.do_to_integer(&mut context, op)?, Opcode::ToString => self.do_to_string(&mut context, op)?, @@ -462,11 +484,11 @@ where Opcode::Mid => self.do_mid(&mut context, op)?, Opcode::Concat => self.do_concat(&mut context, op)?, Opcode::ConcatRes => { - let [Argument::Object(source1), Argument::Object(source2), Argument::Object(target)] = - &op.arguments[..] - else { - panic!() - }; + extract_args!(op => [ + Argument::Object(source1), + Argument::Object(source2), + Argument::Object(target) + ]); let source1 = source1.as_buffer()?; let source2 = source2.as_buffer()?; let result = { @@ -484,9 +506,7 @@ where context.retire_op(op); } Opcode::Reset => { - let [Argument::Object(sync_object)] = &op.arguments[..] else { - panic!(); - }; + extract_args!(op => [Argument::Object(sync_object)]); let sync_object = sync_object.clone().unwrap_reference(); if let Object::Event(ref counter) = *sync_object { @@ -499,9 +519,7 @@ where } } Opcode::Signal => { - let [Argument::Object(sync_object)] = &op.arguments[..] else { - panic!(); - }; + extract_args!(op => [Argument::Object(sync_object)]); let sync_object = sync_object.clone().unwrap_reference(); if let Object::Event(ref counter) = *sync_object { @@ -514,9 +532,7 @@ where } } Opcode::Wait => { - let [Argument::Object(sync_object), Argument::Object(timeout)] = &op.arguments[..] else { - panic!(); - }; + extract_args!(op => [Argument::Object(sync_object), Argument::Object(timeout)]); let sync_object = sync_object.clone().unwrap_reference(); let timeout = u64::min(timeout.as_integer()?, 0xffff); @@ -570,9 +586,7 @@ where } Opcode::Notify => { // TODO: may need special handling on the node to get path? - let [Argument::Namestring(name), Argument::Object(value)] = &op.arguments[..] else { - panic!(); - }; + extract_args!(op => [Argument::Namestring(name), Argument::Object(value)]); let value = value.as_integer()?; info!("Notify {:?} with value {}", name, value); @@ -582,36 +596,25 @@ where Opcode::FromBCD => self.do_from_bcd(&mut context, op)?, Opcode::ToBCD => self.do_to_bcd(&mut context, op)?, Opcode::Name => { - let [Argument::Namestring(name), Argument::Object(object)] = &op.arguments[..] else { - panic!() - }; - + extract_args!(op => [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 => { - let [Argument::ByteData(typ), Argument::DWordData(code), Argument::Object(arg)] = - &op.arguments[..] - else { - panic!() - }; + extract_args!(op => [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 => { - let [ + extract_args!(op => [ Argument::Namestring(name), Argument::ByteData(region_space), Argument::Object(region_offset), Argument::Object(region_length), - ] = &op.arguments[..] - else { - panic!() - }; - + ]); let region_offset = region_offset.clone().unwrap_transparent_reference(); let region_length = region_length.clone().unwrap_transparent_reference(); @@ -625,15 +628,12 @@ where context.retire_op(op); } Opcode::DataRegion => { - let [ + extract_args!(op => [ Argument::Namestring(name), Argument::Object(signature), Argument::Object(oem_id), Argument::Object(oem_table_id), - ] = &op.arguments[..] - else { - panic!() - }; + ]); let _signature = signature.as_string()?; let _oem_id = oem_id.as_string()?; let _oem_table_id = oem_table_id.as_string()?; @@ -653,14 +653,11 @@ where context.retire_op(op); } Opcode::Buffer => { - let [ + extract_args!(op => [ Argument::TrackedPc(start_pc), Argument::PkgLength(pkg_length), Argument::Object(buffer_size), - ] = &op.arguments[..] - else { - panic!() - }; + ]); let buffer_size = buffer_size.clone().unwrap_transparent_reference().as_integer()?; let buffer_len = pkg_length - (context.current_block.pc - start_pc); @@ -677,7 +674,11 @@ where Opcode::Package => { let mut elements = Vec::with_capacity(op.expected_arguments); for arg in &op.arguments { - let Argument::Object(object) = arg else { panic!() }; + let Argument::Object(object) = arg else { + return Err(AmlError::InternalError( + "Invalid argument type produced for package element".to_string(), + )); + }; elements.push(object.clone()); } @@ -699,13 +700,17 @@ where context.retire_op(op); } Opcode::VarPackage => { - let Argument::Object(total_elements) = &op.arguments[0] else { panic!() }; + extract_args!(op[0..1] => [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); for arg in &op.arguments[1..] { - let Argument::Object(object) = arg else { panic!() }; + let Argument::Object(object) = arg else { + return Err(AmlError::InternalError( + "Invalid argument type produced for package element".to_string(), + )); + }; elements.push(object.clone()); } @@ -720,14 +725,11 @@ where context.retire_op(op); } Opcode::If => { - let [ + extract_args!(op => [ Argument::TrackedPc(start_pc), Argument::PkgLength(then_length), Argument::Object(predicate), - ] = &op.arguments[..] - else { - panic!() - }; + ]); let predicate = predicate.as_integer()?; let remaining_then_length = then_length - (context.current_block.pc - start_pc); @@ -759,9 +761,7 @@ where | opcode @ Opcode::CreateWordField | opcode @ Opcode::CreateDWordField | opcode @ Opcode::CreateQWordField => { - let [Argument::Object(buffer), Argument::Object(index)] = &op.arguments[..] else { - panic!() - }; + extract_args!(op => [Argument::Object(buffer), Argument::Object(index)]); let name = context.namestring()?; let index = index.as_integer()?; let (offset, length) = match opcode { @@ -779,11 +779,7 @@ where context.retire_op(op); } Opcode::CreateField => { - let [Argument::Object(buffer), Argument::Object(bit_index), Argument::Object(num_bits)] = - &op.arguments[..] - else { - panic!() - }; + extract_args!(op => [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()?; @@ -800,23 +796,19 @@ where context.retire_op(op); } Opcode::Store => { - let [Argument::Object(object), Argument::Object(target)] = &op.arguments[..] else { - panic!() - }; + extract_args!(op => [Argument::Object(object), Argument::Object(target)]); self.do_store(target.clone(), object.clone())?; context.retire_op(op); } Opcode::RefOf => { - let [Argument::Object(object)] = &op.arguments[..] else { panic!() }; + extract_args!(op => [Argument::Object(object)]); let reference = Object::Reference { kind: ReferenceKind::RefOf, inner: object.clone() }.wrap(); context.contribute_arg(Argument::Object(reference)); context.retire_op(op); } Opcode::CondRefOf => { - let [Argument::Object(object), Argument::Object(target)] = &op.arguments[..] else { - panic!() - }; + extract_args!(op => [Argument::Object(object), Argument::Object(target)]); let result = if let Object::Reference { kind: ReferenceKind::Unresolved, .. } = **object { Object::Integer(0) } else { @@ -829,7 +821,7 @@ where context.retire_op(op); } Opcode::DerefOf => { - let [Argument::Object(object)] = &op.arguments[..] else { panic!() }; + extract_args!(op => [Argument::Object(object)]); let result = if object.typ() == ObjectType::Reference { object.clone().unwrap_reference() } else if object.typ() == ObjectType::String { @@ -846,26 +838,21 @@ where context.retire_op(op); } Opcode::Load => { - let [Argument::Namestring(object), Argument::Object(result)] = &op.arguments[..] else { - panic!(); - }; + extract_args!(op => [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 => { - let [ + extract_args!(op => [ Argument::Object(signature), Argument::Object(oem_id), Argument::Object(oem_table_id), Argument::Object(root_path), Argument::Object(parameter_path), Argument::Object(parameter_data), - ] = &op.arguments[..] - else { - panic!(); - }; + ]); // TODO: search for the table in the RSDT/XSDT and load the contained AML warn!( "Ignoring unsupported DefLoadTable operation (signature = {}, oem_id = {}, oem_table_id = {}, root_path = {}, parameter_path = {}, parameter_data = {})", @@ -875,17 +862,17 @@ where return Err(AmlError::LibUnimplemented); } Opcode::Sleep => { - let [Argument::Object(msec)] = &op.arguments[..] else { panic!() }; + extract_args!(op => [Argument::Object(msec)]); self.handler.sleep(msec.as_integer()?); context.retire_op(op); } Opcode::Stall => { - let [Argument::Object(usec)] = &op.arguments[..] else { panic!() }; + extract_args!(op => [Argument::Object(usec)]); self.handler.stall(usec.as_integer()?); context.retire_op(op); } Opcode::Acquire => { - let [Argument::Object(mutex)] = &op.arguments[..] else { panic!() }; + extract_args!(op => [Argument::Object(mutex)]); let Object::Mutex { mutex, sync_level: _ } = **mutex else { Err(AmlError::InvalidOperationOnObject { op: Operation::Acquire, typ: mutex.typ() })? }; @@ -901,7 +888,7 @@ where context.retire_op(op); } Opcode::Release => { - let [Argument::Object(mutex)] = &op.arguments[..] else { panic!() }; + extract_args!(op => [Argument::Object(mutex)]); let Object::Mutex { mutex, sync_level: _ } = **mutex else { Err(AmlError::InvalidOperationOnObject { op: Operation::Release, typ: mutex.typ() })? }; @@ -916,11 +903,7 @@ where context.retire_op(op); } Opcode::InternalMethodCall => { - let [Argument::Object(method), Argument::Namestring(method_scope)] = &op.arguments[0..2] - else { - panic!() - }; - + extract_args!(op[0..2] => [Argument::Object(method), Argument::Namestring(method_scope)]); let args = op.arguments[2..] .iter() .map(|arg| { @@ -950,7 +933,7 @@ where } } Opcode::Return => { - let [Argument::Object(object)] = &op.arguments[..] else { panic!() }; + extract_args!(op => [Argument::Object(object)]); let object = object.clone().unwrap_transparent_reference(); if let Some(last) = self.context_stack.lock().pop() { @@ -966,47 +949,51 @@ where } } Opcode::ObjectType => { - let [Argument::Object(object)] = &op.arguments[..] else { panic!() }; + extract_args!(op => [Argument::Object(object)]); + // TODO: this should technically support scopes as well - this is less easy // (they should return `0`) - let typ = match object.typ() { - ObjectType::Uninitialized => 0, - ObjectType::Integer => 1, - ObjectType::String => 2, - ObjectType::Buffer => 3, - ObjectType::Package => 4, - ObjectType::FieldUnit => 5, - ObjectType::Device => 6, - ObjectType::Event => 7, - ObjectType::Method => 8, - ObjectType::Mutex => 9, - ObjectType::OpRegion => 10, - ObjectType::PowerResource => 11, - ObjectType::Processor => 12, - ObjectType::ThermalZone => 13, - ObjectType::BufferField => 14, - // XXX: 15 is reserved - ObjectType::Debug => 16, - ObjectType::Reference => panic!(), - ObjectType::RawDataBuffer => 17, - }; + fn object_type(object: &Object) -> u64 { + if let Object::Reference { kind: _, inner } = object { + object_type(&inner) + } else { + match object.typ() { + ObjectType::Uninitialized => 0, + ObjectType::Integer => 1, + ObjectType::String => 2, + ObjectType::Buffer => 3, + ObjectType::Package => 4, + ObjectType::FieldUnit => 5, + ObjectType::Device => 6, + ObjectType::Event => 7, + ObjectType::Method => 8, + ObjectType::Mutex => 9, + ObjectType::OpRegion => 10, + ObjectType::PowerResource => 11, + ObjectType::Processor => 12, + ObjectType::ThermalZone => 13, + ObjectType::BufferField => 14, + // XXX: 15 is reserved + ObjectType::Debug => 16, + ObjectType::RawDataBuffer => 17, + ObjectType::Reference => unreachable!(), + } + } + } - context.contribute_arg(Argument::Object(Object::Integer(typ).wrap())); + context.contribute_arg(Argument::Object(Object::Integer(object_type(&object)).wrap())); context.retire_op(op); } Opcode::SizeOf => self.do_size_of(&mut context, op)?, Opcode::Index => self.do_index(&mut context, op)?, Opcode::BankField => { - let [ + extract_args!(op => [ Argument::TrackedPc(start_pc), Argument::PkgLength(pkg_length), Argument::Namestring(region_name), Argument::Namestring(bank_name), Argument::Object(bank_value), - ] = &op.arguments[..] - else { - panic!() - }; + ]); let bank_value = bank_value.as_integer()?; let field_flags = context.next()?; @@ -1026,8 +1013,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. */ - let [Argument::Object(predicate)] = &op.arguments[..] else { panic!() }; - let predicate = predicate.as_integer()?; + extract_args!(op => [Argument::Object(predicate)]); let predicate = predicate.clone().unwrap_transparent_reference().as_integer()?; if predicate == 0 { @@ -1342,9 +1328,7 @@ where Opcode::Revision => { context.contribute_arg(Argument::Object(Object::Integer(INTERPRETER_REVISION).wrap())); } - Opcode::Debug => { - context.contribute_arg(Argument::Object(Object::Debug.wrap())); - } + Opcode::Debug => context.contribute_arg(Argument::Object(Object::Debug.wrap())), Opcode::Fatal => { let typ = context.next()?; let code = context.next_u32()?; @@ -1861,10 +1845,7 @@ where } fn do_binary_maths(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - let [Argument::Object(left), Argument::Object(right), Argument::Object(target)] = &op.arguments[0..3] - else { - panic!() - }; + extract_args!(op[0..3] => [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()?; @@ -1899,7 +1880,7 @@ where } fn do_unary_maths(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - let [Argument::Object(operand)] = &op.arguments[..] else { panic!() }; + extract_args!(op => [Argument::Object(operand)]); let operand = operand.clone().unwrap_transparent_reference().as_integer()?; let result = match op.op { @@ -1942,7 +1923,7 @@ where fn do_logical_op(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { if op.op == Opcode::LNot { - let [Argument::Object(operand)] = &op.arguments[..] else { panic!() }; + extract_args!(op => [Argument::Object(operand)]); let operand = operand.clone().unwrap_transparent_reference().as_integer()?; let result = if operand == 0 { u64::MAX } else { 0 }; @@ -1951,7 +1932,7 @@ where return Ok(()); } - let [Argument::Object(left), Argument::Object(right)] = &op.arguments[..] else { panic!() }; + extract_args!(op => [Argument::Object(left), Argument::Object(right)]); let left = left.clone().unwrap_transparent_reference(); let right = right.clone().unwrap_transparent_reference(); @@ -2016,7 +1997,7 @@ where } fn do_to_buffer(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - let [Argument::Object(operand), Argument::Object(target)] = &op.arguments[..] else { panic!() }; + extract_args!(op => [Argument::Object(operand), Argument::Object(target)]); let operand = operand.clone().unwrap_transparent_reference(); let result = match *operand { @@ -2049,7 +2030,7 @@ where } fn do_to_integer(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - let [Argument::Object(operand), Argument::Object(target)] = &op.arguments[..] else { panic!() }; + extract_args!(op => [Argument::Object(operand), Argument::Object(target)]); let operand = operand.clone().unwrap_transparent_reference(); let result = match *operand { @@ -2095,10 +2076,7 @@ where } fn do_to_string(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - let [Argument::Object(source), Argument::Object(length), Argument::Object(target)] = &op.arguments[..] - else { - panic!() - }; + extract_args!(op => [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; @@ -2126,7 +2104,7 @@ where /// Perform a `ToDecimalString` or `ToHexString` operation fn do_to_dec_hex_string(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - let [Argument::Object(operand), Argument::Object(target)] = &op.arguments[..] else { panic!() }; + extract_args!(op => [Argument::Object(operand), Argument::Object(target)]); let operand = operand.clone().unwrap_transparent_reference(); let result = match *operand { @@ -2167,15 +2145,7 @@ where } fn do_mid(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - let [ - Argument::Object(source), - Argument::Object(index), - Argument::Object(length), - Argument::Object(target), - ] = &op.arguments[..] - else { - panic!() - }; + extract_args!(op => [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; @@ -2209,11 +2179,7 @@ where } fn do_concat(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - // TODO - let [Argument::Object(source1), Argument::Object(source2), Argument::Object(target)] = &op.arguments[..] - else { - panic!() - }; + extract_args!(op => [Argument::Object(source1), Argument::Object(source2), Argument::Object(target)]); let source1 = source1.clone().unwrap_transparent_reference(); let source2 = source2.clone().unwrap_transparent_reference(); @@ -2273,7 +2239,7 @@ where } fn do_from_bcd(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - let [Argument::Object(value)] = &op.arguments[..] else { panic!() }; + extract_args!(op => [Argument::Object(value)]); let mut value = value.clone().unwrap_transparent_reference().as_integer()?; let mut result = 0; @@ -2290,7 +2256,7 @@ where } fn do_to_bcd(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - let [Argument::Object(value)] = &op.arguments[..] else { panic!() }; + extract_args!(op => [Argument::Object(value)]); let mut value = value.clone().unwrap_transparent_reference().as_integer()?; let mut result = 0; @@ -2307,7 +2273,7 @@ where } fn do_size_of(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - let [Argument::Object(object)] = &op.arguments[..] else { panic!() }; + extract_args!(op => [Argument::Object(object)]); let object = object.clone().unwrap_transparent_reference(); let result = match *object { @@ -2323,12 +2289,7 @@ where } fn do_index(&self, context: &mut MethodContext, op: OpInFlight) -> Result<(), AmlError> { - // TODO - let [Argument::Object(object), Argument::Object(index_value), Argument::Object(target)] = - &op.arguments[..] - else { - panic!() - }; + extract_args!(op => [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()?; @@ -3515,4 +3476,8 @@ pub enum AmlError { /// 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), + + /// 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), } From bd5b51632717e3bd6338bd4f1406e3a0441aa57c Mon Sep 17 00:00:00 2001 From: Isaac Woods Date: Wed, 8 Apr 2026 16:49:26 +0100 Subject: [PATCH 6/8] Introduce `Interpreter::integer_size` helper --- src/aml/mod.rs | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/aml/mod.rs b/src/aml/mod.rs index bedde331..9342d830 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -401,6 +401,12 @@ 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. + pub fn integer_size(&self) -> usize { + if self.dsdt_revision >= 2 { 8 } else { 4 } + } + fn do_execute_method(&self, mut context: MethodContext) -> Result { /* * This is the main loop that executes operations. Every op is handled at the top-level of @@ -1892,10 +1898,10 @@ where * This is a particularly important place to respect the integer width as set * by the DSDT revision. */ - if self.dsdt_revision >= 2 { - (operand.leading_zeros() + 1) as u64 - } else { - ((operand as u32).leading_zeros() + 1) as u64 + match self.integer_size() { + 4 => ((operand as u32).leading_zeros() + 1) as u64, + 8 => (operand.leading_zeros() + 1) as u64, + _ => unreachable!(), } } } @@ -2003,7 +2009,7 @@ where let result = match *operand { Object::Buffer(ref bytes) => Object::Buffer(bytes.clone()), Object::Integer(value) => { - if self.dsdt_revision >= 2 { + if self.integer_size() == 8 { Object::Buffer(value.to_le_bytes().to_vec()) } else { Object::Buffer((value as u32).to_le_bytes().to_vec()) @@ -2209,9 +2215,9 @@ where let result = match source1.typ() { ObjectType::Integer => { let source1 = source1.as_integer()?; - let source2 = source2.to_integer(if self.dsdt_revision >= 2 { 8 } else { 4 })?; + let source2 = source2.to_integer(self.integer_size())?; let mut buffer = Vec::new(); - if self.dsdt_revision >= 2 { + if self.integer_size() == 8 { buffer.extend_from_slice(&source1.to_le_bytes()); buffer.extend_from_slice(&source2.to_le_bytes()); } else { @@ -2222,7 +2228,7 @@ where } ObjectType::Buffer => { let mut buffer = source1.as_buffer()?.to_vec(); - buffer.extend(source2.to_buffer(if self.dsdt_revision >= 2 { 8 } else { 4 })?); + buffer.extend(source2.to_buffer(self.integer_size())?); Object::Buffer(buffer).wrap() } _ => { @@ -2426,7 +2432,7 @@ where /// 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 { - let needs_buffer = if self.dsdt_revision >= 2 { field.bit_length > 64 } else { field.bit_length > 32 }; + let needs_buffer = field.bit_length > (self.integer_size() * 8); let access_width_bits = field.flags.access_type_bytes()? * 8; trace!("AML field read. Field = {:?}", field); From 47693c2275709b775e6a7348cbd2e98931857bc0 Mon Sep 17 00:00:00 2001 From: Isaac Woods Date: Wed, 8 Apr 2026 17:06:35 +0100 Subject: [PATCH 7/8] Implement `DefCopyObject` --- src/aml/mod.rs | 44 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/src/aml/mod.rs b/src/aml/mod.rs index 9342d830..a0aef19a 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -801,6 +801,11 @@ where )?; context.retire_op(op); } + Opcode::CopyObject => { + extract_args!(op => [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)]); self.do_store(target.clone(), object.clone())?; @@ -1526,6 +1531,10 @@ where Opcode::Store, &[ResolveBehaviour::TermArg, ResolveBehaviour::SuperName], )), + Opcode::CopyObject => context.start(OpInFlight::new( + Opcode::CopyObject, + &[ResolveBehaviour::TermArg, ResolveBehaviour::SimpleName], + )), Opcode::RefOf => context.start(OpInFlight::new(Opcode::RefOf, &[ResolveBehaviour::SuperName])), Opcode::CondRefOf => context.start(OpInFlight::new( opcode, @@ -1710,7 +1719,6 @@ where )), Opcode::ObjectType => context.start(OpInFlight::new(opcode, &[ResolveBehaviour::SuperName])), - Opcode::CopyObject => todo!(), Opcode::Mid => context.start(OpInFlight::new( Opcode::Mid, &[ @@ -2427,6 +2435,40 @@ where Ok(object) } + /// Copy `object` into `target`, matching the expected behaviour of `DefCopyObject`, which + /// depends on the object referenced: + /// - Locals are overwritten + /// - Args are overwritten, unless they are references, in which case the referenced object is overwritten + /// - 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> { + let Object::Reference { kind, ref inner } = *target else { + return Err(AmlError::InternalError("Target of CopyObject must be a reference".to_string())); + }; + let object = object.clone().unwrap_transparent_reference(); + let token = self.object_token.lock(); + + let dst = match kind { + ReferenceKind::Named | ReferenceKind::Local => inner.clone().unwrap_transparent_reference(), + ReferenceKind::Arg => { + if let Object::Reference { kind: _, inner: ref inner_inner } = **inner { + inner_inner.clone() + } else { + inner.clone().unwrap_transparent_reference() + } + } + ReferenceKind::Index => todo!(), + ReferenceKind::RefOf | ReferenceKind::Unresolved => return Err(AmlError::StoreToInvalidReferenceType), + }; + + unsafe { + *dst.gain_mut(&token) = (*object).clone(); + } + + Ok(()) + } + /// Do a read from a field by performing one or more well-formed accesses to the underlying /// 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 From ac30fefab78330f75566b95807df94a974456e1b Mon Sep 17 00:00:00 2001 From: Isaac Woods Date: Sat, 11 Apr 2026 10:53:07 +0100 Subject: [PATCH 8/8] Fix formatting issues with `aml_tester` namespace printing --- tools/aml_tester/src/main.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/aml_tester/src/main.rs b/tools/aml_tester/src/main.rs index b48ef4bc..6849fa73 100644 --- a/tools/aml_tester/src/main.rs +++ b/tools/aml_tester/src/main.rs @@ -26,9 +26,8 @@ use std::{ fs::{self}, io::Write, path::{Path, PathBuf}, - process::Command, + process::{Command, ExitCode}, }; -use std::process::ExitCode; /// The result of a test, with all other information (error codes etc.) stripped away. This value /// can then be stored in a Set - the test results with more info cannot. @@ -79,7 +78,6 @@ If the ASL contains a MAIN method, it will be executed.", cmd.print_help().unwrap(); return ExitCode::SUCCESS; } - log::set_max_level(log::LevelFilter::Info); let matches = cmd.get_matches(); @@ -191,13 +189,16 @@ If the ASL contains a MAIN method, it will be executed.", } }; + if let Some(ref interpreter) = interpreter_returned { + println!("Namespace: {}", interpreter.namespace.lock()); + } + interpreter = match interpreter_returned { _ if !combined_test => new_interpreter(new_handler()), None => new_interpreter(new_handler()), Some(i) => i, }; - println!("Namespace: {}", interpreter.namespace.lock()); summaries.insert((file_entry, simple_result)); }