[PATCH] DynamicTablesPkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance

[PATCH] DynamicTablesPkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance

[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

Re: [edk2-devel] [PATCH] DynamicTablesPkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance

[PierreGondois]

Hello Jeff,
I think the patch is not for the DynamicTablesPkg but for the
MdeModulePkg. FWIW, the patch looks good to me,

Regards,
Pierre

On 11/16/22 16:50, Jeff Brasen via groups.io wrote:
> 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;
> +        }
>         }
>       }
>     }