Currently set_one_region() implicitly assumes that we want to map a
region and aggressively splits blocks into tables to do this, but when
called with PTE_TYPE_FAULT to unmap a currently mapped region it may
try to unnecessarily split blocks which doesn't make sense if the entire
block should actually be unmapped. In the end it then has to walk every
single page and create a bunch of empty tables.

Introduce a check for this kind of behaviour and optimise with a fast
path, if we're unmapping a region >= the size of this entry then we can
just unmap the entire PTE and whatever it contains.

This fixes some bogus empty tables being left behind when carving out
reserved memory regions on Qualcomm, and should improve the performance
of the break-before-make in mmu_change_region_attr().

Signed-off-by: Casey Connolly <[email protected]>
---
Changes in v2:
 - Update pte_type() to correctly check the PTE_TYPE_VALID bit
 - Explicitly unset both bits of PTE_TYPE_MASK to be extra safe
 - V1: 
https://lore.kernel.org/u-boot/[email protected]/
---
 arch/arm/cpu/armv8/cache_v8.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
index 39479df7b21f..b6506e8523be 100644
--- a/arch/arm/cpu/armv8/cache_v8.c
+++ b/arch/arm/cpu/armv8/cache_v8.c
@@ -162,9 +162,9 @@ u64 get_tcr(u64 *pips, u64 *pva_bits)
 #define MAX_PTE_ENTRIES 512
 
 static int pte_type(u64 *pte)
 {
-       return *pte & PTE_TYPE_MASK;
+       return *pte & PTE_TYPE_VALID ? *pte & PTE_TYPE_MASK : PTE_TYPE_FAULT;
 }
 
 /* Returns the LSB number for a PTE on level <level> */
 static int level2shift(int level)
@@ -939,8 +939,20 @@ static u64 set_one_region(u64 start, u64 size, u64 attrs, 
bool flag, int level)
        int levelshift = level2shift(level);
        u64 levelsize = 1ULL << levelshift;
        u64 *pte = find_pte(start, level);
 
+       /*
+        * If we're trying to unmap a region then check if it's already 
unmapped or if it's bigger
+        * then the PTE we're looking at right now, in the first case we can do 
nothing and in the
+        * second case we just need to unmap this page/block.
+        * Otherwise we will needlessly create new tables until we have 
traversed every single page
+        * in the region.
+        */
+       if (attrs == PTE_TYPE_FAULT && (pte_type(pte) == PTE_TYPE_FAULT || size 
>= levelsize)) {
+               *pte &= ~(PMD_ATTRMASK | PTE_TYPE_MASK);
+               return levelsize;
+       }
+
        /* Can we can just modify the current level block PTE? */
        if (is_aligned(start, size, levelsize)) {
                if (flag) {
                        *pte &= ~PMD_ATTRMASK;
@@ -1081,8 +1093,12 @@ void mmu_change_region_attr(phys_addr_t addr, size_t 
siz, u64 attrs)
        flush_dcache_range(gd->arch.tlb_addr,
                           gd->arch.tlb_addr + gd->arch.tlb_size);
        __asm_invalidate_tlb_all();
 
+       /* If we were unmapping a region then we're done! */
+       if (attrs == PTE_TYPE_FAULT)
+               return;
+
        mmu_change_region_attr_nobreak(addr, siz, attrs);
 }
 
 int pgprot_set_attrs(phys_addr_t addr, size_t size, enum pgprot_attrs perm)
-- 
2.53.0

Reply via email to