[PATCH v2 0/3] Cortex-A77 erratum 1508412 work-around

[PATCH v2 0/3] Cortex-A77 erratum 1508412 work-around

[Rob Herring]

This series implements the work-around for Cortex-A77 erratum 1508412.
KVM guests which don't implement the work-around 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.

Rob

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/kernel/cpu_errata.c         | 10 ++++++++++
 arch/arm64/kvm/arm.c                   |  5 +++++
 arch/arm64/mm/fault.c                  | 10 ++++++++++
 7 files changed, 50 insertions(+), 1 deletion(-)

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

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

[Rob Herring]

If guests don't have certain CPU erratum work-arounds 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>
---
 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..e2f50fa4d825 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 work-arounds 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 v2 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 v2 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
and the disabling of KVM. KVM must be disabled to prevent the problematic
sequence in guests' EL1. 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.

All the other PAR_EL1 reads besides the one in
is_spurious_el1_translation_fault() are in KVM code, so the work-around is
not needed for them.

[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>
---
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/kernel/cpu_errata.c         | 10 ++++++++++
 arch/arm64/kvm/arm.c                   |  3 ++-
 arch/arm64/mm/fault.c                  | 10 ++++++++++
 6 files changed, 45 insertions(+), 2 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..28993ad4c649 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 work-around implemented or they can
+	  deadlock the system.
+
+	  Workaround 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/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 e2f50fa4d825..9f50e01eea00 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 work-arounds can deadlock system!\n" \
 			 "Only trusted guests should be used on this system.\n");
 
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 8afb238ff335..d599d60f06fd 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 Errata 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 v2 3/3] arm64: Add workaround for Arm Cortex-A77 erratum 1508412

[Will Deacon]

On Wed, Jul 01, 2020 at 03:53:08PM -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
> and the disabling of KVM. KVM must be disabled to prevent the problematic
> sequence in guests' EL1. 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.

This ^^ is out of date not that we're not disabling KVM.

> All the other PAR_EL1 reads besides the one in
> is_spurious_el1_translation_fault() are in KVM code, so the work-around is
> not needed for them.

And I think this now needs some extra work.

> [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>
> ---
> 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/kernel/cpu_errata.c         | 10 ++++++++++
>  arch/arm64/kvm/arm.c                   |  3 ++-
>  arch/arm64/mm/fault.c                  | 10 ++++++++++
>  6 files changed, 45 insertions(+), 2 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..28993ad4c649 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 work-around implemented or they can

work-around => workaround

> +	  deadlock the system.
> +
> +	  Workaround the issue by inserting DMB SY barriers around PAR_EL1

Workaround => work around

> +	  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/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 e2f50fa4d825..9f50e01eea00 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 work-arounds can deadlock system!\n" \
>  			 "Only trusted guests should be used on this system.\n");
>  
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 8afb238ff335..d599d60f06fd 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 Errata 1508412 requires dmb(sy) before and after reads of

Errata => erratum

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

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

[Will Deacon]

On Wed, Jul 01, 2020 at 03:53:06PM -0600, Rob Herring wrote:
> If guests don't have certain CPU erratum work-arounds 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>
> ---
>  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..e2f50fa4d825 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 work-arounds can deadlock system!\n" \

work-arounds => workarounds

(mainly for consistency, I have no clue if this is a real word or not!).

I'd also probably do erratum => errata given that you're about to add a
second one.

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

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

[Rob Herring]

On Thu, Jul 2, 2020 at 5:45 AM Will Deacon <will@kernel.org> wrote:
>
> On Wed, Jul 01, 2020 at 03:53:06PM -0600, Rob Herring wrote:
> > If guests don't have certain CPU erratum work-arounds 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>
> > ---
> >  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..e2f50fa4d825 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 work-arounds can deadlock system!\n" \
>
> work-arounds => workarounds
>
> (mainly for consistency, I have no clue if this is a real word or not!).
>
> I'd also probably do erratum => errata given that you're about to add a
> second one.

Humm, the plural is on workarounds. If I use a more standard singular
vs. plural form like "CPU feature workarounds" vs "CPU features
workarounds", the former seems more correct to me. (working around
features may be a bit nonsensical, but big.LITTLE ;) )

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

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

