From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.88, mailfrom: eric.jin@intel.com) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by groups.io with SMTP; Wed, 31 Jul 2019 08:01:59 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 31 Jul 2019 08:01:59 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,330,1559545200"; d="scan'208";a="191261859" Received: from jjin9-mobl.ccr.corp.intel.com ([10.255.29.207]) by fmsmga001.fm.intel.com with ESMTP; 31 Jul 2019 08:01:57 -0700 From: "Eric Jin" To: devel@edk2.groups.io Cc: Sean Brogan , Bret Barkelew , Jian J Wang , Hao A Wu , Michael D Kinney Subject: [PATCH 1/3] MdeModulePkg/EsrtFmpDxe: Merge multiple FMP into ESRT Date: Wed, 31 Jul 2019 23:00:55 +0800 Message-Id: <20190731150055.21108-1-eric.jin@intel.com> X-Mailer: git-send-email 2.20.1.windows.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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 Cc: Bret Barkelew Cc: Jian J Wang Cc: Hao A Wu Signed-off-by: Michael D Kinney Reviewed-by: Eric Jin --- 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.
+ Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
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