From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eduardo Habkost Subject: Re: [PATCH] KVM: SVM: fix trashing of MSR_TSC_AUX Date: Thu, 7 Jul 2016 13:27:55 -0300 Message-ID: <20160707162755.GD4131@thinpad.lan.raisama.net> References: <1467812596-18903-1-git-send-email-pbonzini@redhat.com> <20160706141847.GF7300@pd.tnic> <361b792f-87ce-863d-5098-9a18aadd6379@redhat.com> <20160707104128.GC13648@pd.tnic> <128bb0be-e597-32c0-f113-9fe02c257790@redhat.com> <20160707114717.GD13648@pd.tnic> <8c66c2b5-58fe-7384-31a3-f6f784461ca6@redhat.com> <20160707124712.GE13648@pd.tnic> <21b7a239-5fac-b6bf-a48f-71fa456a9c97@redhat.com> <20160707160146.GH13648@pd.tnic> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Paolo Bonzini , linux-kernel@vger.kernel.org, kvm@vger.kernel.org, stable@vger.kernel.org, Yazen Ghannam , Brijesh Singh To: Borislav Petkov Return-path: Content-Disposition: inline In-Reply-To: <20160707160146.GH13648@pd.tnic> Sender: stable-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On Thu, Jul 07, 2016 at 06:01:46PM +0200, Borislav Petkov wrote: > On Thu, Jul 07, 2016 at 03:16:21PM +0200, Paolo Bonzini wrote: > > Eduardo is the one to answer, but usually we add features to QEMU > > before the processors are released (typically as soon as KVM supports > > them). So with a new enough QEMU this in theory should not be > > necessary. > > > > Adding a new feature that's not in a CPU model and that's not > > associated to new state is really trivial: > > Cool. > > Btw, how about something like this? > > Specifically, I'd like to test RAS features on the new upcoming AMD > Zen CPU and I've defined one from the stuff we know so far from kernel > patches. You mean KVM kernel patches? I assume the features require additional KVM code to support them in guests. In that case, why wouldn't the kernel return them in GET_SUPPORTED_CPUID? Then you won't need filter=off. > > The "filter=off" thing I've added in case I want to disable > x86_cpu_filter_features() but it works just fine without it when I boot > with -cpu Zen. So I can remove it too. > > Would something like that be acceptable? About filter=off: not sure. Do we really have valid use cases to enable a feature even if the kernel reports it as unsupported in GET_SUPPORTED_CPUID? Specifically about the feature names, I have some question below: > > We can continue improving on this as features become known and even > implement some functionality in qemu/kvm as time allows. > > --- > From: Borislav Petkov > Date: Tue, 5 Jul 2016 16:12:18 +0200 > Subject: [PATCH] Zen emu: first working version > > Boot with "-c Zen,filter=off" to disable CPUID bits filtering. > > Signed-off-by: Borislav Petkov > --- > target-i386/cpu.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- > target-i386/cpu.h | 7 +++++++ > 2 files changed, 66 insertions(+), 1 deletion(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 3bd3cfc3ad16..cc9c97457387 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -307,6 +307,17 @@ static const char *cpuid_6_feature_name[] = { > NULL, NULL, NULL, NULL, > }; > > +static const char *smca_feature_name[] = { > + "overflow_recov", "succor", NULL, "smca", Do those features introduce additional state that need migration support? If they do, you need to add them to feature_word_info[FEAT_8000_0007_EBX].unmigratable_flags until migration support is implemented. > + NULL, NULL, NULL, NULL, > + NULL, NULL, NULL, NULL, > + NULL, NULL, NULL, NULL, > + NULL, NULL, NULL, NULL, > + NULL, NULL, NULL, NULL, > + NULL, NULL, NULL, NULL, > + NULL, NULL, NULL, NULL, > +}; > + > #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE) > #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \ > CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC) > @@ -449,6 +460,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { > .cpuid_eax = 6, .cpuid_reg = R_EAX, > .tcg_features = TCG_6_EAX_FEATURES, > }, > + [FEAT_8000_0007_EBX] = { > + .feat_names = smca_feature_name, > + .cpuid_eax = 0x80000007, > + .cpuid_reg = R_EBX, > + }, > }; > > typedef struct X86RegisterInfo32 { [...] -- Eduardo