diff mbox

[RFC,qom-cpu-next] apic: QOM'ify APICCommonState casts

Message ID 1367148131-1440-1-git-send-email-afaerber@suse.de
State New
Headers show

Commit Message

Andreas Färber April 28, 2013, 11:22 a.m. UTC 
Necessary to change the name of ICCDevice's parent object field.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 Could any of the APIC experts please review whether I'm touching any hot path?
 Not sure if this was a mix of pre- and post-QOM code or intentional... Thanks.

 hw/i386/kvm/apic.c       |  4 ++--
 hw/intc/apic.c           | 20 +++++++++++---------
 hw/intc/apic_common.c    | 10 +++++-----
 include/hw/cpu/icc_bus.h |  2 +-
 4 files changed, 19 insertions(+), 17 deletions(-)

Comments

Jan Kiszka April 28, 2013, 1:03 p.m. UTC | #1 
On 2013-04-28 13:22, Andreas Färber wrote:
> Necessary to change the name of ICCDevice's parent object field.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  Could any of the APIC experts please review whether I'm touching any hot path?
>  Not sure if this was a mix of pre- and post-QOM code or intentional... Thanks.

How "hot" does it have to be before we notice the type check overhead?
Did anyone already measure this, given that they are getting very common
now?

But none of the touched functions are used during normal operation when
the KVM APIC is active. With emulated APIC, they are frequently used, of
course.

Jan

