| Message ID | 20160603060944.17373-3-haozhong.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 03, 2016 at 02:09:44PM +0800, Haozhong Zhang wrote: > LMCE is disabled by default, but a cpu option 'lmce=on/off' is provided > to enable/disable it. Migration is only allowed between VCPUs with the > same lmce option. > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > --- > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Richard Henderson <rth@twiddle.net> > Cc: Eduardo Habkost <ehabkost@redhat.com> > Cc: Boris Petkov <bp@suse.de> > Cc: Tony Luck <tony.luck@intel.com> > Cc: Andi Kleen <andi.kleen@intel.com> > Cc: Ashok Raj <ashok.raj@intel.com> > --- > include/hw/i386/pc.h | 7 ++++++- > target-i386/cpu.c | 1 + > target-i386/cpu.h | 5 +++++ > target-i386/machine.c | 24 ++++++++++++++++++++++++ > 4 files changed, 36 insertions(+), 1 deletion(-) > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index ca23609..058eef9 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -357,7 +357,12 @@ int e820_get_num_entries(void); > bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); > > #define PC_COMPAT_2_6 \ > - HW_COMPAT_2_6 > + HW_COMPAT_2_6 \ > + {\ > + .driver = TYPE_X86_CPU,\ > + .property = "lmce",\ > + .value = "off",\ > + }, You don't need this if lmce is disabled by default. > > #define PC_COMPAT_2_5 \ > PC_COMPAT_2_6 \ > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 9b4dbab..c69cc17 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -3232,6 +3232,7 @@ static Property x86_cpu_properties[] = { > DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0), > DEFINE_PROP_UINT32("xlevel2", X86CPU, env.cpuid_xlevel2, 0), > DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor_id), > + DEFINE_PROP_BOOL("lmce", X86CPU, enable_lmce, false), Maybe this belong to patch 1/2? > DEFINE_PROP_END_OF_LIST() > }; > > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index 2d411ba..b512fd6 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -1182,6 +1182,11 @@ struct X86CPU { > */ > bool enable_pmu; > > + /* Enable LMCE support which is set via cpu option 'lmce=on/off'. LMCE is > + * disabled by default to avoid breaking the migration between QEMU with > + * different LMCE support. Only migrating between QEMU with the same LMCE > + * support is allowed. > + */ > bool enable_lmce; > > /* in order to simplify APIC support, we leave this pointer to the > diff --git a/target-i386/machine.c b/target-i386/machine.c > index cb9adf2..b55d376 100644 > --- a/target-i386/machine.c > +++ b/target-i386/machine.c > @@ -347,6 +347,11 @@ static int cpu_post_load(void *opaque, int version_id) > return -EINVAL; > } > > + if (!cpu->enable_lmce && (env->mcg_cap & MCG_LMCE_P)) { > + error_report("LMCE not enabled"); > + return -EINVAL; > + } Nice. But the error message could be clearer, to indicate that it is about command-line configuration not being the same on both sides. What about something like: config mismatch: VCPU has LMCE is enabled, but "lmce" option is disabled > + > /* > * Real mode guest segments register DPL should be zero. > * Older KVM version were setting it wrongly. > @@ -896,6 +901,24 @@ static const VMStateDescription vmstate_tsc_khz = { > } > }; > > +static bool mcg_ext_ctl_needed(void *opaque) > +{ > + X86CPU *cpu = opaque; > + CPUX86State *env = &cpu->env; > + return cpu->enable_lmce && env->mcg_ext_ctl; > +} > + > +static const VMStateDescription vmstate_mcg_ext_ctl = { > + .name = "cpu/mcg_ext_ctl", > + .version_id = 1, > + .minimum_version_id = 1, > + .needed = mcg_ext_ctl_needed, > + .fields = (VMStateField[]) { > + VMSTATE_UINT64(env.mcg_ext_ctl, X86CPU), > + VMSTATE_END_OF_LIST() > + } > +}; > + > VMStateDescription vmstate_x86_cpu = { > .name = "cpu", > .version_id = 12, > @@ -1022,6 +1045,7 @@ VMStateDescription vmstate_x86_cpu = { > #ifdef TARGET_X86_64 > &vmstate_pkru, > #endif > + &vmstate_mcg_ext_ctl, > NULL > } > }; > -- > 2.8.3 >
On 06/07/16 17:18, Eduardo Habkost wrote: > On Fri, Jun 03, 2016 at 02:09:44PM +0800, Haozhong Zhang wrote: > > LMCE is disabled by default, but a cpu option 'lmce=on/off' is provided > > to enable/disable it. Migration is only allowed between VCPUs with the > > same lmce option. > > > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > > --- > > Cc: "Michael S. Tsirkin" <mst@redhat.com> > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > Cc: Richard Henderson <rth@twiddle.net> > > Cc: Eduardo Habkost <ehabkost@redhat.com> > > Cc: Boris Petkov <bp@suse.de> > > Cc: Tony Luck <tony.luck@intel.com> > > Cc: Andi Kleen <andi.kleen@intel.com> > > Cc: Ashok Raj <ashok.raj@intel.com> > > --- > > include/hw/i386/pc.h | 7 ++++++- > > target-i386/cpu.c | 1 + > > target-i386/cpu.h | 5 +++++ > > target-i386/machine.c | 24 ++++++++++++++++++++++++ > > 4 files changed, 36 insertions(+), 1 deletion(-) > > > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > > index ca23609..058eef9 100644 > > --- a/include/hw/i386/pc.h > > +++ b/include/hw/i386/pc.h > > @@ -357,7 +357,12 @@ int e820_get_num_entries(void); > > bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); > > > > #define PC_COMPAT_2_6 \ > > - HW_COMPAT_2_6 > > + HW_COMPAT_2_6 \ > > + {\ > > + .driver = TYPE_X86_CPU,\ > > + .property = "lmce",\ > > + .value = "off",\ > > + }, > > You don't need this if lmce is disabled by default. > Oh yes, I'll remove in the next version. > > > > #define PC_COMPAT_2_5 \ > > PC_COMPAT_2_6 \ > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index 9b4dbab..c69cc17 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -3232,6 +3232,7 @@ static Property x86_cpu_properties[] = { > > DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0), > > DEFINE_PROP_UINT32("xlevel2", X86CPU, env.cpuid_xlevel2, 0), > > DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor_id), > > + DEFINE_PROP_BOOL("lmce", X86CPU, enable_lmce, false), > > Maybe this belong to patch 1/2? > I think it's better to not allow users to enable LMCE until we fix the migration in patch 2, so I didn't put it in patch 1. > > DEFINE_PROP_END_OF_LIST() > > }; > > > > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > > index 2d411ba..b512fd6 100644 > > --- a/target-i386/cpu.h > > +++ b/target-i386/cpu.h > > @@ -1182,6 +1182,11 @@ struct X86CPU { > > */ > > bool enable_pmu; > > > > + /* Enable LMCE support which is set via cpu option 'lmce=on/off'. LMCE is > > + * disabled by default to avoid breaking the migration between QEMU with > > + * different LMCE support. Only migrating between QEMU with the same LMCE > > + * support is allowed. > > + */ > > bool enable_lmce; > > > > /* in order to simplify APIC support, we leave this pointer to the > > diff --git a/target-i386/machine.c b/target-i386/machine.c > > index cb9adf2..b55d376 100644 > > --- a/target-i386/machine.c > > +++ b/target-i386/machine.c > > @@ -347,6 +347,11 @@ static int cpu_post_load(void *opaque, int version_id) > > return -EINVAL; > > } > > > > + if (!cpu->enable_lmce && (env->mcg_cap & MCG_LMCE_P)) { > > + error_report("LMCE not enabled"); > > + return -EINVAL; > > + } > > Nice. But the error message could be clearer, to indicate that it > is about command-line configuration not being the same on both > sides. What about something like: > config mismatch: VCPU has LMCE is enabled, but "lmce" option is disabled > Yes, yours is much clearer. I'll change in the next version. Thanks, Haozhong > > + > > /* > > * Real mode guest segments register DPL should be zero. > > * Older KVM version were setting it wrongly. > > @@ -896,6 +901,24 @@ static const VMStateDescription vmstate_tsc_khz = { > > } > > }; > > > > +static bool mcg_ext_ctl_needed(void *opaque) > > +{ > > + X86CPU *cpu = opaque; > > + CPUX86State *env = &cpu->env; > > + return cpu->enable_lmce && env->mcg_ext_ctl; > > +} > > + > > +static const VMStateDescription vmstate_mcg_ext_ctl = { > > + .name = "cpu/mcg_ext_ctl", > > + .version_id = 1, > > + .minimum_version_id = 1, > > + .needed = mcg_ext_ctl_needed, > > + .fields = (VMStateField[]) { > > + VMSTATE_UINT64(env.mcg_ext_ctl, X86CPU), > > + VMSTATE_END_OF_LIST() > > + } > > +}; > > + > > VMStateDescription vmstate_x86_cpu = { > > .name = "cpu", > > .version_id = 12, > > @@ -1022,6 +1045,7 @@ VMStateDescription vmstate_x86_cpu = { > > #ifdef TARGET_X86_64 > > &vmstate_pkru, > > #endif > > + &vmstate_mcg_ext_ctl, > > NULL > > } > > }; > > -- > > 2.8.3 > > > > -- > Eduardo
On 03/06/2016 08:09, Haozhong Zhang wrote: > LMCE is disabled by default, but a cpu option 'lmce=on/off' is provided > to enable/disable it. Migration is only allowed between VCPUs with the > same lmce option. This is not needed at all if you do the change in patch 1 that Eduardo requested (refuse to start if the host doesn't have the required capabilities). So if you do that you can just move the lmce property to patch 1. Thanks, Paolo
On 06/08/16 13:36, Paolo Bonzini wrote: > > > On 03/06/2016 08:09, Haozhong Zhang wrote: > > LMCE is disabled by default, but a cpu option 'lmce=on/off' is provided > > to enable/disable it. Migration is only allowed between VCPUs with the > > same lmce option. > > This is not needed at all if you do the change in patch 1 that Eduardo > requested (refuse to start if the host doesn't have the required > capabilities). > > So if you do that you can just move the lmce property to patch 1. > But it doesn't cover the migration from lmce-enabled qemu to lmce-disabled qemu where KVM on both hosts support LMCE. In that case, both qemu can start without failure, but the guest OS will run in a VM with different configurations after migration. To avoid this, I didn't leave the lmce property in patch 1, so that users have no way to enable LMCE without this patch 2. Thanks, Haozhong
On 09/06/2016 09:16, Haozhong Zhang wrote: > On 06/08/16 13:36, Paolo Bonzini wrote: >> >> >> On 03/06/2016 08:09, Haozhong Zhang wrote: >>> LMCE is disabled by default, but a cpu option 'lmce=on/off' is provided >>> to enable/disable it. Migration is only allowed between VCPUs with the >>> same lmce option. >> >> This is not needed at all if you do the change in patch 1 that Eduardo >> requested (refuse to start if the host doesn't have the required >> capabilities). >> >> So if you do that you can just move the lmce property to patch 1. >> > > But it doesn't cover the migration from lmce-enabled qemu to > lmce-disabled qemu where KVM on both hosts support LMCE. That's a configuration problem; configuration is not migrated and is assumed to be the same on the source and the destination. You don't need to test for this scenario. Paolo In that case, > both qemu can start without failure, but the guest OS will run in a VM > with different configurations after migration. To avoid this, I didn't > leave the lmce property in patch 1, so that users have no way to > enable LMCE without this patch 2.
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index ca23609..058eef9 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -357,7 +357,12 @@ int e820_get_num_entries(void); bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); #define PC_COMPAT_2_6 \ - HW_COMPAT_2_6 + HW_COMPAT_2_6 \ + {\ + .driver = TYPE_X86_CPU,\ + .property = "lmce",\ + .value = "off",\ + }, #define PC_COMPAT_2_5 \ PC_COMPAT_2_6 \ diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 9b4dbab..c69cc17 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -3232,6 +3232,7 @@ static Property x86_cpu_properties[] = { DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0), DEFINE_PROP_UINT32("xlevel2", X86CPU, env.cpuid_xlevel2, 0), DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor_id), + DEFINE_PROP_BOOL("lmce", X86CPU, enable_lmce, false), DEFINE_PROP_END_OF_LIST() }; diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 2d411ba..b512fd6 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -1182,6 +1182,11 @@ struct X86CPU { */ bool enable_pmu; + /* Enable LMCE support which is set via cpu option 'lmce=on/off'. LMCE is + * disabled by default to avoid breaking the migration between QEMU with + * different LMCE support. Only migrating between QEMU with the same LMCE + * support is allowed. + */ bool enable_lmce; /* in order to simplify APIC support, we leave this pointer to the diff --git a/target-i386/machine.c b/target-i386/machine.c index cb9adf2..b55d376 100644 --- a/target-i386/machine.c +++ b/target-i386/machine.c @@ -347,6 +347,11 @@ static int cpu_post_load(void *opaque, int version_id) return -EINVAL; } + if (!cpu->enable_lmce && (env->mcg_cap & MCG_LMCE_P)) { + error_report("LMCE not enabled"); + return -EINVAL; + } + /* * Real mode guest segments register DPL should be zero. * Older KVM version were setting it wrongly. @@ -896,6 +901,24 @@ static const VMStateDescription vmstate_tsc_khz = { } }; +static bool mcg_ext_ctl_needed(void *opaque) +{ + X86CPU *cpu = opaque; + CPUX86State *env = &cpu->env; + return cpu->enable_lmce && env->mcg_ext_ctl; +} + +static const VMStateDescription vmstate_mcg_ext_ctl = { + .name = "cpu/mcg_ext_ctl", + .version_id = 1, + .minimum_version_id = 1, + .needed = mcg_ext_ctl_needed, + .fields = (VMStateField[]) { + VMSTATE_UINT64(env.mcg_ext_ctl, X86CPU), + VMSTATE_END_OF_LIST() + } +}; + VMStateDescription vmstate_x86_cpu = { .name = "cpu", .version_id = 12, @@ -1022,6 +1045,7 @@ VMStateDescription vmstate_x86_cpu = { #ifdef TARGET_X86_64 &vmstate_pkru, #endif + &vmstate_mcg_ext_ctl, NULL } };
LMCE is disabled by default, but a cpu option 'lmce=on/off' is provided to enable/disable it. Migration is only allowed between VCPUs with the same lmce option. Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> --- Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Richard Henderson <rth@twiddle.net> Cc: Eduardo Habkost <ehabkost@redhat.com> Cc: Boris Petkov <bp@suse.de> Cc: Tony Luck <tony.luck@intel.com> Cc: Andi Kleen <andi.kleen@intel.com> Cc: Ashok Raj <ashok.raj@intel.com> --- include/hw/i386/pc.h | 7 ++++++- target-i386/cpu.c | 1 + target-i386/cpu.h | 5 +++++ target-i386/machine.c | 24 ++++++++++++++++++++++++ 4 files changed, 36 insertions(+), 1 deletion(-)