[PATCH v6 0/4] AMD processor MSR_IA32_MISC_ENABLE

[PATCH v6 0/4] AMD processor MSR_IA32_MISC_ENABLE

[Kirkendall, Garrett]

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.

1. Prepare PcAtChipsetPkg/PcAtChipsetPkg.dsc to move
StandardSignatureIsAuthenticAMD into UefiCpuLib LibraryClass
BaseUefiCpuLib in UefiCpuPkg.

2. To distinguish between AMD and other processors, refactor
StandardSignatureIsAuthenticAMD into BaseUefiCpuLib.  So there is only
one copy in the source.

3. Skip manipulation of MSR_IA32_MISC_ENABLE register if running
on an AMD processor.

Tested on AMD X64 hardware.
OvmfIa32 and OvmfIa32X64 on Intel hardware.

v1: Move StandardSignatureIsAuthenticAMD. Handle MSR_IA32_MISC_ENABLE
v2: Incorporate Laszlo's feedback
v3: Typo, not sent
v4: Patch in to add UefiCpuLib to PcAtChipsetPkg.dsc
v5: Patch in to add UefiCpuLib to SourceLevelDebugPkg.dsc
v6: Hopefully reformat patch when sending????

Garrett Kirkendall (4):
  PcAtChipsetPkg: PcAtChipsetPkg.dsc add UefiCpuLib LibraryClass
  SourceLevelDebugPkg: SourceLevelDebugPkg.dsc add UefiCpuLib
    LibraryClass
  UefiCpuPkg: Move StandardSignatureIsAuthenticAMD to BaseUefiCpuLib
  UefiCpuPkg: PiSmmCpuDxeSmm skip MSR_IA32_MISC_ENABLE manipulation on
    AMD

 PcAtChipsetPkg/PcAtChipsetPkg.dsc                            |  2 ++
 SourceLevelDebugPkg/SourceLevelDebugPkg.dsc                  |  2 ++
 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/PiSmmCpuDxeSmm/SmmProfileInternal.h               |  3 ++
 UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c           | 38 ++++++++++++++++++++
 UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c               | 25 ++-----------
 UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 25 ++-----------
 UefiCpuPkg/Library/MpInitLib/MpLib.c                         | 23 ------------
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c                       |  9 ++++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm                 | 19 ++++++++--
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm                  | 20 +++++++++--
 14 files changed, 117 insertions(+), 74 deletions(-)
 create mode 100644 UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c

Changes at:
https://github.com/gkirkendall-amd/edk2/tree/smmentry_nasm_skip_msr_xd_bit_on_amd_v6

Pull Request:
https://github.com/tianocore/edk2/pull/716

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com>

-- 
2.27.0

[PATCH v6 1/4] PcAtChipsetPkg: PcAtChipsetPkg.dsc add UefiCpuLib LibraryClass

[Kirkendall, Garrett]

In preparation for moving StandardSignatureIsAuthenticAMD to UefiCpuLib
in UefiCpuPkg, PcAtChipset/PcAtChipsetPkg.dsc needs LibraryClass
UefiCpuLib.
LocalApicLib|UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf will need
UefiCpuLib LibraryClass.  Likely most "real" platforms will be using
BaseX2XApicLib instance which already required UefiCpuLib.

Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
---
 PcAtChipsetPkg/PcAtChipsetPkg.dsc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dsc b/PcAtChipsetPkg/PcAtChipsetPkg.dsc
index 01a3ee716a98..b61b7d1f528e 100644
--- a/PcAtChipsetPkg/PcAtChipsetPkg.dsc
+++ b/PcAtChipsetPkg/PcAtChipsetPkg.dsc
@@ -2,6 +2,7 @@
 #  PC/AT Chipset Package
 #
 #  Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -39,6 +40,7 @@ [LibraryClasses]
   ResetSystemLib|PcAtChipsetPkg/Library/ResetSystemLib/ResetSystemLib.inf
   IoApicLib|PcAtChipsetPkg/Library/BaseIoApicLib/BaseIoApicLib.inf
   LocalApicLib|UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
+  UefiCpuLib|UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
   ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
   HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
 
-- 
2.27.0

[PATCH v6 2/4] SourceLevelDebugPkg: SourceLevelDebugPkg.dsc add UefiCpuLib LibraryClass

[Kirkendall, Garrett]

In preparation for moving StandardSignatureIsAuthenticAMD to UefiCpuLib
in UefiCpuPkg, SourceLevelDebugPkg/SourceLevelDebugPkg.dsc needs
LibraryClass UefiCpuLib.
LocalApicLib|UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf will need
UefiCpuLib LibraryClass.  Likely most "real" platforms will be using
BaseX2XApicLib instance which already required UefiCpuLib.

Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
---
 SourceLevelDebugPkg/SourceLevelDebugPkg.dsc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/SourceLevelDebugPkg/SourceLevelDebugPkg.dsc b/SourceLevelDebugPkg/SourceLevelDebugPkg.dsc
index a1a1b81d03cb..20eb10ba07f8 100644
--- a/SourceLevelDebugPkg/SourceLevelDebugPkg.dsc
+++ b/SourceLevelDebugPkg/SourceLevelDebugPkg.dsc
@@ -2,6 +2,7 @@
 # Source Level Debug Package.
 #
 # Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
 #
 #    SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -32,6 +33,7 @@ [LibraryClasses.common]
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
   SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
   LocalApicLib|UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
