Fix issues with huge mapping in ioremap for ARM64

From V12->V13:

From V11->V12:

From V10->V11:

From V9->V10:

From V8->V9:

From V7->V8:

From V6->V7:

From V5->V6:

From V4->V5:

From V3->V4:

From V2->V3:

From V1->V2:

From: Chintan Pandya <[email protected]>

The following kernel panic was observed on ARM64 platform due to a stale TLB entry.

1. ioremap with 4K size, a valid pte page table is set.
2. iounmap it, its pte entry is set to 0.
3. ioremap the same address with 2M size, update its pmd entry with a new value.
4. CPU may hit an exception because the old pmd entry is still in TLB, which leads to a kernel panic.

Commit b6bdb7517c3d ("mm/vmalloc: add interfaces to free unmapped page table") has addressed this panic by falling to pte mappings in the above case on ARM64.

To support pmd mappings in all cases, TLB purge needs to be performed in this case on ARM64.

Add a new arg, 'addr', to pud_free_pmd_page() and pmd_free_pte_page() so that TLB purge can be added later in seprate patches.

[[email protected]: merge changes, rewrite patch description]

Fixes 28ee90fe6048 ("x86/mm: implement free pmd/pte page interfaces")

Signed-off-by Chintan Pandya <[email protected]>

Signed-off-by Toshi Kani <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Michal Hocko <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc "H. Peter Anvin" <[email protected]>

Cc Will Deacon <[email protected]>

Cc Joerg Roedel <[email protected]>

Cc <[email protected]>

---
 arch/arm64/mm/mmu.c           | 4 ++--
 arch/x86/mm/pgtable.c         | 8 +++++---
 include/asm-generic/pgtable.h | 8 ++++----
 lib/ioremap.c                 | 4 ++--
 4 files changed, 13 insertions(+), 11 deletions(-)

Add an interface to invalidate intermediate page tables from TLB for kernel.

Signed-off-by Chintan Pandya <[email protected]>

---
 arch/arm64/include/asm/tlbflush.h | 7 +++++++
 1 file changed, 7 insertions(+)

arch/arm64/include/asm/tlbflush.h +0 -0

index dfc61d7..a4a1901 100644
@@ -218,6 +218,13 @@ static inline void __flush_tlb_pgtable(struct mm_struct *mm,
 	dsb(ish);

arm64 requires break-before-make. Originally, before setting up new pmd/pud entry for huge mapping, in few cases, the modifying pmd/pud entry was still valid and pointing to next level page table as we only clear off leaf PTE in unmap leg.

a) This was resulting into stale entry in TLBs (as few TLBs also cache intermediate mapping for performance reasons)
b) Also, modifying pmd/pud was the only reference to next level page table and it was getting lost without freeing it. So, page leaks were happening.

Implement pud_free_pmd_page() and pmd_free_pte_page() to enforce BBM and also free the leaking page tables.

Implementation requires,
1) Clearing off the current pud/pmd entry
2) Invalidation of TLB
3) Freeing of the un-used next level page tables

Signed-off-by Chintan Pandya <[email protected]>

---
 arch/arm64/mm/mmu.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)

arch/arm64/mm/mmu.c +2 -2

index 8ae5d7a..65f8627 100644
@@ -45,6 +45,7 @@
 #include <asm/memblock.h>
 #include <asm/mmu_context.h>
 #include <asm/ptdump.h>
+#include <asm/tlbflush.h>
 
 #define NO_BLOCK_MAPPINGS	BIT(0)
 #define NO_CONT_MAPPINGS	BIT(1)

@@ -977,12 +978,51 @@ int pmd_clear_huge(pmd_t *pmdp)
 	return 1;
 }
 
-int pud_free_pmd_page(pud_t *pud, unsigned long addr)
+int pmd_free_pte_page(pmd_t *pmdp, unsigned long addr)
 {
-	return pud_none(*pud);

Quoted 20 lines · Chintan Pandya

arch/arm64/mm/mmu.c +2 -2

index 8ae5d7a..65f8627 100644
@@ -45,6 +45,7 @@
 #include <asm/memblock.h>
 #include <asm/mmu_context.h>
 #include <asm/ptdump.h>
+#include <asm/tlbflush.h>
 
 #define NO_BLOCK_MAPPINGS	BIT(0)
 #define NO_CONT_MAPPINGS	BIT(1)

@@ -977,12 +978,51 @@ int pmd_clear_huge(pmd_t *pmdp)
 	return 1;
 }
 
-int pud_free_pmd_page(pud_t *pud, unsigned long addr)
+int pmd_free_pte_page(pmd_t *pmdp, unsigned long addr)
 {
-	return pud_none(*pud);

arch/arm64/mm/mmu.c int pmd_clear_huge(pmd_t *pmdp) · line 992

Quoted 10 lines

arch/arm64/mm/mmu.c +10 -0

+	pte_t *table;
+	pmd_t pmd;
+
+	pmd = READ_ONCE(*pmdp);
+
+	/* No-op for empty entry and WARN_ON for valid entry */
+	if (!pmd_present(pmd) || !pmd_table(pmd)) {
+		VM_WARN_ON(!pmd_table(pmd));
+		return 1;
+	}

What's this VM_WARN_ON supposed to do here? If the pmd is 0, we trigger it all the time. Did you actually mean something like:

		VM_WARN_ON(!pmd_none(pmd));

or pmd_present(pmd)?

Since the comment mentions empty entry, I'd rather make it explicit:

	if (pmd_none(pmd) || !pmd_table(pmd))
		VM_WARN_ON(!pmd_none(pmd));
		return 1;
	}

This series of patches re-bring huge vmap back for arm64.

Patch 1/3 has been taken by Toshi in his series of patches by name "[PATCH v3 0/3] fix free pmd/pte page handlings on x86" to avoid merge conflict with this series.

These patches are tested on 4.16 kernel with Cortex-A75 based SoC.

The test used for verifying these patches is a stress test on ioremap/unmap which tries to re-use same io-address but changes size of mapping randomly i.e. 4K to 2M to 1G etc. The same test used to reproduce 3rd level translation fault without these fixes (and also of course with Revert "arm64: Enforce BBM for huge IO/VMAP mappings" being part of the tree).

These patches can also go into '-stable' branch (if accepted) for 4.6 onwards.

 arch/arm64/include/asm/tlbflush.h |  7 ++++++
 arch/arm64/mm/mmu.c               | 48 +++++++++++++++++++++++++++++++++++----
 arch/x86/mm/pgtable.c             |  8 ++++---
 include/asm-generic/pgtable.h     |  8 +++----
 lib/ioremap.c                     |  4 ++--

Hi Chintan,

Thanks for sticking with this. I've reviewed the series now and I'm keen for it to land in mainline. Just a couple of things below.

Quoted 1 line

This series of patches re-bring huge vmap back for arm64.

Quoted 2 lines

These patches can also go into '-stable' branch (if accepted) for 4.6 onwards.

Not sure we need to target -stable, since we solved the crash by disabling the use of huge io mappings.

Quoted 6 lines

  arch/arm64/include/asm/tlbflush.h |  7 ++++++
  arch/arm64/mm/mmu.c               | 48 +++++++++++++++++++++++++++++++++++----
  arch/x86/mm/pgtable.c             |  8 ++++---
  include/asm-generic/pgtable.h     |  8 +++----
  lib/ioremap.c                     |  4 ++--

If you get an ack from the x86 folks, then I could take all of this via arm64. Alternatively, now that I've reviewed the series this could happily go via another tree (e.g. akpm).

Thanks,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 6 lines

This series of patches re-bring huge vmap back for arm64.

Patch 1/3 has been taken by Toshi in his series of patches by name "[PATCH v3 0/3] fix free pmd/pte page handlings on x86" to avoid merge conflict with this series.

Quoted 6 lines

  arch/arm64/include/asm/tlbflush.h |  7 ++++++
  arch/arm64/mm/mmu.c               | 48 +++++++++++++++++++++++++++++++++++----
  arch/x86/mm/pgtable.c             |  8 ++++---
  include/asm-generic/pgtable.h     |  8 +++----
  lib/ioremap.c                     |  4 ++--

If you get an ack from the x86 folks, then I could take all of this via arm64. Alternatively, now that I've reviewed the series this could happily go via another tree (e.g. akpm).

Thanks,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel