RE: [PATCH] kvm/fpu: Enable fully eager restore kvm FPU

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Avi Kivity [mailto:avi@xxxxxxxxxx]
> Sent: Thursday, August 16, 2012 6:59 PM
> To: Hao, Xudong
> Cc: kvm@xxxxxxxxxxxxxxx; Zhang, Xiantao; Roedel, Joerg
> Subject: Re: [PATCH] kvm/fpu: Enable fully eager restore kvm FPU
> 
> On 08/16/2012 12:48 PM, Hao, Xudong wrote:
> >> -----Original Message-----
> >> From: kvm-owner@xxxxxxxxxxxxxxx [mailto:kvm-owner@xxxxxxxxxxxxxxx] On
> >> Behalf Of Avi Kivity
> >> Sent: Thursday, August 16, 2012 5:08 PM
> >> To: Hao, Xudong
> >> Cc: kvm@xxxxxxxxxxxxxxx; Zhang, Xiantao
> >> Subject: Re: [PATCH] kvm/fpu: Enable fully eager restore kvm FPU
> >>
> >> On 08/16/2012 08:14 AM, Xudong Hao wrote:
> >> > Enable KVM FPU fully eager restore, if there is other FPU state which isn't
> >> tracked by
> >> > CR0.TS bit.
> >> >
> >> > Tested with these cases:
> >> > 1) SpecCPU2000 workload( 1 VM, 2 VMs)
> >> > 2) Program for floating point caculate
> >>
> >> Is the motivation performance or correctness?
> >>
> >
> > It's not performance improvement, it could be treated as a correctness. I do
> not say current code has issue, but just as code comment, it's for the other FPU
> state.
> >
> >> > +
> >> >  struct kvm_memory_alias {
> >> >  	__u32 slot;  /* this has a different namespace than memory slots */
> >> >  	__u32 flags;
> >> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >> > index b6379e5..2e628e5 100644
> >> > --- a/arch/x86/kvm/x86.c
> >> > +++ b/arch/x86/kvm/x86.c
> >> > @@ -5966,7 +5966,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.
> >>
> >> Which state is that?
> >>
> >
> > Except the last 3 bits, other bit are these state.
> >
> >> > 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 & ~KVM_XSTATE_LAZY))
> >> > +		kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu);
> >> >  	trace_kvm_fpu(0);
> >> >  }
> >>
> >> Is there no way to track accesses to this extended state?
> >>
> >
> > Because I can't define the extended state now, so using this method. But just
> as I say, the extended state are NO-LAZY except the last 3 bit.
> 
> Ok.  Please check that ~KVM_XSTATE_LAZY expands to 64-bits correctly,
> maybe we need to cast it to u64 before negating it.
> 

Thanks.

+   if (!kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE) ||
+       !(vcpu->arch.xcr0 & ~((u64)KVM_XSTATE_LAZY)))
+       kvm_make_request(KVM_REQ_DEACTIVATE_FPU, vcpu);


> Note that we limit xcr0 to the bits allowed by the host, so the currect
> kernel is safe even on hardware with state that isn't tracked by cr0.ts.
>  But it's better to be safe here.
> 
> Joerg, IIRC LWP uses one of these bits?  Should it be added to the mask?
> 

Bit 62? Maybe LWP should change to eager too, I'm not sure. Joerg?

> --
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux