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

2010-05-17 Thread Sheng Yang
On Monday 17 May 2010 16:18:22 Avi Kivity wrote:
> On 05/17/2010 11:09 AM, Sheng Yang wrote:
> > Convert KVM to use generic FPU API.
> > 
> > Signed-off-by: Sheng Yang
> > ---
> > 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.
 
> 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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-05-17 Thread Avi Kivity

On 05/17/2010 11:09 AM, Sheng Yang wrote:

Convert KVM to use generic FPU API.

Signed-off-by: Sheng Yang
---
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.


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

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-05-17 Thread Sheng Yang
Convert KVM to use generic FPU API.

Signed-off-by: Sheng Yang 
---
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(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 0c06148..d08bb4a 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -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 */
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7500cba..7be1d36 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -52,6 +52,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #define MAX_IO_MSRS 256
 #define CR0_RESERVED_BITS  \
@@ -5057,27 +5059,6 @@ out:
 }
 
 /*
- * fxsave fpu state.  Taken from x86_64/processor.h.  To be killed when
- * we have asm/x86/processor.h
- */
-struct fxsave {
-   u16 cwd;
-   u16 swd;
-   u16 twd;
-   u16 fop;
-   u64 rip;
-   u64 rdp;
-   u32 mxcsr;
-   u32 mxcsr_mask;
-   u32 st_space[32];   /* 8*16 bytes for each FP-reg = 128 bytes */
-#ifdef CONFIG_X86_64
-   u32 xmm_space[64];  /* 16*16 bytes for each XMM-reg = 256 bytes */
-#else
-   u32 xmm_space[32];  /* 8*16 bytes for each XMM-reg = 128 bytes */
-#endif
-};
-
-/*
  * Translate a guest virtual address to a guest physical address.
  */
 int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
@@ -5100,7 +5081,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
 
 int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
 {
-   struct fxsave *fxsave = (struct fxsave *)&vcpu->arch.guest_fx_image;
+   struct i387_fxsave_struct *fxsave =
+   &vcpu->arch.guest_fpu.state->fxsave;
 
memcpy(fpu->fpr, fxsave->st_space, 128);
fpu->fcw = fxsave->cwd;
@@ -5116,7 +5098,8 @@ int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, 
struct kvm_fpu *fpu)
 
 int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
 {
-   struct fxsave *fxsave = (struct fxsave *)&vcpu->arch.guest_fx_image;
+   struct i387_fxsave_struct *fxsave =
+   &vcpu->arch.guest_fpu.state->fxsave;
 
memcpy(fxsave->st_space, fpu->fpr, 128);
fxsave->cwd = fpu->fcw;
@@ -5132,41 +5115,26 @@ int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, 
struct kvm_fpu *fpu)
 
 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();
+   fpu_alloc(&vcpu->arch.guest_fpu);
+   fpu_finit(&vcpu->arch.guest_fpu);
 
vcpu->arch.cr0 |= X86_CR0_ET;
-   after_mxcsr_mask = offsetof(struct i387_fxsave_struct, st_space);
-   vcpu->arch.guest_fx_image.mxcsr = 0x1f80;
-   memset((void *)&vcpu->arch.guest_fx_image + after_mxcsr_mask,
-  0, sizeof(struct i387_fxsave_struct) - after_mxcsr_mask);
 }
 EXPORT_SYMBOL_GPL(fx_init);
 
+static void fx_free(struct kvm_vcpu *vcpu)
+{
+   fpu_free(&vcpu->arch.guest_fpu);
+}
+
 void kvm_load_guest_fpu(struct kvm_vcpu *vcpu)
 {
if (vcpu->guest_fpu_loaded)
return;
 
vcpu->guest_fpu_loaded = 1;
-   kvm_fx_save(&vcpu->arch.host_fx_image);
-   kvm_fx_restore(&vcpu->arch.guest_fx_image);
+   unlazy_fpu(current);
+   fpu_restore_checking(&vcpu->arch.guest_fpu);
trace_kvm_fpu(1);
 }
 
@@ -5176,8 +5144,7 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
return;
 
vcpu->

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

2010-05-15 Thread Avi Kivity

On 05/14/2010 06:16 AM, Sheng Yang wrote:

Convert KVM to use generic FPU API.

