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

["Nhi Pham" <nhi@os.amperecomputing.com>]

Hi Sami,

On 8/23/2022 11:21 PM, Sami Mujawar wrote:
> 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?

Good catch. Will fix it.

Thanks,

Nhi

>>
>> +  } else {
>>
>> +    OemUpdateSmbiosInfo (mHiiHandle, ProcessorVersion, 
>> ProcessorVersionType04);
>>
>> +  }
>>
>> +
>>
>>     // Processor Designation
>>
>>     StringBufferSize = sizeof (CHAR16) * SMBIOS_STRING_MAX_LENGTH;
>>
>>     ProcessorStr     = AllocateZeroPool (StringBufferSize);
>>