All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] arm: KVM: VFP lazy switch in KVM Host Mode may save upto 98%
@ 2015-06-25  3:30 ` Mario Smarduch
  0 siblings, 0 replies; 28+ messages in thread
From: Mario Smarduch @ 2015-06-25  3:30 UTC (permalink / raw)
  To: kvmarm, marc.zyngier, christoffer.dall
  Cc: linux-arm-kernel, kvm, pbonzini, catalin.marinas, will.deacon,
	Mario Smarduch

Currently we do a lazy VFP switch in Hyp mode, but once we exit and re-enter hyp
mode we trap again on VFP access. This mode has shown around 30-50% improvement
running hackbench and lmbench.

This patch series extends lazy VFP switch beyond Hyp mode to KVM host mode.

1 - On guest access we switch from host to guest and set a flag accessible to 
    host
2 - On exit to KVM host, VFP state is restored on vcpu_put if flag is marked (1)
3 - Otherwise guest is resumed and continues to use its VFP registers. 
4 - In case of 2 on VM entry we set VFP trap flag to repeat 1.

If guest does not access VFP registers them implemenation remains the same.

Executing hackbench on Fast Models and Exynos arm32 board shows good
results. Considering all exits 2% of the time KVM host lazy vfp switch is 
invoked.

Howeverr this patch set requires more burn in time and testing under various 
loads.

Currently ARM32 is addressed later ARM64.

Mario Smarduch (3):
  define headers and offsets to mange VFP state
  Implement lazy VFP switching outside of Hyp Mode
  Add VFP lazy switch hooks in Host KVM

 arch/arm/include/asm/kvm_asm.h  |    1 +
 arch/arm/include/asm/kvm_host.h |    3 +++
 arch/arm/kernel/asm-offsets.c   |    1 +
 arch/arm/kvm/arm.c              |   15 ++++++++++++
 arch/arm/kvm/interrupts.S       |   49 +++++++++++++++++++++++++--------------
 5 files changed, 51 insertions(+), 18 deletions(-)

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 0/3] arm: KVM: VFP lazy switch in KVM Host Mode may save upto 98%
@ 2015-06-25  3:30 ` Mario Smarduch
  0 siblings, 0 replies; 28+ messages in thread
From: Mario Smarduch @ 2015-06-25  3:30 UTC (permalink / raw)
  To: linux-arm-kernel

Currently we do a lazy VFP switch in Hyp mode, but once we exit and re-enter hyp
mode we trap again on VFP access. This mode has shown around 30-50% improvement
running hackbench and lmbench.

This patch series extends lazy VFP switch beyond Hyp mode to KVM host mode.

1 - On guest access we switch from host to guest and set a flag accessible to 
    host
2 - On exit to KVM host, VFP state is restored on vcpu_put if flag is marked (1)
3 - Otherwise guest is resumed and continues to use its VFP registers. 
4 - In case of 2 on VM entry we set VFP trap flag to repeat 1.

If guest does not access VFP registers them implemenation remains the same.

Executing hackbench on Fast Models and Exynos arm32 board shows good
results. Considering all exits 2% of the time KVM host lazy vfp switch is 
invoked.

Howeverr this patch set requires more burn in time and testing under various 
loads.

Currently ARM32 is addressed later ARM64.

Mario Smarduch (3):
  define headers and offsets to mange VFP state
  Implement lazy VFP switching outside of Hyp Mode
  Add VFP lazy switch hooks in Host KVM

 arch/arm/include/asm/kvm_asm.h  |    1 +
 arch/arm/include/asm/kvm_host.h |    3 +++
 arch/arm/kernel/asm-offsets.c   |    1 +
 arch/arm/kvm/arm.c              |   15 ++++++++++++
 arch/arm/kvm/interrupts.S       |   49 +++++++++++++++++++++++++--------------
 5 files changed, 51 insertions(+), 18 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/3] arm: KVM: define headers and offsets to mange VFP state
  2015-06-25  3:30 ` Mario Smarduch
@ 2015-06-25  3:30   ` Mario Smarduch
  -1 siblings, 0 replies; 28+ messages in thread
From: Mario Smarduch @ 2015-06-25  3:30 UTC (permalink / raw)
  To: kvmarm, marc.zyngier, christoffer.dall
  Cc: kvm, catalin.marinas, will.deacon, pbonzini, linux-arm-kernel

Define the required kvm_vcpu_arch fields, and offsets to manage VFP state. And
declary Hyp interface function to switch VFP registers.


Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
---
 arch/arm/include/asm/kvm_asm.h  |    1 +
 arch/arm/include/asm/kvm_host.h |    3 +++
 arch/arm/kernel/asm-offsets.c   |    1 +
 3 files changed, 5 insertions(+)

diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h
index 25410b2..08dda8c 100644
--- a/arch/arm/include/asm/kvm_asm.h
+++ b/arch/arm/include/asm/kvm_asm.h
@@ -97,6 +97,7 @@ extern char __kvm_hyp_code_end[];
 extern void __kvm_flush_vm_context(void);
 extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
 extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
