[PATCH v3 0/3] Cortex-A77 erratum 1508412 workaround

[PATCH v3 0/3] Cortex-A77 erratum 1508412 workaround

[Rob Herring]

This series implements the work-around for Cortex-A77 erratum 1508412.
KVM guests which don't implement the workaround can still deadlock the
system. This is also the case with the existing Cortex-A57 erratum 832075,
so we add a warning message if an erratum can cause deadlock.

Changes detailed in patches.

Rob

v2: https://lore.kernel.org/linux-arm-kernel/20200701215308.3715856-1-robh@kernel.org/
v1: https://lore.kernel.org/linux-arm-kernel/20200629213321.2953022-1-robh@kernel.org/

Rob Herring (3):
  KVM: arm64: Print warning when cpu erratum can cause guests to
    deadlock
  arm64: Add part number for Arm Cortex-A77
  arm64: Add workaround for Arm Cortex-A77 erratum 1508412

 Documentation/arm64/silicon-errata.rst |  2 ++
 arch/arm64/Kconfig                     | 19 +++++++++++++++++++
 arch/arm64/include/asm/cpucaps.h       |  3 ++-
 arch/arm64/include/asm/cputype.h       |  2 ++
 arch/arm64/include/asm/kvm_hyp.h       | 11 +++++++++++
 arch/arm64/kernel/cpu_errata.c         | 10 ++++++++++
 arch/arm64/kvm/arm.c                   |  5 +++++
 arch/arm64/kvm/hyp/switch.c            |  7 ++++---
 arch/arm64/kvm/hyp/sysreg-sr.c         |  2 +-
 arch/arm64/kvm/sys_regs.c              |  8 +++++++-
 arch/arm64/mm/fault.c                  | 10 ++++++++++
 11 files changed, 73 insertions(+), 6 deletions(-)

--
2.25.1
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v3 1/3] KVM: arm64: Print warning when cpu erratum can cause guests to deadlock

[Rob Herring]

If guests don't have certain CPU erratum workarounds implemented, then
there is a possibility a guest can deadlock the system. IOW, only trusted
guests should be used on systems with the erratum.

This is the case for Cortex-A57 erratum 832075.

Cc: Marc Zyngier <maz@kernel.org>
Cc: James Morse <james.morse@arm.com>
Cc: Julien Thierry <julien.thierry.kdev@gmail.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: kvmarm@lists.cs.columbia.edu
Signed-off-by: Rob Herring <robh@kernel.org>
---
v3:
 - s/work-arounds/workarounds/

 arch/arm64/kvm/arm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 90cb90561446..9b070b5e212b 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1653,6 +1653,10 @@ int kvm_arch_init(void *opaque)
 		return -ENODEV;
 	}

+	if (cpus_have_const_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE))
+		kvm_info("Guests without required CPU erratum workarounds can deadlock system!\n" \
+			 "Only trusted guests should be used on this system.\n");
+
 	for_each_online_cpu(cpu) {
 		smp_call_function_single(cpu, check_kvm_target_cpu, &ret, 1);
 		if (ret < 0) {
--
2.25.1
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v3 2/3] arm64: Add part number for Arm Cortex-A77

[Rob Herring]

Add the MIDR part number info for the Arm Cortex-A77.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Rob Herring <robh@kernel.org>
---
 arch/arm64/include/asm/cputype.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index a87a93f67671..7a2d3c336579 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -71,6 +71,7 @@
 #define ARM_CPU_PART_CORTEX_A55		0xD05
 #define ARM_CPU_PART_CORTEX_A76		0xD0B
 #define ARM_CPU_PART_NEOVERSE_N1	0xD0C
+#define ARM_CPU_PART_CORTEX_A77		0xD0D
 
 #define APM_CPU_PART_POTENZA		0x000
 
@@ -104,6 +105,7 @@
 #define MIDR_CORTEX_A55 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A55)
 #define MIDR_CORTEX_A76	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A76)
 #define MIDR_NEOVERSE_N1 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_NEOVERSE_N1)
+#define MIDR_CORTEX_A77	MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A77)
 #define MIDR_THUNDERX	MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
 #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
 #define MIDR_THUNDERX_83XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_83XX)
-- 
2.25.1

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH v3 3/3] arm64: Add workaround for Arm Cortex-A77 erratum 1508412

[Rob Herring]

On Cortex-A77 r0p0 and r1p0, a sequence of a non-cacheable or device load
and a store exclusive or PAR_EL1 read can cause a deadlock.

The workaround requires a DMB SY before and after a PAR_EL1 register read.
A deadlock is still possible with the workaround as KVM guests must also
have the workaround. IOW, a malicious guest can deadlock an affected
systems.

This workaround also depends on a firmware counterpart to enable the h/w
to insert DMB SY after load and store exclusive instructions. See the
errata document SDEN-1152370 v10 [1] for more information.

