Re: [PATCH v2 68/70] x86/setup: Rework MSR_S_CET handling for CET-IBT

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.02.2022 13:51, Andrew Cooper wrote:
> CET-SS and CET-IBT can be independently controlled, so the configuration of
> MSR_S_CET can't be constant 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@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
albeit with a nit and a remark:

> --- a/xen/arch/x86/acpi/wakeup_prot.S
> +++ b/xen/arch/x86/acpi/wakeup_prot.S
> @@ -63,7 +63,26 @@ 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
> +        jz      .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
> +
> +        /* WARNING! call/ret now fatal (iff SHSTK) until SETSSBSY loads SSP 
> */
> +
> +#if defined(CONFIG_XEN_SHSTK)

Just #ifdef, as it was before?

> @@ -90,10 +101,6 @@ 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 latter part of this comment could do with retaining.

Jan