+extern void __kvm_restore_host_vfp_state(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 d71607c..22cea72 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -111,6 +111,9 @@ struct kvm_vcpu_arch {
 	/* Interrupt related fields */
 	u32 irq_lines;		/* IRQ and FIQ levels */
 
+	/* Track if VFP registers are occupied by Guest while in KVM host mode*/
+	u32 vfp_guest_saved;
+
 	/* Exception Information */
 	struct kvm_vcpu_fault_info fault;
 
diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
index 871b826..35093d0 100644
--- a/arch/arm/kernel/asm-offsets.c
+++ b/arch/arm/kernel/asm-offsets.c
@@ -191,6 +191,7 @@ int main(void)
   DEFINE(VCPU_HPFAR,		offsetof(struct kvm_vcpu, arch.fault.hpfar));
   DEFINE(VCPU_HYP_PC,		offsetof(struct kvm_vcpu, arch.fault.hyp_pc));
   DEFINE(VCPU_VGIC_CPU,		offsetof(struct kvm_vcpu, arch.vgic_cpu));
+  DEFINE(VCPU_VFP_SAVED,	offsetof(struct kvm_vcpu, arch.vfp_guest_saved));
   DEFINE(VGIC_V2_CPU_HCR,	offsetof(struct vgic_cpu, vgic_v2.vgic_hcr));
   DEFINE(VGIC_V2_CPU_VMCR,	offsetof(struct vgic_cpu, vgic_v2.vgic_vmcr));
   DEFINE(VGIC_V2_CPU_MISR,	offsetof(struct vgic_cpu, vgic_v2.vgic_misr));
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 1/3] arm: KVM: define headers and offsets to mange VFP state
@ 2015-06-25  3:30   ` Mario Smarduch
  0 siblings, 0 replies; 28+ messages in thread
From: Mario Smarduch @ 2015-06-25  3:30 UTC (permalink / raw)
  To: linux-arm-kernel

Define the required kvm_vcpu_arch fields, and offsets to manage VFP state. And
declary Hyp interface function to switch VFP registers.


Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
---
 arch/arm/include/asm/kvm_asm.h  |    1 +
 arch/arm/include/asm/kvm_host.h |    3 +++
 arch/arm/kernel/asm-offsets.c   |    1 +
 3 files changed, 5 insertions(+)

diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h
index 25410b2..08dda8c 100644
--- a/arch/arm/include/asm/kvm_asm.h
+++ b/arch/arm/include/asm/kvm_asm.h
@@ -97,6 +97,7 @@ extern char __kvm_hyp_code_end[];
 extern void __kvm_flush_vm_context(void);
 extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
 extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
+extern void __kvm_restore_host_vfp_state(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 d71607c..22cea72 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -111,6 +111,9 @@ struct kvm_vcpu_arch {
 	/* Interrupt related fields */
 	u32 irq_lines;		/* IRQ and FIQ levels */
 
+	/* Track if VFP registers are occupied by Guest while in KVM host mode*/
+	u32 vfp_guest_saved;
+
 	/* Exception Information */
 	struct kvm_vcpu_fault_info fault;
 
diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
index 871b826..35093d0 100644
--- a/arch/arm/kernel/asm-offsets.c
+++ b/arch/arm/kernel/asm-offsets.c
@@ -191,6 +191,7 @@ int main(void)
   DEFINE(VCPU_HPFAR,		offsetof(struct kvm_vcpu, arch.fault.hpfar));
   DEFINE(VCPU_HYP_PC,		offsetof(struct kvm_vcpu, arch.fault.hyp_pc));
   DEFINE(VCPU_VGIC_CPU,		offsetof(struct kvm_vcpu, arch.vgic_cpu));
+  DEFINE(VCPU_VFP_SAVED,	offsetof(struct kvm_vcpu, arch.vfp_guest_saved));
   DEFINE(VGIC_V2_CPU_HCR,	offsetof(struct vgic_cpu, vgic_v2.vgic_hcr));
   DEFINE(VGIC_V2_CPU_VMCR,	offsetof(struct vgic_cpu, vgic_v2.vgic_vmcr));
   DEFINE(VGIC_V2_CPU_MISR,	offsetof(struct vgic_cpu, vgic_v2.vgic_misr));
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 2/3] arm: KVM: Implement lazy VFP switching outside of Hyp Mode
  2015-06-25  3:30 ` Mario Smarduch
@ 2015-06-25  3:30   ` Mario Smarduch
  -1 siblings, 0 replies; 28+ messages in thread
From: Mario Smarduch @ 2015-06-25  3:30 UTC (permalink / raw)
  To: kvmarm, marc.zyngier, christoffer.dall
  Cc: kvm, catalin.marinas, will.deacon, pbonzini, linux-arm-kernel

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 <m.smarduch@samsung.com>
---
 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)
 
 /********************************************************************
  *  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.
+	 */
 	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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 2/3] arm: KVM: Implement lazy VFP switching outside of Hyp Mode
@ 2015-06-25  3:30   ` Mario Smarduch
  0 siblings, 0 replies; 28+ messages in thread
From: Mario Smarduch @ 2015-06-25  3:30 UTC (permalink / raw)
  To: linux-arm-kernel

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 <m.smarduch@samsung.com>
---
 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)
 
 /********************************************************************
  *  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.
+	 */
 	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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 3/3] arm: KVM: Add VFP lazy switch hooks in Host KVM
  2015-06-25  3:30 ` Mario Smarduch
@ 2015-06-25  3:30   ` Mario Smarduch
  -1 siblings, 0 replies; 28+ messages in thread
From: Mario Smarduch @ 2015-06-25  3:30 UTC (permalink / raw)
  To: kvmarm, marc.zyngier, christoffer.dall
  Cc: kvm, catalin.marinas, will.deacon, pbonzini, linux-arm-kernel

This patch implements host KVM interface to Hyp mode VFP function to 
switch out guest and switch in host.

Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
---
 arch/arm/kvm/arm.c |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index d9631ec..77b41f5 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -105,6 +105,17 @@ void kvm_arch_check_processor_compat(void *rtn)
 	*(int *)rtn = 0;
 }
 
+/**
+ * kvm_switch_vp_regs() - switch guest/host VFP registers
+ * @vcpu:	pointer to vcpu structure.
+ *
+ * HYP interface functions to save guest and restore host VFP registers
+ */
+static void kvm_switch_fp_regs(struct kvm_vcpu *vcpu)
+{
+	if (vcpu->arch.vfp_guest_saved == 1)
+		kvm_call_hyp(__kvm_restore_host_vfp_state, vcpu);
+}
 
 /**
  * kvm_arch_init_vm - initializes a VM data structure
@@ -292,6 +303,10 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 {
+
+	/* Check if Guest accessed VFP registers */
+	kvm_switch_fp_regs(vcpu);
+
 	/*
 	 * The arch-generic KVM code expects the cpu field of a vcpu to be -1
 	 * if the vcpu is no longer assigned to a cpu.  This is used for the
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 3/3] arm: KVM: Add VFP lazy switch hooks in Host KVM
@ 2015-06-25  3:30   ` Mario Smarduch
  0 siblings, 0 replies; 28+ messages in thread
From: Mario Smarduch @ 2015-06-25  3:30 UTC (permalink / raw)
  To: linux-arm-kernel

This patch implements host KVM interface to Hyp mode VFP function to 
switch out guest and switch in host.

Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
---
 arch/arm/kvm/arm.c |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index d9631ec..77b41f5 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -105,6 +105,17 @@ void kvm_arch_check_processor_compat(void *rtn)
 	*(int *)rtn = 0;
 }
 
+/**
+ * kvm_switch_vp_regs() - switch guest/host VFP registers
+ * @vcpu:	pointer to vcpu structure.
+ *
+ * HYP interface functions to save guest and restore host VFP registers
+ */
+static void kvm_switch_fp_regs(struct kvm_vcpu *vcpu)
+{
+	if (vcpu->arch.vfp_guest_saved == 1)
+		kvm_call_hyp(__kvm_restore_host_vfp_state, vcpu);
+}
 
 /**
  * kvm_arch_init_vm - initializes a VM data structure
@@ -292,6 +303,10 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 {
+
+	/* Check if Guest accessed VFP registers */
+	kvm_switch_fp_regs(vcpu);
+
 	/*
 	 * The arch-generic KVM code expects the cpu field of a vcpu to be -1
 	 * if the vcpu is no longer assigned to a cpu.  This is used for the
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 0/3] arm: KVM: VFP lazy switch in KVM Host Mode may save upto 98%
  2015-06-25  3:30 ` Mario Smarduch
@ 2015-06-28 17:57   ` Mario Smarduch
  -1 siblings, 0 replies; 28+ messages in thread
From: Mario Smarduch @ 2015-06-28 17:57 UTC (permalink / raw)
  To: kvmarm, marc.zyngier, christoffer.dall
  Cc: pbonzini, will.deacon, kvm, linux-arm-kernel, catalin.marinas

Hi Marc, Christoffer -

to clarify - this series may be causing a conflict with the arm64
basic approach, and arm32 exit code touch ups.

The intent for this series  is more of an RFC or preview, to get some
feedback - if this approach is sensible (I guess later applied to
arm64 as well if it is).

Thanks,
- Mario


On 06/24/2015 08:30 PM, Mario Smarduch wrote:
> Currently we do a lazy VFP switch in Hyp mode, but once we exit and re-enter hyp
> mode we trap again on VFP access. This mode has shown around 30-50% improvement
> running hackbench and lmbench.
> 
> This patch series extends lazy VFP switch beyond Hyp mode to KVM host mode.
> 
> 1 - On guest access we switch from host to guest and set a flag accessible to 
>     host
> 2 - On exit to KVM host, VFP state is restored on vcpu_put if flag is marked (1)
> 3 - Otherwise guest is resumed and continues to use its VFP registers. 
> 4 - In case of 2 on VM entry we set VFP trap flag to repeat 1.
> 
> If guest does not access VFP registers them implemenation remains the same.
> 
> Executing hackbench on Fast Models and Exynos arm32 board shows good
> results. Considering all exits 2% of the time KVM host lazy vfp switch is 
> invoked.
> 
> Howeverr this patch set requires more burn in time and testing under various 
> loads.
> 
> Currently ARM32 is addressed later ARM64.
> 
> Mario Smarduch (3):
>   define headers and offsets to mange VFP state
>   Implement lazy VFP switching outside of Hyp Mode
>   Add VFP lazy switch hooks in Host KVM
> 
>  arch/arm/include/asm/kvm_asm.h  |    1 +
>  arch/arm/include/asm/kvm_host.h |    3 +++
>  arch/arm/kernel/asm-offsets.c   |    1 +
>  arch/arm/kvm/arm.c              |   15 ++++++++++++
>  arch/arm/kvm/interrupts.S       |   49 +++++++++++++++++++++++++--------------
>  5 files changed, 51 insertions(+), 18 deletions(-)
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 0/3] arm: KVM: VFP lazy switch in KVM Host Mode may save upto 98%
@ 2015-06-28 17:57   ` Mario Smarduch
  0 siblings, 0 replies; 28+ messages in thread
From: Mario Smarduch @ 2015-06-28 17:57 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Marc, Christoffer -

to clarify - this series may be causing a conflict with the arm64
basic approach, and arm32 exit code touch ups.

The intent for this series  is more of an RFC or preview, to get some
feedback - if this approach is sensible (I guess later applied to
arm64 as well if it is).

Thanks,
- Mario


On 06/24/2015 08:30 PM, Mario Smarduch wrote:
> Currently we do a lazy VFP switch in Hyp mode, but once we exit and re-enter hyp
> mode we trap again on VFP access. This mode has shown around 30-50% improvement
> running hackbench and lmbench.
> 
> This patch series extends lazy VFP switch beyond Hyp mode to KVM host mode.
> 
> 1 - On guest access we switch from host to guest and set a flag accessible to 
>     host
> 2 - On exit to KVM host, VFP state is restored on vcpu_put if flag is marked (1)
> 3 - Otherwise guest is resumed and continues to use its VFP registers. 
> 4 - In case of 2 on VM entry we set VFP trap flag to repeat 1.
> 
> If guest does not access VFP registers them implemenation remains the same.
> 
> Executing hackbench on Fast Models and Exynos arm32 board shows good
> results. Considering all exits 2% of the time KVM host lazy vfp switch is 
> invoked.
> 
> Howeverr this patch set requires more burn in time and testing under various 
> loads.
> 
> Currently ARM32 is addressed later ARM64.
> 
> Mario Smarduch (3):
>   define headers and offsets to mange VFP state
>   Implement lazy VFP switching outside of Hyp Mode
>   Add VFP lazy switch hooks in Host KVM
> 
>  arch/arm/include/asm/kvm_asm.h  |    1 +
>  arch/arm/include/asm/kvm_host.h |    3 +++
>  arch/arm/kernel/asm-offsets.c   |    1 +
>  arch/arm/kvm/arm.c              |   15 ++++++++++++
>  arch/arm/kvm/interrupts.S       |   49 +++++++++++++++++++++++++--------------
>  5 files changed, 51 insertions(+), 18 deletions(-)
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/3] arm: KVM: define headers and offsets to mange VFP state
  2015-06-25  3:30   ` Mario Smarduch
@ 2015-07-05 19:27     ` Christoffer Dall
  -1 siblings, 0 replies; 28+ messages in thread
From: Christoffer Dall @ 2015-07-05 19:27 UTC (permalink / raw)
  To: Mario Smarduch
  Cc: kvm, marc.zyngier, catalin.marinas, will.deacon, pbonzini,
	kvmarm, linux-arm-kernel

On Wed, Jun 24, 2015 at 08:30:26PM -0700, Mario Smarduch wrote:
> Define the required kvm_vcpu_arch fields, and offsets to manage VFP state. And
> declary Hyp interface function to switch VFP registers.
> 
> 
> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
> ---
>  arch/arm/include/asm/kvm_asm.h  |    1 +
>  arch/arm/include/asm/kvm_host.h |    3 +++
>  arch/arm/kernel/asm-offsets.c   |    1 +
>  3 files changed, 5 insertions(+)
> 
> diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h
> index 25410b2..08dda8c 100644
> --- a/arch/arm/include/asm/kvm_asm.h
> +++ b/arch/arm/include/asm/kvm_asm.h
> @@ -97,6 +97,7 @@ extern char __kvm_hyp_code_end[];
>  extern void __kvm_flush_vm_context(void);
>  extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
>  extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
> +extern void __kvm_restore_host_vfp_state(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 d71607c..22cea72 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -111,6 +111,9 @@ struct kvm_vcpu_arch {
>  	/* Interrupt related fields */
>  	u32 irq_lines;		/* IRQ and FIQ levels */
>  
> +	/* Track if VFP registers are occupied by Guest while in KVM host mode*/

why capitalize guest?

missing space at the end of the line.

I don't really understand what the semantics of this field is by just
lookint at this patch.  I would probably define a u32 flags field in
stead and define a patch akin to what we do for the debug registers and
call it 'vfp_dirty', and in a comment for the flag offset define say
something like:

The vfp_dirty flag must be set if the host VFP register state is
modified.

