diff mbox

[v2,12/15] target-ppc: Refactor debug output macros

Message ID 1361420711-15698-13-git-send-email-afaerber@suse.de
State New
Headers show

Commit Message

Andreas Färber Feb. 21, 2013, 4:25 a.m. UTC 
Make debug output compile-testable even if disabled.

Rename dprintf() in kvm.c to kvm_dprintf() to avoid conflict with glibc.

Inline DEBUG_OP check in excp_helper.c.
Inline LOG_MMU_STATE() in mmu_helper.c.
Inline PPC_{DEBUG_SPR,DUMP_SPR_ACCESSES} checks in translate_init.c.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-ppc/excp_helper.c    |   30 ++++++++++++++----
 target-ppc/kvm.c            |   28 +++++++++++------
 target-ppc/mem_helper.c     |    2 --
 target-ppc/mmu_helper.c     |   71 ++++++++++++++++++++++++++++++++++---------
 target-ppc/translate.c      |   15 +++++++--
 target-ppc/translate_init.c |   40 +++++++++++++++---------
 6 Dateien geändert, 137 Zeilen hinzugefügt(+), 49 Zeilen entfernt(-)

Comments

Alexander Graf Feb. 25, 2013, 12:54 p.m. UTC | #1 
On 21.02.2013, at 05:25, Andreas Färber wrote:

> Make debug output compile-testable even if disabled.
> 
> Rename dprintf() in kvm.c to kvm_dprintf() to avoid conflict with glibc.
> 
> Inline DEBUG_OP check in excp_helper.c.
> Inline LOG_MMU_STATE() in mmu_helper.c.
> Inline PPC_{DEBUG_SPR,DUMP_SPR_ACCESSES} checks in translate_init.c.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>

I assume you verified that all the bits do get optimized out, right? :)

Reviewed-by: Alexander Graf <agraf@suse.de>


Alex
Andreas Färber Feb. 25, 2013, 1:14 p.m. UTC | #2 
Am 25.02.2013 13:54, schrieb Alexander Graf:
> 
> On 21.02.2013, at 05:25, Andreas Färber wrote:
> 
>> Make debug output compile-testable even if disabled.
>>
>> Rename dprintf() in kvm.c to kvm_dprintf() to avoid conflict with glibc.
>>
>> Inline DEBUG_OP check in excp_helper.c.
>> Inline LOG_MMU_STATE() in mmu_helper.c.
>> Inline PPC_{DEBUG_SPR,DUMP_SPR_ACCESSES} checks in translate_init.c.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
> 
> I assume you verified that all the bits do get optimized out, right? :)

No, I didn't for each, my focus was to make debug code compile and keep
it compiling after my CPU changes. :)

Please read up on the new discussion of rth not liking static const and
proposing to go back to v1, modulo do { ... } while (0).

Like I said there, if finding a solution that pleases everyone fails,
then I will leave it to the maintainers (i.e., you) to choose and apply
a solution or to live with the resulting breakages.

Andreas

> 
> Reviewed-by: Alexander Graf <agraf@suse.de>
> 
> 
> Alex
>
Alexander Graf Feb. 25, 2013, 1:51 p.m. UTC | #3 
On 25.02.2013, at 14:14, Andreas Färber wrote:

> Am 25.02.2013 13:54, schrieb Alexander Graf:
>> 
>> On 21.02.2013, at 05:25, Andreas Färber wrote:
>> 
>>> Make debug output compile-testable even if disabled.
>>> 
>>> Rename dprintf() in kvm.c to kvm_dprintf() to avoid conflict with glibc.
>>> 
>>> Inline DEBUG_OP check in excp_helper.c.
>>> Inline LOG_MMU_STATE() in mmu_helper.c.
>>> Inline PPC_{DEBUG_SPR,DUMP_SPR_ACCESSES} checks in translate_init.c.
>>> 
>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> 
>> I assume you verified that all the bits do get optimized out, right? :)
> 
> No, I didn't for each, my focus was to make debug code compile and keep
> it compiling after my CPU changes. :)
> 
> Please read up on the new discussion of rth not liking static const and
> proposing to go back to v1, modulo do { ... } while (0).
> 
> Like I said there, if finding a solution that pleases everyone fails,
> then I will leave it to the maintainers (i.e., you) to choose and apply
> a solution or to live with the resulting breakages.

I care about 2 things:

  * #ifdef works
  * if not defined then the code should get optimized out

:). Everything else is icing on top.


Alex
diff mbox

Patch

diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
index 0a1ac86..9a11e1f 100644
--- a/target-ppc/excp_helper.c
+++ b/target-ppc/excp_helper.c
@@ -24,10 +24,28 @@ 
 //#define DEBUG_OP
 //#define DEBUG_EXCEPTIONS
 
+#ifdef DEBUG_OP
+static const bool debug_op = true;
+#else
+static const bool debug_op;
+#endif
+
 #ifdef DEBUG_EXCEPTIONS
