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

2020-07-06 Thread Dong, Eric 
Reviewed-by: Eric Dong 

> -Original Message-
> From: Garrett Kirkendall 
> Sent: Monday, June 22, 2020 9:18 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Laszlo
> Ersek 
> 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 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Signed-off-by: Garrett Kirkendall 
> ---
> 
> 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.
> +Copyright (c) 2020, AMD Incorporated. All rights reserved.
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -13,6 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  #include
> 
>  #include   #include 
> +#include 
>  #include 
> 
>  #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. -
> Copyright (c) 2017, AMD Incorporated. All rights reserved.
> +Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.
> 
>  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

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

2020-06-22 Thread 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 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Signed-off-by: Garrett Kirkendall 
> ---

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  /
http://mid.mail-archive.com/dcfe4164-c021-7ddc-2891-fffc9b1c279d@redhat.com>:

Reviewed-by: Laszlo Ersek 
Tested-by: Laszlo Ersek 

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.
> +Copyright (c) 2020, AMD Incorporated. All rights reserved.
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -13,6 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #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.
> -Copyright (c) 2017, AMD Incorporated. All rights reserved.
> +Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.
>  
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> @

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

2020-06-22 Thread 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 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Signed-off-by: Garrett Kirkendall 
---

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.
+Copyright (c) 2020, AMD Incorporated. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -13,6 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #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.
-Copyright (c) 2017, AMD Incorporated. All rights reserved.
+Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.
 
 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.
+; Copyright (c) 2020, AMD Incorporated. All rights reserved.
 ; SPDX-License-Identifier: BSD-2-Clause-Patent
 ;
 ; Module Name:
@@