> -----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