diff mbox

[v2,4/7] disas: arm: QOMify target specific disas setup

Message ID 0cdf8f638880c9dc80691d8676dd227b24477b40.1431200693.git.crosthwaite.peter@gmail.com
State New
Headers show

Commit Message

Peter Crosthwaite May 9, 2015, 8:11 p.m. UTC
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(-)

Comments

Peter Maydell May 18, 2015, 4:31 p.m. UTC | #1
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
Peter Crosthwaite May 24, 2015, 9:42 p.m. UTC | #2
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
>
diff mbox

Patch

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)