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.d...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Signed-off-by: Garrett Kirkendall <garrett.kirkend...@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> / <dcfe4164-c021-7ddc-2891-fffc9b1c279d@redhat.com">http://mid.mail-archive.com/dcfe4164-c021-7ddc-2891-fffc9b1c279d@redhat.com>: Reviewed-by: Laszlo Ersek <ler...@redhat.com> Tested-by: Laszlo Ersek <ler...@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 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#61562): https://edk2.groups.io/g/devel/message/61562 Mute This Topic: https://groups.io/mt/75037839/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
