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