KVM: SVM: Disable Lazy FPU optimization because of regressions

KVM: SVM: Disable Lazy FPU optimization because of regressions

[Amit Shah]

>From 76204eda7e03035c16702105e78724137ecad24b Mon Sep 17 00:00:00 2001
From: Amit Shah <amit.shah-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
Date: Sun, 18 Nov 2007 22:42:47 +0530
Subject: [PATCH] KVM: SVM: Disable Lazy FPU optimization because of 
regressions

Host FPU state is leaked into the guest FPU state. This happens
because of the lazy FPU optimization, so just reload the FPU
each time there's a VM exit/entry.

The real fix should follow soon.

This is observed in cases where fonts in a guest aren't rendered
correctly (bug 1807560).

The test program, courtesy Avi is:

double test_fpu_once()
{
    int i;
    double f = 0;

    for (i = 0; i < 10000000; ++i)
        f += 1 / (1.0 + i);
    return f;
}

void test_fpu()
{
    double a, b;
    int runs;

    runs = 0;
    a = test_fpu_once();
    while (1) {
        b = test_fpu_once();
        if (fabs(a - b) > 1e-9)
            printf("error: %20.16f -> %20.16f\n", a, b);
        a = b;
        if (++runs % 100 == 0)
            printf("runs: %8d\n", runs);
    }
}

int main(int ac, char **av)
{
    test_fpu();
    return 0;
}

Run this on the host and the guest, both pinned to the same
host CPU.

Signed-off-by: Amit Shah <amit.shah-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
---
 drivers/kvm/svm.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
index ea2cb83..d7c9b92 100644
--- a/drivers/kvm/svm.c
+++ b/drivers/kvm/svm.c
@@ -1600,11 +1600,11 @@ static void svm_set_cr3(struct kvm_vcpu *vcpu, 
unsigned long root)
 	svm->vmcb->save.cr3 = root;
 	force_new_asid(vcpu);
 
