Re: [Qemu-devel] [PATCH 5/8] hmp: added local 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 local apic registers state, may be
> usefull after guest crashes to understand IRQ routing in guest.
> 
> For command name uses "apic-local" because it has to be grouped with
> command "apic-io".
> 
> (qemu) info apic-local
> apic.lvt    00-timer   000300fd int=fd .H.EMP delmod=0:Fixed
> apic.lvt    00-thermal 00010000 int=00 .H.EM. delmod=0:Fixed
> apic.lvt    00-perfmon 000000fe int=fe .H.E.. delmod=0:Fixed
> apic.lvt    00-LINT0   0001001f int=1f .H.EM. delmod=0:Fixed
> apic.lvt    00-LINT1   000004ff int=ff .H.E.. delmod=4:NMI
> apic.lvt    00-Error   000000e3 int=e3 .H.E.. delmod=0:Fixed
> apic.error  00 esr 00000000 S:... R:... .
> apic.timer  00 DCR=0000000b(b) initial_count=1000090000
> apic.icr    00 02000000000c00d1: int=d1 delmod=0:Fixed P..E
>                shorthand=3:all dest=2
> apic.prio   00 apr=00(0:0)  tpr=40(4:0)
> apic.dest   00 dfr=f0(f)    ldr=01(01)
> apic.svr    00 0000011f vec=1f on focus=off
> apic.interrupt  00 065:R.E
> 
> 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 +
>  include/qom/cpu.h     |  14 +++++
>  monitor.c             |  16 ++++++
>  qom/cpu.c             |  11 ++++
>  target-i386/cpu-qom.h |   2 +
>  target-i386/cpu.c     |   1 +
>  target-i386/helper.c  | 155 
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 201 insertions(+)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index d3b7932..95f554e 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1724,6 +1724,8 @@ show the block devices
>  show block device statistics
>  @item info registers
>  show the cpu registers
> address@hidden info apic-local
> +show local APIC state
>  @item info cpus
>  show infos for each CPU
>  @item info history
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 39f0f19..d0b5867 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -138,6 +138,8 @@ typedef struct CPUClass {
>      bool (*virtio_is_big_endian)(CPUState *cpu);
>      int (*memory_rw_debug)(CPUState *cpu, vaddr addr,
>                             uint8_t *buf, int len, bool is_write);
> +    void (*dump_apic_local_state)(CPUState *cpu, FILE *f,
> +                                  fprintf_function cpu_fprintf, int flags);

Objection, this is a highly x86-specific interface in generic code.
Either make it generic so that it works for a second CPU implementation
and does not contain "apic", or make the HMP command x86-specific and
avoid touching core CPU code then please.

Isn't there already some irq stats info that you could piggy-back on?

Also, you did not use the get_maintainer.pl script, which would've CC'ed
me as CPU maintainer.

One remark below.

Regards,
Andreas

>      void (*dump_state)(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf,
>                         int flags);
>      void (*dump_statistics)(CPUState *cpu, FILE *f,
> @@ -398,6 +400,18 @@ enum CPUDumpFlags {
>  };
>  
>  /**
> + * cpu_dump_apic_local_state:
> + * @cpu: The CPU whose lcoal APIC state is to be dumped.
> + * @f: File to dump to.
> + * @cpu_fprintf: Function to dump with.
> + * @flags: Flags what to dump.
> + *
> + * Dumps local APIC state.
> + */
> +void cpu_dump_apic_local_state(CPUState *cpu, FILE *f,
> +                               fprintf_function cpu_fprintf, int flags);
> +
> +/**
>   * cpu_dump_state:
>   * @cpu: The CPU whose state is to be dumped.
>   * @f: File to dump to.
> diff --git a/monitor.c b/monitor.c
> index 8e1a2e8..aad2792 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -957,6 +957,15 @@ int monitor_get_cpu_index(void)
>      return cpu->cpu_index;
>  }
>  
> +static void hmp_info_apic_local(Monitor *mon, const QDict *qdict)
> +{
> +    CPUState *cpu;
> +    CPUArchState *env;
> +    env = mon_get_cpu();
> +    cpu = ENV_GET_CPU(env);

Note: There is another pending series that will conflict with this.

> +    cpu_dump_apic_local_state(cpu, (FILE *)mon, monitor_fprintf, 
> CPU_DUMP_FPU);
> +}
> +
>  static void hmp_info_registers(Monitor *mon, const QDict *qdict)
>  {
>      CPUState *cpu;
> @@ -2572,6 +2581,13 @@ static mon_cmd_t info_cmds[] = {
>          .mhandler.cmd = hmp_info_registers,
>      },
>      {
> +        .name       = "apic-local",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show local apic state",
> +        .mhandler.cmd = hmp_info_apic_local,
> +    },
> +    {
>          .name       = "cpus",
>          .args_type  = "",
>          .params     = "",
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 108bfa2..56e8a6b 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -201,6 +201,17 @@ static bool cpu_common_exec_interrupt(CPUState *cpu, int 
> int_req)
>      return false;
>  }
>  
> +void cpu_dump_apic_local_state(CPUState *cpu, FILE *f,
> +                               fprintf_function cpu_fprintf, int flags)
> +{
> +    CPUClass *cc = CPU_GET_CLASS(cpu);
> +
> +    if (cc->dump_apic_local_state) {
> +        cpu_synchronize_state(cpu);
> +        cc->dump_apic_local_state(cpu, f, cpu_fprintf, flags);
> +    }
> +}
> +
>  void cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf,
>                      int flags)
>  {
> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> index c35b624..e5daeaf 100644
> --- a/target-i386/cpu-qom.h
> +++ b/target-i386/cpu-qom.h
> @@ -151,6 +151,8 @@ void x86_cpu_get_memory_mapping(CPUState *cpu, 
> MemoryMappingList *list,
>  
>  void x86_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
>                          int flags);
> +void x86_cpu_dump_apic_local_state(CPUState *cs, FILE *f,
> +                                   fprintf_function cpu_fprintf, int flags);
>  
>  hwaddr x86_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
>  
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index af0552a..4178444 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -3147,6 +3147,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
> void *data)
>      cc->has_work = x86_cpu_has_work;
>      cc->do_interrupt = x86_cpu_do_interrupt;
>      cc->cpu_exec_interrupt = x86_cpu_exec_interrupt;
> +    cc->dump_apic_local_state = x86_cpu_dump_apic_local_state;
>      cc->dump_state = x86_cpu_dump_state;
>      cc->set_pc = x86_cpu_set_pc;
>      cc->synchronize_from_tb = x86_cpu_synchronize_from_tb;
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 5480a96..8d883f5 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -23,6 +23,7 @@
>  #ifndef CONFIG_USER_ONLY
>  #include "sysemu/sysemu.h"
>  #include "monitor/monitor.h"
> +#include "hw/i386/apic_internal.h"
>  #endif
>  
>  static void cpu_x86_version(CPUX86State *env, int *family, int *model)
> @@ -177,6 +178,160 @@ done:
>      cpu_fprintf(f, "\n");
>  }
>  
> +#ifndef CONFIG_USER_ONLY
> +
> +/* ARRAY_SIZE check is not required because
> + * DeliveryMode(dm) has a size of 3 bit.
> + */
> +static inline const char *dm2str(uint32_t dm)
> +{
> +    static const char *str[] = {
> +        "Fixed",
> +        "...",
> +        "SMI",
> +        "...",
> +        "NMI",
> +        "INIT",
> +        "...",
> +        "ExtINT"
> +    };
> +    return str[dm];
> +}
> +
> +static void dump_apic_lvt(FILE *f, fprintf_function cpu_fprintf,
> +                          uint32_t cpu_idx, const char *name,
> +                          uint32_t lvt, bool is_timer)
> +{
> +    uint32_t dm = (lvt & APIC_LVT_DELIV_MOD) >> APIC_LVT_DELIV_MOD_SHIFT;
> +    cpu_fprintf(f,
> +                "apic.lvt\t%02u-%-7s %08x int=%02x %c%c%c%c%c%c 
> delmod=%x:%s\n",
> +                cpu_idx, name, lvt,
> +                lvt & APIC_VECTOR_MASK,
> +                lvt & APIC_LVT_DELIV_STS ? 'P' : '.',
> +                lvt & APIC_LVT_INT_POLARITY ? 'L' : 'H',
> +                lvt & APIC_LVT_REMOTE_IRR ? 'R' : '.',
> +                lvt & APIC_LVT_LEVEL_TRIGGER ? 'L' : 'E',
> +                lvt & APIC_LVT_MASKED ? 'M' : '.',
> +                !is_timer ? '.' : (lvt & APIC_LVT_TIMER_PERIODIC ? 'P' : 
> 'S'),
> +                dm,
> +                dm2str(dm));
> +
> +}
> +
> +/* ARRAY_SIZE check is not required because
> + * destination shorthand has a size of 2 bit.
> + */
> +static inline const char *shorthand2str(uint32_t shorthand)
> +{
> +    const char *str[] = {
> +        "no", "self", "all-self", "all"
> +    };
> +    return str[shorthand];
> +}
> +
> +void x86_cpu_dump_apic_local_state(CPUState *cs, FILE *f,
> +                                   fprintf_function cpu_fprintf, int flags)
> +{
> +    X86CPU *cpu = X86_CPU(cs);
> +    APICCommonState *s = APIC_COMMON(cpu->apic_state);
> +    uint32_t *irr_tab = s->irr, *isr_tab = s->isr, *tmr_tab = s->tmr;
> +    uint32_t *lvt = s->lvt;
> +    uint32_t icr0 = s->icr[0], icr1 = s->icr[1];
> +    uint32_t esr = s->esr;
> +    uint32_t cpu_idx = CPU(cpu)->cpu_index;
> +    int i;
> +
> +    dump_apic_lvt(f, cpu_fprintf, cpu_idx, "timer",
> +                  lvt[APIC_LVT_TIMER], true);
> +    dump_apic_lvt(f, cpu_fprintf, cpu_idx, "thermal",
> +                  lvt[APIC_LVT_THERMAL], false);
> +    dump_apic_lvt(f, cpu_fprintf, cpu_idx, "perfmon",
> +                  lvt[APIC_LVT_PERFORM], false);
> +    dump_apic_lvt(f, cpu_fprintf, cpu_idx, "LINT0",
> +                  lvt[APIC_LVT_LINT0], false);
> +    dump_apic_lvt(f, cpu_fprintf, cpu_idx, "LINT1",
> +                  lvt[APIC_LVT_LINT1], false);
> +    dump_apic_lvt(f, cpu_fprintf, cpu_idx, "Error",
> +                  lvt[APIC_LVT_ERROR], false);
> +
> +    cpu_fprintf(f, "apic.error\t%02u esr %08x S:%c%c%c R:%c%c%c %c\n",
> +                cpu_idx, esr,
> +                esr & APIC_ESR_SEND_CHECK_SUM ? 'C' : '.',
> +                esr & APIC_ESR_SEND_ACCEPT ? 'A' : '.',
> +                esr & APIC_ESR_SEND_ILLEGAL_VECT ? 'I' : '.',
> +                esr & APIC_ESR_RECV_CHECK_SUM ? 'C' : '.',
> +                esr & APIC_ESR_RECV_ACCEPT ? 'A' : '.',
> +                esr & APIC_ESR_RECV_ILLEGAL_VECT ? 'I' : '.',
> +                esr & APIC_ESR_ILLEGAL_ADDRESS ? 'R' : '.');
> +
> +    cpu_fprintf(f, "apic.timer\t%02u DCR=%08x(%x) initial_count=%d\n",
> +                cpu_idx, s->divide_conf, s->divide_conf & APIC_DCR_MASK,
> +                s->initial_count);
> +
> +    cpu_fprintf(f, "apic.icr\t%02u %016jx: int=%02x "
> +                "delmod=%x:%s %c%c%c%c shorthand=%x:%s dest=%x\n",
> +                cpu_idx, ((uint64_t *)s->icr)[0],
> +                icr0 & APIC_VECTOR_MASK,
> +                (icr0 & APIC_ICR_DELIV_MOD) >> APIC_ICR_DELIV_MOD_SHIFT,
> +                dm2str((icr0 & APIC_ICR_DELIV_MOD) >> 
> APIC_ICR_DELIV_MOD_SHIFT),
> +                icr0 & APIC_ICR_DEST_MOD ? 'L' : 'P',
> +                icr0 & APIC_ICR_DELIV_STS ? 'P' : '.',
> +                icr0 & APIC_ICR_LEVEL ? 'A' : '.',
> +                icr0 & APIC_ICR_TRIGGER_MOD ? 'L' : 'E',
> +                (icr0 & APIC_ICR_DEST_SHORT) >> APIC_ICR_DEST_SHORT_SHIFT,
> +                shorthand2str(
> +                    (icr0 & APIC_ICR_DEST_SHORT) >> 
> APIC_ICR_DEST_SHORT_SHIFT),
> +                (icr1 >> APIC_ICR_DEST_SHIFT) &
> +                    (icr0 & APIC_ICR_DEST_MOD ? 0xff : 0xf));
> +
> +    cpu_fprintf(f, "apic.prio\t%02u apr=%02x(%x:%x)\ttpr=%02x(%x:%x)\n",
> +                cpu_idx,
> +                s->arb_id,
> +                s->arb_id >> APIC_PR_CLASS_SHIFT,
> +                s->arb_id & APIC_PR_SUB_CLASS,
> +                s->tpr,
> +                s->tpr >> APIC_PR_CLASS_SHIFT,
> +                s->tpr & APIC_PR_SUB_CLASS);
> +
> +    cpu_fprintf(f, "apic.dest\t%02u dfr=%02x(%x)\tldr=%02x",
> +                cpu_idx, s->dest_mode << 4, s->dest_mode, s->log_dest);
> +    if (s->dest_mode == 0) {
> +        cpu_fprintf(f, "(%x:%x)\n",
> +                    s->log_dest & APIC_LOGDEST_APIC_ID,
> +                    s->log_dest >> APIC_LOGDEST_ID_SHIFT);
> +    } else if (s->dest_mode == 0xf) {
> +        cpu_fprintf(f, "(%02x)\n", s->log_dest);
> +    } else {
> +        cpu_fprintf(f, "(BAD)\n");
> +    }
> +
> +    cpu_fprintf(f, "apic.svr\t%02u %08x vec=%02x %s focus=%s\n",
> +                cpu_idx, s->spurious_vec,
> +                s->spurious_vec & APIC_VECTOR_MASK,
> +                s->spurious_vec & APIC_SPURIO_ENABLED ? "on" : "off",
> +                s->spurious_vec & APIC_SPURIO_FOCUS ? "on" : "off");
> +
> +    for (i = 0; i < 256; i++) {
> +        if (irr_tab[i] || isr_tab[i]) {
> +            bool irr_bit = apic_get_bit(irr_tab, i);
> +            bool isr_bit = apic_get_bit(isr_tab, i);
> +
> +            if (irr_bit || isr_bit) {
> +                cpu_fprintf(f, "apic.interrupt\t%02u %03u:%c%c%c\n", 
> cpu_idx, i,
> +                            irr_bit ? 'R' : '.',
> +                            isr_bit ? 'S' : '.',
> +                            apic_get_bit(tmr_tab, i) ? 'L' : 'E');
> +            }
> +        }
> +    }
> +}
> +#else
> +void x86_cpu_dump_apic_local_state(CPUState *cs, FILE *f,
> +                                   fprintf_function cpu_fprintf, int flags)
> +{
> +}
> +#endif /* !CONFIG_USER_ONLY */
> +
>  #define DUMP_CODE_BYTES_TOTAL    50
>  #define DUMP_CODE_BYTES_BACKWARD 20
>  
> 


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