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

[Haozhong Zhang <haozhong.zhang@intel.com>]

On 06/17/16 13:20, Eduardo Habkost wrote:
> 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.
>

Let's stop only for LMCE in this patch series. Other bits may be
changed in future after the kernel support is clarified.

Thanks,
Haozhong

> I need to re-read the thread were kvm_get_mce_cap_supported() was
> discussed, to refresh my memory.
>
> -- 
> Eduardo
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html