| Message ID | 20211126123446.32324-64-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: Support for CET Indirect Branch Tracking | expand |
On 26.11.2021 13:34, Andrew Cooper wrote: > --- a/xen/arch/x86/acpi/wakeup_prot.S > +++ b/xen/arch/x86/acpi/wakeup_prot.S > @@ -63,7 +63,24 @@ ENTRY(s3_resume) > pushq %rax > lretq > 1: > -#ifdef CONFIG_XEN_SHSTK > +#if defined(CONFIG_XEN_SHSTK) || defined(CONFIG_XEN_IBT) > + call xen_msr_s_cet_value > + test %eax, %eax > + je .L_cet_done Nit: I consider it generally misleading to use JE / JNE (and a few other Jcc) with other than CMP-like insns. Only those handle actual "relations", whereas e.g. TEST only produces particular flag states, so would more consistently be followed by JZ / JNZ in cases like this one. But since this is very much a matter of taste, I'm not going to insist on a change here. > + /* Set up MSR_S_CET. */ > + mov $MSR_S_CET, %ecx > + xor %edx, %edx > + wrmsr > + > + /* Enable CR4.CET. */ > + mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ecx > + mov %rcx, %cr4 Is it valid / safe to enable CR4.CET (with CET_SHSTK_EN already active) before ... > +#if defined(CONFIG_XEN_SHSTK) > + test $CET_SHSTK_EN, %eax (Intermediate remark: Using %al would seem to suffice and be a shorter encoding.) > + je .L_cet_done > + > /* > * Restoring SSP is a little complicated, because we are intercepting > * an in-use shadow stack. Write a temporary token under the stack, > @@ -71,14 +88,6 @@ ENTRY(s3_resume) > * reset MSR_PL0_SSP to its usual value and pop the temporary token. > */ > mov saved_ssp(%rip), %rdi > - cmpq $1, %rdi > - je .L_shstk_done > - > - /* Set up MSR_S_CET. */ > - mov $MSR_S_CET, %ecx > - xor %edx, %edx > - mov $CET_SHSTK_EN | CET_WRSS_EN, %eax > - wrmsr > > /* Construct the temporary supervisor token under SSP. */ > sub $8, %rdi > @@ -90,12 +99,9 @@ ENTRY(s3_resume) > mov %edi, %eax > wrmsr > > - /* Enable CET. MSR_INTERRUPT_SSP_TABLE is set up later in load_system_tables(). */ > - mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ebx > - mov %rbx, %cr4 ... the writing of MSR_PL0_SSP in context here? ISTR some ordering issues back at the time when you introduced CET-SS, so I thought I'd better ask to be sure. Jan
On 06/12/2021 10:49, Jan Beulich wrote: > On 26.11.2021 13:34, Andrew Cooper wrote: >> --- a/xen/arch/x86/acpi/wakeup_prot.S >> +++ b/xen/arch/x86/acpi/wakeup_prot.S >> @@ -63,7 +63,24 @@ ENTRY(s3_resume) >> pushq %rax >> lretq >> 1: >> -#ifdef CONFIG_XEN_SHSTK >> +#if defined(CONFIG_XEN_SHSTK) || defined(CONFIG_XEN_IBT) >> + call xen_msr_s_cet_value >> + test %eax, %eax >> + je .L_cet_done > Nit: I consider it generally misleading to use JE / JNE (and a few > other Jcc) with other than CMP-like insns. Only those handle actual > "relations", whereas e.g. TEST only produces particular flag states, > so would more consistently be followed by JZ / JNZ in cases like > this one. But since this is very much a matter of taste, I'm not > going to insist on a change here. Fixed. > >> + /* Set up MSR_S_CET. */ >> + mov $MSR_S_CET, %ecx >> + xor %edx, %edx >> + wrmsr >> + >> + /* Enable CR4.CET. */ >> + mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ecx >> + mov %rcx, %cr4 > Is it valid / safe to enable CR4.CET (with CET_SHSTK_EN already > active) before ... > >> +#if defined(CONFIG_XEN_SHSTK) >> + test $CET_SHSTK_EN, %eax > (Intermediate remark: Using %al would seem to suffice and be a > shorter encoding.) Fixed. > >> + je .L_cet_done >> + >> /* >> * Restoring SSP is a little complicated, because we are intercepting >> * an in-use shadow stack. Write a temporary token under the stack, >> @@ -71,14 +88,6 @@ ENTRY(s3_resume) >> * reset MSR_PL0_SSP to its usual value and pop the temporary token. >> */ >> mov saved_ssp(%rip), %rdi >> - cmpq $1, %rdi >> - je .L_shstk_done >> - >> - /* Set up MSR_S_CET. */ >> - mov $MSR_S_CET, %ecx >> - xor %edx, %edx >> - mov $CET_SHSTK_EN | CET_WRSS_EN, %eax >> - wrmsr >> >> /* Construct the temporary supervisor token under SSP. */ >> sub $8, %rdi >> @@ -90,12 +99,9 @@ ENTRY(s3_resume) >> mov %edi, %eax >> wrmsr >> >> - /* Enable CET. MSR_INTERRUPT_SSP_TABLE is set up later in load_system_tables(). */ >> - mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ebx >> - mov %rbx, %cr4 > ... the writing of MSR_PL0_SSP in context here? ISTR some ordering > issues back at the time when you introduced CET-SS, so I thought I'd > better ask to be sure. Yes, it is safe, but the reasons why aren't entirely trivial. To set up CET-SS, we need to do the following things: 1) CR4.CET=1 2) Configure MSR_S_CET.SHSTK_EN 3) Configure MSR_PL0_SSP pointing at a non-busy supervisor token 4) Configure MSR_ISST_SSP to point at the IST shadow stacks, again with non-busy tokens 5) execute SETSSBSY to load SSP The MSRs can be configured whenever, subject to suitable hardware support. In both of these cases, we've actually pre-configured the non-busy supervisor tokens which is why we don't set those up directly. Furthermore, we defer setting up MSR_ISST_SSP to when we set up the IDT and TSS, and that's fine because it doesn't make interrupts/exceptions any less fatal. The only hard ordering is that SETSSBSY depends on CR4.CET && MSR_S_CET.SHSTK_EN in order to not #UD. However, between CR4.CET && MSR_S_CET.SHSTK_EN and SETSSBSY, we're operating with an SSP of 0, meaning that any call/ret/etc are fatal. That is why I previously grouped the 3 actions as close to together as possible. For the CONFIG_XEN_IBT && !CONFIG_XEN_SHSTK case, we need to set up CR4 and MSR_S_CET only. This was the only way I could find to lay out the logic in a half-reasonable way. It does mean that MSR_PL0_SSP is set up during the critical call/ret region, but that's the smallest price I could find to pay. Anything else would have had more conditionals, and substantially more #ifdef-ary. I have put in this: diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S index 9178b2e6a039..6a4834f9813a 100644 --- a/xen/arch/x86/boot/x86_64.S +++ b/xen/arch/x86/boot/x86_64.S @@ -45,6 +45,8 @@ ENTRY(__high_start) mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ecx mov %rcx, %cr4 + /* WARNING! call/ret/etc now fatal (iff SHSTK) until SETSSBSY loads SSP */ + #if defined(CONFIG_XEN_SHSTK) test $CET_SHSTK_EN, %al jz .L_ap_cet_done which mirrors our Spectre-v2 warning in the entry paths. ~Andrew
On 10.12.2021 17:19, Andrew Cooper wrote: > On 06/12/2021 10:49, Jan Beulich wrote: >> On 26.11.2021 13:34, Andrew Cooper wrote: >>> --- a/xen/arch/x86/acpi/wakeup_prot.S >>> +++ b/xen/arch/x86/acpi/wakeup_prot.S >>> @@ -63,7 +63,24 @@ ENTRY(s3_resume) >>> pushq %rax >>> lretq >>> 1: >>> -#ifdef CONFIG_XEN_SHSTK >>> +#if defined(CONFIG_XEN_SHSTK) || defined(CONFIG_XEN_IBT) >>> + call xen_msr_s_cet_value >>> + test %eax, %eax >>> + je .L_cet_done >> Nit: I consider it generally misleading to use JE / JNE (and a few >> other Jcc) with other than CMP-like insns. Only those handle actual >> "relations", whereas e.g. TEST only produces particular flag states, >> so would more consistently be followed by JZ / JNZ in cases like >> this one. But since this is very much a matter of taste, I'm not >> going to insist on a change here. > > Fixed. > >> >>> + /* Set up MSR_S_CET. */ >>> + mov $MSR_S_CET, %ecx >>> + xor %edx, %edx >>> + wrmsr >>> + >>> + /* Enable CR4.CET. */ >>> + mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ecx >>> + mov %rcx, %cr4 >> Is it valid / safe to enable CR4.CET (with CET_SHSTK_EN already >> active) before ... >> >>> +#if defined(CONFIG_XEN_SHSTK) >>> + test $CET_SHSTK_EN, %eax >> (Intermediate remark: Using %al would seem to suffice and be a >> shorter encoding.) > > Fixed. > >> >>> + je .L_cet_done >>> + >>> /* >>> * Restoring SSP is a little complicated, because we are intercepting >>> * an in-use shadow stack. Write a temporary token under the stack, >>> @@ -71,14 +88,6 @@ ENTRY(s3_resume) >>> * reset MSR_PL0_SSP to its usual value and pop the temporary token. >>> */ >>> mov saved_ssp(%rip), %rdi >>> - cmpq $1, %rdi >>> - je .L_shstk_done >>> - >>> - /* Set up MSR_S_CET. */ >>> - mov $MSR_S_CET, %ecx >>> - xor %edx, %edx >>> - mov $CET_SHSTK_EN | CET_WRSS_EN, %eax >>> - wrmsr >>> >>> /* Construct the temporary supervisor token under SSP. */ >>> sub $8, %rdi >>> @@ -90,12 +99,9 @@ ENTRY(s3_resume) >>> mov %edi, %eax >>> wrmsr >>> >>> - /* Enable CET. MSR_INTERRUPT_SSP_TABLE is set up later in load_system_tables(). */ >>> - mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ebx >>> - mov %rbx, %cr4 >> ... the writing of MSR_PL0_SSP in context here? ISTR some ordering >> issues back at the time when you introduced CET-SS, so I thought I'd >> better ask to be sure. > > Yes, it is safe, but the reasons why aren't entirely trivial. > > To set up CET-SS, we need to do the following things: > > 1) CR4.CET=1 > 2) Configure MSR_S_CET.SHSTK_EN > 3) Configure MSR_PL0_SSP pointing at a non-busy supervisor token > 4) Configure MSR_ISST_SSP to point at the IST shadow stacks, again with > non-busy tokens > 5) execute SETSSBSY to load SSP > > The MSRs can be configured whenever, subject to suitable hardware > support. In both of these cases, we've actually pre-configured the > non-busy supervisor tokens which is why we don't set those up directly. > > Furthermore, we defer setting up MSR_ISST_SSP to when we set up the IDT > and TSS, and that's fine because it doesn't make interrupts/exceptions > any less fatal. > > The only hard ordering is that SETSSBSY depends on CR4.CET && > MSR_S_CET.SHSTK_EN in order to not #UD. > > However, between CR4.CET && MSR_S_CET.SHSTK_EN and SETSSBSY, we're > operating with an SSP of 0, meaning that any call/ret/etc are fatal. > That is why I previously grouped the 3 actions as close to together as > possible. > > For the CONFIG_XEN_IBT && !CONFIG_XEN_SHSTK case, we need to set up CR4 > and MSR_S_CET only. This was the only way I could find to lay out the > logic in a half-reasonable way. It does mean that MSR_PL0_SSP is set up > during the critical call/ret region, but that's the smallest price I > could find to pay. Anything else would have had more conditionals, and > substantially more #ifdef-ary. > > > I have put in this: > > diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S > index 9178b2e6a039..6a4834f9813a 100644 > --- a/xen/arch/x86/boot/x86_64.S > +++ b/xen/arch/x86/boot/x86_64.S > @@ -45,6 +45,8 @@ ENTRY(__high_start) > mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ecx > mov %rcx, %cr4 > > + /* WARNING! call/ret/etc now fatal (iff SHSTK) until SETSSBSY > loads SSP */ > + > #if defined(CONFIG_XEN_SHSTK) > test $CET_SHSTK_EN, %al > jz .L_ap_cet_done > > > which mirrors our Spectre-v2 warning in the entry paths. Thanks, I think this may end up helpful down the road. Jan
diff --git a/xen/arch/x86/acpi/wakeup_prot.S b/xen/arch/x86/acpi/wakeup_prot.S index 15052c300fa1..01eb26ed0769 100644 --- a/xen/arch/x86/acpi/wakeup_prot.S +++ b/xen/arch/x86/acpi/wakeup_prot.S @@ -63,7 +63,24 @@ ENTRY(s3_resume) pushq %rax lretq 1: -#ifdef CONFIG_XEN_SHSTK +#if defined(CONFIG_XEN_SHSTK) || defined(CONFIG_XEN_IBT) + call xen_msr_s_cet_value + test %eax, %eax + je .L_cet_done + + /* Set up MSR_S_CET. */ + mov $MSR_S_CET, %ecx + xor %edx, %edx + wrmsr + + /* Enable CR4.CET. */ + mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ecx + mov %rcx, %cr4 + +#if defined(CONFIG_XEN_SHSTK) + test $CET_SHSTK_EN, %eax + je .L_cet_done + /* * Restoring SSP is a little complicated, because we are intercepting * an in-use shadow stack. Write a temporary token under the stack, @@ -71,14 +88,6 @@ ENTRY(s3_resume) * reset MSR_PL0_SSP to its usual value and pop the temporary token. */ mov saved_ssp(%rip), %rdi - cmpq $1, %rdi - je .L_shstk_done - - /* Set up MSR_S_CET. */ - mov $MSR_S_CET, %ecx - xor %edx, %edx - mov $CET_SHSTK_EN | CET_WRSS_EN, %eax - wrmsr /* Construct the temporary supervisor token under SSP. */ sub $8, %rdi @@ -90,12 +99,9 @@ ENTRY(s3_resume) mov %edi, %eax wrmsr - /* Enable CET. MSR_INTERRUPT_SSP_TABLE is set up later in load_system_tables(). */ - mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ebx - mov %rbx, %cr4 - /* Write the temporary token onto the shadow stack, and activate it. */ wrssq %rdi, (%rdi) + /* MSR_INTERRUPT_SSP_TABLE is set up later in load_system_tables(). */ setssbsy /* Reset MSR_PL0_SSP back to its normal value. */ @@ -106,8 +112,9 @@ ENTRY(s3_resume) /* Pop the temporary token off the stack. */ mov $2, %eax incsspd %eax -.L_shstk_done: -#endif +#endif /* CONFIG_XEN_SHSTK */ +.L_cet_done: +#endif /* CONFIG_XEN_SHSTK || CONFIG_XEN_IBT */ call load_system_tables diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S index d61048c583b3..c05c69f9fa59 100644 --- a/xen/arch/x86/boot/x86_64.S +++ b/xen/arch/x86/boot/x86_64.S @@ -30,18 +30,25 @@ ENTRY(__high_start) test %ebx,%ebx jz .L_bsp - /* APs. Set up shadow stacks before entering C. */ -#ifdef CONFIG_XEN_SHSTK - testl $cpufeat_mask(X86_FEATURE_XEN_SHSTK), \ - CPUINFO_FEATURE_OFFSET(X86_FEATURE_XEN_SHSTK) + boot_cpu_data(%rip) - je .L_ap_shstk_done + /* APs. Set up CET before entering C properly. */ +#if defined(CONFIG_XEN_SHSTK) || defined(CONFIG_XEN_IBT) + call xen_msr_s_cet_value + test %eax, %eax + je .L_ap_cet_done /* Set up MSR_S_CET. */ mov $MSR_S_CET, %ecx xor %edx, %edx - mov $CET_SHSTK_EN | CET_WRSS_EN, %eax wrmsr + /* Enable CR4.CET. */ + mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ecx + mov %rcx, %cr4 + +#if defined(CONFIG_XEN_SHSTK) + test $CET_SHSTK_EN, %eax + je .L_ap_cet_done + /* Derive MSR_PL0_SSP from %rsp (token written when stack is allocated). */ mov $MSR_PL0_SSP, %ecx mov %rsp, %rdx @@ -51,13 +58,13 @@ ENTRY(__high_start) or $(PRIMARY_SHSTK_SLOT + 1) * PAGE_SIZE - 8, %eax wrmsr - /* Enable CET. MSR_INTERRUPT_SSP_TABLE is set up later in load_system_tables(). */ - mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ecx - mov %rcx, %cr4 + /* MSR_INTERRUPT_SSP_TABLE is set up later in load_system_tables(). */ setssbsy -#endif -.L_ap_shstk_done: +#endif /* CONFIG_XEN_SHSTK */ +.L_ap_cet_done: +#endif /* CONFIG_XEN_SHSTK || CONFIG_XEN_IBT */ + call start_secondary BUG /* start_secondary() shouldn't return. */ diff --git a/xen/arch/x86/crash.c b/xen/arch/x86/crash.c index c383f718f5bd..003222c0f1ac 100644 --- a/xen/arch/x86/crash.c +++ b/xen/arch/x86/crash.c @@ -190,8 +190,8 @@ void machine_crash_shutdown(void) /* Reset CPUID masking and faulting to the host's default. */ ctxt_switch_levelling(NULL); - /* Disable shadow stacks. */ - if ( cpu_has_xen_shstk ) + /* Disable CET. */ + if ( cpu_has_xen_shstk || cpu_has_xen_ibt ) { wrmsrl(MSR_S_CET, 0); write_cr4(read_cr4() & ~X86_CR4_CET); diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index 141957c9f6a5..daaba097d57f 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -661,6 +661,21 @@ static void noreturn init_done(void) startup_cpu_idle_loop(); } +#if defined(CONFIG_XEN_SHSTK) || defined(CONFIG_XEN_IBT) +/* + * Used by AP and S3 asm code to calcualte the appropriate MSR_S_CET setting. + * Do not use on the BSP before reinit_bsp_stack(), or it may turn SHSTK on + * too early. + */ +unsigned int xen_msr_s_cet_value(void) +{ + return ((cpu_has_xen_shstk ? CET_SHSTK_EN | CET_WRSS_EN : 0) | + (cpu_has_xen_ibt ? CET_ENDBR_EN : 0)); +} +#else +unsigned int xen_msr_s_cet_value(void); /* To avoid ifdefary */ +#endif + /* Reinitalise all state referring to the old virtual address of the stack. */ static void __init noreturn reinit_bsp_stack(void) { @@ -684,7 +699,7 @@ static void __init noreturn reinit_bsp_stack(void) { wrmsrl(MSR_PL0_SSP, (unsigned long)stack + (PRIMARY_SHSTK_SLOT + 1) * PAGE_SIZE - 8); - wrmsrl(MSR_S_CET, CET_SHSTK_EN | CET_WRSS_EN); + wrmsrl(MSR_S_CET, xen_msr_s_cet_value()); asm volatile ("setssbsy" ::: "memory"); } diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h index ab68ef2681a9..627508233d19 100644 --- a/xen/include/asm-x86/msr-index.h +++ b/xen/include/asm-x86/msr-index.h @@ -115,6 +115,7 @@ #define MSR_S_CET 0x000006a2 #define CET_SHSTK_EN (_AC(1, ULL) << 0) #define CET_WRSS_EN (_AC(1, ULL) << 1) +#define CET_ENDBR_EN (_AC(1, ULL) << 2) #define MSR_PL0_SSP 0x000006a4 #define MSR_PL1_SSP 0x000006a5
CET-SS and CET-IBT can be independently controlled, so the configuration of MSR_S_CET can't be constants any more. Introduce xen_msr_s_cet_value(), mostly because I don't fancy writing/maintaining that logic in assembly. Use this in the 3 paths which alter MSR_S_CET when both features are potentially active. To active CET-IBT, we only need CR4.CET and MSR_S_CET.ENDBR_EN. This is common with the CET-SS setup, so reorder the operations to set up CR4 and MSR_S_CET for any nonzero result from xen_msr_s_cet_value(), and set up MSR_PL0_SSP and SSP if SHSTK_EN was also set. Adjust the crash path to disable CET-IBT too. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> It is quite possible that the S3 path is dead code. CET-IBT only exist on Intel systems from TigerLake onwards, and TGL kills S3 in favour of the newer S0ix power state. AMD Ryzen platforms (Zen3 onwards) support S3 and CET-SS, so partial testing will occur there. --- xen/arch/x86/acpi/wakeup_prot.S | 37 ++++++++++++++++++++++--------------- xen/arch/x86/boot/x86_64.S | 29 ++++++++++++++++++----------- xen/arch/x86/crash.c | 4 ++-- xen/arch/x86/setup.c | 17 ++++++++++++++++- xen/include/asm-x86/msr-index.h | 1 + 5 files changed, 59 insertions(+), 29 deletions(-)