| Message ID | 20210316101312.102925-9-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Enable SVE support on nVHE systems | expand |
On Tuesday 16 Mar 2021 at 10:13:10 (+0000), Marc Zyngier wrote: > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index c4afe3d3397f..9108ccc80653 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -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
On Tue, 16 Mar 2021 10:45:55 +0000, Quentin Perret <qperret@google.com> wrote: > > On Tuesday 16 Mar 2021 at 10:13:10 (+0000), Marc Zyngier wrote: > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > > index c4afe3d3397f..9108ccc80653 100644 > > --- a/arch/arm64/include/asm/kvm_host.h > > +++ b/arch/arm64/include/asm/kvm_host.h > > @@ -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. Did I mention that I positively hate kvm_call_hyp_nvhe()? ;-) But yes, you are right, this can be further simplified. Thanks, M.
On Tue, Mar 16, 2021 at 10:13:10AM +0000, Marc Zyngier wrote: > 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.
On Tue, 16 Mar 2021 14:24:38 +0000, Andrew Scull <ascull@google.com> wrote: > > On Tue, Mar 16, 2021 at 10:13:10AM +0000, Marc Zyngier wrote: > > 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? 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. > 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. > 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.
diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h index 22d933e9b59e..7ae947934ec9 100644 --- a/arch/arm64/include/asm/kvm_asm.h +++ b/arch/arm64/include/asm/kvm_asm.h @@ -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__ diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index c4afe3d3397f..9108ccc80653 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -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; \ \ diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c index f012f8665ecc..9fdd8d6e3554 100644 --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c @@ -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)
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. Signed-off-by: Marc Zyngier <maz@kernel.org> --- 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(-)