public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Nhi Pham" <nhi@os.amperecomputing.com>
To: Leif Lindholm <quic_llindhol@quicinc.com>,
	Nhi Pham <nhi@os.amperecomputing.com>
Cc: devel@edk2.groups.io, ardb+tianocore@kernel.org,
	sami.mujawar@arm.com, quic_rcran@quicinc.com,
	patches@amperecomputing.com,
	Minh Nguyen <minhn@amperecomputing.com>,
	Rebecca Cran <rebecca@quicinc.com>,
	Ard Biesheuvel <ardb@kernel.org>
Subject: Re: [PATCH v3 6/6] ArmPkg/SmbiosMiscDxe: Get SMBIOS information from OemMiscLib
Date: Fri, 16 Sep 2022 01:22:54 +0700	[thread overview]
Message-ID: <402b3f32-ee70-565f-914e-0befd706f2c5@amperemail.onmicrosoft.com> (raw)
In-Reply-To: <YyMETbucmbsCpv/I@qc-i7.hemma.eciton.net>

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
>>

  reply	other threads:[~2022-09-15 18:23 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-13  6:19 [PATCH v3 0/6] ArmPkg/SMBIOS fixes and improvements Nhi Pham
2022-09-13  6:19 ` [PATCH v3 1/6] ArmPkg/ProcessorSubClassDxe: Get processor version from OemMiscLib Nhi Pham
2022-09-13  6:19 ` [PATCH v3 2/6] ArmPkg: Correct return value of "SMCCC_ARCH_SOC_ID" Function ID call Nhi Pham
2022-09-13  6:19 ` [PATCH v3 3/6] ArmPkg/SmbiosMiscDxe: Support fetching System UUID Nhi Pham
2022-09-13  6:19 ` [PATCH v3 4/6] ArmPkg/SmbiosMiscDxe: Fix typo of "AssetTagType02" Nhi Pham
2022-09-13  6:19 ` [PATCH v3 5/6] ArmPkg/SmbiosMiscDxe: Remove redundant updates in SMBIOS Type 2 Nhi Pham
2022-09-13  6:19 ` [PATCH v3 6/6] ArmPkg/SmbiosMiscDxe: Get SMBIOS information from OemMiscLib Nhi Pham
2022-09-15 10:54   ` Leif Lindholm
2022-09-15 18:22     ` Nhi Pham [this message]
2022-09-16  7:32       ` Sami Mujawar
2022-09-16 10:33         ` Nhi Pham

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=402b3f32-ee70-565f-914e-0befd706f2c5@amperemail.onmicrosoft.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox