diff mbox

powerpc/xmon: Add support for dump in reverse

Message ID 51a96060-527a-f9ca-ced8-d261fc526d2a@gmail.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Balbir Singh Nov. 17, 2016, 11:23 p.m. UTC
This patch adds support for dumping bytes in the reverse order of
what they are read in, effectively doing an endian swap on double words.

This makes it easy to debug on a little endian system

The output of "d" on a little endian system is

0:mon> d c000000000e8bd10
c000000000e8bd10 70bde800000000c0 8428002400000000  |p........(.$....|

and with "dR"

0:mon> dR c000000000e8bd10
c000000000e8bd10 c000000000e8bd70 0000000024002884  |p........(.$....|

The patch adds "dR" only for CONFIG_PPC64. A new function digithex
is introduced which helps convert a digit to char in hex. There are
other command line tools to do byte endian swap, but I find this
useful for debugging.

Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
 arch/powerpc/xmon/xmon.c | 58 ++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 51 insertions(+), 7 deletions(-)

Comments

kernel test robot Nov. 18, 2016, 2:02 a.m. UTC | #1
Hi Balbir,

[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.9-rc5 next-20161117]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Balbir-Singh/powerpc-xmon-Add-support-for-dump-in-reverse/20161118-081358
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-ppc64e_defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All errors (new ones prefixed by >>):

   arch/powerpc/xmon/xmon.c: In function 'prdump':
>> arch/powerpc/xmon/xmon.c:2431:10: error: 'idx' may be used uninitialized in this function [-Werror=maybe-uninitialized]
         idx -= 2;
         ~~~~^~~~
   cc1: all warnings being treated as errors

vim +/idx +2431 arch/powerpc/xmon/xmon.c

  2425				}
  2426	
  2427				if (m < nr) {
  2428					if (reverse) {
  2429						buf[idx + 1] = digithex(temp[m] % 16);
  2430						buf[idx] = digithex(temp[m] / 16);
> 2431						idx -= 2;
  2432					} else
  2433						printf("%.2x", temp[m]);
  2434				} else

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Balbir Singh Nov. 21, 2016, 6:24 a.m. UTC | #2
On 18/11/16 13:02, kbuild test robot wrote:
> Hi Balbir,
> 
> [auto build test ERROR on powerpc/next]
> [also build test ERROR on v4.9-rc5 next-20161117]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Balbir-Singh/powerpc-xmon-Add-support-for-dump-in-reverse/20161118-081358
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
> config: powerpc-ppc64e_defconfig (attached as .config)
> compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=powerpc 
> 
> Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
> http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
> 
> All errors (new ones prefixed by >>):
> 
>    arch/powerpc/xmon/xmon.c: In function 'prdump':
>>> arch/powerpc/xmon/xmon.c:2431:10: error: 'idx' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>          idx -= 2;
>          ~~~~^~~~
>    cc1: all warnings being treated as errors
> 
> vim +/idx +2431 arch/powerpc/xmon/xmon.c
> 
>   2425				}
>   2426	
>   2427				if (m < nr) {
>   2428					if (reverse) {
>   2429						buf[idx + 1] = digithex(temp[m] % 16);
>   2430						buf[idx] = digithex(temp[m] / 16);
>> 2431						idx -= 2;
>   2432					} else
>   2433						printf("%.2x", temp[m]);
>   2434				} else
> 

This seems like a false positive, I don't see it with gcc 5.3.1 on my system. I also tried with gcc 6.2.0
I tried with the same config as attached with the email in the reported failure. I'll try to get the debian
6.1.1 compiler and see how that goes.

Balbir Singh.
Michael Ellerman Nov. 21, 2016, 10:21 a.m. UTC | #3
Balbir Singh <bsingharora@gmail.com> writes:

> On 18/11/16 13:02, kbuild test robot wrote:
>> Hi Balbir,
>> 
>> [auto build test ERROR on powerpc/next]
>> [also build test ERROR on v4.9-rc5 next-20161117]
>> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>> 
>> url:    https://github.com/0day-ci/linux/commits/Balbir-Singh/powerpc-xmon-Add-support-for-dump-in-reverse/20161118-081358
>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
>> config: powerpc-ppc64e_defconfig (attached as .config)
>> compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
>> reproduce:
>>         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>>         chmod +x ~/bin/make.cross
>>         # save the attached .config to linux build tree
>>         make.cross ARCH=powerpc 
>> 
>> Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
>> http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
>> 
>> All errors (new ones prefixed by >>):
>> 
>>    arch/powerpc/xmon/xmon.c: In function 'prdump':
>>>> arch/powerpc/xmon/xmon.c:2431:10: error: 'idx' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>          idx -= 2;
>>          ~~~~^~~~
>>    cc1: all warnings being treated as errors
>> 
>> vim +/idx +2431 arch/powerpc/xmon/xmon.c
>> 
>>   2425				}
>>   2426	
>>   2427				if (m < nr) {
>>   2428					if (reverse) {
>>   2429						buf[idx + 1] = digithex(temp[m] % 16);
>>   2430						buf[idx] = digithex(temp[m] / 16);
>>> 2431						idx -= 2;
>>   2432					} else
>>   2433						printf("%.2x", temp[m]);
>>   2434				} else
>> 
>
> This seems like a false positive, I don't see it with gcc 5.3.1 on my system. I also tried with gcc 6.2.0
> I tried with the same config as attached with the email in the reported failure. I'll try to get the debian
> 6.1.1 compiler and see how that goes.

Actually I want you to rework the patch anyway :)

Will talk tomorrow.

cheers
Michael Ellerman Nov. 23, 2016, 12:11 a.m. UTC | #4
Michael Ellerman <mpe@ellerman.id.au> writes:
> Actually I want you to rework the patch anyway :)
>
> Will talk tomorrow.

So as discussed, lets add d1, d2, d4, d8, which dump 1/2/4/8 bytes at a
time, in cpu endian, and which each value separated by space.

cheers
diff mbox

Patch

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 7605455..0f4750c 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -120,7 +120,7 @@  static void byterev(unsigned char *, int);
 static void memex(void);
 static int bsesc(void);
 static void dump(void);
-static void prdump(unsigned long, long);
+static void prdump(unsigned long, long, int);
 static int ppc_inst_dump(unsigned long, long, int);
 static void dump_log_buf(void);
 
@@ -144,6 +144,7 @@  int skipbl(void);
 int scanhex(unsigned long *valp);
 static void scannl(void);
 static int hexdigit(int);
+static int digithex(int);
 void getstring(char *, int);
 static void flush_input(void);
 static int inchar(void);
@@ -220,6 +221,7 @@  Commands:\n\
 #endif
 #ifdef CONFIG_PPC64
   "\
+  dR	dump bytes in reverse (double words) \n\
   dp[#]	dump paca for current cpu, or cpu #\n\
   dpa	dump paca for all possible cpus\n"
 #endif
@@ -2371,43 +2373,76 @@  dump(void)
 		xmon_rawdump(adrs, ndump);
 		adrs += ndump;
 		last_cmd = "dr\n";
+#ifdef CONFIG_PPC64
+	} else if (c == 'R') {
+		scanhex(&ndump);
+		if (ndump == 0)
+			ndump = 64;
+		else if (ndump > MAX_DUMP)
+			ndump = MAX_DUMP;
+		prdump(adrs, ndump, 1);
+		adrs += ndump;
+		last_cmd = "dR\n";
+#endif
 	} else {
 		scanhex(&ndump);
 		if (ndump == 0)
 			ndump = 64;
 		else if (ndump > MAX_DUMP)
 			ndump = MAX_DUMP;
-		prdump(adrs, ndump);
+		prdump(adrs, ndump, 0);
 		adrs += ndump;
 		last_cmd = "d\n";
 	}
 }
 
 static void
-prdump(unsigned long adrs, long ndump)
+prdump(unsigned long adrs, long ndump, int reverse)
 {
 	long n, m, c, r, nr;
 	unsigned char temp[16];
+	unsigned char buf[17];
+	int idx;
 
 	for (n = ndump; n > 0;) {
 		printf(REG, adrs);
 		putchar(' ');
 		r = n < 16? n: 16;
 		nr = mread(adrs, temp, r);
+
+		if (reverse) {
+			idx = 14;
+			buf[16] = '\0';
+		}
 		adrs += nr;
 		for (m = 0; m < r; ++m) {
-			if ((m & (sizeof(long) - 1)) == 0 && m > 0)
+			if ((m & (sizeof(long) - 1)) == 0 && m > 0) {
+				if (reverse) {
+					printf("%s", buf);
+					idx = 14;
+				}
 				putchar(' ');
-			if (m < nr)
-				printf("%.2x", temp[m]);
-			else
+			}
+
+			if (m < nr) {
+				if (reverse) {
+					buf[idx + 1] = digithex(temp[m] % 16);
+					buf[idx] = digithex(temp[m] / 16);
+					idx -= 2;
+				} else
+					printf("%.2x", temp[m]);
+			} else
 				printf("%s", fault_chars[fault_type]);
 		}
+
+		if (reverse)
+			printf("%s", buf);
 		for (; m < 16; ++m) {
 			if ((m & (sizeof(long) - 1)) == 0)
 				putchar(' ');
 			printf("  ");
 		}
+
 		printf("  |");
 		for (m = 0; m < r; ++m) {
 			if (m < nr) {
@@ -2893,6 +2928,15 @@  static int hexdigit(int c)
 	return EOF;
 }
 
+static int digithex(int c)
+{
+	if (c >= 0 && c <= 9)
+		return c + '0';
+	if (c >= 0xa && c <= 0xf)
+		return c + ('a' - 10);
+	return EOF;
+}
+
 void
 getstring(char *s, int size)
 {