On Tue, Jul 16, 2013 at 07:56:25PM +0800, Arthur Chunqi Li wrote:
> On Tue, Jul 16, 2013 at 7:42 PM, Gleb Natapov <gleb@xxxxxxxxxx> wrote:
> > On Sun, Jul 07, 2013 at 11:13:37PM +0800, Arthur Chunqi Li wrote:
> >> The recent KVM patch adds IA32_FEATURE_CONTROL support. QEMU needs
> >> to clear this MSR when reset vCPU and keep the value of it when
> >> migration. This patch add this feature.
> >>
> > So what happens if we migrate from qemu that does not have this patch
> > to qemu that does? Since msr_ia32_feature_control will not be migrated
> > it will not be set on the destination so destination will not be able to
> > use nested vmx. Since nested vmx is experimental it may be to early for
> > us to care about it though, and nested vmx does not work with migration
> > anyway.
> In my test, if migration doesn't care about msr_ia32_feature_control,
> the value will be set to 0 in the destination VM and this may cause
> some logical confusions, but the VMX running on it may not aware of
> this (if migration nested vmx is supported in the future) because once
> VMX initialized, it will not check this msr any more in normal cases.
>
With vmm_exclusive=0 kvm does vmxon/vmxoff while running. But lest not
worry about nested kvm migration for now. There are much harder problems
to overcome before it will work.
> This is also a complex problem since we don't know how many states
> like this msr need to be migrated related to nested virt. If there're
> a lot of states need migrating, it is better to reconstruct the
> relevant codes. But now this patch is enough.
>
> Besides, though migration is not supported in nested vmx, we should
> keep the machine state consistent during migration. So this patch is
> also meaningful.
>
> Arthur
> >
> >> Signed-off-by: Arthur Chunqi Li <yzt356@xxxxxxxxx>
> >> ---
> >> target-i386/cpu.h | 2 ++
> >> target-i386/kvm.c | 4 ++++
> >> target-i386/machine.c | 22 ++++++++++++++++++++++
> >> 3 files changed, 28 insertions(+)
> >>
> >> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> >> index 62e3547..a418e17 100644
> >> --- a/target-i386/cpu.h
> >> +++ b/target-i386/cpu.h
> >> @@ -301,6 +301,7 @@
> >> #define MSR_IA32_APICBASE_BSP (1<<8)
> >> #define MSR_IA32_APICBASE_ENABLE (1<<11)
> >> #define MSR_IA32_APICBASE_BASE (0xfffff<<12)
> >> +#define MSR_IA32_FEATURE_CONTROL 0x0000003a
> >> #define MSR_TSC_ADJUST 0x0000003b
> >> #define MSR_IA32_TSCDEADLINE 0x6e0
> >>
> >> @@ -813,6 +814,7 @@ typedef struct CPUX86State {
> >>
> >> uint64_t mcg_status;
> >> uint64_t msr_ia32_misc_enable;
> >> + uint64_t msr_ia32_feature_control;
> >>
> >> /* exception/interrupt handling */
> >> int error_code;
> >> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> >> index 39f4fbb..3cb2161 100644
> >> --- a/target-i386/kvm.c
> >> +++ b/target-i386/kvm.c
> >> @@ -1122,6 +1122,7 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> >> if (hyperv_vapic_recommended()) {
> >> kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
> >> }
> >> + kvm_msr_entry_set(&msrs[n++], MSR_IA32_FEATURE_CONTROL, env->msr_ia32_feature_control);
> >> }
> >> if (env->mcg_cap) {
> >> int i;
> >> @@ -1346,6 +1347,7 @@ static int kvm_get_msrs(X86CPU *cpu)
> >> if (has_msr_misc_enable) {
> >> msrs[n++].index = MSR_IA32_MISC_ENABLE;
> >> }
> >> + msrs[n++].index = MSR_IA32_FEATURE_CONTROL;
> >>
> >> if (!env->tsc_valid) {
> >> msrs[n++].index = MSR_IA32_TSC;
> >> @@ -1444,6 +1446,8 @@ static int kvm_get_msrs(X86CPU *cpu)
> >> case MSR_IA32_MISC_ENABLE:
> >> env->msr_ia32_misc_enable = msrs[i].data;
> >> break;
> >> + case MSR_IA32_FEATURE_CONTROL:
> >> + env->msr_ia32_feature_control = msrs[i].data;
> >> default:
> >> if (msrs[i].index >= MSR_MC0_CTL &&
> >> msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
> >> diff --git a/target-i386/machine.c b/target-i386/machine.c
> >> index 3659db9..94ca914 100644
> >> --- a/target-i386/machine.c
> >> +++ b/target-i386/machine.c
> >> @@ -399,6 +399,14 @@ static bool misc_enable_needed(void *opaque)
> >> return env->msr_ia32_misc_enable != MSR_IA32_MISC_ENABLE_DEFAULT;
> >> }
> >>
> >> +static bool feature_control_needed(void *opaque)
> >> +{
> >> + X86CPU *cpu = opaque;
> >> + CPUX86State *env = &cpu->env;
> >> +
> >> + return env->msr_ia32_feature_control != 0;
> >> +}
> >> +
> >> static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> >> .name = "cpu/msr_ia32_misc_enable",
> >> .version_id = 1,
> >> @@ -410,6 +418,17 @@ static const VMStateDescription vmstate_msr_ia32_misc_enable = {
> >> }
> >> };
> >>
> >> +static const VMStateDescription vmstate_msr_ia32_feature_control = {
> >> + .name = "cpu/msr_ia32_feature_control",
> >> + .version_id = 1,
> >> + .minimum_version_id = 1,
> >> + .minimum_version_id_old = 1,
> >> + .fields = (VMStateField []) {
> >> + VMSTATE_UINT64(env.msr_ia32_feature_control, X86CPU),
> >> + VMSTATE_END_OF_LIST()
> >> + }
> >> +};
> >> +
> >> const VMStateDescription vmstate_x86_cpu = {
> >> .name = "cpu",
> >> .version_id = 12,
> >> @@ -535,6 +554,9 @@ const VMStateDescription vmstate_x86_cpu = {
> >> }, {
> >> .vmsd = &vmstate_msr_ia32_misc_enable,
> >> .needed = misc_enable_needed,
> >> + }, {
> >> + .vmsd = &vmstate_msr_ia32_feature_control,
> >> + .needed = feature_control_needed,
> >> } , {
> >> /* empty */
> >> }
> >> --
> >> 1.7.9.5
> >
> > --
> > Gleb.
>
>
>
> --
> Arthur Chunqi Li
> Department of Computer Science
> School of EECS
> Peking University
> Beijing, China
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
