| Message ID | 20200901112323.94969-2-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] cpus: Do not dump CPU state when calling hw_error() | expand |
On 01/09/2020 13.23, Philippe Mathieu-Daudé wrote: > We already have cpu_abort() to dump CPU states and abort. > > Restrict hw_error() to peripheral errors, hoping we can completely > remove it by proper functions from "error-report.h" in the future. > > Suggested-by: Thomas Huth <thuth@redhat.com> IIRC I rather suggested to rename the function to "cpu_hw_error" and only use it for real CPU problems... But I think your approach here is fine as well. Please replace the "Suggested-by" with "Reviewed-by" now :-) Thomas > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > softmmu/cpus.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/softmmu/cpus.c b/softmmu/cpus.c > index a802e899abb..c96a04d7f18 100644 > --- a/softmmu/cpus.c > +++ b/softmmu/cpus.c > @@ -913,16 +913,11 @@ static void stop_tcg_kick_timer(void) > void hw_error(const char *fmt, ...) > { > va_list ap; > - CPUState *cpu; > > va_start(ap, fmt); > fprintf(stderr, "qemu: hardware error: "); > 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); > - } You could argue that cpu_abort() only prints the state of one CPU and not of all. But I doubt that dumping the state of *all* CPUs is really helpful in any of the contexts where hw_error() is used. So I think it's fine to remove this CPU_FOREACH loop here. > va_end(ap); > abort(); > } >
On 9/2/20 8:33 AM, Thomas Huth wrote: > On 01/09/2020 13.23, Philippe Mathieu-Daudé wrote: >> We already have cpu_abort() to dump CPU states and abort. >> >> Restrict hw_error() to peripheral errors, hoping we can completely >> remove it by proper functions from "error-report.h" in the future. >> >> Suggested-by: Thomas Huth <thuth@redhat.com> > > IIRC I rather suggested to rename the function to "cpu_hw_error" and > only use it for real CPU problems... > But I think your approach here is fine as well. Please replace the > "Suggested-by" with "Reviewed-by" now :-) > > Thomas > > >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> softmmu/cpus.c | 5 ----- >> 1 file changed, 5 deletions(-) >> >> diff --git a/softmmu/cpus.c b/softmmu/cpus.c >> index a802e899abb..c96a04d7f18 100644 >> --- a/softmmu/cpus.c >> +++ b/softmmu/cpus.c >> @@ -913,16 +913,11 @@ static void stop_tcg_kick_timer(void) >> void hw_error(const char *fmt, ...) >> { >> va_list ap; >> - CPUState *cpu; >> >> va_start(ap, fmt); >> fprintf(stderr, "qemu: hardware error: "); >> 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); >> - } > > You could argue that cpu_abort() only prints the state of one CPU and > not of all. But I doubt that dumping the state of *all* CPUs is really > helpful in any of the contexts where hw_error() is used. So I think it's > fine to remove this CPU_FOREACH loop here. I find this not very practical when using more than 8 vCPUs. Personally I stopped looking at this output because it is too noisy. Are there developers interested in having cpu_abort() dumping all vCPU registers? I can move that there. Or the interested ones can also do it later if they find it useful :) > >> va_end(ap); >> abort(); >> } >> > >
diff --git a/softmmu/cpus.c b/softmmu/cpus.c index a802e899abb..c96a04d7f18 100644 --- a/softmmu/cpus.c +++ b/softmmu/cpus.c @@ -913,16 +913,11 @@ static void stop_tcg_kick_timer(void) void hw_error(const char *fmt, ...) { va_list ap; - CPUState *cpu; va_start(ap, fmt); fprintf(stderr, "qemu: hardware error: "); 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); abort(); }
We already have cpu_abort() to dump CPU states and abort. Restrict hw_error() to peripheral errors, hoping we can completely remove it by proper functions from "error-report.h" in the future. Suggested-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- softmmu/cpus.c | 5 ----- 1 file changed, 5 deletions(-)