Re: [edk2-devel] [PATCH 1/1] ArmPkg/SmbiosMiscDxe: Adjust the priority of getting firmware version

["Rebecca Cran" <rebecca@bsdio.com>]

Given Leif's comments, I was expecting a new version with some changes.


-- 

Rebecca Cran


On 3/16/23 2:30 AM, Tinh Nguyen via groups.io wrote:
> + Rebecca
>
> Could you kindly help me in reviewing this patch?
>
> Thanks,
>
> - Tinh
>
> On 13/03/2023 13:43, Tinh Nguyen 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.
>>
>> Reviewed-by: Nhi Pham <nhi@os.amperecomputing.com>
>> Signed-off-by: Tinh Nguyen <tinhnguyen@os.amperecomputing.com>
>> ---
>> ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c 
>> | 36 ++++++++++++++------
>>   1 file changed, 25 insertions(+), 11 deletions(-)
>>
>> diff --git 
>> a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c 
>> b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
>> index 66ead22a6e2c..31a3f6cde544 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.<BR>
>> +  Copyright (c) 2022 - 2023, Ampere Computing LLC. All rights 
>> reserved.<BR>
>>     Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
>>     Copyright (c) 2009, Intel Corporation. All rights reserved.<BR>
>>     Copyright (c) 2015, Hisilicon Limited. All rights reserved.<BR>
>> @@ -170,6 +170,7 @@ SMBIOS_MISC_TABLE_FUNCTION (MiscBiosVendor) {
>>     EFI_STRING_ID       TokenToGet;
>>     SMBIOS_TABLE_TYPE0  *SmbiosRecord;
>>     SMBIOS_TABLE_TYPE0  *InputData;
>> +  CHAR16              *DefaultVersionString;
>>       //
>>     // First check for invalid parameters.
>> @@ -187,17 +188,30 @@ SMBIOS_MISC_TABLE_FUNCTION (MiscBiosVendor) {
>>       HiiSetString (mSmbiosMiscHiiHandle, TokenToUpdate, Vendor, NULL);
>>     }
>>   -  Version = GetBiosVersion ();
>> +  DefaultVersionString = HiiGetString (
>> +                           mSmbiosMiscHiiHandle,
>> +                           STRING_TOKEN (STR_MISC_BIOS_VERSION),
>> +                           NULL
>> +                           );
>>   -  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
>> -      );
>> +  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 = GetBiosVersion ();
>> +    if (StrLen (Version) > 0) {
>> +      TokenToUpdate = STRING_TOKEN (STR_MISC_BIOS_VERSION);
>> +      HiiSetString (mSmbiosMiscHiiHandle, TokenToUpdate, Version, 
>> NULL);
>> +    }
>>     }
>>       Char16String = GetBiosReleaseDate ();
>
>
> 
>
>