MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c | 394 +++++++++++++------- 1 file changed, 257 insertions(+), 137 deletions(-)
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1525
The patch is to merge multiple FMP instances into single ESRT entry
when they have the same GUID.
The policy to LastAttemptStatus/LastAttemptVersion of ESRT entry is:
If all the LastAttemptStatus are LAST_ATTEMPT_STATUS_SUCCESS, then
LastAttemptVersion should be the smallest of LastAttemptVersion. If
any of the LastAttemptStatus is not LAST_ATTEMPT_STATUS_SUCCESS,
then the LastAttemptVersion/LastAttemptStatus should be the values
of the first FMP instance whose LastAttemptStatus is not
LAST_ATTEMPT_STATUS_SUCCESS.
To detect possible duplicated GUID/HardwareInstance, a table of
GUID/HardwareInstance pairs from all the EFI_FIRMWARE_IMAGE_DESCRIPTORs
from all FMP instances is built. If a duplicate is found, then generate
a DEBUG_ERROR message, generate an ASSERT(), and ignore the duplicate
EFI_FIRMWARE_IMAGE_DESCRIPTOR.
Add an internal worker function called FmpGetFirmwareImageDescriptor()
that retrieves the list of EFI_FIRMWARE_IMAGE_DESCRIPTORs from a single
FMP instance and returns the descriptors in an allocated buffer. This
function is used to get the descriptors used to build the table of
unique GUID/HardwareInstance pairs. It is then used again to generate
the ESRT Table from all the EFI_FIRMWARE_IMAGE_DESCRIPTORs from all the
FMP instances. 2 passes are performed so the total number of
descriptors is known. This allows the correct sized buffers to always
be allocated.
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Eric Jin <eric.jin@intel.com>
---
MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c | 394 +++++++++++++-------
1 file changed, 257 insertions(+), 137 deletions(-)
diff --git a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
index 2356da662e..4670349f89 100644
--- a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
+++ b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
@@ -2,7 +2,7 @@
Publishes ESRT table from Firmware Management Protocol instances
Copyright (c) 2016, Microsoft Corporation
- Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -21,6 +21,22 @@
#include <Guid/EventGroup.h>
#include <Guid/SystemResourceTable.h>
+///
+/// Structure for array of unique GUID/HardwareInstance pairs from the
+/// current set of EFI_FIRMWARE_IMAGE_DESCRIPTORs from all FMP Protocols.
+///
+typedef struct {
+ ///
+ /// A unique GUID identifying the firmware image type.
+ ///
+ EFI_GUID ImageTypeGuid;
+ ///
+ /// An optional number to identify the unique hardware instance within the
+ /// system for devices that may have multiple instances whenever possible.
+ ///
+ UINT64 HardwareInstance;
+} GUID_HARDWAREINSTANCE_PAIR;
+
/**
Print ESRT to debug console.
@@ -33,11 +49,6 @@ PrintTable (
IN EFI_SYSTEM_RESOURCE_TABLE *Table
);
-//
-// Number of ESRT entries to grow by each time we run out of room
-//
-#define GROWTH_STEP 10
-
/**
Install EFI System Resource Table into the UEFI Configuration Table
@@ -101,90 +112,129 @@ IsSystemFmp (
}
/**
- Function to create a single ESRT Entry and add it to the ESRT
- given a FMP descriptor. If the guid is already in the ESRT it
- will be ignored. The ESRT will grow if it does not have enough room.
-
- @param[in, out] Table On input, pointer to the pointer to the ESRT.
- On output, same as input or pointer to the pointer
- to new enlarged ESRT.
- @param[in] FmpImageInfoBuf Pointer to the EFI_FIRMWARE_IMAGE_DESCRIPTOR.
- @param[in] FmpVersion FMP Version.
-
- @return Status code.
+ Function to create a single ESRT Entry and add it to the ESRT with
+ a given FMP descriptor. If the GUID is already in the ESRT, then the ESRT
+ entry is updated.
+
+ @param[in,out] Table Pointer to the ESRT Table.
+ @param[in,out] HardwareInstances Pointer to the GUID_HARDWAREINSTANCE_PAIR.
+ @param[in,out] NumberOfDescriptors The number of EFI_FIRMWARE_IMAGE_DESCRIPTORs.
+ @param[in] FmpImageInfoBuf Pointer to the EFI_FIRMWARE_IMAGE_DESCRIPTOR.
+ @param[in] FmpVersion FMP Version.
+
+ @retval EFI_SUCCESS FmpImageInfoBuf was use to fill in a new ESRT entry
+ in Table.
+ @retval EFI_SUCCESS The ImageTypeId GUID in FmpImageInfoBuf matches an
+ existing ESRT entry in Table, and the information
+ from FmpImageInfoBuf was merged into the the existing
+ ESRT entry.
+ @retval EFI_UNSPOORTED The GUID/HardareInstance in FmpImageInfoBuf has is a
+ duplicate.
**/
EFI_STATUS
CreateEsrtEntry (
- IN OUT EFI_SYSTEM_RESOURCE_TABLE **Table,
- IN EFI_FIRMWARE_IMAGE_DESCRIPTOR *FmpImageInfoBuf,
- IN UINT32 FmpVersion
+ IN OUT EFI_SYSTEM_RESOURCE_TABLE *Table,
+ IN OUT GUID_HARDWAREINSTANCE_PAIR *HardwareInstances,
+ IN OUT UINT32 *NumberOfDescriptors,
+ IN EFI_FIRMWARE_IMAGE_DESCRIPTOR *FmpImageInfoBuf,
+ IN UINT32 FmpVersion
)
{
UINTN Index;
EFI_SYSTEM_RESOURCE_ENTRY *Entry;
- UINTN NewSize;
- EFI_SYSTEM_RESOURCE_TABLE *NewTable;
+ UINT64 FmpHardwareInstance;
- Index = 0;
- Entry = NULL;
+ FmpHardwareInstance = 0;
+ if (FmpVersion >= 3) {
+ FmpHardwareInstance = FmpImageInfoBuf->HardwareInstance;
+ }
- Entry = (EFI_SYSTEM_RESOURCE_ENTRY *)((*Table) + 1);
//
- // Make sure Guid isn't already in the list
+ // Check to see of FmpImageInfoBuf GUID/HardwareInstance is unique
//
- for (Index = 0; Index < (*Table)->FwResourceCount; Index++) {
- if (CompareGuid (&Entry->FwClass, &FmpImageInfoBuf->ImageTypeId)) {
- DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: ESRT Entry already exists for FMP Instance with GUID %g\n", &Entry->FwClass));
- return EFI_INVALID_PARAMETER;
+ for (Index = 0; Index < *NumberOfDescriptors; Index++) {
+ if (CompareGuid (&HardwareInstances[Index].ImageTypeGuid, &FmpImageInfoBuf->ImageTypeId)) {
+ if (HardwareInstances[Index].HardwareInstance == FmpHardwareInstance) {
+ DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Duplicate firmware image descriptor with GUID %g HardwareInstance:0x%x\n", &FmpImageInfoBuf->ImageTypeId, FmpHardwareInstance));
+ ASSERT (
+ !CompareGuid (&HardwareInstances[Index].ImageTypeGuid, &FmpImageInfoBuf->ImageTypeId) ||
+ HardwareInstances[Index].HardwareInstance != FmpHardwareInstance
+ );
+ return EFI_UNSUPPORTED;
+ }
}
- Entry++;
}
//
- // Grow table if needed
+ // Record new GUID/HardwareInstance pair
//
- if ((*Table)->FwResourceCount >= (*Table)->FwResourceCountMax) {
- NewSize = (((*Table)->FwResourceCountMax + GROWTH_STEP) * sizeof (EFI_SYSTEM_RESOURCE_ENTRY)) + sizeof (EFI_SYSTEM_RESOURCE_TABLE);
- NewTable = AllocateZeroPool (NewSize);
- if (NewTable == NULL) {
- DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to allocate memory larger table for ESRT. \n"));
- return EFI_OUT_OF_RESOURCES;
+ CopyGuid (&HardwareInstances[*NumberOfDescriptors].ImageTypeGuid, &FmpImageInfoBuf->ImageTypeId);
+ HardwareInstances[*NumberOfDescriptors].HardwareInstance = FmpHardwareInstance;
+ *NumberOfDescriptors = *NumberOfDescriptors + 1;
+
+ DEBUG ((DEBUG_INFO, "EsrtFmpDxe: Add new image descriptor with GUID %g HardwareInstance:0x%x\n", &FmpImageInfoBuf->ImageTypeId, FmpHardwareInstance));
+
+ //
+ // Check to see if GUID is already in the ESRT table
+ //
+ Entry = (EFI_SYSTEM_RESOURCE_ENTRY *)(Table + 1);
+ for (Index = 0; Index < Table->FwResourceCount; Index++, Entry++) {
+ if (!CompareGuid (&Entry->FwClass, &FmpImageInfoBuf->ImageTypeId)) {
+ continue;
}
+ DEBUG ((DEBUG_INFO, "EsrtFmpDxe: ESRT Entry already exists for FMP Instance with GUID %g\n", &Entry->FwClass));
+
//
- // Copy the whole old table into new table buffer
- //
- CopyMem (
- NewTable,
- (*Table),
- (((*Table)->FwResourceCountMax) * sizeof (EFI_SYSTEM_RESOURCE_ENTRY)) + sizeof (EFI_SYSTEM_RESOURCE_TABLE)
- );
- //
- // Update max
+ // Set ESRT FwVersion to the smaller of the two values
//
- NewTable->FwResourceCountMax = NewTable->FwResourceCountMax + GROWTH_STEP;
+ Entry->FwVersion = MIN (FmpImageInfoBuf->Version, Entry->FwVersion);
+
//
- // Free old table
+ // VERSION 2 has Lowest Supported
//
- FreePool ((*Table));
+ if (FmpVersion >= 2) {
+ //
+ // Set ESRT LowestSupportedFwVersion to the smaller of the two values
+ //
+ Entry->LowestSupportedFwVersion =
+ MIN (
+ FmpImageInfoBuf->LowestSupportedImageVersion,
+ Entry->LowestSupportedFwVersion
+ );
+ }
+
//
- // Reassign pointer to new table.
+ // VERSION 3 supports last attempt values
//
- (*Table) = NewTable;
+ if (FmpVersion >= 3) {
+ //
+ // Update the ESRT entry with the last attempt status and last attempt
+ // version from the first FMP instance whose last attempt status is not
+ // SUCCESS. If all FMP instances are SUCCESS, then set version to the
+ // smallest value from all FMP instances.
+ //
+ if (Entry->LastAttemptStatus == LAST_ATTEMPT_STATUS_SUCCESS) {
+ if (FmpImageInfoBuf->LastAttemptStatus != LAST_ATTEMPT_STATUS_SUCCESS) {
+ Entry->LastAttemptStatus = FmpImageInfoBuf->LastAttemptStatus;
+ Entry->LastAttemptVersion = FmpImageInfoBuf->LastAttemptVersion;
+ } else {
+ Entry->LastAttemptVersion =
+ MIN (
+ FmpImageInfoBuf->LastAttemptVersion,
+ Entry->LastAttemptVersion
+ );
+ }
+ }
+ }
+
+ return EFI_SUCCESS;
}
//
- // ESRT table has enough room for the new entry so add new entry
- //
- Entry = (EFI_SYSTEM_RESOURCE_ENTRY *)(((UINT8 *)(*Table)) + sizeof (EFI_SYSTEM_RESOURCE_TABLE));
- //
- // Move to the location of new entry
- //
- Entry = Entry + (*Table)->FwResourceCount;
+ // Add a new ESRT Table Entry
//
- // Increment resource count
- //
- (*Table)->FwResourceCount++;
+ Entry = (EFI_SYSTEM_RESOURCE_ENTRY *)(Table + 1) + Table->FwResourceCount;
CopyGuid (&Entry->FwClass, &FmpImageInfoBuf->ImageTypeId);
@@ -195,11 +245,11 @@ CreateEsrtEntry (
Entry->FwType = (UINT32)(ESRT_FW_TYPE_DEVICEFIRMWARE);
}
- Entry->FwVersion = FmpImageInfoBuf->Version;
+ Entry->FwVersion = FmpImageInfoBuf->Version;
Entry->LowestSupportedFwVersion = 0;
- Entry->CapsuleFlags = 0;
- Entry->LastAttemptVersion = 0;
- Entry->LastAttemptStatus = 0;
+ Entry->CapsuleFlags = 0;
+ Entry->LastAttemptVersion = 0;
+ Entry->LastAttemptStatus = 0;
//
// VERSION 2 has Lowest Supported
@@ -213,12 +263,93 @@ CreateEsrtEntry (
//
if (FmpVersion >= 3) {
Entry->LastAttemptVersion = FmpImageInfoBuf->LastAttemptVersion;
- Entry->LastAttemptStatus = FmpImageInfoBuf->LastAttemptStatus;
+ Entry->LastAttemptStatus = FmpImageInfoBuf->LastAttemptStatus;
}
+ //
+ // Increment the number of active ESRT Table Entries
+ //
+ Table->FwResourceCount++;
+
return EFI_SUCCESS;
}
+/**
+ Function to retrieve the EFI_FIRMWARE_IMAGE_DESCRIPTOR from an FMP Instance.
+ The returned buffer is allocated using AllocatePool() and must be freed by the
+ caller using FreePool().
+
+ @param[in] Fmp Pointer to an EFI_FIRMWARE_MANAGEMENT_PROTOCOL.
+ @param[out] FmpImageInfoDescriptorVer Pointer to the version number associated
+ with the returned EFI_FIRMWARE_IMAGE_DESCRIPTOR.
+ @param[out] FmpImageInfoCount Pointer to the number of the returned
+ EFI_FIRMWARE_IMAGE_DESCRIPTORs.
+ @param[out] DescriptorSize Pointer to the size, in bytes, of each
+ returned EFI_FIRMWARE_IMAGE_DESCRIPTOR.
+
+ @return Pointer to the retrieved EFI_FIRMWARE_IMAGE_DESCRIPTOR. If the
+ descriptor can not be retrieved, then NULL is returned.
+
+**/
+EFI_FIRMWARE_IMAGE_DESCRIPTOR *
+FmpGetFirmwareImageDescriptor (
+ IN EFI_FIRMWARE_MANAGEMENT_PROTOCOL *Fmp,
+ OUT UINT32 *FmpImageInfoDescriptorVer,
+ OUT UINT8 *FmpImageInfoCount,
+ OUT UINTN *DescriptorSize
+ )
+{
+ EFI_STATUS Status;
+ UINTN ImageInfoSize;
+ UINT32 PackageVersion;
+ CHAR16 *PackageVersionName;
+ EFI_FIRMWARE_IMAGE_DESCRIPTOR *FmpImageInfoBuf;
+
+ ImageInfoSize = 0;
+ Status = Fmp->GetImageInfo (
+ Fmp, // FMP Pointer
+ &ImageInfoSize, // Buffer Size (in this case 0)
+ NULL, // NULL so we can get size
+ FmpImageInfoDescriptorVer, // DescriptorVersion
+ FmpImageInfoCount, // DescriptorCount
+ DescriptorSize, // DescriptorSize
+ &PackageVersion, // PackageVersion
+ &PackageVersionName // PackageVersionName
+ );
+ if (Status != EFI_BUFFER_TOO_SMALL) {
+ DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Unexpected Failure in GetImageInfo. Status = %r\n", Status));
+ return NULL;
+ }
+
+ FmpImageInfoBuf = AllocateZeroPool (ImageInfoSize);
+ if (FmpImageInfoBuf == NULL) {
+ DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to get memory for FMP descriptor.\n"));
+ return NULL;
+ }
+
+ PackageVersionName = NULL;
+ Status = Fmp->GetImageInfo (
+ Fmp, // FMP Pointer
+ &ImageInfoSize, // ImageInfoSize
+ FmpImageInfoBuf, // ImageInfo
+ FmpImageInfoDescriptorVer, // DescriptorVersion
+ FmpImageInfoCount, // DescriptorCount
+ DescriptorSize, // DescriptorSize
+ &PackageVersion, // PackageVersion
+ &PackageVersionName // PackageVersionName
+ );
+ if (PackageVersionName != NULL) {
+ FreePool (PackageVersionName);
+ }
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failure in GetImageInfo. Status = %r\n", Status));
+ FreePool (FmpImageInfoBuf);
+ return NULL;
+ }
+
+ return FmpImageInfoBuf;
+}
+
/**
Function to create ESRT based on FMP Instances.
Create ESRT table, get the descriptors from FMP Instance and
@@ -232,29 +363,26 @@ CreateFmpBasedEsrt (
VOID
)
{
- EFI_STATUS Status;
- EFI_SYSTEM_RESOURCE_TABLE *Table;
- UINTN NoProtocols;
- VOID **Buffer;
- UINTN Index;
- EFI_FIRMWARE_MANAGEMENT_PROTOCOL *Fmp;
- UINTN DescriptorSize;
- EFI_FIRMWARE_IMAGE_DESCRIPTOR *FmpImageInfoBuf;
- EFI_FIRMWARE_IMAGE_DESCRIPTOR *FmpImageInfoBufOrg;
- UINT8 FmpImageInfoCount;
- UINT32 FmpImageInfoDescriptorVer;
- UINTN ImageInfoSize;
- UINT32 PackageVersion;
- CHAR16 *PackageVersionName;
+ EFI_STATUS Status;
+ UINTN NoProtocols;
+ VOID **Buffer;
+ UINTN Index;
+ UINT32 FmpImageInfoDescriptorVer;
+ UINT8 FmpImageInfoCount;
+ UINTN DescriptorSize;
+ UINT32 NumberOfDescriptors;
+ EFI_FIRMWARE_IMAGE_DESCRIPTOR *FmpImageInfoBuf;
+ EFI_FIRMWARE_IMAGE_DESCRIPTOR *OrgFmpImageInfoBuf;
+ EFI_SYSTEM_RESOURCE_TABLE *Table;
+ GUID_HARDWAREINSTANCE_PAIR *HardwareInstances;
Status = EFI_SUCCESS;
- Table = NULL;
NoProtocols = 0;
Buffer = NULL;
- PackageVersionName = NULL;
FmpImageInfoBuf = NULL;
- FmpImageInfoBufOrg = NULL;
- Fmp = NULL;
+ OrgFmpImageInfoBuf = NULL;
+ Table = NULL;
+ HardwareInstances = NULL;
Status = EfiLocateProtocolBuffer (
&gEfiFirmwareManagementProtocolGuid,
@@ -266,69 +394,64 @@ CreateFmpBasedEsrt (
}
//
- // Allocate Memory for table
+ // Count the total number of EFI_FIRMWARE_IMAGE_DESCRIPTORs
+ //
+ for (Index = 0, NumberOfDescriptors = 0; Index < NoProtocols; Index++) {
+ FmpImageInfoBuf = FmpGetFirmwareImageDescriptor (
+ (EFI_FIRMWARE_MANAGEMENT_PROTOCOL *) Buffer[Index],
+ &FmpImageInfoDescriptorVer,
+ &FmpImageInfoCount,
+ &DescriptorSize
+ );
+ if (FmpImageInfoBuf != NULL) {
+ NumberOfDescriptors += FmpImageInfoCount;
+ FreePool (FmpImageInfoBuf);
+ }
+ }
+
+ //
+ // Allocate ESRT Table and GUID/HardwareInstance table
//
Table = AllocateZeroPool (
- (GROWTH_STEP * sizeof (EFI_SYSTEM_RESOURCE_ENTRY)) + sizeof (EFI_SYSTEM_RESOURCE_TABLE)
+ (NumberOfDescriptors * sizeof (EFI_SYSTEM_RESOURCE_ENTRY)) + sizeof (EFI_SYSTEM_RESOURCE_TABLE)
);
if (Table == NULL) {
DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to allocate memory for ESRT.\n"));
- gBS->FreePool (Buffer);
+ FreePool (Buffer);
return NULL;
}
+ HardwareInstances = AllocateZeroPool (NumberOfDescriptors * sizeof (GUID_HARDWAREINSTANCE_PAIR));
+ if (HardwareInstances == NULL) {
+ DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to allocate memory for HW Instance Table.\n"));
+ FreePool (Table);
+ FreePool (Buffer);
+ return NULL;
+ }
+
+ //
+ // Initialize ESRT Table
+ //
Table->FwResourceCount = 0;
- Table->FwResourceCountMax = GROWTH_STEP;
+ Table->FwResourceCountMax = NumberOfDescriptors;
Table->FwResourceVersion = EFI_SYSTEM_RESOURCE_TABLE_FIRMWARE_RESOURCE_VERSION;
+ NumberOfDescriptors = 0;
for (Index = 0; Index < NoProtocols; Index++) {
- Fmp = (EFI_FIRMWARE_MANAGEMENT_PROTOCOL *) Buffer[Index];
-
- ImageInfoSize = 0;
- Status = Fmp->GetImageInfo (
- Fmp, // FMP Pointer
- &ImageInfoSize, // Buffer Size (in this case 0)
- NULL, // NULL so we can get size
- &FmpImageInfoDescriptorVer, // DescriptorVersion
- &FmpImageInfoCount, // DescriptorCount
- &DescriptorSize, // DescriptorSize
- &PackageVersion, // PackageVersion
- &PackageVersionName // PackageVersionName
- );
-
- if (Status != EFI_BUFFER_TOO_SMALL) {
- DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Unexpected Failure in GetImageInfo. Status = %r\n", Status));
- continue;
- }
-
- FmpImageInfoBuf = AllocateZeroPool (ImageInfoSize);
+ FmpImageInfoBuf = FmpGetFirmwareImageDescriptor (
+ (EFI_FIRMWARE_MANAGEMENT_PROTOCOL *) Buffer[Index],
+ &FmpImageInfoDescriptorVer,
+ &FmpImageInfoCount,
+ &DescriptorSize
+ );
if (FmpImageInfoBuf == NULL) {
- DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to get memory for descriptors.\n"));
- continue;
- }
-
- FmpImageInfoBufOrg = FmpImageInfoBuf;
- PackageVersionName = NULL;
- Status = Fmp->GetImageInfo (
- Fmp,
- &ImageInfoSize, // ImageInfoSize
- FmpImageInfoBuf, // ImageInfo
- &FmpImageInfoDescriptorVer, // DescriptorVersion
- &FmpImageInfoCount, // DescriptorCount
- &DescriptorSize, // DescriptorSize
- &PackageVersion, // PackageVersion
- &PackageVersionName // PackageVersionName
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failure in GetImageInfo. Status = %r\n", Status));
- FreePool (FmpImageInfoBufOrg);
- FmpImageInfoBufOrg = NULL;
continue;
}
//
// Check each descriptor and read from the one specified
//
+ OrgFmpImageInfoBuf = FmpImageInfoBuf;
while (FmpImageInfoCount > 0) {
//
// If the descriptor has the IN USE bit set, create ESRT entry otherwise ignore.
@@ -337,7 +460,7 @@ CreateFmpBasedEsrt (
//
// Create ESRT entry
//
- CreateEsrtEntry (&Table, FmpImageInfoBuf, FmpImageInfoDescriptorVer);
+ CreateEsrtEntry (Table, HardwareInstances, &NumberOfDescriptors, FmpImageInfoBuf, FmpImageInfoDescriptorVer);
}
FmpImageInfoCount--;
//
@@ -346,15 +469,12 @@ CreateFmpBasedEsrt (
FmpImageInfoBuf = (EFI_FIRMWARE_IMAGE_DESCRIPTOR *)(((UINT8 *)FmpImageInfoBuf) + DescriptorSize);
}
- if (PackageVersionName != NULL) {
- FreePool (PackageVersionName);
- PackageVersionName = NULL;
- }
- FreePool (FmpImageInfoBufOrg);
- FmpImageInfoBufOrg = NULL;
+ FreePool (OrgFmpImageInfoBuf);
+ OrgFmpImageInfoBuf = NULL;
}
- gBS->FreePool (Buffer);
+ FreePool (Buffer);
+ FreePool (HardwareInstances);
return Table;
}
--
2.20.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#44901): https://edk2.groups.io/g/devel/message/44901
Mute This Topic: https://groups.io/mt/32723459/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
> -----Original Message----- > From: Jin, Eric > Sent: Monday, August 05, 2019 4:03 PM > To: devel@edk2.groups.io > Cc: Sean Brogan; Bret Barkelew; Wang, Jian J; Wu, Hao A; Kinney, Michael D > Subject: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Enhance ESRT to support > multiple controllers > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1525 > > The patch is to merge multiple FMP instances into single ESRT entry > when they have the same GUID. > > The policy to LastAttemptStatus/LastAttemptVersion of ESRT entry is: > If all the LastAttemptStatus are LAST_ATTEMPT_STATUS_SUCCESS, then > LastAttemptVersion should be the smallest of LastAttemptVersion. If > any of the LastAttemptStatus is not LAST_ATTEMPT_STATUS_SUCCESS, > then the LastAttemptVersion/LastAttemptStatus should be the values > of the first FMP instance whose LastAttemptStatus is not > LAST_ATTEMPT_STATUS_SUCCESS. > > To detect possible duplicated GUID/HardwareInstance, a table of > GUID/HardwareInstance pairs from all the > EFI_FIRMWARE_IMAGE_DESCRIPTORs > from all FMP instances is built. If a duplicate is found, then generate > a DEBUG_ERROR message, generate an ASSERT(), and ignore the duplicate > EFI_FIRMWARE_IMAGE_DESCRIPTOR. > > Add an internal worker function called FmpGetFirmwareImageDescriptor() > that retrieves the list of EFI_FIRMWARE_IMAGE_DESCRIPTORs from a single > FMP instance and returns the descriptors in an allocated buffer. This > function is used to get the descriptors used to build the table of > unique GUID/HardwareInstance pairs. It is then used again to generate > the ESRT Table from all the EFI_FIRMWARE_IMAGE_DESCRIPTORs from all > the > FMP instances. 2 passes are performed so the total number of > descriptors is known. This allows the correct sized buffers to always > be allocated. The patch looks good to me. Reviewed-by: Hao A Wu <hao.a.wu@intel.com> I will wait a little longer to push the patch to see if there is additional feedbacks from stewards with regard to the approach when merging changes from the edk2-staging repo. Best Regards, Hao Wu > > Cc: Sean Brogan <sean.brogan@microsoft.com> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> > Cc: Jian J Wang <jian.j.wang@intel.com> > Cc: Hao A Wu <hao.a.wu@intel.com> > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com> > Signed-off-by: Eric Jin <eric.jin@intel.com> > --- > MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c | 394 +++++++++++++--- > ---- > 1 file changed, 257 insertions(+), 137 deletions(-) > > diff --git a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c > b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c > index 2356da662e..4670349f89 100644 > --- a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c > +++ b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c > @@ -2,7 +2,7 @@ > Publishes ESRT table from Firmware Management Protocol instances > > Copyright (c) 2016, Microsoft Corporation > - Copyright (c) 2018, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR> > > All rights reserved. > SPDX-License-Identifier: BSD-2-Clause-Patent > @@ -21,6 +21,22 @@ > #include <Guid/EventGroup.h> > #include <Guid/SystemResourceTable.h> > > +/// > +/// Structure for array of unique GUID/HardwareInstance pairs from the > +/// current set of EFI_FIRMWARE_IMAGE_DESCRIPTORs from all FMP > Protocols. > +/// > +typedef struct { > + /// > + /// A unique GUID identifying the firmware image type. > + /// > + EFI_GUID ImageTypeGuid; > + /// > + /// An optional number to identify the unique hardware instance within > the > + /// system for devices that may have multiple instances whenever > possible. > + /// > + UINT64 HardwareInstance; > +} GUID_HARDWAREINSTANCE_PAIR; > + > /** > Print ESRT to debug console. > > @@ -33,11 +49,6 @@ PrintTable ( > IN EFI_SYSTEM_RESOURCE_TABLE *Table > ); > > -// > -// Number of ESRT entries to grow by each time we run out of room > -// > -#define GROWTH_STEP 10 > - > /** > Install EFI System Resource Table into the UEFI Configuration Table > > @@ -101,90 +112,129 @@ IsSystemFmp ( > } > > /** > - Function to create a single ESRT Entry and add it to the ESRT > - given a FMP descriptor. If the guid is already in the ESRT it > - will be ignored. The ESRT will grow if it does not have enough room. > - > - @param[in, out] Table On input, pointer to the pointer to the ESRT. > - On output, same as input or pointer to the pointer > - to new enlarged ESRT. > - @param[in] FmpImageInfoBuf Pointer to the > EFI_FIRMWARE_IMAGE_DESCRIPTOR. > - @param[in] FmpVersion FMP Version. > - > - @return Status code. > + Function to create a single ESRT Entry and add it to the ESRT with > + a given FMP descriptor. If the GUID is already in the ESRT, then the ESRT > + entry is updated. > + > + @param[in,out] Table Pointer to the ESRT Table. > + @param[in,out] HardwareInstances Pointer to the > GUID_HARDWAREINSTANCE_PAIR. > + @param[in,out] NumberOfDescriptors The number of > EFI_FIRMWARE_IMAGE_DESCRIPTORs. > + @param[in] FmpImageInfoBuf Pointer to the > EFI_FIRMWARE_IMAGE_DESCRIPTOR. > + @param[in] FmpVersion FMP Version. > + > + @retval EFI_SUCCESS FmpImageInfoBuf was use to fill in a new ESRT > entry > + in Table. > + @retval EFI_SUCCESS The ImageTypeId GUID in FmpImageInfoBuf > matches an > + existing ESRT entry in Table, and the information > + from FmpImageInfoBuf was merged into the the existing > + ESRT entry. > + @retval EFI_UNSPOORTED The GUID/HardareInstance in > FmpImageInfoBuf has is a > + duplicate. > > **/ > EFI_STATUS > CreateEsrtEntry ( > - IN OUT EFI_SYSTEM_RESOURCE_TABLE **Table, > - IN EFI_FIRMWARE_IMAGE_DESCRIPTOR *FmpImageInfoBuf, > - IN UINT32 FmpVersion > + IN OUT EFI_SYSTEM_RESOURCE_TABLE *Table, > + IN OUT GUID_HARDWAREINSTANCE_PAIR *HardwareInstances, > + IN OUT UINT32 *NumberOfDescriptors, > + IN EFI_FIRMWARE_IMAGE_DESCRIPTOR *FmpImageInfoBuf, > + IN UINT32 FmpVersion > ) > { > UINTN Index; > EFI_SYSTEM_RESOURCE_ENTRY *Entry; > - UINTN NewSize; > - EFI_SYSTEM_RESOURCE_TABLE *NewTable; > + UINT64 FmpHardwareInstance; > > - Index = 0; > - Entry = NULL; > + FmpHardwareInstance = 0; > + if (FmpVersion >= 3) { > + FmpHardwareInstance = FmpImageInfoBuf->HardwareInstance; > + } > > - Entry = (EFI_SYSTEM_RESOURCE_ENTRY *)((*Table) + 1); > // > - // Make sure Guid isn't already in the list > + // Check to see of FmpImageInfoBuf GUID/HardwareInstance is unique > // > - for (Index = 0; Index < (*Table)->FwResourceCount; Index++) { > - if (CompareGuid (&Entry->FwClass, &FmpImageInfoBuf->ImageTypeId)) { > - DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: ESRT Entry already exists for FMP > Instance with GUID %g\n", &Entry->FwClass)); > - return EFI_INVALID_PARAMETER; > + for (Index = 0; Index < *NumberOfDescriptors; Index++) { > + if (CompareGuid (&HardwareInstances[Index].ImageTypeGuid, > &FmpImageInfoBuf->ImageTypeId)) { > + if (HardwareInstances[Index].HardwareInstance == > FmpHardwareInstance) { > + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Duplicate firmware image > descriptor with GUID %g HardwareInstance:0x%x\n", &FmpImageInfoBuf- > >ImageTypeId, FmpHardwareInstance)); > + ASSERT ( > + !CompareGuid (&HardwareInstances[Index].ImageTypeGuid, > &FmpImageInfoBuf->ImageTypeId) || > + HardwareInstances[Index].HardwareInstance != > FmpHardwareInstance > + ); > + return EFI_UNSUPPORTED; > + } > } > - Entry++; > } > > // > - // Grow table if needed > + // Record new GUID/HardwareInstance pair > // > - if ((*Table)->FwResourceCount >= (*Table)->FwResourceCountMax) { > - NewSize = (((*Table)->FwResourceCountMax + GROWTH_STEP) * sizeof > (EFI_SYSTEM_RESOURCE_ENTRY)) + sizeof (EFI_SYSTEM_RESOURCE_TABLE); > - NewTable = AllocateZeroPool (NewSize); > - if (NewTable == NULL) { > - DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to allocate memory larger > table for ESRT. \n")); > - return EFI_OUT_OF_RESOURCES; > + CopyGuid (&HardwareInstances[*NumberOfDescriptors].ImageTypeGuid, > &FmpImageInfoBuf->ImageTypeId); > + HardwareInstances[*NumberOfDescriptors].HardwareInstance = > FmpHardwareInstance; > + *NumberOfDescriptors = *NumberOfDescriptors + 1; > + > + DEBUG ((DEBUG_INFO, "EsrtFmpDxe: Add new image descriptor with > GUID %g HardwareInstance:0x%x\n", &FmpImageInfoBuf->ImageTypeId, > FmpHardwareInstance)); > + > + // > + // Check to see if GUID is already in the ESRT table > + // > + Entry = (EFI_SYSTEM_RESOURCE_ENTRY *)(Table + 1); > + for (Index = 0; Index < Table->FwResourceCount; Index++, Entry++) { > + if (!CompareGuid (&Entry->FwClass, &FmpImageInfoBuf->ImageTypeId)) > { > + continue; > } > + DEBUG ((DEBUG_INFO, "EsrtFmpDxe: ESRT Entry already exists for FMP > Instance with GUID %g\n", &Entry->FwClass)); > + > // > - // Copy the whole old table into new table buffer > - // > - CopyMem ( > - NewTable, > - (*Table), > - (((*Table)->FwResourceCountMax) * sizeof > (EFI_SYSTEM_RESOURCE_ENTRY)) + sizeof (EFI_SYSTEM_RESOURCE_TABLE) > - ); > - // > - // Update max > + // Set ESRT FwVersion to the smaller of the two values > // > - NewTable->FwResourceCountMax = NewTable->FwResourceCountMax + > GROWTH_STEP; > + Entry->FwVersion = MIN (FmpImageInfoBuf->Version, Entry- > >FwVersion); > + > // > - // Free old table > + // VERSION 2 has Lowest Supported > // > - FreePool ((*Table)); > + if (FmpVersion >= 2) { > + // > + // Set ESRT LowestSupportedFwVersion to the smaller of the two values > + // > + Entry->LowestSupportedFwVersion = > + MIN ( > + FmpImageInfoBuf->LowestSupportedImageVersion, > + Entry->LowestSupportedFwVersion > + ); > + } > + > // > - // Reassign pointer to new table. > + // VERSION 3 supports last attempt values > // > - (*Table) = NewTable; > + if (FmpVersion >= 3) { > + // > + // Update the ESRT entry with the last attempt status and last attempt > + // version from the first FMP instance whose last attempt status is not > + // SUCCESS. If all FMP instances are SUCCESS, then set version to the > + // smallest value from all FMP instances. > + // > + if (Entry->LastAttemptStatus == LAST_ATTEMPT_STATUS_SUCCESS) { > + if (FmpImageInfoBuf->LastAttemptStatus != > LAST_ATTEMPT_STATUS_SUCCESS) { > + Entry->LastAttemptStatus = FmpImageInfoBuf->LastAttemptStatus; > + Entry->LastAttemptVersion = FmpImageInfoBuf->LastAttemptVersion; > + } else { > + Entry->LastAttemptVersion = > + MIN ( > + FmpImageInfoBuf->LastAttemptVersion, > + Entry->LastAttemptVersion > + ); > + } > + } > + } > + > + return EFI_SUCCESS; > } > > // > - // ESRT table has enough room for the new entry so add new entry > - // > - Entry = (EFI_SYSTEM_RESOURCE_ENTRY *)(((UINT8 *)(*Table)) + sizeof > (EFI_SYSTEM_RESOURCE_TABLE)); > - // > - // Move to the location of new entry > - // > - Entry = Entry + (*Table)->FwResourceCount; > + // Add a new ESRT Table Entry > // > - // Increment resource count > - // > - (*Table)->FwResourceCount++; > + Entry = (EFI_SYSTEM_RESOURCE_ENTRY *)(Table + 1) + Table- > >FwResourceCount; > > CopyGuid (&Entry->FwClass, &FmpImageInfoBuf->ImageTypeId); > > @@ -195,11 +245,11 @@ CreateEsrtEntry ( > Entry->FwType = (UINT32)(ESRT_FW_TYPE_DEVICEFIRMWARE); > } > > - Entry->FwVersion = FmpImageInfoBuf->Version; > + Entry->FwVersion = FmpImageInfoBuf->Version; > Entry->LowestSupportedFwVersion = 0; > - Entry->CapsuleFlags = 0; > - Entry->LastAttemptVersion = 0; > - Entry->LastAttemptStatus = 0; > + Entry->CapsuleFlags = 0; > + Entry->LastAttemptVersion = 0; > + Entry->LastAttemptStatus = 0; > > // > // VERSION 2 has Lowest Supported > @@ -213,12 +263,93 @@ CreateEsrtEntry ( > // > if (FmpVersion >= 3) { > Entry->LastAttemptVersion = FmpImageInfoBuf->LastAttemptVersion; > - Entry->LastAttemptStatus = FmpImageInfoBuf->LastAttemptStatus; > + Entry->LastAttemptStatus = FmpImageInfoBuf->LastAttemptStatus; > } > > + // > + // Increment the number of active ESRT Table Entries > + // > + Table->FwResourceCount++; > + > return EFI_SUCCESS; > } > > +/** > + Function to retrieve the EFI_FIRMWARE_IMAGE_DESCRIPTOR from an > FMP Instance. > + The returned buffer is allocated using AllocatePool() and must be freed by > the > + caller using FreePool(). > + > + @param[in] Fmp Pointer to an > EFI_FIRMWARE_MANAGEMENT_PROTOCOL. > + @param[out] FmpImageInfoDescriptorVer Pointer to the version number > associated > + with the returned > EFI_FIRMWARE_IMAGE_DESCRIPTOR. > + @param[out] FmpImageInfoCount Pointer to the number of the > returned > + EFI_FIRMWARE_IMAGE_DESCRIPTORs. > + @param[out] DescriptorSize Pointer to the size, in bytes, of each > + returned EFI_FIRMWARE_IMAGE_DESCRIPTOR. > + > + @return Pointer to the retrieved EFI_FIRMWARE_IMAGE_DESCRIPTOR. If > the > + descriptor can not be retrieved, then NULL is returned. > + > +**/ > +EFI_FIRMWARE_IMAGE_DESCRIPTOR * > +FmpGetFirmwareImageDescriptor ( > + IN EFI_FIRMWARE_MANAGEMENT_PROTOCOL *Fmp, > + OUT UINT32 *FmpImageInfoDescriptorVer, > + OUT UINT8 *FmpImageInfoCount, > + OUT UINTN *DescriptorSize > + ) > +{ > + EFI_STATUS Status; > + UINTN ImageInfoSize; > + UINT32 PackageVersion; > + CHAR16 *PackageVersionName; > + EFI_FIRMWARE_IMAGE_DESCRIPTOR *FmpImageInfoBuf; > + > + ImageInfoSize = 0; > + Status = Fmp->GetImageInfo ( > + Fmp, // FMP Pointer > + &ImageInfoSize, // Buffer Size (in this case 0) > + NULL, // NULL so we can get size > + FmpImageInfoDescriptorVer, // DescriptorVersion > + FmpImageInfoCount, // DescriptorCount > + DescriptorSize, // DescriptorSize > + &PackageVersion, // PackageVersion > + &PackageVersionName // PackageVersionName > + ); > + if (Status != EFI_BUFFER_TOO_SMALL) { > + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Unexpected Failure in > GetImageInfo. Status = %r\n", Status)); > + return NULL; > + } > + > + FmpImageInfoBuf = AllocateZeroPool (ImageInfoSize); > + if (FmpImageInfoBuf == NULL) { > + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to get memory for FMP > descriptor.\n")); > + return NULL; > + } > + > + PackageVersionName = NULL; > + Status = Fmp->GetImageInfo ( > + Fmp, // FMP Pointer > + &ImageInfoSize, // ImageInfoSize > + FmpImageInfoBuf, // ImageInfo > + FmpImageInfoDescriptorVer, // DescriptorVersion > + FmpImageInfoCount, // DescriptorCount > + DescriptorSize, // DescriptorSize > + &PackageVersion, // PackageVersion > + &PackageVersionName // PackageVersionName > + ); > + if (PackageVersionName != NULL) { > + FreePool (PackageVersionName); > + } > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failure in GetImageInfo. Status > = %r\n", Status)); > + FreePool (FmpImageInfoBuf); > + return NULL; > + } > + > + return FmpImageInfoBuf; > +} > + > /** > Function to create ESRT based on FMP Instances. > Create ESRT table, get the descriptors from FMP Instance and > @@ -232,29 +363,26 @@ CreateFmpBasedEsrt ( > VOID > ) > { > - EFI_STATUS Status; > - EFI_SYSTEM_RESOURCE_TABLE *Table; > - UINTN NoProtocols; > - VOID **Buffer; > - UINTN Index; > - EFI_FIRMWARE_MANAGEMENT_PROTOCOL *Fmp; > - UINTN DescriptorSize; > - EFI_FIRMWARE_IMAGE_DESCRIPTOR *FmpImageInfoBuf; > - EFI_FIRMWARE_IMAGE_DESCRIPTOR *FmpImageInfoBufOrg; > - UINT8 FmpImageInfoCount; > - UINT32 FmpImageInfoDescriptorVer; > - UINTN ImageInfoSize; > - UINT32 PackageVersion; > - CHAR16 *PackageVersionName; > + EFI_STATUS Status; > + UINTN NoProtocols; > + VOID **Buffer; > + UINTN Index; > + UINT32 FmpImageInfoDescriptorVer; > + UINT8 FmpImageInfoCount; > + UINTN DescriptorSize; > + UINT32 NumberOfDescriptors; > + EFI_FIRMWARE_IMAGE_DESCRIPTOR *FmpImageInfoBuf; > + EFI_FIRMWARE_IMAGE_DESCRIPTOR *OrgFmpImageInfoBuf; > + EFI_SYSTEM_RESOURCE_TABLE *Table; > + GUID_HARDWAREINSTANCE_PAIR *HardwareInstances; > > Status = EFI_SUCCESS; > - Table = NULL; > NoProtocols = 0; > Buffer = NULL; > - PackageVersionName = NULL; > FmpImageInfoBuf = NULL; > - FmpImageInfoBufOrg = NULL; > - Fmp = NULL; > + OrgFmpImageInfoBuf = NULL; > + Table = NULL; > + HardwareInstances = NULL; > > Status = EfiLocateProtocolBuffer ( > &gEfiFirmwareManagementProtocolGuid, > @@ -266,69 +394,64 @@ CreateFmpBasedEsrt ( > } > > // > - // Allocate Memory for table > + // Count the total number of EFI_FIRMWARE_IMAGE_DESCRIPTORs > + // > + for (Index = 0, NumberOfDescriptors = 0; Index < NoProtocols; Index++) { > + FmpImageInfoBuf = FmpGetFirmwareImageDescriptor ( > + (EFI_FIRMWARE_MANAGEMENT_PROTOCOL *) Buffer[Index], > + &FmpImageInfoDescriptorVer, > + &FmpImageInfoCount, > + &DescriptorSize > + ); > + if (FmpImageInfoBuf != NULL) { > + NumberOfDescriptors += FmpImageInfoCount; > + FreePool (FmpImageInfoBuf); > + } > + } > + > + // > + // Allocate ESRT Table and GUID/HardwareInstance table > // > Table = AllocateZeroPool ( > - (GROWTH_STEP * sizeof (EFI_SYSTEM_RESOURCE_ENTRY)) + sizeof > (EFI_SYSTEM_RESOURCE_TABLE) > + (NumberOfDescriptors * sizeof (EFI_SYSTEM_RESOURCE_ENTRY)) + > sizeof (EFI_SYSTEM_RESOURCE_TABLE) > ); > if (Table == NULL) { > DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to allocate memory for > ESRT.\n")); > - gBS->FreePool (Buffer); > + FreePool (Buffer); > return NULL; > } > > + HardwareInstances = AllocateZeroPool (NumberOfDescriptors * sizeof > (GUID_HARDWAREINSTANCE_PAIR)); > + if (HardwareInstances == NULL) { > + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to allocate memory for HW > Instance Table.\n")); > + FreePool (Table); > + FreePool (Buffer); > + return NULL; > + } > + > + // > + // Initialize ESRT Table > + // > Table->FwResourceCount = 0; > - Table->FwResourceCountMax = GROWTH_STEP; > + Table->FwResourceCountMax = NumberOfDescriptors; > Table->FwResourceVersion = > EFI_SYSTEM_RESOURCE_TABLE_FIRMWARE_RESOURCE_VERSION; > > + NumberOfDescriptors = 0; > for (Index = 0; Index < NoProtocols; Index++) { > - Fmp = (EFI_FIRMWARE_MANAGEMENT_PROTOCOL *) Buffer[Index]; > - > - ImageInfoSize = 0; > - Status = Fmp->GetImageInfo ( > - Fmp, // FMP Pointer > - &ImageInfoSize, // Buffer Size (in this case 0) > - NULL, // NULL so we can get size > - &FmpImageInfoDescriptorVer, // DescriptorVersion > - &FmpImageInfoCount, // DescriptorCount > - &DescriptorSize, // DescriptorSize > - &PackageVersion, // PackageVersion > - &PackageVersionName // PackageVersionName > - ); > - > - if (Status != EFI_BUFFER_TOO_SMALL) { > - DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Unexpected Failure in > GetImageInfo. Status = %r\n", Status)); > - continue; > - } > - > - FmpImageInfoBuf = AllocateZeroPool (ImageInfoSize); > + FmpImageInfoBuf = FmpGetFirmwareImageDescriptor ( > + (EFI_FIRMWARE_MANAGEMENT_PROTOCOL *) Buffer[Index], > + &FmpImageInfoDescriptorVer, > + &FmpImageInfoCount, > + &DescriptorSize > + ); > if (FmpImageInfoBuf == NULL) { > - DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to get memory for > descriptors.\n")); > - continue; > - } > - > - FmpImageInfoBufOrg = FmpImageInfoBuf; > - PackageVersionName = NULL; > - Status = Fmp->GetImageInfo ( > - Fmp, > - &ImageInfoSize, // ImageInfoSize > - FmpImageInfoBuf, // ImageInfo > - &FmpImageInfoDescriptorVer, // DescriptorVersion > - &FmpImageInfoCount, // DescriptorCount > - &DescriptorSize, // DescriptorSize > - &PackageVersion, // PackageVersion > - &PackageVersionName // PackageVersionName > - ); > - if (EFI_ERROR (Status)) { > - DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failure in GetImageInfo. Status > = %r\n", Status)); > - FreePool (FmpImageInfoBufOrg); > - FmpImageInfoBufOrg = NULL; > continue; > } > > // > // Check each descriptor and read from the one specified > // > + OrgFmpImageInfoBuf = FmpImageInfoBuf; > while (FmpImageInfoCount > 0) { > // > // If the descriptor has the IN USE bit set, create ESRT entry otherwise > ignore. > @@ -337,7 +460,7 @@ CreateFmpBasedEsrt ( > // > // Create ESRT entry > // > - CreateEsrtEntry (&Table, FmpImageInfoBuf, > FmpImageInfoDescriptorVer); > + CreateEsrtEntry (Table, HardwareInstances, &NumberOfDescriptors, > FmpImageInfoBuf, FmpImageInfoDescriptorVer); > } > FmpImageInfoCount--; > // > @@ -346,15 +469,12 @@ CreateFmpBasedEsrt ( > FmpImageInfoBuf = (EFI_FIRMWARE_IMAGE_DESCRIPTOR *)(((UINT8 > *)FmpImageInfoBuf) + DescriptorSize); > } > > - if (PackageVersionName != NULL) { > - FreePool (PackageVersionName); > - PackageVersionName = NULL; > - } > - FreePool (FmpImageInfoBufOrg); > - FmpImageInfoBufOrg = NULL; > + FreePool (OrgFmpImageInfoBuf); > + OrgFmpImageInfoBuf = NULL; > } > > - gBS->FreePool (Buffer); > + FreePool (Buffer); > + FreePool (HardwareInstances); > return Table; > } > > -- > 2.20.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44922): https://edk2.groups.io/g/devel/message/44922 Mute This Topic: https://groups.io/mt/32723459/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hao Wu, I agree that patches should be cleaned up for submission to edk2 repo. The work in edk2-staging may contain bug fixes and design changes in the patch history that can be consolidated to a smaller patch set. Also, all feedback to a patch set from edk2-staging must be addressed just like any other submission which may include splitting or merging patches. Mike > -----Original Message----- > From: Wu, Hao A > Sent: Monday, August 5, 2019 7:49 PM > To: Jin, Eric <eric.jin@intel.com>; > devel@edk2.groups.io > Cc: Sean Brogan <sean.brogan@microsoft.com>; Bret > Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J > <jian.j.wang@intel.com>; Kinney, Michael D > <michael.d.kinney@intel.com>; Gao, Liming > <liming.gao@intel.com>; afish@apple.com; > lersek@redhat.com; leif.lindholm@linaro.org > Subject: RE: [PATCH v2] MdeModulePkg/EsrtFmpDxe: > Enhance ESRT to support multiple controllers > > > -----Original Message----- > > From: Jin, Eric > > Sent: Monday, August 05, 2019 4:03 PM > > To: devel@edk2.groups.io > > Cc: Sean Brogan; Bret Barkelew; Wang, Jian J; Wu, Hao > A; Kinney, > > Michael D > > Subject: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Enhance > ESRT to support > > multiple controllers > > > > REF: > https://bugzilla.tianocore.org/show_bug.cgi?id=1525 > > > > The patch is to merge multiple FMP instances into > single ESRT entry > > when they have the same GUID. > > > > The policy to LastAttemptStatus/LastAttemptVersion of > ESRT entry is: > > If all the LastAttemptStatus are > LAST_ATTEMPT_STATUS_SUCCESS, then > > LastAttemptVersion should be the smallest of > LastAttemptVersion. If > > any of the LastAttemptStatus is not > LAST_ATTEMPT_STATUS_SUCCESS, then > > the LastAttemptVersion/LastAttemptStatus should be > the values of the > > first FMP instance whose LastAttemptStatus is not > > LAST_ATTEMPT_STATUS_SUCCESS. > > > > To detect possible duplicated GUID/HardwareInstance, > a table of > > GUID/HardwareInstance pairs from all the > > EFI_FIRMWARE_IMAGE_DESCRIPTORs from all FMP instances > is built. If a > > duplicate is found, then generate a DEBUG_ERROR > message, generate an > > ASSERT(), and ignore the duplicate > EFI_FIRMWARE_IMAGE_DESCRIPTOR. > > > > Add an internal worker function called > FmpGetFirmwareImageDescriptor() > > that retrieves the list of > EFI_FIRMWARE_IMAGE_DESCRIPTORs from a > > single FMP instance and returns the descriptors in an > allocated > > buffer. This function is used to get the descriptors > used to build the > > table of unique GUID/HardwareInstance pairs. It is > then used again to > > generate the ESRT Table from all the > EFI_FIRMWARE_IMAGE_DESCRIPTORs > > from all the FMP instances. 2 passes are performed so > the total number > > of descriptors is known. This allows the correct > sized buffers to > > always be allocated. > > > The patch looks good to me. > Reviewed-by: Hao A Wu <hao.a.wu@intel.com> > > I will wait a little longer to push the patch to see if > there is additional feedbacks from stewards with regard > to the approach when merging changes from the edk2- > staging repo. > > Best Regards, > Hao Wu > > > > > > Cc: Sean Brogan <sean.brogan@microsoft.com> > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> > > Cc: Jian J Wang <jian.j.wang@intel.com> > > Cc: Hao A Wu <hao.a.wu@intel.com> > > Signed-off-by: Michael D Kinney > <michael.d.kinney@intel.com> > > Signed-off-by: Eric Jin <eric.jin@intel.com> > > --- > > MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c | 394 > +++++++++++++--- > > ---- > > 1 file changed, 257 insertions(+), 137 deletions(-) > > > > diff --git > a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c > > b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c > > index 2356da662e..4670349f89 100644 > > --- a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c > > +++ b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c > > @@ -2,7 +2,7 @@ > > Publishes ESRT table from Firmware Management > Protocol instances > > > > Copyright (c) 2016, Microsoft Corporation > > - Copyright (c) 2018, Intel Corporation. All rights > reserved.<BR> > > + Copyright (c) 2018 - 2019, Intel Corporation. All > rights > > + reserved.<BR> > > > > All rights reserved. > > SPDX-License-Identifier: BSD-2-Clause-Patent @@ - > 21,6 +21,22 @@ > > #include <Guid/EventGroup.h> #include > <Guid/SystemResourceTable.h> > > > > +/// > > +/// Structure for array of unique > GUID/HardwareInstance pairs from > > +the /// current set of > EFI_FIRMWARE_IMAGE_DESCRIPTORs from all FMP > > Protocols. > > +/// > > +typedef struct { > > + /// > > + /// A unique GUID identifying the firmware image > type. > > + /// > > + EFI_GUID ImageTypeGuid; > > + /// > > + /// An optional number to identify the unique > hardware instance > > +within > > the > > + /// system for devices that may have multiple > instances whenever > > possible. > > + /// > > + UINT64 HardwareInstance; > > +} GUID_HARDWAREINSTANCE_PAIR; > > + > > /** > > Print ESRT to debug console. > > > > @@ -33,11 +49,6 @@ PrintTable ( > > IN EFI_SYSTEM_RESOURCE_TABLE *Table > > ); > > > > -// > > -// Number of ESRT entries to grow by each time we > run out of room -// > > -#define GROWTH_STEP 10 > > - > > /** > > Install EFI System Resource Table into the UEFI > Configuration Table > > > > @@ -101,90 +112,129 @@ IsSystemFmp ( > > } > > > > /** > > - Function to create a single ESRT Entry and add it > to the ESRT > > - given a FMP descriptor. If the guid is already in > the ESRT it > > - will be ignored. The ESRT will grow if it does > not have enough room. > > - > > - @param[in, out] Table On input, > pointer to the pointer to the ESRT. > > - On output, same > as input or pointer to the pointer > > - to new enlarged > ESRT. > > - @param[in] FmpImageInfoBuf Pointer to the > > EFI_FIRMWARE_IMAGE_DESCRIPTOR. > > - @param[in] FmpVersion FMP Version. > > - > > - @return Status code. > > + Function to create a single ESRT Entry and add it > to the ESRT with > > + a given FMP descriptor. If the GUID is already in > the ESRT, then > > + the ESRT entry is updated. > > + > > + @param[in,out] Table Pointer to the > ESRT Table. > > + @param[in,out] HardwareInstances Pointer to the > > GUID_HARDWAREINSTANCE_PAIR. > > + @param[in,out] NumberOfDescriptors The number of > > EFI_FIRMWARE_IMAGE_DESCRIPTORs. > > + @param[in] FmpImageInfoBuf Pointer to the > > EFI_FIRMWARE_IMAGE_DESCRIPTOR. > > + @param[in] FmpVersion FMP Version. > > + > > + @retval EFI_SUCCESS FmpImageInfoBuf was use > to fill in a new ESRT > > entry > > + in Table. > > + @retval EFI_SUCCESS The ImageTypeId GUID in > FmpImageInfoBuf > > matches an > > + existing ESRT entry in > Table, and the information > > + from FmpImageInfoBuf was > merged into the the existing > > + ESRT entry. > > + @retval EFI_UNSPOORTED The GUID/HardareInstance > in > > FmpImageInfoBuf has is a > > + duplicate. > > > > **/ > > EFI_STATUS > > CreateEsrtEntry ( > > - IN OUT EFI_SYSTEM_RESOURCE_TABLE **Table, > > - IN EFI_FIRMWARE_IMAGE_DESCRIPTOR > *FmpImageInfoBuf, > > - IN UINT32 FmpVersion > > + IN OUT EFI_SYSTEM_RESOURCE_TABLE *Table, > > + IN OUT GUID_HARDWAREINSTANCE_PAIR > *HardwareInstances, > > + IN OUT UINT32 > *NumberOfDescriptors, > > + IN EFI_FIRMWARE_IMAGE_DESCRIPTOR > *FmpImageInfoBuf, > > + IN UINT32 FmpVersion > > ) > > { > > UINTN Index; > > EFI_SYSTEM_RESOURCE_ENTRY *Entry; > > - UINTN NewSize; > > - EFI_SYSTEM_RESOURCE_TABLE *NewTable; > > + UINT64 FmpHardwareInstance; > > > > - Index = 0; > > - Entry = NULL; > > + FmpHardwareInstance = 0; > > + if (FmpVersion >= 3) { > > + FmpHardwareInstance = FmpImageInfoBuf- > >HardwareInstance; > > + } > > > > - Entry = (EFI_SYSTEM_RESOURCE_ENTRY *)((*Table) + > 1); > > // > > - // Make sure Guid isn't already in the list > > + // Check to see of FmpImageInfoBuf > GUID/HardwareInstance is unique > > // > > - for (Index = 0; Index < (*Table)->FwResourceCount; > Index++) { > > - if (CompareGuid (&Entry->FwClass, > &FmpImageInfoBuf->ImageTypeId)) { > > - DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: ESRT Entry > already exists for FMP > > Instance with GUID %g\n", &Entry->FwClass)); > > - return EFI_INVALID_PARAMETER; > > + for (Index = 0; Index < *NumberOfDescriptors; > Index++) { > > + if (CompareGuid > (&HardwareInstances[Index].ImageTypeGuid, > > &FmpImageInfoBuf->ImageTypeId)) { > > + if (HardwareInstances[Index].HardwareInstance > == > > FmpHardwareInstance) { > > + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Duplicate > firmware image > > descriptor with GUID %g HardwareInstance:0x%x\n", > &FmpImageInfoBuf- > > >ImageTypeId, FmpHardwareInstance)); > > + ASSERT ( > > + !CompareGuid > (&HardwareInstances[Index].ImageTypeGuid, > > &FmpImageInfoBuf->ImageTypeId) || > > + HardwareInstances[Index].HardwareInstance > != > > FmpHardwareInstance > > + ); > > + return EFI_UNSUPPORTED; > > + } > > } > > - Entry++; > > } > > > > // > > - // Grow table if needed > > + // Record new GUID/HardwareInstance pair > > // > > - if ((*Table)->FwResourceCount >= (*Table)- > >FwResourceCountMax) { > > - NewSize = (((*Table)->FwResourceCountMax + > GROWTH_STEP) * sizeof > > (EFI_SYSTEM_RESOURCE_ENTRY)) + sizeof > (EFI_SYSTEM_RESOURCE_TABLE); > > - NewTable = AllocateZeroPool (NewSize); > > - if (NewTable == NULL) { > > - DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to > allocate memory larger > > table for ESRT. \n")); > > - return EFI_OUT_OF_RESOURCES; > > + CopyGuid > (&HardwareInstances[*NumberOfDescriptors].ImageTypeGuid > , > > &FmpImageInfoBuf->ImageTypeId); > > + > HardwareInstances[*NumberOfDescriptors].HardwareInstanc > e = > > FmpHardwareInstance; > > + *NumberOfDescriptors = *NumberOfDescriptors + 1; > > + > > + DEBUG ((DEBUG_INFO, "EsrtFmpDxe: Add new image > descriptor with > > GUID %g HardwareInstance:0x%x\n", &FmpImageInfoBuf- > >ImageTypeId, > > FmpHardwareInstance)); > > + > > + // > > + // Check to see if GUID is already in the ESRT > table // Entry = > > + (EFI_SYSTEM_RESOURCE_ENTRY *)(Table + 1); for > (Index = 0; Index < > > + Table->FwResourceCount; Index++, Entry++) { > > + if (!CompareGuid (&Entry->FwClass, > > + &FmpImageInfoBuf->ImageTypeId)) > > { > > + continue; > > } > > + DEBUG ((DEBUG_INFO, "EsrtFmpDxe: ESRT Entry > already exists for > > + FMP > > Instance with GUID %g\n", &Entry->FwClass)); > > + > > // > > - // Copy the whole old table into new table > buffer > > - // > > - CopyMem ( > > - NewTable, > > - (*Table), > > - (((*Table)->FwResourceCountMax) * sizeof > > (EFI_SYSTEM_RESOURCE_ENTRY)) + sizeof > (EFI_SYSTEM_RESOURCE_TABLE) > > - ); > > - // > > - // Update max > > + // Set ESRT FwVersion to the smaller of the two > values > > // > > - NewTable->FwResourceCountMax = NewTable- > >FwResourceCountMax + > > GROWTH_STEP; > > + Entry->FwVersion = MIN (FmpImageInfoBuf- > >Version, Entry- > > >FwVersion); > > + > > // > > - // Free old table > > + // VERSION 2 has Lowest Supported > > // > > - FreePool ((*Table)); > > + if (FmpVersion >= 2) { > > + // > > + // Set ESRT LowestSupportedFwVersion to the > smaller of the two values > > + // > > + Entry->LowestSupportedFwVersion = > > + MIN ( > > + FmpImageInfoBuf- > >LowestSupportedImageVersion, > > + Entry->LowestSupportedFwVersion > > + ); > > + } > > + > > // > > - // Reassign pointer to new table. > > + // VERSION 3 supports last attempt values > > // > > - (*Table) = NewTable; > > + if (FmpVersion >= 3) { > > + // > > + // Update the ESRT entry with the last attempt > status and last attempt > > + // version from the first FMP instance whose > last attempt status is not > > + // SUCCESS. If all FMP instances are SUCCESS, > then set version to the > > + // smallest value from all FMP instances. > > + // > > + if (Entry->LastAttemptStatus == > LAST_ATTEMPT_STATUS_SUCCESS) { > > + if (FmpImageInfoBuf->LastAttemptStatus != > > LAST_ATTEMPT_STATUS_SUCCESS) { > > + Entry->LastAttemptStatus = > FmpImageInfoBuf->LastAttemptStatus; > > + Entry->LastAttemptVersion = > FmpImageInfoBuf->LastAttemptVersion; > > + } else { > > + Entry->LastAttemptVersion = > > + MIN ( > > + FmpImageInfoBuf->LastAttemptVersion, > > + Entry->LastAttemptVersion > > + ); > > + } > > + } > > + } > > + > > + return EFI_SUCCESS; > > } > > > > // > > - // ESRT table has enough room for the new entry so > add new entry > > - // > > - Entry = (EFI_SYSTEM_RESOURCE_ENTRY *)(((UINT8 > *)(*Table)) + sizeof > > (EFI_SYSTEM_RESOURCE_TABLE)); > > - // > > - // Move to the location of new entry > > - // > > - Entry = Entry + (*Table)->FwResourceCount; > > + // Add a new ESRT Table Entry > > // > > - // Increment resource count > > - // > > - (*Table)->FwResourceCount++; > > + Entry = (EFI_SYSTEM_RESOURCE_ENTRY *)(Table + 1) + > Table- > > >FwResourceCount; > > > > CopyGuid (&Entry->FwClass, &FmpImageInfoBuf- > >ImageTypeId); > > > > @@ -195,11 +245,11 @@ CreateEsrtEntry ( > > Entry->FwType = > (UINT32)(ESRT_FW_TYPE_DEVICEFIRMWARE); > > } > > > > - Entry->FwVersion = FmpImageInfoBuf->Version; > > + Entry->FwVersion = FmpImageInfoBuf- > >Version; > > Entry->LowestSupportedFwVersion = 0; > > - Entry->CapsuleFlags = 0; > > - Entry->LastAttemptVersion = 0; > > - Entry->LastAttemptStatus = 0; > > + Entry->CapsuleFlags = 0; > > + Entry->LastAttemptVersion = 0; > > + Entry->LastAttemptStatus = 0; > > > > // > > // VERSION 2 has Lowest Supported > > @@ -213,12 +263,93 @@ CreateEsrtEntry ( > > // > > if (FmpVersion >= 3) { > > Entry->LastAttemptVersion = FmpImageInfoBuf- > >LastAttemptVersion; > > - Entry->LastAttemptStatus = FmpImageInfoBuf- > >LastAttemptStatus; > > + Entry->LastAttemptStatus = FmpImageInfoBuf- > >LastAttemptStatus; > > } > > > > + // > > + // Increment the number of active ESRT Table > Entries // > > + Table->FwResourceCount++; > > + > > return EFI_SUCCESS; > > } > > > > +/** > > + Function to retrieve the > EFI_FIRMWARE_IMAGE_DESCRIPTOR from an > > FMP Instance. > > + The returned buffer is allocated using > AllocatePool() and must be > > + freed by > > the > > + caller using FreePool(). > > + > > + @param[in] Fmp Pointer to > an > > EFI_FIRMWARE_MANAGEMENT_PROTOCOL. > > + @param[out] FmpImageInfoDescriptorVer Pointer to > the version > > + number > > associated > > + with the > returned > > EFI_FIRMWARE_IMAGE_DESCRIPTOR. > > + @param[out] FmpImageInfoCount Pointer to > the number of the > > returned > > + > EFI_FIRMWARE_IMAGE_DESCRIPTORs. > > + @param[out] DescriptorSize Pointer to > the size, in bytes, of each > > + returned > EFI_FIRMWARE_IMAGE_DESCRIPTOR. > > + > > + @return Pointer to the retrieved > EFI_FIRMWARE_IMAGE_DESCRIPTOR. > > + If > > the > > + descriptor can not be retrieved, then > NULL is returned. > > + > > +**/ > > +EFI_FIRMWARE_IMAGE_DESCRIPTOR * > > +FmpGetFirmwareImageDescriptor ( > > + IN EFI_FIRMWARE_MANAGEMENT_PROTOCOL *Fmp, > > + OUT UINT32 > *FmpImageInfoDescriptorVer, > > + OUT UINT8 > *FmpImageInfoCount, > > + OUT UINTN > *DescriptorSize > > + ) > > +{ > > + EFI_STATUS Status; > > + UINTN ImageInfoSize; > > + UINT32 PackageVersion; > > + CHAR16 > *PackageVersionName; > > + EFI_FIRMWARE_IMAGE_DESCRIPTOR *FmpImageInfoBuf; > > + > > + ImageInfoSize = 0; > > + Status = Fmp->GetImageInfo ( > > + Fmp, // FMP > Pointer > > + &ImageInfoSize, // > Buffer Size (in this case 0) > > + NULL, // > NULL so we can get size > > + FmpImageInfoDescriptorVer, // > DescriptorVersion > > + FmpImageInfoCount, // > DescriptorCount > > + DescriptorSize, // > DescriptorSize > > + &PackageVersion, // > PackageVersion > > + &PackageVersionName // > PackageVersionName > > + ); > > + if (Status != EFI_BUFFER_TOO_SMALL) { > > + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Unexpected > Failure in > > GetImageInfo. Status = %r\n", Status)); > > + return NULL; > > + } > > + > > + FmpImageInfoBuf = AllocateZeroPool > (ImageInfoSize); if > > + (FmpImageInfoBuf == NULL) { > > + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to get > memory for FMP > > descriptor.\n")); > > + return NULL; > > + } > > + > > + PackageVersionName = NULL; > > + Status = Fmp->GetImageInfo ( > > + Fmp, // FMP > Pointer > > + &ImageInfoSize, // > ImageInfoSize > > + FmpImageInfoBuf, // > ImageInfo > > + FmpImageInfoDescriptorVer, // > DescriptorVersion > > + FmpImageInfoCount, // > DescriptorCount > > + DescriptorSize, // > DescriptorSize > > + &PackageVersion, // > PackageVersion > > + &PackageVersionName // > PackageVersionName > > + ); > > + if (PackageVersionName != NULL) { > > + FreePool (PackageVersionName); > > + } > > + if (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failure in > GetImageInfo. > > + Status > > = %r\n", Status)); > > + FreePool (FmpImageInfoBuf); > > + return NULL; > > + } > > + > > + return FmpImageInfoBuf; > > +} > > + > > /** > > Function to create ESRT based on FMP Instances. > > Create ESRT table, get the descriptors from FMP > Instance and @@ > > -232,29 +363,26 @@ CreateFmpBasedEsrt ( > > VOID > > ) > > { > > - EFI_STATUS Status; > > - EFI_SYSTEM_RESOURCE_TABLE *Table; > > - UINTN NoProtocols; > > - VOID **Buffer; > > - UINTN Index; > > - EFI_FIRMWARE_MANAGEMENT_PROTOCOL *Fmp; > > - UINTN DescriptorSize; > > - EFI_FIRMWARE_IMAGE_DESCRIPTOR > *FmpImageInfoBuf; > > - EFI_FIRMWARE_IMAGE_DESCRIPTOR > *FmpImageInfoBufOrg; > > - UINT8 > FmpImageInfoCount; > > - UINT32 > FmpImageInfoDescriptorVer; > > - UINTN ImageInfoSize; > > - UINT32 PackageVersion; > > - CHAR16 > *PackageVersionName; > > + EFI_STATUS Status; > > + UINTN NoProtocols; > > + VOID **Buffer; > > + UINTN Index; > > + UINT32 > FmpImageInfoDescriptorVer; > > + UINT8 FmpImageInfoCount; > > + UINTN DescriptorSize; > > + UINT32 > NumberOfDescriptors; > > + EFI_FIRMWARE_IMAGE_DESCRIPTOR *FmpImageInfoBuf; > > + EFI_FIRMWARE_IMAGE_DESCRIPTOR *OrgFmpImageInfoBuf; > > + EFI_SYSTEM_RESOURCE_TABLE *Table; > > + GUID_HARDWAREINSTANCE_PAIR *HardwareInstances; > > > > Status = EFI_SUCCESS; > > - Table = NULL; > > NoProtocols = 0; > > Buffer = NULL; > > - PackageVersionName = NULL; > > FmpImageInfoBuf = NULL; > > - FmpImageInfoBufOrg = NULL; > > - Fmp = NULL; > > + OrgFmpImageInfoBuf = NULL; > > + Table = NULL; > > + HardwareInstances = NULL; > > > > Status = EfiLocateProtocolBuffer ( > > &gEfiFirmwareManagementProtocolGuid, > > @@ -266,69 +394,64 @@ CreateFmpBasedEsrt ( > > } > > > > // > > - // Allocate Memory for table > > + // Count the total number of > EFI_FIRMWARE_IMAGE_DESCRIPTORs // > > + for (Index = 0, NumberOfDescriptors = 0; Index < > NoProtocols; Index++) { > > + FmpImageInfoBuf = FmpGetFirmwareImageDescriptor > ( > > + > (EFI_FIRMWARE_MANAGEMENT_PROTOCOL *) Buffer[Index], > > + &FmpImageInfoDescriptorVer, > > + &FmpImageInfoCount, > > + &DescriptorSize > > + ); > > + if (FmpImageInfoBuf != NULL) { > > + NumberOfDescriptors += FmpImageInfoCount; > > + FreePool (FmpImageInfoBuf); > > + } > > + } > > + > > + // > > + // Allocate ESRT Table and GUID/HardwareInstance > table > > // > > Table = AllocateZeroPool ( > > - (GROWTH_STEP * sizeof > (EFI_SYSTEM_RESOURCE_ENTRY)) + sizeof > > (EFI_SYSTEM_RESOURCE_TABLE) > > + (NumberOfDescriptors * sizeof > > + (EFI_SYSTEM_RESOURCE_ENTRY)) + > > sizeof (EFI_SYSTEM_RESOURCE_TABLE) > > ); > > if (Table == NULL) { > > DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to > allocate memory for > > ESRT.\n")); > > - gBS->FreePool (Buffer); > > + FreePool (Buffer); > > return NULL; > > } > > > > + HardwareInstances = AllocateZeroPool > (NumberOfDescriptors * sizeof > > (GUID_HARDWAREINSTANCE_PAIR)); > > + if (HardwareInstances == NULL) { > > + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to > allocate memory for > > + HW > > Instance Table.\n")); > > + FreePool (Table); > > + FreePool (Buffer); > > + return NULL; > > + } > > + > > + // > > + // Initialize ESRT Table > > + // > > Table->FwResourceCount = 0; > > - Table->FwResourceCountMax = GROWTH_STEP; > > + Table->FwResourceCountMax = NumberOfDescriptors; > > Table->FwResourceVersion = > > EFI_SYSTEM_RESOURCE_TABLE_FIRMWARE_RESOURCE_VERSION; > > > > + NumberOfDescriptors = 0; > > for (Index = 0; Index < NoProtocols; Index++) { > > - Fmp = (EFI_FIRMWARE_MANAGEMENT_PROTOCOL *) > Buffer[Index]; > > - > > - ImageInfoSize = 0; > > - Status = Fmp->GetImageInfo ( > > - Fmp, // > FMP Pointer > > - &ImageInfoSize, // > Buffer Size (in this case 0) > > - NULL, // > NULL so we can get size > > - &FmpImageInfoDescriptorVer, // > DescriptorVersion > > - &FmpImageInfoCount, // > DescriptorCount > > - &DescriptorSize, // > DescriptorSize > > - &PackageVersion, // > PackageVersion > > - &PackageVersionName // > PackageVersionName > > - ); > > - > > - if (Status != EFI_BUFFER_TOO_SMALL) { > > - DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Unexpected > Failure in > > GetImageInfo. Status = %r\n", Status)); > > - continue; > > - } > > - > > - FmpImageInfoBuf = AllocateZeroPool > (ImageInfoSize); > > + FmpImageInfoBuf = FmpGetFirmwareImageDescriptor > ( > > + > (EFI_FIRMWARE_MANAGEMENT_PROTOCOL *) Buffer[Index], > > + &FmpImageInfoDescriptorVer, > > + &FmpImageInfoCount, > > + &DescriptorSize > > + ); > > if (FmpImageInfoBuf == NULL) { > > - DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to > get memory for > > descriptors.\n")); > > - continue; > > - } > > - > > - FmpImageInfoBufOrg = FmpImageInfoBuf; > > - PackageVersionName = NULL; > > - Status = Fmp->GetImageInfo ( > > - Fmp, > > - &ImageInfoSize, // > ImageInfoSize > > - FmpImageInfoBuf, // > ImageInfo > > - &FmpImageInfoDescriptorVer, // > DescriptorVersion > > - &FmpImageInfoCount, // > DescriptorCount > > - &DescriptorSize, // > DescriptorSize > > - &PackageVersion, // > PackageVersion > > - &PackageVersionName // > PackageVersionName > > - ); > > - if (EFI_ERROR (Status)) { > > - DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failure in > GetImageInfo. Status > > = %r\n", Status)); > > - FreePool (FmpImageInfoBufOrg); > > - FmpImageInfoBufOrg = NULL; > > continue; > > } > > > > // > > // Check each descriptor and read from the one > specified > > // > > + OrgFmpImageInfoBuf = FmpImageInfoBuf; > > while (FmpImageInfoCount > 0) { > > // > > // If the descriptor has the IN USE bit set, > create ESRT entry > > otherwise ignore. > > @@ -337,7 +460,7 @@ CreateFmpBasedEsrt ( > > // > > // Create ESRT entry > > // > > - CreateEsrtEntry (&Table, FmpImageInfoBuf, > > FmpImageInfoDescriptorVer); > > + CreateEsrtEntry (Table, HardwareInstances, > > + &NumberOfDescriptors, > > FmpImageInfoBuf, FmpImageInfoDescriptorVer); > > } > > FmpImageInfoCount--; > > // > > @@ -346,15 +469,12 @@ CreateFmpBasedEsrt ( > > FmpImageInfoBuf = > (EFI_FIRMWARE_IMAGE_DESCRIPTOR *)(((UINT8 > > *)FmpImageInfoBuf) + DescriptorSize); > > } > > > > - if (PackageVersionName != NULL) { > > - FreePool (PackageVersionName); > > - PackageVersionName = NULL; > > - } > > - FreePool (FmpImageInfoBufOrg); > > - FmpImageInfoBufOrg = NULL; > > + FreePool (OrgFmpImageInfoBuf); > > + OrgFmpImageInfoBuf = NULL; > > } > > > > - gBS->FreePool (Buffer); > > + FreePool (Buffer); > > + FreePool (HardwareInstances); > > return Table; > > } > > > > -- > > 2.20.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44923): https://edk2.groups.io/g/devel/message/44923 Mute This Topic: https://groups.io/mt/32723459/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
> -----Original Message----- > From: Kinney, Michael D > Sent: Tuesday, August 06, 2019 12:46 PM > To: Wu, Hao A; Jin, Eric; devel@edk2.groups.io; Kinney, Michael D > Cc: Sean Brogan; Bret Barkelew; Wang, Jian J; Gao, Liming; afish@apple.com; > lersek@redhat.com; leif.lindholm@linaro.org > Subject: RE: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Enhance ESRT to > support multiple controllers > > Hao Wu, > > I agree that patches should be cleaned up for > submission to edk2 repo. The work in edk2-staging > may contain bug fixes and design changes in the > patch history that can be consolidated to a smaller > patch set. Also, all feedback to a patch set from > edk2-staging must be addressed just like any other > submission which may include splitting or merging > patches. Hello Mike, Thanks for the confirmation. Since there is no other feedback received, the patch has been pushed via commit e314132fea. Best Regards, Hao Wu > > Mike > > > -----Original Message----- > > From: Wu, Hao A > > Sent: Monday, August 5, 2019 7:49 PM > > To: Jin, Eric <eric.jin@intel.com>; > > devel@edk2.groups.io > > Cc: Sean Brogan <sean.brogan@microsoft.com>; Bret > > Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J > > <jian.j.wang@intel.com>; Kinney, Michael D > > <michael.d.kinney@intel.com>; Gao, Liming > > <liming.gao@intel.com>; afish@apple.com; > > lersek@redhat.com; leif.lindholm@linaro.org > > Subject: RE: [PATCH v2] MdeModulePkg/EsrtFmpDxe: > > Enhance ESRT to support multiple controllers > > > > > -----Original Message----- > > > From: Jin, Eric > > > Sent: Monday, August 05, 2019 4:03 PM > > > To: devel@edk2.groups.io > > > Cc: Sean Brogan; Bret Barkelew; Wang, Jian J; Wu, Hao > > A; Kinney, > > > Michael D > > > Subject: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Enhance > > ESRT to support > > > multiple controllers > > > > > > REF: > > https://bugzilla.tianocore.org/show_bug.cgi?id=1525 > > > > > > The patch is to merge multiple FMP instances into > > single ESRT entry > > > when they have the same GUID. > > > > > > The policy to LastAttemptStatus/LastAttemptVersion of > > ESRT entry is: > > > If all the LastAttemptStatus are > > LAST_ATTEMPT_STATUS_SUCCESS, then > > > LastAttemptVersion should be the smallest of > > LastAttemptVersion. If > > > any of the LastAttemptStatus is not > > LAST_ATTEMPT_STATUS_SUCCESS, then > > > the LastAttemptVersion/LastAttemptStatus should be > > the values of the > > > first FMP instance whose LastAttemptStatus is not > > > LAST_ATTEMPT_STATUS_SUCCESS. > > > > > > To detect possible duplicated GUID/HardwareInstance, > > a table of > > > GUID/HardwareInstance pairs from all the > > > EFI_FIRMWARE_IMAGE_DESCRIPTORs from all FMP instances > > is built. If a > > > duplicate is found, then generate a DEBUG_ERROR > > message, generate an > > > ASSERT(), and ignore the duplicate > > EFI_FIRMWARE_IMAGE_DESCRIPTOR. > > > > > > Add an internal worker function called > > FmpGetFirmwareImageDescriptor() > > > that retrieves the list of > > EFI_FIRMWARE_IMAGE_DESCRIPTORs from a > > > single FMP instance and returns the descriptors in an > > allocated > > > buffer. This function is used to get the descriptors > > used to build the > > > table of unique GUID/HardwareInstance pairs. It is > > then used again to > > > generate the ESRT Table from all the > > EFI_FIRMWARE_IMAGE_DESCRIPTORs > > > from all the FMP instances. 2 passes are performed so > > the total number > > > of descriptors is known. This allows the correct > > sized buffers to > > > always be allocated. > > > > > > The patch looks good to me. > > Reviewed-by: Hao A Wu <hao.a.wu@intel.com> > > > > I will wait a little longer to push the patch to see if > > there is additional feedbacks from stewards with regard > > to the approach when merging changes from the edk2- > > staging repo. > > > > Best Regards, > > Hao Wu > > > > > > > > > > Cc: Sean Brogan <sean.brogan@microsoft.com> > > > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com> > > > Cc: Jian J Wang <jian.j.wang@intel.com> > > > Cc: Hao A Wu <hao.a.wu@intel.com> > > > Signed-off-by: Michael D Kinney > > <michael.d.kinney@intel.com> > > > Signed-off-by: Eric Jin <eric.jin@intel.com> > > > --- > > > MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c | 394 > > +++++++++++++--- > > > ---- > > > 1 file changed, 257 insertions(+), 137 deletions(-) > > > > > > diff --git > > a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c > > > b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c > > > index 2356da662e..4670349f89 100644 > > > --- a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c > > > +++ b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c > > > @@ -2,7 +2,7 @@ > > > Publishes ESRT table from Firmware Management > > Protocol instances > > > > > > Copyright (c) 2016, Microsoft Corporation > > > - Copyright (c) 2018, Intel Corporation. All rights > > reserved.<BR> > > > + Copyright (c) 2018 - 2019, Intel Corporation. All > > rights > > > + reserved.<BR> > > > > > > All rights reserved. > > > SPDX-License-Identifier: BSD-2-Clause-Patent @@ - > > 21,6 +21,22 @@ > > > #include <Guid/EventGroup.h> #include > > <Guid/SystemResourceTable.h> > > > > > > +/// > > > +/// Structure for array of unique > > GUID/HardwareInstance pairs from > > > +the /// current set of > > EFI_FIRMWARE_IMAGE_DESCRIPTORs from all FMP > > > Protocols. > > > +/// > > > +typedef struct { > > > + /// > > > + /// A unique GUID identifying the firmware image > > type. > > > + /// > > > + EFI_GUID ImageTypeGuid; > > > + /// > > > + /// An optional number to identify the unique > > hardware instance > > > +within > > > the > > > + /// system for devices that may have multiple > > instances whenever > > > possible. > > > + /// > > > + UINT64 HardwareInstance; > > > +} GUID_HARDWAREINSTANCE_PAIR; > > > + > > > /** > > > Print ESRT to debug console. > > > > > > @@ -33,11 +49,6 @@ PrintTable ( > > > IN EFI_SYSTEM_RESOURCE_TABLE *Table > > > ); > > > > > > -// > > > -// Number of ESRT entries to grow by each time we > > run out of room -// > > > -#define GROWTH_STEP 10 > > > - > > > /** > > > Install EFI System Resource Table into the UEFI > > Configuration Table > > > > > > @@ -101,90 +112,129 @@ IsSystemFmp ( > > > } > > > > > > /** > > > - Function to create a single ESRT Entry and add it > > to the ESRT > > > - given a FMP descriptor. If the guid is already in > > the ESRT it > > > - will be ignored. The ESRT will grow if it does > > not have enough room. > > > - > > > - @param[in, out] Table On input, > > pointer to the pointer to the ESRT. > > > - On output, same > > as input or pointer to the pointer > > > - to new enlarged > > ESRT. > > > - @param[in] FmpImageInfoBuf Pointer to the > > > EFI_FIRMWARE_IMAGE_DESCRIPTOR. > > > - @param[in] FmpVersion FMP Version. > > > - > > > - @return Status code. > > > + Function to create a single ESRT Entry and add it > > to the ESRT with > > > + a given FMP descriptor. If the GUID is already in > > the ESRT, then > > > + the ESRT entry is updated. > > > + > > > + @param[in,out] Table Pointer to the > > ESRT Table. > > > + @param[in,out] HardwareInstances Pointer to the > > > GUID_HARDWAREINSTANCE_PAIR. > > > + @param[in,out] NumberOfDescriptors The number of > > > EFI_FIRMWARE_IMAGE_DESCRIPTORs. > > > + @param[in] FmpImageInfoBuf Pointer to the > > > EFI_FIRMWARE_IMAGE_DESCRIPTOR. > > > + @param[in] FmpVersion FMP Version. > > > + > > > + @retval EFI_SUCCESS FmpImageInfoBuf was use > > to fill in a new ESRT > > > entry > > > + in Table. > > > + @retval EFI_SUCCESS The ImageTypeId GUID in > > FmpImageInfoBuf > > > matches an > > > + existing ESRT entry in > > Table, and the information > > > + from FmpImageInfoBuf was > > merged into the the existing > > > + ESRT entry. > > > + @retval EFI_UNSPOORTED The GUID/HardareInstance > > in > > > FmpImageInfoBuf has is a > > > + duplicate. > > > > > > **/ > > > EFI_STATUS > > > CreateEsrtEntry ( > > > - IN OUT EFI_SYSTEM_RESOURCE_TABLE **Table, > > > - IN EFI_FIRMWARE_IMAGE_DESCRIPTOR > > *FmpImageInfoBuf, > > > - IN UINT32 FmpVersion > > > + IN OUT EFI_SYSTEM_RESOURCE_TABLE *Table, > > > + IN OUT GUID_HARDWAREINSTANCE_PAIR > > *HardwareInstances, > > > + IN OUT UINT32 > > *NumberOfDescriptors, > > > + IN EFI_FIRMWARE_IMAGE_DESCRIPTOR > > *FmpImageInfoBuf, > > > + IN UINT32 FmpVersion > > > ) > > > { > > > UINTN Index; > > > EFI_SYSTEM_RESOURCE_ENTRY *Entry; > > > - UINTN NewSize; > > > - EFI_SYSTEM_RESOURCE_TABLE *NewTable; > > > + UINT64 FmpHardwareInstance; > > > > > > - Index = 0; > > > - Entry = NULL; > > > + FmpHardwareInstance = 0; > > > + if (FmpVersion >= 3) { > > > + FmpHardwareInstance = FmpImageInfoBuf- > > >HardwareInstance; > > > + } > > > > > > - Entry = (EFI_SYSTEM_RESOURCE_ENTRY *)((*Table) + > > 1); > > > // > > > - // Make sure Guid isn't already in the list > > > + // Check to see of FmpImageInfoBuf > > GUID/HardwareInstance is unique > > > // > > > - for (Index = 0; Index < (*Table)->FwResourceCount; > > Index++) { > > > - if (CompareGuid (&Entry->FwClass, > > &FmpImageInfoBuf->ImageTypeId)) { > > > - DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: ESRT Entry > > already exists for FMP > > > Instance with GUID %g\n", &Entry->FwClass)); > > > - return EFI_INVALID_PARAMETER; > > > + for (Index = 0; Index < *NumberOfDescriptors; > > Index++) { > > > + if (CompareGuid > > (&HardwareInstances[Index].ImageTypeGuid, > > > &FmpImageInfoBuf->ImageTypeId)) { > > > + if (HardwareInstances[Index].HardwareInstance > > == > > > FmpHardwareInstance) { > > > + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Duplicate > > firmware image > > > descriptor with GUID %g HardwareInstance:0x%x\n", > > &FmpImageInfoBuf- > > > >ImageTypeId, FmpHardwareInstance)); > > > + ASSERT ( > > > + !CompareGuid > > (&HardwareInstances[Index].ImageTypeGuid, > > > &FmpImageInfoBuf->ImageTypeId) || > > > + HardwareInstances[Index].HardwareInstance > > != > > > FmpHardwareInstance > > > + ); > > > + return EFI_UNSUPPORTED; > > > + } > > > } > > > - Entry++; > > > } > > > > > > // > > > - // Grow table if needed > > > + // Record new GUID/HardwareInstance pair > > > // > > > - if ((*Table)->FwResourceCount >= (*Table)- > > >FwResourceCountMax) { > > > - NewSize = (((*Table)->FwResourceCountMax + > > GROWTH_STEP) * sizeof > > > (EFI_SYSTEM_RESOURCE_ENTRY)) + sizeof > > (EFI_SYSTEM_RESOURCE_TABLE); > > > - NewTable = AllocateZeroPool (NewSize); > > > - if (NewTable == NULL) { > > > - DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to > > allocate memory larger > > > table for ESRT. \n")); > > > - return EFI_OUT_OF_RESOURCES; > > > + CopyGuid > > (&HardwareInstances[*NumberOfDescriptors].ImageTypeGuid > > , > > > &FmpImageInfoBuf->ImageTypeId); > > > + > > HardwareInstances[*NumberOfDescriptors].HardwareInstanc > > e = > > > FmpHardwareInstance; > > > + *NumberOfDescriptors = *NumberOfDescriptors + 1; > > > + > > > + DEBUG ((DEBUG_INFO, "EsrtFmpDxe: Add new image > > descriptor with > > > GUID %g HardwareInstance:0x%x\n", &FmpImageInfoBuf- > > >ImageTypeId, > > > FmpHardwareInstance)); > > > + > > > + // > > > + // Check to see if GUID is already in the ESRT > > table // Entry = > > > + (EFI_SYSTEM_RESOURCE_ENTRY *)(Table + 1); for > > (Index = 0; Index < > > > + Table->FwResourceCount; Index++, Entry++) { > > > + if (!CompareGuid (&Entry->FwClass, > > > + &FmpImageInfoBuf->ImageTypeId)) > > > { > > > + continue; > > > } > > > + DEBUG ((DEBUG_INFO, "EsrtFmpDxe: ESRT Entry > > already exists for > > > + FMP > > > Instance with GUID %g\n", &Entry->FwClass)); > > > + > > > // > > > - // Copy the whole old table into new table > > buffer > > > - // > > > - CopyMem ( > > > - NewTable, > > > - (*Table), > > > - (((*Table)->FwResourceCountMax) * sizeof > > > (EFI_SYSTEM_RESOURCE_ENTRY)) + sizeof > > (EFI_SYSTEM_RESOURCE_TABLE) > > > - ); > > > - // > > > - // Update max > > > + // Set ESRT FwVersion to the smaller of the two > > values > > > // > > > - NewTable->FwResourceCountMax = NewTable- > > >FwResourceCountMax + > > > GROWTH_STEP; > > > + Entry->FwVersion = MIN (FmpImageInfoBuf- > > >Version, Entry- > > > >FwVersion); > > > + > > > // > > > - // Free old table > > > + // VERSION 2 has Lowest Supported > > > // > > > - FreePool ((*Table)); > > > + if (FmpVersion >= 2) { > > > + // > > > + // Set ESRT LowestSupportedFwVersion to the > > smaller of the two values > > > + // > > > + Entry->LowestSupportedFwVersion = > > > + MIN ( > > > + FmpImageInfoBuf- > > >LowestSupportedImageVersion, > > > + Entry->LowestSupportedFwVersion > > > + ); > > > + } > > > + > > > // > > > - // Reassign pointer to new table. > > > + // VERSION 3 supports last attempt values > > > // > > > - (*Table) = NewTable; > > > + if (FmpVersion >= 3) { > > > + // > > > + // Update the ESRT entry with the last attempt > > status and last attempt > > > + // version from the first FMP instance whose > > last attempt status is not > > > + // SUCCESS. If all FMP instances are SUCCESS, > > then set version to the > > > + // smallest value from all FMP instances. > > > + // > > > + if (Entry->LastAttemptStatus == > > LAST_ATTEMPT_STATUS_SUCCESS) { > > > + if (FmpImageInfoBuf->LastAttemptStatus != > > > LAST_ATTEMPT_STATUS_SUCCESS) { > > > + Entry->LastAttemptStatus = > > FmpImageInfoBuf->LastAttemptStatus; > > > + Entry->LastAttemptVersion = > > FmpImageInfoBuf->LastAttemptVersion; > > > + } else { > > > + Entry->LastAttemptVersion = > > > + MIN ( > > > + FmpImageInfoBuf->LastAttemptVersion, > > > + Entry->LastAttemptVersion > > > + ); > > > + } > > > + } > > > + } > > > + > > > + return EFI_SUCCESS; > > > } > > > > > > // > > > - // ESRT table has enough room for the new entry so > > add new entry > > > - // > > > - Entry = (EFI_SYSTEM_RESOURCE_ENTRY *)(((UINT8 > > *)(*Table)) + sizeof > > > (EFI_SYSTEM_RESOURCE_TABLE)); > > > - // > > > - // Move to the location of new entry > > > - // > > > - Entry = Entry + (*Table)->FwResourceCount; > > > + // Add a new ESRT Table Entry > > > // > > > - // Increment resource count > > > - // > > > - (*Table)->FwResourceCount++; > > > + Entry = (EFI_SYSTEM_RESOURCE_ENTRY *)(Table + 1) + > > Table- > > > >FwResourceCount; > > > > > > CopyGuid (&Entry->FwClass, &FmpImageInfoBuf- > > >ImageTypeId); > > > > > > @@ -195,11 +245,11 @@ CreateEsrtEntry ( > > > Entry->FwType = > > (UINT32)(ESRT_FW_TYPE_DEVICEFIRMWARE); > > > } > > > > > > - Entry->FwVersion = FmpImageInfoBuf->Version; > > > + Entry->FwVersion = FmpImageInfoBuf- > > >Version; > > > Entry->LowestSupportedFwVersion = 0; > > > - Entry->CapsuleFlags = 0; > > > - Entry->LastAttemptVersion = 0; > > > - Entry->LastAttemptStatus = 0; > > > + Entry->CapsuleFlags = 0; > > > + Entry->LastAttemptVersion = 0; > > > + Entry->LastAttemptStatus = 0; > > > > > > // > > > // VERSION 2 has Lowest Supported > > > @@ -213,12 +263,93 @@ CreateEsrtEntry ( > > > // > > > if (FmpVersion >= 3) { > > > Entry->LastAttemptVersion = FmpImageInfoBuf- > > >LastAttemptVersion; > > > - Entry->LastAttemptStatus = FmpImageInfoBuf- > > >LastAttemptStatus; > > > + Entry->LastAttemptStatus = FmpImageInfoBuf- > > >LastAttemptStatus; > > > } > > > > > > + // > > > + // Increment the number of active ESRT Table > > Entries // > > > + Table->FwResourceCount++; > > > + > > > return EFI_SUCCESS; > > > } > > > > > > +/** > > > + Function to retrieve the > > EFI_FIRMWARE_IMAGE_DESCRIPTOR from an > > > FMP Instance. > > > + The returned buffer is allocated using > > AllocatePool() and must be > > > + freed by > > > the > > > + caller using FreePool(). > > > + > > > + @param[in] Fmp Pointer to > > an > > > EFI_FIRMWARE_MANAGEMENT_PROTOCOL. > > > + @param[out] FmpImageInfoDescriptorVer Pointer to > > the version > > > + number > > > associated > > > + with the > > returned > > > EFI_FIRMWARE_IMAGE_DESCRIPTOR. > > > + @param[out] FmpImageInfoCount Pointer to > > the number of the > > > returned > > > + > > EFI_FIRMWARE_IMAGE_DESCRIPTORs. > > > + @param[out] DescriptorSize Pointer to > > the size, in bytes, of each > > > + returned > > EFI_FIRMWARE_IMAGE_DESCRIPTOR. > > > + > > > + @return Pointer to the retrieved > > EFI_FIRMWARE_IMAGE_DESCRIPTOR. > > > + If > > > the > > > + descriptor can not be retrieved, then > > NULL is returned. > > > + > > > +**/ > > > +EFI_FIRMWARE_IMAGE_DESCRIPTOR * > > > +FmpGetFirmwareImageDescriptor ( > > > + IN EFI_FIRMWARE_MANAGEMENT_PROTOCOL *Fmp, > > > + OUT UINT32 > > *FmpImageInfoDescriptorVer, > > > + OUT UINT8 > > *FmpImageInfoCount, > > > + OUT UINTN > > *DescriptorSize > > > + ) > > > +{ > > > + EFI_STATUS Status; > > > + UINTN ImageInfoSize; > > > + UINT32 PackageVersion; > > > + CHAR16 > > *PackageVersionName; > > > + EFI_FIRMWARE_IMAGE_DESCRIPTOR *FmpImageInfoBuf; > > > + > > > + ImageInfoSize = 0; > > > + Status = Fmp->GetImageInfo ( > > > + Fmp, // FMP > > Pointer > > > + &ImageInfoSize, // > > Buffer Size (in this case 0) > > > + NULL, // > > NULL so we can get size > > > + FmpImageInfoDescriptorVer, // > > DescriptorVersion > > > + FmpImageInfoCount, // > > DescriptorCount > > > + DescriptorSize, // > > DescriptorSize > > > + &PackageVersion, // > > PackageVersion > > > + &PackageVersionName // > > PackageVersionName > > > + ); > > > + if (Status != EFI_BUFFER_TOO_SMALL) { > > > + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Unexpected > > Failure in > > > GetImageInfo. Status = %r\n", Status)); > > > + return NULL; > > > + } > > > + > > > + FmpImageInfoBuf = AllocateZeroPool > > (ImageInfoSize); if > > > + (FmpImageInfoBuf == NULL) { > > > + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to get > > memory for FMP > > > descriptor.\n")); > > > + return NULL; > > > + } > > > + > > > + PackageVersionName = NULL; > > > + Status = Fmp->GetImageInfo ( > > > + Fmp, // FMP > > Pointer > > > + &ImageInfoSize, // > > ImageInfoSize > > > + FmpImageInfoBuf, // > > ImageInfo > > > + FmpImageInfoDescriptorVer, // > > DescriptorVersion > > > + FmpImageInfoCount, // > > DescriptorCount > > > + DescriptorSize, // > > DescriptorSize > > > + &PackageVersion, // > > PackageVersion > > > + &PackageVersionName // > > PackageVersionName > > > + ); > > > + if (PackageVersionName != NULL) { > > > + FreePool (PackageVersionName); > > > + } > > > + if (EFI_ERROR (Status)) { > > > + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failure in > > GetImageInfo. > > > + Status > > > = %r\n", Status)); > > > + FreePool (FmpImageInfoBuf); > > > + return NULL; > > > + } > > > + > > > + return FmpImageInfoBuf; > > > +} > > > + > > > /** > > > Function to create ESRT based on FMP Instances. > > > Create ESRT table, get the descriptors from FMP > > Instance and @@ > > > -232,29 +363,26 @@ CreateFmpBasedEsrt ( > > > VOID > > > ) > > > { > > > - EFI_STATUS Status; > > > - EFI_SYSTEM_RESOURCE_TABLE *Table; > > > - UINTN NoProtocols; > > > - VOID **Buffer; > > > - UINTN Index; > > > - EFI_FIRMWARE_MANAGEMENT_PROTOCOL *Fmp; > > > - UINTN DescriptorSize; > > > - EFI_FIRMWARE_IMAGE_DESCRIPTOR > > *FmpImageInfoBuf; > > > - EFI_FIRMWARE_IMAGE_DESCRIPTOR > > *FmpImageInfoBufOrg; > > > - UINT8 > > FmpImageInfoCount; > > > - UINT32 > > FmpImageInfoDescriptorVer; > > > - UINTN ImageInfoSize; > > > - UINT32 PackageVersion; > > > - CHAR16 > > *PackageVersionName; > > > + EFI_STATUS Status; > > > + UINTN NoProtocols; > > > + VOID **Buffer; > > > + UINTN Index; > > > + UINT32 > > FmpImageInfoDescriptorVer; > > > + UINT8 FmpImageInfoCount; > > > + UINTN DescriptorSize; > > > + UINT32 > > NumberOfDescriptors; > > > + EFI_FIRMWARE_IMAGE_DESCRIPTOR *FmpImageInfoBuf; > > > + EFI_FIRMWARE_IMAGE_DESCRIPTOR *OrgFmpImageInfoBuf; > > > + EFI_SYSTEM_RESOURCE_TABLE *Table; > > > + GUID_HARDWAREINSTANCE_PAIR *HardwareInstances; > > > > > > Status = EFI_SUCCESS; > > > - Table = NULL; > > > NoProtocols = 0; > > > Buffer = NULL; > > > - PackageVersionName = NULL; > > > FmpImageInfoBuf = NULL; > > > - FmpImageInfoBufOrg = NULL; > > > - Fmp = NULL; > > > + OrgFmpImageInfoBuf = NULL; > > > + Table = NULL; > > > + HardwareInstances = NULL; > > > > > > Status = EfiLocateProtocolBuffer ( > > > &gEfiFirmwareManagementProtocolGuid, > > > @@ -266,69 +394,64 @@ CreateFmpBasedEsrt ( > > > } > > > > > > // > > > - // Allocate Memory for table > > > + // Count the total number of > > EFI_FIRMWARE_IMAGE_DESCRIPTORs // > > > + for (Index = 0, NumberOfDescriptors = 0; Index < > > NoProtocols; Index++) { > > > + FmpImageInfoBuf = FmpGetFirmwareImageDescriptor > > ( > > > + > > (EFI_FIRMWARE_MANAGEMENT_PROTOCOL *) Buffer[Index], > > > + &FmpImageInfoDescriptorVer, > > > + &FmpImageInfoCount, > > > + &DescriptorSize > > > + ); > > > + if (FmpImageInfoBuf != NULL) { > > > + NumberOfDescriptors += FmpImageInfoCount; > > > + FreePool (FmpImageInfoBuf); > > > + } > > > + } > > > + > > > + // > > > + // Allocate ESRT Table and GUID/HardwareInstance > > table > > > // > > > Table = AllocateZeroPool ( > > > - (GROWTH_STEP * sizeof > > (EFI_SYSTEM_RESOURCE_ENTRY)) + sizeof > > > (EFI_SYSTEM_RESOURCE_TABLE) > > > + (NumberOfDescriptors * sizeof > > > + (EFI_SYSTEM_RESOURCE_ENTRY)) + > > > sizeof (EFI_SYSTEM_RESOURCE_TABLE) > > > ); > > > if (Table == NULL) { > > > DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to > > allocate memory for > > > ESRT.\n")); > > > - gBS->FreePool (Buffer); > > > + FreePool (Buffer); > > > return NULL; > > > } > > > > > > + HardwareInstances = AllocateZeroPool > > (NumberOfDescriptors * sizeof > > > (GUID_HARDWAREINSTANCE_PAIR)); > > > + if (HardwareInstances == NULL) { > > > + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to > > allocate memory for > > > + HW > > > Instance Table.\n")); > > > + FreePool (Table); > > > + FreePool (Buffer); > > > + return NULL; > > > + } > > > + > > > + // > > > + // Initialize ESRT Table > > > + // > > > Table->FwResourceCount = 0; > > > - Table->FwResourceCountMax = GROWTH_STEP; > > > + Table->FwResourceCountMax = NumberOfDescriptors; > > > Table->FwResourceVersion = > > > EFI_SYSTEM_RESOURCE_TABLE_FIRMWARE_RESOURCE_VERSION; > > > > > > + NumberOfDescriptors = 0; > > > for (Index = 0; Index < NoProtocols; Index++) { > > > - Fmp = (EFI_FIRMWARE_MANAGEMENT_PROTOCOL *) > > Buffer[Index]; > > > - > > > - ImageInfoSize = 0; > > > - Status = Fmp->GetImageInfo ( > > > - Fmp, // > > FMP Pointer > > > - &ImageInfoSize, // > > Buffer Size (in this case 0) > > > - NULL, // > > NULL so we can get size > > > - &FmpImageInfoDescriptorVer, // > > DescriptorVersion > > > - &FmpImageInfoCount, // > > DescriptorCount > > > - &DescriptorSize, // > > DescriptorSize > > > - &PackageVersion, // > > PackageVersion > > > - &PackageVersionName // > > PackageVersionName > > > - ); > > > - > > > - if (Status != EFI_BUFFER_TOO_SMALL) { > > > - DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Unexpected > > Failure in > > > GetImageInfo. Status = %r\n", Status)); > > > - continue; > > > - } > > > - > > > - FmpImageInfoBuf = AllocateZeroPool > > (ImageInfoSize); > > > + FmpImageInfoBuf = FmpGetFirmwareImageDescriptor > > ( > > > + > > (EFI_FIRMWARE_MANAGEMENT_PROTOCOL *) Buffer[Index], > > > + &FmpImageInfoDescriptorVer, > > > + &FmpImageInfoCount, > > > + &DescriptorSize > > > + ); > > > if (FmpImageInfoBuf == NULL) { > > > - DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to > > get memory for > > > descriptors.\n")); > > > - continue; > > > - } > > > - > > > - FmpImageInfoBufOrg = FmpImageInfoBuf; > > > - PackageVersionName = NULL; > > > - Status = Fmp->GetImageInfo ( > > > - Fmp, > > > - &ImageInfoSize, // > > ImageInfoSize > > > - FmpImageInfoBuf, // > > ImageInfo > > > - &FmpImageInfoDescriptorVer, // > > DescriptorVersion > > > - &FmpImageInfoCount, // > > DescriptorCount > > > - &DescriptorSize, // > > DescriptorSize > > > - &PackageVersion, // > > PackageVersion > > > - &PackageVersionName // > > PackageVersionName > > > - ); > > > - if (EFI_ERROR (Status)) { > > > - DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failure in > > GetImageInfo. Status > > > = %r\n", Status)); > > > - FreePool (FmpImageInfoBufOrg); > > > - FmpImageInfoBufOrg = NULL; > > > continue; > > > } > > > > > > // > > > // Check each descriptor and read from the one > > specified > > > // > > > + OrgFmpImageInfoBuf = FmpImageInfoBuf; > > > while (FmpImageInfoCount > 0) { > > > // > > > // If the descriptor has the IN USE bit set, > > create ESRT entry > > > otherwise ignore. > > > @@ -337,7 +460,7 @@ CreateFmpBasedEsrt ( > > > // > > > // Create ESRT entry > > > // > > > - CreateEsrtEntry (&Table, FmpImageInfoBuf, > > > FmpImageInfoDescriptorVer); > > > + CreateEsrtEntry (Table, HardwareInstances, > > > + &NumberOfDescriptors, > > > FmpImageInfoBuf, FmpImageInfoDescriptorVer); > > > } > > > FmpImageInfoCount--; > > > // > > > @@ -346,15 +469,12 @@ CreateFmpBasedEsrt ( > > > FmpImageInfoBuf = > > (EFI_FIRMWARE_IMAGE_DESCRIPTOR *)(((UINT8 > > > *)FmpImageInfoBuf) + DescriptorSize); > > > } > > > > > > - if (PackageVersionName != NULL) { > > > - FreePool (PackageVersionName); > > > - PackageVersionName = NULL; > > > - } > > > - FreePool (FmpImageInfoBufOrg); > > > - FmpImageInfoBufOrg = NULL; > > > + FreePool (OrgFmpImageInfoBuf); > > > + OrgFmpImageInfoBuf = NULL; > > > } > > > > > > - gBS->FreePool (Buffer); > > > + FreePool (Buffer); > > > + FreePool (HardwareInstances); > > > return Table; > > > } > > > > > > -- > > > 2.20.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44982): https://edk2.groups.io/g/devel/message/44982 Mute This Topic: https://groups.io/mt/32723459/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.