[edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Enhance ESRT to support multiple controllers

Michael D Kinney michael.d.kinney at intel.com
Tue Aug 6 04:45:52 UTC 2019


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.

Mike

> -----Original Message-----
> From: Wu, Hao A
> Sent: Monday, August 5, 2019 7:49 PM
> To: Jin, Eric <eric.jin at intel.com>;
> devel at edk2.groups.io
> Cc: Sean Brogan <sean.brogan at microsoft.com>; Bret
> Barkelew <Bret.Barkelew at microsoft.com>; Wang, Jian J
> <jian.j.wang at intel.com>; Kinney, Michael D
> <michael.d.kinney at intel.com>; Gao, Liming
> <liming.gao at intel.com>; afish at apple.com;
> lersek at redhat.com; leif.lindholm at 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 at 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 at 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 at microsoft.com>
> > Cc: Bret Barkelew <Bret.Barkelew at microsoft.com>
> > Cc: Jian J Wang <jian.j.wang at intel.com>
> > Cc: Hao A Wu <hao.a.wu at intel.com>
> > Signed-off-by: Michael D Kinney
> <michael.d.kinney at intel.com>
> > Signed-off-by: Eric Jin <eric.jin at 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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44923): https://edk2.groups.io/g/devel/message/44923
Mute This Topic: https://groups.io/mt/32723459/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list