Message ID | 1385975412-25869-1-git-send-email-felix@linux.vnet.ibm.com (mailing list archive) |
---|---|
State | Accepted, archived |
Commit | 72eceef67abbe596a4e93ee79e08d9e6c35430ae |
Delegated to: | Benjamin Herrenschmidt |
Headers | show |
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
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') \
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(+)