target/s390x/excp_helper: Remove DPRINTF() macro
PATCH 1/1 target/s390x/excp_helper: Remove DPRINTF() macro
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.
--- target/s390x/excp_helper.c | 48 +++++++++++----------------------------------- 1 file changed, 11 insertions(+), 37 deletions(-)
target/s390x/excp_helper.c +11 -37
@@ -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 */
Signature · 1 line
1.8.3.1
Quoted 117 lines · Thomas Huth
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 <[email protected]>--- target/s390x/excp_helper.c | 48 +++++++++++----------------------------------- 1 file changed, 11 insertions(+), 37 deletions(-)target/s390x/excp_helper.c +9
-35@@ -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); }
target/s390x/excp_helper.c void s390_cpu_do_interrupt(CPUState *cs) · line 366
Quoted 10 lines
target/s390x/excp_helper.c +2
-2@@ -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!
Signature · 3 lines
Thanks, David / dhildenb
Cornelia Huck
AppliedQuoted 10 lines · On Fri, 5 Oct 2018 17:00:01 +0200 Thomas Huth <[email protected]> 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 <[email protected]>--- 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.