Resolve names used as package elements lazily - #320
Conversation
Namespace entry points require absolute AML names, but some callers can pass relative names from firmware input. Return an error instead of asserting so bad paths propagate as normal interpreter errors. Add `AmlName::require_absolute` and `AmlName::normalize_absolute`, so the check cannot be forgotten by a new entry point that normalizes a path, and a dedicated `AmlError::NameNotAbsolute` variant: `InvalidNormalizedName` means normalization itself failed, which is a different condition. Note that this changes the contract of several public functions from panicking to returning an error. Cover namespace insertion, removal, lookup, alias creation, both scope searches and `AmlName::resolve` with regression tests for relative inputs.
Pure refactor with no functional change: move the bodies of the `ObjectType` and `DerefOf` opcode arms out of the main interpreter loop, so the following commit can extend them without further growing the loop.
Package NameStrings can refer to objects that are defined later, or in another table. Represent such an element as a reference to a new `Object::NamePath`, which retains the name and the scope it was declared in, and resolve it when the reference is used. Resolution is then independent of the order tables are loaded in, and of the scope the package is used from. Resolution goes through a single `resolve_name_path` helper, used by `DerefOf`, `ObjectType` and `SizeOf`, which follows a bounded number of indirections so a malformed table cannot make us loop. The `DerefOf` of a string stays a single namespace lookup for the same reason. Package elements were previously `String` objects, so also decode the `Source` field of a `_PRT` entry from a name reference. Strings are still accepted there, as some tables store the path as one.
Cover the AMD Rhino gpio-leds _DSD that motivated the previous commit, a forward reference consumed from another scope, _PRT sources naming a link device both absolutely and by search rules, and that dereferencing a self-referential string terminates.
|
Thanks Arthur! I've taken a quick first pass and I think it looks good - but I probably won't get a chance to do a thorough review for a couple of days. There are some obvious positives from what I've seen. I feel like positives aren't often brought up in OSS code reviews - which is a shame - so I'm particularly liking:
@IsaacWoods has recently given me merge permissions, but since this would be my first merge to the crate I'd like to check that this falls within the scope he'd expect from me - please bear with us. |
| (*interpreter.evaluate(AmlName::from_str(path).unwrap(), vec![]).unwrap()).clone() | ||
| } | ||
|
|
||
| /// A cut-down version of the `gpio-leds` `_DSD` from the AMD Rhino, which refers to the device the |
There was a problem hiding this comment.
Purely because I'm curious - what's an "AMD Rhino"? Google unhelpfully tells me "it's probably a codename"...
There was a problem hiding this comment.
Yes it's a reference board from AMD for automotive.
A collegue of mine wanted try out my UEFI (CrabEFI) implementation which uses this crate to extract non PCI resources (there is an eMMC ACPI device).
I can scrub the test of the name of it causes confusion.
2 participants
Names in packages currently evaluate to
Stringobjects, which loses the scope the name appearedin. That works for absolute paths, but a relative name (
^LEDS) or a bare NameSeg relying on thesearch rules can't be resolved from wherever the package is eventually used. Here is an extract from AMD reference DSDT
gpio-leds_DSD:Such a name now evaluates to a reference to a new
Object::NamePath, keeping the name and itsdeclaration scope, resolved when the reference is used. Resolution is then also independent of table
load order (only for package elements - the
Externalcase in #290 is unaffected)._PRTdecoding is updated to match, and still accepts strings.ObjectandAmlErrorgainvariants, so this is a breaking change for downstream matches.