Skip to content

Commit db8020f

Browse files
authored
AdvLoggerPkg: Drop buffer migration (#868)
## 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. - [x] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [ ] Includes tests? - [ ] Includes documentation? ## How This Was Tested - AdvLoggerPkg CI - Boot to OS on QEMU Q35 and SBSA - Boot to EFI shell on OVMF X64 w/ edk2 Standalone MM ## Integration Instructions - N/A Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
1 parent 87ab47e commit db8020f

17 files changed

Lines changed: 34 additions & 813 deletions

AdvLoggerPkg/Include/AdvancedLoggerInternal.h

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,6 @@
4545
#define ADVANCED_LOGGER_PHASE_TFA 10
4646
#define ADVANCED_LOGGER_PHASE_CNT 11
4747

48-
//
49-
// The maximum depth to follow when traversing chains of advanced logger info structures
50-
//
51-
#define ADVANCED_LOGGER_MAX_LOGGER_CHAIN_DEPTH 3
52-
5348
//
5449
// These Pcds are used to carve out a PEI memory buffer from the temporary RAM.
5550
//
@@ -84,7 +79,9 @@ typedef volatile struct {
8479
EFI_TIME Time; // Uefi Time Field
8580
UINT32 HwPrintLevel; // Logging level to be printed at hw port
8681
UINT32 Reserved3; //
87-
EFI_PHYSICAL_ADDRESS NewLoggerInfoAddress; // If non-zero, this field holds the address of new logger info that should be used
82+
EFI_PHYSICAL_ADDRESS Deprecated; // Deprecated field that should be preserved for binary compatibility
83+
// and not reused. Previous description: If non-zero, this field
84+
// holds the address of new logger info that should be used
8885
} ADVANCED_LOGGER_INFO;
8986

9087
typedef struct {

AdvLoggerPkg/Library/AdvLoggerMmAccessLib/AdvLoggerMmAccessLib.c

Lines changed: 0 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -13,88 +13,22 @@
1313
#include <AdvancedLoggerInternal.h>
1414

1515
#include <Protocol/AdvancedLogger.h>
16-
#include <Protocol/MmReadyToLock.h>
1716

1817
#include <Library/AdvLoggerAccessLib.h>
1918
#include <Library/BaseLib.h>
2019
#include <Library/BaseMemoryLib.h>
2120
#include <Library/DebugLib.h>
2221
#include <Library/HobLib.h>
2322
#include <Library/PcdLib.h>
24-
#include <Library/MmServicesTableLib.h>
2523
#include <Library/SafeIntLib.h>
2624
#include <Library/UefiBootServicesTableLib.h>
2725

2826
STATIC ADVANCED_LOGGER_INFO *mLoggerInfo = NULL;
2927
STATIC UINT32 mBufferSize = 0;
3028
STATIC EFI_PHYSICAL_ADDRESS mMaxAddress = 0;
3129
STATIC UINTN mLoggerTransferSize = 0;
32-
STATIC BOOLEAN mReadyToLock = FALSE;
3330
extern UINTN mVariableBufferPayloadSize;
3431

35-
/**
36-
Follow the logger info redirection chain and update the provided logger info pointer.
37-
38-
@param[in,out] LoggerInfo Pointer to a logger info pointer to update.
39-
@param[out] MaxAddress Optional pointer to update with the max address of the current logger.
40-
@param[out] BufferSize Optional pointer to update with the buffer size of the current logger.
41-
42-
@retval TRUE A new logger was found and LoggerInfo was updated to the new address.
43-
@retval FALSE A new logger was not found and no modification was made to LoggerInfo.
44-
**/
45-
STATIC
46-
BOOLEAN
47-
AdvancedLoggerCheckForNewerLogger (
48-
IN OUT ADVANCED_LOGGER_INFO **LoggerInfo,
49-
OUT EFI_PHYSICAL_ADDRESS *MaxAddress OPTIONAL,
50-
OUT UINT32 *BufferSize OPTIONAL
51-
)
52-
{
53-
ADVANCED_LOGGER_INFO *CurrentLoggerInfo;
54-
ADVANCED_LOGGER_INFO *NextLoggerInfo;
55-
UINTN Depth;
56-
57-
if ((LoggerInfo == NULL) || (*LoggerInfo == NULL)) {
58-
return FALSE;
59-
}
60-
61-
CurrentLoggerInfo = *LoggerInfo;
62-
Depth = 0;
63-
64-
// Follow the chain to find the current logger
65-
while ((CurrentLoggerInfo->NewLoggerInfoAddress != 0) && (Depth < ADVANCED_LOGGER_MAX_LOGGER_CHAIN_DEPTH)) {
66-
NextLoggerInfo = ALI_FROM_PA (CurrentLoggerInfo->NewLoggerInfoAddress);
67-
68-
if (NextLoggerInfo->Signature != ADVANCED_LOGGER_SIGNATURE) {
69-
return FALSE;
70-
}
71-
72-
CurrentLoggerInfo = NextLoggerInfo;
73-
Depth++;
74-
}
75-
76-
if (Depth >= ADVANCED_LOGGER_MAX_LOGGER_CHAIN_DEPTH) {
77-
return FALSE;
78-
}
79-
80-
// Update if we found a newer logger
81-
if (CurrentLoggerInfo != *LoggerInfo) {
82-
*LoggerInfo = CurrentLoggerInfo;
83-
84-
if (MaxAddress != NULL) {
85-
*MaxAddress = LOG_MAX_ADDRESS (CurrentLoggerInfo);
86-
}
87-
88-
if (BufferSize != NULL) {
89-
*BufferSize = CurrentLoggerInfo->LogBufferSize;
90-
}
91-
92-
return TRUE;
93-
}
94-
95-
return FALSE;
96-
}
97-
9832
/**
9933
CheckAddress
10034
@@ -142,38 +76,6 @@ ValidateInfoBlock (
14276
return TRUE;
14377
}
14478

145-
/**
146-
MM ReadyToLock notification handler.
147-
148-
Ensures any move needed to the new logger buffer is done before ReadyToLock completes.
149-
150-
@param[in] Protocol Protocol GUID pointer.
151-
@param[in] Interface Protocol interface pointer.
152-
@param[in] Handle The handle on which the interface was installed.
153-
154-
@retval EFI_SUCCESS The notification handler completed successfully.
155-
156-
**/
157-
STATIC
158-
EFI_STATUS
159-
EFIAPI
160-
OnMmReadyToLock (
161-
IN CONST EFI_GUID *Protocol,
162-
IN VOID *Interface,
163-
IN EFI_HANDLE Handle
164-
)
165-
{
166-
if ((mLoggerInfo != NULL) && !FeaturePcdGet (PcdAdvancedLoggerFixedInRAM)) {
167-
if (AdvancedLoggerCheckForNewerLogger (&mLoggerInfo, &mMaxAddress, &mBufferSize)) {
168-
DEBUG ((DEBUG_INFO, "%a: Logger buffer migrated at ReadyToLock. LoggerInfo=%p\n", __FUNCTION__, mLoggerInfo));
169-
}
170-
}
171-
172-
mReadyToLock = TRUE;
173-
174-
return EFI_SUCCESS;
175-
}
176-
17779
/**
17880
AdvLoggerInit - Obtain the address of the logger info block.
17981
@@ -187,7 +89,6 @@ AdvLoggerAccessInit (
18789
)
18890
{
18991
EFI_HOB_GUID_TYPE *GuidHob;
190-
VOID *Registration;
19192
ADVANCED_LOGGER_PTR *LogPtr;
19293
EFI_STATUS Status;
19394
UINTN TempSize;
@@ -209,12 +110,6 @@ AdvLoggerAccessInit (
209110

210111
if (mLoggerInfo != NULL) {
211112
mMaxAddress = LOG_MAX_ADDRESS (mLoggerInfo);
212-
213-
if (!FeaturePcdGet (PcdAdvancedLoggerFixedInRAM)) {
214-
if (AdvancedLoggerCheckForNewerLogger (&mLoggerInfo, &mMaxAddress, &mBufferSize)) {
215-
DEBUG ((DEBUG_INFO, "%a: Logger buffer migrated. LoggerInfo=%p\n", __FUNCTION__, mLoggerInfo));
216-
}
217-
}
218113
}
219114

220115
//
@@ -244,15 +139,6 @@ AdvLoggerAccessInit (
244139
//
245140

246141
DEBUG ((DEBUG_INFO, "%a: LoggerInfo=%p, code=%r\n", __FUNCTION__, mLoggerInfo, Status));
247-
248-
Status = gMmst->MmRegisterProtocolNotify (
249-
&gEfiMmReadyToLockProtocolGuid,
250-
OnMmReadyToLock,
251-
&Registration
252-
);
253-
if (EFI_ERROR (Status)) {
254-
DEBUG ((DEBUG_ERROR, "%a: Failed to register ReadyToLock notify - %r\n", __func__, Status));
255-
}
256142
}
257143

258144
/**
@@ -308,12 +194,6 @@ AdvLoggerAccessGetVariable (
308194
UINT8 *LogBufferEnd;
309195
UINTN LogBufferSize;
310196

311-
if ((mLoggerInfo != NULL) && !mReadyToLock && !FeaturePcdGet (PcdAdvancedLoggerFixedInRAM)) {
312-
if (AdvancedLoggerCheckForNewerLogger (&mLoggerInfo, &mMaxAddress, &mBufferSize)) {
313-
DEBUG ((DEBUG_INFO, "%a: Logger buffer migrated during access. LoggerInfo=%p\n", __FUNCTION__, mLoggerInfo));
314-
}
315-
}
316-
317197
if ((!ValidateInfoBlock ()) || (mLoggerTransferSize == 0)) {
318198
return EFI_UNSUPPORTED;
319199
}

AdvLoggerPkg/Library/AdvLoggerMmAccessLib/AdvLoggerMmAccessLib.inf

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,17 +34,13 @@
3434
BaseMemoryLib
3535
DebugLib
3636
HobLib
37-
MmServicesTableLib
3837
PcdLib
3938
SafeIntLib
4039

4140
[Guids]
4241
gAdvancedLoggerHobGuid
4342
gAdvLoggerAccessGuid ## CONSUMES
4443

45-
[Protocols]
46-
gEfiMmReadyToLockProtocolGuid ## NOTIFY
47-
4844
[Pcd]
4945
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize
5046
gAdvLoggerPkgTokenSpaceGuid.PcdAdvancedLoggerBase

0 commit comments

Comments
 (0)