[PATCH 1/1] powerpc/xmon: Dump memory in native endian format.

[PATCH 1/1] powerpc/xmon: Dump memory in native endian format.

[Douglas Miller]

Extend dump command to allow display of 2, 4, and 8 byte words in native
endian format. Also adds dump command for "1 byte words" for the sake
of symmetry. New commands are:

	d1	dump 8 bit values
	d2	dump 16 bit values
	d4	dump 32 bit values
	d8	dump 64 bit values

Signed-off-by: Douglas Miller <dougmill@linux.vnet.ibm.com>
---
 arch/powerpc/xmon/xmon.c |   65 +++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 9c0e17c..6249975 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -212,6 +212,10 @@ static void xmon_print_symbol(unsigned long address, const char *mid,
   "\
   C	checksum\n\
   d	dump bytes\n\
+  d1	dump 8 bit values\n\
+  d2	dump 16 bit values\n\
+  d4	dump 32 bit values\n\
+  d8	dump 64 bit values\n\
   di	dump instructions\n\
   df	dump float values\n\
   dd	dump double values\n\
@@ -2334,9 +2338,49 @@ static void dump_pacas(void)
 }
 #endif
 
+static void dump_by_size(unsigned long addr, long count, int size)
+{
+	unsigned char temp[16];
+	int i, j;
+	u64 val;
+
+	/*
+	 * 'count' was aligned 16. If that changes, the following
+	 * must also change to accommodate other values for 'count'.
+	 */
+	for (i = 0; i < count; i += 16, addr += 16) {
+		printf(REG, addr);
+
+		if (mread(addr, temp, 16) != 16) {
+			printf("Faulted reading %d bytes from 0x"REG"\n", 16, addr);
+			return;
+		}
+
+		for (j = 0; j < 16; j += size) {
+			putchar(' ');
+			switch (size) {
+			case 1: val = temp[j]; break;
+			case 2: val = *(u16 *)&temp[j]; break;
+			case 4: val = *(u32 *)&temp[j]; break;
+			case 8: val = *(u64 *)&temp[j]; break;
+			default: val = 0;
+			}
+
+			printf("%0*lx", size * 2, val);
+		}
+		printf("  |");
+		for (j = 0; j < 16; ++j) {
+			val = temp[j];
+			putchar(' ' <= val && val <= '~' ? val : '.');
+		}
+		printf("|\n");
+	}
+}
+
 static void
 dump(void)
 {
+	static char last[] = { "d?\n" };
 	int c;
 
 	c = inchar();
@@ -2350,8 +2394,9 @@ static void dump_pacas(void)
 	}
 #endif
 
-	if ((isxdigit(c) && c != 'f' && c != 'd') || c == '\n')
+	if (c == '\n')
 		termch = c;
+
 	scanhex((void *)&adrs);
 	if (termch != '\n')
 		termch = 0;
@@ -2383,9 +2428,23 @@ static void dump_pacas(void)
 			ndump = 64;
 		else if (ndump > MAX_DUMP)
 			ndump = MAX_DUMP;
-		prdump(adrs, ndump);
+
+		switch (c) {
+		case '8':
+		case '4':
+		case '2':
+		case '1':
+			ndump = ALIGN(ndump, 16);
+			dump_by_size(adrs, ndump, c - '0');
+			last[1] = c;
+			last_cmd = last;
+			break;
+		default:
+			prdump(adrs, ndump);
+			last_cmd = "d\n";
+		}
+
 		adrs += ndump;
-		last_cmd = "d\n";
 	}
 }
 
-- 
1.7.1

Re: [PATCH 1/1] powerpc/xmon: Dump memory in native endian format.

[Balbir Singh]

On Tue, Feb 07, 2017 at 07:40:44AM -0600, Douglas Miller wrote:
> Extend dump command to allow display of 2, 4, and 8 byte words in native
> endian format. Also adds dump command for "1 byte words" for the sake
> of symmetry. New commands are:
> 
> 	d1	dump 8 bit values
> 	d2	dump 16 bit values
> 	d4	dump 32 bit values
> 	d8	dump 64 bit values
> 
> Signed-off-by: Douglas Miller <dougmill@linux.vnet.ibm.com>
> ---
>  arch/powerpc/xmon/xmon.c |   65 +++++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 62 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 9c0e17c..6249975 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -212,6 +212,10 @@ static void xmon_print_symbol(unsigned long address, const char *mid,
>    "\
>    C	checksum\n\
>    d	dump bytes\n\
> +  d1	dump 8 bit values\n\
> +  d2	dump 16 bit values\n\
> +  d4	dump 32 bit values\n\
> +  d8	dump 64 bit values\n\
>    di	dump instructions\n\
>    df	dump float values\n\
>    dd	dump double values\n\
> @@ -2334,9 +2338,49 @@ static void dump_pacas(void)
>  }
>  #endif
>  
> +static void dump_by_size(unsigned long addr, long count, int size)
> +{
> +	unsigned char temp[16];
> +	int i, j;
> +	u64 val;
> +
> +	/*
> +	 * 'count' was aligned 16. If that changes, the following
> +	 * must also change to accommodate other values for 'count'.
> +	 */
> +	for (i = 0; i < count; i += 16, addr += 16) {
> +		printf(REG, addr);
> +
> +		if (mread(addr, temp, 16) != 16) {
> +			printf("Faulted reading %d bytes from 0x"REG"\n", 16, addr);

We have a method of printing a special character for faults.
Please see fault_chars[]

Otherwise looks good

Acked-by: Balbir Singh <bsingharora@gmail.com>

Re: [PATCH 1/1] powerpc/xmon: Dump memory in native endian format.

[Michael Ellerman]

Balbir Singh <bsingharora@gmail.com> writes:

> On Tue, Feb 07, 2017 at 07:40:44AM -0600, Douglas Miller wrote:
>> Extend dump command to allow display of 2, 4, and 8 byte words in native
>> endian format. Also adds dump command for "1 byte words" for the sake
>> of symmetry. New commands are:
>> 
>> 	d1	dump 8 bit values
>> 	d2	dump 16 bit values
>> 	d4	dump 32 bit values
>> 	d8	dump 64 bit values
>> 
>> Signed-off-by: Douglas Miller <dougmill@linux.vnet.ibm.com>

>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>> index 9c0e17c..6249975 100644
>> --- a/arch/powerpc/xmon/xmon.c
>> +++ b/arch/powerpc/xmon/xmon.c
>> @@ -2334,9 +2338,49 @@ static void dump_pacas(void)
>>  }
>>  #endif
>>  
>> +static void dump_by_size(unsigned long addr, long count, int size)
>> +{
>> +	unsigned char temp[16];
>> +	int i, j;
>> +	u64 val;
>> +
>> +	/*
>> +	 * 'count' was aligned 16. If that changes, the following
>> +	 * must also change to accommodate other values for 'count'.
>> +	 */
>> +	for (i = 0; i < count; i += 16, addr += 16) {
>> +		printf(REG, addr);
>> +
>> +		if (mread(addr, temp, 16) != 16) {
>> +			printf("Faulted reading %d bytes from 0x"REG"\n", 16, addr);
>
> We have a method of printing a special character for faults.
> Please see fault_chars[]

Yeah but it's not worth the complication IMO. In practice you usually
fault on everything you tried to print or nothing.

cheers

Re: [PATCH 1/1] powerpc/xmon: Dump memory in native endian format.

[Balbir Singh]

On Wed, Feb 08, 2017 at 08:18:07PM +1100, Michael Ellerman wrote:
> Balbir Singh <bsingharora@gmail.com> writes:
> 
> >> +			printf("Faulted reading %d bytes from 0x"REG"\n", 16, addr);
> >
> > We have a method of printing a special character for faults.
> > Please see fault_chars[]
> 
> Yeah but it's not worth the complication IMO. In practice you usually
> fault on everything you tried to print or nothing.

Fair enough, I was worried about consistency across commands and its
quite possible that on crossing a page boundary we see inconsistent
output. I'm OK with it, one can always retry the command with smaller
number of bytes.

Balbir Singh.

Re: [PATCH 1/1] powerpc/xmon: Dump memory in native endian format.

[Michael Ellerman]

Douglas Miller <dougmill@linux.vnet.ibm.com> writes:

> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 9c0e17c..6249975 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -2334,9 +2338,49 @@ static void dump_pacas(void)
>  }
>  #endif
>  
> +static void dump_by_size(unsigned long addr, long count, int size)
> +{
> +	unsigned char temp[16];
> +	int i, j;
> +	u64 val;
> +
> +	/*
> +	 * 'count' was aligned 16. If that changes, the following
> +	 * must also change to accommodate other values for 'count'.
> +	 */
> +	for (i = 0; i < count; i += 16, addr += 16) {
> +		printf(REG, addr);
> +
> +		if (mread(addr, temp, 16) != 16) {
> +			printf("Faulted reading %d bytes from 0x"REG"\n", 16, addr);
> +			return;
> +		}
> +
> +		for (j = 0; j < 16; j += size) {
> +			putchar(' ');
> +			switch (size) {
> +			case 1: val = temp[j]; break;
> +			case 2: val = *(u16 *)&temp[j]; break;
> +			case 4: val = *(u32 *)&temp[j]; break;
> +			case 8: val = *(u64 *)&temp[j]; break;
> +			default: val = 0;
> +			}
> +
> +			printf("%0*lx", size * 2, val);
> +		}
> +		printf("  |");
> +		for (j = 0; j < 16; ++j) {
> +			val = temp[j];
> +			putchar(' ' <= val && val <= '~' ? val : '.');
> +		}
> +		printf("|\n");

I know the ascii dump looks nice, but I think it's misleading. Which is
why I omitted it from my version.

eg.

0:mon> d $__kstrtab_init_task
c000000000c03ebe 696e69745f746173 6b006d6d755f6665  |init_task.mmu_fe|
c000000000c03ece 61747572655f6b65 7973006370755f66  |ature_keys.cpu_f|
c000000000c03ede 6561747572655f6b 657973006375725f  |eature_keys.cur_|
c000000000c03eee 6370755f73706563 00766972715f746f  |cpu_spec.virq_to|

0:mon> d8 $__kstrtab_init_task
c000000000c03ebe 7361745f74696e69 65665f756d6d006b  |init_task.mmu_fe|
c000000000c03ece 656b5f6572757461 665f757063007379  |ature_keys.cpu_f|
c000000000c03ede 6b5f657275746165 5f72756300737965  |eature_keys.cur_|
c000000000c03eee 636570735f757063 6f745f7172697600  |cpu_spec.virq_to|


That second dump says at c000000000c03ebe there is a byte with the value
0x73, which prints as 'i' - but that's false.


So I've dropped the ascii printing for now because I want to sneak this
in to v4.11.

If you want to send a follow-up patch to do the ascii byte reversed that
would be nice.

cheers

Re: [PATCH 1/1] powerpc/xmon: Dump memory in native endian format.

[Douglas Miller]

On 02/20/2017 11:01 PM, Michael Ellerman wrote:
> Douglas Miller <dougmill@linux.vnet.ibm.com> writes:
>
>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>> index 9c0e17c..6249975 100644
>> --- a/arch/powerpc/xmon/xmon.c
>> +++ b/arch/powerpc/xmon/xmon.c
>> @@ -2334,9 +2338,49 @@ static void dump_pacas(void)
>>   }
>>   #endif
>>   
>> +static void dump_by_size(unsigned long addr, long count, int size)
>> +{
>> +	unsigned char temp[16];
>> +	int i, j;
>> +	u64 val;
>> +
>> +	/*
>> +	 * 'count' was aligned 16. If that changes, the following
>> +	 * must also change to accommodate other values for 'count'.
>> +	 */
>> +	for (i = 0; i < count; i += 16, addr += 16) {
>> +		printf(REG, addr);
>> +
>> +		if (mread(addr, temp, 16) != 16) {
>> +			printf("Faulted reading %d bytes from 0x"REG"\n", 16, addr);
>> +			return;
>> +		}
>> +
>> +		for (j = 0; j < 16; j += size) {
>> +			putchar(' ');
>> +			switch (size) {
>> +			case 1: val = temp[j]; break;
>> +			case 2: val = *(u16 *)&temp[j]; break;
>> +			case 4: val = *(u32 *)&temp[j]; break;
>> +			case 8: val = *(u64 *)&temp[j]; break;
>> +			default: val = 0;
>> +			}
>> +
>> +			printf("%0*lx", size * 2, val);
>> +		}
>> +		printf("  |");
>> +		for (j = 0; j < 16; ++j) {
>> +			val = temp[j];
>> +			putchar(' ' <= val && val <= '~' ? val : '.');
>> +		}
>> +		printf("|\n");
> I know the ascii dump looks nice, but I think it's misleading. Which is
> why I omitted it from my version.
>
> eg.
>
> 0:mon> d $__kstrtab_init_task
> c000000000c03ebe 696e69745f746173 6b006d6d755f6665  |init_task.mmu_fe|
> c000000000c03ece 61747572655f6b65 7973006370755f66  |ature_keys.cpu_f|
> c000000000c03ede 6561747572655f6b 657973006375725f  |eature_keys.cur_|
> c000000000c03eee 6370755f73706563 00766972715f746f  |cpu_spec.virq_to|
>
> 0:mon> d8 $__kstrtab_init_task
> c000000000c03ebe 7361745f74696e69 65665f756d6d006b  |init_task.mmu_fe|
> c000000000c03ece 656b5f6572757461 665f757063007379  |ature_keys.cpu_f|
> c000000000c03ede 6b5f657275746165 5f72756300737965  |eature_keys.cur_|
> c000000000c03eee 636570735f757063 6f745f7172697600  |cpu_spec.virq_to|
>
>
> That second dump says at c000000000c03ebe there is a byte with the value
> 0x73, which prints as 'i' - but that's false.
>
>
> So I've dropped the ascii printing for now because I want to sneak this
> in to v4.11.
>
> If you want to send a follow-up patch to do the ascii byte reversed that
> would be nice.
>
> cheers
>
I would disagree, anything printed as bytes should be in only one order 
- the order it exists in memory. I maintain that the ascii dump is 
correctly printed. The purpose of an ascii dump like this is to show 
what is in memory. ASCII in memory has only one order.

Thanks,
Doug

Re: [1/1] powerpc/xmon: Dump memory in native endian format.

[Michael Ellerman]

On Tue, 2017-02-07 at 13:40:44 UTC, Douglas Miller wrote:
> Extend dump command to allow display of 2, 4, and 8 byte words in native
> endian format. Also adds dump command for "1 byte words" for the sake
> of symmetry. New commands are:
> 
> Signed-off-by: Douglas Miller <dougmill@linux.vnet.ibm.com>
> Acked-by: Balbir Singh <bsingharora@gmail.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/5e48dc0aa4d9daf93e9bff2a274473

cheers