qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [RFC 16/19] target-ppc: Refactor debug output macros


From: Anthony Liguori
Subject: Re: [Qemu-ppc] [RFC 16/19] target-ppc: Refactor debug output macros
Date: Sun, 27 Jan 2013 08:14:48 -0600
User-agent: Notmuch/0.13.2+93~ged93d79 (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu)

Andreas Färber <address@hidden> writes:

> Make debug output compile-testable even if disabled.
>
> Inline DEBUG_OP check in excp_helper.c.
> Inline LOG_MMU_STATE() in mmu_helper.c.
> Inline PPC_DEBUG_SPR check in translate_init.c.
>
> Signed-off-by: Andreas Färber <address@hidden>
> ---
>  target-ppc/excp_helper.c    |   22 +++++++--------
>  target-ppc/kvm.c            |    9 ++-----
>  target-ppc/mem_helper.c     |    2 --
>  target-ppc/mmu_helper.c     |   63 
> +++++++++++++++++++++----------------------
>  target-ppc/translate.c      |   12 ++++-----
>  target-ppc/translate_init.c |   10 +++----
>  6 Dateien geändert, 55 Zeilen hinzugefügt(+), 63 Zeilen entfernt(-)
>
> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
> index 0a1ac86..54722c4 100644
> --- a/target-ppc/excp_helper.c
> +++ b/target-ppc/excp_helper.c
> @@ -21,14 +21,14 @@
>  
>  #include "helper_regs.h"
>  
> -//#define DEBUG_OP
> -//#define DEBUG_EXCEPTIONS
> +#define DEBUG_OP 0
> +#define DEBUG_EXCEPTIONS 0
>  
> -#ifdef DEBUG_EXCEPTIONS
> -#  define LOG_EXCP(...) qemu_log(__VA_ARGS__)
> -#else
> -#  define LOG_EXCP(...) do { } while (0)
> -#endif
> +#define LOG_EXCP(...) G_STMT_START \
> +    if (DEBUG_EXCEPTIONS) { \
> +        qemu_log(__VA_ARGS__); \
> +    } \
> +    G_STMT_END

Just thinking out loud a bit..  This form becomes pretty common and it's
ashame to use a macro here if we don't have to.

I think:

static inline void LOG_EXCP(const char *fmt, ...)
{
    if (debug_exceptions) {
       va_list ap;
       va_start(ap, fmt);
       qemu_logv(fmt, ap);
       va_end(ap);
    }
}

Probably would have equivalent performance.  debug_exception would be
read-mostly and ought to be very predictable as a result.  I strongly
expect that the compiler would actually inline LOG_EXCP too.

I see LOG_EXCP and LOG_DIS in this series.  Perhaps we could just
introduce these functions and then make these flags run-time
controllable?

BTW, one advantage of this over your original proposal back to your
point is that you still won't catch linker errors with your proposal.
Dead code eliminate will kill off those branches before the linker ever
sees them.

Regards,

Anthony Liguori

>  
>  
> /*****************************************************************************/
>  /* PowerPC Hypercall emulation */
> @@ -777,7 +777,7 @@ void ppc_hw_interrupt(CPUPPCState *env)
>  }
>  #endif /* !CONFIG_USER_ONLY */
>  
> -#if defined(DEBUG_OP)
> +#ifndef CONFIG_USER_ONLY
>  static void cpu_dump_rfi(target_ulong RA, target_ulong msr)
>  {
>      qemu_log("Return from exception at " TARGET_FMT_lx " with flags "
> @@ -835,9 +835,9 @@ static inline void do_rfi(CPUPPCState *env, target_ulong 
> nip, target_ulong msr,
>      /* XXX: beware: this is false if VLE is supported */
>      env->nip = nip & ~((target_ulong)0x00000003);
>      hreg_store_msr(env, msr, 1);
> -#if defined(DEBUG_OP)
> -    cpu_dump_rfi(env->nip, env->msr);
> -#endif
> +    if (DEBUG_OP) {
> +        cpu_dump_rfi(env->nip, env->msr);
> +    }
>      /* No need to raise an exception here,
>       * as rfi is always the last insn of a TB
>       */
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 2f4f068..0dc6657 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -37,15 +37,10 @@
>  #include "hw/spapr.h"
>  #include "hw/spapr_vio.h"
>  
> -//#define DEBUG_KVM
> +#define DEBUG_KVM 0
>  
> -#ifdef DEBUG_KVM
>  #define dprintf(fmt, ...) \
> -    do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
> -#else
> -#define dprintf(fmt, ...) \
> -    do { } while (0)
> -#endif
> +    do { if (DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } } while (0)
>  
>  #define PROC_DEVTREE_CPU      "/proc/device-tree/cpus/"
>  
> diff --git a/target-ppc/mem_helper.c b/target-ppc/mem_helper.c
> index 902b1cd..5c7a5ce 100644
> --- a/target-ppc/mem_helper.c
> +++ b/target-ppc/mem_helper.c
> @@ -26,8 +26,6 @@
>  #include "exec/softmmu_exec.h"
>  #endif /* !defined(CONFIG_USER_ONLY) */
>  
> -//#define DEBUG_OP
> -
>  
> /*****************************************************************************/
>  /* Memory load and stores */
>  
> diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
> index ee168f1..9340fbb 100644
> --- a/target-ppc/mmu_helper.c
> +++ b/target-ppc/mmu_helper.c
> @@ -21,39 +21,36 @@
>  #include "sysemu/kvm.h"
>  #include "kvm_ppc.h"
>  
> -//#define DEBUG_MMU
> -//#define DEBUG_BATS
> -//#define DEBUG_SLB
> -//#define DEBUG_SOFTWARE_TLB
> +#define DEBUG_MMU 0
> +#define DEBUG_BATS 0
> +#define DEBUG_SLB 0
> +#define DEBUG_SOFTWARE_TLB 0
>  //#define DUMP_PAGE_TABLES
> -//#define DEBUG_SOFTWARE_TLB
>  //#define FLUSH_ALL_TLBS
>  
> -#ifdef DEBUG_MMU
> -#  define LOG_MMU(...) qemu_log(__VA_ARGS__)
> -#  define LOG_MMU_STATE(env) log_cpu_state((env), 0)
> -#else
> -#  define LOG_MMU(...) do { } while (0)
> -#  define LOG_MMU_STATE(...) do { } while (0)
> -#endif
> -
> -#ifdef DEBUG_SOFTWARE_TLB
> -#  define LOG_SWTLB(...) qemu_log(__VA_ARGS__)
> -#else
> -#  define LOG_SWTLB(...) do { } while (0)
> -#endif
> -
> -#ifdef DEBUG_BATS
> -#  define LOG_BATS(...) qemu_log(__VA_ARGS__)
> -#else
> -#  define LOG_BATS(...) do { } while (0)
> -#endif
> -
> -#ifdef DEBUG_SLB
> -#  define LOG_SLB(...) qemu_log(__VA_ARGS__)
> -#else
> -#  define LOG_SLB(...) do { } while (0)
> -#endif
> +#define LOG_MMU(...) G_STMT_START \
> +    if (DEBUG_MMU) { \
> +        qemu_log(__VA_ARGS__); \
> +    } \
> +    G_STMT_END
> +
> +#define LOG_SWTLB(...) G_STMT_START \
> +    if (DEBUG_SOFTWARE_TLB) { \
> +        qemu_log(__VA_ARGS__); \
> +    } \
> +    G_STMT_END
> +
> +#define LOG_BATS(...) G_STMT_START \
> +    if (DEBUG_BATS) { \
> +        qemu_log(__VA_ARGS__); \
> +    } \
> +    G_STMT_END
> +
> +#define LOG_SLB(...) G_STMT_START \
> +    if (DEBUG_SLB) { \
> +        qemu_log(__VA_ARGS__); \
> +    } \
> +    G_STMT_END
>  
>  
> /*****************************************************************************/
>  /* PowerPC MMU emulation */
> @@ -534,7 +531,7 @@ static inline int get_bat(CPUPPCState *env, mmu_ctx_t 
> *ctx,
>          }
>      }
>      if (ret < 0) {
> -#if defined(DEBUG_BATS)
> +#if DEBUG_BATS
>          if (qemu_log_enabled()) {
>              LOG_BATS("no BAT match for " TARGET_FMT_lx ":\n", virtual);
>              for (i = 0; i < 4; i++) {
> @@ -1860,7 +1857,9 @@ int cpu_ppc_handle_mmu_fault(CPUPPCState *env, 
> target_ulong address, int rw,
>                       mmu_idx, TARGET_PAGE_SIZE);
>          ret = 0;
>      } else if (ret < 0) {
> -        LOG_MMU_STATE(env);
> +        if (DEBUG_MMU) {
> +            log_cpu_state(env, 0);
> +        }
>          if (access_type == ACCESS_CODE) {
>              switch (ret) {
>              case -1:
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 37ce55f..c599fa7 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -32,14 +32,14 @@
>  #define GDBSTUB_SINGLE_STEP 0x4
>  
>  /* Include definitions for instructions classes and implementations flags */
> -//#define PPC_DEBUG_DISAS
> +#define PPC_DEBUG_DISAS 0
>  //#define DO_PPC_STATISTICS
>  
> -#ifdef PPC_DEBUG_DISAS
> -#  define LOG_DISAS(...) qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__)
> -#else
> -#  define LOG_DISAS(...) do { } while (0)
> -#endif
> +#define LOG_DISAS(...) G_STMT_START \
> +    if (PPC_DEBUG_DISAS) { \
> +        qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__); \
> +    } \
> +    G_STMT_END
>  
> /*****************************************************************************/
>  /* Code translation helpers                                                  
> */
>  
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 4f767c9..7ab4d61 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -31,7 +31,7 @@
>  #include "sysemu/cpus.h"
>  
>  //#define PPC_DUMP_CPU
> -//#define PPC_DEBUG_SPR
> +#define PPC_DEBUG_SPR 0
>  //#define PPC_DUMP_SPR_ACCESSES
>  #if defined(CONFIG_USER_ONLY)
>  #define TODO_USER_ONLY 1
> @@ -611,10 +611,10 @@ static inline void spr_register (CPUPPCState *env, int 
> num,
>          printf("Error: Trying to register SPR %d (%03x) twice !\n", num, 
> num);
>          exit(1);
>      }
> -#if defined(PPC_DEBUG_SPR)
> -    printf("*** register spr %d (%03x) %s val " TARGET_FMT_lx "\n", num, num,
> -           name, initial_value);
> -#endif
> +    if (PPC_DEBUG_SPR) {
> +        printf("*** register spr %d (%03x) %s val " TARGET_FMT_lx "\n",
> +               num, num, name, initial_value);
> +    }
>      spr->name = name;
>      spr->uea_read = uea_read;
>      spr->uea_write = uea_write;
> -- 
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to address@hidden
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



reply via email to

[Prev in Thread] Current Thread [Next in Thread]