From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52335) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tw9wf-0008Dq-QK for qemu-devel@nongnu.org; Fri, 18 Jan 2013 06:17:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tw9we-0002qm-E7 for qemu-devel@nongnu.org; Fri, 18 Jan 2013 06:17:13 -0500 Received: from mx1.redhat.com ([209.132.183.28]:31234) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tw9we-0002qe-6c for qemu-devel@nongnu.org; Fri, 18 Jan 2013 06:17:12 -0500 Date: Fri, 18 Jan 2013 13:00:56 +0200 From: Gleb Natapov Message-ID: <20130118110056.GF10107@redhat.com> References: <1358456378-29248-1-git-send-email-ehabkost@redhat.com> <1358456378-29248-3-git-send-email-ehabkost@redhat.com> <50F92ADA.9080203@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <50F92ADA.9080203@suse.de> Subject: Re: [Qemu-devel] [PATCH for-1.4 02/12] target-i386: Don't set any KVM flag by default if KVM is disabled List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?utf-8?Q?F=C3=A4rber?= Cc: Igor Mammedov , "kvm@vger.kernel.org list" , Eduardo Habkost , Anthony Liguori , qemu-devel@nongnu.org On Fri, Jan 18, 2013 at 11:58:34AM +0100, Andreas F=C3=A4rber wrote: > Am 17.01.2013 21:59, schrieb Eduardo Habkost: > > This is a cleanup that tries to solve two small issues: > >=20 > > - We don't need a separate kvm_pv_eoi_features variable just to keep a > > constant calculated at compile-time, and this style would require > > adding a separate variable (that's declared twice because of the > > CONFIG_KVM ifdef) for each feature that's going to be enabled/disable > > by machine-type compat code. > > - The pc-1.3 code is setting the kvm_pv_eoi flag on cpuid_kvm_features > > even when KVM is disabled at runtime. This small incosistency in > > the cpuid_kvm_features field isn't a problem today because > > cpuid_kvm_features is ignored by the TCG code, but it may cause > > unexpected problems later when refactoring the CPUID handling code. > >=20 > > This patch eliminates the kvm_pv_eoi_features variable and simply uses > > kvm_enabled() inside the enable_kvm_pv_eoi() compat function, so it > > enables kvm_pv_eoi only if KVM is enabled. I believe this makes the > > behavior of enable_kvm_pv_eoi() clearer and easier to understand. > >=20 > > Signed-off-by: Eduardo Habkost >=20 > Fine with me if KVM folks agree. >=20 Acked-by: Gleb Natapov > Andreas >=20 > > --- > > Cc: kvm@vger.kernel.org > > Cc: Michael S. Tsirkin > > Cc: Gleb Natapov > > Cc: Marcelo Tosatti > >=20 > > Changes v2: > > - Coding style fix > >=20 > > Changes v3: > > - Eliminate #ifdef by using the fake KVM_FEATURE_PV_EOI #define > >=20 > > Changes v4: > > - Check kvm_enabled() when actually using kvm_default_features > > - Eliminate Yet Another #ifdef by using the fake KVM_FEATURE_* > > #defines on kvm_default_features initialization > >=20 > > Changes v5: > > - Rebase and move the kvm_enabled() check to cpu_x86_register() > > --- > > target-i386/cpu.c | 12 ++++-------- > > 1 file changed, 4 insertions(+), 8 deletions(-) > >=20 > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index 333745b..754eb6f 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -206,22 +206,16 @@ typedef struct model_features_t { > > int check_cpuid =3D 0; > > int enforce_cpuid =3D 0; > > =20 > > -#if defined(CONFIG_KVM) > > static uint32_t kvm_default_features =3D (1 << KVM_FEATURE_CLOCKSOURCE= ) | > > (1 << KVM_FEATURE_NOP_IO_DELAY) | > > (1 << KVM_FEATURE_CLOCKSOURCE2) | > > (1 << KVM_FEATURE_ASYNC_PF) | > > (1 << KVM_FEATURE_STEAL_TIME) | > > (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT); > > -static const uint32_t kvm_pv_eoi_features =3D (0x1 << KVM_FEATURE_PV_E= OI); > > -#else > > -static uint32_t kvm_default_features =3D 0; > > -static const uint32_t kvm_pv_eoi_features =3D 0; > > -#endif > > =20 > > void enable_kvm_pv_eoi(void) > > { > > - kvm_default_features |=3D kvm_pv_eoi_features; > > + kvm_default_features |=3D (1UL << KVM_FEATURE_PV_EOI); > > } > > =20 > > void host_cpuid(uint32_t function, uint32_t count, > > @@ -1602,7 +1596,9 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu= _model) > > goto out; > > } > > =20 > > - def->kvm_features |=3D kvm_default_features; > > + if (kvm_enabled()) { > > + def->kvm_features |=3D kvm_default_features; > > + } > > def->ext_features |=3D CPUID_EXT_HYPERVISOR; > > =20 > > if (cpu_x86_parse_featurestr(def, features) < 0) { > >=20 >=20 >=20 > --=20 > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3= =BCrnberg -- Gleb.