Merge #13

13: Remove constructed_tests and reorganise testcase creation under modules r=IsaacWoods a=IsaacWoods

This leaves the library completely untested. Obvs that needs to be fixed. I'll test changes locally on QEMU until that's set up.

Co-authored-by: Isaac Woods <isaacwoods.home@gmail.com>

src/constructed_tables_test.rs

@@ -1,128 +0,0 @@
-use super::{fadt::Fadt, parse_rsdp, rsdp::Rsdp, sdt::SdtHeader, AcpiHandler, PhysicalMapping};
-use std::boxed::Box;
-/// These tests cover ideal sets of ACPI tables, which we construct on the fly. Eventually, this
-/// should cover all compliant implementations, but does not guarantee we'll be able to parse a
-/// particular hardware's tables, obviously.
-use std::mem;
-use std::ptr::NonNull;
-
-const OEM_ID: &[u8; 6] = b"RUST  ";
-
-/*
- * We use fake physical addresses to track what is being requested. When a particular table or
- * resource is requested, we just allocate it on the heap and return the "virtual address"
- * (a pointer onto the heap).
- */
-const RSDP_ADDRESS: usize = 0x0;
-const RSDT_ADDRESS: usize = 0x1;
-const FADT_ADDRESS: usize = 0x2;
-
-#[repr(C, packed)]
-struct TestRsdt {
-    header: SdtHeader,
-    fadt: u32,
-    // TODO: We should probably actually add some SDTs
-}
-
-struct TestHandler;
-
-impl AcpiHandler for TestHandler {
-    fn map_physical_region<T>(&mut self, physical_address: usize) -> PhysicalMapping<T> {
-        match physical_address {
-            RSDP_ADDRESS => {
-                let rsdp = Box::new(Rsdp::make_testcase(
-                    *b"RSD PTR ",
-                    None,
-                    *OEM_ID,
-                    0,
-                    RSDT_ADDRESS as u32,
-                    0,
-                    0x0,
-                    None,
-                    [0, 0, 0],
-                ));
-
-                PhysicalMapping {
-                    physical_start: RSDP_ADDRESS,
-                    virtual_start: unsafe {
-                        NonNull::<T>::new_unchecked(Box::into_raw(rsdp) as *mut T)
-                    },
-                    region_length: mem::size_of::<Rsdp>(),
-                    mapped_length: mem::size_of::<Rsdp>(),
-                }
-            }
-
-            RSDT_ADDRESS => {
-                let checksum = 0; // TODO: calculate real checksum
-                let rsdt = Box::new(TestRsdt {
-                    header: SdtHeader::make_testcase(
-                        *b"RSDT",
-                        mem::size_of::<TestRsdt>() as u32,
-                        0,
-                        checksum,
-                        *OEM_ID,
-                        *b"OEMRSDT ",
-                        0xDEADBEEF,
-                        0xDEADBEEF,
-                        0xDEADBEEF,
-                    ),
-                    fadt: FADT_ADDRESS as u32,
-                });
-
-                PhysicalMapping {
-                    physical_start: RSDT_ADDRESS,
-                    virtual_start: unsafe {
-                        NonNull::<T>::new_unchecked(Box::into_raw(rsdt) as *mut T)
-                    },
-                    region_length: mem::size_of::<TestRsdt>(),
-                    mapped_length: mem::size_of::<TestRsdt>(),
-                }
-            }
-            FADT_ADDRESS => {
-                let fadt = Box::new(Fadt::make_testcase(
-                    *OEM_ID,
-                    *b"OEMFADT ",
-                    0xDEADBEEF,
-                    0xDEADBEEF,
-                    0xDEADBEEF,
-                ));
-                PhysicalMapping {
-                    physical_start: FADT_ADDRESS,
-                    virtual_start: unsafe {
-                        NonNull::<T>::new_unchecked(Box::into_raw(fadt) as *mut T)
-                    },
-                    region_length: mem::size_of::<Fadt>(),
-                    mapped_length: mem::size_of::<Fadt>(),
-                }
-            }
-
-            _ => panic!(
-                "ACPI requested invalid physical address: {:#x}",
-                physical_address
-            ),
-        }
-    }
-
-    fn unmap_physical_region<T>(&mut self, region: PhysicalMapping<T>) {
-        match region.physical_start {
-            RSDP_ADDRESS | RSDT_ADDRESS | FADT_ADDRESS => {
-                let _ = unsafe { Box::from_raw(region.virtual_start.as_ptr()) };
-            }
-
-            address => panic!(
-                "ACPI tried to unmap a region not created by test harness: {:#x}",
-                address
-            ),
-        }
-    }
-}
-
-#[test]
-fn test_constructed_tables() {
-    let mut test_handler = TestHandler;
-    match parse_rsdp(&mut test_handler, RSDP_ADDRESS) {
-        Ok(_) => {}
-
-        Err(err) => {}
-    }
-}

src/fadt.rs

@@ -1,6 +1,4 @@
 use sdt::SdtHeader;
-#[cfg(test)]
-use std::mem;
 use {AcpiError, PhysicalMapping};
 
 #[repr(C, packed)]
@@ -12,22 +10,8 @@ struct GenericAddress {
     address: u64,
 }
 
-impl GenericAddress {
-    #[cfg(test)]
-    pub(crate) fn make_testcase() -> GenericAddress {
-        GenericAddress {
-            address_space: 0 as u8,
-            bit_width: 0 as u8,
-            bit_offset: 0 as u8,
-            access_size: 0 as u8,
-            address: 0 as u64,
-        }
-    }
-}
-
 #[repr(C, packed)]
 pub struct Fadt {
-    // header
     header: SdtHeader,
 
     firmware_ctrl: u32,
@@ -90,95 +74,109 @@ pub struct Fadt {
     hypervisor_vendor_id: u64,
 }
 
-impl Fadt {
-    pub fn validate(&self) -> Result<(), AcpiError> {
-        self.header.validate(b"FACP")
-    }
+pub fn parse_fadt(mapping: &PhysicalMapping<Fadt>) -> Result<(), AcpiError> {
+    (*mapping).header.validate(b"FACP")?;
 
-    #[cfg(test)]
-    pub(crate) fn make_testcase(
-        oem_id: [u8; 6],
-        oem_table_id: [u8; 8],
-        oem_revision: u32,
-        creator_id: u32,
-        creator_revision: u32,
-    ) -> Fadt {
-        Fadt {
-            header: SdtHeader::make_testcase(
-                *b"FACP",
-                mem::size_of::<Fadt>() as u32,
-                6,
-                5, //checksum
-                oem_id,
-                oem_table_id,
-                oem_revision,
-                creator_id,
-                creator_revision,
-            ),
-            firmware_ctrl: 0xDEADBEEF as u32,
-            dsdt_address: 0xDEADBEEF as u32,
+    Ok(())
+}
 
-            // used in acpi 1.0; compatibility only, should be zero
-            reserved: 0 as u8,
+#[cfg(test)]
+mod tests {
+    use fadt::{Fadt, GenericAddress};
+    use sdt::SdtHeader;
+    use std::mem;
 
-            preferred_pm_profile: 0 as u8,
-            sci_interrupt: 0 as u16,
-            smi_cmd_port: 0 as u32,
-            acpi_enable: 0 as u8,
-            acpi_disable: 0 as u8,
-            s4bios_req: 0 as u8,
-            pstate_control: 0 as u8,
-            pm1a_event_block: 0xDEADBEEF as u32,
-            pm1b_event_block: 0xDEADBEEF as u32,
-            pm1a_control_block: 0xDEADBEEF as u32,
-            pm1b_control_block: 0xDEADBEEF as u32,
-            pm2_control_block: 0xDEADBEEF as u32,
-            pm_timer_block: 0xDEADBEEF as u32,
-            gpe0_block: 0xDEADBEEF as u32,
-            gpe1_block: 0xDEADBEEF as u32,
-            pm1_event_length: 4 as u8,
-            pm1_control_length: 2 as u8,
-            pm2_control_length: 0 as u8,
-            pm_timer_length: 0 as u8,
-            gpe0_block_length: 2 as u8,
-            gpe1_block_length: 2 as u8,
-            gpe1_base: 0 as u8,
-            c_state_control: 0 as u8,
-            worst_c2_latency: 0 as u16,
-            worst_c3_latency: 0 as u16,
-            flush_size: 0 as u16,
-            flush_stride: 0 as u16,
-            duty_offset: 0 as u8,
-            duty_width: 0 as u8,
-            day_alarm: 0 as u8,
-            month_alarm: 0 as u8,
-            century: 0 as u8,
-            iapc_boot_arch: 0 as u16,
-            reserved2: 0 as u8,
-            flags: 0 as u32,
-            reset_reg: GenericAddress::make_testcase(),
-            reset_value: 0 as u8,
-            arm_boot_arch: 0 as u16,
-            fadt_minor_version: 2 as u8,
-            x_firmware_control: 0 as u64,
-            x_dsdt_address: 0 as u64,
-            x_pm1a_event_block: GenericAddress::make_testcase(),
-            x_pm1b_event_block: GenericAddress::make_testcase(),
-            x_pm1a_control_block: GenericAddress::make_testcase(),
-            x_pm1b_control_block: GenericAddress::make_testcase(),
-            x_pm2_control_block: GenericAddress::make_testcase(),
-            x_pm_timer_block: GenericAddress::make_testcase(),
-            x_gpe0_block: GenericAddress::make_testcase(),
-            x_gpe1_block: GenericAddress::make_testcase(),
-            sleep_control_reg: GenericAddress::make_testcase(),
-            sleep_status_reg: GenericAddress::make_testcase(),
-            hypervisor_vendor_id: 0 as u64,
+    impl GenericAddress {
+        fn make_testcase() -> GenericAddress {
+            GenericAddress {
+                address_space: 0 as u8,
+                bit_width: 0 as u8,
+                bit_offset: 0 as u8,
+                access_size: 0 as u8,
+                address: 0 as u64,
+            }
         }
     }
-}
 
-pub fn parse_fadt(mapping: &PhysicalMapping<Fadt>) -> Result<(), AcpiError> {
-    (*mapping).validate()?;
+    impl Fadt {
+        fn make_testcase(
+            oem_id: [u8; 6],
+            oem_table_id: [u8; 8],
+            oem_revision: u32,
+            creator_id: u32,
+            creator_revision: u32,
+        ) -> Fadt {
+            Fadt {
+                header: SdtHeader::make_testcase(
+                    *b"FACP",
+                    mem::size_of::<Fadt>() as u32,
+                    6,
+                    5, //checksum
+                    oem_id,
+                    oem_table_id,
+                    oem_revision,
+                    creator_id,
+                    creator_revision,
+                ),
+                firmware_ctrl: 0xDEADBEEF as u32,
+                dsdt_address: 0xDEADBEEF as u32,
 
-    Ok(())
+                // used in acpi 1.0; compatibility only, should be zero
+                reserved: 0 as u8,
+
+                preferred_pm_profile: 0 as u8,
+                sci_interrupt: 0 as u16,
+                smi_cmd_port: 0 as u32,
+                acpi_enable: 0 as u8,
+                acpi_disable: 0 as u8,
+                s4bios_req: 0 as u8,
+                pstate_control: 0 as u8,
+                pm1a_event_block: 0xDEADBEEF as u32,
+                pm1b_event_block: 0xDEADBEEF as u32,
+                pm1a_control_block: 0xDEADBEEF as u32,
+                pm1b_control_block: 0xDEADBEEF as u32,
+                pm2_control_block: 0xDEADBEEF as u32,
+                pm_timer_block: 0xDEADBEEF as u32,
+                gpe0_block: 0xDEADBEEF as u32,
+                gpe1_block: 0xDEADBEEF as u32,
+                pm1_event_length: 4 as u8,
+                pm1_control_length: 2 as u8,
+                pm2_control_length: 0 as u8,
+                pm_timer_length: 0 as u8,
+                gpe0_block_length: 2 as u8,
+                gpe1_block_length: 2 as u8,
+                gpe1_base: 0 as u8,
+                c_state_control: 0 as u8,
+                worst_c2_latency: 0 as u16,
+                worst_c3_latency: 0 as u16,
+                flush_size: 0 as u16,
+                flush_stride: 0 as u16,
+                duty_offset: 0 as u8,
+                duty_width: 0 as u8,
+                day_alarm: 0 as u8,
+                month_alarm: 0 as u8,
+                century: 0 as u8,
+                iapc_boot_arch: 0 as u16,
+                reserved2: 0 as u8,
+                flags: 0 as u32,
+                reset_reg: GenericAddress::make_testcase(),
+                reset_value: 0 as u8,
+                arm_boot_arch: 0 as u16,
+                fadt_minor_version: 2 as u8,
+                x_firmware_control: 0 as u64,
+                x_dsdt_address: 0 as u64,
+                x_pm1a_event_block: GenericAddress::make_testcase(),
+                x_pm1b_event_block: GenericAddress::make_testcase(),
+                x_pm1a_control_block: GenericAddress::make_testcase(),
+                x_pm1b_control_block: GenericAddress::make_testcase(),
+                x_pm2_control_block: GenericAddress::make_testcase(),
+                x_pm_timer_block: GenericAddress::make_testcase(),
+                x_gpe0_block: GenericAddress::make_testcase(),
+                x_gpe1_block: GenericAddress::make_testcase(),
+                sleep_control_reg: GenericAddress::make_testcase(),
+                sleep_status_reg: GenericAddress::make_testcase(),
+                hypervisor_vendor_id: 0 as u64,
+            }
+        }
+    }
 }

src/lib.rs

@@ -7,8 +7,6 @@ extern crate std;
 #[macro_use]
 extern crate log;
 
-#[cfg(test)]
-mod constructed_tables_test;
 mod fadt;
 mod rsdp;
 mod sdt;
@@ -113,10 +111,7 @@ where
     H: AcpiHandler,
 {
     let mapping = handler.map_physical_region::<SdtHeader>(physical_address);
-
     // TODO: extend the mapping to header.length
-    // TODO: validate the signature and checksum (XXX: if it's a RSDT, the signature will be
-    // "RSDT", whereas a XSDT will have a signature of "XSDT"
 
     if revision == 0 {
         /*

src/rsdp.rs

@@ -98,43 +98,50 @@ impl Rsdp {
         );
         self.xsdt_address
     }
+}
 
-    /// Create a test RSDP. Checksums are passed as `Option<u8>`; if `None` is passed, the correct
-    /// checksum is calculated and used.
-    #[cfg(test)]
-    pub fn make_testcase(
-        signature: [u8; 8],
-        checksum: Option<u8>,
-        oem_id: [u8; 6],
-        revision: u8,
-        rsdt_address: u32,
-        length: u32,
-        xsdt_address: u64,
-        ext_checksum: Option<u8>,
-        reserved: [u8; 3],
-    ) -> Rsdp {
-        let checksum = checksum.unwrap_or(
-            ((0isize - signature.iter().map(|&b| isize::from(b)).sum::<isize>()
-                - oem_id.iter().map(|&b| isize::from(b)).sum::<isize>()
-                - revision as isize - rsdt_address as isize) & 0b1111_1111) as u8,
-        );
-
-        let ext_checksum = ext_checksum.unwrap_or(
-            ((0isize - length as isize - xsdt_address as isize
-                - reserved.iter().map(|&b| isize::from(b)).sum::<isize>())
-                & 0b1111_1111) as u8,
-        );
-
-        Rsdp {
-            signature,
-            checksum,
-            oem_id,
-            revision,
-            rsdt_address,
-            length,
-            xsdt_address,
-            ext_checksum,
-            reserved,
+#[cfg(test)]
+mod tests {
+    use rsdp::Rsdp;
+
+    impl Rsdp {
+        /// Create a test RSDP. Checksums are passed as `Option<u8>`; if `None` is passed, the
+        /// correct checksum is calculated and used.
+        pub fn make_testcase(
+            signature: [u8; 8],
+            checksum: Option<u8>,
+            oem_id: [u8; 6],
+            revision: u8,
+            rsdt_address: u32,
+            length: u32,
+            xsdt_address: u64,
+            ext_checksum: Option<u8>,
+            reserved: [u8; 3],
+        ) -> Rsdp {
+            let checksum = checksum.unwrap_or(
+                ((0isize - signature.iter().map(|&b| isize::from(b)).sum::<isize>()
+                    - oem_id.iter().map(|&b| isize::from(b)).sum::<isize>()
+                    - revision as isize - rsdt_address as isize) & 0b1111_1111)
+                    as u8,
+            );
+
+            let ext_checksum = ext_checksum.unwrap_or(
+                ((0isize - length as isize - xsdt_address as isize
+                    - reserved.iter().map(|&b| isize::from(b)).sum::<isize>())
+                    & 0b1111_1111) as u8,
+            );
+
+            Rsdp {
+                signature,
+                checksum,
+                oem_id,
+                revision,
+                rsdt_address,
+                length,
+                xsdt_address,
+                ext_checksum,
+                reserved,
+            }
         }
     }
 }