> +	u32 vfp_guest_saved;
> +
>  	/* Exception Information */
>  	struct kvm_vcpu_fault_info fault;
>  
> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
> index 871b826..35093d0 100644
> --- a/arch/arm/kernel/asm-offsets.c
> +++ b/arch/arm/kernel/asm-offsets.c
> @@ -191,6 +191,7 @@ int main(void)
>    DEFINE(VCPU_HPFAR,		offsetof(struct kvm_vcpu, arch.fault.hpfar));
>    DEFINE(VCPU_HYP_PC,		offsetof(struct kvm_vcpu, arch.fault.hyp_pc));
>    DEFINE(VCPU_VGIC_CPU,		offsetof(struct kvm_vcpu, arch.vgic_cpu));
> +  DEFINE(VCPU_VFP_SAVED,	offsetof(struct kvm_vcpu, arch.vfp_guest_saved));
>    DEFINE(VGIC_V2_CPU_HCR,	offsetof(struct vgic_cpu, vgic_v2.vgic_hcr));
>    DEFINE(VGIC_V2_CPU_VMCR,	offsetof(struct vgic_cpu, vgic_v2.vgic_vmcr));
>    DEFINE(VGIC_V2_CPU_MISR,	offsetof(struct vgic_cpu, vgic_v2.vgic_misr));
> -- 
> 1.7.9.5
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/3] arm: KVM: define headers and offsets to mange VFP state
@ 2015-07-05 19:27     ` Christoffer Dall
  0 siblings, 0 replies; 28+ messages in thread
From: Christoffer Dall @ 2015-07-05 19:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jun 24, 2015 at 08:30:26PM -0700, Mario Smarduch wrote:
> Define the required kvm_vcpu_arch fields, and offsets to manage VFP state. And
> declary Hyp interface function to switch VFP registers.
> 
> 
> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
> ---
>  arch/arm/include/asm/kvm_asm.h  |    1 +
>  arch/arm/include/asm/kvm_host.h |    3 +++
>  arch/arm/kernel/asm-offsets.c   |    1 +
>  3 files changed, 5 insertions(+)
> 
> diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h
> index 25410b2..08dda8c 100644
> --- a/arch/arm/include/asm/kvm_asm.h
> +++ b/arch/arm/include/asm/kvm_asm.h
> @@ -97,6 +97,7 @@ extern char __kvm_hyp_code_end[];
>  extern void __kvm_flush_vm_context(void);
>  extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
>  extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
> +extern void __kvm_restore_host_vfp_state(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 d71607c..22cea72 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -111,6 +111,9 @@ struct kvm_vcpu_arch {
>  	/* Interrupt related fields */
>  	u32 irq_lines;		/* IRQ and FIQ levels */
>  
> +	/* Track if VFP registers are occupied by Guest while in KVM host mode*/

why capitalize guest?

missing space at the end of the line.

I don't really understand what the semantics of this field is by just
lookint at this patch.  I would probably define a u32 flags field in
stead and define a patch akin to what we do for the debug registers and
call it 'vfp_dirty', and in a comment for the flag offset define say
something like:

The vfp_dirty flag must be set if the host VFP register state is
modified.

> +	u32 vfp_guest_saved;
> +
>  	/* Exception Information */
>  	struct kvm_vcpu_fault_info fault;
>  
> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
> index 871b826..35093d0 100644
> --- a/arch/arm/kernel/asm-offsets.c
> +++ b/arch/arm/kernel/asm-offsets.c
> @@ -191,6 +191,7 @@ int main(void)
>    DEFINE(VCPU_HPFAR,		offsetof(struct kvm_vcpu, arch.fault.hpfar));
>    DEFINE(VCPU_HYP_PC,		offsetof(struct kvm_vcpu, arch.fault.hyp_pc));
>    DEFINE(VCPU_VGIC_CPU,		offsetof(struct kvm_vcpu, arch.vgic_cpu));
> +  DEFINE(VCPU_VFP_SAVED,	offsetof(struct kvm_vcpu, arch.vfp_guest_saved));
>    DEFINE(VGIC_V2_CPU_HCR,	offsetof(struct vgic_cpu, vgic_v2.vgic_hcr));
>    DEFINE(VGIC_V2_CPU_VMCR,	offsetof(struct vgic_cpu, vgic_v2.vgic_vmcr));
>    DEFINE(VGIC_V2_CPU_MISR,	offsetof(struct vgic_cpu, vgic_v2.vgic_misr));
> -- 
> 1.7.9.5
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/3] arm: KVM: Implement lazy VFP switching outside of Hyp Mode
  2015-06-25  3:30   ` Mario Smarduch
@ 2015-07-05 19:34     ` Christoffer Dall
  -1 siblings, 0 replies; 28+ messages in thread
From: Christoffer Dall @ 2015-07-05 19:34 UTC (permalink / raw)
  To: Mario Smarduch
  Cc: kvmarm, marc.zyngier, linux-arm-kernel, kvm, pbonzini,
	catalin.marinas, will.deacon

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 <m.smarduch@samsung.com>
> ---
>  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.

/*
 * 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).

>  	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.

-Christoffer

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 2/3] arm: KVM: Implement lazy VFP switching outside of Hyp Mode
@ 2015-07-05 19:34     ` Christoffer Dall
  0 siblings, 0 replies; 28+ messages in thread
From: Christoffer Dall @ 2015-07-05 19:34 UTC (permalink / raw)
  To: linux-arm-kernel

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 <m.smarduch@samsung.com>
> ---
>  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.

/*
 * 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).

>  	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.

-Christoffer

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/3] arm: KVM: Add VFP lazy switch hooks in Host KVM
  2015-06-25  3:30   ` Mario Smarduch
@ 2015-07-05 19:37     ` Christoffer Dall
  -1 siblings, 0 replies; 28+ messages in thread
From: Christoffer Dall @ 2015-07-05 19:37 UTC (permalink / raw)
  To: Mario Smarduch
  Cc: kvm, marc.zyngier, catalin.marinas, will.deacon, pbonzini,
	kvmarm, linux-arm-kernel

On Wed, Jun 24, 2015 at 08:30:28PM -0700, Mario Smarduch wrote:
> This patch implements host KVM interface to Hyp mode VFP function to 
> switch out guest and switch in host.
> 
> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
> ---
>  arch/arm/kvm/arm.c |   15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index d9631ec..77b41f5 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -105,6 +105,17 @@ void kvm_arch_check_processor_compat(void *rtn)
>  	*(int *)rtn = 0;
>  }
>  
> +/**
> + * kvm_switch_vp_regs() - switch guest/host VFP registers
> + * @vcpu:	pointer to vcpu structure.
> + *
> + * HYP interface functions to save guest and restore host VFP registers

Not sure I understand what you mean to say with this line, how about:

Calls an assembly routine in HYP mode to actually perform the state
save/restore.

However, why do we actually need to do this in hyp mode?  Can't we just
as well do this in SVC mode or are we changing some trap settings here?

> + */
> +static void kvm_switch_fp_regs(struct kvm_vcpu *vcpu)

should probalby be called kvm_vcpu_put_fp_regs