+  UefiCpuLib|UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
   PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
   SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
   PeCoffExtraActionLib|SourceLevelDebugPkg/Library/PeCoffExtraActionLibDebug/PeCoffExtraActionLibDebug.inf
-- 
2.27.0

[PATCH v6 3/4] UefiCpuPkg: Move StandardSignatureIsAuthenticAMD to BaseUefiCpuLib

[Kirkendall, Garrett]

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>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.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

[PATCH v6 4/4] UefiCpuPkg: PiSmmCpuDxeSmm skip MSR_IA32_MISC_ENABLE manipulation on AMD

[Kirkendall, Garrett]

AMD does not support MSR_IA32_MISC_ENABLE.  Accessing that register
causes and exception on AMD processors.  If Execution Disable is
supported, but 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>
---

Notes:
    Tested on Intel hardware with Laszlo Ersek's help
    
    (1) downloaded two Linux images from provided links.
    (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
    
    Tested on real AMD Hardware

 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..993360a8a8c1 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..d7ed9ab7a770 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..167f5e14dbd4 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
+
+; If MSR_IA32_MISC_ENABLE is supported, clear XD Disable bit
+    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                         ; don't try to restore the XD Disable bit just before RSM
+    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..0e154e5db949 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
+
+; If MSR_IA32_MISC_ENABLE is supported, clear XD Disable bit
+    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                         ; don't try to restore the XD Disable bit just before RSM
+    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

Re: [PATCH v6 4/4] UefiCpuPkg: PiSmmCpuDxeSmm skip MSR_IA32_MISC_ENABLE manipulation on AMD

[Laszlo Ersek]

On 06/22/20 15:18, 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, but 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>
> ---

When carrying forward a patch unmodified from the previous version of
the series, then please pick up the feedback tags given under the
previous version.

See e.g.:

https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-28

So, from <https://edk2.groups.io/g/devel/message/61540> /
<http://mid.mail-archive.com/dcfe4164-c021-7ddc-2891-fffc9b1c279d@redhat.com>:

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

BTW the series looks well-formatted to me, on the list, this time around.

Thanks
Laszlo



> 
> Notes:
>     Tested on Intel hardware with Laszlo Ersek's help
>     
>     (1) downloaded two Linux images from provided links.
>     (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
>     
>     Tested on real AMD Hardware
> 
>  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..993360a8a8c1 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..d7ed9ab7a770 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..167f5e14dbd4 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
> +
> +; If MSR_IA32_MISC_ENABLE is supported, clear XD Disable bit
> +    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                         ; don't try to restore the XD Disable bit just before RSM
> +    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..0e154e5db949 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
> +
> +; If MSR_IA32_MISC_ENABLE is supported, clear XD Disable bit
> +    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                         ; don't try to restore the XD Disable bit just before RSM
> +    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
> 

Re: [edk2-devel] [PATCH v6 1/4] PcAtChipsetPkg: PcAtChipsetPkg.dsc add UefiCpuLib LibraryClass

[Guomin Jiang]

Hi Garrett,

Thanks for report quickly and it work and the 'Mac format issue disappear'.

But another build error occur 'error LNK2005: _InitializeFloatingPointUnits already defined in FspSecCoreT.lib(InitializeFpu.obj)'
It seem that result by ApicLib who depend UefiCpuLib now.

I want to know why you add this dependency, have any other way to do it, for example, add the dependency to the dsc file.

Best Regards
Guomin
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Kirkendall, Garrett
> Sent: Monday, June 22, 2020 9:18 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>
> Subject: [edk2-devel] [PATCH v6 1/4] PcAtChipsetPkg: PcAtChipsetPkg.dsc
> add UefiCpuLib LibraryClass
> 
> In preparation for moving StandardSignatureIsAuthenticAMD to UefiCpuLib
> in UefiCpuPkg, PcAtChipset/PcAtChipsetPkg.dsc needs LibraryClass
> UefiCpuLib.
> LocalApicLib|UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf will need
> UefiCpuLib LibraryClass.  Likely most "real" platforms will be using
> BaseX2XApicLib instance which already required UefiCpuLib.
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
> ---
>  PcAtChipsetPkg/PcAtChipsetPkg.dsc | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dsc
> b/PcAtChipsetPkg/PcAtChipsetPkg.dsc
> index 01a3ee716a98..b61b7d1f528e 100644
> --- a/PcAtChipsetPkg/PcAtChipsetPkg.dsc
> +++ b/PcAtChipsetPkg/PcAtChipsetPkg.dsc
> @@ -2,6 +2,7 @@
>  #  PC/AT Chipset Package
>  #
>  #  Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -39,6 +40,7 @@
> [LibraryClasses]
> 
> ResetSystemLib|PcAtChipsetPkg/Library/ResetSystemLib/ResetSystemLib.in
> f
>    IoApicLib|PcAtChipsetPkg/Library/BaseIoApicLib/BaseIoApicLib.inf
>    LocalApicLib|UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
> +  UefiCpuLib|UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> 
> ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseR
> eportStatusCodeLibNull.inf
>    HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> 
> --
> 2.27.0
> 
> 
> 

Re: [edk2-devel] [PATCH v6 1/4] PcAtChipsetPkg: PcAtChipsetPkg.dsc add UefiCpuLib LibraryClass

[Laszlo Ersek]

On 06/23/20 02:52, Guomin Jiang wrote:
> Hi Garrett,
> 
> Thanks for report quickly and it work and the 'Mac format issue disappear'.
> 
> But another build error occur 'error LNK2005: _InitializeFloatingPointUnits already defined in FspSecCoreT.lib(InitializeFpu.obj)'
> It seem that result by ApicLib who depend UefiCpuLib now.

Yes.

> I want to know why you add this dependency,

It was discussed on the list in advance.

[edk2-devel] UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD
function to BaseUefiCpuLib

  https://edk2.groups.io/g/devel/message/61079

In addition, we have investigated all the platforms in the open source
edk2 and edk2-platforms trees that could require an update due to the
new dependency:

  https://edk2.groups.io/g/devel/message/61525

And this series is in fact addressing them all.

> have any other way to do it, for example, add the dependency to the dsc file.

Please describe how you can reproduce the link error using edk2 and
edk2-platforms content *only* (= using open source components only).

Thanks
Laszlo

Re: [edk2-devel] [PATCH v6 0/4] AMD processor MSR_IA32_MISC_ENABLE

[Kirkendall, Garrett]

[AMD Public Use]

Is there anything else needed from me for this patch at this time?

GARRETT KIRKENDALL
SMTS Firmware Engineer | CTE
7171 Southwest Parkway, Austin, TX 78735 USA 
AMD   facebook  |  amd.com

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kirkendall, Garrett via groups.io
Sent: Monday, June 22, 2020 8:18 AM
To: devel@edk2.groups.io
Cc: Eric Dong <eric.dong@intel.com>; Ray Ni <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Hao A Wu <hao.a.wu@intel.com>
Subject: [edk2-devel] [PATCH v6 0/4] AMD processor MSR_IA32_MISC_ENABLE

[CAUTION: External Email]

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.

1. Prepare PcAtChipsetPkg/PcAtChipsetPkg.dsc to move StandardSignatureIsAuthenticAMD into UefiCpuLib LibraryClass BaseUefiCpuLib in UefiCpuPkg.

2. To distinguish between AMD and other processors, refactor StandardSignatureIsAuthenticAMD into BaseUefiCpuLib.  So there is only one copy in the source.

3. Skip manipulation of MSR_IA32_MISC_ENABLE register if running on an AMD processor.

Tested on AMD X64 hardware.
OvmfIa32 and OvmfIa32X64 on Intel hardware.

v1: Move StandardSignatureIsAuthenticAMD. Handle MSR_IA32_MISC_ENABLE
v2: Incorporate Laszlo's feedback
v3: Typo, not sent
v4: Patch in to add UefiCpuLib to PcAtChipsetPkg.dsc
v5: Patch in to add UefiCpuLib to SourceLevelDebugPkg.dsc
v6: Hopefully reformat patch when sending????

Garrett Kirkendall (4):
  PcAtChipsetPkg: PcAtChipsetPkg.dsc add UefiCpuLib LibraryClass
  SourceLevelDebugPkg: SourceLevelDebugPkg.dsc add UefiCpuLib
    LibraryClass
  UefiCpuPkg: Move StandardSignatureIsAuthenticAMD to BaseUefiCpuLib
  UefiCpuPkg: PiSmmCpuDxeSmm skip MSR_IA32_MISC_ENABLE manipulation on
    AMD

 PcAtChipsetPkg/PcAtChipsetPkg.dsc                            |  2 ++
 SourceLevelDebugPkg/SourceLevelDebugPkg.dsc                  |  2 ++
 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/PiSmmCpuDxeSmm/SmmProfileInternal.h               |  3 ++
 UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c           | 38 ++++++++++++++++++++
 UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c               | 25 ++-----------
 UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 25 ++-----------
 UefiCpuPkg/Library/MpInitLib/MpLib.c                         | 23 ------------
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c                       |  9 ++++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm                 | 19 ++++++++--
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm                  | 20 +++++++++--
 14 files changed, 117 insertions(+), 74 deletions(-)  create mode 100644 UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c

Changes at:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fgkirkendall-amd%2Fedk2%2Ftree%2Fsmmentry_nasm_skip_msr_xd_bit_on_amd_v6&amp;data=02%7C01%7Cgarrett.kirkendall%40amd.com%7C5b2918ff7a2345a5ce7f08d816af8e23%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637284290553548804&amp;sdata=F9ktro2rmOouJYui4jqTFK25TK6l1HBl317RW41QDM4%3D&amp;reserved=0

Pull Request:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fpull%2F716&amp;data=02%7C01%7Cgarrett.kirkendall%40amd.com%7C5b2918ff7a2345a5ce7f08d816af8e23%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637284290553548804&amp;sdata=FcHQc%2BzJHMAJfSWc9z%2FZ4Bxh5Ur4EnM%2BaIurKJ0iNYU%3D&amp;reserved=0

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com>

--
2.27.0

Re: [edk2-devel] [PATCH v6 0/4] AMD processor MSR_IA32_MISC_ENABLE

[Laszlo Ersek]

On 06/24/20 03:04, Kirkendall, Garrett wrote:
> [AMD Public Use]
> 
> Is there anything else needed from me for this patch at this time?

I think we still have an open question here:

  https://edk2.groups.io/g/devel/message/61583

To clarify: I'm not suggesting that we should fix that issue. I'm saying
that we should *understand* Guomin's report enough so we can decide
*whether* this series needs a further update, or not.

If there is no feedback (clarification) from Guomin in a few days, I
think Eric or Ray (the UefiCpuPkg maintainers) should merge v6 -- for
example, on Friday.

Thanks,
Laszlo

> 
> GARRETT KIRKENDALL
> SMTS Firmware Engineer | CTE
> 7171 Southwest Parkway, Austin, TX 78735 USA 
> AMD   facebook  |  amd.com
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kirkendall, Garrett via groups.io
> Sent: Monday, June 22, 2020 8:18 AM
> To: devel@edk2.groups.io
> Cc: Eric Dong <eric.dong@intel.com>; Ray Ni <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Hao A Wu <hao.a.wu@intel.com>
> Subject: [edk2-devel] [PATCH v6 0/4] AMD processor MSR_IA32_MISC_ENABLE
> 
> [CAUTION: External Email]
> 
> 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.
> 
> 1. Prepare PcAtChipsetPkg/PcAtChipsetPkg.dsc to move StandardSignatureIsAuthenticAMD into UefiCpuLib LibraryClass BaseUefiCpuLib in UefiCpuPkg.
> 
> 2. To distinguish between AMD and other processors, refactor StandardSignatureIsAuthenticAMD into BaseUefiCpuLib.  So there is only one copy in the source.
> 
> 3. Skip manipulation of MSR_IA32_MISC_ENABLE register if running on an AMD processor.
> 
> Tested on AMD X64 hardware.
> OvmfIa32 and OvmfIa32X64 on Intel hardware.
> 
> v1: Move StandardSignatureIsAuthenticAMD. Handle MSR_IA32_MISC_ENABLE
> v2: Incorporate Laszlo's feedback
> v3: Typo, not sent
> v4: Patch in to add UefiCpuLib to PcAtChipsetPkg.dsc
> v5: Patch in to add UefiCpuLib to SourceLevelDebugPkg.dsc
> v6: Hopefully reformat patch when sending????
> 
> Garrett Kirkendall (4):
>   PcAtChipsetPkg: PcAtChipsetPkg.dsc add UefiCpuLib LibraryClass
>   SourceLevelDebugPkg: SourceLevelDebugPkg.dsc add UefiCpuLib
>     LibraryClass
>   UefiCpuPkg: Move StandardSignatureIsAuthenticAMD to BaseUefiCpuLib
>   UefiCpuPkg: PiSmmCpuDxeSmm skip MSR_IA32_MISC_ENABLE manipulation on
>     AMD
> 
>  PcAtChipsetPkg/PcAtChipsetPkg.dsc                            |  2 ++
>  SourceLevelDebugPkg/SourceLevelDebugPkg.dsc                  |  2 ++
>  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/PiSmmCpuDxeSmm/SmmProfileInternal.h               |  3 ++
>  UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c           | 38 ++++++++++++++++++++
>  UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c               | 25 ++-----------
>  UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 25 ++-----------
>  UefiCpuPkg/Library/MpInitLib/MpLib.c                         | 23 ------------
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c                       |  9 ++++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm                 | 19 ++++++++--
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm                  | 20 +++++++++--
>  14 files changed, 117 insertions(+), 74 deletions(-)  create mode 100644 UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c
> 
> Changes at:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fgkirkendall-amd%2Fedk2%2Ftree%2Fsmmentry_nasm_skip_msr_xd_bit_on_amd_v6&amp;data=02%7C01%7Cgarrett.kirkendall%40amd.com%7C5b2918ff7a2345a5ce7f08d816af8e23%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637284290553548804&amp;sdata=F9ktro2rmOouJYui4jqTFK25TK6l1HBl317RW41QDM4%3D&amp;reserved=0
> 
> Pull Request:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fpull%2F716&amp;data=02%7C01%7Cgarrett.kirkendall%40amd.com%7C5b2918ff7a2345a5ce7f08d816af8e23%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637284290553548804&amp;sdata=FcHQc%2BzJHMAJfSWc9z%2FZ4Bxh5Ur4EnM%2BaIurKJ0iNYU%3D&amp;reserved=0
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
> 
> --
> 2.27.0
> 
> 
> 
> 

Re: [edk2-devel] [PATCH v6 1/4] PcAtChipsetPkg: PcAtChipsetPkg.dsc add UefiCpuLib LibraryClass

[Guomin Jiang]

Hi Laszlo,

The step to reproduce as below:
1. add below change
diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dsc b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
index 26cd3da43c3f..d43cb5be6472 100644
--- a/IntelFsp2Pkg/IntelFsp2Pkg.dsc
+++ b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
@@ -62,7 +62,10 @@ [Components]
   IntelFsp2Pkg/Library/BaseDebugDeviceLibNull/BaseDebugDeviceLibNull.inf
   IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/SecFspSecPlatformLibNull.inf

-  IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf
+  IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf {
+    <LibraryClasses>
+    NULL|UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
+  }
   IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
   IntelFsp2Pkg/FspSecCore/FspSecCoreS.inf
   IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf

2. build -p IntelFsp2Pkg\IntelFsp2Pkg.dsc -b DEBUG -a IA32 -t VS2019 and I can see BaseUefiCpuLib.lib(InitializeFpu.obj) : error LNK2005: _InitializeFloatingPointUnits already defined in FspSecCoreT.lib(InitializeFpu.obj)

I can't reproduce it with the original edk2 or edk2-platforms, but our project have the depend on ApicLib, so if the ApicLib depend on UefiCpuLib, it will break our project build.

Below
Thanks.

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Tuesday, June 23, 2020 7:16 PM
> To: devel@edk2.groups.io; Jiang, Guomin <guomin.jiang@intel.com>;
> garrett.kirkendall@amd.com
> Cc: Ni, Ray <ray.ni@intel.com>; Dong, Eric <eric.dong@intel.com>
> Subject: Re: [edk2-devel] [PATCH v6 1/4] PcAtChipsetPkg:
> PcAtChipsetPkg.dsc add UefiCpuLib LibraryClass
> 
> On 06/23/20 02:52, Guomin Jiang wrote:
> > Hi Garrett,
> >
> > Thanks for report quickly and it work and the 'Mac format issue disappear'.
> >
> > But another build error occur 'error LNK2005: _InitializeFloatingPointUnits
> already defined in FspSecCoreT.lib(InitializeFpu.obj)'
> > It seem that result by ApicLib who depend UefiCpuLib now.
> 
> Yes.
> 
> > I want to know why you add this dependency,
> 
> It was discussed on the list in advance.
> 
> [edk2-devel] UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD
> function to BaseUefiCpuLib
> 
>   https://edk2.groups.io/g/devel/message/61079
> 
> In addition, we have investigated all the platforms in the open source
> edk2 and edk2-platforms trees that could require an update due to the new
> dependency:
> 
>   https://edk2.groups.io/g/devel/message/61525
> 
> And this series is in fact addressing them all.
> 
> > have any other way to do it, for example, add the dependency to the dsc
> file.
> 
> Please describe how you can reproduce the link error using edk2 and edk2-
> platforms content *only* (= using open source components only).
> 
> Thanks
> Laszlo

Re: [edk2-devel] [PATCH v6 1/4] PcAtChipsetPkg: PcAtChipsetPkg.dsc add UefiCpuLib LibraryClass

[Dong, Eric]

Hi Guomin, Laszlo & Garrett,

Because both UefiCpuLib and FspSecCoreT / FspSecCoreM both define InitializeFloatingPointUnits API. So this build issue been reported.

I have submit a patch to remove InitializeFloatingPointUnits in FspSecCoreT / FspSecCoreM, https://edk2.groups.io/g/devel/message/61757.

Please wait that patch been merged before committing this change.

Thanks,
Eric
> -----Original Message-----
> From: Jiang, Guomin <guomin.jiang@intel.com>
> Sent: Sunday, June 28, 2020 5:11 PM
> To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io;
> garrett.kirkendall@amd.com
> Cc: Ni, Ray <ray.ni@intel.com>; Dong, Eric <eric.dong@intel.com>
> Subject: RE: [edk2-devel] [PATCH v6 1/4] PcAtChipsetPkg:
> PcAtChipsetPkg.dsc add UefiCpuLib LibraryClass
> 
> Hi Laszlo,
> 
> The step to reproduce as below:
> 1. add below change
> diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dsc b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> index 26cd3da43c3f..d43cb5be6472 100644
> --- a/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> +++ b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> @@ -62,7 +62,10 @@ [Components]
>    IntelFsp2Pkg/Library/BaseDebugDeviceLibNull/BaseDebugDeviceLibNull.inf
> 
> IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/SecFspSecPlatformLibNull.in
> f
> 
> -  IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf
> +  IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf {
> +    <LibraryClasses>
> +    NULL|UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> +  }
>    IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
>    IntelFsp2Pkg/FspSecCore/FspSecCoreS.inf
>    IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf
> 
> 2. build -p IntelFsp2Pkg\IntelFsp2Pkg.dsc -b DEBUG -a IA32 -t VS2019 and I
> can see BaseUefiCpuLib.lib(InitializeFpu.obj) : error LNK2005:
> _InitializeFloatingPointUnits already defined in
> FspSecCoreT.lib(InitializeFpu.obj)
> 
> I can't reproduce it with the original edk2 or edk2-platforms, but our project
> have the depend on ApicLib, so if the ApicLib depend on UefiCpuLib, it will
> break our project build.
> 
> Below
> Thanks.
> 
> > -----Original Message-----
> > From: Laszlo Ersek <lersek@redhat.com>
> > Sent: Tuesday, June 23, 2020 7:16 PM
> > To: devel@edk2.groups.io; Jiang, Guomin <guomin.jiang@intel.com>;
> > garrett.kirkendall@amd.com
> > Cc: Ni, Ray <ray.ni@intel.com>; Dong, Eric <eric.dong@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v6 1/4] PcAtChipsetPkg:
> > PcAtChipsetPkg.dsc add UefiCpuLib LibraryClass
> >
> > On 06/23/20 02:52, Guomin Jiang wrote:
> > > Hi Garrett,
> > >
> > > Thanks for report quickly and it work and the 'Mac format issue
> disappear'.
> > >
> > > But another build error occur 'error LNK2005:
> > > _InitializeFloatingPointUnits
> > already defined in FspSecCoreT.lib(InitializeFpu.obj)'
> > > It seem that result by ApicLib who depend UefiCpuLib now.
> >
> > Yes.
> >
> > > I want to know why you add this dependency,
> >
> > It was discussed on the list in advance.
> >
> > [edk2-devel] UefiCpuPkg: Discuss: Move
> StandardSignatureIsAuthenticAMD
> > function to BaseUefiCpuLib
> >
> >   https://edk2.groups.io/g/devel/message/61079
> >
> > In addition, we have investigated all the platforms in the open source
> > edk2 and edk2-platforms trees that could require an update due to the
> > new
> > dependency:
> >
> >   https://edk2.groups.io/g/devel/message/61525
> >
> > And this series is in fact addressing them all.
> >
> > > have any other way to do it, for example, add the dependency to the
> > > dsc
> > file.
> >
> > Please describe how you can reproduce the link error using edk2 and
> > edk2- platforms content *only* (= using open source components only).
> >
> > Thanks
> > Laszlo

Re: [edk2-devel] [PATCH v6 0/4] AMD processor MSR_IA32_MISC_ENABLE

[Laszlo Ersek]

Ray, Hao, Eric,

On 06/22/20 15:18, Kirkendall, Garrett 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.
> 
> 1. Prepare PcAtChipsetPkg/PcAtChipsetPkg.dsc to move
> StandardSignatureIsAuthenticAMD into UefiCpuLib LibraryClass
> BaseUefiCpuLib in UefiCpuPkg.
> 
> 2. To distinguish between AMD and other processors, refactor
> StandardSignatureIsAuthenticAMD into BaseUefiCpuLib.  So there is only
> one copy in the source.
> 
> 3. Skip manipulation of MSR_IA32_MISC_ENABLE register if running
> on an AMD processor.
> 
> Tested on AMD X64 hardware.
> OvmfIa32 and OvmfIa32X64 on Intel hardware.
> 
> v1: Move StandardSignatureIsAuthenticAMD. Handle MSR_IA32_MISC_ENABLE
> v2: Incorporate Laszlo's feedback
> v3: Typo, not sent
> v4: Patch in to add UefiCpuLib to PcAtChipsetPkg.dsc
> v5: Patch in to add UefiCpuLib to SourceLevelDebugPkg.dsc
> v6: Hopefully reformat patch when sending????

With <https://bugzilla.tianocore.org/show_bug.cgi?id=2825> fixed (commit
0060e0a694f3), this patch set should be merged now.

I was about to do just that, but the review status of the series is
still incomplete. Even though this patch series has been on the list for
two weeks now.

- Ray, can you please review patch#1 immediately?

- Hao, can you please review patch#2 immediately?

- Ray and Eric, can one of you please review patch #4 at once?

Thanks
Laszlo

Re: [edk2-devel] [PATCH v6 0/4] AMD processor MSR_IA32_MISC_ENABLE

[Dong, Eric]

[-- Attachment #1: Type: text/plain, Size: 6190 bytes --]

Hi Laszlo,

I have fixed the issue reported by Guomin in below change.

SHA-1: 0060e0a694f3f249c3ec081b0e61287c36f64ebb

* IntelFsp2Pkg/FspSecCore: Use UefiCpuLib.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2825

UefiCpuLib has API InitializeFloatingPointUnits.
Remove internal copy of InitializeFloatingPointUnits
in FspSecCoreM, use UefiCpuLib API.

This change also avoid later potential conflict when
use UefiCpuLib for FspSecCoreM module.

Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>

Thanks,
Eric
From: Laszlo Ersek <lersek@redhat.com>
Sent: Wednesday, June 24, 2020 4:54 PM
To: Kirkendall, Garrett <Garrett.Kirkendall@amd.com>; devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>
Subject: Re: [edk2-devel] [PATCH v6 0/4] AMD processor MSR_IA32_MISC_ENABLE

On 06/24/20 03:04, Kirkendall, Garrett wrote:
> [AMD Public Use]
>
> Is there anything else needed from me for this patch at this time?

I think we still have an open question here:

  https://edk2.groups.io/g/devel/message/61583

To clarify: I'm not suggesting that we should fix that issue. I'm saying
that we should *understand* Guomin's report enough so we can decide
*whether* this series needs a further update, or not.

If there is no feedback (clarification) from Guomin in a few days, I
think Eric or Ray (the UefiCpuPkg maintainers) should merge v6 -- for
example, on Friday.

Thanks,
Laszlo

>
> GARRETT KIRKENDALL
> SMTS Firmware Engineer | CTE
> 7171 Southwest Parkway, Austin, TX 78735 USA
> AMD   facebook  |  amd.com
>
> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Kirkendall, Garrett via groups.io
> Sent: Monday, June 22, 2020 8:18 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Eric Dong <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Hao A Wu <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
> Subject: [edk2-devel] [PATCH v6 0/4] AMD processor MSR_IA32_MISC_ENABLE
>
> [CAUTION: External Email]
>
> 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.
>
> 1. Prepare PcAtChipsetPkg/PcAtChipsetPkg.dsc to move StandardSignatureIsAuthenticAMD into UefiCpuLib LibraryClass BaseUefiCpuLib in UefiCpuPkg.
>
> 2. To distinguish between AMD and other processors, refactor StandardSignatureIsAuthenticAMD into BaseUefiCpuLib.  So there is only one copy in the source.
>
> 3. Skip manipulation of MSR_IA32_MISC_ENABLE register if running on an AMD processor.
>
> Tested on AMD X64 hardware.
> OvmfIa32 and OvmfIa32X64 on Intel hardware.
>
> v1: Move StandardSignatureIsAuthenticAMD. Handle MSR_IA32_MISC_ENABLE
> v2: Incorporate Laszlo's feedback
> v3: Typo, not sent
> v4: Patch in to add UefiCpuLib to PcAtChipsetPkg.dsc
> v5: Patch in to add UefiCpuLib to SourceLevelDebugPkg.dsc
> v6: Hopefully reformat patch when sending????
>
> Garrett Kirkendall (4):
>   PcAtChipsetPkg: PcAtChipsetPkg.dsc add UefiCpuLib LibraryClass
>   SourceLevelDebugPkg: SourceLevelDebugPkg.dsc add UefiCpuLib
>     LibraryClass
>   UefiCpuPkg: Move StandardSignatureIsAuthenticAMD to BaseUefiCpuLib
>   UefiCpuPkg: PiSmmCpuDxeSmm skip MSR_IA32_MISC_ENABLE manipulation on
>     AMD
>
>  PcAtChipsetPkg/PcAtChipsetPkg.dsc                            |  2 ++
>  SourceLevelDebugPkg/SourceLevelDebugPkg.dsc                  |  2 ++
>  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/PiSmmCpuDxeSmm/SmmProfileInternal.h               |  3 ++
>  UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c           | 38 ++++++++++++++++++++
>  UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c               | 25 ++-----------
>  UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 25 ++-----------
>  UefiCpuPkg/Library/MpInitLib/MpLib.c                         | 23 ------------
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c                       |  9 ++++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm                 | 19 ++++++++--
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm                  | 20 +++++++++--
>  14 files changed, 117 insertions(+), 74 deletions(-)  create mode 100644 UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c
>
> Changes at:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fgkirkendall-amd%2Fedk2%2Ftree%2Fsmmentry_nasm_skip_msr_xd_bit_on_amd_v6&amp;data=02%7C01%7Cgarrett.kirkendall%40amd.com%7C5b2918ff7a2345a5ce7f08d816af8e23%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637284290553548804&amp;sdata=F9ktro2rmOouJYui4jqTFK25TK6l1HBl317RW41QDM4%3D&amp;reserved=0
>
> Pull Request:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fpull%2F716&amp;data=02%7C01%7Cgarrett.kirkendall%40amd.com%7C5b2918ff7a2345a5ce7f08d816af8e23%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637284290553548804&amp;sdata=FcHQc%2BzJHMAJfSWc9z%2FZ4Bxh5Ur4EnM%2BaIurKJ0iNYU%3D&amp;reserved=0
>
> Cc: Eric Dong <eric.dong@intel.com<mailto:eric.dong@intel.com>>
> Cc: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
> Cc: Hao A Wu <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
> Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com<mailto:garrett.kirkendall@amd.com>>
>
> --
> 2.27.0
>
>
> 
>

[-- Attachment #2: Type: text/html, Size: 19610 bytes --]

Re: [PATCH v6 4/4] UefiCpuPkg: PiSmmCpuDxeSmm skip MSR_IA32_MISC_ENABLE manipulation on AMD

[Dong, Eric]

Reviewed-by: Eric Dong <eric.dong@intel.com>

> -----Original Message-----
> From: Garrett Kirkendall <Garrett.Kirkendall@amd.com>
> Sent: Monday, June 22, 2020 9:18 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo
> Ersek <lersek@redhat.com>
> Subject: [PATCH v6 4/4] UefiCpuPkg: PiSmmCpuDxeSmm skip
> MSR_IA32_MISC_ENABLE manipulation on AMD
> 
> AMD does not support MSR_IA32_MISC_ENABLE.  Accessing that register
> causes and exception on AMD processors.  If Execution Disable is supported,
> but 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>
> ---
> 
> Notes:
>     Tested on Intel hardware with Laszlo Ersek's help
> 
>     (1) downloaded two Linux images from provided links.
>     (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_GCC
> 5/FV/OVMF_CODE.fd \
>         -drive
> if=pflash,format=raw,unit=1,snapshot=on,file=Build/OvmfIa32/DEBUG_GCC
> 5/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_GCC
> 5/FV/OVMF_CODE.fd \
>         -drive
> if=pflash,format=raw,unit=1,snapshot=on,file=Build/Ovmf3264/DEBUG_GCC
> 5/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
> 
>     Tested on real AMD Hardware
> 
>  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..993360a8a8c1 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..d7ed9ab7a770 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..167f5e14dbd4 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
> +
> +; If MSR_IA32_MISC_ENABLE is supported, clear XD Disable bit
> +    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                         ; don't try to restore the XD Disable bit just before
> RSM
> +    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..0e154e5db949 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
> +
> +; If MSR_IA32_MISC_ENABLE is supported, clear XD Disable bit
> +    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                         ; don't try to restore the XD Disable bit just before
> RSM
> +    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

Re: [PATCH v6 3/4] UefiCpuPkg: Move StandardSignatureIsAuthenticAMD to BaseUefiCpuLib

[Dong, Eric]

Reviewed-by: Eric Dong <eric.dong@intel.com>

> -----Original Message-----
> From: Garrett Kirkendall <Garrett.Kirkendall@amd.com>
> Sent: Monday, June 22, 2020 9:18 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo
> Ersek <lersek@redhat.com>
> Subject: [PATCH v6 3/4] 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>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Reviewed-by: Eric Dong <eric.dong@intel.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

Re: [PATCH v6 2/4] SourceLevelDebugPkg: SourceLevelDebugPkg.dsc add UefiCpuLib LibraryClass

[Wu, Hao A]

> -----Original Message-----
> From: Garrett Kirkendall <Garrett.Kirkendall@amd.com>
> Sent: Monday, June 22, 2020 9:18 PM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>
> Subject: [PATCH v6 2/4] SourceLevelDebugPkg: SourceLevelDebugPkg.dsc
> add UefiCpuLib LibraryClass
> 
> In preparation for moving StandardSignatureIsAuthenticAMD to UefiCpuLib
> in UefiCpuPkg, SourceLevelDebugPkg/SourceLevelDebugPkg.dsc needs
> LibraryClass UefiCpuLib.
> LocalApicLib|UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf will need
> UefiCpuLib LibraryClass.  Likely most "real" platforms will be using
> BaseX2XApicLib instance which already required UefiCpuLib.
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
> ---
>  SourceLevelDebugPkg/SourceLevelDebugPkg.dsc | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/SourceLevelDebugPkg/SourceLevelDebugPkg.dsc
> b/SourceLevelDebugPkg/SourceLevelDebugPkg.dsc
> index a1a1b81d03cb..20eb10ba07f8 100644
> --- a/SourceLevelDebugPkg/SourceLevelDebugPkg.dsc
> +++ b/SourceLevelDebugPkg/SourceLevelDebugPkg.dsc
> @@ -2,6 +2,7 @@
>  # Source Level Debug Package.
>  #
>  # Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2020, AMD Incorporated. All rights reserved.<BR>
>  #
>  #    SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -32,6 +33,7 @@ [LibraryClasses.common]
>    IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> 
> SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchroniz
> ationLib.inf
>    LocalApicLib|UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
> +  UefiCpuLib|UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf


Really sorry for the delayed response.
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu


> 
> PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BaseP
> eCoffGetEntryPointLib.inf
> 
> SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPor
> tLib16550.inf
> 
> PeCoffExtraActionLib|SourceLevelDebugPkg/Library/PeCoffExtraActionLibD
> ebug/PeCoffExtraActionLibDebug.inf
> --
> 2.27.0

Re: [edk2-devel] [PATCH v6 1/4] PcAtChipsetPkg: PcAtChipsetPkg.dsc add UefiCpuLib LibraryClass

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 40 bytes --]

Reviewed-by: Ray Ni <ray.ni@intel.com>

[-- Attachment #2: Type: text/html, Size: 99 bytes --]

Re: [edk2-devel] [PATCH v6 0/4] AMD processor MSR_IA32_MISC_ENABLE

[Laszlo Ersek]

Hi Eric,

On 07/07/20 04:38, Dong, Eric wrote:
> Hi Laszlo,
> 
> I have fixed the issue reported by Guomin in below change.
> 
> SHA-1: 0060e0a694f3f249c3ec081b0e61287c36f64ebb
> 
> * IntelFsp2Pkg/FspSecCore: Use UefiCpuLib.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2825

Thank you very much for filing and fixing that BZ!

Thank you all for granting the necessary feedback tags, as well.

Series merged as commit range 627d1d6693b0..bdafda8c457e, via
<https://github.com/tianocore/edk2/pull/769>.

Thanks,
Laszlo

Re: [edk2-devel] [PATCH v6 0/4] AMD processor MSR_IA32_MISC_ENABLE

[Guomin Jiang]

Thanks all of you that response my confusion.

Both of you are so kindly.

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
> Ersek
> Sent: Wednesday, July 8, 2020 7:32 AM
> To: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Wu, Hao
> A <hao.a.wu@intel.com>
> Cc: devel@edk2.groups.io; Kirkendall, Garrett
> <Garrett.Kirkendall@amd.com>; Jiang, Guomin <guomin.jiang@intel.com>
> Subject: Re: [edk2-devel] [PATCH v6 0/4] AMD processor
> MSR_IA32_MISC_ENABLE
> 
> Hi Eric,
> 
> On 07/07/20 04:38, Dong, Eric wrote:
> > Hi Laszlo,
> >
> > I have fixed the issue reported by Guomin in below change.
> >
> > SHA-1: 0060e0a694f3f249c3ec081b0e61287c36f64ebb
> >
> > * IntelFsp2Pkg/FspSecCore: Use UefiCpuLib.
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2825
> 
> Thank you very much for filing and fixing that BZ!
> 
> Thank you all for granting the necessary feedback tags, as well.
> 
> Series merged as commit range 627d1d6693b0..bdafda8c457e, via
> <https://github.com/tianocore/edk2/pull/769>.
> 
> Thanks,
> Laszlo
> 
> 
>