From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56330) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDiZ3-00006u-VP for qemu-devel@nongnu.org; Thu, 16 Jun 2016 21:27:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bDiYx-0001jR-R9 for qemu-devel@nongnu.org; Thu, 16 Jun 2016 21:27:16 -0400 Received: from mga04.intel.com ([192.55.52.120]:44208) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDiYx-0001j9-Hy for qemu-devel@nongnu.org; Thu, 16 Jun 2016 21:27:11 -0400 Date: Fri, 17 Jun 2016 09:26:57 +0800 From: Haozhong Zhang Message-ID: <20160617012657.36fcgurduuhxnwsj@hz-desktop> References: <20160616060621.30422-1-haozhong.zhang@intel.com> <20160616060621.30422-2-haozhong.zhang@intel.com> <20160616193725.GT18662@thinpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160616193725.GT18662@thinpad.lan.raisama.net> Subject: Re: [Qemu-devel] [PATCH v4 1/3] target-i386: KVM: add basic Intel LMCE support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: qemu-devel@nongnu.org, Paolo Bonzini , Richard Henderson , "Michael S . Tsirkin" , Marcelo Tosatti , kvm@vger.kernel.org, Boris Petkov , Tony Luck , Andi Kleen , rkrcmar@redhat.com, Ashok Raj On 06/16/16 16:37, Eduardo Habkost wrote: > On Thu, Jun 16, 2016 at 02:06:19PM +0800, Haozhong Zhang wrote: > > From: Ashok Raj > > > > This patch adds the support to inject SRAR and SRAO as LMCE, i.e. they > > are injected to only one VCPU rather than broadcast to all VCPUs. As KVM > > reports LMCE support on Intel platforms, this features is only available > > on Intel platforms. > > > > LMCE is disabled by default and can be enabled/disabled by cpu option > > 'lmce=on/off'. > > > > Signed-off-by: Ashok Raj > > [Haozhong: Enable LMCE only on Intel platforms > > Disable LMCE by default and add a cpu option 'lmce' > > Disable LMCE if missing KVM support > > Remove MCG_LMCE_P from MCE_CAP_DEF > > Minor code style changes] > > You are mixing tabs and spaces above. > Oops, I missed to take care tabs in the commit message. will fix > > Signed-off-by: Haozhong Zhang > > --- > > target-i386/cpu.c | 23 +++++++++++++++++++++++ > > target-i386/cpu.h | 12 ++++++++++++ > > target-i386/kvm.c | 35 +++++++++++++++++++++++++++++++---- > > 3 files changed, 66 insertions(+), 4 deletions(-) > > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index 895a386..bd35db2 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -2777,15 +2777,37 @@ static void x86_cpu_machine_reset_cb(void *opaque) > > } > > #endif > > > > +static bool lmce_supported(void) > > +{ > > + uint64_t mce_cap; > > + > > + if (!kvm_enabled() || > > + kvm_ioctl(kvm_state, KVM_X86_GET_MCE_CAP_SUPPORTED, &mce_cap) < 0) { > > + return false; > > + } > > + > > + return !!(mce_cap & MCG_LMCE_P); > > +} > > + > > 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? Thanks, Haozhong