Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1422047 > unrolled thread

[PATCH] Linux VM workaround for Knights Landing A/D leak

Back to article view | Back to linux.kernel

Contents

  [PATCH] Linux VM workaround for Knights Landing A/D leak Lukasz Anaczkowski <lukasz.anaczkowski@intel.com> - 2016-06-14 18:00 +0200
    Re: [PATCH] Linux VM workaround for Knights Landing A/D leak kbuild test robot <lkp@intel.com> - 2016-06-14 18:40 +0200
    Re: [PATCH] Linux VM workaround for Knights Landing A/D leak Nadav Amit <nadav.amit@gmail.com> - 2016-06-14 18:50 +0200
      RE: [PATCH] Linux VM workaround for Knights Landing A/D leak "Anaczkowski, Lukasz" <lukasz.anaczkowski@intel.com> - 2016-06-14 19:00 +0200
      [PATCH v2] Linux VM workaround for Knights Landing A/D leak Lukasz Anaczkowski <lukasz.anaczkowski@intel.com> - 2016-06-14 19:10 +0200
        Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak Dave Hansen <dave.hansen@linux.intel.com> - 2016-06-14 19:30 +0200
          Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-06-14 20:40 +0200
            Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak Dave Hansen <dave.hansen@linux.intel.com> - 2016-06-14 21:00 +0200
              Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak Borislav Petkov <bp@alien8.de> - 2016-06-14 21:20 +0200
                Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak "H. Peter Anvin" <hpa@zytor.com> - 2016-06-14 22:30 +0200
                  Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak Borislav Petkov <bp@alien8.de> - 2016-06-14 22:50 +0200
                    Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak "H. Peter Anvin" <hpa@zytor.com> - 2016-06-14 23:00 +0200
                      Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak "H. Peter Anvin" <hpa@zytor.com> - 2016-06-14 23:10 +0200
                      Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak Borislav Petkov <bp@alien8.de> - 2016-06-14 23:10 +0200
                        Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak "H. Peter Anvin" <hpa@zytor.com> - 2016-06-14 23:20 +0200
        Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak Borislav Petkov <bp@alien8.de> - 2016-06-14 20:20 +0200
          RE: [PATCH v2] Linux VM workaround for Knights Landing A/D leak "Anaczkowski, Lukasz" <lukasz.anaczkowski@intel.com> - 2016-06-15 15:20 +0200
        Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak Nadav Amit <nadav.amit@gmail.com> - 2016-06-14 20:40 +0200
          RE: [PATCH v2] Linux VM workaround for Knights Landing A/D leak "Anaczkowski, Lukasz" <lukasz.anaczkowski@intel.com> - 2016-06-15 15:20 +0200
            Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak Nadav Amit <nadav.amit@gmail.com> - 2016-06-15 22:10 +0200
              Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak Dave Hansen <dave.hansen@linux.intel.com> - 2016-06-15 22:20 +0200
                Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak Nadav Amit <nadav.amit@gmail.com> - 2016-06-15 22:30 +0200
        [PATCH v3] Linux VM workaround for Knights Landing A/D leak Lukasz Anaczkowski <lukasz.anaczkowski@intel.com> - 2016-06-16 17:20 +0200
          Re: [PATCH v3] Linux VM workaround for Knights Landing A/D leak Nadav Amit <nadav.amit@gmail.com> - 2016-06-16 18:50 +0200
          Re: [PATCH v3] Linux VM workaround for Knights Landing A/D leak Dave Hansen <dave.hansen@linux.intel.com> - 2016-06-16 22:30 +0200
      Re: [PATCH] Linux VM workaround for Knights Landing A/D leak Dave Hansen <dave.hansen@linux.intel.com> - 2016-06-14 19:20 +0200
        Re: [PATCH] Linux VM workaround for Knights Landing A/D leak Nadav Amit <nadav.amit@gmail.com> - 2016-06-14 22:20 +0200
          Re: [PATCH] Linux VM workaround for Knights Landing A/D leak Dave Hansen <dave.hansen@linux.intel.com> - 2016-06-14 23:40 +0200
            Re: [PATCH] Linux VM workaround for Knights Landing A/D leak Andy Lutomirski <luto@amacapital.net> - 2016-06-15 04:30 +0200
              Re: [PATCH] Linux VM workaround for Knights Landing A/D leak Nadav Amit <nadav.amit@gmail.com> - 2016-06-15 04:40 +0200
                Re: [PATCH] Linux VM workaround for Knights Landing A/D leak Andy Lutomirski <luto@amacapital.net> - 2016-06-15 04:40 +0200
                  Re: [PATCH] Linux VM workaround for Knights Landing A/D leak Nadav Amit <nadav.amit@gmail.com> - 2016-06-15 04:50 +0200
                    Re: [PATCH] Linux VM workaround for Knights Landing A/D leak Andy Lutomirski <luto@amacapital.net> - 2016-06-15 05:10 +0200
            Re: [PATCH] Linux VM workaround for Knights Landing A/D leak Nadav Amit <nadav.amit@gmail.com> - 2016-06-15 05:30 +0200
    Re: [PATCH] Linux VM workaround for Knights Landing A/D leak kbuild test robot <lkp@intel.com> - 2016-06-14 19:00 +0200
    Re: [PATCH] Linux VM workaround for Knights Landing A/D leak Dave Hansen <dave.hansen@linux.intel.com> - 2016-06-14 19:30 +0200
      RE: [PATCH] Linux VM workaround for Knights Landing A/D leak "Anaczkowski, Lukasz" <lukasz.anaczkowski@intel.com> - 2016-06-15 15:10 +0200

