Re: [edk2-devel] [PATCH 3/3] ArmPkg/ProcessorSubClassDxe: Get serial and part number from OemMiscLib

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

Hi Nhi,

I have a minor suggestion to use FixedPcdGetPtr(), other than that this 
patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 01/12/2021 04:10 AM, Nhi Pham via groups.io wrote:
> Currently, the serial and part number of a processor are filled with
> fixed PCDs. However, they may be updated dynamically according to the
> information being passed from a the pre-UEFI firmware during booting.
> So, this patch is to support updating these string fields from
> OemMiscLib if the PCDs are empty.
>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Rebecca Cran <rebecca@nuviainc.com>
> Signed-off-by: Nhi Pham <nhi@os.amperecomputing.com>
> ---
>   ArmPkg/Include/Library/OemMiscLib.h                              |  2 ++
>   ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c | 14 ++++++++++++--
>   2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPkg/Include/Library/OemMiscLib.h b/ArmPkg/Include/Library/OemMiscLib.h
> index aa68c0a92632..5f0effa9cc17 100644
> --- a/ArmPkg/Include/Library/OemMiscLib.h
> +++ b/ArmPkg/Include/Library/OemMiscLib.h
> @@ -60,6 +60,8 @@ typedef enum
>       ChassisTypeType03,
>       ManufacturerType03,
>       SkuNumberType03,
> +    ProcessorPartNumType04,
> +    ProcessorSerialNumType04,
>       SmbiosHiiStringFieldMax
>   } OEM_MISC_SMBIOS_HII_STRING_FIELD;
>   
> diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
> index fb484086a457..8d884d313bd4 100644
> --- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
> +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
> @@ -493,9 +493,19 @@ AllocateType4AndSetProcessorInformationStrings (
>   
>     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 (PcdProcessorSerialNumber, SerialNumber);
>     SET_HII_STRING_IF_PCD_NOT_EMPTY (PcdProcessorAssetTag, AssetTag);
> -  SET_HII_STRING_IF_PCD_NOT_EMPTY (PcdProcessorPartNumber, PartNumber);
> +
> +  if (StrLen ((CHAR16 *)PcdGetPtr (PcdProcessorSerialNumber)) > 0) {
> +    HiiSetString (mHiiHandle, SerialNumber, (CHAR16 *)PcdGetPtr (PcdProcessorSerialNumber), NULL);
[SAMI] Can FixedPcdGetPtr() be used here instead of PcdGetPtr()?
> +  } else {
> +    OemUpdateSmbiosInfo (mHiiHandle, SerialNumber, ProcessorSerialNumType04);
> +  }
> +
> +  if (StrLen ((CHAR16 *)PcdGetPtr (PcdProcessorPartNumber)) > 0) {
> +    HiiSetString (mHiiHandle, PartNumber, (CHAR16 *)PcdGetPtr (PcdProcessorPartNumber), NULL);
> +  } else {
> +    OemUpdateSmbiosInfo (mHiiHandle, PartNumber, ProcessorPartNumType04);
> +  }
>   
>     // Processor Designation
>     StringBufferSize = sizeof (CHAR16) * SMBIOS_STRING_MAX_LENGTH;