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

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

Reviewed-by: Ray Ni <ray.ni@intel.com>
I will add the "STATIC" when pushing.

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Monday, March 2, 2020 10:00 AM
> To: Leo Duran <leo.duran@amd.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg/MpInitLib: Skip reading
> PlatformId on AMD processors.
> 
> 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
> 
> 
>