diff --git a/src/aml/match_expr.rs b/src/aml/match_expr.rs new file mode 100644 index 00000000..e46954e0 --- /dev/null +++ b/src/aml/match_expr.rs @@ -0,0 +1,108 @@ +use crate::aml::object::Package; + +const ONES: u64 = !0; + +pub fn do_match( + search_package: &Package, + op_a: &MatchOp, + operand_a: u64, + op_b: &MatchOp, + operand_b: u64, + start_index: u64, +) -> u64 { + for (idx, row) in search_package[start_index as usize..].iter().enumerate() { + let Ok(v) = row.as_integer() else { + continue; + }; + + if match_comparison(v, op_a, operand_a) && match_comparison(v, op_b, operand_b) { + return start_index + idx as u64; + } + } + + ONES +} + +fn match_comparison(value: u64, op: &MatchOp, operand: u64) -> bool { + match op { + MatchOp::MTR => true, + MatchOp::MEQ => value == operand, + MatchOp::MLE => value <= operand, + MatchOp::MLT => value < operand, + MatchOp::MGE => value >= operand, + MatchOp::MGT => value > operand, + } +} + +#[allow(clippy::upper_case_acronyms)] // Allow the same capitalisation as the spec. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[repr(u16)] +pub enum MatchOp { + MTR = 0, + MEQ = 1, + MLE = 2, + MLT = 3, + MGE = 4, + MGT = 5, +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::aml::object::Object; + use std::string::ToString; + + #[test] + fn empty_package_doesnt_match() { + assert_eq!(ONES, do_match(&vec![], &MatchOp::MEQ, 0, &MatchOp::MTR, 0, 0)); + } + + #[test] + fn basic_match() { + let pkg = &([0, 1, 2, 3].iter().map(|i| Object::Integer(*i as u64).wrap()).collect()); + assert_eq!(2, do_match(pkg, &MatchOp::MEQ, 2, &MatchOp::MTR, 0, 0)); + } + + #[test] + fn skips_strings() { + let pkg = [ + Object::Integer(0), + Object::String("1".to_string()), + Object::String("2".to_string()), + Object::Integer(3), + ] + .into_iter() + .map(|o| o.wrap()) + .collect(); + + assert_eq!(3, do_match(&pkg, &MatchOp::MGT, 2, &MatchOp::MTR, 0, 0)); + } + + #[test] + fn skips_packages() { + let pkg = [Object::Integer(0), Object::Package(vec![Object::Integer(1).wrap()]), Object::Integer(2)] + .into_iter() + .map(|o| o.wrap()) + .collect(); + + assert_eq!(2, do_match(&pkg, &MatchOp::MGE, 2, &MatchOp::MTR, 0, 0)); + } + + #[test] + fn second_op_also_works() { + let pkg = &([0, 1, 2, 3].iter().map(|i| Object::Integer(*i as u64).wrap()).collect()); + assert_eq!(2, do_match(pkg, &MatchOp::MTR, 0, &MatchOp::MEQ, 2, 0)); + } + + #[test] + fn impossible_match_returns_ones() { + let pkg = &([0, 1, 2, 3].iter().map(|i| Object::Integer(*i as u64).wrap()).collect()); + assert_eq!(ONES, do_match(pkg, &MatchOp::MEQ, 2, &MatchOp::MLT, 1, 0)); + } + + #[test] + fn offset_works_correctly() { + let pkg = &([1, 1, 1, 1].iter().map(|i| Object::Integer(*i as u64).wrap()).collect()); + assert_eq!(2, do_match(pkg, &MatchOp::MEQ, 1, &MatchOp::MTR, 0, 2)); + } +} diff --git a/src/aml/mod.rs b/src/aml/mod.rs index e317f65d..e9c23206 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -9,7 +9,6 @@ * - Load and LoadTable * - DefDataRegion * - Notify - * - DefMatch * * - Method recursion depth? * - Loop timeouts @@ -17,6 +16,8 @@ */ mod interrupt_model_used; +mod match_expr; + pub mod namespace; pub mod object; pub mod op_region; @@ -51,6 +52,7 @@ use core::{ }; pub use interrupt_model_used::InterruptModelUsed; use log::{error, info, trace, warn}; +use match_expr::{MatchOp, do_match}; use namespace::{AmlName, Namespace, NamespaceLevelKind}; use object::{ DeviceStatus, @@ -1076,6 +1078,35 @@ where context.retire_op(op); } } + Opcode::Match => { + extract_args!(op => [ + Argument::Object(search_pkg), + Argument::MatchOp(opcode_a), + Argument::Object(operand_a), + Argument::MatchOp(opcode_b), + Argument::Object(operand_b), + Argument::Object(start_index)] + ); + + let Object::Package(ref pkg) = **search_pkg else { + return Err(AmlError::ObjectNotOfExpectedType { + expected: ObjectType::Package, + got: search_pkg.typ(), + }); + }; + + let result = do_match( + pkg, + opcode_a, + operand_a.as_integer()?, + opcode_b, + operand_b.as_integer()?, + start_index.as_integer()?, + ); + + context.contribute_arg(Argument::Object(Object::Integer(result).wrap())); + context.retire_op(op); + } _ => panic!("Unexpected operation has created in-flight op!"), } } @@ -1212,6 +1243,13 @@ where Opcode::One => { context.contribute_arg(Argument::Object(Object::Integer(1).wrap())); } + Opcode::MatchOp(opcode) => { + if context.expecting_match_opcode() { + context.contribute_arg(Argument::MatchOp(opcode)); + } else { + Err(AmlError::IllegalOpcode(opcode as u16))? + } + } Opcode::Ones => { context.contribute_arg(Argument::Object(Object::Integer(u64::MAX).wrap())); } @@ -1676,6 +1714,7 @@ where ResolveBehaviour::Placeholder => { panic!("Invalid resolve behaviour for name to be resolved!") } + ResolveBehaviour::MatchOpcode => Err(AmlError::InvalidArgType)?, } } @@ -1726,14 +1765,17 @@ where opcode, &[ResolveBehaviour::TermArg, ResolveBehaviour::TermArg, ResolveBehaviour::Target], )), - /* - * TODO - * Match is a difficult opcode to parse, as it interleaves dynamic arguments and - * random bytes that need to be extracted as you go. I think we'll need to use 1+ - * internal in-flight ops to parse the static bytedatas as we go, and then retire - * the real op at the end. - */ - Opcode::Match => todo!(), + Opcode::Match => context.start(OpInFlight::new( + opcode, + &[ + ResolveBehaviour::TermArg, + ResolveBehaviour::MatchOpcode, + ResolveBehaviour::TermArg, + ResolveBehaviour::MatchOpcode, + ResolveBehaviour::TermArg, + ResolveBehaviour::TermArg, + ], + )), Opcode::CreateBitField | Opcode::CreateByteField @@ -2955,6 +2997,8 @@ enum ResolveBehaviour { /// Used with [`OpInFlight::new_with`] to represent arguments that have already been resolved /// when an operation enters flight. Placeholder, + /// The match statement takes two single byte arguments - the "match opcode". + MatchOpcode, } #[derive(Debug)] @@ -2973,6 +3017,7 @@ enum Argument { DWordData(u32), TrackedPc(usize), PkgLength(usize), + MatchOp(MatchOp), } impl OpInFlight { @@ -3114,9 +3159,28 @@ impl MethodContext { other => other as u16, }; + // Opcodes Zero and One overlap with the Match Opcodes that are only used in the match + // expression. It's reasonable to assume that if Zero or One appear at the time we expect a + // Match Opcode, then they *are* Match Opcodes instead of regular arguments. Ok(match opcode { - 0x00 => Opcode::Zero, - 0x01 => Opcode::One, + 0x00 => { + if self.expecting_match_opcode() { + Opcode::MatchOp(MatchOp::MTR) + } else { + Opcode::Zero + } + } + 0x01 => { + if self.expecting_match_opcode() { + Opcode::MatchOp(MatchOp::MEQ) + } else { + Opcode::One + } + } + 0x02 => Opcode::MatchOp(MatchOp::MLE), + 0x03 => Opcode::MatchOp(MatchOp::MLT), + 0x04 => Opcode::MatchOp(MatchOp::MGE), + 0x05 => Opcode::MatchOp(MatchOp::MGT), 0x06 => Opcode::Alias, 0x08 => Opcode::Name, 0x0a => Opcode::BytePrefix, @@ -3362,12 +3426,18 @@ impl MethodContext { Ok(self.current_block.stream()[self.current_block.pc]) } + + fn expecting_match_opcode(&self) -> bool { + let resolve_behaviour = self.in_flight.last().map(|op| op.resolve_behaviour()); + resolve_behaviour.is_some_and(|rb| matches!(rb, ResolveBehaviour::MatchOpcode)) + } } #[derive(Clone, Copy, PartialEq, Debug)] enum Opcode { Zero, One, + MatchOp(MatchOp), Alias, Name, BytePrefix, @@ -3590,6 +3660,12 @@ pub enum AmlError { /// The maximum length of time a loop ([`LOOP_TIMEOUT_NS`]) can execute for has been exceeded. LoopTimeout, + + /// The provided argument to the current operation is invalid. + /// + /// For example, trying to pass an object as the Operation parameter in a Match expression is + /// invalid. + InvalidArgType, } #[derive(Debug, Clone, Copy, PartialEq)] diff --git a/src/aml/object.rs b/src/aml/object.rs index 7c788467..740043c2 100644 --- a/src/aml/object.rs +++ b/src/aml/object.rs @@ -11,6 +11,8 @@ use static_assertions::assert_impl_all; type NativeMethod = dyn Fn(&[WrappedObject]) -> Result + Send + Sync; +pub type Package = Vec; + #[derive(Clone)] pub enum Object { Uninitialized, @@ -26,7 +28,7 @@ pub enum Object { Reference { kind: ReferenceKind, inner: WrappedObject }, NamePath { name: AmlName, scope: AmlName }, OpRegion(OpRegion), - Package(Vec), + Package(Package), PowerResource { system_level: u8, resource_order: u16 }, Processor { proc_id: u8, pblk_address: u32, pblk_length: u8 }, RawDataBuffer, diff --git a/tests/match.asl b/tests/match.asl new file mode 100644 index 00000000..fb39bd91 --- /dev/null +++ b/tests/match.asl @@ -0,0 +1,34 @@ +DefinitionBlock("", "DSDT", 1, "RSACPI", "MATCH", 1) { + Name (AAAA, Package (4) { 0x01, 0x01, 0x02, 0x03 }) + Name (BBBB, Package (4) { 0x01, 0x01, 0x01, 0x01 }) + + Name(FCNT, 0) + + Method (CHEK, 2) { + If (Arg0 != Arg1) { + FCNT++ + } + } + + Method (MAIN) { + // Test all operations in first position. + CHEK(2, Match (AAAA, MEQ, 2, MTR, 0, 0)) + CHEK(Ones, Match (AAAA, MLT, 1, MTR, 0, 0)) + CHEK(0, Match (AAAA, MLE, 1, MTR, 0, 0)) + CHEK(3, Match (AAAA, MGT, 2, MTR, 0, 0)) + CHEK(2, Match (AAAA, MGE, 2, MTR, 0, 0)) + + // Test all operations in second position. + CHEK(2, Match (AAAA, MTR, 0, MEQ, 2, 0)) + CHEK(Ones, Match (AAAA, MTR, 0, MLT, 1, 0)) + CHEK(0, Match (AAAA, MTR, 0, MLE, 1, 0)) + CHEK(3, Match (AAAA, MTR, 0, MGT, 2, 0)) + CHEK(2, Match (AAAA, MTR, 0, MGE, 2, 0)) + + // Check offset works + CHEK(2, Match (BBBB, MEQ, 1, MTR, 0, 2)) + CHEK(Ones, Match (BBBB, MEQ, 2, MTR, 0, 0)) + + Return (FCNT) + } +} diff --git a/tests/match_expr.rs b/tests/match_expr.rs new file mode 100644 index 00000000..cdd87fc5 --- /dev/null +++ b/tests/match_expr.rs @@ -0,0 +1,60 @@ +//! Check that incorrect AML sequences in Match expressions appear to error correctly without panics + +mod test_infra; + +use crate::test_infra::run_opcodes_test_with_result; +use aml_test_tools::{RunTestResult, TestFailureReason, handlers::null_handler::NullHandler}; +use std::assert_matches; + +#[test] +fn invalid_arg_to_match() { + // This AML is plausible looking but invalid - it attempts to pass Local0 instead of a match + // operand. + // + // Generated by compiling the following script and changing the first Match Opcode: + // ``` + // DefinitionBlock("", "DSDT", 1, "RSACPI", "MATCH", 1) { + // Method (MAIN) { + // Name (AAAA, Package (4) { 0xAA, 0xAA}) + // Local1 = Match (AAAA, MTR, 0, MGE, 2, 0) + // Return (0) + // } + // } + // ``` + let opcodes = vec![ + 0x08, // Name op + 0x41, 0x41, 0x41, 0x41, // "AAAA" + 0x12, // Package + 0x06, // 6 bytes long + 0x02, // 4 elements? + 0x0A, 0xAA, // Byte 0xAA + 0x0A, 0xAA, // Byte 0xAA + 0x70, // Store + 0x89, // Match + 0x41, 0x41, 0x41, 0x41, // "AAAA" + 0x68, // ** Invalid match opcode Arg0 ** + 0x0A, 0x00, // Byte 0 + 0x04, // Match Opcode + 0x0A, 0x02, // Byte 2 (operand) + 0x0A, 0x00, // Byte 0 (start offset) + 0x61, // Local1 - store target + 0xA4, 0x0A, 0x00, // Return byte 0 + ]; + + let r = run_opcodes_test_with_result(&opcodes, NullHandler); + assert_matches!(r, RunTestResult::Failed(_, TestFailureReason::ParseFail(_))) +} + +#[test] +fn invalid_use_of_match_opcode() { + let opcodes = vec![ + // Attempt something like `Local1 = MGT` + 0x70, 0x05, 0x61, + // `Return (0)` (If the line above works, then 0 is returned and the test result is `Pass`, + // rather than the `Failed` that we expect) + 0xA4, 0x0A, 0x00, + ]; + + let r = run_opcodes_test_with_result(&opcodes, NullHandler); + assert_matches!(r, RunTestResult::Failed(_, TestFailureReason::ParseFail(_))) +} diff --git a/tests/test_infra/mod.rs b/tests/test_infra/mod.rs index aa94b987..6b69e0f1 100644 --- a/tests/test_infra/mod.rs +++ b/tests/test_infra/mod.rs @@ -56,11 +56,21 @@ pub fn evaluate(interpreter: &Interpreter, path: &str) -> Object { /// opcodes to execute. #[allow(dead_code)] pub fn run_opcodes_test(opcodes: &[u8], handler: impl Handler) { + let result = run_opcodes_test_with_result(opcodes, handler); + assert!(matches!(result, RunTestResult::Pass(_)), "Test failed with: {:?}", TestResult::from(&result)); +} + +/// Run a test against a sequence of AML opcodes and return the raw result. +/// +/// The provided opcodes are wrapped in a `MAIN` method and also have a table header prepended. The +/// `MAIN` function is executed as part of the test, so `opcodes` only needs to include the actual +/// opcodes to execute. +#[allow(dead_code)] +pub fn run_opcodes_test_with_result(opcodes: &[u8], handler: H) -> RunTestResult> { let _ = pretty_env_logger::try_init(); let logged_handler = LoggingHandler::new(handler); let interpreter = new_interpreter(logged_handler); - let result = run_test_for_opcodes(opcodes, interpreter, &None); - assert!(matches!(result, RunTestResult::Pass(_)), "Test failed with: {:?}", TestResult::from(&result)); + run_test_for_opcodes(opcodes, interpreter, &None) }