Reviewed-by: S, Ashraf Ali <ashraf.al...@intel.com>

-----Original Message-----
From: Chiu, Chasel <chasel.c...@intel.com> 
Sent: Tuesday, February 7, 2023 12:20 AM
To: devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.c...@intel.com>; S, Ashraf Ali 
<ashraf.al...@intel.com>; Oram, Isaac W <isaac.w.o...@intel.com>; Chaganty, 
Rangasai V <rangasai.v.chaga...@intel.com>; Ni, Ray <ray...@intel.com>; 
Kubacki, Michael <michael.kuba...@microsoft.com>
Subject: [edk2-platforms: PATCH v2] 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.al...@intel.com>
Cc: Isaac Oram <isaac.w.o...@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaga...@intel.com>
Cc: Ray Ni <ray...@intel.com>
Cc: Michael Kubacki <michael.kuba...@microsoft.com>
Signed-off-by: Chasel Chiu <chasel.c...@intel.com>
---
 Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c    
| 174 
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------
 Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf 
|   4 ++++
 Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec                              
|   8 ++++++++
 3 files changed, 134 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..6af2dfac10 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,116 @@ 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);++ 
       BytesWritten         = FvHeader->HeaderLength;+        
ExpectedBytesWritten = BytesWritten;+        if (Idx != 0) {+          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, "Type: gEfiVariableGuid\n"));+                CopyGuid 
(&VariableStoreHeader->Signature, &gEfiVariableGuid);+                break;+   
           case 1:+                DEBUG ((DEBUG_ERROR, "Type: 
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);+          } else {+       
     Status = EFI_OUT_OF_RESOURCES;+          }+        }+         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 +294,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 +307,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 +338,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 +368,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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99728): https://edk2.groups.io/g/devel/message/99728
Mute This Topic: https://groups.io/mt/96790455/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to