Rewrite acpi crate and entire AML interpreter
#246
Conversation
I don't really understand why this had a lifetime instead of constructing the slice for the lifetime of `self`, but this cleans stuff up a fair bit both in the library and for users.
…e`, `DefMethod`, `DefExternal`
Still need to do a bunch of bit fiddling stuff to facilitate actually reading and storing to them.
This will allow us to add variants without them being breaking changes, and I don't think you should be exhaustively matching our errors regardless.
…g`, and `ToInteger`
|
I have a real-world AML file that is producing an opcode error. I am AFK, but do you want the whole file or just the opcode that is producing the error? |
|
Whole file please. |
This necessitated also unwrapping more references for package and buffer lengths. This may need to be done elsewhere.
|
Thanks for the reports. Interpreting the files via the test harness, both do produce errors, although not the same one. The HP laptop produces The Dell table produces Reassuringly, the interpreter in its current state seems to munch through quite a lot of those tables, which do include some weird stuff. I'll get these fixed when I can. Please do report any new problems you come across. |
|
I found similar cases as well, but thought it was something on my side. I found |
This fixes issues in real tables.
|
@rw-vanc I have fixed issues with name collisions and the use of a I have also added some rudimentary tracing of in-flight operations which make it much easier to find where the issue is with errors like these. It can be seen at the @usadson it's very possible the same problem is happening elsewhere. I'll have a look around, but if you have dumps that aren't parsing, using the tracing might help find the issue. You can also drop me dumps and I can have a look - probs easiest to PM me the dump on the |
|
Thanks. I will test with the AML tester. I tried to merge the new code into Redox but we are using a different compiler version, and I don't have time to deal with that this month. We are going to try to get on the latest Rust compiler in a few months, but when I get time (and when you feel like the AML parser is ready) I will fork and patch for the older compiler, so we can do more real-world testing. |
To match NT's behaviour, we should just evaluate a namestring within a package definition to a string object. Also handles this change in PRT handling.
This (entirely too large) PR represents work I've been doing over the last few months to address a large number of bugs in both the
acpiandamlcrates, and generally improve the quality of the library. While this work is nowhere near complete, it represents the library getting back to a point where I think it is reasonable for projects to use it (in that it is more correct than the current interpreter in all circumstances), and is already too unwieldy to rebase against other PRs that are waiting for review/merging.For contributors awaiting PRs being merged, I apologise for this derailing your contributions, and I am happy to assist in rebasing your changes/creating new patches with you credited as authors. I will review current PRs when I have time.
Summary of changes
Firstly, the
acpicrate has undergone major reworking to simplify logic surrounding table mapping and RSDT enumeration. This should fix #245.Previous work on allowing fallible allocations has been removed, as it added significant complexity for little real-world gain at this time - if the ecosystem around fallible allocation in Rust improves in the future, I'm not against re-adding these capabilities. The split between the portion of the library that can be used without an allocator vs requiring one has been made clearer, with allocating methods being moved under the higher-level
platformmodule.This PR also deprecates the
amlcrate, bringing the interpreter into theacpicrate under a feature-flag. This simplifies the most common use-cases of the library, and was effectively required for ergonomic handling of some AML operations that require us to handle static ACPI tables. Future work with the ACPI event system will likely require closer coupling between the previously-separate parts of the library, as well.I am imagining that these changes will be published as
acpi v6.0.0, with theamlcrate being marked as deprecated and receiving no future updates.Remaining tasks
AcpiHandlervsacpi::aml::Handler- do we want to merge them, makeAcpiHandlera supertrait (perhaps being renamedRegionMapperor similar as this is the only functionality it needs to provide)?GenericAddressto have arbitrary field widths - apparently required by the PCC (see acpi: add support for PCC table #233)DefMatchDefVarPackageDefLoadDefLoadTableDefDataRegionDefToBufferDefToIntegerDefToStringDefToDecimalStringDefToHexStringDefCopyObjectDefNotifyFeedback
I would welcome feedback on the new APIs, crate layout, or changes I should consider before publishing a new major version of
acpi. I particularly welcome feedback + bug-reports from people who have integratedacpiinto a kernel or other project.cc @rw-vanc - I would be interested in any concerns re Redox moving to the new version of the crate, and any further work you'd be interested in.
cc @usadson - this should fix #245. Thank you for your report.