From: Claudio Fontana <claudio.fontana@huawei.com>
To: Peter Crosthwaite <crosthwaitepeter@gmail.com>, qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, claudio.fontana@gmail.com,
Peter Crosthwaite <crosthwaite.peter@gmail.com>,
agraf@suse.de, edgari@xilinx.com, edgar.iglesias@gmail.com,
rth@twiddle.net
Subject: Re: [Qemu-devel] [PATCH 1/7] disas: Create factored out fn for monitor and target disas
Date: Tue, 5 May 2015 16:45:28 +0200 [thread overview]
Message-ID: <5548D788.1040204@huawei.com> (raw)
In-Reply-To: <2a23ffdd74eae74e97b286955f027612502b0217.1430798763.git.crosthwaite.peter@gmail.com>
On 05.05.2015 06:44, Peter Crosthwaite wrote:
> The monitor_ and target_ disas function do mostly the same thing.
> One dissambles target instructions on behalf of the log, the other
> for the monitor command "xp/i" and friends.
>
> There is a #if defined TARGET_FOO switch duplicated between both
> functions and arch-specific setup for disas is copied between the two.
> Factor out this duped code for arches that are consistent between
> monitor and target disas, that is I386, SPARC, M68K, MIPS, SH4, S390X,
> MOXIE and LM32.
>
> No functional change.
>
> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> ---
> disas.c | 148 ++++++++++++++++++++++++++--------------------------------------
> 1 file changed, 61 insertions(+), 87 deletions(-)
>
> diff --git a/disas.c b/disas.c
> index 44a019a..1c80567 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -187,6 +187,55 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info)
> return print_insn_objdump(pc, info, "OBJD-T");
> }
>
> +static inline void
> +target_disas_set_info(int (**print_insn)(bfd_vma pc, disassemble_info *info),
> + CPUDebug *s, CPUArchState *env, int flags)
> +{
> + s->env = env;
> + s->info.print_address_func = generic_print_target_address;
> +
> +#ifdef TARGET_WORDS_BIGENDIAN
> + s->info.endian = BFD_ENDIAN_BIG;
> +#else
> + s->info.endian = BFD_ENDIAN_LITTLE;
> +#endif
> +#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;
> + }
> + *print_insn = print_insn_i386;
> +#elif defined(TARGET_SPARC)
> + *print_insn = print_insn_sparc;
> +#ifdef TARGET_SPARC64
> + s->info.mach = bfd_mach_sparc_v9b;
> +#endif
> +#elif defined(TARGET_M68K)
> + *print_insn = print_insn_m68k;
> +#elif defined(TARGET_MIPS)
> +#ifdef TARGET_WORDS_BIGENDIAN
> + *print_insn = print_insn_big_mips;
> +#else
> + *print_insn = print_insn_little_mips;
> +#endif
> +#elif defined(TARGET_SH4)
> + s->info.mach = bfd_mach_sh4;
> + *print_insn = print_insn_sh;
> +#elif defined(TARGET_S390X)
> + s->info.mach = bfd_mach_s390_64;
> + *print_insn = print_insn_s390;
> +#elif defined(TARGET_MOXIE)
> + s->info.mach = bfd_arch_moxie;
> + *print_insn = print_insn_moxie;
> +#elif defined(TARGET_LM32)
> + s->info.mach = bfd_mach_lm32;
> + *print_insn = print_insn_lm32;
> +#endif
> +}
> +
> /* Disassemble this for me please... (debugging). 'flags' has the following
> values:
> i386 - 1 means 16 bit code, 2 means 64 bit code
> @@ -205,27 +254,13 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code,
>
> INIT_DISASSEMBLE_INFO(s.info, out, fprintf);
>
> - s.env = env;
> + target_disas_set_info(&print_insn, &s, env, flags);
> +
> s.info.read_memory_func = target_read_memory;
> s.info.buffer_vma = code;
> s.info.buffer_length = size;
> - s.info.print_address_func = generic_print_target_address;
>
> -#ifdef TARGET_WORDS_BIGENDIAN
> - s.info.endian = BFD_ENDIAN_BIG;
> -#else
> - s.info.endian = BFD_ENDIAN_LITTLE;
> -#endif
> -#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;
> - }
> - print_insn = print_insn_i386;
> -#elif defined(TARGET_ARM)
> +#if defined(TARGET_ARM)
> if (flags & 4) {
> /* We might not be compiled with the A64 disassembler
> * because it needs a C++ compiler; in that case we will
> @@ -246,11 +281,6 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code,
> s.info.endian = BFD_ENDIAN_BIG;
> #endif
> }
> -#elif defined(TARGET_SPARC)
> - print_insn = print_insn_sparc;
> -#ifdef TARGET_SPARC64
> - s.info.mach = bfd_mach_sparc_v9b;
> -#endif
> #elif defined(TARGET_PPC)
> if ((flags >> 16) & 1) {
> s.info.endian = BFD_ENDIAN_LITTLE;
> @@ -267,17 +297,6 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code,
> }
> s.info.disassembler_options = (char *)"any";
> print_insn = print_insn_ppc;
> -#elif defined(TARGET_M68K)
> - print_insn = print_insn_m68k;
> -#elif defined(TARGET_MIPS)
> -#ifdef TARGET_WORDS_BIGENDIAN
> - print_insn = print_insn_big_mips;
> -#else
> - print_insn = print_insn_little_mips;
> -#endif
> -#elif defined(TARGET_SH4)
> - s.info.mach = bfd_mach_sh4;
> - print_insn = print_insn_sh;
> #elif defined(TARGET_ALPHA)
> s.info.mach = bfd_mach_alpha_ev6;
> print_insn = print_insn_alpha;
> @@ -289,18 +308,9 @@ void target_disas(FILE *out, CPUArchState *env, target_ulong code,
> s.info.mach = bfd_mach_cris_v32;
> print_insn = print_insn_crisv32;
> }
> -#elif defined(TARGET_S390X)
> - s.info.mach = bfd_mach_s390_64;
> - print_insn = print_insn_s390;
> #elif defined(TARGET_MICROBLAZE)
> s.info.mach = bfd_arch_microblaze;
> print_insn = print_insn_microblaze;
> -#elif defined(TARGET_MOXIE)
> - s.info.mach = bfd_arch_moxie;
> - print_insn = print_insn_moxie;
> -#elif defined(TARGET_LM32)
> - s.info.mach = bfd_mach_lm32;
> - print_insn = print_insn_lm32;
> #endif
> if (print_insn == NULL) {
> print_insn = print_insn_od_target;
> @@ -452,10 +462,12 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
> {
> int count, i;
> CPUDebug s;
> - int (*print_insn)(bfd_vma pc, disassemble_info *info);
> + int (*print_insn)(bfd_vma pc, disassemble_info *info) = NULL;
>
> INIT_DISASSEMBLE_INFO(s.info, (FILE *)mon, monitor_fprintf);
>
> + target_disas_set_info(&print_insn, &s, env, flags);
> +
> s.env = env;
> monitor_disas_is_physical = is_physical;
> s.info.read_memory_func = monitor_read_memory;
> @@ -463,29 +475,10 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
>
> s.info.buffer_vma = pc;
>
> -#ifdef TARGET_WORDS_BIGENDIAN
> - s.info.endian = BFD_ENDIAN_BIG;
> -#else
> - s.info.endian = BFD_ENDIAN_LITTLE;
> -#endif
> -#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;
> - }
> - print_insn = print_insn_i386;
> -#elif defined(TARGET_ARM)
> +#if defined(TARGET_ARM)
> print_insn = print_insn_arm;
> #elif defined(TARGET_ALPHA)
> print_insn = print_insn_alpha;
> -#elif defined(TARGET_SPARC)
> - print_insn = print_insn_sparc;
> -#ifdef TARGET_SPARC64
> - s.info.mach = bfd_mach_sparc_v9b;
> -#endif
> #elif defined(TARGET_PPC)
> if (flags & 0xFFFF) {
> /* If we have a precise definition of the instruction set, use it. */
> @@ -501,31 +494,12 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
> s.info.endian = BFD_ENDIAN_LITTLE;
> }
> print_insn = print_insn_ppc;
> -#elif defined(TARGET_M68K)
> - print_insn = print_insn_m68k;
> -#elif defined(TARGET_MIPS)
> -#ifdef TARGET_WORDS_BIGENDIAN
> - print_insn = print_insn_big_mips;
> -#else
> - print_insn = print_insn_little_mips;
> -#endif
> -#elif defined(TARGET_SH4)
> - s.info.mach = bfd_mach_sh4;
> - print_insn = print_insn_sh;
> -#elif defined(TARGET_S390X)
> - s.info.mach = bfd_mach_s390_64;
> - print_insn = print_insn_s390;
> -#elif defined(TARGET_MOXIE)
> - s.info.mach = bfd_arch_moxie;
> - print_insn = print_insn_moxie;
> -#elif defined(TARGET_LM32)
> - s.info.mach = bfd_mach_lm32;
> - print_insn = print_insn_lm32;
> -#else
> - monitor_printf(mon, "0x" TARGET_FMT_lx
> - ": Asm output not supported on this arch\n", pc);
> - return;
> #endif
> + if (!print_insn) {
> + monitor_printf(mon, "0x" TARGET_FMT_lx
> + ": Asm output not supported on this arch\n", pc);
> + return;
> + }
>
> for(i = 0; i < nb_insn; i++) {
> monitor_printf(mon, "0x" TARGET_FMT_lx ": ", pc);
>
Note: tested only the ARMv8 path of this one.
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
next prev parent reply other threads:[~2015-05-05 14:45 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-05 4:44 [Qemu-devel] [PATCH 0/7] disas: Unify target_disas and monitor_disas Peter Crosthwaite
2015-05-05 4:44 ` [Qemu-devel] [PATCH 1/7] disas: Create factored out fn for monitor and target disas Peter Crosthwaite
2015-05-05 14:45 ` Claudio Fontana [this message]
2015-05-05 4:44 ` [Qemu-devel] [PATCH 2/7] disas: microblaze: Migrate setup to common code Peter Crosthwaite
2015-05-05 4:45 ` [Qemu-devel] [PATCH 3/7] disas: cris: Fix 0 buffer length case Peter Crosthwaite
2015-05-05 4:45 ` [Qemu-devel] [PATCH 4/7] disas: cris: Migrate setup to common code Peter Crosthwaite
2015-05-05 4:45 ` [Qemu-devel] [PATCH 5/7] disas: arm-a64: Make printfer and stream variable Peter Crosthwaite
2015-05-05 14:41 ` Claudio Fontana
2015-05-05 17:22 ` Richard Henderson
2015-05-05 4:45 ` [Qemu-devel] [PATCH 6/7] monitor: "i": Add ARM specifics Peter Crosthwaite
2015-05-05 14:40 ` Claudio Fontana
2015-05-05 17:19 ` Peter Maydell
2015-05-05 17:43 ` Richard Henderson
2015-05-06 6:57 ` Peter Crosthwaite
2015-05-06 13:57 ` Richard Henderson
2015-05-06 7:06 ` Peter Crosthwaite
2015-05-06 14:12 ` Richard Henderson
2015-05-06 14:17 ` Paolo Bonzini
2015-05-06 14:32 ` Stefano Stabellini
2015-05-06 15:44 ` Peter Maydell
2015-05-06 18:24 ` Richard Henderson
2015-05-06 18:39 ` Peter Maydell
2015-05-05 4:45 ` [Qemu-devel] [PATCH 7/7] disas: arm: Use target_disas impl for monitor Peter Crosthwaite
2015-05-05 14:38 ` Claudio Fontana
2015-05-05 16:48 ` Peter Crosthwaite
2015-05-05 17:18 ` [Qemu-devel] [PATCH 0/7] disas: Unify target_disas and monitor_disas Richard Henderson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5548D788.1040204@huawei.com \
--to=claudio.fontana@huawei.com \
--cc=agraf@suse.de \
--cc=claudio.fontana@gmail.com \
--cc=crosthwaite.peter@gmail.com \
--cc=crosthwaitepeter@gmail.com \
--cc=edgar.iglesias@gmail.com \
--cc=edgari@xilinx.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=rth@twiddle.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).