[Qemu-devel] [PULL 00/11] Disassembler patches

[Qemu-devel] [PULL 00/11] Disassembler patches

[Richard Henderson]

Support for Capstone, plus an arm32 fix.


r~


The following changes since commit 3d7196d43bfe12efe98568cb60057e273652b99b:

  Merge remote-tracking branch 'remotes/kraxel/tags/usb-20171023-pull-request' into staging (2017-10-24 16:05:57 +0100)

are available in the git repository at:

  git://github.com/rth7680/qemu.git tags/pull-dis-20171025

for you to fetch changes up to 383b90bc6a15f4b18ec34f9c0287b26f9a89fcb8:

  disas: Add capstone as submodule (2017-10-25 11:55:21 +0200)

----------------------------------------------------------------
Capstone disassembler

----------------------------------------------------------------
Richard Henderson (11):
      target/i386: Convert to disas_set_info hook
      target/ppc: Convert to disas_set_info hook
      target/arm: Move BE32 disassembler fixup
      target/arm: Don't set INSN_ARM_BE32 for CONFIG_USER_ONLY
      disas: Remove unused flags arguments
      disas: Support the Capstone disassembler library
      i386: Support Capstone in disas_set_info
      arm: Support Capstone in disas_set_info
      ppc: Support Capstone in disas_set_info
      disas: Remove monitor_disas_is_physical
      disas: Add capstone as submodule

 Makefile                      |  15 ++
 include/disas/bfd.h           |  11 +-
 include/disas/capstone.h      |  38 ++++++
 include/disas/disas.h         |   4 +-
 include/exec/log.h            |   4 +-
 disas.c                       | 308 ++++++++++++++++++++++++++++++------------
 disas/arm.c                   |  21 ++-
 monitor.c                     |  29 +---
 target/alpha/translate.c      |   2 +-
 target/arm/cpu.c              |  49 +++----
 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/translate.c       |   2 +-
 target/nios2/translate.c      |   2 +-
 target/openrisc/translate.c   |   2 +-
 target/ppc/translate.c        |   5 +-
 target/ppc/translate_init.c   |  27 ++++
 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 +-
 .gitmodules                   |   3 +
 capstone                      |   1 +
 configure                     |  64 +++++++++
 33 files changed, 459 insertions(+), 184 deletions(-)
 create mode 100644 include/disas/capstone.h
 create mode 160000 capstone

[Qemu-devel] [PULL 01/11] target/i386: Convert to disas_set_info hook

[Richard Henderson]

Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
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 54eea3f9c9..7e22a80da6 100644
--- a/disas.c
+++ b/disas.c
@@ -204,16 +204,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;
     }
@@ -389,16 +380,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 fe0d1bdbb4..a736ae9a81 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 98732cd65f..13b2f8fbc5 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4097,6 +4097,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 */
@@ -4216,6 +4227,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 5f24a2de3c..69a87de83b 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -8557,15 +8557,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.6

[Qemu-devel] [PULL 02/11] target/ppc: Convert to disas_set_info hook

[Richard Henderson]

Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.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 7e22a80da6..88d16fbd57 100644
--- a/disas.c
+++ b/disas.c
@@ -204,23 +204,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;
     }
@@ -380,22 +363,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 a736ae9a81..2164dfcc3b 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 a81ff69d75..cddbf92564 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7407,12 +7407,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 7b9bf6a773..9b4353437a 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -10519,6 +10519,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,
@@ -10582,6 +10602,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.6

[Qemu-devel] [PULL 03/11] target/arm: Move BE32 disassembler fixup

[Richard Henderson]

The Capstone disassembler has its own big-endian fixup.
Doing this twice does not work, of course.  Move our current
fixup from target/arm/cpu.c to disas/arm.c.

This makes read_memory_inner_func unused and can be removed.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/disas/bfd.h |  7 -------
 disas/arm.c         | 21 ++++++++++++++++-----
 target/arm/cpu.c    | 19 -------------------
 3 files changed, 16 insertions(+), 31 deletions(-)