-#  define LOG_EXCP(...) qemu_log(__VA_ARGS__)
+static const bool debug_exceptions = true;
 #else
-#  define LOG_EXCP(...) do { } while (0)
+static const bool debug_exceptions;
+#endif
+
+#ifndef CONFIG_USER_ONLY
+static void GCC_FMT_ATTR(1, 2) LOG_EXCP(const char *fmt, ...)
+{
+    if (debug_exceptions) {
+        va_list ap;
+        va_start(ap, fmt);
+        qemu_log_vprintf(fmt, ap);
+        va_end(ap);
+    }
+}
 #endif
 
 /*****************************************************************************/
@@ -777,7 +795,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 +853,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 2c64c63..997d400 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -40,13 +40,21 @@ 
 //#define DEBUG_KVM
 
 #ifdef DEBUG_KVM
-#define dprintf(fmt, ...) \
-    do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
+static const bool debug_kvm = true;
 #else
-#define dprintf(fmt, ...) \
-    do { } while (0)
+static const bool debug_kvm;
 #endif
 
+static void GCC_FMT_ATTR(1, 2) kvm_dprintf(const char *fmt, ...)
+{
+    if (debug_kvm) {
+        va_list ap;
+        va_start(ap, fmt);
+        vfprintf(stderr, fmt, ap);
+        va_end(ap);
+    }
+}
+
 #define PROC_DEVTREE_CPU      "/proc/device-tree/cpus/"
 
 const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
@@ -769,7 +777,7 @@  void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run)
          */
         irq = KVM_INTERRUPT_SET;
 
-        dprintf("injected interrupt %d\n", irq);
+        kvm_dprintf("injected interrupt %d\n", irq);
         r = kvm_vcpu_ioctl(cs, KVM_INTERRUPT, &irq);
         if (r < 0) {
             printf("cpu %d fail inject %x\n", cs->cpu_index, irq);
@@ -831,20 +839,20 @@  int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
     switch (run->exit_reason) {
     case KVM_EXIT_DCR:
         if (run->dcr.is_write) {
-            dprintf("handle dcr write\n");
+            kvm_dprintf("handle dcr write\n");
             ret = kvmppc_handle_dcr_write(env, run->dcr.dcrn, run->dcr.data);
         } else {
-            dprintf("handle dcr read\n");
+            kvm_dprintf("handle dcr read\n");
             ret = kvmppc_handle_dcr_read(env, run->dcr.dcrn, &run->dcr.data);
         }
         break;
     case KVM_EXIT_HLT:
-        dprintf("handle halt\n");
+        kvm_dprintf("handle halt\n");
         ret = kvmppc_handle_halt(env);
         break;
 #ifdef CONFIG_PSERIES
     case KVM_EXIT_PAPR_HCALL:
-        dprintf("handle PAPR hypercall\n");
+        kvm_dprintf("handle PAPR hypercall\n");
         run->papr_hcall.ret = spapr_hypercall(cpu,
                                               run->papr_hcall.nr,
                                               run->papr_hcall.args);
@@ -852,7 +860,7 @@  int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
         break;
 #endif
     case KVM_EXIT_EPR:
-        dprintf("handle epr\n");
+        kvm_dprintf("handle epr\n");
         run->epr.epr = ldl_phys(env->mpic_iack);
         ret = 0;
         break;
diff --git a/target-ppc/mem_helper.c b/target-ppc/mem_helper.c
index ba383c8..89c51d0 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 1cc1c16..42bf023 100644
--- a/target-ppc/mmu_helper.c
+++ b/target-ppc/mmu_helper.c
@@ -26,35 +26,76 @@ 
 //#define DEBUG_SLB
 //#define DEBUG_SOFTWARE_TLB
 //#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)
+static const bool debug_mmu = true;
 #else
-#  define LOG_MMU(...) do { } while (0)
-#  define LOG_MMU_STATE(...) do { } while (0)
+static const bool debug_mmu;
 #endif
 
 #ifdef DEBUG_SOFTWARE_TLB
-#  define LOG_SWTLB(...) qemu_log(__VA_ARGS__)
+static const bool debug_software_tlb = true;
 #else
-#  define LOG_SWTLB(...) do { } while (0)
+static const bool debug_software_tlb;
 #endif
 
 #ifdef DEBUG_BATS
-#  define LOG_BATS(...) qemu_log(__VA_ARGS__)
+static const bool debug_bats = true;
 #else
-#  define LOG_BATS(...) do { } while (0)
+static const bool debug_bats;
 #endif
 
 #ifdef DEBUG_SLB
-#  define LOG_SLB(...) qemu_log(__VA_ARGS__)
+static const bool debug_slb = true;
 #else
-#  define LOG_SLB(...) do { } while (0)
+static const bool debug_slb;
 #endif
 
+#ifndef CONFIG_USER_ONLY
+static void GCC_FMT_ATTR(1, 2) LOG_MMU(const char *fmt, ...)
+{
+    if (debug_mmu) {
+        va_list ap;
+        va_start(ap, fmt);
+        qemu_log_vprintf(fmt, ap);
+        va_end(ap);
+    }
+}
+
+static void GCC_FMT_ATTR(1, 2) LOG_SWTLB(const char *fmt, ...)
+{
+    if (debug_software_tlb) {
+        va_list ap;
+        va_start(ap, fmt);
+        qemu_log_vprintf(fmt, ap);
+        va_end(ap);
+    }
+}
+
+static void GCC_FMT_ATTR(1, 2) LOG_BATS(const char *fmt, ...)
+{
+    if (debug_bats) {
+        va_list ap;
+        va_start(ap, fmt);
+        qemu_log_vprintf(fmt, ap);
+        va_end(ap);
+    }
+}
+
+#if defined(TARGET_PPC64)
+static void GCC_FMT_ATTR(1, 2) LOG_SLB(const char *fmt, ...)
+{
+    if (debug_slb) {
+        va_list ap;
+        va_start(ap, fmt);
+        qemu_log_vprintf(fmt, ap);
+        va_end(ap);
+    }
+}
+#endif /* TARGET_PPC64 */
+#endif /* !CONFIG_USER_ONLY */
+
 /*****************************************************************************/
 /* PowerPC MMU emulation */
 #if defined(CONFIG_USER_ONLY)
@@ -534,8 +575,7 @@  static inline int get_bat(CPUPPCState *env, mmu_ctx_t *ctx,
         }
     }
     if (ret < 0) {
-#if defined(DEBUG_BATS)
-        if (qemu_log_enabled()) {
+        if (debug_bats && qemu_log_enabled()) {
             LOG_BATS("no BAT match for " TARGET_FMT_lx ":\n", virtual);
             for (i = 0; i < 4; i++) {
                 BATu = &BATut[i];
@@ -550,7 +590,6 @@  static inline int get_bat(CPUPPCState *env, mmu_ctx_t *ctx,
                          *BATu, *BATl, BEPIu, BEPIl, bl);
             }
         }
-#endif
     }
     /* No hit */
     return ret;
@@ -1858,7 +1897,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 2e74e45..b65c5b4 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -37,10 +37,21 @@ 
 //#define DO_PPC_STATISTICS
 
 #ifdef PPC_DEBUG_DISAS
-#  define LOG_DISAS(...) qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__)
+static const bool debug_disas = true;
 #else
-#  define LOG_DISAS(...) do { } while (0)
+static const bool debug_disas;
 #endif
+
+static void GCC_FMT_ATTR(1, 2) LOG_DISAS(const char *fmt, ...)
+{
+    if (debug_disas) {
+        va_list ap;
+        va_start(ap, fmt);
+        qemu_log_mask_vprintf(CPU_LOG_TB_IN_ASM, fmt, ap);
+        va_end(ap);
+    }
+}
+
 /*****************************************************************************/
 /* Code translation helpers                                                  */
 
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 74d93a4..a3e5de4 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -36,6 +36,18 @@ 
 #define TODO_USER_ONLY 1
 #endif
 
+#ifdef PPC_DEBUG_SPR
+static const bool debug_spr = true;
+#else
+static const bool debug_spr;
+#endif
+
+#ifdef PPC_DUMP_SPR_ACCESSES
+static const bool dump_spr_accesses = true;
+#else
+static const bool dump_spr_accesses;
+#endif
+
 /* For user-mode emulation, we don't emulate any IRQ controller */
 #if defined(CONFIG_USER_ONLY)
 #define PPC_IRQ_INIT_FN(name)                                                 \
@@ -58,11 +70,11 @@  PPC_IRQ_INIT_FN(e500);
  */
 static void spr_load_dump_spr(int sprn)
 {
-#ifdef PPC_DUMP_SPR_ACCESSES
-    TCGv_i32 t0 = tcg_const_i32(sprn);
-    gen_helper_load_dump_spr(cpu_env, t0);
-    tcg_temp_free_i32(t0);
-#endif
+    if (dump_spr_accesses) {
+        TCGv_i32 t0 = tcg_const_i32(sprn);
+        gen_helper_load_dump_spr(cpu_env, t0);
+        tcg_temp_free_i32(t0);
+    }
 }
 
 static void spr_read_generic (void *opaque, int gprn, int sprn)
@@ -73,11 +85,11 @@  static void spr_read_generic (void *opaque, int gprn, int sprn)
 
 static void spr_store_dump_spr(int sprn)
 {
-#ifdef PPC_DUMP_SPR_ACCESSES
-    TCGv_i32 t0 = tcg_const_i32(sprn);
-    gen_helper_store_dump_spr(cpu_env, t0);
-    tcg_temp_free_i32(t0);
-#endif
+    if (dump_spr_accesses) {
+        TCGv_i32 t0 = tcg_const_i32(sprn);
+        gen_helper_store_dump_spr(cpu_env, t0);
+        tcg_temp_free_i32(t0);
+    }
 }
 
 static void spr_write_generic (void *opaque, int sprn, int gprn)
@@ -610,10 +622,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 (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;