From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40035) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W9CUY-0003Au-Hy for qemu-devel@nongnu.org; Fri, 31 Jan 2014 06:42:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W9CUR-0000UF-Ho for qemu-devel@nongnu.org; Fri, 31 Jan 2014 06:42:38 -0500 Received: from mail-ee0-x235.google.com ([2a00:1450:4013:c00::235]:46215) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W9CUR-0000U3-4X for qemu-devel@nongnu.org; Fri, 31 Jan 2014 06:42:31 -0500 Received: by mail-ee0-f53.google.com with SMTP id t10so2217693eei.40 for ; Fri, 31 Jan 2014 03:42:30 -0800 (PST) Sender: Paolo Bonzini Message-ID: <52EB8C20.7000207@redhat.com> Date: Fri, 31 Jan 2014 12:42:24 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1391111339-6958-1-git-send-email-ehabkost@redhat.com> <1391111339-6958-2-git-send-email-ehabkost@redhat.com> In-Reply-To: <1391111339-6958-2-git-send-email-ehabkost@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [uq/master PATCH 1/7] target-i386: Eliminate CONFIG_KVM #ifdefs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost , qemu-devel@nongnu.org Cc: libvir-list@redhat.com, Igor Mammedov , Jiri Denemark , =?ISO-8859-1?Q?Andreas_F=E4rber?= , kvm@vger.kernel.org Il 30/01/2014 20:48, Eduardo Habkost ha scritto: > The compiler is already able to eliminate the kvm_arch_get_supported_cpuid() > calls in kvm_cpu_fill_host() and filter_features_for_kvm(), so we can > eliminate the CONFIG_KVM #ifdefs there. > > Also, kvm_cpu_fill_host() and host_cpuid() don't need to check > CONFIG_KVM, as they don't have any KVM-specific function calls. > > Tested to build successfully with CONFIG_KVM disabled, using the > following CFLAGS combinations: "-DNDEBUG", "-DNDEBUG -O', "-DNDEBUG > -O0", "-DNDEBUG -O1", "-DNDEBUG -O2". > > Signed-off-by: Eduardo Habkost > --- > Changes v2: > * Check for __i386__ on host_cpuid() so the code compiles properly > on non-x86 hosts. > Suggested-by: Paolo Bonzini > > Change v3: > * Don't add assert(kvm_enabled()) line, which is not necessary to help > the compiler (and wouldn't help it if using -DNDEBUG, anyway). > Reported-by: Richard Henderson > * Commit message update > --- > target-i386/cpu.c | 15 +++------------ > 1 file changed, 3 insertions(+), 12 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 1f30efd..8425212 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -374,7 +374,6 @@ void disable_kvm_pv_eoi(void) > void host_cpuid(uint32_t function, uint32_t count, > uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx) > { > -#if defined(CONFIG_KVM) > uint32_t vec[4]; > > #ifdef __x86_64__ > @@ -382,7 +381,7 @@ void host_cpuid(uint32_t function, uint32_t count, > : "=a"(vec[0]), "=b"(vec[1]), > "=c"(vec[2]), "=d"(vec[3]) > : "0"(function), "c"(count) : "cc"); > -#else > +#elif defined(__i386__) > asm volatile("pusha \n\t" > "cpuid \n\t" > "mov %%eax, 0(%2) \n\t" > @@ -392,6 +391,8 @@ void host_cpuid(uint32_t function, uint32_t count, > "popa" > : : "a"(function), "c"(count), "S"(vec) > : "memory", "cc"); > +#else > + abort(); > #endif > > if (eax) > @@ -402,7 +403,6 @@ void host_cpuid(uint32_t function, uint32_t count, > *ecx = vec[2]; > if (edx) > *edx = vec[3]; > -#endif > } > > #define iswhite(c) ((c) && ((c) <= ' ' || '~' < (c))) > @@ -1119,7 +1119,6 @@ void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w, > } > } > > -#ifdef CONFIG_KVM > static int cpu_x86_fill_model_id(char *str) > { > uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; > @@ -1134,7 +1133,6 @@ static int cpu_x86_fill_model_id(char *str) > } > return 0; > } > -#endif > > /* Fill a x86_def_t struct with information about the host CPU, and > * the CPU features supported by the host hardware + host kernel > @@ -1143,7 +1141,6 @@ static int cpu_x86_fill_model_id(char *str) > */ > static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) > { > -#ifdef CONFIG_KVM > KVMState *s = kvm_state; > uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; > > @@ -1173,8 +1170,6 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) > kvm_arch_get_supported_cpuid(s, wi->cpuid_eax, wi->cpuid_ecx, > wi->cpuid_reg); > } > - > -#endif /* CONFIG_KVM */ > } > > static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask) > @@ -1817,7 +1812,6 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp) > return cpu_list; > } > > -#ifdef CONFIG_KVM > static void filter_features_for_kvm(X86CPU *cpu) > { > CPUX86State *env = &cpu->env; > @@ -1834,7 +1828,6 @@ static void filter_features_for_kvm(X86CPU *cpu) > cpu->filtered_features[w] = requested_features & ~env->features[w]; > } > } > -#endif > > static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp) > { > @@ -2545,9 +2538,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) > "Host's CPU doesn't support requested features"); > goto out; > } > -#ifdef CONFIG_KVM > filter_features_for_kvm(cpu); > -#endif > } > > #ifndef CONFIG_USER_ONLY > Reviewed-by: Paolo Bonzini