thr3ads.net - Xen devel - [PATCH 0/5 v2] cpu: make a child of DeviceState [Aug 2012]

If this information is useful, please help other people find it:
Share via:

Igor Mammedov

2012-Aug-19 23:39 UTC

[PATCH 0/5 v2] cpu: make a child of DeviceState

this is th 3rd approach to make CPU a child of DeviceState
for both kinds of targets *-user and *-softmmu. It seems
with current state of qemu it doesn''t take too much effort
to make it compile. Please check if it doesn''t break
something on other targets/archs/hosts than i386.

what''s tested:
  - compile tested building all targets on FC17x64 host.
  - briefly tested i386 user and softmmu targets

Anthony Liguori (1):
  qdev: split up header so it can be used in cpu.h

Igor Mammedov (4):
  move qemu_irq typedef out of cpu-common.h
  qapi-types.h doesn''t really need to include qemu-common.h
  cleanup error.h, included qapi-types.h aready has stdbool.h
  make CPU a child of DeviceState

 error.h               |    1 -
 hw/arm-misc.h         |    1 +
 hw/bt.h               |    2 +
 hw/devices.h          |    2 +
 hw/irq.h              |    2 +
 hw/mc146818rtc.c      |    1 +
 hw/omap.h             |    1 +
 hw/qdev-addr.c        |    1 +
 hw/qdev-core.h        |  240 ++++++++++++++++++++++++++++++++
 hw/qdev-monitor.h     |   16 ++
 hw/qdev-properties.c  |    1 +
 hw/qdev-properties.h  |  128 +++++++++++++++++
 hw/qdev.c             |    1 +
 hw/qdev.h             |  371 +------------------------------------------------
 hw/soc_dma.h          |    1 +
 hw/xen.h              |    1 +
 include/qemu/cpu.h    |    6 +-
 qemu-common.h         |    1 -
 scripts/qapi-types.py |    2 +-
 sysemu.h              |    1 +
 20 files changed, 407 insertions(+), 373 deletions(-)
 create mode 100644 hw/qdev-core.h
 create mode 100644 hw/qdev-monitor.h
 create mode 100644 hw/qdev-properties.h

Igor Mammedov

2012-Aug-19 23:39 UTC

head link

[PATCH 1/5] move qemu_irq typedef out of cpu-common.h

it''s necessary for making CPU child of DEVICE without
causing circular header deps.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/arm-misc.h |    1 +
 hw/bt.h       |    2 ++
 hw/devices.h  |    2 ++
 hw/irq.h      |    2 ++
 hw/omap.h     |    1 +
 hw/soc_dma.h  |    1 +
 hw/xen.h      |    1 +
 qemu-common.h |    1 -
 sysemu.h      |    1 +
 9 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/hw/arm-misc.h b/hw/arm-misc.h
index bdd8fec..b13aa59 100644
--- a/hw/arm-misc.h
+++ b/hw/arm-misc.h
@@ -12,6 +12,7 @@
 #define ARM_MISC_H 1
 
 #include "memory.h"
+#include "hw/irq.h"
 
 /* The CPU is also modeled as an interrupt controller.  */
 #define ARM_PIC_CPU_IRQ 0
diff --git a/hw/bt.h b/hw/bt.h
index a48b8d4..ebf6a37 100644
--- a/hw/bt.h
+++ b/hw/bt.h
@@ -23,6 +23,8 @@
  * along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "hw/irq.h"
+
 /* BD Address */
 typedef struct {
     uint8_t b[6];
diff --git a/hw/devices.h b/hw/devices.h
index 1a55c1e..c60bcab 100644
--- a/hw/devices.h
+++ b/hw/devices.h
@@ -1,6 +1,8 @@
 #ifndef QEMU_DEVICES_H
 #define QEMU_DEVICES_H
 
+#include "hw/irq.h"
+
 /* ??? Not all users of this file can include cpu-common.h.  */
 struct MemoryRegion;
 
diff --git a/hw/irq.h b/hw/irq.h
index 56c55f0..1339a3a 100644
--- a/hw/irq.h
+++ b/hw/irq.h
@@ -3,6 +3,8 @@
 
 /* Generic IRQ/GPIO pin infrastructure.  */
 
+typedef struct IRQState *qemu_irq;
+
 typedef void (*qemu_irq_handler)(void *opaque, int n, int level);
 
 void qemu_set_irq(qemu_irq irq, int level);
diff --git a/hw/omap.h b/hw/omap.h
index 413851b..8b08462 100644
--- a/hw/omap.h
+++ b/hw/omap.h
@@ -19,6 +19,7 @@
 #ifndef hw_omap_h
 #include "memory.h"
 # define hw_omap_h		"omap.h"
+#include "hw/irq.h"
 
 # define OMAP_EMIFS_BASE	0x00000000
 # define OMAP2_Q0_BASE		0x00000000
diff --git a/hw/soc_dma.h b/hw/soc_dma.h
index 904b26c..e386ace 100644
--- a/hw/soc_dma.h
+++ b/hw/soc_dma.h
@@ -19,6 +19,7 @@
  */
 
 #include "memory.h"
+#include "hw/irq.h"
 
 struct soc_dma_s;
 struct soc_dma_ch_s;
diff --git a/hw/xen.h b/hw/xen.h
index e5926b7..ff11dfd 100644
--- a/hw/xen.h
+++ b/hw/xen.h
@@ -8,6 +8,7 @@
  */
 #include <inttypes.h>
 
+#include "hw/irq.h"
 #include "qemu-common.h"
 
 /* xen-machine.c */
diff --git a/qemu-common.h b/qemu-common.h
index e5c2bcd..6677a30 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -273,7 +273,6 @@ typedef struct PCIEPort PCIEPort;
 typedef struct PCIESlot PCIESlot;
 typedef struct MSIMessage MSIMessage;
 typedef struct SerialState SerialState;
-typedef struct IRQState *qemu_irq;
 typedef struct PCMCIACardState PCMCIACardState;
 typedef struct MouseTransformInfo MouseTransformInfo;
 typedef struct uWireSlave uWireSlave;
diff --git a/sysemu.h b/sysemu.h
index 65552ac..f765821 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -9,6 +9,7 @@
 #include "qapi-types.h"
 #include "notify.h"
 #include "main-loop.h"
+#include "hw/irq.h"
 
 /* vl.c */
 
-- 
1.7.1

Igor Mammedov

2012-Aug-19 23:39 UTC

head link

[PATCH 2/5] qdev: split up header so it can be used in cpu.h

From: Anthony Liguori <aliguori@us.ibm.com>

Header file dependency is a frickin'' nightmare right now.  cpu.h tends
to get
included in our ''include everything'' header files but qdev
also needs to include
those headers mainly for qdev-properties since it knows about CharDriverState
and friends.

We can solve this for now by splitting out qdev.h along the same lines that we
previously split the C file.  Then cpu.h just needs to include qdev-core.h

v1->v2:
  move qemu_irq typedef out of this patch into a separate one with an additional
  cleanup of headers to fix build breakage

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/mc146818rtc.c     |    1 +
 hw/qdev-addr.c       |    1 +
 hw/qdev-core.h       |  240 ++++++++++++++++++++++++++++++++
 hw/qdev-monitor.h    |   16 ++
 hw/qdev-properties.c |    1 +
 hw/qdev-properties.h |  128 +++++++++++++++++
 hw/qdev.c            |    1 +
 hw/qdev.h            |  371 +-------------------------------------------------
 8 files changed, 392 insertions(+), 367 deletions(-)
 create mode 100644 hw/qdev-core.h
 create mode 100644 hw/qdev-monitor.h
 create mode 100644 hw/qdev-properties.h

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 3777f85..3780617 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -25,6 +25,7 @@
 #include "qemu-timer.h"
 #include "sysemu.h"
 #include "mc146818rtc.h"
+#include "qapi/qapi-visit-core.h"
 
 #ifdef TARGET_I386
 #include "apic.h"
diff --git a/hw/qdev-addr.c b/hw/qdev-addr.c
index b711b6b..5b5d38f 100644
--- a/hw/qdev-addr.c
+++ b/hw/qdev-addr.c
@@ -1,6 +1,7 @@
 #include "qdev.h"
 #include "qdev-addr.h"
 #include "targphys.h"
+#include "qapi/qapi-visit-core.h"
 
 /* --- target physical address --- */
 
diff --git a/hw/qdev-core.h b/hw/qdev-core.h
new file mode 100644
index 0000000..ca205fc
--- /dev/null
+++ b/hw/qdev-core.h
@@ -0,0 +1,240 @@
+#ifndef QDEV_CORE_H
+#define QDEV_CORE_H
+
+#include "qemu-queue.h"
+#include "qemu-option.h"
+#include "qemu/object.h"
+#include "hw/irq.h"
+#include "error.h"
+
+typedef struct Property Property;
+
+typedef struct PropertyInfo PropertyInfo;
+
+typedef struct CompatProperty CompatProperty;
+
+typedef struct BusState BusState;
+
+typedef struct BusClass BusClass;
+
+enum DevState {
+    DEV_STATE_CREATED = 1,
+    DEV_STATE_INITIALIZED,
+};
+
+enum {
+    DEV_NVECTORS_UNSPECIFIED = -1,
+};
+
+#define TYPE_DEVICE "device"
+#define DEVICE(obj) OBJECT_CHECK(DeviceState, (obj), TYPE_DEVICE)
+#define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass),
TYPE_DEVICE)
+#define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
+
+typedef int (*qdev_initfn)(DeviceState *dev);
+typedef int (*qdev_event)(DeviceState *dev);
+typedef void (*qdev_resetfn)(DeviceState *dev);
+
+struct VMStateDescription;
+
+typedef struct DeviceClass {
+    ObjectClass parent_class;
+
+    const char *fw_name;
+    const char *desc;
+    Property *props;
+    int no_user;
+
+    /* callbacks */
+    void (*reset)(DeviceState *dev);
+
+    /* device state */
+    const struct VMStateDescription *vmsd;
+
+    /* Private to qdev / bus.  */
+    qdev_initfn init;
+    qdev_event unplug;
+    qdev_event exit;
+    const char *bus_type;
+} DeviceClass;
+
+/* This structure should not be accessed directly.  We declare it here
+   so that it can be embedded in individual device state structures.  */
+struct DeviceState {
+    Object parent_obj;
+
+    const char *id;
+    enum DevState state;
+    struct QemuOpts *opts;
+    int hotplugged;
+    BusState *parent_bus;
+    int num_gpio_out;
+    qemu_irq *gpio_out;
+    int num_gpio_in;
+    qemu_irq *gpio_in;
+    QLIST_HEAD(, BusState) child_bus;
+    int num_child_bus;
+    int instance_id_alias;
+    int alias_required_for_version;
+};
+
+/*
+ * This callback is used to create Open Firmware device path in accordance with
+ * OF spec http://forthworks.com/standards/of1275.pdf. Indicidual bus bindings
+ * can be found here http://playground.sun.com/1275/bindings/.
+ */
+
+#define TYPE_BUS "bus"
+#define BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_BUS)
+#define BUS_CLASS(klass) OBJECT_CLASS_CHECK(BusClass, (klass), TYPE_BUS)
+#define BUS_GET_CLASS(obj) OBJECT_GET_CLASS(BusClass, (obj), TYPE_BUS)
+
+struct BusClass {
+    ObjectClass parent_class;
+
+    /* FIXME first arg should be BusState */
+    void (*print_dev)(Monitor *mon, DeviceState *dev, int indent);
+    char *(*get_dev_path)(DeviceState *dev);
+    char *(*get_fw_dev_path)(DeviceState *dev);
+    int (*reset)(BusState *bus);
+};
+
+typedef struct BusChild {
+    DeviceState *child;
+    int index;
+    QTAILQ_ENTRY(BusChild) sibling;
+} BusChild;
+
+/**
+ * BusState:
+ * @qom_allocated: Indicates whether the object was allocated by QOM.
+ * @glib_allocated: Indicates whether the object was initialized in-place
+ * yet is expected to be freed with g_free().
+ */
+struct BusState {
+    Object obj;
+    DeviceState *parent;
+    const char *name;
+    int allow_hotplug;
+    bool qom_allocated;
+    bool glib_allocated;
+    int max_index;
+    QTAILQ_HEAD(ChildrenHead, BusChild) children;
+    QLIST_ENTRY(BusState) sibling;
+};
+
+struct Property {
+    const char   *name;
+    PropertyInfo *info;
+    int          offset;
+    uint8_t      bitnr;
+    uint8_t      qtype;
+    int64_t      defval;
+};
+
+struct PropertyInfo {
+    const char *name;
+    const char *legacy_name;
+    const char **enum_table;
+    int (*parse)(DeviceState *dev, Property *prop, const char *str);
+    int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
+    ObjectPropertyAccessor *get;
+    ObjectPropertyAccessor *set;
+    ObjectPropertyRelease *release;
+};
+
+typedef struct GlobalProperty {
+    const char *driver;
+    const char *property;
+    const char *value;
+    QTAILQ_ENTRY(GlobalProperty) next;
+} GlobalProperty;
+
+/*** Board API.  This should go away once we have a machine config file.  ***/
+
+DeviceState *qdev_create(BusState *bus, const char *name);
+DeviceState *qdev_try_create(BusState *bus, const char *name);
+bool qdev_exists(const char *name);
+int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT;
+void qdev_init_nofail(DeviceState *dev);
+void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
+                                 int required_for_version);
+void qdev_unplug(DeviceState *dev, Error **errp);
+void qdev_free(DeviceState *dev);
+int qdev_simple_unplug_cb(DeviceState *dev);
+void qdev_machine_creation_done(void);
+bool qdev_machine_modified(void);
+
+qemu_irq qdev_get_gpio_in(DeviceState *dev, int n);
+void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
+
+BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
+
+/*** Device API.  ***/
+
+/* Register device properties.  */
+/* GPIO inputs also double as IRQ sinks.  */
+void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n);
+void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n);
+
+BusState *qdev_get_parent_bus(DeviceState *dev);
+
+/*** BUS API. ***/
+
+DeviceState *qdev_find_recursive(BusState *bus, const char *id);
+
+/* Returns 0 to walk children, > 0 to skip walk, < 0 to terminate walk.
*/
+typedef int (qbus_walkerfn)(BusState *bus, void *opaque);
+typedef int (qdev_walkerfn)(DeviceState *dev, void *opaque);
+
+void qbus_create_inplace(BusState *bus, const char *typename,
+                         DeviceState *parent, const char *name);
+BusState *qbus_create(const char *typename, DeviceState *parent, const char
*name);
+/* Returns > 0 if either devfn or busfn skip walk somewhere in cursion,
+ *         < 0 if either devfn or busfn terminate walk somewhere in cursion,
+ *           0 otherwise. */
+int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn,
+                       qbus_walkerfn *busfn, void *opaque);
+int qdev_walk_children(DeviceState *dev, qdev_walkerfn *devfn,
+                       qbus_walkerfn *busfn, void *opaque);
+void qdev_reset_all(DeviceState *dev);
+void qbus_reset_all_fn(void *opaque);
+
+void qbus_free(BusState *bus);
+
+#define FROM_QBUS(type, dev) DO_UPCAST(type, qbus, dev)
+
+/* This should go away once we get rid of the NULL bus hack */
+BusState *sysbus_get_default(void);
+
+char *qdev_get_fw_dev_path(DeviceState *dev);
+
+/**
+ * @qdev_machine_init
+ *
+ * Initialize platform devices before machine init.  This is a hack until full
+ * support for composition is added.
+ */
+void qdev_machine_init(void);
+
+/**
+ * @device_reset
+ *
+ * Reset a single device (by calling the reset method).
+ */
+void device_reset(DeviceState *dev);
+
+const struct VMStateDescription *qdev_get_vmsd(DeviceState *dev);
+
+const char *qdev_fw_name(DeviceState *dev);
+
+Object *qdev_get_machine(void);
+
+/* FIXME: make this a link<> */
+void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
+
+extern int qdev_hotplug;
+
+char *qdev_get_dev_path(DeviceState *dev);
+
+#endif
diff --git a/hw/qdev-monitor.h b/hw/qdev-monitor.h
new file mode 100644
index 0000000..220ceba
--- /dev/null
+++ b/hw/qdev-monitor.h
@@ -0,0 +1,16 @@
+#ifndef QEMU_QDEV_MONITOR_H
+#define QEMU_QDEV_MONITOR_H
+
+#include "qdev-core.h"
+#include "monitor.h"
+
+/*** monitor commands ***/
+
+void do_info_qtree(Monitor *mon);
+void do_info_qdm(Monitor *mon);
+int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
+int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
+int qdev_device_help(QemuOpts *opts);
+DeviceState *qdev_device_add(QemuOpts *opts);
+
+#endif
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 8aca0d4..81d901c 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -4,6 +4,7 @@
 #include "blockdev.h"
 #include "hw/block-common.h"
 #include "net/hub.h"
