Fix issues with huge mapping in ioremap for ARM64

PATCH Superseded Chintan Pandya 1 participant 6 messages May 24, 2018

Subsystems (6)

Stable branch x86 Library code arm64 +2 more subsystems

lore

Share & tools

lore

Chintan Pandya

author

2018-05-24 13:57 UTC #0 lore raw

v10 cover Fix issues with huge mapping in ioremap for ARM64

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

Patch 1/4 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.

Patch 4/4 may conflict with Laura's patch "[PATCHv2] arm64: Make sure permission updates happen for pmd/pud". It looks to me that, problem addressed by this patch will also get resolved by below series of patches. There could be multiple ways to resolve this. I would let maintainer suggest the best approach, and I will do the needful.

This 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 16K 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.

Quoted 1 line

From V9->V10:

- Updated commit log for patch 1/4 by Toshi
- Addressed review comments by Will on patch 3/4

Quoted 1 line

From V8->V9:

- Used __TLBI_VADDR macros in new TLB flush API

Quoted 1 line

From V7->V8:

- Properly fixed compilation issue in x86 file

Quoted 1 line

From V6->V7:

- Fixed compilation issue in x86 case
- V6 patches were not properly enumarated

Quoted 1 line

From V5->V6:

- Use __flush_tlb_kernel_pgtable() for both PUD and PMD. Remove "bool tlb_inv" based variance as it is not need now
- Re-naming for consistency

Quoted 1 line

From V4->V5:

- Add new API __flush_tlb_kernel_pgtable(unsigned long addr) for kernel addresses

Quoted 1 line

From V3->V4:

- Add header for 'addr' in x86 implementation
- Re-order pmd/pud clear and table free
- Avoid redundant TLB invalidatation in one perticular case

Quoted 1 line

From V2->V3:

- Use the exisiting page table free interface to do arm64 specific things

Quoted 1 line

From V1->V2:

- Rebased my patches on top of "[PATCH v2 1/2] mm/vmalloc:
Add interfaces to free unmapped page table"
- Honored BBM for ARM64

Chintan Pandya (4):
ioremap: Update pgtable free interfaces with addr
arm64: tlbflush: Introduce __flush_tlb_kernel_pgtable
arm64: Implement page table free interfaces Revert "arm64: Enforce BBM for huge IO/VMAP mappings"

 arch/arm64/include/asm/tlbflush.h |  7 +++++++
 arch/arm64/mm/mmu.c               | 42 ++++++++++++++++++++++++++++-----------
 arch/x86/mm/pgtable.c             |  8 +++++---
 include/asm-generic/pgtable.h     |  8 ++++----
 lib/ioremap.c                     |  4 ++--
 5 files changed, 48 insertions(+), 21 deletions(-)

Signature · 3 lines

Qualcomm India Private Limited, on behalf of Qualcomm Innovation
Center, Inc., is a member of Code Aurora Forum, a Linux Foundation
Collaborative Project

Chintan Pandya

author

2018-05-24 13:57 UTC #1 lore raw

PATCH v10 1/4 ioremap: Update pgtable free interfaces with addr

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.

[toshi at hpe.com: 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(-)

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

index 2dbb2c9..da98828 100644
@@ -973,12 +973,12 @@ int pmd_clear_huge(pmd_t *pmdp)
 	return 1;
 }
 
-int pud_free_pmd_page(pud_t *pud)
+int pud_free_pmd_page(pud_t *pud, unsigned long addr)
 {
 	return pud_none(*pud);
 }
 
-int pmd_free_pte_page(pmd_t *pmd)
+int pmd_free_pte_page(pmd_t *pmd, unsigned long addr)
 {
 	return pmd_none(*pmd);
 }

arch/x86/mm/pgtable.c +5 -3

index ffc8c13..37e3cba 100644
@@ -718,11 +718,12 @@ int pmd_clear_huge(pmd_t *pmd)
 /**
  * pud_free_pmd_page - Clear pud entry and free pmd page.
  * @pud: Pointer to a PUD.
+ * @addr: Virtual address associated with pud.
  *
  * Context: The pud range has been unmaped and TLB purged.
  * Return: 1 if clearing the entry succeeded. 0 otherwise.
  */
