[PATCH v13 0/3] Fix issues with huge mapping in ioremap for ARM64

[PATCH v13 0/3] Fix issues with huge mapping in ioremap for ARM64

[Chintan Pandya]

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.

>From V12->V13:
 - Dropped v12 3/5 and v12 5/5
 - Using existing APIs like pte_offset_kernel and pmd_offset
   instead of pmd_page_vaddr & pud_page_vaddr
 - Updated commit log message for 3/3
 - Some other cosmetic corrections in 3/3

>From V11->V12:
 - Introduced p*d_page_vaddr helper macros and using them
 - Rebased over current tip

>From V10->V11:
 - Updated pud_free_pmd_page & pmd_free_pte_page to use consistent
   conding style
 - Fixed few bugs by using pmd_page_paddr & pud_page_paddr

>From V9->V10:
 - Updated commit log for patch 1/4 by Toshi
 - Addressed review comments by Will on patch 3/4

>From V8->V9:
 - Used __TLBI_VADDR macros in new TLB flush API

>From V7->V8:
 - Properly fixed compilation issue in x86 file

>From V6->V7:
 - Fixed compilation issue in x86 case
 - V6 patches were not properly enumarated

>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

>From V4->V5:
 - Add new API __flush_tlb_kernel_pgtable(unsigned long addr)
   for kernel addresses

>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

>From V2->V3:
 - Use the exisiting page table free interface to do arm64
   specific things

>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 (3):
  ioremap: Update pgtable free interfaces with addr
  arm64: tlbflush: Introduce __flush_tlb_kernel_pgtable
  arm64: Implement page table free interfaces

 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 ++--
 5 files changed, 62 insertions(+), 13 deletions(-)

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

[PATCH v13 0/3] Fix issues with huge mapping in ioremap for ARM64

[Chintan Pandya]

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.

>From V12->V13:
 - Dropped v12 3/5 and v12 5/5
 - Using existing APIs like pte_offset_kernel and pmd_offset
   instead of pmd_page_vaddr & pud_page_vaddr
 - Updated commit log message for 3/3
 - Some other cosmetic corrections in 3/3

>From V11->V12:
 - Introduced p*d_page_vaddr helper macros and using them
 - Rebased over current tip

>From V10->V11:
 - Updated pud_free_pmd_page & pmd_free_pte_page to use consistent
   conding style
 - Fixed few bugs by using pmd_page_paddr & pud_page_paddr

>From V9->V10:
 - Updated commit log for patch 1/4 by Toshi
 - Addressed review comments by Will on patch 3/4

>From V8->V9:
 - Used __TLBI_VADDR macros in new TLB flush API

>From V7->V8:
 - Properly fixed compilation issue in x86 file

>From V6->V7:
 - Fixed compilation issue in x86 case
 - V6 patches were not properly enumarated

>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

>From V4->V5:
 - Add new API __flush_tlb_kernel_pgtable(unsigned long addr)
   for kernel addresses

>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

>From V2->V3:
 - Use the exisiting page table free interface to do arm64
   specific things

>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 (3):
  ioremap: Update pgtable free interfaces with addr
  arm64: tlbflush: Introduce __flush_tlb_kernel_pgtable
  arm64: Implement page table free interfaces

 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 ++--
 5 files changed, 62 insertions(+), 13 deletions(-)

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

[PATCH v13 1/3] ioremap: Update pgtable free interfaces with addr

[Chintan Pandya]

From: Chintan Pandya <cpandya@codeaurora.org>

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@hpe.com: merge changes, rewrite patch description]
Fixes: 28ee90fe6048 ("x86/mm: implement free pmd/pte page interfaces")
Signed-off-by: Chintan Pandya <cpandya@codeaurora.org>
Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: <stable@vger.kernel.org>

---
 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(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 493ff75..8ae5d7a 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -977,12 +977,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);
 }
diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index ffc8c13..37e3cba 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -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;
 
diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
index f59639a..b081794 100644
--- a/include/asm-generic/pgtable.h
+++ b/include/asm-generic/pgtable.h
@@ -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;
 }
diff --git a/lib/ioremap.c b/lib/ioremap.c
index 54e5bba..517f585 100644
--- a/lib/ioremap.c
+++ b/lib/ioremap.c
@@ -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;
 		}
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation
Center, Inc., is a member of Code Aurora Forum, a Linux Foundation
Collaborative Project

[PATCH v13 1/3] ioremap: Update pgtable free interfaces with addr

[Chintan Pandya]

From: Chintan Pandya <cpandya@codeaurora.org>

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 <cpandya@codeaurora.org>
Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: <stable@vger.kernel.org>

---
 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(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 493ff75..8ae5d7a 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -977,12 +977,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);
 }
diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index ffc8c13..37e3cba 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -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;
 
diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
index f59639a..b081794 100644
--- a/include/asm-generic/pgtable.h
+++ b/include/asm-generic/pgtable.h
@@ -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;
 }
diff --git a/lib/ioremap.c b/lib/ioremap.c
index 54e5bba..517f585 100644
--- a/lib/ioremap.c
+++ b/lib/ioremap.c
@@ -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;
 		}
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation
Center, Inc., is a member of Code Aurora Forum, a Linux Foundation
Collaborative Project

[PATCH v13 2/3] arm64: tlbflush: Introduce __flush_tlb_kernel_pgtable

[Chintan Pandya]

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

Signed-off-by: Chintan Pandya <cpandya@codeaurora.org>
---
 arch/arm64/include/asm/tlbflush.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
index dfc61d7..a4a1901 100644
--- a/arch/arm64/include/asm/tlbflush.h
+++ b/arch/arm64/include/asm/tlbflush.h
@@ -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
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation
Center, Inc., is a member of Code Aurora Forum, a Linux Foundation
Collaborative Project

[PATCH v13 2/3] arm64: tlbflush: Introduce __flush_tlb_kernel_pgtable

[Chintan Pandya]

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

Signed-off-by: Chintan Pandya <cpandya@codeaurora.org>
---
 arch/arm64/include/asm/tlbflush.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
index dfc61d7..a4a1901 100644
--- a/arch/arm64/include/asm/tlbflush.h
+++ b/arch/arm64/include/asm/tlbflush.h
@@ -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
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation
Center, Inc., is a member of Code Aurora Forum, a Linux Foundation
Collaborative Project

[PATCH v13 3/3] arm64: Implement page table free interfaces

[Chintan Pandya]

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 <cpandya@codeaurora.org>
---
 arch/arm64/mm/mmu.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 8ae5d7a..65f8627 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -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);
+	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;
+	}
+
+	table = pte_offset_kernel(pmdp, addr);
+	pmd_clear(pmdp);
+	__flush_tlb_kernel_pgtable(addr);
+	pte_free_kernel(NULL, 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;
+	pmd_t *pmdp;
+	pud_t pud;
+	unsigned long next, end;
+
+	pud = READ_ONCE(*pudp);
+
+	/* No-op for empty entry and WARN_ON for valid entry */
+	if (!pud_present(pud) || !pud_table(pud)) {
+		VM_WARN_ON(!pud_table(pud));
+		return 1;
+	}
+
+	table = pmd_offset(pudp, addr);
+	pmdp = table;
+	next = addr;
+	end = addr + PUD_SIZE;
+	do {
+		pmd_free_pte_page(pmdp, next);
+	} while (pmdp++, next += PMD_SIZE, next != end);
+
+	pud_clear(pudp);
+	__flush_tlb_kernel_pgtable(addr);
+	pmd_free(NULL, table);
+	return 1;
 }
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation
Center, Inc., is a member of Code Aurora Forum, a Linux Foundation
Collaborative Project

[PATCH v13 3/3] arm64: Implement page table free interfaces

[Chintan Pandya]

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 <cpandya@codeaurora.org>
---
 arch/arm64/mm/mmu.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 8ae5d7a..65f8627 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -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);
+	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;
+	}
+
+	table = pte_offset_kernel(pmdp, addr);
+	pmd_clear(pmdp);
+	__flush_tlb_kernel_pgtable(addr);
+	pte_free_kernel(NULL, 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;
+	pmd_t *pmdp;
+	pud_t pud;
+	unsigned long next, end;
+
+	pud = READ_ONCE(*pudp);
+
+	/* No-op for empty entry and WARN_ON for valid entry */
+	if (!pud_present(pud) || !pud_table(pud)) {
+		VM_WARN_ON(!pud_table(pud));
+		return 1;
+	}
+
+	table = pmd_offset(pudp, addr);
+	pmdp = table;
+	next = addr;
+	end = addr + PUD_SIZE;
+	do {
+		pmd_free_pte_page(pmdp, next);
+	} while (pmdp++, next += PMD_SIZE, next != end);
+
+	pud_clear(pudp);
+	__flush_tlb_kernel_pgtable(addr);
+	pmd_free(NULL, table);
+	return 1;
 }
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation
Center, Inc., is a member of Code Aurora Forum, a Linux Foundation
Collaborative Project

Re: [PATCH v13 1/3] ioremap: Update pgtable free interfaces with addr

[Will Deacon]

On Wed, Jun 06, 2018 at 12:31:19PM +0530, Chintan Pandya wrote:
> From: Chintan Pandya <cpandya@codeaurora.org>
> 
> 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@hpe.com: merge changes, rewrite patch description]
> Fixes: 28ee90fe6048 ("x86/mm: implement free pmd/pte page interfaces")
> Signed-off-by: Chintan Pandya <cpandya@codeaurora.org>
> Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: <stable@vger.kernel.org>
> 
> ---
>  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(-)

Reviewed-by: Will Deacon <will.deacon@arm.com>

Thanks,

Will

[PATCH v13 1/3] ioremap: Update pgtable free interfaces with addr

[Will Deacon]

On Wed, Jun 06, 2018 at 12:31:19PM +0530, Chintan Pandya wrote:
> From: Chintan Pandya <cpandya@codeaurora.org>
> 
> 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 <cpandya@codeaurora.org>
> Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: <stable@vger.kernel.org>
> 
> ---
>  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(-)

Reviewed-by: Will Deacon <will.deacon@arm.com>

Thanks,

Will

Re: [PATCH v13 2/3] arm64: tlbflush: Introduce __flush_tlb_kernel_pgtable

[Will Deacon]

On Wed, Jun 06, 2018 at 12:31:20PM +0530, Chintan Pandya wrote:
> Add an interface to invalidate intermediate page tables
> from TLB for kernel.
> 
> Signed-off-by: Chintan Pandya <cpandya@codeaurora.org>
> ---
>  arch/arm64/include/asm/tlbflush.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
> index dfc61d7..a4a1901 100644
> --- a/arch/arm64/include/asm/tlbflush.h
> +++ b/arch/arm64/include/asm/tlbflush.h
> @@ -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

Acked-by: Will Deacon <will.deacon@arm.com>

Will

[PATCH v13 2/3] arm64: tlbflush: Introduce __flush_tlb_kernel_pgtable

[Will Deacon]

On Wed, Jun 06, 2018 at 12:31:20PM +0530, Chintan Pandya wrote:
> Add an interface to invalidate intermediate page tables
> from TLB for kernel.
> 
> Signed-off-by: Chintan Pandya <cpandya@codeaurora.org>
> ---
>  arch/arm64/include/asm/tlbflush.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
> index dfc61d7..a4a1901 100644
> --- a/arch/arm64/include/asm/tlbflush.h
> +++ b/arch/arm64/include/asm/tlbflush.h
> @@ -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

Acked-by: Will Deacon <will.deacon@arm.com>

Will

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

[Will Deacon]

On Wed, Jun 06, 2018 at 12:31:21PM +0530, Chintan Pandya wrote:
> 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 <cpandya@codeaurora.org>
> ---
>  arch/arm64/mm/mmu.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 44 insertions(+), 4 deletions(-)

Thanks, I think this looks really good now:

Reviewed-by: Will Deacon <will.deacon@arm.com>

Will

[PATCH v13 3/3] arm64: Implement page table free interfaces

[Will Deacon]

On Wed, Jun 06, 2018 at 12:31:21PM +0530, Chintan Pandya wrote:
> 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 <cpandya@codeaurora.org>
> ---
>  arch/arm64/mm/mmu.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 44 insertions(+), 4 deletions(-)

Thanks, I think this looks really good now:

Reviewed-by: Will Deacon <will.deacon@arm.com>

Will

Re: [PATCH v13 0/3] Fix issues with huge mapping in ioremap for ARM64

[Will Deacon]

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.

On Wed, Jun 06, 2018 at 12:31:18PM +0530, Chintan Pandya wrote:
> 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.

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

>  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

[PATCH v13 0/3] Fix issues with huge mapping in ioremap for ARM64

[Will Deacon]

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.

On Wed, Jun 06, 2018 at 12:31:18PM +0530, Chintan Pandya wrote:
> 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.

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

>  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

Re: [PATCH v13 0/3] Fix issues with huge mapping in ioremap for ARM64

[Chintan Pandya]

On 6/6/2018 9:15 PM, Will Deacon wrote:
> Hi Chintan,
Hi Will,

> 
> 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.
> 

Thanks for all the reviews so far.

> On Wed, Jun 06, 2018 at 12:31:18PM +0530, Chintan Pandya wrote:
>> This series of patches re-bring huge vmap back for arm64.

...

>> 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.

If disabling of huge io mappings have gone into stable trees, then I
won't push for these changes there.

> 
>>   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).

Sure. I would wait for few days before either of them take notice of
this. If required, I will communicated with them.

> 
> Thanks,
> 
> Will
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

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

[PATCH v13 0/3] Fix issues with huge mapping in ioremap for ARM64

[Chintan Pandya]

On 6/6/2018 9:15 PM, Will Deacon wrote:
> Hi Chintan,
Hi Will,

> 
> 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.
> 

Thanks for all the reviews so far.

> On Wed, Jun 06, 2018 at 12:31:18PM +0530, Chintan Pandya wrote:
>> This series of patches re-bring huge vmap back for arm64.

...

>> 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.

If disabling of huge io mappings have gone into stable trees, then I
won't push for these changes there.

> 
>>   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).

