[PATCH 0/5] Lazy fpu, cr0.ts

[PATCH 0/5] Lazy fpu, cr0.ts

[Avi Kivity]

There are currently some inefficiencies in how we virtualize the fpu and cr0:

- we trap changes to cr0.ts unconditionally; however, when the guest fpu
  is loaded, we're not really interested in cr0.ts (it's only needed when
  the host fpu is loaded, to prevent the guest from accessing it)
- we deactivate the fpu on every guest context switch, for no reason at all

Fix these issues by being as lazy as possible: deactivate the fpu at
heavyweight context switch time, and when the fpu is active, give the guest
ownership of cr0.ts.

Joerg, I wasn't able to extend this to svm/npt.  If we switch to the guest
with the host fpu loaded, then we must set cr0.ts and enable cr0 intercepts.
I think this is much more expensive than keeping the guest fpu active at all
times.

Avi Kivity (5):
  KVM: VMX: trace clts and lmsw instructions as cr accesses
  KVM: Replace read accesses of vcpu->arch.cr0 by an accessor
  KVM: VMX: Allow the guest to own some cr0 bits
  KVM: Lazify fpu activation and deactivation
  KVM: VMX: Give the guest ownership of cr0.ts when the fpu is active

 arch/x86/include/asm/kvm_host.h |    3 ++
 arch/x86/kvm/emulate.c          |    6 ++--
 arch/x86/kvm/kvm_cache_regs.h   |   12 +++++++++
 arch/x86/kvm/mmu.c              |    2 +-
 arch/x86/kvm/mmu.h              |    2 +-
 arch/x86/kvm/svm.c              |   46 ++++++++++++++++++++++--------------
 arch/x86/kvm/vmx.c              |   48 ++++++++++++++++++++++++--------------
 arch/x86/kvm/x86.c              |   26 ++++++++++++---------
 8 files changed, 93 insertions(+), 52 deletions(-)

[PATCH 1/5] KVM: VMX: trace clts and lmsw instructions as cr accesses

[Avi Kivity]

clts writes cr0.ts; lmsw writes cr0[0:15] - record that in ftrace.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/vmx.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 2cc9b7e..f6d4298 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2987,6 +2987,7 @@ static int handle_cr(struct kvm_vcpu *vcpu)
 		vmx_fpu_deactivate(vcpu);
 		vcpu->arch.cr0 &= ~X86_CR0_TS;
 		vmcs_writel(CR0_READ_SHADOW, vcpu->arch.cr0);
+		trace_kvm_cr_write(0, vcpu->arch.cr0);
 		vmx_fpu_activate(vcpu);
 		skip_emulated_instruction(vcpu);
 		return 1;
@@ -3006,7 +3007,9 @@ static int handle_cr(struct kvm_vcpu *vcpu)
 		}
 		break;
 	case 3: /* lmsw */
-		kvm_lmsw(vcpu, (exit_qualification >> LMSW_SOURCE_DATA_SHIFT) & 0x0f);
+		val = (exit_qualification >> LMSW_SOURCE_DATA_SHIFT) & 0x0f;
+		trace_kvm_cr_write(0, (vcpu->arch.cr0 & ~0xful) | val);
+		kvm_lmsw(vcpu, val);
 
 		skip_emulated_instruction(vcpu);
 		return 1;
-- 
1.6.5.3

[PATCH 2/5] KVM: Replace read accesses of vcpu->arch.cr0 by an accessor

[Avi Kivity]

Since we'd like to allow the guest to own a few bits of cr0 at times, we need
to know when we access those bits.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/emulate.c        |    6 +++---
 arch/x86/kvm/kvm_cache_regs.h |   10 ++++++++++
 arch/x86/kvm/mmu.c            |    2 +-
 arch/x86/kvm/mmu.h            |    2 +-
 arch/x86/kvm/svm.c            |    9 +++++----
 arch/x86/kvm/vmx.c            |   16 ++++++++--------
 arch/x86/kvm/x86.c            |   20 ++++++++++----------
 7 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 7e8faea..0f89e32 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1515,7 +1515,7 @@ emulate_syscall(struct x86_emulate_ctxt *ctxt)
 
 	/* syscall is not available in real mode */
 	if (c->lock_prefix || ctxt->mode == X86EMUL_MODE_REAL
-		|| !(ctxt->vcpu->arch.cr0 & X86_CR0_PE))
+	    || !kvm_read_cr0_bits(ctxt->vcpu, X86_CR0_PE))
 		return -1;
 
 	setup_syscalls_segments(ctxt, &cs, &ss);
@@ -1569,7 +1569,7 @@ emulate_sysenter(struct x86_emulate_ctxt *ctxt)
 
 	/* inject #GP if in real mode or paging is disabled */
 	if (ctxt->mode == X86EMUL_MODE_REAL ||
-		!(ctxt->vcpu->arch.cr0 & X86_CR0_PE)) {
+	    !kvm_read_cr0_bits(ctxt->vcpu, X86_CR0_PE)) {
 		kvm_inject_gp(ctxt->vcpu, 0);
 		return -1;
 	}
@@ -1635,7 +1635,7 @@ emulate_sysexit(struct x86_emulate_ctxt *ctxt)
 
 	/* inject #GP if in real mode or paging is disabled */
 	if (ctxt->mode == X86EMUL_MODE_REAL
-		|| !(ctxt->vcpu->arch.cr0 & X86_CR0_PE)) {
+	    || !kvm_read_cr0_bits(ctxt->vcpu, X86_CR0_PE)) {
 		kvm_inject_gp(ctxt->vcpu, 0);
 		return -1;
 	}
diff --git a/arch/x86/kvm/kvm_cache_regs.h b/arch/x86/kvm/kvm_cache_regs.h
index 35acc36..f468597 100644
--- a/arch/x86/kvm/kvm_cache_regs.h
+++ b/arch/x86/kvm/kvm_cache_regs.h
@@ -38,6 +38,16 @@ static inline u64 kvm_pdptr_read(struct kvm_vcpu *vcpu, int index)
 	return vcpu->arch.pdptrs[index];
 }
 