> 
>  hw/i386/kvm/apic.c       |  4 ++--
>  hw/intc/apic.c           | 20 +++++++++++---------
>  hw/intc/apic_common.c    | 10 +++++-----
>  include/hw/cpu/icc_bus.h |  2 +-
>  4 files changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
> index 8f80425..0fe31ae 100644
> --- a/hw/i386/kvm/apic.c
> +++ b/hw/i386/kvm/apic.c
> @@ -27,7 +27,7 @@ static inline uint32_t kvm_apic_get_reg(struct kvm_lapic_state *kapic,
>  
>  void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
>  {
> -    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> +    APICCommonState *s = APIC_COMMON(d);
>      int i;
>  
>      memset(kapic, 0, sizeof(*kapic));
> @@ -53,7 +53,7 @@ void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
>  
>  void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
>  {
> -    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> +    APICCommonState *s = APIC_COMMON(d);
>      int i, v;
>  
>      s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> index 756dff0..f171620 100644
> --- a/hw/intc/apic.c
> +++ b/hw/intc/apic.c
> @@ -173,7 +173,7 @@ static void apic_local_deliver(APICCommonState *s, int vector)
>  
>  void apic_deliver_pic_intr(DeviceState *d, int level)
>  {
> -    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> +    APICCommonState *s = APIC_COMMON(d);
>  
>      if (level) {
>          apic_local_deliver(s, APIC_LVT_LINT0);
> @@ -484,7 +484,7 @@ static void apic_startup(APICCommonState *s, int vector_num)
>  
>  void apic_sipi(DeviceState *d)
>  {
> -    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> +    APICCommonState *s = APIC_COMMON(d);
>  
>      cpu_reset_interrupt(CPU(s->cpu), CPU_INTERRUPT_SIPI);
>  
> @@ -498,7 +498,7 @@ static void apic_deliver(DeviceState *d, 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(d);
>      uint32_t deliver_bitmask[MAX_APIC_WORDS];
>      int dest_shorthand = (s->icr[0] >> 18) & 3;
>      APICCommonState *apic_iter;
> @@ -544,16 +544,18 @@ static void apic_deliver(DeviceState *d, uint8_t dest, uint8_t dest_mode,
>  
>  static bool apic_check_pic(APICCommonState *s)
>  {
> -    if (!apic_accept_pic_intr(&s->busdev.qdev) || !pic_get_output(isa_pic)) {
> +    DeviceState *dev = DEVICE(s);
> +
> +    if (!apic_accept_pic_intr(dev) || !pic_get_output(isa_pic)) {
>          return false;
>      }
> -    apic_deliver_pic_intr(&s->busdev.qdev, 1);
> +    apic_deliver_pic_intr(dev, 1);
>      return true;
>  }
>  
>  int apic_get_interrupt(DeviceState *d)
>  {
> -    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> +    APICCommonState *s = APIC_COMMON(d);
>      int intno;
>  
>      /* if the APIC is installed or enabled, we let the 8259 handle the
> @@ -587,7 +589,7 @@ int apic_get_interrupt(DeviceState *d)
>  
>  int apic_accept_pic_intr(DeviceState *d)
>  {
> -    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> +    APICCommonState *s = APIC_COMMON(d);
>      uint32_t lvt0;
>  
>      if (!s)
> @@ -666,7 +668,7 @@ static uint32_t apic_mem_readl(void *opaque, hwaddr addr)
>      if (!d) {
>          return 0;
>      }
> -    s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> +    s = APIC_COMMON(d);
>  
>      index = (addr >> 4) & 0xff;
>      switch(index) {
> @@ -769,7 +771,7 @@ static void apic_mem_writel(void *opaque, hwaddr addr, uint32_t val)
>      if (!d) {
>          return;
>      }
> -    s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> +    s = APIC_COMMON(d);
>  
>      trace_apic_mem_writel(addr, val);
>  
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index b03e904..0087a14 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -82,7 +82,7 @@ uint8_t cpu_get_apic_tpr(DeviceState *d)
>  
>  void apic_enable_tpr_access_reporting(DeviceState *d, bool enable)
>  {
> -    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> +    APICCommonState *s = APIC_COMMON(d);
>      APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
>  
>      apic_report_tpr_access = enable;
> @@ -93,7 +93,7 @@ void apic_enable_tpr_access_reporting(DeviceState *d, bool enable)
>  
>  void apic_enable_vapic(DeviceState *d, hwaddr paddr)
>  {
> -    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> +    APICCommonState *s = APIC_COMMON(d);
>      APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
>  
>      s->vapic_paddr = paddr;
> @@ -103,7 +103,7 @@ void apic_enable_vapic(DeviceState *d, hwaddr paddr)
>  void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip,
>                                     TPRAccess access)
>  {
> -    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> +    APICCommonState *s = APIC_COMMON(d);
>  
>      vapic_report_tpr_access(s->vapic, CPU(s->cpu), ip, access);
>  }
> @@ -172,7 +172,7 @@ bool apic_next_timer(APICCommonState *s, int64_t current_time)
>  
>  void apic_init_reset(DeviceState *d)
>  {
> -    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> +    APICCommonState *s = APIC_COMMON(d);
>      int i;
>  
>      if (!s) {
> @@ -215,7 +215,7 @@ void apic_designate_bsp(DeviceState *d)
>  
>  static void apic_reset_common(DeviceState *d)
>  {
> -    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> +    APICCommonState *s = APIC_COMMON(d);
>      APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
>      bool bsp;
>  
> diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
> index b550070..f2c8a50 100644
> --- a/include/hw/cpu/icc_bus.h
> +++ b/include/hw/cpu/icc_bus.h
> @@ -51,7 +51,7 @@ typedef struct ICCBus {
>   */
>  typedef struct ICCDevice {
>      /*< private >*/
> -    DeviceState qdev;
> +    DeviceState parent_obj;
>      /*< public >*/
>  } ICCDevice;
>  
>
Andreas Färber April 28, 2013, 1:53 p.m. UTC | #2 
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 28.04.2013 15:03, schrieb Jan Kiszka:
> On 2013-04-28 13:22, Andreas Färber wrote:
>> Necessary to change the name of ICCDevice's parent object field.
>> 
>> Signed-off-by: Andreas Färber <afaerber@suse.de> --- Could any of
>> the APIC experts please review whether I'm touching any hot
>> path? Not sure if this was a mix of pre- and post-QOM code or
>> intentional... Thanks.
> 
> How "hot" does it have to be before we notice the type check
> overhead? Did anyone already measure this, given that they are
> getting very common now?

Heh, if I had a conclusive benchmark I wouldn't ask. ;)

In general init, reset and MMIO were considered cold paths in this
regard. Timer and interrupt code paths by contrast I've usually tried
to spare.

> But none of the touched functions are used during normal operation
> when the KVM APIC is active. With emulated APIC, they are
> frequently used, of course.

Where in doubt, we could just use (APICCommonState *) or a macro
hiding that.

Andreas

>> hw/i386/kvm/apic.c       |  4 ++-- hw/intc/apic.c           | 20
>> +++++++++++--------- hw/intc/apic_common.c    | 10 +++++----- 
>> include/hw/cpu/icc_bus.h |  2 +- 4 files changed, 19
>> insertions(+), 17 deletions(-)
>> 
>> diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c index
>> 8f80425..0fe31ae 100644 --- a/hw/i386/kvm/apic.c +++
>> b/hw/i386/kvm/apic.c @@ -27,7 +27,7 @@ static inline uint32_t
>> kvm_apic_get_reg(struct kvm_lapic_state *kapic,
>> 
>> void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state
>> *kapic) { -    APICCommonState *s = DO_UPCAST(APICCommonState,
>> busdev.qdev, d); +    APICCommonState *s = APIC_COMMON(d); int
>> i;
>> 
>> memset(kapic, 0, sizeof(*kapic)); @@ -53,7 +53,7 @@ void
>> kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state
>> *kapic)
>> 
>> void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state
>> *kapic) { -    APICCommonState *s = DO_UPCAST(APICCommonState,
>> busdev.qdev, d); +    APICCommonState *s = APIC_COMMON(d); int i,
>> v;
>> 
>> s->id = kvm_apic_get_reg(kapic, 0x2) >> 24; diff --git
>> a/hw/intc/apic.c b/hw/intc/apic.c index 756dff0..f171620 100644 
>> --- a/hw/intc/apic.c +++ b/hw/intc/apic.c @@ -173,7 +173,7 @@
>> static void apic_local_deliver(APICCommonState *s, int vector)
>> 
>> void apic_deliver_pic_intr(DeviceState *d, int level) { -
>> APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d); 
>> +    APICCommonState *s = APIC_COMMON(d);
>> 
>> if (level) { apic_local_deliver(s, APIC_LVT_LINT0); @@ -484,7
>> +484,7 @@ static void apic_startup(APICCommonState *s, int
>> vector_num)
>> 
>> void apic_sipi(DeviceState *d) { -    APICCommonState *s =
>> DO_UPCAST(APICCommonState, busdev.qdev, d); +    APICCommonState
>> *s = APIC_COMMON(d);
>> 
>> cpu_reset_interrupt(CPU(s->cpu), CPU_INTERRUPT_SIPI);
>> 
>> @@ -498,7 +498,7 @@ static void apic_deliver(DeviceState *d,
>> 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(d); uint32_t deliver_bitmask[MAX_APIC_WORDS]; 
>> int dest_shorthand = (s->icr[0] >> 18) & 3; APICCommonState
>> *apic_iter; @@ -544,16 +544,18 @@ static void
>> apic_deliver(DeviceState *d, uint8_t dest, uint8_t dest_mode,
>> 
>> static bool apic_check_pic(APICCommonState *s) { -    if
>> (!apic_accept_pic_intr(&s->busdev.qdev) ||
>> !pic_get_output(isa_pic)) { +    DeviceState *dev = DEVICE(s); + 
>> +    if (!apic_accept_pic_intr(dev) || !pic_get_output(isa_pic))
>> { return false; } -    apic_deliver_pic_intr(&s->busdev.qdev,
>> 1); +    apic_deliver_pic_intr(dev, 1); return true; }
>> 
>> int apic_get_interrupt(DeviceState *d) { -    APICCommonState *s
>> = DO_UPCAST(APICCommonState, busdev.qdev, d); +
>> APICCommonState *s = APIC_COMMON(d); int intno;
>> 
>> /* if the APIC is installed or enabled, we let the 8259 handle
>> the @@ -587,7 +589,7 @@ int apic_get_interrupt(DeviceState *d)
>> 
>> int apic_accept_pic_intr(DeviceState *d) { -    APICCommonState
>> *s = DO_UPCAST(APICCommonState, busdev.qdev, d); +
>> APICCommonState *s = APIC_COMMON(d); uint32_t lvt0;
>> 
>> if (!s) @@ -666,7 +668,7 @@ static uint32_t apic_mem_readl(void
>> *opaque, hwaddr addr) if (!d) { return 0; } -    s =
>> DO_UPCAST(APICCommonState, busdev.qdev, d); +    s =
>> APIC_COMMON(d);
>> 
>> index = (addr >> 4) & 0xff; switch(index) { @@ -769,7 +771,7 @@
>> static void apic_mem_writel(void *opaque, hwaddr addr, uint32_t
>> val) if (!d) { return; } -    s = DO_UPCAST(APICCommonState,
>> busdev.qdev, d); +    s = APIC_COMMON(d);
>> 
>> trace_apic_mem_writel(addr, val);
>> 
>> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c index
>> b03e904..0087a14 100644 --- a/hw/intc/apic_common.c +++
>> b/hw/intc/apic_common.c @@ -82,7 +82,7 @@ uint8_t
>> cpu_get_apic_tpr(DeviceState *d)
>> 
>> void apic_enable_tpr_access_reporting(DeviceState *d, bool
>> enable) { -    APICCommonState *s = DO_UPCAST(APICCommonState,
>> busdev.qdev, d); +    APICCommonState *s = APIC_COMMON(d); 
>> APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
>> 
>> apic_report_tpr_access = enable; @@ -93,7 +93,7 @@ void
>> apic_enable_tpr_access_reporting(DeviceState *d, bool enable)
>> 
>> void apic_enable_vapic(DeviceState *d, hwaddr paddr) { -
>> APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d); 
>> +    APICCommonState *s = APIC_COMMON(d); APICCommonClass *info =
>> APIC_COMMON_GET_CLASS(s);
>> 
>> s->vapic_paddr = paddr; @@ -103,7 +103,7 @@ void
>> apic_enable_vapic(DeviceState *d, hwaddr paddr) void
>> apic_handle_tpr_access_report(DeviceState *d, target_ulong ip, 
>> TPRAccess access) { -    APICCommonState *s =
>> DO_UPCAST(APICCommonState, busdev.qdev, d); +    APICCommonState
>> *s = APIC_COMMON(d);
>> 
>> vapic_report_tpr_access(s->vapic, CPU(s->cpu), ip, access); } @@
>> -172,7 +172,7 @@ bool apic_next_timer(APICCommonState *s, int64_t
>> current_time)
>> 
>> void apic_init_reset(DeviceState *d) { -    APICCommonState *s =
>> DO_UPCAST(APICCommonState, busdev.qdev, d); +    APICCommonState
>> *s = APIC_COMMON(d); int i;
>> 
>> if (!s) { @@ -215,7 +215,7 @@ void apic_designate_bsp(DeviceState
>> *d)
>> 
>> static void apic_reset_common(DeviceState *d) { -
>> APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d); 
>> +    APICCommonState *s = APIC_COMMON(d); APICCommonClass *info =
>> APIC_COMMON_GET_CLASS(s); bool bsp;
>> 
>> diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h 
>> index b550070..f2c8a50 100644 --- a/include/hw/cpu/icc_bus.h +++
>> b/include/hw/cpu/icc_bus.h @@ -51,7 +51,7 @@ typedef struct
>> ICCBus { */ typedef struct ICCDevice { /*< private >*/ -
>> DeviceState qdev; +    DeviceState parent_obj; /*< public >*/ }
>> ICCDevice;
>> 
>> 
> 
> 


- -- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)

iQIcBAEBAgAGBQJRfSnoAAoJEPou0S0+fgE/NcgP/iJ99zuWVy+OG13Zr+gpGUu7
gL+GjPjKAo1QYTGBEtpB1g8veyXaVbDWCHzkGsitQRtIRqCJnwQcORcWes395J9N
ffy/fWUSwdzOdv4DP+hPoy8RaFdxKWr4JqUsoSLERM7vefQ86xieWvE7RW6Xcy+r
SDE4Qm67WWyGN5c875deF1/9LlT08vv0KCXzaqMLdm5y17c5td7h82JNNlYfYF9S
QeyPYOtWvEFR4uUaLqgoMbWpMjvobjt5G/tbSss1wfds9fiBM4nU+Blk51eF2N0v
VeNCO5NyNnQyiNR7znwYYnljNvg7Web9ITenCQGPj0eUDdAyb1kBwqCDZo5giqp4
ifvTVi4LAq0vCmBArKDeHpi0xZXPXDLm2QREdO80Qwnt9y2fieV3mMMTXl1X6vSp
R0dmGSbomgEoigQMXSHOdiz4M6CHphpUicG8/05op9Sjf5DVkfaDt7isvvzKifNL
K+vFogG3DLRyL6iw10H2B9A7e4mY/b6VCw+XvzhZa5iiZ7qa/ZVEt84uJQJNXxnv
Wd0rR5nlCdUJBq8IbDyJm80P+bnzgKU+Wfa4wOzxeDEWLJj4bRPdZd+I9mbFEgkf
rtseRM+vjooQhyTE6l/uphX94kzoOAVtwJefo9GXhhduZ26fwVepVsqH4q1Bie+G
xu50SmEmd2N4aw9VaRF9
=qCIU
-----END PGP SIGNATURE-----
Jan Kiszka April 28, 2013, 2:13 p.m. UTC | #3 
On 2013-04-28 15:53, Andreas Färber wrote:
> Am 28.04.2013 15:03, schrieb Jan Kiszka:
>> On 2013-04-28 13:22, Andreas Färber wrote:
>>> Necessary to change the name of ICCDevice's parent object field.
>>>
>>> Signed-off-by: Andreas Färber <afaerber@suse.de> --- Could any of
>>> the APIC experts please review whether I'm touching any hot
>>> path? Not sure if this was a mix of pre- and post-QOM code or
>>> intentional... Thanks.
> 
>> How "hot" does it have to be before we notice the type check
>> overhead? Did anyone already measure this, given that they are
>> getting very common now?
> 
> Heh, if I had a conclusive benchmark I wouldn't ask. ;)

Do object_class_dynamic_cast & friends show up higher in profiling
results when using your patch with an emulated APIC?

> 
> In general init, reset and MMIO were considered cold paths in this
> regard. Timer and interrupt code paths by contrast I've usually tried
> to spare.

...but not in this patch. There are things like "deliver",
"get_interupt", "accept", and the APIC MMIO handlers are stressed at
least once per IRQ as well (EOI). When emulating in userspace.

> 
>> But none of the touched functions are used during normal operation
>> when the KVM APIC is active. With emulated APIC, they are
>> frequently used, of course.
> 
> Where in doubt, we could just use (APICCommonState *) or a macro
> hiding that.

Actually, I would prefer making the typical type check (class up- and
down-casts) so light-weight that we do not have to worry. Something that
ideally boils down to comparing two magic numbers inline.

Jan
Igor Mammedov April 29, 2013, 10:29 a.m. UTC | #4 
On Sun, 28 Apr 2013 13:22:10 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Necessary to change the name of ICCDevice's parent object field.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  Could any of the APIC experts please review whether I'm touching any hot path?
>  Not sure if this was a mix of pre- and post-QOM code or intentional... Thanks.
> 
>  hw/i386/kvm/apic.c       |  4 ++--
>  hw/intc/apic.c           | 20 +++++++++++---------
>  hw/intc/apic_common.c    | 10 +++++-----
>  include/hw/cpu/icc_bus.h |  2 +-
>  4 files changed, 19 insertions(+), 17 deletions(-)
> 
[...]
>  
> diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
> index b550070..f2c8a50 100644
> --- a/include/hw/cpu/icc_bus.h
> +++ b/include/hw/cpu/icc_bus.h
> @@ -51,7 +51,7 @@ typedef struct ICCBus {
>   */
>  typedef struct ICCDevice {
>      /*< private >*/
> -    DeviceState qdev;
> +    DeviceState parent_obj;
This hunk doesn't apply to qpm-cpu-next.
It looks like it's accidentally crept in 
https://github.com/afaerber/qemu-cpu/commit/a7ae7cd073cc2b20de3372edc84ec5a01e88e55d


>      /*< public >*/
>  } ICCDevice;
>  
> -- 
> 1.8.1.4
> 
>
Andreas Färber April 29, 2013, 11:30 a.m. UTC | #5 
Am 29.04.2013 12:29, schrieb Igor Mammedov:
> On Sun, 28 Apr 2013 13:22:10 +0200
> Andreas Färber <afaerber@suse.de> wrote:
> 
>> Necessary to change the name of ICCDevice's parent object field.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>>  Could any of the APIC experts please review whether I'm touching any hot path?
>>  Not sure if this was a mix of pre- and post-QOM code or intentional... Thanks.
>>
>>  hw/i386/kvm/apic.c       |  4 ++--
>>  hw/intc/apic.c           | 20 +++++++++++---------
>>  hw/intc/apic_common.c    | 10 +++++-----
>>  include/hw/cpu/icc_bus.h |  2 +-
>>  4 files changed, 19 insertions(+), 17 deletions(-)
>>
> [...]
>>  
>> diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
>> index b550070..f2c8a50 100644
>> --- a/include/hw/cpu/icc_bus.h
>> +++ b/include/hw/cpu/icc_bus.h
>> @@ -51,7 +51,7 @@ typedef struct ICCBus {
>>   */
>>  typedef struct ICCDevice {
>>      /*< private >*/
>> -    DeviceState qdev;
>> +    DeviceState parent_obj;
> This hunk doesn't apply to qpm-cpu-next.
> It looks like it's accidentally crept in 
> https://github.com/afaerber/qemu-cpu/commit/a7ae7cd073cc2b20de3372edc84ec5a01e88e55d

Yeah, sorry, forgot to push latest qom-cpu-next (but today that includes
preliminary APIC QOM'ification not yet ready for qom-cpu).

Andreas
diff mbox

Patch

diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 8f80425..0fe31ae 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -27,7 +27,7 @@  static inline uint32_t kvm_apic_get_reg(struct kvm_lapic_state *kapic,
 
 void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(d);
     int i;
 
     memset(kapic, 0, sizeof(*kapic));
@@ -53,7 +53,7 @@  void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
 
 void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(d);
     int i, v;
 
     s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 756dff0..f171620 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -173,7 +173,7 @@  static void apic_local_deliver(APICCommonState *s, int vector)
 
 void apic_deliver_pic_intr(DeviceState *d, int level)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(d);
 
     if (level) {
         apic_local_deliver(s, APIC_LVT_LINT0);
@@ -484,7 +484,7 @@  static void apic_startup(APICCommonState *s, int vector_num)
 
 void apic_sipi(DeviceState *d)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(d);
 
     cpu_reset_interrupt(CPU(s->cpu), CPU_INTERRUPT_SIPI);
 
@@ -498,7 +498,7 @@  static void apic_deliver(DeviceState *d, 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(d);
     uint32_t deliver_bitmask[MAX_APIC_WORDS];
     int dest_shorthand = (s->icr[0] >> 18) & 3;
     APICCommonState *apic_iter;
@@ -544,16 +544,18 @@  static void apic_deliver(DeviceState *d, uint8_t dest, uint8_t dest_mode,
 
 static bool apic_check_pic(APICCommonState *s)
 {
-    if (!apic_accept_pic_intr(&s->busdev.qdev) || !pic_get_output(isa_pic)) {
+    DeviceState *dev = DEVICE(s);
+
+    if (!apic_accept_pic_intr(dev) || !pic_get_output(isa_pic)) {
         return false;
     }
-    apic_deliver_pic_intr(&s->busdev.qdev, 1);
+    apic_deliver_pic_intr(dev, 1);
     return true;
 }
 
 int apic_get_interrupt(DeviceState *d)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(d);
     int intno;
 
     /* if the APIC is installed or enabled, we let the 8259 handle the
@@ -587,7 +589,7 @@  int apic_get_interrupt(DeviceState *d)
 
 int apic_accept_pic_intr(DeviceState *d)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(d);
     uint32_t lvt0;
 
     if (!s)
@@ -666,7 +668,7 @@  static uint32_t apic_mem_readl(void *opaque, hwaddr addr)
     if (!d) {
         return 0;
     }
-    s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    s = APIC_COMMON(d);
 
     index = (addr >> 4) & 0xff;
     switch(index) {
@@ -769,7 +771,7 @@  static void apic_mem_writel(void *opaque, hwaddr addr, uint32_t val)
     if (!d) {
         return;
     }
-    s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    s = APIC_COMMON(d);
 
     trace_apic_mem_writel(addr, val);
 
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index b03e904..0087a14 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -82,7 +82,7 @@  uint8_t cpu_get_apic_tpr(DeviceState *d)
 
 void apic_enable_tpr_access_reporting(DeviceState *d, bool enable)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(d);
     APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
 
     apic_report_tpr_access = enable;
@@ -93,7 +93,7 @@  void apic_enable_tpr_access_reporting(DeviceState *d, bool enable)
 
 void apic_enable_vapic(DeviceState *d, hwaddr paddr)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(d);
     APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
 
     s->vapic_paddr = paddr;
@@ -103,7 +103,7 @@  void apic_enable_vapic(DeviceState *d, hwaddr paddr)
 void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip,
                                    TPRAccess access)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(d);
 
     vapic_report_tpr_access(s->vapic, CPU(s->cpu), ip, access);
 }
@@ -172,7 +172,7 @@  bool apic_next_timer(APICCommonState *s, int64_t current_time)
 
 void apic_init_reset(DeviceState *d)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(d);
     int i;
 
     if (!s) {
@@ -215,7 +215,7 @@  void apic_designate_bsp(DeviceState *d)
 
 static void apic_reset_common(DeviceState *d)
 {
-    APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    APICCommonState *s = APIC_COMMON(d);
     APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
     bool bsp;
 
diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
index b550070..f2c8a50 100644
--- a/include/hw/cpu/icc_bus.h
+++ b/include/hw/cpu/icc_bus.h
@@ -51,7 +51,7 @@  typedef struct ICCBus {
  */
 typedef struct ICCDevice {
     /*< private >*/
-    DeviceState qdev;
+    DeviceState parent_obj;
     /*< public >*/
 } ICCDevice;