[Qemu-devel] [RFC PATCH 0/6] decouple board headers from cpu.h

[Qemu-devel] [RFC PATCH 0/6] decouple board headers from cpu.h

[Paolo Bonzini]

Hi,

a while ago I started removing cpu.h dependencies from several headers
in QEMU.  In particular, I wanted to put an end to qemu-common.h's
including cpu.h and vice versa.  This is useful for two reasons: 1)
because both headers bring in a lot of dependencies and thus very small
changes can cause complete recompilations of target-dependent code;
2) in order to remove indirect inclusions of other header files, which
often come from cpu.h itself.

These patches are perhaps the only somewhat controversial part.
Generally, we add opaque typedefs to typedefs.h in order to avoid
indirect header inclusions.  However, this might not be desirable
for target specific types such as FooCPU.  These types are used
mostly in headers for boards, and thus this series uses struct
explicitly in those headers.  Adding a typedef breaks on older
compilers that do not like redefinitions of typedefs.

Everything else is just extremely boring: code movement out of
our catch-all header files, or pushing #includes down to the
users.

Opinions?

Paolo

Paolo Bonzini (6):
  arm: use "struct ARMCPU" in header files
  mips: use struct CPUMIPSState to avoid need for cpu.h
  ppc: use struct to avoid the need for cpu.h
  alpha: use AlphaCPU as an opaque type
  coldfire: use "struct M68kCPU" in header files
  sh: use SuperHCPU struct to avoid cpu.h dependency

 hw/alpha/alpha_sys.h        |  3 ++-
 hw/arm/strongarm.h          |  2 +-
 include/hw/arm/arm.h        | 12 +++++++-----
 include/hw/arm/exynos4210.h |  5 +++--
 include/hw/arm/omap.h       |  4 +++-
 include/hw/arm/pxa.h        |  8 +++++---
 include/hw/m68k/mcf.h       |  5 +++--
 include/hw/mips/cpudevs.h   |  6 ++++--
 include/hw/ppc/ppc.h        | 36 +++++++++++++++++++++---------------
 include/hw/ppc/ppc4xx.h     | 17 +++++++++++------
 include/hw/sh4/sh.h         |  3 ++-
 linux-user/main.c           |  4 ++--
 target-ppc/cpu.h            | 10 ++++------
 13 files changed, 68 insertions(+), 47 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/6] arm: use "struct ARMCPU" in header files

[Paolo Bonzini]

This removes a dependency on cpu.h.  Since this is in most cases the
_only_ dependency on cpu.h, removing it now makes it easier to
disentangle qemu-common.h and cpu.h.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/arm/strongarm.h          |  2 +-
 include/hw/arm/arm.h        | 12 +++++++-----
 include/hw/arm/exynos4210.h |  5 +++--
 include/hw/arm/omap.h       |  4 +++-
 include/hw/arm/pxa.h        |  8 +++++---
 5 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/hw/arm/strongarm.h b/hw/arm/strongarm.h
index 2893f94..d87e41d 100644
--- a/hw/arm/strongarm.h
+++ b/hw/arm/strongarm.h
@@ -53,7 +53,7 @@ enum {
 };
 
 typedef struct {
-    ARMCPU *cpu;
+    struct ARMCPU *cpu;
     MemoryRegion sdram;
     DeviceState *pic;
     DeviceState *gpio;
diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
index c26b0e3..0f9d20f 100644
--- a/include/hw/arm/arm.h
+++ b/include/hw/arm/arm.h
@@ -16,6 +16,8 @@
 #include "qemu/notify.h"
 #include "cpu.h"
 
+struct ARMCPU;
+
 /* armv7m.c */
 DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
                       const char *kernel_filename, const char *cpu_model);
@@ -26,7 +28,7 @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
  */
 typedef struct {
     Notifier notifier; /* actual notifier */
-    ARMCPU *cpu; /* handle to the first cpu object */
+    struct ARMCPU *cpu; /* handle to the first cpu object */
 } ArmLoadKernelNotifier;
 
 /* arm_boot.c */
@@ -60,9 +62,9 @@ struct arm_boot_info {
      * perform any necessary CPU reset handling and set the PC for the
      * secondary CPUs to point at this boot blob.
      */
-    void (*write_secondary_boot)(ARMCPU *cpu,
+    void (*write_secondary_boot)(struct ARMCPU *cpu,
                                  const struct arm_boot_info *info);
-    void (*secondary_cpu_reset_hook)(ARMCPU *cpu,
+    void (*secondary_cpu_reset_hook)(struct ARMCPU *cpu,
                                      const struct arm_boot_info *info);
     /* if a board is able to create a dtb without a dtb file then it
      * sets get_dtb. This will only be used if no dtb file is provided
@@ -95,7 +97,7 @@ struct arm_boot_info {
      * to the specified address.
      */
     hwaddr board_setup_addr;
-    void (*write_board_setup)(ARMCPU *cpu,
+    void (*write_board_setup)(struct ARMCPU *cpu,
                               const struct arm_boot_info *info);
 
     /* If set, the board specific loader/setup blob will be run from secure
@@ -120,7 +122,7 @@ struct arm_boot_info {
  * before sysbus-fdt arm_register_platform_bus_fdt_creator. Indeed the
  * machine init done notifiers are called in registration reverse order.
  */
-void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info);
+void arm_load_kernel(struct ARMCPU *cpu, struct arm_boot_info *info);
 
 /* Multiplication factor to convert from system clock ticks to qemu timer
    ticks.  */
diff --git a/include/hw/arm/exynos4210.h b/include/hw/arm/exynos4210.h
index 5c1820f..a43b05e 100644
--- a/include/hw/arm/exynos4210.h
+++ b/include/hw/arm/exynos4210.h
@@ -28,6 +28,7 @@
 
 #include "qemu-common.h"
 #include "exec/memory.h"
+#include "hw/arm/arm.h"
 
 #define EXYNOS4210_NCPUS                    2
 
@@ -85,7 +86,7 @@ typedef struct Exynos4210Irq {
 } Exynos4210Irq;
 
 typedef struct Exynos4210State {
-    ARMCPU *cpu[EXYNOS4210_NCPUS];
+    struct ARMCPU *cpu[EXYNOS4210_NCPUS];
     Exynos4210Irq irqs;
     qemu_irq *irq_table;
 
@@ -100,7 +101,7 @@ typedef struct Exynos4210State {
     I2CBus *i2c_if[EXYNOS4210_I2C_NUMBER];
 } Exynos4210State;
 
-void exynos4210_write_secondary(ARMCPU *cpu,
+void exynos4210_write_secondary(struct ARMCPU *cpu,
         const struct arm_boot_info *info);
 
 Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
diff --git a/include/hw/arm/omap.h b/include/hw/arm/omap.h
index 0ad5fb8..35035f4 100644
--- a/include/hw/arm/omap.h
+++ b/include/hw/arm/omap.h
@@ -790,6 +790,8 @@ I2CBus *omap_i2c_bus(DeviceState *omap_i2c);
 # define cpu_class_omap3(cpu) \
         (cpu_is_omap3430(cpu) || cpu_is_omap3630(cpu))
 
+struct ARMCPU;
+
 struct omap_mpu_state_s {
     enum omap_mpu_model {
         omap310,
@@ -805,7 +807,7 @@ struct omap_mpu_state_s {
         omap3630,
     } mpu_model;
 
-    ARMCPU *cpu;
+    struct ARMCPU *cpu;
 
     qemu_irq *drq;
 
diff --git a/include/hw/arm/pxa.h b/include/hw/arm/pxa.h
index 259b852..224b129 100644
--- a/include/hw/arm/pxa.h
+++ b/include/hw/arm/pxa.h
@@ -64,12 +64,14 @@
 # define PXA2XX_INTERNAL_BASE	0x5c000000
 # define PXA2XX_INTERNAL_SIZE	0x40000
 
+struct ARMCPU;
+
 /* pxa2xx_pic.c */
-DeviceState *pxa2xx_pic_init(hwaddr base, ARMCPU *cpu);
+DeviceState *pxa2xx_pic_init(hwaddr base, struct ARMCPU *cpu);
 
 /* pxa2xx_gpio.c */
 DeviceState *pxa2xx_gpio_init(hwaddr base,
-                              ARMCPU *cpu, DeviceState *pic, int lines);
+                              struct ARMCPU *cpu, DeviceState *pic, int lines);
 void pxa2xx_gpio_read_notifier(DeviceState *dev, qemu_irq handler);
 
 /* pxa2xx_dma.c */
@@ -122,7 +124,7 @@ typedef struct PXA2xxI2SState PXA2xxI2SState;
 typedef struct PXA2xxFIrState PXA2xxFIrState;
 
 typedef struct {
-    ARMCPU *cpu;
+    struct ARMCPU *cpu;
     DeviceState *pic;
     qemu_irq reset;
     MemoryRegion sdram;
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/6] mips: use struct CPUMIPSState to avoid need for cpu.h

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/hw/mips/cpudevs.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/hw/mips/cpudevs.h b/include/hw/mips/cpudevs.h
index b2626f2..eb2f190 100644
--- a/include/hw/mips/cpudevs.h
+++ b/include/hw/mips/cpudevs.h
@@ -2,6 +2,8 @@
 #define HW_MIPS_CPUDEVS_H
 /* Definitions for MIPS CPU internal devices.  */
 
+struct CPUMIPSState;
+
 /* mips_addr.c */
 uint64_t cpu_mips_kseg0_to_phys(void *opaque, uint64_t addr);
 uint64_t cpu_mips_phys_to_kseg0(void *opaque, uint64_t addr);
@@ -9,9 +11,9 @@ uint64_t cpu_mips_kvm_um_phys_to_kseg0(void *opaque, uint64_t addr);
 
 
 /* mips_int.c */
-void cpu_mips_irq_init_cpu(CPUMIPSState *env);
+void cpu_mips_irq_init_cpu(struct CPUMIPSState *env);
 
 /* mips_timer.c */
-void cpu_mips_clock_init(CPUMIPSState *);
+void cpu_mips_clock_init(struct CPUMIPSState *);
 
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/6] ppc: use struct to avoid the need for cpu.h

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/hw/ppc/ppc.h    | 36 +++++++++++++++++++++---------------
 include/hw/ppc/ppc4xx.h | 17 +++++++++++------
 linux-user/main.c       |  4 ++--
 target-ppc/cpu.h        | 10 ++++------
 4 files changed, 38 insertions(+), 29 deletions(-)

diff --git a/include/hw/ppc/ppc.h b/include/hw/ppc/ppc.h
index 14efd0c..9eaa410 100644
--- a/include/hw/ppc/ppc.h
+++ b/include/hw/ppc/ppc.h
@@ -1,8 +1,6 @@
 #ifndef HW_PPC_H
 #define HW_PPC_H 1
 
-void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level);
-
 /* PowerPC hardware exceptions management helpers */
 typedef void (*clk_setup_cb)(void *opaque, uint32_t freq);
 typedef struct clk_setup_t clk_setup_t;
@@ -16,6 +14,9 @@ static inline void clk_setup (clk_setup_t *clk, uint32_t freq)
         (*clk->cb)(clk->opaque, freq);
 }
 
+typedef struct ppc_tb_t ppc_tb_t;
+typedef struct ppc_dcr_t ppc_dcr_t;
+
 struct ppc_tb_t {
     /* Time base management */
     int64_t  tb_offset;    /* Compensation                    */
@@ -48,33 +49,38 @@ struct ppc_tb_t {
                                                * the most significant bit is 1.
                                                */
 
+struct CPUPPCState;
+struct PowerPCCPU;
+
 uint64_t cpu_ppc_get_tb(ppc_tb_t *tb_env, uint64_t vmclk, int64_t tb_offset);
-clk_setup_cb cpu_ppc_tb_init (CPUPPCState *env, uint32_t freq);
+clk_setup_cb cpu_ppc_tb_init (struct CPUPPCState *env, uint32_t freq);
 /* Embedded PowerPC DCR management */
 typedef uint32_t (*dcr_read_cb)(void *opaque, int dcrn);
 typedef void (*dcr_write_cb)(void *opaque, int dcrn, uint32_t val);
-int ppc_dcr_init (CPUPPCState *env, int (*dcr_read_error)(int dcrn),
+int ppc_dcr_init (struct CPUPPCState *env, int (*dcr_read_error)(int dcrn),
                   int (*dcr_write_error)(int dcrn));
-int ppc_dcr_register (CPUPPCState *env, int dcrn, void *opaque,
+int ppc_dcr_register (struct CPUPPCState *env, int dcrn, void *opaque,
                       dcr_read_cb drc_read, dcr_write_cb dcr_write);
-clk_setup_cb ppc_40x_timers_init (CPUPPCState *env, uint32_t freq,
+clk_setup_cb ppc_40x_timers_init (struct CPUPPCState *env, uint32_t freq,
                                   unsigned int decr_excp);
 
 /* Embedded PowerPC reset */
-void ppc40x_core_reset(PowerPCCPU *cpu);
-void ppc40x_chip_reset(PowerPCCPU *cpu);
-void ppc40x_system_reset(PowerPCCPU *cpu);
+void ppc40x_core_reset(struct PowerPCCPU *cpu);
+void ppc40x_chip_reset(struct PowerPCCPU *cpu);
+void ppc40x_system_reset(struct PowerPCCPU *cpu);
 void PREP_debug_write (void *opaque, uint32_t addr, uint32_t val);
 
 extern CPUWriteMemoryFunc * const PPC_io_write[];
 extern CPUReadMemoryFunc * const PPC_io_read[];
 void PPC_debug_write (void *opaque, uint32_t addr, uint32_t val);
 
-void ppc40x_irq_init (CPUPPCState *env);
-void ppce500_irq_init (CPUPPCState *env);
-void ppc6xx_irq_init (CPUPPCState *env);
-void ppc970_irq_init (CPUPPCState *env);
-void ppcPOWER7_irq_init (CPUPPCState *env);
+void ppc_set_irq(struct PowerPCCPU *cpu, int n_IRQ, int level);
+
+void ppc40x_irq_init (struct CPUPPCState *env);
+void ppce500_irq_init (struct CPUPPCState *env);
+void ppc6xx_irq_init (struct CPUPPCState *env);
+void ppc970_irq_init (struct CPUPPCState *env);
+void ppcPOWER7_irq_init (struct CPUPPCState *env);
 
 /* PPC machines for OpenBIOS */
 enum {
@@ -98,6 +104,6 @@ enum {
 #define PPC_SERIAL_MM_BAUDBASE 399193
 
 /* ppc_booke.c */
-void ppc_booke_timers_init(PowerPCCPU *cpu, uint32_t freq, uint32_t flags);
+void ppc_booke_timers_init(struct PowerPCCPU *cpu, uint32_t freq, uint32_t flags);
 
 #endif
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index 91d84ba..f85e183 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -27,10 +27,12 @@
 
 #include "hw/pci/pci.h"
 
+struct PowerPCCPU;
+
 /* PowerPC 4xx core initialization */
-PowerPCCPU *ppc4xx_init(const char *cpu_model,
-                        clk_setup_t *cpu_clk, clk_setup_t *tb_clk,
-                        uint32_t sysclk);
+struct PowerPCCPU *ppc4xx_init(const char *cpu_model,
+                               clk_setup_t *cpu_clk, clk_setup_t *tb_clk,
+                               uint32_t sysclk);
 
 /* PowerPC 4xx universal interrupt controller */
 enum {
@@ -38,7 +40,10 @@ enum {
     PPCUIC_OUTPUT_CINT = 1,
     PPCUIC_OUTPUT_NB,
 };
-qemu_irq *ppcuic_init (CPUPPCState *env, qemu_irq *irqs,
+
+struct CPUPPCState;
+
+qemu_irq *ppcuic_init (struct CPUPPCState *env, qemu_irq *irqs,
                        uint32_t dcr_base, int has_ssr, int has_vr);
 
 ram_addr_t ppc4xx_sdram_adjust(ram_addr_t ram_size, int nr_banks,
@@ -47,7 +52,7 @@ ram_addr_t ppc4xx_sdram_adjust(ram_addr_t ram_size, int nr_banks,
                                hwaddr ram_sizes[],
                                const unsigned int sdram_bank_sizes[]);
 
-void ppc4xx_sdram_init (CPUPPCState *env, qemu_irq irq, int nbanks,
+void ppc4xx_sdram_init (struct CPUPPCState *env, qemu_irq irq, int nbanks,
                         MemoryRegion ram_memories[],
                         hwaddr *ram_bases,
                         hwaddr *ram_sizes,
@@ -55,7 +60,7 @@ void ppc4xx_sdram_init (CPUPPCState *env, qemu_irq irq, int nbanks,
 
 #define TYPE_PPC4xx_PCI_HOST_BRIDGE "ppc4xx-pcihost"
 
-PCIBus *ppc4xx_pci_init(CPUPPCState *env, qemu_irq pci_irqs[4],
+PCIBus *ppc4xx_pci_init(struct CPUPPCState *env, qemu_irq pci_irqs[4],
                         hwaddr config_space,
                         hwaddr int_ack,
                         hwaddr special_cycle,
diff --git a/linux-user/main.c b/linux-user/main.c
index 35b021a..2d65f3f 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -1458,12 +1458,12 @@ uint32_t cpu_ppc601_load_rtcl(CPUPPCState *env)
 }
 
 /* XXX: to be fixed */
-int ppc_dcr_read (ppc_dcr_t *dcr_env, int dcrn, uint32_t *valp)
+int ppc_dcr_read (struct ppc_dcr_t *dcr_env, int dcrn, uint32_t *valp)
 {
     return -1;
 }
 
-int ppc_dcr_write (ppc_dcr_t *dcr_env, int dcrn, uint32_t val)
+int ppc_dcr_write (struct ppc_dcr_t *dcr_env, int dcrn, uint32_t val)
 {
     return -1;
 }
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 9706000..dd877e6 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -326,9 +326,7 @@ typedef struct opc_handler_t opc_handler_t;
 /* Types used to describe some PowerPC registers */
 typedef struct CPUPPCState CPUPPCState;
 typedef struct DisasContext DisasContext;
-typedef struct ppc_tb_t ppc_tb_t;
 typedef struct ppc_spr_t ppc_spr_t;
-typedef struct ppc_dcr_t ppc_dcr_t;
 typedef union ppc_avr_t ppc_avr_t;
 typedef union ppc_tlb_t ppc_tlb_t;
 
@@ -1081,9 +1079,9 @@ struct CPUPPCState {
 
     /* Internal devices resources */
     /* Time base and decrementer */
-    ppc_tb_t *tb_env;
+    struct ppc_tb_t *tb_env;
     /* Device control registers */
-    ppc_dcr_t *dcr_env;
+    struct ppc_dcr_t *dcr_env;
 
     int dcache_line_size;
     int icache_line_size;
@@ -1263,8 +1261,8 @@ static inline uint64_t ppc_dump_gpr(CPUPPCState *env, int gprn)
 }
 
 /* Device control registers */
-int ppc_dcr_read (ppc_dcr_t *dcr_env, int dcrn, uint32_t *valp);
-int ppc_dcr_write (ppc_dcr_t *dcr_env, int dcrn, uint32_t val);
+int ppc_dcr_read (struct ppc_dcr_t *dcr_env, int dcrn, uint32_t *valp);
+int ppc_dcr_write (struct ppc_dcr_t *dcr_env, int dcrn, uint32_t val);
 
 #define cpu_init(cpu_model) CPU(cpu_ppc_init(cpu_model))
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/6] alpha: use AlphaCPU as an opaque type

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/alpha/alpha_sys.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/alpha/alpha_sys.h b/hw/alpha/alpha_sys.h
index e11025b..8f67ed9 100644
--- a/hw/alpha/alpha_sys.h
+++ b/hw/alpha/alpha_sys.h
@@ -10,7 +10,8 @@
 #include "hw/irq.h"
 
 
-PCIBus *typhoon_init(ram_addr_t, ISABus **, qemu_irq *, AlphaCPU *[4],
+struct AlphaCPU;
+PCIBus *typhoon_init(ram_addr_t, ISABus **, qemu_irq *, struct AlphaCPU *[4],
                      pci_map_irq_fn);
 
 /* alpha_pci.c.  */
-- 
1.8.3.1

[Qemu-devel] [PATCH 5/6] coldfire: use "struct M68kCPU" in header files

[Paolo Bonzini]

This removes a dependency on cpu.h.  Since all dependencies on cpu.h
come from mcf.h, using "struct M68kCPU" makes it easier to disentangle
qemu-common.h and cpu.h.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/hw/m68k/mcf.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/hw/m68k/mcf.h b/include/hw/m68k/mcf.h
index fbc8dc2..1c7d2d6 100644
--- a/include/hw/m68k/mcf.h
+++ b/include/hw/m68k/mcf.h
@@ -3,6 +3,7 @@
 /* Motorola ColdFire device prototypes.  */
 
 struct MemoryRegion;
+struct M68kCPU;
 
 /* mcf_uart.c */
 uint64_t mcf_uart_read(void *opaque, hwaddr addr,
@@ -17,7 +18,7 @@ void mcf_uart_mm_init(struct MemoryRegion *sysmem,
 /* mcf_intc.c */
 qemu_irq *mcf_intc_init(struct MemoryRegion *sysmem,
                         hwaddr base,
-                        M68kCPU *cpu);
+                        struct M68kCPU *cpu);
 
 /* mcf_fec.c */
 void mcf_fec_init(struct MemoryRegion *sysmem, NICInfo *nd,
@@ -25,6 +26,6 @@ void mcf_fec_init(struct MemoryRegion *sysmem, NICInfo *nd,
 
 /* mcf5206.c */
 qemu_irq *mcf5206_init(struct MemoryRegion *sysmem,
-                       uint32_t base, M68kCPU *cpu);
+                       uint32_t base, struct M68kCPU *cpu);
 
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH 6/6] sh: use SuperHCPU struct to avoid cpu.h dependency

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/hw/sh4/sh.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/sh4/sh.h b/include/hw/sh4/sh.h
index e61de9a..32f688c 100644
--- a/include/hw/sh4/sh.h
+++ b/include/hw/sh4/sh.h
@@ -8,10 +8,11 @@
 #define P4ADDR(x) ((x) | 0xe0000000)
 
 /* sh7750.c */
+struct SuperHCPU;
 struct SH7750State;
 struct MemoryRegion;
 
-struct SH7750State *sh7750_init(SuperHCPU *cpu, struct MemoryRegion *sysmem);
+struct SH7750State *sh7750_init(struct SuperHCPU *cpu, struct MemoryRegion *sysmem);
 
 typedef struct {
     /* The callback will be triggered if any of the designated lines change */
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 1/6] arm: use "struct ARMCPU" in header files

[Peter Maydell]

On 15 December 2015 at 14:28, Paolo Bonzini <pbonzini@redhat.com> wrote:
> This removes a dependency on cpu.h.  Since this is in most cases the
> _only_ dependency on cpu.h, removing it now makes it easier to
> disentangle qemu-common.h and cpu.h.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/arm/strongarm.h          |  2 +-
>  include/hw/arm/arm.h        | 12 +++++++-----
>  include/hw/arm/exynos4210.h |  5 +++--
>  include/hw/arm/omap.h       |  4 +++-
>  include/hw/arm/pxa.h        |  8 +++++---
>  5 files changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/hw/arm/strongarm.h b/hw/arm/strongarm.h
> index 2893f94..d87e41d 100644
> --- a/hw/arm/strongarm.h
> +++ b/hw/arm/strongarm.h
> @@ -53,7 +53,7 @@ enum {
>  };
>
>  typedef struct {
> -    ARMCPU *cpu;
> +    struct ARMCPU *cpu;


Couldn't we just put the typedef in typedefs.h instead ?

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH 0/6] decouple board headers from cpu.h

[Peter Maydell]

On 15 December 2015 at 14:28, Paolo Bonzini <pbonzini@redhat.com> wrote:
> These patches are perhaps the only somewhat controversial part.
> Generally, we add opaque typedefs to typedefs.h in order to avoid
> indirect header inclusions.  However, this might not be desirable
> for target specific types such as FooCPU.  These types are used
> mostly in headers for boards, and thus this series uses struct
> explicitly in those headers.  Adding a typedef breaks on older
> compilers that do not like redefinitions of typedefs.

I would prefer us to provide the typedef. There's no problem
with older compilers because you just only define the typedef
in one place (typically in typedefs.h).

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH 0/6] decouple board headers from cpu.h

[Paolo Bonzini]

On 15/12/2015 17:06, Peter Maydell wrote:
> > These patches are perhaps the only somewhat controversial part.
> > Generally, we add opaque typedefs to typedefs.h in order to avoid
> > indirect header inclusions.  However, this might not be desirable
> > for target specific types such as FooCPU.  These types are used
> > mostly in headers for boards, and thus this series uses struct
> > explicitly in those headers.  Adding a typedef breaks on older
> > compilers that do not like redefinitions of typedefs.
>
> I would prefer us to provide the typedef. There's no problem
> with older compilers because you just only define the typedef
> in one place (typically in typedefs.h).

typedefs.h generally has target-independent types only (the only
exception is AllwinnerAHCIState; plus FWCfg* and uWireSlave are close
misses).  I thought about adding target-arm/typedefs.h or
hw/arm/typedefs.h, but it seemed weird to do that for one type only.

But yes, we can do that too.  I went this way first just to check how
many files were affected.

Paolo

Re: [Qemu-devel] [PATCH 1/6] arm: use "struct ARMCPU" in header files

[Paolo Bonzini]

On 15/12/2015 17:05, Peter Maydell wrote:
>> >
>> >  typedef struct {
>> > -    ARMCPU *cpu;
>> > +    struct ARMCPU *cpu;
> 
> Couldn't we just put the typedef in typedefs.h instead ?

Even better: we can put the typedef in cpu-qom.h and move the struct
definition to cpu.h.

Paolo