Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

aml: DefIndexField implementation #186

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

aml: DefIndexField implementation #186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6231028
aml: (WIP) basic `DefIndexField` implementation
alnyan Aug 23, 2023
6c7f593
aml: handle IndexField fields crossing index boundary
alnyan Aug 23, 2023
e65ccf4
aml: Fix tests not building with IndexField
alnyan Oct 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions aml/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,8 +335,17 @@ impl AmlContext {
match self.namespace.get(handle).unwrap().type_of() {
AmlType::FieldUnit => {
let mut field = self.namespace.get(handle).unwrap().clone();
field.write_field(value, self)?;
field.read_field(self)
match field {
AmlValue::Field { .. } => {
field.write_field(value, self)?;
field.read_field(self)
}
AmlValue::IndexField { .. } => {
field.write_index_field(value, self)?;
field.read_index_field(self)
}
_ => unreachable!(),
}
}
AmlType::BufferField => {
let mut buffer_field = self.namespace.get(handle).unwrap().clone();
Expand Down
1 change: 1 addition & 0 deletions aml/src/opcode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ pub const EXT_DEF_DEVICE_OP: u8 = 0x82;
pub const EXT_DEF_PROCESSOR_OP: u8 = 0x83;
pub const EXT_DEF_POWER_RES_OP: u8 = 0x84;
pub const EXT_DEF_THERMAL_ZONE_OP: u8 = 0x85;
pub const EXT_DEF_INDEX_FIELD_OP: u8 = 0x86;

/*
* Statement opcodes
Expand Down
97 changes: 95 additions & 2 deletions aml/src/term_object.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::{
Parser,
Propagate,
},
pkg_length::{pkg_length, region_pkg_length, PkgLength},
pkg_length::{pkg_length, raw_pkg_length, region_pkg_length, PkgLength},
statement::statement_opcode,
value::{AmlValue, FieldFlags, MethodCode, MethodFlags, RegionSpace},
AmlContext,
Expand Down Expand Up @@ -110,7 +110,8 @@ where
def_processor(),
def_power_res(),
def_thermal_zone(),
def_mutex()
def_mutex(),
def_index_field()
),
)
}
Expand Down Expand Up @@ -884,6 +885,98 @@ where
.map(|((), result)| Ok(result))
}

fn index_field_element<'a, 'c>(
index_handle: AmlHandle,
data_handle: AmlHandle,
flags: FieldFlags,
current_offset: u64,
) -> impl Parser<'a, 'c, u64>
where
'c: 'a,
{
/*
* Reserved fields shouldn't actually be added to the namespace; they seem to show gaps in
* the operation region that aren't used for anything.
*
* NOTE: had to use raw_pkg_length() instead of pkg_length() here because pkg_length() performs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm yeah this is a really good point! Looks like we're using the raw length in the other field code but still parsing with a pkg_length, which I guess could cause errors if the op region was longer than the remaining AML slice. We should probably look at that in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I've created a separate issue for that already. The parsing of the field might become a little more complicated, as instead of just pkg_length() we would need to also pass a opreg to the parser

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for implemeting as_* functions on IndexFields, I've done that on my separate branch where I just try to make a dirty but fully working implementation for my laptop, but that involves making almost everything (write_field/write_region/etc.) take an immutable ref to an AmlContext instead of a mutable one which doesn't seem quite right.

I've also tried making as_integer take a mutable ref, but then we'd have a problem in some places where pieces of code like:

// in def_increment()/def_decrement() parsers
let value = try_with_context!(context, context.read_target(&addend));
let value = try_with_context!(context, value.as_integer(context));

fails to compile because the first line now borrows context immutably and the second one tries to borrow it mutably. Fixing that in and preserving mutability of the ref might involve either moving the context inside some kind of Spinlock or making wrapper functions that merge the functionality of read_target() and as_* without needing to borrow twice.

* checks against aml code slice, and in this specific context the check should be
* performed against the parent field's size
*/
let reserved_field =
opcode(opcode::RESERVED_FIELD).then(raw_pkg_length()).map(|((), length)| Ok(length as u64));

let named_field = name_seg().then(pkg_length()).map_with_context(move |(name_seg, length), context| {
try_with_context!(
context,
context.namespace.add_value_at_resolved_path(
AmlName::from_name_seg(name_seg),
&context.current_scope,
AmlValue::IndexField {
index: index_handle,
data: data_handle,
flags,
offset: current_offset,
length: length.raw_length as u64,
}
)
);

(Ok(length.raw_length as u64), context)
});

choice!(reserved_field, named_field)
}

pub fn def_index_field<'a, 'c>() -> impl Parser<'a, 'c, ()>
where
'c: 'a,
{
/*
* DefIndexField := IndexFieldOp PkgLength NameString NameString FieldFlags FieldList
* IndexFieldOp := ExtOpPrefix 0x86
*/
// Declare a group of field that must be accessed by writing to an index
// register and then reading/writing from a data register.
ext_opcode(opcode::EXT_DEF_INDEX_FIELD_OP)
.then(comment_scope(
DebugVerbosity::Scopes,
"DefIndexField",
pkg_length()
.then(name_string())
.then(name_string())
.then(take())
.map_with_context(|(((list_length, index_name), data_name), flags), context| {
let (_, index_handle) =
try_with_context!(context, context.namespace.search(&index_name, &context.current_scope));
let (_, data_handle) =
try_with_context!(context, context.namespace.search(&data_name, &context.current_scope));

(Ok((list_length, index_handle, data_handle, flags)), context)
})
.feed(|(list_length, index_handle, data_handle, flags)| {
move |mut input: &'a [u8], mut context: &'c mut AmlContext| -> ParseResult<'a, 'c, ()> {
let mut current_offset = 0;
while list_length.still_parsing(input) {
let (new_input, new_context, field_length) = index_field_element(
index_handle,
data_handle,
FieldFlags::new(flags),
current_offset,
)
.parse(input, context)?;

input = new_input;
context = new_context;
current_offset += field_length;
}

Ok((input, context, ()))
}
}),
))
.discard_result()
}

pub fn term_arg<'a, 'c>() -> impl Parser<'a, 'c, AmlValue>
where
'c: 'a,
Expand Down
12 changes: 12 additions & 0 deletions aml/src/test_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,18 @@ pub(crate) fn crudely_cmp_values(a: &AmlValue, b: &AmlValue) -> bool {
}
_ => false,
},
AmlValue::IndexField { index, data, flags, offset, length } => match b {
AmlValue::IndexField {
index: b_index,
data: b_data,
flags: b_flags,
offset: b_offset,
length: b_length,
} => {
index == b_index && data == b_data && flags == b_flags && offset == b_offset && length == b_length
}
_ => false,
},
AmlValue::Device => match b {
AmlValue::Device => true,
_ => false,
Expand Down
123 changes: 122 additions & 1 deletion aml/src/value.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,13 @@ pub enum AmlValue {
offset: u64,
length: u64,
},
IndexField {
index: AmlHandle,
data: AmlHandle,
flags: FieldFlags,
offset: u64,
length: u64,
},
Device,
Method {
flags: MethodFlags,
Expand Down Expand Up @@ -252,7 +259,7 @@ impl AmlValue {
AmlValue::Integer(_) => AmlType::Integer,
AmlValue::String(_) => AmlType::String,
AmlValue::OpRegion { .. } => AmlType::OpRegion,
AmlValue::Field { .. } => AmlType::FieldUnit,
AmlValue::Field { .. } | AmlValue::IndexField { .. } => AmlType::FieldUnit,
AmlValue::Device => AmlType::Device,
AmlValue::Method { .. } => AmlType::Method,
AmlValue::Buffer(_) => AmlType::Buffer,
Expand Down Expand Up @@ -314,6 +321,8 @@ impl AmlValue {
* `as_integer` on the result.
*/
AmlValue::Field { .. } => self.read_field(context)?.as_integer(context),
// TODO: IndexField cannot be accessed through an immutable &AmlContext
AmlValue::IndexField { .. } => todo!(),
AmlValue::BufferField { .. } => self.read_buffer_field(context)?.as_integer(context),

_ => Err(AmlError::IncompatibleValueConversion { current: self.type_of(), target: AmlType::Integer }),
Expand All @@ -325,6 +334,8 @@ impl AmlValue {
AmlValue::Buffer(ref bytes) => Ok(bytes.clone()),
// TODO: implement conversion of String and Integer to Buffer
AmlValue::Field { .. } => self.read_field(context)?.as_buffer(context),
// TODO: IndexField cannot be accessed through an immutable &AmlContext
AmlValue::IndexField { .. } => todo!(),
AmlValue::BufferField { .. } => self.read_buffer_field(context)?.as_buffer(context),
_ => Err(AmlError::IncompatibleValueConversion { current: self.type_of(), target: AmlType::Buffer }),
}
Expand All @@ -335,6 +346,8 @@ impl AmlValue {
AmlValue::String(ref string) => Ok(string.clone()),
// TODO: implement conversion of Buffer to String
AmlValue::Field { .. } => self.read_field(context)?.as_string(context),
// TODO: IndexField cannot be accessed through an immutable &AmlContext
AmlValue::IndexField { .. } => todo!(),
_ => Err(AmlError::IncompatibleValueConversion { current: self.type_of(), target: AmlType::String }),
}
}
Expand Down Expand Up @@ -421,6 +434,114 @@ impl AmlValue {
}
}

fn index_field_access<F: FnMut(u64, usize, usize, usize) -> Result<(), AmlError>>(
index_flags: &FieldFlags,
offset: u64,
length: u64,
mut access: F,
) -> Result<(), AmlError> {
let index_align = match index_flags.access_type()? {
FieldAccessType::Any => 8,
FieldAccessType::Byte => 8,
FieldAccessType::Word => 16,
FieldAccessType::DWord => 32,
FieldAccessType::QWord => 64,
FieldAccessType::Buffer => 8,
};

let mut length = length as usize;
let mut index = (offset / 8) & !((index_align >> 3) - 1);

// Bit offset in the target Data field
let mut bit_offset = (offset - index * 8) as usize;
// Bit offset in the source value
let mut pos = 0;

while length != 0 {
// Bit offset within a single value
let value_pos = bit_offset % index_align as usize;
// Number of bits until the end of the value
let value_limit = index_align as usize - value_pos;
// Number of bits to access
let len = cmp::min(length, value_limit);

access(index, pos, value_pos, len)?;

// Advance the bit position
bit_offset += len;
pos += len;
length -= len;

// Move to the next index
index += index_align >> 3;
}

Ok(())
}

/// Reads from an IndexField, returning either an `Integer`
pub fn read_index_field(&self, context: &mut AmlContext) -> Result<AmlValue, AmlError> {
let AmlValue::IndexField { index, data, flags, offset, length } = self else {
return Err(AmlError::IncompatibleValueConversion {
current: self.type_of(),
target: AmlType::FieldUnit,
});
};

let mut index_field = context.namespace.get_mut(*index)?.clone();
let data_field = context.namespace.get_mut(*data)?.clone();

// TODO buffer field accesses
let mut value = 0u64;

Self::index_field_access(flags, *offset, *length, |index, value_offset, field_offset, length| {
// Store the bit range index to the Index field
index_field.write_field(AmlValue::Integer(index), context)?;

// Read the bit range from the Data field
let data = data_field.read_field(context)?.as_integer(context)?;
let bits = data.get_bits(field_offset..field_offset + length);

// Copy the bit range to the value
value.set_bits(value_offset..value_offset + length, bits);

Ok(())
})?;

Ok(AmlValue::Integer(value))
}

pub fn write_index_field(&self, value: AmlValue, context: &mut AmlContext) -> Result<(), AmlError> {
let AmlValue::IndexField { index, data, flags, offset, length } = self else {
return Err(AmlError::IncompatibleValueConversion {
current: self.type_of(),
target: AmlType::FieldUnit,
});
};

let mut index_field = context.namespace.get_mut(*index)?.clone();
let mut data_field = context.namespace.get_mut(*data)?.clone();

let value = value.as_integer(context)?;

Self::index_field_access(flags, *offset, *length, |index, value_offset, field_offset, length| {
// TODO handle the UpdateRule flag

// Store the bit range index to the Index field
index_field.write_field(AmlValue::Integer(index), context)?;

// Extract the bits going to this specific part of the field
let bits = value.get_bits(value_offset..value_offset + length);

// Read/modify/store the data field
let mut data = data_field.read_field(context)?.as_integer(context)?;
data.set_bits(field_offset..field_offset + length, bits);
data_field.write_field(AmlValue::Integer(data), context)?;

Ok(())
})
}

/// 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<AmlValue, AmlError> {
Expand Down
28 changes: 28 additions & 0 deletions tests/index_fields.asl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
DefinitionBlock("index_fields.aml", "DSDT", 1, "RSACPI", "IDXFLD", 1) {
OperationRegion (GIO0, SystemIO, 0x125, 0x100)

Field (GIO0, ByteAcc, NoLock, WriteAsZeros) {
IDX0, 8,
DAT0, 8,
IDX1, 8,
DAT1, 16
}

IndexField (IDX0, DAT0, ByteAcc, NoLock, Preserve) {
Offset(0x10),
, 7, // Skip to offset 0x10 bit 7
REG0, 2, // Covers bit 7 in 0x10 and bit 0 in 0x11
}

IndexField (IDX1, DAT1, WordAcc, NoLock, Preserve) {
Offset(0x07), // Offset 0x06, bits 8..
, 7, // Skip to offset 0x06, bit 15
REG1, 2, // Covers bit 15 in 0x06 and bit 0 in 0x08
}

// Store 0b11 to REG0 (index 0x10, bit 0 + index 0x11, bit 1)
Store (3, REG0)

// Store 0b11 to REG1 (index 0x06, bit 15 + index 0x08, bit 0)
Store (3, REG1)
}
Loading