-int pud_free_pmd_page(pud_t *pud)
+int pud_free_pmd_page(pud_t *pud, unsigned long addr)
 {
 	pmd_t *pmd;
 	int i;

@@ -733,7 +734,7 @@ int pud_free_pmd_page(pud_t *pud)
 	pmd = (pmd_t *)pud_page_vaddr(*pud);
 
 	for (i = 0; i < PTRS_PER_PMD; i++)
-		if (!pmd_free_pte_page(&pmd[i]))
+		if (!pmd_free_pte_page(&pmd[i], addr + (i * PMD_SIZE)))
 			return 0;
 
 	pud_clear(pud);

@@ -745,11 +746,12 @@ int pud_free_pmd_page(pud_t *pud)
 /**
  * pmd_free_pte_page - Clear pmd entry and free pte page.
  * @pmd: Pointer to a PMD.
+ * @addr: Virtual address associated with pmd.
  *
  * Context: The pmd range has been unmaped and TLB purged.
  * Return: 1 if clearing the entry succeeded. 0 otherwise.
  */
-int pmd_free_pte_page(pmd_t *pmd)
+int pmd_free_pte_page(pmd_t *pmd, unsigned long addr)
 {
 	pte_t *pte;

include/asm-generic/pgtable.h +4 -4

index f59639a..b081794 100644
@@ -1019,8 +1019,8 @@ static inline int p4d_clear_huge(p4d_t *p4d)
 int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot);
 int pud_clear_huge(pud_t *pud);
 int pmd_clear_huge(pmd_t *pmd);
-int pud_free_pmd_page(pud_t *pud);
-int pmd_free_pte_page(pmd_t *pmd);
+int pud_free_pmd_page(pud_t *pud, unsigned long addr);
+int pmd_free_pte_page(pmd_t *pmd, unsigned long addr);
 #else	/* !CONFIG_HAVE_ARCH_HUGE_VMAP */
 static inline int p4d_set_huge(p4d_t *p4d, phys_addr_t addr, pgprot_t prot)
 {

@@ -1046,11 +1046,11 @@ static inline int pmd_clear_huge(pmd_t *pmd)
 {
 	return 0;
 }
-static inline int pud_free_pmd_page(pud_t *pud)
+static inline int pud_free_pmd_page(pud_t *pud, unsigned long addr)
 {
 	return 0;
 }
-static inline int pmd_free_pte_page(pmd_t *pmd)
+static inline int pmd_free_pte_page(pmd_t *pmd, unsigned long addr)
 {
 	return 0;
 }

lib/ioremap.c +2 -2

index 54e5bba..517f585 100644
@@ -92,7 +92,7 @@ static inline int ioremap_pmd_range(pud_t *pud, unsigned long addr,
 		if (ioremap_pmd_enabled() &&
 		    ((next - addr) == PMD_SIZE) &&
 		    IS_ALIGNED(phys_addr + addr, PMD_SIZE) &&
-		    pmd_free_pte_page(pmd)) {
+		    pmd_free_pte_page(pmd, addr)) {
 			if (pmd_set_huge(pmd, phys_addr + addr, prot))
 				continue;
 		}

@@ -119,7 +119,7 @@ static inline int ioremap_pud_range(p4d_t *p4d, unsigned long addr,
 		if (ioremap_pud_enabled() &&
 		    ((next - addr) == PUD_SIZE) &&
 		    IS_ALIGNED(phys_addr + addr, PUD_SIZE) &&
-		    pud_free_pmd_page(pud)) {
+		    pud_free_pmd_page(pud, addr)) {
 			if (pud_set_huge(pud, phys_addr + addr, prot))
 				continue;
 		}

Signature · 3 lines

Qualcomm India Private Limited, on behalf of Qualcomm Innovation
Center, Inc., is a member of Code Aurora Forum, a Linux Foundation
Collaborative Project

Chintan Pandya

author

2018-05-24 13:57 UTC #2 lore raw

PATCH v10 2/4 arm64: tlbflush: Introduce __flush_tlb_kernel_pgtable

In reply to Chintan Pandya

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 +7 -0

index dfc61d7..a4a1901 100644
@@ -218,6 +218,13 @@ static inline void __flush_tlb_pgtable(struct mm_struct *mm,
 	dsb(ish);
 }
 
+static inline void __flush_tlb_kernel_pgtable(unsigned long kaddr)
+{
+	unsigned long addr = __TLBI_VADDR(kaddr, 0);
+
+	__tlbi(vaae1is, addr);
+	dsb(ish);
+}
 #endif
 
 #endif

Signature · 3 lines

Qualcomm India Private Limited, on behalf of Qualcomm Innovation
Center, Inc., is a member of Code Aurora Forum, a Linux Foundation
Collaborative Project

Chintan Pandya

author

2018-05-24 13:57 UTC #3 lore raw

PATCH v10 3/4 arm64: Implement page table free interfaces

In reply to Chintan Pandya

Implement pud_free_pmd_page() and pmd_free_pte_page().

Implementation requires,
1) Clearing off the current pud/pmd entry
2) Invalidate TLB which could have previously
valid but not stale entry
3) Freeing of the un-used next level page tables

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

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

arch/arm64/mm/mmu.c +30 -4

index da98828..17d9282 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)

@@ -973,12 +974,37 @@ 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);
+	pmd_t *table;
+	pmd_t pmd;
+
+	pmd = READ_ONCE(*pmdp);
+	if (pmd_present(pmd)) {
+		table = __va(pmd_val(pmd));
+		pmd_clear(pmdp);
+		__flush_tlb_kernel_pgtable(addr);
+		pte_free_kernel(NULL, (pte_t *) table);
+	}
+	return 1;
 }
 