> +{
> +	if (vcpu->arch.vfp_guest_saved == 1)
> +		kvm_call_hyp(__kvm_restore_host_vfp_state, vcpu);
> +}
>  
>  /**
>   * kvm_arch_init_vm - initializes a VM data structure
> @@ -292,6 +303,10 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>  
>  void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>  {
> +
> +	/* Check if Guest accessed VFP registers */
> +	kvm_switch_fp_regs(vcpu);
> +
>  	/*
>  	 * The arch-generic KVM code expects the cpu field of a vcpu to be -1
>  	 * if the vcpu is no longer assigned to a cpu.  This is used for the
> -- 
> 1.7.9.5

How are we sure that the kernel never touches VFP registers between VCPU
exit and kvm_arch_vcpu_put?  Can a kernel-side memcpy implementation use
the FP regs or something like that?

Thanks,
-Christoffer

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 3/3] arm: KVM: Add VFP lazy switch hooks in Host KVM
@ 2015-07-05 19:37     ` Christoffer Dall
  0 siblings, 0 replies; 28+ messages in thread
From: Christoffer Dall @ 2015-07-05 19:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jun 24, 2015 at 08:30:28PM -0700, Mario Smarduch wrote:
> This patch implements host KVM interface to Hyp mode VFP function to 
> switch out guest and switch in host.
> 
> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
> ---
>  arch/arm/kvm/arm.c |   15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index d9631ec..77b41f5 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -105,6 +105,17 @@ void kvm_arch_check_processor_compat(void *rtn)
>  	*(int *)rtn = 0;
>  }
>  
> +/**
> + * kvm_switch_vp_regs() - switch guest/host VFP registers
> + * @vcpu:	pointer to vcpu structure.
> + *
> + * HYP interface functions to save guest and restore host VFP registers

Not sure I understand what you mean to say with this line, how about:

Calls an assembly routine in HYP mode to actually perform the state
save/restore.

However, why do we actually need to do this in hyp mode?  Can't we just
as well do this in SVC mode or are we changing some trap settings here?

> + */
> +static void kvm_switch_fp_regs(struct kvm_vcpu *vcpu)

should probalby be called kvm_vcpu_put_fp_regs

> +{
> +	if (vcpu->arch.vfp_guest_saved == 1)
> +		kvm_call_hyp(__kvm_restore_host_vfp_state, vcpu);
> +}
>  
>  /**
>   * kvm_arch_init_vm - initializes a VM data structure
> @@ -292,6 +303,10 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>  
>  void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>  {
> +
> +	/* Check if Guest accessed VFP registers */
> +	kvm_switch_fp_regs(vcpu);
> +
>  	/*
>  	 * The arch-generic KVM code expects the cpu field of a vcpu to be -1
>  	 * if the vcpu is no longer assigned to a cpu.  This is used for the
> -- 
> 1.7.9.5

How are we sure that the kernel never touches VFP registers between VCPU
exit and kvm_arch_vcpu_put?  Can a kernel-side memcpy implementation use
the FP regs or something like that?

Thanks,
-Christoffer

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 0/3] arm: KVM: VFP lazy switch in KVM Host Mode may save upto 98%
  2015-06-25  3:30 ` Mario Smarduch
@ 2015-07-05 19:37   ` Christoffer Dall
  -1 siblings, 0 replies; 28+ messages in thread
From: Christoffer Dall @ 2015-07-05 19:37 UTC (permalink / raw)
  To: Mario Smarduch
  Cc: kvmarm, marc.zyngier, linux-arm-kernel, kvm, pbonzini,
	catalin.marinas, will.deacon

Hi Mario,

On Wed, Jun 24, 2015 at 08:30:25PM -0700, Mario Smarduch wrote:
> Currently we do a lazy VFP switch in Hyp mode, but once we exit and re-enter hyp
> mode we trap again on VFP access. This mode has shown around 30-50% improvement
> running hackbench and lmbench.
> 
> This patch series extends lazy VFP switch beyond Hyp mode to KVM host mode.
> 
> 1 - On guest access we switch from host to guest and set a flag accessible to 
>     host
> 2 - On exit to KVM host, VFP state is restored on vcpu_put if flag is marked (1)
> 3 - Otherwise guest is resumed and continues to use its VFP registers. 
> 4 - In case of 2 on VM entry we set VFP trap flag to repeat 1.
> 
> If guest does not access VFP registers them implemenation remains the same.
> 
> Executing hackbench on Fast Models and Exynos arm32 board shows good
> results. Considering all exits 2% of the time KVM host lazy vfp switch is 
> invoked.
> 
> Howeverr this patch set requires more burn in time and testing under various 
> loads.
> 
> Currently ARM32 is addressed later ARM64.
> 
I think Marc said that he experimented with a similar patch once, but
that it caused corruption on the host side somehow.

Am I remembering correctly?  If so, we would need to make sure this
doesn't happen with this patch set...

Otherwise I think this sounds like a fairly good idea and I wonder if
the same could be done on arm64?

Thanks,
-Christoffer

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 0/3] arm: KVM: VFP lazy switch in KVM Host Mode may save upto 98%
@ 2015-07-05 19:37   ` Christoffer Dall
  0 siblings, 0 replies; 28+ messages in thread
From: Christoffer Dall @ 2015-07-05 19:37 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Mario,

On Wed, Jun 24, 2015 at 08:30:25PM -0700, Mario Smarduch wrote:
> Currently we do a lazy VFP switch in Hyp mode, but once we exit and re-enter hyp
> mode we trap again on VFP access. This mode has shown around 30-50% improvement
> running hackbench and lmbench.
> 
> This patch series extends lazy VFP switch beyond Hyp mode to KVM host mode.
> 
> 1 - On guest access we switch from host to guest and set a flag accessible to 
>     host
> 2 - On exit to KVM host, VFP state is restored on vcpu_put if flag is marked (1)
> 3 - Otherwise guest is resumed and continues to use its VFP registers. 
> 4 - In case of 2 on VM entry we set VFP trap flag to repeat 1.
> 
> If guest does not access VFP registers them implemenation remains the same.
> 
> Executing hackbench on Fast Models and Exynos arm32 board shows good
> results. Considering all exits 2% of the time KVM host lazy vfp switch is 
> invoked.
> 
> Howeverr this patch set requires more burn in time and testing under various 
> loads.
> 
> Currently ARM32 is addressed later ARM64.
> 
I think Marc said that he experimented with a similar patch once, but
that it caused corruption on the host side somehow.

Am I remembering correctly?  If so, we would need to make sure this
doesn't happen with this patch set...

Otherwise I think this sounds like a fairly good idea and I wonder if
the same could be done on arm64?

Thanks,
-Christoffer

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/3] arm: KVM: define headers and offsets to mange VFP state
  2015-07-05 19:27     ` Christoffer Dall
@ 2015-07-06 17:50       ` Mario Smarduch
  -1 siblings, 0 replies; 28+ messages in thread
From: Mario Smarduch @ 2015-07-06 17:50 UTC (permalink / raw)
  To: Christoffer Dall
  Cc: kvm, marc.zyngier, catalin.marinas, will.deacon, pbonzini,
	kvmarm, linux-arm-kernel

On 07/05/2015 12:27 PM, Christoffer Dall wrote:
> On Wed, Jun 24, 2015 at 08:30:26PM -0700, Mario Smarduch wrote:
>> Define the required kvm_vcpu_arch fields, and offsets to manage VFP state. And
>> declary Hyp interface function to switch VFP registers.
>>
>>
>> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
>> ---
>>  arch/arm/include/asm/kvm_asm.h  |    1 +
>>  arch/arm/include/asm/kvm_host.h |    3 +++
>>  arch/arm/kernel/asm-offsets.c   |    1 +
>>  3 files changed, 5 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h
>> index 25410b2..08dda8c 100644
>> --- a/arch/arm/include/asm/kvm_asm.h
>> +++ b/arch/arm/include/asm/kvm_asm.h
>> @@ -97,6 +97,7 @@ extern char __kvm_hyp_code_end[];
>>  extern void __kvm_flush_vm_context(void);
>>  extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
>>  extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
>> +extern void __kvm_restore_host_vfp_state(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 d71607c..22cea72 100644
>> --- a/arch/arm/include/asm/kvm_host.h
>> +++ b/arch/arm/include/asm/kvm_host.h
>> @@ -111,6 +111,9 @@ struct kvm_vcpu_arch {
>>  	/* Interrupt related fields */
>>  	u32 irq_lines;		/* IRQ and FIQ levels */
>>  
>> +	/* Track if VFP registers are occupied by Guest while in KVM host mode*/
> 
> why capitalize guest?
I guess just highlight we're holding guest state outside of hyp mode
no issues can change.

