| Message ID | 1345094044-28962-1-git-send-email-xudong.hao@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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? > + > 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? > 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? Although I expect that on modern hardware which exits rarely, eager fpu reload might be more performant.
> -----Original Message----- > From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On > Behalf Of Avi Kivity > Sent: Thursday, August 16, 2012 5:08 PM > To: Hao, Xudong > Cc: kvm@vger.kernel.org; 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. > Although I expect that on modern hardware which exits rarely, eager fpu > reload might be more performant. > > > -- > 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@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/16/2012 12:48 PM, Hao, Xudong wrote: >> -----Original Message----- >> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On >> Behalf Of Avi Kivity >> Sent: Thursday, August 16, 2012 5:08 PM >> To: Hao, Xudong >> Cc: kvm@vger.kernel.org; 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. 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?
> -----Original Message----- > From: Avi Kivity [mailto:avi@redhat.com] > Sent: Thursday, August 16, 2012 6:59 PM > To: Hao, Xudong > Cc: kvm@vger.kernel.org; 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@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On > >> Behalf Of Avi Kivity > >> Sent: Thursday, August 16, 2012 5:08 PM > >> To: Hao, Xudong > >> Cc: kvm@vger.kernel.org; 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
(Back from vacation) On Thu, Aug 16, 2012 at 01:59:02PM +0300, Avi Kivity wrote: > Ok. Please check that ~KVM_XSTATE_LAZY expands to 64-bits correctly, > maybe we need to cast it to u64 before negating it. > > 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? LWP uses bit 62 in xcr0 and is not tracked by cr0.ts either. So this bit should be used to the mask too (in other words LWP is a non-lazy state). Regards, Joerg
On 08/20/2012 12:24 PM, Roedel, Joerg wrote: > (Back from vacation) > > On Thu, Aug 16, 2012 at 01:59:02PM +0300, Avi Kivity wrote: >> Ok. Please check that ~KVM_XSTATE_LAZY expands to 64-bits correctly, >> maybe we need to cast it to u64 before negating it. >> >> 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? > > LWP uses bit 62 in xcr0 and is not tracked by cr0.ts either. So this bit > should be used to the mask too (in other words LWP is a non-lazy state). So it was broken all along? Yikes.
On Mon, Aug 20, 2012 at 01:08:14PM +0300, Avi Kivity wrote: > On 08/20/2012 12:24 PM, Roedel, Joerg wrote: > So it was broken all along? Yikes. There is no LWP support in the kernel and thus KVM can't expose it to guests. So for now nothing should be broken, no? Joerg -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/20/2012 01:14 PM, Roedel, Joerg wrote: > On Mon, Aug 20, 2012 at 01:08:14PM +0300, Avi Kivity wrote: >> On 08/20/2012 12:24 PM, Roedel, Joerg wrote: > >> So it was broken all along? Yikes. > > There is no LWP support in the kernel and thus KVM can't expose it to > guests. So for now nothing should be broken, no? Oh, we mask out xcr0 bits not supported by the host. So it's broken in another way: it isn't exposed. Pity, it's such a nice feature.
On Mon, Aug 20, 2012 at 01:43:10PM +0300, Avi Kivity wrote: > On 08/20/2012 01:14 PM, Roedel, Joerg wrote: > > On Mon, Aug 20, 2012 at 01:08:14PM +0300, Avi Kivity wrote: > >> On 08/20/2012 12:24 PM, Roedel, Joerg wrote: > > > >> So it was broken all along? Yikes. > > > > There is no LWP support in the kernel and thus KVM can't expose it to > > guests. So for now nothing should be broken, no? > > Oh, we mask out xcr0 bits not supported by the host. > > So it's broken in another way: it isn't exposed. Pity, it's such a nice > feature. Right, that is very sad. We can support it in KVM even without Linux host support, but there is no OS which enables this feature yet, so it is rather useless to do it in KVM only. Joerg -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On > Behalf Of Avi Kivity > Sent: Monday, August 20, 2012 6:43 PM > To: Roedel, Joerg > Cc: Hao, Xudong; kvm@vger.kernel.org; Zhang, Xiantao > Subject: Re: [PATCH] kvm/fpu: Enable fully eager restore kvm FPU > > On 08/20/2012 01:14 PM, Roedel, Joerg wrote: > > On Mon, Aug 20, 2012 at 01:08:14PM +0300, Avi Kivity wrote: > >> On 08/20/2012 12:24 PM, Roedel, Joerg wrote: > > > >> So it was broken all along? Yikes. > > > > There is no LWP support in the kernel and thus KVM can't expose it to > > guests. So for now nothing should be broken, no? > > Oh, we mask out xcr0 bits not supported by the host. > > So it's broken in another way: it isn't exposed. Pity, it's such a nice > feature. > Hi, Avi/Joerg What's the decision for it? I don't understand LWP, so how about this patch? > -- > 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@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/23/2012 11:51 AM, Hao, Xudong wrote: >> -----Original Message----- >> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On >> Behalf Of Avi Kivity >> Sent: Monday, August 20, 2012 6:43 PM >> To: Roedel, Joerg >> Cc: Hao, Xudong; kvm@vger.kernel.org; Zhang, Xiantao >> Subject: Re: [PATCH] kvm/fpu: Enable fully eager restore kvm FPU >> >> On 08/20/2012 01:14 PM, Roedel, Joerg wrote: >> > On Mon, Aug 20, 2012 at 01:08:14PM +0300, Avi Kivity wrote: >> >> On 08/20/2012 12:24 PM, Roedel, Joerg wrote: >> > >> >> So it was broken all along? Yikes. >> > >> > There is no LWP support in the kernel and thus KVM can't expose it to >> > guests. So for now nothing should be broken, no? >> >> Oh, we mask out xcr0 bits not supported by the host. >> >> So it's broken in another way: it isn't exposed. Pity, it's such a nice >> feature. >> > > Hi, Avi/Joerg > > What's the decision for it? I don't understand LWP, so how about this patch? It's fine (Joerg can send the LWP change), but there was a truncation issue that needs fixing, no?
> -----Original Message----- > From: Avi Kivity [mailto:avi@redhat.com] > Sent: Monday, September 03, 2012 5:23 PM > To: Hao, Xudong > Cc: Roedel, Joerg; kvm@vger.kernel.org; Zhang, Xiantao > Subject: Re: [PATCH] kvm/fpu: Enable fully eager restore kvm FPU > > On 08/23/2012 11:51 AM, Hao, Xudong wrote: > >> -----Original Message----- > >> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On > >> Behalf Of Avi Kivity > >> Sent: Monday, August 20, 2012 6:43 PM > >> To: Roedel, Joerg > >> Cc: Hao, Xudong; kvm@vger.kernel.org; Zhang, Xiantao > >> Subject: Re: [PATCH] kvm/fpu: Enable fully eager restore kvm FPU > >> > >> On 08/20/2012 01:14 PM, Roedel, Joerg wrote: > >> > On Mon, Aug 20, 2012 at 01:08:14PM +0300, Avi Kivity wrote: > >> >> On 08/20/2012 12:24 PM, Roedel, Joerg wrote: > >> > > >> >> So it was broken all along? Yikes. > >> > > >> > There is no LWP support in the kernel and thus KVM can't expose it to > >> > guests. So for now nothing should be broken, no? > >> > >> Oh, we mask out xcr0 bits not supported by the host. > >> > >> So it's broken in another way: it isn't exposed. Pity, it's such a nice > >> feature. > >> > > > > Hi, Avi/Joerg > > > > What's the decision for it? I don't understand LWP, so how about this patch? > > It's fine (Joerg can send the LWP change), but there was a truncation > issue that needs fixing, no? > Yes, I think you means to expand KVM_XSTATE_LAZY to 64-bits, I'll send another version patch. Thanks, -Xudong -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/include/asm/kvm.h b/arch/x86/include/asm/kvm.h index 246617e..91aa1cd 100644 --- a/arch/x86/include/asm/kvm.h +++ b/arch/x86/include/asm/kvm.h @@ -8,6 +8,8 @@ #include <linux/types.h> #include <linux/ioctl.h> +#include <asm/user.h> +#include <asm/xsave.h> /* Select x86 specific features in <linux/kvm.h> */ #define __KVM_HAVE_PIT @@ -29,6 +31,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/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. 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); }
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 Signed-off-by: Xudong Hao <xudong.hao@intel.com> --- arch/x86/include/asm/kvm.h | 4 ++++ arch/x86/kvm/x86.c | 13 ++++++++++++- 2 files changed, 16 insertions(+), 1 deletions(-)