On 02/15/2018 09:47 AM, Nadav Amit wrote: > Dave Hansen <dave.han...@linux.intel.com> wrote: >>> diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c >>> index c67ef3fb4f35..979c7ec6baab 100644 >>> --- a/arch/x86/mm/tlb.c >>> +++ b/arch/x86/mm/tlb.c >>> @@ -74,7 +74,8 @@ static void choose_new_asid(struct mm_struct *next, u64 >>> next_tlb_gen, >>> return; >>> } >>> >>> - if (this_cpu_read(cpu_tlbstate.invalidate_other)) >>> + if (this_cpu_read(cpu_tlbstate.invalidate_other) && >>> + !mm_pti_disable(next)) >>> clear_asid_other(); >> >> This isn't obviously correct. Don't we still need to invalidate other >> user asids? > > I forgot to regard this question: When you reenable PTI (after switching back > to 64-bit process), you flush the global pages, so no kernel mappings for the > 32-bit process are left.
Can you please write up a proper description for this? It's horribly complicated, intertwined with global pages, and sets up a dependency that *ALL* TLB entries invalidated via __flush_tlb_one_kernel() must be _PAGE_GLOBAL. How about you actually clear cpu_tlbstate.invalidate_other when you do the CR4.PGE switching? That seems a much more direct way and is much more self-documenting. That brings up another point: these patches rather ignore cpu_tlbstate. That leads to confusing code (this) and the double-flushing on context switch I brought up earlier. Was this intentional, or is it something you can reconsider going forward?
