public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] cr0/cr4/efer/fpu miscellaneous bits
@ 2010-01-21 13:31 Avi Kivity
  2010-01-21 13:31 ` [PATCH 1/8] KVM: Allow kvm_load_guest_fpu() even when !vcpu->fpu_active Avi Kivity
                   ` (8 more replies)
  0 siblings, 9 replies; 23+ messages in thread
From: Avi Kivity @ 2010-01-21 13:31 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: kvm

Mostly trivial cleanups with the exception of a patch activating the fpu
on clts.

Avi Kivity (8):
  KVM: Allow kvm_load_guest_fpu() even when !vcpu->fpu_active
  KVM: Drop kvm_{load,put}_guest_fpu() exports
  KVM: Activate fpu on clts
  KVM: Add a helper for checking if the guest is in protected mode
  KVM: Move cr0/cr4/efer related helpers to x86.h
  KVM: Rename vcpu->shadow_efer to efer
  KVM: Optimize kvm_read_cr[04]_bits()
  KVM: trace guest fpu loads and unloads

 arch/x86/include/asm/kvm_host.h |    3 ++-
 arch/x86/kvm/emulate.c          |   10 ++++------
 arch/x86/kvm/kvm_cache_regs.h   |    9 +++++++--
 arch/x86/kvm/mmu.c              |    3 ++-
 arch/x86/kvm/mmu.h              |   24 ------------------------
 arch/x86/kvm/svm.c              |   20 +++++++++++++-------
 arch/x86/kvm/vmx.c              |   19 ++++++++++---------
 arch/x86/kvm/x86.c              |   31 ++++++++++++++++---------------
 arch/x86/kvm/x86.h              |   30 ++++++++++++++++++++++++++++++
 include/trace/events/kvm.h      |   19 +++++++++++++++++++
 10 files changed, 103 insertions(+), 65 deletions(-)


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

* [PATCH 1/8] KVM: Allow kvm_load_guest_fpu() even when !vcpu->fpu_active
  2010-01-21 13:31 [PATCH 0/8] cr0/cr4/efer/fpu miscellaneous bits Avi Kivity
@ 2010-01-21 13:31 ` Avi Kivity
  2010-01-21 13:31 ` [PATCH 2/8] KVM: Drop kvm_{load,put}_guest_fpu() exports Avi Kivity
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 23+ messages in thread
From: Avi Kivity @ 2010-01-21 13:31 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: kvm

This allows accessing the guest fpu from the instruction emulator, as well as
being symmetric with kvm_put_guest_fpu().

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

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 47c6e23..e3145d5 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4251,7 +4251,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 	preempt_disable();
 
 	kvm_x86_ops->prepare_guest_switch(vcpu);
-	kvm_load_guest_fpu(vcpu);
+	if (vcpu->fpu_active)
+		kvm_load_guest_fpu(vcpu);
 
 	local_irq_disable();
 
@@ -5297,7 +5298,7 @@ EXPORT_SYMBOL_GPL(fx_init);
 
 void kvm_load_guest_fpu(struct kvm_vcpu *vcpu)
 {
-	if (!vcpu->fpu_active || vcpu->guest_fpu_loaded)
+	if (vcpu->guest_fpu_loaded)
 		return;
 
 	vcpu->guest_fpu_loaded = 1;
-- 
1.6.5.3


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

* [PATCH 2/8] KVM: Drop kvm_{load,put}_guest_fpu() exports
  2010-01-21 13:31 [PATCH 0/8] cr0/cr4/efer/fpu miscellaneous bits Avi Kivity
  2010-01-21 13:31 ` [PATCH 1/8] KVM: Allow kvm_load_guest_fpu() even when !vcpu->fpu_active Avi Kivity
@ 2010-01-21 13:31 ` Avi Kivity
  2010-01-21 13:31 ` [PATCH 3/8] KVM: Activate fpu on clts Avi Kivity
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 23+ messages in thread
From: Avi Kivity @ 2010-01-21 13:31 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: kvm

Not used anymore.

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

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e3145d5..feca59f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5305,7 +5305,6 @@ void kvm_load_guest_fpu(struct kvm_vcpu *vcpu)
 	kvm_fx_save(&vcpu->arch.host_fx_image);
 	kvm_fx_restore(&vcpu->arch.guest_fx_image);
 }
-EXPORT_SYMBOL_GPL(kvm_load_guest_fpu);
 
 void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
 {
@@ -5318,7 +5317,6 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
 	++vcpu->stat.fpu_reload;
 	set_bit(KVM_REQ_DEACTIVATE_FPU, &vcpu->requests);
 }
-EXPORT_SYMBOL_GPL(kvm_put_guest_fpu);
 
 void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
 {
-- 
1.6.5.3


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

* [PATCH 3/8] KVM: Activate fpu on clts
  2010-01-21 13:31 [PATCH 0/8] cr0/cr4/efer/fpu miscellaneous bits Avi Kivity
  2010-01-21 13:31 ` [PATCH 1/8] KVM: Allow kvm_load_guest_fpu() even when !vcpu->fpu_active Avi Kivity
  2010-01-21 13:31 ` [PATCH 2/8] KVM: Drop kvm_{load,put}_guest_fpu() exports Avi Kivity
@ 2010-01-21 13:31 ` Avi Kivity
  2010-01-23 18:56   ` Marcelo Tosatti
  2010-02-02  8:16   ` Paolo Bonzini
  2010-01-21 13:31 ` [PATCH 4/8] KVM: Add a helper for checking if the guest is in protected mode Avi Kivity
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 23+ messages in thread
From: Avi Kivity @ 2010-01-21 13:31 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: kvm

Assume that if the guest executes clts, it knows what it's doing, and load the
guest fpu to prevent an #NM exception.

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

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index a1f0b5d..bf3ec76 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -512,6 +512,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_activate)(struct kvm_vcpu *vcpu);
 	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 8d7cb62..0f3738a 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1259,12 +1259,17 @@ static int ud_interception(struct vcpu_svm *svm)
 	return 1;
 }
 
-static int nm_interception(struct vcpu_svm *svm)
+static void svm_fpu_activate(struct kvm_vcpu *vcpu)
 {
+	struct vcpu_svm *svm = to_svm(vcpu);
 	svm->vmcb->control.intercept_exceptions &= ~(1 << NM_VECTOR);
 	svm->vcpu.fpu_active = 1;
 	update_cr0_intercept(svm);
+}
 
+static int nm_interception(struct vcpu_svm *svm)
+{
+	svm_fpu_activate(&svm->vcpu);
 	return 1;
 }
 
@@ -2971,6 +2976,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_activate = svm_fpu_activate,
 	.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 7375ae1..372bc38 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3011,6 +3011,7 @@ static int handle_cr(struct kvm_vcpu *vcpu)
 		vmcs_writel(CR0_READ_SHADOW, kvm_read_cr0(vcpu));
 		trace_kvm_cr_write(0, kvm_read_cr0(vcpu));
 		skip_emulated_instruction(vcpu);
+		vmx_fpu_activate(vcpu);
 		return 1;
 	case 1: /*mov from cr*/
 		switch (cr) {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index feca59f..09207ba 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3266,6 +3266,7 @@ int emulate_invlpg(struct kvm_vcpu *vcpu, gva_t address)
 int emulate_clts(struct kvm_vcpu *vcpu)
 {
 	kvm_x86_ops->set_cr0(vcpu, kvm_read_cr0_bits(vcpu, ~X86_CR0_TS));
+	kvm_x86_ops->fpu_activate(vcpu);
 	return X86EMUL_CONTINUE;
 }
 
-- 
1.6.5.3


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

* [PATCH 4/8] KVM: Add a helper for checking if the guest is in protected mode
  2010-01-21 13:31 [PATCH 0/8] cr0/cr4/efer/fpu miscellaneous bits Avi Kivity
                   ` (2 preceding siblings ...)
  2010-01-21 13:31 ` [PATCH 3/8] KVM: Activate fpu on clts Avi Kivity
@ 2010-01-21 13:31 ` Avi Kivity
  2010-01-21 13:31 ` [PATCH 5/8] KVM: Move cr0/cr4/efer related helpers to x86.h Avi Kivity
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 23+ messages in thread
From: Avi Kivity @ 2010-01-21 13:31 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: kvm

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/emulate.c |    9 ++++-----
 arch/x86/kvm/vmx.c     |    4 ++--
 arch/x86/kvm/x86.c     |    7 +++----
 arch/x86/kvm/x86.h     |    6 ++++++
 4 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 0f89e32..e46f276 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -32,6 +32,7 @@
 #include <linux/module.h>
 #include <asm/kvm_emulate.h>
 
+#include "x86.h"
 #include "mmu.h"		/* for is_long_mode() */
 
 /*
@@ -1515,7 +1516,7 @@ emulate_syscall(struct x86_emulate_ctxt *ctxt)
 
 	/* syscall is not available in real mode */
 	if (c->lock_prefix || ctxt->mode == X86EMUL_MODE_REAL
-	    || !kvm_read_cr0_bits(ctxt->vcpu, X86_CR0_PE))
+	    || !is_protmode(ctxt->vcpu))
 		return -1;
 
 	setup_syscalls_segments(ctxt, &cs, &ss);
@@ -1568,8 +1569,7 @@ emulate_sysenter(struct x86_emulate_ctxt *ctxt)
 		return -1;
 
 	/* inject #GP if in real mode or paging is disabled */
-	if (ctxt->mode == X86EMUL_MODE_REAL ||
-	    !kvm_read_cr0_bits(ctxt->vcpu, X86_CR0_PE)) {
+	if (ctxt->mode == X86EMUL_MODE_REAL || !is_protmode(ctxt->vcpu)) {
 		kvm_inject_gp(ctxt->vcpu, 0);
 		return -1;
 	}
@@ -1634,8 +1634,7 @@ emulate_sysexit(struct x86_emulate_ctxt *ctxt)
 		return -1;
 
 	/* inject #GP if in real mode or paging is disabled */
-	if (ctxt->mode == X86EMUL_MODE_REAL
-	    || !kvm_read_cr0_bits(ctxt->vcpu, X86_CR0_PE)) {
+	if (ctxt->mode == X86EMUL_MODE_REAL || !is_protmode(ctxt->vcpu)) {
 		kvm_inject_gp(ctxt->vcpu, 0);
 		return -1;
 	}
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 372bc38..cd78049 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1853,7 +1853,7 @@ static void vmx_get_segment(struct kvm_vcpu *vcpu,
 
 static int vmx_get_cpl(struct kvm_vcpu *vcpu)
 {
-	if (!kvm_read_cr0_bits(vcpu, X86_CR0_PE)) /* if real mode */
+	if (!is_protmode(vcpu))
 		return 0;
 
 	if (vmx_get_rflags(vcpu) & X86_EFLAGS_VM) /* if virtual 8086 */
@@ -2108,7 +2108,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 (!kvm_read_cr0_bits(vcpu, X86_CR0_PE)) {
+	if (!is_protmode(vcpu)) {
 		if (!rmode_segment_valid(vcpu, VCPU_SREG_CS))
 			return false;
 		if (!rmode_segment_valid(vcpu, VCPU_SREG_SS))
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 09207ba..6cdead0 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3798,8 +3798,7 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 	 * hypercall generates UD from non zero cpl and real mode
 	 * per HYPER-V spec
 	 */
-	if (kvm_x86_ops->get_cpl(vcpu) != 0 ||
-	    !kvm_read_cr0_bits(vcpu, X86_CR0_PE)) {
+	if (kvm_x86_ops->get_cpl(vcpu) != 0 || !is_protmode(vcpu)) {
 		kvm_queue_exception(vcpu, UD_VECTOR);
 		return 0;
 	}
@@ -4763,7 +4762,7 @@ int kvm_load_segment_descriptor(struct kvm_vcpu *vcpu, u16 selector,
 {
 	struct kvm_segment kvm_seg;
 
-	if (is_vm86_segment(vcpu, seg) || !(kvm_read_cr0_bits(vcpu, X86_CR0_PE)))
+	if (is_vm86_segment(vcpu, seg) || !is_protmode(vcpu))
 		return kvm_load_realmode_segment(vcpu, selector, seg);
 	if (load_segment_descriptor_to_kvm_desct(vcpu, selector, &kvm_seg))
 		return 1;
@@ -5115,7 +5114,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 &&
-	    !(kvm_read_cr0_bits(vcpu, X86_CR0_PE)))
+	    !is_protmode(vcpu))
 		vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE;
 
 	vcpu_put(vcpu);
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 5eadea5..f783d8f 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -2,6 +2,7 @@
 #define ARCH_X86_KVM_X86_H
 
 #include <linux/kvm_host.h>
+#include "kvm_cache_regs.h"
 
 static inline void kvm_clear_exception_queue(struct kvm_vcpu *vcpu)
 {
@@ -35,4 +36,9 @@ static inline bool kvm_exception_is_soft(unsigned int nr)
 struct kvm_cpuid_entry2 *kvm_find_cpuid_entry(struct kvm_vcpu *vcpu,
                                              u32 function, u32 index);
 
+static inline bool is_protmode(struct kvm_vcpu *vcpu)
+{
+	return kvm_read_cr0_bits(vcpu, X86_CR0_PE);
+}
+
 #endif
-- 
1.6.5.3


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

* [PATCH 5/8] KVM: Move cr0/cr4/efer related helpers to x86.h
  2010-01-21 13:31 [PATCH 0/8] cr0/cr4/efer/fpu miscellaneous bits Avi Kivity
                   ` (3 preceding siblings ...)
  2010-01-21 13:31 ` [PATCH 4/8] KVM: Add a helper for checking if the guest is in protected mode Avi Kivity
@ 2010-01-21 13:31 ` Avi Kivity
  2010-01-21 13:31 ` [PATCH 6/8] KVM: Rename vcpu->shadow_efer to efer Avi Kivity
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 23+ messages in thread
From: Avi Kivity @ 2010-01-21 13:31 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: kvm

They have more general scope than the mmu.

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

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index e46f276..a2adec8 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -33,7 +33,6 @@
 #include <asm/kvm_emulate.h>
 
 #include "x86.h"
-#include "mmu.h"		/* for is_long_mode() */
 
 /*
  * Opcode effective-address decode tables.
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index ff2b2e8..6f7158f 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -18,6 +18,7 @@
  */
 
 #include "mmu.h"
+#include "x86.h"
 #include "kvm_cache_regs.h"
 
 #include <linux/kvm_host.h>
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index 599159f..61ef5a6 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -58,30 +58,6 @@ static inline int kvm_mmu_reload(struct kvm_vcpu *vcpu)
 	return kvm_mmu_load(vcpu);
 }
 
-static inline int is_long_mode(struct kvm_vcpu *vcpu)
-{
-#ifdef CONFIG_X86_64
-	return vcpu->arch.shadow_efer & EFER_LMA;
-#else
-	return 0;
-#endif
-}
-
-static inline int is_pae(struct kvm_vcpu *vcpu)
-{
-	return kvm_read_cr4_bits(vcpu, X86_CR4_PAE);
-}
-
-static inline int is_pse(struct kvm_vcpu *vcpu)
-{
-	return kvm_read_cr4_bits(vcpu, X86_CR4_PSE);
-}
-
-static inline int is_paging(struct kvm_vcpu *vcpu)
-{
-	return kvm_read_cr0_bits(vcpu, X86_CR0_PG);
-}
-
 static inline int is_present_gpte(unsigned long pte)
 {
 	return pte & PT_PRESENT_MASK;
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index f783d8f..2dc24a7 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -41,4 +41,28 @@ static inline bool is_protmode(struct kvm_vcpu *vcpu)
 	return kvm_read_cr0_bits(vcpu, X86_CR0_PE);
 }
 
+static inline int is_long_mode(struct kvm_vcpu *vcpu)
+{
+#ifdef CONFIG_X86_64
+	return vcpu->arch.shadow_efer & EFER_LMA;
+#else
+	return 0;
+#endif
+}
+
+static inline int is_pae(struct kvm_vcpu *vcpu)
+{
+	return kvm_read_cr4_bits(vcpu, X86_CR4_PAE);
+}
+
+static inline int is_pse(struct kvm_vcpu *vcpu)
+{
+	return kvm_read_cr4_bits(vcpu, X86_CR4_PSE);
+}
+
+static inline int is_paging(struct kvm_vcpu *vcpu)
+{
+	return kvm_read_cr0_bits(vcpu, X86_CR0_PG);
+}
+
 #endif
-- 
1.6.5.3


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

* [PATCH 6/8] KVM: Rename vcpu->shadow_efer to efer
  2010-01-21 13:31 [PATCH 0/8] cr0/cr4/efer/fpu miscellaneous bits Avi Kivity
                   ` (4 preceding siblings ...)
  2010-01-21 13:31 ` [PATCH 5/8] KVM: Move cr0/cr4/efer related helpers to x86.h Avi Kivity
@ 2010-01-21 13:31 ` Avi Kivity
  2010-01-21 13:31 ` [PATCH 7/8] KVM: Optimize kvm_read_cr[04]_bits() Avi Kivity
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 23+ messages in thread
From: Avi Kivity @ 2010-01-21 13:31 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: kvm

None of the other registers have the shadow_ prefix.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |    2 +-
 arch/x86/kvm/mmu.c              |    2 +-
 arch/x86/kvm/svm.c              |   12 ++++++------
 arch/x86/kvm/vmx.c              |   14 +++++++-------
 arch/x86/kvm/x86.c              |   14 +++++++-------
 arch/x86/kvm/x86.h              |    2 +-
 6 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index bf3ec76..76bf686 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -277,7 +277,7 @@ struct kvm_vcpu_arch {
 	unsigned long cr8;
 	u32 hflags;
 	u64 pdptrs[4]; /* pae */
-	u64 shadow_efer;
+	u64 efer;
 	u64 apic_base;
 	struct kvm_lapic *apic;    /* kernel irqchip context */
 	int32_t apic_arb_prio;
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 6f7158f..599c422 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -237,7 +237,7 @@ static int is_cpuid_PSE36(void)
 
 static int is_nx(struct kvm_vcpu *vcpu)
 {
-	return vcpu->arch.shadow_efer & EFER_NX;
+	return vcpu->arch.efer & EFER_NX;
 }
 
 static int is_shadow_present_pte(u64 pte)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 0f3738a..0242fdd 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -231,7 +231,7 @@ static void svm_set_efer(struct kvm_vcpu *vcpu, u64 efer)
 		efer &= ~EFER_LME;
 
 	to_svm(vcpu)->vmcb->save.efer = efer | EFER_SVME;
-	vcpu->arch.shadow_efer = efer;
+	vcpu->arch.efer = efer;
 }
 
 static void svm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr,
@@ -990,14 +990,14 @@ static void svm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
 	struct vcpu_svm *svm = to_svm(vcpu);
 
 #ifdef CONFIG_X86_64
-	if (vcpu->arch.shadow_efer & EFER_LME) {
+	if (vcpu->arch.efer & EFER_LME) {
 		if (!is_paging(vcpu) && (cr0 & X86_CR0_PG)) {
-			vcpu->arch.shadow_efer |= EFER_LMA;
+			vcpu->arch.efer |= EFER_LMA;
 			svm->vmcb->save.efer |= EFER_LMA | EFER_LME;
 		}
 
 		if (is_paging(vcpu) && !(cr0 & X86_CR0_PG)) {
-			vcpu->arch.shadow_efer &= ~EFER_LMA;
+			vcpu->arch.efer &= ~EFER_LMA;
 			svm->vmcb->save.efer &= ~(EFER_LMA | EFER_LME);
 		}
 	}
@@ -1361,7 +1361,7 @@ static int vmmcall_interception(struct vcpu_svm *svm)
 
 static int nested_svm_check_permissions(struct vcpu_svm *svm)
 {
-	if (!(svm->vcpu.arch.shadow_efer & EFER_SVME)
+	if (!(svm->vcpu.arch.efer & EFER_SVME)
 	    || !is_paging(&svm->vcpu)) {
 		kvm_queue_exception(&svm->vcpu, UD_VECTOR);
 		return 1;
@@ -1764,7 +1764,7 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)
 	hsave->save.ds     = vmcb->save.ds;
 	hsave->save.gdtr   = vmcb->save.gdtr;
 	hsave->save.idtr   = vmcb->save.idtr;
-	hsave->save.efer   = svm->vcpu.arch.shadow_efer;
+	hsave->save.efer   = svm->vcpu.arch.efer;
 	hsave->save.cr0    = kvm_read_cr0(&svm->vcpu);
 	hsave->save.cr4    = svm->vcpu.arch.cr4;
 	hsave->save.rflags = vmcb->save.rflags;
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index cd78049..d4a6260 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -618,7 +618,7 @@ static bool update_transition_efer(struct vcpu_vmx *vmx, int efer_offset)
 	u64 guest_efer;
 	u64 ignore_bits;
 
-	guest_efer = vmx->vcpu.arch.shadow_efer;
+	guest_efer = vmx->vcpu.arch.efer;
 
 	/*
 	 * NX is emulated; LMA and LME handled by hardware; SCE meaninless
@@ -963,7 +963,7 @@ static void setup_msrs(struct vcpu_vmx *vmx)
 		 * if efer.sce is enabled.
 		 */
 		index = __find_msr_index(vmx, MSR_K6_STAR);
-		if ((index >= 0) && (vmx->vcpu.arch.shadow_efer & EFER_SCE))
+		if ((index >= 0) && (vmx->vcpu.arch.efer & EFER_SCE))
 			move_msr_up(vmx, index, save_nmsrs++);
 	}
 #endif
@@ -1608,7 +1608,7 @@ static void vmx_set_efer(struct kvm_vcpu *vcpu, u64 efer)
 	 * of this msr depends on is_long_mode().
 	 */
 	vmx_load_host_state(to_vmx(vcpu));
-	vcpu->arch.shadow_efer = efer;
+	vcpu->arch.efer = efer;
 	if (!msr)
 		return;
 	if (efer & EFER_LMA) {
@@ -1640,13 +1640,13 @@ static void enter_lmode(struct kvm_vcpu *vcpu)
 			     (guest_tr_ar & ~AR_TYPE_MASK)
 			     | AR_TYPE_BUSY_64_TSS);
 	}
-	vcpu->arch.shadow_efer |= EFER_LMA;
-	vmx_set_efer(vcpu, vcpu->arch.shadow_efer);
+	vcpu->arch.efer |= EFER_LMA;
+	vmx_set_efer(vcpu, vcpu->arch.efer);
 }
 
 static void exit_lmode(struct kvm_vcpu *vcpu)
 {
-	vcpu->arch.shadow_efer &= ~EFER_LMA;
+	vcpu->arch.efer &= ~EFER_LMA;
 
 	vmcs_write32(VM_ENTRY_CONTROLS,
 		     vmcs_read32(VM_ENTRY_CONTROLS)
@@ -1753,7 +1753,7 @@ static void vmx_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
 		enter_rmode(vcpu);
 
 #ifdef CONFIG_X86_64
-	if (vcpu->arch.shadow_efer & EFER_LME) {
+	if (vcpu->arch.efer & EFER_LME) {
 		if (!is_paging(vcpu) && (cr0 & X86_CR0_PG))
 			enter_lmode(vcpu);
 		if (is_paging(vcpu) && !(cr0 & X86_CR0_PG))
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 6cdead0..8b42c19 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -452,7 +452,7 @@ void kvm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
 
 	if (!is_paging(vcpu) && (cr0 & X86_CR0_PG)) {
 #ifdef CONFIG_X86_64
-		if ((vcpu->arch.shadow_efer & EFER_LME)) {
+		if ((vcpu->arch.efer & EFER_LME)) {
 			int cs_db, cs_l;
 
 			if (!is_pae(vcpu)) {
@@ -651,7 +651,7 @@ static void set_efer(struct kvm_vcpu *vcpu, u64 efer)
 	}
 
 	if (is_paging(vcpu)
-	    && (vcpu->arch.shadow_efer & EFER_LME) != (efer & EFER_LME)) {
+	    && (vcpu->arch.efer & EFER_LME) != (efer & EFER_LME)) {
 		printk(KERN_DEBUG "set_efer: #GP, change LME while paging\n");
 		kvm_inject_gp(vcpu, 0);
 		return;
@@ -682,9 +682,9 @@ static void set_efer(struct kvm_vcpu *vcpu, u64 efer)
 	kvm_x86_ops->set_efer(vcpu, efer);
 
 	efer &= ~EFER_LMA;
-	efer |= vcpu->arch.shadow_efer & EFER_LMA;
+	efer |= vcpu->arch.efer & EFER_LMA;
 
-	vcpu->arch.shadow_efer = efer;
+	vcpu->arch.efer = efer;
 
 	vcpu->arch.mmu.base_role.nxe = (efer & EFER_NX) && !tdp_enabled;
 	kvm_mmu_reset_context(vcpu);
@@ -1423,7 +1423,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 		data |= (((uint64_t)4ULL) << 40);
 		break;
 	case MSR_EFER:
-		data = vcpu->arch.shadow_efer;
+		data = vcpu->arch.efer;
 		break;
 	case MSR_KVM_WALL_CLOCK:
 		data = vcpu->kvm->arch.wall_clock;
@@ -4581,7 +4581,7 @@ int kvm_arch_vcpu_ioctl_get_sregs(struct kvm_vcpu *vcpu,
 	sregs->cr3 = vcpu->arch.cr3;
 	sregs->cr4 = kvm_read_cr4(vcpu);
 	sregs->cr8 = kvm_get_cr8(vcpu);
-	sregs->efer = vcpu->arch.shadow_efer;
+	sregs->efer = vcpu->arch.efer;
 	sregs->apic_base = kvm_get_apic_base(vcpu);
 
 	memset(sregs->interrupt_bitmap, 0, sizeof sregs->interrupt_bitmap);
@@ -5071,7 +5071,7 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
 
 	kvm_set_cr8(vcpu, sregs->cr8);
 
-	mmu_reset_needed |= vcpu->arch.shadow_efer != sregs->efer;
+	mmu_reset_needed |= vcpu->arch.efer != sregs->efer;
 	kvm_x86_ops->set_efer(vcpu, sregs->efer);
 	kvm_set_apic_base(vcpu, sregs->apic_base);
 
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 2dc24a7..2d10163 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -44,7 +44,7 @@ static inline bool is_protmode(struct kvm_vcpu *vcpu)
 static inline int is_long_mode(struct kvm_vcpu *vcpu)
 {
 #ifdef CONFIG_X86_64
-	return vcpu->arch.shadow_efer & EFER_LMA;
+	return vcpu->arch.efer & EFER_LMA;
 #else
 	return 0;
 #endif
-- 
1.6.5.3


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

* [PATCH 7/8] KVM: Optimize kvm_read_cr[04]_bits()
  2010-01-21 13:31 [PATCH 0/8] cr0/cr4/efer/fpu miscellaneous bits Avi Kivity
                   ` (5 preceding siblings ...)
  2010-01-21 13:31 ` [PATCH 6/8] KVM: Rename vcpu->shadow_efer to efer Avi Kivity
@ 2010-01-21 13:31 ` Avi Kivity
  2010-02-05  8:26   ` Jan Kiszka
  2010-01-21 13:31 ` [PATCH 8/8] KVM: trace guest fpu loads and unloads Avi Kivity
  2010-01-23 19:02 ` [PATCH 0/8] cr0/cr4/efer/fpu miscellaneous bits Marcelo Tosatti
  8 siblings, 1 reply; 23+ messages in thread
From: Avi Kivity @ 2010-01-21 13:31 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: kvm

'mask' is always a constant, so we can check whether it includes a bit that
might be owned by the guest very cheaply, and avoid the decache call.  Saves
a few hundred bytes of module text.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/kvm_cache_regs.h |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/kvm_cache_regs.h b/arch/x86/kvm/kvm_cache_regs.h
index 6b419a3..5a109c6 100644
--- a/arch/x86/kvm/kvm_cache_regs.h
+++ b/arch/x86/kvm/kvm_cache_regs.h
@@ -1,6 +1,9 @@
 #ifndef ASM_KVM_CACHE_REGS_H
 #define ASM_KVM_CACHE_REGS_H
 
+#define KVM_POSSIBLE_CR0_GUEST_BITS X86_CR0_TS
+#define KVM_POSSIBLE_CR4_GUEST_BITS X86_CR4_PGE
+
 static inline unsigned long kvm_register_read(struct kvm_vcpu *vcpu,
 					      enum kvm_reg reg)
 {
@@ -40,7 +43,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)
+	ulong tmask = mask & KVM_POSSIBLE_CR0_GUEST_BITS;
+	if (tmask & vcpu->arch.cr0_guest_owned_bits)
 		kvm_x86_ops->decache_cr0_guest_bits(vcpu);
 	return vcpu->arch.cr0 & mask;
 }
@@ -52,7 +56,8 @@ static inline ulong kvm_read_cr0(struct kvm_vcpu *vcpu)
 
 static inline ulong kvm_read_cr4_bits(struct kvm_vcpu *vcpu, ulong mask)
 {
-	if (mask & vcpu->arch.cr4_guest_owned_bits)
+	ulong tmask = mask & KVM_POSSIBLE_CR4_GUEST_BITS;
+	if (tmask & vcpu->arch.cr4_guest_owned_bits)
 		kvm_x86_ops->decache_cr4_guest_bits(vcpu);
 	return vcpu->arch.cr4 & mask;
 }
-- 
1.6.5.3


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

* [PATCH 8/8] KVM: trace guest fpu loads and unloads
  2010-01-21 13:31 [PATCH 0/8] cr0/cr4/efer/fpu miscellaneous bits Avi Kivity
                   ` (6 preceding siblings ...)
  2010-01-21 13:31 ` [PATCH 7/8] KVM: Optimize kvm_read_cr[04]_bits() Avi Kivity
@ 2010-01-21 13:31 ` Avi Kivity
  2010-01-23 19:02 ` [PATCH 0/8] cr0/cr4/efer/fpu miscellaneous bits Marcelo Tosatti
  8 siblings, 0 replies; 23+ messages in thread
From: Avi Kivity @ 2010-01-21 13:31 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: kvm

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/x86.c         |    2 ++
 include/trace/events/kvm.h |   19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 8b42c19..06a03c1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5304,6 +5304,7 @@ void kvm_load_guest_fpu(struct kvm_vcpu *vcpu)
 	vcpu->guest_fpu_loaded = 1;
 	kvm_fx_save(&vcpu->arch.host_fx_image);
 	kvm_fx_restore(&vcpu->arch.guest_fx_image);
+	trace_kvm_fpu(1);
 }
 
 void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
@@ -5316,6 +5317,7 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
 	kvm_fx_restore(&vcpu->arch.host_fx_image);
 	++vcpu->stat.fpu_reload;
 	set_bit(KVM_REQ_DEACTIVATE_FPU, &vcpu->requests);
+	trace_kvm_fpu(0);
 }
 
 void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
diff --git a/include/trace/events/kvm.h b/include/trace/events/kvm.h
index dbe1084..8abdc12 100644
--- a/include/trace/events/kvm.h
+++ b/include/trace/events/kvm.h
@@ -145,6 +145,25 @@ TRACE_EVENT(kvm_mmio,
 		  __entry->len, __entry->gpa, __entry->val)
 );
 
+#define kvm_fpu_load_symbol	\
+	{0, "unload"},		\
+	{1, "load"}
+
+TRACE_EVENT(kvm_fpu,
+	TP_PROTO(int load),
+	TP_ARGS(load),
+
+	TP_STRUCT__entry(
+		__field(	u32,	        load		)
+	),
+
+	TP_fast_assign(
+		__entry->load		= load;
+	),
+
+	TP_printk("%s", __print_symbolic(__entry->load, kvm_fpu_load_symbol))
+);
+
 #endif /* _TRACE_KVM_MAIN_H */
 
 /* This part must be outside protection */
-- 
1.6.5.3


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

* Re: [PATCH 3/8] KVM: Activate fpu on clts
  2010-01-21 13:31 ` [PATCH 3/8] KVM: Activate fpu on clts Avi Kivity
@ 2010-01-23 18:56   ` Marcelo Tosatti
  2010-01-24  7:20     ` Avi Kivity
  2010-02-02  8:16   ` Paolo Bonzini
  1 sibling, 1 reply; 23+ messages in thread
From: Marcelo Tosatti @ 2010-01-23 18:56 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm

On Thu, Jan 21, 2010 at 03:31:47PM +0200, Avi Kivity wrote:
> Assume that if the guest executes clts, it knows what it's doing, and load the
> guest fpu to prevent an #NM exception.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---

Out of curiosity, previously you assumed lazy activation on clts was a
win. Why undo it?


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

* Re: [PATCH 0/8] cr0/cr4/efer/fpu miscellaneous bits
  2010-01-21 13:31 [PATCH 0/8] cr0/cr4/efer/fpu miscellaneous bits Avi Kivity
                   ` (7 preceding siblings ...)
  2010-01-21 13:31 ` [PATCH 8/8] KVM: trace guest fpu loads and unloads Avi Kivity
@ 2010-01-23 19:02 ` Marcelo Tosatti
  8 siblings, 0 replies; 23+ messages in thread
From: Marcelo Tosatti @ 2010-01-23 19:02 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm

On Thu, Jan 21, 2010 at 03:31:44PM +0200, Avi Kivity wrote:
> Mostly trivial cleanups with the exception of a patch activating the fpu
> on clts.
> 
> Avi Kivity (8):
>   KVM: Allow kvm_load_guest_fpu() even when !vcpu->fpu_active
>   KVM: Drop kvm_{load,put}_guest_fpu() exports
>   KVM: Activate fpu on clts
>   KVM: Add a helper for checking if the guest is in protected mode
>   KVM: Move cr0/cr4/efer related helpers to x86.h
>   KVM: Rename vcpu->shadow_efer to efer
>   KVM: Optimize kvm_read_cr[04]_bits()
>   KVM: trace guest fpu loads and unloads

Applied all, thanks.


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

* Re: [PATCH 3/8] KVM: Activate fpu on clts
  2010-01-23 18:56   ` Marcelo Tosatti
@ 2010-01-24  7:20     ` Avi Kivity
  0 siblings, 0 replies; 23+ messages in thread
From: Avi Kivity @ 2010-01-24  7:20 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: kvm

On 01/23/2010 08:56 PM, Marcelo Tosatti wrote:
> On Thu, Jan 21, 2010 at 03:31:47PM +0200, Avi Kivity wrote:
>    
>> Assume that if the guest executes clts, it knows what it's doing, and load the
>> guest fpu to prevent an #NM exception.
>>
>> Signed-off-by: Avi Kivity<avi@redhat.com>
>> ---
>>      
> Out of curiosity, previously you assumed lazy activation on clts was a
> win. Why undo it?
>    

It wasn't so much assuming it's a win, it's more I was too lazy to find 
out.  But it was easy to see from the traces that clts is often followed 
by #NM.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


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

* Re: [PATCH 3/8] KVM: Activate fpu on clts
  2010-01-21 13:31 ` [PATCH 3/8] KVM: Activate fpu on clts Avi Kivity
  2010-01-23 18:56   ` Marcelo Tosatti
@ 2010-02-02  8:16   ` Paolo Bonzini
  2010-02-03 10:23     ` Gleb Natapov
  2010-02-04 13:05     ` Avi Kivity
  1 sibling, 2 replies; 23+ messages in thread
From: Paolo Bonzini @ 2010-02-02  8:16 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm

On 01/21/2010 02:31 PM, Avi Kivity wrote:
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index feca59f..09207ba 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3266,6 +3266,7 @@ int emulate_invlpg(struct kvm_vcpu *vcpu, gva_t address)
>   int emulate_clts(struct kvm_vcpu *vcpu)
>   {
>   	kvm_x86_ops->set_cr0(vcpu, kvm_read_cr0_bits(vcpu, ~X86_CR0_TS));
> +	kvm_x86_ops->fpu_activate(vcpu);
>   	return X86EMUL_CONTINUE;
>   }

Can this code be reached if CLTS is executed in real mode?  That would 
cause a NULL-pointer access on VMX.

Paolo

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

* Re: [PATCH 3/8] KVM: Activate fpu on clts
  2010-02-02  8:16   ` Paolo Bonzini
@ 2010-02-03 10:23     ` Gleb Natapov
  2010-02-04 13:05     ` Avi Kivity
  1 sibling, 0 replies; 23+ messages in thread
From: Gleb Natapov @ 2010-02-03 10:23 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Avi Kivity, Marcelo Tosatti, kvm

On Tue, Feb 02, 2010 at 09:16:44AM +0100, Paolo Bonzini wrote:
> On 01/21/2010 02:31 PM, Avi Kivity wrote:
> >diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >index feca59f..09207ba 100644
> >--- a/arch/x86/kvm/x86.c
> >+++ b/arch/x86/kvm/x86.c
> >@@ -3266,6 +3266,7 @@ int emulate_invlpg(struct kvm_vcpu *vcpu, gva_t address)
> >  int emulate_clts(struct kvm_vcpu *vcpu)
> >  {
> >  	kvm_x86_ops->set_cr0(vcpu, kvm_read_cr0_bits(vcpu, ~X86_CR0_TS));
> >+	kvm_x86_ops->fpu_activate(vcpu);
> >  	return X86EMUL_CONTINUE;
> >  }
> 
> Can this code be reached if CLTS is executed in real mode?  That
> would cause a NULL-pointer access on VMX.
> 
We should assume that any part of emulator can be reached from a guest
and do check accordingly.

--
			Gleb.

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

* Re: [PATCH 3/8] KVM: Activate fpu on clts
  2010-02-02  8:16   ` Paolo Bonzini
  2010-02-03 10:23     ` Gleb Natapov
@ 2010-02-04 13:05     ` Avi Kivity
  2010-02-04 13:11       ` Gleb Natapov
  1 sibling, 1 reply; 23+ messages in thread
From: Avi Kivity @ 2010-02-04 13:05 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Marcelo Tosatti, kvm

On 02/02/2010 10:16 AM, Paolo Bonzini wrote:
> On 01/21/2010 02:31 PM, Avi Kivity wrote:
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index feca59f..09207ba 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -3266,6 +3266,7 @@ int emulate_invlpg(struct kvm_vcpu *vcpu, gva_t 
>> address)
>>   int emulate_clts(struct kvm_vcpu *vcpu)
>>   {
>>       kvm_x86_ops->set_cr0(vcpu, kvm_read_cr0_bits(vcpu, ~X86_CR0_TS));
>> +    kvm_x86_ops->fpu_activate(vcpu);
>>       return X86EMUL_CONTINUE;
>>   }
>
> Can this code be reached if CLTS is executed in real mode?  That would 
> cause a NULL-pointer access on VMX.

How would this cause a null pointer access?

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


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

* Re: [PATCH 3/8] KVM: Activate fpu on clts
  2010-02-04 13:05     ` Avi Kivity
@ 2010-02-04 13:11       ` Gleb Natapov
  2010-02-04 17:56         ` Avi Kivity
  0 siblings, 1 reply; 23+ messages in thread
From: Gleb Natapov @ 2010-02-04 13:11 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Paolo Bonzini, Marcelo Tosatti, kvm

On Thu, Feb 04, 2010 at 03:05:17PM +0200, Avi Kivity wrote:
> On 02/02/2010 10:16 AM, Paolo Bonzini wrote:
> >On 01/21/2010 02:31 PM, Avi Kivity wrote:
> >>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >>index feca59f..09207ba 100644
> >>--- a/arch/x86/kvm/x86.c
> >>+++ b/arch/x86/kvm/x86.c
> >>@@ -3266,6 +3266,7 @@ int emulate_invlpg(struct kvm_vcpu *vcpu,
> >>gva_t address)
> >>  int emulate_clts(struct kvm_vcpu *vcpu)
> >>  {
> >>      kvm_x86_ops->set_cr0(vcpu, kvm_read_cr0_bits(vcpu, ~X86_CR0_TS));
> >>+    kvm_x86_ops->fpu_activate(vcpu);
> >>      return X86EMUL_CONTINUE;
> >>  }
> >
> >Can this code be reached if CLTS is executed in real mode?  That
> >would cause a NULL-pointer access on VMX.
> 
> How would this cause a null pointer access?
> 
vmx.c doesn't initialize kvm_x86_ops->fpu_activate as far as I see.

--
			Gleb.

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

* Re: [PATCH 3/8] KVM: Activate fpu on clts
  2010-02-04 13:11       ` Gleb Natapov
@ 2010-02-04 17:56         ` Avi Kivity
  2010-02-07 12:25           ` Avi Kivity
  0 siblings, 1 reply; 23+ messages in thread
From: Avi Kivity @ 2010-02-04 17:56 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: Paolo Bonzini, Marcelo Tosatti, kvm

On 02/04/2010 03:11 PM, Gleb Natapov wrote:
> On Thu, Feb 04, 2010 at 03:05:17PM +0200, Avi Kivity wrote:
>    
>> On 02/02/2010 10:16 AM, Paolo Bonzini wrote:
>>      
>>> On 01/21/2010 02:31 PM, Avi Kivity wrote:
>>>        
>>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>>> index feca59f..09207ba 100644
>>>> --- a/arch/x86/kvm/x86.c
>>>> +++ b/arch/x86/kvm/x86.c
>>>> @@ -3266,6 +3266,7 @@ int emulate_invlpg(struct kvm_vcpu *vcpu,
>>>> gva_t address)
>>>>   int emulate_clts(struct kvm_vcpu *vcpu)
>>>>   {
>>>>       kvm_x86_ops->set_cr0(vcpu, kvm_read_cr0_bits(vcpu, ~X86_CR0_TS));
>>>> +    kvm_x86_ops->fpu_activate(vcpu);
>>>>       return X86EMUL_CONTINUE;
>>>>   }
>>>>          
>>> Can this code be reached if CLTS is executed in real mode?  That
>>> would cause a NULL-pointer access on VMX.
>>>        
>> How would this cause a null pointer access?
>>
>>      
> vmx.c doesn't initialize kvm_x86_ops->fpu_activate as far as I see.
>    

Gaak.  Well, that's obviously unintended.

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


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

* Re: [PATCH 7/8] KVM: Optimize kvm_read_cr[04]_bits()
  2010-01-21 13:31 ` [PATCH 7/8] KVM: Optimize kvm_read_cr[04]_bits() Avi Kivity
@ 2010-02-05  8:26   ` Jan Kiszka
  2010-02-07 12:05     ` Avi Kivity
  0 siblings, 1 reply; 23+ messages in thread
From: Jan Kiszka @ 2010-02-05  8:26 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm, Gleb Natapov

[-- Attachment #1: Type: text/plain, Size: 1864 bytes --]

Avi Kivity wrote:
> 'mask' is always a constant, so we can check whether it includes a bit that
> might be owned by the guest very cheaply, and avoid the decache call.  Saves
> a few hundred bytes of module text.

-no-kvm-irqchip -smp 2 is broken for my Linux guests since this commit.
Their user space applications receive #UD and things fall apart.

Jan

> 
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  arch/x86/kvm/kvm_cache_regs.h |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/kvm_cache_regs.h b/arch/x86/kvm/kvm_cache_regs.h
> index 6b419a3..5a109c6 100644
> --- a/arch/x86/kvm/kvm_cache_regs.h
> +++ b/arch/x86/kvm/kvm_cache_regs.h
> @@ -1,6 +1,9 @@
>  #ifndef ASM_KVM_CACHE_REGS_H
>  #define ASM_KVM_CACHE_REGS_H
>  
> +#define KVM_POSSIBLE_CR0_GUEST_BITS X86_CR0_TS
> +#define KVM_POSSIBLE_CR4_GUEST_BITS X86_CR4_PGE
> +
>  static inline unsigned long kvm_register_read(struct kvm_vcpu *vcpu,
>  					      enum kvm_reg reg)
>  {
> @@ -40,7 +43,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)
> +	ulong tmask = mask & KVM_POSSIBLE_CR0_GUEST_BITS;
> +	if (tmask & vcpu->arch.cr0_guest_owned_bits)
>  		kvm_x86_ops->decache_cr0_guest_bits(vcpu);
>  	return vcpu->arch.cr0 & mask;
>  }
> @@ -52,7 +56,8 @@ static inline ulong kvm_read_cr0(struct kvm_vcpu *vcpu)
>  
>  static inline ulong kvm_read_cr4_bits(struct kvm_vcpu *vcpu, ulong mask)
>  {
> -	if (mask & vcpu->arch.cr4_guest_owned_bits)
> +	ulong tmask = mask & KVM_POSSIBLE_CR4_GUEST_BITS;
> +	if (tmask & vcpu->arch.cr4_guest_owned_bits)
>  		kvm_x86_ops->decache_cr4_guest_bits(vcpu);
>  	return vcpu->arch.cr4 & mask;
>  }



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

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

* Re: [PATCH 7/8] KVM: Optimize kvm_read_cr[04]_bits()
  2010-02-05  8:26   ` Jan Kiszka
@ 2010-02-07 12:05     ` Avi Kivity
  2010-02-07 12:33       ` Jan Kiszka
  0 siblings, 1 reply; 23+ messages in thread
From: Avi Kivity @ 2010-02-07 12:05 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Marcelo Tosatti, kvm, Gleb Natapov

On 02/05/2010 10:26 AM, Jan Kiszka wrote:
> Avi Kivity wrote:
>    
>> 'mask' is always a constant, so we can check whether it includes a bit that
>> might be owned by the guest very cheaply, and avoid the decache call.  Saves
>> a few hundred bytes of module text.
>>      
> -no-kvm-irqchip -smp 2 is broken for my Linux guests since this commit.
> Their user space applications receive #UD and things fall apart.
>
>    

Does 2c8232fc help?

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


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

* Re: [PATCH 3/8] KVM: Activate fpu on clts
  2010-02-04 17:56         ` Avi Kivity
@ 2010-02-07 12:25           ` Avi Kivity
  0 siblings, 0 replies; 23+ messages in thread
From: Avi Kivity @ 2010-02-07 12:25 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: Paolo Bonzini, Marcelo Tosatti, kvm

On 02/04/2010 07:56 PM, Avi Kivity wrote:
>> vmx.c doesn't initialize kvm_x86_ops->fpu_activate as far as I see.
>
>
> Gaak.  Well, that's obviously unintended.
>

I committed a patch to fix this.

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


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

* Re: [PATCH 7/8] KVM: Optimize kvm_read_cr[04]_bits()
  2010-02-07 12:05     ` Avi Kivity
@ 2010-02-07 12:33       ` Jan Kiszka
  2010-02-07 12:37         ` Avi Kivity
  0 siblings, 1 reply; 23+ messages in thread
From: Jan Kiszka @ 2010-02-07 12:33 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm, Gleb Natapov

[-- Attachment #1: Type: text/plain, Size: 539 bytes --]

Avi Kivity wrote:
> On 02/05/2010 10:26 AM, Jan Kiszka wrote:
>> Avi Kivity wrote:
>>   
>>> 'mask' is always a constant, so we can check whether it includes a
>>> bit that
>>> might be owned by the guest very cheaply, and avoid the decache
>>> call.  Saves
>>> a few hundred bytes of module text.
>>>      
>> -no-kvm-irqchip -smp 2 is broken for my Linux guests since this commit.
>> Their user space applications receive #UD and things fall apart.
>>
>>    
> 
> Does 2c8232fc help?
> 

Nope, unfortunately.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

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

* Re: [PATCH 7/8] KVM: Optimize kvm_read_cr[04]_bits()
  2010-02-07 12:33       ` Jan Kiszka
@ 2010-02-07 12:37         ` Avi Kivity
  2010-02-07 13:20           ` Jan Kiszka
  0 siblings, 1 reply; 23+ messages in thread
From: Avi Kivity @ 2010-02-07 12:37 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Marcelo Tosatti, kvm, Gleb Natapov

On 02/07/2010 02:33 PM, Jan Kiszka wrote:
> Avi Kivity wrote:
>    
>> On 02/05/2010 10:26 AM, Jan Kiszka wrote:
>>      
>>> Avi Kivity wrote:
>>>
>>>        
>>>> 'mask' is always a constant, so we can check whether it includes a
>>>> bit that
>>>> might be owned by the guest very cheaply, and avoid the decache
>>>> call.  Saves
>>>> a few hundred bytes of module text.
>>>>
>>>>          
>>> -no-kvm-irqchip -smp 2 is broken for my Linux guests since this commit.
>>> Their user space applications receive #UD and things fall apart.
>>>
>>>
>>>        
>> Does 2c8232fc help?
>>
>>      
> Nope, unfortunately.
>
>    

Which specific guests exhibit the behaviour?

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


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

* Re: [PATCH 7/8] KVM: Optimize kvm_read_cr[04]_bits()
  2010-02-07 12:37         ` Avi Kivity
@ 2010-02-07 13:20           ` Jan Kiszka
  0 siblings, 0 replies; 23+ messages in thread
From: Jan Kiszka @ 2010-02-07 13:20 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm, Gleb Natapov

[-- Attachment #1: Type: text/plain, Size: 845 bytes --]

Avi Kivity wrote:
> On 02/07/2010 02:33 PM, Jan Kiszka wrote:
>> Avi Kivity wrote:
>>   
>>> On 02/05/2010 10:26 AM, Jan Kiszka wrote:
>>>     
>>>> Avi Kivity wrote:
>>>>
>>>>       
>>>>> 'mask' is always a constant, so we can check whether it includes a
>>>>> bit that
>>>>> might be owned by the guest very cheaply, and avoid the decache
>>>>> call.  Saves
>>>>> a few hundred bytes of module text.
>>>>>
>>>>>          
>>>> -no-kvm-irqchip -smp 2 is broken for my Linux guests since this commit.
>>>> Their user space applications receive #UD and things fall apart.
>>>>
>>>>
>>>>        
>>> Does 2c8232fc help?
>>>
>>>      
>> Nope, unfortunately.
>>
>>    
> 
> Which specific guests exhibit the behaviour?
> 

64-bit Linux, various kernels from 2.6.27 to 32. x86-32 appears to be
unaffected.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

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

end of thread, other threads:[~2010-02-07 13:20 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-21 13:31 [PATCH 0/8] cr0/cr4/efer/fpu miscellaneous bits Avi Kivity
2010-01-21 13:31 ` [PATCH 1/8] KVM: Allow kvm_load_guest_fpu() even when !vcpu->fpu_active Avi Kivity
2010-01-21 13:31 ` [PATCH 2/8] KVM: Drop kvm_{load,put}_guest_fpu() exports Avi Kivity
2010-01-21 13:31 ` [PATCH 3/8] KVM: Activate fpu on clts Avi Kivity
2010-01-23 18:56   ` Marcelo Tosatti
2010-01-24  7:20     ` Avi Kivity
2010-02-02  8:16   ` Paolo Bonzini
2010-02-03 10:23     ` Gleb Natapov
2010-02-04 13:05     ` Avi Kivity
2010-02-04 13:11       ` Gleb Natapov
2010-02-04 17:56         ` Avi Kivity
2010-02-07 12:25           ` Avi Kivity
2010-01-21 13:31 ` [PATCH 4/8] KVM: Add a helper for checking if the guest is in protected mode Avi Kivity
2010-01-21 13:31 ` [PATCH 5/8] KVM: Move cr0/cr4/efer related helpers to x86.h Avi Kivity
2010-01-21 13:31 ` [PATCH 6/8] KVM: Rename vcpu->shadow_efer to efer Avi Kivity
2010-01-21 13:31 ` [PATCH 7/8] KVM: Optimize kvm_read_cr[04]_bits() Avi Kivity
2010-02-05  8:26   ` Jan Kiszka
2010-02-07 12:05     ` Avi Kivity
2010-02-07 12:33       ` Jan Kiszka
2010-02-07 12:37         ` Avi Kivity
2010-02-07 13:20           ` Jan Kiszka
2010-01-21 13:31 ` [PATCH 8/8] KVM: trace guest fpu loads and unloads Avi Kivity
2010-01-23 19:02 ` [PATCH 0/8] cr0/cr4/efer/fpu miscellaneous bits Marcelo Tosatti

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox