Re: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Enhance ESRT to support multiple controllers

["Wu, Hao A" <hao.a.wu@intel.com>]

> -----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