* xmon memory dump does not handle LE
@ 2017-02-02 14:42 Douglas Miller
2017-02-03 1:46 ` Michael Ellerman
0 siblings, 1 reply; 7+ messages in thread
From: Douglas Miller @ 2017-02-02 14:42 UTC (permalink / raw)
To: linuxppc-dev
I was hoping this would be any easy fix, but now it looks like it will
be more difficult.
The basic problem is that xmon memory commands like 'dd' do not properly
display the data on LE instances. This means that not only is it
difficult to read but one cannot copy-paste addresses from the output.
This severely encumbers debugging using xmon on LE systems.
It looks like prdump() is highly optimized but only works on BE. Have I
missed something, or will it take a significant re-write of this code to
fix the LE problem?
Thanks,
Doug
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: xmon memory dump does not handle LE
2017-02-02 14:42 xmon memory dump does not handle LE Douglas Miller
@ 2017-02-03 1:46 ` Michael Ellerman
2017-02-03 3:07 ` Douglas Miller
0 siblings, 1 reply; 7+ messages in thread
From: Michael Ellerman @ 2017-02-03 1:46 UTC (permalink / raw)
To: Douglas Miller, linuxppc-dev
Douglas Miller <dougmill@linux.vnet.ibm.com> writes:
> I was hoping this would be any easy fix, but now it looks like it will
> be more difficult.
>
> The basic problem is that xmon memory commands like 'dd' do not properly
> display the data on LE instances. This means that not only is it
> difficult to read but one cannot copy-paste addresses from the output.
> This severely encumbers debugging using xmon on LE systems.
What do you mean by "properly"?
I think what you're saying is that dumping bytes of memory on LE doesn't
give you nice u64 pointers, but that's not a bug, that's just how memory
is laid out on LE.
Also memory isn't always filled with u64 pointers, so just byte swapping
at that size is not correct. Sometimes you'll be looking at ints, in
which case you need to swap 4 bytes at a time.
What we need is dump commands that do a byte swap of a given size.
Balbir was working on this but I think he got diverted.
His first attempt was here: https://patchwork.ozlabs.org/patch/696348/
But as I said in my reply:
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.
Here's a quick hack to get it going. Let me know if that works for you,
it's not pretty, but we could probably merge it with a bit of cleanup.
cheers
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index a44b049b9cf6..3903af5fe276 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -2334,10 +2334,43 @@ 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 = ALIGN(count, 16);
+
+ 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("\n");
+ }
+}
+
static void
dump(void)
{
- int c;
+ int size, c;
c = inchar();
@@ -2350,8 +2383,9 @@ dump(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 +2417,21 @@ dump(void)
ndump = 64;
else if (ndump > MAX_DUMP)
ndump = MAX_DUMP;
- prdump(adrs, ndump);
+
+ size = 0;
+ switch (c) {
+ case '8': size += 4;
+ case '4': size += 2;
+ case '2': size += 1;
+ case '1': size += 1;
+ dump_by_size(adrs, ndump, size);
+ break;
+ default:
+ prdump(adrs, ndump);
+ last_cmd = "d\n";
+ }
+
adrs += ndump;
- last_cmd = "d\n";
}
}
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: xmon memory dump does not handle LE
2017-02-03 1:46 ` Michael Ellerman
@ 2017-02-03 3:07 ` Douglas Miller
2017-02-03 10:31 ` Michael Ellerman
0 siblings, 1 reply; 7+ messages in thread
From: Douglas Miller @ 2017-02-03 3:07 UTC (permalink / raw)
To: linuxppc-dev
I'm referring to the three commands listed in the help:
d dump bytes
df dump float values
dd dump double values
As it turns out, all three of these commands do exactly the same thing,
and it's certainly not what I'd expect based on experience with other
debuggers. Maybe the original intent was only to simply output bytes of
memory, but to me the help implies otherwise and certainly something
more is needed.
I'll take a look at Balbir's patch and see if I can help move it along.
Thanks,
Doug
On 02/02/2017 07:46 PM, Michael Ellerman wrote:
> Douglas Miller <dougmill@linux.vnet.ibm.com> writes:
>
>> I was hoping this would be any easy fix, but now it looks like it will
>> be more difficult.
>>
>> The basic problem is that xmon memory commands like 'dd' do not properly
>> display the data on LE instances. This means that not only is it
>> difficult to read but one cannot copy-paste addresses from the output.
>> This severely encumbers debugging using xmon on LE systems.
> What do you mean by "properly"?
>
> I think what you're saying is that dumping bytes of memory on LE doesn't
> give you nice u64 pointers, but that's not a bug, that's just how memory
> is laid out on LE.
>
> Also memory isn't always filled with u64 pointers, so just byte swapping
> at that size is not correct. Sometimes you'll be looking at ints, in
> which case you need to swap 4 bytes at a time.
>
> What we need is dump commands that do a byte swap of a given size.
> Balbir was working on this but I think he got diverted.
>
> His first attempt was here: https://patchwork.ozlabs.org/patch/696348/
>
> But as I said in my reply:
>
> 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.
>
>
> Here's a quick hack to get it going. Let me know if that works for you,
> it's not pretty, but we could probably merge it with a bit of cleanup.
>
> cheers
>
>
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index a44b049b9cf6..3903af5fe276 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -2334,10 +2334,43 @@ 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 = ALIGN(count, 16);
> +
> + 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("\n");
> + }
> +}
> +
> static void
> dump(void)
> {
> - int c;
> + int size, c;
>
> c = inchar();
>
> @@ -2350,8 +2383,9 @@ dump(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 +2417,21 @@ dump(void)
> ndump = 64;
> else if (ndump > MAX_DUMP)
> ndump = MAX_DUMP;
> - prdump(adrs, ndump);
> +
> + size = 0;
> + switch (c) {
> + case '8': size += 4;
> + case '4': size += 2;
> + case '2': size += 1;
> + case '1': size += 1;
> + dump_by_size(adrs, ndump, size);
> + break;
> + default:
> + prdump(adrs, ndump);
> + last_cmd = "d\n";
> + }
> +
> adrs += ndump;
> - last_cmd = "d\n";
> }
> }
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: xmon memory dump does not handle LE
2017-02-03 3:07 ` Douglas Miller
@ 2017-02-03 10:31 ` Michael Ellerman
2017-02-06 13:50 ` Douglas Miller
0 siblings, 1 reply; 7+ messages in thread
From: Michael Ellerman @ 2017-02-03 10:31 UTC (permalink / raw)
To: Douglas Miller, linuxppc-dev
Douglas Miller <dougmill@linux.vnet.ibm.com> writes:
> I'm referring to the three commands listed in the help:
>
> d dump bytes
>
> df dump float values
>
> dd dump double values
>
> As it turns out, all three of these commands do exactly the same thing,
> and it's certainly not what I'd expect based on experience with other
> debuggers. Maybe the original intent was only to simply output bytes of
> memory, but to me the help implies otherwise and certainly something
> more is needed.
OK. I don't think df/dd actually exist in the code, so I think the help
is just out of date, by about 20 years.
'd' definitely works as intended and does what the help says as far as
I'm concerned. The output is pretty similar to hexdump -C for example.
Also xmon isn't a debugger it's a crash handler :)
> I'll take a look at Balbir's patch and see if I can help move it along.
Actually take a look at mine instead.
cheers
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: xmon memory dump does not handle LE
2017-02-03 10:31 ` Michael Ellerman
@ 2017-02-06 13:50 ` Douglas Miller
2017-02-06 20:37 ` Douglas Miller
2017-02-07 1:00 ` Michael Ellerman
0 siblings, 2 replies; 7+ messages in thread
From: Douglas Miller @ 2017-02-06 13:50 UTC (permalink / raw)
To: linuxppc-dev
Hi Michael,
Yes, your patch seems a more complete solution. The idea of "d1", "d2",
"d4", and "d8" commands is more what I needed and makes more sense to
someone hitting xmon "cold". I'll work on getting your patch submitted.
Question on these sorts of patches (PPC only), do we submit initially to
upstream, or here on this (PPC) mailing list? It would be nice to get
this into the distros as soon as possible.
Thanks,
Doug
On 02/03/2017 04:31 AM, Michael Ellerman wrote:
> Douglas Miller <dougmill@linux.vnet.ibm.com> writes:
>
>> I'm referring to the three commands listed in the help:
>>
>> d dump bytes
>>
>> df dump float values
>>
>> dd dump double values
>>
>> As it turns out, all three of these commands do exactly the same thing,
>> and it's certainly not what I'd expect based on experience with other
>> debuggers. Maybe the original intent was only to simply output bytes of
>> memory, but to me the help implies otherwise and certainly something
>> more is needed.
> OK. I don't think df/dd actually exist in the code, so I think the help
> is just out of date, by about 20 years.
>
> 'd' definitely works as intended and does what the help says as far as
> I'm concerned. The output is pretty similar to hexdump -C for example.
>
> Also xmon isn't a debugger it's a crash handler :)
>
>> I'll take a look at Balbir's patch and see if I can help move it along.
> Actually take a look at mine instead.
>
> cheers
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: xmon memory dump does not handle LE
2017-02-06 13:50 ` Douglas Miller
@ 2017-02-06 20:37 ` Douglas Miller
2017-02-07 1:00 ` Michael Ellerman
1 sibling, 0 replies; 7+ messages in thread
From: Douglas Miller @ 2017-02-06 20:37 UTC (permalink / raw)
To: linuxppc-dev
On 02/06/2017 07:50 AM, Douglas Miller wrote:
> Hi Michael,
>
> Yes, your patch seems a more complete solution. The idea of "d1",
> "d2", "d4", and "d8" commands is more what I needed and makes more
> sense to someone hitting xmon "cold". I'll work on getting your patch
> submitted.
>
>
> Question on these sorts of patches (PPC only), do we submit initially
> to upstream, or here on this (PPC) mailing list? It would be nice to
> get this into the distros as soon as possible.
>
> Thanks,
>
> Doug
>
>
> On 02/03/2017 04:31 AM, Michael Ellerman wrote:
>> Douglas Miller <dougmill@linux.vnet.ibm.com> writes:
>>
>>> I'm referring to the three commands listed in the help:
>>>
>>> d dump bytes
>>>
>>> df dump float values
>>>
>>> dd dump double values
>>>
>>> As it turns out, all three of these commands do exactly the same thing,
>>> and it's certainly not what I'd expect based on experience with other
>>> debuggers. Maybe the original intent was only to simply output bytes of
>>> memory, but to me the help implies otherwise and certainly something
>>> more is needed.
>> OK. I don't think df/dd actually exist in the code, so I think the help
>> is just out of date, by about 20 years.
>>
>> 'd' definitely works as intended and does what the help says as far as
>> I'm concerned. The output is pretty similar to hexdump -C for example.
>>
>> Also xmon isn't a debugger it's a crash handler :)
>>
>>> I'll take a look at Balbir's patch and see if I can help move it along.
>> Actually take a look at mine instead.
>>
>> cheers
>>
>
Here's the patch I created and tested today:
From 4aef7bd08950b83af355650e9b61528fb703450b Mon Sep 17 00:00:00 2001
From: Douglas Miller <dougmill@linux.vnet.ibm.com>
Date: Mon, 6 Feb 2017 09:35:54 -0600
Subject: [PATCH] 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
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: xmon memory dump does not handle LE
2017-02-06 13:50 ` Douglas Miller
2017-02-06 20:37 ` Douglas Miller
@ 2017-02-07 1:00 ` Michael Ellerman
1 sibling, 0 replies; 7+ messages in thread
From: Michael Ellerman @ 2017-02-07 1:00 UTC (permalink / raw)
To: Douglas Miller, linuxppc-dev
Douglas Miller <dougmill@linux.vnet.ibm.com> writes:
> Hi Michael,
>
> Yes, your patch seems a more complete solution. The idea of "d1", "d2",
> "d4", and "d8" commands is more what I needed and makes more sense to
> someone hitting xmon "cold". I'll work on getting your patch submitted.
>
>
> Question on these sorts of patches (PPC only), do we submit initially to
> upstream, or here on this (PPC) mailing list?
Yeah send them here. Patches that are only touch powerpc should come
here, from where I merge them and then send them to Linus.
Anything that touches powerpc and some other code should at least be
CC'ed here.
If you're ever in doubt where to send a patch you can use:
$ ./scripts/get_maintainer.pl --nogit-fallback --no-rolestats -f arch/powerpc/xmon/xmon.c
Benjamin Herrenschmidt <benh@kernel.crashing.org>
Paul Mackerras <paulus@samba.org>
Michael Ellerman <mpe@ellerman.id.au>
linuxppc-dev@lists.ozlabs.org
linux-kernel@vger.kernel.org
> It would be nice to get this into the distros as soon as possible.
This isn't a bug fix so the distros won't pick it up automatically.
You'll have to ask them specifically to take a backport.
cheers
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-02-07 1:00 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-02 14:42 xmon memory dump does not handle LE Douglas Miller
2017-02-03 1:46 ` Michael Ellerman
2017-02-03 3:07 ` Douglas Miller
2017-02-03 10:31 ` Michael Ellerman
2017-02-06 13:50 ` Douglas Miller
2017-02-06 20:37 ` Douglas Miller
2017-02-07 1:00 ` Michael Ellerman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.