SMM emulation under KVM crashes the guest when the "jz" branch, added in
commit d4d87596c11d ("UefiCpuPkg/PiSmmCpuDxeSmm: Enable NXE if it's
supported", 2018-01-18), is taken.

Rework the propagation of CPUID.80000001H:EDX.NX [bit 20] to IA32_EFER.NXE
[bit 11] so that no code is executed conditionally.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Ref: http://mid.mail-archive.com/d6fff558-6c4f-9ca6-74a7-e7cd9d007276@redhat.com
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
index 9231aa5b3ded..102e0bdbabc8 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
@@ -44,26 +44,25 @@ global ASM_PFX(SmmStartup)
 
 BITS 16
 ASM_PFX(SmmStartup):
     mov     eax, 0x80000001             ; read capability
     cpuid
     mov     ebx, edx                    ; rdmsr will change edx. keep it in ebx.
+    and     ebx, BIT20                  ; extract XD capability bit
+    shr     ebx, 9                      ; shift bit to IA32_EFER NXE position
     DB      0x66, 0xb8                  ; mov eax, imm32
 ASM_PFX(gSmmCr3): DD 0
     mov     cr3, eax
 o32 lgdt    [cs:ebp + (ASM_PFX(gcSmiInitGdtr) - ASM_PFX(SmmStartup))]
     DB      0x66, 0xb8                  ; mov eax, imm32
 ASM_PFX(gSmmCr4): DD 0
     mov     cr4, eax
     mov     ecx, 0xc0000080             ; IA32_EFER MSR
     rdmsr
-    test    ebx, BIT20                  ; check NXE capability
-    jz      .1
-    or      ah, BIT3                    ; set NXE bit
+    or      eax, ebx                    ; set NXE bit if XD is available
     wrmsr
-.1:
     DB      0x66, 0xb8                  ; mov eax, imm32
 ASM_PFX(gSmmCr0): DD 0
     mov     di, PROTECT_MODE_DS
     mov     cr0, eax
     DB      0x66, 0xea                  ; jmp far [ptr48]
 ASM_PFX(gSmmJmpAddr):
-- 
2.14.1.3.gb7cf6e02401b

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On 1/30/2018 11:33 PM, Laszlo Ersek wrote:
> SMM emulation under KVM crashes the guest when the "jz" branch, added in
> commit d4d87596c11d ("UefiCpuPkg/PiSmmCpuDxeSmm: Enable NXE if it's
> supported", 2018-01-18), is taken.
> 
> Rework the propagation of CPUID.80000001H:EDX.NX [bit 20] to IA32_EFER.NXE
> [bit 11] so that no code is executed conditionally.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Ref: http://mid.mail-archive.com/d6fff558-6c4f-9ca6-74a7-e7cd9d007276@redhat.com
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>   UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
> index 9231aa5b3ded..102e0bdbabc8 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
> @@ -44,26 +44,25 @@ global ASM_PFX(SmmStartup)
>   
>   BITS 16
>   ASM_PFX(SmmStartup):
>       mov     eax, 0x80000001             ; read capability
>       cpuid
>       mov     ebx, edx                    ; rdmsr will change edx. keep it in ebx.
> +    and     ebx, BIT20                  ; extract XD capability bit
Per CPUID_EXTENDED_CPU_SIG_EDX definition in
UefiCpuPkg/Include/Register/CpuId.h, the BIT name is NX.

> +    shr     ebx, 9                      ; shift bit to IA32_EFER NXE position
How about changing above comments to:
; shift bit to IA32_EFER.NXE[BIT11] position?

>       DB      0x66, 0xb8                  ; mov eax, imm32
>   ASM_PFX(gSmmCr3): DD 0
>       mov     cr3, eax
>   o32 lgdt    [cs:ebp + (ASM_PFX(gcSmiInitGdtr) - ASM_PFX(SmmStartup))]
>       DB      0x66, 0xb8                  ; mov eax, imm32
>   ASM_PFX(gSmmCr4): DD 0
>       mov     cr4, eax
>       mov     ecx, 0xc0000080             ; IA32_EFER MSR
>       rdmsr
> -    test    ebx, BIT20                  ; check NXE capability
> -    jz      .1
> -    or      ah, BIT3                    ; set NXE bit
> +    or      eax, ebx                    ; set NXE bit if XD is available
; Bit name is NX.
>       wrmsr
> -.1:
>       DB      0x66, 0xb8                  ; mov eax, imm32
>   ASM_PFX(gSmmCr0): DD 0
>       mov     di, PROTECT_MODE_DS
>       mov     cr0, eax
>       DB      0x66, 0xea                  ; jmp far [ptr48]
>   ASM_PFX(gSmmJmpAddr):
> 
Very clever solution to remove jz.
With the minor comments update,
Reviewed-by: Ruiyu Ni <ruiyu.ni@Intel.com>

-- 
Thanks,
Ray
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel