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....
We could alternatively follow the kernel more closely in having our
generic struct match the kernel's and then having the archs which
have custom versions override that, but that would be a bunch of
extra work to get to the same place in the end.
Let me do the testing on m68k. If the tests fail with the current code,
we need to add custom versions anyway.
Helge
