Hi Philippe,
On 2/6/23 23:16, Philippe Mathieu-Daudé wrote:
> On 7/2/23 00:42, Dongli Zhang wrote:
>> The cpu_dump_state() does not print the cpu index. When the
>> cpu_dump_state() is invoked due to the KVM failure, we are not able to tell
>> from which CPU the state is. The below is an example.
>>
>> KVM internal error. Suberror: 764064
>> RAX=0000000000000002 RBX=ffff8a9e57c38400 RCX=00000000ffffffff
>> RDX=ffff8a9cc00ba8a0
>> RSI=0000000000000003 RDI=ffff8a9e57c38400 RBP=ffffb6120c5b3c50
>> RSP=ffffb6120c5b3c40
>> R8 =0000000000000000 R9 =ffff8a9cc00ba8a0 R10=ffffffff8e467350
>> R11=0000000000000007
>> R12=000000000000000a R13=ffffffff8f987e25 R14=ffffffff8f988a01
>> R15=0000000000000000
>> RIP=ffffffff8e51bb04 RFL=00010046 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
>> ES =0000 0000000000000000 ffffffff 00c00000
>> CS =0010 0000000000000000 ffffffff 00a09b00 DPL=0 CS64 [-RA]
>> SS =0000 0000000000000000 ffffffff 00c00000
>> DS =0000 0000000000000000 ffffffff 00c00000
>> FS =0000 0000000000000000 ffffffff 00c00000
>> GS =0000 ffff8ac27fcc0000 ffffffff 00c00000
>> LDT=0000 0000000000000000 ffffffff 00c00000
>> TR =0040 fffffe0000096000 0000206f 00008b00 DPL=0 TSS64-busy
>> GDT= fffffe0000094000 0000007f
>> IDT= fffffe0000000000 00000fff
>> CR0=80050033 CR2=0000000000000000 CR3=00000010ca40a001 CR4=003606e0
>> DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000
>> DR3=0000000000000000
>> DR6=00000000fffe0ff0 DR7=0000000000000400
>> EFER=0000000000000d01
>> Code=0f 1f ... ...
>>
>> Print the cpu->cpu_index in cpu_dump_state() and remove it from the caller.
>>
>> Cc: Joe Jin <joe....@oracle.com>
>> Signed-off-by: Dongli Zhang <dongli.zh...@oracle.com>
>> ---
>> hw/core/cpu-common.c | 1 +
>> monitor/hmp-cmds-target.c | 2 --
>> softmmu/cpus.c | 1 -
>> 3 files changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
>> index 5ccc3837b6..d2503f2d09 100644
>> --- a/hw/core/cpu-common.c
>> +++ b/hw/core/cpu-common.c
>> @@ -105,6 +105,7 @@ void cpu_dump_state(CPUState *cpu, FILE *f, int flags)
>> if (cc->dump_state) {
>> cpu_synchronize_state(cpu);
>
> Should we check for:
>
> if (cpu->cpu_index != -1) {
>
>> + qemu_fprintf(f, "\nCPU#%d\n", cpu->cpu_index);
>
> }
I think you meant if (cpu->cpu_index != UNASSIGNED_CPU_INDEX).
I do not see this case may happen within my knowledge. The cpu_index is always
expected to be assigned if cpu_exec_realizefn()-->cpu_list_add() is called.
83 void cpu_list_add(CPUState *cpu)
84 {
85 QEMU_LOCK_GUARD(&qemu_cpu_list_lock);
86 if (cpu->cpu_index == UNASSIGNED_CPU_INDEX) {
87 cpu->cpu_index = cpu_get_free_index();
88 assert(cpu->cpu_index != UNASSIGNED_CPU_INDEX);
89 } else {
90 assert(!cpu_index_auto_assigned);
91 }
92 QTAILQ_INSERT_TAIL_RCU(&cpus, cpu, node);
93 cpu_list_generation_id++;
94 }
In addition, the cc->dump_state() is always invoked by cpu_dump_state(). As a
result, e.g., arm_cpu_dump_state() or x86_cpu_dump_state() may always print the
cpu state unconditionally (same for mips, s390 or riscv). I do not see a reason
to hide the cpu_index.
Would you please let me know if the above is wrong? I do not think it is
required to filter the cpu_index with UNASSIGNED_CPU_INDEX.
Thank you very much!
Dongli Zhang
>
> ?
>
>> cc->dump_state(cpu, f, flags);
>> }
>> }
>> diff --git a/monitor/hmp-cmds-target.c b/monitor/hmp-cmds-target.c
>> index 0d3e84d960..f7dd354d2a 100644
>> --- a/monitor/hmp-cmds-target.c
>> +++ b/monitor/hmp-cmds-target.c
>> @@ -99,7 +99,6 @@ void hmp_info_registers(Monitor *mon, const QDict *qdict)
>> if (all_cpus) {
>> CPU_FOREACH(cs) {
>> - monitor_printf(mon, "\nCPU#%d\n", cs->cpu_index);
>> cpu_dump_state(cs, NULL, CPU_DUMP_FPU);
>> }
>
