Re: [Qemu-devel] [PATCH v2 5/6] intc: make HMP 'info irq' and 'info pic' commands use InterruptStatsProvider interface

[Paolo Bonzini]

On 26/09/2016 22:23, Hervé Poussineau wrote:
> Signed-off-by: Hervé Poussineau <address@hidden>
> ---
>  hmp-commands-info.hx       | 12 ---------
>  hmp.c                      | 65 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  hmp.h                      |  2 ++
>  hw/intc/i8259.c            | 36 -------------------------
>  hw/intc/lm32_pic.c         | 31 ----------------------
>  hw/intc/slavio_intctl.c    | 32 -----------------------
>  hw/sparc/sun4m.c           | 15 +----------
>  include/hw/i386/pc.h       |  2 --
>  include/hw/lm32/lm32_pic.h |  3 ---
>  include/hw/sparc/sun4m.h   |  8 ------
>  monitor.c                  |  6 -----
>  11 files changed, 68 insertions(+), 144 deletions(-)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 19729e5..6a7c476 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -179,13 +179,7 @@ ETEXI
>          .args_type  = "",
>          .params     = "",
>          .help       = "show the interrupts statistics (if available)",
> -#ifdef TARGET_SPARC
> -        .cmd        = sun4m_hmp_info_irq,
> -#elif defined(TARGET_LM32)
> -        .cmd        = lm32_hmp_info_irq,
> -#else
>          .cmd        = hmp_info_irq,
> -#endif
>      },
>  
>  STEXI
> @@ -199,13 +193,7 @@ ETEXI
>          .args_type  = "",
>          .params     = "",
>          .help       = "show i8259 (PIC) state",
> -#ifdef TARGET_SPARC
> -        .cmd        = sun4m_hmp_info_pic,
> -#elif defined(TARGET_LM32)
> -        .cmd        = lm32_hmp_info_pic,
> -#else
>          .cmd        = hmp_info_pic,
> -#endif
>      },
>  #endif
>  
> diff --git a/hmp.c b/hmp.c
> index 336e7bf..ec2e9ce 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -36,6 +36,7 @@
>  #include "qemu-io.h"
>  #include "qemu/cutils.h"
>  #include "qemu/error-report.h"
> +#include "hw/intc/intc.h"
>  
>  #ifdef CONFIG_SPICE
>  #include <spice/enums.h>
> @@ -787,6 +788,70 @@ static void hmp_info_pci_device(Monitor *mon, const 
> PciDeviceInfo *dev)
>      }
>  }
>  
> +static int hmp_info_irq_foreach(Object *obj, void *opaque)
> +{
> +    InterruptStatsProvider *intc;
> +    InterruptStatsProviderClass *k;
> +    Monitor *mon = opaque;
> +
> +    if (object_dynamic_cast(obj, TYPE_INTERRUPT_STATS_PROVIDER)) {
> +        intc = INTERRUPT_STATS_PROVIDER(obj);
> +        k = INTERRUPT_STATS_PROVIDER_GET_CLASS(obj);
> +        uint64_t *irq_counts;
> +        unsigned int nb_irqs, i;
> +        if (k->get_statistics &&
> +            k->get_statistics(intc, &irq_counts, &nb_irqs)) {
> +            if (nb_irqs > 0) {
> +                monitor_printf(mon, "IRQ statistics for %s:\n",
> +                               object_get_typename(obj));
> +                for (i = 0; i < nb_irqs; i++) {
> +                    if (irq_counts[i] > 0) {
> +                        monitor_printf(mon, "%2d: %" PRId64 "\n", i,
> +                                       irq_counts[i]);
> +                    }
> +                }
> +            }
> +        } else {
> +            monitor_printf(mon, "IRQ statistics not available for %s.\n",
> +                           object_get_typename(obj));
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +void hmp_info_irq(Monitor *mon, const QDict *qdict)
> +{
> +    object_child_foreach_recursive(object_get_root(),
> +                                   hmp_info_irq_foreach, mon);
> +}
> +
> +static int hmp_info_pic_foreach(Object *obj, void *opaque)
> +{
> +    InterruptStatsProvider *intc;
> +    InterruptStatsProviderClass *k;
> +    Monitor *mon = opaque;
> +
> +    if (object_dynamic_cast(obj, TYPE_INTERRUPT_STATS_PROVIDER)) {
> +        intc = INTERRUPT_STATS_PROVIDER(obj);
> +        k = INTERRUPT_STATS_PROVIDER_GET_CLASS(obj);
> +        if (k->print_info) {
> +            k->print_info(intc, mon);
> +        } else {
> +            monitor_printf(mon, "PIC informations not available for %s.\n",

Replacing with "Interrupt controller information", otherwise I'm queuing
the series.

Paolo

> +                           object_get_typename(obj));
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +void hmp_info_pic(Monitor *mon, const QDict *qdict)
> +{
> +    object_child_foreach_recursive(object_get_root(),
> +                                   hmp_info_pic_foreach, mon);
> +}
> +
>  void hmp_info_pci(Monitor *mon, const QDict *qdict)
>  {
>      PciInfoList *info_list, *info;
> diff --git a/hmp.h b/hmp.h
> index 0876ec0..184769c 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -36,6 +36,8 @@ void hmp_info_blockstats(Monitor *mon, const QDict *qdict);
>  void hmp_info_vnc(Monitor *mon, const QDict *qdict);
>  void hmp_info_spice(Monitor *mon, const QDict *qdict);
>  void hmp_info_balloon(Monitor *mon, const QDict *qdict);
> +void hmp_info_irq(Monitor *mon, const QDict *qdict);
> +void hmp_info_pic(Monitor *mon, const QDict *qdict);
>  void hmp_info_pci(Monitor *mon, const QDict *qdict);
>  void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
>  void hmp_info_tpm(Monitor *mon, const QDict *qdict);
> diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c
> index 75c8d22..fe9ecd6 100644
> --- a/hw/intc/i8259.c
> +++ b/hw/intc/i8259.c
> @@ -461,42 +461,6 @@ static void pic_realize(DeviceState *dev, Error **errp)
>      pc->parent_realize(dev, errp);
>  }
>  
> -void hmp_info_pic(Monitor *mon, const QDict *qdict)
> -{
> -    int i;
> -    PICCommonState *s;
> -
> -    if (!isa_pic) {
> -        return;
> -    }
> -    for (i = 0; i < 2; i++) {
> -        s = i == 0 ? PIC_COMMON(isa_pic) : slave_pic;
> -        monitor_printf(mon, "pic%d: irr=%02x imr=%02x isr=%02x hprio=%d "
> -                       "irq_base=%02x rr_sel=%d elcr=%02x fnm=%d\n",
> -                       i, s->irr, s->imr, s->isr, s->priority_add,
> -                       s->irq_base, s->read_reg_select, s->elcr,
> -                       s->special_fully_nested_mode);
> -    }
> -}
> -
> -void hmp_info_irq(Monitor *mon, const QDict *qdict)
> -{
> -#ifndef DEBUG_IRQ_COUNT
> -    monitor_printf(mon, "irq statistic code not compiled.\n");
> -#else
> -    int i;
> -    int64_t count;
> -
> -    monitor_printf(mon, "IRQ statistics:\n");
> -    for (i = 0; i < 16; i++) {
> -        count = irq_count[i];
> -        if (count > 0) {
> -            monitor_printf(mon, "%2d: %" PRId64 "\n", i, count);
> -        }
> -    }
> -#endif
> -}
> -
>  qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq)
>  {
>      qemu_irq *irq_set;
> diff --git a/hw/intc/lm32_pic.c b/hw/intc/lm32_pic.c
> index c045b99..09e1511 100644
> --- a/hw/intc/lm32_pic.c
> +++ b/hw/intc/lm32_pic.c
> @@ -43,35 +43,6 @@ struct LM32PicState {
>  };
>  typedef struct LM32PicState LM32PicState;
>  
> -static LM32PicState *pic;
> -void lm32_hmp_info_pic(Monitor *mon, const QDict *qdict)
> -{
> -    if (pic == NULL) {
> -        return;
> -    }
> -
> -    monitor_printf(mon, "lm32-pic: im=%08x ip=%08x irq_state=%08x\n",
> -            pic->im, pic->ip, pic->irq_state);
> -}
> -
> -void lm32_hmp_info_irq(Monitor *mon, const QDict *qdict)
> -{
> -    int i;
> -    uint32_t count;
> -
> -    if (pic == NULL) {
> -        return;
> -    }
> -
> -    monitor_printf(mon, "IRQ statistics:\n");
> -    for (i = 0; i < 32; i++) {
> -        count = pic->stats_irq_count[i];
> -        if (count > 0) {
> -            monitor_printf(mon, "%2d: %u\n", i, count);
> -        }
> -    }
> -}
> -
>  static void update_irq(LM32PicState *s)
>  {
>      s->ip |= s->irq_state;
> @@ -177,8 +148,6 @@ static void lm32_pic_init(Object *obj)
>  
>      qdev_init_gpio_in(dev, irq_handler, 32);
>      sysbus_init_irq(sbd, &s->parent_irq);
> -
> -    pic = s;
>  }
>  
>  static const VMStateDescription vmstate_lm32_pic = {
> diff --git a/hw/intc/slavio_intctl.c b/hw/intc/slavio_intctl.c
> index a9acb64..84e0bee 100644
> --- a/hw/intc/slavio_intctl.c
> +++ b/hw/intc/slavio_intctl.c
> @@ -211,38 +211,6 @@ static const MemoryRegionOps slavio_intctlm_mem_ops = {
>      },
>  };
>  
> -void slavio_pic_info(Monitor *mon, DeviceState *dev)
> -{
> -    SLAVIO_INTCTLState *s = SLAVIO_INTCTL(dev);
> -    int i;
> -
> -    for (i = 0; i < MAX_CPUS; i++) {
> -        monitor_printf(mon, "per-cpu %d: pending 0x%08x\n", i,
> -                       s->slaves[i].intreg_pending);
> -    }
> -    monitor_printf(mon, "master: pending 0x%08x, disabled 0x%08x\n",
> -                   s->intregm_pending, s->intregm_disabled);
> -}
> -
> -void slavio_irq_info(Monitor *mon, DeviceState *dev)
> -{
> -#ifndef DEBUG_IRQ_COUNT
> -    monitor_printf(mon, "irq statistic code not compiled.\n");
> -#else
> -    SLAVIO_INTCTLState *s = SLAVIO_INTCTL(dev);
> -    int i;
> -    int64_t count;
> -
> -    s = SLAVIO_INTCTL(dev);
> -    monitor_printf(mon, "IRQ statistics:\n");
> -    for (i = 0; i < 32; i++) {
> -        count = s->irq_count[i];
> -        if (count > 0)
> -            monitor_printf(mon, "%2d: %" PRId64 "\n", i, count);
> -    }
> -#endif
> -}
> -
>  static const uint32_t intbit_to_level[] = {
>      2, 3, 5, 7, 9, 11, 13, 2,   3, 5, 7, 9, 11, 13, 12, 12,
>      6, 13, 4, 10, 8, 9, 11, 0,  0, 0, 0, 15, 15, 15, 15, 0,
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 478fda8..b3915e4 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -159,20 +159,6 @@ static void nvram_init(Nvram *nvram, uint8_t *macaddr,
>      }
>  }
>  
> -static DeviceState *slavio_intctl;
> -
> -void sun4m_hmp_info_pic(Monitor *mon, const QDict *qdict)
> -{
> -    if (slavio_intctl)
> -        slavio_pic_info(mon, slavio_intctl);
> -}
> -
> -void sun4m_hmp_info_irq(Monitor *mon, const QDict *qdict)
> -{
> -    if (slavio_intctl)
> -        slavio_irq_info(mon, slavio_intctl);
> -}
> -
>  void cpu_check_irqs(CPUSPARCState *env)
>  {
>      CPUState *cs;
> @@ -873,6 +859,7 @@ static void dummy_fdc_tc(void *opaque, int irq, int level)
>  static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>                            MachineState *machine)
>  {
> +    DeviceState *slavio_intctl;
>      const char *cpu_model = machine->cpu_model;
>      unsigned int i;
>      void *iommu, *espdma, *ledma, *nvram;
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index ab8e319..73178ae 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -181,8 +181,6 @@ qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq);
>  qemu_irq *kvm_i8259_init(ISABus *bus);
>  int pic_read_irq(DeviceState *d);
>  int pic_get_output(DeviceState *d);
> -void hmp_info_pic(Monitor *mon, const QDict *qdict);
> -void hmp_info_irq(Monitor *mon, const QDict *qdict);
>  
>  /* ioapic.c */
>  
> diff --git a/include/hw/lm32/lm32_pic.h b/include/hw/lm32/lm32_pic.h
> index 189fa38..e6479b8 100644
> --- a/include/hw/lm32/lm32_pic.h
> +++ b/include/hw/lm32/lm32_pic.h
> @@ -8,7 +8,4 @@ uint32_t lm32_pic_get_im(DeviceState *d);
>  void lm32_pic_set_ip(DeviceState *d, uint32_t ip);
>  void lm32_pic_set_im(DeviceState *d, uint32_t im);
>  
> -void lm32_hmp_info_pic(Monitor *mon, const QDict *qdict);
> -void lm32_hmp_info_irq(Monitor *mon, const QDict *qdict);
> -
>  #endif /* QEMU_HW_LM32_PIC_H */
> diff --git a/include/hw/sparc/sun4m.h b/include/hw/sparc/sun4m.h
> index 9c17425..580d87b 100644
> --- a/include/hw/sparc/sun4m.h
> +++ b/include/hw/sparc/sun4m.h
> @@ -24,14 +24,6 @@ static inline void sparc_iommu_memory_write(void *opaque,
>      sparc_iommu_memory_rw(opaque, addr, buf, len, 1);
>  }
>  
> -/* slavio_intctl.c */
> -void slavio_pic_info(Monitor *mon, DeviceState *dev);
> -void slavio_irq_info(Monitor *mon, DeviceState *dev);
> -
> -/* sun4m.c */
> -void sun4m_hmp_info_pic(Monitor *mon, const QDict *qdict);
> -void sun4m_hmp_info_irq(Monitor *mon, const QDict *qdict);
> -
>  /* sparc32_dma.c */
>  #include "hw/sparc/sparc32_dma.h"
>  
> diff --git a/monitor.c b/monitor.c
> index 83c4edf..d26c3bc 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -81,12 +81,6 @@
>  #include "qemu/cutils.h"
>  #include "qapi/qmp/dispatch.h"
>  
> -/* for hmp_info_irq/pic */
> -#if defined(TARGET_SPARC)
> -#include "hw/sparc/sun4m.h"
> -#endif
> -#include "hw/lm32/lm32_pic.h"
> -
>  #if defined(TARGET_S390X)
>  #include "hw/s390x/storage-keys.h"
>  #endif
>