Re: [PATCH 1/1] hw/core/cpu: always print cpu index with cpu state
On Tue, Feb 7, 2023 at 9:46 AM 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=0002 RBX=8a9e57c38400 RCX= > RDX=8a9cc00ba8a0 > RSI=0003 RDI=8a9e57c38400 RBP=b6120c5b3c50 > RSP=b6120c5b3c40 > R8 = R9 =8a9cc00ba8a0 R10=8e467350 > R11=0007 > R12=000a R13=8f987e25 R14=8f988a01 > R15= > RIP=8e51bb04 RFL=00010046 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0 > ES = 00c0 > CS =0010 00a09b00 DPL=0 CS64 [-RA] > SS = 00c0 > DS = 00c0 > FS = 00c0 > GS = 8ac27fcc 00c0 > LDT= 00c0 > TR =0040 fe096000 206f 8b00 DPL=0 TSS64-busy > GDT= fe094000 007f > IDT= fe00 0fff > CR0=80050033 CR2= CR3=0010ca40a001 CR4=003606e0 > DR0= DR1= DR2= > DR3= > DR6=fffe0ff0 DR7=0400 > EFER=0d01 > Code=0f 1f ... ... > > Print the cpu->cpu_index in cpu_dump_state() and remove it from the caller. > > Cc: Joe Jin > Signed-off-by: Dongli Zhang Any more comments or thoughts? Alistair
Re: [PATCH 1/1] hw/core/cpu: always print cpu index with cpu state
Ping?
The patch already got the Reviewed-by from Philippe Mathieu-Daudé and Alistair
Francis.
The current log does not provide much information when (1) multiple CPUs are
involved in the bug, and (2) when the "info registers -a" is not used to collect
the context from all CPUs for comparison.
We may wait for very long timer until the bug filer can reproduce the error
again.
To print the cpu index helps a lot.
Thank you very much!
Dongli Zhang
On 2/6/23 15: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=0002 RBX=8a9e57c38400 RCX=
> RDX=8a9cc00ba8a0
> RSI=0003 RDI=8a9e57c38400 RBP=b6120c5b3c50
> RSP=b6120c5b3c40
> R8 = R9 =8a9cc00ba8a0 R10=8e467350
> R11=0007
> R12=000a R13=8f987e25 R14=8f988a01
> R15=
> RIP=8e51bb04 RFL=00010046 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
> ES = 00c0
> CS =0010 00a09b00 DPL=0 CS64 [-RA]
> SS = 00c0
> DS = 00c0
> FS = 00c0
> GS = 8ac27fcc 00c0
> LDT= 00c0
> TR =0040 fe096000 206f 8b00 DPL=0 TSS64-busy
> GDT= fe094000 007f
> IDT= fe00 0fff
> CR0=80050033 CR2= CR3=0010ca40a001 CR4=003606e0
> DR0= DR1= DR2=
> DR3=
> DR6=fffe0ff0 DR7=0400
> EFER=0d01
> Code=0f 1f ... ...
>
> Print the cpu->cpu_index in cpu_dump_state() and remove it from the caller.
>
> Cc: Joe Jin
> Signed-off-by: Dongli Zhang
> ---
> 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);
> +qemu_fprintf(f, "\nCPU#%d\n", cpu->cpu_index);
> 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);
> }
> } else {
> @@ -114,7 +113,6 @@ void hmp_info_registers(Monitor *mon, const QDict *qdict)
> return;
> }
>
> -monitor_printf(mon, "\nCPU#%d\n", cs->cpu_index);
> cpu_dump_state(cs, NULL, CPU_DUMP_FPU);
> }
> }
> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> index 9cbc8172b5..f69bbe6abc 100644
> --- a/softmmu/cpus.c
> +++ b/softmmu/cpus.c
> @@ -122,7 +122,6 @@ void hw_error(const char *fmt, ...)
> vfprintf(stderr, fmt, ap);
> fprintf(stderr, "\n");
> CPU_FOREACH(cpu) {
> -fprintf(stderr, "CPU #%d:\n", cpu->cpu_index);
> cpu_dump_state(cpu, stderr, CPU_DUMP_FPU);
> }
> va_end(ap);
Re: [PATCH 1/1] hw/core/cpu: always print cpu index with cpu state
On Tue, Feb 7, 2023 at 9:46 AM 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=0002 RBX=8a9e57c38400 RCX=
> RDX=8a9cc00ba8a0
> RSI=0003 RDI=8a9e57c38400 RBP=b6120c5b3c50
> RSP=b6120c5b3c40
> R8 = R9 =8a9cc00ba8a0 R10=8e467350
> R11=0007
> R12=000a R13=8f987e25 R14=8f988a01
> R15=
> RIP=8e51bb04 RFL=00010046 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
> ES = 00c0
> CS =0010 00a09b00 DPL=0 CS64 [-RA]
> SS = 00c0
> DS = 00c0
> FS = 00c0
> GS = 8ac27fcc 00c0
> LDT= 00c0
> TR =0040 fe096000 206f 8b00 DPL=0 TSS64-busy
> GDT= fe094000 007f
> IDT= fe00 0fff
> CR0=80050033 CR2= CR3=0010ca40a001 CR4=003606e0
> DR0= DR1= DR2=
> DR3=
> DR6=fffe0ff0 DR7=0400
> EFER=0d01
> Code=0f 1f ... ...
>
> Print the cpu->cpu_index in cpu_dump_state() and remove it from the caller.
>
> Cc: Joe Jin
> Signed-off-by: Dongli Zhang
Reviewed-by: Alistair Francis
Alistair
> ---
> 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);
> +qemu_fprintf(f, "\nCPU#%d\n", cpu->cpu_index);
> 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);
> }
> } else {
> @@ -114,7 +113,6 @@ void hmp_info_registers(Monitor *mon, const QDict *qdict)
> return;
> }
>
> -monitor_printf(mon, "\nCPU#%d\n", cs->cpu_index);
> cpu_dump_state(cs, NULL, CPU_DUMP_FPU);
> }
> }
> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> index 9cbc8172b5..f69bbe6abc 100644
> --- a/softmmu/cpus.c
> +++ b/softmmu/cpus.c
> @@ -122,7 +122,6 @@ void hw_error(const char *fmt, ...)
> vfprintf(stderr, fmt, ap);
> fprintf(stderr, "\n");
> CPU_FOREACH(cpu) {
> -fprintf(stderr, "CPU #%d:\n", cpu->cpu_index);
> cpu_dump_state(cpu, stderr, CPU_DUMP_FPU);
> }
> va_end(ap);
> --
> 2.34.1
>
>
Re: [PATCH 1/1] hw/core/cpu: always print cpu index with cpu state
On 7/2/23 18:32, Dongli Zhang wrote:
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=0002 RBX=8a9e57c38400 RCX=
RDX=8a9cc00ba8a0
RSI=0003 RDI=8a9e57c38400 RBP=b6120c5b3c50
RSP=b6120c5b3c40
R8 = R9 =8a9cc00ba8a0 R10=8e467350
R11=0007
R12=000a R13=8f987e25 R14=8f988a01
R15=
RIP=8e51bb04 RFL=00010046 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES = 00c0
CS =0010 00a09b00 DPL=0 CS64 [-RA]
SS = 00c0
DS = 00c0
FS = 00c0
GS = 8ac27fcc 00c0
LDT= 00c0
TR =0040 fe096000 206f 8b00 DPL=0 TSS64-busy
GDT= fe094000 007f
IDT= fe00 0fff
CR0=80050033 CR2= CR3=0010ca40a001 CR4=003606e0
DR0= DR1= DR2=
DR3=
DR6=fffe0ff0 DR7=0400
EFER=0d01
Code=0f 1f ... ...
Print the cpu->cpu_index in cpu_dump_state() and remove it from the caller.
Cc: Joe Jin
Signed-off-by: Dongli Zhang
---
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(_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(, 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.
You are right.
Reviewed-by: Philippe Mathieu-Daudé
Thanks for clarifying!
Re: [PATCH 1/1] hw/core/cpu: always print cpu index with cpu state
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=0002 RBX=8a9e57c38400 RCX=
>> RDX=8a9cc00ba8a0
>> RSI=0003 RDI=8a9e57c38400 RBP=b6120c5b3c50
>> RSP=b6120c5b3c40
>> R8 = R9 =8a9cc00ba8a0 R10=8e467350
>> R11=0007
>> R12=000a R13=8f987e25 R14=8f988a01
>> R15=
>> RIP=8e51bb04 RFL=00010046 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
>> ES = 00c0
>> CS =0010 00a09b00 DPL=0 CS64 [-RA]
>> SS = 00c0
>> DS = 00c0
>> FS = 00c0
>> GS = 8ac27fcc 00c0
>> LDT= 00c0
>> TR =0040 fe096000 206f 8b00 DPL=0 TSS64-busy
>> GDT= fe094000 007f
>> IDT= fe00 0fff
>> CR0=80050033 CR2= CR3=0010ca40a001 CR4=003606e0
>> DR0= DR1= DR2=
>> DR3=
>> DR6=fffe0ff0 DR7=0400
>> EFER=0d01
>> Code=0f 1f ... ...
>>
>> Print the cpu->cpu_index in cpu_dump_state() and remove it from the caller.
>>
>> Cc: Joe Jin
>> Signed-off-by: Dongli Zhang
>> ---
>> 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(_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(, 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);
>> }
>
Re: [PATCH 1/1] hw/core/cpu: always print cpu index with cpu state
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=0002 RBX=8a9e57c38400 RCX=
RDX=8a9cc00ba8a0
RSI=0003 RDI=8a9e57c38400 RBP=b6120c5b3c50
RSP=b6120c5b3c40
R8 = R9 =8a9cc00ba8a0 R10=8e467350
R11=0007
R12=000a R13=8f987e25 R14=8f988a01
R15=
RIP=8e51bb04 RFL=00010046 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES = 00c0
CS =0010 00a09b00 DPL=0 CS64 [-RA]
SS = 00c0
DS = 00c0
FS = 00c0
GS = 8ac27fcc 00c0
LDT= 00c0
TR =0040 fe096000 206f 8b00 DPL=0 TSS64-busy
GDT= fe094000 007f
IDT= fe00 0fff
CR0=80050033 CR2= CR3=0010ca40a001 CR4=003606e0
DR0= DR1= DR2=
DR3=
DR6=fffe0ff0 DR7=0400
EFER=0d01
Code=0f 1f ... ...
Print the cpu->cpu_index in cpu_dump_state() and remove it from the caller.
Cc: Joe Jin
Signed-off-by: Dongli Zhang
---
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);
}
?
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);
}
