[Qemu-devel] [PATCH] kvm/apic: use QOM style

[xiaoqiang.zhao]

Change apic and kvm/apic to use QOM interface.

Includes:
1. APICCommonState now use QOM realizefn
2. Remove DO_UPCAST() for APICCommonState
3. Use type constant
4. Change DeviceState pointers from 'd' to 'dev', sounds better?

this is my first qemu patch, if there is a problem, please let me konw ;-)
---
 hw/i386/kvm/apic.c              |   40 ++++++++++++++++++----
 hw/intc/apic.c                  |   70 +++++++++++++++++++++++++-------------
 hw/intc/apic_common.c           |   70 +++++++++++++++++++--------------------
 include/hw/i386/apic_internal.h |    1 -
 4 files changed, 113 insertions(+), 68 deletions(-)

diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 5609063..5733dbb 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -13,6 +13,22 @@
 #include "hw/pci/msi.h"
 #include "sysemu/kvm.h"
 
+#define TYPE_KVM_APIC "kvm-apic"
+#define KVM_APIC_CLASS(class) \
+    OBJECT_CLASS_CHECK(KVMAPICClass, (class), TYPE_KVM_APIC)
+#define KVM_APIC_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(KVMAPICClass, (obj), TYPE_KVM_APIC)
+
+/**
+ * KVMAPICClass:
+ * @parent_realize: The parent's realizefn.
+ */
+typedef struct KVMAPICClass {
+    APICCommonClass parent_class;
+
+    DeviceRealize parent_realize;
+} KVMAPICClass;
+
 static inline void kvm_apic_set_reg(struct kvm_lapic_state *kapic,
                                     int reg_id, uint32_t val)
 {
@@ -25,9 +41,9 @@ static inline uint32_t kvm_apic_get_reg(struct 
kvm_lapic_state *kapic,
     return *((uint32_t *)(kapic->regs + (reg_id << 4)));
 }
 
-void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
+void kvm_put_apic_state(DeviceState *dev, struct kvm_lapic_state *kapic)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(dev);
     int i;
 
     memset(kapic, 0, sizeof(*kapic));
@@ -51,9 +67,9 @@ void kvm_put_apic_state(DeviceState *d, struct 
kvm_lapic_state *kapic)
     kvm_apic_set_reg(kapic, 0x3e, s->divide_conf);
 }
 
-void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
+void kvm_get_apic_state(DeviceState *dev, struct kvm_lapic_state *kapic)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(dev);
     int i, v;
 
     s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
@@ -171,34 +187,44 @@ static const MemoryRegionOps kvm_apic_io_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void kvm_apic_init(APICCommonState *s)
+static void kvm_apic_realize(DeviceState *dev, Error **errp)
 {
+    APICCommonState *s = APIC_COMMON(dev);
+    KVMAPICClass *kac = KVM_APIC_GET_CLASS(dev);
+
     memory_region_init_io(&s->io_memory, NULL, &kvm_apic_io_ops, s, 
"kvm-apic-msi",
                           APIC_SPACE_SIZE);
 
     if (kvm_has_gsi_routing()) {
         msi_supported = true;
     }
+
+    kac->parent_realize(dev, errp);
 }
 
 static void kvm_apic_class_init(ObjectClass *klass, void *data)
 {
     APICCommonClass *k = APIC_COMMON_CLASS(klass);
+    KVMAPICClass *kac = KVM_APIC_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
 
-    k->init = kvm_apic_init;
     k->set_base = kvm_apic_set_base;
     k->set_tpr = kvm_apic_set_tpr;
     k->get_tpr = kvm_apic_get_tpr;
     k->enable_tpr_reporting = kvm_apic_enable_tpr_reporting;
     k->vapic_base_update = kvm_apic_vapic_base_update;
     k->external_nmi = kvm_apic_external_nmi;
+
+    kac->parent_realize = dc->realize;
+    dc->realize = kvm_apic_realize;
 }
 
 static const TypeInfo kvm_apic_info = {
-    .name = "kvm-apic",
+    .name = TYPE_KVM_APIC,
     .parent = TYPE_APIC_COMMON,
     .instance_size = sizeof(APICCommonState),
     .class_init = kvm_apic_class_init,
+    .class_size = sizeof(KVMAPICClass),
 };
 
 static void kvm_apic_register_types(void)
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index a913186..0e0f71c 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -32,6 +32,20 @@
 #define SYNC_TO_VAPIC                   0x2
 #define SYNC_ISR_IRR_TO_VAPIC           0x4
 
