Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR4.CET before restoring MSR IA32_S_CET

["duntan" <dun.tan@intel.com>]

Hi Wei,

Could you please split the patch to into separate patches?
Different patch for different issue may looks clearer.

Thanks,
Dun

-----Original Message-----
From: Sheng, W <w.sheng@intel.com> 
Sent: Friday, November 3, 2023 1:35 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Tan, Dun <dun.tan@intel.com>
Subject: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR4.CET before restoring MSR IA32_S_CET

Clear CR4.CET bit before restoring MSR IA32_S_CET.
Backup/restore MSR IA32_U_CET in SMI.
Use current CR4 value when changing CR4.CET.
Initial mSmmInterruptSspTables to 0.

Signed-off-by: Sheng Wei <w.sheng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Cc: Tan Dun <dun.tan@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm | 62 +++++++++++++----  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm  | 72 ++++++++++++++++----  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c |  2 +-
 3 files changed, 107 insertions(+), 29 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
index 19de5f614e..a087576a54 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
@@ -16,18 +16,19 @@
 %include "StuffRsbNasm.inc" %include "Nasm.inc" +%define MSR_IA32_U_CET                     0x6A0 %define MSR_IA32_S_CET                     0x6A2-%define   MSR_IA32_CET_SH_STK_EN             0x1-%define   MSR_IA32_CET_WR_SHSTK_EN           0x2-%define   MSR_IA32_CET_ENDBR_EN              0x4-%define   MSR_IA32_CET_LEG_IW_EN             0x8-%define   MSR_IA32_CET_NO_TRACK_EN           0x10-%define   MSR_IA32_CET_SUPPRESS_DIS          0x20-%define   MSR_IA32_CET_SUPPRESS              0x400-%define   MSR_IA32_CET_TRACKER               0x800+%define MSR_IA32_CET_SH_STK_EN             0x1+%define MSR_IA32_CET_WR_SHSTK_EN           0x2+%define MSR_IA32_CET_ENDBR_EN              0x4+%define MSR_IA32_CET_LEG_IW_EN             0x8+%define MSR_IA32_CET_NO_TRACK_EN           0x10+%define MSR_IA32_CET_SUPPRESS_DIS          0x20+%define MSR_IA32_CET_SUPPRESS              0x400+%define MSR_IA32_CET_TRACKER               0x800 %define MSR_IA32_PL0_SSP                   0x6A4 -%define CR4_CET                            0x800000+%define CR4_CET_BIT                        23  %define MSR_IA32_MISC_ENABLE 0x1A0 %define MSR_EFER      0xc0000080@@ -214,11 +215,21 @@ ASM_PFX(mPatchCetSupported):
     push    edx     push    eax +    mov     ecx, MSR_IA32_U_CET+    rdmsr+    push    edx+    push    eax+     mov     ecx, MSR_IA32_PL0_SSP     rdmsr     push    edx     push    eax +    mov     ecx, MSR_IA32_U_CET+    xor     eax, eax+    xor     edx, edx+    wrmsr+     mov     ecx, MSR_IA32_S_CET     mov     eax, MSR_IA32_CET_SH_STK_EN     xor     edx, edx@@ -249,7 +260,8 @@ CetInterruptDone:
     bts     ecx, 16                     ; set WP     mov     cr0, ecx -    mov     eax, 0x668 | CR4_CET+    mov     eax, cr4+    bts     eax, CR4_CET_BIT     mov     cr4, eax      setssbsy@@ -276,18 +288,30 @@ CetDone:
     cmp     al, 0     jz      CetDone2 -    mov     eax, 0x668-    mov     cr4, eax       ; disable CET+    mov     ecx, MSR_IA32_S_CET+    xor     eax, eax+    xor     edx, edx+    wrmsr++    ; clear CR4.CET bit+    mov     eax, cr4+    btr     eax, CR4_CET_BIT+    mov     cr4, eax      mov     ecx, MSR_IA32_PL0_SSP     pop     eax     pop     edx     wrmsr -    mov     ecx, MSR_IA32_S_CET+    mov     ecx, MSR_IA32_U_CET     pop     eax     pop     edx     wrmsr++    mov     ecx, MSR_IA32_S_CET+    pop     eax+    pop     edx+    mov     ebx, eax CetDone2:      mov     eax, ASM_PFX(mXdSupported)@@ -305,6 +329,18 @@ CetDone2:
 .7:      StuffRsb32++    mov     eax, ASM_PFX(mCetSupported)+    mov     al, [eax]+    cmp     al, 0+    jz      CetDone3++    mov     ecx, MSR_IA32_S_CET+    mov     eax, ebx+    xor     edx, edx+    wrmsr+CetDone3:+     rsm  ASM_PFX(gcSmiHandlerSize): DW $ - _SmiEntryPointdiff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
index d302ca8d01..7aed7c8dda 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
@@ -20,19 +20,20 @@
 ; Variables referenced by C code ; +%define MSR_IA32_U_CET                     0x6A0 %define MSR_IA32_S_CET                     0x6A2-%define   MSR_IA32_CET_SH_STK_EN             0x1-%define   MSR_IA32_CET_WR_SHSTK_EN           0x2-%define   MSR_IA32_CET_ENDBR_EN              0x4-%define   MSR_IA32_CET_LEG_IW_EN             0x8-%define   MSR_IA32_CET_NO_TRACK_EN           0x10-%define   MSR_IA32_CET_SUPPRESS_DIS          0x20-%define   MSR_IA32_CET_SUPPRESS              0x400-%define   MSR_IA32_CET_TRACKER               0x800+%define MSR_IA32_CET_SH_STK_EN             0x1+%define MSR_IA32_CET_WR_SHSTK_EN           0x2+%define MSR_IA32_CET_ENDBR_EN              0x4+%define MSR_IA32_CET_LEG_IW_EN             0x8+%define MSR_IA32_CET_NO_TRACK_EN           0x10+%define MSR_IA32_CET_SUPPRESS_DIS          0x20+%define MSR_IA32_CET_SUPPRESS              0x400+%define MSR_IA32_CET_TRACKER               0x800 %define MSR_IA32_PL0_SSP                   0x6A4 %define MSR_IA32_INTERRUPT_SSP_TABLE_ADDR  0x6A8 -%define CR4_CET                            0x800000+%define CR4_CET_BIT                        23  %define MSR_IA32_MISC_ENABLE 0x1A0 %define MSR_EFER      0xc0000080@@ -230,6 +231,11 @@ ASM_PFX(mPatchCetSupported):
     push    rdx     push    rax +    mov     ecx, MSR_IA32_U_CET+    rdmsr+    push    rdx+    push    rax+     mov     ecx, MSR_IA32_PL0_SSP     rdmsr     push    rdx@@ -240,6 +246,11 @@ ASM_PFX(mPatchCetSupported):
     push    rdx     push    rax +    mov     ecx, MSR_IA32_U_CET+    xor     eax, eax+    xor     edx, edx+    wrmsr+     mov     ecx, MSR_IA32_S_CET     mov     eax, MSR_IA32_CET_SH_STK_EN     xor     edx, edx@@ -276,7 +287,8 @@ CetInterruptDone:
     bts     ecx, 16                     ; set WP     mov     cr0, rcx -    mov     eax, 0x668 | CR4_CET+    mov     rax, cr4+    bts     rax, CR4_CET_BIT     mov     cr4, rax      setssbsy@@ -316,13 +328,20 @@ CpuSmmDebugExitAbsAddr:
     add     rsp, 0x200      mov     rax, strict qword 0        ;    mov     rax, ASM_PFX(mCetSupported)-mCetSupportedAbsAddr:+mCetSupportedAbsAddr1:     mov     al, [rax]     cmp     al, 0     jz      CetDone2 -    mov     eax, 0x668-    mov     cr4, rax       ; disable CET+    mov     ecx, MSR_IA32_S_CET+    xor     eax, eax+    xor     edx, edx+    wrmsr++    ; clear CR4.CET bit+    mov     rax, cr4+    btr     rax, CR4_CET_BIT+    mov     cr4, rax      mov     ecx, MSR_IA32_INTERRUPT_SSP_TABLE_ADDR     pop     rax@@ -334,10 +353,15 @@ mCetSupportedAbsAddr:
     pop     rdx     wrmsr -    mov     ecx, MSR_IA32_S_CET+    mov     ecx, MSR_IA32_U_CET     pop     rax     pop     rdx     wrmsr++    mov     ecx, MSR_IA32_S_CET+    pop     rax+    pop     rdx+    mov     ebx, eax CetDone2:      mov     rax, strict qword 0         ;       lea     rax, [ASM_PFX(mXdSupported)]@@ -356,6 +380,19 @@ mXdSupportedAbsAddr:
 .1:      StuffRsb64++    mov     rax, strict qword 0        ;    mov     rax, ASM_PFX(mCetSupported)+mCetSupportedAbsAddr2:+    mov     al, [rax]+    cmp     al, 0+    jz      CetDone3++    mov     ecx, MSR_IA32_S_CET+    mov     eax, ebx+    xor     edx, edx+    wrmsr+CetDone3:+     rsm  ASM_PFX(gcSmiHandlerSize)    DW      $ - _SmiEntryPoint@@ -391,6 +428,11 @@ ASM_PFX(PiSmmCpuSmiEntryFixupAddress):
     mov    qword [rcx - 8], rax      lea    rax, [ASM_PFX(mCetSupported)]-    lea    rcx, [mCetSupportedAbsAddr]+    lea    rcx, [mCetSupportedAbsAddr1]     mov    qword [rcx - 8], rax++    lea    rax, [ASM_PFX(mCetSupported)]+    lea    rcx, [mCetSupportedAbsAddr2]+    mov    qword [rcx - 8], rax+     retdiff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
index c4f21e2155..6c53213b0b 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
@@ -20,7 +20,7 @@ UINT32                    mCetPl0Ssp;
 UINT32                    mCetInterruptSsp; UINT32                    mCetInterruptSspTable; -UINTN  mSmmInterruptSspTables;+UINTN  mSmmInterruptSspTables = 0;  /**   Initialize IDT IST Field.-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110618): https://edk2.groups.io/g/devel/message/110618
Mute This Topic: https://groups.io/mt/102358752/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-