qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Xuebing Wang <xbing6@gmail.com>
To: qemu-devel@nongnu.org
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	xbing6@gmail.com, Gleb Natapov <gleb@redhat.com>
Subject: [Qemu-devel] [PATCH 1/7] kvm: remove the hack "#if defined CONFIG_KVM || !defined NEED_CPU_H"
Date: Thu,  6 Mar 2014 18:27:08 +0800	[thread overview]
Message-ID: <1394101634-21509-2-git-send-email-xbing6@gmail.com> (raw)
In-Reply-To: <1394101634-21509-1-git-send-email-xbing6@gmail.com>

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 <xbing6@gmail.com>
---
 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

  reply	other threads:[~2014-03-06 10:27 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-06 10:27 [Qemu-devel] [PATCH 0/7] remove #if defined CONFIG_KVM || !defined NEED_CPU_H Xuebing Wang
2014-03-06 10:27 ` Xuebing Wang [this message]
2014-03-06 14:54   ` [Qemu-devel] [PATCH 1/7] kvm: remove the hack "#if defined CONFIG_KVM || !defined NEED_CPU_H" Andreas Färber
2014-03-06 10:27 ` [Qemu-devel] [PATCH 2/7] kvm-i386: fix build for x86_64-linux-user after applying previous patch Xuebing Wang
2014-03-06 10:27 ` [Qemu-devel] [PATCH 3/7] kvm-i386: remove target-i386/kvm-stub.c Xuebing Wang
2014-03-06 10:27 ` [Qemu-devel] [PATCH 4/7] kvm: fix build for target sh4-softmmu Xuebing Wang
2014-03-06 10:27 ` [Qemu-devel] [PATCH 5/7] kvm-ppc: fix build for ppc64-softmmu Xuebing Wang
2014-03-06 10:27 ` [Qemu-devel] [PATCH 6/7] kvm-ppc: remove target-ppc/kvm-stub.c Xuebing Wang
2014-03-06 10:27 ` [Qemu-devel] [PATCH 7/7] kvm-i386: fix build for "x86_64-softmmu --disable-kvm" Xuebing Wang
2014-03-06 12:00 ` [Qemu-devel] [PATCH 0/7] remove #if defined CONFIG_KVM || !defined NEED_CPU_H Paolo Bonzini
2014-03-06 15:16 ` Andreas Färber

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1394101634-21509-2-git-send-email-xbing6@gmail.com \
    --to=xbing6@gmail.com \
    --cc=gleb@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).