Browse Source

Merge pull request #72 from IsaacWoods/new_namespace

Move to new namespace representation
Isaac Woods 4 năm trước cách đây
mục cha
commit
bfac584a55

+ 1 - 1
acpi/src/rsdp.rs

@@ -1,5 +1,5 @@
 use super::AcpiError;
-use core::{mem, slice, str};
+use core::{slice, str};
 
 /// The first structure found in ACPI. It just tells us where the RSDT is.
 ///

+ 0 - 4
aml/Cargo.toml

@@ -13,7 +13,3 @@ edition = "2018"
 log = "0.4"
 bit_field = "0.10"
 byteorder = { version = "1", default-features = false }
-
-[features]
-debug_parser = []
-debug_parser_verbose = []

+ 72 - 8
aml/src/lib.rs

@@ -60,9 +60,9 @@ pub use crate::{
     value::AmlValue,
 };
 
-use alloc::string::String;
 use log::error;
 use misc::{ArgNum, LocalNum};
+use namespace::LevelType;
 use parser::Parser;
 use pkg_length::PkgLength;
 use term_object::term_list;
@@ -72,10 +72,25 @@ use value::Args;
 /// what this is actually used for, but this is ours.
 pub const AML_INTERPRETER_REVISION: u64 = 0;
 
+/// Describes how much debug information the parser should emit. Set the "maximum" expected verbosity in
+/// the context's `debug_verbosity` - everything will be printed that is less or equal in 'verbosity'.
+#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug)]
+pub enum DebugVerbosity {
+    /// Print no debug information
+    None,
+    /// Print heads and tails when entering and leaving scopes of major objects, but not more minor ones.
+    Scopes,
+    /// Print heads and tails when entering and leaving scopes of all objects.
+    AllScopes,
+    /// Print heads and tails of all objects, and extra debug information as it's parsed.
+    All,
+}
+
 #[derive(Debug)]
 pub struct AmlContext {
+    legacy_mode: bool,
+
     pub namespace: Namespace,
-    current_scope: AmlName,
 
     /*
      * AML local variables. These are used when we invoke a control method. A `None` value
@@ -93,13 +108,30 @@ pub struct AmlContext {
     /// If we're currently invoking a control method, this stores the arguments that were passed to
     /// it. It's `None` if we aren't invoking a method.
     current_args: Option<Args>,
+
+    /*
+     * These track the state of the context while it's parsing an AML table.
+     */
+    current_scope: AmlName,
+    scope_indent: usize,
+    debug_verbosity: DebugVerbosity,
 }
 
 impl AmlContext {
-    pub fn new() -> AmlContext {
-        AmlContext {
+    /// Creates a new `AmlContext` - the central type in managing the AML tables. Only one of these should be
+    /// created, and it should be passed the DSDT and all SSDTs defined by the hardware.
+    ///
+    /// ### Legacy mode
+    /// If `true` is passed in `legacy_mode`, the library will try and remain compatible with a ACPI 1.0
+    /// implementation. The following changes/assumptions are made:
+    ///     - Two extra root namespaces are predefined: `\_PR` and `_TZ`
+    ///     - Processors are expected to be defined with `DefProcessor`, instead of `DefDevice`
+    ///     - Processors are expected to be found in `\_PR`, instead of `\_SB`
+    ///     - Thermal zones are expected to be found in `\_TZ`, instead of `\_SB`
+    pub fn new(legacy_mode: bool, debug_verbosity: DebugVerbosity) -> AmlContext {
+        let mut context = AmlContext {
+            legacy_mode,
             namespace: Namespace::new(),
-            current_scope: AmlName::root(),
             local_0: None,
             local_1: None,
             local_2: None,
@@ -109,7 +141,24 @@ impl AmlContext {
             local_6: None,
             local_7: None,
             current_args: None,
+
+            current_scope: AmlName::root(),
+            scope_indent: 0,
+            debug_verbosity,
+        };
+
+        /*
+         * Add the predefined root namespaces.
+         */
+        context.namespace.add_level(AmlName::from_str("\\_GPE").unwrap(), LevelType::Scope).unwrap();
+        context.namespace.add_level(AmlName::from_str("\\_SB").unwrap(), LevelType::Scope).unwrap();
+        context.namespace.add_level(AmlName::from_str("\\_SI").unwrap(), LevelType::Scope).unwrap();
+        if legacy_mode {
+            context.namespace.add_level(AmlName::from_str("\\_PR").unwrap(), LevelType::Scope).unwrap();
+            context.namespace.add_level(AmlName::from_str("\\_TZ").unwrap(), LevelType::Scope).unwrap();
         }
+
+        context
     }
 
     pub fn parse_table(&mut self, stream: &[u8]) -> Result<(), AmlError> {
@@ -145,6 +194,11 @@ impl AmlContext {
             self.local_6 = None;
             self.local_7 = None;
 
+            /*
+             * Create a namespace level to store local objects created by the invocation.
+             */
+            self.namespace.add_level(path.clone(), LevelType::MethodLocals)?;
+
             log::trace!("Invoking method with {} arguments, code: {:x?}", flags.arg_count(), code);
             let return_value =
                 match term_list(PkgLength::from_raw_length(&code, code.len() as u32)).parse(&code, self) {
@@ -157,6 +211,14 @@ impl AmlContext {
                     }
                 };
 
+            /*
+             * Locally-created objects should be destroyed on method exit (see §5.5.2.3 of the ACPI spec). We do
+             * this by simply removing the method's local object layer.
+             */
+            // TODO: this should also remove objects created by the method outside the method's scope, if they
+            // weren't statically created. This is harder.
+            self.namespace.remove_level(path.clone())?;
+
             /*
              * Now clear the state.
              */
@@ -230,11 +292,13 @@ pub enum AmlError {
     /*
      * Errors produced working with the namespace.
      */
-    /// Produced when a path is given that does not point to an object in the AML namespace.
-    ObjectDoesNotExist(String),
-    HandleDoesNotExist(AmlHandle),
+    /// Produced when a sub-level or value is added to a level that has not yet been added to the namespace. The
+    /// `AmlName` is the name of the entire sub-level/value.
+    LevelDoesNotExist(AmlName),
+    ValueDoesNotExist(AmlName),
     /// Produced when two values with the same name are added to the namespace.
     NameCollision(AmlName),
+    TriedToRemoveRootNamespace,
 
     /*
      * Errors produced executing control methods.

+ 8 - 4
aml/src/misc.rs

@@ -1,6 +1,7 @@
 use crate::{
     opcode::{self, ext_opcode, opcode},
-    parser::{choice, comment_scope_verbose, id, Parser},
+    parser::{choice, comment_scope, id, Parser},
+    DebugVerbosity,
 };
 
 pub fn debug_obj<'a, 'c>() -> impl Parser<'a, 'c, ()>
@@ -32,7 +33,9 @@ where
      * Local7Op := 0x67
      */
     let local_parser = |i, local_opcode| {
-        opcode(local_opcode).then(comment_scope_verbose("LocalObj", id())).map(move |((), _)| Ok(i))
+        opcode(local_opcode)
+            .then(comment_scope(DebugVerbosity::AllScopes, "LocalObj", id()))
+            .map(move |((), _)| Ok(i))
     };
 
     choice!(
@@ -64,8 +67,9 @@ where
      * Arg5Op = 0x6d
      * Arg6Op = 0x6e
      */
-    let arg_parser =
-        |i, arg_opcode| opcode(arg_opcode).then(comment_scope_verbose("ArgObj", id())).map(move |((), _)| Ok(i));
+    let arg_parser = |i, arg_opcode| {
+        opcode(arg_opcode).then(comment_scope(DebugVerbosity::AllScopes, "ArgObj", id())).map(move |((), _)| Ok(i))
+    };
 
     choice!(
         arg_parser(0, opcode::ARG0_OP),

+ 14 - 8
aml/src/name_object.rs

@@ -2,9 +2,10 @@ 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_verbose, consume, n_of, take, take_while, Parser},
+    parser::{choice, comment_scope, consume, n_of, take, take_while, Parser},
     AmlContext,
     AmlError,
+    DebugVerbosity,
 };
 use alloc::vec::Vec;
 use core::{fmt, str};
@@ -26,7 +27,11 @@ where
      * SuperName := SimpleName | DebugObj | Type6Opcode
      * TODO: this doesn't cover Type6Opcode yet
      */
-    comment_scope_verbose("SuperName", choice!(debug_obj().map(|()| Ok(Target::Debug)), simple_name()))
+    comment_scope(
+        DebugVerbosity::AllScopes,
+        "SuperName",
+        choice!(debug_obj().map(|()| Ok(Target::Debug)), simple_name()),
+    )
 }
 
 pub fn simple_name<'a, 'c>() -> impl Parser<'a, 'c, Target>
@@ -36,7 +41,8 @@ where
     /*
      * SimpleName := NameString | ArgObj | LocalObj
      */
-    comment_scope_verbose(
+    comment_scope(
+        DebugVerbosity::AllScopes,
         "SimpleName",
         choice!(
             name_string().map(move |name| Ok(Target::Name(name))),
@@ -68,7 +74,7 @@ where
         });
 
     // TODO: combinator to select a parser based on a peeked byte?
-    comment_scope_verbose("NameString", move |input: &'a [u8], context| {
+    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)),
@@ -222,11 +228,11 @@ fn is_name_char(byte: u8) -> bool {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::{parser::Parser, test_utils::*, AmlContext, AmlError};
+    use crate::{parser::Parser, test_utils::*, AmlContext, AmlError, DebugVerbosity};
 
     #[test]
     fn test_name_seg() {
-        let mut context = AmlContext::new();
+        let mut context = AmlContext::new(false, DebugVerbosity::None);
 
         check_ok!(
             name_seg().parse(&[b'A', b'F', b'3', b'Z'], &mut context),
@@ -248,7 +254,7 @@ mod tests {
 
     #[test]
     fn test_name_path() {
-        let mut context = AmlContext::new();
+        let mut context = AmlContext::new(false, DebugVerbosity::None);
 
         check_err!(name_path().parse(&[], &mut context), AmlError::UnexpectedEndOfStream, &[]);
         check_ok!(name_path().parse(&[0x00], &mut context), alloc::vec![], &[]);
@@ -266,7 +272,7 @@ mod tests {
 
     #[test]
     fn test_prefix_path() {
-        let mut context = AmlContext::new();
+        let mut context = AmlContext::new(false, DebugVerbosity::None);
 
         check_ok!(
             name_string().parse(&[b'^', b'A', b'B', b'C', b'D'], &mut context),

+ 324 - 48
aml/src/namespace.rs

@@ -23,82 +23,157 @@ impl AmlHandle {
     }
 }
 
+#[derive(Clone, Copy, PartialEq, Eq, Debug)]
+pub enum LevelType {
+    Scope,
+    Device,
+    /// A legacy `Processor` object's sub-objects are stored in a level of this type. Modern tables define
+    /// processors as `Device`s.
+    Processor,
+    /// A level of this type is created at the same path as the name of a method when it is invoked. It can be
+    /// used by the method to store local variables.
+    MethodLocals,
+}
+
+pub struct NamespaceLevel {
+    typ: LevelType,
+    children: BTreeMap<NameSeg, NamespaceLevel>,
+    values: BTreeMap<NameSeg, AmlHandle>,
+}
+
+impl NamespaceLevel {
+    pub(crate) fn new(typ: LevelType) -> NamespaceLevel {
+        NamespaceLevel { typ, children: BTreeMap::new(), values: BTreeMap::new() }
+    }
+}
+
 pub struct Namespace {
     /// This is a running count of ids, which are never reused. This is incremented every time we
     /// add a new object to the namespace. We can then remove objects, freeing their memory, without
     /// risking using the same id for two objects.
     next_handle: AmlHandle,
 
-    /// This maps handles to actual values, and is used to access the actual AML values.
+    /// This maps handles to actual values, and is used to access the actual AML values. When removing a value
+    /// from the object map, care must be taken to also remove references to its handle in the level data
+    /// structure, as invalid handles will cause panics.
     object_map: BTreeMap<AmlHandle, AmlValue>,
 
-    /// This maps names to handles, and should be used when you don't already have the handle to a
-    /// value.
-    // XXX: in the future, this could be replaced with a better data structure that doesn't store
-    // the entire name for each id. Instead, it would be a tree-like structure that stores each
-    // name segment, with a list of objects and their names, and a list of child scopes at that
-    // level.
-    name_map: BTreeMap<AmlName, AmlHandle>,
+    /// Holds the first level of the namespace - containing items such as `\_SB`. Subsequent levels are held
+    /// recursively inside this structure. It holds handles to references, which need to be indexed into
+    /// `object_map` to acctually access the object.
+    root: NamespaceLevel,
 }
 
 impl Namespace {
     pub fn new() -> Namespace {
-        Namespace { next_handle: AmlHandle(0), object_map: BTreeMap::new(), name_map: BTreeMap::new() }
+        Namespace {
+            next_handle: AmlHandle(0),
+            object_map: BTreeMap::new(),
+            root: NamespaceLevel::new(LevelType::Scope),
+        }
+    }
+
+    /// Add a new level to the namespace. A "level" is named by a single `NameSeg`, and can contain values, and
+    /// also other further sub-levels. Once a level has been created, AML values can be added to it with
+    /// `add_value`.
+    ///
+    /// ### Note
+    /// At first glance, you might expect `DefDevice` to add a value of type `Device`. However, because all
+    /// `Devices` do is hold other values, we model them as namespace levels, and so they must be created
+    /// accordingly.
+    pub fn add_level(&mut self, path: AmlName, typ: LevelType) -> Result<(), AmlError> {
+        assert!(path.is_absolute());
+        let path = path.normalize()?;
+
+        /*
+         * We need to handle a special case here: if a `Scope(\) { ... }` appears in the AML, the parser will
+         * try and recreate the root scope. Instead of handling this specially in the parser, we just
+         * return nicely here.
+         */
+        if path != AmlName::root() {
+            let (level, last_seg) = self.get_level_for_path_mut(&path)?;
+
+            /*
+             * If the level has already been added, we don't need to add it again. The parser can try to add it
+             * multiple times if the ASL contains multiple blocks that add to the same scope/device.
+             */
+            if !level.children.contains_key(&last_seg) {
+                level.children.insert(last_seg, NamespaceLevel::new(typ));
+            }
+        }
+
+        Ok(())
+    }
+
+    pub fn remove_level(&mut self, path: AmlName) -> Result<(), AmlError> {
+        assert!(path.is_absolute());
+        let path = path.normalize()?;
+
+        if path != AmlName::root() {
+            let (level, last_seg) = self.get_level_for_path_mut(&path)?;
+
+            match level.children.remove(&last_seg) {
+                Some(_) => Ok(()),
+                None => Err(AmlError::LevelDoesNotExist(path)),
+            }
+        } else {
+            Err(AmlError::TriedToRemoveRootNamespace)
+        }
     }
 
     /// Add a value to the namespace at the given path, which must be a normalized, absolute AML
     /// name. If you want to add at a path relative to a given scope, use `add_at_resolved_path`
     /// instead.
-    pub fn add(&mut self, path: AmlName, value: AmlValue) -> Result<AmlHandle, AmlError> {
+    pub fn add_value(&mut self, path: AmlName, value: AmlValue) -> Result<AmlHandle, AmlError> {
         assert!(path.is_absolute());
-        assert!(path.is_normal());
-
-        if self.name_map.contains_key(&path) {
-            return Err(AmlError::NameCollision(path.clone()));
-        }
+        let path = path.normalize()?;
 
         let handle = self.next_handle;
         self.next_handle.increment();
-
         self.object_map.insert(handle, value);
-        self.name_map.insert(path, handle);
 
-        Ok(handle)
+        let (level, last_seg) = self.get_level_for_path_mut(&path)?;
+        match level.values.insert(last_seg, handle) {
+            None => Ok(handle),
+            Some(_) => Err(AmlError::NameCollision(path)),
+        }
     }
 
     /// Helper method for adding a value to the namespace at a path that is relative to the given
     /// scope. This operation involves a lot of error handling in parts of the parser, so is
     /// encapsulated here.
-    pub fn add_at_resolved_path(
+    pub fn add_value_at_resolved_path(
         &mut self,
         path: AmlName,
         scope: &AmlName,
         value: AmlValue,
     ) -> Result<AmlHandle, AmlError> {
-        self.add(path.resolve(scope)?, value)
+        self.add_value(path.resolve(scope)?, value)
     }
 
     pub fn get(&self, handle: AmlHandle) -> Result<&AmlValue, AmlError> {
-        self.object_map.get(&handle).ok_or(AmlError::HandleDoesNotExist(handle))
+        Ok(self.object_map.get(&handle).unwrap())
     }
 
-    pub fn get_by_path(&self, path: &AmlName) -> Result<&AmlValue, AmlError> {
-        let handle = *self.name_map.get(path).ok_or(AmlError::ObjectDoesNotExist(path.as_string()))?;
-        self.get(handle).map_err(|_| AmlError::ObjectDoesNotExist(path.as_string()))
+    pub fn get_mut(&mut self, handle: AmlHandle) -> Result<&mut AmlValue, AmlError> {
+        Ok(self.object_map.get_mut(&handle).unwrap())
     }
 
-    pub fn get_mut(&mut self, handle: AmlHandle) -> Result<&mut AmlValue, AmlError> {
-        self.object_map.get_mut(&handle).ok_or(AmlError::HandleDoesNotExist(handle))
+    pub fn get_by_path(&self, path: &AmlName) -> Result<&AmlValue, AmlError> {
+        let (level, last_seg) = self.get_level_for_path(path)?;
+        let &handle = level.values.get(&last_seg).ok_or(AmlError::ValueDoesNotExist(path.clone()))?;
+        Ok(self.get(handle).unwrap())
     }
 
     pub fn get_by_path_mut(&mut self, path: &AmlName) -> Result<&mut AmlValue, AmlError> {
-        let handle = *self.name_map.get(path).ok_or(AmlError::ObjectDoesNotExist(path.as_string()))?;
-        self.get_mut(handle).map_err(|_| AmlError::ObjectDoesNotExist(path.as_string()))
+        let (level, last_seg) = self.get_level_for_path(path)?;
+        let &handle = level.values.get(&last_seg).ok_or(AmlError::ValueDoesNotExist(path.clone()))?;
+        Ok(self.get_mut(handle).unwrap())
     }
 
-    /// Search for an object at the given path of the namespace, applying the search rules
-    /// described in §5.3 of the ACPI specification, if they are applicable. Returns the resolved name, and the
-    /// handle of the first valid object, if found.
+    /// Search for an object at the given path of the namespace, applying the search rules described in §5.3 of the
+    /// ACPI specification, if they are applicable. Returns the resolved name, and the handle of the first valid
+    /// object, if found.
     pub fn search(&self, path: &AmlName, starting_scope: &AmlName) -> Result<(AmlName, AmlHandle), AmlError> {
         if path.search_rules_apply() {
             /*
@@ -111,40 +186,148 @@ impl Namespace {
             loop {
                 // Search for the name at this namespace level. If we find it, we're done.
                 let name = path.resolve(&scope)?;
-                if let Some(handle) = self.name_map.get(&name) {
-                    return Ok((name, *handle));
+                match self.get_level_for_path(&name) {
+                    Ok((level, last_seg)) => {
+                        if let Some(&handle) = level.values.get(&last_seg) {
+                            return Ok((name, handle));
+                        }
+                    }
+
+                    /*
+                     * This error is caught specially to avoid a case that seems bizzare but is quite useful - when
+                     * the passed starting scope doesn't exist. Certain methods return values that reference names
+                     * from the point of view of the method, so it makes sense for the starting scope to be inside
+                     * the method.  However, because we have destroyed all the objects created by the method
+                     * dynamically, the level no longer exists.
+                     *
+                     * To avoid erroring here, we simply continue to the parent scope. If the whole scope doesn't
+                     * exist, this will error when we get to the root, so this seems unlikely to introduce bugs.
+                     */
+                    Err(AmlError::LevelDoesNotExist(_)) => (),
+                    Err(err) => return Err(err),
                 }
 
-                // If we don't find it, go up a level in the namespace and search for it there,
-                // recursively.
+                // If we don't find it, go up a level in the namespace and search for it there recursively
                 match scope.parent() {
                     Ok(parent) => scope = parent,
                     // If we still haven't found the value and have run out of parents, return `None`.
-                    Err(AmlError::RootHasNoParent) => return Err(AmlError::ObjectDoesNotExist(path.as_string())),
+                    Err(AmlError::RootHasNoParent) => return Err(AmlError::ValueDoesNotExist(path.clone())),
                     Err(err) => return Err(err),
                 }
             }
         } else {
             // If search rules don't apply, simply resolve it against the starting scope
             let name = path.resolve(starting_scope)?;
-            Ok((
-                name,
-                self.name_map
-                    .get(&path.resolve(starting_scope)?)
-                    .map(|&handle| handle)
-                    .ok_or(AmlError::ObjectDoesNotExist(path.as_string()))?,
-            ))
+            let (level, last_seg) = self.get_level_for_path(&path.resolve(starting_scope)?)?;
+
+            if let Some(&handle) = level.values.get(&last_seg) {
+                Ok((name, handle))
+            } else {
+                Err(AmlError::ValueDoesNotExist(path.clone()))
+            }
         }
     }
+
+    pub fn search_for_level(&self, level_name: &AmlName, starting_scope: &AmlName) -> Result<AmlName, AmlError> {
+        if level_name.search_rules_apply() {
+            let mut scope = starting_scope.clone().normalize()?;
+            assert!(scope.is_absolute());
+
+            loop {
+                let name = level_name.resolve(&scope)?;
+                if let Ok((level, last_seg)) = self.get_level_for_path(&name) {
+                    if let Some(_) = level.children.get(&last_seg) {
+                        return Ok(name);
+                    }
+                }
+
+                // If we don't find it, move the scope up a level and search for it there recursively
+                match scope.parent() {
+                    Ok(parent) => scope = parent,
+                    Err(AmlError::RootHasNoParent) => return Err(AmlError::LevelDoesNotExist(level_name.clone())),
+                    Err(err) => return Err(err),
+                }
+            }
+        } else {
+            Ok(level_name.clone())
+        }
+    }
+
+    fn get_level_for_path(&self, path: &AmlName) -> Result<(&NamespaceLevel, NameSeg), AmlError> {
+        let (last_seg, levels) = path.0[1..].split_last().unwrap();
+        let last_seg = last_seg.as_segment().unwrap();
+
+        // TODO: this helps with diagnostics, but requires a heap allocation just in case we need to error.
+        let mut traversed_path = AmlName::root();
+
+        let mut current_level = &self.root;
+        for level in levels {
+            traversed_path.0.push(*level);
+            current_level = current_level
+                .children
+                .get(&level.as_segment().unwrap())
+                .ok_or(AmlError::LevelDoesNotExist(traversed_path.clone()))?;
+        }
+
+        Ok((current_level, last_seg))
+    }
+
+    /// Split an absolute path into a bunch of level segments (used to traverse the level data structure), and a
+    /// last segment to index into that level. This must not be called on `\\`.
+    fn get_level_for_path_mut(&mut self, path: &AmlName) -> Result<(&mut NamespaceLevel, NameSeg), AmlError> {
+        let (last_seg, levels) = path.0[1..].split_last().unwrap();
+        let last_seg = last_seg.as_segment().unwrap();
+
+        // TODO: this helps with diagnostics, but requires a heap allocation just in case we need to error. We can
+        // improve this by changing the `levels` interation into an `enumerate()`, and then using the index to
+        // create the correct path on the error path
+        let mut traversed_path = AmlName::root();
+
+        let mut current_level = &mut self.root;
+        for level in levels {
+            traversed_path.0.push(*level);
+            current_level = current_level
+                .children
+                .get_mut(&level.as_segment().unwrap())
+                .ok_or(AmlError::LevelDoesNotExist(traversed_path.clone()))?;
+        }
+
+        Ok((current_level, last_seg))
+    }
 }
 
 impl fmt::Debug for Namespace {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        for (name, handle) in self.name_map.iter() {
-            write!(f, "{}: {:?}\n", name, self.object_map.get(handle).unwrap())?;
-        }
+        const INDENT_PER_LEVEL: usize = 4;
+
+        fn print_level(
+            namespace: &Namespace,
+            f: &mut fmt::Formatter<'_>,
+            level_name: &str,
+            level: &NamespaceLevel,
+            indent: usize,
+        ) -> fmt::Result {
+            writeln!(f, "{:indent$}{}:", "", level_name, indent = indent)?;
+
+            for (name, handle) in level.values.iter() {
+                writeln!(
+                    f,
+                    "{:indent$}{}: {:?}",
+                    "",
+                    name.as_str(),
+                    namespace.object_map.get(handle).unwrap(),
+                    indent = indent + INDENT_PER_LEVEL
+                )?;
+            }
 
-        Ok(())
+            for (name, sub_level) in level.children.iter() {
+                print_level(namespace, f, name.as_str(), sub_level, indent + INDENT_PER_LEVEL)?;
+            }
+
+            Ok(())
+        };
+
+        print_level(self, f, "\\", &self.root, 0)
     }
 }
 
@@ -228,7 +411,15 @@ impl AmlName {
     /// Normalize an AML path, resolving prefix chars. Returns `AmlError::InvalidNormalizedName` if the path
     /// normalizes to an invalid path (e.g. `\^_FOO`)
     pub fn normalize(self) -> Result<AmlName, AmlError> {
-        Ok(AmlName(self.0.iter().try_fold(alloc::vec![], |mut name, &component| match component {
+        /*
+         * If the path is already normal, just return it as-is. This avoids an unneccessary heap allocation and
+         * free.
+         */
+        if self.is_normal() {
+            return Ok(self);
+        }
+
+        Ok(AmlName(self.0.iter().try_fold(Vec::new(), |mut name, &component| match component {
             seg @ NameComponent::Segment(_) => {
                 name.push(seg);
                 Ok(name)
@@ -294,6 +485,15 @@ pub enum NameComponent {
     Segment(NameSeg),
 }
 
+impl NameComponent {
+    pub fn as_segment(self) -> Result<NameSeg, ()> {
+        match self {
+            NameComponent::Segment(seg) => Ok(seg),
+            NameComponent::Root | NameComponent::Prefix => Err(()),
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -388,4 +588,80 @@ mod tests {
         assert_eq!(AmlName::from_str("\\_SB.PCI0").unwrap().parent(), Ok(AmlName::from_str("\\_SB").unwrap()));
         assert_eq!(AmlName::from_str("\\_SB.PCI0").unwrap().parent().unwrap().parent(), Ok(AmlName::root()));
     }
+
+    #[test]
+    fn test_namespace() {
+        let mut namespace = Namespace::new();
+
+        /*
+         * This should succeed but do nothing.
+         */
+        assert_eq!(namespace.add_level(AmlName::from_str("\\").unwrap(), LevelType::Scope), Ok(()));
+
+        /*
+         * Add `\_SB`, also testing that adding a level twice succeeds.
+         */
+        assert_eq!(namespace.add_level(AmlName::from_str("\\_SB").unwrap(), LevelType::Scope), Ok(()));
+        assert_eq!(namespace.add_level(AmlName::from_str("\\_SB").unwrap(), LevelType::Scope), Ok(()));
+
+        /*
+         * Add a device under a level that already exists.
+         */
+        assert_eq!(namespace.add_level(AmlName::from_str("\\_SB.PCI0").unwrap(), LevelType::Device), Ok(()));
+
+        /*
+         * Add some deeper scopes.
+         */
+        assert_eq!(namespace.add_level(AmlName::from_str("\\FOO").unwrap(), LevelType::Scope), Ok(()));
+        assert_eq!(namespace.add_level(AmlName::from_str("\\FOO.BAR").unwrap(), LevelType::Scope), Ok(()));
+        assert_eq!(namespace.add_level(AmlName::from_str("\\FOO.BAR.BAZ").unwrap(), LevelType::Scope), Ok(()));
+        assert_eq!(namespace.add_level(AmlName::from_str("\\FOO.BAR.BAZ").unwrap(), LevelType::Scope), Ok(()));
+        assert_eq!(namespace.add_level(AmlName::from_str("\\FOO.BAR.BAZ.QUX").unwrap(), LevelType::Scope), Ok(()));
+
+        /*
+         * Add some things to the scopes to query later.
+         */
+        assert!(namespace.add_value(AmlName::from_str("\\MOO").unwrap(), AmlValue::Boolean(true)).is_ok());
+        assert!(namespace.add_value(AmlName::from_str("\\FOO.BAR.A").unwrap(), AmlValue::Integer(12345)).is_ok());
+        assert!(namespace.add_value(AmlName::from_str("\\FOO.BAR.B").unwrap(), AmlValue::Integer(6)).is_ok());
+        assert!(namespace
+            .add_value(AmlName::from_str("\\FOO.BAR.C").unwrap(), AmlValue::String(String::from("hello, world!")))
+            .is_ok());
+
+        /*
+         * 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!")))
+        );
+
+        /*
+         * Search for some objects that should use search rules.
+         */
+        {
+            let (name, _) = namespace
+                .search(&AmlName::from_str("MOO").unwrap(), &AmlName::from_str("\\FOO.BAR.BAZ").unwrap())
+                .unwrap();
+            assert_eq!(name, AmlName::from_str("\\MOO").unwrap());
+        }
+        {
+            let (name, _) = namespace
+                .search(&AmlName::from_str("A").unwrap(), &AmlName::from_str("\\FOO.BAR").unwrap())
+                .unwrap();
+            assert_eq!(name, AmlName::from_str("\\FOO.BAR.A").unwrap());
+        }
+        {
+            let (name, _) = namespace
+                .search(&AmlName::from_str("A").unwrap(), &AmlName::from_str("\\FOO.BAR.BAZ.QUX").unwrap())
+                .unwrap();
+            assert_eq!(name, AmlName::from_str("\\FOO.BAR.A").unwrap());
+        }
+    }
 }

+ 4 - 4
aml/src/opcode.rs

@@ -88,18 +88,18 @@ where
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::{test_utils::*, AmlError};
+    use crate::{test_utils::*, AmlError, DebugVerbosity};
 
     #[test]
     fn empty() {
-        let mut context = AmlContext::new();
+        let mut context = AmlContext::new(false, DebugVerbosity::None);
         check_err!(opcode(NULL_NAME).parse(&[], &mut context), AmlError::UnexpectedEndOfStream, &[]);
         check_err!(ext_opcode(EXT_DEF_FIELD_OP).parse(&[], &mut context), AmlError::UnexpectedEndOfStream, &[]);
     }
 
     #[test]
     fn simple_opcodes() {
-        let mut context = AmlContext::new();
+        let mut context = AmlContext::new(false, DebugVerbosity::None);
         check_ok!(opcode(DEF_SCOPE_OP).parse(&[DEF_SCOPE_OP], &mut context), (), &[]);
         check_ok!(
             opcode(DEF_NAME_OP).parse(&[DEF_NAME_OP, 0x31, 0x55, 0xf3], &mut context),
@@ -110,7 +110,7 @@ mod tests {
 
     #[test]
     fn extended_opcodes() {
-        let mut context = AmlContext::new();
+        let mut context = AmlContext::new(false, DebugVerbosity::None);
         check_err!(
             ext_opcode(EXT_DEF_FIELD_OP).parse(&[EXT_DEF_FIELD_OP, EXT_DEF_FIELD_OP], &mut context),
             AmlError::WrongParser,

+ 33 - 30
aml/src/parser.rs

@@ -1,6 +1,21 @@
-use crate::{pkg_length::PkgLength, AmlContext, AmlError};
+use crate::{pkg_length::PkgLength, AmlContext, AmlError, DebugVerbosity};
 use alloc::vec::Vec;
 use core::marker::PhantomData;
+use log::trace;
+
+/// This is the number of spaces added to indent a scope when printing parser debug messages.
+pub const INDENT_PER_SCOPE: usize = 2;
+
+impl AmlContext {
+    /// This is used by the parser to provide debug comments about the current object, which are indented to the
+    /// correct level for the current object. We most often need to print these comments from `map_with_context`s,
+    /// so it's most convenient to have this method on `AmlContext`.
+    pub(crate) fn comment(&self, verbosity: DebugVerbosity, message: &str) {
+        if verbosity <= self.debug_verbosity {
+            log::trace!("{:indent$}{}", "", message, indent = self.scope_indent);
+        }
+    }
+}
 
 pub type ParseResult<'a, 'c, R> =
     Result<(&'a [u8], &'c mut AmlContext, R), (&'a [u8], &'c mut AmlContext, AmlError)>;
@@ -224,41 +239,29 @@ where
     }
 }
 
-pub fn comment_scope<'a, 'c, P, R>(scope_name: &'a str, parser: P) -> impl Parser<'a, 'c, R>
-where
-    'c: 'a,
-    R: core::fmt::Debug,
-    P: Parser<'a, 'c, R>,
-{
-    move |input, context| {
-        #[cfg(feature = "debug_parser")]
-        log::trace!("--> {}", scope_name);
-
-        // Return if the parse fails, so we don't print the tail. Makes it easier to debug.
-        let (new_input, context, result) = parser.parse(input, context)?;
-
-        #[cfg(feature = "debug_parser")]
-        log::trace!("<-- {}", scope_name);
-
-        Ok((new_input, context, result))
-    }
-}
-
-pub fn comment_scope_verbose<'a, 'c, P, R>(scope_name: &'a str, parser: P) -> impl Parser<'a, 'c, R>
+pub fn comment_scope<'a, 'c, P, R>(
+    verbosity: DebugVerbosity,
+    scope_name: &'a str,
+    parser: P,
+) -> impl Parser<'a, 'c, R>
 where
     'c: 'a,
     R: core::fmt::Debug,
     P: Parser<'a, 'c, R>,
 {
-    move |input, context| {
-        #[cfg(feature = "debug_parser_verbose")]
-        log::trace!("--> {}", scope_name);
+    move |input, context: &'c mut AmlContext| {
+        if verbosity <= context.debug_verbosity {
+            trace!("{:indent$}--> {}", "", scope_name, indent = context.scope_indent);
+            context.scope_indent += INDENT_PER_SCOPE;
+        }
 
         // Return if the parse fails, so we don't print the tail. Makes it easier to debug.
         let (new_input, context, result) = parser.parse(input, context)?;
 
-        #[cfg(feature = "debug_parser_verbose")]
-        log::trace!("<-- {}", scope_name);
+        if verbosity <= context.debug_verbosity {
+            context.scope_indent -= INDENT_PER_SCOPE;
+            trace!("{:indent$}<-- {}", "", scope_name, indent = context.scope_indent);
+        }
 
         Ok((new_input, context, result))
     }
@@ -469,11 +472,11 @@ pub(crate) macro try_with_context($context: expr, $expr: expr) {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::test_utils::*;
+    use crate::{test_utils::*, DebugVerbosity};
 
     #[test]
     fn test_take_n() {
-        let mut context = AmlContext::new();
+        let mut context = AmlContext::new(false, DebugVerbosity::None);
         check_err!(take_n(1).parse(&[], &mut context), AmlError::UnexpectedEndOfStream, &[]);
         check_err!(take_n(2).parse(&[0xf5], &mut context), AmlError::UnexpectedEndOfStream, &[0xf5]);
 
@@ -484,7 +487,7 @@ mod tests {
 
     #[test]
     fn test_take_ux() {
-        let mut context = AmlContext::new();
+        let mut context = AmlContext::new(false, DebugVerbosity::None);
         check_err!(take_u16().parse(&[0x34], &mut context), AmlError::UnexpectedEndOfStream, &[0x34]);
         check_ok!(take_u16().parse(&[0x34, 0x12], &mut context), 0x1234, &[]);
 

+ 2 - 2
aml/src/pci_routing.rs

@@ -117,8 +117,8 @@ impl PciRoutingTable {
                             });
                         }
                         AmlValue::String(ref name) => {
-                            let (link_object_name, _) =
-                                context.namespace.search(&AmlName::from_str(name)?, &prt_path)?;
+                            let link_object_name =
+                                context.namespace.search_for_level(&AmlName::from_str(name)?, &prt_path)?;
                             entries.push(PciRoute {
                                 device,
                                 function,

+ 4 - 4
aml/src/pkg_length.rs

@@ -91,10 +91,10 @@ where
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::{test_utils::*, AmlError};
+    use crate::{test_utils::*, AmlError, DebugVerbosity};
 
     fn test_correct_pkglength(stream: &[u8], expected_raw_length: u32, expected_leftover: &[u8]) {
-        let mut context = AmlContext::new();
+        let mut context = AmlContext::new(false, DebugVerbosity::None);
         check_ok!(
             pkg_length().parse(stream, &mut context),
             PkgLength::from_raw_length(stream, expected_raw_length),
@@ -104,7 +104,7 @@ mod tests {
 
     #[test]
     fn test_raw_pkg_length() {
-        let mut context = AmlContext::new();
+        let mut context = AmlContext::new(false, DebugVerbosity::None);
         check_ok!(raw_pkg_length().parse(&[0b01000101, 0x14], &mut context), 325, &[]);
         check_ok!(raw_pkg_length().parse(&[0b01000111, 0x14, 0x46], &mut context), 327, &[0x46]);
         check_ok!(raw_pkg_length().parse(&[0b10000111, 0x14, 0x46], &mut context), 287047, &[]);
@@ -112,7 +112,7 @@ mod tests {
 
     #[test]
     fn test_pkg_length() {
-        let mut context = AmlContext::new();
+        let mut context = AmlContext::new(false, DebugVerbosity::None);
         check_err!(pkg_length().parse(&[], &mut context), AmlError::UnexpectedEndOfStream, &[]);
         test_correct_pkglength(&[0x00], 0, &[]);
         test_correct_pkglength(&[0x05, 0xf5, 0x7f, 0x3e, 0x54, 0x03], 5, &[0xf5, 0x7f, 0x3e, 0x54, 0x03]);

+ 53 - 25
aml/src/term_object.rs

@@ -1,12 +1,11 @@
 use crate::{
     misc::{arg_obj, local_obj},
     name_object::{name_seg, name_string},
-    namespace::AmlName,
+    namespace::{AmlName, LevelType},
     opcode::{self, ext_opcode, opcode},
     parser::{
         choice,
         comment_scope,
-        comment_scope_verbose,
         make_parser_concrete,
         take,
         take_to_end_of_pkglength,
@@ -23,6 +22,7 @@ use crate::{
     value::{AmlValue, FieldFlags, MethodFlags, RegionSpace},
     AmlContext,
     AmlError,
+    DebugVerbosity,
 };
 use alloc::string::String;
 use core::str;
@@ -56,7 +56,8 @@ where
     /*
      * TermObj := NamespaceModifierObj | NamedObj | Type1Opcode | Type2Opcode
      */
-    comment_scope_verbose(
+    comment_scope(
+        DebugVerbosity::AllScopes,
         "TermObj",
         choice!(
             namespace_modifier().map(|()| Ok(None)),
@@ -90,7 +91,8 @@ where
      * XXX: DefMethod and DefMutex (at least) are not included in any rule in the AML grammar,
      * but are defined in the NamedObj section so we assume they're part of NamedObj
      */
-    comment_scope_verbose(
+    comment_scope(
+        DebugVerbosity::AllScopes,
         "NamedObj",
         choice!(def_op_region(), def_field(), def_method(), def_device(), def_processor(), def_mutex()),
     )
@@ -105,11 +107,12 @@ where
      */
     opcode(opcode::DEF_NAME_OP)
         .then(comment_scope(
+            DebugVerbosity::Scopes,
             "DefName",
             name_string().then(data_ref_object()).map_with_context(|(name, data_ref_object), context| {
                 try_with_context!(
                     context,
-                    context.namespace.add_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)
             }),
@@ -126,12 +129,24 @@ where
      */
     opcode(opcode::DEF_SCOPE_OP)
         .then(comment_scope(
+            DebugVerbosity::Scopes,
             "DefScope",
             pkg_length()
                 .then(name_string())
                 .map_with_context(|(length, name), context| {
                     let previous_scope = context.current_scope.clone();
                     context.current_scope = try_with_context!(context, name.resolve(&context.current_scope));
+
+                    context.comment(
+                        DebugVerbosity::Scopes,
+                        &(String::from("Scope name: ") + &context.current_scope.as_string()),
+                    );
+
+                    try_with_context!(
+                        context,
+                        context.namespace.add_level(context.current_scope.clone(), LevelType::Scope)
+                    );
+
                     (Ok((length, previous_scope)), context)
                 })
                 .feed(|(pkg_length, previous_scope)| {
@@ -168,6 +183,7 @@ where
      */
     ext_opcode(opcode::EXT_DEF_OP_REGION_OP)
         .then(comment_scope(
+            DebugVerbosity::Scopes,
             "DefOpRegion",
             name_string().then(take()).then(term_arg()).then(term_arg()).map_with_context(
                 |(((name, space), offset), length), context| {
@@ -196,7 +212,7 @@ where
 
                     try_with_context!(
                         context,
-                        context.namespace.add_at_resolved_path(
+                        context.namespace.add_value_at_resolved_path(
                             name,
                             &context.current_scope,
                             AmlValue::OpRegion { region, offset, length }
@@ -219,6 +235,7 @@ where
      */
     ext_opcode(opcode::EXT_DEF_FIELD_OP)
         .then(comment_scope(
+            DebugVerbosity::Scopes,
             "DefField",
             pkg_length().then(name_string()).then(take()).feed(|((list_length, region_name), flags)| {
                 move |mut input: &'a [u8], mut context: &'c mut AmlContext| -> ParseResult<'a, 'c, ()> {
@@ -287,7 +304,7 @@ where
     let named_field = name_seg().then(pkg_length()).map_with_context(move |(name_seg, length), context| {
         try_with_context!(
             context,
-            context.namespace.add_at_resolved_path(
+            context.namespace.add_value_at_resolved_path(
                 AmlName::from_name_seg(name_seg),
                 &context.current_scope,
                 AmlValue::Field {
@@ -317,6 +334,7 @@ where
      */
     opcode(opcode::DEF_METHOD_OP)
         .then(comment_scope(
+            DebugVerbosity::Scopes,
             "DefMethod",
             pkg_length()
                 .then(name_string())
@@ -327,7 +345,7 @@ where
                 .map_with_context(|(name, flags, code), context| {
                     try_with_context!(
                         context,
-                        context.namespace.add_at_resolved_path(
+                        context.namespace.add_value_at_resolved_path(
                             name,
                             &context.current_scope,
                             AmlValue::Method { flags: MethodFlags::new(flags), code: code.to_vec() },
@@ -348,21 +366,19 @@ where
      */
     ext_opcode(opcode::EXT_DEF_DEVICE_OP)
         .then(comment_scope(
+            DebugVerbosity::Scopes,
             "DefDevice",
             pkg_length()
                 .then(name_string())
                 .map_with_context(|(length, name), context| {
+                    let resolved_name = try_with_context!(context, name.clone().resolve(&context.current_scope));
                     try_with_context!(
                         context,
-                        context.namespace.add_at_resolved_path(
-                            name.clone(),
-                            &context.current_scope,
-                            AmlValue::Device
-                        )
+                        context.namespace.add_level(resolved_name.clone(), LevelType::Device)
                     );
 
                     let previous_scope = context.current_scope.clone();
-                    context.current_scope = try_with_context!(context, name.resolve(&context.current_scope));
+                    context.current_scope = resolved_name;
 
                     (Ok((length, previous_scope)), context)
                 })
@@ -387,6 +403,7 @@ where
      */
     ext_opcode(opcode::EXT_DEF_PROCESSOR_OP)
         .then(comment_scope(
+            DebugVerbosity::Scopes,
             "DefProcessor",
             pkg_length()
                 .then(name_string())
@@ -394,16 +411,24 @@ where
                 .then(take_u32())
                 .then(take())
                 .map_with_context(|((((pkg_length, name), proc_id), pblk_address), pblk_len), context| {
+                    /*
+                     * Legacy `Processor` objects contain data within themselves, and can also have sub-objects,
+                     * so we add both a level for the sub-objects, and a value for the data.
+                     */
+                    let resolved_name = try_with_context!(context, name.resolve(&context.current_scope));
                     try_with_context!(
                         context,
-                        context.namespace.add_at_resolved_path(
-                            name.clone(),
-                            &context.current_scope,
+                        context.namespace.add_level(resolved_name.clone(), LevelType::Processor)
+                    );
+                    try_with_context!(
+                        context,
+                        context.namespace.add_value(
+                            resolved_name.clone(),
                             AmlValue::Processor { id: proc_id, pblk_address, pblk_len }
                         )
                     );
                     let previous_scope = context.current_scope.clone();
-                    context.current_scope = try_with_context!(context, name.resolve(&context.current_scope));
+                    context.current_scope = resolved_name;
 
                     (Ok((previous_scope, pkg_length)), context)
                 })
@@ -429,11 +454,12 @@ where
      */
     ext_opcode(opcode::EXT_DEF_MUTEX_OP)
         .then(comment_scope(
+            DebugVerbosity::Scopes,
             "DefMutex",
             name_string().then(take()).map_with_context(|(name, sync_level), context| {
                 try_with_context!(
                     context,
-                    context.namespace.add_at_resolved_path(
+                    context.namespace.add_value_at_resolved_path(
                         name,
                         &context.current_scope,
                         AmlValue::Mutex { sync_level }
@@ -452,7 +478,8 @@ where
     /*
      * TermArg := Type2Opcode | DataObject | ArgObj | LocalObj
      */
-    comment_scope_verbose(
+    comment_scope(
+        DebugVerbosity::AllScopes,
         "TermArg",
         choice!(
             data_object(),
@@ -474,7 +501,7 @@ where
     /*
      * DataRefObject := DataObject | ObjectReference | DDBHandle
      */
-    comment_scope_verbose("DataRefObject", choice!(data_object()))
+    comment_scope(DebugVerbosity::AllScopes, "DataRefObject", choice!(data_object()))
 }
 
 pub fn data_object<'a, 'c>() -> impl Parser<'a, 'c, AmlValue>
@@ -488,7 +515,7 @@ where
      * accidently parsed as ComputationalDatas.
      */
     // TODO: this doesn't yet parse DefVarPackage
-    comment_scope_verbose("DataObject", choice!(def_package(), computational_data()))
+    comment_scope(DebugVerbosity::AllScopes, "DataObject", choice!(def_package(), computational_data()))
 }
 
 pub fn computational_data<'a, 'c>() -> impl Parser<'a, 'c, AmlValue>
@@ -546,7 +573,8 @@ where
         }
     };
 
-    comment_scope_verbose(
+    comment_scope(
+        DebugVerbosity::AllScopes,
         "ComputationalData",
         choice!(
             ext_opcode(opcode::EXT_REVISION_OP).map(|_| Ok(AmlValue::Integer(crate::AML_INTERPRETER_REVISION))),
@@ -559,11 +587,11 @@ where
 #[cfg(test)]
 mod test {
     use super::*;
-    use crate::test_utils::*;
+    use crate::{test_utils::*, AmlContext, DebugVerbosity};
 
     #[test]
     fn test_computational_data() {
-        let mut context = AmlContext::new();
+        let mut context = AmlContext::new(false, DebugVerbosity::None);
         check_ok!(
             computational_data().parse(&[0x00, 0x34, 0x12], &mut context),
             AmlValue::Integer(0),

+ 8 - 4
aml/src/type1.rs

@@ -1,9 +1,10 @@
 use crate::{
     opcode::{self, opcode},
-    parser::{choice, comment_scope, comment_scope_verbose, take_to_end_of_pkglength, ParseResult, Parser},
+    parser::{choice, comment_scope, take_to_end_of_pkglength, ParseResult, Parser},
     pkg_length::{pkg_length, PkgLength},
     term_object::{term_arg, term_list},
     AmlError,
+    DebugVerbosity,
 };
 
 /// Type 1 opcodes do not return a value and so can't be used in expressions.
@@ -16,7 +17,7 @@ where
      *                DefNotify | DefRelease | DefReset | DefReturn | DefSignal | DefSleep | DefStall |
      *                DefWhile
      */
-    comment_scope_verbose("Type1Opcode", choice!(def_if_else(), def_return()))
+    comment_scope(DebugVerbosity::AllScopes, "Type1Opcode", choice!(def_if_else(), def_return()))
 }
 
 fn def_if_else<'a, 'c>() -> impl Parser<'a, 'c, ()>
@@ -30,6 +31,7 @@ where
      */
     opcode(opcode::DEF_IF_ELSE_OP)
         .then(comment_scope(
+            DebugVerbosity::Scopes,
             "DefIfElse",
             pkg_length()
                 .then(term_arg())
@@ -39,7 +41,8 @@ where
                 })
                 .then(choice!(
                     opcode(opcode::DEF_ELSE_OP)
-                        .then(comment_scope_verbose(
+                        .then(comment_scope(
+                            DebugVerbosity::AllScopes,
                             "DefElse",
                             pkg_length().feed(|length| take_to_end_of_pkglength(length))
                         ))
@@ -75,7 +78,8 @@ where
      * ArgObject := TermArg => DataRefObject
      */
     opcode(opcode::DEF_RETURN_OP)
-        .then(comment_scope_verbose(
+        .then(comment_scope(
+            DebugVerbosity::Scopes,
             "DefReturn",
             term_arg().map(|return_arg| -> Result<(), AmlError> {
                 /*

+ 14 - 18
aml/src/type2.rs

@@ -1,20 +1,11 @@
 use crate::{
     name_object::{name_string, super_name, Target},
     opcode::{self, opcode},
-    parser::{
-        choice,
-        comment_scope,
-        comment_scope_verbose,
-        id,
-        take,
-        take_to_end_of_pkglength,
-        try_with_context,
-        Parser,
-    },
+    parser::{choice, comment_scope, id, take, take_to_end_of_pkglength, try_with_context, Parser},
     pkg_length::pkg_length,
     term_object::{data_ref_object, term_arg},
     value::AmlValue,
-    AmlError,
+    DebugVerbosity,
 };
 use alloc::vec::Vec;
 
@@ -34,7 +25,8 @@ where
      *                DefSubtract | DefTimer | DefToBCD | DefToBuffer | DefToDecimalString |
      *                DefToHexString | DefToInteger | DefToString | DefWait | DefXOr | MethodInvocation
      */
-    comment_scope_verbose(
+    comment_scope(
+        DebugVerbosity::AllScopes,
         "Type2Opcode",
         choice!(def_buffer(), def_l_equal(), def_package(), def_store(), method_invocation()),
     )
@@ -54,6 +46,7 @@ where
      */
     opcode(opcode::DEF_BUFFER_OP)
         .then(comment_scope(
+            DebugVerbosity::AllScopes,
             "DefBuffer",
             pkg_length().then(term_arg()).feed(|(pkg_length, buffer_size)| {
                 take_to_end_of_pkglength(pkg_length)
@@ -72,7 +65,8 @@ where
      * Operand := TermArg => Integer
      */
     opcode(opcode::DEF_L_EQUAL_OP)
-        .then(comment_scope_verbose(
+        .then(comment_scope(
+            DebugVerbosity::AllScopes,
             "DefLEqual",
             term_arg().then(term_arg()).map(|(left_arg, right_arg)| {
                 Ok(AmlValue::Boolean(left_arg.as_integer()? == right_arg.as_integer()?))
@@ -93,6 +87,7 @@ where
      */
     opcode(opcode::DEF_PACKAGE_OP)
         .then(comment_scope(
+            DebugVerbosity::AllScopes,
             "DefPackage",
             pkg_length().then(take()).feed(|(pkg_length, num_elements)| {
                 move |mut input, mut context| {
@@ -134,8 +129,9 @@ where
      * after the store (as opposed to the data we think we put into it), because some stores can
      * alter the data during the store.
      */
-    opcode(opcode::DEF_STORE_OP).then(comment_scope("DefStore", term_arg().then(super_name()))).map_with_context(
-        |((), (value, target)), context| {
+    opcode(opcode::DEF_STORE_OP)
+        .then(comment_scope(DebugVerbosity::Scopes, "DefStore", term_arg().then(super_name())))
+        .map_with_context(|((), (value, target)), context| {
             match target {
                 Target::Name(ref path) => {
                     let (_, handle) =
@@ -162,8 +158,7 @@ where
                     unimplemented!()
                 }
             }
-        },
-    )
+        })
 }
 
 fn method_invocation<'a, 'c>() -> impl Parser<'a, 'c, AmlValue>
@@ -179,7 +174,8 @@ where
      * so parsing them properly can be very difficult.
      * NOTE: We don't support the case of the definition appearing after the invocation.
      */
-    comment_scope_verbose(
+    comment_scope(
+        DebugVerbosity::Scopes,
         "MethodInvocation",
         name_string()
             .map_with_context(move |name, context| {

+ 0 - 3
aml/src/value.rs

@@ -99,7 +99,6 @@ pub enum AmlType {
     /// Handle to a definition block handle. Returned by the `Load` operator.
     DdbHandle,
     DebugObject,
-    Device,
     Event,
     FieldUnit,
     Integer,
@@ -122,7 +121,6 @@ pub enum AmlValue {
     String(String),
     OpRegion { region: RegionSpace, offset: u64, length: u64 },
     Field { region: AmlName, flags: FieldFlags, offset: u64, length: u64 },
-    Device,
     Method { flags: MethodFlags, code: Vec<u8> },
     Buffer { bytes: Vec<u8>, size: u64 },
     Processor { id: u8, pblk_address: u32, pblk_len: u8 },
@@ -139,7 +137,6 @@ impl AmlValue {
             AmlValue::String(_) => AmlType::String,
             AmlValue::OpRegion { .. } => AmlType::OpRegion,
             AmlValue::Field { .. } => AmlType::FieldUnit,
-            AmlValue::Device => AmlType::Device,
             AmlValue::Method { .. } => AmlType::Method,
             AmlValue::Buffer { .. } => AmlType::Buffer,
             AmlValue::Processor { .. } => AmlType::Processor,

+ 1 - 0
aml_tester/Cargo.toml

@@ -7,4 +7,5 @@ edition = "2018"
 [dependencies]
 clap = "2"
 termion = "1"
+log = "0.4"
 aml = { path = "../aml" }

+ 26 - 4
aml_tester/src/main.rs

@@ -9,18 +9,21 @@
  *      - For failing tests, print out a nice summary of the errors for each file
  */
 
-use aml::AmlContext;
+use aml::{AmlContext, DebugVerbosity};
 use clap::{App, Arg};
 use std::{
     ffi::OsStr,
     fs::{self, File},
-    io::Read,
+    io::{Read, Write},
     ops::Not,
     path::Path,
     process::Command,
 };
 
 fn main() -> std::io::Result<()> {
+    log::set_logger(&Logger).unwrap();
+    log::set_max_level(log::LevelFilter::Trace);
+
     let matches = App::new("aml_tester")
         .version("v0.1.0")
         .author("Isaac Woods")
@@ -29,8 +32,8 @@ fn main() -> std::io::Result<()> {
         .arg(Arg::with_name("no_compile").long("no-compile"))
         .get_matches();
 
-    println!("Running tests in directory: {:?}", matches.value_of("path"));
     let dir_path = Path::new(matches.value_of("path").unwrap());
+    println!("Running tests in directory: {:?}", dir_path);
 
     if !matches.is_present("no_compile") {
         compile_asl_files(dir_path)?;
@@ -46,22 +49,25 @@ fn main() -> std::io::Result<()> {
 
     let (passed, failed) = aml_files.fold((0, 0), |(passed, failed), file_entry| {
         print!("Testing AML file: {:?}... ", file_entry.path());
+        std::io::stdout().flush().unwrap();
 
         let mut file = File::open(file_entry.path()).unwrap();
         let mut contents = Vec::new();
         file.read_to_end(&mut contents).unwrap();
 
         const AML_TABLE_HEADER_LENGTH: usize = 36;
-        let mut context = AmlContext::new();
+        let mut context = AmlContext::new(false, DebugVerbosity::None);
 
         match context.parse_table(&contents[AML_TABLE_HEADER_LENGTH..]) {
             Ok(()) => {
                 println!("{}OK{}", termion::color::Fg(termion::color::Green), termion::style::Reset);
+                println!("Namespace: {:#?}", context.namespace);
                 (passed + 1, failed)
             }
 
             Err(err) => {
                 println!("{}Failed ({:?}){}", termion::color::Fg(termion::color::Red), err, termion::style::Reset);
+                println!("Namespace: {:#?}", context.namespace);
                 (passed, failed + 1)
             }
         }
@@ -115,3 +121,19 @@ fn compile_asl_files(dir_path: &Path) -> std::io::Result<()> {
 
     Ok(())
 }
+
+struct Logger;
+
+impl log::Log for Logger {
+    fn enabled(&self, _: &log::Metadata) -> bool {
+        true
+    }
+
+    fn log(&self, record: &log::Record) {
+        println!("[{}] {}", record.level(), record.args());
+    }
+
+    fn flush(&self) {
+        std::io::stdout().flush().unwrap();
+    }
+}

+ 0 - 2
rustfmt.toml

@@ -6,7 +6,5 @@ imports_layout = "HorizontalVertical"
 use_field_init_shorthand = true
 use_try_shorthand = true
 format_code_in_doc_comments = true
-wrap_comments = true
 max_width = 115
-comment_width = 115
 use_small_heuristics = "max"

+ 2 - 2
tests/scopes.asl

@@ -1,7 +1,7 @@
-DefinitionBlock("scopes.aml", "DSDT", 1, "RSOSDEV", "SCOPES", 1) {
+DefinitionBlock("scopes.aml", "DSDT", 1, "RSACPI", "SCOPES", 1) {
 	Scope(_SB) {
 		Name(X, 320)
-		Device(\PCI0) {
+		Device(PCI0) {
 			Name(Y, 15)
 			Scope(\) {
 				Name(Z, 413)