Re: [PATCH] x86/mm: Rename flush_tlb_single() and flush_tlb_one()
From: Dave Hansen
Date: Wed Jan 31 2018 - 14:41:16 EST
> @@ -437,18 +437,31 @@ static inline void __flush_tlb_all(void)
> /*
> * flush one page in the kernel mapping
> */
> -static inline void __flush_tlb_one(unsigned long addr)
> +static inline void __flush_tlb_one_kernel(unsigned long addr)
> {
> count_vm_tlb_event(NR_TLB_LOCAL_FLUSH_ONE);
> - __flush_tlb_single(addr);
> +
> + /*
> + * If PTI is off, then __flush_tlb_one_user() is just INVLPG or its
> + * paravirt equivalent. Even with PCID, this is sufficient: we only
> + * use PCID if we also use global PTEs for the kernel mapping, and
> + * INVLPG flushes global translations across all address spaces.
This looks good.
> + * If PTI is on, then the kernel is mapped with non-global PTEs, and
> + * __flush_tlb_one_user() will flush the given address for the current
> + * kernel address space and for its usermode counterpart, but it goes
> + * not flush it for other address spaces.
> + */
> + __flush_tlb_one_user(addr);
s/goes/does/
It also goes off and flushes the address out of the user asid. That
_seems_ a bit goofy, but it is needed for addresses that might be mapped
into the user asid, so it's definitely safe. Might be worth calling out.
Maybe add a (if one exists) or something like:
... kernel address space and for its usermode counterpart (if one exists).
> if (!static_cpu_has(X86_FEATURE_PTI))
> return;
>
> /*
> - * __flush_tlb_single() will have cleared the TLB entry for this ASID,
> - * but since kernel space is replicated across all, we must also
> - * invalidate all others.
> + * See above. We need to propagate the flush to all other address
> + * spaces. In principle, we only need to propagate it to kernelmode
> + * address spaces, but the extra bookkeeping we would need is not
> + * worth it.
> */
> invalidate_other_asid();
> }
That comment is true, except if we were invalidating a user-mapped
address. Right?
We've just been pretending so far for the purposes of TLB invalidation
that all kernel addresses are potentially user-mapped.
The name change looks really good to me, though. Thanks for doing this.