mailing list of musl libc
 help / color / mirror / Atom feed
* [musl] riscv32 v2
@ 2020-09-04  5:48 Stefan O'Rear
  2020-09-07 10:47 ` Stefan O'Rear
                   ` (3 more replies)
  0 siblings, 4 replies; 21+ messages in thread
From: Stefan O'Rear @ 2020-09-04  5:48 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 3397 bytes --]

Changes since v1:

Fixed ptrace support by passing through high bits of WSTOPSIG.
WEXITSTATUS is still masked (required by POSIX); WTERMSIG is also
masked because bits 8-15 have nowhere to go.

Added SYS_futex as an alias of SYS_futex_time64.

Changed conditionals in patch 2.  __wait4 is significantly reorganized
and now uses a conditionally defined wrapper in src/internal/syscall.h.
Duplication reduced in statx-using patches.

Arnd Bergmann's comment about identical fcntl.h files has NOT been
addressed.

Rich Felker's suggestion (on IRC) to use a 0-instruction __get_tp was
NOT implemented after discovering that it generates dramatically worse
code on clang and cannot easily be conditionalized.  Bug reports to come.

Patches other than 2, 6, 7, 10 are unchanged.

Testing:

Smoke tested on riscv32, replacing the musl libc.so in an
OpenEmbedded-generated VM with a dynamically linked systemd and verified
boot.  Smoke testing on i386 and x86_64 by replacing libc.so in an
Alpine chroot and running build tools.

libc-test was run on all three architectures.  The errors on riscv32
are as follows:

FAIL src/api/main.exe [status 1]          
FAIL src/functional/fcntl-static.exe [status 1]                                                                                        
FAIL src/functional/fcntl.exe [status 1]                                                                                               
FAIL src/functional/ipc_msg-static.exe [status 1]
FAIL src/functional/ipc_msg.exe [status 1]                                                                                             
FAIL src/functional/ipc_sem-static.exe [status 1]                                                                                      
FAIL src/functional/ipc_sem.exe [status 1]
FAIL src/functional/ipc_shm-static.exe [status 1]               
FAIL src/functional/ipc_shm.exe [status 1]                     
FAIL src/functional/strptime-static.exe [status 1]         
FAIL src/functional/strptime.exe [status 1]                
FAIL src/math/fma.exe [status 1]                         
FAIL src/math/fmaf.exe [status 1]                 
FAIL src/math/powf.exe [status 1]                               
FAIL src/regression/malloc-brk-fail-static.exe [status 1]      
FAIL src/regression/malloc-brk-fail.exe [status 1]         
FAIL src/regression/pthread_atfork-errno-clobber-static.exe [status 1]
FAIL src/regression/pthread_atfork-errno-clobber.exe [status 1]

The fcntl and sysvipc errors do not correspond to any error in x86_64
and potentially require investigation, although they could be kernel
configuration issues.  x86_64 has a different but overlapping set of
math errors; qemu is known to not give bit-exact results for RISC-V
floating point.  The malloc, pthread, and src/api/main.exe failures
match failures on x86_64.

The test results are identical between master and my branch on x86_64.
On i386, I saw a utime.exe and utime-static.exe error but have not
managed to reproduce them.

I was not able to run LTP on musl on any of the three architectures
following the instructions in its README.

make autotools && ./configure && make all -j16
eventually results in:
confstr01.c:51:3: error: '_CS_XBS5_ILP32_OFF32_CFLAGS' undeclared here (not in a function)

A cloneable repository with the present version is:
git clone https://github.com/sorear/riscv-musl -b rv32_submit_v2

