monitor+disas: Remove uses of ENV_GET_CPU

PATCH Superseded Peter Crosthwaite 5 participants 9 messages May 17, 2015 – May 24, 2015

Share & tools

Peter Crosthwaite

author

2015-05-17 19:52 UTC #0 lore raw

cover monitor+disas: Remove uses of ENV_GET_CPU

Neither the monitor or disassembly core has a good reason to navigate from an env pointer to a cpu pointer. Disas should not need env awarness at all, that is removed in P2.

The monitor is trickier, the env is still needed by some #ifdef switched target specific code but all common code only needs to trade in CPU pointers. As the monitor always has access to a CPU pointer naturally, remove ENV_GET_CPU usages (P1).

This is related to my multi-arch work, where the goal is to minimise use of architecture defined global definitions, ENV_GET_CPU being a major headache in that whole effort. The longer term goal is to limit ENV_GET_CPU use to genuinely architecture specific code.

But I think these two patches stand in their own right, so sending ahead of the motherload series. This brings both modules closer to common-oby-y'ification.

First RFC for multi arch is avaiable here:

https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg01771.html

The two patches are done together to avoid a conflict with monitor_disas which is touched by both patches. If one patch gets acked, the other nacked then either can be merged independently with trivial edits.

Peter Crosthwaite (2):
monitor: Split mon_get_cpu fn to remove ENV_GET_CPU
disas: Remove uses of CPU env

 disas.c                       | 14 +++++-----
 include/disas/disas.h         |  4 +--
 include/qemu/log.h            |  4 +--
 monitor.c                     | 62 +++++++++++++++++++------------------------
 target-alpha/translate.c      |  2 +-
 target-arm/translate-a64.c    |  2 +-
 target-arm/translate.c        |  2 +-
 target-cris/translate.c       |  2 +-
 target-i386/translate.c       |  2 +-
 target-lm32/translate.c       |  2 +-
 target-m68k/translate.c       |  2 +-
 target-microblaze/translate.c |  2 +-
 target-mips/translate.c       |  2 +-
 target-openrisc/translate.c   |  2 +-
 target-ppc/translate.c        |  2 +-
 target-s390x/translate.c      |  2 +-
 target-sh4/translate.c        |  2 +-
 target-sparc/translate.c      |  2 +-
 target-tricore/translate.c    |  2 +-
 target-unicore32/translate.c  |  2 +-
 target-xtensa/translate.c     |  2 +-
 21 files changed, 56 insertions(+), 62 deletions(-)

Signature · 1 line

1.9.1

Peter Crosthwaite

author

2015-05-17 19:52 UTC #1 lore raw

PATCH 1/2 monitor: Split mon_get_cpu fn to remove ENV_GET_CPU

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 <[email protected]>

---
 monitor.c | 62 ++++++++++++++++++++++++++++----------------------------------
 1 file changed, 28 insertions(+), 34 deletions(-)

monitor.c +28 ~~-34~~

index b2561e1..e6df072 100644
@@ -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:

Signature · 1 line

1.9.1

Peter Crosthwaite

author

2015-05-17 19:52 UTC #6 lore raw

PATCH 2/2 disas: Remove uses of CPU env

In reply to Peter Crosthwaite

disas does not need to access the CPU env for any reason. Change the APIs to accept CPU pointers instead. Small change pattern needs to be applied to all target translate.c. This brings us closer to making disas.o a common-obj and less architecture specific in general.

Cc Richard Henderson <[email protected]>

Cc Peter Maydell <[email protected]>

Cc "Edgar E. Iglesias" <[email protected]>

Cc Paolo Bonzini <[email protected]>

Cc Eduardo Habkost <[email protected]>

Cc Michael Walle <[email protected]>

Cc Aurelien Jarno <[email protected]>

Cc Leon Alrae <[email protected]>

Cc Jia Liu <[email protected]>

Cc Alexander Graf <[email protected]>

Cc Mark Cave-Ayland <[email protected]>

Cc Bastian Koppelmann <[email protected]>

Cc Guan Xuetao <[email protected]>

Cc Max Filippov <[email protected]>

Signed-off-by Peter Crosthwaite <[email protected]>

---
 disas.c                       | 14 +++++++-------
 include/disas/disas.h         |  4 ++--
 include/qemu/log.h            |  4 ++--
 monitor.c                     |  2 +-
 target-alpha/translate.c      |  2 +-
 target-arm/translate-a64.c    |  2 +-
 target-arm/translate.c        |  2 +-
 target-cris/translate.c       |  2 +-
 target-i386/translate.c       |  2 +-
 target-lm32/translate.c       |  2 +-
 target-m68k/translate.c       |  2 +-
 target-microblaze/translate.c |  2 +-
 target-mips/translate.c       |  2 +-
 target-openrisc/translate.c   |  2 +-
 target-ppc/translate.c        |  2 +-
 target-s390x/translate.c      |  2 +-
 target-sh4/translate.c        |  2 +-
 target-sparc/translate.c      |  2 +-
 target-tricore/translate.c    |  2 +-
 target-unicore32/translate.c  |  2 +-
 target-xtensa/translate.c     |  2 +-
 21 files changed, 29 insertions(+), 29 deletions(-)

disas.c +7 -7

index 44a019a..576c6a4 100644
@@ -9,7 +9,7 @@
 
 typedef struct CPUDebug {
     struct disassemble_info info;
-    CPUArchState *env;
+    CPUState *cpu;
 } CPUDebug;
 
 /* Filled in by elfload.c.  Simplistic, but will do for now. */

@@ -39,7 +39,7 @@ target_read_memory (bfd_vma memaddr,
 {
     CPUDebug *s = container_of(info, CPUDebug, info);
 
-    cpu_memory_rw_debug(ENV_GET_CPU(s->env), memaddr, myaddr, length, 0);
+    cpu_memory_rw_debug(s->cpu, memaddr, myaddr, length, 0);
     return 0;
 }

@@ -195,7 +195,7 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info)
            bit 16 indicates little endian.
     other targets - unused
  */
-void target_disas(FILE *out, CPUArchState *env, target_ulong code,
+void target_disas(FILE *out, CPUState *cpu, target_ulong code,
                   target_ulong size, int flags)
 {
     target_ulong pc;

@@ -205,7 +205,7 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code,
 
     INIT_DISASSEMBLE_INFO(s.info, out, fprintf);
 
-    s.env = env;
+    s.cpu = cpu;
     s.info.read_memory_func = target_read_memory;
     s.info.buffer_vma = code;
     s.info.buffer_length = size;

@@ -430,7 +430,7 @@ monitor_read_memory (bfd_vma memaddr, bfd_byte *myaddr, int length,
     if (monitor_disas_is_physical) {
         cpu_physical_memory_read(memaddr, myaddr, length);
     } else {
-        cpu_memory_rw_debug(ENV_GET_CPU(s->env), memaddr, myaddr, length, 0);
+        cpu_memory_rw_debug(s->cpu, memaddr, myaddr, length, 0);
     }
     return 0;
 }

@@ -447,7 +447,7 @@ monitor_fprintf(FILE *stream, const char *fmt, ...)
 
 /* Disassembler for the monitor.
    See target_disas for a description of flags. */
-void monitor_disas(Monitor *mon, CPUArchState *env,
+void monitor_disas(Monitor *mon, CPUState *cpu,
                    target_ulong pc, int nb_insn, int is_physical, int flags)
 {
     int count, i;

@@ -456,7 +456,7 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
 
     INIT_DISASSEMBLE_INFO(s.info, (FILE *)mon, monitor_fprintf);
 
-    s.env = env;
+    s.cpu = cpu;
     monitor_disas_is_physical = is_physical;
     s.info.read_memory_func = monitor_read_memory;
     s.info.print_address_func = generic_print_target_address;

include/disas/disas.h +2 -2

index c13ca9a..2b9293b 100644
@@ -6,10 +6,10 @@
 #ifdef NEED_CPU_H
 /* Disassemble this for me please... (debugging). */
 void disas(FILE *out, void *code, unsigned long size);
-void target_disas(FILE *out, CPUArchState *env, target_ulong code,
+void target_disas(FILE *out, CPUState *cpu, target_ulong code,
                   target_ulong size, int flags);
 
-void monitor_disas(Monitor *mon, CPUArchState *env,
+void monitor_disas(Monitor *mon, CPUState *cpu,
                    target_ulong pc, int nb_insn, int is_physical, int flags);
 
 /* Look up symbol for debugging purpose.  Returns "" if unknown. */

include/qemu/log.h +2 -2

index 195f665..f880e66 100644
@@ -104,10 +104,10 @@ static inline void log_cpu_state_mask(int mask, CPUState *cpu, int flags)
 
 #ifdef NEED_CPU_H
 /* disas() and target_disas() to qemu_logfile: */
-static inline void log_target_disas(CPUArchState *env, target_ulong start,
+static inline void log_target_disas(CPUState *cpu, target_ulong start,
                                     target_ulong len, int flags)
 {
-    target_disas(qemu_logfile, env, start, len, flags);
+    target_disas(qemu_logfile, cpu, start, len, flags);
 }
 
 static inline void log_disas(void *code, unsigned long size)

monitor.c +1 -1

index e6df072..9d29d06 100644
@@ -1236,7 +1236,7 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize,
         flags = msr_le << 16;
         flags |= env->bfd_mach;
 #endif
-        monitor_disas(mon, mon_get_env(), addr, count, is_physical, flags);
+        monitor_disas(mon, mon_get_cpu(), addr, count, is_physical, flags);
         return;
     }

target-alpha/translate.c +1 -1

index efeeb05..5fe7ecd 100644
@@ -2926,7 +2926,7 @@ static inline void gen_intermediate_code_internal(AlphaCPU *cpu,
 #ifdef DEBUG_DISAS
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(env, pc_start, ctx.pc - pc_start, 1);
+        log_target_disas(cs, pc_start, ctx.pc - pc_start, 1);
         qemu_log("\n");
     }
 #endif

target-arm/translate-a64.c +1 -1

index 0b192a1..220f055 100644
@@ -11114,7 +11114,7 @@ done_generating:
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
         qemu_log("----------------\n");
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(env, pc_start, dc->pc - pc_start,
+        log_target_disas(cs, pc_start, dc->pc - pc_start,
                          4 | (dc->bswap_code << 1));
         qemu_log("\n");
     }

target-arm/translate.c +1 -1

index 9116529..cd91b13 100644
@@ -11365,7 +11365,7 @@ done_generating:
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
         qemu_log("----------------\n");
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(env, pc_start, dc->pc - pc_start,
+        log_target_disas(cs, pc_start, dc->pc - pc_start,
                          dc->thumb | (dc->bswap_code << 1));
         qemu_log("\n");
     }

target-cris/translate.c +1 -1

index 687c88b..3e59601 100644
@@ -3345,7 +3345,7 @@ gen_intermediate_code_internal(CRISCPU *cpu, TranslationBlock *tb,
 #ifdef DEBUG_DISAS
 #if !DISAS_CRIS
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
-        log_target_disas(env, pc_start, dc->pc - pc_start,
+        log_target_disas(cs, pc_start, dc->pc - pc_start,
                          env->pregs[PR_VR]);
         qemu_log("\nisize=%d osize=%d\n",
                  dc->pc - pc_start, tcg_op_buf_count());

target-i386/translate.c +1 -1

index 305ce50..723e0cb 100644
@@ -8093,7 +8093,7 @@ done_generating:
         else
 #endif
             disas_flags = !dc->code32;
-        log_target_disas(env, pc_start, pc_ptr - pc_start, disas_flags);
+        log_target_disas(cs, pc_start, pc_ptr - pc_start, disas_flags);
         qemu_log("\n");
     }
 #endif

target-lm32/translate.c +1 -1

index 81a204f..cf7042e 100644
@@ -1168,7 +1168,7 @@ void gen_intermediate_code_internal(LM32CPU *cpu,
 #ifdef DEBUG_DISAS
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
         qemu_log("\n");
-        log_target_disas(env, pc_start, dc->pc - pc_start, 0);
+        log_target_disas(cs, pc_start, dc->pc - pc_start, 0);
         qemu_log("\nisize=%d osize=%d\n",
                  dc->pc - pc_start, tcg_op_buf_count());
     }

target-m68k/translate.c +1 -1

index 4959b97..22ecc20 100644
@@ -3077,7 +3077,7 @@ gen_intermediate_code_internal(M68kCPU *cpu, TranslationBlock *tb,
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
         qemu_log("----------------\n");
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(env, pc_start, dc->pc - pc_start, 0);
+        log_target_disas(cs, pc_start, dc->pc - pc_start, 0);
         qemu_log("\n");
     }
 #endif

target-microblaze/translate.c +1 -1

index 4068946..633131e 100644
@@ -1855,7 +1855,7 @@ gen_intermediate_code_internal(MicroBlazeCPU *cpu, TranslationBlock *tb,
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
         qemu_log("\n");
 #if DISAS_GNU
-        log_target_disas(env, pc_start, dc->pc - pc_start, 0);
+        log_target_disas(cs, pc_start, dc->pc - pc_start, 0);
 #endif
         qemu_log("\nisize=%d osize=%d\n",
                  dc->pc - pc_start, tcg_op_buf_count());

target-mips/translate.c +1 -1

index fd063a2..f8f8f94 100644
@@ -19273,7 +19273,7 @@ done_generating:
     LOG_DISAS("\n");
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(env, pc_start, ctx.pc - pc_start, 0);
+        log_target_disas(cs, pc_start, ctx.pc - pc_start, 0);
         qemu_log("\n");
     }
 #endif

target-openrisc/translate.c +1 -1

index dc76789..a62cbf4 100644
@@ -1770,7 +1770,7 @@ static inline void gen_intermediate_code_internal(OpenRISCCPU *cpu,
 #ifdef DEBUG_DISAS
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
         qemu_log("\n");
-        log_target_disas(&cpu->env, pc_start, dc->pc - pc_start, 0);
+        log_target_disas(cs, pc_start, dc->pc - pc_start, 0);
         qemu_log("\nisize=%d osize=%d\n",
                  dc->pc - pc_start, tcg_op_buf_count());
     }

target-ppc/translate.c +1 -1

index 8f255ea..84c5cea 100644
@@ -11607,7 +11607,7 @@ static inline void gen_intermediate_code_internal(PowerPCCPU *cpu,
         flags = env->bfd_mach;
         flags |= ctx.le_mode << 16;
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(env, pc_start, ctx.nip - pc_start, flags);
+        log_target_disas(cs, pc_start, ctx.nip - pc_start, flags);
         qemu_log("\n");
     }
 #endif

target-s390x/translate.c +1 -1

index 80e3a54..2fdd542 100644
@@ -4990,7 +4990,7 @@ static inline void gen_intermediate_code_internal(S390CPU *cpu,
 #if defined(S390X_DEBUG_DISAS)
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(env, pc_start, dc.pc - pc_start, 1);
+        log_target_disas(cs, pc_start, dc.pc - pc_start, 1);
         qemu_log("\n");
     }
 #endif

target-sh4/translate.c +1 -1

index 41aa928..27858e5 100644
@@ -1974,7 +1974,7 @@ gen_intermediate_code_internal(SuperHCPU *cpu, TranslationBlock *tb,
 #ifdef DEBUG_DISAS
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
 	qemu_log("IN:\n");	/* , lookup_symbol(pc_start)); */
-        log_target_disas(env, pc_start, ctx.pc - pc_start, 0);
+        log_target_disas(cs, pc_start, ctx.pc - pc_start, 0);
 	qemu_log("\n");
     }
 #endif

target-sparc/translate.c +1 -1

index 3708c01..c58dd4e 100644
@@ -5334,7 +5334,7 @@ static inline void gen_intermediate_code_internal(SPARCCPU *cpu,
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
         qemu_log("--------------\n");
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(env, pc_start, last_pc + 4 - pc_start, 0);
+        log_target_disas(cs, pc_start, last_pc + 4 - pc_start, 0);
         qemu_log("\n");
     }
 #endif

target-tricore/translate.c +1 -1

index 663b2a0..5abfbbf 100644
@@ -8173,7 +8173,7 @@ gen_intermediate_code_internal(TriCoreCPU *cpu, struct TranslationBlock *tb,
 #ifdef DEBUG_DISAS
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(env, pc_start, ctx.pc - pc_start, 0);
+        log_target_disas(cs, pc_start, ctx.pc - pc_start, 0);
         qemu_log("\n");
     }
 #endif

target-unicore32/translate.c +1 -1

index 9efcff5..2fc78e6 100644
@@ -2039,7 +2039,7 @@ done_generating:
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
         qemu_log("----------------\n");
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(env, pc_start, dc->pc - pc_start, 0);
+        log_target_disas(cs, pc_start, dc->pc - pc_start, 0);
         qemu_log("\n");
     }
 #endif

target-xtensa/translate.c +1 -1

index 6e5096c..3bdd052 100644
@@ -3137,7 +3137,7 @@ void gen_intermediate_code_internal(XtensaCPU *cpu,
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
         qemu_log("----------------\n");
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(env, pc_start, dc.pc - pc_start, 0);
+        log_target_disas(cs, pc_start, dc.pc - pc_start, 0);
         qemu_log("\n");
     }
 #endif

Signature · 1 line

1.9.1

Richard Henderson

Reviewed-by on v1 1/2

2015-05-18 16:13 UTC #2 lore raw

Re: [Qemu-devel] [PATCH 1/2] monitor: Split mon_get_cpu fn to remove ENV_GET_CPU

In reply to Peter Crosthwaite

Quoted 9 lines · Peter Crosthwaite

monitor.c +0 -1

@@ -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;

monitor.c static void memory_dump(Monitor *mon, int count, int format, int wsize, · line 1215

Quoted 7 lines

monitor.c +1 -1

@@ -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 <[email protected]>

Richard Henderson

Reviewed-by on v1 2/2

2015-05-18 16:15 UTC #7 lore raw

Re: [Qemu-devel] [PATCH 2/2] disas: Remove uses of CPU env

In reply to Peter Crosthwaite

Quoted 35 lines · Peter Crosthwaite

disas does not need to access the CPU env for any reason. Change the APIs to accept CPU pointers instead. Small change pattern needs to be applied to all target translate.c. This brings us closer to making disas.o a common-obj and less architecture specific in general.
Cc Richard Henderson <[email protected]>
Cc Peter Maydell <[email protected]>
Cc "Edgar E. Iglesias" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Eduardo Habkost <[email protected]>
Cc Michael Walle <[email protected]>
Cc Aurelien Jarno <[email protected]>
Cc Leon Alrae <[email protected]>
Cc Jia Liu <[email protected]>
Cc Alexander Graf <[email protected]>
Cc Mark Cave-Ayland <[email protected]>
Cc Bastian Koppelmann <[email protected]>
Cc Guan Xuetao <[email protected]>
Cc Max Filippov <[email protected]>
Signed-off-by Peter Crosthwaite <[email protected]>
---
 disas.c                       | 14 +++++++-------
 include/disas/disas.h         |  4 ++--
 include/qemu/log.h            |  4 ++--
 monitor.c                     |  2 +-
 target-alpha/translate.c      |  2 +-
 target-arm/translate-a64.c    |  2 +-
 target-arm/translate.c        |  2 +-
 target-cris/translate.c       |  2 +-
 target-i386/translate.c       |  2 +-
 target-lm32/translate.c       |  2 +-
 target-m68k/translate.c       |  2 +-
 target-microblaze/translate.c |  2 +-
 target-mips/translate.c       |  2 +-
 target-openrisc/translate.c   |  2 +-

Quoted 8 lines

 target-ppc/translate.c        |  2 +-
 target-s390x/translate.c      |  2 +-
 target-sh4/translate.c        |  2 +-
 target-sparc/translate.c      |  2 +-
 target-tricore/translate.c    |  2 +-
 target-unicore32/translate.c  |  2 +-
 target-xtensa/translate.c     |  2 +-
 21 files changed, 29 insertions(+), 29 deletions(-)

Reviewed-by Richard Henderson <[email protected]>

Peter Crosthwaite

2015-05-18 16:44 UTC #3 lore raw

Re: [Qemu-devel] [PATCH 1/2] monitor: Split mon_get_cpu fn to remove ENV_GET_CPU

In reply to Richard Henderson

Quoted 11 lines · Richard Henderson

Quoted 10 lines · Peter Crosthwaite

monitor.c +0 -1

@@ -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') {

Quoted 8 lines

monitor.c static void memory_dump(Monitor *mon, int count, int format, int wsize, · line 1215
Quoted 6 lines
monitor.c +1 -1
         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.

Quoted 4 lines

Anyway, fixable by changing the line above to
    int flags = 0;

I'll just make this change.

Quoted 4 lines

Otherwise,
Reviewed-by Richard Henderson <[email protected]>

Thanks.

Regards,
Peter

Quoted 3 lines

r~

Andreas Färber

Reviewed-by

2015-05-18 16:52 UTC #4 lore raw

Re: [Qemu-devel] [PATCH 1/2] monitor: Split mon_get_cpu fn to remove ENV_GET_CPU

Am 18.05.2015 um 18:44 schrieb Peter Crosthwaite:

Quoted 12 lines · Richard Henderson

Quoted 11 lines · Peter Crosthwaite

Quoted 10 lines

monitor.c +0 -1

@@ -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') {

Quoted 8 lines

Quoted 8 lines
monitor.c static void memory_dump(Monitor *mon, int count, int format, int wsize, · line 1215
Quoted 6 lines
monitor.c +1 -1
         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.

Quoted 8 lines

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.

Quoted 6 lines

Quoted 4 lines
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 <[email protected]>

Regards,
Andreas

Quoted 8 lines

Quoted 3 lines
Otherwise,
Reviewed-by Richard Henderson <[email protected]>
Thanks.
Regards,
Peter

Signature · 3 lines

SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

Eduardo Habkost

Reviewed-by on v1 2/2

2015-05-18 17:21 UTC #8 lore raw

Re: [Qemu-devel] [PATCH 2/2] disas: Remove uses of CPU env

In reply to Peter Crosthwaite

Quoted 5 lines · Peter Crosthwaite

disas does not need to access the CPU env for any reason. Change the APIs to accept CPU pointers instead. Small change pattern needs to be applied to all target translate.c. This brings us closer to making disas.o a common-obj and less architecture specific in general.

Quoted 15 lines

Cc Richard Henderson <[email protected]>
Cc Peter Maydell <[email protected]>
Cc "Edgar E. Iglesias" <[email protected]>
Cc Paolo Bonzini <[email protected]>
Cc Eduardo Habkost <[email protected]>
Cc Michael Walle <[email protected]>
Cc Aurelien Jarno <[email protected]>
Cc Leon Alrae <[email protected]>
Cc Jia Liu <[email protected]>
Cc Alexander Graf <[email protected]>
Cc Mark Cave-Ayland <[email protected]>
Cc Bastian Koppelmann <[email protected]>
Cc Guan Xuetao <[email protected]>
Cc Max Filippov <[email protected]>
Signed-off-by Peter Crosthwaite <[email protected]>

Reviewed-by Eduardo Habkost <[email protected]>

Signature · 1 line

Eduardo

Peter Crosthwaite

2015-05-24 20:31 UTC #5 lore raw

Re: [Qemu-devel] [PATCH 1/2] monitor: Split mon_get_cpu fn to remove ENV_GET_CPU

In reply to Andreas Färber

Quoted 39 lines · Andreas Färber

Am 18.05.2015 um 18:44 schrieb Peter Crosthwaite:
Quoted 12 lines · Richard Henderson
Quoted 11 lines · Peter Crosthwaite
Quoted 10 lines
monitor.c +0 -1
@@ -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') {
Quoted 8 lines
Quoted 8 lines
monitor.c static void memory_dump(Monitor *mon, int count, int format, int wsize, · line 1215
Quoted 6 lines
monitor.c +1 -1
         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.
Quoted 8 lines
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.
Quoted 6 lines
Quoted 4 lines
Anyway, fixable by changing the line above to
    int flags = 0;
I'll just make this change.

Quoted 9 lines

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

Quoted 2 lines

Reviewed-by Andreas Färber <[email protected]>

Thanks,

Regards,
Peter

Quoted 17 lines

Regards,
Andreas
Quoted 8 lines
Quoted 3 lines
Otherwise,
Reviewed-by Richard Henderson <[email protected]>
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)