* Re: [edk2-devel] [edk2-platforms][PATCH v2 1/1] IntelSiliconPkg/SpiFvbService: Add support for VariableFlashInfoLib
[not found] <16FDBB890A0E8B8C.7323@groups.io>
@ 2022-07-12 23:04 ` Michael Kubacki
2022-07-13 15:51 ` Oram, Isaac W
2022-07-13 16:49 ` Chaganty, Rangasai V
0 siblings, 2 replies; 4+ messages in thread
From: Michael Kubacki @ 2022-07-12 23:04 UTC (permalink / raw)
To: devel; +Cc: Rangasai V Chaganty, Ray Ni, Isaac Oram, Nate DeSimone
Reminder about this patch.
Thanks,
Michael
On 7/1/2022 10:39 AM, Michael Kubacki wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3478
>
> Adds support for getting the variable flash information from
> VariableFlashInfoLib. This library abstracts the source of flash
> information so platforms could expect the information to come
> from a different source in the library implementation than the
> PCDs previously used as the information source in this module.
>
> In particular, the library allows Standalone MM platforms to
> dynamically pass the information behind the library API.
>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Isaac Oram <isaac.w.oram@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>
> Notes:
> V2 changes:
>
> 1. Updated the buffer that is returned from
> GetFvbInfo() to be allocated from pool.
> 2. Updated calls to that function in FvbInitialize()
> to free the new buffer after usage.
> 3. Updated the multiplication operation to determine
> the FV length to use safe multiplication in
> GenerateNvStorageFvbMediaInfo().
>
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c | 128 +++++++++++++-------
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c | 93 ++++++++++++--
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c | 49 ++++++--
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.h | 18 ++-
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf | 6 +-
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceStandaloneMm.inf | 6 +-
> Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc | 7 ++
> 7 files changed, 243 insertions(+), 64 deletions(-)
>
> diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
> index 7f2678fa9e5a..634a44218c7a 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
> @@ -3,6 +3,7 @@
> These data is intent to decouple FVB driver with FV header.
>
> Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -11,51 +12,92 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
> #define FIRMWARE_BLOCK_SIZE 0x10000
> #define FVB_MEDIA_BLOCK_SIZE FIRMWARE_BLOCK_SIZE
> -
> -#define NV_STORAGE_BASE_ADDRESS FixedPcdGet32(PcdFlashNvStorageVariableBase)
> -#define SYSTEM_NV_BLOCK_NUM ((FixedPcdGet32(PcdFlashNvStorageVariableSize)+ FixedPcdGet32(PcdFlashNvStorageFtwWorkingSize) + FixedPcdGet32(PcdFlashNvStorageFtwSpareSize))/ FVB_MEDIA_BLOCK_SIZE)
> -
> typedef struct {
> EFI_PHYSICAL_ADDRESS BaseAddress;
> EFI_FIRMWARE_VOLUME_HEADER FvbInfo;
> EFI_FV_BLOCK_MAP_ENTRY End[1];
> } EFI_FVB2_MEDIA_INFO;
>
> -//
> -// This data structure contains a template of all correct FV headers, which is used to restore
> -// Fv header if it's corrupted.
> -//
> -EFI_FVB2_MEDIA_INFO mPlatformFvbMediaInfo[] = {
> - //
> - // Systen NvStorage FVB
> - //
> - {
> - NV_STORAGE_BASE_ADDRESS,
> - {
> - {0,}, //ZeroVector[16]
> - EFI_SYSTEM_NV_DATA_FV_GUID,
> - FVB_MEDIA_BLOCK_SIZE * SYSTEM_NV_BLOCK_NUM,
> - EFI_FVH_SIGNATURE,
> - 0x0004feff, // check MdePkg/Include/Pi/PiFirmwareVolume.h for details on EFI_FVB_ATTRIBUTES_2
> - sizeof (EFI_FIRMWARE_VOLUME_HEADER) + sizeof (EFI_FV_BLOCK_MAP_ENTRY),
> - 0, //CheckSum which will be calucated dynamically.
> - 0, //ExtHeaderOffset
> - {0,}, //Reserved[1]
> - 2, //Revision
> - {
> - {
> - SYSTEM_NV_BLOCK_NUM,
> - FVB_MEDIA_BLOCK_SIZE,
> - }
> - }
> - },
> - {
> - {
> - 0,
> - 0
> - }
> - }
> +/**
> + Returns FVB media information for NV variable storage.
> +
> + @return FvbMediaInfo A pointer to an instance of FVB media info produced by this function.
> + The buffer is allocated internally to this function and it is the caller's
> + responsibility to free the memory
> +
> +**/
> +typedef
> +EFI_STATUS
> +(*FVB_MEDIA_INFO_GENERATOR)(
> + OUT EFI_FVB2_MEDIA_INFO *FvbMediaInfo
> + );
> +
> +/**
> + Returns FVB media information for NV variable storage.
> +
> + @param[out] FvbMediaInfo A pointer to an instance of FVB media info produced by this function.
> +
> + @retval EFI_SUCCESS A structure was successfully written to the FvbMediaInfo buffer.
> + @retval EFI_INVALID_PARAMETER The FvbMediaInfo parameter is NULL.
> + @retval EFI_UNSUPPORTED An error occurred retrieving variable FV information.
> + @retval EFI_BAD_BUFFER_SIZE An overflow or underflow of the FV buffer occurred with the information found.
> +
> +**/
> +EFI_STATUS
> +GenerateNvStorageFvbMediaInfo (
> + OUT EFI_FVB2_MEDIA_INFO *FvbMediaInfo
> + )
> +{
> + EFI_STATUS Status;
> + UINT32 NvBlockNum;
> + UINT32 TotalNvVariableStorageSize;
> + EFI_PHYSICAL_ADDRESS NvStorageBaseAddress;
> + EFI_FIRMWARE_VOLUME_HEADER FvbInfo = {
> + {0,}, //ZeroVector[16]
> + EFI_SYSTEM_NV_DATA_FV_GUID, //FileSystemGuid
> + 0, //FvLength
> + EFI_FVH_SIGNATURE, //Signature
> + 0x0004feff, //Attributes
> + sizeof (EFI_FIRMWARE_VOLUME_HEADER) + //HeaderLength
> + sizeof (EFI_FV_BLOCK_MAP_ENTRY),
> + 0, //Checksum
> + 0, //ExtHeaderOffset
> + {0,}, //Reserved[1]
> + 2, //Revision
> + { //BlockMap[1]
> + {0,0}
> + }
> + };
> +
> + if (FvbMediaInfo == NULL) {
> + return EFI_INVALID_PARAMETER;
> }
> +
> + ZeroMem (FvbMediaInfo, sizeof (*FvbMediaInfo));
> +
> + GetVariableFvInfo (&NvStorageBaseAddress, &TotalNvVariableStorageSize);
> + if ((NvStorageBaseAddress == 0) || (TotalNvVariableStorageSize == 0)) {
> + return EFI_UNSUPPORTED;
> + }
> +
> + NvBlockNum = TotalNvVariableStorageSize / FVB_MEDIA_BLOCK_SIZE;
> +
> + Status = SafeUint64Mult ((UINT64)NvBlockNum, FVB_MEDIA_BLOCK_SIZE, &FvbInfo.FvLength);
> + if (EFI_ERROR (Status)) {
> + return EFI_BAD_BUFFER_SIZE;
> + }
> +
> + FvbInfo.BlockMap[0].NumBlocks = NvBlockNum;
> + FvbInfo.BlockMap[0].Length = FVB_MEDIA_BLOCK_SIZE;
> +
> + FvbMediaInfo->BaseAddress = NvStorageBaseAddress;
> + CopyMem (&FvbMediaInfo->FvbInfo, &FvbInfo, sizeof (FvbInfo));
> +
> + return EFI_SUCCESS;
> +}
> +
> +FVB_MEDIA_INFO_GENERATOR mFvbMediaInfoGenerators[] = {
> + GenerateNvStorageFvbMediaInfo
> };
>
> EFI_STATUS
> @@ -64,12 +106,16 @@ GetFvbInfo (
> OUT EFI_FIRMWARE_VOLUME_HEADER **FvbInfo
> )
> {
> + EFI_STATUS Status;
> + EFI_FVB2_MEDIA_INFO FvbMediaInfo;
> UINTN Index;
> EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
>
> - for (Index = 0; Index < sizeof (mPlatformFvbMediaInfo) / sizeof (EFI_FVB2_MEDIA_INFO); Index++) {
> - if (mPlatformFvbMediaInfo[Index].BaseAddress == FvBaseAddress) {
> - FvHeader = &mPlatformFvbMediaInfo[Index].FvbInfo;
> + for (Index = 0; Index < ARRAY_SIZE (mFvbMediaInfoGenerators); Index++) {
> + Status = mFvbMediaInfoGenerators[Index](&FvbMediaInfo);
> + ASSERT_EFI_ERROR (Status);
> + if (!EFI_ERROR (Status) && (FvbMediaInfo.BaseAddress == FvBaseAddress)) {
> + FvHeader = AllocateCopyPool (sizeof (EFI_FIRMWARE_VOLUME_HEADER), &FvbMediaInfo.FvbInfo);
>
> //
> // Update the checksum value of FV header.
> diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
> index dab818e98087..942abf95a64f 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
> @@ -3,6 +3,7 @@
> which are compliant with the Intel(R) Serial Flash Interface Compatibility Specification.
>
> Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -25,12 +26,6 @@ FV_INFO mPlatformFvBaseAddress[] = {
> {0, 0}
> };
>
> -FV_INFO mPlatformDefaultBaseAddress[] = {
> - {0, 0}, // {FixedPcdGet32(PcdFlashNvStorageVariableBase), FixedPcdGet32(PcdFlashNvStorageVariableSize)},
> - {0, 0}, // {FixedPcdGet32(PcdFlashMicrocodeFvBase), FixedPcdGet32(PcdFlashMicrocodeFvSize)},
> - {0, 0}
> -};
> -
> FV_MEMMAP_DEVICE_PATH mFvMemmapDevicePathTemplate = {
> {
> {
> @@ -530,6 +525,85 @@ FvbSetVolumeAttributes (
> return EFI_SUCCESS;
> }
>
> +/**
> + Get the total size of the firmware volume on flash used for variable store operations.
> +
> + @param[out] BaseAddress Base address of the variable store firmware volume.
> + @param[out] Length Length in bytes of the firmware volume used for variable store operations.
> +
> +**/
> +VOID
> +GetVariableFvInfo (
> + OUT EFI_PHYSICAL_ADDRESS *BaseAddress,
> + OUT UINT32 *Length
> + )
> +{
> + EFI_STATUS Status;
> + EFI_PHYSICAL_ADDRESS NvBaseAddress;
> + EFI_PHYSICAL_ADDRESS NvVariableBaseAddress;
> + UINT64 Length64;
> + UINT32 NvStoreLength;
> + UINT32 FtwSpareLength;
> + UINT32 FtwWorkingLength;
> + UINT32 TotalLength;
> +
> + TotalLength = 0;
> + Status = EFI_SUCCESS;
> +
> + if ((BaseAddress == NULL) || (Length == NULL)) {
> + ASSERT ((BaseAddress != NULL) && (Length != NULL));
> + return;
> + }
> + *BaseAddress = 0;
> + *Length = 0;
> +
> + Status = GetVariableFlashNvStorageInfo (&NvBaseAddress, &Length64);
> + if (!EFI_ERROR (Status)) {
> + NvVariableBaseAddress = NvBaseAddress;
> + // Stay within the current UINT32 size assumptions in the variable stack.
> + Status = SafeUint64ToUint32 (Length64, &NvStoreLength);
> + }
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + Status = GetVariableFlashFtwSpareInfo (&NvBaseAddress, &Length64);
> + if (!EFI_ERROR (Status)) {
> + // Stay within the current UINT32 size assumptions in the variable stack.
> + Status = SafeUint64ToUint32 (Length64, &FtwSpareLength);
> + }
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + Status = GetVariableFlashFtwWorkingInfo (&NvBaseAddress, &Length64);
> + if (!EFI_ERROR (Status)) {
> + // Stay within the current UINT32 size assumptions in the variable stack.
> + Status = SafeUint64ToUint32 (Length64, &FtwWorkingLength);
> + }
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + Status = SafeUint32Add (NvStoreLength, FtwSpareLength, &TotalLength);
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + Status = SafeUint32Add (TotalLength, FtwWorkingLength, &TotalLength);
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + *BaseAddress = NvVariableBaseAddress;
> + *Length = TotalLength;
> +}
> +
> /**
> Check the integrity of firmware volume header
>
> @@ -545,7 +619,12 @@ IsFvHeaderValid (
> IN CONST EFI_FIRMWARE_VOLUME_HEADER *FvHeader
> )
> {
> - if (FvBase == PcdGet32(PcdFlashNvStorageVariableBase)) {
> + EFI_PHYSICAL_ADDRESS NvStorageFvBaseAddress;
> + UINT32 NvStorageSize;
> +
> + GetVariableFvInfo (&NvStorageFvBaseAddress, &NvStorageSize);
> +
> + if (FvBase == NvStorageFvBaseAddress) {
> if (CompareMem (&FvHeader->FileSystemGuid, &gEfiSystemNvDataFvGuid, sizeof(EFI_GUID)) != 0 ) {
> return FALSE;
> }
> diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c
> index 42a0828c6fae..6b4bcdcfe3c4 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c
> @@ -113,15 +113,31 @@ FvbInitialize (
> UINT32 MaxLbaSize;
> UINT32 BytesWritten;
> UINTN BytesErased;
> + UINT64 NvStorageFvSize;
> +
> + Status = GetVariableFlashNvStorageInfo (&BaseAddress, &NvStorageFvSize);
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + DEBUG ((DEBUG_ERROR, "[%a] - An error ocurred getting variable info - %r.\n", __FUNCTION__, Status));
> + return;
> + }
> +
> + // Stay within the current UINT32 size assumptions in the variable stack.
> + Status = SafeUint64ToUint32 (BaseAddress, &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);
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + DEBUG ((DEBUG_ERROR, "[%a] - 64-bit variable storage size not supported.\n", __FUNCTION__));
> + return;
> + }
>
> - mPlatformFvBaseAddress[0].FvBase = PcdGet32(PcdFlashNvStorageVariableBase);
> - mPlatformFvBaseAddress[0].FvSize = PcdGet32(PcdFlashNvStorageVariableSize);
> mPlatformFvBaseAddress[1].FvBase = PcdGet32(PcdFlashMicrocodeFvBase);
> mPlatformFvBaseAddress[1].FvSize = PcdGet32(PcdFlashMicrocodeFvSize);
> - mPlatformDefaultBaseAddress[0].FvBase = PcdGet32(PcdFlashNvStorageVariableBase);
> - mPlatformDefaultBaseAddress[0].FvSize = PcdGet32(PcdFlashNvStorageVariableSize);
> - mPlatformDefaultBaseAddress[1].FvBase = PcdGet32(PcdFlashMicrocodeFvBase);
> - mPlatformDefaultBaseAddress[1].FvSize = PcdGet32(PcdFlashMicrocodeFvSize);
>
> //
> // We will only continue with FVB installation if the
> @@ -143,7 +159,8 @@ FvbInitialize (
> BytesWritten = 0;
> BytesErased = 0;
> DEBUG ((DEBUG_ERROR, "ERROR - The FV in 0x%x is invalid!\n", FvHeader));
> - Status = GetFvbInfo (BaseAddress, &FvHeader);
> + FvHeader = NULL;
> + Status = GetFvbInfo (BaseAddress, &FvHeader);
> if (EFI_ERROR (Status)) {
> DEBUG ((DEBUG_WARN, "ERROR - Can't recovery FV header at 0x%x. GetFvbInfo Status %r\n", BaseAddress, Status));
> continue;
> @@ -156,27 +173,42 @@ FvbInitialize (
> 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 (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 (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));
> @@ -184,6 +216,9 @@ FvbInitialize (
> // Clear cache for this range.
> //
> WriteBackInvalidateDataCacheRange ( (VOID *) (UINTN) BaseAddress, FvHeader->BlockMap->Length);
> + if (FvHeader != NULL) {
> + FreePool (FvHeader);
> + }
> }
> }
>
> diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.h b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.h
> index e9d69e985814..7b0908b03fdc 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.h
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.h
> @@ -2,6 +2,7 @@
> Common source definitions used in serial flash drivers
>
> Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -12,6 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Guid/EventGroup.h>
> #include <Guid/FirmwareFileSystem2.h>
> #include <Guid/SystemNvDataGuid.h>
> +#include <Pi/PiFirmwareVolume.h>
> #include <Protocol/DevicePath.h>
> #include <Protocol/FirmwareVolumeBlock.h>
>
> @@ -24,6 +26,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Library/PcdLib.h>
> #include <Library/DevicePathLib.h>
> #include <Library/HobLib.h>
> +#include <Library/VariableFlashInfoLib.h>
> +#include <Library/SafeIntLib.h>
>
> #include <Library/SpiFlashCommonLib.h>
>
> @@ -148,11 +152,23 @@ GetFvbInfo (
> OUT EFI_FIRMWARE_VOLUME_HEADER **FvbInfo
> );
>
> +/**
> + Get the total size of the firmware volume on flash used for variable store operations.
> +
> + @param[out] BaseAddress Base address of the variable store firmware volume.
> + @param[out] Length Length in bytes of the firmware volume used for variable store operations.
> +
> +**/
> +VOID
> +GetVariableFvInfo (
> + OUT EFI_PHYSICAL_ADDRESS *BaseAddress,
> + OUT UINT32 *Length
> + );
> +
> extern FVB_GLOBAL mFvbModuleGlobal;
> extern FV_MEMMAP_DEVICE_PATH mFvMemmapDevicePathTemplate;
> extern FV_PIWG_DEVICE_PATH mFvPIWGDevicePathTemplate;
> extern EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL mFvbProtocolTemplate;
> extern FV_INFO mPlatformFvBaseAddress[];
> -extern FV_INFO mPlatformDefaultBaseAddress[];
>
> #endif
> diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
> index bf1306f00201..0cfa3f909bdf 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf
> @@ -32,8 +32,10 @@ [LibraryClasses]
> BaseLib
> UefiBootServicesTableLib
> UefiDriverEntryPoint
> + SafeIntLib
> SpiFlashCommonLib
> MmServicesTableLib
> + VariableFlashInfoLib
>
> [Packages]
> MdePkg/MdePkg.dec
> @@ -41,10 +43,6 @@ [Packages]
> IntelSiliconPkg/IntelSiliconPkg.dec
>
> [Pcd]
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize ## CONSUMES
> gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CONSUMES
> gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CONSUMES
>
> diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceStandaloneMm.inf b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceStandaloneMm.inf
> index b66233968247..152cf0036fdb 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceStandaloneMm.inf
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceStandaloneMm.inf
> @@ -31,8 +31,10 @@ [LibraryClasses]
> MemoryAllocationLib
> PcdLib
> MmServicesTableLib
> + SafeIntLib
> SpiFlashCommonLib
> StandaloneMmDriverEntryPoint
> + VariableFlashInfoLib
>
> [Packages]
> MdePkg/MdePkg.dec
> @@ -40,10 +42,6 @@ [Packages]
> IntelSiliconPkg/IntelSiliconPkg.dec
>
> [Pcd]
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize ## CONSUMES
> gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CONSUMES
> gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CONSUMES
>
> diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
> index 1e826a080f28..170eb480ada1 100644
> --- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
> +++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
> @@ -40,9 +40,11 @@ [LibraryClasses]
> PeiGetVtdPmrAlignmentLib|IntelSiliconPkg/Library/PeiGetVtdPmrAlignmentLib/PeiGetVtdPmrAlignmentLib.inf
> TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
> MicrocodeLib|UefiCpuPkg/Library/MicrocodeLib/MicrocodeLib.inf
> + SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> SpiFlashCommonLib|IntelSiliconPkg/Library/SpiFlashCommonLibNull/SpiFlashCommonLibNull.inf
> UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
> UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
> + VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/BaseVariableFlashInfoLib.inf
>
> [LibraryClasses.common.PEIM]
> PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
> @@ -63,11 +65,16 @@ [LibraryClasses.common.DXE_DRIVER]
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>
> [LibraryClasses.common.DXE_SMM_DRIVER]
> + DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
> + HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.inf
> MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf
> SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf
> + UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
> + UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
>
> [LibraryClasses.common.MM_STANDALONE]
> + HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
> MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
> MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
> StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH v2 1/1] IntelSiliconPkg/SpiFvbService: Add support for VariableFlashInfoLib
2022-07-12 23:04 ` [edk2-devel] [edk2-platforms][PATCH v2 1/1] IntelSiliconPkg/SpiFvbService: Add support for VariableFlashInfoLib Michael Kubacki
@ 2022-07-13 15:51 ` Oram, Isaac W
2022-07-13 16:49 ` Chaganty, Rangasai V
1 sibling, 0 replies; 4+ messages in thread
From: Oram, Isaac W @ 2022-07-13 15:51 UTC (permalink / raw)
To: devel@edk2.groups.io, mikuback@linux.microsoft.com
Cc: Chaganty, Rangasai V, Ni, Ray, Desimone, Nathaniel L
Reviewed-by: Isaac Oram <isaac.w.oram@intel.com>
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
Sent: Tuesday, July 12, 2022 4:04 PM
To: devel@edk2.groups.io
Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Ni, Ray <ray.ni@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH v2 1/1] IntelSiliconPkg/SpiFvbService: Add support for VariableFlashInfoLib
Reminder about this patch.
Thanks,
Michael
On 7/1/2022 10:39 AM, Michael Kubacki wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3478
>
> Adds support for getting the variable flash information from
> VariableFlashInfoLib. This library abstracts the source of flash
> information so platforms could expect the information to come from a
> different source in the library implementation than the PCDs
> previously used as the information source in this module.
>
> In particular, the library allows Standalone MM platforms to
> dynamically pass the information behind the library API.
>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Isaac Oram <isaac.w.oram@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>
> Notes:
> V2 changes:
>
> 1. Updated the buffer that is returned from
> GetFvbInfo() to be allocated from pool.
> 2. Updated calls to that function in FvbInitialize()
> to free the new buffer after usage.
> 3. Updated the multiplication operation to determine
> the FV length to use safe multiplication in
> GenerateNvStorageFvbMediaInfo().
>
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c | 128 +++++++++++++-------
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c | 93 ++++++++++++--
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c | 49 ++++++--
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.h | 18 ++-
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf | 6 +-
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceStandaloneMm.inf | 6 +-
> Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc | 7 ++
> 7 files changed, 243 insertions(+), 64 deletions(-)
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
> index 7f2678fa9e5a..634a44218c7a 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInf
> +++ o.c
> @@ -3,6 +3,7 @@
> These data is intent to decouple FVB driver with FV header.
>
> Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -11,51 +12,92 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
> #define FIRMWARE_BLOCK_SIZE 0x10000
> #define FVB_MEDIA_BLOCK_SIZE FIRMWARE_BLOCK_SIZE
> -
> -#define NV_STORAGE_BASE_ADDRESS FixedPcdGet32(PcdFlashNvStorageVariableBase)
> -#define SYSTEM_NV_BLOCK_NUM ((FixedPcdGet32(PcdFlashNvStorageVariableSize)+ FixedPcdGet32(PcdFlashNvStorageFtwWorkingSize) + FixedPcdGet32(PcdFlashNvStorageFtwSpareSize))/ FVB_MEDIA_BLOCK_SIZE)
> -
> typedef struct {
> EFI_PHYSICAL_ADDRESS BaseAddress;
> EFI_FIRMWARE_VOLUME_HEADER FvbInfo;
> EFI_FV_BLOCK_MAP_ENTRY End[1];
> } EFI_FVB2_MEDIA_INFO;
>
> -//
> -// This data structure contains a template of all correct FV headers,
> which is used to restore -// Fv header if it's corrupted.
> -//
> -EFI_FVB2_MEDIA_INFO mPlatformFvbMediaInfo[] = {
> - //
> - // Systen NvStorage FVB
> - //
> - {
> - NV_STORAGE_BASE_ADDRESS,
> - {
> - {0,}, //ZeroVector[16]
> - EFI_SYSTEM_NV_DATA_FV_GUID,
> - FVB_MEDIA_BLOCK_SIZE * SYSTEM_NV_BLOCK_NUM,
> - EFI_FVH_SIGNATURE,
> - 0x0004feff, // check MdePkg/Include/Pi/PiFirmwareVolume.h for details on EFI_FVB_ATTRIBUTES_2
> - sizeof (EFI_FIRMWARE_VOLUME_HEADER) + sizeof (EFI_FV_BLOCK_MAP_ENTRY),
> - 0, //CheckSum which will be calucated dynamically.
> - 0, //ExtHeaderOffset
> - {0,}, //Reserved[1]
> - 2, //Revision
> - {
> - {
> - SYSTEM_NV_BLOCK_NUM,
> - FVB_MEDIA_BLOCK_SIZE,
> - }
> - }
> - },
> - {
> - {
> - 0,
> - 0
> - }
> - }
> +/**
> + Returns FVB media information for NV variable storage.
> +
> + @return FvbMediaInfo A pointer to an instance of FVB media info produced by this function.
> + The buffer is allocated internally to this function and it is the caller's
> + responsibility to free the
> + memory
> +
> +**/
> +typedef
> +EFI_STATUS
> +(*FVB_MEDIA_INFO_GENERATOR)(
> + OUT EFI_FVB2_MEDIA_INFO *FvbMediaInfo
> + );
> +
> +/**
> + Returns FVB media information for NV variable storage.
> +
> + @param[out] FvbMediaInfo A pointer to an instance of FVB media info produced by this function.
> +
> + @retval EFI_SUCCESS A structure was successfully written to the FvbMediaInfo buffer.
> + @retval EFI_INVALID_PARAMETER The FvbMediaInfo parameter is NULL.
> + @retval EFI_UNSUPPORTED An error occurred retrieving variable FV information.
> + @retval EFI_BAD_BUFFER_SIZE An overflow or underflow of the FV buffer occurred with the information found.
> +
> +**/
> +EFI_STATUS
> +GenerateNvStorageFvbMediaInfo (
> + OUT EFI_FVB2_MEDIA_INFO *FvbMediaInfo
> + )
> +{
> + EFI_STATUS Status;
> + UINT32 NvBlockNum;
> + UINT32 TotalNvVariableStorageSize;
> + EFI_PHYSICAL_ADDRESS NvStorageBaseAddress;
> + EFI_FIRMWARE_VOLUME_HEADER FvbInfo = {
> + {0,}, //ZeroVector[16]
> + EFI_SYSTEM_NV_DATA_FV_GUID, //FileSystemGuid
> + 0, //FvLength
> + EFI_FVH_SIGNATURE, //Signature
> + 0x0004feff, //Attributes
> + sizeof (EFI_FIRMWARE_VOLUME_HEADER) + //HeaderLength
> + sizeof (EFI_FV_BLOCK_MAP_ENTRY),
> + 0, //Checksum
> + 0, //ExtHeaderOffset
> + {0,}, //Reserved[1]
> + 2, //Revision
> + { //BlockMap[1]
> + {0,0}
> + }
> + };
> +
> + if (FvbMediaInfo == NULL) {
> + return EFI_INVALID_PARAMETER;
> }
> +
> + ZeroMem (FvbMediaInfo, sizeof (*FvbMediaInfo));
> +
> + GetVariableFvInfo (&NvStorageBaseAddress,
> + &TotalNvVariableStorageSize); if ((NvStorageBaseAddress == 0) || (TotalNvVariableStorageSize == 0)) {
> + return EFI_UNSUPPORTED;
> + }
> +
> + NvBlockNum = TotalNvVariableStorageSize / FVB_MEDIA_BLOCK_SIZE;
> +
> + Status = SafeUint64Mult ((UINT64)NvBlockNum, FVB_MEDIA_BLOCK_SIZE,
> + &FvbInfo.FvLength); if (EFI_ERROR (Status)) {
> + return EFI_BAD_BUFFER_SIZE;
> + }
> +
> + FvbInfo.BlockMap[0].NumBlocks = NvBlockNum;
> + FvbInfo.BlockMap[0].Length = FVB_MEDIA_BLOCK_SIZE;
> +
> + FvbMediaInfo->BaseAddress = NvStorageBaseAddress; CopyMem
> + (&FvbMediaInfo->FvbInfo, &FvbInfo, sizeof (FvbInfo));
> +
> + return EFI_SUCCESS;
> +}
> +
> +FVB_MEDIA_INFO_GENERATOR mFvbMediaInfoGenerators[] = {
> + GenerateNvStorageFvbMediaInfo
> };
>
> EFI_STATUS
> @@ -64,12 +106,16 @@ GetFvbInfo (
> OUT EFI_FIRMWARE_VOLUME_HEADER **FvbInfo
> )
> {
> + EFI_STATUS Status;
> + EFI_FVB2_MEDIA_INFO FvbMediaInfo;
> UINTN Index;
> EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
>
> - for (Index = 0; Index < sizeof (mPlatformFvbMediaInfo) / sizeof (EFI_FVB2_MEDIA_INFO); Index++) {
> - if (mPlatformFvbMediaInfo[Index].BaseAddress == FvBaseAddress) {
> - FvHeader = &mPlatformFvbMediaInfo[Index].FvbInfo;
> + for (Index = 0; Index < ARRAY_SIZE (mFvbMediaInfoGenerators); Index++) {
> + Status = mFvbMediaInfoGenerators[Index](&FvbMediaInfo);
> + ASSERT_EFI_ERROR (Status);
> + if (!EFI_ERROR (Status) && (FvbMediaInfo.BaseAddress == FvBaseAddress)) {
> + FvHeader = AllocateCopyPool (sizeof
> + (EFI_FIRMWARE_VOLUME_HEADER), &FvbMediaInfo.FvbInfo);
>
> //
> // Update the checksum value of FV header.
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.c index dab818e98087..942abf95a64f 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.c
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
> +++ ServiceCommon.c
> @@ -3,6 +3,7 @@
> which are compliant with the Intel(R) Serial Flash Interface Compatibility Specification.
>
> Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -25,12 +26,6 @@ FV_INFO mPlatformFvBaseAddress[] = {
> {0, 0}
> };
>
> -FV_INFO mPlatformDefaultBaseAddress[] = {
> - {0, 0}, // {FixedPcdGet32(PcdFlashNvStorageVariableBase),
> FixedPcdGet32(PcdFlashNvStorageVariableSize)},
> - {0, 0}, // {FixedPcdGet32(PcdFlashMicrocodeFvBase),
> FixedPcdGet32(PcdFlashMicrocodeFvSize)},
> - {0, 0}
> -};
> -
> FV_MEMMAP_DEVICE_PATH mFvMemmapDevicePathTemplate = {
> {
> {
> @@ -530,6 +525,85 @@ FvbSetVolumeAttributes (
> return EFI_SUCCESS;
> }
>
> +/**
> + Get the total size of the firmware volume on flash used for variable store operations.
> +
> + @param[out] BaseAddress Base address of the variable store firmware volume.
> + @param[out] Length Length in bytes of the firmware volume used for variable store operations.
> +
> +**/
> +VOID
> +GetVariableFvInfo (
> + OUT EFI_PHYSICAL_ADDRESS *BaseAddress,
> + OUT UINT32 *Length
> + )
> +{
> + EFI_STATUS Status;
> + EFI_PHYSICAL_ADDRESS NvBaseAddress;
> + EFI_PHYSICAL_ADDRESS NvVariableBaseAddress;
> + UINT64 Length64;
> + UINT32 NvStoreLength;
> + UINT32 FtwSpareLength;
> + UINT32 FtwWorkingLength;
> + UINT32 TotalLength;
> +
> + TotalLength = 0;
> + Status = EFI_SUCCESS;
> +
> + if ((BaseAddress == NULL) || (Length == NULL)) {
> + ASSERT ((BaseAddress != NULL) && (Length != NULL));
> + return;
> + }
> + *BaseAddress = 0;
> + *Length = 0;
> +
> + Status = GetVariableFlashNvStorageInfo (&NvBaseAddress, &Length64);
> + if (!EFI_ERROR (Status)) {
> + NvVariableBaseAddress = NvBaseAddress;
> + // Stay within the current UINT32 size assumptions in the variable stack.
> + Status = SafeUint64ToUint32 (Length64, &NvStoreLength); } if
> + (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + Status = GetVariableFlashFtwSpareInfo (&NvBaseAddress, &Length64);
> + if (!EFI_ERROR (Status)) {
> + // Stay within the current UINT32 size assumptions in the variable stack.
> + Status = SafeUint64ToUint32 (Length64, &FtwSpareLength); } if
> + (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + Status = GetVariableFlashFtwWorkingInfo (&NvBaseAddress,
> + &Length64); if (!EFI_ERROR (Status)) {
> + // Stay within the current UINT32 size assumptions in the variable stack.
> + Status = SafeUint64ToUint32 (Length64, &FtwWorkingLength); } if
> + (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + Status = SafeUint32Add (NvStoreLength, FtwSpareLength,
> + &TotalLength); if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + Status = SafeUint32Add (TotalLength, FtwWorkingLength,
> + &TotalLength); if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + *BaseAddress = NvVariableBaseAddress;
> + *Length = TotalLength;
> +}
> +
> /**
> Check the integrity of firmware volume header
>
> @@ -545,7 +619,12 @@ IsFvHeaderValid (
> IN CONST EFI_FIRMWARE_VOLUME_HEADER *FvHeader
> )
> {
> - if (FvBase == PcdGet32(PcdFlashNvStorageVariableBase)) {
> + EFI_PHYSICAL_ADDRESS NvStorageFvBaseAddress;
> + UINT32 NvStorageSize;
> +
> + GetVariableFvInfo (&NvStorageFvBaseAddress, &NvStorageSize);
> +
> + if (FvBase == NvStorageFvBaseAddress) {
> if (CompareMem (&FvHeader->FileSystemGuid, &gEfiSystemNvDataFvGuid, sizeof(EFI_GUID)) != 0 ) {
> return FALSE;
> }
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceMm.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceMm.c index 42a0828c6fae..6b4bcdcfe3c4 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceMm.c
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
> +++ ServiceMm.c
> @@ -113,15 +113,31 @@ FvbInitialize (
> UINT32 MaxLbaSize;
> UINT32 BytesWritten;
> UINTN BytesErased;
> + UINT64 NvStorageFvSize;
> +
> + Status = GetVariableFlashNvStorageInfo (&BaseAddress,
> + &NvStorageFvSize); if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + DEBUG ((DEBUG_ERROR, "[%a] - An error ocurred getting variable info - %r.\n", __FUNCTION__, Status));
> + return;
> + }
> +
> + // Stay within the current UINT32 size assumptions in the variable stack.
> + Status = SafeUint64ToUint32 (BaseAddress,
> + &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);
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + DEBUG ((DEBUG_ERROR, "[%a] - 64-bit variable storage size not supported.\n", __FUNCTION__));
> + return;
> + }
>
> - mPlatformFvBaseAddress[0].FvBase =
> PcdGet32(PcdFlashNvStorageVariableBase);
> - mPlatformFvBaseAddress[0].FvSize = PcdGet32(PcdFlashNvStorageVariableSize);
> mPlatformFvBaseAddress[1].FvBase = PcdGet32(PcdFlashMicrocodeFvBase);
> mPlatformFvBaseAddress[1].FvSize =
> PcdGet32(PcdFlashMicrocodeFvSize);
> - mPlatformDefaultBaseAddress[0].FvBase =
> PcdGet32(PcdFlashNvStorageVariableBase);
> - mPlatformDefaultBaseAddress[0].FvSize =
> PcdGet32(PcdFlashNvStorageVariableSize);
> - mPlatformDefaultBaseAddress[1].FvBase =
> PcdGet32(PcdFlashMicrocodeFvBase);
> - mPlatformDefaultBaseAddress[1].FvSize =
> PcdGet32(PcdFlashMicrocodeFvSize);
>
> //
> // We will only continue with FVB installation if the @@ -143,7
> +159,8 @@ FvbInitialize (
> BytesWritten = 0;
> BytesErased = 0;
> DEBUG ((DEBUG_ERROR, "ERROR - The FV in 0x%x is invalid!\n", FvHeader));
> - Status = GetFvbInfo (BaseAddress, &FvHeader);
> + FvHeader = NULL;
> + Status = GetFvbInfo (BaseAddress, &FvHeader);
> if (EFI_ERROR (Status)) {
> DEBUG ((DEBUG_WARN, "ERROR - Can't recovery FV header at 0x%x. GetFvbInfo Status %r\n", BaseAddress, Status));
> continue;
> @@ -156,27 +173,42 @@ FvbInitialize (
> 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 (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 (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)); @@ -184,6 +216,9 @@ FvbInitialize (
> // Clear cache for this range.
> //
> WriteBackInvalidateDataCacheRange ( (VOID *) (UINTN)
> BaseAddress, FvHeader->BlockMap->Length);
> + if (FvHeader != NULL) {
> + FreePool (FvHeader);
> + }
> }
> }
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.h
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.h index e9d69e985814..7b0908b03fdc 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.h
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
> +++ ServiceCommon.h
> @@ -2,6 +2,7 @@
> Common source definitions used in serial flash drivers
>
> Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -12,6 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Guid/EventGroup.h>
> #include <Guid/FirmwareFileSystem2.h>
> #include <Guid/SystemNvDataGuid.h>
> +#include <Pi/PiFirmwareVolume.h>
> #include <Protocol/DevicePath.h>
> #include <Protocol/FirmwareVolumeBlock.h>
>
> @@ -24,6 +26,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Library/PcdLib.h>
> #include <Library/DevicePathLib.h>
> #include <Library/HobLib.h>
> +#include <Library/VariableFlashInfoLib.h> #include
> +<Library/SafeIntLib.h>
>
> #include <Library/SpiFlashCommonLib.h>
>
> @@ -148,11 +152,23 @@ GetFvbInfo (
> OUT EFI_FIRMWARE_VOLUME_HEADER **FvbInfo
> );
>
> +/**
> + Get the total size of the firmware volume on flash used for variable store operations.
> +
> + @param[out] BaseAddress Base address of the variable store firmware volume.
> + @param[out] Length Length in bytes of the firmware volume used for variable store operations.
> +
> +**/
> +VOID
> +GetVariableFvInfo (
> + OUT EFI_PHYSICAL_ADDRESS *BaseAddress,
> + OUT UINT32 *Length
> + );
> +
> extern FVB_GLOBAL mFvbModuleGlobal;
> extern FV_MEMMAP_DEVICE_PATH mFvMemmapDevicePathTemplate;
> extern FV_PIWG_DEVICE_PATH mFvPIWGDevicePathTemplate;
> extern EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL mFvbProtocolTemplate;
> extern FV_INFO mPlatformFvBaseAddress[];
> -extern FV_INFO mPlatformDefaultBaseAddress[];
>
> #endif
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceSmm.inf
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceSmm.inf index bf1306f00201..0cfa3f909bdf 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceSmm.inf
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
> +++ ServiceSmm.inf
> @@ -32,8 +32,10 @@ [LibraryClasses]
> BaseLib
> UefiBootServicesTableLib
> UefiDriverEntryPoint
> + SafeIntLib
> SpiFlashCommonLib
> MmServicesTableLib
> + VariableFlashInfoLib
>
> [Packages]
> MdePkg/MdePkg.dec
> @@ -41,10 +43,6 @@ [Packages]
> IntelSiliconPkg/IntelSiliconPkg.dec
>
> [Pcd]
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize ## CONSUMES
> gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CONSUMES
> gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CONSUMES
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceStandaloneMm.inf
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceStandaloneMm.inf index b66233968247..152cf0036fdb 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceStandaloneMm.inf
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
> +++ ServiceStandaloneMm.inf
> @@ -31,8 +31,10 @@ [LibraryClasses]
> MemoryAllocationLib
> PcdLib
> MmServicesTableLib
> + SafeIntLib
> SpiFlashCommonLib
> StandaloneMmDriverEntryPoint
> + VariableFlashInfoLib
>
> [Packages]
> MdePkg/MdePkg.dec
> @@ -40,10 +42,6 @@ [Packages]
> IntelSiliconPkg/IntelSiliconPkg.dec
>
> [Pcd]
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize ## CONSUMES
> gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CONSUMES
> gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CONSUMES
>
> diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
> b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
> index 1e826a080f28..170eb480ada1 100644
> --- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
> +++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
> @@ -40,9 +40,11 @@ [LibraryClasses]
> PeiGetVtdPmrAlignmentLib|IntelSiliconPkg/Library/PeiGetVtdPmrAlignmentLib/PeiGetVtdPmrAlignmentLib.inf
> TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
> MicrocodeLib|UefiCpuPkg/Library/MicrocodeLib/MicrocodeLib.inf
> + SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> SpiFlashCommonLib|IntelSiliconPkg/Library/SpiFlashCommonLibNull/SpiFlashCommonLibNull.inf
> UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
>
> UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEnt
> ryPoint.inf
> +
> + VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/B
> + aseVariableFlashInfoLib.inf
>
> [LibraryClasses.common.PEIM]
> PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
> @@ -63,11 +65,16 @@ [LibraryClasses.common.DXE_DRIVER]
>
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryA
> llocationLib.inf
>
> [LibraryClasses.common.DXE_SMM_DRIVER]
> +
> + DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
> + HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.inf
> MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf
>
> SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTabl
> eLib.inf
> + UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
> +
> + UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableL
> + ib/UefiRuntimeServicesTableLib.inf
>
> [LibraryClasses.common.MM_STANDALONE]
> +
> + HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib
> + .inf
> MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
> MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
>
> StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoi
> nt/StandaloneMmDriverEntryPoint.inf
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH v2 1/1] IntelSiliconPkg/SpiFvbService: Add support for VariableFlashInfoLib
2022-07-12 23:04 ` [edk2-devel] [edk2-platforms][PATCH v2 1/1] IntelSiliconPkg/SpiFvbService: Add support for VariableFlashInfoLib Michael Kubacki
2022-07-13 15:51 ` Oram, Isaac W
@ 2022-07-13 16:49 ` Chaganty, Rangasai V
2022-07-13 20:59 ` Oram, Isaac W
1 sibling, 1 reply; 4+ messages in thread
From: Chaganty, Rangasai V @ 2022-07-13 16:49 UTC (permalink / raw)
To: devel@edk2.groups.io, mikuback@linux.microsoft.com
Cc: Ni, Ray, Oram, Isaac W, Desimone, Nathaniel L
Reviewed-by: Sai Chaganty <rangasai.v.chaganty@intel.com>
-----Original Message-----
From: Michael Kubacki <mikuback@linux.microsoft.com>
Sent: Tuesday, July 12, 2022 4:04 PM
To: devel@edk2.groups.io
Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Ni, Ray <ray.ni@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH v2 1/1] IntelSiliconPkg/SpiFvbService: Add support for VariableFlashInfoLib
Reminder about this patch.
Thanks,
Michael
On 7/1/2022 10:39 AM, Michael Kubacki wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3478
>
> Adds support for getting the variable flash information from
> VariableFlashInfoLib. This library abstracts the source of flash
> information so platforms could expect the information to come from a
> different source in the library implementation than the PCDs
> previously used as the information source in this module.
>
> In particular, the library allows Standalone MM platforms to
> dynamically pass the information behind the library API.
>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Isaac Oram <isaac.w.oram@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>
> Notes:
> V2 changes:
>
> 1. Updated the buffer that is returned from
> GetFvbInfo() to be allocated from pool.
> 2. Updated calls to that function in FvbInitialize()
> to free the new buffer after usage.
> 3. Updated the multiplication operation to determine
> the FV length to use safe multiplication in
> GenerateNvStorageFvbMediaInfo().
>
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c | 128 +++++++++++++-------
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c | 93 ++++++++++++--
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c | 49 ++++++--
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.h | 18 ++-
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf | 6 +-
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceStandaloneMm.inf | 6 +-
> Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc | 7 ++
> 7 files changed, 243 insertions(+), 64 deletions(-)
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
> index 7f2678fa9e5a..634a44218c7a 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInf
> +++ o.c
> @@ -3,6 +3,7 @@
> These data is intent to decouple FVB driver with FV header.
>
> Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -11,51 +12,92 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
> #define FIRMWARE_BLOCK_SIZE 0x10000
> #define FVB_MEDIA_BLOCK_SIZE FIRMWARE_BLOCK_SIZE
> -
> -#define NV_STORAGE_BASE_ADDRESS FixedPcdGet32(PcdFlashNvStorageVariableBase)
> -#define SYSTEM_NV_BLOCK_NUM ((FixedPcdGet32(PcdFlashNvStorageVariableSize)+ FixedPcdGet32(PcdFlashNvStorageFtwWorkingSize) + FixedPcdGet32(PcdFlashNvStorageFtwSpareSize))/ FVB_MEDIA_BLOCK_SIZE)
> -
> typedef struct {
> EFI_PHYSICAL_ADDRESS BaseAddress;
> EFI_FIRMWARE_VOLUME_HEADER FvbInfo;
> EFI_FV_BLOCK_MAP_ENTRY End[1];
> } EFI_FVB2_MEDIA_INFO;
>
> -//
> -// This data structure contains a template of all correct FV headers,
> which is used to restore -// Fv header if it's corrupted.
> -//
> -EFI_FVB2_MEDIA_INFO mPlatformFvbMediaInfo[] = {
> - //
> - // Systen NvStorage FVB
> - //
> - {
> - NV_STORAGE_BASE_ADDRESS,
> - {
> - {0,}, //ZeroVector[16]
> - EFI_SYSTEM_NV_DATA_FV_GUID,
> - FVB_MEDIA_BLOCK_SIZE * SYSTEM_NV_BLOCK_NUM,
> - EFI_FVH_SIGNATURE,
> - 0x0004feff, // check MdePkg/Include/Pi/PiFirmwareVolume.h for details on EFI_FVB_ATTRIBUTES_2
> - sizeof (EFI_FIRMWARE_VOLUME_HEADER) + sizeof (EFI_FV_BLOCK_MAP_ENTRY),
> - 0, //CheckSum which will be calucated dynamically.
> - 0, //ExtHeaderOffset
> - {0,}, //Reserved[1]
> - 2, //Revision
> - {
> - {
> - SYSTEM_NV_BLOCK_NUM,
> - FVB_MEDIA_BLOCK_SIZE,
> - }
> - }
> - },
> - {
> - {
> - 0,
> - 0
> - }
> - }
> +/**
> + Returns FVB media information for NV variable storage.
> +
> + @return FvbMediaInfo A pointer to an instance of FVB media info produced by this function.
> + The buffer is allocated internally to this function and it is the caller's
> + responsibility to free the
> + memory
> +
> +**/
> +typedef
> +EFI_STATUS
> +(*FVB_MEDIA_INFO_GENERATOR)(
> + OUT EFI_FVB2_MEDIA_INFO *FvbMediaInfo
> + );
> +
> +/**
> + Returns FVB media information for NV variable storage.
> +
> + @param[out] FvbMediaInfo A pointer to an instance of FVB media info produced by this function.
> +
> + @retval EFI_SUCCESS A structure was successfully written to the FvbMediaInfo buffer.
> + @retval EFI_INVALID_PARAMETER The FvbMediaInfo parameter is NULL.
> + @retval EFI_UNSUPPORTED An error occurred retrieving variable FV information.
> + @retval EFI_BAD_BUFFER_SIZE An overflow or underflow of the FV buffer occurred with the information found.
> +
> +**/
> +EFI_STATUS
> +GenerateNvStorageFvbMediaInfo (
> + OUT EFI_FVB2_MEDIA_INFO *FvbMediaInfo
> + )
> +{
> + EFI_STATUS Status;
> + UINT32 NvBlockNum;
> + UINT32 TotalNvVariableStorageSize;
> + EFI_PHYSICAL_ADDRESS NvStorageBaseAddress;
> + EFI_FIRMWARE_VOLUME_HEADER FvbInfo = {
> + {0,}, //ZeroVector[16]
> + EFI_SYSTEM_NV_DATA_FV_GUID, //FileSystemGuid
> + 0, //FvLength
> + EFI_FVH_SIGNATURE, //Signature
> + 0x0004feff, //Attributes
> + sizeof (EFI_FIRMWARE_VOLUME_HEADER) + //HeaderLength
> + sizeof (EFI_FV_BLOCK_MAP_ENTRY),
> + 0, //Checksum
> + 0, //ExtHeaderOffset
> + {0,}, //Reserved[1]
> + 2, //Revision
> + { //BlockMap[1]
> + {0,0}
> + }
> + };
> +
> + if (FvbMediaInfo == NULL) {
> + return EFI_INVALID_PARAMETER;
> }
> +
> + ZeroMem (FvbMediaInfo, sizeof (*FvbMediaInfo));
> +
> + GetVariableFvInfo (&NvStorageBaseAddress,
> + &TotalNvVariableStorageSize); if ((NvStorageBaseAddress == 0) || (TotalNvVariableStorageSize == 0)) {
> + return EFI_UNSUPPORTED;
> + }
> +
> + NvBlockNum = TotalNvVariableStorageSize / FVB_MEDIA_BLOCK_SIZE;
> +
> + Status = SafeUint64Mult ((UINT64)NvBlockNum, FVB_MEDIA_BLOCK_SIZE,
> + &FvbInfo.FvLength); if (EFI_ERROR (Status)) {
> + return EFI_BAD_BUFFER_SIZE;
> + }
> +
> + FvbInfo.BlockMap[0].NumBlocks = NvBlockNum;
> + FvbInfo.BlockMap[0].Length = FVB_MEDIA_BLOCK_SIZE;
> +
> + FvbMediaInfo->BaseAddress = NvStorageBaseAddress; CopyMem
> + (&FvbMediaInfo->FvbInfo, &FvbInfo, sizeof (FvbInfo));
> +
> + return EFI_SUCCESS;
> +}
> +
> +FVB_MEDIA_INFO_GENERATOR mFvbMediaInfoGenerators[] = {
> + GenerateNvStorageFvbMediaInfo
> };
>
> EFI_STATUS
> @@ -64,12 +106,16 @@ GetFvbInfo (
> OUT EFI_FIRMWARE_VOLUME_HEADER **FvbInfo
> )
> {
> + EFI_STATUS Status;
> + EFI_FVB2_MEDIA_INFO FvbMediaInfo;
> UINTN Index;
> EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
>
> - for (Index = 0; Index < sizeof (mPlatformFvbMediaInfo) / sizeof (EFI_FVB2_MEDIA_INFO); Index++) {
> - if (mPlatformFvbMediaInfo[Index].BaseAddress == FvBaseAddress) {
> - FvHeader = &mPlatformFvbMediaInfo[Index].FvbInfo;
> + for (Index = 0; Index < ARRAY_SIZE (mFvbMediaInfoGenerators); Index++) {
> + Status = mFvbMediaInfoGenerators[Index](&FvbMediaInfo);
> + ASSERT_EFI_ERROR (Status);
> + if (!EFI_ERROR (Status) && (FvbMediaInfo.BaseAddress == FvBaseAddress)) {
> + FvHeader = AllocateCopyPool (sizeof
> + (EFI_FIRMWARE_VOLUME_HEADER), &FvbMediaInfo.FvbInfo);
>
> //
> // Update the checksum value of FV header.
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.c index dab818e98087..942abf95a64f 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.c
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
> +++ ServiceCommon.c
> @@ -3,6 +3,7 @@
> which are compliant with the Intel(R) Serial Flash Interface Compatibility Specification.
>
> Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -25,12 +26,6 @@ FV_INFO mPlatformFvBaseAddress[] = {
> {0, 0}
> };
>
> -FV_INFO mPlatformDefaultBaseAddress[] = {
> - {0, 0}, // {FixedPcdGet32(PcdFlashNvStorageVariableBase),
> FixedPcdGet32(PcdFlashNvStorageVariableSize)},
> - {0, 0}, // {FixedPcdGet32(PcdFlashMicrocodeFvBase),
> FixedPcdGet32(PcdFlashMicrocodeFvSize)},
> - {0, 0}
> -};
> -
> FV_MEMMAP_DEVICE_PATH mFvMemmapDevicePathTemplate = {
> {
> {
> @@ -530,6 +525,85 @@ FvbSetVolumeAttributes (
> return EFI_SUCCESS;
> }
>
> +/**
> + Get the total size of the firmware volume on flash used for variable store operations.
> +
> + @param[out] BaseAddress Base address of the variable store firmware volume.
> + @param[out] Length Length in bytes of the firmware volume used for variable store operations.
> +
> +**/
> +VOID
> +GetVariableFvInfo (
> + OUT EFI_PHYSICAL_ADDRESS *BaseAddress,
> + OUT UINT32 *Length
> + )
> +{
> + EFI_STATUS Status;
> + EFI_PHYSICAL_ADDRESS NvBaseAddress;
> + EFI_PHYSICAL_ADDRESS NvVariableBaseAddress;
> + UINT64 Length64;
> + UINT32 NvStoreLength;
> + UINT32 FtwSpareLength;
> + UINT32 FtwWorkingLength;
> + UINT32 TotalLength;
> +
> + TotalLength = 0;
> + Status = EFI_SUCCESS;
> +
> + if ((BaseAddress == NULL) || (Length == NULL)) {
> + ASSERT ((BaseAddress != NULL) && (Length != NULL));
> + return;
> + }
> + *BaseAddress = 0;
> + *Length = 0;
> +
> + Status = GetVariableFlashNvStorageInfo (&NvBaseAddress, &Length64);
> + if (!EFI_ERROR (Status)) {
> + NvVariableBaseAddress = NvBaseAddress;
> + // Stay within the current UINT32 size assumptions in the variable stack.
> + Status = SafeUint64ToUint32 (Length64, &NvStoreLength); } if
> + (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + Status = GetVariableFlashFtwSpareInfo (&NvBaseAddress, &Length64);
> + if (!EFI_ERROR (Status)) {
> + // Stay within the current UINT32 size assumptions in the variable stack.
> + Status = SafeUint64ToUint32 (Length64, &FtwSpareLength); } if
> + (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + Status = GetVariableFlashFtwWorkingInfo (&NvBaseAddress,
> + &Length64); if (!EFI_ERROR (Status)) {
> + // Stay within the current UINT32 size assumptions in the variable stack.
> + Status = SafeUint64ToUint32 (Length64, &FtwWorkingLength); } if
> + (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + Status = SafeUint32Add (NvStoreLength, FtwSpareLength,
> + &TotalLength); if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + Status = SafeUint32Add (TotalLength, FtwWorkingLength,
> + &TotalLength); if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + *BaseAddress = NvVariableBaseAddress;
> + *Length = TotalLength;
> +}
> +
> /**
> Check the integrity of firmware volume header
>
> @@ -545,7 +619,12 @@ IsFvHeaderValid (
> IN CONST EFI_FIRMWARE_VOLUME_HEADER *FvHeader
> )
> {
> - if (FvBase == PcdGet32(PcdFlashNvStorageVariableBase)) {
> + EFI_PHYSICAL_ADDRESS NvStorageFvBaseAddress;
> + UINT32 NvStorageSize;
> +
> + GetVariableFvInfo (&NvStorageFvBaseAddress, &NvStorageSize);
> +
> + if (FvBase == NvStorageFvBaseAddress) {
> if (CompareMem (&FvHeader->FileSystemGuid, &gEfiSystemNvDataFvGuid, sizeof(EFI_GUID)) != 0 ) {
> return FALSE;
> }
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceMm.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceMm.c index 42a0828c6fae..6b4bcdcfe3c4 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceMm.c
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
> +++ ServiceMm.c
> @@ -113,15 +113,31 @@ FvbInitialize (
> UINT32 MaxLbaSize;
> UINT32 BytesWritten;
> UINTN BytesErased;
> + UINT64 NvStorageFvSize;
> +
> + Status = GetVariableFlashNvStorageInfo (&BaseAddress,
> + &NvStorageFvSize); if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + DEBUG ((DEBUG_ERROR, "[%a] - An error ocurred getting variable info - %r.\n", __FUNCTION__, Status));
> + return;
> + }
> +
> + // Stay within the current UINT32 size assumptions in the variable stack.
> + Status = SafeUint64ToUint32 (BaseAddress,
> + &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);
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + DEBUG ((DEBUG_ERROR, "[%a] - 64-bit variable storage size not supported.\n", __FUNCTION__));
> + return;
> + }
>
> - mPlatformFvBaseAddress[0].FvBase =
> PcdGet32(PcdFlashNvStorageVariableBase);
> - mPlatformFvBaseAddress[0].FvSize = PcdGet32(PcdFlashNvStorageVariableSize);
> mPlatformFvBaseAddress[1].FvBase = PcdGet32(PcdFlashMicrocodeFvBase);
> mPlatformFvBaseAddress[1].FvSize =
> PcdGet32(PcdFlashMicrocodeFvSize);
> - mPlatformDefaultBaseAddress[0].FvBase =
> PcdGet32(PcdFlashNvStorageVariableBase);
> - mPlatformDefaultBaseAddress[0].FvSize =
> PcdGet32(PcdFlashNvStorageVariableSize);
> - mPlatformDefaultBaseAddress[1].FvBase =
> PcdGet32(PcdFlashMicrocodeFvBase);
> - mPlatformDefaultBaseAddress[1].FvSize =
> PcdGet32(PcdFlashMicrocodeFvSize);
>
> //
> // We will only continue with FVB installation if the @@ -143,7
> +159,8 @@ FvbInitialize (
> BytesWritten = 0;
> BytesErased = 0;
> DEBUG ((DEBUG_ERROR, "ERROR - The FV in 0x%x is invalid!\n", FvHeader));
> - Status = GetFvbInfo (BaseAddress, &FvHeader);
> + FvHeader = NULL;
> + Status = GetFvbInfo (BaseAddress, &FvHeader);
> if (EFI_ERROR (Status)) {
> DEBUG ((DEBUG_WARN, "ERROR - Can't recovery FV header at 0x%x. GetFvbInfo Status %r\n", BaseAddress, Status));
> continue;
> @@ -156,27 +173,42 @@ FvbInitialize (
> 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 (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 (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)); @@ -184,6 +216,9 @@ FvbInitialize (
> // Clear cache for this range.
> //
> WriteBackInvalidateDataCacheRange ( (VOID *) (UINTN)
> BaseAddress, FvHeader->BlockMap->Length);
> + if (FvHeader != NULL) {
> + FreePool (FvHeader);
> + }
> }
> }
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.h
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.h index e9d69e985814..7b0908b03fdc 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.h
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
> +++ ServiceCommon.h
> @@ -2,6 +2,7 @@
> Common source definitions used in serial flash drivers
>
> Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -12,6 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Guid/EventGroup.h>
> #include <Guid/FirmwareFileSystem2.h>
> #include <Guid/SystemNvDataGuid.h>
> +#include <Pi/PiFirmwareVolume.h>
> #include <Protocol/DevicePath.h>
> #include <Protocol/FirmwareVolumeBlock.h>
>
> @@ -24,6 +26,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Library/PcdLib.h>
> #include <Library/DevicePathLib.h>
> #include <Library/HobLib.h>
> +#include <Library/VariableFlashInfoLib.h> #include
> +<Library/SafeIntLib.h>
>
> #include <Library/SpiFlashCommonLib.h>
>
> @@ -148,11 +152,23 @@ GetFvbInfo (
> OUT EFI_FIRMWARE_VOLUME_HEADER **FvbInfo
> );
>
> +/**
> + Get the total size of the firmware volume on flash used for variable store operations.
> +
> + @param[out] BaseAddress Base address of the variable store firmware volume.
> + @param[out] Length Length in bytes of the firmware volume used for variable store operations.
> +
> +**/
> +VOID
> +GetVariableFvInfo (
> + OUT EFI_PHYSICAL_ADDRESS *BaseAddress,
> + OUT UINT32 *Length
> + );
> +
> extern FVB_GLOBAL mFvbModuleGlobal;
> extern FV_MEMMAP_DEVICE_PATH mFvMemmapDevicePathTemplate;
> extern FV_PIWG_DEVICE_PATH mFvPIWGDevicePathTemplate;
> extern EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL mFvbProtocolTemplate;
> extern FV_INFO mPlatformFvBaseAddress[];
> -extern FV_INFO mPlatformDefaultBaseAddress[];
>
> #endif
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceSmm.inf
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceSmm.inf index bf1306f00201..0cfa3f909bdf 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceSmm.inf
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
> +++ ServiceSmm.inf
> @@ -32,8 +32,10 @@ [LibraryClasses]
> BaseLib
> UefiBootServicesTableLib
> UefiDriverEntryPoint
> + SafeIntLib
> SpiFlashCommonLib
> MmServicesTableLib
> + VariableFlashInfoLib
>
> [Packages]
> MdePkg/MdePkg.dec
> @@ -41,10 +43,6 @@ [Packages]
> IntelSiliconPkg/IntelSiliconPkg.dec
>
> [Pcd]
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize ## CONSUMES
> gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CONSUMES
> gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CONSUMES
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceStandaloneMm.inf
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceStandaloneMm.inf index b66233968247..152cf0036fdb 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceStandaloneMm.inf
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
> +++ ServiceStandaloneMm.inf
> @@ -31,8 +31,10 @@ [LibraryClasses]
> MemoryAllocationLib
> PcdLib
> MmServicesTableLib
> + SafeIntLib
> SpiFlashCommonLib
> StandaloneMmDriverEntryPoint
> + VariableFlashInfoLib
>
> [Packages]
> MdePkg/MdePkg.dec
> @@ -40,10 +42,6 @@ [Packages]
> IntelSiliconPkg/IntelSiliconPkg.dec
>
> [Pcd]
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize ## CONSUMES
> gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CONSUMES
> gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CONSUMES
>
> diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
> b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
> index 1e826a080f28..170eb480ada1 100644
> --- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
> +++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
> @@ -40,9 +40,11 @@ [LibraryClasses]
> PeiGetVtdPmrAlignmentLib|IntelSiliconPkg/Library/PeiGetVtdPmrAlignmentLib/PeiGetVtdPmrAlignmentLib.inf
> TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
> MicrocodeLib|UefiCpuPkg/Library/MicrocodeLib/MicrocodeLib.inf
> + SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> SpiFlashCommonLib|IntelSiliconPkg/Library/SpiFlashCommonLibNull/SpiFlashCommonLibNull.inf
> UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
>
> UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEnt
> ryPoint.inf
> +
> + VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/B
> + aseVariableFlashInfoLib.inf
>
> [LibraryClasses.common.PEIM]
> PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
> @@ -63,11 +65,16 @@ [LibraryClasses.common.DXE_DRIVER]
>
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryA
> llocationLib.inf
>
> [LibraryClasses.common.DXE_SMM_DRIVER]
> +
> + DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
> + HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.inf
> MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf
>
> SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTabl
> eLib.inf
> + UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
> +
> + UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableL
> + ib/UefiRuntimeServicesTableLib.inf
>
> [LibraryClasses.common.MM_STANDALONE]
> +
> + HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib
> + .inf
> MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
> MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
>
> StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoi
> nt/StandaloneMmDriverEntryPoint.inf
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH v2 1/1] IntelSiliconPkg/SpiFvbService: Add support for VariableFlashInfoLib
2022-07-13 16:49 ` Chaganty, Rangasai V
@ 2022-07-13 20:59 ` Oram, Isaac W
0 siblings, 0 replies; 4+ messages in thread
From: Oram, Isaac W @ 2022-07-13 20:59 UTC (permalink / raw)
To: Chaganty, Rangasai V, devel@edk2.groups.io,
mikuback@linux.microsoft.com
Cc: Ni, Ray, Desimone, Nathaniel L
Pushed as 9935472b19..bc93dea9b9
-----Original Message-----
From: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>
Sent: Wednesday, July 13, 2022 9:49 AM
To: devel@edk2.groups.io; mikuback@linux.microsoft.com
Cc: Ni, Ray <ray.ni@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Subject: RE: [edk2-devel] [edk2-platforms][PATCH v2 1/1] IntelSiliconPkg/SpiFvbService: Add support for VariableFlashInfoLib
Reviewed-by: Sai Chaganty <rangasai.v.chaganty@intel.com>
-----Original Message-----
From: Michael Kubacki <mikuback@linux.microsoft.com>
Sent: Tuesday, July 12, 2022 4:04 PM
To: devel@edk2.groups.io
Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Ni, Ray <ray.ni@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH v2 1/1] IntelSiliconPkg/SpiFvbService: Add support for VariableFlashInfoLib
Reminder about this patch.
Thanks,
Michael
On 7/1/2022 10:39 AM, Michael Kubacki wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3478
>
> Adds support for getting the variable flash information from
> VariableFlashInfoLib. This library abstracts the source of flash
> information so platforms could expect the information to come from a
> different source in the library implementation than the PCDs
> previously used as the information source in this module.
>
> In particular, the library allows Standalone MM platforms to
> dynamically pass the information behind the library API.
>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Isaac Oram <isaac.w.oram@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>
> Notes:
> V2 changes:
>
> 1. Updated the buffer that is returned from
> GetFvbInfo() to be allocated from pool.
> 2. Updated calls to that function in FvbInitialize()
> to free the new buffer after usage.
> 3. Updated the multiplication operation to determine
> the FV length to use safe multiplication in
> GenerateNvStorageFvbMediaInfo().
>
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c | 128 +++++++++++++-------
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c | 93 ++++++++++++--
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c | 49 ++++++--
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.h | 18 ++-
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceSmm.inf | 6 +-
> Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceStandaloneMm.inf | 6 +-
> Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc | 7 ++
> 7 files changed, 243 insertions(+), 64 deletions(-)
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
> index 7f2678fa9e5a..634a44218c7a 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInf
> +++ o.c
> @@ -3,6 +3,7 @@
> These data is intent to decouple FVB driver with FV header.
>
> Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -11,51 +12,92 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
> #define FIRMWARE_BLOCK_SIZE 0x10000
> #define FVB_MEDIA_BLOCK_SIZE FIRMWARE_BLOCK_SIZE
> -
> -#define NV_STORAGE_BASE_ADDRESS FixedPcdGet32(PcdFlashNvStorageVariableBase)
> -#define SYSTEM_NV_BLOCK_NUM ((FixedPcdGet32(PcdFlashNvStorageVariableSize)+ FixedPcdGet32(PcdFlashNvStorageFtwWorkingSize) + FixedPcdGet32(PcdFlashNvStorageFtwSpareSize))/ FVB_MEDIA_BLOCK_SIZE)
> -
> typedef struct {
> EFI_PHYSICAL_ADDRESS BaseAddress;
> EFI_FIRMWARE_VOLUME_HEADER FvbInfo;
> EFI_FV_BLOCK_MAP_ENTRY End[1];
> } EFI_FVB2_MEDIA_INFO;
>
> -//
> -// This data structure contains a template of all correct FV headers,
> which is used to restore -// Fv header if it's corrupted.
> -//
> -EFI_FVB2_MEDIA_INFO mPlatformFvbMediaInfo[] = {
> - //
> - // Systen NvStorage FVB
> - //
> - {
> - NV_STORAGE_BASE_ADDRESS,
> - {
> - {0,}, //ZeroVector[16]
> - EFI_SYSTEM_NV_DATA_FV_GUID,
> - FVB_MEDIA_BLOCK_SIZE * SYSTEM_NV_BLOCK_NUM,
> - EFI_FVH_SIGNATURE,
> - 0x0004feff, // check MdePkg/Include/Pi/PiFirmwareVolume.h for details on EFI_FVB_ATTRIBUTES_2
> - sizeof (EFI_FIRMWARE_VOLUME_HEADER) + sizeof (EFI_FV_BLOCK_MAP_ENTRY),
> - 0, //CheckSum which will be calucated dynamically.
> - 0, //ExtHeaderOffset
> - {0,}, //Reserved[1]
> - 2, //Revision
> - {
> - {
> - SYSTEM_NV_BLOCK_NUM,
> - FVB_MEDIA_BLOCK_SIZE,
> - }
> - }
> - },
> - {
> - {
> - 0,
> - 0
> - }
> - }
> +/**
> + Returns FVB media information for NV variable storage.
> +
> + @return FvbMediaInfo A pointer to an instance of FVB media info produced by this function.
> + The buffer is allocated internally to this function and it is the caller's
> + responsibility to free the
> + memory
> +
> +**/
> +typedef
> +EFI_STATUS
> +(*FVB_MEDIA_INFO_GENERATOR)(
> + OUT EFI_FVB2_MEDIA_INFO *FvbMediaInfo
> + );
> +
> +/**
> + Returns FVB media information for NV variable storage.
> +
> + @param[out] FvbMediaInfo A pointer to an instance of FVB media info produced by this function.
> +
> + @retval EFI_SUCCESS A structure was successfully written to the FvbMediaInfo buffer.
> + @retval EFI_INVALID_PARAMETER The FvbMediaInfo parameter is NULL.
> + @retval EFI_UNSUPPORTED An error occurred retrieving variable FV information.
> + @retval EFI_BAD_BUFFER_SIZE An overflow or underflow of the FV buffer occurred with the information found.
> +
> +**/
> +EFI_STATUS
> +GenerateNvStorageFvbMediaInfo (
> + OUT EFI_FVB2_MEDIA_INFO *FvbMediaInfo
> + )
> +{
> + EFI_STATUS Status;
> + UINT32 NvBlockNum;
> + UINT32 TotalNvVariableStorageSize;
> + EFI_PHYSICAL_ADDRESS NvStorageBaseAddress;
> + EFI_FIRMWARE_VOLUME_HEADER FvbInfo = {
> + {0,}, //ZeroVector[16]
> + EFI_SYSTEM_NV_DATA_FV_GUID, //FileSystemGuid
> + 0, //FvLength
> + EFI_FVH_SIGNATURE, //Signature
> + 0x0004feff, //Attributes
> + sizeof (EFI_FIRMWARE_VOLUME_HEADER) + //HeaderLength
> + sizeof (EFI_FV_BLOCK_MAP_ENTRY),
> + 0, //Checksum
> + 0, //ExtHeaderOffset
> + {0,}, //Reserved[1]
> + 2, //Revision
> + { //BlockMap[1]
> + {0,0}
> + }
> + };
> +
> + if (FvbMediaInfo == NULL) {
> + return EFI_INVALID_PARAMETER;
> }
> +
> + ZeroMem (FvbMediaInfo, sizeof (*FvbMediaInfo));
> +
> + GetVariableFvInfo (&NvStorageBaseAddress,
> + &TotalNvVariableStorageSize); if ((NvStorageBaseAddress == 0) || (TotalNvVariableStorageSize == 0)) {
> + return EFI_UNSUPPORTED;
> + }
> +
> + NvBlockNum = TotalNvVariableStorageSize / FVB_MEDIA_BLOCK_SIZE;
> +
> + Status = SafeUint64Mult ((UINT64)NvBlockNum, FVB_MEDIA_BLOCK_SIZE,
> + &FvbInfo.FvLength); if (EFI_ERROR (Status)) {
> + return EFI_BAD_BUFFER_SIZE;
> + }
> +
> + FvbInfo.BlockMap[0].NumBlocks = NvBlockNum;
> + FvbInfo.BlockMap[0].Length = FVB_MEDIA_BLOCK_SIZE;
> +
> + FvbMediaInfo->BaseAddress = NvStorageBaseAddress; CopyMem
> + (&FvbMediaInfo->FvbInfo, &FvbInfo, sizeof (FvbInfo));
> +
> + return EFI_SUCCESS;
> +}
> +
> +FVB_MEDIA_INFO_GENERATOR mFvbMediaInfoGenerators[] = {
> + GenerateNvStorageFvbMediaInfo
> };
>
> EFI_STATUS
> @@ -64,12 +106,16 @@ GetFvbInfo (
> OUT EFI_FIRMWARE_VOLUME_HEADER **FvbInfo
> )
> {
> + EFI_STATUS Status;
> + EFI_FVB2_MEDIA_INFO FvbMediaInfo;
> UINTN Index;
> EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
>
> - for (Index = 0; Index < sizeof (mPlatformFvbMediaInfo) / sizeof (EFI_FVB2_MEDIA_INFO); Index++) {
> - if (mPlatformFvbMediaInfo[Index].BaseAddress == FvBaseAddress) {
> - FvHeader = &mPlatformFvbMediaInfo[Index].FvbInfo;
> + for (Index = 0; Index < ARRAY_SIZE (mFvbMediaInfoGenerators); Index++) {
> + Status = mFvbMediaInfoGenerators[Index](&FvbMediaInfo);
> + ASSERT_EFI_ERROR (Status);
> + if (!EFI_ERROR (Status) && (FvbMediaInfo.BaseAddress == FvBaseAddress)) {
> + FvHeader = AllocateCopyPool (sizeof
> + (EFI_FIRMWARE_VOLUME_HEADER), &FvbMediaInfo.FvbInfo);
>
> //
> // Update the checksum value of FV header.
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.c index dab818e98087..942abf95a64f 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.c
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
> +++ ServiceCommon.c
> @@ -3,6 +3,7 @@
> which are compliant with the Intel(R) Serial Flash Interface Compatibility Specification.
>
> Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -25,12 +26,6 @@ FV_INFO mPlatformFvBaseAddress[] = {
> {0, 0}
> };
>
> -FV_INFO mPlatformDefaultBaseAddress[] = {
> - {0, 0}, // {FixedPcdGet32(PcdFlashNvStorageVariableBase),
> FixedPcdGet32(PcdFlashNvStorageVariableSize)},
> - {0, 0}, // {FixedPcdGet32(PcdFlashMicrocodeFvBase),
> FixedPcdGet32(PcdFlashMicrocodeFvSize)},
> - {0, 0}
> -};
> -
> FV_MEMMAP_DEVICE_PATH mFvMemmapDevicePathTemplate = {
> {
> {
> @@ -530,6 +525,85 @@ FvbSetVolumeAttributes (
> return EFI_SUCCESS;
> }
>
> +/**
> + Get the total size of the firmware volume on flash used for variable store operations.
> +
> + @param[out] BaseAddress Base address of the variable store firmware volume.
> + @param[out] Length Length in bytes of the firmware volume used for variable store operations.
> +
> +**/
> +VOID
> +GetVariableFvInfo (
> + OUT EFI_PHYSICAL_ADDRESS *BaseAddress,
> + OUT UINT32 *Length
> + )
> +{
> + EFI_STATUS Status;
> + EFI_PHYSICAL_ADDRESS NvBaseAddress;
> + EFI_PHYSICAL_ADDRESS NvVariableBaseAddress;
> + UINT64 Length64;
> + UINT32 NvStoreLength;
> + UINT32 FtwSpareLength;
> + UINT32 FtwWorkingLength;
> + UINT32 TotalLength;
> +
> + TotalLength = 0;
> + Status = EFI_SUCCESS;
> +
> + if ((BaseAddress == NULL) || (Length == NULL)) {
> + ASSERT ((BaseAddress != NULL) && (Length != NULL));
> + return;
> + }
> + *BaseAddress = 0;
> + *Length = 0;
> +
> + Status = GetVariableFlashNvStorageInfo (&NvBaseAddress, &Length64);
> + if (!EFI_ERROR (Status)) {
> + NvVariableBaseAddress = NvBaseAddress;
> + // Stay within the current UINT32 size assumptions in the variable stack.
> + Status = SafeUint64ToUint32 (Length64, &NvStoreLength); } if
> + (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + Status = GetVariableFlashFtwSpareInfo (&NvBaseAddress, &Length64);
> + if (!EFI_ERROR (Status)) {
> + // Stay within the current UINT32 size assumptions in the variable stack.
> + Status = SafeUint64ToUint32 (Length64, &FtwSpareLength); } if
> + (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + Status = GetVariableFlashFtwWorkingInfo (&NvBaseAddress,
> + &Length64); if (!EFI_ERROR (Status)) {
> + // Stay within the current UINT32 size assumptions in the variable stack.
> + Status = SafeUint64ToUint32 (Length64, &FtwWorkingLength); } if
> + (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + Status = SafeUint32Add (NvStoreLength, FtwSpareLength,
> + &TotalLength); if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + Status = SafeUint32Add (TotalLength, FtwWorkingLength,
> + &TotalLength); if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return;
> + }
> +
> + *BaseAddress = NvVariableBaseAddress;
> + *Length = TotalLength;
> +}
> +
> /**
> Check the integrity of firmware volume header
>
> @@ -545,7 +619,12 @@ IsFvHeaderValid (
> IN CONST EFI_FIRMWARE_VOLUME_HEADER *FvHeader
> )
> {
> - if (FvBase == PcdGet32(PcdFlashNvStorageVariableBase)) {
> + EFI_PHYSICAL_ADDRESS NvStorageFvBaseAddress;
> + UINT32 NvStorageSize;
> +
> + GetVariableFvInfo (&NvStorageFvBaseAddress, &NvStorageSize);
> +
> + if (FvBase == NvStorageFvBaseAddress) {
> if (CompareMem (&FvHeader->FileSystemGuid, &gEfiSystemNvDataFvGuid, sizeof(EFI_GUID)) != 0 ) {
> return FALSE;
> }
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceMm.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceMm.c index 42a0828c6fae..6b4bcdcfe3c4 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceMm.c
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
> +++ ServiceMm.c
> @@ -113,15 +113,31 @@ FvbInitialize (
> UINT32 MaxLbaSize;
> UINT32 BytesWritten;
> UINTN BytesErased;
> + UINT64 NvStorageFvSize;
> +
> + Status = GetVariableFlashNvStorageInfo (&BaseAddress,
> + &NvStorageFvSize); if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + DEBUG ((DEBUG_ERROR, "[%a] - An error ocurred getting variable info - %r.\n", __FUNCTION__, Status));
> + return;
> + }
> +
> + // Stay within the current UINT32 size assumptions in the variable stack.
> + Status = SafeUint64ToUint32 (BaseAddress,
> + &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);
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + DEBUG ((DEBUG_ERROR, "[%a] - 64-bit variable storage size not supported.\n", __FUNCTION__));
> + return;
> + }
>
> - mPlatformFvBaseAddress[0].FvBase =
> PcdGet32(PcdFlashNvStorageVariableBase);
> - mPlatformFvBaseAddress[0].FvSize = PcdGet32(PcdFlashNvStorageVariableSize);
> mPlatformFvBaseAddress[1].FvBase = PcdGet32(PcdFlashMicrocodeFvBase);
> mPlatformFvBaseAddress[1].FvSize =
> PcdGet32(PcdFlashMicrocodeFvSize);
> - mPlatformDefaultBaseAddress[0].FvBase =
> PcdGet32(PcdFlashNvStorageVariableBase);
> - mPlatformDefaultBaseAddress[0].FvSize =
> PcdGet32(PcdFlashNvStorageVariableSize);
> - mPlatformDefaultBaseAddress[1].FvBase =
> PcdGet32(PcdFlashMicrocodeFvBase);
> - mPlatformDefaultBaseAddress[1].FvSize =
> PcdGet32(PcdFlashMicrocodeFvSize);
>
> //
> // We will only continue with FVB installation if the @@ -143,7
> +159,8 @@ FvbInitialize (
> BytesWritten = 0;
> BytesErased = 0;
> DEBUG ((DEBUG_ERROR, "ERROR - The FV in 0x%x is invalid!\n", FvHeader));
> - Status = GetFvbInfo (BaseAddress, &FvHeader);
> + FvHeader = NULL;
> + Status = GetFvbInfo (BaseAddress, &FvHeader);
> if (EFI_ERROR (Status)) {
> DEBUG ((DEBUG_WARN, "ERROR - Can't recovery FV header at 0x%x. GetFvbInfo Status %r\n", BaseAddress, Status));
> continue;
> @@ -156,27 +173,42 @@ FvbInitialize (
> 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 (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 (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)); @@ -184,6 +216,9 @@ FvbInitialize (
> // Clear cache for this range.
> //
> WriteBackInvalidateDataCacheRange ( (VOID *) (UINTN)
> BaseAddress, FvHeader->BlockMap->Length);
> + if (FvHeader != NULL) {
> + FreePool (FvHeader);
> + }
> }
> }
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.h
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.h index e9d69e985814..7b0908b03fdc 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceCommon.h
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
> +++ ServiceCommon.h
> @@ -2,6 +2,7 @@
> Common source definitions used in serial flash drivers
>
> Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -12,6 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Guid/EventGroup.h>
> #include <Guid/FirmwareFileSystem2.h>
> #include <Guid/SystemNvDataGuid.h>
> +#include <Pi/PiFirmwareVolume.h>
> #include <Protocol/DevicePath.h>
> #include <Protocol/FirmwareVolumeBlock.h>
>
> @@ -24,6 +26,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Library/PcdLib.h>
> #include <Library/DevicePathLib.h>
> #include <Library/HobLib.h>
> +#include <Library/VariableFlashInfoLib.h> #include
> +<Library/SafeIntLib.h>
>
> #include <Library/SpiFlashCommonLib.h>
>
> @@ -148,11 +152,23 @@ GetFvbInfo (
> OUT EFI_FIRMWARE_VOLUME_HEADER **FvbInfo
> );
>
> +/**
> + Get the total size of the firmware volume on flash used for variable store operations.
> +
> + @param[out] BaseAddress Base address of the variable store firmware volume.
> + @param[out] Length Length in bytes of the firmware volume used for variable store operations.
> +
> +**/
> +VOID
> +GetVariableFvInfo (
> + OUT EFI_PHYSICAL_ADDRESS *BaseAddress,
> + OUT UINT32 *Length
> + );
> +
> extern FVB_GLOBAL mFvbModuleGlobal;
> extern FV_MEMMAP_DEVICE_PATH mFvMemmapDevicePathTemplate;
> extern FV_PIWG_DEVICE_PATH mFvPIWGDevicePathTemplate;
> extern EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL mFvbProtocolTemplate;
> extern FV_INFO mPlatformFvBaseAddress[];
> -extern FV_INFO mPlatformDefaultBaseAddress[];
>
> #endif
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceSmm.inf
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceSmm.inf index bf1306f00201..0cfa3f909bdf 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceSmm.inf
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
> +++ ServiceSmm.inf
> @@ -32,8 +32,10 @@ [LibraryClasses]
> BaseLib
> UefiBootServicesTableLib
> UefiDriverEntryPoint
> + SafeIntLib
> SpiFlashCommonLib
> MmServicesTableLib
> + VariableFlashInfoLib
>
> [Packages]
> MdePkg/MdePkg.dec
> @@ -41,10 +43,6 @@ [Packages]
> IntelSiliconPkg/IntelSiliconPkg.dec
>
> [Pcd]
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize ## CONSUMES
> gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CONSUMES
> gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CONSUMES
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceStandaloneMm.inf
> b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceStandaloneMm.inf index b66233968247..152cf0036fdb 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServ
> iceStandaloneMm.inf
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvb
> +++ ServiceStandaloneMm.inf
> @@ -31,8 +31,10 @@ [LibraryClasses]
> MemoryAllocationLib
> PcdLib
> MmServicesTableLib
> + SafeIntLib
> SpiFlashCommonLib
> StandaloneMmDriverEntryPoint
> + VariableFlashInfoLib
>
> [Packages]
> MdePkg/MdePkg.dec
> @@ -40,10 +42,6 @@ [Packages]
> IntelSiliconPkg/IntelSiliconPkg.dec
>
> [Pcd]
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize ## CONSUMES
> gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvBase ## CONSUMES
> gIntelSiliconPkgTokenSpaceGuid.PcdFlashMicrocodeFvSize ## CONSUMES
>
> diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
> b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
> index 1e826a080f28..170eb480ada1 100644
> --- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
> +++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc
> @@ -40,9 +40,11 @@ [LibraryClasses]
> PeiGetVtdPmrAlignmentLib|IntelSiliconPkg/Library/PeiGetVtdPmrAlignmentLib/PeiGetVtdPmrAlignmentLib.inf
> TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
> MicrocodeLib|UefiCpuPkg/Library/MicrocodeLib/MicrocodeLib.inf
> + SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> SpiFlashCommonLib|IntelSiliconPkg/Library/SpiFlashCommonLibNull/SpiFlashCommonLibNull.inf
>
> UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiB
> ootServicesTableLib.inf
>
> UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEnt
> ryPoint.inf
> +
> + VariableFlashInfoLib|MdeModulePkg/Library/BaseVariableFlashInfoLib/B
> + aseVariableFlashInfoLib.inf
>
> [LibraryClasses.common.PEIM]
> PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
> @@ -63,11 +65,16 @@ [LibraryClasses.common.DXE_DRIVER]
>
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryA
> llocationLib.inf
>
> [LibraryClasses.common.DXE_SMM_DRIVER]
> +
> + DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
> + HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.inf
>
> MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLi
> b.inf
>
> SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTabl
> eLib.inf
> + UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
> +
> + UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableL
> + ib/UefiRuntimeServicesTableLib.inf
>
> [LibraryClasses.common.MM_STANDALONE]
> +
> + HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib
> + .inf
> MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
>
> MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Standal
> oneMmServicesTableLib.inf
>
> StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoi
> nt/StandaloneMmDriverEntryPoint.inf
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-07-13 20:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <16FDBB890A0E8B8C.7323@groups.io>
2022-07-12 23:04 ` [edk2-devel] [edk2-platforms][PATCH v2 1/1] IntelSiliconPkg/SpiFvbService: Add support for VariableFlashInfoLib Michael Kubacki
2022-07-13 15:51 ` Oram, Isaac W
2022-07-13 16:49 ` Chaganty, Rangasai V
2022-07-13 20:59 ` Oram, Isaac W
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox