Jason,

The only time the HardwareInstance is optional (and set to 0) is if the system can guarantee that there is at most one instance of the device in the system. This can only be guaranteed for an integrated device. Any devices that an end user can add/remove from the system through slots or ports may potentially have multiple instances and for those FMP use cases the FMP driver must provide a unique HardwareInstance value.

You did not quote all the text that applies in point (2) below. You need the additional sentence in front to know that 0 can only be used if there is a guarantee there will never be more than 1 instance.

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.

Mike

From: Spottswood, Jason <jason.spottswood@hpe.com>
Sent: Tuesday, October 1, 2019 2:21 PM
To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io; Rothman, Michael A <michael.a.rothman@intel.com>
Subject: RE: [edk2-devel] Recent changes to EsrtFmp causing ASSERTs

Hey Mike,

Specifically, my team has run into ASSERTs on systems loaded with many identical model HDDs.  Each HDD provides an FMP where the type GUID is identical, and the HW instance is not provided (zero).  Attached serial log with additional debug output to print the FMP version. (We commented out the ASSERT to gather the log.)

Given the current text in the UEFI spec, I do not believe the device vendor is necessarily at fault here, even if I would like for them to implement the HW instance.

1. First sentence from the FW image HW instance description says this field is optional.

 ///

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

2. Near the end of the FW image HW instance description says that an FMP can use zero to mean uniqueness cannot be determined.

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

Another point is that the ESRT can only have one entry for each given type GUID.  There is also no HW instance field for entries in the ESRT.  As it relates to building the ESRT, checking for duplicate type GUID/IDs is the only requirement, not HW instance.

-Jason

From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Tuesday, October 1, 2019 12:31 PM
To: devel@edk2.groups.io; Spottswood, Jason <jason.spottswood@hpe.com>; Rothman, Michael A <michael.a.rothman@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel] Recent changes to EsrtFmp causing ASSERTs

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