diff --git a/include/disas/bfd.h b/include/disas/bfd.h
index d99da68267..2852f80ed6 100644
--- a/include/disas/bfd.h
+++ b/include/disas/bfd.h
@@ -307,12 +307,6 @@ typedef struct disassemble_info {
     (bfd_vma memaddr, bfd_byte *myaddr, int length,
 	     struct disassemble_info *info);
 
-  /* A place to stash the real read_memory_func if read_memory_func wants to
-     do some funky address arithmetic or similar (e.g. for ARM BE32 mode).  */
-  int (*read_memory_inner_func)
-    (bfd_vma memaddr, bfd_byte *myaddr, int length,
-             struct disassemble_info *info);
-
   /* Function which should be called if we get an error that we can't
      recover from.  STATUS is the errno value from read_memory_func and
      MEMADDR is the address that we were trying to read.  INFO is a
@@ -479,7 +473,6 @@ int generic_symbol_at_address(bfd_vma, struct disassemble_info *);
   (INFO).buffer_vma = 0, \
   (INFO).buffer_length = 0, \
   (INFO).read_memory_func = buffer_read_memory, \
-  (INFO).read_memory_inner_func = NULL, \
   (INFO).memory_error_func = perror_memory, \
   (INFO).print_address_func = generic_print_address, \
   (INFO).print_insn = NULL, \
diff --git a/disas/arm.c b/disas/arm.c
index 27396dd3e1..9967c45990 100644
--- a/disas/arm.c
+++ b/disas/arm.c
@@ -70,6 +70,17 @@ static void floatformat_to_double (unsigned char *data, double *dest)
     *dest = u.f;
 }
 
+static int arm_read_memory(bfd_vma memaddr, bfd_byte *b, int length,
+                           struct disassemble_info *info)
+{
+    assert((info->flags & INSN_ARM_BE32) == 0 || length == 2 || length == 4);
+
+    if ((info->flags & INSN_ARM_BE32) != 0 && length == 2) {
+        memaddr ^= 2;
+    }
+    return info->read_memory_func(memaddr, b, length, info);
+}
+
 /* End of qemu specific additions.  */
 
 struct opcode32
@@ -3810,7 +3821,7 @@ find_ifthen_state (bfd_vma pc, struct disassemble_info *info,
 	  return;
 	}
       addr -= 2;
-      status = info->read_memory_func (addr, (bfd_byte *)b, 2, info);
+      status = arm_read_memory (addr, (bfd_byte *)b, 2, info);
       if (status)
 	return;
 
@@ -3882,7 +3893,7 @@ print_insn_arm (bfd_vma pc, struct disassemble_info *info)
       info->bytes_per_chunk = size;
       printer = print_insn_data;
 
-      status = info->read_memory_func (pc, (bfd_byte *)b, size, info);
+      status = arm_read_memory (pc, (bfd_byte *)b, size, info);
       given = 0;
       if (little)
 	for (i = size - 1; i >= 0; i--)
@@ -3899,7 +3910,7 @@ print_insn_arm (bfd_vma pc, struct disassemble_info *info)
       info->bytes_per_chunk = 4;
       size = 4;
 
-      status = info->read_memory_func (pc, (bfd_byte *)b, 4, info);
+      status = arm_read_memory (pc, (bfd_byte *)b, 4, info);
       if (little)
 	given = (b[0]) | (b[1] << 8) | (b[2] << 16) | ((unsigned)b[3] << 24);
       else
@@ -3915,7 +3926,7 @@ print_insn_arm (bfd_vma pc, struct disassemble_info *info)
       info->bytes_per_chunk = 2;
       size = 2;
 
-      status = info->read_memory_func (pc, (bfd_byte *)b, 2, info);
+      status = arm_read_memory (pc, (bfd_byte *)b, 2, info);
       if (little)
 	given = (b[0]) | (b[1] << 8);
       else
@@ -3929,7 +3940,7 @@ print_insn_arm (bfd_vma pc, struct disassemble_info *info)
 	      || (given & 0xF800) == 0xF000
 	      || (given & 0xF800) == 0xE800)
 	    {
-	      status = info->read_memory_func (pc + 2, (bfd_byte *)b, 2, info);
+	      status = arm_read_memory (pc + 2, (bfd_byte *)b, 2, info);
 	      if (little)
 		given = (b[0]) | (b[1] << 8) | (given << 16);
 	      else
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 88578f360e..82dad0b721 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -473,21 +473,6 @@ print_insn_thumb1(bfd_vma pc, disassemble_info *info)
   return print_insn_arm(pc | 1, info);
 }
 
-static int arm_read_memory_func(bfd_vma memaddr, bfd_byte *b,
-                                int length, struct disassemble_info *info)
-{
-    assert(info->read_memory_inner_func);
-    assert((info->flags & INSN_ARM_BE32) == 0 || length == 2 || length == 4);
-
-    if ((info->flags & INSN_ARM_BE32) != 0 && length == 2) {
-        assert(info->endian == BFD_ENDIAN_LITTLE);
-        return info->read_memory_inner_func(memaddr ^ 2, (bfd_byte *)b, 2,
-                                            info);
-    } else {
-        return info->read_memory_inner_func(memaddr, b, length, info);
-    }
-}
-
 static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
 {
     ARMCPU *ac = ARM_CPU(cpu);
@@ -513,10 +498,6 @@ static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
         info->endian = BFD_ENDIAN_BIG;
 #endif
     }
-    if (info->read_memory_inner_func == NULL) {
-        info->read_memory_inner_func = info->read_memory_func;
-        info->read_memory_func = arm_read_memory_func;
-    }
     info->flags &= ~INSN_ARM_BE32;
     if (arm_sctlr_b(env)) {
         info->flags |= INSN_ARM_BE32;
-- 
2.13.6

[Qemu-devel] [PULL 04/11] target/arm: Don't set INSN_ARM_BE32 for CONFIG_USER_ONLY

[Richard Henderson]

This matches translator behaviour in arm_lduw_code.

Fixes: https://bugs.launchpad.net/qemu/+bug/1724485
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 82dad0b721..a92d86faa0 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -477,6 +477,7 @@ static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
 {
     ARMCPU *ac = ARM_CPU(cpu);
     CPUARMState *env = &ac->env;
+    bool sctlr_b;
 
     if (is_a64(env)) {
         /* We might not be compiled with the A64 disassembler
@@ -491,7 +492,9 @@ static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
     } else {
         info->print_insn = print_insn_arm;
     }
-    if (bswap_code(arm_sctlr_b(env))) {
+
+    sctlr_b = arm_sctlr_b(env);
+    if (bswap_code(sctlr_b)) {
 #ifdef TARGET_WORDS_BIGENDIAN
         info->endian = BFD_ENDIAN_LITTLE;
 #else
@@ -499,9 +502,11 @@ static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
 #endif
     }
     info->flags &= ~INSN_ARM_BE32;
-    if (arm_sctlr_b(env)) {
+#ifndef CONFIG_USER_ONLY
+    if (sctlr_b) {
         info->flags |= INSN_ARM_BE32;
     }
+#endif
 }
 
 uint64_t arm_cpu_mp_affinity(int idx, uint8_t clustersz)
-- 
2.13.6

[Qemu-devel] [PULL 05/11] disas: Remove unused flags arguments

[Richard Henderson]

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

Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
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 88d16fbd57..2b26466b61 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;
@@ -335,10 +329,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 2164dfcc3b..7a802a345e 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 f32c95b9a1..3de369b17e 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 a39b9d3633..fc5419df7f 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -11403,8 +11403,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 4da1a4cbc6..9d31769c8d 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -12371,8 +12371,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 26242f4b3c..ca6a6d3372 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 69a87de83b..e81479a50c 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -8559,7 +8559,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 ac05f3aa09..7c96aff1a0 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -20370,7 +20370,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 54fbe898df..0d2d03d2d0 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 cddbf92564..469ebeb446 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7409,7 +7409,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 6ecf764a98..1ccdb35df2 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -5972,7 +5972,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 8db9fba26e..27067cbd30 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.6

[Qemu-devel] [PULL 06/11] disas: Support the Capstone disassembler library

[Richard Henderson]

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.

Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/disas/bfd.h      |   4 +
 include/disas/capstone.h |  38 ++++++++
 disas.c                  | 219 ++++++++++++++++++++++++++++++++++++++++++++---
 configure                |  26 ++++++
 4 files changed, 274 insertions(+), 13 deletions(-)
 create mode 100644 include/disas/capstone.h

diff --git a/include/disas/bfd.h b/include/disas/bfd.h
index 2852f80ed6..1f88c9e9d5 100644
--- a/include/disas/bfd.h
+++ b/include/disas/bfd.h
@@ -371,6 +371,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 2b26466b61..e392a2926e 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,192 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info)
     return print_insn_objdump(pc, info, "OBJD-T");
 }
 
+#ifdef CONFIG_CAPSTONE
+/* Temporary storage for the capstone library.  This will be alloced via
+   malloc with a size private to the library; thus there's no reason not
+   to share this across calls and across host vs target disassembly.  */
+static __thread cs_insn *cap_insn;
+
+/* Initialize the Capstone library.  */
+/* ??? It would be nice to cache this.  We would need one handle for the
+   host and one for the target.  For most targets we can reset specific
+   parameters via cs_option(CS_OPT_MODE, new_mode), but we cannot change
+   CS_ARCH_* in this way.  Thus we would need to be able to close and
+   re-open the target handle with a different arch for the target in order
+   to handle AArch64 vs AArch32 mode switching.  */
+static cs_err cap_disas_start(disassemble_info *info, csh *handle)
+{
+    cs_mode cap_mode = info->cap_mode;
+    cs_err err;
+
+    cap_mode += (info->endian == BFD_ENDIAN_BIG ? CS_MODE_BIG_ENDIAN
+                 : CS_MODE_LITTLE_ENDIAN);
+
+    err = cs_open(info->cap_arch, cap_mode, handle);
+    if (err != CS_ERR_OK) {
+        return err;
+    }
+
+    /* ??? 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);
+    }
+
+    /* "Disassemble" unknown insns as ".byte W,X,Y,Z".  */
+    cs_option(*handle, CS_OPT_SKIPDATA, CS_OPT_ON);
+
+    /* Allocate temp space for cs_disasm_iter.  */
+    if (cap_insn == NULL) {
+        cap_insn = cs_malloc(*handle);
+        if (cap_insn == NULL) {
+            cs_close(handle);
+            return CS_ERR_MEM;
+        }
+    }
+    return CS_ERR_OK;
+}
+
+/* Disassemble SIZE bytes at PC for the target.  */
+static bool cap_disas_target(disassemble_info *info, uint64_t pc, size_t size)
+{
+    uint8_t cap_buf[1024];
+    csh handle;
+    cs_insn *insn;
+    size_t csize = 0;
+
+    if (cap_disas_start(info, &handle) != CS_ERR_OK) {
+        return false;
+    }
+    insn = cap_insn;
+
+    while (1) {
+        size_t tsize = MIN(sizeof(cap_buf) - csize, size);
+        const uint8_t *cbuf = cap_buf;
+
+        target_read_memory(pc + csize, cap_buf + csize, tsize, info);
+        csize += tsize;
+        size -= tsize;
+
+        while (cs_disasm_iter(handle, &cbuf, &csize, &pc, insn)) {
+            (*info->fprintf_func)(info->stream,
+                                  "0x%08" PRIx64 ":  %-12s %s\n",
+                                  insn->address, insn->mnemonic,
+                                  insn->op_str);
+        }
+
+        /* If the target memory is not consumed, go back for more... */
+        if (size != 0) {
+            /* ... taking care to move any remaining fractional insn
+               to the beginning of the buffer.  */
+            if (csize != 0) {
+                memmove(cap_buf, cbuf, csize);
+            }
+            continue;
+        }
+
+        /* Since the target memory is consumed, we should not have
+           a remaining fractional insn.  */
+        if (csize != 0) {
+            (*info->fprintf_func)(info->stream,
+                "Disassembler disagrees with translator "
+                "over instruction decoding\n"
+                "Please report this to qemu-devel@nongnu.org\n");
+        }
+        break;
+    }
+
+    cs_close(&handle);
+    return true;
+}
+
+/* Disassemble SIZE bytes at CODE for the host.  */
+static bool cap_disas_host(disassemble_info *info, void *code, size_t size)
+{
+    csh handle;
+    const uint8_t *cbuf;
+    cs_insn *insn;
+    uint64_t pc;
+
+    if (cap_disas_start(info, &handle) != CS_ERR_OK) {
+        return false;
+    }
+    insn = cap_insn;
+
+    cbuf = code;
+    pc = (uintptr_t)code;
+
+    while (cs_disasm_iter(handle, &cbuf, &size, &pc, insn)) {
+        (*info->fprintf_func)(info->stream,
+                              "0x%08" PRIx64 ":  %-12s %s\n",
+                              insn->address, insn->mnemonic,
+                              insn->op_str);
+    }
+    if (size != 0) {
+        (*info->fprintf_func)(info->stream,
+            "Disassembler disagrees with TCG over instruction encoding\n"
+            "Please report this to qemu-devel@nongnu.org\n");
+    }
+
+    cs_close(&handle);
+    return true;
+}
+
+#if !defined(CONFIG_USER_ONLY)
+/* Disassemble COUNT insns at PC for the target.  */
+static bool cap_disas_monitor(disassemble_info *info, uint64_t pc, int count)
+{
+    uint8_t cap_buf[32];
+    csh handle;
+    cs_insn *insn;
+    size_t csize = 0;
+
+    if (cap_disas_start(info, &handle) != CS_ERR_OK) {
+        return false;
+    }
+    insn = cap_insn;
+
+    while (1) {
+        /* We want to read memory for one insn, but generically we do not
+           know how much memory that is.  We have a small buffer which is
+           known to be sufficient for all supported targets.  Try to not
+           read beyond the page, Just In Case.  For even more simplicity,
+           ignore the actual target page size and use a 1k boundary.  If
+           that turns out to be insufficient, we'll come back around the
+           loop and read more.  */
+        uint64_t epc = QEMU_ALIGN_UP(pc + csize + 1, 1024);
+        size_t tsize = MIN(sizeof(cap_buf) - csize, epc - pc);
+        const uint8_t *cbuf = cap_buf;
+
+        /* Make certain that we can make progress.  */
+        assert(tsize != 0);
+        info->read_memory_func(pc, cap_buf + csize, tsize, info);
+        csize += tsize;
+
+        if (cs_disasm_iter(handle, &cbuf, &csize, &pc, insn)) {
+            (*info->fprintf_func)(info->stream,
+                                  "0x%08" PRIx64 ":  %-12s %s\n",
+                                  insn->address, insn->mnemonic,
+                                  insn->op_str);
+            if (--count <= 0) {
+                break;
+            }
+        }
+        memmove(cap_buf, cbuf, csize);
+    }
+
+    cs_close(&handle);
+    return true;
+}
+#endif /* !CONFIG_USER_ONLY */
+#else
+# define cap_disas_target(i, p, s)  false
+# define cap_disas_host(i, p, s)  false
+# define cap_disas_monitor(i, p, c)  false
+#endif /* CONFIG_CAPSTONE */
+
 /* Disassemble this for me please... (debugging).  */
 void target_disas(FILE *out, CPUState *cpu, target_ulong code,
                   target_ulong size)
@@ -187,6 +374,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;
@@ -198,6 +387,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_target(&s.info, code, size)) {
+        return;
+    }
+
     if (s.info.print_insn == NULL) {
         s.info.print_insn = print_insn_od_target;
     }
@@ -205,18 +398,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;
@@ -244,6 +425,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;
@@ -281,6 +464,11 @@ void disas(FILE *out, void *code, unsigned long size)
 #elif defined(__hppa__)
     print_insn = print_insn_hppa;
 #endif
+
+    if (s.info.cap_arch >= 0 && cap_disas_host(&s.info, code, size)) {
+        return;
+    }
+
     if (print_insn == NULL) {
         print_insn = print_insn_od_host;
     }
@@ -343,8 +531,9 @@ void monitor_disas(Monitor *mon, CPUState *cpu,
     monitor_disas_is_physical = is_physical;
     s.info.read_memory_func = monitor_read_memory;
     s.info.print_address_func = generic_print_address;
-
     s.info.buffer_vma = pc;
+    s.info.cap_arch = -1;
+    s.info.cap_mode = 0;
 
 #ifdef TARGET_WORDS_BIGENDIAN
     s.info.endian = BFD_ENDIAN_BIG;
@@ -356,6 +545,10 @@ void monitor_disas(Monitor *mon, CPUState *cpu,
         cc->disas_set_info(cpu, &s.info);
     }
 
+    if (s.info.cap_arch >= 0 && cap_disas_monitor(&s.info, pc, 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 03547cea6a..d06ad64058 100755
--- a/configure
+++ b/configure
@@ -375,6 +375,7 @@ opengl_dmabuf="no"
 cpuid_h="no"
 avx2_opt="no"
 zlib="yes"
+capstone=""
 lzo=""
 snappy=""
 bzip2=""
@@ -1294,6 +1295,10 @@ for opt do
           error_exit "vhost-user isn't available on win32"
       fi
   ;;
+  --disable-capstone) capstone="no"
+  ;;
+  --enable-capstone) capstone="yes"
+  ;;
   *)
       echo "ERROR: unknown option $opt"
       echo "Try '$0 --help' for more information"
@@ -1541,6 +1546,7 @@ disabled with --disable-FEATURE, default is enabled if available:
   vxhs            Veritas HyperScale vDisk backend support
   crypto-afalg    Linux AF_ALG crypto backend driver
   vhost-user      vhost-user support
+  capstone        capstone disassembler support
 
 NOTE: The object files are built at the place where configure is launched
 EOF
@@ -4411,6 +4417,22 @@ EOF
 fi
 
 ##########################################
+# capstone
+
+if test "$capstone" != no; then
+  if $pkg_config capstone; then
+    capstone=yes
+    QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
+    LIBS="$($pkg_config --libs capstone) $LIBS"
+  else
+    if test "$capstone" = yes; then
+      feature_not_found capstone
+    fi
+    capstone=no
+  fi
+fi
+
+##########################################
 # check if we have fdatasync
 
 fdatasync=no
@@ -5468,6 +5490,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"
@@ -6142,6 +6165,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.6

[Qemu-devel] [PULL 07/11] i386: Support Capstone in disas_set_info

[Richard Henderson]

Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 disas.c           | 4 ++++
 target/i386/cpu.c | 7 +++++++
 2 files changed, 11 insertions(+)

diff --git a/disas.c b/disas.c
index e392a2926e..63dc573e9f 100644
--- a/disas.c
+++ b/disas.c
@@ -438,9 +438,13 @@ 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;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 13b2f8fbc5..cf890b763b 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: */
 
@@ -4106,6 +4108,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.6

[Qemu-devel] [PULL 08/11] arm: Support Capstone in disas_set_info

[Richard Henderson]

Tested-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 disas.c          |  3 +++
 target/arm/cpu.c | 21 ++++++++++++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/disas.c b/disas.c
index 63dc573e9f..8d9bd4901c 100644
--- a/disas.c
+++ b/disas.c
@@ -450,6 +450,7 @@ void disas(FILE *out, void *code, unsigned long size)
     print_insn = print_insn_ppc;
 #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__)
@@ -457,6 +458,8 @@ void disas(FILE *out, void *code, unsigned long size)
     s.info.mach = bfd_mach_sparc_v9b;
 #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__)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index a92d86faa0..a0ed11c9a5 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)
 {
@@ -487,10 +488,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;
     }
 
     sctlr_b = arm_sctlr_b(env);
-- 
2.13.6

[Qemu-devel] [PULL 09/11] ppc: Support Capstone in disas_set_info

[Richard Henderson]

Cc: qemu-ppc@nongnu.org
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 disas.c                     | 4 ++++
 target/ppc/translate_init.c | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/disas.c b/disas.c
index 8d9bd4901c..e52e776a60 100644
--- a/disas.c
+++ b/disas.c
@@ -448,6 +448,10 @@ void disas(FILE *out, void *code, unsigned long size)
 #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;
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index 9b4353437a..41f46193a1 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
@@ -10537,6 +10538,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.6

[Qemu-devel] [PULL 10/11] disas: Remove monitor_disas_is_physical

[Richard Henderson]

Even though there is only one monitor, and thus no race on this
global data object, there is also no point in having it.  We can
just as well record the decision in the read_memory_function that
we select.

Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 disas.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/disas.c b/disas.c
index e52e776a60..92b389d25f 100644
--- a/disas.c
+++ b/disas.c
@@ -512,19 +512,11 @@ const char *lookup_symbol(target_ulong orig_addr)
 
 #include "monitor/monitor.h"
 
-static int monitor_disas_is_physical;
-
 static int
-monitor_read_memory (bfd_vma memaddr, bfd_byte *myaddr, int length,
+physical_read_memory(bfd_vma memaddr, bfd_byte *myaddr, int length,
                      struct disassemble_info *info)
 {
-    CPUDebug *s = container_of(info, CPUDebug, info);
-
-    if (monitor_disas_is_physical) {
-        cpu_physical_memory_read(memaddr, myaddr, length);
-    } else {
-        cpu_memory_rw_debug(s->cpu, memaddr, myaddr, length, 0);
-    }
+    cpu_physical_memory_read(memaddr, myaddr, length);
     return 0;
 }
 
@@ -539,8 +531,8 @@ void monitor_disas(Monitor *mon, CPUState *cpu,
     INIT_DISASSEMBLE_INFO(s.info, (FILE *)mon, monitor_fprintf);
 
     s.cpu = cpu;
-    monitor_disas_is_physical = is_physical;
-    s.info.read_memory_func = monitor_read_memory;
+    s.info.read_memory_func
+        = (is_physical ? physical_read_memory : target_read_memory);
     s.info.print_address_func = generic_print_address;
     s.info.buffer_vma = pc;
     s.info.cap_arch = -1;
-- 
2.13.6

[Qemu-devel] [PULL 11/11] disas: Add capstone as submodule

[Richard Henderson]

Do not require the submodule, but use it if present.  Allow the
command-line to override system or git submodule either way.

Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 Makefile    | 15 +++++++++++++++
 .gitmodules |  3 +++
 capstone    |  1 +
 configure   | 60 +++++++++++++++++++++++++++++++++++++++++++++++++-----------
 4 files changed, 68 insertions(+), 11 deletions(-)
 create mode 160000 capstone

diff --git a/Makefile b/Makefile
index 9372742f86..33b4ce6e6e 100644
--- a/Makefile
+++ b/Makefile
@@ -383,6 +383,21 @@ subdir-dtc: .git-submodule-status dtc/libfdt dtc/tests
 dtc/%: .git-submodule-status
 	mkdir -p $@
 
+# Overriding CFLAGS causes us to lose defines added in the sub-makefile.
+# Not overriding CFLAGS leads to mis-matches between compilation modes.
+# Therefore we replicate some of the logic in the sub-makefile.
+# Remove all the extra -Warning flags that QEMU uses that Capstone doesn't;
+# no need to annoy QEMU developers with such things.
+CAP_CFLAGS = $(patsubst -W%,,$(CFLAGS) $(QEMU_CFLAGS))
+CAP_CFLAGS += -DCAPSTONE_USE_SYS_DYN_MEM
+CAP_CFLAGS += -DCAPSTONE_HAS_ARM
+CAP_CFLAGS += -DCAPSTONE_HAS_ARM64
+CAP_CFLAGS += -DCAPSTONE_HAS_POWERPC
+CAP_CFLAGS += -DCAPSTONE_HAS_X86
+
+subdir-capstone: .git-submodule-status
+	$(call quiet-command,$(MAKE) -C $(SRC_PATH)/capstone CAPSTONE_SHARED=no BUILDDIR="$(BUILD_DIR)/capstone" CC="$(CC)" AR="$(AR)" LD="$(LD)" CFLAGS="$(CAP_CFLAGS)" $(SUBDIR_MAKEFLAGS) $(BUILD_DIR)/capstone/libcapstone.a)
+
 $(SUBDIR_RULES): libqemuutil.a $(common-obj-y) $(chardev-obj-y) \
 	$(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY))
 
diff --git a/.gitmodules b/.gitmodules
index 7c981a42b6..1500579638 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -37,3 +37,6 @@
 [submodule "ui/keycodemapdb"]
 	path = ui/keycodemapdb
 	url = git://git.qemu.org/keycodemapdb.git
+[submodule "capstone"]
+	path = capstone
+	url = git://git.qemu.org/capstone.git
diff --git a/capstone b/capstone
new file mode 160000
index 0000000000..22ead3e0bf
--- /dev/null
+++ b/capstone
@@ -0,0 +1 @@
+Subproject commit 22ead3e0bfdb87516656453336160e0a37b066bf
diff --git a/configure b/configure
index d06ad64058..9ab10b0261 100755
--- a/configure
+++ b/configure
@@ -1299,6 +1299,10 @@ for opt do
   ;;
   --enable-capstone) capstone="yes"
   ;;
+  --enable-capstone=git) capstone="git"
+  ;;
+  --enable-capstone=system) capstone="system"
+  ;;
   *)
       echo "ERROR: unknown option $opt"
       echo "Try '$0 --help' for more information"
