Patchwork [1/3] powerpc: Enable emulate_step In Little Endian Mode

login
register
mail settings
Submitter Tom Musta
Date Oct. 18, 2013, 7:40 p.m.
Message ID <1382125235.2206.24.camel@tmusta-sc.rchland.ibm.com>
Download mbox | patch
Permalink /patch/284708/
State Changes Requested
Headers show

Comments

Tom Musta - Oct. 18, 2013, 7:40 p.m.
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(-)

 	 * and the access faults.
Benjamin Herrenschmidt - Oct. 30, 2013, 12:13 a.m.
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.
Andreas Schwab - Oct. 30, 2013, 5:43 p.m.
Tom Musta <tommusta@gmail.com> writes:

> +#ifdef __LITTLE_ENDIAN__
> +	if (!regs->msr & MSR_LE)

That won't work.

Andreas.
Tom Musta - Oct. 30, 2013, 7:35 p.m.
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.
Geert Uytterhoeven - Oct. 30, 2013, 7:43 p.m.
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
Tom Musta - Oct. 30, 2013, 9:45 p.m.
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.

Patch

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