Page 1 of 2  [1] 2  Next page →

#1422047 — [PATCH] Linux VM workaround for Knights Landing A/D leak

From: Andi Kleen <ak@linux.intel.com>

Knights Landing has a issue that a thread setting A or D bits
may not do so atomically against checking the present bit.
A thread which is going to page fault may still set those
bits, even though the present bit was already atomically cleared.

This implies that when the kernel clears present atomically,
some time later the supposed to be zero entry could be corrupted
with stray A or D bits.

Since the PTE could be already used for storing a swap index,
or a NUMA migration index, this cannot be tolerated. Most
of the time the kernel detects the problem, but in some
rare cases it may not.

This patch enforces that the page unmap path in vmscan/direct reclaim
always flushes other CPUs after clearing each page, and also
clears the PTE again after the flush.

For reclaim this brings the performance back to before Mel's
flushing changes, but for unmap it disables batching.

This makes sure any leaked A/D bits are immediately cleared before the entry
is used for something else.

Any parallel faults that check for entry is zero may loop,
but they should eventually recover after the entry is written.

Also other users may spin in the page table lock until we
"fixed" the PTE. This is ensured by always taking the page table lock
even for the swap cache case. Previously this was only done
on architectures with non atomic PTE accesses (such as 32bit PTE),
but now it is also done when this bug workaround is active.

I audited apply_pte_range and other users of arch_enter_lazy...
and they seem to all not clear the present bit.

Right now the extra flush is done in the low level
architecture code, while the higher level code still
does batched TLB flush. This means there is always one extra
unnecessary TLB flush now. As a followon optimization
this could be avoided by telling the callers that
the flush already happenend.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 arch/x86/include/asm/cpufeatures.h |  1 +
 arch/x86/include/asm/hugetlb.h     |  9 ++++++++-
 arch/x86/include/asm/pgtable.h     |  5 +++++
 arch/x86/include/asm/pgtable_64.h  |  6 ++++++
 arch/x86/kernel/cpu/intel.c        | 10 ++++++++++
 arch/x86/mm/tlb.c                  | 20 ++++++++++++++++++++
 include/linux/mm.h                 |  4 ++++
 mm/memory.c                        |  3 ++-
 8 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 4a41348..2c48011 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -303,6 +303,7 @@
 #define X86_BUG_SYSRET_SS_ATTRS	X86_BUG(8) /* SYSRET doesn't fix up SS attrs */
 #define X86_BUG_NULL_SEG	X86_BUG(9) /* Nulling a selector preserves the base */
 #define X86_BUG_SWAPGS_FENCE	X86_BUG(10) /* SWAPGS without input dep on GS */
+#define X86_BUG_PTE_LEAK        X86_BUG(11) /* PTE may leak A/D bits after clear */
 
 
 #ifdef CONFIG_X86_32
diff --git a/arch/x86/include/asm/hugetlb.h b/arch/x86/include/asm/hugetlb.h
index 3a10616..58e1ca9 100644
--- a/arch/x86/include/asm/hugetlb.h
+++ b/arch/x86/include/asm/hugetlb.h
@@ -41,10 +41,17 @@ static inline void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
 	set_pte_at(mm, addr, ptep, pte);
 }
 
+extern void fix_pte_leak(struct mm_struct *mm, unsigned long addr,
+			 pte_t *ptep);
+
 static inline pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
 					    unsigned long addr, pte_t *ptep)
 {
-	return ptep_get_and_clear(mm, addr, ptep);
+	pte_t pte = ptep_get_and_clear(mm, addr, ptep);
+
+	if (boot_cpu_has_bug(X86_BUG_PTE_LEAK))
+		fix_pte_leak(mm, addr, ptep);
+	return pte;
 }
 
 static inline void huge_ptep_clear_flush(struct vm_area_struct *vma,
diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 1a27396..9769355 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -794,11 +794,16 @@ extern int ptep_test_and_clear_young(struct vm_area_struct *vma,
 extern int ptep_clear_flush_young(struct vm_area_struct *vma,
 				  unsigned long address, pte_t *ptep);
 
+extern void fix_pte_leak(struct mm_struct *mm, unsigned long addr,
+			 pte_t *ptep);
+
 #define __HAVE_ARCH_PTEP_GET_AND_CLEAR
 static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned long addr,
 				       pte_t *ptep)
 {
 	pte_t pte = native_ptep_get_and_clear(ptep);
+	if (boot_cpu_has_bug(X86_BUG_PTE_LEAK))
+		fix_pte_leak(mm, addr, ptep);
 	pte_update(mm, addr, ptep);
 	return pte;
 }
diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h
index 2ee7811..6fa4079 100644
--- a/arch/x86/include/asm/pgtable_64.h
+++ b/arch/x86/include/asm/pgtable_64.h
@@ -178,6 +178,12 @@ extern void cleanup_highmap(void);
 extern void init_extra_mapping_uc(unsigned long phys, unsigned long size);
 extern void init_extra_mapping_wb(unsigned long phys, unsigned long size);
 
+#define ARCH_HAS_NEEDS_SWAP_PTL 1
+static inline bool arch_needs_swap_ptl(void)
+{
+       return boot_cpu_has_bug(X86_BUG_PTE_LEAK);
+}
+
 #endif /* !__ASSEMBLY__ */
 
 #endif /* _ASM_X86_PGTABLE_64_H */
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 6e2ffbe..f499513 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -181,6 +181,16 @@ static void early_init_intel(struct cpuinfo_x86 *c)
 		}
 	}
 