Sure. I would wait for few days before either of them take notice of
this. If required, I will communicated with them.

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

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

Re: [PATCH v13 0/3] Fix issues with huge mapping in ioremap for ARM64

[Chintan Pandya]

Hi Andrew,

On 6/6/2018 9:15 PM, Will Deacon wrote:

[...]

> On Wed, Jun 06, 2018 at 12:31:18PM +0530, Chintan Pandya wrote:
>> 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.
>>

[...]

> 
>>   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).
> 

Would you be able to take a look at this series ?
- 1/3 has been reviewed by Will and Toshi (as it touches arm64 and x86).
- 2/3 & 3/3 are arm64 specific changes.

If you think to take these patches, great ! Otherwise, I will try to
reach-out to x86 folks for their ack.

> Thanks,
> 
> Will
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

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

[PATCH v13 0/3] Fix issues with huge mapping in ioremap for ARM64

[Chintan Pandya]

Hi Andrew,

On 6/6/2018 9:15 PM, Will Deacon wrote:

[...]

> On Wed, Jun 06, 2018 at 12:31:18PM +0530, Chintan Pandya wrote:
>> 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.
>>

[...]

> 
>>   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).
> 

Would you be able to take a look at this series ?
- 1/3 has been reviewed by Will and Toshi (as it touches arm64 and x86).
- 2/3 & 3/3 are arm64 specific changes.

If you think to take these patches, great ! Otherwise, I will try to
reach-out to x86 folks for their ack.

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

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

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

[Catalin Marinas]

Hi Chintan,

On Wed, Jun 06, 2018 at 12:31:21PM +0530, Chintan Pandya wrote:
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 8ae5d7a..65f8627 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -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);
> +	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;
	}

Similarly for the pud_free_pmd_page():

----------8<--------------------------

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 65f86271f02b..2662937ef879 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -986,8 +986,8 @@ int pmd_free_pte_page(pmd_t *pmdp, unsigned long addr)
 	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));
+	if (pmd_none(pmd) || !pmd_table(pmd)) {
+		VM_WARN_ON(!pmd_none(pmd));
 		return 1;
 	}
 
@@ -1008,8 +1008,8 @@ int pud_free_pmd_page(pud_t *pudp, unsigned long addr)
 	pud = READ_ONCE(*pudp);
 
 	/* No-op for empty entry and WARN_ON for valid entry */
-	if (!pud_present(pud) || !pud_table(pud)) {
-		VM_WARN_ON(!pud_table(pud));
+	if (pud_none(pud) || !pud_table(pud)) {
+		VM_WARN_ON(!pud_none(pud));
 		return 1;
 	}
 

[PATCH v13 3/3] arm64: Implement page table free interfaces

[Catalin Marinas]

Hi Chintan,

On Wed, Jun 06, 2018 at 12:31:21PM +0530, Chintan Pandya wrote:
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 8ae5d7a..65f8627 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -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);
> +	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;
	}

Similarly for the pud_free_pmd_page():

----------8<--------------------------

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 65f86271f02b..2662937ef879 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -986,8 +986,8 @@ int pmd_free_pte_page(pmd_t *pmdp, unsigned long addr)
 	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));
+	if (pmd_none(pmd) || !pmd_table(pmd)) {
+		VM_WARN_ON(!pmd_none(pmd));
 		return 1;
 	}
 
@@ -1008,8 +1008,8 @@ int pud_free_pmd_page(pud_t *pudp, unsigned long addr)
 	pud = READ_ONCE(*pudp);
 
 	/* No-op for empty entry and WARN_ON for valid entry */
-	if (!pud_present(pud) || !pud_table(pud)) {
-		VM_WARN_ON(!pud_table(pud));
+	if (pud_none(pud) || !pud_table(pud)) {
+		VM_WARN_ON(!pud_none(pud));
 		return 1;
 	}
 

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

[Catalin Marinas]

On Thu, Sep 20, 2018 at 06:25:29PM +0100, Catalin Marinas wrote:
> On Wed, Jun 06, 2018 at 12:31:21PM +0530, Chintan Pandya wrote:
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> > index 8ae5d7a..65f8627 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -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);
> > +	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;
> 	}

Ignore this, fixed in -rc4 (fac880c7d074 "arm64: fix erroneous warnings
in page freeing functions").

-- 
Catalin

[PATCH v13 3/3] arm64: Implement page table free interfaces

[Catalin Marinas]

On Thu, Sep 20, 2018 at 06:25:29PM +0100, Catalin Marinas wrote:
> On Wed, Jun 06, 2018 at 12:31:21PM +0530, Chintan Pandya wrote:
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> > index 8ae5d7a..65f8627 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -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);
> > +	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;
> 	}

Ignore this, fixed in -rc4 (fac880c7d074 "arm64: fix erroneous warnings
in page freeing functions").

-- 
Catalin