[PATCH 20/25] arm64:ilp32: add sys_ilp32.c and a separate table (in entry.S) to use it

Catalin Marinas
2016-04-26T10:08:43Z

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.

--
Catalin