+	if (c->x86_model == 87) {
+		static bool printed;
+
+		if (!printed) {
+			pr_info("Enabling PTE leaking workaround\n");
+			printed = true;
+		}
+		set_cpu_bug(c, X86_BUG_PTE_LEAK);
+	}
+
 	/*
 	 * Intel Quark Core DevMan_001.pdf section 6.4.11
 	 * "The operating system also is required to invalidate (i.e., flush)
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 5643fd0..3d54488 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -468,4 +468,24 @@ static int __init create_tlb_single_page_flush_ceiling(void)
 }
 late_initcall(create_tlb_single_page_flush_ceiling);
 
+/*
+ * Workaround for KNL issue:
+ *
+ * A thread that is going to page fault due to P=0, may still
+ * non atomically set A or D bits, which could corrupt swap entries.
+ * Always flush the other CPUs and clear the PTE again to avoid
+ * this leakage. We are excluded using the pagetable lock.
+ */
+
+void fix_pte_leak(struct mm_struct *mm, unsigned long addr, pte_t *ptep)
+{
+	if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids) {
+		trace_tlb_flush(TLB_LOCAL_SHOOTDOWN, TLB_FLUSH_ALL);
+		flush_tlb_others(mm_cpumask(mm), mm, addr,
+				 addr + PAGE_SIZE);
+		mb();
+		set_pte(ptep, __pte(0));
+	}
+}
+
 #endif /* CONFIG_SMP */
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 5df5feb..5c80fe09 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2404,6 +2404,10 @@ static inline bool debug_guardpage_enabled(void) { return false; }
 static inline bool page_is_guard(struct page *page) { return false; }
 #endif /* CONFIG_DEBUG_PAGEALLOC */
 
+#ifndef ARCH_HAS_NEEDS_SWAP_PTL
+static inline bool arch_needs_swap_ptl(void) { return false; }
+#endif
+
 #if MAX_NUMNODES > 1
 void __init setup_nr_node_ids(void);
 #else
