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

[PATCH v3 0/8] 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.

There is some additional complexity in the TLB invalidation code,
where we most make sure that a speculated AT instruction cannot mess
the stage-1 TLBs.

* From v2:
  - Dropped a bunch of spurious ISBs after James review
  - Added James RBs.

* From v1:
  - VHE TLB invalidation now atomic
  - Avoid speculated AT during TLB invalidation
  - Addressed most comments from Christoffer
  - Resplit to ease reviewing

Marc Zyngier (8):
  arm64: KVM: Make VHE Stage-2 TLB invalidation operations
    non-interruptible
  KVM: arm64: Rework detection of SVE, !VHE systems
  arm64: KVM: Install stage-2 translation before enabling traps
  arm64: Add TCR_EPD{0,1} definitions
  arm64: KVM: Force VHE for systems affected by erratum 1165522
  arm64: KVM: Add synchronization on translation regime change for
    erratum 1165522
  arm64: KVM: Handle ARM erratum 1165522 in TLB invalidation
  arm64: Add configuration/documentation for Cortex-A76 erratum 1165522

 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm/include/asm/kvm_host.h        |  2 +-
 arch/arm64/Kconfig                     | 12 +++++
 arch/arm64/include/asm/cpucaps.h       |  3 +-
 arch/arm64/include/asm/kvm_host.h      | 10 ++--
 arch/arm64/include/asm/kvm_hyp.h       |  8 +++
 arch/arm64/include/asm/pgtable-hwdef.h |  4 ++
 arch/arm64/kernel/cpu_errata.c         |  8 +++
 arch/arm64/kvm/hyp/switch.c            | 23 ++++++++-
 arch/arm64/kvm/hyp/tlb.c               | 71 ++++++++++++++++++++++----
 virt/kvm/arm/arm.c                     |  8 +--
 11 files changed, 129 insertions(+), 21 deletions(-)

-- 
2.19.2

[PATCH v3 0/8] 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.

There is some additional complexity in the TLB invalidation code,
where we most make sure that a speculated AT instruction cannot mess
the stage-1 TLBs.

* From v2:
  - Dropped a bunch of spurious ISBs after James review
  - Added James RBs.

* From v1:
  - VHE TLB invalidation now atomic
  - Avoid speculated AT during TLB invalidation
  - Addressed most comments from Christoffer
  - Resplit to ease reviewing

Marc Zyngier (8):
  arm64: KVM: Make VHE Stage-2 TLB invalidation operations
    non-interruptible
  KVM: arm64: Rework detection of SVE, !VHE systems
  arm64: KVM: Install stage-2 translation before enabling traps
  arm64: Add TCR_EPD{0,1} definitions
  arm64: KVM: Force VHE for systems affected by erratum 1165522
  arm64: KVM: Add synchronization on translation regime change for
    erratum 1165522
  arm64: KVM: Handle ARM erratum 1165522 in TLB invalidation
  arm64: Add configuration/documentation for Cortex-A76 erratum 1165522

 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm/include/asm/kvm_host.h        |  2 +-
 arch/arm64/Kconfig                     | 12 +++++
 arch/arm64/include/asm/cpucaps.h       |  3 +-
 arch/arm64/include/asm/kvm_host.h      | 10 ++--
 arch/arm64/include/asm/kvm_hyp.h       |  8 +++
 arch/arm64/include/asm/pgtable-hwdef.h |  4 ++
 arch/arm64/kernel/cpu_errata.c         |  8 +++
 arch/arm64/kvm/hyp/switch.c            | 23 ++++++++-
 arch/arm64/kvm/hyp/tlb.c               | 71 ++++++++++++++++++++++----
 virt/kvm/arm/arm.c                     |  8 +--
 11 files changed, 129 insertions(+), 21 deletions(-)

-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 1/8] arm64: KVM: Make VHE Stage-2 TLB invalidation operations non-interruptible

[Marc Zyngier]

Contrary to the non-VHE version of the TLB invalidation helpers, the VHE
code  has interrupts enabled, meaning that we can take an interrupt in
the middle of such a sequence, and start running something else with
HCR_EL2.TGE cleared.

That's really not a good idea.

Take the heavy-handed option and disable interrupts in
__tlb_switch_to_guest_vhe, restoring them in __tlb_switch_to_host_vhe.
The latter also gain an ISB in order to make sure that TGE really has
taken effect.

Cc: stable@vger.kernel.org
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kvm/hyp/tlb.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/kvm/hyp/tlb.c b/arch/arm64/kvm/hyp/tlb.c
index 4dbd9c69a96d..7fcc9c1a5f45 100644
--- a/arch/arm64/kvm/hyp/tlb.c
+++ b/arch/arm64/kvm/hyp/tlb.c
@@ -15,14 +15,19 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/irqflags.h>
+
 #include <asm/kvm_hyp.h>
 #include <asm/kvm_mmu.h>
 #include <asm/tlbflush.h>
 
-static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm)
+static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
+						 unsigned long *flags)
 {
 	u64 val;
 
+	local_irq_save(*flags);
+
 	/*
 	 * With VHE enabled, we have HCR_EL2.{E2H,TGE} = {1,1}, and
 	 * most TLB operations target EL2/EL0. In order to affect the
@@ -37,7 +42,8 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm)
 	isb();
 }
 
-static void __hyp_text __tlb_switch_to_guest_nvhe(struct kvm *kvm)
+static void __hyp_text __tlb_switch_to_guest_nvhe(struct kvm *kvm,
+						  unsigned long *flags)
 {
 	__load_guest_stage2(kvm);
 	isb();
@@ -48,7 +54,8 @@ static hyp_alternate_select(__tlb_switch_to_guest,
 			    __tlb_switch_to_guest_vhe,
 			    ARM64_HAS_VIRT_HOST_EXTN);
 
-static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm)
+static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
+						unsigned long flags)
 {
 	/*
 	 * We're done with the TLB operation, let's restore the host's
@@ -56,9 +63,12 @@ static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm)
 	 */
 	write_sysreg(0, vttbr_el2);
 	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
+	isb();
+	local_irq_restore(flags);
 }
 
-static void __hyp_text __tlb_switch_to_host_nvhe(struct kvm *kvm)
+static void __hyp_text __tlb_switch_to_host_nvhe(struct kvm *kvm,
+						 unsigned long flags)
 {
 	write_sysreg(0, vttbr_el2);
 }
@@ -70,11 +80,13 @@ static hyp_alternate_select(__tlb_switch_to_host,
 
 void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
 {
+	unsigned long flags;
+
 	dsb(ishst);
 
 	/* Switch to requested VMID */
 	kvm = kern_hyp_va(kvm);
-	__tlb_switch_to_guest()(kvm);
+	__tlb_switch_to_guest()(kvm, &flags);
 
 	/*
 	 * We could do so much better if we had the VA as well.
@@ -117,36 +129,39 @@ void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
 	if (!has_vhe() && icache_is_vpipt())
 		__flush_icache_all();
 
-	__tlb_switch_to_host()(kvm);
+	__tlb_switch_to_host()(kvm, flags);
 }
 
 void __hyp_text __kvm_tlb_flush_vmid(struct kvm *kvm)
 {
+	unsigned long flags;
+
 	dsb(ishst);
 
 	/* Switch to requested VMID */
 	kvm = kern_hyp_va(kvm);
-	__tlb_switch_to_guest()(kvm);
+	__tlb_switch_to_guest()(kvm, &flags);
 
 	__tlbi(vmalls12e1is);
 	dsb(ish);
 	isb();
 
-	__tlb_switch_to_host()(kvm);
+	__tlb_switch_to_host()(kvm, flags);
 }
 
 void __hyp_text __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu)
 {
 	struct kvm *kvm = kern_hyp_va(kern_hyp_va(vcpu)->kvm);
+	unsigned long flags;
 
 	/* Switch to requested VMID */
-	__tlb_switch_to_guest()(kvm);
+	__tlb_switch_to_guest()(kvm, &flags);
 
 	__tlbi(vmalle1);
 	dsb(nsh);
 	isb();
 
-	__tlb_switch_to_host()(kvm);
+	__tlb_switch_to_host()(kvm, flags);
 }
 
 void __hyp_text __kvm_flush_vm_context(void)
-- 
2.19.2

[PATCH v3 1/8] arm64: KVM: Make VHE Stage-2 TLB invalidation operations non-interruptible

[Marc Zyngier]

Contrary to the non-VHE version of the TLB invalidation helpers, the VHE
code  has interrupts enabled, meaning that we can take an interrupt in
the middle of such a sequence, and start running something else with
HCR_EL2.TGE cleared.

That's really not a good idea.

Take the heavy-handed option and disable interrupts in
__tlb_switch_to_guest_vhe, restoring them in __tlb_switch_to_host_vhe.
The latter also gain an ISB in order to make sure that TGE really has
taken effect.

Cc: stable@vger.kernel.org
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kvm/hyp/tlb.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/kvm/hyp/tlb.c b/arch/arm64/kvm/hyp/tlb.c
index 4dbd9c69a96d..7fcc9c1a5f45 100644
--- a/arch/arm64/kvm/hyp/tlb.c
+++ b/arch/arm64/kvm/hyp/tlb.c
@@ -15,14 +15,19 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/irqflags.h>
+
 #include <asm/kvm_hyp.h>
 #include <asm/kvm_mmu.h>
 #include <asm/tlbflush.h>
 
-static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm)
+static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
+						 unsigned long *flags)
 {
 	u64 val;
 
+	local_irq_save(*flags);
+
 	/*
 	 * With VHE enabled, we have HCR_EL2.{E2H,TGE} = {1,1}, and
 	 * most TLB operations target EL2/EL0. In order to affect the
@@ -37,7 +42,8 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm)
 	isb();
 }
 
-static void __hyp_text __tlb_switch_to_guest_nvhe(struct kvm *kvm)
+static void __hyp_text __tlb_switch_to_guest_nvhe(struct kvm *kvm,
+						  unsigned long *flags)
 {
 	__load_guest_stage2(kvm);
 	isb();
@@ -48,7 +54,8 @@ static hyp_alternate_select(__tlb_switch_to_guest,
 			    __tlb_switch_to_guest_vhe,
 			    ARM64_HAS_VIRT_HOST_EXTN);
 
-static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm)
+static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
+						unsigned long flags)
 {
 	/*
 	 * We're done with the TLB operation, let's restore the host's
@@ -56,9 +63,12 @@ static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm)
 	 */
 	write_sysreg(0, vttbr_el2);
 	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
+	isb();
+	local_irq_restore(flags);
 }
 
-static void __hyp_text __tlb_switch_to_host_nvhe(struct kvm *kvm)
+static void __hyp_text __tlb_switch_to_host_nvhe(struct kvm *kvm,
+						 unsigned long flags)
 {
 	write_sysreg(0, vttbr_el2);
 }
@@ -70,11 +80,13 @@ static hyp_alternate_select(__tlb_switch_to_host,
 
 void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
 {
+	unsigned long flags;
+
 	dsb(ishst);
 
 	/* Switch to requested VMID */
 	kvm = kern_hyp_va(kvm);
-	__tlb_switch_to_guest()(kvm);
+	__tlb_switch_to_guest()(kvm, &flags);
 
 	/*
 	 * We could do so much better if we had the VA as well.
@@ -117,36 +129,39 @@ void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
 	if (!has_vhe() && icache_is_vpipt())
 		__flush_icache_all();
 
-	__tlb_switch_to_host()(kvm);
+	__tlb_switch_to_host()(kvm, flags);
 }
 
 void __hyp_text __kvm_tlb_flush_vmid(struct kvm *kvm)
 {
+	unsigned long flags;
+
 	dsb(ishst);
 
 	/* Switch to requested VMID */
 	kvm = kern_hyp_va(kvm);
-	__tlb_switch_to_guest()(kvm);
+	__tlb_switch_to_guest()(kvm, &flags);
 
 	__tlbi(vmalls12e1is);
 	dsb(ish);
 	isb();
 
-	__tlb_switch_to_host()(kvm);
+	__tlb_switch_to_host()(kvm, flags);
 }
 
 void __hyp_text __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu)
 {
 	struct kvm *kvm = kern_hyp_va(kern_hyp_va(vcpu)->kvm);
+	unsigned long flags;
 
 	/* Switch to requested VMID */
-	__tlb_switch_to_guest()(kvm);
+	__tlb_switch_to_guest()(kvm, &flags);
 
 	__tlbi(vmalle1);
 	dsb(nsh);
 	isb();
 
-	__tlb_switch_to_host()(kvm);
+	__tlb_switch_to_host()(kvm, flags);
 }
 
 void __hyp_text __kvm_flush_vm_context(void)
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 2/8] 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 | 6 +++---
 virt/kvm/arm/arm.c                | 8 ++++----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 5ca5d9af0c26..2184d9ddb418 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_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..d6d9aa76a943 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -422,7 +422,7 @@ 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_requires_vhe(void)
 {
 	/*
 	 * The Arm architecture specifies that implementation of SVE
@@ -430,9 +430,9 @@ static inline bool kvm_arch_check_sve_has_vhe(void)
 	 * relies on this when SVE is present:
 	 */
 	if (system_supports_sve())
-		return has_vhe();
-	else
 		return true;
+
+	return false;
 }
 
 static inline void kvm_arch_hardware_unsetup(void) {}
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 23774970c9df..1db4c15edcdd 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1640,8 +1640,10 @@ 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?");
+	in_hyp_mode = is_kernel_in_hyp_mode();
+
+	if (!in_hyp_mode && kvm_arch_requires_vhe()) {
+		kvm_pr_unimpl("CPU requiring VHE was booted in non-VHE mode");
 		return -ENODEV;
 	}
 
@@ -1657,8 +1659,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.2

[PATCH v3 2/8] 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 | 6 +++---
 virt/kvm/arm/arm.c                | 8 ++++----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 5ca5d9af0c26..2184d9ddb418 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_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..d6d9aa76a943 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -422,7 +422,7 @@ 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_requires_vhe(void)
 {
 	/*
 	 * The Arm architecture specifies that implementation of SVE
@@ -430,9 +430,9 @@ static inline bool kvm_arch_check_sve_has_vhe(void)
 	 * relies on this when SVE is present:
 	 */
 	if (system_supports_sve())
-		return has_vhe();
-	else
 		return true;
+
+	return false;
 }
 
 static inline void kvm_arch_hardware_unsetup(void) {}
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 23774970c9df..1db4c15edcdd 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1640,8 +1640,10 @@ 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?");
+	in_hyp_mode = is_kernel_in_hyp_mode();
+
+	if (!in_hyp_mode && kvm_arch_requires_vhe()) {
+		kvm_pr_unimpl("CPU requiring VHE was booted in non-VHE mode");
 		return -ENODEV;
 	}
 
@@ -1657,8 +1659,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.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 3/8] arm64: KVM: Install stage-2 translation before enabling traps

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

While we're at it, do the same thing for !VHE. It doesn't hurt,
and keeps things symmetric.

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

diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 7cc175c88a37..a8fa61c68c32 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);
@@ -545,8 +545,8 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
 
 	__sysreg_save_state_nvhe(host_ctxt);
 
-	__activate_traps(vcpu);
 	__activate_vm(kern_hyp_va(vcpu->kvm));
+	__activate_traps(vcpu);
 
 	__hyp_vgic_restore_state(vcpu);
 	__timer_enable_traps(vcpu);
-- 
2.19.2

[PATCH v3 3/8] arm64: KVM: Install stage-2 translation before enabling traps

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

While we're at it, do the same thing for !VHE. It doesn't hurt,
and keeps things symmetric.

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

diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 7cc175c88a37..a8fa61c68c32 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);
@@ -545,8 +545,8 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
 
 	__sysreg_save_state_nvhe(host_ctxt);
 
-	__activate_traps(vcpu);
 	__activate_vm(kern_hyp_va(vcpu->kvm));
+	__activate_traps(vcpu);
 
 	__hyp_vgic_restore_state(vcpu);
 	__timer_enable_traps(vcpu);
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 4/8] arm64: Add TCR_EPD{0,1} definitions

[Marc Zyngier]

We are soon going to play with TCR_EL1.EPD{0,1}, so let's add the
relevant definitions.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/pgtable-hwdef.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
index 1d7d8da2ef9b..a7d5d6e459eb 100644
--- a/arch/arm64/include/asm/pgtable-hwdef.h
+++ b/arch/arm64/include/asm/pgtable-hwdef.h
@@ -224,6 +224,8 @@
 #define TCR_TxSZ_WIDTH		6
 #define TCR_T0SZ_MASK		(((UL(1) << TCR_TxSZ_WIDTH) - 1) << TCR_T0SZ_OFFSET)
 
+#define TCR_EPD0_SHIFT		7
+#define TCR_EPD0_MASK		(UL(1) << TCR_EPD0_SHIFT)
 #define TCR_IRGN0_SHIFT		8
 #define TCR_IRGN0_MASK		(UL(3) << TCR_IRGN0_SHIFT)
 #define TCR_IRGN0_NC		(UL(0) << TCR_IRGN0_SHIFT)
@@ -231,6 +233,8 @@
 #define TCR_IRGN0_WT		(UL(2) << TCR_IRGN0_SHIFT)
 #define TCR_IRGN0_WBnWA		(UL(3) << TCR_IRGN0_SHIFT)
 
+#define TCR_EPD1_SHIFT		23
+#define TCR_EPD1_MASK		(UL(1) << TCR_EPD1_SHIFT)
 #define TCR_IRGN1_SHIFT		24
 #define TCR_IRGN1_MASK		(UL(3) << TCR_IRGN1_SHIFT)
 #define TCR_IRGN1_NC		(UL(0) << TCR_IRGN1_SHIFT)
-- 
2.19.2

[PATCH v3 4/8] arm64: Add TCR_EPD{0,1} definitions

[Marc Zyngier]

We are soon going to play with TCR_EL1.EPD{0,1}, so let's add the
relevant definitions.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/pgtable-hwdef.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
index 1d7d8da2ef9b..a7d5d6e459eb 100644
--- a/arch/arm64/include/asm/pgtable-hwdef.h
+++ b/arch/arm64/include/asm/pgtable-hwdef.h
@@ -224,6 +224,8 @@
 #define TCR_TxSZ_WIDTH		6
 #define TCR_T0SZ_MASK		(((UL(1) << TCR_TxSZ_WIDTH) - 1) << TCR_T0SZ_OFFSET)
 
+#define TCR_EPD0_SHIFT		7
+#define TCR_EPD0_MASK		(UL(1) << TCR_EPD0_SHIFT)
 #define TCR_IRGN0_SHIFT		8
 #define TCR_IRGN0_MASK		(UL(3) << TCR_IRGN0_SHIFT)
 #define TCR_IRGN0_NC		(UL(0) << TCR_IRGN0_SHIFT)
@@ -231,6 +233,8 @@
 #define TCR_IRGN0_WT		(UL(2) << TCR_IRGN0_SHIFT)
 #define TCR_IRGN0_WBnWA		(UL(3) << TCR_IRGN0_SHIFT)
 
+#define TCR_EPD1_SHIFT		23
+#define TCR_EPD1_MASK		(UL(1) << TCR_EPD1_SHIFT)
 #define TCR_IRGN1_SHIFT		24
 #define TCR_IRGN1_MASK		(UL(3) << TCR_IRGN1_SHIFT)
 #define TCR_IRGN1_NC		(UL(0) << TCR_IRGN1_SHIFT)
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 5/8] arm64: KVM: Force VHE for systems affected by erratum 1165522

[Marc Zyngier]

In order to easily mitigate ARM erratum 1165522, we need to force
affected CPUs to run in VHE mode if using KVM.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/cpucaps.h  | 3 ++-
 arch/arm64/include/asm/kvm_host.h | 4 ++++
 arch/arm64/kernel/cpu_errata.c    | 8 ++++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

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 d6d9aa76a943..9217759afa6b 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -432,6 +432,10 @@ static inline bool kvm_arch_requires_vhe(void)
 	if (system_supports_sve())
 		return true;
 
+	/* 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/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
 	{
 	}
-- 
2.19.2

[PATCH v3 5/8] arm64: KVM: Force VHE for systems affected by erratum 1165522

[Marc Zyngier]

In order to easily mitigate ARM erratum 1165522, we need to force
affected CPUs to run in VHE mode if using KVM.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/cpucaps.h  | 3 ++-
 arch/arm64/include/asm/kvm_host.h | 4 ++++
 arch/arm64/kernel/cpu_errata.c    | 8 ++++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

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 d6d9aa76a943..9217759afa6b 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -432,6 +432,10 @@ static inline bool kvm_arch_requires_vhe(void)
 	if (system_supports_sve())
 		return true;
 
+	/* 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/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
 	{
 	}
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 6/8] arm64: KVM: Add synchronization on translation regime change for erratum 1165522

[Marc Zyngier]

In order to ensure that slipping HCR_EL2.TGE is done at the right
time when switching translation regime, let insert the required ISBs
that will be patched in when erratum 1165522 is detected.

Take this opportunity to add the missing include of asm/alternative.h
which was getting there by pure luck.

Reviewed-by: James Morse <james.morse@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/kvm_hyp.h |  8 ++++++++
 arch/arm64/kvm/hyp/switch.c      | 19 +++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
index 23aca66767f9..a80a7ef57325 100644
--- a/arch/arm64/include/asm/kvm_hyp.h
+++ b/arch/arm64/include/asm/kvm_hyp.h
@@ -20,6 +20,7 @@
 
 #include <linux/compiler.h>
 #include <linux/kvm_host.h>
+#include <asm/alternative.h>
 #include <asm/sysreg.h>
 
 #define __hyp_text __section(.hyp.text) notrace
@@ -163,6 +164,13 @@ 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 EL1/EL0 translation regime used by
+	 * the guest.
+	 */
+	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
 }
 
 #endif /* __ARM64_KVM_HYP_H__ */
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index a8fa61c68c32..31ee0bfc432f 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -143,6 +143,14 @@ 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 EL2/EL0 translation regime used by
+	 * the host.
+	 */
+	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
+
 	write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
 	write_sysreg(vectors, vbar_el1);
 }
@@ -499,6 +507,17 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
 
 	sysreg_save_host_state_vhe(host_ctxt);
 
+	/*
+	 * 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.2

[PATCH v3 6/8] arm64: KVM: Add synchronization on translation regime change for erratum 1165522

[Marc Zyngier]

In order to ensure that slipping HCR_EL2.TGE is done at the right
time when switching translation regime, let insert the required ISBs
that will be patched in when erratum 1165522 is detected.

Take this opportunity to add the missing include of asm/alternative.h
which was getting there by pure luck.

Reviewed-by: James Morse <james.morse@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/kvm_hyp.h |  8 ++++++++
 arch/arm64/kvm/hyp/switch.c      | 19 +++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
index 23aca66767f9..a80a7ef57325 100644
--- a/arch/arm64/include/asm/kvm_hyp.h
+++ b/arch/arm64/include/asm/kvm_hyp.h
@@ -20,6 +20,7 @@
 
 #include <linux/compiler.h>
 #include <linux/kvm_host.h>
+#include <asm/alternative.h>
 #include <asm/sysreg.h>
 
 #define __hyp_text __section(.hyp.text) notrace
@@ -163,6 +164,13 @@ 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 EL1/EL0 translation regime used by
+	 * the guest.
+	 */
+	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
 }
 
 #endif /* __ARM64_KVM_HYP_H__ */
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index a8fa61c68c32..31ee0bfc432f 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -143,6 +143,14 @@ 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 EL2/EL0 translation regime used by
+	 * the host.
+	 */
+	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
+
 	write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
 	write_sysreg(vectors, vbar_el1);
 }
@@ -499,6 +507,17 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
 
 	sysreg_save_host_state_vhe(host_ctxt);
 
+	/*
+	 * 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.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 7/8] arm64: KVM: Handle ARM erratum 1165522 in TLB invalidation

[Marc Zyngier]

In order to avoid TLB corruption whilst invalidating TLBs on CPUs
affected by erratum 1165522, we need to prevent S1 page tables
from being usable.

For this, we set the EL1 S1 MMU on, and also disable the page table
walker (by setting the TCR_EL1.EPD* bits to 1).

This ensures that once we switch to the EL1/EL0 translation regime,
speculated AT instructions won't be able to parse the page tables.

Reviewed-by: James Morse <james.morse@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kvm/hyp/tlb.c | 66 +++++++++++++++++++++++++++++++---------
 1 file changed, 51 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/kvm/hyp/tlb.c b/arch/arm64/kvm/hyp/tlb.c
index 7fcc9c1a5f45..ec157543d5a9 100644
--- a/arch/arm64/kvm/hyp/tlb.c
+++ b/arch/arm64/kvm/hyp/tlb.c
@@ -21,12 +21,36 @@
 #include <asm/kvm_mmu.h>
 #include <asm/tlbflush.h>
 
+struct tlb_inv_context {
+	unsigned long	flags;
+	u64		tcr;
+	u64		sctlr;
+};
+
 static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
-						 unsigned long *flags)
+						 struct tlb_inv_context *cxt)
 {
 	u64 val;
 
-	local_irq_save(*flags);
+	local_irq_save(cxt->flags);
+
+	if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
+		/*
+		 * For CPUs that are affected by ARM erratum 1165522, we
+		 * cannot trust stage-1 to be in a correct state at that
+		 * point. Since we do not want to force a full load of the
+		 * vcpu state, we prevent the EL1 page-table walker to
+		 * allocate new TLBs. This is done by setting the EPD bits
+		 * in the TCR_EL1 register. We also need to prevent it to
+		 * allocate IPA->PA walks, so we enable the S1 MMU...
+		 */
+		val = cxt->tcr = read_sysreg_el1(tcr);
+		val |= TCR_EPD1_MASK | TCR_EPD0_MASK;
+		write_sysreg_el1(val, tcr);
+		val = cxt->sctlr = read_sysreg_el1(sctlr);
+		val |= SCTLR_ELx_M;
+		write_sysreg_el1(val, sctlr);
+	}
 
 	/*
 	 * With VHE enabled, we have HCR_EL2.{E2H,TGE} = {1,1}, and
@@ -34,6 +58,11 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
 	 * guest TLBs (EL1/EL0), we need to change one of these two
 	 * bits. Changing E2H is impossible (goodbye TTBR1_EL2), so
 	 * let's flip TGE before executing the TLB operation.
+	 *
+	 * ARM erratum 1165522 requires some special handling (again),
+	 * as we need to make sure both stages of translation are in
+	 * place before clearing TGE. __load_guest_stage2() already
+	 * has an ISB in order to deal with this.
 	 */
 	__load_guest_stage2(kvm);
 	val = read_sysreg(hcr_el2);
@@ -43,7 +72,7 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
 }
 
 static void __hyp_text __tlb_switch_to_guest_nvhe(struct kvm *kvm,
-						  unsigned long *flags)
+						  struct tlb_inv_context *cxt)
 {
 	__load_guest_stage2(kvm);
 	isb();
@@ -55,7 +84,7 @@ static hyp_alternate_select(__tlb_switch_to_guest,
 			    ARM64_HAS_VIRT_HOST_EXTN);
 
 static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
-						unsigned long flags)
+						struct tlb_inv_context *cxt)
 {
 	/*
 	 * We're done with the TLB operation, let's restore the host's
@@ -64,11 +93,18 @@ static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
 	write_sysreg(0, vttbr_el2);
 	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
 	isb();
-	local_irq_restore(flags);
+
+	if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
+		/* Restore the guest's registers to what they were */
+		write_sysreg_el1(cxt->tcr, tcr);
+		write_sysreg_el1(cxt->sctlr, sctlr);
+	}
+
+	local_irq_restore(cxt->flags);
 }
 
 static void __hyp_text __tlb_switch_to_host_nvhe(struct kvm *kvm,
-						 unsigned long flags)
+						 struct tlb_inv_context *cxt)
 {
 	write_sysreg(0, vttbr_el2);
 }
@@ -80,13 +116,13 @@ static hyp_alternate_select(__tlb_switch_to_host,
 
 void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
 {
-	unsigned long flags;
+	struct tlb_inv_context cxt;
 
 	dsb(ishst);
 
 	/* Switch to requested VMID */
 	kvm = kern_hyp_va(kvm);
-	__tlb_switch_to_guest()(kvm, &flags);
+	__tlb_switch_to_guest()(kvm, &cxt);
 
 	/*
 	 * We could do so much better if we had the VA as well.
@@ -129,39 +165,39 @@ void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
 	if (!has_vhe() && icache_is_vpipt())
 		__flush_icache_all();
 
-	__tlb_switch_to_host()(kvm, flags);
+	__tlb_switch_to_host()(kvm, &cxt);
 }
 
 void __hyp_text __kvm_tlb_flush_vmid(struct kvm *kvm)
 {
-	unsigned long flags;
+	struct tlb_inv_context cxt;
 
 	dsb(ishst);
 
 	/* Switch to requested VMID */
 	kvm = kern_hyp_va(kvm);
-	__tlb_switch_to_guest()(kvm, &flags);
+	__tlb_switch_to_guest()(kvm, &cxt);
 
 	__tlbi(vmalls12e1is);
 	dsb(ish);
 	isb();
 
-	__tlb_switch_to_host()(kvm, flags);
+	__tlb_switch_to_host()(kvm, &cxt);
 }
 
 void __hyp_text __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu)
 {
 	struct kvm *kvm = kern_hyp_va(kern_hyp_va(vcpu)->kvm);
-	unsigned long flags;
+	struct tlb_inv_context cxt;
 
 	/* Switch to requested VMID */
-	__tlb_switch_to_guest()(kvm, &flags);
+	__tlb_switch_to_guest()(kvm, &cxt);
 
 	__tlbi(vmalle1);
 	dsb(nsh);
 	isb();
 
-	__tlb_switch_to_host()(kvm, flags);
+	__tlb_switch_to_host()(kvm, &cxt);
 }
 
 void __hyp_text __kvm_flush_vm_context(void)
-- 
2.19.2

[PATCH v3 7/8] arm64: KVM: Handle ARM erratum 1165522 in TLB invalidation

[Marc Zyngier]

In order to avoid TLB corruption whilst invalidating TLBs on CPUs
affected by erratum 1165522, we need to prevent S1 page tables
from being usable.

For this, we set the EL1 S1 MMU on, and also disable the page table
walker (by setting the TCR_EL1.EPD* bits to 1).

This ensures that once we switch to the EL1/EL0 translation regime,
speculated AT instructions won't be able to parse the page tables.

Reviewed-by: James Morse <james.morse@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kvm/hyp/tlb.c | 66 +++++++++++++++++++++++++++++++---------
 1 file changed, 51 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/kvm/hyp/tlb.c b/arch/arm64/kvm/hyp/tlb.c
index 7fcc9c1a5f45..ec157543d5a9 100644
--- a/arch/arm64/kvm/hyp/tlb.c
+++ b/arch/arm64/kvm/hyp/tlb.c
@@ -21,12 +21,36 @@
 #include <asm/kvm_mmu.h>
 #include <asm/tlbflush.h>
 
+struct tlb_inv_context {
+	unsigned long	flags;
+	u64		tcr;
+	u64		sctlr;
+};
+
 static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
-						 unsigned long *flags)
+						 struct tlb_inv_context *cxt)
 {
 	u64 val;
 
-	local_irq_save(*flags);
+	local_irq_save(cxt->flags);
+
+	if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
+		/*
+		 * For CPUs that are affected by ARM erratum 1165522, we
+		 * cannot trust stage-1 to be in a correct state at that
+		 * point. Since we do not want to force a full load of the
+		 * vcpu state, we prevent the EL1 page-table walker to
+		 * allocate new TLBs. This is done by setting the EPD bits
+		 * in the TCR_EL1 register. We also need to prevent it to
+		 * allocate IPA->PA walks, so we enable the S1 MMU...
+		 */
+		val = cxt->tcr = read_sysreg_el1(tcr);
+		val |= TCR_EPD1_MASK | TCR_EPD0_MASK;
+		write_sysreg_el1(val, tcr);
+		val = cxt->sctlr = read_sysreg_el1(sctlr);
+		val |= SCTLR_ELx_M;
+		write_sysreg_el1(val, sctlr);
+	}
 
 	/*
 	 * With VHE enabled, we have HCR_EL2.{E2H,TGE} = {1,1}, and
@@ -34,6 +58,11 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
 	 * guest TLBs (EL1/EL0), we need to change one of these two
 	 * bits. Changing E2H is impossible (goodbye TTBR1_EL2), so
 	 * let's flip TGE before executing the TLB operation.
+	 *
+	 * ARM erratum 1165522 requires some special handling (again),
+	 * as we need to make sure both stages of translation are in
+	 * place before clearing TGE. __load_guest_stage2() already
+	 * has an ISB in order to deal with this.
 	 */
 	__load_guest_stage2(kvm);
 	val = read_sysreg(hcr_el2);
@@ -43,7 +72,7 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
 }
 
 static void __hyp_text __tlb_switch_to_guest_nvhe(struct kvm *kvm,
-						  unsigned long *flags)
+						  struct tlb_inv_context *cxt)
 {
 	__load_guest_stage2(kvm);
 	isb();
@@ -55,7 +84,7 @@ static hyp_alternate_select(__tlb_switch_to_guest,
 			    ARM64_HAS_VIRT_HOST_EXTN);
 
 static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
-						unsigned long flags)
+						struct tlb_inv_context *cxt)
 {
 	/*
 	 * We're done with the TLB operation, let's restore the host's
@@ -64,11 +93,18 @@ static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
 	write_sysreg(0, vttbr_el2);
 	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
 	isb();
-	local_irq_restore(flags);
+
+	if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
+		/* Restore the guest's registers to what they were */
+		write_sysreg_el1(cxt->tcr, tcr);
+		write_sysreg_el1(cxt->sctlr, sctlr);
+	}
+
+	local_irq_restore(cxt->flags);
 }
 
 static void __hyp_text __tlb_switch_to_host_nvhe(struct kvm *kvm,
-						 unsigned long flags)
+						 struct tlb_inv_context *cxt)
 {
 	write_sysreg(0, vttbr_el2);
 }
@@ -80,13 +116,13 @@ static hyp_alternate_select(__tlb_switch_to_host,
 
 void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
 {
-	unsigned long flags;
+	struct tlb_inv_context cxt;
 
 	dsb(ishst);
 
 	/* Switch to requested VMID */
 	kvm = kern_hyp_va(kvm);
-	__tlb_switch_to_guest()(kvm, &flags);
+	__tlb_switch_to_guest()(kvm, &cxt);
 
 	/*
 	 * We could do so much better if we had the VA as well.
@@ -129,39 +165,39 @@ void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
 	if (!has_vhe() && icache_is_vpipt())
 		__flush_icache_all();
 
-	__tlb_switch_to_host()(kvm, flags);
+	__tlb_switch_to_host()(kvm, &cxt);
 }
 
 void __hyp_text __kvm_tlb_flush_vmid(struct kvm *kvm)
 {
-	unsigned long flags;
+	struct tlb_inv_context cxt;
 
 	dsb(ishst);
 
 	/* Switch to requested VMID */
 	kvm = kern_hyp_va(kvm);
-	__tlb_switch_to_guest()(kvm, &flags);
+	__tlb_switch_to_guest()(kvm, &cxt);
 
 	__tlbi(vmalls12e1is);
 	dsb(ish);
 	isb();
 
-	__tlb_switch_to_host()(kvm, flags);
+	__tlb_switch_to_host()(kvm, &cxt);
 }
 
 void __hyp_text __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu)
 {
 	struct kvm *kvm = kern_hyp_va(kern_hyp_va(vcpu)->kvm);
-	unsigned long flags;
+	struct tlb_inv_context cxt;
 
 	/* Switch to requested VMID */
-	__tlb_switch_to_guest()(kvm, &flags);
+	__tlb_switch_to_guest()(kvm, &cxt);
 
 	__tlbi(vmalle1);
 	dsb(nsh);
 	isb();
 
-	__tlb_switch_to_host()(kvm, flags);
+	__tlb_switch_to_host()(kvm, &cxt);
 }
 
 void __hyp_text __kvm_flush_vm_context(void)
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 8/8] arm64: Add configuration/documentation for Cortex-A76 erratum 1165522

[Marc Zyngier]

Now that the infrastructure to handle erratum 1165522 is in place,
let's make it a selectable option and add the required documentation.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig                     | 12 ++++++++++++
 2 files changed, 13 insertions(+)

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
-- 
2.19.2

[PATCH v3 8/8] arm64: Add configuration/documentation for Cortex-A76 erratum 1165522

[Marc Zyngier]

Now that the infrastructure to handle erratum 1165522 is in place,
let's make it a selectable option and add the required documentation.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig                     | 12 ++++++++++++
 2 files changed, 13 insertions(+)

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
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 0/8] Workaround for Cortex-A76 erratum 1165522

[James Morse]

Hi Marc,

On 06/12/2018 17:31, Marc Zyngier wrote:
> 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.
> 
> There is some additional complexity in the TLB invalidation code,
> where we most make sure that a speculated AT instruction cannot mess
> the stage-1 TLBs.

For the series:
Reviewed-by: James Morse <james.morse@arm.com>


Thanks,

James

Re: [PATCH v3 0/8] Workaround for Cortex-A76 erratum 1165522

[James Morse]

Hi Marc,

On 06/12/2018 17:31, Marc Zyngier wrote:
> 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.
> 
> There is some additional complexity in the TLB invalidation code,
> where we most make sure that a speculated AT instruction cannot mess
> the stage-1 TLBs.

For the series:
Reviewed-by: James Morse <james.morse@arm.com>


Thanks,

James

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/8] arm64: KVM: Make VHE Stage-2 TLB invalidation operations non-interruptible

[Christoffer Dall]

On Thu, Dec 06, 2018 at 05:31:19PM +0000, Marc Zyngier wrote:
> Contrary to the non-VHE version of the TLB invalidation helpers, the VHE
> code  has interrupts enabled, meaning that we can take an interrupt in
> the middle of such a sequence, and start running something else with
> HCR_EL2.TGE cleared.

Do we have to clear TGE to perform the TLB invalidation, or is that just
a side-effect of re-using code?

Also, do we generally perform TLB invalidations in the kernel with
interrupts disabled, or is this just a result of clearing TGE?

Somehow I feel like this should look more like just another TLB
invalidation in the kernel, but if there's a good reason why it can't
then this is fine.

Thanks,

    Christoffer

