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 <[EMAIL PROTECTED]> > 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 <[EMAIL PROTECTED]> > > 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@lists.sourceforge.net > 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/ _______________________________________________ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel