On Mon, Jul 06, 2015 at 10:54:46AM -0700, Mario Smarduch wrote: > On 07/05/2015 12:34 PM, Christoffer Dall wrote: > > On Wed, Jun 24, 2015 at 08:30:27PM -0700, Mario Smarduch wrote: > >> This patch implements the VFP context switch code called from vcpu_put in > >> Host KVM. In addition it implements the logic to skip setting a VFP trap > >> if one > >> is not needed. Also resets the flag if Host KVM switched registers to trap > >> new > >> guest vfp accesses. > >> > >> > >> Signed-off-by: Mario Smarduch <[email protected]> > >> --- > >> arch/arm/kvm/interrupts.S | 49 > >> ++++++++++++++++++++++++++++----------------- > >> 1 file changed, 31 insertions(+), 18 deletions(-) > >> > >> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S > >> index 79caf79..0912edd 100644 > >> --- a/arch/arm/kvm/interrupts.S > >> +++ b/arch/arm/kvm/interrupts.S > >> @@ -96,6 +96,21 @@ ENTRY(__kvm_flush_vm_context) > >> bx lr > >> ENDPROC(__kvm_flush_vm_context) > >> > >> +ENTRY(__kvm_restore_host_vfp_state) > >> + push {r3-r7} > >> + > >> + mov r1, #0 > >> + str r1, [r0, #VCPU_VFP_SAVED] > >> + > >> + add r7, r0, #VCPU_VFP_GUEST > >> + store_vfp_state r7 > >> + add r7, r0, #VCPU_VFP_HOST > >> + ldr r7, [r7] > >> + restore_vfp_state r7 > >> + > >> + pop {r3-r7} > >> + bx lr > >> +ENDPROC(__kvm_restore_host_vfp_state) > > > > it feels a bit strange to introduce this function here when I cannot see > > how it's called. > > > > At the very least, could you provide the C equivalent prototype in a > > comment or specify what the input registers are? E.g. > > Yes again that's on a todo list. > > > > /* > > * void __kvm_restore_host_vfp_state(struct kvm_vcpu *vcpu); > > */ > > > >> > >> /******************************************************************** > >> * Hypervisor world-switch code > >> @@ -131,7 +146,13 @@ ENTRY(__kvm_vcpu_run) > >> > >> @ Trap coprocessor CRx accesses > >> set_hstr vmentry > >> + > >> + ldr r1, [vcpu, #VCPU_VFP_SAVED] > >> + cmp r1, #1 > >> + beq skip_guest_vfp_trap > >> set_hcptr vmentry, (HCPTR_TTA | HCPTR_TCP(10) | HCPTR_TCP(11)) > >> +skip_guest_vfp_trap: > >> + > >> set_hdcr vmentry > >> > >> @ Write configured ID register into MIDR alias > >> @@ -173,18 +194,6 @@ __kvm_vcpu_return: > >> set_hcptr vmexit, (HCPTR_TTA | HCPTR_TCP(10) | HCPTR_TCP(11)) > >> > >> #ifdef CONFIG_VFPv3 > >> - @ Save floating point registers we if let guest use them. > >> - tst r2, #(HCPTR_TCP(10) | HCPTR_TCP(11)) > >> - bne after_vfp_restore > >> - > >> - @ Switch VFP/NEON hardware state to the host's > >> - add r7, vcpu, #VCPU_VFP_GUEST > >> - store_vfp_state r7 > >> - add r7, vcpu, #VCPU_VFP_HOST > >> - ldr r7, [r7] > >> - restore_vfp_state r7 > >> - > >> -after_vfp_restore: > >> @ Restore FPEXC_EN which we clobbered on entry > >> pop {r2} > >> VFPFMXR FPEXC, r2 > >> @@ -363,10 +372,6 @@ hyp_hvc: > >> @ Check syndrome register > >> mrc p15, 4, r1, c5, c2, 0 @ HSR > >> lsr r0, r1, #HSR_EC_SHIFT > >> -#ifdef CONFIG_VFPv3 > >> - cmp r0, #HSR_EC_CP_0_13 > >> - beq switch_to_guest_vfp > >> -#endif > >> cmp r0, #HSR_EC_HVC > >> bne guest_trap @ Not HVC instr. > >> > >> @@ -380,7 +385,10 @@ hyp_hvc: > >> cmp r2, #0 > >> bne guest_trap @ Guest called HVC > >> > >> -host_switch_to_hyp: > >> + /* > >> + * Getting here means host called HVC, we shift parameters and branch > >> + * to Hyp function. > >> + */ > > > > not sure this comment change belongs in this patch (but the comment is > > well-written). > > I built this patch on top of previous one. But IMO this series > is not ready for upstream yet. > > > > >> pop {r0, r1, r2} > >> > >> /* Check for __hyp_get_vectors */ > >> @@ -411,6 +419,10 @@ guest_trap: > >> > >> @ Check if we need the fault information > >> lsr r1, r1, #HSR_EC_SHIFT > >> +#ifdef CONFIG_VFPv3 > >> + cmp r1, #HSR_EC_CP_0_13 > >> + beq switch_to_guest_vfp > >> +#endif > >> cmp r1, #HSR_EC_IABT > >> mrceq p15, 4, r2, c6, c0, 2 @ HIFAR > >> beq 2f > >> @@ -479,11 +491,12 @@ guest_trap: > >> */ > >> #ifdef CONFIG_VFPv3 > >> switch_to_guest_vfp: > >> - load_vcpu @ Load VCPU pointer to r0 > >> push {r3-r7} > >> > >> @ NEON/VFP used. Turn on VFP access. > >> set_hcptr vmexit, (HCPTR_TCP(10) | HCPTR_TCP(11)) > >> + mov r1, #1 > >> + str r1, [vcpu, #VCPU_VFP_SAVED] > >> > >> @ Switch VFP/NEON hardware state to the guest's > >> add r7, r0, #VCPU_VFP_HOST > >> -- > >> 1.7.9.5 > >> > > It would probably be easier to just rebase this on the previous series > > and refer to that in the cover letter, but the approach here looks > > otherwise right to me. > > What if we used the simplified approach (as Marc mentioned) and > let it run for quite a while and then move this series?
Sounds good, let's merge these things separately. -Christoffer _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
