public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
@ 2022-12-12 19:27 Jeff Brasen
  2023-02-01 16:21 ` Jeff Brasen
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff Brasen @ 2022-12-12 19:27 UTC (permalink / raw)
  To: devel; +Cc: jian.j.wang, gaoliming, guomin.jiang, Jeff Brasen

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
  2022-12-12 19:27 [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance Jeff Brasen
@ 2023-02-01 16:21 ` Jeff Brasen
  2023-02-09 22:02   ` FW: " Jeff Brasen
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff Brasen @ 2023-02-01 16:21 UTC (permalink / raw)
  To: devel@edk2.groups.io
  Cc: jian.j.wang@intel.com, gaoliming@byosoft.com.cn,
	guomin.jiang@intel.com

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* FW: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
  2023-02-01 16:21 ` Jeff Brasen
@ 2023-02-09 22:02   ` Jeff Brasen
  2023-02-10  3:42     ` Michael D Kinney
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff Brasen @ 2023-02-09 22:02 UTC (permalink / raw)
  To: miki.demeter@intel.com; +Cc: devel@edk2.groups.io

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
  2023-02-09 22:02   ` FW: " Jeff Brasen
@ 2023-02-10  3:42     ` Michael D Kinney
  2023-02-10 16:44       ` Jeff Brasen
  0 siblings, 1 reply; 11+ messages in thread
From: Michael D Kinney @ 2023-02-10  3:42 UTC (permalink / raw)
  To: devel@edk2.groups.io, jbrasen@nvidia.com, Demeter, Miki; +Cc: Kinney, Michael D

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
  2023-02-10  3:42     ` Michael D Kinney
@ 2023-02-10 16:44       ` Jeff Brasen
  2023-02-10 19:47         ` [edk2-devel] " Michael D Kinney
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff Brasen @ 2023-02-10 16:44 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io, Demeter, Miki

That was my original approach when I saw this but I was seeing this occur on a device with FmpVersion == 3. The UEFI spec isn't completely clear but could be read that 0 is legal if this part isn't being used.

https://uefi.org/specs/UEFI/2.10/23_Firmware_Update_and_Reporting.html#efi-firmware-management-protocol-getimageinfo

It does state that this is Optional, and has a statement "A zero means the FMP provider is not able to determine a unique hardware instance number or a hardware instance number is not needed."

Also, our ESRT implementation just merges all instances to one anyways so it doesn't currently consume the hardware instance except for this check.

Thanks,
Jeff

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Thursday, February 9, 2023 8:43 PM
> To: devel@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>; Demeter,
> Miki <miki.demeter@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple
> devices with 0 HardwareInstance
> 
> External email: Use caution opening links or attachments
> 
> 
> 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
> >
> >
> >
> > 
> >


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
  2023-02-10 16:44       ` Jeff Brasen
@ 2023-02-10 19:47         ` Michael D Kinney
  2023-02-10 20:43           ` Jeff Brasen
  0 siblings, 1 reply; 11+ messages in thread
From: Michael D Kinney @ 2023-02-10 19:47 UTC (permalink / raw)
  To: devel@edk2.groups.io, jbrasen@nvidia.com, Demeter, Miki; +Cc: Kinney, Michael D

Did your original approach work for all your use cases?

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jeff Brasen via groups.io
> Sent: Friday, February 10, 2023 8:45 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io; Demeter, Miki <miki.demeter@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
> 
> That was my original approach when I saw this but I was seeing this occur on a device with FmpVersion == 3. The UEFI spec isn't
> completely clear but could be read that 0 is legal if this part isn't being used.
> 
> https://uefi.org/specs/UEFI/2.10/23_Firmware_Update_and_Reporting.html#efi-firmware-management-protocol-getimageinfo
> 
> It does state that this is Optional, and has a statement "A zero means the FMP provider is not able to determine a unique
> hardware instance number or a hardware instance number is not needed."
> 
> Also, our ESRT implementation just merges all instances to one anyways so it doesn't currently consume the hardware instance
> except for this check.
> 
> Thanks,
> Jeff
> 
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Thursday, February 9, 2023 8:43 PM
> > To: devel@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>; Demeter,
> > Miki <miki.demeter@intel.com>
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > Subject: RE: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple
> > devices with 0 HardwareInstance
> >
> > External email: Use caution opening links or attachments
> >
> >
> > 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
> > >
> > >
> > >
> > >
> > >
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
  2023-02-10 19:47         ` [edk2-devel] " Michael D Kinney
@ 2023-02-10 20:43           ` Jeff Brasen
  2023-02-10 21:17             ` Michael D Kinney
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff Brasen @ 2023-02-10 20:43 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io, Demeter, Miki

It was not as we were seeing devices expose FMP via an option rom driver with Version == 3 and HardwareInstance = 0. If there are multiple of these device it would hit the error in the EsrtCode. The patch included does resolve the issues we were seeing.

-Jeff


> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Friday, February 10, 2023 12:48 PM
> To: devel@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>; Demeter,
> Miki <miki.demeter@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support
> multiple devices with 0 HardwareInstance
> 
> External email: Use caution opening links or attachments
> 
> 
> Did your original approach work for all your use cases?
> 
> Mike
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jeff
> > Brasen via groups.io
> > Sent: Friday, February 10, 2023 8:45 AM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > devel@edk2.groups.io; Demeter, Miki <miki.demeter@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe:
> Support
> > multiple devices with 0 HardwareInstance
> >
> > That was my original approach when I saw this but I was seeing this
> > occur on a device with FmpVersion == 3. The UEFI spec isn't completely
> clear but could be read that 0 is legal if this part isn't being used.
> >
> >
> https://uefi.org/specs/UEFI/2.10/23_Firmware_Update_and_Reporting.htm
> l
> > #efi-firmware-management-protocol-getimageinfo
> >
> > It does state that this is Optional, and has a statement "A zero means
> > the FMP provider is not able to determine a unique hardware instance
> number or a hardware instance number is not needed."
> >
> > Also, our ESRT implementation just merges all instances to one anyways
> > so it doesn't currently consume the hardware instance except for this
> check.
> >
> > Thanks,
> > Jeff
> >
> > > -----Original Message-----
> > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Sent: Thursday, February 9, 2023 8:43 PM
> > > To: devel@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>; Demeter,
> > > Miki <miki.demeter@intel.com>
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Subject: RE: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple
> > > devices with 0 HardwareInstance
> > >
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > 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
> > > >
> > > >
> > > >
> > > >
> > > >
> >
> >
> >
> > 
> >


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
  2023-02-10 20:43           ` Jeff Brasen
@ 2023-02-10 21:17             ` Michael D Kinney
  2023-02-10 21:20               ` Jeff Brasen
  0 siblings, 1 reply; 11+ messages in thread
From: Michael D Kinney @ 2023-02-10 21:17 UTC (permalink / raw)
  To: devel@edk2.groups.io, jbrasen@nvidia.com, Demeter, Miki; +Cc: Kinney, Michael D

Based on UEFI Spec, would you consider those option roms to be non-conformant?

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jeff Brasen via groups.io
> Sent: Friday, February 10, 2023 12:43 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io; Demeter, Miki <miki.demeter@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
> 
> It was not as we were seeing devices expose FMP via an option rom driver with Version == 3 and HardwareInstance = 0. If there
> are multiple of these device it would hit the error in the EsrtCode. The patch included does resolve the issues we were seeing.
> 
> -Jeff
> 
> 
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Friday, February 10, 2023 12:48 PM
> > To: devel@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>; Demeter,
> > Miki <miki.demeter@intel.com>
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support
> > multiple devices with 0 HardwareInstance
> >
> > External email: Use caution opening links or attachments
> >
> >
> > Did your original approach work for all your use cases?
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jeff
> > > Brasen via groups.io
> > > Sent: Friday, February 10, 2023 8:45 AM
> > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > > devel@edk2.groups.io; Demeter, Miki <miki.demeter@intel.com>
> > > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe:
> > Support
> > > multiple devices with 0 HardwareInstance
> > >
> > > That was my original approach when I saw this but I was seeing this
> > > occur on a device with FmpVersion == 3. The UEFI spec isn't completely
> > clear but could be read that 0 is legal if this part isn't being used.
> > >
> > >
> > https://uefi.org/specs/UEFI/2.10/23_Firmware_Update_and_Reporting.htm
> > l
> > > #efi-firmware-management-protocol-getimageinfo
> > >
> > > It does state that this is Optional, and has a statement "A zero means
> > > the FMP provider is not able to determine a unique hardware instance
> > number or a hardware instance number is not needed."
> > >
> > > Also, our ESRT implementation just merges all instances to one anyways
> > > so it doesn't currently consume the hardware instance except for this
> > check.
> > >
> > > Thanks,
> > > Jeff
> > >
> > > > -----Original Message-----
> > > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > Sent: Thursday, February 9, 2023 8:43 PM
> > > > To: devel@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>; Demeter,
> > > > Miki <miki.demeter@intel.com>
> > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > Subject: RE: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple
> > > > devices with 0 HardwareInstance
> > > >
> > > > External email: Use caution opening links or attachments
> > > >
> > > >
> > > > 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
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > >
> > >
> > >
> > >
> > >
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
  2023-02-10 21:17             ` Michael D Kinney
