From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58378) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bADWE-0001yP-SP for qemu-devel@nongnu.org; Tue, 07 Jun 2016 05:41:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bADW9-0006tL-Sq for qemu-devel@nongnu.org; Tue, 07 Jun 2016 05:41:53 -0400 Received: from mga01.intel.com ([192.55.52.88]:31294) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bADW9-0006t5-HO for qemu-devel@nongnu.org; Tue, 07 Jun 2016 05:41:49 -0400 Date: Tue, 7 Jun 2016 17:41:18 +0800 From: Haozhong Zhang Message-ID: <20160607094118.2t4muhk72dzza3dg@hz-desktop> References: <20160603060944.17373-1-haozhong.zhang@intel.com> <20160603060944.17373-2-haozhong.zhang@intel.com> <20160604210317.GA17952@thinpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160604210317.GA17952@thinpad.lan.raisama.net> Subject: Re: [Qemu-devel] [PATCH v3 1/2] target-i386: KVM: add basic Intel LMCE support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Boris Petkov , qemu-devel@nongnu.org, Paolo Bonzini , Richard Henderson , Marcelo Tosatti , "Michael S . Tsirkin" , kvm@vger.kernel.org, Tony Luck , Andi Kleen , Ashok Raj On 06/04/16 18:03, Eduardo Habkost wrote: > On Sat, Jun 04, 2016 at 12:34:39PM +0200, Boris Petkov wrote: > > Haozhong Zhang wrote: > > > > >This patch adds the support to inject SRAR and SRAO as LMCE, i.e. they > > >will be 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. > > > > > >Signed-off-by: Ashok Raj > > >Signed-off-by: Haozhong Zhang > > >--- > > >Cc: Paolo Bonzini > > >Cc: Richard Henderson > > >Cc: Eduardo Habkost > > >Cc: Marcelo Tosatti > > >Cc: Boris Petkov > > >Cc: kvm@vger.kernel.org > > >Cc: Tony Luck > > >Cc: Andi Kleen > > >--- > > > target-i386/cpu.c | 26 ++++++++++++++++++++++++++ > > > target-i386/cpu.h | 13 ++++++++++++- > > > target-i386/kvm.c | 35 +++++++++++++++++++++++++++++++---- > > > 3 files changed, 69 insertions(+), 5 deletions(-) > > > > ... > > > > >@@ -1173,6 +1182,8 @@ struct X86CPU { > > > */ > > > bool enable_pmu; > > > > > >+ bool enable_lmce; > > > > That struct would go fat pretty fast if it grows a bool per CPU > > feature. Perhaps a more clever, a-bit-per-featurebit scheme > > would be in order. > > We already have X86CPU.features, but it's specific for feature > flags appearing on CPUID. We can eventually extend > FeatureWord/FeatureWordInfo/x86_cpu_get_supported_feature_word() > to represent features that don't appear directly on CPUID. > You mean CPUX86State.features? enable_lmce is also used in patch 2 to avoid migrating from lmce-enabled qemu to lmce-disabled qemu, so I don't think CPUX86State.features is the correct place for enable_lmce. Or, we may append one or more bit words to X86CPU.filtered_features for enable_pmu, enable_lmce and future features, e.g. modified target-i386/cpu.h @@ -455,6 +455,14 @@ typedef enum FeatureWord { typedef uint32_t FeatureWordArray[FEATURE_WORDS]; +typedef enum ExtFeatureBit { + EXT_FEAT_PMU, + EXT_FEAT_LMCE, + EXT_FEATURE_BITS, +} ExtFeatureBit; + +#define EXT_FEATURE_WORDS ((EXT_FEATURE_BITS + 31) / 32) + +#define EXT_FEATURE_FILTER_ADD(words, feat) \ + do { \ + uint32_t *__words = (words); \ + int __idx = (feat) / 32; \ + int __oft = (feat) % 32; \ + __words[__idx + FEATURE_WORDS] |= (1 << __oft); \ + } while (0) + +#define EXT_FEATURE_FILTER_REMOVE(words, feat) \ + do { \ + uint32_t *__words = (words); \ + int __idx = (feat) / 32; \ + int __oft = (feat) % 32; \ + __words[__idx + FEATURE_WORDS] &= ~(1 << __oft); \ + } while (0) + +#define EXT_FEATURE_FILTERED(words, feat) \ + ({ \ + uint32_t *__words = (words); \ + int __idx = (feat) / 32; \ + int __oft = (feat) % 32; \ + __words[__idx + FEATURE_WORDS] & (1 << oft) \ + }) + /* cpuid_features bits */ #define CPUID_FP87 (1U << 0) #define CPUID_VME (1U << 1) @@ -1173,21 +1181,7 @@ struct X86CPU { /* Features that were filtered out because of missing host capabilities */ - uint32_t filtered_features[FEATURE_WORDS]; + /* Features that were filtered out because of missing host capabilities or + being disabled by default */ + uint32_t filtered_features[FEATURE_WORDS + EXT_FEATURE_WORDS]; - - /* Enable PMU CPUID bits. This can't be enabled by default yet because - * it doesn't have ABI stability guarantees, as it passes all PMU CPUID - * bits returned by GET_SUPPORTED_CPUID (that depend on host CPU and kernel - * capabilities) directly to the guest. - */ - bool enable_pmu; - - /* Enable LMCE support which is set via cpu option 'lmce=on/off'. LMCE is - * disabled by default to avoid breaking the migration between QEMU with - * different LMCE support. Only migrating between QEMU with the same LMCE - * support is allowed. - */ - bool enable_lmce; Every place using X86CPU.enable_pmu and .enable_lmce is then replaced by above macros EXT_FEATURE_FILTER_ADD, EXT_FEATURE_FILTER_REMOVE and EXT_FEATURE_FILTERED. Of course, those bits in X86CPU.filtered_features have the opposite meaning to original enable_lmce and enable_pmu. Haozhong