[PATCH 0/4] Workaround for Cortex-A76 erratum 1165522

[PATCH 0/4] Workaround for Cortex-A76 erratum 1165522

[Marc Zyngier]

Early Cortex-A76 suffer from an erratum that can result in invalid
TLBs when the CPU speculatively executes an AT instruction in the
middle of a guest world switch, while the guest virtual memory
configuration is in an inconsistent state.

We handle this issue by mandating the use of VHE and making sure that
the guest context is fully installed before switching HCR_EL2.TGE to
zero. This ensures that a speculated AT instruction is either executed
on the host context (TGE set) or the guest context (TGE clear), and
that there is no intermediate state.

Marc Zyngier (4):
  KVM: arm64: Rework detection of SVE, !VHE systems
  KVM: arm64: Allow implementations to be confined to using VHE
  arm64: KVM: Install stage-2 translation before enabling traps on VHE
  arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm/include/asm/kvm_host.h        |  3 ++-
 arch/arm64/Kconfig                     | 12 ++++++++++++
 arch/arm64/include/asm/cpucaps.h       |  3 ++-
 arch/arm64/include/asm/kvm_host.h      | 14 ++++++++++----
 arch/arm64/include/asm/kvm_hyp.h       |  6 ++++++
 arch/arm64/kernel/cpu_errata.c         |  8 ++++++++
 arch/arm64/kvm/hyp/switch.c            | 16 +++++++++++++++-
 virt/kvm/arm/arm.c                     | 17 ++++++++++++-----
 9 files changed, 68 insertions(+), 12 deletions(-)

-- 
2.19.1

[PATCH 0/4] Workaround for Cortex-A76 erratum 1165522

[Marc Zyngier]

Early Cortex-A76 suffer from an erratum that can result in invalid
TLBs when the CPU speculatively executes an AT instruction in the
middle of a guest world switch, while the guest virtual memory
configuration is in an inconsistent state.

We handle this issue by mandating the use of VHE and making sure that
the guest context is fully installed before switching HCR_EL2.TGE to
zero. This ensures that a speculated AT instruction is either executed
on the host context (TGE set) or the guest context (TGE clear), and
that there is no intermediate state.

Marc Zyngier (4):
  KVM: arm64: Rework detection of SVE, !VHE systems
  KVM: arm64: Allow implementations to be confined to using VHE
  arm64: KVM: Install stage-2 translation before enabling traps on VHE
  arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm/include/asm/kvm_host.h        |  3 ++-
 arch/arm64/Kconfig                     | 12 ++++++++++++
 arch/arm64/include/asm/cpucaps.h       |  3 ++-
 arch/arm64/include/asm/kvm_host.h      | 14 ++++++++++----
 arch/arm64/include/asm/kvm_hyp.h       |  6 ++++++
 arch/arm64/kernel/cpu_errata.c         |  8 ++++++++
 arch/arm64/kvm/hyp/switch.c            | 16 +++++++++++++++-
 virt/kvm/arm/arm.c                     | 17 ++++++++++++-----
 9 files changed, 68 insertions(+), 12 deletions(-)

-- 
2.19.1

[PATCH 1/4] KVM: arm64: Rework detection of SVE, !VHE systems

[Marc Zyngier]

An SVE system is so far the only case where we mandate VHE. As we're
starting to grow this requirements, let's slightly rework the way we
deal with that situation, allowing for easy extension of this check.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/kvm_host.h   |  2 +-
 arch/arm64/include/asm/kvm_host.h |  7 ++-----
 virt/kvm/arm/arm.c                | 12 +++++++-----
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 5ca5d9af0c26..c3469729f40c 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -285,7 +285,7 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
 
 struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
 
-static inline bool kvm_arch_check_sve_has_vhe(void) { return true; }
+static inline bool kvm_arch_sve_requires_vhe(void) { return false; }
 static inline void kvm_arch_hardware_unsetup(void) {}
 static inline void kvm_arch_sync_events(struct kvm *kvm) {}
 static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 52fbc823ff8c..ca1714148400 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -422,17 +422,14 @@ static inline void __cpu_init_hyp_mode(phys_addr_t pgd_ptr,
 	}
 }
 
-static inline bool kvm_arch_check_sve_has_vhe(void)
+static inline bool kvm_arch_sve_requires_vhe(void)
 {
 	/*
 	 * The Arm architecture specifies that implementation of SVE
 	 * requires VHE also to be implemented.  The KVM code for arm64
 	 * relies on this when SVE is present:
 	 */
-	if (system_supports_sve())
-		return has_vhe();
-	else
-		return true;
+	return system_supports_sve();
 }
 
 static inline void kvm_arch_hardware_unsetup(void) {}
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 23774970c9df..66de2efddfca 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1640,9 +1640,13 @@ int kvm_arch_init(void *opaque)
 		return -ENODEV;
 	}
 
-	if (!kvm_arch_check_sve_has_vhe()) {
-		kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
-		return -ENODEV;
+	in_hyp_mode = is_kernel_in_hyp_mode();
+
+	if (!in_hyp_mode) {
+		if (kvm_arch_sve_requires_vhe()) {
+			kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
+			return -ENODEV;
+		}
 	}
 
 	for_each_online_cpu(cpu) {
@@ -1657,8 +1661,6 @@ int kvm_arch_init(void *opaque)
 	if (err)
 		return err;
 
-	in_hyp_mode = is_kernel_in_hyp_mode();
-
 	if (!in_hyp_mode) {
 		err = init_hyp_mode();
 		if (err)
-- 
2.19.1

[PATCH 1/4] KVM: arm64: Rework detection of SVE, !VHE systems

[Marc Zyngier]

An SVE system is so far the only case where we mandate VHE. As we're
starting to grow this requirements, let's slightly rework the way we
deal with that situation, allowing for easy extension of this check.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/kvm_host.h   |  2 +-
 arch/arm64/include/asm/kvm_host.h |  7 ++-----
 virt/kvm/arm/arm.c                | 12 +++++++-----
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 5ca5d9af0c26..c3469729f40c 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -285,7 +285,7 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
 
 struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
 
-static inline bool kvm_arch_check_sve_has_vhe(void) { return true; }
+static inline bool kvm_arch_sve_requires_vhe(void) { return false; }
 static inline void kvm_arch_hardware_unsetup(void) {}
 static inline void kvm_arch_sync_events(struct kvm *kvm) {}
 static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 52fbc823ff8c..ca1714148400 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -422,17 +422,14 @@ static inline void __cpu_init_hyp_mode(phys_addr_t pgd_ptr,
 	}
 }
 
-static inline bool kvm_arch_check_sve_has_vhe(void)
+static inline bool kvm_arch_sve_requires_vhe(void)
 {
 	/*
 	 * The Arm architecture specifies that implementation of SVE
 	 * requires VHE also to be implemented.  The KVM code for arm64
 	 * relies on this when SVE is present:
 	 */
-	if (system_supports_sve())
-		return has_vhe();
-	else
-		return true;
+	return system_supports_sve();
 }
 
 static inline void kvm_arch_hardware_unsetup(void) {}
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 23774970c9df..66de2efddfca 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1640,9 +1640,13 @@ int kvm_arch_init(void *opaque)
 		return -ENODEV;
 	}
 
-	if (!kvm_arch_check_sve_has_vhe()) {
-		kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
-		return -ENODEV;
+	in_hyp_mode = is_kernel_in_hyp_mode();
+
+	if (!in_hyp_mode) {
+		if (kvm_arch_sve_requires_vhe()) {
+			kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
+			return -ENODEV;
+		}
 	}
 
 	for_each_online_cpu(cpu) {
@@ -1657,8 +1661,6 @@ int kvm_arch_init(void *opaque)
 	if (err)
 		return err;
 
-	in_hyp_mode = is_kernel_in_hyp_mode();
-
 	if (!in_hyp_mode) {
 		err = init_hyp_mode();
 		if (err)
-- 
2.19.1

[PATCH 2/4] KVM: arm64: Allow implementations to be confined to using VHE

[Marc Zyngier]

Some implementations may be forced to use VHE to work around HW
errata, for example. Let's introduce a helper that checks for
these cases.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/kvm_host.h   | 1 +
 arch/arm64/include/asm/kvm_host.h | 6 ++++++
 virt/kvm/arm/arm.c                | 5 +++++
 3 files changed, 12 insertions(+)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index c3469729f40c..0f2f782548cb 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -286,6 +286,7 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
 struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
 
 static inline bool kvm_arch_sve_requires_vhe(void) { return false; }
+static inline bool kvm_arch_impl_requires_vhe(void) { return false; }
 static inline void kvm_arch_hardware_unsetup(void) {}
 static inline void kvm_arch_sync_events(struct kvm *kvm) {}
 static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index ca1714148400..7d6e974d024a 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -432,6 +432,12 @@ static inline bool kvm_arch_sve_requires_vhe(void)
 	return system_supports_sve();
 }
 
+static inline bool kvm_arch_impl_requires_vhe(void)
+{
+	/* Some implementations have defects that confine them to VHE */
+	return false;
+}
+
 static inline void kvm_arch_hardware_unsetup(void) {}
 static inline void kvm_arch_sync_events(struct kvm *kvm) {}
 static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 66de2efddfca..bc90a1cdd27f 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1647,6 +1647,11 @@ int kvm_arch_init(void *opaque)
 			kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
 			return -ENODEV;
 		}
+
+		if (kvm_arch_impl_requires_vhe()) {
+			kvm_pr_unimpl("CPU requiring VHE in non-VHE mode");
+			return -ENODEV;
+		}
 	}
 
 	for_each_online_cpu(cpu) {
-- 
2.19.1

[PATCH 2/4] KVM: arm64: Allow implementations to be confined to using VHE

[Marc Zyngier]

Some implementations may be forced to use VHE to work around HW
errata, for example. Let's introduce a helper that checks for
these cases.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/kvm_host.h   | 1 +
 arch/arm64/include/asm/kvm_host.h | 6 ++++++
 virt/kvm/arm/arm.c                | 5 +++++
 3 files changed, 12 insertions(+)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index c3469729f40c..0f2f782548cb 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -286,6 +286,7 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
 struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
 
 static inline bool kvm_arch_sve_requires_vhe(void) { return false; }
+static inline bool kvm_arch_impl_requires_vhe(void) { return false; }
 static inline void kvm_arch_hardware_unsetup(void) {}
 static inline void kvm_arch_sync_events(struct kvm *kvm) {}
 static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index ca1714148400..7d6e974d024a 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -432,6 +432,12 @@ static inline bool kvm_arch_sve_requires_vhe(void)
 	return system_supports_sve();
 }
 
+static inline bool kvm_arch_impl_requires_vhe(void)
+{
+	/* Some implementations have defects that confine them to VHE */
+	return false;
+}
+
 static inline void kvm_arch_hardware_unsetup(void) {}
 static inline void kvm_arch_sync_events(struct kvm *kvm) {}
 static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 66de2efddfca..bc90a1cdd27f 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1647,6 +1647,11 @@ int kvm_arch_init(void *opaque)
 			kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
 			return -ENODEV;
 		}
+
+		if (kvm_arch_impl_requires_vhe()) {
+			kvm_pr_unimpl("CPU requiring VHE in non-VHE mode");
+			return -ENODEV;
+		}
 	}
 
 	for_each_online_cpu(cpu) {
-- 
2.19.1

[PATCH 3/4] arm64: KVM: Install stage-2 translation before enabling traps on VHE

[Marc Zyngier]

It is a bit odd that we only install stage-2 translation after having
cleared HCR_EL2.TGE, which means that there is a window during which
AT requests could fail as stage-2 is not configured yet.

Let's move stage-2 configuration before we clear TGE, making the
guest entry sequence clearer: we first configure all the guest stuff,
then only switch to the guest translation regime.

Non-VHE doesn't have that kind of behaviour, and is left alone.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kvm/hyp/switch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 7cc175c88a37..51d5d966d9e5 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -499,8 +499,8 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
 
 	sysreg_save_host_state_vhe(host_ctxt);
 
-	__activate_traps(vcpu);
 	__activate_vm(vcpu->kvm);
+	__activate_traps(vcpu);
 
 	sysreg_restore_guest_state_vhe(guest_ctxt);
 	__debug_switch_to_guest(vcpu);
-- 
2.19.1

[PATCH 3/4] arm64: KVM: Install stage-2 translation before enabling traps on VHE

[Marc Zyngier]

It is a bit odd that we only install stage-2 translation after having
cleared HCR_EL2.TGE, which means that there is a window during which
AT requests could fail as stage-2 is not configured yet.

Let's move stage-2 configuration before we clear TGE, making the
guest entry sequence clearer: we first configure all the guest stuff,
then only switch to the guest translation regime.

Non-VHE doesn't have that kind of behaviour, and is left alone.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kvm/hyp/switch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 7cc175c88a37..51d5d966d9e5 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -499,8 +499,8 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
 
 	sysreg_save_host_state_vhe(host_ctxt);
 
-	__activate_traps(vcpu);
 	__activate_vm(vcpu->kvm);
+	__activate_traps(vcpu);
 
 	sysreg_restore_guest_state_vhe(guest_ctxt);
 	__debug_switch_to_guest(vcpu);
-- 
2.19.1

[PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[Marc Zyngier]

Early versions of Cortex-A76 can end-up with corrupt TLBs if they
speculate an AT instruction in during a guest switch while the
S1/S2 system registers are in an inconsistent state.

Work around it by:
- Mandating VHE
- Make sure that S1 and S2 system registers are consistent before
  clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
  regime

These two things together ensure that we cannot hit this erratum.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig                     | 12 ++++++++++++
 arch/arm64/include/asm/cpucaps.h       |  3 ++-
 arch/arm64/include/asm/kvm_host.h      |  3 +++
 arch/arm64/include/asm/kvm_hyp.h       |  6 ++++++
 arch/arm64/kernel/cpu_errata.c         |  8 ++++++++
 arch/arm64/kvm/hyp/switch.c            | 14 ++++++++++++++
 7 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
index 76ccded8b74c..04f0bc4690c6 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -57,6 +57,7 @@ stable kernels.
 | ARM            | Cortex-A73      | #858921         | ARM64_ERRATUM_858921        |
 | ARM            | Cortex-A55      | #1024718        | ARM64_ERRATUM_1024718       |
 | ARM            | Cortex-A76      | #1188873        | ARM64_ERRATUM_1188873       |
+| ARM            | Cortex-A76      | #1165522        | ARM64_ERRATUM_1165522       |
 | ARM            | MMU-500         | #841119,#826419 | N/A                         |
 |                |                 |                 |                             |
 | Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375        |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 787d7850e064..a68bc6cc2167 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873
 
 	  If unsure, say Y.
 
+config ARM64_ERRATUM_1165522
+	bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation"
+	default y
+	help
+	  This option adds work arounds for ARM Cortex-A76 erratum 1165522
+
+	  Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with
+	  corrupted TLBs by speculating an AT instruction during a guest
+	  context switch.
+
+	  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 6e2d254c09eb..62d8cd15fdf2 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -54,7 +54,8 @@
 #define ARM64_HAS_CRC32				33
 #define ARM64_SSBS				34
 #define ARM64_WORKAROUND_1188873		35
+#define ARM64_WORKAROUND_1165522		36
 
-#define ARM64_NCAPS				36
+#define ARM64_NCAPS				37
 
 #endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 7d6e974d024a..8f486026ac87 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -435,6 +435,9 @@ static inline bool kvm_arch_sve_requires_vhe(void)
 static inline bool kvm_arch_impl_requires_vhe(void)
 {
 	/* Some implementations have defects that confine them to VHE */
+	if (cpus_have_cap(ARM64_WORKAROUND_1165522))
+		return true;
+
 	return false;
 }
 
diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
index 23aca66767f9..9681360d0959 100644
--- a/arch/arm64/include/asm/kvm_hyp.h
+++ b/arch/arm64/include/asm/kvm_hyp.h
@@ -163,6 +163,12 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm)
 {
 	write_sysreg(kvm->arch.vtcr, vtcr_el2);
 	write_sysreg(kvm->arch.vttbr, vttbr_el2);
+
+	/*
+	 * ARM erratum 1165522 requires the actual execution of the
+	 * above before we can switch to the guest translation regime.
+	 */
+	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
 }
 
 #endif /* __ARM64_KVM_HYP_H__ */
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index a509e35132d2..476e738e6c46 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -739,6 +739,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		.capability = ARM64_WORKAROUND_1188873,
 		ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
 	},
+#endif
+#ifdef CONFIG_ARM64_ERRATUM_1165522
+	{
+		/* Cortex-A76 r0p0 to r2p0 */
+		.desc = "ARM erratum 1165522",
+		.capability = ARM64_WORKAROUND_1165522,
+		ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
+	},
 #endif
 	{
 	}
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 51d5d966d9e5..322109183853 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void)
 {
 	extern char vectors[];	/* kernel exception vectors */
 	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
+
+	/*
+	 * ARM erratum 1165522 requires the actual execution of the
+	 * above before we can switch to the host translation regime.
+	 */
+	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
+
 	write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
 	write_sysreg(vectors, vbar_el1);
 }
@@ -499,6 +506,13 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
 
 	sysreg_save_host_state_vhe(host_ctxt);
 
+	/*
+	 * ARM erratum 1165522 requires us to have all the translation
+	 * context in place before we clear HCR_TGE. All the offending
+	 * guest sysregs are loaded in kvm_vcpu_load_sysregs, and
+	 * __activate_vm has the stage-2 configuration. Once this is
+	 * done, __activate_trap clears HCR_TGE (among other things).
+	 */
 	__activate_vm(vcpu->kvm);
 	__activate_traps(vcpu);
 
-- 
2.19.1

[PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[Marc Zyngier]

Early versions of Cortex-A76 can end-up with corrupt TLBs if they
speculate an AT instruction in during a guest switch while the
S1/S2 system registers are in an inconsistent state.

Work around it by:
- Mandating VHE
- Make sure that S1 and S2 system registers are consistent before
  clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
  regime

These two things together ensure that we cannot hit this erratum.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig                     | 12 ++++++++++++
 arch/arm64/include/asm/cpucaps.h       |  3 ++-
 arch/arm64/include/asm/kvm_host.h      |  3 +++
 arch/arm64/include/asm/kvm_hyp.h       |  6 ++++++
 arch/arm64/kernel/cpu_errata.c         |  8 ++++++++
 arch/arm64/kvm/hyp/switch.c            | 14 ++++++++++++++
 7 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
index 76ccded8b74c..04f0bc4690c6 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -57,6 +57,7 @@ stable kernels.
 | ARM            | Cortex-A73      | #858921         | ARM64_ERRATUM_858921        |
 | ARM            | Cortex-A55      | #1024718        | ARM64_ERRATUM_1024718       |
 | ARM            | Cortex-A76      | #1188873        | ARM64_ERRATUM_1188873       |
+| ARM            | Cortex-A76      | #1165522        | ARM64_ERRATUM_1165522       |
 | ARM            | MMU-500         | #841119,#826419 | N/A                         |
 |                |                 |                 |                             |
 | Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375        |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 787d7850e064..a68bc6cc2167 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873
 
 	  If unsure, say Y.
 
+config ARM64_ERRATUM_1165522
+	bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation"
+	default y
+	help
+	  This option adds work arounds for ARM Cortex-A76 erratum 1165522
+
+	  Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with
+	  corrupted TLBs by speculating an AT instruction during a guest
+	  context switch.
+
+	  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 6e2d254c09eb..62d8cd15fdf2 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -54,7 +54,8 @@
 #define ARM64_HAS_CRC32				33
 #define ARM64_SSBS				34
 #define ARM64_WORKAROUND_1188873		35
+#define ARM64_WORKAROUND_1165522		36
 
-#define ARM64_NCAPS				36
+#define ARM64_NCAPS				37
 
 #endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 7d6e974d024a..8f486026ac87 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -435,6 +435,9 @@ static inline bool kvm_arch_sve_requires_vhe(void)
 static inline bool kvm_arch_impl_requires_vhe(void)
 {
 	/* Some implementations have defects that confine them to VHE */
+	if (cpus_have_cap(ARM64_WORKAROUND_1165522))
+		return true;
+
 	return false;
 }
 
diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
index 23aca66767f9..9681360d0959 100644
--- a/arch/arm64/include/asm/kvm_hyp.h
+++ b/arch/arm64/include/asm/kvm_hyp.h
@@ -163,6 +163,12 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm)
 {
 	write_sysreg(kvm->arch.vtcr, vtcr_el2);
 	write_sysreg(kvm->arch.vttbr, vttbr_el2);
+
+	/*
+	 * ARM erratum 1165522 requires the actual execution of the
+	 * above before we can switch to the guest translation regime.
+	 */
+	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
 }
 
 #endif /* __ARM64_KVM_HYP_H__ */
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index a509e35132d2..476e738e6c46 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -739,6 +739,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		.capability = ARM64_WORKAROUND_1188873,
 		ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
 	},
+#endif
+#ifdef CONFIG_ARM64_ERRATUM_1165522
+	{
+		/* Cortex-A76 r0p0 to r2p0 */
+		.desc = "ARM erratum 1165522",
+		.capability = ARM64_WORKAROUND_1165522,
+		ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
+	},
 #endif
 	{
 	}
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 51d5d966d9e5..322109183853 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void)
 {
 	extern char vectors[];	/* kernel exception vectors */
 	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
+
+	/*
+	 * ARM erratum 1165522 requires the actual execution of the
+	 * above before we can switch to the host translation regime.
+	 */
+	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
+
 	write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
 	write_sysreg(vectors, vbar_el1);
 }
@@ -499,6 +506,13 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
 
 	sysreg_save_host_state_vhe(host_ctxt);
 
+	/*
+	 * ARM erratum 1165522 requires us to have all the translation
+	 * context in place before we clear HCR_TGE. All the offending
+	 * guest sysregs are loaded in kvm_vcpu_load_sysregs, and
+	 * __activate_vm has the stage-2 configuration. Once this is
+	 * done, __activate_trap clears HCR_TGE (among other things).
+	 */
 	__activate_vm(vcpu->kvm);
 	__activate_traps(vcpu);
 
-- 
2.19.1

Re: [PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[James Morse]

Hi Marc,

On 05/11/2018 14:36, Marc Zyngier wrote:
> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
> speculate an AT instruction in during a guest switch while the

                              (in during?)

> S1/S2 system registers are in an inconsistent state.
> 
> Work around it by:
> - Mandating VHE
> - Make sure that S1 and S2 system registers are consistent before
>    clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
>    regime
> 
> These two things together ensure that we cannot hit this erratum.


> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 51d5d966d9e5..322109183853 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void)
>   {
>   	extern char vectors[];	/* kernel exception vectors */
>   	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
> +
> +	/*
> +	 * ARM erratum 1165522 requires the actual execution of the
> +	 * above before we can switch to the host translation regime.
> +	 */
> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
> +

Host regime too ... does __tlb_switch_to_host_vhe() need the same 
treatment? It writes vttbr_el2 and hcr_el2 back to back.



Thanks,

James

[PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[James Morse]

Hi Marc,

On 05/11/2018 14:36, Marc Zyngier wrote:
> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
> speculate an AT instruction in during a guest switch while the

                              (in during?)

> S1/S2 system registers are in an inconsistent state.
> 
> Work around it by:
> - Mandating VHE
> - Make sure that S1 and S2 system registers are consistent before
>    clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
>    regime
> 
> These two things together ensure that we cannot hit this erratum.


> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 51d5d966d9e5..322109183853 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void)
>   {
>   	extern char vectors[];	/* kernel exception vectors */
>   	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
> +
> +	/*
> +	 * ARM erratum 1165522 requires the actual execution of the
> +	 * above before we can switch to the host translation regime.
> +	 */
> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
> +

Host regime too ... does __tlb_switch_to_host_vhe() need the same 
treatment? It writes vttbr_el2 and hcr_el2 back to back.



Thanks,

James

Re: [PATCH 1/4] KVM: arm64: Rework detection of SVE, !VHE systems

[Christoffer Dall]

On Mon, Nov 05, 2018 at 02:36:13PM +0000, Marc Zyngier wrote:
> An SVE system is so far the only case where we mandate VHE. As we're
> starting to grow this requirements, let's slightly rework the way we
> deal with that situation, allowing for easy extension of this check.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/include/asm/kvm_host.h   |  2 +-
>  arch/arm64/include/asm/kvm_host.h |  7 ++-----
>  virt/kvm/arm/arm.c                | 12 +++++++-----
>  3 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 5ca5d9af0c26..c3469729f40c 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -285,7 +285,7 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>  
>  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>  
> -static inline bool kvm_arch_check_sve_has_vhe(void) { return true; }
> +static inline bool kvm_arch_sve_requires_vhe(void) { return false; }
>  static inline void kvm_arch_hardware_unsetup(void) {}
>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 52fbc823ff8c..ca1714148400 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -422,17 +422,14 @@ static inline void __cpu_init_hyp_mode(phys_addr_t pgd_ptr,
>  	}
>  }
>  
> -static inline bool kvm_arch_check_sve_has_vhe(void)
> +static inline bool kvm_arch_sve_requires_vhe(void)
>  {
>  	/*
>  	 * The Arm architecture specifies that implementation of SVE
>  	 * requires VHE also to be implemented.  The KVM code for arm64
>  	 * relies on this when SVE is present:
>  	 */
> -	if (system_supports_sve())
> -		return has_vhe();
> -	else
> -		return true;
> +	return system_supports_sve();
>  }

nit: Can we call this function 'kvm_arch_requires_vhe()' and let it test
for all the cases (maybe that's coming in future patches though).  I
just find this naming confusing.

>  
>  static inline void kvm_arch_hardware_unsetup(void) {}
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 23774970c9df..66de2efddfca 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1640,9 +1640,13 @@ int kvm_arch_init(void *opaque)
>  		return -ENODEV;
>  	}
>  
> -	if (!kvm_arch_check_sve_has_vhe()) {
> -		kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
> -		return -ENODEV;
> +	in_hyp_mode = is_kernel_in_hyp_mode();
> +
> +	if (!in_hyp_mode) {
> +		if (kvm_arch_sve_requires_vhe()) {
> +			kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
> +			return -ENODEV;
> +		}
>  	}
>  
>  	for_each_online_cpu(cpu) {
> @@ -1657,8 +1661,6 @@ int kvm_arch_init(void *opaque)
>  	if (err)
>  		return err;
>  
> -	in_hyp_mode = is_kernel_in_hyp_mode();
> -
>  	if (!in_hyp_mode) {
>  		err = init_hyp_mode();
>  		if (err)
> -- 
> 2.19.1
> 


Thanks,

    Christoffer

[PATCH 1/4] KVM: arm64: Rework detection of SVE, !VHE systems

[Christoffer Dall]

On Mon, Nov 05, 2018 at 02:36:13PM +0000, Marc Zyngier wrote:
> An SVE system is so far the only case where we mandate VHE. As we're
> starting to grow this requirements, let's slightly rework the way we
> deal with that situation, allowing for easy extension of this check.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/include/asm/kvm_host.h   |  2 +-
>  arch/arm64/include/asm/kvm_host.h |  7 ++-----
>  virt/kvm/arm/arm.c                | 12 +++++++-----
>  3 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 5ca5d9af0c26..c3469729f40c 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -285,7 +285,7 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>  
>  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>  
> -static inline bool kvm_arch_check_sve_has_vhe(void) { return true; }
> +static inline bool kvm_arch_sve_requires_vhe(void) { return false; }
>  static inline void kvm_arch_hardware_unsetup(void) {}
>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 52fbc823ff8c..ca1714148400 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -422,17 +422,14 @@ static inline void __cpu_init_hyp_mode(phys_addr_t pgd_ptr,
>  	}
>  }
>  
> -static inline bool kvm_arch_check_sve_has_vhe(void)
> +static inline bool kvm_arch_sve_requires_vhe(void)
>  {
>  	/*
>  	 * The Arm architecture specifies that implementation of SVE
>  	 * requires VHE also to be implemented.  The KVM code for arm64
>  	 * relies on this when SVE is present:
>  	 */
> -	if (system_supports_sve())
> -		return has_vhe();
> -	else
> -		return true;
> +	return system_supports_sve();
>  }

nit: Can we call this function 'kvm_arch_requires_vhe()' and let it test
for all the cases (maybe that's coming in future patches though).  I
just find this naming confusing.

>  
>  static inline void kvm_arch_hardware_unsetup(void) {}
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 23774970c9df..66de2efddfca 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1640,9 +1640,13 @@ int kvm_arch_init(void *opaque)
>  		return -ENODEV;
>  	}
>  
> -	if (!kvm_arch_check_sve_has_vhe()) {
> -		kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
> -		return -ENODEV;
> +	in_hyp_mode = is_kernel_in_hyp_mode();
> +
> +	if (!in_hyp_mode) {
> +		if (kvm_arch_sve_requires_vhe()) {
> +			kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
> +			return -ENODEV;
> +		}
>  	}
>  
>  	for_each_online_cpu(cpu) {
> @@ -1657,8 +1661,6 @@ int kvm_arch_init(void *opaque)
>  	if (err)
>  		return err;
>  
> -	in_hyp_mode = is_kernel_in_hyp_mode();
> -
>  	if (!in_hyp_mode) {
>  		err = init_hyp_mode();
>  		if (err)
> -- 
> 2.19.1
> 


Thanks,

    Christoffer

Re: [PATCH 2/4] KVM: arm64: Allow implementations to be confined to using VHE

[Christoffer Dall]

On Mon, Nov 05, 2018 at 02:36:14PM +0000, Marc Zyngier wrote:
> Some implementations may be forced to use VHE to work around HW
> errata, for example. Let's introduce a helper that checks for
> these cases.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/include/asm/kvm_host.h   | 1 +
>  arch/arm64/include/asm/kvm_host.h | 6 ++++++
>  virt/kvm/arm/arm.c                | 5 +++++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index c3469729f40c..0f2f782548cb 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -286,6 +286,7 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>  
>  static inline bool kvm_arch_sve_requires_vhe(void) { return false; }
> +static inline bool kvm_arch_impl_requires_vhe(void) { return false; }
>  static inline void kvm_arch_hardware_unsetup(void) {}
>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index ca1714148400..7d6e974d024a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -432,6 +432,12 @@ static inline bool kvm_arch_sve_requires_vhe(void)
>  	return system_supports_sve();
>  }
>  
> +static inline bool kvm_arch_impl_requires_vhe(void)
> +{
> +	/* Some implementations have defects that confine them to VHE */
> +	return false;
> +}
> +
>  static inline void kvm_arch_hardware_unsetup(void) {}
>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 66de2efddfca..bc90a1cdd27f 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1647,6 +1647,11 @@ int kvm_arch_init(void *opaque)
>  			kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
>  			return -ENODEV;
>  		}
> +
> +		if (kvm_arch_impl_requires_vhe()) {
> +			kvm_pr_unimpl("CPU requiring VHE in non-VHE mode");
> +			return -ENODEV;
> +		}

So as per my previous comment, I'd prefer this function to only issue a
single call, and have the function in kvm_host.h check for all the
conditions, do the printing, etc.

Thoughts?


Thanks,

    Christoffer

>  	}
>  
>  	for_each_online_cpu(cpu) {
> -- 
> 2.19.1
> 

[PATCH 2/4] KVM: arm64: Allow implementations to be confined to using VHE

[Christoffer Dall]

On Mon, Nov 05, 2018 at 02:36:14PM +0000, Marc Zyngier wrote:
> Some implementations may be forced to use VHE to work around HW
> errata, for example. Let's introduce a helper that checks for
> these cases.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/include/asm/kvm_host.h   | 1 +
>  arch/arm64/include/asm/kvm_host.h | 6 ++++++
>  virt/kvm/arm/arm.c                | 5 +++++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index c3469729f40c..0f2f782548cb 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -286,6 +286,7 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>  
>  static inline bool kvm_arch_sve_requires_vhe(void) { return false; }
> +static inline bool kvm_arch_impl_requires_vhe(void) { return false; }
>  static inline void kvm_arch_hardware_unsetup(void) {}
>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index ca1714148400..7d6e974d024a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -432,6 +432,12 @@ static inline bool kvm_arch_sve_requires_vhe(void)
>  	return system_supports_sve();
>  }
>  
> +static inline bool kvm_arch_impl_requires_vhe(void)
> +{
> +	/* Some implementations have defects that confine them to VHE */
> +	return false;
> +}
> +
>  static inline void kvm_arch_hardware_unsetup(void) {}
>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 66de2efddfca..bc90a1cdd27f 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1647,6 +1647,11 @@ int kvm_arch_init(void *opaque)
>  			kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
>  			return -ENODEV;
>  		}
> +
> +		if (kvm_arch_impl_requires_vhe()) {
> +			kvm_pr_unimpl("CPU requiring VHE in non-VHE mode");
> +			return -ENODEV;
> +		}

So as per my previous comment, I'd prefer this function to only issue a
single call, and have the function in kvm_host.h check for all the
conditions, do the printing, etc.

Thoughts?


Thanks,

    Christoffer

>  	}
>  
>  	for_each_online_cpu(cpu) {
> -- 
> 2.19.1
> 

Re: [PATCH 3/4] arm64: KVM: Install stage-2 translation before enabling traps on VHE

[Christoffer Dall]

On Mon, Nov 05, 2018 at 02:36:15PM +0000, Marc Zyngier wrote:
> It is a bit odd that we only install stage-2 translation after having
> cleared HCR_EL2.TGE, which means that there is a window during which
> AT requests could fail as stage-2 is not configured yet.
> 
> Let's move stage-2 configuration before we clear TGE, making the
> guest entry sequence clearer: we first configure all the guest stuff,
> then only switch to the guest translation regime.
> 
> Non-VHE doesn't have that kind of behaviour, and is left alone.

I'm a bit confused about this statement.  You can still issue a S12E1x
AT instruction after activating traps (setting HCR_EL2.VM) on non-VHE
and get at the same behavior, right?

Is the point here that we are not aware of any non-VHE implementations
that speculate AT instructions in this window, or am I missing some
architectural nugget that prevents problems on non-VHE systems?

In any case, why not change the non-VHE code as well to preserve
symmetry for both types of systems?


Thanks,

    Christoffer

> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm64/kvm/hyp/switch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 7cc175c88a37..51d5d966d9e5 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -499,8 +499,8 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>  
>  	sysreg_save_host_state_vhe(host_ctxt);
>  
> -	__activate_traps(vcpu);
>  	__activate_vm(vcpu->kvm);
> +	__activate_traps(vcpu);
>  
>  	sysreg_restore_guest_state_vhe(guest_ctxt);
>  	__debug_switch_to_guest(vcpu);
> -- 
> 2.19.1
> 

[PATCH 3/4] arm64: KVM: Install stage-2 translation before enabling traps on VHE

[Christoffer Dall]

On Mon, Nov 05, 2018 at 02:36:15PM +0000, Marc Zyngier wrote:
> It is a bit odd that we only install stage-2 translation after having
> cleared HCR_EL2.TGE, which means that there is a window during which
> AT requests could fail as stage-2 is not configured yet.
> 
> Let's move stage-2 configuration before we clear TGE, making the
> guest entry sequence clearer: we first configure all the guest stuff,
> then only switch to the guest translation regime.
> 
> Non-VHE doesn't have that kind of behaviour, and is left alone.

I'm a bit confused about this statement.  You can still issue a S12E1x
AT instruction after activating traps (setting HCR_EL2.VM) on non-VHE
and get at the same behavior, right?

Is the point here that we are not aware of any non-VHE implementations
that speculate AT instructions in this window, or am I missing some
architectural nugget that prevents problems on non-VHE systems?

In any case, why not change the non-VHE code as well to preserve
symmetry for both types of systems?


Thanks,

    Christoffer

> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm64/kvm/hyp/switch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 7cc175c88a37..51d5d966d9e5 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -499,8 +499,8 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>  
>  	sysreg_save_host_state_vhe(host_ctxt);
>  
> -	__activate_traps(vcpu);
>  	__activate_vm(vcpu->kvm);
> +	__activate_traps(vcpu);
>  
>  	sysreg_restore_guest_state_vhe(guest_ctxt);
>  	__debug_switch_to_guest(vcpu);
> -- 
> 2.19.1
> 

Re: [PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[Christoffer Dall]

On Mon, Nov 05, 2018 at 02:36:16PM +0000, Marc Zyngier wrote:
> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
> speculate an AT instruction in during a guest switch while the
> S1/S2 system registers are in an inconsistent state.
> 
> Work around it by:
> - Mandating VHE
> - Make sure that S1 and S2 system registers are consistent before
>   clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
>   regime
> 
> These two things together ensure that we cannot hit this erratum.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  Documentation/arm64/silicon-errata.txt |  1 +
>  arch/arm64/Kconfig                     | 12 ++++++++++++
>  arch/arm64/include/asm/cpucaps.h       |  3 ++-
>  arch/arm64/include/asm/kvm_host.h      |  3 +++
>  arch/arm64/include/asm/kvm_hyp.h       |  6 ++++++
>  arch/arm64/kernel/cpu_errata.c         |  8 ++++++++
>  arch/arm64/kvm/hyp/switch.c            | 14 ++++++++++++++
>  7 files changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
> index 76ccded8b74c..04f0bc4690c6 100644
> --- a/Documentation/arm64/silicon-errata.txt
> +++ b/Documentation/arm64/silicon-errata.txt
> @@ -57,6 +57,7 @@ stable kernels.
>  | ARM            | Cortex-A73      | #858921         | ARM64_ERRATUM_858921        |
>  | ARM            | Cortex-A55      | #1024718        | ARM64_ERRATUM_1024718       |
>  | ARM            | Cortex-A76      | #1188873        | ARM64_ERRATUM_1188873       |
> +| ARM            | Cortex-A76      | #1165522        | ARM64_ERRATUM_1165522       |
>  | ARM            | MMU-500         | #841119,#826419 | N/A                         |
>  |                |                 |                 |                             |
>  | Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375        |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 787d7850e064..a68bc6cc2167 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873
>  
>  	  If unsure, say Y.
>  
> +config ARM64_ERRATUM_1165522
> +	bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation"
> +	default y
> +	help
> +	  This option adds work arounds for ARM Cortex-A76 erratum 1165522
> +
> +	  Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with
> +	  corrupted TLBs by speculating an AT instruction during a guest
> +	  context switch.
> +
> +	  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 6e2d254c09eb..62d8cd15fdf2 100644
> --- a/arch/arm64/include/asm/cpucaps.h
> +++ b/arch/arm64/include/asm/cpucaps.h
> @@ -54,7 +54,8 @@
>  #define ARM64_HAS_CRC32				33
>  #define ARM64_SSBS				34
>  #define ARM64_WORKAROUND_1188873		35
> +#define ARM64_WORKAROUND_1165522		36
>  
> -#define ARM64_NCAPS				36
> +#define ARM64_NCAPS				37
>  
>  #endif /* __ASM_CPUCAPS_H */
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 7d6e974d024a..8f486026ac87 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -435,6 +435,9 @@ static inline bool kvm_arch_sve_requires_vhe(void)
>  static inline bool kvm_arch_impl_requires_vhe(void)
>  {
>  	/* Some implementations have defects that confine them to VHE */
> +	if (cpus_have_cap(ARM64_WORKAROUND_1165522))
> +		return true;
> +
>  	return false;
>  }
>  
> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> index 23aca66767f9..9681360d0959 100644
> --- a/arch/arm64/include/asm/kvm_hyp.h
> +++ b/arch/arm64/include/asm/kvm_hyp.h
> @@ -163,6 +163,12 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm)
>  {
>  	write_sysreg(kvm->arch.vtcr, vtcr_el2);
>  	write_sysreg(kvm->arch.vttbr, vttbr_el2);
> +
> +	/*
> +	 * ARM erratum 1165522 requires the actual execution of the
> +	 * above before we can switch to the guest translation regime.
> +	 */

Is it about a guest translation 'regime' or should this just say before
we can enable stage 2 translation?

> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
>  }
>  
>  #endif /* __ARM64_KVM_HYP_H__ */
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index a509e35132d2..476e738e6c46 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -739,6 +739,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>  		.capability = ARM64_WORKAROUND_1188873,
>  		ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
>  	},
> +#endif
> +#ifdef CONFIG_ARM64_ERRATUM_1165522
> +	{
> +		/* Cortex-A76 r0p0 to r2p0 */
> +		.desc = "ARM erratum 1165522",
> +		.capability = ARM64_WORKAROUND_1165522,
> +		ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
> +	},
>  #endif
>  	{
>  	}
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 51d5d966d9e5..322109183853 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void)
>  {
>  	extern char vectors[];	/* kernel exception vectors */
>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
> +
> +	/*
> +	 * ARM erratum 1165522 requires the actual execution of the
> +	 * above before we can switch to the host translation regime.
> +	 */

same here, is it not rather about disabling stage 2 translation than
about the host (EL2 and EL0 stage 1) translation regimes?

> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
> +
>  	write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
>  	write_sysreg(vectors, vbar_el1);
>  }
> @@ -499,6 +506,13 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>  
>  	sysreg_save_host_state_vhe(host_ctxt);
>  
> +	/*
> +	 * ARM erratum 1165522 requires us to have all the translation
> +	 * context in place before we clear HCR_TGE. All the offending
> +	 * guest sysregs are loaded in kvm_vcpu_load_sysregs, and
> +	 * __activate_vm has the stage-2 configuration. Once this is
> +	 * done, __activate_trap clears HCR_TGE (among other things).
> +	 */

