Implement Index and Bank fields

Author: martin-hughes

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 };
 
             /*

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);
+}