* [edk2-devel] [PATCH v2 0/3] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR4.CET before
@ 2023-11-06 9:07 Sheng Wei
2023-11-06 9:07 ` [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR4.CET before restoring MSR IA32_S_CET Sheng Wei
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Sheng Wei @ 2023-11-06 9:07 UTC (permalink / raw)
To: devel
Patch V2:
No function change with Patch V1.
Split the patch to into 3 separate patches.
Sheng Wei (3):
UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR4.CET before restoring MSR
IA32_S_CET
UefiCpuPkg/PiSmmCpuDxeSmm: Change CR4.CET bit only
UefiCpuPkg/PiSmmCpuDxeSmm: Set mSmmInterruptSspTables initial value
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(-)
--
2.26.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110742): https://edk2.groups.io/g/devel/message/110742
Mute This Topic: https://groups.io/mt/102416571/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 8+ messages in thread* [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR4.CET before restoring MSR IA32_S_CET 2023-11-06 9:07 [edk2-devel] [PATCH v2 0/3] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR4.CET before Sheng Wei @ 2023-11-06 9:07 ` Sheng Wei 2023-11-08 21:16 ` Laszlo Ersek 2023-11-06 9:07 ` [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/PiSmmCpuDxeSmm: Change CR4.CET bit only Sheng Wei 2023-11-06 9:07 ` [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Set mSmmInterruptSspTables initial value Sheng Wei 2 siblings, 1 reply; 8+ messages in thread From: Sheng Wei @ 2023-11-06 9:07 UTC (permalink / raw) To: devel; +Cc: Eric Dong, Ray Ni, Laszlo Ersek, Wu Jiaxin, Tan Dun Clear CR4.CET bit before restoring MSR IA32_S_CET. Backup/restore MSR IA32_U_CET in SMI. 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 | 53 ++++++++++++--- UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 69 ++++++++++++++++---- 2 files changed, 98 insertions(+), 24 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm index 19de5f614e..68332e2c3f 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 @@ -276,6 +287,11 @@ CetDone: cmp al, 0 jz CetDone2 + mov ecx, MSR_IA32_S_CET + xor eax, eax + xor edx, edx + wrmsr + mov eax, 0x668 mov cr4, eax ; disable CET @@ -284,10 +300,15 @@ CetDone: 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 +326,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 $ - _SmiEntryPoint diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm index d302ca8d01..007fbff640 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 @@ -316,13 +327,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 +352,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 +379,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 +427,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 + ret -- 2.26.2.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110743): https://edk2.groups.io/g/devel/message/110743 Mute This Topic: https://groups.io/mt/102416572/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR4.CET before restoring MSR IA32_S_CET 2023-11-06 9:07 ` [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR4.CET before restoring MSR IA32_S_CET Sheng Wei @ 2023-11-08 21:16 ` Laszlo Ersek 2023-11-09 7:50 ` Sheng Wei 0 siblings, 1 reply; 8+ messages in thread From: Laszlo Ersek @ 2023-11-08 21:16 UTC (permalink / raw) To: devel, w.sheng; +Cc: Eric Dong, Ray Ni, Wu Jiaxin, Tan Dun On 11/6/23 10:07, Sheng Wei wrote: > Clear CR4.CET bit before restoring MSR IA32_S_CET. > Backup/restore MSR IA32_U_CET in SMI. (1) As far as I understand, these are still two separate fixes. And I think this patch has issues due to trying to fix both issues at the same time. (I could be wrong of course, I'm not familiar with CET, but this is my impression.) More details on this below. (2) Each issue / fix (i.e., the one issue / fix per patch) should be explained in detail, even if you think the issue that each patch fixes is obvious. > > 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 | 53 ++++++++++++--- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 69 ++++++++++++++++---- > 2 files changed, 98 insertions(+), 24 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm > index 19de5f614e..68332e2c3f 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 (3) These assembly language macros should have been introduced in an include file (*.inc). These "SmiEntry.nasm" files already %include "StuffRsbNasm.inc" and "Nasm.inc", so placing the CET-related macros side-by-side with those files, for example in a new file called "Cet.inc", would be the right thing. It would eliminate the duplication between the IA32 and X64 nasm files. Please prepend a patch to the series that moves the existent macros to "Cet.nasm", and then in this patch, add the new macros to "Cet.nasm" / modify the old ones inside "Cet.nasm". > @@ -214,11 +215,21 @@ ASM_PFX(mPatchCetSupported): > push edx > push eax > > + mov ecx, MSR_IA32_U_CET > + rdmsr > + push edx > + push eax > + So this is related to saving CET_U state; we're pushing the MSR contents to the stack right after having saving CET_S state similarly. > 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 This seems to clear CET_U state. Why is that necessary? The commit message only says "backup/restore"; it does not say "clear". I assume your main goal is *clearing* CET_U state for the duration of the SMI, and that's why you want to backup/restore (so that the clearing does not destroy information). I guess. (4) But this should be explained in the commit message. > @@ -276,6 +287,11 @@ CetDone: > cmp al, 0 > jz CetDone2 > > + mov ecx, MSR_IA32_S_CET > + xor eax, eax > + xor edx, edx > + wrmsr > + > mov eax, 0x668 > mov cr4, eax ; disable CET > (5) This logic then appears to belong to the *other* fix, namely nulling (?) CET_S state before clearing CR4.CET. > @@ -284,10 +300,15 @@ CetDone: > 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) I admit again that I know effectively nothing about CET, but this looks like a bug. The patch tries to pop CET_U (for the purpose of restoring it) before it (already) pops CET_S (for the purpose of restoring it). The ordering seems fine; it mirrors the pushing. (6) However, the last instruction before the CetDone2 label should be "wrmsr"; should it not? I have no idea why we move EAX to EBX instead; it makes no sense to me. > @@ -305,6 +326,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 This makes my head spin! First, it explains why we store EAX to EBX under (6). The reason is that we want to delay the CET_S restoration right until the RSM. And because the MSR_IA32_MISC_ENABLE restoration clobbers EAX, we stash it in EBX. (7) But *why* do we have to delay CET_S restoration right until the RSM? It is not explained anywhere at all in the patch. (8) The stashing of EAX in EBX needs a conspicuous comment in the code, so that future code additions don't clobber EBX. (9) Why don't we stash EDX similary? Why is it safe to set EDX to 0 before the WRMSR? If we don't care about the EDX that we just popped from the stack, then why do we push it originally? (10) Given that we skip the CET_S restoration if "mCetSupported" is zero, why do we *not* skip the CET_U restoration similarly? Sorry, I'm totally confused by this patch. It feels like the patch is trying to do at least three separate things. I don't think I can sensibly review the X64 counterpart until this patch is further split into *minimal* actions. (In fact the "clear CR4.CET" in the subject line and in the commit message seems to apply only to the X64 code!) Thanks Laszlo > +CetDone3: > + > rsm > > ASM_PFX(gcSmiHandlerSize): DW $ - _SmiEntryPoint > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > index d302ca8d01..007fbff640 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 > @@ -316,13 +327,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 +352,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 +379,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 +427,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 > + > ret -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110926): https://edk2.groups.io/g/devel/message/110926 Mute This Topic: https://groups.io/mt/102416572/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR4.CET before restoring MSR IA32_S_CET 2023-11-08 21:16 ` Laszlo Ersek @ 2023-11-09 7:50 ` Sheng Wei 0 siblings, 0 replies; 8+ messages in thread From: Sheng Wei @ 2023-11-09 7:50 UTC (permalink / raw) To: Laszlo Ersek, devel@edk2.groups.io Cc: Dong, Eric, Ni, Ray, Wu, Jiaxin, Tan, Dun Hi Laszlo, Please ignore the patch V3. I will refine the patches and raise patch V4. Thank you. BR Sheng Wei > -----Original Message----- > From: Laszlo Ersek <lersek@redhat.com> > Sent: Thursday, November 9, 2023 5:16 AM > To: devel@edk2.groups.io; Sheng, W <w.sheng@intel.com> > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Wu, Jiaxin > <jiaxin.wu@intel.com>; Tan, Dun <dun.tan@intel.com> > Subject: Re: [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: > Clear CR4.CET before restoring MSR IA32_S_CET > > On 11/6/23 10:07, Sheng Wei wrote: > > Clear CR4.CET bit before restoring MSR IA32_S_CET. > > Backup/restore MSR IA32_U_CET in SMI. > > (1) As far as I understand, these are still two separate fixes. And I think this > patch has issues due to trying to fix both issues at the same time. (I could be > wrong of course, I'm not familiar with CET, but this is my impression.) More > details on this below. > > (2) Each issue / fix (i.e., the one issue / fix per patch) should be explained in > detail, even if you think the issue that each patch fixes is obvious. > > > > > 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 | 53 > ++++++++++++--- > > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 69 > ++++++++++++++++---- > > 2 files changed, 98 insertions(+), 24 deletions(-) > > > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm > > b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm > > index 19de5f614e..68332e2c3f 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 > > (3) These assembly language macros should have been introduced in an > include file (*.inc). > > These "SmiEntry.nasm" files already %include "StuffRsbNasm.inc" and > "Nasm.inc", so placing the CET-related macros side-by-side with those files, for > example in a new file called "Cet.inc", would be the right thing. It would > eliminate the duplication between the IA32 and X64 nasm files. > > Please prepend a patch to the series that moves the existent macros to > "Cet.nasm", and then in this patch, add the new macros to "Cet.nasm" / > modify the old ones inside "Cet.nasm". > > > > @@ -214,11 +215,21 @@ ASM_PFX(mPatchCetSupported): > > push edx > > push eax > > > > + mov ecx, MSR_IA32_U_CET > > + rdmsr > > + push edx > > + push eax > > + > > So this is related to saving CET_U state; we're pushing the MSR contents to the > stack right after having saving CET_S state similarly. > > > 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 > > This seems to clear CET_U state. Why is that necessary? > > The commit message only says "backup/restore"; it does not say "clear". > > I assume your main goal is *clearing* CET_U state for the duration of the SMI, > and that's why you want to backup/restore (so that the clearing does not > destroy information). I guess. > > (4) But this should be explained in the commit message. > > > @@ -276,6 +287,11 @@ CetDone: > > cmp al, 0 > > jz CetDone2 > > > > + mov ecx, MSR_IA32_S_CET > > + xor eax, eax > > + xor edx, edx > > + wrmsr > > + > > mov eax, 0x668 > > mov cr4, eax ; disable CET > > > > (5) This logic then appears to belong to the *other* fix, namely nulling > (?) CET_S state before clearing CR4.CET. > > > @@ -284,10 +300,15 @@ CetDone: > > 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) > > I admit again that I know effectively nothing about CET, but this looks like a > bug. > > The patch tries to pop CET_U (for the purpose of restoring it) before it > (already) pops CET_S (for the purpose of restoring it). > > The ordering seems fine; it mirrors the pushing. > > (6) However, the last instruction before the CetDone2 label should be > "wrmsr"; should it not? > > I have no idea why we move EAX to EBX instead; it makes no sense to me. > > > @@ -305,6 +326,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 > > This makes my head spin! > > First, it explains why we store EAX to EBX under (6). The reason is that we > want to delay the CET_S restoration right until the RSM. And because the > MSR_IA32_MISC_ENABLE restoration clobbers EAX, we stash it in EBX. > > (7) But *why* do we have to delay CET_S restoration right until the RSM? > It is not explained anywhere at all in the patch. > > (8) The stashing of EAX in EBX needs a conspicuous comment in the code, so > that future code additions don't clobber EBX. > > (9) Why don't we stash EDX similary? Why is it safe to set EDX to 0 before the > WRMSR? > > If we don't care about the EDX that we just popped from the stack, then why > do we push it originally? > > (10) Given that we skip the CET_S restoration if "mCetSupported" is zero, why > do we *not* skip the CET_U restoration similarly? > > Sorry, I'm totally confused by this patch. It feels like the patch is trying to do at > least three separate things. > > I don't think I can sensibly review the X64 counterpart until this patch is > further split into *minimal* actions. (In fact the "clear CR4.CET" in the subject > line and in the commit message seems to apply only to the > X64 code!) > > Thanks > Laszlo > > > +CetDone3: > > + > > rsm > > > > ASM_PFX(gcSmiHandlerSize): DW $ - _SmiEntryPoint diff --git > > a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > > index d302ca8d01..007fbff640 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 > > @@ -316,13 +327,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 +352,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 +379,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 +427,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 > > + > > ret -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110951): https://edk2.groups.io/g/devel/message/110951 Mute This Topic: https://groups.io/mt/102416572/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 8+ messages in thread
* [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/PiSmmCpuDxeSmm: Change CR4.CET bit only 2023-11-06 9:07 [edk2-devel] [PATCH v2 0/3] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR4.CET before Sheng Wei 2023-11-06 9:07 ` [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR4.CET before restoring MSR IA32_S_CET Sheng Wei @ 2023-11-06 9:07 ` Sheng Wei 2023-11-08 21:22 ` Laszlo Ersek 2023-11-06 9:07 ` [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Set mSmmInterruptSspTables initial value Sheng Wei 2 siblings, 1 reply; 8+ messages in thread From: Sheng Wei @ 2023-11-06 9:07 UTC (permalink / raw) To: devel; +Cc: Eric Dong, Ray Ni, Laszlo Ersek, Wu Jiaxin, Tan Dun Do not use fixed CR4 value 0x668, change CR4.CET bit only. 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 | 9 ++++++--- UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 3 ++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm index 68332e2c3f..a087576a54 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm @@ -260,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 @@ -292,8 +293,10 @@ CetDone: xor edx, edx wrmsr - mov eax, 0x668 - mov cr4, eax ; disable CET + ; clear CR4.CET bit + mov eax, cr4 + btr eax, CR4_CET_BIT + mov cr4, eax mov ecx, MSR_IA32_PL0_SSP pop eax diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm index 007fbff640..7aed7c8dda 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm @@ -287,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 -- 2.26.2.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110744): https://edk2.groups.io/g/devel/message/110744 Mute This Topic: https://groups.io/mt/102416574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/PiSmmCpuDxeSmm: Change CR4.CET bit only 2023-11-06 9:07 ` [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/PiSmmCpuDxeSmm: Change CR4.CET bit only Sheng Wei @ 2023-11-08 21:22 ` Laszlo Ersek 0 siblings, 0 replies; 8+ messages in thread From: Laszlo Ersek @ 2023-11-08 21:22 UTC (permalink / raw) To: devel, w.sheng; +Cc: Eric Dong, Ray Ni, Wu Jiaxin, Tan Dun On 11/6/23 10:07, Sheng Wei wrote: > Do not use fixed CR4 value 0x668, change CR4.CET bit only. > > 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 | 9 ++++++--- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 3 ++- > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm > index 68332e2c3f..a087576a54 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm > @@ -260,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 > @@ -292,8 +293,10 @@ CetDone: > xor edx, edx > wrmsr > > - mov eax, 0x668 > - mov cr4, eax ; disable CET > + ; clear CR4.CET bit > + mov eax, cr4 > + btr eax, CR4_CET_BIT > + mov cr4, eax > > mov ecx, MSR_IA32_PL0_SSP > pop eax > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > index 007fbff640..7aed7c8dda 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm > @@ -287,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 I didn't understand why the X64 code here didn't contain the "btr" counterpart of "bts". Well the reason is that the "missing" btr is actually introduced in the previous patch. I find that confusing. I think that, once you have "Cet.inc", you should separately replace CR4_CET with CR4_CET_BIT, both in "Cet.inc" and in the three existent locations (two in the IA32 entry code and one in the X64 entry code). *Then* you could proceed to clearing CR4.CET in the subsequent patch, using CR4_CET_BIT. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110927): https://edk2.groups.io/g/devel/message/110927 Mute This Topic: https://groups.io/mt/102416574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 8+ messages in thread
* [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Set mSmmInterruptSspTables initial value 2023-11-06 9:07 [edk2-devel] [PATCH v2 0/3] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR4.CET before Sheng Wei 2023-11-06 9:07 ` [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR4.CET before restoring MSR IA32_S_CET Sheng Wei 2023-11-06 9:07 ` [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/PiSmmCpuDxeSmm: Change CR4.CET bit only Sheng Wei @ 2023-11-06 9:07 ` Sheng Wei 2023-11-08 21:24 ` Laszlo Ersek 2 siblings, 1 reply; 8+ messages in thread From: Sheng Wei @ 2023-11-06 9:07 UTC (permalink / raw) To: devel; +Cc: Eric Dong, Ray Ni, Laszlo Ersek, Wu Jiaxin, Tan Dun Initial the value of 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/X64/SmmFuncsArch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --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 (#110745): https://edk2.groups.io/g/devel/message/110745 Mute This Topic: https://groups.io/mt/102416578/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Set mSmmInterruptSspTables initial value 2023-11-06 9:07 ` [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Set mSmmInterruptSspTables initial value Sheng Wei @ 2023-11-08 21:24 ` Laszlo Ersek 0 siblings, 0 replies; 8+ messages in thread From: Laszlo Ersek @ 2023-11-08 21:24 UTC (permalink / raw) To: devel, w.sheng; +Cc: Eric Dong, Ray Ni, Wu Jiaxin, Tan Dun On 11/6/23 10:07, Sheng Wei wrote: > Initial the value of 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/X64/SmmFuncsArch.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --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. Please state your reason for this change in the commit message. (Functionally, the patch is a no-op.) Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110928): https://edk2.groups.io/g/devel/message/110928 Mute This Topic: https://groups.io/mt/102416578/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-11-09 7:50 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-11-06 9:07 [edk2-devel] [PATCH v2 0/3] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR4.CET before Sheng Wei 2023-11-06 9:07 ` [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR4.CET before restoring MSR IA32_S_CET Sheng Wei 2023-11-08 21:16 ` Laszlo Ersek 2023-11-09 7:50 ` Sheng Wei 2023-11-06 9:07 ` [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/PiSmmCpuDxeSmm: Change CR4.CET bit only Sheng Wei 2023-11-08 21:22 ` Laszlo Ersek 2023-11-06 9:07 ` [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Set mSmmInterruptSspTables initial value Sheng Wei 2023-11-08 21:24 ` Laszlo Ersek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox