From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46256) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WxI8W-0001LR-M1 for qemu-devel@nongnu.org; Wed, 18 Jun 2014 11:51:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WxI8P-0000iC-L2 for qemu-devel@nongnu.org; Wed, 18 Jun 2014 11:50:56 -0400 Received: from cantor2.suse.de ([195.135.220.15]:55557 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WxI8P-0000i4-Ax for qemu-devel@nongnu.org; Wed, 18 Jun 2014 11:50:49 -0400 Message-ID: <53A1B557.4020104@suse.de> Date: Wed, 18 Jun 2014 17:50:47 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1398876525-28831-1-git-send-email-ehabkost@redhat.com> <1398876525-28831-13-git-send-email-ehabkost@redhat.com> <53750D57.7010403@suse.de> <20140515191215.GU3302@otherpad.lan.raisama.net> In-Reply-To: <20140515191215.GU3302@otherpad.lan.raisama.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 12/18] target-i386: Support check/enforce flags in TCG mode, too List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Marcelo Tosatti , qemu-devel@nongnu.org, Paolo Bonzini , Igor Mammedov , Aurelien Jarno , Richard Henderson Am 15.05.2014 21:12, schrieb Eduardo Habkost: > On Thu, May 15, 2014 at 08:54:15PM +0200, Andreas F=E4rber wrote: >> Am 30.04.2014 18:48, schrieb Eduardo Habkost: >>> If enforce/check is specified in TCG mode, QEMU will ensure all CPU >>> features are supported by TCG, so no CPU feature is silently disabled= . >>> >>> Reviewed-by: Richard Henderson >>> Signed-off-by: Eduardo Habkost >>> --- >>> Changes v1 -> v2: >>> * Trivial rebase to latest qom-cpu (commit 90c5d39c) >>> (Reviewed-by line kept) >>> Changes v2 -> v3: >>> * Trivial rebase after QEMU 2.0 (onto commit 2d03b49) >>> (Reviewed-by line kept) >>> --- >>> target-i386/cpu.c | 34 ++++++++++++++++------------------ >>> 1 file changed, 16 insertions(+), 18 deletions(-) >>> >>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c >>> index b2e30ca..53b5038 100644 >>> --- a/target-i386/cpu.c >>> +++ b/target-i386/cpu.c >>> @@ -1265,8 +1265,9 @@ static int report_unavailable_features(FeatureW= ord w, uint32_t mask) >>> if (1 << i & mask) { >>> const char *reg =3D get_register_name_32(f->cpuid_reg); >>> assert(reg); >>> - fprintf(stderr, "warning: host doesn't support requested= feature: " >>> + fprintf(stderr, "warning: %s doesn't support requested f= eature: " >>> "CPUID.%02XH:%s%s%s [bit %d]\n", >>> + kvm_enabled() ? "host" : "TCG", >>> f->cpuid_eax, reg, >>> f->feat_names[i] ? "." : "", >>> f->feat_names[i] ? f->feat_names[i] : "", i); >>> @@ -1826,17 +1827,18 @@ CpuDefinitionInfoList *arch_query_cpu_definit= ions(Error **errp) >>> static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w) >>> { >>> FeatureWordInfo *wi =3D &feature_word_info[w]; >>> - assert(kvm_enabled()); >>> - return kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid_eax, >>> - wi->cpuid_ecx, >>> - wi->cpuid_reg); >>> + if (kvm_enabled()) { >>> + return kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid_eax= , >>> + wi->cpuid_ecx= , >>> + wi->cpuid_reg= ); >>> + } else { >>> + return wi->tcg_features; >>> + } >>> } >> >> This function is called unconditionally now, so apply the following? >> >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c >> index 48ba1d8..112b437 100644 >> --- a/target-i386/cpu.c >> +++ b/target-i386/cpu.c >> @@ -1839,8 +1839,10 @@ static uint32_t >> x86_cpu_get_supported_feature_word(FeatureWord w) >> return kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid_eax, >> wi->cpuid_ecx, >> wi->cpuid_reg)= ; >> - } else { >> + } else if (tcg_enabled()) { >> return wi->tcg_features; >> + } else { >> + return UINT32_MAX; >=20 > Agreed, but I would prefer writing it as ~0 instead of UINT32_MAX. FTR done as ~0u to avoid any signedness issues. >=20 >> } >> } >> >> >> Not sure what to do about the warning message. It wouldn't occur thoug= h >> due to the suggested mask, so we could just ignore it for now. >=20 > One day we may be able to simply ask the machine object for the current > accelerator name. In the meantime, we could use: >=20 > "warning: host (%s) doesn't support requested feature [...]", > kvm_enabled() ? "KVM" : (tcg_enabled() ? "TCG" : "QEMU") >=20 > (But I won't object if you prefer to keep the warning message I > originally sent.) I think I did the latter, yes... Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg