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
