Re: [PATCH 1/6] ArmPkg/ProcessorSubClassDxe: Get processor version from OemMiscLib

["Sami Mujawar" <sami.mujawar@arm.com>]

Hi Nhi,

Thank you for this patch.

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar

On 23/08/2022 11:59 am, Nhi Pham wrote:
> From: Minh Nguyen <minhn@amperecomputing.com>
>
> In some scenarios, the processor version may be updated dynamically
> from pre-UEFI firmware during booting. But the processor version is
> fixed with PCD (PcdProcessorVersion), so it can not be updated it
> dynamically. This patch will support setting that value both
> statically and dynamically.
>
> Signed-off-by: Nhi Pham <nhi@os.amperecomputing.com>
> ---
>   ArmPkg/Include/Library/OemMiscLib.h                              | 2 ++
>   ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c | 8 +++++++-
>   2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/ArmPkg/Include/Library/OemMiscLib.h b/ArmPkg/Include/Library/OemMiscLib.h
> index 47cb30d84a62..330bb4b014de 100644
> --- a/ArmPkg/Include/Library/OemMiscLib.h
> +++ b/ArmPkg/Include/Library/OemMiscLib.h
> @@ -1,5 +1,6 @@
>   /** @file
>
>   *
>
> +*  Copyright (c) 2022, Ampere Computing LLC. All rights reserved.
>
>   *  Copyright (c) 2021, NUVIA Inc. All rights reserved.
>
>   *  Copyright (c) 2015, Hisilicon Limited. All rights reserved.
>
>   *  Copyright (c) 2015, Linaro Limited. All rights reserved.
>
> @@ -58,6 +59,7 @@ typedef enum {
>     SkuNumberType03,
>
>     ProcessorPartNumType04,
>
>     ProcessorSerialNumType04,
>
> +  ProcessorVersionType04,
>
>     SmbiosHiiStringFieldMax
>
>   } OEM_MISC_SMBIOS_HII_STRING_FIELD;
>
>   
>
> diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
> index 0b9af9bd7e1c..524b37b10479 100644
> --- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
> +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
> @@ -1,6 +1,7 @@
>   /** @file
>
>     ProcessorSubClass.c
>
>   
>
> +  Copyright (c) 2022, Ampere Computing LLC. All rights reserved.
>
>     Copyright (c) 2021, NUVIA Inc. All rights reserved.<BR>
>
>     Copyright (c) 2015, Hisilicon Limited. All rights reserved.
>
>     Copyright (c) 2015, Linaro Limited. All rights reserved.
>
> @@ -512,7 +513,6 @@ AllocateType4AndSetProcessorInformationStrings (
>     PartNumber       = STRING_TOKEN (STR_PROCESSOR_PART_NUMBER);
>
>   
>
>     SET_HII_STRING_IF_PCD_NOT_EMPTY (PcdProcessorManufacturer, ProcessorManu);
>
> -  SET_HII_STRING_IF_PCD_NOT_EMPTY (PcdProcessorVersion, ProcessorVersion);
>
>     SET_HII_STRING_IF_PCD_NOT_EMPTY (PcdProcessorAssetTag, AssetTag);
>
>   
>
>     if (StrLen ((CHAR16 *)FixedPcdGetPtr (PcdProcessorSerialNumber)) > 0) {
>
> @@ -527,6 +527,12 @@ AllocateType4AndSetProcessorInformationStrings (
>       OemUpdateSmbiosInfo (mHiiHandle, PartNumber, ProcessorPartNumType04);
>
>     }
>
>   
>
> +  if (StrLen ((CHAR16 *)FixedPcdGetPtr (PcdProcessorVersion)) > 0) {
>
> +    HiiSetString (mHiiHandle, PartNumber, (CHAR16 *)FixedPcdGetPtr (PcdProcessorVersion), NULL);
[SAMI] Should the second parameter be ProcessorVersion?
>
> +  } else {
>
> +    OemUpdateSmbiosInfo (mHiiHandle, ProcessorVersion, ProcessorVersionType04);
>
> +  }
>
> +
>
>     // Processor Designation
>
>     StringBufferSize = sizeof (CHAR16) * SMBIOS_STRING_MAX_LENGTH;
>
>     ProcessorStr     = AllocateZeroPool (StringBufferSize);
>