> 
> That's really not a good idea.
> 
> Take the heavy-handed option and disable interrupts in
> __tlb_switch_to_guest_vhe, restoring them in __tlb_switch_to_host_vhe.
> The latter also gain an ISB in order to make sure that TGE really has
> taken effect.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm64/kvm/hyp/tlb.c | 35 +++++++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/tlb.c b/arch/arm64/kvm/hyp/tlb.c
> index 4dbd9c69a96d..7fcc9c1a5f45 100644
> --- a/arch/arm64/kvm/hyp/tlb.c
> +++ b/arch/arm64/kvm/hyp/tlb.c
> @@ -15,14 +15,19 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <linux/irqflags.h>
> +
>  #include <asm/kvm_hyp.h>
>  #include <asm/kvm_mmu.h>
>  #include <asm/tlbflush.h>
>  
> -static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm)
> +static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
> +						 unsigned long *flags)
>  {
>  	u64 val;
>  
> +	local_irq_save(*flags);
> +
>  	/*
>  	 * With VHE enabled, we have HCR_EL2.{E2H,TGE} = {1,1}, and
>  	 * most TLB operations target EL2/EL0. In order to affect the
> @@ -37,7 +42,8 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm)
>  	isb();
>  }
>  
> -static void __hyp_text __tlb_switch_to_guest_nvhe(struct kvm *kvm)
> +static void __hyp_text __tlb_switch_to_guest_nvhe(struct kvm *kvm,
> +						  unsigned long *flags)
>  {
>  	__load_guest_stage2(kvm);
>  	isb();
> @@ -48,7 +54,8 @@ static hyp_alternate_select(__tlb_switch_to_guest,
>  			    __tlb_switch_to_guest_vhe,
>  			    ARM64_HAS_VIRT_HOST_EXTN);
>  
> -static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm)
> +static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
> +						unsigned long flags)
>  {
>  	/*
>  	 * We're done with the TLB operation, let's restore the host's
> @@ -56,9 +63,12 @@ static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm)
>  	 */
>  	write_sysreg(0, vttbr_el2);
>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
> +	isb();
> +	local_irq_restore(flags);
>  }
>  
> -static void __hyp_text __tlb_switch_to_host_nvhe(struct kvm *kvm)
> +static void __hyp_text __tlb_switch_to_host_nvhe(struct kvm *kvm,
> +						 unsigned long flags)
>  {
>  	write_sysreg(0, vttbr_el2);
>  }
> @@ -70,11 +80,13 @@ static hyp_alternate_select(__tlb_switch_to_host,
>  
>  void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
>  {
> +	unsigned long flags;
> +
>  	dsb(ishst);
>  
>  	/* Switch to requested VMID */
>  	kvm = kern_hyp_va(kvm);
> -	__tlb_switch_to_guest()(kvm);
> +	__tlb_switch_to_guest()(kvm, &flags);
>  
>  	/*
>  	 * We could do so much better if we had the VA as well.
> @@ -117,36 +129,39 @@ void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
>  	if (!has_vhe() && icache_is_vpipt())
>  		__flush_icache_all();
>  
> -	__tlb_switch_to_host()(kvm);
> +	__tlb_switch_to_host()(kvm, flags);
>  }
>  
>  void __hyp_text __kvm_tlb_flush_vmid(struct kvm *kvm)
>  {
> +	unsigned long flags;
> +
>  	dsb(ishst);
>  
>  	/* Switch to requested VMID */
>  	kvm = kern_hyp_va(kvm);
> -	__tlb_switch_to_guest()(kvm);
> +	__tlb_switch_to_guest()(kvm, &flags);
>  
>  	__tlbi(vmalls12e1is);
>  	dsb(ish);
>  	isb();
>  
> -	__tlb_switch_to_host()(kvm);
> +	__tlb_switch_to_host()(kvm, flags);
>  }
>  
>  void __hyp_text __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm *kvm = kern_hyp_va(kern_hyp_va(vcpu)->kvm);
> +	unsigned long flags;
>  
>  	/* Switch to requested VMID */
> -	__tlb_switch_to_guest()(kvm);
> +	__tlb_switch_to_guest()(kvm, &flags);
>  
>  	__tlbi(vmalle1);
>  	dsb(nsh);
>  	isb();
>  
> -	__tlb_switch_to_host()(kvm);
> +	__tlb_switch_to_host()(kvm, flags);
>  }
>  
>  void __hyp_text __kvm_flush_vm_context(void)
> -- 
> 2.19.2
> 

Re: [PATCH v3 1/8] arm64: KVM: Make VHE Stage-2 TLB invalidation operations non-interruptible

[Christoffer Dall]

On Thu, Dec 06, 2018 at 05:31:19PM +0000, Marc Zyngier wrote:
> Contrary to the non-VHE version of the TLB invalidation helpers, the VHE
> code  has interrupts enabled, meaning that we can take an interrupt in
> the middle of such a sequence, and start running something else with
> HCR_EL2.TGE cleared.

Do we have to clear TGE to perform the TLB invalidation, or is that just
a side-effect of re-using code?

Also, do we generally perform TLB invalidations in the kernel with
interrupts disabled, or is this just a result of clearing TGE?

Somehow I feel like this should look more like just another TLB
invalidation in the kernel, but if there's a good reason why it can't
then this is fine.

Thanks,

    Christoffer

> 
> That's really not a good idea.
> 
> Take the heavy-handed option and disable interrupts in
> __tlb_switch_to_guest_vhe, restoring them in __tlb_switch_to_host_vhe.
> The latter also gain an ISB in order to make sure that TGE really has
> taken effect.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm64/kvm/hyp/tlb.c | 35 +++++++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/tlb.c b/arch/arm64/kvm/hyp/tlb.c
> index 4dbd9c69a96d..7fcc9c1a5f45 100644
> --- a/arch/arm64/kvm/hyp/tlb.c
> +++ b/arch/arm64/kvm/hyp/tlb.c
> @@ -15,14 +15,19 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <linux/irqflags.h>
> +
>  #include <asm/kvm_hyp.h>
>  #include <asm/kvm_mmu.h>
>  #include <asm/tlbflush.h>
>  
> -static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm)
> +static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
> +						 unsigned long *flags)
>  {
>  	u64 val;
>  
> +	local_irq_save(*flags);
> +
>  	/*
>  	 * With VHE enabled, we have HCR_EL2.{E2H,TGE} = {1,1}, and
>  	 * most TLB operations target EL2/EL0. In order to affect the
> @@ -37,7 +42,8 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm)
>  	isb();
>  }
>  
> -static void __hyp_text __tlb_switch_to_guest_nvhe(struct kvm *kvm)
> +static void __hyp_text __tlb_switch_to_guest_nvhe(struct kvm *kvm,
> +						  unsigned long *flags)
>  {
>  	__load_guest_stage2(kvm);
>  	isb();
> @@ -48,7 +54,8 @@ static hyp_alternate_select(__tlb_switch_to_guest,
>  			    __tlb_switch_to_guest_vhe,
>  			    ARM64_HAS_VIRT_HOST_EXTN);
>  
> -static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm)
> +static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
> +						unsigned long flags)
>  {
>  	/*
>  	 * We're done with the TLB operation, let's restore the host's
> @@ -56,9 +63,12 @@ static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm)
>  	 */
>  	write_sysreg(0, vttbr_el2);
>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
> +	isb();
> +	local_irq_restore(flags);
>  }
>  
> -static void __hyp_text __tlb_switch_to_host_nvhe(struct kvm *kvm)
> +static void __hyp_text __tlb_switch_to_host_nvhe(struct kvm *kvm,
> +						 unsigned long flags)
>  {
>  	write_sysreg(0, vttbr_el2);
>  }
> @@ -70,11 +80,13 @@ static hyp_alternate_select(__tlb_switch_to_host,
>  
>  void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
>  {
> +	unsigned long flags;
> +
>  	dsb(ishst);
>  
>  	/* Switch to requested VMID */
>  	kvm = kern_hyp_va(kvm);
> -	__tlb_switch_to_guest()(kvm);
> +	__tlb_switch_to_guest()(kvm, &flags);
>  
>  	/*
>  	 * We could do so much better if we had the VA as well.
> @@ -117,36 +129,39 @@ void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
>  	if (!has_vhe() && icache_is_vpipt())
>  		__flush_icache_all();
>  
> -	__tlb_switch_to_host()(kvm);
> +	__tlb_switch_to_host()(kvm, flags);
>  }
>  
>  void __hyp_text __kvm_tlb_flush_vmid(struct kvm *kvm)
>  {
> +	unsigned long flags;
> +
>  	dsb(ishst);
>  
>  	/* Switch to requested VMID */
>  	kvm = kern_hyp_va(kvm);
> -	__tlb_switch_to_guest()(kvm);
> +	__tlb_switch_to_guest()(kvm, &flags);
>  
>  	__tlbi(vmalls12e1is);
>  	dsb(ish);
>  	isb();
>  
> -	__tlb_switch_to_host()(kvm);
> +	__tlb_switch_to_host()(kvm, flags);
>  }
>  
>  void __hyp_text __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm *kvm = kern_hyp_va(kern_hyp_va(vcpu)->kvm);
> +	unsigned long flags;
>  
>  	/* Switch to requested VMID */
> -	__tlb_switch_to_guest()(kvm);
> +	__tlb_switch_to_guest()(kvm, &flags);
>  
>  	__tlbi(vmalle1);
>  	dsb(nsh);
>  	isb();
>  
> -	__tlb_switch_to_host()(kvm);
> +	__tlb_switch_to_host()(kvm, flags);
>  }
>  
>  void __hyp_text __kvm_flush_vm_context(void)
> -- 
> 2.19.2
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 2/8] KVM: arm64: Rework detection of SVE, !VHE systems

[Christoffer Dall]

On Thu, Dec 06, 2018 at 05:31:20PM +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 | 6 +++---
>  virt/kvm/arm/arm.c                | 8 ++++----
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 5ca5d9af0c26..2184d9ddb418 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_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..d6d9aa76a943 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -422,7 +422,7 @@ 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_requires_vhe(void)
>  {
>  	/*
>  	 * The Arm architecture specifies that implementation of SVE
> @@ -430,9 +430,9 @@ static inline bool kvm_arch_check_sve_has_vhe(void)
>  	 * relies on this when SVE is present:
>  	 */
>  	if (system_supports_sve())
> -		return has_vhe();
> -	else
>  		return true;
> +
> +	return false;
>  }
>  
>  static inline void kvm_arch_hardware_unsetup(void) {}
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 23774970c9df..1db4c15edcdd 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1640,8 +1640,10 @@ 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?");
> +	in_hyp_mode = is_kernel_in_hyp_mode();
> +
> +	if (!in_hyp_mode && kvm_arch_requires_vhe()) {
> +		kvm_pr_unimpl("CPU requiring VHE was booted in non-VHE mode");

nit: The error message feels weird to me (are we reporting CPU bugs?)
and I'm not sure about the unimpl and I think there's a linse space
missing.

How about:

	kvm_err("Cannot support this CPU in non-VHE mode, not initializing\n");

If we wanted to be super helpful, we could expand
kvm_arch_requires_vhe() with a print statement saying:
		
	kvm_err("SVE detected, requiring VHE mode\n");

But thay may be overkill.


>  		return -ENODEV;
>  	}
>  
> @@ -1657,8 +1659,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.2
> 

Otherwise:

Acked-by: Christoffer Dall <christoffer.dall@arm.com>

Re: [PATCH v3 2/8] KVM: arm64: Rework detection of SVE, !VHE systems

[Christoffer Dall]

On Thu, Dec 06, 2018 at 05:31:20PM +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 | 6 +++---
>  virt/kvm/arm/arm.c                | 8 ++++----
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 5ca5d9af0c26..2184d9ddb418 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_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..d6d9aa76a943 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -422,7 +422,7 @@ 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_requires_vhe(void)
>  {
>  	/*
>  	 * The Arm architecture specifies that implementation of SVE
> @@ -430,9 +430,9 @@ static inline bool kvm_arch_check_sve_has_vhe(void)
>  	 * relies on this when SVE is present:
>  	 */
>  	if (system_supports_sve())
> -		return has_vhe();
> -	else
>  		return true;
> +
> +	return false;
>  }
>  
>  static inline void kvm_arch_hardware_unsetup(void) {}
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 23774970c9df..1db4c15edcdd 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1640,8 +1640,10 @@ 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?");
> +	in_hyp_mode = is_kernel_in_hyp_mode();
> +
> +	if (!in_hyp_mode && kvm_arch_requires_vhe()) {
> +		kvm_pr_unimpl("CPU requiring VHE was booted in non-VHE mode");

nit: The error message feels weird to me (are we reporting CPU bugs?)
and I'm not sure about the unimpl and I think there's a linse space
missing.

How about:

	kvm_err("Cannot support this CPU in non-VHE mode, not initializing\n");

If we wanted to be super helpful, we could expand
kvm_arch_requires_vhe() with a print statement saying:
		
	kvm_err("SVE detected, requiring VHE mode\n");

But thay may be overkill.


>  		return -ENODEV;
>  	}
>  
> @@ -1657,8 +1659,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.2
> 

Otherwise:

Acked-by: Christoffer Dall <christoffer.dall@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 3/8] arm64: KVM: Install stage-2 translation before enabling traps

