Skip to content

[2502] AdvLoggerPkg: Drop buffer migration#869

Merged
apop5 merged 1 commit intomicrosoft:release/202502from
makubacki:drop_adv_logger_buffer_migration_2502
Apr 18, 2026
Merged

[2502] AdvLoggerPkg: Drop buffer migration#869
apop5 merged 1 commit intomicrosoft:release/202502from
makubacki:drop_adv_logger_buffer_migration_2502

Conversation

@makubacki
Copy link
Copy Markdown
Member

@makubacki makubacki commented Apr 8, 2026

Description

Effectively reverts commits af0f64c and 9fc8521 preserving structure compatibility and general cleanup.

The advanced logger buffer starts accumulating messages in the first phase advanced logger is active, and the buffer continues to be used throughout remaining phases. Each phase has to account for either allocating or using an existing logger buffer. Due to the persistent nature of the buffer throughout boot, it is allocated as a runtime memory type early in boot to support its preservation and access at OS runtime.

However, S4 resume relies upon a stable memory map across suspend and resume. Since the advanced logger buffer is allocated as a runtime memory type prior to DXE, it is in a buffer outside the DXE memory bins. This leads to an increased likelihood of the buffer being at a different address across suspend and resume affecting hibernate resume. Advanced logger buffer migration was a mechanism to address this but doing so complicates MM treatment of the buffer.

Because a single advanced logger buffer is used across all phases, including a shared buffer between PEI and MM and DXE and MM, the buffer must be unlocked for MM access to accommodate a PEI MM IPL. This creates a problem where both requirements cannot simultaneously be met without additional complexity in MM access code. In addition, MM environments are inconsistent in their support for memory unlocking.

In the end, hibernation has always been a best effort scenario with this flow and other MM solutions already require runtime allocations outside of DXE memory bins, so this change removes the functional logic for migration.

The memory bin scenario will be addressed in the future when the bins are allocated in a pre-DXE phase (e.g. PEI) and picked up by DXE.

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

How This Was Tested

  • AdvLoggerPkg CI
  • Boot to OS on Intel physical platform and dump in-memory log to file

Integration Instructions

  • N/A

@makubacki makubacki self-assigned this Apr 8, 2026
@mu-automation mu-automation bot added impact:non-functional Does not have a functional impact and removed impact:non-functional Does not have a functional impact labels Apr 8, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 8, 2026

Codecov Report

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

Files with missing lines Patch % Lines
...rary/AdvancedLoggerLib/Runtime/AdvancedLoggerLib.c 0.00% 9 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff                @@
##             release/202502    #869   +/-   ##
================================================
  Coverage                  ?   4.99%           
================================================
  Files                     ?      36           
  Lines                     ?    3966           
  Branches                  ?     235           
================================================
  Hits                      ?     198           
  Misses                    ?    3766           
  Partials                  ?       2           
Flag Coverage Δ
AdvLoggerPkg 4.99% <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 makubacki marked this pull request as ready for review April 9, 2026 00:28
@makubacki makubacki requested review from cfernald, kuqin12 and os-d April 9, 2026 00:29
Copy link
Copy Markdown
Contributor

@cfernald cfernald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved pending comments in 2511 PR

Copy link
Copy Markdown
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.

With Chris's suggestion from the other PR on marking the field reserved

Effectively reverts commits af0f64c and 9fc8521 preserving structure
compatibility and general cleanup.

The advanced logger buffer starts accumulating messages in the first
phase advanced logger is active, and the buffer continues to be used
throughout remaining phases. Each phase has to account for either
allocating or using an existing logger buffer. Due to the persistent
nature of the buffer throughout boot, it is allocated as a runtime
memory type early in boot to support its preservation and access at OS
runtime.

However, S4 resume relies upon a stable memory map across suspend and
resume. Since the advanced logger buffer is allocated as a runtime
memory type prior to DXE, it is in a buffer outside the DXE memory
bins. This leads to an increased likelihood of the buffer being at
a different address across suspend and resume affecting hibernate
resume. Advanced logger buffer migration was a mechanism to address
this but doing so complicates MM treatment of the buffer.

Because a single advanced logger buffer is used across all phases,
including a shared buffer between PEI and MM and DXE and MM, the
buffer must be unlocked for MM access to accommodate a PEI MM IPL.
This creates a problem where both requirements cannot simultaneously
be met without additional complexity in MM access code. In addition,
MM environments are inconsistent in their support for memory
unlocking.

In the end, hibernation has always been a best effort scenario with
this flow and other MM solutions already require runtime allocations
outside of DXE memory bins so this change removes the functional
logic for migration.

The memory bin scenario will be addressed in the future when the bins
are allocated in a pre-DXE phase (e.g. PEI) and picked up by DXE.

Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
@makubacki makubacki force-pushed the drop_adv_logger_buffer_migration_2502 branch from ffc8fb6 to 6135290 Compare April 17, 2026 23:12
@makubacki
Copy link
Copy Markdown
Member Author

approved pending comments in 2511 PR

Same change pushed here

@makubacki
Copy link
Copy Markdown
Member Author

Note: Waiting for final server platform testing to conclude before merging

@mu-automation
Copy link
Copy Markdown
Contributor

mu-automation bot commented Apr 17, 2026

⏩ QEMU Validation Skipped

Validation was skipped (reason: wrong_target_branch).

This comment was automatically generated by the Mu QEMU PR Validation workflow.

@apop5
Copy link
Copy Markdown
Collaborator

apop5 commented Apr 18, 2026

Note: Waiting for final server platform testing to conclude before merging

This is good to go, it was a switch of the MmUnblockMemoryLib from an actual instance to the NULL instance that was causing the test hang. Once it switched back, the system booted.

@apop5 apop5 merged commit 8b646ce into microsoft:release/202502 Apr 18, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants