| Message ID | 1eddf7c8960c0082d7db599ff3486224a3e6635d.1431891113.git.crosthwaite.peter@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 05/17/2015 12:51 PM, Peter Crosthwaite wrote: > @@ -1208,7 +1203,6 @@ static void monitor_printc(Monitor *mon, int c) > static void memory_dump(Monitor *mon, int count, int format, int wsize, > hwaddr addr, int is_physical) > { > - CPUArchState *env; > int l, line_size, i, max_digits, len; > uint8_t buf[16]; > uint64_t v; > @@ -1216,8 +1210,8 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize, > if (format == 'i') { > int flags; > flags = 0; > - env = mon_get_cpu(); > #ifdef TARGET_I386 > + CPUArchState *env = mon_get_env(); > if (wsize == 2) { C99 declaration after statement. I forget if we care or not? Anyway, fixable by changing the line above to int flags = 0; Otherwise, Reviewed-by: Richard Henderson <rth@twiddle.net> r~
On Mon, May 18, 2015 at 9:13 AM, Richard Henderson <rth@twiddle.net> wrote: > On 05/17/2015 12:51 PM, Peter Crosthwaite wrote: >> @@ -1208,7 +1203,6 @@ static void monitor_printc(Monitor *mon, int c) >> static void memory_dump(Monitor *mon, int count, int format, int wsize, >> hwaddr addr, int is_physical) >> { >> - CPUArchState *env; >> int l, line_size, i, max_digits, len; >> uint8_t buf[16]; >> uint64_t v; >> @@ -1216,8 +1210,8 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize, >> if (format == 'i') { >> int flags; >> flags = 0; >> - env = mon_get_cpu(); >> #ifdef TARGET_I386 >> + CPUArchState *env = mon_get_env(); >> if (wsize == 2) { > > C99 declaration after statement. I forget if we care or not? Generally we do, but I have seen incidences of the notable exception of conditionally compiled code. Otherwise would need two complicated sets of #ifdef. Unfortunately we can't just unconditionally define it anymore, as the hunk below removes the only unconditional usage throwing an "unused" werror. > Anyway, fixable by changing the line above to > > int flags = 0; > I'll just make this change. > Otherwise, > > Reviewed-by: Richard Henderson <rth@twiddle.net> > Thanks. Regards, Peter > > r~ >
Am 18.05.2015 um 18:44 schrieb Peter Crosthwaite: > On Mon, May 18, 2015 at 9:13 AM, Richard Henderson <rth@twiddle.net> wrote: >> On 05/17/2015 12:51 PM, Peter Crosthwaite wrote: >>> @@ -1208,7 +1203,6 @@ static void monitor_printc(Monitor *mon, int c) >>> static void memory_dump(Monitor *mon, int count, int format, int wsize, >>> hwaddr addr, int is_physical) >>> { >>> - CPUArchState *env; >>> int l, line_size, i, max_digits, len; >>> uint8_t buf[16]; >>> uint64_t v; >>> @@ -1216,8 +1210,8 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize, >>> if (format == 'i') { >>> int flags; >>> flags = 0; >>> - env = mon_get_cpu(); >>> #ifdef TARGET_I386 >>> + CPUArchState *env = mon_get_env(); >>> if (wsize == 2) { >> >> C99 declaration after statement. I forget if we care or not? We care, but we haven't strictly enforced it everywhere. > Generally we do, but I have seen incidences of the notable exception > of conditionally compiled code. Otherwise would need two complicated > sets of #ifdef. > > Unfortunately we can't just unconditionally define it anymore, as the > hunk below removes the only unconditional usage throwing an "unused" > werror. > >> Anyway, fixable by changing the line above to >> >> int flags = 0; >> > > I'll just make this change. If this is supposed to go through qom-cpu, I could do that change myself. The alternative would've been #if defined(TARGET_foo) || defined(TARGET_bar) ... #endif for the declaration. I was rather wondering whether mon_get_env() should be ameliorated to mon_get_cpu_env() for clarity? env is just short for environment. Otherwise modulo the above nit, Reviewed-by: Andreas Färber <afaerber@suse.de> Regards, Andreas >> Otherwise, >> >> Reviewed-by: Richard Henderson <rth@twiddle.net> > > Thanks. > > Regards, > Peter
On Mon, May 18, 2015 at 9:52 AM, Andreas Färber <afaerber@suse.de> wrote: > Am 18.05.2015 um 18:44 schrieb Peter Crosthwaite: >> On Mon, May 18, 2015 at 9:13 AM, Richard Henderson <rth@twiddle.net> wrote: >>> On 05/17/2015 12:51 PM, Peter Crosthwaite wrote: >>>> @@ -1208,7 +1203,6 @@ static void monitor_printc(Monitor *mon, int c) >>>> static void memory_dump(Monitor *mon, int count, int format, int wsize, >>>> hwaddr addr, int is_physical) >>>> { >>>> - CPUArchState *env; >>>> int l, line_size, i, max_digits, len; >>>> uint8_t buf[16]; >>>> uint64_t v; >>>> @@ -1216,8 +1210,8 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize, >>>> if (format == 'i') { >>>> int flags; >>>> flags = 0; >>>> - env = mon_get_cpu(); >>>> #ifdef TARGET_I386 >>>> + CPUArchState *env = mon_get_env(); >>>> if (wsize == 2) { >>> >>> C99 declaration after statement. I forget if we care or not? > > We care, but we haven't strictly enforced it everywhere. > >> Generally we do, but I have seen incidences of the notable exception >> of conditionally compiled code. Otherwise would need two complicated >> sets of #ifdef. >> >> Unfortunately we can't just unconditionally define it anymore, as the >> hunk below removes the only unconditional usage throwing an "unused" >> werror. >> >>> Anyway, fixable by changing the line above to >>> >>> int flags = 0; >>> >> >> I'll just make this change. > > If this is supposed to go through qom-cpu, I could do that change > myself. The alternative would've been #if defined(TARGET_foo) || > defined(TARGET_bar) ... #endif for the declaration. > > I was rather wondering whether mon_get_env() should be ameliorated to > mon_get_cpu_env() for clarity? env is just short for environment. > > Otherwise modulo the above nit, > Change made, s/mon_get_env/mon_get_cpu_env > Reviewed-by: Andreas Färber <afaerber@suse.de> > Thanks, Regards, Peter > Regards, > Andreas > >>> Otherwise, >>> >>> Reviewed-by: Richard Henderson <rth@twiddle.net> >> >> Thanks. >> >> Regards, >> Peter > > -- > SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB > 21284 (AG Nürnberg) >
diff --git a/monitor.c b/monitor.c index b2561e1..e6df072 100644 --- a/monitor.c +++ b/monitor.c @@ -1010,28 +1010,28 @@ int monitor_set_cpu(int cpu_index) return 0; } -static CPUArchState *mon_get_cpu(void) +static CPUState *mon_get_cpu(void) { if (!cur_mon->mon_cpu) { monitor_set_cpu(0); } cpu_synchronize_state(cur_mon->mon_cpu); - return cur_mon->mon_cpu->env_ptr; + return cur_mon->mon_cpu; +} + +static CPUArchState *mon_get_env(void) +{ + return mon_get_cpu()->env_ptr; } int monitor_get_cpu_index(void) { - CPUState *cpu = ENV_GET_CPU(mon_get_cpu()); - return cpu->cpu_index; + return mon_get_cpu()->cpu_index; } static void hmp_info_registers(Monitor *mon, const QDict *qdict) { - CPUState *cpu; - CPUArchState *env; - env = mon_get_cpu(); - cpu = ENV_GET_CPU(env); - cpu_dump_state(cpu, (FILE *)mon, monitor_fprintf, CPU_DUMP_FPU); + cpu_dump_state(mon_get_cpu(), (FILE *)mon, monitor_fprintf, CPU_DUMP_FPU); } static void hmp_info_jit(Monitor *mon, const QDict *qdict) @@ -1064,12 +1064,7 @@ static void hmp_info_history(Monitor *mon, const QDict *qdict) static void hmp_info_cpustats(Monitor *mon, const QDict *qdict) { - CPUState *cpu; - CPUArchState *env; - - env = mon_get_cpu(); - cpu = ENV_GET_CPU(env); - cpu_dump_statistics(cpu, (FILE *)mon, &monitor_fprintf, 0); + cpu_dump_statistics(mon_get_cpu(), (FILE *)mon, &monitor_fprintf, 0); } static void hmp_info_trace_events(Monitor *mon, const QDict *qdict) @@ -1208,7 +1203,6 @@ static void monitor_printc(Monitor *mon, int c) static void memory_dump(Monitor *mon, int count, int format, int wsize, hwaddr addr, int is_physical) { - CPUArchState *env; int l, line_size, i, max_digits, len; uint8_t buf[16]; uint64_t v; @@ -1216,8 +1210,8 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize, if (format == 'i') { int flags; flags = 0; - env = mon_get_cpu(); #ifdef TARGET_I386 + CPUArchState *env = mon_get_env(); if (wsize == 2) { flags = 1; } else if (wsize == 4) { @@ -1238,10 +1232,11 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize, } #endif #ifdef TARGET_PPC + CPUArchState *env = mon_get_env(); flags = msr_le << 16; flags |= env->bfd_mach; #endif - monitor_disas(mon, env, addr, count, is_physical, flags); + monitor_disas(mon, mon_get_env(), addr, count, is_physical, flags); return; } @@ -1280,8 +1275,7 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize, if (is_physical) { cpu_physical_memory_read(addr, buf, l); } else { - env = mon_get_cpu(); - if (cpu_memory_rw_debug(ENV_GET_CPU(env), addr, buf, l, 0) < 0) { + if (cpu_memory_rw_debug(mon_get_cpu(), addr, buf, l, 0) < 0) { monitor_printf(mon, " Cannot access memory\n"); break; } @@ -1660,7 +1654,7 @@ static void hmp_info_tlb(Monitor *mon, const QDict *qdict) { CPUArchState *env; - env = mon_get_cpu(); + env = mon_get_env(); if (!(env->cr[0] & CR0_PG_MASK)) { monitor_printf(mon, "PG disabled\n"); @@ -1883,7 +1877,7 @@ static void hmp_info_mem(Monitor *mon, const QDict *qdict) { CPUArchState *env; - env = mon_get_cpu(); + env = mon_get_env(); if (!(env->cr[0] & CR0_PG_MASK)) { monitor_printf(mon, "PG disabled\n"); @@ -1920,7 +1914,7 @@ static void print_tlb(Monitor *mon, int idx, tlb_t *tlb) static void hmp_info_tlb(Monitor *mon, const QDict *qdict) { - CPUArchState *env = mon_get_cpu(); + CPUArchState *env = mon_get_env(); int i; monitor_printf (mon, "ITLB:\n"); @@ -1936,7 +1930,7 @@ static void hmp_info_tlb(Monitor *mon, const QDict *qdict) #if defined(TARGET_SPARC) || defined(TARGET_PPC) || defined(TARGET_XTENSA) static void hmp_info_tlb(Monitor *mon, const QDict *qdict) { - CPUArchState *env1 = mon_get_cpu(); + CPUArchState *env1 = mon_get_env(); dump_mmu((FILE*)mon, (fprintf_function)monitor_printf, env1); } @@ -2969,7 +2963,7 @@ typedef struct MonitorDef { #if defined(TARGET_I386) static target_long monitor_get_pc (const struct MonitorDef *md, int val) { - CPUArchState *env = mon_get_cpu(); + CPUArchState *env = mon_get_env(); return env->eip + env->segs[R_CS].base; } #endif @@ -2977,7 +2971,7 @@ static target_long monitor_get_pc (const struct MonitorDef *md, int val) #if defined(TARGET_PPC) static target_long monitor_get_ccr (const struct MonitorDef *md, int val) { - CPUArchState *env = mon_get_cpu(); + CPUArchState *env = mon_get_env(); unsigned int u; int i; @@ -2990,31 +2984,31 @@ static target_long monitor_get_ccr (const struct MonitorDef *md, int val) static target_long monitor_get_msr (const struct MonitorDef *md, int val) { - CPUArchState *env = mon_get_cpu(); + CPUArchState *env = mon_get_env(); return env->msr; } static target_long monitor_get_xer (const struct MonitorDef *md, int val) { - CPUArchState *env = mon_get_cpu(); + CPUArchState *env = mon_get_env(); return env->xer; } static target_long monitor_get_decr (const struct MonitorDef *md, int val) { - CPUArchState *env = mon_get_cpu(); + CPUArchState *env = mon_get_env(); return cpu_ppc_load_decr(env); } static target_long monitor_get_tbu (const struct MonitorDef *md, int val) { - CPUArchState *env = mon_get_cpu(); + CPUArchState *env = mon_get_env(); return cpu_ppc_load_tbu(env); } static target_long monitor_get_tbl (const struct MonitorDef *md, int val) { - CPUArchState *env = mon_get_cpu(); + CPUArchState *env = mon_get_env(); return cpu_ppc_load_tbl(env); } #endif @@ -3023,7 +3017,7 @@ static target_long monitor_get_tbl (const struct MonitorDef *md, int val) #ifndef TARGET_SPARC64 static target_long monitor_get_psr (const struct MonitorDef *md, int val) { - CPUArchState *env = mon_get_cpu(); + CPUArchState *env = mon_get_env(); return cpu_get_psr(env); } @@ -3031,7 +3025,7 @@ static target_long monitor_get_psr (const struct MonitorDef *md, int val) static target_long monitor_get_reg(const struct MonitorDef *md, int val) { - CPUArchState *env = mon_get_cpu(); + CPUArchState *env = mon_get_env(); return env->regwptr[val]; } #endif @@ -3367,7 +3361,7 @@ static int get_monitor_def(target_long *pval, const char *name) if (md->get_value) { *pval = md->get_value(md, md->offset); } else { - CPUArchState *env = mon_get_cpu(); + CPUArchState *env = mon_get_env(); ptr = (uint8_t *)env + md->offset; switch(md->type) { case MD_I32:
The monitor currently has one helper, mon_get_cpu() which will return an env pointer. The target specific users of this API want an env, but all the target agnostic users really just want the cpu pointer. These users then need to use the target-specifically defined ENV_GET_CPU to navigate back up to the CPU from the ENV. Split the API for the two uses cases to remove all need for ENV_GET_CPU. Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com> --- monitor.c | 62 ++++++++++++++++++++++++++++---------------------------------- 1 file changed, 28 insertions(+), 34 deletions(-)