On 02/12/15 11:53, Christoffer Dall wrote: > On Fri, Nov 27, 2015 at 06:50:06PM +0000, Marc Zyngier wrote: >> Implement the fpsimd save restore, keeping the lazy part in >> assembler (as returning to C would be overkill). >> >> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx> >> --- >> arch/arm64/kvm/hyp/Makefile | 1 + >> arch/arm64/kvm/hyp/entry.S | 32 +++++++++++++++++++++++++++++++- >> arch/arm64/kvm/hyp/fpsimd.S | 33 +++++++++++++++++++++++++++++++++ >> arch/arm64/kvm/hyp/hyp.h | 7 +++++++ >> arch/arm64/kvm/hyp/switch.c | 8 ++++++++ >> arch/arm64/kvm/hyp/sysreg-sr.c | 2 +- >> 6 files changed, 81 insertions(+), 2 deletions(-) >> create mode 100644 arch/arm64/kvm/hyp/fpsimd.S >> >> diff --git a/arch/arm64/kvm/hyp/Makefile b/arch/arm64/kvm/hyp/Makefile >> index 9c11b0f..56238d0 100644 >> --- a/arch/arm64/kvm/hyp/Makefile >> +++ b/arch/arm64/kvm/hyp/Makefile >> @@ -9,3 +9,4 @@ obj-$(CONFIG_KVM_ARM_HOST) += sysreg-sr.o >> obj-$(CONFIG_KVM_ARM_HOST) += debug-sr.o >> obj-$(CONFIG_KVM_ARM_HOST) += entry.o >> obj-$(CONFIG_KVM_ARM_HOST) += switch.o >> +obj-$(CONFIG_KVM_ARM_HOST) += fpsimd.o >> diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S >> index 2c4449a..7552922 100644 >> --- a/arch/arm64/kvm/hyp/entry.S >> +++ b/arch/arm64/kvm/hyp/entry.S >> @@ -27,6 +27,7 @@ >> >> #define CPU_GP_REG_OFFSET(x) (CPU_GP_REGS + x) >> #define CPU_XREG_OFFSET(x) CPU_GP_REG_OFFSET(CPU_USER_PT_REGS + 8*x) >> +#define CPU_SYSREG_OFFSET(x) (CPU_SYSREGS + 8*x) >> >> .text >> .pushsection .hyp.text, "ax" >> @@ -152,4 +153,33 @@ ENTRY(__guest_exit) >> ret >> ENDPROC(__guest_exit) >> >> - /* Insert fault handling here */ >> +ENTRY(__fpsimd_guest_restore) >> + push x4, lr >> + >> + mrs x2, cptr_el2 >> + bic x2, x2, #CPTR_EL2_TFP >> + msr cptr_el2, x2 >> + isb >> + >> + mrs x3, tpidr_el2 >> + >> + ldr x0, [x3, #VCPU_HOST_CONTEXT] >> + kern_hyp_va x0 >> + add x0, x0, #CPU_GP_REG_OFFSET(CPU_FP_REGS) >> + bl __fpsimd_save_state >> + >> + add x2, x3, #VCPU_CONTEXT >> + add x0, x2, #CPU_GP_REG_OFFSET(CPU_FP_REGS) >> + bl __fpsimd_restore_state >> + >> + mrs x1, hcr_el2 >> + tbnz x1, #HCR_RW_SHIFT, 1f > > nit: Add a comment along the lines of: > // Skip restoring fpexc32 for AArch64 guests > >> + ldr x4, [x2, #CPU_SYSREG_OFFSET(FPEXC32_EL2)] >> + msr fpexc32_el2, x4 >> +1: >> + pop x4, lr >> + pop x2, x3 >> + pop x0, x1 >> + >> + eret >> +ENDPROC(__fpsimd_guest_restore) >> diff --git a/arch/arm64/kvm/hyp/fpsimd.S b/arch/arm64/kvm/hyp/fpsimd.S >> new file mode 100644 >> index 0000000..da3f22c >> --- /dev/null >> +++ b/arch/arm64/kvm/hyp/fpsimd.S >> @@ -0,0 +1,33 @@ >> +/* >> + * Copyright (C) 2015 - ARM Ltd >> + * Author: Marc Zyngier <marc.zyngier@xxxxxxx> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program. If not, see <http://www.gnu.org/licenses/>. >> + */ >> + >> +#include <linux/linkage.h> >> + >> +#include <asm/fpsimdmacros.h> >> + >> + .text >> + .pushsection .hyp.text, "ax" >> + >> +ENTRY(__fpsimd_save_state) >> + fpsimd_save x0, 1 >> + ret >> +ENDPROC(__fpsimd_save_state) >> + >> +ENTRY(__fpsimd_restore_state) >> + fpsimd_restore x0, 1 >> + ret >> +ENDPROC(__fpsimd_restore_state) >> diff --git a/arch/arm64/kvm/hyp/hyp.h b/arch/arm64/kvm/hyp/hyp.h >> index f0427ee..18365dd 100644 >> --- a/arch/arm64/kvm/hyp/hyp.h >> +++ b/arch/arm64/kvm/hyp/hyp.h >> @@ -66,6 +66,13 @@ void __debug_restore_state(struct kvm_vcpu *vcpu, >> void __debug_cond_save_host_state(struct kvm_vcpu *vcpu); >> void __debug_cond_restore_host_state(struct kvm_vcpu *vcpu); >> >> +void __fpsimd_save_state(struct user_fpsimd_state *fp_regs); >> +void __fpsimd_restore_state(struct user_fpsimd_state *fp_regs); >> +static inline bool __fpsimd_enabled(void) >> +{ >> + return !(read_sysreg(cptr_el2) & CPTR_EL2_TFP); >> +} >> + >> u64 __guest_enter(struct kvm_vcpu *vcpu, struct kvm_cpu_context *host_ctxt); >> >> #endif /* __ARM64_KVM_HYP_H__ */ >> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c >> index d67ed9e..8affc19 100644 >> --- a/arch/arm64/kvm/hyp/switch.c >> +++ b/arch/arm64/kvm/hyp/switch.c >> @@ -88,6 +88,7 @@ int __hyp_text __guest_run(struct kvm_vcpu *vcpu) >> { >> struct kvm_cpu_context *host_ctxt; >> struct kvm_cpu_context *guest_ctxt; >> + bool fp_enabled; >> u64 exit_code; >> >> vcpu = kern_hyp_va(vcpu); >> @@ -117,6 +118,8 @@ int __hyp_text __guest_run(struct kvm_vcpu *vcpu) >> exit_code = __guest_enter(vcpu, host_ctxt); >> /* And we're baaack! */ >> >> + fp_enabled = __fpsimd_enabled(); >> + > > what does 'enabled' really mean here? Isn't it really > __fpsimd_is_dirty() or __fpsimd_is_guest() or something like that (I > suck at naming too). It really means "the FP regs are accessible and won't explode in your face". It doesn't necessary means dirty (though that's the way we use it below. Your call, really. Thanks, M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html