Reviewed-by: Ashraf Ali S <ashraf.al...@intel.com>
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
Sent: Friday, June 30, 2023 8:03 AM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray...@intel.com>; Chaganty, Rangasai V
<rangasai.v.chaga...@intel.com>; Oram, Isaac W <isaac.w.o...@intel.com>; S,
Ashraf Ali <ashraf.al...@intel.com>
Subject: [edk2-devel] [edk2-platforms][PATCH v1 1/1]
IntelSiliconPkg/SpiFvbService: Non-functional cleanup
From: Michael Kubacki <michael.kuba...@microsoft.com>
During a review of this driver a number of improvements were noted such as
strengthening function input validation, checking return values, making debug
print error levels consistent in certain code blocks, etc.
These type of changes are made with no explicit change to driver functionality.
Cc: Ray Ni <ray...@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaga...@intel.com>
Cc: Isaac Oram <isaac.w.o...@intel.com>
Cc: Ashraf Ali S <ashraf.al...@intel.com>
Signed-off-by: Michael Kubacki <michael.kuba...@microsoft.com>
---
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
| 35 ++-
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
| 248 ++++++++++++--------
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c
| 54 +++--
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.h
| 31 ++-
Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.h
| 6 +-
5 files changed, 239 insertions(+), 135 deletions(-)
diff --git
a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
index ab1cb2ef1622..ebbd9edaada3 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/FvbInfo.
+++ c
@@ -2,16 +2,17 @@
Defines data structure that is the volume header found.
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
+ Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) Microsoft Corporation.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
**/
#include "SpiFvbServiceCommon.h"
-#define FIRMWARE_BLOCK_SIZE 0x10000
+#define FIRMWARE_BLOCK_SIZE SIZE_64KB
#define FVB_MEDIA_BLOCK_SIZE FIRMWARE_BLOCK_SIZE
+
typedef struct {
EFI_PHYSICAL_ADDRESS BaseAddress;
EFI_FIRMWARE_VOLUME_HEADER FvbInfo;
@@ -19,7 +20,7 @@ typedef struct {
} EFI_FVB2_MEDIA_INFO;
/**
- Returns FVB media information for NV variable storage.
+ Returns FVB media information for a firmware volume.
@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 @@ -100,8 +101,21 @@
FVB_MEDIA_INFO_GENERATOR mFvbMediaInfoGenerators[] = {
GenerateNvStorageFvbMediaInfo
};
+/**
+ Returns an empty firmware volume for the firmware volume at the given base
address.
+
+ @param[in] FvBaseAddress The base address of the firmware volume
requested.
+ @param[out] FvbInfo A pointer that will be set to a buffer for
the firmware volume header
+ at the given base address. The buffer is a
pool allocation made in this function.
+
+ @retval EFI_SUCCESS The firmware volume was returned
successfully.
+ @retval EFI_INVALID_PARAMETER The FvbInfo pointer argument is NULL.
+ @retval EFI_NOT_FOUND The firmware volume was not found for the
given base address.
+ @retval EFI_OUT_OF_RESOURCES Insufficient memory to allocate a buffer
to the hold the firmware volume.
+
+**/
EFI_STATUS
-GetFvbInfo (
+GetGeneratedFvByAddress (
IN EFI_PHYSICAL_ADDRESS FvBaseAddress,
OUT EFI_FIRMWARE_VOLUME_HEADER **FvbInfo
)
@@ -111,11 +125,19 @@ GetFvbInfo (
UINTN Index;
EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
+ if (FvbInfo == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
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 (FvbMediaInfo.FvbInfo.HeaderLength,
&FvbMediaInfo.FvbInfo);
+ if (FvHeader == NULL) {
+ ASSERT (FvHeader != NULL);
+ return EFI_OUT_OF_RESOURCES;
+ }
//
// Update the checksum value of FV header.
@@ -136,5 +158,6 @@ GetFvbInfo (
return EFI_SUCCESS;
}
}
+
return EFI_NOT_FOUND;
}
diff --git
a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
index fcdc715263f2..e21113682f4f 100644
---
a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSe
+++ rviceCommon.c
@@ -2,9 +2,9 @@
Common driver source for several Serial Flash devices
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
+ Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) Microsoft Corporation.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -92,13 +92,19 @@ EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL mFvbProtocolTemplate = {
@param[in] FvbInstance The pointer to the EFI_FVB_INSTANCE.
@return Attributes of the FV identified by FvbInstance.
+ Zero is returned if the FvbInstance pointer is NULL.
**/
EFI_FVB_ATTRIBUTES_2
FvbGetVolumeAttributes (
- IN EFI_FVB_INSTANCE *FvbInstance
+ IN CONST EFI_FVB_INSTANCE *FvbInstance
)
{
+ if (FvbInstance == NULL) {
+ ASSERT (FvbInstance != NULL);
+ return 0;
+ }
+
return FvbInstance->FvHeader.Attributes; }
@@ -109,24 +115,25 @@ FvbGetVolumeAttributes (
@param[in] FvbInstance The pointer to the EFI_FVB_INSTANCE.
@param[in] Lba The logical block address
@param[out] LbaAddress On output, contains the physical starting address
- of the Lba
- @param[out] LbaLength On output, contains the length of the block
+ of the Lba. This pointer is optional and may be
NULL.
+ @param[out] LbaLength On output, contains the length of the block.
+ This pointer is optional and may be NULL.
@param[out] NumOfBlocks A pointer to a caller allocated UINTN in which
the
number of consecutive blocks starting with Lba is
returned. All blocks in this range have a size of
- BlockSize
+ BlockSize. This pointer is optional and may be
NULL.
- @retval EFI_SUCCESS Successfully returns
- @retval EFI_INVALID_PARAMETER Instance not found
+ @retval EFI_SUCCESS Successfully returns
+ @retval EFI_INVALID_PARAMETER FvbInstance is NULL.
**/
EFI_STATUS
FvbGetLbaAddress (
- IN EFI_FVB_INSTANCE *FvbInstance,
+ IN CONST EFI_FVB_INSTANCE *FvbInstance,
IN EFI_LBA Lba,
- OUT UINTN *LbaAddress,
- OUT UINTN *LbaLength,
- OUT UINTN *NumOfBlocks
+ OUT UINTN *LbaAddress OPTIONAL,
+ OUT UINTN *LbaLength OPTIONAL,
+ OUT UINTN *NumOfBlocks OPTIONAL
)
{
UINT32 NumBlocks;
@@ -134,10 +141,15 @@ FvbGetLbaAddress (
UINTN Offset;
EFI_LBA StartLba;
EFI_LBA NextLba;
- EFI_FV_BLOCK_MAP_ENTRY *BlockMap;
+ CONST EFI_FV_BLOCK_MAP_ENTRY *BlockMap;
StartLba = 0;
Offset = 0;
+
+ if (FvbInstance == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
BlockMap = &(FvbInstance->FvHeader.BlockMap[0]);
//
@@ -158,15 +170,15 @@ FvbGetLbaAddress (
//
if (Lba >= StartLba && Lba < NextLba) {
Offset = Offset + (UINTN)MultU64x32((Lba - StartLba), BlockLength);
- if (LbaAddress ) {
+ if (LbaAddress != NULL) {
*LbaAddress = FvbInstance->FvBase + Offset;
}
- if (LbaLength ) {
+ if (LbaLength != NULL) {
*LbaLength = BlockLength;
}
- if (NumOfBlocks ) {
+ if (NumOfBlocks != NULL) {
*NumOfBlocks = (UINTN)(NextLba - Lba);
}
return EFI_SUCCESS;
@@ -190,7 +202,6 @@ FvbGetLbaAddress (
@param[in] Buffer Pointer to a caller allocated buffer
that will be
used to hold the data read
-
@retval EFI_SUCCESS The firmware volume was read
successfully and
contents are in Buffer
@retval EFI_BAD_BUFFER_SIZE Read attempted across a LBA boundary.
On output,
@@ -199,16 +210,16 @@ FvbGetLbaAddress (
@retval EFI_ACCESS_DENIED The firmware volume is in the
ReadDisabled state
@retval EFI_DEVICE_ERROR The block device is not functioning
correctly and
could not be read
- @retval EFI_INVALID_PARAMETER Instance not found, or NumBytes,
Buffer are NULL
+ @retval EFI_INVALID_PARAMETER FvbInstance, NumBytes, and/or Buffer
are NULL
**/
EFI_STATUS
FvbReadBlock (
- IN EFI_FVB_INSTANCE *FvbInstance,
- IN EFI_LBA Lba,
- IN UINTN BlockOffset,
- IN OUT UINTN *NumBytes,
- IN UINT8 *Buffer
+ IN CONST EFI_FVB_INSTANCE *FvbInstance,
+ IN EFI_LBA Lba,
+ IN UINTN BlockOffset,
+ IN OUT UINTN *NumBytes,
+ IN UINT8 *Buffer
)
{
EFI_FVB_ATTRIBUTES_2 Attributes;
@@ -217,9 +228,10 @@ FvbReadBlock (
EFI_STATUS Status;
BOOLEAN BadBufferSize = FALSE;
- if ((NumBytes == NULL) || (Buffer == NULL)) {
+ if ((FvbInstance == NULL) || (NumBytes == NULL) || (Buffer == NULL))
+ {
return EFI_INVALID_PARAMETER;
}
+
if (*NumBytes == 0) {
return EFI_INVALID_PARAMETER;
}
@@ -269,6 +281,7 @@ FvbReadBlock (
of bytes actually written
@param[in] Buffer Pointer to a caller allocated buffer that
contains
the source for the write
+
@retval EFI_SUCCESS The firmware volume was written
successfully
@retval EFI_BAD_BUFFER_SIZE Write attempted across a LBA boundary. On
output,
NumBytes contains the total number of
bytes @@ -276,16 +289,16 @@ FvbReadBlock (
@retval EFI_ACCESS_DENIED The firmware volume is in the
WriteDisabled state
@retval EFI_DEVICE_ERROR The block device is not functioning
correctly and
could not be written
- @retval EFI_INVALID_PARAMETER Instance not found, or NumBytes, Buffer
are NULL
+ @retval EFI_INVALID_PARAMETER FvbInstance, NumBytes, and/or Buffer are
NULL
**/
EFI_STATUS
FvbWriteBlock (
- IN EFI_FVB_INSTANCE *FvbInstance,
- IN EFI_LBA Lba,
- IN UINTN BlockOffset,
- IN OUT UINTN *NumBytes,
- IN UINT8 *Buffer
+ IN CONST EFI_FVB_INSTANCE *FvbInstance,
+ IN EFI_LBA Lba,
+ IN UINTN BlockOffset,
+ IN OUT UINTN *NumBytes,
+ IN UINT8 *Buffer
)
{
EFI_FVB_ATTRIBUTES_2 Attributes;
@@ -294,7 +307,7 @@ FvbWriteBlock (
EFI_STATUS Status;
BOOLEAN BadBufferSize = FALSE;
- if ((NumBytes == NULL) || (Buffer == NULL)) {
+ if ((FvbInstance == NULL) || (NumBytes == NULL) || (Buffer == NULL))
+ {
return EFI_INVALID_PARAMETER;
}
if (*NumBytes == 0) {
@@ -350,8 +363,6 @@ FvbWriteBlock (
}
}
-
-
/**
Erases and initializes a firmware volume block.
@@ -363,13 +374,13 @@ FvbWriteBlock (
@retval EFI_DEVICE_ERROR The block device is not functioning
correctly and
could not be written. Firmware device may
have been
partially erased
- @retval EFI_INVALID_PARAMETER Instance not found
+ @retval EFI_INVALID_PARAMETER FvbInstance is NULL
**/
EFI_STATUS
FvbEraseBlock (
- IN EFI_FVB_INSTANCE *FvbInstance,
- IN EFI_LBA Lba
+ IN CONST EFI_FVB_INSTANCE *FvbInstance,
+ IN EFI_LBA Lba
)
{
@@ -378,6 +389,10 @@ FvbEraseBlock (
UINTN LbaLength;
EFI_STATUS Status;
+ if (FvbInstance == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
//
// Check if the FV is write enabled
//
@@ -422,9 +437,9 @@ FvbEraseBlock (
@retval EFI_SUCCESS Successfully returns
@retval EFI_ACCESS_DENIED The volume setting is locked and cannot be
modified
- @retval EFI_INVALID_PARAMETER Instance not found, or The attributes
requested are
- in conflict with the capabilities as
declared in the
- firmware volume header
+ @retval EFI_INVALID_PARAMETER FvbInstance or Attributes is NULL.
+ Or the attributes requested are in
conflict with the
+ capabilities as declared in the firmware
volume header.
**/
EFI_STATUS
@@ -439,6 +454,10 @@ FvbSetVolumeAttributes (
UINT32 Capabilities;
UINT32 OldStatus, NewStatus;
+ if ((FvbInstance == NULL) || (Attributes == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
AttribPtr = (EFI_FVB_ATTRIBUTES_2 *) &(FvbInstance->FvHeader.Attributes);
OldAttributes = *AttribPtr;
Capabilities = OldAttributes & EFI_FVB2_CAPABILITIES; @@ -554,6 +573,7 @@
GetVariableFvInfo (
ASSERT ((BaseAddress != NULL) && (Length != NULL));
return;
}
+
*BaseAddress = 0;
*Length = 0;
@@ -631,8 +651,9 @@ GetVariableFvInfo (
@param[in] FvHeader A pointer to a firmware volume header
- @retval TRUE The firmware volume is consistent
- @retval FALSE The firmware volume has corrupted.
+ @retval TRUE The firmware volume is consistent.
+ @retval FALSE The firmware volume has corrupted or an invalid
firmware
+ volume was provided.
**/
BOOLEAN
@@ -644,6 +665,11 @@ IsFvHeaderValid (
EFI_PHYSICAL_ADDRESS NvStorageFvBaseAddress;
UINT32 NvStorageSize;
+ if (FvHeader == NULL) {
+ ASSERT (FvHeader != NULL);
+ return FALSE;
+ }
+
GetVariableFvInfo (&NvStorageFvBaseAddress, &NvStorageSize);
if (FvBase == NvStorageFvBaseAddress) { @@ -679,18 +705,23 @@
IsFvHeaderValid (
@param[in] This A pointer to EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL.
@param[out] Address Output buffer containing the address.
- retval EFI_SUCCESS The function always return successfully.
+ @retval EFI_SUCCESS The function always return successfully.
+ @retval EFI_INVALID_PARAMETER A pointer argument provided is NULL.
**/
EFI_STATUS
EFIAPI
FvbProtocolGetPhysicalAddress (
- IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
- OUT EFI_PHYSICAL_ADDRESS *Address
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ OUT EFI_PHYSICAL_ADDRESS *Address
)
{
EFI_FVB_INSTANCE *FvbInstance;
+ if ((This == NULL) || (Address == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
FvbInstance = FVB_INSTANCE_FROM_THIS (This);
*Address = FvbInstance->FvBase;
@@ -710,28 +741,34 @@ FvbProtocolGetPhysicalAddress (
returned. All blocks in this range have a size of
BlockSize
- @retval EFI_SUCCESS The function always return successfully.
+ @retval EFI_SUCCESS The function always return successfully.
+ @retval EFI_INVALID_PARAMETER A pointer argument provided is NULL.
**/
EFI_STATUS
EFIAPI
FvbProtocolGetBlockSize (
- IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
- IN EFI_LBA Lba,
- OUT UINTN *BlockSize,
- OUT UINTN *NumOfBlocks
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ IN EFI_LBA Lba,
+ OUT UINTN *BlockSize,
+ OUT UINTN *NumOfBlocks
)
{
EFI_FVB_INSTANCE *FvbInstance;
+ if ((This == NULL) || (BlockSize == NULL) || (NumOfBlocks == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
FvbInstance = FVB_INSTANCE_FROM_THIS (This);
- DEBUG((DEBUG_INFO,
+ DEBUG ((
+ DEBUG_INFO,
"FvbProtocolGetBlockSize: Lba: 0x%lx BlockSize: 0x%x NumOfBlocks: 0x%x\n",
Lba,
- BlockSize,
- NumOfBlocks)
- );
+ *BlockSize,
+ *NumOfBlocks
+ ));
return FvbGetLbaAddress (
FvbInstance,
@@ -748,27 +785,33 @@ FvbProtocolGetBlockSize (
@param[in] This Calling context
@param[out] Attributes Output buffer which contains attributes
- @retval EFI_SUCCESS The function always return successfully.
+ @retval EFI_SUCCESS The function always return
successfully.
+ @retval EFI_INVALID_PARAMETER A pointer argument provided is NULL.
**/
EFI_STATUS
EFIAPI
FvbProtocolGetAttributes (
- IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
- OUT EFI_FVB_ATTRIBUTES_2 *Attributes
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ OUT EFI_FVB_ATTRIBUTES_2 *Attributes
)
{
EFI_FVB_INSTANCE *FvbInstance;
+ if ((This == NULL) || (Attributes == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
FvbInstance = FVB_INSTANCE_FROM_THIS (This);
*Attributes = FvbGetVolumeAttributes (FvbInstance);
- DEBUG ((DEBUG_INFO,
+ DEBUG ((
+ DEBUG_INFO,
"FvbProtocolGetAttributes: This: 0x%x Attributes: 0x%x\n",
This,
- *Attributes)
- );
+ *Attributes
+ ));
return EFI_SUCCESS;
}
@@ -785,28 +828,34 @@ FvbProtocolGetAttributes ( EFI_STATUS EFIAPI
FvbProtocolSetAttributes (
- IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
- IN OUT EFI_FVB_ATTRIBUTES_2 *Attributes
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ IN OUT EFI_FVB_ATTRIBUTES_2 *Attributes
)
{
EFI_STATUS Status;
EFI_FVB_INSTANCE *FvbInstance;
- DEBUG((DEBUG_INFO,
+ if ((This == NULL) || (Attributes == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
"FvbProtocolSetAttributes: Before SET - This: 0x%x Attributes: 0x%x\n",
This,
- *Attributes)
- );
+ *Attributes
+ ));
FvbInstance = FVB_INSTANCE_FROM_THIS (This);
Status = FvbSetVolumeAttributes (FvbInstance, Attributes);
- DEBUG((DEBUG_INFO,
+ DEBUG ((
+ DEBUG_INFO,
"FvbProtocolSetAttributes: After SET - This: 0x%x Attributes: 0x%x\n",
This,
- *Attributes)
- );
+ *Attributes
+ ));
return Status;
}
@@ -823,17 +872,18 @@ FvbProtocolSetAttributes (
@param[in] ... Starting LBA followed by Number of Lba to erase.
a -1 to terminate the list.
- @retval EFI_SUCCESS The erase request was successfully completed
- @retval EFI_ACCESS_DENIED The firmware volume is in the WriteDisabled state
- @retval EFI_DEVICE_ERROR The block device is not functioning correctly and
- could not be written. Firmware device may have been
- partially erased
+ @retval EFI_SUCCESS The erase request was successfully completed
+ @retval EFI_INVALID_PARAMETER A pointer argument provided is NULL.
+ @retval EFI_ACCESS_DENIED The firmware volume is in the WriteDisabled
state
+ @retval EFI_DEVICE_ERROR The block device is not functioning
correctly and
+ could not be written. Firmware device may
have been
+ partially erased
**/
EFI_STATUS
EFIAPI
FvbProtocolEraseBlocks (
- IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
...
)
{
@@ -846,6 +896,10 @@ FvbProtocolEraseBlocks (
DEBUG((DEBUG_INFO, "FvbProtocolEraseBlocks: \n"));
+ if (This == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
FvbInstance = FVB_INSTANCE_FROM_THIS (This);
NumOfBlocks = FvbInstance->NumOfBlocks; @@ -923,30 +977,35 @@
FvbProtocolEraseBlocks (
@retval EFI_ACCESS_DENIED The firmware volume is in the WriteDisabled
state
@retval EFI_DEVICE_ERROR The block device is not functioning correctly
and
could not be written
- @retval EFI_INVALID_PARAMETER NumBytes or Buffer are NULL
+ @retval EFI_INVALID_PARAMETER A pointer argument provided is NULL.
**/
EFI_STATUS
EFIAPI
FvbProtocolWrite (
- IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
- IN EFI_LBA Lba,
- IN UINTN Offset,
- IN OUT UINTN *NumBytes,
- IN UINT8 *Buffer
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ IN EFI_LBA Lba,
+ IN UINTN Offset,
+ IN OUT UINTN *NumBytes,
+ IN UINT8 *Buffer
)
{
EFI_FVB_INSTANCE *FvbInstance;
+ if ((This == NULL) || (NumBytes == NULL) || (Buffer == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
FvbInstance = FVB_INSTANCE_FROM_THIS (This);
- DEBUG((DEBUG_INFO,
+ DEBUG ((
+ DEBUG_INFO,
"FvbProtocolWrite: Lba: 0x%lx Offset: 0x%x NumBytes: 0x%x, Buffer: 0x%x\n",
Lba,
Offset,
*NumBytes,
- Buffer)
- );
+ Buffer
+ ));
return FvbWriteBlock (FvbInstance, Lba, Offset, NumBytes, Buffer); } @@
-974,31 +1033,36 @@ FvbProtocolWrite (
@retval EFI_ACCESS_DENIED The firmware volume is in the ReadDisabled
state
@retval EFI_DEVICE_ERROR The block device is not functioning correctly
and
could not be read
- @retval EFI_INVALID_PARAMETER NumBytes or Buffer are NULL
+ @retval EFI_INVALID_PARAMETER A pointer argument provided is NULL.
**/
EFI_STATUS
EFIAPI
FvbProtocolRead (
- IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
- IN EFI_LBA Lba,
- IN UINTN Offset,
- IN OUT UINTN *NumBytes,
- OUT UINT8 *Buffer
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *This,
+ IN EFI_LBA Lba,
+ IN UINTN Offset,
+ IN OUT UINTN *NumBytes,
+ OUT UINT8 *Buffer
)
{
EFI_FVB_INSTANCE *FvbInstance;
EFI_STATUS Status;
+ if ((This == NULL) || (NumBytes == NULL) || (Buffer == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
FvbInstance = FVB_INSTANCE_FROM_THIS (This);
Status = FvbReadBlock (FvbInstance, Lba, Offset, NumBytes, Buffer);
- DEBUG((DEBUG_INFO,
+ DEBUG ((
+ DEBUG_INFO,
"FvbProtocolRead: Lba: 0x%lx Offset: 0x%x NumBytes: 0x%x, Buffer: 0x%x\n",
Lba,
Offset,
*NumBytes,
- Buffer)
- );
+ Buffer
+ ));
return Status;
}
diff --git
a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c
b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c
index f9543c5ba677..5d7591480319 100644
---
a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSe
+++ rviceMm.c
@@ -15,14 +15,11 @@
#include <Guid/VariableFormat.h>
/**
- The function installs EFI_FIRMWARE_VOLUME_BLOCK protocol
- for each FV in the system.
+ The function installs the given EFI_FIRMWARE_VOLUME_BLOCK protocol instance.
@param[in] FvbInstance The pointer to a FW volume instance structure,
which contains the information about one FV.
- @retval VOID
-
**/
VOID
InstallFvbProtocol (
@@ -33,8 +30,8 @@ InstallFvbProtocol (
EFI_STATUS Status;
EFI_HANDLE FvbHandle;
- ASSERT (FvbInstance != NULL);
if (FvbInstance == NULL) {
+ ASSERT (FvbInstance != NULL);
return;
}
@@ -48,14 +45,14 @@ InstallFvbProtocol (
//
// Set up the devicepath
//
- DEBUG ((DEBUG_INFO, "FwBlockService.c: Setting up DevicePath for 0x%lx:\n",
FvbInstance->FvBase));
+ DEBUG ((DEBUG_INFO, "SpiFvbServiceSmm.c: Setting up DevicePath for
+ 0x%lx:\n", FvbInstance->FvBase));
if (FvHeader->ExtHeaderOffset == 0) {
//
// FV does not contains extension header, then produce MEMMAP_DEVICE_PATH
//
FvbInstance->DevicePath = (EFI_DEVICE_PATH_PROTOCOL *)
AllocateRuntimeCopyPool (sizeof (FV_MEMMAP_DEVICE_PATH),
&mFvMemmapDevicePathTemplate);
if (FvbInstance->DevicePath == NULL) {
- DEBUG ((DEBUG_INFO, "SpiFvbServiceSmm.c: Memory allocation for
MEMMAP_DEVICE_PATH failed\n"));
+ DEBUG ((DEBUG_ERROR, "SpiFvbServiceSmm.c: Memory allocation for
+ MEMMAP_DEVICE_PATH failed\n"));
return;
}
((FV_MEMMAP_DEVICE_PATH *)
FvbInstance->DevicePath)->MemMapDevPath.StartingAddress = FvbInstance->FvBase;
@@ -63,7 +60,7 @@ InstallFvbProtocol (
} else {
FvbInstance->DevicePath = (EFI_DEVICE_PATH_PROTOCOL *)
AllocateRuntimeCopyPool (sizeof (FV_PIWG_DEVICE_PATH),
&mFvPIWGDevicePathTemplate);
if (FvbInstance->DevicePath == NULL) {
- DEBUG ((DEBUG_INFO, "SpiFvbServiceSmm.c: Memory allocation for
FV_PIWG_DEVICE_PATH failed\n"));
+ DEBUG ((DEBUG_ERROR, "SpiFvbServiceSmm.c: Memory allocation for
+ FV_PIWG_DEVICE_PATH failed\n"));
return;
}
CopyGuid (
@@ -96,7 +93,7 @@ InstallFvbProtocol (
/**
The function does the necessary initialization work for
- Firmware Volume Block Driver.
+ the Firmware Volume Block Driver.
**/
VOID
@@ -148,10 +145,6 @@ FvbInitialize (
mPlatformFvBaseAddress[1].FvBase = PcdGet32(PcdFlashMicrocodeFvBase);
mPlatformFvBaseAddress[1].FvSize = PcdGet32(PcdFlashMicrocodeFvSize);
- //
- // We will only continue with FVB installation if the
- // SPI is the active BIOS state
- //
{
//
// Make sure all FVB are valid and/or fix if possible @@ -167,11 +160,20
@@ FvbInitialize (
if (!IsFvHeaderValid (BaseAddress, FvHeader)) {
BytesWritten = 0;
BytesErased = 0;
- DEBUG ((DEBUG_ERROR, "ERROR - The FV in 0x%x is invalid!\n",
FvHeader));
+ DEBUG ((DEBUG_ERROR, "ERROR - The FV at 0x%x is invalid!\n",
+ FvHeader));
+
+ //
+ // Attempt to recover the FV
+ //
FvHeader = NULL;
- Status = GetFvbInfo (BaseAddress, &FvHeader);
+ Status = GetGeneratedFvByAddress (BaseAddress, &FvHeader);
if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_WARN, "ERROR - Can't recovery FV header at 0x%x.
GetFvbInfo Status %r\n", BaseAddress, Status));
+ DEBUG ((
+ DEBUG_WARN | DEBUG_ERROR,
+ "ERROR - Can't recover FV header at 0x%x. GetGeneratedFvByAddress
Status %r\n",
+ BaseAddress,
+ Status
+ ));
continue;
}
DEBUG ((DEBUG_INFO, "Rewriting FV header at 0x%X with static data\n",
BaseAddress)); @@ -181,15 +183,15 @@ FvbInitialize (
BytesErased = (UINTN) FvHeader->BlockMap->Length;
Status = SpiFlashBlockErase( (UINTN) BaseAddress, &BytesErased);
if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_WARN, "ERROR - SpiFlashBlockErase Error %r\n",
Status));
+ DEBUG ((DEBUG_WARN | DEBUG_ERROR, "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));
+ DEBUG ((DEBUG_WARN | DEBUG_ERROR, "ERROR - BytesErased !=
FvHeader->BlockMap->Length\n"));
+ DEBUG ((DEBUG_ERROR, " BytesErased = 0x%X\n Length = 0x%X\n",
+ BytesErased, FvHeader->BlockMap->Length));
if (FvHeader != NULL) {
FreePool (FvHeader);
}
@@ -249,15 +251,15 @@ FvbInitialize (
}
if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_WARN, "ERROR - SpiFlashWrite Error %r\n", Status));
+ DEBUG ((DEBUG_WARN | DEBUG_ERROR, "ERROR - SpiFlashWrite
+ Error %r\n", Status));
if (FvHeader != NULL) {
FreePool (FvHeader);
}
continue;
}
if (BytesWritten != ExpectedBytesWritten) {
- DEBUG ((DEBUG_WARN, "ERROR - BytesWritten !=
ExpectedBytesWritten\n"));
- DEBUG ((DEBUG_INFO, " BytesWritten = 0x%X\n ExpectedBytesWritten =
0x%X\n", BytesWritten, ExpectedBytesWritten));
+ DEBUG ((DEBUG_WARN | DEBUG_ERROR, "ERROR - BytesWritten !=
ExpectedBytesWritten\n"));
+ DEBUG ((DEBUG_ERROR, " BytesWritten = 0x%X\n
+ ExpectedBytesWritten = 0x%X\n", BytesWritten, ExpectedBytesWritten));
if (FvHeader != NULL) {
FreePool (FvHeader);
}
@@ -265,13 +267,13 @@ FvbInitialize (
}
Status = SpiFlashLock ();
if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_WARN, "ERROR - SpiFlashLock Error %r\n", Status));
+ DEBUG ((DEBUG_WARN | DEBUG_ERROR, "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));
+ DEBUG ((DEBUG_ERROR, "FV Header @ 0x%X restored with static
+ data\n", BaseAddress));
//
// Clear cache for this range.
//
@@ -294,7 +296,7 @@ FvbInitialize (
FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) BaseAddress;
if (!IsFvHeaderValid (BaseAddress, FvHeader)) {
- DEBUG ((DEBUG_WARN, "ERROR - The FV in 0x%x is invalid!\n", FvHeader));
+ DEBUG ((DEBUG_WARN | DEBUG_ERROR, "ERROR - The FV in 0x%x is
+ invalid!\n", FvHeader));
continue;
}
@@ -322,7 +324,7 @@ FvbInitialize (
FvHeader = (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) BaseAddress;
if (!IsFvHeaderValid (BaseAddress, FvHeader)) {
- DEBUG ((DEBUG_WARN, "ERROR - The FV in 0x%x is invalid!\n", FvHeader));
+ DEBUG ((DEBUG_WARN | DEBUG_ERROR, "ERROR - The FV in 0x%x is
+ invalid!\n", FvHeader));
continue;
}
diff --git
a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.h
b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.h
index 7b0908b03fdc..7664670457a2 100644
---
a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceCommon.h
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSe
+++ rviceCommon.h
@@ -1,14 +1,14 @@
/** @file
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
+ Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) Microsoft Corporation.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
**/
-#ifndef _SPI_FVB_SERVICE_COMMON_H
-#define _SPI_FVB_SERVICE_COMMON_H
+#ifndef SPI_FVB_SERVICE_COMMON_H_
+#define SPI_FVB_SERVICE_COMMON_H_
#include <Guid/EventGroup.h>
#include <Guid/FirmwareFileSystem2.h>
@@ -61,7 +61,7 @@ typedef struct {
} FVB_GLOBAL;
//
-// Fvb Protocol instance data
+// Firmware Volume Block (FVB) Protocol instance data
//
#define FVB_INSTANCE_FROM_THIS(a) CR(a, EFI_FVB_INSTANCE, FvbProtocol,
FVB_INSTANCE_SIGNATURE)
@@ -81,7 +81,7 @@ typedef struct {
} FV_INFO;
//
-// Protocol APIs
+// Firmware Volume Block (FVB) Protocol APIs
//
EFI_STATUS
EFIAPI
@@ -146,8 +146,23 @@ IsFvHeaderValid (
IN CONST EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader
);
+//
+// Module Local Functions
+//
+
+/**
+ Returns an empty firmware volume for the firmware volume at the given base
address.
+
+ @param[in] FvBaseAddress The base address of the firmware volume
requested.
+ @param[out] FvbInfo A pointer that will be set to a buffer for
the firmware volume header
+ at the given base address.
+
+ @retval EFI_SUCCESS The firmware volume was returned
successfully.
+ @retval EFI_NOT_FOUND The firmware volume was not found for the
given base address.
+
+**/
EFI_STATUS
-GetFvbInfo (
+GetGeneratedFvByAddress (
IN EFI_PHYSICAL_ADDRESS FvBaseAddress,
OUT EFI_FIRMWARE_VOLUME_HEADER **FvbInfo
);
diff --git
a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.h
b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.h
index 36af1130c8ee..512844197a4d 100644
---
a/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbServiceMm.h
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/Flash/SpiFvbService/SpiFvbSe
+++ rviceMm.h
@@ -6,12 +6,12 @@
**/
-#ifndef _SPI_FVB_SERVICE_MM_H_
-#define _SPI_FVB_SERVICE_MM_H_
+#ifndef SPI_FVB_SERVICE_MM_H_
+#define SPI_FVB_SERVICE_MM_H_
/**
The function does the necessary initialization work for
- Firmware Volume Block Driver.
+ the Firmware Volume Block Driver.
**/
VOID
--
2.41.0.windows.1
-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106553): https://edk2.groups.io/g/devel/message/106553
Mute This Topic: https://groups.io/mt/99866031/6226280
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [ashraf.al...@intel.com]
-=-=-=-=-=-=
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106578): https://edk2.groups.io/g/devel/message/106578
Mute This Topic: https://groups.io/mt/99866031/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