@ 2023-02-10 21:20               ` Jeff Brasen
  2023-02-14 23:50                 ` Michael D Kinney
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff Brasen @ 2023-02-10 21:20 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io, Demeter, Miki

This is unclear in my opinion. The spec does have
This number must be unique within the namespace of the ImageTypeId GUID and ImageIndex. And
For implementations that will never have more than one instance a zero can be used.

But it also states the parameter is optional and states that zero can be used when if it is not able to determine a unique hardware instance number or a hardware instance number is not needed.

I could see reasonable arguments on both sides.

-Jeff

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Friday, February 10, 2023 2:17 PM
> To: devel@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>; Demeter,
> Miki <miki.demeter@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support
> multiple devices with 0 HardwareInstance
> 
> External email: Use caution opening links or attachments
> 
> 
> Based on UEFI Spec, would you consider those option roms to be non-
> conformant?
> 
> Mike
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jeff
> > Brasen via groups.io
> > Sent: Friday, February 10, 2023 12:43 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > devel@edk2.groups.io; Demeter, Miki <miki.demeter@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe:
> Support
> > multiple devices with 0 HardwareInstance
> >
> > It was not as we were seeing devices expose FMP via an option rom
> > driver with Version == 3 and HardwareInstance = 0. If there are multiple of
> these device it would hit the error in the EsrtCode. The patch included does
> resolve the issues we were seeing.
> >
> > -Jeff
> >
> >
> > > -----Original Message-----
> > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Sent: Friday, February 10, 2023 12:48 PM
> > > To: devel@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>; Demeter,
> > > Miki <miki.demeter@intel.com>
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe:
> > > Support multiple devices with 0 HardwareInstance
> > >
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > Did your original approach work for all your use cases?
> > >
> > > Mike
> > >
> > > > -----Original Message-----
> > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > Jeff Brasen via groups.io
> > > > Sent: Friday, February 10, 2023 8:45 AM
> > > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > > > devel@edk2.groups.io; Demeter, Miki <miki.demeter@intel.com>
> > > > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe:
> > > Support
> > > > multiple devices with 0 HardwareInstance
> > > >
> > > > That was my original approach when I saw this but I was seeing
> > > > this occur on a device with FmpVersion == 3. The UEFI spec isn't
> > > > completely
> > > clear but could be read that 0 is legal if this part isn't being used.
> > > >
> > > >
> > >
> https://uefi.org/specs/UEFI/2.10/23_Firmware_Update_and_Reporting.ht
> > > m
> > > l
> > > > #efi-firmware-management-protocol-getimageinfo
> > > >
> > > > It does state that this is Optional, and has a statement "A zero
> > > > means the FMP provider is not able to determine a unique hardware
> > > > instance
> > > number or a hardware instance number is not needed."
> > > >
> > > > Also, our ESRT implementation just merges all instances to one
> > > > anyways so it doesn't currently consume the hardware instance
> > > > except for this
> > > check.
> > > >
> > > > Thanks,
> > > > Jeff
> > > >
> > > > > -----Original Message-----
> > > > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > > Sent: Thursday, February 9, 2023 8:43 PM
> > > > > To: devel@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>;
> > > > > Demeter, Miki <miki.demeter@intel.com>
> > > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > > Subject: RE: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support
> > > > > multiple devices with 0 HardwareInstance
> > > > >
> > > > > External email: Use caution opening links or attachments
> > > > >
> > > > >
> > > > > 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
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> >
> >
> >
> > 
> >


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
  2023-02-10 21:20               ` Jeff Brasen
@ 2023-02-14 23:50                 ` Michael D Kinney
  2023-02-15  1:59                   ` Michael D Kinney
  0 siblings, 1 reply; 11+ messages in thread
From: Michael D Kinney @ 2023-02-14 23:50 UTC (permalink / raw)
  To: Jeff Brasen, devel@edk2.groups.io, Demeter, Miki; +Cc: Kinney, Michael D

Hi Jeff,

I have been studying the code for side effects from Version >= 3 and HardwareInstance = 0.

I think the only side effect with your patch is extra entries in HardwareIntances with the
same GUID value and HardwareInstance value of 0.

  IN OUT GUID_HARDWAREINSTANCE_PAIR     *HardwareInstances,
  IN OUT UINT32                         *NumberOfDescriptors,

This table is only used to look for duplicate GUID/HardwareInstance pairs and is not used
anywhere else and is freed after ESRT entries are determined.  The same number of ESRT entries
are be created even if these extra HardwareInstances entries are present.

I have also not seen any comments or concerns from anyone else on this topic.



Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>



Best regards,

Mike

> -----Original Message-----
> From: Jeff Brasen <jbrasen@nvidia.com>
> Sent: Friday, February 10, 2023 1:21 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io; Demeter, Miki <miki.demeter@intel.com>
> Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
> 
> This is unclear in my opinion. The spec does have
> This number must be unique within the namespace of the ImageTypeId GUID and ImageIndex. And
> For implementations that will never have more than one instance a zero can be used.
> 
> But it also states the parameter is optional and states that zero can be used when if it is not able to determine a unique
> hardware instance number or a hardware instance number is not needed.
> 
> I could see reasonable arguments on both sides.
> 
> -Jeff
> 
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Friday, February 10, 2023 2:17 PM
> > To: devel@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>; Demeter,
> > Miki <miki.demeter@intel.com>
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support
> > multiple devices with 0 HardwareInstance
> >
> > External email: Use caution opening links or attachments
> >
> >
> > Based on UEFI Spec, would you consider those option roms to be non-
> > conformant?
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jeff
> > > Brasen via groups.io
> > > Sent: Friday, February 10, 2023 12:43 PM
> > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > > devel@edk2.groups.io; Demeter, Miki <miki.demeter@intel.com>
> > > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe:
> > Support
> > > multiple devices with 0 HardwareInstance
> > >
> > > It was not as we were seeing devices expose FMP via an option rom
> > > driver with Version == 3 and HardwareInstance = 0. If there are multiple of
> > these device it would hit the error in the EsrtCode. The patch included does
> > resolve the issues we were seeing.
> > >
> > > -Jeff
> > >
> > >
> > > > -----Original Message-----
> > > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > Sent: Friday, February 10, 2023 12:48 PM
> > > > To: devel@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>; Demeter,
> > > > Miki <miki.demeter@intel.com>
> > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe:
> > > > Support multiple devices with 0 HardwareInstance
> > > >
> > > > External email: Use caution opening links or attachments
> > > >
> > > >
> > > > Did your original approach work for all your use cases?
> > > >
> > > > Mike
> > > >
> > > > > -----Original Message-----
> > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > > Jeff Brasen via groups.io
> > > > > Sent: Friday, February 10, 2023 8:45 AM
> > > > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > > > > devel@edk2.groups.io; Demeter, Miki <miki.demeter@intel.com>
> > > > > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe:
> > > > Support
> > > > > multiple devices with 0 HardwareInstance
> > > > >
> > > > > That was my original approach when I saw this but I was seeing
> > > > > this occur on a device with FmpVersion == 3. The UEFI spec isn't
> > > > > completely
> > > > clear but could be read that 0 is legal if this part isn't being used.
> > > > >
> > > > >
> > > >
> > https://uefi.org/specs/UEFI/2.10/23_Firmware_Update_and_Reporting.ht
> > > > m
> > > > l
> > > > > #efi-firmware-management-protocol-getimageinfo
> > > > >
> > > > > It does state that this is Optional, and has a statement "A zero
> > > > > means the FMP provider is not able to determine a unique hardware
> > > > > instance
> > > > number or a hardware instance number is not needed."
> > > > >
> > > > > Also, our ESRT implementation just merges all instances to one
> > > > > anyways so it doesn't currently consume the hardware instance
> > > > > except for this
> > > > check.
> > > > >
> > > > > Thanks,
> > > > > Jeff
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > > > Sent: Thursday, February 9, 2023 8:43 PM
> > > > > > To: devel@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>;
> > > > > > Demeter, Miki <miki.demeter@intel.com>
> > > > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > > > Subject: RE: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support
> > > > > > multiple devices with 0 HardwareInstance
> > > > > >
> > > > > > External email: Use caution opening links or attachments
> > > > > >
> > > > > >
> > > > > > 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
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > >
> > >
> > >
> > > 
> > >


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
  2023-02-14 23:50                 ` Michael D Kinney
