Messages in this thread | | | From | Nadav Amit <> | Subject | Re: [RFC PATCH v2 11/12] x86/mm/tlb: Use async and inline messages for flushing | Date | Fri, 31 May 2019 20:42:43 +0000 |
| |
> On May 31, 2019, at 1:37 PM, Andy Lutomirski <luto@kernel.org> wrote: > > On Fri, May 31, 2019 at 1:13 PM Dave Hansen <dave.hansen@intel.com> wrote: >> On 5/31/19 12:31 PM, Nadav Amit wrote: >>>> On May 31, 2019, at 11:44 AM, Andy Lutomirski <luto@amacapital.net> wrote: >>>> >>>> >>>> >>>>> On May 31, 2019, at 3:57 AM, Peter Zijlstra <peterz@infradead.org> 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. > > Wait, does that mean that you can do a lock cmpxchg or similar to > clear the dirty and writable bits together and, if the dirty bit was > clear, skip the TLB flush? If so, nifty! Modulo errata, of course. > And I seem to remember some exceptions relating to CET shadow stack > involving the dirty bit being set on not-present pages.
I did something similar with the access-bit in the past.
Anyhow, I have a bug here - the code does not wait for the indication that the IPI was received. I need to rerun performance measurements again once I fix it.
| |