Factor out accesses to op-regions

The logic to do proper field accesses in all cases is going to get a fair
bit more complex, so I thought it was worth factoring out the actual
field access so it can be accessed from the context. If we need to be able
to override those accesses at any point, that's a better place to have it
anyways.

aml/src/lib.rs

@@ -384,6 +384,94 @@ impl AmlContext {
             Target::Null => Ok(value),
         }
     }
+
+    /// 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<u64, 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)?
+            {
+                (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.namespace.search(&AmlName::from_str("_SEG").unwrap(), parent_device) {
+                    Ok((_, handle)) => self
+                        .namespace
+                        .get(handle)?
+                        .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)?,
+                    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 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 trait Handler {

aml/src/value.rs

@@ -295,92 +295,13 @@ impl AmlValue {
     /// depending on the size of the field.
     pub fn read_field(&self, context: &AmlContext) -> Result<AmlValue, AmlError> {
         if let AmlValue::Field { region, flags, offset, length } = self {
-            let (region_space, region_base, region_length, parent_device) = {
-                if let AmlValue::OpRegion { region, offset, length, parent_device } =
-                    context.namespace.get(*region)?
-                {
-                    (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(AmlValue::Integer(context.handler.read_u8(address) as u64)),
-                        16 => Ok(AmlValue::Integer(context.handler.read_u16(address) as u64)),
-                        32 => Ok(AmlValue::Integer(context.handler.read_u32(address) as u64)),
-                        64 => Ok(AmlValue::Integer(context.handler.read_u64(address))),
-                        _ => Err(AmlError::FieldInvalidAccessSize),
-                    }
-                }
-
-                RegionSpace::SystemIo => {
-                    let port = (region_base + offset).try_into().map_err(|_| AmlError::FieldInvalidAddress)?;
-                    match length {
-                        8 => Ok(AmlValue::Integer(context.handler.read_io_u8(port) as u64)),
-                        16 => Ok(AmlValue::Integer(context.handler.read_io_u16(port) as u64)),
-                        32 => Ok(AmlValue::Integer(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 = parent_device.as_ref().unwrap();
-                    let seg = match context.namespace.search(&AmlName::from_str("_SEG").unwrap(), parent_device) {
-                        Ok((_, handle)) => context
-                            .namespace
-                            .get(handle)?
-                            .as_integer(context)?
-                            .try_into()
-                            .map_err(|_| AmlError::FieldInvalidAddress)?,
-                        Err(AmlError::ValueDoesNotExist(_)) => 0,
-                        Err(err) => return Err(err),
-                    };
-                    let bbn = match context.namespace.search(&AmlName::from_str("_BBN").unwrap(), parent_device) {
-                        Ok((_, handle)) => context
-                            .namespace
-                            .get(handle)?
-                            .as_integer(context)?
-                            .try_into()
-                            .map_err(|_| AmlError::FieldInvalidAddress)?,
-                        Err(AmlError::ValueDoesNotExist(_)) => 0,
-                        Err(err) => return Err(err),
-                    };
-                    let adr = {
-                        let (_, handle) =
-                            context.namespace.search(&AmlName::from_str("_ADR").unwrap(), parent_device)?;
-                        context.namespace.get(handle)?.as_integer(context)?
-                    };
-
-                    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(AmlValue::Integer(
-                            context.handler.read_pci_u8(seg, bbn, device, function, offset) as u64,
-                        )),
-                        16 => Ok(AmlValue::Integer(
-                            context.handler.read_pci_u16(seg, bbn, device, function, offset) as u64,
-                        )),
-                        32 => Ok(AmlValue::Integer(
-                            context.handler.read_pci_u32(seg, bbn, device, function, offset) as u64,
-                        )),
-                        _ => Err(AmlError::FieldInvalidAccessSize),
-                    }
-                }
-
-                // TODO
-                _ => unimplemented!(),
-            }
+            /*
+             * 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, *length)?))
         } else {
             Err(AmlError::IncompatibleValueConversion)
         }