diff --git a/lib/propolis/src/firmware/acpi/dsdt.rs b/lib/propolis/src/firmware/acpi/dsdt.rs index acb40a5b1..fc329edc3 100644 --- a/lib/propolis/src/firmware/acpi/dsdt.rs +++ b/lib/propolis/src/firmware/acpi/dsdt.rs @@ -13,6 +13,14 @@ //! //! Structs that implement the [`DsdtGenerator`] trait can be passed to //! [`DsdtConfig`] and their AML code will be added to the scope they selected. +//! [`DsdtGenerator`] implementations can also use [`DsdtDeviceType`] to +//! indicate that the code being generated represents a specific type of +//! device that should be placed in specific parts of the table. +//! +//! # Panics +//! +//! Table generation panics if a [`DsdtGenerator`] is defined in [`DsdtConfig`] +//! but it is not used during table generation. // The DSDT and SSDT tables generated here are kept consistent with the // original EDK2 static tables. @@ -26,6 +34,8 @@ // // This pattern is already used for some devices when possible. +use std::cell::RefCell; + use super::aml::{devids, methods, names, paths, *}; use super::{ AcpiVariant, GPE0_BLK_ADDR, GPE0_BLK_LEN, IO_APIC_ADDR, IO_APIC_LEN, @@ -50,6 +60,13 @@ pub enum DsdtScope { Lpc, // \_SB.PCI0.LPC scope. } +/// The types of devices represented in specific parts of the DSDT. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum DsdtDeviceType { + PS2Ctrl, + Uart, +} + /// An implementer of DsdtGenerator is able to generate AML code to be loaded /// into the DSDT ACPI table. pub trait DsdtGenerator { @@ -57,6 +74,16 @@ pub trait DsdtGenerator { /// should be placed. fn dsdt_scope(&self) -> DsdtScope; + /// Returns the device type this generator represents. + /// + /// This value can be used to place the generated code into specific + /// positions in the DSDT. + /// + /// If `None`, the generated code is not guaranteed have a stable position. + fn device_type(&self) -> Option { + None + } + fn to_aml_bytes( &self, acpi_variant: AcpiVariant, @@ -68,19 +95,26 @@ pub trait DsdtGenerator { /// Wraps a list of DsdtGenerators to help generate their AML code in places /// where a `&dyn Aml` is needed. struct DsdtGeneratorAml<'a> { + dsdt: &'a Dsdt<'a>, scope: DsdtScope, - config: &'a DsdtConfig<'a>, + device_type: Option, } impl<'a> Aml for DsdtGeneratorAml<'a> { fn to_aml_bytes(&self, sink: &mut dyn AmlSink) { - self.config + self.dsdt + .config .generators .iter() - .filter(|&&g| g.dsdt_scope() == self.scope) - .for_each(|&g| { + .enumerate() + .filter(|&(_, &g)| { + g.dsdt_scope() == self.scope + && g.device_type() == self.device_type + }) + .for_each(|(i, &g)| { + self.dsdt.generator_used(i); g.to_aml_bytes( - self.config.acpi_variant, - self.config.device_metadata, + self.dsdt.config.acpi_variant, + self.dsdt.config.device_metadata, sink, ) }); @@ -116,11 +150,40 @@ pub struct DsdtConfig<'a> { /// ACPI rev. 6.6 section 5.2.11.1 "Differentiated System Description Table (DSDT)" pub struct Dsdt<'a> { config: DsdtConfig<'a>, + + /// Track which generators have been used during table generation to ensure + /// all of them are called at least once. + /// + /// Table generation is a sequential single-threaded process that writes + /// to a common sink, so generators are guaranteed to be called (and + /// `borrow_mut()` this `RefCell`) one at a time. + generators_used: RefCell>, } impl<'a> Dsdt<'a> { pub fn new(config: DsdtConfig<'a>) -> Self { - Self { config } + let generators_used = + RefCell::new(vec![false; config.generators.len()]); + Self { config, generators_used } + } + + fn generator_used(&self, i: usize) { + self.generators_used.borrow_mut()[i] = true + } +} + +impl<'a> Drop for Dsdt<'a> { + fn drop(&mut self) { + let leftovers: Vec<_> = self + .generators_used + .borrow() + .iter() + .enumerate() + .filter_map(|(i, &used)| if used { None } else { Some(i) }) + .collect(); + if leftovers.len() > 0 { + panic!("DSDT generators not called: {:?}", leftovers); + } } } @@ -152,10 +215,11 @@ impl<'a> Aml for Dsdt<'a> { aml::Scope::new( "_SB_".into(), vec![ - &PciRootBridge { config: &self.config }, + &PciRootBridge { dsdt: &self }, &DsdtGeneratorAml { + dsdt: &self, scope: DsdtScope::SystemBus, - config: &self.config, + device_type: None, }, ], ) @@ -170,7 +234,7 @@ impl<'a> Aml for Dsdt<'a> { /// PCI root bridge namespace (\_SB.PCI0). struct PciRootBridge<'a> { - config: &'a DsdtConfig<'a>, + dsdt: &'a Dsdt<'a>, } impl<'a> Aml for PciRootBridge<'a> { @@ -184,10 +248,11 @@ impl<'a> Aml for PciRootBridge<'a> { &names::uid(&aml::ZERO), &PciRootBridgeCrs {}, &PciRootBridgePrt {}, - &PciRootBridgeLpc { config: self.config }, + &PciRootBridgeLpc { dsdt: self.dsdt }, &DsdtGeneratorAml { + dsdt: self.dsdt, scope: DsdtScope::PciRoot, - config: self.config, + device_type: None, }, ], ) @@ -538,7 +603,7 @@ const NMISC_PORT: u16 = 0x61; /// /// struct PciRootBridgeLpc<'a> { - config: &'a DsdtConfig<'a>, + dsdt: &'a Dsdt<'a>, } // This table is currently kept the same as the original EDK2 static table, but @@ -663,7 +728,7 @@ impl<'a> Aml for PciRootBridgeLpc<'a> { // table. Its IO ports are not actually handled anywhere. // It can be removed in the future . &AcpiVariantFilter::new( - self.config.acpi_variant, + self.dsdt.config.acpi_variant, vec![AcpiVariant::V0], &aml::Device::new( "DMAC".into(), @@ -730,7 +795,7 @@ impl<'a> Aml for PciRootBridgeLpc<'a> { // table. Its IO ports are not actually handled anywhere. // It can be removed in the future . &AcpiVariantFilter::new( - self.config.acpi_variant, + self.dsdt.config.acpi_variant, vec![AcpiVariant::V0], &aml::Device::new( "FPU_".into(), @@ -804,8 +869,14 @@ impl<'a> Aml for PciRootBridgeLpc<'a> { ], ), &DsdtGeneratorAml { + dsdt: self.dsdt, scope: DsdtScope::Lpc, - config: self.config, + device_type: Some(DsdtDeviceType::PS2Ctrl), + }, + &DsdtGeneratorAml { + dsdt: self.dsdt, + scope: DsdtScope::Lpc, + device_type: Some(DsdtDeviceType::Uart), }, // QEMU panic device. // @@ -859,6 +930,11 @@ impl<'a> Aml for PciRootBridgeLpc<'a> { ), ], ), + &DsdtGeneratorAml { + dsdt: self.dsdt, + scope: DsdtScope::Lpc, + device_type: None, + }, ], ) .to_aml_bytes(sink); @@ -1043,12 +1119,17 @@ mod test { struct MockDsdtGenerator { scope: DsdtScope, + device_type: Option, } impl DsdtGenerator for MockDsdtGenerator { fn dsdt_scope(&self) -> DsdtScope { self.scope } + fn device_type(&self) -> Option { + self.device_type + } + fn to_aml_bytes( &self, acpi_variant: AcpiVariant, @@ -1089,11 +1170,20 @@ mod test { #[test] fn dsdt_generator_aml() { - let sb = Arc::new(MockDsdtGenerator { scope: DsdtScope::SystemBus }); - let pci = Arc::new(MockDsdtGenerator { scope: DsdtScope::PciRoot }); - let lpc = Arc::new(MockDsdtGenerator { scope: DsdtScope::Lpc }); - - let generators: Vec<&dyn DsdtGenerator> = vec![&*sb, &*pci, &*lpc]; + let sb = Arc::new(MockDsdtGenerator { + scope: DsdtScope::SystemBus, + device_type: None, + }); + let pci = Arc::new(MockDsdtGenerator { + scope: DsdtScope::PciRoot, + device_type: None, + }); + let lpc_ps2 = Arc::new(MockDsdtGenerator { + scope: DsdtScope::Lpc, + device_type: Some(DsdtDeviceType::PS2Ctrl), + }); + + let generators: Vec<&dyn DsdtGenerator> = vec![&*sb, &*pci, &*lpc_ps2]; let mut got = Vec::new(); let mut expected = Vec::new(); @@ -1104,17 +1194,21 @@ mod test { device_metadata .insert(&pci, Box::new(MockDsdtGeneratorMetadata { data: 2 })); device_metadata - .insert(&lpc, Box::new(MockDsdtGeneratorMetadata { data: 3 })); + .insert(&lpc_ps2, Box::new(MockDsdtGeneratorMetadata { data: 3 })); - let config = DsdtConfig { + let dsdt = Dsdt::new(DsdtConfig { acpi_variant: AcpiVariant::V0, generators: &generators, device_metadata: &device_metadata, - }; + }); // Filter by SystemBus. - DsdtGeneratorAml { scope: DsdtScope::SystemBus, config: &config } - .to_aml_bytes(&mut got); + DsdtGeneratorAml { + dsdt: &dsdt, + scope: DsdtScope::SystemBus, + device_type: None, + } + .to_aml_bytes(&mut got); aml::ResourceTemplate::new(vec![ &aml::Name::new("SCOP".into(), &"SystemBus"), @@ -1129,8 +1223,12 @@ mod test { got.clear(); expected.clear(); - DsdtGeneratorAml { scope: DsdtScope::PciRoot, config: &config } - .to_aml_bytes(&mut got); + DsdtGeneratorAml { + dsdt: &dsdt, + scope: DsdtScope::PciRoot, + device_type: None, + } + .to_aml_bytes(&mut got); aml::ResourceTemplate::new(vec![ &aml::Name::new("SCOP".into(), &"PciRoot"), @@ -1141,12 +1239,24 @@ mod test { assert_eq!(expected, got); - // Filter by Lpc. + // Filter by Lpc scope and device type. got.clear(); expected.clear(); - DsdtGeneratorAml { scope: DsdtScope::Lpc, config: &config } - .to_aml_bytes(&mut got); + DsdtGeneratorAml { + dsdt: &dsdt, + scope: DsdtScope::Lpc, + device_type: Some(DsdtDeviceType::PS2Ctrl), + } + .to_aml_bytes(&mut got); + + // Filter on device type without generators. + DsdtGeneratorAml { + dsdt: &dsdt, + scope: DsdtScope::Lpc, + device_type: Some(DsdtDeviceType::Uart), + } + .to_aml_bytes(&mut got); aml::ResourceTemplate::new(vec![ &aml::Name::new("SCOP".into(), &"Lpc"), @@ -1158,18 +1268,49 @@ mod test { assert_eq!(expected, got); } + #[test] + #[should_panic(expected = "DSDT generators not called")] + fn panic_if_generator_not_called() { + let lpc_uart = Arc::new(MockDsdtGenerator { + scope: DsdtScope::Lpc, + device_type: Some(DsdtDeviceType::Uart), + }); + let generators: Vec<&dyn DsdtGenerator> = vec![&*lpc_uart]; + let device_metadata = DeviceMetadataMap::new(); + let dsdt = Dsdt::new(DsdtConfig { + acpi_variant: AcpiVariant::V0, + generators: &generators, + device_metadata: &device_metadata, + }); + + // Generate AML code without calling the lpc_uart generator. + let mut sink = Vec::new(); + DsdtGeneratorAml { + dsdt: &dsdt, + scope: DsdtScope::SystemBus, + device_type: None, + } + .to_aml_bytes(&mut sink); + } + #[test] fn dsdt_valid_aml() { - let config = DsdtConfig { + let device_metadata = DeviceMetadataMap::new(); + let dsdt = Dsdt::new(DsdtConfig { acpi_variant: AcpiVariant::V0, generators: &[ - &MockDsdtGenerator { scope: DsdtScope::SystemBus }, - &MockDsdtGenerator { scope: DsdtScope::PciRoot }, - &MockDsdtGenerator { scope: DsdtScope::Lpc }, + &MockDsdtGenerator { + scope: DsdtScope::SystemBus, + device_type: None, + }, + &MockDsdtGenerator { + scope: DsdtScope::PciRoot, + device_type: None, + }, + &MockDsdtGenerator { scope: DsdtScope::Lpc, device_type: None }, ], - device_metadata: &DeviceMetadataMap::new(), - }; - let dsdt = Dsdt::new(config); + device_metadata: &device_metadata, + }); let mut aml = Vec::new(); dsdt.to_aml_bytes(&mut aml); diff --git a/lib/propolis/src/firmware/acpi/mod.rs b/lib/propolis/src/firmware/acpi/mod.rs index 84340bd76..f5f5330fd 100644 --- a/lib/propolis/src/firmware/acpi/mod.rs +++ b/lib/propolis/src/firmware/acpi/mod.rs @@ -32,7 +32,7 @@ pub mod rsdp; pub mod ssdt_edk2; pub mod xsdt; -pub use dsdt::{Dsdt, DsdtConfig, DsdtGenerator, DsdtScope}; +pub use dsdt::{Dsdt, DsdtConfig, DsdtDeviceType, DsdtGenerator, DsdtScope}; pub use facs::{Facs, FacsConfig}; pub use fadt::{ Fadt, FadtConfig, FADT_DSDT_LEN, FADT_DSDT_OFFSET, FADT_FACS_LEN, diff --git a/lib/propolis/src/hw/ps2/ctrl.rs b/lib/propolis/src/hw/ps2/ctrl.rs index 43d4937fc..a1c10f777 100644 --- a/lib/propolis/src/hw/ps2/ctrl.rs +++ b/lib/propolis/src/hw/ps2/ctrl.rs @@ -714,6 +714,10 @@ impl acpi::DsdtGenerator for PS2Ctrl { acpi::DsdtScope::Lpc } + fn device_type(&self) -> Option { + Some(acpi::DsdtDeviceType::PS2Ctrl) + } + fn to_aml_bytes( &self, _: acpi::AcpiVariant, diff --git a/lib/propolis/src/hw/uart/lpc.rs b/lib/propolis/src/hw/uart/lpc.rs index 8e9a6c37b..4e3da8063 100644 --- a/lib/propolis/src/hw/uart/lpc.rs +++ b/lib/propolis/src/hw/uart/lpc.rs @@ -208,6 +208,10 @@ impl acpi::DsdtGenerator for LpcUart { acpi::DsdtScope::Lpc } + fn device_type(&self) -> Option { + Some(acpi::DsdtDeviceType::Uart) + } + // This AML code is inherited from the original EDK2 static tables. fn to_aml_bytes( &self, diff --git a/phd-tests/tests/testdata/acpi/v0/dsdt.dat b/phd-tests/tests/testdata/acpi/v0/dsdt.dat index fa6c52316..a25050b9a 100644 Binary files a/phd-tests/tests/testdata/acpi/v0/dsdt.dat and b/phd-tests/tests/testdata/acpi/v0/dsdt.dat differ