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