Re: [Qemu-devel] [PULL v3 47/55] linux headers: update against Linux 5.2-rc1

["Alex Bennée" <alex.bennee@linaro.org>]

Cornelia Huck <cohuck@redhat.com> writes:

> On Wed, 22 May 2019 14:10:39 +0200
> Laurent Vivier <laurent@vivier.eu> wrote:
>
>> On 22/05/2019 14:07, Cornelia Huck wrote:
>> > On Wed, 22 May 2019 13:47:25 +0200
>> > Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>> >
>> >> On 5/21/19 5:28 PM, Cornelia Huck wrote:
>> >>> commit a188339ca5a396acc588e5851ed7e19f66b0ebd9
>> >>>
>> >>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>> >>> ---
>> >> [...]
>> >>>   #define __NR_mq_notify 184
>> >>>   __SC_COMP(__NR_mq_notify, sys_mq_notify, compat_sys_mq_notify)
>> >>>   #define __NR_mq_getsetattr 185
>> >>> @@ -536,8 +567,10 @@ __SC_COMP(__NR_msgsnd, sys_msgsnd, compat_sys_msgsnd)
>> >>>   __SYSCALL(__NR_semget, sys_semget)
>> >>>   #define __NR_semctl 191
>> >>>   __SC_COMP(__NR_semctl, sys_semctl, compat_sys_semctl)
>> >>> +#if defined(__ARCH_WANT_TIME32_SYSCALLS) || __BITS_PER_LONG != 32
>> >
>> > Eww. It seems only aarch64 sets __ARCH_WANT_TIME32_SYSCALLS, and the
>> > second condition probably catches others but not mipsel.
>> >
>> >>>   #define __NR_semtimedop 192
>> >>> -__SC_COMP(__NR_semtimedop, sys_semtimedop, compat_sys_semtimedop)
>> >>> +__SC_COMP(__NR_semtimedop, sys_semtimedop, sys_semtimedop_time32)
>> >>> +#endif
>> >>>   #define __NR_semop 193
>> >>>   __SYSCALL(__NR_semop, sys_semop)
>> >> [...]
>> >>
>> >> https://app.shippable.com/github/qemu/qemu/runs/1703/summary/console
>> >>
>> >> It seems this commit introduce a regression on mips32:
>> >>
>> >>    CC      mipsel-linux-user/linux-user/syscall.o
>> >> ./linux-user/syscall.c: In function 'safe_semtimedop':
>> >> ./linux-user/syscall.c:697:25: error: '__NR_semtimedop' undeclared
>> >> (first use in this function)
>> >>       return safe_syscall(__NR_##name, arg1, arg2, arg3, arg4); \
>> >
>> > So, we unconditionally deal with this syscall, i.e. we assume it is
>> > always present? (I'm not sure of the logic in linux-user code.)
>> >
>>
>> linux-user assumes it is present if __NR_msgsnd is present.
>
> Hm. The kernel change seems to break that assumption. Does anyone with
> mips knowledge have an idea whether that was intentional (and the
> linux-user code needs to be changed), or whether that's an issue on the
> kernel side?

Well as far as I understand it the kernel has been introducing new
individual syscalls across the board for all architectures (and may
eventually allow kernels to be built without the legacy stuff).
Meanwhile on the backend some architectures have never had a IPC syscall
to multiplex these things.

We could do:

modified   linux-user/syscall.c
@@ -761,7 +761,7 @@ safe_syscall2(int, nanosleep, const struct timespec *, req,
 safe_syscall4(int, clock_nanosleep, const clockid_t, clock, int, flags,
               const struct timespec *, req, struct timespec *, rem)
 #endif
-#ifdef __NR_msgsnd
+#ifndef __NR_ipc
 safe_syscall4(int, msgsnd, int, msgid, const void *, msgp, size_t, sz,
               int, flags)

which would revert to the old behaviour for systems that had the ipc
multiplexer while still using individual system calls for the systems
that never had it. Chatting to the kernel devs on IRC also makes me
wonder if our mapping between the single syscalls and the multiplexer
might be a bit naive. The IPC_64 flags was mentioned as behaving
differently on the new individual syscalls but I haven't been able to
track down exactly what that difference is yet.

--
Alex Bennée