[Qemu-devel] [PATCH 0/7] remove #if defined CONFIG_KVM || !defined NEED_CPU_H

[Qemu-devel] [PATCH 0/7] remove #if defined CONFIG_KVM || !defined NEED_CPU_H

[Xuebing Wang]

Hi Community,

I am not sure if there is value for this patchset.

After the first pach:
Size of x86_64-softmmu/qemu-system-x86_64 is unchanged.
Size of sh4-softmmu/qemu-system-sh4 increases by about 2.8KB.

Xuebing Wang (7):
  kvm: remove the hack "#if defined CONFIG_KVM || !defined NEED_CPU_H"
  kvm-i386: fix build for x86_64-linux-user after applying previous
    patch
  kvm-i386: remove target-i386/kvm-stub.c
  kvm: fix build for target sh4-softmmu
  kvm-ppc: fix build for ppc64-softmmu
  kvm-ppc: remove target-ppc/kvm-stub.c
  kvm-i386: fix build for "x86_64-softmmu --disable-kvm"

 hw/i386/kvm/i8259.c       |    2 +-
 hw/i386/kvm/ioapic.c      |    2 +-
 hw/i386/pc_piix.c         |    2 +-
 hw/i386/pc_q35.c          |    2 +-
 include/hw/i386/pc.h      |    1 -
 include/hw/ppc/openpic.h  |    1 -
 include/sysemu/kvm.h      |   18 ------------------
 kvm-stub.c                |    1 +
 target-i386/Makefile.objs |    1 -
 target-i386/cpu.c         |    2 +-
 target-i386/kvm-stub.c    |   30 ------------------------------
 target-i386/kvm_i386.h    |   32 ++++++++++++++++++++++++++++++++
 target-ppc/Makefile.objs  |    1 -
 target-ppc/kvm-stub.c     |   18 ------------------
 target-ppc/kvm_ppc.h      |   17 +++++++++++++++++
 15 files changed, 55 insertions(+), 75 deletions(-)
 delete mode 100644 target-i386/kvm-stub.c
 delete mode 100644 target-ppc/kvm-stub.c

-- 
1.7.9.5

[Qemu-devel] [PATCH 1/7] kvm: remove the hack "#if defined CONFIG_KVM || !defined NEED_CPU_H"

[Xuebing Wang]

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

[Qemu-devel] [PATCH 2/7] kvm-i386: fix build for x86_64-linux-user after applying previous patch

[Xuebing Wang]

As explained in previous patch, kvm functions won't be optimized out for
non-CONFIG_KVM.

Signed-off-by: Xuebing Wang <xbing6@gmail.com>
---
 target-i386/kvm-stub.c |    6 ------
 1 file changed, 6 deletions(-)

diff --git a/target-i386/kvm-stub.c b/target-i386/kvm-stub.c
index 2b9e801..0ef642d 100644
--- a/target-i386/kvm-stub.c
+++ b/target-i386/kvm-stub.c
@@ -17,14 +17,8 @@ bool kvm_allows_irq0_override(void)
     return 1;
 }
 
-#ifndef __OPTIMIZE__
-/* This function is only called inside conditionals which we
- * rely on the compiler to optimize out when CONFIG_KVM is not
- * defined.
- */
 uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
                                       uint32_t index, int reg)
 {
     abort();
 }
-#endif
-- 
1.7.9.5

[Qemu-devel] [PATCH 3/7] kvm-i386: remove target-i386/kvm-stub.c

[Xuebing Wang]

To eliminate kvm-stub for target-i386.

Size of text section of x86_64-linux-user/qemu-x86_64 is reduced by 104 bytes
by inline these 2 functions.

Signed-off-by: Xuebing Wang <xbing6@gmail.com>
---
 include/sysemu/kvm.h      |    3 ---
 target-i386/Makefile.objs |    1 -
 target-i386/cpu.c         |    2 +-
 target-i386/kvm-stub.c    |   24 ------------------------
 target-i386/kvm_i386.h    |   26 ++++++++++++++++++++++++++
 5 files changed, 27 insertions(+), 29 deletions(-)
 delete mode 100644 target-i386/kvm-stub.c

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 1829206..60e95d8 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -281,9 +281,6 @@ bool kvm_arch_stop_on_emulation_error(CPUState *cpu);
 
 int kvm_check_extension(KVMState *s, unsigned int extension);
 
-uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
-                                      uint32_t index, int reg);
-
 #if !defined(CONFIG_USER_ONLY)
 int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr,
                                        hwaddr *phys_addr);
diff --git a/target-i386/Makefile.objs b/target-i386/Makefile.objs
index 027b94e..abebea8 100644
--- a/target-i386/Makefile.objs
+++ b/target-i386/Makefile.objs
@@ -4,6 +4,5 @@ obj-y += smm_helper.o misc_helper.o mem_helper.o seg_helper.o
 obj-y += gdbstub.o
 obj-$(CONFIG_SOFTMMU) += machine.o arch_memory_mapping.o arch_dump.o
 obj-$(CONFIG_KVM) += kvm.o
-obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o
 obj-$(CONFIG_LINUX_USER) += ioport-user.o
 obj-$(CONFIG_BSD_USER) += ioport-user.o
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0e8812a..31ebef0 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -22,7 +22,7 @@
 #include <inttypes.h>
 
 #include "cpu.h"
-#include "sysemu/kvm.h"
+#include "kvm_i386.h"
 #include "sysemu/cpus.h"
 #include "topology.h"
 
diff --git a/target-i386/kvm-stub.c b/target-i386/kvm-stub.c
deleted file mode 100644
index 0ef642d..0000000
--- a/target-i386/kvm-stub.c
+++ /dev/null
@@ -1,24 +0,0 @@
-/*
- * QEMU KVM x86 specific function stubs
- *
- * Copyright Linaro Limited 2012
- *
- * Author: Peter Maydell <peter.maydell@linaro.org>
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- *
- */
-#include "qemu-common.h"
-#include "kvm_i386.h"
-
-bool kvm_allows_irq0_override(void)
-{
-    return 1;
-}
-
-uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
-                                      uint32_t index, int reg)
-{
-    abort();
-}
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index 4392ab4..202b811 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -11,9 +11,24 @@
 #ifndef QEMU_KVM_I386_H
 #define QEMU_KVM_I386_H
 
+/* Adding "config-target.h" is to ensure this header file is correct by itself.
+ *
+ * Only source files in *-softmmu/ or *-linux-user/ folder can
+ * include "config-target.h". If this header is incorrectly included,
+ * compiler errors out.
+ */
+#include "config-target.h" /* for CONFIG_KVM */
+
 #include "sysemu/kvm.h"
 
+#if defined(CONFIG_KVM)
 bool kvm_allows_irq0_override(void);
+#else
+static inline bool kvm_allows_irq0_override(void)
+{
+    return 1;
+}
+#endif
 
 int kvm_device_pci_assign(KVMState *s, PCIHostDeviceAddress *dev_addr,
                           uint32_t flags, uint32_t *dev_id);
@@ -35,4 +50,15 @@ int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
 int kvm_device_msix_assign(KVMState *s, uint32_t dev_id);
 int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id);
 
+#if defined(CONFIG_KVM)
+uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
+                                      uint32_t index, int reg);
+#else
+static inline uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
+                                      uint32_t index, int reg)
+{
+    abort();
+}
 #endif
+
+#endif /* QEMU_KVM_I386_H */
-- 
1.7.9.5

[Qemu-devel] [PATCH 4/7] kvm: fix build for target sh4-softmmu

[Xuebing Wang]

The build issue was caused by the patch of removing
"#if defined CONFIG_KVM || !defined NEED_CPU_H"

Signed-off-by: Xuebing Wang <xbing6@gmail.com>
---
 kvm-stub.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/kvm-stub.c b/kvm-stub.c
index e979f76..f859249 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -22,6 +22,7 @@
 KVMState *kvm_state;
 bool kvm_kernel_irqchip;
 bool kvm_async_interrupts_allowed;
+bool kvm_halt_in_kernel_allowed;
 bool kvm_irqfds_allowed;
 bool kvm_msi_via_irqfd_allowed;
 bool kvm_gsi_routing_allowed;
-- 
1.7.9.5

[Qemu-devel] [PATCH 5/7] kvm-ppc: fix build for ppc64-softmmu

[Xuebing Wang]

The build issue was caused by the patch of removing
"#if defined CONFIG_KVM || !defined NEED_CPU_H"

Signed-off-by: Xuebing Wang <xbing6@gmail.com>
---
 target-ppc/kvm_ppc.h |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 5f78e4b..c532d2a 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -11,7 +11,19 @@
 
 #define TYPE_HOST_POWERPC_CPU "host-" TYPE_POWERPC_CPU
 
+/* Adding "config-target.h" is to ensure this header file is correct by itself.
+ *
+ * Only source files in *-softmmu/ or *-linux-user/ folder can
+ * include "config-target.h". If this header is incorrectly included,
+ * compiler errors out.
+ */
+#include "config-target.h" /* for CONFIG_KVM, CONFIG_USER_ONLY */
+
+#ifdef CONFIG_KVM
 void kvmppc_init(void);
+#else
+static inline void kvmppc_init(void) {}
+#endif
 
 #ifdef CONFIG_KVM
 
-- 
1.7.9.5

[Qemu-devel] [PATCH 6/7] kvm-ppc: remove target-ppc/kvm-stub.c

[Xuebing Wang]

Size of text section of ppc64-softmmu/qemu-system-ppc64 is reduced by
152 bytes.

Signed-off-by: Xuebing Wang <xbing6@gmail.com>
---
 include/hw/ppc/openpic.h |    1 -
 target-ppc/Makefile.objs |    1 -
 target-ppc/kvm-stub.c    |   18 ------------------
 target-ppc/kvm_ppc.h     |    5 +++++
 4 files changed, 5 insertions(+), 20 deletions(-)
 delete mode 100644 target-ppc/kvm-stub.c

diff --git a/include/hw/ppc/openpic.h b/include/hw/ppc/openpic.h
index ee67098..8c8719c 100644
--- a/include/hw/ppc/openpic.h
+++ b/include/hw/ppc/openpic.h
@@ -27,6 +27,5 @@ enum {
                              OPENPIC_MAX_TMR)
 
 #define TYPE_KVM_OPENPIC "kvm-openpic"
-int kvm_openpic_connect_vcpu(DeviceState *d, CPUState *cs);
 
 #endif /* __OPENPIC_H__ */
diff --git a/target-ppc/Makefile.objs b/target-ppc/Makefile.objs
index 3cb23e0..9428df3 100644
--- a/target-ppc/Makefile.objs
+++ b/target-ppc/Makefile.objs
@@ -5,7 +5,6 @@ obj-y += machine.o mmu_helper.o mmu-hash32.o
 obj-$(TARGET_PPC64) += mmu-hash64.o arch_dump.o
 endif
 obj-$(CONFIG_KVM) += kvm.o kvm_ppc.o
-obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o
 obj-y += excp_helper.o
 obj-y += fpu_helper.o
 obj-y += int_helper.o
diff --git a/target-ppc/kvm-stub.c b/target-ppc/kvm-stub.c
deleted file mode 100644
index ee3f5d2..0000000
--- a/target-ppc/kvm-stub.c
+++ /dev/null
@@ -1,18 +0,0 @@
-/*
- * QEMU KVM PPC specific function stubs
- *
- * Copyright Freescale Inc. 2013
- *
- * Author: Alexander Graf <agraf@suse.de>
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- *
- */
-#include "qemu-common.h"
-#include "hw/ppc/openpic.h"
-
-int kvm_openpic_connect_vcpu(DeviceState *d, CPUState *cs)
-{
-    return -EINVAL;
-}
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index c532d2a..27d855d 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -21,8 +21,13 @@
 
 #ifdef CONFIG_KVM
 void kvmppc_init(void);
+int kvm_openpic_connect_vcpu(DeviceState *d, CPUState *cs);
 #else
 static inline void kvmppc_init(void) {}
+static inline int kvm_openpic_connect_vcpu(DeviceState *d, CPUState *cs)
+{
+    return -EINVAL;
+}
 #endif
 
 #ifdef CONFIG_KVM
-- 
1.7.9.5

[Qemu-devel] [PATCH 7/7] kvm-i386: fix build for "x86_64-softmmu --disable-kvm"

[Xuebing Wang]

Probably no one uses disable-kvm for x86_64.

Also move 3 kvm-i386 function declarations from sysemu/kvm.h
     => target-i386/kvm_i386.h
- kvm_pc_gsi_handler()
- kvm_pc_setup_irq_routing()
- kvm_i8259_init()

Signed-off-by: Xuebing Wang <xbing6@gmail.com>
---
 hw/i386/kvm/i8259.c    |    2 +-
 hw/i386/kvm/ioapic.c   |    2 +-
 hw/i386/pc_piix.c      |    2 +-
 hw/i386/pc_q35.c       |    2 +-
 include/hw/i386/pc.h   |    1 -
 include/sysemu/kvm.h   |    2 --
 target-i386/kvm_i386.h |    6 ++++++
 7 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/i386/kvm/i8259.c b/hw/i386/kvm/i8259.c
index 53e3ca8..3ec717e 100644
--- a/hw/i386/kvm/i8259.c
+++ b/hw/i386/kvm/i8259.c
@@ -11,7 +11,7 @@
  */
 #include "hw/isa/i8259_internal.h"
 #include "hw/i386/apic_internal.h"
-#include "sysemu/kvm.h"
+#include "kvm_i386.h"
 
 #define TYPE_KVM_I8259 "kvm-i8259"
 #define KVM_PIC_CLASS(class) \
diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
index d2a6c4c..4837d25 100644
--- a/hw/i386/kvm/ioapic.c
+++ b/hw/i386/kvm/ioapic.c
@@ -13,7 +13,7 @@
 #include "hw/i386/pc.h"
 #include "hw/i386/ioapic_internal.h"
 #include "hw/i386/apic_internal.h"
-#include "sysemu/kvm.h"
+#include "kvm_i386.h"
 
 /* PC Utility function */
 void kvm_pc_setup_irq_routing(bool pci_enabled)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index d5dc1ef..a71f2e8 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -35,7 +35,7 @@
 #include "net/net.h"
 #include "hw/boards.h"
 #include "hw/ide.h"
-#include "sysemu/kvm.h"
+#include "kvm_i386.h"
 #include "hw/kvm/clock.h"
 #include "sysemu/sysemu.h"
 #include "hw/sysbus.h"
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index a7f6260..3c5fb01 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -34,7 +34,7 @@
 #include "hw/boards.h"
 #include "hw/timer/mc146818rtc.h"
 #include "hw/xen/xen.h"
-#include "sysemu/kvm.h"
+#include "kvm_i386.h"
 #include "hw/kvm/clock.h"
 #include "hw/pci-host/q35.h"
 #include "exec/address-spaces.h"
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 9010246..ff8cf57 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -72,7 +72,6 @@ bool parallel_mm_init(MemoryRegion *address_space,
 
 extern DeviceState *isa_pic;
 qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq);
-qemu_irq *kvm_i8259_init(ISABus *bus);
 int pic_read_irq(DeviceState *d);
 int pic_get_output(DeviceState *d);
 void pic_info(Monitor *mon, const QDict *qdict);
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 60e95d8..2bf29fc 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -322,8 +322,6 @@ void kvm_irqchip_release_virq(KVMState *s, int virq);
 int kvm_irqchip_add_irqfd_notifier(KVMState *s, EventNotifier *n,
                                    EventNotifier *rn, int virq);
 int kvm_irqchip_remove_irqfd_notifier(KVMState *s, EventNotifier *n, int virq);
