Re: [PATCH v4 03/20] powerpc/32: Remove powerpc select specialisation

[Rohan McLure <rmclure@linux.ibm.com>]

> On 12 Sep 2022, at 7:03 pm, Nicholas Piggin <npiggin@gmail.com> wrote:
> 
> On Wed Aug 24, 2022 at 12:05 PM AEST, Rohan McLure wrote:
>> Syscall #82 has been implemented for 32-bit platforms in a unique way on
>> powerpc systems. This hack will in effect guess whether the caller is
>> expecting new select semantics or old select semantics. It does so via a
>> guess, based off the first parameter. In new select, this parameter
>> represents the length of a user-memory array of file descriptors, and in
>> old select this is a pointer to an arguments structure.
>> 
>> The heuristic simply interprets sufficiently large values of its first
>> parameter as being a call to old select. The following is a discussion
>> on how this syscall should be handled.
>> 
>> Link: https://lore.kernel.org/lkml/13737de5-0eb7-e881-9af0-163b0d29a1a0@csgroup.eu/
> 
> Seems okay to me, probably Christophe needs to ack it.
> Should some of that history be included directly in this changelog?
> 
> Should ppc64 compat be added back too, if this is being updated instead
> of removed? I don't know much about compat but it seems odd not provide
> it (considering it's just using compat_sys_old_select, isn't it?

That would make sense to me. I’ll put that in syscall.tbl.

> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> 
>> 
>> As discussed in this thread, the existence of such a hack suggests that for
>> whatever powerpc binaries may predate glibc, it is most likely that they
>> would have taken use of the old select semantics. x86 and arm64 both
>> implement this syscall with oldselect semantics.
>> 
>> Remove the powerpc implementation, and update syscall.tbl to refer to emit
>> a reference to sys_old_select for 32-bit binaries, in keeping with how
>> other architectures support syscall #82.
>> 
>> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
>> ---
>> V1 -> V2: Remove arch-specific select handler
>> V2 -> V3: Remove ppc_old_select prototype in <asm/syscalls.h>. Move to
>> earlier in patch series
>> ---
>> arch/powerpc/include/asm/syscalls.h           |  2 --
>> arch/powerpc/kernel/syscalls.c                | 17 -----------------
>> arch/powerpc/kernel/syscalls/syscall.tbl      |  2 +-
>> .../arch/powerpc/entry/syscalls/syscall.tbl   |  2 +-
>> 4 files changed, 2 insertions(+), 21 deletions(-)
>> 
>> diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
>> index 675a8f5ec3ca..739498c358a1 100644
>> --- a/arch/powerpc/include/asm/syscalls.h
>> +++ b/arch/powerpc/include/asm/syscalls.h
>> @@ -18,8 +18,6 @@ long sys_mmap2(unsigned long addr, size_t len,
>> 	       unsigned long fd, unsigned long pgoff);
>> long ppc64_personality(unsigned long personality);
>> long sys_rtas(struct rtas_args __user *uargs);
>> -int ppc_select(int n, fd_set __user *inp, fd_set __user *outp,
>> -	       fd_set __user *exp, struct __kernel_old_timeval __user *tvp);
>> long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
>> 		      u32 len_high, u32 len_low);
>> 
>> diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
>> index fc999140bc27..ef5896bee818 100644
>> --- a/arch/powerpc/kernel/syscalls.c
>> +++ b/arch/powerpc/kernel/syscalls.c
>> @@ -63,23 +63,6 @@ SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
>> 	return do_mmap2(addr, len, prot, flags, fd, offset, PAGE_SHIFT);
>> }
>> 
>> -#ifdef CONFIG_PPC32
>> -/*
>> - * Due to some executables calling the wrong select we sometimes
>> - * get wrong args.  This determines how the args are being passed
>> - * (a single ptr to them all args passed) then calls
>> - * sys_select() with the appropriate args. -- Cort
>> - */
>> -int
>> -ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp, struct __kernel_old_timeval __user *tvp)
>> -{
>> -	if ((unsigned long)n >= 4096)
>> -		return sys_old_select((void __user *)n);
>> -
>> -	return sys_select(n, inp, outp, exp, tvp);
>> -}
>> -#endif
>> -
>> #ifdef CONFIG_PPC64
>> long ppc64_personality(unsigned long personality)
>> {
>> diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
>> index 2600b4237292..4cbbb810ae10 100644
>> --- a/arch/powerpc/kernel/syscalls/syscall.tbl
>> +++ b/arch/powerpc/kernel/syscalls/syscall.tbl
>> @@ -110,7 +110,7 @@
>> 79	common	settimeofday			sys_settimeofday		compat_sys_settimeofday
>> 80	common	getgroups			sys_getgroups
>> 81	common	setgroups			sys_setgroups
>> -82	32	select				ppc_select			sys_ni_syscall
>> +82	32	select				sys_old_select			sys_ni_syscall
>> 82	64	select				sys_ni_syscall
>> 82	spu	select				sys_ni_syscall
>> 83	common	symlink				sys_symlink
>> diff --git a/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl b/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
>> index 2600b4237292..4cbbb810ae10 100644
>> --- a/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
>> +++ b/tools/perf/arch/powerpc/entry/syscalls/syscall.tbl
>> @@ -110,7 +110,7 @@
>> 79	common	settimeofday			sys_settimeofday		compat_sys_settimeofday
>> 80	common	getgroups			sys_getgroups
>> 81	common	setgroups			sys_setgroups
>> -82	32	select				ppc_select			sys_ni_syscall
>> +82	32	select				sys_old_select			sys_ni_syscall
>> 82	64	select				sys_ni_syscall
>> 82	spu	select				sys_ni_syscall
>> 83	common	symlink				sys_symlink
>> -- 
>> 2.34.1