On Mon, Jul 30, 2018 at 05:29:35PM +0100, Robin Murphy wrote:

On 30/07/18 16:42, Catalin Marinas wrote:

quoted

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

Oh, very good point.

quoted

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

Or we could keep phys_to_ttbr() as it is for both the asm and C and
apply the CNP bit afterwards (as you've already noticed in patch 2). For
cpu_replace_ttbr1(), we'd also need to change idmap_cpu_replace_ttbr1 to
actually take the TTBR1_EL1 value directly so that it doesn't have to
invoke phys_to_ttbr().

-- 
Catalin