[PATCH] x86, mm: make alternatives code do stronger TLB flush

[PATCH] x86, mm: make alternatives code do stronger TLB flush

[Dave Hansen]

From: Dave Hansen <dave.hansen@linux.intel.com>

local_flush_tlb() does a CR3 write.  But, that kind of TLB flush is
not guaranteed to invalidate global pages.  The entire kernel is
mapped with global pages.

Also, now that we have PCIDs, local_flush_tlb() will only flush the
*current* PCID.  It would not flush the entries for all PCIDs.
At the moment, this is a moot point because all kernel pages are
_PAGE_GLOBAL which do not really *have* a particular PCID.

Use the stronger __flush_tlb_all() which does flush global pages.

This was found because of a warning I added to __native_flush_tlb()
to look for calls to it when PCIDs are enabled.  This patch does
not fix any bug known to be hit in practice.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: x86@kernel.org
Cc: Andy Lutomirski <luto@kernel.org>
---

 b/arch/x86/kernel/alternative.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff -puN arch/x86/kernel/alternative.c~x86-mm-text-poke-misses-global-pages arch/x86/kernel/alternative.c
--- a/arch/x86/kernel/alternative.c~x86-mm-text-poke-misses-global-pages	2017-10-31 10:28:44.306557256 -0700
+++ b/arch/x86/kernel/alternative.c	2017-10-31 10:28:44.309557393 -0700
@@ -722,7 +722,8 @@ void *text_poke(void *addr, const void *
 	clear_fixmap(FIX_TEXT_POKE0);
 	if (pages[1])
 		clear_fixmap(FIX_TEXT_POKE1);
-	local_flush_tlb();
+	/* Make sure to flush Global pages: */
+	__flush_tlb_all();
 	sync_core();
 	/* Could also do a CLFLUSH here to speed up CPU recovery; but
 	   that causes hangs on some VIA CPUs. */
_

Re: [PATCH] x86, mm: make alternatives code do stronger TLB flush

[Andy Lutomirski]

On Tue, Oct 31, 2017 at 11:07 AM, Dave Hansen
<dave.hansen@linux.intel.com> wrote:
>
> From: Dave Hansen <dave.hansen@linux.intel.com>
>
> local_flush_tlb() does a CR3 write.  But, that kind of TLB flush is
> not guaranteed to invalidate global pages.  The entire kernel is
> mapped with global pages.
>
> Also, now that we have PCIDs, local_flush_tlb() will only flush the
> *current* PCID.  It would not flush the entries for all PCIDs.
> At the moment, this is a moot point because all kernel pages are
> _PAGE_GLOBAL which do not really *have* a particular PCID.
>
> Use the stronger __flush_tlb_all() which does flush global pages.
>
> This was found because of a warning I added to __native_flush_tlb()
> to look for calls to it when PCIDs are enabled.  This patch does
> not fix any bug known to be hit in practice.

I'm very confused here.  set_fixmap() does a flush.  clear_fixmap()
calls set_fixmap() and therefore also flushes.  So I don't see why the
flush you're modifying is needed at all.  Could you just delete it
instead?

If your KAISER series were applied, then the situation is slightly
different.  We have this code:

static void __set_pte_vaddr(pud_t *pud, unsigned long vaddr, pte_t new_pte)
{
        pmd_t *pmd = fill_pmd(pud, vaddr);
        pte_t *pte = fill_pte(pmd, vaddr);

        set_pte(pte, new_pte);

        /*
         * It's enough to flush this one mapping.
         * (PGE mappings get flushed as well)
         */
        __flush_tlb_one(vaddr);
}

and that is no longer correct.  You may need to add a helper
__flush_tlb_kernel_one() that does the right thing.  For the
alternatives case, you could skip it since you know that the mapping
never got propagated to any other PCID slot on the current CPU, but
that's probably not worth trying to optimize.

--Andy