[Will Deacon]

On Thu, Jul 02, 2020 at 08:04:43AM -0600, Rob Herring wrote:
> On Thu, Jul 2, 2020 at 5:45 AM Will Deacon <will@kernel.org> wrote:
> >
> > On Wed, Jul 01, 2020 at 03:53:06PM -0600, Rob Herring wrote:
> > > If guests don't have certain CPU erratum work-arounds 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>
> > > ---
> > >  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..e2f50fa4d825 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 work-arounds can deadlock system!\n" \
> >
> > work-arounds => workarounds
> >
> > (mainly for consistency, I have no clue if this is a real word or not!).
> >
> > I'd also probably do erratum => errata given that you're about to add a
> > second one.
> 
> Humm, the plural is on workarounds. If I use a more standard singular
> vs. plural form like "CPU feature workarounds" vs "CPU features
> workarounds", the former seems more correct to me. (working around
> features may be a bit nonsensical, but big.LITTLE ;) )

Ok, "erratum" it is then!

One other thing on the actual workaround... Case B in the document says:

  | In Program Order,
  | 1. The core executes any load with device memory attributes.
  | 2. The core executes a store-exclusive or register read of PAR_EL1.

and the patch register sequence says:

  | Use the following write sequence to several IMPLEMENTATION DEFINED
  | registers to have the hardware insert a DMB SY after all load-exclusive and
  | store-exclusive instructions.

but that would mean that the sequence is effectively:

	LDR	Xa, [device address]
	STXR	Wb, Xc, [Xd]
	DMB	SY

Does that really prevent the deadlock?

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

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

[Rob Herring]

On Thu, Jul 2, 2020 at 5:42 AM Will Deacon <will@kernel.org> wrote:
>
> On Wed, Jul 01, 2020 at 03:53:08PM -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
> > and the disabling of KVM. KVM must be disabled to prevent the problematic
> > sequence in guests' EL1. 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.
>
> This ^^ is out of date not that we're not disabling KVM.

Indeed, I fixed the kconfig text, but missed this.

> > All the other PAR_EL1 reads besides the one in
> > is_spurious_el1_translation_fault() are in KVM code, so the work-around is
> > not needed for them.
>
> And I think this now needs some extra work.

Ugg, that was too easy...

The KVM code in __translate_far_to_hpfar() has:

read PAR
read PAR
write PAR

I'm wondering if we need 2 dmbs or 4 here. I'm checking on that.

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

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

[Will Deacon]

On Thu, Jul 02, 2020 at 08:56:05AM -0600, Rob Herring wrote:
> On Thu, Jul 2, 2020 at 5:42 AM Will Deacon <will@kernel.org> wrote:
> >
> > On Wed, Jul 01, 2020 at 03:53:08PM -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
> > > and the disabling of KVM. KVM must be disabled to prevent the problematic
> > > sequence in guests' EL1. 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.
> >
> > This ^^ is out of date not that we're not disabling KVM.
> 
> Indeed, I fixed the kconfig text, but missed this.
> 
> > > All the other PAR_EL1 reads besides the one in
> > > is_spurious_el1_translation_fault() are in KVM code, so the work-around is
> > > not needed for them.
> >
> > And I think this now needs some extra work.
> 
> Ugg, that was too easy...
> 
> The KVM code in __translate_far_to_hpfar() has:
> 
> read PAR
> read PAR
> write PAR
> 
> I'm wondering if we need 2 dmbs or 4 here. I'm checking on that.

Also worth checking what happens if the PAR access is executed
speculatively, as in that case we probably can't guarantee that the DMB
instructions are executed at all...

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