Convert KVM to use FPU API

PATCH In review Sheng Yang 2 participants 11 messages May 17, 2010 – May 19, 2010

Subsystems (3)

x86 KVM KVM x86

lore

Share & tools

lore

Sheng Yang

author

2010-05-17 09:08 UTC #0 lore raw

v2 cover Convert KVM to use FPU API

Change from v1:
Use unlazy_fpu() to handle host FPU, avoiding save/restore of host FPU states.

Sheng Yang (3):
x86: Export FPU API for KVM use
KVM: x86: Use unlazy_fpu() for host FPU
KVM: x86: Use FPU API

 arch/x86/include/asm/i387.h     |    2 +
 arch/x86/include/asm/kvm_host.h |   18 +---------
 arch/x86/include/asm/xsave.h    |    3 ++
 arch/x86/kernel/i387.c          |    3 +-
 arch/x86/kernel/process.c       |    1 +
 arch/x86/kvm/x86.c              |   70 ++++++++++-----------------------------
 6 files changed, 27 insertions(+), 70 deletions(-)

Sheng Yang

author

2010-05-17 09:08 UTC #1 lore raw

PATCH v2 1/3 x86: Export FPU API for KVM use

Also add some constants.

Signed-off-by Sheng Yang <[email protected]>

---
 arch/x86/include/asm/i387.h  |    2 ++
 arch/x86/include/asm/xsave.h |    3 +++
 arch/x86/kernel/i387.c       |    3 ++-
 arch/x86/kernel/process.c    |    1 +
 4 files changed, 8 insertions(+), 1 deletions(-)

arch/x86/include/asm/i387.h +2 -0

index 8002e9c..9610628 100644
@@ -488,6 +488,8 @@ static inline void fpu_copy(struct fpu *dst, struct fpu *src)
 	memcpy(dst->state, src->state, xstate_size);
 }
 
+extern void fpu_finit(struct fpu *fpu);
+
 #endif /* __ASSEMBLY__ */
 
 #define PSHUFB_XMM5_XMM0 .byte 0x66, 0x0f, 0x38, 0x00, 0xc5

arch/x86/include/asm/xsave.h +3 -0

index 2c4390c..29ee4e4 100644
@@ -13,6 +13,9 @@
 
 #define FXSAVE_SIZE	512
 
+#define XSTATE_YMM_SIZE 256
+#define XSTATE_YMM_OFFSET (512 + 64)
+
 /*
  * These are the features that the OS can handle currently.
  */

arch/x86/kernel/i387.c +2 -1

index 86cef6b..c4444bc 100644
@@ -107,7 +107,7 @@ void __cpuinit fpu_init(void)
 }
 #endif	/* CONFIG_X86_64 */
 
-static void fpu_finit(struct fpu *fpu)
+void fpu_finit(struct fpu *fpu)
 {
 #ifdef CONFIG_X86_32
 	if (!HAVE_HWFP) {

@@ -132,6 +132,7 @@ static void fpu_finit(struct fpu *fpu)
 		fp->fos = 0xffff0000u;
 	}
 }
+EXPORT_SYMBOL_GPL(fpu_finit);
 
 /*
  * The _current_ task is using the FPU for the first time

arch/x86/kernel/process.c +1 -0

index 8bcc21f..373fec9 100644
@@ -28,6 +28,7 @@ unsigned long idle_nomwait;
 EXPORT_SYMBOL(idle_nomwait);
 
 struct kmem_cache *task_xstate_cachep;
+EXPORT_SYMBOL(task_xstate_cachep);
 
 int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
 {

Signature · 1 line

1.7.0.1

Avi Kivity

2010-05-17 09:19 UTC #2 lore raw

Re: [PATCH][v2 1/3] x86: Export FPU API for KVM use

arch/x86/kernel/process.c unsigned long idle_nomwait; · line 34

Quoted 13 lines · Sheng Yang

Also add some constants.

+++ b/arch/x86/kernel/process.c

arch/x86/kernel/process.c +1 -0

@@ -28,6 +28,7 @@ unsigned long idle_nomwait;
  EXPORT_SYMBOL(idle_nomwait);
 
  struct kmem_cache *task_xstate_cachep;
+EXPORT_SYMBOL(task_xstate_cachep);
 
  int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
  {

_GPL() unless good reason not to.

Signature · 1 line

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

Sheng Yang

author

2010-05-17 09:19 UTC #3 lore raw

Re: [PATCH][v2 1/3] x86: Export FPU API for KVM use

Quoted 9 lines · On Monday 17 May 2010 17:19:02 Avi Kivity wrote:

Quoted 8 lines · Sheng Yang
Also add some constants.
+++ b/arch/x86/kernel/process.c
arch/x86/kernel/process.c +0 -0
@@ -28,6 +28,7 @@ unsigned long idle_nomwait;
 
  EXPORT_SYMBOL(idle_nomwait);
 

Quoted 8 lines

Quoted 6 lines

arch/x86/kernel/process.c +1 -0

  struct kmem_cache *task_xstate_cachep;
 
+EXPORT_SYMBOL(task_xstate_cachep);

  int arch_dup_task_struct(struct task_struct *dst, struct task_struct
  *src) {

_GPL() unless good reason not to.

Oops... Just copied and pasted the above line...

--
regards
Yang, Sheng

Avi Kivity

2010-05-17 09:24 UTC #4 lore raw

Re: [PATCH][v2 1/3] x86: Export FPU API for KVM use

Quoted 14 lines · Sheng Yang

Quoted 12 lines · On Monday 17 May 2010 17:19:02 Avi Kivity wrote:
Quoted 10 lines · Sheng Yang
Also add some constants.
+++ b/arch/x86/kernel/process.c
arch/x86/kernel/process.c +0 -0
@@ -28,6 +28,7 @@ unsigned long idle_nomwait;
 
   EXPORT_SYMBOL(idle_nomwait);
 
   struct kmem_cache *task_xstate_cachep;
 

Quoted 10 lines

Quoted 7 lines
Quoted 5 lines
arch/x86/kernel/process.c +1 -0
+EXPORT_SYMBOL(task_xstate_cachep);
 
   int arch_dup_task_struct(struct task_struct *dst, struct task_struct
   *src) {
_GPL() unless good reason not to.
Oops... Just copied and pasted the above line...

No matter, I'll update it when applying if no further comments (reviewing 3 now).

Signature · 1 line

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

Sheng Yang

author

2010-05-17 09:22 UTC #5 lore raw

PATCH v2 1/3 x86: Export FPU API for KVM use

In reply to Avi Kivity

Also add some constants.

Signed-off-by Sheng Yang <[email protected]>

---
 arch/x86/include/asm/i387.h  |    2 ++
 arch/x86/include/asm/xsave.h |    3 +++
 arch/x86/kernel/i387.c       |    3 ++-
 arch/x86/kernel/process.c    |    1 +
 4 files changed, 8 insertions(+), 1 deletions(-)

arch/x86/include/asm/i387.h +2 -0

index 8002e9c..9610628 100644
@@ -488,6 +488,8 @@ static inline void fpu_copy(struct fpu *dst, struct fpu *src)
 	memcpy(dst->state, src->state, xstate_size);
 }
 
+extern void fpu_finit(struct fpu *fpu);
+
 #endif /* __ASSEMBLY__ */
 
 #define PSHUFB_XMM5_XMM0 .byte 0x66, 0x0f, 0x38, 0x00, 0xc5

arch/x86/include/asm/xsave.h +3 -0

index 2c4390c..29ee4e4 100644
@@ -13,6 +13,9 @@
 
 #define FXSAVE_SIZE	512
 
+#define XSTATE_YMM_SIZE 256
+#define XSTATE_YMM_OFFSET (512 + 64)
+
 /*
  * These are the features that the OS can handle currently.
  */

arch/x86/kernel/i387.c +2 -1

