KVM: arm64: Enable SVE support on nVHE systems

---
 arch/arm64/include/asm/fpsimdmacros.h   |  2 ++
 arch/arm64/include/asm/kvm_hyp.h        |  2 ++
 arch/arm64/kvm/hyp/fpsimd.S             | 10 ++++++++++
 arch/arm64/kvm/hyp/include/hyp/switch.h | 10 +++++-----
 4 files changed, 19 insertions(+), 5 deletions(-)

arch/arm64/include/asm/fpsimdmacros.h +2 -0

index af43367534c7..e9b72d35b867 100644
@@ -6,6 +6,8 @@
  * Author: Catalin Marinas <[email protected]>
  */
 
+#include <asm/assembler.h>
+
 .macro fpsimd_save state, tmpnr
 	stp	q0, q1, [\state, #16 * 0]
 	stp	q2, q3, [\state, #16 * 2]

arch/arm64/include/asm/kvm_hyp.h +2 -0

index c0450828378b..e8b0f7fcd86b 100644
@@ -85,6 +85,8 @@ void __debug_switch_to_host(struct kvm_vcpu *vcpu);
 
 void __fpsimd_save_state(struct user_fpsimd_state *fp_regs);
 void __fpsimd_restore_state(struct user_fpsimd_state *fp_regs);
+void __sve_save_state(void *sve_pffr, u32 *fpsr);
+void __sve_restore_state(void *sve_pffr, u32 *fpsr, unsigned int vqminus1);
 
 #ifndef __KVM_NVHE_HYPERVISOR__
 void activate_traps_vhe_load(struct kvm_vcpu *vcpu);

arch/arm64/kvm/hyp/fpsimd.S +10 -0

index 01f114aa47b0..e4010d1acb79 100644
@@ -19,3 +19,13 @@ SYM_FUNC_START(__fpsimd_restore_state)
 	fpsimd_restore	x0, 1
 	ret
 SYM_FUNC_END(__fpsimd_restore_state)
+
+SYM_FUNC_START(__sve_restore_state)
+	sve_load 0, x1, x2, 3, x4
+	ret
+SYM_FUNC_END(__sve_restore_state)
+
+SYM_FUNC_START(__sve_save_state)
+	sve_save 0, x1, 2
+	ret
+SYM_FUNC_END(__sve_restore_state)

arch/arm64/kvm/hyp/include/hyp/switch.h +5 -5

index 54f4860cd87c..807bc4734828 100644
@@ -256,8 +256,8 @@ static inline bool __hyp_handle_fpsimd(struct kvm_vcpu *vcpu)
 				vcpu->arch.host_fpsimd_state,
 				struct thread_struct, uw.fpsimd_state);
 
-			sve_save_state(sve_pffr(thread),
-				       &vcpu->arch.host_fpsimd_state->fpsr);
+			__sve_save_state(sve_pffr(thread),
+					 &vcpu->arch.host_fpsimd_state->fpsr);
 		} else {
 			__fpsimd_save_state(vcpu->arch.host_fpsimd_state);
 		}