-	if (vcpu->fpu_active) {
-		svm->vmcb->control.intercept_exceptions |= (1 << NM_VECTOR);
-		svm->vmcb->save.cr0 |= X86_CR0_TS;
-		vcpu->fpu_active = 0;
-	}
+/* 	if (vcpu->fpu_active) { */
+/* 		svm->vmcb->control.intercept_exceptions |= (1 << NM_VECTOR); */
+/* 		svm->vmcb->save.cr0 |= X86_CR0_TS; */
+/* 		vcpu->fpu_active = 0; */
+/* 	} */
 }
 
 static void svm_inject_page_fault(struct kvm_vcpu *vcpu,
-- 
1.4.4.2


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: KVM: SVM: Disable Lazy FPU optimization because of regressions

[Avi Kivity]

Amit Shah wrote:
> >From 76204eda7e03035c16702105e78724137ecad24b Mon Sep 17 00:00:00 2001
> From: Amit Shah <amit.shah-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
> Date: Sun, 18 Nov 2007 22:42:47 +0530
> Subject: [PATCH] KVM: SVM: Disable Lazy FPU optimization because of 
> regressions
>
> Host FPU state is leaked into the guest FPU state. This happens
> because of the lazy FPU optimization, so just reload the FPU
> each time there's a VM exit/entry.
>
> The real fix should follow soon.
>
>   
Applied, thanks, and:

> diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
> index ea2cb83..d7c9b92 100644
> --- a/drivers/kvm/svm.c
> +++ b/drivers/kvm/svm.c
> @@ -1600,11 +1600,11 @@ static void svm_set_cr3(struct kvm_vcpu *vcpu, 
> unsigned long root)
>  	svm->vmcb->save.cr3 = root;
>  	force_new_asid(vcpu);
>  
> -	if (vcpu->fpu_active) {
> -		svm->vmcb->control.intercept_exceptions |= (1 << NM_VECTOR);
> -		svm->vmcb->save.cr0 |= X86_CR0_TS;
> -		vcpu->fpu_active = 0;
> -	}
> +/* 	if (vcpu->fpu_active) { */
> +/* 		svm->vmcb->control.intercept_exceptions |= (1 << NM_VECTOR); */
> +/* 		svm->vmcb->save.cr0 |= X86_CR0_TS; */
> +/* 		vcpu->fpu_active = 0; */
> +/* 	} */
>  }
>  
>  static void svm_inject_page_fault(struct kvm_vcpu *vcpu,
>   

Changed those into a #if 0.  Those comments were unsightly.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: KVM: SVM: Disable Lazy FPU optimization because of regressions

[Amit Shah]

On Sunday 18 November 2007 22:58:56 Avi Kivity wrote:
> Amit Shah wrote:

> > diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
> > index ea2cb83..d7c9b92 100644
> > --- a/drivers/kvm/svm.c
> > +++ b/drivers/kvm/svm.c
> > @@ -1600,11 +1600,11 @@ static void svm_set_cr3(struct kvm_vcpu *vcpu,
> > unsigned long root)
> >  	svm->vmcb->save.cr3 = root;
> >  	force_new_asid(vcpu);
> >
> > -	if (vcpu->fpu_active) {
> > -		svm->vmcb->control.intercept_exceptions |= (1 << NM_VECTOR);
> > -		svm->vmcb->save.cr0 |= X86_CR0_TS;
> > -		vcpu->fpu_active = 0;
> > -	}
> > +/* 	if (vcpu->fpu_active) { */
> > +/* 		svm->vmcb->control.intercept_exceptions |= (1 << NM_VECTOR); */
> > +/* 		svm->vmcb->save.cr0 |= X86_CR0_TS; */
> > +/* 		vcpu->fpu_active = 0; */
> > +/* 	} */
> >  }
> >
> >  static void svm_inject_page_fault(struct kvm_vcpu *vcpu,
>
> Changed those into a #if 0.  Those comments were unsightly.

Oh, I really just let emacs do it for me..

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: KVM: SVM: Disable Lazy FPU optimization because of regressions

[Mike Lampard]

On Mon, 19 Nov 2007 03:44:51 am Amit Shah wrote:
> >From 76204eda7e03035c16702105e78724137ecad24b Mon Sep 17 00:00:00 2001
>
> From: Amit Shah <amit.shah-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
> Date: Sun, 18 Nov 2007 22:42:47 +0530
> Subject: [PATCH] KVM: SVM: Disable Lazy FPU optimization because of
> regressions
>
> Host FPU state is leaked into the guest FPU state. This happens
> because of the lazy FPU optimization, so just reload the FPU
> each time there's a VM exit/entry.
>
> The real fix should follow soon.
>
> This is observed in cases where fonts in a guest aren't rendered
> correctly (bug 1807560).
>
> The test program, courtesy Avi is:
>
> double test_fpu_once()
> {
>     int i;
>     double f = 0;
>
>     for (i = 0; i < 10000000; ++i)
>         f += 1 / (1.0 + i);
>     return f;
> }
>
> void test_fpu()
> {
>     double a, b;
>     int runs;
>
>     runs = 0;
>     a = test_fpu_once();
>     while (1) {
>         b = test_fpu_once();
>         if (fabs(a - b) > 1e-9)
>             printf("error: %20.16f -> %20.16f\n", a, b);
>         a = b;
>         if (++runs % 100 == 0)
>             printf("runs: %8d\n", runs);
>     }
> }
>
> int main(int ac, char **av)
> {
>     test_fpu();
>     return 0;
> }
>
> Run this on the host and the guest, both pinned to the same
> host CPU.
>
> Signed-off-by: Amit Shah <amit.shah-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/kvm/svm.c |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
> index ea2cb83..d7c9b92 100644
> --- a/drivers/kvm/svm.c
> +++ b/drivers/kvm/svm.c
> @@ -1600,11 +1600,11 @@ static void svm_set_cr3(struct kvm_vcpu *vcpu,
> unsigned long root)
>  	svm->vmcb->save.cr3 = root;
>  	force_new_asid(vcpu);
>
> -	if (vcpu->fpu_active) {
> -		svm->vmcb->control.intercept_exceptions |= (1 << NM_VECTOR);
> -		svm->vmcb->save.cr0 |= X86_CR0_TS;
> -		vcpu->fpu_active = 0;
> -	}
> +/* 	if (vcpu->fpu_active) { */
> +/* 		svm->vmcb->control.intercept_exceptions |= (1 << NM_VECTOR); */
> +/* 		svm->vmcb->save.cr0 |= X86_CR0_TS; */
> +/* 		vcpu->fpu_active = 0; */
> +/* 	} */
>  }
>
>  static void svm_inject_page_fault(struct kvm_vcpu *vcpu,

This patch didnt resolve the font issues for me, however the other patch 
(unload FPU state) did fix it.  I didnt run the test app (no compiler on the 
(32bit ubuntu) guest atm) but reverting this patch on my tree didnt appear to 
adversely affect anything.  Are both patches necessary?

Mike

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: KVM: SVM: Disable Lazy FPU optimization because of regressions

[Amit Shah]

On Sunday 18 November 2007 23:49:38 Mike Lampard wrote:
> This patch didnt resolve the font issues for me, however the other patch
> (unload FPU state) did fix it.  I didnt run the test app (no compiler on
> the (32bit ubuntu) guest atm) but reverting this patch on my tree didnt
> appear to adversely affect anything.  Are both patches necessary?

OK, thanks for confirming, even though a different patch fixed it :-)

Yes, both patches will be necessary as they fix two different things, but both 
cause FPU leaks.

With this patch, though, the guest will run slower as we disable an 
optimization (because it causes a regression).

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/