REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1525
If there are multiple FMP instances for the same GUID, then merge them together into a single ESRT entry. Generate DEBUG_ERROR message and ASSERT() if the same GUID/HardwareInstance is produced by more than one FMP. Cc: Sean Brogan <sean.bro...@microsoft.com> Cc: Bret Barkelew <bret.barke...@microsoft.com> Cc: Jian J Wang <jian.j.w...@intel.com> Cc: Hao A Wu <hao.a...@intel.com> Signed-off-by: Michael D Kinney <michael.d.kin...@intel.com> Reviewed-by: Eric Jin <eric....@intel.com> --- MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c | 137 +++++++++++++++++--- 1 file changed, 120 insertions(+), 17 deletions(-) diff --git a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c index 2356da662e..d48f205797 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 @@ -102,8 +102,8 @@ 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. + given a FMP descriptor. If the guid is already in the ESRT, then the ESRT + entry is updated. 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 @@ -117,6 +117,7 @@ IsSystemFmp ( EFI_STATUS CreateEsrtEntry ( IN OUT EFI_SYSTEM_RESOURCE_TABLE **Table, + IN OUT UINT64 **HardwareInstances, IN EFI_FIRMWARE_IMAGE_DESCRIPTOR *FmpImageInfoBuf, IN UINT32 FmpVersion ) @@ -125,18 +126,78 @@ CreateEsrtEntry ( EFI_SYSTEM_RESOURCE_ENTRY *Entry; UINTN NewSize; EFI_SYSTEM_RESOURCE_TABLE *NewTable; + UINT64 *NewHardwareInstances; + UINT64 FmpHardwareInstance; Index = 0; Entry = NULL; Entry = (EFI_SYSTEM_RESOURCE_ENTRY *)((*Table) + 1); // - // Make sure Guid isn't already in the list + // Check to see if GUID is already in the table // 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; + // + // If HardwareInstance in ESRT and FmpImageInfoBuf are the same value + // for the same ImageTypeId GUID, then there is more than one FMP + // instance for the same FW device, which is an error condition. + // If FmpVersion is less than 3, then assume HardwareInstance is 0. + // + FmpHardwareInstance = 0; + if (FmpVersion >= 3) { + FmpHardwareInstance = FmpImageInfoBuf->HardwareInstance; + } + if ((*HardwareInstances)[Index] == FmpHardwareInstance) { + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: ESRT Entry already exists for FMP Instance with GUID %g and HardwareInstance %016lx\n", &Entry->FwClass, (*HardwareInstances)[Index])); + ASSERT ((*HardwareInstances)[Index] != FmpHardwareInstance); + return EFI_UNSUPPORTED; + } + + DEBUG ((DEBUG_INFO, "EsrtFmpDxe: ESRT Entry already exists for FMP Instance with GUID %g\n", &Entry->FwClass)); + + // + // Set ESRT FwVersion to the smaller of the two values + // + Entry->FwVersion = MIN (FmpImageInfoBuf->Version, Entry->FwVersion); + + // + // VERSION 2 has Lowest Supported + // + if (FmpVersion >= 2) { + // + // Set ESRT LowestSupportedFwVersion to the smaller of the two values + // + Entry->LowestSupportedFwVersion = + MIN ( + FmpImageInfoBuf->LowestSupportedImageVersion, + Entry->LowestSupportedFwVersion + ); + } + + // + // VERSION 3 supports last attempt values + // + if (FmpVersion >= 3) { + Entry->LastAttemptVersion = + MIN ( + FmpImageInfoBuf->LastAttemptVersion, + Entry->LastAttemptVersion + ); + // + // 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 (Entry->LastAttemptStatus == LAST_ATTEMPT_STATUS_SUCCESS) { + if (FmpImageInfoBuf->LastAttemptStatus != LAST_ATTEMPT_STATUS_SUCCESS) { + Entry->LastAttemptStatus = FmpImageInfoBuf->LastAttemptStatus; + Entry->LastAttemptVersion = FmpImageInfoBuf->LastAttemptVersion; + } + } + } + + return EFI_SUCCESS; } Entry++; } @@ -171,6 +232,29 @@ CreateEsrtEntry ( // Reassign pointer to new table. // (*Table) = NewTable; + + NewSize = ((*Table)->FwResourceCountMax) * sizeof (UINT64); + NewHardwareInstances = AllocateZeroPool (NewSize); + if (NewHardwareInstances == NULL) { + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to allocate memory larger table for Hardware Instances.\n")); + return EFI_OUT_OF_RESOURCES; + } + // + // Copy the whole old table into new table buffer + // + CopyMem ( + NewHardwareInstances, + (*HardwareInstances), + ((*Table)->FwResourceCountMax) * sizeof (UINT64) + ); + // + // Free old table + // + FreePool ((*HardwareInstances)); + // + // Reassign pointer to new table. + // + (*HardwareInstances) = NewHardwareInstances; } // @@ -181,10 +265,6 @@ CreateEsrtEntry ( // Move to the location of new entry // Entry = Entry + (*Table)->FwResourceCount; - // - // Increment resource count - // - (*Table)->FwResourceCount++; CopyGuid (&Entry->FwClass, &FmpImageInfoBuf->ImageTypeId); @@ -195,11 +275,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,9 +293,22 @@ CreateEsrtEntry ( // if (FmpVersion >= 3) { Entry->LastAttemptVersion = FmpImageInfoBuf->LastAttemptVersion; - Entry->LastAttemptStatus = FmpImageInfoBuf->LastAttemptStatus; + Entry->LastAttemptStatus = FmpImageInfoBuf->LastAttemptStatus; + } + + // + // VERSION 3 supports hardware instance + // + (*HardwareInstances)[(*Table)->FwResourceCount] = 0; + if (FmpVersion >= 3) { + (*HardwareInstances)[(*Table)->FwResourceCount] = FmpImageInfoBuf->HardwareInstance; } + // + // Increment resource count + // + (*Table)->FwResourceCount++; + return EFI_SUCCESS; } @@ -234,6 +327,7 @@ CreateFmpBasedEsrt ( { EFI_STATUS Status; EFI_SYSTEM_RESOURCE_TABLE *Table; + UINT64 *HardwareInstances; UINTN NoProtocols; VOID **Buffer; UINTN Index; @@ -249,6 +343,7 @@ CreateFmpBasedEsrt ( Status = EFI_SUCCESS; Table = NULL; + HardwareInstances = NULL; NoProtocols = 0; Buffer = NULL; PackageVersionName = NULL; @@ -266,7 +361,7 @@ CreateFmpBasedEsrt ( } // - // Allocate Memory for table + // Allocate Memory for tables // Table = AllocateZeroPool ( (GROWTH_STEP * sizeof (EFI_SYSTEM_RESOURCE_ENTRY)) + sizeof (EFI_SYSTEM_RESOURCE_TABLE) @@ -277,6 +372,14 @@ CreateFmpBasedEsrt ( return NULL; } + HardwareInstances = AllocateZeroPool (GROWTH_STEP * sizeof (UINT64)); + if (HardwareInstances == NULL) { + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to allocate memory for HW Instance Table.\n")); + gBS->FreePool (Table); + gBS->FreePool (Buffer); + return NULL; + } + Table->FwResourceCount = 0; Table->FwResourceCountMax = GROWTH_STEP; Table->FwResourceVersion = EFI_SYSTEM_RESOURCE_TABLE_FIRMWARE_RESOURCE_VERSION; @@ -337,7 +440,7 @@ CreateFmpBasedEsrt ( // // Create ESRT entry // - CreateEsrtEntry (&Table, FmpImageInfoBuf, FmpImageInfoDescriptorVer); + CreateEsrtEntry (&Table, &HardwareInstances, FmpImageInfoBuf, FmpImageInfoDescriptorVer); } FmpImageInfoCount--; // -- 2.20.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44681): https://edk2.groups.io/g/devel/message/44681 Mute This Topic: https://groups.io/mt/32667784/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-