@@ -266,9 +266,9 @@ static inline bool __hyp_handle_fpsimd(struct kvm_vcpu *vcpu)
 	}
 
 	if (sve_guest) {
-		sve_load_state(vcpu_sve_pffr(vcpu),
-			       &vcpu->arch.ctxt.fp_regs.fpsr,
-			       sve_vq_from_vl(vcpu->arch.sve_max_vl) - 1);
+		__sve_restore_state(vcpu_sve_pffr(vcpu),
+				    &vcpu->arch.ctxt.fp_regs.fpsr,
+				    sve_vq_from_vl(vcpu->arch.sve_max_vl) - 1);
 		write_sysreg_s(__vcpu_sys_reg(vcpu, ZCR_EL1), SYS_ZCR_EL12);
 	} else {
 		__fpsimd_restore_state(&vcpu->arch.ctxt.fp_regs);

Signature · 6 lines

2.29.2

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Quentin Perret

2021-03-16 10:31 UTC #2 lore raw

Re: [PATCH 01/10] KVM: arm64: Provide KVM's own save/restore SVE primitives

arch/arm64/kvm/hyp/fpsimd.S SYM_FUNC_START(__fpsimd_restore_state) · line 26

Quoted 13 lines · Marc Zyngier

arch/arm64/kvm/hyp/fpsimd.S +5 -0

index 01f114aa47b0..e4010d1acb79 100644
@@ -19,3 +19,13 @@ SYM_FUNC_START(__fpsimd_restore_state)
 	fpsimd_restore	x0, 1
 	ret
 SYM_FUNC_END(__fpsimd_restore_state)
+
+SYM_FUNC_START(__sve_restore_state)
+	sve_load 0, x1, x2, 3, x4
+	ret
+SYM_FUNC_END(__sve_restore_state)

Nit: maybe this could be named __sve_load_state() for consistency with the EL1 version?

arch/arm64/kvm/hyp/fpsimd.S SYM_FUNC_START(__fpsimd_restore_state) · line 31

Quoted 4 lines

arch/arm64/kvm/hyp/fpsimd.S +4 -0

+SYM_FUNC_START(__sve_save_state)
+	sve_save 0, x1, 2
+	ret
+SYM_FUNC_END(__sve_restore_state)

SYM_FUNC_END(__sve_save_state) here?

Thanks,
Quentin
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Marc Zyngier

author maintainer

2021-03-16 12:17 UTC #3 lore raw

Re: [PATCH 01/10] KVM: arm64: Provide KVM's own save/restore SVE primitives

Quoted 18 lines · Quentin Perret

arch/arm64/kvm/hyp/fpsimd.S SYM_FUNC_START(__fpsimd_restore_state) · line 26
Quoted 13 lines · Marc Zyngier
arch/arm64/kvm/hyp/fpsimd.S +5 -0
index 01f114aa47b0..e4010d1acb79 100644
@@ -19,3 +19,13 @@ SYM_FUNC_START(__fpsimd_restore_state)
 	fpsimd_restore	x0, 1
 	ret
 SYM_FUNC_END(__fpsimd_restore_state)
+
+SYM_FUNC_START(__sve_restore_state)
+	sve_load 0, x1, x2, 3, x4
+	ret
+SYM_FUNC_END(__sve_restore_state)
Nit: maybe this could be named __sve_load_state() for consistency with the EL1 version?

Well, we already have the discrepancy for fpsimd in the same file, so I opted for another kind of consistency...

Quoted 7 lines

arch/arm64/kvm/hyp/fpsimd.S SYM_FUNC_START(__fpsimd_restore_state) · line 31
Quoted 5 lines
arch/arm64/kvm/hyp/fpsimd.S +4 -0
+SYM_FUNC_START(__sve_save_state)
+	sve_save 0, x1, 2
+	ret
+SYM_FUNC_END(__sve_restore_state)
SYM_FUNC_END(__sve_save_state) here?

Yup, good catch.

Thanks,

M.

Signature · 5 lines

Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Will Deacon

maintainer Acked-by on v1 1/10

2021-03-17 14:30 UTC #4 lore raw

Re: [PATCH 01/10] KVM: arm64: Provide KVM's own save/restore SVE primitives

Quoted 5 lines · Marc Zyngier

as we are about to change the way KVM deals with SVE, provide KVM with its own save/restore SVE primitives.
No functional change intended.

Quoted 7 lines

---
 arch/arm64/include/asm/fpsimdmacros.h   |  2 ++
 arch/arm64/include/asm/kvm_hyp.h        |  2 ++
 arch/arm64/kvm/hyp/fpsimd.S             | 10 ++++++++++
 arch/arm64/kvm/hyp/include/hyp/switch.h | 10 +++++-----
 4 files changed, 19 insertions(+), 5 deletions(-)

With the typo spotted by Quentin fixed:

Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Marc Zyngier

author maintainer

2021-03-16 10:13 UTC #5 lore raw

PATCH 2/10 KVM: arm64: Use {read,write}_sysreg_el1 to access ZCR_EL1

Switch to the unified EL1 accessors for ZCR_EL1, which will make things easier for nVHE support.

---
 arch/arm64/kvm/fpsimd.c                 | 3 ++-
 arch/arm64/kvm/hyp/include/hyp/switch.h | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

arch/arm64/kvm/fpsimd.c +2 -1

index 3e081d556e81..b7e36a506d3d 100644
@@ -11,6 +11,7 @@
 #include <linux/kvm_host.h>
 #include <asm/fpsimd.h>
 #include <asm/kvm_asm.h>
+#include <asm/kvm_hyp.h>
 #include <asm/kvm_mmu.h>
 #include <asm/sysreg.h>

@@ -112,7 +113,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
 		fpsimd_save_and_flush_cpu_state();
 
 		if (guest_has_sve)
-			__vcpu_sys_reg(vcpu, ZCR_EL1) = read_sysreg_s(SYS_ZCR_EL12);
+			__vcpu_sys_reg(vcpu, ZCR_EL1) = read_sysreg_el1(SYS_ZCR);
 	} else if (host_has_sve) {
 		/*
 		 * The FPSIMD/SVE state in the CPU has not been touched, and we

arch/arm64/kvm/hyp/include/hyp/switch.h +1 -1

index 807bc4734828..d762d5bdc2d5 100644
@@ -269,7 +269,7 @@ static inline bool __hyp_handle_fpsimd(struct kvm_vcpu *vcpu)
 		__sve_restore_state(vcpu_sve_pffr(vcpu),
 				    &vcpu->arch.ctxt.fp_regs.fpsr,
 				    sve_vq_from_vl(vcpu->arch.sve_max_vl) - 1);
-		write_sysreg_s(__vcpu_sys_reg(vcpu, ZCR_EL1), SYS_ZCR_EL12);
+		write_sysreg_el1(__vcpu_sys_reg(vcpu, ZCR_EL1), SYS_ZCR);
 	} else {
 		__fpsimd_restore_state(&vcpu->arch.ctxt.fp_regs);
 	}

Signature · 6 lines

2.29.2

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Will Deacon

maintainer Acked-by on v1 2/10

2021-03-17 14:31 UTC #6 lore raw

Re: [PATCH 02/10] KVM: arm64: Use {read,write}_sysreg_el1 to access ZCR_EL1

Quoted 35 lines · Marc Zyngier

Switch to the unified EL1 accessors for ZCR_EL1, which will make things easier for nVHE support.

---
 arch/arm64/kvm/fpsimd.c                 | 3 ++-
 arch/arm64/kvm/hyp/include/hyp/switch.h | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

arch/arm64/kvm/fpsimd.c +2 -1

index 3e081d556e81..b7e36a506d3d 100644
@@ -11,6 +11,7 @@
 #include <linux/kvm_host.h>
 #include <asm/fpsimd.h>
 #include <asm/kvm_asm.h>
+#include <asm/kvm_hyp.h>
 #include <asm/kvm_mmu.h>
 #include <asm/sysreg.h>

@@ -112,7 +113,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
 		fpsimd_save_and_flush_cpu_state();
 
 		if (guest_has_sve)
-			__vcpu_sys_reg(vcpu, ZCR_EL1) = read_sysreg_s(SYS_ZCR_EL12);
+			__vcpu_sys_reg(vcpu, ZCR_EL1) = read_sysreg_el1(SYS_ZCR);
 	} else if (host_has_sve) {
 		/*
 		 * The FPSIMD/SVE state in the CPU has not been touched, and we

arch/arm64/kvm/hyp/include/hyp/switch.h +0 -0

index 807bc4734828..d762d5bdc2d5 100644

arch/arm64/kvm/hyp/include/hyp/switch.h static inline bool __hyp_handle_fpsimd(struct kvm_vcpu *vcpu) · line 274

Quoted 7 lines

arch/arm64/kvm/hyp/include/hyp/switch.h +1 -1

@@ -269,7 +269,7 @@ static inline bool __hyp_handle_fpsimd(struct kvm_vcpu *vcpu)
 		__sve_restore_state(vcpu_sve_pffr(vcpu),
 				    &vcpu->arch.ctxt.fp_regs.fpsr,
 				    sve_vq_from_vl(vcpu->arch.sve_max_vl) - 1);
-		write_sysreg_s(__vcpu_sys_reg(vcpu, ZCR_EL1), SYS_ZCR_EL12);
+		write_sysreg_el1(__vcpu_sys_reg(vcpu, ZCR_EL1), SYS_ZCR);
 	} else {
 		__fpsimd_restore_state(&vcpu->arch.ctxt.fp_regs);

Looks straightforward enough:

Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Marc Zyngier

author maintainer

2021-03-16 10:13 UTC #7 lore raw

PATCH 3/10 KVM: arm64: Let vcpu_sve_pffr() handle HYP VAs

The vcpu_sve_pffr() returns a pointer, which can be an interesting thing to do on nVHE. Wrap the pointer with kern_hyp_va(), and take this opportunity to remove the unnecessary casts (sve_state being a void *).

---
 arch/arm64/include/asm/kvm_host.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm64/include/asm/kvm_host.h +2 -2

index 3d10e6527f7d..fb1d78299ba0 100644
@@ -372,8 +372,8 @@ struct kvm_vcpu_arch {
 };
 
 /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
-#define vcpu_sve_pffr(vcpu) ((void *)((char *)((vcpu)->arch.sve_state) + \
-				      sve_ffr_offset((vcpu)->arch.sve_max_vl)))
+#define vcpu_sve_pffr(vcpu) (kern_hyp_va((vcpu)->arch.sve_state) +	\
+			     sve_ffr_offset((vcpu)->arch.sve_max_vl))
 
 #define vcpu_sve_state_size(vcpu) ({					\
 	size_t __size_ret;						\

Signature · 6 lines

2.29.2

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Will Deacon

maintainer Acked-by on v1 3/10

2021-03-17 14:32 UTC #8 lore raw

Re: [PATCH 03/10] KVM: arm64: Let vcpu_sve_pffr() handle HYP VAs

Quoted 15 lines · Marc Zyngier

The vcpu_sve_pffr() returns a pointer, which can be an interesting thing to do on nVHE. Wrap the pointer with kern_hyp_va(), and take this opportunity to remove the unnecessary casts (sve_state being a void *).
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/kvm_host.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/include/asm/kvm_host.h +0 -0
index 3d10e6527f7d..fb1d78299ba0 100644

arch/arm64/include/asm/kvm_host.h struct kvm_vcpu_arch { · line 379

Quoted 10 lines

arch/arm64/include/asm/kvm_host.h +2 -2

@@ -372,8 +372,8 @@ struct kvm_vcpu_arch {
 };
 
 /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
-#define vcpu_sve_pffr(vcpu) ((void *)((char *)((vcpu)->arch.sve_state) + \
-				      sve_ffr_offset((vcpu)->arch.sve_max_vl)))
+#define vcpu_sve_pffr(vcpu) (kern_hyp_va((vcpu)->arch.sve_state) +	\
+			     sve_ffr_offset((vcpu)->arch.sve_max_vl))
 
 #define vcpu_sve_state_size(vcpu) ({					\
 	size_t __size_ret;						\

Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Marc Zyngier

author maintainer

2021-03-16 10:13 UTC #9 lore raw

PATCH 4/10 KVM: arm64: Introduce vcpu_sve_vq() helper

The KVM code contains a number of "sve_vq_from_vl(vcpu->arch.sve_max_vl)" instances, and we are about to add more.

Introduce vcpu_sve_vq() as a shorthand for this expression.

---
 arch/arm64/include/asm/kvm_host.h       | 4 +++-
 arch/arm64/kvm/guest.c                  | 6 +++---
 arch/arm64/kvm/hyp/include/hyp/switch.h | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

arch/arm64/include/asm/kvm_host.h +3 -1

index fb1d78299ba0..c4afe3d3397f 100644
@@ -375,6 +375,8 @@ struct kvm_vcpu_arch {
 #define vcpu_sve_pffr(vcpu) (kern_hyp_va((vcpu)->arch.sve_state) +	\
 			     sve_ffr_offset((vcpu)->arch.sve_max_vl))
 
+#define vcpu_sve_vq(vcpu)	sve_vq_from_vl((vcpu)->arch.sve_max_vl)
+
 #define vcpu_sve_state_size(vcpu) ({					\
 	size_t __size_ret;						\
 	unsigned int __vcpu_vq;						\

@@ -382,7 +384,7 @@ struct kvm_vcpu_arch {
 	if (WARN_ON(!sve_vl_valid((vcpu)->arch.sve_max_vl))) {		\
 		__size_ret = 0;						\
 	} else {							\
-		__vcpu_vq = sve_vq_from_vl((vcpu)->arch.sve_max_vl);	\
+		__vcpu_vq = vcpu_sve_vq(vcpu);				\
 		__size_ret = SVE_SIG_REGS_SIZE(__vcpu_vq);		\
 	}								\
 									\

arch/arm64/kvm/guest.c +3 -3

index 9bbd30e62799..5e7cefbe8509 100644
@@ -299,7 +299,7 @@ static int get_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
 
 	memset(vqs, 0, sizeof(vqs));
 
-	max_vq = sve_vq_from_vl(vcpu->arch.sve_max_vl);
+	max_vq = vcpu_sve_vq(vcpu);
 	for (vq = SVE_VQ_MIN; vq <= max_vq; ++vq)
 		if (sve_vq_available(vq))
 			vqs[vq_word(vq)] |= vq_mask(vq);

@@ -427,7 +427,7 @@ static int sve_reg_to_region(struct sve_state_reg_region *region,
 		if (!vcpu_has_sve(vcpu) || (reg->id & SVE_REG_SLICE_MASK) > 0)
 			return -ENOENT;
 
-		vq = sve_vq_from_vl(vcpu->arch.sve_max_vl);
+		vq = vcpu_sve_vq(vcpu);
 
 		reqoffset = SVE_SIG_ZREG_OFFSET(vq, reg_num) -
 				SVE_SIG_REGS_OFFSET;

@@ -437,7 +437,7 @@ static int sve_reg_to_region(struct sve_state_reg_region *region,
 		if (!vcpu_has_sve(vcpu) || (reg->id & SVE_REG_SLICE_MASK) > 0)
 			return -ENOENT;
 
-		vq = sve_vq_from_vl(vcpu->arch.sve_max_vl);
+		vq = vcpu_sve_vq(vcpu);
 
 		reqoffset = SVE_SIG_PREG_OFFSET(vq, reg_num) -
 				SVE_SIG_REGS_OFFSET;

arch/arm64/kvm/hyp/include/hyp/switch.h +1 -1

index d762d5bdc2d5..fb68271c1a0f 100644
@@ -268,7 +268,7 @@ static inline bool __hyp_handle_fpsimd(struct kvm_vcpu *vcpu)
 	if (sve_guest) {
 		__sve_restore_state(vcpu_sve_pffr(vcpu),
 				    &vcpu->arch.ctxt.fp_regs.fpsr,
-				    sve_vq_from_vl(vcpu->arch.sve_max_vl) - 1);
+				    vcpu_sve_vq(vcpu) - 1);
 		write_sysreg_el1(__vcpu_sys_reg(vcpu, ZCR_EL1), SYS_ZCR);
 	} else {
 		__fpsimd_restore_state(&vcpu->arch.ctxt.fp_regs);

Signature · 6 lines

2.29.2

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Will Deacon

maintainer Acked-by on v1 4/10

2021-03-17 14:01 UTC #10 lore raw

Re: [PATCH 04/10] KVM: arm64: Introduce vcpu_sve_vq() helper

Quoted 17 lines · Marc Zyngier

The KVM code contains a number of "sve_vq_from_vl(vcpu->arch.sve_max_vl)" instances, and we are about to add more.
Introduce vcpu_sve_vq() as a shorthand for this expression.
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/kvm_host.h       | 4 +++-
 arch/arm64/kvm/guest.c                  | 6 +++---
 arch/arm64/kvm/hyp/include/hyp/switch.h | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)
arch/arm64/include/asm/kvm_host.h +0 -0
index fb1d78299ba0..c4afe3d3397f 100644

arch/arm64/include/asm/kvm_host.h struct kvm_vcpu_arch { · line 378

Quoted 4 lines

arch/arm64/include/asm/kvm_host.h +1 -0

@@ -375,6 +375,8 @@ struct kvm_vcpu_arch {
 #define vcpu_sve_pffr(vcpu) (kern_hyp_va((vcpu)->arch.sve_state) +	\
 			     sve_ffr_offset((vcpu)->arch.sve_max_vl))
 
+#define vcpu_sve_vq(vcpu)	sve_vq_from_vl((vcpu)->arch.sve_max_vl)

nit: maybe vcpu_sve_max_vq() would be a better name?

Either way:

Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Marc Zyngier

author maintainer

2021-03-16 10:13 UTC #11 lore raw

PATCH 5/10 KVM: arm64: Rework SVE host-save/guest-restore

In order to keep the code readable, move the host-save/guest-restore sequences in their own functions, with the following changes:
- the hypervisor ZCR is now set from C code
- ZCR_EL2 is always used as the EL2 accessor

This results in some minor assembler macro rework. No functional change intended.

---
 arch/arm64/include/asm/fpsimdmacros.h   |  8 +++--
 arch/arm64/include/asm/kvm_hyp.h        |  2 +-
 arch/arm64/kvm/hyp/fpsimd.S             |  2 +-
 arch/arm64/kvm/hyp/include/hyp/switch.h | 41 +++++++++++++++----------
 4 files changed, 33 insertions(+), 20 deletions(-)

arch/arm64/include/asm/fpsimdmacros.h +6 -2

index e9b72d35b867..a2563992d2dc 100644
@@ -232,8 +232,7 @@
 		str		w\nxtmp, [\xpfpsr, #4]
 .endm
 
-.macro sve_load nxbase, xpfpsr, xvqminus1, nxtmp, xtmp2
-		sve_load_vq	\xvqminus1, x\nxtmp, \xtmp2
+.macro __sve_load nxbase, xpfpsr, nxtmp
  _for n, 0, 31,	_sve_ldr_v	\n, \nxbase, \n - 34
 		_sve_ldr_p	0, \nxbase
 		_sve_wrffr	0

@@ -244,3 +243,8 @@
 		ldr		w\nxtmp, [\xpfpsr, #4]
 		msr		fpcr, x\nxtmp
 .endm
+
+.macro sve_load nxbase, xpfpsr, xvqminus1, nxtmp, xtmp2
+		sve_load_vq	\xvqminus1, x\nxtmp, \xtmp2
+		__sve_load	\nxbase, \xpfpsr, \nxtmp
+.endm

arch/arm64/include/asm/kvm_hyp.h +1 -1

index e8b0f7fcd86b..a3e89424ae63 100644
@@ -86,7 +86,7 @@ void __debug_switch_to_host(struct kvm_vcpu *vcpu);
 void __fpsimd_save_state(struct user_fpsimd_state *fp_regs);
 void __fpsimd_restore_state(struct user_fpsimd_state *fp_regs);
 void __sve_save_state(void *sve_pffr, u32 *fpsr);
-void __sve_restore_state(void *sve_pffr, u32 *fpsr, unsigned int vqminus1);
+void __sve_restore_state(void *sve_pffr, u32 *fpsr);
 
 #ifndef __KVM_NVHE_HYPERVISOR__
 void activate_traps_vhe_load(struct kvm_vcpu *vcpu);

arch/arm64/kvm/hyp/fpsimd.S +1 -1

index e4010d1acb79..a36d363a5981 100644
@@ -21,7 +21,7 @@ SYM_FUNC_START(__fpsimd_restore_state)
 SYM_FUNC_END(__fpsimd_restore_state)
 
 SYM_FUNC_START(__sve_restore_state)
-	sve_load 0, x1, x2, 3, x4
+	__sve_load 0, x1, 2
 	ret
 SYM_FUNC_END(__sve_restore_state)

arch/arm64/kvm/hyp/include/hyp/switch.h +25 ~~-16~~

59 lines not on this page · raw message

Signature · 6 lines

2.29.2

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Will Deacon

maintainer Acked-by on v1 5/10

2021-03-17 14:29 UTC #12 lore raw

Re: [PATCH 05/10] KVM: arm64: Rework SVE host-save/guest-restore

Quoted 15 lines · Marc Zyngier

In order to keep the code readable, move the host-save/guest-restore sequences in their own functions, with the following changes:
- the hypervisor ZCR is now set from C code
- ZCR_EL2 is always used as the EL2 accessor
This results in some minor assembler macro rework. No functional change intended.
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/fpsimdmacros.h   |  8 +++--
 arch/arm64/include/asm/kvm_hyp.h        |  2 +-
 arch/arm64/kvm/hyp/fpsimd.S             |  2 +-
 arch/arm64/kvm/hyp/include/hyp/switch.h | 41 +++++++++++++++----------
 4 files changed, 33 insertions(+), 20 deletions(-)

[...]

Quoted 11 lines

arch/arm64/kvm/hyp/include/hyp/switch.h +3 -0

index fb68271c1a0f..d34dc220a1ce 100644
@@ -196,6 +196,25 @@ static inline bool __populate_fault_info(struct kvm_vcpu *vcpu)
 	return true;
 }
 
+static inline void __hyp_sve_save_host(struct kvm_vcpu *vcpu)
+{
+	struct thread_struct *thread;

arch/arm64/kvm/hyp/include/hyp/switch.h static inline bool __populate_fault_info(struct kvm_vcpu *vcpu) · line 211

Quoted 10 lines

arch/arm64/kvm/hyp/include/hyp/switch.h +10 -0

+
+	thread = container_of(vcpu->arch.host_fpsimd_state, struct thread_struct,
+			      uw.fpsimd_state);
+
+	__sve_save_state(sve_pffr(thread), &vcpu->arch.host_fpsimd_state->fpsr);
+}
+
+static inline void __hyp_sve_restore_guest(struct kvm_vcpu *vcpu)
+{
+	if (read_sysreg_s(SYS_ZCR_EL2) != (vcpu_sve_vq(vcpu) - 1))

Strictly speaking, we should probably be extracting the LEN field from ZCR_EL2, otherwise this has the potential to go horribly wrong if any of the RES0 bits are allocated in future.

Other than that:

Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Marc Zyngier

author maintainer

2021-03-17 14:55 UTC #13 lore raw

Re: [PATCH 05/10] KVM: arm64: Rework SVE host-save/guest-restore

Quoted 37 lines · Will Deacon

Quoted 15 lines · Marc Zyngier
In order to keep the code readable, move the host-save/guest-restore sequences in their own functions, with the following changes:
- the hypervisor ZCR is now set from C code
- ZCR_EL2 is always used as the EL2 accessor
This results in some minor assembler macro rework. No functional change intended.
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/fpsimdmacros.h   |  8 +++--
 arch/arm64/include/asm/kvm_hyp.h        |  2 +-
 arch/arm64/kvm/hyp/fpsimd.S             |  2 +-
 arch/arm64/kvm/hyp/include/hyp/switch.h | 41 +++++++++++++++----------
 4 files changed, 33 insertions(+), 20 deletions(-)
[...]
Quoted 17 lines
arch/arm64/kvm/hyp/include/hyp/switch.h +9 -0
index fb68271c1a0f..d34dc220a1ce 100644
@@ -196,6 +196,25 @@ static inline bool __populate_fault_info(struct kvm_vcpu *vcpu)
 	return true;
 }
 
+static inline void __hyp_sve_save_host(struct kvm_vcpu *vcpu)
+{
+	struct thread_struct *thread;
+
+	thread = container_of(vcpu->arch.host_fpsimd_state, struct thread_struct,
+			      uw.fpsimd_state);
+
+	__sve_save_state(sve_pffr(thread), &vcpu->arch.host_fpsimd_state->fpsr);
+}

Quoted 8 lines

arch/arm64/kvm/hyp/include/hyp/switch.h static inline bool __populate_fault_info(struct kvm_vcpu *vcpu) · line 211
Quoted 4 lines
arch/arm64/kvm/hyp/include/hyp/switch.h +4 -0
+
+static inline void __hyp_sve_restore_guest(struct kvm_vcpu *vcpu)
+{
+	if (read_sysreg_s(SYS_ZCR_EL2) != (vcpu_sve_vq(vcpu) - 1))
Strictly speaking, we should probably be extracting the LEN field from ZCR_EL2, otherwise this has the potential to go horribly wrong if any of the RES0 bits are allocated in future.

Good point, the equivalent asm macro has a BIC to that effect. I'll fix that in the next round, as there are similar patterns in a number of places.

Quoted 3 lines

Other than that:
Acked-by Will Deacon <[email protected]>

Thanks,

M.

Signature · 5 lines

Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Marc Zyngier

author maintainer

2021-03-16 10:13 UTC #14 lore raw

PATCH 6/10 KVM: arm64: Map SVE context at EL2 when available

When running on nVHE, and that the vcpu supports SVE, map the SVE state at EL2 so that KVM can access it.

---
 arch/arm64/kvm/fpsimd.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

arch/arm64/kvm/fpsimd.c +11 -0

index b7e36a506d3d..84afca5ed6f2 100644
@@ -43,6 +43,17 @@ int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu)
 	if (ret)
 		goto error;
 
+	if (vcpu->arch.sve_state) {
+		void *sve_end;
+
+		sve_end = vcpu->arch.sve_state + vcpu_sve_state_size(vcpu) + 1;
+
+		ret = create_hyp_mappings(vcpu->arch.sve_state, sve_end,
+					  PAGE_HYP);
+		if (ret)
+			goto error;
+	}
+
 	vcpu->arch.host_thread_info = kern_hyp_va(ti);
 	vcpu->arch.host_fpsimd_state = kern_hyp_va(fpsimd);
 error:

Signature · 6 lines

2.29.2

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Will Deacon

maintainer

2021-03-17 16:01 UTC #15 lore raw

Re: [PATCH 06/10] KVM: arm64: Map SVE context at EL2 when available

Quoted 13 lines · Marc Zyngier

When running on nVHE, and that the vcpu supports SVE, map the SVE state at EL2 so that KVM can access it.
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/fpsimd.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
arch/arm64/kvm/fpsimd.c +0 -0
index b7e36a506d3d..84afca5ed6f2 100644

arch/arm64/kvm/fpsimd.c int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu) · line 49

Quoted 7 lines

arch/arm64/kvm/fpsimd.c +4 -0

@@ -43,6 +43,17 @@ int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu)
 	if (ret)
 		goto error;
 
+	if (vcpu->arch.sve_state) {
+		void *sve_end;
+
+		sve_end = vcpu->arch.sve_state + vcpu_sve_state_size(vcpu) + 1;

Why do you need the '+ 1' here?

Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Marc Zyngier

author maintainer

2021-03-18 08:56 UTC #16 lore raw

Re: [PATCH 06/10] KVM: arm64: Map SVE context at EL2 when available

Quoted 18 lines · Will Deacon

Quoted 16 lines · Marc Zyngier
When running on nVHE, and that the vcpu supports SVE, map the SVE state at EL2 so that KVM can access it.
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/fpsimd.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
arch/arm64/kvm/fpsimd.c +0 -0
index b7e36a506d3d..84afca5ed6f2 100644
@@ -43,6 +43,17 @@ int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu)
 	if (ret)
 		goto error;
 

Quoted 6 lines

arch/arm64/kvm/fpsimd.c int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu) · line 49
Quoted 4 lines
arch/arm64/kvm/fpsimd.c +4 -0
+	if (vcpu->arch.sve_state) {
+		void *sve_end;
+
+		sve_end = vcpu->arch.sve_state + vcpu_sve_state_size(vcpu) + 1;
Why do you need the '+ 1' here?

Only the need to add off-by-one bugs once in a while. I'll fix that in the next round.

Thanks,

M.

Signature · 5 lines

Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Marc Zyngier

author maintainer

2021-03-16 10:13 UTC #17 lore raw

PATCH 7/10 KVM: arm64: Save guest's ZCR_EL1 before saving the FPSIMD state

Make sure the guest's ZCR_EL1 is saved before we save/flush the state. This will be useful in later patches.

---
 arch/arm64/kvm/fpsimd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm64/kvm/fpsimd.c +2 -2

index 84afca5ed6f2..b5f95abd23f5 100644
@@ -121,10 +121,10 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
 	local_irq_save(flags);
 
 	if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED) {
-		fpsimd_save_and_flush_cpu_state();
-
 		if (guest_has_sve)
 			__vcpu_sys_reg(vcpu, ZCR_EL1) = read_sysreg_el1(SYS_ZCR);
+
+		fpsimd_save_and_flush_cpu_state();
 	} else if (host_has_sve) {
 		/*
 		 * The FPSIMD/SVE state in the CPU has not been touched, and we

Signature · 6 lines

2.29.2

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Will Deacon

maintainer

2021-03-17 17:17 UTC #18 lore raw

Re: [PATCH 07/10] KVM: arm64: Save guest's ZCR_EL1 before saving the FPSIMD state

Quoted 13 lines · Marc Zyngier

Make sure the guest's ZCR_EL1 is saved before we save/flush the state. This will be useful in later patches.
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/fpsimd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kvm/fpsimd.c +0 -0
index 84afca5ed6f2..b5f95abd23f5 100644

arch/arm64/kvm/fpsimd.c void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu) · line 127

Quoted 9 lines

arch/arm64/kvm/fpsimd.c +2 -2

@@ -121,10 +121,10 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
 	local_irq_save(flags);
 
 	if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED) {
-		fpsimd_save_and_flush_cpu_state();
-
 		if (guest_has_sve)
 			__vcpu_sys_reg(vcpu, ZCR_EL1) = read_sysreg_el1(SYS_ZCR);
+
+		fpsimd_save_and_flush_cpu_state();

I _think_ fpsimd_save_and_flush_cpu_state() contains a RDVL instruction to get at the vector length for sve_get_vl(), and this ends up reading from ZCR_EL1. So I'm not sure it's save to move it like this.

Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Will Deacon

maintainer Acked-by

2021-03-17 17:20 UTC #19 lore raw

Re: [PATCH 07/10] KVM: arm64: Save guest's ZCR_EL1 before saving the FPSIMD state

Quoted 16 lines · Will Deacon

Quoted 15 lines · Marc Zyngier
Make sure the guest's ZCR_EL1 is saved before we save/flush the state. This will be useful in later patches.
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/fpsimd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kvm/fpsimd.c +0 -0
index 84afca5ed6f2..b5f95abd23f5 100644
@@ -121,10 +121,10 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
 	local_irq_save(flags);
 

Quoted 11 lines

arch/arm64/kvm/fpsimd.c void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu) · line 127
Quoted 7 lines
arch/arm64/kvm/fpsimd.c +2 -2
 	if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED) {
-		fpsimd_save_and_flush_cpu_state();
-
 		if (guest_has_sve)
 			__vcpu_sys_reg(vcpu, ZCR_EL1) = read_sysreg_el1(SYS_ZCR);
+
+		fpsimd_save_and_flush_cpu_state();
I _think_ fpsimd_save_and_flush_cpu_state() contains a RDVL instruction to get at the vector length for sve_get_vl(), and this ends up reading from ZCR_EL1. So I'm not sure it's save to move it like this.

Duh, we're not changing the register here, but just saving it off. So it's fine:

Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Marc Zyngier

author maintainer

2021-03-16 10:13 UTC #20 lore raw

PATCH 8/10 KVM: arm64: Add a nVHE-specific SVE VQ reset hypercall

ZCR_EL2 controls the upper bound for ZCR_EL1, and is set to a potentially lower limit when the guest uses SVE.

In order to restore the SVE state on the EL1 host, we must first reset ZCR_EL2 to its original value.

Provide a hypervall that perform this reset.

---
 arch/arm64/include/asm/kvm_asm.h   | 1 +
 arch/arm64/include/asm/kvm_host.h  | 4 +++-
 arch/arm64/kvm/hyp/nvhe/hyp-main.c | 8 ++++++++
 3 files changed, 12 insertions(+), 1 deletion(-)

arch/arm64/include/asm/kvm_asm.h +1 -0

index 22d933e9b59e..7ae947934ec9 100644
@@ -57,6 +57,7 @@
 #define __KVM_HOST_SMCCC_FUNC___kvm_get_mdcr_el2		12
 #define __KVM_HOST_SMCCC_FUNC___vgic_v3_save_aprs		13
 #define __KVM_HOST_SMCCC_FUNC___vgic_v3_restore_aprs		14
+#define __KVM_HOST_SMCCC_FUNC___kvm_reset_sve_vq		15
 
 #ifndef __ASSEMBLY__

arch/arm64/include/asm/kvm_host.h +3 -1

index c4afe3d3397f..9108ccc80653 100644
@@ -593,7 +593,9 @@ int kvm_test_age_hva(struct kvm *kvm, unsigned long hva);
 void kvm_arm_halt_guest(struct kvm *kvm);
 void kvm_arm_resume_guest(struct kvm *kvm);
 
-#define kvm_call_hyp_nvhe(f, ...)						\
+static inline void __kvm_reset_sve_vq(void) {}
+
+#define kvm_call_hyp_nvhe(f, ...)					\
 	({								\
 		struct arm_smccc_res res;				\
 									\

arch/arm64/kvm/hyp/nvhe/hyp-main.c +8 -0

index f012f8665ecc..9fdd8d6e3554 100644
@@ -106,6 +106,13 @@ static void handle___vgic_v3_restore_aprs(struct kvm_cpu_context *host_ctxt)
 	__vgic_v3_restore_aprs(kern_hyp_va(cpu_if));
 }
 
+static void handle___kvm_reset_sve_vq(struct kvm_cpu_context *host_ctxt)
+{
+	if (system_supports_sve() &&
+	    read_sysreg_s(SYS_ZCR_EL2) != ZCR_ELx_LEN_MASK)
+		write_sysreg_s(ZCR_ELx_LEN_MASK, SYS_ZCR_EL2);
+}
+
 typedef void (*hcall_t)(struct kvm_cpu_context *);
 
 #define HANDLE_FUNC(x)	[__KVM_HOST_SMCCC_FUNC_##x] = (hcall_t)handle_##x

@@ -125,6 +132,7 @@ static const hcall_t host_hcall[] = {
 	HANDLE_FUNC(__kvm_get_mdcr_el2),
 	HANDLE_FUNC(__vgic_v3_save_aprs),
 	HANDLE_FUNC(__vgic_v3_restore_aprs),
+	HANDLE_FUNC(__kvm_reset_sve_vq),
 };
 
 static void handle_host_hcall(struct kvm_cpu_context *host_ctxt)

Signature · 6 lines

2.29.2

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Quentin Perret

2021-03-16 10:46 UTC #21 lore raw

Re: [PATCH 08/10] KVM: arm64: Add a nVHE-specific SVE VQ reset hypercall

arch/arm64/include/asm/kvm_host.h int kvm_test_age_hva(struct kvm *kvm, unsigned long hva); · line 596

Quoted 10 lines · Marc Zyngier

arch/arm64/include/asm/kvm_host.h +1 -1

index c4afe3d3397f..9108ccc80653 100644
@@ -593,7 +593,9 @@ int kvm_test_age_hva(struct kvm *kvm, unsigned long hva);
 void kvm_arm_halt_guest(struct kvm *kvm);
 void kvm_arm_resume_guest(struct kvm *kvm);
 
-#define kvm_call_hyp_nvhe(f, ...)						\
+static inline void __kvm_reset_sve_vq(void) {}

Why is this one needed? With an explicit call to kvm_call_hyp_nvhe() you shouldn't need to provide a VHE implementation I think.

Thanks,
Quentin
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Marc Zyngier

author maintainer

2021-03-16 12:18 UTC #22 lore raw

Re: [PATCH 08/10] KVM: arm64: Add a nVHE-specific SVE VQ reset hypercall

Quoted 10 lines · Quentin Perret

Quoted 8 lines · Marc Zyngier

arch/arm64/include/asm/kvm_host.h +0 -0

index c4afe3d3397f..9108ccc80653 100644
@@ -593,7 +593,9 @@ int kvm_test_age_hva(struct kvm *kvm, unsigned long hva);
 void kvm_arm_halt_guest(struct kvm *kvm);
 void kvm_arm_resume_guest(struct kvm *kvm);

Quoted 5 lines

arch/arm64/include/asm/kvm_host.h int kvm_test_age_hva(struct kvm *kvm, unsigned long hva); · line 596
Quoted 2 lines
arch/arm64/include/asm/kvm_host.h +1 -1
-#define kvm_call_hyp_nvhe(f, ...)						\
+static inline void __kvm_reset_sve_vq(void) {}
Why is this one needed? With an explicit call to kvm_call_hyp_nvhe() you shouldn't need to provide a VHE implementation I think.

Did I mention that I positively hate kvm_call_hyp_nvhe()? ;-)

But yes, you are right, this can be further simplified.

Thanks,

M.

Signature · 5 lines

Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Andrew Scull

2021-03-16 14:24 UTC #23 lore raw

Re: [PATCH 08/10] KVM: arm64: Add a nVHE-specific SVE VQ reset hypercall

Quoted 7 lines · Marc Zyngier

ZCR_EL2 controls the upper bound for ZCR_EL1, and is set to a potentially lower limit when the guest uses SVE.
In order to restore the SVE state on the EL1 host, we must first reset ZCR_EL2 to its original value.
Provide a hypervall that perform this reset.

Is there a good reason to have an explicit hypercall vs trapping the host access to SVE and restoring in that event?

It's quite easy to do trap handling at EL2 now and it could let things be even lazier, if that's any benefit in this case.

Trapping seems to have had a bad rep in other conversations but I'm not sure the same reasoning applies to this as well, or not.
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Marc Zyngier

author maintainer

2021-03-16 15:00 UTC #24 lore raw

Re: [PATCH 08/10] KVM: arm64: Add a nVHE-specific SVE VQ reset hypercall

Quoted 5 lines · Andrew Scull

Quoted 3 lines · Marc Zyngier
ZCR_EL2 controls the upper bound for ZCR_EL1, and is set to a potentially lower limit when the guest uses SVE.

Quoted 7 lines

Quoted 4 lines
In order to restore the SVE state on the EL1 host, we must first reset ZCR_EL2 to its original value.
Provide a hypervall that perform this reset.
Is there a good reason to have an explicit hypercall vs trapping the host access to SVE and restoring in that event?

Trapping ZCR_EL2 isn't possible, as it would UNDEF at EL1. Trapping ZCR_EL1 accesses is possible though, but it'd mean leaving the SVE traps enabled on guest exit, something we currently don't do.

Quoted 2 lines

It's quite easy to do trap handling at EL2 now and it could let things be even lazier, if that's any benefit in this case.

We don't really have a good infrastructure for dealing with individual sysregs (pKVM will eventually change that, but we're not there yet, and it isn't clear how that'd apply to non-protected), so we'd have to deal with the whole SVE EC.

What we could do is:

- set CPTR_EL2.TZ set on guest exit

- on SVE trap, reset ZCR_EL2, clear CPTR_EL2.TZ, reexecute the faulting instruction.

I can have a look at how badly it looks.

Quoted 2 lines

Trapping seems to have had a bad rep in other conversations but I'm not sure the same reasoning applies to this as well, or not.

HVC and traps have the same basic cost. I seriously doubt you can measure the difference on any real CPU.

Thanks,

M.

Signature · 5 lines

Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Marc Zyngier

author maintainer

2021-03-16 10:13 UTC #25 lore raw

PATCH 9/10 KVM: arm64: Save/restore SVE state for nVHE

Implement the SVE save/restore for nVHE, following a similar logic to that of the VHE implementation:

- the SVE state is switched on trap from EL1 to EL2

- no further changes to ZCR_EL2 occur as long as the guest isn't preempted or exit to userspace

- on vcpu_put(), ZCR_EL2 is reset to its default value, and ZCR_EL1 restored to the default guest value

---
 arch/arm64/kvm/fpsimd.c                 | 15 ++++++++--
 arch/arm64/kvm/hyp/include/hyp/switch.h | 37 +++++++++----------------
 arch/arm64/kvm/hyp/nvhe/switch.c        |  4 +--
 3 files changed, 28 insertions(+), 28 deletions(-)

arch/arm64/kvm/fpsimd.c +13 -2

index b5f95abd23f5..cc6cdea69596 100644
@@ -121,11 +121,22 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
 	local_irq_save(flags);
 
 	if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED) {
-		if (guest_has_sve)
+		if (guest_has_sve) {
 			__vcpu_sys_reg(vcpu, ZCR_EL1) = read_sysreg_el1(SYS_ZCR);
 
+			/* Restore the VL that was saved when bound to the CPU */
+			if (!has_vhe()) {
+				u64 zcr;
+
+				kvm_call_hyp(__kvm_reset_sve_vq);
+				zcr = vcpu_sve_vq(vcpu) - 1;
+				if (read_sysreg_el1(SYS_ZCR) != zcr)
+					write_sysreg_el1(zcr, SYS_ZCR);
+			}
+		}
+
 		fpsimd_save_and_flush_cpu_state();
-	} else if (host_has_sve) {
+	} else if (has_vhe() && host_has_sve) {
 		/*
 		 * The FPSIMD/SVE state in the CPU has not been touched, and we
 		 * have SVE (and VHE): CPACR_EL1 (alias CPTR_EL2) has been

arch/arm64/kvm/hyp/include/hyp/switch.h +13 ~~-24~~

index d34dc220a1ce..777b9bfa4478 100644
@@ -218,25 +218,19 @@ static inline void __hyp_sve_restore_guest(struct kvm_vcpu *vcpu)
 /* Check for an FPSIMD/SVE trap and handle as appropriate */
 static inline bool __hyp_handle_fpsimd(struct kvm_vcpu *vcpu)
 {
-	bool vhe, sve_guest, sve_host;
+	bool sve_guest, sve_host;
 	u8 esr_ec;
+	u64 reg;
 
 	if (!system_supports_fpsimd())
 		return false;
 
-	/*
-	 * Currently system_supports_sve() currently implies has_vhe(),
-	 * so the check is redundant. However, has_vhe() can be determined
-	 * statically and helps the compiler remove dead code.
-	 */
-	if (has_vhe() && system_supports_sve()) {
+	if (system_supports_sve()) {
 		sve_guest = vcpu_has_sve(vcpu);
 		sve_host = vcpu->arch.flags & KVM_ARM64_HOST_SVE_IN_USE;
-		vhe = true;
 	} else {
 		sve_guest = false;
 		sve_host = false;
-		vhe = has_vhe();
 	}
 
 	esr_ec = kvm_vcpu_trap_get_class(vcpu);

@@ -245,31 +239,26 @@ static inline bool __hyp_handle_fpsimd(struct kvm_vcpu *vcpu)
 		return false;
 
 	/* Don't handle SVE traps for non-SVE vcpus here: */
-	if (!sve_guest)
-		if (esr_ec != ESR_ELx_EC_FP_ASIMD)
-			return false;
+	if (!sve_guest && esr_ec != ESR_ELx_EC_FP_ASIMD)
+		return false;
 
 	/* Valid trap.  Switch the context: */
-
-	if (vhe) {
-		u64 reg = read_sysreg(cpacr_el1) | CPACR_EL1_FPEN;
-
+	if (has_vhe()) {
+		reg = CPACR_EL1_FPEN;
 		if (sve_guest)
 			reg |= CPACR_EL1_ZEN;
 
-		write_sysreg(reg, cpacr_el1);
+		sysreg_clear_set(cpacr_el1, 0, reg);
 	} else {
-		write_sysreg(read_sysreg(cptr_el2) & ~(u64)CPTR_EL2_TFP,
-			     cptr_el2);
-	}
+		reg = CPTR_EL2_TFP;
+		if (sve_guest)
+			reg |= CPTR_EL2_TZ;
 
+		sysreg_clear_set(cptr_el2, reg, 0);
+	}
 	isb();
 
 	if (vcpu->arch.flags & KVM_ARM64_FP_HOST) {
-		/*
-		 * In the SVE case, VHE is assumed: it is enforced by
-		 * Kconfig and kvm_arch_init().
-		 */
 		if (sve_host)
 			__hyp_sve_save_host(vcpu);
 		else

arch/arm64/kvm/hyp/nvhe/switch.c +2 -2

index f3d0e9eca56c..ef47abcef63f 100644
@@ -41,9 +41,9 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
 	__activate_traps_common(vcpu);
 
 	val = CPTR_EL2_DEFAULT;
-	val |= CPTR_EL2_TTA | CPTR_EL2_TZ | CPTR_EL2_TAM;
+	val |= CPTR_EL2_TTA | CPTR_EL2_TAM;
 	if (!update_fp_enabled(vcpu)) {
-		val |= CPTR_EL2_TFP;
+		val |= CPTR_EL2_TFP | CPTR_EL2_TZ;
 		__activate_traps_fpsimd32(vcpu);
 	}

Signature · 6 lines

2.29.2

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Will Deacon

maintainer

2021-03-17 17:57 UTC #26 lore raw

Re: [PATCH 09/10] KVM: arm64: Save/restore SVE state for nVHE

Quoted 25 lines · Marc Zyngier

Implement the SVE save/restore for nVHE, following a similar logic to that of the VHE implementation:
- the SVE state is switched on trap from EL1 to EL2
- no further changes to ZCR_EL2 occur as long as the guest isn't preempted or exit to userspace
- on vcpu_put(), ZCR_EL2 is reset to its default value, and ZCR_EL1 restored to the default guest value
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/fpsimd.c                 | 15 ++++++++--
 arch/arm64/kvm/hyp/include/hyp/switch.h | 37 +++++++++----------------
 arch/arm64/kvm/hyp/nvhe/switch.c        |  4 +--
 3 files changed, 28 insertions(+), 28 deletions(-)
arch/arm64/kvm/fpsimd.c +0 -0
index b5f95abd23f5..cc6cdea69596 100644
@@ -121,11 +121,22 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
 	local_irq_save(flags);
 

arch/arm64/kvm/fpsimd.c void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu) · line 131

Quoted 10 lines

arch/arm64/kvm/fpsimd.c +6 -1

 	if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED) {
-		if (guest_has_sve)
+		if (guest_has_sve) {
 			__vcpu_sys_reg(vcpu, ZCR_EL1) = read_sysreg_el1(SYS_ZCR);
 
+			/* Restore the VL that was saved when bound to the CPU */
+			if (!has_vhe()) {
+				u64 zcr;
+
+				kvm_call_hyp(__kvm_reset_sve_vq);

What would go wrong if we did this unconditionally on return to the host, or is it just a performance thing to move work off the fast path where we return back to the same vCPU?

Will
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Marc Zyngier

author maintainer

2021-03-18 09:12 UTC #27 lore raw

Re: [PATCH 09/10] KVM: arm64: Save/restore SVE state for nVHE

Quoted 32 lines · Will Deacon

Quoted 30 lines · Marc Zyngier
Implement the SVE save/restore for nVHE, following a similar logic to that of the VHE implementation:
- the SVE state is switched on trap from EL1 to EL2
- no further changes to ZCR_EL2 occur as long as the guest isn't preempted or exit to userspace
- on vcpu_put(), ZCR_EL2 is reset to its default value, and ZCR_EL1 restored to the default guest value
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/fpsimd.c                 | 15 ++++++++--
 arch/arm64/kvm/hyp/include/hyp/switch.h | 37 +++++++++----------------
 arch/arm64/kvm/hyp/nvhe/switch.c        |  4 +--
 3 files changed, 28 insertions(+), 28 deletions(-)
arch/arm64/kvm/fpsimd.c +1 -1
index b5f95abd23f5..cc6cdea69596 100644
@@ -121,11 +121,22 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
 	local_irq_save(flags);
 
 	if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED) {
-		if (guest_has_sve)
+		if (guest_has_sve) {
 			__vcpu_sys_reg(vcpu, ZCR_EL1) = read_sysreg_el1(SYS_ZCR);
 

Quoted 9 lines

arch/arm64/kvm/fpsimd.c void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu) · line 131
Quoted 5 lines
arch/arm64/kvm/fpsimd.c +5 -0
+			/* Restore the VL that was saved when bound to the CPU */
+			if (!has_vhe()) {
+				u64 zcr;
+
+				kvm_call_hyp(__kvm_reset_sve_vq);
What would go wrong if we did this unconditionally on return to the host, or is it just a performance thing to move work off the fast path where we return back to the same vCPU?

Nothing would go wrong, but we'd also need to re-adjust it on entry if the FPSIMD file is dirty, making it doubly painful on the fast path.

Doing the reset on vcpu_put() ensures this is only done once, either on preemption or on return to userspace, at the expense of an EL2 round trip. That's consistent with what we do in general for CPU state that doesn't directly affect the execution of the kernel.

M.

Signature · 5 lines

Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Marc Zyngier

author maintainer

2021-03-16 10:23 UTC #28 lore raw

PATCH 10/10 KVM: arm64: Enable SVE support for nVHE

From: Daniel Kiss <[email protected]>

Now that KVM is equipped to deal with SVE on nVHE, remove the code preventing it from being used as well as the bits of documentation that were mentioning the incompatibility.

Signed-off-by Daniel Kiss <[email protected]>

---
 arch/arm64/Kconfig                |  7 -------
 arch/arm64/include/asm/kvm_host.h | 13 -------------
 arch/arm64/kvm/arm.c              |  5 -----
 arch/arm64/kvm/reset.c            |  4 ----
 4 files changed, 29 deletions(-)

arch/arm64/Kconfig +0 -7

index 1f212b47a48a..2690543799fb 100644
@@ -1686,7 +1686,6 @@ endmenu
 config ARM64_SVE
 	bool "ARM Scalable Vector Extension support"
 	default y
-	depends on !KVM || ARM64_VHE
 	help
 	  The Scalable Vector Extension (SVE) is an extension to the AArch64
 	  execution state which complements and extends the SIMD functionality

@@ -1715,12 +1714,6 @@ config ARM64_SVE
 	  booting the kernel.  If unsure and you are not observing these
 	  symptoms, you should assume that it is safe to say Y.
 
-	  CPUs that support SVE are architecturally required to support the
-	  Virtualization Host Extensions (VHE), so the kernel makes no
-	  provision for supporting SVE alongside KVM without VHE enabled.
-	  Thus, you will need to enable CONFIG_ARM64_VHE if you want to support
-	  KVM in the same kernel image.
-
 config ARM64_MODULE_PLTS
 	bool "Use PLTs to allow module memory to spill over into vmalloc area"
 	depends on MODULES

arch/arm64/include/asm/kvm_host.h +0 ~~-13~~

index 9108ccc80653..62a5f14dde36 100644
@@ -696,19 +696,6 @@ static inline void kvm_init_host_cpu_context(struct kvm_cpu_context *cpu_ctxt)
 	ctxt_sys_reg(cpu_ctxt, MPIDR_EL1) = read_cpuid_mpidr();
 }
 
-static inline bool kvm_arch_requires_vhe(void)
-{
-	/*
-	 * The Arm architecture specifies that implementation of SVE
-	 * requires VHE also to be implemented.  The KVM code for arm64
-	 * relies on this when SVE is present:
-	 */
-	if (system_supports_sve())
-		return true;
-
-	return false;
-}
-
 void kvm_arm_vcpu_ptrauth_trap(struct kvm_vcpu *vcpu);
 
 static inline void kvm_arch_hardware_unsetup(void) {}

arch/arm64/kvm/arm.c +0 -5

index fc4c95dd2d26..ef92b7d32ebd 100644
@@ -1889,11 +1889,6 @@ int kvm_arch_init(void *opaque)
 
 	in_hyp_mode = is_kernel_in_hyp_mode();
 
-	if (!in_hyp_mode && kvm_arch_requires_vhe()) {
-		kvm_pr_unimpl("CPU unsupported in non-VHE mode, not initializing\n");
-		return -ENODEV;
-	}
-
 	if (cpus_have_final_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE) ||
 	    cpus_have_final_cap(ARM64_WORKAROUND_1508412))
 		kvm_info("Guests without required CPU erratum workarounds can deadlock system!\n" \

arch/arm64/kvm/reset.c +0 -4

index 47f3f035f3ea..f08b1e7ebf68 100644
@@ -74,10 +74,6 @@ static int kvm_vcpu_enable_sve(struct kvm_vcpu *vcpu)
 	if (!system_supports_sve())
 		return -EINVAL;
 
-	/* Verify that KVM startup enforced this when SVE was detected: */
-	if (WARN_ON(!has_vhe()))
-		return -EINVAL;
-
 	vcpu->arch.sve_max_vl = kvm_sve_max_vl;
 
 	/*

Signature · 6 lines

2.29.2

_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Will Deacon

maintainer Acked-by on v1 10/10

2021-03-17 18:00 UTC #29 lore raw

Re: [PATCH 10/10] KVM: arm64: Enable SVE support for nVHE

Quoted 6 lines · Marc Zyngier

From: Daniel Kiss <[email protected]>
Now that KVM is equipped to deal with SVE on nVHE, remove the code preventing it from being used as well as the bits of documentation that were mentioning the incompatibility.

Quoted 8 lines

Signed-off-by Daniel Kiss <[email protected]>

---
 arch/arm64/Kconfig                |  7 -------
 arch/arm64/include/asm/kvm_host.h | 13 -------------
 arch/arm64/kvm/arm.c              |  5 -----
 arch/arm64/kvm/reset.c            |  4 ----
 4 files changed, 29 deletions(-)