Skip to contentSkip to conversation

Convert KVM to use FPU API

PATCH Superseded Sheng Yang 2 participants 8 messages May 14, 2010 – May 17, 2010

Subsystems (3)
x86 KVM KVM x86

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Sheng Yang

author
#0 lore raw

cover Convert KVM to use FPU API

This patchset based on Avi's FPU API patchset.

Sheng Yang (3):
x86: Split fpu_save_init() to fpu_save() and fpu_clear()
x86: Export FPU API for KVM use
KVM: x86: Use FPU API

 arch/x86/include/asm/i387.h     |   73 ++++++++++++++++++++++++---------------
 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              |   73 ++++++++++++---------------------------
 6 files changed, 74 insertions(+), 97 deletions(-)
author
#1 lore raw

PATCH 1/3 x86: Split fpu_save_init() to fpu_save() and fpu_clear()

fpu_save() would be used later by KVM.

Signed-off-by Sheng Yang <[email protected]>
---
 arch/x86/include/asm/i387.h |   71 ++++++++++++++++++++++++++-----------------
 1 files changed, 43 insertions(+), 28 deletions(-)
arch/x86/include/asm/i387.h +43 -28
index 1a8cca3..989d3b7 100644
@@ -180,13 +180,17 @@ static inline void fpu_fxsave(struct fpu *fpu)
 #endif
 }
 
-static inline void fpu_save_init(struct fpu *fpu)
+static inline void fpu_save(struct fpu *fpu)
 {
 	if (use_xsave())
 		fpu_xsave(fpu);
 	else
 		fpu_fxsave(fpu);
+}
 
+static inline void fpu_save_init(struct fpu *fpu)
+{
+	fpu_save(fpu);
 	fpu_clear(fpu);
 }
 

@@ -237,19 +241,41 @@ static inline int fxrstor_checking(struct i387_fxsave_struct *fx)
 /*
  * These must be called with preempt disabled
  */
-static inline void fpu_save_init(struct fpu *fpu)
+static inline void fpu_fxsave(struct fpu *fpu)
 {
-	if (use_xsave()) {
-		struct xsave_struct *xstate = &fpu->state->xsave;
-		struct i387_fxsave_struct *fx = &fpu->state->fxsave;
+	/* Use more nops than strictly needed in case the compiler
+	   varies code */
+	alternative_input(
+		"fnsave %[fx] ;fwait;" GENERIC_NOP8 GENERIC_NOP4,
+		"fxsave %[fx]\n"
+		"bt $7,%[fsw] ; jnc 1f ; fnclex\n1:",
+		X86_FEATURE_FXSR,
+		[fx] "m" (fpu->state->fxsave),
+		[fsw] "m" (fpu->state->fxsave.swd) : "memory");
+}
 
+static inline void fpu_save(struct fpu *fpu)
+{
+	if (use_xsave())
 		fpu_xsave(fpu);
+	else
+		fpu_fxsave(fpu);
+}
+
+/* AMD K7/K8 CPUs don't save/restore FDP/FIP/FOP unless an exception
+   is pending.  Clear the x87 state here by setting it to fixed
+   values. safe_address is a random variable that should be in L1 */
+static inline void fpu_clear(struct fpu *fpu)
+{
+	struct xsave_struct *xstate = &fpu->state->xsave;
+	struct i387_fxsave_struct *fx = &fpu->state->fxsave;
 
+	if (use_xsave()) {
 		/*
 		 * xsave header may indicate the init state of the FP.
 		 */
 		if (!(xstate->xsave_hdr.xstate_bv & XSTATE_FP))
-			goto end;
+			return;
 
 		if (unlikely(fx->swd & X87_FSW_ES))
 			asm volatile("fnclex");

@@ -257,30 +283,19 @@ static inline void fpu_save_init(struct fpu *fpu)
 		/*
 		 * we can do a simple return here or be paranoid :)
 		 */
-		goto clear_state;
 	}
+ 	alternative_input(
+ 		GENERIC_NOP8 GENERIC_NOP2,
+ 		"emms\n\t"	  	/* clear stack tags */
+ 		"fildl %[addr]", 	/* set F?P to defined value */
+ 		X86_FEATURE_FXSAVE_LEAK,
+ 		[addr] "m" (safe_address));
+}
 
-	/* Use more nops than strictly needed in case the compiler
-	   varies code */
-	alternative_input(
-		"fnsave %[fx] ;fwait;" GENERIC_NOP8 GENERIC_NOP4,
-		"fxsave %[fx]\n"
-		"bt $7,%[fsw] ; jnc 1f ; fnclex\n1:",
-		X86_FEATURE_FXSR,
-		[fx] "m" (fpu->state->fxsave),
-		[fsw] "m" (fpu->state->fxsave.swd) : "memory");
-clear_state:
-	/* AMD K7/K8 CPUs don't save/restore FDP/FIP/FOP unless an exception
-	   is pending.  Clear the x87 state here by setting it to fixed
-	   values. safe_address is a random variable that should be in L1 */
-	alternative_input(
-		GENERIC_NOP8 GENERIC_NOP2,
-		"emms\n\t"	  	/* clear stack tags */
-		"fildl %[addr]", 	/* set F?P to defined value */
-		X86_FEATURE_FXSAVE_LEAK,
-		[addr] "m" (safe_address));
-end:
-	;
+static inline void fpu_save_init(struct fpu *fpu)
+{
+	fpu_save(fpu);
+	fpu_clear(fpu);
 }
 
 static inline void __save_init_fpu(struct task_struct *tsk)
Signature · 1 line
1.7.0.1
author
#2 lore raw

PATCH 2/3 x86: Export FPU API for KVM use

In reply to Sheng Yang

Also add some more constants.

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

---

Do we need to rename "task_xstate_cachep"? It's not only for task struct now.

 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 989d3b7..f19bdf9 100644
@@ -503,6 +503,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
author
#3 lore raw

PATCH 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 |   18 +---------
 arch/x86/kvm/x86.c              |   73 ++++++++++++---------------------------
 2 files changed, 23 insertions(+), 68 deletions(-)
arch/x86/include/asm/kvm_host.h +1 -17
index ed48904..beba6f5 100644
@@ -300,8 +300,7 @@ struct kvm_vcpu_arch {
 		unsigned long mmu_seq;
 	} update_pte;
 
-	struct i387_fxsave_struct host_fx_image;
-	struct i387_fxsave_struct guest_fx_image;
+	struct fpu host_fpu, guest_fpu;
 
 	gva_t mmio_fault_cr2;
 	struct kvm_pio_request pio;

@@ -709,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 +22 -51

139 lines not on this page · raw message

Signature · 1 line
1.7.0.1
#4 lore raw

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

Quoted 13 lines · Sheng Yang

Convert KVM to use generic FPU API.

Signed-off-by Sheng Yang<[email protected]>
---
  arch/x86/include/asm/kvm_host.h |   18 +---------
  arch/x86/kvm/x86.c              |   73 ++++++++++++---------------------------
  2 files changed, 23 insertions(+), 68 deletions(-)
arch/x86/include/asm/kvm_host.h +0 -0
index ed48904..beba6f5 100644

arch/x86/include/asm/kvm_host.h struct kvm_vcpu_arch { · line 305

Quoted 8 lines
arch/x86/include/asm/kvm_host.h +1 -2
@@ -300,8 +300,7 @@ struct kvm_vcpu_arch {
  		unsigned long mmu_seq;
  	} update_pte;
 
-	struct i387_fxsave_struct host_fx_image;
-	struct i387_fxsave_struct guest_fx_image;
+	struct fpu host_fpu, guest_fpu;

Do we really need host_fpu? I think we can call unlazy_fpu() instead and drop host_fpu completely. This might reduce the need for changes in patch 1 (as well as generally improving the code).

Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
author
#5 lore raw

PATCH 3/3 KVM: x86: Use FPU API

Convert KVM to use generic FPU API.

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

---
Like this? (Drop patch 1)

 arch/x86/include/asm/kvm_host.h |   18 +---------
 arch/x86/kvm/x86.c              |   70 ++++++++++-----------------------------
 2 files changed, 19 insertions(+), 69 deletions(-)
arch/x86/include/asm/kvm_host.h +1 -17
index 0c06148..d08bb4a 100644
@@ -301,8 +301,7 @@ struct kvm_vcpu_arch {
 		unsigned long mmu_seq;
 	} update_pte;
 
-	struct i387_fxsave_struct host_fx_image;
-	struct i387_fxsave_struct guest_fx_image;
+	struct fpu guest_fpu;
 
 	gva_t mmio_fault_cr2;
 	struct kvm_pio_request pio;

@@ -709,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 +18 -52

135 lines not on this page · raw message

Signature · 1 line
1.7.0.1
#6 lore raw

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

Quoted 7 lines · Sheng Yang

Convert KVM to use generic FPU API.

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

---
Like this? (Drop patch 1)

Will be more readable with a patch that converts host_fx_image to unlazy_fpu(), and a second patch that converts guest_fx_image to the fpu API.

I think unlazy_fpu() is even a performance win in case userspace doesn't do a lot of floating point (which is the case with qemu). I wonder why we didn't think of it before.

Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
author
#7 lore raw

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

Quoted 10 lines · On Monday 17 May 2010 16:18:22 Avi Kivity wrote:
Quoted 5 lines · Sheng Yang

Convert KVM to use generic FPU API.

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

---
Like this? (Drop patch 1)

Will be more readable with a patch that converts host_fx_image to unlazy_fpu(), and a second patch that converts guest_fx_image to the fpu API.

OK.

Quoted 3 lines

I think unlazy_fpu() is even a performance win in case userspace doesn't do a lot of floating point (which is the case with qemu). I wonder why we didn't think of it before.

...
--
regards
Yang, Sheng