From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:32973)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <ehabkost@redhat.com>) id 1bDwV9-0006fk-3m
	for qemu-devel@nongnu.org; Fri, 17 Jun 2016 12:20:15 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <ehabkost@redhat.com>) id 1bDwV5-00085o-0L
	for qemu-devel@nongnu.org; Fri, 17 Jun 2016 12:20:11 -0400
Received: from mx1.redhat.com ([209.132.183.28]:48415)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <ehabkost@redhat.com>) id 1bDwV4-00085D-RY
	for qemu-devel@nongnu.org; Fri, 17 Jun 2016 12:20:06 -0400
Date: Fri, 17 Jun 2016 13:20:03 -0300
From: Eduardo Habkost <ehabkost@redhat.com>
Message-ID: <20160617162003.GI18662@thinpad.lan.raisama.net>
References: <20160616060621.30422-1-haozhong.zhang@intel.com>
	<20160616060621.30422-2-haozhong.zhang@intel.com>
	<20160616193725.GT18662@thinpad.lan.raisama.net>
	<20160617012657.36fcgurduuhxnwsj@hz-desktop>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160617012657.36fcgurduuhxnwsj@hz-desktop>
Subject: Re: [Qemu-devel] [PATCH v4 1/3] target-i386: KVM: add basic Intel
 LMCE support
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: qemu-devel@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <rth@twiddle.net>, "Michael S . Tsirkin" <mst@redhat.com>, Marcelo Tosatti <mtosatti@redhat.com>, kvm@vger.kernel.org, Boris Petkov <bp@suse.de>, Tony Luck <tony.luck@intel.com>, Andi Kleen <andi.kleen@intel.com>, rkrcmar@redhat.com, Ashok Raj <ashok.raj@intel.com>

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