Re: [PATCH v2 09/13] powerpc/xmon: Dump prefixed instructions

From: Christophe Leroy <christophe.leroy@c-s.fr>
To: Jordan Niethe <jniethe5@gmail.com>, linuxppc-dev@lists.ozlabs.org
Cc: bala24@linux.ibm.com, alistair@popple.id.au, mpe@ellerman.id.a,
	dja@axtens.net
Subject: Re: [PATCH v2 09/13] powerpc/xmon: Dump prefixed instructions
Date: Tue, 11 Feb 2020 07:39:09 +0100	[thread overview]
Message-ID: <52a33d2b-0dd7-1e29-564d-3ec982b36ef9@c-s.fr> (raw)
In-Reply-To: <20200211053355.21574-10-jniethe5@gmail.com>

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