| Message ID | 1946d8ed86a92a9a6827ec260d0d9c99e0d88184.1549630193.git.christophe.leroy@c-s.fr (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc/32: Implement fast syscall entry | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
| snowpatch_ozlabs/checkpatch | fail | total: 1 errors, 1 warnings, 0 checks, 42 lines checked |
On Fri, 2019-02-08 at 12:52 +0000, Christophe Leroy wrote: > /* > + * MSR_KERNEL is > 0x8000 on 4xx/Book-E since it include MSR_CE. > + */ > +.macro __LOAD_MSR_KERNEL r, x > +.if \x >= 0x8000 > + lis \r, (\x)@h > + ori \r, \r, (\x)@l > +.else > + li \r, (\x) > +.endif > +.endm > +#define LOAD_MSR_KERNEL(r, x) __LOAD_MSR_KERNEL r, x > + You changed the limit from >= 0x10000 to >= 0x8000 without a corresponding explanation as to why... Ben.
Le 11/02/2019 à 01:21, Benjamin Herrenschmidt a écrit : > On Fri, 2019-02-08 at 12:52 +0000, Christophe Leroy wrote: >> /* >> + * MSR_KERNEL is > 0x8000 on 4xx/Book-E since it include MSR_CE. >> + */ >> +.macro __LOAD_MSR_KERNEL r, x >> +.if \x >= 0x8000 >> + lis \r, (\x)@h >> + ori \r, \r, (\x)@l >> +.else >> + li \r, (\x) >> +.endif >> +.endm >> +#define LOAD_MSR_KERNEL(r, x) __LOAD_MSR_KERNEL r, x >> + > > You changed the limit from >= 0x10000 to >= 0x8000 without a > corresponding explanation as to why... Yes, the existing LOAD_MSR_KERNEL() was buggy because 'li' takes a signed u16, ie between -0x8000 and 0x7999. By chance it was working because until now nobody was trying to set MSR_KERNEL | MSR_EE. Christophe
On Mon, 2019-02-11 at 07:26 +0100, Christophe Leroy wrote: > > Le 11/02/2019 à 01:21, Benjamin Herrenschmidt a écrit : > > On Fri, 2019-02-08 at 12:52 +0000, Christophe Leroy wrote: > > > /* > > > + * MSR_KERNEL is > 0x8000 on 4xx/Book-E since it include MSR_CE. > > > + */ > > > +.macro __LOAD_MSR_KERNEL r, x > > > +.if \x >= 0x8000 > > > + lis \r, (\x)@h > > > + ori \r, \r, (\x)@l > > > +.else > > > + li \r, (\x) > > > +.endif > > > +.endm > > > +#define LOAD_MSR_KERNEL(r, x) __LOAD_MSR_KERNEL r, x > > > + > > > > You changed the limit from >= 0x10000 to >= 0x8000 without a > > corresponding explanation as to why... > > Yes, the existing LOAD_MSR_KERNEL() was buggy because 'li' takes a > signed u16, ie between -0x8000 and 0x7999. Ah yes, I was only looking at the "large" case which is fine... > By chance it was working because until now nobody was trying to set > MSR_KERNEL | MSR_EE. > > Christophe
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S index a5e2d5585dcb..b489aebdc5c5 100644 --- a/arch/powerpc/kernel/entry_32.S +++ b/arch/powerpc/kernel/entry_32.S @@ -37,14 +37,7 @@ #include <asm/feature-fixups.h> #include <asm/barrier.h> -/* - * MSR_KERNEL is > 0x10000 on 4xx/Book-E since it include MSR_CE. - */ -#if MSR_KERNEL >= 0x10000 -#define LOAD_MSR_KERNEL(r, x) lis r,(x)@h; ori r,r,(x)@l -#else -#define LOAD_MSR_KERNEL(r, x) li r,(x) -#endif +#include "head_32.h" /* * Align to 4k in order to ensure that all functions modyfing srr0/srr1 diff --git a/arch/powerpc/kernel/head_32.h b/arch/powerpc/kernel/head_32.h index 7456e2a45acc..cf3d00844597 100644 --- a/arch/powerpc/kernel/head_32.h +++ b/arch/powerpc/kernel/head_32.h @@ -5,6 +5,19 @@ #include <asm/ptrace.h> /* for STACK_FRAME_REGS_MARKER */ /* + * MSR_KERNEL is > 0x8000 on 4xx/Book-E since it include MSR_CE. + */ +.macro __LOAD_MSR_KERNEL r, x +.if \x >= 0x8000 + lis \r, (\x)@h + ori \r, \r, (\x)@l +.else + li \r, (\x) +.endif +.endm +#define LOAD_MSR_KERNEL(r, x) __LOAD_MSR_KERNEL r, x + +/* * Exception entry code. This code runs with address translation * turned off, i.e. using physical addresses. * We assume sprg3 has the physical address of the current @@ -89,7 +102,7 @@ #define EXC_XFER_TEMPLATE(n, hdlr, trap, copyee, tfer, ret) \ li r10,trap; \ stw r10,_TRAP(r11); \ - li r10,MSR_KERNEL; \ + LOAD_MSR_KERNEL(r10, MSR_KERNEL); \ copyee(r10, r9); \ bl tfer; \ i##n: \
As preparation for using head_32.h for head_40x.S, move LOAD_MSR_KERNEL() there and use it to load r10 with MSR_KERNEL value. In the mean time, this patch modifies it so that it takes into account the size of the passed value to determine if 'li' can be used or if 'lis/ori' is needed instead of using the size of MSR_KERNEL. This is done by using gas macro. Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> --- arch/powerpc/kernel/entry_32.S | 9 +-------- arch/powerpc/kernel/head_32.h | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 9 deletions(-)