@ 2023-02-15  1:59                   ` Michael D Kinney
  0 siblings, 0 replies; 11+ messages in thread
From: Michael D Kinney @ 2023-02-15  1:59 UTC (permalink / raw)
  To: Jeff Brasen, devel@edk2.groups.io, Demeter, Miki; +Cc: Kinney, Michael D

Merged

PR: https://github.com/tianocore/edk2/pull/4043
Commit: https://github.com/tianocore/edk2/commit/090642db7ac124c336da72e1954e1fb09e816fb0

Mike

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Tuesday, February 14, 2023 3:50 PM
> To: Jeff Brasen <jbrasen@nvidia.com>; devel@edk2.groups.io; Demeter, Miki <miki.demeter@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
> 
> Hi Jeff,
> 
> I have been studying the code for side effects from Version >= 3 and HardwareInstance = 0.
> 
> I think the only side effect with your patch is extra entries in HardwareIntances with the
> same GUID value and HardwareInstance value of 0.
> 
>   IN OUT GUID_HARDWAREINSTANCE_PAIR     *HardwareInstances,
>   IN OUT UINT32                         *NumberOfDescriptors,
> 
> This table is only used to look for duplicate GUID/HardwareInstance pairs and is not used
> anywhere else and is freed after ESRT entries are determined.  The same number of ESRT entries
> are be created even if these extra HardwareInstances entries are present.
> 
> I have also not seen any comments or concerns from anyone else on this topic.
> 
> 
> 
> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
> 
> 
> 
> Best regards,
> 
> Mike
> 
> > -----Original Message-----
> > From: Jeff Brasen <jbrasen@nvidia.com>
> > Sent: Friday, February 10, 2023 1:21 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io; Demeter, Miki <miki.demeter@intel.com>
> > Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
> >
> > This is unclear in my opinion. The spec does have
> > This number must be unique within the namespace of the ImageTypeId GUID and ImageIndex. And
> > For implementations that will never have more than one instance a zero can be used.
> >
> > But it also states the parameter is optional and states that zero can be used when if it is not able to determine a unique
> > hardware instance number or a hardware instance number is not needed.
> >
> > I could see reasonable arguments on both sides.
> >
> > -Jeff
> >
> > > -----Original Message-----
> > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Sent: Friday, February 10, 2023 2:17 PM
> > > To: devel@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>; Demeter,
> > > Miki <miki.demeter@intel.com>
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support
> > > multiple devices with 0 HardwareInstance
> > >
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > Based on UEFI Spec, would you consider those option roms to be non-
> > > conformant?
> > >
> > > Mike
> > >
> > > > -----Original Message-----
> > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jeff
> > > > Brasen via groups.io
> > > > Sent: Friday, February 10, 2023 12:43 PM
> > > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > > > devel@edk2.groups.io; Demeter, Miki <miki.demeter@intel.com>
> > > > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe:
> > > Support
> > > > multiple devices with 0 HardwareInstance
> > > >
> > > > It was not as we were seeing devices expose FMP via an option rom
> > > > driver with Version == 3 and HardwareInstance = 0. If there are multiple of
> > > these device it would hit the error in the EsrtCode. The patch included does
> > > resolve the issues we were seeing.
> > > >
> > > > -Jeff
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > > Sent: Friday, February 10, 2023 12:48 PM
> > > > > To: devel@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>; Demeter,
> > > > > Miki <miki.demeter@intel.com>
> > > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > > Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe:
> > > > > Support multiple devices with 0 HardwareInstance
> > > > >
> > > > > External email: Use caution opening links or attachments
> > > > >
> > > > >
> > > > > Did your original approach work for all your use cases?
> > > > >
> > > > > Mike
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > > > Jeff Brasen via groups.io
> > > > > > Sent: Friday, February 10, 2023 8:45 AM
> > > > > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > > > > > devel@edk2.groups.io; Demeter, Miki <miki.demeter@intel.com>
> > > > > > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe:
> > > > > Support
> > > > > > multiple devices with 0 HardwareInstance
> > > > > >
> > > > > > That was my original approach when I saw this but I was seeing
> > > > > > this occur on a device with FmpVersion == 3. The UEFI spec isn't
> > > > > > completely
> > > > > clear but could be read that 0 is legal if this part isn't being used.
> > > > > >
> > > > > >
> > > > >
> > > https://uefi.org/specs/UEFI/2.10/23_Firmware_Update_and_Reporting.ht
> > > > > m
> > > > > l
> > > > > > #efi-firmware-management-protocol-getimageinfo
> > > > > >
> > > > > > It does state that this is Optional, and has a statement "A zero
> > > > > > means the FMP provider is not able to determine a unique hardware
> > > > > > instance
> > > > > number or a hardware instance number is not needed."
> > > > > >
> > > > > > Also, our ESRT implementation just merges all instances to one
> > > > > > anyways so it doesn't currently consume the hardware instance
> > > > > > except for this
> > > > > check.
> > > > > >
> > > > > > Thanks,
> > > > > > Jeff
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > > > > Sent: Thursday, February 9, 2023 8:43 PM
> > > > > > > To: devel@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>;
> > > > > > > Demeter, Miki <miki.demeter@intel.com>
> > > > > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > > > > Subject: RE: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support
> > > > > > > multiple devices with 0 HardwareInstance
> > > > > > >
> > > > > > > External email: Use caution opening links or attachments
> > > > > > >
> > > > > > >
> > > > > > > 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
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > >
> > > >
> > > >
> > > > 
> > > >


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-02-15  1:59 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-12 19:27 [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance Jeff Brasen
2023-02-01 16:21 ` Jeff Brasen
2023-02-09 22:02   ` FW: " Jeff Brasen
2023-02-10  3:42     ` Michael D Kinney
2023-02-10 16:44       ` Jeff Brasen
2023-02-10 19:47         ` [edk2-devel] " Michael D Kinney
2023-02-10 20:43           ` Jeff Brasen
2023-02-10 21:17             ` Michael D Kinney
2023-02-10 21:20               ` Jeff Brasen
2023-02-14 23:50                 ` Michael D Kinney
2023-02-15  1:59                   ` Michael D Kinney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox