From 51f2affd636350586429e1acacdd190ec592d35f Mon Sep 17 00:00:00 2001 From: Ron Williams Date: Sun, 5 Feb 2023 19:08:31 -0800 Subject: [PATCH 1/3] Add DefExternal with value merging --- aml/src/lib.rs | 2 ++ aml/src/namespace.rs | 80 +++++++++++++++++++++++++++++++++--------- aml/src/opcode.rs | 25 +++++++++++++ aml/src/statement.rs | 26 ++++++++++---- aml/src/term_object.rs | 73 +++++++++++++++++++++++++++++++++++++- aml/src/test_utils.rs | 4 +++ aml/src/value.rs | 6 +++- 7 files changed, 191 insertions(+), 25 deletions(-) diff --git a/aml/src/lib.rs b/aml/src/lib.rs index 8dc82e5b..87e78c7a 100644 --- a/aml/src/lib.rs +++ b/aml/src/lib.rs @@ -285,6 +285,7 @@ impl AmlContext { LevelType::PowerResource => Ok(false), LevelType::ThermalZone => Ok(false), LevelType::MethodLocals => Ok(false), + LevelType::External => Ok(false), })?; Ok(()) @@ -707,6 +708,7 @@ pub enum AmlError { InvalidNameSeg, InvalidPkgLength, InvalidFieldFlags, + InvalidObjectType(u8), UnterminatedStringConstant, InvalidStringConstant, InvalidRegionSpace(u8), diff --git a/aml/src/namespace.rs b/aml/src/namespace.rs index e4162cda..a2294810 100644 --- a/aml/src/namespace.rs +++ b/aml/src/namespace.rs @@ -37,6 +37,8 @@ pub enum LevelType { /// 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, + /// A level that is declared as an External object + External, } #[derive(Clone, Debug)] @@ -96,19 +98,41 @@ impl Namespace { * 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 path == AmlName::root() { + return Ok(()); + } - /* - * 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) { + let (level, last_seg) = self.get_level_for_path_mut(&path)?; + + match level.children.get_mut(&last_seg) { + Some(child) if typ == LevelType::External || typ == LevelType::Scope => { + Ok(()) + } + Some(child) if child.typ == LevelType::External || child.typ == LevelType::Scope => { + child.typ = typ; + Ok(()) + } + Some(child) => { + Err(AmlError::NameCollision(path)) + } + None => { level.children.insert(last_seg, NamespaceLevel::new(typ)); + Ok(()) } } + } - Ok(()) + /// Add an External level to the namespace. The parent level may not exist, so recursively + /// visit parents first and add as needed. + /// Expects that add_level with not error when the level exists. + pub fn add_external_levels(&mut self, path: AmlName) -> Result<(), AmlError> { + assert!(path.is_absolute()); + if path == AmlName::root() { + return Ok(()); + } + + self.add_external_levels(path.parent()?)?; + self.add_level(path, LevelType::External) } pub fn remove_level(&mut self, path: AmlName) -> Result<(), AmlError> { @@ -134,14 +158,38 @@ impl Namespace { assert!(path.is_absolute()); let path = path.normalize()?; - let handle = self.next_handle; - self.next_handle.increment(); - self.object_map.insert(handle, value); - - 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)), + // Check if the name already exists in the namespace, and whether it is an External + let (level, last_seg) = self.get_level_for_path(&path)?; + match level.values.get(&last_seg) { + Some(old_handle) if matches!(value, AmlValue::External) => { + Ok(old_handle.clone()) + } + Some(old_handle) => { + match self.object_map.get(old_handle) { + Some(old_value) if matches!(old_value, AmlValue::External) => { + let handle = old_handle.clone(); + let _ = self.object_map.insert(handle, value); + Ok(handle) + } + Some(old_value) => { + Err(AmlError::NameCollision(path)) + } + _ => { + Err(AmlError::FatalError) + } + } + } + None => { + let handle = self.next_handle; + self.next_handle.increment(); + self.object_map.insert(handle, value); + + 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)), + } + } } } diff --git a/aml/src/opcode.rs b/aml/src/opcode.rs index 89b6bea6..adfc3e98 100644 --- a/aml/src/opcode.rs +++ b/aml/src/opcode.rs @@ -97,6 +97,31 @@ pub const ARG6_OP: u8 = 0x6e; pub const EXT_OPCODE_PREFIX: u8 = 0x5b; +/* + * Object Types (https://github.com/acpica/acpica/blob/master/source/common/dmextern.c) + * Used for DefExternal + */ +pub const OBJTYPE_UNKNOWN: u8 = 0x00; +pub const OBJTYPE_INTEGER: u8 = 0x01; +pub const OBJTYPE_STRING: u8 = 0x02; +pub const OBJTYPE_BUFFER: u8 = 0x03; +pub const OBJTYPE_PACKAGE: u8 = 0x04; +pub const OBJTYPE_FIELD_UNIT: u8 = 0x05; +pub const OBJTYPE_DEVICE: u8 = 0x06; +pub const OBJTYPE_EVENT: u8 = 0x07; +pub const OBJTYPE_METHOD: u8 = 0x08; +pub const OBJTYPE_MUTEX: u8 = 0x09; +pub const OBJTYPE_OP_REGION: u8 = 0x0a; +pub const OBJTYPE_POWER_RES: u8 = 0x0b; +pub const OBJTYPE_PROCESSOR: u8 = 0x0c; +pub const OBJTYPE_THERMAL_ZONE: u8 = 0x0d; +pub const OBJTYPE_BUFFER_FIELD: u8 = 0x0e; +pub const OBJTYPE_DDBHANDLE: u8 = 0x0f; +pub const OBJTYPE_DEBUG_OBJ: u8 = 0x10; +pub const OBJTYPE_FIELD_UNIT_2: u8 = 0x11; +pub const OBJTYPE_FIELD_UNIT_3: u8 = 0x12; +pub const OBJTYPE_FIELD_UNIT_4: u8 = 0x13; + pub(crate) fn opcode<'a, 'c>(opcode: u8) -> impl Parser<'a, 'c, ()> where 'c: 'a, diff --git a/aml/src/statement.rs b/aml/src/statement.rs index 815a080c..e8372cdc 100644 --- a/aml/src/statement.rs +++ b/aml/src/statement.rs @@ -14,7 +14,7 @@ use crate::{ Propagate, }, pkg_length::{pkg_length, PkgLength}, - term_object::{term_arg, term_list}, + term_object::{term_arg, term_list, externals_list}, AmlContext, AmlError, DebugVerbosity, @@ -153,13 +153,25 @@ where } )) .map_with_context(|((predicate, then_branch), else_branch), context| { - let branch = if predicate { then_branch } else { else_branch }; + /* + * Special case: External defs are sometimes wrapped in If(false) + */ + if !predicate && else_branch.len() == 0 { + match externals_list(PkgLength::from_raw_length(then_branch, then_branch.len() as u32).unwrap()) + .parse(then_branch, context) + { + Ok((_, context, result)) => (Ok(result), context), + Err((_, context, err)) => (Err(err), context), + } + } else { + let branch = if predicate { then_branch } else { else_branch }; - match term_list(PkgLength::from_raw_length(branch, branch.len() as u32).unwrap()) - .parse(branch, context) - { - Ok((_, context, result)) => (Ok(result), context), - Err((_, context, err)) => (Err(err), context), + match term_list(PkgLength::from_raw_length(branch, branch.len() as u32).unwrap()) + .parse(branch, context) + { + Ok((_, context, result)) => (Ok(result), context), + Err((_, context, err)) => (Err(err), context), + } } }), )) diff --git a/aml/src/term_object.rs b/aml/src/term_object.rs index 246c1827..9e58f3b6 100644 --- a/aml/src/term_object.rs +++ b/aml/src/term_object.rs @@ -606,6 +606,31 @@ where .discard_result() } +pub fn externals_list<'a, 'c>(list_length: PkgLength) -> impl Parser<'a, 'c, ()> +where 'c: 'a, +{ + /* + * Used for a special case where Externals are defined in an If(false). + * Only Externals are parsed in this situation. + * Any non-External ends the list. Skip the rest of this input. + * This does not appear to be defined in the spec. + * ExternalsList := Nothing | + */ + // TODO: why does this use still_parsing, instead of just taking the whole thing and parsing it til it's empty? + // Copied from term_list(), update here when fixing term_list() + move |mut input: &'a [u8], mut context: &'c mut AmlContext| { + while list_length.still_parsing(input) { + let (new_input, new_context, _) = choice!( + def_external(), + take_to_end_of_pkglength(list_length).discard_result()) + .parse(input, context)?; + input = new_input; + context = new_context; + } + Ok((input, context, ())) + } +} + pub fn def_external<'a, 'c>() -> impl Parser<'a, 'c, ()> where 'c: 'a, @@ -616,7 +641,53 @@ where * ArgumentCount := ByteData (0 to 7) */ opcode(opcode::DEF_EXTERNAL_OP) - .then(comment_scope(DebugVerbosity::Scopes, "DefExternal", name_string().then(take()).then(take()))) + .then(comment_scope( + DebugVerbosity::Scopes, + "DefExternal", + name_string() + .then(take()) + .map_with_context(|(name, object_type), context| { + let is_level = match object_type { + opcode::OBJTYPE_UNKNOWN => false, + opcode::OBJTYPE_INTEGER => false, + opcode::OBJTYPE_STRING => false, + opcode::OBJTYPE_BUFFER => false, + opcode::OBJTYPE_PACKAGE => false, + opcode::OBJTYPE_FIELD_UNIT => false, + opcode::OBJTYPE_DEVICE => true, + opcode::OBJTYPE_EVENT => false, + opcode::OBJTYPE_METHOD => true, + opcode::OBJTYPE_MUTEX => false, + opcode::OBJTYPE_OP_REGION => false, + opcode::OBJTYPE_POWER_RES => true, + opcode::OBJTYPE_PROCESSOR => true, + opcode::OBJTYPE_THERMAL_ZONE => true, + opcode::OBJTYPE_BUFFER_FIELD => false, + other_type => { + return (Err(Propagate::Err(AmlError::InvalidObjectType(other_type))), context); + } + }; + let resolved_name = try_with_context!(context, name.resolve(&context.current_scope)); + let parent_name = try_with_context!(context, name.parent()); + if is_level { + try_with_context!( + context, + context.namespace.add_external_levels(resolved_name.clone()) + ); + } else { + try_with_context!( + context, + context.namespace.add_external_levels(parent_name) + ); + } + try_with_context!( + context, + context.namespace.add_value(resolved_name.clone(), AmlValue::External) + ); + + (Ok(()), context) + }) + .then(take()))) .discard_result() } diff --git a/aml/src/test_utils.rs b/aml/src/test_utils.rs index 8cca96f7..72113f13 100644 --- a/aml/src/test_utils.rs +++ b/aml/src/test_utils.rs @@ -211,5 +211,9 @@ pub(crate) fn crudely_cmp_values(a: &AmlValue, b: &AmlValue) -> bool { AmlValue::ThermalZone => true, _ => false, }, + AmlValue::External => match b { + AmlValue::External => true, + _ => false, + } } } diff --git a/aml/src/value.rs b/aml/src/value.rs index 538e3647..de60328b 100644 --- a/aml/src/value.rs +++ b/aml/src/value.rs @@ -155,6 +155,7 @@ pub enum AmlType { RawDataBuffer, String, ThermalZone, + External, } #[derive(Clone)] @@ -221,6 +222,7 @@ pub enum AmlValue { resource_order: u16, }, ThermalZone, + External, } impl AmlValue { @@ -260,6 +262,7 @@ impl AmlValue { AmlValue::Package(_) => AmlType::Package, AmlValue::PowerResource { .. } => AmlType::PowerResource, AmlValue::ThermalZone => AmlType::ThermalZone, + AmlValue::External => AmlType::External, } } @@ -286,7 +289,7 @@ impl AmlValue { let bytes = bytes.lock(); let bytes = if bytes.len() > 8 { &bytes[0..8] } else { &bytes[..] }; - Ok(bytes.iter().rev().fold(0: u64, |mut i, &popped| { + Ok(bytes.iter().rev().fold(0u64, |mut i, &popped| { i <<= 8; i += popped as u64; i @@ -347,6 +350,7 @@ impl AmlValue { AmlType::PowerResource => AmlValue::String("[Power Resource]".to_string()), AmlType::RawDataBuffer => AmlValue::String("[Raw Data Buffer]".to_string()), AmlType::ThermalZone => AmlValue::String("[Thermal Zone]".to_string()), + AmlType::External => AmlValue::String("[External]".to_string()), } } From 0f8c4805aaf212a193cd8d60972bbbd6ff22af8f Mon Sep 17 00:00:00 2001 From: Ron Williams Date: Sun, 5 Feb 2023 19:49:22 -0800 Subject: [PATCH 2/3] Add External type unimplemented! to DefObjectType --- aml/src/expression.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/aml/src/expression.rs b/aml/src/expression.rs index 81c2070c..dea3e1da 100644 --- a/aml/src/expression.rs +++ b/aml/src/expression.rs @@ -564,6 +564,7 @@ where // TODO: what to do with this? AmlType::DdbHandle => 0, AmlType::ObjReference => todo!(), + AmlType::External => unimplemented!(), }; (Ok(AmlValue::Integer(typ)), context) From 2986487efb7aef31ef4b1f41d28ef5b438db8e11 Mon Sep 17 00:00:00 2001 From: Ron Williams Date: Sun, 5 Feb 2023 21:26:01 -0800 Subject: [PATCH 3/3] Add test, don't create value for External levels --- aml/src/namespace.rs | 27 +++++++++++++++++++++++++++ aml/src/term_object.rs | 13 ++++++------- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/aml/src/namespace.rs b/aml/src/namespace.rs index a2294810..b79f7778 100644 --- a/aml/src/namespace.rs +++ b/aml/src/namespace.rs @@ -717,6 +717,33 @@ mod tests { 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 Externals, check for type corrections (allowed) and collisions (disallowed). + */ + assert_eq!(namespace.add_external_levels(AmlName::from_str("\\L1.L2.L3").unwrap()), Ok(())); + assert!(namespace.add_value(AmlName::from_str("\\L1.L2.L3.L4").unwrap(), AmlValue::External).is_ok()); + // Allow attempted adding levels that are already created + assert_eq!(namespace.add_external_levels(AmlName::from_str("\\L1.L2").unwrap()), Ok(())); + // Created value is still present - levels were not changed + assert!(crudely_cmp_values(namespace.get_by_path(&AmlName::from_str("\\L1.L2.L3.L4").unwrap()).unwrap(), &AmlValue::External)); + // Scope and External do not cause a collision + assert_eq!(namespace.add_level(AmlName::from_str("\\L1.L2.L3").unwrap(), LevelType::Scope), Ok(())); + assert!(crudely_cmp_values(namespace.get_by_path(&AmlName::from_str("\\L1.L2.L3.L4").unwrap()).unwrap(), &AmlValue::External)); + // Change level.typ to Device + assert_eq!(namespace.add_level(AmlName::from_str("\\L1.L2.L3").unwrap(), LevelType::Device), Ok(())); + assert_eq!(namespace.add_level(AmlName::from_str("\\L1.L2.L3").unwrap(), LevelType::Scope), Ok(())); + assert_eq!(namespace.add_level(AmlName::from_str("\\L1.L2.L3").unwrap(), LevelType::External), Ok(())); + // Change Device to Processor is not allowed + assert!(namespace.add_level(AmlName::from_str("\\L1.L2.L3").unwrap(), LevelType::Processor).is_err()); + assert!(crudely_cmp_values(namespace.get_by_path(&AmlName::from_str("\\L1.L2.L3.L4").unwrap()).unwrap(), &AmlValue::External)); + // Change value from External to something more specific + assert!(namespace.add_value(AmlName::from_str("\\L1.L2.L3.L4").unwrap(), AmlValue::Integer(6)).is_ok()); + // External does not cause collision + assert!(namespace.add_value(AmlName::from_str("\\L1.L2.L3.L4").unwrap(), AmlValue::External).is_ok()); + assert!(crudely_cmp_values(namespace.get_by_path(&AmlName::from_str("\\L1.L2.L3.L4").unwrap()).unwrap(), &AmlValue::Integer(6))); + // Change value from Integer to Boolean causes a collision + assert!(namespace.add_value(AmlName::from_str("\\L1.L2.L3.L4").unwrap(), AmlValue::Boolean(true)).is_err()); + /* * Add some things to the scopes to query later. */ diff --git a/aml/src/term_object.rs b/aml/src/term_object.rs index 9e58f3b6..db171638 100644 --- a/aml/src/term_object.rs +++ b/aml/src/term_object.rs @@ -668,23 +668,22 @@ where } }; let resolved_name = try_with_context!(context, name.resolve(&context.current_scope)); - let parent_name = try_with_context!(context, name.parent()); if is_level { try_with_context!( context, - context.namespace.add_external_levels(resolved_name.clone()) + context.namespace.add_external_levels(resolved_name) ); } else { + let parent_name = try_with_context!(context, resolved_name.parent()); try_with_context!( context, context.namespace.add_external_levels(parent_name) ); + try_with_context!( + context, + context.namespace.add_value(resolved_name.clone(), AmlValue::External) + ); } - try_with_context!( - context, - context.namespace.add_value(resolved_name.clone(), AmlValue::External) - ); - (Ok(()), context) }) .then(take())))