| Message ID | 1527531079-32717-1-git-send-email-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/ppc: Use proper logging function for possible guest errors | expand |
On 05/28/2018 03:11 PM, Thomas Huth wrote: > fprintf() and qemu_log_separate() are frowned upon these days for printing > logging information in QEMU. Accessing the wrong SPRs indicates wrong guest > behaviour in most cases, and we've got a proper way to log such situations, > which is the qemu_log_mask(LOG_GUEST_ERROR, ...) function. So use this > function now for logging the bad SPR accesses instead. > > Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > target/ppc/translate.c | 37 ++++++++++++------------------------- > 1 file changed, 12 insertions(+), 25 deletions(-) > > diff --git a/target/ppc/translate.c b/target/ppc/translate.c > index e30d99f..0806ee0 100644 > --- a/target/ppc/translate.c > +++ b/target/ppc/translate.c > @@ -3933,13 +3933,9 @@ static inline void gen_op_mfspr(DisasContext *ctx) > * allowing userland application to read the PVR > */ > if (sprn != SPR_PVR) { > - fprintf(stderr, "Trying to read privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - if (qemu_log_separate()) { > - qemu_log("Trying to read privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, > - ctx->base.pc_next - 4); > - } > + qemu_log_mask(LOG_GUEST_ERROR, "Trying to read privileged spr " > + "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn, > + ctx->base.pc_next - 4); > } > gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); > } > @@ -3951,12 +3947,9 @@ static inline void gen_op_mfspr(DisasContext *ctx) > return; > } > /* Not defined */ > - fprintf(stderr, "Trying to read invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - if (qemu_log_separate()) { > - qemu_log("Trying to read invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - } > + qemu_log_mask(LOG_GUEST_ERROR, > + "Trying to read invalid spr %d (0x%03x) at " > + TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > > /* The behaviour depends on MSR:PR and SPR# bit 0x10, > * it can generate a priv, a hv emu or a no-op > @@ -4097,12 +4090,9 @@ static void gen_mtspr(DisasContext *ctx) > (*write_cb)(ctx, sprn, rS(ctx->opcode)); > } else { > /* Privilege exception */ > - fprintf(stderr, "Trying to write privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - if (qemu_log_separate()) { > - qemu_log("Trying to write privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - } > + qemu_log_mask(LOG_GUEST_ERROR, "Trying to write privileged spr " > + "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn, > + ctx->base.pc_next - 4); > gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); > } > } else { > @@ -4114,12 +4104,9 @@ static void gen_mtspr(DisasContext *ctx) > } > > /* Not defined */ > - if (qemu_log_separate()) { > - qemu_log("Trying to write invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - } > - fprintf(stderr, "Trying to write invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > + qemu_log_mask(LOG_GUEST_ERROR, > + "Trying to write invalid spr %d (0x%03x) at " > + TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > > > /* The behaviour depends on MSR:PR and SPR# bit 0x10, >
On Mon, 28 May 2018 20:11:19 +0200 Thomas Huth <thuth@redhat.com> wrote: > fprintf() and qemu_log_separate() are frowned upon these days for printing > logging information in QEMU. Accessing the wrong SPRs indicates wrong guest > behaviour in most cases, and we've got a proper way to log such situations, > which is the qemu_log_mask(LOG_GUEST_ERROR, ...) function. So use this > function now for logging the bad SPR accesses instead. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- Reviewed-by: Greg Kurz <groug@kaod.org> > target/ppc/translate.c | 37 ++++++++++++------------------------- > 1 file changed, 12 insertions(+), 25 deletions(-) > > diff --git a/target/ppc/translate.c b/target/ppc/translate.c > index e30d99f..0806ee0 100644 > --- a/target/ppc/translate.c > +++ b/target/ppc/translate.c > @@ -3933,13 +3933,9 @@ static inline void gen_op_mfspr(DisasContext *ctx) > * allowing userland application to read the PVR > */ > if (sprn != SPR_PVR) { > - fprintf(stderr, "Trying to read privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - if (qemu_log_separate()) { > - qemu_log("Trying to read privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, > - ctx->base.pc_next - 4); > - } > + qemu_log_mask(LOG_GUEST_ERROR, "Trying to read privileged spr " > + "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn, > + ctx->base.pc_next - 4); > } > gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); > } > @@ -3951,12 +3947,9 @@ static inline void gen_op_mfspr(DisasContext *ctx) > return; > } > /* Not defined */ > - fprintf(stderr, "Trying to read invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - if (qemu_log_separate()) { > - qemu_log("Trying to read invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - } > + qemu_log_mask(LOG_GUEST_ERROR, > + "Trying to read invalid spr %d (0x%03x) at " > + TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > > /* The behaviour depends on MSR:PR and SPR# bit 0x10, > * it can generate a priv, a hv emu or a no-op > @@ -4097,12 +4090,9 @@ static void gen_mtspr(DisasContext *ctx) > (*write_cb)(ctx, sprn, rS(ctx->opcode)); > } else { > /* Privilege exception */ > - fprintf(stderr, "Trying to write privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - if (qemu_log_separate()) { > - qemu_log("Trying to write privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - } > + qemu_log_mask(LOG_GUEST_ERROR, "Trying to write privileged spr " > + "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn, > + ctx->base.pc_next - 4); > gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); > } > } else { > @@ -4114,12 +4104,9 @@ static void gen_mtspr(DisasContext *ctx) > } > > /* Not defined */ > - if (qemu_log_separate()) { > - qemu_log("Trying to write invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - } > - fprintf(stderr, "Trying to write invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > + qemu_log_mask(LOG_GUEST_ERROR, > + "Trying to write invalid spr %d (0x%03x) at " > + TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > > > /* The behaviour depends on MSR:PR and SPR# bit 0x10,
On Mon, May 28, 2018 at 11:11 AM, Thomas Huth <thuth@redhat.com> wrote: > fprintf() and qemu_log_separate() are frowned upon these days for printing > logging information in QEMU. Accessing the wrong SPRs indicates wrong guest > behaviour in most cases, and we've got a proper way to log such situations, > which is the qemu_log_mask(LOG_GUEST_ERROR, ...) function. So use this > function now for logging the bad SPR accesses instead. > > Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > target/ppc/translate.c | 37 ++++++++++++------------------------- > 1 file changed, 12 insertions(+), 25 deletions(-) > > diff --git a/target/ppc/translate.c b/target/ppc/translate.c > index e30d99f..0806ee0 100644 > --- a/target/ppc/translate.c > +++ b/target/ppc/translate.c > @@ -3933,13 +3933,9 @@ static inline void gen_op_mfspr(DisasContext *ctx) > * allowing userland application to read the PVR > */ > if (sprn != SPR_PVR) { > - fprintf(stderr, "Trying to read privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - if (qemu_log_separate()) { > - qemu_log("Trying to read privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, > - ctx->base.pc_next - 4); > - } > + qemu_log_mask(LOG_GUEST_ERROR, "Trying to read privileged spr " > + "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn, > + ctx->base.pc_next - 4); > } > gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); > } > @@ -3951,12 +3947,9 @@ static inline void gen_op_mfspr(DisasContext *ctx) > return; > } > /* Not defined */ > - fprintf(stderr, "Trying to read invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - if (qemu_log_separate()) { > - qemu_log("Trying to read invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - } > + qemu_log_mask(LOG_GUEST_ERROR, > + "Trying to read invalid spr %d (0x%03x) at " > + TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > > /* The behaviour depends on MSR:PR and SPR# bit 0x10, > * it can generate a priv, a hv emu or a no-op > @@ -4097,12 +4090,9 @@ static void gen_mtspr(DisasContext *ctx) > (*write_cb)(ctx, sprn, rS(ctx->opcode)); > } else { > /* Privilege exception */ > - fprintf(stderr, "Trying to write privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - if (qemu_log_separate()) { > - qemu_log("Trying to write privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - } > + qemu_log_mask(LOG_GUEST_ERROR, "Trying to write privileged spr " > + "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn, > + ctx->base.pc_next - 4); > gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); > } > } else { > @@ -4114,12 +4104,9 @@ static void gen_mtspr(DisasContext *ctx) > } > > /* Not defined */ > - if (qemu_log_separate()) { > - qemu_log("Trying to write invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - } > - fprintf(stderr, "Trying to write invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > + qemu_log_mask(LOG_GUEST_ERROR, > + "Trying to write invalid spr %d (0x%03x) at " > + TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > > > /* The behaviour depends on MSR:PR and SPR# bit 0x10, > -- > 1.8.3.1 > >
On Mon, May 28, 2018 at 08:11:19PM +0200, Thomas Huth wrote: > fprintf() and qemu_log_separate() are frowned upon these days for printing > logging information in QEMU. Accessing the wrong SPRs indicates wrong guest > behaviour in most cases, and we've got a proper way to log such situations, > which is the qemu_log_mask(LOG_GUEST_ERROR, ...) function. So use this > function now for logging the bad SPR accesses instead. > > Signed-off-by: Thomas Huth <thuth@redhat.com> Applied to ppc-for-2.13, thanks. > --- > target/ppc/translate.c | 37 ++++++++++++------------------------- > 1 file changed, 12 insertions(+), 25 deletions(-) > > diff --git a/target/ppc/translate.c b/target/ppc/translate.c > index e30d99f..0806ee0 100644 > --- a/target/ppc/translate.c > +++ b/target/ppc/translate.c > @@ -3933,13 +3933,9 @@ static inline void gen_op_mfspr(DisasContext *ctx) > * allowing userland application to read the PVR > */ > if (sprn != SPR_PVR) { > - fprintf(stderr, "Trying to read privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - if (qemu_log_separate()) { > - qemu_log("Trying to read privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, > - ctx->base.pc_next - 4); > - } > + qemu_log_mask(LOG_GUEST_ERROR, "Trying to read privileged spr " > + "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn, > + ctx->base.pc_next - 4); > } > gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); > } > @@ -3951,12 +3947,9 @@ static inline void gen_op_mfspr(DisasContext *ctx) > return; > } > /* Not defined */ > - fprintf(stderr, "Trying to read invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - if (qemu_log_separate()) { > - qemu_log("Trying to read invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - } > + qemu_log_mask(LOG_GUEST_ERROR, > + "Trying to read invalid spr %d (0x%03x) at " > + TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > > /* The behaviour depends on MSR:PR and SPR# bit 0x10, > * it can generate a priv, a hv emu or a no-op > @@ -4097,12 +4090,9 @@ static void gen_mtspr(DisasContext *ctx) > (*write_cb)(ctx, sprn, rS(ctx->opcode)); > } else { > /* Privilege exception */ > - fprintf(stderr, "Trying to write privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - if (qemu_log_separate()) { > - qemu_log("Trying to write privileged spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - } > + qemu_log_mask(LOG_GUEST_ERROR, "Trying to write privileged spr " > + "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn, > + ctx->base.pc_next - 4); > gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); > } > } else { > @@ -4114,12 +4104,9 @@ static void gen_mtspr(DisasContext *ctx) > } > > /* Not defined */ > - if (qemu_log_separate()) { > - qemu_log("Trying to write invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > - } > - fprintf(stderr, "Trying to write invalid spr %d (0x%03x) at " > - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > + qemu_log_mask(LOG_GUEST_ERROR, > + "Trying to write invalid spr %d (0x%03x) at " > + TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); > > > /* The behaviour depends on MSR:PR and SPR# bit 0x10,
diff --git a/target/ppc/translate.c b/target/ppc/translate.c index e30d99f..0806ee0 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -3933,13 +3933,9 @@ static inline void gen_op_mfspr(DisasContext *ctx) * allowing userland application to read the PVR */ if (sprn != SPR_PVR) { - fprintf(stderr, "Trying to read privileged spr %d (0x%03x) at " - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); - if (qemu_log_separate()) { - qemu_log("Trying to read privileged spr %d (0x%03x) at " - TARGET_FMT_lx "\n", sprn, sprn, - ctx->base.pc_next - 4); - } + qemu_log_mask(LOG_GUEST_ERROR, "Trying to read privileged spr " + "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn, + ctx->base.pc_next - 4); } gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); } @@ -3951,12 +3947,9 @@ static inline void gen_op_mfspr(DisasContext *ctx) return; } /* Not defined */ - fprintf(stderr, "Trying to read invalid spr %d (0x%03x) at " - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); - if (qemu_log_separate()) { - qemu_log("Trying to read invalid spr %d (0x%03x) at " - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); - } + qemu_log_mask(LOG_GUEST_ERROR, + "Trying to read invalid spr %d (0x%03x) at " + TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); /* The behaviour depends on MSR:PR and SPR# bit 0x10, * it can generate a priv, a hv emu or a no-op @@ -4097,12 +4090,9 @@ static void gen_mtspr(DisasContext *ctx) (*write_cb)(ctx, sprn, rS(ctx->opcode)); } else { /* Privilege exception */ - fprintf(stderr, "Trying to write privileged spr %d (0x%03x) at " - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); - if (qemu_log_separate()) { - qemu_log("Trying to write privileged spr %d (0x%03x) at " - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); - } + qemu_log_mask(LOG_GUEST_ERROR, "Trying to write privileged spr " + "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn, + ctx->base.pc_next - 4); gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG); } } else { @@ -4114,12 +4104,9 @@ static void gen_mtspr(DisasContext *ctx) } /* Not defined */ - if (qemu_log_separate()) { - qemu_log("Trying to write invalid spr %d (0x%03x) at " - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); - } - fprintf(stderr, "Trying to write invalid spr %d (0x%03x) at " - TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); + qemu_log_mask(LOG_GUEST_ERROR, + "Trying to write invalid spr %d (0x%03x) at " + TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4); /* The behaviour depends on MSR:PR and SPR# bit 0x10,
fprintf() and qemu_log_separate() are frowned upon these days for printing logging information in QEMU. Accessing the wrong SPRs indicates wrong guest behaviour in most cases, and we've got a proper way to log such situations, which is the qemu_log_mask(LOG_GUEST_ERROR, ...) function. So use this function now for logging the bad SPR accesses instead. Signed-off-by: Thomas Huth <thuth@redhat.com> --- target/ppc/translate.c | 37 ++++++++++++------------------------- 1 file changed, 12 insertions(+), 25 deletions(-)