* [Qemu-devel] [PATCH v4 1/9] target/i386: Convert to disas_set_info hook
2017-09-28 16:54 [Qemu-devel] [PATCH v4 0/9] Support the Capstone disassembler Richard Henderson
@ 2017-09-28 16:54 ` Richard Henderson
2017-10-02 12:56 ` Philippe Mathieu-Daudé
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 2/9] target/ppc: " Richard Henderson
` (8 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2017-09-28 16:54 UTC (permalink / raw)
To: qemu-devel
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 d6a1eb9c8e..2be716fdb2 100644
--- a/disas.c
+++ b/disas.c
@@ -205,16 +205,7 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
cc->disas_set_info(cpu, &s.info);
}
-#if defined(TARGET_I386)
- if (flags == 2) {
- s.info.mach = bfd_mach_x86_64;
- } else if (flags == 1) {
- s.info.mach = bfd_mach_i386_i8086;
- } else {
- s.info.mach = bfd_mach_i386_i386;
- }
- s.info.print_insn = print_insn_i386;
-#elif defined(TARGET_PPC)
+#if defined(TARGET_PPC)
if ((flags >> 16) & 1) {
s.info.endian = BFD_ENDIAN_LITTLE;
}
@@ -390,16 +381,7 @@ void monitor_disas(Monitor *mon, CPUState *cpu,
cc->disas_set_info(cpu, &s.info);
}
-#if defined(TARGET_I386)
- if (flags == 2) {
- s.info.mach = bfd_mach_x86_64;
- } else if (flags == 1) {
- s.info.mach = bfd_mach_i386_i8086;
- } else {
- s.info.mach = bfd_mach_i386_i386;
- }
- s.info.print_insn = print_insn_i386;
-#elif defined(TARGET_PPC)
+#if defined(TARGET_PPC)
if (flags & 0xFFFF) {
/* If we have a precise definition of the instruction set, use it. */
s.info.mach = flags & 0xFFFF;
diff --git a/monitor.c b/monitor.c
index f4856b9268..1184bec678 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 a8986f4c1a..9932d64f2e 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -8527,15 +8527,9 @@ static void i386_tr_disas_log(const DisasContextBase *dcbase,
CPUState *cpu)
{
DisasContext *dc = container_of(dcbase, DisasContext, base);
- int disas_flags = !dc->code32;
qemu_log("IN: %s\n", lookup_symbol(dc->base.pc_first));
-#ifdef TARGET_X86_64
- if (dc->code64) {
- disas_flags = 2;
- }
-#endif
- log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size, disas_flags);
+ log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size, 0);
}
static const TranslatorOps i386_tr_ops = {
--
2.13.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 1/9] target/i386: Convert to disas_set_info hook
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 1/9] target/i386: Convert to disas_set_info hook Richard Henderson
@ 2017-10-02 12:56 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-10-02 12:56 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On 09/28/2017 01:54 PM, Richard Henderson wrote:
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.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 d6a1eb9c8e..2be716fdb2 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -205,16 +205,7 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
> cc->disas_set_info(cpu, &s.info);
> }
>
> -#if defined(TARGET_I386)
> - if (flags == 2) {
> - s.info.mach = bfd_mach_x86_64;
> - } else if (flags == 1) {
> - s.info.mach = bfd_mach_i386_i8086;
> - } else {
> - s.info.mach = bfd_mach_i386_i386;
> - }
> - s.info.print_insn = print_insn_i386;
> -#elif defined(TARGET_PPC)
> +#if defined(TARGET_PPC)
> if ((flags >> 16) & 1) {
> s.info.endian = BFD_ENDIAN_LITTLE;
> }
> @@ -390,16 +381,7 @@ void monitor_disas(Monitor *mon, CPUState *cpu,
> cc->disas_set_info(cpu, &s.info);
> }
>
> -#if defined(TARGET_I386)
> - if (flags == 2) {
> - s.info.mach = bfd_mach_x86_64;
> - } else if (flags == 1) {
> - s.info.mach = bfd_mach_i386_i8086;
> - } else {
> - s.info.mach = bfd_mach_i386_i386;
> - }
> - s.info.print_insn = print_insn_i386;
> -#elif defined(TARGET_PPC)
> +#if defined(TARGET_PPC)
> if (flags & 0xFFFF) {
> /* If we have a precise definition of the instruction set, use it. */
> s.info.mach = flags & 0xFFFF;
> diff --git a/monitor.c b/monitor.c
> index f4856b9268..1184bec678 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 a8986f4c1a..9932d64f2e 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -8527,15 +8527,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 = {
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v4 2/9] target/ppc: Convert to disas_set_info hook
2017-09-28 16:54 [Qemu-devel] [PATCH v4 0/9] Support the Capstone disassembler Richard Henderson
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 1/9] target/i386: Convert to disas_set_info hook Richard Henderson
@ 2017-09-28 16:54 ` Richard Henderson
2017-10-02 12:56 ` Philippe Mathieu-Daudé
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 3/9] disas: Remove unused flags arguments Richard Henderson
` (7 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2017-09-28 16:54 UTC (permalink / raw)
To: qemu-devel
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 2be716fdb2..3a375a3b6c 100644
--- a/disas.c
+++ b/disas.c
@@ -205,23 +205,6 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
cc->disas_set_info(cpu, &s.info);
}
-#if defined(TARGET_PPC)
- if ((flags >> 16) & 1) {
- s.info.endian = BFD_ENDIAN_LITTLE;
- }
- if (flags & 0xFFFF) {
- /* If we have a precise definition of the instruction set, use it. */
- s.info.mach = flags & 0xFFFF;
- } else {
-#ifdef TARGET_PPC64
- s.info.mach = bfd_mach_ppc64;
-#else
- s.info.mach = bfd_mach_ppc;
-#endif
- }
- s.info.disassembler_options = (char *)"any";
- s.info.print_insn = print_insn_ppc;
-#endif
if (s.info.print_insn == NULL) {
s.info.print_insn = print_insn_od_target;
}
@@ -381,22 +364,6 @@ void monitor_disas(Monitor *mon, CPUState *cpu,
cc->disas_set_info(cpu, &s.info);
}
-#if defined(TARGET_PPC)
- if (flags & 0xFFFF) {
- /* If we have a precise definition of the instruction set, use it. */
- s.info.mach = flags & 0xFFFF;
- } else {
-#ifdef TARGET_PPC64
- s.info.mach = bfd_mach_ppc64;
-#else
- s.info.mach = bfd_mach_ppc;
-#endif
- }
- if ((flags >> 16) & 1) {
- s.info.endian = BFD_ENDIAN_LITTLE;
- }
- s.info.print_insn = print_insn_ppc;
-#endif
if (!s.info.print_insn) {
monitor_printf(mon, "0x" TARGET_FMT_lx
": Asm output not supported on this arch\n", pc);
diff --git a/monitor.c b/monitor.c
index 1184bec678..d55ad61dbb 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1310,11 +1310,6 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize,
if (format == 'i') {
int flags = 0;
-#ifdef TARGET_PPC
- CPUArchState *env = mon_get_cpu_env();
- flags = msr_le << 16;
- flags |= env->bfd_mach;
-#endif
monitor_disas(mon, cs, addr, count, is_physical, flags);
return;
}
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 606b605ba0..bc155f1036 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7395,12 +7395,9 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
#if defined(DEBUG_DISAS)
if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
&& qemu_log_in_addr_range(pc_start)) {
- int flags;
- flags = env->bfd_mach;
- flags |= ctx.le_mode << 16;
qemu_log_lock();
qemu_log("IN: %s\n", lookup_symbol(pc_start));
- log_target_disas(cs, pc_start, ctx.nip - pc_start, flags);
+ log_target_disas(cs, pc_start, ctx.nip - pc_start, 0);
qemu_log("\n");
qemu_log_unlock();
}
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index c6399a3a0d..2863e2c0b0 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -10681,6 +10681,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,
@@ -10742,6 +10762,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
#ifndef CONFIG_USER_ONLY
cc->virtio_is_big_endian = ppc_cpu_is_big_endian;
#endif
+ cc->disas_set_info = ppc_disas_set_info;
dc->fw_name = "PowerPC,UNKNOWN";
}
--
2.13.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/9] target/ppc: Convert to disas_set_info hook
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 2/9] target/ppc: " Richard Henderson
@ 2017-10-02 12:56 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-10-02 12:56 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On 09/28/2017 01:54 PM, Richard Henderson wrote:
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> disas.c | 33 ---------------------------------
> monitor.c | 5 -----
> target/ppc/translate.c | 5 +----
> target/ppc/translate_init.c | 21 +++++++++++++++++++++
> 4 files changed, 22 insertions(+), 42 deletions(-)
>
> diff --git a/disas.c b/disas.c
> index 2be716fdb2..3a375a3b6c 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -205,23 +205,6 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
> cc->disas_set_info(cpu, &s.info);
> }
>
> -#if defined(TARGET_PPC)
> - if ((flags >> 16) & 1) {
> - s.info.endian = BFD_ENDIAN_LITTLE;
> - }
> - if (flags & 0xFFFF) {
> - /* If we have a precise definition of the instruction set, use it. */
> - s.info.mach = flags & 0xFFFF;
> - } else {
> -#ifdef TARGET_PPC64
> - s.info.mach = bfd_mach_ppc64;
> -#else
> - s.info.mach = bfd_mach_ppc;
> -#endif
> - }
> - s.info.disassembler_options = (char *)"any";
> - s.info.print_insn = print_insn_ppc;
> -#endif
> if (s.info.print_insn == NULL) {
> s.info.print_insn = print_insn_od_target;
> }
> @@ -381,22 +364,6 @@ void monitor_disas(Monitor *mon, CPUState *cpu,
> cc->disas_set_info(cpu, &s.info);
> }
>
> -#if defined(TARGET_PPC)
> - if (flags & 0xFFFF) {
> - /* If we have a precise definition of the instruction set, use it. */
> - s.info.mach = flags & 0xFFFF;
> - } else {
> -#ifdef TARGET_PPC64
> - s.info.mach = bfd_mach_ppc64;
> -#else
> - s.info.mach = bfd_mach_ppc;
> -#endif
> - }
> - if ((flags >> 16) & 1) {
> - s.info.endian = BFD_ENDIAN_LITTLE;
> - }
> - s.info.print_insn = print_insn_ppc;
> -#endif
> if (!s.info.print_insn) {
> monitor_printf(mon, "0x" TARGET_FMT_lx
> ": Asm output not supported on this arch\n", pc);
> diff --git a/monitor.c b/monitor.c
> index 1184bec678..d55ad61dbb 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1310,11 +1310,6 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize,
>
> if (format == 'i') {
> int flags = 0;
> -#ifdef TARGET_PPC
> - CPUArchState *env = mon_get_cpu_env();
> - flags = msr_le << 16;
> - flags |= env->bfd_mach;
> -#endif
> monitor_disas(mon, cs, addr, count, is_physical, flags);
> return;
> }
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 606b605ba0..bc155f1036 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -7395,12 +7395,9 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
> #if defined(DEBUG_DISAS)
> if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
> && qemu_log_in_addr_range(pc_start)) {
> - int flags;
> - flags = env->bfd_mach;
> - flags |= ctx.le_mode << 16;
> qemu_log_lock();
> qemu_log("IN: %s\n", lookup_symbol(pc_start));
> - log_target_disas(cs, pc_start, ctx.nip - pc_start, flags);
> + log_target_disas(cs, pc_start, ctx.nip - pc_start, 0);
> qemu_log("\n");
> qemu_log_unlock();
> }
> diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> index c6399a3a0d..2863e2c0b0 100644
> --- a/target/ppc/translate_init.c
> +++ b/target/ppc/translate_init.c
> @@ -10681,6 +10681,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,
> @@ -10742,6 +10762,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";
> }
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v4 3/9] disas: Remove unused flags arguments
2017-09-28 16:54 [Qemu-devel] [PATCH v4 0/9] Support the Capstone disassembler Richard Henderson
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 1/9] target/i386: Convert to disas_set_info hook Richard Henderson
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 2/9] target/ppc: " Richard Henderson
@ 2017-09-28 16:54 ` Richard Henderson
2017-10-02 12:56 ` Philippe Mathieu-Daudé
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 4/9] disas: Support the Capstone disassembler library Richard Henderson
` (6 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2017-09-28 16:54 UTC (permalink / raw)
To: qemu-devel
Now that every target is using the disas_set_info hook,
the flags argument is unused. Remove it.
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 3a375a3b6c..ad675dc361 100644
--- a/disas.c
+++ b/disas.c
@@ -171,15 +171,9 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info)
return print_insn_objdump(pc, info, "OBJD-T");
}
-/* Disassemble this for me please... (debugging). 'flags' has the following
- values:
- i386 - 1 means 16 bit code, 2 means 64 bit code
- ppc - bits 0:15 specify (optionally) the machine instruction set;
- bit 16 indicates little endian.
- other targets - unused
- */
+/* Disassemble this for me please... (debugging). */
void target_disas(FILE *out, CPUState *cpu, target_ulong code,
- target_ulong size, int flags)
+ target_ulong size)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
target_ulong pc;
@@ -336,10 +330,9 @@ monitor_read_memory (bfd_vma memaddr, bfd_byte *myaddr, int length,
return 0;
}
-/* Disassembler for the monitor.
- See target_disas for a description of flags. */
+/* Disassembler for the monitor. */
void monitor_disas(Monitor *mon, CPUState *cpu,
- target_ulong pc, int nb_insn, int is_physical, int flags)
+ target_ulong pc, int nb_insn, int is_physical)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
int count, i;
diff --git a/monitor.c b/monitor.c
index d55ad61dbb..8fca25513d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1309,8 +1309,7 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize,
}
if (format == 'i') {
- int flags = 0;
- monitor_disas(mon, cs, addr, count, is_physical, flags);
+ monitor_disas(mon, cs, addr, count, is_physical);
return;
}
diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index 5a92c4accb..e9a245f9c5 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -3048,7 +3048,7 @@ static void alpha_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
static void alpha_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu)
{
qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
- log_target_disas(cpu, dcbase->pc_first, dcbase->tb->size, 1);
+ log_target_disas(cpu, dcbase->pc_first, dcbase->tb->size);
}
static const TranslatorOps alpha_tr_ops = {
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 899ffb96fc..0975c883a7 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 ab1a12a1b8..93e9dbe33d 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -12258,8 +12258,7 @@ static void arm_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu)
DisasContext *dc = container_of(dcbase, DisasContext, base);
qemu_log("IN: %s\n", lookup_symbol(dc->base.pc_first));
- log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size,
- dc->thumb | (dc->sctlr_b << 1));
+ log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size);
}
static const TranslatorOps arm_translator_ops = {
diff --git a/target/cris/translate.c b/target/cris/translate.c
index 38a999e6f1..b1fda57c74 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -3297,8 +3297,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
qemu_log_lock();
qemu_log("--------------\n");
qemu_log("IN: %s\n", lookup_symbol(pc_start));
- log_target_disas(cs, pc_start, dc->pc - pc_start,
- env->pregs[PR_VR]);
+ log_target_disas(cs, pc_start, dc->pc - pc_start);
qemu_log("\nisize=%d osize=%d\n",
dc->pc - pc_start, tcg_op_buf_count());
qemu_log_unlock();
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index b6e2652341..fc2a9f5896 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -3904,7 +3904,7 @@ static void hppa_tr_disas_log(const DisasContextBase *dcbase, CPUState *cs)
break;
default:
qemu_log("IN: %s\n", lookup_symbol(tb->pc));
- log_target_disas(cs, tb->pc, tb->size, 1);
+ log_target_disas(cs, tb->pc, tb->size);
break;
}
}
diff --git a/target/i386/translate.c b/target/i386/translate.c
index 9932d64f2e..6567e5438f 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -8529,7 +8529,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 d16d879df7..ea4a796381 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 6b0961837d..7a0fa860da 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -907,7 +907,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb)
&& qemu_log_in_addr_range(tb->pc)) {
qemu_log_lock();
qemu_log("IN: %s\n", lookup_symbol(tb->pc));
- log_target_disas(cs, tb->pc, dc->pc - tb->pc, 0);
+ log_target_disas(cs, tb->pc, dc->pc - tb->pc);
qemu_log("\n");
qemu_log_unlock();
}
diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index 112db1ad0f..99f2b463ce 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -1653,7 +1653,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
&& qemu_log_in_addr_range(pc_start)) {
- log_target_disas(cs, pc_start, tb->size, 0);
+ log_target_disas(cs, pc_start, tb->size);
qemu_log("\n");
qemu_log_unlock();
}
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index bc155f1036..8e92e4579c 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7397,7 +7397,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
&& qemu_log_in_addr_range(pc_start)) {
qemu_log_lock();
qemu_log("IN: %s\n", lookup_symbol(pc_start));
- log_target_disas(cs, pc_start, ctx.nip - pc_start, 0);
+ log_target_disas(cs, pc_start, ctx.nip - pc_start);
qemu_log("\n");
qemu_log_unlock();
}
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 5abd34fb34..9838ae4584 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -5907,7 +5907,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
qemu_log("IN: EXECUTE %016" PRIx64 "\n", dc.ex_value);
} else {
qemu_log("IN: %s\n", lookup_symbol(pc_start));
- log_target_disas(cs, pc_start, dc.pc - pc_start, 1);
+ log_target_disas(cs, pc_start, dc.pc - pc_start);
qemu_log("\n");
}
qemu_log_unlock();
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 10191073b2..7532bf74c1 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -2347,7 +2347,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
&& qemu_log_in_addr_range(pc_start)) {
qemu_log_lock();
qemu_log("IN:\n"); /* , lookup_symbol(pc_start)); */
- log_target_disas(cs, pc_start, ctx.pc - pc_start, 0);
+ log_target_disas(cs, pc_start, ctx.pc - pc_start);
qemu_log("\n");
qemu_log_unlock();
}
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 6290705b11..e89b6227f2 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -5855,7 +5855,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock * tb)
qemu_log_lock();
qemu_log("--------------\n");
qemu_log("IN: %s\n", lookup_symbol(pc_start));
- log_target_disas(cs, pc_start, last_pc + 4 - pc_start, 0);
+ log_target_disas(cs, pc_start, last_pc + 4 - pc_start);
qemu_log("\n");
qemu_log_unlock();
}
diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index 4e4198e887..e807500e26 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -8839,7 +8839,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
&& qemu_log_in_addr_range(pc_start)) {
qemu_log_lock();
qemu_log("IN: %s\n", lookup_symbol(pc_start));
- log_target_disas(cs, pc_start, ctx.pc - pc_start, 0);
+ log_target_disas(cs, pc_start, ctx.pc - pc_start);
qemu_log("\n");
qemu_log_unlock();
}
diff --git a/target/unicore32/translate.c b/target/unicore32/translate.c
index 6c094d59d7..f9aa248a80 100644
--- a/target/unicore32/translate.c
+++ b/target/unicore32/translate.c
@@ -2031,7 +2031,7 @@ done_generating:
qemu_log_lock();
qemu_log("----------------\n");
qemu_log("IN: %s\n", lookup_symbol(pc_start));
- log_target_disas(cs, pc_start, dc->pc - pc_start, 0);
+ log_target_disas(cs, pc_start, dc->pc - pc_start);
qemu_log("\n");
qemu_log_unlock();
}
diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index d7bf07e8e6..03719ce12b 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -3250,7 +3250,7 @@ done:
qemu_log_lock();
qemu_log("----------------\n");
qemu_log("IN: %s\n", lookup_symbol(pc_start));
- log_target_disas(cs, pc_start, dc.pc - pc_start, 0);
+ log_target_disas(cs, pc_start, dc.pc - pc_start);
qemu_log("\n");
qemu_log_unlock();
}
--
2.13.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/9] disas: Remove unused flags arguments
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 3/9] disas: Remove unused flags arguments Richard Henderson
@ 2017-10-02 12:56 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-10-02 12:56 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On 09/28/2017 01:54 PM, Richard Henderson wrote:
> Now that every target is using the disas_set_info hook,
> the flags argument is unused. Remove it.
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.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 3a375a3b6c..ad675dc361 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -171,15 +171,9 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info)
> return print_insn_objdump(pc, info, "OBJD-T");
> }
>
> -/* Disassemble this for me please... (debugging). 'flags' has the following
> - values:
> - i386 - 1 means 16 bit code, 2 means 64 bit code
> - ppc - bits 0:15 specify (optionally) the machine instruction set;
> - bit 16 indicates little endian.
> - other targets - unused
> - */
> +/* Disassemble this for me please... (debugging). */
> void target_disas(FILE *out, CPUState *cpu, target_ulong code,
> - target_ulong size, int flags)
> + target_ulong size)
> {
> CPUClass *cc = CPU_GET_CLASS(cpu);
> target_ulong pc;
> @@ -336,10 +330,9 @@ monitor_read_memory (bfd_vma memaddr, bfd_byte *myaddr, int length,
> return 0;
> }
>
> -/* Disassembler for the monitor.
> - See target_disas for a description of flags. */
> +/* Disassembler for the monitor. */
> void monitor_disas(Monitor *mon, CPUState *cpu,
> - target_ulong pc, int nb_insn, int is_physical, int flags)
> + target_ulong pc, int nb_insn, int is_physical)
> {
> CPUClass *cc = CPU_GET_CLASS(cpu);
> int count, i;
> diff --git a/monitor.c b/monitor.c
> index d55ad61dbb..8fca25513d 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1309,8 +1309,7 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize,
> }
>
> if (format == 'i') {
> - int flags = 0;
> - monitor_disas(mon, cs, addr, count, is_physical, flags);
> + monitor_disas(mon, cs, addr, count, is_physical);
> return;
> }
>
> diff --git a/target/alpha/translate.c b/target/alpha/translate.c
> index 5a92c4accb..e9a245f9c5 100644
> --- a/target/alpha/translate.c
> +++ b/target/alpha/translate.c
> @@ -3048,7 +3048,7 @@ static void alpha_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
> static void alpha_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu)
> {
> qemu_log("IN: %s\n", lookup_symbol(dcbase->pc_first));
> - log_target_disas(cpu, dcbase->pc_first, dcbase->tb->size, 1);
> + log_target_disas(cpu, dcbase->pc_first, dcbase->tb->size);
> }
>
> static const TranslatorOps alpha_tr_ops = {
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 899ffb96fc..0975c883a7 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 ab1a12a1b8..93e9dbe33d 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -12258,8 +12258,7 @@ static void arm_tr_disas_log(const DisasContextBase *dcbase, CPUState *cpu)
> DisasContext *dc = container_of(dcbase, DisasContext, base);
>
> qemu_log("IN: %s\n", lookup_symbol(dc->base.pc_first));
> - log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size,
> - dc->thumb | (dc->sctlr_b << 1));
> + log_target_disas(cpu, dc->base.pc_first, dc->base.tb->size);
> }
>
> static const TranslatorOps arm_translator_ops = {
> diff --git a/target/cris/translate.c b/target/cris/translate.c
> index 38a999e6f1..b1fda57c74 100644
> --- a/target/cris/translate.c
> +++ b/target/cris/translate.c
> @@ -3297,8 +3297,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
> qemu_log_lock();
> qemu_log("--------------\n");
> qemu_log("IN: %s\n", lookup_symbol(pc_start));
> - log_target_disas(cs, pc_start, dc->pc - pc_start,
> - env->pregs[PR_VR]);
> + log_target_disas(cs, pc_start, dc->pc - pc_start);
> qemu_log("\nisize=%d osize=%d\n",
> dc->pc - pc_start, tcg_op_buf_count());
> qemu_log_unlock();
> diff --git a/target/hppa/translate.c b/target/hppa/translate.c
> index b6e2652341..fc2a9f5896 100644
> --- a/target/hppa/translate.c
> +++ b/target/hppa/translate.c
> @@ -3904,7 +3904,7 @@ static void hppa_tr_disas_log(const DisasContextBase *dcbase, CPUState *cs)
> break;
> default:
> qemu_log("IN: %s\n", lookup_symbol(tb->pc));
> - log_target_disas(cs, tb->pc, tb->size, 1);
> + log_target_disas(cs, tb->pc, tb->size);
> break;
> }
> }
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index 9932d64f2e..6567e5438f 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -8529,7 +8529,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 d16d879df7..ea4a796381 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 6b0961837d..7a0fa860da 100644
> --- a/target/nios2/translate.c
> +++ b/target/nios2/translate.c
> @@ -907,7 +907,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb)
> && qemu_log_in_addr_range(tb->pc)) {
> qemu_log_lock();
> qemu_log("IN: %s\n", lookup_symbol(tb->pc));
> - log_target_disas(cs, tb->pc, dc->pc - tb->pc, 0);
> + log_target_disas(cs, tb->pc, dc->pc - tb->pc);
> qemu_log("\n");
> qemu_log_unlock();
> }
> diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
> index 112db1ad0f..99f2b463ce 100644
> --- a/target/openrisc/translate.c
> +++ b/target/openrisc/translate.c
> @@ -1653,7 +1653,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
>
> if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
> && qemu_log_in_addr_range(pc_start)) {
> - log_target_disas(cs, pc_start, tb->size, 0);
> + log_target_disas(cs, pc_start, tb->size);
> qemu_log("\n");
> qemu_log_unlock();
> }
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index bc155f1036..8e92e4579c 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -7397,7 +7397,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
> && qemu_log_in_addr_range(pc_start)) {
> qemu_log_lock();
> qemu_log("IN: %s\n", lookup_symbol(pc_start));
> - log_target_disas(cs, pc_start, ctx.nip - pc_start, 0);
> + log_target_disas(cs, pc_start, ctx.nip - pc_start);
> qemu_log("\n");
> qemu_log_unlock();
> }
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 5abd34fb34..9838ae4584 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -5907,7 +5907,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
> qemu_log("IN: EXECUTE %016" PRIx64 "\n", dc.ex_value);
> } else {
> qemu_log("IN: %s\n", lookup_symbol(pc_start));
> - log_target_disas(cs, pc_start, dc.pc - pc_start, 1);
> + log_target_disas(cs, pc_start, dc.pc - pc_start);
> qemu_log("\n");
> }
> qemu_log_unlock();
> diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> index 10191073b2..7532bf74c1 100644
> --- a/target/sh4/translate.c
> +++ b/target/sh4/translate.c
> @@ -2347,7 +2347,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
> && qemu_log_in_addr_range(pc_start)) {
> qemu_log_lock();
> qemu_log("IN:\n"); /* , lookup_symbol(pc_start)); */
> - log_target_disas(cs, pc_start, ctx.pc - pc_start, 0);
> + log_target_disas(cs, pc_start, ctx.pc - pc_start);
> qemu_log("\n");
> qemu_log_unlock();
> }
> diff --git a/target/sparc/translate.c b/target/sparc/translate.c
> index 6290705b11..e89b6227f2 100644
> --- a/target/sparc/translate.c
> +++ b/target/sparc/translate.c
> @@ -5855,7 +5855,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock * tb)
> qemu_log_lock();
> qemu_log("--------------\n");
> qemu_log("IN: %s\n", lookup_symbol(pc_start));
> - log_target_disas(cs, pc_start, last_pc + 4 - pc_start, 0);
> + log_target_disas(cs, pc_start, last_pc + 4 - pc_start);
> qemu_log("\n");
> qemu_log_unlock();
> }
> diff --git a/target/tricore/translate.c b/target/tricore/translate.c
> index 4e4198e887..e807500e26 100644
> --- a/target/tricore/translate.c
> +++ b/target/tricore/translate.c
> @@ -8839,7 +8839,7 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
> && qemu_log_in_addr_range(pc_start)) {
> qemu_log_lock();
> qemu_log("IN: %s\n", lookup_symbol(pc_start));
> - log_target_disas(cs, pc_start, ctx.pc - pc_start, 0);
> + log_target_disas(cs, pc_start, ctx.pc - pc_start);
> qemu_log("\n");
> qemu_log_unlock();
> }
> diff --git a/target/unicore32/translate.c b/target/unicore32/translate.c
> index 6c094d59d7..f9aa248a80 100644
> --- a/target/unicore32/translate.c
> +++ b/target/unicore32/translate.c
> @@ -2031,7 +2031,7 @@ done_generating:
> qemu_log_lock();
> qemu_log("----------------\n");
> qemu_log("IN: %s\n", lookup_symbol(pc_start));
> - log_target_disas(cs, pc_start, dc->pc - pc_start, 0);
> + log_target_disas(cs, pc_start, dc->pc - pc_start);
> qemu_log("\n");
> qemu_log_unlock();
> }
> diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
> index d7bf07e8e6..03719ce12b 100644
> --- a/target/xtensa/translate.c
> +++ b/target/xtensa/translate.c
> @@ -3250,7 +3250,7 @@ done:
> qemu_log_lock();
> qemu_log("----------------\n");
> qemu_log("IN: %s\n", lookup_symbol(pc_start));
> - log_target_disas(cs, pc_start, dc.pc - pc_start, 0);
> + log_target_disas(cs, pc_start, dc.pc - pc_start);
> qemu_log("\n");
> qemu_log_unlock();
> }
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v4 4/9] disas: Support the Capstone disassembler library
2017-09-28 16:54 [Qemu-devel] [PATCH v4 0/9] Support the Capstone disassembler Richard Henderson
` (2 preceding siblings ...)
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 3/9] disas: Remove unused flags arguments Richard Henderson
@ 2017-09-28 16:54 ` Richard Henderson
2017-10-02 13:36 ` Philippe Mathieu-Daudé
2017-10-02 14:40 ` Alex Bennée
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 5/9] i386: Support Capstone in disas_set_info Richard Henderson
` (5 subsequent siblings)
9 siblings, 2 replies; 30+ messages in thread
From: Richard Henderson @ 2017-09-28 16:54 UTC (permalink / raw)
To: qemu-devel
If configured, prefer this over our rather dated copy of the
GPLv2-only binutils. This will be especially apparent with
the proposed vector extensions to TCG, as disas/i386.c does
not handle AVX.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/disas/bfd.h | 4 +
include/disas/capstone.h | 38 ++++++++
disas.c | 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 b01e002b4c..0f4ecdeb88 100644
--- a/include/disas/bfd.h
+++ b/include/disas/bfd.h
@@ -377,6 +377,10 @@ typedef struct disassemble_info {
/* Command line options specific to the target disassembler. */
char * disassembler_options;
+ /* Options for Capstone disassembly. */
+ int cap_arch;
+ int cap_mode;
+
} disassemble_info;
\f
diff --git a/include/disas/capstone.h b/include/disas/capstone.h
new file mode 100644
index 0000000000..84e214956d
--- /dev/null
+++ b/include/disas/capstone.h
@@ -0,0 +1,38 @@
+#ifndef QEMU_CAPSTONE_H
+#define QEMU_CAPSTONE_H 1
+
+#ifdef CONFIG_CAPSTONE
+
+#include <capstone.h>
+
+#else
+
+/* Just enough to allow backends to init without ifdefs. */
+
+#define CS_ARCH_ARM -1
+#define CS_ARCH_ARM64 -1
+#define CS_ARCH_MIPS -1
+#define CS_ARCH_X86 -1
+#define CS_ARCH_PPC -1
+#define CS_ARCH_SPARC -1
+#define CS_ARCH_SYSZ -1
+
+#define CS_MODE_LITTLE_ENDIAN 0
+#define CS_MODE_BIG_ENDIAN 0
+#define CS_MODE_ARM 0
+#define CS_MODE_16 0
+#define CS_MODE_32 0
+#define CS_MODE_64 0
+#define CS_MODE_THUMB 0
+#define CS_MODE_MCLASS 0
+#define CS_MODE_V8 0
+#define CS_MODE_MICRO 0
+#define CS_MODE_MIPS3 0
+#define CS_MODE_MIPS32R6 0
+#define CS_MODE_MIPSGP64 0
+#define CS_MODE_V9 0
+#define CS_MODE_MIPS32 0
+#define CS_MODE_MIPS64 0
+
+#endif /* CONFIG_CAPSTONE */
+#endif /* QEMU_CAPSTONE_H */
diff --git a/disas.c b/disas.c
index ad675dc361..746d76c07d 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)
@@ -188,6 +375,8 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
s.info.buffer_vma = code;
s.info.buffer_length = size;
s.info.print_address_func = generic_print_address;
+ s.info.cap_arch = -1;
+ s.info.cap_mode = 0;
#ifdef TARGET_WORDS_BIGENDIAN
s.info.endian = BFD_ENDIAN_BIG;
@@ -199,6 +388,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;
}
@@ -206,18 +399,6 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
for (pc = code; size > 0; pc += count, size -= count) {
fprintf(out, "0x" TARGET_FMT_lx ": ", pc);
count = s.info.print_insn(pc, &s.info);
-#if 0
- {
- int i;
- uint8_t b;
- fprintf(out, " {");
- for(i = 0; i < count; i++) {
- target_read_memory(pc + i, &b, 1, &s.info);
- fprintf(out, " %02x", b);
- }
- fprintf(out, " }");
- }
-#endif
fprintf(out, "\n");
if (count < 0)
break;
@@ -245,6 +426,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;
@@ -282,6 +465,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;
}
@@ -344,8 +532,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;
@@ -357,6 +546,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 6587e8014b..3777db91b6 100755
--- a/configure
+++ b/configure
@@ -367,6 +367,7 @@ opengl_dmabuf="no"
cpuid_h="no"
avx2_opt="no"
zlib="yes"
+capstone=""
lzo=""
snappy=""
bzip2=""
@@ -1286,6 +1287,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"
@@ -1533,6 +1538,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
@@ -4371,6 +4377,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
@@ -5423,6 +5445,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"
@@ -6088,6 +6111,9 @@ fi
if test "$ivshmem" = "yes" ; then
echo "CONFIG_IVSHMEM=y" >> $config_host_mak
fi
+if test "$capstone" = "yes" ; then
+ echo "CONFIG_CAPSTONE=y" >> $config_host_mak
+fi
# Hold two types of flag:
# CONFIG_THREAD_SETNAME_BYTHREAD - we've got a way of setting the name on
--
2.13.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 4/9] disas: Support the Capstone disassembler library
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 4/9] disas: Support the Capstone disassembler library Richard Henderson
@ 2017-10-02 13:36 ` Philippe Mathieu-Daudé
2017-10-02 18:34 ` Richard Henderson
2017-10-02 14:40 ` Alex Bennée
1 sibling, 1 reply; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-10-02 13:36 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On 09/28/2017 01:54 PM, Richard Henderson wrote:
> If configured, prefer this over our rather dated copy of the
> GPLv2-only binutils. This will be especially apparent with
> the proposed vector extensions to TCG, as disas/i386.c does
> not handle AVX.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/disas/bfd.h | 4 +
> include/disas/capstone.h | 38 ++++++++
> disas.c | 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 b01e002b4c..0f4ecdeb88 100644
> --- a/include/disas/bfd.h
> +++ b/include/disas/bfd.h
> @@ -377,6 +377,10 @@ typedef struct disassemble_info {
> /* Command line options specific to the target disassembler. */
> char * disassembler_options;
>
> + /* Options for Capstone disassembly. */
> + int cap_arch;
> + int cap_mode;
> +
> } disassemble_info;
>
> \f
> diff --git a/include/disas/capstone.h b/include/disas/capstone.h
> new file mode 100644
> index 0000000000..84e214956d
> --- /dev/null
> +++ b/include/disas/capstone.h
> @@ -0,0 +1,38 @@
> +#ifndef QEMU_CAPSTONE_H
> +#define QEMU_CAPSTONE_H 1
> +
> +#ifdef CONFIG_CAPSTONE
> +
> +#include <capstone.h>
> +
> +#else
> +
> +/* Just enough to allow backends to init without ifdefs. */
> +
> +#define CS_ARCH_ARM -1
> +#define CS_ARCH_ARM64 -1
> +#define CS_ARCH_MIPS -1
> +#define CS_ARCH_X86 -1
> +#define CS_ARCH_PPC -1
> +#define CS_ARCH_SPARC -1
> +#define CS_ARCH_SYSZ -1
> +
> +#define CS_MODE_LITTLE_ENDIAN 0
> +#define CS_MODE_BIG_ENDIAN 0
> +#define CS_MODE_ARM 0
> +#define CS_MODE_16 0
> +#define CS_MODE_32 0
> +#define CS_MODE_64 0
> +#define CS_MODE_THUMB 0
> +#define CS_MODE_MCLASS 0
> +#define CS_MODE_V8 0
> +#define CS_MODE_MICRO 0
> +#define CS_MODE_MIPS3 0
> +#define CS_MODE_MIPS32R6 0
> +#define CS_MODE_MIPSGP64 0
> +#define CS_MODE_V9 0
> +#define CS_MODE_MIPS32 0
> +#define CS_MODE_MIPS64 0
> +
> +#endif /* CONFIG_CAPSTONE */
> +#endif /* QEMU_CAPSTONE_H */
> diff --git a/disas.c b/disas.c
> index ad675dc361..746d76c07d 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)) {
way nicer than v1 :)
> + (*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");
couldn't trigger this so far.
> + }
> + 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");
neither this one, but you'll be the first notified!
> + }
> +
> + cs_close(&handle);
> + return true;
> +}
> +
> +#if !defined(CONFIG_USER_ONLY)
Maybe some defines like:
#define LARGEST_TARGET_INSTR_SIZE 32
#define MINIMUM_TARGET_PAGE_SIZE 1024
> +/* 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)
> @@ -188,6 +375,8 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
> s.info.buffer_vma = code;
> s.info.buffer_length = size;
> s.info.print_address_func = generic_print_address;
> + s.info.cap_arch = -1;
> + s.info.cap_mode = 0;
>
> #ifdef TARGET_WORDS_BIGENDIAN
> s.info.endian = BFD_ENDIAN_BIG;
> @@ -199,6 +388,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;
> }
> @@ -206,18 +399,6 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
> for (pc = code; size > 0; pc += count, size -= count) {
> fprintf(out, "0x" TARGET_FMT_lx ": ", pc);
> count = s.info.print_insn(pc, &s.info);
> -#if 0
> - {
> - int i;
> - uint8_t b;
> - fprintf(out, " {");
> - for(i = 0; i < count; i++) {
> - target_read_memory(pc + i, &b, 1, &s.info);
> - fprintf(out, " %02x", b);
> - }
> - fprintf(out, " }");
> - }
> -#endif
> fprintf(out, "\n");
> if (count < 0)
> break;
> @@ -245,6 +426,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;
> @@ -282,6 +465,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;
> }
> @@ -344,8 +532,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;
> @@ -357,6 +546,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 6587e8014b..3777db91b6 100755
> --- a/configure
> +++ b/configure
> @@ -367,6 +367,7 @@ opengl_dmabuf="no"
> cpuid_h="no"
> avx2_opt="no"
> zlib="yes"
> +capstone=""
> lzo=""
> snappy=""
> bzip2=""
> @@ -1286,6 +1287,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"
> @@ -1533,6 +1538,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
> @@ -4371,6 +4377,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
> @@ -5423,6 +5445,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"
> @@ -6088,6 +6111,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
>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 4/9] disas: Support the Capstone disassembler library
2017-10-02 13:36 ` Philippe Mathieu-Daudé
@ 2017-10-02 18:34 ` Richard Henderson
2017-10-02 18:45 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2017-10-02 18:34 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-devel
On 10/02/2017 09:36 AM, Philippe Mathieu-Daudé wrote:
> Maybe some defines like:
>
> #define LARGEST_TARGET_INSTR_SIZE 32
>
> #define MINIMUM_TARGET_PAGE_SIZE 1024
Eh. If I weren't simply pulling numbers out of my hat, perhaps.
Does it really make things clearer beyond sizeof or a great big comment?
r~
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4/9] disas: Support the Capstone disassembler library
2017-10-02 18:34 ` Richard Henderson
@ 2017-10-02 18:45 ` Philippe Mathieu-Daudé
2017-10-03 1:51 ` Richard Henderson
0 siblings, 1 reply; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-10-02 18:45 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On 10/02/2017 03:34 PM, Richard Henderson wrote:
> On 10/02/2017 09:36 AM, Philippe Mathieu-Daudé wrote:
>> Maybe some defines like:
>>
>> #define LARGEST_TARGET_INSTR_SIZE 32
>>
>> #define MINIMUM_TARGET_PAGE_SIZE 1024
>
> Eh. If I weren't simply pulling numbers out of my hat, perhaps.
> Does it really make things clearer beyond sizeof or a great big comment?
big comment is great!
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4/9] disas: Support the Capstone disassembler library
2017-10-02 18:45 ` Philippe Mathieu-Daudé
@ 2017-10-03 1:51 ` Richard Henderson
2017-10-03 13:45 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2017-10-03 1:51 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-devel
On 10/02/2017 02:45 PM, Philippe Mathieu-Daudé wrote:
> On 10/02/2017 03:34 PM, Richard Henderson wrote:
>> On 10/02/2017 09:36 AM, Philippe Mathieu-Daudé wrote:
>>> Maybe some defines like:
>>>
>>> #define LARGEST_TARGET_INSTR_SIZE 32
>>>
>>> #define MINIMUM_TARGET_PAGE_SIZE 1024
>>
>> Eh. If I weren't simply pulling numbers out of my hat, perhaps.
>> Does it really make things clearer beyond sizeof or a great big comment?
>
> big comment is great!
Like the one that's already there?
+ /* 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. */
r~
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4/9] disas: Support the Capstone disassembler library
2017-10-03 1:51 ` Richard Henderson
@ 2017-10-03 13:45 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-10-03 13:45 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On 10/02/2017 10:51 PM, Richard Henderson wrote:
> On 10/02/2017 02:45 PM, Philippe Mathieu-Daudé wrote:
>> On 10/02/2017 03:34 PM, Richard Henderson wrote:
>>> On 10/02/2017 09:36 AM, Philippe Mathieu-Daudé wrote:
>>>> Maybe some defines like:
>>>>
>>>> #define LARGEST_TARGET_INSTR_SIZE 32
>>>>
>>>> #define MINIMUM_TARGET_PAGE_SIZE 1024
>>>
>>> Eh. If I weren't simply pulling numbers out of my hat, perhaps.
>>> Does it really make things clearer beyond sizeof or a great big comment?
>>
>> big comment is great!
>
> Like the one that's already there?
Yes, sorry for being that unclear, I wanted to say "_this_ big comment
_is_ great!". I suppose I think about using #defines because my English
isn't that conveying than yours :)
>
> + /* 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. */
>
>
> r~
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4/9] disas: Support the Capstone disassembler library
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 4/9] disas: Support the Capstone disassembler library Richard Henderson
2017-10-02 13:36 ` Philippe Mathieu-Daudé
@ 2017-10-02 14:40 ` Alex Bennée
2017-10-02 18:31 ` Richard Henderson
1 sibling, 1 reply; 30+ messages in thread
From: Alex Bennée @ 2017-10-02 14:40 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> If configured, prefer this over our rather dated copy of the
> GPLv2-only binutils. This will be especially apparent with
> the proposed vector extensions to TCG, as disas/i386.c does
> not handle AVX.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/disas/bfd.h | 4 +
> include/disas/capstone.h | 38 ++++++++
> disas.c | 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 b01e002b4c..0f4ecdeb88 100644
> --- a/include/disas/bfd.h
> +++ b/include/disas/bfd.h
> @@ -377,6 +377,10 @@ typedef struct disassemble_info {
> /* Command line options specific to the target disassembler. */
> char * disassembler_options;
>
> + /* Options for Capstone disassembly. */
> + int cap_arch;
> + int cap_mode;
> +
> } disassemble_info;
>
> \f
> diff --git a/include/disas/capstone.h b/include/disas/capstone.h
> new file mode 100644
> index 0000000000..84e214956d
> --- /dev/null
> +++ b/include/disas/capstone.h
> @@ -0,0 +1,38 @@
> +#ifndef QEMU_CAPSTONE_H
> +#define QEMU_CAPSTONE_H 1
> +
> +#ifdef CONFIG_CAPSTONE
> +
> +#include <capstone.h>
> +
> +#else
> +
> +/* Just enough to allow backends to init without ifdefs. */
> +
> +#define CS_ARCH_ARM -1
> +#define CS_ARCH_ARM64 -1
> +#define CS_ARCH_MIPS -1
> +#define CS_ARCH_X86 -1
> +#define CS_ARCH_PPC -1
> +#define CS_ARCH_SPARC -1
> +#define CS_ARCH_SYSZ -1
> +
> +#define CS_MODE_LITTLE_ENDIAN 0
> +#define CS_MODE_BIG_ENDIAN 0
> +#define CS_MODE_ARM 0
> +#define CS_MODE_16 0
> +#define CS_MODE_32 0
> +#define CS_MODE_64 0
> +#define CS_MODE_THUMB 0
> +#define CS_MODE_MCLASS 0
> +#define CS_MODE_V8 0
> +#define CS_MODE_MICRO 0
> +#define CS_MODE_MIPS3 0
> +#define CS_MODE_MIPS32R6 0
> +#define CS_MODE_MIPSGP64 0
> +#define CS_MODE_V9 0
> +#define CS_MODE_MIPS32 0
> +#define CS_MODE_MIPS64 0
> +
> +#endif /* CONFIG_CAPSTONE */
> +#endif /* QEMU_CAPSTONE_H */
> diff --git a/disas.c b/disas.c
> index ad675dc361..746d76c07d 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) {
if (!cap_insn)?
> + 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) {
Given we are moving stuff and both size/csize are of size_t wouldn't if
(size > 0) be clearer here?
> + memmove(cap_buf, cbuf, csize);
> + }
> + continue;
> + }
> +
> + /* Since the target memory is consumed, we should not have
> + a remaining fractional insn. */
> + if (csize != 0) {
And here
> + (*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)
> @@ -188,6 +375,8 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
> s.info.buffer_vma = code;
> s.info.buffer_length = size;
> s.info.print_address_func = generic_print_address;
> + s.info.cap_arch = -1;
> + s.info.cap_mode = 0;
>
> #ifdef TARGET_WORDS_BIGENDIAN
> s.info.endian = BFD_ENDIAN_BIG;
> @@ -199,6 +388,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;
> }
> @@ -206,18 +399,6 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
> for (pc = code; size > 0; pc += count, size -= count) {
> fprintf(out, "0x" TARGET_FMT_lx ": ", pc);
> count = s.info.print_insn(pc, &s.info);
> -#if 0
> - {
> - int i;
> - uint8_t b;
> - fprintf(out, " {");
> - for(i = 0; i < count; i++) {
> - target_read_memory(pc + i, &b, 1, &s.info);
> - fprintf(out, " %02x", b);
> - }
> - fprintf(out, " }");
> - }
> -#endif
> fprintf(out, "\n");
> if (count < 0)
> break;
> @@ -245,6 +426,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;
> @@ -282,6 +465,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;
> }
> @@ -344,8 +532,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;
> @@ -357,6 +546,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 6587e8014b..3777db91b6 100755
> --- a/configure
> +++ b/configure
> @@ -367,6 +367,7 @@ opengl_dmabuf="no"
> cpuid_h="no"
> avx2_opt="no"
> zlib="yes"
> +capstone=""
> lzo=""
> snappy=""
> bzip2=""
> @@ -1286,6 +1287,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"
> @@ -1533,6 +1538,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
> @@ -4371,6 +4377,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
> @@ -5423,6 +5445,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"
> @@ -6088,6 +6111,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
Other than those minor nits:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 4/9] disas: Support the Capstone disassembler library
2017-10-02 14:40 ` Alex Bennée
@ 2017-10-02 18:31 ` Richard Henderson
0 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2017-10-02 18:31 UTC (permalink / raw)
To: Alex Bennée; +Cc: qemu-devel
On 10/02/2017 10:40 AM, Alex Bennée wrote:
>> + if (cap_insn == NULL) {
>> + cap_insn = cs_malloc(*handle);
>> + if (cap_insn == NULL) {
>
> if (!cap_insn)?
Eh. Depends on your philosophy, I suppose. I'm not especially keen on !
except on boolean expressions. Though of course both expressions are identical.
>> + /* 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) {
>
> Given we are moving stuff and both size/csize are of size_t wouldn't if
> (size > 0) be clearer here?
*shrug* I dunno, would it? For me, not obviously so. Of course the two
expressions compile to the same code.
r~
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v4 5/9] i386: Support Capstone in disas_set_info
2017-09-28 16:54 [Qemu-devel] [PATCH v4 0/9] Support the Capstone disassembler Richard Henderson
` (3 preceding siblings ...)
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 4/9] disas: Support the Capstone disassembler library Richard Henderson
@ 2017-09-28 16:54 ` Richard Henderson
2017-10-02 13:37 ` Philippe Mathieu-Daudé
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 6/9] arm: " Richard Henderson
` (4 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2017-09-28 16:54 UTC (permalink / raw)
To: qemu-devel
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 746d76c07d..1c44514254 100644
--- a/disas.c
+++ b/disas.c
@@ -439,9 +439,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.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 5/9] i386: Support Capstone in disas_set_info
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 5/9] i386: Support Capstone in disas_set_info Richard Henderson
@ 2017-10-02 13:37 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-10-02 13:37 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On 09/28/2017 01:54 PM, Richard Henderson wrote:
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
i386, x86_64:
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.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 746d76c07d..1c44514254 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -439,9 +439,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[] = {
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v4 6/9] arm: Support Capstone in disas_set_info
2017-09-28 16:54 [Qemu-devel] [PATCH v4 0/9] Support the Capstone disassembler Richard Henderson
` (4 preceding siblings ...)
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 5/9] i386: Support Capstone in disas_set_info Richard Henderson
@ 2017-09-28 16:54 ` Richard Henderson
2017-10-02 13:37 ` Philippe Mathieu-Daudé
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 7/9] ppc: " Richard Henderson
` (3 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2017-09-28 16:54 UTC (permalink / raw)
To: qemu-devel
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.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 1c44514254..23c4742f8d 100644
--- a/disas.c
+++ b/disas.c
@@ -451,6 +451,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__)
@@ -458,6 +459,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 4300de66e2..e5f84066b4 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)
{
@@ -489,10 +490,24 @@ static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
#if defined(CONFIG_ARM_A64_DIS)
info->print_insn = print_insn_arm_a64;
#endif
- } else if (env->thumb) {
- info->print_insn = print_insn_thumb1;
+ info->cap_arch = CS_ARCH_ARM64;
} else {
- info->print_insn = print_insn_arm;
+ int cap_mode;
+ if (env->thumb) {
+ info->print_insn = print_insn_thumb1;
+ cap_mode = CS_MODE_THUMB;
+ } else {
+ info->print_insn = print_insn_arm;
+ cap_mode = CS_MODE_ARM;
+ }
+ if (arm_feature(env, ARM_FEATURE_V8)) {
+ cap_mode |= CS_MODE_V8;
+ }
+ if (arm_feature(env, ARM_FEATURE_M)) {
+ cap_mode |= CS_MODE_MCLASS;
+ }
+ info->cap_arch = CS_ARCH_ARM;
+ info->cap_mode = cap_mode;
}
if (bswap_code(arm_sctlr_b(env))) {
#ifdef TARGET_WORDS_BIGENDIAN
--
2.13.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 6/9] arm: Support Capstone in disas_set_info
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 6/9] arm: " Richard Henderson
@ 2017-10-02 13:37 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-10-02 13:37 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On 09/28/2017 01:54 PM, Richard Henderson wrote:
> Tested-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
aarch32 (not thumb), aarch64 (without switching to aarch32):
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-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 1c44514254..23c4742f8d 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -451,6 +451,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__)
> @@ -458,6 +459,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 4300de66e2..e5f84066b4 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)
> {
> @@ -489,10 +490,24 @@ static void arm_disas_set_info(CPUState *cpu, disassemble_info *info)
> #if defined(CONFIG_ARM_A64_DIS)
> info->print_insn = print_insn_arm_a64;
> #endif
> - } else if (env->thumb) {
> - info->print_insn = print_insn_thumb1;
> + info->cap_arch = CS_ARCH_ARM64;
> } else {
> - info->print_insn = print_insn_arm;
> + int cap_mode;
> + if (env->thumb) {
> + info->print_insn = print_insn_thumb1;
> + cap_mode = CS_MODE_THUMB;
> + } else {
> + info->print_insn = print_insn_arm;
> + cap_mode = CS_MODE_ARM;
> + }
> + if (arm_feature(env, ARM_FEATURE_V8)) {
> + cap_mode |= CS_MODE_V8;
> + }
> + if (arm_feature(env, ARM_FEATURE_M)) {
> + cap_mode |= CS_MODE_MCLASS;
> + }
> + info->cap_arch = CS_ARCH_ARM;
> + info->cap_mode = cap_mode;
> }
> if (bswap_code(arm_sctlr_b(env))) {
> #ifdef TARGET_WORDS_BIGENDIAN
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v4 7/9] ppc: Support Capstone in disas_set_info
2017-09-28 16:54 [Qemu-devel] [PATCH v4 0/9] Support the Capstone disassembler Richard Henderson
` (5 preceding siblings ...)
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 6/9] arm: " Richard Henderson
@ 2017-09-28 16:54 ` Richard Henderson
2017-10-02 13:56 ` Philippe Mathieu-Daudé
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 8/9] disas: Remove monitor_disas_is_physical Richard Henderson
` (2 subsequent siblings)
9 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2017-09-28 16:54 UTC (permalink / raw)
To: qemu-devel
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 23c4742f8d..0d212f2ac5 100644
--- a/disas.c
+++ b/disas.c
@@ -449,6 +449,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 2863e2c0b0..69a9485f66 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
@@ -10699,6 +10700,11 @@ static void ppc_disas_set_info(CPUState *cs, disassemble_info *info)
}
info->disassembler_options = (char *)"any";
info->print_insn = print_insn_ppc;
+
+ info->cap_arch = CS_ARCH_PPC;
+#ifdef TARGET_PPC64
+ info->cap_mode = CS_MODE_64;
+#endif
}
static Property ppc_cpu_properties[] = {
--
2.13.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 7/9] ppc: Support Capstone in disas_set_info
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 7/9] ppc: " Richard Henderson
@ 2017-10-02 13:56 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-10-02 13:56 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On 09/28/2017 01:54 PM, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> disas.c | 4 ++++
> target/ppc/translate_init.c | 6 ++++++
> 2 files changed, 10 insertions(+)
>
> diff --git a/disas.c b/disas.c
> index 23c4742f8d..0d212f2ac5 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -449,6 +449,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 2863e2c0b0..69a9485f66 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
> @@ -10699,6 +10700,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[] = {
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v4 8/9] disas: Remove monitor_disas_is_physical
2017-09-28 16:54 [Qemu-devel] [PATCH v4 0/9] Support the Capstone disassembler Richard Henderson
` (6 preceding siblings ...)
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 7/9] ppc: " Richard Henderson
@ 2017-09-28 16:54 ` Richard Henderson
2017-10-02 13:55 ` Philippe Mathieu-Daudé
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 9/9] disas: Add capstone as submodule Richard Henderson
2017-10-12 12:34 ` [Qemu-devel] [PATCH v4 0/9] Support the Capstone disassembler Peter Maydell
9 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2017-09-28 16:54 UTC (permalink / raw)
To: qemu-devel
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.
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 0d212f2ac5..194c523885 100644
--- a/disas.c
+++ b/disas.c
@@ -513,19 +513,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;
}
@@ -540,8 +532,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.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 8/9] disas: Remove monitor_disas_is_physical
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 8/9] disas: Remove monitor_disas_is_physical Richard Henderson
@ 2017-10-02 13:55 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-10-02 13:55 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On 09/28/2017 01:54 PM, Richard Henderson wrote:
> 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.
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.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 0d212f2ac5..194c523885 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -513,19 +513,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;
> }
>
> @@ -540,8 +532,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;
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v4 9/9] disas: Add capstone as submodule
2017-09-28 16:54 [Qemu-devel] [PATCH v4 0/9] Support the Capstone disassembler Richard Henderson
` (7 preceding siblings ...)
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 8/9] disas: Remove monitor_disas_is_physical Richard Henderson
@ 2017-09-28 16:54 ` Richard Henderson
2017-10-02 13:54 ` Philippe Mathieu-Daudé
2017-10-02 14:43 ` Alex Bennée
2017-10-12 12:34 ` [Qemu-devel] [PATCH v4 0/9] Support the Capstone disassembler Peter Maydell
9 siblings, 2 replies; 30+ messages in thread
From: Richard Henderson @ 2017-09-28 16:54 UTC (permalink / raw)
To: qemu-devel
Do not require the submodule, but use it if present (in preference
even to a system copy). This will allow us to easily use capstone
in older systems for which a package is not available, and also
easily track bug fixes from upstream.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Makefile | 13 +++++++++++++
.gitmodules | 3 +++
capstone | 1 +
configure | 12 +++++++++++-
4 files changed, 28 insertions(+), 1 deletion(-)
create mode 160000 capstone
diff --git a/Makefile b/Makefile
index cee6e28659..7ff366f6ca 100644
--- a/Makefile
+++ b/Makefile
@@ -336,6 +336,19 @@ subdir-dtc:dtc/libfdt dtc/tests
dtc/%:
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.
+CAP_CFLAGS = $(subst -Werror,,$(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:
+ $(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 84c54cdc49..bcb80d701a 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -34,3 +34,6 @@
[submodule "roms/QemuMacDrivers"]
path = roms/QemuMacDrivers
url = git://git.qemu.org/QemuMacDrivers.git
+[submodule "capstone"]
+ path = capstone
+ url = https://github.com/aquynh/capstone.git
diff --git a/capstone b/capstone
new file mode 160000
index 0000000000..a279481dbf
--- /dev/null
+++ b/capstone
@@ -0,0 +1 @@
+Subproject commit a279481dbfd54bb1e2336d771e89978cc6d43176
diff --git a/configure b/configure
index 3777db91b6..a6cf0a5a15 100755
--- a/configure
+++ b/configure
@@ -4380,7 +4380,14 @@ fi
# capstone
if test "$capstone" != no; then
- if $pkg_config capstone; then
+ if test -f ${source_path}/capstone/Makefile ; then
+ # have submodule capstone - use it
+ capstone=yes
+ capstone_internal=yes
+ mkdir -p capstone
+ QEMU_CFLAGS="$QEMU_CFLAGS -I\$(SRC_PATH)/capstone/include"
+ LIBS="\$(BUILD_DIR)/capstone/libcapstone.a $LIBS"
+ elif $pkg_config capstone; then
capstone=yes
QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
LIBS="$($pkg_config --libs capstone) $LIBS"
@@ -6596,6 +6603,9 @@ done # for target in $targets
if [ "$dtc_internal" = "yes" ]; then
echo "config-host.h: subdir-dtc" >> $config_host_mak
fi
+if [ "$capstone_internal" = "yes" ]; 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.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 9/9] disas: Add capstone as submodule
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 9/9] disas: Add capstone as submodule Richard Henderson
@ 2017-10-02 13:54 ` Philippe Mathieu-Daudé
2017-10-02 14:43 ` Alex Bennée
1 sibling, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-10-02 13:54 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
Hi Richard,
just a small suggestion to have more useful output if some user fill a
bug report.
On 09/28/2017 01:54 PM, Richard Henderson wrote:
> Do not require the submodule, but use it if present (in preference
> even to a system copy). This will allow us to easily use capstone
> in older systems for which a package is not available, and also
> easily track bug fixes from upstream.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> Makefile | 13 +++++++++++++
> .gitmodules | 3 +++
> capstone | 1 +
> configure | 12 +++++++++++-
> 4 files changed, 28 insertions(+), 1 deletion(-)
> create mode 160000 capstone
>
> diff --git a/Makefile b/Makefile
> index cee6e28659..7ff366f6ca 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -336,6 +336,19 @@ subdir-dtc:dtc/libfdt dtc/tests
> dtc/%:
> 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.
> +CAP_CFLAGS = $(subst -Werror,,$(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:
> + $(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 84c54cdc49..bcb80d701a 100644
> --- a/.gitmodules
> +++ b/.gitmodules
> @@ -34,3 +34,6 @@
> [submodule "roms/QemuMacDrivers"]
> path = roms/QemuMacDrivers
> url = git://git.qemu.org/QemuMacDrivers.git
> +[submodule "capstone"]
> + path = capstone
> + url = https://github.com/aquynh/capstone.git
> diff --git a/capstone b/capstone
> new file mode 160000
> index 0000000000..a279481dbf
> --- /dev/null
> +++ b/capstone
> @@ -0,0 +1 @@
> +Subproject commit a279481dbfd54bb1e2336d771e89978cc6d43176
> diff --git a/configure b/configure
> index 3777db91b6..a6cf0a5a15 100755
> --- a/configure
> +++ b/configure
> @@ -4380,7 +4380,14 @@ fi
> # capstone
>
> if test "$capstone" != no; then
> - if $pkg_config capstone; then
> + if test -f ${source_path}/capstone/Makefile ; then
> + # have submodule capstone - use it
> + capstone=yes
capstone="submodule"
or "internal"
> + capstone_internal=yes
drop
> + mkdir -p capstone
> + QEMU_CFLAGS="$QEMU_CFLAGS -I\$(SRC_PATH)/capstone/include"
> + LIBS="\$(BUILD_DIR)/capstone/libcapstone.a $LIBS"
> + elif $pkg_config capstone; then
> capstone=yes
capstone="system"
...
-if test "$capstone" = "yes" ; then
+if test "$capstone" != "no" ; then
echo "CONFIG_CAPSTONE=y" >> $config_host_mak
fi
...
> QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
> LIBS="$($pkg_config --libs capstone) $LIBS"
> @@ -6596,6 +6603,9 @@ done # for target in $targets
> if [ "$dtc_internal" = "yes" ]; then
> echo "config-host.h: subdir-dtc" >> $config_host_mak
> fi
> +if [ "$capstone_internal" = "yes" ]; then
if [ "$capstone" = "submodule" ]; then
> + echo "config-host.h: subdir-capstone" >> $config_host_mak
> +fi
>
> if test "$numa" = "yes"; then
> echo "CONFIG_NUMA=y" >> $config_host_mak
>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
both:
- debian + libcapstone-dev
- git submodule update --init (no libcapstone-dev)
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Regards,
Phil.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 9/9] disas: Add capstone as submodule
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 9/9] disas: Add capstone as submodule Richard Henderson
2017-10-02 13:54 ` Philippe Mathieu-Daudé
@ 2017-10-02 14:43 ` Alex Bennée
2017-10-02 18:27 ` Richard Henderson
1 sibling, 1 reply; 30+ messages in thread
From: Alex Bennée @ 2017-10-02 14:43 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> Do not require the submodule, but use it if present (in preference
> even to a system copy). This will allow us to easily use capstone
> in older systems for which a package is not available, and also
> easily track bug fixes from upstream.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> Makefile | 13 +++++++++++++
> .gitmodules | 3 +++
> capstone | 1 +
> configure | 12 +++++++++++-
> 4 files changed, 28 insertions(+), 1 deletion(-)
> create mode 160000 capstone
>
> diff --git a/Makefile b/Makefile
> index cee6e28659..7ff366f6ca 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -336,6 +336,19 @@ subdir-dtc:dtc/libfdt dtc/tests
> dtc/%:
> 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.
> +CAP_CFLAGS = $(subst -Werror,,$(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:
> + $(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 84c54cdc49..bcb80d701a 100644
> --- a/.gitmodules
> +++ b/.gitmodules
> @@ -34,3 +34,6 @@
> [submodule "roms/QemuMacDrivers"]
> path = roms/QemuMacDrivers
> url = git://git.qemu.org/QemuMacDrivers.git
> +[submodule "capstone"]
> + path = capstone
> + url = https://github.com/aquynh/capstone.git
> diff --git a/capstone b/capstone
> new file mode 160000
> index 0000000000..a279481dbf
> --- /dev/null
> +++ b/capstone
> @@ -0,0 +1 @@
> +Subproject commit a279481dbfd54bb1e2336d771e89978cc6d43176
> diff --git a/configure b/configure
> index 3777db91b6..a6cf0a5a15 100755
> --- a/configure
> +++ b/configure
> @@ -4380,7 +4380,14 @@ fi
> # capstone
>
> if test "$capstone" != no; then
> - if $pkg_config capstone; then
> + if test -f ${source_path}/capstone/Makefile ; then
> + # have submodule capstone - use it
> + capstone=yes
> + capstone_internal=yes
> + mkdir -p capstone
Having a mkdir -p here seems a little out of place. Shouldn't the build
directory creation be handled by the make recipe?
> + QEMU_CFLAGS="$QEMU_CFLAGS -I\$(SRC_PATH)/capstone/include"
> + LIBS="\$(BUILD_DIR)/capstone/libcapstone.a $LIBS"
> + elif $pkg_config capstone; then
> capstone=yes
> QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
> LIBS="$($pkg_config --libs capstone) $LIBS"
> @@ -6596,6 +6603,9 @@ done # for target in $targets
> if [ "$dtc_internal" = "yes" ]; then
> echo "config-host.h: subdir-dtc" >> $config_host_mak
> fi
> +if [ "$capstone_internal" = "yes" ]; then
> + echo "config-host.h: subdir-capstone" >> $config_host_mak
> +fi
>
> if test "$numa" = "yes"; then
> echo "CONFIG_NUMA=y" >> $config_host_mak
--
Alex Bennée
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v4 9/9] disas: Add capstone as submodule
2017-10-02 14:43 ` Alex Bennée
@ 2017-10-02 18:27 ` Richard Henderson
2017-10-13 7:50 ` Alex Bennée
0 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2017-10-02 18:27 UTC (permalink / raw)
To: Alex Bennée; +Cc: qemu-devel
On 10/02/2017 10:43 AM, Alex Bennée wrote:
>> + mkdir -p capstone
>
> Having a mkdir -p here seems a little out of place. Shouldn't the build
> directory creation be handled by the make recipe?
*shrug* We do this for the other directories at configure time.
r~
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v4 9/9] disas: Add capstone as submodule
2017-10-02 18:27 ` Richard Henderson
@ 2017-10-13 7:50 ` Alex Bennée
0 siblings, 0 replies; 30+ messages in thread
From: Alex Bennée @ 2017-10-13 7:50 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> On 10/02/2017 10:43 AM, Alex Bennée wrote:
>>> + mkdir -p capstone
>>
>> Having a mkdir -p here seems a little out of place. Shouldn't the build
>> directory creation be handled by the make recipe?
>
> *shrug* We do this for the other directories at configure time.
fair enough.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>
>
> r~
--
Alex Bennée
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v4 0/9] Support the Capstone disassembler
2017-09-28 16:54 [Qemu-devel] [PATCH v4 0/9] Support the Capstone disassembler Richard Henderson
` (8 preceding siblings ...)
2017-09-28 16:54 ` [Qemu-devel] [PATCH v4 9/9] disas: Add capstone as submodule Richard Henderson
@ 2017-10-12 12:34 ` Peter Maydell
2017-10-12 14:49 ` Richard Henderson
9 siblings, 1 reply; 30+ messages in thread
From: Peter Maydell @ 2017-10-12 12:34 UTC (permalink / raw)
To: Richard Henderson; +Cc: QEMU Developers
On 28 September 2017 at 17:54, Richard Henderson
<richard.henderson@linaro.org> wrote:
> Changes since v3:
> * Adjust how the submodule is detected and invoked.
> - This should fix the reported patchew failure,
> - Fixes e.g. -m32 "cross" compilation, or similar
> abi-changing option sets.
>
>
> r~
>
>
> Richard Henderson (9):
> target/i386: Convert to disas_set_info hook
> target/ppc: Convert to disas_set_info hook
> disas: Remove unused flags arguments
> disas: Support the Capstone disassembler library
> 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 the "add as submodule" patch might want to be
revised in the light of the changes to handling of
submodules that the current UI pull request is making,
but are the first 8 patches here more or less ready to
go in?
My other issue with patch 9 is that I think all our
submodules should be for git repos hosted on
git.qemu.org (ie mirrored from somewhere else). So
we should get capstone mirrored first and then use it.
thanks
-- PMM
^ permalink raw reply [flat|nested] 30+ messages in thread