From mboxrd@z Thu Jan 1 00:00:00 1970 From: Haozhong Zhang Subject: Re: [PATCH v4 1/3] target-i386: KVM: add basic Intel LMCE support Date: Fri, 17 Jun 2016 09:26:57 +0800 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 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 To: Eduardo Habkost Return-path: Received: from mga03.intel.com ([134.134.136.65]:65193 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754991AbcFQB1B (ORCPT ); Thu, 16 Jun 2016 21:27:01 -0400 Content-Disposition: inline In-Reply-To: <20160616193725.GT18662@thinpad.lan.raisama.net> Sender: kvm-owner@vger.kernel.org List-ID: 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 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