From: "Michael D Kinney" <michael.d.kinney@intel.com>
To: "Jin, Eric" <eric.jin@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Subject: Re: [Staging/Bug_1525_FmpDevicePkg_MultipleControllers][PATCH] MdeModulePkg/EsrtFmpDxe: Detect duplicate GUID/HardwareInstance
Date: Mon, 22 Apr 2019 14:42:14 +0000 [thread overview]
Message-ID: <E92EE9817A31E24EB0585FDF735412F5B9C9C2F7@ORSMSX113.amr.corp.intel.com> (raw)
In-Reply-To: <20190422054818.1468-1-eric.jin@intel.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
> -----Original Message-----
> From: Jin, Eric
> Sent: Sunday, April 21, 2019 10:48 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> Subject:
> [Staging/Bug_1525_FmpDevicePkg_MultipleControllers][PAT
> CH] MdeModulePkg/EsrtFmpDxe: Detect duplicate
> GUID/HardwareInstance
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=1525
>
> Fix the following issues identified in #1525:
> EsrtFmpDxe - The check for more than one instance
> of the same
> GUID/HardwareInstance is not being done at the
> right point.
> It is being done as EFI_FIRMWARE_IMAGE_DESCRIPTORs
> are being merged
> into an ESRT Table Entry. This means a case will
> be missed if
> there are 3 EFI_FIRMWARE_IMAGE_DESCRIPTORs with the
> first and 3rd
> one being having the same GUID/HardwareInstance.
> Instead, this check
> should be performed across all
> EFI_FIRMWARE_IMAGE_DESCRIPTORs in the
> entire handle database.
>
> Build a table of GUID/HardwareInstance pairs from all
> the
> EFI_FIRMWARE_IMAGE_DESCRIPTORs from all FMP instances.
> 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 and the extra logic to grow tables is
> removed.
>
> CC: Eric Jin <eric.jin@intel.com>
> CC: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Jin <eric.jin@intel.com>
> ---
> MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c | 461
> ++++++++++----------
> 1 file changed, 238 insertions(+), 223 deletions(-)
>
> diff --git
> a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
> b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
> index 848bd44e9d..9c273fe79e 100644
> --- a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
> +++ b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
> @@ -38,6 +38,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.
>
> @@ -50,11 +66,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
>
> @@ -118,172 +129,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, 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
> - 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 OUT UINT64
> **HardwareInstances,
> - 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 *NewHardwareInstances;
> UINT64 FmpHardwareInstance;
>
> - Index = 0;
> - Entry = NULL;
> + FmpHardwareInstance = 0;
> + if (FmpVersion >= 3) {
> + FmpHardwareInstance = FmpImageInfoBuf-
> >HardwareInstance;
> + }
>
> - Entry = (EFI_SYSTEM_RESOURCE_ENTRY *)((*Table) + 1);
> //
> - // Check to see if GUID is already in the table
> + // Check to see of FmpImageInfoBuf
> GUID/HardwareInstance is unique
> //
> - for (Index = 0; Index < (*Table)->FwResourceCount;
> Index++) {
> - if (CompareGuid (&Entry->FwClass,
> &FmpImageInfoBuf->ImageTypeId)) {
> - //
> - // 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);
> + 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;
> }
> -
> - 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) {
> - //
> - // 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;
> }
> - 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;
> - }
> - //
> - // 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
> - //
> - NewTable->FwResourceCountMax = NewTable-
> >FwResourceCountMax + GROWTH_STEP;
> - //
> - // Free old table
> - //
> - FreePool ((*Table));
> - //
> - // Reassign pointer to new table.
> - //
> - (*Table) = NewTable;
> + 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));
>
> - 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;
> + //
> + // 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
> + // Set ESRT FwVersion to the smaller of the two
> values
> //
> - CopyMem (
> - NewHardwareInstances,
> - (*HardwareInstances),
> - ((*Table)->FwResourceCountMax) * sizeof (UINT64)
> - );
> + Entry->FwVersion = MIN (FmpImageInfoBuf->Version,
> Entry->FwVersion);
> +
> //
> - // Free old table
> + // VERSION 2 has Lowest Supported
> //
> - FreePool ((*HardwareInstances));
> + 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
> //
> - (*HardwareInstances) = NewHardwareInstances;
> + 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));
> + // Add a new ESRT Table Entry
> //
> - // Move to the location of new entry
> - //
> - Entry = Entry + (*Table)->FwResourceCount;
> + Entry = (EFI_SYSTEM_RESOURCE_ENTRY *)(Table + 1) +
> Table->FwResourceCount;
>
> CopyGuid (&Entry->FwClass, &FmpImageInfoBuf-
> >ImageTypeId);
>
> @@ -316,19 +284,87 @@ CreateEsrtEntry (
> }
>
> //
> - // VERSION 3 supports hardware instance
> + // Increment the number of active ESRT Table Entries
> //
> - (*HardwareInstances)[(*Table)->FwResourceCount] = 0;
> - if (FmpVersion >= 3) {
> - (*HardwareInstances)[(*Table)->FwResourceCount] =
> FmpImageInfoBuf->HardwareInstance;
> + 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;
> }
>
> - //
> - // Increment resource count
> - //
> - (*Table)->FwResourceCount++;
> + FmpImageInfoBuf = AllocateZeroPool (ImageInfoSize);
> + if (FmpImageInfoBuf == NULL) {
> + DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to get
> memory for FMP descriptor.\n"));
> + return NULL;
> + }
>
> - return EFI_SUCCESS;
> + 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;
> }
>
> /**
> @@ -344,31 +380,26 @@ CreateFmpBasedEsrt (
> VOID
> )
> {
> - EFI_STATUS Status;
> - EFI_SYSTEM_RESOURCE_TABLE *Table;
> - UINT64
> *HardwareInstances;
> - 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;
> - HardwareInstances = NULL;
> NoProtocols = 0;
> Buffer = NULL;
> - PackageVersionName = NULL;
> FmpImageInfoBuf = NULL;
> - FmpImageInfoBufOrg = NULL;
> - Fmp = NULL;
> + OrgFmpImageInfoBuf = NULL;
> + Table = NULL;
> + HardwareInstances = NULL;
>
> Status = EfiLocateProtocolBuffer (
> &gEfiFirmwareManagementProtocolGuid,
> @@ -380,77 +411,64 @@ CreateFmpBasedEsrt (
> }
>
> //
> - // Allocate Memory for tables
> + // 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 (GROWTH_STEP *
> sizeof (UINT64));
> + HardwareInstances = AllocateZeroPool
> (NumberOfDescriptors * sizeof
> (GUID_HARDWAREINSTANCE_PAIR));
> if (HardwareInstances == NULL) {
> DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Failed to
> allocate memory for HW Instance Table.\n"));
> - gBS->FreePool (Table);
> - gBS->FreePool (Buffer);
> + 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.
> @@ -459,7 +477,7 @@ CreateFmpBasedEsrt (
> //
> // Create ESRT entry
> //
> - CreateEsrtEntry (&Table, &HardwareInstances,
> FmpImageInfoBuf, FmpImageInfoDescriptorVer);
> + CreateEsrtEntry (Table, HardwareInstances,
> &NumberOfDescriptors, FmpImageInfoBuf,
> FmpImageInfoDescriptorVer);
> }
> FmpImageInfoCount--;
> //
> @@ -468,15 +486,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
prev parent reply other threads:[~2019-04-22 14:42 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-22 5:48 [Staging/Bug_1525_FmpDevicePkg_MultipleControllers][PATCH] MdeModulePkg/EsrtFmpDxe: Detect duplicate GUID/HardwareInstance Eric Jin
2019-04-22 14:42 ` Michael D Kinney [this message]
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=E92EE9817A31E24EB0585FDF735412F5B9C9C2F7@ORSMSX113.amr.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