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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 02/03/18 11:34, Jan Beulich wrote:
>>>> 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).

I haven't caught up with mail yet. If this isn't the right fix, then my
R-b stands so long as is a fix somewhere.

~Andrew

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