Re: [PATCH] linux-user: Fix msqid_ds struct wrt 32-bit big endian architectures

CC linux-m68k

On Sat, 1 Aug 2026 at 23:39, Helge Deller <[email protected]> wrote:
>
> (Adding Geert as kernel m68k maintainer)
>
> On 8/1/26 22:58, Helge Deller wrote:
> > On 8/1/26 14:11, Peter Maydell wrote:
> >> On Fri, 31 Jul 2026 at 20:36, Helge Deller <[email protected]> wrote:
> >>>
> >>> Hello Peter,
> >>>
> >>> On 7/31/26 20:05, Peter Maydell wrote:
> >>>> On Tue, 28 Jul 2026 at 20:32, Helge Deller <[email protected]> wrote:
> >>>>>
> >>>>> From: Helge Deller <[email protected]>
> >>>>>
> >>>>> Make sure that the time entries (msg_stime, msg_rtime and msg_ctime)
> >>>>> are defined as 64-bit time_t values, since the userspace may access
> >>>>> the whole 64-bit value. By this change we fix the word ordering for
> >>>>> 32-bit big endian architectures as well.
> >>>>>
> >>>>> This fixes the msgctl01 LTP testcase on hppa32.
> >>>>>
> >>>>> Signed-off-by: Helge Deller <[email protected]>
> >>>>> ---
> >>>>>    linux-user/syscall.c | 30 ++++++++++++------------------
> >>>>>    1 file changed, 12 insertions(+), 18 deletions(-)
> >>>>>
> >>>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> >>>>> index 740142825d..c93b770ced 100644
> >>>>> --- a/linux-user/syscall.c
> >>>>> +++ b/linux-user/syscall.c
> >>>>> @@ -4216,21 +4216,15 @@ static inline abi_long do_semtimedop(int semid,
> >>>>>    }
> >>>>>    #endif
> >>>>
> >>>> I see this has already gone into git, but some late review
> >>>> comments. I suspect this is mostly me being confused rather
> >>>> than actual problems.
> >>>>
> >>>>>
> >>>>> +#define target_time64_t         abi_ullong
> >>>>> +#define target_swap_time64(x)   tswap64(x)
> >>>>> +
> >>>>>    struct target_msqid_ds
> >>>>
> >>>> Is this the kernel's "struct msqid_ds" (which it calls "Obsolete, used
> >>>> only for backwards compatibility and libc5 compiles") or its msqid64_ds?
> >>>
> >>> Yes, it's msqid64_ds.
> >>>
> >>>> The layout matches msqid64_ds, which makes our struct a bit
> >>>> confusingly named.
> >>>
> >>> True.
> >>> In a follow-up patch this better should be renamed to target_msqid64_ds.
> >>>
> >>>>>    {
> >>>>>        struct target_ipc_perm msg_perm;
> >>>>> -    abi_ulong msg_stime;
> >>>>> -#if TARGET_ABI_BITS == 32
> >>>>> -    abi_ulong __unused1;
> >>>>> -#endif
> >>>>> -    abi_ulong msg_rtime;
> >>>>> -#if TARGET_ABI_BITS == 32
> >>>>> -    abi_ulong __unused2;
> >>>>> -#endif
> >>>>> -    abi_ulong msg_ctime;
> >>>>> -#if TARGET_ABI_BITS == 32
> >>>>> -    abi_ulong __unused3;
> >>>>> -#endif
> >>>>> +    target_time64_t msg_stime;
> >>>>> +    target_time64_t msg_rtime;
> >>>>> +    target_time64_t msg_ctime;
> >>>>
> >>>> Assuming msqid64_ds, the kernel version of this struct has a comment:
> >>>>    * 64 bit architectures use a 64-bit long time field here, while
> >>>>    * 32 bit architectures have a pair of unsigned long values.
> >>>>    * On big-endian systems, the lower half is in the wrong place.
> >>>
> >>> Those comments are from the generic header:
> >>> include/uapi/asm-generic/msgbuf.h
> >>> and I think the last sentence about big-endian systems is wrong, as
> >>> most platforms provide an own architecture-specific header file, e.g:
> >>> arch/xtensa/include/uapi/asm/msgbuf.h
> >>> and the big-endian platforms seem to have their high-word first.
> >>
> >> The comment is correct for any bigendian 32-bit system that
> >> uses the kernel's asm-generic structure. Maybe there just aren't
> >> any of those that we care about?
> >
> > m68k could be a problem.
> > The other big-arches seem to be handled correctly by the kernel:
> > arch/sparc/include/uapi/asm/msgbuf.h:struct msqid64_ds {
> > arch/parisc/include/uapi/asm/msgbuf.h:struct msqid64_ds {
> > arch/xtensa/include/uapi/asm/msgbuf.h:struct msqid64_ds {
> > arch/x86/include/uapi/asm/msgbuf.h:struct msqid64_ds {
> > arch/powerpc/include/uapi/asm/msgbuf.h:struct msqid64_ds {
> > arch/mips/include/uapi/asm/msgbuf.h:struct msqid64_ds {
> > arch/mips/include/uapi/asm/msgbuf.h:struct msqid64_ds {
> > arch/mips/include/uapi/asm/msgbuf.h:struct msqid64_ds {
> >
> >>>> That would make tswap64() not the right swap for 32-bit big
> >>>> endian guests.
> >>>
> >>> I did my testing on the 32-bit hppa/parisc platform, and with my patch
> >>> the results in qemu were correct and were the same as on physical machines.
> >>>
> >>> I just tried in a 32-bit powerpc chroot and the msgctl01 now succeeds there as well.
> >>> So, I think my patch is generally ok.
> >>
> >> Yeah, 32-bit powerpc also provides its own structure definition.
> >>
> >> I think it's worth having a comment about this, at least, because
> >> otherwise the obvious assumption is that our struct which has
> >> no target-specific ifdefs is supposed to match the kernel's
> >> asm-generic struct.
> >
> > Yes, I will the testing on m68k, afterwards I can come up with a proposal.
> > Maybe it makes sense to modify the kernel comment too....
>
>
> The msgctl01 testcase from LTP gives this preprocessed code on m68k:
> ...
> # 27 "/usr/include/m68k-linux-gnu/bits/types/struct_msqid_ds.h" 3 4
> struct msqid_ds
> {
>
> # 1 "/usr/include/m68k-linux-gnu/bits/types/struct_msqid64_ds_helper.h" 1 3 4
> # 20 "/usr/include/m68k-linux-gnu/bits/types/struct_msqid64_ds_helper.h" 3 4
>    struct ipc_perm msg_perm;
>    __time64_t msg_stime;
>    __time64_t msg_rtime;
>    __time64_t msg_ctime;
>    __syscall_ulong_t __msg_cbytes;
>    msgqnum_t msg_qnum;
>    msglen_t msg_qbytes;
>    __pid_t msg_lspid;
>    __pid_t msg_lrpid;
>    unsigned long int __glibc_reserved4;
>    unsigned long int __glibc_reserved5;
> # 31 "/usr/include/m68k-linux-gnu/bits/types/struct_msqid_ds.h" 2 3 4
> # 53 "/usr/include/m68k-linux-gnu/bits/types/struct_msqid_ds.h" 3 4
> };
>
> but the testcase fails with:
> msgctl01.c:53: TFAIL: msg_ctime = 6a6e65d900000000, expected 6a6e65d9
>
> So, for m68k either the glibc or the kernel is wrong.
> At least they don't agree with each other.
>
> I think it would be beneficial, if the m68k kernel would follow m68k's
> glibc implementation of msqid64_ds.
>
> Helge