> 
> missing space at the end of the line.
> 
> I don't really understand what the semantics of this field is by just
> lookint at this patch.  I would probably define a u32 flags field in
> stead and define a patch akin to what we do for the debug registers and
> call it 'vfp_dirty', and in a comment for the flag offset define say
> something like:
> 
> The vfp_dirty flag must be set if the host VFP register state is
> modified.

Yes agree, this was put together quickly to get comments.
> 
>> +	u32 vfp_guest_saved;
>> +
>>  	/* Exception Information */
>>  	struct kvm_vcpu_fault_info fault;
>>  
>> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
>> index 871b826..35093d0 100644
>> --- a/arch/arm/kernel/asm-offsets.c
>> +++ b/arch/arm/kernel/asm-offsets.c
>> @@ -191,6 +191,7 @@ int main(void)
>>    DEFINE(VCPU_HPFAR,		offsetof(struct kvm_vcpu, arch.fault.hpfar));
>>    DEFINE(VCPU_HYP_PC,		offsetof(struct kvm_vcpu, arch.fault.hyp_pc));
>>    DEFINE(VCPU_VGIC_CPU,		offsetof(struct kvm_vcpu, arch.vgic_cpu));
>> +  DEFINE(VCPU_VFP_SAVED,	offsetof(struct kvm_vcpu, arch.vfp_guest_saved));
>>    DEFINE(VGIC_V2_CPU_HCR,	offsetof(struct vgic_cpu, vgic_v2.vgic_hcr));
>>    DEFINE(VGIC_V2_CPU_VMCR,	offsetof(struct vgic_cpu, vgic_v2.vgic_vmcr));
>>    DEFINE(VGIC_V2_CPU_MISR,	offsetof(struct vgic_cpu, vgic_v2.vgic_misr));
>> -- 
>> 1.7.9.5
>>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/3] arm: KVM: define headers and offsets to mange VFP state
@ 2015-07-06 17:50       ` Mario Smarduch
  0 siblings, 0 replies; 28+ messages in thread
From: Mario Smarduch @ 2015-07-06 17:50 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/05/2015 12:27 PM, Christoffer Dall wrote:
> On Wed, Jun 24, 2015 at 08:30:26PM -0700, Mario Smarduch wrote:
>> Define the required kvm_vcpu_arch fields, and offsets to manage VFP state. And
>> declary Hyp interface function to switch VFP registers.
>>
>>
>> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
>> ---
>>  arch/arm/include/asm/kvm_asm.h  |    1 +
>>  arch/arm/include/asm/kvm_host.h |    3 +++
>>  arch/arm/kernel/asm-offsets.c   |    1 +
>>  3 files changed, 5 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h
>> index 25410b2..08dda8c 100644
>> --- a/arch/arm/include/asm/kvm_asm.h
>> +++ b/arch/arm/include/asm/kvm_asm.h
>> @@ -97,6 +97,7 @@ extern char __kvm_hyp_code_end[];
>>  extern void __kvm_flush_vm_context(void);
>>  extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
>>  extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
>> +extern void __kvm_restore_host_vfp_state(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 d71607c..22cea72 100644
>> --- a/arch/arm/include/asm/kvm_host.h
>> +++ b/arch/arm/include/asm/kvm_host.h
>> @@ -111,6 +111,9 @@ struct kvm_vcpu_arch {
>>  	/* Interrupt related fields */
>>  	u32 irq_lines;		/* IRQ and FIQ levels */
>>  
>> +	/* Track if VFP registers are occupied by Guest while in KVM host mode*/
> 
> why capitalize guest?
I guess just highlight we're holding guest state outside of hyp mode
no issues can change.

> 
> missing space at the end of the line.
> 
> I don't really understand what the semantics of this field is by just
> lookint at this patch.  I would probably define a u32 flags field in
> stead and define a patch akin to what we do for the debug registers and
> call it 'vfp_dirty', and in a comment for the flag offset define say
> something like:
> 
> The vfp_dirty flag must be set if the host VFP register state is
> modified.

Yes agree, this was put together quickly to get comments.
> 
>> +	u32 vfp_guest_saved;
>> +
>>  	/* Exception Information */
>>  	struct kvm_vcpu_fault_info fault;
>>  
>> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
>> index 871b826..35093d0 100644
>> --- a/arch/arm/kernel/asm-offsets.c
>> +++ b/arch/arm/kernel/asm-offsets.c
>> @@ -191,6 +191,7 @@ int main(void)
>>    DEFINE(VCPU_HPFAR,		offsetof(struct kvm_vcpu, arch.fault.hpfar));
>>    DEFINE(VCPU_HYP_PC,		offsetof(struct kvm_vcpu, arch.fault.hyp_pc));
>>    DEFINE(VCPU_VGIC_CPU,		offsetof(struct kvm_vcpu, arch.vgic_cpu));
>> +  DEFINE(VCPU_VFP_SAVED,	offsetof(struct kvm_vcpu, arch.vfp_guest_saved));
>>    DEFINE(VGIC_V2_CPU_HCR,	offsetof(struct vgic_cpu, vgic_v2.vgic_hcr));
>>    DEFINE(VGIC_V2_CPU_VMCR,	offsetof(struct vgic_cpu, vgic_v2.vgic_vmcr));
>>    DEFINE(VGIC_V2_CPU_MISR,	offsetof(struct vgic_cpu, vgic_v2.vgic_misr));
>> -- 
>> 1.7.9.5
>>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/3] arm: KVM: Implement lazy VFP switching outside of Hyp Mode
  2015-07-05 19:34     ` Christoffer Dall
@ 2015-07-06 17:54       ` Mario Smarduch
  -1 siblings, 0 replies; 28+ messages in thread
From: Mario Smarduch @ 2015-07-06 17:54 UTC (permalink / raw)
  To: Christoffer Dall
  Cc: kvm, marc.zyngier, catalin.marinas, will.deacon, pbonzini,
	kvmarm, linux-arm-kernel

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 <m.smarduch@samsung.com>
>> ---
>>  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?
> 
> -Christoffer
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 2/3] arm: KVM: Implement lazy VFP switching outside of Hyp Mode
@ 2015-07-06 17:54       ` Mario Smarduch
  0 siblings, 0 replies; 28+ messages in thread
From: Mario Smarduch @ 2015-07-06 17:54 UTC (permalink / raw)
  To: linux-arm-kernel

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 <m.smarduch@samsung.com>
>> ---
>>  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?
> 
> -Christoffer
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/3] arm: KVM: Add VFP lazy switch hooks in Host KVM
  2015-07-05 19:37     ` Christoffer Dall
@ 2015-07-06 18:35       ` Mario Smarduch
  -1 siblings, 0 replies; 28+ messages in thread
From: Mario Smarduch @ 2015-07-06 18:35 UTC (permalink / raw)
  To: Christoffer Dall
  Cc: kvmarm, marc.zyngier, linux-arm-kernel, kvm, pbonzini,
	catalin.marinas, will.deacon

On 07/05/2015 12:37 PM, Christoffer Dall wrote:
> On Wed, Jun 24, 2015 at 08:30:28PM -0700, Mario Smarduch wrote:
>> This patch implements host KVM interface to Hyp mode VFP function to 
>> switch out guest and switch in host.
>>
>> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
>> ---
>>  arch/arm/kvm/arm.c |   15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>> index d9631ec..77b41f5 100644
>> --- a/arch/arm/kvm/arm.c
>> +++ b/arch/arm/kvm/arm.c
>> @@ -105,6 +105,17 @@ void kvm_arch_check_processor_compat(void *rtn)
>>  	*(int *)rtn = 0;
>>  }
>>  
>> +/**
>> + * kvm_switch_vp_regs() - switch guest/host VFP registers
>> + * @vcpu:	pointer to vcpu structure.
>> + *
>> + * HYP interface functions to save guest and restore host VFP registers
> 
> Not sure I understand what you mean to say with this line, how about:
> 
> Calls an assembly routine in HYP mode to actually perform the state
> save/restore.
> 
> However, why do we actually need to do this in hyp mode?  Can't we just
> as well do this in SVC mode or are we changing some trap settings here?

Yes it should be since non hyp registers are accessed.
I reuse it since all the code was there to do the switch.

> 
>> + */
>> +static void kvm_switch_fp_regs(struct kvm_vcpu *vcpu)
> 
> should probalby be called kvm_vcpu_put_fp_regs
> 
>> +{
>> +	if (vcpu->arch.vfp_guest_saved == 1)
>> +		kvm_call_hyp(__kvm_restore_host_vfp_state, vcpu);
>> +}
>>  
>>  /**
>>   * kvm_arch_init_vm - initializes a VM data structure
>> @@ -292,6 +303,10 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>>  
>>  void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>  {
>> +
>> +	/* Check if Guest accessed VFP registers */
>> +	kvm_switch_fp_regs(vcpu);
>> +
>>  	/*
>>  	 * The arch-generic KVM code expects the cpu field of a vcpu to be -1
>>  	 * if the vcpu is no longer assigned to a cpu.  This is used for the
>> -- 
>> 1.7.9.5
> 
> How are we sure that the kernel never touches VFP registers between VCPU
> exit and kvm_arch_vcpu_put?  Can a kernel-side memcpy implementation use
> the FP regs or something like that?

Exceptions, interrupts - don't save any VFP context, if
these VFP registers are touched by the kernel they should
be saved/restored. The x86 version appears to the same.

> 
> Thanks,
> -Christoffer
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 3/3] arm: KVM: Add VFP lazy switch hooks in Host KVM
@ 2015-07-06 18:35       ` Mario Smarduch
  0 siblings, 0 replies; 28+ messages in thread
From: Mario Smarduch @ 2015-07-06 18:35 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/05/2015 12:37 PM, Christoffer Dall wrote:
> On Wed, Jun 24, 2015 at 08:30:28PM -0700, Mario Smarduch wrote:
>> This patch implements host KVM interface to Hyp mode VFP function to 
>> switch out guest and switch in host.
>>
>> Signed-off-by: Mario Smarduch <m.smarduch@samsung.com>
>> ---
>>  arch/arm/kvm/arm.c |   15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>> index d9631ec..77b41f5 100644
>> --- a/arch/arm/kvm/arm.c
>> +++ b/arch/arm/kvm/arm.c
>> @@ -105,6 +105,17 @@ void kvm_arch_check_processor_compat(void *rtn)
>>  	*(int *)rtn = 0;
>>  }
>>  
>> +/**
>> + * kvm_switch_vp_regs() - switch guest/host VFP registers
>> + * @vcpu:	pointer to vcpu structure.
>> + *
>> + * HYP interface functions to save guest and restore host VFP registers
> 
> Not sure I understand what you mean to say with this line, how about:
> 
> Calls an assembly routine in HYP mode to actually perform the state
> save/restore.
> 
> However, why do we actually need to do this in hyp mode?  Can't we just
> as well do this in SVC mode or are we changing some trap settings here?

Yes it should be since non hyp registers are accessed.
I reuse it since all the code was there to do the switch.

> 
>> + */
>> +static void kvm_switch_fp_regs(struct kvm_vcpu *vcpu)
> 
> should probalby be called kvm_vcpu_put_fp_regs
> 
>> +{
>> +	if (vcpu->arch.vfp_guest_saved == 1)
>> +		kvm_call_hyp(__kvm_restore_host_vfp_state, vcpu);
>> +}
>>  
>>  /**
>>   * kvm_arch_init_vm - initializes a VM data structure
>> @@ -292,6 +303,10 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>>  
>>  void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>  {
>> +
>> +	/* Check if Guest accessed VFP registers */
>> +	kvm_switch_fp_regs(vcpu);
>> +
>>  	/*
>>  	 * The arch-generic KVM code expects the cpu field of a vcpu to be -1
>>  	 * if the vcpu is no longer assigned to a cpu.  This is used for the
>> -- 
>> 1.7.9.5
> 
> How are we sure that the kernel never touches VFP registers between VCPU
> exit and kvm_arch_vcpu_put?  Can a kernel-side memcpy implementation use
> the FP regs or something like that?

Exceptions, interrupts - don't save any VFP context, if
these VFP registers are touched by the kernel they should
be saved/restored. The x86 version appears to the same.

> 
> Thanks,
> -Christoffer
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 0/3] arm: KVM: VFP lazy switch in KVM Host Mode may save upto 98%
  2015-07-05 19:37   ` Christoffer Dall
@ 2015-07-06 18:43     ` Mario Smarduch
  -1 siblings, 0 replies; 28+ messages in thread
From: Mario Smarduch @ 2015-07-06 18:43 UTC (permalink / raw)
  To: Christoffer Dall
  Cc: kvm, marc.zyngier, catalin.marinas, will.deacon, pbonzini,
	kvmarm, linux-arm-kernel

On 07/05/2015 12:37 PM, Christoffer Dall wrote:
> Hi Mario,
> 
> On Wed, Jun 24, 2015 at 08:30:25PM -0700, Mario Smarduch wrote:
>> Currently we do a lazy VFP switch in Hyp mode, but once we exit and re-enter hyp
>> mode we trap again on VFP access. This mode has shown around 30-50% improvement
>> running hackbench and lmbench.
>>
>> This patch series extends lazy VFP switch beyond Hyp mode to KVM host mode.
>>
>> 1 - On guest access we switch from host to guest and set a flag accessible to 
>>     host
>> 2 - On exit to KVM host, VFP state is restored on vcpu_put if flag is marked (1)
>> 3 - Otherwise guest is resumed and continues to use its VFP registers. 
>> 4 - In case of 2 on VM entry we set VFP trap flag to repeat 1.
>>
>> If guest does not access VFP registers them implemenation remains the same.
>>
>> Executing hackbench on Fast Models and Exynos arm32 board shows good
>> results. Considering all exits 2% of the time KVM host lazy vfp switch is 
>> invoked.
>>
>> Howeverr this patch set requires more burn in time and testing under various 
>> loads.
>>
>> Currently ARM32 is addressed later ARM64.
>>
> I think Marc said that he experimented with a similar patch once, but
> that it caused corruption on the host side somehow.
> 
> Am I remembering correctly?  If so, we would need to make sure this
> doesn't happen with this patch set...

I think upstreaming the basic approach first (arm64, arm32 cleanups),
and let this series get some good runtime - would be better
and safer approach.


> 
> Otherwise I think this sounds like a fairly good idea and I wonder if
> the same could be done on arm64?

Yes that's the intent, doing both architectures at once would be preferable.

Thanks,
  Mario
> 
> Thanks,
> -Christoffer
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 0/3] arm: KVM: VFP lazy switch in KVM Host Mode may save upto 98%
@ 2015-07-06 18:43     ` Mario Smarduch
  0 siblings, 0 replies; 28+ messages in thread
From: Mario Smarduch @ 2015-07-06 18:43 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/05/2015 12:37 PM, Christoffer Dall wrote:
> Hi Mario,
> 
> On Wed, Jun 24, 2015 at 08:30:25PM -0700, Mario Smarduch wrote:
>> Currently we do a lazy VFP switch in Hyp mode, but once we exit and re-enter hyp
>> mode we trap again on VFP access. This mode has shown around 30-50% improvement
>> running hackbench and lmbench.
>>
>> This patch series extends lazy VFP switch beyond Hyp mode to KVM host mode.
>>
>> 1 - On guest access we switch from host to guest and set a flag accessible to 
>>     host
>> 2 - On exit to KVM host, VFP state is restored on vcpu_put if flag is marked (1)
>> 3 - Otherwise guest is resumed and continues to use its VFP registers. 
>> 4 - In case of 2 on VM entry we set VFP trap flag to repeat 1.
>>
>> If guest does not access VFP registers them implemenation remains the same.
>>
>> Executing hackbench on Fast Models and Exynos arm32 board shows good
>> results. Considering all exits 2% of the time KVM host lazy vfp switch is 
>> invoked.
>>
>> Howeverr this patch set requires more burn in time and testing under various 
>> loads.
>>
>> Currently ARM32 is addressed later ARM64.
>>
> I think Marc said that he experimented with a similar patch once, but
> that it caused corruption on the host side somehow.
> 
> Am I remembering correctly?  If so, we would need to make sure this
> doesn't happen with this patch set...

I think upstreaming the basic approach first (arm64, arm32 cleanups),
and let this series get some good runtime - would be better
and safer approach.


> 
> Otherwise I think this sounds like a fairly good idea and I wonder if
> the same could be done on arm64?

Yes that's the intent, doing both architectures at once would be preferable.

Thanks,
  Mario
> 
> Thanks,
> -Christoffer
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/3] arm: KVM: Implement lazy VFP switching outside of Hyp Mode
  2015-07-06 17:54       ` Mario Smarduch
@ 2015-07-07 15:27         ` Christoffer Dall
  -1 siblings, 0 replies; 28+ messages in thread
From: Christoffer Dall @ 2015-07-07 15:27 UTC (permalink / raw)
  To: Mario Smarduch
  Cc: kvmarm, marc.zyngier, linux-arm-kernel, kvm, pbonzini,
	catalin.marinas, will.deacon

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 <m.smarduch@samsung.com>
> >> ---
> >>  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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 2/3] arm: KVM: Implement lazy VFP switching outside of Hyp Mode
@ 2015-07-07 15:27         ` Christoffer Dall
  0 siblings, 0 replies; 28+ messages in thread
From: Christoffer Dall @ 2015-07-07 15:27 UTC (permalink / raw)
  To: linux-arm-kernel

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 <m.smarduch@samsung.com>
> >> ---
> >>  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

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2015-07-07 15:27 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-25  3:30 [PATCH 0/3] arm: KVM: VFP lazy switch in KVM Host Mode may save upto 98% Mario Smarduch
2015-06-25  3:30 ` Mario Smarduch
2015-06-25  3:30 ` [PATCH 1/3] arm: KVM: define headers and offsets to mange VFP state Mario Smarduch
2015-06-25  3:30   ` Mario Smarduch
2015-07-05 19:27   ` Christoffer Dall
2015-07-05 19:27     ` Christoffer Dall
2015-07-06 17:50     ` Mario Smarduch
2015-07-06 17:50       ` Mario Smarduch
2015-06-25  3:30 ` [PATCH 2/3] arm: KVM: Implement lazy VFP switching outside of Hyp Mode Mario Smarduch
2015-06-25  3:30   ` Mario Smarduch
2015-07-05 19:34   ` Christoffer Dall
2015-07-05 19:34     ` Christoffer Dall
2015-07-06 17:54     ` Mario Smarduch
2015-07-06 17:54       ` Mario Smarduch
2015-07-07 15:27       ` Christoffer Dall
2015-07-07 15:27         ` Christoffer Dall
2015-06-25  3:30 ` [PATCH 3/3] arm: KVM: Add VFP lazy switch hooks in Host KVM Mario Smarduch
2015-06-25  3:30   ` Mario Smarduch
2015-07-05 19:37   ` Christoffer Dall
2015-07-05 19:37     ` Christoffer Dall
2015-07-06 18:35     ` Mario Smarduch
2015-07-06 18:35       ` Mario Smarduch
2015-06-28 17:57 ` [PATCH 0/3] arm: KVM: VFP lazy switch in KVM Host Mode may save upto 98% Mario Smarduch
2015-06-28 17:57   ` Mario Smarduch
2015-07-05 19:37 ` Christoffer Dall
2015-07-05 19:37   ` Christoffer Dall
2015-07-06 18:43   ` Mario Smarduch
2015-07-06 18:43     ` Mario Smarduch

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.