Re: [PATCH 1/2] util/log: Add vu to dump content of vector unit

[Víctor Colombo]

On 08/07/2022 05:57, Kito Cheng wrote:
> Add new option for -d vu to dump the content of vector unit, many target
> has vector register, but there is no easy way to dump the content, we
> use this on downstream for a while to help debug, and I feel that's
> really useful, so I think it would be great to upstream that to save debug time
> for other people :)
>
> Signed-off-by: Kito Cheng <kito.cheng@sifive.com>
> ---
>   accel/tcg/cpu-exec.c  | 3 +++
>   include/hw/core/cpu.h | 2 ++
>   include/qemu/log.h    | 1 +
>   util/log.c            | 2 ++
>   4 files changed, 8 insertions(+)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index a565a3f8ec..2cbec0a6ed 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -221,6 +221,9 @@ static inline void log_cpu_exec(target_ulong pc, CPUState 
> *cpu,
>                   if (qemu_loglevel_mask(CPU_LOG_TB_FPU)) {
>                       flags |= CPU_DUMP_FPU;
>                   }
> +                if (qemu_loglevel_mask(CPU_LOG_TB_VU)) {
> +                    flags |= CPU_DUMP_VU;
> +                }
>   #if defined(TARGET_I386)
>                   flags |= CPU_DUMP_CCOP;
>   #endif
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 996f94059f..7a767e17cd 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -535,11 +535,13 @@ GuestPanicInformation *cpu_get_crash_info(CPUState *cpu);
>    * @CPU_DUMP_CODE:
>    * @CPU_DUMP_FPU: dump FPU register state, not just integer
>    * @CPU_DUMP_CCOP: dump info about TCG QEMU's condition code optimization 
> state
> + * @CPU_DUMP_VU: dump vector register state
>    */
>   enum CPUDumpFlags {
>       CPU_DUMP_CODE = 0x00010000,
>       CPU_DUMP_FPU  = 0x00020000,
>       CPU_DUMP_CCOP = 0x00040000,
> +    CPU_DUMP_VU   = 0x00080000,
>   };
>
>   /**
> diff --git a/include/qemu/log.h b/include/qemu/log.h
> index c5643d8dd5..49bd0b0fbc 100644
> --- a/include/qemu/log.h
> +++ b/include/qemu/log.h
> @@ -35,6 +35,7 @@ bool qemu_log_separate(void);
>   /* LOG_STRACE is used for user-mode strace logging. */
>   #define LOG_STRACE         (1 << 19)
>   #define LOG_PER_THREAD     (1 << 20)
> +#define CPU_LOG_TB_VU      (1 << 21)
>
>   /* Lock/unlock output. */
>
> diff --git a/util/log.c b/util/log.c
> index d6eb0378c3..775d122c2e 100644
> --- a/util/log.c
> +++ b/util/log.c
> @@ -441,6 +441,8 @@ const QEMULogItem qemu_log_items[] = {
>   #ifdef CONFIG_PLUGIN
>       { CPU_LOG_PLUGIN, "plugin", "output from TCG plugins\n"},
>   #endif
> +    { CPU_LOG_TB_VU, "vu",
> +      "include vector unit registers in the 'cpu' logging" },
>       { LOG_STRACE, "strace",
>         "log every user-mode syscall, its input, and its result" },
>       { LOG_PER_THREAD, "tid",
> --
> 2.34.0

I think this looks like a good idea, can see myself using it.
I see a lot of places in the code where cpu_dump_state() is
used with CPU_DUMP_FPU flag as a parameter. Do you think it
is also the case to add this new flag CPU_DUMP_VU there too?

Best regards,

--
Víctor Cora Colombo
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>