Re: [Qemu-devel] [PATCH 7/8] hmp: added io apic dump state

[Andreas Färber]

Am 19.06.2015 um 16:48 schrieb Denis V. Lunev:
> From: Pavel Butsykin <address@hidden>
> 
> Added the hmp command to query io apic state, may be usefull after guest
> crashes to understand IRQ routing in guest.
> 
> Implementation is only for kvm here. The dump will look like
> (qemu) info apic-io
> ioapic ID=00 IRR=00000000 SEL=18
> ioapic 00 00000000000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
> ioapic 01 0300000000000993: int=93 delmod=1:LowPri L.H.E. dest=3
> ...
> ioapic 23 00000000000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
> 
> Signed-off-by: Pavel Butsykin <address@hidden>
> Signed-off-by: Denis V. Lunev <address@hidden>
> CC: Paolo Bonzini <address@hidden>
> CC: Luiz Capitulino <address@hidden>
> ---
>  hmp-commands.hx                   |  2 ++
>  hw/i386/kvm/ioapic.c              | 10 ++++++++++
>  hw/intc/ioapic_common.c           | 31 +++++++++++++++++++++++++++++++
>  include/hw/i386/ioapic_internal.h |  2 ++
>  include/hw/i386/pc.h              |  4 ++++
>  monitor.c                         | 20 ++++++++++++++++++++
>  6 files changed, 69 insertions(+)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 95f554e..894ff65 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1726,6 +1726,8 @@ show block device statistics
>  show the cpu registers
>  @item info apic-local
>  show local APIC state
> address@hidden info apic-io
> +show io APIC state
>  @item info cpus
>  show infos for each CPU
>  @item info history
> diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
> index d2a6c4c..b7390ca 100644
> --- a/hw/i386/kvm/ioapic.c
> +++ b/hw/i386/kvm/ioapic.c
> @@ -10,6 +10,7 @@
>   * See the COPYING file in the top-level directory.
>   */
>  
> +#include "monitor/monitor.h"
>  #include "hw/i386/pc.h"
>  #include "hw/i386/ioapic_internal.h"
>  #include "hw/i386/apic_internal.h"
> @@ -110,6 +111,15 @@ static void kvm_ioapic_put(IOAPICCommonState *s)
>      }
>  }
>  
> +void kvm_ioapic_dump_state(Monitor *mon, const QDict *qdict)
> +{
> +    IOAPICCommonState s;
> +
> +    kvm_ioapic_get(&s);
> +
> +    ioapic_print_redtbl(mon, &s);
> +}
> +
>  static void kvm_ioapic_reset(DeviceState *dev)
>  {
>      IOAPICCommonState *s = IOAPIC_COMMON(dev);
> diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
> index 8b7d118..83edf69 100644
> --- a/hw/intc/ioapic_common.c
> +++ b/hw/intc/ioapic_common.c
> @@ -19,6 +19,7 @@
>   * License along with this library; if not, see 
> <http://www.gnu.org/licenses/>.
>   */
>  
> +#include "monitor/monitor.h"
>  #include "hw/i386/ioapic.h"
>  #include "hw/i386/ioapic_internal.h"
>  #include "hw/sysbus.h"
> @@ -31,6 +32,36 @@
>   */
>  int ioapic_no;
>  
> +void ioapic_print_redtbl(Monitor *mon, IOAPICCommonState *s)
> +{
> +    static const char *delm_str[] = {
> +        "Fixed", "LowPri", "SMI", "...", "NMI", "INIT", "...", "ExtINT"};
> +    int i;
> +
> +    monitor_printf(mon, "ioapic ID=%02x IRR=%08x SEL=%02x\n",
> +                   s->id, s->irr, s->ioregsel);
> +
> +    for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> +        uint64_t entry = s->ioredtbl[i];
> +        uint32_t delm = (uint32_t)((entry & IOAPIC_LVT_DELIV_MODE) >>
> +                                   IOAPIC_LVT_DELIV_MODE_SHIFT);
> +        monitor_printf(mon, "ioapic %02u %016jx: int=%02jx "
> +                       "delmod=%x:%-6s %c%c%c%c%c%c dest=%jx\n",
> +                       i, entry,
> +                       entry & IOAPIC_VECTOR_MASK,
> +                       delm,
> +                       delm_str[delm],
> +                       entry & IOAPIC_LVT_DEST_MODE ? 'L' : 'P',
> +                       entry & IOAPIC_LVT_DELIV_STATUS ? 'P' : '.',
> +                       entry & IOAPIC_LVT_POLARITY ? 'L' : 'H',
> +                       entry & IOAPIC_LVT_REMOTE_IRR ? 'R' : '.',
> +                       entry & IOAPIC_LVT_TRIGGER_MODE ? 'L' : 'E',
> +                       entry & IOAPIC_LVT_MASKED ? 'M' : '.',
> +                       (entry >> IOAPIC_LVT_DEST_SHIFT) &
> +                            (entry & IOAPIC_LVT_DEST_MODE ? 0xff : 0xf));
> +    }
> +}
> +
>  void ioapic_reset_common(DeviceState *dev)
>  {
>      IOAPICCommonState *s = IOAPIC_COMMON(dev);
> diff --git a/include/hw/i386/ioapic_internal.h 
> b/include/hw/i386/ioapic_internal.h
> index 4f7764e..797ed47 100644
> --- a/include/hw/i386/ioapic_internal.h
> +++ b/include/hw/i386/ioapic_internal.h
> @@ -105,4 +105,6 @@ struct IOAPICCommonState {
>  
>  void ioapic_reset_common(DeviceState *dev);
>  
> +void ioapic_print_redtbl(Monitor *mon, IOAPICCommonState *s);
> +
>  #endif /* !QEMU_IOAPIC_INTERNAL_H */
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 86c5651..437085f 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -123,6 +123,10 @@ 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 */
> +
> +void kvm_ioapic_dump_state(Monitor *mon, const QDict *qdict);
> +
>  /* Global System Interrupts */
>  
>  #define GSI_NUM_PINS IOAPIC_NUM_PINS
> diff --git a/monitor.c b/monitor.c
> index aad2792..5449663 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -957,6 +957,19 @@ int monitor_get_cpu_index(void)
>      return cpu->cpu_index;
>  }
>  
> +#if defined(TARGET_I386)
> +static void hmp_info_apic_io(Monitor *mon, const QDict *qdict)
> +{
> +    if (kvm_irqchip_in_kernel()) {
> +        kvm_ioapic_dump_state(mon, qdict);
> +    }

Why no else clause for TCG?

> +}
> +#else
> +static void hmp_info_apic_io(Monitor *mon, const QDict *qdict)
> +{
> +}

Rather than having a no-op info apic-io on ARM etc., I would suggest to
#ifdef the below array entry too, so that it is for x86 exclusively.

Regards,
Andreas

> +#endif
> +
>  static void hmp_info_apic_local(Monitor *mon, const QDict *qdict)
>  {
>      CPUState *cpu;
> @@ -2588,6 +2601,13 @@ static mon_cmd_t info_cmds[] = {
>          .mhandler.cmd = hmp_info_apic_local,
>      },
>      {
> +        .name       = "apic-io",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show io apic state",
> +        .mhandler.cmd = hmp_info_apic_io,
> +    },
> +    {
>          .name       = "cpus",
>          .args_type  = "",
>          .params     = "",

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)