From: Minh Nguyen <minhn@amperecomputing.com>
In some scenarios, the information of Bios Version, Bios Release
and Embedded Controller Firmware Release are fetched during UEFI
booting. This patch supports updating those fields dynamically
when the PCDs are empty.
Signed-off-by: Nhi Pham <nhi@os.amperecomputing.com>
Reviewed-by: Rebecca Cran <rebecca@quicinc.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
---
ArmPkg/Include/Library/OemMiscLib.h | 21 +++++++++++++
ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c | 28 +++++++++++++++++
ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c | 32 +++++++++++++-------
3 files changed, 70 insertions(+), 11 deletions(-)
diff --git a/ArmPkg/Include/Library/OemMiscLib.h b/ArmPkg/Include/Library/OemMiscLib.h
index 1936619d9b5b..541274999e5c 100644
--- a/ArmPkg/Include/Library/OemMiscLib.h
+++ b/ArmPkg/Include/Library/OemMiscLib.h
@@ -37,6 +37,7 @@ typedef struct {
} OEM_MISC_PROCESSOR_DATA;
typedef enum {
+ BiosVersionType00,
ProductNameType01,
SerialNumType01,
UuidType01,
@@ -247,4 +248,24 @@ OemGetSystemUuid (
OUT GUID *SystemUuid
);
+/** Fetches the BIOS release.
+
+ @return The BIOS release.
+**/
+UINT16
+EFIAPI
+OemGetBiosRelease (
+ VOID
+ );
+
+/** Fetches the embedded controller firmware release.
+
+ @return The embedded controller firmware release.
+**/
+UINT16
+EFIAPI
+OemGetEmbeddedControllerFirmwareRelease (
+ VOID
+ );
+
#endif // OEM_MISC_LIB_H_
diff --git a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
index 32f6d55c1a9a..788ccab9e8c1 100644
--- a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
+++ b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
@@ -254,3 +254,31 @@ OemGetSystemUuid (
ASSERT (FALSE);
CopyGuid (SystemUuid, &gZeroGuid);
}
+
+/** Fetches the BIOS release.
+
+ @return The BIOS release.
+**/
+UINT16
+EFIAPI
+OemGetBiosRelease (
+ VOID
+ )
+{
+ ASSERT (FALSE);
+ return 0xFFFF;
+}
+
+/** Fetches the embedded controller firmware release.
+
+ @return The embedded controller firmware release.
+**/
+UINT16
+EFIAPI
+OemGetEmbeddedControllerFirmwareRelease (
+ VOID
+ )
+{
+ ASSERT (FALSE);
+ return 0xFFFF;
+}
diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
index b49c4b754cab..e9106a8a2fec 100644
--- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
+++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
@@ -1,5 +1,6 @@
/** @file
+ Copyright (c) 2022, 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>
@@ -13,6 +14,7 @@
#include <Library/DebugLib.h>
#include <Library/HiiLib.h>
#include <Library/MemoryAllocationLib.h>
+#include <Library/OemMiscLib.h>
#include <Library/PrintLib.h>
#include <Library/UefiBootServicesTableLib.h>
@@ -191,11 +193,11 @@ SMBIOS_MISC_TABLE_FUNCTION (MiscBiosVendor) {
TokenToUpdate = STRING_TOKEN (STR_MISC_BIOS_VERSION);
HiiSetString (mSmbiosMiscHiiHandle, TokenToUpdate, Version, NULL);
} else {
- Version = (CHAR16 *)PcdGetPtr (PcdFirmwareVersionString);
- if (StrLen (Version) > 0) {
- TokenToUpdate = STRING_TOKEN (STR_MISC_BIOS_VERSION);
- HiiSetString (mSmbiosMiscHiiHandle, TokenToUpdate, Version, NULL);
- }
+ OemUpdateSmbiosInfo (
+ mSmbiosMiscHiiHandle,
+ STRING_TOKEN (STR_MISC_BIOS_VERSION),
+ BiosVersionType00
+ );
}
Char16String = GetBiosReleaseDate ();
@@ -251,13 +253,21 @@ SMBIOS_MISC_TABLE_FUNCTION (MiscBiosVendor) {
}
}
- SmbiosRecord->SystemBiosMajorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) >> 8);
- SmbiosRecord->SystemBiosMinorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) & 0xFF);
+ if (PcdGet16 (PcdSystemBiosRelease) != 0xFFFF) {
+ SmbiosRecord->SystemBiosMajorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) >> 8);
+ SmbiosRecord->SystemBiosMinorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) & 0xFF);
+ } else {
+ SmbiosRecord->SystemBiosMajorRelease = (UINT8)(OemGetBiosRelease () >> 8);
+ SmbiosRecord->SystemBiosMinorRelease = (UINT8)(OemGetBiosRelease () & 0xFF);
+ }
- SmbiosRecord->EmbeddedControllerFirmwareMajorRelease = (UINT16)
- (PcdGet16 (PcdEmbeddedControllerFirmwareRelease) >> 8);
- SmbiosRecord->EmbeddedControllerFirmwareMinorRelease = (UINT16)
- (PcdGet16 (PcdEmbeddedControllerFirmwareRelease) & 0xFF);
+ if (PcdGet16 (PcdEmbeddedControllerFirmwareRelease) != 0xFFFF) {
+ SmbiosRecord->EmbeddedControllerFirmwareMajorRelease = (UINT16)(PcdGet16 (PcdEmbeddedControllerFirmwareRelease) >> 8);
+ SmbiosRecord->EmbeddedControllerFirmwareMinorRelease = (UINT16)(PcdGet16 (PcdEmbeddedControllerFirmwareRelease) & 0xFF);
+ } else {
+ SmbiosRecord->EmbeddedControllerFirmwareMajorRelease = (UINT16)(OemGetEmbeddedControllerFirmwareRelease () >> 8);
+ SmbiosRecord->EmbeddedControllerFirmwareMinorRelease = (UINT16)(OemGetEmbeddedControllerFirmwareRelease () & 0xFF);
+ }
OptionalStrStart = (CHAR8 *)(SmbiosRecord + 1);
UnicodeStrToAsciiStrS (Vendor, OptionalStrStart, VendorStrLen + 1);
--
2.25.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#93705): https://edk2.groups.io/g/devel/message/93705
Mute This Topic: https://groups.io/mt/93650292/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Tue, Sep 13, 2022 at 13:19:47 +0700, Nhi Pham wrote:
> From: Minh Nguyen <minhn@amperecomputing.com>
>
> In some scenarios, the information of Bios Version, Bios Release
> and Embedded Controller Firmware Release are fetched during UEFI
> booting. This patch supports updating those fields dynamically
> when the PCDs are empty.
>
> Signed-off-by: Nhi Pham <nhi@os.amperecomputing.com>
> Reviewed-by: Rebecca Cran <rebecca@quicinc.com>
> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> Acked-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> ArmPkg/Include/Library/OemMiscLib.h | 21 +++++++++++++
> ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c | 28 +++++++++++++++++
> ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c | 32 +++++++++++++-------
> 3 files changed, 70 insertions(+), 11 deletions(-)
>
> diff --git a/ArmPkg/Include/Library/OemMiscLib.h b/ArmPkg/Include/Library/OemMiscLib.h
> index 1936619d9b5b..541274999e5c 100644
> --- a/ArmPkg/Include/Library/OemMiscLib.h
> +++ b/ArmPkg/Include/Library/OemMiscLib.h
> @@ -37,6 +37,7 @@ typedef struct {
> } OEM_MISC_PROCESSOR_DATA;
>
> typedef enum {
> + BiosVersionType00,
> ProductNameType01,
> SerialNumType01,
> UuidType01,
> @@ -247,4 +248,24 @@ OemGetSystemUuid (
> OUT GUID *SystemUuid
> );
>
> +/** Fetches the BIOS release.
> +
> + @return The BIOS release.
> +**/
> +UINT16
> +EFIAPI
> +OemGetBiosRelease (
> + VOID
> + );
> +
> +/** Fetches the embedded controller firmware release.
> +
> + @return The embedded controller firmware release.
> +**/
> +UINT16
> +EFIAPI
> +OemGetEmbeddedControllerFirmwareRelease (
> + VOID
> + );
> +
> #endif // OEM_MISC_LIB_H_
> diff --git a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> index 32f6d55c1a9a..788ccab9e8c1 100644
> --- a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> +++ b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> @@ -254,3 +254,31 @@ OemGetSystemUuid (
> ASSERT (FALSE);
> CopyGuid (SystemUuid, &gZeroGuid);
> }
> +
> +/** Fetches the BIOS release.
> +
> + @return The BIOS release.
> +**/
> +UINT16
> +EFIAPI
> +OemGetBiosRelease (
> + VOID
> + )
> +{
> + ASSERT (FALSE);
> + return 0xFFFF;
This is a change in behaviour.
The pre-existing behaviour would be preserved by returning the value
of PcdGet16 (PcdSystemBiosRelease), which defaults to 0xFFFF.
> +}
> +
> +/** Fetches the embedded controller firmware release.
> +
> + @return The embedded controller firmware release.
> +**/
> +UINT16
> +EFIAPI
> +OemGetEmbeddedControllerFirmwareRelease (
> + VOID
> + )
> +{
> + ASSERT (FALSE);
> + return 0xFFFF;
Same as above, but PcdEmbeddedControllerFirmwareRelease.
No other comments on this set.
(Feel free to see that as Acked-by: Leif Lindholm <quic_llindhol@quicinc.com>
for 1-5/6, but you already have the tags you need for those.)
/
Leif
> +}
> diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
> index b49c4b754cab..e9106a8a2fec 100644
> --- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
> +++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
> @@ -1,5 +1,6 @@
> /** @file
>
> + Copyright (c) 2022, 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>
> @@ -13,6 +14,7 @@
> #include <Library/DebugLib.h>
> #include <Library/HiiLib.h>
> #include <Library/MemoryAllocationLib.h>
> +#include <Library/OemMiscLib.h>
> #include <Library/PrintLib.h>
> #include <Library/UefiBootServicesTableLib.h>
>
> @@ -191,11 +193,11 @@ SMBIOS_MISC_TABLE_FUNCTION (MiscBiosVendor) {
> TokenToUpdate = STRING_TOKEN (STR_MISC_BIOS_VERSION);
> HiiSetString (mSmbiosMiscHiiHandle, TokenToUpdate, Version, NULL);
> } else {
> - Version = (CHAR16 *)PcdGetPtr (PcdFirmwareVersionString);
> - if (StrLen (Version) > 0) {
> - TokenToUpdate = STRING_TOKEN (STR_MISC_BIOS_VERSION);
> - HiiSetString (mSmbiosMiscHiiHandle, TokenToUpdate, Version, NULL);
> - }
> + OemUpdateSmbiosInfo (
> + mSmbiosMiscHiiHandle,
> + STRING_TOKEN (STR_MISC_BIOS_VERSION),
> + BiosVersionType00
> + );
> }
>
> Char16String = GetBiosReleaseDate ();
> @@ -251,13 +253,21 @@ SMBIOS_MISC_TABLE_FUNCTION (MiscBiosVendor) {
> }
> }
>
> - SmbiosRecord->SystemBiosMajorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) >> 8);
> - SmbiosRecord->SystemBiosMinorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) & 0xFF);
> + if (PcdGet16 (PcdSystemBiosRelease) != 0xFFFF) {
> + SmbiosRecord->SystemBiosMajorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) >> 8);
> + SmbiosRecord->SystemBiosMinorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) & 0xFF);
> + } else {
> + SmbiosRecord->SystemBiosMajorRelease = (UINT8)(OemGetBiosRelease () >> 8);
> + SmbiosRecord->SystemBiosMinorRelease = (UINT8)(OemGetBiosRelease () & 0xFF);
> + }
>
> - SmbiosRecord->EmbeddedControllerFirmwareMajorRelease = (UINT16)
> - (PcdGet16 (PcdEmbeddedControllerFirmwareRelease) >> 8);
> - SmbiosRecord->EmbeddedControllerFirmwareMinorRelease = (UINT16)
> - (PcdGet16 (PcdEmbeddedControllerFirmwareRelease) & 0xFF);
> + if (PcdGet16 (PcdEmbeddedControllerFirmwareRelease) != 0xFFFF) {
> + SmbiosRecord->EmbeddedControllerFirmwareMajorRelease = (UINT16)(PcdGet16 (PcdEmbeddedControllerFirmwareRelease) >> 8);
> + SmbiosRecord->EmbeddedControllerFirmwareMinorRelease = (UINT16)(PcdGet16 (PcdEmbeddedControllerFirmwareRelease) & 0xFF);
> + } else {
> + SmbiosRecord->EmbeddedControllerFirmwareMajorRelease = (UINT16)(OemGetEmbeddedControllerFirmwareRelease () >> 8);
> + SmbiosRecord->EmbeddedControllerFirmwareMinorRelease = (UINT16)(OemGetEmbeddedControllerFirmwareRelease () & 0xFF);
> + }
>
> OptionalStrStart = (CHAR8 *)(SmbiosRecord + 1);
> UnicodeStrToAsciiStrS (Vendor, OptionalStrStart, VendorStrLen + 1);
> --
> 2.25.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#93821): https://edk2.groups.io/g/devel/message/93821
Mute This Topic: https://groups.io/mt/93650292/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Thanks Leif. I will fix as your suggestion.
-Nhi
On 9/15/2022 5:54 PM, Leif Lindholm wrote:
> On Tue, Sep 13, 2022 at 13:19:47 +0700, Nhi Pham wrote:
>> From: Minh Nguyen <minhn@amperecomputing.com>
>>
>> In some scenarios, the information of Bios Version, Bios Release
>> and Embedded Controller Firmware Release are fetched during UEFI
>> booting. This patch supports updating those fields dynamically
>> when the PCDs are empty.
>>
>> Signed-off-by: Nhi Pham <nhi@os.amperecomputing.com>
>> Reviewed-by: Rebecca Cran <rebecca@quicinc.com>
>> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
>> Acked-by: Ard Biesheuvel <ardb@kernel.org>
>> ---
>> ArmPkg/Include/Library/OemMiscLib.h | 21 +++++++++++++
>> ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c | 28 +++++++++++++++++
>> ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c | 32 +++++++++++++-------
>> 3 files changed, 70 insertions(+), 11 deletions(-)
>>
>> diff --git a/ArmPkg/Include/Library/OemMiscLib.h b/ArmPkg/Include/Library/OemMiscLib.h
>> index 1936619d9b5b..541274999e5c 100644
>> --- a/ArmPkg/Include/Library/OemMiscLib.h
>> +++ b/ArmPkg/Include/Library/OemMiscLib.h
>> @@ -37,6 +37,7 @@ typedef struct {
>> } OEM_MISC_PROCESSOR_DATA;
>>
>> typedef enum {
>> + BiosVersionType00,
>> ProductNameType01,
>> SerialNumType01,
>> UuidType01,
>> @@ -247,4 +248,24 @@ OemGetSystemUuid (
>> OUT GUID *SystemUuid
>> );
>>
>> +/** Fetches the BIOS release.
>> +
>> + @return The BIOS release.
>> +**/
>> +UINT16
>> +EFIAPI
>> +OemGetBiosRelease (
>> + VOID
>> + );
>> +
>> +/** Fetches the embedded controller firmware release.
>> +
>> + @return The embedded controller firmware release.
>> +**/
>> +UINT16
>> +EFIAPI
>> +OemGetEmbeddedControllerFirmwareRelease (
>> + VOID
>> + );
>> +
>> #endif // OEM_MISC_LIB_H_
>> diff --git a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
>> index 32f6d55c1a9a..788ccab9e8c1 100644
>> --- a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
>> +++ b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
>> @@ -254,3 +254,31 @@ OemGetSystemUuid (
>> ASSERT (FALSE);
>> CopyGuid (SystemUuid, &gZeroGuid);
>> }
>> +
>> +/** Fetches the BIOS release.
>> +
>> + @return The BIOS release.
>> +**/
>> +UINT16
>> +EFIAPI
>> +OemGetBiosRelease (
>> + VOID
>> + )
>> +{
>> + ASSERT (FALSE);
>> + return 0xFFFF;
> This is a change in behaviour.
> The pre-existing behaviour would be preserved by returning the value
> of PcdGet16 (PcdSystemBiosRelease), which defaults to 0xFFFF.
>
>> +}
>> +
>> +/** Fetches the embedded controller firmware release.
>> +
>> + @return The embedded controller firmware release.
>> +**/
>> +UINT16
>> +EFIAPI
>> +OemGetEmbeddedControllerFirmwareRelease (
>> + VOID
>> + )
>> +{
>> + ASSERT (FALSE);
>> + return 0xFFFF;
> Same as above, but PcdEmbeddedControllerFirmwareRelease.
>
> No other comments on this set.
> (Feel free to see that as Acked-by: Leif Lindholm <quic_llindhol@quicinc.com>
> for 1-5/6, but you already have the tags you need for those.)
>
> /
> Leif
>
>> +}
>> diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
>> index b49c4b754cab..e9106a8a2fec 100644
>> --- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
>> +++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
>> @@ -1,5 +1,6 @@
>> /** @file
>>
>> + Copyright (c) 2022, 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>
>> @@ -13,6 +14,7 @@
>> #include <Library/DebugLib.h>
>> #include <Library/HiiLib.h>
>> #include <Library/MemoryAllocationLib.h>
>> +#include <Library/OemMiscLib.h>
>> #include <Library/PrintLib.h>
>> #include <Library/UefiBootServicesTableLib.h>
>>
>> @@ -191,11 +193,11 @@ SMBIOS_MISC_TABLE_FUNCTION (MiscBiosVendor) {
>> TokenToUpdate = STRING_TOKEN (STR_MISC_BIOS_VERSION);
>> HiiSetString (mSmbiosMiscHiiHandle, TokenToUpdate, Version, NULL);
>> } else {
>> - Version = (CHAR16 *)PcdGetPtr (PcdFirmwareVersionString);
>> - if (StrLen (Version) > 0) {
>> - TokenToUpdate = STRING_TOKEN (STR_MISC_BIOS_VERSION);
>> - HiiSetString (mSmbiosMiscHiiHandle, TokenToUpdate, Version, NULL);
>> - }
>> + OemUpdateSmbiosInfo (
>> + mSmbiosMiscHiiHandle,
>> + STRING_TOKEN (STR_MISC_BIOS_VERSION),
>> + BiosVersionType00
>> + );
>> }
>>
>> Char16String = GetBiosReleaseDate ();
>> @@ -251,13 +253,21 @@ SMBIOS_MISC_TABLE_FUNCTION (MiscBiosVendor) {
>> }
>> }
>>
>> - SmbiosRecord->SystemBiosMajorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) >> 8);
>> - SmbiosRecord->SystemBiosMinorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) & 0xFF);
>> + if (PcdGet16 (PcdSystemBiosRelease) != 0xFFFF) {
>> + SmbiosRecord->SystemBiosMajorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) >> 8);
>> + SmbiosRecord->SystemBiosMinorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) & 0xFF);
>> + } else {
>> + SmbiosRecord->SystemBiosMajorRelease = (UINT8)(OemGetBiosRelease () >> 8);
>> + SmbiosRecord->SystemBiosMinorRelease = (UINT8)(OemGetBiosRelease () & 0xFF);
>> + }
>>
>> - SmbiosRecord->EmbeddedControllerFirmwareMajorRelease = (UINT16)
>> - (PcdGet16 (PcdEmbeddedControllerFirmwareRelease) >> 8);
>> - SmbiosRecord->EmbeddedControllerFirmwareMinorRelease = (UINT16)
>> - (PcdGet16 (PcdEmbeddedControllerFirmwareRelease) & 0xFF);
>> + if (PcdGet16 (PcdEmbeddedControllerFirmwareRelease) != 0xFFFF) {
>> + SmbiosRecord->EmbeddedControllerFirmwareMajorRelease = (UINT16)(PcdGet16 (PcdEmbeddedControllerFirmwareRelease) >> 8);
>> + SmbiosRecord->EmbeddedControllerFirmwareMinorRelease = (UINT16)(PcdGet16 (PcdEmbeddedControllerFirmwareRelease) & 0xFF);
>> + } else {
>> + SmbiosRecord->EmbeddedControllerFirmwareMajorRelease = (UINT16)(OemGetEmbeddedControllerFirmwareRelease () >> 8);
>> + SmbiosRecord->EmbeddedControllerFirmwareMinorRelease = (UINT16)(OemGetEmbeddedControllerFirmwareRelease () & 0xFF);
>> + }
>>
>> OptionalStrStart = (CHAR8 *)(SmbiosRecord + 1);
>> UnicodeStrToAsciiStrS (Vendor, OptionalStrStart, VendorStrLen + 1);
>> --
>> 2.25.1
>>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#93858): https://edk2.groups.io/g/devel/message/93858
Mute This Topic: https://groups.io/mt/93650292/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Nhi,
Please see my response inline marked [SAMI].
Regards,
Sami Mujawar
On 15/09/2022, 19:23, "Nhi Pham" <nhi@amperemail.onmicrosoft.com> wrote:
Thanks Leif. I will fix as your suggestion.
-Nhi
On 9/15/2022 5:54 PM, Leif Lindholm wrote:
> On Tue, Sep 13, 2022 at 13:19:47 +0700, Nhi Pham wrote:
>> From: Minh Nguyen <minhn@amperecomputing.com>
>>
>> In some scenarios, the information of Bios Version, Bios Release
>> and Embedded Controller Firmware Release are fetched during UEFI
>> booting. This patch supports updating those fields dynamically
>> when the PCDs are empty.
>>
>> Signed-off-by: Nhi Pham <nhi@os.amperecomputing.com>
>> Reviewed-by: Rebecca Cran <rebecca@quicinc.com>
>> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
>> Acked-by: Ard Biesheuvel <ardb@kernel.org>
>> ---
>> ArmPkg/Include/Library/OemMiscLib.h | 21 +++++++++++++
>> ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c | 28 +++++++++++++++++
>> ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c | 32 +++++++++++++-------
>> 3 files changed, 70 insertions(+), 11 deletions(-)
>>
>> diff --git a/ArmPkg/Include/Library/OemMiscLib.h b/ArmPkg/Include/Library/OemMiscLib.h
>> index 1936619d9b5b..541274999e5c 100644
>> --- a/ArmPkg/Include/Library/OemMiscLib.h
>> +++ b/ArmPkg/Include/Library/OemMiscLib.h
>> @@ -37,6 +37,7 @@ typedef struct {
>> } OEM_MISC_PROCESSOR_DATA;
>>
>> typedef enum {
>> + BiosVersionType00,
>> ProductNameType01,
>> SerialNumType01,
>> UuidType01,
>> @@ -247,4 +248,24 @@ OemGetSystemUuid (
>> OUT GUID *SystemUuid
>> );
>>
>> +/** Fetches the BIOS release.
>> +
>> + @return The BIOS release.
>> +**/
>> +UINT16
>> +EFIAPI
>> +OemGetBiosRelease (
>> + VOID
>> + );
>> +
>> +/** Fetches the embedded controller firmware release.
>> +
>> + @return The embedded controller firmware release.
>> +**/
>> +UINT16
>> +EFIAPI
>> +OemGetEmbeddedControllerFirmwareRelease (
>> + VOID
>> + );
>> +
>> #endif // OEM_MISC_LIB_H_
>> diff --git a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
>> index 32f6d55c1a9a..788ccab9e8c1 100644
>> --- a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
>> +++ b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
>> @@ -254,3 +254,31 @@ OemGetSystemUuid (
>> ASSERT (FALSE);
>> CopyGuid (SystemUuid, &gZeroGuid);
>> }
>> +
>> +/** Fetches the BIOS release.
>> +
>> + @return The BIOS release.
>> +**/
>> +UINT16
>> +EFIAPI
>> +OemGetBiosRelease (
>> + VOID
>> + )
>> +{
>> + ASSERT (FALSE);
>> + return 0xFFFF;
> This is a change in behaviour.
> The pre-existing behaviour would be preserved by returning the value
> of PcdGet16 (PcdSystemBiosRelease), which defaults to 0xFFFF.
>
>> +}
>> +
>> +/** Fetches the embedded controller firmware release.
>> +
>> + @return The embedded controller firmware release.
>> +**/
>> +UINT16
>> +EFIAPI
>> +OemGetEmbeddedControllerFirmwareRelease (
>> + VOID
>> + )
>> +{
>> + ASSERT (FALSE);
>> + return 0xFFFF;
> Same as above, but PcdEmbeddedControllerFirmwareRelease.
>
> No other comments on this set.
> (Feel free to see that as Acked-by: Leif Lindholm <quic_llindhol@quicinc.com>
> for 1-5/6, but you already have the tags you need for those.)
>
> /
> Leif
>
>> +}
>> diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
>> index b49c4b754cab..e9106a8a2fec 100644
>> --- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
>> +++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
>> @@ -1,5 +1,6 @@
>> /** @file
>>
>> + Copyright (c) 2022, 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>
>> @@ -13,6 +14,7 @@
>> #include <Library/DebugLib.h>
>> #include <Library/HiiLib.h>
>> #include <Library/MemoryAllocationLib.h>
>> +#include <Library/OemMiscLib.h>
>> #include <Library/PrintLib.h>
>> #include <Library/UefiBootServicesTableLib.h>
>>
>> @@ -191,11 +193,11 @@ SMBIOS_MISC_TABLE_FUNCTION (MiscBiosVendor) {
>> TokenToUpdate = STRING_TOKEN (STR_MISC_BIOS_VERSION);
>> HiiSetString (mSmbiosMiscHiiHandle, TokenToUpdate, Version, NULL);
>> } else {
>> - Version = (CHAR16 *)PcdGetPtr (PcdFirmwareVersionString);
>> - if (StrLen (Version) > 0) {
>> - TokenToUpdate = STRING_TOKEN (STR_MISC_BIOS_VERSION);
>> - HiiSetString (mSmbiosMiscHiiHandle, TokenToUpdate, Version, NULL);
>> - }
>> + OemUpdateSmbiosInfo (
>> + mSmbiosMiscHiiHandle,
>> + STRING_TOKEN (STR_MISC_BIOS_VERSION),
>> + BiosVersionType00
>> + );
>> }
>>
>> Char16String = GetBiosReleaseDate ();
>> @@ -251,13 +253,21 @@ SMBIOS_MISC_TABLE_FUNCTION (MiscBiosVendor) {
>> }
>> }
>>
>> - SmbiosRecord->SystemBiosMajorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) >> 8);
>> - SmbiosRecord->SystemBiosMinorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) & 0xFF);
>> + if (PcdGet16 (PcdSystemBiosRelease) != 0xFFFF) {
>> + SmbiosRecord->SystemBiosMajorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) >> 8);
>> + SmbiosRecord->SystemBiosMinorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) & 0xFF);
[SAMI] Considering that ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c will be updated to use PcdSystemBiosRelease,
can you check whether the 'if' code block above is required, please?
[/SAMI]
>> + } else {
>> + SmbiosRecord->SystemBiosMajorRelease = (UINT8)(OemGetBiosRelease () >> 8);
>> + SmbiosRecord->SystemBiosMinorRelease = (UINT8)(OemGetBiosRelease () & 0xFF);
>> + }
>>
>> - SmbiosRecord->EmbeddedControllerFirmwareMajorRelease = (UINT16)
>> - (PcdGet16 (PcdEmbeddedControllerFirmwareRelease) >> 8);
>> - SmbiosRecord->EmbeddedControllerFirmwareMinorRelease = (UINT16)
>> - (PcdGet16 (PcdEmbeddedControllerFirmwareRelease) & 0xFF);
>> + if (PcdGet16 (PcdEmbeddedControllerFirmwareRelease) != 0xFFFF) {
>> + SmbiosRecord->EmbeddedControllerFirmwareMajorRelease = (UINT16)(PcdGet16 (PcdEmbeddedControllerFirmwareRelease) >> 8);
>> + SmbiosRecord->EmbeddedControllerFirmwareMinorRelease = (UINT16)(PcdGet16 (PcdEmbeddedControllerFirmwareRelease) & 0xFF);
[SAMI] Similar comment as the previous one.
>> + } else {
>> + SmbiosRecord->EmbeddedControllerFirmwareMajorRelease = (UINT16)(OemGetEmbeddedControllerFirmwareRelease () >> 8);
>> + SmbiosRecord->EmbeddedControllerFirmwareMinorRelease = (UINT16)(OemGetEmbeddedControllerFirmwareRelease () & 0xFF);
>> + }
>>
>> OptionalStrStart = (CHAR8 *)(SmbiosRecord + 1);
>> UnicodeStrToAsciiStrS (Vendor, OptionalStrStart, VendorStrLen + 1);
>> --
>> 2.25.1
>>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#93905): https://edk2.groups.io/g/devel/message/93905
Mute This Topic: https://groups.io/mt/93650292/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Sami,
On 9/16/2022 2:32 PM, Sami Mujawar wrote:
> Hi Nhi,
>
> Please see my response inline marked [SAMI].
>
> Regards,
>
> Sami Mujawar
>
> On 15/09/2022, 19:23, "Nhi Pham" <nhi@amperemail.onmicrosoft.com> wrote:
>
> Thanks Leif. I will fix as your suggestion.
>
> -Nhi
>
> On 9/15/2022 5:54 PM, Leif Lindholm wrote:
> > On Tue, Sep 13, 2022 at 13:19:47 +0700, Nhi Pham wrote:
> >> From: Minh Nguyen <minhn@amperecomputing.com>
> >>
> >> In some scenarios, the information of Bios Version, Bios Release
> >> and Embedded Controller Firmware Release are fetched during UEFI
> >> booting. This patch supports updating those fields dynamically
> >> when the PCDs are empty.
> >>
> >> Signed-off-by: Nhi Pham <nhi@os.amperecomputing.com>
> >> Reviewed-by: Rebecca Cran <rebecca@quicinc.com>
> >> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> >> Acked-by: Ard Biesheuvel <ardb@kernel.org>
> >> ---
> >> ArmPkg/Include/Library/OemMiscLib.h | 21 +++++++++++++
> >> ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c | 28 +++++++++++++++++
> >> ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c | 32 +++++++++++++-------
> >> 3 files changed, 70 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/ArmPkg/Include/Library/OemMiscLib.h b/ArmPkg/Include/Library/OemMiscLib.h
> >> index 1936619d9b5b..541274999e5c 100644
> >> --- a/ArmPkg/Include/Library/OemMiscLib.h
> >> +++ b/ArmPkg/Include/Library/OemMiscLib.h
> >> @@ -37,6 +37,7 @@ typedef struct {
> >> } OEM_MISC_PROCESSOR_DATA;
> >>
> >> typedef enum {
> >> + BiosVersionType00,
> >> ProductNameType01,
> >> SerialNumType01,
> >> UuidType01,
> >> @@ -247,4 +248,24 @@ OemGetSystemUuid (
> >> OUT GUID *SystemUuid
> >> );
> >>
> >> +/** Fetches the BIOS release.
> >> +
> >> + @return The BIOS release.
> >> +**/
> >> +UINT16
> >> +EFIAPI
> >> +OemGetBiosRelease (
> >> + VOID
> >> + );
> >> +
> >> +/** Fetches the embedded controller firmware release.
> >> +
> >> + @return The embedded controller firmware release.
> >> +**/
> >> +UINT16
> >> +EFIAPI
> >> +OemGetEmbeddedControllerFirmwareRelease (
> >> + VOID
> >> + );
> >> +
> >> #endif // OEM_MISC_LIB_H_
> >> diff --git a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> >> index 32f6d55c1a9a..788ccab9e8c1 100644
> >> --- a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> >> +++ b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> >> @@ -254,3 +254,31 @@ OemGetSystemUuid (
> >> ASSERT (FALSE);
> >> CopyGuid (SystemUuid, &gZeroGuid);
> >> }
> >> +
> >> +/** Fetches the BIOS release.
> >> +
> >> + @return The BIOS release.
> >> +**/
> >> +UINT16
> >> +EFIAPI
> >> +OemGetBiosRelease (
> >> + VOID
> >> + )
> >> +{
> >> + ASSERT (FALSE);
> >> + return 0xFFFF;
> > This is a change in behaviour.
> > The pre-existing behaviour would be preserved by returning the value
> > of PcdGet16 (PcdSystemBiosRelease), which defaults to 0xFFFF.
> >
> >> +}
> >> +
> >> +/** Fetches the embedded controller firmware release.
> >> +
> >> + @return The embedded controller firmware release.
> >> +**/
> >> +UINT16
> >> +EFIAPI
> >> +OemGetEmbeddedControllerFirmwareRelease (
> >> + VOID
> >> + )
> >> +{
> >> + ASSERT (FALSE);
> >> + return 0xFFFF;
> > Same as above, but PcdEmbeddedControllerFirmwareRelease.
> >
> > No other comments on this set.
> > (Feel free to see that as Acked-by: Leif Lindholm <quic_llindhol@quicinc.com>
> > for 1-5/6, but you already have the tags you need for those.)
> >
> > /
> > Leif
> >
> >> +}
> >> diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
> >> index b49c4b754cab..e9106a8a2fec 100644
> >> --- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
> >> +++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
> >> @@ -1,5 +1,6 @@
> >> /** @file
> >>
> >> + Copyright (c) 2022, 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>
> >> @@ -13,6 +14,7 @@
> >> #include <Library/DebugLib.h>
> >> #include <Library/HiiLib.h>
> >> #include <Library/MemoryAllocationLib.h>
> >> +#include <Library/OemMiscLib.h>
> >> #include <Library/PrintLib.h>
> >> #include <Library/UefiBootServicesTableLib.h>
> >>
> >> @@ -191,11 +193,11 @@ SMBIOS_MISC_TABLE_FUNCTION (MiscBiosVendor) {
> >> TokenToUpdate = STRING_TOKEN (STR_MISC_BIOS_VERSION);
> >> HiiSetString (mSmbiosMiscHiiHandle, TokenToUpdate, Version, NULL);
> >> } else {
> >> - Version = (CHAR16 *)PcdGetPtr (PcdFirmwareVersionString);
> >> - if (StrLen (Version) > 0) {
> >> - TokenToUpdate = STRING_TOKEN (STR_MISC_BIOS_VERSION);
> >> - HiiSetString (mSmbiosMiscHiiHandle, TokenToUpdate, Version, NULL);
> >> - }
> >> + OemUpdateSmbiosInfo (
> >> + mSmbiosMiscHiiHandle,
> >> + STRING_TOKEN (STR_MISC_BIOS_VERSION),
> >> + BiosVersionType00
> >> + );
> >> }
> >>
> >> Char16String = GetBiosReleaseDate ();
> >> @@ -251,13 +253,21 @@ SMBIOS_MISC_TABLE_FUNCTION (MiscBiosVendor) {
> >> }
> >> }
> >>
> >> - SmbiosRecord->SystemBiosMajorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) >> 8);
> >> - SmbiosRecord->SystemBiosMinorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) & 0xFF);
> >> + if (PcdGet16 (PcdSystemBiosRelease) != 0xFFFF) {
> >> + SmbiosRecord->SystemBiosMajorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) >> 8);
> >> + SmbiosRecord->SystemBiosMinorRelease = (UINT8)(PcdGet16 (PcdSystemBiosRelease) & 0xFF);
>
> [SAMI] Considering that ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c will be updated to use PcdSystemBiosRelease,
> can you check whether the 'if' code block above is required, please?
> [/SAMI]
That's really a great point. I think the if statement above is not
needed as the OemMiscLib can do what they want.
I will update the patch for this.
Thanks,
Nhi
>
> >> + } else {
> >> + SmbiosRecord->SystemBiosMajorRelease = (UINT8)(OemGetBiosRelease () >> 8);
> >> + SmbiosRecord->SystemBiosMinorRelease = (UINT8)(OemGetBiosRelease () & 0xFF);
> >> + }
> >>
> >> - SmbiosRecord->EmbeddedControllerFirmwareMajorRelease = (UINT16)
> >> - (PcdGet16 (PcdEmbeddedControllerFirmwareRelease) >> 8);
> >> - SmbiosRecord->EmbeddedControllerFirmwareMinorRelease = (UINT16)
> >> - (PcdGet16 (PcdEmbeddedControllerFirmwareRelease) & 0xFF);
> >> + if (PcdGet16 (PcdEmbeddedControllerFirmwareRelease) != 0xFFFF) {
> >> + SmbiosRecord->EmbeddedControllerFirmwareMajorRelease = (UINT16)(PcdGet16 (PcdEmbeddedControllerFirmwareRelease) >> 8);
> >> + SmbiosRecord->EmbeddedControllerFirmwareMinorRelease = (UINT16)(PcdGet16 (PcdEmbeddedControllerFirmwareRelease) & 0xFF);
>
> [SAMI] Similar comment as the previous one.
>
> >> + } else {
> >> + SmbiosRecord->EmbeddedControllerFirmwareMajorRelease = (UINT16)(OemGetEmbeddedControllerFirmwareRelease () >> 8);
> >> + SmbiosRecord->EmbeddedControllerFirmwareMinorRelease = (UINT16)(OemGetEmbeddedControllerFirmwareRelease () & 0xFF);
> >> + }
> >>
> >> OptionalStrStart = (CHAR8 *)(SmbiosRecord + 1);
> >> UnicodeStrToAsciiStrS (Vendor, OptionalStrStart, VendorStrLen + 1);
> >> --
> >> 2.25.1
> >>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#93907): https://edk2.groups.io/g/devel/message/93907
Mute This Topic: https://groups.io/mt/93650292/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.