Re: [PATCH v9 00/24] ILP32 for ARM64

[Yuri Norov]

On Mon, Jan 07, 2019 at 09:48:44AM -0800, Andy Lutomirski wrote:
> 
> 
> > On Jan 7, 2019, at 7:50 AM, Yuri Norov <ynorov@xxxxxxxxxxx> wrote:
> > 
> > Hi all,
> > 
> >> On Wed, May 16, 2018 at 11:18:45AM +0300, Yury Norov wrote:
> >> This series enables AARCH64 with ILP32 mode.
> >> 
> >> As supporting work, it introduces ARCH_32BIT_OFF_T configuration
> >> option that is enabled for existing 32-bit architectures but disabled
> >> for new arches (so 64-bit off_t userspace type is used by new userspace).
> >> Also it deprecates getrlimit and setrlimit syscalls prior to prlimit64.
> >> 
> >> Based on kernel v4.16. Tested with LTP, glibc testsuite, trinity, lmbench,
> >> CPUSpec.
> >> 
> >> This series on github: 
> >> https://github.com/norov/linux/tree/ilp32-4.16
> >> Linaro toolchain:
> >> http://snapshots.linaro.org/components/toolchain/binaries/7.3-2018.04-rc1/aarch64-linux-gnu_ilp32/
> >> Debian repo:
> >> http://people.linaro.org/~wookey/ilp32/
> >> OpenSUSE repo:
> >> https://build.opensuse.org/project/show/devel:ARM:Factory:Contrib:ILP32
> >> 
> >> Changes:
> >> v3: https://lkml.org/lkml/2014/9/3/704
> >> v4: https://lkml.org/lkml/2015/4/13/691
> >> v5: https://lkml.org/lkml/2015/9/29/911
> >> v6: https://lkml.org/lkml/2016/5/23/661
> >> v7: https://lkml.org/lkml/2017/1/9/213
> >> v8: https://lkml.org/lkml/2017/6/19/624
> >> v9: - rebased on top of v4.16;
> >>    - signal subsystem reworked to avoid code duplication, as requested
> >>      by Dave Martin (patches 18 and 20);
> >>    - new files introduced in series use SPDX notation for license;
> >>    - linux-api and linux-arch CCed as the series changes kernel ABI;
> >>    - checkpatch and other minor fixes.
> >>    - Zhou Chengming's reported-by for patch 2 and signed-off-by for
> >>      patch 21 removed because his email became invalid. Zhou, please
> >>      share your new email.
> > 
> > This is 4.20-based version.
> > https://github.com/norov/linux/tree/ilp32-4.20
> > 
> > There's no important changes comparing to 4.19, but I would like to remind
> > that this series contains some generic arch patches that ACKed, but still
> > not upstreamed:
> > Yury Norov      asm-generic: Drop getrlimit and setrlimit syscalls from default list
> > Yury Norov      32-bit userspace ABI: introduce ARCH_32BIT_OFF_T config option
> > Yury Norov      compat ABI: use non-compat openat and open_by_handle_at variants
> > James Morse     ptrace: Add compat PTRACE_{G,S}ETSIGMASK handlers
> > Dave Martin     arm64: signal: Make parse_user_sigframe() independent of rt_sigframe layout
> > 
> > Please also notice that my email address is changed, from now it's
> > ynorov@xxxxxxxxxxx
> > 
> 
> Having spent some time hating x32 lately, here is a cursory review:

I followed the recent discussion on x32 drop, but I don't track the story
from the beginning. Can you summarize your complains here (or maybe in
patch for Documentation)? It will be good for ilp32 because we will check
the series against all possible problems, and it will be good for history
because future compat architectures developers will be aware.

On your following questions, I'll try answer according to my understanding.
If it's wrong or incomplete, I kindly ask others fix me.

> You say âILP32 has context-related structures different from both aarch32 and
> aarch64/lp64.â  Why?  Why not just use the lp64 context structure?

Because the alternative is nothing better than how we do now. We can make
rt_sigframe for ilp32 look exactly like lp64. But on user side we will
have to add pads therefore. On the kernel side, before passing rt_sigframe
to kernel, we will have to make sure that pads are zeroed, just like we do
with x0-x7 in delouse_pt_regs() before passing syscall arguments to handlers.
So we cannot get rid of get/set_sigframe() converters in this case.

Current approach lets us keep user part looking more natural, and on
kernel side we don't add complexity because we reuse existing aarch32
or existing lp64 handlers, and don't invent something new.

> Do you have some objection to letting greg_t be a u64 on ilp32?

In binfmt_ilp32.c we
        #define compat_elf_gregset_t    elf_gregset_t
So in fact we use 64-bit gregset. Or I misunderstood you?

> Since you canât tell LP64 and ILP32 syscalls apart by number, you need to add
> and wire up a new AUDIT_ARCH value for ILP32. syscall_get_arch() needs to work correctly.

Thanks for pointing to this. I'll append AUDIT_ARCH patch to this series.

Yury