* [edk2-platforms: PATCH] IntelSiliconPkg/SpiFvbServiceSmm: Rewrite VariableStore header.
@ 2023-02-03 21:55 Chiu, Chasel
2023-02-04 16:02 ` Ashraf Ali S
0 siblings, 1 reply; 3+ messages in thread
From: Chiu, Chasel @ 2023-02-03 21:55 UTC (permalink / raw)
To: devel
Cc: Chasel Chiu, Ashraf Ali S, Isaac Oram, Rangasai V Chaganty,
Ray Ni, Michael Kubacki
When invalid VariableStore FV header detected, current SpiFvbService
will erase both FV and VariableStore headers from flash, however,
it will only rewrite FV header back and cause invalid VariableStore
header.
This patch adding the support for rewriting both FV header and
VariableStore header when VariableStore corruption happened.
Platform has to set PcdFlashVariableStoreType to inform SpiFvbService
which VariableStoreType should be rewritten.
Cc: Ashraf Ali S <ashraf.ali.s@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
---
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c | 172 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf | 4 ++++
Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec | 8 ++++++++
3 files changed, 132 insertions(+), 52 deletions(-)
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c
index 6b4bcdcfe3..f29540c62c 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c
@@ -12,6 +12,7 @@
#include <Library/MmServicesTableLib.h>
#include <Library/UefiDriverEntryPoint.h>
#include <Protocol/SmmFirmwareVolumeBlock.h>
+#include <Guid/VariableFormat.h>
/**
The function installs EFI_FIRMWARE_VOLUME_BLOCK protocol
@@ -25,12 +26,12 @@
**/
VOID
InstallFvbProtocol (
- IN EFI_FVB_INSTANCE *FvbInstance
+ IN EFI_FVB_INSTANCE *FvbInstance
)
{
- EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
- EFI_STATUS Status;
- EFI_HANDLE FvbHandle;
+ EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
+ EFI_STATUS Status;
+ EFI_HANDLE FvbHandle;
ASSERT (FvbInstance != NULL);
if (FvbInstance == NULL) {
@@ -52,19 +53,21 @@ InstallFvbProtocol (
//
// FV does not contains extension header, then produce MEMMAP_DEVICE_PATH
//
- FvbInstance->DevicePath = (EFI_DEVICE_PATH_PROTOCOL *) AllocateRuntimeCopyPool (sizeof (FV_MEMMAP_DEVICE_PATH), &mFvMemmapDevicePathTemplate);
+ FvbInstance->DevicePath = (EFI_DEVICE_PATH_PROTOCOL *)AllocateRuntimeCopyPool (sizeof (FV_MEMMAP_DEVICE_PATH), &mFvMemmapDevicePathTemplate);
if (FvbInstance->DevicePath == NULL) {
DEBUG ((DEBUG_INFO, "SpiFvbServiceSmm.c: Memory allocation for MEMMAP_DEVICE_PATH failed\n"));
return;
}
- ((FV_MEMMAP_DEVICE_PATH *) FvbInstance->DevicePath)->MemMapDevPath.StartingAddress = FvbInstance->FvBase;
- ((FV_MEMMAP_DEVICE_PATH *) FvbInstance->DevicePath)->MemMapDevPath.EndingAddress = FvbInstance->FvBase + FvHeader->FvLength - 1;
+
+ ((FV_MEMMAP_DEVICE_PATH *)FvbInstance->DevicePath)->MemMapDevPath.StartingAddress = FvbInstance->FvBase;
+ ((FV_MEMMAP_DEVICE_PATH *)FvbInstance->DevicePath)->MemMapDevPath.EndingAddress = FvbInstance->FvBase + FvHeader->FvLength - 1;
} else {
- FvbInstance->DevicePath = (EFI_DEVICE_PATH_PROTOCOL *) AllocateRuntimeCopyPool (sizeof (FV_PIWG_DEVICE_PATH), &mFvPIWGDevicePathTemplate);
+ FvbInstance->DevicePath = (EFI_DEVICE_PATH_PROTOCOL *)AllocateRuntimeCopyPool (sizeof (FV_PIWG_DEVICE_PATH), &mFvPIWGDevicePathTemplate);
if (FvbInstance->DevicePath == NULL) {
DEBUG ((DEBUG_INFO, "SpiFvbServiceSmm.c: Memory allocation for FV_PIWG_DEVICE_PATH failed\n"));
return;
}
+
CopyGuid (
&((FV_PIWG_DEVICE_PATH *)FvbInstance->DevicePath)->FvDevPath.FvName,
(GUID *)(UINTN)(FvbInstance->FvBase + FvHeader->ExtHeaderOffset)
@@ -103,17 +106,21 @@ FvbInitialize (
VOID
)
{
- EFI_FVB_INSTANCE *FvbInstance;
- EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
- EFI_FV_BLOCK_MAP_ENTRY *PtrBlockMapEntry;
- EFI_PHYSICAL_ADDRESS BaseAddress;
- EFI_STATUS Status;
- UINTN BufferSize;
- UINTN Idx;
- UINT32 MaxLbaSize;
- UINT32 BytesWritten;
- UINTN BytesErased;
- UINT64 NvStorageFvSize;
+ EFI_FVB_INSTANCE *FvbInstance;
+ EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
+ EFI_FV_BLOCK_MAP_ENTRY *PtrBlockMapEntry;
+ EFI_PHYSICAL_ADDRESS BaseAddress;
+ EFI_STATUS Status;
+ UINTN BufferSize;
+ UINTN Idx;
+ UINT32 MaxLbaSize;
+ UINT32 BytesWritten;
+ UINTN BytesErased;
+ UINT64 NvStorageFvSize;
+ UINT32 ExpectedBytesWritten;
+ VARIABLE_STORE_HEADER *VariableStoreHeader;
+ UINT8 VariableStoreType;
+ UINT8 *NvStoreBuffer;
Status = GetVariableFlashNvStorageInfo (&BaseAddress, &NvStorageFvSize);
if (EFI_ERROR (Status)) {
@@ -129,6 +136,7 @@ FvbInitialize (
DEBUG ((DEBUG_ERROR, "[%a] - 64-bit variable storage base address not supported.\n", __FUNCTION__));
return;
}
+
Status = SafeUint64ToUint32 (NvStorageFvSize, &mPlatformFvBaseAddress[0].FvSize);
if (EFI_ERROR (Status)) {
ASSERT_EFI_ERROR (Status);
@@ -136,8 +144,8 @@ FvbInitialize (
return;
}
- mPlatformFvBaseAddress[1].FvBase = PcdGet32(PcdFlashMicrocodeFvBase);
- mPlatformFvBaseAddress[1].FvSize = PcdGet32(PcdFlashMicrocodeFvSize);
+ mPlatformFvBaseAddress[1].FvBase = PcdGet32 (PcdFlashMicrocodeFvBase);
+ mPlatformFvBaseAddress[1].FvSize = PcdGet32 (PcdFlashMicrocodeFvSize);
//
// We will only continue with FVB installation if the
@@ -147,17 +155,17 @@ FvbInitialize (
//
// Make sure all FVB are valid and/or fix if possible
//
- for (Idx = 0;; Idx++) {
- if (mPlatformFvBaseAddress[Idx].FvSize == 0 && mPlatformFvBaseAddress[Idx].FvBase == 0) {
+ for (Idx = 0; ; Idx++) {
+ if ((mPlatformFvBaseAddress[Idx].FvSize == 0) && (mPlatformFvBaseAddress[Idx].FvBase == 0)) {
break;
}
BaseAddress = mPlatformFvBaseAddress[Idx].FvBase;
- FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) BaseAddress;
+ FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)BaseAddress;
if (!IsFvHeaderValid (BaseAddress, FvHeader)) {
BytesWritten = 0;
- BytesErased = 0;
+ BytesErased = 0;
DEBUG ((DEBUG_ERROR, "ERROR - The FV in 0x%x is invalid!\n", FvHeader));
FvHeader = NULL;
Status = GetFvbInfo (BaseAddress, &FvHeader);
@@ -165,57 +173,114 @@ FvbInitialize (
DEBUG ((DEBUG_WARN, "ERROR - Can't recovery FV header at 0x%x. GetFvbInfo Status %r\n", BaseAddress, Status));
continue;
}
+
DEBUG ((DEBUG_INFO, "Rewriting FV header at 0x%X with static data\n", BaseAddress));
//
// Spi erase
//
- BytesErased = (UINTN) FvHeader->BlockMap->Length;
- Status = SpiFlashBlockErase( (UINTN) BaseAddress, &BytesErased);
+ BytesErased = (UINTN)FvHeader->BlockMap->Length;
+ Status = SpiFlashBlockErase ((UINTN)BaseAddress, &BytesErased);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_WARN, "ERROR - SpiFlashBlockErase Error %r\n", Status));
if (FvHeader != NULL) {
FreePool (FvHeader);
}
+
continue;
}
+
if (BytesErased != FvHeader->BlockMap->Length) {
DEBUG ((DEBUG_WARN, "ERROR - BytesErased != FvHeader->BlockMap->Length\n"));
DEBUG ((DEBUG_INFO, " BytesErased = 0x%X\n Length = 0x%X\n", BytesErased, FvHeader->BlockMap->Length));
if (FvHeader != NULL) {
FreePool (FvHeader);
}
+
continue;
}
- BytesWritten = FvHeader->HeaderLength;
- Status = SpiFlashWrite ((UINTN)BaseAddress, &BytesWritten, (UINT8*)FvHeader);
+
+ if (Idx != 0) {
+ BytesWritten = FvHeader->HeaderLength;
+ ExpectedBytesWritten = BytesWritten;
+ Status = SpiFlashWrite ((UINTN)BaseAddress, &BytesWritten, (UINT8 *)FvHeader);
+ } else {
+ //
+ // This is Variable Store, rewrite both EFI_FIRMWARE_VOLUME_HEADER and VARIABLE_STORE_HEADER
+ //
+ NvStoreBuffer = NULL;
+ NvStoreBuffer = AllocateZeroPool (sizeof (VARIABLE_STORE_HEADER) + FvHeader->HeaderLength);
+ if (NvStoreBuffer != NULL) {
+ //
+ // Combine FV header and VariableStore header into the buffer.
+ //
+ CopyMem (NvStoreBuffer, FvHeader, FvHeader->HeaderLength);
+ VariableStoreHeader = (VARIABLE_STORE_HEADER *)(NvStoreBuffer + FvHeader->HeaderLength);
+ VariableStoreType = PcdGet8 (PcdFlashVariableStoreType);
+ switch (VariableStoreType) {
+ case 0:
+ DEBUG ((DEBUG_ERROR, "gEfiVariableGuid\n"));
+ CopyGuid (&VariableStoreHeader->Signature, &gEfiVariableGuid);
+ break;
+ case 1:
+ DEBUG ((DEBUG_ERROR, "gEfiAuthenticatedVariableGuid\n"));
+ CopyGuid (&VariableStoreHeader->Signature, &gEfiAuthenticatedVariableGuid);
+ break;
+ default:
+ break;
+ }
+
+ //
+ // Initialize common VariableStore header fields
+ //
+ VariableStoreHeader->Size = PcdGet32 (PcdFlashNvStorageVariableSize) - FvHeader->HeaderLength;
+ VariableStoreHeader->Format = VARIABLE_STORE_FORMATTED;
+ VariableStoreHeader->State = VARIABLE_STORE_HEALTHY;
+ VariableStoreHeader->Reserved = 0;
+ VariableStoreHeader->Reserved1 = 0;
+ //
+ // Write buffer to flash
+ //
+ BytesWritten = FvHeader->HeaderLength + sizeof (VARIABLE_STORE_HEADER);
+ ExpectedBytesWritten = BytesWritten;
+ Status = SpiFlashWrite ((UINTN)BaseAddress, &BytesWritten, NvStoreBuffer);
+ FreePool (NvStoreBuffer);
+ }
+ }
+
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_WARN, "ERROR - SpiFlashWrite Error %r\n", Status));
if (FvHeader != NULL) {
FreePool (FvHeader);
}
+
continue;
}
- if (BytesWritten != FvHeader->HeaderLength) {
- DEBUG ((DEBUG_WARN, "ERROR - BytesWritten != HeaderLength\n"));
- DEBUG ((DEBUG_INFO, " BytesWritten = 0x%X\n HeaderLength = 0x%X\n", BytesWritten, FvHeader->HeaderLength));
+
+ if (BytesWritten != ExpectedBytesWritten) {
+ DEBUG ((DEBUG_WARN, "ERROR - BytesWritten != ExpectedBytesWritten\n"));
+ DEBUG ((DEBUG_INFO, " BytesWritten = 0x%X\n ExpectedBytesWritten = 0x%X\n", BytesWritten, ExpectedBytesWritten));
if (FvHeader != NULL) {
FreePool (FvHeader);
}
+
continue;
}
+
Status = SpiFlashLock ();
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_WARN, "ERROR - SpiFlashLock Error %r\n", Status));
if (FvHeader != NULL) {
FreePool (FvHeader);
}
+
continue;
}
+
DEBUG ((DEBUG_INFO, "FV Header @ 0x%X restored with static data\n", BaseAddress));
//
// Clear cache for this range.
//
- WriteBackInvalidateDataCacheRange ( (VOID *) (UINTN) BaseAddress, FvHeader->BlockMap->Length);
+ WriteBackInvalidateDataCacheRange ((VOID *)(UINTN)BaseAddress, FvHeader->BlockMap->Length);
if (FvHeader != NULL) {
FreePool (FvHeader);
}
@@ -227,11 +292,12 @@ FvbInitialize (
//
BufferSize = 0;
for (Idx = 0; ; Idx++) {
- if (mPlatformFvBaseAddress[Idx].FvSize == 0 && mPlatformFvBaseAddress[Idx].FvBase == 0) {
+ if ((mPlatformFvBaseAddress[Idx].FvSize == 0) && (mPlatformFvBaseAddress[Idx].FvBase == 0)) {
break;
}
+
BaseAddress = mPlatformFvBaseAddress[Idx].FvBase;
- FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) BaseAddress;
+ FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)BaseAddress;
if (!IsFvHeaderValid (BaseAddress, FvHeader)) {
DEBUG ((DEBUG_WARN, "ERROR - The FV in 0x%x is invalid!\n", FvHeader));
@@ -239,27 +305,28 @@ FvbInitialize (
}
BufferSize += (FvHeader->HeaderLength +
- sizeof (EFI_FVB_INSTANCE) -
- sizeof (EFI_FIRMWARE_VOLUME_HEADER)
- );
+ sizeof (EFI_FVB_INSTANCE) -
+ sizeof (EFI_FIRMWARE_VOLUME_HEADER)
+ );
}
- mFvbModuleGlobal.FvbInstance = (EFI_FVB_INSTANCE *) AllocateRuntimeZeroPool (BufferSize);
+ mFvbModuleGlobal.FvbInstance = (EFI_FVB_INSTANCE *)AllocateRuntimeZeroPool (BufferSize);
if (mFvbModuleGlobal.FvbInstance == NULL) {
ASSERT (FALSE);
return;
}
- MaxLbaSize = 0;
- FvbInstance = mFvbModuleGlobal.FvbInstance;
- mFvbModuleGlobal.NumFv = 0;
+ MaxLbaSize = 0;
+ FvbInstance = mFvbModuleGlobal.FvbInstance;
+ mFvbModuleGlobal.NumFv = 0;
for (Idx = 0; ; Idx++) {
- if (mPlatformFvBaseAddress[Idx].FvSize == 0 && mPlatformFvBaseAddress[Idx].FvBase == 0) {
+ if ((mPlatformFvBaseAddress[Idx].FvSize == 0) && (mPlatformFvBaseAddress[Idx].FvBase == 0)) {
break;
}
+
BaseAddress = mPlatformFvBaseAddress[Idx].FvBase;
- FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) BaseAddress;
+ FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)BaseAddress;
if (!IsFvHeaderValid (BaseAddress, FvHeader)) {
DEBUG ((DEBUG_WARN, "ERROR - The FV in 0x%x is invalid!\n", FvHeader));
@@ -269,22 +336,24 @@ FvbInitialize (
FvbInstance->Signature = FVB_INSTANCE_SIGNATURE;
CopyMem (&(FvbInstance->FvHeader), FvHeader, FvHeader->HeaderLength);
- FvHeader = &(FvbInstance->FvHeader);
+ FvHeader = &(FvbInstance->FvHeader);
FvbInstance->FvBase = (UINTN)BaseAddress;
//
// Process the block map for each FV
//
- FvbInstance->NumOfBlocks = 0;
+ FvbInstance->NumOfBlocks = 0;
for (PtrBlockMapEntry = FvHeader->BlockMap;
PtrBlockMapEntry->NumBlocks != 0;
- PtrBlockMapEntry++) {
+ PtrBlockMapEntry++)
+ {
//
// Get the maximum size of a block.
//
if (MaxLbaSize < PtrBlockMapEntry->Length) {
- MaxLbaSize = PtrBlockMapEntry->Length;
+ MaxLbaSize = PtrBlockMapEntry->Length;
}
+
FvbInstance->NumOfBlocks += PtrBlockMapEntry->NumBlocks;
}
@@ -297,10 +366,9 @@ FvbInitialize (
//
// Move on to the next FvbInstance
//
- FvbInstance = (EFI_FVB_INSTANCE *) ((UINTN)((UINT8 *)FvbInstance) +
- FvHeader->HeaderLength +
- (sizeof (EFI_FVB_INSTANCE) - sizeof (EFI_FIRMWARE_VOLUME_HEADER)));
-
+ FvbInstance = (EFI_FVB_INSTANCE *)((UINTN)((UINT8 *)FvbInstance) +
+ FvHeader->HeaderLength +
+ (sizeof (EFI_FVB_INSTANCE) - sizeof (EFI_FIRMWARE_VOLUME_HEADER)));
}
}
}
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
index 0cfa3f909b..0485b73679 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
@@ -45,6 +45,8 @@
[Pcd]
gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CONSUMES
gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ## SOMETIMES_CONSUMES
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashVariableStoreType ## SOMETIMES_CONSUMES
[Sources]
FvbInfo.c
@@ -61,6 +63,8 @@
[Guids]
gEfiFirmwareFileSystem2Guid ## CONSUMES
gEfiSystemNvDataFvGuid ## CONSUMES
+ gEfiVariableGuid ## SOMETIMES_CONSUMES
+ gEfiAuthenticatedVariableGuid ## SOMETIMES_CONSUMES
[Depex]
TRUE
diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
index 485cb3e80a..63dae756ad 100644
--- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
+++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
@@ -186,3 +186,11 @@
# @Prompt VTd abort DMA mode support.
gIntelSiliconPkgTokenSpaceGuid.PcdVTdSupportAbortDmaMode|FALSE|BOOLEAN|0x0000000C
+ ## Define Flash Variable Store type.<BR><BR>
+ # When Flash Variable Store corruption happened, the SpiFvbService will recreate Variable Store
+ # with valid header information provided by this PCD value.<BR>
+ # 0: Variable Store is gEfiVariableGuid type.<BR>
+ # 1: Variable Store is gEfiAuthenticatedVariableGuid type.<BR>
+ # Other value: reserved for future use.<BR>
+ # @Prompt Flash Variable Store type.
+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashVariableStoreType|0x00|UINT8|0x0000000E
--
2.35.0.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [edk2-platforms: PATCH] IntelSiliconPkg/SpiFvbServiceSmm: Rewrite VariableStore header.
2023-02-03 21:55 [edk2-platforms: PATCH] IntelSiliconPkg/SpiFvbServiceSmm: Rewrite VariableStore header Chiu, Chasel
@ 2023-02-04 16:02 ` Ashraf Ali S
2023-02-06 18:52 ` Chiu, Chasel
0 siblings, 1 reply; 3+ messages in thread
From: Ashraf Ali S @ 2023-02-04 16:02 UTC (permalink / raw)
To: Chiu, Chasel, devel@edk2.groups.io
Cc: Oram, Isaac W, Chaganty, Rangasai V, Ni, Ray, Kubacki, Michael
Hi., Chasel
If the NvStoreBuffer AllocateZeroPool failed then Status variable needs to updated.
Can we have some meaning full debug message rather than just printing the GUID name.
ExpectedBytesWritten will be uninitialed when NvStoreBuffer allocation fails. Due to which comparison of BytesWritten and ExpectedBytesWritten will be incorrect.
-----Original Message-----
From: Chiu, Chasel <chasel.chiu@intel.com>
Sent: Saturday, February 4, 2023 3:25 AM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@intel.com>; S, Ashraf Ali <ashraf.ali.s@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Ni, Ray <ray.ni@intel.com>; Kubacki, Michael <michael.kubacki@microsoft.com>
Subject: [edk2-platforms: PATCH] IntelSiliconPkg/SpiFvbServiceSmm: Rewrite VariableStore header.
When invalid VariableStore FV header detected, current SpiFvbService will erase both FV and VariableStore headers from flash, however, it will only rewrite FV header back and cause invalid VariableStore header.
This patch adding the support for rewriting both FV header and VariableStore header when VariableStore corruption happened.
Platform has to set PcdFlashVariableStoreType to inform SpiFvbService which VariableStoreType should be rewritten.
Cc: Ashraf Ali S <ashraf.ali.s@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
---
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c | 172 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf | 4 ++++
Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec | 8 ++++++++
3 files changed, 132 insertions(+), 52 deletions(-)
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c
index 6b4bcdcfe3..f29540c62c 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSe
+++ rviceMm.c
@@ -12,6 +12,7 @@
#include <Library/MmServicesTableLib.h> #include <Library/UefiDriverEntryPoint.h> #include <Protocol/SmmFirmwareVolumeBlock.h>+#include <Guid/VariableFormat.h> /** The function installs EFI_FIRMWARE_VOLUME_BLOCK protocol@@ -25,12 +26,12 @@
**/ VOID InstallFvbProtocol (- IN EFI_FVB_INSTANCE *FvbInstance+ IN EFI_FVB_INSTANCE *FvbInstance ) {- EFI_FIRMWARE_VOLUME_HEADER *FvHeader;- EFI_STATUS Status;- EFI_HANDLE FvbHandle;+ EFI_FIRMWARE_VOLUME_HEADER *FvHeader;+ EFI_STATUS Status;+ EFI_HANDLE FvbHandle; ASSERT (FvbInstance != NULL); if (FvbInstance == NULL) {@@ -52,19 +53,21 @@ InstallFvbProtocol (
// // FV does not contains extension header, then produce MEMMAP_DEVICE_PATH //- FvbInstance->DevicePath = (EFI_DEVICE_PATH_PROTOCOL *) AllocateRuntimeCopyPool (sizeof (FV_MEMMAP_DEVICE_PATH), &mFvMemmapDevicePathTemplate);+ FvbInstance->DevicePath = (EFI_DEVICE_PATH_PROTOCOL *)AllocateRuntimeCopyPool (sizeof (FV_MEMMAP_DEVICE_PATH), &mFvMemmapDevicePathTemplate); if (FvbInstance->DevicePath == NULL) { DEBUG ((DEBUG_INFO, "SpiFvbServiceSmm.c: Memory allocation for MEMMAP_DEVICE_PATH failed\n")); return; }- ((FV_MEMMAP_DEVICE_PATH *) FvbInstance->DevicePath)->MemMapDevPath.StartingAddress = FvbInstance->FvBase;- ((FV_MEMMAP_DEVICE_PATH *) FvbInstance->DevicePath)->MemMapDevPath.EndingAddress = FvbInstance->FvBase + FvHeader->FvLength - 1;++ ((FV_MEMMAP_DEVICE_PATH *)FvbInstance->DevicePath)->MemMapDevPath.StartingAddress = FvbInstance->FvBase;+ ((FV_MEMMAP_DEVICE_PATH *)FvbInstance->DevicePath)->MemMapDevPath.EndingAddress = FvbInstance->FvBase + FvHeader->FvLength - 1; } else {- FvbInstance->DevicePath = (EFI_DEVICE_PATH_PROTOCOL *) AllocateRuntimeCopyPool (sizeof (FV_PIWG_DEVICE_PATH), &mFvPIWGDevicePathTemplate);+ FvbInstance->DevicePath = (EFI_DEVICE_PATH_PROTOCOL *)AllocateRuntimeCopyPool (sizeof (FV_PIWG_DEVICE_PATH), &mFvPIWGDevicePathTemplate); if (FvbInstance->DevicePath == NULL) { DEBUG ((DEBUG_INFO, "SpiFvbServiceSmm.c: Memory allocation for FV_PIWG_DEVICE_PATH failed\n")); return; }+ CopyGuid ( &((FV_PIWG_DEVICE_PATH *)FvbInstance->DevicePath)->FvDevPath.FvName, (GUID *)(UINTN)(FvbInstance->FvBase + FvHeader->ExtHeaderOffset)@@ -103,17 +106,21 @@ FvbInitialize (
VOID ) {- EFI_FVB_INSTANCE *FvbInstance;- EFI_FIRMWARE_VOLUME_HEADER *FvHeader;- EFI_FV_BLOCK_MAP_ENTRY *PtrBlockMapEntry;- EFI_PHYSICAL_ADDRESS BaseAddress;- EFI_STATUS Status;- UINTN BufferSize;- UINTN Idx;- UINT32 MaxLbaSize;- UINT32 BytesWritten;- UINTN BytesErased;- UINT64 NvStorageFvSize;+ EFI_FVB_INSTANCE *FvbInstance;+ EFI_FIRMWARE_VOLUME_HEADER *FvHeader;+ EFI_FV_BLOCK_MAP_ENTRY *PtrBlockMapEntry;+ EFI_PHYSICAL_ADDRESS BaseAddress;+ EFI_STATUS Status;+ UINTN BufferSize;+ UINTN Idx;+ UINT32 MaxLbaSize;+ UINT32 BytesWritten;+ UINTN BytesErased;+ UINT64 NvStorageFvSize;+ UINT32 ExpectedBytesWritten;+ VARIABLE_STORE_HEADER *VariableStoreHeader;+ UINT8 VariableStoreType;+ UINT8 *NvStoreBuffer; Status = GetVariableFlashNvStorageInfo (&BaseAddress, &NvStorageFvSize); if (EFI_ERROR (Status)) {@@ -129,6 +136,7 @@ FvbInitialize (
DEBUG ((DEBUG_ERROR, "[%a] - 64-bit variable storage base address not supported.\n", __FUNCTION__)); return; }+ Status = SafeUint64ToUint32 (NvStorageFvSize, &mPlatformFvBaseAddress[0].FvSize); if (EFI_ERROR (Status)) { ASSERT_EFI_ERROR (Status);@@ -136,8 +144,8 @@ FvbInitialize (
return; } - mPlatformFvBaseAddress[1].FvBase = PcdGet32(PcdFlashMicrocodeFvBase);- mPlatformFvBaseAddress[1].FvSize = PcdGet32(PcdFlashMicrocodeFvSize);+ mPlatformFvBaseAddress[1].FvBase = PcdGet32 (PcdFlashMicrocodeFvBase);+ mPlatformFvBaseAddress[1].FvSize = PcdGet32 (PcdFlashMicrocodeFvSize); // // We will only continue with FVB installation if the@@ -147,17 +155,17 @@ FvbInitialize (
// // Make sure all FVB are valid and/or fix if possible //- for (Idx = 0;; Idx++) {- if (mPlatformFvBaseAddress[Idx].FvSize == 0 && mPlatformFvBaseAddress[Idx].FvBase == 0) {+ for (Idx = 0; ; Idx++) {+ if ((mPlatformFvBaseAddress[Idx].FvSize == 0) && (mPlatformFvBaseAddress[Idx].FvBase == 0)) { break; } BaseAddress = mPlatformFvBaseAddress[Idx].FvBase;- FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) BaseAddress;+ FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)BaseAddress; if (!IsFvHeaderValid (BaseAddress, FvHeader)) { BytesWritten = 0;- BytesErased = 0;+ BytesErased = 0; DEBUG ((DEBUG_ERROR, "ERROR - The FV in 0x%x is invalid!\n", FvHeader)); FvHeader = NULL; Status = GetFvbInfo (BaseAddress, &FvHeader);@@ -165,57 +173,114 @@ FvbInitialize (
DEBUG ((DEBUG_WARN, "ERROR - Can't recovery FV header at 0x%x. GetFvbInfo Status %r\n", BaseAddress, Status)); continue; }+ DEBUG ((DEBUG_INFO, "Rewriting FV header at 0x%X with static data\n", BaseAddress)); // // Spi erase //- BytesErased = (UINTN) FvHeader->BlockMap->Length;- Status = SpiFlashBlockErase( (UINTN) BaseAddress, &BytesErased);+ BytesErased = (UINTN)FvHeader->BlockMap->Length;+ Status = SpiFlashBlockErase ((UINTN)BaseAddress, &BytesErased); if (EFI_ERROR (Status)) { DEBUG ((DEBUG_WARN, "ERROR - SpiFlashBlockErase Error %r\n", Status)); if (FvHeader != NULL) { FreePool (FvHeader); }+ continue; }+ if (BytesErased != FvHeader->BlockMap->Length) { DEBUG ((DEBUG_WARN, "ERROR - BytesErased != FvHeader->BlockMap->Length\n")); DEBUG ((DEBUG_INFO, " BytesErased = 0x%X\n Length = 0x%X\n", BytesErased, FvHeader->BlockMap->Length)); if (FvHeader != NULL) { FreePool (FvHeader); }+ continue; }- BytesWritten = FvHeader->HeaderLength;- Status = SpiFlashWrite ((UINTN)BaseAddress, &BytesWritten, (UINT8*)FvHeader);++ if (Idx != 0) {+ BytesWritten = FvHeader->HeaderLength;+ ExpectedBytesWritten = BytesWritten;+ Status = SpiFlashWrite ((UINTN)BaseAddress, &BytesWritten, (UINT8 *)FvHeader);+ } else {+ //+ // This is Variable Store, rewrite both EFI_FIRMWARE_VOLUME_HEADER and VARIABLE_STORE_HEADER+ //+ NvStoreBuffer = NULL;+ NvStoreBuffer = AllocateZeroPool (sizeof (VARIABLE_STORE_HEADER) + FvHeader->HeaderLength);+ if (NvStoreBuffer != NULL) {+ //+ // Combine FV header and VariableStore header into the buffer.+ //+ CopyMem (NvStoreBuffer, FvHeader, FvHeader->HeaderLength);+ VariableStoreHeader = (VARIABLE_STORE_HEADER *)(NvStoreBuffer + FvHeader->HeaderLength);+ VariableStoreType = PcdGet8 (PcdFlashVariableStoreType);+ switch (VariableStoreType) {+ case 0:+ DEBUG ((DEBUG_ERROR, "gEfiVariableGuid\n"));+ CopyGuid (&VariableStoreHeader->Signature, &gEfiVariableGuid);+ break;+ case 1:+ DEBUG ((DEBUG_ERROR, "gEfiAuthenticatedVariableGuid\n"));+ CopyGuid (&VariableStoreHeader->Signature, &gEfiAuthenticatedVariableGuid);+ break;+ default:+ break;+ }++ //+ // Initialize common VariableStore header fields+ //+ VariableStoreHeader->Size = PcdGet32 (PcdFlashNvStorageVariableSize) - FvHeader->HeaderLength;+ VariableStoreHeader->Format = VARIABLE_STORE_FORMATTED;+ VariableStoreHeader->State = VARIABLE_STORE_HEALTHY;+ VariableStoreHeader->Reserved = 0;+ VariableStoreHeader->Reserved1 = 0;+ //+ // Write buffer to flash+ //+ BytesWritten = FvHeader->HeaderLength + sizeof (VARIABLE_STORE_HEADER);+ ExpectedBytesWritten = BytesWritten;+ Status = SpiFlashWrite ((UINTN)BaseAddress, &BytesWritten, NvStoreBuffer);+ FreePool (NvStoreBuffer);+ }+ }+ if (EFI_ERROR (Status)) { DEBUG ((DEBUG_WARN, "ERROR - SpiFlashWrite Error %r\n", Status)); if (FvHeader != NULL) { FreePool (FvHeader); }+ continue; }- if (BytesWritten != FvHeader->HeaderLength) {- DEBUG ((DEBUG_WARN, "ERROR - BytesWritten != HeaderLength\n"));- DEBUG ((DEBUG_INFO, " BytesWritten = 0x%X\n HeaderLength = 0x%X\n", BytesWritten, FvHeader->HeaderLength));++ if (BytesWritten != ExpectedBytesWritten) {+ DEBUG ((DEBUG_WARN, "ERROR - BytesWritten != ExpectedBytesWritten\n"));+ DEBUG ((DEBUG_INFO, " BytesWritten = 0x%X\n ExpectedBytesWritten = 0x%X\n", BytesWritten, ExpectedBytesWritten)); if (FvHeader != NULL) { FreePool (FvHeader); }+ continue; }+ Status = SpiFlashLock (); if (EFI_ERROR (Status)) { DEBUG ((DEBUG_WARN, "ERROR - SpiFlashLock Error %r\n", Status)); if (FvHeader != NULL) { FreePool (FvHeader); }+ continue; }+ DEBUG ((DEBUG_INFO, "FV Header @ 0x%X restored with static data\n", BaseAddress)); // // Clear cache for this range. //- WriteBackInvalidateDataCacheRange ( (VOID *) (UINTN) BaseAddress, FvHeader->BlockMap->Length);+ WriteBackInvalidateDataCacheRange ((VOID *)(UINTN)BaseAddress, FvHeader->BlockMap->Length); if (FvHeader != NULL) { FreePool (FvHeader); }@@ -227,11 +292,12 @@ FvbInitialize (
// BufferSize = 0; for (Idx = 0; ; Idx++) {- if (mPlatformFvBaseAddress[Idx].FvSize == 0 && mPlatformFvBaseAddress[Idx].FvBase == 0) {+ if ((mPlatformFvBaseAddress[Idx].FvSize == 0) && (mPlatformFvBaseAddress[Idx].FvBase == 0)) { break; }+ BaseAddress = mPlatformFvBaseAddress[Idx].FvBase;- FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) BaseAddress;+ FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)BaseAddress; if (!IsFvHeaderValid (BaseAddress, FvHeader)) { DEBUG ((DEBUG_WARN, "ERROR - The FV in 0x%x is invalid!\n", FvHeader));@@ -239,27 +305,28 @@ FvbInitialize (
} BufferSize += (FvHeader->HeaderLength +- sizeof (EFI_FVB_INSTANCE) -- sizeof (EFI_FIRMWARE_VOLUME_HEADER)- );+ sizeof (EFI_FVB_INSTANCE) -+ sizeof (EFI_FIRMWARE_VOLUME_HEADER)+ ); } - mFvbModuleGlobal.FvbInstance = (EFI_FVB_INSTANCE *) AllocateRuntimeZeroPool (BufferSize);+ mFvbModuleGlobal.FvbInstance = (EFI_FVB_INSTANCE *)AllocateRuntimeZeroPool (BufferSize); if (mFvbModuleGlobal.FvbInstance == NULL) { ASSERT (FALSE); return; } - MaxLbaSize = 0;- FvbInstance = mFvbModuleGlobal.FvbInstance;- mFvbModuleGlobal.NumFv = 0;+ MaxLbaSize = 0;+ FvbInstance = mFvbModuleGlobal.FvbInstance;+ mFvbModuleGlobal.NumFv = 0; for (Idx = 0; ; Idx++) {- if (mPlatformFvBaseAddress[Idx].FvSize == 0 && mPlatformFvBaseAddress[Idx].FvBase == 0) {+ if ((mPlatformFvBaseAddress[Idx].FvSize == 0) && (mPlatformFvBaseAddress[Idx].FvBase == 0)) { break; }+ BaseAddress = mPlatformFvBaseAddress[Idx].FvBase;- FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) BaseAddress;+ FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)BaseAddress; if (!IsFvHeaderValid (BaseAddress, FvHeader)) { DEBUG ((DEBUG_WARN, "ERROR - The FV in 0x%x is invalid!\n", FvHeader));@@ -269,22 +336,24 @@ FvbInitialize (
FvbInstance->Signature = FVB_INSTANCE_SIGNATURE; CopyMem (&(FvbInstance->FvHeader), FvHeader, FvHeader->HeaderLength); - FvHeader = &(FvbInstance->FvHeader);+ FvHeader = &(FvbInstance->FvHeader); FvbInstance->FvBase = (UINTN)BaseAddress; // // Process the block map for each FV //- FvbInstance->NumOfBlocks = 0;+ FvbInstance->NumOfBlocks = 0; for (PtrBlockMapEntry = FvHeader->BlockMap; PtrBlockMapEntry->NumBlocks != 0;- PtrBlockMapEntry++) {+ PtrBlockMapEntry++)+ { // // Get the maximum size of a block. // if (MaxLbaSize < PtrBlockMapEntry->Length) {- MaxLbaSize = PtrBlockMapEntry->Length;+ MaxLbaSize = PtrBlockMapEntry->Length; }+ FvbInstance->NumOfBlocks += PtrBlockMapEntry->NumBlocks; } @@ -297,10 +366,9 @@ FvbInitialize (
// // Move on to the next FvbInstance //- FvbInstance = (EFI_FVB_INSTANCE *) ((UINTN)((UINT8 *)FvbInstance) +- FvHeader->HeaderLength +- (sizeof (EFI_FVB_INSTANCE) - sizeof (EFI_FIRMWARE_VOLUME_HEADER)));-+ FvbInstance = (EFI_FVB_INSTANCE *)((UINTN)((UINT8 *)FvbInstance) ++ FvHeader->HeaderLength ++ (sizeof (EFI_FVB_INSTANCE) - sizeof (EFI_FIRMWARE_VOLUME_HEADER))); } } }diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
index 0cfa3f909b..0485b73679 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSe
+++ rviceSmm.inf
@@ -45,6 +45,8 @@
[Pcd] gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CONSUMES gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CONSUMES+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ## SOMETIMES_CONSUMES+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashVariableStoreType ## SOMETIMES_CONSUMES [Sources] FvbInfo.c@@ -61,6 +63,8 @@
[Guids] gEfiFirmwareFileSystem2Guid ## CONSUMES gEfiSystemNvDataFvGuid ## CONSUMES+ gEfiVariableGuid ## SOMETIMES_CONSUMES+ gEfiAuthenticatedVariableGuid ## SOMETIMES_CONSUMES [Depex] TRUEdiff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
index 485cb3e80a..63dae756ad 100644
--- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
+++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
@@ -186,3 +186,11 @@
# @Prompt VTd abort DMA mode support. gIntelSiliconPkgTokenSpaceGuid.PcdVTdSupportAbortDmaMode|FALSE|BOOLEAN|0x0000000C + ## Define Flash Variable Store type.<BR><BR>+ # When Flash Variable Store corruption happened, the SpiFvbService will recreate Variable Store+ # with valid header information provided by this PCD value.<BR>+ # 0: Variable Store is gEfiVariableGuid type.<BR>+ # 1: Variable Store is gEfiAuthenticatedVariableGuid type.<BR>+ # Other value: reserved for future use.<BR>+ # @Prompt Flash Variable Store type.+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashVariableStoreType|0x00|UINT8|0x0000000E--
2.35.0.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [edk2-platforms: PATCH] IntelSiliconPkg/SpiFvbServiceSmm: Rewrite VariableStore header.
2023-02-04 16:02 ` Ashraf Ali S
@ 2023-02-06 18:52 ` Chiu, Chasel
0 siblings, 0 replies; 3+ messages in thread
From: Chiu, Chasel @ 2023-02-06 18:52 UTC (permalink / raw)
To: S, Ashraf Ali, devel@edk2.groups.io
Cc: Oram, Isaac W, Chaganty, Rangasai V, Ni, Ray, Kubacki, Michael
Thanks for the good catch Ashraf!
I have sent V2 patch for addressing your feedbacks, please help to review again.
Thanks,
Chasel
> -----Original Message-----
> From: S, Ashraf Ali <ashraf.ali.s@intel.com>
> Sent: Saturday, February 4, 2023 8:03 AM
> To: Chiu, Chasel <chasel.chiu@intel.com>; devel@edk2.groups.io
> Cc: Oram, Isaac W <isaac.w.oram@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Ni, Ray <ray.ni@intel.com>; Kubacki,
> Michael <michael.kubacki@microsoft.com>
> Subject: RE: [edk2-platforms: PATCH] IntelSiliconPkg/SpiFvbServiceSmm:
> Rewrite VariableStore header.
>
> Hi., Chasel
>
> If the NvStoreBuffer AllocateZeroPool failed then Status variable needs to
> updated.
> Can we have some meaning full debug message rather than just printing the
> GUID name.
> ExpectedBytesWritten will be uninitialed when NvStoreBuffer allocation fails.
> Due to which comparison of BytesWritten and ExpectedBytesWritten will be
> incorrect.
>
>
> -----Original Message-----
> From: Chiu, Chasel <chasel.chiu@intel.com>
> Sent: Saturday, February 4, 2023 3:25 AM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; S, Ashraf Ali <ashraf.ali.s@intel.com>;
> Oram, Isaac W <isaac.w.oram@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Ni, Ray <ray.ni@intel.com>; Kubacki,
> Michael <michael.kubacki@microsoft.com>
> Subject: [edk2-platforms: PATCH] IntelSiliconPkg/SpiFvbServiceSmm: Rewrite
> VariableStore header.
>
> When invalid VariableStore FV header detected, current SpiFvbService will erase
> both FV and VariableStore headers from flash, however, it will only rewrite FV
> header back and cause invalid VariableStore header.
>
> This patch adding the support for rewriting both FV header and VariableStore
> header when VariableStore corruption happened.
>
> Platform has to set PcdFlashVariableStoreType to inform SpiFvbService which
> VariableStoreType should be rewritten.
>
> Cc: Ashraf Ali S <ashraf.ali.s@intel.com>
> Cc: Isaac Oram <isaac.w.oram@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> ---
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c |
> 172
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------
> -------------------------------------
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
> | 4 ++++
> Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec | 8 ++++++++
> 3 files changed, 132 insertions(+), 52 deletions(-)
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c
> index 6b4bcdcfe3..f29540c62c 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSe
> +++ rviceMm.c
> @@ -12,6 +12,7 @@
> #include <Library/MmServicesTableLib.h> #include
> <Library/UefiDriverEntryPoint.h> #include
> <Protocol/SmmFirmwareVolumeBlock.h>+#include <Guid/VariableFormat.h>
> /** The function installs EFI_FIRMWARE_VOLUME_BLOCK protocol@@ -25,12
> +26,12 @@
> **/ VOID InstallFvbProtocol (- IN EFI_FVB_INSTANCE *FvbInstance+ IN
> EFI_FVB_INSTANCE *FvbInstance ) {- EFI_FIRMWARE_VOLUME_HEADER
> *FvHeader;- EFI_STATUS Status;- EFI_HANDLE
> FvbHandle;+ EFI_FIRMWARE_VOLUME_HEADER *FvHeader;+ EFI_STATUS
> Status;+ EFI_HANDLE FvbHandle; ASSERT (FvbInstance != NULL); if
> (FvbInstance == NULL) {@@ -52,19 +53,21 @@ InstallFvbProtocol (
> // // FV does not contains extension header, then produce
> MEMMAP_DEVICE_PATH //- FvbInstance->DevicePath =
> (EFI_DEVICE_PATH_PROTOCOL *) AllocateRuntimeCopyPool (sizeof
> (FV_MEMMAP_DEVICE_PATH), &mFvMemmapDevicePathTemplate);+
> FvbInstance->DevicePath = (EFI_DEVICE_PATH_PROTOCOL
> *)AllocateRuntimeCopyPool (sizeof (FV_MEMMAP_DEVICE_PATH),
> &mFvMemmapDevicePathTemplate); if (FvbInstance->DevicePath == NULL)
> { DEBUG ((DEBUG_INFO, "SpiFvbServiceSmm.c: Memory allocation for
> MEMMAP_DEVICE_PATH failed\n")); return; }-
> ((FV_MEMMAP_DEVICE_PATH *) FvbInstance->DevicePath)-
> >MemMapDevPath.StartingAddress = FvbInstance->FvBase;-
> ((FV_MEMMAP_DEVICE_PATH *) FvbInstance->DevicePath)-
> >MemMapDevPath.EndingAddress = FvbInstance->FvBase + FvHeader-
> >FvLength - 1;++ ((FV_MEMMAP_DEVICE_PATH *)FvbInstance->DevicePath)-
> >MemMapDevPath.StartingAddress = FvbInstance->FvBase;+
> ((FV_MEMMAP_DEVICE_PATH *)FvbInstance->DevicePath)-
> >MemMapDevPath.EndingAddress = FvbInstance->FvBase + FvHeader-
> >FvLength - 1; } else {- FvbInstance->DevicePath =
> (EFI_DEVICE_PATH_PROTOCOL *) AllocateRuntimeCopyPool (sizeof
> (FV_PIWG_DEVICE_PATH), &mFvPIWGDevicePathTemplate);+ FvbInstance-
> >DevicePath = (EFI_DEVICE_PATH_PROTOCOL *)AllocateRuntimeCopyPool
> (sizeof (FV_PIWG_DEVICE_PATH), &mFvPIWGDevicePathTemplate); if
> (FvbInstance->DevicePath == NULL) { DEBUG ((DEBUG_INFO,
> "SpiFvbServiceSmm.c: Memory allocation for FV_PIWG_DEVICE_PATH
> failed\n")); return; }+ CopyGuid ( &((FV_PIWG_DEVICE_PATH
> *)FvbInstance->DevicePath)->FvDevPath.FvName, (GUID
> *)(UINTN)(FvbInstance->FvBase + FvHeader->ExtHeaderOffset)@@ -103,17
> +106,21 @@ FvbInitialize (
> VOID ) {- EFI_FVB_INSTANCE *FvbInstance;-
> EFI_FIRMWARE_VOLUME_HEADER *FvHeader;-
> EFI_FV_BLOCK_MAP_ENTRY *PtrBlockMapEntry;-
> EFI_PHYSICAL_ADDRESS BaseAddress;- EFI_STATUS
> Status;- UINTN BufferSize;- UINTN Idx;-
> UINT32 MaxLbaSize;- UINT32 BytesWritten;-
> UINTN BytesErased;- UINT64
> NvStorageFvSize;+ EFI_FVB_INSTANCE *FvbInstance;+
> EFI_FIRMWARE_VOLUME_HEADER *FvHeader;+ EFI_FV_BLOCK_MAP_ENTRY
> *PtrBlockMapEntry;+ EFI_PHYSICAL_ADDRESS BaseAddress;+ EFI_STATUS
> Status;+ UINTN BufferSize;+ UINTN Idx;+ UINT32
> MaxLbaSize;+ UINT32 BytesWritten;+ UINTN
> BytesErased;+ UINT64 NvStorageFvSize;+ UINT32
> ExpectedBytesWritten;+ VARIABLE_STORE_HEADER *VariableStoreHeader;+
> UINT8 VariableStoreType;+ UINT8 *NvStoreBuffer;
> Status = GetVariableFlashNvStorageInfo (&BaseAddress, &NvStorageFvSize); if
> (EFI_ERROR (Status)) {@@ -129,6 +136,7 @@ FvbInitialize (
> DEBUG ((DEBUG_ERROR, "[%a] - 64-bit variable storage base address not
> supported.\n", __FUNCTION__)); return; }+ Status = SafeUint64ToUint32
> (NvStorageFvSize, &mPlatformFvBaseAddress[0].FvSize); if (EFI_ERROR (Status))
> { ASSERT_EFI_ERROR (Status);@@ -136,8 +144,8 @@ FvbInitialize (
> return; } - mPlatformFvBaseAddress[1].FvBase =
> PcdGet32(PcdFlashMicrocodeFvBase);- mPlatformFvBaseAddress[1].FvSize =
> PcdGet32(PcdFlashMicrocodeFvSize);+ mPlatformFvBaseAddress[1].FvBase =
> PcdGet32 (PcdFlashMicrocodeFvBase);+ mPlatformFvBaseAddress[1].FvSize =
> PcdGet32 (PcdFlashMicrocodeFvSize); // // We will only continue with FVB
> installation if the@@ -147,17 +155,17 @@ FvbInitialize (
> // // Make sure all FVB are valid and/or fix if possible //- for (Idx = 0;;
> Idx++) {- if (mPlatformFvBaseAddress[Idx].FvSize == 0 &&
> mPlatformFvBaseAddress[Idx].FvBase == 0) {+ for (Idx = 0; ; Idx++) {+ if
> ((mPlatformFvBaseAddress[Idx].FvSize == 0) &&
> (mPlatformFvBaseAddress[Idx].FvBase == 0)) { break; } BaseAddress
> = mPlatformFvBaseAddress[Idx].FvBase;- FvHeader =
> (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) BaseAddress;+ FvHeader =
> (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)BaseAddress; if
> (!IsFvHeaderValid (BaseAddress, FvHeader)) { BytesWritten = 0;-
> BytesErased = 0;+ BytesErased = 0; DEBUG ((DEBUG_ERROR, "ERROR -
> The FV in 0x%x is invalid!\n", FvHeader)); FvHeader = NULL; Status =
> GetFvbInfo (BaseAddress, &FvHeader);@@ -165,57 +173,114 @@ FvbInitialize (
> DEBUG ((DEBUG_WARN, "ERROR - Can't recovery FV header at 0x%x.
> GetFvbInfo Status %r\n", BaseAddress, Status)); continue; }+
> DEBUG ((DEBUG_INFO, "Rewriting FV header at 0x%X with static data\n",
> BaseAddress)); // // Spi erase //- BytesErased = (UINTN)
> FvHeader->BlockMap->Length;- Status = SpiFlashBlockErase( (UINTN)
> BaseAddress, &BytesErased);+ BytesErased = (UINTN)FvHeader->BlockMap-
> >Length;+ Status = SpiFlashBlockErase ((UINTN)BaseAddress,
> &BytesErased); if (EFI_ERROR (Status)) { DEBUG ((DEBUG_WARN,
> "ERROR - SpiFlashBlockErase Error %r\n", Status)); if (FvHeader != NULL)
> { FreePool (FvHeader); }+ continue; }+ if
> (BytesErased != FvHeader->BlockMap->Length) { DEBUG ((DEBUG_WARN,
> "ERROR - BytesErased != FvHeader->BlockMap->Length\n")); DEBUG
> ((DEBUG_INFO, " BytesErased = 0x%X\n Length = 0x%X\n", BytesErased,
> FvHeader->BlockMap->Length)); if (FvHeader != NULL) { FreePool
> (FvHeader); }+ continue; }- BytesWritten = FvHeader-
> >HeaderLength;- Status = SpiFlashWrite ((UINTN)BaseAddress,
> &BytesWritten, (UINT8*)FvHeader);++ if (Idx != 0) {+ BytesWritten
> = FvHeader->HeaderLength;+ ExpectedBytesWritten = BytesWritten;+
> Status = SpiFlashWrite ((UINTN)BaseAddress, &BytesWritten, (UINT8
> *)FvHeader);+ } else {+ //+ // This is Variable Store, rewrite both
> EFI_FIRMWARE_VOLUME_HEADER and VARIABLE_STORE_HEADER+ //+
> NvStoreBuffer = NULL;+ NvStoreBuffer = AllocateZeroPool (sizeof
> (VARIABLE_STORE_HEADER) + FvHeader->HeaderLength);+ if
> (NvStoreBuffer != NULL) {+ //+ // Combine FV header and
> VariableStore header into the buffer.+ //+ CopyMem (NvStoreBuffer,
> FvHeader, FvHeader->HeaderLength);+ VariableStoreHeader =
> (VARIABLE_STORE_HEADER *)(NvStoreBuffer + FvHeader->HeaderLength);+
> VariableStoreType = PcdGet8 (PcdFlashVariableStoreType);+ switch
> (VariableStoreType) {+ case 0:+ DEBUG ((DEBUG_ERROR,
> "gEfiVariableGuid\n"));+ CopyGuid (&VariableStoreHeader->Signature,
> &gEfiVariableGuid);+ break;+ case 1:+ DEBUG
> ((DEBUG_ERROR, "gEfiAuthenticatedVariableGuid\n"));+ CopyGuid
> (&VariableStoreHeader->Signature, &gEfiAuthenticatedVariableGuid);+
> break;+ default:+ break;+ }++ //+ // Initialize
> common VariableStore header fields+ //+ VariableStoreHeader-
> >Size = PcdGet32 (PcdFlashNvStorageVariableSize) - FvHeader-
> >HeaderLength;+ VariableStoreHeader->Format =
> VARIABLE_STORE_FORMATTED;+ VariableStoreHeader->State =
> VARIABLE_STORE_HEALTHY;+ VariableStoreHeader->Reserved = 0;+
> VariableStoreHeader->Reserved1 = 0;+ //+ // Write buffer to flash+
> //+ BytesWritten = FvHeader->HeaderLength + sizeof
> (VARIABLE_STORE_HEADER);+ ExpectedBytesWritten = BytesWritten;+
> Status = SpiFlashWrite ((UINTN)BaseAddress, &BytesWritten,
> NvStoreBuffer);+ FreePool (NvStoreBuffer);+ }+ }+ if
> (EFI_ERROR (Status)) { DEBUG ((DEBUG_WARN, "ERROR - SpiFlashWrite
> Error %r\n", Status)); if (FvHeader != NULL) { FreePool
> (FvHeader); }+ continue; }- if (BytesWritten != FvHeader-
> >HeaderLength) {- DEBUG ((DEBUG_WARN, "ERROR - BytesWritten !=
> HeaderLength\n"));- DEBUG ((DEBUG_INFO, " BytesWritten = 0x%X\n
> HeaderLength = 0x%X\n", BytesWritten, FvHeader->HeaderLength));++ if
> (BytesWritten != ExpectedBytesWritten) {+ DEBUG ((DEBUG_WARN,
> "ERROR - BytesWritten != ExpectedBytesWritten\n"));+ DEBUG
> ((DEBUG_INFO, " BytesWritten = 0x%X\n ExpectedBytesWritten = 0x%X\n",
> BytesWritten, ExpectedBytesWritten)); if (FvHeader != NULL)
> { FreePool (FvHeader); }+ continue; }+ Status =
> SpiFlashLock (); if (EFI_ERROR (Status)) { DEBUG ((DEBUG_WARN,
> "ERROR - SpiFlashLock Error %r\n", Status)); if (FvHeader != NULL)
> { FreePool (FvHeader); }+ continue; }+ DEBUG
> ((DEBUG_INFO, "FV Header @ 0x%X restored with static data\n", BaseAddress));
> // // Clear cache for this range. //-
> WriteBackInvalidateDataCacheRange ( (VOID *) (UINTN) BaseAddress,
> FvHeader->BlockMap->Length);+ WriteBackInvalidateDataCacheRange
> ((VOID *)(UINTN)BaseAddress, FvHeader->BlockMap->Length); if
> (FvHeader != NULL) { FreePool (FvHeader); }@@ -227,11 +292,12 @@
> FvbInitialize (
> // BufferSize = 0; for (Idx = 0; ; Idx++) {- if
> (mPlatformFvBaseAddress[Idx].FvSize == 0 &&
> mPlatformFvBaseAddress[Idx].FvBase == 0) {+ if
> ((mPlatformFvBaseAddress[Idx].FvSize == 0) &&
> (mPlatformFvBaseAddress[Idx].FvBase == 0)) { break; }+ BaseAddress
> = mPlatformFvBaseAddress[Idx].FvBase;- FvHeader =
> (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) BaseAddress;+ FvHeader =
> (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)BaseAddress; if
> (!IsFvHeaderValid (BaseAddress, FvHeader)) { DEBUG ((DEBUG_WARN,
> "ERROR - The FV in 0x%x is invalid!\n", FvHeader));@@ -239,27 +305,28 @@
> FvbInitialize (
> } BufferSize += (FvHeader->HeaderLength +- sizeof
> (EFI_FVB_INSTANCE) -- sizeof (EFI_FIRMWARE_VOLUME_HEADER)-
> );+ sizeof (EFI_FVB_INSTANCE) -+ sizeof
> (EFI_FIRMWARE_VOLUME_HEADER)+ ); } -
> mFvbModuleGlobal.FvbInstance = (EFI_FVB_INSTANCE *)
> AllocateRuntimeZeroPool (BufferSize);+ mFvbModuleGlobal.FvbInstance =
> (EFI_FVB_INSTANCE *)AllocateRuntimeZeroPool (BufferSize); if
> (mFvbModuleGlobal.FvbInstance == NULL) { ASSERT (FALSE); return; } -
> MaxLbaSize = 0;- FvbInstance = mFvbModuleGlobal.FvbInstance;-
> mFvbModuleGlobal.NumFv = 0;+ MaxLbaSize = 0;+ FvbInstance
> = mFvbModuleGlobal.FvbInstance;+ mFvbModuleGlobal.NumFv = 0; for (Idx
> = 0; ; Idx++) {- if (mPlatformFvBaseAddress[Idx].FvSize == 0 &&
> mPlatformFvBaseAddress[Idx].FvBase == 0) {+ if
> ((mPlatformFvBaseAddress[Idx].FvSize == 0) &&
> (mPlatformFvBaseAddress[Idx].FvBase == 0)) { break; }+ BaseAddress
> = mPlatformFvBaseAddress[Idx].FvBase;- FvHeader =
> (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) BaseAddress;+ FvHeader =
> (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)BaseAddress; if
> (!IsFvHeaderValid (BaseAddress, FvHeader)) { DEBUG ((DEBUG_WARN,
> "ERROR - The FV in 0x%x is invalid!\n", FvHeader));@@ -269,22 +336,24 @@
> FvbInitialize (
> FvbInstance->Signature = FVB_INSTANCE_SIGNATURE; CopyMem
> (&(FvbInstance->FvHeader), FvHeader, FvHeader->HeaderLength); - FvHeader
> = &(FvbInstance->FvHeader);+ FvHeader = &(FvbInstance->FvHeader);
> FvbInstance->FvBase = (UINTN)BaseAddress; // // Process the block map
> for each FV //- FvbInstance->NumOfBlocks = 0;+ FvbInstance-
> >NumOfBlocks = 0; for (PtrBlockMapEntry = FvHeader->BlockMap;
> PtrBlockMapEntry->NumBlocks != 0;- PtrBlockMapEntry++) {+
> PtrBlockMapEntry++)+ { // // Get the maximum size of a block.
> // if (MaxLbaSize < PtrBlockMapEntry->Length) {- MaxLbaSize =
> PtrBlockMapEntry->Length;+ MaxLbaSize = PtrBlockMapEntry-
> >Length; }+ FvbInstance->NumOfBlocks += PtrBlockMapEntry-
> >NumBlocks; } @@ -297,10 +366,9 @@ FvbInitialize (
> // // Move on to the next FvbInstance //- FvbInstance =
> (EFI_FVB_INSTANCE *) ((UINTN)((UINT8 *)FvbInstance) +-
> FvHeader->HeaderLength +- (sizeof (EFI_FVB_INSTANCE)
> - sizeof (EFI_FIRMWARE_VOLUME_HEADER)));-+ FvbInstance =
> (EFI_FVB_INSTANCE *)((UINTN)((UINT8 *)FvbInstance) ++
> FvHeader->HeaderLength ++ (sizeof (EFI_FVB_INSTANCE) -
> sizeof (EFI_FIRMWARE_VOLUME_HEADER))); } } }diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.in
> f
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.in
> f
> index 0cfa3f909b..0485b73679 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.in
> f
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSe
> +++ rviceSmm.inf
> @@ -45,6 +45,8 @@
> [Pcd] gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ##
> CONSUMES gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ##
> CONSUMES+
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ##
> SOMETIMES_CONSUMES+
> gIntelSiliconPkgTokenSpaceGuid.PcdFlashVariableStoreType ##
> SOMETIMES_CONSUMES [Sources] FvbInfo.c@@ -61,6 +63,8 @@
> [Guids] gEfiFirmwareFileSystem2Guid ## CONSUMES
> gEfiSystemNvDataFvGuid ## CONSUMES+ gEfiVariableGuid
> ## SOMETIMES_CONSUMES+ gEfiAuthenticatedVariableGuid ##
> SOMETIMES_CONSUMES [Depex] TRUEdiff --git
> a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> index 485cb3e80a..63dae756ad 100644
> --- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> +++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> @@ -186,3 +186,11 @@
> # @Prompt VTd abort DMA mode support.
> gIntelSiliconPkgTokenSpaceGuid.PcdVTdSupportAbortDmaMode|FALSE|BOOLE
> AN|0x0000000C + ## Define Flash Variable Store type.<BR><BR>+ # When
> Flash Variable Store corruption happened, the SpiFvbService will recreate
> Variable Store+ # with valid header information provided by this PCD
> value.<BR>+ # 0: Variable Store is gEfiVariableGuid type.<BR>+ # 1: Variable
> Store is gEfiAuthenticatedVariableGuid type.<BR>+ # Other value: reserved for
> future use.<BR>+ # @Prompt Flash Variable Store type.+
> gIntelSiliconPkgTokenSpaceGuid.PcdFlashVariableStoreType|0x00|UINT8|0x000
> 0000E--
> 2.35.0.windows.1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-02-06 18:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-03 21:55 [edk2-platforms: PATCH] IntelSiliconPkg/SpiFvbServiceSmm: Rewrite VariableStore header Chiu, Chasel
2023-02-04 16:02 ` Ashraf Ali S
2023-02-06 18:52 ` Chiu, Chasel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox