[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 9 Oct 2018 14:05:15 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: riscv port for review
Ping.
On Thu, Sep 27, 2018 at 10:46:33PM -0400, Rich Felker wrote:
> On Thu, Sep 27, 2018 at 10:24:04PM -0400, Rich Felker wrote:
> > Pulled from here:
> > https://github.com/riscv/riscv-musl/commit/6a4f4a9c774608add4b02f95322518bd2f5f51ee
> >
> > Attached for review.
>
> Review inline below:
>
> > 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
> > +_init:
> > + ret
> > +
> > +.section .fini
> > +.global _fini
> > +.type _fini,%function
> > +_fini:
> > + 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
> > +__syscall_cp_asm:
> > +__cp_begin:
> > + 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
> > +__cp_cancel:
> > + ret
> > +__cp_end:
> > + 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
> > +__syscall_cp_asm:
> > +__cp_begin:
> > + 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
> > +__cp_cancel:
> > + ret
> > +__cp_end:
> > + j __cancel
> > --
> > 2.10.0
> >
>
> Likewise here.
>
> Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
