[musl] riscv port for review

Discussion:

Rich Felker

2018-09-28 02:24:04 UTC

Pulled from here:
https://github.com/riscv/riscv-musl/commit/6a4f4a9c774608add4b02f95322518bd2f5f51ee

Attached for review.

Rich Felker

2018-09-28 02:46:33 UTC

Permalink

Post by Rich Felker
https://github.com/riscv/riscv-musl/commit/6a4f4a9c774608add4b02f95322518bd2f5f51ee
Attached for review.
diff --git a/arch/riscv32/atomic_arch.h b/arch/riscv32/atomic_arch.h
new file mode 100644
index 0000000..93c89cc
--- /dev/null
+++ b/arch/riscv32/atomic_arch.h
@@ -0,0 +1,35 @@
+#define a_barrier a_barrier
+static inline void a_barrier()
+{
+ __asm__ __volatile__ ("fence rw,rw" : : : "memory");
+}
+
+#define a_ll a_ll
+static inline int a_ll(volatile int *p)
+{
+ int v;
+ __asm__ __volatile__ ("lr.w %0, (%1)" : "=&r"(v) : "r"(p));
+ return v;
+}
+
+#define a_sc a_sc
+static inline int a_sc(volatile int *p, int v)
+{
+ int r;
+ __asm__ __volatile__ ("sc.w %0, %2, (%1)" : "=&r"(r) : "r"(p), "r"(v) : "memory");
+ return !r;
+}
+
+#define a_cas a_cas
+static inline int a_cas(volatile int *p, int t, int s)
+{
+ int old, tmp;
+ __asm__("1: lr.w %0, %2 \n"
+ " bne %0, %3, 1f \n"
+ " sc.w %1, %4, %2 \n"
+ " bnez %1, 1b \n"
+ "1: \n"
+ : "=&r"(old), "+r"(tmp), "+A"(*p)
+ : "r"(t), "r"(s));
+ return old;
+}

Why are both a_ll/a_sc and a_cas defined, and why is a_cas missing
barriers? Normally if a_ll/a_sc/a_barrier are defined, the top-level
atomic.h should be allowed to generate a_cas in terms of them.

Post by Rich Felker
diff --git a/arch/riscv32/bits/signal.h b/arch/riscv32/bits/signal.h
new file mode 100644
index 0000000..8b992cc
--- /dev/null
+++ b/arch/riscv32/bits/signal.h
@@ -0,0 +1,113 @@
+#if defined(_POSIX_SOURCE) || defined(_POSIX_C_SOURCE) \
+ || defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
+
+#if defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
+# define MINSIGSTKSZ 2048
+# define SIGSTKSZ 8192
+#endif
+
+/* gregs[0] holds the program counter. */
+
+#if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
+typedef unsigned long greg_t;
+typedef unsigned long gregset_t[32];
+
+struct __riscv_f_ext_state {
+ unsigned int f[32];
+ unsigned int fcsr;
+};
+
+struct __riscv_d_ext_state {
+ unsigned long long f[32];
+ unsigned int fcsr;
+};
+
+struct __riscv_q_ext_state {
+ unsigned long long f[64] __attribute__((aligned(16)));
+ unsigned int fcsr;
+ unsigned int reserved[3];
+};
+
+union __riscv_fp_state {
+ struct __riscv_f_ext_state f;
+ struct __riscv_d_ext_state d;
+ struct __riscv_q_ext_state q;
+};
+
+typedef union __riscv_fp_state fpregset_t;
+
+typedef struct sigcontext {
+ gregset_t gregs;
+ fpregset_t fpregs;
+} mcontext_t;
+
+#else
+typedef struct {
+ unsigned long gregs[32];
+ unsigned long long fpregs[66];
+} mcontext_t;
+#endif

In the namespace-safe version of mcontext_t, the names gregs and
fpregs are not valid here. They would need to be __-prefixed or in
some other reserved namespace.

Post by Rich Felker
+struct sigaltstack {
+ void *ss_sp;
+ int ss_flags;
+ size_t ss_size;
+};
+
+typedef struct __ucontext
+{
+ unsigned long uc_flags;
+ struct __ucontext *uc_link;
+ stack_t uc_stack;
+ sigset_t uc_sigmask;
+ char __unused[1024 / 8 - sizeof(sigset_t)];

This is an invalid array of size zero and should just be removed.

Post by Rich Felker
+ mcontext_t uc_mcontext;
+} ucontext_t;

...

Post by Rich Felker
diff --git a/arch/riscv32/crt_arch.h b/arch/riscv32/crt_arch.h
new file mode 100644
index 0000000..65187e1
--- /dev/null
+++ b/arch/riscv32/crt_arch.h
@@ -0,0 +1,18 @@
+__asm__(
+".text\n"
+".global " START "\n"
+".type " START ",%function\n"
+START ":\n"
+".weak __global_pointer$\n"
+".hidden __global_pointer$\n\t"
+".option push\n"
+".option norelax\n\t"
+"lla gp, __global_pointer$\n"
+".option pop\n\t"
+"mv a0, sp\n"
+".weak _DYNAMIC\n"
+".hidden _DYNAMIC\n\t"
+"lla a1, _DYNAMIC\n\t"
+"andi sp, sp, -16\n\t"
+"jal " START "_c"
+);
diff --git a/arch/riscv32/pthread_arch.h b/arch/riscv32/pthread_arch.h
new file mode 100644
index 0000000..feffaa4
--- /dev/null
+++ b/arch/riscv32/pthread_arch.h
@@ -0,0 +1,12 @@
+static inline struct pthread *__pthread_self()
+{
+ char *tp;
+ __asm__ __volatile__("mv %0, tp" : "=r"(tp));
+ return (void *)(tp - sizeof(struct pthread));
+}
+
+#define TLS_ABOVE_TP
+#define GAP_ABOVE_TP 0
+#define TP_ADJ(p) ((char *)p + sizeof(struct pthread))
+
+#define MC_PC gregs[0]
diff --git a/arch/riscv32/reloc.h b/arch/riscv32/reloc.h
new file mode 100644
index 0000000..d057bbe
--- /dev/null
+++ b/arch/riscv32/reloc.h
@@ -0,0 +1,27 @@
+#if defined __riscv_float_abi_soft
+#define RISCV_FP_SUFFIX "-sf"
+#elif defined __riscv_float_abi_single
+#define RISCV_FP_SUFFIX "-sp"
+#elif defined __riscv_float_abi_double
+#define RISCV_FP_SUFFIX ""
+#endif
+
+#define RISCV_LDSO_HELPER(x) "riscv" #x
+#define RISCV_LDSO(x) RISCV_LDSO_HELPER(x)
+
+#define LDSO_ARCH RISCV_LDSO(__riscv_xlen) RISCV_FP_SUFFIX

Elsewhere it looks like little/big endian are both options, but I see
no endian variant here. If so this needs to be fixed.

Also what is __riscv_xlen? A predefined macro that expands to 32 or
64? Since this file is just for 32-bit it should just be hard-coded
rather than assuming a macro would expand to the token 32 and not
(31+1) or some other expression equal to 32, I think.

