Re: [PATCH v2 10/15] RISC-V: Linux Syscall Interface

[Palmer Dabbelt <palmer at dabbelt dot com>]

On Wed, 20 Dec 2017 09:24:21 PST (-0800), joseph@codesourcery.com wrote:
> On Tue, 19 Dec 2017, Palmer Dabbelt wrote:
>
> > diff --git a/sysdeps/unix/sysv/linux/riscv/getmsg.c b/sysdeps/unix/sysv/linux/riscv/getmsg.c
> > new file mode 100644
> > index 000000000000..3a1fa0852504
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/riscv/getmsg.c
> > @@ -0,0 +1 @@
> > +#include <sysdeps/unix/sysv/linux/i386/getmsg.c>
>
> Really?  I don't see getpmsg in the generic or RISC-V syscall ABI.
>
> > diff --git a/sysdeps/unix/sysv/linux/riscv/kernel-features.h b/sysdeps/unix/sysv/linux/riscv/kernel-features.h
>
> My comments from
> <https://sourceware.org/ml/libc-alpha/2017-06/msg00661.html> still apply:
> you shouldn't need this file.
>
> > diff --git a/sysdeps/unix/sysv/linux/riscv/putmsg.c b/sysdeps/unix/sysv/linux/riscv/putmsg.c
> > new file mode 100644
> > index 000000000000..ebc1680ca7d1
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/riscv/putmsg.c
> > @@ -0,0 +1 @@
> > +#include <sysdeps/unix/sysv/linux/i386/putmsg.c>
>
> Same comment as for getmsg.

Thanks, we must have gotten our signals crossed on your original messages.  
You're right here, I'll remove the files for the v3.

> > diff --git a/sysdeps/unix/sysv/linux/riscv/rv32/readahead.c b/sysdeps/unix/sysv/linux/riscv/rv32/readahead.c
> > new file mode 100644
> > index 000000000000..80170c3e8a4d
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/riscv/rv32/readahead.c
> > @@ -0,0 +1 @@
> > +#include <sysdeps/unix/sysv/linux/arm/readahead.c>
>
> I commented on this before
> <https://sourceware.org/ml/libc-alpha/2017-06/msg00661.html>.  Are you
> sure about it?  The ARM file is dealing with a peculiarity of the syscall
> interface in the case where (a) it follows the C function interface and
> (b) a 32-bit function argument followed by a 64-bit function argument
> requires a one-register gap between them.
>
> Now, RISC-V doesn't seem to do anything special for this syscall in the
> kernel, so I presume it does follow the C function interface.  But
> <https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md>
> says that aligned register pairs are only used for variadic arguments, and
> readahead is not a variadic function.  So I'd expect there not to be a gap
> in the arguments to this syscall, and so use of the ARM function to be
> wrong.  (As the syscall has no errors and no testable semantic effect, as
> long as the fd argument is valid, getting this wrong would not result in
> any test failures.)

Sorry, again, that we missed that.  You're correct here, we'll remove it for 
the v3.

> > +#ifdef __ASSEMBLER__
> > +
> > +#include <sys/asm.h>
> > +
> > +#define ENTRY(name) LEAF(name)
>
> Missing preprocessor indentation here and subsequently (inconsistently) in
> this file.
>
> > +#endif /* linux/mips/sysdep.h */
>
> This comment is clearly wrong here.

These should all be fixed.  Thanks!