| Message ID | 20230711165434.4123674-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [for-8.1] accel/tcg: Zero-pad PC in TCG CPU exec trace lines | expand |
On 11/7/23 18:54, Peter Maydell wrote: > In commit f0a08b0913befbd we changed the type of the PC from > target_ulong to vaddr. In doing so we inadvertently dropped the > zero-padding on the PC in trace lines (the second item inside the [] > in these lines). They used to look like this on AArch64, for > instance: > > Trace 0: 0x7f2260000100 [00000000/0000000040000000/00000061/ff200000] > > and now they look like this: > Trace 0: 0x7f4f50000100 [00000000/40000000/00000061/ff200000] > > and if the PC happens to be somewhere low like 0x5000 > then the field is shown as /5000/. > > This is because TARGET_FMT_lx is a "%08x" or "%016x" specifier, > depending on TARGET_LONG_SIZE, whereas VADDR_PRIx is just PRIx64 > with no width specifier. > > Restore the zero-padding by adding an 016 width specifier to > this tracing and a couple of others that were similarly recently > changed to use VADDR_PRIx without a width specifier. > > We can't unfortunately restore the "32-bit guests are padded to > 8 hex digits and 64-bit guests to 16 hex digits" behaviour so > easily. > > Fixes: f0a08b0913befbd ("accel/tcg/cpu-exec.c: Widen pc to vaddr") > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > I have a workflow that parses log files to get the executed > PC values; I don't suppose I'm the only one doing that... > --- > accel/tcg/cpu-exec.c | 4 ++-- > accel/tcg/translate-all.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> In commit f0a08b0913befbd we changed the type of the PC from > target_ulong to vaddr. In doing so we inadvertently dropped the > zero-padding on the PC in trace lines (the second item inside the [] > in these lines). They used to look like this on AArch64, for > instance: > > Trace 0: 0x7f2260000100 [00000000/0000000040000000/00000061/ff200000] > > and now they look like this: > Trace 0: 0x7f4f50000100 [00000000/40000000/00000061/ff200000] > > and if the PC happens to be somewhere low like 0x5000 > then the field is shown as /5000/. > > This is because TARGET_FMT_lx is a "%08x" or "%016x" specifier, > depending on TARGET_LONG_SIZE, whereas VADDR_PRIx is just PRIx64 > with no width specifier. > > Restore the zero-padding by adding an 016 width specifier to > this tracing and a couple of others that were similarly recently > changed to use VADDR_PRIx without a width specifier. > > We can't unfortunately restore the "32-bit guests are padded to > 8 hex digits and 64-bit guests to 16 hex digits" behaviour so > easily. > > Fixes: f0a08b0913befbd ("accel/tcg/cpu-exec.c: Widen pc to vaddr") > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > I have a workflow that parses log files to get the executed > PC values; I don't suppose I'm the only one doing that... > --- > accel/tcg/cpu-exec.c | 4 ++-- > accel/tcg/translate-all.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c > index ba1890a373d..db1e82811fa 100644 > --- a/accel/tcg/cpu-exec.c > +++ b/accel/tcg/cpu-exec.c > @@ -298,7 +298,7 @@ static void log_cpu_exec(vaddr pc, CPUState *cpu, > if (qemu_log_in_addr_range(pc)) { > qemu_log_mask(CPU_LOG_EXEC, > "Trace %d: %p [%08" PRIx64 > - "/%" VADDR_PRIx "/%08x/%08x] %s\n", > + "/%016" VADDR_PRIx "/%08x/%08x] %s\n", > cpu->cpu_index, tb->tc.ptr, tb->cs_base, pc, > tb->flags, tb->cflags, lookup_symbol(pc)); > > @@ -487,7 +487,7 @@ cpu_tb_exec(CPUState *cpu, TranslationBlock *itb, int *tb_exit) > if (qemu_loglevel_mask(CPU_LOG_EXEC)) { > vaddr pc = log_pc(cpu, last_tb); > if (qemu_log_in_addr_range(pc)) { > - qemu_log("Stopped execution of TB chain before %p [%" > + qemu_log("Stopped execution of TB chain before %p [%016" > VADDR_PRIx "] %s\n", > last_tb->tc.ptr, pc, lookup_symbol(pc)); > } > diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c > index d3d4fbc1a41..bb225afa04f 100644 > --- a/accel/tcg/translate-all.c > +++ b/accel/tcg/translate-all.c > @@ -604,7 +604,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr) > if (qemu_loglevel_mask(CPU_LOG_EXEC)) { > vaddr pc = log_pc(cpu, tb); > if (qemu_log_in_addr_range(pc)) { > - qemu_log("cpu_io_recompile: rewound execution of TB to %" > + qemu_log("cpu_io_recompile: rewound execution of TB to %016" > VADDR_PRIx "\n", pc); > } > } This was definitely a mistake on my part, thanks for the quick fix! Reviewed-by: Anton Johansson <anjo@rev.ng>
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index ba1890a373d..db1e82811fa 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -298,7 +298,7 @@ static void log_cpu_exec(vaddr pc, CPUState *cpu, if (qemu_log_in_addr_range(pc)) { qemu_log_mask(CPU_LOG_EXEC, "Trace %d: %p [%08" PRIx64 - "/%" VADDR_PRIx "/%08x/%08x] %s\n", + "/%016" VADDR_PRIx "/%08x/%08x] %s\n", cpu->cpu_index, tb->tc.ptr, tb->cs_base, pc, tb->flags, tb->cflags, lookup_symbol(pc)); @@ -487,7 +487,7 @@ cpu_tb_exec(CPUState *cpu, TranslationBlock *itb, int *tb_exit) if (qemu_loglevel_mask(CPU_LOG_EXEC)) { vaddr pc = log_pc(cpu, last_tb); if (qemu_log_in_addr_range(pc)) { - qemu_log("Stopped execution of TB chain before %p [%" + qemu_log("Stopped execution of TB chain before %p [%016" VADDR_PRIx "] %s\n", last_tb->tc.ptr, pc, lookup_symbol(pc)); } diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index d3d4fbc1a41..bb225afa04f 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -604,7 +604,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr) if (qemu_loglevel_mask(CPU_LOG_EXEC)) { vaddr pc = log_pc(cpu, tb); if (qemu_log_in_addr_range(pc)) { - qemu_log("cpu_io_recompile: rewound execution of TB to %" + qemu_log("cpu_io_recompile: rewound execution of TB to %016" VADDR_PRIx "\n", pc); } }
In commit f0a08b0913befbd we changed the type of the PC from target_ulong to vaddr. In doing so we inadvertently dropped the zero-padding on the PC in trace lines (the second item inside the [] in these lines). They used to look like this on AArch64, for instance: Trace 0: 0x7f2260000100 [00000000/0000000040000000/00000061/ff200000] and now they look like this: Trace 0: 0x7f4f50000100 [00000000/40000000/00000061/ff200000] and if the PC happens to be somewhere low like 0x5000 then the field is shown as /5000/. This is because TARGET_FMT_lx is a "%08x" or "%016x" specifier, depending on TARGET_LONG_SIZE, whereas VADDR_PRIx is just PRIx64 with no width specifier. Restore the zero-padding by adding an 016 width specifier to this tracing and a couple of others that were similarly recently changed to use VADDR_PRIx without a width specifier. We can't unfortunately restore the "32-bit guests are padded to 8 hex digits and 64-bit guests to 16 hex digits" behaviour so easily. Fixes: f0a08b0913befbd ("accel/tcg/cpu-exec.c: Widen pc to vaddr") Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- I have a workflow that parses log files to get the executed PC values; I don't suppose I'm the only one doing that... --- accel/tcg/cpu-exec.c | 4 ++-- accel/tcg/translate-all.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)