I'm not this comment is needed or is helpful here.  For example, I don't
understand what you mean with the offending guest sysregs and how that
relates to the problem of configuring stage 2 before clearing TGE.  Is
this about gettting the stage 1 configuration in place first?

If so, could I suggest a reword along the lines of:

	/*
	 * ARM erratum 1165522 requires us to configure both stage 1 and
	 * stage 2 translation for the guest context before we clear
	 * HCR_EL2.TGE.
	 *
	 * We have already configured the guest's stage 1 translation in
	 * kvm_vcpu_load_sysregs above.  We must now call __activate_vm
	 * before __activate_traps, because __activate_vm configures
	 * stage 2 translation, and __activate_traps clear HCR_EL2.TGE
	 * (among other things).
	 */
	 

>  	__activate_vm(vcpu->kvm);
>  	__activate_traps(vcpu);
>  
> -- 
> 2.19.1
> 

Thanks,

    Christoffer

[PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[Christoffer Dall]

On Mon, Nov 05, 2018 at 02:36:16PM +0000, Marc Zyngier wrote:
> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
> speculate an AT instruction in during a guest switch while the
> S1/S2 system registers are in an inconsistent state.
> 
> Work around it by:
> - Mandating VHE
> - Make sure that S1 and S2 system registers are consistent before
>   clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
>   regime
> 
> These two things together ensure that we cannot hit this erratum.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  Documentation/arm64/silicon-errata.txt |  1 +
>  arch/arm64/Kconfig                     | 12 ++++++++++++
>  arch/arm64/include/asm/cpucaps.h       |  3 ++-
>  arch/arm64/include/asm/kvm_host.h      |  3 +++
>  arch/arm64/include/asm/kvm_hyp.h       |  6 ++++++
>  arch/arm64/kernel/cpu_errata.c         |  8 ++++++++
>  arch/arm64/kvm/hyp/switch.c            | 14 ++++++++++++++
>  7 files changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
> index 76ccded8b74c..04f0bc4690c6 100644
> --- a/Documentation/arm64/silicon-errata.txt
> +++ b/Documentation/arm64/silicon-errata.txt
> @@ -57,6 +57,7 @@ stable kernels.
>  | ARM            | Cortex-A73      | #858921         | ARM64_ERRATUM_858921        |
>  | ARM            | Cortex-A55      | #1024718        | ARM64_ERRATUM_1024718       |
>  | ARM            | Cortex-A76      | #1188873        | ARM64_ERRATUM_1188873       |
> +| ARM            | Cortex-A76      | #1165522        | ARM64_ERRATUM_1165522       |
>  | ARM            | MMU-500         | #841119,#826419 | N/A                         |
>  |                |                 |                 |                             |
>  | Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375        |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 787d7850e064..a68bc6cc2167 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873
>  
>  	  If unsure, say Y.
>  
> +config ARM64_ERRATUM_1165522
> +	bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation"
> +	default y
> +	help
> +	  This option adds work arounds for ARM Cortex-A76 erratum 1165522
> +
> +	  Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with
> +	  corrupted TLBs by speculating an AT instruction during a guest
> +	  context switch.
> +
> +	  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 6e2d254c09eb..62d8cd15fdf2 100644
> --- a/arch/arm64/include/asm/cpucaps.h
> +++ b/arch/arm64/include/asm/cpucaps.h
> @@ -54,7 +54,8 @@
>  #define ARM64_HAS_CRC32				33
>  #define ARM64_SSBS				34
>  #define ARM64_WORKAROUND_1188873		35
> +#define ARM64_WORKAROUND_1165522		36
>  
> -#define ARM64_NCAPS				36
> +#define ARM64_NCAPS				37
>  
>  #endif /* __ASM_CPUCAPS_H */
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 7d6e974d024a..8f486026ac87 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -435,6 +435,9 @@ static inline bool kvm_arch_sve_requires_vhe(void)
>  static inline bool kvm_arch_impl_requires_vhe(void)
>  {
>  	/* Some implementations have defects that confine them to VHE */
> +	if (cpus_have_cap(ARM64_WORKAROUND_1165522))
> +		return true;
> +
>  	return false;
>  }
>  
> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> index 23aca66767f9..9681360d0959 100644
> --- a/arch/arm64/include/asm/kvm_hyp.h
> +++ b/arch/arm64/include/asm/kvm_hyp.h
> @@ -163,6 +163,12 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm)
>  {
>  	write_sysreg(kvm->arch.vtcr, vtcr_el2);
>  	write_sysreg(kvm->arch.vttbr, vttbr_el2);
> +
> +	/*
> +	 * ARM erratum 1165522 requires the actual execution of the
> +	 * above before we can switch to the guest translation regime.
> +	 */

Is it about a guest translation 'regime' or should this just say before
we can enable stage 2 translation?

> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
>  }
>  
>  #endif /* __ARM64_KVM_HYP_H__ */
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index a509e35132d2..476e738e6c46 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -739,6 +739,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>  		.capability = ARM64_WORKAROUND_1188873,
>  		ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
>  	},
> +#endif
> +#ifdef CONFIG_ARM64_ERRATUM_1165522
> +	{
> +		/* Cortex-A76 r0p0 to r2p0 */
> +		.desc = "ARM erratum 1165522",
> +		.capability = ARM64_WORKAROUND_1165522,
> +		ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
> +	},
>  #endif
>  	{
>  	}
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 51d5d966d9e5..322109183853 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void)
>  {
>  	extern char vectors[];	/* kernel exception vectors */
>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
> +
> +	/*
> +	 * ARM erratum 1165522 requires the actual execution of the
> +	 * above before we can switch to the host translation regime.
> +	 */

same here, is it not rather about disabling stage 2 translation than
about the host (EL2 and EL0 stage 1) translation regimes?

> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
> +
>  	write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
>  	write_sysreg(vectors, vbar_el1);
>  }
> @@ -499,6 +506,13 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>  
>  	sysreg_save_host_state_vhe(host_ctxt);
>  
> +	/*
> +	 * ARM erratum 1165522 requires us to have all the translation
> +	 * context in place before we clear HCR_TGE. All the offending
> +	 * guest sysregs are loaded in kvm_vcpu_load_sysregs, and
> +	 * __activate_vm has the stage-2 configuration. Once this is
> +	 * done, __activate_trap clears HCR_TGE (among other things).
> +	 */

I'm not this comment is needed or is helpful here.  For example, I don't
understand what you mean with the offending guest sysregs and how that
relates to the problem of configuring stage 2 before clearing TGE.  Is
this about gettting the stage 1 configuration in place first?

If so, could I suggest a reword along the lines of:

	/*
	 * ARM erratum 1165522 requires us to configure both stage 1 and
	 * stage 2 translation for the guest context before we clear
	 * HCR_EL2.TGE.
	 *
	 * We have already configured the guest's stage 1 translation in
	 * kvm_vcpu_load_sysregs above.  We must now call __activate_vm
	 * before __activate_traps, because __activate_vm configures
	 * stage 2 translation, and __activate_traps clear HCR_EL2.TGE
	 * (among other things).
	 */
	 

>  	__activate_vm(vcpu->kvm);
>  	__activate_traps(vcpu);
>  
> -- 
> 2.19.1
> 

Thanks,

    Christoffer

Re: [PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[Marc Zyngier]

On 05/11/18 18:34, James Morse wrote:
> Hi Marc,
> 
> On 05/11/2018 14:36, Marc Zyngier wrote:
>> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
>> speculate an AT instruction in during a guest switch while the
> 
>                               (in during?)
> 
>> S1/S2 system registers are in an inconsistent state.
>>
>> Work around it by:
>> - Mandating VHE
>> - Make sure that S1 and S2 system registers are consistent before
>>    clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
>>    regime
>>
>> These two things together ensure that we cannot hit this erratum.
> 
> 
>> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
>> index 51d5d966d9e5..322109183853 100644
>> --- a/arch/arm64/kvm/hyp/switch.c
>> +++ b/arch/arm64/kvm/hyp/switch.c
>> @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void)
>>   {
>>   	extern char vectors[];	/* kernel exception vectors */
>>   	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
>> +
>> +	/*
>> +	 * ARM erratum 1165522 requires the actual execution of the
>> +	 * above before we can switch to the host translation regime.
>> +	 */
>> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
>> +
> 
> Host regime too ... does __tlb_switch_to_host_vhe() need the same 
> treatment? It writes vttbr_el2 and hcr_el2 back to back.

It turns out that our VHE TLB invalidation are a tiny bit broken, and
that's before we work around this very erratum.

You're perfectly right that we're mitting an ISB in
__tlb_switch_to_host_vhe(). We also have the problem that we can
perfectly take an interrupt here, and maybe schedule another process
from there (very unlikely, but I couldn't fully convince myself that it
couldn't happen).

What I'm planning to do is to make these TLB invalidation sequence
atomic by disabling interrupts. Yes, this is quite a hammer, but that'
no different from !VHE, and that's a very rare event anyway.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[Marc Zyngier]

On 05/11/18 18:34, James Morse wrote:
> Hi Marc,
> 
> On 05/11/2018 14:36, Marc Zyngier wrote:
>> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
>> speculate an AT instruction in during a guest switch while the
> 
>                               (in during?)
> 
>> S1/S2 system registers are in an inconsistent state.
>>
>> Work around it by:
>> - Mandating VHE
>> - Make sure that S1 and S2 system registers are consistent before
>>    clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
>>    regime
>>
>> These two things together ensure that we cannot hit this erratum.
> 
> 
>> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
>> index 51d5d966d9e5..322109183853 100644
>> --- a/arch/arm64/kvm/hyp/switch.c
>> +++ b/arch/arm64/kvm/hyp/switch.c
>> @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void)
>>   {
>>   	extern char vectors[];	/* kernel exception vectors */
>>   	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
>> +
>> +	/*
>> +	 * ARM erratum 1165522 requires the actual execution of the
>> +	 * above before we can switch to the host translation regime.
>> +	 */
>> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
>> +
> 
> Host regime too ... does __tlb_switch_to_host_vhe() need the same 
> treatment? It writes vttbr_el2 and hcr_el2 back to back.

It turns out that our VHE TLB invalidation are a tiny bit broken, and
that's before we work around this very erratum.

You're perfectly right that we're mitting an ISB in
__tlb_switch_to_host_vhe(). We also have the problem that we can
perfectly take an interrupt here, and maybe schedule another process
from there (very unlikely, but I couldn't fully convince myself that it
couldn't happen).

What I'm planning to do is to make these TLB invalidation sequence
atomic by disabling interrupts. Yes, this is quite a hammer, but that'
no different from !VHE, and that's a very rare event anyway.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 2/4] KVM: arm64: Allow implementations to be confined to using VHE

[Marc Zyngier]

On 06/11/18 07:53, Christoffer Dall wrote:
> On Mon, Nov 05, 2018 at 02:36:14PM +0000, Marc Zyngier wrote:
>> Some implementations may be forced to use VHE to work around HW
>> errata, for example. Let's introduce a helper that checks for
>> these cases.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  arch/arm/include/asm/kvm_host.h   | 1 +
>>  arch/arm64/include/asm/kvm_host.h | 6 ++++++
>>  virt/kvm/arm/arm.c                | 5 +++++
>>  3 files changed, 12 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>> index c3469729f40c..0f2f782548cb 100644
>> --- a/arch/arm/include/asm/kvm_host.h
>> +++ b/arch/arm/include/asm/kvm_host.h
>> @@ -286,6 +286,7 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>>  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>>  
>>  static inline bool kvm_arch_sve_requires_vhe(void) { return false; }
>> +static inline bool kvm_arch_impl_requires_vhe(void) { return false; }
>>  static inline void kvm_arch_hardware_unsetup(void) {}
>>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>> index ca1714148400..7d6e974d024a 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -432,6 +432,12 @@ static inline bool kvm_arch_sve_requires_vhe(void)
>>  	return system_supports_sve();
>>  }
>>  
>> +static inline bool kvm_arch_impl_requires_vhe(void)
>> +{
>> +	/* Some implementations have defects that confine them to VHE */
>> +	return false;
>> +}
>> +
>>  static inline void kvm_arch_hardware_unsetup(void) {}
>>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
>> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
>> index 66de2efddfca..bc90a1cdd27f 100644
>> --- a/virt/kvm/arm/arm.c
>> +++ b/virt/kvm/arm/arm.c
>> @@ -1647,6 +1647,11 @@ int kvm_arch_init(void *opaque)
>>  			kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
>>  			return -ENODEV;
>>  		}
>> +
>> +		if (kvm_arch_impl_requires_vhe()) {
>> +			kvm_pr_unimpl("CPU requiring VHE in non-VHE mode");
>> +			return -ENODEV;
>> +		}
> 
> So as per my previous comment, I'd prefer this function to only issue a
> single call, and have the function in kvm_host.h check for all the
> conditions, do the printing, etc.
> 
> Thoughts?

Indeed, this seems a sensible alternative.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 2/4] KVM: arm64: Allow implementations to be confined to using VHE

