Re: [PATCH v3] kvm/fpu: Enable fully eager restore kvm FPU
On 09/21/2012 11:47 AM, Hao, Xudong wrote: > > -Original Message- > > From: Avi Kivity [mailto:a...@redhat.com] > > Sent: Thursday, September 20, 2012 5:20 PM > > To: Hao, Xudong > > Cc: Marcelo Tosatti; kvm@vger.kernel.org; Zhang, Xiantao > > Subject: Re: [PATCH v3] kvm/fpu: Enable fully eager restore kvm FPU > > >> >> On guest entry: > > >> >> if (!lazy_fpu_allowed(vcpu)) > > >> >> kvm_x86_ops->fpu_activate(vcpu); > > >> > > > >> > > >> But we already have that: > > >> > > >> if (vcpu->fpu_active) > > >> kvm_load_guest_fpu(vcpu); > > >> > > >> so why not manage fpu_active to be always set when needed? I don't > > want > > >> more checks in the entry path. > > >> > > > I means add fpu_active() in kvm_set_xcr(), not in guest entry. Then the > > fpu_active will be set always when guest initialize xstate. > > > > > > @@ -574,6 +574,9 @@ int kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, > > u64 xcr) > > > kvm_inject_gp(vcpu, 0); > > > return 1; > > > } > > > + if (!lazy_fpu_allowed(vcpu)) > > > + kvm_x86_ops->fpu_activate(vcpu); > > > return 0; > > > > > > > And of course on cr4 update. So a function update_lazy_fpu() to be > > called from both places is needed. > > > > Complete consideration, thanks. > > So I will define a function update_lazy_fpu(), insert it into kvm_set_xcr() > and handle_cr(). Comments? Sounds good. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain. -- 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 v3] kvm/fpu: Enable fully eager restore kvm FPU
> -Original Message- > From: Avi Kivity [mailto:a...@redhat.com] > Sent: Thursday, September 20, 2012 5:20 PM > To: Hao, Xudong > Cc: Marcelo Tosatti; kvm@vger.kernel.org; Zhang, Xiantao > Subject: Re: [PATCH v3] kvm/fpu: Enable fully eager restore kvm FPU > >> >> On guest entry: > >> >> if (!lazy_fpu_allowed(vcpu)) > >> >> kvm_x86_ops->fpu_activate(vcpu); > >> > > >> > >> But we already have that: > >> > >>if (vcpu->fpu_active) > >>kvm_load_guest_fpu(vcpu); > >> > >> so why not manage fpu_active to be always set when needed? I don't > want > >> more checks in the entry path. > >> > > I means add fpu_active() in kvm_set_xcr(), not in guest entry. Then the > fpu_active will be set always when guest initialize xstate. > > > > @@ -574,6 +574,9 @@ int kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, > u64 xcr) > > kvm_inject_gp(vcpu, 0); > > return 1; > > } > > + if (!lazy_fpu_allowed(vcpu)) > > + kvm_x86_ops->fpu_activate(vcpu); > > return 0; > > > > And of course on cr4 update. So a function update_lazy_fpu() to be > called from both places is needed. > Complete consideration, thanks. So I will define a function update_lazy_fpu(), insert it into kvm_set_xcr() and handle_cr(). Comments? -- 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 v3] kvm/fpu: Enable fully eager restore kvm FPU
On 09/20/2012 04:43 AM, Hao, Xudong wrote:
>> -Original Message-
>> From: Avi Kivity [mailto:a...@redhat.com]
>> Sent: Wednesday, September 19, 2012 6:24 PM
>> To: Hao, Xudong
>> Cc: Marcelo Tosatti; kvm@vger.kernel.org; Zhang, Xiantao
>> Subject: Re: [PATCH v3] kvm/fpu: Enable fully eager restore kvm FPU
>> > That may be:
>> >
>> > static bool lazy_fpu_allowed()
>> > {
>> >return !(vcpu->arch.xcr0 & ~((u64)KVM_XSTATE_LAZY));
>> > }
>>
>> Shouldn't it depend on cr4.osxsave as well?
>>
>
> It do need to check cr4.osxsave due to a separate function.
>
> static bool lazy_fpu_allowed(struct kvm_vcpu *vcpu)
> {
> return !kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) ||
> !(vcpu->arch.xcr0 & ~((u64)KVM_XSTATE_LAZY));
> }
Yes.
>
>> >
>> >> On guest entry:
>> >> if (!lazy_fpu_allowed(vcpu))
>> >> kvm_x86_ops->fpu_activate(vcpu);
>> >
>>
>> But we already have that:
>>
>> if (vcpu->fpu_active)
>> kvm_load_guest_fpu(vcpu);
>>
>> so why not manage fpu_active to be always set when needed? I don't want
>> more checks in the entry path.
>>
> I means add fpu_active() in kvm_set_xcr(), not in guest entry. Then the
> fpu_active will be set always when guest initialize xstate.
>
> @@ -574,6 +574,9 @@ int kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
> kvm_inject_gp(vcpu, 0);
> return 1;
> }
> + if (!lazy_fpu_allowed(vcpu))
> + kvm_x86_ops->fpu_activate(vcpu);
> return 0;
>
And of course on cr4 update. So a function update_lazy_fpu() to be
called from both places is needed.
--
error compiling committee.c: too many arguments to function
--
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 v3] kvm/fpu: Enable fully eager restore kvm FPU
> -Original Message-
> From: Avi Kivity [mailto:a...@redhat.com]
> Sent: Wednesday, September 19, 2012 6:24 PM
> To: Hao, Xudong
> Cc: Marcelo Tosatti; kvm@vger.kernel.org; Zhang, Xiantao
> Subject: Re: [PATCH v3] kvm/fpu: Enable fully eager restore kvm FPU
> > That may be:
> >
> > static bool lazy_fpu_allowed()
> > {
> > return !(vcpu->arch.xcr0 & ~((u64)KVM_XSTATE_LAZY));
> > }
>
> Shouldn't it depend on cr4.osxsave as well?
>
It do need to check cr4.osxsave due to a separate function.
static bool lazy_fpu_allowed(struct kvm_vcpu *vcpu)
{
return !kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) ||
!(vcpu->arch.xcr0 & ~((u64)KVM_XSTATE_LAZY));
}
> >
> >> On guest entry:
> >> if (!lazy_fpu_allowed(vcpu))
> >> kvm_x86_ops->fpu_activate(vcpu);
> >
>
> But we already have that:
>
> if (vcpu->fpu_active)
> kvm_load_guest_fpu(vcpu);
>
> so why not manage fpu_active to be always set when needed? I don't want
> more checks in the entry path.
>
I means add fpu_active() in kvm_set_xcr(), not in guest entry. Then the
fpu_active will be set always when guest initialize xstate.
@@ -574,6 +574,9 @@ int kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
kvm_inject_gp(vcpu, 0);
return 1;
}
+ if (!lazy_fpu_allowed(vcpu))
+ kvm_x86_ops->fpu_activate(vcpu);
return 0;
--
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 v3] kvm/fpu: Enable fully eager restore kvm FPU
On 09/18/2012 04:08 AM, Hao, Xudong wrote:
>>
>> The objective of the change is to disable lazy fpu loading (that is,
>> host fpu loaded in guest and vice-versa),
Not vice versa. We allow the guest fpu loaded in the host, but save it
on heavyweight exit or task switch.
when some bit except the
>> initial tree bits set in guest XCR0 (initial tree being XSTATE_FP|XSTATE_SSE|
>> XSTATE_YMM). Yes?
>>
>
> Yes, it's just the object.
>
>> If i get that right, then the suggestion seems to be:
>>
>> static bool lazy_fpu_allowed()
>> {
>> return (vcpu->arch.xcr0 & ~((u64)KVM_XSTATE_LAZY));
>> }
>>
>
> That may be:
>
> static bool lazy_fpu_allowed()
> {
> return !(vcpu->arch.xcr0 & ~((u64)KVM_XSTATE_LAZY));
> }
Shouldn't it depend on cr4.osxsave as well?
>
>> On guest entry:
>> if (!lazy_fpu_allowed(vcpu))
>> kvm_x86_ops->fpu_activate(vcpu);
>
But we already have that:
if (vcpu->fpu_active)
kvm_load_guest_fpu(vcpu);
so why not manage fpu_active to be always set when needed? I don't want
more checks in the entry path.
--
error compiling committee.c: too many arguments to function
--
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 v3] kvm/fpu: Enable fully eager restore kvm FPU
> -Original Message-
> From: Marcelo Tosatti [mailto:mtosa...@redhat.com]
> Sent: Monday, September 17, 2012 9:31 PM
> To: Hao, Xudong
> Cc: Avi Kivity; kvm@vger.kernel.org; Zhang, Xiantao
> Subject: Re: [PATCH v3] kvm/fpu: Enable fully eager restore kvm FPU
>
> On Mon, Sep 17, 2012 at 02:07:43AM +, Hao, Xudong wrote:
> > > -Original Message-
> > > From: Avi Kivity [mailto:a...@redhat.com]
> > > Sent: Friday, September 14, 2012 12:40 AM
> > > To: Marcelo Tosatti
> > > Cc: Hao, Xudong; kvm@vger.kernel.org; Zhang, Xiantao
> > > Subject: Re: [PATCH v3] kvm/fpu: Enable fully eager restore kvm FPU
> > >
> > > On 09/13/2012 07:29 PM, Marcelo Tosatti wrote:
> > > > On Thu, Sep 13, 2012 at 01:26:36PM -0300, Marcelo Tosatti wrote:
> > > >> On Wed, Sep 12, 2012 at 04:10:24PM +0800, Xudong Hao wrote:
> > > >> > Enable KVM FPU fully eager restore, if there is other FPU state which
> isn't
> > > >> > tracked by CR0.TS bit.
> > > >> >
> > > >> > v3 changes from v2:
> > > >> > - Make fpu active explicitly while guest xsave is enabling and
> > > >> > non-lazy
> > > xstate bit
> > > >> > exist.
> > > >>
> > > >> How about a "guest_xcr0_can_lazy_saverestore" bool to control this?
> > > >> It only needs to be updated when guest xcr0 is updated.
> > > >>
> > > >> That seems cleaner. Avi?
> > > >
> > > > Reasoning below.
> > > >
> > > >> > v2 changes from v1:
> > > >> > - Expand KVM_XSTATE_LAZY to 64 bits before negating it.
> > > >> >
> > > >> > Signed-off-by: Xudong Hao
> > > >> > ---
> > > >> > arch/x86/include/asm/kvm.h |4
> > > >> > arch/x86/kvm/vmx.c |2 ++
> > > >> > arch/x86/kvm/x86.c | 15 ++-
> > > >> > 3 files changed, 20 insertions(+), 1 deletions(-)
> > > >> >
> > > >> > diff --git a/arch/x86/include/asm/kvm.h b/arch/x86/include/asm/kvm.h
> > > >> > index 521bf25..4c27056 100644
> > > >> > --- a/arch/x86/include/asm/kvm.h
> > > >> > +++ b/arch/x86/include/asm/kvm.h
> > > >> > @@ -8,6 +8,8 @@
> > > >> >
> > > >> > #include
> > > >> > #include
> > > >> > +#include
> > > >> > +#include
> > > >> >
> > > >> > /* Select x86 specific features in */
> > > >> > #define __KVM_HAVE_PIT
> > > >> > @@ -30,6 +32,8 @@
> > > >> > /* Architectural interrupt line count. */
> > > >> > #define KVM_NR_INTERRUPTS 256
> > > >> >
> > > >> > +#define KVM_XSTATE_LAZY (XSTATE_FP | XSTATE_SSE |
> XSTATE_YMM)
> > > >> > +
> > > >> > struct kvm_memory_alias {
> > > >> > __u32 slot; /* this has a different namespace than memory
> slots */
> > > >> > __u32 flags;
> > > >> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > > >> > index 248c2b4..853e875 100644
> > > >> > --- a/arch/x86/kvm/vmx.c
> > > >> > +++ b/arch/x86/kvm/vmx.c
> > > >> > @@ -3028,6 +3028,8 @@ static void vmx_set_cr0(struct kvm_vcpu
> *vcpu,
> > > unsigned long cr0)
> > > >> >
> > > >> > if (!vcpu->fpu_active)
> > > >> > hw_cr0 |= X86_CR0_TS | X86_CR0_MP;
> > > >> > +else
> > > >> > +hw_cr0 &= ~(X86_CR0_TS | X86_CR0_MP);
> > > >> >
> > > >> > vmcs_writel(CR0_READ_SHADOW, cr0);
> > > >> > vmcs_writel(GUEST_CR0, hw_cr0);
> > > >> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > >> > index 20f2266..183cf60 100644
> > > >> > --- a/arch/x86/kvm/x86.c
> > > >> > +++ b/arch/x86/kvm/x86.c
> > > >> > @@ -560,6 +560,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu,
> u32
> > > index, u64 xcr)
> > > >> > return 1;
> > > >> > if (xcr0 & ~host_xcr0)
> > > >> > return 1;
> > > >> > +if (xcr0 & ~((u64)KVM_XSTATE_LAZY))
> &
Re: [PATCH v3] kvm/fpu: Enable fully eager restore kvm FPU
On Mon, Sep 17, 2012 at 02:07:43AM +, Hao, Xudong wrote:
> > -Original Message-
> > From: Avi Kivity [mailto:a...@redhat.com]
> > Sent: Friday, September 14, 2012 12:40 AM
> > To: Marcelo Tosatti
> > Cc: Hao, Xudong; kvm@vger.kernel.org; Zhang, Xiantao
> > Subject: Re: [PATCH v3] kvm/fpu: Enable fully eager restore kvm FPU
> >
> > On 09/13/2012 07:29 PM, Marcelo Tosatti wrote:
> > > On Thu, Sep 13, 2012 at 01:26:36PM -0300, Marcelo Tosatti wrote:
> > >> On Wed, Sep 12, 2012 at 04:10:24PM +0800, Xudong Hao wrote:
> > >> > Enable KVM FPU fully eager restore, if there is other FPU state which
> > >> > isn't
> > >> > tracked by CR0.TS bit.
> > >> >
> > >> > v3 changes from v2:
> > >> > - Make fpu active explicitly while guest xsave is enabling and non-lazy
> > xstate bit
> > >> > exist.
> > >>
> > >> How about a "guest_xcr0_can_lazy_saverestore" bool to control this?
> > >> It only needs to be updated when guest xcr0 is updated.
> > >>
> > >> That seems cleaner. Avi?
> > >
> > > Reasoning below.
> > >
> > >> > v2 changes from v1:
> > >> > - Expand KVM_XSTATE_LAZY to 64 bits before negating it.
> > >> >
> > >> > Signed-off-by: Xudong Hao
> > >> > ---
> > >> > arch/x86/include/asm/kvm.h |4
> > >> > arch/x86/kvm/vmx.c |2 ++
> > >> > arch/x86/kvm/x86.c | 15 ++-
> > >> > 3 files changed, 20 insertions(+), 1 deletions(-)
> > >> >
> > >> > diff --git a/arch/x86/include/asm/kvm.h b/arch/x86/include/asm/kvm.h
> > >> > index 521bf25..4c27056 100644
> > >> > --- a/arch/x86/include/asm/kvm.h
> > >> > +++ b/arch/x86/include/asm/kvm.h
> > >> > @@ -8,6 +8,8 @@
> > >> >
> > >> > #include
> > >> > #include
> > >> > +#include
> > >> > +#include
> > >> >
> > >> > /* Select x86 specific features in */
> > >> > #define __KVM_HAVE_PIT
> > >> > @@ -30,6 +32,8 @@
> > >> > /* Architectural interrupt line count. */
> > >> > #define KVM_NR_INTERRUPTS 256
> > >> >
> > >> > +#define KVM_XSTATE_LAZY (XSTATE_FP | XSTATE_SSE | XSTATE_YMM)
> > >> > +
> > >> > struct kvm_memory_alias {
> > >> >__u32 slot; /* this has a different namespace than memory
> > >> > slots */
> > >> >__u32 flags;
> > >> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > >> > index 248c2b4..853e875 100644
> > >> > --- a/arch/x86/kvm/vmx.c
> > >> > +++ b/arch/x86/kvm/vmx.c
> > >> > @@ -3028,6 +3028,8 @@ static void vmx_set_cr0(struct kvm_vcpu *vcpu,
> > unsigned long cr0)
> > >> >
> > >> >if (!vcpu->fpu_active)
> > >> >hw_cr0 |= X86_CR0_TS | X86_CR0_MP;
> > >> > + else
> > >> > + hw_cr0 &= ~(X86_CR0_TS | X86_CR0_MP);
> > >> >
> > >> >vmcs_writel(CR0_READ_SHADOW, cr0);
> > >> >vmcs_writel(GUEST_CR0, hw_cr0);
> > >> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > >> > index 20f2266..183cf60 100644
> > >> > --- a/arch/x86/kvm/x86.c
> > >> > +++ b/arch/x86/kvm/x86.c
> > >> > @@ -560,6 +560,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32
> > index, u64 xcr)
> > >> >return 1;
> > >> >if (xcr0 & ~host_xcr0)
> > >> >return 1;
> > >> > + if (xcr0 & ~((u64)KVM_XSTATE_LAZY))
> > >> > + vcpu->fpu_active = 1;
> > >
> > > This is confusing. The variable allows to decrease the number of places
> > > the decision is made.
> >
> > Better to have a helper function (lazy_fpu_allowed(), for example).
> > Variables raise the question of whether they are maintained correctly.
> >
>
> I realized to modifying the fpu_active variable is incorrect, it must update
> exception bitmap.
> To avoid the cr0 and xcrs setting order for live migrate case, how about
> calling fpu_activate() in kvm_set_xcr()? I can add code comments in this
> function calling.
Th
RE: [PATCH v3] kvm/fpu: Enable fully eager restore kvm FPU
> -Original Message-
> From: Avi Kivity [mailto:a...@redhat.com]
> Sent: Friday, September 14, 2012 12:40 AM
> To: Marcelo Tosatti
> Cc: Hao, Xudong; kvm@vger.kernel.org; Zhang, Xiantao
> Subject: Re: [PATCH v3] kvm/fpu: Enable fully eager restore kvm FPU
>
> On 09/13/2012 07:29 PM, Marcelo Tosatti wrote:
> > On Thu, Sep 13, 2012 at 01:26:36PM -0300, Marcelo Tosatti wrote:
> >> On Wed, Sep 12, 2012 at 04:10:24PM +0800, Xudong Hao wrote:
> >> > Enable KVM FPU fully eager restore, if there is other FPU state which
> >> > isn't
> >> > tracked by CR0.TS bit.
> >> >
> >> > v3 changes from v2:
> >> > - Make fpu active explicitly while guest xsave is enabling and non-lazy
> xstate bit
> >> > exist.
> >>
> >> How about a "guest_xcr0_can_lazy_saverestore" bool to control this?
> >> It only needs to be updated when guest xcr0 is updated.
> >>
> >> That seems cleaner. Avi?
> >
> > Reasoning below.
> >
> >> > v2 changes from v1:
> >> > - Expand KVM_XSTATE_LAZY to 64 bits before negating it.
> >> >
> >> > Signed-off-by: Xudong Hao
> >> > ---
> >> > arch/x86/include/asm/kvm.h |4
> >> > arch/x86/kvm/vmx.c |2 ++
> >> > arch/x86/kvm/x86.c | 15 ++-
> >> > 3 files changed, 20 insertions(+), 1 deletions(-)
> >> >
> >> > diff --git a/arch/x86/include/asm/kvm.h b/arch/x86/include/asm/kvm.h
> >> > index 521bf25..4c27056 100644
> >> > --- a/arch/x86/include/asm/kvm.h
> >> > +++ b/arch/x86/include/asm/kvm.h
> >> > @@ -8,6 +8,8 @@
> >> >
> >> > #include
> >> > #include
> >> > +#include
> >> > +#include
> >> >
> >> > /* Select x86 specific features in */
> >> > #define __KVM_HAVE_PIT
> >> > @@ -30,6 +32,8 @@
> >> > /* Architectural interrupt line count. */
> >> > #define KVM_NR_INTERRUPTS 256
> >> >
> >> > +#define KVM_XSTATE_LAZY (XSTATE_FP | XSTATE_SSE | XSTATE_YMM)
> >> > +
> >> > struct kvm_memory_alias {
> >> > __u32 slot; /* this has a different namespace than memory
> >> > slots */
> >> > __u32 flags;
> >> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> >> > index 248c2b4..853e875 100644
> >> > --- a/arch/x86/kvm/vmx.c
> >> > +++ b/arch/x86/kvm/vmx.c
> >> > @@ -3028,6 +3028,8 @@ static void vmx_set_cr0(struct kvm_vcpu *vcpu,
> unsigned long cr0)
> >> >
> >> > if (!vcpu->fpu_active)
> >> > hw_cr0 |= X86_CR0_TS | X86_CR0_MP;
> >> > +else
> >> > +hw_cr0 &= ~(X86_CR0_TS | X86_CR0_MP);
> >> >
> >> > vmcs_writel(CR0_READ_SHADOW, cr0);
> >> > vmcs_writel(GUEST_CR0, hw_cr0);
> >> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >> > index 20f2266..183cf60 100644
> >> > --- a/arch/x86/kvm/x86.c
> >> > +++ b/arch/x86/kvm/x86.c
> >> > @@ -560,6 +560,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32
> index, u64 xcr)
> >> > return 1;
> >> > if (xcr0 & ~host_xcr0)
> >> > return 1;
> >> > +if (xcr0 & ~((u64)KVM_XSTATE_LAZY))
> >> > +vcpu->fpu_active = 1;
> >
> > This is confusing. The variable allows to decrease the number of places
> > the decision is made.
>
> Better to have a helper function (lazy_fpu_allowed(), for example).
> Variables raise the question of whether they are maintained correctly.
>
I realized to modifying the fpu_active variable is incorrect, it must update
exception bitmap.
To avoid the cr0 and xcrs setting order for live migrate case, how about
calling fpu_activate() in kvm_set_xcr()? I can add code comments in this
function calling.
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index be6d549..e4646d9 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -574,6 +574,9 @@ int kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
kvm_inject_gp(vcpu, 0);
return 1;
}
+ if (xcr & ~((u64)KVM_XSTATE_LAZY))
+ /* Allow fpu eager restore */
+ kvm_x86_ops->fpu_activate(vcpu);
return 0;
}
Thanks,
-Xudong
--
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 v3] kvm/fpu: Enable fully eager restore kvm FPU
On Fri, Sep 14, 2012 at 08:54:40AM +, Hao, Xudong wrote:
> > -Original Message-
> > From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org] On
> > Behalf Of Marcelo Tosatti
> > Sent: Friday, September 14, 2012 12:29 AM
> > To: Hao, Xudong; Avi Kivity
> > Cc: kvm@vger.kernel.org; Zhang, Xiantao
> > Subject: Re: [PATCH v3] kvm/fpu: Enable fully eager restore kvm FPU
> >
> > On Thu, Sep 13, 2012 at 01:26:36PM -0300, Marcelo Tosatti wrote:
> > > On Wed, Sep 12, 2012 at 04:10:24PM +0800, Xudong Hao wrote:
> > > > Enable KVM FPU fully eager restore, if there is other FPU state which
> > > > isn't
> > > > tracked by CR0.TS bit.
> > > >
> > > > v3 changes from v2:
> > > > - Make fpu active explicitly while guest xsave is enabling and non-lazy
> > > > xstate
> > bit
> > > > exist.
> > >
> > > How about a "guest_xcr0_can_lazy_saverestore" bool to control this?
> > > It only needs to be updated when guest xcr0 is updated.
> > >
> > > That seems cleaner. Avi?
> >
> > Reasoning below.
> >
> > > > v2 changes from v1:
> > > > - Expand KVM_XSTATE_LAZY to 64 bits before negating it.
> > > >
> > > > Signed-off-by: Xudong Hao
> > > > ---
> > > > arch/x86/include/asm/kvm.h |4
> > > > arch/x86/kvm/vmx.c |2 ++
> > > > arch/x86/kvm/x86.c | 15 ++-
> > > > 3 files changed, 20 insertions(+), 1 deletions(-)
> > > >
> > > > diff --git a/arch/x86/include/asm/kvm.h b/arch/x86/include/asm/kvm.h
> > > > index 521bf25..4c27056 100644
> > > > --- a/arch/x86/include/asm/kvm.h
> > > > +++ b/arch/x86/include/asm/kvm.h
> > > > @@ -8,6 +8,8 @@
> > > >
> > > > #include
> > > > #include
> > > > +#include
> > > > +#include
> > > >
> > > > /* Select x86 specific features in */
> > > > #define __KVM_HAVE_PIT
> > > > @@ -30,6 +32,8 @@
> > > > /* Architectural interrupt line count. */
> > > > #define KVM_NR_INTERRUPTS 256
> > > >
> > > > +#define KVM_XSTATE_LAZY(XSTATE_FP | XSTATE_SSE | XSTATE_YMM)
> > > > +
> > > > struct kvm_memory_alias {
> > > > __u32 slot; /* this has a different namespace than memory
> > > > slots */
> > > > __u32 flags;
> > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > > > index 248c2b4..853e875 100644
> > > > --- a/arch/x86/kvm/vmx.c
> > > > +++ b/arch/x86/kvm/vmx.c
> > > > @@ -3028,6 +3028,8 @@ static void vmx_set_cr0(struct kvm_vcpu *vcpu,
> > unsigned long cr0)
> > > >
> > > > if (!vcpu->fpu_active)
> > > > hw_cr0 |= X86_CR0_TS | X86_CR0_MP;
> > > > + else
> > > > + hw_cr0 &= ~(X86_CR0_TS | X86_CR0_MP);
> > > >
> > > > vmcs_writel(CR0_READ_SHADOW, cr0);
> > > > vmcs_writel(GUEST_CR0, hw_cr0);
> > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > > index 20f2266..183cf60 100644
> > > > --- a/arch/x86/kvm/x86.c
> > > > +++ b/arch/x86/kvm/x86.c
> > > > @@ -560,6 +560,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32
> > index, u64 xcr)
> > > > return 1;
> > > > if (xcr0 & ~host_xcr0)
> > > > return 1;
> > > > + if (xcr0 & ~((u64)KVM_XSTATE_LAZY))
> > > > + vcpu->fpu_active = 1;
> >
> > This is confusing. The variable allows to decrease the number of places
> > the decision is made.
> >
>
> Hi, Marcelo, What does it mean?
See Avi's reply. Better create a function
lazy_fpu_allowed
Which can be used to decide whether or not to allow
guest owning FPU state.
--
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 v3] kvm/fpu: Enable fully eager restore kvm FPU
> -Original Message-
> From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org] On
> Behalf Of Marcelo Tosatti
> Sent: Friday, September 14, 2012 12:29 AM
> To: Hao, Xudong; Avi Kivity
> Cc: kvm@vger.kernel.org; Zhang, Xiantao
> Subject: Re: [PATCH v3] kvm/fpu: Enable fully eager restore kvm FPU
>
> On Thu, Sep 13, 2012 at 01:26:36PM -0300, Marcelo Tosatti wrote:
> > On Wed, Sep 12, 2012 at 04:10:24PM +0800, Xudong Hao wrote:
> > > Enable KVM FPU fully eager restore, if there is other FPU state which
> > > isn't
> > > tracked by CR0.TS bit.
> > >
> > > v3 changes from v2:
> > > - Make fpu active explicitly while guest xsave is enabling and non-lazy
> > > xstate
> bit
> > > exist.
> >
> > How about a "guest_xcr0_can_lazy_saverestore" bool to control this?
> > It only needs to be updated when guest xcr0 is updated.
> >
> > That seems cleaner. Avi?
>
> Reasoning below.
>
> > > v2 changes from v1:
> > > - Expand KVM_XSTATE_LAZY to 64 bits before negating it.
> > >
> > > Signed-off-by: Xudong Hao
> > > ---
> > > arch/x86/include/asm/kvm.h |4
> > > arch/x86/kvm/vmx.c |2 ++
> > > arch/x86/kvm/x86.c | 15 ++-
> > > 3 files changed, 20 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/arch/x86/include/asm/kvm.h b/arch/x86/include/asm/kvm.h
> > > index 521bf25..4c27056 100644
> > > --- a/arch/x86/include/asm/kvm.h
> > > +++ b/arch/x86/include/asm/kvm.h
> > > @@ -8,6 +8,8 @@
> > >
> > > #include
> > > #include
> > > +#include
> > > +#include
> > >
> > > /* Select x86 specific features in */
> > > #define __KVM_HAVE_PIT
> > > @@ -30,6 +32,8 @@
> > > /* Architectural interrupt line count. */
> > > #define KVM_NR_INTERRUPTS 256
> > >
> > > +#define KVM_XSTATE_LAZY (XSTATE_FP | XSTATE_SSE | XSTATE_YMM)
> > > +
> > > struct kvm_memory_alias {
> > > __u32 slot; /* this has a different namespace than memory slots */
> > > __u32 flags;
> > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > > index 248c2b4..853e875 100644
> > > --- a/arch/x86/kvm/vmx.c
> > > +++ b/arch/x86/kvm/vmx.c
> > > @@ -3028,6 +3028,8 @@ static void vmx_set_cr0(struct kvm_vcpu *vcpu,
> unsigned long cr0)
> > >
> > > if (!vcpu->fpu_active)
> > > hw_cr0 |= X86_CR0_TS | X86_CR0_MP;
> > > + else
> > > + hw_cr0 &= ~(X86_CR0_TS | X86_CR0_MP);
> > >
> > > vmcs_writel(CR0_READ_SHADOW, cr0);
> > > vmcs_writel(GUEST_CR0, hw_cr0);
> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index 20f2266..183cf60 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -560,6 +560,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32
> index, u64 xcr)
> > > return 1;
> > > if (xcr0 & ~host_xcr0)
> > > return 1;
> > > + if (xcr0 & ~((u64)KVM_XSTATE_LAZY))
> > > + vcpu->fpu_active = 1;
>
> This is confusing. The variable allows to decrease the number of places
> the decision is made.
>
Hi, Marcelo, What does it mean?
--
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 v3] kvm/fpu: Enable fully eager restore kvm FPU
> -Original Message- > From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org] On > Behalf Of Avi Kivity > Sent: Friday, September 14, 2012 12:39 AM > To: Hao, Xudong > Cc: kvm@vger.kernel.org; Zhang, Xiantao > Subject: Re: [PATCH v3] kvm/fpu: Enable fully eager restore kvm FPU > > On 09/12/2012 11:10 AM, Xudong Hao wrote: > > Enable KVM FPU fully eager restore, if there is other FPU state which isn't > > tracked by CR0.TS bit. > > > > v3 changes from v2: > > - Make fpu active explicitly while guest xsave is enabling and non-lazy > > xstate > bit > > exist. > > > > v2 changes from v1: > > - Expand KVM_XSTATE_LAZY to 64 bits before negating it. > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > index 248c2b4..853e875 100644 > > --- a/arch/x86/kvm/vmx.c > > +++ b/arch/x86/kvm/vmx.c > > @@ -3028,6 +3028,8 @@ static void vmx_set_cr0(struct kvm_vcpu *vcpu, > unsigned long cr0) > > > > if (!vcpu->fpu_active) > > hw_cr0 |= X86_CR0_TS | X86_CR0_MP; > > + else > > + hw_cr0 &= ~(X86_CR0_TS | X86_CR0_MP); > > > > Why? The guest may wish to receive #NM faults. > Hmm, I wanted to clear TS bit to avoid vmexit if fpu_active=1, but missing to consider the guest inside. > > vmcs_writel(CR0_READ_SHADOW, cr0); > > vmcs_writel(GUEST_CR0, hw_cr0); > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index 20f2266..183cf60 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -560,6 +560,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, > u64 xcr) > > return 1; > > if (xcr0 & ~host_xcr0) > > return 1; > > + if (xcr0 & ~((u64)KVM_XSTATE_LAZY)) > > + vcpu->fpu_active = 1; > > vcpu->arch.xcr0 = xcr0; > > vcpu->guest_xcr0_loaded = 0; > > return 0; > > @@ -5969,7 +5971,18 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu) > > vcpu->guest_fpu_loaded = 0; > > fpu_save_init(&vcpu->arch.guest_fpu); > > ++vcpu->stat.fpu_reload; > > - kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); > > + /* > > +* Currently KVM trigger FPU restore by #NM (via CR0.TS), > > +* till now only XCR0.bit0, XCR0.bit1, XCR0.bit2 is tracked > > "currently", "till now", don't tell someone reading the code in six > months anything. Just say how the code works. > Okay. -- 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 v3] kvm/fpu: Enable fully eager restore kvm FPU
On 09/13/2012 07:29 PM, Marcelo Tosatti wrote:
> On Thu, Sep 13, 2012 at 01:26:36PM -0300, Marcelo Tosatti wrote:
>> On Wed, Sep 12, 2012 at 04:10:24PM +0800, Xudong Hao wrote:
>> > Enable KVM FPU fully eager restore, if there is other FPU state which isn't
>> > tracked by CR0.TS bit.
>> >
>> > v3 changes from v2:
>> > - Make fpu active explicitly while guest xsave is enabling and non-lazy
>> > xstate bit
>> > exist.
>>
>> How about a "guest_xcr0_can_lazy_saverestore" bool to control this?
>> It only needs to be updated when guest xcr0 is updated.
>>
>> That seems cleaner. Avi?
>
> Reasoning below.
>
>> > v2 changes from v1:
>> > - Expand KVM_XSTATE_LAZY to 64 bits before negating it.
>> >
>> > Signed-off-by: Xudong Hao
>> > ---
>> > arch/x86/include/asm/kvm.h |4
>> > arch/x86/kvm/vmx.c |2 ++
>> > arch/x86/kvm/x86.c | 15 ++-
>> > 3 files changed, 20 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/arch/x86/include/asm/kvm.h b/arch/x86/include/asm/kvm.h
>> > index 521bf25..4c27056 100644
>> > --- a/arch/x86/include/asm/kvm.h
>> > +++ b/arch/x86/include/asm/kvm.h
>> > @@ -8,6 +8,8 @@
>> >
>> > #include
>> > #include
>> > +#include
>> > +#include
>> >
>> > /* Select x86 specific features in */
>> > #define __KVM_HAVE_PIT
>> > @@ -30,6 +32,8 @@
>> > /* Architectural interrupt line count. */
>> > #define KVM_NR_INTERRUPTS 256
>> >
>> > +#define KVM_XSTATE_LAZY (XSTATE_FP | XSTATE_SSE | XSTATE_YMM)
>> > +
>> > struct kvm_memory_alias {
>> >__u32 slot; /* this has a different namespace than memory slots */
>> >__u32 flags;
>> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> > index 248c2b4..853e875 100644
>> > --- a/arch/x86/kvm/vmx.c
>> > +++ b/arch/x86/kvm/vmx.c
>> > @@ -3028,6 +3028,8 @@ static void vmx_set_cr0(struct kvm_vcpu *vcpu,
>> > unsigned long cr0)
>> >
>> >if (!vcpu->fpu_active)
>> >hw_cr0 |= X86_CR0_TS | X86_CR0_MP;
>> > + else
>> > + hw_cr0 &= ~(X86_CR0_TS | X86_CR0_MP);
>> >
>> >vmcs_writel(CR0_READ_SHADOW, cr0);
>> >vmcs_writel(GUEST_CR0, hw_cr0);
>> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> > index 20f2266..183cf60 100644
>> > --- a/arch/x86/kvm/x86.c
>> > +++ b/arch/x86/kvm/x86.c
>> > @@ -560,6 +560,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index,
>> > u64 xcr)
>> >return 1;
>> >if (xcr0 & ~host_xcr0)
>> >return 1;
>> > + if (xcr0 & ~((u64)KVM_XSTATE_LAZY))
>> > + vcpu->fpu_active = 1;
>
> This is confusing. The variable allows to decrease the number of places
> the decision is made.
Better to have a helper function (lazy_fpu_allowed(), for example).
Variables raise the question of whether they are maintained correctly.
--
error compiling committee.c: too many arguments to function
--
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 v3] kvm/fpu: Enable fully eager restore kvm FPU
On 09/12/2012 11:10 AM, Xudong Hao wrote: > Enable KVM FPU fully eager restore, if there is other FPU state which isn't > tracked by CR0.TS bit. > > v3 changes from v2: > - Make fpu active explicitly while guest xsave is enabling and non-lazy > xstate bit > exist. > > v2 changes from v1: > - Expand KVM_XSTATE_LAZY to 64 bits before negating it. > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 248c2b4..853e875 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -3028,6 +3028,8 @@ static void vmx_set_cr0(struct kvm_vcpu *vcpu, unsigned > long cr0) > > if (!vcpu->fpu_active) > hw_cr0 |= X86_CR0_TS | X86_CR0_MP; > + else > + hw_cr0 &= ~(X86_CR0_TS | X86_CR0_MP); > Why? The guest may wish to receive #NM faults. > vmcs_writel(CR0_READ_SHADOW, cr0); > vmcs_writel(GUEST_CR0, hw_cr0); > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 20f2266..183cf60 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -560,6 +560,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 > xcr) > return 1; > if (xcr0 & ~host_xcr0) > return 1; > + if (xcr0 & ~((u64)KVM_XSTATE_LAZY)) > + vcpu->fpu_active = 1; > vcpu->arch.xcr0 = xcr0; > vcpu->guest_xcr0_loaded = 0; > return 0; > @@ -5969,7 +5971,18 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu) > vcpu->guest_fpu_loaded = 0; > fpu_save_init(&vcpu->arch.guest_fpu); > ++vcpu->stat.fpu_reload; > - kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); > + /* > + * Currently KVM trigger FPU restore by #NM (via CR0.TS), > + * till now only XCR0.bit0, XCR0.bit1, XCR0.bit2 is tracked "currently", "till now", don't tell someone reading the code in six months anything. Just say how the code works. > + * by TS bit, there might be other FPU state is not tracked > + * by TS bit. Here it only make FPU deactivate request and do > + * FPU lazy restore for these cases: 1)xsave isn't enabled > + * in guest, 2)all guest FPU states can be tracked by TS bit. > + * For others, doing fully FPU eager restore. > + */ > + if (!kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) || > + !(vcpu->arch.xcr0 & ~((u64)KVM_XSTATE_LAZY))) > + kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu); > trace_kvm_fpu(0); > } > > -- error compiling committee.c: too many arguments to function -- 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 v3] kvm/fpu: Enable fully eager restore kvm FPU
On Thu, Sep 13, 2012 at 01:26:36PM -0300, Marcelo Tosatti wrote:
> On Wed, Sep 12, 2012 at 04:10:24PM +0800, Xudong Hao wrote:
> > Enable KVM FPU fully eager restore, if there is other FPU state which isn't
> > tracked by CR0.TS bit.
> >
> > v3 changes from v2:
> > - Make fpu active explicitly while guest xsave is enabling and non-lazy
> > xstate bit
> > exist.
>
> How about a "guest_xcr0_can_lazy_saverestore" bool to control this?
> It only needs to be updated when guest xcr0 is updated.
>
> That seems cleaner. Avi?
Reasoning below.
> > v2 changes from v1:
> > - Expand KVM_XSTATE_LAZY to 64 bits before negating it.
> >
> > Signed-off-by: Xudong Hao
> > ---
> > arch/x86/include/asm/kvm.h |4
> > arch/x86/kvm/vmx.c |2 ++
> > arch/x86/kvm/x86.c | 15 ++-
> > 3 files changed, 20 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/kvm.h b/arch/x86/include/asm/kvm.h
> > index 521bf25..4c27056 100644
> > --- a/arch/x86/include/asm/kvm.h
> > +++ b/arch/x86/include/asm/kvm.h
> > @@ -8,6 +8,8 @@
> >
> > #include
> > #include
> > +#include
> > +#include
> >
> > /* Select x86 specific features in */
> > #define __KVM_HAVE_PIT
> > @@ -30,6 +32,8 @@
> > /* Architectural interrupt line count. */
> > #define KVM_NR_INTERRUPTS 256
> >
> > +#define KVM_XSTATE_LAZY(XSTATE_FP | XSTATE_SSE | XSTATE_YMM)
> > +
> > struct kvm_memory_alias {
> > __u32 slot; /* this has a different namespace than memory slots */
> > __u32 flags;
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index 248c2b4..853e875 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -3028,6 +3028,8 @@ static void vmx_set_cr0(struct kvm_vcpu *vcpu,
> > unsigned long cr0)
> >
> > if (!vcpu->fpu_active)
> > hw_cr0 |= X86_CR0_TS | X86_CR0_MP;
> > + else
> > + hw_cr0 &= ~(X86_CR0_TS | X86_CR0_MP);
> >
> > vmcs_writel(CR0_READ_SHADOW, cr0);
> > vmcs_writel(GUEST_CR0, hw_cr0);
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 20f2266..183cf60 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -560,6 +560,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64
> > xcr)
> > return 1;
> > if (xcr0 & ~host_xcr0)
> > return 1;
> > + if (xcr0 & ~((u64)KVM_XSTATE_LAZY))
> > + vcpu->fpu_active = 1;
This is confusing. The variable allows to decrease the number of places
the decision is made.
> > vcpu->arch.xcr0 = xcr0;
> > vcpu->guest_xcr0_loaded = 0;
> > return 0;
> > @@ -5969,7 +5971,18 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
> > vcpu->guest_fpu_loaded = 0;
> > fpu_save_init(&vcpu->arch.guest_fpu);
> > ++vcpu->stat.fpu_reload;
> > - kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu);
> > + /*
> > +* Currently KVM trigger FPU restore by #NM (via CR0.TS),
> > +* till now only XCR0.bit0, XCR0.bit1, XCR0.bit2 is tracked
> > +* by TS bit, there might be other FPU state is not tracked
> > +* by TS bit. Here it only make FPU deactivate request and do
> > +* FPU lazy restore for these cases: 1)xsave isn't enabled
> > +* in guest, 2)all guest FPU states can be tracked by TS bit.
> > +* For others, doing fully FPU eager restore.
> > +*/
> > + if (!kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) ||
> > + !(vcpu->arch.xcr0 & ~((u64)KVM_XSTATE_LAZY)))
> > + kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu);
> > trace_kvm_fpu(0);
> > }
> >
> > --
> > 1.5.5
> >
> > --
> > 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
> --
> 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
--
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 v3] kvm/fpu: Enable fully eager restore kvm FPU
On Wed, Sep 12, 2012 at 04:10:24PM +0800, Xudong Hao wrote:
> Enable KVM FPU fully eager restore, if there is other FPU state which isn't
> tracked by CR0.TS bit.
>
> v3 changes from v2:
> - Make fpu active explicitly while guest xsave is enabling and non-lazy
> xstate bit
> exist.
How about a "guest_xcr0_can_lazy_saverestore" bool to control this?
It only needs to be updated when guest xcr0 is updated.
That seems cleaner. Avi?
> v2 changes from v1:
> - Expand KVM_XSTATE_LAZY to 64 bits before negating it.
>
> Signed-off-by: Xudong Hao
> ---
> arch/x86/include/asm/kvm.h |4
> arch/x86/kvm/vmx.c |2 ++
> arch/x86/kvm/x86.c | 15 ++-
> 3 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm.h b/arch/x86/include/asm/kvm.h
> index 521bf25..4c27056 100644
> --- a/arch/x86/include/asm/kvm.h
> +++ b/arch/x86/include/asm/kvm.h
> @@ -8,6 +8,8 @@
>
> #include
> #include
> +#include
> +#include
>
> /* Select x86 specific features in */
> #define __KVM_HAVE_PIT
> @@ -30,6 +32,8 @@
> /* Architectural interrupt line count. */
> #define KVM_NR_INTERRUPTS 256
>
> +#define KVM_XSTATE_LAZY (XSTATE_FP | XSTATE_SSE | XSTATE_YMM)
> +
> struct kvm_memory_alias {
> __u32 slot; /* this has a different namespace than memory slots */
> __u32 flags;
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 248c2b4..853e875 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3028,6 +3028,8 @@ static void vmx_set_cr0(struct kvm_vcpu *vcpu, unsigned
> long cr0)
>
> if (!vcpu->fpu_active)
> hw_cr0 |= X86_CR0_TS | X86_CR0_MP;
> + else
> + hw_cr0 &= ~(X86_CR0_TS | X86_CR0_MP);
>
> vmcs_writel(CR0_READ_SHADOW, cr0);
> vmcs_writel(GUEST_CR0, hw_cr0);
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 20f2266..183cf60 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -560,6 +560,8 @@ int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64
> xcr)
> return 1;
> if (xcr0 & ~host_xcr0)
> return 1;
> + if (xcr0 & ~((u64)KVM_XSTATE_LAZY))
> + vcpu->fpu_active = 1;
> vcpu->arch.xcr0 = xcr0;
> vcpu->guest_xcr0_loaded = 0;
> return 0;
> @@ -5969,7 +5971,18 @@ void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
> vcpu->guest_fpu_loaded = 0;
> fpu_save_init(&vcpu->arch.guest_fpu);
> ++vcpu->stat.fpu_reload;
> - kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu);
> + /*
> + * Currently KVM trigger FPU restore by #NM (via CR0.TS),
> + * till now only XCR0.bit0, XCR0.bit1, XCR0.bit2 is tracked
> + * by TS bit, there might be other FPU state is not tracked
> + * by TS bit. Here it only make FPU deactivate request and do
> + * FPU lazy restore for these cases: 1)xsave isn't enabled
> + * in guest, 2)all guest FPU states can be tracked by TS bit.
> + * For others, doing fully FPU eager restore.
> + */
> + if (!kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) ||
> + !(vcpu->arch.xcr0 & ~((u64)KVM_XSTATE_LAZY)))
> + kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu);
> trace_kvm_fpu(0);
> }
>
> --
> 1.5.5
>
> --
> 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
--
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