+#define TYPE_APIC "apic"
+#define APIC_CLASS(class) OBJECT_CLASS_CHECK(APICClass, (class), TYPE_APIC)
+#define APIC_GET_CLASS(obj) OBJECT_GET_CLASS(APICClass, (obj), TYPE_APIC)
+
+/**
+ * APICClass:
+ * @parent_realize: The parent's realizefn
+ */
+typedef struct APICClass {
+    APICCommonClass parent_class;
+
+    DeviceRealize parent_realize;
+} APICClass;
+
 static APICCommonState *local_apics[MAX_APICS + 1];
 
 static void apic_set_irq(APICCommonState *s, int vector_num, int trigger_mode);
@@ -171,9 +185,9 @@ static void apic_local_deliver(APICCommonState *s, int 
vector)
     }
 }
 
-void apic_deliver_pic_intr(DeviceState *d, int level)
+void apic_deliver_pic_intr(DeviceState *dev, int level)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(dev);
 
     if (level) {
         apic_local_deliver(s, APIC_LVT_LINT0);
@@ -376,9 +390,9 @@ static void apic_update_irq(APICCommonState *s)
     }
 }
 
-void apic_poll_irq(DeviceState *d)
+void apic_poll_irq(DeviceState *dev)
 {
-    APICCommonState *s = APIC_COMMON(d);
+    APICCommonState *s = APIC_COMMON(dev);
 
     apic_sync_vapic(s, SYNC_FROM_VAPIC);
     apic_update_irq(s);
@@ -482,9 +496,9 @@ static void apic_startup(APICCommonState *s, int vector_num)
     cpu_interrupt(CPU(s->cpu), CPU_INTERRUPT_SIPI);
 }
 
-void apic_sipi(DeviceState *d)
+void apic_sipi(DeviceState *dev)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(dev);
 
     cpu_reset_interrupt(CPU(s->cpu), CPU_INTERRUPT_SIPI);
 
@@ -494,11 +508,11 @@ void apic_sipi(DeviceState *d)
     s->wait_for_sipi = 0;
 }
 
-static void apic_deliver(DeviceState *d, uint8_t dest, uint8_t dest_mode,
+static void apic_deliver(DeviceState *dev, uint8_t dest, uint8_t dest_mode,
                          uint8_t delivery_mode, uint8_t vector_num,
                          uint8_t trigger_mode)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(dev);
     uint32_t deliver_bitmask[MAX_APIC_WORDS];
     int dest_shorthand = (s->icr[0] >> 18) & 3;
     APICCommonState *apic_iter;
@@ -551,9 +565,9 @@ static bool apic_check_pic(APICCommonState *s)
     return true;
 }
 
-int apic_get_interrupt(DeviceState *d)
+int apic_get_interrupt(DeviceState *dev)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(dev);
     int intno;
 
     /* if the APIC is installed or enabled, we let the 8259 handle the
@@ -585,9 +599,9 @@ int apic_get_interrupt(DeviceState *d)
     return intno;
 }
 
-int apic_accept_pic_intr(DeviceState *d)
+int apic_accept_pic_intr(DeviceState *dev)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(dev);
     uint32_t lvt0;
 
     if (!s)
@@ -657,16 +671,16 @@ static void apic_mem_writew(void *opaque, hwaddr addr, 
uint32_t val)
 
 static uint32_t apic_mem_readl(void *opaque, hwaddr addr)
 {
-    DeviceState *d;
+    DeviceState *dev;
     APICCommonState *s;
     uint32_t val;
     int index;
 
-    d = cpu_get_current_apic();
-    if (!d) {
+    dev = cpu_get_current_apic();
+    if (!dev) {
         return 0;
     }
-    s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    s = APIC_COMMON(dev);
 
     index = (addr >> 4) & 0xff;
     switch(index) {
@@ -752,7 +766,7 @@ static void apic_send_msi(hwaddr addr, uint32_t data)
 
 static void apic_mem_writel(void *opaque, hwaddr addr, uint32_t val)
 {
-    DeviceState *d;
+    DeviceState *dev;
     APICCommonState *s;
     int index = (addr >> 4) & 0xff;
     if (addr > 0xfff || !index) {
@@ -765,11 +779,11 @@ static void apic_mem_writel(void *opaque, hwaddr addr, 
uint32_t val)
         return;
     }
 
-    d = cpu_get_current_apic();
-    if (!d) {
+    dev = cpu_get_current_apic();
+    if (!dev) {
         return;
     }
-    s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    s = APIC_COMMON(dev);
 
     trace_apic_mem_writel(addr, val);
 
@@ -810,7 +824,7 @@ static void apic_mem_writel(void *opaque, hwaddr addr, 
uint32_t val)
         break;
     case 0x30:
         s->icr[0] = val;
-        apic_deliver(d, (s->icr[1] >> 24) & 0xff, (s->icr[0] >> 11) & 1,
+        apic_deliver(dev, (s->icr[1] >> 24) & 0xff, (s->icr[0] >> 11) & 1,
                      (s->icr[0] >> 8) & 7, (s->icr[0] & 0xff),
                      (s->icr[0] >> 15) & 1);
         break;
@@ -871,8 +885,11 @@ static const MemoryRegionOps apic_io_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void apic_init(APICCommonState *s)
+static void apic_realize(DeviceState *dev, Error **errp)
 {
+    APICCommonState *s = APIC_COMMON(dev);
+    APICClass *ac = APIC_GET_CLASS(dev);
+
     memory_region_init_io(&s->io_memory, OBJECT(s), &apic_io_ops, s, 
"apic-msi",
                           APIC_SPACE_SIZE);
 
@@ -880,13 +897,15 @@ static void apic_init(APICCommonState *s)
     local_apics[s->idx] = s;
 
     msi_supported = true;
+    ac->parent_realize(dev, errp);
 }
 
 static void apic_class_init(ObjectClass *klass, void *data)
 {
     APICCommonClass *k = APIC_COMMON_CLASS(klass);
+    APICClass *ac = APIC_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
 
-    k->init = apic_init;
     k->set_base = apic_set_base;
     k->set_tpr = apic_set_tpr;
     k->get_tpr = apic_get_tpr;
@@ -894,13 +913,16 @@ static void apic_class_init(ObjectClass *klass, void 
*data)
     k->external_nmi = apic_external_nmi;
     k->pre_save = apic_pre_save;
     k->post_load = apic_post_load;
+    ac->parent_realize = dc->realize;
+    dc->realize = apic_realize;
 }
 
 static const TypeInfo apic_info = {
-    .name          = "apic",
+    .name          = TYPE_APIC,
     .instance_size = sizeof(APICCommonState),
     .parent        = TYPE_APIC_COMMON,
     .class_init    = apic_class_init,
+    .class_size    = sizeof(APICClass),
 };
 
 static void apic_register_types(void)
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index a0beb10..e28f996 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -27,21 +27,21 @@
 static int apic_irq_delivered;
 bool apic_report_tpr_access;
 
-void cpu_set_apic_base(DeviceState *d, uint64_t val)
+void cpu_set_apic_base(DeviceState *dev, uint64_t val)
 {
     trace_cpu_set_apic_base(val);
 
-    if (d) {
-        APICCommonState *s = APIC_COMMON(d);
+    if (dev) {
+        APICCommonState *s = APIC_COMMON(dev);
         APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
         info->set_base(s, val);
     }
 }
 
-uint64_t cpu_get_apic_base(DeviceState *d)
+uint64_t cpu_get_apic_base(DeviceState *dev)
 {
-    if (d) {
-        APICCommonState *s = APIC_COMMON(d);
+    if (dev) {
+        APICCommonState *s = APIC_COMMON(dev);
         trace_cpu_get_apic_base((uint64_t)s->apicbase);
         return s->apicbase;
     } else {
@@ -50,39 +50,39 @@ uint64_t cpu_get_apic_base(DeviceState *d)
     }
 }
 
-void cpu_set_apic_tpr(DeviceState *d, uint8_t val)
+void cpu_set_apic_tpr(DeviceState *dev, uint8_t val)
 {
     APICCommonState *s;
     APICCommonClass *info;
 
-    if (!d) {
+    if (!dev) {
         return;
     }
 
-    s = APIC_COMMON(d);
+    s = APIC_COMMON(dev);
     info = APIC_COMMON_GET_CLASS(s);
 
     info->set_tpr(s, val);
 }
 
-uint8_t cpu_get_apic_tpr(DeviceState *d)
+uint8_t cpu_get_apic_tpr(DeviceState *dev)
 {
     APICCommonState *s;
     APICCommonClass *info;
 
-    if (!d) {
+    if (!dev) {
         return 0;
     }
 
-    s = APIC_COMMON(d);
+    s = APIC_COMMON(dev);
     info = APIC_COMMON_GET_CLASS(s);
 
     return info->get_tpr(s);
 }
 
-void apic_enable_tpr_access_reporting(DeviceState *d, bool enable)
+void apic_enable_tpr_access_reporting(DeviceState *dev, bool enable)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(dev);
     APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
 
     apic_report_tpr_access = enable;
@@ -91,19 +91,19 @@ void apic_enable_tpr_access_reporting(DeviceState *d, bool 
enable)
     }
 }
 
-void apic_enable_vapic(DeviceState *d, hwaddr paddr)
+void apic_enable_vapic(DeviceState *dev, hwaddr paddr)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(dev);
     APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
 
     s->vapic_paddr = paddr;
     info->vapic_base_update(s);
 }
 
-void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip,
+void apic_handle_tpr_access_report(DeviceState *dev, target_ulong ip,
                                    TPRAccess access)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(dev);
 
     vapic_report_tpr_access(s->vapic, CPU(s->cpu), ip, access);
 }
@@ -129,9 +129,9 @@ int apic_get_irq_delivered(void)
     return apic_irq_delivered;
 }
 
-void apic_deliver_nmi(DeviceState *d)
+void apic_deliver_nmi(DeviceState *dev)
 {
-    APICCommonState *s = APIC_COMMON(d);
+    APICCommonState *s = APIC_COMMON(dev);
     APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
 
     info->external_nmi(s);
@@ -170,9 +170,9 @@ bool apic_next_timer(APICCommonState *s, int64_t 
current_time)
     return true;
 }
 
-void apic_init_reset(DeviceState *d)
+void apic_init_reset(DeviceState *dev)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(dev);
     int i;
 
     if (!s) {
@@ -203,19 +203,19 @@ void apic_init_reset(DeviceState *d)
     s->timer_expiry = -1;
 }
 
-void apic_designate_bsp(DeviceState *d)
+void apic_designate_bsp(DeviceState *dev)
 {
-    if (d == NULL) {
+    if (dev == NULL) {
         return;
     }
 
-    APICCommonState *s = APIC_COMMON(d);
+    APICCommonState *s = APIC_COMMON(dev);
     s->apicbase |= MSR_IA32_APICBASE_BSP;
 }
 
-static void apic_reset_common(DeviceState *d)
+static void apic_reset_common(DeviceState *dev)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(dev);
     APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
     bool bsp;
 
@@ -226,7 +226,7 @@ static void apic_reset_common(DeviceState *d)
     s->vapic_paddr = 0;
     info->vapic_base_update(s);
 
-    apic_init_reset(d);
+    apic_init_reset(dev);
 
     if (bsp) {
         /*
@@ -284,7 +284,7 @@ static int apic_load_old(QEMUFile *f, void *opaque, int 
version_id)
     return 0;
 }
 
-static int apic_init_common(ICCDevice *dev)
+static void apic_common_realize(DeviceState *dev, Error **errp)
 {
     APICCommonState *s = APIC_COMMON(dev);
     APICCommonClass *info;
@@ -293,12 +293,12 @@ static int apic_init_common(ICCDevice *dev)
     static bool mmio_registered;
 
     if (apic_no >= MAX_APICS) {
-        return -1;
+        return;
     }
     s->idx = apic_no++;
 
     info = APIC_COMMON_GET_CLASS(s);
-    info->init(s);
+
     if (!mmio_registered) {
         ICCBus *b = ICC_BUS(qdev_get_parent_bus(DEVICE(dev)));
         memory_region_add_subregion(b->apic_address_space, 0, &s->io_memory);
@@ -315,7 +315,6 @@ static int apic_init_common(ICCDevice *dev)
         info->enable_tpr_reporting(s, true);
     }
 
-    return 0;
 }
 
 static void apic_dispatch_pre_save(void *opaque)
@@ -381,14 +380,13 @@ static Property apic_properties_common[] = {
 
 static void apic_common_class_init(ObjectClass *klass, void *data)
 {
-    ICCDeviceClass *idc = ICC_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->vmsd = &vmstate_apic_common;
     dc->reset = apic_reset_common;
     dc->no_user = 1;
     dc->props = apic_properties_common;
-    idc->init = apic_init_common;
+    dc->realize = apic_common_realize;
 }
 
 static const TypeInfo apic_common_type = {
@@ -400,9 +398,9 @@ static const TypeInfo apic_common_type = {
     .abstract = true,
 };
 
-static void register_types(void)
+static void apic_common_register_types(void)
 {
     type_register_static(&apic_common_type);
 }
 
-type_init(register_types)
+type_init(apic_common_register_types)
diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index 1b0a7fb..d900f0a 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -80,7 +80,6 @@ typedef struct APICCommonClass
 {
     ICCDeviceClass parent_class;
 
-    void (*init)(APICCommonState *s);
     void (*set_base)(APICCommonState *s, uint64_t val);
     void (*set_tpr)(APICCommonState *s, uint8_t val);
     uint8_t (*get_tpr)(APICCommonState *s);
-- 
1.7.1