Handle DerefOf buffer fields

Allow DerefOf to read directly from BufferField objects. This matches AML
patterns emitted by firmware that creates named fields over buffers and then
uses DerefOf(field).

Author: ArthurHeymans

src/aml/mod.rs

@@ -839,19 +839,17 @@ where
                     }
                     Opcode::DerefOf => {
                         extract_args!(op => [Argument::Object(object)]);
-                        let result = match **object {
-                            Object::Reference { kind: _, inner: _ } => object.clone().unwrap_reference(),
-                            Object::String(_) => {
-                                let path = AmlName::from_str(&object.as_string().unwrap())?;
+                        let object = object.clone().unwrap_reference();
+                        let result = match &*object {
+                            Object::BufferField { .. } => object.read_buffer_field(self.integer_size)?.wrap(),
+                            Object::FieldUnit(field) => self.do_field_read(field)?,
+                            Object::String(path) => {
+                                let path = AmlName::from_str(path)?;
                                 let (_, object) = self.namespace.lock().search(&path, &context.current_scope)?;
                                 object.clone()
                             }
-                            _ => {
-                                return Err(AmlError::ObjectNotOfExpectedType {
-                                    expected: ObjectType::Reference,
-                                    got: object.typ(),
-                                });
-                            }
+                            Object::Reference { .. } => unreachable!(),
+                            _ => object,
                         };
                         context.contribute_arg(Argument::Object(result));
                         context.retire_op(op);
@@ -1675,7 +1673,7 @@ where
                 Opcode::FindSetLeftBit | Opcode::FindSetRightBit => {
                     context.start(OpInFlight::new(opcode, &[ResolveBehaviour::TermArg, ResolveBehaviour::Target]))
                 }
-                Opcode::DerefOf => context.start(OpInFlight::new(opcode, &[ResolveBehaviour::TermArg])),
+                Opcode::DerefOf => context.start(OpInFlight::new(opcode, &[ResolveBehaviour::SuperName])),
                 Opcode::ConcatRes => context.start(OpInFlight::new(
                     opcode,
                     &[ResolveBehaviour::TermArg, ResolveBehaviour::TermArg, ResolveBehaviour::Target],

src/aml/object.rs

@@ -261,10 +261,16 @@ impl Object {
 
     pub fn read_buffer_field(&self, integer_size: IntegerSize) -> Result<Object, AmlError> {
         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!(),
+            let buffer = buffer.clone().unwrap_transparent_reference();
+            let buffer = match &*buffer {
+                Object::Buffer(buffer) => buffer.as_slice(),
+                Object::String(string) => string.as_bytes(),
+                typ => {
+                    return Err(AmlError::InvalidOperationOnObject {
+                        op: Operation::ReadBufferField,
+                        typ: typ.typ(),
+                    });
+                }
             };
             if *length <= integer_size as usize {
                 let mut dst = [0u8; 8];
@@ -283,12 +289,18 @@ impl Object {
     pub fn write_buffer_field(&mut self, value: &[u8], token: &ObjectToken) -> Result<(), AmlError> {
         // TODO: bounds check the buffer first to avoid panicking
         if let Self::BufferField { buffer, offset, length } = self {
+            let buffer = buffer.clone().unwrap_transparent_reference();
             let buffer = match unsafe { buffer.gain_mut(token) } {
                 Object::Buffer(buffer) => buffer.as_mut_slice(),
                 // XXX: this unfortunately requires us to trust AML to keep the string as valid
                 // UTF8... maybe there is a better way?
                 Object::String(string) => unsafe { string.as_bytes_mut() },
-                _ => panic!(),
+                typ => {
+                    return Err(AmlError::InvalidOperationOnObject {
+                        op: Operation::WriteBufferField,
+                        typ: typ.typ(),
+                    });
+                }
             };
             copy_bits(value, 0, buffer, *offset, *length);
             Ok(())

tests/de_ref_of.rs

@@ -26,3 +26,72 @@ DefinitionBlock ("", "SSDT", 2, "RSACPI", "DerefOf", 0x00000002) {
     let handler = NullHandler {};
     test_infra::run_aml_test(AML, handler);
 }
+
+#[test]
+fn test_deref_of_direct_buffer_field() {
+    const AML: &str = r#"
+DefinitionBlock ("", "SSDT", 2, "RSACPI", "DerefOf", 0x00000002) {
+    Name (ADAT, Buffer (0x01) { 0xaa })
+    CreateByteField (ADAT, 0x00, BF00)
+
+    Method(MAIN, 0, NotSerialized) {
+        Return (DerefOf(BF00) - 0xaa)
+    }
+}
+"#;
+
+    let handler = NullHandler {};
+    test_infra::run_aml_test(AML, handler);
+}
+
+#[test]
+fn test_deref_of_local_buffer_field_after_store() {
+    const AML: &str = r#"
+DefinitionBlock ("", "SSDT", 2, "RSACPI", "DerefOf", 0x00000002) {
+    Method(MAIN, 0, NotSerialized) {
+        Local0 = Buffer (0x01) { 0x00 }
+        CreateByteField (Local0, 0x00, BF00)
+        BF00 = 0xaa
+        Return (DerefOf(BF00) - 0xaa)
+    }
+}
+"#;
+
+    let handler = NullHandler {};
+    test_infra::run_aml_test(AML, handler);
+}
+
+#[test]
+fn test_deref_of_named_buffer_field_stores_value() {
+    const AML: &str = r#"
+DefinitionBlock ("", "SSDT", 2, "RSACPI", "DerefOf", 0x00000002) {
+    Name (ADAT, Buffer (0x01) { 0xaa })
+    CreateByteField (ADAT, 0x00, BF00)
+
+    Method(MAIN, 0, NotSerialized) {
+        Local0 = DerefOf(BF00)
+        Return (Local0 - 0xaa)
+    }
+}
+"#;
+
+    let handler = NullHandler {};
+    test_infra::run_aml_test(AML, handler);
+}
+
+#[test]
+fn test_deref_of_named_string_path() {
+    const AML: &str = r#"
+DefinitionBlock ("", "SSDT", 2, "RSACPI", "DerefOf", 0x00000002) {
+    Name (TGT, 0xaa)
+    Name (STR, "\\TGT")
+
+    Method(MAIN, 0, NotSerialized) {
+        Return (DerefOf(STR) - 0xaa)
+    }
+}
+"#;
+
+    let handler = NullHandler {};
+    test_infra::run_aml_test(AML, handler);
+}

tests/operation_region.rs

@@ -1,4 +1,12 @@
-use aml_test_tools::handlers::std_test_handler::{Command, construct_std_handler, create_mutex, read_u8, write_pci_u8, write_u8, read_pci_u8};
+use aml_test_tools::handlers::std_test_handler::{
+    Command,
+    construct_std_handler,
+    create_mutex,
+    read_pci_u8,
+    read_u8,
+    write_pci_u8,
+    write_u8,
+};
 use pci_types::PciAddress;
 
 mod test_infra;

tests/test_infra/mod.rs

@@ -1,5 +1,12 @@
 use acpi::Handler;
-use aml_test_tools::{RunTestResult, TestResult, handlers::logging_handler::LoggingHandler, new_interpreter, run_test_for_string, run_test_for_opcodes};
+use aml_test_tools::{
+    RunTestResult,
+    TestResult,
+    handlers::logging_handler::LoggingHandler,
+    new_interpreter,
+    run_test_for_opcodes,
+    run_test_for_string,
+};
 
 // The following two functions are very similar in structure, but whilst there are only two of them
 // it's not worth adding complexity to make them DRY.

tests/uacpi_examples.rs

@@ -9,7 +9,7 @@
 
 mod test_infra;
 
-use aml_test_tools::{handlers::null_handler::NullHandler};
+use aml_test_tools::handlers::null_handler::NullHandler;
 
 #[test]
 fn expressions_with_package() {

tools/aml_test_tools/src/handlers/check_cmd_handler.rs

@@ -271,8 +271,8 @@ where
 
 #[cfg(test)]
 mod test {
-    use crate::handlers::null_handler::NullHandler;
     use super::*;
+    use crate::handlers::null_handler::NullHandler;
 
     #[test]
     fn handler_basic_functions() {
@@ -322,4 +322,4 @@ mod test {
         let handler = CheckCommandHandler::new(test_commands, NullHandler {});
         handler.read_io_u8(2);
     }
-}
+}

tools/aml_test_tools/src/handlers/std_test_handler.rs

@@ -1,12 +1,12 @@
 //! Rather than defining a [`Handler`], this module defines useful functions to streamline the most-
 //! used case of a [`CheckCommandHandler`] wrapping a [`ListedResponseHandler`].
 
-use pci_types::PciAddress;
 use crate::handlers::{
     check_cmd_handler::{AcpiCommands as Check, CheckCommandHandler},
     listed_response_handler::{AcpiCommands as Response, ListedResponseHandler},
 };
 use acpi::{Handle, Handler};
+use pci_types::PciAddress;
 
 /// Simplifies the construction of a standard test [`Handler`].
 ///
@@ -162,4 +162,4 @@ pub const fn acquire(mutex: Handle, timeout: u16) -> Command {
 /// A simple helper to generate a [`Command`] for [`Handler::release`].
 pub const fn release(mutex: Handle) -> Command {
     (Check::Release(mutex), Response::Skip())
-}
+}