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@xxxxxxxxx>
> > ---
> > Cc: "Michael S. Tsirkin" <mst@xxxxxxxxxx>
> > Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> > Cc: Richard Henderson <rth@xxxxxxxxxxx>
> > Cc: Eduardo Habkost <ehabkost@xxxxxxxxxx>
> > Cc: Boris Petkov <bp@xxxxxxx>
> > Cc: Tony Luck <tony.luck@xxxxxxxxx>
> > Cc: Andi Kleen <andi.kleen@xxxxxxxxx>
> > Cc: Ashok Raj <ashok.raj@xxxxxxxxx>
> > ---
> > 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
--
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