[1] https://static.docs.arm.com/101992/0010/Arm_Cortex_A77_MP074_Software_Developer_Errata_Notice_v10.pdf

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Julien Thierry <julien.thierry.kdev@gmail.com>
Cc: kvmarm@lists.cs.columbia.edu
Signed-off-by: Rob Herring <robh@kernel.org>
---
v3:
- Add dmbs around PAR reads in KVM code
- Clean-up 'work-around' and 'errata'

v2:
- Don't disable KVM, just print warning
---
 Documentation/arm64/silicon-errata.rst |  2 ++
 arch/arm64/Kconfig                     | 19 +++++++++++++++++++
 arch/arm64/include/asm/cpucaps.h       |  3 ++-
 arch/arm64/include/asm/kvm_hyp.h       | 11 +++++++++++
 arch/arm64/kernel/cpu_errata.c         | 10 ++++++++++
 arch/arm64/kvm/arm.c                   |  3 ++-
 arch/arm64/kvm/hyp/switch.c            |  7 ++++---
 arch/arm64/kvm/hyp/sysreg-sr.c         |  2 +-
 arch/arm64/kvm/sys_regs.c              |  8 +++++++-
 arch/arm64/mm/fault.c                  | 10 ++++++++++
 10 files changed, 68 insertions(+), 7 deletions(-)

diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
index 936cf2a59ca4..716b279e3b33 100644
--- a/Documentation/arm64/silicon-errata.rst
+++ b/Documentation/arm64/silicon-errata.rst
@@ -90,6 +90,8 @@ stable kernels.
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A76      | #1463225        | ARM64_ERRATUM_1463225       |
 +----------------+-----------------+-----------------+-----------------------------+
+| ARM            | Cortex-A77      | #1508412        | ARM64_ERRATUM_1508412       |
++----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Neoverse-N1     | #1188873,1418040| ARM64_ERRATUM_1418040       |
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Neoverse-N1     | #1349291        | N/A                         |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index a4a094bedcb2..6638444ce0d8 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -626,6 +626,25 @@ config ARM64_ERRATUM_1542419
 
 	  If unsure, say Y.
 
+config ARM64_ERRATUM_1508412
+	bool "Cortex-A77: 1508412: workaround deadlock on sequence of NC/Device load and store exclusive or PAR read"
+	default y
+	help
+	  This option adds a workaround for Arm Cortex-A77 erratum 1508412.
+
+	  Affected Cortex-A77 cores (r0p0, r1p0) could deadlock on a sequence
+	  of a store-exclusive or read of PAR_EL1 and a load with device or
+	  non-cacheable memory attributes. The workaround depends on a firmware
+	  counterpart.
+
+	  KVM guests must also have the workaround implemented or they can
+	  deadlock the system.
+
+	  Work around the issue by inserting DMB SY barriers around PAR_EL1
+	  register reads and warning KVM users.
+
+	  If unsure, say Y.
+
 config CAVIUM_ERRATUM_22375
 	bool "Cavium erratum 22375, 24313"
 	default y
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index d7b3bb0cb180..2a2cdb4ced8b 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -62,7 +62,8 @@
 #define ARM64_HAS_GENERIC_AUTH			52
 #define ARM64_HAS_32BIT_EL1			53
 #define ARM64_BTI				54
+#define ARM64_WORKAROUND_1508412		55
 
-#define ARM64_NCAPS				55
+#define ARM64_NCAPS				56
 
 #endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
index ce3080834bfa..ce5b0d9b12bf 100644
--- a/arch/arm64/include/asm/kvm_hyp.h
+++ b/arch/arm64/include/asm/kvm_hyp.h
@@ -46,6 +46,17 @@
 #define read_sysreg_el2(r)	read_sysreg_elx(r, _EL2, _EL1)
 #define write_sysreg_el2(v,r)	write_sysreg_elx(v, r, _EL2, _EL1)
 
+static inline u64 __hyp_text read_sysreg_par(void)
+{
+	u64 par;
+	if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
+		dmb(sy);
+	par = read_sysreg(par_el1);
+	if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
+		dmb(sy);
+	return par;
+}
+
 /*
  * Without an __arch_swab32(), we fall back to ___constant_swab32(), but the
  * static inline can allow the compiler to out-of-line this. KVM always wants
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index ad06d6802d2e..5eee8a75540c 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -938,6 +938,16 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		.matches = has_neoverse_n1_erratum_1542419,
 		.cpu_enable = cpu_enable_trap_ctr_access,
 	},
+#endif
+#ifdef CONFIG_ARM64_ERRATUM_1508412
+	{
+		/* we depend on the firmware portion for correctness */
+		.desc = "ARM erratum 1508412 (kernel portion)",
+		.capability = ARM64_WORKAROUND_1508412,
+		ERRATA_MIDR_RANGE(MIDR_CORTEX_A77,
+				  0, 0,
+				  1, 0),
+	},
 #endif
 	{
 	}
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 9b070b5e212b..21d8b3ca5bd7 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1653,7 +1653,8 @@ int kvm_arch_init(void *opaque)
 		return -ENODEV;
 	}
 
-	if (cpus_have_const_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE))
+	if (cpus_have_const_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE) ||
+	    cpus_have_const_cap(ARM64_WORKAROUND_1508412))
 		kvm_info("Guests without required CPU erratum workarounds can deadlock system!\n" \
 			 "Only trusted guests should be used on this system.\n");
 
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index db1c4487d95d..d76b6638b705 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -298,11 +298,12 @@ static bool __hyp_text __translate_far_to_hpfar(u64 far, u64 *hpfar)
 	 * We do need to save/restore PAR_EL1 though, as we haven't
 	 * saved the guest context yet, and we may return early...
 	 */
-	par = read_sysreg(par_el1);
+	par = read_sysreg_par();
+
 	asm volatile("at s1e1r, %0" : : "r" (far));
 	isb();
 
-	tmp = read_sysreg(par_el1);
+	tmp = read_sysreg_par();
 	write_sysreg(par, par_el1);
 
 	if (unlikely(tmp & SYS_PAR_EL1_F))
@@ -925,7 +926,7 @@ void __hyp_text __noreturn hyp_panic(struct kvm_cpu_context *host_ctxt)
 {
 	u64 spsr = read_sysreg_el2(SYS_SPSR);
 	u64 elr = read_sysreg_el2(SYS_ELR);
-	u64 par = read_sysreg(par_el1);
+	u64 par = read_sysreg_par();
 
 	if (!has_vhe())
 		__hyp_call_panic_nvhe(spsr, elr, par, host_ctxt);
diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
index cc7e957f5b2c..f522cbff291d 100644
--- a/arch/arm64/kvm/hyp/sysreg-sr.c
+++ b/arch/arm64/kvm/hyp/sysreg-sr.c
@@ -52,7 +52,7 @@ static void __hyp_text __sysreg_save_el1_state(struct kvm_cpu_context *ctxt)
 	ctxt->sys_regs[CONTEXTIDR_EL1]	= read_sysreg_el1(SYS_CONTEXTIDR);
 	ctxt->sys_regs[AMAIR_EL1]	= read_sysreg_el1(SYS_AMAIR);
 	ctxt->sys_regs[CNTKCTL_EL1]	= read_sysreg_el1(SYS_CNTKCTL);
-	ctxt->sys_regs[PAR_EL1]		= read_sysreg(par_el1);
+	ctxt->sys_regs[PAR_EL1]		= read_sysreg_par();
 	ctxt->sys_regs[TPIDR_EL1]	= read_sysreg(tpidr_el1);
 
 	ctxt->gp_regs.sp_el1		= read_sysreg(sp_el1);
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index baf5ce9225ce..3f798e0f1419 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -94,10 +94,16 @@ static bool __vcpu_read_sys_reg_from_cpu(int reg, u64 *val)
 	case TPIDR_EL1:		*val = read_sysreg_s(SYS_TPIDR_EL1);	break;
 	case AMAIR_EL1:		*val = read_sysreg_s(SYS_AMAIR_EL12);	break;
 	case CNTKCTL_EL1:	*val = read_sysreg_s(SYS_CNTKCTL_EL12);	break;
-	case PAR_EL1:		*val = read_sysreg_s(SYS_PAR_EL1);	break;
 	case DACR32_EL2:	*val = read_sysreg_s(SYS_DACR32_EL2);	break;
 	case IFSR32_EL2:	*val = read_sysreg_s(SYS_IFSR32_EL2);	break;
 	case DBGVCR32_EL2:	*val = read_sysreg_s(SYS_DBGVCR32_EL2);	break;
+	case PAR_EL1:
+		if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
+			dmb(sy);
+		*val = read_sysreg_s(SYS_PAR_EL1);
+		if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
+			dmb(sy);
+		break;
 	default:		return false;
 	}
 
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 8afb238ff335..98609532e61a 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -260,7 +260,17 @@ static bool __kprobes is_spurious_el1_translation_fault(unsigned long addr,
 	local_irq_save(flags);
 	asm volatile("at s1e1r, %0" :: "r" (addr));
 	isb();
+	/*
+	 * Arm Erratum 1508412 requires dmb(sy) before and after reads of
+	 * PAR_EL1.
+	 * As this location is not a hot path, just condition it on the config
+	 * option.
+	 */
+	if (IS_ENABLED(CONFIG_ARM64_ERRATUM_1508412))
+		dmb(sy);
 	par = read_sysreg(par_el1);
+	if (IS_ENABLED(CONFIG_ARM64_ERRATUM_1508412))
+		dmb(sy);
 	local_irq_restore(flags);
 
 	/*
-- 
2.25.1

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 3/3] arm64: Add workaround for Arm Cortex-A77 erratum 1508412

[Andrew Scull]

Hi Rob, a couple of suggestions for way this erratum is gated, but I
haven't delved into the details of the errata itself.

On Fri, Jul 17, 2020 at 02:52:33PM -0600, Rob Herring wrote:
> On Cortex-A77 r0p0 and r1p0, a sequence of a non-cacheable or device load
> and a store exclusive or PAR_EL1 read can cause a deadlock.
> 
> The workaround requires a DMB SY before and after a PAR_EL1 register read.
> A deadlock is still possible with the workaround as KVM guests must also
> have the workaround. IOW, a malicious guest can deadlock an affected
> systems.
> 
> This workaround also depends on a firmware counterpart to enable the h/w
> to insert DMB SY after load and store exclusive instructions. See the
> errata document SDEN-1152370 v10 [1] for more information.
> 
> [1] https://static.docs.arm.com/101992/0010/Arm_Cortex_A77_MP074_Software_Developer_Errata_Notice_v10.pdf
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Julien Thierry <julien.thierry.kdev@gmail.com>
> Cc: kvmarm@lists.cs.columbia.edu
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> v3:
> - Add dmbs around PAR reads in KVM code
> - Clean-up 'work-around' and 'errata'
> 
> v2:
> - Don't disable KVM, just print warning
> ---
>  Documentation/arm64/silicon-errata.rst |  2 ++
>  arch/arm64/Kconfig                     | 19 +++++++++++++++++++
>  arch/arm64/include/asm/cpucaps.h       |  3 ++-
>  arch/arm64/include/asm/kvm_hyp.h       | 11 +++++++++++
>  arch/arm64/kernel/cpu_errata.c         | 10 ++++++++++
>  arch/arm64/kvm/arm.c                   |  3 ++-
>  arch/arm64/kvm/hyp/switch.c            |  7 ++++---
>  arch/arm64/kvm/hyp/sysreg-sr.c         |  2 +-
>  arch/arm64/kvm/sys_regs.c              |  8 +++++++-
>  arch/arm64/mm/fault.c                  | 10 ++++++++++
>  10 files changed, 68 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
> index 936cf2a59ca4..716b279e3b33 100644
> --- a/Documentation/arm64/silicon-errata.rst
> +++ b/Documentation/arm64/silicon-errata.rst
> @@ -90,6 +90,8 @@ stable kernels.
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-A76      | #1463225        | ARM64_ERRATUM_1463225       |
>  +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Cortex-A77      | #1508412        | ARM64_ERRATUM_1508412       |
> ++----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Neoverse-N1     | #1188873,1418040| ARM64_ERRATUM_1418040       |
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Neoverse-N1     | #1349291        | N/A                         |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index a4a094bedcb2..6638444ce0d8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -626,6 +626,25 @@ config ARM64_ERRATUM_1542419
>  
>  	  If unsure, say Y.
>  
> +config ARM64_ERRATUM_1508412
> +	bool "Cortex-A77: 1508412: workaround deadlock on sequence of NC/Device load and store exclusive or PAR read"
> +	default y
> +	help
> +	  This option adds a workaround for Arm Cortex-A77 erratum 1508412.
> +
> +	  Affected Cortex-A77 cores (r0p0, r1p0) could deadlock on a sequence
> +	  of a store-exclusive or read of PAR_EL1 and a load with device or
> +	  non-cacheable memory attributes. The workaround depends on a firmware
> +	  counterpart.
> +
> +	  KVM guests must also have the workaround implemented or they can
> +	  deadlock the system.
> +
> +	  Work around the issue by inserting DMB SY barriers around PAR_EL1
> +	  register reads and warning KVM users.
> +
> +	  If unsure, say Y.
> +
>  config CAVIUM_ERRATUM_22375
>  	bool "Cavium erratum 22375, 24313"
>  	default y
> diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
> index d7b3bb0cb180..2a2cdb4ced8b 100644
> --- a/arch/arm64/include/asm/cpucaps.h
> +++ b/arch/arm64/include/asm/cpucaps.h
> @@ -62,7 +62,8 @@
>  #define ARM64_HAS_GENERIC_AUTH			52
>  #define ARM64_HAS_32BIT_EL1			53
>  #define ARM64_BTI				54
> +#define ARM64_WORKAROUND_1508412		55
>  
> -#define ARM64_NCAPS				55
> +#define ARM64_NCAPS				56
>  
>  #endif /* __ASM_CPUCAPS_H */
> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> index ce3080834bfa..ce5b0d9b12bf 100644
> --- a/arch/arm64/include/asm/kvm_hyp.h
> +++ b/arch/arm64/include/asm/kvm_hyp.h
> @@ -46,6 +46,17 @@
>  #define read_sysreg_el2(r)	read_sysreg_elx(r, _EL2, _EL1)
>  #define write_sysreg_el2(v,r)	write_sysreg_elx(v, r, _EL2, _EL1)
>  
> +static inline u64 __hyp_text read_sysreg_par(void)
> +{
> +	u64 par;
> +	if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
> +		dmb(sy);
> +	par = read_sysreg(par_el1);
> +	if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
> +		dmb(sy);
> +	return par;
> +}
> +
>  /*
>   * Without an __arch_swab32(), we fall back to ___constant_swab32(), but the
>   * static inline can allow the compiler to out-of-line this. KVM always wants
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index ad06d6802d2e..5eee8a75540c 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -938,6 +938,16 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>  		.matches = has_neoverse_n1_erratum_1542419,
>  		.cpu_enable = cpu_enable_trap_ctr_access,
>  	},
> +#endif
> +#ifdef CONFIG_ARM64_ERRATUM_1508412
> +	{
> +		/* we depend on the firmware portion for correctness */
> +		.desc = "ARM erratum 1508412 (kernel portion)",
> +		.capability = ARM64_WORKAROUND_1508412,
> +		ERRATA_MIDR_RANGE(MIDR_CORTEX_A77,
> +				  0, 0,
> +				  1, 0),
> +	},
>  #endif
>  	{
>  	}
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 9b070b5e212b..21d8b3ca5bd7 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1653,7 +1653,8 @@ int kvm_arch_init(void *opaque)
>  		return -ENODEV;
>  	}
>  
> -	if (cpus_have_const_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE))
> +	if (cpus_have_const_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE) ||
> +	    cpus_have_const_cap(ARM64_WORKAROUND_1508412))

