qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Discussion 03/10] NEED_CPU_H: remove unnecessary use o


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [Discussion 03/10] NEED_CPU_H: remove unnecessary use of NEED_CPU_H
Date: Tue, 04 Mar 2014 11:20:49 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0

Il 04/03/2014 03:47, Xuebing Wang ha scritto:
Note: there is a FIXME to be addressed in this patch.

For every appearance of NEED_CPU_H, there must be '#include "cpu.h"' to
include "target-xxx/cpu.h", because the code below NEED_CPU_H depends on
architecture-specific information.

Signed-off-by: Xuebing Wang <address@hidden>

Here I disagree. I think that if cpu.h is needed by a header, it should be included by whoever needs that header.

---
 include/exec/cpu-common.h |    5 +++++
 include/hw/hw.h           |   13 +++++++++----
 include/hw/xen/xen.h      |    2 +-
 include/qemu/log.h        |    7 ++++---
 include/sysemu/kvm.h      |    2 +-
 ui/input.c                |    2 --
 6 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index a21b65a..a696667 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -98,6 +98,11 @@ void stl_be_phys(AddressSpace *as, hwaddr addr, uint32_t 
val);
 void stq_le_phys(AddressSpace *as, hwaddr addr, uint64_t val);
 void stq_be_phys(AddressSpace *as, hwaddr addr, uint64_t val);

+/*
+ * FIXME: although it's nice to keep ld/st APIs together with above,
+ *        below violates the idea that every NEED_CPU_H should be followed
+ *        by inclusion of "target-xxx/cpu.h"
+ */
 #ifdef NEED_CPU_H
 uint32_t lduw_phys(AddressSpace *as, hwaddr addr);
 uint32_t ldl_phys(AddressSpace *as, hwaddr addr);
diff --git a/include/hw/hw.h b/include/hw/hw.h
index 33bdb92..3accd44 100644
--- a/include/hw/hw.h
+++ b/include/hw/hw.h
@@ -4,7 +4,7 @@

 #include "qemu-common.h"

-#if !defined(CONFIG_USER_ONLY) && !defined(NEED_CPU_H)
+#if !defined(CONFIG_USER_ONLY)
 #include "exec/cpu-common.h"
 #endif

@@ -15,6 +15,8 @@
 #include "qemu/log.h"

 #ifdef NEED_CPU_H
+#include "cpu.h" /* target-xxx/cpu.h, required for TARGET_LONG_BITS */
+
 #if TARGET_LONG_BITS == 64
 #define qemu_put_betl qemu_put_be64
 #define qemu_get_betl qemu_get_be64
@@ -34,7 +36,7 @@
 #define qemu_put_sbetls qemu_put_sbe32s
 #define qemu_get_sbetls qemu_get_sbe32s
 #endif
-#endif
+#endif /* NEED_CPU_H */

 typedef void QEMUResetHandler(void *opaque);

@@ -48,6 +50,8 @@ void qemu_register_boot_set(QEMUBootSetHandler *func, void 
*opaque);
 int qemu_boot_set(const char *boot_order);

 #ifdef NEED_CPU_H
+#include "cpu.h" /* target-xxx/cpu.h, required for TARGET_LONG_BITS */
+
 #if TARGET_LONG_BITS == 64
 #define VMSTATE_UINTTL_V(_f, _s, _v)                                  \
     VMSTATE_UINT64_V(_f, _s, _v)
@@ -63,6 +67,7 @@ int qemu_boot_set(const char *boot_order);
 #define VMSTATE_UINTTL_ARRAY_V(_f, _s, _n, _v)                        \
     VMSTATE_UINT32_ARRAY_V(_f, _s, _n, _v)
 #endif
+
 #define VMSTATE_UINTTL(_f, _s)                                        \
     VMSTATE_UINTTL_V(_f, _s, 0)
 #define VMSTATE_UINTTL_EQUAL(_f, _s)                                  \
@@ -70,6 +75,6 @@ int qemu_boot_set(const char *boot_order);
 #define VMSTATE_UINTTL_ARRAY(_f, _s, _n)                              \
     VMSTATE_UINTTL_ARRAY_V(_f, _s, _n, 0)

-#endif
+#endif /* NEED_CPU_H */

-#endif
+#endif /* QEMU_HW_H */
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index 34773b9..638817b 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -40,7 +40,7 @@ int xen_init(void);
 int xen_hvm_init(MemoryRegion **ram_memory);
 void xenstore_store_pv_console_info(int i, struct CharDriverState *chr);

-#if defined(NEED_CPU_H) && !defined(CONFIG_USER_ONLY)
+#if !defined(CONFIG_USER_ONLY)
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
                    struct MemoryRegion *mr);
 void xen_modified_memory(ram_addr_t start, ram_addr_t length);
diff --git a/include/qemu/log.h b/include/qemu/log.h
index d515424..715accb 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -6,9 +6,7 @@
 #include <stdio.h>
 #include "qemu/compiler.h"
 #include "qom/cpu.h"
-#ifdef NEED_CPU_H
 #include "disas/disas.h"
-#endif

 /* Private global variables, don't use */
 extern FILE *qemu_logfile;
@@ -102,6 +100,9 @@ static inline void log_cpu_state_mask(int mask, CPUState 
*cpu, int flags)
 }

 #ifdef NEED_CPU_H
+#include "cpu.h" /* target-xxx/cpu.h, required for target_ulong,
+                    CPUArchState */
+
 /* disas() and target_disas() to qemu_logfile: */
 static inline void log_target_disas(CPUArchState *env, target_ulong start,
                                     target_ulong len, int flags)
@@ -121,7 +122,7 @@ static inline void log_page_dump(void)
     page_dump(qemu_logfile);
 }
 #endif
-#endif
+#endif /* NEED_CPU_H */


 /* Maintenance: */
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 112721d..d1e5389 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -58,7 +58,7 @@ 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
+#if defined CONFIG_KVM
 #define kvm_enabled()           (kvm_allowed)

This is wrong. There are files that do not need cpu.h and need kvm_enabled().

Paolo

 /**
  * kvm_irqchip_in_kernel:
diff --git a/ui/input.c b/ui/input.c
index 1c70f60..b6d216d 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -187,7 +187,6 @@ static const int key_defs[] = {

     [Q_KEY_CODE_INSERT] = 0xd2,
     [Q_KEY_CODE_DELETE] = 0xd3,
-#ifdef NEED_CPU_H
 #if defined(TARGET_SPARC) && !defined(TARGET_SPARC64)
     [Q_KEY_CODE_STOP] = 0xf0,
     [Q_KEY_CODE_AGAIN] = 0xf1,
@@ -205,7 +204,6 @@ static const int key_defs[] = {
     [Q_KEY_CODE_META_R] = 0xfd,
     [Q_KEY_CODE_COMPOSE] = 0xfe,
 #endif
-#endif
     [Q_KEY_CODE_MAX] = 0,
 };






reply via email to

[Prev in Thread] Current Thread [Next in Thread]