Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 14 Apr 2016 03:01:38 -0500
From: Bobby Bingham <koorogi@...rogi.info>
To: musl@...ts.openwall.com
Subject: Re: [PATCH v2] add powerpc64 port

On Thu, Apr 14, 2016 at 01:05:07AM +0200, Szabolcs Nagy wrote:
> * Bobby Bingham <koorogi@...rogi.info> [2016-04-04 00:26:11 -0500]:
> > +++ b/arch/powerpc64/bits/setjmp.h
> > @@ -0,0 +1 @@
> > +typedef unsigned long long __jmp_buf[66];
> 
> hm glibc seems to use long[64] with 16byte alignment,
> is the size diff because of alignment?

Yes.  Though apparently the glibc setjmp asm has code to detect a
misaligned jmp_buf, but its handling of that case ends up overflowing
the jmp_buf.

I can make some changes to get our jmp_buf down to 65, but the only ways
to get it down to 64 are either with 16 byte alignment, or to have setjmp
spill vector registers to the stack first so it can copy them from there
to the jmp_buf through a gpr.

How important is it to match glibc here?

> 
> > +#if defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
> > +#define MINSIGSTKSZ 2048
> > +#define SIGSTKSZ    8192
> > +#endif
> 
> i think these should be bigger, e.g. follow ppc 4k, 10k
> 

Ok.

> > +typedef struct {
> > +	unsigned long vrregs[32][2];
> > +	unsigned _pad[3];
> > +	unsigned vrsave;
> > +	unsigned vscr;
> > +	unsigned _pad2[3];
> > +} vrregset_t;
> 
> it seems this type should be 16 byte aligned like elf_vrreg_t
> 
> (aarch64 has a similar issue i plan to fix: there we use
> 128bit long double to get correct alignment, but kernel
> and glibc uses __int128 which is visible in public headers.
> in powerpc64 at least __vector128 is not exposed in glibc,
> only 16byte aligned structs.)
> 
> > +++ b/arch/powerpc64/bits/syscall.h
> > @@ -0,0 +1,718 @@
> > +#define __NR_restart_syscall          0
> > +#define __NR_exit                     1
> > +#define __NR_fork                     2
> ...
> > +#define SYS_restart_syscall __NR_restart_syscall
> > +#define SYS_exit __NR_exit
> > +#define SYS_fork __NR_fork
> 
> i prefer SYS_ to use numbers too (easier to update for me),
> but it should be probably fixed together across archs.

I can do this for now, but I like Rich's suggestion to remove this
duplication.
> 
> > +++ b/arch/powerpc64/reloc.h
> > @@ -0,0 +1,32 @@
> > +#include <endian.h>
> > +
> > +#if __BYTE_ORDER == __LITTLE_ENDIAN
> > +#define ENDIAN_SUFFIX "le"
> > +#else
> > +#define ENDIAN_SUFFIX ""
> > +#endif
> > +
> > +#define LDSO_ARCH "powerpc64" ENDIAN_SUFFIX
> > +
> 
> gcc also has -sf, i'm not sure if we should care about
> ppc64 soft-float

I'll see if we can at least detect it in configure and error out.

> 
> > +#define TPOFF_K (-0x7000)
> > +
> > +#define REL_SYMBOLIC    R_PPC64_ADDR64
> > +#define REL_GOT         R_PPC64_GLOB_DAT
> > +#define REL_PLT         R_PPC64_JMP_SLOT
> > +#define REL_RELATIVE    R_PPC64_RELATIVE
> > +#define REL_COPY        R_PPC64_COPY
> > +#define REL_DTPMOD      R_PPC64_DTPMOD64
> > +#define REL_DTPOFF      R_PPC64_DTPREL64
> > +#define REL_TPOFF       R_PPC64_TPREL64
> 
> this reminds me that ppc(64) now has a tls optimization
> if the module is tagged with DT_PPC(64)_OPT, we don't
> need to implement it yet, but eventually elf.h should
> be updated.
> 
> > +++ b/src/fenv/powerpc64/fenv.c
> > @@ -0,0 +1,68 @@
> > +#define _GNU_SOURCE
> > +#include <fenv.h>
> > +
> > +static inline double get_fpscr_f(void)
> > +{
> > +	double d;
> > +	__asm__ __volatile__("mffs %0" : "=d"(d));
> > +	return d;
> > +}
> > +
> > +static inline long get_fpscr(void)
> > +{
> > +	return (union {double f; long i;}) {get_fpscr_f()}.i;
> > +}
> > +
> > +static inline void set_fpscr_f(double fpscr)
> > +{
> > +	__asm__ __volatile__("mtfsf 255, %0" : : "d"(fpscr));
> > +}
> > +
> > +static void set_fpscr(long fpscr)
> > +{
> > +	set_fpscr_f((union {long i; double f;}) {fpscr}.f);
> > +}
> > +
> 
> yes now that .c is allowed under arch/ dirs, it
> makes sense to do fenv with such set/get_fpscr
> 
> otherwise the patch looked good.

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.