2016-03-18 10:37:43

by Zhangjian (Bamvor)

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

Hi, Yury

We are trying to test ilp32 in our arm64 board. But we got more
failure compare with you. So, I am wondering if we could align
the test environment with you. The source code we used:

1. glibc: the new-api branch of glibc from
[email protected]:gnu/norov_glibc.git.

2. Kernel: rfc5 from https://github.com/norov/linux.git. Is the
rc6 in your branch is the latest one? From the commit message, I
do not find any difference. Is it just a rebase?

3. Toolchain 4.9.3, could you provide the binary of toolchain(both
cross and native, gcc, gdb...)? It may be very useful for me or
other guys who interested build their own filesystem from
buildroot or something.

4. LTP: master.

For the glibc part, I found that there are 11 patches of ilp32 in top,
but the original 28 patches of ilp32 is not in the top, there are more
than 900 patches between them(referece the list below). Are you
willing rebase all the ilp32 relative patches. It is very useful for
reviewing and debugging. I saw andrew request the account in glibc,
maybe it has already been in processs?).

Regards

Bamvor

ILP32 relative patch in norov's glibc:
1 b5c4968 fix stat
2 8332b7a fix3
3 d16a202 fix statfs
4 351b872 fix IPC_64 for msgctl semctl shmctl
5 07f8ead fix readdir ????
6 4be481a fix lseek
7 4231518 fix getdents
8 de55857 fstat fix
9 5d42904 stat
10 3f70665 mmap, stat syscalls
11 574af33 Make ilp32 use the compat (old 32bit ABI).

972 8addb06 [AARCH64] Make lp64 and ilp32 directories.
973 bfe0619 [AARCH64] Add typesizes.h for ILP32
974 916581b [AARCH64] Fix up ucontext for ILP32
975 0f646ee [AARCH64] Add sigstack.h header for ILP32 reasons.
976 68e11d8 [AARCH64] Add kernel_sigaction.h for AARCH64 ILP32
977 a0087ba [AARCH64] Add ldd-rewrite.sed so that ilp32 ld.so can be found
978 0f800d9 [AARCH64] Add ILP32 ld.so to the known interpreter names.
979 45d74fd [AARCH64] Add support to ldconfig for ILP32 and libilp32
980 9926da9 [AARCH64] Add ILP32 to makefiles
981 987b522 [AARCH64] Set up wordsize for ILP32.
982 5e34e4c [AARCH64] Add ILP32 support to elf_machine_load_address.
983 1ff4dd9 [AARCH64] Reformat inline-asm in elf_machine_load_address.
984 f7ff9aa [AARCH64] Syscalls for ILP32 are passed always via 64bit values.
985 9316667 [AARCH64] Detect ILP32 in configure scripts.
986 ff0dca1 [AARCH64] Use PTR_REG in getcontext.S.
987 7c9f1b0 [AARCH64] Use PTR_* in start.S
988 ed747ff [AARCH64] Use PTR_* macros in dl-trampoline.S
989 71f6986 [AARCH64] Use PTR_REG/PTR_SIZE/PTR_SIZE_LOG in dl-tlsesc.S
990 3294b6f [AARCH64] Use PTR_REG in crti.S.
991 c8fef18 [AARCH64] Add PTR_REG, PTR_LOG_SIZE, and PTR_SIZE. Use it in LDST_PCREL and LDST_GLOBAL.
992 585197d Add dynamic ILP32 AARCH64 relocations to elf.h
993 49f1345 [AARCH64] Add header guards to sysdep.h headers.
994 2594068 Allow generic stat and statfs not have padding for 32bit targets
995 86f0aa7 Allow some fields of siginfo to be different from the generic one
996 fdffbc5 Allow fd_mask type not be an array of long.
997 453987a Allow rusage work on a big-endian 32bit-on-64bit target
998 70e6485 Add ability for the IPC structures (msqid_ds, semid_ds, shmid_ds, etc.) to have time_t being 64bit
999 9ff1730 Allow sigset be an array of a different type
1000 dc09040 [AARCH64] Fix utmp struct for compatibility reasons.


On 2016/2/26 4:28, Yury Norov wrote:
> On Thu, Feb 25, 2016 at 11:50:31AM +0100, Andreas Schwab wrote:
>> Yury Norov <[email protected]> writes:
>>
>>> I have new glibc that follows new ABI:
>>> https://github.com/norov/glibc/tree/new-api
>>
>> sysdeps/unix/sysv/linux/aarch64/ilp32/getdents64.c is wrong, struct
>> dirent64 is not the same as struct dirent. The file needs to be renamed
>> to sysdeps/unix/sysv/linux/aarch64/ilp32/getdents.c so that __getdents64
>> comes from sysdeps/unix/sysv/linux/generic/getdents64.c.
>>
>> sysdeps/unix/sysv/linux/aarch64/ilp32/*xstat*.c should not set errno,
>> INLINE_SYSCALL already does that, and returns -1 on error.
>>
>> Andreas.
>>
>
> Thank you. I'll fix it
>
>> --
>> Andreas Schwab, SUSE Labs, [email protected]
>> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
>> "And now for something completely different."


2016-03-18 15:49:36

by Yury Norov

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

On Fri, Mar 18, 2016 at 06:28:29PM +0800, Zhangjian (Bamvor) wrote:
> Hi, Yury
>
> We are trying to test ilp32 in our arm64 board. But we got more
> failure compare with you. So, I am wondering if we could align
> the test environment with you. The source code we used:
>

Hi, I mostly use QEMU but Thunder X CPU works similarly. I also
noticed that some tests fail sporadically, and some fail only in
scenario. The current fail list, as for me, is at the bottom.

Below links to my sources.

> 1. glibc: the new-api branch of glibc from
> [email protected]:gnu/norov_glibc.git.

It has few bugfixes:
https://github.com/norov/glibc/tree/new-api

>
> 2. Kernel: rfc5 from https://github.com/norov/linux.git. Is the
> rc6 in your branch is the latest one? From the commit message, I
> do not find any difference. Is it just a rebase?

https://github.com/norov/linux/tree/pin1
This branch is v4.5 plus
- Andrew's Thunder X patches (not mandatory);
- http://comments.gmane.org/gmane.linux.kernel/2116021
- http://comments.gmane.org/gmane.linux.kernel/2162853
- current ILP32 (RFC5 + few fixes)

>
> 3. Toolchain 4.9.3, could you provide the binary of toolchain(both
> cross and native, gcc, gdb...)? It may be very useful for me or
> other guys who interested build their own filesystem from
> buildroot or something.

I use one Andrew gave me. AFAIK, he's preparing useful package.
Andrew?

I use this cross-toolchain. Find sys-root under aarch64-thunderx-linux-gnu/
https://drive.google.com/file/d/0B93nHerV55yNdXBlVTRLNzF0aFE/view?usp=sharing

>
> 4. LTP: master.

Find LTP here:
https://drive.google.com/file/d/0B93nHerV55yNSTlZX3drRTBCaEU/view?usp=sharing

Notice that I configure it with command:
echo $(PREFIX)
/home/yury/work/toolchain/thunderx-tools/
./configure --host=aarch64 --prefix=/home/yury/work/ltp/ltp \
CC=$(PREFIX)/bin/a arch64-thunderx-linux-gnu-gcc \
AR=$(PREFIX)/bin/aarch64-thunderx-linux-gnu-ar \
STRIP =$(PREFIX)/bin/aarch64-thunderx-linux-gnu-strip \
RANLIB=$(PREFIX)/bin/aarch64-thunderx-linux-gnu-ranlib \
CFLAGS=-mabi=ilp32 \
LDFLAGS=-mabi=ilp32 -Wl,--rpath=/root/sys-root/libilp32 \
-Wl,--dynamic-linker=/root/sys-root/libilp32/ld-2.21.90.so

Notice that you don't need to place experimental sys-root to system paths,
but path is hardcoded to '/root/sys-root'

>
> For the glibc part, I found that there are 11 patches of ilp32 in top,
> but the original 28 patches of ilp32 is not in the top, there are more
> than 900 patches between them(referece the list below). Are you
> willing rebase all the ilp32 relative patches. It is very useful for
> reviewing and debugging. I saw andrew request the account in glibc,
> maybe it has already been in processs?).
>

I already told there's mess there, and I'd prefer to make things work
first and then do cleanup.

Yury.

Total Tests: 787
Total Skipped Tests: 25
Total Failures: 24

float_bessel FAIL 137
float_exp_log FAIL 137
float_iperb FAIL 137
float_power FAIL 137
float_trigo FAIL 137
abort01 FAIL 2
clone02 FAIL 4
fcntl11 FAIL 4
fcntl21 FAIL 4
kill10 FAIL 2
kill11 FAIL 2
mmap16 FAIL 6
nftw01 FAIL 1
nftw6401 FAIL 1
open12 FAIL 2
pathconf01 FAIL 1
profil01 FAIL 11
rename11 FAIL 2
rmdir02 FAIL 2
umount2_01 FAIL 2
umount2_02 FAIL 2
umount2_03 FAIL 2
utime06 FAIL 2
mtest06 FAIL 11

2016-03-18 15:55:34

by Alexander Graf

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64



On 18.03.16 16:49, Yury Norov wrote:
> On Fri, Mar 18, 2016 at 06:28:29PM +0800, Zhangjian (Bamvor) wrote:
>>
>> For the glibc part, I found that there are 11 patches of ilp32 in top,
>> but the original 28 patches of ilp32 is not in the top, there are more
>> than 900 patches between them(referece the list below). Are you
>> willing rebase all the ilp32 relative patches. It is very useful for
>> reviewing and debugging. I saw andrew request the account in glibc,
>> maybe it has already been in processs?).
>>
>
> I already told there's mess there, and I'd prefer to make things work
> first and then do cleanup.

So how is progress going overall? The last submission I've seen is
already 2 months ago. Are there particular bits holding you up?


Alex

2016-03-18 16:46:53

by Yury Norov

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

On Fri, Mar 18, 2016 at 04:55:26PM +0100, Alexander Graf wrote:
>
>
> On 18.03.16 16:49, Yury Norov wrote:
> > On Fri, Mar 18, 2016 at 06:28:29PM +0800, Zhangjian (Bamvor) wrote:
> >>
> >> For the glibc part, I found that there are 11 patches of ilp32 in top,
> >> but the original 28 patches of ilp32 is not in the top, there are more
> >> than 900 patches between them(referece the list below). Are you
> >> willing rebase all the ilp32 relative patches. It is very useful for
> >> reviewing and debugging. I saw andrew request the account in glibc,
> >> maybe it has already been in processs?).
> >>
> >
> > I already told there's mess there, and I'd prefer to make things work
> > first and then do cleanup.
>
> So how is progress going overall? The last submission I've seen is
> already 2 months ago. Are there particular bits holding you up?
>
>
> Alex

Hi Alexander,

For last time I mostly work on library, as it needs to be reworked
well. But yes, there's one serious bug puzzling me.

Tests like umount or pathconf fail but I see no major problem with
it, as it's most probably structure padding mismatch between kernel and
glibc. But there's (at least) one major problem I see.

Float tests fail due to NULL-dereferencing (0x14 actually) at
pthread_join(). It calls tgkill(), and after that child thread crashes.
See stack trace at the end.

The minimal test reproducing it is attached. The similar test where
parent forks a child and then kills it, works fine. (Attached too).

I see that in case of pthread, there's much more stuff that is cloned.
Other's looking similar.

pthread_create():
clone(child_stack=0xb953cea0, flags=CLONE_VM|CLONE_FS|CLONE_FILES
|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS
|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
parent_tidptr=0xb953d398, tls=0xb953d7c0, child_tidptr=0xb953d398) = 1650

fork():
clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD,
child_tidptr=0xe5af6278) = 30537

So this most probably means that ilp32 code doesn't handle one of cloned
item properly. I have already discovered a bug where child processes
used parent TLS, so maybe this is something similar...

Except of this, I think ILP32 series is looking pretty well, at least
kernel part.

If you have any ideas/suggestions, I'll really appreciate it.

Yury.

strace -f ./trigo
[...]
clone(child_stack=0xdbbfb000,
flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND
|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS
|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
parent_tidptr=0xdbbfb4f8, tls=0xdbbfb920, child_tidptr=0xdbbfb4f8) = 32030
rt_sigprocmask(SIG_BLOCK, [CHLD], Process 32030 attached [], 8) = 0
[pid 32029] rt_sigaction(SIGCHLD, NULL, <unfinished ...>
[pid 32030] set_robust_list(0xdbbfb504, 12 <unfinished ...>
[pid 32029] <... rt_sigaction resumed> {SIG_DFL, [ILL ABRT SEGV URG], 0}, 8) = 0
[pid 32030] <... set_robust_list resumed> ) = 0
[pid 32029] rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
[pid 32030] write(1, "started\n", 8started
<unfinished ...>
[pid 32029] nanosleep({1, 65536}, <unfinished ...>
[pid 32030] <... write resumed> ) = 8
[pid 32030] rt_sigprocmask(SIG_BLOCK, NULL, [], 8) = 0
[pid 32030] rt_sigsuspend([] <unfinished ...>
[pid 32029] <... nanosleep resumed> 0xfff9fd98) = 0
[pid 32029] write(1, "stoping...\n", 11stoping...) = 11
[pid 32029] openat(AT_FDCWD, "/root/sys-root/libilp32/libgcc_s.so.1", O_RDONLY|O_CLOEXEC) = 3
[pid 32029] read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\267\0\1\0\0\0 \0\0004\0\0\0"..., 512) = 512
[pid 32029] fstat(3, {st_mode=S_IFREG|0644, st_size=429138, ...}) = 0
[pid 32029] mmap(NULL, 135104, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xdb3db000
[pid 32029] mprotect(0xdb3ec000, 61440, PROT_NONE) = 0
[pid 32029] mmap(0xdb3fb000, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x10000) = 0xdb3fb000
[pid 32029] close(3) = 0
[pid 32029] tgkill(32029, 32030, SIGRTMIN) = 0
[pid 32030] <... rt_sigsuspend resumed> ) = ? ERESTARTNOHAND (To be
restarted if no handler)
[pid 32029] write(1, "pthread_cancel == 0\n", 20pthread_cancel == 0) = 20
[pid 32030] --- SIGRTMIN {si_signo=SIGRTMIN, si_code=SI_TKILL, si_pid=32029, si_uid=0} ---
[pid 32029] write(1, "stopped\n", 8stopped
<unfinished ...>
[pid 32030] --- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0x14} ---
[pid 32029] <... write resumed> ) = ? <unavailable>
[pid 32030] +++ killed by SIGSEGV +++
+++ killed by SIGSEGV +++
Segmentation fault

dmesg:
trigo[32246]: unhandled level 2 translation fault (11) at 0x00000014,
esr 0x90000006
pgd = ffffffc009335000
[00000014] *pgd=000000007917c003, *pud=000000007917c003,
*pmd=0000000000000000

CPU: 2 PID: 32246 Comm: trigo Not tainted 4.5.0+ #91
Hardware name: linux,dummy-virt (DT)
task: ffffffc00900e400 ti: ffffffc009078000 task.ti: ffffffc009078000
PC is at 0xda6853f0
LR is at 0xda6d5440
pc : [<00000000da6853f0>] lr : [<00000000da6d5440>] pstate: 60000000
sp : 00000000da511bc0
x29: 00000000da512e10 x28: 00000000da6a7000
x27: 0000000000000000 x26: 00000000da513490
x25: 0000000000000000 x24: 0000000000400820
x23: 00000000da6a9000 x22: 00000000ff869acb
x21: 00000000da6a9000 x20: 00000000da512e50
x19: 0000000000000000 x18: 0000000000000001
x17: 0000000000410bd8 x16: 00000000da691138
x15: 0000000000000000 x14: 0000000000000000
x13: 00000000da535970 x12: 0000000000000038
x11: 0000000000000028 x10: 0101010101010101
x9 : ff63647371607372 x8 : 0000000000000085
x7 : 0000000000007df5 x6 : 00000000da512e1c
x5 : 00000000da513518 x4 : 0000000000000002
x3 : 00000000da513920 x2 : 0000000000000000
x1 : 0000000000000008 x0 : 00000000da513490


Attachments:
(No filename) (5.55 kB)
mykill.tar.gz (1.05 kB)
trigo.tar.gz (1.18 kB)
Download all attachments

2016-03-20 08:13:44

by Zhangjian (Bamvor)

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

Hi, Yury

On 2016/3/19 0:46, Yury Norov wrote:
> On Fri, Mar 18, 2016 at 04:55:26PM +0100, Alexander Graf wrote:
>>
>>
>> On 18.03.16 16:49, Yury Norov wrote:
>>> On Fri, Mar 18, 2016 at 06:28:29PM +0800, Zhangjian (Bamvor) wrote:
>>>>
>>>> For the glibc part, I found that there are 11 patches of ilp32 in top,
>>>> but the original 28 patches of ilp32 is not in the top, there are more
>>>> than 900 patches between them(referece the list below). Are you
>>>> willing rebase all the ilp32 relative patches. It is very useful for
>>>> reviewing and debugging. I saw andrew request the account in glibc,
>>>> maybe it has already been in processs?).
>>>>
>>>
>>> I already told there's mess there, and I'd prefer to make things work
>>> first and then do cleanup.
>>
>> So how is progress going overall? The last submission I've seen is
>> already 2 months ago. Are there particular bits holding you up?
>>
>>
>> Alex
>
> Hi Alexander,
>
> For last time I mostly work on library, as it needs to be reworked
> well. But yes, there's one serious bug puzzling me.
>
> Tests like umount or pathconf fail but I see no major problem with
> it, as it's most probably structure padding mismatch between kernel and
> glibc. But there's (at least) one major problem I see.
>
> Float tests fail due to NULL-dereferencing (0x14 actually) at
> pthread_join(). It calls tgkill(), and after that child thread crashes.
> See stack trace at the end.
>
> The minimal test reproducing it is attached. The similar test where
> parent forks a child and then kills it, works fine. (Attached too).
>
> I see that in case of pthread, there's much more stuff that is cloned.
> Other's looking similar.
>
> pthread_create():
> clone(child_stack=0xb953cea0, flags=CLONE_VM|CLONE_FS|CLONE_FILES
> |CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS
> |CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
> parent_tidptr=0xb953d398, tls=0xb953d7c0, child_tidptr=0xb953d398) = 1650
>
> fork():
> clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD,
> child_tidptr=0xe5af6278) = 30537
>
> So this most probably means that ilp32 code doesn't handle one of cloned
> item properly. I have already discovered a bug where child processes
> used parent TLS,
It is a kernel bug or glibc bug? Could you please explain it or show the patch?
The current ILP32 patches looks good to me. Recently, I backport these patches
to our 4.1 kernel. And I saw crash frequently even if I only do a single print
or infinite loop. There is some small changes about tls register after 4.1. I
am not sure if it is a similar issue. It is great if you have some suggestions/
ideas.

Thanks.

Bamvor
> so maybe this is something similar...
>
> Except of this, I think ILP32 series is looking pretty well, at least
> kernel part.
>
> If you have any ideas/suggestions, I'll really appreciate it.
>
> Yury.
>
> strace -f ./trigo
> [...]
> clone(child_stack=0xdbbfb000,
> flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND
> |CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS
> |CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
> parent_tidptr=0xdbbfb4f8, tls=0xdbbfb920, child_tidptr=0xdbbfb4f8) = 32030
> rt_sigprocmask(SIG_BLOCK, [CHLD], Process 32030 attached [], 8) = 0
> [pid 32029] rt_sigaction(SIGCHLD, NULL, <unfinished ...>
> [pid 32030] set_robust_list(0xdbbfb504, 12 <unfinished ...>
> [pid 32029] <... rt_sigaction resumed> {SIG_DFL, [ILL ABRT SEGV URG], 0}, 8) = 0
> [pid 32030] <... set_robust_list resumed> ) = 0
> [pid 32029] rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
> [pid 32030] write(1, "started\n", 8started
> <unfinished ...>
> [pid 32029] nanosleep({1, 65536}, <unfinished ...>
> [pid 32030] <... write resumed> ) = 8
> [pid 32030] rt_sigprocmask(SIG_BLOCK, NULL, [], 8) = 0
> [pid 32030] rt_sigsuspend([] <unfinished ...>
> [pid 32029] <... nanosleep resumed> 0xfff9fd98) = 0
> [pid 32029] write(1, "stoping...\n", 11stoping...) = 11
> [pid 32029] openat(AT_FDCWD, "/root/sys-root/libilp32/libgcc_s.so.1", O_RDONLY|O_CLOEXEC) = 3
> [pid 32029] read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\267\0\1\0\0\0 \0\0004\0\0\0"..., 512) = 512
> [pid 32029] fstat(3, {st_mode=S_IFREG|0644, st_size=429138, ...}) = 0
> [pid 32029] mmap(NULL, 135104, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xdb3db000
> [pid 32029] mprotect(0xdb3ec000, 61440, PROT_NONE) = 0
> [pid 32029] mmap(0xdb3fb000, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x10000) = 0xdb3fb000
> [pid 32029] close(3) = 0
> [pid 32029] tgkill(32029, 32030, SIGRTMIN) = 0
> [pid 32030] <... rt_sigsuspend resumed> ) = ? ERESTARTNOHAND (To be
> restarted if no handler)
> [pid 32029] write(1, "pthread_cancel == 0\n", 20pthread_cancel == 0) = 20
> [pid 32030] --- SIGRTMIN {si_signo=SIGRTMIN, si_code=SI_TKILL, si_pid=32029, si_uid=0} ---
> [pid 32029] write(1, "stopped\n", 8stopped
> <unfinished ...>
> [pid 32030] --- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0x14} ---
> [pid 32029] <... write resumed> ) = ? <unavailable>
> [pid 32030] +++ killed by SIGSEGV +++
> +++ killed by SIGSEGV +++
> Segmentation fault
>
> dmesg:
> trigo[32246]: unhandled level 2 translation fault (11) at 0x00000014,
> esr 0x90000006
> pgd = ffffffc009335000
> [00000014] *pgd=000000007917c003, *pud=000000007917c003,
> *pmd=0000000000000000
>
> CPU: 2 PID: 32246 Comm: trigo Not tainted 4.5.0+ #91
> Hardware name: linux,dummy-virt (DT)
> task: ffffffc00900e400 ti: ffffffc009078000 task.ti: ffffffc009078000
> PC is at 0xda6853f0
> LR is at 0xda6d5440
> pc : [<00000000da6853f0>] lr : [<00000000da6d5440>] pstate: 60000000
> sp : 00000000da511bc0
> x29: 00000000da512e10 x28: 00000000da6a7000
> x27: 0000000000000000 x26: 00000000da513490
> x25: 0000000000000000 x24: 0000000000400820
> x23: 00000000da6a9000 x22: 00000000ff869acb
> x21: 00000000da6a9000 x20: 00000000da512e50
> x19: 0000000000000000 x18: 0000000000000001
> x17: 0000000000410bd8 x16: 00000000da691138
> x15: 0000000000000000 x14: 0000000000000000
> x13: 00000000da535970 x12: 0000000000000038
> x11: 0000000000000028 x10: 0101010101010101
> x9 : ff63647371607372 x8 : 0000000000000085
> x7 : 0000000000007df5 x6 : 00000000da512e1c
> x5 : 00000000da513518 x4 : 0000000000000002
> x3 : 00000000da513920 x2 : 0000000000000000
> x1 : 0000000000000008 x0 : 00000000da513490
>

2016-03-21 09:08:01

by Andreas Schwab

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

This patch may fix a few LTP tests.

Andreas.

diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
index 3631903..d1010db 100644
--- a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
+++ b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
@@ -25,18 +25,29 @@
#define __O_NOFOLLOW 0100000
#define __O_DIRECT 0200000

-#define __O_LARGEFILE 0
+#ifdef __ILP32__
+# define __O_LARGEFILE 0400000
+#else
+# define __O_LARGEFILE 0
+#endif

+#ifndef __ILP32__
# define F_GETLK64 5
# define F_SETLK64 6
# define F_SETLKW64 7
+#endif

struct flock
{
short int l_type; /* Type of lock: F_RDLCK, F_WRLCK, or F_UNLCK. */
short int l_whence; /* Where `l_start' is relative to (like `lseek'). */
+#ifndef __USE_FILE_OFFSET64
__off_t l_start; /* Offset where the lock begins. */
__off_t l_len; /* Size of the locked area; zero means until EOF. */
+#else
+ __off64_t l_start; /* Offset where the lock begins. */
+ __off64_t l_len; /* Size of the locked area; zero means until EOF. */
+#endif
__pid_t l_pid; /* Process holding the lock. */
};

