* [Qemu-devel] [PATCH 1/2] kvm/apic: use QOM style
@ 2013-09-27 5:30 xiaoqiang zhao
2013-09-27 5:30 ` [Qemu-devel] [PATCH 2/2] ioapic " xiaoqiang zhao
2013-11-04 6:41 ` [Qemu-devel] [PATCH 1/2] kvm/apic: " 赵小强
0 siblings, 2 replies; 5+ messages in thread
From: xiaoqiang zhao @ 2013-09-27 5:30 UTC (permalink / raw)
To: qemu-devel; +Cc: xiaoqiang.zhao, pbonzini, afaerber
From: "xiaoqiang.zhao" <zxq_yx_007@163.com>
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?
Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
---
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.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH 2/2] ioapic use QOM style
2013-09-27 5:30 [Qemu-devel] [PATCH 1/2] kvm/apic: use QOM style xiaoqiang zhao
@ 2013-09-27 5:30 ` xiaoqiang zhao
2013-11-04 6:41 ` [Qemu-devel] [PATCH 1/2] kvm/apic: " 赵小强
1 sibling, 0 replies; 5+ messages in thread
From: xiaoqiang zhao @ 2013-09-27 5:30 UTC (permalink / raw)
To: qemu-devel; +Cc: xiaoqiang zhao, pbonzini, afaerber
Change intc/ioapic kvm/ioapic to use QOM' realizefn.
To achive this, I move variable 'ioapic_no' from static to global.
Then, ioapic_realize and kvm_ioapic_realize can drop the 'instance_no' argument. Instead of parent, child increase ioapic_no now.
Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
---
hw/i386/kvm/ioapic.c | 35 ++++++++++++++++++++++++++-----
hw/intc/ioapic.c | 41 ++++++++++++++++++++++++++++++-------
hw/intc/ioapic_common.c | 17 +++++++++------
include/hw/i386/ioapic_internal.h | 1 -
4 files changed, 75 insertions(+), 19 deletions(-)
diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
index f11a540..b08865d 100644
--- a/hw/i386/kvm/ioapic.c
+++ b/hw/i386/kvm/ioapic.c
@@ -15,6 +15,22 @@
#include "hw/i386/apic_internal.h"
#include "sysemu/kvm.h"
+#define TYPE_KVM_IOAPIC "kvm-ioapic"
+#define KVM_IOAPIC_CLASS(class) \
+ OBJECT_CLASS_CHECK(KVMIOAPICClass, (class), TYPE_KVM_IOAPIC)
+#define KVM_IOAPIC_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(KVMIOAPICClass, (obj), TYPE_KVM_IOAPIC)
+
+/**
+ * KVMIOAPICClass:
+ * @parent_ralize: The parent's ralizefn
+ */
+typedef struct KVMIOAPICClass {
+ IOAPICCommonClass parent_class;
+
+ DeviceRealize parent_realize;
+} KVMIOAPICClass;
+
/* PC Utility function */
void kvm_pc_setup_irq_routing(bool pci_enabled)
{
@@ -127,11 +143,16 @@ static void kvm_ioapic_set_irq(void *opaque, int irq, int level)
apic_report_irq_delivered(delivered);
}
-static void kvm_ioapic_init(IOAPICCommonState *s, int instance_no)
+static void kvm_ioapic_realize(DeviceState *dev, Error **errp)
{
- memory_region_init_reservation(&s->io_memory, NULL, "kvm-ioapic", 0x1000);
+ IOAPICCommonState *s = IOAPIC_COMMON(dev);
+ KVMIOAPICClass *kic = KVM_IOAPIC_GET_CLASS(dev);
- qdev_init_gpio_in(DEVICE(s), kvm_ioapic_set_irq, IOAPIC_NUM_PINS);
+ memory_region_init_reservation(&s->io_memory, NULL, TYPE_KVM_IOAPIC, 0x1000);
+
+ qdev_init_gpio_in(dev, kvm_ioapic_set_irq, IOAPIC_NUM_PINS);
+
+ kic->parent_realize(dev, errp);
}
static Property kvm_ioapic_properties[] = {
@@ -143,19 +164,23 @@ static void kvm_ioapic_class_init(ObjectClass *klass, void *data)
{
IOAPICCommonClass *k = IOAPIC_COMMON_CLASS(klass);
DeviceClass *dc = DEVICE_CLASS(klass);
+ KVMIOAPICClass *kic = KVM_IOAPIC_CLASS(klass);
- k->init = kvm_ioapic_init;
k->pre_save = kvm_ioapic_get;
k->post_load = kvm_ioapic_put;
dc->reset = kvm_ioapic_reset;
dc->props = kvm_ioapic_properties;
+
+ kic->parent_realize = dc->realize;
+ dc->realize = kvm_ioapic_realize;
}
static const TypeInfo kvm_ioapic_info = {
- .name = "kvm-ioapic",
+ .name = TYPE_KVM_IOAPIC,
.parent = TYPE_IOAPIC_COMMON,
.instance_size = sizeof(KVMIOAPICState),
.class_init = kvm_ioapic_class_init,
+ .class_size = sizeof(KVMIOAPICClass),
};
static void kvm_ioapic_register_types(void)
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index d866e00..aeb3ed1 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -27,6 +27,24 @@
//#define DEBUG_IOAPIC
+#define TYPE_IOAPIC "ioapic"
+#define IOAPIC_CLASS(class) \
+ OBJECT_CLASS_CHECK(IOAPICClass, (class), TYPE_IOAPIC)
+#define IOAPIC_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(IOAPICClass, (obj), TYPE_IOAPIC)
+
+/**
+ * IOAPICClass:
+ * @parent_realize: The parent's ralizefn
+ */
+typedef struct IOAPICClass {
+ IOAPICCommonClass parent_class;
+
+ DeviceRealize parent_realize;
+} IOAPICClass;
+
+extern int ioapic_no;
+
#ifdef DEBUG_IOAPIC
#define DPRINTF(fmt, ...) \
do { printf("ioapic: " fmt , ## __VA_ARGS__); } while (0)
@@ -225,30 +243,39 @@ static const MemoryRegionOps ioapic_io_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
-static void ioapic_init(IOAPICCommonState *s, int instance_no)
+static void ioapic_realize(DeviceState *dev, Error **errp)
{
+ IOAPICCommonState *s = IOAPIC_COMMON(dev);
+ IOAPICClass *ic = IOAPIC_GET_CLASS(dev);
+
memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s,
- "ioapic", 0x1000);
+ TYPE_IOAPIC, 0x1000);
+
+ qdev_init_gpio_in(dev, ioapic_set_irq, IOAPIC_NUM_PINS);
- qdev_init_gpio_in(DEVICE(s), ioapic_set_irq, IOAPIC_NUM_PINS);
+ ioapics[ioapic_no] = s;
+ ic->parent_realize(dev, errp);
- ioapics[instance_no] = s;
+ /* increase the counter */
+ ioapic_no++;
}
static void ioapic_class_init(ObjectClass *klass, void *data)
{
- IOAPICCommonClass *k = IOAPIC_COMMON_CLASS(klass);
+ IOAPICClass *ic = IOAPIC_CLASS(klass);
DeviceClass *dc = DEVICE_CLASS(klass);
- k->init = ioapic_init;
dc->reset = ioapic_reset_common;
+ ic->parent_realize = dc->realize;
+ dc->realize = ioapic_realize;
}
static const TypeInfo ioapic_info = {
- .name = "ioapic",
+ .name = TYPE_IOAPIC,
.parent = TYPE_IOAPIC_COMMON,
.instance_size = sizeof(IOAPICCommonState),
.class_init = ioapic_class_init,
+ .class_size = sizeof(IOAPICClass),
};
static void ioapic_register_types(void)
diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
index 6b705c1..1dde813 100644
--- a/hw/intc/ioapic_common.c
+++ b/hw/intc/ioapic_common.c
@@ -23,6 +23,13 @@
#include "hw/i386/ioapic_internal.h"
#include "hw/sysbus.h"
+/* ioapic_no count start from 0 to MAX_IOAPICS
+ * remove from static variable in ioapic_common_init
+ * now as a global variable, let child to increase the counter
+ * then we can convert to our ralizefn style
+ */
+int ioapic_no = 0;
+
void ioapic_reset_common(DeviceState *dev)
{
IOAPICCommonState *s = IOAPIC_COMMON(dev);
@@ -61,7 +68,6 @@ static void ioapic_common_realize(DeviceState *dev, Error **errp)
{
IOAPICCommonState *s = IOAPIC_COMMON(dev);
IOAPICCommonClass *info;
- static int ioapic_no;
if (ioapic_no >= MAX_IOAPICS) {
error_setg(errp, "Only %d ioapics allowed", MAX_IOAPICS);
@@ -69,10 +75,9 @@ static void ioapic_common_realize(DeviceState *dev, Error **errp)
}
info = IOAPIC_COMMON_GET_CLASS(s);
- info->init(s, ioapic_no);
-
+
sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->io_memory);
- ioapic_no++;
+
}
static const VMStateDescription vmstate_ioapic_common = {
@@ -110,9 +115,9 @@ static const TypeInfo ioapic_common_type = {
.abstract = true,
};
-static void register_types(void)
+static void ioapic_common_register_types(void)
{
type_register_static(&ioapic_common_type);
}
-type_init(register_types)
+type_init(ioapic_common_register_types)
diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/i386/ioapic_internal.h
index 25576c8..34d77eb 100644
--- a/include/hw/i386/ioapic_internal.h
+++ b/include/hw/i386/ioapic_internal.h
@@ -83,7 +83,6 @@ typedef struct IOAPICCommonState IOAPICCommonState;
typedef struct IOAPICCommonClass {
SysBusDeviceClass parent_class;
- void (*init)(IOAPICCommonState *s, int instance_no);
void (*pre_save)(IOAPICCommonState *s);
void (*post_load)(IOAPICCommonState *s);
} IOAPICCommonClass;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] kvm/apic: use QOM style
2013-09-27 5:30 [Qemu-devel] [PATCH 1/2] kvm/apic: use QOM style xiaoqiang zhao
2013-09-27 5:30 ` [Qemu-devel] [PATCH 2/2] ioapic " xiaoqiang zhao
@ 2013-11-04 6:41 ` 赵小强
2013-11-04 17:23 ` Andreas Färber
1 sibling, 1 reply; 5+ messages in thread
From: 赵小强 @ 2013-11-04 6:41 UTC (permalink / raw)
To: xiaoqiang zhao; +Cc: pbonzini, qemu-devel, afaerber
于 09/27/2013 01:30 PM, xiaoqiang zhao 写道:
> From: "xiaoqiang.zhao" <zxq_yx_007@163.com>
>
> 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?
>
> Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
> ---
> 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);
ping.
patch link:
http://patchwork.ozlabs.org/patch/278481
http://patchwork.ozlabs.org/patch/278476
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] kvm/apic: use QOM style
2013-11-04 6:41 ` [Qemu-devel] [PATCH 1/2] kvm/apic: " 赵小强
@ 2013-11-04 17:23 ` Andreas Färber
2013-11-05 1:23 ` 赵小强
0 siblings, 1 reply; 5+ messages in thread
From: Andreas Färber @ 2013-11-04 17:23 UTC (permalink / raw)
To: 赵小强; +Cc: pbonzini, qemu-devel, Anthony Liguori
Hi,
Am 04.11.2013 07:41, schrieb 赵小强:
> 于 09/27/2013 01:30 PM, xiaoqiang zhao 写道:
>> From: "xiaoqiang.zhao" <zxq_yx_007@163.com>
>>
>> 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?
>>
>> Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
>> ---
>> 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(-)
[...]
> ping.
> patch link:
> http://patchwork.ozlabs.org/patch/278481
> http://patchwork.ozlabs.org/patch/278476
Thanks for reminding. A couple of issues with these two patches:
When sending more than one patch, please include a small cover letter.
That gives a better overview and gives you a central place to ping or us
to provide review comments covering both patches.
The DO_UPCAST() is a good cleanup, please put it in a separate patch to
decouple it from the rest.
As for d vs. dev, I've usually only changed it for the init/realize
functions I touched or when there was a new name conflict between
generic and specific device. But I'm fine with all your d changes. Just
please put them in a cleanup patch together with DO_UPCAST(), same for
the register_types prefix, then the actual "dangerous" patch becomes
smaller and thus easier to review for us. (Instead of 2 you'd have 4.)
The main issue though is that Anthony has requested to change the
pattern for overriding virtual methods: Can you please update the patch
so that void (*init)(APICCommonState *dev); simply gets replaced by
DeviceRealize realize;? I.e. drop the two new ...Class'es and
parent_realize, leave the call logic as it is now and just update the
function signature. Same for the ioapic.
For the second patch, I'd appreciate a second reviewer for the static
variable being refactored.
Note that you can pull the DEVICE(s) -> dev conversion into a preceding
cleanup patch by introducing a local DeviceState *dev = DEVICE(s).
Also there's some minor whitespace issues in ioapic_common_realize()
that we'll need to fix.
Thanks in advance and sorry for the delay,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] kvm/apic: use QOM style
2013-11-04 17:23 ` Andreas Färber
@ 2013-11-05 1:23 ` 赵小强
0 siblings, 0 replies; 5+ messages in thread
From: 赵小强 @ 2013-11-05 1:23 UTC (permalink / raw)
To: Andreas Färber; +Cc: pbonzini, qemu-devel, Anthony Liguori
于 11/05/2013 01:23 AM, Andreas Färber 写道:
> Hi,
>
> Am 04.11.2013 07:41, schrieb 赵小强:
>> 于 09/27/2013 01:30 PM, xiaoqiang zhao 写道:
>>> From: "xiaoqiang.zhao" <zxq_yx_007@163.com>
>>>
>>> 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?
>>>
>>> Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
>>> ---
>>> 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(-)
> [...]
>> ping.
>> patch link:
>> http://patchwork.ozlabs.org/patch/278481
>> http://patchwork.ozlabs.org/patch/278476
> Thanks for reminding. A couple of issues with these two patches:
>
> When sending more than one patch, please include a small cover letter.
> That gives a better overview and gives you a central place to ping or us
> to provide review comments covering both patches.
>
> The DO_UPCAST() is a good cleanup, please put it in a separate patch to
> decouple it from the rest.
>
> As for d vs. dev, I've usually only changed it for the init/realize
> functions I touched or when there was a new name conflict between
> generic and specific device. But I'm fine with all your d changes. Just
> please put them in a cleanup patch together with DO_UPCAST(), same for
> the register_types prefix, then the actual "dangerous" patch becomes
> smaller and thus easier to review for us. (Instead of 2 you'd have 4.)
>
> The main issue though is that Anthony has requested to change the
> pattern for overriding virtual methods: Can you please update the patch
> so that void (*init)(APICCommonState *dev); simply gets replaced by
> DeviceRealize realize;? I.e. drop the two new ...Class'es and
> parent_realize, leave the call logic as it is now and just update the
> function signature. Same for the ioapic.
>
> For the second patch, I'd appreciate a second reviewer for the static
> variable being refactored.
> Note that you can pull the DEVICE(s) -> dev conversion into a preceding
> cleanup patch by introducing a local DeviceState *dev = DEVICE(s).
> Also there's some minor whitespace issues in ioapic_common_realize()
> that we'll need to fix.
>
> Thanks in advance and sorry for the delay,
>
> Andreas
>
Thanks for your reply!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-11-05 1:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-27 5:30 [Qemu-devel] [PATCH 1/2] kvm/apic: use QOM style xiaoqiang zhao
2013-09-27 5:30 ` [Qemu-devel] [PATCH 2/2] ioapic " xiaoqiang zhao
2013-11-04 6:41 ` [Qemu-devel] [PATCH 1/2] kvm/apic: " 赵小强
2013-11-04 17:23 ` Andreas Färber
2013-11-05 1:23 ` 赵小强
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.