ShellPkg/AcpiView: Adds FPDT Parser #11774
Conversation
tianocore-assign-reviewers
bot
requested review from
ajfish,
ardbiesheuvel,
leiflindholm,
makubacki,
mdkinney and
pierregondois
Add a new parser for the Firmware Performance Data Table (FPDT), as per the ACPI6.5 specification. Signed-off-by: Abdul Lateef Attar <[email protected]>
abdattar
force-pushed
the
AcpiViewFpdt
branch
from
6b25cc1 to
5a0eb9d
Compare
pierregondois
left a comment
pierregondois
left a comment
There was a problem hiding this comment.
Hello Abdul,
Thanks for the new parser. I added some comments
| #include "AcpiViewConfig.h" | ||
|
|
||
| // Local Variables | ||
| STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo; |
There was a problem hiding this comment.
Should be mAcpiHdrInfo for static structures
| { L"Length", 1, 2, L"0x%x", NULL, NULL, NULL, NULL }, | ||
| { L"Revision", 1, 3, L"0x%x", NULL, NULL, NULL, NULL }, | ||
| { L"Reserved", 4, 4, L"0x%x", NULL, NULL, NULL, NULL }, | ||
| { L"BootPerformanceTablePointer", 8, 8, L"0x%lx", NULL, NULL, NULL, NULL } |
There was a problem hiding this comment.
NIT: personal preference, but I think it might be easier to read the acronyms instead when reading the table:
BootPerformanceTablePointer->FBPTS3PerformanceTablePointer- >S3PT- etc.
| { L"Length", 1, 2, L"0x%x", NULL, NULL, NULL, NULL }, | ||
| { L"Revision", 1, 3, L"0x%x", NULL, NULL, NULL, NULL }, | ||
| { L"Reserved", 4, 4, L"0x%x", NULL, NULL, NULL, NULL }, | ||
| { L"BootPerformanceTablePointer", 8, 8, L"0x%lx", NULL, NULL, NULL, NULL } |
There was a problem hiding this comment.
Also, would it be possible to check if this is possible to parse the structure pointed by BootPerformanceTablePointer by providing a ACPI_PARSER.PrintFormatter ?
It seems the DumpSmmuNode is doing that for instance.
It would allow to simplify the logic in ParseAcpiFpdt()
| FpdtBasicBootPtr->Header.Length, | ||
| PARSER_PARAMS (FpdtBootPerfPointerParser) | ||
| ); | ||
| // Parse the header AcpiFpdtHdrParser |
There was a problem hiding this comment.
NIT: I think it should be ok to remove these comments
| FpdtS3ResumeRec->Header.Length, | ||
| PARSER_PARAMS (FpdtS3ResumeRecordParser) | ||
| ); | ||
| } else if (FpdtRecHdr->Type == EFI_ACPI_6_5_FPDT_RUNTIME_RECORD_TYPE_S3_SUSPEND) { |
There was a problem hiding this comment.
NIT:
I think there are new structures defined in ACPI 6.6.
If you have some time feel free to add support for them. Otherwise it s already nice to have this parser!
2 participants
Add a new parser for the Firmware Performance Data Table (FPDT), as per the ACPI6.5 specification.
How This Was Tested
Tested on AMD platform by running "acpiview -S FPDT" from UEFI shell.
Integration Instructions
N/A