From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40731) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ug3BM-0008Df-JB for qemu-devel@nongnu.org; Fri, 24 May 2013 21:22:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ug3BK-0006HB-17 for qemu-devel@nongnu.org; Fri, 24 May 2013 21:22:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41022) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ug3BJ-0006GT-Ny for qemu-devel@nongnu.org; Fri, 24 May 2013 21:22:01 -0400 From: Bandan Das References: Date: Fri, 24 May 2013 21:21:59 -0400 Message-ID: MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH] target-i386: Disable CPUID_EXT_MONITOR when KVM is enabled List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Igor Mammedov , Eduardo Habkost , Andreas =?utf-8?Q?F=C3=A4rber?= Forwarding message by Eduardo. I had misspelled nongnu.org in my first attempt! The spaces/tab comment by Eduardo has been fixed. Eduardo Habkost writes: > > By default, CPUID_EXT_MONITOR is enabled for some cpu models > such as Opteron_G3. Disable it if kvm_enabled() is true since > monitor/mwait aren't supported by KVM yet. > > Signed-off-by: Bandan Das Interesting, I haven't noticed that TCG supports CPUID_EXT_MONITOR. I believe that's yet another reason to make the KVM CPU models separate classes from the TCG CPU models: because "-machine ...,accel=kvm -cpu Foo" and "-machine ...,accel=tcg -cpu Foo" _already_ have different meanings today and result in different CPUs. Making them classes would just make the fact that they _are_ different CPU models explicit. > --- > There is no user visible side-effect to this behavior, the aim > is to clean up the default flags that are not supported (yet). There is one user-visible effect: "-cpu ...,enforce" will stop failing because of missing KVM support for CPUID_EXT_MONITOR. But that's exactly the point: there's no point in having CPU model definitions that would never work as-is with neither TCG or KVM. This patch is changing the meaning of (e.g.) "-machine ...,accel=kvm -cpu Opteron_G3" to match what was already happening in practice. > > target-i386/cpu.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 1a501d9..c83ba1c 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -1749,6 +1749,7 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp) > > if (kvm_enabled()) { > def->features[FEAT_KVM] |= kvm_default_features; > + def->features[FEAT_1_ECX] &= ~CPUID_EXT_MONITOR; You are mixing spaces and tabs, here. > } > def->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR; > > -- > 1.8.1.4 > -- Eduardo > By default, CPUID_EXT_MONITOR is enabled for some cpu models > such as Opteron_G3. Disable it if kvm_enabled() is true since > monitor/mwait aren't supported by KVM yet. > > Signed-off-by: Bandan Das > --- > There is no user visible side-effect to this behavior, the aim > is to clean up the default flags that are not supported (yet). > > target-i386/cpu.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 1a501d9..c83ba1c 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -1749,6 +1749,7 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp) > > if (kvm_enabled()) { > def->features[FEAT_KVM] |= kvm_default_features; > + def->features[FEAT_1_ECX] &= ~CPUID_EXT_MONITOR; > } > def->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;