| Message ID | 1360600885-6917-5-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
Am 11.02.2013 17:41, schrieb Peter Maydell: > Rename the public-facing function cpu_set_log to qemu_set_log. This > requires us to rename the internal-only qemu_set_log() to > do_qemu_set_log(). > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > bsd-user/main.c | 2 +- > cpus.c | 2 +- > hw/ppc.c | 2 +- > include/qemu/log.h | 12 ++++++++---- > linux-user/main.c | 2 +- > monitor.c | 2 +- > qemu-log.c | 4 ++-- > target-i386/translate.c | 2 +- > tcg/tci/tcg-target.c | 2 +- > 9 files changed, 17 insertions(+), 13 deletions(-) [...] > diff --git a/hw/ppc.c b/hw/ppc.c > index c52e22f..6053bd5 100644 > --- a/hw/ppc.c > +++ b/hw/ppc.c > @@ -1189,7 +1189,7 @@ void PPC_debug_write (void *opaque, uint32_t addr, uint32_t val) > break; > case 2: > printf("Set loglevel to %04" PRIx32 "\n", val); > - cpu_set_log(val | 0x100); > + qemu_set_log(val | 0x100); > break; > } > } I haven't the foggiest what this magic number is about, did you find out? But since it's only a mechanical renaming, Reviewed-by: Andreas Färber <afaerber@suse.de> Andreas
On 11 February 2013 19:28, Andreas Färber <afaerber@suse.de> wrote: > Am 11.02.2013 17:41, schrieb Peter Maydell: >> diff --git a/hw/ppc.c b/hw/ppc.c >> index c52e22f..6053bd5 100644 >> --- a/hw/ppc.c >> +++ b/hw/ppc.c >> @@ -1189,7 +1189,7 @@ void PPC_debug_write (void *opaque, uint32_t addr, uint32_t val) >> break; >> case 2: >> printf("Set loglevel to %04" PRIx32 "\n", val); >> - cpu_set_log(val | 0x100); >> + qemu_set_log(val | 0x100); >> break; >> } >> } > > I haven't the foggiest what this magic number is about, did you find out? Alex said PReP was yours and I should ask you :-) 0x100 is CPU_LOG_TB_CPU. This raw 0x100 was introduced in commit fd0bbb12 by Fabrice in 2004 with the helpful commit message "cmdline init fix". At that time 0x100 meant the same thing it does today, so you could just replace it with the symbolic constant. I have no idea if there's any documentation of the OpenFirmware/QEMU contract for what the semantics of the magic io port are :-) The OpenHackWare end of this connection does this: #ifdef DEBUG_BIOS emit_buffer[emit_pos] = '\0'; if (strcmp(emit_buffer, "Call Kernel!") == 0) { /* Set qemu in debug mode: * log in_asm,op,int,ioport,cpu */ uint16_t loglevel = 0x02 | 0x10 | 0x80; // outw(0xFF02, loglevel); outb(0x0F02, loglevel); } emit_pos = 0; #endif and frankly encoding QEMU's internal constant values into the BIOS is insane (for a start, the specified constant doesn't do what the comment says, because it's missing 'op', which is 0x4). So, er, yeah. I think if I were you I'd change the code to one of: (a) use the symbolic constant rather than 0x100 (b) drop the "| 0x100", ie change it to "just pass the value through", on the basis that anybody messing with DEBUG_BIOS is capable of setting the flags they actually want on their end (c) change it to "anything non-zero means we set the log level to some 'debug info we think you might like' setting; zero means set log level to zero" (keeps QEMU internal constant values internal) (d) change it to just ignore attempts to set the loglevel via the debug port (don't let the guest mess with user command line specified logging level) (e) rip the entire case out as a gross hack and assume nobody was weird enough to be running a DEBUG_BIOS openhackware (as d, plus remove odd code from QEMU) depending on how ugly you think it is and how spiky you're feeling :-) -- PMM
diff --git a/bsd-user/main.c b/bsd-user/main.c index 4b12e65..097fbfe 100644 --- a/bsd-user/main.c +++ b/bsd-user/main.c @@ -870,7 +870,7 @@ int main(int argc, char **argv) qemu_print_log_usage(stdout); exit(1); } - cpu_set_log(mask); + qemu_set_log(mask); } if (optind >= argc) { diff --git a/cpus.c b/cpus.c index 63cfb73..24e6aff 100644 --- a/cpus.c +++ b/cpus.c @@ -1184,7 +1184,7 @@ void set_cpu_log(const char *optarg) qemu_print_log_usage(stdout); exit(1); } - cpu_set_log(mask); + qemu_set_log(mask); } void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg) diff --git a/hw/ppc.c b/hw/ppc.c index c52e22f..6053bd5 100644 --- a/hw/ppc.c +++ b/hw/ppc.c @@ -1189,7 +1189,7 @@ void PPC_debug_write (void *opaque, uint32_t addr, uint32_t val) break; case 2: printf("Set loglevel to %04" PRIx32 "\n", val); - cpu_set_log(val | 0x100); + qemu_set_log(val | 0x100); break; } } diff --git a/include/qemu/log.h b/include/qemu/log.h index 10792ce..5dcbe11 100644 --- a/include/qemu/log.h +++ b/include/qemu/log.h @@ -143,14 +143,18 @@ typedef struct CPULogItem { extern const CPULogItem cpu_log_items[]; -void qemu_set_log(int log_flags, bool use_own_buffers); +/* This is the function that actually does the work of + * changing the log level; it should only be accessed via + * the qemu_set_log() wrapper. + */ +void do_qemu_set_log(int log_flags, bool use_own_buffers); -static inline void cpu_set_log(int log_flags) +static inline void qemu_set_log(int log_flags) { #ifdef CONFIG_USER_ONLY - qemu_set_log(log_flags, true); + do_qemu_set_log(log_flags, true); #else - qemu_set_log(log_flags, false); + do_qemu_set_log(log_flags, false); #endif } diff --git a/linux-user/main.c b/linux-user/main.c index 5257552..8a61ea4 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -3104,7 +3104,7 @@ static void handle_arg_log(const char *arg) qemu_print_log_usage(stdout); exit(1); } - cpu_set_log(mask); + qemu_set_log(mask); } static void handle_arg_log_filename(const char *arg) diff --git a/monitor.c b/monitor.c index 2a55d56..6aac4c2 100644 --- a/monitor.c +++ b/monitor.c @@ -981,7 +981,7 @@ static void do_log(Monitor *mon, const QDict *qdict) return; } } - cpu_set_log(mask); + qemu_set_log(mask); } static void do_singlestep(Monitor *mon, const QDict *qdict) diff --git a/qemu-log.c b/qemu-log.c index 10a8581..a96db88 100644 --- a/qemu-log.c +++ b/qemu-log.c @@ -54,7 +54,7 @@ void qemu_log_mask(int mask, const char *fmt, ...) } /* enable or disable low levels log */ -void qemu_set_log(int log_flags, bool use_own_buffers) +void do_qemu_set_log(int log_flags, bool use_own_buffers) { const char *fname = logfilename ?: DEFAULT_LOGFILENAME; @@ -94,7 +94,7 @@ void qemu_set_log_filename(const char *filename) fclose(qemu_logfile); qemu_logfile = NULL; } - cpu_set_log(qemu_loglevel); + qemu_set_log(qemu_loglevel); } const CPULogItem cpu_log_items[] = { diff --git a/target-i386/translate.c b/target-i386/translate.c index 32d21f5..112c310 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -6854,7 +6854,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, #else /* start debug */ tb_flush(env); - cpu_set_log(CPU_LOG_INT | CPU_LOG_TB_IN_ASM); + qemu_set_log(CPU_LOG_INT | CPU_LOG_TB_IN_ASM); #endif break; #endif diff --git a/tcg/tci/tcg-target.c b/tcg/tci/tcg-target.c index 1707169..2d561b3 100644 --- a/tcg/tci/tcg-target.c +++ b/tcg/tci/tcg-target.c @@ -888,7 +888,7 @@ static void tcg_target_init(TCGContext *s) #if defined(CONFIG_DEBUG_TCG_INTERPRETER) const char *envval = getenv("DEBUG_TCG"); if (envval) { - cpu_set_log(strtol(envval, NULL, 0)); + qemu_set_log(strtol(envval, NULL, 0)); } #endif
Rename the public-facing function cpu_set_log to qemu_set_log. This requires us to rename the internal-only qemu_set_log() to do_qemu_set_log(). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- bsd-user/main.c | 2 +- cpus.c | 2 +- hw/ppc.c | 2 +- include/qemu/log.h | 12 ++++++++---- linux-user/main.c | 2 +- monitor.c | 2 +- qemu-log.c | 4 ++-- target-i386/translate.c | 2 +- tcg/tci/tcg-target.c | 2 +- 9 files changed, 17 insertions(+), 13 deletions(-)