* [PATCH v1 1/2] UefiCpuPkg: Move StandardSignatureIsAuthenticAMD to BaseUefiCpuLib
2020-06-15 18:30 [PATCH v1 0/2] UefiCpuPkg: AMD procesor MSR_IA32_MISC_ENABLE Kirkendall, Garrett
@ 2020-06-15 18:30 ` Kirkendall, Garrett
2020-06-16 15:29 ` Laszlo Ersek
2020-06-17 0:30 ` [edk2-devel] " Dong, Eric
2020-06-15 18:30 ` [PATCH v1 2/2] UefiCpuPkg: PiSmmCpuDxeSmm skip MSR_IA32_MISC_ENABLE manipulation on AMD Kirkendall, Garrett
2020-06-16 17:01 ` [PATCH v1 0/2] UefiCpuPkg: AMD procesor MSR_IA32_MISC_ENABLE Laszlo Ersek
2 siblings, 2 replies; 7+ messages in thread
From: Kirkendall, Garrett @ 2020-06-15 18:30 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Laszlo Ersek
Refactor StandardSignatureIsAuthenticAMD into BaseUefiCpuLib from
separate copies in BaseXApicLib, BaseXApicX2ApicLib, and MpInitLib.
This allows for future use of StandarSignatureIsAuthinticAMD without
creating more instances in other modules.
This function allows IA32/X64 code to determine if it is running on an
AMD brand processor.
UefiCpuLib is already included directly or indirectly in all modified
modules. Complete move is made in this change.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
---
UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf | 7 ++++
UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf | 2 ++
UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf | 2 ++
UefiCpuPkg/Include/Library/UefiCpuLib.h | 14 ++++++++
UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c | 38 ++++++++++++++++++++
UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c | 25 ++-----------
UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c | 25 ++-----------
UefiCpuPkg/Library/MpInitLib/MpLib.c | 23 ------------
8 files changed, 67 insertions(+), 69 deletions(-)
diff --git a/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
index 006b7acbf14e..34d3a7bb4303 100644
--- a/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
+++ b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
@@ -4,6 +4,7 @@
# The library routines are UEFI specification compliant.
#
# Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -29,6 +30,12 @@ [Sources.IA32]
[Sources.X64]
X64/InitializeFpu.nasm
+[Sources]
+ BaseUefiCpuLib.c
+
[Packages]
MdePkg/MdePkg.dec
UefiCpuPkg/UefiCpuPkg.dec
+
+[LibraryClasses]
+ BaseLib
diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
index bdb2ff372677..561baa44b0e6 100644
--- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
+++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
@@ -5,6 +5,7 @@
# where local APIC is disabled.
#
# Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -37,6 +38,7 @@ [LibraryClasses]
TimerLib
IoLib
PcdLib
+ UefiCpuLib
[Pcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuInitIpiDelayInMicroSeconds ## SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
index ac1e0a1c9896..1e2a4f8b790f 100644
--- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
+++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
@@ -5,6 +5,7 @@
# where local APIC is disabled.
#
# Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -37,6 +38,7 @@ [LibraryClasses]
TimerLib
IoLib
PcdLib
+ UefiCpuLib
[Pcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuInitIpiDelayInMicroSeconds ## SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/Include/Library/UefiCpuLib.h b/UefiCpuPkg/Include/Library/UefiCpuLib.h
index 82e53bab3a0f..5326e7246301 100644
--- a/UefiCpuPkg/Include/Library/UefiCpuLib.h
+++ b/UefiCpuPkg/Include/Library/UefiCpuLib.h
@@ -5,6 +5,7 @@
to be UEFI specification compliant.
Copyright (c) 2009, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -29,4 +30,17 @@ InitializeFloatingPointUnits (
VOID
);
+/**
+ Determine if the standard CPU signature is "AuthenticAMD".
+
+ @retval TRUE The CPU signature matches.
+ @retval FALSE The CPU signature does not match.
+
+**/
+BOOLEAN
+EFIAPI
+StandardSignatureIsAuthenticAMD (
+ VOID
+ );
+
#endif
diff --git a/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c
new file mode 100644
index 000000000000..c2cc3ff9a709
--- /dev/null
+++ b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c
@@ -0,0 +1,38 @@
+/** @file
+ This library defines some routines that are generic for IA32 family CPU.
+
+ The library routines are UEFI specification compliant.
+
+ Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Register/Intel/Cpuid.h>
+#include <Register/Amd/Cpuid.h>
+
+#include <Library/BaseLib.h>
+#include <Library/UefiCpuLib.h>
+
+/**
+ Determine if the standard CPU signature is "AuthenticAMD".
+
+ @retval TRUE The CPU signature matches.
+ @retval FALSE The CPU signature does not match.
+
+**/
+BOOLEAN
+EFIAPI
+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);
+}
diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
index 33ea15ca2916..52bd90d33428 100644
--- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
@@ -4,7 +4,7 @@
This local APIC library instance supports xAPIC mode only.
Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
- Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+ Copyright (c) 2017 - 2020, AMD Inc. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -21,33 +21,12 @@
#include <Library/IoLib.h>
#include <Library/TimerLib.h>
#include <Library/PcdLib.h>
+#include <Library/UefiCpuLib.h>
//
// Library internal functions
//
-/**
- 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);
-}
-
/**
Determine if the CPU supports the Local APIC Base Address MSR.
diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
index d0f92b33dc8c..cdcbca046191 100644
--- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
@@ -5,7 +5,7 @@
which have xAPIC and x2APIC modes.
Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
- Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+ Copyright (c) 2017 - 2020, AMD Inc. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -22,33 +22,12 @@
#include <Library/IoLib.h>
#include <Library/TimerLib.h>
#include <Library/PcdLib.h>
+#include <Library/UefiCpuLib.h>
//
// Library internal functions
//
-/**
- 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);
-}
-
/**
Determine if the CPU supports the Local APIC Base Address MSR.
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index ab7a8ed6633a..9b0660a5d4ea 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -13,29 +13,6 @@
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.
-
-**/
-STATIC
-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.
--
2.27.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v1 1/2] UefiCpuPkg: Move StandardSignatureIsAuthenticAMD to BaseUefiCpuLib
2020-06-15 18:30 ` [PATCH v1 1/2] UefiCpuPkg: Move StandardSignatureIsAuthenticAMD to BaseUefiCpuLib Kirkendall, Garrett
@ 2020-06-16 15:29 ` Laszlo Ersek
2020-06-17 0:30 ` [edk2-devel] " Dong, Eric
1 sibling, 0 replies; 7+ messages in thread
From: Laszlo Ersek @ 2020-06-16 15:29 UTC (permalink / raw)
To: Garrett Kirkendall, devel; +Cc: Eric Dong, Ray Ni
On 06/15/20 20:30, Garrett Kirkendall wrote:
> Refactor StandardSignatureIsAuthenticAMD into BaseUefiCpuLib from
> separate copies in BaseXApicLib, BaseXApicX2ApicLib, and MpInitLib.
> This allows for future use of StandarSignatureIsAuthinticAMD without
> creating more instances in other modules.
>
> This function allows IA32/X64 code to determine if it is running on an
> AMD brand processor.
>
> UefiCpuLib is already included directly or indirectly in all modified
> modules. Complete move is made in this change.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
> ---
> UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf | 7 ++++
> UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf | 2 ++
> UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf | 2 ++
> UefiCpuPkg/Include/Library/UefiCpuLib.h | 14 ++++++++
> UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c | 38 ++++++++++++++++++++
> UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c | 25 ++-----------
> UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c | 25 ++-----------
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 23 ------------
> 8 files changed, 67 insertions(+), 69 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> index 006b7acbf14e..34d3a7bb4303 100644
> --- a/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> +++ b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> @@ -4,6 +4,7 @@
> # The library routines are UEFI specification compliant.
> #
> # Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> ##
> @@ -29,6 +30,12 @@ [Sources.IA32]
> [Sources.X64]
> X64/InitializeFpu.nasm
>
> +[Sources]
> + BaseUefiCpuLib.c
> +
> [Packages]
> MdePkg/MdePkg.dec
> UefiCpuPkg/UefiCpuPkg.dec
> +
> +[LibraryClasses]
> + BaseLib
> diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
> index bdb2ff372677..561baa44b0e6 100644
> --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
> +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
> @@ -5,6 +5,7 @@
> # where local APIC is disabled.
> #
> # Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> ##
> @@ -37,6 +38,7 @@ [LibraryClasses]
> TimerLib
> IoLib
> PcdLib
> + UefiCpuLib
>
> [Pcd]
> gUefiCpuPkgTokenSpaceGuid.PcdCpuInitIpiDelayInMicroSeconds ## SOMETIMES_CONSUMES
> diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> index ac1e0a1c9896..1e2a4f8b790f 100644
> --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> @@ -5,6 +5,7 @@
> # where local APIC is disabled.
> #
> # Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> ##
> @@ -37,6 +38,7 @@ [LibraryClasses]
> TimerLib
> IoLib
> PcdLib
> + UefiCpuLib
>
> [Pcd]
> gUefiCpuPkgTokenSpaceGuid.PcdCpuInitIpiDelayInMicroSeconds ## SOMETIMES_CONSUMES
> diff --git a/UefiCpuPkg/Include/Library/UefiCpuLib.h b/UefiCpuPkg/Include/Library/UefiCpuLib.h
> index 82e53bab3a0f..5326e7246301 100644
> --- a/UefiCpuPkg/Include/Library/UefiCpuLib.h
> +++ b/UefiCpuPkg/Include/Library/UefiCpuLib.h
> @@ -5,6 +5,7 @@
> to be UEFI specification compliant.
>
> Copyright (c) 2009, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -29,4 +30,17 @@ InitializeFloatingPointUnits (
> VOID
> );
>
> +/**
> + Determine if the standard CPU signature is "AuthenticAMD".
> +
> + @retval TRUE The CPU signature matches.
> + @retval FALSE The CPU signature does not match.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +StandardSignatureIsAuthenticAMD (
> + VOID
> + );
> +
> #endif
> diff --git a/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c
> new file mode 100644
> index 000000000000..c2cc3ff9a709
> --- /dev/null
> +++ b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c
> @@ -0,0 +1,38 @@
> +/** @file
> + This library defines some routines that are generic for IA32 family CPU.
> +
> + The library routines are UEFI specification compliant.
> +
> + Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Register/Intel/Cpuid.h>
> +#include <Register/Amd/Cpuid.h>
> +
> +#include <Library/BaseLib.h>
> +#include <Library/UefiCpuLib.h>
> +
> +/**
> + Determine if the standard CPU signature is "AuthenticAMD".
> +
> + @retval TRUE The CPU signature matches.
> + @retval FALSE The CPU signature does not match.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +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);
> +}
> diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> index 33ea15ca2916..52bd90d33428 100644
> --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> @@ -4,7 +4,7 @@
> This local APIC library instance supports xAPIC mode only.
>
> Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
> - Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> + Copyright (c) 2017 - 2020, AMD Inc. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -21,33 +21,12 @@
> #include <Library/IoLib.h>
> #include <Library/TimerLib.h>
> #include <Library/PcdLib.h>
> +#include <Library/UefiCpuLib.h>
>
> //
> // Library internal functions
> //
>
> -/**
> - 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);
> -}
> -
> /**
> Determine if the CPU supports the Local APIC Base Address MSR.
>
> diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> index d0f92b33dc8c..cdcbca046191 100644
> --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> @@ -5,7 +5,7 @@
> which have xAPIC and x2APIC modes.
>
> Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
> - Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> + Copyright (c) 2017 - 2020, AMD Inc. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -22,33 +22,12 @@
> #include <Library/IoLib.h>
> #include <Library/TimerLib.h>
> #include <Library/PcdLib.h>
> +#include <Library/UefiCpuLib.h>
>
> //
> // Library internal functions
> //
>
> -/**
> - 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);
> -}
> -
> /**
> Determine if the CPU supports the Local APIC Base Address MSR.
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index ab7a8ed6633a..9b0660a5d4ea 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -13,29 +13,6 @@
> 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.
> -
> -**/
> -STATIC
> -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.
>
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [edk2-devel] [PATCH v1 1/2] UefiCpuPkg: Move StandardSignatureIsAuthenticAMD to BaseUefiCpuLib
2020-06-15 18:30 ` [PATCH v1 1/2] UefiCpuPkg: Move StandardSignatureIsAuthenticAMD to BaseUefiCpuLib Kirkendall, Garrett
2020-06-16 15:29 ` Laszlo Ersek
@ 2020-06-17 0:30 ` Dong, Eric
1 sibling, 0 replies; 7+ messages in thread
From: Dong, Eric @ 2020-06-17 0:30 UTC (permalink / raw)
To: devel@edk2.groups.io, garrett.kirkendall@amd.com; +Cc: Ni, Ray, Laszlo Ersek
Reviewed-by: Eric Dong <eric.dong@intel.com>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Kirkendall, Garrett
> Sent: Tuesday, June 16, 2020 2:30 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo
> Ersek <lersek@redhat.com>
> Subject: [edk2-devel] [PATCH v1 1/2] UefiCpuPkg: Move
> StandardSignatureIsAuthenticAMD to BaseUefiCpuLib
>
> Refactor StandardSignatureIsAuthenticAMD into BaseUefiCpuLib from
> separate copies in BaseXApicLib, BaseXApicX2ApicLib, and MpInitLib.
> This allows for future use of StandarSignatureIsAuthinticAMD without
> creating more instances in other modules.
>
> This function allows IA32/X64 code to determine if it is running on an AMD
> brand processor.
>
> UefiCpuLib is already included directly or indirectly in all modified modules.
> Complete move is made in this change.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
> ---
> UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf | 7 ++++
> UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf | 2 ++
> UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf | 2 ++
> UefiCpuPkg/Include/Library/UefiCpuLib.h | 14 ++++++++
> UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c | 38
> ++++++++++++++++++++
> UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c | 25 ++-----------
> UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c | 25 ++------
> -----
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 23 ------------
> 8 files changed, 67 insertions(+), 69 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> index 006b7acbf14e..34d3a7bb4303 100644
> --- a/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> +++ b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> @@ -4,6 +4,7 @@
> # The library routines are UEFI specification compliant.
> #
> # Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
> # SPDX-License-Identifier: BSD-2-Clause-Patent # ## @@ -29,6 +30,12 @@
> [Sources.IA32] [Sources.X64]
> X64/InitializeFpu.nasm
>
> +[Sources]
> + BaseUefiCpuLib.c
> +
> [Packages]
> MdePkg/MdePkg.dec
> UefiCpuPkg/UefiCpuPkg.dec
> +
> +[LibraryClasses]
> + BaseLib
> diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
> b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
> index bdb2ff372677..561baa44b0e6 100644
> --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
> +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
> @@ -5,6 +5,7 @@
> # where local APIC is disabled.
> #
> # Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
> # SPDX-License-Identifier: BSD-2-Clause-Patent # ## @@ -37,6 +38,7 @@
> [LibraryClasses]
> TimerLib
> IoLib
> PcdLib
> + UefiCpuLib
>
> [Pcd]
> gUefiCpuPkgTokenSpaceGuid.PcdCpuInitIpiDelayInMicroSeconds ##
> SOMETIMES_CONSUMES diff --git
> a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> index ac1e0a1c9896..1e2a4f8b790f 100644
> --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf
> @@ -5,6 +5,7 @@
> # where local APIC is disabled.
> #
> # Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
> # SPDX-License-Identifier: BSD-2-Clause-Patent # ## @@ -37,6 +38,7 @@
> [LibraryClasses]
> TimerLib
> IoLib
> PcdLib
> + UefiCpuLib
>
> [Pcd]
> gUefiCpuPkgTokenSpaceGuid.PcdCpuInitIpiDelayInMicroSeconds ##
> SOMETIMES_CONSUMES diff --git
> a/UefiCpuPkg/Include/Library/UefiCpuLib.h
> b/UefiCpuPkg/Include/Library/UefiCpuLib.h
> index 82e53bab3a0f..5326e7246301 100644
> --- a/UefiCpuPkg/Include/Library/UefiCpuLib.h
> +++ b/UefiCpuPkg/Include/Library/UefiCpuLib.h
> @@ -5,6 +5,7 @@
> to be UEFI specification compliant.
>
> Copyright (c) 2009, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -29,4 +30,17 @@ InitializeFloatingPointUnits (
> VOID
> );
>
> +/**
> + Determine if the standard CPU signature is "AuthenticAMD".
> +
> + @retval TRUE The CPU signature matches.
> + @retval FALSE The CPU signature does not match.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +StandardSignatureIsAuthenticAMD (
> + VOID
> + );
> +
> #endif
> diff --git a/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c
> b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c
> new file mode 100644
> index 000000000000..c2cc3ff9a709
> --- /dev/null
> +++ b/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c
> @@ -0,0 +1,38 @@
> +/** @file
> + This library defines some routines that are generic for IA32 family CPU.
> +
> + The library routines are UEFI specification compliant.
> +
> + Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Register/Intel/Cpuid.h>
> +#include <Register/Amd/Cpuid.h>
> +
> +#include <Library/BaseLib.h>
> +#include <Library/UefiCpuLib.h>
> +
> +/**
> + Determine if the standard CPU signature is "AuthenticAMD".
> +
> + @retval TRUE The CPU signature matches.
> + @retval FALSE The CPU signature does not match.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +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);
> +}
> diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> index 33ea15ca2916..52bd90d33428 100644
> --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
> @@ -4,7 +4,7 @@
> This local APIC library instance supports xAPIC mode only.
>
> Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
> - Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> + Copyright (c) 2017 - 2020, AMD Inc. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -21,33 +21,12 @@
> #include <Library/IoLib.h>
> #include <Library/TimerLib.h>
> #include <Library/PcdLib.h>
> +#include <Library/UefiCpuLib.h>
>
> //
> // Library internal functions
> //
>
> -/**
> - 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);
> -}
> -
> /**
> Determine if the CPU supports the Local APIC Base Address MSR.
>
> diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> index d0f92b33dc8c..cdcbca046191 100644
> --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c
> @@ -5,7 +5,7 @@
> which have xAPIC and x2APIC modes.
>
> Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
> - Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> + Copyright (c) 2017 - 2020, AMD Inc. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -22,33 +22,12 @@
> #include <Library/IoLib.h>
> #include <Library/TimerLib.h>
> #include <Library/PcdLib.h>
> +#include <Library/UefiCpuLib.h>
>
> //
> // Library internal functions
> //
>
> -/**
> - 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);
> -}
> -
> /**
> Determine if the CPU supports the Local APIC Base Address MSR.
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index ab7a8ed6633a..9b0660a5d4ea 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -13,29 +13,6 @@
> 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.
> -
> -**/
> -STATIC
> -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.
>
> --
> 2.27.0
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v1 2/2] UefiCpuPkg: PiSmmCpuDxeSmm skip MSR_IA32_MISC_ENABLE manipulation on AMD
2020-06-15 18:30 [PATCH v1 0/2] UefiCpuPkg: AMD procesor MSR_IA32_MISC_ENABLE Kirkendall, Garrett
2020-06-15 18:30 ` [PATCH v1 1/2] UefiCpuPkg: Move StandardSignatureIsAuthenticAMD to BaseUefiCpuLib Kirkendall, Garrett
@ 2020-06-15 18:30 ` Kirkendall, Garrett
2020-06-16 16:28 ` Laszlo Ersek
2020-06-16 17:01 ` [PATCH v1 0/2] UefiCpuPkg: AMD procesor MSR_IA32_MISC_ENABLE Laszlo Ersek
2 siblings, 1 reply; 7+ messages in thread
From: Kirkendall, Garrett @ 2020-06-15 18:30 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Laszlo Ersek
AMD does not support MSR_IA32_MISC_ENABLE. Accessing that register
causes and exception on AMD processors. If Execution Disable is
supported, and if the processor is an AMD processor skip manipulating
MSR_IA32_MISC_ENABLE[34] XD Disable bit.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h | 3 +++
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 9 ++++++++-
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm | 19 +++++++++++++++++--
UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 20 ++++++++++++++++++--
4 files changed, 46 insertions(+), 5 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
index 43f6935cf9dc..0f6e0c9c98ad 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
@@ -2,6 +2,7 @@
SMM profile internal header file.
Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -13,6 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/UefiRuntimeServicesTableLib.h>
#include <Library/DxeServicesTableLib.h>
#include <Library/CpuLib.h>
+#include <Library/UefiCpuLib.h>
#include <IndustryStandard/Acpi.h>
#include "SmmProfileArch.h"
@@ -99,6 +101,7 @@ extern SMM_S3_RESUME_STATE *mSmmS3ResumeState;
extern UINTN gSmiExceptionHandlers[];
extern BOOLEAN mXdSupported;
X86_ASSEMBLY_PATCH_LABEL gPatchXdSupported;
+X86_ASSEMBLY_PATCH_LABEL gPatchMsrIA32MiscEnableSupported;
extern UINTN *mPFEntryCount;
extern UINT64 (*mLastPFEntryValue)[MAX_PF_ENTRY_COUNT];
extern UINT64 *(*mLastPFEntryPointer)[MAX_PF_ENTRY_COUNT];
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index c47b5573e366..146600e6c3b4 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -2,7 +2,7 @@
Enable SMM profile.
Copyright (c) 2012 - 2019, Intel Corporation. All rights reserved.<BR>
-Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
+Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -1015,6 +1015,13 @@ CheckFeatureSupported (
mXdSupported = FALSE;
PatchInstructionX86 (gPatchXdSupported, mXdSupported, 1);
}
+
+ if (StandardSignatureIsAuthenticAMD()) {
+ //
+ // AMD processors do not support MSR_IA32_MISC_ENABLE
+ //
+ PatchInstructionX86 (gPatchMsrIA32MiscEnableSupported, FALSE, 1);
+ }
}
if (mBtsSupported) {
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
index f96de9bdeb43..e4ef5899f274 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
@@ -1,5 +1,6 @@
;------------------------------------------------------------------------------ ;
; Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
; SPDX-License-Identifier: BSD-2-Clause-Patent
;
; Module Name:
@@ -59,6 +60,7 @@ global ASM_PFX(gPatchSmiStack)
global ASM_PFX(gPatchSmbase)
extern ASM_PFX(mXdSupported)
global ASM_PFX(gPatchXdSupported)
+global ASM_PFX(gPatchMsrIA32MiscEnableSupported)
extern ASM_PFX(gSmiHandlerIdtr)
extern ASM_PFX(mCetSupported)
@@ -153,17 +155,30 @@ ASM_PFX(gPatchSmiCr3):
ASM_PFX(gPatchXdSupported):
cmp al, 0
jz @SkipXd
+
+; Clear XD Disable bit if supported
+ mov al, strict byte 1 ; source operand may be patched
+ASM_PFX(gPatchMsrIA32MiscEnableSupported):
+ cmp al, 1
+ jz MsrIA32MiscEnableSupported
+
+; MSR_IA32_MISC_ENABLE not supported
+ xor edx, edx
+ push edx
+ jmp EnableNxe
+
;
; Check XD disable bit
;
+MsrIA32MiscEnableSupported:
mov ecx, MSR_IA32_MISC_ENABLE
rdmsr
push edx ; save MSR_IA32_MISC_ENABLE[63-32]
test edx, BIT2 ; MSR_IA32_MISC_ENABLE[34]
- jz .5
+ jz EnableNxe
and dx, 0xFFFB ; clear XD Disable bit if it is set
wrmsr
-.5:
+EnableNxe:
mov ecx, MSR_EFER
rdmsr
or ax, MSR_EFER_XD ; enable NXE
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
index 8bfba55b5d08..5a4678c7aa63 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
@@ -1,5 +1,6 @@
;------------------------------------------------------------------------------ ;
; Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
; SPDX-License-Identifier: BSD-2-Clause-Patent
;
; Module Name:
@@ -67,6 +68,7 @@ extern ASM_PFX(CpuSmmDebugExit)
global ASM_PFX(gPatchSmbase)
extern ASM_PFX(mXdSupported)
global ASM_PFX(gPatchXdSupported)
+global ASM_PFX(gPatchMsrIA32MiscEnableSupported)
global ASM_PFX(gPatchSmiStack)
global ASM_PFX(gPatchSmiCr3)
global ASM_PFX(gPatch5LevelPagingNeeded)
@@ -152,18 +154,32 @@ SkipEnable5LevelPaging:
ASM_PFX(gPatchXdSupported):
cmp al, 0
jz @SkipXd
+
+; Clear XD Disable bit if supported
+ mov al, strict byte 1 ; source operand may be patched
+ASM_PFX(gPatchMsrIA32MiscEnableSupported):
+ cmp al, 1
+ jz MsrIA32MiscEnableSupported
+
+; MSR_IA32_MISC_ENABLE not supported
+ sub esp, 4
+ xor rdx, rdx
+ push rdx
+ jmp EnableNxe
+
;
; Check XD disable bit
;
+MsrIA32MiscEnableSupported:
mov ecx, MSR_IA32_MISC_ENABLE
rdmsr
sub esp, 4
push rdx ; save MSR_IA32_MISC_ENABLE[63-32]
test edx, BIT2 ; MSR_IA32_MISC_ENABLE[34]
- jz .0
+ jz EnableNxe
and dx, 0xFFFB ; clear XD Disable bit if it is set
wrmsr
-.0:
+EnableNxe:
mov ecx, MSR_EFER
rdmsr
or ax, MSR_EFER_XD ; enable NXE
--
2.27.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v1 2/2] UefiCpuPkg: PiSmmCpuDxeSmm skip MSR_IA32_MISC_ENABLE manipulation on AMD
2020-06-15 18:30 ` [PATCH v1 2/2] UefiCpuPkg: PiSmmCpuDxeSmm skip MSR_IA32_MISC_ENABLE manipulation on AMD Kirkendall, Garrett
@ 2020-06-16 16:28 ` Laszlo Ersek
0 siblings, 0 replies; 7+ messages in thread
From: Laszlo Ersek @ 2020-06-16 16:28 UTC (permalink / raw)
To: Garrett Kirkendall, devel; +Cc: Eric Dong, Ray Ni
On 06/15/20 20:30, Garrett Kirkendall wrote:
> AMD does not support MSR_IA32_MISC_ENABLE. Accessing that register
> causes and exception on AMD processors. If Execution Disable is
> supported, and if the processor is an AMD processor skip manipulating
> MSR_IA32_MISC_ENABLE[34] XD Disable bit.
(1a) I suggest replacing "and if" with "but".
(1b) I suggest inserting a comman before "skip".
(Side comment: someone give a medal to whomever invented the name
"Execution Disable Disable".)
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
> ---
> UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h | 3 +++
> UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 9 ++++++++-
> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm | 19 +++++++++++++++++--
> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 20 ++++++++++++++++++--
> 4 files changed, 46 insertions(+), 5 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
> index 43f6935cf9dc..0f6e0c9c98ad 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
> @@ -2,6 +2,7 @@
> SMM profile internal header file.
>
> Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -13,6 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Library/UefiRuntimeServicesTableLib.h>
> #include <Library/DxeServicesTableLib.h>
> #include <Library/CpuLib.h>
> +#include <Library/UefiCpuLib.h>
> #include <IndustryStandard/Acpi.h>
>
> #include "SmmProfileArch.h"
> @@ -99,6 +101,7 @@ extern SMM_S3_RESUME_STATE *mSmmS3ResumeState;
> extern UINTN gSmiExceptionHandlers[];
> extern BOOLEAN mXdSupported;
> X86_ASSEMBLY_PATCH_LABEL gPatchXdSupported;
> +X86_ASSEMBLY_PATCH_LABEL gPatchMsrIA32MiscEnableSupported;
The edk2 coding style subjects acronyms to CamelCasing. For example,
"PCI" is CamelCased as "Pci" in identifiers.
(2) Therefore, please replace "IA32" with "Ia32" in the above.
> extern UINTN *mPFEntryCount;
> extern UINT64 (*mLastPFEntryValue)[MAX_PF_ENTRY_COUNT];
> extern UINT64 *(*mLastPFEntryPointer)[MAX_PF_ENTRY_COUNT];
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> index c47b5573e366..146600e6c3b4 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> @@ -2,7 +2,7 @@
> Enable SMM profile.
>
> Copyright (c) 2012 - 2019, Intel Corporation. All rights reserved.<BR>
> -Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
> +Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -1015,6 +1015,13 @@ CheckFeatureSupported (
> mXdSupported = FALSE;
> PatchInstructionX86 (gPatchXdSupported, mXdSupported, 1);
> }
> +
> + if (StandardSignatureIsAuthenticAMD()) {
(3) Please insert a space character before the opening parenthesis.
> + //
> + // AMD processors do not support MSR_IA32_MISC_ENABLE
> + //
> + PatchInstructionX86 (gPatchMsrIA32MiscEnableSupported, FALSE, 1);
> + }
> }
>
> if (mBtsSupported) {
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> index f96de9bdeb43..e4ef5899f274 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
> @@ -1,5 +1,6 @@
> ;------------------------------------------------------------------------------ ;
> ; Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
> ; SPDX-License-Identifier: BSD-2-Clause-Patent
> ;
> ; Module Name:
> @@ -59,6 +60,7 @@ global ASM_PFX(gPatchSmiStack)
> global ASM_PFX(gPatchSmbase)
> extern ASM_PFX(mXdSupported)
> global ASM_PFX(gPatchXdSupported)
> +global ASM_PFX(gPatchMsrIA32MiscEnableSupported)
> extern ASM_PFX(gSmiHandlerIdtr)
>
> extern ASM_PFX(mCetSupported)
> @@ -153,17 +155,30 @@ ASM_PFX(gPatchSmiCr3):
> ASM_PFX(gPatchXdSupported):
> cmp al, 0
> jz @SkipXd
> +
> +; Clear XD Disable bit if supported
(4) please expand the comment: "... if MSR_IA32_MISC_ENABLE is supported"
> + mov al, strict byte 1 ; source operand may be patched
> +ASM_PFX(gPatchMsrIA32MiscEnableSupported):
> + cmp al, 1
> + jz MsrIA32MiscEnableSupported
> +
> +; MSR_IA32_MISC_ENABLE not supported
> + xor edx, edx
> + push edx
(5) Please append a comment to the right:
"; don't try to restore the XD Disable bit just before RSM"
> + jmp EnableNxe
> +
> ;
> ; Check XD disable bit
> ;
> +MsrIA32MiscEnableSupported:
(6) Please CamelCase "IA32"
> mov ecx, MSR_IA32_MISC_ENABLE
> rdmsr
> push edx ; save MSR_IA32_MISC_ENABLE[63-32]
> test edx, BIT2 ; MSR_IA32_MISC_ENABLE[34]
> - jz .5
> + jz EnableNxe
> and dx, 0xFFFB ; clear XD Disable bit if it is set
> wrmsr
> -.5:
> +EnableNxe:
> mov ecx, MSR_EFER
> rdmsr
> or ax, MSR_EFER_XD ; enable NXE
Please apply changes (2) through (6) to the X64 version too.
Other than these cosmetic comments, the patch looks good to me. I'm
ready to R-b with the coding style / comments / commit message updated.
Thanks!
Laszlo
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> index 8bfba55b5d08..5a4678c7aa63 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> @@ -1,5 +1,6 @@
> ;------------------------------------------------------------------------------ ;
> ; Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
> ; SPDX-License-Identifier: BSD-2-Clause-Patent
> ;
> ; Module Name:
> @@ -67,6 +68,7 @@ extern ASM_PFX(CpuSmmDebugExit)
> global ASM_PFX(gPatchSmbase)
> extern ASM_PFX(mXdSupported)
> global ASM_PFX(gPatchXdSupported)
> +global ASM_PFX(gPatchMsrIA32MiscEnableSupported)
> global ASM_PFX(gPatchSmiStack)
> global ASM_PFX(gPatchSmiCr3)
> global ASM_PFX(gPatch5LevelPagingNeeded)
> @@ -152,18 +154,32 @@ SkipEnable5LevelPaging:
> ASM_PFX(gPatchXdSupported):
> cmp al, 0
> jz @SkipXd
> +
> +; Clear XD Disable bit if supported
> + mov al, strict byte 1 ; source operand may be patched
> +ASM_PFX(gPatchMsrIA32MiscEnableSupported):
> + cmp al, 1
> + jz MsrIA32MiscEnableSupported
> +
> +; MSR_IA32_MISC_ENABLE not supported
> + sub esp, 4
> + xor rdx, rdx
> + push rdx
> + jmp EnableNxe
> +
> ;
> ; Check XD disable bit
> ;
> +MsrIA32MiscEnableSupported:
> mov ecx, MSR_IA32_MISC_ENABLE
> rdmsr
> sub esp, 4
> push rdx ; save MSR_IA32_MISC_ENABLE[63-32]
> test edx, BIT2 ; MSR_IA32_MISC_ENABLE[34]
> - jz .0
> + jz EnableNxe
> and dx, 0xFFFB ; clear XD Disable bit if it is set
> wrmsr
> -.0:
> +EnableNxe:
> mov ecx, MSR_EFER
> rdmsr
> or ax, MSR_EFER_XD ; enable NXE
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 0/2] UefiCpuPkg: AMD procesor MSR_IA32_MISC_ENABLE
2020-06-15 18:30 [PATCH v1 0/2] UefiCpuPkg: AMD procesor MSR_IA32_MISC_ENABLE Kirkendall, Garrett
2020-06-15 18:30 ` [PATCH v1 1/2] UefiCpuPkg: Move StandardSignatureIsAuthenticAMD to BaseUefiCpuLib Kirkendall, Garrett
2020-06-15 18:30 ` [PATCH v1 2/2] UefiCpuPkg: PiSmmCpuDxeSmm skip MSR_IA32_MISC_ENABLE manipulation on AMD Kirkendall, Garrett
@ 2020-06-16 17:01 ` Laszlo Ersek
2 siblings, 0 replies; 7+ messages in thread
From: Laszlo Ersek @ 2020-06-16 17:01 UTC (permalink / raw)
To: Garrett Kirkendall, devel; +Cc: Eric Dong, Ray Ni
Hi Garrett,
On 06/15/20 20:30, Garrett Kirkendall wrote:
> AMD processor does not support MSR_IA32_MISC_ENABLE register. Accessing
> this register on AMD causes an unhandled exception in SmmEntry.nasm and
> a subsequent failure to boot since this is too early in SMM path for the
> exception handler to be loaded.
>
> First, to distinguish between AMD and other processors, refactor
> StandardSignatureIsAuthenticAMD into BaseUefiCpuLib. So there is only
> one copy in the source. All changed modules already include UefiCpuLib
> either directly or indirectly so could not easly split first patch.
>
> Second, Skip manipulation of MSR_IA32_MISC_ENABLE register if running
> on an AMD processor.
>
> Tested on AMD X64 processor.
>
> Modified source patching in 2/2 for FALSE
> and TRUE to test failure and passing case when AMD processor detected.
> Did not have a way to test UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm.
Please test the series as follows:
(1) Download
https://www.kraxel.org/repos/images/fedora-30-efi-systemd-i686.qcow2.xz
https://www.kraxel.org/repos/images/fedora-31-efi-grub2-x86_64.qcow2.xz
and decompress both files.
(2) Test using a 32-bit guest on an Intel host (standing in your edk2 tree, with the patches applied):
$ build -a IA32 -b DEBUG -p OvmfPkg/OvmfPkgIa32.dsc -t GCC5 -D SMM_REQUIRE
$ qemu-system-i386 \
-cpu coreduo,-nx \
-machine q35,smm=on,accel=kvm \
-m 4096 \
-smp 4 \
-global driver=cfi.pflash01,property=secure,value=on \
-drive if=pflash,format=raw,unit=0,readonly=on,file=Build/OvmfIa32/DEBUG_GCC5/FV/OVMF_CODE.fd \
-drive if=pflash,format=raw,unit=1,snapshot=on,file=Build/OvmfIa32/DEBUG_GCC5/FV/OVMF_VARS.fd \
-drive id=hdd,if=none,format=qcow2,snapshot=on,file=fedora-30-efi-systemd-i686.qcow2 \
-device virtio-scsi-pci,id=scsi0 \
-device scsi-hd,drive=hdd,bus=scsi0.0,bootindex=1
(Once you get a login prompt, feel free to interrupt QEMU with Ctrl-C.)
(3) Test using a 64-bit guest on an Intel host:
$ build -a IA32 -a X64 -b DEBUG -p OvmfPkg/OvmfPkgIa32X64.dsc -t GCC5 -D SMM_REQUIRE
$ qemu-system-x86_64 \
-cpu host \
-machine q35,smm=on,accel=kvm \
-m 4096 \
-smp 4 \
-global driver=cfi.pflash01,property=secure,value=on \
-drive if=pflash,format=raw,unit=0,readonly=on,file=Build/Ovmf3264/DEBUG_GCC5/FV/OVMF_CODE.fd \
-drive if=pflash,format=raw,unit=1,snapshot=on,file=Build/Ovmf3264/DEBUG_GCC5/FV/OVMF_VARS.fd \
-drive id=hdd,if=none,format=qcow2,snapshot=on,file=fedora-31-efi-grub2-x86_64.qcow2 \
-device virtio-scsi-pci,id=scsi0 \
-device scsi-hd,drive=hdd,bus=scsi0.0,bootindex=1
(4) Test using a 64-bit guest on an AMD host -- just repeat step (3) on an AMD host.
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 7+ messages in thread