By the time KVM is initialized, it's safe to use cpus_have_final_cap
rather than cpus_have_const_cap as the capabilities will have been
finalized.

It looks as though the other places that the capabilities are checked
happen after this point so they can also use cpus_have_final_cap.

>  		kvm_info("Guests without required CPU erratum workarounds can deadlock system!\n" \
>  			 "Only trusted guests should be used on this system.\n");
>  
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index db1c4487d95d..d76b6638b705 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -298,11 +298,12 @@ static bool __hyp_text __translate_far_to_hpfar(u64 far, u64 *hpfar)
>  	 * We do need to save/restore PAR_EL1 though, as we haven't
>  	 * saved the guest context yet, and we may return early...
>  	 */
> -	par = read_sysreg(par_el1);
> +	par = read_sysreg_par();
> +
>  	asm volatile("at s1e1r, %0" : : "r" (far));
>  	isb();
>  
> -	tmp = read_sysreg(par_el1);
> +	tmp = read_sysreg_par();
>  	write_sysreg(par, par_el1);
>  
>  	if (unlikely(tmp & SYS_PAR_EL1_F))
> @@ -925,7 +926,7 @@ void __hyp_text __noreturn hyp_panic(struct kvm_cpu_context *host_ctxt)
>  {
>  	u64 spsr = read_sysreg_el2(SYS_SPSR);
>  	u64 elr = read_sysreg_el2(SYS_ELR);
> -	u64 par = read_sysreg(par_el1);
> +	u64 par = read_sysreg_par();
>  
>  	if (!has_vhe())
>  		__hyp_call_panic_nvhe(spsr, elr, par, host_ctxt);
> diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
> index cc7e957f5b2c..f522cbff291d 100644
> --- a/arch/arm64/kvm/hyp/sysreg-sr.c
> +++ b/arch/arm64/kvm/hyp/sysreg-sr.c
> @@ -52,7 +52,7 @@ static void __hyp_text __sysreg_save_el1_state(struct kvm_cpu_context *ctxt)
>  	ctxt->sys_regs[CONTEXTIDR_EL1]	= read_sysreg_el1(SYS_CONTEXTIDR);
>  	ctxt->sys_regs[AMAIR_EL1]	= read_sysreg_el1(SYS_AMAIR);
>  	ctxt->sys_regs[CNTKCTL_EL1]	= read_sysreg_el1(SYS_CNTKCTL);
> -	ctxt->sys_regs[PAR_EL1]		= read_sysreg(par_el1);
> +	ctxt->sys_regs[PAR_EL1]		= read_sysreg_par();
>  	ctxt->sys_regs[TPIDR_EL1]	= read_sysreg(tpidr_el1);
>  
>  	ctxt->gp_regs.sp_el1		= read_sysreg(sp_el1);
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index baf5ce9225ce..3f798e0f1419 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -94,10 +94,16 @@ static bool __vcpu_read_sys_reg_from_cpu(int reg, u64 *val)
>  	case TPIDR_EL1:		*val = read_sysreg_s(SYS_TPIDR_EL1);	break;
>  	case AMAIR_EL1:		*val = read_sysreg_s(SYS_AMAIR_EL12);	break;
>  	case CNTKCTL_EL1:	*val = read_sysreg_s(SYS_CNTKCTL_EL12);	break;
> -	case PAR_EL1:		*val = read_sysreg_s(SYS_PAR_EL1);	break;
>  	case DACR32_EL2:	*val = read_sysreg_s(SYS_DACR32_EL2);	break;
>  	case IFSR32_EL2:	*val = read_sysreg_s(SYS_IFSR32_EL2);	break;
>  	case DBGVCR32_EL2:	*val = read_sysreg_s(SYS_DBGVCR32_EL2);	break;
> +	case PAR_EL1:
> +		if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
> +			dmb(sy);
> +		*val = read_sysreg_s(SYS_PAR_EL1);
> +		if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
> +			dmb(sy);
> +		break;
>  	default:		return false;
>  	}
>  
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 8afb238ff335..98609532e61a 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -260,7 +260,17 @@ static bool __kprobes is_spurious_el1_translation_fault(unsigned long addr,
>  	local_irq_save(flags);
>  	asm volatile("at s1e1r, %0" :: "r" (addr));
>  	isb();
> +	/*
> +	 * Arm Erratum 1508412 requires dmb(sy) before and after reads of
> +	 * PAR_EL1.
> +	 * As this location is not a hot path, just condition it on the config
> +	 * option.
> +	 */
> +	if (IS_ENABLED(CONFIG_ARM64_ERRATUM_1508412))
> +		dmb(sy);
>  	par = read_sysreg(par_el1);
> +	if (IS_ENABLED(CONFIG_ARM64_ERRATUM_1508412))
> +		dmb(sy);

Having the condition simply based on the config can bring the additional
DMBs even if the CPU isn't affected by this erratum. The config is
default y so this doesn't seem like an unlikely situation.

The comment mentions that this is a hot path so would an alternative be
applicable? That's the approach taken for the speculative AT errata.

  asm(ALTERNATIVE("nop", "dmb sy", ARM64_ERRATUM_1508412));

>  	local_irq_restore(flags);
>  
>  	/*
> -- 
> 2.25.1
> 
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 3/3] arm64: Add workaround for Arm Cortex-A77 erratum 1508412

[Catalin Marinas]

On Fri, Jul 17, 2020 at 02:52:33PM -0600, Rob Herring wrote:
> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> index ce3080834bfa..ce5b0d9b12bf 100644
> --- a/arch/arm64/include/asm/kvm_hyp.h
> +++ b/arch/arm64/include/asm/kvm_hyp.h
> @@ -46,6 +46,17 @@
>  #define read_sysreg_el2(r)	read_sysreg_elx(r, _EL2, _EL1)
>  #define write_sysreg_el2(v,r)	write_sysreg_elx(v, r, _EL2, _EL1)
>  
> +static inline u64 __hyp_text read_sysreg_par(void)
> +{
> +	u64 par;
> +	if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
> +		dmb(sy);
> +	par = read_sysreg(par_el1);
> +	if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
> +		dmb(sy);
> +	return par;
> +}

Even if that's not always called on a critical path, I agree with Andrew
that we could use alternatives here for dmb(sy).

> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index baf5ce9225ce..3f798e0f1419 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -94,10 +94,16 @@ static bool __vcpu_read_sys_reg_from_cpu(int reg, u64 *val)
>  	case TPIDR_EL1:		*val = read_sysreg_s(SYS_TPIDR_EL1);	break;
>  	case AMAIR_EL1:		*val = read_sysreg_s(SYS_AMAIR_EL12);	break;
>  	case CNTKCTL_EL1:	*val = read_sysreg_s(SYS_CNTKCTL_EL12);	break;
> -	case PAR_EL1:		*val = read_sysreg_s(SYS_PAR_EL1);	break;
>  	case DACR32_EL2:	*val = read_sysreg_s(SYS_DACR32_EL2);	break;
>  	case IFSR32_EL2:	*val = read_sysreg_s(SYS_IFSR32_EL2);	break;
>  	case DBGVCR32_EL2:	*val = read_sysreg_s(SYS_DBGVCR32_EL2);	break;
> +	case PAR_EL1:
> +		if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
> +			dmb(sy);
> +		*val = read_sysreg_s(SYS_PAR_EL1);
> +		if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
> +			dmb(sy);
> +		break;
>  	default:		return false;
>  	}

Can't we use read_sysreg_par() directly here?

> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 8afb238ff335..98609532e61a 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -260,7 +260,17 @@ static bool __kprobes is_spurious_el1_translation_fault(unsigned long addr,
>  	local_irq_save(flags);
>  	asm volatile("at s1e1r, %0" :: "r" (addr));
>  	isb();
> +	/*
> +	 * Arm Erratum 1508412 requires dmb(sy) before and after reads of
> +	 * PAR_EL1.
> +	 * As this location is not a hot path, just condition it on the config
> +	 * option.
> +	 */
> +	if (IS_ENABLED(CONFIG_ARM64_ERRATUM_1508412))
> +		dmb(sy);
>  	par = read_sysreg(par_el1);
> +	if (IS_ENABLED(CONFIG_ARM64_ERRATUM_1508412))
> +		dmb(sy);
>  	local_irq_restore(flags);

Why not read_sysreg_par()?

-- 
Catalin
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 3/3] arm64: Add workaround for Arm Cortex-A77 erratum 1508412