diff --git a/mm/memory.c b/mm/memory.c
index 15322b7..0d6ef39 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1960,7 +1960,8 @@ static inline int pte_unmap_same(struct mm_struct *mm, pmd_t *pmd,
 {
 	int same = 1;
 #if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT)
-	if (sizeof(pte_t) > sizeof(unsigned long)) {
+	if (arch_needs_swap_ptl() ||
+	    sizeof(pte_t) > sizeof(unsigned long)) {
 		spinlock_t *ptl = pte_lockptr(mm, pmd);
 		spin_lock(ptl);
 		same = pte_same(*page_table, orig_pte);
-- 
1.8.3.1

[toc] | [next] | [standalone]

#1422076

[Multipart message — attachments visible in raw view] — view raw

Hi,

[auto build test ERROR on v4.7-rc3]
[also build test ERROR on next-20160614]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Lukasz-Anaczkowski/Linux-VM-workaround-for-Knights-Landing-A-D-leak/20160615-000610
config: i386-alldefconfig (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   mm/built-in.o: In function `unmap_page_range':
   (.text+0x1e9e8): undefined reference to `fix_pte_leak'
   mm/built-in.o: In function `change_protection_range':
>> mprotect.c:(.text+0x25578): undefined reference to `fix_pte_leak'
   mm/built-in.o: In function `move_page_tables':
   (.text+0x25d81): undefined reference to `fix_pte_leak'
   mm/built-in.o: In function `vunmap_page_range':
   vmalloc.c:(.text+0x28419): undefined reference to `fix_pte_leak'
   mm/built-in.o: In function `ptep_clear_flush':
   (.text+0x2a7b3): undefined reference to `fix_pte_leak'
   mm/built-in.o:madvise.c:(.text+0x2b4b0): more undefined references to `fix_pte_leak' follow

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1422087

Lukasz Anaczkowski <lukasz.anaczkowski@intel.com> wrote:

> From: Andi Kleen <ak@linux.intel.com>
> +void fix_pte_leak(struct mm_struct *mm, unsigned long addr, pte_t *ptep)
> +{
Here there should be a call to smp_mb__after_atomic() to synchronize with
switch_mm. I submitted a similar patch, which is still pending (hint).

> +	if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids) {
> +		trace_tlb_flush(TLB_LOCAL_SHOOTDOWN, TLB_FLUSH_ALL);
> +		flush_tlb_others(mm_cpumask(mm), mm, addr,
> +				 addr + PAGE_SIZE);
> +		mb();
> +		set_pte(ptep, __pte(0));
> +	}
> +}

Regards,
Nadav

[toc] | [prev] | [next] | [standalone]

#1422099

From: Nadav Amit [mailto:nadav.amit@gmail.com] 
Sent: Tuesday, June 14, 2016 6:48 PM

> Lukasz Anaczkowski <lukasz.anaczkowski@intel.com> wrote:

>> From: Andi Kleen <ak@linux.intel.com>
>> +void fix_pte_leak(struct mm_struct *mm, unsigned long addr, pte_t *ptep)
>> +{
> Here there should be a call to smp_mb__after_atomic() to synchronize with
> switch_mm. I submitted a similar patch, which is still pending (hint).

Thanks, Nadav!
I'll add this and re-submit the patch.

Cheers,
Lukasz

[toc] | [prev] | [next] | [standalone]

#1422111 — [PATCH v2] Linux VM workaround for Knights Landing A/D leak

From: Andi Kleen <ak@linux.intel.com>

Knights Landing has a issue that a thread setting A or D bits
may not do so atomically against checking the present bit.
A thread which is going to page fault may still set those
bits, even though the present bit was already atomically cleared.

This implies that when the kernel clears present atomically,
some time later the supposed to be zero entry could be corrupted
with stray A or D bits.

Since the PTE could be already used for storing a swap index,
or a NUMA migration index, this cannot be tolerated. Most
of the time the kernel detects the problem, but in some
rare cases it may not.

This patch enforces that the page unmap path in vmscan/direct reclaim
always flushes other CPUs after clearing each page, and also
clears the PTE again after the flush.

For reclaim this brings the performance back to before Mel's
flushing changes, but for unmap it disables batching.

This makes sure any leaked A/D bits are immediately cleared before the entry
is used for something else.

Any parallel faults that check for entry is zero may loop,
but they should eventually recover after the entry is written.

Also other users may spin in the page table lock until we
"fixed" the PTE. This is ensured by always taking the page table lock
even for the swap cache case. Previously this was only done
on architectures with non atomic PTE accesses (such as 32bit PTE),
but now it is also done when this bug workaround is active.

I audited apply_pte_range and other users of arch_enter_lazy...
and they seem to all not clear the present bit.

Right now the extra flush is done in the low level
architecture code, while the higher level code still
does batched TLB flush. This means there is always one extra
unnecessary TLB flush now. As a followon optimization
this could be avoided by telling the callers that
the flush already happenend.

v2 (Lukasz Anaczkowski):
    () added call to smp_mb__after_atomic() to synchornize with
       switch_mm, based on Nadav's comment
    () fixed compilation breakage

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Lukasz Anaczkowski <lukasz.anaczkowski@intel.com>
---
 arch/x86/include/asm/cpufeatures.h |  1 +
 arch/x86/include/asm/hugetlb.h     |  9 ++++++++-
 arch/x86/include/asm/pgtable.h     |  5 +++++
 arch/x86/include/asm/pgtable_64.h  |  6 ++++++
 arch/x86/kernel/cpu/intel.c        | 10 ++++++++++
 arch/x86/mm/tlb.c                  | 22 ++++++++++++++++++++++
 include/linux/mm.h                 |  4 ++++
 mm/memory.c                        |  3 ++-
 8 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 4a41348..2c48011 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -303,6 +303,7 @@
 #define X86_BUG_SYSRET_SS_ATTRS	X86_BUG(8) /* SYSRET doesn't fix up SS attrs */
 #define X86_BUG_NULL_SEG	X86_BUG(9) /* Nulling a selector preserves the base */
 #define X86_BUG_SWAPGS_FENCE	X86_BUG(10) /* SWAPGS without input dep on GS */
+#define X86_BUG_PTE_LEAK        X86_BUG(11) /* PTE may leak A/D bits after clear */
 
 
 #ifdef CONFIG_X86_32
diff --git a/arch/x86/include/asm/hugetlb.h b/arch/x86/include/asm/hugetlb.h
index 3a10616..58e1ca9 100644
--- a/arch/x86/include/asm/hugetlb.h
+++ b/arch/x86/include/asm/hugetlb.h
@@ -41,10 +41,17 @@ static inline void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
 	set_pte_at(mm, addr, ptep, pte);
 }
 
+extern void fix_pte_leak(struct mm_struct *mm, unsigned long addr,
+			 pte_t *ptep);
+
 static inline pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
 					    unsigned long addr, pte_t *ptep)
 {
-	return ptep_get_and_clear(mm, addr, ptep);
+	pte_t pte = ptep_get_and_clear(mm, addr, ptep);
+
+	if (boot_cpu_has_bug(X86_BUG_PTE_LEAK))
+		fix_pte_leak(mm, addr, ptep);
+	return pte;
 }
 
 static inline void huge_ptep_clear_flush(struct vm_area_struct *vma,
diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 1a27396..9769355 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -794,11 +794,16 @@ extern int ptep_test_and_clear_young(struct vm_area_struct *vma,
 extern int ptep_clear_flush_young(struct vm_area_struct *vma,
 				  unsigned long address, pte_t *ptep);
 
+extern void fix_pte_leak(struct mm_struct *mm, unsigned long addr,
+			 pte_t *ptep);
+
 #define __HAVE_ARCH_PTEP_GET_AND_CLEAR
 static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned long addr,
 				       pte_t *ptep)
 {
 	pte_t pte = native_ptep_get_and_clear(ptep);
+	if (boot_cpu_has_bug(X86_BUG_PTE_LEAK))
+		fix_pte_leak(mm, addr, ptep);
 	pte_update(mm, addr, ptep);
 	return pte;
 }
diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h
index 2ee7811..6fa4079 100644
--- a/arch/x86/include/asm/pgtable_64.h
+++ b/arch/x86/include/asm/pgtable_64.h
@@ -178,6 +178,12 @@ extern void cleanup_highmap(void);
 extern void init_extra_mapping_uc(unsigned long phys, unsigned long size);
 extern void init_extra_mapping_wb(unsigned long phys, unsigned long size);
 
+#define ARCH_HAS_NEEDS_SWAP_PTL 1
+static inline bool arch_needs_swap_ptl(void)
+{
+       return boot_cpu_has_bug(X86_BUG_PTE_LEAK);
+}
+
 #endif /* !__ASSEMBLY__ */
 
 #endif /* _ASM_X86_PGTABLE_64_H */
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 6e2ffbe..f499513 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -181,6 +181,16 @@ static void early_init_intel(struct cpuinfo_x86 *c)
 		}
 	}
 
+	if (c->x86_model == 87) {
+		static bool printed;
+
+		if (!printed) {
+			pr_info("Enabling PTE leaking workaround\n");
+			printed = true;
+		}
+		set_cpu_bug(c, X86_BUG_PTE_LEAK);
+	}
+
 	/*
 	 * Intel Quark Core DevMan_001.pdf section 6.4.11
 	 * "The operating system also is required to invalidate (i.e., flush)
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 5643fd0..9b4c575 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -469,3 +469,25 @@ static int __init create_tlb_single_page_flush_ceiling(void)
 late_initcall(create_tlb_single_page_flush_ceiling);
 
 #endif /* CONFIG_SMP */
+
+/*
+ * Workaround for KNL issue:
+ *
+ * A thread that is going to page fault due to P=0, may still
+ * non atomically set A or D bits, which could corrupt swap entries.
+ * Always flush the other CPUs and clear the PTE again to avoid
+ * this leakage. We are excluded using the pagetable lock.
+ */
+
+void fix_pte_leak(struct mm_struct *mm, unsigned long addr, pte_t *ptep)
+{
+	smp_mb__after_atomic();
+	if (cpumask_any_but(mm_cpumask(mm), smp_processor_id()) < nr_cpu_ids) {
+		trace_tlb_flush(TLB_LOCAL_SHOOTDOWN, TLB_FLUSH_ALL);
+		flush_tlb_others(mm_cpumask(mm), mm, addr,
+				 addr + PAGE_SIZE);
+		mb();
+		set_pte(ptep, __pte(0));
+	}
+}
+
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 5df5feb..5c80fe09 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2404,6 +2404,10 @@ static inline bool debug_guardpage_enabled(void) { return false; }
 static inline bool page_is_guard(struct page *page) { return false; }
 #endif /* CONFIG_DEBUG_PAGEALLOC */
 
+#ifndef ARCH_HAS_NEEDS_SWAP_PTL
+static inline bool arch_needs_swap_ptl(void) { return false; }
+#endif
+
 #if MAX_NUMNODES > 1
 void __init setup_nr_node_ids(void);
 #else
diff --git a/mm/memory.c b/mm/memory.c
index 15322b7..0d6ef39 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1960,7 +1960,8 @@ static inline int pte_unmap_same(struct mm_struct *mm, pmd_t *pmd,
 {
 	int same = 1;
 #if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT)
-	if (sizeof(pte_t) > sizeof(unsigned long)) {
+	if (arch_needs_swap_ptl() ||
+	    sizeof(pte_t) > sizeof(unsigned long)) {
 		spinlock_t *ptl = pte_lockptr(mm, pmd);
 		spin_lock(ptl);
 		same = pte_same(*page_table, orig_pte);
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1422125 — Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak

On 06/14/2016 10:01 AM, Lukasz Anaczkowski wrote:
> v2 (Lukasz Anaczkowski):
>     () fixed compilation breakage
...

By unconditionally defining the workaround code, even on kernels where
there is no chance of ever hitting this bug.  I think that's a pretty
poor way to do it.

Can we please stick this in one of the intel.c files, so it's only
present on CPU_SUP_INTEL builds?

Which reminds me...

> --- a/arch/x86/include/asm/pgtable_64.h
> +++ b/arch/x86/include/asm/pgtable_64.h
> @@ -178,6 +178,12 @@ extern void cleanup_highmap(void);
>  extern void init_extra_mapping_uc(unsigned long phys, unsigned long size);
>  extern void init_extra_mapping_wb(unsigned long phys, unsigned long size);
>  
> +#define ARCH_HAS_NEEDS_SWAP_PTL 1
> +static inline bool arch_needs_swap_ptl(void)
> +{
> +       return boot_cpu_has_bug(X86_BUG_PTE_LEAK);
> +}

Does this *REALLY* only affect 64-bit kernels?

[toc] | [prev] | [next] | [standalone]

#1422212 — Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak

On Tue, 14 Jun 2016 10:24:16 -0700
Dave Hansen <dave.hansen@linux.intel.com> wrote:

> On 06/14/2016 10:01 AM, Lukasz Anaczkowski wrote:
> > v2 (Lukasz Anaczkowski):
> >     () fixed compilation breakage  
> ...
> 
> By unconditionally defining the workaround code, even on kernels where
> there is no chance of ever hitting this bug.  I think that's a pretty
> poor way to do it.
> 
> Can we please stick this in one of the intel.c files, so it's only
> present on CPU_SUP_INTEL builds?

Can we please make it use alternatives or somesuch so that it just goes
away at boot if its not a Knights Landing box ?

Alan

[toc] | [prev] | [next] | [standalone]

#1422230 — Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak

On 06/14/2016 11:34 AM, One Thousand Gnomes wrote:
> On Tue, 14 Jun 2016 10:24:16 -0700
> Dave Hansen <dave.hansen@linux.intel.com> wrote:
> 
>> On 06/14/2016 10:01 AM, Lukasz Anaczkowski wrote:
>>> v2 (Lukasz Anaczkowski):
>>>     () fixed compilation breakage  
>> ...
>>
>> By unconditionally defining the workaround code, even on kernels where
>> there is no chance of ever hitting this bug.  I think that's a pretty
>> poor way to do it.
>>
>> Can we please stick this in one of the intel.c files, so it's only
>> present on CPU_SUP_INTEL builds?
> 
> Can we please make it use alternatives or somesuch so that it just goes
> away at boot if its not a Knights Landing box ?

Lukasz, Borislav suggested using static_cpu_has_bug(), which will do the
alternatives patching.  It's definitely the right thing to use here.

[toc] | [prev] | [next] | [standalone]

#1422241 — Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak

On Tue, Jun 14, 2016 at 11:54:24AM -0700, Dave Hansen wrote:
> Lukasz, Borislav suggested using static_cpu_has_bug(), which will do the
> alternatives patching.  It's definitely the right thing to use here.

Yeah, either that or do an

alternative_call(null_func, fix_pte_peak, X86_BUG_PTE_LEAK, ...)

or so and you'll need a dummy function to call on !X86_BUG_PTE_LEAK
CPUs.

The static_cpu_has_bug() thing should be most likely a penalty
of a single JMP (I have to look at the asm) but then since the
callers are inlined, you'll have to patch all those places where
*ptep_get_and_clear() get inlined.

Shouldn't be a big deal still but...

"debug-alternative" and a kvm guest should help you there to get a quick
idea.

HTH.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1422302 — Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak

On 06/14/16 12:19, Borislav Petkov wrote:
> On Tue, Jun 14, 2016 at 11:54:24AM -0700, Dave Hansen wrote:
>> Lukasz, Borislav suggested using static_cpu_has_bug(), which will do the
>> alternatives patching.  It's definitely the right thing to use here.
> 
> Yeah, either that or do an
> 
> alternative_call(null_func, fix_pte_peak, X86_BUG_PTE_LEAK, ...)
> 
> or so and you'll need a dummy function to call on !X86_BUG_PTE_LEAK
> CPUs.
> 
> The static_cpu_has_bug() thing should be most likely a penalty
> of a single JMP (I have to look at the asm) but then since the
> callers are inlined, you'll have to patch all those places where
> *ptep_get_and_clear() get inlined.
> 
> Shouldn't be a big deal still but...
> 
> "debug-alternative" and a kvm guest should help you there to get a quick
> idea.
> 

static_cpu_has_bug() should turn into 5-byte NOP in the common (bugless)
case.

	-hpa

[toc] | [prev] | [next] | [standalone]

#1422324 — Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak

On Tue, Jun 14, 2016 at 01:20:06PM -0700, H. Peter Anvin wrote:
> static_cpu_has_bug() should turn into 5-byte NOP in the common (bugless)
> case.

Yeah, it does. I looked at the asm.

I wasn't 100% sure because I vaguely remember gcc reordering things in
some pathological case but I'm most likely remembering wrong because if
it were doing that, then the whole nopping out won't work. F'get about
it. :)

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1422329 — Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak

On 06/14/16 13:47, Borislav Petkov wrote:
> On Tue, Jun 14, 2016 at 01:20:06PM -0700, H. Peter Anvin wrote:
>> static_cpu_has_bug() should turn into 5-byte NOP in the common (bugless)
>> case.
> 
> Yeah, it does. I looked at the asm.
> 
> I wasn't 100% sure because I vaguely remember gcc reordering things in
> some pathological case but I'm most likely remembering wrong because if
> it were doing that, then the whole nopping out won't work. F'get about
> it. :)
> 

There was that.  It is still possible that we end up with NOP a JMP
right before another JMP; we could perhaps make the patching code
smarter and see if we have a JMP immediately after.

	-hpa

[toc] | [prev] | [next] | [standalone]

#1422332 — Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak

On 06/14/16 14:02, Borislav Petkov wrote:
> On Tue, Jun 14, 2016 at 01:54:25PM -0700, H. Peter Anvin wrote:
>> There was that.  It is still possible that we end up with NOP a JMP
>> right before another JMP; we could perhaps make the patching code
>> smarter and see if we have a JMP immediately after.
> 
> Yeah, I still can't get reproduce that reliably - I remember seeing it
> at some point but then dismissing it for another, higher-prio thing. And
> now the whole memory is hazy at best.
> 
> But, you're giving me a great idea right now - I have this kernel
> disassembler tool which dumps alternative sections already and I could
> teach it to look for pathological cases around the patching sites and
> scream.
> 
> Something for my TODO list when I get a quiet moment.
> 

It's not really pathological; the issue is that asm goto() with an
unreachable clause after it doesn't tell gcc that a certain code path
ought to be linear, so we tell it to fall through.  However, if gcc then
wants to have a jump there for whatever reason (perhaps it is part of a
loop) we end up with a redundant jump, so a patch site followed by a JMP
is entirely reasonable.

	-hpa

[toc] | [prev] | [next] | [standalone]

#1422340 — Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak

On Tue, Jun 14, 2016 at 01:54:25PM -0700, H. Peter Anvin wrote:
> There was that.  It is still possible that we end up with NOP a JMP
> right before another JMP; we could perhaps make the patching code
> smarter and see if we have a JMP immediately after.

Yeah, I still can't get reproduce that reliably - I remember seeing it
at some point but then dismissing it for another, higher-prio thing. And
now the whole memory is hazy at best.

But, you're giving me a great idea right now - I have this kernel
disassembler tool which dumps alternative sections already and I could
teach it to look for pathological cases around the patching sites and
scream.

Something for my TODO list when I get a quiet moment.

Thanks!

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1422351 — Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak

On June 14, 2016 2:02:55 PM PDT, Borislav Petkov <bp@alien8.de> wrote:
>On Tue, Jun 14, 2016 at 01:54:25PM -0700, H. Peter Anvin wrote:
>> There was that.  It is still possible that we end up with NOP a JMP
>> right before another JMP; we could perhaps make the patching code
>> smarter and see if we have a JMP immediately after.
>
>Yeah, I still can't get reproduce that reliably - I remember seeing it
>at some point but then dismissing it for another, higher-prio thing.
>And
>now the whole memory is hazy at best.
>
>But, you're giving me a great idea right now - I have this kernel
>disassembler tool which dumps alternative sections already and I could
>teach it to look for pathological cases around the patching sites and
>scream.
>
>Something for my TODO list when I get a quiet moment.
>
>Thanks!

We talked with the GCC people about always bias asm goto toward the first label even if followed by __builtin_unreachable().  I don't know if that happened; if so we should probably insert the unreachable for those versions of gcc only.
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

[toc] | [prev] | [next] | [standalone]

#1422177 — Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak

On Tue, Jun 14, 2016 at 07:01:12PM +0200, Lukasz Anaczkowski wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Knights Landing has a issue that a thread setting A or D bits
> may not do so atomically against checking the present bit.
> A thread which is going to page fault may still set those
> bits, even though the present bit was already atomically cleared.
> 
> This implies that when the kernel clears present atomically,
> some time later the supposed to be zero entry could be corrupted
> with stray A or D bits.
> 
> Since the PTE could be already used for storing a swap index,
> or a NUMA migration index, this cannot be tolerated. Most
> of the time the kernel detects the problem, but in some
> rare cases it may not.
> 
> This patch enforces that the page unmap path in vmscan/direct reclaim
> always flushes other CPUs after clearing each page, and also
> clears the PTE again after the flush.
> 
> For reclaim this brings the performance back to before Mel's
> flushing changes, but for unmap it disables batching.
> 
> This makes sure any leaked A/D bits are immediately cleared before the entry
> is used for something else.
> 
> Any parallel faults that check for entry is zero may loop,
> but they should eventually recover after the entry is written.
> 
> Also other users may spin in the page table lock until we
> "fixed" the PTE. This is ensured by always taking the page table lock
> even for the swap cache case. Previously this was only done
> on architectures with non atomic PTE accesses (such as 32bit PTE),
> but now it is also done when this bug workaround is active.
> 
> I audited apply_pte_range and other users of arch_enter_lazy...
> and they seem to all not clear the present bit.
> 
> Right now the extra flush is done in the low level
> architecture code, while the higher level code still
> does batched TLB flush. This means there is always one extra
> unnecessary TLB flush now. As a followon optimization
> this could be avoided by telling the callers that
> the flush already happenend.
> 
> v2 (Lukasz Anaczkowski):
>     () added call to smp_mb__after_atomic() to synchornize with
>        switch_mm, based on Nadav's comment
>     () fixed compilation breakage
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> Signed-off-by: Lukasz Anaczkowski <lukasz.anaczkowski@intel.com>
> ---
>  arch/x86/include/asm/cpufeatures.h |  1 +
>  arch/x86/include/asm/hugetlb.h     |  9 ++++++++-
>  arch/x86/include/asm/pgtable.h     |  5 +++++
>  arch/x86/include/asm/pgtable_64.h  |  6 ++++++
>  arch/x86/kernel/cpu/intel.c        | 10 ++++++++++
>  arch/x86/mm/tlb.c                  | 22 ++++++++++++++++++++++
>  include/linux/mm.h                 |  4 ++++
>  mm/memory.c                        |  3 ++-
>  8 files changed, 58 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 4a41348..2c48011 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -303,6 +303,7 @@
>  #define X86_BUG_SYSRET_SS_ATTRS	X86_BUG(8) /* SYSRET doesn't fix up SS attrs */
>  #define X86_BUG_NULL_SEG	X86_BUG(9) /* Nulling a selector preserves the base */
>  #define X86_BUG_SWAPGS_FENCE	X86_BUG(10) /* SWAPGS without input dep on GS */
> +#define X86_BUG_PTE_LEAK        X86_BUG(11) /* PTE may leak A/D bits after clear */
>  
>  
>  #ifdef CONFIG_X86_32
> diff --git a/arch/x86/include/asm/hugetlb.h b/arch/x86/include/asm/hugetlb.h
> index 3a10616..58e1ca9 100644
> --- a/arch/x86/include/asm/hugetlb.h
> +++ b/arch/x86/include/asm/hugetlb.h
> @@ -41,10 +41,17 @@ static inline void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
>  	set_pte_at(mm, addr, ptep, pte);
>  }
>  
> +extern void fix_pte_leak(struct mm_struct *mm, unsigned long addr,
> +			 pte_t *ptep);
> +
>  static inline pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
>  					    unsigned long addr, pte_t *ptep)
>  {
> -	return ptep_get_and_clear(mm, addr, ptep);
> +	pte_t pte = ptep_get_and_clear(mm, addr, ptep);
> +
> +	if (boot_cpu_has_bug(X86_BUG_PTE_LEAK))

static_cpu_has_bug()

> +		fix_pte_leak(mm, addr, ptep);
> +	return pte;
>  }
>  
>  static inline void huge_ptep_clear_flush(struct vm_area_struct *vma,
> diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
> index 1a27396..9769355 100644
> --- a/arch/x86/include/asm/pgtable.h
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -794,11 +794,16 @@ extern int ptep_test_and_clear_young(struct vm_area_struct *vma,
>  extern int ptep_clear_flush_young(struct vm_area_struct *vma,
>  				  unsigned long address, pte_t *ptep);
>  
> +extern void fix_pte_leak(struct mm_struct *mm, unsigned long addr,
> +			 pte_t *ptep);
> +
>  #define __HAVE_ARCH_PTEP_GET_AND_CLEAR
>  static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned long addr,
>  				       pte_t *ptep)
>  {
>  	pte_t pte = native_ptep_get_and_clear(ptep);
> +	if (boot_cpu_has_bug(X86_BUG_PTE_LEAK))

static_cpu_has_bug()

> +		fix_pte_leak(mm, addr, ptep);
>  	pte_update(mm, addr, ptep);
>  	return pte;
>  }
> diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h
> index 2ee7811..6fa4079 100644
> --- a/arch/x86/include/asm/pgtable_64.h
> +++ b/arch/x86/include/asm/pgtable_64.h
> @@ -178,6 +178,12 @@ extern void cleanup_highmap(void);
>  extern void init_extra_mapping_uc(unsigned long phys, unsigned long size);
>  extern void init_extra_mapping_wb(unsigned long phys, unsigned long size);
>  
> +#define ARCH_HAS_NEEDS_SWAP_PTL 1
> +static inline bool arch_needs_swap_ptl(void)
> +{
> +       return boot_cpu_has_bug(X86_BUG_PTE_LEAK);

static_cpu_has_bug()

> +}
> +
>  #endif /* !__ASSEMBLY__ */
>  
>  #endif /* _ASM_X86_PGTABLE_64_H */
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index 6e2ffbe..f499513 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -181,6 +181,16 @@ static void early_init_intel(struct cpuinfo_x86 *c)
>  		}
>  	}
>  
> +	if (c->x86_model == 87) {

That should be INTEL_FAM6_XEON_PHI_KNL, AFAICT.

> +		static bool printed;
> +
> +		if (!printed) {
> +			pr_info("Enabling PTE leaking workaround\n");
> +			printed = true;
> +		}

pr_info_once

> +		set_cpu_bug(c, X86_BUG_PTE_LEAK);
> +	}
> +
>  	/*
>  	 * Intel Quark Core DevMan_001.pdf section 6.4.11
>  	 * "The operating system also is required to invalidate (i.e., flush)
-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1423007 — RE: [PATCH v2] Linux VM workaround for Knights Landing A/D leak

From: Borislav Petkov [mailto:bp@alien8.de] 
Sent: Tuesday, June 14, 2016 8:10 PM

>> +	if (boot_cpu_has_bug(X86_BUG_PTE_LEAK))
>
> static_cpu_has_bug()

>> +	if (c->x86_model == 87) {
>
> That should be INTEL_FAM6_XEON_PHI_KNL, AFAICT.

>> +		static bool printed;
>> +
>> +		if (!printed) {
>> +			pr_info("Enabling PTE leaking workaround\n");
>> +			printed = true;
>> +		}
>
> pr_info_once

Thanks, Boris! This is very valuable. I'll address  those comments in next version of the patch.

Cheers,
Lukasz

[toc] | [prev] | [next] | [standalone]

#1422192 — Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak

Lukasz Anaczkowski <lukasz.anaczkowski@intel.com> wrote:

> From: Andi Kleen <ak@linux.intel.com>

> static inline pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
> 					    unsigned long addr, pte_t *ptep)
> {
> -	return ptep_get_and_clear(mm, addr, ptep);
> +	pte_t pte = ptep_get_and_clear(mm, addr, ptep);
> +
> +	if (boot_cpu_has_bug(X86_BUG_PTE_LEAK))
> +		fix_pte_leak(mm, addr, ptep);
> +	return pte;
> }

I missed it on the previous iteration: ptep_get_and_clear already calls 
fix_pte_leak when needed. So do you need to call it again here?

Thanks,
Nadav

[toc] | [prev] | [next] | [standalone]

#1423008 — RE: [PATCH v2] Linux VM workaround for Knights Landing A/D leak

From: Nadav Amit [mailto:nadav.amit@gmail.com] 
Sent: Tuesday, June 14, 2016 8:38 PM

>> +	pte_t pte = ptep_get_and_clear(mm, addr, ptep);
>> +
>> +	if (boot_cpu_has_bug(X86_BUG_PTE_LEAK))
>> +		fix_pte_leak(mm, addr, ptep);
>> +	return pte;
>> }
>
> I missed it on the previous iteration: ptep_get_and_clear already calls 
> fix_pte_leak when needed. So do you need to call it again here?

You're right, Nadav. Not needing this. Will be removed in next version of the patch.

Cheers,
Lukasz

[toc] | [prev] | [next] | [standalone]

#1423370 — Re: [PATCH v2] Linux VM workaround for Knights Landing A/D leak

Lukasz <lukasz.anaczkowski@intel.com> wrote:

> From: Nadav Amit [mailto:nadav.amit@gmail.com] 
> Sent: Tuesday, June 14, 2016 8:38 PM
> 
>>> +	pte_t pte = ptep_get_and_clear(mm, addr, ptep);
>>> +
>>> +	if (boot_cpu_has_bug(X86_BUG_PTE_LEAK))
>>> +		fix_pte_leak(mm, addr, ptep);
>>> +	return pte;
>>> }
>> 
>> I missed it on the previous iteration: ptep_get_and_clear already calls 
>> fix_pte_leak when needed. So do you need to call it again here?
> 
> You're right, Nadav. Not needing this. Will be removed in next version of the patch.

Be careful here. According to the SDM when invalidating a huge-page,
each 4KB page needs to be invalidated separately. In practice, when
Linux invalidates 2MB/1GB pages it performs a full TLB flush. The
full flush may not be required on knights landing, and specifically
for the workaround, but you should check.  

Regards,
Nadav

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web