On Wed, Oct 7, 2026 at 7:02 PM Daniel P. Berrangé <[email protected]> wrote:
>
> On Wed, Oct 07, 2026 at 12:47:24PM +0200, Philippe Mathieu-Daudé wrote:
> > On 7/10/26 11:44, Philippe Mathieu-Daudé wrote:
> > > Add log_raise_exception(), a common helper cpu_loop_*() can
> > > call to log a raised exception when '-d int' is used, and
> > > qemu_excp_name() to resolve QEMU-internal exceptions to names.
> > >
> > > Use it in cpu_loop_exit_atomic() to log the EXCP_ATOMIC reason.
> > >
> > > Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
> > > ---
> > > accel/tcg/cpu-exec-common.c | 32 ++++++++++++++++++++++++++++++++
> > > 1 file changed, 32 insertions(+)
> > >
> > > diff --git a/accel/tcg/cpu-exec-common.c b/accel/tcg/cpu-exec-common.c
> > > index 68077c8fd9d..2ac8380d0bb 100644
> > > --- a/accel/tcg/cpu-exec-common.c
> > > +++ b/accel/tcg/cpu-exec-common.c
> > > @@ -89,6 +89,37 @@ void tcg_update_all_cflags(void)
> > > }
> > > }
> > > +#define EXCP_QEMU_INTERNAL 0x10000
> > > +
> > > +static const char *qemu_excp_name(int excp)
> > > +{
> > > + static const char *excp_name[] = {
> > > + [EXCP_INTERRUPT - EXCP_QEMU_INTERNAL] = "async interruption",
> > > + [EXCP_HLT - EXCP_QEMU_INTERNAL] = "hlt instruction",
> > > + [EXCP_DEBUG - EXCP_QEMU_INTERNAL] = "debug",
> > > + [EXCP_HALTED - EXCP_QEMU_INTERNAL] = "halted",
> > > + [EXCP_YIELD - EXCP_QEMU_INTERNAL] = "yield",
> > > + [EXCP_ATOMIC - EXCP_QEMU_INTERNAL] = "emulate atomic",
> > > + };
> > > + assert(excp >= EXCP_QEMU_INTERNAL);
> > > + excp -= EXCP_QEMU_INTERNAL;
> > > + if ((excp < ARRAY_SIZE(excp_name)) && excp_name[excp]) {
> > > + return excp_name[excp];
> > > + }
> > > + return "undocumented";
> > > +}
> > > +
> > > +static void log_raise_exception(const CPUState *cpu, int excp)
> > > +{
> > > + if (!qemu_loglevel_mask(CPU_LOG_INT)) {
> > > + return;
> > > + }
> > > + if (excp >= EXCP_QEMU_INTERNAL) {
> > > + qemu_log_mask(CPU_LOG_INT, "raise exception: 0x%x (internal:
> > > %s)\n",
> > > + excp, qemu_excp_name(excp));
> >
> > Daniel suggested on IRC we can use target_name() here to distinguish
> > between different kinds of vCPUs. Alternatively we can add a
> > CPUClass::log_friendly_format_name() defaulting to current "%p index:%d"
> > but overwritable i.e. as "accel:%s target:%s cluster:%d index:%u", so
> > each time we log something on a vCPU context we have an unified prefix
> > format to parse.
>
> That would be problematic. Those two different format strings have
> different args. Also since it is not a static string, it can't be
> concatenated with the format strnig being passed to qemu_log_mask().
> So that would imply sprintf'ing the CPU format string prefix first
> with whatever args it needs, before passing the result into the
> qemu_log_mask() call.
>
> g_autofree char *prefix = g_strdup_printf(
> "accel:%s target:%s cluster:%d index:%u",
> accel, targe,t cluster, index);
> cpu_log_mask(CPU_LOG_INT,
> "%s raise exception: 0x%x (internal: %s)\n",
> prefix, excp, qemu_excp_name(excp));
>
> the double layers of printf makes logging quite unpleasant,
> and while it could be hidden in a macro, the runtime overhead
> still exists.
>
> On IRC, I suggested an approach that would work at the preprocessor
> level to concatenate the arch target name at compile time on to the
> front of all log messages, so there is no runtime impact at all,
> nor any code impact beyond s/qemu_log_mask/qemu_log_mask_target/.
>
> #define qemu_log_mask_target(MASK, FMT, ...) \
> do { \
> if (unlikely(qemu_loglevel_mask(MASK))) { \
> qemu_log(TARGET_NAME # FMT, ## __VA_ARGS__);\
> } \
> } while (0)
>
> Note, TARGE_NAME is poisoned, so we can't add this to to main
> include/qemu/log.h file. It would need to live in a header file
> that is built per-target, and thus only used from target specific
> code like the TCG impls.
>
> As a slight variation, *if* we can assume each target arch's VCPUs
> are in a separate thread, we could avoid using TARGET_NAME directly
> and instead use a thread local
>
> __thread const char *target_name;
I'm a bit confused, we already have target_name(), we can directly use
that, is that costing for logging?
the __thread things can be hidden in target_name() function wrapper.
>
> ...which would be initialized to TARGET_NAME when the vcpu's thread
> starts.
>
> Then we would change the macro to be
>
> #define qemu_log_mask_target(MASK, FMT, ...) \
> do { \
> if (unlikely(qemu_loglevel_mask(MASK))) { \
> qemu_log("%s" # FMT, target_name, ## __VA_ARGS__);\
> } \
> } while (0)
>
> That would make it possible to include more dynamic information
> by adding more thread-locals and hiding more fmt args in the
> macro.
>
> With regards,
> Daniel
> --
> |: https://berrange.com ~~ https://hachyderm.io/@berrange :|
> |: https://libvirt.org ~~ https://entangle-photo.org :|
> |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
>
>
--
此致
礼
罗勇刚
Yours
sincerely,
Yonggang Luo