[Will Deacon]

On Wed, Jul 29, 2020 at 05:38:00PM +0100, Catalin Marinas wrote:
> On Fri, Jul 17, 2020 at 02:52:33PM -0600, Rob Herring wrote:
> > diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> > index ce3080834bfa..ce5b0d9b12bf 100644
> > --- a/arch/arm64/include/asm/kvm_hyp.h
> > +++ b/arch/arm64/include/asm/kvm_hyp.h
> > @@ -46,6 +46,17 @@
> >  #define read_sysreg_el2(r)	read_sysreg_elx(r, _EL2, _EL1)
> >  #define write_sysreg_el2(v,r)	write_sysreg_elx(v, r, _EL2, _EL1)
> >  
> > +static inline u64 __hyp_text read_sysreg_par(void)
> > +{
> > +	u64 par;
> > +	if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
> > +		dmb(sy);
> > +	par = read_sysreg(par_el1);
> > +	if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
> > +		dmb(sy);
> > +	return par;
> > +}
> 
> Even if that's not always called on a critical path, I agree with Andrew
> that we could use alternatives here for dmb(sy).

Even then, I'm not sure how this helps if the CPU can speculatively branch
to the PAR access without executing the DMB. Is that not possible on A77?

Will
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 3/3] arm64: Add workaround for Arm Cortex-A77 erratum 1508412