index 86cef6b..c4444bc 100644
@@ -107,7 +107,7 @@ void __cpuinit fpu_init(void)
 }
 #endif	/* CONFIG_X86_64 */
 
-static void fpu_finit(struct fpu *fpu)
+void fpu_finit(struct fpu *fpu)
 {
 #ifdef CONFIG_X86_32
 	if (!HAVE_HWFP) {

@@ -132,6 +132,7 @@ static void fpu_finit(struct fpu *fpu)
 		fp->fos = 0xffff0000u;
 	}
 }
+EXPORT_SYMBOL_GPL(fpu_finit);
 
 /*
  * The _current_ task is using the FPU for the first time

arch/x86/kernel/process.c +1 -0

index 8bcc21f..0f331f4 100644
@@ -28,6 +28,7 @@ unsigned long idle_nomwait;
 EXPORT_SYMBOL(idle_nomwait);
 
 struct kmem_cache *task_xstate_cachep;
+EXPORT_SYMBOL_GPL(task_xstate_cachep);
 
 int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
 {

Signature · 1 line

1.7.0.1

Sheng Yang

author

2010-05-17 09:08 UTC #6 lore raw

PATCH v2 2/3 KVM: x86: Use unlazy_fpu() for host FPU

In reply to Sheng Yang

We can avoid unnecessary fpu load when userspace process didn't use FPU frequently.

Derived from Avi's idea.

Signed-off-by Sheng Yang <[email protected]>

---
 arch/x86/include/asm/kvm_host.h |    1 -
 arch/x86/kvm/x86.c              |   18 ++----------------
 2 files changed, 2 insertions(+), 17 deletions(-)

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

index 0c06148..d93601c 100644
@@ -301,7 +301,6 @@ struct kvm_vcpu_arch {
 		unsigned long mmu_seq;
 	} update_pte;
 
-	struct i387_fxsave_struct host_fx_image;
 	struct i387_fxsave_struct guest_fx_image;
 
 	gva_t mmio_fault_cr2;

arch/x86/kvm/x86.c +2 ~~-16~~

index 7500cba..ba8c2d2 100644
@@ -52,6 +52,7 @@
 #include <asm/desc.h>
 #include <asm/mtrr.h>
 #include <asm/mce.h>
+#include <asm/i387.h>
 
 #define MAX_IO_MSRS 256
 #define CR0_RESERVED_BITS						\

@@ -5134,21 +5135,10 @@ void fx_init(struct kvm_vcpu *vcpu)
 {
 	unsigned after_mxcsr_mask;
 
-	/*
-	 * Touch the fpu the first time in non atomic context as if
-	 * this is the first fpu instruction the exception handler
-	 * will fire before the instruction returns and it'll have to
-	 * allocate ram with GFP_KERNEL.
-	 */
-	if (!used_math())
-		kvm_fx_save(&vcpu->arch.host_fx_image);
-
 	/* Initialize guest FPU by resetting ours and saving into guest's */
 	preempt_disable();
-	kvm_fx_save(&vcpu->arch.host_fx_image);
 	kvm_fx_finit();
 	kvm_fx_save(&vcpu->arch.guest_fx_image);
-	kvm_fx_restore(&vcpu->arch.host_fx_image);
 	preempt_enable();
 
 	vcpu->arch.cr0 |= X86_CR0_ET;

@@ -5165,7 +5155,7 @@ void kvm_load_guest_fpu(struct kvm_vcpu *vcpu)
 		return;
 
 	vcpu->guest_fpu_loaded = 1;
-	kvm_fx_save(&vcpu->arch.host_fx_image);
+	unlazy_fpu(current);
 	kvm_fx_restore(&vcpu->arch.guest_fx_image);
 	trace_kvm_fpu(1);
 }

@@ -5177,7 +5167,6 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
 
 	vcpu->guest_fpu_loaded = 0;
 	kvm_fx_save(&vcpu->arch.guest_fx_image);
-	kvm_fx_restore(&vcpu->arch.host_fx_image);
 	++vcpu->stat.fpu_reload;
 	set_bit(KVM_REQ_DEACTIVATE_FPU, &vcpu->requests);
 	trace_kvm_fpu(0);

