From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48630) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z617b-00029C-Oj for qemu-devel@nongnu.org; Fri, 19 Jun 2015 14:34:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z617Y-0003Vj-BS for qemu-devel@nongnu.org; Fri, 19 Jun 2015 14:34:35 -0400 Received: from mx2.parallels.com ([199.115.105.18]:46528) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z617Y-0003VP-3P for qemu-devel@nongnu.org; Fri, 19 Jun 2015 14:34:32 -0400 Message-ID: <5584592E.9030801@odin.com> Date: Fri, 19 Jun 2015 21:02:22 +0300 From: Pavel MIME-Version: 1.0 References: <1434725298-22666-1-git-send-email-den@openvz.org> <1434725298-22666-6-git-send-email-den@openvz.org> <55843907.7060305@suse.de> In-Reply-To: <55843907.7060305@suse.de> Content-Type: text/plain; charset="iso-8859-15"; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 5/8] hmp: added local apic dump state List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?ISO-8859-15?Q?Andreas_F=E4rber?= , "Denis V. Lunev" Cc: Paolo Bonzini , Pavel Butsykin , qemu-devel@nongnu.org, Luiz Capitulino On 19.06.2015 18:45, Andreas Färber wrote: > Am 19.06.2015 um 16:48 schrieb Denis V. Lunev: >> From: Pavel Butsykin >> >> 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 >> Signed-off-by: Denis V. Lunev >> CC: Paolo Bonzini >> CC: Luiz Capitulino >> --- >> 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 >> +@item 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 Yes, indeed. I missed this moment, it needs to be fixed. I think APIC dump do better separately, because it is necessary to outputs apic state and not irq statistic past. Thanks for the reminder about get_maintainer.pl script. >> 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. ok, we will merge it. if that.. >> + 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 >> >> >