Post by Rich Felker
diff --git a/arch/riscv32/syscall_arch.h b/arch/riscv32/syscall_arch.h
new file mode 100644
index 0000000..bc60d1f
--- /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) 0, __SYSCALL_LL_E((x))
+
+#define __asm_syscall(...) \
+ __asm__ __volatile__ ("scall\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" */

In that case VDSO_USEFUL might as well also be omitted for now.

Post by Rich Felker
diff --git a/arch/riscv64/atomic_arch.h b/arch/riscv64/atomic_arch.h
new file mode 100644
index 0000000..018c7fd
--- /dev/null
+++ b/arch/riscv64/atomic_arch.h
@@ -0,0 +1,66 @@
+#define a_barrier a_barrier
+static inline void a_barrier()
+{
+ __asm__ __volatile__ ("fence rw,rw" : : : "memory");
+}
+
+#define a_ll a_ll
+static inline int a_ll(volatile int *p)
+{
+ int v;
+ __asm__ __volatile__ ("lr.w %0, %1" : "=&r"(v), "+A"(*p));
+ return v;
+}
+
+#define a_sc a_sc
+static inline int a_sc(volatile int *p, int v)
+{
+ int r;
+ __asm__ __volatile__ ("sc.w %0, %2, %1" : "=&r"(r), "+A"(*p) : "r"(v) : "memory");
+return !r;
+}
+
+#define a_cas a_cas
+static inline int a_cas(volatile int *p, int t, int s)
+{
+ int old, tmp;
+ __asm__("1: lr.w %0, %2 \n"
+ " bne %0, %3, 1f \n"
+ " sc.w %1, %4, %2 \n"
+ " bnez %1, 1b \n"
+ "1: \n"
+ : "=&r"(old), "+r"(tmp), "+A"(*p)
+ : "r"(t), "r"(s));
+ return old;
+}
+
+#define a_ll_p a_ll_p
+static inline void *a_ll_p(volatile void *p)
+{
+ void *v;
+ __asm__ __volatile__ ("lr.d %0, %1" : "=&r"(v), "+A"(*(long *)p));
+ return v;
+}
+
+#define a_sc_p a_sc_p
+static inline int a_sc_p(volatile int *p, void *v)
+{
+ int r;
+ __asm__ __volatile__ ("sc.d %0, %2, %1" : "=&r"(r), "+A"(*(long *)p) : "r"(v) : "memory");
+ return !r;
+}
+
+#define a_cas_p a_cas_p
+static inline void *a_cas_p(volatile void *p, void *t, void *s)
+{
+ void *old;
+ int tmp;
+ __asm__("1: lr.d %0, %2 \n"
+ " bne %0, %3, 1f \n"
+ " sc.d %1, %4, %2 \n"
+ " bnez %1, 1b \n"
+ "1: \n"
+ : "=&r"(old), "+r"(tmp), "+A"(*(long *)p)
+ : "r"(t), "r"(s));
+ return old;
+}

Same comments about cas vs ll/sc, and lack of barrier, as 32-bit version.

Post by Rich Felker
diff --git a/arch/riscv64/bits/signal.h b/arch/riscv64/bits/signal.h
new file mode 100644
index 0000000..8b992cc
--- /dev/null
+++ b/arch/riscv64/bits/signal.h
@@ -0,0 +1,113 @@
+#if defined(_POSIX_SOURCE) || defined(_POSIX_C_SOURCE) \
+ || defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
+
+#if defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
+# define MINSIGSTKSZ 2048
+# define SIGSTKSZ 8192
+#endif
+
+/* gregs[0] holds the program counter. */
+
+#if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
+typedef unsigned long greg_t;
+typedef unsigned long gregset_t[32];
+
+struct __riscv_f_ext_state {
+ unsigned int f[32];
+ unsigned int fcsr;
+};
+
+struct __riscv_d_ext_state {
+ unsigned long long f[32];
+ unsigned int fcsr;
+};
+
+struct __riscv_q_ext_state {
+ unsigned long long f[64] __attribute__((aligned(16)));
+ unsigned int fcsr;
+ unsigned int reserved[3];
+};
+
+union __riscv_fp_state {
+ struct __riscv_f_ext_state f;
+ struct __riscv_d_ext_state d;
+ struct __riscv_q_ext_state q;
+};
+
+typedef union __riscv_fp_state fpregset_t;
+
+typedef struct sigcontext {
+ gregset_t gregs;
+ fpregset_t fpregs;
+} mcontext_t;
+
+#else
+typedef struct {
+ unsigned long gregs[32];
+ unsigned long long fpregs[66];
+} mcontext_t;
+#endif
+
+struct sigaltstack {
+ void *ss_sp;
+ int ss_flags;
+ size_t ss_size;
+};
+
+typedef struct __ucontext
+{
+ unsigned long uc_flags;
+ struct __ucontext *uc_link;
+ stack_t uc_stack;
+ sigset_t uc_sigmask;
+ char __unused[1024 / 8 - sizeof(sigset_t)];
+ mcontext_t uc_mcontext;
+} ucontext_t;

Same issues here as 32-bit.

Post by Rich Felker
diff --git a/arch/riscv64/reloc.h b/arch/riscv64/reloc.h
new file mode 100644
index 0000000..8bd90dd
--- /dev/null
+++ b/arch/riscv64/reloc.h
@@ -0,0 +1,27 @@
+#if defined __riscv_float_abi_soft
+#define RISCV_FP_SUFFIX "-sf"
+#elif defined __riscv_float_abi_single
+#define RISCV_FP_SUFFIX "-sp"
+#elif defined __riscv_float_abi_double
+#define RISCV_FP_SUFFIX ""
+#endif
+
+#define RISCV_LDSO_HELPER(x) "riscv" #x
+#define RISCV_LDSO(x) RISCV_LDSO_HELPER(x)
+
+#define LDSO_ARCH RISCV_LDSO(__riscv_xlen) RISCV_FP_SUFFIX

Same here.

Post by Rich Felker
diff --git a/configure b/configure
index 997e665..4d3d8b4 100755
--- a/configure
+++ b/configure
@@ -322,6 +322,8 @@ microblaze*) ARCH=microblaze ;;
or1k*) ARCH=or1k ;;
powerpc64*) ARCH=powerpc64 ;;
powerpc*) ARCH=powerpc ;;
+riscv64*) ARCH=riscv64 ;;
+riscv*) ARCH=riscv32 ;;
sh[1-9bel-]*|sh|superh*) ARCH=sh ;;
s390x*) ARCH=s390x ;;
unknown) fail "$0: unable to detect target arch; try $0 --target=..." ;;
@@ -640,6 +642,11 @@ trycppif __LITTLE_ENDIAN__ "$t" && SUBARCH=${SUBARCH}le
trycppif _SOFT_FLOAT "$t" && fail "$0: error: soft-float not supported on powerpc64"
fi
+if test "$ARCH" = "riscv" || test "$ARCH" = "riscv64" ; then
+trycppif "RISCVEB || _RISCVEB || __RISCVEB || __RISCVEB__" "$t" && SUBARCH=${SUBARCH}eb

Predefined macros that violate the namespace (RISCVEB) shouldn't be
defined or observed.

Post by Rich Felker
+trycppif __riscv_soft_float "$t" && SUBARCH=${SUBARCH}-sf
+fi
+
if test "$ARCH" = "sh" ; then
tryflag CFLAGS_AUTO -Wa,--isa=any
trycppif __BIG_ENDIAN__ "$t" && SUBARCH=${SUBARCH}eb
diff --git a/crt/riscv32/crti.s b/crt/riscv32/crti.s
new file mode 100644
index 0000000..6916bfd
--- /dev/null
+++ b/crt/riscv32/crti.s
@@ -0,0 +1,11 @@
+.section .init
+.global _init
+.type _init,%function
+ ret
+
+.section .fini
+.global _fini
+.type _fini,%function
+ ret
diff --git a/crt/riscv32/crtn.s b/crt/riscv32/crtn.s
new file mode 100644
index 0000000..e69de29

It looks like these are not used, right?

Post by Rich Felker
diff --git a/include/elf.h b/include/elf.h
index c229735..ec2e8fd 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -3164,6 +3164,62 @@ enum
#define R_BPF_NONE 0
#define R_BPF_MAP_FD 1
+#define R_RISCV_NONE 0
+#define R_RISCV_32 1
+#define R_RISCV_64 2
+#define R_RISCV_RELATIVE 3
+#define R_RISCV_COPY 4
+#define R_RISCV_JUMP_SLOT 5
+#define R_RISCV_TLS_DTPMOD32 6
+#define R_RISCV_TLS_DTPMOD64 7
+#define R_RISCV_TLS_DTPREL32 8
+#define R_RISCV_TLS_DTPREL64 9
+#define R_RISCV_TLS_TPREL32 10
+#define R_RISCV_TLS_TPREL64 11
+
+#define R_RISCV_BRANCH 16
+#define R_RISCV_JAL 17
+#define R_RISCV_CALL 18
+#define R_RISCV_CALL_PLT 19
+#define R_RISCV_GOT_HI20 20
+#define R_RISCV_TLS_GOT_HI20 21
+#define R_RISCV_TLS_GD_HI20 22
+#define R_RISCV_PCREL_HI20 23
+#define R_RISCV_PCREL_LO12_I 24
+#define R_RISCV_PCREL_LO12_S 25
+#define R_RISCV_HI20 26
+#define R_RISCV_LO12_I 27
+#define R_RISCV_LO12_S 28
+#define R_RISCV_TPREL_HI20 29
+#define R_RISCV_TPREL_LO12_I 30
+#define R_RISCV_TPREL_LO12_S 31
+#define R_RISCV_TPREL_ADD 32
+#define R_RISCV_ADD8 33
+#define R_RISCV_ADD16 34
+#define R_RISCV_ADD32 35
+#define R_RISCV_ADD64 36
+#define R_RISCV_SUB8 37
+#define R_RISCV_SUB16 38
+#define R_RISCV_SUB32 39
+#define R_RISCV_SUB64 40
+#define R_RISCV_GNU_VTINHERIT 41
+#define R_RISCV_GNU_VTENTRY 42
+#define R_RISCV_ALIGN 43
+#define R_RISCV_RVC_BRANCH 44
+#define R_RISCV_RVC_JUMP 45
+#define R_RISCV_RVC_LUI 46
+#define R_RISCV_GPREL_I 47
+#define R_RISCV_GPREL_S 48
+#define R_RISCV_TPREL_I 49
+#define R_RISCV_TPREL_S 50
+#define R_RISCV_RELAX 51
+#define R_RISCV_SUB6 52
+#define R_RISCV_SET6 53
+#define R_RISCV_SET8 54
+#define R_RISCV_SET16 55
+#define R_RISCV_SET32 56
+#define R_RISCV_32_PCREL 57
+
#ifdef __cplusplus
}
#endif

This should be its own patch independent of the port; I can commit it
earlier.

Post by Rich Felker
diff --git a/src/thread/riscv32/syscall_cp.s b/src/thread/riscv32/syscall_cp.s
new file mode 100644
index 0000000..71bf6d3
--- /dev/null
+++ b/src/thread/riscv32/syscall_cp.s
@@ -0,0 +1,29 @@
+.global __cp_begin
+.hidden __cp_begin
+.global __cp_end
+.hidden __cp_end
+.global __cp_cancel
+.hidden __cp_cancel
+.hidden __cancel
+.global __syscall_cp_asm
+.hidden __syscall_cp_asm
+.type __syscall_cp_asm, %function
+ lw t0, 0(a0)
+ bnez t0, __cp_cancel
+
+ mv t0, a1
+ mv a0, a2
+ mv a1, a3
+ mv a2, a4
+ mv a3, a5
+ mv a4, a6
+ mv a5, a7
+ lw a6, 0(sp)
+ mv a7, t0
+ scall
+ ret
+ j __cancel

The labels here are backwards. __cp_end must point immediately after
the syscall instruction, and __cp_end needs to jump to __cancel.

Post by Rich Felker
diff --git a/src/thread/riscv64/syscall_cp.s b/src/thread/riscv64/syscall_cp.s
new file mode 100644
index 0000000..c745b32
--- /dev/null
+++ b/src/thread/riscv64/syscall_cp.s
@@ -0,0 +1,29 @@
+.global __cp_begin
+.hidden __cp_begin
+.global __cp_end
+.hidden __cp_end
+.global __cp_cancel
+.hidden __cp_cancel
+.hidden __cancel
+.global __syscall_cp_asm
+.hidden __syscall_cp_asm
+.type __syscall_cp_asm, %function
+ ld t0, 0(a0)
+ bnez t0, __cp_cancel
+
+ mv t0, a1
+ mv a0, a2
+ mv a1, a3
+ mv a2, a4
+ mv a3, a5
+ mv a4, a6
+ mv a5, a7
+ ld a6, 0(sp)
+ mv a7, t0
+ scall
+ ret
+ j __cancel
--
2.10.0

Likewise here.

Rich

Rich Felker

2018-10-09 18:05:15 UTC

Permalink

Ping.

Post by Rich Felker

Why are both a_ll/a_sc and a_cas defined, and why is a_cas missing
barriers? Normally if a_ll/a_sc/a_barrier are defined, the top-level
atomic.h should be allowed to generate a_cas in terms of them.

In the namespace-safe version of mcontext_t, the names gregs and
fpregs are not valid here. They would need to be __-prefixed or in
some other reserved namespace.

This is an invalid array of size zero and should just be removed.

Post by Rich Felker
+ mcontext_t uc_mcontext;
+} ucontext_t;

....

Elsewhere it looks like little/big endian are both options, but I see
no endian variant here. If so this needs to be fixed.
Also what is __riscv_xlen? A predefined macro that expands to 32 or
64? Since this file is just for 32-bit it should just be hard-coded
rather than assuming a macro would expand to the token 32 and not
(31+1) or some other expression equal to 32, I think.

In that case VDSO_USEFUL might as well also be omitted for now.

Same comments about cas vs ll/sc, and lack of barrier, as 32-bit version.

Same issues here as 32-bit.

Same here.

Predefined macros that violate the namespace (RISCVEB) shouldn't be
defined or observed.

It looks like these are not used, right?

This should be its own patch independent of the port; I can commit it
earlier.

The labels here are backwards. __cp_end must point immediately after
the syscall instruction, and __cp_end needs to jump to __cancel.

Likewise here.
Rich

Michael Clark

2018-10-09 21:36:14 UTC

Permalink

Hi,

I have a status update. I havenât made any changes to the musl port since last week but I did investigate and fix the stat issue in qemu-riscv32. I ran tests with riscv32 Linux kernel with glibc in QEMU full system emulation as Linux kernel is canonical for the ABI.

riscv32 is indeed a unique port in linux-kernel land. It does not define __ARCH_WANT_STAT64 in arch/riscv/include/asm/unistd.h (unlike all other 32-bit ports which confused me a bit). It does not have fstat64 and its stat structure matches rv64. Iâm kind of stubborn so I had to see the test pass/fail so I spent some time getting riscv32 linux to build. I should submit a couple of small patches for riscv-linux.

I fixed riscv32 stat in riscv-qemu: https://github.com/riscv/riscv-qemu

RISC-V QEMU had the 32-bit asm-generic which has a 32-bit stat, but no fstat64. We may have missed a chunk when we rebased the 2016 riscv-qemu baseline. The stat patch is in qemu-for-upstream branch in the riscv-qemu branch.

We need to work on getting our QEMU changes upstream. We have problems with getting code into upstream QEMU as they have very strict code review policy on pull requests. Every commit must have âReviewed-by:â with a âtrusted reviewerâ. It doesnât seem to prevent changes coming from Linaro breaking the RISC-V port but it does prevent our fixes going in. My current focus is working on automated test cases for the fixes in the QEMU queue so we can change the discourse to does it pass an extensive set of tests. Currently the baseline for inclusion is does it compile and passes âmake checkâ, have Reviewed-by and Signed-off-by tags and does it conform to the QEMU coding style (checkpatch.pl). The issue is we are making changes that typically require reading the Privileged ISA manual for verification and we donât have high test case coverage for the Privileged ISA. The Base ISA has pretty good test coverage.

I would be happy if someone volunteered to help with the RISC-V musl port...

Michael

Ping.

Post by Rich Felker

Why are both a_ll/a_sc and a_cas defined, and why is a_cas missing
barriers? Normally if a_ll/a_sc/a_barrier are defined, the top-level
atomic.h should be allowed to generate a_cas in terms of them.

In the namespace-safe version of mcontext_t, the names gregs and
fpregs are not valid here. They would need to be __-prefixed or in
some other reserved namespace.

This is an invalid array of size zero and should just be removed.

Post by Rich Felker
+ mcontext_t uc_mcontext;
+} ucontext_t;

....

Elsewhere it looks like little/big endian are both options, but I see
no endian variant here. If so this needs to be fixed.
Also what is __riscv_xlen? A predefined macro that expands to 32 or
64? Since this file is just for 32-bit it should just be hard-coded
rather than assuming a macro would expand to the token 32 and not
(31+1) or some other expression equal to 32, I think.

In that case VDSO_USEFUL might as well also be omitted for now.

Same comments about cas vs ll/sc, and lack of barrier, as 32-bit version.

Same issues here as 32-bit.

Same here.

Predefined macros that violate the namespace (RISCVEB) shouldn't be
defined or observed.

It looks like these are not used, right?

This should be its own patch independent of the port; I can commit it
earlier.

The labels here are backwards. __cp_end must point immediately after
the syscall instruction, and __cp_end needs to jump to __cancel.

Likewise here.
Rich

Khem Raj

2018-10-10 01:14:06 UTC

Permalink

Hi,
I have a status update. I haven’t made any changes to the musl port since last week but I did investigate and fix the stat issue in qemu-riscv32. I ran tests with riscv32 Linux kernel with glibc in QEMU full system emulation as Linux kernel is canonical for the ABI.
riscv32 is indeed a unique port in linux-kernel land. It does not define __ARCH_WANT_STAT64 in arch/riscv/include/asm/unistd.h (unlike all other 32-bit ports which confused me a bit). It does not have fstat64 and its stat structure matches rv64. I’m kind of stubborn so I had to see the test pass/fail so I spent some time getting riscv32 linux to build. I should submit a couple of small patches for riscv-linux.
I fixed riscv32 stat in riscv-qemu: https://github.com/riscv/riscv-qemu
RISC-V QEMU had the 32-bit asm-generic which has a 32-bit stat, but no fstat64. We may have missed a chunk when we rebased the 2016 riscv-qemu baseline. The stat patch is in qemu-for-upstream branch in the riscv-qemu branch.
We need to work on getting our QEMU changes upstream. We have problems with getting code into upstream QEMU as they have very strict code review policy on pull requests. Every commit must have “Reviewed-by:” with a “trusted reviewer”. It doesn’t seem to prevent changes coming from Linaro breaking the RISC-V port but it does prevent our fixes going in. My current focus is working on automated test cases for the fixes in the QEMU queue so we can change the discourse to does it pass an extensive set of tests. Currently the baseline for inclusion is does it compile and passes “make check”, have Reviewed-by and Signed-off-by tags and does it conform to the QEMU coding style (checkpatch.pl). The issue is we are making changes that typically require reading the Privileged ISA manual for verification and we don’t have high test case coverage for the Privileged ISA. The Base ISA has pretty good test coverage.
I would be happy if someone volunteered to help with the RISC-V musl port...

Thanks Michael, its on my TODO list after ELCE, I have setup OE builds
already, but need to cherry-pick your patches and test a
core-image-minimal to start the process.

Michael
Ping.
https://github.com/riscv/riscv-musl/commit/6a4f4a9c774608add4b02f95322518bd2f5f51ee
Attached for review.
diff --git a/arch/riscv32/atomic_arch.h b/arch/riscv32/atomic_arch.h
new file mode 100644
index 0000000..93c89cc
--- /dev/null
+++ b/arch/riscv32/atomic_arch.h
@@ -0,0 +1,35 @@
+#define a_barrier a_barrier
+static inline void a_barrier()
+{
+ __asm__ __volatile__ ("fence rw,rw" : : : "memory");
+}
+
+#define a_ll a_ll
+static inline int a_ll(volatile int *p)
+{
+ int v;
+ __asm__ __volatile__ ("lr.w %0, (%1)" : "=&r"(v) : "r"(p));
+ return v;
+}
+
+#define a_sc a_sc
+static inline int a_sc(volatile int *p, int v)
+{
+ int r;
+ __asm__ __volatile__ ("sc.w %0, %2, (%1)" : "=&r"(r) : "r"(p), "r"(v) : "memory");
+ return !r;
+}
+
+#define a_cas a_cas
+static inline int a_cas(volatile int *p, int t, int s)
+{
+ int old, tmp;
+ __asm__("1: lr.w %0, %2 \n"
+ " bne %0, %3, 1f \n"
+ " sc.w %1, %4, %2 \n"
+ " bnez %1, 1b \n"
+ "1: \n"
+ : "=&r"(old), "+r"(tmp), "+A"(*p)
+ : "r"(t), "r"(s));
+ return old;
+}
Why are both a_ll/a_sc and a_cas defined, and why is a_cas missing
barriers? Normally if a_ll/a_sc/a_barrier are defined, the top-level
atomic.h should be allowed to generate a_cas in terms of them.
diff --git a/arch/riscv32/bits/signal.h b/arch/riscv32/bits/signal.h
new file mode 100644
index 0000000..8b992cc
--- /dev/null
+++ b/arch/riscv32/bits/signal.h
@@ -0,0 +1,113 @@
+#if defined(_POSIX_SOURCE) || defined(_POSIX_C_SOURCE) \
+ || defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
+
+#if defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
+# define MINSIGSTKSZ 2048
+# define SIGSTKSZ 8192
+#endif
+
+/* gregs[0] holds the program counter. */
+
+#if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
+typedef unsigned long greg_t;
+typedef unsigned long gregset_t[32];
+
+struct __riscv_f_ext_state {
+ unsigned int f[32];
+ unsigned int fcsr;
+};
+
+struct __riscv_d_ext_state {
+ unsigned long long f[32];
+ unsigned int fcsr;
+};
+
+struct __riscv_q_ext_state {
+ unsigned long long f[64] __attribute__((aligned(16)));
+ unsigned int fcsr;
+ unsigned int reserved[3];
+};
+
+union __riscv_fp_state {
+ struct __riscv_f_ext_state f;
+ struct __riscv_d_ext_state d;
+ struct __riscv_q_ext_state q;
+};
+
+typedef union __riscv_fp_state fpregset_t;
+
+typedef struct sigcontext {
+ gregset_t gregs;
+ fpregset_t fpregs;
+} mcontext_t;
+
+#else
+typedef struct {
+ unsigned long gregs[32];
+ unsigned long long fpregs[66];
+} mcontext_t;
+#endif
In the namespace-safe version of mcontext_t, the names gregs and
fpregs are not valid here. They would need to be __-prefixed or in
some other reserved namespace.
+struct sigaltstack {
+ void *ss_sp;
+ int ss_flags;
+ size_t ss_size;
+};
+
+typedef struct __ucontext
+{
+ unsigned long uc_flags;
+ struct __ucontext *uc_link;
+ stack_t uc_stack;
+ sigset_t uc_sigmask;
+ char __unused[1024 / 8 - sizeof(sigset_t)];
This is an invalid array of size zero and should just be removed.
+ mcontext_t uc_mcontext;
+} ucontext_t;
....
diff --git a/arch/riscv32/crt_arch.h b/arch/riscv32/crt_arch.h
new file mode 100644
index 0000000..65187e1
--- /dev/null
+++ b/arch/riscv32/crt_arch.h
@@ -0,0 +1,18 @@
+__asm__(
+".text\n"
+".global " START "\n"
+".type " START ",%function\n"
+START ":\n"
+".weak __global_pointer$\n"
+".hidden __global_pointer$\n\t"
+".option push\n"
+".option norelax\n\t"
+"lla gp, __global_pointer$\n"
+".option pop\n\t"
+"mv a0, sp\n"
+".weak _DYNAMIC\n"
+".hidden _DYNAMIC\n\t"
+"lla a1, _DYNAMIC\n\t"
+"andi sp, sp, -16\n\t"
+"jal " START "_c"
+);
diff --git a/arch/riscv32/pthread_arch.h b/arch/riscv32/pthread_arch.h
new file mode 100644
index 0000000..feffaa4
--- /dev/null
+++ b/arch/riscv32/pthread_arch.h
@@ -0,0 +1,12 @@
+static inline struct pthread *__pthread_self()
+{
+ char *tp;
+ __asm__ __volatile__("mv %0, tp" : "=r"(tp));
+ return (void *)(tp - sizeof(struct pthread));
+}
+
+#define TLS_ABOVE_TP
+#define GAP_ABOVE_TP 0
+#define TP_ADJ(p) ((char *)p + sizeof(struct pthread))
+
+#define MC_PC gregs[0]
diff --git a/arch/riscv32/reloc.h b/arch/riscv32/reloc.h
new file mode 100644
index 0000000..d057bbe
--- /dev/null
+++ b/arch/riscv32/reloc.h
@@ -0,0 +1,27 @@
+#if defined __riscv_float_abi_soft
+#define RISCV_FP_SUFFIX "-sf"
+#elif defined __riscv_float_abi_single
+#define RISCV_FP_SUFFIX "-sp"
+#elif defined __riscv_float_abi_double
+#define RISCV_FP_SUFFIX ""
+#endif
+
+#define RISCV_LDSO_HELPER(x) "riscv" #x
+#define RISCV_LDSO(x) RISCV_LDSO_HELPER(x)
+
+#define LDSO_ARCH RISCV_LDSO(__riscv_xlen) RISCV_FP_SUFFIX
Elsewhere it looks like little/big endian are both options, but I see
no endian variant here. If so this needs to be fixed.
Also what is __riscv_xlen? A predefined macro that expands to 32 or
64? Since this file is just for 32-bit it should just be hard-coded
rather than assuming a macro would expand to the token 32 and not
(31+1) or some other expression equal to 32, I think.
diff --git a/arch/riscv32/syscall_arch.h b/arch/riscv32/syscall_arch.h
new file mode 100644
index 0000000..bc60d1f
--- /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) 0, __SYSCALL_LL_E((x))
+
+#define __asm_syscall(...) \
+ __asm__ __volatile__ ("scall\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" */
In that case VDSO_USEFUL might as well also be omitted for now.
diff --git a/arch/riscv64/atomic_arch.h b/arch/riscv64/atomic_arch.h
new file mode 100644
index 0000000..018c7fd
--- /dev/null
+++ b/arch/riscv64/atomic_arch.h
@@ -0,0 +1,66 @@
+#define a_barrier a_barrier
+static inline void a_barrier()
+{
+ __asm__ __volatile__ ("fence rw,rw" : : : "memory");
+}
+
+#define a_ll a_ll
+static inline int a_ll(volatile int *p)
+{
+ int v;
+ __asm__ __volatile__ ("lr.w %0, %1" : "=&r"(v), "+A"(*p));
+ return v;
+}
+
+#define a_sc a_sc
+static inline int a_sc(volatile int *p, int v)
+{
+ int r;
+ __asm__ __volatile__ ("sc.w %0, %2, %1" : "=&r"(r), "+A"(*p) : "r"(v) : "memory");
+return !r;
+}
+
+#define a_cas a_cas
+static inline int a_cas(volatile int *p, int t, int s)
+{
+ int old, tmp;
+ __asm__("1: lr.w %0, %2 \n"
+ " bne %0, %3, 1f \n"
+ " sc.w %1, %4, %2 \n"
+ " bnez %1, 1b \n"
+ "1: \n"
+ : "=&r"(old), "+r"(tmp), "+A"(*p)
+ : "r"(t), "r"(s));
+ return old;
+}
+
+#define a_ll_p a_ll_p
+static inline void *a_ll_p(volatile void *p)
+{
+ void *v;
+ __asm__ __volatile__ ("lr.d %0, %1" : "=&r"(v), "+A"(*(long *)p));
+ return v;
+}
+
+#define a_sc_p a_sc_p
+static inline int a_sc_p(volatile int *p, void *v)
+{
+ int r;
+ __asm__ __volatile__ ("sc.d %0, %2, %1" : "=&r"(r), "+A"(*(long *)p) : "r"(v) : "memory");
+ return !r;
+}
+
+#define a_cas_p a_cas_p
+static inline void *a_cas_p(volatile void *p, void *t, void *s)
+{
+ void *old;
+ int tmp;
+ __asm__("1: lr.d %0, %2 \n"
+ " bne %0, %3, 1f \n"
+ " sc.d %1, %4, %2 \n"
+ " bnez %1, 1b \n"
+ "1: \n"
+ : "=&r"(old), "+r"(tmp), "+A"(*(long *)p)
+ : "r"(t), "r"(s));
+ return old;
+}
Same comments about cas vs ll/sc, and lack of barrier, as 32-bit version.
diff --git a/arch/riscv64/bits/signal.h b/arch/riscv64/bits/signal.h
new file mode 100644
index 0000000..8b992cc
--- /dev/null
+++ b/arch/riscv64/bits/signal.h
@@ -0,0 +1,113 @@
+#if defined(_POSIX_SOURCE) || defined(_POSIX_C_SOURCE) \
+ || defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
+
+#if defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
+# define MINSIGSTKSZ 2048
+# define SIGSTKSZ 8192
+#endif
+
+/* gregs[0] holds the program counter. */
+
+#if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
+typedef unsigned long greg_t;
+typedef unsigned long gregset_t[32];
+
+struct __riscv_f_ext_state {
+ unsigned int f[32];
+ unsigned int fcsr;
+};
+
+struct __riscv_d_ext_state {
+ unsigned long long f[32];
+ unsigned int fcsr;
+};
+
+struct __riscv_q_ext_state {
+ unsigned long long f[64] __attribute__((aligned(16)));
+ unsigned int fcsr;
+ unsigned int reserved[3];
+};
+
+union __riscv_fp_state {
+ struct __riscv_f_ext_state f;
+ struct __riscv_d_ext_state d;
+ struct __riscv_q_ext_state q;
+};
+
+typedef union __riscv_fp_state fpregset_t;
+
+typedef struct sigcontext {
+ gregset_t gregs;
+ fpregset_t fpregs;
+} mcontext_t;
+
+#else
+typedef struct {
+ unsigned long gregs[32];
+ unsigned long long fpregs[66];
+} mcontext_t;
+#endif
+
+struct sigaltstack {
+ void *ss_sp;
+ int ss_flags;
+ size_t ss_size;
+};
+
+typedef struct __ucontext
+{
+ unsigned long uc_flags;
+ struct __ucontext *uc_link;
+ stack_t uc_stack;
+ sigset_t uc_sigmask;
+ char __unused[1024 / 8 - sizeof(sigset_t)];
+ mcontext_t uc_mcontext;
+} ucontext_t;
Same issues here as 32-bit.
diff --git a/arch/riscv64/reloc.h b/arch/riscv64/reloc.h
new file mode 100644
index 0000000..8bd90dd
--- /dev/null
+++ b/arch/riscv64/reloc.h
@@ -0,0 +1,27 @@
+#if defined __riscv_float_abi_soft
+#define RISCV_FP_SUFFIX "-sf"
+#elif defined __riscv_float_abi_single
+#define RISCV_FP_SUFFIX "-sp"
+#elif defined __riscv_float_abi_double
+#define RISCV_FP_SUFFIX ""
+#endif
+
+#define RISCV_LDSO_HELPER(x) "riscv" #x
+#define RISCV_LDSO(x) RISCV_LDSO_HELPER(x)
+
+#define LDSO_ARCH RISCV_LDSO(__riscv_xlen) RISCV_FP_SUFFIX
Same here.
diff --git a/configure b/configure
index 997e665..4d3d8b4 100755
--- a/configure
+++ b/configure
@@ -322,6 +322,8 @@ microblaze*) ARCH=microblaze ;;
or1k*) ARCH=or1k ;;
powerpc64*) ARCH=powerpc64 ;;
powerpc*) ARCH=powerpc ;;
+riscv64*) ARCH=riscv64 ;;
+riscv*) ARCH=riscv32 ;;
sh[1-9bel-]*|sh|superh*) ARCH=sh ;;
s390x*) ARCH=s390x ;;
unknown) fail "$0: unable to detect target arch; try $0 --target=..." ;;
@@ -640,6 +642,11 @@ trycppif __LITTLE_ENDIAN__ "$t" && SUBARCH=${SUBARCH}le
trycppif _SOFT_FLOAT "$t" && fail "$0: error: soft-float not supported on powerpc64"
fi
+if test "$ARCH" = "riscv" || test "$ARCH" = "riscv64" ; then
+trycppif "RISCVEB || _RISCVEB || __RISCVEB || __RISCVEB__" "$t" && SUBARCH=${SUBARCH}eb
Predefined macros that violate the namespace (RISCVEB) shouldn't be
defined or observed.
+trycppif __riscv_soft_float "$t" && SUBARCH=${SUBARCH}-sf
+fi
+
if test "$ARCH" = "sh" ; then
tryflag CFLAGS_AUTO -Wa,--isa=any
trycppif __BIG_ENDIAN__ "$t" && SUBARCH=${SUBARCH}eb
diff --git a/crt/riscv32/crti.s b/crt/riscv32/crti.s
new file mode 100644
index 0000000..6916bfd
--- /dev/null
+++ b/crt/riscv32/crti.s
@@ -0,0 +1,11 @@
+.section .init
+.global _init
+.type _init,%function
+ ret
+
+.section .fini
+.global _fini
+.type _fini,%function
+ ret
diff --git a/crt/riscv32/crtn.s b/crt/riscv32/crtn.s
new file mode 100644
index 0000000..e69de29
It looks like these are not used, right?
diff --git a/include/elf.h b/include/elf.h
index c229735..ec2e8fd 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -3164,6 +3164,62 @@ enum
#define R_BPF_NONE 0
#define R_BPF_MAP_FD 1
+#define R_RISCV_NONE 0
+#define R_RISCV_32 1
+#define R_RISCV_64 2
+#define R_RISCV_RELATIVE 3
+#define R_RISCV_COPY 4
+#define R_RISCV_JUMP_SLOT 5
+#define R_RISCV_TLS_DTPMOD32 6
+#define R_RISCV_TLS_DTPMOD64 7
+#define R_RISCV_TLS_DTPREL32 8
+#define R_RISCV_TLS_DTPREL64 9
+#define R_RISCV_TLS_TPREL32 10
+#define R_RISCV_TLS_TPREL64 11
+
+#define R_RISCV_BRANCH 16
+#define R_RISCV_JAL 17
+#define R_RISCV_CALL 18
+#define R_RISCV_CALL_PLT 19
+#define R_RISCV_GOT_HI20 20
+#define R_RISCV_TLS_GOT_HI20 21
+#define R_RISCV_TLS_GD_HI20 22
+#define R_RISCV_PCREL_HI20 23
+#define R_RISCV_PCREL_LO12_I 24
+#define R_RISCV_PCREL_LO12_S 25
+#define R_RISCV_HI20 26
+#define R_RISCV_LO12_I 27
+#define R_RISCV_LO12_S 28
+#define R_RISCV_TPREL_HI20 29
+#define R_RISCV_TPREL_LO12_I 30
+#define R_RISCV_TPREL_LO12_S 31
+#define R_RISCV_TPREL_ADD 32
+#define R_RISCV_ADD8 33
+#define R_RISCV_ADD16 34
+#define R_RISCV_ADD32 35
+#define R_RISCV_ADD64 36
+#define R_RISCV_SUB8 37
+#define R_RISCV_SUB16 38
+#define R_RISCV_SUB32 39
+#define R_RISCV_SUB64 40
+#define R_RISCV_GNU_VTINHERIT 41
+#define R_RISCV_GNU_VTENTRY 42
+#define R_RISCV_ALIGN 43
+#define R_RISCV_RVC_BRANCH 44
+#define R_RISCV_RVC_JUMP 45
+#define R_RISCV_RVC_LUI 46
+#define R_RISCV_GPREL_I 47
+#define R_RISCV_GPREL_S 48
+#define R_RISCV_TPREL_I 49
+#define R_RISCV_TPREL_S 50
+#define R_RISCV_RELAX 51
+#define R_RISCV_SUB6 52
+#define R_RISCV_SET6 53
+#define R_RISCV_SET8 54
+#define R_RISCV_SET16 55
+#define R_RISCV_SET32 56
+#define R_RISCV_32_PCREL 57
+
#ifdef __cplusplus
}
#endif
This should be its own patch independent of the port; I can commit it
earlier.
diff --git a/src/thread/riscv32/syscall_cp.s b/src/thread/riscv32/syscall_cp.s
new file mode 100644
index 0000000..71bf6d3
--- /dev/null
+++ b/src/thread/riscv32/syscall_cp.s
@@ -0,0 +1,29 @@
+.global __cp_begin
+.hidden __cp_begin
+.global __cp_end
+.hidden __cp_end
+.global __cp_cancel
+.hidden __cp_cancel
+.hidden __cancel
+.global __syscall_cp_asm
+.hidden __syscall_cp_asm
+.type __syscall_cp_asm, %function
+ lw t0, 0(a0)
+ bnez t0, __cp_cancel
+
+ mv t0, a1
+ mv a0, a2
+ mv a1, a3
+ mv a2, a4
+ mv a3, a5
+ mv a4, a6
+ mv a5, a7
+ lw a6, 0(sp)
+ mv a7, t0
+ scall
+ ret
+ j __cancel
The labels here are backwards. __cp_end must point immediately after
the syscall instruction, and __cp_end needs to jump to __cancel.
diff --git a/src/thread/riscv64/syscall_cp.s b/src/thread/riscv64/syscall_cp.s
new file mode 100644
index 0000000..c745b32
--- /dev/null
+++ b/src/thread/riscv64/syscall_cp.s
@@ -0,0 +1,29 @@
+.global __cp_begin
+.hidden __cp_begin
+.global __cp_end
+.hidden __cp_end
+.global __cp_cancel
+.hidden __cp_cancel
+.hidden __cancel
+.global __syscall_cp_asm
+.hidden __syscall_cp_asm
+.type __syscall_cp_asm, %function
+ ld t0, 0(a0)
+ bnez t0, __cp_cancel
+
+ mv t0, a1
+ mv a0, a2
+ mv a1, a3
+ mv a2, a4
+ mv a3, a5
+ mv a4, a6
+ mv a5, a7
+ ld a6, 0(sp)
+ mv a7, t0
+ scall
+ ret
+ j __cancel
--
2.10.0
Likewise here.
Rich

Michael Clark

2018-10-10 03:41:15 UTC

Permalink

Post by Khem Raj

Thanks Michael, its on my TODO list after ELCE, I have setup OE builds
already, but need to cherry-pick your patches and test a
core-image-minimal to start the process.

Awesome! Thanks.

I’ve squashed everything to make it easy to cherry-pick (a habit for work-in-progress code). There was a bit of patch churn during some testing and alignment with glibc/Linux. I tend to squash patch churn like this; when moving between trees. Fortunately it is easy with musl’s layout because it is pretty much restricted to arch/riscv32 and arch/riscv64

Of course for changes this approach can make it harder for reviewers. I actually think GitHub adds some value here. I like to see the code as a whole, not as a context diff.

It can be split appropriately when the time comes. It does make one consider our view of history. Fortunately I think we are in the hands of good custodians...

Hopefully we don’t have to split it up into 100 different patches. Maybe we will. Who knows... I haven’t actually checked the whitespace is conforming or not.

Post by Khem Raj

Rich Felker

2018-09-28 02:47:49 UTC

Permalink

Post by Rich Felker
https://github.com/riscv/riscv-musl/commit/6a4f4a9c774608add4b02f95322518bd2f5f51ee
Attached for review.
diff --git a/arch/riscv32/bits/alltypes.h.in b/arch/riscv32/bits/alltypes.h.in
new file mode 100644
index 0000000..66ca18a
--- /dev/null
+++ b/arch/riscv32/bits/alltypes.h.in
@@ -0,0 +1,26 @@
+#define _Addr int
+#define _Int64 long long
+#define _Reg int
+
+TYPEDEF __builtin_va_list va_list;
+TYPEDEF __builtin_va_list __isoc_va_list;
+
+#ifndef __cplusplus
+TYPEDEF int wchar_t;
+#endif
+
+TYPEDEF float float_t;
+TYPEDEF double double_t;
+
+TYPEDEF struct { long long __ll; long double __ld; } max_align_t;
+
+TYPEDEF long time_t;

Is riscv32 time_t really 32-bit? If so that's really disappointing,
but presumably unfixable...

Rich

Michael Clark

2018-09-28 06:33:18 UTC

Permalink

Hi Rich,

Post by Rich Felker

Is riscv32 time_t really 32-bit? If so that's really disappointing,
but presumably unfixable...

This definitely is fixable as the riscv32 Linux ABI is not final. This is the first time a Linux libc ABI expert has looked closely at the musl port, and while riscv32 is present, all of my own testing has been performed on riscv64 Linux.

The RISC-V Glibc port currently only contains riscv64. The riscv32 ABI is still in the process of being finalised. Palmer likely knows the exact status. SiFive doesnât have any 32-bit cores with MMUs so it hasnât been high on the list of priorities.

In any case, now is a very good time to do some cross-checking between musl riscv32, qemu-riscv32, the glibc riscv32 port and riscv32 linux-kernel. I think we are just getting the default asm-generic in QEMU as we have not done anything special... yet...

I believe there is still an alignment issue with the stat structure in qemu-riscv32. When we last looked at this issue earlier in the year we didnât have linux-kernel support for riscv32, as the toolchain/ABI focus this year was on finalising riscv64 in Glibc and Linux.

Debian and Fedora only have riscv64 ports and afaik riscv64 is all that is present in Glibc. Of course linux-kernel is authoritative for the ABI so we need to run tests using riscv32 Linux in full system emulation in qemu-system-riscv32; qemu-riscv32 also needs to be audited. At the time we were submitting qemu upstream we could build riscv32 kernels. This has solved and we need to get regular build and test for riscv32 and riscv64 Linux and QEMU in CI...

There is an open bug on riscv-qemu stat which we can look at now that linux-kernel has initial riscv32 support. We should write a test that prints offsetof and sizeof for the stat structure members using musl, glibc and linux headers to find out whatâs happening...

I think there is still some time to nail down any remaining issues with riscv32. We definitely want 64-bit time_t and any other types that should be 64-bit should be audited now. Given the indirection through multiple levels of typedefs itâs not immediately clear without a bit of analysis.

We may do the same thing as was done with glibc and split the port up, first submitting riscv64, which is locked down.

I also had a question regarding whether we needed to #define __ARCH_WANT_STAT64 in linux/arch/riscv/include/asm/unistd.h or whether this is a legacy flag. All of the other 32-bit ports define it, but I do not know if it is necessary for a legacy free 32-bit port:

https://github.com/torvalds/linux/blob/master/arch/riscv/include/asm/unistd.h

BTW Thanks very much for the review... Iâll go through the remainder of the patch review comments and work on another revision... __riscv_xlen contains either 32 or 64 at present and is how RISC-V code distinguishes between riscv32 and riscv64. We can remove references to it from the musl riscv ports due to them not sharing code. Also, there is no official RISC-V big endian so we can remove EB. Big endian may be considered at some point, and it may well have appeared in a previous draft of the ISA manual, but at present, it is not defined in the ISA manual or toolchain, so weâll remove it.

Weâll do some 32-bit testing... now is a good time. SiFiveâs Linux board, the HiFive Unleashed, is riscv64, which likely explains the situation with riscv32.

Thanks,
Michael

Michael Clark

2018-09-28 06:49:05 UTC

Permalink

Post by Michael Clark
we could build riscv32 kernels

Typo: we couldn’t build riscv32 kernels.

We can perhaps use the musl riscv32 port as a good chance to sort out the riscv32 issues. SiFive’s Freedom Unleashed Linux SDK is riscv64 and all of my test kernels have been riscv64.

SiFive’s Freedom Everywhere riscv32 cores are no-MMU and the our riscv32 testing in QEMU has been emulating the HiFive1 board with the E-Series FE310G000 MCU.

It shouldn’t be too hard to sort out riscv32 linux. There have just been other priorities until now...

Szabolcs Nagy

2018-09-28 10:33:54 UTC

Permalink

Post by Michael Clark

Post by Rich Felker

Is riscv32 time_t really 32-bit? If so that's really disappointing,
but presumably unfixable...

This definitely is fixable as the riscv32 Linux ABI is not final.

i think linux is the problem: when x32 tried to do 64bit time_t
on an ilp32 target it turned out to be a disaster, because
various driver and fs code can only deal with 32bit time_t when
long is 32bit (although x32 had other issues too: mixing 64bit
kernel space and 32bit userspace types).

the consensus after x32 was that new 32bit targets will keep
using 32bit time_t and there is an independent project to add
64bit time_t support (by introducing new syscalls, types etc)
that will apply to all 32bit targets once it's done.

Rich Felker

2018-09-28 14:26:12 UTC

Permalink

Post by Szabolcs Nagy

Post by Michael Clark

Post by Rich Felker

Is riscv32 time_t really 32-bit? If so that's really disappointing,
but presumably unfixable...

This definitely is fixable as the riscv32 Linux ABI is not final.

Are you talking about ioctl interfaces? These need to be fixed at some
point anyway (both for glibc TIME64 profile and musl2) and we can shim
them in userspace with a big list of them. I'd rather do it now than
introduce a new arch/ABI that's broken by design and destined to be
unusable in a few years.

Post by Szabolcs Nagy
the consensus after x32 was that new 32bit targets will keep
using 32bit time_t and there is an independent project to add
64bit time_t support (by introducing new syscalls, types etc)
that will apply to all 32bit targets once it's done.

This is really unfortunate. I'd love it if the rv32 people could
provide some pushback against it.

Rich

Szabolcs Nagy

2018-09-28 11:43:24 UTC

Permalink

diff --git a/src/math/riscv64/fmax.s b/src/math/riscv64/fmax.s
new file mode 100644
index 0000000..40655d3
--- /dev/null
+++ b/src/math/riscv64/fmax.s
@@ -0,0 +1,5 @@
+.global fmax
+.type fmax, %function
+ fmax.d fa0, fa0, fa1
+ ret

this is ok, but note that

riscv fmax is ieee-754-2018 maximumNumber(x,y)
iso c fmax (with ts 18661) is ieee-754-2008 maxNum(x,y)

(see http://754r.ucbtest.org/drafts/
and http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1778.pdf )

they only differ in snan handling, current iso c (and musl)
does not care about signaling nans, but that might change.
(glibc cares and gcc has flags to make it care.)

musl is moving away from asm to c code with gcc style inline
asm wherever possible (the drawback is the dependency on gcc
asm syntax and constraints, the benefit is that pcs and
prologue/epilogue are handled by the compiler so all sorts of
instrumentations like debug info, -fstack-protector-all, etc
just work).

so i'd prefer to convert all these asm to c code.
(can be done after the port goes in)

Rich Felker

2018-09-28 14:28:14 UTC

Permalink

Post by Szabolcs Nagy

Yes, for now I think it doesn't matter.

Post by Szabolcs Nagy
musl is moving away from asm to c code with gcc style inline
asm wherever possible (the drawback is the dependency on gcc
asm syntax and constraints, the benefit is that pcs and
prologue/epilogue are handled by the compiler so all sorts of
instrumentations like debug info, -fstack-protector-all, etc
just work).
so i'd prefer to convert all these asm to c code.
(can be done after the port goes in)

Indeed, I'd rather do this later as a fixup for all archs at once than
try to work it out now as part of the riscv porting. They're logically
separate tasks.

Rich

Michael Forney

2018-10-11 07:34:40 UTC

Permalink

Post by Rich Felker
https://github.com/riscv/riscv-musl/commit/6a4f4a9c774608add4b02f95322518bd2f5f51ee
Attached for review.

I noticed that some fcntl.h constants are incorrect (O_DIRECTORY,
O_NOFOLLOW, O_DIRECT, O_LARGEFILE, and O_TMPFILE). Linux doesn't seem
to have a riscv-specific fcntl.h, so I think they just come from
asm-generic.

Rich Felker

2018-10-11 15:49:27 UTC

Permalink

Post by Michael Forney

Post by Rich Felker
https://github.com/riscv/riscv-musl/commit/6a4f4a9c774608add4b02f95322518bd2f5f51ee
Attached for review.

If that's the case the riscv32/riscv64 bits/fcntl.h files should just
be removed and the generic one will be used.

Rich

Michael Forney

2018-10-18 21:52:53 UTC

Permalink

Post by Michael Forney

Post by Rich Felker
https://github.com/riscv/riscv-musl/commit/6a4f4a9c774608add4b02f95322518bd2f5f51ee
Attached for review.

While playing around with a riscv32-linux-musl toolchain and tinyemu,
I found a few more issues:

- riscv linux has no renameat syscall. I think __NR_renameat needs to
be removed from arch/riscv*/bits/syscall.h.in, and musl's rename and
renameat need to fallback to SYS_renameat2 if SYS_renameat is not
defined.

- arch/riscv32/bits/syscall.h.in defines syscall names as if it were
64-bit. I think the following changes are necessary
__NR_fcntl -> __NR_fcntl64
__NR_statfs -> __NR_statfs64
__NR_fstatfs -> __NR_fstatfs64
__NR_truncate -> __NR_truncate64
__NR_ftruncate -> __NR_ftruncate64
__NR_lseek -> __NR__llseek (and __NR_llseek?)
__NR_fstatat -> __NR_fstatat64
__NR_fstat -> __NR_fstat64
__NR_mmap -> __NR_mmap2
__NR_fadvise64 -> __NR_fadvise64_64

- Since riscv32 uses fcntl64, and musl's struct flock corresponds to
struct flock64 on 32-bit, F_GETLK, F_SETLK, and F_SETLKW should be
defined to the corresponding *64 values (12, 13, 14). This matches
arch/generic/bits/fcntl.h, so I think arch/riscv32/bits/fcntl.h should
just be completely removed as Rich suggested.
arch/riscv64/bits/fcntl.h needs to stay to define the non *64 values
(5, 6, 7).

- For detecting soft float, configure compares $ARCH to riscv and
riscv64, but ARCH is set to riscv32 or riscv64 above. Also ARCH is set
to riscv32 when $target is riscv* but not riscv64*. Should this be
riscv32*?

- There are several instances of preprocessor checks
__riscv_soft_float, but this does not seem to be defined by gcc.
Perhaps this is superseded by __riscv_flen or __riscv_float_abi_soft?

- The functions in src/math/riscv* don't fall back to the C
implementation for soft float.

Michael Forney

2018-11-11 06:34:04 UTC

Permalink

Post by Michael Forney

Post by Rich Felker
https://github.com/riscv/riscv-musl/commit/6a4f4a9c774608add4b02f95322518bd2f5f51ee
Attached for review.

While playing around with a riscv32-linux-musl toolchain and tinyemu,
- riscv linux has no renameat syscall. I think __NR_renameat needs to
be removed from arch/riscv*/bits/syscall.h.in, and musl's rename and
renameat need to fallback to SYS_renameat2 if SYS_renameat is not
defined.
- arch/riscv32/bits/syscall.h.in defines syscall names as if it were
64-bit. I think the following changes are necessary
__NR_fcntl -> __NR_fcntl64
__NR_statfs -> __NR_statfs64
__NR_fstatfs -> __NR_fstatfs64
__NR_truncate -> __NR_truncate64
__NR_ftruncate -> __NR_ftruncate64
__NR_lseek -> __NR__llseek (and __NR_llseek?)
__NR_fstatat -> __NR_fstatat64
__NR_fstat -> __NR_fstat64
__NR_mmap -> __NR_mmap2
__NR_fadvise64 -> __NR_fadvise64_64
- Since riscv32 uses fcntl64, and musl's struct flock corresponds to
struct flock64 on 32-bit, F_GETLK, F_SETLK, and F_SETLKW should be
defined to the corresponding *64 values (12, 13, 14). This matches
arch/generic/bits/fcntl.h, so I think arch/riscv32/bits/fcntl.h should
just be completely removed as Rich suggested.
arch/riscv64/bits/fcntl.h needs to stay to define the non *64 values
(5, 6, 7).
- For detecting soft float, configure compares $ARCH to riscv and
riscv64, but ARCH is set to riscv32 or riscv64 above. Also ARCH is set
to riscv32 when $target is riscv* but not riscv64*. Should this be
riscv32*?
- There are several instances of preprocessor checks
__riscv_soft_float, but this does not seem to be defined by gcc.
Perhaps this is superseded by __riscv_flen or __riscv_float_abi_soft?
- The functions in src/math/riscv* don't fall back to the C
implementation for soft float.

Forgot to mention one other thing I found: __SYSCALL_LL_O on riscv32
is defined as if registers pairs need to be aligned for syscalls, but
that does not appear to be the case.

My proposed fixes are available here:
https://github.com/michaelforney/musl/compare/6a4f4a9...riscv

Zach van Rijn has applied those patches in his toolchains at
https://musl.cc, and Fabrice Bellard is using them in his buildroot
port at https://bellard.org/tinyemu/buildroot.html. It'd be nice to
get them applied/squashed into https://github.com/riscv/riscv-musl.

I haven't done any actual review of the port (and I don't actually
know much about riscv). I just ran into these issues while trying to
get some programs to run under linux in tinyemu.