Re: [Qemu-devel] [PATCH v4 1/3] target-i386: KVM: add basic Intel LMCE support

[Eduardo Habkost]

On Fri, Jun 17, 2016 at 09:26:57AM +0800, Haozhong Zhang wrote:
[...]
> > >  static void mce_init(X86CPU *cpu)
> > >  {
> > >      CPUX86State *cenv = &cpu->env;
> > >      unsigned int bank;
> > > +    Error *local_err = NULL;
> > >  
> > >      if (((cenv->cpuid_version >> 8) & 0xf) >= 6
> > >          && (cenv->features[FEAT_1_EDX] & (CPUID_MCE | CPUID_MCA)) ==
> > >              (CPUID_MCE | CPUID_MCA)) {
> > >          cenv->mcg_cap = MCE_CAP_DEF | MCE_BANKS_DEF;
> > > +
> > > +        if (cpu->enable_lmce) {
> > > +            if (!lmce_supported()) {
> > > +                error_setg(&local_err, "KVM unavailable or LMCE not 
> > > supported");
> > > +                error_propagate(&error_abort, local_err);
> > > +            }
> > > +            cenv->mcg_cap |= MCG_LMCE_P;
> > > +        }
> > > +
> > 
> > This duplicates the existing check in kvm_arch_init_vcpu(). The
> > difference is that the existing code is KVM-specific and doesn't
> > stop initialization when capabilities are missing. We can unify
> > them into a single mcg_cap-checking function as a follow-up.
> >
> 
> If I reuse the existing MCE capability check in kvm_arch_init_vcpu(),
> is it reasonable to make change to stop initialization if missing
> capabilities? Or should we stop only for missing newly added capabilities
> (e.g. LMCE) in order to keep backwards compatibility?

Ideally, yes. But in practice we need to check if we won't break
existing setups that were working. If all kernel versions we care
about always MCG_CTL_P|MCG_SER_P + 10 banks as supported, we can
make all bits mandatory.

I need to re-read the thread were kvm_get_mce_cap_supported() was
discussed, to refresh my memory.

-- 
Eduardo