Hi Jason,

I believe the logic to check for uniqueness of FMP Descriptor is correct.

The UEFI Spec has 2 structs with HardwareInstance. One is FMP Descriptor and the other is UEFI Capsule for FMP.

The HardwareInstance in FMP must be unique and not 0 unless there is a guarantee there is only one instance.

///

/// An optional number to identify the unique hardware instance within the system for

/// devices that may have multiple instances (Example: a plug in pci network card). This

/// number must be unique within the namespace of the ImageTypeId GUID and

/// ImageIndex. For FMP instances that have multiple descriptors for a single

/// hardware instance, all descriptors must have the same HardwareInstance value.

/// This number must be consistent between boots and should be based on some sort of

/// hardware identified unique id (serial number, etc) whenever possible. If a hardware

/// based number is not available the FMP provider may use some other characteristic

/// such as device path, bus/dev/function, slot num, etc for generating the

/// HardwareInstance. For implementations that will never have more than one

/// instance a zero can be used. A zero means the FMP provider is not able to determine a

/// unique hardware instance number or a hardware instance number is not needed. Only

/// present in version 3 or higher.

///

UINT64 HardwareInstance;

} EFI_FIRMWARE_IMAGE_DESCRIPTOR;

The UEFI Capsule one can be 0 to specify match any.

///

/// The HardwareInstance to target with this update. If value is zero it means match all

/// HardwareInstances. This field allows update software to target only a single device in

/// cases where there are more than one device with the same ImageTypeId GUID.

/// This header is outside the signed data of the Authentication Info structure and

/// therefore can be modified without changing the Auth data.

///

UINT64 UpdateHardwareInstance;

} EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER;

Best regards,

Mike

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Spottswood, Jason
Sent: Monday, September 30, 2019 2:24 PM
To: Rothman, Michael A <michael.a.rothman@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] Recent changes to EsrtFmp causing ASSERTs

Cut-n-paste problem. My apologies. 

Copying again in plain text:

  //

  // Check to see of FmpImageInfoBuf GUID/HardwareInstance is unique

  //

  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;

      }

    }

  }

-Jason

From: Rothman, Michael A <michael.a.rothman@intel.com>
Sent: Monday, September 30, 2019 3:53 PM
To: devel@edk2.groups.io; Spottswood, Jason <jason.spottswood@hpe.com>
Subject: Re: [edk2-devel] Recent changes to EsrtFmp causing ASSERTs

Jason, Agreed - though that image you sent was challenging for these old eyes. Black on dark grey? Ack!

Thanks,

Michael A. Rothman

---------------------------------------------------------------

Let no excuse be a barrier to your success.

On Sep 30, 2019, at 1:48 PM, "jason.spottswood@hpe.com" <jason.spottswood@hpe.com> wrote:

In EsrtFmp.c, function CreateEsrtEntry, line 196, the code asserts if the FMP image hardware instance matches that of an existing instance.  This is fine if the hardware instance is supported.  The field is optional though.  In the UEFI spec, "a zero hardware instance means the FMP provider is not able to determine a unique hardware instance number or a hardware instance number is not needed."  The code below needs to also check if the hardware instance is supported (by comparing it to zero) before checking it against existing entries.

  //
  // Check to see of FmpImageInfoBuf GUID/HardwareInstance is unique
  //
  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;
      }
    }
  }