From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/10656 Path: news.gmane.org!.POSTED!not-for-mail From: Bobby Bingham Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH 3/3] add s390x port Date: Fri, 21 Oct 2016 19:15:19 -0500 Message-ID: <20161022001519.GA26769@dora.lan> References: <20160726035300.10255-1-koorogi@koorogi.info> <20160726035300.10255-3-koorogi@koorogi.info> <20161021202702.GZ19318@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 X-Trace: blaine.gmane.org 1477095357 14153 195.159.176.226 (22 Oct 2016 00:15:57 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sat, 22 Oct 2016 00:15:57 +0000 (UTC) User-Agent: Mutt/1.7.1 (2016-10-04) To: musl@lists.openwall.com Original-X-From: musl-return-10669-gllmg-musl=m.gmane.org@lists.openwall.com Sat Oct 22 02:15:53 2016 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1bxjyH-0000ex-DY for gllmg-musl@m.gmane.org; Sat, 22 Oct 2016 02:15:33 +0200 Original-Received: (qmail 7323 invoked by uid 550); 22 Oct 2016 00:15:33 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 7282 invoked from network); 22 Oct 2016 00:15:32 -0000 Content-Disposition: inline In-Reply-To: <20161021202702.GZ19318@brightrain.aerifal.cx> X-Operating-System: Linux dora 4.4.24 Xref: news.gmane.org gmane.linux.lib.musl.general:10656 Archived-At: On Fri, Oct 21, 2016 at 04:27:02PM -0400, Rich Felker wrote: > Sorry for taking so long to give this the attention it deserves. I > think it looks good and is mostly ready to commit. A few > questions/comments though: Thanks for the review. > > On Mon, Jul 25, 2016 at 10:53:00PM -0500, Bobby Bingham wrote: > > diff --git a/arch/s390x/bits/alltypes.h.in b/arch/s390x/bits/alltypes.h.in > > new file mode 100644 > > index 0000000..1a83846 > > --- /dev/null > > +++ b/arch/s390x/bits/alltypes.h.in > > @@ -0,0 +1,26 @@ > > +#define _Addr long > > +#define _Int64 long > > +#define _Reg long > > + > > +TYPEDEF __builtin_va_list va_list; > > +TYPEDEF __builtin_va_list __isoc_va_list; > > + > > +#ifndef __cplusplus > > +TYPEDEF int wchar_t; > > +#endif > > + > > +TYPEDEF double float_t; > > Reportedly this is wrong (a bug copied from glibc) and floats are > actually evaluated as float on s390. Can you check and confirm? It's evaluating as double here on gcc 4.9. printf("%f", FLT_MAX * 2.0f) outputs 680564693277057719623408366969033850880.000000 rather than inf. > > > diff --git a/arch/s390x/bits/ipc.h b/arch/s390x/bits/ipc.h > > new file mode 100644 > > index 0000000..4710c12 > > --- /dev/null > > +++ b/arch/s390x/bits/ipc.h > > @@ -0,0 +1,14 @@ > > +struct ipc_perm { > > + key_t __ipc_perm_key; > > + uid_t uid; > > + gid_t gid; > > + uid_t cuid; > > + gid_t cgid; > > + mode_t mode; > > + unsigned short __pad1; > > + unsigned short __ipc_perm_seq; > > + unsigned long __pad2; > > + unsigned long __pad3; > > +}; > > __ipc_perm_seq is int on other archs, without the padding. It's not a > standard type so I don't think we're forced to have matching type, but > I wonder why this is. OTOH maybe some current archs are buggy because > of this.. > Strange. FWIW, it is unsigned short on aarch64 already as well. And the kernel type is int for ppc(64), so that architecture is probably fine. > > diff --git a/arch/s390x/syscall_arch.h b/arch/s390x/syscall_arch.h > > [...] > > +#define SYSCALL_USE_SOCKETCALL > > Is this intentional? s390x does not (or did not always) have dedicated > syscalls for socket ops? The dedicated syscalls were only added a year ago in kernel 4.3 (kernel commit 977108f89c989b1eeb5c8d938e1e71913391eb5f) > > > diff --git a/src/thread/s390x/__tls_get_offset.s b/src/thread/s390x/__tls_get_offset.s > > new file mode 100644 > > index 0000000..8ee92de > > --- /dev/null > > +++ b/src/thread/s390x/__tls_get_offset.s > > @@ -0,0 +1,17 @@ > > + .global __tls_get_offset > > + .type __tls_get_offset,%function > > +__tls_get_offset: > > + stmg %r14, %r15, 112(%r15) > > + aghi %r15, -160 > > + > > + la %r2, 0(%r2, %r12) > > + brasl %r14, __tls_get_addr > > + > > + ear %r1, %a0 > > + sllg %r1, %r1, 32 > > + ear %r1, %a1 > > + > > + sgr %r2, %r1 > > + > > + lmg %r14, %r15, 272(%r15) > > + br %r14 > > Can you explain this function and why a tail-call to __tls_get_addr > doesn't work? Based on the (albeit 32-bit) info in > https://www.akkadia.org/drepper/tls.pdf it seems like it could but > maybe I'm missing something. Is there a special contract about > clobbers? A tail call doesn't work because the return values aren't the same. From the linked document: > The return value of __tls_get_offset is an offset to the thread pointer. > To get the address of the requested variable the thread pointer needs to > be added to the return value. This function is essentially: size_t __tls_get_offset(size_t offset, char *got) { return (char*)__tls_get_addr(got + offset) - (char*)__pthread_self(); } Except that the second parameter is passed via r12 instead of the usual r3, which is why it's written in assembly. -- Bobby