acpi: add DSDT device type #1165
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,13 +50,30 @@ 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 { | ||
| /// Returns the scope of the DSDT table in which the generated AML code | ||
| /// 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<DsdtDeviceType> { | ||
| None | ||
| } | ||
|
|
||
| fn to_aml_bytes( | ||
| &self, | ||
| acpi_variant: AcpiVariant, | ||
|
|
@@ -69,14 +86,18 @@ pub trait DsdtGenerator { | |
| /// where a `&dyn Aml` is needed. | ||
| struct DsdtGeneratorAml<'a> { | ||
| scope: DsdtScope, | ||
| device_type: Option<DsdtDeviceType>, | ||
| config: &'a DsdtConfig<'a>, | ||
| } | ||
| impl<'a> Aml for DsdtGeneratorAml<'a> { | ||
| fn to_aml_bytes(&self, sink: &mut dyn AmlSink) { | ||
| self.config | ||
| .generators | ||
| .iter() | ||
| .filter(|&&g| { | ||
| g.dsdt_scope() == self.scope | ||
| && g.device_type() == self.device_type | ||
| }) | ||
| .for_each(|&g| { | ||
| g.to_aml_bytes( | ||
| self.config.acpi_variant, | ||
|
|
@@ -155,6 +176,7 @@ impl<'a> Aml for Dsdt<'a> { | |
| &PciRootBridge { config: &self.config }, | ||
| &DsdtGeneratorAml { | ||
| scope: DsdtScope::SystemBus, | ||
| device_type: None, | ||
|
Member
There was a problem hiding this comment. the main thought i've got here is: while i think this gives Propolis good control over how to lay out items in the tables (and so, broadly, i'm a fan!) if we have a device on i'm imagining that maybe we could have a bit on devices to report if we've printed their AML, though
Member
Author
There was a problem hiding this comment. Yeah, that's a good point. Is 8d38895 more or less what you had in mind?
Member
Author
There was a problem hiding this comment. Looking at this with fresh eyes today, I think it makes more sense to keep state in |
||
| config: &self.config, | ||
| }, | ||
| ], | ||
|
|
@@ -187,6 +209,7 @@ impl<'a> Aml for PciRootBridge<'a> { | |
| &PciRootBridgeLpc { config: self.config }, | ||
| &DsdtGeneratorAml { | ||
| scope: DsdtScope::PciRoot, | ||
| device_type: None, | ||
| config: self.config, | ||
| }, | ||
| ], | ||
|
|
@@ -805,6 +828,12 @@ impl<'a> Aml for PciRootBridgeLpc<'a> { | |
| ), | ||
| &DsdtGeneratorAml { | ||
| scope: DsdtScope::Lpc, | ||
| device_type: Some(DsdtDeviceType::PS2Ctrl), | ||
| config: self.config, | ||
| }, | ||
| &DsdtGeneratorAml { | ||
| scope: DsdtScope::Lpc, | ||
| device_type: Some(DsdtDeviceType::Uart), | ||
| config: self.config, | ||
| }, | ||
| // QEMU panic device. | ||
|
|
@@ -859,6 +888,11 @@ impl<'a> Aml for PciRootBridgeLpc<'a> { | |
| ), | ||
| ], | ||
| ), | ||
| &DsdtGeneratorAml { | ||
| scope: DsdtScope::Lpc, | ||
| device_type: None, | ||
| config: self.config, | ||
| }, | ||
| ], | ||
| ) | ||
| .to_aml_bytes(sink); | ||
|
|
@@ -1043,12 +1077,17 @@ mod test { | |
|
|
||
| struct MockDsdtGenerator { | ||
| scope: DsdtScope, | ||
| device_type: Option<DsdtDeviceType>, | ||
| } | ||
| impl DsdtGenerator for MockDsdtGenerator { | ||
| fn dsdt_scope(&self) -> DsdtScope { | ||
| self.scope | ||
| } | ||
|
|
||
| fn device_type(&self) -> Option<DsdtDeviceType> { | ||
| self.device_type | ||
| } | ||
|
|
||
| fn to_aml_bytes( | ||
| &self, | ||
| acpi_variant: AcpiVariant, | ||
|
|
@@ -1089,11 +1128,20 @@ mod test { | |
|
|
||
| #[test] | ||
| fn dsdt_generator_aml() { | ||
| 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,7 +1152,7 @@ mod test { | |
| device_metadata | ||
| .insert(&pci, Box::new(MockDsdtGeneratorMetadata { data: 2 })); | ||
| device_metadata | ||
| .insert(&lpc_ps2, Box::new(MockDsdtGeneratorMetadata { data: 3 })); | ||
|
|
||
| let config = DsdtConfig { | ||
| acpi_variant: AcpiVariant::V0, | ||
|
|
@@ -1113,8 +1161,12 @@ mod test { | |
| }; | ||
|
|
||
| // Filter by SystemBus. | ||
| DsdtGeneratorAml { | ||
| scope: DsdtScope::SystemBus, | ||
| device_type: None, | ||
| config: &config, | ||
| } | ||
| .to_aml_bytes(&mut got); | ||
|
|
||
| aml::ResourceTemplate::new(vec![ | ||
| &aml::Name::new("SCOP".into(), &"SystemBus"), | ||
|
|
@@ -1129,8 +1181,12 @@ mod test { | |
| got.clear(); | ||
| expected.clear(); | ||
|
|
||
| DsdtGeneratorAml { | ||
| scope: DsdtScope::PciRoot, | ||
| device_type: None, | ||
| config: &config, | ||
| } | ||
| .to_aml_bytes(&mut got); | ||
|
|
||
| aml::ResourceTemplate::new(vec![ | ||
| &aml::Name::new("SCOP".into(), &"PciRoot"), | ||
|
|
@@ -1141,12 +1197,24 @@ mod test { | |
|
|
||
| assert_eq!(expected, got); | ||
|
|
||
| // Filter by Lpc scope and device type. | ||
| got.clear(); | ||
| expected.clear(); | ||
|
|
||
| DsdtGeneratorAml { | ||
| scope: DsdtScope::Lpc, | ||
| device_type: Some(DsdtDeviceType::PS2Ctrl), | ||
| config: &config, | ||
| } | ||
| .to_aml_bytes(&mut got); | ||
|
|
||
| // Filter on device type without generators. | ||
| DsdtGeneratorAml { | ||
| scope: DsdtScope::Lpc, | ||
| device_type: Some(DsdtDeviceType::Uart), | ||
| config: &config, | ||
| } | ||
| .to_aml_bytes(&mut got); | ||
|
|
||
| aml::ResourceTemplate::new(vec![ | ||
| &aml::Name::new("SCOP".into(), &"Lpc"), | ||
|
|
@@ -1163,9 +1231,15 @@ mod test { | |
| let config = DsdtConfig { | ||
| acpi_variant: AcpiVariant::V0, | ||
| generators: &[ | ||
| &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(), | ||
| }; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Binary file not shown.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so.. I'm realizing one wrinkle we'll still have is that we're at the whim of SpecKey ordering for a given
device_type. that is, if something sendspropolis-serveran InstanceSpec which has four serial ports namedcom{1,2,3,4}, then changes how it adds serial ports such that they're namedcom{4,3,2,1}but name ports in ascending order, we'll dutifully reverse the order of those ports' AML.we can probably demand that items implement
Ordso we can sort the filtered generators? but this is all pretty niche compared to the current issue you're fixing. and it's not like we have anything else that's present multiple times in the ACPI tables. so as emphatically as I can say: I think we should land this with an issue and note about the remainingSpecKeysensitivityThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum...right. Although the generators are sorted by
SpecKey, so you would need to have a weird combination where the serial port key didn't quite match the port number. Weird, but still possible 😄I added a note about it in a2f33de and I will open a follow-up issue.