Message ID | 1538751601-7433-1-git-send-email-thuth@redhat.com |
---|---|
State | New |
Headers | show |
Series | target/s390x/excp_helper: Remove DPRINTF() macro | expand |
On 05/10/2018 17:00, Thomas Huth wrote: > Debug macros that are disabled by default should be avoided (since the > code bit-rots quite easily). Thus turn these debug prints into proper > qemu_log_mask(CPU_LOG_xxx, ...) statements instead. The DPRINTF statements > in do_[ext|io|mchk]_interrupt can even be removed completely since we can > log the information in a central place, s390_cpu_do_interrupt, instead. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > target/s390x/excp_helper.c | 48 +++++++++++----------------------------------- > 1 file changed, 11 insertions(+), 37 deletions(-) > > diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c > index f0ce60c..b803e04 100644 > --- a/target/s390x/excp_helper.c > +++ b/target/s390x/excp_helper.c > @@ -31,23 +31,6 @@ > #include "hw/s390x/s390_flic.h" > #endif > > -/* #define DEBUG_S390 */ > -/* #define DEBUG_S390_STDOUT */ > - > -#ifdef DEBUG_S390 > -#ifdef DEBUG_S390_STDOUT > -#define DPRINTF(fmt, ...) \ > - do { fprintf(stderr, fmt, ## __VA_ARGS__); \ > - if (qemu_log_separate()) { qemu_log(fmt, ##__VA_ARGS__); } } while (0) > -#else > -#define DPRINTF(fmt, ...) \ > - do { qemu_log(fmt, ## __VA_ARGS__); } while (0) > -#endif > -#else > -#define DPRINTF(fmt, ...) \ > - do { } while (0) > -#endif > - > #if defined(CONFIG_USER_ONLY) > > void s390_cpu_do_interrupt(CPUState *cs) > @@ -92,8 +75,8 @@ int s390_cpu_handle_mmu_fault(CPUState *cs, vaddr orig_vaddr, int size, > uint64_t asc; > int prot; > > - DPRINTF("%s: address 0x%" VADDR_PRIx " rw %d mmu_idx %d\n", > - __func__, orig_vaddr, rw, mmu_idx); > + qemu_log_mask(CPU_LOG_MMU, "%s: addr 0x%" VADDR_PRIx " rw %d mmu_idx %d\n", > + __func__, orig_vaddr, rw, mmu_idx); > > vaddr = orig_vaddr; > > @@ -122,8 +105,9 @@ int s390_cpu_handle_mmu_fault(CPUState *cs, vaddr orig_vaddr, int size, > if (!address_space_access_valid(&address_space_memory, raddr, > TARGET_PAGE_SIZE, rw, > MEMTXATTRS_UNSPECIFIED)) { > - DPRINTF("%s: raddr %" PRIx64 " > ram_size %" PRIx64 "\n", __func__, > - (uint64_t)raddr, (uint64_t)ram_size); > + qemu_log_mask(CPU_LOG_MMU, > + "%s: raddr %" PRIx64 " > ram_size %" PRIx64 "\n", > + __func__, (uint64_t)raddr, (uint64_t)ram_size); > trigger_pgm_exception(env, PGM_ADDRESSING, ILEN_AUTO); > return 1; > } > @@ -181,8 +165,10 @@ static void do_program_interrupt(CPUS390XState *env) > break; > } > > - qemu_log_mask(CPU_LOG_INT, "%s: code=0x%x ilen=%d\n", > - __func__, env->int_pgm_code, ilen); > + qemu_log_mask(CPU_LOG_INT, > + "%s: code=0x%x ilen=%d psw: %" PRIx64 " %" PRIx64 "\n", > + __func__, env->int_pgm_code, ilen, env->psw.mask, > + env->psw.addr); > > lowcore = cpu_map_lowcore(env); > > @@ -204,10 +190,6 @@ static void do_program_interrupt(CPUS390XState *env) > > cpu_unmap_lowcore(lowcore); > > - DPRINTF("%s: %x %x %" PRIx64 " %" PRIx64 "\n", __func__, > - env->int_pgm_code, ilen, env->psw.mask, > - env->psw.addr); > - > load_psw(env, mask, addr); > } > > @@ -298,9 +280,6 @@ static void do_ext_interrupt(CPUS390XState *env) > > cpu_unmap_lowcore(lowcore); > > - DPRINTF("%s: %" PRIx64 " %" PRIx64 "\n", __func__, > - env->psw.mask, env->psw.addr); > - > load_psw(env, mask, addr); > } > > @@ -329,8 +308,6 @@ static void do_io_interrupt(CPUS390XState *env) > cpu_unmap_lowcore(lowcore); > g_free(io); > > - DPRINTF("%s: %" PRIx64 " %" PRIx64 "\n", __func__, env->psw.mask, > - env->psw.addr); > load_psw(env, mask, addr); > } > > @@ -372,9 +349,6 @@ static void do_mchk_interrupt(CPUS390XState *env) > > cpu_unmap_lowcore(lowcore); > > - DPRINTF("%s: %" PRIx64 " %" PRIx64 "\n", __func__, > - env->psw.mask, env->psw.addr); > - > load_psw(env, mask, addr); > } > > @@ -385,8 +359,8 @@ void s390_cpu_do_interrupt(CPUState *cs) > CPUS390XState *env = &cpu->env; > bool stopped = false; > > - qemu_log_mask(CPU_LOG_INT, "%s: %d at pc=%" PRIx64 "\n", > - __func__, cs->exception_index, env->psw.addr); > + qemu_log_mask(CPU_LOG_INT, "%s: %d at psw=%" PRIx64 ":%" PRIx64 "\n", > + __func__, cs->exception_index, env->psw.mask, env->psw.addr); > > try_deliver: > /* handle machine checks */ > Looks good to me! Reviewed-by: David Hildenbrand <david@redhat.com>
On Fri, 5 Oct 2018 17:00:01 +0200 Thomas Huth <thuth@redhat.com> wrote: > Debug macros that are disabled by default should be avoided (since the > code bit-rots quite easily). Thus turn these debug prints into proper > qemu_log_mask(CPU_LOG_xxx, ...) statements instead. The DPRINTF statements > in do_[ext|io|mchk]_interrupt can even be removed completely since we can > log the information in a central place, s390_cpu_do_interrupt, instead. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > target/s390x/excp_helper.c | 48 +++++++++++----------------------------------- > 1 file changed, 11 insertions(+), 37 deletions(-) [patch had gained a trivial-to-fix contextual conflict in the meanwhile; fixed] Thanks, applied.
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c index f0ce60c..b803e04 100644 --- a/target/s390x/excp_helper.c +++ b/target/s390x/excp_helper.c @@ -31,23 +31,6 @@ #include "hw/s390x/s390_flic.h" #endif -/* #define DEBUG_S390 */ -/* #define DEBUG_S390_STDOUT */ - -#ifdef DEBUG_S390 -#ifdef DEBUG_S390_STDOUT -#define DPRINTF(fmt, ...) \ - do { fprintf(stderr, fmt, ## __VA_ARGS__); \ - if (qemu_log_separate()) { qemu_log(fmt, ##__VA_ARGS__); } } while (0) -#else -#define DPRINTF(fmt, ...) \ - do { qemu_log(fmt, ## __VA_ARGS__); } while (0) -#endif -#else -#define DPRINTF(fmt, ...) \ - do { } while (0) -#endif - #if defined(CONFIG_USER_ONLY) void s390_cpu_do_interrupt(CPUState *cs) @@ -92,8 +75,8 @@ int s390_cpu_handle_mmu_fault(CPUState *cs, vaddr orig_vaddr, int size, uint64_t asc; int prot; - DPRINTF("%s: address 0x%" VADDR_PRIx " rw %d mmu_idx %d\n", - __func__, orig_vaddr, rw, mmu_idx); + qemu_log_mask(CPU_LOG_MMU, "%s: addr 0x%" VADDR_PRIx " rw %d mmu_idx %d\n", + __func__, orig_vaddr, rw, mmu_idx); vaddr = orig_vaddr; @@ -122,8 +105,9 @@ int s390_cpu_handle_mmu_fault(CPUState *cs, vaddr orig_vaddr, int size, if (!address_space_access_valid(&address_space_memory, raddr, TARGET_PAGE_SIZE, rw, MEMTXATTRS_UNSPECIFIED)) { - DPRINTF("%s: raddr %" PRIx64 " > ram_size %" PRIx64 "\n", __func__, - (uint64_t)raddr, (uint64_t)ram_size); + qemu_log_mask(CPU_LOG_MMU, + "%s: raddr %" PRIx64 " > ram_size %" PRIx64 "\n", + __func__, (uint64_t)raddr, (uint64_t)ram_size); trigger_pgm_exception(env, PGM_ADDRESSING, ILEN_AUTO); return 1; } @@ -181,8 +165,10 @@ static void do_program_interrupt(CPUS390XState *env) break; } - qemu_log_mask(CPU_LOG_INT, "%s: code=0x%x ilen=%d\n", - __func__, env->int_pgm_code, ilen); + qemu_log_mask(CPU_LOG_INT, + "%s: code=0x%x ilen=%d psw: %" PRIx64 " %" PRIx64 "\n", + __func__, env->int_pgm_code, ilen, env->psw.mask, + env->psw.addr); lowcore = cpu_map_lowcore(env); @@ -204,10 +190,6 @@ static void do_program_interrupt(CPUS390XState *env) cpu_unmap_lowcore(lowcore); - DPRINTF("%s: %x %x %" PRIx64 " %" PRIx64 "\n", __func__, - env->int_pgm_code, ilen, env->psw.mask, - env->psw.addr); - load_psw(env, mask, addr); } @@ -298,9 +280,6 @@ static void do_ext_interrupt(CPUS390XState *env) cpu_unmap_lowcore(lowcore); - DPRINTF("%s: %" PRIx64 " %" PRIx64 "\n", __func__, - env->psw.mask, env->psw.addr); - load_psw(env, mask, addr); } @@ -329,8 +308,6 @@ static void do_io_interrupt(CPUS390XState *env) cpu_unmap_lowcore(lowcore); g_free(io); - DPRINTF("%s: %" PRIx64 " %" PRIx64 "\n", __func__, env->psw.mask, - env->psw.addr); load_psw(env, mask, addr); } @@ -372,9 +349,6 @@ static void do_mchk_interrupt(CPUS390XState *env) cpu_unmap_lowcore(lowcore); - DPRINTF("%s: %" PRIx64 " %" PRIx64 "\n", __func__, - env->psw.mask, env->psw.addr); - load_psw(env, mask, addr); } @@ -385,8 +359,8 @@ void s390_cpu_do_interrupt(CPUState *cs) CPUS390XState *env = &cpu->env; bool stopped = false; - qemu_log_mask(CPU_LOG_INT, "%s: %d at pc=%" PRIx64 "\n", - __func__, cs->exception_index, env->psw.addr); + qemu_log_mask(CPU_LOG_INT, "%s: %d at psw=%" PRIx64 ":%" PRIx64 "\n", + __func__, cs->exception_index, env->psw.mask, env->psw.addr); try_deliver: /* handle machine checks */
Debug macros that are disabled by default should be avoided (since the code bit-rots quite easily). Thus turn these debug prints into proper qemu_log_mask(CPU_LOG_xxx, ...) statements instead. The DPRINTF statements in do_[ext|io|mchk]_interrupt can even be removed completely since we can log the information in a central place, s390_cpu_do_interrupt, instead. Signed-off-by: Thomas Huth <thuth@redhat.com> --- target/s390x/excp_helper.c | 48 +++++++++++----------------------------------- 1 file changed, 11 insertions(+), 37 deletions(-)