[v2,1/3] arm64: mm: Support Common Not Private translations
diff mbox

Message ID 1507724395-13735-2-git-send-email-vladimir.murzin@arm.com
State New, archived
Headers show

Commit Message

Vladimir Murzin Oct. 11, 2017, 12:19 p.m. UTC
Common Not Private (CNP) is a feature of ARMv8.2 extension which
allows translation table entries to be shared between different PEs in
the same inner shareable domain, so the hardware can use this fact to
optimise the caching of such entries in the TLB.

CNP occupies one bit in TTBRx_ELy and VTTBR_EL2, which advertises to
the hardware that the translation table entries pointed to by this
TTBR are the same as every PE in the same inner shareable domain for
which the equivalent TTBR also has CNP bit set. In case CNP bit is set
but TTBR does not point at the same translation table entries or a
given ASID and VMID, then the system is mis-configured, so the results
of translations are UNPREDICTABLE.

This patch adds support for Common Not Private translations on
different exceptions levels:

(1) For EL0 there are a few cases we need to care of changes in
    TTBR0_EL1:
    - a switch to idmap
    - software emulated PAN
    we rule out latter via Kconfig options and for the former we make
    sure that CNP is set for non-zero ASIDs only.

(2) For EL1 we postpone setting CNP till all cpus are up and rely on
    cpufeature framework to 1) patch the code which is sensitive to
    CNP and 2) update TTBR1_EL1 with CNP bit set. TTBR1_EL1 can be
    reprogrammed as result of hibernation or cpuidle (via __enable_mmu).
    cpuidle's path has been changed to restore CnP and for hibernation
    the code has been changed to save raw TTBR1_EL1 and blindly restore
    it on resume.

Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
---
 arch/arm64/Kconfig                     | 14 ++++++++++++++
 arch/arm64/include/asm/cpucaps.h       |  3 ++-
 arch/arm64/include/asm/cpufeature.h    |  6 ++++++
 arch/arm64/include/asm/mmu_context.h   | 12 ++++++++++++
 arch/arm64/include/asm/pgtable-hwdef.h |  2 ++
 arch/arm64/kernel/cpufeature.c         | 19 +++++++++++++++++++
 arch/arm64/kernel/hibernate.c          |  2 +-
 arch/arm64/kernel/suspend.c            |  3 +++
 arch/arm64/mm/proc.S                   |  5 +++++
 9 files changed, 64 insertions(+), 2 deletions(-)

Comments

James Morse Oct. 18, 2017, 3 p.m. UTC | #1
Hi Vladimir,

On 11/10/17 13:19, Vladimir Murzin wrote:
> Common Not Private (CNP) is a feature of ARMv8.2 extension which
> allows translation table entries to be shared between different PEs in
> the same inner shareable domain, so the hardware can use this fact to
> optimise the caching of such entries in the TLB.
> 
> CNP occupies one bit in TTBRx_ELy and VTTBR_EL2, which advertises to
> the hardware that the translation table entries pointed to by this
> TTBR are the same as every PE in the same inner shareable domain for
> which the equivalent TTBR also has CNP bit set. In case CNP bit is set
> but TTBR does not point at the same translation table entries or a
> given ASID and VMID, then the system is mis-configured, so the results
> of translations are UNPREDICTABLE.
> 
> This patch adds support for Common Not Private translations on
> different exceptions levels:
> 
> (1) For EL0 there are a few cases we need to care of changes in
>     TTBR0_EL1:
>     - a switch to idmap
>     - software emulated PAN
>     we rule out latter via Kconfig options and for the former we make
>     sure that CNP is set for non-zero ASIDs only.

I've been looking at how CNP interacts with the asid allocator. I think we
depend on a subtlety that wasn't obvious to me at first.

Can you check I'm reading this properly:
The ARM-ARM's 'D4.8.1 Use of ASIDs and VMIDs to reduce TLB maintenance
requirements' reads as if you can only share a TLB entry if both CPUs are using
that ASID at the same time:

> When the value of a TTBR_ELx.CnP field is 1,

(on CPU-A)

> translation table entries pointed to by that TTBR_ELx are shared with all
> other PEs in the Inner Shareable domain for which the following conditions
> are met:

>         The corresponding TTBR_ELx.CnP field has the value 1.

(CPU-B's corresponding TTBR right?)

This would suggest CPU-A stops sharing its TLB entries for an asid when it
changes asid by scheduling a new task. A single-threaded task would never
benefit from CNP.


We will depend on this behaviour when we re-use an asid that was previously used
on a remote CPU that hasn't yet noticed the rollover and invalidated its TLB.


> diff --git a/arch/arm64/kernel/suspend.c b/arch/arm64/kernel/suspend.c
> index 1e3be90..f28c44a 100644
> --- a/arch/arm64/kernel/suspend.c
> +++ b/arch/arm64/kernel/suspend.c
> @@ -46,6 +46,9 @@ void notrace __cpu_suspend_exit(void)
>  	 */
>  	cpu_uninstall_idmap();


> +	/* Restore CnP bit in TTBR1_EL1 */
> +	cpu_replace_ttbr1(lm_alias(swapper_pg_dir));

Could you wrap this in system_supports_cnp(). Otherwise it replaces ttbr1
unnecessarily.

This function is called with the idmap loaded, it seems unnecessary to remove it
twice. You could refactor cpu_replace_ttbr1() to have a __version that is called
with the idmap loaded, then call that before the cpu_uninstall_idmap() above.


Thanks,

James
James Morse Dec. 13, 2017, 2:19 p.m. UTC | #2
Hi Vladimir,

On 11/10/17 13:19, Vladimir Murzin wrote:
> Common Not Private (CNP) is a feature of ARMv8.2 extension which
> allows translation table entries to be shared between different PEs in
> the same inner shareable domain, so the hardware can use this fact to
> optimise the caching of such entries in the TLB.
> 
> CNP occupies one bit in TTBRx_ELy and VTTBR_EL2, which advertises to
> the hardware that the translation table entries pointed to by this
> TTBR are the same as every PE in the same inner shareable domain for
> which the equivalent TTBR also has CNP bit set. In case CNP bit is set
> but TTBR does not point at the same translation table entries or a
> given ASID and VMID, then the system is mis-configured, so the results
> of translations are UNPREDICTABLE.
> 
> This patch adds support for Common Not Private translations on
> different exceptions levels:
> 
> (1) For EL0 there are a few cases we need to care of changes in
>     TTBR0_EL1:
>     - a switch to idmap
>     - software emulated PAN
>     we rule out latter via Kconfig options and for the former we make
>     sure that CNP is set for non-zero ASIDs only.
> 
> (2) For EL1 we postpone setting CNP till all cpus are up and rely on
>     cpufeature framework to 1) patch the code which is sensitive to
>     CNP and 2) update TTBR1_EL1 with CNP bit set. TTBR1_EL1 can be
>     reprogrammed as result of hibernation or cpuidle (via __enable_mmu).
>     cpuidle's path has been changed to restore CnP and for hibernation
>     the code has been changed to save raw TTBR1_EL1 and blindly restore
>     it on resume.


While I remember:

This feature is going to be fun for kdump, we may leave secondary CPUs running
if they don't take the IPI to crash-out of the kernel. Worse, if we don't have
PSCI they just spin in a loop while the surviving CPU brings up the crash kernel
and maybe-enables CNP...

I think the best fix for this is to refuse to enable CNP at all if we're a crash
kernel. There is stuff in the DT to indicate this... we should know about the
'elfcorehdr' before cpufeature runs. (I don't think we should rely on the
cmdline option).

kexec is unaffected because it always powers-off the secondary CPUs before
leaving the old kernel. This behaves much more like a normal boot.


Thanks,

James
Vladimir Murzin Dec. 13, 2017, 4:59 p.m. UTC | #3
Hi James,

On 13/12/17 14:19, James Morse wrote:
> Hi Vladimir,
> 
> On 11/10/17 13:19, Vladimir Murzin wrote:
>> Common Not Private (CNP) is a feature of ARMv8.2 extension which
>> allows translation table entries to be shared between different PEs in
>> the same inner shareable domain, so the hardware can use this fact to
>> optimise the caching of such entries in the TLB.
>>
>> CNP occupies one bit in TTBRx_ELy and VTTBR_EL2, which advertises to
>> the hardware that the translation table entries pointed to by this
>> TTBR are the same as every PE in the same inner shareable domain for
>> which the equivalent TTBR also has CNP bit set. In case CNP bit is set
>> but TTBR does not point at the same translation table entries or a
>> given ASID and VMID, then the system is mis-configured, so the results
>> of translations are UNPREDICTABLE.
>>
>> This patch adds support for Common Not Private translations on
>> different exceptions levels:
>>
>> (1) For EL0 there are a few cases we need to care of changes in
>>     TTBR0_EL1:
>>     - a switch to idmap
>>     - software emulated PAN
>>     we rule out latter via Kconfig options and for the former we make
>>     sure that CNP is set for non-zero ASIDs only.
>>
>> (2) For EL1 we postpone setting CNP till all cpus are up and rely on
>>     cpufeature framework to 1) patch the code which is sensitive to
>>     CNP and 2) update TTBR1_EL1 with CNP bit set. TTBR1_EL1 can be
>>     reprogrammed as result of hibernation or cpuidle (via __enable_mmu).
>>     cpuidle's path has been changed to restore CnP and for hibernation
>>     the code has been changed to save raw TTBR1_EL1 and blindly restore
>>     it on resume.
> 
> 
> While I remember:
> 
> This feature is going to be fun for kdump, we may leave secondary CPUs running
> if they don't take the IPI to crash-out of the kernel. Worse, if we don't have
> PSCI they just spin in a loop while the surviving CPU brings up the crash kernel
> and maybe-enables CNP...
> 
> I think the best fix for this is to refuse to enable CNP at all if we're a crash
> kernel. There is stuff in the DT to indicate this... we should know about the
> 'elfcorehdr' before cpufeature runs. (I don't think we should rely on the
> cmdline option).
> 
> kexec is unaffected because it always powers-off the secondary CPUs before
> leaving the old kernel. This behaves much more like a normal boot.

Thanks, I'll look into it.

Vladimir 

> 
> 
> Thanks,
> 
> James
>

Patch
diff mbox

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 0df64a6..c8ab280 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -973,6 +973,20 @@  config ARM64_PMEM
 	  operations if DC CVAP is not supported (following the behaviour of
 	  DC CVAP itself if the system does not define a point of persistence).
 
+config ARM64_CNP
+	bool "Enable support for Common Not Private (CNP) translations"
+	depends on ARM64_PAN || !ARM64_SW_TTBR0_PAN
+	default y
+	help
+	  Common Not Private (CNP) allows translation table entries to
+	  be shared between different PEs in the same inner shareable
+	  domain, so the hardware can use this fact to optimise the
+	  caching of such entries in the TLB.
+
+	  Selecting this option allows the CNP feature to be detected
+	  at runtime, and does not affect PEs that do not implement
+	  this feature.
+
 endmenu
 
 config ARM64_MODULE_CMODEL_LARGE
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index 8da6216..9b7d94c 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -40,7 +40,8 @@ 
 #define ARM64_WORKAROUND_858921			19
 #define ARM64_WORKAROUND_CAVIUM_30115		20
 #define ARM64_HAS_DCPOP				21
+#define ARM64_HAS_CNP				22
 
-#define ARM64_NCAPS				22
+#define ARM64_NCAPS				23
 
 #endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 428ee1f..f7c75ac 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -262,6 +262,12 @@  static inline bool system_uses_ttbr0_pan(void)
 		!cpus_have_const_cap(ARM64_HAS_PAN);
 }
 
+static inline bool system_supports_cnp(void)
+{
+	return IS_ENABLED(CONFIG_ARM64_CNP) &&
+		cpus_have_const_cap(ARM64_HAS_CNP);
+}
+
 #endif /* __ASSEMBLY__ */
 
 #endif
diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index 3257895a..8e78a51 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -135,6 +135,18 @@  static inline void cpu_replace_ttbr1(pgd_t *pgd)
 
 	phys_addr_t pgd_phys = virt_to_phys(pgd);
 
+	if (system_supports_cnp() && !WARN_ON(pgd != 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);
 
 	cpu_install_idmap();
diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
index eb0c2bd..59247e7 100644
--- a/arch/arm64/include/asm/pgtable-hwdef.h
+++ b/arch/arm64/include/asm/pgtable-hwdef.h
@@ -199,6 +199,8 @@ 
 #define PHYS_MASK_SHIFT		(48)
 #define PHYS_MASK		((UL(1) << PHYS_MASK_SHIFT) - 1)
 
+#define TTBR_CNP_BIT		(UL(1) << 0)
+
 /*
  * TCR flags.
  */
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 21e2c95..83809a8 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -101,6 +101,7 @@  EXPORT_SYMBOL(cpu_hwcap_keys);
 static bool __maybe_unused
 cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused);
 
+static int cpu_enable_cnp(void *__unused);
 
 /*
  * NOTE: Any changes to the visibility of features should be kept in
@@ -898,6 +899,18 @@  static const struct arm64_cpu_capabilities arm64_features[] = {
 		.sys_reg = SYS_ID_AA64ISAR1_EL1,
 		.field_pos = ID_AA64ISAR1_DPB_SHIFT,
 		.min_field_value = 1,
+#endif
+#ifdef CONFIG_ARM64_CNP
+	{
+		.desc = "Common not Private translations",
+		.capability = ARM64_HAS_CNP,
+		.def_scope = SCOPE_SYSTEM,
+		.matches = has_cpuid_feature,
+		.sys_reg = SYS_ID_AA64MMFR2_EL1,
+		.sign = FTR_UNSIGNED,
+		.field_pos = ID_AA64MMFR2_CNP_SHIFT,
+		.min_field_value = 1,
+		.enable = cpu_enable_cnp,
 	},
 #endif
 	{},
@@ -1211,6 +1224,12 @@  cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused)
 	return (cpus_have_const_cap(ARM64_HAS_PAN) && !cpus_have_const_cap(ARM64_HAS_UAO));
 }
 
+static int __maybe_unused cpu_enable_cnp(void *__unused)
+{
+	cpu_replace_ttbr1(lm_alias(swapper_pg_dir));
+	return 0;
+}
+
 /*
  * We emulate only the following system register space.
  * Op0 = 0x3, CRn = 0x0, Op1 = 0x0, CRm = [0, 4 - 7]
diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index 095d3c1..1d056f3 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c
@@ -124,7 +124,7 @@  int arch_hibernation_header_save(void *addr, unsigned int max_size)
 		return -EOVERFLOW;
 
 	arch_hdr_invariants(&hdr->invariants);
-	hdr->ttbr1_el1		= __pa_symbol(swapper_pg_dir);
+	hdr->ttbr1_el1		= read_sysreg(ttbr1_el1);
 	hdr->reenter_kernel	= _cpu_resume;
 
 	/* We can't use __hyp_get_vectors() because kvm may still be loaded */
diff --git a/arch/arm64/kernel/suspend.c b/arch/arm64/kernel/suspend.c
index 1e3be90..f28c44a 100644
--- a/arch/arm64/kernel/suspend.c
+++ b/arch/arm64/kernel/suspend.c
@@ -46,6 +46,9 @@  void notrace __cpu_suspend_exit(void)
 	 */
 	cpu_uninstall_idmap();
 
+	/* Restore CnP bit in TTBR1_EL1 */
+	cpu_replace_ttbr1(lm_alias(swapper_pg_dir));
+
 	/*
 	 * PSTATE was not saved over suspend/resume, re-enable any detected
 	 * features that might not have been set correctly.
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 877d42f..d5b9f6b 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -141,6 +141,11 @@  ENTRY(cpu_do_switch_mm)
 	pre_ttbr0_update_workaround x0, x2, x3
 	mmid	x1, x1				// get mm->context.id
 	bfi	x0, x1, #48, #16		// set the ASID
+alternative_if ARM64_HAS_CNP
+	cbz	x1, 1f                          // skip CNP for ASID == 0
+	orr	x0, x0, #TTBR_CNP_BIT
+1:
+alternative_else_nop_endif
 	msr	ttbr0_el1, x0			// set TTBR0
 	isb
 	post_ttbr0_update_workaround