Resolve names used as package elements lazily by ArthurHeymans · Pull Request #320 · rust-osdev/acpi

ArthurHeymans · 2026-07-27T09:41:46Z

Names in packages currently evaluate to String objects, which loses the scope the name appeared
in. That works for absolute paths, but a relative name (^LEDS) or a bare NameSeg relying on the
search rules can't be resolved from wherever the package is eventually used. Here is an extract from AMD reference DSDT
gpio-leds _DSD:

Device (LEDS) {
    Name (LED0, Package () {
        Package () { "gpios", Package () { ^LEDS, 0, 0, 1 } },
    })
}

Such a name now evaluates to a reference to a new Object::NamePath, keeping the name and its
declaration scope, resolved when the reference is used. Resolution is then also independent of table
load order (only for package elements - the External case in #290 is unaffected).

_PRT decoding is updated to match, and still accepts strings. Object and AmlError gain
variants, so this is a breaking change for downstream matches.

martin-hughes · 2026-07-27T19:11:29Z

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:

the clear explanation above
a good selection of tests (the crate could always use more!)

@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.

martin-hughes · 2026-07-27T19:12:34Z

+    (*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


Purely because I'm curious - what's an "AMD Rhino"? Google unhelpfully tells me "it's probably a codename"...

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.

Ah cool - thanks, no need to remove the name. Nice to hear a little about how the crate is being used.

Ah cool - thanks, no need to remove the name. Nice to hear a little about how the crate is being used.

I echo this, very cool!

martin-hughes · 2026-07-30T20:19:22Z

-            // references?
+            // TODO: maybe this should differentiate internal/real references?
+            // An unresolved package name also behaves as a reference until dereferenced.
+            Object::Reference { .. } | Object::NamePath { .. } => ObjectType::Reference,


Why not add ObjectType::NamePath?

I realise NamePath is a form of reference, so adding a new ObjectType would mostly be for the diagnostic value when constructing certain AmlErrors.

I have a soft view here that having ObjectTypes that aren't real types in the spec is probably not the way I'd like to go.

martin-hughes · 2026-07-30T20:23:11Z

+use aml_test_tools::{RunTestResult, handlers::null_handler::NullHandler, new_interpreter, run_test_for_string};
+use std::str::FromStr;
+
+fn interpret(asl: &'static str) -> Interpreter<impl Handler> {


interpret and evaluate feel like they have more common use than just in this file - if you fancy it, I'd be willing to take a PR where they move into the "test_infra" module.

interpret is pretty similar to run_aml_test but without any logging - did you consider updating run_aml_test to return the Interpreter?

Good suggestion. I’ve updated run_aml_test to return the interpreter and moved evaluate into test_infra, then reused both here.

martin-hughes · 2026-07-30T20:27:11Z

Thanks again for your efforts @ArthurHeymans. I've taken a more thorough look and come up with only pretty minor questions - it basically looks great to me.

IsaacWoods

Nice! Thanks for the PR and for the review from @martin-hughes!

This is a good point I hadn't really considered with these package elements. I know other interpreters also model these as strings, and seem to agree with our interpretation that you should resolve these paths against the path of the object that produced the package (see e.g. here) but that is like many things in AML not really well specified. Clearly other firmware authors have interpreted it differently.

Overall I think this is a good change and agree that the approach and documentation are really good - thanks! I'd be happy for it to be merged after a review round by @martin-hughes - thanks!

IsaacWoods · 2026-08-03T20:57:46Z

-            // references?
+            // TODO: maybe this should differentiate internal/real references?
+            // An unresolved package name also behaves as a reference until dereferenced.
+            Object::Reference { .. } | Object::NamePath { .. } => ObjectType::Reference,


I have a soft view here that having ObjectTypes that aren't real types in the spec is probably not the way I'd like to go.

IsaacWoods · 2026-08-03T21:11:44Z

+    (*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


Ah cool - thanks, no need to remove the name. Nice to hear a little about how the crate is being used.

I echo this, very cool!

IsaacWoods · 2026-08-03T21:14:20Z

+                             */
+                            let name_path = Object::NamePath { name, scope: context.current_scope.clone() }.wrap();
+                            context.contribute_arg(Argument::Object(
+                                Object::Reference { kind: ReferenceKind::RefOf, inner: name_path }.wrap(),


I may be missing something obvious here, but why do we need a reference to the NamePath object instead of having it directly in the package?

You’re right—the extra reference wasn’t necessary. Package elements now contain the NamePath directly. Index creates a reference to the package element when reference semantics are needed.

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 new `Object::NamePath`, which retains the name and the scope it was declared in, and resolve it when the package element is consumed. 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.

martin-hughes · 2026-08-04T14:53:29Z

Updates all look good to me, thank you again @ArthurHeymans!

martin-hughes · 2026-08-04T15:07:10Z

Made a bad assumption here - I thought "rebase merge" in the web UI would do a fast-forward if it could, but turns out it does not. Lesson learned for next time!

martin-hughes reviewed Jul 27, 2026

View reviewed changes

martin-hughes reviewed Jul 30, 2026

View reviewed changes

IsaacWoods reviewed Aug 3, 2026

View reviewed changes

ArthurHeymans added 4 commits August 4, 2026 10:23

aml: extract ObjectType and DerefOf handling into methods

854a9b8

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.

aml: test package name references

9918ff5

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.

ArthurHeymans force-pushed the RelativeNamePath branch from 1ea5a06 to 9918ff5 Compare August 4, 2026 08:28

martin-hughes merged commit 784a35b into rust-osdev:main Aug 4, 2026
6 checks passed

Uh oh!

Uh oh!

Conversation

ArthurHeymans commented Jul 27, 2026

Uh oh!

martin-hughes commented Jul 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

martin-hughes commented Jul 30, 2026

Uh oh!

IsaacWoods left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

martin-hughes commented Aug 4, 2026

Uh oh!

Uh oh!

martin-hughes commented Aug 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants