From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] lighweight VM Exit Date: Mon, 14 May 2007 18:14:31 +0300 Message-ID: <46487CD7.8070408@qumranet.com> References: <10EA09EFD8728347A513008B6B0DA77A016FBD91@pdsmsx411.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel To: "Dong, Eddie" Return-path: In-Reply-To: <10EA09EFD8728347A513008B6B0DA77A016FBD91-wq7ZOvIWXbNpB2pF5aRoyrfspsVTdybXVpNB7YpNyf8@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 Dong, Eddie wrote: > OK, how about this patch which further reduce the light weight VM Exit > MSR save/restore? > > > diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c > index 1288cff..ef96fae 100644 > --- a/drivers/kvm/kvm_main.c > +++ b/drivers/kvm/kvm_main.c > @@ -1596,6 +1596,30 @@ void kvm_resched(struct kvm_vcpu *vcpu) > } > EXPORT_SYMBOL_GPL(kvm_resched); > > +void load_msrs_select(struct vmx_msr_entry *e, int bitmap) > +{ > + unsigned long nr; > + > + while (bitmap) { > + nr = __ffs(bitmap); > + clear_bit(nr,&bitmap); > + wrmsrl(e[nr].index, e[nr].data); > + } > +} > +EXPORT_SYMBOL_GPL(load_msrs_select); > + > +void save_msrs_select(struct vmx_msr_entry *e, int bitmap) > +{ > + unsigned long nr; > + > + while (bitmap) { > + nr = __ffs(bitmap); > + clear_bit(nr,&bitmap); > + rdmsrl(e[nr].index, e[nr].data); > + } > +} > +EXPORT_SYMBOL_GPL(save_msrs_select); > + > __clear_bit() is faster here (no LOCK prefix). But maybe we can avoid the entire thing by having a vcpu->active_msr_list (array of struct vmx_msr_entry) which is re-constructed every time the mode changes (instead of constructing the bitmap). vmx_get_msr() can first look at the active msr list and then at the regular msr list. > > /* > @@ -469,35 +466,51 @@ static void vmx_inject_gp(struct kvm_vcpu *vcpu, > unsigned error_code) > */ > static void setup_msrs(struct kvm_vcpu *vcpu) > { > - int nr_skip, nr_good_msrs; > - > - if (is_long_mode(vcpu)) > - nr_skip = NR_BAD_MSRS; > - else > - nr_skip = NR_64BIT_MSRS; > - nr_good_msrs = vcpu->nmsrs - nr_skip; > + int index,save_msrs; > space after comma > > - /* > - * MSR_K6_STAR is only needed on long mode guests, and only > - * if efer.sce is enabled. > - */ > - if (find_msr_entry(vcpu, MSR_K6_STAR)) { > - --nr_good_msrs; > -#ifdef CONFIG_X86_64 > - if (is_long_mode(vcpu) && (vcpu->shadow_efer & > EFER_SCE)) > - ++nr_good_msrs; > + vcpu->smsrs_bitmap = 0; > + if (is_long_mode(vcpu)) { > + if ((index=__find_msr_index(vcpu, MSR_SYSCALL_MASK)) >= > 0) { > + set_bit(index, &vcpu->smsrs_bitmap); > + } > Assignment outside if (), spaces around =, please. Single statements without {}. Also __set_bit() applies here. > + /* > + * MSR_K6_STAR is only needed on long mode guests, and > only > + * if efer.sce is enabled. > + */ > + if ((index=__find_msr_index(vcpu, MSR_K6_STAR)) >= 0 > +#ifdef X86_64 > + && (vcpu->shadow_efer & EFER_SCE) > #endif > + ) { > + set_bit(index, &vcpu->smsrs_bitmap); > You're saving MSR_K6_STAR unnecessarily on i386. Since we don't export EFER on i386 (one day we should...), the guest can't use syscall. > + } > } > > + if ((index = __find_msr_index(vcpu, MSR_EFER)) >= 0) { > + save_msrs = 1; > + } > + else { > + save_msrs = 0; > + index = 0; > + } > Why not use hardware autoloading? Is it slower than software? Otherwise looks good. Did you measure performance improvement? I usually use user/test/vmexit.c from kvm-userspace.git. -- error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- 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/