From: 赵小强 <zxq_yx_007@163.com>
To: xiaoqiang zhao <zxq_yx_007@163.com>
Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, afaerber@suse.de
Subject: Re: [Qemu-devel] [PATCH 1/2] kvm/apic: use QOM style
Date: Mon, 04 Nov 2013 14:41:23 +0800 [thread overview]
Message-ID: <52774193.3030908@163.com> (raw)
In-Reply-To: <1380259852-20797-1-git-send-email-zxq_yx_007@163.com>
于 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
next prev parent reply other threads:[~2013-11-04 6:41 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
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 ` 赵小强 [this message]
2013-11-04 17:23 ` [Qemu-devel] [PATCH 1/2] kvm/apic: " Andreas Färber
2013-11-05 1:23 ` 赵小强
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52774193.3030908@163.com \
--to=zxq_yx_007@163.com \
--cc=afaerber@suse.de \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.