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

[Christian Borntraeger]

On 22.05.19 15:50, Aleksandar Markovic wrote:
> 
> On May 22, 2019 3:42 PM, "Alex Bennée" <address@hidden 
> <mailto:address@hidden>> wrote:
>>
>>
>> Aleksandar Markovic <address@hidden <mailto:address@hidden>> writes:
>>
>> > On May 22, 2019 2:24 PM, "Cornelia Huck" <address@hidden 
>> > <mailto:address@hidden>> wrote:
>> >>
>> >> On Wed, 22 May 2019 14:10:39 +0200
>> >> Laurent Vivier <address@hidden <mailto:address@hidden>> wrote:
>> >>
>> >> > On 22/05/2019 14:07, Cornelia Huck wrote:
>> >> > > On Wed, 22 May 2019 13:47:25 +0200
>> >> > > Philippe Mathieu-Daudé <address@hidden <mailto:address@hidden>> wrote:
>> >> > >
>> >> > >> On 5/21/19 5:28 PM, Cornelia Huck wrote:
>> >> > >>> commit a188339ca5a396acc588e5851ed7e19f66b0ebd9
>> >> > >>>
>> >> > >>> Signed-off-by: Cornelia Huck <address@hidden 
>> >> > >>> <mailto:address@hidden>>
>> >> > >>> ---
>> >> > >> [...]
>> >> > >>>   #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?
>> >>
>> >
>> > Hi, Cornelia.
>> >
>> > Thanks for your involving into this issue!
>> >
>> > It could be that (not-originating-from-MIPS) kernel commit:
>> >
>> > https://github.com/torvalds/linux/commit/1a787fc5ba18ac767e635c58d06a0b46876184e3
>> >
>> > made a mess with system call availability for MIPS (I will forward this to
>> > MIPS kernel maintainer Paul Burton). My impression is that this was not
>> > intentional, and is a temporary instability of kernel interface.
>>
>> I think this stems from 2038 time bomb work. Eventually they want it to
>> be possible to build non-legacy kernels that don't expose time32 to the
>> outside world. As part of that new system calls are being introduced
>> where needed. The IPC syscall which orignally multiplexed a bunch of
>> these operations on some systems would eventually be potentially phased
>> out.
>>
>> > However, I think that QEMU nevertheless should not make the assumption that
>> > if __NR_MSGSND, than semtimedop() is present. It could be true, but it is
>> > still just self-imposed belief in QEMU, kernel never guarantied such 
>> > things.
>> >
>> > The alternative way of invoking via IPCV6 (else part of “ifdef
>> > __NR_MSGSND”) should work for MIPS in the present stage of headers and
>> > kernel.
>>
>> Yeah I think #ifndef __NR_ipc would work for now. It shouldn't affect
>> architectures that never had the IPC call.
>>
>> > As a side note, perhaps we shoul update kernel headers only off of stable
>> > kernel releases.
>>
>> I guess that's a part of the tension for supporting new kernel APIs
>> quickly. At least 5.2-rc1 wasn't a random tree - you would expect the
>> external facing ABI to be stable after the merge window closed. It would
>> be nice to know what new features were being exposed though.
>>
> 
> Yes, one would expect no intentional changes in ABI kernel headers would 
> happen after RC1. However, one must expect that there could certainly be bugs 
> in RC1 - and there is a larger risk of propagating these bugs to QEMU with 
> header updates from non-stable code.

An alternative would be to ONLY update the relevant changes of the headers
that are necessary for this particular pull request( in this case the s390
parts) and then do a full header sync on each release (e.g. 5.1, 5.2) to still
make sure that we are pretty much up2date.