Re: [PATCH] Lazy FPU for SVM

[Anthony Liguori <anthony-rdkfGonbjUSkNkDKm+mE6A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2223 bytes --]

Avi Kivity wrote:
> Anthony Liguori wrote:
>> Author: Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
>> Date:   Sun Apr 22 20:34:03 2007 -0500
>>
>>     Lazy FPU support for SVM.
>>         Signed-off-by: Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
>>
>> 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

[-- Attachment #2: kvm-svm-lazy-fpu-2.diff --]
[-- Type: text/x-patch, Size: 3391 bytes --]

Author: Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Date:   Mon Apr 23 09:17:21 2007 -0500

    Lazy FPU support for SVM.
    
    Signed-off-by: Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

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,

[-- Attachment #3: Type: text/plain, Size: 286 bytes --]

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

[-- Attachment #4: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel