Re: [PATCH v5 05/20] linux-user: mips: Update syscall numbers to kernel 5.5 rc3 level

[Aleksandar Markovic]

On Tuesday, February 4, 2020, Laurent Vivier <address@hidden> wrote:
> Le 04/02/2020 à 12:56, Aleksandar Markovic a écrit :
> > On Tue, Jan 14, 2020 at 5:40 PM Laurent Vivier <address@hidden> wrote:
> >>
> >> Le 13/01/2020 à 21:34, Aleksandar Markovic a écrit :
> >>> From: Aleksandar Markovic <address@hidden>
> >>>
> >>> Update mips syscall numbers based on Linux kernel tag v5.5-rc3
> >>> (commit 46cf053e).
> >>>
> >>> Signed-off-by: Aleksandar Markovic <address@hidden>
> >>> ---
> >>>  linux-user/mips/cpu_loop.c     | 78 +++++++++++++++++++++++++++++++++++++++++-
> >>>  linux-user/mips/syscall_nr.h   | 45 ++++++++++++++++++++++++
> >>>  linux-user/mips64/syscall_nr.h | 13 +++++++
> >>>  3 files changed, 135 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/linux-user/mips/cpu_loop.c b/linux-user/mips/cpu_loop.c
> >>> index 39915b3..b81479b 100644
> >>> --- a/linux-user/mips/cpu_loop.c
> >>> +++ b/linux-user/mips/cpu_loop.c
> >>> @@ -25,8 +25,9 @@
> >>>  #include "internal.h"
> >>>
> >>>  # ifdef TARGET_ABI_MIPSO32
> >>> +#  define MIPS_SYSCALL_NUMBER_UNUSED -1
> >>
> >> I'm not sure you need to introduce this change.
> >>
> >> The case already exists (stat, fstat, old_select, lstat, ...) and the
> >> entry that was used is:
> >>
> >>            MIPS_SYS(sys_ni_syscall , 0)
> >>
> >> perhaps you can do the same ?
> >>
> > 
> > I like better the new way of dealing with this (like in this patch), since it is
> > more explicit. For now, I won't change this patch, if you agree, and later on
> > I will synchronize all such cases to be the same (but this is not critical for
> > the time being).
>
> So Perhaps you can do:
>
> #  define MIPS_SYSCALL_NUMBER_UNUSED 0
>
> And so the real availability of the syscall will be detected in
> do_syscall1() based on a "#ifdef TARGET_XXXX" (well, not sure
> syscall_nr.h differentiates O32 and N32)
>
> By the way, do we really need this table?
>
> It seems kernel is always copying 4 words from the stack.
> But kernel code is not really clear, perhaps I'm missing something...

Yes, Laurent, we actually don't need that table. We could have just a series of preprocessor constants, like any other ABI, but it would have required larger code reorganization. It started that way long time ago, and nobody bothered to correct this, partially for sure because of fear of regressions.

I am not in the mood for refactoring this area of code right now in any substantial way, until the situation stabilizes with new syscalls. The current solution in this patch is good enough for me, and it is not that intrusive, which I like. Please accept it as is, and the time for refactoring will hopefully come at some later point of time.

I prefer several incremental smaller refactoring to one large one. I'll take into account all your suggestions and hints.

Thanks, Aleksandar

 

> In the end, as you are the MIPS maintainer, you can choose the solution
> you prefer.
>
> Thanks,
> Laurent