Signed-off-by: Sheng Yang
---
  arch/x86/include/asm/kvm_host.h |   18 +-
  arch/x86/kvm/x86.c  |   73 ---
  2 files changed, 23 insertions(+), 68 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index ed48904..beba6f5 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -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).


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

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-05-13 Thread Sheng Yang
Convert KVM to use generic FPU API.

Signed-off-by: Sheng Yang 
---
 arch/x86/include/asm/kvm_host.h |   18 +-
 arch/x86/kvm/x86.c  |   73 ---
 2 files changed, 23 insertions(+), 68 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index ed48904..beba6f5 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -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 */
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index cd8a606..2313f76 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -52,6 +52,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #define MAX_IO_MSRS 256
 #define CR0_RESERVED_BITS  \
@@ -5069,27 +5071,6 @@ unlock_out:
 }
 
 /*
- * fxsave fpu state.  Taken from x86_64/processor.h.  To be killed when
- * we have asm/x86/processor.h
- */
-struct fxsave {
-   u16 cwd;
-   u16 swd;
-   u16 twd;
-   u16 fop;
-   u64 rip;
-   u64 rdp;
-   u32 mxcsr;
-   u32 mxcsr_mask;
-   u32 st_space[32];   /* 8*16 bytes for each FP-reg = 128 bytes */
-#ifdef CONFIG_X86_64
-   u32 xmm_space[64];  /* 16*16 bytes for each XMM-reg = 256 bytes */
-#else
-   u32 xmm_space[32];  /* 8*16 bytes for each XMM-reg = 128 bytes */
-#endif
-};
-
-/*
  * Translate a guest virtual address to a guest physical address.
  */
 int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
@@ -5114,7 +5095,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
 
 int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
 {
-   struct fxsave *fxsave = (struct fxsave *)&vcpu->arch.guest_fx_image;
+   struct i387_fxsave_struct *fxsave =
+   &vcpu->arch.guest_fpu.state->fxsave;
 
vcpu_load(vcpu);
 
@@ -5134,7 +5116,8 @@ int kvm_arch_vcpu_ioctl_get_fpu(struct kvm_vcpu *vcpu, 
struct kvm_fpu *fpu)
 
 int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, struct kvm_fpu *fpu)
 {
-   struct fxsave *fxsave = (struct fxsave *)&vcpu->arch.guest_fx_image;
+   struct i387_fxsave_struct *fxsave =
+   &vcpu->arch.guest_fpu.state->fxsave;
 
vcpu_load(vcpu);
 
@@ -5154,41 +5137,30 @@ int kvm_arch_vcpu_ioctl_set_fpu(struct kvm_vcpu *vcpu, 
struct kvm_fpu *fpu)
 
 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);
+   fpu_alloc(&vcpu->arch.host_fpu);
+   fpu_alloc(&vcpu->arch.guest_fpu);
 
-   /* 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();
+   fpu_save(&vcpu->arch.host_fpu);
+   fpu_finit(&vcpu->arch.guest_fpu);
 
vcpu->arch.cr0 |= X86_CR0_ET;
-   after_mxcsr_mask = offsetof(struct i387_fxsave_struct, st_space);
-   vcpu->arch.guest_fx_image.mxcsr = 0x1f80;
-   memset((void *)&vcpu->arch.guest_fx_image + after_mxcsr_mask,
-  0, sizeof(struct i387_fxsave_struct) - after_mxcsr_mask);
 }
 EXPORT_SYMBOL_GPL(fx_init);
 
+static void fx_free(struct kvm_vcpu *vcpu)
+{
+   fpu_free(&vcpu->arch.host_fpu);
+   fpu_free(&vcpu->arch.guest_fpu);
+}
+
 void kvm_load_guest_fpu(struct kvm_vcpu *vcpu)
 {
if (vcpu->guest_fpu_loaded)
return;
 
vcpu->guest_fpu_loaded = 1;
-   kvm_fx_save(&vcpu->arch.host_fx_image);
-   kvm_fx_restore(&vcpu->arch.guest_fx_image);
+   fpu_save(&vcpu->arch.host_fpu);
+   fpu_restore_checking(&vcpu->arch.guest_fpu);
trace_kvm_fpu(1);
 }
 
@@ -5198,8 +5170,8 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
return;