Thread (16 messages) 16 messages, 5 authors, 2018-07-31

Lifecycle

  1. Posted robin.murphy@arm.com (Robin Murphy)

[PATCH v5 1/3] arm64: mm: Support Common Not Private translations

From: robin.murphy@arm.com (Robin Murphy)
Date: 2018-07-30 16:29:35

On 30/07/18 16:42, Catalin Marinas wrote:
On Mon, Jul 30, 2018 at 11:08:27AM +0100, Vladimir Murzin wrote:
quoted
On 27/07/18 12:35, Catalin Marinas wrote:
quoted
On Tue, Jun 19, 2018 at 11:18:20AM +0100, Vladimir Murzin wrote:
quoted
diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index 39ec0b8..c506fb7 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -149,6 +149,18 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp)
  
  	phys_addr_t pgd_phys = virt_to_phys(pgdp);
  
+	if (system_supports_cnp() && !WARN_ON(pgdp != lm_alias(swapper_pg_dir))) {
+		/*
+		 * cpu_replace_ttbr1() is used when there's a boot CPU
+		 * up (i.e. cpufeature framework is not up yet) and
+		 * latter only when we enable CNP via cpufeature's
+		 * enable() callback.
+		 * Also we rely on the cpu_hwcap bit being set before
+		 * calling the enable() function.
+		 */
+		pgd_phys |= TTBR_CNP_BIT;
+	}
+
  	replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1);
So the above code sets the TTBR_CNP_BIT (bit 0) in pgd_phys and calls
the idmap_cpu_replace_ttbr1() with this value. Looking at the latter, it
performs a phys_to_ttbr transformation of pgd_phys which masks out the
bottom 2 bits when CONFIG_ARM64_PA_BITS_52 is enabled. I think we need
to tweak TTBR_BADDR_MASK_52 to start from bit 0.
Something like bellow?
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index 0bcc98d..e0b4b2f 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -524,11 +524,10 @@ USER(\label, ic	ivau, \tmp2)			// invalidate I line PoU
   * 	ttbr:	returns the TTBR value
   */
  	.macro	phys_to_ttbr, ttbr, phys
-#ifdef CONFIG_ARM64_PA_BITS_52
-	orr	\ttbr, \phys, \phys, lsr #46
-	and	\ttbr, \ttbr, #TTBR_BADDR_MASK_52
-#else
  	mov	\ttbr, \phys
+#ifdef CONFIG_ARM64_PA_BITS_52
+	ubfx	\ttbr, \ttbr, #48, #4
+	orr	\ttbr, \phys, \ttbr, lsl #2
  #endif
  	.endm
This would do, I don't have a better idea on how to write it. But I'd
like a comment to say that this is moving bits 51:48 of the address to
bits 5:2 of TTBR_ELx.
The diff above is *copying* 51:48 into 5:2; it doesn't *move* it like 
the existing code does. That's pretty crucial, because without the BADDR 
masking operation it's going to leave the upper address bits in the 
ASID/VMID field, which looks like a recipe for disaster if the reserved 
TTBR1 happens to be at a high enough address (at least cpu_switch_mm 
might just be OK by virtue of using BFI rather than ORR for the ASID).
quoted
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 1bdeca8..1b9d0e9 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -770,7 +770,7 @@ static inline void update_mmu_cache(struct vm_area_struct *vma,
  #define kc_offset_to_vaddr(o)	((o) | VA_START)
  
  #ifdef CONFIG_ARM64_PA_BITS_52
-#define phys_to_ttbr(addr)	(((addr) | ((addr) >> 46)) & TTBR_BADDR_MASK_52)
+#define phys_to_ttbr(addr)	((addr) | (((addr) >> 46) & TTBR_BADDR_MASK_52))
Ditto - by the look of things, this definitely stands to corrupt the 
VMID in update_vttbr(). If we really have no choice but to smuggle the 
CnP value in something which is logically an address, then I think we'd 
need to handle it more like this:

#define TTBR_CNP (1)
#define phys_to_ttbr(addr) \
((((addr) | ((addr) >> 46)) & TTBR_BADDR_MASK_52) | (addr & TTBR_CNP))

Although in patch #2 we're only applying CnP *after* the BADDR 
conversion, so is changing this one even necessary? If I've understood 
the intent correctly, all that might be needed is something like the 
below (untested, of course).

Robin.

----->8-----
diff --git a/arch/arm64/include/asm/assembler.h 
b/arch/arm64/include/asm/assembler.h
index 0bcc98dbba56..5b0f8f80238f 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -527,6 +527,7 @@ USER(\label, ic	ivau, \tmp2)			// invalidate I line PoU
  #ifdef CONFIG_ARM64_PA_BITS_52
  	orr	\ttbr, \phys, \phys, lsr #46
  	and	\ttbr, \ttbr, #TTBR_BADDR_MASK_52
+	bfi	\ttbr, \addr, #0, #1	// CnP
  #else
  	mov	\ttbr, \phys
  #endif
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help