Merge pull request #88 from IsaacWoods/propagate

Reorganise to allow native methods, then implement them

aml/src/lib.rs

@@ -67,7 +67,7 @@ use log::error;
 use misc::{ArgNum, LocalNum};
 use name_object::Target;
 use namespace::LevelType;
-use parser::Parser;
+use parser::{Parser, Propagate};
 use pkg_length::PkgLength;
 use term_object::term_list;
 use value::{AmlType, Args};
@@ -167,15 +167,21 @@ impl AmlContext {
         let table_length = PkgLength::from_raw_length(stream, stream.len() as u32).unwrap();
         match term_object::term_list(table_length).parse(stream, self) {
             Ok(_) => Ok(()),
-            Err((_, _, err)) => {
+            Err((_, _, Propagate::Err(err))) => {
                 error!("Failed to parse AML stream. Err = {:?}", err);
                 Err(err)
             }
+            Err((_, _, other)) => {
+                error!("AML table evaluated to unexpected result: {:?}", other);
+                Err(AmlError::MalformedStream)
+            }
         }
     }
 
     // TODO: docs
     pub fn invoke_method(&mut self, path: &AmlName, args: Args) -> Result<AmlValue, AmlError> {
+        use value::MethodCode;
+
         match self.namespace.get_by_path(path)?.clone() {
             AmlValue::Method { flags, code } => {
                 /*
@@ -191,16 +197,28 @@ impl AmlContext {
                  */
                 self.namespace.add_level(path.clone(), LevelType::MethodLocals)?;
 
-                let return_value = match term_list(PkgLength::from_raw_length(&code, code.len() as u32).unwrap())
-                    .parse(&code, self)
-                {
-                    // If the method doesn't return a value, we implicitly return `0`
-                    Ok(_) => Ok(AmlValue::Integer(0)),
-                    Err((_, _, AmlError::Return(result))) => Ok(result),
-                    Err((_, _, err)) => {
-                        error!("Failed to execute control method: {:?}", err);
-                        Err(err)
+                let return_value = match code {
+                    MethodCode::Aml(ref code) => {
+                        match term_list(PkgLength::from_raw_length(code, code.len() as u32).unwrap())
+                            .parse(code, self)
+                        {
+                            // If the method doesn't return a value, we implicitly return `0`
+                            Ok(_) => Ok(AmlValue::Integer(0)),
+                            Err((_, _, Propagate::Return(result))) => Ok(result),
+                            Err((_, _, Propagate::Err(err))) => {
+                                error!("Failed to execute control method: {:?}", err);
+                                Err(err)
+                            }
+                        }
                     }
+
+                    MethodCode::Native(ref method) => match (method)(self) {
+                        Ok(result) => Ok(result),
+                        Err(err) => {
+                            error!("Failed to execute control method: {:?}", err);
+                            Err(err)
+                        }
+                    },
                 };
 
                 /*
@@ -560,6 +578,16 @@ impl AmlContext {
     }
 
     fn add_predefined_objects(&mut self) {
+        /*
+         * These are the scopes predefined by the spec. Some tables will try to access them without defining them
+         * themselves, and so we have to pre-create them.
+         */
+        self.namespace.add_level(AmlName::from_str("\\_GPE").unwrap(), LevelType::Scope).unwrap();
+        self.namespace.add_level(AmlName::from_str("\\_SB").unwrap(), LevelType::Scope).unwrap();
+        self.namespace.add_level(AmlName::from_str("\\_SI").unwrap(), LevelType::Scope).unwrap();
+        self.namespace.add_level(AmlName::from_str("\\_PR").unwrap(), LevelType::Scope).unwrap();
+        self.namespace.add_level(AmlName::from_str("\\_TZ").unwrap(), LevelType::Scope).unwrap();
+
         /*
          * In the dark ages of ACPI 1.0, before `\_OSI`, `\_OS` was used to communicate to the firmware which OS
          * was running. This was predictably not very good, and so was replaced in ACPI 3.0 with `_OSI`, which
@@ -625,13 +653,15 @@ pub trait Handler {
     fn write_pci_u32(&self, segment: u16, bus: u8, device: u8, function: u8, offset: u16, value: u32);
 }
 
-#[derive(Clone, Debug, PartialEq, Eq)]
+#[derive(Clone, PartialEq, Eq, Debug)]
 pub enum AmlError {
     /*
      * Errors produced parsing the AML stream.
      */
     UnexpectedEndOfStream,
     UnexpectedByte(u8),
+    /// Produced when the stream evaluates to something other than nothing or an error.
+    MalformedStream,
     InvalidNameSeg,
     InvalidPkgLength,
     InvalidFieldFlags,
@@ -680,11 +710,6 @@ pub enum AmlError {
     InvalidArgAccess(ArgNum),
     /// Produced when a method accesses a local that it has not stored into.
     InvalidLocalAccess(LocalNum),
-    /// This is not a real error, but is used to propagate return values from within the deep
-    /// parsing call-stack. It should only be emitted when parsing a `DefReturn`. We use the
-    /// error system here because the way errors are propagated matches how we want to handle
-    /// return values.
-    Return(AmlValue),
 
     /*
      * Errors produced parsing the PCI routing tables (_PRT objects).

aml/src/name_object.rs

@@ -2,7 +2,7 @@ use crate::{
     misc::{arg_obj, debug_obj, local_obj, ArgNum, LocalNum},
     namespace::{AmlName, NameComponent},
     opcode::{opcode, DUAL_NAME_PREFIX, MULTI_NAME_PREFIX, NULL_NAME, PREFIX_CHAR, ROOT_CHAR},
-    parser::{choice, comment_scope, consume, n_of, take, take_while, Parser},
+    parser::{choice, comment_scope, consume, n_of, take, take_while, Parser, Propagate},
     AmlContext,
     AmlError,
     DebugVerbosity,
@@ -93,7 +93,7 @@ where
     comment_scope(DebugVerbosity::AllScopes, "NameString", move |input: &'a [u8], context| {
         let first_char = match input.first() {
             Some(&c) => c,
-            None => return Err((input, context, AmlError::UnexpectedEndOfStream)),
+            None => return Err((input, context, Propagate::Err(AmlError::UnexpectedEndOfStream))),
         };
 
         match first_char {
@@ -102,7 +102,7 @@ where
             _ => name_path()
                 .map(|path| {
                     if path.len() == 0 {
-                        return Err(AmlError::EmptyNamesAreInvalid);
+                        return Err(Propagate::Err(AmlError::EmptyNamesAreInvalid));
                     }
 
                     Ok(AmlName::from_components(path))

aml/src/namespace.rs

@@ -543,6 +543,7 @@ impl NameComponent {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::test_utils::crudely_cmp_values;
 
     #[test]
     fn test_aml_name_from_str() {
@@ -677,16 +678,22 @@ mod tests {
         /*
          * Get objects using their absolute paths.
          */
-        assert_eq!(namespace.get_by_path(&AmlName::from_str("\\MOO").unwrap()), Ok(&AmlValue::Boolean(true)));
-        assert_eq!(
-            namespace.get_by_path(&AmlName::from_str("\\FOO.BAR.A").unwrap()),
-            Ok(&AmlValue::Integer(12345))
-        );
-        assert_eq!(namespace.get_by_path(&AmlName::from_str("\\FOO.BAR.B").unwrap()), Ok(&AmlValue::Integer(6)));
-        assert_eq!(
-            namespace.get_by_path(&AmlName::from_str("\\FOO.BAR.C").unwrap()),
-            Ok(&AmlValue::String(String::from("hello, world!")))
-        );
+        assert!(crudely_cmp_values(
+            namespace.get_by_path(&AmlName::from_str("\\MOO").unwrap()).unwrap(),
+            &AmlValue::Boolean(true)
+        ));
+        assert!(crudely_cmp_values(
+            namespace.get_by_path(&AmlName::from_str("\\FOO.BAR.A").unwrap()).unwrap(),
+            &AmlValue::Integer(12345)
+        ));
+        assert!(crudely_cmp_values(
+            namespace.get_by_path(&AmlName::from_str("\\FOO.BAR.B").unwrap()).unwrap(),
+            &AmlValue::Integer(6)
+        ));
+        assert!(crudely_cmp_values(
+            namespace.get_by_path(&AmlName::from_str("\\FOO.BAR.C").unwrap()).unwrap(),
+            &AmlValue::String(String::from("hello, world!"))
+        ));
 
         /*
          * Search for some objects that should use search rules.

aml/src/opcode.rs

@@ -82,9 +82,9 @@ where
     'c: 'a,
 {
     move |input: &'a [u8], context: &'c mut AmlContext| match input.first() {
-        None => Err((input, context, AmlError::UnexpectedEndOfStream)),
+        None => Err((input, context, Propagate::Err(AmlError::UnexpectedEndOfStream))),
         Some(&byte) if byte == opcode => Ok((&input[1..], context, ())),
-        Some(_) => Err((input, context, AmlError::WrongParser)),
+        Some(_) => Err((input, context, Propagate::Err(AmlError::WrongParser))),
     }
 }
 

aml/src/parser.rs

@@ -1,4 +1,4 @@
-use crate::{pkg_length::PkgLength, AmlContext, AmlError, DebugVerbosity};
+use crate::{pkg_length::PkgLength, AmlContext, AmlError, AmlValue, DebugVerbosity};
 use alloc::vec::Vec;
 use core::{convert::TryInto, marker::PhantomData};
 use log::trace;
@@ -17,8 +17,20 @@ impl AmlContext {
     }
 }
 
+#[derive(Debug)]
+pub enum Propagate {
+    Err(AmlError),
+    Return(AmlValue),
+}
+
+impl From<AmlError> for Propagate {
+    fn from(error: AmlError) -> Self {
+        Self::Err(error)
+    }
+}
+
 pub type ParseResult<'a, 'c, R> =
-    Result<(&'a [u8], &'c mut AmlContext, R), (&'a [u8], &'c mut AmlContext, AmlError)>;
+    Result<(&'a [u8], &'c mut AmlContext, R), (&'a [u8], &'c mut AmlContext, Propagate)>;
 
 pub trait Parser<'a, 'c, R>: Sized
 where
@@ -28,14 +40,14 @@ where
 
     fn map<F, A>(self, map_fn: F) -> Map<'a, 'c, Self, F, R, A>
     where
-        F: Fn(R) -> Result<A, AmlError>,
+        F: Fn(R) -> Result<A, Propagate>,
     {
         Map { parser: self, map_fn, _phantom: PhantomData }
     }
 
     fn map_with_context<F, A>(self, map_fn: F) -> MapWithContext<'a, 'c, Self, F, R, A>
     where
-        F: Fn(R, &'c mut AmlContext) -> (Result<A, AmlError>, &'c mut AmlContext),
+        F: Fn(R, &'c mut AmlContext) -> (Result<A, Propagate>, &'c mut AmlContext),
     {
         MapWithContext { parser: self, map_fn, _phantom: PhantomData }
     }
@@ -101,7 +113,7 @@ where
 {
     move |input: &'a [u8], context: &'c mut AmlContext| match input.first() {
         Some(&byte) => Ok((&input[1..], context, byte)),
-        None => Err((input, context, AmlError::UnexpectedEndOfStream)),
+        None => Err((input, context, Propagate::Err(AmlError::UnexpectedEndOfStream))),
     }
 }
 
@@ -111,7 +123,7 @@ where
 {
     move |input: &'a [u8], context: &'c mut AmlContext| {
         if input.len() < 2 {
-            return Err((input, context, AmlError::UnexpectedEndOfStream));
+            return Err((input, context, Propagate::Err(AmlError::UnexpectedEndOfStream)));
         }
 
         Ok((&input[2..], context, u16::from_le_bytes(input[0..2].try_into().unwrap())))
@@ -124,7 +136,7 @@ where
 {
     move |input: &'a [u8], context: &'c mut AmlContext| {
         if input.len() < 4 {
-            return Err((input, context, AmlError::UnexpectedEndOfStream));
+            return Err((input, context, Propagate::Err(AmlError::UnexpectedEndOfStream)));
         }
 
         Ok((&input[4..], context, u32::from_le_bytes(input[0..4].try_into().unwrap())))
@@ -137,7 +149,7 @@ where
 {
     move |input: &'a [u8], context: &'c mut AmlContext| {
         if input.len() < 8 {
-            return Err((input, context, AmlError::UnexpectedEndOfStream));
+            return Err((input, context, Propagate::Err(AmlError::UnexpectedEndOfStream)));
         }
 
         Ok((&input[8..], context, u64::from_le_bytes(input[0..8].try_into().unwrap())))
@@ -150,7 +162,7 @@ where
 {
     move |input: &'a [u8], context| {
         if (input.len() as u32) < n {
-            return Err((input, context, AmlError::UnexpectedEndOfStream));
+            return Err((input, context, Propagate::Err(AmlError::UnexpectedEndOfStream)));
         }
 
         let (result, new_input) = input.split_at(n as usize);
@@ -169,7 +181,7 @@ where
          */
         let bytes_to_take = match (input.len() as u32).checked_sub(length.end_offset) {
             Some(bytes_to_take) => bytes_to_take,
-            None => return Err((input, context, AmlError::InvalidPkgLength)),
+            None => return Err((input, context, Propagate::Err(AmlError::InvalidPkgLength))),
         };
         take_n(bytes_to_take).parse(input, context)
     }
@@ -187,7 +199,7 @@ where
         for _ in 0..n {
             let (new_input, new_context, result) = match parser.parse(input, context) {
                 Ok((input, context, result)) => (input, context, result),
-                Err((_, context, err)) => return Err((input, context, err)),
+                Err((_, context, propagate)) => return Err((input, context, propagate)),
             };
             results.push(result);
             input = new_input;
@@ -212,7 +224,9 @@ where
                     context = new_context;
                     num_passed += 1;
                 }
-                Err((_, context, AmlError::WrongParser)) => return Ok((input, context, num_passed)),
+                Err((_, context, Propagate::Err(AmlError::WrongParser))) => {
+                    return Ok((input, context, num_passed))
+                }
                 Err((_, context, err)) => return Err((input, context, err)),
             }
         }
@@ -226,8 +240,8 @@ where
 {
     move |input: &'a [u8], context: &'c mut AmlContext| match input.first() {
         Some(&byte) if condition(byte) => Ok((&input[1..], context, byte)),
-        Some(&byte) => Err((input, context, AmlError::UnexpectedByte(byte))),
-        None => Err((input, context, AmlError::UnexpectedEndOfStream)),
+        Some(&byte) => Err((input, context, Propagate::Err(AmlError::UnexpectedByte(byte)))),
+        None => Err((input, context, Propagate::Err(AmlError::UnexpectedEndOfStream))),
     }
 }
 
@@ -279,7 +293,7 @@ where
     fn parse(&self, input: &'a [u8], context: &'c mut AmlContext) -> ParseResult<'a, 'c, R> {
         match self.p1.parse(input, context) {
             Ok(parse_result) => Ok(parse_result),
-            Err((_, context, AmlError::WrongParser)) => self.p2.parse(input, context),
+            Err((_, context, Propagate::Err(AmlError::WrongParser))) => self.p2.parse(input, context),
             Err((_, context, err)) => Err((input, context, err)),
         }
     }
@@ -289,7 +303,7 @@ pub struct Map<'a, 'c, P, F, R, A>
 where
     'c: 'a,
     P: Parser<'a, 'c, R>,
-    F: Fn(R) -> Result<A, AmlError>,
+    F: Fn(R) -> Result<A, Propagate>,
 {
     parser: P,
     map_fn: F,
@@ -300,7 +314,7 @@ impl<'a, 'c, P, F, R, A> Parser<'a, 'c, A> for Map<'a, 'c, P, F, R, A>
 where
     'c: 'a,
     P: Parser<'a, 'c, R>,
-    F: Fn(R) -> Result<A, AmlError>,
+    F: Fn(R) -> Result<A, Propagate>,
 {
     fn parse(&self, input: &'a [u8], context: &'c mut AmlContext) -> ParseResult<'a, 'c, A> {
         match self.parser.parse(input, context) {
@@ -317,7 +331,7 @@ pub struct MapWithContext<'a, 'c, P, F, R, A>
 where
     'c: 'a,
     P: Parser<'a, 'c, R>,
-    F: Fn(R, &'c mut AmlContext) -> (Result<A, AmlError>, &'c mut AmlContext),
+    F: Fn(R, &'c mut AmlContext) -> (Result<A, Propagate>, &'c mut AmlContext),
 {
     parser: P,
     map_fn: F,
@@ -328,7 +342,7 @@ impl<'a, 'c, P, F, R, A> Parser<'a, 'c, A> for MapWithContext<'a, 'c, P, F, R, A
 where
     'c: 'a,
     P: Parser<'a, 'c, R>,
-    F: Fn(R, &'c mut AmlContext) -> (Result<A, AmlError>, &'c mut AmlContext),
+    F: Fn(R, &'c mut AmlContext) -> (Result<A, Propagate>, &'c mut AmlContext),
 {
     fn parse(&self, input: &'a [u8], context: &'c mut AmlContext) -> ParseResult<'a, 'c, A> {
         match self.parser.parse(input, context) {
@@ -442,11 +456,11 @@ pub(crate) macro choice {
             $(
                 match ($parser).parse(input, context) {
                     Ok(parse_result) => return Ok(parse_result),
-                    Err((_, new_context, AmlError::WrongParser)) => { context = new_context; },
-                    Err((_, context, err)) => return Err((input, context, err)),
+                    Err((_, new_context, Propagate::Err(AmlError::WrongParser))) => { context = new_context; },
+                    Err((_, context, propagate)) => return Err((input, context, propagate)),
                 }
              )+
-            Err((input, context, AmlError::WrongParser))
+            Err((input, context, Propagate::Err(AmlError::WrongParser)))
         }
     }
 }
@@ -473,7 +487,7 @@ pub(crate) macro make_parser_concrete($parser: expr) {
 pub(crate) macro try_with_context($context: expr, $expr: expr) {
     match $expr {
         Ok(result) => result,
-        Err(err) => return (Err(err), $context),
+        Err(err) => return (Err(Propagate::Err(err)), $context),
     }
 }
 

aml/src/pkg_length.rs

@@ -1,5 +1,5 @@
 use crate::{
-    parser::{take, take_n, Parser},
+    parser::{take, take_n, Parser, Propagate},
     AmlContext,
     AmlError,
 };
@@ -42,7 +42,7 @@ where
          */
         match PkgLength::from_raw_length(input, raw_length) {
             Ok(pkg_length) => Ok((new_input, context, pkg_length)),
-            Err(err) => Err((input, context, err)),
+            Err(err) => Err((input, context, Propagate::Err(err))),
         }
     }
 }
@@ -70,26 +70,27 @@ where
             return Ok((new_input, context, length));
         }
 
-        let (new_input, context, length): (&[u8], &mut AmlContext, u32) =
-            match take_n(byte_count as u32).parse(new_input, context) {
-                Ok((new_input, context, bytes)) => {
-                    let initial_length = u32::from(lead_byte.get_bits(0..4));
-                    (
-                        new_input,
-                        context,
-                        bytes
-                            .iter()
-                            .enumerate()
-                            .fold(initial_length, |length, (i, &byte)| length + (u32::from(byte) << (4 + i * 8))),
-                    )
-                }
-
-                /*
-                 * The stream was too short. We return an error, making sure to return the
-                 * *original* stream (that we haven't consumed any of).
-                 */
-                Err((_, context, _)) => return Err((input, context, AmlError::UnexpectedEndOfStream)),
-            };
+        let (new_input, context, length): (&[u8], &mut AmlContext, u32) = match take_n(byte_count as u32)
+            .parse(new_input, context)
+        {
+            Ok((new_input, context, bytes)) => {
+                let initial_length = u32::from(lead_byte.get_bits(0..4));
+                (
+                    new_input,
+                    context,
+                    bytes
+                        .iter()
+                        .enumerate()
+                        .fold(initial_length, |length, (i, &byte)| length + (u32::from(byte) << (4 + i * 8))),
+                )
+            }
+
+            /*
+             * The stream was too short. We return an error, making sure to return the
+             * *original* stream (that we haven't consumed any of).
+             */
+            Err((_, context, _)) => return Err((input, context, Propagate::Err(AmlError::UnexpectedEndOfStream))),
+        };
 
         Ok((new_input, context, length))
     }

aml/src/term_object.rs

@@ -15,11 +15,12 @@ use crate::{
         try_with_context,
         ParseResult,
         Parser,
+        Propagate,
     },
     pkg_length::{pkg_length, PkgLength},
     type1::type1_opcode,
     type2::{def_buffer, def_package, type2_opcode},
-    value::{AmlValue, FieldFlags, MethodFlags, RegionSpace},
+    value::{AmlValue, FieldFlags, MethodCode, MethodFlags, RegionSpace},
     AmlContext,
     AmlError,
     AmlHandle,
@@ -114,7 +115,7 @@ where
             name_string().then(data_ref_object()).map_with_context(|(name, data_ref_object), context| {
                 try_with_context!(
                     context,
-                    context.namespace.add_value_at_resolved_path(name, &context.current_scope, data_ref_object,)
+                    context.namespace.add_value_at_resolved_path(name, &context.current_scope, data_ref_object)
                 );
                 (Ok(()), context)
             }),
@@ -201,15 +202,15 @@ where
                         0x08 => RegionSpace::GeneralPurposeIo,
                         0x09 => RegionSpace::GenericSerialBus,
                         space @ 0x80..=0xff => RegionSpace::OemDefined(space),
-                        byte => return (Err(AmlError::InvalidRegionSpace(byte)), context),
+                        byte => return (Err(Propagate::Err(AmlError::InvalidRegionSpace(byte))), context),
                     };
                     let offset = match offset.as_integer(context) {
                         Ok(offset) => offset,
-                        Err(err) => return (Err(err), context),
+                        Err(err) => return (Err(Propagate::Err(err)), context),
                     };
                     let length = match length.as_integer(context) {
                         Ok(length) => length,
-                        Err(err) => return (Err(err), context),
+                        Err(err) => return (Err(Propagate::Err(err)), context),
                     };
                     let parent_device = match region {
                         RegionSpace::PciConfig | RegionSpace::IPMI | RegionSpace::GenericSerialBus => {
@@ -367,7 +368,10 @@ where
                         context.namespace.add_value_at_resolved_path(
                             name,
                             &context.current_scope,
-                            AmlValue::Method { flags: MethodFlags::new(flags), code: code.to_vec() },
+                            AmlValue::Method {
+                                flags: MethodFlags::new(flags),
+                                code: MethodCode::Aml(code.to_vec())
+                            },
                         )
                     );
                     (Ok(()), context)
@@ -560,12 +564,12 @@ where
              */
             let nul_position = match input.iter().position(|&c| c == b'\0') {
                 Some(position) => position,
-                None => return Err((input, context, AmlError::UnterminatedStringConstant)),
+                None => return Err((input, context, Propagate::Err(AmlError::UnterminatedStringConstant))),
             };
 
             let string = String::from(match str::from_utf8(&input[0..nul_position]) {
                 Ok(string) => string,
-                Err(_) => return Err((input, context, AmlError::InvalidStringConstant)),
+                Err(_) => return Err((input, context, Propagate::Err(AmlError::InvalidStringConstant))),
             });
 
             Ok((&input[(nul_position + 1)..], context, AmlValue::String(string)))
@@ -588,7 +592,7 @@ where
             opcode::ONE_OP => Ok((new_input, context, AmlValue::Integer(1))),
             opcode::ONES_OP => Ok((new_input, context, AmlValue::Integer(u64::max_value()))),
 
-            _ => Err((input, context, AmlError::WrongParser)),
+            _ => Err((input, context, Propagate::Err(AmlError::WrongParser))),
         }
     };
 
@@ -611,40 +615,48 @@ mod test {
     #[test]
     fn test_computational_data() {
         let mut context = make_test_context();
-        check_ok!(
+        check_ok_value!(
             computational_data().parse(&[0x00, 0x34, 0x12], &mut context),
             AmlValue::Integer(0),
             &[0x34, 0x12]
         );
-        check_ok!(
+        check_ok_value!(
             computational_data().parse(&[0x01, 0x18, 0xf3], &mut context),
             AmlValue::Integer(1),
             &[0x18, 0xf3]
         );
-        check_ok!(
+        check_ok_value!(
             computational_data().parse(&[0xff, 0x98, 0xc3], &mut context),
             AmlValue::Integer(u64::max_value()),
             &[0x98, 0xc3]
         );
-        check_ok!(
+        check_ok_value!(
             computational_data().parse(&[0x5b, 0x30], &mut context),
             AmlValue::Integer(crate::AML_INTERPRETER_REVISION),
             &[]
         );
-        check_ok!(computational_data().parse(&[0x0a, 0xf3, 0x35], &mut context), AmlValue::Integer(0xf3), &[0x35]);
-        check_ok!(computational_data().parse(&[0x0b, 0xf3, 0x35], &mut context), AmlValue::Integer(0x35f3), &[]);
-        check_ok!(
+        check_ok_value!(
+            computational_data().parse(&[0x0a, 0xf3, 0x35], &mut context),
+            AmlValue::Integer(0xf3),
+            &[0x35]
+        );
+        check_ok_value!(
+            computational_data().parse(&[0x0b, 0xf3, 0x35], &mut context),
+            AmlValue::Integer(0x35f3),
+            &[]
+        );
+        check_ok_value!(
             computational_data().parse(&[0x0c, 0xf3, 0x35, 0x12, 0x65, 0xff, 0x00], &mut context),
             AmlValue::Integer(0x651235f3),
             &[0xff, 0x00]
         );
-        check_ok!(
+        check_ok_value!(
             computational_data()
                 .parse(&[0x0e, 0xf3, 0x35, 0x12, 0x65, 0xff, 0x00, 0x67, 0xde, 0x28], &mut context),
             AmlValue::Integer(0xde6700ff651235f3),
             &[0x28]
         );
-        check_ok!(
+        check_ok_value!(
             computational_data().parse(&[0x0d, b'A', b'B', b'C', b'D', b'\0', 0xff, 0xf5], &mut context),
             AmlValue::String(String::from("ABCD")),
             &[0xff, 0xf5]

aml/src/test_utils.rs

@@ -1,4 +1,4 @@
-use crate::{AmlContext, Handler};
+use crate::{parser::Propagate, AmlContext, AmlValue, Handler};
 use alloc::boxed::Box;
 
 struct TestHandler;
@@ -71,14 +71,16 @@ impl Handler for TestHandler {
 }
 
 pub(crate) fn make_test_context() -> AmlContext {
-    AmlContext::new(Box::new(TestHandler), false, crate::DebugVerbosity::None)
+    AmlContext::new(Box::new(TestHandler), crate::DebugVerbosity::None)
 }
 
 pub(crate) macro check_err($parse: expr, $error: pat, $remains: expr) {
     match $parse {
         Ok((remains, _, result)) => panic!("Expected Err, got {:#?}. Remaining = {:#x?}", result, remains),
-        Err((remains, _, $error)) if *remains == *$remains => (),
-        Err((remains, _, $error)) => panic!("Correct error, incorrect stream returned: {:#x?}", remains),
+        Err((remains, _, Propagate::Err($error))) if *remains == *$remains => (),
+        Err((remains, _, Propagate::Err($error))) => {
+            panic!("Correct error, incorrect stream returned: {:#x?}", remains)
+        }
         Err((_, _, err)) => panic!("Got wrong error: {:?}", err),
     }
 }
@@ -93,3 +95,99 @@ pub(crate) macro check_ok($parse: expr, $expected: expr, $remains: expr) {
         Err((_, _, err)) => panic!("Expected Ok, got {:#?}", err),
     }
 }
+
+pub(crate) macro check_ok_value($parse: expr, $expected: expr, $remains: expr) {
+    match $parse {
+        Ok((remains, _, ref result)) if remains == *$remains && crudely_cmp_values(result, &$expected) => (),
+        Ok((remains, _, ref result)) if crudely_cmp_values(result, &$expected) => {
+            panic!("Correct result, incorrect slice returned: {:x?}", remains)
+        }
+        Ok((_, _, ref result)) => panic!("Successfully parsed Ok, but it was wrong: {:#?}", result),
+        Err((_, _, err)) => panic!("Expected Ok, got {:#?}", err),
+    }
+}
+
+/// This is a bad (but good for testing) way of comparing `AmlValue`s, which tests that they're exactly the same if
+/// it can, and gives up if it can't. It's useful in tests to be able to see if you're getting the `AmlValue` that
+/// you're expecting.
+///
+/// NOTE: almost all of the `AmlValue` variants are `Eq`, and so for a long time, `AmlValue` implemented `Eq`.
+/// However, this is a footgun as, in the real interpreter, you rarely want to directly compare values, as you need
+/// to apply the AML value conversion rules to compare them correctly. This is therefore only useful for artificial
+/// testing scenarios.
+pub(crate) fn crudely_cmp_values(a: &AmlValue, b: &AmlValue) -> bool {
+    use crate::value::MethodCode;
+
+    match a {
+        AmlValue::Boolean(a) => match b {
+            AmlValue::Boolean(b) => a == b,
+            _ => false,
+        },
+        AmlValue::Integer(a) => match b {
+            AmlValue::Integer(b) => a == b,
+            _ => false,
+        },
+        AmlValue::String(ref a) => match b {
+            AmlValue::String(ref b) => a == b,
+            _ => false,
+        },
+        AmlValue::OpRegion { region, offset, length, parent_device } => match b {
+            AmlValue::OpRegion {
+                region: b_region,
+                offset: b_offset,
+                length: b_length,
+                parent_device: b_parent_device,
+            } => {
+                region == b_region && offset == b_offset && length == b_length && parent_device == b_parent_device
+            }
+            _ => false,
+        },
+        AmlValue::Field { region, flags, offset, length } => match b {
+            AmlValue::Field { region: b_region, flags: b_flags, offset: b_offset, length: b_length } => {
+                region == b_region && flags == b_flags && offset == b_offset && length == b_length
+            }
+            _ => false,
+        },
+        AmlValue::Method { flags, code } => match b {
+            AmlValue::Method { flags: b_flags, code: b_code } => {
+                if flags != b_flags {
+                    return false;
+                }
+
+                match (code, b_code) {
+                    (MethodCode::Aml(a), MethodCode::Aml(b)) => a == b,
+                    (MethodCode::Aml(_), MethodCode::Native(_)) => false,
+                    (MethodCode::Native(_), MethodCode::Aml(_)) => false,
+                    (MethodCode::Native(_), MethodCode::Native(_)) => panic!("Can't compare two native methods"),
+                }
+            }
+            _ => false,
+        },
+        AmlValue::Buffer(a) => match b {
+            AmlValue::Buffer(b) => a == b,
+            _ => false,
+        },
+        AmlValue::Processor { id, pblk_address, pblk_len } => match b {
+            AmlValue::Processor { id: b_id, pblk_address: b_pblk_address, pblk_len: b_pblk_len } => {
+                id == b_id && pblk_address == b_pblk_address && pblk_len == b_pblk_len
+            }
+            _ => false,
+        },
+        AmlValue::Mutex { sync_level } => match b {
+            AmlValue::Mutex { sync_level: b_sync_level } => sync_level == b_sync_level,
+            _ => false,
+        },
+        AmlValue::Package(a) => match b {
+            AmlValue::Package(b) => {
+                for (a, b) in a.iter().zip(b) {
+                    if crudely_cmp_values(a, b) == false {
+                        return false;
+                    }
+                }
+
+                true
+            }
+            _ => false,
+        },
+    }
+}

aml/src/type1.rs

@@ -1,9 +1,8 @@
 use crate::{
     opcode::{self, opcode},
-    parser::{choice, comment_scope, id, take_to_end_of_pkglength, ParseResult, Parser},
+    parser::{choice, comment_scope, id, take_to_end_of_pkglength, ParseResult, Parser, Propagate},
     pkg_length::{pkg_length, PkgLength},
     term_object::{term_arg, term_list},
-    AmlError,
     DebugVerbosity,
 };
 
@@ -110,14 +109,14 @@ where
         .then(comment_scope(
             DebugVerbosity::Scopes,
             "DefReturn",
-            term_arg().map(|return_arg| -> Result<(), AmlError> {
+            term_arg().map(|return_arg| -> Result<(), Propagate> {
                 /*
                  * To return a value, we want to halt execution of the method and propagate the
                  * return value all the way up to the start of the method invocation. To do this,
                  * we emit a special error that is intercepted during method invocation and turned
                  * into a valid result.
                  */
-                Err(AmlError::Return(return_arg))
+                Err(Propagate::Return(return_arg))
             }),
         ))
         .discard_result()

aml/src/type2.rs

@@ -10,6 +10,7 @@ use crate::{
         take_to_end_of_pkglength,
         try_with_context,
         Parser,
+        Propagate,
     },
     pkg_length::pkg_length,
     term_object::{data_ref_object, term_arg},
@@ -134,7 +135,7 @@ where
                     let buffer_size = try_with_context!(context, buffer_size.as_integer(context)) as usize;
 
                     if buffer_size < bytes.len() {
-                        return (Err(AmlError::MalformedBuffer), context);
+                        return (Err(Propagate::Err(AmlError::MalformedBuffer)), context);
                     }
 
                     let mut buffer = vec![0; buffer_size];
@@ -312,7 +313,7 @@ where
                     }
 
                     if package_contents.len() != num_elements as usize {
-                        return Err((input, context, AmlError::MalformedPackage));
+                        return Err((input, context, Propagate::Err(AmlError::MalformedPackage)));
                     }
 
                     Ok((input, context, AmlValue::Package(package_contents)))

aml/src/value.rs

@@ -1,7 +1,7 @@
 use crate::{misc::ArgNum, AmlContext, AmlError, AmlHandle, AmlName};
-use alloc::{string::String, vec::Vec};
+use alloc::{rc::Rc, string::String, vec::Vec};
 use bit_field::BitField;
-use core::cmp;
+use core::{cmp, fmt, fmt::Debug};
 
 #[derive(Clone, Copy, PartialEq, Eq, Debug)]
 pub enum RegionSpace {
@@ -140,7 +140,22 @@ pub enum AmlType {
     ThermalZone,
 }
 
-#[derive(Clone, PartialEq, Eq, Debug)]
+#[derive(Clone)]
+pub enum MethodCode {
+    Aml(Vec<u8>),
+    Native(Rc<dyn Fn(&mut AmlContext) -> Result<AmlValue, AmlError>>),
+}
+
+impl fmt::Debug for MethodCode {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            MethodCode::Aml(ref code) => f.debug_struct("AML method").field("code", code).finish(),
+            MethodCode::Native(_) => f.debug_struct("Native method").finish(),
+        }
+    }
+}
+
+#[derive(Clone, Debug)]
 pub enum AmlValue {
     Boolean(bool),
     Integer(u64),
@@ -163,7 +178,7 @@ pub enum AmlValue {
     },
     Method {
         flags: MethodFlags,
-        code: Vec<u8>,
+        code: MethodCode,
     },
     Buffer(Vec<u8>),
     Processor {

aml_tester/src/main.rs

@@ -56,7 +56,7 @@ fn main() -> std::io::Result<()> {
         file.read_to_end(&mut contents).unwrap();
 
         const AML_TABLE_HEADER_LENGTH: usize = 36;
-        let mut context = AmlContext::new(Box::new(Handler), false, DebugVerbosity::None);
+        let mut context = AmlContext::new(Box::new(Handler), DebugVerbosity::None);
 
         match context.parse_table(&contents[AML_TABLE_HEADER_LENGTH..]) {
             Ok(()) => {