[Qemu-devel] [PATCH v2 0/7] Unify and QOMify (target|monitor)_disas

[Qemu-devel] [PATCH v2 0/7] Unify and QOMify (target|monitor)_disas

[Peter Crosthwaite]

These two functions are mostly trying to do the same thing, which is
disassemble a target instruction (sequence) for printfing. The
architecture specific setup is largely duped between the two functions.

The approach is to add a single QOM hook on the CPU level to setup the
disassembler (P1&2). The two stage flags system is removed. That is,
the old scheme, is for the translate/montitor code to pass in flags
that disas.c then interprets. Instead the entire job of setting up arch
specifics is outsourced to target-specific code (via the new QOM hook)
removing the need for the flags system. Both monitor_disas and
target_disas then calls this singly defined hook if it is available.

Three architectures (microblaze, cris and ARM) are patched
to use the new QOMification and at the same time, make the
monitor_disas consistent with target_disas. The #if defined TARGET_FOO
for each is removed from disas.c (bringing us closer to the exciting
goal of no #ifdef TARGET_FOO in system mode code).

Microblaze is trivial, the target_disas setup is directly applicable
to monitor_disas to bring in microblaze monitor disas support (P5).

Cris had a small hiccup, a patch is needed to handle monitor_disas's
0 buffer length (P6). Then cris is patched to enable monitor disas
in same way as microblaze (P7).

ARM is the harder. The vixl A64 disas was hardcoded to fprintf with
a statically inited output stream (matching target_disas). The vixl
printfery is patched to be runtime variable (P3). P4 brings
ARM monitor disassembly online (via using the target_disas
implementation as the QOMified implementation).

Changed since v1 (RTH review):
Use QOMified approach.
Remove flags system.
Limit scope to only the 3 converted arches
Addressed comments on CPP constructor changes

Peter Crosthwaite (7):
  disas: Add print_insn to disassemble info
  disas: QOMify target specific setup
  disas: arm-a64: Make printfer and stream variable
  disas: arm: QOMify target specific disas setup
  disas: microblaze: QOMify target specific disas setup
  disas: cris: Fix 0 buffer length case
  disas: cris: QOMify target specific disas setup

 disas.c                 | 121 ++++++++++++++++++------------------------------
 disas/arm-a64.cc        |  25 ++++++++--
 disas/cris.c            |   6 +--
 include/disas/bfd.h     |   6 +++
 include/qom/cpu.h       |   4 ++
 target-arm/cpu.c        |  35 ++++++++++++++
 target-cris/cpu.c       |  16 +++++++
 target-microblaze/cpu.c |   8 ++++
 8 files changed, 138 insertions(+), 83 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v2 1/7] disas: Add print_insn to disassemble info

[Peter Crosthwaite]

Add the print_insn pointer to the disassemble info structure. This is
to prepare for QOMification support, where a QOM CPU hook function will
be responsible for setting the print_insn function. Add this function
to the existing struct to consolidate such that only the one struct
needs to be passed to the new QOM API.

Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 disas.c             | 68 ++++++++++++++++++++++++++---------------------------
 include/disas/bfd.h |  6 +++++
 2 files changed, 39 insertions(+), 35 deletions(-)

diff --git a/disas.c b/disas.c
index 44a019a..32c6518 100644
--- a/disas.c
+++ b/disas.c
@@ -201,7 +201,6 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code,
     target_ulong pc;
     int count;
     CPUDebug s;
-    int (*print_insn)(bfd_vma pc, disassemble_info *info) = NULL;
 
     INIT_DISASSEMBLE_INFO(s.info, out, fprintf);
 
@@ -224,7 +223,7 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code,
     } else {
         s.info.mach = bfd_mach_i386_i386;
     }
-    print_insn = print_insn_i386;
+    s.info.print_insn = print_insn_i386;
 #elif defined(TARGET_ARM)
     if (flags & 4) {
         /* We might not be compiled with the A64 disassembler
@@ -232,12 +231,12 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code,
          * fall through to the default print_insn_od case.
          */
 #if defined(CONFIG_ARM_A64_DIS)
-        print_insn = print_insn_arm_a64;
+        s.info.print_insn = print_insn_arm_a64;
 #endif
     } else if (flags & 1) {
-        print_insn = print_insn_thumb1;
+        s.info.print_insn = print_insn_thumb1;
     } else {
-        print_insn = print_insn_arm;
+        s.info.print_insn = print_insn_arm;
     }
     if (flags & 2) {
 #ifdef TARGET_WORDS_BIGENDIAN
@@ -247,7 +246,7 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code,
 #endif
     }
 #elif defined(TARGET_SPARC)
-    print_insn = print_insn_sparc;
+    s.info.print_insn = print_insn_sparc;
 #ifdef TARGET_SPARC64
     s.info.mach = bfd_mach_sparc_v9b;
 #endif
@@ -266,49 +265,49 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code,
 #endif
     }
     s.info.disassembler_options = (char *)"any";
-    print_insn = print_insn_ppc;
+    s.info.print_insn = print_insn_ppc;
 #elif defined(TARGET_M68K)
-    print_insn = print_insn_m68k;
+    s.info.print_insn = print_insn_m68k;
 #elif defined(TARGET_MIPS)
 #ifdef TARGET_WORDS_BIGENDIAN
-    print_insn = print_insn_big_mips;
+    s.info.print_insn = print_insn_big_mips;
 #else
-    print_insn = print_insn_little_mips;
+    s.info.print_insn = print_insn_little_mips;
 #endif
 #elif defined(TARGET_SH4)
     s.info.mach = bfd_mach_sh4;
-    print_insn = print_insn_sh;
+    s.info.print_insn = print_insn_sh;
 #elif defined(TARGET_ALPHA)
     s.info.mach = bfd_mach_alpha_ev6;
