diff mbox

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

Message ID 1383244738-5986-2-git-send-email-tommusta@gmail.com (mailing list archive)
State Superseded
Headers show

Commit Message

Tom Musta Oct. 31, 2013, 6:38 p.m. UTC 
From: Tom Musta <tommusta@gmail.com>

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).

V2: fixed bug in MSR[LE] check identified by Andreas Schwab and
Geert Uytterhoeven.

Signed-off-by: Tom Musta <tommusta@gmail.com>
---
 arch/powerpc/lib/sstep.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

Comments

Paul Mackerras Nov. 4, 2013, 2:28 a.m. UTC | #1 
On Thu, Oct 31, 2013 at 01:38:56PM -0500, Tom wrote:
> From: Tom Musta <tommusta@gmail.com>
> 
> This patch modifies the endian chicken switch in the single step

"Chicken switch" is IBM jargon, perhaps best avoided where possible in
commit messages.

> 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).

The patch adds #ifdefs inside code, which is generally frowned upon
in kernel code as it can make the code flow harder to see.  Perhaps
you could do something like

	if ((regs->msr & MSR_LE) != (MSR_KERNEL & MSR_LE))

as an alternative that wouldn't require an #ifdef.  Or, define a
symbol that is 0 in a BE kernel and MSR_LE in a LE kernel, and compare
to that.

Paul.
diff mbox

Patch

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index b1faa15..7bfaa9d 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.