[PATCH] powerpc: fix xmon disassembler for little-endian

Subsystems: linux for powerpc (32-bit and 64-bit), the rest

STALE4670d

5 messages, 3 authors, 2013-12-05 · open the first message on its own page

[PATCH] powerpc: fix xmon disassembler for little-endian

From: Philippe Bergheaud <hidden>
Date: 2013-12-02 09:10:37

This patch fixes the disassembler of the powerpc kernel debugger xmon,
for little-endian.

Signed-off-by: Philippe Bergheaud <redacted>
---
 arch/powerpc/xmon/xmon.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index af9d346..6c27804 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c

@@ -171,7 +171,11 @@ extern void xmon_leave(void);
 #define REG		"%.8lx"
 #endif
 
+#ifdef __LITTLE_ENDIAN__
+#define GETWORD(v)	(((v)[3] << 24) + ((v)[2] << 16) + ((v)[1] << 8) + (v)[0])
+#else
 #define GETWORD(v)	(((v)[0] << 24) + ((v)[1] << 16) + ((v)[2] << 8) + (v)[3])
+#endif
 
 #define isxdigit(c)	(('0' <= (c) && (c) <= '9') \
 			 || ('a' <= (c) && (c) <= 'f') \

-- 
1.7.10.4

Re: [PATCH] powerpc: fix xmon disassembler for little-endian

From: Tom Musta <hidden>
Date: 2013-12-02 14:05:18

On 12/2/2013 3:10 AM, Philippe Bergheaud wrote:

quoted hunk

This patch fixes the disassembler of the powerpc kernel debugger xmon,
for little-endian.

Signed-off-by: Philippe Bergheaud <redacted>
---
 arch/powerpc/xmon/xmon.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index af9d346..6c27804 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c

@@ -171,7 +171,11 @@ extern void xmon_leave(void);
 #define REG		"%.8lx"
 #endif
 
+#ifdef __LITTLE_ENDIAN__
+#define GETWORD(v)	(((v)[3] << 24) + ((v)[2] << 16) + ((v)[1] << 8) + (v)[0])
+#else
 #define GETWORD(v)	(((v)[0] << 24) + ((v)[1] << 16) + ((v)[2] << 8) + (v)[3])
+#endif
 
 #define isxdigit(c)	(('0' <= (c) && (c) <= '9') \
 			 || ('a' <= (c) && (c) <= 'f') \

Philippe:  Wouldn't it be better to just do a 32-bit load and let the endianness be worked out
by the hardware?  i.e.

#define GETWORD(v) (*(u32 *)v)


Tom

Re: [PATCH] powerpc: fix xmon disassembler for little-endian

From: Philippe Bergheaud <hidden>
Date: 2013-12-04 13:44:50

Tom Musta wrote:

On 12/2/2013 3:10 AM, Philippe Bergheaud wrote:

quoted

This patch fixes the disassembler of the powerpc kernel debugger xmon,
for little-endian.

Signed-off-by: Philippe Bergheaud <redacted>
---
arch/powerpc/xmon/xmon.c |    4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index af9d346..6c27804 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c

@@ -171,7 +171,11 @@ extern void xmon_leave(void);

#define REG		"%.8lx"
#endif

+#ifdef __LITTLE_ENDIAN__
+#define GETWORD(v)	(((v)[3] << 24) + ((v)[2] << 16) + ((v)[1] << 8) + (v)[0])
+#else
#define GETWORD(v)	(((v)[0] << 24) + ((v)[1] << 16) + ((v)[2] << 8) + (v)[3])
+#endif

#define isxdigit(c)	(('0' <= (c) && (c) <= '9') \
			 || ('a' <= (c) && (c) <= 'f') \


Philippe:  Wouldn't it be better to just do a 32-bit load and let the endianness be worked out
by the hardware?  i.e.

#define GETWORD(v) (*(u32 *)v)

Yes, your alternative is better.
Wouldn't it narrow the scope of the macro to aligned words on POWER7?
I think that all references to GETWORD operate on aligned words anyway.

Philippe

Re: [PATCH] powerpc: fix xmon disassembler for little-endian

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2013-12-05 04:39:40

On Wed, 2013-12-04 at 14:45 +0100, Philippe Bergheaud wrote:

quoted

+#ifdef __LITTLE_ENDIAN__
+#define GETWORD(v)	(((v)[3] << 24) + ((v)[2] << 16) + ((v)[1] << 8) + (v)[0])
+#else
#define GETWORD(v)	(((v)[0] << 24) + ((v)[1] << 16) + ((v)[2] << 8) + (v)[3])
+#endif

#define isxdigit(c)	(('0' <= (c) && (c) <= '9') \
			 || ('a' <= (c) && (c) <= 'f') \


Philippe:  Wouldn't it be better to just do a 32-bit load and let the endianness be worked out
by the hardware?  i.e.

#define GETWORD(v) (*(u32 *)v)

Yes, your alternative is better.
Wouldn't it narrow the scope of the macro to aligned words on POWER7?
I think that all references to GETWORD operate on aligned words anyway.

Well, xmon has to be robust ... as long as you are *certain* that even
with crap entry state it won't try to access unaligned boundaries then
go for it but we aren't looking at performance here.

Cheers,
Ben.

Re: [PATCH] powerpc: fix xmon disassembler for little-endian

From: Philippe Bergheaud <hidden>
Date: 2013-12-05 08:49:45

Benjamin Herrenschmidt wrote:

On Wed, 2013-12-04 at 14:45 +0100, Philippe Bergheaud wrote:

quoted

+#ifdef __LITTLE_ENDIAN__
+#define GETWORD(v)	(((v)[3] << 24) + ((v)[2] << 16) + ((v)[1] << 8) + (v)[0])
+#else
#define GETWORD(v)	(((v)[0] << 24) + ((v)[1] << 16) + ((v)[2] << 8) + (v)[3])
+#endif

#define isxdigit(c)	(('0' <= (c) && (c) <= '9') \
		 || ('a' <= (c) && (c) <= 'f') \


Philippe:  Wouldn't it be better to just do a 32-bit load and let the endianness be worked out
by the hardware?  i.e.

#define GETWORD(v) (*(u32 *)v)

Yes, your alternative is better.
Wouldn't it narrow the scope of the macro to aligned words on POWER7?
I think that all references to GETWORD operate on aligned words anyway.


Well, xmon has to be robust ... as long as you are *certain* that even
with crap entry state it won't try to access unaligned boundaries then
go for it but we aren't looking at performance here.

Thank you Tom and Ben.  We are definitely not looking at performance here.
I prefer to stay on the safe side, and leave the original patch untouched.

Philippe

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help