Re: [PATCH v2 14/14] x86/shstk: Activate Supervisor Shadow Stacks

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.05.2020 21:18, Andrew Cooper wrote:
> With all other plumbing in place, activate shadow stacks when possible.  Note
> that this requires prohibiting the use of PV32.  Compatibility can be
> maintained if necessary via PV-Shim.

In the revision log you say "Discuss CET-SS disabling PV32", and I
agree both here and in the command line doc you mention the "that"
aspect. But what about the "why"? Aiui "is incompatible" or
"requires" are too strong statements: It could be made work (by
disabling / enabling CET on the way out of / back into Xen), but
besides losing some of the intended protection that way, it would
be quite a bit of overhead. So it's more like a design decision,
and it would be nice to express it like this at least in the
commit message.

> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -664,6 +664,13 @@ static void __init noreturn reinit_bsp_stack(void)
>      stack_base[0] = stack;
>      memguard_guard_stack(stack);
>  
> +    if ( cpu_has_xen_shstk )
> +    {
> +        wrmsrl(MSR_PL0_SSP, (unsigned long)stack + 0x5ff8);

Please replace this remaining literal number accordingly.

> @@ -1801,6 +1823,10 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>  
>      alternative_branches();
>  
> +    /* Defer CR4.CET until alternatives have finished playing with CR4.WP */
> +    if ( cpu_has_xen_shstk )
> +        set_in_cr4(X86_CR4_CET);

Nit: The comment still wants changing to CR0.WP.

With these taken care of in some form
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan