[PATCH v2 08/15] target-i386: Refactor debug output macros

[PATCH v2 08/15] target-i386: Refactor debug output macros

[Andreas Färber]

Make debug output compile-testable even if disabled.

Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Richard Henderson <rth@twiddle.net>
---
 target-i386/helper.c     |   42 ++++++++++++++++++++++++++----------------
 target-i386/kvm.c        |   16 ++++++++++++----
 target-i386/seg_helper.c |   24 +++++++++++++++++++-----
 3 Dateien geändert, 57 Zeilen hinzugefügt(+), 25 Zeilen entfernt(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index 4bf9db7..696e4e1 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -26,6 +26,22 @@
 
 //#define DEBUG_MMU
 
+#ifdef DEBUG_MMU
+static const bool debug_mmu = true;
+#else
+static const bool debug_mmu;
+#endif
+
+static void GCC_FMT_ATTR(1, 2) mmu_dprintf(const char *fmt, ...)
+{
+    if (debug_mmu) {
+        va_list ap;
+        va_start(ap, fmt);
+        vprintf(fmt, ap);
+        va_end(ap);
+    }
+}
+
 static void cpu_x86_version(CPUX86State *env, int *family, int *model)
 {
     int cpuver = env->cpuid_version;
@@ -372,9 +388,8 @@ void x86_cpu_set_a20(X86CPU *cpu, int a20_state)
 
     a20_state = (a20_state != 0);
     if (a20_state != ((env->a20_mask >> 20) & 1)) {
-#if defined(DEBUG_MMU)
-        printf("A20 update: a20=%d\n", a20_state);
-#endif
+        mmu_dprintf("A20 update: a20=%d\n", a20_state);
+
         /* if the cpu is currently executing code, we must unlink it and
            all the potentially executing TB */
         cpu_interrupt(env, CPU_INTERRUPT_EXITTB);
@@ -390,9 +405,8 @@ void cpu_x86_update_cr0(CPUX86State *env, uint32_t new_cr0)
 {
     int pe_state;
 
-#if defined(DEBUG_MMU)
-    printf("CR0 update: CR0=0x%08x\n", new_cr0);
-#endif
+    mmu_dprintf("CR0 update: CR0=0x%08x\n", new_cr0);
+
     if ((new_cr0 & (CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK)) !=
         (env->cr[0] & (CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK))) {
         tlb_flush(env, 1);
@@ -433,18 +447,15 @@ void cpu_x86_update_cr3(CPUX86State *env, target_ulong new_cr3)
 {
     env->cr[3] = new_cr3;
     if (env->cr[0] & CR0_PG_MASK) {
-#if defined(DEBUG_MMU)
-        printf("CR3 update: CR3=" TARGET_FMT_lx "\n", new_cr3);
-#endif
+        mmu_dprintf("CR3 update: CR3=" TARGET_FMT_lx "\n", new_cr3);
         tlb_flush(env, 0);
     }
 }
 
 void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4)
 {
-#if defined(DEBUG_MMU)
-    printf("CR4 update: CR4=%08x\n", (uint32_t)env->cr[4]);
-#endif
+    mmu_dprintf("CR4 update: CR4=%08x\n", (uint32_t)env->cr[4]);
+
     if ((new_cr4 ^ env->cr[4]) &
         (CR4_PGE_MASK | CR4_PAE_MASK | CR4_PSE_MASK |
          CR4_SMEP_MASK | CR4_SMAP_MASK)) {
@@ -510,10 +521,9 @@ int cpu_x86_handle_mmu_fault(CPUX86State *env, target_ulong addr,
     target_ulong vaddr, virt_addr;
 
     is_user = mmu_idx == MMU_USER_IDX;
-#if defined(DEBUG_MMU)
-    printf("MMU fault: addr=" TARGET_FMT_lx " w=%d u=%d eip=" TARGET_FMT_lx "\n",
-           addr, is_write1, is_user, env->eip);
-#endif
+    mmu_dprintf("MMU fault: addr=" TARGET_FMT_lx " w=%d u=%d"
+                " eip=" TARGET_FMT_lx "\n",
+                addr, is_write1, is_user, env->eip);
     is_write = is_write1 & 1;
 
     if (!(env->cr[0] & CR0_PG_MASK)) {
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 0cf413d..9cbef77 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -37,13 +37,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) DPRINTF(const char *fmt, ...)
+{
+    if (debug_kvm) {
+        va_list ap;
+        va_start(ap, fmt);
+        vfprintf(stderr, fmt, ap);
+        va_end(ap);
+    }
+}
+
 #define MSR_KVM_WALL_CLOCK  0x11
 #define MSR_KVM_SYSTEM_TIME 0x12
 
diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index 3247dee..2338d54 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -29,14 +29,28 @@
 #endif /* !defined(CONFIG_USER_ONLY) */
 
 #ifdef DEBUG_PCALL
-# define LOG_PCALL(...) qemu_log_mask(CPU_LOG_PCALL, ## __VA_ARGS__)
-# define LOG_PCALL_STATE(env)                                  \
-    log_cpu_state_mask(CPU_LOG_PCALL, (env), CPU_DUMP_CCOP)
+static const bool debug_pcall = true;
 #else
-# define LOG_PCALL(...) do { } while (0)
-# define LOG_PCALL_STATE(env) do { } while (0)
+static const bool debug_pcall;
 #endif
 
+static void GCC_FMT_ATTR(1, 2) LOG_PCALL(const char *fmt, ...)
+{
+    if (debug_pcall) {
+        va_list ap;
+        va_start(ap, fmt);
+        qemu_log_mask(CPU_LOG_PCALL, fmt, ap);
+        va_end(ap);
+    }
+}
+
+static inline void LOG_PCALL_STATE(CPUX86State *env)
+{
+    if (debug_pcall) {
+        log_cpu_state_mask(CPU_LOG_PCALL, (env), CPU_DUMP_CCOP);
+    }
+}
+
 /* return non zero if error */
 static inline int load_segment(CPUX86State *env, uint32_t *e1_ptr,
                                uint32_t *e2_ptr, int selector)
-- 
1.7.10.4

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

[Andreas Färber]

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(-)

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;
-- 
1.7.10.4

[PATCH v2 13/15] target-s390x: Refactor debug output macros

[Andreas Färber]

Make debug output compile-testable even if disabled.

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

Drop unused DEBUG_HELPER and LOG_HELPER() in fpu_helper.c.
Drop unused LOG_DISAS() in translate.c and inline S390X_DEBUG_DISAS.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-s390x/cc_helper.c   |   14 ++++++++++--
 target-s390x/fpu_helper.c  |    7 ------
 target-s390x/helper.c      |   53 ++++++++++++++++++++++++++++++++++----------
 target-s390x/int_helper.c  |   14 ++++++++++--
 target-s390x/kvm.c         |   33 +++++++++++++++++----------
 target-s390x/mem_helper.c  |   14 ++++++++++--
 target-s390x/misc_helper.c |   14 ++++++++++--
 target-s390x/translate.c   |   23 +++++++++----------
 8 Dateien geändert, 120 Zeilen hinzugefügt(+), 52 Zeilen entfernt(-)

diff --git a/target-s390x/cc_helper.c b/target-s390x/cc_helper.c
index a6d60bf..e45a19a 100644
--- a/target-s390x/cc_helper.c
+++ b/target-s390x/cc_helper.c
@@ -24,11 +24,21 @@
 
 /* #define DEBUG_HELPER */
 #ifdef DEBUG_HELPER
-#define HELPER_LOG(x...) qemu_log(x)
+static const bool debug_helper = true;
 #else
-#define HELPER_LOG(x...)
+static const bool debug_helper;
 #endif
 
+static void GCC_FMT_ATTR(1, 2) HELPER_LOG(const char *fmt, ...)
+{
+    if (debug_helper) {
+        va_list ap;
+        va_start(ap, fmt);
+        qemu_log_vprintf(fmt, ap);
+        va_end(ap);
+    }
+}
+
 static uint32_t cc_calc_ltgt_32(int32_t src, int32_t dst)
 {
     if (src == dst) {
diff --git a/target-s390x/fpu_helper.c b/target-s390x/fpu_helper.c
index 94375b6..937da59 100644
--- a/target-s390x/fpu_helper.c
+++ b/target-s390x/fpu_helper.c
@@ -25,13 +25,6 @@
 #include "exec/softmmu_exec.h"
 #endif
 
-/* #define DEBUG_HELPER */
-#ifdef DEBUG_HELPER
-#define HELPER_LOG(x...) qemu_log(x)
-#else
-#define HELPER_LOG(x...)
-#endif
-
 #define RET128(F) (env->retxl = F.low, F.high)
 
 #define convert_bit(mask, from, to) \
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index 1183b45..4a02251 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -30,24 +30,53 @@
 //#define DEBUG_S390_STDOUT
 
 #ifdef DEBUG_S390
-#ifdef DEBUG_S390_STDOUT
-#define DPRINTF(fmt, ...) \
-    do { fprintf(stderr, fmt, ## __VA_ARGS__); \
-         qemu_log(fmt, ##__VA_ARGS__); } while (0)
+static const bool debug_helper = true;
 #else
-#define DPRINTF(fmt, ...) \
-    do { qemu_log(fmt, ## __VA_ARGS__); } while (0)
+static const bool debug_helper;
 #endif
+
+#ifdef DEBUG_S390_PTE
+static const bool debug_pte = true;
 #else
-#define DPRINTF(fmt, ...) \
-    do { } while (0)
+static const bool debug_pte;
 #endif
 
-#ifdef DEBUG_S390_PTE
-#define PTE_DPRINTF DPRINTF
+#ifdef DEBUG_S390_STDOUT
+static const bool debug_to_stdout = true;
 #else
-#define PTE_DPRINTF(fmt, ...) \
-    do { } while (0)
+static const bool debug_to_stdout;
+#endif
+
+#ifndef CONFIG_USER_ONLY
+static inline void GCC_FMT_ATTR(1, 0) vDPRINTF(const char *fmt, va_list ap)
+{
+    if (debug_helper) {
+        if (debug_to_stdout) {
+            vfprintf(stderr, fmt, ap);
+        }
+        qemu_log_vprintf(fmt, ap);
+    }
+}
+
+static void GCC_FMT_ATTR(1, 2) DPRINTF(const char *fmt, ...)
+{
+    if (debug_helper) {
+        va_list ap;
+        va_start(ap, fmt);
+        vDPRINTF(fmt, ap);
+        va_end(ap);
+    }
+}
+
+static void GCC_FMT_ATTR(1, 2) PTE_DPRINTF(const char *fmt, ...)
+{
+    if (debug_pte) {
+        va_list ap;
+        va_start(ap, fmt);
+        vDPRINTF(fmt, ap);
+        va_end(ap);
+    }
+}
 #endif
 
 #ifndef CONFIG_USER_ONLY
diff --git a/target-s390x/int_helper.c b/target-s390x/int_helper.c
index 6858301..c7da7e6 100644
--- a/target-s390x/int_helper.c
+++ b/target-s390x/int_helper.c
@@ -24,11 +24,21 @@
 
 /* #define DEBUG_HELPER */
 #ifdef DEBUG_HELPER
-#define HELPER_LOG(x...) qemu_log(x)
+static const bool debug_helper = true;
 #else
-#define HELPER_LOG(x...)
+static const bool debug_helper;
 #endif
 
+static void GCC_FMT_ATTR(1, 2) HELPER_LOG(const char *fmt, ...)
+{
+    if (debug_helper) {
+        va_list ap;
+        va_start(ap, fmt);
+        qemu_log_vprintf(fmt, ap);
+        va_end(ap);
+    }
+}
+
 /* 64/64 -> 128 unsigned multiplication */
 uint64_t HELPER(mul128)(CPUS390XState *env, uint64_t v1, uint64_t v2)
 {
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 3929771..8eea7f1 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -38,13 +38,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 IPA0_DIAG                       0x8300
 #define IPA0_SIGP                       0xae00
 #define IPA0_B2                         0xb200
@@ -518,7 +526,7 @@ static int handle_priv(S390CPU *cpu, struct kvm_run *run,
     uint16_t ipbh0 = (run->s390_sieic.ipb & 0xffff0000) >> 16;
     uint8_t ipb = run->s390_sieic.ipb & 0xff;
 
-    dprintf("KVM: PRIV: %d\n", ipa1);
+    kvm_dprintf("KVM: PRIV: %d\n", ipa1);
     switch (ipa1) {
         case PRIV_SCLP_CALL:
             r = kvm_sclp_service_call(cpu, run, ipbh0);
@@ -531,7 +539,7 @@ static int handle_priv(S390CPU *cpu, struct kvm_run *run,
                     r = 0;
                 }
             } else {
-                dprintf("KVM: unknown PRIV: 0x%x\n", ipa1);
+                kvm_dprintf("KVM: unknown PRIV: 0x%x\n", ipa1);
                 r = -1;
             }
             break;
@@ -560,7 +568,7 @@ static int handle_diag(CPUS390XState *env, struct kvm_run *run, int ipb_code)
             sleep(10);
             break;
         default:
-            dprintf("KVM: unknown DIAG: 0x%x\n", ipb_code);
+            kvm_dprintf("KVM: unknown DIAG: 0x%x\n", ipb_code);
             r = -1;
             break;
     }
@@ -573,7 +581,7 @@ static int s390_cpu_restart(S390CPU *cpu)
     kvm_s390_interrupt(cpu, KVM_S390_RESTART, 0);
     s390_add_running_cpu(cpu);
     qemu_cpu_kick(CPU(cpu));
-    dprintf("DONE: SIGP cpu restart: %p\n", &cpu->env);
+    kvm_dprintf("DONE: SIGP cpu restart: %p\n", &cpu->env);
     return 0;
 }
 
@@ -600,7 +608,7 @@ static int s390_cpu_initial_reset(S390CPU *cpu)
         env->regs[i] = 0;
     }
 
-    dprintf("DONE: SIGP initial reset: %p\n", env);
+    kvm_dprintf("DONE: SIGP initial reset: %p\n", env);
     return 0;
 }
 
@@ -670,7 +678,8 @@ static int handle_instruction(S390CPU *cpu, struct kvm_run *run)
     int ipb_code = (run->s390_sieic.ipb & 0x0fff0000) >> 16;
     int r = -1;
 
-    dprintf("handle_instruction 0x%x 0x%x\n", run->s390_sieic.ipa, run->s390_sieic.ipb);
+    kvm_dprintf("handle_instruction 0x%x 0x%x\n",
+                run->s390_sieic.ipa, run->s390_sieic.ipb);
     switch (ipa0) {
     case IPA0_B2:
     case IPA0_B9:
@@ -704,8 +713,8 @@ static int handle_intercept(S390CPU *cpu)
     int icpt_code = run->s390_sieic.icptcode;
     int r = 0;
 
-    dprintf("intercept: 0x%x (at 0x%lx)\n", icpt_code,
-            (long)cs->kvm_run->psw_addr);
+    kvm_dprintf("intercept: 0x%x (at 0x%lx)\n", icpt_code,
+                (long)cs->kvm_run->psw_addr);
     switch (icpt_code) {
         case ICPT_INSTRUCTION:
             r = handle_instruction(cpu, run);
diff --git a/target-s390x/mem_helper.c b/target-s390x/mem_helper.c
index 372334b..4108124 100644
--- a/target-s390x/mem_helper.c
+++ b/target-s390x/mem_helper.c
@@ -63,11 +63,21 @@ void tlb_fill(CPUS390XState *env, target_ulong addr, int is_write, int mmu_idx,
 
 /* #define DEBUG_HELPER */
 #ifdef DEBUG_HELPER
-#define HELPER_LOG(x...) qemu_log(x)
+static const bool debug_helper = true;
 #else
-#define HELPER_LOG(x...)
+static const bool debug_helper;
 #endif
 
+static void GCC_FMT_ATTR(1, 2) HELPER_LOG(const char *fmt, ...)
+{
+    if (debug_helper) {
+        va_list ap;
+        va_start(ap, fmt);
+        qemu_log_vprintf(fmt, ap);
+        va_end(ap);
+    }
+}
+
 #ifndef CONFIG_USER_ONLY
 static void mvc_fast_memset(CPUS390XState *env, uint32_t l, uint64_t dest,
                             uint8_t byte)
diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c
index 09301d0..1cb7514 100644
--- a/target-s390x/misc_helper.c
+++ b/target-s390x/misc_helper.c
@@ -36,11 +36,21 @@
 
 /* #define DEBUG_HELPER */
 #ifdef DEBUG_HELPER
-#define HELPER_LOG(x...) qemu_log(x)
+static const bool debug_helper = true;
 #else
-#define HELPER_LOG(x...)
+static const bool debug_helper;
 #endif
 
+static void GCC_FMT_ATTR(1, 2) HELPER_LOG(const char *fmt, ...)
+{
+    if (debug_helper) {
+        va_list ap;
+        va_start(ap, fmt);
+        qemu_log_vprintf(fmt, ap);
+        va_end(ap);
+    }
+}
+
 /* Raise an exception dynamically from a helper function.  */
 void QEMU_NORETURN runtime_exception(CPUS390XState *env, int excp,
                                      uintptr_t retaddr)
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index a57296c..6e3cd2a 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -18,22 +18,21 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 
-/* #define DEBUG_INLINE_BRANCHES */
-#define S390X_DEBUG_DISAS
-/* #define S390X_DEBUG_DISAS_VERBOSE */
-
-#ifdef S390X_DEBUG_DISAS_VERBOSE
-#  define LOG_DISAS(...) qemu_log(__VA_ARGS__)
-#else
-#  define LOG_DISAS(...) do { } while (0)
-#endif
-
 #include "cpu.h"
 #include "disas/disas.h"
 #include "tcg-op.h"
 #include "qemu/log.h"
 #include "qemu/host-utils.h"
 
+/* #define DEBUG_INLINE_BRANCHES */
+#define S390X_DEBUG_DISAS
+
+#ifdef S390X_DEBUG_DISAS
+static const bool debug_disas = true;
+#else
+static const bool debug_disas;
+#endif
+
 /* global register indexes */
 static TCGv_ptr cpu_env;
 
@@ -4859,13 +4858,11 @@ static inline void gen_intermediate_code_internal(CPUS390XState *env,
         tb->icount = num_insns;
     }
 
-#if defined(S390X_DEBUG_DISAS)
-    if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
+    if (debug_disas && qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
         log_target_disas(env, pc_start, dc.pc - pc_start, 1);
         qemu_log("\n");
     }
-#endif
 }
 
 void gen_intermediate_code (CPUS390XState *env, struct TranslationBlock *tb)
-- 
1.7.10.4

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

[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? :)

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


Alex

Re: [PATCH v2 13/15] target-s390x: Refactor debug output macros

[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() due to a conflict with glibc.
> 
> Drop unused DEBUG_HELPER and LOG_HELPER() in fpu_helper.c.
> Drop unused LOG_DISAS() in translate.c and inline S390X_DEBUG_DISAS.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>

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


Alex

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

[Andreas Färber]

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
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

[Alexander Graf]

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