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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).