clean up aml symbols

acpid/Cargo.toml

@@ -18,6 +18,5 @@ redox-log = "0.1.1"
 redox_syscall = "0.3"
 rustc-hash = "1.1.0"
 thiserror = "1"
-toml = "0.7"
 serde_json = "1.0.94"
 amlserde = { path = "../amlserde" }

acpid/src/acpi.rs

@@ -1,10 +1,9 @@
-use amlserde::{AmlHandleLookup, AmlSerde};
 use rustc_hash::FxHashMap;
 use std::convert::{TryFrom, TryInto};
+use std::fmt::Write;
 use std::ops::Deref;
 use std::sync::Arc;
 use std::{fmt, mem};
-use std::fmt::Write;
 
 use syscall::flag::PhysmapFlags;
 
@@ -14,8 +13,9 @@ use syscall::io::{Io, Pio};
 use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard};
 use thiserror::Error;
 
-use aml::{AmlContext, AmlError, AmlHandle, AmlName, AmlValue};
-use amlserde::pretty_name;
+use aml::{AmlContext, AmlError, AmlHandle, AmlName};
+use amlserde::aml_serde_name::aml_to_symbol;
+use amlserde::{AmlHandleLookup, AmlSerde};
 
 pub mod dmar;
 use self::dmar::Dmar;
@@ -54,8 +54,7 @@ impl SdtHeader {
         }
     }
     pub fn length(&self) -> usize {
-        self
-            .length
+        self.length
             .try_into()
             .expect("expected usize to be at least 32 bits")
     }
@@ -120,9 +119,7 @@ impl Deref for PhysmapGuard {
     type Target = [u8];
 
     fn deref(&self) -> &Self::Target {
-        unsafe {
-            std::slice::from_raw_parts(self.virt as *const u8, self.size)
-        }
+        unsafe { std::slice::from_raw_parts(self.virt as *const u8, self.size) }
     }
 }
 impl Drop for PhysmapGuard {
@@ -247,8 +244,107 @@ pub struct Ssdt(Sdt);
 pub struct AmlSymbols {
     aml_context: AmlContext,
     // k = name, v = description
-    symbols_cache: FxHashMap<String, String>,
-    pub symbols_str: String,
+    cache: FxHashMap<String, String>,
+    list: String,
+}
+
+impl AmlSymbols {
+    pub fn new() -> Self {
+        Self {
+            aml_context: AmlContext::new(Box::new(AmlPhysMemHandler), aml::DebugVerbosity::None),
+            cache: FxHashMap::default(),
+            list: "".to_string(),
+        }
+    }
+
+    pub fn mut_aml_context(&mut self) -> &mut AmlContext {
+        &mut self.aml_context
+    }
+
+    pub fn symbols_str(&self) -> &String {
+        &self.list
+    }
+
+    pub fn symbols_cache(&self) -> &FxHashMap<String, String> {
+        &self.cache
+    }
+
+    pub fn parse_table(&mut self, aml: &[u8]) -> Result<(), AmlError> {
+        self.aml_context.parse_table(aml)
+    }
+
+    pub fn lookup(&self, symbol: &str) -> Option<String> {
+        if let Some(description) = self.cache.get(symbol) {
+            log::trace!("Found symbol in cache, {}, {}", symbol, description);
+            return Some(description.to_owned());
+        }
+        None
+    }
+
+    pub fn build_cache(&mut self) {
+        let mut symbol_list: Vec<(AmlName, String, AmlHandle)> = Vec::with_capacity(5000);
+
+        if self
+            .aml_context
+            .namespace
+            .traverse(|level_aml_name, level| {
+                for (child_seg, handle) in level.values.iter() {
+                    if let Ok(aml_name) =
+                        AmlName::from_name_seg(child_seg.to_owned()).resolve(level_aml_name)
+                    {
+                        let name = aml_to_symbol(&aml_name);
+                        symbol_list.push((aml_name, name, handle.to_owned()));
+                    } else {
+                        log::error!(
+                            "AmlName resolve failed, {:?}:{:?}",
+                            level_aml_name,
+                            child_seg
+                        );
+                    }
+                }
+                Ok(true)
+            })
+            .is_err()
+        {
+            log::error!("Namespace traverse failed");
+            return;
+        }
+
+        let mut symbols_str = String::with_capacity(symbol_list.len() * 10);
+        let mut handle_lookup = AmlHandleLookup::new();
+
+        for (aml_name, name, handle) in &symbol_list {
+            let _ = writeln!(symbols_str, "{}", &name);
+            handle_lookup.insert(handle.to_owned(), aml_name.to_owned());
+        }
+
+        symbols_str.shrink_to_fit();
+
+        let mut symbol_cache: FxHashMap<String, String> = FxHashMap::default();
+
+        for (aml_name, name, handle) in &symbol_list {
+            // create an empty entry, in case something goes wrong with serialization
+            symbol_cache.insert(name.to_owned(), "".to_owned());
+            if let Some(ser_value) = AmlSerde::from_aml(
+                &mut self.aml_context,
+                &handle_lookup,
+                &aml_to_symbol(aml_name),
+                aml_name,
+                handle,
+            ) {
+                if let Ok(ser_string) = serde_json::to_string_pretty(&ser_value) {
+                    // replace the empty entry
+                    symbol_cache.insert(name.to_owned(), ser_string);
+                }
+            }
+        }
+
+        // Cache the new list
+        log::trace!("Updating symbols list");
+
+        self.list = symbols_str;
+        self.cache = symbol_cache;
+    }
 }
 
 pub struct AcpiContext {
@@ -286,14 +382,7 @@ impl AcpiContext {
             fadt: None,
 
             // Temporary values
-            aml_symbols: RwLock::new(AmlSymbols {
-                aml_context: AmlContext::new(
-                    Box::new(AmlPhysMemHandler),
-                    aml::DebugVerbosity::None,
-                ),
-                symbols_cache: FxHashMap::default(),
-                symbols_str: "".to_string(),
-            }),
+            aml_symbols: RwLock::new(AmlSymbols::new()),
 
             next_ctx: RwLock::new(0),
 
@@ -307,17 +396,16 @@ impl AcpiContext {
         Fadt::init(&mut this);
         //TODO (hangs on real hardware): Dmar::init(&this);
 
-        this.aml_symbols.write().aml_context = AcpiContext::build_aml_context(&this);
+        AcpiContext::build_aml_context(&this);
 
         this
     }
 
-    fn build_aml_context(acpi: &AcpiContext) -> AmlContext {
-        let mut aml_context =
-            AmlContext::new(Box::new(AmlPhysMemHandler), aml::DebugVerbosity::None);
+    fn build_aml_context(acpi: &AcpiContext) {
+        let mut aml_symbols = acpi.aml_symbols.write();
 
         if let Some(dsdt) = acpi.dsdt() {
-            match aml_context.parse_table(dsdt.aml()) {
+            match aml_symbols.parse_table(dsdt.aml()) {
                 Ok(_) => log::trace!("Parsed DSDT"),
                 Err(e) => {
                     log::error!("DSDT: {:?}", e);
@@ -328,14 +416,13 @@ impl AcpiContext {
         }
 
         for ssdt in acpi.ssdts() {
-            match aml_context.parse_table(ssdt.aml()) {
+            match aml_symbols.parse_table(ssdt.aml()) {
                 Ok(_) => log::trace!("Parsed SSDT"),
                 Err(e) => {
                     log::error!("SSDT: {:?}", e);
                 }
             }
         }
-        aml_context
     }
 
     pub fn dsdt(&self) -> Option<&Dsdt> {
@@ -402,18 +489,16 @@ impl AcpiContext {
 
     pub fn aml_lookup(&self, symbol: &str) -> Option<String> {
         if let Ok(aml_symbols) = self.aml_symbols() {
-            if let Some(description) = aml_symbols.symbols_cache.get(symbol) {
-                log::trace!("Found symbol in cache, {}, {}", symbol, description);
-                return Some(description.to_owned());
-            }
+            aml_symbols.lookup(symbol)
+        } else {
+            None
         }
-        None
     }
 
     pub fn aml_symbols(&self) -> Result<RwLockReadGuard<'_, AmlSymbols>, AmlError> {
         // return the cached value if it exists
         let symbols = self.aml_symbols.read();
-        if !symbols.symbols_cache.is_empty() {
+        if !symbols.symbols_cache().is_empty() {
             return Ok(symbols);
         }
         // free the read lock
@@ -422,57 +507,9 @@ impl AcpiContext {
         // List has not been initialized, we have to build it
         log::trace!("Creating symbols list");
 
-        let mut symbol_list: Vec<(AmlName, String, AmlHandle)> = Vec::with_capacity(5000);
-
         let mut aml_symbols = self.aml_symbols.write();
 
-        aml_symbols
-            .aml_context
-            .namespace
-            .traverse(|level_aml_name, level| {
-                for (child_seg, handle) in level.values.iter() {
-                    if let Ok(aml_name) =
-                        AmlName::from_name_seg(child_seg.to_owned()).resolve(level_aml_name)
-                    {
-                        let name = pretty_name(&aml_name);
-                        symbol_list.push((aml_name, name, handle.to_owned()));
-                    } else {
-                        log::error!(
-                            "AmlName resolve failed, {:?}:{:?}",
-                            level_aml_name,
-                            child_seg
-                        );
-                    }
-                }
-                Ok(true)
-            })?;
-
-        let mut symbols_str = String::with_capacity(symbol_list.len() * 10);
-        let mut handle_lookup = AmlHandleLookup::new();
-
-        for (_aml_name, name, handle) in &symbol_list {
-            let _ = writeln!(symbols_str, "{}", &name);
-            handle_lookup.insert(handle.to_owned(), name.to_owned());
-        }
-
-        symbols_str.shrink_to_fit();
-
-        let mut symbol_cache: FxHashMap<String, String> = FxHashMap::default();
-
-        for (_aml_name, name, handle) in &symbol_list {
-            if let Some(ser_value) = AmlSerde::from_aml(&aml_symbols.aml_context, &handle_lookup, handle) {
-                if let Ok(ser_string) = serde_json::to_string_pretty(&ser_value) {
-                    symbol_cache.insert(name.to_owned(), ser_string);
-                }
-            }
-        }
-
-        
-        // Cache the new list
-        log::trace!("Updating symbols list");
-
-        aml_symbols.symbols_str = symbols_str;
-        aml_symbols.symbols_cache = symbol_cache;
+        aml_symbols.build_cache();
 
         // return the cached value
         Ok(RwLockWriteGuard::downgrade(aml_symbols))
@@ -481,8 +518,8 @@ impl AcpiContext {
     /// Discard any cached symbols list. To be called if the AML namespace changes.
     pub fn aml_symbols_reset(&self) {
         let mut aml_symbols = self.aml_symbols.write();
-        aml_symbols.symbols_cache = FxHashMap::default();
-        aml_symbols.symbols_str = "".to_string();
+        aml_symbols.cache = FxHashMap::default();
+        aml_symbols.list = "".to_string();
     }
 
     /// Set Power State

acpid/src/scheme.rs

@@ -45,7 +45,7 @@ impl HandleKind<'_> {
             Self::TopLevel => TOPLEVEL_CONTENTS.len(),
             Self::Tables => acpi_ctx.tables().len().checked_mul(TABLE_DENTRY_LENGTH).unwrap_or(usize::max_value()),
             Self::Table(signature) => acpi_ctx.sdt_from_signature(signature).ok_or(Error::new(EBADFD))?.length(),
-            Self::Symbols(aml_symbols) => aml_symbols.symbols_str.len(),
+            Self::Symbols(aml_symbols) => aml_symbols.symbols_str().len(),
             Self::Symbol(description) => description.len(),
         })
     }
@@ -287,7 +287,7 @@ impl SchemeMut for AcpiScheme<'_> {
             }
 
             HandleKind::Symbols(aml_symbols) => {
-                let symbols = &aml_symbols.symbols_str;
+                let symbols = aml_symbols.symbols_str();
                 let offset = std::cmp::min(symbols.len(), handle.offset);
                 let src_buf = &symbols.as_bytes()[offset..];
 

amlserde/src/lib.rs

@@ -1,7 +1,5 @@
-use aml::{
-    value::{FieldAccessType, FieldUpdateRule, RegionSpace},
-    AmlContext, AmlHandle, AmlName, AmlValue,
-};
+use aml::value::{FieldAccessType, FieldUpdateRule, RegionSpace};
+use aml::{AmlContext, AmlHandle, AmlName, AmlValue};
 use rustc_hash::FxHashMap;
 use serde::{Deserialize, Serialize};
 
@@ -75,9 +73,9 @@ pub enum AmlSerdeRegionSpace {
 
 #[derive(Debug, Serialize, Deserialize)]
 pub struct AmlSerdeFieldFlags {
-    access_type: AmlSerdeFieldAccessType,
-    lock_rule: bool,
-    update_rule: AmlSerdeFieldUpdateRule,
+    pub access_type: AmlSerdeFieldAccessType,
+    pub lock_rule: bool,
+    pub update_rule: AmlSerdeFieldUpdateRule,
 }
 
 #[derive(Debug, Serialize, Deserialize)]
@@ -106,16 +104,12 @@ impl AmlSerde {
     }
 
     pub fn from_aml(
-        aml_context: &AmlContext,
+        aml_context: &mut AmlContext,
         aml_lookup: &AmlHandleLookup,
+        name: &String,
+        aml_name: &AmlName,
         handle: &AmlHandle,
     ) -> Option<Self> {
-        let name = if let Some((name, _handle)) = aml_lookup.get(handle) {
-            name.to_owned()
-        } else {
-            return None;
-        };
-
         let aml_value = if let Ok(aml_value) = aml_context.namespace.get(handle.clone()) {
             aml_value
         } else {
@@ -128,7 +122,10 @@ impl AmlSerde {
             return None;
         };
 
-        Some(AmlSerde { name, value })
+        Some(AmlSerde {
+            name: aml_name.to_string(),
+            value,
+        })
     }
 }
 
@@ -167,7 +164,7 @@ impl AmlSerdeValue {
                 offset: offset.to_owned(),
                 length: length.to_owned(),
                 parent_device: if let Some(parent) = parent_device {
-                    Some(pretty_name(parent))
+                    Some(parent.to_string())
                 } else {
                     None
                 },
@@ -180,7 +177,7 @@ impl AmlSerdeValue {
                 length,
             } => AmlSerdeValue::Field {
                 region: if let Some((region, _handle)) = aml_lookup.get(region) {
-                    region.to_owned()
+                    region.to_string()
                 } else {
                     return None;
                 },
@@ -254,8 +251,42 @@ impl AmlSerdeValue {
     }
 }
 
+pub mod aml_serde_name {
+    use aml::AmlName;
+
+    /// Add a leading backslash to make the name a valid
+    /// namespace reference
+    pub fn to_aml_format(pretty_name: &String) -> String {
+        format!("\\{}", pretty_name)
+    }
+
+    /// convert a string from AML namespace style to
+    /// acpi symbol style
+    pub fn to_symbol(aml_style_name: &String) -> String {
+        let mut name = aml_style_name.to_owned();
+
+        // remove leading slash
+        name = name.trim_start_matches("\\").to_owned();
+        // remove unnecessary underscores
+        while let Some(index) = name.find("_.") {
+            name.remove(index);
+        }
+        while name.len() > 0 && &name[name.len() - 1..] == "_" {
+            name.pop();
+        }
+        name.shrink_to_fit();
+        name
+    }
+
+    /// Convert to string and remove
+    /// trailing underscores from each name segment
+    pub fn aml_to_symbol(aml_name: &AmlName) -> String {
+        to_symbol(&aml_name.as_string())
+    }
+}
+
 pub struct AmlHandleLookup {
-    map: FxHashMap<String, (String, AmlHandle)>,
+    map: FxHashMap<String, (AmlName, AmlHandle)>,
 }
 
 impl AmlHandleLookup {
@@ -269,27 +300,12 @@ impl AmlHandleLookup {
         format!("{:?}", handle)
     }
 
-    pub fn insert(&mut self, handle: AmlHandle, name: String) {
-        self.map.insert(self.handle_to_key(&handle), (name, handle));
+    pub fn insert(&mut self, handle: AmlHandle, aml_name: AmlName) {
+        self.map
+            .insert(self.handle_to_key(&handle), (aml_name, handle));
     }
 
-    pub fn get(&self, handle: &AmlHandle) -> Option<&(String, AmlHandle)> {
+    pub fn get(&self, handle: &AmlHandle) -> Option<&(AmlName, AmlHandle)> {
         self.map.get(&self.handle_to_key(handle))
     }
 }
-
-/// Remove trailing underscores from each name segment
-pub fn pretty_name(aml_name: &AmlName) -> String {
-    let mut name = aml_name.as_string();
-    // remove leading slash
-    name = name.trim_start_matches("\\").to_owned();
-    // remove unnecessary underscores
-    while let Some(index) = name.find("_.") {
-        name.remove(index);
-    }
-    while name.len() > 0 && &name[name.len() - 1..] == "_" {
-        name.pop();
-    }
-    name.shrink_to_fit();
-    name
-}