Re: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance

["Michael D Kinney" <michael.d.kinney@intel.com>]

Hi Jeff,

Thank you for the reminder.

I am wondering if the check should be for FmpVersion < 3 and not
FmpHardwareInstance != 0.

It is possible for an FmpInfoBuffer to have an FmpVersion >= 3 and
a HardwareInstance of 0.  If more than one of these it found, then
that would be an EFI_UNSUPPORTED condition.

If FmpVersion >= 3, then the HardwareInstance must be filled in
with a unique value.  0 can be used if there is at most 1 instance,
so the duplicate check would never be triggered.  If FmpVersion >= 3
and there can be more then one instance, then the HardwareInstance 
must be a non-zero unique value.

Thanks,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jeff Brasen via groups.io
> Sent: Thursday, February 9, 2023 2:03 PM
> To: Demeter, Miki <miki.demeter@intel.com>
> Cc: devel@edk2.groups.io
> Subject: [edk2-devel] FW: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
> 
> Here is a patch that has been pending for a bit
> 
> Thanks,
> Jeff
> 
> -----Original Message-----
> From: Jeff Brasen
> Sent: Wednesday, February 1, 2023 9:21 AM
> To: devel@edk2.groups.io
> Cc: jian.j.wang@intel.com; gaoliming@byosoft.com.cn; guomin.jiang@intel.com
> Subject: RE: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
> 
> Any updates on this would be great to get this in for the next stable release if possible.
> 
> > -----Original Message-----
> > From: Jeff Brasen <jbrasen@nvidia.com>
> > Sent: Monday, December 12, 2022 12:27 PM
> > To: devel@edk2.groups.io
> > Cc: jian.j.wang@intel.com; gaoliming@byosoft.com.cn;
> > guomin.jiang@intel.com; Jeff Brasen <jbrasen@nvidia.com>
> > Subject: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices
> > with 0 HardwareInstance
> >
> > Skip error check if HardwareInstance is 0 as this either means that
> > FmpVersion < 3 and not supported or, "A zero means the FMP provider is
> > not able to determine a unique hardware instance number or a hardware
> > instance number is not needed." per UEFI specification.
> >
> > As the FmpInstances are merged and HardwareInstance is not used remove
> > error check in this case.
> >
> >
> > Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> > ---
> >  MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c | 22 ++++++++++++------
> > ---
> >  1 file changed, 13 insertions(+), 9 deletions(-)
> >
> > diff --git a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
> > b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
> > index 4f47c55cce..5bc627461d 100644
> > --- a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
> > +++ b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c
> > @@ -153,16 +153,20 @@ CreateEsrtEntry (
> >
> >    //
> >    // Check to see of FmpImageInfoBuf GUID/HardwareInstance is unique
> > +  // Skip if HardwareInstance is 0 as this is the case if FmpVersion
> > + <
> > + 3  // or the device can not create a unique ID per UEFI
> > + specification
> >    //
> > -  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;
> > +  if (FmpHardwareInstance != 0) {
> > +    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;
> > +        }
> >        }
> >      }
> >    }
> > --
> > 2.25.1
> 
> 
> 
> 
>