SecurityPkg: Introduce Dynamic TCG Log Scaling#1788
Conversation
✅ QEMU Validation PassedSource Dependencies
Results
Workflow run: https://github.com/microsoft/mu_basecore/actions/runs/25770214018 This comment was automatically generated by the Mu QEMU PR Validation workflow. |
83f4da1 to
7711c4f
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release/202511 #1788 +/- ##
=================================================
Coverage ? 2.21%
=================================================
Files ? 1636
Lines ? 420850
Branches ? 4949
=================================================
Hits ? 9308
Misses ? 411467
Partials ? 75
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
makubacki
left a comment
There was a problem hiding this comment.
@Raymond-MS, I found the accompanying documentation helpful. I left some comments in a quick first pass, I might follow up when I have time to look in more detail.
7711c4f to
2ee0b3c
Compare
…come truncated it instead now dynamically scales doubling the size each time. An ERROR log is reported that an increase to your base log size should occur such that scaling is not necessary. This is a precaution against platforms that log a lot and the addition of new hashing algorithms for PQC. The log is allocated in BootServices memory. The ACPI log is created on ReadyToBoot with logs being added to both until they would need to scale. In this instance a truncation event is added to the ACPI log to indicate that the log is no longer valid and/or may not contain the entirety of the log. This ACPI table is allocated in NVS memory. If the ACPI table was already allocated at the time of the ACPI log creation, it is uninstalled and reinstalled with the updated LAML and LASA PCDs. Tests were added via TcgLogTest which includes a DXE driver and a UEFI shelld UnitTest app. The DXE driver handles pre-ReadyToBoot tests while the TestApp handles post-ReadyToBoot tests as well as gathering the test results from the DXE driver. Markdown documents were created to detail the changes.
…cause possible loss of data.
…used dynamic scaling to occur. Scaling the log now makes sure the LastEvent pointer is always valid.
ee53d3f to
4d886b7
Compare
| // String logged as a NO_ACTION event to mark the ACPI-visible TCG | ||
| // log as truncated when dynamic scaling occurs post ReadyToBoot. | ||
| #define TCG_LOG_TRUNCATION_EVENT_STRING "TCG Event Log Truncated" | ||
|
|
There was a problem hiding this comment.
follow existing pattern to add len
| // String logged as a NO_ACTION event to mark the ACPI-visible TCG | ||
| // log as truncated when dynamic scaling occurs post ReadyToBoot. | ||
| #define TCG_LOG_TRUNCATION_EVENT_STRING "TCG Event Log Truncated" | ||
|
|
There was a problem hiding this comment.
Add comment that this is not yet spec defined. do we have plans to take to tcg?
There was a problem hiding this comment.
I believe that when we upstream, if this feature is adopted by the EDK2 community, it would be in our best interest to bring this to the TCG spec.
| EFI_TPM2_ACPI_START_METHOD_SPECIFIC_PARAMETERS_ARM_FFA FfaParameters; | ||
| UINT32 Laml; // Optional | ||
| UINT64 Lasa; // Optional | ||
| } EFI_TPM2_ACPI_TABLE_V5; |
There was a problem hiding this comment.
is this really a version 5 table and is it really hard coded to FFA? Seems like we should use unions and make a more flexible data structure. What is the guidance for non FFA TPMs. What table version do they use? I also don't see a place for EFI_TPM2_ACPI_START_METHOD_SPECIFIC_PARAMETERS_ARM_SMC in the table. Again, I assume there is a flavor of the table that has this.
There was a problem hiding this comment.
I created a union of PlatformSpecificParameters, SmcParameters, and FfaParameters. All have the same length and allow backwards compatibility with the other versions of the table structure.
| UINT8 PlatformSpecificParameters[12]; // size up to 12 | ||
| UINT32 Laml; // Optional | ||
| UINT64 Lasa; // Optional | ||
| } EFI_TPM2_ACPI_TABLE_V4; |
There was a problem hiding this comment.
need to combine this with EFI_TPM2_ACPI_TABLE
| events the log fills up. Traditionally, when the log is full, subsequent events | ||
| are dropped and the log is marked as truncated. | ||
|
|
||
| Tcg2Dxe extends this behavior with **dynamic scaling**: when the log is about |
There was a problem hiding this comment.
probably important to call out here the different behaviors based on time of execution (pre ready to boot vs post).
| SecurityPkg/Tcg/TcgLogTest/TcgLogTestApp.inf { | ||
| <LibraryClasses> | ||
| UnitTestLib|UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.inf | ||
| UnitTestPersistenceLib|UnitTestFrameworkPkg/Library/UnitTestPersistenceLibNull/UnitTestPersistenceLibNull.inf |
There was a problem hiding this comment.
these don't look right for a functioning app
| ## MU_CHANGE - END - Add a new protocol to support Log-only events. | ||
|
|
||
| ## MU_CHANGE - [BEGIN] | ||
| ## Protocol used to test dynamic TCG log scaling functionality. |
There was a problem hiding this comment.
this is a private protocol with visibility only to the test app and test dxe driver.
I could see two options.
- just note that here
- don't create a package defined global for it and just include the guid define in the header file.
| #define TCG_LOG_TEST_EVENT_TYPE EV_NO_ACTION | ||
| #define TCG_LOG_TEST_EVENT_PAYLOAD "TcgLogTestDxeEvent" | ||
|
|
||
| STATIC CHAR8 mCommonEventPayload[] = TCG_LOG_TEST_EVENT_PAYLOAD; |
There was a problem hiding this comment.
this will probably cause a problem with initializer and compiler intrinsics that are not supported.
There was a problem hiding this comment.
Pull request overview
Introduces dynamic scaling for the TCG2 event log and adds a multi-boot test driver/application pair to validate pre- and post-ReadyToBoot behavior, including ACPI log truncation marking.
Changes:
- Adds dynamic TCG2 log reallocation in
Tcg2Dxe, plus a fixed ACPI NVS log generated at ReadyToBoot. - Adds truncation marker support for the ACPI-visible log when post-ReadyToBoot scaling occurs.
- Adds
TcgLogTestDXE/app components, protocol definitions, DSC/DEC integration, and documentation.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
SecurityPkg/Tcg/TcgLogTest/TcgLogTestDxe.inf |
Defines the new DXE test driver build metadata. |
SecurityPkg/Tcg/TcgLogTest/TcgLogTestDxe.c |
Implements pre-ReadyToBoot scaling test coordination and protocol publication. |
SecurityPkg/Tcg/TcgLogTest/TcgLogTestCommon.h |
Declares shared TCG log test helpers. |
SecurityPkg/Tcg/TcgLogTest/TcgLogTestCommon.c |
Implements event parsing, event logging until scale, and log dumping helpers. |
SecurityPkg/Tcg/TcgLogTest/TcgLogTestApp.inf |
Defines the new UEFI shell unit test app build metadata. |
SecurityPkg/Tcg/TcgLogTest/TcgLogTestApp.c |
Implements post-ReadyToBoot scaling tests and multi-boot test flow. |
SecurityPkg/Tcg/TcgLogTest/TcgLogTest.h |
Defines the test coordination protocol and enable variable. |
SecurityPkg/Tcg/TcgLogTest/README.md |
Documents the TcgLogTest components and boot flow. |
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf |
Adds ACPI protocol dependencies for TPM2 table updates. |
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c |
Adds dynamic log scaling, ACPI log generation, and truncation marker handling. |
SecurityPkg/Tcg/Tcg2Dxe/README.md |
Documents dynamic scaling and log lifetime behavior. |
SecurityPkg/Tcg/Tcg2AcpiFfa/Tcg2AcpiFfa.c |
Switches TPM2 ACPI v5 struct usage to the shared industry-standard header. |
SecurityPkg/Tcg/Tcg2Acpi/Tcg2Acpi.c |
Switches TPM2 ACPI v4 struct usage to the shared industry-standard header. |
SecurityPkg/SecurityPkg.dsc |
Adds the new TcgLogTest modules to the package build. |
SecurityPkg/SecurityPkg.dec |
Publishes the TcgLogTest protocol GUID. |
MdePkg/Include/IndustryStandard/UefiTcgPlatform.h |
Adds the TCG log truncation event string constant. |
MdePkg/Include/IndustryStandard/Tpm2Acpi.h |
Adds TPM2 ACPI v4/v5 table structs for shared use. |
Comments suppressed due to low confidence (1)
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c:3009
GenerateAcpiLog()can fail (for example, NVS allocation failure), but the result is ignored andmReadyToBootis set immediately afterward. That leaves the TPM2 ACPI table pointing at the pre-ReadyToBoot boot-services log/PCDs while later code assumes the ACPI NVS log exists. Check and handle this status before marking ReadyToBoot complete.
if ((PcdGet8 (PcdTpm2AcpiTableRev) >= 4) && !mReadyToBoot) {
GenerateAcpiLog (&mTcgDxeData.EventLogAreaStruct[Index]);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| *PcrIndex = *(UINT32 *)EventPtr; | ||
| } | ||
|
|
||
| // Store the EventType, if provided. | ||
| if (EventType != NULL) { | ||
| *EventType = *(UINT32 *)(EventPtr + sizeof (UINT32)); |
| // Uninstall and reinstall the ACPI table with the updated LAML/LASA. | ||
| UpdateTpm2AcpiTable (); |
| *LogBuffer = mLogBuffer; | ||
| *LogSize = mLogOffset + 1; | ||
| return EFI_SUCCESS; |
|
|
||
| Tcg2Dxe is a DXE-phase UEFI driver that publishes the TCG2 protocol defined | ||
| by the [TCG EFI Protocol Specification](https://trustedcomputinggroup.org/resource/tcg-efi-protocol-specification/). | ||
| It's main responsibilites are to expose a standard interface to a TPM device, |
| SecurityPkg/Tcg/TcgLogTest/TcgLogTestApp.inf { | ||
| <LibraryClasses> | ||
| UnitTestLib|UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.inf | ||
| UnitTestPersistenceLib|UnitTestFrameworkPkg/Library/UnitTestPersistenceLibNull/UnitTestPersistenceLibNull.inf |
| IN UNIT_TEST_CONTEXT Context | ||
| ) | ||
| { | ||
| SaveFrameworkState (NULL, 0); |
| Cleanup for TestPostReadyToBootScaling: enable the DXE pre-ReadyToBoot test | ||
| for the next boot, then save and reboot so the DXE driver runs before | ||
| ReadyToBoot on the second boot. | ||
|
|
||
| @param[in] Context Unit test context (unused). | ||
| **/ | ||
| STATIC | ||
| VOID | ||
| EFIAPI | ||
| EnableDxeTestAndReboot ( | ||
| IN UNIT_TEST_CONTEXT Context | ||
| ) | ||
| { | ||
| EFI_STATUS Status; | ||
|
|
||
| if (mTcgLogTestProtocol != NULL) { | ||
| Status = mTcgLogTestProtocol->Enable (mTcgLogTestProtocol, TRUE); | ||
| DEBUG ((DEBUG_INFO, "%a: Enable (TRUE) - %r\n", __func__, Status)); | ||
| } else { | ||
| DEBUG ((DEBUG_ERROR, "%a: mTcgLogTestProtocol is NULL, cannot enable\n", __func__)); | ||
| } | ||
|
|
||
| SaveAndReboot (Context); | ||
| } | ||
|
|
||
| /** | ||
| Prerequisite: locate the TCG2 and TcgLogTest protocols. | ||
|
|
||
| @param[in] Context Unit test context (unused). | ||
|
|
||
| @retval UNIT_TEST_PASSED Protocols located. | ||
| @retval UNIT_TEST_ERROR_PREREQUISITE_NOT_MET Protocol not found. | ||
| **/ |
| // Free the old log region. | ||
| gBS->FreePages (OldLasa, EFI_SIZE_TO_PAGES ((UINTN)OldLaml)); |
Description
Implemented dynamic TCG log scaling in Tcg2Dxe. When the log would become truncated it instead now dynamically scales doubling the size each time. An ERROR log is reported that an increase to your base log size should occur such that scaling is not necessary. This is a precaution against platforms that log a lot and the addition of new hashing algorithms for PQC. The log is allocated in BootServices memory. The ACPI log is created on ReadyToBoot with logs being added to both until they would need to scale. In this instance a truncation event is added to the ACPI log to indicate that the log is no longer valid and/or may not contain the entirety of the log. This ACPI log is allocated in NVS memory. If the ACPI table was already allocated at the time of the ACPI log creation, it is uninstalled and reinstalled with the updated LAML and LASA PCDs. Tests were added via TcgLogTest which includes a DXE driver and a UEFI shell UnitTest app. The DXE driver handles pre-ReadyToBoot tests while the TestApp handles post-ReadyToBoot tests as well as gathering the test results from the DXE driver. Markdown documents were created to detail the changes.
For details on how to complete these options and their meaning refer to CONTRIBUTING.md.
How This Was Tested
Tested via TcgLogTest included in the reference QEMU SBSA platform with TPM enabled. Confirmed the UnitTest results. Both tests report PASS.
Integration Instructions
Include the TcgLogTest .inf's to your platform .dsc and .fdf files. You will need to include both the TcgLogTestDxe and TcgLogTestApp for full functionality.