[Qemu-devel] [PATCH 00/10] Support the Capstone disassembler

qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed

* [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler
@ 2017-09-14 18:35 Richard Henderson
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 01/10] target/i386: Convert to disas_set_info hook Richard Henderson
                   ` (10 more replies)
  0 siblings, 11 replies; 23+ messages in thread
From: Richard Henderson @ 2017-09-14 18:35 UTC (permalink / raw)
  To: qemu-devel

As occasionally discussed on this list, due to licensing conflicts,
we are restricted to a version of libopcodes that pre-dates its
upstream re-licensing to gplv3.  That makes our copy rather old
and dated.

I've already seen this as problematic for s390x guest.  I'm sure
the same problem exists for Power8+, though I haven't looked.
As we go forward with vector operations we'll see this for x86 host.

An alternative is to use a BSD-licensed disassembler:

  https://www.capstone-engine.org/

This is an actively maintained project derived from llvm.  Moreover,
it is already in the major Linux distributions, which makes it easy
to phase in its use.

I've arranged the code such that we attempt to use capstone first,
and if that initialization fails, fall back to the existing code
from binutils.


r~


Richard Henderson (10):
  target/i386: Convert to disas_set_info hook
  target/ppc: Convert to disas_set_info hook
  disas: Remove unused flags arguments
  disas: Support the Capstone disassembler library
  target/i386: Support Capstone in disas_set_info
  target/arm: Support Capstone in disas_set_info
  target/ppc: Support Capstone in disas_set_info
  target/s390x: Support Capstone in disas_set_info
  target/sparc: Support Capstone in disas_set_info
  target/mips: Support Capstone in disas_set_info

 include/disas/bfd.h           |   4 ++
 include/disas/capstone.h      |  38 ++++++++++
 include/disas/disas.h         |   4 +-
 include/exec/log.h            |   4 +-
 target/mips/cpu.h             |   2 +
 disas.c                       | 161 +++++++++++++++++++++++-------------------
 monitor.c                     |  29 +-------
 target/alpha/translate.c      |   2 +-
 target/arm/cpu.c              |  21 +++++-
 target/arm/translate-a64.c    |   3 +-
 target/arm/translate.c        |   3 +-
 target/cris/translate.c       |   3 +-
 target/hppa/translate.c       |   2 +-
 target/i386/cpu.c             |  19 +++++
 target/i386/translate.c       |   8 +--
 target/lm32/translate.c       |   2 +-
 target/m68k/translate.c       |   2 +-
 target/microblaze/translate.c |   2 +-
 target/mips/cpu.c             |   8 ---
 target/mips/translate.c       |   2 +-
 target/mips/translate_init.c  |  36 ++++++++++
 target/nios2/translate.c      |   2 +-
 target/openrisc/translate.c   |   2 +-
 target/ppc/translate.c        |   5 +-
 target/ppc/translate_init.c   |  27 +++++++
 target/s390x/cpu.c            |   2 +
 target/s390x/translate.c      |   2 +-
 target/sh4/translate.c        |   2 +-
 target/sparc/cpu.c            |   3 +
 target/sparc/translate.c      |   2 +-
 target/tricore/translate.c    |   2 +-
 target/unicore32/translate.c  |   2 +-
 target/xtensa/translate.c     |   2 +-
 configure                     |  17 +++++
 34 files changed, 279 insertions(+), 146 deletions(-)
 create mode 100644 include/disas/capstone.h

-- 
2.13.5

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 01/10] target/i386: Convert to disas_set_info hook
  2017-09-14 18:35 [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler Richard Henderson
@ 2017-09-14 18:35 ` Richard Henderson
  2017-09-18 11:47   ` Alex Bennée
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 02/10] target/ppc: " Richard Henderson
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Richard Henderson @ 2017-09-14 18:35 UTC (permalink / raw)
  To: qemu-devel

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 disas.c                 | 22 ++--------------------
 monitor.c               | 21 ---------------------
 target/i386/cpu.c       | 12 ++++++++++++
 target/i386/translate.c |  8 +-------
 4 files changed, 15 insertions(+), 48 deletions(-)

diff --git a/disas.c b/disas.c
index d6a1eb9c8e..2be716fdb2 100644
--- a/disas.c
+++ b/disas.c
@@ -205,16 +205,7 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
         cc->disas_set_info(cpu, &s.info);
     }
 
-#if defined(TARGET_I386)
-    if (flags == 2) {
-        s.info.mach = bfd_mach_x86_64;
-    } else if (flags == 1) {
-        s.info.mach = bfd_mach_i386_i8086;
-    } else {
-        s.info.mach = bfd_mach_i386_i386;
-    }
-    s.info.print_insn = print_insn_i386;
-#elif defined(TARGET_PPC)
+#if defined(TARGET_PPC)
     if ((flags >> 16) & 1) {
         s.info.endian = BFD_ENDIAN_LITTLE;
     }
@@ -390,16 +381,7 @@ void monitor_disas(Monitor *mon, CPUState *cpu,
         cc->disas_set_info(cpu, &s.info);
     }
 
-#if defined(TARGET_I386)
-    if (flags == 2) {
-        s.info.mach = bfd_mach_x86_64;
-    } else if (flags == 1) {
-        s.info.mach = bfd_mach_i386_i8086;
-    } else {
-        s.info.mach = bfd_mach_i386_i386;
-    }
-    s.info.print_insn = print_insn_i386;
-#elif defined(TARGET_PPC)
+#if defined(TARGET_PPC)
     if (flags & 0xFFFF) {
         /* If we have a precise definition of the instruction set, use it. */
         s.info.mach = flags & 0xFFFF;
diff --git a/monitor.c b/monitor.c
index 9239f7adde..3f3ebc31ef 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1310,27 +1310,6 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize,
 
     if (format == 'i') {
         int flags = 0;
-#ifdef TARGET_I386
-        CPUArchState *env = mon_get_cpu_env();
-        if (wsize == 2) {
-            flags = 1;
-        } else if (wsize == 4) {
-            flags = 0;
-        } else {
-            /* as default we use the current CS size */
-            flags = 0;
-            if (env) {
-#ifdef TARGET_X86_64
-                if ((env->efer & MSR_EFER_LMA) &&
-                    (env->segs[R_CS].flags & DESC_L_MASK))
-                    flags = 2;
-                else
-#endif
-                if (!(env->segs[R_CS].flags & DESC_B_MASK))
-                    flags = 1;
-            }
-        }
-#endif
 #ifdef TARGET_PPC
         CPUArchState *env = mon_get_cpu_env();
         flags = msr_le << 16;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 69676e13e1..b869a69c53 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4099,6 +4099,17 @@ static bool x86_cpu_has_work(CPUState *cs)
             !(env->hflags & HF_SMM_MASK));
 }
 
+static void x86_disas_set_info(CPUState *cs, disassemble_info *info)
+{
+    X86CPU *cpu = X86_CPU(cs);
+    CPUX86State *env = &cpu->env;
+
+    info->mach = (env->hflags & HF_CS64_MASK ? bfd_mach_x86_64
+                  : env->hflags & HF_CS32_MASK ? bfd_mach_i386_i386
+                  : bfd_mach_i386_i8086);
+    info->print_insn = print_insn_i386;
+}
+
 static Property x86_cpu_properties[] = {
 #ifdef CONFIG_USER_ONLY
     /* apic_id = 0 by default for *-user, see commit 9886e834 */
@@ -4204,6 +4215,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
 #endif
     cc->cpu_exec_enter = x86_cpu_exec_enter;
     cc->cpu_exec_exit = x86_cpu_exec_exit;
+    cc->disas_set_info = x86_disas_set_info;
 
     dc->user_creatable = true;
 }
diff --git a/target/i386/translate.c b/target/i386/translate.c
index de0c989763..06c2cb9e64 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -8526,15 +8526,9 @@ static void i386_tr_disas_log(const DisasContextBase *dcbase,
                               CPUState *cpu)
 {
     DisasContext *dc = container_of(dcbase, DisasContext, base);
-    int disas_flags = !dc->code32;
 
     qemu_log("IN: %s\n", lookup_symbol(dc->base.pc_first));
-#ifdef TARGET_X86_64
-    if (dc->code64) {
-        disas_flags = 2;
-    }
-#endif
-    log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size, disas_flags);
+    log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size, 0);
 }
 
 static const TranslatorOps i386_tr_ops = {
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 02/10] target/ppc: Convert to disas_set_info hook
  2017-09-14 18:35 [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler Richard Henderson
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 01/10] target/i386: Convert to disas_set_info hook Richard Henderson
@ 2017-09-14 18:35 ` Richard Henderson
  2017-09-18 11:58   ` Alex Bennée
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 03/10] disas: Remove unused flags arguments Richard Henderson
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Richard Henderson @ 2017-09-14 18:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc

Cc: qemu-ppc@nongnu.org
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 disas.c                     | 33 ---------------------------------
 monitor.c                   |  5 -----
 target/ppc/translate.c      |  5 +----
 target/ppc/translate_init.c | 21 +++++++++++++++++++++
 4 files changed, 22 insertions(+), 42 deletions(-)

diff --git a/disas.c b/disas.c
index 2be716fdb2..3a375a3b6c 100644
--- a/disas.c
+++ b/disas.c
@@ -205,23 +205,6 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
         cc->disas_set_info(cpu, &s.info);
     }
 
-#if defined(TARGET_PPC)
-    if ((flags >> 16) & 1) {
-        s.info.endian = BFD_ENDIAN_LITTLE;
-    }
-    if (flags & 0xFFFF) {
-        /* If we have a precise definition of the instruction set, use it. */
-        s.info.mach = flags & 0xFFFF;
-    } else {
-#ifdef TARGET_PPC64
-        s.info.mach = bfd_mach_ppc64;
-#else
-        s.info.mach = bfd_mach_ppc;
-#endif
-    }
-    s.info.disassembler_options = (char *)"any";
-    s.info.print_insn = print_insn_ppc;
-#endif
     if (s.info.print_insn == NULL) {
         s.info.print_insn = print_insn_od_target;
     }
@@ -381,22 +364,6 @@ void monitor_disas(Monitor *mon, CPUState *cpu,
         cc->disas_set_info(cpu, &s.info);
     }
 
-#if defined(TARGET_PPC)
-    if (flags & 0xFFFF) {
-        /* If we have a precise definition of the instruction set, use it. */
-        s.info.mach = flags & 0xFFFF;
-    } else {
-#ifdef TARGET_PPC64
-        s.info.mach = bfd_mach_ppc64;
-#else
-        s.info.mach = bfd_mach_ppc;
-#endif
-    }
-    if ((flags >> 16) & 1) {
-        s.info.endian = BFD_ENDIAN_LITTLE;
-    }
-    s.info.print_insn = print_insn_ppc;
-#endif
     if (!s.info.print_insn) {
         monitor_printf(mon, "0x" TARGET_FMT_lx
                        ": Asm output not supported on this arch\n", pc);
diff --git a/monitor.c b/monitor.c
index 3f3ebc31ef..a0f43f27e7 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1310,11 +1310,6 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize,
 
     if (format == 'i') {
         int flags = 0;
-#ifdef TARGET_PPC
-        CPUArchState *env = mon_get_cpu_env();
-        flags = msr_le << 16;
-        flags |= env->bfd_mach;
-#endif
         monitor_disas(mon, cs, addr, count, is_physical, flags);
         return;
     }
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 606b605ba0..bc155f1036 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7395,12 +7395,9 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
 #if defined(DEBUG_DISAS)
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
         && qemu_log_in_addr_range(pc_start)) {
-        int flags;
-        flags = env->bfd_mach;
-        flags |= ctx.le_mode << 16;
         qemu_log_lock();
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(cs, pc_start, ctx.nip - pc_start, flags);
+        log_target_disas(cs, pc_start, ctx.nip - pc_start, 0);
         qemu_log("\n");
         qemu_log_unlock();
     }
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index c827d1e388..c7b4a7c02a 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -10644,6 +10644,26 @@ static gchar *ppc_gdb_arch_name(CPUState *cs)
 #endif
 }
 
+static void ppc_disas_set_info(CPUState *cs, disassemble_info *info)
+{
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+
+    if ((env->hflags >> MSR_LE) & 1) {
+        info->endian = BFD_ENDIAN_LITTLE;
+    }
+    info->mach = env->bfd_mach;
+    if (!env->bfd_mach) {
+#ifdef TARGET_PPC64
+        info->mach = bfd_mach_ppc64;
+#else
+        info->mach = bfd_mach_ppc;
+#endif
+    }
+    info->disassembler_options = (char *)"any";
+    info->print_insn = print_insn_ppc;
+}
+
 static Property ppc_cpu_properties[] = {
     DEFINE_PROP_BOOL("pre-2.8-migration", PowerPCCPU, pre_2_8_migration, false),
     DEFINE_PROP_BOOL("pre-2.10-migration", PowerPCCPU, pre_2_10_migration,
@@ -10705,6 +10725,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
 #ifndef CONFIG_USER_ONLY
     cc->virtio_is_big_endian = ppc_cpu_is_big_endian;
 #endif
+    cc->disas_set_info = ppc_disas_set_info;
 
     dc->fw_name = "PowerPC,UNKNOWN";
 }
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 03/10] disas: Remove unused flags arguments
  2017-09-14 18:35 [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler Richard Henderson
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 01/10] target/i386: Convert to disas_set_info hook Richard Henderson
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 02/10] target/ppc: " Richard Henderson
@ 2017-09-14 18:35 ` Richard Henderson
  2017-09-18 11:59   ` Alex Bennée
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 04/10] disas: Support the Capstone disassembler library Richard Henderson
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Richard Henderson @ 2017-09-14 18:35 UTC (permalink / raw)
  To: qemu-devel

Now that every target is using the disas_set_info hook,
the flags argument is unused.  Remove it.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/disas/disas.h         |  4 ++--
 include/exec/log.h            |  4 ++--
 disas.c                       | 15 ++++-----------
 monitor.c                     |  3 +--
 target/alpha/translate.c      |  2 +-
 target/arm/translate-a64.c    |  3 +--
 target/arm/translate.c        |  3 +--
 target/cris/translate.c       |  3 +--
 target/hppa/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/nios2/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 +-
 23 files changed, 28 insertions(+), 39 deletions(-)

diff --git a/include/disas/disas.h b/include/disas/disas.h
index e549ca24a1..4d48c13c65 100644
--- a/include/disas/disas.h
+++ b/include/disas/disas.h
@@ -9,10 +9,10 @@
 /* Disassemble this for me please... (debugging). */
 void disas(FILE *out, void *code, unsigned long size);
 void target_disas(FILE *out, CPUState *cpu, target_ulong code,
-                  target_ulong size, int flags);
+                  target_ulong size);
 
 void monitor_disas(Monitor *mon, CPUState *cpu,
-                   target_ulong pc, int nb_insn, int is_physical, int flags);
+                   target_ulong pc, int nb_insn, int is_physical);
 
 /* Look up symbol for debugging purpose.  Returns "" if unknown. */
 const char *lookup_symbol(target_ulong orig_addr);
diff --git a/include/exec/log.h b/include/exec/log.h
index ba1c9b5682..c249307911 100644
--- a/include/exec/log.h
+++ b/include/exec/log.h
@@ -38,9 +38,9 @@ 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(CPUState *cpu, target_ulong start,
-                                    target_ulong len, int flags)
+                                    target_ulong len)
 {
-    target_disas(qemu_logfile, cpu, start, len, flags);
+    target_disas(qemu_logfile, cpu, start, len);
 }
 
 static inline void log_disas(void *code, unsigned long size)
diff --git a/disas.c b/disas.c
index 3a375a3b6c..ad675dc361 100644
--- a/disas.c
+++ b/disas.c
@@ -171,15 +171,9 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info)
     return print_insn_objdump(pc, info, "OBJD-T");
 }
 
-/* Disassemble this for me please... (debugging). 'flags' has the following
-   values:
-    i386 - 1 means 16 bit code, 2 means 64 bit code
-    ppc  - bits 0:15 specify (optionally) the machine instruction set;
-           bit 16 indicates little endian.
-    other targets - unused
- */
+/* Disassemble this for me please... (debugging).  */
 void target_disas(FILE *out, CPUState *cpu, target_ulong code,
-                  target_ulong size, int flags)
+                  target_ulong size)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
     target_ulong pc;
@@ -336,10 +330,9 @@ monitor_read_memory (bfd_vma memaddr, bfd_byte *myaddr, int length,
     return 0;
 }
 
-/* Disassembler for the monitor.
-   See target_disas for a description of flags. */
+/* Disassembler for the monitor.  */
 void monitor_disas(Monitor *mon, CPUState *cpu,
-                   target_ulong pc, int nb_insn, int is_physical, int flags)
+                   target_ulong pc, int nb_insn, int is_physical)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
     int count, i;
diff --git a/monitor.c b/monitor.c
index a0f43f27e7..2125b54101 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1309,8 +1309,7 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize,
     }
 
     if (format == 'i') {
-        int flags = 0;
-        monitor_disas(mon, cs, addr, count, is_physical, flags);
+        monitor_disas(mon, cs, addr, count, is_physical);
         return;
     }
 
diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index 5a92c4accb..e9a245f9c5 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -3048,7 +3048,7 @@ static void alpha_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
 static void alpha_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu)
 {
     qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
-    log_target_disas(cpu, dcbase->pc_first, dcbase->tb->size, 1);
+    log_target_disas(cpu, dcbase->pc_first, dcbase->tb->size);
 }
 
 static const TranslatorOps alpha_tr_ops = {
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 9017e30510..a3984c9a0d 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -11409,8 +11409,7 @@ static void aarch64_tr_disas_log(const DisasContextBase *dcbase,
     DisasContext *dc = container_of(dcbase, DisasContext, base);
 
     qemu_log("IN: %s\n", lookup_symbol(dc->base.pc_first));
-    log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size,
-                     4 | (bswap_code(dc->sctlr_b) ? 2 : 0));
+    log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size);
 }
 
 const TranslatorOps aarch64_translator_ops = {
diff --git a/target/arm/translate.c b/target/arm/translate.c
index ab1a12a1b8..93e9dbe33d 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -12258,8 +12258,7 @@ static void arm_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu)
     DisasContext *dc = container_of(dcbase, DisasContext, base);
 
     qemu_log("IN: %s\n", lookup_symbol(dc->base.pc_first));
-    log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size,
-                     dc->thumb | (dc->sctlr_b << 1));
+    log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size);
 }
 
 static const TranslatorOps arm_translator_ops = {
diff --git a/target/cris/translate.c b/target/cris/translate.c
index 38a999e6f1..b1fda57c74 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -3297,8 +3297,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
         qemu_log_lock();
         qemu_log("--------------\n");
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(cs, pc_start, dc->pc - pc_start,
-                         env->pregs[PR_VR]);
+        log_target_disas(cs, pc_start, dc->pc - pc_start);
         qemu_log("\nisize=%d osize=%d\n",
                  dc->pc - pc_start, tcg_op_buf_count());
         qemu_log_unlock();
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index b6e2652341..fc2a9f5896 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -3904,7 +3904,7 @@ static void hppa_tr_disas_log(const DisasContextBase *dcbase, CPUState *cs)
         break;
     default:
         qemu_log("IN: %s\n", lookup_symbol(tb->pc));
-        log_target_disas(cs, tb->pc, tb->size, 1);
+        log_target_disas(cs, tb->pc, tb->size);
         break;
     }
 }
diff --git a/target/i386/translate.c b/target/i386/translate.c
index 06c2cb9e64..23d9e05426 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -8528,7 +8528,7 @@ static void i386_tr_disas_log(const DisasContextBase *dcbase,
     DisasContext *dc = container_of(dcbase, DisasContext, base);
 
     qemu_log("IN: %s\n", lookup_symbol(dc->base.pc_first));
-    log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size, 0);
+    log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size);
 }
 
 static const TranslatorOps i386_tr_ops = {
diff --git a/target/lm32/translate.c b/target/lm32/translate.c
index 65bc9c0bf6..a83cbdf729 100644
--- a/target/lm32/translate.c
+++ b/target/lm32/translate.c
@@ -1156,7 +1156,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
         && qemu_log_in_addr_range(pc_start)) {
         qemu_log_lock();
         qemu_log("\n");
-        log_target_disas(cs, pc_start, dc->pc - pc_start, 0);
+        log_target_disas(cs, pc_start, dc->pc - pc_start);
         qemu_log("\nisize=%d osize=%d\n",
                  dc->pc - pc_start, tcg_op_buf_count());
         qemu_log_unlock();
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index d738f32f9c..e1e31f622c 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -5620,7 +5620,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb)
         qemu_log_lock();
         qemu_log("----------------\n");
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(cs, pc_start, dc->pc - pc_start, 0);
+        log_target_disas(cs, pc_start, dc->pc - pc_start);
         qemu_log("\n");
         qemu_log_unlock();
     }
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 067b0878d6..fecc61a1ec 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -1810,7 +1810,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
         qemu_log_lock();
         qemu_log("--------------\n");
 #if DISAS_GNU
-        log_target_disas(cs, pc_start, dc->pc - pc_start, 0);
+        log_target_disas(cs, pc_start, dc->pc - pc_start);
 #endif
         qemu_log("\nisize=%d osize=%d\n",
                  dc->pc - pc_start, tcg_op_buf_count());
diff --git a/target/mips/translate.c b/target/mips/translate.c
index c78d27294c..3b9cf9a236 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -20369,7 +20369,7 @@ done_generating:
         && qemu_log_in_addr_range(pc_start)) {
         qemu_log_lock();
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(cs, pc_start, ctx.pc - pc_start, 0);
+        log_target_disas(cs, pc_start, ctx.pc - pc_start);
         qemu_log("\n");
         qemu_log_unlock();
     }
diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index 6b0961837d..7a0fa860da 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -907,7 +907,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb)
         && qemu_log_in_addr_range(tb->pc)) {
         qemu_log_lock();
         qemu_log("IN: %s\n", lookup_symbol(tb->pc));
-        log_target_disas(cs, tb->pc, dc->pc - tb->pc, 0);
+        log_target_disas(cs, tb->pc, dc->pc - tb->pc);
         qemu_log("\n");
         qemu_log_unlock();
     }
diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index 112db1ad0f..99f2b463ce 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -1653,7 +1653,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
 
     if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
         && qemu_log_in_addr_range(pc_start)) {
-        log_target_disas(cs, pc_start, tb->size, 0);
+        log_target_disas(cs, pc_start, tb->size);
         qemu_log("\n");
         qemu_log_unlock();
     }
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index bc155f1036..8e92e4579c 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7397,7 +7397,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
         && qemu_log_in_addr_range(pc_start)) {
         qemu_log_lock();
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(cs, pc_start, ctx.nip - pc_start, 0);
+        log_target_disas(cs, pc_start, ctx.nip - pc_start);
         qemu_log("\n");
         qemu_log_unlock();
     }
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 909b12818d..28bd02ceb3 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -5910,7 +5910,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
             qemu_log("IN: EXECUTE %016" PRIx64 "\n", dc.ex_value);
         } else {
             qemu_log("IN: %s\n", lookup_symbol(pc_start));
-            log_target_disas(cs, pc_start, dc.pc - pc_start, 1);
+            log_target_disas(cs, pc_start, dc.pc - pc_start);
             qemu_log("\n");
         }
         qemu_log_unlock();
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 10191073b2..7532bf74c1 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -2347,7 +2347,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
         && qemu_log_in_addr_range(pc_start)) {
         qemu_log_lock();
 	qemu_log("IN:\n");	/* , lookup_symbol(pc_start)); */
-        log_target_disas(cs, pc_start, ctx.pc - pc_start, 0);
+        log_target_disas(cs, pc_start, ctx.pc - pc_start);
 	qemu_log("\n");
         qemu_log_unlock();
     }
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 6290705b11..e89b6227f2 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -5855,7 +5855,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock * tb)
         qemu_log_lock();
         qemu_log("--------------\n");
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(cs, pc_start, last_pc + 4 - pc_start, 0);
+        log_target_disas(cs, pc_start, last_pc + 4 - pc_start);
         qemu_log("\n");
         qemu_log_unlock();
     }
diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index 4e4198e887..e807500e26 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -8839,7 +8839,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
         && qemu_log_in_addr_range(pc_start)) {
         qemu_log_lock();
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(cs, pc_start, ctx.pc - pc_start, 0);
+        log_target_disas(cs, pc_start, ctx.pc - pc_start);
         qemu_log("\n");
         qemu_log_unlock();
     }
diff --git a/target/unicore32/translate.c b/target/unicore32/translate.c
index 6c094d59d7..f9aa248a80 100644
--- a/target/unicore32/translate.c
+++ b/target/unicore32/translate.c
@@ -2031,7 +2031,7 @@ done_generating:
         qemu_log_lock();
         qemu_log("----------------\n");
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(cs, pc_start, dc->pc - pc_start, 0);
+        log_target_disas(cs, pc_start, dc->pc - pc_start);
         qemu_log("\n");
         qemu_log_unlock();
     }
diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index d7bf07e8e6..03719ce12b 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -3250,7 +3250,7 @@ done:
         qemu_log_lock();
         qemu_log("----------------\n");
         qemu_log("IN: %s\n", lookup_symbol(pc_start));
-        log_target_disas(cs, pc_start, dc.pc - pc_start, 0);
+        log_target_disas(cs, pc_start, dc.pc - pc_start);
         qemu_log("\n");
         qemu_log_unlock();
     }
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 04/10] disas: Support the Capstone disassembler library
  2017-09-14 18:35 [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler Richard Henderson
                   ` (2 preceding siblings ...)
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 03/10] disas: Remove unused flags arguments Richard Henderson
@ 2017-09-14 18:35 ` Richard Henderson
  2017-09-15  4:46   ` Philippe Mathieu-Daudé
                     ` (2 more replies)
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 05/10] target/i386: Support Capstone in disas_set_info Richard Henderson
                   ` (6 subsequent siblings)
  10 siblings, 3 replies; 23+ messages in thread
From: Richard Henderson @ 2017-09-14 18:35 UTC (permalink / raw)
  To: qemu-devel

If configured, prefer this over our rather dated copy of the
GPLv2-only binutils.  This will be especially apparent with
the proposed vector extensions to TCG, as disas/i386.c does
not handle AVX.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/disas/bfd.h      |  4 ++
 include/disas/capstone.h | 38 +++++++++++++++++++
 disas.c                  | 99 ++++++++++++++++++++++++++++++++++++++++++------
 configure                | 17 +++++++++
 4 files changed, 146 insertions(+), 12 deletions(-)
 create mode 100644 include/disas/capstone.h

diff --git a/include/disas/bfd.h b/include/disas/bfd.h
index b01e002b4c..0f4ecdeb88 100644
--- a/include/disas/bfd.h
+++ b/include/disas/bfd.h
@@ -377,6 +377,10 @@ typedef struct disassemble_info {
   /* Command line options specific to the target disassembler.  */
   char * disassembler_options;
 
+  /* Options for Capstone disassembly.  */
+  int cap_arch;
+  int cap_mode;
+
 } disassemble_info;
 
 \f

diff --git a/include/disas/capstone.h b/include/disas/capstone.h
new file mode 100644
index 0000000000..84e214956d
--- /dev/null
+++ b/include/disas/capstone.h
@@ -0,0 +1,38 @@
+#ifndef QEMU_CAPSTONE_H
+#define QEMU_CAPSTONE_H 1
+
+#ifdef CONFIG_CAPSTONE
+
+#include <capstone.h>
+
+#else
+
+/* Just enough to allow backends to init without ifdefs.  */
+
+#define CS_ARCH_ARM     -1
+#define CS_ARCH_ARM64   -1
+#define CS_ARCH_MIPS    -1
+#define CS_ARCH_X86     -1
+#define CS_ARCH_PPC     -1
+#define CS_ARCH_SPARC   -1
+#define CS_ARCH_SYSZ    -1
+
+#define CS_MODE_LITTLE_ENDIAN    0
+#define CS_MODE_BIG_ENDIAN       0
+#define CS_MODE_ARM              0
+#define CS_MODE_16               0
+#define CS_MODE_32               0
+#define CS_MODE_64               0
+#define CS_MODE_THUMB            0
+#define CS_MODE_MCLASS           0
+#define CS_MODE_V8               0
+#define CS_MODE_MICRO            0
+#define CS_MODE_MIPS3            0
+#define CS_MODE_MIPS32R6         0
+#define CS_MODE_MIPSGP64         0
+#define CS_MODE_V9               0
+#define CS_MODE_MIPS32           0
+#define CS_MODE_MIPS64           0
+
+#endif /* CONFIG_CAPSTONE */
+#endif /* QEMU_CAPSTONE_H */
diff --git a/disas.c b/disas.c
index ad675dc361..76ea76b026 100644
--- a/disas.c
+++ b/disas.c
@@ -6,6 +6,7 @@
 
 #include "cpu.h"
 #include "disas/disas.h"
+#include "disas/capstone.h"
 
 typedef struct CPUDebug {
     struct disassemble_info info;
@@ -171,6 +172,57 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info)
     return print_insn_objdump(pc, info, "OBJD-T");
 }
 
+static bool cap_disas(disassemble_info *info, uint64_t pc, size_t size)
+{
+    bool ret = false;
+#ifdef CONFIG_CAPSTONE
+    csh handle;
+    cs_insn *insn;
+    uint8_t *buf;
+    const uint8_t *cbuf;
+    uint64_t pc_start;
+    cs_mode cap_mode = info->cap_mode;
+
+    cap_mode += (info->endian == BFD_ENDIAN_BIG ? CS_MODE_BIG_ENDIAN
+                 : CS_MODE_LITTLE_ENDIAN);
+
+    if (cs_open(info->cap_arch, cap_mode, &handle) != CS_ERR_OK) {
+        goto err0;
+    }
+
+    /* ??? There probably ought to be a better place to put this.  */
+    if (info->cap_arch == CS_ARCH_X86) {
+        /* We don't care about errors (if for some reason the library
+           is compiled without AT&T syntax); the user will just have
+           to deal with the Intel syntax.  */
+        cs_option(handle, CS_OPT_SYNTAX, CS_OPT_SYNTAX_ATT);
+    }
+
+    insn = cs_malloc(handle);
+    if (insn == NULL) {
+        goto err1;
+    }
+
+    cbuf = buf = g_malloc(size);
+    info->read_memory_func(pc, buf, size, info);
+
+    pc_start = pc;
+    while (cs_disasm_iter(handle, &cbuf, &size, &pc, insn)) {
+        (*info->fprintf_func)(info->stream,
+                              "0x%08" PRIx64 ":  %-12s %s\n",
+                              pc_start, insn->mnemonic, insn->op_str);
+        pc_start = pc;
+    }
+    ret = true;
+
+    g_free(buf);
+ err1:
+    cs_close(&handle);
+ err0:
+#endif /* CONFIG_CAPSTONE */
+    return ret;
+}
+
 /* Disassemble this for me please... (debugging).  */
 void target_disas(FILE *out, CPUState *cpu, target_ulong code,
                   target_ulong size)
@@ -188,6 +240,8 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
     s.info.buffer_vma = code;
     s.info.buffer_length = size;
     s.info.print_address_func = generic_print_address;
+    s.info.cap_arch = -1;
+    s.info.cap_mode = 0;
 
 #ifdef TARGET_WORDS_BIGENDIAN
     s.info.endian = BFD_ENDIAN_BIG;
@@ -199,6 +253,10 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
         cc->disas_set_info(cpu, &s.info);
     }
 
+    if (s.info.cap_arch >= 0 && cap_disas(&s.info, code, size)) {
+        return;
+    }
+
     if (s.info.print_insn == NULL) {
         s.info.print_insn = print_insn_od_target;
     }
@@ -206,18 +264,6 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
     for (pc = code; size > 0; pc += count, size -= count) {
 	fprintf(out, "0x" TARGET_FMT_lx ":  ", pc);
 	count = s.info.print_insn(pc, &s.info);
-#if 0
-        {
-            int i;
-            uint8_t b;
-            fprintf(out, " {");
-            for(i = 0; i < count; i++) {
-                target_read_memory(pc + i, &b, 1, &s.info);
-                fprintf(out, " %02x", b);
-            }
-            fprintf(out, " }");
-        }
-#endif
 	fprintf(out, "\n");
 	if (count < 0)
 	    break;
@@ -245,6 +291,8 @@ void disas(FILE *out, void *code, unsigned long size)
     s.info.buffer = code;
     s.info.buffer_vma = (uintptr_t)code;
     s.info.buffer_length = size;
+    s.info.cap_arch = -1;
+    s.info.cap_mode = 0;
 
 #ifdef HOST_WORDS_BIGENDIAN
     s.info.endian = BFD_ENDIAN_BIG;
@@ -256,21 +304,34 @@ void disas(FILE *out, void *code, unsigned long size)
 #elif defined(__i386__)
     s.info.mach = bfd_mach_i386_i386;
     print_insn = print_insn_i386;
+    s.info.cap_arch = CS_ARCH_X86;
+    s.info.cap_mode = CS_MODE_32;
 #elif defined(__x86_64__)
     s.info.mach = bfd_mach_x86_64;
     print_insn = print_insn_i386;
+    s.info.cap_arch = CS_ARCH_X86;
+    s.info.cap_mode = CS_MODE_64;
 #elif defined(_ARCH_PPC)
     s.info.disassembler_options = (char *)"any";
     print_insn = print_insn_ppc;
+    s.info.cap_arch = CS_ARCH_PPC;
+# ifdef _ARCH_PPC64
+    s.info.cap_mode = CS_MODE_64;
+# endif
 #elif defined(__aarch64__) && defined(CONFIG_ARM_A64_DIS)
     print_insn = print_insn_arm_a64;
+    s.info.cap_arch = CS_ARCH_ARM64;
 #elif defined(__alpha__)
     print_insn = print_insn_alpha;
 #elif defined(__sparc__)
     print_insn = print_insn_sparc;
     s.info.mach = bfd_mach_sparc_v9b;
+    s.info.cap_arch = CS_ARCH_SPARC;
+    s.info.cap_mode = CS_MODE_V9;
 #elif defined(__arm__)
     print_insn = print_insn_arm;
+    s.info.cap_arch = CS_ARCH_ARM;
+    /* TCG only generates code for arm mode.  */
 #elif defined(__MIPSEB__)
     print_insn = print_insn_big_mips;
 #elif defined(__MIPSEL__)
@@ -279,9 +340,15 @@ void disas(FILE *out, void *code, unsigned long size)
     print_insn = print_insn_m68k;
 #elif defined(__s390__)
     print_insn = print_insn_s390;
+    s.info.cap_arch = CS_ARCH_SYSZ;
 #elif defined(__hppa__)
     print_insn = print_insn_hppa;
 #endif
+
+    if (s.info.cap_arch >= 0 && cap_disas(&s.info, (uintptr_t)code, size)) {
+        return;
+    }
+
     if (print_insn == NULL) {
         print_insn = print_insn_od_host;
     }
@@ -357,6 +424,14 @@ void monitor_disas(Monitor *mon, CPUState *cpu,
         cc->disas_set_info(cpu, &s.info);
     }
 
+    /* ??? Capstone requires that we copy the data into a host-addressable
+       buffer first and has no call-back to read more.  Therefore we need
+       an estimate of buffer size.  This will work for most RISC, but we'll
+       need to figure out something else for variable-length ISAs.  */
+    if (s.info.cap_arch >= 0 && cap_disas(&s.info, pc, 4 * nb_insn)) {
+        return;
+    }
+
     if (!s.info.print_insn) {
         monitor_printf(mon, "0x" TARGET_FMT_lx
                        ": Asm output not supported on this arch\n", pc);
diff --git a/configure b/configure
index fd7e3a5e81..a55a8eda8a 100755
--- a/configure
+++ b/configure
@@ -366,6 +366,7 @@ opengl_dmabuf="no"
 cpuid_h="no"
 avx2_opt="no"
 zlib="yes"
+capstone=""
 lzo=""
 snappy=""
 bzip2=""
@@ -4381,6 +4382,18 @@ EOF
 fi
 
 ##########################################
+# capstone
+
+if test "$capstone" != no; then
+  capstone=no
+  if $pkg_config capstone; then
+    capstone=yes
+    QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
+    LDFLAGS="$LDFLAGS $($pkg_config --libs capstone)"
+  fi
+fi
+
+##########################################
 # check if we have fdatasync
 
 fdatasync=no
@@ -5402,6 +5415,7 @@ echo "jemalloc support  $jemalloc"
 echo "avx2 optimization $avx2_opt"
 echo "replication support $replication"
 echo "VxHS block device $vxhs"
+echo "capstone          $capstone"
 
 if test "$sdl_too_old" = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -6050,6 +6064,9 @@ fi
 if test "$ivshmem" = "yes" ; then
   echo "CONFIG_IVSHMEM=y" >> $config_host_mak
 fi
+if test "$capstone" = "yes" ; then
+  echo "CONFIG_CAPSTONE=y" >> $config_host_mak
+fi
 
 # Hold two types of flag:
 #   CONFIG_THREAD_SETNAME_BYTHREAD  - we've got a way of setting the name on
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 05/10] target/i386: Support Capstone in disas_set_info
  2017-09-14 18:35 [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler Richard Henderson
                   ` (3 preceding siblings ...)
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 04/10] disas: Support the Capstone disassembler library Richard Henderson
@ 2017-09-14 18:35 ` Richard Henderson
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 06/10] target/arm: " Richard Henderson
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Richard Henderson @ 2017-09-14 18:35 UTC (permalink / raw)
  To: qemu-devel

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/i386/cpu.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b869a69c53..c3980b3864 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -51,6 +51,8 @@
 #include "hw/i386/apic_internal.h"
 #endif
 
+#include "disas/capstone.h"
+
 
 /* Cache topology CPUID constants: */
 
@@ -4108,6 +4110,11 @@ static void x86_disas_set_info(CPUState *cs, disassemble_info *info)
                   : env->hflags & HF_CS32_MASK ? bfd_mach_i386_i386
                   : bfd_mach_i386_i8086);
     info->print_insn = print_insn_i386;
+
+    info->cap_arch = CS_ARCH_X86;
+    info->cap_mode = (env->hflags & HF_CS64_MASK ? CS_MODE_64
+                      : env->hflags & HF_CS32_MASK ? CS_MODE_32
+                      : CS_MODE_16);
 }
 
 static Property x86_cpu_properties[] = {
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 06/10] target/arm: Support Capstone in disas_set_info
  2017-09-14 18:35 [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler Richard Henderson
                   ` (4 preceding siblings ...)
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 05/10] target/i386: Support Capstone in disas_set_info Richard Henderson
@ 2017-09-14 18:35 ` Richard Henderson
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 07/10] target/ppc: " Richard Henderson
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Richard Henderson @ 2017-09-14 18:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-arm

Cc: qemu-arm@nongnu.org
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index a1acce3c7a..92159ca0b1 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -33,6 +33,7 @@
 #include "sysemu/sysemu.h"
 #include "sysemu/hw_accel.h"
 #include "kvm_arm.h"
+#include "disas/capstone.h"
 
 static void arm_cpu_set_pc(CPUState *cs, vaddr value)
 {
@@ -476,10 +477,24 @@ static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
 #if defined(CONFIG_ARM_A64_DIS)
         info->print_insn = print_insn_arm_a64;
 #endif
-    } else if (env->thumb) {
-        info->print_insn = print_insn_thumb1;
+        info->cap_arch = CS_ARCH_ARM64;
     } else {
-        info->print_insn = print_insn_arm;
+        int cap_mode;
+        if (env->thumb) {
+            info->print_insn = print_insn_thumb1;
+            cap_mode = CS_MODE_THUMB;
+        } else {
+            info->print_insn = print_insn_arm;
+            cap_mode = CS_MODE_ARM;
+        }
+        if (arm_feature(env, ARM_FEATURE_V8)) {
+            cap_mode |= CS_MODE_V8;
+        }
+        if (arm_feature(env, ARM_FEATURE_M)) {
+            cap_mode |= CS_MODE_MCLASS;
+        }
+        info->cap_arch = CS_ARCH_ARM;
+        info->cap_mode = cap_mode;
     }
     if (bswap_code(arm_sctlr_b(env))) {
 #ifdef TARGET_WORDS_BIGENDIAN
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 07/10] target/ppc: Support Capstone in disas_set_info
  2017-09-14 18:35 [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler Richard Henderson
                   ` (5 preceding siblings ...)
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 06/10] target/arm: " Richard Henderson
@ 2017-09-14 18:35 ` Richard Henderson
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 08/10] target/s390x: " Richard Henderson
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Richard Henderson @ 2017-09-14 18:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc

Cc: qemu-ppc@nongnu.org
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/translate_init.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index c7b4a7c02a..b976784d21 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -35,6 +35,7 @@
 #include "mmu-book3s-v3.h"
 #include "sysemu/qtest.h"
 #include "qemu/cutils.h"
+#include "disas/capstone.h"
 
 //#define PPC_DUMP_CPU
 //#define PPC_DEBUG_SPR
@@ -10662,6 +10663,11 @@ static void ppc_disas_set_info(CPUState *cs, disassemble_info *info)
     }
     info->disassembler_options = (char *)"any";
     info->print_insn = print_insn_ppc;
+
+    info->cap_arch = CS_ARCH_PPC;
+#ifdef TARGET_PPC64
+    info->cap_mode = CS_MODE_64;
+#endif
 }
 
 static Property ppc_cpu_properties[] = {
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 08/10] target/s390x: Support Capstone in disas_set_info
  2017-09-14 18:35 [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler Richard Henderson
                   ` (6 preceding siblings ...)
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 07/10] target/ppc: " Richard Henderson
@ 2017-09-14 18:35 ` Richard Henderson
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 09/10] target/sparc: " Richard Henderson
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Richard Henderson @ 2017-09-14 18:35 UTC (permalink / raw)
  To: qemu-devel

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/cpu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 74b3e4fd0d..a4f7ff3da8 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -42,6 +42,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/s390x/sclp.h"
 #endif
+#include "disas/capstone.h"
 
 #define CR0_RESET       0xE0UL
 #define CR14_RESET      0xC2000000UL;
@@ -172,6 +173,7 @@ static void s390_cpu_disas_set_info(CPUState *cpu, disassemble_info *info)
 {
     info->mach = bfd_mach_s390_64;
     info->print_insn = print_insn_s390;
+    info->cap_arch = CS_ARCH_SYSZ;
 }
 
 static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 09/10] target/sparc: Support Capstone in disas_set_info
  2017-09-14 18:35 [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler Richard Henderson
                   ` (7 preceding siblings ...)
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 08/10] target/s390x: " Richard Henderson
@ 2017-09-14 18:35 ` Richard Henderson
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 10/10] target/mips: " Richard Henderson
  2017-09-15  4:53 ` [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler Philippe Mathieu-Daudé
  10 siblings, 0 replies; 23+ messages in thread
From: Richard Henderson @ 2017-09-14 18:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark Cave-Ayland

Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/sparc/cpu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 0806d699e6..7eabf410de 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -24,6 +24,7 @@
 #include "exec/exec-all.h"
 #include "hw/qdev-properties.h"
 #include "qapi/visitor.h"
+#include "disas/capstone.h"
 
 //#define DEBUG_FEATURES
 
@@ -99,8 +100,10 @@ static bool sparc_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
 static void cpu_sparc_disas_set_info(CPUState *cpu, disassemble_info *info)
 {
     info->print_insn = print_insn_sparc;
+    info->cap_arch = CS_ARCH_SPARC;
 #ifdef TARGET_SPARC64
     info->mach = bfd_mach_sparc_v9b;
+    info->cap_mode = CS_MODE_V9;
 #endif
 }
 
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 10/10] target/mips: Support Capstone in disas_set_info
  2017-09-14 18:35 [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler Richard Henderson
                   ` (8 preceding siblings ...)
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 09/10] target/sparc: " Richard Henderson
@ 2017-09-14 18:35 ` Richard Henderson
  2017-09-15  2:47   ` Philippe Mathieu-Daudé
  2017-09-15  4:53 ` [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler Philippe Mathieu-Daudé
  10 siblings, 1 reply; 23+ messages in thread
From: Richard Henderson @ 2017-09-14 18:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: Aurelien Jarno, Yongbok Kim

Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Yongbok Kim <yongbok.kim@imgtec.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/mips/cpu.h            |  2 ++
 target/mips/cpu.c            |  8 --------
 target/mips/translate_init.c | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 74f6a5b098..dca713825d 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -1118,4 +1118,6 @@ static inline void QEMU_NORETURN do_raise_exception(CPUMIPSState *env,
     do_raise_exception_err(env, exception, 0, pc);
 }
 
+void mips_cpu_disas_set_info(CPUState *s, disassemble_info *info);
+
 #endif /* MIPS_CPU_H */
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 1bb66b7a5a..898f1b3759 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -111,14 +111,6 @@ static void mips_cpu_reset(CPUState *s)
 #endif
 }
 
-static void mips_cpu_disas_set_info(CPUState *s, disassemble_info *info) {
-#ifdef TARGET_WORDS_BIGENDIAN
-    info->print_insn = print_insn_big_mips;
-#else
-    info->print_insn = print_insn_little_mips;
-#endif
-}
-
 static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
diff --git a/target/mips/translate_init.c b/target/mips/translate_init.c
index 255d25bacd..1d43b3c36d 100644
--- a/target/mips/translate_init.c
+++ b/target/mips/translate_init.c
@@ -947,3 +947,39 @@ static void msa_reset(CPUMIPSState *env)
     /* set proper signanling bit meaning ("1" means "quiet") */
     set_snan_bit_is_one(0, &env->active_tc.msa_fp_status);
 }
+
+#include "disas/capstone.h"
+
+void mips_cpu_disas_set_info(CPUState *s, disassemble_info *info)
+{
+    MIPSCPU *cpu = MIPS_CPU(s);
+    CPUMIPSState *env = &cpu->env;
+    int insn_flags = env->cpu_model->insn_flags;
+    int cap_mode;
+
+#ifdef TARGET_WORDS_BIGENDIAN
+    info->print_insn = print_insn_big_mips;
+#else
+    info->print_insn = print_insn_little_mips;
+#endif
+
+    cap_mode = 0;
+    if (insn_flags & ISA_MIPS3) {
+        cap_mode |= CS_MODE_MIPS3;
+    }
+    if (insn_flags & ISA_MIPS32) {
+        cap_mode |= CS_MODE_MIPS32;
+    }
+    if (insn_flags & ISA_MIPS64) {
+        cap_mode |= CS_MODE_MIPS64;
+    }
+    if (insn_flags & ISA_MIPS32R6) {
+        cap_mode |= CS_MODE_MIPS32R6;
+    }
+#ifdef TARGET_MIPS64
+    cap_mode |= CS_MODE_MIPSGP64;
+#endif
+
+    info->cap_arch = CS_ARCH_MIPS;
+    info->cap_mode = cap_mode;
+}
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 10/10] target/mips: Support Capstone in disas_set_info
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 10/10] target/mips: " Richard Henderson
@ 2017-09-15  2:47   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-09-15  2:47 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: Yongbok Kim, Aurelien Jarno

On 09/14/2017 03:35 PM, Richard Henderson wrote:
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yongbok Kim <yongbok.kim@imgtec.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/mips/cpu.h            |  2 ++
>   target/mips/cpu.c            |  8 --------
>   target/mips/translate_init.c | 36 ++++++++++++++++++++++++++++++++++++
>   3 files changed, 38 insertions(+), 8 deletions(-)
> 
> diff --git a/target/mips/cpu.h b/target/mips/cpu.h
> index 74f6a5b098..dca713825d 100644
> --- a/target/mips/cpu.h
> +++ b/target/mips/cpu.h
> @@ -1118,4 +1118,6 @@ static inline void QEMU_NORETURN do_raise_exception(CPUMIPSState *env,
>       do_raise_exception_err(env, exception, 0, pc);
>   }
>   
> +void mips_cpu_disas_set_info(CPUState *s, disassemble_info *info);
> +
>   #endif /* MIPS_CPU_H */
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index 1bb66b7a5a..898f1b3759 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -111,14 +111,6 @@ static void mips_cpu_reset(CPUState *s)
>   #endif
>   }
>   
> -static void mips_cpu_disas_set_info(CPUState *s, disassemble_info *info) {
> -#ifdef TARGET_WORDS_BIGENDIAN
> -    info->print_insn = print_insn_big_mips;
> -#else
> -    info->print_insn = print_insn_little_mips;
> -#endif
> -}
> -

this clashes with the pending mips-cpu-qomify series, however the 
conflict is benign and easy fixable, I expect your series to enter first.

>   static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
>   {
>       CPUState *cs = CPU(dev);
> diff --git a/target/mips/translate_init.c b/target/mips/translate_init.c
> index 255d25bacd..1d43b3c36d 100644
> --- a/target/mips/translate_init.c
> +++ b/target/mips/translate_init.c
> @@ -947,3 +947,39 @@ static void msa_reset(CPUMIPSState *env)
>       /* set proper signanling bit meaning ("1" means "quiet") */
>       set_snan_bit_is_one(0, &env->active_tc.msa_fp_status);
>   }
> +
> +#include "disas/capstone.h"
> +
> +void mips_cpu_disas_set_info(CPUState *s, disassemble_info *info)
> +{
> +    MIPSCPU *cpu = MIPS_CPU(s);
> +    CPUMIPSState *env = &cpu->env;
> +    int insn_flags = env->cpu_model->insn_flags;
> +    int cap_mode;

int cap_mode = 0; ?

> +
> +#ifdef TARGET_WORDS_BIGENDIAN
> +    info->print_insn = print_insn_big_mips;
> +#else
> +    info->print_insn = print_insn_little_mips;
> +#endif
> +
> +    cap_mode = 0;
> +    if (insn_flags & ISA_MIPS3) {
> +        cap_mode |= CS_MODE_MIPS3;
> +    }
> +    if (insn_flags & ISA_MIPS32) {
> +        cap_mode |= CS_MODE_MIPS32;
> +    }
> +    if (insn_flags & ISA_MIPS64) {
> +        cap_mode |= CS_MODE_MIPS64;
> +    }
> +    if (insn_flags & ISA_MIPS32R6) {
> +        cap_mode |= CS_MODE_MIPS32R6;
> +    }

quite an improvement for the MIPS target!

> +#ifdef TARGET_MIPS64
> +    cap_mode |= CS_MODE_MIPSGP64;
> +#endif
> +
> +    info->cap_arch = CS_ARCH_MIPS;
> +    info->cap_mode = cap_mode;
> +}
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 04/10] disas: Support the Capstone disassembler library
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 04/10] disas: Support the Capstone disassembler library Richard Henderson
@ 2017-09-15  4:46   ` Philippe Mathieu-Daudé
  2017-09-15 16:58     ` Richard Henderson
  2017-09-16 18:32   ` Peter Maydell
  2017-09-16 18:52   ` Peter Maydell
  2 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-09-15  4:46 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel

Hi Richard,

see inlined comments.

On 09/14/2017 03:35 PM, Richard Henderson wrote:
> If configured, prefer this over our rather dated copy of the
> GPLv2-only binutils.  This will be especially apparent with
> the proposed vector extensions to TCG, as disas/i386.c does
> not handle AVX.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/disas/bfd.h      |  4 ++
>   include/disas/capstone.h | 38 +++++++++++++++++++
>   disas.c                  | 99 ++++++++++++++++++++++++++++++++++++++++++------
>   configure                | 17 +++++++++
>   4 files changed, 146 insertions(+), 12 deletions(-)
>   create mode 100644 include/disas/capstone.h
> 
> diff --git a/include/disas/bfd.h b/include/disas/bfd.h
> index b01e002b4c..0f4ecdeb88 100644
> --- a/include/disas/bfd.h
> +++ b/include/disas/bfd.h
> @@ -377,6 +377,10 @@ typedef struct disassemble_info {
>     /* Command line options specific to the target disassembler.  */
>     char * disassembler_options;
>   
> +  /* Options for Capstone disassembly.  */
> +  int cap_arch;
> +  int cap_mode;
> +
>   } disassemble_info;
>   
>   \f

> diff --git a/include/disas/capstone.h b/include/disas/capstone.h
> new file mode 100644
> index 0000000000..84e214956d
> --- /dev/null
> +++ b/include/disas/capstone.h
> @@ -0,0 +1,38 @@
> +#ifndef QEMU_CAPSTONE_H
> +#define QEMU_CAPSTONE_H 1
> +
> +#ifdef CONFIG_CAPSTONE
> +
> +#include <capstone.h>
> +
> +#else
> +
> +/* Just enough to allow backends to init without ifdefs.  */
> +
> +#define CS_ARCH_ARM     -1
> +#define CS_ARCH_ARM64   -1
> +#define CS_ARCH_MIPS    -1
> +#define CS_ARCH_X86     -1
> +#define CS_ARCH_PPC     -1
> +#define CS_ARCH_SPARC   -1
> +#define CS_ARCH_SYSZ    -1
> +
> +#define CS_MODE_LITTLE_ENDIAN    0
> +#define CS_MODE_BIG_ENDIAN       0
> +#define CS_MODE_ARM              0
> +#define CS_MODE_16               0
> +#define CS_MODE_32               0
> +#define CS_MODE_64               0
> +#define CS_MODE_THUMB            0
> +#define CS_MODE_MCLASS           0
> +#define CS_MODE_V8               0
> +#define CS_MODE_MICRO            0
> +#define CS_MODE_MIPS3            0
> +#define CS_MODE_MIPS32R6         0
> +#define CS_MODE_MIPSGP64         0
> +#define CS_MODE_V9               0
> +#define CS_MODE_MIPS32           0
> +#define CS_MODE_MIPS64           0
> +
> +#endif /* CONFIG_CAPSTONE */
> +#endif /* QEMU_CAPSTONE_H */
> diff --git a/disas.c b/disas.c
> index ad675dc361..76ea76b026 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -6,6 +6,7 @@
>   
>   #include "cpu.h"
>   #include "disas/disas.h"
> +#include "disas/capstone.h"
>   
>   typedef struct CPUDebug {
>       struct disassemble_info info;
> @@ -171,6 +172,57 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info)
>       return print_insn_objdump(pc, info, "OBJD-T");
>   }
>   
> +static bool cap_disas(disassemble_info *info, uint64_t pc, size_t size)

I'd rather use:

..,, target_ulong code, ...
> +{

uint64_t pc = (uint64_t)code;

> +    bool ret = false;

Isn't it cleaner to have a stubs/disas_capstone.c?

> +#ifdef CONFIG_CAPSTONE

this check here once:

     if (info->cap_arch < 0) {
         return false;
     }

> +    csh handle;

     cs_err err;

> +    cs_insn *insn;
> +    uint8_t *buf;
> +    const uint8_t *cbuf;
> +    uint64_t pc_start;
> +    cs_mode cap_mode = info->cap_mode;
> +
> +    cap_mode += (info->endian == BFD_ENDIAN_BIG ? CS_MODE_BIG_ENDIAN
> +                 : CS_MODE_LITTLE_ENDIAN);
> +

     assert(size); ?

> +    if (cs_open(info->cap_arch, cap_mode, &handle) != CS_ERR_OK) {

     err = cs_open(info->cap_arch, cap_mode, &handle);
     if (err != CS_ERR_OK) {
         (*info->fprintf_func)(info->stream, "Capstone: %s\n",
                               cs_strerror(err));

> +        goto err0;
> +    }
> +
> +    /* ??? There probably ought to be a better place to put this.  */

looks fine.

> +    if (info->cap_arch == CS_ARCH_X86) {
> +        /* We don't care about errors (if for some reason the library
> +           is compiled without AT&T syntax); the user will just have
> +           to deal with the Intel syntax.  */
> +        cs_option(handle, CS_OPT_SYNTAX, CS_OPT_SYNTAX_ATT);
> +    }
> +
> +    insn = cs_malloc(handle);
> +    if (insn == NULL) {
> +        goto err1;
> +    }
> +
> +    cbuf = buf = g_malloc(size);

     if (buf == NULL) {
         goto err2;
     }

> +    info->read_memory_func(pc, buf, size, info);
> +
> +    pc_start = pc;
> +    while (cs_disasm_iter(handle, &cbuf, &size, &pc, insn)) {
> +        (*info->fprintf_func)(info->stream,
> +                              "0x%08" PRIx64 ":  %-12s %s\n",
> +                              pc_start, insn->mnemonic, insn->op_str);
> +        pc_start = pc;
> +    }
> +    ret = true;
> +

     cs_free(insn, 1);
err2:

> +    g_free(buf);
> + err1:
> +    cs_close(&handle);
> + err0:
> +#endif /* CONFIG_CAPSTONE */
> +    return ret;
> +}
> +
>   /* Disassemble this for me please... (debugging).  */
>   void target_disas(FILE *out, CPUState *cpu, target_ulong code,
>                     target_ulong size)
> @@ -188,6 +240,8 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
>       s.info.buffer_vma = code;
>       s.info.buffer_length = size;
>       s.info.print_address_func = generic_print_address;
> +    s.info.cap_arch = -1;
> +    s.info.cap_mode = 0;
>   
>   #ifdef TARGET_WORDS_BIGENDIAN
>       s.info.endian = BFD_ENDIAN_BIG;
> @@ -199,6 +253,10 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
>           cc->disas_set_info(cpu, &s.info);
>       }
>   
> +    if (s.info.cap_arch >= 0 && cap_disas(&s.info, code, size)) {
> +        return;
> +    }
> +
>       if (s.info.print_insn == NULL) {
>           s.info.print_insn = print_insn_od_target;
>       }
> @@ -206,18 +264,6 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
>       for (pc = code; size > 0; pc += count, size -= count) {
>   	fprintf(out, "0x" TARGET_FMT_lx ":  ", pc);
>   	count = s.info.print_insn(pc, &s.info);
> -#if 0
> -        {
> -            int i;
> -            uint8_t b;
> -            fprintf(out, " {");
> -            for(i = 0; i < count; i++) {
> -                target_read_memory(pc + i, &b, 1, &s.info);
> -                fprintf(out, " %02x", b);
> -            }
> -            fprintf(out, " }");
> -        }
> -#endif
>   	fprintf(out, "\n");
>   	if (count < 0)
>   	    break;
> @@ -245,6 +291,8 @@ void disas(FILE *out, void *code, unsigned long size)
>       s.info.buffer = code;
>       s.info.buffer_vma = (uintptr_t)code;
>       s.info.buffer_length = size;
> +    s.info.cap_arch = -1;
> +    s.info.cap_mode = 0;
>   
>   #ifdef HOST_WORDS_BIGENDIAN
>       s.info.endian = BFD_ENDIAN_BIG;
> @@ -256,21 +304,34 @@ void disas(FILE *out, void *code, unsigned long size)
>   #elif defined(__i386__)
>       s.info.mach = bfd_mach_i386_i386;
>       print_insn = print_insn_i386;
> +    s.info.cap_arch = CS_ARCH_X86;
> +    s.info.cap_mode = CS_MODE_32;
>   #elif defined(__x86_64__)
>       s.info.mach = bfd_mach_x86_64;
>       print_insn = print_insn_i386;
> +    s.info.cap_arch = CS_ARCH_X86;
> +    s.info.cap_mode = CS_MODE_64;
>   #elif defined(_ARCH_PPC)
>       s.info.disassembler_options = (char *)"any";
>       print_insn = print_insn_ppc;
> +    s.info.cap_arch = CS_ARCH_PPC;
> +# ifdef _ARCH_PPC64
> +    s.info.cap_mode = CS_MODE_64;
> +# endif
>   #elif defined(__aarch64__) && defined(CONFIG_ARM_A64_DIS)
>       print_insn = print_insn_arm_a64;
> +    s.info.cap_arch = CS_ARCH_ARM64;
>   #elif defined(__alpha__)
>       print_insn = print_insn_alpha;
>   #elif defined(__sparc__)
>       print_insn = print_insn_sparc;
>       s.info.mach = bfd_mach_sparc_v9b;
> +    s.info.cap_arch = CS_ARCH_SPARC;
> +    s.info.cap_mode = CS_MODE_V9;
>   #elif defined(__arm__)
>       print_insn = print_insn_arm;
> +    s.info.cap_arch = CS_ARCH_ARM;
> +    /* TCG only generates code for arm mode.  */
>   #elif defined(__MIPSEB__)
>       print_insn = print_insn_big_mips;
>   #elif defined(__MIPSEL__)
> @@ -279,9 +340,15 @@ void disas(FILE *out, void *code, unsigned long size)
>       print_insn = print_insn_m68k;
>   #elif defined(__s390__)
>       print_insn = print_insn_s390;
> +    s.info.cap_arch = CS_ARCH_SYSZ;
>   #elif defined(__hppa__)
>       print_insn = print_insn_hppa;
>   #endif
> +
> +    if (s.info.cap_arch >= 0 && cap_disas(&s.info, (uintptr_t)code, size)) {

(target_ulong)(uintptr_t)code, ?

> +        return;
> +    }
> +
>       if (print_insn == NULL) {
>           print_insn = print_insn_od_host;
>       }
> @@ -357,6 +424,14 @@ void monitor_disas(Monitor *mon, CPUState *cpu,
>           cc->disas_set_info(cpu, &s.info);
>       }
>   
> +    /* ??? Capstone requires that we copy the data into a host-addressable
> +       buffer first and has no call-back to read more.  Therefore we need
> +       an estimate of buffer size.  This will work for most RISC, but we'll
> +       need to figure out something else for variable-length ISAs.  */
> +    if (s.info.cap_arch >= 0 && cap_disas(&s.info, pc, 4 * nb_insn)) {

.., MIN(16 * nb_insn, TARGET_PAGE_SIZE))) ?

> +        return;
> +    }
> +
>       if (!s.info.print_insn) {
>           monitor_printf(mon, "0x" TARGET_FMT_lx
>                          ": Asm output not supported on this arch\n", pc);
> diff --git a/configure b/configure
> index fd7e3a5e81..a55a8eda8a 100755
> --- a/configure
> +++ b/configure
> @@ -366,6 +366,7 @@ opengl_dmabuf="no"
>   cpuid_h="no"
>   avx2_opt="no"
>   zlib="yes"
> +capstone=""
>   lzo=""
>   snappy=""
>   bzip2=""
> @@ -4381,6 +4382,18 @@ EOF
>   fi
>   
>   ##########################################
> +# capstone
> +
> +if test "$capstone" != no; then
> +  capstone=no
> +  if $pkg_config capstone; then
> +    capstone=yes
> +    QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
> +    LDFLAGS="$LDFLAGS $($pkg_config --libs capstone)"
> +  fi
> +fi
> +
> +##########################################
>   # check if we have fdatasync
>   
>   fdatasync=no
> @@ -5402,6 +5415,7 @@ echo "jemalloc support  $jemalloc"
>   echo "avx2 optimization $avx2_opt"
>   echo "replication support $replication"
>   echo "VxHS block device $vxhs"
> +echo "capstone          $capstone"
>   
>   if test "$sdl_too_old" = "yes"; then
>   echo "-> Your SDL version is too old - please upgrade to have SDL support"
> @@ -6050,6 +6064,9 @@ fi
>   if test "$ivshmem" = "yes" ; then
>     echo "CONFIG_IVSHMEM=y" >> $config_host_mak
>   fi
> +if test "$capstone" = "yes" ; then
> +  echo "CONFIG_CAPSTONE=y" >> $config_host_mak
> +fi
>   
>   # Hold two types of flag:
>   #   CONFIG_THREAD_SETNAME_BYTHREAD  - we've got a way of setting the name on
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler
  2017-09-14 18:35 [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler Richard Henderson
                   ` (9 preceding siblings ...)
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 10/10] target/mips: " Richard Henderson
@ 2017-09-15  4:53 ` Philippe Mathieu-Daudé
  2017-09-19 16:13   ` Richard Henderson
  10 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-09-15  4:53 UTC (permalink / raw)
  To: Richard Henderson, Aurelien Jarno, Yongbok Kim, qemu-arm; +Cc: qemu-devel

On 09/14/2017 03:35 PM, Richard Henderson wrote:
> As occasionally discussed on this list, due to licensing conflicts,
> we are restricted to a version of libopcodes that pre-dates its
> upstream re-licensing to gplv3.  That makes our copy rather old
> and dated.
> 
> I've already seen this as problematic for s390x guest.  I'm sure
> the same problem exists for Power8+, though I haven't looked.
> As we go forward with vector operations we'll see this for x86 host.
> 
> An alternative is to use a BSD-licensed disassembler:
> 
>    https://www.capstone-engine.org/
> 
> This is an actively maintained project derived from llvm.  Moreover,
> it is already in the major Linux distributions, which makes it easy
> to phase in its use.
> 
> I've arranged the code such that we attempt to use capstone first,
> and if that initialization fails, fall back to the existing code
> from binutils.
> 
> 
> r~
> 
> 
> Richard Henderson (10):
>    target/i386: Convert to disas_set_info hook
>    target/ppc: Convert to disas_set_info hook
>    disas: Remove unused flags arguments
>    disas: Support the Capstone disassembler library
>    target/i386: Support Capstone in disas_set_info
>    target/arm: Support Capstone in disas_set_info
>    target/ppc: Support Capstone in disas_set_info
>    target/s390x: Support Capstone in disas_set_info
>    target/sparc: Support Capstone in disas_set_info
>    target/mips: Support Capstone in disas_set_info

At least this msg disappeared:

"Disassembler disagrees with translator over instruction decoding"

i386 comparison:

  ----------------
  IN:
  0xfffffc30:  cli
-0xfffffc31:  mov    %eax,%ebp
-0xfffffc34:  mov    $0x1,%al
-0xfffffc36:  out    %al,$0x80
-0xfffffc38:  xor    %eax,%eax
+0xfffffc31:  movl         %eax, %ebp
+0xfffffc34:  movb         $1, %al
+0xfffffc36:  outb         %al, $0x80
+0xfffffc38:  xorl         %eax, %eax

  IN:
  0x000fd5b8:  cli
  0x000fd5b9:  cld
-0x000fd5ba:  push   %ds
-0x000fd5bb:  push   %eax
+0x000fd5ba:  pushw        %ds
+0x000fd5bb:  pushl        %eax
-0x000fd5bd:  mov    $0xe000,%eax
-0x000fd5c3:  mov    %ax,%ds
-0x000fd5c5:  mov    0xf2f8,%eax
+0x000fd5bd:  movl         $0xe000, %eax
+0x000fd5c3:  movw         %ax, %ds
+0x000fd5c5:  movl         0xf2f8, %eax
+0x000fd5c9:  subl         $0x28, %eax
-0x000fd5c9:  sub    $0x28,%eax
+0x000fd5cd:  popl         0x1c(%eax)
+0x000fd5d2:  popw         (%eax)
-0x000fd5cd:  addr32 popl 0x1c(%eax)
-0x000fd5d2:  addr32 popw (%eax)

For i386, arm, mips32/64:
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

This series but patch 4/10:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Regards,

Phil.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 04/10] disas: Support the Capstone disassembler library
  2017-09-15  4:46   ` Philippe Mathieu-Daudé
@ 2017-09-15 16:58     ` Richard Henderson
  0 siblings, 0 replies; 23+ messages in thread
From: Richard Henderson @ 2017-09-15 16:58 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel

On 09/14/2017 09:46 PM, Philippe Mathieu-Daudé wrote:
>>   +static bool cap_disas(disassemble_info *info, uint64_t pc, size_t size)
> 
> I'd rather use:
> 
> ..,, target_ulong code, ...
>> +{
> 
> uint64_t pc = (uint64_t)code;

Why?

> 
>> +    bool ret = false;
> 
> Isn't it cleaner to have a stubs/disas_capstone.c?
> 
>> +#ifdef CONFIG_CAPSTONE

If this one function warranted a separate file of its own, maybe, but certainly
not without that.

>> +    cap_mode += (info->endian == BFD_ENDIAN_BIG ? CS_MODE_BIG_ENDIAN
>> +                 : CS_MODE_LITTLE_ENDIAN);
>> +
> 
>     assert(size); ?

The existing disassembler doesn't have it.  So, no?

>     err = cs_open(info->cap_arch, cap_mode, &handle);
>     if (err != CS_ERR_OK) {
>         (*info->fprintf_func)(info->stream, "Capstone: %s\n",
>                               cs_strerror(err));

No.  Or not without extra limits.  We don't want 100k instances of this error
as we dump all of the hunks for "-d in_asm".

We'd be assuming the distro didn't do anything silly wrt compiling with "diet"
mode or with only the host cpu enabled.  I really don't know what to do about
such an eventuality except continue to fall back to the binutils code.

>> +    cbuf = buf = g_malloc(size);
> 
>     if (buf == NULL) {
>         goto err2;
>     }

g_malloc cannot fail.

> 
>     cs_free(insn, 1);
> err2:
> 
>> +    g_free(buf);
>> + err1:

Oops, yes.

>> +
>> +    if (s.info.cap_arch >= 0 && cap_disas(&s.info, (uintptr_t)code, size)) {
> 
> (target_ulong)(uintptr_t)code, ?

Why?  Even if I did change the type of the argument?
The extra cast would be implied by the language.

>>   +    /* ??? Capstone requires that we copy the data into a host-addressable
>> +       buffer first and has no call-back to read more.  Therefore we need
>> +       an estimate of buffer size.  This will work for most RISC, but we'll
>> +       need to figure out something else for variable-length ISAs.  */
>> +    if (s.info.cap_arch >= 0 && cap_disas(&s.info, pc, 4 * nb_insn)) {
> 
> .., MIN(16 * nb_insn, TARGET_PAGE_SIZE))) ?

That's no better than what I have; it simply prints more.  I need a *real*
solution.  It will probably involve not re-using cap_disas but writing code
just for the monitor.


r~

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 04/10] disas: Support the Capstone disassembler library
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 04/10] disas: Support the Capstone disassembler library Richard Henderson
  2017-09-15  4:46   ` Philippe Mathieu-Daudé
@ 2017-09-16 18:32   ` Peter Maydell
  2017-09-16 18:52   ` Peter Maydell
  2 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2017-09-16 18:32 UTC (permalink / raw)
  To: Richard Henderson; +Cc: QEMU Developers

On 14 September 2017 at 19:35, Richard Henderson
<richard.henderson@linaro.org> wrote:
> If configured, prefer this over our rather dated copy of the
> GPLv2-only binutils.  This will be especially apparent with
> the proposed vector extensions to TCG, as disas/i386.c does
> not handle AVX.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> diff --git a/configure b/configure
> index fd7e3a5e81..a55a8eda8a 100755
> --- a/configure
> +++ b/configure
> @@ -366,6 +366,7 @@ opengl_dmabuf="no"
>  cpuid_h="no"
>  avx2_opt="no"
>  zlib="yes"
> +capstone=""
>  lzo=""
>  snappy=""
>  bzip2=""
> @@ -4381,6 +4382,18 @@ EOF
>  fi

We should have the usual --enable-capstone/--disable-capstone
option flags so distros can enforce that configure fails
if it can't find the dependency.

It's also tempting to suggesting making it into a submodule
so that we can dump the old binutils code entirely (or
just requiring capstone if you want the debug logging to
be any good).

thanks
-- PMM

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 04/10] disas: Support the Capstone disassembler library
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 04/10] disas: Support the Capstone disassembler library Richard Henderson
  2017-09-15  4:46   ` Philippe Mathieu-Daudé
  2017-09-16 18:32   ` Peter Maydell
@ 2017-09-16 18:52   ` Peter Maydell
  2 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2017-09-16 18:52 UTC (permalink / raw)
  To: Richard Henderson; +Cc: QEMU Developers

On 14 September 2017 at 19:35, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> +    /* ??? Capstone requires that we copy the data into a host-addressable
> +       buffer first and has no call-back to read more.  Therefore we need
> +       an estimate of buffer size.  This will work for most RISC, but we'll
> +       need to figure out something else for variable-length ISAs.  */
> +    if (s.info.cap_arch >= 0 && cap_disas(&s.info, pc, 4 * nb_insn)) {
> +        return;
> +    }

Can we do something with gradually adding more to our host buffer
until capstone says it's managed to disassemble the right number
of instructions?

thanks
-- PMM

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 01/10] target/i386: Convert to disas_set_info hook
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 01/10] target/i386: Convert to disas_set_info hook Richard Henderson
@ 2017-09-18 11:47   ` Alex Bennée
  0 siblings, 0 replies; 23+ messages in thread
From: Alex Bennée @ 2017-09-18 11:47 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  disas.c                 | 22 ++--------------------
>  monitor.c               | 21 ---------------------
>  target/i386/cpu.c       | 12 ++++++++++++
>  target/i386/translate.c |  8 +-------
>  4 files changed, 15 insertions(+), 48 deletions(-)
>
> diff --git a/disas.c b/disas.c
> index d6a1eb9c8e..2be716fdb2 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -205,16 +205,7 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
>          cc->disas_set_info(cpu, &s.info);
>      }
>
> -#if defined(TARGET_I386)
> -    if (flags == 2) {
> -        s.info.mach = bfd_mach_x86_64;
> -    } else if (flags == 1) {
> -        s.info.mach = bfd_mach_i386_i8086;
> -    } else {
> -        s.info.mach = bfd_mach_i386_i386;
> -    }
> -    s.info.print_insn = print_insn_i386;
> -#elif defined(TARGET_PPC)
> +#if defined(TARGET_PPC)
>      if ((flags >> 16) & 1) {
>          s.info.endian = BFD_ENDIAN_LITTLE;
>      }
> @@ -390,16 +381,7 @@ void monitor_disas(Monitor *mon, CPUState *cpu,
>          cc->disas_set_info(cpu, &s.info);
>      }
>
> -#if defined(TARGET_I386)
> -    if (flags == 2) {
> -        s.info.mach = bfd_mach_x86_64;
> -    } else if (flags == 1) {
> -        s.info.mach = bfd_mach_i386_i8086;
> -    } else {
> -        s.info.mach = bfd_mach_i386_i386;
> -    }
> -    s.info.print_insn = print_insn_i386;
> -#elif defined(TARGET_PPC)
> +#if defined(TARGET_PPC)
>      if (flags & 0xFFFF) {
>          /* If we have a precise definition of the instruction set, use it. */
>          s.info.mach = flags & 0xFFFF;
> diff --git a/monitor.c b/monitor.c
> index 9239f7adde..3f3ebc31ef 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1310,27 +1310,6 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize,
>
>      if (format == 'i') {
>          int flags = 0;
> -#ifdef TARGET_I386
> -        CPUArchState *env = mon_get_cpu_env();
> -        if (wsize == 2) {
> -            flags = 1;
> -        } else if (wsize == 4) {
> -            flags = 0;
> -        } else {
> -            /* as default we use the current CS size */
> -            flags = 0;
> -            if (env) {
> -#ifdef TARGET_X86_64
> -                if ((env->efer & MSR_EFER_LMA) &&
> -                    (env->segs[R_CS].flags & DESC_L_MASK))
> -                    flags = 2;
> -                else
> -#endif
> -                if (!(env->segs[R_CS].flags & DESC_B_MASK))
> -                    flags = 1;
> -            }
> -        }
> -#endif
>  #ifdef TARGET_PPC
>          CPUArchState *env = mon_get_cpu_env();
>          flags = msr_le << 16;
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 69676e13e1..b869a69c53 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4099,6 +4099,17 @@ static bool x86_cpu_has_work(CPUState *cs)
>              !(env->hflags & HF_SMM_MASK));
>  }
>
> +static void x86_disas_set_info(CPUState *cs, disassemble_info *info)
> +{
> +    X86CPU *cpu = X86_CPU(cs);
> +    CPUX86State *env = &cpu->env;
> +
> +    info->mach = (env->hflags & HF_CS64_MASK ? bfd_mach_x86_64
> +                  : env->hflags & HF_CS32_MASK ? bfd_mach_i386_i386
> +                  : bfd_mach_i386_i8086);
> +    info->print_insn = print_insn_i386;
> +}
> +
>  static Property x86_cpu_properties[] = {
>  #ifdef CONFIG_USER_ONLY
>      /* apic_id = 0 by default for *-user, see commit 9886e834 */
> @@ -4204,6 +4215,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>  #endif
>      cc->cpu_exec_enter = x86_cpu_exec_enter;
>      cc->cpu_exec_exit = x86_cpu_exec_exit;
> +    cc->disas_set_info = x86_disas_set_info;
>
>      dc->user_creatable = true;
>  }
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index de0c989763..06c2cb9e64 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -8526,15 +8526,9 @@ static void i386_tr_disas_log(const DisasContextBase *dcbase,
>                                CPUState *cpu)
>  {
>      DisasContext *dc = container_of(dcbase, DisasContext, base);
> -    int disas_flags = !dc->code32;
>
>      qemu_log("IN: %s\n", lookup_symbol(dc->base.pc_first));
> -#ifdef TARGET_X86_64
> -    if (dc->code64) {
> -        disas_flags = 2;
> -    }
> -#endif
> -    log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size, disas_flags);
> +    log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size, 0);
>  }
>
>  static const TranslatorOps i386_tr_ops = {


--
Alex Bennée

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 02/10] target/ppc: Convert to disas_set_info hook
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 02/10] target/ppc: " Richard Henderson
@ 2017-09-18 11:58   ` Alex Bennée
  0 siblings, 0 replies; 23+ messages in thread
From: Alex Bennée @ 2017-09-18 11:58 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel, qemu-ppc


Richard Henderson <richard.henderson@linaro.org> writes:

> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  disas.c                     | 33 ---------------------------------
>  monitor.c                   |  5 -----
>  target/ppc/translate.c      |  5 +----
>  target/ppc/translate_init.c | 21 +++++++++++++++++++++
>  4 files changed, 22 insertions(+), 42 deletions(-)
>
> diff --git a/disas.c b/disas.c
> index 2be716fdb2..3a375a3b6c 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -205,23 +205,6 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
>          cc->disas_set_info(cpu, &s.info);
>      }
>
> -#if defined(TARGET_PPC)
> -    if ((flags >> 16) & 1) {
> -        s.info.endian = BFD_ENDIAN_LITTLE;
> -    }
> -    if (flags & 0xFFFF) {
> -        /* If we have a precise definition of the instruction set, use it. */
> -        s.info.mach = flags & 0xFFFF;
> -    } else {
> -#ifdef TARGET_PPC64
> -        s.info.mach = bfd_mach_ppc64;
> -#else
> -        s.info.mach = bfd_mach_ppc;
> -#endif
> -    }
> -    s.info.disassembler_options = (char *)"any";
> -    s.info.print_insn = print_insn_ppc;
> -#endif
>      if (s.info.print_insn == NULL) {
>          s.info.print_insn = print_insn_od_target;
>      }
> @@ -381,22 +364,6 @@ void monitor_disas(Monitor *mon, CPUState *cpu,
>          cc->disas_set_info(cpu, &s.info);
>      }
>
> -#if defined(TARGET_PPC)
> -    if (flags & 0xFFFF) {
> -        /* If we have a precise definition of the instruction set, use it. */
> -        s.info.mach = flags & 0xFFFF;
> -    } else {
> -#ifdef TARGET_PPC64
> -        s.info.mach = bfd_mach_ppc64;
> -#else
> -        s.info.mach = bfd_mach_ppc;
> -#endif
> -    }
> -    if ((flags >> 16) & 1) {
> -        s.info.endian = BFD_ENDIAN_LITTLE;
> -    }
> -    s.info.print_insn = print_insn_ppc;
> -#endif
>      if (!s.info.print_insn) {
>          monitor_printf(mon, "0x" TARGET_FMT_lx
>                         ": Asm output not supported on this arch\n", pc);
> diff --git a/monitor.c b/monitor.c
> index 3f3ebc31ef..a0f43f27e7 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1310,11 +1310,6 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize,
>
>      if (format == 'i') {
>          int flags = 0;
> -#ifdef TARGET_PPC
> -        CPUArchState *env = mon_get_cpu_env();
> -        flags = msr_le << 16;
> -        flags |= env->bfd_mach;
> -#endif
>          monitor_disas(mon, cs, addr, count, is_physical, flags);
>          return;
>      }
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 606b605ba0..bc155f1036 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -7395,12 +7395,9 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
>  #if defined(DEBUG_DISAS)
>      if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
>          && qemu_log_in_addr_range(pc_start)) {
> -        int flags;
> -        flags = env->bfd_mach;
> -        flags |= ctx.le_mode << 16;
>          qemu_log_lock();
>          qemu_log("IN: %s\n", lookup_symbol(pc_start));
> -        log_target_disas(cs, pc_start, ctx.nip - pc_start, flags);
> +        log_target_disas(cs, pc_start, ctx.nip - pc_start, 0);
>          qemu_log("\n");
>          qemu_log_unlock();
>      }
> diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> index c827d1e388..c7b4a7c02a 100644
> --- a/target/ppc/translate_init.c
> +++ b/target/ppc/translate_init.c
> @@ -10644,6 +10644,26 @@ static gchar *ppc_gdb_arch_name(CPUState *cs)
>  #endif
>  }
>
> +static void ppc_disas_set_info(CPUState *cs, disassemble_info *info)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUPPCState *env = &cpu->env;
> +
> +    if ((env->hflags >> MSR_LE) & 1) {
> +        info->endian = BFD_ENDIAN_LITTLE;
> +    }
> +    info->mach = env->bfd_mach;
> +    if (!env->bfd_mach) {
> +#ifdef TARGET_PPC64
> +        info->mach = bfd_mach_ppc64;
> +#else
> +        info->mach = bfd_mach_ppc;
> +#endif
> +    }
> +    info->disassembler_options = (char *)"any";
> +    info->print_insn = print_insn_ppc;
> +}
> +
>  static Property ppc_cpu_properties[] = {
>      DEFINE_PROP_BOOL("pre-2.8-migration", PowerPCCPU, pre_2_8_migration, false),
>      DEFINE_PROP_BOOL("pre-2.10-migration", PowerPCCPU, pre_2_10_migration,
> @@ -10705,6 +10725,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
>  #ifndef CONFIG_USER_ONLY
>      cc->virtio_is_big_endian = ppc_cpu_is_big_endian;
>  #endif
> +    cc->disas_set_info = ppc_disas_set_info;
>
>      dc->fw_name = "PowerPC,UNKNOWN";
>  }


--
Alex Bennée

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 03/10] disas: Remove unused flags arguments
  2017-09-14 18:35 ` [Qemu-devel] [PATCH 03/10] disas: Remove unused flags arguments Richard Henderson
@ 2017-09-18 11:59   ` Alex Bennée
  0 siblings, 0 replies; 23+ messages in thread
From: Alex Bennée @ 2017-09-18 11:59 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> Now that every target is using the disas_set_info hook,
> the flags argument is unused.  Remove it.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  include/disas/disas.h         |  4 ++--
>  include/exec/log.h            |  4 ++--
>  disas.c                       | 15 ++++-----------
>  monitor.c                     |  3 +--
>  target/alpha/translate.c      |  2 +-
>  target/arm/translate-a64.c    |  3 +--
>  target/arm/translate.c        |  3 +--
>  target/cris/translate.c       |  3 +--
>  target/hppa/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/nios2/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 +-
>  23 files changed, 28 insertions(+), 39 deletions(-)
>
> diff --git a/include/disas/disas.h b/include/disas/disas.h
> index e549ca24a1..4d48c13c65 100644
> --- a/include/disas/disas.h
> +++ b/include/disas/disas.h
> @@ -9,10 +9,10 @@
>  /* Disassemble this for me please... (debugging). */
>  void disas(FILE *out, void *code, unsigned long size);
>  void target_disas(FILE *out, CPUState *cpu, target_ulong code,
> -                  target_ulong size, int flags);
> +                  target_ulong size);
>
>  void monitor_disas(Monitor *mon, CPUState *cpu,
> -                   target_ulong pc, int nb_insn, int is_physical, int flags);
> +                   target_ulong pc, int nb_insn, int is_physical);
>
>  /* Look up symbol for debugging purpose.  Returns "" if unknown. */
>  const char *lookup_symbol(target_ulong orig_addr);
> diff --git a/include/exec/log.h b/include/exec/log.h
> index ba1c9b5682..c249307911 100644
> --- a/include/exec/log.h
> +++ b/include/exec/log.h
> @@ -38,9 +38,9 @@ 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(CPUState *cpu, target_ulong start,
> -                                    target_ulong len, int flags)
> +                                    target_ulong len)
>  {
> -    target_disas(qemu_logfile, cpu, start, len, flags);
> +    target_disas(qemu_logfile, cpu, start, len);
>  }
>
>  static inline void log_disas(void *code, unsigned long size)
> diff --git a/disas.c b/disas.c
> index 3a375a3b6c..ad675dc361 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -171,15 +171,9 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info)
>      return print_insn_objdump(pc, info, "OBJD-T");
>  }
>
> -/* Disassemble this for me please... (debugging). 'flags' has the following
> -   values:
> -    i386 - 1 means 16 bit code, 2 means 64 bit code
> -    ppc  - bits 0:15 specify (optionally) the machine instruction set;
> -           bit 16 indicates little endian.
> -    other targets - unused
> - */
> +/* Disassemble this for me please... (debugging).  */
>  void target_disas(FILE *out, CPUState *cpu, target_ulong code,
> -                  target_ulong size, int flags)
> +                  target_ulong size)
>  {
>      CPUClass *cc = CPU_GET_CLASS(cpu);
>      target_ulong pc;
> @@ -336,10 +330,9 @@ monitor_read_memory (bfd_vma memaddr, bfd_byte *myaddr, int length,
>      return 0;
>  }
>
> -/* Disassembler for the monitor.
> -   See target_disas for a description of flags. */
> +/* Disassembler for the monitor.  */
>  void monitor_disas(Monitor *mon, CPUState *cpu,
> -                   target_ulong pc, int nb_insn, int is_physical, int flags)
> +                   target_ulong pc, int nb_insn, int is_physical)
>  {
>      CPUClass *cc = CPU_GET_CLASS(cpu);
>      int count, i;
> diff --git a/monitor.c b/monitor.c
> index a0f43f27e7..2125b54101 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1309,8 +1309,7 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize,
>      }
>
>      if (format == 'i') {
> -        int flags = 0;
> -        monitor_disas(mon, cs, addr, count, is_physical, flags);
> +        monitor_disas(mon, cs, addr, count, is_physical);
>          return;
>      }
>
> diff --git a/target/alpha/translate.c b/target/alpha/translate.c
> index 5a92c4accb..e9a245f9c5 100644
> --- a/target/alpha/translate.c
> +++ b/target/alpha/translate.c
> @@ -3048,7 +3048,7 @@ static void alpha_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
>  static void alpha_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu)
>  {
>      qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
> -    log_target_disas(cpu, dcbase->pc_first, dcbase->tb->size, 1);
> +    log_target_disas(cpu, dcbase->pc_first, dcbase->tb->size);
>  }
>
>  static const TranslatorOps alpha_tr_ops = {
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 9017e30510..a3984c9a0d 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -11409,8 +11409,7 @@ static void aarch64_tr_disas_log(const DisasContextBase *dcbase,
>      DisasContext *dc = container_of(dcbase, DisasContext, base);
>
>      qemu_log("IN: %s\n", lookup_symbol(dc->base.pc_first));
> -    log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size,
> -                     4 | (bswap_code(dc->sctlr_b) ? 2 : 0));
> +    log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size);
>  }
>
>  const TranslatorOps aarch64_translator_ops = {
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index ab1a12a1b8..93e9dbe33d 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -12258,8 +12258,7 @@ static void arm_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu)
>      DisasContext *dc = container_of(dcbase, DisasContext, base);
>
>      qemu_log("IN: %s\n", lookup_symbol(dc->base.pc_first));
> -    log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size,
> -                     dc->thumb | (dc->sctlr_b << 1));
> +    log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size);
>  }
>
>  static const TranslatorOps arm_translator_ops = {
> diff --git a/target/cris/translate.c b/target/cris/translate.c
> index 38a999e6f1..b1fda57c74 100644
> --- a/target/cris/translate.c
> +++ b/target/cris/translate.c
> @@ -3297,8 +3297,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
>          qemu_log_lock();
>          qemu_log("--------------\n");
>          qemu_log("IN: %s\n", lookup_symbol(pc_start));
> -        log_target_disas(cs, pc_start, dc->pc - pc_start,
> -                         env->pregs[PR_VR]);
> +        log_target_disas(cs, pc_start, dc->pc - pc_start);
>          qemu_log("\nisize=%d osize=%d\n",
>                   dc->pc - pc_start, tcg_op_buf_count());
>          qemu_log_unlock();
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index b6e2652341..fc2a9f5896 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -3904,7 +3904,7 @@ static void hppa_tr_disas_log(const DisasContextBase *dcbase, CPUState *cs)
>          break;
>      default:
>          qemu_log("IN: %s\n", lookup_symbol(tb->pc));
> -        log_target_disas(cs, tb->pc, tb->size, 1);
> +        log_target_disas(cs, tb->pc, tb->size);
>          break;
>      }
>  }
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index 06c2cb9e64..23d9e05426 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -8528,7 +8528,7 @@ static void i386_tr_disas_log(const DisasContextBase *dcbase,
>      DisasContext *dc = container_of(dcbase, DisasContext, base);
>
>      qemu_log("IN: %s\n", lookup_symbol(dc->base.pc_first));
> -    log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size, 0);
> +    log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size);
>  }
>
>  static const TranslatorOps i386_tr_ops = {
> diff --git a/target/lm32/translate.c b/target/lm32/translate.c
> index 65bc9c0bf6..a83cbdf729 100644
> --- a/target/lm32/translate.c
> +++ b/target/lm32/translate.c
> @@ -1156,7 +1156,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
>          && qemu_log_in_addr_range(pc_start)) {
>          qemu_log_lock();
>          qemu_log("\n");
> -        log_target_disas(cs, pc_start, dc->pc - pc_start, 0);
> +        log_target_disas(cs, pc_start, dc->pc - pc_start);
>          qemu_log("\nisize=%d osize=%d\n",
>                   dc->pc - pc_start, tcg_op_buf_count());
>          qemu_log_unlock();
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index d738f32f9c..e1e31f622c 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -5620,7 +5620,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb)
>          qemu_log_lock();
>          qemu_log("----------------\n");
>          qemu_log("IN: %s\n", lookup_symbol(pc_start));
> -        log_target_disas(cs, pc_start, dc->pc - pc_start, 0);
> +        log_target_disas(cs, pc_start, dc->pc - pc_start);
>          qemu_log("\n");
>          qemu_log_unlock();
>      }
> diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
> index 067b0878d6..fecc61a1ec 100644
> --- a/target/microblaze/translate.c
> +++ b/target/microblaze/translate.c
> @@ -1810,7 +1810,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
>          qemu_log_lock();
>          qemu_log("--------------\n");
>  #if DISAS_GNU
> -        log_target_disas(cs, pc_start, dc->pc - pc_start, 0);
> +        log_target_disas(cs, pc_start, dc->pc - pc_start);
>  #endif
>          qemu_log("\nisize=%d osize=%d\n",
>                   dc->pc - pc_start, tcg_op_buf_count());
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index c78d27294c..3b9cf9a236 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -20369,7 +20369,7 @@ done_generating:
>          && qemu_log_in_addr_range(pc_start)) {
>          qemu_log_lock();
>          qemu_log("IN: %s\n", lookup_symbol(pc_start));
> -        log_target_disas(cs, pc_start, ctx.pc - pc_start, 0);
> +        log_target_disas(cs, pc_start, ctx.pc - pc_start);
>          qemu_log("\n");
>          qemu_log_unlock();
>      }
> diff --git a/target/nios2/translate.c b/target/nios2/translate.c
> index 6b0961837d..7a0fa860da 100644
> --- a/target/nios2/translate.c
> +++ b/target/nios2/translate.c
> @@ -907,7 +907,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb)
>          && qemu_log_in_addr_range(tb->pc)) {
>          qemu_log_lock();
>          qemu_log("IN: %s\n", lookup_symbol(tb->pc));
> -        log_target_disas(cs, tb->pc, dc->pc - tb->pc, 0);
> +        log_target_disas(cs, tb->pc, dc->pc - tb->pc);
>          qemu_log("\n");
>          qemu_log_unlock();
>      }
> diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
> index 112db1ad0f..99f2b463ce 100644
> --- a/target/openrisc/translate.c
> +++ b/target/openrisc/translate.c
> @@ -1653,7 +1653,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
>
>      if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
>          && qemu_log_in_addr_range(pc_start)) {
> -        log_target_disas(cs, pc_start, tb->size, 0);
> +        log_target_disas(cs, pc_start, tb->size);
>          qemu_log("\n");
>          qemu_log_unlock();
>      }
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index bc155f1036..8e92e4579c 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -7397,7 +7397,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
>          && qemu_log_in_addr_range(pc_start)) {
>          qemu_log_lock();
>          qemu_log("IN: %s\n", lookup_symbol(pc_start));
> -        log_target_disas(cs, pc_start, ctx.nip - pc_start, 0);
> +        log_target_disas(cs, pc_start, ctx.nip - pc_start);
>          qemu_log("\n");
>          qemu_log_unlock();
>      }
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 909b12818d..28bd02ceb3 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -5910,7 +5910,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
>              qemu_log("IN: EXECUTE %016" PRIx64 "\n", dc.ex_value);
>          } else {
>              qemu_log("IN: %s\n", lookup_symbol(pc_start));
> -            log_target_disas(cs, pc_start, dc.pc - pc_start, 1);
> +            log_target_disas(cs, pc_start, dc.pc - pc_start);
>              qemu_log("\n");
>          }
>          qemu_log_unlock();
> diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> index 10191073b2..7532bf74c1 100644
> --- a/target/sh4/translate.c
> +++ b/target/sh4/translate.c
> @@ -2347,7 +2347,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
>          && qemu_log_in_addr_range(pc_start)) {
>          qemu_log_lock();
>  	qemu_log("IN:\n");	/* , lookup_symbol(pc_start)); */
> -        log_target_disas(cs, pc_start, ctx.pc - pc_start, 0);
> +        log_target_disas(cs, pc_start, ctx.pc - pc_start);
>  	qemu_log("\n");
>          qemu_log_unlock();
>      }
> diff --git a/target/sparc/translate.c b/target/sparc/translate.c
> index 6290705b11..e89b6227f2 100644
> --- a/target/sparc/translate.c
> +++ b/target/sparc/translate.c
> @@ -5855,7 +5855,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock * tb)
>          qemu_log_lock();
>          qemu_log("--------------\n");
>          qemu_log("IN: %s\n", lookup_symbol(pc_start));
> -        log_target_disas(cs, pc_start, last_pc + 4 - pc_start, 0);
> +        log_target_disas(cs, pc_start, last_pc + 4 - pc_start);
>          qemu_log("\n");
>          qemu_log_unlock();
>      }
> diff --git a/target/tricore/translate.c b/target/tricore/translate.c
> index 4e4198e887..e807500e26 100644
> --- a/target/tricore/translate.c
> +++ b/target/tricore/translate.c
> @@ -8839,7 +8839,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
>          && qemu_log_in_addr_range(pc_start)) {
>          qemu_log_lock();
>          qemu_log("IN: %s\n", lookup_symbol(pc_start));
> -        log_target_disas(cs, pc_start, ctx.pc - pc_start, 0);
> +        log_target_disas(cs, pc_start, ctx.pc - pc_start);
>          qemu_log("\n");
>          qemu_log_unlock();
>      }
> diff --git a/target/unicore32/translate.c b/target/unicore32/translate.c
> index 6c094d59d7..f9aa248a80 100644
> --- a/target/unicore32/translate.c
> +++ b/target/unicore32/translate.c
> @@ -2031,7 +2031,7 @@ done_generating:
>          qemu_log_lock();
>          qemu_log("----------------\n");
>          qemu_log("IN: %s\n", lookup_symbol(pc_start));
> -        log_target_disas(cs, pc_start, dc->pc - pc_start, 0);
> +        log_target_disas(cs, pc_start, dc->pc - pc_start);
>          qemu_log("\n");
>          qemu_log_unlock();
>      }
> diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
> index d7bf07e8e6..03719ce12b 100644
> --- a/target/xtensa/translate.c
> +++ b/target/xtensa/translate.c
> @@ -3250,7 +3250,7 @@ done:
>          qemu_log_lock();
>          qemu_log("----------------\n");
>          qemu_log("IN: %s\n", lookup_symbol(pc_start));
> -        log_target_disas(cs, pc_start, dc.pc - pc_start, 0);
> +        log_target_disas(cs, pc_start, dc.pc - pc_start);
>          qemu_log("\n");
>          qemu_log_unlock();
>      }


--
Alex Bennée

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler
  2017-09-15  4:53 ` [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler Philippe Mathieu-Daudé
@ 2017-09-19 16:13   ` Richard Henderson
  2017-09-19 17:30     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 23+ messages in thread
From: Richard Henderson @ 2017-09-19 16:13 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Aurelien Jarno, Yongbok Kim,
	qemu-arm
  Cc: qemu-devel

[ Just saw this, so missed adding tags to the v2 patch set. ]

On 09/14/2017 11:53 PM, Philippe Mathieu-Daudé wrote:
> At least this msg disappeared:
> 
> "Disassembler disagrees with translator over instruction decoding"

It's back in v2.

> For i386, arm, mips32/64:
> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Which patches?  Which mips versions?

Can you, by any chance, test micro-mips?  I'm certain I've got that wrong in
the v1 patch, and thus I dropped the mips patch from v2.  But in theory
capstone supports umips too and should be trivially fixable.


r~

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler
  2017-09-19 16:13   ` Richard Henderson
@ 2017-09-19 17:30     ` Philippe Mathieu-Daudé
  2017-09-19 18:36       ` Richard Henderson
  0 siblings, 1 reply; 23+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-09-19 17:30 UTC (permalink / raw)
  To: Richard Henderson, Aurelien Jarno, Yongbok Kim; +Cc: qemu-devel

On 09/19/2017 01:13 PM, Richard Henderson wrote:
> [ Just saw this, so missed adding tags to the v2 patch set. ]
> 
> On 09/14/2017 11:53 PM, Philippe Mathieu-Daudé wrote:
>> At least this msg disappeared:
>>
>> "Disassembler disagrees with translator over instruction decoding"
> 
> It's back in v2.
> 
>> For i386, arm, mips32/64:
>> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Which patches?  Which mips versions?

full series, Malta board default cpu

> Can you, by any chance, test micro-mips?  I'm certain I've got that wrong in
> the v1 patch, and thus I dropped the mips patch from v2.  But in theory
> capstone supports umips too and should be trivially fixable.

$ mipsel-softmmu/qemu-system-mipsel -machine malta -cpu M14Kc -append 
"ttyS0 rw" -nographic -d in_asm -kernel vmlinux -initrd initrd.gz

  IN: kernel_entry
  0x801039e0:  syscall   0x3f004
  0x801039e4:  b 0x8011406c
-0x801039e8:  addu      t2,zero,ra
-0x801039ec:  c0        0x900028
-0x801039f0:  0x1f7108
-0x801039f4:  syscall   0xbf004
+0x801039e8:  addu         $t2, $zero, $ra

  IN: kernel_entry
-0x801039f8:  blezalc   zero,zero,0x801039fc
-0x801039fc:  lb        s0,16808(zero)
-0x80103a00:  xori      t1,s0,0x3108
+0x801039f8:  blez         $zero, 0x801039fc
+0x801039fc:  lb           $s0, 0x41a8($zero)
+0x80103a00:  xori         $t1, $s0, 0x3108
  0x80103a04:  jal       0x80011620

  IN: kernel_entry
-0x80103a08:  lb        t9,16808(at)
-0x80103a0c:  beqzalc   zero,zero,0x8010fe30
-0x80103a10:  0xf808
-0x80103a14:  lb        gp,16809(at)
-0x80103a18:  sdr       gp,12585(a1)
-0x80103a1c:  jialc     t0,19720
-0x80103a20:  sdr       t0,0(t1)
-0x80103a24:  jal       0x8003ffec
+0x80103a08:  lb           $t9, 0x41a8($at)
+0x80103a0c:  addi         $zero, $zero, 0x3108

  IN: kernel_entry
-0x80103a1c:  jialc     t0,19720
-0x80103a20:  sdr       t0,0(t1)
-0x80103a24:  jal       0x8003ffec
+0x80103a1c:  sdc2         $8, 0x4d08($zero)

  IN: kernel_entry
-0x80103a28:  lb        t9,16801(at)
-0x80103a2c:  sltiu     a0,t6,-1919
-0x80103a30:  lb        t9,16801(at)
-0x80103a34:  sltiu     t0,t6,-1887
-0x80103a38:  lb        t9,16801(at)
-0x80103a3c:  sltiu     t4,t6,-1855
-0x80103a40:  lb        t9,16801(at)
-0x80103a44:  sltiu     s0,t6,-1823
-0x80103a48:  sllv      zero,gp,s7
-0x80103a4c:  lb        s5,16828(at)
-0x80103a50:  sc        zero,13212(zero)
-0x80103a54:  bltuc     ra,zero,0x801108d8
-0x80103a58:  balc      0x855048d0
-0x80103a5c:  blezalc   zero,zero,0x80103a60
-0x80103a60:  lb        t9,16801(at)
-0x80103a64:  sltiu     t8,t6,-1119
-0x80103a68:  sd        s0,13245(ra)
-0x80103a6c:  lwl       s6,-11237(s2)
-0x80103a70:  jal       0x80003000
-Disassembler disagrees with translator over instruction decoding
-Please report this to qemu-devel@nongnu.org
+0x80103a28:  lb           $t9, 0x41a1($at)
+0x80103a2c:  sltiu        $a0, $t6, -0x77f
+0x80103a30:  lb           $t9, 0x41a1($at)
+0x80103a34:  sltiu        $t0, $t6, -0x75f
+0x80103a38:  lb           $t9, 0x41a1($at)
+0x80103a3c:  sltiu        $t4, $t6, -0x73f
+0x80103a40:  lb           $t9, 0x41a1($at)
+0x80103a44:  sltiu        $s0, $t6, -0x71f
+0x80103a48:  sllv         $zero, $gp, $s7
+0x80103a4c:  lb           $s5, 0x41bc($at)
+0x80103a50:  sc           $zero, 0x339c($zero)
+0x80103a54:  bgtz         $ra, 0x801108d8
+0x80103a58:  swc2         $16, 0x39d($t2)
+0x80103a5c:  blez         $zero, 0x80103a60
+0x80103a60:  lb           $t9, 0x41a1($at)
+0x80103a64:  sltiu        $t8, $t6, -0x45f

...

  IN: try_acquire_console_sem
-0x8011fb26:  bnezc     v1,0x819db12e
-0x8011fb2a:  0x41a2a3d4
-0x8011fb2e:  jialc     v0,-32710
-0x8011fb32:  jal       0x818291a0
-0x8011fb36:  bc1f      $fcc1,0x80122c46
-0x8011fb3a:  sdc1      $f9,18182(zero)
-Disassembler disagrees with translator over instruction decoding
-Please report this to qemu-devel@nongnu.org
+0x8011fb26:  sdc2         $2, -0x127f($v1)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler
  2017-09-19 17:30     ` Philippe Mathieu-Daudé
@ 2017-09-19 18:36       ` Richard Henderson
  0 siblings, 0 replies; 23+ messages in thread
From: Richard Henderson @ 2017-09-19 18:36 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Aurelien Jarno, Yongbok Kim; +Cc: qemu-devel

On 09/19/2017 12:30 PM, Philippe Mathieu-Daudé wrote:
> On 09/19/2017 01:13 PM, Richard Henderson wrote:
>> [ Just saw this, so missed adding tags to the v2 patch set. ]
>>
>> On 09/14/2017 11:53 PM, Philippe Mathieu-Daudé wrote:
>>> At least this msg disappeared:
>>>
>>> "Disassembler disagrees with translator over instruction decoding"
>>
>> It's back in v2.
>>
>>> For i386, arm, mips32/64:
>>> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>
>> Which patches?  Which mips versions?
> 
> full series, Malta board default cpu
> 
>> Can you, by any chance, test micro-mips?  I'm certain I've got that wrong in
>> the v1 patch, and thus I dropped the mips patch from v2.  But in theory
>> capstone supports umips too and should be trivially fixable.
> 
> $ mipsel-softmmu/qemu-system-mipsel -machine malta -cpu M14Kc -append "ttyS0
> rw" -nographic -d in_asm -kernel vmlinux -initrd initrd.gz
> 
>  IN: kernel_entry
>  0x801039e0:  syscall   0x3f004
>  0x801039e4:  b 0x8011406c
> -0x801039e8:  addu      t2,zero,ra
> -0x801039ec:  c0        0x900028
> -0x801039f0:  0x1f7108
> -0x801039f4:  syscall   0xbf004
> +0x801039e8:  addu         $t2, $zero, $ra

This is indicative of the other bug that I fixed in v2, where we would silently
ignore unknown instructions.

>From this and the other hunks it would appear that either (1) I messed up the
CS_MODE_* bits for mips or (2) the capstone backend for mips is not in terribly
good shape.

I think I was right to drop the patch from v2.


r~

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2017-09-19 18:36 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-14 18:35 [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler Richard Henderson
2017-09-14 18:35 ` [Qemu-devel] [PATCH 01/10] target/i386: Convert to disas_set_info hook Richard Henderson
2017-09-18 11:47   ` Alex Bennée
2017-09-14 18:35 ` [Qemu-devel] [PATCH 02/10] target/ppc: " Richard Henderson
2017-09-18 11:58   ` Alex Bennée
2017-09-14 18:35 ` [Qemu-devel] [PATCH 03/10] disas: Remove unused flags arguments Richard Henderson
2017-09-18 11:59   ` Alex Bennée
2017-09-14 18:35 ` [Qemu-devel] [PATCH 04/10] disas: Support the Capstone disassembler library Richard Henderson
2017-09-15  4:46   ` Philippe Mathieu-Daudé
2017-09-15 16:58     ` Richard Henderson
2017-09-16 18:32   ` Peter Maydell
2017-09-16 18:52   ` Peter Maydell
2017-09-14 18:35 ` [Qemu-devel] [PATCH 05/10] target/i386: Support Capstone in disas_set_info Richard Henderson
2017-09-14 18:35 ` [Qemu-devel] [PATCH 06/10] target/arm: " Richard Henderson
2017-09-14 18:35 ` [Qemu-devel] [PATCH 07/10] target/ppc: " Richard Henderson
2017-09-14 18:35 ` [Qemu-devel] [PATCH 08/10] target/s390x: " Richard Henderson
2017-09-14 18:35 ` [Qemu-devel] [PATCH 09/10] target/sparc: " Richard Henderson
2017-09-14 18:35 ` [Qemu-devel] [PATCH 10/10] target/mips: " Richard Henderson
2017-09-15  2:47   ` Philippe Mathieu-Daudé
2017-09-15  4:53 ` [Qemu-devel] [PATCH 00/10] Support the Capstone disassembler Philippe Mathieu-Daudé
2017-09-19 16:13   ` Richard Henderson
2017-09-19 17:30     ` Philippe Mathieu-Daudé
2017-09-19 18:36       ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).