From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout.perfora.net (mout.perfora.net [74.208.4.196]) by mx.groups.io with SMTP id smtpd.web10.19365.1679417876026424178 for ; Tue, 21 Mar 2023 09:57:56 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: smith-denny.com, ip: 74.208.4.196, mailfrom: osd@smith-denny.com) Received: from [10.137.194.171] ([131.107.8.107]) by mrelay.perfora.net (mreueus002 [74.208.5.2]) with ESMTPSA (Nemesis) id 0MaYs3-1ptjLW2OGP-00K6sO; Tue, 21 Mar 2023 17:57:47 +0100 Message-ID: Date: Tue, 21 Mar 2023 09:57:46 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [edk2-devel] [PATCH v3 1/1] ArmPkg/SmbiosMiscDxe: Adjust the priority of getting firmware version To: devel@edk2.groups.io, tinhnguyen@os.amperecomputing.com Cc: patches@amperecomputing.com, quic_llindhol@quicinc.com, ardb+tianocore@kernel.org, nhi@os.amperecomputing.com, rebecca@bsdio.com References: <20230321031638.1241041-1-tinhnguyen@os.amperecomputing.com> From: "Oliver Smith-Denny" In-Reply-To: <20230321031638.1241041-1-tinhnguyen@os.amperecomputing.com> X-Provags-ID: V03:K1:L6FJ/3HWt4bFUg38NILIxQFJbLoLy7WDAUM+9OhfIpItlG3ozcK Q8lNXJPXrgqAizHZpePyqugN86rBdwh6mid5O9tmBKLj8STqEffE0qpIrcSqEszRMZvMG2L 0Wjvh9rafsV+0eiNivBHlV4MctZsE94OVPS3YoutSUy+9E7qJ4vICOnd6Lt7kn4xXOjCc3w d/gUF08K6ZVCKRD+7CouQ== X-Spam-Flag: NO UI-OutboundReport: notjunk:1;M01:P0:beIxS2DQ0TM=;Ws6xbfUdv1H/fPV3RFqVc5jxbWb wuzIeYqJbaP2zw2OT1vFz4dv5dLo/6hZk86dqA+1rLUoyM+T4yghvUuUCo4LOR+pmOfItgS8H SEVkyfa+SD6w08qjKntdGAnApH3XuTmWPSxjG1PmJgUghvrPS/jFrxGwhTmYwn7KsPqFgS1Gd OK0VN6cV4w1p1kjwJct9jnYwgnAVRdl9akUICI/Rw+DasDeWSOBq5S4Em7kS2VIfcnqrbatYh kTakv7i/J7bbhmIxXZqUmZ4wMP2HQ526gCCFOAEzgIQ+q2ef1h8ec3b+zp1e3wWGwxbMTbrgy XIYJjhyZ1WjBzhgUhyoE/+Ql3EMpdUGVdumvk7hPttM3unmMlrIeNb6BmFwDCQo2rtlQbkw2s x5PHa0mA1dOp0Dx7CQsSA+Pac0/MQk4oaNPhL2R3a1A5zY5tf9/trri2xlO4n4hyRkHOyoT/S bYGmZCwMrXi1TVlQzf1VVzf4/7dzWtfEndfU4Y3CmNfOG6KmwvCdQ5Pps+8JPYlWBZy0ICGXb jSO8z7/Cn9fVHZr2HZiYz0xnhIK5aVNKuXafbu9v6FmKzyjqGkcgeLexzlvd0jEvmazDLfLtD GXyXT9lclkQ+F5KR6QZ81ax34kqNjEYGiy0+G8+sXr4QAbf5Xb49TP40drlIqNCiHu4PhAy/5 Tn2+wr17uKMdxXwfYJh0cKrrcJyNaUnPessVBcfvmw== Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit One nit below, otherwise: Reviewed-by: Oliver Smith-Denny Thanks! On 3/20/2023 8:16 PM, Tinh Nguyen via groups.io wrote: > The BIOS Firmware Version in the SMBIOS Type 0 can be fetched from > the fixed PcdFirmwareVersionString or platform specific OemMiscLib. > In fact, the support from OemMiscLib comes into play when the firmware > version may be modified at boot time for extended information. > Therefore, the priority of getting the version from OemMiscLib should > be higher. > > In case there is no modification in the OemMiscLib, we have to keep > HII string STR_MISC_BIOS_VERSION empty or 'Not Specified' > to indicate that the firmware version should be fetched from > the PcdFirmwareVersionString. > > Signed-off-by: Tinh Nguyen > Reviewed-by: Rebecca Cran > --- > > Changes since v1: > + Change GetBiosVersion () to SetBiosVersion () and move the selection logic > fully into SetBiosVersion (). > Changes since v2: > + Add Reviewed-by: Rebecca Cran and remove @retval > VOID > > ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c | 57 ++++++++++++-------- > 1 file changed, 35 insertions(+), 22 deletions(-) > > diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c > index 66ead22a6e2c..876a74614285 100644 > --- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c > +++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c > @@ -1,6 +1,6 @@ > /** @file > > - Copyright (c) 2022, Ampere Computing LLC. All rights reserved.
> + Copyright (c) 2022 - 2023, Ampere Computing LLC. All rights reserved.
> Copyright (c) 2021, NUVIA Inc. All rights reserved.
> Copyright (c) 2009, Intel Corporation. All rights reserved.
> Copyright (c) 2015, Hisilicon Limited. All rights reserved.
> @@ -124,22 +124,46 @@ GetBiosReleaseDate ( > return ReleaseDate; > } > > -/** > - Fetches the firmware ('BIOS') version from the > - FirmwareVersionInfo HOB. > +/** Fetches the Firmware version string for SMBIOS type 0 > + > + This function get the Firmware version string from OemMiscLib first, > + if it is invalid then PcdFirmwareVersionString is used as a fallback. nit: this function comment seems a bit at odds with the name of the function (i.e. the comment says it gets the FW version, but the name of the function is SetBiosVersion, which I see was changed in this patch). Perhaps updating the comment to indicate it retrieves the BIOS version from OemMiscLib or PcdFirmwareVersionString and then sets it in SMBIOS. > > - @return The version as a UTF-16 string > **/ > -CHAR16 * > -GetBiosVersion ( > +VOID > +SetBiosVersion ( > VOID > ) > { > - CHAR16 *ReleaseString; > + CHAR16 *DefaultVersionString; > + CHAR16 *Version; > + EFI_STRING_ID TokenToUpdate; > > - ReleaseString = (CHAR16 *)FixedPcdGetPtr (PcdFirmwareVersionString); > + DefaultVersionString = HiiGetString ( > + mSmbiosMiscHiiHandle, > + STRING_TOKEN (STR_MISC_BIOS_VERSION), > + NULL > + ); > > - return ReleaseString; > + OemUpdateSmbiosInfo ( > + mSmbiosMiscHiiHandle, > + STRING_TOKEN (STR_MISC_BIOS_VERSION), > + BiosVersionType00 > + ); > + > + Version = HiiGetString ( > + mSmbiosMiscHiiHandle, > + STRING_TOKEN (STR_MISC_BIOS_VERSION), > + NULL > + ); > + > + if (((StrCmp (Version, DefaultVersionString) == 0) || (StrLen (Version) == 0))) { > + Version = (CHAR16 *)FixedPcdGetPtr (PcdFirmwareVersionString); > + if (StrLen (Version) > 0) { > + TokenToUpdate = STRING_TOKEN (STR_MISC_BIOS_VERSION); > + HiiSetString (mSmbiosMiscHiiHandle, TokenToUpdate, Version, NULL); > + } > + } > } > > /** > @@ -187,18 +211,7 @@ SMBIOS_MISC_TABLE_FUNCTION (MiscBiosVendor) { > HiiSetString (mSmbiosMiscHiiHandle, TokenToUpdate, Vendor, NULL); > } > > - Version = GetBiosVersion (); > - > - if (StrLen (Version) > 0) { > - TokenToUpdate = STRING_TOKEN (STR_MISC_BIOS_VERSION); > - HiiSetString (mSmbiosMiscHiiHandle, TokenToUpdate, Version, NULL); > - } else { > - OemUpdateSmbiosInfo ( > - mSmbiosMiscHiiHandle, > - STRING_TOKEN (STR_MISC_BIOS_VERSION), > - BiosVersionType00 > - ); > - } > + SetBiosVersion (); > > Char16String = GetBiosReleaseDate (); > if (StrLen (Char16String) > 0) { > -- > 2.39.2 > > > > >