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

[Joseph Myers]

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.

> 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.)

> +#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.

-- 
Joseph S. Myers
joseph@codesourcery.com