From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43281) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VdDqn-0007qO-2Z for qemu-devel@nongnu.org; Mon, 04 Nov 2013 01:41:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VdDqh-0007fq-1Q for qemu-devel@nongnu.org; Mon, 04 Nov 2013 01:41:25 -0500 Received: from m50-132.163.com ([123.125.50.132]:52904) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VdDqf-0007fm-N6 for qemu-devel@nongnu.org; Mon, 04 Nov 2013 01:41:18 -0500 Message-ID: <52774193.3030908@163.com> Date: Mon, 04 Nov 2013 14:41:23 +0800 From: =?UTF-8?B?6LW15bCP5by6?= MIME-Version: 1.0 References: <1380259852-20797-1-git-send-email-zxq_yx_007@163.com> In-Reply-To: <1380259852-20797-1-git-send-email-zxq_yx_007@163.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 1/2] kvm/apic: use QOM style List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: xiaoqiang zhao Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, afaerber@suse.de 于 09/27/2013 01:30 PM, xiaoqiang zhao 写道: > From: "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? > > Signed-off-by: xiaoqiang zhao > --- > 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