From: "Wu, Hao A" <hao.a.wu@intel.com>
To: "Jin, Eric" <eric.jin@intel.com>,
"devel@edk2.groups.io" <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" <afish@apple.com>,
"lersek@redhat.com" <lersek@redhat.com>,
"leif.lindholm@linaro.org" <leif.lindholm@linaro.org>
Subject: Re: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Enhance ESRT to support multiple controllers
Date: Tue, 6 Aug 2019 02:49:14 +0000 [thread overview]
Message-ID: <B80AF82E9BFB8E4FBD8C89DA810C6A093C916885@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <20190805080248.10360-1-eric.jin@intel.com>
> -----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
next prev parent reply other threads:[~2019-08-06 2:49 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-05 8:02 [PATCH v2] MdeModulePkg/EsrtFmpDxe: Enhance ESRT to support multiple controllers Eric Jin
2019-08-06 2:49 ` Wu, Hao A [this message]
2019-08-06 4:45 ` Michael D Kinney
2019-08-07 5:20 ` Wu, Hao A
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=B80AF82E9BFB8E4FBD8C89DA810C6A093C916885@SHSMSX104.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox