Re: [PATCH 09/10] Add savevm/loadvm support for MCE

Anthony Liguori
2010-10-20T19:54:45Z

On 10/20/2010 12:43 PM, Marcelo Tosatti wrote:
> Port qemu-kvm's
>
> commit 1bab5d11545d8de5facf46c28630085a2f9651ae
> Author: Huang Ying<ying.huang@intel.com>
> Date: Wed Mar 3 16:52:46 2010 +0800
>
> Add savevm/loadvm support for MCE
>
> MCE registers are saved/load into/from CPUState in
> kvm_arch_save/load_regs. To simulate the MCG_STATUS clearing upon
> reset, MSR_MCG_STATUS is set to 0 for KVM_PUT_RESET_STATE.
>
> Signed-off-by: Marcelo Tosatti<mtosatti@redhat.com>
> Signed-off-by: Avi Kivity<avi@redhat.com>
> ---
> target-i386/kvm.c | 39 ++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 38 insertions(+), 1 deletions(-)
>
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 8e26bc4..1701cb9 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -777,7 +777,7 @@ static int kvm_put_msrs(CPUState *env, int level)
> struct kvm_msr_entry entries[100];
> } msr_data;
> struct kvm_msr_entry *msrs = msr_data.entries;
> - int n = 0;
> + int i, n = 0;
>
> kvm_msr_entry_set(&msrs[n++], MSR_IA32_SYSENTER_CS, env->sysenter_cs);
> kvm_msr_entry_set(&msrs[n++], MSR_IA32_SYSENTER_ESP, env->sysenter_esp);
> @@ -797,6 +797,18 @@ static int kvm_put_msrs(CPUState *env, int level)
> env->system_time_msr);
> kvm_msr_entry_set(&msrs[n++], MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
> }
> +#ifdef KVM_CAP_MCE
> + if (env->mcg_cap) {
> + if (level == KVM_PUT_RESET_STATE)
> + kvm_msr_entry_set(&msrs[n++], MSR_MCG_STATUS, env->mcg_status);
> + else if (level == KVM_PUT_FULL_STATE) {
> + kvm_msr_entry_set(&msrs[n++], MSR_MCG_STATUS, env->mcg_status);
> + kvm_msr_entry_set(&msrs[n++], MSR_MCG_CTL, env->mcg_ctl);
> + for (i = 0; i< (env->mcg_cap& 0xff) * 4; i++)
> + kvm_msr_entry_set(&msrs[n++], MSR_MC0_CTL + i, env->mce_banks[i]);
> + }
> + }
> +#endif
>

What happens if we live migration from a kernel with KVM_CAP_MCE to a
kernel without KVM_CAP_MCE. Don't we need to bump a version somewhere?

> msr_data.info.nmsrs = n;
>
> @@ -1004,6 +1016,15 @@ static int kvm_get_msrs(CPUState *env)
> msrs[n++].index = MSR_KVM_SYSTEM_TIME;
> msrs[n++].index = MSR_KVM_WALL_CLOCK;
>
> +#ifdef KVM_CAP_MCE
> + if (env->mcg_cap) {
> + msrs[n++].index = MSR_MCG_STATUS;
> + msrs[n++].index = MSR_MCG_CTL;
> + for (i = 0; i< (env->mcg_cap& 0xff) * 4; i++)
> + msrs[n++].index = MSR_MC0_CTL + i;
> + }
> +#endif
> +
>

This patch does not respect CODING_STYLE with respect to single line ifs
at all.

Regards,

Anthony Liguori

> msr_data.info.nmsrs = n;
> ret = kvm_vcpu_ioctl(env, KVM_GET_MSRS,&msr_data);
> if (ret< 0)
> @@ -1046,6 +1067,22 @@ static int kvm_get_msrs(CPUState *env)
> case MSR_KVM_WALL_CLOCK:
> env->wall_clock_msr = msrs[i].data;
> break;
> +#ifdef KVM_CAP_MCE
> + case MSR_MCG_STATUS:
> + env->mcg_status = msrs[i].data;
> + break;
> + case MSR_MCG_CTL:
> + env->mcg_ctl = msrs[i].data;
> + break;
> +#endif
> + default:
> +#ifdef KVM_CAP_MCE
> + if (msrs[i].index>= MSR_MC0_CTL&&
> + msrs[i].index< MSR_MC0_CTL + (env->mcg_cap& 0xff) * 4) {
> + env->mce_banks[msrs[i].index - MSR_MC0_CTL] = msrs[i].data;
> + break;
> + }
> +#endif
> }
> }
>
>