From 7bb865f349b592366a15387e69bc1d18c6441149 Mon Sep 17 00:00:00 2001 From: Isaac Woods Date: Tue, 5 Mar 2024 20:58:39 +0000 Subject: [PATCH 1/7] Make `read_field` and friends take a mutable context This is necessary because these methods will need to invoke methods in the future. Co-authored-by: Ron Williams --- aml/src/expression.rs | 10 ++++++++-- aml/src/lib.rs | 14 ++++++++++---- aml/src/statement.rs | 4 ++-- aml/src/value.rs | 10 +++++----- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/aml/src/expression.rs b/aml/src/expression.rs index 3a5f5a0f..1f2a3849 100644 --- a/aml/src/expression.rs +++ b/aml/src/expression.rs @@ -331,7 +331,10 @@ where DebugVerbosity::AllScopes, "DefIncrement", super_name().map_with_context(|addend, context| { - let value = try_with_context!(context, context.read_target(&addend)); + let value = { + let value = try_with_context!(context, context.read_target(&addend)); + value.clone() + }; let value = try_with_context!(context, value.as_integer(context)); let new_value = AmlValue::Integer(value + 1); try_with_context!(context, context.store(addend, new_value.clone())); @@ -353,7 +356,10 @@ where DebugVerbosity::AllScopes, "DefDecrement", super_name().map_with_context(|minuend, context| { - let value = try_with_context!(context, context.read_target(&minuend)); + let value = { + let value = try_with_context!(context, context.read_target(&minuend)); + value.clone() + }; let value = try_with_context!(context, value.as_integer(context)); let new_value = AmlValue::Integer(value - 1); try_with_context!(context, context.store(minuend, new_value.clone())); diff --git a/aml/src/lib.rs b/aml/src/lib.rs index d5bd7915..f2b627ee 100644 --- a/aml/src/lib.rs +++ b/aml/src/lib.rs @@ -389,14 +389,19 @@ impl AmlContext { /// Read from an operation-region, performing only standard-sized reads (supported powers-of-2 only. If a field /// is not one of these sizes, it may need to be masked, or multiple reads may need to be performed). - pub(crate) fn read_region(&self, region_handle: AmlHandle, offset: u64, length: u64) -> Result { + pub(crate) fn read_region( + &mut self, + region_handle: AmlHandle, + offset: u64, + length: u64, + ) -> Result { use bit_field::BitField; use core::convert::TryInto; use value::RegionSpace; let (region_space, region_base, _region_length, parent_device) = { if let AmlValue::OpRegion { region, offset, length, parent_device } = - self.namespace.get(region_handle)? + self.namespace.get(region_handle)?.clone() { (region, offset, length, parent_device) } else { @@ -488,7 +493,7 @@ impl AmlContext { let (region_space, region_base, _region_length, parent_device) = { if let AmlValue::OpRegion { region, offset, length, parent_device } = - self.namespace.get(region_handle)? + self.namespace.get(region_handle)?.clone() { (region, offset, length, parent_device) } else { @@ -605,7 +610,8 @@ impl AmlContext { .add_value( AmlName::from_str("\\_OSI").unwrap(), AmlValue::native_method(1, false, 0, |context| { - Ok(match context.current_arg(0)?.as_string(context)?.as_str() { + let value = context.current_arg(0)?.clone(); + Ok(match value.as_string(context)?.as_str() { "Windows 2000" => true, // 2000 "Windows 2001" => true, // XP "Windows 2001 SP1" => true, // XP SP1 diff --git a/aml/src/statement.rs b/aml/src/statement.rs index d65a6df6..46f8f9c3 100644 --- a/aml/src/statement.rs +++ b/aml/src/statement.rs @@ -224,7 +224,7 @@ where DebugVerbosity::Scopes, "DefSleep", term_arg().map_with_context(|milliseconds, context| { - let milliseconds = try_with_context!(context, milliseconds.as_integer(&context)); + let milliseconds = try_with_context!(context, milliseconds.as_integer(context)); context.handler.sleep(milliseconds); (Ok(()), context) }), @@ -245,7 +245,7 @@ where DebugVerbosity::Scopes, "DefStall", term_arg().map_with_context(|microseconds, context| { - let microseconds = try_with_context!(context, microseconds.as_integer(&context)); + let microseconds = try_with_context!(context, microseconds.as_integer(context)); context.handler.stall(microseconds); (Ok(()), context) }), diff --git a/aml/src/value.rs b/aml/src/value.rs index 99bc3bea..ae735ac2 100644 --- a/aml/src/value.rs +++ b/aml/src/value.rs @@ -288,7 +288,7 @@ impl AmlValue { } } - pub fn as_integer(&self, context: &AmlContext) -> Result { + pub fn as_integer(&self, context: &mut AmlContext) -> Result { match self { AmlValue::Integer(value) => Ok(*value), AmlValue::Boolean(value) => Ok(if *value { u64::max_value() } else { 0 }), @@ -320,7 +320,7 @@ impl AmlValue { } } - pub fn as_buffer(&self, context: &AmlContext) -> Result>>, AmlError> { + pub fn as_buffer(&self, context: &mut AmlContext) -> Result>>, AmlError> { match self { AmlValue::Buffer(ref bytes) => Ok(bytes.clone()), // TODO: implement conversion of String and Integer to Buffer @@ -330,7 +330,7 @@ impl AmlValue { } } - pub fn as_string(&self, context: &AmlContext) -> Result { + pub fn as_string(&self, context: &mut AmlContext) -> Result { match self { AmlValue::String(ref string) => Ok(string.clone()), // TODO: implement conversion of Buffer to String @@ -404,7 +404,7 @@ impl AmlValue { /// `Integer` from: `Buffer`, `BufferField`, `DdbHandle`, `FieldUnit`, `String`, `Debug` /// `Package` from: `Debug` /// `String` from: `Integer`, `Buffer`, `Debug` - pub fn as_type(&self, desired_type: AmlType, context: &AmlContext) -> Result { + pub fn as_type(&self, desired_type: AmlType, context: &mut AmlContext) -> Result { // If the value is already of the correct type, just return it as is if self.type_of() == desired_type { return Ok(self.clone()); @@ -423,7 +423,7 @@ impl AmlValue { /// Reads from a field of an opregion, returning either a `AmlValue::Integer` or an `AmlValue::Buffer`, /// depending on the size of the field. - pub fn read_field(&self, context: &AmlContext) -> Result { + pub fn read_field(&self, context: &mut AmlContext) -> Result { if let AmlValue::Field { region, flags, offset, length } = self { let _maximum_access_size = { if let AmlValue::OpRegion { region, .. } = context.namespace.get(*region)? { From 67753c5da63a61a9d6b3082110daa0107b569862 Mon Sep 17 00:00:00 2001 From: Isaac Woods Date: Tue, 5 Mar 2024 20:59:58 +0000 Subject: [PATCH 2/7] Invoke `_SEG`, `_BBN`, and `_ADR` as methods for PCI region accesses Co-authored-by: Ron Williams --- aml/src/lib.rs | 62 ++++++++++++++++++++++++-------------------------- 1 file changed, 30 insertions(+), 32 deletions(-) diff --git a/aml/src/lib.rs b/aml/src/lib.rs index f2b627ee..6c9e9e0a 100644 --- a/aml/src/lib.rs +++ b/aml/src/lib.rs @@ -438,29 +438,28 @@ impl AmlContext { * (e.g. systems with a single segment group and a single root, respectively). */ let parent_device = parent_device.as_ref().unwrap(); - let seg = match self.namespace.search(&AmlName::from_str("_SEG").unwrap(), parent_device) { - Ok((_, handle)) => self - .namespace - .get(handle)? - .as_integer(self)? - .try_into() - .map_err(|_| AmlError::FieldInvalidAddress)?, + let seg = match self.invoke_method( + &AmlName::from_str("_SEG").unwrap().resolve(parent_device).unwrap(), + Args::EMPTY, + ) { + Ok(seg) => seg.as_integer(self)?.try_into().map_err(|_| AmlError::FieldInvalidAddress)?, Err(AmlError::ValueDoesNotExist(_)) => 0, Err(err) => return Err(err), }; - let bbn = match self.namespace.search(&AmlName::from_str("_BBN").unwrap(), parent_device) { - Ok((_, handle)) => self - .namespace - .get(handle)? - .as_integer(self)? - .try_into() - .map_err(|_| AmlError::FieldInvalidAddress)?, + let bbn = match self.invoke_method( + &AmlName::from_str("_BBN").unwrap().resolve(parent_device).unwrap(), + Args::EMPTY, + ) { + Ok(bbn) => bbn.as_integer(self)?.try_into().map_err(|_| AmlError::FieldInvalidAddress)?, Err(AmlError::ValueDoesNotExist(_)) => 0, Err(err) => return Err(err), }; let adr = { - let (_, handle) = self.namespace.search(&AmlName::from_str("_ADR").unwrap(), parent_device)?; - self.namespace.get(handle)?.as_integer(self)? + let adr = self.invoke_method( + &AmlName::from_str("_ADR").unwrap().resolve(parent_device).unwrap(), + Args::EMPTY, + )?; + adr.as_integer(self)? }; let device = adr.get_bits(16..24) as u8; @@ -530,29 +529,28 @@ impl AmlContext { * (e.g. systems with a single segment group and a single root, respectively). */ let parent_device = parent_device.as_ref().unwrap(); - let seg = match self.namespace.search(&AmlName::from_str("_SEG").unwrap(), parent_device) { - Ok((_, handle)) => self - .namespace - .get(handle)? - .as_integer(self)? - .try_into() - .map_err(|_| AmlError::FieldInvalidAddress)?, + let seg = match self.invoke_method( + &AmlName::from_str("_SEG").unwrap().resolve(parent_device).unwrap(), + Args::EMPTY, + ) { + Ok(seg) => seg.as_integer(self)?.try_into().map_err(|_| AmlError::FieldInvalidAddress)?, Err(AmlError::ValueDoesNotExist(_)) => 0, Err(err) => return Err(err), }; - let bbn = match self.namespace.search(&AmlName::from_str("_BBN").unwrap(), parent_device) { - Ok((_, handle)) => self - .namespace - .get(handle)? - .as_integer(self)? - .try_into() - .map_err(|_| AmlError::FieldInvalidAddress)?, + let bbn = match self.invoke_method( + &AmlName::from_str("_BBN").unwrap().resolve(parent_device).unwrap(), + Args::EMPTY, + ) { + Ok(bbn) => bbn.as_integer(self)?.try_into().map_err(|_| AmlError::FieldInvalidAddress)?, Err(AmlError::ValueDoesNotExist(_)) => 0, Err(err) => return Err(err), }; let adr = { - let (_, handle) = self.namespace.search(&AmlName::from_str("_ADR").unwrap(), parent_device)?; - self.namespace.get(handle)?.as_integer(self)? + let adr = self.invoke_method( + &AmlName::from_str("_ADR").unwrap().resolve(parent_device).unwrap(), + Args::EMPTY, + )?; + adr.as_integer(self)? }; let device = adr.get_bits(16..24) as u8; From 986a1a6fa93b0f82d75580bb22dc4e07c7b2a395 Mon Sep 17 00:00:00 2001 From: Isaac Woods Date: Tue, 5 Mar 2024 23:23:06 +0000 Subject: [PATCH 3/7] Move the `aml` crate to the 2021 Edition Easy change while we're here --- aml/Cargo.toml | 2 +- aml/src/expression.rs | 2 +- aml/src/parser.rs | 2 +- aml/src/pci_routing.rs | 1 - aml/src/value.rs | 2 -- 5 files changed, 3 insertions(+), 6 deletions(-) diff --git a/aml/Cargo.toml b/aml/Cargo.toml index 0d1673f0..2b79b3dc 100644 --- a/aml/Cargo.toml +++ b/aml/Cargo.toml @@ -7,7 +7,7 @@ description = "Library for parsing AML" categories = ["hardware-support", "no-std"] readme = "../README.md" license = "MIT/Apache-2.0" -edition = "2018" +edition = "2021" [dependencies] log = "0.4" diff --git a/aml/src/expression.rs b/aml/src/expression.rs index 1f2a3849..05bb1c96 100644 --- a/aml/src/expression.rs +++ b/aml/src/expression.rs @@ -15,7 +15,7 @@ use alloc::{ vec, vec::Vec, }; -use core::{cmp::Ordering, convert::TryInto, mem, ops::Deref}; +use core::{cmp::Ordering, mem, ops::Deref}; pub fn expression_opcode<'a, 'c>() -> impl Parser<'a, 'c, AmlValue> where diff --git a/aml/src/parser.rs b/aml/src/parser.rs index 0a130f38..1aa5842e 100644 --- a/aml/src/parser.rs +++ b/aml/src/parser.rs @@ -1,6 +1,6 @@ use crate::{pkg_length::PkgLength, AmlContext, AmlError, AmlValue, DebugVerbosity}; use alloc::vec::Vec; -use core::{convert::TryInto, marker::PhantomData}; +use core::marker::PhantomData; use log::trace; /// This is the number of spaces added to indent a scope when printing parser debug messages. diff --git a/aml/src/pci_routing.rs b/aml/src/pci_routing.rs index 619384df..247ea94a 100644 --- a/aml/src/pci_routing.rs +++ b/aml/src/pci_routing.rs @@ -9,7 +9,6 @@ use crate::{ }; use alloc::vec::Vec; use bit_field::BitField; -use core::convert::TryInto; pub use crate::resource::IrqDescriptor; diff --git a/aml/src/value.rs b/aml/src/value.rs index ae735ac2..c4a36331 100644 --- a/aml/src/value.rs +++ b/aml/src/value.rs @@ -642,8 +642,6 @@ impl Args { pub const EMPTY: Self = Self([None, None, None, None, None, None, None]); pub fn from_list(list: Vec) -> Result { - use core::convert::TryInto; - if list.len() > 7 { return Err(AmlError::TooManyArgs); } From 96e3efeef1cd1cbc1de9b296fda6470bd647b479 Mon Sep 17 00:00:00 2001 From: Isaac Woods Date: Tue, 5 Mar 2024 23:24:38 +0000 Subject: [PATCH 4/7] Create `OpRegion` type to abstract away field manipulation Lots of the field reading/writing behaviour isn't quite correct yet, but this was getting unwieldly and spread about multiple places. Creating this abstraction will hopefully help in future. --- aml/src/lib.rs | 184 +--------------------------- aml/src/opregion.rs | 270 +++++++++++++++++++++++++++++++++++++++++ aml/src/pkg_length.rs | 2 +- aml/src/term_object.rs | 5 +- aml/src/value.rs | 120 +++--------------- 5 files changed, 290 insertions(+), 291 deletions(-) create mode 100644 aml/src/opregion.rs diff --git a/aml/src/lib.rs b/aml/src/lib.rs index 6c9e9e0a..360be8d9 100644 --- a/aml/src/lib.rs +++ b/aml/src/lib.rs @@ -50,6 +50,7 @@ pub(crate) mod misc; pub(crate) mod name_object; pub(crate) mod namespace; pub(crate) mod opcode; +pub mod opregion; pub(crate) mod parser; pub mod pci_routing; pub(crate) mod pkg_length; @@ -387,189 +388,6 @@ impl AmlContext { } } - /// Read from an operation-region, performing only standard-sized reads (supported powers-of-2 only. If a field - /// is not one of these sizes, it may need to be masked, or multiple reads may need to be performed). - pub(crate) fn read_region( - &mut self, - region_handle: AmlHandle, - offset: u64, - length: u64, - ) -> Result { - use bit_field::BitField; - use core::convert::TryInto; - use value::RegionSpace; - - let (region_space, region_base, _region_length, parent_device) = { - if let AmlValue::OpRegion { region, offset, length, parent_device } = - self.namespace.get(region_handle)?.clone() - { - (region, offset, length, parent_device) - } else { - return Err(AmlError::FieldRegionIsNotOpRegion); - } - }; - - match region_space { - RegionSpace::SystemMemory => { - let address = (region_base + offset).try_into().map_err(|_| AmlError::FieldInvalidAddress)?; - match length { - 8 => Ok(self.handler.read_u8(address) as u64), - 16 => Ok(self.handler.read_u16(address) as u64), - 32 => Ok(self.handler.read_u32(address) as u64), - 64 => Ok(self.handler.read_u64(address)), - _ => Err(AmlError::FieldInvalidAccessSize), - } - } - - RegionSpace::SystemIo => { - let port = (region_base + offset).try_into().map_err(|_| AmlError::FieldInvalidAddress)?; - match length { - 8 => Ok(self.handler.read_io_u8(port) as u64), - 16 => Ok(self.handler.read_io_u16(port) as u64), - 32 => Ok(self.handler.read_io_u32(port) as u64), - _ => Err(AmlError::FieldInvalidAccessSize), - } - } - - RegionSpace::PciConfig => { - /* - * First, we need to get some extra information out of objects in the parent object. Both - * `_SEG` and `_BBN` seem optional, with defaults that line up with legacy PCI implementations - * (e.g. systems with a single segment group and a single root, respectively). - */ - let parent_device = parent_device.as_ref().unwrap(); - let seg = match self.invoke_method( - &AmlName::from_str("_SEG").unwrap().resolve(parent_device).unwrap(), - Args::EMPTY, - ) { - Ok(seg) => seg.as_integer(self)?.try_into().map_err(|_| AmlError::FieldInvalidAddress)?, - Err(AmlError::ValueDoesNotExist(_)) => 0, - Err(err) => return Err(err), - }; - let bbn = match self.invoke_method( - &AmlName::from_str("_BBN").unwrap().resolve(parent_device).unwrap(), - Args::EMPTY, - ) { - Ok(bbn) => bbn.as_integer(self)?.try_into().map_err(|_| AmlError::FieldInvalidAddress)?, - Err(AmlError::ValueDoesNotExist(_)) => 0, - Err(err) => return Err(err), - }; - let adr = { - let adr = self.invoke_method( - &AmlName::from_str("_ADR").unwrap().resolve(parent_device).unwrap(), - Args::EMPTY, - )?; - adr.as_integer(self)? - }; - - let device = adr.get_bits(16..24) as u8; - let function = adr.get_bits(0..8) as u8; - let offset = (region_base + offset).try_into().map_err(|_| AmlError::FieldInvalidAddress)?; - - match length { - 8 => Ok(self.handler.read_pci_u8(seg, bbn, device, function, offset) as u64), - 16 => Ok(self.handler.read_pci_u16(seg, bbn, device, function, offset) as u64), - 32 => Ok(self.handler.read_pci_u32(seg, bbn, device, function, offset) as u64), - _ => Err(AmlError::FieldInvalidAccessSize), - } - } - - // TODO - _ => unimplemented!(), - } - } - - pub(crate) fn write_region( - &mut self, - region_handle: AmlHandle, - offset: u64, - length: u64, - value: u64, - ) -> Result<(), AmlError> { - use bit_field::BitField; - use core::convert::TryInto; - use value::RegionSpace; - - let (region_space, region_base, _region_length, parent_device) = { - if let AmlValue::OpRegion { region, offset, length, parent_device } = - self.namespace.get(region_handle)?.clone() - { - (region, offset, length, parent_device) - } else { - return Err(AmlError::FieldRegionIsNotOpRegion); - } - }; - - match region_space { - RegionSpace::SystemMemory => { - let address = (region_base + offset).try_into().map_err(|_| AmlError::FieldInvalidAddress)?; - match length { - 8 => Ok(self.handler.write_u8(address, value as u8)), - 16 => Ok(self.handler.write_u16(address, value as u16)), - 32 => Ok(self.handler.write_u32(address, value as u32)), - 64 => Ok(self.handler.write_u64(address, value)), - _ => Err(AmlError::FieldInvalidAccessSize), - } - } - - RegionSpace::SystemIo => { - let port = (region_base + offset).try_into().map_err(|_| AmlError::FieldInvalidAddress)?; - match length { - 8 => Ok(self.handler.write_io_u8(port, value as u8)), - 16 => Ok(self.handler.write_io_u16(port, value as u16)), - 32 => Ok(self.handler.write_io_u32(port, value as u32)), - _ => Err(AmlError::FieldInvalidAccessSize), - } - } - - RegionSpace::PciConfig => { - /* - * First, we need to get some extra information out of objects in the parent object. Both - * `_SEG` and `_BBN` seem optional, with defaults that line up with legacy PCI implementations - * (e.g. systems with a single segment group and a single root, respectively). - */ - let parent_device = parent_device.as_ref().unwrap(); - let seg = match self.invoke_method( - &AmlName::from_str("_SEG").unwrap().resolve(parent_device).unwrap(), - Args::EMPTY, - ) { - Ok(seg) => seg.as_integer(self)?.try_into().map_err(|_| AmlError::FieldInvalidAddress)?, - Err(AmlError::ValueDoesNotExist(_)) => 0, - Err(err) => return Err(err), - }; - let bbn = match self.invoke_method( - &AmlName::from_str("_BBN").unwrap().resolve(parent_device).unwrap(), - Args::EMPTY, - ) { - Ok(bbn) => bbn.as_integer(self)?.try_into().map_err(|_| AmlError::FieldInvalidAddress)?, - Err(AmlError::ValueDoesNotExist(_)) => 0, - Err(err) => return Err(err), - }; - let adr = { - let adr = self.invoke_method( - &AmlName::from_str("_ADR").unwrap().resolve(parent_device).unwrap(), - Args::EMPTY, - )?; - adr.as_integer(self)? - }; - - let device = adr.get_bits(16..24) as u8; - let function = adr.get_bits(0..8) as u8; - let offset = (region_base + offset).try_into().map_err(|_| AmlError::FieldInvalidAddress)?; - - match length { - 8 => Ok(self.handler.write_pci_u8(seg, bbn, device, function, offset, value as u8)), - 16 => Ok(self.handler.write_pci_u16(seg, bbn, device, function, offset, value as u16)), - 32 => Ok(self.handler.write_pci_u32(seg, bbn, device, function, offset, value as u32)), - _ => Err(AmlError::FieldInvalidAccessSize), - } - } - - // TODO - _ => unimplemented!(), - } - } - fn add_predefined_objects(&mut self) { /* * These are the scopes predefined by the spec. Some tables will try to access them without defining them diff --git a/aml/src/opregion.rs b/aml/src/opregion.rs new file mode 100644 index 00000000..533f329a --- /dev/null +++ b/aml/src/opregion.rs @@ -0,0 +1,270 @@ +use crate::{ + value::{Args, FieldAccessType, FieldFlags, FieldUpdateRule}, + AmlContext, + AmlError, + AmlName, + AmlValue, +}; +use bit_field::BitField; + +#[derive(Clone, Debug)] +pub struct OpRegion { + region: RegionSpace, + base: u64, + length: u64, + parent_device: Option, +} + +impl OpRegion { + pub fn new(region: RegionSpace, base: u64, length: u64, parent_device: Option) -> OpRegion { + OpRegion { region, base, length, parent_device } + } + + /// Get the length of this op-region, in **bits**. + pub fn length(&self) -> u64 { + self.length + } + + /// Read a field from this op-region. This has looser requirements than `read`, and will + /// perform multiple standard-sized reads and mask the result as required. + pub fn read_field( + &self, + offset: u64, + length: u64, + flags: FieldFlags, + context: &mut AmlContext, + ) -> Result { + let _max_access_size = match self.region { + RegionSpace::SystemMemory => 64, + RegionSpace::SystemIo | RegionSpace::PciConfig => 32, + _ => unimplemented!(), + }; + let minimum_access_size = match flags.access_type()? { + FieldAccessType::Any => 8, + FieldAccessType::Byte => 8, + FieldAccessType::Word => 16, + FieldAccessType::DWord => 32, + FieldAccessType::QWord => 64, + FieldAccessType::Buffer => 8, // TODO + }; + + /* + * Find the access size, as either the minimum access size allowed by the region, or the field length + * rounded up to the next power-of-2, whichever is larger. + */ + let access_size = u64::max(minimum_access_size, length.next_power_of_two()); + + /* + * TODO: we need to decide properly how to read from the region itself. Complications: + * - if the region has a minimum access size greater than the desired length, we need to read the + * minimum and mask it (reading a byte from a WordAcc region) + * - if the desired length is larger than we can read, we need to do multiple reads + */ + let value = self.read(offset, access_size, context)?.get_bits(0..(length as usize)); + Ok(AmlValue::Integer(value)) + } + + pub fn write_field( + &self, + offset: u64, + length: u64, + flags: FieldFlags, + value: AmlValue, + context: &mut AmlContext, + ) -> Result<(), AmlError> { + /* + * If the field's update rule is `Preserve`, we need to read the initial value of the field, so we can + * overwrite the correct bits. We destructure the field to do the actual write, so we read from it if + * needed here, otherwise the borrow-checker doesn't understand. + */ + let mut field_value = match flags.field_update_rule()? { + FieldUpdateRule::Preserve => self.read_field(offset, length, flags, context)?.as_integer(context)?, + FieldUpdateRule::WriteAsOnes => 0xffffffff_ffffffff, + FieldUpdateRule::WriteAsZeros => 0x0, + }; + + let _maximum_access_size = match self.region { + RegionSpace::SystemMemory => 64, + RegionSpace::SystemIo | RegionSpace::PciConfig => 32, + _ => unimplemented!(), + }; + let minimum_access_size = match flags.access_type()? { + FieldAccessType::Any => 8, + FieldAccessType::Byte => 8, + FieldAccessType::Word => 16, + FieldAccessType::DWord => 32, + FieldAccessType::QWord => 64, + FieldAccessType::Buffer => 8, // TODO + }; + + /* + * Find the access size, as either the minimum access size allowed by the region, or the field length + * rounded up to the next power-of-2, whichever is larger. + */ + let access_size = u64::max(minimum_access_size, length.next_power_of_two()); + + field_value.set_bits(0..(length as usize), value.as_integer(context)?); + self.write(offset, access_size, field_value, context) + } + + /// Perform a standard-size read from this op-region. `length` must be a supported power-of-2, + /// and `offset` correctly aligned for that `length`. `value` must be appropriately sized. + pub fn read(&self, offset: u64, length: u64, context: &mut AmlContext) -> Result { + match self.region { + RegionSpace::SystemMemory => { + let address = (self.base + offset).try_into().map_err(|_| AmlError::FieldInvalidAddress)?; + match length { + 8 => Ok(context.handler.read_u8(address) as u64), + 16 => Ok(context.handler.read_u16(address) as u64), + 32 => Ok(context.handler.read_u32(address) as u64), + 64 => Ok(context.handler.read_u64(address)), + _ => Err(AmlError::FieldInvalidAccessSize), + } + } + + RegionSpace::SystemIo => { + let port = (self.base + offset).try_into().map_err(|_| AmlError::FieldInvalidAddress)?; + match length { + 8 => Ok(context.handler.read_io_u8(port) as u64), + 16 => Ok(context.handler.read_io_u16(port) as u64), + 32 => Ok(context.handler.read_io_u32(port) as u64), + _ => Err(AmlError::FieldInvalidAccessSize), + } + } + + RegionSpace::PciConfig => { + /* + * First, we need to get some extra information out of objects in the parent object. Both + * `_SEG` and `_BBN` seem optional, with defaults that line up with legacy PCI implementations + * (e.g. systems with a single segment group and a single root, respectively). + */ + let parent_device = self.parent_device.as_ref().unwrap(); + let seg = match context.invoke_method( + &AmlName::from_str("_SEG").unwrap().resolve(parent_device).unwrap(), + Args::EMPTY, + ) { + Ok(seg) => seg.as_integer(context)?.try_into().map_err(|_| AmlError::FieldInvalidAddress)?, + Err(AmlError::ValueDoesNotExist(_)) => 0, + Err(err) => return Err(err), + }; + let bbn = match context.invoke_method( + &AmlName::from_str("_BBN").unwrap().resolve(parent_device).unwrap(), + Args::EMPTY, + ) { + Ok(bbn) => bbn.as_integer(context)?.try_into().map_err(|_| AmlError::FieldInvalidAddress)?, + Err(AmlError::ValueDoesNotExist(_)) => 0, + Err(err) => return Err(err), + }; + let adr = { + let adr = context.invoke_method( + &AmlName::from_str("_ADR").unwrap().resolve(parent_device).unwrap(), + Args::EMPTY, + )?; + adr.as_integer(context)? + }; + + let device = adr.get_bits(16..24) as u8; + let function = adr.get_bits(0..8) as u8; + let offset = (self.base + offset).try_into().map_err(|_| AmlError::FieldInvalidAddress)?; + + match length { + 8 => Ok(context.handler.read_pci_u8(seg, bbn, device, function, offset) as u64), + 16 => Ok(context.handler.read_pci_u16(seg, bbn, device, function, offset) as u64), + 32 => Ok(context.handler.read_pci_u32(seg, bbn, device, function, offset) as u64), + _ => Err(AmlError::FieldInvalidAccessSize), + } + } + + // TODO + _ => unimplemented!(), + } + } + + /// Perform a standard-size write to this op-region. `length` must be a supported power-of-2, + /// and `offset` correctly aligned for that `length`. `value` must be appropriately sized. + pub fn write(&self, offset: u64, length: u64, value: u64, context: &mut AmlContext) -> Result<(), AmlError> { + match self.region { + RegionSpace::SystemMemory => { + let address = (self.base + offset).try_into().map_err(|_| AmlError::FieldInvalidAddress)?; + match length { + 8 => Ok(context.handler.write_u8(address, value as u8)), + 16 => Ok(context.handler.write_u16(address, value as u16)), + 32 => Ok(context.handler.write_u32(address, value as u32)), + 64 => Ok(context.handler.write_u64(address, value)), + _ => Err(AmlError::FieldInvalidAccessSize), + } + } + + RegionSpace::SystemIo => { + let port = (self.base + offset).try_into().map_err(|_| AmlError::FieldInvalidAddress)?; + match length { + 8 => Ok(context.handler.write_io_u8(port, value as u8)), + 16 => Ok(context.handler.write_io_u16(port, value as u16)), + 32 => Ok(context.handler.write_io_u32(port, value as u32)), + _ => Err(AmlError::FieldInvalidAccessSize), + } + } + + RegionSpace::PciConfig => { + /* + * First, we need to get some extra information out of objects in the parent object. Both + * `_SEG` and `_BBN` seem optional, with defaults that line up with legacy PCI implementations + * (e.g. systems with a single segment group and a single root, respectively). + */ + let parent_device = self.parent_device.as_ref().unwrap(); + let seg = match context.invoke_method( + &AmlName::from_str("_SEG").unwrap().resolve(parent_device).unwrap(), + Args::EMPTY, + ) { + Ok(seg) => seg.as_integer(context)?.try_into().map_err(|_| AmlError::FieldInvalidAddress)?, + Err(AmlError::ValueDoesNotExist(_)) => 0, + Err(err) => return Err(err), + }; + let bbn = match context.invoke_method( + &AmlName::from_str("_BBN").unwrap().resolve(parent_device).unwrap(), + Args::EMPTY, + ) { + Ok(bbn) => bbn.as_integer(context)?.try_into().map_err(|_| AmlError::FieldInvalidAddress)?, + Err(AmlError::ValueDoesNotExist(_)) => 0, + Err(err) => return Err(err), + }; + let adr = { + let adr = context.invoke_method( + &AmlName::from_str("_ADR").unwrap().resolve(parent_device).unwrap(), + Args::EMPTY, + )?; + adr.as_integer(context)? + }; + + let device = adr.get_bits(16..24) as u8; + let function = adr.get_bits(0..8) as u8; + let offset = (self.base + offset).try_into().map_err(|_| AmlError::FieldInvalidAddress)?; + + match length { + 8 => Ok(context.handler.write_pci_u8(seg, bbn, device, function, offset, value as u8)), + 16 => Ok(context.handler.write_pci_u16(seg, bbn, device, function, offset, value as u16)), + 32 => Ok(context.handler.write_pci_u32(seg, bbn, device, function, offset, value as u32)), + _ => Err(AmlError::FieldInvalidAccessSize), + } + } + + // TODO + _ => unimplemented!(), + } + } +} + +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum RegionSpace { + SystemMemory, + SystemIo, + PciConfig, + EmbeddedControl, + SMBus, + SystemCmos, + PciBarTarget, + IPMI, + GeneralPurposeIo, + GenericSerialBus, + OemDefined(u8), +} diff --git a/aml/src/pkg_length.rs b/aml/src/pkg_length.rs index 16b1e71f..214d1253 100644 --- a/aml/src/pkg_length.rs +++ b/aml/src/pkg_length.rs @@ -74,7 +74,7 @@ where * OperationRegion length is in bytes, PkgLength is in bits, so conversion is needed */ let region_bit_length = match region_value { - AmlValue::OpRegion { length, .. } => *length * 8, + AmlValue::OpRegion(region) => region.length() * 8, _ => return Err((input, context, Propagate::Err(AmlError::FieldRegionIsNotOpRegion))), }; diff --git a/aml/src/term_object.rs b/aml/src/term_object.rs index bdb2cffb..3859e81f 100644 --- a/aml/src/term_object.rs +++ b/aml/src/term_object.rs @@ -4,6 +4,7 @@ use crate::{ name_object::{name_seg, name_string, target, Target}, namespace::{AmlName, LevelType}, opcode::{self, ext_opcode, opcode}, + opregion::{OpRegion, RegionSpace}, parser::{ choice, comment_scope, @@ -19,7 +20,7 @@ use crate::{ }, pkg_length::{pkg_length, region_pkg_length, PkgLength}, statement::statement_opcode, - value::{AmlValue, FieldFlags, MethodCode, MethodFlags, RegionSpace}, + value::{AmlValue, FieldFlags, MethodCode, MethodFlags}, AmlContext, AmlError, AmlHandle, @@ -474,7 +475,7 @@ where context.namespace.add_value_at_resolved_path( name, &context.current_scope, - AmlValue::OpRegion { region, offset, length, parent_device } + AmlValue::OpRegion(OpRegion::new(region, offset, length, parent_device)) ) ); (Ok(()), context) diff --git a/aml/src/value.rs b/aml/src/value.rs index c4a36331..afeb1327 100644 --- a/aml/src/value.rs +++ b/aml/src/value.rs @@ -1,4 +1,4 @@ -use crate::{misc::ArgNum, AmlContext, AmlError, AmlHandle, AmlName}; +use crate::{misc::ArgNum, opregion::OpRegion, AmlContext, AmlError, AmlHandle}; use alloc::{ string::{String, ToString}, sync::Arc, @@ -8,21 +8,6 @@ use bit_field::BitField; use core::{cmp, fmt, fmt::Debug}; use spinning_top::Spinlock; -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub enum RegionSpace { - SystemMemory, - SystemIo, - PciConfig, - EmbeddedControl, - SMBus, - SystemCmos, - PciBarTarget, - IPMI, - GeneralPurposeIo, - GenericSerialBus, - OemDefined(u8), -} - #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum FieldAccessType { Any, @@ -181,12 +166,7 @@ pub enum AmlValue { /// Describes an operation region. Some regions require other objects to be declared under their parent device /// (e.g. an `_ADR` object for a `PciConfig` region), in which case an absolute path to the object is stored in /// `parent_device`. - OpRegion { - region: RegionSpace, - offset: u64, - length: u64, - parent_device: Option, - }, + OpRegion(OpRegion), /// Describes a field unit within an operation region. Field { region: AmlHandle, @@ -421,99 +401,29 @@ impl AmlValue { } } - /// Reads from a field of an opregion, returning either a `AmlValue::Integer` or an `AmlValue::Buffer`, + /// Reads from a field of an op-region, returning either a `AmlValue::Integer` or an `AmlValue::Buffer`, /// depending on the size of the field. pub fn read_field(&self, context: &mut AmlContext) -> Result { if let AmlValue::Field { region, flags, offset, length } = self { - let _maximum_access_size = { - if let AmlValue::OpRegion { region, .. } = context.namespace.get(*region)? { - match region { - RegionSpace::SystemMemory => 64, - RegionSpace::SystemIo | RegionSpace::PciConfig => 32, - _ => unimplemented!(), - } - } else { - return Err(AmlError::FieldRegionIsNotOpRegion); - } - }; - let minimum_access_size = match flags.access_type()? { - FieldAccessType::Any => 8, - FieldAccessType::Byte => 8, - FieldAccessType::Word => 16, - FieldAccessType::DWord => 32, - FieldAccessType::QWord => 64, - FieldAccessType::Buffer => 8, // TODO - }; - - /* - * Find the access size, as either the minimum access size allowed by the region, or the field length - * rounded up to the next power-of-2, whichever is larger. - */ - let access_size = u64::max(minimum_access_size, length.next_power_of_two()); - - /* - * TODO: we need to decide properly how to read from the region itself. Complications: - * - if the region has a minimum access size greater than the desired length, we need to read the - * minimum and mask it (reading a byte from a WordAcc region) - * - if the desired length is larger than we can read, we need to do multiple reads - */ - Ok(AmlValue::Integer( - context.read_region(*region, *offset, access_size)?.get_bits(0..(*length as usize)), - )) + if let AmlValue::OpRegion(region) = context.namespace.get(*region)?.clone() { + region.read_field(*offset, *length, *flags, context) + } else { + Err(AmlError::FieldRegionIsNotOpRegion) + } } else { Err(AmlError::IncompatibleValueConversion { current: self.type_of(), target: AmlType::FieldUnit }) } } + /// Write to a field of an op-region, from either a `AmlValue::Integer` or `AmlValue::Buffer` + /// as necessary. pub fn write_field(&mut self, value: AmlValue, context: &mut AmlContext) -> Result<(), AmlError> { - /* - * If the field's update rule is `Preserve`, we need to read the initial value of the field, so we can - * overwrite the correct bits. We destructure the field to do the actual write, so we read from it if - * needed here, otherwise the borrow-checker doesn't understand. - */ - let field_update_rule = if let AmlValue::Field { flags, .. } = self { - flags.field_update_rule()? - } else { - return Err(AmlError::IncompatibleValueConversion { - current: self.type_of(), - target: AmlType::FieldUnit, - }); - }; - let mut field_value = match field_update_rule { - FieldUpdateRule::Preserve => self.read_field(context)?.as_integer(context)?, - FieldUpdateRule::WriteAsOnes => 0xffffffff_ffffffff, - FieldUpdateRule::WriteAsZeros => 0x0, - }; - if let AmlValue::Field { region, flags, offset, length } = self { - let _maximum_access_size = { - if let AmlValue::OpRegion { region, .. } = context.namespace.get(*region)? { - match region { - RegionSpace::SystemMemory => 64, - RegionSpace::SystemIo | RegionSpace::PciConfig => 32, - _ => unimplemented!(), - } - } else { - return Err(AmlError::FieldRegionIsNotOpRegion); - } - }; - let minimum_access_size = match flags.access_type()? { - FieldAccessType::Any => 8, - FieldAccessType::Byte => 8, - FieldAccessType::Word => 16, - FieldAccessType::DWord => 32, - FieldAccessType::QWord => 64, - FieldAccessType::Buffer => 8, // TODO - }; - - /* - * Find the access size, as either the minimum access size allowed by the region, or the field length - * rounded up to the next power-of-2, whichever is larger. - */ - let access_size = u64::max(minimum_access_size, length.next_power_of_two()); - - field_value.set_bits(0..(*length as usize), value.as_integer(context)?); - context.write_region(*region, *offset, access_size, field_value) + if let AmlValue::OpRegion(region) = context.namespace.get(*region)?.clone() { + region.write_field(*offset, *length, *flags, value, context) + } else { + Err(AmlError::FieldRegionIsNotOpRegion) + } } else { Err(AmlError::IncompatibleValueConversion { current: self.type_of(), target: AmlType::FieldUnit }) } From c4d6029e78befae099fd8d58ee02dbc66a46116b Mon Sep 17 00:00:00 2001 From: Isaac Woods Date: Tue, 5 Mar 2024 23:25:37 +0000 Subject: [PATCH 5/7] Simplify `DefIncrement` and `DefDecrement` borrow checking For some reason, this *definitely* required an extra scope before, but doesn't seem to now... --- aml/src/expression.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/aml/src/expression.rs b/aml/src/expression.rs index 05bb1c96..ea256136 100644 --- a/aml/src/expression.rs +++ b/aml/src/expression.rs @@ -331,10 +331,7 @@ where DebugVerbosity::AllScopes, "DefIncrement", super_name().map_with_context(|addend, context| { - let value = { - let value = try_with_context!(context, context.read_target(&addend)); - value.clone() - }; + let value = try_with_context!(context, context.read_target(&addend)).clone(); let value = try_with_context!(context, value.as_integer(context)); let new_value = AmlValue::Integer(value + 1); try_with_context!(context, context.store(addend, new_value.clone())); @@ -356,10 +353,7 @@ where DebugVerbosity::AllScopes, "DefDecrement", super_name().map_with_context(|minuend, context| { - let value = { - let value = try_with_context!(context, context.read_target(&minuend)); - value.clone() - }; + let value = try_with_context!(context, context.read_target(&minuend)).clone(); let value = try_with_context!(context, value.as_integer(context)); let new_value = AmlValue::Integer(value - 1); try_with_context!(context, context.store(minuend, new_value.clone())); From 649e579a3e44b92bb50755c21c0ef5744bf665d0 Mon Sep 17 00:00:00 2001 From: Isaac Woods Date: Tue, 5 Mar 2024 23:44:05 +0000 Subject: [PATCH 6/7] Add test for `DefIncrement` and `DefDecrement` --- tests/inc.asl | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 tests/inc.asl diff --git a/tests/inc.asl b/tests/inc.asl new file mode 100644 index 00000000..729231c7 --- /dev/null +++ b/tests/inc.asl @@ -0,0 +1,9 @@ +DefinitionBlock("inc.aml", "DSDT", 1, "RSACPI", "INCDEC", 1) { + Name(X, 0) + X++ + X++ + X++ + Name(Y, 0) + Y = X + X-- +} From f769475182e53ff7786bec108b93b6fe21a3aa02 Mon Sep 17 00:00:00 2001 From: Isaac Woods Date: Tue, 5 Mar 2024 23:47:43 +0000 Subject: [PATCH 7/7] Fix test utils --- aml/src/test_utils.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/aml/src/test_utils.rs b/aml/src/test_utils.rs index e0616871..75e2e6ab 100644 --- a/aml/src/test_utils.rs +++ b/aml/src/test_utils.rs @@ -141,15 +141,8 @@ pub(crate) fn crudely_cmp_values(a: &AmlValue, b: &AmlValue) -> bool { AmlValue::String(ref b) => a == b, _ => false, }, - AmlValue::OpRegion { region, offset, length, parent_device } => match b { - AmlValue::OpRegion { - region: b_region, - offset: b_offset, - length: b_length, - parent_device: b_parent_device, - } => { - region == b_region && offset == b_offset && length == b_length && parent_device == b_parent_device - } + AmlValue::OpRegion(_) => match b { + AmlValue::OpRegion(_) => panic!("Can't compare two op-regions"), _ => false, }, AmlValue::Field { region, flags, offset, length } => match b {