acpi: Add SdtHeaderIterator to get all headers.
#202
Conversation
fslongjin
force-pushed
the
patch-add-header-iterator
branch
from
ab16970
to
7601baf
Compare
|
ping for code review~ |
There was a problem hiding this comment.
Apologies for the delay in reviewing this!
Overall this looks reasonable, and I have no particular issues with exposing this functionality. Mainly out of interest - do you have a particular usecase for iterating through the headers particularly?
Also worth considering is that we currently have the internal TablesPhysPtrsIter iterator that gives us a more general view of each table as a pointer to the header - I wonder if we could clean that up (i.e. make it a concrete type) and use it to expose this sort of functionality (this would effectively be a map over it to extract each header)? Not too fussed either way though.
acpi/src/lib.rs
Outdated
| }; | ||
| let r = header_mapping.validate(header_mapping.signature); | ||
| if r.is_err() { | ||
| log::warn!("Found invalid SSDT at physical address {:p}: {:?}", table_phys_ptr, r); |
There was a problem hiding this comment.
SSDTs are a specific type of table - this should probably be SDT to refer to any sort of ACPI table
Sure! I've used it in my hobby OS to generate attributes in sysfs: https://opengrok.ringotek.cn/xref/DragonOS/kernel/src/driver/acpi/sysfs.rs?r=7eda31b2f07c6ef41dc0d2bd13051f0fce5e5976#108 |
|
Thanks, and thanks for providing your usecase! My understanding is that the If so, you can either do that in this PR, or if you're not sure what I'm going for, I'm happy to give it a go (and would add you as a co-author to those commits :)). Lmk. |
Yes, but during the development process, I read the acpi-rs code and didn't think about how to implement functionality more reasonably within this crate. The reasons are:
What do you think we should do? |
|
ping |
|
Hey, thanks for working on this and apologies for the slow reviews. I'm going to merge this as is but not release a new version just yet, just so it can potentially be reworked in a larger refactor of the table iterators at a later date. You can still use it from a fork / git dependency on this repo, and it'll be released with the next version when that happens :)
For most kernels, this is not a big problem (if you have a direct memory map of physical memory it's literally a no-op) imo. Even cloning all the tables is not a huge deal, as they don't tend to be massive (especially if you leave out of the AML ones (DSDT and SSDTs)). |
This PR add
SdtHeaderIteratorto get all sdt headers.