All of lore.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 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.