Hi Leif,
My comments is in below
On 3/13/2023 10:03 PM, Leif Lindholm wrote:
On Mon, Mar 13, 2023 at 13:43:21 +0700, 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.
Wait. Firmware version modified at boot time for extended information?
What type of extended information?
That could be SCP version, TF-A version, etc.
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 <n...@os.amperecomputing.com>
Signed-off-by: Tinh Nguyen <tinhngu...@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 ();
GetBiosVersion exists as a helper function to avoid cluttering
higher-level functions with unnecessary details. If this change is
needed, that is where it should go.
This change is based on existing code, and goal is to get the Firmware
version string from OemMiscLib first, rather than PcdFirmwareVersionString.
I would rather not make any changes that aren't relevant.
But I still don't understand the purpose of this patch, so cannot
comment on whether I feel this is the right approach or not.
Please elaborate.
The firmware version is currently being updated:
1. Get the string from Fixed PcdFirmwareVersionString.
2. Check to see if this string is null; if not, update the SMBIOS
firmware version string.
3. if PcdFirmwareVersionString is null, retrieve the string from OemMiscLib.
As you can see, the implementation is intended to honor the
PcdFirmwareVersionString.
We can't get the extend information (which can be derived from runtime)
into the SMBIOS firmware version string if we don't set
PcdFirmwareVersionString null
However, in some cases we don't want to set PcdFirmwareVersionString to
null because it might be used by other modules.
I think it makes sense to prioritize OemMiscLib since it's more
flexible than Pcd.
/
Leif
+ 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 ();
--
2.39.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101103): https://edk2.groups.io/g/devel/message/101103
Mute This Topic: https://groups.io/mt/97575576/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
