Re: [musl] riscv32 v2

[Arnd Bergmann <arnd@arndb.de>]

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