From 2757a27049ebc8e3f1e4b996cafc61c5bcc7a95e Mon Sep 17 00:00:00 2001 From: Martin Hughes Date: Wed, 11 Mar 2026 20:19:30 +0000 Subject: [PATCH] Implement Index and Bank fields --- src/aml/mod.rs | 118 ++++++++++++++---- tests/bank_fields.rs | 71 +++++++++++ tests/index_fields.rs | 283 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 450 insertions(+), 22 deletions(-) create mode 100644 tests/bank_fields.rs create mode 100644 tests/index_fields.rs diff --git a/src/aml/mod.rs b/src/aml/mod.rs index b567eae0..dd92b876 100644 --- a/src/aml/mod.rs +++ b/src/aml/mod.rs @@ -1374,6 +1374,33 @@ where let (_, data) = namespace.search(&data_name, &context.current_scope)?; (index, data) }; + + if let Object::FieldUnit(ref data_fu) = *data { + if data_fu.flags.access_type_bytes()? < FieldFlags(field_flags).access_type_bytes()? { + // On ACPICA this causes reads/writes to be truncated to the width of + // the data register. + // + // The issue comes from this part of the spec: + // "The value written to the IndexName register is defined to be a byte + // offset that is aligned on an AccessType boundary." + // + // Consider the case where the index field is WordAcc but the data + // register is ByteAcc. Only even numbers could be written to the index + // register - multiples of width of word (2 bytes). But to access the + // high byte of the word for the field, we'd need to write an odd number + // to the index register. Which is not compatible with the spec. + // + // The ASL writer shouldn't have allowed this. And it seems that most + // uses of IndexField are ByteAcc all around. So whatever strange + // behaviour we allow is probably OK. + // + // But warn the user, just in case. + warn!("Data field access width is smaller than normal field width at {:?}", start_pc); + } + } else { + warn!("Wrong data field type in IndexField: {:?}", data); + }; + let kind = FieldUnitKind::Index { index, data }; self.parse_field_list(&mut context, kind, start_pc, pkg_length, field_flags)?; } @@ -2376,17 +2403,19 @@ where Output::Integer(value) => value, }; - let read_region = match field.kind { - FieldUnitKind::Normal { ref region } => region, - // FieldUnitKind::Bank { ref region, ref bank, bank_value } => { - FieldUnitKind::Bank { .. } => { - // TODO: put the bank_value in the bank - todo!(); + let (read_region, index_field_idx) = match field.kind { + FieldUnitKind::Normal { ref region } => (region, 0), + FieldUnitKind::Bank { ref region, ref bank, bank_value } => { + let Object::FieldUnit(ref bank) = **bank else { panic!() }; + assert!(matches!(bank.kind, FieldUnitKind::Normal { .. })); + self.do_field_write(bank, Object::Integer(bank_value).wrap())?; + (region, 0) } - // FieldUnitKind::Index { ref index, ref data } => { - FieldUnitKind::Index { .. } => { - // TODO: configure the correct index - todo!(); + FieldUnitKind::Index { index: _, ref data } => { + let Object::FieldUnit(ref data) = **data else { panic!() }; + let FieldUnitKind::Normal { region } = &data.kind else { panic!() }; + let reg_idx = field.bit_index / 8; + (region, reg_idx) } }; let Object::OpRegion(ref read_region) = **read_region else { panic!() }; @@ -2406,7 +2435,27 @@ where / access_width_bits; let mut read_so_far = 0; for i in 0..native_accesses_needed { - let aligned_offset = object::align_down(field.bit_index + i * access_width_bits, access_width_bits); + // Advance the read pointer. For Index fields, this also means updating the Index + // register. + let aligned_offset = match field.kind { + FieldUnitKind::Normal { .. } | FieldUnitKind::Bank { .. } => { + object::align_down(field.bit_index + i * access_width_bits, access_width_bits) + } + FieldUnitKind::Index { ref index, ref data } => { + // Update index register + let Object::FieldUnit(ref index) = **index else { panic!() }; + let Object::FieldUnit(ref data) = **data else { panic!() }; + self.do_field_write( + index, + Object::Integer((index_field_idx + i * (access_width_bits / 8)) as u64).wrap(), + )?; + + // The offset is always that of the data register, as we always read from the + // base of the data register. + data.bit_index + } + }; + let raw = self.do_native_region_read(read_region, aligned_offset / 8, access_width_bits / 8)?; let src_index = if i == 0 { field.bit_index % access_width_bits } else { 0 }; let remaining_length = field.bit_length - read_so_far; @@ -2436,17 +2485,23 @@ where }; let access_width_bits = field.flags.access_type_bytes()? * 8; - let write_region = match field.kind { - FieldUnitKind::Normal { ref region } => region, - // FieldUnitKind::Bank { ref region, ref bank, bank_value } => { - FieldUnitKind::Bank { .. } => { - // TODO: put the bank_value in the bank - todo!(); + // In this tuple: + // - write_region is the region that the data will be written to. + // - index_field_idx is the initial index to write into the Index register of an index + // field. For all other field types it is unused and set to zero. + let (write_region, index_field_idx) = match field.kind { + FieldUnitKind::Normal { ref region } => (region, 0), + FieldUnitKind::Bank { ref region, ref bank, bank_value } => { + let Object::FieldUnit(ref bank) = **bank else { panic!() }; + assert!(matches!(bank.kind, FieldUnitKind::Normal { .. })); + self.do_field_write(bank, Object::Integer(bank_value).wrap())?; + (region, 0) } - // FieldUnitKind::Index { ref index, ref data } => { - FieldUnitKind::Index { .. } => { - // TODO: configure the correct index - todo!(); + FieldUnitKind::Index { index: _, ref data } => { + let Object::FieldUnit(ref data) = **data else { panic!() }; + let FieldUnitKind::Normal { region: data_region } = &data.kind else { panic!() }; + let reg_idx = field.bit_index / 8; + (data_region, reg_idx) } }; let Object::OpRegion(ref write_region) = **write_region else { panic!() }; @@ -2461,7 +2516,26 @@ where let mut written_so_far = 0; for i in 0..native_accesses_needed { - let aligned_offset = object::align_down(field.bit_index + i * access_width_bits, access_width_bits); + // Advance the write pointer... For normal and bank fields this is straightforward. For + // Index fields, this involves updating the index register. + let aligned_offset = match field.kind { + FieldUnitKind::Normal { .. } | FieldUnitKind::Bank { .. } => { + object::align_down(field.bit_index + i * access_width_bits, access_width_bits) + } + FieldUnitKind::Index { ref index, ref data } => { + // Update index register + let Object::FieldUnit(ref index) = **index else { panic!() }; + let Object::FieldUnit(ref data) = **data else { panic!() }; + self.do_field_write( + index, + Object::Integer((index_field_idx + i * (access_width_bits / 8)) as u64).wrap(), + )?; + + // The offset is always that of the data register, as we always read from the + // base of the data register. + data.bit_index + } + }; let dst_index = if i == 0 { field.bit_index % access_width_bits } else { 0 }; /* diff --git a/tests/bank_fields.rs b/tests/bank_fields.rs new file mode 100644 index 00000000..b056f7a2 --- /dev/null +++ b/tests/bank_fields.rs @@ -0,0 +1,71 @@ +// Test operations on "Bank" fields + +use aml_test_tools::handlers::std_test_handler::{ + Command, + construct_std_handler, + create_mutex, + read_u16, + write_u8, + write_u16, +}; + +mod test_infra; + +#[test] +fn test_basic_bank_store_and_load() { + // This is a straightforward test of banked fields. + // Internally: Apart from setting the bank index beforehand, the field read/write is identical + // to normal fields. So this test is probably sufficient testing of banked fields. + const AML: &str = r#"DefinitionBlock("", "DSDT", 1, "RSACPI", "BNKFLD", 1) { + OperationRegion(MEM, SystemMemory, 0x40000, 0x1000) + Field(MEM, ByteAcc, NoLock, Preserve) { + INDX, 8 + } + + BankField (MEM, INDX, 0, WordAcc, NoLock, Preserve) { + OFFSET (0x02), // Prevent aliasing INDX + A, 16, + B, 16, + C, 16 + } + + BankField (MEM, INDX, 1, WordAcc, NoLock, Preserve) { + OFFSET (0x02), + D, 16, + E, 16, + F, 16 + } + + Method(MAIN, 0, NotSerialized) { + C = 0xA55A + D = C + E = 0x5AA5 + A = E + Return (0) + } +} +"#; + + const EXPECTED_COMMANDS: &[Command] = &[ + create_mutex(), + // C = 0xA55A + write_u8(0x40000, 0), // Select bank 0. + write_u16(0x40006, 0xA55A), // Write the value to C. + // D = C + write_u8(0x40000, 0), // Select bank 0. + read_u16(0x40006, 0xA55A), // Read the value from C. + write_u8(0x40000, 1), // Select bank 1. + write_u16(0x40002, 0xA55A), // Write the value to D. + // E = 0x5AA5 + write_u8(0x40000, 1), // Select bank 1. + write_u16(0x40004, 0x5AA5), // Write the value to E. + // A = E + write_u8(0x40000, 1), // Select bank 1. + read_u16(0x40004, 0x5AA5), // Read from E + write_u8(0x40000, 0), // Select bank 0. + write_u16(0x40002, 0x5AA5), // Write the value to A. + ]; + + let handler = construct_std_handler(EXPECTED_COMMANDS.to_vec()); + test_infra::run_aml_test(AML, handler); +} diff --git a/tests/index_fields.rs b/tests/index_fields.rs new file mode 100644 index 00000000..c3b9c406 --- /dev/null +++ b/tests/index_fields.rs @@ -0,0 +1,283 @@ +// Test operations on "Index" fields + +use aml_test_tools::handlers::std_test_handler::{ + Command, + construct_std_handler, + create_mutex, + read_u8, + read_u16, + write_u8, + write_u16, +}; + +mod test_infra; + +#[test] +fn test_basic_index_store_and_load_8_bit() { + // In this test, the data register has the same width as the fields and all fields are correctly + // aligned. We should see single reads and writes for each store operation. + const AML: &str = r#"DefinitionBlock("", "DSDT", 1, "RSACPI", "IDXFLD", 1) { + OperationRegion(MEM, SystemMemory, 0x40000, 0x1000) + Field(MEM, ByteAcc, NoLock, Preserve) { + OFFSET(0x10), + INDX, 8, + DATA, 8 + } + + IndexField (INDX, DATA, ByteAcc, NoLock, Preserve) { + A, 8, + B, 8, + C, 8 + } + + Method(MAIN, 0, NotSerialized) { + C = 0xA5 + A = 0x5A + B = A + C = A + Return (0) + } +} +"#; + + const EXPECTED_COMMANDS: &[Command] = &[ + create_mutex(), + // C = 0xA5 + write_u8(0x40010, 0x02), // Set index to point at C + write_u8(0x40011, 0xA5), // Set C = 0xA5 + // A = 0x5A + write_u8(0x40010, 0x00), // Set index to point at A + write_u8(0x40011, 0x5A), // Set A = 0x5A + // Read A + write_u8(0x40010, 0x00), // etc. + read_u8(0x40011, 0x5A), + // B = A + write_u8(0x40010, 0x01), + write_u8(0x40011, 0x5A), + // Read A + write_u8(0x40010, 0x00), + read_u8(0x40011, 0x5A), + // C = A + write_u8(0x40010, 0x02), + write_u8(0x40011, 0x5A), + ]; + + let handler = construct_std_handler(EXPECTED_COMMANDS.to_vec()); + test_infra::run_aml_test(AML, handler); +} + +#[test] +fn test_basic_index_store_and_load_16_bit() { + // In this test, the data register has the same width as the fields and all fields are correctly + // aligned. We should see single reads and writes for each store operation. + const AML: &str = r#"DefinitionBlock("", "DSDT", 1, "RSACPI", "IDXFLD", 1) { + OperationRegion(MEM, SystemMemory, 0x40000, 0x1000) + Field(MEM, WordAcc, NoLock, Preserve) { + OFFSET(0x20), + INDX, 16, + DATA, 16 + } + + IndexField (INDX, DATA, WordAcc, NoLock, Preserve) { + A, 16, + B, 16, + C, 16 + } + + Method(MAIN, 0, NotSerialized) { + C = 0xA5B6 + A = 0x5A6B + B = A + C = A + Return (0) + } +} +"#; + + const EXPECTED_COMMANDS: &[Command] = &[ + create_mutex(), + // C = 0xA5B6 + write_u16(0x40020, 0x04), // Set index to point at C + write_u16(0x40022, 0xA5B6), // Set C = 0xA5B6 + // A = 0x5A6B + write_u16(0x40020, 0x00), // Set index to point at A + write_u16(0x40022, 0x5A6B), // Set A = 0x5A6B + // Read A + write_u16(0x40020, 0x00), // etc. + read_u16(0x40022, 0x5A6B), + // B = A + write_u16(0x40020, 0x02), + write_u16(0x40022, 0x5A6B), + // Read A + write_u16(0x40020, 0x00), + read_u16(0x40022, 0x5A6B), + // C = A + write_u16(0x40020, 0x04), + write_u16(0x40022, 0x5A6B), + ]; + + let handler = construct_std_handler(EXPECTED_COMMANDS.to_vec()); + test_infra::run_aml_test(AML, handler); +} + +#[test] +fn test_index_multiple_aligned_reads() { + // In this test, the data register is narrower than the fields, so multiple data register + // reads are needed to access each field. + const AML: &str = r#"DefinitionBlock("", "DSDT", 1, "RSACPI", "IDXFLD", 1) { + OperationRegion(MEM, SystemMemory, 0x40000, 0x1000) + Field(MEM, ByteAcc, NoLock, Preserve) { + OFFSET(0x04), + INDX, 8, + DATA, 8 + } + + IndexField (INDX, DATA, ByteAcc, NoLock, Preserve) { + A, 16, + B, 16, + C, 16 + } + + Method(MAIN, 0, NotSerialized) { + C = 0xA5B6 + B = C + Return (0) + } +} +"#; + + const EXPECTED_COMMANDS: &[Command] = &[ + create_mutex(), + // C = 0xA5B6 + write_u8(0x40004, 0x04), // Set index to point at C (low byte) + write_u8(0x40005, 0xB6), // Set low C = 0xB6 + write_u8(0x40004, 0x05), // Set index to point at C (high byte) + write_u8(0x40005, 0xA5), // Set low C = 0xA5 + // B = C. Read C + write_u8(0x40004, 0x04), + read_u8(0x40005, 0xB6), + write_u8(0x40004, 0x05), + read_u8(0x40005, 0xA5), + // Write B + write_u8(0x40004, 0x02), + write_u8(0x40005, 0xB6), + write_u8(0x40004, 0x03), + write_u8(0x40005, 0xA5), + ]; + + let handler = construct_std_handler(EXPECTED_COMMANDS.to_vec()); + test_infra::run_aml_test(AML, handler); +} + +#[test] +fn test_index_narrower_than_data() { + // In this test, the access width of the index field is smaller than the data register. Even + // though it looks as though individual 16-bit reads/writes would be OK, actually multiple + // 8-bit reads/writes are needed to access each field. (Not intuitive, but matches ACPICA) + const AML: &str = r#"DefinitionBlock("", "DSDT", 1, "RSACPI", "IDXFLD", 1) { + OperationRegion(MEM, SystemMemory, 0x40000, 0x1000) + Field(MEM, WordAcc, NoLock, Preserve) { + OFFSET(0x06), + INDX, 16, + DATA, 16 + } + + IndexField (INDX, DATA, ByteAcc, NoLock, Preserve) { + A, 16, + B, 16, + C, 16 + } + + Method(MAIN, 0, NotSerialized) { + C = 0xA5B6 + B = C + Return (0) + } +} +"#; + + const EXPECTED_COMMANDS: &[Command] = &[ + create_mutex(), + // C = 0xA5B6 + write_u16(0x40006, 0x04), // Set index to point at C, low byte + write_u8(0x40008, 0xB6), // Set C (low) = 0xB6 + write_u16(0x40006, 0x05), // Set index to point at C, high byte + write_u8(0x40008, 0xA5), // Set C (high) = 0xA5 + // Read C + write_u16(0x40006, 0x04), // etc. + read_u8(0x40008, 0xB6), + write_u16(0x40006, 0x05), + read_u8(0x40008, 0xA5), + // B = C + write_u16(0x40006, 0x02), + write_u8(0x40008, 0xB6), + write_u16(0x40006, 0x03), + write_u8(0x40008, 0xA5), + ]; + + //let handler = NullHandler; + let handler = construct_std_handler(EXPECTED_COMMANDS.to_vec()); + test_infra::run_aml_test(AML, handler); +} + +#[test] +fn test_index_misaligned_field() { + // Check that we can successfully update non-aligned index fields. + const AML: &str = r#"DefinitionBlock("", "DSDT", 1, "RSACPI", "IDXFLD", 1) { + OperationRegion(MEM, SystemMemory, 0x40000, 0x1000) + Field(MEM, ByteAcc, NoLock, Preserve) { + OFFSET(0x08), + INDX, 8, + DATA, 8 + } + + IndexField (INDX, DATA, ByteAcc, NoLock, Preserve) { + SKIP, 4, // Nybbles make it easier to model mentally! + A, 8, + B, 8 + } + + Method(MAIN, 0, NotSerialized) { + B = 0xB6 + A = B + Return (0) + } +} +"#; + + const EXPECTED_COMMANDS: &[Command] = &[ + create_mutex(), + // B = 0xB6 + // Set index to point at B, low nybble. Which is actually the byte containing A's high + // nybble as well. + write_u8(0x40008, 0x01), + // Read that byte so as to be able to preserve A's high nybble. Pretend that A's high nybble + // is 0xA. + read_u8(0x40009, 0x0A), + // Write back A's high nybble and B's low nybble. This is as much as we test Preserve + // behaviour in the index field tests - it's assumed that if it works for normal fields, it + // works for index fields as well. + write_u8(0x40009, 0x6A), + // Set the index to point at B, high nybble. + write_u8(0x40008, 0x02), + // Read that byte, which also contains an unused nybble. (Which we've just set to zero) + read_u8(0x40009, 0x00), + // Write B's high nybble, preserving the unused zero nybble. + write_u8(0x40009, 0x0B), + // A = B. Start by reading B + write_u8(0x40008, 0x01), + read_u8(0x40009, 0x60), + write_u8(0x40008, 0x02), + read_u8(0x40009, 0x0B), + // Set A, remembering that there are some reads needed to preserve the other nybbles. + write_u8(0x40008, 0x00), + read_u8(0x40009, 0x00), + write_u8(0x40009, 0x60), + write_u8(0x40008, 0x01), + read_u8(0x40009, 0x60), + write_u8(0x40009, 0x6B), + ]; + + let handler = construct_std_handler(EXPECTED_COMMANDS.to_vec()); + test_infra::run_aml_test(AML, handler); +}