-int pmd_free_pte_page(pmd_t *pmd, unsigned long addr)
+int pud_free_pmd_page(pud_t *pudp, unsigned long addr)
 {
-	return pmd_none(*pmd);
+	pmd_t *table;
+	pud_t pud;
+	int i = 0;
+
+	pud = READ_ONCE(*pudp);
+	if (pud_present(pud)) {
+		table = __va(pud_val(pud));
+		do {
+			pmd_free_pte_page(&table[i], addr + (i * PMD_SIZE));
+		} while (i++, i < PTRS_PER_PMD);
+
+		pud_clear(pudp);
+		__flush_tlb_kernel_pgtable(addr);
+		pmd_free(NULL, table);
+	}
+	return 1;
 }

Signature · 3 lines

Qualcomm India Private Limited, on behalf of Qualcomm Innovation
Center, Inc., is a member of Code Aurora Forum, a Linux Foundation
Collaborative Project

Chintan Pandya

author

2018-05-24 14:59 UTC #4 lore raw

Re: [PATCH v10 3/4] arm64: Implement page table free interfaces

Quoted 50 lines · Chintan Pandya

Implement pud_free_pmd_page() and pmd_free_pte_page().

Implementation requires,
1) Clearing off the current pud/pmd entry
2) Invalidate TLB which could have previously valid but not stale entry
3) Freeing of the un-used next level page tables

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

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

arch/arm64/mm/mmu.c +14 -4

index da98828..17d9282 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)

@@ -973,12 +974,37 @@ 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);
+	pmd_t *table;
+	pmd_t pmd;
+
+	pmd = READ_ONCE(*pmdp);
+	if (pmd_present(pmd)) {
+		table = __va(pmd_val(pmd));
+		pmd_clear(pmdp);
+		__flush_tlb_kernel_pgtable(addr);
+		pte_free_kernel(NULL, (pte_t *) table);
+	}
+	return 1;
  }
 
-int pmd_free_pte_page(pmd_t *pmd, unsigned long addr)
+int pud_free_pmd_page(pud_t *pudp, unsigned long addr)
  {
-	return pmd_none(*pmd);

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

Quoted 10 lines

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

+	pmd_t *table;
+	pud_t pud;
+	int i = 0;
+
+	pud = READ_ONCE(*pudp);
+	if (pud_present(pud)) {
+		table = __va(pud_val(pud));
+		do {
+			pmd_free_pte_page(&table[i], addr + (i * PMD_SIZE));
+		} while (i++, i < PTRS_PER_PMD);

Hi Will,

Right after sending these patches, I realized that do-while condition can be made to look like what we see in ioremap/vmalloc code. I guess, that's what you suggested. So, I'll raise v11 fixing that. Any more concerns ?

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

Quoted 8 lines

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

+
+		pud_clear(pudp);
+		__flush_tlb_kernel_pgtable(addr);
+		pmd_free(NULL, table);
+	}
+	return 1;
  }

Chintan

Signature · 3 lines

Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation
Collaborative Project

Chintan Pandya

author

2018-05-24 13:57 UTC #5 lore raw

PATCH v10 4/4 Revert "arm64: Enforce BBM for huge IO/VMAP mappings"

In reply to Chintan Pandya

This commit 15122ee2c515a ("arm64: Enforce BBM for huge IO/VMAP mappings") is a temporary work-around until the issues with CONFIG_HAVE_ARCH_HUGE_VMAP gets fixed.

Revert this change as we have fixes for the issue.

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

---
 arch/arm64/mm/mmu.c | 8 --------
 1 file changed, 8 deletions(-)

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

index 17d9282..defb3bb 100644
@@ -935,10 +935,6 @@ int pud_set_huge(pud_t *pudp, phys_addr_t phys, pgprot_t prot)
 	pgprot_t sect_prot = __pgprot(PUD_TYPE_SECT |
 					pgprot_val(mk_sect_prot(prot)));
 
-	/* ioremap_page_range doesn't honour BBM */
-	if (pud_present(READ_ONCE(*pudp)))
-		return 0;
-
 	BUG_ON(phys & ~PUD_MASK);
 	set_pud(pudp, pfn_pud(__phys_to_pfn(phys), sect_prot));
 	return 1;

@@ -949,10 +945,6 @@ int pmd_set_huge(pmd_t *pmdp, phys_addr_t phys, pgprot_t prot)
 	pgprot_t sect_prot = __pgprot(PMD_TYPE_SECT |
 					pgprot_val(mk_sect_prot(prot)));
 
-	/* ioremap_page_range doesn't honour BBM */
-	if (pmd_present(READ_ONCE(*pmdp)))
-		return 0;
-
 	BUG_ON(phys & ~PMD_MASK);
 	set_pmd(pmdp, pfn_pmd(__phys_to_pfn(phys), sect_prot));
 	return 1;

Signature · 3 lines

Qualcomm India Private Limited, on behalf of Qualcomm Innovation
Center, Inc., is a member of Code Aurora Forum, a Linux Foundation
Collaborative Project