[PATCH v4] ARM: LPAE: Fix mapping in alloc_init_pte for unaligned addresses

[Catalin Marinas]

On Tue, Jan 29, 2013 at 03:07:16PM +0000, R Sricharan wrote:
> With LPAE enabled, alloc_init_section() does not map the
> entire address space for unaligned addresses.
> 
> The issue also reproduced with CMA + LPAE. CMA tries to map 16MB
> with page granularity mappings during boot. alloc_init_pte()
> is called and out of 16MB, only 2MB gets mapped and rest remains
> unaccessible.
> 
> Because of this OMAP5 boot is broken with CMA + LPAE enabled.
> Fix the issue by ensuring that the entire addresses are
> mapped.
> 
> Signed-off-by: R Sricharan <r.sricharan at ti.com>
> Cc: Catalin Marinas <catalin.marinas at arm.com>
> Cc: Christoffer Dall <chris at cloudcar.com>
> Cc: Russell King <linux at arm.linux.org.uk>
> Acked-by: Santosh Shilimkar <santosh.shilimkar at ti.com>
> Tested-by: Christoffer Dall  <chris at cloudcar.com>
> ---
>  [V2] Moved the loop to alloc_init_pte as per Russell's
>      feedback and changed the subject accordingly.
>      Using PMD_XXX instead of SECTION_XXX to avoid
>      different loop increments with/without LPAE.
> 
>  [v3] Removed the dummy variable phys and updated
>       the commit log for CMA case.
> 
>  [v4] Resending with updated change log and 
>       updating the tags.
> 
>  arch/arm/mm/mmu.c |   20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index f8388ad..b94c313 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -569,11 +569,23 @@ static void __init alloc_init_pte(pmd_t *pmd, unsigned long addr,
>  				  unsigned long end, unsigned long pfn,
>  				  const struct mem_type *type)
>  {
> -	pte_t *pte = early_pte_alloc(pmd, addr, type->prot_l1);
> +	unsigned long next;
> +	pte_t *pte;
> +
>  	do {
> -		set_pte_ext(pte, pfn_pte(pfn, __pgprot(type->prot_pte)), 0);
> -		pfn++;
> -	} while (pte++, addr += PAGE_SIZE, addr != end);
> +		if ((end-addr) & PMD_MASK)
> +			next = (addr + PMD_SIZE) & PMD_MASK;
> +		else
> +			next = end;

Can use pmd_addr_end(addr, end) here?

> +		pte = early_pte_alloc(pmd, addr, type->prot_l1);
> +		do {
> +			set_pte_ext(pte, pfn_pte(pfn,
> +					__pgprot(type->prot_pte)), 0);
> +			pfn++;
> +		} while (pte++, addr += PAGE_SIZE, addr != next);
> +
> +	} while (pmd++, addr = next, addr != end);

I would actually keep the loop in alloc_init_section(). There is even a
comment in there saying "no need to loop" but you actually moved the
loop in alloc_init_pte().

I'm proposing a simpler patch below (only lightly tested on VE/C-A9).
The only difference is that we do more flush_pmd_entry() calls but I'm
not really bothered, it's during boot and you won't notice.


diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index 9c82f98..eaa8ba8 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -205,6 +205,11 @@ static inline pte_t *pmd_page_vaddr(pmd_t pmd)
 
 #define pte_present_user(pte)  (pte_present(pte) && (pte_val(pte) & L_PTE_USER))
 
+#define section_addr_end(addr, end)						\
+({	unsigned long __boundary = ((addr) + SECTION_SIZE) & SECTION_MASK;	\
+	(__boundary - 1 < (end) - 1)? __boundary: (end);			\
+})
+
 #if __LINUX_ARM_ARCH__ < 6
 static inline void __sync_icache_dcache(pte_t pteval)
 {
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 9f06102..0d0faed 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -581,34 +581,19 @@ static void __init alloc_init_section(pud_t *pud, unsigned long addr,
 				      const struct mem_type *type)
 {
 	pmd_t *pmd = pmd_offset(pud, addr);
+	unsigned long next;
 
-	/*
-	 * Try a section mapping - end, addr and phys must all be aligned
-	 * to a section boundary.  Note that PMDs refer to the individual
-	 * L1 entries, whereas PGDs refer to a group of L1 entries making
-	 * up one logical pointer to an L2 table.
-	 */
-	if (type->prot_sect && ((addr | end | phys) & ~SECTION_MASK) == 0) {
-		pmd_t *p = pmd;
-
-#ifndef CONFIG_ARM_LPAE
-		if (addr & SECTION_SIZE)
-			pmd++;
-#endif
-
-		do {
+	do {
+		next = section_addr_end(addr, end);
+		/* try section mapping first */
+		if (((addr | next | phys) & ~SECTION_MASK) == 0) {
 			*pmd = __pmd(phys | type->prot_sect);
-			phys += SECTION_SIZE;
-		} while (pmd++, addr += SECTION_SIZE, addr != end);
-
-		flush_pmd_entry(p);
-	} else {
-		/*
-		 * No need to loop; pte's aren't interested in the
-		 * individual L1 entries.
-		 */
-		alloc_init_pte(pmd, addr, end, __phys_to_pfn(phys), type);
-	}
+			flush_pmd_entry(pmd);
+		} else {
+			alloc_init_pte(pmd, addr, next, __phys_to_pfn(phys), type);
+		}
+		phys += next - addr;
+	} while (pmd++, addr = next, addr != end);
 }
 
 static void __init alloc_init_pud(pgd_t *pgd, unsigned long addr,