From: Pavel <pbutsykin@odin.com>
To: "Andreas Färber" <afaerber@suse.de>, "Denis V. Lunev" <den@openvz.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Pavel Butsykin <pbutsykin@virtuozzo.com>,
qemu-devel@nongnu.org, Luiz Capitulino <lcapitulino@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 5/8] hmp: added local apic dump state
Date: Fri, 19 Jun 2015 21:02:22 +0300 [thread overview]
Message-ID: <5584592E.9030801@odin.com> (raw)
In-Reply-To: <55843907.7060305@suse.de>
On 19.06.2015 18:45, Andreas Färber wrote:
> Am 19.06.2015 um 16:48 schrieb Denis V. Lunev:
>> From: Pavel Butsykin <pbutsykin@virtuozzo.com>
>>
>> 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 <pbutsykin@virtuozzo.com>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Paolo Bonzini <pbonzini@redhat.com>
>> CC: Luiz Capitulino <lcapitulino@redhat.com>
>> ---
>> 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
>>
>>
>
next prev parent reply other threads:[~2015-06-19 18:34 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-19 14:48 [Qemu-devel] [PATCH 0/8] hmp command IO- and Local APIC dump state Denis V. Lunev
2015-06-19 14:48 ` [Qemu-devel] [PATCH 1/8] apic_internal.h: move apic_get_bit(), apic_set_bit() to apic_internal.h Denis V. Lunev
2015-06-19 14:48 ` [Qemu-devel] [PATCH 2/8] apic_internal.h: rename ESR_ILLEGAL_ADDRESS to APIC_ESR_ILLEGAL_ADDRESS Denis V. Lunev
2015-06-19 14:48 ` [Qemu-devel] [PATCH 3/8] apic_internal.h: added more constants Denis V. Lunev
2015-06-19 14:48 ` [Qemu-devel] [PATCH 4/8] apic_internal.h: Fix formatting and drop unused consts Denis V. Lunev
2015-06-19 14:48 ` [Qemu-devel] [PATCH 5/8] hmp: added local apic dump state Denis V. Lunev
2015-06-19 15:45 ` Andreas Färber
2015-06-19 18:02 ` Pavel [this message]
2015-06-19 14:48 ` [Qemu-devel] [PATCH 6/8] ioapic_internal.h: added more constants Denis V. Lunev
2015-06-19 14:48 ` [Qemu-devel] [PATCH 7/8] hmp: added io apic dump state Denis V. Lunev
2015-06-19 15:53 ` Andreas Färber
2015-06-19 18:02 ` Pavel
2015-06-19 16:08 ` Peter Maydell
2015-06-19 18:03 ` Pavel
2015-06-19 14:48 ` [Qemu-devel] [PATCH 8/8] hmp: implemented io apic dump state for emulator Denis V. Lunev
2015-06-19 14:56 ` [Qemu-devel] [PATCH 0/8] hmp command IO- and Local APIC dump state Eric Blake
2015-06-19 15:01 ` Denis V. Lunev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5584592E.9030801@odin.com \
--to=pbutsykin@odin.com \
--cc=afaerber@suse.de \
--cc=den@openvz.org \
--cc=lcapitulino@redhat.com \
--cc=pbonzini@redhat.com \
--cc=pbutsykin@virtuozzo.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).