Re: [PATCH 4/6] UefiCpuPkg/PiSmmCpuDxeSmm: Enable NXE if it's supported

["Dong, Eric" <eric.dong@intel.com>]

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

> -----Original Message-----
> From: Wang, Jian J
> Sent: Monday, January 15, 2018 4:55 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>;
> Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH 4/6] UefiCpuPkg/PiSmmCpuDxeSmm: Enable NXE if it's
> supported
> 
> If PcdDxeNxMemoryProtectionPolicy is set to enable protection for memory
> of EfiBootServicesCode, EfiConventionalMemory, the BIOS will hang at a
> page fault exception triggered by PiSmmCpuDxeSmm.
> 
> The root cause is that PiSmmCpuDxeSmm will access default SMM RAM
> starting at 0x30000 which is marked as non-executable, but NX feature was
> not enabled during SMM initialization. Accessing memory which has invalid
> attributes set will cause page fault exception. This patch fixes it by checking
> NX capability in cpuid and enable NXE in EFER MSR if it's available.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm | 14 ++++++++++++++
> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm  | 12 +++++++++++-
>  2 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
> index d9df3626c7..db172f108a 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmInit.nasm
> @@ -42,6 +42,11 @@ ASM_PFX(gcSmiInitGdtr):
> 
>  global ASM_PFX(SmmStartup)
>  ASM_PFX(SmmStartup):
> +    DB      0x66
> +    mov     eax, 0x80000001             ; read capability
> +    cpuid
> +    DB      0x66
> +    mov     ebx, edx                    ; rdmsr will change edx. keep it in ebx.
>      DB      0x66, 0xb8
>  ASM_PFX(gSmmCr3): DD 0
>      mov     cr3, eax
> @@ -50,6 +55,15 @@ ASM_PFX(gSmmCr3): DD 0
>      DB      0x66, 0xb8
>  ASM_PFX(gSmmCr4): DD 0
>      mov     cr4, eax
> +    DB      0x66
> +    mov     ecx, 0xc0000080             ; IA32_EFER MSR
> +    rdmsr
> +    DB      0x66
> +    test    ebx, BIT20                  ; check NXE capability
> +    jz      .1
> +    or      ah, BIT3                    ; set NXE bit
> +    wrmsr
> +.1:
>      DB      0x66, 0xb8
>  ASM_PFX(gSmmCr0): DD 0
>      DB      0xbf, PROTECT_MODE_DS, 0    ; mov di, PROTECT_MODE_DS
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm
> index 9d05e2cb05..2a3a1141c3 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmInit.nasm
> @@ -42,6 +42,11 @@ ASM_PFX(gcSmiInitGdtr):
> 
>  global ASM_PFX(SmmStartup)
>  ASM_PFX(SmmStartup):
> +    DB      0x66
> +    mov     eax, 0x80000001             ; read capability
> +    cpuid
> +    DB      0x66
> +    mov     ebx, edx                    ; rdmsr will change edx. keep it in ebx.
>      DB      0x66, 0xb8                   ; mov eax, imm32
>  ASM_PFX(gSmmCr3): DD 0
>      mov     cr3, rax
> @@ -54,7 +59,12 @@ ASM_PFX(gSmmCr4): DD 0
>      DB      0x66
>      mov     ecx, 0xc0000080             ; IA32_EFER MSR
>      rdmsr
> -    or      ah, 1                       ; set LME bit
> +    or      ah, BIT0                    ; set LME bit
> +    DB      0x66
> +    test    ebx, BIT20                  ; check NXE capability
> +    jz      .1
> +    or      ah, BIT3                    ; set NXE bit
> +.1:
>      wrmsr
>      DB      0x66, 0xb8                   ; mov eax, imm32
>  ASM_PFX(gSmmCr0): DD 0
> --
> 2.15.1.windows.2