Re: [Xen-devel] [PATCH] x86: slightly reduce Meltdown band-aid overhead

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 18/01/18 15:39, Jan Beulich wrote:
> I'm not sure why I didn't do this right away: By avoiding to make any
> of the cloned directmap PTEs global, there's no need to fiddle with

"avoiding to make" is a very odd way of phrasing this.  Something like
"By avoiding the use of global PTEs in the cloned directmap, " perhaps?

> CR4.PGE on any of the entry paths. Only the exit paths need to flush
> global mappings.
>
> The reduced flushing, however, implies that we now need to have
> interrupts off on all entry paths until after the page table switch, so
> that flush IPIs can't arrive with the restricted page tables still
> active, but only a non-global flush happening with the CR3 loads.

I can't parse this last clause in the context of the sentence, which
looks to finish after "active".

> Along those lines the "sync" IPI after L4 entry updates now needs to become a
> real (and global) flush IPI, so that inside Xen we'll also pick up such
> changes.

The entry paths don't tick the TLB clock, so we are in no worse of a
position than before.  IOW, I don't see why this needs to change to
being a FLUSH_GLOBAL.

>
> Take the opportunity and also do a GET_CURRENT() -> __GET_CURRENT()
> transition the original patch missed.

I've just submitted an alternative to this.

>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -726,6 +726,7 @@ static int clone_mapping(const void *ptr
>      }
>  
>      pl1e += l1_table_offset(linear);
> +    flags &= ~_PAGE_GLOBAL;
>  
>      if ( l1e_get_flags(*pl1e) & _PAGE_PRESENT )
>      {
> @@ -1009,8 +1010,17 @@ void __init smp_prepare_cpus(unsigned in
>      if ( rc )
>          panic("Error %d setting up PV root page table\n", rc);
>      if ( per_cpu(root_pgt, 0) )
> +    {
>          get_cpu_info()->pv_cr3 = __pa(per_cpu(root_pgt, 0));
>  
> +        /*
> +         * All entry points which may need to switch page tables have to 
> start
> +         * with interrupts off. Re-write what pv_trap_init() has put there.
> +         */
> +        _set_gate(idt_table + LEGACY_SYSCALL_VECTOR, SYS_DESC_irq_gate, 3,
> +                  &int80_direct_trap);

The int82 path is also a trap gate.  Given how subtle this is, and how
hard a resulting crash would be to debug, I think it would be better to
unilaterally switch both to being interrupt gates.

Neither are fastpaths, so the single extra sti in their execution paths
will be negligible.

~Andrew

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