[PATCH v6 5/6] arm/arm64: KVM: Introduce armv8 fp/simd vcpu fields and helpers
Christoffer Dall
christoffer.dall at linaro.org
Tue Jan 12 06:13:09 PST 2016
On Mon, Jan 11, 2016 at 04:31:03PM -0800, Mario Smarduch wrote:
> On 1/10/2016 8:32 AM, Christoffer Dall wrote:
> > On Sat, Dec 26, 2015 at 01:56:29PM -0800, Mario Smarduch wrote:
> >> Similar to armv7 add helper functions to enable access to fp/smid registers on
> >> guest entry. Save guest fpexc32_el2 on vcpu_put, check if guest is 32 bit.
> >> Save guest and restore host registers from host kernel, and check
> >> if fp/simd registers are dirty, lastly add cptr_el2 vcpu field.
> >>
> >> Signed-off-by: Mario Smarduch <m.smarduch at samsung.com>
> >> ---
> >> arch/arm/include/asm/kvm_emulate.h | 12 ++++++++++++
> >> arch/arm64/include/asm/kvm_asm.h | 5 +++++
> >> arch/arm64/include/asm/kvm_emulate.h | 26 ++++++++++++++++++++++++--
> >> arch/arm64/include/asm/kvm_host.h | 12 +++++++++++-
> >> arch/arm64/kvm/hyp/hyp-entry.S | 26 ++++++++++++++++++++++++++
> >> 5 files changed, 78 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/arm/include/asm/kvm_emulate.h b/arch/arm/include/asm/kvm_emulate.h
> >> index d4d9da1..a434dc5 100644
> >> --- a/arch/arm/include/asm/kvm_emulate.h
> >> +++ b/arch/arm/include/asm/kvm_emulate.h
> >> @@ -284,6 +284,12 @@ static inline bool vcpu_vfp_isdirty(struct kvm_vcpu *vcpu)
> >> {
> >> return !(vcpu->arch.hcptr & (HCPTR_TCP(10) | HCPTR_TCP(11)));
> >> }
> >> +
> >> +static inline bool vcpu_guest_is_32bit(struct kvm_vcpu *vcpu)
> >> +{
> >> + return true;
> >> +}
> >> +static inline void vcpu_save_fpexc(struct kvm_vcpu *vcpu) {}
> >> #else
> >> static inline void vcpu_trap_vfp_enable(struct kvm_vcpu *vcpu)
> >> {
> >> @@ -295,6 +301,12 @@ static inline bool vcpu_vfp_isdirty(struct kvm_vcpu *vcpu)
> >> {
> >> return false;
> >> }
> >> +
> >> +static inline bool vcpu_guest_is_32bit(struct kvm_vcpu *vcpu)
> >> +{
> >> + return true;
> >> +}
> >> +static inline void vcpu_save_fpexc(struct kvm_vcpu *vcpu) {}
> >> #endif
> >>
> >> #endif /* __ARM_KVM_EMULATE_H__ */
> >> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> >> index 52b777b..ddae814 100644
> >> --- a/arch/arm64/include/asm/kvm_asm.h
> >> +++ b/arch/arm64/include/asm/kvm_asm.h
> >> @@ -48,6 +48,11 @@ extern u64 __vgic_v3_get_ich_vtr_el2(void);
> >>
> >> extern u32 __kvm_get_mdcr_el2(void);
> >>
> >> +extern void __fpsimd_prepare_fpexc32(void);
> >> +extern void __fpsimd_save_fpexc32(struct kvm_vcpu *vcpu);
> >> +extern void __fpsimd_save_state(struct user_fpsimd_state *);
> >> +extern void __fpsimd_restore_state(struct user_fpsimd_state *);
> >> +
> >> #endif
> >>
> >> #endif /* __ARM_KVM_ASM_H__ */
> >> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> >> index ffe8ccf..f8203c7 100644
> >> --- a/arch/arm64/include/asm/kvm_emulate.h
> >> +++ b/arch/arm64/include/asm/kvm_emulate.h
> >> @@ -299,12 +299,34 @@ static inline unsigned long vcpu_data_host_to_guest(struct kvm_vcpu *vcpu,
> >> return data; /* Leave LE untouched */
> >> }
> >>
> >> -static inline void vcpu_trap_vfp_enable(struct kvm_vcpu *vcpu) {}
> >> +static inline bool vcpu_guest_is_32bit(struct kvm_vcpu *vcpu)
> >> +{
> >> + return !(vcpu->arch.hcr_el2 & HCR_RW);
> >> +}
> >> +
> >> +static inline void vcpu_trap_vfp_enable(struct kvm_vcpu *vcpu)
> >> +{
> >> + /* For 32 bit guest enable access to fp/simd registers */
> >> + if (vcpu_guest_is_32bit(vcpu))
> >> + vcpu_prepare_fpexc();
> >> +
> >> + vcpu->arch.cptr_el2 = CPTR_EL2_TTA | CPTR_EL2_TFP;
> >> +}
> >> +
> >> static inline void vcpu_restore_host_fpexc(struct kvm_vcpu *vcpu) {}
> >>
> >> static inline bool vcpu_vfp_isdirty(struct kvm_vcpu *vcpu)
> >> {
> >> - return false;
> >> + return !(vcpu->arch.cptr_el2 & CPTR_EL2_TFP);
> >> +}
> >> +
> >> +static inline void vcpu_restore_host_vfp_state(struct kvm_vcpu *vcpu)
> >> +{
> >> + struct kvm_cpu_context *host_ctxt = vcpu->arch.host_cpu_context;
> >> + struct kvm_cpu_context *guest_ctxt = &vcpu->arch.ctxt;
> >> +
> >> + __fpsimd_save_state(&guest_ctxt->gp_regs.fp_regs);
> >> + __fpsimd_restore_state(&host_ctxt->gp_regs.fp_regs);
> >> }
> >>
> >> #endif /* __ARM64_KVM_EMULATE_H__ */
> >> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> >> index bfe4d4e..5d0c256 100644
> >> --- a/arch/arm64/include/asm/kvm_host.h
> >> +++ b/arch/arm64/include/asm/kvm_host.h
> >> @@ -26,6 +26,7 @@
> >> #include <linux/kvm_types.h>
> >> #include <asm/kvm.h>
> >> #include <asm/kvm_mmio.h>
> >> +#include <asm/kvm_asm.h>
> >>
> >> #define __KVM_HAVE_ARCH_INTC_INITIALIZED
> >>
> >> @@ -180,6 +181,7 @@ struct kvm_vcpu_arch {
> >> /* HYP configuration */
> >> u64 hcr_el2;
> >> u32 mdcr_el2;
> >> + u32 cptr_el2;
> >>
> >> /* Exception Information */
> >> struct kvm_vcpu_fault_info fault;
> >> @@ -338,7 +340,15 @@ static inline void kvm_arch_sync_events(struct kvm *kvm) {}
> >> static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
> >> static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
> >>
> >> -static inline void vcpu_restore_host_vfp_state(struct kvm_vcpu *vcpu) {}
> >> +static inline void vcpu_prepare_fpexc(void)
> >> +{
> >> + kvm_call_hyp(__fpsimd_prepare_fpexc32);
> >> +}
> >> +
> >> +static inline void vcpu_save_fpexc(struct kvm_vcpu *vcpu)
> >> +{
> >> + kvm_call_hyp(__fpsimd_save_fpexc32, vcpu);
> >> +}
> >>
> >> void kvm_arm_init_debug(void);
> >> void kvm_arm_setup_debug(struct kvm_vcpu *vcpu);
> >> diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S
> >> index 93e8d983..a9235e2 100644
> >> --- a/arch/arm64/kvm/hyp/hyp-entry.S
> >> +++ b/arch/arm64/kvm/hyp/hyp-entry.S
> >> @@ -164,6 +164,32 @@ ENTRY(__hyp_do_panic)
> >> eret
> >> ENDPROC(__hyp_do_panic)
> >>
> >> +/**
> >> + * void __fpsimd_prepare_fpexc32(void) -
> >> + * We may be entering the guest and set CPTR_EL2.TFP to trap all floating
> >> + * point register accesses to EL2, however, the ARM manual clearly states
> >> + * that traps are only taken to EL2 if the operation would not otherwise
> >> + * trap to EL1. Therefore, always make sure that for 32-bit guests,
> >> + * we set FPEXC.EN to prevent traps to EL1, when setting the TFP bit.
> >> + */
> >> +ENTRY(__fpsimd_prepare_fpexc32)
> >> + mov x2, #(1 << 30)
> >> + msr fpexc32_el2, x2
> >> + ret
> >> +ENDPROC(__fpsimd_prepare_fpexc32)
> >
> > Why is this code in assembly?
> >
> > It feels like you should be able to stick this in switch.c or something?
> Yeah you're right just reusing code and not integrating into the new structure.
> >
> > Also, it's strange to review this patch as duplicating an entire comment
> > from elsewhere and expecting it to get removed in the next patch, but
> > ok...
>
> Not sure what to do here, the comment is required. Maybe this is a
> one off you could overlook :)
It is related to how the patches are split, but yes, I'll overlook it.
Assuming the patches are bisectable, I'll live with it.
> >
> >> +
> >> +/**
> >> + * void __fpsimd_save_fpexc32(void) -
> >> + * This function restores guest FPEXC to its vcpu context, we call this
> >> + * function from vcpu_put.
> >> + */
> >> +ENTRY(__fpsimd_save_fpexc32)
> >> + kern_hyp_va x0
> >
> > the C prototype for this function indicates you don't take any
> > parameters, but you seem to rely on the vcpu pointer being in x0 for
> > this?
>
> Yes right again should have spotted it.
Thanks,
-Christoffer
More information about the linux-arm-kernel
mailing list