AdvLoggerPkg: Use Scratchpad Region instead of PEI_CORE_INSTANCE#913
Open
os-d wants to merge 1 commit into
Open
AdvLoggerPkg: Use Scratchpad Region instead of PEI_CORE_INSTANCE#913os-d wants to merge 1 commit into
os-d wants to merge 1 commit into
Conversation
Currently, The SecDebugAgent and PeiCore instances of advanced logger use the internal PEI_CORE_INSTANCE to store a pointer to the advanced logger buffer. This is a performance optimization to avoid a HOB lookup on every debug message. However, this is also a hack and requires a basecore override to reach into the core internals. This commit changes that behavior by introducing a PCD to define a scratchpad page where the pointer can be stored. This must exist outside of the PHIT free memory region so that it is not handed out as another allocation in PEI, because the allocator does not respect memory allocation HOBs for that region. If a platform does not specify this PCD, a HOB lookup will be performed on every debug message. Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
Contributor
❌ QEMU Validation FailedSource Dependencies
Results
Workflow run: https://github.com/microsoft/mu_plus/actions/runs/28256825348 This comment was automatically generated by the Mu QEMU PR Validation workflow. |
mu-automation
Bot
added
the
impact:breaking-change
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
1 participant
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
Currently, The SecDebugAgent and PeiCore instances of advanced logger use the internal PEI_CORE_INSTANCE to store a pointer to the advanced logger buffer. This is a performance optimization to avoid a HOB lookup on every debug message.
However, this is also a hack and requires a basecore override to reach into the core internals.
This commit changes that behavior by introducing a PCD to define a scratchpad page where the pointer can be stored. This must exist outside of the PHIT free memory region so that it is not handed out as another allocation in PEI, because the allocator does not respect memory allocation HOBs for that region.
If a platform does not specify this PCD, a HOB lookup will be performed on every debug message.
Closes #910 .
How This Was Tested
Booting to shell on Q35, seeing PEI logs. Added a spin loop when the PCD is used to confirm it wasn't falling back to the HOB lookup. Physical Intel testing is going on in parallel with this PR.
Integration Instructions
This is marked as a breaking change because there is a significant performance impact if a platform does not take action here. However, the build and run will still work if not.
In order to keep the same performance guarantees as the current code, platforms must provide a page address that is outside of the PHIT free memory region in
gAdvLoggerPkgTokenSpaceGuid.PcdAdvancedLoggerScratchpadBase. This must be available to write to in SEC and PEI_CORE.