-
Notifications
You must be signed in to change notification settings - Fork 112
AdvLoggerPkg: Fix memory bucket stability #799
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AdvLoggerPkg: Fix memory bucket stability #799
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release/202502 #799 +/- ##
================================================
Coverage ? 4.71%
================================================
Files ? 36
Lines ? 4196
Branches ? 276
================================================
Hits ? 198
Misses ? 3996
Partials ? 2
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:
|
|
My understanding is that there are at least two additional changes likely needed.
For others that have worked more with adv logger, is that true? Also, any quick sanity checks of the current changes (while those updates are being made) is appreciated. |
os-d
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As noted offline, I think this works, but we should only do this if we aren't doing the fixed buffer region
85a781d to
af44b6e
Compare
d326db1 to
2c59de3
Compare
|
@cfernald, @kuqin12, @os-d, this is now ready for review. We do plan to move to a fixed runtime region allocated in PEI. This is being checked into release/202502 which is needed by current platforms. The PEI fixed buffer may or may not be something we want to risk merging into 202502 (leaning more toward not). This can simply be adjusted at that time to not perform the buffer migration. That already is controlled with It could also be dropped entirely at that time if all platforms based on future release branches are forced to switch to a fixed PEI RT memory region. |
AdvLoggerPkg/Library/AdvancedLoggerLib/DxeCore/AdvancedLoggerLib.c
Outdated
Show resolved
Hide resolved
Explain what the versioning scheme is for the advanced logger strucuture and messages. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
d29e790 to
66a1ab5
Compare
Right now, the PEI Core instance of AdvancedLoggerLib will always allocate the logger buffer as `EfiRuntimeServicesData`. This means it is not allocated into the DXE Core managed RT Services Data memory bucket. The DXE Core instance of AdvancedLoggerLib either continues to use this buffer it is provided or allocates a new `EfiReservedMemoryType` buffer. This change always allocates a `EfiReservedMemoryType` buffer in the DXE Core instance to ensure it is allocated from the reserved memory type buckets setup by the DXE Core. The PEI Core logger buffer is updated to be `EfiBootServicesData` so it does not affect runtime allocations. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Updates the logger info structure to account for the NewLoggerInfoAddress field. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Update the library to use the standard C predefined identifier `__func__` instead of `__FUNCTION__` to improve code portability. This was started in some recently modified lines in the library. The remaining instances have been updated for consistency in this change. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
66a1ab5 to
0be8648
Compare
Description
Right now, the PEI Core instance of AdvancedLoggerLib will always allocate the logger buffer as
EfiRuntimeServicesData. This means it is not allocated into the DXE Core managed RT Services Data memory bucket. The DXE Core instance of AdvancedLoggerLib either continues to use this buffer it is provided or allocates a newEfiReservedMemoryTypebuffer.This change always allocates a
EfiReservedMemoryTypebuffer in the DXE Core instance to ensure it is allocated from the reserved memory type buckets setup by the DXE Core. The PEI Core logger buffer is updated to beEfiBootServicesDataso it does not affect runtime allocations.The logger buffer will not attempt to be migrated if
PcdAdvancedLoggerFixedInRAMisFALSE.How This Was Tested
Integration Instructions