[PATCH][SVM] Lazy fast syscall MSR save/restore

[PATCH][SVM] Lazy fast syscall MSR save/restore

[Anthony Liguori]

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

Howdy,

The attached patch only reloads the host fast syscall MSRs when we leave 
KVM (when we put_cpu()).  Since we'll never execute a 
sys{enter,call,ret,exit} until we put_cpu() in the host, this should be 
safe.

This has a huge impact on in-kernel vmexit performance.  I see a savings 
of about a 1,000 cycles going from 4,700 to about 3,700 or ~21%.

I've only tested on a 64bit host with a 32bit FC6 guest but I wanted to 
get it out before calling it a night.  I had fumbled my previous attempt 
at this by switching the rdmsr/wrmsrs around so ignore my previous 
rambles about improperly blocking :-)

Regards,

Anthony Liguori

[-- Attachment #2: svm-lazy-msrs.diff --]
[-- Type: text/x-patch, Size: 2423 bytes --]

From: Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Subject: [PATCH][SVM] Only save fast syscall MSRS when leaving VCPU

  We only need to restore the host fast syscall related MSRS when leaving the
  VCPU.  This should only occur when we {get,put}_cpu().

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

Index: kernel/drivers/kvm/svm.c
===================================================================
--- kernel.orig/drivers/kvm/svm.c	2007-04-26 18:07:09.909905480 -0500
+++ kernel/drivers/kvm/svm.c	2007-04-27 00:38:57.671182560 -0500
@@ -611,7 +611,7 @@
 
 static void svm_vcpu_load(struct kvm_vcpu *vcpu)
 {
-	int cpu;
+	int cpu, i;
 
 	cpu = get_cpu();
 	if (unlikely(cpu != vcpu->cpu)) {
@@ -626,10 +626,18 @@
 		vcpu->svm->vmcb->control.tsc_offset += delta;
 		vcpu->cpu = cpu;
 	}
+
+	for (i = 0; i < NR_HOST_SAVE_USER_MSRS; i++)
+		rdmsrl(host_save_user_msrs[i], vcpu->svm->host_user_msrs[i]);
 }
 
 static void svm_vcpu_put(struct kvm_vcpu *vcpu)
 {
+	int i;
+
+	for (i = 0; i < NR_HOST_SAVE_USER_MSRS; i++)
+		wrmsrl(host_save_user_msrs[i], vcpu->svm->host_user_msrs[i]);
+
 	rdtscll(vcpu->host_tsc);
 	put_cpu();
 }
Index: kernel/drivers/kvm/kvm_svm.h
===================================================================
--- kernel.orig/drivers/kvm/kvm_svm.h	2007-04-26 18:07:09.949899400 -0500
+++ kernel/drivers/kvm/kvm_svm.h	2007-04-27 00:38:30.310342040 -0500
@@ -11,15 +11,19 @@
 
 static const u32 host_save_msrs[] = {
 #ifdef CONFIG_X86_64
-	MSR_STAR, MSR_LSTAR, MSR_CSTAR, MSR_SYSCALL_MASK, MSR_KERNEL_GS_BASE,
-	MSR_FS_BASE, MSR_GS_BASE,
+	MSR_KERNEL_GS_BASE, MSR_FS_BASE, MSR_GS_BASE,
 #endif
-	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
 	MSR_IA32_DEBUGCTLMSR, /*MSR_IA32_LASTBRANCHFROMIP,
 	MSR_IA32_LASTBRANCHTOIP, MSR_IA32_LASTINTFROMIP,MSR_IA32_LASTINTTOIP,*/
 };
 
+static const u32 host_save_user_msrs[] = {
+	MSR_STAR, MSR_LSTAR, MSR_CSTAR, MSR_SYSCALL_MASK,
+	MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
+};
+
 #define NR_HOST_SAVE_MSRS ARRAY_SIZE(host_save_msrs)
+#define NR_HOST_SAVE_USER_MSRS ARRAY_SIZE(host_save_user_msrs)
 #define NUM_DB_REGS 4
 
 struct vcpu_svm {
@@ -33,6 +37,7 @@
 	u64 next_rip;
 
 	u64 host_msrs[NR_HOST_SAVE_MSRS];
+	u64 host_user_msrs[NR_HOST_SAVE_USER_MSRS];
 	unsigned long host_cr2;
 	unsigned long host_db_regs[NUM_DB_REGS];
 	unsigned long host_dr6;

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

Re: [PATCH][SVM] Lazy fast syscall MSR save/restore

[Avi Kivity]

Anthony Liguori wrote:
> Howdy,
>
> The attached patch only reloads the host fast syscall MSRs when we
> leave KVM (when we put_cpu()).  Since we'll never execute a
> sys{enter,call,ret,exit} until we put_cpu() in the host, this should
> be safe.
>
> This has a huge impact on in-kernel vmexit performance.  I see a
> savings of about a 1,000 cycles going from 4,700 to about 3,700 or ~21%.
>

Which means on a 3GHz Opteron we're getting close to a million vmexits
per second when spinning...

>  
>  static void svm_vcpu_load(struct kvm_vcpu *vcpu)
>  {
> -	int cpu;
> +	int cpu, i;
>  
>  	cpu = get_cpu();
>  	if (unlikely(cpu != vcpu->cpu)) {
> @@ -626,10 +626,18 @@
>  		vcpu->svm->vmcb->control.tsc_offset += delta;
>  		vcpu->cpu = cpu;
>  	}
> +
> +	for (i = 0; i < NR_HOST_SAVE_USER_MSRS; i++)
> +		rdmsrl(host_save_user_msrs[i], vcpu->svm->host_user_msrs[i]);
>  }
>   

All the msrs you mentioned are constant throughout the lifetime of the
host and don't vary by processor, so the save loop can be moved to
module init (it's in vcpu init for vmx).  Not sure how expensive rdmsr
is, though, so it may not be worthwhile to move it, see below.

>  
> Index: kernel/drivers/kvm/kvm_svm.h
> ===================================================================
> --- kernel.orig/drivers/kvm/kvm_svm.h	2007-04-26 18:07:09.949899400 -0500
> +++ kernel/drivers/kvm/kvm_svm.h	2007-04-27 00:38:30.310342040 -0500
> @@ -11,15 +11,19 @@
>  
>  static const u32 host_save_msrs[] = {
>  #ifdef CONFIG_X86_64
> -	MSR_STAR, MSR_LSTAR, MSR_CSTAR, MSR_SYSCALL_MASK, MSR_KERNEL_GS_BASE,
> -	MSR_FS_BASE, MSR_GS_BASE,
> +	MSR_KERNEL_GS_BASE, MSR_FS_BASE, MSR_GS_BASE,
>   

MSR_KERNEL_GS_BASE, as its name so clearly indicates, is a user msr. 
True, while executing in userspace it holds a kernel address, but once
we hit the kernel we execute swapgs, which writes the value of
MSR_GS_BASE into MSR_KERNEL_GS_BASE.  So, we can save/restore it on
vcpu_load()/vcpu_put().

MSR_FS_BASE is a also a user msr (MSR_GS_BASE is not, since it holds the
value of MSR_KERNEL_GS_BASE while in the kernel and is used to access
the pda).

An additional consideration is that non-longmode guests will not touch
longmode msrs, so the saving and loading thereof can be avoided
completely.  The vmx code does that.  It's worthy of an independenet
patch, though.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


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

Re: [PATCH][SVM] Lazy fast syscall MSR save/restore

[Anthony Liguori]

Avi Kivity wrote:
> Anthony Liguori wrote:
>   
>> Howdy,
>>
>> The attached patch only reloads the host fast syscall MSRs when we
>> leave KVM (when we put_cpu()).  Since we'll never execute a
>> sys{enter,call,ret,exit} until we put_cpu() in the host, this should
>> be safe.
>>
>> This has a huge impact on in-kernel vmexit performance.  I see a
>> savings of about a 1,000 cycles going from 4,700 to about 3,700 or ~21%.
>>
>>     
>
> Which means on a 3GHz Opteron we're getting close to a million vmexits
> per second when spinning...
>
>   
>>  
>>  static void svm_vcpu_load(struct kvm_vcpu *vcpu)
>>  {
>> -	int cpu;
>> +	int cpu, i;
>>  
>>  	cpu = get_cpu();
>>  	if (unlikely(cpu != vcpu->cpu)) {
>> @@ -626,10 +626,18 @@
>>  		vcpu->svm->vmcb->control.tsc_offset += delta;
>>  		vcpu->cpu = cpu;
>>  	}
>> +
>> +	for (i = 0; i < NR_HOST_SAVE_USER_MSRS; i++)
>> +		rdmsrl(host_save_user_msrs[i], vcpu->svm->host_user_msrs[i]);
>>  }
>>   
>>     
>
> All the msrs you mentioned are constant throughout the lifetime of the
> host and don't vary by processor, so the save loop can be moved to
> module init (it's in vcpu init for vmx).  Not sure how expensive rdmsr
> is, though, so it may not be worthwhile to move it, see below.
>
>   
>>  
>> Index: kernel/drivers/kvm/kvm_svm.h
>> ===================================================================
>> --- kernel.orig/drivers/kvm/kvm_svm.h	2007-04-26 18:07:09.949899400 -0500
>> +++ kernel/drivers/kvm/kvm_svm.h	2007-04-27 00:38:30.310342040 -0500
>> @@ -11,15 +11,19 @@
>>  
>>  static const u32 host_save_msrs[] = {
>>  #ifdef CONFIG_X86_64
>> -	MSR_STAR, MSR_LSTAR, MSR_CSTAR, MSR_SYSCALL_MASK, MSR_KERNEL_GS_BASE,
>> -	MSR_FS_BASE, MSR_GS_BASE,
>> +	MSR_KERNEL_GS_BASE, MSR_FS_BASE, MSR_GS_BASE,
>>   
>>     
>
> MSR_KERNEL_GS_BASE, as its name so clearly indicates, is a user msr. 
> True, while executing in userspace it holds a kernel address, but once
> we hit the kernel we execute swapgs, which writes the value of
> MSR_GS_BASE into MSR_KERNEL_GS_BASE.  So, we can save/restore it on
> vcpu_load()/vcpu_put().
>   

This trims another 200 cycles.  Another one I wanted to look at today 
was DEBUGCTL but I can't seem to figure out how it gets loaded by the 
guest.  DEBUGCTL costs 300 cycles to save/restore.

> MSR_FS_BASE is a also a user msr (MSR_GS_BASE is not, since it holds the
> value of MSR_KERNEL_GS_BASE while in the kernel and is used to access
> the pda).
>
> An additional consideration is that non-longmode guests will not touch
> longmode msrs, so the saving and loading thereof can be avoided
> completely.  The vmx code does that.  It's worthy of an independenet
> patch, though.
>   

Regards,

Anthony Liguori


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