[v2,09/13] powerpc/xmon: Dump prefixed instructions
diff mbox series

Message ID 20200211053355.21574-10-jniethe5@gmail.com
State New
Headers show
Series
  • Initial Prefixed Instruction support
Related show

Checks

Context Check Description
snowpatch_ozlabs/needsstable success Patch has no Fixes tags
snowpatch_ozlabs/checkpatch warning total: 0 errors, 0 warnings, 2 checks, 76 lines checked
snowpatch_ozlabs/apply_patch success Successfully applied on branch powerpc/merge (a5bc6e124219546a81ce334dc9b16483d55e9abf)

Commit Message

Jordan Niethe Feb. 11, 2020, 5:33 a.m. UTC
Currently when xmon is dumping instructions it reads a word at a time
and then prints that instruction (either as a hex number or by
disassembling it). For prefixed instructions it would be nice to show
its prefix and suffix as together. Use read_instr() so that if a prefix
is encountered its suffix is loaded too. Then print these in the form:
    prefix:suffix
Xmon uses the disassembly routines from GNU binutils. These currently do
not support prefixed instructions so we will not disassemble the
prefixed instructions yet.

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: Rename sufx to suffix
---
 arch/powerpc/xmon/xmon.c | 50 +++++++++++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 11 deletions(-)

Comments

Christophe Leroy Feb. 11, 2020, 6:39 a.m. UTC | #1
Le 11/02/2020 à 06:33, Jordan Niethe a écrit :
> Currently when xmon is dumping instructions it reads a word at a time
> and then prints that instruction (either as a hex number or by
> disassembling it). For prefixed instructions it would be nice to show
> its prefix and suffix as together. Use read_instr() so that if a prefix
> is encountered its suffix is loaded too. Then print these in the form:
>      prefix:suffix
> Xmon uses the disassembly routines from GNU binutils. These currently do
> not support prefixed instructions so we will not disassemble the
> prefixed instructions yet.
> 
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v2: Rename sufx to suffix
> ---
>   arch/powerpc/xmon/xmon.c | 50 +++++++++++++++++++++++++++++++---------
>   1 file changed, 39 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 0b085642bbe7..513901ee18b0 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -2903,6 +2903,21 @@ prdump(unsigned long adrs, long ndump)
>   	}
>   }
>   
> +static bool instrs_are_equal(unsigned long insta, unsigned long suffixa,
> +			     unsigned long instb, unsigned long suffixb)
> +{
> +	if (insta != instb)
> +		return false;
> +
> +	if (!IS_PREFIX(insta) && !IS_PREFIX(instb))
> +		return true;
> +
> +	if (IS_PREFIX(insta) && IS_PREFIX(instb))
> +		return suffixa == suffixb;
> +
> +	return false;
> +}
> +
>   typedef int (*instruction_dump_func)(unsigned long inst, unsigned long addr);
>   
>   static int
> @@ -2911,12 +2926,11 @@ generic_inst_dump(unsigned long adr, long count, int praddr,
>   {
>   	int nr, dotted;
>   	unsigned long first_adr;
> -	unsigned int inst, last_inst = 0;
> -	unsigned char val[4];
> +	unsigned int inst, suffix, last_inst = 0, last_suffix = 0;
>   
>   	dotted = 0;
> -	for (first_adr = adr; count > 0; --count, adr += 4) {
> -		nr = mread(adr, val, 4);
> +	for (first_adr = adr; count > 0; --count, adr += nr) {
> +		nr = read_instr(adr, &inst, &suffix);
>   		if (nr == 0) {
>   			if (praddr) {
>   				const char *x = fault_chars[fault_type];
> @@ -2924,8 +2938,9 @@ generic_inst_dump(unsigned long adr, long count, int praddr,
>   			}
>   			break;
>   		}
> -		inst = GETWORD(val);
> -		if (adr > first_adr && inst == last_inst) {
> +		if (adr > first_adr && instrs_are_equal(inst, suffix,
> +							last_inst,
> +							last_suffix)) {
>   			if (!dotted) {
>   				printf(" ...\n");
>   				dotted = 1;
> @@ -2934,11 +2949,24 @@ generic_inst_dump(unsigned long adr, long count, int praddr,
>   		}
>   		dotted = 0;
>   		last_inst = inst;
> -		if (praddr)
> -			printf(REG"  %.8x", adr, inst);
> -		printf("\t");
> -		dump_func(inst, adr);
> -		printf("\n");
> +		last_suffix = suffix;
> +		if (IS_PREFIX(inst)) {
> +			if (praddr)
> +				printf(REG"  %.8x:%.8x", adr, inst, suffix);
> +			printf("\t");
> +			/*
> +			 * Just use this until binutils ppc disassembly
> +			 * prints prefixed instructions.
> +			 */
> +			printf("%.8x:%.8x", inst, suffix);
> +			printf("\n");
> +		} else {
> +			if (praddr)
> +				printf(REG"  %.8x", adr, inst);
> +			printf("\t");
> +			dump_func(inst, adr);
> +			printf("\n");
> +		}

What about:


		if (pr_addr) {
			printf(REG"  %.8x", adr, inst);
			if (IS_PREFIX(inst))
				printf(":%.8x", suffix);
		}
		printf("\t");
		if (IS_PREFIX(inst))
			printf("%.8x:%.8x", inst, suffix);
		else
			dump_func(inst, adr);
		printf("\n");

>   	}
>   	return adr - first_adr;
>   }
> 

Christophe
Jordan Niethe Feb. 12, 2020, 12:31 a.m. UTC | #2
On Tue, Feb 11, 2020 at 5:39 PM Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
>
>
>
> Le 11/02/2020 à 06:33, Jordan Niethe a écrit :
> > Currently when xmon is dumping instructions it reads a word at a time
> > and then prints that instruction (either as a hex number or by
> > disassembling it). For prefixed instructions it would be nice to show
> > its prefix and suffix as together. Use read_instr() so that if a prefix
> > is encountered its suffix is loaded too. Then print these in the form:
> >      prefix:suffix
> > Xmon uses the disassembly routines from GNU binutils. These currently do
> > not support prefixed instructions so we will not disassemble the
> > prefixed instructions yet.
> >
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v2: Rename sufx to suffix
> > ---
> >   arch/powerpc/xmon/xmon.c | 50 +++++++++++++++++++++++++++++++---------
> >   1 file changed, 39 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > index 0b085642bbe7..513901ee18b0 100644
> > --- a/arch/powerpc/xmon/xmon.c
> > +++ b/arch/powerpc/xmon/xmon.c
> > @@ -2903,6 +2903,21 @@ prdump(unsigned long adrs, long ndump)
> >       }
> >   }
> >
> > +static bool instrs_are_equal(unsigned long insta, unsigned long suffixa,
> > +                          unsigned long instb, unsigned long suffixb)
> > +{
> > +     if (insta != instb)
> > +             return false;
> > +
> > +     if (!IS_PREFIX(insta) && !IS_PREFIX(instb))
> > +             return true;
> > +
> > +     if (IS_PREFIX(insta) && IS_PREFIX(instb))
> > +             return suffixa == suffixb;
> > +
> > +     return false;
> > +}
> > +
> >   typedef int (*instruction_dump_func)(unsigned long inst, unsigned long addr);
> >
> >   static int
> > @@ -2911,12 +2926,11 @@ generic_inst_dump(unsigned long adr, long count, int praddr,
> >   {
> >       int nr, dotted;
> >       unsigned long first_adr;
> > -     unsigned int inst, last_inst = 0;
> > -     unsigned char val[4];
> > +     unsigned int inst, suffix, last_inst = 0, last_suffix = 0;
> >
> >       dotted = 0;
> > -     for (first_adr = adr; count > 0; --count, adr += 4) {
> > -             nr = mread(adr, val, 4);
> > +     for (first_adr = adr; count > 0; --count, adr += nr) {
> > +             nr = read_instr(adr, &inst, &suffix);
> >               if (nr == 0) {
> >                       if (praddr) {
> >                               const char *x = fault_chars[fault_type];
> > @@ -2924,8 +2938,9 @@ generic_inst_dump(unsigned long adr, long count, int praddr,
> >                       }
> >                       break;
> >               }
> > -             inst = GETWORD(val);
> > -             if (adr > first_adr && inst == last_inst) {
> > +             if (adr > first_adr && instrs_are_equal(inst, suffix,
> > +                                                     last_inst,
> > +                                                     last_suffix)) {
> >                       if (!dotted) {
> >                               printf(" ...\n");
> >                               dotted = 1;
> > @@ -2934,11 +2949,24 @@ generic_inst_dump(unsigned long adr, long count, int praddr,
> >               }
> >               dotted = 0;
> >               last_inst = inst;
> > -             if (praddr)
> > -                     printf(REG"  %.8x", adr, inst);
> > -             printf("\t");
> > -             dump_func(inst, adr);
> > -             printf("\n");
> > +             last_suffix = suffix;
> > +             if (IS_PREFIX(inst)) {
> > +                     if (praddr)
> > +                             printf(REG"  %.8x:%.8x", adr, inst, suffix);
> > +                     printf("\t");
> > +                     /*
> > +                      * Just use this until binutils ppc disassembly
> > +                      * prints prefixed instructions.
> > +                      */
> > +                     printf("%.8x:%.8x", inst, suffix);
> > +                     printf("\n");
> > +             } else {
> > +                     if (praddr)
> > +                             printf(REG"  %.8x", adr, inst);
> > +                     printf("\t");
> > +                     dump_func(inst, adr);
> > +                     printf("\n");
> > +             }
>
> What about:
>
>
>                 if (pr_addr) {
>                         printf(REG"  %.8x", adr, inst);
>                         if (IS_PREFIX(inst))
>                                 printf(":%.8x", suffix);
>                 }
>                 printf("\t");
>                 if (IS_PREFIX(inst))
>                         printf("%.8x:%.8x", inst, suffix);
>                 else
>                         dump_func(inst, adr);
>                 printf("\n");
>
Yeah that looks better.
> >       }
> >       return adr - first_adr;
> >   }
> >
>
> Christophe

Patch
diff mbox series

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 0b085642bbe7..513901ee18b0 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -2903,6 +2903,21 @@  prdump(unsigned long adrs, long ndump)
 	}
 }
 
+static bool instrs_are_equal(unsigned long insta, unsigned long suffixa,
+			     unsigned long instb, unsigned long suffixb)
+{
+	if (insta != instb)
+		return false;
+
+	if (!IS_PREFIX(insta) && !IS_PREFIX(instb))
+		return true;
+
+	if (IS_PREFIX(insta) && IS_PREFIX(instb))
+		return suffixa == suffixb;
+
+	return false;
+}
+
 typedef int (*instruction_dump_func)(unsigned long inst, unsigned long addr);
 
 static int
@@ -2911,12 +2926,11 @@  generic_inst_dump(unsigned long adr, long count, int praddr,
 {
 	int nr, dotted;
 	unsigned long first_adr;
-	unsigned int inst, last_inst = 0;
-	unsigned char val[4];
+	unsigned int inst, suffix, last_inst = 0, last_suffix = 0;
 
 	dotted = 0;
-	for (first_adr = adr; count > 0; --count, adr += 4) {
-		nr = mread(adr, val, 4);
+	for (first_adr = adr; count > 0; --count, adr += nr) {
+		nr = read_instr(adr, &inst, &suffix);
 		if (nr == 0) {
 			if (praddr) {
 				const char *x = fault_chars[fault_type];
@@ -2924,8 +2938,9 @@  generic_inst_dump(unsigned long adr, long count, int praddr,
 			}
 			break;
 		}
-		inst = GETWORD(val);
-		if (adr > first_adr && inst == last_inst) {
+		if (adr > first_adr && instrs_are_equal(inst, suffix,
+							last_inst,
+							last_suffix)) {
 			if (!dotted) {
 				printf(" ...\n");
 				dotted = 1;
@@ -2934,11 +2949,24 @@  generic_inst_dump(unsigned long adr, long count, int praddr,
 		}
 		dotted = 0;
 		last_inst = inst;
-		if (praddr)
-			printf(REG"  %.8x", adr, inst);
-		printf("\t");
-		dump_func(inst, adr);
-		printf("\n");
+		last_suffix = suffix;
+		if (IS_PREFIX(inst)) {
+			if (praddr)
+				printf(REG"  %.8x:%.8x", adr, inst, suffix);
+			printf("\t");
+			/*
+			 * Just use this until binutils ppc disassembly
+			 * prints prefixed instructions.
+			 */
+			printf("%.8x:%.8x", inst, suffix);
+			printf("\n");
+		} else {
+			if (praddr)
+				printf(REG"  %.8x", adr, inst);
+			printf("\t");
+			dump_func(inst, adr);
+			printf("\n");
+		}
 	}
 	return adr - first_adr;
 }