[Christoffer Dall]

On Thu, Dec 06, 2018 at 05:31:21PM +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.
> 
> While we're at it, do the same thing for !VHE. It doesn't hurt,
> and keeps things symmetric.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm64/kvm/hyp/switch.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 7cc175c88a37..a8fa61c68c32 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);
> @@ -545,8 +545,8 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
>  
>  	__sysreg_save_state_nvhe(host_ctxt);
>  
> -	__activate_traps(vcpu);
>  	__activate_vm(kern_hyp_va(vcpu->kvm));
> +	__activate_traps(vcpu);
>  
>  	__hyp_vgic_restore_state(vcpu);
>  	__timer_enable_traps(vcpu);
> -- 
> 2.19.2
> 

Acked-by: Christoffer Dall <christoffer.dall@arm.com>

Re: [PATCH v3 3/8] arm64: KVM: Install stage-2 translation before enabling traps

[Christoffer Dall]

On Thu, Dec 06, 2018 at 05:31:21PM +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.
> 
> While we're at it, do the same thing for !VHE. It doesn't hurt,
> and keeps things symmetric.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm64/kvm/hyp/switch.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 7cc175c88a37..a8fa61c68c32 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);
> @@ -545,8 +545,8 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
>  
>  	__sysreg_save_state_nvhe(host_ctxt);
>  
> -	__activate_traps(vcpu);
>  	__activate_vm(kern_hyp_va(vcpu->kvm));
> +	__activate_traps(vcpu);
>  
>  	__hyp_vgic_restore_state(vcpu);
>  	__timer_enable_traps(vcpu);
> -- 
> 2.19.2
> 

Acked-by: Christoffer Dall <christoffer.dall@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 6/8] arm64: KVM: Add synchronization on translation regime change for erratum 1165522

[Christoffer Dall]

On Thu, Dec 06, 2018 at 05:31:24PM +0000, Marc Zyngier wrote:
> In order to ensure that slipping HCR_EL2.TGE is done at the right
> time when switching translation regime, let insert the required ISBs
> that will be patched in when erratum 1165522 is detected.
> 
> Take this opportunity to add the missing include of asm/alternative.h
> which was getting there by pure luck.
> 
> Reviewed-by: James Morse <james.morse@arm.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm64/include/asm/kvm_hyp.h |  8 ++++++++
>  arch/arm64/kvm/hyp/switch.c      | 19 +++++++++++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> index 23aca66767f9..a80a7ef57325 100644
> --- a/arch/arm64/include/asm/kvm_hyp.h
> +++ b/arch/arm64/include/asm/kvm_hyp.h
> @@ -20,6 +20,7 @@
>  
>  #include <linux/compiler.h>
>  #include <linux/kvm_host.h>
> +#include <asm/alternative.h>
>  #include <asm/sysreg.h>
>  
>  #define __hyp_text __section(.hyp.text) notrace
> @@ -163,6 +164,13 @@ 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 EL1/EL0 translation regime used by
> +	 * the guest.
> +	 */
> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
>  }
>  
>  #endif /* __ARM64_KVM_HYP_H__ */
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index a8fa61c68c32..31ee0bfc432f 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -143,6 +143,14 @@ 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 EL2/EL0 translation regime used by
> +	 * the host.
> +	 */
> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
> +
>  	write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
>  	write_sysreg(vectors, vbar_el1);
>  }
> @@ -499,6 +507,17 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>  
>  	sysreg_save_host_state_vhe(host_ctxt);
>  
> +	/*
> +	 * 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.2
> 

Acked-by: Christoffer Dall <christoffer.dall@arm.com>

Re: [PATCH v3 6/8] arm64: KVM: Add synchronization on translation regime change for erratum 1165522

[Christoffer Dall]

On Thu, Dec 06, 2018 at 05:31:24PM +0000, Marc Zyngier wrote:
> In order to ensure that slipping HCR_EL2.TGE is done at the right
> time when switching translation regime, let insert the required ISBs
> that will be patched in when erratum 1165522 is detected.
> 
> Take this opportunity to add the missing include of asm/alternative.h
> which was getting there by pure luck.
> 
> Reviewed-by: James Morse <james.morse@arm.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm64/include/asm/kvm_hyp.h |  8 ++++++++
>  arch/arm64/kvm/hyp/switch.c      | 19 +++++++++++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> index 23aca66767f9..a80a7ef57325 100644
> --- a/arch/arm64/include/asm/kvm_hyp.h
> +++ b/arch/arm64/include/asm/kvm_hyp.h
> @@ -20,6 +20,7 @@
>  
>  #include <linux/compiler.h>
>  #include <linux/kvm_host.h>
> +#include <asm/alternative.h>
>  #include <asm/sysreg.h>
>  
>  #define __hyp_text __section(.hyp.text) notrace
> @@ -163,6 +164,13 @@ 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 EL1/EL0 translation regime used by
> +	 * the guest.
> +	 */
> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
>  }
>  
>  #endif /* __ARM64_KVM_HYP_H__ */
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index a8fa61c68c32..31ee0bfc432f 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -143,6 +143,14 @@ 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 EL2/EL0 translation regime used by
> +	 * the host.
> +	 */
> +	asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522));
> +
>  	write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1);
>  	write_sysreg(vectors, vbar_el1);
>  }
> @@ -499,6 +507,17 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>  
>  	sysreg_save_host_state_vhe(host_ctxt);
>  
> +	/*
> +	 * 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.2
> 

Acked-by: Christoffer Dall <christoffer.dall@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 7/8] arm64: KVM: Handle ARM erratum 1165522 in TLB invalidation

[Christoffer Dall]

On Thu, Dec 06, 2018 at 05:31:25PM +0000, Marc Zyngier wrote:
> In order to avoid TLB corruption whilst invalidating TLBs on CPUs
> affected by erratum 1165522, we need to prevent S1 page tables
> from being usable.
> 
> For this, we set the EL1 S1 MMU on, and also disable the page table
> walker (by setting the TCR_EL1.EPD* bits to 1).
> 
> This ensures that once we switch to the EL1/EL0 translation regime,
> speculated AT instructions won't be able to parse the page tables.
> 
> Reviewed-by: James Morse <james.morse@arm.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm64/kvm/hyp/tlb.c | 66 +++++++++++++++++++++++++++++++---------
>  1 file changed, 51 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/tlb.c b/arch/arm64/kvm/hyp/tlb.c
> index 7fcc9c1a5f45..ec157543d5a9 100644
> --- a/arch/arm64/kvm/hyp/tlb.c
> +++ b/arch/arm64/kvm/hyp/tlb.c
> @@ -21,12 +21,36 @@
>  #include <asm/kvm_mmu.h>
>  #include <asm/tlbflush.h>
>  
> +struct tlb_inv_context {
> +	unsigned long	flags;
> +	u64		tcr;
> +	u64		sctlr;
> +};
> +
>  static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
> -						 unsigned long *flags)
> +						 struct tlb_inv_context *cxt)
>  {
>  	u64 val;
>  
> -	local_irq_save(*flags);
> +	local_irq_save(cxt->flags);
> +
> +	if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
> +		/*
> +		 * For CPUs that are affected by ARM erratum 1165522, we
> +		 * cannot trust stage-1 to be in a correct state at that
> +		 * point. Since we do not want to force a full load of the
> +		 * vcpu state, we prevent the EL1 page-table walker to
> +		 * allocate new TLBs. This is done by setting the EPD bits
> +		 * in the TCR_EL1 register. We also need to prevent it to
> +		 * allocate IPA->PA walks, so we enable the S1 MMU...
> +		 */
> +		val = cxt->tcr = read_sysreg_el1(tcr);
> +		val |= TCR_EPD1_MASK | TCR_EPD0_MASK;
> +		write_sysreg_el1(val, tcr);
> +		val = cxt->sctlr = read_sysreg_el1(sctlr);
> +		val |= SCTLR_ELx_M;
> +		write_sysreg_el1(val, sctlr);
> +	}
>  
>  	/*
>  	 * With VHE enabled, we have HCR_EL2.{E2H,TGE} = {1,1}, and
> @@ -34,6 +58,11 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
>  	 * guest TLBs (EL1/EL0), we need to change one of these two
>  	 * bits. Changing E2H is impossible (goodbye TTBR1_EL2), so
>  	 * let's flip TGE before executing the TLB operation.
> +	 *
> +	 * ARM erratum 1165522 requires some special handling (again),
> +	 * as we need to make sure both stages of translation are in
> +	 * place before clearing TGE. __load_guest_stage2() already
> +	 * has an ISB in order to deal with this.
>  	 */
>  	__load_guest_stage2(kvm);
>  	val = read_sysreg(hcr_el2);
> @@ -43,7 +72,7 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
>  }
>  
>  static void __hyp_text __tlb_switch_to_guest_nvhe(struct kvm *kvm,
> -						  unsigned long *flags)
> +						  struct tlb_inv_context *cxt)
>  {
>  	__load_guest_stage2(kvm);
>  	isb();
> @@ -55,7 +84,7 @@ static hyp_alternate_select(__tlb_switch_to_guest,
>  			    ARM64_HAS_VIRT_HOST_EXTN);
>  
>  static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
> -						unsigned long flags)
> +						struct tlb_inv_context *cxt)
>  {
>  	/*
>  	 * We're done with the TLB operation, let's restore the host's
> @@ -64,11 +93,18 @@ static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
>  	write_sysreg(0, vttbr_el2);
>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
>  	isb();
> -	local_irq_restore(flags);
> +
> +	if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
> +		/* Restore the guest's registers to what they were */

host's ?

> +		write_sysreg_el1(cxt->tcr, tcr);
> +		write_sysreg_el1(cxt->sctlr, sctlr);
> +	}
> +
> +	local_irq_restore(cxt->flags);
>  }
>  
>  static void __hyp_text __tlb_switch_to_host_nvhe(struct kvm *kvm,
> -						 unsigned long flags)
> +						 struct tlb_inv_context *cxt)
>  {
>  	write_sysreg(0, vttbr_el2);
>  }
> @@ -80,13 +116,13 @@ static hyp_alternate_select(__tlb_switch_to_host,
>  
>  void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
>  {
> -	unsigned long flags;
> +	struct tlb_inv_context cxt;
>  
>  	dsb(ishst);
>  
>  	/* Switch to requested VMID */
>  	kvm = kern_hyp_va(kvm);
> -	__tlb_switch_to_guest()(kvm, &flags);
> +	__tlb_switch_to_guest()(kvm, &cxt);
>  
>  	/*
>  	 * We could do so much better if we had the VA as well.
> @@ -129,39 +165,39 @@ void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
>  	if (!has_vhe() && icache_is_vpipt())
>  		__flush_icache_all();
>  
> -	__tlb_switch_to_host()(kvm, flags);
> +	__tlb_switch_to_host()(kvm, &cxt);
>  }
>  
>  void __hyp_text __kvm_tlb_flush_vmid(struct kvm *kvm)
>  {
> -	unsigned long flags;
> +	struct tlb_inv_context cxt;
>  
>  	dsb(ishst);
>  
>  	/* Switch to requested VMID */
>  	kvm = kern_hyp_va(kvm);
> -	__tlb_switch_to_guest()(kvm, &flags);
> +	__tlb_switch_to_guest()(kvm, &cxt);
>  
>  	__tlbi(vmalls12e1is);
>  	dsb(ish);
>  	isb();
>  
> -	__tlb_switch_to_host()(kvm, flags);
> +	__tlb_switch_to_host()(kvm, &cxt);
>  }
>  
>  void __hyp_text __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm *kvm = kern_hyp_va(kern_hyp_va(vcpu)->kvm);
> -	unsigned long flags;
> +	struct tlb_inv_context cxt;
>  
>  	/* Switch to requested VMID */
> -	__tlb_switch_to_guest()(kvm, &flags);
> +	__tlb_switch_to_guest()(kvm, &cxt);
>  
>  	__tlbi(vmalle1);
>  	dsb(nsh);
>  	isb();
>  
> -	__tlb_switch_to_host()(kvm, flags);
> +	__tlb_switch_to_host()(kvm, &cxt);
>  }
>  
>  void __hyp_text __kvm_flush_vm_context(void)
> -- 
> 2.19.2
> 

Otherwise:

Acked-by: Christoffer Dall <christoffer.dall@arm.com>

Re: [PATCH v3 7/8] arm64: KVM: Handle ARM erratum 1165522 in TLB invalidation

[Christoffer Dall]

On Thu, Dec 06, 2018 at 05:31:25PM +0000, Marc Zyngier wrote:
> In order to avoid TLB corruption whilst invalidating TLBs on CPUs
> affected by erratum 1165522, we need to prevent S1 page tables
> from being usable.
> 
> For this, we set the EL1 S1 MMU on, and also disable the page table
> walker (by setting the TCR_EL1.EPD* bits to 1).
> 
> This ensures that once we switch to the EL1/EL0 translation regime,
> speculated AT instructions won't be able to parse the page tables.
> 
> Reviewed-by: James Morse <james.morse@arm.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm64/kvm/hyp/tlb.c | 66 +++++++++++++++++++++++++++++++---------
>  1 file changed, 51 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/tlb.c b/arch/arm64/kvm/hyp/tlb.c
> index 7fcc9c1a5f45..ec157543d5a9 100644
> --- a/arch/arm64/kvm/hyp/tlb.c
> +++ b/arch/arm64/kvm/hyp/tlb.c
> @@ -21,12 +21,36 @@
>  #include <asm/kvm_mmu.h>
>  #include <asm/tlbflush.h>
>  
> +struct tlb_inv_context {
> +	unsigned long	flags;
> +	u64		tcr;
> +	u64		sctlr;
> +};
> +
>  static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
> -						 unsigned long *flags)
> +						 struct tlb_inv_context *cxt)
>  {
>  	u64 val;
>  
> -	local_irq_save(*flags);
> +	local_irq_save(cxt->flags);
> +
> +	if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
> +		/*
> +		 * For CPUs that are affected by ARM erratum 1165522, we
> +		 * cannot trust stage-1 to be in a correct state at that
> +		 * point. Since we do not want to force a full load of the
> +		 * vcpu state, we prevent the EL1 page-table walker to
> +		 * allocate new TLBs. This is done by setting the EPD bits
> +		 * in the TCR_EL1 register. We also need to prevent it to
> +		 * allocate IPA->PA walks, so we enable the S1 MMU...
> +		 */
> +		val = cxt->tcr = read_sysreg_el1(tcr);
> +		val |= TCR_EPD1_MASK | TCR_EPD0_MASK;
> +		write_sysreg_el1(val, tcr);
> +		val = cxt->sctlr = read_sysreg_el1(sctlr);
> +		val |= SCTLR_ELx_M;
> +		write_sysreg_el1(val, sctlr);
> +	}
>  
>  	/*
>  	 * With VHE enabled, we have HCR_EL2.{E2H,TGE} = {1,1}, and
> @@ -34,6 +58,11 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
>  	 * guest TLBs (EL1/EL0), we need to change one of these two
>  	 * bits. Changing E2H is impossible (goodbye TTBR1_EL2), so
>  	 * let's flip TGE before executing the TLB operation.
> +	 *
> +	 * ARM erratum 1165522 requires some special handling (again),
> +	 * as we need to make sure both stages of translation are in
> +	 * place before clearing TGE. __load_guest_stage2() already
> +	 * has an ISB in order to deal with this.
>  	 */
>  	__load_guest_stage2(kvm);
>  	val = read_sysreg(hcr_el2);
> @@ -43,7 +72,7 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
>  }
>  
>  static void __hyp_text __tlb_switch_to_guest_nvhe(struct kvm *kvm,
> -						  unsigned long *flags)
> +						  struct tlb_inv_context *cxt)
>  {
>  	__load_guest_stage2(kvm);
>  	isb();
> @@ -55,7 +84,7 @@ static hyp_alternate_select(__tlb_switch_to_guest,
>  			    ARM64_HAS_VIRT_HOST_EXTN);
>  
>  static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
> -						unsigned long flags)
> +						struct tlb_inv_context *cxt)
>  {
>  	/*
>  	 * We're done with the TLB operation, let's restore the host's
> @@ -64,11 +93,18 @@ static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
>  	write_sysreg(0, vttbr_el2);
>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
>  	isb();
> -	local_irq_restore(flags);
> +
> +	if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
> +		/* Restore the guest's registers to what they were */

host's ?

> +		write_sysreg_el1(cxt->tcr, tcr);
> +		write_sysreg_el1(cxt->sctlr, sctlr);
> +	}
> +
> +	local_irq_restore(cxt->flags);
>  }
>  
>  static void __hyp_text __tlb_switch_to_host_nvhe(struct kvm *kvm,
> -						 unsigned long flags)
> +						 struct tlb_inv_context *cxt)
>  {
>  	write_sysreg(0, vttbr_el2);
>  }
> @@ -80,13 +116,13 @@ static hyp_alternate_select(__tlb_switch_to_host,
>  
>  void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
>  {
> -	unsigned long flags;
> +	struct tlb_inv_context cxt;
>  
>  	dsb(ishst);
>  
>  	/* Switch to requested VMID */
>  	kvm = kern_hyp_va(kvm);
> -	__tlb_switch_to_guest()(kvm, &flags);
> +	__tlb_switch_to_guest()(kvm, &cxt);
>  
>  	/*
>  	 * We could do so much better if we had the VA as well.
> @@ -129,39 +165,39 @@ void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
>  	if (!has_vhe() && icache_is_vpipt())
>  		__flush_icache_all();
>  
> -	__tlb_switch_to_host()(kvm, flags);
> +	__tlb_switch_to_host()(kvm, &cxt);
>  }
>  
>  void __hyp_text __kvm_tlb_flush_vmid(struct kvm *kvm)
>  {
> -	unsigned long flags;
> +	struct tlb_inv_context cxt;
>  
>  	dsb(ishst);
>  
>  	/* Switch to requested VMID */
>  	kvm = kern_hyp_va(kvm);
> -	__tlb_switch_to_guest()(kvm, &flags);
> +	__tlb_switch_to_guest()(kvm, &cxt);
>  
>  	__tlbi(vmalls12e1is);
>  	dsb(ish);
>  	isb();
>  
> -	__tlb_switch_to_host()(kvm, flags);
> +	__tlb_switch_to_host()(kvm, &cxt);
>  }
>  
>  void __hyp_text __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm *kvm = kern_hyp_va(kern_hyp_va(vcpu)->kvm);
> -	unsigned long flags;
> +	struct tlb_inv_context cxt;
>  
>  	/* Switch to requested VMID */
> -	__tlb_switch_to_guest()(kvm, &flags);
> +	__tlb_switch_to_guest()(kvm, &cxt);
>  
>  	__tlbi(vmalle1);
>  	dsb(nsh);
>  	isb();
>  
> -	__tlb_switch_to_host()(kvm, flags);
> +	__tlb_switch_to_host()(kvm, &cxt);
>  }
>  
>  void __hyp_text __kvm_flush_vm_context(void)
> -- 
> 2.19.2
> 

Otherwise:

Acked-by: Christoffer Dall <christoffer.dall@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/8] arm64: KVM: Make VHE Stage-2 TLB invalidation operations non-interruptible

[Marc Zyngier]

Hi Christoffer,

On 10/12/2018 10:03, Christoffer Dall wrote:
> On Thu, Dec 06, 2018 at 05:31:19PM +0000, Marc Zyngier wrote:
>> Contrary to the non-VHE version of the TLB invalidation helpers, the VHE
>> code  has interrupts enabled, meaning that we can take an interrupt in
>> the middle of such a sequence, and start running something else with
>> HCR_EL2.TGE cleared.
> 
> Do we have to clear TGE to perform the TLB invalidation, or is that just
> a side-effect of re-using code?

We really do need to clear TGE. From the description of TLBI VMALLE1IS:

<quote>
When EL2 is implemented and enabled in the current Security state:
— If HCR_EL2.{E2H, TGE} is not {1, 1}, the entry would be used with the
current VMID and would be required to translate the specified VA using
the EL1&0 translation regime.
— If HCR_EL2.{E2H, TGE} is {1, 1}, the entry would be required to
translate the specified VA using the EL2&0 translation regime.
</quote>

> Also, do we generally perform TLB invalidations in the kernel with
> interrupts disabled, or is this just a result of clearing TGE?

That's definitely a result of clearing TGE. We could be taking an
interrupt here, and execute a user access on the back of it (perf will
happily walk a user-space stack in that context, for example). Having
TGE clear in that context. An alternative solution would be to
save/restore TGE on interrupt entry/exit, but that's a bit overkill when
you consider how rarely we issue such TLB invalidation.

> Somehow I feel like this should look more like just another TLB
> invalidation in the kernel, but if there's a good reason why it can't
> then this is fine.

The rest of the TLB invalidation in the kernel doesn't need to
save/restore any context. They apply to a set of parameters that are
already loaded on the CPU. What we have here is substantially different.

Thanks,

	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

Re: [PATCH v3 1/8] arm64: KVM: Make VHE Stage-2 TLB invalidation operations non-interruptible

[Marc Zyngier]

Hi Christoffer,

On 10/12/2018 10:03, Christoffer Dall wrote:
> On Thu, Dec 06, 2018 at 05:31:19PM +0000, Marc Zyngier wrote:
>> Contrary to the non-VHE version of the TLB invalidation helpers, the VHE
>> code  has interrupts enabled, meaning that we can take an interrupt in
>> the middle of such a sequence, and start running something else with
>> HCR_EL2.TGE cleared.
> 
> Do we have to clear TGE to perform the TLB invalidation, or is that just
> a side-effect of re-using code?

We really do need to clear TGE. From the description of TLBI VMALLE1IS:

<quote>
When EL2 is implemented and enabled in the current Security state:
— If HCR_EL2.{E2H, TGE} is not {1, 1}, the entry would be used with the
current VMID and would be required to translate the specified VA using
the EL1&0 translation regime.
— If HCR_EL2.{E2H, TGE} is {1, 1}, the entry would be required to
translate the specified VA using the EL2&0 translation regime.
</quote>

> Also, do we generally perform TLB invalidations in the kernel with
> interrupts disabled, or is this just a result of clearing TGE?

That's definitely a result of clearing TGE. We could be taking an
interrupt here, and execute a user access on the back of it (perf will
happily walk a user-space stack in that context, for example). Having
TGE clear in that context. An alternative solution would be to
save/restore TGE on interrupt entry/exit, but that's a bit overkill when
you consider how rarely we issue such TLB invalidation.

> Somehow I feel like this should look more like just another TLB
> invalidation in the kernel, but if there's a good reason why it can't
> then this is fine.

The rest of the TLB invalidation in the kernel doesn't need to
save/restore any context. They apply to a set of parameters that are
already loaded on the CPU. What we have here is substantially different.

Thanks,

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 2/8] KVM: arm64: Rework detection of SVE, !VHE systems

[Marc Zyngier]

On 10/12/2018 10:13, Christoffer Dall wrote:
> On Thu, Dec 06, 2018 at 05:31:20PM +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 | 6 +++---
>>  virt/kvm/arm/arm.c                | 8 ++++----
>>  3 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>> index 5ca5d9af0c26..2184d9ddb418 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_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..d6d9aa76a943 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -422,7 +422,7 @@ 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_requires_vhe(void)
>>  {
>>  	/*
>>  	 * The Arm architecture specifies that implementation of SVE
>> @@ -430,9 +430,9 @@ static inline bool kvm_arch_check_sve_has_vhe(void)
>>  	 * relies on this when SVE is present:
>>  	 */
>>  	if (system_supports_sve())
>> -		return has_vhe();
>> -	else
>>  		return true;
>> +
>> +	return false;
>>  }
>>  
>>  static inline void kvm_arch_hardware_unsetup(void) {}
>> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
>> index 23774970c9df..1db4c15edcdd 100644
>> --- a/virt/kvm/arm/arm.c
>> +++ b/virt/kvm/arm/arm.c
>> @@ -1640,8 +1640,10 @@ 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?");
>> +	in_hyp_mode = is_kernel_in_hyp_mode();
>> +
>> +	if (!in_hyp_mode && kvm_arch_requires_vhe()) {
>> +		kvm_pr_unimpl("CPU requiring VHE was booted in non-VHE mode");
> 
> nit: The error message feels weird to me (are we reporting CPU bugs?)
> and I'm not sure about the unimpl and I think there's a linse space
> missing.
> 
> How about:
> 
> 	kvm_err("Cannot support this CPU in non-VHE mode, not initializing\n");

Yup, works for me. Will, do you mind changing this message for me?

> 
> If we wanted to be super helpful, we could expand
> kvm_arch_requires_vhe() with a print statement saying:
> 		
> 	kvm_err("SVE detected, requiring VHE mode\n");
> 
> But thay may be overkill.

I started with that, but having kvm_pr*() in an include file has proved
to be challenging, so I decided to spend my time on something more
useful and quickly gave up... :-/

> 
> 
>>  		return -ENODEV;
>>  	}
>>  
>> @@ -1657,8 +1659,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.2
>>
> 
> Otherwise:
> 
> Acked-by: Christoffer Dall <christoffer.dall@arm.com>
> 

Thanks,

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

Re: [PATCH v3 2/8] KVM: arm64: Rework detection of SVE, !VHE systems

[Marc Zyngier]

On 10/12/2018 10:13, Christoffer Dall wrote:
> On Thu, Dec 06, 2018 at 05:31:20PM +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 | 6 +++---
>>  virt/kvm/arm/arm.c                | 8 ++++----
>>  3 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
>> index 5ca5d9af0c26..2184d9ddb418 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_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..d6d9aa76a943 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -422,7 +422,7 @@ 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_requires_vhe(void)
>>  {
>>  	/*
>>  	 * The Arm architecture specifies that implementation of SVE
>> @@ -430,9 +430,9 @@ static inline bool kvm_arch_check_sve_has_vhe(void)
>>  	 * relies on this when SVE is present:
>>  	 */
>>  	if (system_supports_sve())
>> -		return has_vhe();
>> -	else
>>  		return true;
>> +
>> +	return false;
>>  }
>>  
>>  static inline void kvm_arch_hardware_unsetup(void) {}
>> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
>> index 23774970c9df..1db4c15edcdd 100644
>> --- a/virt/kvm/arm/arm.c
>> +++ b/virt/kvm/arm/arm.c
>> @@ -1640,8 +1640,10 @@ 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?");
>> +	in_hyp_mode = is_kernel_in_hyp_mode();
>> +
>> +	if (!in_hyp_mode && kvm_arch_requires_vhe()) {
>> +		kvm_pr_unimpl("CPU requiring VHE was booted in non-VHE mode");
> 
> nit: The error message feels weird to me (are we reporting CPU bugs?)
> and I'm not sure about the unimpl and I think there's a linse space
> missing.
> 
> How about:
> 
> 	kvm_err("Cannot support this CPU in non-VHE mode, not initializing\n");

Yup, works for me. Will, do you mind changing this message for me?

> 
> If we wanted to be super helpful, we could expand
> kvm_arch_requires_vhe() with a print statement saying:
> 		
> 	kvm_err("SVE detected, requiring VHE mode\n");
> 
> But thay may be overkill.

I started with that, but having kvm_pr*() in an include file has proved
to be challenging, so I decided to spend my time on something more
useful and quickly gave up... :-/

> 
> 
>>  		return -ENODEV;
>>  	}
>>  
>> @@ -1657,8 +1659,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.2
>>
> 
> Otherwise:
> 
> Acked-by: Christoffer Dall <christoffer.dall@arm.com>
> 

Thanks,

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 7/8] arm64: KVM: Handle ARM erratum 1165522 in TLB invalidation

[Marc Zyngier]

On 10/12/2018 10:19, Christoffer Dall wrote:
> On Thu, Dec 06, 2018 at 05:31:25PM +0000, Marc Zyngier wrote:
>> In order to avoid TLB corruption whilst invalidating TLBs on CPUs
>> affected by erratum 1165522, we need to prevent S1 page tables
>> from being usable.
>>
>> For this, we set the EL1 S1 MMU on, and also disable the page table
>> walker (by setting the TCR_EL1.EPD* bits to 1).
>>
>> This ensures that once we switch to the EL1/EL0 translation regime,
>> speculated AT instructions won't be able to parse the page tables.
>>
>> Reviewed-by: James Morse <james.morse@arm.com>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  arch/arm64/kvm/hyp/tlb.c | 66 +++++++++++++++++++++++++++++++---------
>>  1 file changed, 51 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/hyp/tlb.c b/arch/arm64/kvm/hyp/tlb.c
>> index 7fcc9c1a5f45..ec157543d5a9 100644
>> --- a/arch/arm64/kvm/hyp/tlb.c
>> +++ b/arch/arm64/kvm/hyp/tlb.c
>> @@ -21,12 +21,36 @@
>>  #include <asm/kvm_mmu.h>
>>  #include <asm/tlbflush.h>
>>  
>> +struct tlb_inv_context {
>> +	unsigned long	flags;
>> +	u64		tcr;
>> +	u64		sctlr;
>> +};
>> +
>>  static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
>> -						 unsigned long *flags)
>> +						 struct tlb_inv_context *cxt)
>>  {
>>  	u64 val;
>>  
>> -	local_irq_save(*flags);
>> +	local_irq_save(cxt->flags);
>> +
>> +	if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
>> +		/*
>> +		 * For CPUs that are affected by ARM erratum 1165522, we
>> +		 * cannot trust stage-1 to be in a correct state at that
>> +		 * point. Since we do not want to force a full load of the
>> +		 * vcpu state, we prevent the EL1 page-table walker to
>> +		 * allocate new TLBs. This is done by setting the EPD bits
>> +		 * in the TCR_EL1 register. We also need to prevent it to
>> +		 * allocate IPA->PA walks, so we enable the S1 MMU...
>> +		 */
>> +		val = cxt->tcr = read_sysreg_el1(tcr);
>> +		val |= TCR_EPD1_MASK | TCR_EPD0_MASK;
>> +		write_sysreg_el1(val, tcr);
>> +		val = cxt->sctlr = read_sysreg_el1(sctlr);
>> +		val |= SCTLR_ELx_M;
>> +		write_sysreg_el1(val, sctlr);
>> +	}
>>  
>>  	/*
>>  	 * With VHE enabled, we have HCR_EL2.{E2H,TGE} = {1,1}, and
>> @@ -34,6 +58,11 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
>>  	 * guest TLBs (EL1/EL0), we need to change one of these two
>>  	 * bits. Changing E2H is impossible (goodbye TTBR1_EL2), so
>>  	 * let's flip TGE before executing the TLB operation.
>> +	 *
>> +	 * ARM erratum 1165522 requires some special handling (again),
>> +	 * as we need to make sure both stages of translation are in
>> +	 * place before clearing TGE. __load_guest_stage2() already
>> +	 * has an ISB in order to deal with this.
>>  	 */
>>  	__load_guest_stage2(kvm);
>>  	val = read_sysreg(hcr_el2);
>> @@ -43,7 +72,7 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
>>  }
>>  
>>  static void __hyp_text __tlb_switch_to_guest_nvhe(struct kvm *kvm,
>> -						  unsigned long *flags)
>> +						  struct tlb_inv_context *cxt)
>>  {
>>  	__load_guest_stage2(kvm);
>>  	isb();
>> @@ -55,7 +84,7 @@ static hyp_alternate_select(__tlb_switch_to_guest,
>>  			    ARM64_HAS_VIRT_HOST_EXTN);
>>  
>>  static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
>> -						unsigned long flags)
>> +						struct tlb_inv_context *cxt)
>>  {
>>  	/*
>>  	 * We're done with the TLB operation, let's restore the host's
>> @@ -64,11 +93,18 @@ static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
>>  	write_sysreg(0, vttbr_el2);
>>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
>>  	isb();
>> -	local_irq_restore(flags);
>> +
>> +	if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
>> +		/* Restore the guest's registers to what they were */
> 
> host's ?

Hum... Yes, silly thinko.

[...]

> 
> Otherwise:
> 
> Acked-by: Christoffer Dall <christoffer.dall@arm.com>
> 

Thanks,

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

Re: [PATCH v3 7/8] arm64: KVM: Handle ARM erratum 1165522 in TLB invalidation

[Marc Zyngier]

On 10/12/2018 10:19, Christoffer Dall wrote:
> On Thu, Dec 06, 2018 at 05:31:25PM +0000, Marc Zyngier wrote:
>> In order to avoid TLB corruption whilst invalidating TLBs on CPUs
>> affected by erratum 1165522, we need to prevent S1 page tables
>> from being usable.
>>
>> For this, we set the EL1 S1 MMU on, and also disable the page table
>> walker (by setting the TCR_EL1.EPD* bits to 1).
>>
>> This ensures that once we switch to the EL1/EL0 translation regime,
>> speculated AT instructions won't be able to parse the page tables.
>>
>> Reviewed-by: James Morse <james.morse@arm.com>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  arch/arm64/kvm/hyp/tlb.c | 66 +++++++++++++++++++++++++++++++---------
>>  1 file changed, 51 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/hyp/tlb.c b/arch/arm64/kvm/hyp/tlb.c
>> index 7fcc9c1a5f45..ec157543d5a9 100644
>> --- a/arch/arm64/kvm/hyp/tlb.c
>> +++ b/arch/arm64/kvm/hyp/tlb.c
>> @@ -21,12 +21,36 @@
>>  #include <asm/kvm_mmu.h>
>>  #include <asm/tlbflush.h>
>>  
>> +struct tlb_inv_context {
>> +	unsigned long	flags;
>> +	u64		tcr;
>> +	u64		sctlr;
>> +};
>> +
>>  static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
>> -						 unsigned long *flags)
>> +						 struct tlb_inv_context *cxt)
>>  {
>>  	u64 val;
>>  
>> -	local_irq_save(*flags);
>> +	local_irq_save(cxt->flags);
>> +
>> +	if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
>> +		/*
>> +		 * For CPUs that are affected by ARM erratum 1165522, we
>> +		 * cannot trust stage-1 to be in a correct state at that
>> +		 * point. Since we do not want to force a full load of the
>> +		 * vcpu state, we prevent the EL1 page-table walker to
>> +		 * allocate new TLBs. This is done by setting the EPD bits
>> +		 * in the TCR_EL1 register. We also need to prevent it to
>> +		 * allocate IPA->PA walks, so we enable the S1 MMU...
>> +		 */
>> +		val = cxt->tcr = read_sysreg_el1(tcr);
>> +		val |= TCR_EPD1_MASK | TCR_EPD0_MASK;
>> +		write_sysreg_el1(val, tcr);
>> +		val = cxt->sctlr = read_sysreg_el1(sctlr);
>> +		val |= SCTLR_ELx_M;
>> +		write_sysreg_el1(val, sctlr);
>> +	}
>>  
>>  	/*
>>  	 * With VHE enabled, we have HCR_EL2.{E2H,TGE} = {1,1}, and
>> @@ -34,6 +58,11 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
>>  	 * guest TLBs (EL1/EL0), we need to change one of these two
>>  	 * bits. Changing E2H is impossible (goodbye TTBR1_EL2), so
>>  	 * let's flip TGE before executing the TLB operation.
>> +	 *
>> +	 * ARM erratum 1165522 requires some special handling (again),
>> +	 * as we need to make sure both stages of translation are in
>> +	 * place before clearing TGE. __load_guest_stage2() already
>> +	 * has an ISB in order to deal with this.
>>  	 */
>>  	__load_guest_stage2(kvm);
>>  	val = read_sysreg(hcr_el2);
>> @@ -43,7 +72,7 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
>>  }
>>  
>>  static void __hyp_text __tlb_switch_to_guest_nvhe(struct kvm *kvm,
>> -						  unsigned long *flags)
>> +						  struct tlb_inv_context *cxt)
>>  {
>>  	__load_guest_stage2(kvm);
>>  	isb();
>> @@ -55,7 +84,7 @@ static hyp_alternate_select(__tlb_switch_to_guest,
>>  			    ARM64_HAS_VIRT_HOST_EXTN);
>>  
>>  static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
>> -						unsigned long flags)
>> +						struct tlb_inv_context *cxt)
>>  {
>>  	/*
>>  	 * We're done with the TLB operation, let's restore the host's
>> @@ -64,11 +93,18 @@ static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
>>  	write_sysreg(0, vttbr_el2);
>>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
>>  	isb();
>> -	local_irq_restore(flags);
>> +
>> +	if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
>> +		/* Restore the guest's registers to what they were */
> 
> host's ?

Hum... Yes, silly thinko.

[...]

> 
> Otherwise:
> 
> Acked-by: Christoffer Dall <christoffer.dall@arm.com>
> 

Thanks,

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/8] arm64: KVM: Make VHE Stage-2 TLB invalidation operations non-interruptible

[Christoffer Dall]

On Mon, Dec 10, 2018 at 10:24:31AM +0000, Marc Zyngier wrote:
> Hi Christoffer,
> 
> On 10/12/2018 10:03, Christoffer Dall wrote:
> > On Thu, Dec 06, 2018 at 05:31:19PM +0000, Marc Zyngier wrote:
> >> Contrary to the non-VHE version of the TLB invalidation helpers, the VHE
> >> code  has interrupts enabled, meaning that we can take an interrupt in
> >> the middle of such a sequence, and start running something else with
> >> HCR_EL2.TGE cleared.
> > 
> > Do we have to clear TGE to perform the TLB invalidation, or is that just
> > a side-effect of re-using code?
> 
> We really do need to clear TGE. From the description of TLBI VMALLE1IS:
> 
> <quote>
> When EL2 is implemented and enabled in the current Security state:
> — If HCR_EL2.{E2H, TGE} is not {1, 1}, the entry would be used with the
> current VMID and would be required to translate the specified VA using
> the EL1&0 translation regime.
> — If HCR_EL2.{E2H, TGE} is {1, 1}, the entry would be required to
> translate the specified VA using the EL2&0 translation regime.
> </quote>
> 
> > Also, do we generally perform TLB invalidations in the kernel with
> > interrupts disabled, or is this just a result of clearing TGE?
> 
> That's definitely a result of clearing TGE. We could be taking an
> interrupt here, and execute a user access on the back of it (perf will
> happily walk a user-space stack in that context, for example). Having
> TGE clear in that context. An alternative solution would be to
> save/restore TGE on interrupt entry/exit, but that's a bit overkill when
> you consider how rarely we issue such TLB invalidation.
> 
> > Somehow I feel like this should look more like just another TLB
> > invalidation in the kernel, but if there's a good reason why it can't
> > then this is fine.
> 
> The rest of the TLB invalidation in the kernel doesn't need to
> save/restore any context. They apply to a set of parameters that are
> already loaded on the CPU. What we have here is substantially different.
> 

