Re: [PATCH 01/27] cpus: Add helper to log raised exceptions with '-d int'

["Daniel P. Berrangé" <berrange@redhat.com>]

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é <philmd@oss.qualcomm.com>
> > ---
> >   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;

...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 :|