| Message ID | 20191223184523.70113-1-tuliom@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | powerpc: Move cache line size to rtld_global_ro | expand |
On 23/12/2019 15:45, Tulio Magno Quites Machado Filho wrote: > GCC 10.0 enabled -fno-common by default and this started to point that > __cache_line_size had been implemented in 2 different places: loader and > libc. > > In order to avoid this duplication, the libc variable has been removed > and the loader variable is moved to rtld_global_ro. > > File sysdeps/unix/sysv/linux/powerpc/dl-auxv.h has been added in order > to reuse code for both static and dynamic linking scenarios. > > Signed-off-by: Tulio Magno Quites Machado Filho <tuliom@linux.ibm.com> We do not use signed-off. Although not strickly necessary since the memset.c implementation already check for non set _dl_cache_line_size, you may also add the cache-line initialization for the static dlopen on dl-static.c (as powerpc already does for dl_pagesize). I assume you have also checked on powerpc-linux-gnu and powerpc64-linux-gnu, correct? > --- > sysdeps/powerpc/dl-procinfo.c | 17 ++++++++++++ > sysdeps/powerpc/powerpc32/a2/memcpy.S | 23 ++++++++-------- > sysdeps/powerpc/powerpc32/dl-machine.c | 11 ++------ > sysdeps/powerpc/powerpc32/memset.S | 29 +++++++++----------- > sysdeps/powerpc/powerpc32/sysdep.h | 26 ++++++++++++++++++ > sysdeps/powerpc/powerpc64/a2/memcpy.S | 21 +++++++++++--- > sysdeps/powerpc/powerpc64/memset.S | 24 ++++++++++++++-- > sysdeps/powerpc/rtld-global-offsets.sym | 1 + > sysdeps/unix/sysv/linux/powerpc/dl-auxv.h | 24 ++++++++++++++++ > sysdeps/unix/sysv/linux/powerpc/dl-support.c | 23 ++++++++++++++++ > sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c | 12 +------- > sysdeps/unix/sysv/linux/powerpc/libc-start.c | 10 ++----- > 12 files changed, 160 insertions(+), 61 deletions(-) > create mode 100644 sysdeps/unix/sysv/linux/powerpc/dl-auxv.h > create mode 100644 sysdeps/unix/sysv/linux/powerpc/dl-support.c > > diff --git a/sysdeps/powerpc/dl-procinfo.c b/sysdeps/powerpc/dl-procinfo.c > index 20706a648a..94b33d0c16 100644 > --- a/sysdeps/powerpc/dl-procinfo.c > +++ b/sysdeps/powerpc/dl-procinfo.c > @@ -89,5 +89,22 @@ PROCINFO_CLASS const char _dl_powerpc_cap_flags[64][15] > , > #endif > > +#if !IS_IN (ldconfig) > +# if !defined PROCINFO_DECL && defined SHARED > + ._dl_cache_line_size > +# else > +PROCINFO_CLASS int _dl_cache_line_size > +# endif > +# ifndef PROCINFO_DECL > + = 0 > +# endif > +# if !defined SHARED || defined PROCINFO_DECL > +; > +# else > +, > +# endif > +#endif > + > + > #undef PROCINFO_DECL > #undef PROCINFO_CLASS Ok. > diff --git a/sysdeps/powerpc/powerpc32/a2/memcpy.S b/sysdeps/powerpc/powerpc32/a2/memcpy.S > index 9bc91a8df1..ecfea5c832 100644 > --- a/sysdeps/powerpc/powerpc32/a2/memcpy.S > +++ b/sysdeps/powerpc/powerpc32/a2/memcpy.S > @@ -18,6 +18,7 @@ > <https://www.gnu.org/licenses/>. */ > > #include <sysdep.h> > +#include <rtld-global-offsets.h> > > #define PREFETCH_AHEAD 4 /* no cache lines SRC prefetching ahead */ > #define ZERO_AHEAD 2 /* no cache lines DST zeroing ahead */ > @@ -106,25 +107,23 @@ EALIGN (memcpy, 5, 0) > L(dst_aligned): > > > -#ifdef SHARED > +#ifdef PIC > mflr r0 > -/* Establishes GOT addressability so we can load __cache_line_size > - from static. This value was set from the aux vector during startup. */ > +/* Establishes GOT addressability so we can load the cache line size > + from rtld_global_ro. This value was set from the aux vector during > + startup. */ My understanding is code guidelines states two spaces after the end of a sentence in comments. > SETUP_GOT_ACCESS(r9,got_label) > - addis r9,r9,__cache_line_size-got_label@ha > - lwz r9,__cache_line_size-got_label@l(r9) > - mtlr r0 > -#else > -/* Load __cache_line_size from static. This value was set from the > - aux vector during startup. */ > - lis r9,__cache_line_size@ha > - lwz r9,__cache_line_size@l(r9) > + addis r9,r9,_GLOBAL_OFFSET_TABLE_-got_label@ha > + addi r9,r9,_GLOBAL_OFFSET_TABLE_-got_label@l > + mtlr r0 > #endif > + __GLRO(r9, r9, _dl_cache_line_size, > + RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET) > > cmplwi cr5, r9, 0 > bne+ cr5,L(cachelineset) > > -/* __cache_line_size not set: generic byte copy without much optimization */ > +/* Cache line size not set: generic byte copy without much optimization */ > andi. r0,r5,1 /* If length is odd copy one byte. */ > beq L(cachelinenotset_align) > lbz r7,0(r4) /* Read one byte from source. */ Ok. > diff --git a/sysdeps/powerpc/powerpc32/dl-machine.c b/sysdeps/powerpc/powerpc32/dl-machine.c > index a90cbc1ae3..740ff8f2ec 100644 > --- a/sysdeps/powerpc/powerpc32/dl-machine.c > +++ b/sysdeps/powerpc/powerpc32/dl-machine.c > @@ -25,11 +25,6 @@ > #include <dl-machine.h> > #include <_itoa.h> > > -/* The value __cache_line_size is defined in dl-sysdep.c and is initialised > - by _dl_sysdep_start via DL_PLATFORM_INIT. */ > -extern int __cache_line_size attribute_hidden; > - > - > /* Stuff for the PLT. */ > #define PLT_INITIAL_ENTRY_WORDS 18 > #define PLT_LONGBRANCH_ENTRY_WORDS 0 > @@ -309,14 +304,14 @@ __elf_machine_runtime_setup (struct link_map *map, int lazy, int profile) > > Assumes that dcbst and icbi apply to lines of 16 bytes or > more. Current known line sizes are 16, 32, and 128 bytes. > - The following gets the __cache_line_size, when available. */ > + The following gets the cache line size, when available. */ > > /* Default minimum 4 words per cache line. */ > int line_size_words = 4; > > - if (lazy && __cache_line_size != 0) > + if (lazy && GLRO(dl_cache_line_size) != 0) > /* Convert bytes to words. */ > - line_size_words = __cache_line_size / 4; > + line_size_words = GLRO(dl_cache_line_size) / 4; > > size_modified = lazy ? rel_offset_words : 6; > for (i = 0; i < size_modified; i += line_size_words) Ok. > diff --git a/sysdeps/powerpc/powerpc32/memset.S b/sysdeps/powerpc/powerpc32/memset.S > index fc8b1a2547..ea96913e4e 100644 > --- a/sysdeps/powerpc/powerpc32/memset.S > +++ b/sysdeps/powerpc/powerpc32/memset.S > @@ -17,12 +17,13 @@ > <https://www.gnu.org/licenses/>. */ > > #include <sysdep.h> > +#include <rtld-global-offsets.h> > > /* void * [r3] memset (void *s [r3], int c [r4], size_t n [r5])); > Returns 's'. > > The memset is done in four sizes: byte (8 bits), word (32 bits), > - 32-byte blocks (256 bits) and __cache_line_size (128, 256, 1024 bits). > + 32-byte blocks (256 bits) and cache line size (128, 256, 1024 bits). > There is a special case for setting whole cache lines to 0, which > takes advantage of the dcbz instruction. */ > > @@ -95,7 +96,7 @@ L(caligned): > > /* Check if we can use the special case for clearing memory using dcbz. > This requires that we know the correct cache line size for this > - processor. Getting the __cache_line_size may require establishing GOT > + processor. Getting the cache line size may require establishing GOT > addressability, so branch out of line to set this up. */ > beq cr1, L(checklinesize) > Ok. > @@ -230,26 +231,22 @@ L(medium_28t): > blr > > L(checklinesize): > -#ifdef SHARED > - mflr rTMP > /* If the remaining length is less the 32 bytes then don't bother getting > the cache line size. */ > beq L(medium) > -/* Establishes GOT addressability so we can load __cache_line_size > - from static. This value was set from the aux vector during startup. */ > +#ifdef PIC > + mflr rTMP > +/* Establishes GOT addressability so we can load the cache line size > + from rtld_global_ro. This value was set from the aux vector during > + startup. */ > SETUP_GOT_ACCESS(rGOT,got_label) > - addis rGOT,rGOT,__cache_line_size-got_label@ha > - lwz rCLS,__cache_line_size-got_label@l(rGOT) > + addis rGOT,rGOT,_GLOBAL_OFFSET_TABLE_-got_label@ha > + addi rGOT,rGOT,_GLOBAL_OFFSET_TABLE_-got_label@l > mtlr rTMP > -#else > -/* Load __cache_line_size from static. This value was set from the > - aux vector during startup. */ > - lis rCLS,__cache_line_size@ha > -/* If the remaining length is less the 32 bytes then don't bother getting > - the cache line size. */ > - beq L(medium) > - lwz rCLS,__cache_line_size@l(rCLS) > #endif > +/* Load rtld_global_ro._dl_cache_line_size. */ > + __GLRO(rCLS, rGOT, _dl_cache_line_size, > + RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET) > > /* If the cache line size was not set then goto to L(nondcbz), which is > safe for any cache line size. */ Ok. > diff --git a/sysdeps/powerpc/powerpc32/sysdep.h b/sysdeps/powerpc/powerpc32/sysdep.h > index c21ea87f5a..4605276ebf 100644 > --- a/sysdeps/powerpc/powerpc32/sysdep.h > +++ b/sysdeps/powerpc/powerpc32/sysdep.h > @@ -157,4 +157,30 @@ GOT_LABEL: ; \ > /* Label in text section. */ > #define C_TEXT(name) name > > +/* Read the value of member from rtld_global_ro. */ > +#ifdef PIC > +# ifdef SHARED > +# if IS_IN (rtld) > +/* Inside ld.so we use the local alias to avoid runtime GOT > + relocations. */ > +# define __GLRO(rOUT, rGOT, member, offset) \ > + lwz rOUT,_rtld_local_ro@got(rGOT); \ > + lwz rOUT,offset(rOUT) > +# else > +# define __GLRO(rOUT, rGOT, member, offset) \ > + lwz rOUT,_rtld_global_ro@got(rGOT); \ > + lwz rOUT,offset(rOUT) > +# endif > +# else > +# define __GLRO(rOUT, rGOT, member, offset) \ > + lwz rOUT,member@got(rGOT); \ > + lwz rOUT,0(rOUT) > +# endif > +#else > +/* Position-dependent code does not require access to the GOT. */ > +# define __GLRO(rOUT, rGOT, member, offset) \ > + lis rOUT,(member+LOWORD)@ha \ > + lwz rOUT,(member+LOWORD)@l(rOUT) > +#endif /* PIC */ > + > #endif /* __ASSEMBLER__ */ Ok. > diff --git a/sysdeps/powerpc/powerpc64/a2/memcpy.S b/sysdeps/powerpc/powerpc64/a2/memcpy.S > index 6d5c5afddb..e515255126 100644 > --- a/sysdeps/powerpc/powerpc64/a2/memcpy.S > +++ b/sysdeps/powerpc/powerpc64/a2/memcpy.S > @@ -18,6 +18,7 @@ > <https://www.gnu.org/licenses/>. */ > > #include <sysdep.h> > +#include <rtld-global-offsets.h> > > #ifndef MEMCPY > # define MEMCPY memcpy > @@ -27,8 +28,19 @@ > #define ZERO_AHEAD 2 /* no cache lines DST zeroing ahead */ > > .section ".toc","aw" > -.LC0: > - .tc __cache_line_size[TC],__cache_line_size > +.LC__dl_cache_line_size: > +#ifdef SHARED > +# if IS_IN (rtld) > + /* Inside ld.so we use the local alias to avoid runtime GOT > + relocations. */ > + .tc _rtld_local_ro[TC],_rtld_local_ro > +# else > + .tc _rtld_global_ro[TC],_rtld_global_ro > +# endif > +#else > + .tc _dl_cache_line_size[TC],_dl_cache_line_size > +#endif > + > .section ".text" > .align 2 > Ok. Maybe add a similar macro for powerpc64 as you did for powerpc32 with __GLRO ? > @@ -55,7 +67,8 @@ ENTRY (MEMCPY, 5) > */ > > neg r8,r3 /* LS 4 bits = # bytes to 8-byte dest bdry */ > - ld r9,.LC0@toc(r2) /* Get cache line size (part 1) */ > + /* Get cache line size (part 1) */ > + ld r9,.LC__dl_cache_line_size@toc(r2) > clrldi r8,r8,64-4 /* align to 16byte boundary */ > sub r7,r4,r3 /* compute offset to src from dest */ > lwz r9,0(r9) /* Get cache line size (part 2) */ Ok. > @@ -121,7 +134,7 @@ L(dst_aligned): > cmpdi cr0,r9,0 /* Cache line size set? */ > bne+ cr0,L(cachelineset) > > -/* __cache_line_size not set: generic byte copy without much optimization */ > +/* Cache line size not set: generic byte copy without much optimization */ > clrldi. r0,r5,63 /* If length is odd copy one byte */ > beq L(cachelinenotset_align) > lbz r7,0(r4) /* Read one byte from source */ Ok. > diff --git a/sysdeps/powerpc/powerpc64/memset.S b/sysdeps/powerpc/powerpc64/memset.S > index 5d96696e87..d49ef102d4 100644 > --- a/sysdeps/powerpc/powerpc64/memset.S > +++ b/sysdeps/powerpc/powerpc64/memset.S > @@ -17,10 +17,22 @@ > <https://www.gnu.org/licenses/>. */ > > #include <sysdep.h> > +#include <rtld-global-offsets.h> > > .section ".toc","aw" > -.LC0: > - .tc __cache_line_size[TC],__cache_line_size > +.LC__dl_cache_line_size: > +#ifdef SHARED > +# if IS_IN (rtld) > + /* Inside ld.so we use the local alias to avoid runtime GOT > + relocations. */ > + .tc _rtld_local_ro[TC],_rtld_local_ro > +# else > + .tc _rtld_global_ro[TC],_rtld_global_ro > +# endif > +#else > + .tc _dl_cache_line_size[TC],_dl_cache_line_size > +#endif > + > .section ".text" > .align 2 > Ok. > @@ -146,8 +158,14 @@ L(zloopstart): > /* If the remaining length is less the 32 bytes, don't bother getting > the cache line size. */ > beq L(medium) > - ld rCLS,.LC0@toc(r2) > + ld rCLS,.LC__dl_cache_line_size@toc(r2) > +# ifdef SHARED > + /* Load _rtld_global_ro._dl_cache_line_size. */ > + lwz rCLS,RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET(rCLS) > +# else > + /* Load _dl_cache_line_size. */ > lwz rCLS,0(rCLS) > +# endif > /* If the cache line size was not set just goto to L(nondcbz) which is > safe for any cache line size. */ > cmpldi cr1,rCLS,0 Ok. > diff --git a/sysdeps/powerpc/rtld-global-offsets.sym b/sysdeps/powerpc/rtld-global-offsets.sym > index f5ea5a1466..6b348fd522 100644 > --- a/sysdeps/powerpc/rtld-global-offsets.sym > +++ b/sysdeps/powerpc/rtld-global-offsets.sym > @@ -6,3 +6,4 @@ > > RTLD_GLOBAL_RO_DL_HWCAP_OFFSET rtld_global_ro_offsetof (_dl_hwcap) > RTLD_GLOBAL_RO_DL_HWCAP2_OFFSET rtld_global_ro_offsetof (_dl_hwcap2) > +RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET rtld_global_ro_offsetof (_dl_cache_line_size) Ok. > diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-auxv.h b/sysdeps/unix/sysv/linux/powerpc/dl-auxv.h > new file mode 100644 > index 0000000000..44bea09974 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/powerpc/dl-auxv.h > @@ -0,0 +1,24 @@ > +/* Auxiliary vector processing. Linux/PPC version. > + Copyright (C) 2019 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +/* Scan the Aux Vector for the "Data Cache Block Size" entry and assign it > + to dl_cache_line_size. */ > +#define DL_PLATFORM_AUXV \ > + case AT_DCACHEBSIZE: \ > + GLRO(dl_cache_line_size) = av->a_un.a_val; \ > + break; Ok. > diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-support.c b/sysdeps/unix/sysv/linux/powerpc/dl-support.c > new file mode 100644 > index 0000000000..2a792d7092 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/powerpc/dl-support.c > @@ -0,0 +1,23 @@ > +/* Support for dynamic linking code in static libc. Linux/PPC version. > + Copyright (C) 2019 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include "dl-auxv.h" > +#include <ldsodefs.h> > +int GLRO(dl_cache_line_size); > + > +#include <elf/dl-support.c> Why is it required? It sould be already covered by inclusion of sysdeps/powerpc/dl-procinfo.c and it does not required the dl-auxv.h definitions here. > diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c b/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c > index fa19cc66c0..6d51d6061e 100644 > --- a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c > +++ b/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c > @@ -18,16 +18,6 @@ > > #include <config.h> > #include <ldsodefs.h> > - > -int __cache_line_size attribute_hidden; > - > -/* Scan the Aux Vector for the "Data Cache Block Size" entry. If found > - verify that the static extern __cache_line_size is defined by checking > - for not NULL. If it is defined then assign the cache block size > - value to __cache_line_size. */ > -#define DL_PLATFORM_AUXV \ > - case AT_DCACHEBSIZE: \ > - __cache_line_size = av->a_un.a_val; \ > - break; > +#include <dl-auxv.h> > > #include <sysdeps/unix/sysv/linux/dl-sysdep.c> This is essentially an empty file that just include dl-auxv.h. I think it would be better to just create a generic empty dl-auxv.h, include it on default dl-sysdep.c, and remove this file. > diff --git a/sysdeps/unix/sysv/linux/powerpc/libc-start.c b/sysdeps/unix/sysv/linux/powerpc/libc-start.c > index a659a9144f..3a5eb0e952 100644 > --- a/sysdeps/unix/sysv/linux/powerpc/libc-start.c > +++ b/sysdeps/unix/sysv/linux/powerpc/libc-start.c > @@ -24,7 +24,6 @@ > #include <hwcapinfo.h> > #endif > > -int __cache_line_size attribute_hidden; > /* The main work is done in the generic function. */ > #define LIBC_START_MAIN generic_start_main > #define LIBC_START_DISABLE_INLINE > @@ -71,15 +70,12 @@ __libc_start_main (int argc, char **argv, > rtld_fini = NULL; > } > > - /* Initialize the __cache_line_size variable from the aux vector. For the > - static case, we also need _dl_hwcap, _dl_hwcap2 and _dl_platform, so we > - can call __tcb_parse_hwcap_and_convert_at_platform (). */ > for (ElfW (auxv_t) * av = auxvec; av->a_type != AT_NULL; ++av) > switch (av->a_type) > { > - case AT_DCACHEBSIZE: > - __cache_line_size = av->a_un.a_val; > - break; > + /* For the static case, we also need _dl_hwcap, _dl_hwcap2 and > + _dl_platform, so we can call > + __tcb_parse_hwcap_and_convert_at_platform (). */ > #ifndef SHARED > case AT_HWCAP: > _dl_hwcap = (unsigned long int) av->a_un.a_val; > Ok.
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes: > On 23/12/2019 15:45, Tulio Magno Quites Machado Filho wrote: >> GCC 10.0 enabled -fno-common by default and this started to point that >> __cache_line_size had been implemented in 2 different places: loader and >> libc. >> >> In order to avoid this duplication, the libc variable has been removed >> and the loader variable is moved to rtld_global_ro. >> >> File sysdeps/unix/sysv/linux/powerpc/dl-auxv.h has been added in order >> to reuse code for both static and dynamic linking scenarios. >> >> Signed-off-by: Tulio Magno Quites Machado Filho <tuliom@linux.ibm.com> > > We do not use signed-off. Ack. > Although not strickly necessary since the memset.c implementation already > check for non set _dl_cache_line_size, you may also add the cache-line > initialization for the static dlopen on dl-static.c (as powerpc > already does for dl_pagesize). Good point. That would allow a dlopened DSO from a statically linked executable access dl_cache_line_size too. > I assume you have also checked on powerpc-linux-gnu and powerpc64-linux-gnu, > correct? Correct. >> diff --git a/sysdeps/powerpc/powerpc32/a2/memcpy.S b/sysdeps/powerpc/powerpc32/a2/memcpy.S >> index 9bc91a8df1..ecfea5c832 100644 >> --- a/sysdeps/powerpc/powerpc32/a2/memcpy.S >> +++ b/sysdeps/powerpc/powerpc32/a2/memcpy.S >> @@ -18,6 +18,7 @@ >> <https://www.gnu.org/licenses/>. */ >> >> #include <sysdep.h> >> +#include <rtld-global-offsets.h> >> >> #define PREFETCH_AHEAD 4 /* no cache lines SRC prefetching ahead */ >> #define ZERO_AHEAD 2 /* no cache lines DST zeroing ahead */ >> @@ -106,25 +107,23 @@ EALIGN (memcpy, 5, 0) >> L(dst_aligned): >> >> >> -#ifdef SHARED >> +#ifdef PIC >> mflr r0 >> -/* Establishes GOT addressability so we can load __cache_line_size >> - from static. This value was set from the aux vector during startup. */ >> +/* Establishes GOT addressability so we can load the cache line size >> + from rtld_global_ro. This value was set from the aux vector during >> + startup. */ > > My understanding is code guidelines states two spaces after the end of a > sentence in comments. Indeed. >> diff --git a/sysdeps/powerpc/powerpc64/a2/memcpy.S b/sysdeps/powerpc/powerpc64/a2/memcpy.S >> index 6d5c5afddb..e515255126 100644 >> --- a/sysdeps/powerpc/powerpc64/a2/memcpy.S >> +++ b/sysdeps/powerpc/powerpc64/a2/memcpy.S >> @@ -18,6 +18,7 @@ >> <https://www.gnu.org/licenses/>. */ >> >> #include <sysdep.h> >> +#include <rtld-global-offsets.h> >> >> #ifndef MEMCPY >> # define MEMCPY memcpy >> @@ -27,8 +28,19 @@ >> #define ZERO_AHEAD 2 /* no cache lines DST zeroing ahead */ >> >> .section ".toc","aw" >> -.LC0: >> - .tc __cache_line_size[TC],__cache_line_size >> +.LC__dl_cache_line_size: >> +#ifdef SHARED >> +# if IS_IN (rtld) >> + /* Inside ld.so we use the local alias to avoid runtime GOT >> + relocations. */ >> + .tc _rtld_local_ro[TC],_rtld_local_ro >> +# else >> + .tc _rtld_global_ro[TC],_rtld_global_ro >> +# endif >> +#else >> + .tc _dl_cache_line_size[TC],_dl_cache_line_size >> +#endif >> + >> .section ".text" >> .align 2 >> > > Ok. Maybe add a similar macro for powerpc64 as you did for powerpc32 with > __GLRO ? Are you suggesting to create a __GLRO macro that would define this label? >> diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-support.c b/sysdeps/unix/sysv/linux/powerpc/dl-support.c >> new file mode 100644 >> index 0000000000..2a792d7092 >> --- /dev/null >> +++ b/sysdeps/unix/sysv/linux/powerpc/dl-support.c >> @@ -0,0 +1,23 @@ >> +/* Support for dynamic linking code in static libc. Linux/PPC version. >> + Copyright (C) 2019 Free Software Foundation, Inc. >> + This file is part of the GNU C Library. >> + >> + The GNU C Library is free software; you can redistribute it and/or >> + modify it under the terms of the GNU Lesser General Public >> + License as published by the Free Software Foundation; either >> + version 2.1 of the License, or (at your option) any later version. >> + >> + The GNU C Library is distributed in the hope that it will be useful, >> + but WITHOUT ANY WARRANTY; without even the implied warranty of >> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + Lesser General Public License for more details. >> + >> + You should have received a copy of the GNU Lesser General Public >> + License along with the GNU C Library; if not, see >> + <https://www.gnu.org/licenses/>. */ >> + >> +#include "dl-auxv.h" >> +#include <ldsodefs.h> >> +int GLRO(dl_cache_line_size); >> + >> +#include <elf/dl-support.c> > > Why is it required? It sould be already covered by inclusion of > sysdeps/powerpc/dl-procinfo.c and it does not required the dl-auxv.h > definitions here. This is the code that initializes it statically. It avoids duplicating code. dl-procinfo.c does not copy AT_DCACHEBSIZE to GLRO(dl_cache_line_size). >> diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c b/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c >> index fa19cc66c0..6d51d6061e 100644 >> --- a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c >> +++ b/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c >> @@ -18,16 +18,6 @@ >> >> #include <config.h> >> #include <ldsodefs.h> >> - >> -int __cache_line_size attribute_hidden; >> - >> -/* Scan the Aux Vector for the "Data Cache Block Size" entry. If found >> - verify that the static extern __cache_line_size is defined by checking >> - for not NULL. If it is defined then assign the cache block size >> - value to __cache_line_size. */ >> -#define DL_PLATFORM_AUXV \ >> - case AT_DCACHEBSIZE: \ >> - __cache_line_size = av->a_un.a_val; \ >> - break; >> +#include <dl-auxv.h> >> >> #include <sysdeps/unix/sysv/linux/dl-sysdep.c> > > This is essentially an empty file that just include dl-auxv.h. I think > it would be better to just create a generic empty dl-auxv.h, include it > on default dl-sysdep.c, and remove this file. I'm fine with this proposal, but I don't think it's going to help neither alpha or x86 which have slightly different implementations.
On 27/12/2019 12:42, Tulio Magno Quites Machado Filho wrote: > Adhemerval Zanella <adhemerval.zanella@linaro.org> writes: > >> On 23/12/2019 15:45, Tulio Magno Quites Machado Filho wrote: >>> GCC 10.0 enabled -fno-common by default and this started to point that >>> __cache_line_size had been implemented in 2 different places: loader and >>> libc. >>> >>> In order to avoid this duplication, the libc variable has been removed >>> and the loader variable is moved to rtld_global_ro. >>> >>> File sysdeps/unix/sysv/linux/powerpc/dl-auxv.h has been added in order >>> to reuse code for both static and dynamic linking scenarios. >>> >>> Signed-off-by: Tulio Magno Quites Machado Filho <tuliom@linux.ibm.com> >> >> We do not use signed-off. > > Ack. > >> Although not strickly necessary since the memset.c implementation already >> check for non set _dl_cache_line_size, you may also add the cache-line >> initialization for the static dlopen on dl-static.c (as powerpc >> already does for dl_pagesize). > > Good point. That would allow a dlopened DSO from a statically linked > executable access dl_cache_line_size too. > >> I assume you have also checked on powerpc-linux-gnu and powerpc64-linux-gnu, >> correct? > > Correct. > >>> diff --git a/sysdeps/powerpc/powerpc32/a2/memcpy.S b/sysdeps/powerpc/powerpc32/a2/memcpy.S >>> index 9bc91a8df1..ecfea5c832 100644 >>> --- a/sysdeps/powerpc/powerpc32/a2/memcpy.S >>> +++ b/sysdeps/powerpc/powerpc32/a2/memcpy.S >>> @@ -18,6 +18,7 @@ >>> <https://www.gnu.org/licenses/>. */ >>> >>> #include <sysdep.h> >>> +#include <rtld-global-offsets.h> >>> >>> #define PREFETCH_AHEAD 4 /* no cache lines SRC prefetching ahead */ >>> #define ZERO_AHEAD 2 /* no cache lines DST zeroing ahead */ >>> @@ -106,25 +107,23 @@ EALIGN (memcpy, 5, 0) >>> L(dst_aligned): >>> >>> >>> -#ifdef SHARED >>> +#ifdef PIC >>> mflr r0 >>> -/* Establishes GOT addressability so we can load __cache_line_size >>> - from static. This value was set from the aux vector during startup. */ >>> +/* Establishes GOT addressability so we can load the cache line size >>> + from rtld_global_ro. This value was set from the aux vector during >>> + startup. */ >> >> My understanding is code guidelines states two spaces after the end of a >> sentence in comments. > > Indeed. > >>> diff --git a/sysdeps/powerpc/powerpc64/a2/memcpy.S b/sysdeps/powerpc/powerpc64/a2/memcpy.S >>> index 6d5c5afddb..e515255126 100644 >>> --- a/sysdeps/powerpc/powerpc64/a2/memcpy.S >>> +++ b/sysdeps/powerpc/powerpc64/a2/memcpy.S >>> @@ -18,6 +18,7 @@ >>> <https://www.gnu.org/licenses/>. */ >>> >>> #include <sysdep.h> >>> +#include <rtld-global-offsets.h> >>> >>> #ifndef MEMCPY >>> # define MEMCPY memcpy >>> @@ -27,8 +28,19 @@ >>> #define ZERO_AHEAD 2 /* no cache lines DST zeroing ahead */ >>> >>> .section ".toc","aw" >>> -.LC0: >>> - .tc __cache_line_size[TC],__cache_line_size >>> +.LC__dl_cache_line_size: >>> +#ifdef SHARED >>> +# if IS_IN (rtld) >>> + /* Inside ld.so we use the local alias to avoid runtime GOT >>> + relocations. */ >>> + .tc _rtld_local_ro[TC],_rtld_local_ro >>> +# else >>> + .tc _rtld_global_ro[TC],_rtld_global_ro >>> +# endif >>> +#else >>> + .tc _dl_cache_line_size[TC],_dl_cache_line_size >>> +#endif >>> + >>> .section ".text" >>> .align 2 >>> >> >> Ok. Maybe add a similar macro for powerpc64 as you did for powerpc32 with >> __GLRO ? > > Are you suggesting to create a __GLRO macro that would define this label? Or something similar to avoid the duplicated defition here and at sysdeps/powerpc/powerpc64/memset.S. > >>> diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-support.c b/sysdeps/unix/sysv/linux/powerpc/dl-support.c >>> new file mode 100644 >>> index 0000000000..2a792d7092 >>> --- /dev/null >>> +++ b/sysdeps/unix/sysv/linux/powerpc/dl-support.c >>> @@ -0,0 +1,23 @@ >>> +/* Support for dynamic linking code in static libc. Linux/PPC version. >>> + Copyright (C) 2019 Free Software Foundation, Inc. >>> + This file is part of the GNU C Library. >>> + >>> + The GNU C Library is free software; you can redistribute it and/or >>> + modify it under the terms of the GNU Lesser General Public >>> + License as published by the Free Software Foundation; either >>> + version 2.1 of the License, or (at your option) any later version. >>> + >>> + The GNU C Library is distributed in the hope that it will be useful, >>> + but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >>> + Lesser General Public License for more details. >>> + >>> + You should have received a copy of the GNU Lesser General Public >>> + License along with the GNU C Library; if not, see >>> + <https://www.gnu.org/licenses/>. */ >>> + >>> +#include "dl-auxv.h" >>> +#include <ldsodefs.h> >>> +int GLRO(dl_cache_line_size); >>> + >>> +#include <elf/dl-support.c> >> >> Why is it required? It sould be already covered by inclusion of >> sysdeps/powerpc/dl-procinfo.c and it does not required the dl-auxv.h >> definitions here. > > This is the code that initializes it statically. It avoids duplicating code. > dl-procinfo.c does not copy AT_DCACHEBSIZE to GLRO(dl_cache_line_size). But with a generic dl-auxv.h that defines DL_PLATFORM_AUXV, this files is not really required. > >>> diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c b/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c >>> index fa19cc66c0..6d51d6061e 100644 >>> --- a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c >>> +++ b/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c >>> @@ -18,16 +18,6 @@ >>> >>> #include <config.h> >>> #include <ldsodefs.h> >>> - >>> -int __cache_line_size attribute_hidden; >>> - >>> -/* Scan the Aux Vector for the "Data Cache Block Size" entry. If found >>> - verify that the static extern __cache_line_size is defined by checking >>> - for not NULL. If it is defined then assign the cache block size >>> - value to __cache_line_size. */ >>> -#define DL_PLATFORM_AUXV \ >>> - case AT_DCACHEBSIZE: \ >>> - __cache_line_size = av->a_un.a_val; \ >>> - break; >>> +#include <dl-auxv.h> >>> >>> #include <sysdeps/unix/sysv/linux/dl-sysdep.c> >> >> This is essentially an empty file that just include dl-auxv.h. I think >> it would be better to just create a generic empty dl-auxv.h, include it >> on default dl-sysdep.c, and remove this file. > > I'm fine with this proposal, but I don't think it's going to help neither alpha > or x86 which have slightly different implementations. Indeed, but I think we can refactor alpha or x86 later.
What's the status here? I think it's desirable to fix this before the release. Thanks, Florian
On Thu, 2020-01-09 at 11:29 +0100, Florian Weimer wrote: > What's the status here? > > I think it's desirable to fix this before the release. Presumably this fixes the problem with it being used as a common symbol and thus not working with gcc-10 on ppc64? If so, yea, seems like it'd be desirable before the release. jeff
Jeff Law <law@redhat.com> writes: > On Thu, 2020-01-09 at 11:29 +0100, Florian Weimer wrote: >> What's the status here? >> >> I think it's desirable to fix this before the release. While working on the changes requested by Adhemerval, I hit 2 other bugs: 1. getauxval() does not work well from a DSO dlopen'ed by a statically linked executable. 2. errno from a DSO is lost in a statically linked executable (BZ #25335). I've fixed the first issue, but I'm stuck with the second one. > Presumably this fixes the problem with it being used as a common symbol > and thus not working with gcc-10 on ppc64? If so, yea, seems like it'd > be desirable before the release. The tests I implemented are failing because of these issues. Would it be fine if I disabled the failing tests while we do not have a fix for BZ #25335?
On 09/01/2020 14:20, Tulio Magno Quites Machado Filho wrote: > Jeff Law <law@redhat.com> writes: > >> On Thu, 2020-01-09 at 11:29 +0100, Florian Weimer wrote: >>> What's the status here? >>> >>> I think it's desirable to fix this before the release. > > While working on the changes requested by Adhemerval, I hit 2 other bugs: > > 1. getauxval() does not work well from a DSO dlopen'ed by a statically linked > executable. This is BZ#20802 and I although we might fix it by adding the initialization on dl-static.c, I think Florian suggestion [1] to proper initialize shared ld.so variables is a better overall solution. However it is also a much more extensive change and not feasible on current release phase. > 2. errno from a DSO is lost in a statically linked executable (BZ #25335). > > I've fixed the first issue, but I'm stuck with the second one. I don't see how easily we would fix it, it probability require to change how errno is definde in static manner and most likely more hacks to use the one from loaded libc.so. This would also require some discussion if it is really an expected scenario we should support. > >> Presumably this fixes the problem with it being used as a common symbol >> and thus not working with gcc-10 on ppc64? If so, yea, seems like it'd >> be desirable before the release. > > The tests I implemented are failing because of these issues. > > Would it be fine if I disabled the failing tests while we do not have a fix > for BZ #25335? > You can change the test to save/restore a passed errno in the wrapper if it is required to check for errno value. I don't see provide a fix for BZ#25535 as a blocker here. [1] https://sourceware.org/ml/libc-alpha/2019-12/msg00483.html
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes: > This is BZ#20802 and I although we might fix it by adding the initialization > on dl-static.c, I think Florian suggestion [1] to proper initialize shared ld.so > variables is a better overall solution. However it is also a much more extensive > change and not feasible on current release phase. Ack. > I don't see how easily we would fix it, it probability require to change > how errno is definde in static manner and most likely more hacks to use > the one from loaded libc.so. This would also require some discussion if it > is really an expected scenario we should support. Makes sense. >> Would it be fine if I disabled the failing tests while we do not have a fix >> for BZ #25335? > > You can change the test to save/restore a passed errno in the wrapper > if it is required to check for errno value. I don't see provide a > fix for BZ#25535 as a blocker here. What are you referring as a wrapper in the getauxval() case?
On 09/01/2020 15:49, Tulio Magno Quites Machado Filho wrote: > Adhemerval Zanella <adhemerval.zanella@linaro.org> writes: > >> This is BZ#20802 and I although we might fix it by adding the initialization >> on dl-static.c, I think Florian suggestion [1] to proper initialize shared ld.so >> variables is a better overall solution. However it is also a much more extensive >> change and not feasible on current release phase. > > Ack. > >> I don't see how easily we would fix it, it probability require to change >> how errno is definde in static manner and most likely more hacks to use >> the one from loaded libc.so. This would also require some discussion if it >> is really an expected scenario we should support. > > Makes sense. > >>> Would it be fine if I disabled the failing tests while we do not have a fix >>> for BZ #25335? >> >> You can change the test to save/restore a passed errno in the wrapper >> if it is required to check for errno value. I don't see provide a >> fix for BZ#25535 as a blocker here. > > What are you referring as a wrapper in the getauxval() case? > Yes, on the test. Something like: unsigned long int getauxval_wrapper (unsigned long type, int *errnop) { errno = *errnop; unsigned long int r = getauxval (type); *errnop = errno; return r; } And then you use on static build which will dlopen it: { unsigned long int (*wrapper)(unsigned long, int *) = xdlsym (handler, "getauxval_wrapper"); int ierrno = 0; TEST_COMPARE (wrapper (...), ...); TEST_COMPARE (ierrno, ...); }
Tulio Magno Quites Machado Filho <tuliom@ascii.art.br> writes: > Adhemerval Zanella <adhemerval.zanella@linaro.org> writes: > >> You can change the test to save/restore a passed errno in the wrapper >> if it is required to check for errno value. I don't see provide a >> fix for BZ#25535 as a blocker here. > > What are you referring as a wrapper in the getauxval() case? Oh! Forget it. You're referring to Florian's patch. I'll use that. Thanks!
* Tulio Magno Quites Machado Filho: > Jeff Law <law@redhat.com> writes: > >> On Thu, 2020-01-09 at 11:29 +0100, Florian Weimer wrote: >>> What's the status here? >>> >>> I think it's desirable to fix this before the release. > > While working on the changes requested by Adhemerval, I hit 2 other bugs: > > 1. getauxval() does not work well from a DSO dlopen'ed by a statically linked > executable. > 2. errno from a DSO is lost in a statically linked executable (BZ #25335). > > I've fixed the first issue, but I'm stuck with the second one. Based on the subsequent discussion, they are no longer blockers, right? Thanks, Florian
Not trying to say anything stupid, but why can't we just detect the cache line size using the "dcbz" instruction, as documented in the PowerISA document? It is because the ancient G5 also has a "dcbzl" instruction that clears a wider size (the actually cache line size)?
On 10/01/2020 13:09, Shawn Landden wrote: > Not trying to say anything stupid, but why can't we just detect the cache line size using the "dcbz" instruction, as documented in the PowerISA document? It is because the ancient G5 also has a "dcbzl" instruction that clears a wider size (the actually cache line size)? > AFAIK you can't detect the data cache block size using the dcbz instruction, in fact PowerISA 3.0B states that this information should be provided by the operation system (Book II, Chapter 4.1 Parameters Useful to Application Programs). Linux provided it by AT_DCACHEBSIZE field in auxv and it is implemented in the kernel by a pre-defined table (arch/powerpc/kernel/cputable.c). This is how glibc obtain such information (sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c). The issue I pointed out is for static binaries that dlopen a shared library, mem* calls done by the library (which will call the loaded libc.so one) won't see the field properly initialized. Some architectures fix rtld initialization by reimplementing the _dl_static_init. It looks up for the '_dl_var_init' and calls it, taking care of relro segments by unmap/mmap the segment. And the dcbzl seems to be a hack pushed by Apple for some reason, which should not be required on Linux: "dcbz" only operates on 32 bytes when the special HID5 compatiblity bit that apple added to the 970 is set. This is _NOT_ the normal case and this bit isn't set in linux unless you explicitely set it by modifying the kernel [1]." [1] https://lists.ozlabs.org/pipermail/linuxppc64-dev/2004-March/001383.html
diff --git a/sysdeps/powerpc/dl-procinfo.c b/sysdeps/powerpc/dl-procinfo.c index 20706a648a..94b33d0c16 100644 --- a/sysdeps/powerpc/dl-procinfo.c +++ b/sysdeps/powerpc/dl-procinfo.c @@ -89,5 +89,22 @@ PROCINFO_CLASS const char _dl_powerpc_cap_flags[64][15] , #endif +#if !IS_IN (ldconfig) +# if !defined PROCINFO_DECL && defined SHARED + ._dl_cache_line_size +# else +PROCINFO_CLASS int _dl_cache_line_size +# endif +# ifndef PROCINFO_DECL + = 0 +# endif +# if !defined SHARED || defined PROCINFO_DECL +; +# else +, +# endif +#endif + + #undef PROCINFO_DECL #undef PROCINFO_CLASS diff --git a/sysdeps/powerpc/powerpc32/a2/memcpy.S b/sysdeps/powerpc/powerpc32/a2/memcpy.S index 9bc91a8df1..ecfea5c832 100644 --- a/sysdeps/powerpc/powerpc32/a2/memcpy.S +++ b/sysdeps/powerpc/powerpc32/a2/memcpy.S @@ -18,6 +18,7 @@ <https://www.gnu.org/licenses/>. */ #include <sysdep.h> +#include <rtld-global-offsets.h> #define PREFETCH_AHEAD 4 /* no cache lines SRC prefetching ahead */ #define ZERO_AHEAD 2 /* no cache lines DST zeroing ahead */ @@ -106,25 +107,23 @@ EALIGN (memcpy, 5, 0) L(dst_aligned): -#ifdef SHARED +#ifdef PIC mflr r0 -/* Establishes GOT addressability so we can load __cache_line_size - from static. This value was set from the aux vector during startup. */ +/* Establishes GOT addressability so we can load the cache line size + from rtld_global_ro. This value was set from the aux vector during + startup. */ SETUP_GOT_ACCESS(r9,got_label) - addis r9,r9,__cache_line_size-got_label@ha - lwz r9,__cache_line_size-got_label@l(r9) - mtlr r0 -#else -/* Load __cache_line_size from static. This value was set from the - aux vector during startup. */ - lis r9,__cache_line_size@ha - lwz r9,__cache_line_size@l(r9) + addis r9,r9,_GLOBAL_OFFSET_TABLE_-got_label@ha + addi r9,r9,_GLOBAL_OFFSET_TABLE_-got_label@l + mtlr r0 #endif + __GLRO(r9, r9, _dl_cache_line_size, + RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET) cmplwi cr5, r9, 0 bne+ cr5,L(cachelineset) -/* __cache_line_size not set: generic byte copy without much optimization */ +/* Cache line size not set: generic byte copy without much optimization */ andi. r0,r5,1 /* If length is odd copy one byte. */ beq L(cachelinenotset_align) lbz r7,0(r4) /* Read one byte from source. */ diff --git a/sysdeps/powerpc/powerpc32/dl-machine.c b/sysdeps/powerpc/powerpc32/dl-machine.c index a90cbc1ae3..740ff8f2ec 100644 --- a/sysdeps/powerpc/powerpc32/dl-machine.c +++ b/sysdeps/powerpc/powerpc32/dl-machine.c @@ -25,11 +25,6 @@ #include <dl-machine.h> #include <_itoa.h> -/* The value __cache_line_size is defined in dl-sysdep.c and is initialised - by _dl_sysdep_start via DL_PLATFORM_INIT. */ -extern int __cache_line_size attribute_hidden; - - /* Stuff for the PLT. */ #define PLT_INITIAL_ENTRY_WORDS 18 #define PLT_LONGBRANCH_ENTRY_WORDS 0 @@ -309,14 +304,14 @@ __elf_machine_runtime_setup (struct link_map *map, int lazy, int profile) Assumes that dcbst and icbi apply to lines of 16 bytes or more. Current known line sizes are 16, 32, and 128 bytes. - The following gets the __cache_line_size, when available. */ + The following gets the cache line size, when available. */ /* Default minimum 4 words per cache line. */ int line_size_words = 4; - if (lazy && __cache_line_size != 0) + if (lazy && GLRO(dl_cache_line_size) != 0) /* Convert bytes to words. */ - line_size_words = __cache_line_size / 4; + line_size_words = GLRO(dl_cache_line_size) / 4; size_modified = lazy ? rel_offset_words : 6; for (i = 0; i < size_modified; i += line_size_words) diff --git a/sysdeps/powerpc/powerpc32/memset.S b/sysdeps/powerpc/powerpc32/memset.S index fc8b1a2547..ea96913e4e 100644 --- a/sysdeps/powerpc/powerpc32/memset.S +++ b/sysdeps/powerpc/powerpc32/memset.S @@ -17,12 +17,13 @@ <https://www.gnu.org/licenses/>. */ #include <sysdep.h> +#include <rtld-global-offsets.h> /* void * [r3] memset (void *s [r3], int c [r4], size_t n [r5])); Returns 's'. The memset is done in four sizes: byte (8 bits), word (32 bits), - 32-byte blocks (256 bits) and __cache_line_size (128, 256, 1024 bits). + 32-byte blocks (256 bits) and cache line size (128, 256, 1024 bits). There is a special case for setting whole cache lines to 0, which takes advantage of the dcbz instruction. */ @@ -95,7 +96,7 @@ L(caligned): /* Check if we can use the special case for clearing memory using dcbz. This requires that we know the correct cache line size for this - processor. Getting the __cache_line_size may require establishing GOT + processor. Getting the cache line size may require establishing GOT addressability, so branch out of line to set this up. */ beq cr1, L(checklinesize) @@ -230,26 +231,22 @@ L(medium_28t): blr L(checklinesize): -#ifdef SHARED - mflr rTMP /* If the remaining length is less the 32 bytes then don't bother getting the cache line size. */ beq L(medium) -/* Establishes GOT addressability so we can load __cache_line_size - from static. This value was set from the aux vector during startup. */ +#ifdef PIC + mflr rTMP +/* Establishes GOT addressability so we can load the cache line size + from rtld_global_ro. This value was set from the aux vector during + startup. */ SETUP_GOT_ACCESS(rGOT,got_label) - addis rGOT,rGOT,__cache_line_size-got_label@ha - lwz rCLS,__cache_line_size-got_label@l(rGOT) + addis rGOT,rGOT,_GLOBAL_OFFSET_TABLE_-got_label@ha + addi rGOT,rGOT,_GLOBAL_OFFSET_TABLE_-got_label@l mtlr rTMP -#else -/* Load __cache_line_size from static. This value was set from the - aux vector during startup. */ - lis rCLS,__cache_line_size@ha -/* If the remaining length is less the 32 bytes then don't bother getting - the cache line size. */ - beq L(medium) - lwz rCLS,__cache_line_size@l(rCLS) #endif +/* Load rtld_global_ro._dl_cache_line_size. */ + __GLRO(rCLS, rGOT, _dl_cache_line_size, + RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET) /* If the cache line size was not set then goto to L(nondcbz), which is safe for any cache line size. */ diff --git a/sysdeps/powerpc/powerpc32/sysdep.h b/sysdeps/powerpc/powerpc32/sysdep.h index c21ea87f5a..4605276ebf 100644 --- a/sysdeps/powerpc/powerpc32/sysdep.h +++ b/sysdeps/powerpc/powerpc32/sysdep.h @@ -157,4 +157,30 @@ GOT_LABEL: ; \ /* Label in text section. */ #define C_TEXT(name) name +/* Read the value of member from rtld_global_ro. */ +#ifdef PIC +# ifdef SHARED +# if IS_IN (rtld) +/* Inside ld.so we use the local alias to avoid runtime GOT + relocations. */ +# define __GLRO(rOUT, rGOT, member, offset) \ + lwz rOUT,_rtld_local_ro@got(rGOT); \ + lwz rOUT,offset(rOUT) +# else +# define __GLRO(rOUT, rGOT, member, offset) \ + lwz rOUT,_rtld_global_ro@got(rGOT); \ + lwz rOUT,offset(rOUT) +# endif +# else +# define __GLRO(rOUT, rGOT, member, offset) \ + lwz rOUT,member@got(rGOT); \ + lwz rOUT,0(rOUT) +# endif +#else +/* Position-dependent code does not require access to the GOT. */ +# define __GLRO(rOUT, rGOT, member, offset) \ + lis rOUT,(member+LOWORD)@ha \ + lwz rOUT,(member+LOWORD)@l(rOUT) +#endif /* PIC */ + #endif /* __ASSEMBLER__ */ diff --git a/sysdeps/powerpc/powerpc64/a2/memcpy.S b/sysdeps/powerpc/powerpc64/a2/memcpy.S index 6d5c5afddb..e515255126 100644 --- a/sysdeps/powerpc/powerpc64/a2/memcpy.S +++ b/sysdeps/powerpc/powerpc64/a2/memcpy.S @@ -18,6 +18,7 @@ <https://www.gnu.org/licenses/>. */ #include <sysdep.h> +#include <rtld-global-offsets.h> #ifndef MEMCPY # define MEMCPY memcpy @@ -27,8 +28,19 @@ #define ZERO_AHEAD 2 /* no cache lines DST zeroing ahead */ .section ".toc","aw" -.LC0: - .tc __cache_line_size[TC],__cache_line_size +.LC__dl_cache_line_size: +#ifdef SHARED +# if IS_IN (rtld) + /* Inside ld.so we use the local alias to avoid runtime GOT + relocations. */ + .tc _rtld_local_ro[TC],_rtld_local_ro +# else + .tc _rtld_global_ro[TC],_rtld_global_ro +# endif +#else + .tc _dl_cache_line_size[TC],_dl_cache_line_size +#endif + .section ".text" .align 2 @@ -55,7 +67,8 @@ ENTRY (MEMCPY, 5) */ neg r8,r3 /* LS 4 bits = # bytes to 8-byte dest bdry */ - ld r9,.LC0@toc(r2) /* Get cache line size (part 1) */ + /* Get cache line size (part 1) */ + ld r9,.LC__dl_cache_line_size@toc(r2) clrldi r8,r8,64-4 /* align to 16byte boundary */ sub r7,r4,r3 /* compute offset to src from dest */ lwz r9,0(r9) /* Get cache line size (part 2) */ @@ -121,7 +134,7 @@ L(dst_aligned): cmpdi cr0,r9,0 /* Cache line size set? */ bne+ cr0,L(cachelineset) -/* __cache_line_size not set: generic byte copy without much optimization */ +/* Cache line size not set: generic byte copy without much optimization */ clrldi. r0,r5,63 /* If length is odd copy one byte */ beq L(cachelinenotset_align) lbz r7,0(r4) /* Read one byte from source */ diff --git a/sysdeps/powerpc/powerpc64/memset.S b/sysdeps/powerpc/powerpc64/memset.S index 5d96696e87..d49ef102d4 100644 --- a/sysdeps/powerpc/powerpc64/memset.S +++ b/sysdeps/powerpc/powerpc64/memset.S @@ -17,10 +17,22 @@ <https://www.gnu.org/licenses/>. */ #include <sysdep.h> +#include <rtld-global-offsets.h> .section ".toc","aw" -.LC0: - .tc __cache_line_size[TC],__cache_line_size +.LC__dl_cache_line_size: +#ifdef SHARED +# if IS_IN (rtld) + /* Inside ld.so we use the local alias to avoid runtime GOT + relocations. */ + .tc _rtld_local_ro[TC],_rtld_local_ro +# else + .tc _rtld_global_ro[TC],_rtld_global_ro +# endif +#else + .tc _dl_cache_line_size[TC],_dl_cache_line_size +#endif + .section ".text" .align 2 @@ -146,8 +158,14 @@ L(zloopstart): /* If the remaining length is less the 32 bytes, don't bother getting the cache line size. */ beq L(medium) - ld rCLS,.LC0@toc(r2) + ld rCLS,.LC__dl_cache_line_size@toc(r2) +# ifdef SHARED + /* Load _rtld_global_ro._dl_cache_line_size. */ + lwz rCLS,RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET(rCLS) +# else + /* Load _dl_cache_line_size. */ lwz rCLS,0(rCLS) +# endif /* If the cache line size was not set just goto to L(nondcbz) which is safe for any cache line size. */ cmpldi cr1,rCLS,0 diff --git a/sysdeps/powerpc/rtld-global-offsets.sym b/sysdeps/powerpc/rtld-global-offsets.sym index f5ea5a1466..6b348fd522 100644 --- a/sysdeps/powerpc/rtld-global-offsets.sym +++ b/sysdeps/powerpc/rtld-global-offsets.sym @@ -6,3 +6,4 @@ RTLD_GLOBAL_RO_DL_HWCAP_OFFSET rtld_global_ro_offsetof (_dl_hwcap) RTLD_GLOBAL_RO_DL_HWCAP2_OFFSET rtld_global_ro_offsetof (_dl_hwcap2) +RTLD_GLOBAL_RO_DL_CACHE_LINE_SIZE_OFFSET rtld_global_ro_offsetof (_dl_cache_line_size) diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-auxv.h b/sysdeps/unix/sysv/linux/powerpc/dl-auxv.h new file mode 100644 index 0000000000..44bea09974 --- /dev/null +++ b/sysdeps/unix/sysv/linux/powerpc/dl-auxv.h @@ -0,0 +1,24 @@ +/* Auxiliary vector processing. Linux/PPC version. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +/* Scan the Aux Vector for the "Data Cache Block Size" entry and assign it + to dl_cache_line_size. */ +#define DL_PLATFORM_AUXV \ + case AT_DCACHEBSIZE: \ + GLRO(dl_cache_line_size) = av->a_un.a_val; \ + break; diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-support.c b/sysdeps/unix/sysv/linux/powerpc/dl-support.c new file mode 100644 index 0000000000..2a792d7092 --- /dev/null +++ b/sysdeps/unix/sysv/linux/powerpc/dl-support.c @@ -0,0 +1,23 @@ +/* Support for dynamic linking code in static libc. Linux/PPC version. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include "dl-auxv.h" +#include <ldsodefs.h> +int GLRO(dl_cache_line_size); + +#include <elf/dl-support.c> diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c b/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c index fa19cc66c0..6d51d6061e 100644 --- a/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c +++ b/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c @@ -18,16 +18,6 @@ #include <config.h> #include <ldsodefs.h> - -int __cache_line_size attribute_hidden; - -/* Scan the Aux Vector for the "Data Cache Block Size" entry. If found - verify that the static extern __cache_line_size is defined by checking - for not NULL. If it is defined then assign the cache block size - value to __cache_line_size. */ -#define DL_PLATFORM_AUXV \ - case AT_DCACHEBSIZE: \ - __cache_line_size = av->a_un.a_val; \ - break; +#include <dl-auxv.h> #include <sysdeps/unix/sysv/linux/dl-sysdep.c> diff --git a/sysdeps/unix/sysv/linux/powerpc/libc-start.c b/sysdeps/unix/sysv/linux/powerpc/libc-start.c index a659a9144f..3a5eb0e952 100644 --- a/sysdeps/unix/sysv/linux/powerpc/libc-start.c +++ b/sysdeps/unix/sysv/linux/powerpc/libc-start.c @@ -24,7 +24,6 @@ #include <hwcapinfo.h> #endif -int __cache_line_size attribute_hidden; /* The main work is done in the generic function. */ #define LIBC_START_MAIN generic_start_main #define LIBC_START_DISABLE_INLINE @@ -71,15 +70,12 @@ __libc_start_main (int argc, char **argv, rtld_fini = NULL; } - /* Initialize the __cache_line_size variable from the aux vector. For the - static case, we also need _dl_hwcap, _dl_hwcap2 and _dl_platform, so we - can call __tcb_parse_hwcap_and_convert_at_platform (). */ for (ElfW (auxv_t) * av = auxvec; av->a_type != AT_NULL; ++av) switch (av->a_type) { - case AT_DCACHEBSIZE: - __cache_line_size = av->a_un.a_val; - break; + /* For the static case, we also need _dl_hwcap, _dl_hwcap2 and + _dl_platform, so we can call + __tcb_parse_hwcap_and_convert_at_platform (). */ #ifndef SHARED case AT_HWCAP: _dl_hwcap = (unsigned long int) av->a_un.a_val;
GCC 10.0 enabled -fno-common by default and this started to point that __cache_line_size had been implemented in 2 different places: loader and libc. In order to avoid this duplication, the libc variable has been removed and the loader variable is moved to rtld_global_ro. File sysdeps/unix/sysv/linux/powerpc/dl-auxv.h has been added in order to reuse code for both static and dynamic linking scenarios. Signed-off-by: Tulio Magno Quites Machado Filho <tuliom@linux.ibm.com> --- sysdeps/powerpc/dl-procinfo.c | 17 ++++++++++++ sysdeps/powerpc/powerpc32/a2/memcpy.S | 23 ++++++++-------- sysdeps/powerpc/powerpc32/dl-machine.c | 11 ++------ sysdeps/powerpc/powerpc32/memset.S | 29 +++++++++----------- sysdeps/powerpc/powerpc32/sysdep.h | 26 ++++++++++++++++++ sysdeps/powerpc/powerpc64/a2/memcpy.S | 21 +++++++++++--- sysdeps/powerpc/powerpc64/memset.S | 24 ++++++++++++++-- sysdeps/powerpc/rtld-global-offsets.sym | 1 + sysdeps/unix/sysv/linux/powerpc/dl-auxv.h | 24 ++++++++++++++++ sysdeps/unix/sysv/linux/powerpc/dl-support.c | 23 ++++++++++++++++ sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c | 12 +------- sysdeps/unix/sysv/linux/powerpc/libc-start.c | 10 ++----- 12 files changed, 160 insertions(+), 61 deletions(-) create mode 100644 sysdeps/unix/sysv/linux/powerpc/dl-auxv.h create mode 100644 sysdeps/unix/sysv/linux/powerpc/dl-support.c