* [PATCH v3 1/1] ArmPkg/SmbiosMiscDxe: Adjust the priority of getting firmware version
@ 2023-03-21 3:16 Tinh Nguyen
2023-03-21 16:57 ` [edk2-devel] " Oliver Smith-Denny
2023-03-22 3:38 ` Tinh Nguyen
0 siblings, 2 replies; 5+ messages in thread
From: Tinh Nguyen @ 2023-03-21 3:16 UTC (permalink / raw)
To: devel; +Cc: patches, quic_llindhol, ardb+tianocore, nhi, rebecca, Tinh Nguyen
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 <tinhnguyen@os.amperecomputing.com>
Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
---
Changes since v1:
+ Change GetBiosVersion () to SetBiosVersion () and move the selection logic
fully into SetBiosVersion ().
Changes since v2:
+ Add Reviewed-by: Rebecca Cran <rebecca@bsdio.com> 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.<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>
@@ -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.
- @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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH v3 1/1] ArmPkg/SmbiosMiscDxe: Adjust the priority of getting firmware version
2023-03-21 3:16 [PATCH v3 1/1] ArmPkg/SmbiosMiscDxe: Adjust the priority of getting firmware version Tinh Nguyen
@ 2023-03-21 16:57 ` Oliver Smith-Denny
2023-03-22 3:51 ` Tinh Nguyen
2023-03-22 3:38 ` Tinh Nguyen
1 sibling, 1 reply; 5+ messages in thread
From: Oliver Smith-Denny @ 2023-03-21 16:57 UTC (permalink / raw)
To: devel, tinhnguyen; +Cc: patches, quic_llindhol, ardb+tianocore, nhi, rebecca
One nit below, otherwise:
Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>
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 <tinhnguyen@os.amperecomputing.com>
> Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
> ---
>
> Changes since v1:
> + Change GetBiosVersion () to SetBiosVersion () and move the selection logic
> fully into SetBiosVersion ().
> Changes since v2:
> + Add Reviewed-by: Rebecca Cran <rebecca@bsdio.com> 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.<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>
> @@ -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
>
>
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/1] ArmPkg/SmbiosMiscDxe: Adjust the priority of getting firmware version
2023-03-21 3:16 [PATCH v3 1/1] ArmPkg/SmbiosMiscDxe: Adjust the priority of getting firmware version Tinh Nguyen
2023-03-21 16:57 ` [edk2-devel] " Oliver Smith-Denny
@ 2023-03-22 3:38 ` Tinh Nguyen
2023-03-24 18:29 ` Leif Lindholm
1 sibling, 1 reply; 5+ messages in thread
From: Tinh Nguyen @ 2023-03-22 3:38 UTC (permalink / raw)
To: Tinh Nguyen, devel, Leif Lindholm; +Cc: patches, ardb+tianocore, nhi, rebecca
Hi Leif,
Do you have any concerns? Could I add you to the review list?
Thanks,
Tinh
On 3/21/2023 10:16 AM, 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.
>
> Signed-off-by: Tinh Nguyen <tinhnguyen@os.amperecomputing.com>
> Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
> ---
>
> Changes since v1:
> + Change GetBiosVersion () to SetBiosVersion () and move the selection logic
> fully into SetBiosVersion ().
> Changes since v2:
> + Add Reviewed-by: Rebecca Cran <rebecca@bsdio.com> 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.<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>
> @@ -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.
>
> - @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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH v3 1/1] ArmPkg/SmbiosMiscDxe: Adjust the priority of getting firmware version
2023-03-21 16:57 ` [edk2-devel] " Oliver Smith-Denny
@ 2023-03-22 3:51 ` Tinh Nguyen
0 siblings, 0 replies; 5+ messages in thread
From: Tinh Nguyen @ 2023-03-22 3:51 UTC (permalink / raw)
To: Oliver Smith-Denny, devel, tinhnguyen
Cc: patches, quic_llindhol, ardb+tianocore, nhi, rebecca
Thanks Oliver
will be improved in the next version
- Tinh
On 3/21/2023 11:57 PM, Oliver Smith-Denny wrote:
> One nit below, otherwise:
>
> Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>
>
> 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 <tinhnguyen@os.amperecomputing.com>
>> Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
>> ---
>>
>> Changes since v1:
>> + Change GetBiosVersion () to SetBiosVersion () and move the
>> selection logic
>> fully into SetBiosVersion ().
>> Changes since v2:
>> + Add Reviewed-by: Rebecca Cran <rebecca@bsdio.com> 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.<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>
>> @@ -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
>>
>>
>>
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/1] ArmPkg/SmbiosMiscDxe: Adjust the priority of getting firmware version
2023-03-22 3:38 ` Tinh Nguyen
@ 2023-03-24 18:29 ` Leif Lindholm
0 siblings, 0 replies; 5+ messages in thread
From: Leif Lindholm @ 2023-03-24 18:29 UTC (permalink / raw)
To: Tinh Nguyen; +Cc: Tinh Nguyen, devel, patches, ardb+tianocore, nhi, rebecca
Hi Tinh,
Sorry, was waiting for a v4 based on your last reply to Oliver.
Yes, from my end (with Oliver's feedback addressed):
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
/
Leif
On Wed, Mar 22, 2023 at 10:38:21 +0700, Tinh Nguyen wrote:
> Hi Leif,
>
> Do you have any concerns? Could I add you to the review list?
>
> Thanks,
>
> Tinh
>
> On 3/21/2023 10:16 AM, 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.
> >
> > Signed-off-by: Tinh Nguyen <tinhnguyen@os.amperecomputing.com>
> > Reviewed-by: Rebecca Cran <rebecca@bsdio.com>
> > ---
> >
> > Changes since v1:
> > + Change GetBiosVersion () to SetBiosVersion () and move the selection logic
> > fully into SetBiosVersion ().
> > Changes since v2:
> > + Add Reviewed-by: Rebecca Cran <rebecca@bsdio.com> 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.<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>
> > @@ -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.
> > - @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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-03-24 18:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-21 3:16 [PATCH v3 1/1] ArmPkg/SmbiosMiscDxe: Adjust the priority of getting firmware version Tinh Nguyen
2023-03-21 16:57 ` [edk2-devel] " Oliver Smith-Denny
2023-03-22 3:51 ` Tinh Nguyen
2023-03-22 3:38 ` Tinh Nguyen
2023-03-24 18:29 ` Leif Lindholm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox