Date: Sat, 10 Nov 2018 22:34:04 -0800 From: Michael Forney <mforney@...rney.org> To: musl@...ts.openwall.com Subject: Re: riscv port for review On 2018-10-18, Michael Forney <mforney@...rney.org> wrote: > On 2018-10-11, Michael Forney <mforney@...rney.org> wrote: >> On 2018-09-27, Rich Felker <dalias@...c.org> wrote: >>> Pulled from here: >>> https://github.com/riscv/riscv-musl/commit/6a4f4a9c774608add4b02f95322518bd2f5f51ee >>> >>> Attached for review. >> >> I noticed that some fcntl.h constants are incorrect (O_DIRECTORY, >> O_NOFOLLOW, O_DIRECT, O_LARGEFILE, and O_TMPFILE). Linux doesn't seem >> to have a riscv-specific fcntl.h, so I think they just come from >> asm-generic. > > While playing around with a riscv32-linux-musl toolchain and tinyemu, > I found a few more issues: > > - riscv linux has no renameat syscall. I think __NR_renameat needs to > be removed from arch/riscv*/bits/syscall.h.in, and musl's rename and > renameat need to fallback to SYS_renameat2 if SYS_renameat is not > defined. > > - arch/riscv32/bits/syscall.h.in defines syscall names as if it were > 64-bit. I think the following changes are necessary > __NR_fcntl -> __NR_fcntl64 > __NR_statfs -> __NR_statfs64 > __NR_fstatfs -> __NR_fstatfs64 > __NR_truncate -> __NR_truncate64 > __NR_ftruncate -> __NR_ftruncate64 > __NR_lseek -> __NR__llseek (and __NR_llseek?) > __NR_fstatat -> __NR_fstatat64 > __NR_fstat -> __NR_fstat64 > __NR_mmap -> __NR_mmap2 > __NR_fadvise64 -> __NR_fadvise64_64 > > - Since riscv32 uses fcntl64, and musl's struct flock corresponds to > struct flock64 on 32-bit, F_GETLK, F_SETLK, and F_SETLKW should be > defined to the corresponding *64 values (12, 13, 14). This matches > arch/generic/bits/fcntl.h, so I think arch/riscv32/bits/fcntl.h should > just be completely removed as Rich suggested. > arch/riscv64/bits/fcntl.h needs to stay to define the non *64 values > (5, 6, 7). > > - For detecting soft float, configure compares $ARCH to riscv and > riscv64, but ARCH is set to riscv32 or riscv64 above. Also ARCH is set > to riscv32 when $target is riscv* but not riscv64*. Should this be > riscv32*? > > - There are several instances of preprocessor checks > __riscv_soft_float, but this does not seem to be defined by gcc. > Perhaps this is superseded by __riscv_flen or __riscv_float_abi_soft? > > - The functions in src/math/riscv* don't fall back to the C > implementation for soft float. Forgot to mention one other thing I found: __SYSCALL_LL_O on riscv32 is defined as if registers pairs need to be aligned for syscalls, but that does not appear to be the case. My proposed fixes are available here: https://github.com/michaelforney/musl/compare/6a4f4a9...riscv Zach van Rijn has applied those patches in his toolchains at https://musl.cc, and Fabrice Bellard is using them in his buildroot port at https://bellard.org/tinyemu/buildroot.html. It'd be nice to get them applied/squashed into https://github.com/riscv/riscv-musl. I haven't done any actual review of the port (and I don't actually know much about riscv). I just ran into these issues while trying to get some programs to run under linux in tinyemu.
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.