Re: [RFC PATCH v2 11/12] x86/mm/tlb: Use async and inline messages for flushing

[Dave Hansen]

On 5/31/19 12:31 PM, Nadav Amit wrote:
>> On May 31, 2019, at 11:44 AM, Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote:
>>
>>
>>
>>> On May 31, 2019, at 3:57 AM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>>>
>>>> On Thu, May 30, 2019 at 11:36:44PM -0700, Nadav Amit wrote:
>>>> When we flush userspace mappings, we can defer the TLB flushes, as long
>>>> the following conditions are met:
>>>>
>>>> 1. No tables are freed, since otherwise speculative page walks might
>>>>  cause machine-checks.
>>>>
>>>> 2. No one would access userspace before flush takes place. Specifically,
>>>>  NMI handlers and kprobes would avoid accessing userspace.
>>>>
>>>> Use the new SMP support to execute remote function calls with inlined
>>>> data for the matter. The function remote TLB flushing function would be
>>>> executed asynchronously and the local CPU would continue execution as
>>>> soon as the IPI was delivered, before the function was actually
>>>> executed. Since tlb_flush_info is copied, there is no risk it would
>>>> change before the TLB flush is actually executed.
>>>>
>>>> Change nmi_uaccess_okay() to check whether a remote TLB flush is
>>>> currently in progress on this CPU by checking whether the asynchronously
>>>> called function is the remote TLB flushing function. The current
>>>> implementation disallows access in such cases, but it is also possible
>>>> to flush the entire TLB in such case and allow access.
>>>
>>> ARGGH, brain hurt. I'm not sure I fully understand this one. How is it
>>> different from today, where the NMI can hit in the middle of the TLB
>>> invalidation?
>>>
>>> Also; since we're not waiting on the IPI, what prevents us from freeing
>>> the user pages before the remote CPU is 'done' with them? Currently the
>>> synchronous IPI is like a sync point where we *know* the remote CPU is
>>> completely done accessing the page.
>>>
>>> Where getting an IPI stops speculation, speculation again restarts
>>> inside the interrupt handler, and until we've passed the INVLPG/MOV CR3,
>>> speculation can happen on that TLB entry, even though we've already
>>> freed and re-used the user-page.
>>>
>>> Also, what happens if the TLB invalidation IPI is stuck behind another
>>> smp_function_call IPI that is doing user-access?
>>>
>>> As said,.. brain hurts.
>>
>> Speculation aside, any code doing dirty tracking needs the flush to happen
>> for real before it reads the dirty bit.
>>
>> How does this patch guarantee that the flush is really done before someone
>> depends on it?
> 
> I was always under the impression that the dirty-bit is pass-through - the
> A/D-assist walks the tables and sets the dirty bit upon access. Otherwise,
> what happens when you invalidate the PTE, and have already marked the PTE as
> non-present? Would the CPU set the dirty-bit at this point?

Modulo bugs^Werrata...  No.  What actually happens is that a
try-to-set-dirty-bit page table walk acts just like a TLB miss.  The old
contents of the TLB are discarded and only the in-memory contents matter
for forward progress.  If Present=0 when the PTE is reached, you'll get
a normal Present=0 page fault.

> In this regard, I remember this thread of Dave Hansen [1], which also seems
> to me as supporting the notion the dirty-bit is set on write and not on
> INVLPG.

... and that's the erratum I was hoping you wouldn't mention. :)

But, yeah, I don't think it's possible to set the Dirty bit on INVLPG.
The bits are set on establishing TLB entries, not on evicting or
flushing them.

I hope that clears it up.