From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [PATCH] Lazy FPU for SVM Date: Mon, 23 Apr 2007 09:32:32 -0500 Message-ID: <462CC380.5020405@codemonkey.ws> References: <462C2048.4040700@us.ibm.com> <462C574E.2030304@qumranet.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------050405010102080208080709" Cc: kvm-devel To: Avi Kivity Return-path: In-Reply-To: <462C574E.2030304-atKUWr5tajBWk0Htik3J/w@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org This is a multi-part message in MIME format. --------------050405010102080208080709 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Avi Kivity wrote: > Anthony Liguori wrote: >> Author: Anthony Liguori >> Date: Sun Apr 22 20:34:03 2007 -0500 >> >> Lazy FPU support for SVM. >> Signed-off-by: Anthony Liguori >> >> diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h >> index d1a90c5..4859c32 100644 >> --- a/drivers/kvm/kvm.h >> +++ b/drivers/kvm/kvm.h >> @@ -63,6 +63,9 @@ >> #define FX_BUF_SIZE (2 * FX_IMAGE_SIZE + FX_IMAGE_ALIGN) >> >> #define DE_VECTOR 0 >> +#define DB_VECTOR 2 >> +#define UD_VECTOR 6 >> +#define NM_VECTOR 7 >> > > This, while a nice cleanup, is unrelated. I had to add the NM_VECTOR so I moved the others too. Attached is a patch that doesn't move those. I noticed that the #define's in general need some love. There's quite a lot in kvm.h that's specific to vmx and there's a bit of asymmetry between vmx and svm (svm has kvm_svm.h, vmx used to but it was removed recently). There's also a ton of unused #define's (quite a few that are typos). I was planning on submitting a cleanup patch series today so I'll just fold the above changes into that. >> >> +static int nm_interception(struct kvm_vcpu *vcpu, struct kvm_run >> *kvm_run) >> +{ >> + spin_lock(&vcpu->kvm->lock); >> > > Why is the lock needed? everything below is vcpu-local AFAICS. You're right, removed in attached patch. >> @@ -1664,6 +1685,12 @@ static void svm_set_cr3(struct kvm_vcpu *vcpu, >> unsigned long root) >> { >> vcpu->svm->vmcb->save.cr3 = root; >> force_new_asid(vcpu); >> + + if (vcpu->fpu_active) { >> + vcpu->svm->vmcb->control.intercept_exceptions |= (1 << >> NM_VECTOR); >> + vcpu->svm->vmcb->save.cr0 |= CR0_TS_MASK; >> + vcpu->fpu_active = 0; >> + } >> } >> >> static void svm_inject_page_fault(struct kvm_vcpu *vcpu, >> > > Any numbers? I use user/test/vmexit.c to test as it gives the highest > relative speedup. Hard to say exactly because of the noise. I did two runs of 4 test/vmexit: Before 4091, 4194, 4559, 4439 After: 3979, 4324, 3918, 3910 So there's definitely a speedup, but probably only 100-200 cycles. Regards, Anthony Liguori --------------050405010102080208080709 Content-Type: text/x-patch; name="kvm-svm-lazy-fpu-2.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="kvm-svm-lazy-fpu-2.diff" Author: Anthony Liguori Date: Mon Apr 23 09:17:21 2007 -0500 Lazy FPU support for SVM. Signed-off-by: Anthony Liguori diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h index d1a90c5..61ff085 100644 --- a/drivers/kvm/kvm.h +++ b/drivers/kvm/kvm.h @@ -63,6 +63,7 @@ #define FX_BUF_SIZE (2 * FX_IMAGE_SIZE + FX_IMAGE_ALIGN) #define DE_VECTOR 0 +#define NM_VECTOR 7 #define DF_VECTOR 8 #define TS_VECTOR 10 #define NP_VECTOR 11 @@ -301,6 +302,7 @@ struct kvm_vcpu { char fx_buf[FX_BUF_SIZE]; char *host_fx_image; char *guest_fx_image; + int fpu_active; int mmio_needed; int mmio_read_completed; diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c index 644efc5..9438931 100644 --- a/drivers/kvm/svm.c +++ b/drivers/kvm/svm.c @@ -587,6 +587,7 @@ static int svm_create_vcpu(struct kvm_vcpu *vcpu) init_vmcb(vcpu->svm->vmcb); fx_init(vcpu); + vcpu->fpu_active = 1; vcpu->apic_base = 0xfee00000 | /*for vcpu 0*/ MSR_IA32_APICBASE_BSP | MSR_IA32_APICBASE_ENABLE; @@ -756,6 +757,11 @@ static void svm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0) } } #endif + if ((vcpu->cr0 & CR0_TS_MASK) && !(cr0 & CR0_TS_MASK)) { + vcpu->svm->vmcb->control.intercept_exceptions &= ~(1 << NM_VECTOR); + vcpu->fpu_active = 1; + } + vcpu->cr0 = cr0; cr0 |= CR0_PG_MASK | CR0_WP_MASK; cr0 &= ~(CR0_CD_MASK | CR0_NW_MASK); @@ -928,6 +934,16 @@ static int pf_interception(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) return 0; } +static int nm_interception(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) +{ + vcpu->svm->vmcb->control.intercept_exceptions &= ~(1 << NM_VECTOR); + if (!(vcpu->cr0 & CR0_TS_MASK)) + vcpu->svm->vmcb->save.cr0 &= ~CR0_TS_MASK; + vcpu->fpu_active = 1; + + return 1; +} + static int shutdown_interception(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) { /* @@ -1292,6 +1308,7 @@ static int (*svm_exit_handlers[])(struct kvm_vcpu *vcpu, [SVM_EXIT_WRITE_DR5] = emulate_on_interception, [SVM_EXIT_WRITE_DR7] = emulate_on_interception, [SVM_EXIT_EXCP_BASE + PF_VECTOR] = pf_interception, + [SVM_EXIT_EXCP_BASE + NM_VECTOR] = nm_interception, [SVM_EXIT_INTR] = nop_on_interception, [SVM_EXIT_NMI] = nop_on_interception, [SVM_EXIT_SMI] = nop_on_interception, @@ -1481,8 +1498,10 @@ again: load_db_regs(vcpu->svm->db_regs); } - fx_save(vcpu->host_fx_image); - fx_restore(vcpu->guest_fx_image); + if (vcpu->fpu_active) { + fx_save(vcpu->host_fx_image); + fx_restore(vcpu->guest_fx_image); + } asm volatile ( #ifdef CONFIG_X86_64 @@ -1593,8 +1612,10 @@ again: #endif : "cc", "memory" ); - fx_save(vcpu->guest_fx_image); - fx_restore(vcpu->host_fx_image); + if (vcpu->fpu_active) { + fx_save(vcpu->guest_fx_image); + fx_restore(vcpu->host_fx_image); + } if ((vcpu->svm->vmcb->save.dr7 & 0xff)) load_db_regs(vcpu->svm->host_db_regs); @@ -1664,6 +1685,12 @@ static void svm_set_cr3(struct kvm_vcpu *vcpu, unsigned long root) { vcpu->svm->vmcb->save.cr3 = root; force_new_asid(vcpu); + + if (vcpu->fpu_active) { + vcpu->svm->vmcb->control.intercept_exceptions |= (1 << NM_VECTOR); + vcpu->svm->vmcb->save.cr0 |= CR0_TS_MASK; + vcpu->fpu_active = 0; + } } static void svm_inject_page_fault(struct kvm_vcpu *vcpu, --------------050405010102080208080709 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ --------------050405010102080208080709 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ kvm-devel mailing list kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org https://lists.sourceforge.net/lists/listinfo/kvm-devel --------------050405010102080208080709--