[edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance

Jeff Brasen via groups.io posted 1 patch 1 year, 3 months ago
Failed in applying to current master (apply log)
MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c | 22 ++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
[edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
Posted by Jeff Brasen via groups.io 1 year, 3 months ago
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97277): https://edk2.groups.io/g/devel/message/97277
Mute This Topic: https://groups.io/mt/95629273/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
Posted by Jeff Brasen via groups.io 1 year, 1 month ago
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99409): https://edk2.groups.io/g/devel/message/99409
Mute This Topic: https://groups.io/mt/95629273/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] FW: [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
Posted by Jeff Brasen via groups.io 1 year, 1 month ago
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99931): https://edk2.groups.io/g/devel/message/99931
Mute This Topic: https://groups.io/mt/96864087/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
Posted by Michael D Kinney 1 year, 1 month ago
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
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99946): https://edk2.groups.io/g/devel/message/99946
Mute This Topic: https://groups.io/mt/96869771/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
Posted by Jeff Brasen via groups.io 1 year, 1 month ago
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
> >
> >
> >
> > 
> >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100026): https://edk2.groups.io/g/devel/message/100026
Mute This Topic: https://groups.io/mt/96869771/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
Posted by Michael D Kinney 1 year, 1 month ago
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
> > >
> > >
> > >
> > >
> > >
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100034): https://edk2.groups.io/g/devel/message/100034
Mute This Topic: https://groups.io/mt/96869771/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
Posted by Jeff Brasen via groups.io 1 year, 1 month ago
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
> > > >
> > > >
> > > >
> > > >
> > > >
> >
> >
> >
> > 
> >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100035): https://edk2.groups.io/g/devel/message/100035
Mute This Topic: https://groups.io/mt/96869771/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
Posted by Michael D Kinney 1 year, 1 month ago
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
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > >
> > >
> > >
> > >
> > >
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100036): https://edk2.groups.io/g/devel/message/100036
Mute This Topic: https://groups.io/mt/96869771/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
Posted by Jeff Brasen via groups.io 1 year, 1 month ago
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
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> >
> >
> >
> > 
> >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100037): https://edk2.groups.io/g/devel/message/100037
Mute This Topic: https://groups.io/mt/96869771/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
Posted by Michael D Kinney 1 year, 1 month ago
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
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > >
> > >
> > >
> > > 
> > >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100202): https://edk2.groups.io/g/devel/message/100202
Mute This Topic: https://groups.io/mt/96869771/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2] MdeModulePkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance
Posted by Michael D Kinney 1 year, 1 month ago
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
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > >
> > > >
> > > >
> > > > 
> > > >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100209): https://edk2.groups.io/g/devel/message/100209
Mute This Topic: https://groups.io/mt/96869771/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-