From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41586) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLVWd-0008Tz-HS for qemu-devel@nongnu.org; Thu, 06 Mar 2014 05:27:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WLVWV-0001si-3x for qemu-devel@nongnu.org; Thu, 06 Mar 2014 05:27:39 -0500 Received: from mail-pd0-x231.google.com ([2607:f8b0:400e:c02::231]:34427) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLVWU-0001sb-QE for qemu-devel@nongnu.org; Thu, 06 Mar 2014 05:27:31 -0500 Received: by mail-pd0-f177.google.com with SMTP id g10so2332414pdj.8 for ; Thu, 06 Mar 2014 02:27:30 -0800 (PST) From: Xuebing Wang Date: Thu, 6 Mar 2014 18:27:08 +0800 Message-Id: <1394101634-21509-2-git-send-email-xbing6@gmail.com> In-Reply-To: <1394101634-21509-1-git-send-email-xbing6@gmail.com> References: <1394101634-21509-1-git-send-email-xbing6@gmail.com> Subject: [Qemu-devel] [PATCH 1/7] kvm: remove the hack "#if defined CONFIG_KVM || !defined NEED_CPU_H" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Paolo Bonzini , xbing6@gmail.com, Gleb Natapov There is no direct relevance between CONFIG_KVM and NEED_CPU_H. Thus logic of why using "#if defined CONFIG_KVM || !defined NEED_CPU_H" is hard to explain and hard to understand. The root cause is that we can not *always* get correct CONFIG_KVM, because: - CONFIG_KVM is defined in [*-softmmu | *-linux-user]/config-target.h - The "common" code outside folder [*-softmmu | *-linux-user] (in build time) can NOT include config-target.h and can NOT *always* have valid CONFIG_KVM. By removing "#if defined CONFIG_KVM || !defined NEED_CPU_H", we completely rely on runtime value. Advantage of applying this patch is: logic is clear Disadvantage is: binary code for non-CONFIG_KVM target is bigger (like sh4-softmmu or *-linux-user) - kvm_enabled() is always runtime (kvm_allowed), and compiler won't optimize if ( kvm_enabled() ) { ... } type of code. - Before the patch, for condition (!CONFIG_KVM && NEED_CPU_H), above example is preprocessed to be if (0) { ... } and then optimized >>From another perspective to explain why this patch does not cause troubles. 1) Before this patch, true value table: -------------------------------------------------------------------------- kvm_enabled = runtime value? | CONFIG_KVM defined? | NEED_CPU_H defined? | kvm_allowed | | | kvm_irqchip_in_kernel | | | ... for the rest 7 defines | | | -------------------------------------------------------------------------- runtime value | Y | Y | runtime value | Y | N | (a) const (0/false) | N | Y | runtime value | N | N | -------------------------------------------------------------------------- 2) After this patch, true value table: -------------------------------------------------------------------------- kvm_enabled = runtime value? | CONFIG_KVM defined? | NEED_CPU_H defined? | ... for the rest 8 defines | | | -------------------------------------------------------------------------- runtime value | Y | Y | runtime value | Y | N | (b) runtime value (0/false) | N | Y | runtime value | N | N | -------------------------------------------------------------------------- - Logic of runtime value kvm_allowed is: ( (kvm_available() in arch_init.c) && ((-machine accel=kvm) || (-enable-kvm) in runtime) ) - If CONFIG_KVM not defined, kvm_available() is always returns 0 - Thus for (b), runtime is always 0 - (b) is identical to (a) - result of after patch is identical to before patch Signed-off-by: Xuebing Wang --- include/sysemu/kvm.h | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index a02d67c..1829206 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -49,7 +49,6 @@ extern bool kvm_gsi_routing_allowed; extern bool kvm_gsi_direct_mapping; extern bool kvm_readonly_mem_allowed; -#if defined CONFIG_KVM || !defined NEED_CPU_H #define kvm_enabled() (kvm_allowed) /** * kvm_irqchip_in_kernel: @@ -123,18 +122,6 @@ extern bool kvm_readonly_mem_allowed; */ #define kvm_readonly_mem_enabled() (kvm_readonly_mem_allowed) -#else -#define kvm_enabled() (0) -#define kvm_irqchip_in_kernel() (false) -#define kvm_async_interrupts_enabled() (false) -#define kvm_halt_in_kernel() (false) -#define kvm_irqfds_enabled() (false) -#define kvm_msi_via_irqfd_enabled() (false) -#define kvm_gsi_routing_allowed() (false) -#define kvm_gsi_direct_mapping() (false) -#define kvm_readonly_mem_enabled() (false) -#endif - struct kvm_run; struct kvm_lapic_state; -- 1.7.9.5