On Mon, 25 Jun 2012 08:29:14 +0200, Antonios Motakis <a.motakis at virtualopensystems.com> wrote: > Hello, > > I was looking into getting this patch up to date, so I had the chance to > test it on the Fast Model. Unfortunately with your changes included I'm > unable to boot a Linux guest; init fails to start somehow: > > Freeing init memory: 164K > Kernel panic - not syncing: Attempted to kill init! > > I spent some time investigating trying to find the cause, but nothing so > far... I will investigate more, but I thought I'll let you know in case you > have seen this before. KVM version is the v7 patchset and the guest is a > simple busybox system. Quite possibly a bug; I have a version of the patch below which I've been meaning to get back to, unfortunately I've been busy with other things and foolishly chose to update to the latest patches. Here they are, destined to be squashed into one commit. I won't be getting back to this until at least next week, so you're welcome to take them over! Cheers, Rusty. ARM: KVM: lazy save/restore of vfp/neon. From: Rusty Russell <rusty.russell at linaro.org> This is based on the non-lazy save/restore patch by Antonios Motakis <a.motakis at virtualopensystems.com>. In this patch, we use the Hyp Coprocessor Trap Register (HPCTR) to trap VFP/NEON instructions, and switch the FPU state at that point. This has subtleties, including that we save the host state on the stack (so we have to manage that layout carefully so a trap which exits can pop it correctly), and that the FPEXC.EN could be switched off by the host or guest. Note that we trap every time the guest touches FPEXC, which Linux does on every context switch (but it could keep that in a per-cpu variable). Signed-off-by: Rusty Russell <rusty.russell at linaro.org> --- arch/arm/include/asm/kvm_host.h | 12 ++++ arch/arm/kernel/asm-offsets.c | 2 + arch/arm/kvm/interrupts.S | 117 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 1 deletion(-) diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h index 44abdc8..71b92e6 100644 --- a/arch/arm/include/asm/kvm_host.h +++ b/arch/arm/include/asm/kvm_host.h @@ -85,12 +85,24 @@ enum cp15_regs { nr_cp15_regs }; +enum cp10_regs { + FPEXC, /* Floating Point Exception Control Register */ + FPSCR, /* Floating Point Status and Control Register */ + FPINST, /* Common VFP Subarchitecture Registers */ + FPINST2, + nr_cp10_regs +}; + struct kvm_vcpu_arch { struct kvm_vcpu_regs regs; /* System control coprocessor (cp15) */ u32 cp15[nr_cp15_regs]; + /* Floating point registers (VFP and Advanced SIMD/NEON) */ + u32 cp10[nr_cp10_regs]; + u32 cp11[64]; + /* Exception Information */ u32 hsr; /* Hyp Syndrom Register */ u32 hdfar; /* Hyp Data Fault Address Register */ diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c index c8c1b91..5bd1849 100644 --- a/arch/arm/kernel/asm-offsets.c +++ b/arch/arm/kernel/asm-offsets.c @@ -161,6 +161,8 @@ int main(void) DEFINE(VCPU_TID_URW, offsetof(struct kvm_vcpu, arch.cp15[c13_TID_URW])); DEFINE(VCPU_TID_URO, offsetof(struct kvm_vcpu, arch.cp15[c13_TID_URO])); DEFINE(VCPU_TID_PRIV, offsetof(struct kvm_vcpu, arch.cp15[c13_TID_PRIV])); + DEFINE(VCPU_CP10, offsetof(struct kvm_vcpu, arch.cp10)); + DEFINE(VCPU_CP11, offsetof(struct kvm_vcpu, arch.cp11)); DEFINE(VCPU_REGS, offsetof(struct kvm_vcpu, arch.regs)); DEFINE(VCPU_USR_REGS, offsetof(struct kvm_vcpu, arch.regs.usr_regs)); DEFINE(VCPU_SVC_REGS, offsetof(struct kvm_vcpu, arch.regs.svc_regs)); diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S index 0cf4965..43c2ee6 100644 --- a/arch/arm/kvm/interrupts.S +++ b/arch/arm/kvm/interrupts.S @@ -21,6 +21,7 @@ #include <asm/asm-offsets.h> #include <asm/kvm_asm.h> #include <asm/kvm_arm.h> +#include <asm/vfp.h> #define VCPU_USR_REG(_reg_nr) (VCPU_USR_REGS + (_reg_nr * 4)) #define VCPU_USR_SP (VCPU_USR_REG(13)) @@ -236,6 +237,77 @@ ENTRY(__kvm_flush_vm_context) mcr p15, 0, r11, c10, c2, 1 @ NMRR .endm +.macro store_vfp_state vcpu=0, vcpup + mrc p10, 7, r2, cr8, cr0, 0 @ FPEXC + @ Make sure VFP is enabled so we can touch the registers. + orr r3, r2, #FPEXC_EN + mcr p10, 7, r3, cr8, cr0, 0 @ FPEXC + .if \vcpu == 0 + vpush {d0-d15} + .else + add r11, \vcpup, #VCPU_CP11 + vstm r11!, {d0-d15} + .endif + mrc p10, 7, r9, cr7, cr0, 0 @ MVFR0 + and r9, r9, #MVFR0_A_SIMD_MASK + cmp r9, #2 @ Check for 32 registers + .if \vcpu == 0 + vpusheq {d16-d31} + .else + vstmeq r11!, {d16-d31} + .endif + + mrc p10, 7, r3, cr1, cr0, 0 @ FPSCR + tst r2, #FPEXC_EX @ Check for VFP Subarchitecture + beq 1f + mrc p10, 7, r4, cr9, cr0, 0 @ FPINST + tst r2, #FPEXC_FP2V + beq 1f + mrc p10, 7, r5, cr10, cr0, 0 @ FPINST2 + +1: + .if \vcpu == 0 + push {r2-r5} + .else + add r10, \vcpup, #VCPU_CP10 + stm r10, {r2-r5} @ Save FPEXC, FPSCR, FPINST, FPINST2 + .endif +.endm + +/* Assumed FPEXC.EN on. */ +.macro restore_vfp_state vcpu=0, vcpup + .if \vcpu == 0 + pop {r2-r5} + .else + add r10, \vcpup, #VCPU_CP10 + ldm r10, {r2-r5} @ Load FPEXC, FPSCR, FPINST, FPINST2 + .endif + + mcr p10, 7, r3, cr1, cr0, 0 @ FPSCR + tst r2, #FPEXC_EX @ Check for VFP Subarchitecture + beq 1f + mcr p10, 7, r4, cr9, cr0, 0 @ FPINST + tst r2, #FPEXC_FP2V + beq 1f + mcr p10, 7, r5, cr10, cr0, 0 @ FPINST2 + +1: + .if \vcpu == 1 + add r11, \vcpup, #VCPU_CP11 + vldm r11!, {d0-d15} + .endif + mrc p10, 7, r9, cr7, cr0, 0 @ MVFR0 + and r9, r9, #MVFR0_A_SIMD_MASK + cmp r9, #2 @ Check for 32 registers + .if \vcpu == 0 + vpopeq {d16-d31} + vpop {d0-d15} + .else + vldmeq r11!, {d16-d31} + .endif + mcr p10, 7, r2, cr8, cr0, 0 @ FPEXC (last, in case !EN) +.endm + /* Configures the HSTR (Hyp System Trap Register) on entry/return * (hardware reset value is 0) */ .macro set_hstr entry @@ -298,6 +370,11 @@ ENTRY(__kvm_vcpu_run) @ Trap coprocessor CRx for all x except 2 and 14 set_hstr 1 + @ Trap floating point accesses so we can lazy restore. + mrc p15, 4, r1, c1, c1, 2 + orr r1, r1, #((1 << 10) | (1 << 11)) @ Trap cp10 and cp11 + mcr p15, 4, r1, c1, c1, 2 + @ Write standard A-9 CPU id in MIDR ldr r1, [r0, #VCPU_MIDR] mcr p15, 4, r1, c0, c0, 0 @@ -345,6 +422,22 @@ __kvm_vcpu_return: @ Don't trap coprocessor accesses for host kernel set_hstr 0 + @ Save floating point registers we if let guest use them. + mrc p15, 4, r2, c1, c1, 2 + tst r2, #((1 << 10) | (1 << 11)) + beq switch_to_host_vfp + + @ Don't trap VFP accesses for host kernel. + bic r2, r2, #((1 << 10) | (1 << 11)) + mcr p15, 4, r2, c1, c1, 2 + b after_vfp_restore + +switch_to_host_vfp: + @ Switch VFP/NEON hardware state to the host's + store_vfp_state 1, r1 + restore_vfp_state + +after_vfp_restore: @ Reset Hyp-role configure_hyp_role 0, r1 @@ -532,8 +625,11 @@ guest_trap: stmia r1, {r3, r4, r5} sub r1, r1, #VCPU_USR_REG(0) - @ Check if we need the fault information lsr r2, r0, #HSR_EC_SHIFT + cmp r2, #HSR_EC_CP_0_13 + beq switch_to_guest_vfp + + @ Check if we need the fault information cmp r2, #HSR_EC_IABT beq 2f cmpne r2, #HSR_EC_DABT @@ -558,6 +654,25 @@ guest_trap: 1: mov r0, #ARM_EXCEPTION_HVC b __kvm_vcpu_return +switch_to_guest_vfp: + @ NEON/VFP used. Turn on VFP access. + mrc p15, 4, r2, c1, c1, 2 + bic r2, r2, #((1 << 10) | (1 << 11)) + mcr p15, 4, r2, c1, c1, 2 + + @ Push host vfp state onto stack, restore guest from VCPU ptr. + @ On a normal exit, we will pop host VFP state in __kvm_vcpu_return. + store_vfp_state + restore_vfp_state 1, r1 + + @ Last of all, push vcpu # back into the stack. + push {r1} + + @ We just need to restore guest regs, then return to guest. + add r0, r1, #VCPU_USR_REG(0) + ldmia r0, {r0-r12} + eret + .align hyp_irq: push {r0} ARM: KVM: save vfp state in cpu struct. From: Rusty Russell <rusty.russell at linaro.org> As suggested by Alexander Graf, instead of saving guest vfp state on the stack, put it in the vcpu struct. This way we only need to restore it when we do vcpu_put(). Signed-off-by: Rusty Russell <rusty.russell at linaro.org> --- arch/arm/include/asm/kvm_asm.h | 2 arch/arm/include/asm/kvm_host.h | 9 ++ arch/arm/kernel/asm-offsets.c | 7 +- arch/arm/kvm/arm.c | 9 ++ arch/arm/kvm/interrupts.S | 166 ++++++++++++++++++--------------------- 5 files changed, 98 insertions(+), 95 deletions(-) diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h index c7cbe24..0225095 100644 --- a/arch/arm/include/asm/kvm_asm.h +++ b/arch/arm/include/asm/kvm_asm.h @@ -47,6 +47,8 @@ extern void __kvm_tlb_flush_vmid(struct kvm *kvm); extern void __kvm_flush_vm_context(void); +extern void __kvm_restore_host_vfp(struct kvm_vcpu *vcpu); + extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu); #endif diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h index 71b92e6..665531c 100644 --- a/arch/arm/include/asm/kvm_host.h +++ b/arch/arm/include/asm/kvm_host.h @@ -100,8 +100,13 @@ struct kvm_vcpu_arch { u32 cp15[nr_cp15_regs]; /* Floating point registers (VFP and Advanced SIMD/NEON) */ - u32 cp10[nr_cp10_regs]; - u32 cp11[64]; + u32 guest_cp10[nr_cp10_regs]; + u32 guest_cp11[64]; + + /* Saved host vfp state. */ + bool host_vfp_saved; + u32 host_cp10[nr_cp10_regs]; + u32 host_cp11[64]; /* Exception Information */ u32 hsr; /* Hyp Syndrom Register */ diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c index 5bd1849..8e1c7c4 100644 --- a/arch/arm/kernel/asm-offsets.c +++ b/arch/arm/kernel/asm-offsets.c @@ -161,8 +161,11 @@ int main(void) DEFINE(VCPU_TID_URW, offsetof(struct kvm_vcpu, arch.cp15[c13_TID_URW])); DEFINE(VCPU_TID_URO, offsetof(struct kvm_vcpu, arch.cp15[c13_TID_URO])); DEFINE(VCPU_TID_PRIV, offsetof(struct kvm_vcpu, arch.cp15[c13_TID_PRIV])); - DEFINE(VCPU_CP10, offsetof(struct kvm_vcpu, arch.cp10)); - DEFINE(VCPU_CP11, offsetof(struct kvm_vcpu, arch.cp11)); + DEFINE(VCPU_GUEST_CP10, offsetof(struct kvm_vcpu, arch.guest_cp10)); + DEFINE(VCPU_GUEST_CP11, offsetof(struct kvm_vcpu, arch.guest_cp11)); + DEFINE(VCPU_HOST_VFP_SAVED, offsetof(struct kvm_vcpu, arch.host_vfp_saved)); + DEFINE(VCPU_HOST_CP10, offsetof(struct kvm_vcpu, arch.host_cp10)); + DEFINE(VCPU_HOST_CP11, offsetof(struct kvm_vcpu, arch.host_cp11)); DEFINE(VCPU_REGS, offsetof(struct kvm_vcpu, arch.regs)); DEFINE(VCPU_USR_REGS, offsetof(struct kvm_vcpu, arch.regs.usr_regs)); DEFINE(VCPU_SVC_REGS, offsetof(struct kvm_vcpu, arch.regs.svc_regs)); diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c index 1cce7af..1be737e 100644 --- a/arch/arm/kvm/arm.c +++ b/arch/arm/kvm/arm.c @@ -270,6 +270,15 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu) void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) { +#if 0 + printk("kvm_arch_vcpu_put for %i on %i\n", + vcpu->cpu, smp_processor_id()); + /* This gets called if we schedule or are preempted, so restore vfp */ + BUG_ON(vcpu->cpu != smp_processor_id()); + if (vcpu->arch.host_vfp_saved) + __kvm_restore_host_vfp(vcpu); + BUG_ON(vcpu->arch.host_vfp_saved); +#endif } int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S index 43c2ee6..d1f8f27 100644 --- a/arch/arm/kvm/interrupts.S +++ b/arch/arm/kvm/interrupts.S @@ -88,6 +88,75 @@ ENTRY(__kvm_flush_vm_context) hvc #0 @ switch back to svc-mode, see hyp_svc bx lr +/* Clobbers {r2-r5} and {r9-r11} */ +.macro store_vfp_state cp10_off, cp11_off, vcpup + mrc p10, 7, r2, cr8, cr0, 0 @ FPEXC + @ Make sure VFP is enabled so we can touch the registers. + orr r3, r2, #FPEXC_EN + mcr p10, 7, r3, cr8, cr0, 0 @ FPEXC + add r11, \vcpup, \cp11_off + vstm r11!, {d0-d15} + mrc p10, 7, r9, cr7, cr0, 0 @ MVFR0 + and r9, r9, #MVFR0_A_SIMD_MASK + cmp r9, #2 @ Check for 32 registers + vstmeq r11!, {d16-d31} + + mrc p10, 7, r3, cr1, cr0, 0 @ FPSCR + tst r2, #FPEXC_EX @ Check for VFP Subarchitecture + beq 1f + mrc p10, 7, r4, cr9, cr0, 0 @ FPINST + tst r2, #FPEXC_FP2V + beq 1f + mrc p10, 7, r5, cr10, cr0, 0 @ FPINST2 + +1: + add r10, \vcpup, \cp10_off + stm r10, {r2-r5} @ Save FPEXC, FPSCR, FPINST, FPINST2 +.endm + +/* Assumed FPEXC.EN on. */ +/* Clobbers {r2-r5} and {r9-r11} */ +.macro restore_vfp_state cp10_off, cp11_off, vcpup + add r10, \vcpup, \cp10_off + ldm r10, {r2-r5} @ Load FPEXC, FPSCR, FPINST, FPINST2 + + mcr p10, 7, r3, cr1, cr0, 0 @ FPSCR + tst r2, #FPEXC_EX @ Check for VFP Subarchitecture + beq 1f + mcr p10, 7, r4, cr9, cr0, 0 @ FPINST + tst r2, #FPEXC_FP2V + beq 1f + mcr p10, 7, r5, cr10, cr0, 0 @ FPINST2 + +1: + add r11, \vcpup, \cp11_off + vldm r11!, {d0-d15} + mrc p10, 7, r9, cr7, cr0, 0 @ MVFR0 + and r9, r9, #MVFR0_A_SIMD_MASK + cmp r9, #2 @ Check for 32 registers + vldmeq r11!, {d16-d31} + mcr p10, 7, r2, cr8, cr0, 0 @ FPEXC (last, in case !EN) +.endm + +/* + * void __kvm_restore_host_vfp(struct kvm_vcpu *vcpu); + * + * vcpu->arch.host_vfp_saved must be true! Which means FPEXEC is enabled, + * since we turned it on for the guest to use vfp. + */ +ENTRY(__kvm_restore_host_vfp) + push {r4-r5} + push {r9-r11} + @ Save guest state, restore host. + store_vfp_state #VCPU_GUEST_CP10, #VCPU_GUEST_CP11, r0 + restore_vfp_state #VCPU_HOST_CP10, #VCPU_HOST_CP11, r0 + @ Unset vcpu->arch.host_vfp_saved + movs r2, #0 + strb r2, [r0, #VCPU_HOST_VFP_SAVED] + pop {r9-r11} + pop {r4-r5} + bx lr + @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ @ Hypervisor world-switch code @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@ @@ -237,77 +306,6 @@ ENTRY(__kvm_flush_vm_context) mcr p15, 0, r11, c10, c2, 1 @ NMRR .endm -.macro store_vfp_state vcpu=0, vcpup - mrc p10, 7, r2, cr8, cr0, 0 @ FPEXC - @ Make sure VFP is enabled so we can touch the registers. - orr r3, r2, #FPEXC_EN - mcr p10, 7, r3, cr8, cr0, 0 @ FPEXC - .if \vcpu == 0 - vpush {d0-d15} - .else - add r11, \vcpup, #VCPU_CP11 - vstm r11!, {d0-d15} - .endif - mrc p10, 7, r9, cr7, cr0, 0 @ MVFR0 - and r9, r9, #MVFR0_A_SIMD_MASK - cmp r9, #2 @ Check for 32 registers - .if \vcpu == 0 - vpusheq {d16-d31} - .else - vstmeq r11!, {d16-d31} - .endif - - mrc p10, 7, r3, cr1, cr0, 0 @ FPSCR - tst r2, #FPEXC_EX @ Check for VFP Subarchitecture - beq 1f - mrc p10, 7, r4, cr9, cr0, 0 @ FPINST - tst r2, #FPEXC_FP2V - beq 1f - mrc p10, 7, r5, cr10, cr0, 0 @ FPINST2 - -1: - .if \vcpu == 0 - push {r2-r5} - .else - add r10, \vcpup, #VCPU_CP10 - stm r10, {r2-r5} @ Save FPEXC, FPSCR, FPINST, FPINST2 - .endif -.endm - -/* Assumed FPEXC.EN on. */ -.macro restore_vfp_state vcpu=0, vcpup - .if \vcpu == 0 - pop {r2-r5} - .else - add r10, \vcpup, #VCPU_CP10 - ldm r10, {r2-r5} @ Load FPEXC, FPSCR, FPINST, FPINST2 - .endif - - mcr p10, 7, r3, cr1, cr0, 0 @ FPSCR - tst r2, #FPEXC_EX @ Check for VFP Subarchitecture - beq 1f - mcr p10, 7, r4, cr9, cr0, 0 @ FPINST - tst r2, #FPEXC_FP2V - beq 1f - mcr p10, 7, r5, cr10, cr0, 0 @ FPINST2 - -1: - .if \vcpu == 1 - add r11, \vcpup, #VCPU_CP11 - vldm r11!, {d0-d15} - .endif - mrc p10, 7, r9, cr7, cr0, 0 @ MVFR0 - and r9, r9, #MVFR0_A_SIMD_MASK - cmp r9, #2 @ Check for 32 registers - .if \vcpu == 0 - vpopeq {d16-d31} - vpop {d0-d15} - .else - vldmeq r11!, {d16-d31} - .endif - mcr p10, 7, r2, cr8, cr0, 0 @ FPEXC (last, in case !EN) -.endm - /* Configures the HSTR (Hyp System Trap Register) on entry/return * (hardware reset value is 0) */ .macro set_hstr entry @@ -422,22 +420,6 @@ __kvm_vcpu_return: @ Don't trap coprocessor accesses for host kernel set_hstr 0 - @ Save floating point registers we if let guest use them. - mrc p15, 4, r2, c1, c1, 2 - tst r2, #((1 << 10) | (1 << 11)) - beq switch_to_host_vfp - - @ Don't trap VFP accesses for host kernel. - bic r2, r2, #((1 << 10) | (1 << 11)) - mcr p15, 4, r2, c1, c1, 2 - b after_vfp_restore - -switch_to_host_vfp: - @ Switch VFP/NEON hardware state to the host's - store_vfp_state 1, r1 - restore_vfp_state - -after_vfp_restore: @ Reset Hyp-role configure_hyp_role 0, r1 @@ -660,10 +642,12 @@ switch_to_guest_vfp: bic r2, r2, #((1 << 10) | (1 << 11)) mcr p15, 4, r2, c1, c1, 2 - @ Push host vfp state onto stack, restore guest from VCPU ptr. - @ On a normal exit, we will pop host VFP state in __kvm_vcpu_return. - store_vfp_state - restore_vfp_state 1, r1 + @ Save host vfp into host_cp10 and host_cp11, mark as valid. + store_vfp_state #VCPU_HOST_CP10, #VCPU_HOST_CP11, r1 + movs r2, #1 + strb r2, [r1, #VCPU_HOST_VFP_SAVED] + @ Restore gest vfp state. + restore_vfp_state #VCPU_GUEST_CP10, #VCPU_GUEST_CP11, r1 @ Last of all, push vcpu # back into the stack. push {r1}