[Marc Zyngier]

On 06/11/18 07:53, Christoffer Dall wrote:
> On Mon, Nov 05, 2018 at 02:36:14PM +0000, Marc Zyngier wrote:
>> Some implementations may be forced to use VHE to work around HW
>> errata, for example. Let's introduce a helper that checks for
>> these cases.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  arch/arm/include/asm/kvm_host.h   | 1 +
>>  arch/arm64/include/asm/kvm_host.h | 6 ++++++
>>  virt/kvm/arm/arm.c                | 5 +++++
>>  3 files changed, 12 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>> index c3469729f40c..0f2f782548cb 100644
>> --- a/arch/arm/include/asm/kvm_host.h
>> +++ b/arch/arm/include/asm/kvm_host.h
>> @@ -286,6 +286,7 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>>  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>>  
>>  static inline bool kvm_arch_sve_requires_vhe(void) { return false; }
>> +static inline bool kvm_arch_impl_requires_vhe(void) { return false; }
>>  static inline void kvm_arch_hardware_unsetup(void) {}
>>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>> index ca1714148400..7d6e974d024a 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -432,6 +432,12 @@ static inline bool kvm_arch_sve_requires_vhe(void)
>>  	return system_supports_sve();
>>  }
>>  
>> +static inline bool kvm_arch_impl_requires_vhe(void)
>> +{
>> +	/* Some implementations have defects that confine them to VHE */
>> +	return false;
>> +}
>> +
>>  static inline void kvm_arch_hardware_unsetup(void) {}
>>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
>> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
>> index 66de2efddfca..bc90a1cdd27f 100644
>> --- a/virt/kvm/arm/arm.c
>> +++ b/virt/kvm/arm/arm.c
>> @@ -1647,6 +1647,11 @@ int kvm_arch_init(void *opaque)
>>  			kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
>>  			return -ENODEV;
>>  		}
>> +
>> +		if (kvm_arch_impl_requires_vhe()) {
>> +			kvm_pr_unimpl("CPU requiring VHE in non-VHE mode");
>> +			return -ENODEV;
>> +		}
> 
> So as per my previous comment, I'd prefer this function to only issue a
> single call, and have the function in kvm_host.h check for all the
> conditions, do the printing, etc.
> 
> Thoughts?

Indeed, this seems a sensible alternative.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 3/4] arm64: KVM: Install stage-2 translation before enabling traps on VHE

[Marc Zyngier]

On 06/11/18 08:06, Christoffer Dall wrote:
> On Mon, Nov 05, 2018 at 02:36:15PM +0000, Marc Zyngier wrote:
>> It is a bit odd that we only install stage-2 translation after having
>> cleared HCR_EL2.TGE, which means that there is a window during which
>> AT requests could fail as stage-2 is not configured yet.
>>
>> Let's move stage-2 configuration before we clear TGE, making the
>> guest entry sequence clearer: we first configure all the guest stuff,
>> then only switch to the guest translation regime.
>>
>> Non-VHE doesn't have that kind of behaviour, and is left alone.
> 
> I'm a bit confused about this statement.  You can still issue a S12E1x
> AT instruction after activating traps (setting HCR_EL2.VM) on non-VHE
> and get at the same behavior, right?
> 
> Is the point here that we are not aware of any non-VHE implementations
> that speculate AT instructions in this window, or am I missing some
> architectural nugget that prevents problems on non-VHE systems?

You're right. This is not an issue on non-VHE so far because we don't
know of any such system that is broken in such a way (speculative AT
instruction leading to inconsistent TLBs).

> In any case, why not change the non-VHE code as well to preserve
> symmetry for both types of systems?

Happy to change that too.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 3/4] arm64: KVM: Install stage-2 translation before enabling traps on VHE

[Marc Zyngier]

On 06/11/18 08:06, Christoffer Dall wrote:
> On Mon, Nov 05, 2018 at 02:36:15PM +0000, Marc Zyngier wrote:
>> It is a bit odd that we only install stage-2 translation after having
>> cleared HCR_EL2.TGE, which means that there is a window during which
>> AT requests could fail as stage-2 is not configured yet.
>>
>> Let's move stage-2 configuration before we clear TGE, making the
>> guest entry sequence clearer: we first configure all the guest stuff,
>> then only switch to the guest translation regime.
>>
>> Non-VHE doesn't have that kind of behaviour, and is left alone.
> 
> I'm a bit confused about this statement.  You can still issue a S12E1x
> AT instruction after activating traps (setting HCR_EL2.VM) on non-VHE
> and get at the same behavior, right?
> 
> Is the point here that we are not aware of any non-VHE implementations
> that speculate AT instructions in this window, or am I missing some
> architectural nugget that prevents problems on non-VHE systems?

You're right. This is not an issue on non-VHE so far because we don't
know of any such system that is broken in such a way (speculative AT
instruction leading to inconsistent TLBs).

> In any case, why not change the non-VHE code as well to preserve
> symmetry for both types of systems?

Happy to change that too.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[Marc Zyngier]

On 06/11/18 08:15, Christoffer Dall wrote:
> On Mon, Nov 05, 2018 at 02:36:16PM +0000, Marc Zyngier wrote:
>> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
>> speculate an AT instruction in during a guest switch while the
>> S1/S2 system registers are in an inconsistent state.
>>
>> Work around it by:
>> - Mandating VHE
>> - Make sure that S1 and S2 system registers are consistent before
>>   clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
>>   regime
>>
>> These two things together ensure that we cannot hit this erratum.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  Documentation/arm64/silicon-errata.txt |  1 +
>>  arch/arm64/Kconfig                     | 12 ++++++++++++
>>  arch/arm64/include/asm/cpucaps.h       |  3 ++-
>>  arch/arm64/include/asm/kvm_host.h      |  3 +++
>>  arch/arm64/include/asm/kvm_hyp.h       |  6 ++++++
>>  arch/arm64/kernel/cpu_errata.c         |  8 ++++++++
>>  arch/arm64/kvm/hyp/switch.c            | 14 ++++++++++++++
>>  7 files changed, 46 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
>> index 76ccded8b74c..04f0bc4690c6 100644
>> --- a/Documentation/arm64/silicon-errata.txt
>> +++ b/Documentation/arm64/silicon-errata.txt
>> @@ -57,6 +57,7 @@ stable kernels.
>>  | ARM            | Cortex-A73      | #858921         | ARM64_ERRATUM_858921        |
>>  | ARM            | Cortex-A55      | #1024718        | ARM64_ERRATUM_1024718       |
>>  | ARM            | Cortex-A76      | #1188873        | ARM64_ERRATUM_1188873       |
>> +| ARM            | Cortex-A76      | #1165522        | ARM64_ERRATUM_1165522       |
>>  | ARM            | MMU-500         | #841119,#826419 | N/A                         |
>>  |                |                 |                 |                             |
>>  | Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375        |
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 787d7850e064..a68bc6cc2167 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873
>>  
>>  	  If unsure, say Y.
>>  
>> +config ARM64_ERRATUM_1165522
>> +	bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation"
>> +	default y
>> +	help
>> +	  This option adds work arounds for ARM Cortex-A76 erratum 1165522
>> +
>> +	  Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with
>> +	  corrupted TLBs by speculating an AT instruction during a guest
>> +	  context switch.
>> +
>> +	  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 6e2d254c09eb..62d8cd15fdf2 100644
>> --- a/arch/arm64/include/asm/cpucaps.h
>> +++ b/arch/arm64/include/asm/cpucaps.h
>> @@ -54,7 +54,8 @@
>>  #define ARM64_HAS_CRC32				33
>>  #define ARM64_SSBS				34
>>  #define ARM64_WORKAROUND_1188873		35
>> +#define ARM64_WORKAROUND_1165522		36
>>  
>> -#define ARM64_NCAPS				36
>> +#define ARM64_NCAPS				37
>>  
>>  #endif /* __ASM_CPUCAPS_H */
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>> index 7d6e974d024a..8f486026ac87 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -435,6 +435,9 @@ static inline bool kvm_arch_sve_requires_vhe(void)
>>  static inline bool kvm_arch_impl_requires_vhe(void)
>>  {
>>  	/* Some implementations have defects that confine them to VHE */
>> +	if (cpus_have_cap(ARM64_WORKAROUND_1165522))
>> +		return true;
>> +
>>  	return false;
>>  }
>>  
>> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
>> index 23aca66767f9..9681360d0959 100644
>> --- a/arch/arm64/include/asm/kvm_hyp.h
>> +++ b/arch/arm64/include/asm/kvm_hyp.h
>> @@ -163,6 +163,12 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm)
>>  {
>>  	write_sysreg(kvm->arch.vtcr, vtcr_el2);
>>  	write_sysreg(kvm->arch.vttbr, vttbr_el2);
>> +
>> +	/*
>> +	 * ARM erratum 1165522 requires the actual execution of the
>> +	 * above before we can switch to the guest translation regime.
>> +	 */
> 
> Is it about a guest translation 'regime' or should this just say before
> we can enable stage 2 translation?

No, this isn't strictly about enabling stage-2 translation. This is
about making sure that anything that impacts the guest translations is
actually executed.

I wonder if it would be clearer to move this outside of
__load_guest_stage2 and make it explicit in the callers of this helper...

Thoughts?

> 
>> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
>>  }
>>  
>>  #endif /* __ARM64_KVM_HYP_H__ */
>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>> index a509e35132d2..476e738e6c46 100644
>> --- a/arch/arm64/kernel/cpu_errata.c
>> +++ b/arch/arm64/kernel/cpu_errata.c
>> @@ -739,6 +739,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>>  		.capability = ARM64_WORKAROUND_1188873,
>>  		ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
>>  	},
>> +#endif
>> +#ifdef CONFIG_ARM64_ERRATUM_1165522
>> +	{
>> +		/* Cortex-A76 r0p0 to r2p0 */
>> +		.desc = "ARM erratum 1165522",
>> +		.capability = ARM64_WORKAROUND_1165522,
>> +		ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
>> +	},
>>  #endif
>>  	{
>>  	}
>> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
>> index 51d5d966d9e5..322109183853 100644
>> --- a/arch/arm64/kvm/hyp/switch.c
>> +++ b/arch/arm64/kvm/hyp/switch.c
>> @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void)
>>  {
>>  	extern char vectors[];	/* kernel exception vectors */
>>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
>> +
>> +	/*
>> +	 * ARM erratum 1165522 requires the actual execution of the
>> +	 * above before we can switch to the host translation regime.
>> +	 */
> 
> same here, is it not rather about disabling stage 2 translation than
> about the host (EL2 and EL0 stage 1) translation regimes?
> 
>> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
>> +
>>  	write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
>>  	write_sysreg(vectors, vbar_el1);
>>  }
>> @@ -499,6 +506,13 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>>  
>>  	sysreg_save_host_state_vhe(host_ctxt);
>>  
>> +	/*
>> +	 * ARM erratum 1165522 requires us to have all the translation
>> +	 * context in place before we clear HCR_TGE. All the offending
>> +	 * guest sysregs are loaded in kvm_vcpu_load_sysregs, and
>> +	 * __activate_vm has the stage-2 configuration. Once this is
>> +	 * done, __activate_trap clears HCR_TGE (among other things).
>> +	 */
> 
> I'm not this comment is needed or is helpful here.  For example, I don't
> understand what you mean with the offending guest sysregs and how that

TTBR*, TCR, SCTLR... Anything that deals with stage-1 translations.

> relates to the problem of configuring stage 2 before clearing TGE.  Is
> this about gettting the stage 1 configuration in place first?

Not only stage-1. Stage-2 is involved as well, as the CPU could
otherwise end-up with the wrong translations (bypassing stage-2 altogether).

> 
> If so, could I suggest a reword along the lines of:
> 
> 	/*
> 	 * ARM erratum 1165522 requires us to configure both stage 1 and
> 	 * stage 2 translation for the guest context before we clear
> 	 * HCR_EL2.TGE.
> 	 *
> 	 * We have already configured the guest's stage 1 translation in
> 	 * kvm_vcpu_load_sysregs above.  We must now call __activate_vm
> 	 * before __activate_traps, because __activate_vm configures
> 	 * stage 2 translation, and __activate_traps clear HCR_EL2.TGE
> 	 * (among other things).
> 	 */

Works for me (and shows that contrary to what you wrote above, you have
perfectly understood the problem)!.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[Marc Zyngier]

On 06/11/18 08:15, Christoffer Dall wrote:
> On Mon, Nov 05, 2018 at 02:36:16PM +0000, Marc Zyngier wrote:
>> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
>> speculate an AT instruction in during a guest switch while the
>> S1/S2 system registers are in an inconsistent state.
>>
>> Work around it by:
>> - Mandating VHE
>> - Make sure that S1 and S2 system registers are consistent before
>>   clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
>>   regime
>>
>> These two things together ensure that we cannot hit this erratum.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  Documentation/arm64/silicon-errata.txt |  1 +
>>  arch/arm64/Kconfig                     | 12 ++++++++++++
>>  arch/arm64/include/asm/cpucaps.h       |  3 ++-
>>  arch/arm64/include/asm/kvm_host.h      |  3 +++
>>  arch/arm64/include/asm/kvm_hyp.h       |  6 ++++++
>>  arch/arm64/kernel/cpu_errata.c         |  8 ++++++++
>>  arch/arm64/kvm/hyp/switch.c            | 14 ++++++++++++++
>>  7 files changed, 46 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
>> index 76ccded8b74c..04f0bc4690c6 100644
>> --- a/Documentation/arm64/silicon-errata.txt
>> +++ b/Documentation/arm64/silicon-errata.txt
>> @@ -57,6 +57,7 @@ stable kernels.
>>  | ARM            | Cortex-A73      | #858921         | ARM64_ERRATUM_858921        |
>>  | ARM            | Cortex-A55      | #1024718        | ARM64_ERRATUM_1024718       |
>>  | ARM            | Cortex-A76      | #1188873        | ARM64_ERRATUM_1188873       |
>> +| ARM            | Cortex-A76      | #1165522        | ARM64_ERRATUM_1165522       |
>>  | ARM            | MMU-500         | #841119,#826419 | N/A                         |
>>  |                |                 |                 |                             |
>>  | Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375        |
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 787d7850e064..a68bc6cc2167 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873
>>  
>>  	  If unsure, say Y.
>>  
>> +config ARM64_ERRATUM_1165522
>> +	bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation"
>> +	default y
>> +	help
>> +	  This option adds work arounds for ARM Cortex-A76 erratum 1165522
>> +
>> +	  Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with
>> +	  corrupted TLBs by speculating an AT instruction during a guest
>> +	  context switch.
>> +
>> +	  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 6e2d254c09eb..62d8cd15fdf2 100644
>> --- a/arch/arm64/include/asm/cpucaps.h
>> +++ b/arch/arm64/include/asm/cpucaps.h
>> @@ -54,7 +54,8 @@
>>  #define ARM64_HAS_CRC32				33
>>  #define ARM64_SSBS				34
>>  #define ARM64_WORKAROUND_1188873		35
>> +#define ARM64_WORKAROUND_1165522		36
>>  
>> -#define ARM64_NCAPS				36
>> +#define ARM64_NCAPS				37
>>  
>>  #endif /* __ASM_CPUCAPS_H */
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>> index 7d6e974d024a..8f486026ac87 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -435,6 +435,9 @@ static inline bool kvm_arch_sve_requires_vhe(void)
>>  static inline bool kvm_arch_impl_requires_vhe(void)
>>  {
>>  	/* Some implementations have defects that confine them to VHE */
>> +	if (cpus_have_cap(ARM64_WORKAROUND_1165522))
>> +		return true;
>> +
>>  	return false;
>>  }
>>  
>> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
>> index 23aca66767f9..9681360d0959 100644
>> --- a/arch/arm64/include/asm/kvm_hyp.h
>> +++ b/arch/arm64/include/asm/kvm_hyp.h
>> @@ -163,6 +163,12 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm)
>>  {
>>  	write_sysreg(kvm->arch.vtcr, vtcr_el2);
>>  	write_sysreg(kvm->arch.vttbr, vttbr_el2);
>> +
>> +	/*
>> +	 * ARM erratum 1165522 requires the actual execution of the
>> +	 * above before we can switch to the guest translation regime.
>> +	 */
> 
> Is it about a guest translation 'regime' or should this just say before
> we can enable stage 2 translation?

No, this isn't strictly about enabling stage-2 translation. This is
about making sure that anything that impacts the guest translations is
actually executed.

I wonder if it would be clearer to move this outside of
__load_guest_stage2 and make it explicit in the callers of this helper...

Thoughts?

> 
>> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
>>  }
>>  
>>  #endif /* __ARM64_KVM_HYP_H__ */
>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>> index a509e35132d2..476e738e6c46 100644
>> --- a/arch/arm64/kernel/cpu_errata.c
>> +++ b/arch/arm64/kernel/cpu_errata.c
>> @@ -739,6 +739,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>>  		.capability = ARM64_WORKAROUND_1188873,
>>  		ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
>>  	},
>> +#endif
>> +#ifdef CONFIG_ARM64_ERRATUM_1165522
>> +	{
>> +		/* Cortex-A76 r0p0 to r2p0 */
>> +		.desc = "ARM erratum 1165522",
>> +		.capability = ARM64_WORKAROUND_1165522,
>> +		ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
>> +	},
>>  #endif
>>  	{
>>  	}
>> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
>> index 51d5d966d9e5..322109183853 100644
>> --- a/arch/arm64/kvm/hyp/switch.c
>> +++ b/arch/arm64/kvm/hyp/switch.c
>> @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void)
>>  {
>>  	extern char vectors[];	/* kernel exception vectors */
>>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
>> +
>> +	/*
>> +	 * ARM erratum 1165522 requires the actual execution of the
>> +	 * above before we can switch to the host translation regime.
>> +	 */
> 
> same here, is it not rather about disabling stage 2 translation than
> about the host (EL2 and EL0 stage 1) translation regimes?
> 
>> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
>> +
>>  	write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
>>  	write_sysreg(vectors, vbar_el1);
>>  }
>> @@ -499,6 +506,13 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>>  
>>  	sysreg_save_host_state_vhe(host_ctxt);
>>  
>> +	/*
>> +	 * ARM erratum 1165522 requires us to have all the translation
>> +	 * context in place before we clear HCR_TGE. All the offending
>> +	 * guest sysregs are loaded in kvm_vcpu_load_sysregs, and
>> +	 * __activate_vm has the stage-2 configuration. Once this is
>> +	 * done, __activate_trap clears HCR_TGE (among other things).
>> +	 */
> 
> I'm not this comment is needed or is helpful here.  For example, I don't
> understand what you mean with the offending guest sysregs and how that

