From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web11.15742.1670421412485666759 for ; Wed, 07 Dec 2022 05:56:52 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: pierre.gondois@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 340E523A; Wed, 7 Dec 2022 05:56:58 -0800 (PST) Received: from [10.34.100.128] (pierre123.nice.arm.com [10.34.100.128]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 76B3A3F73D; Wed, 7 Dec 2022 05:56:50 -0800 (PST) Message-ID: <901a43ff-138d-7ff4-2e0b-ec87618b751f@arm.com> Date: Wed, 7 Dec 2022 14:56:46 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [edk2-devel] [PATCH] DynamicTablesPkg/EsrtFmpDxe: Support multiple devices with 0 HardwareInstance To: devel@edk2.groups.io, jbrasen@nvidia.com Cc: jian.j.wang@intel.com, gaoliming@byosoft.com.cn, guomin.jiang@intel.com References: From: "PierreGondois" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 > --- > 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; > + } > } > } > }