Bug fix patch has been merged:
https://github.com/tianocore/edk2-platforms/commit/e95c7988994c73918ffa282e2d2f5af11f8addc4
Thanks,
Chasel
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chiu, Chasel
> Sent: Wednesday, February 8, 2023 2:17 PM
> 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-devel] [edk2-platforms: PATCH v4]
> 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.
> The Corrupted variable content should be taken care by FaultTolerantWrite
> driver later.
>
> 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
> |
> 69
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> ---
>
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
> | 3 +++
> Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> | 8 ++++++++
> 3 files changed, 75 insertions(+), 5 deletions(-)
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c
> index 6b4bcdcfe3..052be97872 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@@ -113,7
> +114,12 @@ FvbInitialize (
> UINT32 MaxLbaSize; UINT32
> BytesWritten;
> UINTN BytesErased;+ EFI_PHYSICAL_ADDRESS
> NvStorageBaseAddress; UINT64
> NvStorageFvSize;+ UINT32
> ExpectedBytesWritten;+ VARIABLE_STORE_HEADER
> *VariableStoreHeader;+ UINT8
> VariableStoreType;+ UINT8
> *NvStoreBuffer; Status = GetVariableFlashNvStorageInfo (&BaseAddress,
> &NvStorageFvSize); if (EFI_ERROR (Status)) {@@ -124,12 +130,14 @@
> FvbInitialize (
> // Stay within the current UINT32 size assumptions in the variable stack.
> Status = SafeUint64ToUint32 (BaseAddress,
> &mPlatformFvBaseAddress[0].FvBase);+ NvStorageBaseAddress =
> mPlatformFvBaseAddress[0].FvBase; if (EFI_ERROR (Status))
> { ASSERT_EFI_ERROR (Status); DEBUG ((DEBUG_ERROR, "[%a] - 64-bit
> variable storage base address not supported.\n", __FUNCTION__)); return;
> }
> Status = SafeUint64ToUint32 (NvStorageFvSize,
> &mPlatformFvBaseAddress[0].FvSize);+ NvStorageFvSize =
> mPlatformFvBaseAddress[0].FvSize; if (EFI_ERROR (Status))
> { ASSERT_EFI_ERROR (Status); DEBUG ((DEBUG_ERROR, "[%a] - 64-bit
> variable storage size not supported.\n", __FUNCTION__));@@ -186,8 +194,59
> @@ FvbInitialize (
> } continue; }- BytesWritten =
> FvHeader->HeaderLength;-
> Status = SpiFlashWrite ((UINTN)BaseAddress, &BytesWritten,
> (UINT8*)FvHeader);++ BytesWritten = FvHeader->HeaderLength;+
> ExpectedBytesWritten = BytesWritten;+ if (BaseAddress !=
> NvStorageBaseAddress) {+ Status = SpiFlashWrite ((UINTN)BaseAddress,
> &BytesWritten, (UINT8 *)FvHeader);+ } else {+ //+ //
> This is
> Variable Store, rewrite both EFI_FIRMWARE_VOLUME_HEADER and
> VARIABLE_STORE_HEADER.+ // The corrupted Variable content should be
> taken care by FaultTolerantWrite driver later.+ //+
> 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 = (UINT32) (NvStorageFvSize - 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) {@@ -195,9 +254,9 @@ FvbInitialize (
> } 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); }diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.in
> f
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.in
> f
> index 0cfa3f909b..73049eceb2 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.in
> f
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSe
> +++ rviceSmm.inf
> @@ -45,6 +45,7 @@
> [Pcd] gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ##
> CONSUMES gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ##
> CONSUMES+ gIntelSiliconPkgTokenSpaceGuid.PcdFlashVariableStoreType ##
> SOMETIMES_CONSUMES [Sources] FvbInfo.c@@ -61,6 +62,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
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#99818): https://edk2.groups.io/g/devel/message/99818
> Mute This Topic: https://groups.io/mt/96841486/1777047
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [chasel.c...@intel.com] -=-
> =-=-=-=-=
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99839): https://edk2.groups.io/g/devel/message/99839
Mute This Topic: https://groups.io/mt/96841486/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
