[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 21 May 2022 14:38:20 +0800
From: 王洪亮 <wanghongliang@...ngson.cn>
To: musl@...ts.openwall.com
Subject: Re: add loongarch64 port v3
在 2022/5/16 下午10:27, Rich Felker 写道:
> OK, review follows:
>> diff --git a/arch/loongarch64/bits/signal.h b/arch/loongarch64/bits/signal.h
>> new file mode 100644
>> index 00000000..e6613516
>> --- /dev/null
>> +++ b/arch/loongarch64/bits/signal.h
>> @@ -0,0 +1,79 @@
>> +#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 4096
>> +#define SIGSTKSZ 16384
>> +#endif
>> +
>> +typedef unsigned long greg_t, gregset_t[32];
>> +
>> +typedef struct sigcontext {
>> + unsigned long pc;
>> + gregset_t gregs;
>> + unsigned int flags;
>> + unsigned long extcontext[0] __attribute__((__aligned__(16)));
>> +}mcontext_t;
> Again, [0] is not valid C. If the extension field is going to be
> declared at all it needs to be declared in a way it can be accessed
> without invoking UB, e.g. as a FAM.
[0] is allowed in GNU C, we can not use it in musl ?
I don't understand UB ? e.g. as a FAM ?
> I'm also not clear on how
> specifying the alignment here helps since any object created in a way
> that the alignment would affect cannot have the FAM present.
>
the __aligned__(16) here used to save 128bit vector later.
>
>> diff --git a/arch/loongarch64/kstat.h b/arch/loongarch64/kstat.h
>> new file mode 100644
>> index 00000000..900f119d
>> --- /dev/null
>> +++ b/arch/loongarch64/kstat.h
>> @@ -0,0 +1,21 @@
>> +struct kstat {
>> + dev_t st_dev;
>> + ino_t st_ino;
>> + mode_t st_mode;
>> + nlink_t st_nlink;
>> + uid_t st_uid;
>> + gid_t st_gid;
>> + dev_t st_rdev;
>> + unsigned long __pad;
>> + off_t st_size;
>> + blksize_t st_blksize;
>> + int __pad2;
>> + blkcnt_t st_blocks;
>> + long st_atime_sec;
>> + unsigned long st_atime_nsec;
>> + long st_mtime_sec;
>> + unsigned long st_mtime_nsec;
>> + long st_ctime_sec;
>> + unsigned long st_ctime_nsec;
>> + unsigned __unused[2];
>> +};
> I think this file should be removed, right? -- since there is no
> legacy stat sycall with a kstat structure.
yes, I removed this file.
>> diff --git a/arch/loongarch64/reloc.h b/arch/loongarch64/reloc.h
>> new file mode 100644
>> index 00000000..249cb40c
>> --- /dev/null
>> +++ b/arch/loongarch64/reloc.h
>> @@ -0,0 +1,35 @@
>> +#ifndef __RELOC_H__
>> +#define __RELOC_H__
>> +
>> +#define _GNU_SOURCE
>> +#include <endian.h>
> You cannot define FTMs in the middle of a header file that might be
> included somewhere other than the very top of a source file. But there
> does not seem to be any reason it's defined at all.
I fixed this error:
diff --git a/arch/loongarch64/reloc.h b/arch/loongarch64/reloc.h
index 249cb40c..2f759a34 100644
--- a/arch/loongarch64/reloc.h
+++ b/arch/loongarch64/reloc.h
@@ -1,9 +1,3 @@
-#ifndef __RELOC_H__
-#define __RELOC_H__
-
-#define _GNU_SOURCE
-#include <endian.h>
-
#ifdef __loongarch64_soft_float
#define FP_SUFFIX "-sf"
#else
@@ -31,5 +25,3 @@
" la.local $t1, "#sym" \n" \
" or %0, $t1, $zero \n" \
: "=r"(*(fp)) : : "memory" )
-
-#endif
>> diff --git a/src/ldso/loongarch64/dlsym.s b/src/ldso/loongarch64/dlsym.s
>> new file mode 100644
>> index 00000000..10ac5c7f
>> --- /dev/null
>> +++ b/src/ldso/loongarch64/dlsym.s
>> @@ -0,0 +1,12 @@
>> +.global dlsym
>> +.hidden __dlsym
>> +.type dlsym,@function
>> +dlsym:
>> + move $a2, $ra
>> + la.global $r16, __dlsym
>> + addi.d $sp, $sp, -8
>> + st.d $ra, $sp, 0
>> + jirl $ra, $r16, 0
>> + ld.d $ra, $sp, 0
>> + addi.d $sp, $sp, 8
>> + jirl $r0, $ra, 0
> Is there a reason this can't be a tail call? Maybe the ABI requires
> that the caller reserve space for the callee to spill arg registers
> that are used?
I optimized the code implementation:
diff --git a/src/ldso/loongarch64/dlsym.s b/src/ldso/loongarch64/dlsym.s
index 10ac5c7f..5f51ed85 100644
--- a/src/ldso/loongarch64/dlsym.s
+++ b/src/ldso/loongarch64/dlsym.s
@@ -4,9 +4,4 @@
dlsym:
move $a2, $ra
la.global $r16, __dlsym
- addi.d $sp, $sp, -8
- st.d $ra, $sp, 0
- jirl $ra, $r16, 0
- ld.d $ra, $sp, 0
- addi.d $sp, $sp, 8
- jirl $r0, $ra, 0
+ jirl $r0, $r16, 0
--
>> diff --git a/src/thread/loongarch64/clone.s b/src/thread/loongarch64/clone.s
>> new file mode 100644
>> index 00000000..ec920c7b
>> --- /dev/null
>> +++ b/src/thread/loongarch64/clone.s
>> @@ -0,0 +1,47 @@
>> +#__clone(func, stack, flags, arg, ptid, tls, ctid)
>> +# a0, a1, a2, a3, a4, a5, a6
>> +# sys_clone3(struct clone_args *cl_args, size_t size)
>> +# a0 a1
>> +
>> +.global __clone
>> +.hidden __clone
>> +.type __clone,@function
>> +__clone:
>> + # Save function pointer and argument pointer on new thread stack
>> + addi.d $a1, $a1, -16
>> + st.d $a0, $a1, 0 # save function pointer
>> + st.d $a3, $a1, 8 # save argument pointer
>> +
>> + li.d $t0, ~0x004000ff # mask CSIGNAL and CLONE_DETACHED
>> + and $t1, $a2, $t0 # cl_args.flags
>> + li.d $t0, 0x000000ff # CSIGNAL
>> + and $t2, $a2, $t0 # cl_args.exit_signal
>> +
>> + bstrins.d $sp, $zero, 2, 0 # align stack to 8 bytes
>> + addi.d $sp, $sp, -88 # struct clone_args
>> + st.d $t1, $sp, 0 # flags
>> + st.d $a4, $sp, 8 # pidfd
>> + st.d $a6, $sp, 16 # child_tid
>> + st.d $a4, $sp, 24 # parent_tid
>> + st.d $t2, $sp, 32 # exit_signal
>> + st.d $a1, $sp, 40 # stack
>> + st.d $zero, $sp, 48 # stack_size
>> + st.d $a5, $sp, 56 # tls
>> + st.d $zero, $sp, 64 # set_tid
>> + st.d $zero, $sp, 72 # set_tid_size
>> + st.d $zero, $sp, 80 # cgroup
>> +
>> + move $a0, $sp
>> + li.d $a1, 88
>> + li.d $a7, 435 # __NR_clone3
>> + syscall 0 # call clone3
>> +
>> + beqz $a0, 1f # whether child process
>> + addi.d $sp, $sp, 88
>> + jirl $zero, $ra, 0 # parent process return
>> +1:
>> + ld.d $t8, $sp, 0 # function pointer
>> + ld.d $a0, $sp, 8 # argument pointer
>> + jirl $ra, $t8, 0 # call the user's function
>> + li.d $a7, 93
>> + syscall 0 # child process exit
> Based on the outcome of the kernel discussiong about __NR_clone3 vs
> __NR_clone, this likely needs to be reverted to use __NR_clone again.
> I'm waiting to see how that comes out. Sorry for the back and forth
> while we try to get this right on the kernel side.
OK,No problem.
Content of type "text/html" skipped
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.