Thanks for the explanation and Arm ARM quote.  I failed to find that on
my own this particular Monday morning.

Acked-by: Christoffer Dall <christoffer.dall@arm.com>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH v3 1/8] arm64: KVM: Make VHE Stage-2 TLB invalidation operations non-interruptible

[Christoffer Dall]

On Mon, Dec 10, 2018 at 10:24:31AM +0000, Marc Zyngier wrote:
> Hi Christoffer,
> 
> On 10/12/2018 10:03, Christoffer Dall wrote:
> > On Thu, Dec 06, 2018 at 05:31:19PM +0000, Marc Zyngier wrote:
> >> Contrary to the non-VHE version of the TLB invalidation helpers, the VHE
> >> code  has interrupts enabled, meaning that we can take an interrupt in
> >> the middle of such a sequence, and start running something else with
> >> HCR_EL2.TGE cleared.
> > 
> > Do we have to clear TGE to perform the TLB invalidation, or is that just
> > a side-effect of re-using code?
> 
> We really do need to clear TGE. From the description of TLBI VMALLE1IS:
> 
> <quote>
> When EL2 is implemented and enabled in the current Security state:
> — If HCR_EL2.{E2H, TGE} is not {1, 1}, the entry would be used with the
> current VMID and would be required to translate the specified VA using
> the EL1&0 translation regime.
> — If HCR_EL2.{E2H, TGE} is {1, 1}, the entry would be required to
> translate the specified VA using the EL2&0 translation regime.
> </quote>
> 
> > Also, do we generally perform TLB invalidations in the kernel with
> > interrupts disabled, or is this just a result of clearing TGE?
> 
> That's definitely a result of clearing TGE. We could be taking an
> interrupt here, and execute a user access on the back of it (perf will
> happily walk a user-space stack in that context, for example). Having
> TGE clear in that context. An alternative solution would be to
> save/restore TGE on interrupt entry/exit, but that's a bit overkill when
> you consider how rarely we issue such TLB invalidation.
> 
> > Somehow I feel like this should look more like just another TLB
> > invalidation in the kernel, but if there's a good reason why it can't
> > then this is fine.
> 
> The rest of the TLB invalidation in the kernel doesn't need to
> save/restore any context. They apply to a set of parameters that are
> already loaded on the CPU. What we have here is substantially different.
> 

Thanks for the explanation and Arm ARM quote.  I failed to find that on
my own this particular Monday morning.

Acked-by: Christoffer Dall <christoffer.dall@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 7/8] arm64: KVM: Handle ARM erratum 1165522 in TLB invalidation

[James Morse]

Hi Marc, Christoffer,

On 10/12/2018 10:46, Marc Zyngier wrote:
> On 10/12/2018 10:19, Christoffer Dall wrote:
>> On Thu, Dec 06, 2018 at 05:31:25PM +0000, Marc Zyngier wrote:
>>> In order to avoid TLB corruption whilst invalidating TLBs on CPUs
>>> affected by erratum 1165522, we need to prevent S1 page tables
>>> from being usable.
>>>
>>> For this, we set the EL1 S1 MMU on, and also disable the page table
>>> walker (by setting the TCR_EL1.EPD* bits to 1).
>>>
>>> This ensures that once we switch to the EL1/EL0 translation regime,
>>> speculated AT instructions won't be able to parse the page tables.

>>> @@ -64,11 +93,18 @@ static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
>>>  	write_sysreg(0, vttbr_el2);
>>>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
>>>  	isb();
>>> -	local_irq_restore(flags);
>>> +
>>> +	if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
>>> +		/* Restore the guest's registers to what they were */
>>
>> host's ?
> 
> Hum... Yes, silly thinko.

I thought these were the guests registers because they are EL1 registers and
this is a VHE-only path.
'interrupted guest' was how I read this. This stuff can get called if memory is
allocated for guest-A while a vcpu is loaded, and reclaims memory from guest-B
causing an mmu-notifier call for stage2. This is why we have to put guest-A's
registers back as we weren't pre-empted, and we expect EL1 to be untouched.

I agree they could belong to no-guest if a vcpu isn't loaded at all... is host
the term used here?


Thanks,

James

Re: [PATCH v3 7/8] arm64: KVM: Handle ARM erratum 1165522 in TLB invalidation

[James Morse]

Hi Marc, Christoffer,

On 10/12/2018 10:46, Marc Zyngier wrote:
> On 10/12/2018 10:19, Christoffer Dall wrote:
>> On Thu, Dec 06, 2018 at 05:31:25PM +0000, Marc Zyngier wrote:
>>> In order to avoid TLB corruption whilst invalidating TLBs on CPUs
>>> affected by erratum 1165522, we need to prevent S1 page tables
>>> from being usable.
>>>
>>> For this, we set the EL1 S1 MMU on, and also disable the page table
>>> walker (by setting the TCR_EL1.EPD* bits to 1).
>>>
>>> This ensures that once we switch to the EL1/EL0 translation regime,
>>> speculated AT instructions won't be able to parse the page tables.

>>> @@ -64,11 +93,18 @@ static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
>>>  	write_sysreg(0, vttbr_el2);
>>>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
>>>  	isb();
>>> -	local_irq_restore(flags);
>>> +
>>> +	if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
>>> +		/* Restore the guest's registers to what they were */
>>
>> host's ?
> 
> Hum... Yes, silly thinko.

I thought these were the guests registers because they are EL1 registers and
this is a VHE-only path.
'interrupted guest' was how I read this. This stuff can get called if memory is
allocated for guest-A while a vcpu is loaded, and reclaims memory from guest-B
causing an mmu-notifier call for stage2. This is why we have to put guest-A's
registers back as we weren't pre-empted, and we expect EL1 to be untouched.

I agree they could belong to no-guest if a vcpu isn't loaded at all... is host
the term used here?


Thanks,

James

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 7/8] arm64: KVM: Handle ARM erratum 1165522 in TLB invalidation

[Christoffer Dall]

On Mon, Dec 10, 2018 at 11:15:00AM +0000, James Morse wrote:
> Hi Marc, Christoffer,
> 
> On 10/12/2018 10:46, Marc Zyngier wrote:
> > On 10/12/2018 10:19, Christoffer Dall wrote:
> >> On Thu, Dec 06, 2018 at 05:31:25PM +0000, Marc Zyngier wrote:
> >>> In order to avoid TLB corruption whilst invalidating TLBs on CPUs
> >>> affected by erratum 1165522, we need to prevent S1 page tables
> >>> from being usable.
> >>>
> >>> For this, we set the EL1 S1 MMU on, and also disable the page table
> >>> walker (by setting the TCR_EL1.EPD* bits to 1).
> >>>
> >>> This ensures that once we switch to the EL1/EL0 translation regime,
> >>> speculated AT instructions won't be able to parse the page tables.
> 
> >>> @@ -64,11 +93,18 @@ static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
> >>>  	write_sysreg(0, vttbr_el2);
> >>>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
> >>>  	isb();
> >>> -	local_irq_restore(flags);
> >>> +
> >>> +	if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
> >>> +		/* Restore the guest's registers to what they were */
> >>
> >> host's ?
> > 
> > Hum... Yes, silly thinko.
> 
> I thought these were the guests registers because they are EL1 registers and
> this is a VHE-only path.
> 'interrupted guest' was how I read this. This stuff can get called if memory is
> allocated for guest-A while a vcpu is loaded, and reclaims memory from guest-B
> causing an mmu-notifier call for stage2. This is why we have to put guest-A's
> registers back as we weren't pre-empted, and we expect EL1 to be untouched.
> 
> I agree they could belong to no-guest if a vcpu isn't loaded at all... is host
> the term used here?
> 

Ah, you're right.  Host is not the right term either.

I haven't done the call path analysis, so not sure about all the
possible contexts where all this can be called, but if it's really truly
only in guest context, then we don't need to save the values to a
temporary struct at all, but can save them on the vcpu.

We can also just side-step the whole thing and just say "Restore the
registers to what they were".


Thanks,

    Christoffer

Re: [PATCH v3 7/8] arm64: KVM: Handle ARM erratum 1165522 in TLB invalidation

[Christoffer Dall]

On Mon, Dec 10, 2018 at 11:15:00AM +0000, James Morse wrote:
> Hi Marc, Christoffer,
> 
> On 10/12/2018 10:46, Marc Zyngier wrote:
> > On 10/12/2018 10:19, Christoffer Dall wrote:
> >> On Thu, Dec 06, 2018 at 05:31:25PM +0000, Marc Zyngier wrote:
> >>> In order to avoid TLB corruption whilst invalidating TLBs on CPUs
> >>> affected by erratum 1165522, we need to prevent S1 page tables
> >>> from being usable.
> >>>
> >>> For this, we set the EL1 S1 MMU on, and also disable the page table
> >>> walker (by setting the TCR_EL1.EPD* bits to 1).
> >>>
> >>> This ensures that once we switch to the EL1/EL0 translation regime,
> >>> speculated AT instructions won't be able to parse the page tables.
> 
> >>> @@ -64,11 +93,18 @@ static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
> >>>  	write_sysreg(0, vttbr_el2);
> >>>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
> >>>  	isb();
> >>> -	local_irq_restore(flags);
> >>> +
> >>> +	if (cpus_have_const_cap(ARM64_WORKAROUND_1165522)) {
> >>> +		/* Restore the guest's registers to what they were */
> >>
> >> host's ?
> > 
> > Hum... Yes, silly thinko.
> 
> I thought these were the guests registers because they are EL1 registers and
> this is a VHE-only path.
> 'interrupted guest' was how I read this. This stuff can get called if memory is
> allocated for guest-A while a vcpu is loaded, and reclaims memory from guest-B
> causing an mmu-notifier call for stage2. This is why we have to put guest-A's
> registers back as we weren't pre-empted, and we expect EL1 to be untouched.
> 
> I agree they could belong to no-guest if a vcpu isn't loaded at all... is host
> the term used here?
> 

Ah, you're right.  Host is not the right term either.

I haven't done the call path analysis, so not sure about all the
possible contexts where all this can be called, but if it's really truly
only in guest context, then we don't need to save the values to a
temporary struct at all, but can save them on the vcpu.

We can also just side-step the whole thing and just say "Restore the
registers to what they were".


Thanks,

    Christoffer

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 2/8] KVM: arm64: Rework detection of SVE, !VHE systems

[Will Deacon]

On Mon, Dec 10, 2018 at 10:28:05AM +0000, Marc Zyngier wrote:
> On 10/12/2018 10:13, Christoffer Dall wrote:
> > On Thu, Dec 06, 2018 at 05:31:20PM +0000, Marc Zyngier wrote:
> >> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> >> index 23774970c9df..1db4c15edcdd 100644
> >> --- a/virt/kvm/arm/arm.c
> >> +++ b/virt/kvm/arm/arm.c
> >> @@ -1640,8 +1640,10 @@ 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?");
> >> +	in_hyp_mode = is_kernel_in_hyp_mode();
> >> +
> >> +	if (!in_hyp_mode && kvm_arch_requires_vhe()) {
> >> +		kvm_pr_unimpl("CPU requiring VHE was booted in non-VHE mode");
> > 
> > nit: The error message feels weird to me (are we reporting CPU bugs?)
> > and I'm not sure about the unimpl and I think there's a linse space
> > missing.
> > 
> > How about:
> > 
> > 	kvm_err("Cannot support this CPU in non-VHE mode, not initializing\n");
> 
> Yup, works for me. Will, do you mind changing this message for me?

I pushed out an updated branch here:

https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=kvm/cortex-a76-erratum-1165522

which should address all of Christoffer's comments and add his tags.

I plan to merge that into for-next/core later today.

Will

Re: [PATCH v3 2/8] KVM: arm64: Rework detection of SVE, !VHE systems

[Will Deacon]

On Mon, Dec 10, 2018 at 10:28:05AM +0000, Marc Zyngier wrote:
> On 10/12/2018 10:13, Christoffer Dall wrote:
> > On Thu, Dec 06, 2018 at 05:31:20PM +0000, Marc Zyngier wrote:
> >> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> >> index 23774970c9df..1db4c15edcdd 100644
> >> --- a/virt/kvm/arm/arm.c
> >> +++ b/virt/kvm/arm/arm.c
> >> @@ -1640,8 +1640,10 @@ 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?");
> >> +	in_hyp_mode = is_kernel_in_hyp_mode();
> >> +
> >> +	if (!in_hyp_mode && kvm_arch_requires_vhe()) {
> >> +		kvm_pr_unimpl("CPU requiring VHE was booted in non-VHE mode");
> > 
> > nit: The error message feels weird to me (are we reporting CPU bugs?)
> > and I'm not sure about the unimpl and I think there's a linse space
> > missing.
> > 
> > How about:
> > 
> > 	kvm_err("Cannot support this CPU in non-VHE mode, not initializing\n");
> 
> Yup, works for me. Will, do you mind changing this message for me?

I pushed out an updated branch here:

https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=kvm/cortex-a76-erratum-1165522

which should address all of Christoffer's comments and add his tags.

I plan to merge that into for-next/core later today.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel