Skip to content

Conversation

@makubacki
Copy link
Member

@makubacki makubacki commented Nov 26, 2025

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 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.

The logger buffer will not attempt to be migrated if PcdAdvancedLoggerFixedInRAM is FALSE.

  • Impacts functionality?
  • Impacts security?
  • Breaking change?
  • Includes tests?
  • Includes documentation?

How This Was Tested

  • CI
  • Boot on a virtual and physical Intel platform and confirm the logger is functional
    • Note: Has not been tested on an ARM64 platform yet

Integration Instructions

  • N/A

@makubacki makubacki self-assigned this Nov 26, 2025
@codecov-commenter
Copy link

codecov-commenter commented Nov 26, 2025

Codecov Report

❌ Patch coverage is 0% with 383 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (release/202502@fc05e0b). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...rary/AdvancedLoggerLib/DxeCore/AdvancedLoggerLib.c 0.00% 103 Missing ⚠️
.../AdvancedLoggerAccessLib/AdvancedLoggerAccessLib.c 0.00% 65 Missing ⚠️
...ibrary/AdvLoggerMmAccessLib/AdvLoggerMmAccessLib.c 0.00% 63 Missing ⚠️
...rary/AdvLoggerSmmAccessLib/AdvLoggerSmmAccessLib.c 0.00% 63 Missing ⚠️
...rary/AdvancedLoggerLib/Runtime/AdvancedLoggerLib.c 0.00% 48 Missing ⚠️
...g/Library/AdvancedLoggerLib/AdvancedLoggerCommon.c 0.00% 34 Missing ⚠️
...rary/AdvancedLoggerLib/PeiCore/AdvancedLoggerLib.c 0.00% 5 Missing ⚠️
...brary/AdvancedLoggerLib/MmCore/AdvancedLoggerLib.c 0.00% 2 Missing ⚠️
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           
Flag Coverage Δ
AdvLoggerPkg 4.71% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@makubacki
Copy link
Member Author

makubacki commented Nov 26, 2025

My understanding is that there are at least two additional changes likely needed.

  1. Allow an option for platforms to use a static memory region throughout all of boot. This region would be platform-defined (fixed) and not dynamically allocated. This will split the incoming buffer to DXE into either a "static" and "dynamic" case. DXE will only migrate the buffer if it falls under the "dynamic" case. The case might be able to be simply detected by checked with: PcdAdvancedLoggerBase != 0.

  2. In the "dynamic" buffer case, where it is migrated to a DXE allocated buffer, the MM Core AdvancedLoggerLib instance will initially start working with the Boot Services Data buffer as described in gAdvancedLoggerHobGuid, but we'd need to send the updated shared buffer to the MM Core when it is allocated by the DXE Core instance.

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.

Copy link
Contributor

@os-d os-d left a 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

@makubacki makubacki force-pushed the adv_logger_bucket_frag branch from 85a781d to af44b6e Compare December 5, 2025 03:40
@makubacki makubacki linked an issue Dec 18, 2025 that may be closed by this pull request
1 task
@makubacki makubacki force-pushed the adv_logger_bucket_frag branch 5 times, most recently from d326db1 to 2c59de3 Compare January 7, 2026 21:07
@makubacki makubacki marked this pull request as ready for review January 7, 2026 21:19
@makubacki makubacki requested review from cfernald and kuqin12 January 7, 2026 21:19
@makubacki
Copy link
Member Author

@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 PcdAdvancedLoggerFixedInRAM. Perhaps then it would be !PcdAdvancedLoggerFixedInRAM || gEfiMemoryTypeInformationGuid Resource Descriptor HOB present or similar logic. The main point is it can relatively co-exist with PEI carved out RT memory

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.

Explain what the versioning scheme is for the advanced logger
strucuture and messages.

Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
@makubacki makubacki force-pushed the adv_logger_bucket_frag branch 3 times, most recently from d29e790 to 66a1ab5 Compare January 12, 2026 17:16
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>
@makubacki makubacki force-pushed the adv_logger_bucket_frag branch from 66a1ab5 to 0be8648 Compare January 13, 2026 18:09
@mu-automation mu-automation bot added the language:python Pull requests that update Python code label Jan 13, 2026
@makubacki makubacki merged commit c4a5d04 into microsoft:release/202502 Jan 13, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

language:python Pull requests that update Python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Advanced Logger Does Not Reallocate Buffer In DXE

5 participants