+static inline ulong kvm_read_cr0_bits(struct kvm_vcpu *vcpu, ulong mask)
+{
+	return vcpu->arch.cr0 & mask;
+}
+
+static inline ulong kvm_read_cr0(struct kvm_vcpu *vcpu)
+{
+	return kvm_read_cr0_bits(vcpu, ~0UL);
+}
+
 static inline ulong kvm_read_cr4_bits(struct kvm_vcpu *vcpu, ulong mask)
 {
 	if (mask & vcpu->arch.cr4_guest_owned_bits)
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index c43c2ab..1250fa2 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -229,7 +229,7 @@ EXPORT_SYMBOL_GPL(kvm_mmu_set_mask_ptes);
 
 static int is_write_protection(struct kvm_vcpu *vcpu)
 {
-	return vcpu->arch.cr0 & X86_CR0_WP;
+	return kvm_read_cr0_bits(vcpu, X86_CR0_WP);
 }
 
 static int is_cpuid_PSE36(void)
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index 4567d80..9a0f6bf 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -75,7 +75,7 @@ static inline int is_pse(struct kvm_vcpu *vcpu)
 
 static inline int is_paging(struct kvm_vcpu *vcpu)
 {
-	return vcpu->arch.cr0 & X86_CR0_PG;
+	return kvm_read_cr0_bits(vcpu, X86_CR0_PG);
 }
 
 static inline int is_present_gpte(unsigned long pte)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index b373ae6..3fea2b8 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -980,7 +980,7 @@ static void svm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
 	if (npt_enabled)
 		goto set;
 
-	if ((vcpu->arch.cr0 & X86_CR0_TS) && !(cr0 & X86_CR0_TS)) {
+	if (kvm_read_cr0_bits(vcpu, X86_CR0_TS) && !(cr0 & X86_CR0_TS)) {
 		svm->vmcb->control.intercept_exceptions &= ~(1 << NM_VECTOR);
 		vcpu->fpu_active = 1;
 	}
@@ -1244,7 +1244,7 @@ static int ud_interception(struct vcpu_svm *svm)
 static int nm_interception(struct vcpu_svm *svm)
 {
 	svm->vmcb->control.intercept_exceptions &= ~(1 << NM_VECTOR);
-	if (!(svm->vcpu.arch.cr0 & X86_CR0_TS))
+	if (!kvm_read_cr0_bits(&svm->vcpu, X86_CR0_TS))
 		svm->vmcb->save.cr0 &= ~X86_CR0_TS;
 	svm->vcpu.fpu_active = 1;
 
@@ -1743,7 +1743,7 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)
 	hsave->save.gdtr   = vmcb->save.gdtr;
 	hsave->save.idtr   = vmcb->save.idtr;
 	hsave->save.efer   = svm->vcpu.arch.shadow_efer;
-	hsave->save.cr0    = svm->vcpu.arch.cr0;
+	hsave->save.cr0    = kvm_read_cr0(&svm->vcpu);
 	hsave->save.cr4    = svm->vcpu.arch.cr4;
 	hsave->save.rflags = vmcb->save.rflags;
 	hsave->save.rip    = svm->next_rip;
@@ -2387,7 +2387,8 @@ static int handle_exit(struct kvm_vcpu *vcpu)
 
 	if (npt_enabled) {
 		int mmu_reload = 0;
-		if ((vcpu->arch.cr0 ^ svm->vmcb->save.cr0) & X86_CR0_PG) {
+		if ((kvm_read_cr0_bits(vcpu, X86_CR0_PG) ^ svm->vmcb->save.cr0)
+		    & X86_CR0_PG) {
 			svm_set_cr0(vcpu, svm->vmcb->save.cr0);
 			mmu_reload = 1;
 		}
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index f6d4298..4d3bf4d 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -794,7 +794,7 @@ static void vmx_fpu_activate(struct kvm_vcpu *vcpu)
 		return;
 	vcpu->fpu_active = 1;
 	vmcs_clear_bits(GUEST_CR0, X86_CR0_TS);
-	if (vcpu->arch.cr0 & X86_CR0_TS)
+	if (kvm_read_cr0_bits(vcpu, X86_CR0_TS))
 		vmcs_set_bits(GUEST_CR0, X86_CR0_TS);
 	update_exception_bitmap(vcpu);
 }
@@ -1775,7 +1775,7 @@ static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
 
 	vmx_flush_tlb(vcpu);
 	vmcs_writel(GUEST_CR3, guest_cr3);
-	if (vcpu->arch.cr0 & X86_CR0_PE)
+	if (kvm_read_cr0_bits(vcpu, X86_CR0_PE))
 		vmx_fpu_deactivate(vcpu);
 }
 
@@ -1830,7 +1830,7 @@ static void vmx_get_segment(struct kvm_vcpu *vcpu,
 
 static int vmx_get_cpl(struct kvm_vcpu *vcpu)
 {
-	if (!(vcpu->arch.cr0 & X86_CR0_PE)) /* if real mode */
+	if (!kvm_read_cr0_bits(vcpu, X86_CR0_PE)) /* if real mode */
 		return 0;
 
 	if (vmx_get_rflags(vcpu) & X86_EFLAGS_VM) /* if virtual 8086 */
@@ -2085,7 +2085,7 @@ static bool cs_ss_rpl_check(struct kvm_vcpu *vcpu)
 static bool guest_state_valid(struct kvm_vcpu *vcpu)
 {
 	/* real mode guest state checks */
-	if (!(vcpu->arch.cr0 & X86_CR0_PE)) {
+	if (!kvm_read_cr0_bits(vcpu, X86_CR0_PE)) {
 		if (!rmode_segment_valid(vcpu, VCPU_SREG_CS))
 			return false;
 		if (!rmode_segment_valid(vcpu, VCPU_SREG_SS))
@@ -2570,7 +2570,7 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 		vmcs_write16(VIRTUAL_PROCESSOR_ID, vmx->vpid);
 
 	vmx->vcpu.arch.cr0 = X86_CR0_NW | X86_CR0_CD | X86_CR0_ET;
-	vmx_set_cr0(&vmx->vcpu, vmx->vcpu.arch.cr0); /* enter rmode */
+	vmx_set_cr0(&vmx->vcpu, kvm_read_cr0(vcpu)); /* enter rmode */
 	vmx_set_cr4(&vmx->vcpu, 0);
 	vmx_set_efer(&vmx->vcpu, 0);
 	vmx_fpu_activate(&vmx->vcpu);
@@ -2986,8 +2986,8 @@ static int handle_cr(struct kvm_vcpu *vcpu)
 	case 2: /* clts */
 		vmx_fpu_deactivate(vcpu);
 		vcpu->arch.cr0 &= ~X86_CR0_TS;
-		vmcs_writel(CR0_READ_SHADOW, vcpu->arch.cr0);
-		trace_kvm_cr_write(0, vcpu->arch.cr0);
+		vmcs_writel(CR0_READ_SHADOW, kvm_read_cr0(vcpu));
+		trace_kvm_cr_write(0, kvm_read_cr0(vcpu));
 		vmx_fpu_activate(vcpu);
 		skip_emulated_instruction(vcpu);
 		return 1;
@@ -3008,7 +3008,7 @@ static int handle_cr(struct kvm_vcpu *vcpu)
 		break;
 	case 3: /* lmsw */
 		val = (exit_qualification >> LMSW_SOURCE_DATA_SHIFT) & 0x0f;
-		trace_kvm_cr_write(0, (vcpu->arch.cr0 & ~0xful) | val);
+		trace_kvm_cr_write(0, (kvm_read_cr0(vcpu) & ~0xful) | val);
 		kvm_lmsw(vcpu, val);
 
 		skip_emulated_instruction(vcpu);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index fd6e1a5..78c4397 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -429,7 +429,7 @@ void kvm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
 {
 	if (cr0 & CR0_RESERVED_BITS) {
 		printk(KERN_DEBUG "set_cr0: 0x%lx #GP, reserved bits 0x%lx\n",
-		       cr0, vcpu->arch.cr0);
+		       cr0, kvm_read_cr0(vcpu));
 		kvm_inject_gp(vcpu, 0);
 		return;
 	}
@@ -487,7 +487,7 @@ EXPORT_SYMBOL_GPL(kvm_set_cr0);
 
 void kvm_lmsw(struct kvm_vcpu *vcpu, unsigned long msw)
 {
-	kvm_set_cr0(vcpu, (vcpu->arch.cr0 & ~0x0ful) | (msw & 0x0f));
+	kvm_set_cr0(vcpu, kvm_read_cr0_bits(vcpu, ~0x0ful) | (msw & 0x0f));
 }
 EXPORT_SYMBOL_GPL(kvm_lmsw);
 
@@ -3015,7 +3015,7 @@ int emulate_invlpg(struct kvm_vcpu *vcpu, gva_t address)
 
 int emulate_clts(struct kvm_vcpu *vcpu)
 {
-	kvm_x86_ops->set_cr0(vcpu, vcpu->arch.cr0 & ~X86_CR0_TS);
+	kvm_x86_ops->set_cr0(vcpu, kvm_read_cr0_bits(vcpu, ~X86_CR0_TS));
 	return X86EMUL_CONTINUE;
 }
 
@@ -3633,7 +3633,7 @@ unsigned long realmode_get_cr(struct kvm_vcpu *vcpu, int cr)
 
 	switch (cr) {
 	case 0:
-		value = vcpu->arch.cr0;
+		value = kvm_read_cr0(vcpu);
 		break;
 	case 2:
 		value = vcpu->arch.cr2;
@@ -3660,7 +3660,7 @@ void realmode_set_cr(struct kvm_vcpu *vcpu, int cr, unsigned long val,
 {
 	switch (cr) {
 	case 0:
-		kvm_set_cr0(vcpu, mk_cr_64(vcpu->arch.cr0, val));
+		kvm_set_cr0(vcpu, mk_cr_64(kvm_read_cr0(vcpu), val));
 		*rflags = kvm_get_rflags(vcpu);
 		break;
 	case 2:
@@ -4252,7 +4252,7 @@ int kvm_arch_vcpu_ioctl_get_sregs(struct kvm_vcpu *vcpu,
 	sregs->gdt.limit = dt.limit;
 	sregs->gdt.base = dt.base;
 
-	sregs->cr0 = vcpu->arch.cr0;
+	sregs->cr0 = kvm_read_cr0(vcpu);
 	sregs->cr2 = vcpu->arch.cr2;
 	sregs->cr3 = vcpu->arch.cr3;
 	sregs->cr4 = kvm_read_cr4(vcpu);
@@ -4438,7 +4438,7 @@ int kvm_load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector,
 {
 	struct kvm_segment kvm_seg;
 
-	if (is_vm86_segment(vcpu, seg) || !(vcpu->arch.cr0 & X86_CR0_PE))
+	if (is_vm86_segment(vcpu, seg) || !(kvm_read_cr0_bits(vcpu, X86_CR0_PE)))
 		return kvm_load_realmode_segment(vcpu, selector, seg);
 	if (load_segment_descriptor_to_kvm_desct(vcpu, selector, &kvm_seg))
 		return 1;
@@ -4716,7 +4716,7 @@ int kvm_task_switch(struct kvm_vcpu *vcpu, u16 tss_selector, int reason)
 					      &nseg_desc);
 	}
 
-	kvm_x86_ops->set_cr0(vcpu, vcpu->arch.cr0 | X86_CR0_TS);
+	kvm_x86_ops->set_cr0(vcpu, kvm_read_cr0(vcpu) | X86_CR0_TS);
 	seg_desct_to_kvm_desct(&nseg_desc, tss_selector, &tr_seg);
 	tr_seg.type = 11;
 	kvm_set_segment(vcpu, &tr_seg, VCPU_SREG_TR);
@@ -4751,7 +4751,7 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
 	kvm_x86_ops->set_efer(vcpu, sregs->efer);
 	kvm_set_apic_base(vcpu, sregs->apic_base);
 
-	mmu_reset_needed |= vcpu->arch.cr0 != sregs->cr0;
+	mmu_reset_needed |= kvm_read_cr0(vcpu) != sregs->cr0;
 	kvm_x86_ops->set_cr0(vcpu, sregs->cr0);
 	vcpu->arch.cr0 = sregs->cr0;
 
@@ -4790,7 +4790,7 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
 	/* Older userspace won't unhalt the vcpu on reset. */
 	if (kvm_vcpu_is_bsp(vcpu) && kvm_rip_read(vcpu) == 0xfff0 &&
 	    sregs->cs.selector == 0xf000 && sregs->cs.base == 0xffff0000 &&
-	    !(vcpu->arch.cr0 & X86_CR0_PE))
+	    !(kvm_read_cr0_bits(vcpu, X86_CR0_PE)))
 		vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE;
 
 	vcpu_put(vcpu);
-- 
1.6.5.3

[PATCH 3/5] KVM: VMX: Allow the guest to own some cr0 bits

[Avi Kivity]

We will use this later to give the guest ownership of cr0.ts.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |    2 ++
 arch/x86/kvm/kvm_cache_regs.h   |    2 ++
 arch/x86/kvm/svm.c              |    5 +++++
 arch/x86/kvm/vmx.c              |    9 +++++++++
 4 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index fe4df46..627d50b 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -269,6 +269,7 @@ struct kvm_vcpu_arch {
 	u32 regs_dirty;
 
 	unsigned long cr0;
+	unsigned long cr0_guest_owned_bits;
 	unsigned long cr2;
 	unsigned long cr3;
 	unsigned long cr4;
@@ -481,6 +482,7 @@ struct kvm_x86_ops {
 	void (*set_segment)(struct kvm_vcpu *vcpu,
 			    struct kvm_segment *var, int seg);
 	void (*get_cs_db_l_bits)(struct kvm_vcpu *vcpu, int *db, int *l);
+	void (*decache_cr0_guest_bits)(struct kvm_vcpu *vcpu);
 	void (*decache_cr4_guest_bits)(struct kvm_vcpu *vcpu);
 	void (*set_cr0)(struct kvm_vcpu *vcpu, unsigned long cr0);
 	void (*set_cr3)(struct kvm_vcpu *vcpu, unsigned long cr3);
diff --git a/arch/x86/kvm/kvm_cache_regs.h b/arch/x86/kvm/kvm_cache_regs.h
index f468597..6b419a3 100644
--- a/arch/x86/kvm/kvm_cache_regs.h
+++ b/arch/x86/kvm/kvm_cache_regs.h
@@ -40,6 +40,8 @@ static inline u64 kvm_pdptr_read(struct kvm_vcpu *vcpu, int index)
 
 static inline ulong kvm_read_cr0_bits(struct kvm_vcpu *vcpu, ulong mask)
 {
+	if (mask & vcpu->arch.cr0_guest_owned_bits)
+		kvm_x86_ops->decache_cr0_guest_bits(vcpu);
 	return vcpu->arch.cr0 & mask;
 }
 
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 3fea2b8..b2a7155 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -956,6 +956,10 @@ static void svm_set_gdt(struct kvm_vcpu *vcpu, struct descriptor_table *dt)
 	svm->vmcb->save.gdtr.base = dt->base ;
 }
 
+static void svm_decache_cr0_guest_bits(struct kvm_vcpu *vcpu)
+{
+}
+
 static void svm_decache_cr4_guest_bits(struct kvm_vcpu *vcpu)
 {
 }
@@ -2948,6 +2952,7 @@ static struct kvm_x86_ops svm_x86_ops = {
 	.set_segment = svm_set_segment,
 	.get_cpl = svm_get_cpl,
 	.get_cs_db_l_bits = kvm_get_cs_db_l_bits,
+	.decache_cr0_guest_bits = svm_decache_cr0_guest_bits,
 	.decache_cr4_guest_bits = svm_decache_cr4_guest_bits,
 	.set_cr0 = svm_set_cr0,
 	.set_cr3 = svm_set_cr3,
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 4d3bf4d..0baf62f 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1643,6 +1643,14 @@ static void vmx_flush_tlb(struct kvm_vcpu *vcpu)
 		ept_sync_context(construct_eptp(vcpu->arch.mmu.root_hpa));
 }
 
+static void vmx_decache_cr0_guest_bits(struct kvm_vcpu *vcpu)
+{
+	ulong cr0_guest_owned_bits = vcpu->arch.cr0_guest_owned_bits;
+
+	vcpu->arch.cr0 &= ~cr0_guest_owned_bits;
+	vcpu->arch.cr0 |= vmcs_readl(GUEST_CR0) & cr0_guest_owned_bits;
+}
+
 static void vmx_decache_cr4_guest_bits(struct kvm_vcpu *vcpu)
 {
 	ulong cr4_guest_owned_bits = vcpu->arch.cr4_guest_owned_bits;
@@ -4072,6 +4080,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
 	.set_segment = vmx_set_segment,
 	.get_cpl = vmx_get_cpl,
 	.get_cs_db_l_bits = vmx_get_cs_db_l_bits,
+	.decache_cr0_guest_bits = vmx_decache_cr0_guest_bits,
 	.decache_cr4_guest_bits = vmx_decache_cr4_guest_bits,
 	.set_cr0 = vmx_set_cr0,
 	.set_cr3 = vmx_set_cr3,
-- 
1.6.5.3

[PATCH 4/5] KVM: Lazify fpu activation and deactivation

[Avi Kivity]

Defer fpu deactivation as much as possible - if the guest fpu is loaded, keep
it loaded until the next heavyweight exit (where we are forced to unload it).
This reduces unnecessary exits.

We also defer fpu activation on clts; while clts signals the intent to use the
fpu, we can't be sure the guest will actually use it.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/kvm/svm.c              |   34 +++++++++++++++++++---------------
 arch/x86/kvm/vmx.c              |   13 +------------
 arch/x86/kvm/x86.c              |    6 +++++-
 4 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 627d50b..b3b3ce1 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -498,6 +498,7 @@ struct kvm_x86_ops {
 	void (*cache_reg)(struct kvm_vcpu *vcpu, enum kvm_reg reg);
 	unsigned long (*get_rflags)(struct kvm_vcpu *vcpu);
 	void (*set_rflags)(struct kvm_vcpu *vcpu, unsigned long rflags);
+	void (*fpu_deactivate)(struct kvm_vcpu *vcpu);
 
 	void (*tlb_flush)(struct kvm_vcpu *vcpu);
 
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index b2a7155..2a3890f 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -984,17 +984,8 @@ static void svm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
 	if (npt_enabled)
 		goto set;
 
-	if (kvm_read_cr0_bits(vcpu, X86_CR0_TS) && !(cr0 & X86_CR0_TS)) {
-		svm->vmcb->control.intercept_exceptions &= ~(1 << NM_VECTOR);
-		vcpu->fpu_active = 1;
-	}
-
 	vcpu->arch.cr0 = cr0;
 	cr0 |= X86_CR0_PG | X86_CR0_WP;
-	if (!vcpu->fpu_active) {
-		svm->vmcb->control.intercept_exceptions |= (1 << NM_VECTOR);
-		cr0 |= X86_CR0_TS;
-	}
 set:
 	/*
 	 * re-enable caching here because the QEMU bios
@@ -1250,6 +1241,8 @@ static int nm_interception(struct vcpu_svm *svm)
 	svm->vmcb->control.intercept_exceptions &= ~(1 << NM_VECTOR);
 	if (!kvm_read_cr0_bits(&svm->vcpu, X86_CR0_TS))
 		svm->vmcb->save.cr0 &= ~X86_CR0_TS;
+	else
+		svm->vmcb->save.cr0 |= X86_CR0_TS;
 	svm->vcpu.fpu_active = 1;
 
 	return 1;
@@ -2586,6 +2579,8 @@ static void svm_flush_tlb(struct kvm_vcpu *vcpu)
 
 static void svm_prepare_guest_switch(struct kvm_vcpu *vcpu)
 {
+	if (npt_enabled)
+		vcpu->fpu_active = 1;
 }
 
 static inline void sync_cr8_to_lapic(struct kvm_vcpu *vcpu)
@@ -2805,12 +2800,6 @@ static void svm_set_cr3(struct kvm_vcpu *vcpu, unsigned long root)
 
 	svm->vmcb->save.cr3 = root;
 	force_new_asid(vcpu);
-
-	if (vcpu->fpu_active) {
-		svm->vmcb->control.intercept_exceptions |= (1 << NM_VECTOR);
-		svm->vmcb->save.cr0 |= X86_CR0_TS;
-		vcpu->fpu_active = 0;
-	}
 }
 
 static int is_disabled(void)
@@ -2926,6 +2915,20 @@ static bool svm_rdtscp_supported(void)
 	return false;
 }
 
+static void svm_fpu_deactivate(struct kvm_vcpu *vcpu)
+{
+	struct vcpu_svm *svm = to_svm(vcpu);
+
+	if (npt_enabled) {
+		/* hack: npt requires active fpu at this time */
+		vcpu->fpu_active = 1;
+		return;
+	}
+
+	svm->vmcb->control.intercept_exceptions |= 1 << NM_VECTOR;
+	svm->vmcb->save.cr0 |= X86_CR0_TS;
+}
+
 static struct kvm_x86_ops svm_x86_ops = {
 	.cpu_has_kvm_support = has_svm,
 	.disabled_by_bios = is_disabled,
@@ -2967,6 +2970,7 @@ static struct kvm_x86_ops svm_x86_ops = {
 	.cache_reg = svm_cache_reg,
 	.get_rflags = svm_get_rflags,
 	.set_rflags = svm_set_rflags,
+	.fpu_deactivate = svm_fpu_deactivate,
 
 	.tlb_flush = svm_flush_tlb,
 
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 0baf62f..7e0b45e 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -801,9 +801,6 @@ static void vmx_fpu_activate(struct kvm_vcpu *vcpu)
 
 static void vmx_fpu_deactivate(struct kvm_vcpu *vcpu)
 {
-	if (!vcpu->fpu_active)
-		return;
-	vcpu->fpu_active = 0;
 	vmcs_set_bits(GUEST_CR0, X86_CR0_TS);
 	update_exception_bitmap(vcpu);
 }
@@ -1727,8 +1724,6 @@ static void vmx_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
 	else
 		hw_cr0 = (cr0 & ~KVM_GUEST_CR0_MASK) | KVM_VM_CR0_ALWAYS_ON;
 
-	vmx_fpu_deactivate(vcpu);
-
 	if (vmx->rmode.vm86_active && (cr0 & X86_CR0_PE))
 		enter_pmode(vcpu);
 
@@ -1750,9 +1745,6 @@ static void vmx_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
 	vmcs_writel(CR0_READ_SHADOW, cr0);
 	vmcs_writel(GUEST_CR0, hw_cr0);
 	vcpu->arch.cr0 = cr0;
-
-	if (!(cr0 & X86_CR0_TS) || !(cr0 & X86_CR0_PE))
-		vmx_fpu_activate(vcpu);
 }
 
 static u64 construct_eptp(unsigned long root_hpa)
@@ -1783,8 +1775,6 @@ static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
 
 	vmx_flush_tlb(vcpu);
 	vmcs_writel(GUEST_CR3, guest_cr3);
-	if (kvm_read_cr0_bits(vcpu, X86_CR0_PE))
-		vmx_fpu_deactivate(vcpu);
 }
 
 static void vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
@@ -2992,11 +2982,9 @@ static int handle_cr(struct kvm_vcpu *vcpu)
 		};
 		break;
 	case 2: /* clts */
-		vmx_fpu_deactivate(vcpu);
 		vcpu->arch.cr0 &= ~X86_CR0_TS;
 		vmcs_writel(CR0_READ_SHADOW, kvm_read_cr0(vcpu));
 		trace_kvm_cr_write(0, kvm_read_cr0(vcpu));
-		vmx_fpu_activate(vcpu);
 		skip_emulated_instruction(vcpu);
 		return 1;
 	case 1: /*mov from cr*/
@@ -4093,6 +4081,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
 	.cache_reg = vmx_cache_reg,
 	.get_rflags = vmx_get_rflags,
 	.set_rflags = vmx_set_rflags,
+	.fpu_deactivate = vmx_fpu_deactivate,
 
 	.tlb_flush = vmx_flush_tlb,
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 78c4397..28c2020 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1509,8 +1509,8 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 {
-	kvm_x86_ops->vcpu_put(vcpu);
 	kvm_put_guest_fpu(vcpu);
+	kvm_x86_ops->vcpu_put(vcpu);
 }
 
 static int is_efer_nx(void)
@@ -4988,6 +4988,10 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
 		return;
 
 	vcpu->guest_fpu_loaded = 0;
+	if (vcpu->fpu_active) {
+		vcpu->fpu_active = 0;
+		kvm_x86_ops->fpu_deactivate(vcpu);
+	}
 	kvm_fx_save(&vcpu->arch.guest_fx_image);
 	kvm_fx_restore(&vcpu->arch.host_fx_image);
 	++vcpu->stat.fpu_reload;
-- 
1.6.5.3

[PATCH 5/5] KVM: VMX: Give the guest ownership of cr0.ts when the fpu is active

[Avi Kivity]

If the guest fpu is loaded, there is nothing interesing about cr0.ts; let
the guest play with it as it will.  This makes context switches between fpu
intensive guest processes faster, as we won't trap the clts and cr0 write
instructions.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/vmx.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 7e0b45e..81dc432 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -797,12 +797,23 @@ static void vmx_fpu_activate(struct kvm_vcpu *vcpu)
 	if (kvm_read_cr0_bits(vcpu, X86_CR0_TS))
 		vmcs_set_bits(GUEST_CR0, X86_CR0_TS);
 	update_exception_bitmap(vcpu);
+	vcpu->arch.cr0_guest_owned_bits = X86_CR0_TS;
+	vmcs_writel(CR0_GUEST_HOST_MASK, ~vcpu->arch.cr0_guest_owned_bits);
 }
 
 static void vmx_fpu_deactivate(struct kvm_vcpu *vcpu)
 {
+	ulong old_ts, old_cr0;
+
+	old_ts = kvm_read_cr0_bits(vcpu, X86_CR0_TS);
 	vmcs_set_bits(GUEST_CR0, X86_CR0_TS);
 	update_exception_bitmap(vcpu);
+	vcpu->arch.cr0_guest_owned_bits = 0;
+	vmcs_writel(CR0_GUEST_HOST_MASK, ~vcpu->arch.cr0_guest_owned_bits);
+	old_cr0 = vcpu->arch.cr0;
+	vcpu->arch.cr0 = (vcpu->arch.cr0 & ~X86_CR0_TS) | old_ts;
+	if (vcpu->arch.cr0 != old_cr0)
+		vmcs_writel(CR0_READ_SHADOW, vcpu->arch.cr0);
 }
 
 static unsigned long vmx_get_rflags(struct kvm_vcpu *vcpu)
-- 
1.6.5.3

Re: [PATCH 0/5] Lazy fpu, cr0.ts

[Sheng Yang]

On Thursday 31 December 2009 00:25:37 Avi Kivity wrote:
> There are currently some inefficiencies in how we virtualize the fpu and
>  cr0:
> 
> - we trap changes to cr0.ts unconditionally; however, when the guest fpu
>   is loaded, we're not really interested in cr0.ts (it's only needed when
>   the host fpu is loaded, to prevent the guest from accessing it)
> - we deactivate the fpu on every guest context switch, for no reason at all
> 
> Fix these issues by being as lazy as possible: deactivate the fpu at
> heavyweight context switch time, and when the fpu is active, give the guest
> ownership of cr0.ts.

Looks fine to me.

-- 
regards
Yang, Sheng

> 
> Joerg, I wasn't able to extend this to svm/npt.  If we switch to the guest
> with the host fpu loaded, then we must set cr0.ts and enable cr0
>  intercepts. I think this is much more expensive than keeping the guest fpu
>  active at all times.
> 
> Avi Kivity (5):
>   KVM: VMX: trace clts and lmsw instructions as cr accesses
>   KVM: Replace read accesses of vcpu->arch.cr0 by an accessor
>   KVM: VMX: Allow the guest to own some cr0 bits
>   KVM: Lazify fpu activation and deactivation
>   KVM: VMX: Give the guest ownership of cr0.ts when the fpu is active
> 
>  arch/x86/include/asm/kvm_host.h |    3 ++
>  arch/x86/kvm/emulate.c          |    6 ++--
>  arch/x86/kvm/kvm_cache_regs.h   |   12 +++++++++
>  arch/x86/kvm/mmu.c              |    2 +-
>  arch/x86/kvm/mmu.h              |    2 +-
>  arch/x86/kvm/svm.c              |   46
>  ++++++++++++++++++++++-------------- arch/x86/kvm/vmx.c              |  
>  48 ++++++++++++++++++++++++-------------- arch/x86/kvm/x86.c             
>  |   26 ++++++++++++---------
>  8 files changed, 93 insertions(+), 52 deletions(-)
> 

Re: [PATCH 4/5] KVM: Lazify fpu activation and deactivation

[Marcelo Tosatti]

On Wed, Dec 30, 2009 at 06:25:41PM +0200, Avi Kivity wrote:
> Defer fpu deactivation as much as possible - if the guest fpu is loaded, keep
> it loaded until the next heavyweight exit (where we are forced to unload it).
> This reduces unnecessary exits.
> 
> We also defer fpu activation on clts; while clts signals the intent to use the
> fpu, we can't be sure the guest will actually use it.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>

> +static void svm_fpu_deactivate(struct kvm_vcpu *vcpu)
> +{
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +
> +	if (npt_enabled) {
> +		/* hack: npt requires active fpu at this time */
> +		vcpu->fpu_active = 1;
> +		return;
> +	}

Why is that ?

>  void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>  {
> -	kvm_x86_ops->vcpu_put(vcpu);
>  	kvm_put_guest_fpu(vcpu);
> +	kvm_x86_ops->vcpu_put(vcpu);
>  }

It might be possible to defer host FPU restoration to
user-return-notifier/kernel_fpu_begin time, so you'd keep the guest FPU
loaded across qemukvm->kernel task->qemukvm switches. Not sure if its
worthwhile though.

Re: [PATCH 5/5] KVM: VMX: Give the guest ownership of cr0.ts when the fpu is active

[Marcelo Tosatti]

On Wed, Dec 30, 2009 at 06:25:42PM +0200, Avi Kivity wrote:
> If the guest fpu is loaded, there is nothing interesing about cr0.ts; let
> the guest play with it as it will.  This makes context switches between fpu
> intensive guest processes faster, as we won't trap the clts and cr0 write
> instructions.

Looks fine.

Re: [PATCH 4/5] KVM: Lazify fpu activation and deactivation

[Avi Kivity]

On 01/06/2010 02:25 AM, Marcelo Tosatti wrote:
> On Wed, Dec 30, 2009 at 06:25:41PM +0200, Avi Kivity wrote:
>    
>> Defer fpu deactivation as much as possible - if the guest fpu is loaded, keep
>> it loaded until the next heavyweight exit (where we are forced to unload it).
>> This reduces unnecessary exits.
>>
>> We also defer fpu activation on clts; while clts signals the intent to use the
>> fpu, we can't be sure the guest will actually use it.
>>
>> Signed-off-by: Avi Kivity<avi@redhat.com>
>>      
>    
>> +static void svm_fpu_deactivate(struct kvm_vcpu *vcpu)
>> +{
>> +	struct vcpu_svm *svm = to_svm(vcpu);
>> +
>> +	if (npt_enabled) {
>> +		/* hack: npt requires active fpu at this time */
>> +		vcpu->fpu_active = 1;
>> +		return;
>> +	}
>>      
> Why is that ?
>    

A guest context switch will involve setting cr0.ts and possibly issuing 
clts after the fpu is first used:

   _switch_to()
      unlazy_fpu()
         stts()

So we will get an exit on cr0 writes on every guest context switch until 
the fpu is loaded.  vmx avoids this by allowing writes that don't change 
important bits to proceed.

Hmm,  I see the write is conditional, so it may not be as bad as that.  
We'll have to test other guests to make sure they all do conditional stts().

Joerg, what was the reason the initial npt implementation did not do 
lazy fpu switching?

>>   void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>   {
>> -	kvm_x86_ops->vcpu_put(vcpu);
>>   	kvm_put_guest_fpu(vcpu);
>> +	kvm_x86_ops->vcpu_put(vcpu);
>>   }
>>      
> It might be possible to defer host FPU restoration to
> user-return-notifier/kernel_fpu_begin time, so you'd keep the guest FPU
> loaded across qemukvm->kernel task->qemukvm switches. Not sure if its
> worthwhile though.
>    

I have some vague plans to do that, as well as make kernel_fpu_begin() 
preemptable.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [PATCH 4/5] KVM: Lazify fpu activation and deactivation

[Avi Kivity]

On 01/06/2010 05:18 AM, Avi Kivity wrote:
>>> +static void svm_fpu_deactivate(struct kvm_vcpu *vcpu)
>>> +{
>>> +    struct vcpu_svm *svm = to_svm(vcpu);
>>> +
>>> +    if (npt_enabled) {
>>> +        /* hack: npt requires active fpu at this time */
>>> +        vcpu->fpu_active = 1;
>>> +        return;
>>> +    }
>> Why is that ?
>
> A guest context switch will involve setting cr0.ts and possibly 
> issuing clts after the fpu is first used:
>
>   _switch_to()
>      unlazy_fpu()
>         stts()
>
> So we will get an exit on cr0 writes on every guest context switch 
> until the fpu is loaded.  vmx avoids this by allowing writes that 
> don't change important bits to proceed.
>
> Hmm,  I see the write is conditional, so it may not be as bad as 
> that.  We'll have to test other guests to make sure they all do 
> conditional stts().

It can be done.  If we see a cr0 write that doesn't change cr0, we can 
assume it's an stts(), load the guest fpu, and turn on selective cr0 
intercepts.  So the net effect will be:

- guests which only stts() when cr0.ts is clear will behave much like vmx
- guests which stts() unconditionally will see one extra exit and then 
load the fpu, like current behaviour

The only tricky bit is guests with cr0.wp=0 when npt is disabled, we 
have to keep cr0 intercepts there.  But that's fine.

I'll write a patch.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [PATCH 4/5] KVM: Lazify fpu activation and deactivation

[Joerg Roedel]

On Wed, Jan 06, 2010 at 05:18:13AM +0200, Avi Kivity wrote:
> Joerg, what was the reason the initial npt implementation did not do
> lazy fpu switching?

The lazy fpu switching code needed cr3 accesses to be intercepted. With
NPT this was the only reason left to intercept cr3 so I decided to
switch lazy fpu switching off and don't intercept cr3 accesses.

	Joerg

Re: [PATCH 4/5] KVM: Lazify fpu activation and deactivation

[Avi Kivity]

On 01/06/2010 12:47 PM, Joerg Roedel wrote:
> On Wed, Jan 06, 2010 at 05:18:13AM +0200, Avi Kivity wrote:
>    
>> Joerg, what was the reason the initial npt implementation did not do
>> lazy fpu switching?
>>      
> The lazy fpu switching code needed cr3 accesses to be intercepted. With
> NPT this was the only reason left to intercept cr3 so I decided to
> switch lazy fpu switching off and don't intercept cr3 accesses.
>    

Oh.  In fact the interaction of cr3 intercepts with lazy fpu was a 
mistake which this patchset removes; I'll replace cr0 intercepts with 
selective cr0 write intercept in a new patch.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 4/5] KVM: Lazify fpu activation and deactivation

[Avi Kivity]

On 12/30/2009 06:25 PM, Avi Kivity wrote:
> Defer fpu deactivation as much as possible - if the guest fpu is loaded, keep
> it loaded until the next heavyweight exit (where we are forced to unload it).
> This reduces unnecessary exits.
>
> We also defer fpu activation on clts; while clts signals the intent to use the
> fpu, we can't be sure the guest will actually use it.
>
>    

...


> @@ -4988,6 +4988,10 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
>   		return;
>
>   	vcpu->guest_fpu_loaded = 0;
> +	if (vcpu->fpu_active) {
> +		vcpu->fpu_active = 0;
> +		kvm_x86_ops->fpu_deactivate(vcpu);
> +	}
>   	kvm_fx_save(&vcpu->arch.guest_fx_image);
>   	kvm_fx_restore(&vcpu->arch.host_fx_image);
>   	++vcpu->stat.fpu_reload;
>    

This is broken badly; kvm_put_guest_fpu() can be called from preempt 
notifier context, that is during normal execution of vcpu processing.  
Code which modifies the same variables as ->fpu_deactivate() or that 
depends on ->fpu_active will break.

I fixed this by calling ->fpu_deactivate() from a synchronous context 
using vcpu->requests, like we do everywhere else.

Strangely, autotest only caught this on AMD and even it took a while.  
Lucas, can you integrate something like the following into autotest, so 
we exercise the preemption code harder?

#!/usr/bin/python

import sys, os, re, random, ctypes, time

tasks = sys.argv[1:]

threads = [int(t)
            for k in tasks
            for t in os.listdir('/proc/%s/task' % (k,))]

cpus = [int(c[3:])
         for c in os.listdir('/sys/devices/system/cpu')
         if re.match(r'cpu[0-9]+', c)]

rand = random.Random()

sched_setaffinity = ctypes.CDLL('libc.so.6').sched_setaffinity

while True:
     pid = rand.choice(threads)
     cpu = rand.choice(cpus)
     mask = 1 << cpu
     sched_setaffinity(ctypes.c_int(pid), ctypes.c_size_t(4), 
ctypes.byref(ctypes.c_int(mask)))
     try:
         time.sleep(0.01)
     except:
         break


-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 4/5] KVM: Lazify fpu activation and deactivation

[Avi Kivity]

(actually copying Lucas).

On 01/14/2010 03:11 PM, Avi Kivity wrote:
> On 12/30/2009 06:25 PM, Avi Kivity wrote:
>> Defer fpu deactivation as much as possible - if the guest fpu is 
>> loaded, keep
>> it loaded until the next heavyweight exit (where we are forced to 
>> unload it).
>> This reduces unnecessary exits.
>>
>> We also defer fpu activation on clts; while clts signals the intent 
>> to use the
>> fpu, we can't be sure the guest will actually use it.
>>
>
> ...
>
>
>> @@ -4988,6 +4988,10 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
>>           return;
>>
>>       vcpu->guest_fpu_loaded = 0;
>> +    if (vcpu->fpu_active) {
>> +        vcpu->fpu_active = 0;
>> +        kvm_x86_ops->fpu_deactivate(vcpu);
>> +    }
>>       kvm_fx_save(&vcpu->arch.guest_fx_image);
>>       kvm_fx_restore(&vcpu->arch.host_fx_image);
>>       ++vcpu->stat.fpu_reload;
>
> This is broken badly; kvm_put_guest_fpu() can be called from preempt 
> notifier context, that is during normal execution of vcpu processing.  
> Code which modifies the same variables as ->fpu_deactivate() or that 
> depends on ->fpu_active will break.
>
> I fixed this by calling ->fpu_deactivate() from a synchronous context 
> using vcpu->requests, like we do everywhere else.
>
> Strangely, autotest only caught this on AMD and even it took a while.  
> Lucas, can you integrate something like the following into autotest, 
> so we exercise the preemption code harder?
>
> #!/usr/bin/python
>
> import sys, os, re, random, ctypes, time
>
> tasks = sys.argv[1:]
>
> threads = [int(t)
>            for k in tasks
>            for t in os.listdir('/proc/%s/task' % (k,))]
>
> cpus = [int(c[3:])
>         for c in os.listdir('/sys/devices/system/cpu')
>         if re.match(r'cpu[0-9]+', c)]
>
> rand = random.Random()
>
> sched_setaffinity = ctypes.CDLL('libc.so.6').sched_setaffinity
>
> while True:
>     pid = rand.choice(threads)
>     cpu = rand.choice(cpus)
>     mask = 1 << cpu
>     sched_setaffinity(ctypes.c_int(pid), ctypes.c_size_t(4), 
> ctypes.byref(ctypes.c_int(mask)))
>     try:
>         time.sleep(0.01)
>     except:
>         break
>
>


-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 4/5] KVM: Lazify fpu activation and deactivation

[Lucas Meneghel Rodrigues]

On Thu, 2010-01-14 at 15:11 +0200, Avi Kivity wrote:
> > Strangely, autotest only caught this on AMD and even it took a while.  
> > Lucas, can you integrate something like the following into autotest, 
> > so we exercise the preemption code harder?

All right Avi, just added it to our TODO list, thanks!

> > #!/usr/bin/python
> >
> > import sys, os, re, random, ctypes, time
> >
> > tasks = sys.argv[1:]
> >
> > threads = [int(t)
> >            for k in tasks
> >            for t in os.listdir('/proc/%s/task' % (k,))]
> >
> > cpus = [int(c[3:])
> >         for c in os.listdir('/sys/devices/system/cpu')
> >         if re.match(r'cpu[0-9]+', c)]
> >
> > rand = random.Random()
> >
> > sched_setaffinity = ctypes.CDLL('libc.so.6').sched_setaffinity
> >
> > while True:
> >     pid = rand.choice(threads)
> >     cpu = rand.choice(cpus)
> >     mask = 1 << cpu
> >     sched_setaffinity(ctypes.c_int(pid), ctypes.c_size_t(4), 
> > ctypes.byref(ctypes.c_int(mask)))
> >     try:
> >         time.sleep(0.01)
> >     except:
> >         break
> >

Re: [PATCH 4/5] KVM: Lazify fpu activation and deactivation

[Avi Kivity]

On 01/14/2010 04:34 PM, Lucas Meneghel Rodrigues wrote:
> On Thu, 2010-01-14 at 15:11 +0200, Avi Kivity wrote:
>    
>>> Strangely, autotest only caught this on AMD and even it took a while.
>>> Lucas, can you integrate something like the following into autotest,
>>> so we exercise the preemption code harder?
>>>        
> All right Avi, just added it to our TODO list, thanks!
>    

On second thoughts, this will cause a severe performance loss (which may 
also affect test results), due to cpu overcommit and excessive vcpu 
migrations.

How about, instead:

Launch a background process with nthreads == ncpus, bind each thread to 
a cpu, and execute a loop on each thread that sleeps for a random amount 
of time from 1 to 10 milliseconds.  This will exercise the preemption code.

In addition, every second or so, offline and online a random cpu.  This 
will cause tasks to be migrated and exercise the task migration code.

-- 
error compiling committee.c: too many arguments to function