[Rob Herring]

On Thu, Jul 30, 2020 at 2:22 AM Will Deacon <will@kernel.org> wrote:
>
> On Wed, Jul 29, 2020 at 05:38:00PM +0100, Catalin Marinas wrote:
> > On Fri, Jul 17, 2020 at 02:52:33PM -0600, Rob Herring wrote:
> > > diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> > > index ce3080834bfa..ce5b0d9b12bf 100644
> > > --- a/arch/arm64/include/asm/kvm_hyp.h
> > > +++ b/arch/arm64/include/asm/kvm_hyp.h
> > > @@ -46,6 +46,17 @@
> > >  #define read_sysreg_el2(r) read_sysreg_elx(r, _EL2, _EL1)
> > >  #define write_sysreg_el2(v,r)      write_sysreg_elx(v, r, _EL2, _EL1)
> > >
> > > +static inline u64 __hyp_text read_sysreg_par(void)
> > > +{
> > > +   u64 par;
> > > +   if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
> > > +           dmb(sy);
> > > +   par = read_sysreg(par_el1);
> > > +   if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
> > > +           dmb(sy);
> > > +   return par;
> > > +}
> >
> > Even if that's not always called on a critical path, I agree with Andrew
> > that we could use alternatives here for dmb(sy).
>
> Even then, I'm not sure how this helps if the CPU can speculatively branch
> to the PAR access without executing the DMB. Is that not possible on A77?

