| Message ID | 20160425184740.GA2559@yury-N73SV (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 25, 2016 at 09:47:40PM +0300, Yury Norov wrote: > On Mon, Apr 25, 2016 at 09:19:13PM +0300, Yury Norov wrote: > > On Mon, Apr 25, 2016 at 06:26:56PM +0100, Catalin Marinas wrote: > > > On Wed, Apr 06, 2016 at 01:08:42AM +0300, Yury Norov wrote: > > > > --- a/arch/arm64/kernel/entry.S > > > > +++ b/arch/arm64/kernel/entry.S > > > > @@ -715,9 +715,13 @@ ENDPROC(ret_from_fork) > > > > */ > > > > .align 6 > > > > el0_svc: > > > > - adrp stbl, sys_call_table // load syscall table pointer > > > > uxtw scno, w8 // syscall number in w8 > > > > mov sc_nr, #__NR_syscalls > > > > +#ifdef CONFIG_ARM64_ILP32 > > > > + ldr x16, [tsk, #TI_FLAGS] > > > > + tbnz x16, #TIF_32BIT_AARCH64, el0_ilp32_svc // We are using ILP32 > > > > +#endif > > > > > > There is another ldr x16, [tsk, #TI_FLAGS] load further down in the > > > el0_svc_naked block. We should rework these a bit to avoid loading the > > > same location twice unnecessarily. E.g. move the ldr x16 just before > > > el0_svc_naked and branch one line after in case of the ILP32 syscall. > > > > > > > Yes, I thiks we can refactor it. Thanks for a catch. > > Now it's better, I think > > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S > index cf4d1ae..21312bb 100644 > --- a/arch/arm64/kernel/entry.S > +++ b/arch/arm64/kernel/entry.S > @@ -715,16 +715,22 @@ ENDPROC(ret_from_fork) > */ > .align 6 > el0_svc: > - adrp stbl, sys_call_table // load syscall table pointer > uxtw scno, w8 // syscall number in w8 > mov sc_nr, #__NR_syscalls > + ldr x16, [tsk, #TI_FLAGS] You can move this higher up for interlocking reasons (though these days CPUs do a lot of speculative loads). > +#ifdef CONFIG_ARM64_ILP32 > + tbz x16, #TIF_32BIT_AARCH64, el0_lp64_svc // We are using ILP32 // We are *not* using ILP32 > + adrp stbl, sys_call_ilp32_table // load ilp32 syscall table pointer > + b el0_svc_naked > +el0_lp64_svc: > +#endif > + adrp stbl, sys_call_table // load syscall table pointer You can avoid the branches by using csel, something like this: ldr x16, [tsk, #TI_FLAGS] adrp stbl, sys_call_table ... #ifdef CONFIG_ARM64_ILP32 adrp x17, sys_call_ilp32_table tst x16, #_TIF_32BIT_AARCH64 csel stbl, stbl, x17, eq #endif el0_svc_naked: ... > el0_svc_naked: // compat entry point > stp x0, scno, [sp, #S_ORIG_X0] // save the original x0 and syscall number > enable_dbg_and_irq > ct_user_exit 1 > > - ldr x16, [tsk, #TI_FLAGS] // check for syscall hooks > - tst x16, #_TIF_SYSCALL_WORK > + tst x16, #_TIF_SYSCALL_WORK // check for syscall hooks > b.ne __sys_trace > cmp scno, sc_nr // check upper syscall limit > b.hs ni_sys There is el0_svc_compat branching to el0_svc_naked and it won't have x16 set anymore. So you need to add an ldr x16 to el0_svc_compat as well.
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S index cf4d1ae..21312bb 100644 --- a/arch/arm64/kernel/entry.S +++ b/arch/arm64/kernel/entry.S @@ -715,16 +715,22 @@ ENDPROC(ret_from_fork) */ .align 6 el0_svc: - adrp stbl, sys_call_table // load syscall table pointer uxtw scno, w8 // syscall number in w8 mov sc_nr, #__NR_syscalls + ldr x16, [tsk, #TI_FLAGS] +#ifdef CONFIG_ARM64_ILP32 + tbz x16, #TIF_32BIT_AARCH64, el0_lp64_svc // We are using ILP32 + adrp stbl, sys_call_ilp32_table // load ilp32 syscall table pointer + b el0_svc_naked +el0_lp64_svc: +#endif + adrp stbl, sys_call_table // load syscall table pointer el0_svc_naked: // compat entry point stp x0, scno, [sp, #S_ORIG_X0] // save the original x0 and syscall number enable_dbg_and_irq ct_user_exit 1 - ldr x16, [tsk, #TI_FLAGS] // check for syscall hooks - tst x16, #_TIF_SYSCALL_WORK + tst x16, #_TIF_SYSCALL_WORK // check for syscall hooks b.ne __sys_trace cmp scno, sc_nr // check upper syscall limit b.hs ni_sys