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

[Blue Swirl]

On Wed, Sep 28, 2011 at 9:26 PM, Jan Kiszka <address@hidden> wrote:
> On 2011-09-28 20:19, Blue Swirl wrote:
>>
>> 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?
>
> Actually, I don't want to over-design this rather useless legacy monitor
> command, rather completely remove it once device_show is merged. This patch
> is just an intermediate step to enable moving i8259 to hwlib.

Fine, but then don't introduce the global variable but leave it static
at sun4m.c.