--
2.7.3

--
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

2016-03-21 09:45:00

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

On Monday 21 March 2016 10:07:49 Andreas Schwab wrote:
> This patch may fix a few LTP tests.
>

Thanks for analyzing.

> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
> index 3631903..d1010db 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
> @@ -25,18 +25,29 @@
> #define __O_NOFOLLOW 0100000
> #define __O_DIRECT 0200000
>
> -#define __O_LARGEFILE 0
> +#ifdef __ILP32__
> +# define __O_LARGEFILE 0400000
> +#else
> +# define __O_LARGEFILE 0
> +#endif
>

I guess this means I screwed up when I said I'd merged the kernel patch
that Yury did to fix it, sorry about that.

We need the patch to make all new architecture in the kernel default to
O_LARGEFILE, and not do this in user space. I'd suggest now to keep the
patches as part of the ILP32 series after all, to make sure they are
merged at the point when they are needed.

> +#ifndef __ILP32__
> # define F_GETLK64 5
> # define F_SETLK64 6
> # define F_SETLKW64 7
> +#endif
>
> struct flock
> {
> short int l_type; /* Type of lock: F_RDLCK, F_WRLCK, or F_UNLCK. */
> short int l_whence; /* Where `l_start' is relative to (like `lseek'). */
> +#ifndef __USE_FILE_OFFSET64
> __off_t l_start; /* Offset where the lock begins. */
> __off_t l_len; /* Size of the locked area; zero means until EOF. */
> +#else
> + __off64_t l_start; /* Offset where the lock begins. */
> + __off64_t l_len; /* Size of the locked area; zero means until EOF. */
> +#endif
> __pid_t l_pid; /* Process holding the lock. */
> };

This looks like there is another bug as well, but I think this is in
libc, not in the kernel. I'm sure we had discussed this at some point
but I forgot what the outcome was. Defining 'struct flock' to have a
32-bit l_start and l_len member cannot be right if the kernel only
supports 64-bit offsets.

My guess is that the libc should either not define __off_t at all for
ILP32, and always use __off64_t in struct flock, or __off_t should
be defined as __kernel_loff_t a.k.a. long long so the #ifdef can be
avoided.

What exactly do you need to define F_GETLK64 for on LP64?

Arnd

2016-03-21 10:53:06

by Andreas Schwab

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

Arnd Bergmann <[email protected]> writes:

> What exactly do you need to define F_GETLK64 for on LP64?

To override the generic definitions.

Andreas.

--
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

2016-03-21 11:25:52

by Zhangjian (Bamvor)

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

Hi, Yury

On 2016/3/20 16:12, Zhangjian (Bamvor) wrote:
> Hi, Yury
>
> On 2016/3/19 0:46, Yury Norov wrote:
[...]
>> The minimal test reproducing it is attached. The similar test where
>> parent forks a child and then kills it, works fine. (Attached too).
>>
>> I see that in case of pthread, there's much more stuff that is cloned.
>> Other's looking similar.
>>
>> pthread_create():
>> clone(child_stack=0xb953cea0, flags=CLONE_VM|CLONE_FS|CLONE_FILES
>> |CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS
>> |CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
>> parent_tidptr=0xb953d398, tls=0xb953d7c0, child_tidptr=0xb953d398) = 1650
>>
>> fork():
>> clone(child_stack=0, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD,
>> child_tidptr=0xe5af6278) = 30537
>>
>> So this most probably means that ilp32 code doesn't handle one of cloned
>> item properly. I have already discovered a bug where child processes
>> used parent TLS,
> It is a kernel bug or glibc bug? Could you please explain it or show the patch?
> The current ILP32 patches looks good to me. Recently, I backport these patches
> to our 4.1 kernel. And I saw crash frequently even if I only do a single print
> or infinite loop. There is some small changes about tls register after 4.1. I
> am not sure if it is a similar issue. It is great if you have some suggestions/
> ideas.
My issue is because I forget to change is_compat_task to
is_a32_compat_task in arch/arm64/kernel/process.c such piece of code
is delete after commit d00a3810c162 ("arm64: context-switch user tls
register tpidr_el0 for compat tasks). It is not exist in upstream
kernel, never mind.

Meanwhile, I found that it seem that there is another is_compat_task
in tls_thread_flush. Is it relative the issue you mentioned?

```
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 432b094..9ab968c 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -209,7 +209,7 @@ static void tls_thread_flush(void)
{
asm ("msr tpidr_el0, xzr");

- if (is_compat_task()) {
+ if (is_a32_compat_task()) {
current->thread.tp_value = 0;

/*
```

Regards

Bamvor

> Thanks.
>
> Bamvor
> > so maybe this is something similar...
>>
>> Except of this, I think ILP32 series is looking pretty well, at least
>> kernel part.
>>
>> If you have any ideas/suggestions, I'll really appreciate it.
>>
>> Yury.
>>
>> strace -f ./trigo
>> [...]
>> clone(child_stack=0xdbbfb000,
>> flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND
>> |CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS
>> |CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
>> parent_tidptr=0xdbbfb4f8, tls=0xdbbfb920, child_tidptr=0xdbbfb4f8) = 32030
>> rt_sigprocmask(SIG_BLOCK, [CHLD], Process 32030 attached [], 8) = 0
>> [pid 32029] rt_sigaction(SIGCHLD, NULL, <unfinished ...>
>> [pid 32030] set_robust_list(0xdbbfb504, 12 <unfinished ...>
>> [pid 32029] <... rt_sigaction resumed> {SIG_DFL, [ILL ABRT SEGV URG], 0}, 8) = 0
>> [pid 32030] <... set_robust_list resumed> ) = 0
>> [pid 32029] rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
>> [pid 32030] write(1, "started\n", 8started
>> <unfinished ...>
>> [pid 32029] nanosleep({1, 65536}, <unfinished ...>
>> [pid 32030] <... write resumed> ) = 8
>> [pid 32030] rt_sigprocmask(SIG_BLOCK, NULL, [], 8) = 0
>> [pid 32030] rt_sigsuspend([] <unfinished ...>
>> [pid 32029] <... nanosleep resumed> 0xfff9fd98) = 0
>> [pid 32029] write(1, "stoping...\n", 11stoping...) = 11
>> [pid 32029] openat(AT_FDCWD, "/root/sys-root/libilp32/libgcc_s.so.1", O_RDONLY|O_CLOEXEC) = 3
>> [pid 32029] read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\267\0\1\0\0\0 \0\0004\0\0\0"..., 512) = 512
>> [pid 32029] fstat(3, {st_mode=S_IFREG|0644, st_size=429138, ...}) = 0
>> [pid 32029] mmap(NULL, 135104, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xdb3db000
>> [pid 32029] mprotect(0xdb3ec000, 61440, PROT_NONE) = 0
>> [pid 32029] mmap(0xdb3fb000, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x10000) = 0xdb3fb000
>> [pid 32029] close(3) = 0
>> [pid 32029] tgkill(32029, 32030, SIGRTMIN) = 0
>> [pid 32030] <... rt_sigsuspend resumed> ) = ? ERESTARTNOHAND (To be
>> restarted if no handler)
>> [pid 32029] write(1, "pthread_cancel == 0\n", 20pthread_cancel == 0) = 20
>> [pid 32030] --- SIGRTMIN {si_signo=SIGRTMIN, si_code=SI_TKILL, si_pid=32029, si_uid=0} ---
>> [pid 32029] write(1, "stopped\n", 8stopped
>> <unfinished ...>
>> [pid 32030] --- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0x14} ---
>> [pid 32029] <... write resumed> ) = ? <unavailable>
>> [pid 32030] +++ killed by SIGSEGV +++
>> +++ killed by SIGSEGV +++
>> Segmentation fault
>>
>> dmesg:
>> trigo[32246]: unhandled level 2 translation fault (11) at 0x00000014,
>> esr 0x90000006
>> pgd = ffffffc009335000
>> [00000014] *pgd=000000007917c003, *pud=000000007917c003,
>> *pmd=0000000000000000
>>
>> CPU: 2 PID: 32246 Comm: trigo Not tainted 4.5.0+ #91
>> Hardware name: linux,dummy-virt (DT)
>> task: ffffffc00900e400 ti: ffffffc009078000 task.ti: ffffffc009078000
>> PC is at 0xda6853f0
>> LR is at 0xda6d5440
>> pc : [<00000000da6853f0>] lr : [<00000000da6d5440>] pstate: 60000000
>> sp : 00000000da511bc0
>> x29: 00000000da512e10 x28: 00000000da6a7000
>> x27: 0000000000000000 x26: 00000000da513490
>> x25: 0000000000000000 x24: 0000000000400820
>> x23: 00000000da6a9000 x22: 00000000ff869acb
>> x21: 00000000da6a9000 x20: 00000000da512e50
>> x19: 0000000000000000 x18: 0000000000000001
>> x17: 0000000000410bd8 x16: 00000000da691138
>> x15: 0000000000000000 x14: 0000000000000000
>> x13: 00000000da535970 x12: 0000000000000038
>> x11: 0000000000000028 x10: 0101010101010101
>> x9 : ff63647371607372 x8 : 0000000000000085
>> x7 : 0000000000007df5 x6 : 00000000da512e1c
>> x5 : 00000000da513518 x4 : 0000000000000002
>> x3 : 00000000da513920 x2 : 0000000000000000
>> x1 : 0000000000000008 x0 : 00000000da513490
>>
>

2016-03-21 17:04:06

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

On Monday 21 March 2016 11:52:54 Andreas Schwab wrote:
> Arnd Bergmann <[email protected]> writes:
>
> > What exactly do you need to define F_GETLK64 for on LP64?
>
> To override the generic definitions.


Ok, got it. I misread that part as adding definitions for LP64, but it
is correctly removing the definitions for ILP32.

Arnd

2016-03-21 19:18:56

by Yury Norov

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

On Mon, Mar 21, 2016 at 07:23:28PM +0800, Zhangjian (Bamvor) wrote:
> >>So this most probably means that ilp32 code doesn't handle one of cloned
> >>item properly. I have already discovered a bug where child processes
> >>used parent TLS,
> >It is a kernel bug or glibc bug? Could you please explain it or show the patch?
> >The current ILP32 patches looks good to me. Recently, I backport these patches
> >to our 4.1 kernel. And I saw crash frequently even if I only do a single print
> >or infinite loop. There is some small changes about tls register after 4.1. I
> >am not sure if it is a similar issue. It is great if you have some suggestions/
> >ideas.
> My issue is because I forget to change is_compat_task to
> is_a32_compat_task in arch/arm64/kernel/process.c such piece of code
> is delete after commit d00a3810c162 ("arm64: context-switch user tls
> register tpidr_el0 for compat tasks). It is not exist in upstream
> kernel, never mind.
>
> Meanwhile, I found that it seem that there is another is_compat_task
> in tls_thread_flush. Is it relative the issue you mentioned?
>
> ```
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 432b094..9ab968c 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -209,7 +209,7 @@ static void tls_thread_flush(void)
> {
> asm ("msr tpidr_el0, xzr");
>
> - if (is_compat_task()) {
> + if (is_a32_compat_task()) {
> current->thread.tp_value = 0;
>
> /*
> ```
>
> Regards
>
> Bamvor

Hi,

This fix looks correct, though doesn't fix issue.
Thank you.

Yury.

2016-03-21 19:22:20

by Yury Norov

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

On Mon, Mar 21, 2016 at 10:07:49AM +0100, Andreas Schwab wrote:
> This patch may fix a few LTP tests.
>
> Andreas.
>
> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
> index 3631903..d1010db 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
> @@ -25,18 +25,29 @@
> #define __O_NOFOLLOW 0100000
> #define __O_DIRECT 0200000
>
> -#define __O_LARGEFILE 0
> +#ifdef __ILP32__
> +# define __O_LARGEFILE 0400000
> +#else
> +# define __O_LARGEFILE 0
> +#endif
>
> +#ifndef __ILP32__
> # define F_GETLK64 5
> # define F_SETLK64 6
> # define F_SETLKW64 7
> +#endif
>
> struct flock
> {
> short int l_type; /* Type of lock: F_RDLCK, F_WRLCK, or F_UNLCK. */
> short int l_whence; /* Where `l_start' is relative to (like `lseek'). */
> +#ifndef __USE_FILE_OFFSET64
> __off_t l_start; /* Offset where the lock begins. */
> __off_t l_len; /* Size of the locked area; zero means until EOF. */
> +#else
> + __off64_t l_start; /* Offset where the lock begins. */
> + __off64_t l_len; /* Size of the locked area; zero means until EOF. */
> +#endif
> __pid_t l_pid; /* Process holding the lock. */
> };
>
> --
> 2.7.3
>
> --
> Andreas Schwab, SUSE Labs, [email protected]
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."

Hi Andreas,

Thank you for your patch. It seems like it fixed a couple of tests.
I applied it to the library branch. Current list of fails is like this:
float_bessel FAIL 137
float_exp_log FAIL 137
float_iperb FAIL 137
float_power FAIL 137
float_trigo FAIL 137
pipeio_3 FAIL 5
abort01 FAIL 2
clone02 FAIL 4
kill10 FAIL 2
kill11 FAIL 2
mmap16 FAIL 6
nftw01 FAIL 1
nftw6401 FAIL 1
open12 FAIL 2
pathconf01 FAIL 1
profil01 FAIL 1
rename11 FAIL 2
rmdir02 FAIL 2
umount2_01 FAIL 2
umount2_02 FAIL 2
umount2_03 FAIL 2
utime06 FAIL 2
mtest06 FAIL 11

Yury.

2016-03-22 01:50:23

by Yury Norov

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

On Mon, Mar 21, 2016 at 09:43:12PM +0300, Yury Norov wrote:
> On Mon, Mar 21, 2016 at 07:23:28PM +0800, Zhangjian (Bamvor) wrote:
> > >>So this most probably means that ilp32 code doesn't handle one of cloned
> > >>item properly. I have already discovered a bug where child processes
> > >>used parent TLS,
> > >It is a kernel bug or glibc bug? Could you please explain it or show the patch?
> > >The current ILP32 patches looks good to me. Recently, I backport these patches
> > >to our 4.1 kernel. And I saw crash frequently even if I only do a single print
> > >or infinite loop. There is some small changes about tls register after 4.1. I
> > >am not sure if it is a similar issue. It is great if you have some suggestions/
> > >ideas.
> > My issue is because I forget to change is_compat_task to
> > is_a32_compat_task in arch/arm64/kernel/process.c such piece of code
> > is delete after commit d00a3810c162 ("arm64: context-switch user tls
> > register tpidr_el0 for compat tasks). It is not exist in upstream
> > kernel, never mind.
> >
> > Meanwhile, I found that it seem that there is another is_compat_task
> > in tls_thread_flush. Is it relative the issue you mentioned?
> >
> > ```
> > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> > index 432b094..9ab968c 100644
> > --- a/arch/arm64/kernel/process.c
> > +++ b/arch/arm64/kernel/process.c
> > @@ -209,7 +209,7 @@ static void tls_thread_flush(void)
> > {
> > asm ("msr tpidr_el0, xzr");
> >
> > - if (is_compat_task()) {
> > + if (is_a32_compat_task()) {
> > current->thread.tp_value = 0;
> >
> > /*
> > ```
> >
> > Regards
> >
> > Bamvor
>
> Hi,
>
> This fix looks correct, though doesn't fix issue.
> Thank you.
>
> Yury.

Hi again.

Next fix helps with SIGSEGV crash of trigo test. But now it hangs on
futex, so work is not finished yet. Nevertheless, you can apply it and
do your tests.

Signed-off-by: Yury Norov <[email protected]>
---
arch/arm64/kernel/signal_ilp32.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kernel/signal_ilp32.c b/arch/arm64/kernel/signal_ilp32.c
index 455b0fb..1bb0ea8 100644
--- a/arch/arm64/kernel/signal_ilp32.c
+++ b/arch/arm64/kernel/signal_ilp32.c
@@ -107,6 +107,7 @@ int ilp32_setup_rt_frame(int usig, struct ksignal *ksig,

if (!frame)
return 1;
+ err |= copy_siginfo_to_user32(&frame->info, &ksig->info);

__put_user_error(0, &frame->sig.uc.uc_flags, err);
__put_user_error(NULL, &frame->sig.uc.uc_link, err);
@@ -115,12 +116,9 @@ int ilp32_setup_rt_frame(int usig, struct ksignal *ksig,
err |= setup_sigframe(&frame->sig, regs, set);
if (err == 0) {
setup_return(regs, &ksig->ka, frame,
- offsetof(struct ilp32_rt_sigframe, sig), usig);
- if (ksig->ka.sa.sa_flags & SA_SIGINFO) {
- err |= copy_siginfo_to_user32(&frame->info, &ksig->info);
- regs->regs[1] = (unsigned long)&frame->info;
- regs->regs[2] = (unsigned long)&frame->sig.uc;
- }
+ offsetof(struct ilp32_rt_sigframe, sig), usig);
+ regs->regs[1] = (unsigned long)&frame->info;
+ regs->regs[2] = (unsigned long)&frame->sig.uc;
}

return err;
--
2.5.0

2016-03-26 13:17:31

by Zhangjian (Bamvor)

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

Hi, Yury

On 2016/3/22 2:40, Yury Norov wrote:
> On Mon, Mar 21, 2016 at 10:07:49AM +0100, Andreas Schwab wrote:
[...]
>
> Hi Andreas,
>
> Thank you for your patch. It seems like it fixed a couple of tests.
> I applied it to the library branch. Current list of fails is like this:
> float_bessel FAIL 137
> float_exp_log FAIL 137
> float_iperb FAIL 137
> float_power FAIL 137
> float_trigo FAIL 137
> pipeio_3 FAIL 5
> abort01 FAIL 2
> clone02 FAIL 4
> kill10 FAIL 2
> kill11 FAIL 2
> mmap16 FAIL 6
> nftw01 FAIL 1
> nftw6401 FAIL 1
> open12 FAIL 2
> pathconf01 FAIL 1
> profil01 FAIL 1
> rename11 FAIL 2
> rmdir02 FAIL 2
> umount2_01 FAIL 2
> umount2_02 FAIL 2
> umount2_03 FAIL 2
> utime06 FAIL 2
> mtest06 FAIL 11
This is a patch for glibc.
I found 64bit register is used in sysdep.h. It could fix some failure
in bigendian. I do not test it on little endian yet. Hope It helps.

Regards

Bamvor
From a4af2b7a8903ac5e033ba838ec3328bdeb1113ba Mon Sep 17 00:00:00 2001
From: Yang Yingliang <[email protected]>
Date: Thu, 13 Nov 2014 16:05:58 +0800
Subject: [PATCH] ARM64: ILP32: change register x1 to PTR_REG

It should use 32-bit register instead of 64-bit register in ILP32.

Suggested-by: Andrew Pinski <[email protected]>
Signed-off-by: Yang Yingliang <[email protected]>
Signed-off-by: Bamvor Jian Zhang <[email protected]>
---
sysdeps/aarch64/sysdep.h | 2 +-
sysdeps/unix/sysv/linux/aarch64/sysdep.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
index 6673242..742d23c 100644
--- a/sysdeps/aarch64/sysdep.h
+++ b/sysdeps/aarch64/sysdep.h
@@ -104,7 +104,7 @@
#define LDST_GLOBAL(OP, R, T, EXPR) \
adrp x##T, :got:EXPR; \
ldr PTR_REG (T), [x##T, #:got_lo12:EXPR]; \
- OP x##R, [x##T];
+ OP PTR_REG (R), [x##T];

/* Since C identifiers are not normally prefixed with an underscore
on this system, the asm identifier `syscall_error' intrudes on the
diff --git a/sysdeps/unix/sysv/linux/aarch64/sysdep.h b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
index 2bfec77..8fb8a6b 100644
--- a/sysdeps/unix/sysv/linux/aarch64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
@@ -108,7 +108,7 @@
.Lsyscall_error: \
adrp x1, :gottprel:errno; \
neg w2, w0; \
- ldr x1, [x1, :gottprel_lo12:errno]; \
+ ldr PTR_REG(1), [x1, :gottprel_lo12:errno]; \
mrs x3, tpidr_el0; \
mov x0, -1; \
str w2, [x1, x3]; \
--
1.8.4.5


> Yury.
>

2016-03-26 13:47:18

by Zhangjian (Bamvor)

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

Hi, guys

Does any body test the bigendian? We found lots of failures in be in
our arm64 hardware. E.g. the signal issue.

IIUC, the signal of struct in ILP32 is align with the aarch32. If so,
we need to revert the following patch wrote by Andrew in 2014 which
align the kernel_sigaction of ilp32 to lp64:
Revert "[AARCH64] Add kernel_sigaction.h for AARCH64 ILP32"

And we also need to handle the uc_stack properly in kernel. After
apply these two patches, we could fix lots of failure in bigendian.

Regards

Bamvor

From cb08043a1f14eb997892711c2e1e5016b0e9eef6 Mon Sep 17 00:00:00 2001
From: Bamvor Jian Zhang <[email protected]>
Date: Thu, 24 Mar 2016 10:40:47 +0800
Subject: [PATCH] try to fix the signal issue in be

Currently, there is different layout in uc_stack in le and be.
Try to fix the issue by processing uc_stack through corresponding
compat function.

Signed-off-by: Bamvor Jian Zhang <[email protected]>
---
arch/arm64/kernel/signal_ilp32.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/signal_ilp32.c b/arch/arm64/kernel/signal_ilp32.c
index 26b3121..9af29e0 100644
--- a/arch/arm64/kernel/signal_ilp32.c
+++ b/arch/arm64/kernel/signal_ilp32.c
@@ -60,7 +60,7 @@ asmlinkage int ilp32_sys_rt_sigreturn(struct pt_regs *regs)
if (restore_sigframe(regs, &frame->sig))
goto badframe;

- if (restore_altstack(&frame->sig.uc.uc_stack))
+ if (compat_restore_altstack(&frame->sig.uc.uc_stack))
goto badframe;

return regs->regs[0];
@@ -112,7 +112,7 @@ int ilp32_setup_rt_frame(int usig, struct ksignal *ksig,
__put_user_error(0, &frame->sig.uc.uc_flags, err);
__put_user_error(NULL, &frame->sig.uc.uc_link, err);

- err |= __save_altstack(&frame->sig.uc.uc_stack, regs->sp);
+ err |= __compat_save_altstack(&frame->sig.uc.uc_stack, regs->sp);
err |= setup_sigframe(&frame->sig, regs, set);
if (err == 0) {
setup_return(regs, &ksig->ka, frame,
--
1.8.4.5

From f6cde6e2a75a4b153758eea679c5a839fc1c39d2 Mon Sep 17 00:00:00 2001
From: "Zhang Jian(Bamvor)" <[email protected]>
Date: Sat, 26 Mar 2016 18:10:38 +0800
Subject: [PATCH] Revert "[AARCH64] Add kernel_sigaction.h for AARCH64 ILP32"

This reverts commit 68e11d8643cfd08a62cea3555e92d77a21bf41de.
---
sysdeps/unix/sysv/linux/aarch64/kernel_sigaction.h | 12 ------------
sysdeps/unix/sysv/linux/aarch64/sigaction.c | 10 ++++------
2 files changed, 4 insertions(+), 18 deletions(-)
delete mode 100644 sysdeps/unix/sysv/linux/aarch64/kernel_sigaction.h

diff --git a/sysdeps/unix/sysv/linux/aarch64/kernel_sigaction.h b/sysdeps/unix/sysv/linux/aarch64/kernel_sigaction.h
deleted file mode 100644
index 7b3023b..0000000
--- a/sysdeps/unix/sysv/linux/aarch64/kernel_sigaction.h
+++ /dev/null
@@ -1,12 +0,0 @@
-
-#define HAVE_SA_RESTORER
-
-/* This is the sigaction structure in aarch64 kernel.
- Note the ILP32 struct uses the same struct as LP64
- which is why the fields are 64bit in size. */
-struct kernel_sigaction {
- unsigned long long k_sa_handler;
- unsigned long long sa_flags;
- unsigned long long sa_restorer;
- sigset_t sa_mask;
-};
diff --git a/sysdeps/unix/sysv/linux/aarch64/sigaction.c b/sysdeps/unix/sysv/linux/aarch64/sigaction.c
index 5d22b68..2679acd 100644
--- a/sysdeps/unix/sysv/linux/aarch64/sigaction.c
+++ b/sysdeps/unix/sysv/linux/aarch64/sigaction.c
@@ -39,17 +39,15 @@ __libc_sigaction (int sig, const struct sigaction *act, struct sigaction *oact)

if (act)
{
- kact.k_sa_handler = (unsigned long long)(uintptr_t)act->sa_handler;
+ kact.k_sa_handler = act->sa_handler;
memcpy (&kact.sa_mask, &act->sa_mask, sizeof (sigset_t));
kact.sa_flags = act->sa_flags;
#ifdef HAVE_SA_RESTORER
if (kact.sa_flags & SA_RESTORER)
- kact.sa_restorer = (unsigned long long)(uintptr_t)act->sa_restorer;
+ kact.sa_restorer = act->sa_restorer;
#endif
}

- /* This is needed for ILP32 as the structures are two different sizes due to
- using the LP64 structure. */
result = INLINE_SYSCALL (rt_sigaction, 4, sig,
act ? &kact : NULL,
oact ? &koact : NULL, _NSIG / 8);
@@ -57,11 +55,11 @@ __libc_sigaction (int sig, const struct sigaction *act, struct sigaction *oact)
{
if (oact && result >= 0)
{
- oact->sa_handler = (void*)(uintptr_t)koact.k_sa_handler;
+ oact->sa_handler = koact.k_sa_handler;
memcpy (&oact->sa_mask, &koact.sa_mask, sizeof (sigset_t));
oact->sa_flags = koact.sa_flags;
#ifdef HAVE_SA_RESTORER
- oact->sa_restorer = (void*)(uintptr_t)koact.sa_restorer;
+ oact->sa_restorer = koact.sa_restorer;
#endif
}
}
--
1.8.4.5


2016-03-26 12:37:21

by Zhangjian (Bamvor)

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

Hi, Arnd

On 2016/3/21 17:43, Arnd Bergmann wrote:
> On Monday 21 March 2016 10:07:49 Andreas Schwab wrote:
>> This patch may fix a few LTP tests.
>>
>
> Thanks for analyzing.
>
>> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
>> index 3631903..d1010db 100644
>> --- a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
>> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
>> @@ -25,18 +25,29 @@
>> #define __O_NOFOLLOW 0100000
>> #define __O_DIRECT 0200000
>>
>> -#define __O_LARGEFILE 0
>> +#ifdef __ILP32__
>> +# define __O_LARGEFILE 0400000
>> +#else
>> +# define __O_LARGEFILE 0
>> +#endif
>>
>
> I guess this means I screwed up when I said I'd merged the kernel patch
> that Yury did to fix it, sorry about that.
>
> We need the patch to make all new architecture in the kernel default to
> O_LARGEFILE, and not do this in user space. I'd suggest now to keep the
> patches as part of the ILP32 series after all, to make sure they are
> merged at the point when they are needed.

I am a little bit confuse about off_t. In "[PATCH 08/33] 32-bit
ABI: introduce ARCH_32BIT_OFF_T config option", it mentioned that all
the new 32bit architecture should use 64bit off_t.

Should we define off_t in aarch64(for both ilp32 and lp64) in
typesize.h as following?

diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
index 7073493..13b77c5 100644
--- a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
+++ b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
@@ -33,7 +33,7 @@
#define __INO64_T_TYPE __UQUAD_TYPE
#define __MODE_T_TYPE __U32_TYPE
#define __NLINK_T_TYPE __U32_TYPE
-#define __OFF_T_TYPE __SLONGWORD_TYPE
+#define __OFF_T_TYPE __SQUAD_TYPE
#define __OFF64_T_TYPE __SQUAD_TYPE
#define __PID_T_TYPE __S32_TYPE
#define __RLIM_T_TYPE __ULONGWORD_TYPE

Then we could remove the __USE_FILE_OFFSET64 in stat.h and fcnt.h in
aarch64. And truncate and ftruncate is same as truncate64 and
ftruncate64.

Otherwise we need to handle the pad like yury do it in
stat.h, and we need to handle the bigendian as well:

From 61949bf70527b9cc450e7bbdba9182f7f120c5bd Sun March 26 00:00:00 2016
From: j00321192 <[email protected]>
Date: Thu, 24 Mar 2016 22:10:25 +0800
Subject: [PATCH] Fix endian issue in struct stat and stat64

There is endian issue in the existence bits/stat.h in aarch64. Fix
it by add the __AARCH64EB__ with proper pad.

Tested in our arm64 hardware. It could fix all the *stat test cases.

Signed-off-by: Yongliang Gao <[email protected]>
Signed-off-by: Jun Ji <[email protected]>
Signed-off-by: Yang Liu(Young) <[email protected]>
Signed-off-by: Zhang Jian(Bamvor) <[email protected]>
---
sysdeps/unix/sysv/linux/aarch64/bits/stat.h | 38 +++++++++++++++++++++++++----
1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/stat.h b/sysdeps/unix/sysv/linux/aarch64/bits/stat.h
index 3d50e7a..4c6e072 100644
--- a/sysdeps/unix/sysv/linux/aarch64/bits/stat.h
+++ b/sysdeps/unix/sysv/linux/aarch64/bits/stat.h
@@ -35,12 +35,21 @@ struct stat
{
__dev_t st_dev; /* Device. */
#ifdef __ILP32__
+
+#if !defined(__AARCH64EB__)
unsigned int __st_ino_pad;
+#endif
+
# ifndef __USE_FILE_OFFSET64
__ino_t st_ino; /* File serial number. */
# else
__ino_t __st_ino; /* 32bit file serial number. */
# endif
+
+#if defined(__AARCH64EB__)
+ unsigned int __st_ino_pad;
+#endif
+
#else
# ifndef __USE_FILE_OFFSET64
__ino_t st_ino; /* File serial number. */
@@ -55,10 +64,17 @@ struct stat
__dev_t st_rdev; /* Device number, if device. */
__dev_t __pad1;
#ifndef __USE_FILE_OFFSET64
+
+#if defined(__ILP32__) && defined(__AARCH64EB__)
+ int __st_size_pad;
+#endif
+
__off_t st_size; /* Size of file, in bytes. */
-# ifdef __ILP32__
+
+#if defined(__ILP32__) && !defined(__AARCH64EB__)
int __st_size_pad;
-# endif
+#endif
+
#else
__off64_t st_size; /* Size of file, in bytes. */
#endif
@@ -66,10 +82,17 @@ struct stat
int __pad2;

#ifndef __USE_FILE_OFFSET64
+
+#if defined (__ILP32__) && defined(__AARCH64EB__)
+ int __st_blocks_pad;
+#endif
+
__blkcnt_t st_blocks; /* Number 512-byte blocks allocated. */
-# ifdef __ILP32__
+
+#if defined (__ILP32__) && !defined(__AARCH64EB__)
int __st_blocks_pad;
-# endif
+#endif
+
#else
__blkcnt64_t st_blocks; /* Number 512-byte blocks allocated. */
#endif
@@ -94,7 +117,7 @@ struct stat
__time_t st_ctime; /* Time of last status change. */
unsigned long int st_ctimensec; /* Nsecs of last status change. */
#endif
-#if !defined __ILP32__ || !defined __USE_FILE_OFFSET64
+#if !defined (__ILP32__) || !defined __USE_FILE_OFFSET64
int __glibc_reserved[2];
#else
__ino64_t st_ino; /* File serial number. */
@@ -106,8 +129,13 @@ struct stat64
{
__dev_t st_dev; /* Device. */
# ifdef __ILP32__
+ #if !defined(__AARCH64EB__)
unsigned int __st_ino_pad;
+ #endif
__ino_t __st_ino; /* 32bit file serial number. */
+ #if defined(__AARCH64EB__)
+ unsigned int __st_ino_pad;
+ #endif
# else
__ino64_t st_ino; /* File serial number. */
# endif
--
1.8.4.5

Regards

bamvor


>> +#ifndef __ILP32__
>> # define F_GETLK64 5
>> # define F_SETLK64 6
>> # define F_SETLKW64 7
>> +#endif
>>
>> struct flock
>> {
>> short int l_type; /* Type of lock: F_RDLCK, F_WRLCK, or F_UNLCK. */
>> short int l_whence; /* Where `l_start' is relative to (like `lseek'). */
>> +#ifndef __USE_FILE_OFFSET64
>> __off_t l_start; /* Offset where the lock begins. */
>> __off_t l_len; /* Size of the locked area; zero means until EOF. */
>> +#else
>> + __off64_t l_start; /* Offset where the lock begins. */
>> + __off64_t l_len; /* Size of the locked area; zero means until EOF. */
>> +#endif
>> __pid_t l_pid; /* Process holding the lock. */
>> };
>
> This looks like there is another bug as well, but I think this is in
> libc, not in the kernel. I'm sure we had discussed this at some point
> but I forgot what the outcome was. Defining 'struct flock' to have a
> 32-bit l_start and l_len member cannot be right if the kernel only
> supports 64-bit offsets.
>
> My guess is that the libc should either not define __off_t at all for
> ILP32, and always use __off64_t in struct flock, or __off_t should
> be defined as __kernel_loff_t a.k.a. long long so the #ifdef can be
> avoided.
>
> What exactly do you need to define F_GETLK64 for on LP64?
>
> Arnd
>

2016-03-26 22:46:39

by Yury Norov

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

On Sat, Mar 26, 2016 at 09:45:53PM +0800, Zhangjian (Bamvor) wrote:
> Hi, guys

Hi,
>
> Does any body test the bigendian? We found lots of failures in be in
> our arm64 hardware. E.g. the signal issue.

I'm afraid, nobody yet. Thank you for work on it.

>
> IIUC, the signal of struct in ILP32 is align with the aarch32. If so,
> we need to revert the following patch wrote by Andrew in 2014 which
> align the kernel_sigaction of ilp32 to lp64:
> Revert "[AARCH64] Add kernel_sigaction.h for AARCH64 ILP32"
>
> And we also need to handle the uc_stack properly in kernel. After
> apply these two patches, we could fix lots of failure in bigendian.
>
> Regards
>
> Bamvor

Andrew sent me similar patches yesterday. See
https://github.com/apinski-cavium/linux (branch pin1)
https://github.com/norov/glibc.git (branch new-api)

There are also some other fixes, so my fail list is like this:
[Float tests skipped but fail too]
pipeio_1 FAIL 5
pipeio_6 FAIL 1
abort01 FAIL 2
clone02 FAIL 4
execve03 FAIL 4
fcntl17 FAIL 4
fcntl21 FAIL 4
kill11 FAIL 2
mmap16 FAIL 6
open12 FAIL 2
rename11 FAIL 2
rmdir02 FAIL 2
umount2_01 FAIL 2
umount2_02 FAIL 2
umount2_03 FAIL 2
utime06 FAIL 2
mtest06 FAIL 11

Some tests fail both on lp64 and ilp32, so it seems, it's not a
problem of ilp32 itself.

Some tests fail only when run in scenario, so I cannot reproduce it.
I suspect it's because core dump fails.

Also, Andrew told, there's an issue in unwind, and it is caused by GCC
bug, not kernel or glibc. To reproduce, run unwind from signal
handler (trigo does it). I think, at least float tests fail due to it.
Andrew is in vacation now, so he may answer longer than usual.

See Andrew's kernel signal patch:

>From b95c5250c5c869d8852886ed49e34fa11c29663e Mon Sep 17 00:00:00 2001
From: Andrew Pinski <[email protected]>
Date: Thu, 24 Mar 2016 23:10:08 -0700
Subject: [PATCH] Fix signals

Signed-off-by: Andrew Pinski <[email protected]>
Signed-off-by: Yury Norov <[email protected]>
---
arch/arm64/include/asm/signal_common.h | 10 +----
arch/arm64/kernel/signal.c | 59 +++++++++++++++++--------
arch/arm64/kernel/signal_ilp32.c | 78 +++++++++++++++++++++++++++++++---
3 files changed, 115 insertions(+), 32 deletions(-)

diff --git a/arch/arm64/include/asm/signal_common.h b/arch/arm64/include/asm/signal_common.h
index faa82c0..402e0c4 100644
--- a/arch/arm64/include/asm/signal_common.h
+++ b/arch/arm64/include/asm/signal_common.h
@@ -23,16 +23,10 @@
#include <asm/ucontext.h>
#include <asm/fpsimd.h>

-struct sigframe {
- struct ucontext uc;
- u64 fp;
- u64 lr;
-};
-
int preserve_fpsimd_context(struct fpsimd_context __user *ctx);
int restore_fpsimd_context(struct fpsimd_context __user *ctx);
-int setup_sigframe(struct sigframe __user *sf, struct pt_regs *regs, sigset_t *set);
-int restore_sigframe(struct pt_regs *regs, struct sigframe __user *sf);
+int setup_sigcontex(struct sigcontext __user *uc_mcontext, struct pt_regs *regs);
+int restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sf);
void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
void __user *frame, off_t sigframe_off, int usig);

diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index 0648aa5..5f2faf2 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -37,6 +37,12 @@
#include <asm/signal_common.h>
#include <asm/signal_ilp32.h>

+struct sigframe {
+ struct ucontext uc;
+ u64 fp;
+ u64 lr;
+};
+
/*
* Do a signal return; undo the signal stack. These are aligned to 128-bit.
*/
@@ -92,23 +98,31 @@ int restore_fpsimd_context(struct fpsimd_context __user *ctx)
return err ? -EFAULT : 0;
}

-int restore_sigframe(struct pt_regs *regs,
+static int restore_sigframe(struct pt_regs *regs,
struct sigframe __user *sf)
{
sigset_t set;
- int i, err;
- void *aux = sf->uc.uc_mcontext.__reserved;
-
+ int err;
err = __copy_from_user(&set, &sf->uc.uc_sigmask, sizeof(set));
if (err == 0)
set_current_blocked(&set);

+ err |= restore_sigcontext(regs, &sf->uc.uc_mcontext);
+ return err;
+}
+
+
+int restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *uc_mcontext)
+{
+ int i, err = 0;
+ void *aux = uc_mcontext->__reserved;
+
for (i = 0; i < 31; i++)
- __get_user_error(regs->regs[i], &sf->uc.uc_mcontext.regs[i],
+ __get_user_error(regs->regs[i], &uc_mcontext->regs[i],
err);
- __get_user_error(regs->sp, &sf->uc.uc_mcontext.sp, err);
- __get_user_error(regs->pc, &sf->uc.uc_mcontext.pc, err);
- __get_user_error(regs->pstate, &sf->uc.uc_mcontext.pstate, err);
+ __get_user_error(regs->sp, &uc_mcontext->sp, err);
+ __get_user_error(regs->pc, &uc_mcontext->pc, err);
+ __get_user_error(regs->pstate, &uc_mcontext->pstate, err);

/*
* Avoid sys_rt_sigreturn() restarting.
@@ -162,27 +176,36 @@ badframe:
return 0;
}

-int setup_sigframe(struct sigframe __user *sf,
+static int setup_sigframe(struct sigframe __user *sf,
struct pt_regs *regs, sigset_t *set)
{
- int i, err = 0;
- void *aux = sf->uc.uc_mcontext.__reserved;
- struct _aarch64_ctx *end;
+ int err = 0;

/* set up the stack frame for unwinding */
__put_user_error(regs->regs[29], &sf->fp, err);
__put_user_error(regs->regs[30], &sf->lr, err);
+ err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set));
+ err |= setup_sigcontex (&sf->uc.uc_mcontext, regs);
+
+ return err;
+}
+
+int setup_sigcontex(struct sigcontext __user *uc_mcontext,
+ struct pt_regs *regs)
+{
+ void *aux = uc_mcontext->__reserved;
+ struct _aarch64_ctx *end;
+ int i, err = 0;

for (i = 0; i < 31; i++)
- __put_user_error(regs->regs[i], &sf->uc.uc_mcontext.regs[i],
+ __put_user_error(regs->regs[i], &uc_mcontext->regs[i],
err);
- __put_user_error(regs->sp, &sf->uc.uc_mcontext.sp, err);
- __put_user_error(regs->pc, &sf->uc.uc_mcontext.pc, err);
- __put_user_error(regs->pstate, &sf->uc.uc_mcontext.pstate, err);

- __put_user_error(current->thread.fault_address, &sf->uc.uc_mcontext.fault_address, err);
+ __put_user_error(regs->sp, &uc_mcontext->sp, err);
+ __put_user_error(regs->pc, &uc_mcontext->pc, err);
+ __put_user_error(regs->pstate, &uc_mcontext->pstate, err);

- err |= __copy_to_user(&sf->uc.uc_sigmask, set, sizeof(*set));
+ __put_user_error(current->thread.fault_address, &uc_mcontext->fault_address, err);

if (err == 0) {
struct fpsimd_context *fpsimd_ctx =
diff --git a/arch/arm64/kernel/signal_ilp32.c b/arch/arm64/kernel/signal_ilp32.c
index 1bb0ea8..d399ed0 100644
--- a/arch/arm64/kernel/signal_ilp32.c
+++ b/arch/arm64/kernel/signal_ilp32.c
@@ -32,11 +32,76 @@
#include <asm/unistd.h>
#include <asm/ucontext.h>

+
+struct ilp32_ucontext {
+ u32 uc_flags;
+ u32 uc_link;
+ compat_stack_t uc_stack;
+ compat_sigset_t uc_sigmask;
+ /* glibc uses a 1024-bit sigset_t */
+ __u8 __unused[1024 / 8 - sizeof(compat_sigset_t)];
+ /* last for future expansion */
+ struct sigcontext uc_mcontext;
+};
+
+struct ilp32_sigframe {
+ struct ilp32_ucontext uc;
+ u64 fp;
+ u64 lr;
+};
+
struct ilp32_rt_sigframe {
struct compat_siginfo info;
- struct sigframe sig;
+ struct ilp32_sigframe sig;
};

+static inline int put_sigset_t(compat_sigset_t __user *uset, sigset_t *set)
+{
+ compat_sigset_t cset;
+
+ cset.sig[0] = set->sig[0] & 0xffffffffull;
+ cset.sig[1] = set->sig[0] >> 32;
+
+ return copy_to_user(uset, &cset, sizeof(*uset));
+}
+
+static inline int get_sigset_t(sigset_t *set,
+ const compat_sigset_t __user *uset)
+{
+ compat_sigset_t s32;
+
+ if (copy_from_user(&s32, uset, sizeof(*uset)))
+ return -EFAULT;
+
+ set->sig[0] = s32.sig[0] | (((long)s32.sig[1]) << 32);
+ return 0;
+}
+
+static int restore_ilp32_sigframe(struct pt_regs *regs,
+ struct ilp32_sigframe __user *sf)
+{
+ sigset_t set;
+ int err;
+ err = get_sigset_t(&set, &sf->uc.uc_sigmask);
+ if (err == 0)
+ set_current_blocked(&set);
+ err |= restore_sigcontext(regs, &sf->uc.uc_mcontext);
+ return err;
+}
+
+static int setup_ilp32_sigframe(struct ilp32_sigframe __user *sf,
+ struct pt_regs *regs, sigset_t *set)
+{
+ int err = 0;
+ /* set up the stack frame for unwinding */
+ __put_user_error(regs->regs[29], &sf->fp, err);
+ __put_user_error(regs->regs[30], &sf->lr, err);
+
+ err |= put_sigset_t(&sf->uc.uc_sigmask, set);
+ err |= setup_sigcontex (&sf->uc.uc_mcontext, regs);
+ return err;
+}
+
asmlinkage long ilp32_sys_rt_sigreturn(struct pt_regs *regs)
{
struct ilp32_rt_sigframe __user *frame;
@@ -57,10 +122,10 @@ asmlinkage long ilp32_sys_rt_sigreturn(struct pt_regs *regs)
if (!access_ok(VERIFY_READ, frame, sizeof (*frame)))
goto badframe;

- if (restore_sigframe(regs, &frame->sig))
+ if (restore_ilp32_sigframe(regs, &frame->sig))
goto badframe;

- if (restore_altstack(&frame->sig.uc.uc_stack))
+ if (compat_restore_altstack(&frame->sig.uc.uc_stack))
goto badframe;

return regs->regs[0];
@@ -107,13 +172,14 @@ int ilp32_setup_rt_frame(int usig, struct ksignal *ksig,

if (!frame)
return 1;
+
err |= copy_siginfo_to_user32(&frame->info, &ksig->info);

__put_user_error(0, &frame->sig.uc.uc_flags, err);
- __put_user_error(NULL, &frame->sig.uc.uc_link, err);
+ __put_user_error(0, &frame->sig.uc.uc_link, err);

- err |= __save_altstack(&frame->sig.uc.uc_stack, regs->sp);
- err |= setup_sigframe(&frame->sig, regs, set);
+ err |= __compat_save_altstack(&frame->sig.uc.uc_stack, regs->sp);
+ err |= setup_ilp32_sigframe(&frame->sig, regs, set);
if (err == 0) {
setup_return(regs, &ksig->ka, frame,
offsetof(struct ilp32_rt_sigframe, sig), usig);
--
2.5.0

2016-03-29 10:59:28

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

On Saturday 26 March 2016 20:36:43 Zhangjian wrote:
> Hi, Arnd
>
> On 2016/3/21 17:43, Arnd Bergmann wrote:
> > On Monday 21 March 2016 10:07:49 Andreas Schwab wrote:
> >> This patch may fix a few LTP tests.
> >>
> >
> > Thanks for analyzing.
> >
> >> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
> >> index 3631903..d1010db 100644
> >> --- a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
> >> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
> >> @@ -25,18 +25,29 @@
> >> #define __O_NOFOLLOW 0100000
> >> #define __O_DIRECT 0200000
> >>
> >> -#define __O_LARGEFILE 0
> >> +#ifdef __ILP32__
> >> +# define __O_LARGEFILE 0400000
> >> +#else
> >> +# define __O_LARGEFILE 0
> >> +#endif
> >>
> >
> > I guess this means I screwed up when I said I'd merged the kernel patch
> > that Yury did to fix it, sorry about that.
> >
> > We need the patch to make all new architecture in the kernel default to
> > O_LARGEFILE, and not do this in user space. I'd suggest now to keep the
> > patches as part of the ILP32 series after all, to make sure they are
> > merged at the point when they are needed.
>
> I am a little bit confuse about off_t. In "[PATCH 08/33] 32-bit
> ABI: introduce ARCH_32BIT_OFF_T config option", it mentioned that all
> the new 32bit architecture should use 64bit off_t.

Ah, so it is part of the series. I had not checked that here.

> Should we define off_t in aarch64(for both ilp32 and lp64) in
> typesize.h as following?
>
> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
> index 7073493..13b77c5 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
> @@ -33,7 +33,7 @@
> #define __INO64_T_TYPE __UQUAD_TYPE
> #define __MODE_T_TYPE __U32_TYPE
> #define __NLINK_T_TYPE __U32_TYPE
> -#define __OFF_T_TYPE __SLONGWORD_TYPE
> +#define __OFF_T_TYPE __SQUAD_TYPE
> #define __OFF64_T_TYPE __SQUAD_TYPE
> #define __PID_T_TYPE __S32_TYPE
> #define __RLIM_T_TYPE __ULONGWORD_TYPE
>
> Then we could remove the __USE_FILE_OFFSET64 in stat.h and fcnt.h in
> aarch64. And truncate and ftruncate is same as truncate64 and
> ftruncate64.

I don't know what the glibc developers prefer, but I think the
result needs to be something like that: either __OFF_T_TYPE is
defined as you write above as a 64-bit type, or the user-visible
off_t typedef unconditionally uses __OFF64_T_TYPE rather than
__OFF_T_TYPE.

> Otherwise we need to handle the pad like yury do it in
> stat.h, and we need to handle the bigendian as well:

I see.

> @@ -35,12 +35,21 @@ struct stat
> {
> __dev_t st_dev; /* Device. */
> #ifdef __ILP32__
> +
> +#if !defined(__AARCH64EB__)
> unsigned int __st_ino_pad;
> +#endif
> +
> # ifndef __USE_FILE_OFFSET64
> __ino_t st_ino; /* File serial number. */
> # else
> __ino_t __st_ino; /* 32bit file serial number. */
> # endif
> +
> +#if defined(__AARCH64EB__)
> + unsigned int __st_ino_pad;
> +#endif
> +
> #else

This would indeed be silly, we really don't want anyone
to access the old __st_ino field or the 32-bit version of
the offset here.

Arnd

2016-03-29 12:02:08

by Yury Norov

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

On Tue, Mar 29, 2016 at 12:58:25PM +0200, Arnd Bergmann wrote:
> On Saturday 26 March 2016 20:36:43 Zhangjian wrote:
> > Hi, Arnd
> >
> > On 2016/3/21 17:43, Arnd Bergmann wrote:
> > > On Monday 21 March 2016 10:07:49 Andreas Schwab wrote:
> > >> This patch may fix a few LTP tests.
> > >>
> > >
> > > Thanks for analyzing.
> > >
> > >> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
> > >> index 3631903..d1010db 100644
> > >> --- a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
> > >> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
> > >> @@ -25,18 +25,29 @@
> > >> #define __O_NOFOLLOW 0100000
> > >> #define __O_DIRECT 0200000
> > >>
> > >> -#define __O_LARGEFILE 0
> > >> +#ifdef __ILP32__
> > >> +# define __O_LARGEFILE 0400000
> > >> +#else
> > >> +# define __O_LARGEFILE 0
> > >> +#endif
> > >>
> > >
> > > I guess this means I screwed up when I said I'd merged the kernel patch
> > > that Yury did to fix it, sorry about that.
> > >
> > > We need the patch to make all new architecture in the kernel default to
> > > O_LARGEFILE, and not do this in user space. I'd suggest now to keep the
> > > patches as part of the ILP32 series after all, to make sure they are
> > > merged at the point when they are needed.
> >
> > I am a little bit confuse about off_t. In "[PATCH 08/33] 32-bit
> > ABI: introduce ARCH_32BIT_OFF_T config option", it mentioned that all
> > the new 32bit architecture should use 64bit off_t.
>
> Ah, so it is part of the series. I had not checked that here.
>

I'm preparing new submission now. I can join off_t, s390 and ilp32
patchsets. It seems, they will not be grabbed separately anyway, so
this may decrease confusions like this.

Arnd?

> > Should we define off_t in aarch64(for both ilp32 and lp64) in
> > typesize.h as following?
> >
> > diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
> > index 7073493..13b77c5 100644
> > --- a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
> > +++ b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
> > @@ -33,7 +33,7 @@
> > #define __INO64_T_TYPE __UQUAD_TYPE
> > #define __MODE_T_TYPE __U32_TYPE
> > #define __NLINK_T_TYPE __U32_TYPE
> > -#define __OFF_T_TYPE __SLONGWORD_TYPE
> > +#define __OFF_T_TYPE __SQUAD_TYPE
> > #define __OFF64_T_TYPE __SQUAD_TYPE
> > #define __PID_T_TYPE __S32_TYPE
> > #define __RLIM_T_TYPE __ULONGWORD_TYPE
> >
> > Then we could remove the __USE_FILE_OFFSET64 in stat.h and fcnt.h in
> > aarch64. And truncate and ftruncate is same as truncate64 and
> > ftruncate64.
>
> I don't know what the glibc developers prefer, but I think the
> result needs to be something like that: either __OFF_T_TYPE is
> defined as you write above as a 64-bit type, or the user-visible
> off_t typedef unconditionally uses __OFF64_T_TYPE rather than
> __OFF_T_TYPE.
>

I'm not the glibc developer as well, but I think it's OK.

> > Otherwise we need to handle the pad like yury do it in
> > stat.h, and we need to handle the bigendian as well:
>
> I see.
>
> > @@ -35,12 +35,21 @@ struct stat
> > {
> > __dev_t st_dev; /* Device. */
> > #ifdef __ILP32__
> > +
> > +#if !defined(__AARCH64EB__)
> > unsigned int __st_ino_pad;
> > +#endif
> > +
> > # ifndef __USE_FILE_OFFSET64
> > __ino_t st_ino; /* File serial number. */
> > # else
> > __ino_t __st_ino; /* 32bit file serial number. */
> > # endif
> > +
> > +#if defined(__AARCH64EB__)
> > + unsigned int __st_ino_pad;
> > +#endif
> > +
> > #else
>
> This would indeed be silly, we really don't want anyone
> to access the old __st_ino field or the 32-bit version of
> the offset here.
>
> Arnd

2016-03-29 12:43:08

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

On Tuesday 29 March 2016 15:01:47 Yury Norov wrote:
> On Tue, Mar 29, 2016 at 12:58:25PM +0200, Arnd Bergmann wrote:
> > On Saturday 26 March 2016 20:36:43 Zhangjian wrote:
> > >
> > > I am a little bit confuse about off_t. In "[PATCH 08/33] 32-bit
> > > ABI: introduce ARCH_32BIT_OFF_T config option", it mentioned that all
> > > the new 32bit architecture should use 64bit off_t.
> >
> > Ah, so it is part of the series. I had not checked that here.
> >
>
> I'm preparing new submission now. I can join off_t, s390 and ilp32
> patchsets. It seems, they will not be grabbed separately anyway, so
> this may decrease confusions like this.
>
> Arnd?

Yes, that sounds good.

> > > Should we define off_t in aarch64(for both ilp32 and lp64) in
> > > typesize.h as following?
> > >
> > > diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
> > > index 7073493..13b77c5 100644
> > > --- a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
> > > +++ b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
> > > @@ -33,7 +33,7 @@
> > > #define __INO64_T_TYPE __UQUAD_TYPE
> > > #define __MODE_T_TYPE __U32_TYPE
> > > #define __NLINK_T_TYPE __U32_TYPE
> > > -#define __OFF_T_TYPE __SLONGWORD_TYPE
> > > +#define __OFF_T_TYPE __SQUAD_TYPE
> > > #define __OFF64_T_TYPE __SQUAD_TYPE
> > > #define __PID_T_TYPE __S32_TYPE
> > > #define __RLIM_T_TYPE __ULONGWORD_TYPE
> > >
> > > Then we could remove the __USE_FILE_OFFSET64 in stat.h and fcnt.h in
> > > aarch64. And truncate and ftruncate is same as truncate64 and
> > > ftruncate64.
> >
> > I don't know what the glibc developers prefer, but I think the
> > result needs to be something like that: either __OFF_T_TYPE is
> > defined as you write above as a 64-bit type, or the user-visible
> > off_t typedef unconditionally uses __OFF64_T_TYPE rather than
> > __OFF_T_TYPE.
> >
>
> I'm not the glibc developer as well, but I think it's OK.

Which of the two? I guess with the example that Bamvor gave
regarding struct stat, the latter is what we want, forcing the
use of __USE_FILE_OFFSET64 rather than changing the definition of
__OFF_T_TYPE.

Arnd

2016-03-29 13:23:45

by Zhangjian (Bamvor)

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

Hi, Yury

On 2016/3/29 20:01, Yury Norov wrote:
> On Tue, Mar 29, 2016 at 12:58:25PM +0200, Arnd Bergmann wrote:
>> On Saturday 26 March 2016 20:36:43 Zhangjian wrote:
>>> Hi, Arnd
>>>
>>> On 2016/3/21 17:43, Arnd Bergmann wrote:
>>>> On Monday 21 March 2016 10:07:49 Andreas Schwab wrote:
>>>>> This patch may fix a few LTP tests.
>>>>>
>>>>
>>>> Thanks for analyzing.
>>>>
>>>>> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
>>>>> index 3631903..d1010db 100644
>>>>> --- a/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
>>>>> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/fcntl.h
>>>>> @@ -25,18 +25,29 @@
>>>>> #define __O_NOFOLLOW 0100000
>>>>> #define __O_DIRECT 0200000
>>>>>
>>>>> -#define __O_LARGEFILE 0
>>>>> +#ifdef __ILP32__
>>>>> +# define __O_LARGEFILE 0400000
>>>>> +#else
>>>>> +# define __O_LARGEFILE 0
>>>>> +#endif
>>>>>
>>>>
>>>> I guess this means I screwed up when I said I'd merged the kernel patch
>>>> that Yury did to fix it, sorry about that.
>>>>
>>>> We need the patch to make all new architecture in the kernel default to
>>>> O_LARGEFILE, and not do this in user space. I'd suggest now to keep the
>>>> patches as part of the ILP32 series after all, to make sure they are
>>>> merged at the point when they are needed.
>>>
>>> I am a little bit confuse about off_t. In "[PATCH 08/33] 32-bit
>>> ABI: introduce ARCH_32BIT_OFF_T config option", it mentioned that all
>>> the new 32bit architecture should use 64bit off_t.
>>
>> Ah, so it is part of the series. I had not checked that here.
>>
>
> I'm preparing new submission now.
Cool:)
> I can join off_t, s390 and ilp32
> patchsets. It seems, they will not be grabbed separately anyway, so
> this may decrease confusions like this.
>
> Arnd?
I am curious which one is more easily to get ack:p
>
>>> Should we define off_t in aarch64(for both ilp32 and lp64) in
>>> typesize.h as following?
>>>
>>> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
>>> index 7073493..13b77c5 100644
>>> --- a/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
>>> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/typesizes.h
>>> @@ -33,7 +33,7 @@
>>> #define __INO64_T_TYPE __UQUAD_TYPE
>>> #define __MODE_T_TYPE __U32_TYPE
>>> #define __NLINK_T_TYPE __U32_TYPE
>>> -#define __OFF_T_TYPE __SLONGWORD_TYPE
>>> +#define __OFF_T_TYPE __SQUAD_TYPE
>>> #define __OFF64_T_TYPE __SQUAD_TYPE
>>> #define __PID_T_TYPE __S32_TYPE
>>> #define __RLIM_T_TYPE __ULONGWORD_TYPE
>>>
>>> Then we could remove the __USE_FILE_OFFSET64 in stat.h and fcnt.h in
>>> aarch64. And truncate and ftruncate is same as truncate64 and
>>> ftruncate64.
>>
>> I don't know what the glibc developers prefer, but I think the
>> result needs to be something like that: either __OFF_T_TYPE is
>> defined as you write above as a 64-bit type, or the user-visible
>> off_t typedef unconditionally uses __OFF64_T_TYPE rather than
>> __OFF_T_TYPE.
>>
>
> I'm not the glibc developer as well, but I think it's OK.
IIUC, it is usually what glibc does.
If we want to define off_t to 64bit in ilp32, the follow syscall may
need to define as non-compat too:
sys_fadvise64
sys_sendfile
sys_sendfile64
sys_lseek
sys_splice
sys_sync_file_range2
sys_truncate
sys_ftruncate

Regards

Bamvor

>
>>> Otherwise we need to handle the pad like yury do it in
>>> stat.h, and we need to handle the bigendian as well:
>>
>> I see.
>>
>>> @@ -35,12 +35,21 @@ struct stat
>>> {
>>> __dev_t st_dev; /* Device. */
>>> #ifdef __ILP32__
>>> +
>>> +#if !defined(__AARCH64EB__)
>>> unsigned int __st_ino_pad;
>>> +#endif
>>> +
>>> # ifndef __USE_FILE_OFFSET64
>>> __ino_t st_ino; /* File serial number. */
>>> # else
>>> __ino_t __st_ino; /* 32bit file serial number. */
>>> # endif
>>> +
>>> +#if defined(__AARCH64EB__)
>>> + unsigned int __st_ino_pad;
>>> +#endif
>>> +
>>> #else
>>
>> This would indeed be silly, we really don't want anyone
>> to access the old __st_ino field or the 32-bit version of
>> the offset here.
>>
>> Arnd

2016-03-29 13:28:21

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

On Tuesday 29 March 2016 21:21:49 Zhangjian wrote:
> >>>
> >>> Then we could remove the __USE_FILE_OFFSET64 in stat.h and fcnt.h in
> >>> aarch64. And truncate and ftruncate is same as truncate64 and
> >>> ftruncate64.
> >>
> >> I don't know what the glibc developers prefer, but I think the
> >> result needs to be something like that: either __OFF_T_TYPE is
> >> defined as you write above as a 64-bit type, or the user-visible
> >> off_t typedef unconditionally uses __OFF64_T_TYPE rather than
> >> __OFF_T_TYPE.
> >>
> >
> > I'm not the glibc developer as well, but I think it's OK.
> IIUC, it is usually what glibc does.
> If we want to define off_t to 64bit in ilp32, the follow syscall may
> need to define as non-compat too:
> sys_fadvise64
> sys_sendfile
> sys_sendfile64
> sys_lseek
> sys_splice
> sys_sync_file_range2
> sys_truncate
> sys_ftruncate

I'm not following here. Do you mean in the kernel or in glibc?

In the kernel, the list of syscalls is fine, because we already only
provide syscalls passing loff_t as I said, and that is 64-bit.

In glibc, I think we need to define fewer entry points, not more.
Instead of having both lseek and lseek64, only one of them should
be provided, and that should always take a 64-bit offset, calling
into the kernel with the _llseek syscall entry.

Arnd

2016-03-29 15:55:02

by Joseph Myers

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

On Tue, 29 Mar 2016, Arnd Bergmann wrote:

> In glibc, I think we need to define fewer entry points, not more.
> Instead of having both lseek and lseek64, only one of them should
> be provided, and that should always take a 64-bit offset, calling
> into the kernel with the _llseek syscall entry.

lseek64 is part of the public API, on all platforms. It should be aliased
to lseek where possible.

Strictly, it would be possible to provide it in the API without it being
part of the ABI, by arranging the headers so that calls to lseek64 result
in objects with a reference to lseek (because it uses the off64_t typedef,
it's not valid to declare it yourself rather than including a header that
declares it). I don't think it would be a good idea for a new
sub-architecture port to try introducing such a difference from all other
ports, however.

--
Joseph S. Myers
[email protected]

2016-03-29 19:31:43

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

On Tuesday 29 March 2016 15:54:52 Joseph Myers wrote:
> On Tue, 29 Mar 2016, Arnd Bergmann wrote:
>
> > In glibc, I think we need to define fewer entry points, not more.
> > Instead of having both lseek and lseek64, only one of them should
> > be provided, and that should always take a 64-bit offset, calling
> > into the kernel with the _llseek syscall entry.
>
> lseek64 is part of the public API, on all platforms. It should be aliased
> to lseek where possible.

Right, makes sense.

> Strictly, it would be possible to provide it in the API without it being
> part of the ABI, by arranging the headers so that calls to lseek64 result
> in objects with a reference to lseek (because it uses the off64_t typedef,
> it's not valid to declare it yourself rather than including a header that
> declares it). I don't think it would be a good idea for a new
> sub-architecture port to try introducing such a difference from all other
> ports, however.

How do we do it then? Should we just define __USE_FILE_OFFSET64
unconditionally for all new 32-bit architectures and leave the
code dealing with 32-bit off_t/ino_t in place but unreachable, to
minimize the differences?

Or should all the obsolete types be defined the same way as their
replacements so we have 64-bit __OFF_T_TYPE/__INO_T_TYPE
and use the same binary implementation regardless of FILE_OFFSET_BITS?

Arnd

2016-03-29 20:15:22

by Joseph Myers

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

On Tue, 29 Mar 2016, Arnd Bergmann wrote:

> How do we do it then? Should we just define __USE_FILE_OFFSET64
> unconditionally for all new 32-bit architectures and leave the
> code dealing with 32-bit off_t/ino_t in place but unreachable, to
> minimize the differences?

Defining __USE_FILE_OFFSET64 unconditionally would prevent glibc from
building (see: how the patches a while back prototyping changing the
default had to disable the change when glibc itself is built). A change
in the default, though desired (someone needs to pick up those patches
together with the analysis done of possible impact on distributions),
should not be tied to a new port, and would need to be discussed
thoroughly on libc-alpha.

> Or should all the obsolete types be defined the same way as their
> replacements so we have 64-bit __OFF_T_TYPE/__INO_T_TYPE
> and use the same binary implementation regardless of FILE_OFFSET_BITS?

I think so (along with using wordsize-64 sysdeps directories as far as
possible, like x32 does). But design questions for a glibc port really
belong on libc-alpha to get any sort of community consensus.

--
Joseph S. Myers
[email protected]

2016-03-29 20:25:08

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

On Tuesday 29 March 2016 20:15:10 Joseph Myers wrote:
> On Tue, 29 Mar 2016, Arnd Bergmann wrote:
>
> > How do we do it then? Should we just define __USE_FILE_OFFSET64
> > unconditionally for all new 32-bit architectures and leave the
> > code dealing with 32-bit off_t/ino_t in place but unreachable, to
> > minimize the differences?
>
> Defining __USE_FILE_OFFSET64 unconditionally would prevent glibc from
> building (see: how the patches a while back prototyping changing the
> default had to disable the change when glibc itself is built). A change
> in the default, though desired (someone needs to pick up those patches
> together with the analysis done of possible impact on distributions),
> should not be tied to a new port, and would need to be discussed
> thoroughly on libc-alpha.

Ok

> > Or should all the obsolete types be defined the same way as their
> > replacements so we have 64-bit __OFF_T_TYPE/__INO_T_TYPE
> > and use the same binary implementation regardless of FILE_OFFSET_BITS?
>
> I think so (along with using wordsize-64 sysdeps directories as far as
> possible, like x32 does). But design questions for a glibc port really
> belong on libc-alpha to get any sort of community consensus.

I thought the wordsize-64 stuff was for the x86 mode where they
define __kernel_long_t as 64-bit. We don't really want to do that in
the kernel for new 32-bit architectures, that would make the kernel
ABI different from all the existing architectures.

The kernel ABI for ilp32 follows the usual wordsize-32 definitions
to make it easy for glibc while avoiding the problems that came from
redefining __kernel_long_t.

Arnd

2016-03-29 21:01:03

by Joseph Myers

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

On Tue, 29 Mar 2016, Arnd Bergmann wrote:

> > I think so (along with using wordsize-64 sysdeps directories as far as
> > possible, like x32 does). But design questions for a glibc port really
> > belong on libc-alpha to get any sort of community consensus.
>
> I thought the wordsize-64 stuff was for the x86 mode where they
> define __kernel_long_t as 64-bit. We don't really want to do that in
> the kernel for new 32-bit architectures, that would make the kernel
> ABI different from all the existing architectures.

In general the wordsize-64 directories cover various relations of the form
"function X is an alias for function Y", which derive from "type X is
ABI-compatible with type Y". (Unfortunately, the precise set isn't
well-defined, resulting in problems for cases that want a subset of those
relations - e.g. MIPS n64 where struct stat and struct stat64 are
different, and so sysdeps/unix/sysv/linux/wordsize-64 isn't used.)

The person doing the port will need to do a detailed review of the exact
effects of the wordsize-64 directories in current glibc, and which of
those effects are appropriate for this port, to determine what is
appropriate, and to include that analysis with the port submission.

Many of the relations relate to things controlled by _FILE_OFFSET_BITS=64
- if _FILE_OFFSET_BITS=64 does not affect the ABI of off_t, struct stat,
etc., then many of the aliases are correct. Some relations may relate to
other things such as long and long long being ABI compatible - where an
alias in the correct direction can be OK for long arguments (not returns)
if a long argument is always sign-extended to long long when passed to a
function, for example.

--
Joseph S. Myers
[email protected]

2016-03-29 21:40:44

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64

On Tuesday 29 March 2016 21:00:56 Joseph Myers wrote:
> On Tue, 29 Mar 2016, Arnd Bergmann wrote:
>
> > > I think so (along with using wordsize-64 sysdeps directories as far as
> > > possible, like x32 does). But design questions for a glibc port really
> > > belong on libc-alpha to get any sort of community consensus.
> >
> > I thought the wordsize-64 stuff was for the x86 mode where they
> > define __kernel_long_t as 64-bit. We don't really want to do that in
> > the kernel for new 32-bit architectures, that would make the kernel
> > ABI different from all the existing architectures.
>
> In general the wordsize-64 directories cover various relations of the form
> "function X is an alias for function Y", which derive from "type X is
> ABI-compatible with type Y". (Unfortunately, the precise set isn't
> well-defined, resulting in problems for cases that want a subset of those
> relations - e.g. MIPS n64 where struct stat and struct stat64 are
> different, and so sysdeps/unix/sysv/linux/wordsize-64 isn't used.)

For all new 32-bit architectures like this, the kernel should at least
behave in a consistent way, but it's somewhere inbetween wordsize-32 and
wordsize-64 for 32-bit architectures, because off_t and ino_t are mapped
to the 64-bit __kernel_loff_t and __kernel_ino_t, while time_t, clock_t
and size_t are mapped to 32 bit types.

> The person doing the port will need to do a detailed review of the exact
> effects of the wordsize-64 directories in current glibc, and which of
> those effects are appropriate for this port, to determine what is
> appropriate, and to include that analysis with the port submission.

I think the analysis will have to be about two separate things:

* Whether new 32-bit architectures in general should use wordsize-32
or wordsize-64 or something else, based on what you write above.
I would still guess that we are better off adapting wordsize-32
to the current kernel ABI for 32-bit architectures while leaving
wordsize-64 to real 64-bit architectures and x86/x32.

* How we deal with the special case of this architecture having
nonstandard calling conventions for a couple of syscalls that
take 64-bit arguments in a single register rather than two registers
as every other 32-bit architecture does.

Arnd

2016-03-31 07:37:43

by Zhangjian (Bamvor)

[permalink] [raw]
Subject: Re: [RFC5 PATCH v6 00/21] ILP32 for ARM64



On 2016/3/29 21:27, Arnd Bergmann wrote:
> On Tuesday 29 March 2016 21:21:49 Zhangjian wrote:
>>>>>
>>>>> Then we could remove the __USE_FILE_OFFSET64 in stat.h and fcnt.h in
>>>>> aarch64. And truncate and ftruncate is same as truncate64 and
>>>>> ftruncate64.
>>>>
>>>> I don't know what the glibc developers prefer, but I think the
>>>> result needs to be something like that: either __OFF_T_TYPE is
>>>> defined as you write above as a 64-bit type, or the user-visible
>>>> off_t typedef unconditionally uses __OFF64_T_TYPE rather than
>>>> __OFF_T_TYPE.
>>>>
>>>
>>> I'm not the glibc developer as well, but I think it's OK.
>> IIUC, it is usually what glibc does.
>> If we want to define off_t to 64bit in ilp32, the follow syscall may
>> need to define as non-compat too:
>> sys_fadvise64
>> sys_sendfile
>> sys_sendfile64
>> sys_lseek
>> sys_splice
>> sys_sync_file_range2
>> sys_truncate
>> sys_ftruncate
>
> I'm not following here. Do you mean in the kernel or in glibc?
kernel.
>
> In the kernel, the list of syscalls is fine, because we already only
> provide syscalls passing loff_t as I said, and that is 64-bit.
Sorry I am lost here. I understand that the syscall passing loff_t
should wrap to 64bit syscall. But if we define off_t as 64bit,
then all the offset relative syscall should wrap to 64bit syscall.
>
> In glibc, I think we need to define fewer entry points, not more.
> Instead of having both lseek and lseek64, only one of them should
> be provided, and that should always take a 64-bit offset, calling
> into the kernel with the _llseek syscall entry.
Agree. We should avoid the duplicated definition.

Regards

Bamvor

>
> Arnd
>