Re: [PATCH][UPDATE] Shortcut MSR save/restore for lightweight VM Exit

Avi Kivity
2007-05-08T11:15:18Z

Dong, Eddie wrote:
> kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org wrote:
>
>> BTW, I have another patch in hand to further reduce MSR
>> save/restore and
>> thus
>> improve performance more for lightweight VM Exit. Base on my
>> observation for FC5 32 bits
>> guest, 93% VM Exit will fall into the lightweight path.
>>
>>
> This patch reduce the VM Exit handling cost continuously
> for those lightweight VM Exit which occupies 93% of VM Exit in
> KB case if 64 bits OS has similar situation with 32 bits. In my old
> machine, I saw 20% performance increasement of KB within 64 bits
> RHEL5 guest and flat for 32bits FC5.
> There are still some room to improvment here, but this one
> focus on basic MSR save/restore framework only for now and leave
> future to opitmize specific MSRs like GS_BASE etc.
> thx,eddie
>
> Signed-off-by: Yaozu(Eddie) Dong Eddie.Dong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
>
> against 5cf48c367dec74ba8553c53ed332cd075fa38b88
>
>

Much has changed. Please rebase against HEAD.

Also, there have been a lot of regressions with the msr code. Please
test on i386 and on Core Duo i386 (which is a little different) in
addition to the regular x86_64.

> commit a7294eae555b7d42f7e44b8d7955becad2feebf8
> Author: root <root@vt32-pae.(none)>
> Date: Tue May 8 17:32:24 2007 +0800
>
> Avoid MSR save/restore for lightweight VM Exit
>
> Signed-off-by: Yaozu(Eddie) Dong Eddie.Dong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
>
> diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h
> index 11eb25e..86abf2d 100644
> --- a/drivers/kvm/kvm.h
> +++ b/drivers/kvm/kvm.h
> @@ -285,6 +285,7 @@ struct kvm_vcpu {
> u64 apic_base;
> u64 ia32_misc_enable_msr;
> int nmsrs;
> + int sw_save_msrs;
> struct vmx_msr_entry *guest_msrs;
> struct vmx_msr_entry *host_msrs;
>
> diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
> index 4e04b85..c2d06b5 100644
> --- a/drivers/kvm/vmx.c
> +++ b/drivers/kvm/vmx.c
> @@ -80,23 +80,11 @@ static const u32 vmx_msr_index[] = {
> #ifdef CONFIG_X86_64
> MSR_SYSCALL_MASK, MSR_LSTAR, MSR_CSTAR, MSR_KERNEL_GS_BASE,
> #endif
> - MSR_EFER, MSR_K6_STAR,
> + MSR_K6_STAR, MSR_EFER,
> };
> +#define NR_HW_SAVE_MSRS 1 /* HW save MSR_EFER */
> #define NR_VMX_MSR ARRAY_SIZE(vmx_msr_index)
>

The code has a comment that MSR_K6_STAR should be last... the comment
should be removed.

Also, does this mean that software msr saving is faster than hardware
msr saving? Will this be true in future processors as well?

>
> static inline int is_page_fault(u32 intr_info)
> {
> return (intr_info & (INTR_INFO_INTR_TYPE_MASK |
> INTR_INFO_VECTOR_MASK |
> @@ -339,23 +327,19 @@ 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;
> + int nr_skip;
>
> - if (is_long_mode(vcpu))
> - nr_skip = NR_BAD_MSRS;
> - else
> - nr_skip = NR_64BIT_MSRS;
> - nr_good_msrs = vcpu->nmsrs - nr_skip;
> + vcpu->sw_save_msrs = nr_skip = vcpu->nmsrs - NR_HW_SAVE_MSRS;
>

One assignment per statement please.

--
error compiling committee.c: too many arguments to function