-    print_insn = print_insn_alpha;
+    s.info.print_insn = print_insn_alpha;
 #elif defined(TARGET_CRIS)
     if (flags != 32) {
         s.info.mach = bfd_mach_cris_v0_v10;
-        print_insn = print_insn_crisv10;
+        s.info.print_insn = print_insn_crisv10;
     } else {
         s.info.mach = bfd_mach_cris_v32;
-        print_insn = print_insn_crisv32;
+        s.info.print_insn = print_insn_crisv32;
     }
 #elif defined(TARGET_S390X)
     s.info.mach = bfd_mach_s390_64;
-    print_insn = print_insn_s390;
+    s.info.print_insn = print_insn_s390;
 #elif defined(TARGET_MICROBLAZE)
     s.info.mach = bfd_arch_microblaze;
-    print_insn = print_insn_microblaze;
+    s.info.print_insn = print_insn_microblaze;
 #elif defined(TARGET_MOXIE)
     s.info.mach = bfd_arch_moxie;
-    print_insn = print_insn_moxie;
+    s.info.print_insn = print_insn_moxie;
 #elif defined(TARGET_LM32)
     s.info.mach = bfd_mach_lm32;
-    print_insn = print_insn_lm32;
+    s.info.print_insn = print_insn_lm32;
 #endif
-    if (print_insn == NULL) {
-        print_insn = print_insn_od_target;
+    if (s.info.print_insn == NULL) {
+        s.info.print_insn = print_insn_od_target;
     }
 
     for (pc = code; size > 0; pc += count, size -= count) {
 	fprintf(out, "0x" TARGET_FMT_lx ":  ", pc);
-	count = print_insn(pc, &s.info);
+	count = s.info.print_insn(pc, &s.info);
 #if 0
         {
             int i;
@@ -452,7 +451,6 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
 {
     int count, i;
     CPUDebug s;
-    int (*print_insn)(bfd_vma pc, disassemble_info *info);
 
     INIT_DISASSEMBLE_INFO(s.info, (FILE *)mon, monitor_fprintf);
 
@@ -476,13 +474,13 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
     } else {
         s.info.mach = bfd_mach_i386_i386;
     }
-    print_insn = print_insn_i386;
+    s.info.print_insn = print_insn_i386;
 #elif defined(TARGET_ARM)
-    print_insn = print_insn_arm;
+    s.info.print_insn = print_insn_arm;
 #elif defined(TARGET_ALPHA)
-    print_insn = print_insn_alpha;
+    s.info.print_insn = print_insn_alpha;
 #elif defined(TARGET_SPARC)
-    print_insn = print_insn_sparc;
+    s.info.print_insn = print_insn_sparc;
 #ifdef TARGET_SPARC64
     s.info.mach = bfd_mach_sparc_v9b;
 #endif
@@ -500,27 +498,27 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
     if ((flags >> 16) & 1) {
         s.info.endian = BFD_ENDIAN_LITTLE;
     }
-    print_insn = print_insn_ppc;
+    s.info.print_insn = print_insn_ppc;
 #elif defined(TARGET_M68K)
-    print_insn = print_insn_m68k;
+    s.info.print_insn = print_insn_m68k;
 #elif defined(TARGET_MIPS)
 #ifdef TARGET_WORDS_BIGENDIAN
-    print_insn = print_insn_big_mips;
+    s.info.print_insn = print_insn_big_mips;
 #else
-    print_insn = print_insn_little_mips;
+    s.info.print_insn = print_insn_little_mips;
 #endif
 #elif defined(TARGET_SH4)
     s.info.mach = bfd_mach_sh4;
-    print_insn = print_insn_sh;
+    s.info.print_insn = print_insn_sh;
 #elif defined(TARGET_S390X)
     s.info.mach = bfd_mach_s390_64;
-    print_insn = print_insn_s390;
+    s.info.print_insn = print_insn_s390;
 #elif defined(TARGET_MOXIE)
     s.info.mach = bfd_arch_moxie;
-    print_insn = print_insn_moxie;
+    s.info.print_insn = print_insn_moxie;
 #elif defined(TARGET_LM32)
     s.info.mach = bfd_mach_lm32;
-    print_insn = print_insn_lm32;
+    s.info.print_insn = print_insn_lm32;
 #else
     monitor_printf(mon, "0x" TARGET_FMT_lx
                    ": Asm output not supported on this arch\n", pc);
@@ -529,7 +527,7 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
 
     for(i = 0; i < nb_insn; i++) {
 	monitor_printf(mon, "0x" TARGET_FMT_lx ":  ", pc);
-        count = print_insn(pc, &s.info);
+        count = s.info.print_insn(pc, &s.info);
 	monitor_printf(mon, "\n");
 	if (count < 0)
 	    break;
diff --git a/include/disas/bfd.h b/include/disas/bfd.h
index 8bd703c..a112e9c 100644
--- a/include/disas/bfd.h
+++ b/include/disas/bfd.h
@@ -313,6 +313,11 @@ typedef struct disassemble_info {
   void (*print_address_func)
     (bfd_vma addr, struct disassemble_info *info);
 
+    /* Function called to print an instruction. The function is architecture
+     * specific.
+     */
+    int (*print_insn)(bfd_vma addr, struct disassemble_info *info);
+
   /* Function called to determine if there is a symbol at the given ADDR.
      If there is, the function returns 1, otherwise it returns 0.
      This is used by ports which support an overlay manager where
@@ -463,6 +468,7 @@ int generic_symbol_at_address(bfd_vma, struct disassemble_info *);
   (INFO).read_memory_func = buffer_read_memory, \
   (INFO).memory_error_func = perror_memory, \
   (INFO).print_address_func = generic_print_address, \
+  (INFO).print_insn = NULL, \
   (INFO).symbol_at_address_func = generic_symbol_at_address, \
   (INFO).flags = 0, \
   (INFO).bytes_per_line = 0, \
-- 
1.9.1

[Qemu-devel] [PATCH v2 2/7] disas: QOMify target specific setup

[Peter Crosthwaite]

Add a QOM function hook for target-specific disassembly setup. This
allows removal of the #ifdeffery currently implementing target specific
disas setup from disas.c.

Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 disas.c           | 24 ++++++++++++++++++++----
 include/qom/cpu.h |  4 ++++
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/disas.c b/disas.c
index 32c6518..1d46d25 100644
--- a/disas.c
+++ b/disas.c
@@ -1,5 +1,6 @@
 /* General "disassemble this chunk" code.  Used for debugging. */
 #include "config.h"
+#include "qemu-common.h"
 #include "disas/bfd.h"
 #include "elf.h"
 #include <errno.h>
@@ -198,6 +199,8 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info)
 void target_disas(FILE *out, CPUArchState *env, target_ulong code,
                   target_ulong size, int flags)
 {
+    CPUState *cpu = ENV_GET_CPU(env);
+    CPUClass *cc = CPU_GET_CLASS(cpu);
     target_ulong pc;
     int count;
     CPUDebug s;
@@ -215,6 +218,11 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code,
 #else
     s.info.endian = BFD_ENDIAN_LITTLE;
 #endif
+
+    if (cc->disas_set_info) {
+        cc->disas_set_info(cpu, &s.info);
+    }
+
 #if defined(TARGET_I386)
     if (flags == 2) {
         s.info.mach = bfd_mach_x86_64;
@@ -449,6 +457,8 @@ monitor_fprintf(FILE *stream, const char *fmt, ...)
 void monitor_disas(Monitor *mon, CPUArchState *env,
                    target_ulong pc, int nb_insn, int is_physical, int flags)
 {
+    CPUState *cpu = ENV_GET_CPU(env);
+    CPUClass *cc = CPU_GET_CLASS(cpu);
     int count, i;
     CPUDebug s;
 
@@ -466,6 +476,11 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
 #else
     s.info.endian = BFD_ENDIAN_LITTLE;
 #endif
+
+    if (cc->disas_set_info) {
+        cc->disas_set_info(cpu, &s.info);
+    }
+
 #if defined(TARGET_I386)
     if (flags == 2) {
         s.info.mach = bfd_mach_x86_64;
@@ -519,11 +534,12 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
 #elif defined(TARGET_LM32)
     s.info.mach = bfd_mach_lm32;
     s.info.print_insn = print_insn_lm32;
-#else
-    monitor_printf(mon, "0x" TARGET_FMT_lx
-                   ": Asm output not supported on this arch\n", pc);
-    return;
 #endif
+    if (!s.info.print_insn) {
+        monitor_printf(mon, "0x" TARGET_FMT_lx
+                       ": Asm output not supported on this arch\n", pc);
+        return;
+    }
 
     for(i = 0; i < nb_insn; i++) {
 	monitor_printf(mon, "0x" TARGET_FMT_lx ":  ", pc);
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 39f0f19..363c928 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -23,6 +23,7 @@
 #include <signal.h>
 #include <setjmp.h>
 #include "hw/qdev-core.h"
+#include "disas/bfd.h"
 #include "exec/hwaddr.h"
 #include "exec/memattrs.h"
 #include "qemu/queue.h"
@@ -117,6 +118,7 @@ struct TranslationBlock;
  * @cpu_exec_enter: Callback for cpu_exec preparation.
  * @cpu_exec_exit: Callback for cpu_exec cleanup.
  * @cpu_exec_interrupt: Callback for processing interrupts in cpu_exec.
+ * @disas_set_info: Setup architecture specific components of disassembly info
  *
  * Represents a CPU family or model.
  */
@@ -172,6 +174,8 @@ typedef struct CPUClass {
     void (*cpu_exec_enter)(CPUState *cpu);
     void (*cpu_exec_exit)(CPUState *cpu);
     bool (*cpu_exec_interrupt)(CPUState *cpu, int interrupt_request);
+
+    void (*disas_set_info)(CPUState *cpu, disassemble_info *info);
 } CPUClass;
 
 #ifdef HOST_WORDS_BIGENDIAN
-- 
1.9.1

[Qemu-devel] [PATCH v2 3/7] disas: arm-a64: Make printfer and stream variable

[Peter Crosthwaite]

In a normal disassembly flow, the printf and stream being used varies
from disas job to job. In particular it varies if mixing monitor_disas
and target_disas.

Make both the printfer function and target stream settable in the
QEMUDisassmbler class. Remove the stream_ initialisation from the
constructor as it will now runtime vary (and an initial value won't
mean very much).

Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
Tested-by: Claudio Fontana <claudio.fontana@huawei.com>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
Changed since v1:
Drop explicit from constructor
Keep NULL stream_ initialiser
Initialise printf_ to NULL
---
 disas/arm-a64.cc | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/disas/arm-a64.cc b/disas/arm-a64.cc
index e04f946..2fe5e78 100644
--- a/disas/arm-a64.cc
+++ b/disas/arm-a64.cc
@@ -35,16 +35,28 @@ static Disassembler *vixl_disasm = NULL;
  */
 class QEMUDisassembler : public Disassembler {
 public:
-    explicit QEMUDisassembler(FILE *stream) : stream_(stream) { }
+    QEMUDisassembler() {
+        printf_ = NULL;
+        stream_ = NULL;
+    }
     ~QEMUDisassembler() { }
 
+    void SetStream(FILE *stream) {
+        stream_ = stream;
+    }
+
+    void SetPrintf(int (*printf_fn)(FILE *, const char *, ...)) {
+        printf_ = printf_fn;
+    }
+
 protected:
     virtual void ProcessOutput(const Instruction *instr) {
-        fprintf(stream_, "%08" PRIx32 "      %s",
+        printf_(stream_, "%08" PRIx32 "      %s",
                 instr->InstructionBits(), GetOutput());
     }
 
 private:
+    int (*printf_)(FILE *, const char *, ...);
     FILE *stream_;
 };
 
@@ -53,9 +65,9 @@ static int vixl_is_initialized(void)
     return vixl_decoder != NULL;
 }
 
-static void vixl_init(FILE *f) {
+static void vixl_init() {
     vixl_decoder = new Decoder();
-    vixl_disasm = new QEMUDisassembler(f);
+    vixl_disasm = new QEMUDisassembler();
     vixl_decoder->AppendVisitor(vixl_disasm);
 }
 
@@ -78,9 +90,12 @@ int print_insn_arm_a64(uint64_t addr, disassemble_info *info)
     }
 
     if (!vixl_is_initialized()) {
-        vixl_init(info->stream);
+        vixl_init();
     }
 
+    ((QEMUDisassembler *)vixl_disasm)->SetPrintf(info->fprintf_func);
+    ((QEMUDisassembler *)vixl_disasm)->SetStream(info->stream);
+
     instrval = bytes[0] | bytes[1] << 8 | bytes[2] << 16 | bytes[3] << 24;
     instr = reinterpret_cast<const Instruction *>(&instrval);
     vixl_disasm->MapCodeAddress(addr, instr);
-- 
1.9.1

[Qemu-devel] [PATCH v2 4/7] disas: arm: QOMify target specific disas setup

[Peter Crosthwaite]

Move the target_disas() ARM specifics to the QOM disas_set_info hook
and delete the ARM specific code in disas.c.

This has the extra advantage of the more fully featured target_disas()
implementation now applying to monitor_disas().

Currently, target_disas() has multi-endian, thumb and AArch64
support whereas the existing monitor_disas() support only has vanilla
AA32 support.

E.G. Running an AA64 linux kernel the follow -d in_asm disas happens
(taget_disas()):

IN:
0x0000000040000000:  580000c0      ldr x0, pc+24 (addr 0x40000018)
0x0000000040000004:  aa1f03e1      mov x1, xzr

However before this patch, disasing the same from the monitor:

(qemu) xp/i 0x40000000
0x0000000040000000:  580000c0      stmdapl  r0, {r6, r7}

After this patch:
(qemu) xp/i 0x40000000
0x0000000040000000:  580000c0      ldr x0, pc+24 (addr 0x40000018)

Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 disas.c          | 32 --------------------------------
 target-arm/cpu.c | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 32 deletions(-)

diff --git a/disas.c b/disas.c
index 1d46d25..1700537 100644
--- a/disas.c
+++ b/disas.c
@@ -151,14 +151,6 @@ bfd_vma bfd_getb16 (const bfd_byte *addr)
   return (bfd_vma) v;
 }
 
-#ifdef TARGET_ARM
-static int
-print_insn_thumb1(bfd_vma pc, disassemble_info *info)
-{
-  return print_insn_arm(pc | 1, info);
-}
-#endif
-
 static int print_insn_objdump(bfd_vma pc, disassemble_info *info,
                               const char *prefix)
 {
@@ -191,7 +183,6 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info)
 /* Disassemble this for me please... (debugging). 'flags' has the following
    values:
     i386 - 1 means 16 bit code, 2 means 64 bit code
-    arm  - bit 0 = thumb, bit 1 = reverse endian, bit 2 = A64
     ppc  - bits 0:15 specify (optionally) the machine instruction set;
            bit 16 indicates little endian.
     other targets - unused
@@ -232,27 +223,6 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code,
         s.info.mach = bfd_mach_i386_i386;
     }
     s.info.print_insn = print_insn_i386;
-#elif defined(TARGET_ARM)
-    if (flags & 4) {
-        /* We might not be compiled with the A64 disassembler
-         * because it needs a C++ compiler; in that case we will
-         * fall through to the default print_insn_od case.
-         */
-#if defined(CONFIG_ARM_A64_DIS)
-        s.info.print_insn = print_insn_arm_a64;
-#endif
-    } else if (flags & 1) {
-        s.info.print_insn = print_insn_thumb1;
-    } else {
-        s.info.print_insn = print_insn_arm;
-    }
-    if (flags & 2) {
-#ifdef TARGET_WORDS_BIGENDIAN
-        s.info.endian = BFD_ENDIAN_LITTLE;
-#else
-        s.info.endian = BFD_ENDIAN_BIG;
-#endif
-    }
 #elif defined(TARGET_SPARC)
     s.info.print_insn = print_insn_sparc;
 #ifdef TARGET_SPARC64
@@ -490,8 +460,6 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
         s.info.mach = bfd_mach_i386_i386;
     }
     s.info.print_insn = print_insn_i386;
-#elif defined(TARGET_ARM)
-    s.info.print_insn = print_insn_arm;
 #elif defined(TARGET_ALPHA)
     s.info.print_insn = print_insn_alpha;
 #elif defined(TARGET_SPARC)
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 3ca3fa8..cfa761a 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -362,6 +362,39 @@ static inline void unset_feature(CPUARMState *env, int feature)
     env->features &= ~(1ULL << feature);
 }
 
+static int
+print_insn_thumb1(bfd_vma pc, disassemble_info *info)
+{
+  return print_insn_arm(pc | 1, info);
+}
+
+static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
+{
+    ARMCPU *ac = ARM_CPU(cpu);
+    CPUARMState *env = &ac->env;
+
+    if (env->aarch64) {
+        /* We might not be compiled with the A64 disassembler
+         * because it needs a C++ compiler. Leave print_insn
+         * unset in this case to use the caller default behaviour.
+         */
+#if defined(CONFIG_ARM_A64_DIS)
+        info->print_insn = print_insn_arm_a64;
+#endif
+    } else if (env->thumb) {
+        info->print_insn = print_insn_thumb1;
+    } else {
+        info->print_insn = print_insn_arm;
+    }
+    if (env->bswap_code) {
+#ifdef TARGET_WORDS_BIGENDIAN
+        info->endian = BFD_ENDIAN_LITTLE;
+#else
+        info->endian = BFD_ENDIAN_BIG;
+#endif
+    }
+}
+
 static void arm_cpu_initfn(Object *obj)
 {
     CPUState *cs = CPU(obj);
@@ -1229,6 +1262,8 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
     cc->gdb_core_xml_file = "arm-core.xml";
     cc->gdb_stop_before_watchpoint = true;
     cc->debug_excp_handler = arm_debug_excp_handler;
+
+    cc->disas_set_info = arm_disas_set_info;
 }
 
 static void cpu_register(const ARMCPUInfo *info)
-- 
1.9.1

[Qemu-devel] [PATCH v2 5/7] disas: microblaze: QOMify target specific disas setup

[Peter Crosthwaite]

Move the target_disas() MB specifics to the QOM disas_set_info hook
and delete the MB specific code in disas.c.

This also now adds support for monitor disas to Microblaze.

E.g.
(qemu) xp 0x90000000
0000000090000000: 0x94208001

And before this patch:
(qemu) xp/i 0x90000000
0x90000000: Asm output not supported on this arch

After:
(qemu) xp/i 0x90000000
0x90000000:  mfs    r1, rmsr

Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 disas.c                 | 3 ---
 target-microblaze/cpu.c | 8 ++++++++
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/disas.c b/disas.c
index 1700537..087f4b4 100644
--- a/disas.c
+++ b/disas.c
@@ -269,9 +269,6 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code,
 #elif defined(TARGET_S390X)
     s.info.mach = bfd_mach_s390_64;
     s.info.print_insn = print_insn_s390;
-#elif defined(TARGET_MICROBLAZE)
-    s.info.mach = bfd_arch_microblaze;
-    s.info.print_insn = print_insn_microblaze;
 #elif defined(TARGET_MOXIE)
     s.info.mach = bfd_arch_moxie;
     s.info.print_insn = print_insn_moxie;
diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
index 67e3182..89b8363 100644
--- a/target-microblaze/cpu.c
+++ b/target-microblaze/cpu.c
@@ -111,6 +111,12 @@ static void mb_cpu_reset(CPUState *s)
 #endif
 }
 
+static void mb_disas_set_info(CPUState *cpu, disassemble_info *info)
+{
+    info->mach = bfd_arch_microblaze;
+    info->print_insn = print_insn_microblaze;
+}
+
 static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
@@ -183,6 +189,8 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
     dc->vmsd = &vmstate_mb_cpu;
     dc->props = mb_properties;
     cc->gdb_num_core_regs = 32 + 5;
+
+    cc->disas_set_info = mb_disas_set_info;
 }
 
 static const TypeInfo mb_cpu_type_info = {
-- 
1.9.1

[Qemu-devel] [PATCH v2 6/7] disas: cris: Fix 0 buffer length case

[Peter Crosthwaite]

Cris has the complication of variable length instructions and has
a check in place to clamp memory reads in case the disas request
doesn't have enough bytes for the instruction being disas'd. This
breaks down in the case where disassembling for the monitor where
the buffer length is defaulted to 0.

The buffer length should never be zero for a regular target_disas,
so we can safely assume the 0 case is for the monitor in which case
consider the buffer length to be the max for cris instructions.

Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 disas/cris.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/disas/cris.c b/disas/cris.c
index e6cff7a..1b76a09 100644
--- a/disas/cris.c
+++ b/disas/cris.c
@@ -2575,9 +2575,9 @@ print_insn_cris_generic (bfd_vma memaddr,
      If we can't get any data, or we do not get enough data, we print
      the error message.  */
 
-  nbytes = info->buffer_length;
-  if (nbytes > MAX_BYTES_PER_CRIS_INSN)
-	  nbytes = MAX_BYTES_PER_CRIS_INSN;
+  nbytes = info->buffer_length ? info->buffer_length
+                               : MAX_BYTES_PER_CRIS_INSN;
+  nbytes = MIN(nbytes, MAX_BYTES_PER_CRIS_INSN);
   status = (*info->read_memory_func) (memaddr, buffer, nbytes, info);  
 
   /* If we did not get all we asked for, then clear the rest.
-- 
1.9.1

[Qemu-devel] [PATCH v2 7/7] disas: cris: QOMify target specific disas setup

[Peter Crosthwaite]

Move the target_disas() cris specifics to the QOM disas_set_info hook
and delete the cris specific code in disas.c.

This also now adds support for monitor disas to cris.

E.g.
(qemu) xp 0x40004000
0000000040004000: 0x1e6f25f0

And before this patch:
(qemu) xp/i 0x40004000
0x40004000: Asm output not supported on this arch

After:
(qemu) xp/i 0x40004000
0x40004000:  di
(qemu) xp/i 0x40004002
0x40004002:  move.d 0xb003c004,$r1

Note: second example is 6-byte misaligned instruction!

Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 disas.c           |  8 --------
 target-cris/cpu.c | 16 ++++++++++++++++
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/disas.c b/disas.c
index 087f4b4..184a180 100644
--- a/disas.c
+++ b/disas.c
@@ -258,14 +258,6 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code,
 #elif defined(TARGET_ALPHA)
     s.info.mach = bfd_mach_alpha_ev6;
     s.info.print_insn = print_insn_alpha;
-#elif defined(TARGET_CRIS)
-    if (flags != 32) {
-        s.info.mach = bfd_mach_cris_v0_v10;
-        s.info.print_insn = print_insn_crisv10;
-    } else {
-        s.info.mach = bfd_mach_cris_v32;
-        s.info.print_insn = print_insn_crisv32;
-    }
 #elif defined(TARGET_S390X)
     s.info.mach = bfd_mach_s390_64;
     s.info.print_insn = print_insn_s390;
diff --git a/target-cris/cpu.c b/target-cris/cpu.c
index 16cfba9..d555ea0 100644
--- a/target-cris/cpu.c
+++ b/target-cris/cpu.c
@@ -161,6 +161,20 @@ static void cris_cpu_set_irq(void *opaque, int irq, int level)
 }
 #endif
 
+static void cris_disas_set_info(CPUState *cpu, disassemble_info *info)
+{
+    CRISCPU *cc = CRIS_CPU(cpu);
+    CPUCRISState *env = &cc->env;
+
+    if (env->pregs[PR_VR] != 32) {
+        info->mach = bfd_mach_cris_v0_v10;
+        info->print_insn = print_insn_crisv10;
+    } else {
+        info->mach = bfd_mach_cris_v32;
+        info->print_insn = print_insn_crisv32;
+    }
+}
+
 static void cris_cpu_initfn(Object *obj)
 {
     CPUState *cs = CPU(obj);
@@ -292,6 +306,8 @@ static void cris_cpu_class_init(ObjectClass *oc, void *data)
 
     cc->gdb_num_core_regs = 49;
     cc->gdb_stop_before_watchpoint = true;
+
+    cc->disas_set_info = cris_disas_set_info;
 }
 
 static const TypeInfo cris_cpu_type_info = {
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 2/7] disas: QOMify target specific setup

[Paolo Bonzini]

On 09/05/2015 22:11, Peter Crosthwaite wrote:
> @@ -198,6 +199,8 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info)
>  void target_disas(FILE *out, CPUArchState *env, target_ulong code,
>                    target_ulong size, int flags)
>  {
> +    CPUState *cpu = ENV_GET_CPU(env);
> +    CPUClass *cc = CPU_GET_CLASS(cpu);
>      target_ulong pc;
>      int count;
>      CPUDebug s;
> @@ -215,6 +218,11 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code,
>  #else
>      s.info.endian = BFD_ENDIAN_LITTLE;
>  #endif
> +
> +    if (cc->disas_set_info) {
> +        cc->disas_set_info(cpu, &s.info);
> +    }
> +
>  #if defined(TARGET_I386)

Perhaps pass down the flags too?

Paolo

Re: [Qemu-devel] [PATCH v2 2/7] disas: QOMify target specific setup

[Richard Henderson]

On 05/11/2015 07:46 AM, Paolo Bonzini wrote:
> 
> 
> On 09/05/2015 22:11, Peter Crosthwaite wrote:
>> @@ -198,6 +199,8 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info)
>>  void target_disas(FILE *out, CPUArchState *env, target_ulong code,
>>                    target_ulong size, int flags)
>>  {
>> +    CPUState *cpu = ENV_GET_CPU(env);
>> +    CPUClass *cc = CPU_GET_CLASS(cpu);
>>      target_ulong pc;
>>      int count;
>>      CPUDebug s;
>> @@ -215,6 +218,11 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code,
>>  #else
>>      s.info.endian = BFD_ENDIAN_LITTLE;
>>  #endif
>> +
>> +    if (cc->disas_set_info) {
>> +        cc->disas_set_info(cpu, &s.info);
>> +    }
>> +
>>  #if defined(TARGET_I386)
> 
> Perhaps pass down the flags too?

The hook is allowing us to ditch the flags.  See 4/7 and 7/7 for arm and cris
portions that do exactly that.


r~

Re: [Qemu-devel] [PATCH v2 1/7] disas: Add print_insn to disassemble info

[Richard Henderson]

On 05/09/2015 01:11 PM, Peter Crosthwaite wrote:
> Add the print_insn pointer to the disassemble info structure. This is
> to prepare for QOMification support, where a QOM CPU hook function will
> be responsible for setting the print_insn function. Add this function
> to the existing struct to consolidate such that only the one struct
> needs to be passed to the new QOM API.
> 
> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> ---
>  disas.c             | 68 ++++++++++++++++++++++++++---------------------------
>  include/disas/bfd.h |  6 +++++
>  2 files changed, 39 insertions(+), 35 deletions(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH v2 2/7] disas: QOMify target specific setup

[Richard Henderson]

On 05/09/2015 01:11 PM, Peter Crosthwaite wrote:
> Add a QOM function hook for target-specific disassembly setup. This
> allows removal of the #ifdeffery currently implementing target specific
> disas setup from disas.c.
> 
> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> ---
>  disas.c           | 24 ++++++++++++++++++++----
>  include/qom/cpu.h |  4 ++++
>  2 files changed, 24 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH v2 3/7] disas: arm-a64: Make printfer and stream variable

[Richard Henderson]

On 05/09/2015 01:11 PM, Peter Crosthwaite wrote:
>  class QEMUDisassembler : public Disassembler {
>  public:
> -    explicit QEMUDisassembler(FILE *stream) : stream_(stream) { }
> +    QEMUDisassembler() {
> +        printf_ = NULL;
> +        stream_ = NULL;
> +    }

As a nit, I would have written this

  QEMUDisassembler(FILE *stream) : stream_(NULL), printf_(NULL) { }

but the difference is unlikely to matter here.

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH v2 0/7] Unify and QOMify (target|monitor)_disas

[Peter Crosthwaite]

Ping!

Richard has RB'd the core stuff but do we need CPU arch maintainer
acks on the latter patches?

What queue should this go via? TCG?

Regards,
Peter

On Sat, May 9, 2015 at 1:11 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> These two functions are mostly trying to do the same thing, which is
> disassemble a target instruction (sequence) for printfing. The
> architecture specific setup is largely duped between the two functions.
>
> The approach is to add a single QOM hook on the CPU level to setup the
> disassembler (P1&2). The two stage flags system is removed. That is,
> the old scheme, is for the translate/montitor code to pass in flags
> that disas.c then interprets. Instead the entire job of setting up arch
> specifics is outsourced to target-specific code (via the new QOM hook)
> removing the need for the flags system. Both monitor_disas and
> target_disas then calls this singly defined hook if it is available.
>
> Three architectures (microblaze, cris and ARM) are patched
> to use the new QOMification and at the same time, make the
> monitor_disas consistent with target_disas. The #if defined TARGET_FOO
> for each is removed from disas.c (bringing us closer to the exciting
> goal of no #ifdef TARGET_FOO in system mode code).
>
> Microblaze is trivial, the target_disas setup is directly applicable
> to monitor_disas to bring in microblaze monitor disas support (P5).
>
> Cris had a small hiccup, a patch is needed to handle monitor_disas's
> 0 buffer length (P6). Then cris is patched to enable monitor disas
> in same way as microblaze (P7).
>
> ARM is the harder. The vixl A64 disas was hardcoded to fprintf with
> a statically inited output stream (matching target_disas). The vixl
> printfery is patched to be runtime variable (P3). P4 brings
> ARM monitor disassembly online (via using the target_disas
> implementation as the QOMified implementation).
>
> Changed since v1 (RTH review):
> Use QOMified approach.
> Remove flags system.
> Limit scope to only the 3 converted arches
> Addressed comments on CPP constructor changes
>
> Peter Crosthwaite (7):
>   disas: Add print_insn to disassemble info
>   disas: QOMify target specific setup
>   disas: arm-a64: Make printfer and stream variable
>   disas: arm: QOMify target specific disas setup
>   disas: microblaze: QOMify target specific disas setup
>   disas: cris: Fix 0 buffer length case
>   disas: cris: QOMify target specific disas setup
>
>  disas.c                 | 121 ++++++++++++++++++------------------------------
>  disas/arm-a64.cc        |  25 ++++++++--
>  disas/cris.c            |   6 +--
>  include/disas/bfd.h     |   6 +++
>  include/qom/cpu.h       |   4 ++
>  target-arm/cpu.c        |  35 ++++++++++++++
>  target-cris/cpu.c       |  16 +++++++
>  target-microblaze/cpu.c |   8 ++++
>  8 files changed, 138 insertions(+), 83 deletions(-)
>
> --
> 1.9.1
>
>

Re: [Qemu-devel] [PATCH v2 0/7] Unify and QOMify (target|monitor)_disas

[Richard Henderson]

On 05/14/2015 09:52 PM, Peter Crosthwaite wrote:
> Ping!
> 
> Richard has RB'd the core stuff but do we need CPU arch maintainer
> acks on the latter patches?

Yes, I'd prefer especially the arm patches get another look.

> What queue should this go via? TCG?

QOM via Andreas would be my first preference.  Otherwise, I'm happy with you
sending the pull request yourself once you've got the acks.


r~

Re: [Qemu-devel] [PATCH v2 0/7] Unify and QOMify (target|monitor)_disas

[Peter Crosthwaite]

On Fri, May 15, 2015 at 6:33 AM, Richard Henderson <rth@twiddle.net> wrote:
> On 05/14/2015 09:52 PM, Peter Crosthwaite wrote:
>> Ping!
>>
>> Richard has RB'd the core stuff but do we need CPU arch maintainer
>> acks on the latter patches?
>
> Yes, I'd prefer especially the arm patches get another look.
>
>> What queue should this go via? TCG?
>
> QOM via Andreas would be my first preference.  Otherwise, I'm happy with you
> sending the pull request yourself once you've got the acks.
>

CC Andreas :)

Regards,
Peter

>
> r~
>

Re: [Qemu-devel] [PATCH v2 0/7] Unify and QOMify (target|monitor)_disas

[Peter Crosthwaite]

On Fri, May 15, 2015 at 6:33 AM, Richard Henderson <rth@twiddle.net> wrote:
> On 05/14/2015 09:52 PM, Peter Crosthwaite wrote:
>> Ping!
>>
>> Richard has RB'd the core stuff but do we need CPU arch maintainer
>> acks on the latter patches?
>
> Yes, I'd prefer especially the arm patches get another look.
>
>> What queue should this go via? TCG?
>
> QOM via Andreas would be my first preference.  Otherwise, I'm happy with you
> sending the pull request yourself once you've got the acks.
>

I'm guessing I need a signed key for that. Anyone know of someone in
San Francisco bay-area able to sign me?

Regards,
Peter

>
> r~
>

Re: [Qemu-devel] [PATCH v2 0/7] Unify and QOMify (target|monitor)_disas

[Peter Maydell]

On 15 May 2015 at 22:01, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> I'm guessing I need a signed key for that. Anyone know of someone in
> San Francisco bay-area able to sign me?

At the moment we're requiring signed tags, but not necessarily
that all submaintainers have a web-of-trust path from me to
them (though that is certainly good to have and we'll be going
there at some point). Basically I'm trying to ratchet up the
level of signedness gradually.

-- PMM

Re: [Qemu-devel] [PATCH v2 4/7] disas: arm: QOMify target specific disas setup

[Peter Maydell]

On 9 May 2015 at 21:11, Peter Crosthwaite <crosthwaitepeter@gmail.com> wrote:
> Move the target_disas() ARM specifics to the QOM disas_set_info hook
> and delete the ARM specific code in disas.c.
>
> This has the extra advantage of the more fully featured target_disas()
> implementation now applying to monitor_disas().
>
> Currently, target_disas() has multi-endian, thumb and AArch64
> support whereas the existing monitor_disas() support only has vanilla
> AA32 support.
>
> E.G. Running an AA64 linux kernel the follow -d in_asm disas happens
> (taget_disas()):
>
> IN:
> 0x0000000040000000:  580000c0      ldr x0, pc+24 (addr 0x40000018)
> 0x0000000040000004:  aa1f03e1      mov x1, xzr
>
> However before this patch, disasing the same from the monitor:
>
> (qemu) xp/i 0x40000000
> 0x0000000040000000:  580000c0      stmdapl  r0, {r6, r7}
>
> After this patch:
> (qemu) xp/i 0x40000000
> 0x0000000040000000:  580000c0      ldr x0, pc+24 (addr 0x40000018)
>
> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> ---
>  disas.c          | 32 --------------------------------
>  target-arm/cpu.c | 35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+), 32 deletions(-)
>
> diff --git a/disas.c b/disas.c
> index 1d46d25..1700537 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -151,14 +151,6 @@ bfd_vma bfd_getb16 (const bfd_byte *addr)
>    return (bfd_vma) v;
>  }
>
> -#ifdef TARGET_ARM
> -static int
> -print_insn_thumb1(bfd_vma pc, disassemble_info *info)
> -{
> -  return print_insn_arm(pc | 1, info);
> -}
> -#endif
> -
>  static int print_insn_objdump(bfd_vma pc, disassemble_info *info,
>                                const char *prefix)
>  {
> @@ -191,7 +183,6 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info)
>  /* Disassemble this for me please... (debugging). 'flags' has the following
>     values:
>      i386 - 1 means 16 bit code, 2 means 64 bit code
> -    arm  - bit 0 = thumb, bit 1 = reverse endian, bit 2 = A64
>      ppc  - bits 0:15 specify (optionally) the machine instruction set;
>             bit 16 indicates little endian.
>      other targets - unused
> @@ -232,27 +223,6 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code,
>          s.info.mach = bfd_mach_i386_i386;
>      }
>      s.info.print_insn = print_insn_i386;
> -#elif defined(TARGET_ARM)
> -    if (flags & 4) {
> -        /* We might not be compiled with the A64 disassembler
> -         * because it needs a C++ compiler; in that case we will
> -         * fall through to the default print_insn_od case.
> -         */
> -#if defined(CONFIG_ARM_A64_DIS)
> -        s.info.print_insn = print_insn_arm_a64;
> -#endif
> -    } else if (flags & 1) {
> -        s.info.print_insn = print_insn_thumb1;
> -    } else {
> -        s.info.print_insn = print_insn_arm;
> -    }
> -    if (flags & 2) {
> -#ifdef TARGET_WORDS_BIGENDIAN
> -        s.info.endian = BFD_ENDIAN_LITTLE;
> -#else
> -        s.info.endian = BFD_ENDIAN_BIG;
> -#endif
> -    }
>  #elif defined(TARGET_SPARC)
>      s.info.print_insn = print_insn_sparc;
>  #ifdef TARGET_SPARC64
> @@ -490,8 +460,6 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
>          s.info.mach = bfd_mach_i386_i386;
>      }
>      s.info.print_insn = print_insn_i386;
> -#elif defined(TARGET_ARM)
> -    s.info.print_insn = print_insn_arm;
>  #elif defined(TARGET_ALPHA)
>      s.info.print_insn = print_insn_alpha;
>  #elif defined(TARGET_SPARC)
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 3ca3fa8..cfa761a 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -362,6 +362,39 @@ static inline void unset_feature(CPUARMState *env, int feature)
>      env->features &= ~(1ULL << feature);
>  }
>
> +static int
> +print_insn_thumb1(bfd_vma pc, disassemble_info *info)
> +{
> +  return print_insn_arm(pc | 1, info);
> +}
> +
> +static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
> +{
> +    ARMCPU *ac = ARM_CPU(cpu);
> +    CPUARMState *env = &ac->env;
> +
> +    if (env->aarch64) {

if (is_a64(env)) please.
(At some point I'm likely to tidy up handling of A64 pstate,
and maybe this flag will disappear back into the uncached pstate.)

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 3/7] disas: arm-a64: Make printfer and stream variable

[Peter Crosthwaite]

On Mon, May 11, 2015 at 8:57 AM, Richard Henderson <rth@twiddle.net> wrote:
> On 05/09/2015 01:11 PM, Peter Crosthwaite wrote:
>>  class QEMUDisassembler : public Disassembler {
>>  public:
>> -    explicit QEMUDisassembler(FILE *stream) : stream_(stream) { }
>> +    QEMUDisassembler() {
>> +        printf_ = NULL;
>> +        stream_ = NULL;
>> +    }
>
> As a nit, I would have written this
>
>   QEMUDisassembler(FILE *stream) : stream_(NULL), printf_(NULL) { }
>

Changed made in V3.

> but the difference is unlikely to matter here.
>
> Reviewed-by: Richard Henderson <rth@twiddle.net>
>

Thanks,

Regards,
Peter

>
> r~
>

Re: [Qemu-devel] [PATCH v2 4/7] disas: arm: QOMify target specific disas setup

[Peter Crosthwaite]

On Mon, May 18, 2015 at 9:31 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 9 May 2015 at 21:11, Peter Crosthwaite <crosthwaitepeter@gmail.com> wrote:
>> Move the target_disas() ARM specifics to the QOM disas_set_info hook

>>
>> +static int
>> +print_insn_thumb1(bfd_vma pc, disassemble_info *info)
>> +{
>> +  return print_insn_arm(pc | 1, info);
>> +}
>> +
>> +static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
>> +{
>> +    ARMCPU *ac = ARM_CPU(cpu);
>> +    CPUARMState *env = &ac->env;
>> +
>> +    if (env->aarch64) {
>
> if (is_a64(env)) please.

Fixed.

> (At some point I'm likely to tidy up handling of A64 pstate,
> and maybe this flag will disappear back into the uncached pstate.)
>
> Otherwise
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>

Thanks.

Regards,
Peter

> thanks
> -- PMM
>