powerpc: fix xmon disassembler for little-endian

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

Signed-off-by: Philippe Bergheaud <felix@linux.vnet.ibm.com>
---
 arch/powerpc/xmon/xmon.c |    4 ++++
 1 file changed, 4 insertions(+)

On 12/2/2013 3:10 AM, Philippe Bergheaud wrote:
> This patch fixes the disassembler of the powerpc kernel debugger xmon,
> for little-endian.
> 
> Signed-off-by: Philippe Bergheaud <felix@linux.vnet.ibm.com>
> ---
>  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

Tom Musta wrote:
> On 12/2/2013 3:10 AM, Philippe Bergheaud wrote:
> 
>>This patch fixes the disassembler of the powerpc kernel debugger xmon,
>>for little-endian.
>>
>>Signed-off-by: Philippe Bergheaud <felix@linux.vnet.ibm.com>
>>---
>> 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

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

> >>+#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.

Benjamin Herrenschmidt wrote:
> On Wed, 2013-12-04 at 14:45 +0100, Philippe Bergheaud wrote:
> 
> 
>>>>+#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