Re: [PATCH] MSR_EFER acceleration for lighweight VM Exit

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

Dong, Eddie wrote:
> Avi:
>
> 	This patch is to avoid saving and restoring of msr_efer on
> lightweight vmexit.
>
> 	With this patch, the Kernel build get 10% increasement for
> 64bits on 64 bits,
> 	and 5-8% increasement for 32bits on 64 bits.
> 	Vmexit.flat can see ~1185 cycles save for 64 bits on 64 bits,
> which is 29%
> 	save of total VM Exit. It can save ~1206 cycles for 32bits on
> 64bits, which is 
> 	27.5% save of total.
> 	BTW, I use 4 core clovertown processor, the total VM Exit time
> reported by
> 	vmexit.flat is about 2860 cycles now.
>
>   

Very impressive speedup!

Comments below.

>  
> +void cpuid_fix_nx_cap(struct kvm_vcpu *vcpu)
>   

static function, please.

> +{
> +	__u64 efer;
> +
> +	rdmsrl(MSR_EFER, efer);
> +	if ((vcpu->cpuid_nent >= 1) && !(efer & EFER_NX)) {
> +		vcpu->cpuid_entries[1].edx &= ~(1<<20);
>   

Nothing guarantees that function 1 is in entry 1.  You need to iterate 
over the loop.

> +		printk(KERN_INFO ": guest NX capibility removed\n");
>   

The warning should be printed only if the capability is actually present 
in the cpuid_entry.  Otherwise non-nx capable processors will always 
show the warning.

> @@ -41,6 +41,7 @@ static struct page *vmx_io_bitmap_b;
>  #else
>  #define HOST_IS_64 0
>  #endif
> +#define EFER_SR_BITS EFER_SCE
>   

I'm guessing "SR" is save/restore?  Please use the full name.

>  
>  static struct vmcs_descriptor {
>  	int size;
> @@ -56,6 +57,10 @@ static struct vmcs_descriptor {
>  		.ar_bytes = GUEST_##seg##_AR_BYTES,	   	\
>  	}
>  
> +#define  efer_msr_srbits_changed(vcpu)
> \
> +	((vcpu->host_msrs[vcpu->msr_offset_efer].data & EFER_SR_BITS)!=
> \
> +	(vcpu->guest_msrs[vcpu->msr_offset_efer].data & EFER_SR_BITS))
>   

static function instead of macro.

> +
>  static struct kvm_vmx_segment_field {
>  	unsigned selector;
>  	unsigned base;
> @@ -264,6 +269,18 @@ static void reload_tss(void)
>  #endif
>  }
>  
> +void load_transition_efer(struct kvm_vcpu *vcpu)
>   

static function

> +{
> +	__u64 msr_efer_trans;
>   

kernel code should use u64, not __u64.  __u64 is for kernel code that is 
shared with userspace.

>  
>  static void vmx_load_host_state(struct kvm_vcpu *vcpu)
> @@ -335,6 +354,8 @@ static void vmx_load_host_state(struct kvm_vcpu
> *vcpu)
>  	}
>  	save_msrs(vcpu->guest_msrs, vcpu->save_nmsrs);
>  	load_msrs(vcpu->host_msrs, vcpu->save_nmsrs);
> +	if (efer_msr_srbits_changed(vcpu))
> +		load_msrs(vcpu->host_msrs+vcpu->msr_offset_efer, 1);
>   

space around '+'.


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