Repository navigation
SecurityPkg: Introduce Tpm2StartupLib V2 - #1974
Open
Raymond Diaz (Raymond-MS) wants to merge 3 commits into
Open
Raymond Diaz (Raymond-MS) wants to merge 3 commits into
Raymond Diaz (Raymond-MS) wants to merge 3 commits into
Conversation
Raymond Diaz (Raymond-MS)
requested review from
Chris Fernald (cfernald),
kuqin12 and
Sean Brogan (spbrogan)
and
a balanced review from Copilot
October 13, 2026 17:49
Contributor
✅ QEMU Validation PassedSource Dependencies
Results
Workflow run: https://github.com/microsoft/mu_basecore/actions/runs/40781194993 This comment was automatically generated by the Mu QEMU PR Validation workflow. |
Raymond Diaz (Raymond-MS)
force-pushed
the
personal/rdiaz/Tpm2StartupLib_202608
branch
from
October 13, 2026 17:49
9a29b24 to
e21b88c
Compare
There was a problem hiding this comment.
🟡 Changes recommended
S3 restore failures are discarded in two initialization paths, and the public API break is not identified or migrated.
4 open findings
What changed in this PR
Expands Tpm2StartupLib to manage TPM availability, startup, PCR-bank synchronization, and active-bank-based hashing.
Changes:
- Adds phased TPM startup and PCR synchronization APIs and policies.
- Uses active PCR banks for hashing and event logging.
- Reports unsuccessful PCR allocation and updates Peiless SEC integration.
| File | Description |
|---|---|
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf |
Adds startup library and policy dependencies. |
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c |
Integrates startup synchronization and active-bank logging. |
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf |
Adds startup library and PCD dependencies. |
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c |
Integrates startup synchronization in DXE. |
SecurityPkg/SecurityPkg.dsc |
Configures new TPM policies and hash support. |
SecurityPkg/SecurityPkg.dec |
Defines new policy and support PCDs. |
SecurityPkg/Library/Tpm2StartupLibNull/Tpm2StartupLibNull.c |
Implements null versions of expanded APIs. |
SecurityPkg/Library/Tpm2StartupLib/Tpm2StartupLib.inf |
Updates library dependencies. |
SecurityPkg/Library/Tpm2StartupLib/Tpm2StartupLib.c |
Implements startup validation and PCR synchronization. |
SecurityPkg/Library/Tpm2StartupLib/README.md |
Documents configuration and APIs. |
SecurityPkg/Library/Tpm2CommandLib/Tpm2Integrity.c |
Rejects unsuccessful PCR allocation responses. |
SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.uni |
Updates PEI router description. |
SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf |
Removes intent-mask dependency. |
SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c |
Hashes using active PCR banks. |
SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.uni |
Updates DXE router description. |
SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.inf |
Removes intent-mask dependency. |
SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.c |
Hashes using active PCR banks. |
SecurityPkg/Include/Library/Tpm2StartupLib.h |
Exposes expanded startup APIs. |
ArmPlatformPkg/PeilessSec/PeilessSec.inf |
Adds TPM policy dependencies. |
ArmPlatformPkg/PeilessSec/PeilessSec.h |
Adds startup-related declarations. |
ArmPlatformPkg/PeilessSec/PeilessSec.c |
Integrates phased TPM startup and measurement. |
ArmPlatformPkg/ArmPlatformPkg.dsc |
Adds supporting library configuration. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+226
to
+233
| if (PcdGet8 (PcdTpm2InitializationPolicy) == 1) { | ||
| // TODO: Need to figure out what to do if S3ErrorReport returns TRUE. | ||
| Status = Tpm2StartupInit (GetBootMode () == BOOT_ON_S3_RESUME, NULL); | ||
| if (EFI_ERROR (Status)) { | ||
| DEBUG ((DEBUG_ERROR, "%a - TPM failed StartupInit: %r\n", __func__, Status)); | ||
| goto Error; | ||
| } | ||
| } |
Comment on lines
61
to
+63
| Tpm2StartupInit ( | ||
| IN BOOLEAN IsS3Resume, | ||
| OUT BOOLEAN *S3ErrorReport |
Comment on lines
+3273
to
+3280
| if (PcdGet8 (PcdTpm2InitializationPolicy) == 1) { | ||
| // TODO: Need to figure out what to do if S3ErrorReport returns TRUE. | ||
| Status = Tpm2StartupInit (GetBootModeHob () == BOOT_ON_S3_RESUME, NULL); | ||
| if (EFI_ERROR (Status)) { | ||
| DEBUG ((DEBUG_ERROR, "%a - TPM failed StartupInit!\n", __func__)); | ||
| return Status; | ||
| } | ||
| } |
|
|
||
| [PcdsDynamic, PcdsDynamicEx] | ||
|
|
||
| ## MU_CHANGE |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release/202608 #1974 +/- ##
================================================
Coverage ? 1.54%
================================================
Files ? 164
Lines ? 38861
Branches ? 172
================================================
Hits ? 601
Misses ? 38260
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Raymond Diaz (Raymond-MS)
force-pushed
the
personal/rdiaz/Tpm2StartupLib_202608
branch
from
October 13, 2026 18:22
e21b88c to
2fc1900
Compare
Contributor
|
Raymond Diaz (@Raymond-MS), is there significant difference between this is and #1922 we should be aware of? |
Contributor
Author
No, there is nothing different. I just moved this to 202608. I addressed your comments from the previous review so there shouldn't be anything new that you need to look at. |
HashLibBaseCryptoRouterPei and HashLibBaseCryptoRouterDxe both used PcdTpm2HashMask in order to mask which algorithms get registered via the platform. It also used PcdTpm2HashMask in order to know which hash algorithms to use as PcdTpm2HashMask after syncing PCR banks would match the active PCR banks. This update uses the active banks, queried on HashStart, to know which banks to hash and extend to. It also removes the gating behind PcdTpm2HashMask allowing the platform to register as many algorithms as it would like. This allows the platform to have support for more algorithms than what is indicated by PcdTpm2HashMask while still using it to indicate what the platform would like to default to. Signed-off-by: Raymond Diaz <raymond.diaz@microsoft.com>
Tpm2PcrAllocate takes in a value "AllocationSuccess" which indicates whether or not the call to enable/disable banks was successful in the TPM. It is possible for the Tpm2PcrAllocate function to return EFI_SUCCESS with "AllocationSuccess" being "NO". This means the command was sent successfully but the command was not successful. Updated the call to return an error if "AllocationSuccess" is "NO" rather than silently allowing the error through. Signed-off-by: Raymond Diaz <raymond.diaz@microsoft.com>
Updated Tpm2StartupLib to no longer just handle calling Tpm2Startup. The library is now responsible for validating TPM is available and working, initializing the TPM via Tpm2Startup, and syncing the active PCR banks. Sycning PCR banks happens in two phases: Security and Intent. Security makes sure that there are no currently active PCR banks that the platform doesn't support. Platform support is indicated by a PCD: PcdTcg2HashLibSupportMask. Intent makes sure that the active banks are defaulted to an intent value provided by the caller; this can be platform or user intent. Platform intent is indicated by a PCD: PcdTpm2HashMask. PcdTpm2InitializationPolicy now indicates which phase is responsible for calling Tpm2StartupInit. A new PCD: PcdTpm2SyncPolicy, determines which phase is responsible for syncing to intent. The security sync is called in every phase as there are no pre-requisites to calling it and calling it as soon as possible is preferred. PcdTcg2HashAlgorithmBitmap indicates which hash algorithms were registered via HashLib. This value is checked against PcdTcg2HashLibSupportMask to make sure that what the platform says it supports is actually what gets registered. If there is a mismatch then it means the platform is misconfigured. Signed-off-by: Raymond Diaz <raymond.diaz@microsoft.com>
Raymond Diaz (Raymond-MS)
force-pushed
the
personal/rdiaz/Tpm2StartupLib_202608
branch
from
October 28, 2026 22:29
2fc1900 to
1e5a980
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Updated Tpm2StartupLib to no longer just handle calling Tpm2Startup. The library is now responsible for validating TPM is available and working, initializing the TPM via Tpm2Startup, and syncing the active PCR banks. Sycning PCR banks happens in two phases: Security and Intent. Security makes sure that there are no currently active PCR banks that the platform doesn't support. Platform support is indicated by a PCD: PcdTcg2HashLibSupportMask. Intent makes sure that the active banks are defaulted to an intent value provided by the caller; this can be platform or user intent. Platform intent is indicated by a PCD: PcdTpm2HashMask. PcdTpm2InitializationPolicy now indicates which phase is responsible for calling Tpm2StartupInit. A new PCD: PcdTpm2SyncPolicy, determines which phase is responsible for syncing to intent. The security sync is called in every phase as there are no pre-requisites to calling it and calling it as soon as possible is preferred. PcdTcg2HashAlgorithmBitmap indicates which hash algorithms were registered via HashLib. This value is checked against PcdTcg2HashLibSupportMask to make sure that what the platform says it supports is actually what gets registered. If there is a mismatch then it means the platform is misconfigured.
Tpm2PcrAllocate takes in a value "AllocationSuccess" which indicates whether or not the call to enable/disable banks was successful in the TPM. It is possible for the Tpm2PcrAllocate function to return EFI_SUCCESS with "AllocationSuccess" being "NO". This means the command was sent successfully but the command was not successful. Updated the call to return an error if "AllocationSuccess" is "NO" rather than silently allowing the error through.
HashLibBaseCryptoRouterPei and HashLibBaseCryptoRouterDxe both used PcdTpm2HashMask in order to mask which algorithms get registered via the platform. It also used PcdTpm2HashMask in order to know which hash algorithms to use as PcdTpm2HashMask after syncing PCR banks would match the active PCR banks. This update uses the active banks, queried on HashStart, to know which banks to hash and extend to. It also removes the gating behind PcdTpm2HashMask allowing the platform to register as many algorithms as it would like. This allows the platform to have support for more algorithms than what is indicated by PcdTpm2HashMask while still using it to indicate what the platform would like to default to.
For details on how to complete these options and their meaning refer to CONTRIBUTING.md.
How This Was Tested
Tested both Q35 and ARM VIRT with TPM enabled and TpmShellApp included. Verified TPM functionality.
Integration Instructions
Include Tpm2StartupLib to your platform.dsc.