qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v2 2/4] apic: QOM'ify apic & icc_bus


From: 赵小强
Subject: Re: [Qemu-devel] [PATCH v2 2/4] apic: QOM'ify apic & icc_bus
Date: Tue, 05 Nov 2013 16:51:47 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.0

于 2013年11月05日 16:25, Chen Fan 写道:
On Tue, 2013-11-05 at 15:55 +0800, xiaoqiang zhao wrote:
changes includes:
1. use type constant for apic and kvm_apic
2. convert function 'init' to QOM's 'realize' for apic/kvm_apic
3. for consistency, also QOM'ify apic's parent bus 'icc_bus'
---
  hw/cpu/icc_bus.c                |   14 ++++++--------
  hw/i386/kvm/apic.c              |   10 +++++++---
  hw/intc/apic.c                  |   10 +++++++---
  hw/intc/apic_common.c           |   13 +++++++------
  include/hw/cpu/icc_bus.h        |    3 ++-
  include/hw/i386/apic_internal.h |    5 +++--
  6 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
index 9a4ea7e..1cc64ac 100644
--- a/hw/cpu/icc_bus.c
+++ b/hw/cpu/icc_bus.c
@@ -43,15 +43,13 @@ static const TypeInfo icc_bus_info = {
static void icc_device_realize(DeviceState *dev, Error **errp)
  {
-    ICCDevice *id = ICC_DEVICE(dev);
-    ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(id);
-
-    if (idc->init) {
-        if (idc->init(id) < 0) {
-            error_setg(errp, "%s initialization failed.",
-                       object_get_typename(OBJECT(dev)));
-        }
+    ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(dev);
+
+    /* convert to QOM */
+    if (idc->realize) {
+       idc->realize(dev, errp);
      }
+
  }
static void icc_device_class_init(ObjectClass *oc, void *data)
diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 84f6056..55f4a53 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -13,6 +13,8 @@
  #include "hw/pci/msi.h"
  #include "sysemu/kvm.h"
+#define TYPE_KVM_APIC "kvm-apic"
+
  static inline void kvm_apic_set_reg(struct kvm_lapic_state *kapic,
                                      int reg_id, uint32_t val)
  {
@@ -171,8 +173,10 @@ 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);
+
      memory_region_init_io(&s->io_memory, NULL, &kvm_apic_io_ops, s, 
"kvm-apic-msi",
                            APIC_SPACE_SIZE);
@@ -185,7 +189,7 @@ static void kvm_apic_class_init(ObjectClass *klass, void *data)
  {
      APICCommonClass *k = APIC_COMMON_CLASS(klass);
- k->init = kvm_apic_init;
+    k->realize = kvm_apic_realize;
      k->set_base = kvm_apic_set_base;
      k->set_tpr = kvm_apic_set_tpr;
      k->get_tpr = kvm_apic_get_tpr;
@@ -195,7 +199,7 @@ static void kvm_apic_class_init(ObjectClass *klass, void 
*data)
  }
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,
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index b542628..2d7891d 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -32,6 +32,8 @@
  #define SYNC_TO_VAPIC                   0x2
  #define SYNC_ISR_IRR_TO_VAPIC           0x4
+#define TYPE_APIC "apic"
+
  static APICCommonState *local_apics[MAX_APICS + 1];
static void apic_set_irq(APICCommonState *s, int vector_num, int trigger_mode);
@@ -871,8 +873,10 @@ 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);
+
      memory_region_init_io(&s->io_memory, OBJECT(s), &apic_io_ops, s, 
"apic-msi",
                            APIC_SPACE_SIZE);
@@ -886,7 +890,7 @@ static void apic_class_init(ObjectClass *klass, void *data)
  {
      APICCommonClass *k = APIC_COMMON_CLASS(klass);
- k->init = apic_init;
+    k->realize = apic_realize;
      k->set_base = apic_set_base;
      k->set_tpr = apic_set_tpr;
      k->get_tpr = apic_get_tpr;
@@ -897,7 +901,7 @@ static void apic_class_init(ObjectClass *klass, void *data)
  }
static const TypeInfo apic_info = {
-    .name          = "apic",
+    .name          = TYPE_APIC,
      .instance_size = sizeof(APICCommonState),
      .parent        = TYPE_APIC_COMMON,
      .class_init    = apic_class_init,
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index f3edf48..5a413cc 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -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,14 +293,16 @@ static int apic_init_common(ICCDevice *dev)
      static bool mmio_registered;
if (apic_no >= MAX_APICS) {
-        return -1;
+       error_setg(errp, "%s initialization failed.",
          ^^
+                  object_get_typename(OBJECT(dev)));
+       return;
      }
Indentation looks bad.

      s->idx = apic_no++;
info = APIC_COMMON_GET_CLASS(s);
-    info->init(s);
+    info->realize(dev, errp);
      if (!mmio_registered) {
-        ICCBus *b = ICC_BUS(qdev_get_parent_bus(DEVICE(dev)));
+        ICCBus *b = ICC_BUS(qdev_get_parent_bus(dev));
          memory_region_add_subregion(b->apic_address_space, 0, &s->io_memory);
          mmio_registered = true;
      }
@@ -315,7 +317,6 @@ static int apic_init_common(ICCDevice *dev)
          info->enable_tpr_reporting(s, true);
      }
- return 0;
  }
static void apic_dispatch_pre_save(void *opaque)
@@ -388,7 +389,7 @@ static void apic_common_class_init(ObjectClass *klass, void 
*data)
      dc->reset = apic_reset_common;
      dc->no_user = 1;
      dc->props = apic_properties_common;
-    idc->init = apic_init_common;
+    idc->realize = apic_common_realize;
  }
static const TypeInfo apic_common_type = {
diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
index b550070..b32a549 100644
--- a/include/hw/cpu/icc_bus.h
+++ b/include/hw/cpu/icc_bus.h
@@ -66,7 +66,8 @@ typedef struct ICCDeviceClass {
      DeviceClass parent_class;
      /*< public >*/
- int (*init)(ICCDevice *dev); /* TODO replace with QOM realize */
+    /* QOM realize */
+    DeviceRealize realize;
Maybe adding 'realize' in ICCDeviceClass is redundant, because parent
class has included 'realize' field.

  } ICCDeviceClass;
#define TYPE_ICC_DEVICE "icc-device"
diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index 1b0a7fb..bd3a5fc 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -79,8 +79,9 @@ typedef struct APICCommonState APICCommonState;
  typedef struct APICCommonClass
  {
      ICCDeviceClass parent_class;
-
-    void (*init)(APICCommonState *s);
+
+    /* QOM realize */
+    DeviceRealize realize;
as above.

Thanks,
Chen
      void (*set_base)(APICCommonState *s, uint64_t val);
      void (*set_tpr)(APICCommonState *s, uint8_t val);
      uint8_t (*get_tpr)(APICCommonState *s);

Thanks for your review!!




reply via email to

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