@@ -5203,9 +5192,6 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
 {
 	int r;
 
-	/* We do fxsave: this must be aligned. */
-	BUG_ON((unsigned long)&vcpu->arch.host_fx_image & 0xF);
-
 	vcpu->arch.mtrr_state.have_fixed = 1;
 	vcpu_load(vcpu);
 	r = kvm_arch_vcpu_reset(vcpu);

Signature · 1 line

1.7.0.1

Sheng Yang

author

2010-05-17 09:08 UTC #7 lore raw

PATCH v2 3/3 KVM: x86: Use FPU API

In reply to Sheng Yang

Convert KVM to use generic FPU API.

Signed-off-by Sheng Yang <[email protected]>

---
 arch/x86/include/asm/kvm_host.h |   17 +------------
 arch/x86/kvm/x86.c              |   52 ++++++++++++---------------------------
 2 files changed, 17 insertions(+), 52 deletions(-)

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

index d93601c..d08bb4a 100644
@@ -301,7 +301,7 @@ struct kvm_vcpu_arch {
 		unsigned long mmu_seq;
 	} update_pte;
 
-	struct i387_fxsave_struct guest_fx_image;
+	struct fpu guest_fpu;
 
 	gva_t mmio_fault_cr2;
 	struct kvm_pio_request pio;

@@ -708,21 +708,6 @@ static inline unsigned long read_msr(unsigned long msr)
 }
 #endif
 
-static inline void kvm_fx_save(struct i387_fxsave_struct *image)
-{
-	asm("fxsave (%0)":: "r" (image));
-}
-
-static inline void kvm_fx_restore(struct i387_fxsave_struct *image)
-{
-	asm("fxrstor (%0)":: "r" (image));
-}
-
-static inline void kvm_fx_finit(void)
-{
-	asm("finit");
-}
-
 static inline u32 get_rdx_init_val(void)
 {
 	return 0x600; /* P6 family */

arch/x86/kvm/x86.c +16 ~~-36~~

111 lines not on this page · raw message

Signature · 1 line

1.7.0.1

Avi Kivity

2010-05-17 09:37 UTC #8 lore raw

Re: [PATCH][v2 3/3] KVM: x86: Use FPU API

Quoted 6 lines · Sheng Yang

Convert KVM to use generic FPU API.

  void kvm_load_guest_fpu(struct kvm_vcpu *vcpu)
  {
  	if (vcpu->guest_fpu_loaded)

arch/x86/kvm/x86.c void kvm_load_guest_fpu(struct kvm_vcpu *vcpu) · line 5140

Quoted 8 lines

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

@@ -5156,7 +5134,7 @@ void kvm_load_guest_fpu(struct kvm_vcpu *vcpu)
 
  	vcpu->guest_fpu_loaded = 1;
  	unlazy_fpu(current);
-	kvm_fx_restore(&vcpu->arch.guest_fx_image);
+	fpu_restore_checking(&vcpu->arch.guest_fpu);
  	trace_kvm_fpu(1);
  }

Do we need to do something on error here?

I think an error can only occur if userspace sets bad fpu state, so maybe do the check in the ioctl.

In any case, unrelated to this patch.

Signature · 1 line

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

Avi Kivity

2010-05-17 09:37 UTC #9 lore raw

In reply to Sheng Yang

Quoted 4 lines · Sheng Yang

Change from v1:
Use unlazy_fpu() to handle host FPU, avoiding save/restore of host FPU states.

Looks good, will wait a bit for more reviews and apply.

Signature · 1 line

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

Avi Kivity

2010-05-19 16:17 UTC #10 lore raw

Quoted 7 lines · Avi Kivity

Quoted 3 lines · Sheng Yang
Change from v1:
Use unlazy_fpu() to handle host FPU, avoiding save/restore of host FPU states.
Looks good, will wait a bit for more reviews and apply.

Now applied. Thanks.

Signature · 1 line

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