* [PATCH v3] UefiCpuPkg: Fix bug in MpInitLib @ 2020-02-28 18:58 Leo Duran 2020-02-28 18:58 ` [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors Leo Duran 0 siblings, 1 reply; 8+ messages in thread From: Leo Duran @ 2020-02-28 18:58 UTC (permalink / raw) To: devel; +Cc: Leo Duran This patch fixes an issue introduced recently in MpInitLib, where we read a PlatformId MSR that is not implemented on AMD processors. The patch uses CPUID signature check to skip reading the PlatformId MSR. Changes since v2: Fix typo (PCD name) on commit log. Leo Duran (1): UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors. UefiCpuPkg/Library/MpInitLib/MpLib.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors. 2020-02-28 18:58 [PATCH v3] UefiCpuPkg: Fix bug in MpInitLib Leo Duran @ 2020-02-28 18:58 ` Leo Duran 2020-02-29 7:32 ` Laszlo Ersek 0 siblings, 1 reply; 8+ messages in thread From: Leo Duran @ 2020-02-28 18:58 UTC (permalink / raw) To: devel; +Cc: Leo Duran, Eric Dong, Ray Ni, Laszlo Ersek 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors. 2020-02-28 18:58 ` [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors Leo Duran @ 2020-02-29 7:32 ` Laszlo Ersek 2020-02-29 14:11 ` Ni, Ray 0 siblings, 1 reply; 8+ messages in thread From: Laszlo Ersek @ 2020-02-29 7:32 UTC (permalink / raw) To: Leo Duran, devel; +Cc: Eric Dong, Ray Ni 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> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors. 2020-02-29 7:32 ` Laszlo Ersek @ 2020-02-29 14:11 ` Ni, Ray 0 siblings, 0 replies; 8+ messages in thread From: Ni, Ray @ 2020-02-29 14:11 UTC (permalink / raw) To: Laszlo Ersek, Leo Duran, devel@edk2.groups.io; +Cc: Dong, Eric 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> ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v4] UefiCpuPkg: Fix bug in MpInitLib @ 2020-02-29 15:05 Leo Duran 2020-02-29 15:05 ` [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors Leo Duran 0 siblings, 1 reply; 8+ messages in thread From: Leo Duran @ 2020-02-29 15:05 UTC (permalink / raw) To: devel; +Cc: Leo Duran This patch fixes an issue introduced recently in MpInitLib, where we read a PlatformId MSR that is not implemented on AMD processors. The patch uses CPUID signature check to skip reading the PlatformId MSR. Changes since v3: Add header file entry for CPUID definitions: Register/Amd/Cpuid.h Leo Duran (1): UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors. 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 -- 2.7.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors. 2020-02-29 15:05 [PATCH v4] UefiCpuPkg: Fix bug in MpInitLib Leo Duran @ 2020-02-29 15:05 ` Leo Duran 2020-02-29 19:50 ` Laszlo Ersek 2020-03-02 1:59 ` Ni, Ray 0 siblings, 2 replies; 8+ messages in thread From: Leo Duran @ 2020-02-29 15:05 UTC (permalink / raw) To: devel; +Cc: Leo Duran, Eric Dong, Ray Ni, Laszlo Ersek 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors. 2020-02-29 15:05 ` [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors Leo Duran @ 2020-02-29 19:50 ` Laszlo Ersek 2020-03-02 1:59 ` Ni, Ray 1 sibling, 0 replies; 8+ messages in thread From: Laszlo Ersek @ 2020-02-29 19:50 UTC (permalink / raw) To: Leo Duran, devel; +Cc: Eric Dong, Ray Ni On 02/29/20 16:05, 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 ++++++++++++++++++++++++++++++++-- > 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> > Reviewed-by: Laszlo Ersek <lersek@redhat.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors. 2020-02-29 15:05 ` [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors Leo Duran 2020-02-29 19:50 ` Laszlo Ersek @ 2020-03-02 1:59 ` Ni, Ray 1 sibling, 0 replies; 8+ messages in thread From: Ni, Ray @ 2020-03-02 1:59 UTC (permalink / raw) To: Leo Duran, devel@edk2.groups.io; +Cc: Dong, Eric, Laszlo Ersek 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] UefiCpuPkg: Fix bug in MpInitLib @ 2020-02-28 18:45 Leo Duran 2020-02-28 18:45 ` [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors Leo Duran 0 siblings, 1 reply; 8+ messages in thread From: Leo Duran @ 2020-02-28 18:45 UTC (permalink / raw) To: devel; +Cc: Leo Duran This patch fixes an issue introduced recently in MpInitLib, where we read a PlatformId MSR that is not implemented on AMD processors. The patch uses CPUID signature check to skip reading the PlatformId MSR. Changes since v1: - Undo changes to LocalApicLib to not export CPUID signature check. - Undo changes to MpInitLib/Microcode.c and rely on existing PCDs to manage loading of microcode patches. Leo Duran (1): UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors. UefiCpuPkg/Library/MpInitLib/MpLib.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors. 2020-02-28 18:45 [PATCH v2] UefiCpuPkg: Fix bug in MpInitLib Leo Duran @ 2020-02-28 18:45 ` Leo Duran 0 siblings, 0 replies; 8+ messages in thread From: Leo Duran @ 2020-02-28 18:45 UTC (permalink / raw) To: devel; +Cc: Leo Duran, Eric Dong, Ray Ni, Laszlo Ersek 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 PcdCpuMMicrodePatchRegionSize 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-03-02 1:59 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-02-28 18:58 [PATCH v3] UefiCpuPkg: Fix bug in MpInitLib Leo Duran 2020-02-28 18:58 ` [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors Leo Duran 2020-02-29 7:32 ` Laszlo Ersek 2020-02-29 14:11 ` Ni, Ray -- strict thread matches above, loose matches on Subject: below -- 2020-02-29 15:05 [PATCH v4] UefiCpuPkg: Fix bug in MpInitLib Leo Duran 2020-02-29 15:05 ` [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors Leo Duran 2020-02-29 19:50 ` Laszlo Ersek 2020-03-02 1:59 ` Ni, Ray 2020-02-28 18:45 [PATCH v2] UefiCpuPkg: Fix bug in MpInitLib Leo Duran 2020-02-28 18:45 ` [PATCH] UefiCpuPkg/MpInitLib: Skip reading PlatformId on AMD processors Leo Duran
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox