cr0/cr4/efer/fpu miscellaneous bits

PATCH Applied Avi Kivity 5 participants 23 messages Jan 21, 2010 – Feb 7, 2010

Subsystems (4)

x86 KVM Tracing KVM x86

lore

Share & tools

lore

Avi Kivity

author

2010-01-21 13:31 UTC #0 lore raw

cover cr0/cr4/efer/fpu miscellaneous bits

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(-)

Avi Kivity

author

2010-01-21 13:31 UTC #1 lore raw

PATCH 1/8 KVM: Allow kvm_load_guest_fpu() even when !vcpu->fpu_active

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 <[email protected]>

---
 arch/x86/kvm/x86.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

arch/x86/kvm/x86.c +3 -2

index 47c6e23..e3145d5 100644
@@ -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;

Signature · 1 line

1.6.5.3

Avi Kivity

author

2010-01-21 13:31 UTC #2 lore raw

PATCH 2/8 KVM: Drop kvm_{load,put}_guest_fpu() exports

In reply to Avi Kivity

Not used anymore.

Signed-off-by Avi Kivity <[email protected]>

---
 arch/x86/kvm/x86.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

arch/x86/kvm/x86.c +0 -2

index e3145d5..feca59f 100644
@@ -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)
 {

Signature · 1 line

1.6.5.3

Avi Kivity

author

2010-01-21 13:31 UTC #3 lore raw

PATCH 3/8 KVM: Activate fpu on clts

In reply to Avi Kivity

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 <[email protected]>

---
 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(-)

arch/x86/include/asm/kvm_host.h +1 -0

index a1f0b5d..bf3ec76 100644
@@ -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);

arch/x86/kvm/svm.c +7 -1

index 8d7cb62..0f3738a 100644
@@ -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,

arch/x86/kvm/vmx.c +1 -0

index 7375ae1..372bc38 100644
@@ -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) {

arch/x86/kvm/x86.c +1 -0

index feca59f..09207ba 100644
@@ -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;
 }

Signature · 1 line

1.6.5.3

Marcelo Tosatti

2010-01-23 19:36 UTC #4 lore raw

Re: [PATCH 3/8] KVM: Activate fpu on clts

Quoted 5 lines · Avi Kivity

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 <[email protected]>
---

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

Avi Kivity

author

2010-01-24 07:20 UTC #5 lore raw

Re: [PATCH 3/8] KVM: Activate fpu on clts

Quoted 11 lines · Marcelo Tosatti

Quoted 6 lines · Avi Kivity
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<[email protected]>
---
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.

Signature · 1 line

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

Paolo Bonzini

maintainer

2010-02-02 08:40 UTC #6 lore raw

Re: [PATCH 3/8] KVM: Activate fpu on clts

In reply to Avi Kivity

arch/x86/kvm/x86.c int emulate_invlpg(struct kvm_vcpu *vcpu, gva_t address) · line 3271

Quoted 11 lines · Avi Kivity

arch/x86/kvm/x86.c +1 -0

index feca59f..09207ba 100644
@@ -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

Gleb Natapov

2010-02-03 10:23 UTC #7 lore raw

Re: [PATCH 3/8] KVM: Activate fpu on clts

Quoted 16 lines · Paolo Bonzini

arch/x86/kvm/x86.c int emulate_invlpg(struct kvm_vcpu *vcpu, gva_t address) · line 3271
Quoted 11 lines · Avi Kivity
arch/x86/kvm/x86.c +1 -0
index feca59f..09207ba 100644
@@ -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.

Avi Kivity

author

2010-02-04 13:05 UTC #8 lore raw

Re: [PATCH 3/8] KVM: Activate fpu on clts

In reply to Paolo Bonzini

Quoted 15 lines · Paolo Bonzini

arch/x86/kvm/x86.c int emulate_invlpg(struct kvm_vcpu *vcpu, gva_t · line 3272
Quoted 12 lines · Avi Kivity
arch/x86/kvm/x86.c +1 -0
index feca59f..09207ba 100644
@@ -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?

Signature · 1 line

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

Gleb Natapov

2010-02-04 13:11 UTC #9 lore raw

Re: [PATCH 3/8] KVM: Activate fpu on clts

Quoted 18 lines · Avi Kivity

Quoted 16 lines · Paolo Bonzini
arch/x86/kvm/x86.c int emulate_invlpg(struct kvm_vcpu *vcpu, · line 3272
Quoted 12 lines · Avi Kivity
arch/x86/kvm/x86.c +1 -0
index feca59f..09207ba 100644
@@ -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.

Quoted 2 lines

How would this cause a null pointer access?

vmx.c doesn't initialize kvm_x86_ops->fpu_activate as far as I see.

--
Gleb.

Avi Kivity

author

2010-02-04 17:56 UTC #10 lore raw

Re: [PATCH 3/8] KVM: Activate fpu on clts

Quoted 22 lines · Gleb Natapov

Quoted 20 lines · Avi Kivity
Quoted 18 lines · Paolo Bonzini
arch/x86/kvm/x86.c int emulate_invlpg(struct kvm_vcpu *vcpu, · line 3272
Quoted 13 lines · Avi Kivity
arch/x86/kvm/x86.c +1 -0
index feca59f..09207ba 100644
@@ -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.

Quoted 5 lines

Quoted 3 lines
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.

Signature · 2 lines

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

Avi Kivity

author

2010-02-07 12:25 UTC #11 lore raw

Re: [PATCH 3/8] KVM: Activate fpu on clts

Quoted 5 lines · Avi Kivity

Quoted 1 line
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.

Signature · 1 line

error compiling committee.c: too many arguments to function

Avi Kivity

author

2010-01-21 13:31 UTC #12 lore raw

PATCH 4/8 KVM: Add a helper for checking if the guest is in protected mode

In reply to Avi Kivity

Signed-off-by Avi Kivity <[email protected]>

---
 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(-)

arch/x86/kvm/emulate.c +4 -5

index 0f89e32..e46f276 100644
@@ -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;
 	}

arch/x86/kvm/vmx.c +2 -2

index 372bc38..cd78049 100644
@@ -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))

arch/x86/kvm/x86.c +3 -4

index 09207ba..6cdead0 100644
@@ -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);

arch/x86/kvm/x86.h +6 -0

index 5eadea5..f783d8f 100644
@@ -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

Signature · 1 line

1.6.5.3

Avi Kivity

author

2010-01-21 13:31 UTC #13 lore raw

PATCH 5/8 KVM: Move cr0/cr4/efer related helpers to x86.h

In reply to Avi Kivity

They have more general scope than the mmu.

Signed-off-by Avi Kivity <[email protected]>

---
 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(-)

arch/x86/kvm/emulate.c +0 -1

index e46f276..a2adec8 100644
@@ -33,7 +33,6 @@
 #include <asm/kvm_emulate.h>
 
 #include "x86.h"
-#include "mmu.h"		/* for is_long_mode() */
 
 /*
  * Opcode effective-address decode tables.

arch/x86/kvm/mmu.c +1 -0

index ff2b2e8..6f7158f 100644
@@ -18,6 +18,7 @@
  */
 
 #include "mmu.h"
+#include "x86.h"
 #include "kvm_cache_regs.h"
 
 #include <linux/kvm_host.h>

arch/x86/kvm/mmu.h +0 ~~-24~~

index 599159f..61ef5a6 100644
@@ -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;

arch/x86/kvm/x86.h +24 -0

index f783d8f..2dc24a7 100644
@@ -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

Signature · 1 line

1.6.5.3

Avi Kivity

author

2010-01-21 13:31 UTC #14 lore raw

PATCH 6/8 KVM: Rename vcpu->shadow_efer to efer

In reply to Avi Kivity

None of the other registers have the shadow_ prefix.

Signed-off-by Avi Kivity <[email protected]>

---
 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(-)

arch/x86/include/asm/kvm_host.h +1 -1

index bf3ec76..76bf686 100644
@@ -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;

arch/x86/kvm/mmu.c +1 -1

index 6f7158f..599c422 100644
@@ -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)

arch/x86/kvm/svm.c +6 -6

index 0f3738a..0242fdd 100644
@@ -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;

arch/x86/kvm/vmx.c +7 -7

index cd78049..d4a6260 100644
@@ -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))

arch/x86/kvm/x86.c +7 -7

index 6cdead0..8b42c19 100644
@@ -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);

arch/x86/kvm/x86.h +1 -1

index 2dc24a7..2d10163 100644
@@ -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

Signature · 1 line

1.6.5.3

Avi Kivity

author

2010-01-21 13:31 UTC #15 lore raw

PATCH 7/8 KVM: Optimize kvm_read_cr[04]_bits()

In reply to Avi Kivity

'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 <[email protected]>

---
 arch/x86/kvm/kvm_cache_regs.h |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

arch/x86/kvm/kvm_cache_regs.h +7 -2

index 6b419a3..5a109c6 100644
@@ -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;
 }

Signature · 1 line

1.6.5.3

Jan Kiszka

2010-02-05 08:26 UTC #16 lore raw

Re: [PATCH 7/8] KVM: Optimize kvm_read_cr[04]_bits()

Quoted 3 lines · Avi Kivity

'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

Quoted 40 lines

Signed-off-by Avi Kivity <[email protected]>

---
 arch/x86/kvm/kvm_cache_regs.h |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

arch/x86/kvm/kvm_cache_regs.h +7 -2

index 6b419a3..5a109c6 100644
@@ -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;
 }

Avi Kivity

author

2010-02-07 12:05 UTC #17 lore raw

Re: [PATCH 7/8] KVM: Optimize kvm_read_cr[04]_bits()

Quoted 10 lines · Jan Kiszka

Quoted 4 lines · Avi Kivity
'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?

Signature · 1 line

error compiling committee.c: too many arguments to function

Jan Kiszka

2010-02-07 12:33 UTC #18 lore raw

Re: [PATCH 7/8] KVM: Optimize kvm_read_cr[04]_bits()

Quoted 9 lines · Avi Kivity

Quoted 8 lines · Jan Kiszka
Quoted 6 lines · Avi Kivity
'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.

Quoted 7 lines

Quoted 4 lines
-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

Avi Kivity

author

2010-02-07 12:37 UTC #19 lore raw

Re: [PATCH 7/8] KVM: Optimize kvm_read_cr[04]_bits()

Quoted 19 lines · Jan Kiszka

Quoted 17 lines · Avi Kivity
Quoted 15 lines · Jan Kiszka
Quoted 7 lines · Avi Kivity
'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.

Quoted 6 lines

Quoted 3 lines
Does 2c8232fc help?
Nope, unfortunately.

Which specific guests exhibit the behaviour?

Signature · 1 line

error compiling committee.c: too many arguments to function

Jan Kiszka

2010-02-07 13:20 UTC #20 lore raw

Re: [PATCH 7/8] KVM: Optimize kvm_read_cr[04]_bits()

Quoted 20 lines · Avi Kivity

Quoted 19 lines · Jan Kiszka
Quoted 17 lines · Avi Kivity
Quoted 15 lines · Jan Kiszka
Quoted 7 lines · Avi Kivity
'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.

Quoted 9 lines

Quoted 6 lines
Quoted 3 lines
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

Avi Kivity

author

2010-01-21 13:31 UTC #21 lore raw

PATCH 8/8 KVM: trace guest fpu loads and unloads

In reply to Avi Kivity

Signed-off-by Avi Kivity <[email protected]>

---
 arch/x86/kvm/x86.c         |    2 ++
 include/trace/events/kvm.h |   19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 0 deletions(-)

arch/x86/kvm/x86.c +2 -0

index 8b42c19..06a03c1 100644
@@ -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)

include/trace/events/kvm.h +19 -0

index dbe1084..8abdc12 100644
@@ -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 */

Signature · 1 line

1.6.5.3

Marcelo Tosatti

Applied

2010-01-23 19:36 UTC #22 lore raw

In reply to Avi Kivity

Quoted 12 lines · Avi Kivity

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.