Re: [Xen-devel] [PATCH v2 1/7] x86: slightly reduce Meltdown band-aid overhead

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.03.18 at 20:21, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 08/02/18 09:20, Jan Beulich wrote:
>>>>> On 07.02.18 at 20:35, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 07/02/18 16:12, Jan Beulich wrote:
>>>> I'm not sure why I didn't do this right away: By avoiding the use of
>>>> global PTEs in the cloned directmap, there's no need to fiddle with
>>>> 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. 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.
>>> Actually, on second consideration, why does reenabling interrupts need
>>> to be deferred?
>>>
>>> The safety of the sync_guest path (which previously entered Xen, did
>>> nothing, and exited again) relied on the entry part flushing global
>>> mappings for safety, as the return-to-xen path doesn't necessarily
>>> switch mappings.
>>>
>>> However, the first hunk upgrading the "do nothing" to a proper global
>>> flush, covers that case.
>>>
>>> I don't see anything else which affects the safety of taking TLB flush
>>> IPIs early in the entry-from-guest path.  What am I missing?
>> If a sync IPI arrives before we switch away from the restricted page
>> tables, the processor may re-fetch a global entry from those tables
>> through an L4 with the sync IPI is supposed to tell the processor to
>> get rid of (or modify). The subsequent CR3 write won't invalidate such
>> a TLB entry, and hence whatever we do internally may reference a
>> stale mapping.
> 
> In which case, can I propose that the commit message reads:
> 
> The reduced flushing, however, requires that we now have
> interrupts off on all entry paths until after the page table
> switch, so that flush IPIs can't be serviced while on the
> restricted pagetables, leaving a window where a potentially stale
> guest global mapping can be brought into the TLB.  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.
> 
> Or something similar?

I've used the above.

> Also, you've got a bugfix needed in clone_mapping() as found by Juergen,
> asserting that the flags are the same after clobbering PAGE_GLOBAL.
> 
> With both of these suitably addressed, Reviewed-by: Andrew Cooper
> <andrew.cooper3@xxxxxxxxxx>

As you will likely have seen from the patch just sent, the fix is not
to be made here. Please confirm that I can apply the R-b with just
the description change. Of course this change should then go in
only after that other bug fix has (in whatever shape).

Jan


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