[-- Attachment #2: 0001-Remove-ARMSUBARCH-relic-from-configure.patch --]
[-- Type: application/octet-stream, Size: 721 bytes --]

[-- Attachment #3: 0002-time64-Don-t-make-aliases-to-nonexistent-syscalls.patch --]
[-- Type: application/octet-stream, Size: 1920 bytes --]

[-- Attachment #4: 0003-time64-Only-getrlimit-setrlimit-if-they-exist.patch --]
[-- Type: application/octet-stream, Size: 1949 bytes --]

[-- Attachment #5: 0004-time64-Only-gettimeofday-settimeofday-if-exist.patch --]
[-- Type: application/octet-stream, Size: 1328 bytes --]

[-- Attachment #6: 0005-Add-src-internal-statx.h.patch --]
[-- Type: application/octet-stream, Size: 2418 bytes --]

[-- Attachment #7: 0006-Only-call-fstatat-if-defined.patch --]
[-- Type: application/octet-stream, Size: 5779 bytes --]

[-- Attachment #8: 0007-Emulate-wait4-using-waitid.patch --]
[-- Type: application/octet-stream, Size: 5480 bytes --]

[-- Attachment #9: 0008-riscv-Fall-back-to-syscall-__riscv_flush_icache.patch --]
[-- Type: application/octet-stream, Size: 822 bytes --]

[-- Attachment #10: 0009-riscv32-Target-and-subtarget-detection.patch --]
[-- Type: application/octet-stream, Size: 1090 bytes --]

[-- Attachment #11: 0010-riscv32-add-arch-headers.patch --]
[-- Type: application/octet-stream, Size: 23648 bytes --]

[-- Attachment #12: 0011-riscv32-Add-fenv-and-math.patch --]
[-- Type: application/octet-stream, Size: 7215 bytes --]

[-- Attachment #13: 0012-riscv32-Add-dlsym.patch --]
[-- Type: application/octet-stream, Size: 610 bytes --]

[-- Attachment #14: 0013-riscv32-Add-jmp_buf-and-sigreturn.patch --]
[-- Type: application/octet-stream, Size: 3481 bytes --]

[-- Attachment #15: 0014-riscv32-Add-thread-support.patch --]
[-- Type: application/octet-stream, Size: 2837 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-04  5:48 [musl] riscv32 v2 Stefan O'Rear
@ 2020-09-07 10:47 ` Stefan O'Rear
  2020-09-07 18:06   ` Rich Felker
  2020-09-07 11:27 ` Stefan O'Rear
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 21+ messages in thread
From: Stefan O'Rear @ 2020-09-07 10:47 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 2104 bytes --]

On Fri, Sep 4, 2020, at 1:48 AM, Stefan O'Rear wrote:
> FAIL src/api/main.exe [status 1]
> FAIL src/functional/fcntl-static.exe [status 1]
> FAIL src/functional/fcntl.exe [status 1]
> FAIL src/functional/ipc_msg-static.exe [status 1]
> FAIL src/functional/ipc_msg.exe [status 1]
> FAIL src/functional/ipc_sem-static.exe [status 1]
> FAIL src/functional/ipc_sem.exe [status 1]
> FAIL src/functional/ipc_shm-static.exe [status 1]
> FAIL src/functional/ipc_shm.exe [status 1]
> FAIL src/functional/strptime-static.exe [status 1]
> FAIL src/functional/strptime.exe [status 1]
> FAIL src/math/fma.exe [status 1]
> FAIL src/math/fmaf.exe [status 1]
> FAIL src/math/powf.exe [status 1]
> FAIL src/regression/malloc-brk-fail-static.exe [status 1]
> FAIL src/regression/malloc-brk-fail.exe [status 1]
> FAIL src/regression/pthread_atfork-errno-clobber-static.exe [status 1]
> FAIL src/regression/pthread_atfork-errno-clobber.exe [status 1]
> 
> The fcntl and sysvipc errors do not correspond to any error in x86_64
> and potentially require investigation, although they could be kernel
> configuration issues.  x86_64 has a different but overlapping set of
> math errors; qemu is known to not give bit-exact results for RISC-V
> floating point.  The malloc, pthread, and src/api/main.exe failures
> match failures on x86_64.

Attached patch reaches test failure parity between riscv32 and riscv64
and will be included in v3.

* gdb HEAD wants ELF_NFPREG, so I set it in bits/user.h to the value
  gdb needs.  (glibc does #define ELF_NFPREG NFPREG and expects gdb
  to define NFPREG. I don't get this.)

* Restore accidentally removed errno setting in waitpid, fixes a gdb
  assertion failure.

* Zero IPC_64 because the kernel only recognizes one set of IPC commands.

* Copy the IPC_TIME64 bits from arch/arm/bits to trigger the musl code
  for fixing time64 IPC_STAT results.  I'm not super happy with this,
  maybe there should be a new mechanism in musl for fixing IPC_STAT for
  unconditionally-time64 architectures.

* riscv32 _does_ provide both F_GETLK and F_GETLK32; make sure we use
  the right one.

-s

[-- Attachment #2: post-v2.diff --]
[-- Type: application/octet-stream, Size: 4293 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-04  5:48 [musl] riscv32 v2 Stefan O'Rear
  2020-09-07 10:47 ` Stefan O'Rear
@ 2020-09-07 11:27 ` Stefan O'Rear
  2020-09-07 18:09   ` Rich Felker
  2020-09-08  1:54 ` Rich Felker
  2020-09-09 20:28 ` Rich Felker
  3 siblings, 1 reply; 21+ messages in thread
From: Stefan O'Rear @ 2020-09-07 11:27 UTC (permalink / raw)
  To: musl

On Fri, Sep 4, 2020, at 1:48 AM, Stefan O'Rear wrote:
> Rich Felker's suggestion (on IRC) to use a 0-instruction __get_tp was
> NOT implemented after discovering that it generates dramatically worse
> code on clang and cannot easily be conditionalized.  Bug reports to come.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96952
https://bugs.llvm.org/show_bug.cgi?id=47447

-s

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-07 10:47 ` Stefan O'Rear
@ 2020-09-07 18:06   ` Rich Felker
  2020-09-07 21:35     ` Arnd Bergmann
  0 siblings, 1 reply; 21+ messages in thread
From: Rich Felker @ 2020-09-07 18:06 UTC (permalink / raw)
  To: musl

On Mon, Sep 07, 2020 at 06:47:00AM -0400, Stefan O'Rear wrote:
> On Fri, Sep 4, 2020, at 1:48 AM, Stefan O'Rear wrote:
> > FAIL src/api/main.exe [status 1]
> > FAIL src/functional/fcntl-static.exe [status 1]
> > FAIL src/functional/fcntl.exe [status 1]
> > FAIL src/functional/ipc_msg-static.exe [status 1]
> > FAIL src/functional/ipc_msg.exe [status 1]
> > FAIL src/functional/ipc_sem-static.exe [status 1]
> > FAIL src/functional/ipc_sem.exe [status 1]
> > FAIL src/functional/ipc_shm-static.exe [status 1]
> > FAIL src/functional/ipc_shm.exe [status 1]
> > FAIL src/functional/strptime-static.exe [status 1]
> > FAIL src/functional/strptime.exe [status 1]
> > FAIL src/math/fma.exe [status 1]
> > FAIL src/math/fmaf.exe [status 1]
> > FAIL src/math/powf.exe [status 1]
> > FAIL src/regression/malloc-brk-fail-static.exe [status 1]
> > FAIL src/regression/malloc-brk-fail.exe [status 1]
> > FAIL src/regression/pthread_atfork-errno-clobber-static.exe [status 1]
> > FAIL src/regression/pthread_atfork-errno-clobber.exe [status 1]
> > 
> > The fcntl and sysvipc errors do not correspond to any error in x86_64
> > and potentially require investigation, although they could be kernel
> > configuration issues.  x86_64 has a different but overlapping set of
> > math errors; qemu is known to not give bit-exact results for RISC-V
> > floating point.  The malloc, pthread, and src/api/main.exe failures
> > match failures on x86_64.
> 
> Attached patch reaches test failure parity between riscv32 and riscv64
> and will be included in v3.
> 
> * gdb HEAD wants ELF_NFPREG, so I set it in bits/user.h to the value
>   gdb needs.  (glibc does #define ELF_NFPREG NFPREG and expects gdb
>   to define NFPREG. I don't get this.)

Ick. Indeed I think this is wrong/probably an oversight in glibc, and
the way you've done it sounds better.

> * Restore accidentally removed errno setting in waitpid, fixes a gdb
>   assertion failure.

OK. As an aside (I haven't gotten to sending review for this yet,
sorry) I think I'd prefer to name the function __sys_wait4 or similar
to make it more clear that it's analogous to __sys_open[23] etc. (a
function or macro emulating the syscall) and not a namespace-safe
version of wait4. (musl's having __wait be a futex wait makes this
even more confusing, btw)

Perhaps also leave the int cp argument to the function but make
separate __sys_wait4 and __sys_wait4_cp macros to call it via so that
there's not a mysterious boolean argument that doesn't correspond to
an actual syscall argument. (This would also be parallel with how
__sys_open[23] is done.)

> * Zero IPC_64 because the kernel only recognizes one set of IPC commands.

OK.

> * Copy the IPC_TIME64 bits from arch/arm/bits to trigger the musl code
>   for fixing time64 IPC_STAT results.  I'm not super happy with this,
>   maybe there should be a new mechanism in musl for fixing IPC_STAT for
>   unconditionally-time64 architectures.

If the riscv32 IPC syscalls don't actually provide in-place time64 but
require translation, I think it's fairly appropriate as-is.

From the definitions in your patch, it looks like all the time fields
are fixed-word-order (little endian) and possibly not aligned, so it
seems like they can't be used in-place. Is this correct?

> * riscv32 _does_ provide both F_GETLK and F_GETLK32; make sure we use
>   the right one.

IIRC someone already suggested using the generic bits/fcntl.h, which
would have solved this. I also have unpushed changed that let 64-bit
archs share the generic bits/fcntl.h too, via #if on __LONG_MAX.

Rich

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-07 11:27 ` Stefan O'Rear
@ 2020-09-07 18:09   ` Rich Felker
  0 siblings, 0 replies; 21+ messages in thread
From: Rich Felker @ 2020-09-07 18:09 UTC (permalink / raw)
  To: musl

On Mon, Sep 07, 2020 at 07:27:37AM -0400, Stefan O'Rear wrote:
> On Fri, Sep 4, 2020, at 1:48 AM, Stefan O'Rear wrote:
> > Rich Felker's suggestion (on IRC) to use a 0-instruction __get_tp was
> > NOT implemented after discovering that it generates dramatically worse
> > code on clang and cannot easily be conditionalized.  Bug reports to come.
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96952
> https://bugs.llvm.org/show_bug.cgi?id=47447

Thanks! I think the GCC issue & response to it supports my preference
not to use GCC __builtin_*, but I would like to see them fix it. The
form with __asm__("" : ...) is semantically correct and has always
worked, but I don't see enough benefit over the current non-empty asm
to justify accepting the worse codegen from llvm getting it wrong.
Maybe we can change it at some point in the future after llvm is
fixed, but it's really no big deal.

Rich

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-07 18:06   ` Rich Felker
@ 2020-09-07 21:35     ` Arnd Bergmann
  2020-09-07 21:45       ` Rich Felker
  0 siblings, 1 reply; 21+ messages in thread
From: Arnd Bergmann @ 2020-09-07 21:35 UTC (permalink / raw)
  To: musl

On Mon, Sep 7, 2020 at 8:06 PM Rich Felker <dalias@libc.org> wrote:
> On Mon, Sep 07, 2020 at 06:47:00AM -0400, Stefan O'Rear wrote:

> > * Copy the IPC_TIME64 bits from arch/arm/bits to trigger the musl code
> >   for fixing time64 IPC_STAT results.  I'm not super happy with this,
> >   maybe there should be a new mechanism in musl for fixing IPC_STAT for
> >   unconditionally-time64 architectures.
>
> If the riscv32 IPC syscalls don't actually provide in-place time64 but
> require translation, I think it's fairly appropriate as-is.
>
> From the definitions in your patch, it looks like all the time fields
> are fixed-word-order (little endian) and possibly not aligned, so it
> seems like they can't be used in-place. Is this correct?

Yes, rv32 uses the generic system call arguments, which are
unfortunately defined this way. In retrospect I wish I had
replaced the ipc syscalls with a sane version for time64, but at
the time time it seemed as easy way out to use the fields that
had been reserved for this purpose despite the broken
byte order and alignment.

       Arnd

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-07 21:35     ` Arnd Bergmann
@ 2020-09-07 21:45       ` Rich Felker
  2020-09-07 21:58         ` Arnd Bergmann
  0 siblings, 1 reply; 21+ messages in thread
From: Rich Felker @ 2020-09-07 21:45 UTC (permalink / raw)
  To: musl

On Mon, Sep 07, 2020 at 11:35:45PM +0200, Arnd Bergmann wrote:
> On Mon, Sep 7, 2020 at 8:06 PM Rich Felker <dalias@libc.org> wrote:
> > On Mon, Sep 07, 2020 at 06:47:00AM -0400, Stefan O'Rear wrote:
> 
> > > * Copy the IPC_TIME64 bits from arch/arm/bits to trigger the musl code
> > >   for fixing time64 IPC_STAT results.  I'm not super happy with this,
> > >   maybe there should be a new mechanism in musl for fixing IPC_STAT for
> > >   unconditionally-time64 architectures.
> >
> > If the riscv32 IPC syscalls don't actually provide in-place time64 but
> > require translation, I think it's fairly appropriate as-is.
> >
> > From the definitions in your patch, it looks like all the time fields
> > are fixed-word-order (little endian) and possibly not aligned, so it
> > seems like they can't be used in-place. Is this correct?
> 
> Yes, rv32 uses the generic system call arguments, which are
> unfortunately defined this way. In retrospect I wish I had
> replaced the ipc syscalls with a sane version for time64, but at
> the time time it seemed as easy way out to use the fields that
> had been reserved for this purpose despite the broken
> byte order and alignment.

Thanks for clarifying. BTW does passing IPC_64 produce an error on
rv32? If so, this is another advantage of keeping the IPC_TIME64 bit
-- it would catch programs bypassing libc and making the syscalls
directly.

Rich

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-07 21:45       ` Rich Felker
@ 2020-09-07 21:58         ` Arnd Bergmann
  2020-09-07 22:11           ` Rich Felker
  0 siblings, 1 reply; 21+ messages in thread
From: Arnd Bergmann @ 2020-09-07 21:58 UTC (permalink / raw)
  To: musl

On Mon, Sep 7, 2020 at 11:46 PM Rich Felker <dalias@libc.org> wrote:
> On Mon, Sep 07, 2020 at 11:35:45PM +0200, Arnd Bergmann wrote:
> > On Mon, Sep 7, 2020 at 8:06 PM Rich Felker <dalias@libc.org> wrote:
> > > On Mon, Sep 07, 2020 at 06:47:00AM -0400, Stefan O'Rear wrote:
> >
> > > > * Copy the IPC_TIME64 bits from arch/arm/bits to trigger the musl code
> > > >   for fixing time64 IPC_STAT results.  I'm not super happy with this,
> > > >   maybe there should be a new mechanism in musl for fixing IPC_STAT for
> > > >   unconditionally-time64 architectures.
> > >
> > > If the riscv32 IPC syscalls don't actually provide in-place time64 but
> > > require translation, I think it's fairly appropriate as-is.
> > >
> > > From the definitions in your patch, it looks like all the time fields
> > > are fixed-word-order (little endian) and possibly not aligned, so it
> > > seems like they can't be used in-place. Is this correct?
> >
> > Yes, rv32 uses the generic system call arguments, which are
> > unfortunately defined this way. In retrospect I wish I had
> > replaced the ipc syscalls with a sane version for time64, but at
> > the time time it seemed as easy way out to use the fields that
> > had been reserved for this purpose despite the broken
> > byte order and alignment.
>
> Thanks for clarifying. BTW does passing IPC_64 produce an error on
> rv32? If so, this is another advantage of keeping the IPC_TIME64 bit
> -- it would catch programs bypassing libc and making the syscalls
> directly.

Yes, this is now the generic behavior for the split IPC syscalls
(as opposed to sys_ipc on older architectures). The only architectures
that parse the version in the split ipc syscalls are the ones that
already had these and were interpreting IPC_64 before linux-5.1:
alpha, arm32, microblaze, mips-n32, mips-n64, and xtensa.

There are additional architectures that require passing IPC_64
in sys_ipc() but reject it in the split syscalls: m68k, mips-o32,
powerpc, s390, sh, sparc, and x86.

        Arnd

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-07 21:58         ` Arnd Bergmann
@ 2020-09-07 22:11           ` Rich Felker
  2020-09-07 22:30             ` Arnd Bergmann
  0 siblings, 1 reply; 21+ messages in thread
From: Rich Felker @ 2020-09-07 22:11 UTC (permalink / raw)
  To: musl

On Mon, Sep 07, 2020 at 11:58:20PM +0200, Arnd Bergmann wrote:
> On Mon, Sep 7, 2020 at 11:46 PM Rich Felker <dalias@libc.org> wrote:
> > On Mon, Sep 07, 2020 at 11:35:45PM +0200, Arnd Bergmann wrote:
> > > On Mon, Sep 7, 2020 at 8:06 PM Rich Felker <dalias@libc.org> wrote:
> > > > On Mon, Sep 07, 2020 at 06:47:00AM -0400, Stefan O'Rear wrote:
> > >
> > > > > * Copy the IPC_TIME64 bits from arch/arm/bits to trigger the musl code
> > > > >   for fixing time64 IPC_STAT results.  I'm not super happy with this,
> > > > >   maybe there should be a new mechanism in musl for fixing IPC_STAT for
> > > > >   unconditionally-time64 architectures.
> > > >
> > > > If the riscv32 IPC syscalls don't actually provide in-place time64 but
> > > > require translation, I think it's fairly appropriate as-is.
> > > >
> > > > From the definitions in your patch, it looks like all the time fields
> > > > are fixed-word-order (little endian) and possibly not aligned, so it
> > > > seems like they can't be used in-place. Is this correct?
> > >
> > > Yes, rv32 uses the generic system call arguments, which are
> > > unfortunately defined this way. In retrospect I wish I had
> > > replaced the ipc syscalls with a sane version for time64, but at
> > > the time time it seemed as easy way out to use the fields that
> > > had been reserved for this purpose despite the broken
> > > byte order and alignment.
> >
> > Thanks for clarifying. BTW does passing IPC_64 produce an error on
> > rv32? If so, this is another advantage of keeping the IPC_TIME64 bit
> > -- it would catch programs bypassing libc and making the syscalls
> > directly.
> 
> Yes, this is now the generic behavior for the split IPC syscalls

Great!

> (as opposed to sys_ipc on older architectures). The only architectures
> that parse the version in the split ipc syscalls are the ones that
> already had these and were interpreting IPC_64 before linux-5.1:
> alpha, arm32, microblaze, mips-n32, mips-n64, and xtensa.
> 
> There are additional architectures that require passing IPC_64
> in sys_ipc() but reject it in the split syscalls: m68k, mips-o32,
> powerpc, s390, sh, sparc, and x86.

Uhg, good to know. I just re-checked, and at present we don't use the
new split syscalls unless SYS_ipc doesn't exist. musl's arch-specific
IPC_64 definition (0 or 0x100) serves as the value needed for SYS_ipc
if SYS_ipc is defined, and as the value needed for the split syscalls
if SYS_ipc is not defined. So if in the future we want to use the new
ones with fallback to SYS_ipc, we'd need the archs to define the
needed IPC_64 flag separately for each...

As an aside, I should probably cleanup the current definition
framework where IPC_64==0x100 is the default and archs that want 0
have to define it explicitly. It looks like, for the most part, IPC_64
is needed iff SYS_ipc is defined. Of the archs we support, arm
(32-bit) and mips{n32,64} seem to be the only ones that lack SYS_ipc
but need the IPC_64 bit set. Does this agree with your assessment?

Rich

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-07 22:11           ` Rich Felker
@ 2020-09-07 22:30             ` Arnd Bergmann
  2020-09-08  1:02               ` Rich Felker
  0 siblings, 1 reply; 21+ messages in thread
From: Arnd Bergmann @ 2020-09-07 22:30 UTC (permalink / raw)
  To: musl

On Tue, Sep 8, 2020 at 12:12 AM Rich Felker <dalias@libc.org> wrote:
> As an aside, I should probably cleanup the current definition
> framework where IPC_64==0x100 is the default and archs that want 0
> have to define it explicitly. It looks like, for the most part, IPC_64
> is needed iff SYS_ipc is defined.

Right, there are no architectures that provide sys_ipc and want the
flag to be zero.

> Of the archs we support, arm
> (32-bit) and mips{n32,64} seem to be the only ones that lack SYS_ipc
> but need the IPC_64 bit set. Does this agree with your assessment?

I think microblaze is in the same group. Note that for odd reasons it
has always defined the __NR_ipc macro to 117 but hooked it up
to -ENOSYS instead of sys_ipc in the kernel. I'm never quite sure
whether we should treat that as a bug in the header file that we want
to fix, or whether we should keep such constants around in new
headers that were present in older ones.

      Arnd

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-07 22:30             ` Arnd Bergmann
@ 2020-09-08  1:02               ` Rich Felker
  2020-09-08  7:00                 ` Arnd Bergmann
  0 siblings, 1 reply; 21+ messages in thread
From: Rich Felker @ 2020-09-08  1:02 UTC (permalink / raw)
  To: musl

On Tue, Sep 08, 2020 at 12:30:27AM +0200, Arnd Bergmann wrote:
> On Tue, Sep 8, 2020 at 12:12 AM Rich Felker <dalias@libc.org> wrote:
> > As an aside, I should probably cleanup the current definition
> > framework where IPC_64==0x100 is the default and archs that want 0
> > have to define it explicitly. It looks like, for the most part, IPC_64
> > is needed iff SYS_ipc is defined.
> 
> Right, there are no architectures that provide sys_ipc and want the
> flag to be zero.
> 
> > Of the archs we support, arm
> > (32-bit) and mips{n32,64} seem to be the only ones that lack SYS_ipc
> > but need the IPC_64 bit set. Does this agree with your assessment?
> 
> I think microblaze is in the same group. Note that for odd reasons it
> has always defined the __NR_ipc macro to 117 but hooked it up
> to -ENOSYS instead of sys_ipc in the kernel. I'm never quite sure
> whether we should treat that as a bug in the header file that we want
> to fix, or whether we should keep such constants around in new
> headers that were present in older ones.

Oh, really? In that case musl's almost surely broken on microblaze,
and yes it would be another exception.

Rich

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-04  5:48 [musl] riscv32 v2 Stefan O'Rear
  2020-09-07 10:47 ` Stefan O'Rear
  2020-09-07 11:27 ` Stefan O'Rear
@ 2020-09-08  1:54 ` Rich Felker
  2020-09-09  6:07   ` Rich Felker
  2020-09-09 20:28 ` Rich Felker
  3 siblings, 1 reply; 21+ messages in thread
From: Rich Felker @ 2020-09-08  1:54 UTC (permalink / raw)
  To: musl

On Fri, Sep 04, 2020 at 01:48:19AM -0400, Stefan O'Rear wrote:
> From cd57a6b47783c5302f931e543b608cb3ba58387d Mon Sep 17 00:00:00 2001
> From: Stefan O'Rear <sorear@fastmail.com>
> Date: Thu, 3 Sep 2020 03:59:59 -0400
> Subject: [PATCH 06/14] Only call fstatat if defined
> 
> riscv32 and future architectures lack it.
> ---
>  src/stat/fchmodat.c   | 23 ++++++++++++++++++++---
>  src/stat/fstatat.c    |  6 ++++++
>  src/stdio/tempnam.c   |  9 +++++++--
>  src/stdio/tmpnam.c    |  9 +++++++--
>  src/time/__map_file.c | 19 +++++++++++++++----
>  5 files changed, 55 insertions(+), 11 deletions(-)
> 
> diff --git a/src/stat/fchmodat.c b/src/stat/fchmodat.c
> index 4ee00b0a..857e84e5 100644
> --- a/src/stat/fchmodat.c
> +++ b/src/stat/fchmodat.c
> @@ -1,8 +1,10 @@
>  #include <sys/stat.h>
>  #include <fcntl.h>
>  #include <errno.h>
> +#include <stdint.h>
>  #include "syscall.h"
>  #include "kstat.h"
> +#include "statx.h"
>  
>  int fchmodat(int fd, const char *path, mode_t mode, int flag)
>  {
> @@ -11,13 +13,22 @@ int fchmodat(int fd, const char *path, mode_t mode, int flag)
>  	if (flag != AT_SYMLINK_NOFOLLOW)
>  		return __syscall_ret(-EINVAL);
>  
> -	struct kstat st;
>  	int ret, fd2;
>  	char proc[15+3*sizeof(int)];
>  
> +#ifdef SYS_fstatat
> +	struct kstat st;
>  	if ((ret = __syscall(SYS_fstatat, fd, path, &st, flag)))
>  		return __syscall_ret(ret);
> -	if (S_ISLNK(st.st_mode))
> +	mode_t get_mode = st.st_mode;
> +#else
> +	struct statx st;
> +	if ((ret = __syscall(SYS_statx, fd, path, flag, STATX_TYPE, &st)))
> +		return __syscall_ret(ret);
> +	mode_t get_mode = st.stx_mode;
> +#endif
> +
> +	if (S_ISLNK(get_mode))
>  		return __syscall_ret(-EOPNOTSUPP);
>  
>  	if ((fd2 = __syscall(SYS_openat, fd, path, O_RDONLY|O_PATH|O_NOFOLLOW|O_NOCTTY|O_CLOEXEC)) < 0) {
> @@ -27,9 +38,15 @@ int fchmodat(int fd, const char *path, mode_t mode, int flag)
>  	}
>  
>  	__procfdname(proc, fd2);
> +#ifdef SYS_fstatat
>  	ret = __syscall(SYS_fstatat, AT_FDCWD, proc, &st, 0);
> +	get_mode = st.st_mode;
> +#else
> +	ret = __syscall(SYS_statx, AT_FDCWD, proc, 0, STATX_TYPE, &st);
> +	get_mode = st.stx_mode;
> +#endif
>  	if (!ret) {
> -		if (S_ISLNK(st.st_mode)) ret = -EOPNOTSUPP;
> +		if (S_ISLNK(get_mode)) ret = -EOPNOTSUPP;
>  		else ret = __syscall(SYS_fchmodat, AT_FDCWD, proc, mode);
>  	}
>  

I was just looking at this file for another reason, and wondered why
we're not just calling the fstatat function here. There's no namespace
reason not to. According to the description of commit
c9ebff4736128186121424364c1c62224b02aee3, use of struct kstat here was
done "as a low-risk fix" right before release of 1.2.0, and I probably
had in mind changing it to fstatat later and just never got around to
it.

The same could be done for tempnam (POSIX) but not tmpnam (C) or
__map_file (used in plain C interfaces like setlocale and time
functions) without adding a namespace-safe alias for fstatat.

Of course this does pull in more code that's not needed, so maybe your
version of the change is best, and maybe this is what I had in mind
when I hesitated to make the bigger change back in February. I don't
like that knowledge of different syscall alternatives leaks all over
the place, but I also don't like increasing code size unnecessarily on
all archs...

Rich

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-08  1:02               ` Rich Felker
@ 2020-09-08  7:00                 ` Arnd Bergmann
  0 siblings, 0 replies; 21+ messages in thread
From: Arnd Bergmann @ 2020-09-08  7:00 UTC (permalink / raw)
  To: musl

On Tue, Sep 8, 2020 at 3:03 AM Rich Felker <dalias@libc.org> wrote:
>
> On Tue, Sep 08, 2020 at 12:30:27AM +0200, Arnd Bergmann wrote:
> > On Tue, Sep 8, 2020 at 12:12 AM Rich Felker <dalias@libc.org> wrote:
> > > As an aside, I should probably cleanup the current definition
> > > framework where IPC_64==0x100 is the default and archs that want 0
> > > have to define it explicitly. It looks like, for the most part, IPC_64
> > > is needed iff SYS_ipc is defined.
> >
> > Right, there are no architectures that provide sys_ipc and want the
> > flag to be zero.
> >
> > > Of the archs we support, arm
> > > (32-bit) and mips{n32,64} seem to be the only ones that lack SYS_ipc
> > > but need the IPC_64 bit set. Does this agree with your assessment?
> >
> > I think microblaze is in the same group. Note that for odd reasons it
> > has always defined the __NR_ipc macro to 117 but hooked it up
> > to -ENOSYS instead of sys_ipc in the kernel. I'm never quite sure
> > whether we should treat that as a bug in the header file that we want
> > to fix, or whether we should keep such constants around in new
> > headers that were present in older ones.
>
> Oh, really? In that case musl's almost surely broken on microblaze,
> and yes it would be another exception.

There was (very briefly) a sys_ipc implementation on microblaze
in 2009 as the architecture got merged, but this was never part of
a released kernel as far as I can tell.

I'm not surprised that this was never caught though, as sysvipc
is not that common on the super-small softcore implementations
that microblaze tends to be used for.

On sparc32, sysvipc had been broken in a slightly different way
in the kernel for over 11 years without anyone complaining (it
was working in compat mode on 64-bit kernels though).

For future 64-bit microblaze, we will have to decide which ABI
to use, I'd probably go with the new variant (only split calls,
no IPC_64 flag).

        Arnd

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-08  1:54 ` Rich Felker
@ 2020-09-09  6:07   ` Rich Felker
  0 siblings, 0 replies; 21+ messages in thread
From: Rich Felker @ 2020-09-09  6:07 UTC (permalink / raw)
  To: musl

On Mon, Sep 07, 2020 at 09:54:56PM -0400, Rich Felker wrote:
> On Fri, Sep 04, 2020 at 01:48:19AM -0400, Stefan O'Rear wrote:
> > From cd57a6b47783c5302f931e543b608cb3ba58387d Mon Sep 17 00:00:00 2001
> > From: Stefan O'Rear <sorear@fastmail.com>
> > Date: Thu, 3 Sep 2020 03:59:59 -0400
> > Subject: [PATCH 06/14] Only call fstatat if defined
> > 
> > riscv32 and future architectures lack it.
> > ---
> >  src/stat/fchmodat.c   | 23 ++++++++++++++++++++---
> >  src/stat/fstatat.c    |  6 ++++++
> >  src/stdio/tempnam.c   |  9 +++++++--
> >  src/stdio/tmpnam.c    |  9 +++++++--
> >  src/time/__map_file.c | 19 +++++++++++++++----
> >  5 files changed, 55 insertions(+), 11 deletions(-)
> > 
> > diff --git a/src/stat/fchmodat.c b/src/stat/fchmodat.c
> > index 4ee00b0a..857e84e5 100644
> > --- a/src/stat/fchmodat.c
> > +++ b/src/stat/fchmodat.c
> > @@ -1,8 +1,10 @@
> >  #include <sys/stat.h>
> >  #include <fcntl.h>
> >  #include <errno.h>
> > +#include <stdint.h>
> >  #include "syscall.h"
> >  #include "kstat.h"
> > +#include "statx.h"
> >  
> >  int fchmodat(int fd, const char *path, mode_t mode, int flag)
> >  {
> > @@ -11,13 +13,22 @@ int fchmodat(int fd, const char *path, mode_t mode, int flag)
> >  	if (flag != AT_SYMLINK_NOFOLLOW)
> >  		return __syscall_ret(-EINVAL);
> >  
> > -	struct kstat st;
> >  	int ret, fd2;
> >  	char proc[15+3*sizeof(int)];
> >  
> > +#ifdef SYS_fstatat
> > +	struct kstat st;
> >  	if ((ret = __syscall(SYS_fstatat, fd, path, &st, flag)))
> >  		return __syscall_ret(ret);
> > -	if (S_ISLNK(st.st_mode))
> > +	mode_t get_mode = st.st_mode;
> > +#else
> > +	struct statx st;
> > +	if ((ret = __syscall(SYS_statx, fd, path, flag, STATX_TYPE, &st)))
> > +		return __syscall_ret(ret);
> > +	mode_t get_mode = st.stx_mode;
> > +#endif
> > +
> > +	if (S_ISLNK(get_mode))
> >  		return __syscall_ret(-EOPNOTSUPP);
> >  
> >  	if ((fd2 = __syscall(SYS_openat, fd, path, O_RDONLY|O_PATH|O_NOFOLLOW|O_NOCTTY|O_CLOEXEC)) < 0) {
> > @@ -27,9 +38,15 @@ int fchmodat(int fd, const char *path, mode_t mode, int flag)
> >  	}
> >  
> >  	__procfdname(proc, fd2);
> > +#ifdef SYS_fstatat
> >  	ret = __syscall(SYS_fstatat, AT_FDCWD, proc, &st, 0);
> > +	get_mode = st.st_mode;
> > +#else
> > +	ret = __syscall(SYS_statx, AT_FDCWD, proc, 0, STATX_TYPE, &st);
> > +	get_mode = st.stx_mode;
> > +#endif
> >  	if (!ret) {
> > -		if (S_ISLNK(st.st_mode)) ret = -EOPNOTSUPP;
> > +		if (S_ISLNK(get_mode)) ret = -EOPNOTSUPP;
> >  		else ret = __syscall(SYS_fchmodat, AT_FDCWD, proc, mode);
> >  	}
> >  
> 
> I was just looking at this file for another reason, and wondered why
> we're not just calling the fstatat function here. There's no namespace
> reason not to. According to the description of commit
> c9ebff4736128186121424364c1c62224b02aee3, use of struct kstat here was
> done "as a low-risk fix" right before release of 1.2.0, and I probably
> had in mind changing it to fstatat later and just never got around to
> it.
> 
> The same could be done for tempnam (POSIX) but not tmpnam (C) or
> __map_file (used in plain C interfaces like setlocale and time
> functions) without adding a namespace-safe alias for fstatat.
> 
> Of course this does pull in more code that's not needed, so maybe your
> version of the change is best, and maybe this is what I had in mind
> when I hesitated to make the bigger change back in February. I don't
> like that knowledge of different syscall alternatives leaks all over
> the place, but I also don't like increasing code size unnecessarily on
> all archs...

Not something that requires action, but possible streamlining I just
discovered: in places where lstat is being used to probe file
existence (tmpnam, etc.), SYS_readlink also works, yields smaller
code, and is #ifdef-free. You get the same ENOENT for nonexistence,
and success or EINVAL indicating existence as a symlink or
non-symlink, respectively.

Rich

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-04  5:48 [musl] riscv32 v2 Stefan O'Rear
                   ` (2 preceding siblings ...)
  2020-09-08  1:54 ` Rich Felker
@ 2020-09-09 20:28 ` Rich Felker
  2020-09-09 21:28   ` Palmer Dabbelt
  3 siblings, 1 reply; 21+ messages in thread
From: Rich Felker @ 2020-09-09 20:28 UTC (permalink / raw)
  To: musl

On Fri, Sep 04, 2020 at 01:48:19AM -0400, Stefan O'Rear wrote:
> Changes since v1:
> 
> Fixed ptrace support by passing through high bits of WSTOPSIG.
> WEXITSTATUS is still masked (required by POSIX); WTERMSIG is also
> masked because bits 8-15 have nowhere to go.
> 
> Added SYS_futex as an alias of SYS_futex_time64.
> 
> Changed conditionals in patch 2.  __wait4 is significantly reorganized
> and now uses a conditionally defined wrapper in src/internal/syscall.h.
> Duplication reduced in statx-using patches.
> 
> Arnd Bergmann's comment about identical fcntl.h files has NOT been
> addressed.
> 
> Rich Felker's suggestion (on IRC) to use a 0-instruction __get_tp was
> NOT implemented after discovering that it generates dramatically worse
> code on clang and cannot easily be conditionalized.  Bug reports to come.
> 
> Patches other than 2, 6, 7, 10 are unchanged.
> 
> Testing:
> 
> Smoke tested on riscv32, replacing the musl libc.so in an
> OpenEmbedded-generated VM with a dynamically linked systemd and verified
> boot.  Smoke testing on i386 and x86_64 by replacing libc.so in an
> Alpine chroot and running build tools.
> 
> libc-test was run on all three architectures.  The errors on riscv32
> are as follows:
> 
> FAIL src/api/main.exe [status 1]          
> FAIL src/functional/fcntl-static.exe [status 1]                                                                                        
> FAIL src/functional/fcntl.exe [status 1]                                                                                               
> FAIL src/functional/ipc_msg-static.exe [status 1]
> FAIL src/functional/ipc_msg.exe [status 1]                                                                                             
> FAIL src/functional/ipc_sem-static.exe [status 1]                                                                                      
> FAIL src/functional/ipc_sem.exe [status 1]
> FAIL src/functional/ipc_shm-static.exe [status 1]               
> FAIL src/functional/ipc_shm.exe [status 1]                     
> FAIL src/functional/strptime-static.exe [status 1]         
> FAIL src/functional/strptime.exe [status 1]                
> FAIL src/math/fma.exe [status 1]                         
> FAIL src/math/fmaf.exe [status 1]                 
> FAIL src/math/powf.exe [status 1]                               
> FAIL src/regression/malloc-brk-fail-static.exe [status 1]      
> FAIL src/regression/malloc-brk-fail.exe [status 1]         
> FAIL src/regression/pthread_atfork-errno-clobber-static.exe [status 1]
> FAIL src/regression/pthread_atfork-errno-clobber.exe [status 1]
> 
> The fcntl and sysvipc errors do not correspond to any error in x86_64
> and potentially require investigation, although they could be kernel
> configuration issues.  x86_64 has a different but overlapping set of
> math errors; qemu is known to not give bit-exact results for RISC-V
> floating point.  The malloc, pthread, and src/api/main.exe failures
> match failures on x86_64.
> 
> The test results are identical between master and my branch on x86_64.
> On i386, I saw a utime.exe and utime-static.exe error but have not
> managed to reproduce them.
> 
> I was not able to run LTP on musl on any of the three architectures
> following the instructions in its README.
> 
> make autotools && ./configure && make all -j16
> eventually results in:
> confstr01.c:51:3: error: '_CS_XBS5_ILP32_OFF32_CFLAGS' undeclared here (not in a function)
> 
> A cloneable repository with the present version is:
> git clone https://github.com/sorear/riscv-musl -b rv32_submit_v2

> From 020ccd0e2c77ded655bab68c2b3a0d3dc1151aab Mon Sep 17 00:00:00 2001
> From: Stefan O'Rear <sorear@fastmail.com>
> Date: Thu, 3 Sep 2020 03:17:45 -0400
> Subject: [PATCH 01/14] Remove ARMSUBARCH relic from configure

commit 0f814a4e57e80d2512934820b878211e9d71c93e removed its use.

> From d3c237f0b0f7e5d1d2a53f5382e370ce3f0c493c Mon Sep 17 00:00:00 2001
> From: Stefan O'Rear <sorear@fastmail.com>
> Date: Thu, 3 Sep 2020 03:27:03 -0400
> Subject: [PATCH 02/14] time64: Don't make aliases to nonexistent syscalls
> 
> riscv32 and future architectures lack the _time32 variants entirely, so
> don't try to use their numbers.

commit 4bbd7baea7c8538b3fb8e30f7b022a1eee071450 was written with the
intent that future time64-only archs, including riscv32, not need to
explicitly define the unadorned syscall names; the logic in
internal/syscall.h would automatically define them as the
corresponding _time64 syscall numbers. however, subsequent commits
beginning with 5a105f19b5aae79dd302899e634b6b18b3dcd0d6 broke this
when they renamed legacy time32 syscalls externally and introduced
preprocessor logic in internal/syscall.h to define the unadorned names
in terms of the renamed _time32 ones.

flip the preprocessor logic for the latter to be dependent on the
_time32 names being defined. this has the added benefit of producing a
diagnostic for redefinition if a conflicting definition ever arises.

> From f8cec3f6ff1e0a3737f1b55321e826f2208f940c Mon Sep 17 00:00:00 2001
> From: Stefan O'Rear <sorear@fastmail.com>
> Date: Thu, 3 Sep 2020 03:31:05 -0400
> Subject: [PATCH 03/14] time64: Only getrlimit/setrlimit if they exist
> 
> riscv32 and future architectures only provide prlimit64.
> ---
>  src/misc/getrlimit.c | 6 +++++-
>  src/misc/setrlimit.c | 6 +++++-
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/src/misc/getrlimit.c b/src/misc/getrlimit.c
> index 2ab2f0f4..bf676307 100644
> --- a/src/misc/getrlimit.c
> +++ b/src/misc/getrlimit.c
> @@ -6,12 +6,13 @@
>  
>  int getrlimit(int resource, struct rlimit *rlim)
>  {
> -	unsigned long k_rlim[2];
>  	int ret = syscall(SYS_prlimit64, 0, resource, 0, rlim);
>  	if (!ret) {
>  		FIX(rlim->rlim_cur);
>  		FIX(rlim->rlim_max);
>  	}
> +#ifdef SYS_getrlimit
> +	unsigned long k_rlim[2];
>  	if (!ret || errno != ENOSYS)
>  		return ret;
>  	if (syscall(SYS_getrlimit, resource, k_rlim) < 0)
> @@ -21,6 +22,9 @@ int getrlimit(int resource, struct rlimit *rlim)
>  	FIX(rlim->rlim_cur);
>  	FIX(rlim->rlim_max);
>  	return 0;
> +#else
> +	return ret;
> +#endif
>  }

No action required, but this could be improved by moving to __syscall
with return __syscall_ret(ret) at the end outside the #endif. That's
an independent change we can make later.

> From 9860fca6d45169b2c299f526243b12bff3f8180e Mon Sep 17 00:00:00 2001
> From: Stefan O'Rear <sorear@fastmail.com>
> Date: Thu, 3 Sep 2020 03:33:10 -0400
> Subject: [PATCH 04/14] time64: Only gettimeofday/settimeofday if exist
> 
> riscv64 and future architectures only provide the clock_ functions.

Commit message mentions settimeofday but it does not appear in the
diff. There's presently no fallback for settimeofday anywhere in musl,
and commit 2c2c3605d3b3ff32902c406d17ac44e7544be4e2 noted that it's
not needed (although perhaps it would be nice to have anyway?). In any
case, only action needed now is fixing the commit message.

> From daab92fbd69f7c8e3c0ff6faba142de827d007e6 Mon Sep 17 00:00:00 2001
> From: Stefan O'Rear <sorear@fastmail.com>
> Date: Thu, 3 Sep 2020 03:45:08 -0400
> Subject: [PATCH 05/14] Add src/internal/statx.h
> 
> We need to make internal syscalls to SYS_statx when SYS_fstatat is not
> available without changing the musl API.

This wording is confusing. Perhaps just "make struct statx available
for internal use outside fstatat.c."

> From 9ca6f23f7fcb6a387a394bc09a2aad1971b27857 Mon Sep 17 00:00:00 2001
> From: Stefan O'Rear <sorear@fastmail.com>
> Date: Thu, 3 Sep 2020 05:20:45 -0400
> Subject: [PATCH 07/14] Emulate wait4 using waitid
> 
> riscv32 and future architectures lack wait4.
> 
> waitpid is required by POSIX to be a cancellation point.  pclose is
> specified as undefined if a cancellation occurs, so it would be
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This is not the case. It's specified as an optional cancellation
point, not UB, but the only possible actions on cancellation are
incompatible with other requirements of POSIX and with consistency of
the program state. These essentially impose a requirement that it not
be a cancellation point, or at least that it can't act on
cancellation after the close finishes.

> permitted for it to call a cancellable wait function; however, as a
> quality of implementation matter, pclose must close the pipe fd before
> it can wait (consider popen("yes","r")) and if the wait could be
> interrupted the pipe FILE would be left in an intermediate state that
> portable software cannot recover from, so the only useful behavior is
> for pclose to NOT be a cancellation point.  We therefore support both at
> a small cost in code size.
> 
> wait4 is historically not a cancellation point in musl; we retain that
> since we need the non-cancellable version of __wait4 anyway.

With the above fixed, I don't object to keeping this kind of message,
but I'd rather focus on (or at least also have) an explanation of why
this is needed. Key points seem to be that Linux has dropped SYS_wait4
for new archs, but it's nontrivial to get the semantics needed for
functions that use waitpid in terms of waitid, and that the rusage
logic is only needed for wait4() not other functions that use
SYS_wait4, so a common place to do the conversion is required.

> ---
>  src/internal/__wait4.c | 55 ++++++++++++++++++++++++++++++++++++++++++
>  src/internal/syscall.h | 12 +++++++++
>  src/linux/wait4.c      |  2 +-
>  src/process/waitpid.c  |  2 +-
>  src/stdio/pclose.c     |  2 +-
>  src/unistd/faccessat.c |  6 ++++-
>  6 files changed, 75 insertions(+), 4 deletions(-)
>  create mode 100644 src/internal/__wait4.c
> 
> diff --git a/src/internal/__wait4.c b/src/internal/__wait4.c
> new file mode 100644
> index 00000000..04d7dc64
> --- /dev/null
> +++ b/src/internal/__wait4.c
> @@ -0,0 +1,55 @@
> +#include <sys/wait.h>
> +#include "syscall.h"
> +
> +#ifndef SYS_wait4
> +hidden pid_t __wait4(pid_t pid, int *status, int options, void *kru, int cp)

As mentioned before, I'd like to rename this to __sys_wait4 and make
macros to call it as __sys_wait4 or __sys_wait4_cp (hiding the last
argument) via internal/syscall.h (matching __sys_open pattern).

If SYS_wait4 is defined, the macros in internal/syscall.h can then
directly expand to __syscall[_cp](SYS_wait4, ...). Then the source
files don't need their own #ifdef's.

> From 3e6bd3fd86883b448fc250d96cde9d37f9efa879 Mon Sep 17 00:00:00 2001
> From: Stefan O'Rear <sorear@fastmail.com>
> Date: Thu, 3 Sep 2020 05:23:40 -0400
> Subject: [PATCH 08/14] riscv: Fall back to syscall __riscv_flush_icache
> 
> Matches glibc behavior and fixes a case where we could fall off the
> function without returning a value.

I would highlight in the commit title (first line) that this is fixing
an actual bug, the case where the vdso function isn't defined.
Something like:

    fix __riscv_flush_icache when vdso function is not available

    previously execution fell off the end of the function without
    performing any fallback or returning any value when the vdso
    function was not available.

> From 8aabc20dade2b2c6019f46a528857bb434a38167 Mon Sep 17 00:00:00 2001
> From: Stefan O'Rear <sorear@fastmail.com>
> Date: Thu, 3 Sep 2020 05:26:50 -0400
> Subject: [PATCH 09/14] riscv32: Target and subtarget detection

Having them split out has been ok for review, but I think this and the
remaining commits can be squashed for upstreaming. They don't
individually produce consistent states you could build or use.

> From aae7aeed7378f10cba709b6643acbd46f0b36213 Mon Sep 17 00:00:00 2001
> From: Stefan O'Rear <sorear@fastmail.com>
> Date: Thu, 3 Sep 2020 05:40:29 -0400
> Subject: [PATCH 10/14] riscv32: add arch headers

This is the only commit where you had significant informative message
content, and it should probably be kept but revised slightly to apply
too the whole port.

> These are mostly copied from riscv64.  _Addr and _Reg had to become int
> to avoid errors in libstdc++ when size_t and std::size_t mismatch.

This is just the psABI, not a libstdc++ issue. Almost all 32-bit archs
use int rather than long for wordsize types.

> There is no kernel stat struct; the userspace stat matches glibc in the
> sizes and offsets of all fields (including glibc's __dev_t __pad1).  The
> jump buffer is 12 words larger to account for 12 saved double-precision
> floats; additionally it should be 64-bit aligned to save doubles.

"Should be" is confusing here and suggests it's not. Maybe explain it
as the jmp_buf using 64-bit slots so that it remains sufficiently
aligned for doubles.

> The syscall list was significantly revised by deleting all time32 and
> pre-statx syscalls, and renaming several syscalls that have different
> names depending on __BITS_PER_LONG, notably mmap2 and _llseek.
> 
> futex was added as an alias to futex_time64 since it is widely used by
> software which does not pass time arguments.

OK.

> diff --git a/arch/riscv32/bits/fcntl.h b/arch/riscv32/bits/fcntl.h
> new file mode 100644
> index 00000000..ecb4d18f
> --- /dev/null
> +++ b/arch/riscv32/bits/fcntl.h
> @@ -0,0 +1,38 @@
> +#define O_CREAT        0100
> +#define O_EXCL         0200
> +#define O_NOCTTY       0400
> +#define O_TRUNC       01000
> +#define O_APPEND      02000
> +#define O_NONBLOCK    04000
> +#define O_DSYNC      010000
> +#define O_SYNC     04010000
> +#define O_RSYNC    04010000
> +#define O_DIRECTORY 0200000
> +#define O_NOFOLLOW  0400000
> +#define O_CLOEXEC  02000000
> +
> +#define O_ASYNC      020000
> +#define O_DIRECT     040000
> +#define O_LARGEFILE 0100000
> +#define O_NOATIME  01000000
> +#define O_PATH    010000000
> +#define O_TMPFILE 020200000
> +#define O_NDELAY O_NONBLOCK
> +
> +#define F_DUPFD  0
> +#define F_GETFD  1
> +#define F_SETFD  2
> +#define F_GETFL  3
> +#define F_SETFL  4
> +#define F_GETLK  5
> +#define F_SETLK  6
> +#define F_SETLKW 7
> +#define F_SETOWN 8
> +#define F_GETOWN 9
> +#define F_SETSIG 10
> +#define F_GETSIG 11
> +
> +#define F_SETOWN_EX 15
> +#define F_GETOWN_EX 16
> +
> +#define F_GETOWNER_UIDS 17

I think this file can be removed; after fixes it's identical to the
generic one.

> diff --git a/arch/riscv32/syscall_arch.h b/arch/riscv32/syscall_arch.h
> new file mode 100644
> index 00000000..9e916c76
> --- /dev/null
> +++ b/arch/riscv32/syscall_arch.h
> @@ -0,0 +1,78 @@
> +#define __SYSCALL_LL_E(x) \
> +((union { long long ll; long l[2]; }){ .ll = x }).l[0], \
> +((union { long long ll; long l[2]; }){ .ll = x }).l[1]
> +#define __SYSCALL_LL_O(x) __SYSCALL_LL_E((x))
> +
> +#define __asm_syscall(...) \
> +	__asm__ __volatile__ ("ecall\n\t" \
> +	: "=r"(a0) : __VA_ARGS__ : "memory"); \
> +	return a0; \
> +
> +static inline long __syscall0(long n)
> +{
> +	register long a7 __asm__("a7") = n;
> +	register long a0 __asm__("a0");
> +	__asm_syscall("r"(a7))
> +}
> +
> +static inline long __syscall1(long n, long a)
> +{
> +	register long a7 __asm__("a7") = n;
> +	register long a0 __asm__("a0") = a;
> +	__asm_syscall("r"(a7), "0"(a0))
> +}
> +
> +static inline long __syscall2(long n, long a, long b)
> +{
> +	register long a7 __asm__("a7") = n;
> +	register long a0 __asm__("a0") = a;
> +	register long a1 __asm__("a1") = b;
> +	__asm_syscall("r"(a7), "0"(a0), "r"(a1))
> +}
> +
> +static inline long __syscall3(long n, long a, long b, long c)
> +{
> +	register long a7 __asm__("a7") = n;
> +	register long a0 __asm__("a0") = a;
> +	register long a1 __asm__("a1") = b;
> +	register long a2 __asm__("a2") = c;
> +	__asm_syscall("r"(a7), "0"(a0), "r"(a1), "r"(a2))
> +}
> +
> +static inline long __syscall4(long n, long a, long b, long c, long d)
> +{
> +	register long a7 __asm__("a7") = n;
> +	register long a0 __asm__("a0") = a;
> +	register long a1 __asm__("a1") = b;
> +	register long a2 __asm__("a2") = c;
> +	register long a3 __asm__("a3") = d;
> +	__asm_syscall("r"(a7), "0"(a0), "r"(a1), "r"(a2), "r"(a3))
> +}
> +
> +static inline long __syscall5(long n, long a, long b, long c, long d, long e)
> +{
> +	register long a7 __asm__("a7") = n;
> +	register long a0 __asm__("a0") = a;
> +	register long a1 __asm__("a1") = b;
> +	register long a2 __asm__("a2") = c;
> +	register long a3 __asm__("a3") = d;
> +	register long a4 __asm__("a4") = e;
> +	__asm_syscall("r"(a7), "0"(a0), "r"(a1), "r"(a2), "r"(a3), "r"(a4))
> +}
> +
> +static inline long __syscall6(long n, long a, long b, long c, long d, long e, long f)
> +{
> +	register long a7 __asm__("a7") = n;
> +	register long a0 __asm__("a0") = a;
> +	register long a1 __asm__("a1") = b;
> +	register long a2 __asm__("a2") = c;
> +	register long a3 __asm__("a3") = d;
> +	register long a4 __asm__("a4") = e;
> +	register long a5 __asm__("a5") = f;
> +	__asm_syscall("r"(a7), "0"(a0), "r"(a1), "r"(a2), "r"(a3), "r"(a4), "r"(a5))
> +}
> +
> +#define VDSO_USEFUL
> +/* We don't have a clock_gettime function.
> +#define VDSO_CGT_SYM "__vdso_clock_gettime"
> +#define VDSO_CGT_VER "LINUX_2.6" */
> -- 
> 2.25.4
> 

Is this correct? I see the comment is just copied from riscv64, but it
seems wrong there, and here too. Also, is the vdso function named
"clock_gettime" or "clock_gettime64" for riscv32? Or is there none at
all and this macro just wrong?

Rich

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-09 20:28 ` Rich Felker
@ 2020-09-09 21:28   ` Palmer Dabbelt
  2020-09-09 21:36     ` Rich Felker
  0 siblings, 1 reply; 21+ messages in thread
From: Palmer Dabbelt @ 2020-09-09 21:28 UTC (permalink / raw)
  To: dalias; +Cc: musl

On Wed, 09 Sep 2020 13:28:27 PDT (-0700), dalias@libc.org wrote:
> On Fri, Sep 04, 2020 at 01:48:19AM -0400, Stefan O'Rear wrote:
>> Changes since v1:
>>
>> Fixed ptrace support by passing through high bits of WSTOPSIG.
>> WEXITSTATUS is still masked (required by POSIX); WTERMSIG is also
>> masked because bits 8-15 have nowhere to go.
>>
>> Added SYS_futex as an alias of SYS_futex_time64.
>>
>> Changed conditionals in patch 2.  __wait4 is significantly reorganized
>> and now uses a conditionally defined wrapper in src/internal/syscall.h.
>> Duplication reduced in statx-using patches.
>>
>> Arnd Bergmann's comment about identical fcntl.h files has NOT been
>> addressed.
>>
>> Rich Felker's suggestion (on IRC) to use a 0-instruction __get_tp was
>> NOT implemented after discovering that it generates dramatically worse
>> code on clang and cannot easily be conditionalized.  Bug reports to come.
>>
>> Patches other than 2, 6, 7, 10 are unchanged.
>>
>> Testing:
>>
>> Smoke tested on riscv32, replacing the musl libc.so in an
>> OpenEmbedded-generated VM with a dynamically linked systemd and verified
>> boot.  Smoke testing on i386 and x86_64 by replacing libc.so in an
>> Alpine chroot and running build tools.
>>
>> libc-test was run on all three architectures.  The errors on riscv32
>> are as follows:
>>
>> FAIL src/api/main.exe [status 1]
>> FAIL src/functional/fcntl-static.exe [status 1]
>> FAIL src/functional/fcntl.exe [status 1]
>> FAIL src/functional/ipc_msg-static.exe [status 1]
>> FAIL src/functional/ipc_msg.exe [status 1]
>> FAIL src/functional/ipc_sem-static.exe [status 1]
>> FAIL src/functional/ipc_sem.exe [status 1]
>> FAIL src/functional/ipc_shm-static.exe [status 1]
>> FAIL src/functional/ipc_shm.exe [status 1]
>> FAIL src/functional/strptime-static.exe [status 1]
>> FAIL src/functional/strptime.exe [status 1]
>> FAIL src/math/fma.exe [status 1]
>> FAIL src/math/fmaf.exe [status 1]
>> FAIL src/math/powf.exe [status 1]
>> FAIL src/regression/malloc-brk-fail-static.exe [status 1]
>> FAIL src/regression/malloc-brk-fail.exe [status 1]
>> FAIL src/regression/pthread_atfork-errno-clobber-static.exe [status 1]
>> FAIL src/regression/pthread_atfork-errno-clobber.exe [status 1]
>>
>> The fcntl and sysvipc errors do not correspond to any error in x86_64
>> and potentially require investigation, although they could be kernel
>> configuration issues.  x86_64 has a different but overlapping set of
>> math errors; qemu is known to not give bit-exact results for RISC-V
>> floating point.  The malloc, pthread, and src/api/main.exe failures
>> match failures on x86_64.
>>
>> The test results are identical between master and my branch on x86_64.
>> On i386, I saw a utime.exe and utime-static.exe error but have not
>> managed to reproduce them.
>>
>> I was not able to run LTP on musl on any of the three architectures
>> following the instructions in its README.
>>
>> make autotools && ./configure && make all -j16
>> eventually results in:
>> confstr01.c:51:3: error: '_CS_XBS5_ILP32_OFF32_CFLAGS' undeclared here (not in a function)
>>
>> A cloneable repository with the present version is:
>> git clone https://github.com/sorear/riscv-musl -b rv32_submit_v2
>
>> From 020ccd0e2c77ded655bab68c2b3a0d3dc1151aab Mon Sep 17 00:00:00 2001
>> From: Stefan O'Rear <sorear@fastmail.com>
>> Date: Thu, 3 Sep 2020 03:17:45 -0400
>> Subject: [PATCH 01/14] Remove ARMSUBARCH relic from configure
>
> commit 0f814a4e57e80d2512934820b878211e9d71c93e removed its use.
>
>> From d3c237f0b0f7e5d1d2a53f5382e370ce3f0c493c Mon Sep 17 00:00:00 2001
>> From: Stefan O'Rear <sorear@fastmail.com>
>> Date: Thu, 3 Sep 2020 03:27:03 -0400
>> Subject: [PATCH 02/14] time64: Don't make aliases to nonexistent syscalls
>>
>> riscv32 and future architectures lack the _time32 variants entirely, so
>> don't try to use their numbers.
>
> commit 4bbd7baea7c8538b3fb8e30f7b022a1eee071450 was written with the
> intent that future time64-only archs, including riscv32, not need to
> explicitly define the unadorned syscall names; the logic in
> internal/syscall.h would automatically define them as the
> corresponding _time64 syscall numbers. however, subsequent commits
> beginning with 5a105f19b5aae79dd302899e634b6b18b3dcd0d6 broke this
> when they renamed legacy time32 syscalls externally and introduced
> preprocessor logic in internal/syscall.h to define the unadorned names
> in terms of the renamed _time32 ones.
>
> flip the preprocessor logic for the latter to be dependent on the
> _time32 names being defined. this has the added benefit of producing a
> diagnostic for redefinition if a conflicting definition ever arises.
>
>> From f8cec3f6ff1e0a3737f1b55321e826f2208f940c Mon Sep 17 00:00:00 2001
>> From: Stefan O'Rear <sorear@fastmail.com>
>> Date: Thu, 3 Sep 2020 03:31:05 -0400
>> Subject: [PATCH 03/14] time64: Only getrlimit/setrlimit if they exist
>>
>> riscv32 and future architectures only provide prlimit64.
>> ---
>>  src/misc/getrlimit.c | 6 +++++-
>>  src/misc/setrlimit.c | 6 +++++-
>>  2 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/misc/getrlimit.c b/src/misc/getrlimit.c
>> index 2ab2f0f4..bf676307 100644
>> --- a/src/misc/getrlimit.c
>> +++ b/src/misc/getrlimit.c
>> @@ -6,12 +6,13 @@
>>
>>  int getrlimit(int resource, struct rlimit *rlim)
>>  {
>> -	unsigned long k_rlim[2];
>>  	int ret = syscall(SYS_prlimit64, 0, resource, 0, rlim);
>>  	if (!ret) {
>>  		FIX(rlim->rlim_cur);
>>  		FIX(rlim->rlim_max);
>>  	}
>> +#ifdef SYS_getrlimit
>> +	unsigned long k_rlim[2];
>>  	if (!ret || errno != ENOSYS)
>>  		return ret;
>>  	if (syscall(SYS_getrlimit, resource, k_rlim) < 0)
>> @@ -21,6 +22,9 @@ int getrlimit(int resource, struct rlimit *rlim)
>>  	FIX(rlim->rlim_cur);
>>  	FIX(rlim->rlim_max);
>>  	return 0;
>> +#else
>> +	return ret;
>> +#endif
>>  }
>
> No action required, but this could be improved by moving to __syscall
> with return __syscall_ret(ret) at the end outside the #endif. That's
> an independent change we can make later.
>
>> From 9860fca6d45169b2c299f526243b12bff3f8180e Mon Sep 17 00:00:00 2001
>> From: Stefan O'Rear <sorear@fastmail.com>
>> Date: Thu, 3 Sep 2020 03:33:10 -0400
>> Subject: [PATCH 04/14] time64: Only gettimeofday/settimeofday if exist
>>
>> riscv64 and future architectures only provide the clock_ functions.
>
> Commit message mentions settimeofday but it does not appear in the
> diff. There's presently no fallback for settimeofday anywhere in musl,
> and commit 2c2c3605d3b3ff32902c406d17ac44e7544be4e2 noted that it's
> not needed (although perhaps it would be nice to have anyway?). In any
> case, only action needed now is fixing the commit message.
>
>> From daab92fbd69f7c8e3c0ff6faba142de827d007e6 Mon Sep 17 00:00:00 2001
>> From: Stefan O'Rear <sorear@fastmail.com>
>> Date: Thu, 3 Sep 2020 03:45:08 -0400
>> Subject: [PATCH 05/14] Add src/internal/statx.h
>>
>> We need to make internal syscalls to SYS_statx when SYS_fstatat is not
>> available without changing the musl API.
>
> This wording is confusing. Perhaps just "make struct statx available
> for internal use outside fstatat.c."
>
>> From 9ca6f23f7fcb6a387a394bc09a2aad1971b27857 Mon Sep 17 00:00:00 2001
>> From: Stefan O'Rear <sorear@fastmail.com>
>> Date: Thu, 3 Sep 2020 05:20:45 -0400
>> Subject: [PATCH 07/14] Emulate wait4 using waitid
>>
>> riscv32 and future architectures lack wait4.
>>
>> waitpid is required by POSIX to be a cancellation point.  pclose is
>> specified as undefined if a cancellation occurs, so it would be
>   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> This is not the case. It's specified as an optional cancellation
> point, not UB, but the only possible actions on cancellation are
> incompatible with other requirements of POSIX and with consistency of
> the program state. These essentially impose a requirement that it not
> be a cancellation point, or at least that it can't act on
> cancellation after the close finishes.
>
>> permitted for it to call a cancellable wait function; however, as a
>> quality of implementation matter, pclose must close the pipe fd before
>> it can wait (consider popen("yes","r")) and if the wait could be
>> interrupted the pipe FILE would be left in an intermediate state that
>> portable software cannot recover from, so the only useful behavior is
>> for pclose to NOT be a cancellation point.  We therefore support both at
>> a small cost in code size.
>>
>> wait4 is historically not a cancellation point in musl; we retain that
>> since we need the non-cancellable version of __wait4 anyway.
>
> With the above fixed, I don't object to keeping this kind of message,
> but I'd rather focus on (or at least also have) an explanation of why
> this is needed. Key points seem to be that Linux has dropped SYS_wait4
> for new archs, but it's nontrivial to get the semantics needed for
> functions that use waitpid in terms of waitid, and that the rusage
> logic is only needed for wait4() not other functions that use
> SYS_wait4, so a common place to do the conversion is required.
>
>> ---
>>  src/internal/__wait4.c | 55 ++++++++++++++++++++++++++++++++++++++++++
>>  src/internal/syscall.h | 12 +++++++++
>>  src/linux/wait4.c      |  2 +-
>>  src/process/waitpid.c  |  2 +-
>>  src/stdio/pclose.c     |  2 +-
>>  src/unistd/faccessat.c |  6 ++++-
>>  6 files changed, 75 insertions(+), 4 deletions(-)
>>  create mode 100644 src/internal/__wait4.c
>>
>> diff --git a/src/internal/__wait4.c b/src/internal/__wait4.c
>> new file mode 100644
>> index 00000000..04d7dc64
>> --- /dev/null
>> +++ b/src/internal/__wait4.c
>> @@ -0,0 +1,55 @@
>> +#include <sys/wait.h>
>> +#include "syscall.h"
>> +
>> +#ifndef SYS_wait4
>> +hidden pid_t __wait4(pid_t pid, int *status, int options, void *kru, int cp)
>
> As mentioned before, I'd like to rename this to __sys_wait4 and make
> macros to call it as __sys_wait4 or __sys_wait4_cp (hiding the last
> argument) via internal/syscall.h (matching __sys_open pattern).
>
> If SYS_wait4 is defined, the macros in internal/syscall.h can then
> directly expand to __syscall[_cp](SYS_wait4, ...). Then the source
> files don't need their own #ifdef's.
>
>> From 3e6bd3fd86883b448fc250d96cde9d37f9efa879 Mon Sep 17 00:00:00 2001
>> From: Stefan O'Rear <sorear@fastmail.com>
>> Date: Thu, 3 Sep 2020 05:23:40 -0400
>> Subject: [PATCH 08/14] riscv: Fall back to syscall __riscv_flush_icache
>>
>> Matches glibc behavior and fixes a case where we could fall off the
>> function without returning a value.
>
> I would highlight in the commit title (first line) that this is fixing
> an actual bug, the case where the vdso function isn't defined.
> Something like:
>
>     fix __riscv_flush_icache when vdso function is not available
>
>     previously execution fell off the end of the function without
>     performing any fallback or returning any value when the vdso
>     function was not available.
>
>> From 8aabc20dade2b2c6019f46a528857bb434a38167 Mon Sep 17 00:00:00 2001
>> From: Stefan O'Rear <sorear@fastmail.com>
>> Date: Thu, 3 Sep 2020 05:26:50 -0400
>> Subject: [PATCH 09/14] riscv32: Target and subtarget detection
>
> Having them split out has been ok for review, but I think this and the
> remaining commits can be squashed for upstreaming. They don't
> individually produce consistent states you could build or use.
>
>> From aae7aeed7378f10cba709b6643acbd46f0b36213 Mon Sep 17 00:00:00 2001
>> From: Stefan O'Rear <sorear@fastmail.com>
>> Date: Thu, 3 Sep 2020 05:40:29 -0400
>> Subject: [PATCH 10/14] riscv32: add arch headers
>
> This is the only commit where you had significant informative message
> content, and it should probably be kept but revised slightly to apply
> too the whole port.
>
>> These are mostly copied from riscv64.  _Addr and _Reg had to become int
>> to avoid errors in libstdc++ when size_t and std::size_t mismatch.
>
> This is just the psABI, not a libstdc++ issue. Almost all 32-bit archs
> use int rather than long for wordsize types.
>
>> There is no kernel stat struct; the userspace stat matches glibc in the
>> sizes and offsets of all fields (including glibc's __dev_t __pad1).  The
>> jump buffer is 12 words larger to account for 12 saved double-precision
>> floats; additionally it should be 64-bit aligned to save doubles.
>
> "Should be" is confusing here and suggests it's not. Maybe explain it
> as the jmp_buf using 64-bit slots so that it remains sufficiently
> aligned for doubles.
>
>> The syscall list was significantly revised by deleting all time32 and
>> pre-statx syscalls, and renaming several syscalls that have different
>> names depending on __BITS_PER_LONG, notably mmap2 and _llseek.
>>
>> futex was added as an alias to futex_time64 since it is widely used by
>> software which does not pass time arguments.
>
> OK.
>
>> diff --git a/arch/riscv32/bits/fcntl.h b/arch/riscv32/bits/fcntl.h
>> new file mode 100644
>> index 00000000..ecb4d18f
>> --- /dev/null
>> +++ b/arch/riscv32/bits/fcntl.h
>> @@ -0,0 +1,38 @@
>> +#define O_CREAT        0100
>> +#define O_EXCL         0200
>> +#define O_NOCTTY       0400
>> +#define O_TRUNC       01000
>> +#define O_APPEND      02000
>> +#define O_NONBLOCK    04000
>> +#define O_DSYNC      010000
>> +#define O_SYNC     04010000
>> +#define O_RSYNC    04010000
>> +#define O_DIRECTORY 0200000
>> +#define O_NOFOLLOW  0400000
>> +#define O_CLOEXEC  02000000
>> +
>> +#define O_ASYNC      020000
>> +#define O_DIRECT     040000
>> +#define O_LARGEFILE 0100000
>> +#define O_NOATIME  01000000
>> +#define O_PATH    010000000
>> +#define O_TMPFILE 020200000
>> +#define O_NDELAY O_NONBLOCK
>> +
>> +#define F_DUPFD  0
>> +#define F_GETFD  1
>> +#define F_SETFD  2
>> +#define F_GETFL  3
>> +#define F_SETFL  4
>> +#define F_GETLK  5
>> +#define F_SETLK  6
>> +#define F_SETLKW 7
>> +#define F_SETOWN 8
>> +#define F_GETOWN 9
>> +#define F_SETSIG 10
>> +#define F_GETSIG 11
>> +
>> +#define F_SETOWN_EX 15
>> +#define F_GETOWN_EX 16
>> +
>> +#define F_GETOWNER_UIDS 17
>
> I think this file can be removed; after fixes it's identical to the
> generic one.
>
>> diff --git a/arch/riscv32/syscall_arch.h b/arch/riscv32/syscall_arch.h
>> new file mode 100644
>> index 00000000..9e916c76
>> --- /dev/null
>> +++ b/arch/riscv32/syscall_arch.h
>> @@ -0,0 +1,78 @@
>> +#define __SYSCALL_LL_E(x) \
>> +((union { long long ll; long l[2]; }){ .ll = x }).l[0], \
>> +((union { long long ll; long l[2]; }){ .ll = x }).l[1]
>> +#define __SYSCALL_LL_O(x) __SYSCALL_LL_E((x))
>> +
>> +#define __asm_syscall(...) \
>> +	__asm__ __volatile__ ("ecall\n\t" \
>> +	: "=r"(a0) : __VA_ARGS__ : "memory"); \
>> +	return a0; \
>> +
>> +static inline long __syscall0(long n)
>> +{
>> +	register long a7 __asm__("a7") = n;
>> +	register long a0 __asm__("a0");
>> +	__asm_syscall("r"(a7))
>> +}
>> +
>> +static inline long __syscall1(long n, long a)
>> +{
>> +	register long a7 __asm__("a7") = n;
>> +	register long a0 __asm__("a0") = a;
>> +	__asm_syscall("r"(a7), "0"(a0))
>> +}
>> +
>> +static inline long __syscall2(long n, long a, long b)
>> +{
>> +	register long a7 __asm__("a7") = n;
>> +	register long a0 __asm__("a0") = a;
>> +	register long a1 __asm__("a1") = b;
>> +	__asm_syscall("r"(a7), "0"(a0), "r"(a1))
>> +}
>> +
>> +static inline long __syscall3(long n, long a, long b, long c)
>> +{
>> +	register long a7 __asm__("a7") = n;
>> +	register long a0 __asm__("a0") = a;
>> +	register long a1 __asm__("a1") = b;
>> +	register long a2 __asm__("a2") = c;
>> +	__asm_syscall("r"(a7), "0"(a0), "r"(a1), "r"(a2))
>> +}
>> +
>> +static inline long __syscall4(long n, long a, long b, long c, long d)
>> +{
>> +	register long a7 __asm__("a7") = n;
>> +	register long a0 __asm__("a0") = a;
>> +	register long a1 __asm__("a1") = b;
>> +	register long a2 __asm__("a2") = c;
>> +	register long a3 __asm__("a3") = d;
>> +	__asm_syscall("r"(a7), "0"(a0), "r"(a1), "r"(a2), "r"(a3))
>> +}
>> +
>> +static inline long __syscall5(long n, long a, long b, long c, long d, long e)
>> +{
>> +	register long a7 __asm__("a7") = n;
>> +	register long a0 __asm__("a0") = a;
>> +	register long a1 __asm__("a1") = b;
>> +	register long a2 __asm__("a2") = c;
>> +	register long a3 __asm__("a3") = d;
>> +	register long a4 __asm__("a4") = e;
>> +	__asm_syscall("r"(a7), "0"(a0), "r"(a1), "r"(a2), "r"(a3), "r"(a4))
>> +}
>> +
>> +static inline long __syscall6(long n, long a, long b, long c, long d, long e, long f)
>> +{
>> +	register long a7 __asm__("a7") = n;
>> +	register long a0 __asm__("a0") = a;
>> +	register long a1 __asm__("a1") = b;
>> +	register long a2 __asm__("a2") = c;
>> +	register long a3 __asm__("a3") = d;
>> +	register long a4 __asm__("a4") = e;
>> +	register long a5 __asm__("a5") = f;
>> +	__asm_syscall("r"(a7), "0"(a0), "r"(a1), "r"(a2), "r"(a3), "r"(a4), "r"(a5))
>> +}
>> +
>> +#define VDSO_USEFUL
>> +/* We don't have a clock_gettime function.
>> +#define VDSO_CGT_SYM "__vdso_clock_gettime"
>> +#define VDSO_CGT_VER "LINUX_2.6" */
>> --
>> 2.25.4
>>
>
> Is this correct? I see the comment is just copied from riscv64, but it
> seems wrong there, and here too. Also, is the vdso function named
> "clock_gettime" or "clock_gettime64" for riscv32? Or is there none at
> all and this macro just wrong?

Looks like we don't have __vdso_clock_gettime on rv32 but we do have one on
rv64.  glibc doesn't have the clock VDSO calls on rv32.

I'm not opposed to adding some sort of clock-related VDSO calls on rv32, but it
looks like doing so will require some thought.  Maybe it's best to wait on that
so we don't hold up the initial port?

>
> Rich

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-09 21:28   ` Palmer Dabbelt
@ 2020-09-09 21:36     ` Rich Felker
  2020-09-09 23:08       ` Palmer Dabbelt
  0 siblings, 1 reply; 21+ messages in thread
From: Rich Felker @ 2020-09-09 21:36 UTC (permalink / raw)
  To: Palmer Dabbelt; +Cc: musl

On Wed, Sep 09, 2020 at 02:28:55PM -0700, Palmer Dabbelt wrote:
> On Wed, 09 Sep 2020 13:28:27 PDT (-0700), dalias@libc.org wrote:
> >On Fri, Sep 04, 2020 at 01:48:19AM -0400, Stefan O'Rear wrote:
> >>+#define VDSO_USEFUL
> >>+/* We don't have a clock_gettime function.
> >>+#define VDSO_CGT_SYM "__vdso_clock_gettime"
> >>+#define VDSO_CGT_VER "LINUX_2.6" */
> >>--
> >>2.25.4
> >>
> >
> >Is this correct? I see the comment is just copied from riscv64, but it
> >seems wrong there, and here too. Also, is the vdso function named
> >"clock_gettime" or "clock_gettime64" for riscv32? Or is there none at
> >all and this macro just wrong?
> 
> Looks like we don't have __vdso_clock_gettime on rv32 but we do have one on
> rv64.  glibc doesn't have the clock VDSO calls on rv32.
> 
> I'm not opposed to adding some sort of clock-related VDSO calls on rv32, but it
> looks like doing so will require some thought.  Maybe it's best to wait on that
> so we don't hold up the initial port?

Possible addition of vdso clock_gettime isn't a blocker for moving
forward with the musl port, but syscall_arch.h should accurately
describe what's available and should not attempt to use vdso before
it's a public kernel interface (e.g. resolving the question of what
the function name will be). So I think it should be removed for now.
But VDSO_USEFUL must be kept if we want to support the vdso icache
flush function (is that actually supported on rv32 either? if not it
should be made conditional on rv64 in src/linux/cache.c.

Rich

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-09 21:36     ` Rich Felker
@ 2020-09-09 23:08       ` Palmer Dabbelt
  2020-09-10  7:36         ` Arnd Bergmann
  0 siblings, 1 reply; 21+ messages in thread
From: Palmer Dabbelt @ 2020-09-09 23:08 UTC (permalink / raw)
  To: dalias, Arnd Bergmann; +Cc: musl

On Wed, 09 Sep 2020 14:36:44 PDT (-0700), dalias@libc.org wrote:
> On Wed, Sep 09, 2020 at 02:28:55PM -0700, Palmer Dabbelt wrote:
>> On Wed, 09 Sep 2020 13:28:27 PDT (-0700), dalias@libc.org wrote:
>> >On Fri, Sep 04, 2020 at 01:48:19AM -0400, Stefan O'Rear wrote:
>> >>+#define VDSO_USEFUL
>> >>+/* We don't have a clock_gettime function.
>> >>+#define VDSO_CGT_SYM "__vdso_clock_gettime"
>> >>+#define VDSO_CGT_VER "LINUX_2.6" */
>> >>--
>> >>2.25.4
>> >>
>> >
>> >Is this correct? I see the comment is just copied from riscv64, but it
>> >seems wrong there, and here too. Also, is the vdso function named
>> >"clock_gettime" or "clock_gettime64" for riscv32? Or is there none at
>> >all and this macro just wrong?
>>
>> Looks like we don't have __vdso_clock_gettime on rv32 but we do have one on
>> rv64.  glibc doesn't have the clock VDSO calls on rv32.
>>
>> I'm not opposed to adding some sort of clock-related VDSO calls on rv32, but it
>> looks like doing so will require some thought.  Maybe it's best to wait on that
>> so we don't hold up the initial port?
>
> Possible addition of vdso clock_gettime isn't a blocker for moving
> forward with the musl port, but syscall_arch.h should accurately
> describe what's available and should not attempt to use vdso before
> it's a public kernel interface (e.g. resolving the question of what
> the function name will be). So I think it should be removed for now.

Sorry if that was confusing, but I definitely agree.

I guess my point was that the lack of VDSO clock functions on rv32 was probably
an oversight, but one that shouldn't block the port.  We definitely can't just
make up a kernel interface, particularly as the reason we don't have these on
rv32 is because the generic versions of the functions we're using don't appear
to run on 32-bit targets.

That probably means there's some more subtle issue, though TBH I don't know
enough about the 64-bit-ification of time_t for it to just jump out at me.  I
don't want to derail the thread too much, but I tried the obvious thing

    diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
    index 33dde87218dd..1cf24a8f76c4 100644
    --- a/arch/riscv/Kconfig
    +++ b/arch/riscv/Kconfig
    @@ -65,7 +65,7 @@ config RISCV
     	select HAVE_EBPF_JIT if MMU
     	select HAVE_FUTEX_CMPXCHG if FUTEX
     	select HAVE_GCC_PLUGINS
    -	select HAVE_GENERIC_VDSO if MMU && 64BIT
    +	select HAVE_GENERIC_VDSO if MMU
     	select HAVE_PCI
     	select HAVE_PERF_EVENTS
     	select HAVE_PERF_REGS
    diff --git a/arch/riscv/kernel/vdso/Makefile b/arch/riscv/kernel/vdso/Makefile
    index 478e7338ddc1..10f7a07ce85a 100644
    --- a/arch/riscv/kernel/vdso/Makefile
    +++ b/arch/riscv/kernel/vdso/Makefile
    @@ -7,9 +7,7 @@ ARCH_REL_TYPE_ABS := R_RISCV_32|R_RISCV_64|R_RISCV_JUMP_SLOT
     include $(srctree)/lib/vdso/Makefile
     # Symbols present in the vdso
     vdso-syms  = rt_sigreturn
    -ifdef CONFIG_64BIT
     vdso-syms += vgettimeofday
    -endif
     vdso-syms += getcpu
     vdso-syms += flush_icache

and it doesn't build.  I've added Arnd, who might have a better idea of what's
going on.  Whatever happens, I think the best bet is to just drop the clock
functions (specifically __vdso_{clock_gettime,gettimeofday,clock_getres}) from
the rv32 port right now.

> But VDSO_USEFUL must be kept if we want to support the vdso icache
> flush function (is that actually supported on rv32 either? if not it
> should be made conditional on rv64 in src/linux/cache.c.

Yes, we have __vdso_flush_icache on rv32 and as far as I know it should work
fine (I guess QEMU isn't really going to find fence.i issues, but this
interface in particular is quite simple).  There's no way to build a working
system without some kernel-based fence.i mechanism, and IIRC we added the VDSO
entry at the same time as the syscall so it should always work itself out
(though we do have the syscall-based fallback in glibc)).  One of my working
directories reports

    $ riscv64-linux-gnu-objdump -d arch/riscv/kernel/vdso/vdso.so
    
    arch/riscv/kernel/vdso/vdso.so:     file format elf32-littleriscv
    
    
    Disassembly of section .text:
    
    00000800 <__vdso_rt_sigreturn@@LINUX_4.15>:
     800:	08b00893          	li	a7,139
     804:	00000073          	ecall
     808:	0000                	unimp
    	...
    
    0000080c <__vdso_getcpu@@LINUX_4.15>:
     80c:	0a800893          	li	a7,168
     810:	00000073          	ecall
     814:	8082                	ret
    	...
    
    00000818 <__vdso_flush_icache@@LINUX_4.15>:
     818:	10300893          	li	a7,259
     81c:	00000073          	ecall
     820:	8082                	ret

when built for rv32 (despite the rv64 objdump command).

>
> Rich

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-09 23:08       ` Palmer Dabbelt
@ 2020-09-10  7:36         ` Arnd Bergmann
  2020-09-10 10:01           ` Vincenzo Frascino
  0 siblings, 1 reply; 21+ messages in thread
From: Arnd Bergmann @ 2020-09-10  7:36 UTC (permalink / raw)
  To: Palmer Dabbelt; +Cc: Rich Felker, musl, Vincenzo Frascino

On Thu, Sep 10, 2020 at 1:08 AM Palmer Dabbelt <palmerdabbelt@google.com> wrote:
> On Wed, 09 Sep 2020 14:36:44 PDT (-0700), dalias@libc.org wrote:
> > On Wed, Sep 09, 2020 at 02:28:55PM -0700, Palmer Dabbelt wrote:
> >> On Wed, 09 Sep 2020 13:28:27 PDT (-0700), dalias@libc.org wrote:
> > Possible addition of vdso clock_gettime isn't a blocker for moving
> > forward with the musl port, but syscall_arch.h should accurately
> > describe what's available and should not attempt to use vdso before
> > it's a public kernel interface (e.g. resolving the question of what
> > the function name will be). So I think it should be removed for now.
>
> Sorry if that was confusing, but I definitely agree.
>
> I guess my point was that the lack of VDSO clock functions on rv32 was probably
> an oversight, but one that shouldn't block the port.  We definitely can't just
> make up a kernel interface, particularly as the reason we don't have these on
> rv32 is because the generic versions of the functions we're using don't appear
> to run on 32-bit targets.
>
> That probably means there's some more subtle issue, though TBH I don't know
> enough about the 64-bit-ification of time_t for it to just jump out at me.  I
> don't want to derail the thread too much, but I tried the obvious thing

When the vdso for rv64 was added, there was no time64 support in the
vdso code in general, as this only came with the "generic vdso" infrastructure
that was added later on, with commit ad5d1122b82f ("riscv: use vDSO
common flow to reduce the latency of the time-related functions") in v5.8.

At that point it probably should have been added as well.

>     --- a/arch/riscv/kernel/vdso/Makefile
>     +++ b/arch/riscv/kernel/vdso/Makefile
>     @@ -7,9 +7,7 @@ ARCH_REL_TYPE_ABS := R_RISCV_32|R_RISCV_64|R_RISCV_JUMP_SLOT
>      include $(srctree)/lib/vdso/Makefile
>      # Symbols present in the vdso
>      vdso-syms  = rt_sigreturn
>     -ifdef CONFIG_64BIT
>      vdso-syms += vgettimeofday
>     -endif
>      vdso-syms += getcpu
>      vdso-syms += flush_icache
>
> and it doesn't build.  I've added Arnd, who might have a better idea of what's
> going on.  Whatever happens, I think the best bet is to just drop the clock
> functions (specifically __vdso_{clock_gettime,gettimeofday,clock_getres}) from
> the rv32 port right now.

For rv32 you need clock_gettime64, not clock_gettime, which in the Linux
ABI refers to the interface with the old timespec. There was some debate
over whether clock_getres_time64 and gettimeofday_time64 would make
sense to be added here, but I have so far leaned to the position that these
are not as performance critical and not worth the effort.

Vincenzo has argued that we might want to extend the generic vdso code
to include a number of additional syscall implementations, which would
then include gettimeofday_time64 and clock_getres_time64.

        Arnd

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-10  7:36         ` Arnd Bergmann
@ 2020-09-10 10:01           ` Vincenzo Frascino
  2020-09-11  0:08             ` Palmer Dabbelt
  0 siblings, 1 reply; 21+ messages in thread
From: Vincenzo Frascino @ 2020-09-10 10:01 UTC (permalink / raw)
  To: Arnd Bergmann, Palmer Dabbelt; +Cc: Rich Felker, musl



On 9/10/20 8:36 AM, Arnd Bergmann wrote:
> On Thu, Sep 10, 2020 at 1:08 AM Palmer Dabbelt <palmerdabbelt@google.com> wrote:
>> On Wed, 09 Sep 2020 14:36:44 PDT (-0700), dalias@libc.org wrote:
>>> On Wed, Sep 09, 2020 at 02:28:55PM -0700, Palmer Dabbelt wrote:
>>>> On Wed, 09 Sep 2020 13:28:27 PDT (-0700), dalias@libc.org wrote:
>>> Possible addition of vdso clock_gettime isn't a blocker for moving
>>> forward with the musl port, but syscall_arch.h should accurately
>>> describe what's available and should not attempt to use vdso before
>>> it's a public kernel interface (e.g. resolving the question of what
>>> the function name will be). So I think it should be removed for now.
>>
>> Sorry if that was confusing, but I definitely agree.
>>
>> I guess my point was that the lack of VDSO clock functions on rv32 was probably
>> an oversight, but one that shouldn't block the port.  We definitely can't just
>> make up a kernel interface, particularly as the reason we don't have these on
>> rv32 is because the generic versions of the functions we're using don't appear
>> to run on 32-bit targets.
>>
>> That probably means there's some more subtle issue, though TBH I don't know
>> enough about the 64-bit-ification of time_t for it to just jump out at me.  I
>> don't want to derail the thread too much, but I tried the obvious thing
> 
> When the vdso for rv64 was added, there was no time64 support in the
> vdso code in general, as this only came with the "generic vdso" infrastructure
> that was added later on, with commit ad5d1122b82f ("riscv: use vDSO
> common flow to reduce the latency of the time-related functions") in v5.8.
> 
> At that point it probably should have been added as well.
> 
>>     --- a/arch/riscv/kernel/vdso/Makefile
>>     +++ b/arch/riscv/kernel/vdso/Makefile
>>     @@ -7,9 +7,7 @@ ARCH_REL_TYPE_ABS := R_RISCV_32|R_RISCV_64|R_RISCV_JUMP_SLOT
>>      include $(srctree)/lib/vdso/Makefile
>>      # Symbols present in the vdso
>>      vdso-syms  = rt_sigreturn
>>     -ifdef CONFIG_64BIT
>>      vdso-syms += vgettimeofday
>>     -endif
>>      vdso-syms += getcpu
>>      vdso-syms += flush_icache
>>
>> and it doesn't build.  I've added Arnd, who might have a better idea of what's
>> going on.  Whatever happens, I think the best bet is to just drop the clock
>> functions (specifically __vdso_{clock_gettime,gettimeofday,clock_getres}) from
>> the rv32 port right now.
> 
> For rv32 you need clock_gettime64, not clock_gettime, which in the Linux
> ABI refers to the interface with the old timespec. There was some debate
> over whether clock_getres_time64 and gettimeofday_time64 would make
> sense to be added here, but I have so far leaned to the position that these
> are not as performance critical and not worth the effort.
> 
> Vincenzo has argued that we might want to extend the generic vdso code
> to include a number of additional syscall implementations, which would
> then include gettimeofday_time64 and clock_getres_time64.
> 

I agree with Arnd, clock_getres_time64 and gettimeofday_time64 were not added in
the original port because not considered as performance critical as
clock_gettime64. We might reconsider if there is a strong use case for those.

>         Arnd
> 

-- 
Regards,
Vincenzo

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [musl] riscv32 v2
  2020-09-10 10:01           ` Vincenzo Frascino
@ 2020-09-11  0:08             ` Palmer Dabbelt
  0 siblings, 0 replies; 21+ messages in thread
From: Palmer Dabbelt @ 2020-09-11  0:08 UTC (permalink / raw)
  To: vincenzo.frascino; +Cc: Arnd Bergmann, dalias, musl

On Thu, 10 Sep 2020 03:01:31 PDT (-0700), vincenzo.frascino@arm.com wrote:
>
>
> On 9/10/20 8:36 AM, Arnd Bergmann wrote:
>> On Thu, Sep 10, 2020 at 1:08 AM Palmer Dabbelt <palmerdabbelt@google.com> wrote:
>>> On Wed, 09 Sep 2020 14:36:44 PDT (-0700), dalias@libc.org wrote:
>>>> On Wed, Sep 09, 2020 at 02:28:55PM -0700, Palmer Dabbelt wrote:
>>>>> On Wed, 09 Sep 2020 13:28:27 PDT (-0700), dalias@libc.org wrote:
>>>> Possible addition of vdso clock_gettime isn't a blocker for moving
>>>> forward with the musl port, but syscall_arch.h should accurately
>>>> describe what's available and should not attempt to use vdso before
>>>> it's a public kernel interface (e.g. resolving the question of what
>>>> the function name will be). So I think it should be removed for now.
>>>
>>> Sorry if that was confusing, but I definitely agree.
>>>
>>> I guess my point was that the lack of VDSO clock functions on rv32 was probably
>>> an oversight, but one that shouldn't block the port.  We definitely can't just
>>> make up a kernel interface, particularly as the reason we don't have these on
>>> rv32 is because the generic versions of the functions we're using don't appear
>>> to run on 32-bit targets.
>>>
>>> That probably means there's some more subtle issue, though TBH I don't know
>>> enough about the 64-bit-ification of time_t for it to just jump out at me.  I
>>> don't want to derail the thread too much, but I tried the obvious thing
>>
>> When the vdso for rv64 was added, there was no time64 support in the
>> vdso code in general, as this only came with the "generic vdso" infrastructure
>> that was added later on, with commit ad5d1122b82f ("riscv: use vDSO
>> common flow to reduce the latency of the time-related functions") in v5.8.
>>
>> At that point it probably should have been added as well.
>>
>>>     --- a/arch/riscv/kernel/vdso/Makefile
>>>     +++ b/arch/riscv/kernel/vdso/Makefile
>>>     @@ -7,9 +7,7 @@ ARCH_REL_TYPE_ABS := R_RISCV_32|R_RISCV_64|R_RISCV_JUMP_SLOT
>>>      include $(srctree)/lib/vdso/Makefile
>>>      # Symbols present in the vdso
>>>      vdso-syms  = rt_sigreturn
>>>     -ifdef CONFIG_64BIT
>>>      vdso-syms += vgettimeofday
>>>     -endif
>>>      vdso-syms += getcpu
>>>      vdso-syms += flush_icache
>>>
>>> and it doesn't build.  I've added Arnd, who might have a better idea of what's
>>> going on.  Whatever happens, I think the best bet is to just drop the clock
>>> functions (specifically __vdso_{clock_gettime,gettimeofday,clock_getres}) from
>>> the rv32 port right now.
>>
>> For rv32 you need clock_gettime64, not clock_gettime, which in the Linux
>> ABI refers to the interface with the old timespec. There was some debate
>> over whether clock_getres_time64 and gettimeofday_time64 would make
>> sense to be added here, but I have so far leaned to the position that these
>> are not as performance critical and not worth the effort.
>>
>> Vincenzo has argued that we might want to extend the generic vdso code
>> to include a number of additional syscall implementations, which would
>> then include gettimeofday_time64 and clock_getres_time64.
>>
>
> I agree with Arnd, clock_getres_time64 and gettimeofday_time64 were not added in
> the original port because not considered as performance critical as
> clock_gettime64. We might reconsider if there is a strong use case for those.

OK, seems reasonable to me.  I guess we can always add things later if they end
up being important, though I don't really have any feel for this sort of stuff
so I don't really have an opinion either way.

Thanks!

>
>>         Arnd
>>

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2020-09-11  0:28 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-04  5:48 [musl] riscv32 v2 Stefan O'Rear
2020-09-07 10:47 ` Stefan O'Rear
2020-09-07 18:06   ` Rich Felker
2020-09-07 21:35     ` Arnd Bergmann
2020-09-07 21:45       ` Rich Felker
2020-09-07 21:58         ` Arnd Bergmann
2020-09-07 22:11           ` Rich Felker
2020-09-07 22:30             ` Arnd Bergmann
2020-09-08  1:02               ` Rich Felker
2020-09-08  7:00                 ` Arnd Bergmann
2020-09-07 11:27 ` Stefan O'Rear
2020-09-07 18:09   ` Rich Felker
2020-09-08  1:54 ` Rich Felker
2020-09-09  6:07   ` Rich Felker
2020-09-09 20:28 ` Rich Felker
2020-09-09 21:28   ` Palmer Dabbelt
2020-09-09 21:36     ` Rich Felker
2020-09-09 23:08       ` Palmer Dabbelt
2020-09-10  7:36         ` Arnd Bergmann
2020-09-10 10:01           ` Vincenzo Frascino
2020-09-11  0:08             ` Palmer Dabbelt

mailing list of musl libc

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.vuxu.org/musl

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 musl musl/ http://inbox.vuxu.org/musl \
		musl@inbox.vuxu.org
	public-inbox-index musl

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.musl


code repositories for the project(s) associated with this inbox:

	https://git.vuxu.org/mirror/musl/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git