+#include "qapi/qapi-visit-core.h"
 
 void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
 {
diff --git a/hw/qdev-properties.h b/hw/qdev-properties.h
new file mode 100644
index 0000000..e93336a
--- /dev/null
+++ b/hw/qdev-properties.h
@@ -0,0 +1,128 @@
+#ifndef QEMU_QDEV_PROPERTIES_H
+#define QEMU_QDEV_PROPERTIES_H
+
+#include "qdev-core.h"
+
+/*** qdev-properties.c ***/
+
+extern PropertyInfo qdev_prop_bit;
+extern PropertyInfo qdev_prop_uint8;
+extern PropertyInfo qdev_prop_uint16;
+extern PropertyInfo qdev_prop_uint32;
+extern PropertyInfo qdev_prop_int32;
+extern PropertyInfo qdev_prop_uint64;
+extern PropertyInfo qdev_prop_hex8;
+extern PropertyInfo qdev_prop_hex32;
+extern PropertyInfo qdev_prop_hex64;
+extern PropertyInfo qdev_prop_string;
+extern PropertyInfo qdev_prop_chr;
+extern PropertyInfo qdev_prop_ptr;
+extern PropertyInfo qdev_prop_macaddr;
+extern PropertyInfo qdev_prop_losttickpolicy;
+extern PropertyInfo qdev_prop_bios_chs_trans;
+extern PropertyInfo qdev_prop_drive;
+extern PropertyInfo qdev_prop_netdev;
+extern PropertyInfo qdev_prop_vlan;
+extern PropertyInfo qdev_prop_pci_devfn;
+extern PropertyInfo qdev_prop_blocksize;
+
+#define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
+        .name      = (_name),                                    \
+        .info      = &(_prop),                                   \
+        .offset    = offsetof(_state, _field)                    \
+            + type_check(_type,typeof_field(_state, _field)),    \
+        }
+#define DEFINE_PROP_DEFAULT(_name, _state, _field, _defval, _prop, _type) { \
+        .name      = (_name),                                           \
+        .info      = &(_prop),                                          \
+        .offset    = offsetof(_state, _field)                           \
+            + type_check(_type,typeof_field(_state, _field)),           \
+        .qtype     = QTYPE_QINT,                                        \
+        .defval    = (_type)_defval,                                    \
+        }
+#define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) {  \
+        .name      = (_name),                                    \
+        .info      = &(qdev_prop_bit),                           \
+        .bitnr    = (_bit),                                      \
+        .offset    = offsetof(_state, _field)                    \
+            + type_check(uint32_t,typeof_field(_state, _field)), \
+        .qtype     = QTYPE_QBOOL,                                \
+        .defval    = (bool)_defval,                              \
+        }
+
+#define DEFINE_PROP_UINT8(_n, _s, _f, _d)                       \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
+#define DEFINE_PROP_UINT16(_n, _s, _f, _d)                      \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint16, uint16_t)
+#define DEFINE_PROP_UINT32(_n, _s, _f, _d)                      \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint32, uint32_t)
+#define DEFINE_PROP_INT32(_n, _s, _f, _d)                      \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_int32, int32_t)
+#define DEFINE_PROP_UINT64(_n, _s, _f, _d)                      \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint64, uint64_t)
+#define DEFINE_PROP_HEX8(_n, _s, _f, _d)                       \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex8, uint8_t)
+#define DEFINE_PROP_HEX32(_n, _s, _f, _d)                       \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex32, uint32_t)
+#define DEFINE_PROP_HEX64(_n, _s, _f, _d)                       \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex64, uint64_t)
+#define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d)                   \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_pci_devfn, int32_t)
+
+#define DEFINE_PROP_PTR(_n, _s, _f)             \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, void*)
+#define DEFINE_PROP_CHR(_n, _s, _f)             \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_chr, CharDriverState*)
+#define DEFINE_PROP_STRING(_n, _s, _f)             \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_string, char*)
+#define DEFINE_PROP_NETDEV(_n, _s, _f)             \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_netdev, NetClientState*)
+#define DEFINE_PROP_VLAN(_n, _s, _f)             \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_vlan, NetClientState*)
+#define DEFINE_PROP_DRIVE(_n, _s, _f) \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_drive, BlockDriverState *)
+#define DEFINE_PROP_MACADDR(_n, _s, _f)         \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_macaddr, MACAddr)
+#define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_losttickpolicy, \
+                        LostTickPolicy)
+#define DEFINE_PROP_BIOS_CHS_TRANS(_n, _s, _f, _d) \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_bios_chs_trans, int)
+#define DEFINE_PROP_BLOCKSIZE(_n, _s, _f, _d) \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t)
+
+#define DEFINE_PROP_END_OF_LIST()               \
+    {}
+
+/* Set properties between creation and init.  */
+void *qdev_get_prop_ptr(DeviceState *dev, Property *prop);
+int qdev_prop_parse(DeviceState *dev, const char *name, const char *value);
+void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value);
+void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value);
+void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value);
+void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value);
+void qdev_prop_set_int32(DeviceState *dev, const char *name, int32_t value);
+void qdev_prop_set_uint64(DeviceState *dev, const char *name, uint64_t value);
+void qdev_prop_set_string(DeviceState *dev, const char *name, const char
*value);
+void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState
*value);
+void qdev_prop_set_netdev(DeviceState *dev, const char *name, NetClientState
*value);
+void qdev_prop_set_vlan(DeviceState *dev, const char *name, NetClientState
*value);
+int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState
*value) QEMU_WARN_UNUSED_RESULT;
+void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name,
BlockDriverState *value);
+void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value);
+void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
+/* FIXME: Remove opaque pointer properties.  */
+void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value);
+
+void qdev_prop_register_global_list(GlobalProperty *props);
+void qdev_prop_set_globals(DeviceState *dev);
+void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
+                                    Property *prop, const char *value);
+
+/**
+ * @qdev_property_add_static - add a @Property to a device referencing a
+ * field in a struct.
+ */
+void qdev_property_add_static(DeviceState *dev, Property *prop, Error **errp);
+
+#endif
diff --git a/hw/qdev.c b/hw/qdev.c
index b5b74b9..36c3e4b 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -29,6 +29,7 @@
 #include "qdev.h"
 #include "sysemu.h"
 #include "error.h"
+#include "qapi/qapi-visit-core.h"
 
 int qdev_hotplug = 0;
 static bool qdev_hot_added = false;
diff --git a/hw/qdev.h b/hw/qdev.h
index d699194..365b8d6 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -1,372 +1,9 @@
 #ifndef QDEV_H
 #define QDEV_H
 
-#include "hw.h"
-#include "qemu-queue.h"
-#include "qemu-char.h"
-#include "qemu-option.h"
-#include "qapi/qapi-visit-core.h"
-#include "qemu/object.h"
-#include "error.h"
-
-typedef struct Property Property;
-
-typedef struct PropertyInfo PropertyInfo;
-
-typedef struct CompatProperty CompatProperty;
-
-typedef struct BusState BusState;
-
-typedef struct BusClass BusClass;
-
-enum DevState {
-    DEV_STATE_CREATED = 1,
-    DEV_STATE_INITIALIZED,
-};
-
-enum {
-    DEV_NVECTORS_UNSPECIFIED = -1,
-};
-
-#define TYPE_DEVICE "device"
-#define DEVICE(obj) OBJECT_CHECK(DeviceState, (obj), TYPE_DEVICE)
-#define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass),
TYPE_DEVICE)
-#define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
-
-typedef int (*qdev_initfn)(DeviceState *dev);
-typedef int (*qdev_event)(DeviceState *dev);
-typedef void (*qdev_resetfn)(DeviceState *dev);
-
-typedef struct DeviceClass {
-    ObjectClass parent_class;
-
-    const char *fw_name;
-    const char *desc;
-    Property *props;
-    int no_user;
-
-    /* callbacks */
-    void (*reset)(DeviceState *dev);
-
-    /* device state */
-    const VMStateDescription *vmsd;
-
-    /* Private to qdev / bus.  */
-    qdev_initfn init;
-    qdev_event unplug;
-    qdev_event exit;
-    const char *bus_type;
-} DeviceClass;
-
-/* This structure should not be accessed directly.  We declare it here
-   so that it can be embedded in individual device state structures.  */
-struct DeviceState {
-    Object parent_obj;
-
-    const char *id;
-    enum DevState state;
-    QemuOpts *opts;
-    int hotplugged;
-    BusState *parent_bus;
-    int num_gpio_out;
-    qemu_irq *gpio_out;
-    int num_gpio_in;
-    qemu_irq *gpio_in;
-    QLIST_HEAD(, BusState) child_bus;
-    int num_child_bus;
-    int instance_id_alias;
-    int alias_required_for_version;
-};
-
-#define TYPE_BUS "bus"
-#define BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_BUS)
-#define BUS_CLASS(klass) OBJECT_CLASS_CHECK(BusClass, (klass), TYPE_BUS)
-#define BUS_GET_CLASS(obj) OBJECT_GET_CLASS(BusClass, (obj), TYPE_BUS)
-
-struct BusClass {
-    ObjectClass parent_class;
-
-    /* FIXME first arg should be BusState */
-    void (*print_dev)(Monitor *mon, DeviceState *dev, int indent);
-    char *(*get_dev_path)(DeviceState *dev);
-    /*
-     * This callback is used to create Open Firmware device path in accordance
-     * with OF spec http://forthworks.com/standards/of1275.pdf. Individual bus
-     * bindings can be found at http://playground.sun.com/1275/bindings/.
-     */
-    char *(*get_fw_dev_path)(DeviceState *dev);
-    int (*reset)(BusState *bus);
-};
-
-typedef struct BusChild {
-    DeviceState *child;
-    int index;
-    QTAILQ_ENTRY(BusChild) sibling;
-} BusChild;
-
-/**
- * BusState:
- * @qom_allocated: Indicates whether the object was allocated by QOM.
- * @glib_allocated: Indicates whether the object was initialized in-place
- * yet is expected to be freed with g_free().
- */
-struct BusState {
-    Object obj;
-    DeviceState *parent;
-    const char *name;
-    int allow_hotplug;
-    bool qom_allocated;
-    bool glib_allocated;
-    int max_index;
-    QTAILQ_HEAD(ChildrenHead, BusChild) children;
-    QLIST_ENTRY(BusState) sibling;
-};
-
-struct Property {
-    const char   *name;
-    PropertyInfo *info;
-    int          offset;
-    uint8_t      bitnr;
-    uint8_t      qtype;
-    int64_t      defval;
-};
-
-struct PropertyInfo {
-    const char *name;
-    const char *legacy_name;
-    const char **enum_table;
-    int (*parse)(DeviceState *dev, Property *prop, const char *str);
-    int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
-    ObjectPropertyAccessor *get;
-    ObjectPropertyAccessor *set;
-    ObjectPropertyRelease *release;
-};
-
-typedef struct GlobalProperty {
-    const char *driver;
-    const char *property;
-    const char *value;
-    QTAILQ_ENTRY(GlobalProperty) next;
-} GlobalProperty;
-
-/*** Board API.  This should go away once we have a machine config file.  ***/
-
-DeviceState *qdev_create(BusState *bus, const char *name);
-DeviceState *qdev_try_create(BusState *bus, const char *name);
-bool qdev_exists(const char *name);
-int qdev_device_help(QemuOpts *opts);
-DeviceState *qdev_device_add(QemuOpts *opts);
-int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT;
-void qdev_init_nofail(DeviceState *dev);
-void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
-                                 int required_for_version);
-void qdev_unplug(DeviceState *dev, Error **errp);
-void qdev_free(DeviceState *dev);
-int qdev_simple_unplug_cb(DeviceState *dev);
-void qdev_machine_creation_done(void);
-bool qdev_machine_modified(void);
-
-qemu_irq qdev_get_gpio_in(DeviceState *dev, int n);
-void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
-
-BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
-
-/*** Device API.  ***/
-
-/* Register device properties.  */
-/* GPIO inputs also double as IRQ sinks.  */
-void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n);
-void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n);
-
-BusState *qdev_get_parent_bus(DeviceState *dev);
-
-/*** BUS API. ***/
-
-DeviceState *qdev_find_recursive(BusState *bus, const char *id);
-
-/* Returns 0 to walk children, > 0 to skip walk, < 0 to terminate walk.
*/
-typedef int (qbus_walkerfn)(BusState *bus, void *opaque);
-typedef int (qdev_walkerfn)(DeviceState *dev, void *opaque);
-
-void qbus_create_inplace(BusState *bus, const char *typename,
-                         DeviceState *parent, const char *name);
-BusState *qbus_create(const char *typename, DeviceState *parent, const char
*name);
-/* Returns > 0 if either devfn or busfn skip walk somewhere in cursion,
- *         < 0 if either devfn or busfn terminate walk somewhere in cursion,
- *           0 otherwise. */
-int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn,
-                       qbus_walkerfn *busfn, void *opaque);
-int qdev_walk_children(DeviceState *dev, qdev_walkerfn *devfn,
-                       qbus_walkerfn *busfn, void *opaque);
-void qdev_reset_all(DeviceState *dev);
-void qbus_reset_all_fn(void *opaque);
-
-void qbus_free(BusState *bus);
-
-#define FROM_QBUS(type, dev) DO_UPCAST(type, qbus, dev)
-
-/* This should go away once we get rid of the NULL bus hack */
-BusState *sysbus_get_default(void);
-
-/*** monitor commands ***/
-
-void do_info_qtree(Monitor *mon);
-void do_info_qdm(Monitor *mon);
-int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
-int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
-
-/*** qdev-properties.c ***/
-
-extern PropertyInfo qdev_prop_bit;
-extern PropertyInfo qdev_prop_uint8;
-extern PropertyInfo qdev_prop_uint16;
-extern PropertyInfo qdev_prop_uint32;
-extern PropertyInfo qdev_prop_int32;
-extern PropertyInfo qdev_prop_uint64;
-extern PropertyInfo qdev_prop_hex8;
-extern PropertyInfo qdev_prop_hex32;
-extern PropertyInfo qdev_prop_hex64;
-extern PropertyInfo qdev_prop_string;
-extern PropertyInfo qdev_prop_chr;
-extern PropertyInfo qdev_prop_ptr;
-extern PropertyInfo qdev_prop_macaddr;
-extern PropertyInfo qdev_prop_losttickpolicy;
-extern PropertyInfo qdev_prop_bios_chs_trans;
-extern PropertyInfo qdev_prop_drive;
-extern PropertyInfo qdev_prop_netdev;
-extern PropertyInfo qdev_prop_vlan;
-extern PropertyInfo qdev_prop_pci_devfn;
-extern PropertyInfo qdev_prop_blocksize;
-extern PropertyInfo qdev_prop_pci_host_devaddr;
-
-#define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
-        .name      = (_name),                                    \
-        .info      = &(_prop),                                   \
-        .offset    = offsetof(_state, _field)                    \
-            + type_check(_type,typeof_field(_state, _field)),    \
-        }
-#define DEFINE_PROP_DEFAULT(_name, _state, _field, _defval, _prop, _type) { \
-        .name      = (_name),                                           \
-        .info      = &(_prop),                                          \
-        .offset    = offsetof(_state, _field)                           \
-            + type_check(_type,typeof_field(_state, _field)),           \
-        .qtype     = QTYPE_QINT,                                        \
-        .defval    = (_type)_defval,                                    \
-        }
-#define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) {  \
-        .name      = (_name),                                    \
-        .info      = &(qdev_prop_bit),                           \
-        .bitnr    = (_bit),                                      \
-        .offset    = offsetof(_state, _field)                    \
-            + type_check(uint32_t,typeof_field(_state, _field)), \
-        .qtype     = QTYPE_QBOOL,                                \
-        .defval    = (bool)_defval,                              \
-        }
-
-#define DEFINE_PROP_UINT8(_n, _s, _f, _d)                       \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
-#define DEFINE_PROP_UINT16(_n, _s, _f, _d)                      \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint16, uint16_t)
-#define DEFINE_PROP_UINT32(_n, _s, _f, _d)                      \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint32, uint32_t)
-#define DEFINE_PROP_INT32(_n, _s, _f, _d)                      \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_int32, int32_t)
-#define DEFINE_PROP_UINT64(_n, _s, _f, _d)                      \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint64, uint64_t)
-#define DEFINE_PROP_HEX8(_n, _s, _f, _d)                       \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex8, uint8_t)
-#define DEFINE_PROP_HEX32(_n, _s, _f, _d)                       \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex32, uint32_t)
-#define DEFINE_PROP_HEX64(_n, _s, _f, _d)                       \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex64, uint64_t)
-#define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d)                   \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_pci_devfn, int32_t)
-
-#define DEFINE_PROP_PTR(_n, _s, _f)             \
-    DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, void*)
-#define DEFINE_PROP_CHR(_n, _s, _f)             \
-    DEFINE_PROP(_n, _s, _f, qdev_prop_chr, CharDriverState*)
-#define DEFINE_PROP_STRING(_n, _s, _f)             \
-    DEFINE_PROP(_n, _s, _f, qdev_prop_string, char*)
-#define DEFINE_PROP_NETDEV(_n, _s, _f)             \
-    DEFINE_PROP(_n, _s, _f, qdev_prop_netdev, NetClientState*)
-#define DEFINE_PROP_VLAN(_n, _s, _f)             \
-    DEFINE_PROP(_n, _s, _f, qdev_prop_vlan, NetClientState*)
-#define DEFINE_PROP_DRIVE(_n, _s, _f) \
-    DEFINE_PROP(_n, _s, _f, qdev_prop_drive, BlockDriverState *)
-#define DEFINE_PROP_MACADDR(_n, _s, _f)         \
-    DEFINE_PROP(_n, _s, _f, qdev_prop_macaddr, MACAddr)
-#define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_losttickpolicy, \
-                        LostTickPolicy)
-#define DEFINE_PROP_BIOS_CHS_TRANS(_n, _s, _f, _d) \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_bios_chs_trans, int)
-#define DEFINE_PROP_BLOCKSIZE(_n, _s, _f, _d) \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t)
-#define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
-    DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
-
-#define DEFINE_PROP_END_OF_LIST()               \
-    {}
-
-/* Set properties between creation and init.  */
-void *qdev_get_prop_ptr(DeviceState *dev, Property *prop);
-int qdev_prop_parse(DeviceState *dev, const char *name, const char *value);
-void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value);
-void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value);
-void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value);
-void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value);
-void qdev_prop_set_int32(DeviceState *dev, const char *name, int32_t value);
-void qdev_prop_set_uint64(DeviceState *dev, const char *name, uint64_t value);
-void qdev_prop_set_string(DeviceState *dev, const char *name, const char
*value);
-void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState
*value);
-void qdev_prop_set_netdev(DeviceState *dev, const char *name, NetClientState
*value);
-int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState
*value) QEMU_WARN_UNUSED_RESULT;
-void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name,
BlockDriverState *value);
-void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value);
-void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
-/* FIXME: Remove opaque pointer properties.  */
-void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value);
-
-void qdev_prop_register_global_list(GlobalProperty *props);
-void qdev_prop_set_globals(DeviceState *dev);
-void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
-                                    Property *prop, const char *value);
-
-char *qdev_get_fw_dev_path(DeviceState *dev);
-
-/**
- * @qdev_property_add_static - add a @Property to a device referencing a
- * field in a struct.
- */
-void qdev_property_add_static(DeviceState *dev, Property *prop, Error **errp);
-
-/**
- * @qdev_machine_init
- *
- * Initialize platform devices before machine init.  This is a hack until full
- * support for composition is added.
- */
-void qdev_machine_init(void);
-
-/**
- * @device_reset
- *
- * Reset a single device (by calling the reset method).
- */
-void device_reset(DeviceState *dev);
-
-const VMStateDescription *qdev_get_vmsd(DeviceState *dev);
-
-const char *qdev_fw_name(DeviceState *dev);
-
-Object *qdev_get_machine(void);
-
-/* FIXME: make this a link<> */
-void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
-
-extern int qdev_hotplug;
-
-char *qdev_get_dev_path(DeviceState *dev);
+#include "hw/hw.h"
+#include "qdev-core.h"
+#include "qdev-properties.h"
+#include "qdev-monitor.h"
 
 #endif
-- 
1.7.1

Igor Mammedov

2012-Aug-19 23:39 UTC

head link

[PATCH 3/5] qapi-types.h doesn''t really need to include qemu-common.h

needed to prevent build breakage when CPU becomes a child of DeviceState

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 scripts/qapi-types.py |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index cf601ae..f34addb 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -263,7 +263,7 @@ fdecl.write(mcgen(''''''
 #ifndef %(guard)s
 #define %(guard)s
 
-#include "qemu-common.h"
+#include <stdbool.h>
 
 '''''',
                   guard=guardname(h_file)))
-- 
1.7.1

Igor Mammedov

2012-Aug-19 23:39 UTC

head link

[PATCH 4/5] cleanup error.h, included qapi-types.h aready has stdbool.h

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 error.h |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/error.h b/error.h
index 96fc203..643a372 100644
--- a/error.h
+++ b/error.h
@@ -14,7 +14,6 @@
 
 #include "compiler.h"
 #include "qapi-types.h"
-#include <stdbool.h>
 
 /**
  * A class representing internal errors within QEMU.  An error has a ErrorClass
-- 
1.7.1

Igor Mammedov

2012-Aug-19 23:39 UTC

head link

[PATCH 5/5] make CPU a child of DeviceState

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/qemu/cpu.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
index ad706a6..ac44057 100644
--- a/include/qemu/cpu.h
+++ b/include/qemu/cpu.h
@@ -20,7 +20,7 @@
 #ifndef QEMU_CPU_H
 #define QEMU_CPU_H
 
-#include "qemu/object.h"
+#include "hw/qdev-core.h"
 #include "qemu-thread.h"
 
 /**
@@ -46,7 +46,7 @@ typedef struct CPUState CPUState;
  */
 typedef struct CPUClass {
     /*< private >*/
-    ObjectClass parent_class;
+    DeviceClass parent_class;
     /*< public >*/
 
     void (*reset)(CPUState *cpu);
@@ -59,7 +59,7 @@ typedef struct CPUClass {
  */
 struct CPUState {
     /*< private >*/
-    Object parent_obj;
+    DeviceState parent_obj;
     /*< public >*/
 
     struct QemuThread *thread;
-- 
1.7.1

Stefan Weil

2012-Aug-20 04:41 UTC

head link

Re: [PATCH 1/5] move qemu_irq typedef out of cpu-common.h

Am 20.08.2012 01:39, schrieb Igor Mammedov:> it''s necessary for making CPU child of DEVICE without
> causing circular header deps.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>   hw/arm-misc.h |    1 +
>   hw/bt.h       |    2 ++
>   hw/devices.h  |    2 ++
>   hw/irq.h      |    2 ++
>   hw/omap.h     |    1 +
>   hw/soc_dma.h  |    1 +
>   hw/xen.h      |    1 +
>   qemu-common.h |    1 -
>   sysemu.h      |    1 +
>   9 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/hw/arm-misc.h b/hw/arm-misc.h
> index bdd8fec..b13aa59 100644
> --- a/hw/arm-misc.h
> +++ b/hw/arm-misc.h
> @@ -12,6 +12,7 @@
>   #define ARM_MISC_H 1
>   
>   #include "memory.h"
> +#include "hw/irq.h"
>   
>   /* The CPU is also modeled as an interrupt controller.  */
>   #define ARM_PIC_CPU_IRQ 0
> diff --git a/hw/bt.h b/hw/bt.h
> index a48b8d4..ebf6a37 100644
> --- a/hw/bt.h
> +++ b/hw/bt.h
> @@ -23,6 +23,8 @@
>    * along with this program; if not, see
<http://www.gnu.org/licenses/>.
>    */
>   
> +#include "hw/irq.h"
> +
>   /* BD Address */
>   typedef struct {
>       uint8_t b[6];
> diff --git a/hw/devices.h b/hw/devices.h
> index 1a55c1e..c60bcab 100644
> --- a/hw/devices.h
> +++ b/hw/devices.h
> @@ -1,6 +1,8 @@
>   #ifndef QEMU_DEVICES_H
>   #define QEMU_DEVICES_H
>   
> +#include "hw/irq.h"
> +
>   /* ??? Not all users of this file can include cpu-common.h.  */
>   struct MemoryRegion;
>   
> diff --git a/hw/irq.h b/hw/irq.h
> index 56c55f0..1339a3a 100644
> --- a/hw/irq.h
> +++ b/hw/irq.h
> @@ -3,6 +3,8 @@
>   
>   /* Generic IRQ/GPIO pin infrastructure.  */
>   
> +typedef struct IRQState *qemu_irq;
> +
>   typedef void (*qemu_irq_handler)(void *opaque, int n, int level);
>   
>   void qemu_set_irq(qemu_irq irq, int level);
> diff --git a/hw/omap.h b/hw/omap.h
> index 413851b..8b08462 100644
> --- a/hw/omap.h
> +++ b/hw/omap.h
> @@ -19,6 +19,7 @@
>   #ifndef hw_omap_h
>   #include "memory.h"
>   # define hw_omap_h		"omap.h"
> +#include "hw/irq.h"
>   
>   # define OMAP_EMIFS_BASE	0x00000000
>   # define OMAP2_Q0_BASE		0x00000000
> diff --git a/hw/soc_dma.h b/hw/soc_dma.h
> index 904b26c..e386ace 100644
> --- a/hw/soc_dma.h
> +++ b/hw/soc_dma.h
> @@ -19,6 +19,7 @@
>    */
>   
>   #include "memory.h"
> +#include "hw/irq.h"
>   
>   struct soc_dma_s;
>   struct soc_dma_ch_s;
> diff --git a/hw/xen.h b/hw/xen.h
> index e5926b7..ff11dfd 100644
> --- a/hw/xen.h
> +++ b/hw/xen.h
> @@ -8,6 +8,7 @@
>    */
>   #include <inttypes.h>
>   
> +#include "hw/irq.h"
>   #include "qemu-common.h"
>   
>   /* xen-machine.c */
> diff --git a/qemu-common.h b/qemu-common.h
> index e5c2bcd..6677a30 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -273,7 +273,6 @@ typedef struct PCIEPort PCIEPort;
>   typedef struct PCIESlot PCIESlot;
>   typedef struct MSIMessage MSIMessage;
>   typedef struct SerialState SerialState;
> -typedef struct IRQState *qemu_irq;
>   typedef struct PCMCIACardState PCMCIACardState;
>   typedef struct MouseTransformInfo MouseTransformInfo;
>   typedef struct uWireSlave uWireSlave;
Just move the declaration of qemu_irq to the beginning of qemu-common.h
and leave the rest of files untouched. That also fixes the circular 
dependency.

I already have a patch that does this, so you can integrate it in your 
series
instead of this one.

> diff --git a/sysemu.h b/sysemu.h
> index 65552ac..f765821 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -9,6 +9,7 @@
>   #include "qapi-types.h"
>   #include "notify.h"
>   #include "main-loop.h"
> +#include "hw/irq.h"
>   
>   /* vl.c */
>

Stefan Weil

2012-Aug-20 04:52 UTC

head link

Re: [PATCH 0/5 v2] cpu: make a child of DeviceState

Am 20.08.2012 01:39, schrieb Igor Mammedov:> this is th 3rd approach to make CPU a child of DeviceState
> for both kinds of targets *-user and *-softmmu. It seems
> with current state of qemu it doesn''t take too much effort
> to make it compile. Please check if it doesn''t break
> something on other targets/archs/hosts than i386.
>
> what''s tested:
>    - compile tested building all targets on FC17x64 host.
>    - briefly tested i386 user and softmmu targets
>
> Anthony Liguori (1):
>    qdev: split up header so it can be used in cpu.h
>
> Igor Mammedov (4):
>    move qemu_irq typedef out of cpu-common.h
>    qapi-types.h doesn''t really need to include qemu-common.h
>    cleanup error.h, included qapi-types.h aready has stdbool.h
>    make CPU a child of DeviceState
>
>   error.h               |    1 -
>   hw/arm-misc.h         |    1 +
>   hw/bt.h               |    2 +
>   hw/devices.h          |    2 +
>   hw/irq.h              |    2 +
>   hw/mc146818rtc.c      |    1 +
>   hw/omap.h             |    1 +
>   hw/qdev-addr.c        |    1 +
>   hw/qdev-core.h        |  240 ++++++++++++++++++++++++++++++++
>   hw/qdev-monitor.h     |   16 ++
>   hw/qdev-properties.c  |    1 +
>   hw/qdev-properties.h  |  128 +++++++++++++++++
>   hw/qdev.c             |    1 +
>   hw/qdev.h             |  371
+------------------------------------------------
>   hw/soc_dma.h          |    1 +
>   hw/xen.h              |    1 +
>   include/qemu/cpu.h    |    6 +-
>   qemu-common.h         |    1 -
>   scripts/qapi-types.py |    2 +-
>   sysemu.h              |    1 +
>   20 files changed, 407 insertions(+), 373 deletions(-)
>   create mode 100644 hw/qdev-core.h
>   create mode 100644 hw/qdev-monitor.h
>   create mode 100644 hw/qdev-properties.h
>
I''d prefer if you could keep the following simple pattern:

* Start includes in *.c files with config.h (optionally)
   and qemu-common.h.

* Don''t include standard include files which are already
   included in qemu-common.h

* Don''t include qemu-common.h in *.h files.

Regards,

Stefan Weil

Igor Mammedov

2012-Aug-20 11:13 UTC

head link

Re: [PATCH 1/5] move qemu_irq typedef out of cpu-common.h

On Mon, 20 Aug 2012 06:41:04 +0200
Stefan Weil <sw@weilnetz.de> wrote:
> Am 20.08.2012 01:39, schrieb Igor Mammedov:
> > it''s necessary for making CPU child of DEVICE without
> > causing circular header deps.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >   hw/arm-misc.h |    1 +
> >   hw/bt.h       |    2 ++
> >   hw/devices.h  |    2 ++
> >   hw/irq.h      |    2 ++
> >   hw/omap.h     |    1 +
> >   hw/soc_dma.h  |    1 +
> >   hw/xen.h      |    1 +
> >   qemu-common.h |    1 -
> >   sysemu.h      |    1 +
> >   9 files changed, 11 insertions(+), 1 deletions(-)
> >
> > diff --git a/hw/arm-misc.h b/hw/arm-misc.h
> > index bdd8fec..b13aa59 100644
> > --- a/hw/arm-misc.h
> > +++ b/hw/arm-misc.h
> > @@ -12,6 +12,7 @@
> >   #define ARM_MISC_H 1
> >   
> >   #include "memory.h"
> > +#include "hw/irq.h"
> >   
> >   /* The CPU is also modeled as an interrupt controller.  */
> >   #define ARM_PIC_CPU_IRQ 0
> > diff --git a/hw/bt.h b/hw/bt.h
> > index a48b8d4..ebf6a37 100644
> > --- a/hw/bt.h
> > +++ b/hw/bt.h
> > @@ -23,6 +23,8 @@
> >    * along with this program; if not, see
<http://www.gnu.org/licenses/>.
> >    */
> >   
> > +#include "hw/irq.h"
> > +
> >   /* BD Address */
> >   typedef struct {
> >       uint8_t b[6];
> > diff --git a/hw/devices.h b/hw/devices.h
> > index 1a55c1e..c60bcab 100644
> > --- a/hw/devices.h
> > +++ b/hw/devices.h
> > @@ -1,6 +1,8 @@
> >   #ifndef QEMU_DEVICES_H
> >   #define QEMU_DEVICES_H
> >   
> > +#include "hw/irq.h"
> > +
> >   /* ??? Not all users of this file can include cpu-common.h.  */
> >   struct MemoryRegion;
> >   
> > diff --git a/hw/irq.h b/hw/irq.h
> > index 56c55f0..1339a3a 100644
> > --- a/hw/irq.h
> > +++ b/hw/irq.h
> > @@ -3,6 +3,8 @@
> >   
> >   /* Generic IRQ/GPIO pin infrastructure.  */
> >   
> > +typedef struct IRQState *qemu_irq;
> > +
> >   typedef void (*qemu_irq_handler)(void *opaque, int n, int level);
> >   
> >   void qemu_set_irq(qemu_irq irq, int level);
> > diff --git a/hw/omap.h b/hw/omap.h
> > index 413851b..8b08462 100644
> > --- a/hw/omap.h
> > +++ b/hw/omap.h
> > @@ -19,6 +19,7 @@
> >   #ifndef hw_omap_h
> >   #include "memory.h"
> >   # define hw_omap_h		"omap.h"
> > +#include "hw/irq.h"
> >   
> >   # define OMAP_EMIFS_BASE	0x00000000
> >   # define OMAP2_Q0_BASE		0x00000000
> > diff --git a/hw/soc_dma.h b/hw/soc_dma.h
> > index 904b26c..e386ace 100644
> > --- a/hw/soc_dma.h
> > +++ b/hw/soc_dma.h
> > @@ -19,6 +19,7 @@
> >    */
> >   
> >   #include "memory.h"
> > +#include "hw/irq.h"
> >   
> >   struct soc_dma_s;
> >   struct soc_dma_ch_s;
> > diff --git a/hw/xen.h b/hw/xen.h
> > index e5926b7..ff11dfd 100644
> > --- a/hw/xen.h
> > +++ b/hw/xen.h
> > @@ -8,6 +8,7 @@
> >    */
> >   #include <inttypes.h>
> >   
> > +#include "hw/irq.h"
> >   #include "qemu-common.h"
> >   
> >   /* xen-machine.c */
> > diff --git a/qemu-common.h b/qemu-common.h
> > index e5c2bcd..6677a30 100644
> > --- a/qemu-common.h
> > +++ b/qemu-common.h
> > @@ -273,7 +273,6 @@ typedef struct PCIEPort PCIEPort;
> >   typedef struct PCIESlot PCIESlot;
> >   typedef struct MSIMessage MSIMessage;
> >   typedef struct SerialState SerialState;
> > -typedef struct IRQState *qemu_irq;
> >   typedef struct PCMCIACardState PCMCIACardState;
> >   typedef struct MouseTransformInfo MouseTransformInfo;
> >   typedef struct uWireSlave uWireSlave;
> 
> Just move the declaration of qemu_irq to the beginning of qemu-common.h
> and leave the rest of files untouched. That also fixes the circular 
> dependency.
> 
> I already have a patch that does this, so you can integrate it in your 
> series
> instead of this one.No doubt it''s more simpler way, but IMHO It''s more of a hack
than fixing
problem.
It works for now but doesn''t alleviate problem with header nightmare in
qemu,
where everything is included in qemu-common.h and everything includes it as
well.

Any way if majority prefer simple move, I''ll drop this patch in favor
of yours.
 > 
> 
> > diff --git a/sysemu.h b/sysemu.h
> > index 65552ac..f765821 100644
> > --- a/sysemu.h
> > +++ b/sysemu.h
> > @@ -9,6 +9,7 @@
> >   #include "qapi-types.h"
> >   #include "notify.h"
> >   #include "main-loop.h"
> > +#include "hw/irq.h"
> >   
> >   /* vl.c */
> >   
> 

-- 
Regards,
  Igor

Igor Mammedov

2012-Aug-20 11:47 UTC

head link

Re: [PATCH 0/5 v2] cpu: make a child of DeviceState

On Mon, 20 Aug 2012 06:52:51 +0200
Stefan Weil <sw@weilnetz.de> wrote:
> Am 20.08.2012 01:39, schrieb Igor Mammedov:
> > this is th 3rd approach to make CPU a child of DeviceState
> > for both kinds of targets *-user and *-softmmu. It seems
> > with current state of qemu it doesn''t take too much effort
> > to make it compile. Please check if it doesn''t break
> > something on other targets/archs/hosts than i386.
> >
> > what''s tested:
> >    - compile tested building all targets on FC17x64 host.
> >    - briefly tested i386 user and softmmu targets
> >
> > Anthony Liguori (1):
> >    qdev: split up header so it can be used in cpu.h
> >
> > Igor Mammedov (4):
> >    move qemu_irq typedef out of cpu-common.h
> >    qapi-types.h doesn''t really need to include qemu-common.h
> >    cleanup error.h, included qapi-types.h aready has stdbool.h
> >    make CPU a child of DeviceState
> >
> >   error.h               |    1 -
> >   hw/arm-misc.h         |    1 +
> >   hw/bt.h               |    2 +
> >   hw/devices.h          |    2 +
> >   hw/irq.h              |    2 +
> >   hw/mc146818rtc.c      |    1 +
> >   hw/omap.h             |    1 +
> >   hw/qdev-addr.c        |    1 +
> >   hw/qdev-core.h        |  240 ++++++++++++++++++++++++++++++++
> >   hw/qdev-monitor.h     |   16 ++
> >   hw/qdev-properties.c  |    1 +
> >   hw/qdev-properties.h  |  128 +++++++++++++++++
> >   hw/qdev.c             |    1 +
> >   hw/qdev.h             |  371
+------------------------------------------------
> >   hw/soc_dma.h          |    1 +
> >   hw/xen.h              |    1 +
> >   include/qemu/cpu.h    |    6 +-
> >   qemu-common.h         |    1 -
> >   scripts/qapi-types.py |    2 +-
> >   sysemu.h              |    1 +
> >   20 files changed, 407 insertions(+), 373 deletions(-)
> >   create mode 100644 hw/qdev-core.h
> >   create mode 100644 hw/qdev-monitor.h
> >   create mode 100644 hw/qdev-properties.h
> >
> 
> I''d prefer if you could keep the following simple pattern:
> 
> * Start includes in *.c files with config.h (optionally)
>    and qemu-common.h.Can''t agree with you on this. I''d say that every header should
be be self
sufficient, include other headers if it uses types from them and NOT depend on
the position where it''s included, it should provide all its own deps.
> 
> * Don''t include standard include files which are already
>    included in qemu-common.hProbably initially qemu-common.h was intended to simplify inclusion
of standard headers and glue stuff in multi-os build environment. but it seems
to be become misused. It includes now a lot of stuff that is not common to
every file it''s included in.
Perhaps we should split out of it std includes and glue layer into something
like std/host-common.h and case on case basis move out other type
definitions that are not common into there appropriate places. Like with
qemu_irq.
> 
> * Don''t include qemu-common.h in *.h files.I''m all for it. But it''s difficult right now because it tends
to be included
in a lot of headers that don''t really need it and then other headers
happens
to depend on previous inclusion of it. It''s difficult to untangle this
in one
take, but could be possible in small steps.

[1/1] is a direction that could be used to put type custom types in their
proper places. It would be better way long term and help to avoid problems
with including one header in another. 
> 
> Regards,
> 
> Stefan Weil
> 

-- 
Regards,
  Igor

Luiz Capitulino

2012-Aug-20 15:22 UTC

head link

Re: [PATCH 3/5] qapi-types.h doesn''t really need to include qemu-common.h

On Mon, 20 Aug 2012 01:39:37 +0200
Igor Mammedov <imammedo@redhat.com> wrote:
> needed to prevent build breakage when CPU becomes a child of DeviceState
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  scripts/qapi-types.py |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index cf601ae..f34addb 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -263,7 +263,7 @@ fdecl.write(mcgen(''''''
>  #ifndef %(guard)s
>  #define %(guard)s
>  
> -#include "qemu-common.h"
> +#include <stdbool.h>
Please, also include <stdint.h>, as int64_t is used in qapi-types.h.

The build doesn''t break probably because files including qapi-types.h
are
including <stdint.h> first.
>  
>  '''''',
>                    guard=guardname(h_file)))

Luiz Capitulino

2012-Aug-20 15:28 UTC

head link

Re: [PATCH 4/5] cleanup error.h, included qapi-types.h aready has stdbool.h

On Mon, 20 Aug 2012 01:39:38 +0200
Igor Mammedov <imammedo@redhat.com> wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  error.h |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/error.h b/error.h
> index 96fc203..643a372 100644
> --- a/error.h
> +++ b/error.h
> @@ -14,7 +14,6 @@
>  
>  #include "compiler.h"
>  #include "qapi-types.h"
> -#include <stdbool.h>
Hmm, not good. qapi-types.h includes <stdbool.h> for internal matters,
files
including qapi-types.h shouldn''t rely on this (as they can break if
qapi-types.h
is changed not to include <stdbool.h>).

You can keep this code as it is.
>  
>  /**
>   * A class representing internal errors within QEMU.  An error has a
ErrorClass

Blue Swirl

2012-Aug-20 19:46 UTC

head link

Re: [PATCH 1/5] move qemu_irq typedef out of cpu-common.h

On Mon, Aug 20, 2012 at 11:13 AM, Igor Mammedov <imammedo@redhat.com>
wrote:> On Mon, 20 Aug 2012 06:41:04 +0200
> Stefan Weil <sw@weilnetz.de> wrote:
>
>> Am 20.08.2012 01:39, schrieb Igor Mammedov:
>> > it''s necessary for making CPU child of DEVICE without
>> > causing circular header deps.
>> >
>> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>> > ---
>> >   hw/arm-misc.h |    1 +
>> >   hw/bt.h       |    2 ++
>> >   hw/devices.h  |    2 ++
>> >   hw/irq.h      |    2 ++
>> >   hw/omap.h     |    1 +
>> >   hw/soc_dma.h  |    1 +
>> >   hw/xen.h      |    1 +
>> >   qemu-common.h |    1 -
>> >   sysemu.h      |    1 +
>> >   9 files changed, 11 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/hw/arm-misc.h b/hw/arm-misc.h
>> > index bdd8fec..b13aa59 100644
>> > --- a/hw/arm-misc.h
>> > +++ b/hw/arm-misc.h
>> > @@ -12,6 +12,7 @@
>> >   #define ARM_MISC_H 1
>> >
>> >   #include "memory.h"
>> > +#include "hw/irq.h"
>> >
>> >   /* The CPU is also modeled as an interrupt controller.  */
>> >   #define ARM_PIC_CPU_IRQ 0
>> > diff --git a/hw/bt.h b/hw/bt.h
>> > index a48b8d4..ebf6a37 100644
>> > --- a/hw/bt.h
>> > +++ b/hw/bt.h
>> > @@ -23,6 +23,8 @@
>> >    * along with this program; if not, see
<http://www.gnu.org/licenses/>.
>> >    */
>> >
>> > +#include "hw/irq.h"
>> > +
>> >   /* BD Address */
>> >   typedef struct {
>> >       uint8_t b[6];
>> > diff --git a/hw/devices.h b/hw/devices.h
>> > index 1a55c1e..c60bcab 100644
>> > --- a/hw/devices.h
>> > +++ b/hw/devices.h
>> > @@ -1,6 +1,8 @@
>> >   #ifndef QEMU_DEVICES_H
>> >   #define QEMU_DEVICES_H
>> >
>> > +#include "hw/irq.h"
>> > +
>> >   /* ??? Not all users of this file can include cpu-common.h.  */
>> >   struct MemoryRegion;
>> >
>> > diff --git a/hw/irq.h b/hw/irq.h
>> > index 56c55f0..1339a3a 100644
>> > --- a/hw/irq.h
>> > +++ b/hw/irq.h
>> > @@ -3,6 +3,8 @@
>> >
>> >   /* Generic IRQ/GPIO pin infrastructure.  */
>> >
>> > +typedef struct IRQState *qemu_irq;
>> > +
>> >   typedef void (*qemu_irq_handler)(void *opaque, int n, int
level);
>> >
>> >   void qemu_set_irq(qemu_irq irq, int level);
>> > diff --git a/hw/omap.h b/hw/omap.h
>> > index 413851b..8b08462 100644
>> > --- a/hw/omap.h
>> > +++ b/hw/omap.h
>> > @@ -19,6 +19,7 @@
>> >   #ifndef hw_omap_h
>> >   #include "memory.h"
>> >   # define hw_omap_h                "omap.h"
>> > +#include "hw/irq.h"
>> >
>> >   # define OMAP_EMIFS_BASE  0x00000000
>> >   # define OMAP2_Q0_BASE            0x00000000
>> > diff --git a/hw/soc_dma.h b/hw/soc_dma.h
>> > index 904b26c..e386ace 100644
>> > --- a/hw/soc_dma.h
>> > +++ b/hw/soc_dma.h
>> > @@ -19,6 +19,7 @@
>> >    */
>> >
>> >   #include "memory.h"
>> > +#include "hw/irq.h"
>> >
>> >   struct soc_dma_s;
>> >   struct soc_dma_ch_s;
>> > diff --git a/hw/xen.h b/hw/xen.h
>> > index e5926b7..ff11dfd 100644
>> > --- a/hw/xen.h
>> > +++ b/hw/xen.h
>> > @@ -8,6 +8,7 @@
>> >    */
>> >   #include <inttypes.h>
>> >
>> > +#include "hw/irq.h"
>> >   #include "qemu-common.h"
>> >
>> >   /* xen-machine.c */
>> > diff --git a/qemu-common.h b/qemu-common.h
>> > index e5c2bcd..6677a30 100644
>> > --- a/qemu-common.h
>> > +++ b/qemu-common.h
>> > @@ -273,7 +273,6 @@ typedef struct PCIEPort PCIEPort;
>> >   typedef struct PCIESlot PCIESlot;
>> >   typedef struct MSIMessage MSIMessage;
>> >   typedef struct SerialState SerialState;
>> > -typedef struct IRQState *qemu_irq;
>> >   typedef struct PCMCIACardState PCMCIACardState;
>> >   typedef struct MouseTransformInfo MouseTransformInfo;
>> >   typedef struct uWireSlave uWireSlave;
>>
>> Just move the declaration of qemu_irq to the beginning of qemu-common.h
>> and leave the rest of files untouched. That also fixes the circular
>> dependency.
>>
>> I already have a patch that does this, so you can integrate it in your
>> series
>> instead of this one.
> No doubt it''s more simpler way, but IMHO It''s more of a
hack than fixing
> problem.
> It works for now but doesn''t alleviate problem with header
nightmare in qemu,
> where everything is included in qemu-common.h and everything includes it as
> well.
>
> Any way if majority prefer simple move, I''ll drop this patch in
favor of yours.
I like Igor''s approach more.
>
>>
>>
>> > diff --git a/sysemu.h b/sysemu.h
>> > index 65552ac..f765821 100644
>> > --- a/sysemu.h
>> > +++ b/sysemu.h
>> > @@ -9,6 +9,7 @@
>> >   #include "qapi-types.h"
>> >   #include "notify.h"
>> >   #include "main-loop.h"
>> > +#include "hw/irq.h"
>> >
>> >   /* vl.c */
>> >
>>
>
>
> --
> Regards,
>   Igor

Igor Mammedov

2012-Aug-20 19:59 UTC

head link

Re: [PATCH 3/5] qapi-types.h doesn''t really need to include qemu-common.h

On Mon, 20 Aug 2012 12:22:10 -0300
Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Mon, 20 Aug 2012 01:39:37 +0200
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > needed to prevent build breakage when CPU becomes a child of
DeviceState
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  scripts/qapi-types.py |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> > index cf601ae..f34addb 100644
> > --- a/scripts/qapi-types.py
> > +++ b/scripts/qapi-types.py
> > @@ -263,7 +263,7 @@ fdecl.write(mcgen(''''''
> >  #ifndef %(guard)s
> >  #define %(guard)s
> >  
> > -#include "qemu-common.h"
> > +#include <stdbool.h>
> 
> Please, also include <stdint.h>, as int64_t is used in qapi-types.h.
> 
> The build doesn''t break probably because files including
qapi-types.h are
> including <stdint.h> first.Thanks for suggestion. I''ll fix it for next respin.
> 
> >  
> >  '''''',
> >                    guard=guardname(h_file)))
> 

-- 
Regards,
  Igor

Igor Mammedov

2012-Aug-20 20:00 UTC

head link

Re: [Xen-devel] [PATCH 4/5] cleanup error.h, included qapi-types.h aready has stdbool.h

On Mon, 20 Aug 2012 12:28:05 -0300
Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Mon, 20 Aug 2012 01:39:38 +0200
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  error.h |    1 -
> >  1 files changed, 0 insertions(+), 1 deletions(-)
> > 
> > diff --git a/error.h b/error.h
> > index 96fc203..643a372 100644
> > --- a/error.h
> > +++ b/error.h
> > @@ -14,7 +14,6 @@
> >  
> >  #include "compiler.h"
> >  #include "qapi-types.h"
> > -#include <stdbool.h>
> 
> Hmm, not good. qapi-types.h includes <stdbool.h> for internal
matters, files
> including qapi-types.h shouldn''t rely on this (as they can break
if qapi-types.h
> is changed not to include <stdbool.h>).
> 
> You can keep this code as it is.
Agreed, I''ll drop this patch.> 
> >  
> >  /**
> >   * A class representing internal errors within QEMU.  An error has a
ErrorClass
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

-- 
Regards,
  Igor

Stefan Weil

2012-Aug-20 20:07 UTC

head link

[RFC] How should QEMU code handle include statements (was: Re: [PATCH 0/5 v2] cpu: make a child of DeviceState)

Am 20.08.2012 13:47, schrieb Igor Mammedov:> On Mon, 20 Aug 2012 06:52:51 +0200
> Stefan Weil<sw@weilnetz.de>  wrote:
>> I''d prefer if you could keep the following simple pattern:
>>
>> * Start includes in *.c files with config.h (optionally)
>>     and qemu-common.h.
> Can''t agree with you on this. I''d say that every header
should be be self
> sufficient, include other headers if it uses types from them and NOT depend
on
> the position where it''s included, it should provide all its own
deps.
>> * Don''t include standard include files which are already
>>     included in qemu-common.h
> Probably initially qemu-common.h was intended to simplify inclusion
> of standard headers and glue stuff in multi-os build environment. but it
seems
> to be become misused. It includes now a lot of stuff that is not common to
> every file it''s included in.
> Perhaps we should split out of it std includes and glue layer into
something
> like std/host-common.h and case on case basis move out other type
> definitions that are not common into there appropriate places. Like with
> qemu_irq.

There are several possible strategies regarding include statements:

1. Each header file which represents some public interface must include
    any header file which it depends on, so applications which include
    xxx.h can rely on the fact that they won''t need anything else to
    compile xxx.h.

    To minimize dependencies, this rule can be extended: each header
    file must not include unneeded header files.

    Usually header files in /usr/include are built according to these rules.

2. There is one header file (or a small set of header files) which includes
    a basic set of features which normal code needs. Any C code file starts
    by including this header file.

    Other header files can rely on the fact that the basic set of features
    are already available.

3. In a modification of (2), each header file can include the basic
    header file(s).

4. The status quo of QEMU is a wild mixture of those strategies.

IMHO, there should be a consensus about the strategy which is used
for QEMU code.

While I personally prefer (1) and used it for my first contributions,
QEMU introduced qemu-common.h. I had the impression that from then on
QEMU preferred strategy (2). Obviously not everybody shares that
impression.

Which strategy / rule do we want to use for QEMU code?

Regards,

Stefan Weil

Anthony Liguori

2012-Aug-20 20:14 UTC

head link

Re: [PATCH 1/5] move qemu_irq typedef out of cpu-common.h

Blue Swirl <blauwirbel@gmail.com> writes:
> On Mon, Aug 20, 2012 at 11:13 AM, Igor Mammedov <imammedo@redhat.com>
wrote:
>> On Mon, 20 Aug 2012 06:41:04 +0200
>> Stefan Weil <sw@weilnetz.de> wrote:
>>
>>> Am 20.08.2012 01:39, schrieb Igor Mammedov:
>>> > it''s necessary for making CPU child of DEVICE without
>>> > causing circular header deps.
>>> >
>>> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>> > ---
>>> >   hw/arm-misc.h |    1 +
>>> >   hw/bt.h       |    2 ++
>>> >   hw/devices.h  |    2 ++
>>> >   hw/irq.h      |    2 ++
>>> >   hw/omap.h     |    1 +
>>> >   hw/soc_dma.h  |    1 +
>>> >   hw/xen.h      |    1 +
>>> >   qemu-common.h |    1 -
>>> >   sysemu.h      |    1 +
>>> >   9 files changed, 11 insertions(+), 1 deletions(-)
>>> >
>>> > diff --git a/hw/arm-misc.h b/hw/arm-misc.h
>>> > index bdd8fec..b13aa59 100644
>>> > --- a/hw/arm-misc.h
>>> > +++ b/hw/arm-misc.h
>>> > @@ -12,6 +12,7 @@
>>> >   #define ARM_MISC_H 1
>>> >
>>> >   #include "memory.h"
>>> > +#include "hw/irq.h"
>>> >
>>> >   /* The CPU is also modeled as an interrupt controller.  */
>>> >   #define ARM_PIC_CPU_IRQ 0
>>> > diff --git a/hw/bt.h b/hw/bt.h
>>> > index a48b8d4..ebf6a37 100644
>>> > --- a/hw/bt.h
>>> > +++ b/hw/bt.h
>>> > @@ -23,6 +23,8 @@
>>> >    * along with this program; if not, see
<http://www.gnu.org/licenses/>.
>>> >    */
>>> >
>>> > +#include "hw/irq.h"
>>> > +
>>> >   /* BD Address */
>>> >   typedef struct {
>>> >       uint8_t b[6];
>>> > diff --git a/hw/devices.h b/hw/devices.h
>>> > index 1a55c1e..c60bcab 100644
>>> > --- a/hw/devices.h
>>> > +++ b/hw/devices.h
>>> > @@ -1,6 +1,8 @@
>>> >   #ifndef QEMU_DEVICES_H
>>> >   #define QEMU_DEVICES_H
>>> >
>>> > +#include "hw/irq.h"
>>> > +
>>> >   /* ??? Not all users of this file can include cpu-common.h. 
*/
>>> >   struct MemoryRegion;
>>> >
>>> > diff --git a/hw/irq.h b/hw/irq.h
>>> > index 56c55f0..1339a3a 100644
>>> > --- a/hw/irq.h
>>> > +++ b/hw/irq.h
>>> > @@ -3,6 +3,8 @@
>>> >
>>> >   /* Generic IRQ/GPIO pin infrastructure.  */
>>> >
>>> > +typedef struct IRQState *qemu_irq;
>>> > +
>>> >   typedef void (*qemu_irq_handler)(void *opaque, int n, int
level);
>>> >
>>> >   void qemu_set_irq(qemu_irq irq, int level);
>>> > diff --git a/hw/omap.h b/hw/omap.h
>>> > index 413851b..8b08462 100644
>>> > --- a/hw/omap.h
>>> > +++ b/hw/omap.h
>>> > @@ -19,6 +19,7 @@
>>> >   #ifndef hw_omap_h
>>> >   #include "memory.h"
>>> >   # define hw_omap_h                "omap.h"
>>> > +#include "hw/irq.h"
>>> >
>>> >   # define OMAP_EMIFS_BASE  0x00000000
>>> >   # define OMAP2_Q0_BASE            0x00000000
>>> > diff --git a/hw/soc_dma.h b/hw/soc_dma.h
>>> > index 904b26c..e386ace 100644
>>> > --- a/hw/soc_dma.h
>>> > +++ b/hw/soc_dma.h
>>> > @@ -19,6 +19,7 @@
>>> >    */
>>> >
>>> >   #include "memory.h"
>>> > +#include "hw/irq.h"
>>> >
>>> >   struct soc_dma_s;
>>> >   struct soc_dma_ch_s;
>>> > diff --git a/hw/xen.h b/hw/xen.h
>>> > index e5926b7..ff11dfd 100644
>>> > --- a/hw/xen.h
>>> > +++ b/hw/xen.h
>>> > @@ -8,6 +8,7 @@
>>> >    */
>>> >   #include <inttypes.h>
>>> >
>>> > +#include "hw/irq.h"
>>> >   #include "qemu-common.h"
>>> >
>>> >   /* xen-machine.c */
>>> > diff --git a/qemu-common.h b/qemu-common.h
>>> > index e5c2bcd..6677a30 100644
>>> > --- a/qemu-common.h
>>> > +++ b/qemu-common.h
>>> > @@ -273,7 +273,6 @@ typedef struct PCIEPort PCIEPort;
>>> >   typedef struct PCIESlot PCIESlot;
>>> >   typedef struct MSIMessage MSIMessage;
>>> >   typedef struct SerialState SerialState;
>>> > -typedef struct IRQState *qemu_irq;
>>> >   typedef struct PCMCIACardState PCMCIACardState;
>>> >   typedef struct MouseTransformInfo MouseTransformInfo;
>>> >   typedef struct uWireSlave uWireSlave;
>>>
>>> Just move the declaration of qemu_irq to the beginning of
qemu-common.h
>>> and leave the rest of files untouched. That also fixes the circular
>>> dependency.
>>>
>>> I already have a patch that does this, so you can integrate it in
your
>>> series
>>> instead of this one.
>> No doubt it''s more simpler way, but IMHO It''s more of
a hack than fixing
>> problem.
>> It works for now but doesn''t alleviate problem with header
nightmare in qemu,
>> where everything is included in qemu-common.h and everything includes
it as
>> well.
>>
>> Any way if majority prefer simple move, I''ll drop this patch
in favor of yours.
>
> I like Igor''s approach more.
Ack.

We should move away from dumping ground includes like qemu-common in
favor of meaningful headers that are explicitly included where needed.

Regards,

Anthony Liguori
>
>>
>>>
>>>
>>> > diff --git a/sysemu.h b/sysemu.h
>>> > index 65552ac..f765821 100644
>>> > --- a/sysemu.h
>>> > +++ b/sysemu.h
>>> > @@ -9,6 +9,7 @@
>>> >   #include "qapi-types.h"
>>> >   #include "notify.h"
>>> >   #include "main-loop.h"
>>> > +#include "hw/irq.h"
>>> >
>>> >   /* vl.c */
>>> >
>>>
>>
>>
>> --
>> Regards,
>>   Igor

Avi Kivity

2012-Aug-21 10:19 UTC

head link

Re: [RFC] How should QEMU code handle include statements

On 08/20/2012 11:07 PM, Stefan Weil wrote:
> While I personally prefer (1) and used it for my first contributions,
> QEMU introduced qemu-common.h. I had the impression that from then on
> QEMU preferred strategy (2). Obviously not everybody shares that
> impression.
> 
> Which strategy / rule do we want to use for QEMU code?
(1).


-- 
error compiling committee.c: too many arguments to function

Eduardo Habkost

2012-Aug-21 14:03 UTC

head link

Re: [PATCH 5/5] make CPU a child of DeviceState

On Mon, Aug 20, 2012 at 01:39:39AM +0200, Igor Mammedov
wrote:> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  include/qemu/cpu.h |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
> index ad706a6..ac44057 100644
> --- a/include/qemu/cpu.h
> +++ b/include/qemu/cpu.h
> @@ -20,7 +20,7 @@
>  #ifndef QEMU_CPU_H
>  #define QEMU_CPU_H
>  
> -#include "qemu/object.h"
> +#include "hw/qdev-core.h"
>  #include "qemu-thread.h"
>  
>  /**
> @@ -46,7 +46,7 @@ typedef struct CPUState CPUState;
>   */
>  typedef struct CPUClass {
>      /*< private >*/
> -    ObjectClass parent_class;
> +    DeviceClass parent_class;
>      /*< public >*/
>  
>      void (*reset)(CPUState *cpu);
> @@ -59,7 +59,7 @@ typedef struct CPUClass {
>   */
>  struct CPUState {
>      /*< private >*/
> -    Object parent_obj;
> +    DeviceState parent_obj;
>      /*< public >*/
>  
>      struct QemuThread *thread;
Don''t you need to update cpu_type_info on qom/cpu.c as well? It still
has .parent = TYPE_OBJECT.

-- 
Eduardo

Xen devel - Aug 2012 - [PATCH 0/5 v2] cpu: make a child of DeviceState

[PATCH 0/5 v2] cpu: make a child of DeviceState

[PATCH 1/5] move qemu_irq typedef out of cpu-common.h

[PATCH 2/5] qdev: split up header so it can be used in cpu.h

[PATCH 3/5] qapi-types.h doesn''t really need to include qemu-common.h

[PATCH 4/5] cleanup error.h, included qapi-types.h aready has stdbool.h

[PATCH 5/5] make CPU a child of DeviceState

Re: [PATCH 1/5] move qemu_irq typedef out of cpu-common.h

Re: [PATCH 0/5 v2] cpu: make a child of DeviceState

Re: [PATCH 1/5] move qemu_irq typedef out of cpu-common.h

Re: [PATCH 0/5 v2] cpu: make a child of DeviceState

Re: [PATCH 3/5] qapi-types.h doesn''t really need to include qemu-common.h

Re: [PATCH 4/5] cleanup error.h, included qapi-types.h aready has stdbool.h

Re: [PATCH 1/5] move qemu_irq typedef out of cpu-common.h

Re: [PATCH 3/5] qapi-types.h doesn''t really need to include qemu-common.h

Re: [Xen-devel] [PATCH 4/5] cleanup error.h, included qapi-types.h aready has stdbool.h

[RFC] How should QEMU code handle include statements (was: Re: [PATCH 0/5 v2] cpu: make a child of DeviceState)

Re: [PATCH 1/5] move qemu_irq typedef out of cpu-common.h

Re: [RFC] How should QEMU code handle include statements

Re: [PATCH 5/5] make CPU a child of DeviceState