I'm told by the h/w folks speculation is not possible in this case.

Rob
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 3/3] arm64: Add workaround for Arm Cortex-A77 erratum 1508412

[Rob Herring]

On Wed, Jul 29, 2020 at 10:38 AM Catalin Marinas
<catalin.marinas@arm.com> wrote:
>
> On Fri, Jul 17, 2020 at 02:52:33PM -0600, Rob Herring wrote:
> > diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> > index ce3080834bfa..ce5b0d9b12bf 100644
> > --- a/arch/arm64/include/asm/kvm_hyp.h
> > +++ b/arch/arm64/include/asm/kvm_hyp.h
> > @@ -46,6 +46,17 @@
> >  #define read_sysreg_el2(r)   read_sysreg_elx(r, _EL2, _EL1)
> >  #define write_sysreg_el2(v,r)        write_sysreg_elx(v, r, _EL2, _EL1)
> >
> > +static inline u64 __hyp_text read_sysreg_par(void)
> > +{
> > +     u64 par;
> > +     if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
> > +             dmb(sy);
> > +     par = read_sysreg(par_el1);
> > +     if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
> > +             dmb(sy);
> > +     return par;
> > +}
>
> Even if that's not always called on a critical path, I agree with Andrew
> that we could use alternatives here for dmb(sy).

His suggestion in the KVM code was to use cpus_have_final_cap() rather
than cpus_have_const_cap. But given it's just a dmb or nop,
alternatives is a better choice for all of these?

> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index baf5ce9225ce..3f798e0f1419 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -94,10 +94,16 @@ static bool __vcpu_read_sys_reg_from_cpu(int reg, u64 *val)
> >       case TPIDR_EL1:         *val = read_sysreg_s(SYS_TPIDR_EL1);    break;
> >       case AMAIR_EL1:         *val = read_sysreg_s(SYS_AMAIR_EL12);   break;
> >       case CNTKCTL_EL1:       *val = read_sysreg_s(SYS_CNTKCTL_EL12); break;
> > -     case PAR_EL1:           *val = read_sysreg_s(SYS_PAR_EL1);      break;
> >       case DACR32_EL2:        *val = read_sysreg_s(SYS_DACR32_EL2);   break;
> >       case IFSR32_EL2:        *val = read_sysreg_s(SYS_IFSR32_EL2);   break;
> >       case DBGVCR32_EL2:      *val = read_sysreg_s(SYS_DBGVCR32_EL2); break;
> > +     case PAR_EL1:
> > +             if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
> > +                     dmb(sy);
> > +             *val = read_sysreg_s(SYS_PAR_EL1);
> > +             if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
> > +                     dmb(sy);
> > +             break;
> >       default:                return false;
> >       }
>
> Can't we use read_sysreg_par() directly here?

I assumed read_sysreg_s() was used here for some reason instead of
read_sysreg()?

> > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> > index 8afb238ff335..98609532e61a 100644
> > --- a/arch/arm64/mm/fault.c
> > +++ b/arch/arm64/mm/fault.c
> > @@ -260,7 +260,17 @@ static bool __kprobes is_spurious_el1_translation_fault(unsigned long addr,
> >       local_irq_save(flags);
> >       asm volatile("at s1e1r, %0" :: "r" (addr));
> >       isb();
> > +     /*
> > +      * Arm Erratum 1508412 requires dmb(sy) before and after reads of
> > +      * PAR_EL1.
> > +      * As this location is not a hot path, just condition it on the config
> > +      * option.
> > +      */
> > +     if (IS_ENABLED(CONFIG_ARM64_ERRATUM_1508412))
> > +             dmb(sy);
> >       par = read_sysreg(par_el1);
> > +     if (IS_ENABLED(CONFIG_ARM64_ERRATUM_1508412))
> > +             dmb(sy);
> >       local_irq_restore(flags);
>
> Why not read_sysreg_par()?

Okay with read_sysreg_par() going in asm/sysreg.h instead? I was
hesitant to add it there as there didn't seem to be any other
instances of a function for a specific register there.

Rob
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 3/3] arm64: Add workaround for Arm Cortex-A77 erratum 1508412

[Will Deacon]

On Fri, Jul 31, 2020 at 04:55:26PM -0600, Rob Herring wrote:
> On Thu, Jul 30, 2020 at 2:22 AM Will Deacon <will@kernel.org> wrote:
> >
> > On Wed, Jul 29, 2020 at 05:38:00PM +0100, Catalin Marinas wrote:
> > > On Fri, Jul 17, 2020 at 02:52:33PM -0600, Rob Herring wrote:
> > > > diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> > > > index ce3080834bfa..ce5b0d9b12bf 100644
> > > > --- a/arch/arm64/include/asm/kvm_hyp.h
> > > > +++ b/arch/arm64/include/asm/kvm_hyp.h
> > > > @@ -46,6 +46,17 @@
> > > >  #define read_sysreg_el2(r) read_sysreg_elx(r, _EL2, _EL1)
> > > >  #define write_sysreg_el2(v,r)      write_sysreg_elx(v, r, _EL2, _EL1)
> > > >
> > > > +static inline u64 __hyp_text read_sysreg_par(void)
> > > > +{
> > > > +   u64 par;
> > > > +   if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
> > > > +           dmb(sy);
> > > > +   par = read_sysreg(par_el1);
> > > > +   if (cpus_have_const_cap(ARM64_WORKAROUND_1508412))
> > > > +           dmb(sy);
> > > > +   return par;
> > > > +}
> > >
> > > Even if that's not always called on a critical path, I agree with Andrew
> > > that we could use alternatives here for dmb(sy).
> >
> > Even then, I'm not sure how this helps if the CPU can speculatively branch
> > to the PAR access without executing the DMB. Is that not possible on A77?
> 
> I'm told by the h/w folks speculation is not possible in this case.

Thanks. Could you add a comment to that effect, please?

Will
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm