Re: [Xen-devel] [PATCH v8 12/17] x86/entry: Organise the use of MSR_SPEC_CTRL at each entry/exit point

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.01.18 at 19:01, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -668,6 +668,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>      set_processor_id(0);
>      set_current(INVALID_VCPU); /* debug sanity. */
>      idle_vcpu[0] = current;
> +    init_shadow_spec_ctrl_state();

Considering the strict need to fill struct cpu_info fields early on (also
in my SP3 band-aid) I wonder whether we wouldn't be better off
uniformly memset()-ing the whole structure first thing here and in
start_secondary(). But this is just a remark, not necessarily
something to be done in this patch.

> @@ -586,6 +611,10 @@ ENTRY(double_fault)
>          movl  $TRAP_double_fault,4(%rsp)
>          /* Set AC to reduce chance of further SMAP faults */
>          SAVE_ALL STAC
> +
> +        SPEC_CTRL_ENTRY_FROM_INTR /* Req: %rsp=regs Clob: acd */
> +        /* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */

Is it actually useful to do _anything_ in the double fault handler?

> +.macro DO_SPEC_CTRL_ENTRY maybexen:req ibrs_val:req
> +/*
> + * Requires %rsp=regs (also cpuinfo if !maybexen)
> + * Clobbers %rax, %rcx, %rdx
> + *
> + * PV guests can't update MSR_SPEC_CTRL behind Xen's back, so no need to read
> + * it back.  Entries from guest context need to clear SPEC_CTRL shadowing,
> + * while entries from Xen must leave shadowing in its current state.
> + */
> +    mov $MSR_SPEC_CTRL, %ecx
> +
> +    .if \maybexen
> +        cmpw $__HYPERVISOR_CS, UREGS_cs(%rsp)

I see you've changed to cmpw here. To eliminate your length-
changing-prefix concern, how about using testb instead to
just evaluate RPL or the selector, as I'm doing in the SP3
band-aid?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel