Re: [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors.

["Ni, Ray" <ray.ni@intel.com>]

Leo,
The function name is the same as another local function defined in LocalApicLib.
You may need to add a STATIC keyword for this local function, or change it to a
different name.

Thanks,
Ray

> -----Original Message-----
> From: Leo Duran <leo.duran@amd.com>
> Sent: Saturday, February 29, 2020 11:06 PM
> To: devel@edk2.groups.io
> Cc: Leo Duran <leo.duran@amd.com>; Dong, Eric <eric.dong@intel.com>; Ni,
> Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD
> processors.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2556
> 
> This patch uses CPUID signature check to skip reading the PlatformId MSR,
> which is not implemented on AMD processors.
> 
> The PlatformId is used for loading microcode patches, which is also not
> supported and AMD-based platforms. To mitigate the PlatformId
> dependency,
> PcdCpuMicrocodePatchAddress and PcdCpuMicrodePatchRegionSize must
> be set
> to 0 (default value), in order to bypass microcode loading code paths.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Leo Duran <leo.duran@amd.com>
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 34
> ++++++++++++++++++++++++++++++++--
>  UefiCpuPkg/Library/MpInitLib/MpLib.h |  3 +++
>  2 files changed, 35 insertions(+), 2 deletions(-)
>  mode change 100644 => 100755 UefiCpuPkg/Library/MpInitLib/MpLib.h
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index d0fbc17..d2200c3 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -2,6 +2,8 @@
>    CPU MP Initialize Library common functions.
> 
>    Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
> +
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -10,6 +12,29 @@
> 
>  EFI_GUID mCpuInitMpLibHobGuid = CPU_INIT_MP_LIB_HOB_GUID;
> 
> +
> +/**
> +  Determine if the standard CPU signature is "AuthenticAMD".
> +
> +  @retval TRUE  The CPU signature matches.
> +  @retval FALSE The CPU signature does not match.
> +
> +**/
> +BOOLEAN
> +StandardSignatureIsAuthenticAMD (
> +  VOID
> +  )
> +{
> +  UINT32  RegEbx;
> +  UINT32  RegEcx;
> +  UINT32  RegEdx;
> +
> +  AsmCpuid (CPUID_SIGNATURE, NULL, &RegEbx, &RegEcx, &RegEdx);
> +  return (RegEbx == CPUID_SIGNATURE_AUTHENTIC_AMD_EBX &&
> +          RegEcx == CPUID_SIGNATURE_AUTHENTIC_AMD_ECX &&
> +          RegEdx == CPUID_SIGNATURE_AUTHENTIC_AMD_EDX);
> +}
> +
>  /**
>    The function will check if BSP Execute Disable is enabled.
> 
> @@ -564,8 +589,13 @@ InitializeApData (
>    CpuMpData->CpuData[ProcessorNumber].Waiting    = FALSE;
>    CpuMpData->CpuData[ProcessorNumber].CpuHealthy = (BistData == 0) ?
> TRUE : FALSE;
> 
> -  PlatformIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_PLATFORM_ID);
> -  CpuMpData->CpuData[ProcessorNumber].PlatformId = (UINT8)
> PlatformIdMsr.Bits.PlatformId;
> +  //
> +  // NOTE: PlatformId is not relevant on AMD platforms.
> +  //
> +  if (!StandardSignatureIsAuthenticAMD ()) {
> +    PlatformIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_PLATFORM_ID);
> +    CpuMpData->CpuData[ProcessorNumber].PlatformId =
> (UINT8)PlatformIdMsr.Bits.PlatformId;
> +  }
> 
>    AsmCpuid (
>      CPUID_VERSION_INFO,
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> old mode 100644
> new mode 100755
> index 455cb3f..0c89f8a
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -2,6 +2,8 @@
>    Common header file for MP Initialize Library.
> 
>    Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
> +
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -12,6 +14,7 @@
>  #include <PiPei.h>
> 
>  #include <Register/Intel/Cpuid.h>
> +#include <Register/Amd/Cpuid.h>
>  #include <Register/Intel/Msr.h>
>  #include <Register/Intel/LocalApic.h>
>  #include <Register/Intel/Microcode.h>
> --
> 2.7.4