TTBR*, TCR, SCTLR... Anything that deals with stage-1 translations.

> relates to the problem of configuring stage 2 before clearing TGE.  Is
> this about gettting the stage 1 configuration in place first?

Not only stage-1. Stage-2 is involved as well, as the CPU could
otherwise end-up with the wrong translations (bypassing stage-2 altogether).

> 
> If so, could I suggest a reword along the lines of:
> 
> 	/*
> 	 * ARM erratum 1165522 requires us to configure both stage 1 and
> 	 * stage 2 translation for the guest context before we clear
> 	 * HCR_EL2.TGE.
> 	 *
> 	 * We have already configured the guest's stage 1 translation in
> 	 * kvm_vcpu_load_sysregs above.  We must now call __activate_vm
> 	 * before __activate_traps, because __activate_vm configures
> 	 * stage 2 translation, and __activate_traps clear HCR_EL2.TGE
> 	 * (among other things).
> 	 */

Works for me (and shows that contrary to what you wrote above, you have
perfectly understood the problem)!.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 1/4] KVM: arm64: Rework detection of SVE, !VHE systems

[Dave Martin]

On Tue, Nov 06, 2018 at 08:52:51AM +0100, Christoffer Dall wrote:
> On Mon, Nov 05, 2018 at 02:36:13PM +0000, Marc Zyngier wrote:
> > An SVE system is so far the only case where we mandate VHE. As we're
> > starting to grow this requirements, let's slightly rework the way we
> > deal with that situation, allowing for easy extension of this check.
> > 
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >  arch/arm/include/asm/kvm_host.h   |  2 +-
> >  arch/arm64/include/asm/kvm_host.h |  7 ++-----
> >  virt/kvm/arm/arm.c                | 12 +++++++-----
> >  3 files changed, 10 insertions(+), 11 deletions(-)
> > 

[...]

> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index 52fbc823ff8c..ca1714148400 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -422,17 +422,14 @@ static inline void __cpu_init_hyp_mode(phys_addr_t pgd_ptr,
> >  	}
> >  }
> >  
> > -static inline bool kvm_arch_check_sve_has_vhe(void)
> > +static inline bool kvm_arch_sve_requires_vhe(void)
> >  {
> >  	/*
> >  	 * The Arm architecture specifies that implementation of SVE
> >  	 * requires VHE also to be implemented.  The KVM code for arm64
> >  	 * relies on this when SVE is present:
> >  	 */
> > -	if (system_supports_sve())
> > -		return has_vhe();
> > -	else
> > -		return true;
> > +	return system_supports_sve();
> >  }
> 
> nit: Can we call this function 'kvm_arch_requires_vhe()' and let it test
> for all the cases (maybe that's coming in future patches though).  I
> just find this naming confusing.

Agreed.  I was never keen on my original name, and the logic was
somewhat backwards anyway.

"kvm_arch_requires_vhe()" is a reasonable name for a function that
returns true iff the kernel needs VHE as a consequence of some other
prior runtime decision.

> >  
> >  static inline void kvm_arch_hardware_unsetup(void) {}
> > diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> > index 23774970c9df..66de2efddfca 100644
> > --- a/virt/kvm/arm/arm.c
> > +++ b/virt/kvm/arm/arm.c
> > @@ -1640,9 +1640,13 @@ int kvm_arch_init(void *opaque)
> >  		return -ENODEV;
> >  	}
> >  
> > -	if (!kvm_arch_check_sve_has_vhe()) {
> > -		kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
> > -		return -ENODEV;
> > +	in_hyp_mode = is_kernel_in_hyp_mode();
> > +
> > +	if (!in_hyp_mode) {
> > +		if (kvm_arch_sve_requires_vhe()) {

Can we just have

	if (!in_hyp_mode && kvm_arch_requires_vhe()) {

That's less nesty but still readable (for me, at least).

Otherwise, Ack.


[...]

Cheers
---Dave

[PATCH 1/4] KVM: arm64: Rework detection of SVE, !VHE systems

[Dave Martin]

On Tue, Nov 06, 2018 at 08:52:51AM +0100, Christoffer Dall wrote:
> On Mon, Nov 05, 2018 at 02:36:13PM +0000, Marc Zyngier wrote:
> > An SVE system is so far the only case where we mandate VHE. As we're
> > starting to grow this requirements, let's slightly rework the way we
> > deal with that situation, allowing for easy extension of this check.
> > 
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >  arch/arm/include/asm/kvm_host.h   |  2 +-
> >  arch/arm64/include/asm/kvm_host.h |  7 ++-----
> >  virt/kvm/arm/arm.c                | 12 +++++++-----
> >  3 files changed, 10 insertions(+), 11 deletions(-)
> > 

[...]

> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index 52fbc823ff8c..ca1714148400 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -422,17 +422,14 @@ static inline void __cpu_init_hyp_mode(phys_addr_t pgd_ptr,
> >  	}
> >  }
> >  
> > -static inline bool kvm_arch_check_sve_has_vhe(void)
> > +static inline bool kvm_arch_sve_requires_vhe(void)
> >  {
> >  	/*
> >  	 * The Arm architecture specifies that implementation of SVE
> >  	 * requires VHE also to be implemented.  The KVM code for arm64
> >  	 * relies on this when SVE is present:
> >  	 */
> > -	if (system_supports_sve())
> > -		return has_vhe();
> > -	else
> > -		return true;
> > +	return system_supports_sve();
> >  }
> 
> nit: Can we call this function 'kvm_arch_requires_vhe()' and let it test
> for all the cases (maybe that's coming in future patches though).  I
> just find this naming confusing.

Agreed.  I was never keen on my original name, and the logic was
somewhat backwards anyway.

"kvm_arch_requires_vhe()" is a reasonable name for a function that
returns true iff the kernel needs VHE as a consequence of some other
prior runtime decision.

> >  
> >  static inline void kvm_arch_hardware_unsetup(void) {}
> > diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> > index 23774970c9df..66de2efddfca 100644
> > --- a/virt/kvm/arm/arm.c
> > +++ b/virt/kvm/arm/arm.c
> > @@ -1640,9 +1640,13 @@ int kvm_arch_init(void *opaque)
> >  		return -ENODEV;
> >  	}
> >  
> > -	if (!kvm_arch_check_sve_has_vhe()) {
> > -		kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
> > -		return -ENODEV;
> > +	in_hyp_mode = is_kernel_in_hyp_mode();
> > +
> > +	if (!in_hyp_mode) {
> > +		if (kvm_arch_sve_requires_vhe()) {

Can we just have

	if (!in_hyp_mode && kvm_arch_requires_vhe()) {

That's less nesty but still readable (for me, at least).

Otherwise, Ack.


[...]

Cheers
---Dave

Re: [PATCH 2/4] KVM: arm64: Allow implementations to be confined to using VHE

[Dave Martin]

On Mon, Nov 05, 2018 at 02:36:14PM +0000, Marc Zyngier wrote:
> Some implementations may be forced to use VHE to work around HW
> errata, for example. Let's introduce a helper that checks for
> these cases.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/include/asm/kvm_host.h   | 1 +
>  arch/arm64/include/asm/kvm_host.h | 6 ++++++
>  virt/kvm/arm/arm.c                | 5 +++++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index c3469729f40c..0f2f782548cb 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -286,6 +286,7 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>  
>  static inline bool kvm_arch_sve_requires_vhe(void) { return false; }
> +static inline bool kvm_arch_impl_requires_vhe(void) { return false; }
>  static inline void kvm_arch_hardware_unsetup(void) {}
>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index ca1714148400..7d6e974d024a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -432,6 +432,12 @@ static inline bool kvm_arch_sve_requires_vhe(void)
>  	return system_supports_sve();
>  }
>  
> +static inline bool kvm_arch_impl_requires_vhe(void)
> +{
> +	/* Some implementations have defects that confine them to VHE */
> +	return false;
> +}
> +
>  static inline void kvm_arch_hardware_unsetup(void) {}
>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 66de2efddfca..bc90a1cdd27f 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1647,6 +1647,11 @@ int kvm_arch_init(void *opaque)
>  			kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
>  			return -ENODEV;
>  		}
> +
> +		if (kvm_arch_impl_requires_vhe()) {
> +			kvm_pr_unimpl("CPU requiring VHE in non-VHE mode");

( Scratch my previous comment if unless you restructure the code so
there's only one nested if() here. )

Also, minor nit: it sounds like "VHE in non-VHE mode" is the thing this
CPU requires, which is somewhat confusing.

Does it work to word this as
"CPU requiring VHE was booted in non-VHE mode"

?

Cheers
---Dave

[PATCH 2/4] KVM: arm64: Allow implementations to be confined to using VHE

[Dave Martin]

On Mon, Nov 05, 2018 at 02:36:14PM +0000, Marc Zyngier wrote:
> Some implementations may be forced to use VHE to work around HW
> errata, for example. Let's introduce a helper that checks for
> these cases.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/include/asm/kvm_host.h   | 1 +
>  arch/arm64/include/asm/kvm_host.h | 6 ++++++
>  virt/kvm/arm/arm.c                | 5 +++++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index c3469729f40c..0f2f782548cb 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -286,6 +286,7 @@ void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
>  struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>  
>  static inline bool kvm_arch_sve_requires_vhe(void) { return false; }
> +static inline bool kvm_arch_impl_requires_vhe(void) { return false; }
>  static inline void kvm_arch_hardware_unsetup(void) {}
>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index ca1714148400..7d6e974d024a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -432,6 +432,12 @@ static inline bool kvm_arch_sve_requires_vhe(void)
>  	return system_supports_sve();
>  }
>  
> +static inline bool kvm_arch_impl_requires_vhe(void)
> +{
> +	/* Some implementations have defects that confine them to VHE */
> +	return false;
> +}
> +
>  static inline void kvm_arch_hardware_unsetup(void) {}
>  static inline void kvm_arch_sync_events(struct kvm *kvm) {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 66de2efddfca..bc90a1cdd27f 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1647,6 +1647,11 @@ int kvm_arch_init(void *opaque)
>  			kvm_pr_unimpl("SVE system without VHE unsupported.  Broken cpu?");
>  			return -ENODEV;
>  		}
> +
> +		if (kvm_arch_impl_requires_vhe()) {
> +			kvm_pr_unimpl("CPU requiring VHE in non-VHE mode");

( Scratch my previous comment if unless you restructure the code so
there's only one nested if() here. )

Also, minor nit: it sounds like "VHE in non-VHE mode" is the thing this
CPU requires, which is somewhat confusing.

Does it work to word this as
"CPU requiring VHE was booted in non-VHE mode"

?

Cheers
---Dave

Re: [PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[Christoffer Dall]

On Thu, Nov 08, 2018 at 06:05:55PM +0000, Marc Zyngier wrote:
> On 06/11/18 08:15, Christoffer Dall wrote:
> > On Mon, Nov 05, 2018 at 02:36:16PM +0000, Marc Zyngier wrote:
> >> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
> >> speculate an AT instruction in during a guest switch while the
> >> S1/S2 system registers are in an inconsistent state.
> >>
> >> Work around it by:
> >> - Mandating VHE
> >> - Make sure that S1 and S2 system registers are consistent before
> >>   clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
> >>   regime
> >>
> >> These two things together ensure that we cannot hit this erratum.
> >>
> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> >> ---
> >>  Documentation/arm64/silicon-errata.txt |  1 +
> >>  arch/arm64/Kconfig                     | 12 ++++++++++++
> >>  arch/arm64/include/asm/cpucaps.h       |  3 ++-
> >>  arch/arm64/include/asm/kvm_host.h      |  3 +++
> >>  arch/arm64/include/asm/kvm_hyp.h       |  6 ++++++
> >>  arch/arm64/kernel/cpu_errata.c         |  8 ++++++++
> >>  arch/arm64/kvm/hyp/switch.c            | 14 ++++++++++++++
> >>  7 files changed, 46 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
> >> index 76ccded8b74c..04f0bc4690c6 100644
> >> --- a/Documentation/arm64/silicon-errata.txt
> >> +++ b/Documentation/arm64/silicon-errata.txt
> >> @@ -57,6 +57,7 @@ stable kernels.
> >>  | ARM            | Cortex-A73      | #858921         | ARM64_ERRATUM_858921        |
> >>  | ARM            | Cortex-A55      | #1024718        | ARM64_ERRATUM_1024718       |
> >>  | ARM            | Cortex-A76      | #1188873        | ARM64_ERRATUM_1188873       |
> >> +| ARM            | Cortex-A76      | #1165522        | ARM64_ERRATUM_1165522       |
> >>  | ARM            | MMU-500         | #841119,#826419 | N/A                         |
> >>  |                |                 |                 |                             |
> >>  | Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375        |
> >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> >> index 787d7850e064..a68bc6cc2167 100644
> >> --- a/arch/arm64/Kconfig
> >> +++ b/arch/arm64/Kconfig
> >> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873
> >>  
> >>  	  If unsure, say Y.
> >>  
> >> +config ARM64_ERRATUM_1165522
> >> +	bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation"
> >> +	default y
> >> +	help
> >> +	  This option adds work arounds for ARM Cortex-A76 erratum 1165522
> >> +
> >> +	  Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with
> >> +	  corrupted TLBs by speculating an AT instruction during a guest
> >> +	  context switch.
> >> +
> >> +	  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 6e2d254c09eb..62d8cd15fdf2 100644
> >> --- a/arch/arm64/include/asm/cpucaps.h
> >> +++ b/arch/arm64/include/asm/cpucaps.h
> >> @@ -54,7 +54,8 @@
> >>  #define ARM64_HAS_CRC32				33
> >>  #define ARM64_SSBS				34
> >>  #define ARM64_WORKAROUND_1188873		35
> >> +#define ARM64_WORKAROUND_1165522		36
> >>  
> >> -#define ARM64_NCAPS				36
> >> +#define ARM64_NCAPS				37
> >>  
> >>  #endif /* __ASM_CPUCAPS_H */
> >> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> >> index 7d6e974d024a..8f486026ac87 100644
> >> --- a/arch/arm64/include/asm/kvm_host.h
> >> +++ b/arch/arm64/include/asm/kvm_host.h
> >> @@ -435,6 +435,9 @@ static inline bool kvm_arch_sve_requires_vhe(void)
> >>  static inline bool kvm_arch_impl_requires_vhe(void)
> >>  {
> >>  	/* Some implementations have defects that confine them to VHE */
> >> +	if (cpus_have_cap(ARM64_WORKAROUND_1165522))
> >> +		return true;
> >> +
> >>  	return false;
> >>  }
> >>  
> >> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> >> index 23aca66767f9..9681360d0959 100644
> >> --- a/arch/arm64/include/asm/kvm_hyp.h
> >> +++ b/arch/arm64/include/asm/kvm_hyp.h
> >> @@ -163,6 +163,12 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm)
> >>  {
> >>  	write_sysreg(kvm->arch.vtcr, vtcr_el2);
> >>  	write_sysreg(kvm->arch.vttbr, vttbr_el2);
> >> +
> >> +	/*
> >> +	 * ARM erratum 1165522 requires the actual execution of the
> >> +	 * above before we can switch to the guest translation regime.
> >> +	 */
> > 
> > Is it about a guest translation 'regime' or should this just say before
> > we can enable stage 2 translation?
> 
> No, this isn't strictly about enabling stage-2 translation. This is
> about making sure that anything that impacts the guest translations is
> actually executed.
> 
> I wonder if it would be clearer to move this outside of
> __load_guest_stage2 and make it explicit in the callers of this helper...

I think it makes sense to have this here to explain the alternative.

But it's the 'switch to guest translation regime' thing that bothers me
a bit.  Is that an architectural concept (I thought we only had EL1 and
EL2 translation regimes in stage 1, and then stage 2 translations).  So
When you say 'guest translation regime' I'm just not entirely sure what
that means, unless I'm missing something.

> > 
> >> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
> >>  }
> >>  
> >>  #endif /* __ARM64_KVM_HYP_H__ */
> >> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> >> index a509e35132d2..476e738e6c46 100644
> >> --- a/arch/arm64/kernel/cpu_errata.c
> >> +++ b/arch/arm64/kernel/cpu_errata.c
> >> @@ -739,6 +739,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
> >>  		.capability = ARM64_WORKAROUND_1188873,
> >>  		ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
> >>  	},
> >> +#endif
> >> +#ifdef CONFIG_ARM64_ERRATUM_1165522
> >> +	{
> >> +		/* Cortex-A76 r0p0 to r2p0 */
> >> +		.desc = "ARM erratum 1165522",
> >> +		.capability = ARM64_WORKAROUND_1165522,
> >> +		ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
> >> +	},
> >>  #endif
> >>  	{
> >>  	}
> >> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> >> index 51d5d966d9e5..322109183853 100644
> >> --- a/arch/arm64/kvm/hyp/switch.c
> >> +++ b/arch/arm64/kvm/hyp/switch.c
> >> @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void)
> >>  {
> >>  	extern char vectors[];	/* kernel exception vectors */
> >>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
> >> +
> >> +	/*
> >> +	 * ARM erratum 1165522 requires the actual execution of the
> >> +	 * above before we can switch to the host translation regime.
> >> +	 */
> > 
> > same here, is it not rather about disabling stage 2 translation than
> > about the host (EL2 and EL0 stage 1) translation regimes?
> > 
> >> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
> >> +
> >>  	write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
> >>  	write_sysreg(vectors, vbar_el1);
> >>  }
> >> @@ -499,6 +506,13 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
> >>  
> >>  	sysreg_save_host_state_vhe(host_ctxt);
> >>  
> >> +	/*
> >> +	 * ARM erratum 1165522 requires us to have all the translation
> >> +	 * context in place before we clear HCR_TGE. All the offending
> >> +	 * guest sysregs are loaded in kvm_vcpu_load_sysregs, and
> >> +	 * __activate_vm has the stage-2 configuration. Once this is
> >> +	 * done, __activate_trap clears HCR_TGE (among other things).
> >> +	 */
> > 
> > I'm not this comment is needed or is helpful here.  For example, I don't
> > understand what you mean with the offending guest sysregs and how that
> 
> TTBR*, TCR, SCTLR... Anything that deals with stage-1 translations.
> 
> > relates to the problem of configuring stage 2 before clearing TGE.  Is
> > this about gettting the stage 1 configuration in place first?
> 
> Not only stage-1. Stage-2 is involved as well, as the CPU could
> otherwise end-up with the wrong translations (bypassing stage-2 altogether).
> 
> > 
> > If so, could I suggest a reword along the lines of:
> > 
> > 	/*
> > 	 * ARM erratum 1165522 requires us to configure both stage 1 and
> > 	 * stage 2 translation for the guest context before we clear
> > 	 * HCR_EL2.TGE.
> > 	 *
> > 	 * We have already configured the guest's stage 1 translation in
> > 	 * kvm_vcpu_load_sysregs above.  We must now call __activate_vm
> > 	 * before __activate_traps, because __activate_vm configures
> > 	 * stage 2 translation, and __activate_traps clear HCR_EL2.TGE
> > 	 * (among other things).
> > 	 */
> 
> Works for me (and shows that contrary to what you wrote above, you have
> perfectly understood the problem)!.
> 

I may have actually understood the problem by writing up that piece of
commentary.

Great!

Thanks,

    Christoffer

[PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[Christoffer Dall]

On Thu, Nov 08, 2018 at 06:05:55PM +0000, Marc Zyngier wrote:
> On 06/11/18 08:15, Christoffer Dall wrote:
> > On Mon, Nov 05, 2018 at 02:36:16PM +0000, Marc Zyngier wrote:
> >> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
> >> speculate an AT instruction in during a guest switch while the
> >> S1/S2 system registers are in an inconsistent state.
> >>
> >> Work around it by:
> >> - Mandating VHE
> >> - Make sure that S1 and S2 system registers are consistent before
> >>   clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
> >>   regime
> >>
> >> These two things together ensure that we cannot hit this erratum.
> >>
> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> >> ---
> >>  Documentation/arm64/silicon-errata.txt |  1 +
> >>  arch/arm64/Kconfig                     | 12 ++++++++++++
> >>  arch/arm64/include/asm/cpucaps.h       |  3 ++-
> >>  arch/arm64/include/asm/kvm_host.h      |  3 +++
> >>  arch/arm64/include/asm/kvm_hyp.h       |  6 ++++++
> >>  arch/arm64/kernel/cpu_errata.c         |  8 ++++++++
> >>  arch/arm64/kvm/hyp/switch.c            | 14 ++++++++++++++
> >>  7 files changed, 46 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
> >> index 76ccded8b74c..04f0bc4690c6 100644
> >> --- a/Documentation/arm64/silicon-errata.txt
> >> +++ b/Documentation/arm64/silicon-errata.txt
> >> @@ -57,6 +57,7 @@ stable kernels.
> >>  | ARM            | Cortex-A73      | #858921         | ARM64_ERRATUM_858921        |
> >>  | ARM            | Cortex-A55      | #1024718        | ARM64_ERRATUM_1024718       |
> >>  | ARM            | Cortex-A76      | #1188873        | ARM64_ERRATUM_1188873       |
> >> +| ARM            | Cortex-A76      | #1165522        | ARM64_ERRATUM_1165522       |
> >>  | ARM            | MMU-500         | #841119,#826419 | N/A                         |
> >>  |                |                 |                 |                             |
> >>  | Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375        |
> >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> >> index 787d7850e064..a68bc6cc2167 100644
> >> --- a/arch/arm64/Kconfig
> >> +++ b/arch/arm64/Kconfig
> >> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873
> >>  
> >>  	  If unsure, say Y.
> >>  
> >> +config ARM64_ERRATUM_1165522
> >> +	bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation"
> >> +	default y
> >> +	help
> >> +	  This option adds work arounds for ARM Cortex-A76 erratum 1165522
> >> +
> >> +	  Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with
> >> +	  corrupted TLBs by speculating an AT instruction during a guest
> >> +	  context switch.
> >> +
> >> +	  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 6e2d254c09eb..62d8cd15fdf2 100644
> >> --- a/arch/arm64/include/asm/cpucaps.h
> >> +++ b/arch/arm64/include/asm/cpucaps.h
> >> @@ -54,7 +54,8 @@
> >>  #define ARM64_HAS_CRC32				33
> >>  #define ARM64_SSBS				34
> >>  #define ARM64_WORKAROUND_1188873		35
> >> +#define ARM64_WORKAROUND_1165522		36
> >>  
> >> -#define ARM64_NCAPS				36
> >> +#define ARM64_NCAPS				37
> >>  
> >>  #endif /* __ASM_CPUCAPS_H */
> >> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> >> index 7d6e974d024a..8f486026ac87 100644
> >> --- a/arch/arm64/include/asm/kvm_host.h
> >> +++ b/arch/arm64/include/asm/kvm_host.h
> >> @@ -435,6 +435,9 @@ static inline bool kvm_arch_sve_requires_vhe(void)
> >>  static inline bool kvm_arch_impl_requires_vhe(void)
> >>  {
> >>  	/* Some implementations have defects that confine them to VHE */
> >> +	if (cpus_have_cap(ARM64_WORKAROUND_1165522))
> >> +		return true;
> >> +
> >>  	return false;
> >>  }
> >>  
> >> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> >> index 23aca66767f9..9681360d0959 100644
> >> --- a/arch/arm64/include/asm/kvm_hyp.h
> >> +++ b/arch/arm64/include/asm/kvm_hyp.h
> >> @@ -163,6 +163,12 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm)
> >>  {
> >>  	write_sysreg(kvm->arch.vtcr, vtcr_el2);
> >>  	write_sysreg(kvm->arch.vttbr, vttbr_el2);
> >> +
> >> +	/*
> >> +	 * ARM erratum 1165522 requires the actual execution of the
> >> +	 * above before we can switch to the guest translation regime.
> >> +	 */
> > 
> > Is it about a guest translation 'regime' or should this just say before
> > we can enable stage 2 translation?
> 
> No, this isn't strictly about enabling stage-2 translation. This is
> about making sure that anything that impacts the guest translations is
> actually executed.
> 
> I wonder if it would be clearer to move this outside of
> __load_guest_stage2 and make it explicit in the callers of this helper...

I think it makes sense to have this here to explain the alternative.

But it's the 'switch to guest translation regime' thing that bothers me
a bit.  Is that an architectural concept (I thought we only had EL1 and
EL2 translation regimes in stage 1, and then stage 2 translations).  So
When you say 'guest translation regime' I'm just not entirely sure what
that means, unless I'm missing something.

> > 
> >> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
> >>  }
> >>  
> >>  #endif /* __ARM64_KVM_HYP_H__ */
> >> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> >> index a509e35132d2..476e738e6c46 100644
> >> --- a/arch/arm64/kernel/cpu_errata.c
> >> +++ b/arch/arm64/kernel/cpu_errata.c
> >> @@ -739,6 +739,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
> >>  		.capability = ARM64_WORKAROUND_1188873,
> >>  		ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
> >>  	},
> >> +#endif
> >> +#ifdef CONFIG_ARM64_ERRATUM_1165522
> >> +	{
> >> +		/* Cortex-A76 r0p0 to r2p0 */
> >> +		.desc = "ARM erratum 1165522",
> >> +		.capability = ARM64_WORKAROUND_1165522,
> >> +		ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
> >> +	},
> >>  #endif
> >>  	{
> >>  	}
> >> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> >> index 51d5d966d9e5..322109183853 100644
> >> --- a/arch/arm64/kvm/hyp/switch.c
> >> +++ b/arch/arm64/kvm/hyp/switch.c
> >> @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void)
> >>  {
> >>  	extern char vectors[];	/* kernel exception vectors */
> >>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
> >> +
> >> +	/*
> >> +	 * ARM erratum 1165522 requires the actual execution of the
> >> +	 * above before we can switch to the host translation regime.
> >> +	 */
> > 
> > same here, is it not rather about disabling stage 2 translation than
> > about the host (EL2 and EL0 stage 1) translation regimes?
> > 
> >> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
> >> +
> >>  	write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
> >>  	write_sysreg(vectors, vbar_el1);
> >>  }
> >> @@ -499,6 +506,13 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
> >>  
> >>  	sysreg_save_host_state_vhe(host_ctxt);
> >>  
> >> +	/*
> >> +	 * ARM erratum 1165522 requires us to have all the translation
> >> +	 * context in place before we clear HCR_TGE. All the offending
> >> +	 * guest sysregs are loaded in kvm_vcpu_load_sysregs, and
> >> +	 * __activate_vm has the stage-2 configuration. Once this is
> >> +	 * done, __activate_trap clears HCR_TGE (among other things).
> >> +	 */
> > 
> > I'm not this comment is needed or is helpful here.  For example, I don't
> > understand what you mean with the offending guest sysregs and how that
> 
> TTBR*, TCR, SCTLR... Anything that deals with stage-1 translations.
> 
> > relates to the problem of configuring stage 2 before clearing TGE.  Is
> > this about gettting the stage 1 configuration in place first?
> 
> Not only stage-1. Stage-2 is involved as well, as the CPU could
> otherwise end-up with the wrong translations (bypassing stage-2 altogether).
> 
> > 
> > If so, could I suggest a reword along the lines of:
> > 
> > 	/*
> > 	 * ARM erratum 1165522 requires us to configure both stage 1 and
> > 	 * stage 2 translation for the guest context before we clear
> > 	 * HCR_EL2.TGE.
> > 	 *
> > 	 * We have already configured the guest's stage 1 translation in
> > 	 * kvm_vcpu_load_sysregs above.  We must now call __activate_vm
> > 	 * before __activate_traps, because __activate_vm configures
> > 	 * stage 2 translation, and __activate_traps clear HCR_EL2.TGE
> > 	 * (among other things).
> > 	 */
> 
> Works for me (and shows that contrary to what you wrote above, you have
> perfectly understood the problem)!.
> 

I may have actually understood the problem by writing up that piece of
commentary.

Great!

Thanks,

    Christoffer

Re: [PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[James Morse]

Hi Marc,

On 08/11/2018 17:18, Marc Zyngier wrote:
> On 05/11/18 18:34, James Morse wrote:
>> On 05/11/2018 14:36, Marc Zyngier wrote:
>>> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
>>> speculate an AT instruction in during a guest switch while the
>>
>>                                (in during?)
>>
>>> S1/S2 system registers are in an inconsistent state.
>>>
>>> Work around it by:
>>> - Mandating VHE
>>> - Make sure that S1 and S2 system registers are consistent before
>>>     clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
>>>     regime
>>>
>>> These two things together ensure that we cannot hit this erratum.
>>
>>
>>> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
>>> index 51d5d966d9e5..322109183853 100644
>>> --- a/arch/arm64/kvm/hyp/switch.c
>>> +++ b/arch/arm64/kvm/hyp/switch.c
>>> @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void)
>>>    {
>>>    	extern char vectors[];	/* kernel exception vectors */
>>>    	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
>>> +
>>> +	/*
>>> +	 * ARM erratum 1165522 requires the actual execution of the
>>> +	 * above before we can switch to the host translation regime.
>>> +	 */
>>> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
>>> +
>>
>> Host regime too ... does __tlb_switch_to_host_vhe() need the same
>> treatment? It writes vttbr_el2 and hcr_el2 back to back.
> 
> It turns out that our VHE TLB invalidation are a tiny bit broken, and
> that's before we work around this very erratum.
> 
> You're perfectly right that we're mitting an ISB in
> __tlb_switch_to_host_vhe().

I thought that would only be needed for this erratum workaround, in the 
regular case don't we rely on the isb hiding in __vhe_hyp_call()?


> We also have the problem that we can
> perfectly take an interrupt here, and maybe schedule another process
> from there (very unlikely, but I couldn't fully convince myself that it
> couldn't happen).

> What I'm planning to do is to make these TLB invalidation sequence
> atomic by disabling interrupts. Yes, this is quite a hammer, but that'
> no different from !VHE, and that's a very rare event anyway.

Anything running on the back of an IRQ should not be allowed to see 
HCR_EL2.TGE being clear. I think this is the right thing to do.
One example is TGE changes PANs behaviour, which could become 
inexplicably-trappy if we're also using the LDRT instructions in the 
uaccess helpers from an IRQ. (who would do such a thing? perf).


Thanks,

James

[PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[James Morse]

Hi Marc,

On 08/11/2018 17:18, Marc Zyngier wrote:
> On 05/11/18 18:34, James Morse wrote:
>> On 05/11/2018 14:36, Marc Zyngier wrote:
>>> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
>>> speculate an AT instruction in during a guest switch while the
>>
>>                                (in during?)
>>
>>> S1/S2 system registers are in an inconsistent state.
>>>
>>> Work around it by:
>>> - Mandating VHE
>>> - Make sure that S1 and S2 system registers are consistent before
>>>     clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
>>>     regime
>>>
>>> These two things together ensure that we cannot hit this erratum.
>>
>>
>>> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
>>> index 51d5d966d9e5..322109183853 100644
>>> --- a/arch/arm64/kvm/hyp/switch.c
>>> +++ b/arch/arm64/kvm/hyp/switch.c
>>> @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void)
>>>    {
>>>    	extern char vectors[];	/* kernel exception vectors */
>>>    	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
>>> +
>>> +	/*
>>> +	 * ARM erratum 1165522 requires the actual execution of the
>>> +	 * above before we can switch to the host translation regime.
>>> +	 */
>>> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
>>> +
>>
>> Host regime too ... does __tlb_switch_to_host_vhe() need the same
>> treatment? It writes vttbr_el2 and hcr_el2 back to back.
> 
> It turns out that our VHE TLB invalidation are a tiny bit broken, and
> that's before we work around this very erratum.
> 
> You're perfectly right that we're mitting an ISB in
> __tlb_switch_to_host_vhe().

I thought that would only be needed for this erratum workaround, in the 
regular case don't we rely on the isb hiding in __vhe_hyp_call()?


> We also have the problem that we can
> perfectly take an interrupt here, and maybe schedule another process
> from there (very unlikely, but I couldn't fully convince myself that it
> couldn't happen).

> What I'm planning to do is to make these TLB invalidation sequence
> atomic by disabling interrupts. Yes, this is quite a hammer, but that'
> no different from !VHE, and that's a very rare event anyway.

Anything running on the back of an IRQ should not be allowed to see 
HCR_EL2.TGE being clear. I think this is the right thing to do.
One example is TGE changes PANs behaviour, which could become 
inexplicably-trappy if we're also using the LDRT instructions in the 
uaccess helpers from an IRQ. (who would do such a thing? perf).


Thanks,

James

Re: [PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[Julien Grall]

Hi Marc,

On 05/11/2018 14:36, Marc Zyngier wrote:
> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
> speculate an AT instruction in during a guest switch while the
> S1/S2 system registers are in an inconsistent state.
> 
> Work around it by:
> - Mandating VHE
> - Make sure that S1 and S2 system registers are consistent before
>    clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
>    regime
> 
> These two things together ensure that we cannot hit this erratum.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>   Documentation/arm64/silicon-errata.txt |  1 +
>   arch/arm64/Kconfig                     | 12 ++++++++++++
>   arch/arm64/include/asm/cpucaps.h       |  3 ++-
>   arch/arm64/include/asm/kvm_host.h      |  3 +++
>   arch/arm64/include/asm/kvm_hyp.h       |  6 ++++++
>   arch/arm64/kernel/cpu_errata.c         |  8 ++++++++
>   arch/arm64/kvm/hyp/switch.c            | 14 ++++++++++++++
>   7 files changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
> index 76ccded8b74c..04f0bc4690c6 100644
> --- a/Documentation/arm64/silicon-errata.txt
> +++ b/Documentation/arm64/silicon-errata.txt
> @@ -57,6 +57,7 @@ stable kernels.
>   | ARM            | Cortex-A73      | #858921         | ARM64_ERRATUM_858921        |
>   | ARM            | Cortex-A55      | #1024718        | ARM64_ERRATUM_1024718       |
>   | ARM            | Cortex-A76      | #1188873        | ARM64_ERRATUM_1188873       |
> +| ARM            | Cortex-A76      | #1165522        | ARM64_ERRATUM_1165522       |
>   | ARM            | MMU-500         | #841119,#826419 | N/A                         |
>   |                |                 |                 |                             |
>   | Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375        |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 787d7850e064..a68bc6cc2167 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873
>   
>   	  If unsure, say Y.
>   
> +config ARM64_ERRATUM_1165522
> +	bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation"
> +	default y
> +	help
> +	  This option adds work arounds for ARM Cortex-A76 erratum 1165522
> +
> +	  Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with
> +	  corrupted TLBs by speculating an AT instruction during a guest
> +	  context switch.
> +
> +	  If unsure, say Y.

Most of the code in the patch is not guarded by #ifdef ARM64_*. So is there any 
benefits to add a Kconfig for this option?

Cheers,

-- 
Julien Grall

[PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[Julien Grall]

Hi Marc,

On 05/11/2018 14:36, Marc Zyngier wrote:
> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
> speculate an AT instruction in during a guest switch while the
> S1/S2 system registers are in an inconsistent state.
> 
> Work around it by:
> - Mandating VHE
> - Make sure that S1 and S2 system registers are consistent before
>    clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
>    regime
> 
> These two things together ensure that we cannot hit this erratum.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>   Documentation/arm64/silicon-errata.txt |  1 +
>   arch/arm64/Kconfig                     | 12 ++++++++++++
>   arch/arm64/include/asm/cpucaps.h       |  3 ++-
>   arch/arm64/include/asm/kvm_host.h      |  3 +++
>   arch/arm64/include/asm/kvm_hyp.h       |  6 ++++++
>   arch/arm64/kernel/cpu_errata.c         |  8 ++++++++
>   arch/arm64/kvm/hyp/switch.c            | 14 ++++++++++++++
>   7 files changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
> index 76ccded8b74c..04f0bc4690c6 100644
> --- a/Documentation/arm64/silicon-errata.txt
> +++ b/Documentation/arm64/silicon-errata.txt
> @@ -57,6 +57,7 @@ stable kernels.
>   | ARM            | Cortex-A73      | #858921         | ARM64_ERRATUM_858921        |
>   | ARM            | Cortex-A55      | #1024718        | ARM64_ERRATUM_1024718       |
>   | ARM            | Cortex-A76      | #1188873        | ARM64_ERRATUM_1188873       |
> +| ARM            | Cortex-A76      | #1165522        | ARM64_ERRATUM_1165522       |
>   | ARM            | MMU-500         | #841119,#826419 | N/A                         |
>   |                |                 |                 |                             |
>   | Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375        |
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 787d7850e064..a68bc6cc2167 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873
>   
>   	  If unsure, say Y.
>   
> +config ARM64_ERRATUM_1165522
> +	bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation"
> +	default y
> +	help
> +	  This option adds work arounds for ARM Cortex-A76 erratum 1165522
> +
> +	  Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with
> +	  corrupted TLBs by speculating an AT instruction during a guest
> +	  context switch.
> +
> +	  If unsure, say Y.

Most of the code in the patch is not guarded by #ifdef ARM64_*. So is there any 
benefits to add a Kconfig for this option?

Cheers,

-- 
Julien Grall

Re: [PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[Marc Zyngier]

On 21/11/2018 12:23, Julien Grall wrote:
> Hi Marc,
> 
> On 05/11/2018 14:36, Marc Zyngier wrote:
>> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
>> speculate an AT instruction in during a guest switch while the
>> S1/S2 system registers are in an inconsistent state.
>>
>> Work around it by:
>> - Mandating VHE
>> - Make sure that S1 and S2 system registers are consistent before
>>    clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
>>    regime
>>
>> These two things together ensure that we cannot hit this erratum.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>   Documentation/arm64/silicon-errata.txt |  1 +
>>   arch/arm64/Kconfig                     | 12 ++++++++++++
>>   arch/arm64/include/asm/cpucaps.h       |  3 ++-
>>   arch/arm64/include/asm/kvm_host.h      |  3 +++
>>   arch/arm64/include/asm/kvm_hyp.h       |  6 ++++++
>>   arch/arm64/kernel/cpu_errata.c         |  8 ++++++++
>>   arch/arm64/kvm/hyp/switch.c            | 14 ++++++++++++++
>>   7 files changed, 46 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
>> index 76ccded8b74c..04f0bc4690c6 100644
>> --- a/Documentation/arm64/silicon-errata.txt
>> +++ b/Documentation/arm64/silicon-errata.txt
>> @@ -57,6 +57,7 @@ stable kernels.
>>   | ARM            | Cortex-A73      | #858921         | ARM64_ERRATUM_858921        |
>>   | ARM            | Cortex-A55      | #1024718        | ARM64_ERRATUM_1024718       |
>>   | ARM            | Cortex-A76      | #1188873        | ARM64_ERRATUM_1188873       |
>> +| ARM            | Cortex-A76      | #1165522        | ARM64_ERRATUM_1165522       |
>>   | ARM            | MMU-500         | #841119,#826419 | N/A                         |
>>   |                |                 |                 |                             |
>>   | Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375        |
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 787d7850e064..a68bc6cc2167 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873
>>   
>>   	  If unsure, say Y.
>>   
>> +config ARM64_ERRATUM_1165522
>> +	bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation"
>> +	default y
>> +	help
>> +	  This option adds work arounds for ARM Cortex-A76 erratum 1165522
>> +
>> +	  Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with
>> +	  corrupted TLBs by speculating an AT instruction during a guest
>> +	  context switch.
>> +
>> +	  If unsure, say Y.
> 
> Most of the code in the patch is not guarded by #ifdef ARM64_*. So is there any 
> benefits to add a Kconfig for this option?

The detection code is guarded by this config option, which is the
important thing. In general, we try to compile everything, all the time,
unless this is too big to be the case. It drastically simplify the
maintenance.

See the VHE code for example, which is always compiled in, and is only
gated by the detection code that gets compiled out if the option isn't
selected.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[Marc Zyngier]

On 21/11/2018 12:23, Julien Grall wrote:
> Hi Marc,
> 
> On 05/11/2018 14:36, Marc Zyngier wrote:
>> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
>> speculate an AT instruction in during a guest switch while the
>> S1/S2 system registers are in an inconsistent state.
>>
>> Work around it by:
>> - Mandating VHE
>> - Make sure that S1 and S2 system registers are consistent before
>>    clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
>>    regime
>>
>> These two things together ensure that we cannot hit this erratum.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>   Documentation/arm64/silicon-errata.txt |  1 +
>>   arch/arm64/Kconfig                     | 12 ++++++++++++
>>   arch/arm64/include/asm/cpucaps.h       |  3 ++-
>>   arch/arm64/include/asm/kvm_host.h      |  3 +++
>>   arch/arm64/include/asm/kvm_hyp.h       |  6 ++++++
>>   arch/arm64/kernel/cpu_errata.c         |  8 ++++++++
>>   arch/arm64/kvm/hyp/switch.c            | 14 ++++++++++++++
>>   7 files changed, 46 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
>> index 76ccded8b74c..04f0bc4690c6 100644
>> --- a/Documentation/arm64/silicon-errata.txt
>> +++ b/Documentation/arm64/silicon-errata.txt
>> @@ -57,6 +57,7 @@ stable kernels.
>>   | ARM            | Cortex-A73      | #858921         | ARM64_ERRATUM_858921        |
>>   | ARM            | Cortex-A55      | #1024718        | ARM64_ERRATUM_1024718       |
>>   | ARM            | Cortex-A76      | #1188873        | ARM64_ERRATUM_1188873       |
>> +| ARM            | Cortex-A76      | #1165522        | ARM64_ERRATUM_1165522       |
>>   | ARM            | MMU-500         | #841119,#826419 | N/A                         |
>>   |                |                 |                 |                             |
>>   | Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375        |
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 787d7850e064..a68bc6cc2167 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873
>>   
>>   	  If unsure, say Y.
>>   
>> +config ARM64_ERRATUM_1165522
>> +	bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation"
>> +	default y
>> +	help
>> +	  This option adds work arounds for ARM Cortex-A76 erratum 1165522
>> +
>> +	  Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with
>> +	  corrupted TLBs by speculating an AT instruction during a guest
>> +	  context switch.
>> +
>> +	  If unsure, say Y.
> 
> Most of the code in the patch is not guarded by #ifdef ARM64_*. So is there any 
> benefits to add a Kconfig for this option?

The detection code is guarded by this config option, which is the
important thing. In general, we try to compile everything, all the time,
unless this is too big to be the case. It drastically simplify the
maintenance.

See the VHE code for example, which is always compiled in, and is only
gated by the detection code that gets compiled out if the option isn't
selected.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[Marc Zyngier]

On 08/11/2018 20:10, Christoffer Dall wrote:
> On Thu, Nov 08, 2018 at 06:05:55PM +0000, Marc Zyngier wrote:
>> On 06/11/18 08:15, Christoffer Dall wrote:
>>> On Mon, Nov 05, 2018 at 02:36:16PM +0000, Marc Zyngier wrote:
>>>> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
>>>> speculate an AT instruction in during a guest switch while the
>>>> S1/S2 system registers are in an inconsistent state.
>>>>
>>>> Work around it by:
>>>> - Mandating VHE
>>>> - Make sure that S1 and S2 system registers are consistent before
>>>>   clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
>>>>   regime
>>>>
>>>> These two things together ensure that we cannot hit this erratum.
>>>>
>>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>>> ---
>>>>  Documentation/arm64/silicon-errata.txt |  1 +
>>>>  arch/arm64/Kconfig                     | 12 ++++++++++++
>>>>  arch/arm64/include/asm/cpucaps.h       |  3 ++-
>>>>  arch/arm64/include/asm/kvm_host.h      |  3 +++
>>>>  arch/arm64/include/asm/kvm_hyp.h       |  6 ++++++
>>>>  arch/arm64/kernel/cpu_errata.c         |  8 ++++++++
>>>>  arch/arm64/kvm/hyp/switch.c            | 14 ++++++++++++++
>>>>  7 files changed, 46 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
>>>> index 76ccded8b74c..04f0bc4690c6 100644
>>>> --- a/Documentation/arm64/silicon-errata.txt
>>>> +++ b/Documentation/arm64/silicon-errata.txt
>>>> @@ -57,6 +57,7 @@ stable kernels.
>>>>  | ARM            | Cortex-A73      | #858921         | ARM64_ERRATUM_858921        |
>>>>  | ARM            | Cortex-A55      | #1024718        | ARM64_ERRATUM_1024718       |
>>>>  | ARM            | Cortex-A76      | #1188873        | ARM64_ERRATUM_1188873       |
>>>> +| ARM            | Cortex-A76      | #1165522        | ARM64_ERRATUM_1165522       |
>>>>  | ARM            | MMU-500         | #841119,#826419 | N/A                         |
>>>>  |                |                 |                 |                             |
>>>>  | Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375        |
>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>> index 787d7850e064..a68bc6cc2167 100644
>>>> --- a/arch/arm64/Kconfig
>>>> +++ b/arch/arm64/Kconfig
>>>> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873
>>>>  
>>>>  	  If unsure, say Y.
>>>>  
>>>> +config ARM64_ERRATUM_1165522
>>>> +	bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation"
>>>> +	default y
>>>> +	help
>>>> +	  This option adds work arounds for ARM Cortex-A76 erratum 1165522
>>>> +
>>>> +	  Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with
>>>> +	  corrupted TLBs by speculating an AT instruction during a guest
>>>> +	  context switch.
>>>> +
>>>> +	  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 6e2d254c09eb..62d8cd15fdf2 100644
>>>> --- a/arch/arm64/include/asm/cpucaps.h
>>>> +++ b/arch/arm64/include/asm/cpucaps.h
>>>> @@ -54,7 +54,8 @@
>>>>  #define ARM64_HAS_CRC32				33
>>>>  #define ARM64_SSBS				34
>>>>  #define ARM64_WORKAROUND_1188873		35
>>>> +#define ARM64_WORKAROUND_1165522		36
>>>>  
>>>> -#define ARM64_NCAPS				36
>>>> +#define ARM64_NCAPS				37
>>>>  
>>>>  #endif /* __ASM_CPUCAPS_H */
>>>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>>>> index 7d6e974d024a..8f486026ac87 100644
>>>> --- a/arch/arm64/include/asm/kvm_host.h
>>>> +++ b/arch/arm64/include/asm/kvm_host.h
>>>> @@ -435,6 +435,9 @@ static inline bool kvm_arch_sve_requires_vhe(void)
>>>>  static inline bool kvm_arch_impl_requires_vhe(void)
>>>>  {
>>>>  	/* Some implementations have defects that confine them to VHE */
>>>> +	if (cpus_have_cap(ARM64_WORKAROUND_1165522))
>>>> +		return true;
>>>> +
>>>>  	return false;
>>>>  }
>>>>  
>>>> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
>>>> index 23aca66767f9..9681360d0959 100644
>>>> --- a/arch/arm64/include/asm/kvm_hyp.h
>>>> +++ b/arch/arm64/include/asm/kvm_hyp.h
>>>> @@ -163,6 +163,12 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm)
>>>>  {
>>>>  	write_sysreg(kvm->arch.vtcr, vtcr_el2);
>>>>  	write_sysreg(kvm->arch.vttbr, vttbr_el2);
>>>> +
>>>> +	/*
>>>> +	 * ARM erratum 1165522 requires the actual execution of the
>>>> +	 * above before we can switch to the guest translation regime.
>>>> +	 */
>>>
>>> Is it about a guest translation 'regime' or should this just say before
>>> we can enable stage 2 translation?
>>
>> No, this isn't strictly about enabling stage-2 translation. This is
>> about making sure that anything that impacts the guest translations is
>> actually executed.
>>
>> I wonder if it would be clearer to move this outside of
>> __load_guest_stage2 and make it explicit in the callers of this helper...
> 
> I think it makes sense to have this here to explain the alternative.
> 
> But it's the 'switch to guest translation regime' thing that bothers me
> a bit.  Is that an architectural concept (I thought we only had EL1 and
> EL2 translation regimes in stage 1, and then stage 2 translations).  So
> When you say 'guest translation regime' I'm just not entirely sure what
> that means, unless I'm missing something.

[coming back to this as I'm preparing the next round]

What I'm trying to convey by "guest translation regime" is the following
from the ARM ARM:

C5.4.7 AT S1E1R, Address Translate Stage 1 EL1 Read
[...]

When EL2 is implemented and enabled in the current Security state:
— If HCR_EL2.{E2H, TGE} is not {1, 1}, the EL1&0 translation regime,
accessed from EL1.
— If HCR_EL2.{E2H, TGE} is {1, 1}, the EL2&0 translation regime,
accessed from EL2.

So maybe I should just bite the bullet and call out EL1/EL2 translation
regime instead of guest/host.

Thoughts?

	M.
-- 
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[Marc Zyngier]

On 08/11/2018 20:10, Christoffer Dall wrote:
> On Thu, Nov 08, 2018 at 06:05:55PM +0000, Marc Zyngier wrote:
>> On 06/11/18 08:15, Christoffer Dall wrote:
>>> On Mon, Nov 05, 2018 at 02:36:16PM +0000, Marc Zyngier wrote:
>>>> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
>>>> speculate an AT instruction in during a guest switch while the
>>>> S1/S2 system registers are in an inconsistent state.
>>>>
>>>> Work around it by:
>>>> - Mandating VHE
>>>> - Make sure that S1 and S2 system registers are consistent before
>>>>   clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
>>>>   regime
>>>>
>>>> These two things together ensure that we cannot hit this erratum.
>>>>
>>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>>> ---
>>>>  Documentation/arm64/silicon-errata.txt |  1 +
>>>>  arch/arm64/Kconfig                     | 12 ++++++++++++
>>>>  arch/arm64/include/asm/cpucaps.h       |  3 ++-
>>>>  arch/arm64/include/asm/kvm_host.h      |  3 +++
>>>>  arch/arm64/include/asm/kvm_hyp.h       |  6 ++++++
>>>>  arch/arm64/kernel/cpu_errata.c         |  8 ++++++++
>>>>  arch/arm64/kvm/hyp/switch.c            | 14 ++++++++++++++
>>>>  7 files changed, 46 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
>>>> index 76ccded8b74c..04f0bc4690c6 100644
>>>> --- a/Documentation/arm64/silicon-errata.txt
>>>> +++ b/Documentation/arm64/silicon-errata.txt
>>>> @@ -57,6 +57,7 @@ stable kernels.
>>>>  | ARM            | Cortex-A73      | #858921         | ARM64_ERRATUM_858921        |
>>>>  | ARM            | Cortex-A55      | #1024718        | ARM64_ERRATUM_1024718       |
>>>>  | ARM            | Cortex-A76      | #1188873        | ARM64_ERRATUM_1188873       |
>>>> +| ARM            | Cortex-A76      | #1165522        | ARM64_ERRATUM_1165522       |
>>>>  | ARM            | MMU-500         | #841119,#826419 | N/A                         |
>>>>  |                |                 |                 |                             |
>>>>  | Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375        |
>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>> index 787d7850e064..a68bc6cc2167 100644
>>>> --- a/arch/arm64/Kconfig
>>>> +++ b/arch/arm64/Kconfig
>>>> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873
>>>>  
>>>>  	  If unsure, say Y.
>>>>  
>>>> +config ARM64_ERRATUM_1165522
>>>> +	bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation"
>>>> +	default y
>>>> +	help
>>>> +	  This option adds work arounds for ARM Cortex-A76 erratum 1165522
>>>> +
>>>> +	  Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with
>>>> +	  corrupted TLBs by speculating an AT instruction during a guest
>>>> +	  context switch.
>>>> +
>>>> +	  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 6e2d254c09eb..62d8cd15fdf2 100644
>>>> --- a/arch/arm64/include/asm/cpucaps.h
>>>> +++ b/arch/arm64/include/asm/cpucaps.h
>>>> @@ -54,7 +54,8 @@
>>>>  #define ARM64_HAS_CRC32				33
>>>>  #define ARM64_SSBS				34
>>>>  #define ARM64_WORKAROUND_1188873		35
>>>> +#define ARM64_WORKAROUND_1165522		36
>>>>  
>>>> -#define ARM64_NCAPS				36
>>>> +#define ARM64_NCAPS				37
>>>>  
>>>>  #endif /* __ASM_CPUCAPS_H */
>>>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>>>> index 7d6e974d024a..8f486026ac87 100644
>>>> --- a/arch/arm64/include/asm/kvm_host.h
>>>> +++ b/arch/arm64/include/asm/kvm_host.h
>>>> @@ -435,6 +435,9 @@ static inline bool kvm_arch_sve_requires_vhe(void)
>>>>  static inline bool kvm_arch_impl_requires_vhe(void)
>>>>  {
>>>>  	/* Some implementations have defects that confine them to VHE */
>>>> +	if (cpus_have_cap(ARM64_WORKAROUND_1165522))
>>>> +		return true;
>>>> +
>>>>  	return false;
>>>>  }
>>>>  
>>>> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
>>>> index 23aca66767f9..9681360d0959 100644
>>>> --- a/arch/arm64/include/asm/kvm_hyp.h
>>>> +++ b/arch/arm64/include/asm/kvm_hyp.h
>>>> @@ -163,6 +163,12 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm)
>>>>  {
>>>>  	write_sysreg(kvm->arch.vtcr, vtcr_el2);
>>>>  	write_sysreg(kvm->arch.vttbr, vttbr_el2);
>>>> +
>>>> +	/*
>>>> +	 * ARM erratum 1165522 requires the actual execution of the
>>>> +	 * above before we can switch to the guest translation regime.
>>>> +	 */
>>>
>>> Is it about a guest translation 'regime' or should this just say before
>>> we can enable stage 2 translation?
>>
>> No, this isn't strictly about enabling stage-2 translation. This is
>> about making sure that anything that impacts the guest translations is
>> actually executed.
>>
>> I wonder if it would be clearer to move this outside of
>> __load_guest_stage2 and make it explicit in the callers of this helper...
> 
> I think it makes sense to have this here to explain the alternative.
> 
> But it's the 'switch to guest translation regime' thing that bothers me
> a bit.  Is that an architectural concept (I thought we only had EL1 and
> EL2 translation regimes in stage 1, and then stage 2 translations).  So
> When you say 'guest translation regime' I'm just not entirely sure what
> that means, unless I'm missing something.

[coming back to this as I'm preparing the next round]

What I'm trying to convey by "guest translation regime" is the following
from the ARM ARM:

C5.4.7 AT S1E1R, Address Translate Stage 1 EL1 Read
[...]

When EL2 is implemented and enabled in the current Security state:
? If HCR_EL2.{E2H, TGE} is not {1, 1}, the EL1&0 translation regime,
accessed from EL1.
? If HCR_EL2.{E2H, TGE} is {1, 1}, the EL2&0 translation regime,
accessed from EL2.

So maybe I should just bite the bullet and call out EL1/EL2 translation
regime instead of guest/host.

Thoughts?

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[Christoffer Dall]

On Thu, Nov 22, 2018 at 04:13:16PM +0000, Marc Zyngier wrote:
> On 08/11/2018 20:10, Christoffer Dall wrote:
> > On Thu, Nov 08, 2018 at 06:05:55PM +0000, Marc Zyngier wrote:
> >> On 06/11/18 08:15, Christoffer Dall wrote:
> >>> On Mon, Nov 05, 2018 at 02:36:16PM +0000, Marc Zyngier wrote:
> >>>> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
> >>>> speculate an AT instruction in during a guest switch while the
> >>>> S1/S2 system registers are in an inconsistent state.
> >>>>
> >>>> Work around it by:
> >>>> - Mandating VHE
> >>>> - Make sure that S1 and S2 system registers are consistent before
> >>>>   clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
> >>>>   regime
> >>>>
> >>>> These two things together ensure that we cannot hit this erratum.
> >>>>
> >>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> >>>> ---
> >>>>  Documentation/arm64/silicon-errata.txt |  1 +
> >>>>  arch/arm64/Kconfig                     | 12 ++++++++++++
> >>>>  arch/arm64/include/asm/cpucaps.h       |  3 ++-
> >>>>  arch/arm64/include/asm/kvm_host.h      |  3 +++
> >>>>  arch/arm64/include/asm/kvm_hyp.h       |  6 ++++++
> >>>>  arch/arm64/kernel/cpu_errata.c         |  8 ++++++++
> >>>>  arch/arm64/kvm/hyp/switch.c            | 14 ++++++++++++++
> >>>>  7 files changed, 46 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
> >>>> index 76ccded8b74c..04f0bc4690c6 100644
> >>>> --- a/Documentation/arm64/silicon-errata.txt
> >>>> +++ b/Documentation/arm64/silicon-errata.txt
> >>>> @@ -57,6 +57,7 @@ stable kernels.
> >>>>  | ARM            | Cortex-A73      | #858921         | ARM64_ERRATUM_858921        |
> >>>>  | ARM            | Cortex-A55      | #1024718        | ARM64_ERRATUM_1024718       |
> >>>>  | ARM            | Cortex-A76      | #1188873        | ARM64_ERRATUM_1188873       |
> >>>> +| ARM            | Cortex-A76      | #1165522        | ARM64_ERRATUM_1165522       |
> >>>>  | ARM            | MMU-500         | #841119,#826419 | N/A                         |
> >>>>  |                |                 |                 |                             |
> >>>>  | Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375        |
> >>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> >>>> index 787d7850e064..a68bc6cc2167 100644
> >>>> --- a/arch/arm64/Kconfig
> >>>> +++ b/arch/arm64/Kconfig
> >>>> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873
> >>>>  
> >>>>  	  If unsure, say Y.
> >>>>  
> >>>> +config ARM64_ERRATUM_1165522
> >>>> +	bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation"
> >>>> +	default y
> >>>> +	help
> >>>> +	  This option adds work arounds for ARM Cortex-A76 erratum 1165522
> >>>> +
> >>>> +	  Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with
> >>>> +	  corrupted TLBs by speculating an AT instruction during a guest
> >>>> +	  context switch.
> >>>> +
> >>>> +	  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 6e2d254c09eb..62d8cd15fdf2 100644
> >>>> --- a/arch/arm64/include/asm/cpucaps.h
> >>>> +++ b/arch/arm64/include/asm/cpucaps.h
> >>>> @@ -54,7 +54,8 @@
> >>>>  #define ARM64_HAS_CRC32				33
> >>>>  #define ARM64_SSBS				34
> >>>>  #define ARM64_WORKAROUND_1188873		35
> >>>> +#define ARM64_WORKAROUND_1165522		36
> >>>>  
> >>>> -#define ARM64_NCAPS				36
> >>>> +#define ARM64_NCAPS				37
> >>>>  
> >>>>  #endif /* __ASM_CPUCAPS_H */
> >>>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> >>>> index 7d6e974d024a..8f486026ac87 100644
> >>>> --- a/arch/arm64/include/asm/kvm_host.h
> >>>> +++ b/arch/arm64/include/asm/kvm_host.h
> >>>> @@ -435,6 +435,9 @@ static inline bool kvm_arch_sve_requires_vhe(void)
> >>>>  static inline bool kvm_arch_impl_requires_vhe(void)
> >>>>  {
> >>>>  	/* Some implementations have defects that confine them to VHE */
> >>>> +	if (cpus_have_cap(ARM64_WORKAROUND_1165522))
> >>>> +		return true;
> >>>> +
> >>>>  	return false;
> >>>>  }
> >>>>  
> >>>> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> >>>> index 23aca66767f9..9681360d0959 100644
> >>>> --- a/arch/arm64/include/asm/kvm_hyp.h
> >>>> +++ b/arch/arm64/include/asm/kvm_hyp.h
> >>>> @@ -163,6 +163,12 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm)
> >>>>  {
> >>>>  	write_sysreg(kvm->arch.vtcr, vtcr_el2);
> >>>>  	write_sysreg(kvm->arch.vttbr, vttbr_el2);
> >>>> +
> >>>> +	/*
> >>>> +	 * ARM erratum 1165522 requires the actual execution of the
> >>>> +	 * above before we can switch to the guest translation regime.
> >>>> +	 */
> >>>
> >>> Is it about a guest translation 'regime' or should this just say before
> >>> we can enable stage 2 translation?
> >>
> >> No, this isn't strictly about enabling stage-2 translation. This is
> >> about making sure that anything that impacts the guest translations is
> >> actually executed.
> >>
> >> I wonder if it would be clearer to move this outside of
> >> __load_guest_stage2 and make it explicit in the callers of this helper...
> > 
> > I think it makes sense to have this here to explain the alternative.
> > 
> > But it's the 'switch to guest translation regime' thing that bothers me
> > a bit.  Is that an architectural concept (I thought we only had EL1 and
> > EL2 translation regimes in stage 1, and then stage 2 translations).  So
> > When you say 'guest translation regime' I'm just not entirely sure what
> > that means, unless I'm missing something.
> 
> [coming back to this as I'm preparing the next round]
> 
> What I'm trying to convey by "guest translation regime" is the following
> from the ARM ARM:
> 
> C5.4.7 AT S1E1R, Address Translate Stage 1 EL1 Read
> [...]
> 
> When EL2 is implemented and enabled in the current Security state:
> — If HCR_EL2.{E2H, TGE} is not {1, 1}, the EL1&0 translation regime,
> accessed from EL1.
> — If HCR_EL2.{E2H, TGE} is {1, 1}, the EL2&0 translation regime,
> accessed from EL2.
> 
> So maybe I should just bite the bullet and call out EL1/EL2 translation
> regime instead of guest/host.
> 

How about the following, does that work?

	/*
	 * ARM erratum 1165522 requires the actual execution of the
	 * above before we can switch to the EL1/EL0 translation regime
	 * used by the guest.
	 */


Thanks,

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

[PATCH 4/4] arm64: KVM: Implement workaround for Cortex-A76 erratum 1165522

[Christoffer Dall]

On Thu, Nov 22, 2018 at 04:13:16PM +0000, Marc Zyngier wrote:
> On 08/11/2018 20:10, Christoffer Dall wrote:
> > On Thu, Nov 08, 2018 at 06:05:55PM +0000, Marc Zyngier wrote:
> >> On 06/11/18 08:15, Christoffer Dall wrote:
> >>> On Mon, Nov 05, 2018 at 02:36:16PM +0000, Marc Zyngier wrote:
> >>>> Early versions of Cortex-A76 can end-up with corrupt TLBs if they
> >>>> speculate an AT instruction in during a guest switch while the
> >>>> S1/S2 system registers are in an inconsistent state.
> >>>>
> >>>> Work around it by:
> >>>> - Mandating VHE
> >>>> - Make sure that S1 and S2 system registers are consistent before
> >>>>   clearing HCR_EL2.TGE, which allows AT to target the EL1 translation
> >>>>   regime
> >>>>
> >>>> These two things together ensure that we cannot hit this erratum.
> >>>>
> >>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> >>>> ---
> >>>>  Documentation/arm64/silicon-errata.txt |  1 +
> >>>>  arch/arm64/Kconfig                     | 12 ++++++++++++
> >>>>  arch/arm64/include/asm/cpucaps.h       |  3 ++-
> >>>>  arch/arm64/include/asm/kvm_host.h      |  3 +++
> >>>>  arch/arm64/include/asm/kvm_hyp.h       |  6 ++++++
> >>>>  arch/arm64/kernel/cpu_errata.c         |  8 ++++++++
> >>>>  arch/arm64/kvm/hyp/switch.c            | 14 ++++++++++++++
> >>>>  7 files changed, 46 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
> >>>> index 76ccded8b74c..04f0bc4690c6 100644
> >>>> --- a/Documentation/arm64/silicon-errata.txt
> >>>> +++ b/Documentation/arm64/silicon-errata.txt
> >>>> @@ -57,6 +57,7 @@ stable kernels.
> >>>>  | ARM            | Cortex-A73      | #858921         | ARM64_ERRATUM_858921        |
> >>>>  | ARM            | Cortex-A55      | #1024718        | ARM64_ERRATUM_1024718       |
> >>>>  | ARM            | Cortex-A76      | #1188873        | ARM64_ERRATUM_1188873       |
> >>>> +| ARM            | Cortex-A76      | #1165522        | ARM64_ERRATUM_1165522       |
> >>>>  | ARM            | MMU-500         | #841119,#826419 | N/A                         |
> >>>>  |                |                 |                 |                             |
> >>>>  | Cavium         | ThunderX ITS    | #22375, #24313  | CAVIUM_ERRATUM_22375        |
> >>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> >>>> index 787d7850e064..a68bc6cc2167 100644
> >>>> --- a/arch/arm64/Kconfig
> >>>> +++ b/arch/arm64/Kconfig
> >>>> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873
> >>>>  
> >>>>  	  If unsure, say Y.
> >>>>  
> >>>> +config ARM64_ERRATUM_1165522
> >>>> +	bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation"
> >>>> +	default y
> >>>> +	help
> >>>> +	  This option adds work arounds for ARM Cortex-A76 erratum 1165522
> >>>> +
> >>>> +	  Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with
> >>>> +	  corrupted TLBs by speculating an AT instruction during a guest
> >>>> +	  context switch.
> >>>> +
> >>>> +	  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 6e2d254c09eb..62d8cd15fdf2 100644
> >>>> --- a/arch/arm64/include/asm/cpucaps.h
> >>>> +++ b/arch/arm64/include/asm/cpucaps.h
> >>>> @@ -54,7 +54,8 @@
> >>>>  #define ARM64_HAS_CRC32				33
> >>>>  #define ARM64_SSBS				34
> >>>>  #define ARM64_WORKAROUND_1188873		35
> >>>> +#define ARM64_WORKAROUND_1165522		36
> >>>>  
> >>>> -#define ARM64_NCAPS				36
> >>>> +#define ARM64_NCAPS				37
> >>>>  
> >>>>  #endif /* __ASM_CPUCAPS_H */
> >>>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> >>>> index 7d6e974d024a..8f486026ac87 100644
> >>>> --- a/arch/arm64/include/asm/kvm_host.h
> >>>> +++ b/arch/arm64/include/asm/kvm_host.h
> >>>> @@ -435,6 +435,9 @@ static inline bool kvm_arch_sve_requires_vhe(void)
> >>>>  static inline bool kvm_arch_impl_requires_vhe(void)
> >>>>  {
> >>>>  	/* Some implementations have defects that confine them to VHE */
> >>>> +	if (cpus_have_cap(ARM64_WORKAROUND_1165522))
> >>>> +		return true;
> >>>> +
> >>>>  	return false;
> >>>>  }
> >>>>  
> >>>> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> >>>> index 23aca66767f9..9681360d0959 100644
> >>>> --- a/arch/arm64/include/asm/kvm_hyp.h
> >>>> +++ b/arch/arm64/include/asm/kvm_hyp.h
> >>>> @@ -163,6 +163,12 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm)
> >>>>  {
> >>>>  	write_sysreg(kvm->arch.vtcr, vtcr_el2);
> >>>>  	write_sysreg(kvm->arch.vttbr, vttbr_el2);
> >>>> +
> >>>> +	/*
> >>>> +	 * ARM erratum 1165522 requires the actual execution of the
> >>>> +	 * above before we can switch to the guest translation regime.
> >>>> +	 */
> >>>
> >>> Is it about a guest translation 'regime' or should this just say before
> >>> we can enable stage 2 translation?
> >>
> >> No, this isn't strictly about enabling stage-2 translation. This is
> >> about making sure that anything that impacts the guest translations is
> >> actually executed.
> >>
> >> I wonder if it would be clearer to move this outside of
> >> __load_guest_stage2 and make it explicit in the callers of this helper...
> > 
> > I think it makes sense to have this here to explain the alternative.
> > 
> > But it's the 'switch to guest translation regime' thing that bothers me
> > a bit.  Is that an architectural concept (I thought we only had EL1 and
> > EL2 translation regimes in stage 1, and then stage 2 translations).  So
> > When you say 'guest translation regime' I'm just not entirely sure what
> > that means, unless I'm missing something.
> 
> [coming back to this as I'm preparing the next round]
> 
> What I'm trying to convey by "guest translation regime" is the following
> from the ARM ARM:
> 
> C5.4.7 AT S1E1R, Address Translate Stage 1 EL1 Read
> [...]
> 
> When EL2 is implemented and enabled in the current Security state:
> ? If HCR_EL2.{E2H, TGE} is not {1, 1}, the EL1&0 translation regime,
> accessed from EL1.
> ? If HCR_EL2.{E2H, TGE} is {1, 1}, the EL2&0 translation regime,
> accessed from EL2.
> 
> So maybe I should just bite the bullet and call out EL1/EL2 translation
> regime instead of guest/host.
> 

How about the following, does that work?

	/*
	 * ARM erratum 1165522 requires the actual execution of the
	 * above before we can switch to the EL1/EL0 translation regime
	 * used by the guest.
	 */


Thanks,

    Christoffer