* [Qemu-devel] [PATCH] target/s390x/excp_helper: Remove DPRINTF() macro
@ 2018-10-05 15:00 Thomas Huth
2018-10-08 8:46 ` David Hildenbrand
2018-10-08 10:01 ` Cornelia Huck
0 siblings, 2 replies; 3+ messages in thread
From: Thomas Huth @ 2018-10-05 15:00 UTC (permalink / raw)
To: Cornelia Huck, David Hildenbrand; +Cc: qemu-s390x, qemu-devel
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 */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] target/s390x/excp_helper: Remove DPRINTF() macro
2018-10-05 15:00 [Qemu-devel] [PATCH] target/s390x/excp_helper: Remove DPRINTF() macro Thomas Huth
@ 2018-10-08 8:46 ` David Hildenbrand
2018-10-08 10:01 ` Cornelia Huck
1 sibling, 0 replies; 3+ messages in thread
From: David Hildenbrand @ 2018-10-08 8:46 UTC (permalink / raw)
To: Thomas Huth, Cornelia Huck; +Cc: qemu-s390x, qemu-devel
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>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] target/s390x/excp_helper: Remove DPRINTF() macro
2018-10-05 15:00 [Qemu-devel] [PATCH] target/s390x/excp_helper: Remove DPRINTF() macro Thomas Huth
2018-10-08 8:46 ` David Hildenbrand
@ 2018-10-08 10:01 ` Cornelia Huck
1 sibling, 0 replies; 3+ messages in thread
From: Cornelia Huck @ 2018-10-08 10:01 UTC (permalink / raw)
To: Thomas Huth; +Cc: David Hildenbrand, qemu-s390x, qemu-devel
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-10-08 10:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-05 15:00 [Qemu-devel] [PATCH] target/s390x/excp_helper: Remove DPRINTF() macro Thomas Huth
2018-10-08 8:46 ` David Hildenbrand
2018-10-08 10:01 ` Cornelia Huck
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.