@@ -4419,18 +4423,49 @@ fi
 ##########################################
 # capstone
 
-if test "$capstone" != no; then
-  if $pkg_config capstone; then
-    capstone=yes
+case "$capstone" in
+  "" | yes)
+    if $pkg_config capstone; then
+      capstone=system
+    elif test -e "${source_path}/.git" ; then
+      capstone=git
+    elif test -e "${source_path}/capstone/Makefile" ; then
+      capstone=internal
+    elif test -z "$capstone" ; then
+      capstone=no
+    else
+      feature_not_found "capstone" "Install capstone devel or git submodule"
+    fi
+    ;;
+
+  system)
+    if ! $pkg_config capstone; then
+      feature_not_found "capstone" "Install capstone devel"
+    fi
+    ;;
+esac
+
+case "$capstone" in
+  git | internal)
+    if test "$capstone" = git; then
+      git_submodules="${git_submodules} capstone"
+    fi
+    mkdir -p capstone
+    QEMU_CFLAGS="$QEMU_CFLAGS -I\$(SRC_PATH)/capstone/include"
+    LIBS="\$(BUILD_DIR)/capstone/libcapstone.a $LIBS"
+    ;;
+
+  system)
     QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
     LIBS="$($pkg_config --libs capstone) $LIBS"
-  else
-    if test "$capstone" = yes; then
-      feature_not_found capstone
-    fi
-    capstone=no
-  fi
-fi
+    ;;
+
+  no)
+    ;;
+  *)
+    error_exit "Unknown state for capstone: $capstone"
+    ;;
+esac
 
 ##########################################
 # check if we have fdatasync
@@ -6165,7 +6200,7 @@ fi
 if test "$ivshmem" = "yes" ; then
   echo "CONFIG_IVSHMEM=y" >> $config_host_mak
 fi
-if test "$capstone" = "yes" ; then
+if test "$capstone" != "no" ; then
   echo "CONFIG_CAPSTONE=y" >> $config_host_mak
 fi
 
@@ -6650,6 +6685,9 @@ done # for target in $targets
 if [ "$dtc_internal" = "yes" ]; then
   echo "config-host.h: subdir-dtc" >> $config_host_mak
 fi
+if [ "$capstone" = "git" -o "$capstone" = "internal" ]; then
+  echo "config-host.h: subdir-capstone" >> $config_host_mak
+fi
 
 if test "$numa" = "yes"; then
   echo "CONFIG_NUMA=y" >> $config_host_mak
-- 
2.13.6

Re: [Qemu-devel] [PULL 00/11] Disassembler patches

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PULL 00/11] Disassembler patches
Type: series
Message-id: 20171025123056.3165-1-richard.henderson@linaro.org

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/1508006342-5304-1-git-send-email-mark.cave-ayland@ilande.co.uk -> patchew/1508006342-5304-1-git-send-email-mark.cave-ayland@ilande.co.uk
 * [new tag]               patchew/20171025123056.3165-1-richard.henderson@linaro.org -> patchew/20171025123056.3165-1-richard.henderson@linaro.org
Switched to a new branch 'test'
1b87049c52 disas: Add capstone as submodule
1a149369df disas: Remove monitor_disas_is_physical
6c35e6706c ppc: Support Capstone in disas_set_info
52e10752ca arm: Support Capstone in disas_set_info
4b31f39e81 i386: Support Capstone in disas_set_info
279e046dc1 disas: Support the Capstone disassembler library
4da4345ecf disas: Remove unused flags arguments
9e561441a6 target/arm: Don't set INSN_ARM_BE32 for CONFIG_USER_ONLY
9c25d63ac2 target/arm: Move BE32 disassembler fixup
6836256946 target/ppc: Convert to disas_set_info hook
667b551cc7 target/i386: Convert to disas_set_info hook

=== OUTPUT BEGIN ===
Checking PATCH 1/11: target/i386: Convert to disas_set_info hook...
Checking PATCH 2/11: target/ppc: Convert to disas_set_info hook...
Checking PATCH 3/11: target/arm: Move BE32 disassembler fixup...
ERROR: space prohibited between function name and open parenthesis '('
#46: FILE: disas/arm.c:3824:
+      status = arm_read_memory (addr, (bfd_byte *)b, 2, info);

ERROR: space prohibited between function name and open parenthesis '('
#55: FILE: disas/arm.c:3896:
+      status = arm_read_memory (pc, (bfd_byte *)b, size, info);

ERROR: space prohibited between function name and open parenthesis '('
#64: FILE: disas/arm.c:3913:
+      status = arm_read_memory (pc, (bfd_byte *)b, 4, info);

ERROR: space prohibited between function name and open parenthesis '('
#73: FILE: disas/arm.c:3929:
+      status = arm_read_memory (pc, (bfd_byte *)b, 2, info);

ERROR: code indent should never use tabs
#82: FILE: disas/arm.c:3943:
+^I      status = arm_read_memory (pc + 2, (bfd_byte *)b, 2, info);$

ERROR: space prohibited between function name and open parenthesis '('
#82: FILE: disas/arm.c:3943:
+	      status = arm_read_memory (pc + 2, (bfd_byte *)b, 2, info);

total: 6 errors, 0 warnings, 107 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 4/11: target/arm: Don't set INSN_ARM_BE32 for CONFIG_USER_ONLY...
Checking PATCH 5/11: disas: Remove unused flags arguments...
Checking PATCH 6/11: disas: Support the Capstone disassembler library...
Checking PATCH 7/11: i386: Support Capstone in disas_set_info...
Checking PATCH 8/11: arm: Support Capstone in disas_set_info...
Checking PATCH 9/11: ppc: Support Capstone in disas_set_info...
Checking PATCH 10/11: disas: Remove monitor_disas_is_physical...
Checking PATCH 11/11: disas: Add capstone as submodule...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PULL 00/11] Disassembler patches

[Peter Maydell]

On 25 October 2017 at 13:30, Richard Henderson
<richard.henderson@linaro.org> wrote:
> Support for Capstone, plus an arm32 fix.
>
>
> r~
>
>
> The following changes since commit 3d7196d43bfe12efe98568cb60057e273652b99b:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/usb-20171023-pull-request' into staging (2017-10-24 16:05:57 +0100)
>
> are available in the git repository at:
>
>   git://github.com/rth7680/qemu.git tags/pull-dis-20171025
>
> for you to fetch changes up to 383b90bc6a15f4b18ec34f9c0287b26f9a89fcb8:
>
>   disas: Add capstone as submodule (2017-10-25 11:55:21 +0200)
>
> ----------------------------------------------------------------
> Capstone disassembler


Hi. This failed to build for Windows:

make[1]: *** No rule to make target
'/home/petmay01/linaro/qemu-for-merges/build/w32-new/capstone/libcapstone.a'.
Stop.
Makefile:399: recipe for target 'subdir-capstone' failed

FreeBSD ar also prints a warning:
  AR      libcapstone.a
ar: warning: creating /root/qemu/build/all/capstone/libcapstone.a

though the build otherwise succeeds. Is it possible to silence this?
(Otherwise it shows up in my build output scripts; I could silence it
there but it would be neater if it just wasn't emitted.)

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/11] Disassembler patches

[Peter Maydell]

On 26 October 2017 at 07:07, Peter Maydell <peter.maydell@linaro.org> wrote:
> Hi. This failed to build for Windows:

Also, after it failed and I backed out the merge, the next thing I
tried to build failed everywhere with:

warning: unable to rmdir capstone: Directory not empty
error: pathspec 'capstone' did not match any file(s) known to git.
error: pathspec 'capstone' did not match any file(s) known to git.

That's bad, because it suggests that bisection is going to break
across this merge commit.

Dan, is this going to be a generic problem with the new submodule
stuff, or is it specific to something with how the capstone
submodule is being handled in this patchset?

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/11] Disassembler patches

[Daniel P. Berrange]

On Thu, Oct 26, 2017 at 07:16:45AM +0100, Peter Maydell wrote:
> On 26 October 2017 at 07:07, Peter Maydell <peter.maydell@linaro.org> wrote:
> > Hi. This failed to build for Windows:
> 
> Also, after it failed and I backed out the merge, the next thing I
> tried to build failed everywhere with:
> 
> warning: unable to rmdir capstone: Directory not empty
> error: pathspec 'capstone' did not match any file(s) known to git.
> error: pathspec 'capstone' did not match any file(s) known to git.
> 
> That's bad, because it suggests that bisection is going to break
> across this merge commit.
> 
> Dan, is this going to be a generic problem with the new submodule
> stuff, or is it specific to something with how the capstone
> submodule is being handled in this patchset?

If you make your checkout go back in time to before the submodule
existed, git won't delete your checked out submodule - in case you
have commits there you still care about. So it'll print that warning
about unable to 'rmdir'. This should be harmless to the build process
in general though.

I'm not sure what's giving you the 'pathspec' message though ? I would
expect anything to ignore the capstone dir - its just like any other
untracked file once you go back in time before it was committed.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PULL 00/11] Disassembler patches

[Richard Henderson]

On 10/26/2017 08:07 AM, Peter Maydell wrote:
> On 25 October 2017 at 13:30, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>> Support for Capstone, plus an arm32 fix.
>>
>>
>> r~
>>
>>
>> The following changes since commit 3d7196d43bfe12efe98568cb60057e273652b99b:
>>
>>   Merge remote-tracking branch 'remotes/kraxel/tags/usb-20171023-pull-request' into staging (2017-10-24 16:05:57 +0100)
>>
>> are available in the git repository at:
>>
>>   git://github.com/rth7680/qemu.git tags/pull-dis-20171025
>>
>> for you to fetch changes up to 383b90bc6a15f4b18ec34f9c0287b26f9a89fcb8:
>>
>>   disas: Add capstone as submodule (2017-10-25 11:55:21 +0200)
>>
>> ----------------------------------------------------------------
>> Capstone disassembler
> 
> 
> Hi. This failed to build for Windows:
> 
> make[1]: *** No rule to make target
> '/home/petmay01/linaro/qemu-for-merges/build/w32-new/capstone/libcapstone.a'.
> Stop.
> Makefile:399: recipe for target 'subdir-capstone' failed

Ug.  Ok, capstone is forcing the use of .lib for windows.

> 
> FreeBSD ar also prints a warning:
>   AR      libcapstone.a
> ar: warning: creating /root/qemu/build/all/capstone/libcapstone.a

Feh.  Capstone has no ARFLAGS variable.  So, no, I can't fix this without
actually modifying the sub-makefile.


r~

Re: [Qemu-devel] [PULL 00/11] Disassembler patches

[Peter Maydell]

On 26 October 2017 at 08:06, Daniel P. Berrange <berrange@redhat.com> wrote:
> I'm not sure what's giving you the 'pathspec' message though ? I would
> expect anything to ignore the capstone dir - its just like any other
> untracked file once you go back in time before it was committed.

Sorry, just realized that was the output of my filtering of the
log. Here's what I should have quoted:

>From git://git.linaro.org/people/pmaydell/qemu-arm
 + a872ea5...6315472 staging    -> pmaydell/staging  (forced update)
warning: unable to rmdir capstone: Directory not empty
error: pathspec 'capstone' did not match any file(s) known to git.
Did you forget to 'git add'?
make: Entering directory `/home/pm215/qemu/build/all'
config-host.mak is out-of-date, running configure
  GIT     ui/keycodemapdb dtc capstone
make[1]: *** No rule to make target `all'.  Stop.
error: pathspec 'capstone' did not match any file(s) known to git.
Did you forget to 'git add'?
make: *** [git-submodule-update] Error 1
make: *** Waiting for unfinished jobs....
make: *** [subdir-capstone] Error 2
Install prefix    /usr/local
BIOS directory    /usr/local/share/qemu
firmware path     /usr/local/share/qemu-firmware
binary directory  /usr/local/bin
[rest of configure output skipped]
replication support yes
VxHS block device no
make: Leaving directory `/home/pm215/qemu/build/all'

It looks like the git update script thinks that there ought to
be a 'capstone' submodule, which of course there isn't any more,
so it barfs trying to update it.

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/11] Disassembler patches

[Richard Henderson]

On 10/26/2017 09:21 AM, Peter Maydell wrote:
> On 26 October 2017 at 08:06, Daniel P. Berrange <berrange@redhat.com> wrote:
>> I'm not sure what's giving you the 'pathspec' message though ? I would
>> expect anything to ignore the capstone dir - its just like any other
>> untracked file once you go back in time before it was committed.
> 
> Sorry, just realized that was the output of my filtering of the
> log. Here's what I should have quoted:
> 
> From git://git.linaro.org/people/pmaydell/qemu-arm
>  + a872ea5...6315472 staging    -> pmaydell/staging  (forced update)
> warning: unable to rmdir capstone: Directory not empty
> error: pathspec 'capstone' did not match any file(s) known to git.
> Did you forget to 'git add'?
> make: Entering directory `/home/pm215/qemu/build/all'
> config-host.mak is out-of-date, running configure
>   GIT     ui/keycodemapdb dtc capstone
> make[1]: *** No rule to make target `all'.  Stop.
> error: pathspec 'capstone' did not match any file(s) known to git.
> Did you forget to 'git add'?
> make: *** [git-submodule-update] Error 1
> make: *** Waiting for unfinished jobs....
> make: *** [subdir-capstone] Error 2

Looks like we need to make git-submodule-update depend on config-host.mak, so
that we have already re-run configure, so that vanishing modules will have been
de-selected.

Forcing a configure re-run by hand would have masked this problem.  I think all
of the testing that I have been doing has been builds from empty build directories.


r~

Re: [Qemu-devel] [PULL 00/11] Disassembler patches

[Daniel P. Berrange]

On Thu, Oct 26, 2017 at 08:21:48AM +0100, Peter Maydell wrote:
> On 26 October 2017 at 08:06, Daniel P. Berrange <berrange@redhat.com> wrote:
> > I'm not sure what's giving you the 'pathspec' message though ? I would
> > expect anything to ignore the capstone dir - its just like any other
> > untracked file once you go back in time before it was committed.
> 
> Sorry, just realized that was the output of my filtering of the
> log. Here's what I should have quoted:
> 
> From git://git.linaro.org/people/pmaydell/qemu-arm
>  + a872ea5...6315472 staging    -> pmaydell/staging  (forced update)
> warning: unable to rmdir capstone: Directory not empty
> error: pathspec 'capstone' did not match any file(s) known to git.
> Did you forget to 'git add'?
> make: Entering directory `/home/pm215/qemu/build/all'
> config-host.mak is out-of-date, running configure
>   GIT     ui/keycodemapdb dtc capstone
> make[1]: *** No rule to make target `all'.  Stop.
> error: pathspec 'capstone' did not match any file(s) known to git.
> Did you forget to 'git add'?
> make: *** [git-submodule-update] Error 1
> make: *** Waiting for unfinished jobs....
> make: *** [subdir-capstone] Error 2
> Install prefix    /usr/local
> BIOS directory    /usr/local/share/qemu
> firmware path     /usr/local/share/qemu-firmware
> binary directory  /usr/local/bin
> [rest of configure output skipped]
> replication support yes
> VxHS block device no
> make: Leaving directory `/home/pm215/qemu/build/all'
> 
> It looks like the git update script thinks that there ought to
> be a 'capstone' submodule, which of course there isn't any more,
> so it barfs trying to update it.

Yeah, ok that makes more sense.  The 'config-host.mak' rules have the
list of desired submodules and those correspond to the state when you
ran configure with the patches applied.

Do we really expect make/configure todo the right thing when going
backwards in time ?  I've always assumed that if you go back in time
when you need to do a 'git clean -f -x d' and re-run configure from
scratch. Certainly in the past various makefile changes in QEMU would
break, or silently not correctly recompile stuff when going backwards
in time.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PULL 00/11] Disassembler patches

[Philippe Mathieu-Daudé]

On 10/26/2017 07:06 AM, Daniel P. Berrange wrote:
> Do we really expect make/configure todo the right thing when going
> backwards in time ?

"Yes"? Ideally at least :)

> I've always assumed that if you go back in time
> when you need to do a 'git clean -f -x d' and re-run configure from> scratch.
I certainly don't do that and would rather not have to think about it.

I also don't think about calling it when bisecting, however looking at
the man page, this is somehow suggested in the example:

·   Automatically bisect with temporary modifications (hot-fix):
[...]
        # undo the tweak to allow clean flipping to the next commit
        git reset --hard

This could be a warning displayed via a post-checkout hook, for this
particular merge hash, or if you go way too back in time...

> Certainly in the past various makefile changes in QEMU would> break, or silently not correctly recompile stuff when going backwards
> in time.

Travis is already taking too long, but we could add a such test
(merge/build/checkout backward/configure/quick build), not sure if we
gain much.

Re: [Qemu-devel] [PULL 00/11] Disassembler patches

[Daniel P. Berrange]

On Thu, Oct 26, 2017 at 10:29:59AM -0300, Philippe Mathieu-Daudé wrote:
> On 10/26/2017 07:06 AM, Daniel P. Berrange wrote:
> > Do we really expect make/configure todo the right thing when going
> > backwards in time ?
> 
> "Yes"? Ideally at least :)

What we could do is get scripts/git-submodule.sh to filter the list of
modules it receives, against the list of modules that exist. This would
probably make it safe against going backwards over a commit that added
a new submodule


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PULL 00/11] Disassembler patches

[Peter Maydell]

On 26 October 2017 at 11:06, Daniel P. Berrange <berrange@redhat.com> wrote:
> Do we really expect make/configure todo the right thing when going
> backwards in time ?  I've always assumed that if you go back in time
> when you need to do a 'git clean -f -x d' and re-run configure from
> scratch. Certainly in the past various makefile changes in QEMU would
> break, or silently not correctly recompile stuff when going backwards
> in time.

Yes. Occasionally we accidentally break this, as you note, and then
git bisection across that kind of boundary requires a painful build
from clean. But almost always bisection (across small spans of history)
works without having to do that, as do things like "check out this
branch I was working on a month ago and do a build without bothering
to clean first".

In this case you broke my standard workflow for rolling back from
an attempted merge that didn't actually build, which IME is a pretty
rare thing to have go wrong.

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/11] Disassembler patches

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1275 bytes --]

On 10/25/2017 02:30 PM, Richard Henderson wrote:
> Support for Capstone, plus an arm32 fix.
> 

> ----------------------------------------------------------------
> Capstone disassembler
> 
> ----------------------------------------------------------------
> Richard Henderson (11):
>       target/i386: Convert to disas_set_info hook
>       target/ppc: Convert to disas_set_info hook
>       target/arm: Move BE32 disassembler fixup
>       target/arm: Don't set INSN_ARM_BE32 for CONFIG_USER_ONLY
>       disas: Remove unused flags arguments
>       disas: Support the Capstone disassembler library
>       i386: Support Capstone in disas_set_info
>       arm: Support Capstone in disas_set_info
>       ppc: Support Capstone in disas_set_info
>       disas: Remove monitor_disas_is_physical
>       disas: Add capstone as submodule

I think (but haven't bisected) that this series is now making 'make' noisy:

$ make -j3 qemu-nbd
make[1]: '/home/eblake/qemu-tmp/capstone/libcapstone.a' is up to date.
make: 'qemu-nbd' is up to date.

Can we figure out how to shut up make when libcapstone.a has nothing needed?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]