[Qemu-devel] [PATCH] tcg: Optionally log FPU state in TCG -d cpu logging

[Qemu-devel] [PATCH] tcg: Optionally log FPU state in TCG -d cpu logging

[Peter Maydell]

Usually the logging of the CPU state produced by -d cpu is sufficient
to diagnose problems, but sometimes you want to see the state of
the floating point registers as well. We don't want to enable that
by default as it adds a lot of extra data to the log; instead,
allow it to be optionally enabled via -d fpu.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I've found this helpful while tracking down fp-emulation related bugs.

 include/qemu/log.h   | 1 +
 accel/tcg/cpu-exec.c | 9 ++++++---
 util/log.c           | 2 ++
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/include/qemu/log.h b/include/qemu/log.h
index ff92a8b86a..b097a6cae1 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -44,6 +44,7 @@ static inline bool qemu_log_separate(void)
 #define CPU_LOG_PAGE       (1 << 14)
 /* LOG_TRACE (1 << 15) is defined in log-for-trace.h */
 #define CPU_LOG_TB_OP_IND  (1 << 16)
+#define CPU_LOG_TB_FPU     (1 << 17)
 
 /* Lock output for a series of related logs.  Since this is not needed
  * for a single qemu_log / qemu_log_mask / qemu_log_mask_and_addr, we
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 81153e7a13..0b154cc678 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -156,11 +156,14 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, TranslationBlock *itb)
     if (qemu_loglevel_mask(CPU_LOG_TB_CPU)
         && qemu_log_in_addr_range(itb->pc)) {
         qemu_log_lock();
+        int flags = 0;
+        if (qemu_loglevel_mask(CPU_LOG_TB_FPU)) {
+            flags |= CPU_DUMP_FPU;
+        }
 #if defined(TARGET_I386)
-        log_cpu_state(cpu, CPU_DUMP_CCOP);
-#else
-        log_cpu_state(cpu, 0);
+        flags |= CPU_DUMP_CCOP;
 #endif
+        log_cpu_state(cpu, flags);
         qemu_log_unlock();
     }
 #endif /* DEBUG_DISAS */
diff --git a/util/log.c b/util/log.c
index 96f30dd21a..c0dbbd4700 100644
--- a/util/log.c
+++ b/util/log.c
@@ -256,6 +256,8 @@ const QEMULogItem qemu_log_items[] = {
       "show trace before each executed TB (lots of logs)" },
     { CPU_LOG_TB_CPU, "cpu",
       "show CPU registers before entering a TB (lots of logs)" },
+    { CPU_LOG_TB_FPU, "fpu",
+      "include FPU registers in the 'cpu' logging" },
     { CPU_LOG_MMU, "mmu",
       "log MMU-related activities" },
     { CPU_LOG_PCALL, "pcall",
-- 
2.17.0

Re: [Qemu-devel] [PATCH] tcg: Optionally log FPU state in TCG -d cpu logging

[Richard Henderson]

On 05/10/2018 06:00 AM, Peter Maydell wrote:
> Usually the logging of the CPU state produced by -d cpu is sufficient
> to diagnose problems, but sometimes you want to see the state of
> the floating point registers as well. We don't want to enable that
> by default as it adds a lot of extra data to the log; instead,
> allow it to be optionally enabled via -d fpu.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I've found this helpful while tracking down fp-emulation related bugs.
> 
>  include/qemu/log.h   | 1 +
>  accel/tcg/cpu-exec.c | 9 ++++++---
>  util/log.c           | 2 ++
>  3 files changed, 9 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

I'll also note that only i386 and arm check this flag;
something to fix for the rest...


r~

Re: [Qemu-devel] [PATCH] tcg: Optionally log FPU state in TCG -d cpu logging

[Peter Maydell]

On 10 May 2018 at 15:36, Richard Henderson <rth@twiddle.net> wrote:
> On 05/10/2018 06:00 AM, Peter Maydell wrote:
>> Usually the logging of the CPU state produced by -d cpu is sufficient
>> to diagnose problems, but sometimes you want to see the state of
>> the floating point registers as well. We don't want to enable that
>> by default as it adds a lot of extra data to the log; instead,
>> allow it to be optionally enabled via -d fpu.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> I've found this helpful while tracking down fp-emulation related bugs.
>>
>>  include/qemu/log.h   | 1 +
>>  accel/tcg/cpu-exec.c | 9 ++++++---
>>  util/log.c           | 2 ++
>>  3 files changed, 9 insertions(+), 3 deletions(-)
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> I'll also note that only i386 and arm check this flag;
> something to fix for the rest...

Mmm. It would also be nice not to have that TARGET_I386
special case for CPU_DUMP_CCOP -- we should either care about
that for everything, or for nothing (my vote would be for not
printing it unless user-requested, since it's an internal
tcg target detail.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH] tcg: Optionally log FPU state in TCG -d cpu logging

[Philippe Mathieu-Daudé]

On 05/10/2018 11:36 AM, Richard Henderson wrote:
> On 05/10/2018 06:00 AM, Peter Maydell wrote:
>> Usually the logging of the CPU state produced by -d cpu is sufficient
>> to diagnose problems, but sometimes you want to see the state of
>> the floating point registers as well. We don't want to enable that
>> by default as it adds a lot of extra data to the log; instead,
>> allow it to be optionally enabled via -d fpu.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> I've found this helpful while tracking down fp-emulation related bugs.
>>
>>  include/qemu/log.h   | 1 +
>>  accel/tcg/cpu-exec.c | 9 ++++++---
>>  util/log.c           | 2 ++
>>  3 files changed, 9 insertions(+), 3 deletions(-)
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> I'll also note that only i386 and arm check this flag;
> something to fix for the rest...

Indeed, this would reduce mips_cpu_dump_state() output.

Re: [Qemu-devel] [PATCH] tcg: Optionally log FPU state in TCG -d cpu logging

[Paolo Bonzini]

On 10/05/2018 16:38, Peter Maydell wrote:
> On 10 May 2018 at 15:36, Richard Henderson <rth@twiddle.net> wrote:
>> On 05/10/2018 06:00 AM, Peter Maydell wrote:
>>> Usually the logging of the CPU state produced by -d cpu is sufficient
>>> to diagnose problems, but sometimes you want to see the state of
>>> the floating point registers as well. We don't want to enable that
>>> by default as it adds a lot of extra data to the log; instead,
>>> allow it to be optionally enabled via -d fpu.
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>> I've found this helpful while tracking down fp-emulation related bugs.
>>>
>>>  include/qemu/log.h   | 1 +
>>>  accel/tcg/cpu-exec.c | 9 ++++++---
>>>  util/log.c           | 2 ++
>>>  3 files changed, 9 insertions(+), 3 deletions(-)
>>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>
>> I'll also note that only i386 and arm check this flag;
>> something to fix for the rest...
> 
> Mmm. It would also be nice not to have that TARGET_I386
> special case for CPU_DUMP_CCOP -- we should either care about
> that for everything, or for nothing (my vote would be for not
> printing it unless user-requested, since it's an internal
> tcg target detail.)

That's fine by me.

Paolo