Re: [Qemu-devel] [PATCH RFC qom-cpu-next] apic: QOM'ify APICCommonState casts

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 6979 bytes --]

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;
>  
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]