Re: [Qemu-devel] [PATCH 21/22] monitor: Restrict pic/irq_info to supporting targets

[Blue Swirl]

On Wed, Sep 28, 2011 at 11:01 AM, Jan Kiszka <address@hidden> wrote:
> Signed-off-by: Jan Kiszka <address@hidden>
> ---
>  hw/an5206.c             |   10 ----------
>  hw/arm_pic.c            |   11 -----------
>  hw/cris_pic_cpu.c       |    6 ------
>  hw/etraxfs.h            |    1 +
>  hw/lm32_pic.c           |    4 ++--
>  hw/lm32_pic.h           |    3 +++
>  hw/microblaze_pic_cpu.c |    6 ------
>  hw/s390-virtio.c        |   11 -----------
>  hw/shix.c               |   11 -----------
>  hw/slavio_intctl.c      |   14 ++++++++++----
>  hw/sun4m.c              |   16 ++--------------
>  hw/sun4m.h              |    6 ++++--
>  hw/sun4u.c              |    8 --------
>  hw/xtensa_pic.c         |   10 ----------
>  monitor.c               |   21 +++++++++++++++++++++
>  15 files changed, 43 insertions(+), 95 deletions(-)
>
> diff --git a/hw/an5206.c b/hw/an5206.c
> index 481ae60..3fe1f00 100644
> --- a/hw/an5206.c
> +++ b/hw/an5206.c
> @@ -7,7 +7,6 @@
>  */
>
>  #include "hw.h"
> -#include "pc.h"
>  #include "mcf.h"
>  #include "boards.h"
>  #include "loader.h"
> @@ -18,15 +17,6 @@
>  #define AN5206_MBAR_ADDR 0x10000000
>  #define AN5206_RAMBAR_ADDR 0x20000000
>
> -/* Stub functions for hardware that doesn't exist.  */
> -void pic_info(Monitor *mon)
> -{
> -}
> -
> -void irq_info(Monitor *mon)
> -{
> -}
> -
>  /* Board init.  */
>
>  static void an5206_init(ram_addr_t ram_size,
> diff --git a/hw/arm_pic.c b/hw/arm_pic.c
> index 985148a..4e63845 100644
> --- a/hw/arm_pic.c
> +++ b/hw/arm_pic.c
> @@ -8,19 +8,8 @@
>  */
>
>  #include "hw.h"
> -#include "pc.h"
>  #include "arm-misc.h"
>
> -/* Stub functions for hardware that doesn't exist.  */
> -void pic_info(Monitor *mon)
> -{
> -}
> -
> -void irq_info(Monitor *mon)
> -{
> -}
> -
> -
>  /* Input 0 is IRQ and input 1 is FIQ.  */
>  static void arm_pic_cpu_handler(void *opaque, int irq, int level)
>  {
> diff --git a/hw/cris_pic_cpu.c b/hw/cris_pic_cpu.c
> index 7f1e4ab..06ae484 100644
> --- a/hw/cris_pic_cpu.c
> +++ b/hw/cris_pic_cpu.c
> @@ -24,16 +24,10 @@
>
>  #include "sysbus.h"
>  #include "hw.h"
> -#include "pc.h"
>  #include "etraxfs.h"
>
>  #define D(x)
>
> -void pic_info(Monitor *mon)
> -{}
> -void irq_info(Monitor *mon)
> -{}
> -
>  static void cris_pic_cpu_handler(void *opaque, int irq, int level)
>  {
>     CPUState *env = (CPUState *)opaque;
> diff --git a/hw/etraxfs.h b/hw/etraxfs.h
> index 1554b0b..24e8fd8 100644
> --- a/hw/etraxfs.h
> +++ b/hw/etraxfs.h
> @@ -22,6 +22,7 @@
>  * THE SOFTWARE.
>  */
>
> +#include "net.h"
>  #include "etraxfs_dma.h"
>
>  qemu_irq *cris_pic_init_cpu(CPUState *env);
> diff --git a/hw/lm32_pic.c b/hw/lm32_pic.c
> index 02941a7..8dd0050 100644
> --- a/hw/lm32_pic.c
> +++ b/hw/lm32_pic.c
> @@ -39,7 +39,7 @@ struct LM32PicState {
>  typedef struct LM32PicState LM32PicState;
>
>  static LM32PicState *pic;
> -void pic_info(Monitor *mon)
> +void lm32_do_pic_info(Monitor *mon)
>  {
>     if (pic == NULL) {
>         return;
> @@ -49,7 +49,7 @@ void pic_info(Monitor *mon)
>             pic->im, pic->ip, pic->irq_state);
>  }
>
> -void irq_info(Monitor *mon)
> +void lm32_irq_info(Monitor *mon)
>  {
>     int i;
>     uint32_t count;
> diff --git a/hw/lm32_pic.h b/hw/lm32_pic.h
> index e6479b8..14456f3 100644
> --- a/hw/lm32_pic.h
> +++ b/hw/lm32_pic.h
> @@ -8,4 +8,7 @@ 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_do_pic_info(Monitor *mon);
> +void lm32_irq_info(Monitor *mon);
> +
>  #endif /* QEMU_HW_LM32_PIC_H */
> diff --git a/hw/microblaze_pic_cpu.c b/hw/microblaze_pic_cpu.c
> index 9ad48b4..8b5623c 100644
> --- a/hw/microblaze_pic_cpu.c
> +++ b/hw/microblaze_pic_cpu.c
> @@ -23,16 +23,10 @@
>  */
>
>  #include "hw.h"
> -#include "pc.h"
>  #include "microblaze_pic_cpu.h"
>
>  #define D(x)
>
> -void pic_info(Monitor *mon)
> -{}
> -void irq_info(Monitor *mon)
> -{}
> -
>  static void microblaze_pic_cpu_handler(void *opaque, int irq, int level)
>  {
>     CPUState *env = (CPUState *)opaque;
> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
> index acbf026..778cffe 100644
> --- a/hw/s390-virtio.c
> +++ b/hw/s390-virtio.c
> @@ -62,17 +62,6 @@
>  static VirtIOS390Bus *s390_bus;
>  static CPUState **ipi_states;
>
> -void irq_info(Monitor *mon);
> -void pic_info(Monitor *mon);
> -
> -void irq_info(Monitor *mon)
> -{
> -}
> -
> -void pic_info(Monitor *mon)
> -{
> -}
> -
>  CPUState *s390_cpu_addr2state(uint16_t cpu_addr)
>  {
>     if (cpu_addr >= smp_cpus) {
> diff --git a/hw/shix.c b/hw/shix.c
> index 638bf16..dbf4764 100644
> --- a/hw/shix.c
> +++ b/hw/shix.c
> @@ -28,7 +28,6 @@
>    More information in target-sh4/README.sh4
>  */
>  #include "hw.h"
> -#include "pc.h"
>  #include "sh.h"
>  #include "sysemu.h"
>  #include "boards.h"
> @@ -37,16 +36,6 @@
>  #define BIOS_FILENAME "shix_bios.bin"
>  #define BIOS_ADDRESS 0xA0000000
>
> -void irq_info(Monitor *mon)
> -{
> -    /* XXXXX */
> -}
> -
> -void pic_info(Monitor *mon)
> -{
> -    /* XXXXX */
> -}
> -
>  static void shix_init(ram_addr_t ram_size,
>                const char *boot_device,
>               const char *kernel_filename, const char *kernel_cmdline,
> diff --git a/hw/slavio_intctl.c b/hw/slavio_intctl.c
> index 329c251..2d1dc12 100644
> --- a/hw/slavio_intctl.c
> +++ b/hw/slavio_intctl.c
> @@ -204,13 +204,16 @@ static CPUWriteMemoryFunc * const 
> slavio_intctlm_mem_write[3] = {
>     slavio_intctlm_mem_writel,
>  };
>
> -void slavio_pic_info(Monitor *mon, DeviceState *dev)
> +void slavio_pic_info(Monitor *mon)
>  {
>     SysBusDevice *sd;
>     SLAVIO_INTCTLState *s;
>     int i;
>
> -    sd = sysbus_from_qdev(dev);
> +    if (!slavio_intctl) {
> +        return;
> +    }
> +    sd = sysbus_from_qdev(slavio_intctl);
>     s = FROM_SYSBUS(SLAVIO_INTCTLState, sd);
>     for (i = 0; i < MAX_CPUS; i++) {
>         monitor_printf(mon, "per-cpu %d: pending 0x%08x\n", i,
> @@ -220,7 +223,7 @@ void slavio_pic_info(Monitor *mon, DeviceState *dev)
>                    s->intregm_pending, s->intregm_disabled);
>  }
>
> -void slavio_irq_info(Monitor *mon, DeviceState *dev)
> +void slavio_irq_info(Monitor *mon)
>  {
>  #ifndef DEBUG_IRQ_COUNT
>     monitor_printf(mon, "irq statistic code not compiled.\n");
> @@ -230,7 +233,10 @@ void slavio_irq_info(Monitor *mon, DeviceState *dev)
>     int i;
>     int64_t count;
>
> -    sd = sysbus_from_qdev(dev);
> +    if (!slavio_intctl) {
> +        return;
> +    }
> +    sd = sysbus_from_qdev(slavio_intctl);
>     s = FROM_SYSBUS(SLAVIO_INTCTLState, sd);
>     monitor_printf(mon, "IRQ statistics:\n");
>     for (i = 0; i < 32; i++) {
> diff --git a/hw/sun4m.c b/hw/sun4m.c
> index dcaed38..589b505 100644
> --- a/hw/sun4m.c
> +++ b/hw/sun4m.c
> @@ -135,6 +135,8 @@ struct sun4c_hwdef {
>     uint8_t nvram_machine_id;
>  };
>
> +DeviceState *slavio_intctl;

Instead of adding new global variables, could you add a registration
mechanism instead to pass the DeviceState used by the handler?

Or maybe I should revive this patch set:
http://lists.nongnu.org/archive/html/qemu-devel/2009-09/msg00459.html

> +
>  int DMA_get_channel_mode (int nchan)
>  {
>     return 0;
> @@ -214,20 +216,6 @@ static void nvram_init(M48t59State *nvram, uint8_t 
> *macaddr,
>         m48t59_write(nvram, i, image[i]);
>  }
>
> -static DeviceState *slavio_intctl;
> -
> -void pic_info(Monitor *mon)
> -{
> -    if (slavio_intctl)
> -        slavio_pic_info(mon, slavio_intctl);
> -}
> -
> -void irq_info(Monitor *mon)
> -{
> -    if (slavio_intctl)
> -        slavio_irq_info(mon, slavio_intctl);
> -}
> -
>  void cpu_check_irqs(CPUState *env)
>  {
>     if (env->pil_in && (env->interrupt_index == 0 ||
> diff --git a/hw/sun4m.h b/hw/sun4m.h
> index ce97ee5..39a73aa 100644
> --- a/hw/sun4m.h
> +++ b/hw/sun4m.h
> @@ -3,6 +3,8 @@
>
>  #include "qemu-common.h"
>
> +extern DeviceState *slavio_intctl;
> +
>  /* Devices used by sparc32 system.  */
>
>  /* iommu.c */
> @@ -23,8 +25,8 @@ static inline void sparc_iommu_memory_write(void *opaque,
>  }
>
>  /* slavio_intctl.c */
> -void slavio_pic_info(Monitor *mon, DeviceState *dev);
> -void slavio_irq_info(Monitor *mon, DeviceState *dev);
> +void slavio_pic_info(Monitor *mon);
> +void slavio_irq_info(Monitor *mon);
>
>  /* sun4c_intctl.c */
>  void sun4c_pic_info(Monitor *mon, void *opaque);
> diff --git a/hw/sun4u.c b/hw/sun4u.c
> index fbef350..568819d 100644
> --- a/hw/sun4u.c
> +++ b/hw/sun4u.c
> @@ -242,14 +242,6 @@ static unsigned long sun4u_load_kernel(const char 
> *kernel_filename,
>     return kernel_size;
>  }
>
> -void pic_info(Monitor *mon)
> -{
> -}
> -
> -void irq_info(Monitor *mon)
> -{
> -}
> -
>  void cpu_check_irqs(CPUState *env)
>  {
>     uint32_t pil = env->pil_in |
> diff --git a/hw/xtensa_pic.c b/hw/xtensa_pic.c
> index 3033ae2..9357684 100644
> --- a/hw/xtensa_pic.c
> +++ b/hw/xtensa_pic.c
> @@ -26,19 +26,9 @@
>  */
>
>  #include "hw.h"
> -#include "pc.h"
>  #include "qemu-log.h"
>  #include "qemu-timer.h"
>
> -/* Stub functions for hardware that doesn't exist.  */
> -void pic_info(Monitor *mon)
> -{
> -}
> -
> -void irq_info(Monitor *mon)
> -{
> -}
> -
>  void xtensa_advance_ccount(CPUState *env, uint32_t d)
>  {
>     uint32_t old_ccount = env->sregs[CCOUNT];
> diff --git a/monitor.c b/monitor.c
> index 8ec2c5e..418161c 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -64,6 +64,12 @@
>  #include "trace/control.h"
>  #include "ui/qemu-spice.h"
>
> +/* for pic/irq_info */
> +#if defined(TARGET_SPARC)
> +#include "hw/sun4m.h"
> +#endif
> +#include "hw/lm32_pic.h"
> +
>  //#define DEBUG
>  //#define DEBUG_COMPLETION
>
> @@ -2937,20 +2943,35 @@ static const mon_cmd_t info_cmds[] = {
>         .help       = "show the command line history",
>         .mhandler.info = do_info_history,
>     },
> +#if defined(TARGET_I386) || defined(TARGET_PPC) || defined(TARGET_MIPS) || \
> +    defined(TARGET_LM32) || (defined(TARGET_SPARC) && 
> !defined(TARGET_SPARC64))
>     {
>         .name       = "irq",
>         .args_type  = "",
>         .params     = "",
>         .help       = "show the interrupts statistics (if available)",
> +#ifdef TARGET_SPARC
> +        .mhandler.info = slavio_irq_info,
> +#elif defined(TARGET_LM32)
> +        .mhandler.info = lm32_irq_info,
> +#else
>         .mhandler.info = irq_info,
> +#endif
>     },
>     {
>         .name       = "pic",
>         .args_type  = "",
>         .params     = "",
>         .help       = "show i8259 (PIC) state",
> +#ifdef TARGET_SPARC
> +        .mhandler.info = slavio_pic_info,
> +#elif defined(TARGET_LM32)
> +        .mhandler.info = lm32_do_pic_info,
> +#else
>         .mhandler.info = pic_info,
> +#endif
>     },
> +#endif
>     {
>         .name       = "pci",
>         .args_type  = "",
> --
> 1.7.3.4
>
>