-void kvm_pc_gsi_handler(void *opaque, int n, int level);
-void kvm_pc_setup_irq_routing(bool pci_enabled);
 void kvm_init_irq_routing(KVMState *s);
 
 /**
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index 202b811..e2ffbde 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -51,9 +51,15 @@ int kvm_device_msix_assign(KVMState *s, uint32_t dev_id);
 int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id);
 
 #if defined(CONFIG_KVM)
+void kvm_pc_gsi_handler(void *opaque, int n, int level);
+void kvm_pc_setup_irq_routing(bool pci_enabled);
+qemu_irq *kvm_i8259_init(ISABus *bus);
 uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
                                       uint32_t index, int reg);
 #else
+static inline void kvm_pc_gsi_handler(void *opaque, int n, int level) {}
+static inline void kvm_pc_setup_irq_routing(bool pci_enabled) {}
+static inline qemu_irq *kvm_i8259_init(ISABus *bus) {return NULL;}
 static inline uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
                                       uint32_t index, int reg)
 {
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 0/7] remove #if defined CONFIG_KVM || !defined NEED_CPU_H

[Paolo Bonzini]

Il 06/03/2014 11:27, Xuebing Wang ha scritto:
> Hi Community,
>
> I am not sure if there is value for this patchset.

I am not sure this is too useful, since after all the code works and the 
patches are tricky.

I'd be more interested in seeing work done to remove qemu-common.h and 
cpu.h inclusions from header files, and possibly splitting TCG-specific 
parts out of exec-all.h.  I hope we convinced you that this is the right 
thing to do for such central files.

Paolo

> After the first pach:
> Size of x86_64-softmmu/qemu-system-x86_64 is unchanged.
> Size of sh4-softmmu/qemu-system-sh4 increases by about 2.8KB.
>
> Xuebing Wang (7):
>   kvm: remove the hack "#if defined CONFIG_KVM || !defined NEED_CPU_H"
>   kvm-i386: fix build for x86_64-linux-user after applying previous
>     patch
>   kvm-i386: remove target-i386/kvm-stub.c
>   kvm: fix build for target sh4-softmmu
>   kvm-ppc: fix build for ppc64-softmmu
>   kvm-ppc: remove target-ppc/kvm-stub.c
>   kvm-i386: fix build for "x86_64-softmmu --disable-kvm"
>
>  hw/i386/kvm/i8259.c       |    2 +-
>  hw/i386/kvm/ioapic.c      |    2 +-
>  hw/i386/pc_piix.c         |    2 +-
>  hw/i386/pc_q35.c          |    2 +-
>  include/hw/i386/pc.h      |    1 -
>  include/hw/ppc/openpic.h  |    1 -
>  include/sysemu/kvm.h      |   18 ------------------
>  kvm-stub.c                |    1 +
>  target-i386/Makefile.objs |    1 -
>  target-i386/cpu.c         |    2 +-
>  target-i386/kvm-stub.c    |   30 ------------------------------
>  target-i386/kvm_i386.h    |   32 ++++++++++++++++++++++++++++++++
>  target-ppc/Makefile.objs  |    1 -
>  target-ppc/kvm-stub.c     |   18 ------------------
>  target-ppc/kvm_ppc.h      |   17 +++++++++++++++++
>  15 files changed, 55 insertions(+), 75 deletions(-)
>  delete mode 100644 target-i386/kvm-stub.c
>  delete mode 100644 target-ppc/kvm-stub.c
>

Re: [Qemu-devel] [PATCH 1/7] kvm: remove the hack "#if defined CONFIG_KVM || !defined NEED_CPU_H"

[Andreas Färber]

Am 06.03.2014 11:27, schrieb Xuebing Wang:
> 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
[...]
> 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

Have you tested compiling with --disable-kvm? CONFIG_KVM is not just
about targets like sh4 that don't support KVM but also about non-Linux
hosts.

Regards,
Andreas

>  #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;
>  

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 0/7] remove #if defined CONFIG_KVM || !defined NEED_CPU_H

[Andreas Färber]

Hi,

Am 06.03.2014 11:27, schrieb Xuebing Wang:
> I am not sure if there is value for this patchset.
> 
> After the first pach:
> Size of x86_64-softmmu/qemu-system-x86_64 is unchanged.
> Size of sh4-softmmu/qemu-system-sh4 increases by about 2.8KB.
> 
> Xuebing Wang (7):
>   kvm: remove the hack "#if defined CONFIG_KVM || !defined NEED_CPU_H"
>   kvm-i386: fix build for x86_64-linux-user after applying previous
>     patch
>   kvm-i386: remove target-i386/kvm-stub.c
>   kvm: fix build for target sh4-softmmu
>   kvm-ppc: fix build for ppc64-softmmu
>   kvm-ppc: remove target-ppc/kvm-stub.c
>   kvm-i386: fix build for "x86_64-softmmu --disable-kvm"

Generally, you can't just break things and then later fix them. You need
to keep things building in each commit you make, for bisectability.

Also may I propose that we continue these cleanup discussions once we're
in Hard Freeze next week? There's still actual features and bug fixes to
be reviewed until then.

http://wiki.qemu.org/Planning/2.0

Further I notice that Alex and the qemu-ppc list were not CC'ed on
patches 5-6. You may find the following useful to automate this:

$ git config sendemail.cccmd \
  "scripts/get_maintainer.pl --nogit-fallback"

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg