> -----Original Message----- > From: Rich Felker > Sent: Tuesday, May 5, 2020 7:59 PM > To: sidneym@codeaurora.org > Cc: musl@lists.openwall.com > Subject: Re: [musl] Hexagon DSP support > > On Tue, May 05, 2020 at 06:37:41PM -0500, sidneym@codeaurora.org wrote: > > >- The definitions in bits/msg.h, bits/sem.h, and bits/shm.h are not > > > time64-compatible. In fact bits/sem.h is a regression; you had it > > > right before and broke it. > > As you point out below the core of the problem was the missing time64 > > calls. I've updated the structures to use what I hope or the correct > > types. > > I think they're still wrong and you need to do like the existing 32-bit archs, > defining IPC_STAT to 0x102 and putting separate unsigned long *_lo/*_hi > members in the right places for the kernel ABI and adding the time_t members > at the ends. This is almost surely the case if _Alignof(long long) is 8 on hexagon, > since the kernel's include/uapi/asm-generic/msgbuf.h etc. have the lo/hi pairs > misaligned (struct ipc_perm is an odd number of 32-bit words). The fact that > you had to change qemu to make this work suggests that you still have it > wrong too -- you should not have to change qemu or the kernel to make it > work if you do it right. The IPC_STAT setting was a key element missing from the port. > > > >- The definition in bits/stat.h is using the wrong types (you need to > > > use libc type names for all the members; this is a policy > > > requirement to ensure that none inadvertently mismatch). Note that > > > there's not even really a need to use an arch-specific header here > > > anymore; the kernel definition comes from arch/hexagon/kstat.h and > > > the bits/stat.h can just be copied from aarch64 or something else > > > with a clean layout. > > > > I used OpenRISC's stat.h. > > Is there a reason? It has old __st_*tim32 members but you don't need them > since there's no old-musl-ABI involved. I think you had originally suggested using aarch64 and that is what is there now. > > > src/api/ftw.c:44:1: error: switch condition has boolean value > > [-Werror,-Wswitch-bool] > > C(S_ISBLK(0)) > > ^~~~~~~~~~~~~ > > You're still building libc-test with -Werror which will give bogus results. > > > src/api/sys_ipc.c:13:1: error: initializing 'uid_t *' (aka 'unsigned > > int *') with an expression of type 'int *' converts between pointers > > to integer types with different sign [-Werror,-Wpointer-sign] > > F(uid_t,uid) > > ^~~~~~~~~~~~ > > src/api/sys_ipc.c:3:20: note: expanded from macro 'F' > > #define F(t,n) {t *y = &x.n;} > > ^ ~~~~ > > This is happening because struct ipc_perm's members have the wrong types > (but I think you should just delete your bits/ipc.h because the generic one is > correct and matches the kernel's ABI for hexagon). QEMU for Hexagon's target_ipc_perm was based on the deprecated one cited in uapi/linux/ipc.h instead of the generic version in ./uapi/asm-generic/ipcbuf.h I removed the file, ipc.h from musl and corrected QEMU. > > > src/functional/ipc_msg.c:63: qid_ds.msg_qnum == 0 failed: got 16384, > > want 0 > > src/functional/ipc_msg.c:67: (long long)qid_ds.msg_rtime == 0 failed: > > got 6823500725968961536, want 0 > > src/functional/ipc_msg.c:69: qid_ds.msg_ctime >= t failed: got 69713, > > want >= 1588720066 > > src/functional/ipc_msg.c:73: qid_ds.msg_qbytes > 0 failed: got 0, want > > > 0 > > src/functional/ipc_msg.c:78: qid_ds.msg_qnum == 1 failed: got 16384, > > want 1 > > src/functional/ipc_msg.c:79: qid_ds.msg_lspid == getpid() failed: got > > 0, want 240818 > > src/functional/ipc_msg.c:81: msg_stime is 0 want >= 1588720066 > > src/functional/ipc_msg.c:130: child exit status: 256 FAIL > > src/functional/ipc_msg-static.exe [status 1] > > src/functional/ipc_msg.c:63: qid_ds.msg_qnum == 0 failed: got 16384, > > want 0 > > src/functional/ipc_msg.c:67: (long long)qid_ds.msg_rtime == 0 failed: > > got 6823500725968961536, want 0 > > src/functional/ipc_msg.c:69: qid_ds.msg_ctime >= t failed: got 0, want > > >= 1588720066 > > src/functional/ipc_msg.c:73: qid_ds.msg_qbytes > 0 failed: got 0, want > > > 0 > > src/functional/ipc_msg.c:78: qid_ds.msg_qnum == 1 failed: got 16384, > > want 1 > > src/functional/ipc_msg.c:79: qid_ds.msg_lspid == getpid() failed: got > > 0, want 240786 > > src/functional/ipc_msg.c:81: msg_stime is 0 want >= 1588720066 > > src/functional/ipc_msg.c:130: child exit status: 256 FAIL > > src/functional/ipc_msg.exe [status 1] > > These indicate your struct ipc_perm/msqid_ds mismatch kernel, as described > above. > > Otherwise I didn't see anything obviously wrong in tests. Having less noise > from -Werror and the above issues would make it easier to review the report > though and ensure I'm not missing anything. I attached the updated REPORT with warning output disabled, -w and -fno-rounding-math (See https://bugs.llvm.org/show_bug.cgi?id=45329) along with the patch. I've rebased a couple of times without any conflicts and the git repo is here: https://github.com/quic/musl/tree/hexagon My most recent rebase was yesterday and that is what I used for the patch and the libc-test. I'd like to get Hexagon added as a supported architecture and I am certain there are preferred windows when new architectures are accepted. Understanding when those are and what requirements for inclusion are would be good to know. Thanks > > Rich