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

Leo Duran posted 1 patch 4 years, 1 month ago
Failed in applying to current master (apply log)
There is a newer version of this series
UefiCpuPkg/Library/MpInitLib/MpLib.c | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
[edk2-devel] [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors.
Posted by Leo Duran 4 years, 1 month ago
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 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

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,
-- 
2.7.4


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55105): https://edk2.groups.io/g/devel/message/55105
Mute This Topic: https://groups.io/mt/71626798/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors.
Posted by Laszlo Ersek 4 years, 1 month ago
On 02/28/20 19:58, Leo Duran wrote:
> 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 ++++++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> 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,
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55109): https://edk2.groups.io/g/devel/message/55109
Mute This Topic: https://groups.io/mt/71626798/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors.
Posted by Ni, Ray 4 years ago
Did you try the build?
I saw "error C2065: CPUID_SIGNATURE_AUTHENTIC_AMD_EBX: undeclared identifier" in my internal build report with this patch.

Thanks,
Ray

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Saturday, February 29, 2020 3:33 PM
> To: Leo Duran <leo.duran@amd.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: Re: [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors.
> 
> On 02/28/20 19:58, Leo Duran wrote:
> > 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 ++++++++++++++++++++++++++++++++--
> >  1 file changed, 32 insertions(+), 2 deletions(-)
> >
> > 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,
> >
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55110): https://edk2.groups.io/g/devel/message/55110
Mute This Topic: https://groups.io/mt/71626798/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-