* [PATCH 0/3] powerpc: Fix Little Endian Bugs in Single Step Code
@ 2013-10-18 19:38 Tom Musta
2013-10-18 19:40 ` [PATCH 1/3] powerpc: Enable emulate_step In Little Endian Mode Tom Musta
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Tom Musta @ 2013-10-18 19:38 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tmusta
This patch series addresses bugs in the PowerPC single-step emulation
code (arch/powerpc/lib/sstep.c) pertaining to Little Endian.
The existing code has a chicken switch for little endian. The first
patch softens the restriction so that only cross-endian modes are not
supported.
There is a general problem with unaligned little endian loads and stores.
This is addressed by the second patch.
Finally, there is a problem with unaligned single precision floating point
loads and stores which is addressed by the third patch.
Tom Musta (3):
powerpc: Enable emulate_step In Little Endian Mode
powerpc: Fix Unaligned Fixed Point Loads and Stores
powerpc: Fix Unaligned LE Floating Point Loads and Stores
arch/powerpc/lib/sstep.c | 109 +++++++++++++++++++++++++++++++++++++++------
1 files changed, 94 insertions(+), 15 deletions(-)
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/3] powerpc: Enable emulate_step In Little Endian Mode 2013-10-18 19:38 [PATCH 0/3] powerpc: Fix Little Endian Bugs in Single Step Code Tom Musta @ 2013-10-18 19:40 ` Tom Musta 2013-10-30 0:13 ` Benjamin Herrenschmidt 2013-10-30 17:43 ` Andreas Schwab 2013-10-18 19:42 ` [PATCH 2/3] powerpc: Fix Unaligned Loads and Stores Tom Musta 2013-10-18 19:44 ` [PATCH 3/3] powerpc: Fix Unaligned LE Floating Point " Tom Musta 2 siblings, 2 replies; 9+ messages in thread From: Tom Musta @ 2013-10-18 19:40 UTC (permalink / raw) To: linuxppc-dev; +Cc: tmusta This patch modifies the endian chicken switch in the single step emulation code (emulate_step()). The old (big endian) code bailed early if a load or store instruction was to be emulated in little endian mode. The new code modifies the check and only bails in a cross-endian situation (LE mode in a kernel compiled for BE and vice verse). Signed-off-by: Tom Musta <tmusta@gmail.com> --- arch/powerpc/lib/sstep.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c index b1faa15..5e0d0e9 100644 --- a/arch/powerpc/lib/sstep.c +++ b/arch/powerpc/lib/sstep.c @@ -1222,12 +1222,18 @@ int __kprobes emulate_step(struct pt_regs *regs, unsigned int instr) } /* - * Following cases are for loads and stores, so bail out - * if we're in little-endian mode. + * Following cases are for loads and stores and this + * implementation does not support cross-endian. So + * bail out if this is the case. */ +#ifdef __BIG_ENDIAN__ if (regs->msr & MSR_LE) return 0; - +#endif +#ifdef __LITTLE_ENDIAN__ + if (!regs->msr & MSR_LE) + return 0; +#endif /* * Save register RA in case it's an update form load or store * and the access faults. -- 1.7.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] powerpc: Enable emulate_step In Little Endian Mode 2013-10-18 19:40 ` [PATCH 1/3] powerpc: Enable emulate_step In Little Endian Mode Tom Musta @ 2013-10-30 0:13 ` Benjamin Herrenschmidt 2013-10-30 17:43 ` Andreas Schwab 1 sibling, 0 replies; 9+ messages in thread From: Benjamin Herrenschmidt @ 2013-10-30 0:13 UTC (permalink / raw) To: Tom Musta; +Cc: tmusta, linuxppc-dev On Fri, 2013-10-18 at 14:40 -0500, Tom Musta wrote: > This patch modifies the endian chicken switch in the single step > emulation code (emulate_step()). The old (big endian) code bailed > early if a load or store instruction was to be emulated in little > endian mode. > > The new code modifies the check and only bails in a cross-endian > situation (LE mode in a kernel compiled for BE and vice verse). I get a malformed patch error, looks like it got wrapped. Cheers, Ben. > Signed-off-by: Tom Musta <tmusta@gmail.com> > --- > arch/powerpc/lib/sstep.c | 12 +++++++++--- > 1 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c > index b1faa15..5e0d0e9 100644 > --- a/arch/powerpc/lib/sstep.c > +++ b/arch/powerpc/lib/sstep.c > @@ -1222,12 +1222,18 @@ int __kprobes emulate_step(struct pt_regs *regs, > unsigned int instr) > } > > /* > - * Following cases are for loads and stores, so bail out > - * if we're in little-endian mode. > + * Following cases are for loads and stores and this > + * implementation does not support cross-endian. So > + * bail out if this is the case. > */ > +#ifdef __BIG_ENDIAN__ > if (regs->msr & MSR_LE) > return 0; > - > +#endif > +#ifdef __LITTLE_ENDIAN__ > + if (!regs->msr & MSR_LE) > + return 0; > +#endif > /* > * Save register RA in case it's an update form load or store > * and the access faults. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] powerpc: Enable emulate_step In Little Endian Mode 2013-10-18 19:40 ` [PATCH 1/3] powerpc: Enable emulate_step In Little Endian Mode Tom Musta 2013-10-30 0:13 ` Benjamin Herrenschmidt @ 2013-10-30 17:43 ` Andreas Schwab 2013-10-30 19:35 ` Tom Musta 1 sibling, 1 reply; 9+ messages in thread From: Andreas Schwab @ 2013-10-30 17:43 UTC (permalink / raw) To: Tom Musta; +Cc: tmusta, linuxppc-dev Tom Musta <tommusta@gmail.com> writes: > +#ifdef __LITTLE_ENDIAN__ > + if (!regs->msr & MSR_LE) That won't work. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] powerpc: Enable emulate_step In Little Endian Mode 2013-10-30 17:43 ` Andreas Schwab @ 2013-10-30 19:35 ` Tom Musta 2013-10-30 19:43 ` Geert Uytterhoeven 0 siblings, 1 reply; 9+ messages in thread From: Tom Musta @ 2013-10-30 19:35 UTC (permalink / raw) To: Andreas Schwab; +Cc: tmusta, linuxppc-dev On 10/30/2013 12:43 PM, Andreas Schwab wrote: > Tom Musta <tommusta@gmail.com> writes: > >> +#ifdef __LITTLE_ENDIAN__ >> + if (!regs->msr & MSR_LE) > > That won't work. > > Andreas. > Please elaborate. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] powerpc: Enable emulate_step In Little Endian Mode 2013-10-30 19:35 ` Tom Musta @ 2013-10-30 19:43 ` Geert Uytterhoeven 2013-10-30 21:45 ` Tom Musta 0 siblings, 1 reply; 9+ messages in thread From: Geert Uytterhoeven @ 2013-10-30 19:43 UTC (permalink / raw) To: Tom Musta; +Cc: linuxppc-dev, tmusta, Andreas Schwab On Wed, Oct 30, 2013 at 8:35 PM, Tom Musta <tommusta@gmail.com> wrote: > On 10/30/2013 12:43 PM, Andreas Schwab wrote: >> >> Tom Musta <tommusta@gmail.com> writes: >> >>> +#ifdef __LITTLE_ENDIAN__ >>> + if (!regs->msr & MSR_LE) >> >> >> That won't work. >> >> Andreas. >> > > Please elaborate. You want to test for "!(regs & MSR_LE)". Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] powerpc: Enable emulate_step In Little Endian Mode 2013-10-30 19:43 ` Geert Uytterhoeven @ 2013-10-30 21:45 ` Tom Musta 0 siblings, 0 replies; 9+ messages in thread From: Tom Musta @ 2013-10-30 21:45 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: linuxppc-dev, tmusta, Andreas Schwab On 10/30/2013 2:43 PM, Geert Uytterhoeven wrote: > On Wed, Oct 30, 2013 at 8:35 PM, Tom Musta <tommusta@gmail.com> wrote: >> On 10/30/2013 12:43 PM, Andreas Schwab wrote: >>> >>> Tom Musta <tommusta@gmail.com> writes: >>> >>>> +#ifdef __LITTLE_ENDIAN__ >>>> + if (!regs->msr & MSR_LE) >>> >>> >>> That won't work. >>> >>> Andreas. >>> >> >> Please elaborate. > > You want to test for "!(regs & MSR_LE)". > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds > Thanks Adnreas and Geert. I will fix and resubmit. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] powerpc: Fix Unaligned Loads and Stores 2013-10-18 19:38 [PATCH 0/3] powerpc: Fix Little Endian Bugs in Single Step Code Tom Musta 2013-10-18 19:40 ` [PATCH 1/3] powerpc: Enable emulate_step In Little Endian Mode Tom Musta @ 2013-10-18 19:42 ` Tom Musta 2013-10-18 19:44 ` [PATCH 3/3] powerpc: Fix Unaligned LE Floating Point " Tom Musta 2 siblings, 0 replies; 9+ messages in thread From: Tom Musta @ 2013-10-18 19:42 UTC (permalink / raw) To: linuxppc-dev; +Cc: tmusta This patch modifies the unaligned access routines of the sstep.c module so that it properly reverses the bytes of storage operands in the little endian kernel kernel. This is implemented by breaking an unaligned little endian access into a combination of single byte accesses plus an overal byte reversal operation. Signed-off-by: Tom Musta <tmusta@gmail.com> --- arch/powerpc/lib/sstep.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 45 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c index 5e0d0e9..570f2af 100644 --- a/arch/powerpc/lib/sstep.c +++ b/arch/powerpc/lib/sstep.c @@ -212,11 +212,19 @@ static int __kprobes read_mem_unaligned(unsigned long *dest, unsigned long ea, { int err; unsigned long x, b, c; +#ifdef __LITTLE_ENDIAN__ + int len = nb; /* save a copy of the length for byte reversal */ +#endif /* unaligned, do this in pieces */ x = 0; for (; nb > 0; nb -= c) { +#ifdef __LITTLE_ENDIAN__ + c = 1; +#endif +#ifdef __BIG_ENDIAN__ c = max_align(ea); +#endif if (c > nb) c = max_align(nb); err = read_mem_aligned(&b, ea, c); @@ -225,7 +233,24 @@ static int __kprobes read_mem_unaligned(unsigned long *dest, unsigned long ea, x = (x << (8 * c)) + b; ea += c; } +#ifdef __LITTLE_ENDIAN__ + switch (len) { + case 2: + *dest = byterev_2(x); + break; + case 4: + *dest = byterev_4(x); + break; +#ifdef __powerpc64__ + case 8: + *dest = byterev_8(x); + break; +#endif + } +#endif +#ifdef __BIG_ENDIAN__ *dest = x; +#endif return 0; } @@ -273,9 +298,29 @@ static int __kprobes write_mem_unaligned(unsigned long val, unsigned long ea, int err; unsigned long c; +#ifdef __LITTLE_ENDIAN__ + switch (nb) { + case 2: + val = byterev_2(val); + break; + case 4: + val = byterev_4(val); + break; +#ifdef __powerpc64__ + case 8: + val = byterev_8(val); + break; +#endif + } +#endif /* unaligned or little-endian, do this in pieces */ for (; nb > 0; nb -= c) { +#ifdef __LITTLE_ENDIAN__ + c = 1; +#endif +#ifdef __BIG_ENDIAN__ c = max_align(ea); +#endif if (c > nb) c = max_align(nb); err = write_mem_aligned(val >> (nb - c) * 8, ea, c); -- 1.7.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] powerpc: Fix Unaligned LE Floating Point Loads and Stores 2013-10-18 19:38 [PATCH 0/3] powerpc: Fix Little Endian Bugs in Single Step Code Tom Musta 2013-10-18 19:40 ` [PATCH 1/3] powerpc: Enable emulate_step In Little Endian Mode Tom Musta 2013-10-18 19:42 ` [PATCH 2/3] powerpc: Fix Unaligned Loads and Stores Tom Musta @ 2013-10-18 19:44 ` Tom Musta 2 siblings, 0 replies; 9+ messages in thread From: Tom Musta @ 2013-10-18 19:44 UTC (permalink / raw) To: linuxppc-dev; +Cc: tmusta This patch addresses unaligned single precision floating point loads and stores in the single-step code. The old implementation improperly treated an 8 byte structure as an array of two 4 byte words, which is a classic little endian bug. Signed-off-by: Tom Musta <tmusta@gmail.com> --- arch/powerpc/lib/sstep.c | 52 +++++++++++++++++++++++++++++++++++---------- 1 files changed, 40 insertions(+), 12 deletions(-) diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c index 570f2af..f6f17aa 100644 --- a/arch/powerpc/lib/sstep.c +++ b/arch/powerpc/lib/sstep.c @@ -355,22 +355,36 @@ static int __kprobes do_fp_load(int rn, int (*func)(int, unsigned long), struct pt_regs *regs) { int err; - unsigned long val[sizeof(double) / sizeof(long)]; + union { + double dbl; + unsigned long ul[2]; + struct { +#ifdef __BIG_ENDIAN__ + unsigned _pad_; + unsigned word; +#endif +#ifdef __LITTLE_ENDIAN__ + unsigned word; + unsigned _pad_; +#endif + } single; + } data; unsigned long ptr; if (!address_ok(regs, ea, nb)) return -EFAULT; if ((ea & 3) == 0) return (*func)(rn, ea); - ptr = (unsigned long) &val[0]; + ptr = (unsigned long) &data.ul; if (sizeof(unsigned long) == 8 || nb == 4) { - err = read_mem_unaligned(&val[0], ea, nb, regs); - ptr += sizeof(unsigned long) - nb; + err = read_mem_unaligned(&data.ul[0], ea, nb, regs); + if (nb == 4) + ptr = (unsigned long)&(data.single.word); } else { /* reading a double on 32-bit */ - err = read_mem_unaligned(&val[0], ea, 4, regs); + err = read_mem_unaligned(&data.ul[0], ea, 4, regs); if (!err) - err = read_mem_unaligned(&val[1], ea + 4, 4, regs); + err = read_mem_unaligned(&data.ul[1], ea + 4, 4, regs); } if (err) return err; @@ -382,28 +396,42 @@ static int __kprobes do_fp_store(int rn, int (*func)(int, unsigned long), struct pt_regs *regs) { int err; - unsigned long val[sizeof(double) / sizeof(long)]; + union { + double dbl; + unsigned long ul[2]; + struct { +#ifdef __BIG_ENDIAN__ + unsigned _pad_; + unsigned word; +#endif +#ifdef __LITTLE_ENDIAN__ + unsigned word; + unsigned _pad_; +#endif + } single; + } data; unsigned long ptr; if (!address_ok(regs, ea, nb)) return -EFAULT; if ((ea & 3) == 0) return (*func)(rn, ea); - ptr = (unsigned long) &val[0]; + ptr = (unsigned long) &data.ul[0]; if (sizeof(unsigned long) == 8 || nb == 4) { - ptr += sizeof(unsigned long) - nb; + if (nb == 4) + ptr = (unsigned long)&(data.single.word); err = (*func)(rn, ptr); if (err) return err; - err = write_mem_unaligned(val[0], ea, nb, regs); + err = write_mem_unaligned(data.ul[0], ea, nb, regs); } else { /* writing a double on 32-bit */ err = (*func)(rn, ptr); if (err) return err; - err = write_mem_unaligned(val[0], ea, 4, regs); + err = write_mem_unaligned(data.ul[0], ea, 4, regs); if (!err) - err = write_mem_unaligned(val[1], ea + 4, 4, regs); + err = write_mem_unaligned(data.ul[1], ea + 4, 4, regs); } return err; } -- 1.7.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-10-30 21:46 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-10-18 19:38 [PATCH 0/3] powerpc: Fix Little Endian Bugs in Single Step Code Tom Musta 2013-10-18 19:40 ` [PATCH 1/3] powerpc: Enable emulate_step In Little Endian Mode Tom Musta 2013-10-30 0:13 ` Benjamin Herrenschmidt 2013-10-30 17:43 ` Andreas Schwab 2013-10-30 19:35 ` Tom Musta 2013-10-30 19:43 ` Geert Uytterhoeven 2013-10-30 21:45 ` Tom Musta 2013-10-18 19:42 ` [PATCH 2/3] powerpc: Fix Unaligned Loads and Stores Tom Musta 2013-10-18 19:44 ` [PATCH 3/3] powerpc: Fix Unaligned LE Floating Point " Tom Musta
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).