Re: [PATCH][SVM] Only save/restore MSRs when needed

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

Anthony Liguori wrote:
> I've tested this with 32-bit and 64-bit guests on a 64-bit host and 
> with 32-bit guests on a 32-bit host.
>
> I *think* it's doing the right thing wrt to DEBUGCTL but an extra set 
> of eyes would be helpful.

I think the follow patch preserves the current behavior but I'm starting 
to believe that the current behavior is wrong.

The current code assumes that the guest DEBUGCTL gets loaded/saved 
during vmrun/vmexit.  By my reading of the spec (I finally printed out 
the new one :-)), DEBUGCTL only gets saved if LBR virtualization is 
enabled.  LBR virtualization also triggers the save/restore of the other 
4 debug registers.  The spec seems to suggest that the host version is 
saved before the guest is loaded too.

I think the right behavior is to do nothing if LBR is available and 
enabled; hw will do it all for us.  If LBR isn't available, I think we 
should be saving/restoring not just DEBUGCTL but the other 4.  We can 
further optimize that path though by doing the same sort of tricks I 
have below.

If that all seems sane, I'll submit another patch to fix that up.

Regards,

Anthony Liguori

> Depending on my machine's mood, we're at somewhere between 3200 and 
> 2700 cycles per VMEXIT.
>
> Regards,
>
> Anthony Liguori
> ------------------------------------------------------------------------
>
> From: Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> Subject: [PATCH][SVM] Only save/restore MSRs when needed
>
>   We only have to save/restore MSR_GS_BASE on every VMEXIT.  The rest can be
>   saved/restored when we leave the VCPU.  As a special case, MSR_IA32_DEBUGCTL
>   only needs to be saved/restored on every exit if debugging is enabled in
>   either the host or the guest.
>
> 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-27 17:52:37.200780944 -0500
> +++ kernel/drivers/kvm/svm.c	2007-04-27 18:14:02.098446808 -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,24 @@
>  		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]);
> +
> +	rdmsrl(MSR_IA32_DEBUGCTLMSR, vcpu->svm->host_ia32_debugctl);
>  }
>  
>  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]);
> +
> +	wrmsrl(MSR_IA32_DEBUGCTLMSR, vcpu->svm->host_ia32_debugctl);
> +	/* FIXME: LASTBRANCHFROMIP, LASTBRANCHTOIP, LASTINTFROMIP,
> +	   LASTINTTOIP */
> +
>  	rdtscll(vcpu->host_tsc);
>  	put_cpu();
>  }
> @@ -815,18 +829,16 @@
>  
>  static void load_host_msrs(struct kvm_vcpu *vcpu)
>  {
> -	int i;
> -
> -	for ( i = 0; i < NR_HOST_SAVE_MSRS; i++)
> -		wrmsrl(host_save_msrs[i], vcpu->svm->host_msrs[i]);
> +#ifdef CONFIG_X86_64
> +	wrmsrl(MSR_GS_BASE, vcpu->svm->host_gs_base);
> +#endif
>  }
>  
>  static void save_host_msrs(struct kvm_vcpu *vcpu)
>  {
> -	int i;
> -
> -	for ( i = 0; i < NR_HOST_SAVE_MSRS; i++)
> -		rdmsrl(host_save_msrs[i], vcpu->svm->host_msrs[i]);
> +#ifdef CONFIG_X86_64
> +	rdmsrl(MSR_GS_BASE, vcpu->svm->host_gs_base);
> +#endif
>  }
>  
>  static void new_asid(struct kvm_vcpu *vcpu, struct svm_cpu_data *svm_data)
> @@ -1498,6 +1510,11 @@
>  		load_db_regs(vcpu->svm->db_regs);
>  	}
>  
> +	if ((vcpu->svm->vmcb->save.dr7 & 0xff) ||
> +	    (vcpu->svm->host_dr7 & 0xff)) {
> +		wrmsrl(MSR_IA32_DEBUGCTLMSR, vcpu->svm->host_ia32_debugctl);
> +	}
> +
>  	if (vcpu->fpu_active) {
>  		fx_save(vcpu->host_fx_image);
>  		fx_restore(vcpu->guest_fx_image);
> @@ -1617,6 +1634,11 @@
>  		fx_restore(vcpu->host_fx_image);
>  	}
>  
> +	if ((vcpu->svm->vmcb->save.dr7 & 0xff) ||
> +	    (vcpu->svm->host_dr7 & 0xff)) {
> +		rdmsrl(MSR_IA32_DEBUGCTLMSR, vcpu->svm->host_ia32_debugctl);
> +	}
> +
>  	if ((vcpu->svm->vmcb->save.dr7 & 0xff))
>  		load_db_regs(vcpu->svm->host_db_regs);
>  
> Index: kernel/drivers/kvm/kvm_svm.h
> ===================================================================
> --- kernel.orig/drivers/kvm/kvm_svm.h	2007-04-27 17:52:37.240774864 -0500
> +++ kernel/drivers/kvm/kvm_svm.h	2007-04-27 17:53:42.509852456 -0500
> @@ -9,17 +9,15 @@
>  #include "svm.h"
>  #include "kvm.h"
>  
> -static const u32 host_save_msrs[] = {
> +static const u32 host_save_user_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_FS_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,*/
>  };
>  
> -#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 {
> @@ -32,7 +30,9 @@
>  
>  	u64 next_rip;
>  
> -	u64 host_msrs[NR_HOST_SAVE_MSRS];
> +	u64 host_user_msrs[NR_HOST_SAVE_USER_MSRS];
> +	u64 host_ia32_debugctl;
> +	u64 host_gs_base;
>  	unsigned long host_cr2;
>  	unsigned long host_db_regs[NUM_DB_REGS];
>  	unsigned long host_dr6;
>   
> ------------------------------------------------------------------------
>
> -------------------------------------------------------------------------
> 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/
> ------------------------------------------------------------------------
>
> _______________________________________________
> kvm-devel mailing list
> kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/kvm-devel
>   


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