This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCHv3] powerpc: ABI change - add HWCAP/HWCAP2/platform info to TCB
- From: Steven Munroe <munroesj at linux dot vnet dot ibm dot com>
- To: "Carlos O'Donell" <carlos at redhat dot com>
- Cc: Carlos Eduardo Seo <cseo at linux dot vnet dot ibm dot com>, Peter Bergner <bergner at vnet dot ibm dot com>, Roland McGrath <roland at hack dot frob dot com>, Joseph Myers <joseph at codesourcery dot com>, GNU C Library <libc-alpha at sourceware dot org>, Tulio Machado <tuliom at linux dot vnet dot ibm dot com>, "Steven J. Munroe" <sjmunroe at us dot ibm dot com>
- Date: Thu, 08 Oct 2015 10:30:22 -0500
- Subject: Re: [PATCHv3] powerpc: ABI change - add HWCAP/HWCAP2/platform info to TCB
- Authentication-results: sourceware.org; auth=none
- References: <4BE991CB-F601-4D63-A416-5991FE870FC4 at linux dot vnet dot ibm dot com> <alpine dot DEB dot 2 dot 10 dot 1509232114070 dot 10585 at digraph dot polyomino dot org dot uk> <B650304C-6207-46AC-B6A8-FA29959B305D at linux dot vnet dot ibm dot com> <alpine dot DEB dot 2 dot 10 dot 1509232141250 dot 10585 at digraph dot polyomino dot org dot uk> <1443046986 dot 23503 dot 68 dot camel at otta> <alpine dot DEB dot 2 dot 10 dot 1509232234500 dot 10585 at digraph dot polyomino dot org dot uk> <1443050731 dot 23503 dot 78 dot camel at otta> <alpine dot DEB dot 2 dot 10 dot 1509232340470 dot 10585 at digraph dot polyomino dot org dot uk> <1443057751 dot 13186 dot 6 dot camel at otta> <alpine dot DEB dot 2 dot 10 dot 1509240126140 dot 10585 at digraph dot polyomino dot org dot uk> <1443058522 dot 13186 dot 9 dot camel at otta> <1443654009 dot 4885 dot 4 dot camel at oc7878010663> <B2EB15A6-FEB5-4DBA-8719-0BAA5B65EF5E at linux dot vnet dot ibm dot com> <5615CD5D dot 1080409 at redhat dot com>
- Reply-to: munroesj at linux dot vnet dot ibm dot com
On Wed, 2015-10-07 at 21:56 -0400, Carlos O'Donell wrote:
> On 10/02/2015 11:48 AM, Carlos Eduardo Seo wrote:
> >> > On Sep 30, 2015, at 8:00 PM, Steven Munroe <munroesj@linux.vnet.ibm.com> wrote:
> >> >
> >> > With this agreed to, and the changes suggested by Joseph and Roland, I
> >> > think we have consensus.
> >> >
> >> > Carlos please proceed.
> >> >
> > Okay. Here is a patch that incorporates the changes suggested by
> > Joseph and Roland. Changed the versioned symbol name and namespace.
>
> Excellent work integrating the various suggestions and requirements,
> There are still a few lingering problems though, and I expect within
> one or two more iterations we'll have this done.
>
> (a) Reference to GLIBC_2.23 in ppc.h functions?
>
> It is clear that Peter Bergner is going to use gcc to emit code that
> references the TCB offsets directly to produce the builtins, and that
> a data reference to __parse_hwcap_and_convert_at_platform will ensure
> the application depends on a versioned dependency of GLIBC_2.23 to
> avoid application compatibility issues.
>
> However, it doesn't appear that __ppc_get_hwcap or __ppc_get_at_platform
> generate any references to __parse_hwcap_and_convert_at_platform? These
> two functions as added in the manual could be used by applications
> directly by including the platform header and calling those functions.
> Such applications would have no such references to __parse* and as such
> would not be protected by the dynamic loader library version checking.
> The same referecne to __parse_hwcap_and_convert_at_platform as gcc is
> using needs to be emitted in the inline assembly from those functions
> and you should double check that using these functions results in a binary
> that has a reference to a GLIBC_2.23 versioned symbol, and that running
> it on an older glibc causes an error.
>
We can drop these (__ppc_get_hwcap or __ppc_get_at_platform) if we can
guarantee that the TCB fields are initialized before we need to run
library init or CTORs. This guarantee applies to static and dynamic
linking.
> (b) Definition of _dl_powerpc_platforms in test program.
>
> This is an indication that something is wrong. No user code should ever
> need to define a _dl_* data symbol. Everything should just work for -static
> and -shared, otherwise you have something wrong and still need to fix it.
>
> Why is this definition needed?
>
> (c) Avoid custom dl-support.c
>
> Are you sure you can't avoid this? Can we add AT_PLATFORM directly to the
> case statement used in _dl_aux_init?
>
> (c) libc-start.c handling of AT_*
>
> This seems like your double processing these entries here and in _dl_aux_init?
> Could you please verify this?
>
> (d) Nit-pics about FAIL/PASS
>
> See POSIX compliant testing notes from DejaGNU.
>
> > 0001-powerpc-Add-hwcap-hwcap2-platform-data-to-TCB.patch
> >
> >
> > From 3c6f51f776766d2136e4f66519537a8dd24303ca Mon Sep 17 00:00:00 2001
> > From: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
> > Date: Fri, 2 Oct 2015 12:36:48 -0300
> > Subject: [PATCH] powerpc: Add hwcap/hwcap2/platform data to TCB.
> >
> > This patch adds a new feature for powerpc. In order to get faster access to
> > the HWCAP/HWCAP2 bits and platform number (i.e. for implementing
> > __builtin_cpu_is () / __builtin_cpu_supports () in GCC) without the overhead of
> > reading from the auxiliary vector, we now reserve space for them in the TCB.
> > This is an ABI change for GLIBC 2.23.
> >
> > A new versioned symbol '__parse_hwcap_and_convert_at_platform' is available to
> > get the data from the auxiliary vector and parse it, and store it for later use
> > in the TLS initialization code. This function is called very early
> > (in _dl_sysdep_start () via DL_PLATFORM_INFO for the dynamic linking case, and
> > in __libc_start_main () for the static linking case) to make sure the data is
> > available at the time of TLS initialization.
> >
> > A new API is published in ppc.h to get the HWCAP+HWCAP2 and platform number
> > data from the TCB as well.
> >
> > 2015-10-02 Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
> >
> > * manual/platform.texi: Added a manual entry for the new API.
> > * sysdeps/powerpc/Makefile: Added testcases and new hwcapinfo file to
> > the Makefile.
> > * sysdeps/powerpc/Versions: Added new
> > __parse_hwcap_and_convert_at_platform symbol to GLIBC-2.23.
> > * sysdeps/powerpc/hwcapinfo.c: New file.
> > (__tcb_parse_hwcap_and_convert_at_platform): New function to initialize
> > and parse hwcap, hwcap2 and platform number information.
> > * sysdeps/powerpc/hwcapinfo.h: New file. Creates global variables
> > to store HWCAP+HWCAP2 and platform number.
> > * sysdeps/powerpc/nptl/tcb-offsets.sym: Added new offests
> > for HWCAP+HWCAP2 and platform number in the TCB.
> > * sysdeps/powerpc/nptl/tls.h: New functionality. Stores
> > the HWCAP, HWCAP2 and platform number in the TCB.
> > (dtv): Added new fields for HWCAP+HWCAP2 and platform number.
> > (TLS_INIT_TP): Included calls to add the hwcap and
> > at_platform values in the TCB in TP initialization.
> > (TLS_DEFINE_INIT_TP): Likewise.
> > (THREAD_GET_HWCAP): New macro.
> > (THREAD_SET_HWCAP): Likewise.
> > (THREAD_GET_AT_PLATFORM): Likewise.
> > (THREAD_SET_AT_PLATFORM): Likewise.
> > * sysdeps/powerpc/powerpc32/dl-machine.h:
> > (dl_platform_init): New function that calls
> > __parse_hwcap_and_convert_at_platform for the dymanic linking case for
> > powerpc32.
> > * sysdeps/powerpc/powerpc64/dl-machine.h: Likewise, for powerpc64.
> > * sysdeps/powerpc/sys/platform/ppc.h: Added new functions
> > to get the HWCAP+HWCAP2 and platform number values from the TCB.
> > * sysdeps/powerpc/test-get_hwcap-static.c: New file. Testcase for
> > this functionality, static linking case.
> > * sysdeps/powerpc/test-get_hwcap.c: New file. Likewise, dynamic
> > linking case.
> > * sysdeps/unix/sysv/linux/powerpc/dl-support.c: New file.
> > Adds support for AT_PLATFORM in static glibc.
> > * sysdeps/unix/sysv/linux/powerpc/libc-start.c: Added call to
> > __parse_hwcap_and_convert_at_platform for the static linking case.
> > * sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist:
> > Included the new __parse_hwcap_and_convert_at_platform symbol in the
> > ABI list for GLIBC 2.23.
> > * sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist:
> > Likewise.
> > * sysdeps/unix/sysv/linux/powerpc/powerpc64/libc.abilist:
> > Likewise.
> > ---
> > manual/platform.texi | 20 +++
> > sysdeps/powerpc/Makefile | 12 +-
> > sysdeps/powerpc/Versions | 5 +
> > sysdeps/powerpc/hwcapinfo.c | 71 +++++++++
> > sysdeps/powerpc/hwcapinfo.h | 30 ++++
> > sysdeps/powerpc/nptl/tcb-offsets.sym | 8 +
> > sysdeps/powerpc/nptl/tls.h | 44 +++++-
> > sysdeps/powerpc/powerpc32/dl-machine.h | 14 ++
> > sysdeps/powerpc/powerpc64/dl-machine.h | 14 ++
> > sysdeps/powerpc/sys/platform/ppc.h | 57 +++++++
> > sysdeps/powerpc/test-get_hwcap-static.c | 24 +++
> > sysdeps/powerpc/test-get_hwcap.c | 164 +++++++++++++++++++++
> > sysdeps/unix/sysv/linux/powerpc/dl-support.c | 25 ++++
> > sysdeps/unix/sysv/linux/powerpc/libc-start.c | 24 ++-
> > .../unix/sysv/linux/powerpc/powerpc32/ld.abilist | 3 +
> > .../sysv/linux/powerpc/powerpc64/ld-le.abilist | 3 +
> > .../unix/sysv/linux/powerpc/powerpc64/ld.abilist | 3 +
> > 17 files changed, 514 insertions(+), 7 deletions(-)
> > create mode 100644 sysdeps/powerpc/hwcapinfo.c
> > create mode 100644 sysdeps/powerpc/hwcapinfo.h
> > create mode 100644 sysdeps/powerpc/test-get_hwcap-static.c
> > create mode 100644 sysdeps/powerpc/test-get_hwcap.c
> > create mode 100644 sysdeps/unix/sysv/linux/powerpc/dl-support.c
> >
> > diff --git a/manual/platform.texi b/manual/platform.texi
> > index cb16664..e47b170 100644
> > --- a/manual/platform.texi
> > +++ b/manual/platform.texi
> > @@ -14,6 +14,26 @@
> > Facilities specific to PowerPC that are not specific to a particular
> > operating system are declared in @file{sys/platform/ppc.h}.
> >
> > +@deftypefun {uint64_t} __ppc_get_hwcap (void)
> > +@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
> > +Read the current value of the hardware capability (HWCAP and HCWCAP2) bits.
> > +This function, as opposed to @code{getauxval}, is designed to be as fast
> > +as possible including caching the result to reduce access latency.
> > +
> > +The result returned is a single doubleword containing the HWCAP value in the
> > +32 high-order bits and the HWCAP2 value in the lower 32 bits.
>
> OK.
>
> > +@end deftypefun
> > +
> > +@deftypefun {uint32_t} __ppc_get_at_platform (void)
> > +@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
> > +Read the current value of the platform number. This function, as opposed to
> > +@code{getauxval}, is designed to be as fast as possible including caching the
> > +result to reduce access latency.
> > +
> > +The platform number identifies the current processor. It is derived from
> > +the AT_PLATFORM variable in the auxiliary vector.
> > +@end deftypefun
> > +
>
> OK.
>
> > @deftypefun {uint64_t} __ppc_get_timebase (void)
> > @safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
> > Read the current value of the Time Base Register.
> > diff --git a/sysdeps/powerpc/Makefile b/sysdeps/powerpc/Makefile
> > index 533d763..77dcb7d 100644
> > --- a/sysdeps/powerpc/Makefile
> > +++ b/sysdeps/powerpc/Makefile
> > @@ -4,10 +4,10 @@ endif
> >
> > ifeq ($(subdir),elf)
> > # extra shared linker files to link into dl-allobjs.so and libc
> > -sysdep-dl-routines += dl-machine
> > -sysdep_routines += dl-machine
> > +sysdep-dl-routines += dl-machine hwcapinfo
> > +sysdep_routines += dl-machine hwcapinfo
>
> OK.
>
> > # extra shared linker files to link only into dl-allobjs.so
> > -sysdep-rtld-routines += dl-machine
> > +sysdep-rtld-routines += dl-machine hwcapinfo
>
> OK.
>
> > # Don't optimize GD tls sequence to LE.
> > LDFLAGS-tst-tlsopt-powerpc += -Wl,--no-tls-optimize
> > tests += tst-tlsopt-powerpc
> > @@ -26,6 +26,12 @@ gen-as-const-headers += rtld-global-offsets.sym
> > gen-as-const-headers += locale-defines.sym
> > endif
> >
> > +ifeq ($(subdir),nptl)
> > +sysdep_headers += sys/platform/ppc.h
> > +tests += test-get_hwcap test-get_hwcap-static
> > +tests-static += test-get_hwcap-static
> > +endif
> > +
>
> OK.
>
> > ifeq ($(subdir),misc)
> > sysdep_headers += sys/platform/ppc.h
> > tests += test-gettimebase
> > diff --git a/sysdeps/powerpc/Versions b/sysdeps/powerpc/Versions
> > index 2aebf7c..4af8de2 100644
> > --- a/sysdeps/powerpc/Versions
> > +++ b/sysdeps/powerpc/Versions
> > @@ -20,4 +20,9 @@ ld {
> > GLIBC_2.22 {
> > __tls_get_addr_opt;
> > }
> > + GLIBC_2.23 {
> > + # symbol used to version control when the ABI started to specify that HWCAP
> > + # and AT_PLATFORM data should be stored into the TCB.
>
> Please mention that gcc will emit a data reference to this symbol when it generates
> code that uses the new TCB entries.
>
> OK with that change.
>
> > + __parse_hwcap_and_convert_at_platform;
> > + }
> > }
> > diff --git a/sysdeps/powerpc/hwcapinfo.c b/sysdeps/powerpc/hwcapinfo.c
> > new file mode 100644
> > index 0000000..0afacdf
> > --- /dev/null
> > +++ b/sysdeps/powerpc/hwcapinfo.c
> > @@ -0,0 +1,71 @@
> > +/* powerpc HWCAP/HWCAP2 and AT_PLATFORM data pre-processing.
> > + Copyright (C) 2015 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
> > + <http://www.gnu.org/licenses/>. */
> > +
> > +#include <unistd.h>
> > +#include <shlib-compat.h>
> > +#include <dl-procinfo.h>
> > +
> > +uint64_t __tcb_hwcap __attribute__ ((visibility ("hidden")));
> > +uint32_t __tcb_platform __attribute__ ((visibility ("hidden")));
>
> OK.
>
> I hope that's enough bits ;-)
>
> > +
> > +/* This function parses the HWCAP/HWCAP2 fields, adding the previous supported
> > + ISA bits, as well as converting the AT_PLATFORM string to a number. This
> > + data is stored in two global variables that can be used later by the
> > + powerpc-specific code to store it into the TCB. */
> > +void
> > +__tcb_parse_hwcap_and_convert_at_platform (void)
> > +{
> > +
> > + uint64_t h1, h2;
> > +
> > + /* Read AT_PLATFORM string from auxv and convert it to a number. */
> > + __tcb_platform = _dl_string_platform (GLRO (dl_platform));
> > +
> > + /* Read HWCAP and HWCAP2 from auxv. */
> > + h1 = GLRO (dl_hwcap);
> > + h2 = GLRO (dl_hwcap2);
> > +
> > + /* hwcap contains only the latest supported ISA, the code checks which is
> > + and fills the previous supported ones. */
> > +
> > + if (h1 & PPC_FEATURE_ARCH_2_06)
> > + h1 |= PPC_FEATURE_ARCH_2_05 | \
> > + PPC_FEATURE_POWER5_PLUS | \
> > + PPC_FEATURE_POWER5 | \
> > + PPC_FEATURE_POWER4;
> > + else if (h1 & PPC_FEATURE_ARCH_2_05)
> > + h1 |= PPC_FEATURE_POWER5_PLUS | \
> > + PPC_FEATURE_POWER5 | \
> > + PPC_FEATURE_POWER4;
> > + else if (h1 & PPC_FEATURE_POWER5_PLUS)
> > + h1 |= PPC_FEATURE_POWER5 | \
> > + PPC_FEATURE_POWER4;
> > + else if (h1 & PPC_FEATURE_POWER5)
> > + h1|= PPC_FEATURE_POWER4;
> > +
> > + /* Consolidate both HWCAP and HWCAP2 into a single doubleword so that
> > + we can read both in a single load later. */
> > + __tcb_hwcap = h2;
> > + __tcb_hwcap = (h1 << 32) | __tcb_hwcap;
>
> OK.
>
> > +
> > +}
> > +#if IS_IN (rtld)
> > +versioned_symbol (ld, __tcb_parse_hwcap_and_convert_at_platform, \
> > + __parse_hwcap_and_convert_at_platform, GLIBC_2_23);
>
> OK.
>
> > +#endif
> > +
> > diff --git a/sysdeps/powerpc/hwcapinfo.h b/sysdeps/powerpc/hwcapinfo.h
> > new file mode 100644
> > index 0000000..72bcea8
> > --- /dev/null
> > +++ b/sysdeps/powerpc/hwcapinfo.h
> > @@ -0,0 +1,30 @@
> > +/* powerpc HWCAP/HWCAP2 and AT_PLATFORM data pre-processing.
> > + Copyright (C) 2015 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
> > + <http://www.gnu.org/licenses/>. */
> > +
> > +#include <stdint.h>
> > +
> > +#ifndef HWCAPINFO_H
> > +# define HWCAPINFO_H
> > +
> > +extern uint64_t __tcb_hwcap attribute_hidden;
> > +extern uint32_t __tcb_platform attribute_hidden;
> > +
> > +extern void __tcb_parse_hwcap_and_convert_at_platform (void);
>
> OK.
>
> > +
> > +#endif
> > +
> > diff --git a/sysdeps/powerpc/nptl/tcb-offsets.sym b/sysdeps/powerpc/nptl/tcb-offsets.sym
> > index d955142..f580e69 100644
> > --- a/sysdeps/powerpc/nptl/tcb-offsets.sym
> > +++ b/sysdeps/powerpc/nptl/tcb-offsets.sym
> > @@ -19,7 +19,15 @@ POINTER_GUARD (offsetof (tcbhead_t, pointer_guard) - TLS_TCB_OFFSET - sizeof (
> > TAR_SAVE (offsetof (tcbhead_t, tar_save) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
> > DSO_SLOT1 (offsetof (tcbhead_t, dso_slot1) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
> > DSO_SLOT2 (offsetof (tcbhead_t, dso_slot2) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
> > +#ifdef __powerpc64__
> > +TCB_AT_PLATFORM (offsetof (tcbhead_t, at_platform) - TLS_TCB_OFFSET - sizeof(tcbhead_t))
> > +#endif
> > TM_CAPABLE (offsetof (tcbhead_t, tm_capable) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
> > +#ifndef __powerpc64__
> > +TCB_AT_PLATFORM (offsetof (tcbhead_t, at_platform) - TLS_TCB_OFFSET - sizeof(tcbhead_t))
> > +PADDING (offsetof (tcbhead_t, padding) - TLS_TCB_OFFSET - sizeof(tcbhead_t))
> > +#endif
> > +TCB_HWCAP (offsetof (tcbhead_t, hwcap) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
>
> OK.
>
> > #ifndef __ASSUME_PRIVATE_FUTEX
> > PRIVATE_FUTEX_OFFSET thread_offsetof (header.private_futex)
> > #endif
> > diff --git a/sysdeps/powerpc/nptl/tls.h b/sysdeps/powerpc/nptl/tls.h
> > index 1f3d97a..7b6682c 100644
> > --- a/sysdeps/powerpc/nptl/tls.h
> > +++ b/sysdeps/powerpc/nptl/tls.h
> > @@ -44,6 +44,8 @@ typedef union dtv
> >
> > #ifndef __ASSEMBLER__
> >
> > +# include <hwcapinfo.h>
> > +
> > /* Get system call information. */
> > # include <sysdep.h>
> >
> > @@ -63,8 +65,24 @@ typedef union dtv
> > are private. */
> > typedef struct
> > {
> > + /* Reservation for HWCAP data. To be accessed by GCC in
> > + __builtin_cpu_supports(), so it is a part of public ABI. */
>
> s/part of/part of the/g
>
> OK with that change.
>
> > + uint64_t hwcap;
> > + /* Reservation for AT_PLATFORM data. To be accessed by GCC in
> > + __builtin_cpu_is(), so it is a part of public ABI. Since there
>
> Likewise.
>
> > + are different ABIs for 32 and 64 bit, we put this field in a
> > + previously empty padding space for powerpc64. */
> > +#ifndef __powerpc64__
> > + /* Padding to maintain alignment. */
> > + uint32_t padding;
> > + uint32_t at_platform;
> > +#endif
> > /* Indicate if HTM capable (ISA 2.07). */
> > - int tm_capable;
> > + uint32_t tm_capable;
> > + /* Reservation for AT_PLATFORM data - powerpc64. */
> > +#ifdef __powerpc64__
> > + uint32_t at_platform;
> > +#endif
> > /* Reservation for Dynamic System Optimizer ABI. */
> > uintptr_t dso_slot2;
> > uintptr_t dso_slot1;
> > @@ -134,7 +152,9 @@ register void *__thread_register __asm__ ("r13");
> > # define TLS_INIT_TP(tcbp) \
> > ({ \
> > __thread_register = (void *) (tcbp) + TLS_TCB_OFFSET; \
> > - THREAD_SET_TM_CAPABLE (GLRO (dl_hwcap2) & PPC_FEATURE2_HAS_HTM ? 1 : 0); \
> > + THREAD_SET_TM_CAPABLE (__tcb_hwcap & PPC_FEATURE2_HAS_HTM ? 1 : 0); \
> > + THREAD_SET_HWCAP (__tcb_hwcap); \
> > + THREAD_SET_AT_PLATFORM (__tcb_platform); \
>
> OK, setup main thread.
>
> > NULL; \
> > })
> >
> > @@ -142,7 +162,11 @@ register void *__thread_register __asm__ ("r13");
> > # define TLS_DEFINE_INIT_TP(tp, pd) \
> > void *tp = (void *) (pd) + TLS_TCB_OFFSET + TLS_PRE_TCB_SIZE; \
> > (((tcbhead_t *) ((char *) tp - TLS_TCB_OFFSET))[-1].tm_capable) = \
> > - THREAD_GET_TM_CAPABLE ();
> > + THREAD_GET_TM_CAPABLE (); \
> > + (((tcbhead_t *) ((char *) tp - TLS_TCB_OFFSET))[-1].hwcap) = \
> > + THREAD_GET_HWCAP (); \
> > + (((tcbhead_t *) ((char *) tp - TLS_TCB_OFFSET))[-1].at_platform) = \
> > + THREAD_GET_AT_PLATFORM ();
>
> OK, setup all other threads.
>
> >
> > /* Return the address of the dtv for the current thread. */
> > # define THREAD_DTV() \
> > @@ -203,6 +227,20 @@ register void *__thread_register __asm__ ("r13");
> > # define THREAD_SET_TM_CAPABLE(value) \
> > (THREAD_GET_TM_CAPABLE () = (value))
> >
> > +/* hwcap field in TCB head. */
> > +# define THREAD_GET_HWCAP() \
> > + (((tcbhead_t *) ((char *) __thread_register \
> > + - TLS_TCB_OFFSET))[-1].hwcap)
> > +# define THREAD_SET_HWCAP(value) \
> > + (THREAD_GET_HWCAP () = (value))
> > +
> > +/* at_platform field in TCB head. */
> > +# define THREAD_GET_AT_PLATFORM() \
> > + (((tcbhead_t *) ((char *) __thread_register \
> > + - TLS_TCB_OFFSET))[-1].at_platform)
> > +# define THREAD_SET_AT_PLATFORM(value) \
> > + (THREAD_GET_AT_PLATFORM () = (value))
> > +
>
> OK.
>
> > /* l_tls_offset == 0 is perfectly valid on PPC, so we have to use some
> > different value to mean unset l_tls_offset. */
> > # define NO_TLS_OFFSET -1
> > diff --git a/sysdeps/powerpc/powerpc32/dl-machine.h b/sysdeps/powerpc/powerpc32/dl-machine.h
> > index 8b0c067..8d2f311 100644
> > --- a/sysdeps/powerpc/powerpc32/dl-machine.h
> > +++ b/sysdeps/powerpc/powerpc32/dl-machine.h
> > @@ -24,6 +24,7 @@
> > #include <assert.h>
> > #include <dl-tls.h>
> > #include <dl-irel.h>
> > +#include <hwcapinfo.h>
> >
> > /* Translate a processor specific dynamic tag to the index
> > in l_info array. */
> > @@ -150,6 +151,19 @@ __elf_preferred_address(struct link_map *loader, size_t maplength,
> > #define ELF_MACHINE_NO_REL 1
> > #define ELF_MACHINE_NO_RELA 0
> >
> > +/* We define an initialization function to initialize HWCAP/HWCAP2 and
> > + platform data so it can be copied into the TCB later. This is called
> > + very early in _dl_sysdep_start for dynamically linked binaries. */
> > +#ifdef SHARED
> > +# define DL_PLATFORM_INIT dl_platform_init ()
> > +
> > +static inline void __attribute__ ((unused))
> > +dl_platform_init (void)
> > +{
> > + __tcb_parse_hwcap_and_convert_at_platform ();
> > +}
> > +#endif
>
> OK.
>
> > +
> > /* Set up the loaded object described by MAP so its unrelocated PLT
> > entries will jump to the on-demand fixup code in dl-runtime.c.
> > Also install a small trampoline to be used by entries that have
> > diff --git a/sysdeps/powerpc/powerpc64/dl-machine.h b/sysdeps/powerpc/powerpc64/dl-machine.h
> > index 0576781..dc0f522 100644
> > --- a/sysdeps/powerpc/powerpc64/dl-machine.h
> > +++ b/sysdeps/powerpc/powerpc64/dl-machine.h
> > @@ -26,6 +26,7 @@
> > #include <sys/param.h>
> > #include <dl-tls.h>
> > #include <sysdep.h>
> > +#include <hwcapinfo.h>
> >
> > /* Translate a processor specific dynamic tag to the index
> > in l_info array. */
> > @@ -296,6 +297,19 @@ BODY_PREFIX "_dl_start_user:\n" \
> > #define ELF_MACHINE_NO_REL 1
> > #define ELF_MACHINE_NO_RELA 0
> >
> > +/* We define an initialization function to initialize HWCAP/HWCAP2 and
> > + platform data so it can be copied into the TCB later. This is called
> > + very early in _dl_sysdep_start for dynamically linked binaries. */
> > +#ifdef SHARED
> > +# define DL_PLATFORM_INIT dl_platform_init ()
> > +
> > +static inline void __attribute__ ((unused))
> > +dl_platform_init (void)
> > +{
> > + __tcb_parse_hwcap_and_convert_at_platform ();
> > +}
> > +#endif
>
> OK.
>
> > +
> > /* Stuff for the PLT. */
> > #if _CALL_ELF != 2
> > #define PLT_INITIAL_ENTRY_WORDS 3
> > diff --git a/sysdeps/powerpc/sys/platform/ppc.h b/sysdeps/powerpc/sys/platform/ppc.h
> > index 2594606..dab03ac 100644
> > --- a/sysdeps/powerpc/sys/platform/ppc.h
> > +++ b/sysdeps/powerpc/sys/platform/ppc.h
> > @@ -23,6 +23,63 @@
> > #include <stdint.h>
> > #include <bits/ppc.h>
> >
> > +/* Offsets copied from tcb-offsets.h. */
> > +
> > +#ifdef __powerpc64__
> > +# define __TPREG "r13"
> > +# define __HWCAPOFF -28776
> > +# define __ATPLATOFF -28764
> > +#else
> > +# define __TPREG "r2"
> > +# define __HWCAPOFF -28736
> > +# define __HWCAP2OFF -28732
> > +# define __ATPLATOFF -28724
> > +#endif
>
> OK.
>
> > +
> > +/* Get the hwcap/hwcap2 information from the TCB. */
> > +
> > +static __inline__ uint64_t
> > +__ppc_get_hwcap (void)
> > +{
> > +
> > + uint64_t __tcb_hwcap;
> > +
> > + register unsigned long __tp __asm__ (__TPREG);
> > +#ifdef __powerpc64__
> > + __asm__ ("ld %0,%1(%2)\n"
> > + : "=r" (__tcb_hwcap)
> > + : "i" (__HWCAPOFF), "b" (__tp));
> > +#else
> > + uint64_t h1, h2;
> > +
> > + __asm__ ("lwz %0,%1(%2)\n"
> > + : "=r" (h1)
> > + : "i" (__HWCAPOFF), "b" (__tp));
> > + __asm__ ("lwz %0,%1(%2)\n"
> > + : "=r" (h2)
> > + : "i" (__HWCAP2OFF), "b" (__tp));
> > + __tcb_hwcap = (h1 >> 32) << 32 | (h2 >> 32);
> > +#endif
> > +
> > + return __tcb_hwcap;
> > +}
> > +
> > +/* Get the AT_PLATFORM number from the TCB. */
> > +
> > +static __inline__ uint32_t
> > +__ppc_get_at_platform (void)
> > +{
> > +
> > + uint32_t __tcb_at_platform;
> > +
> > + register unsigned long __tp __asm__ (__TPREG);
> > + __asm__ ("lwz %0,%1(%2)\n"
> > + : "=r" (__tcb_at_platform)
> > + : "i" (__ATPLATOFF), "b" (__tp));
> > +
> > + return __tcb_at_platform;
> > +}
> > +
>
> Not OK. Neither of these functions reference the dummy symbol we're
> using to ensure compatibility?
>
> It would appear to me that an application including the platform
> specific header, and calling the inline function, would not have any
> reference against GLIBC_2.23 symbols, and would therefore fail to
> run correctly on older glibc.
>
> > /* Read the Time Base Register. */
> > static inline uint64_t
> > __ppc_get_timebase (void)
> > diff --git a/sysdeps/powerpc/test-get_hwcap-static.c b/sysdeps/powerpc/test-get_hwcap-static.c
> > new file mode 100644
> > index 0000000..965d8bd
> > --- /dev/null
> > +++ b/sysdeps/powerpc/test-get_hwcap-static.c
> > @@ -0,0 +1,24 @@
> > +/* Check __ppc_get_hwcap() and __ppc_get_at_plaftorm() functionality.
> > + Copyright (C) 2015 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
> > + <http://www.gnu.org/licenses/>. */
> > +
> > +/* Tests if the hwcap, hwcap2 and platform data are stored in the TCB. */
> > +
> > +#define STATIC_TST_HWCAP 1
> > +
> > +#include "test-get_hwcap.c"
> > +
>
> OK.
>
> > diff --git a/sysdeps/powerpc/test-get_hwcap.c b/sysdeps/powerpc/test-get_hwcap.c
> > new file mode 100644
> > index 0000000..1d2dfbf
> > --- /dev/null
> > +++ b/sysdeps/powerpc/test-get_hwcap.c
> > @@ -0,0 +1,164 @@
> > +/* Check __ppc_get_hwcap() and __ppc_get_at_plaftorm() functionality.
> > + Copyright (C) 2015 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
> > + <http://www.gnu.org/licenses/>. */
> > +
> > +/* Tests if the hwcap, hwcap2 and platform data are stored in the TCB. */
> > +
> > +#include <inttypes.h>
> > +#include <stdio.h>
> > +#include <stdint.h>
> > +#include <pthread.h>
> > +
> > +#include <sys/auxv.h>
> > +#include <sys/platform/ppc.h>
> > +
> > +#include <dl-procinfo.h>
> > +
> > +#ifndef STATIC_TST_HWCAP
> > +/* We need to duplicate this from dl-procinfo.c so _dl_string_platform can
> > + use it (dynamic linking case only - static case can get this from
> > + dl-support). */
> > +const char _dl_powerpc_platforms[14][12]
> > += {
> > + [PPC_PLATFORM_POWER4] = "power4",
> > + [PPC_PLATFORM_PPC970] = "ppc970",
> > + [PPC_PLATFORM_POWER5] = "power5",
> > + [PPC_PLATFORM_POWER5_PLUS] = "power5+",
> > + [PPC_PLATFORM_POWER6] = "power6",
> > + [PPC_PLATFORM_CELL_BE] = "ppc-cell-be",
> > + [PPC_PLATFORM_POWER6X] = "power6x",
> > + [PPC_PLATFORM_POWER7] = "power7",
> > + [PPC_PLATFORM_PPCA2] = "ppca2",
> > + [PPC_PLATFORM_PPC405] = "ppc405",
> > + [PPC_PLATFORM_PPC440] = "ppc440",
> > + [PPC_PLATFORM_PPC464] = "ppc464",
> > + [PPC_PLATFORM_PPC476] = "ppc476",
> > + [PPC_PLATFORM_POWER8] = "power8",
> > + };
> > +#endif
> > +
>
> This does not look correct.
>
> There should be no scenario where a user program is required to have
> a _dl_* symbol to make one of the API routines work. It should work
> transparently in -static and -shared compilations.
>
> You need to fix something or explain clearly what's going on here.
>
> > +uint64_t check_tcbhwcap (long tid)
> > +{
> > +
> > + uint32_t a1, at_platform;
> > + uint64_t h1, h2, hwcap, hwcap2;
> > + const char *at_platform_string;
> > +
> > + /* Testing the get functions and check if the hwcap/hwcap2 data is
> > + correctly initialized by TLS_TP_INIT. */
> > +
> > + h1 = __ppc_get_hwcap ();
> > + hwcap = getauxval (AT_HWCAP);
> > + hwcap2 = getauxval (AT_HWCAP2);
> > +
> > + /* hwcap contains only the latest supported ISA, the code checks which is
> > + and fills the previous supported ones. This is necessary because the
> > + same is done in hwcapinfo.c when setting the values that are copied to
> > + the TCB. */
> > +
> > + if (hwcap & PPC_FEATURE_ARCH_2_06)
> > + hwcap |= PPC_FEATURE_ARCH_2_05 | PPC_FEATURE_POWER5_PLUS |
> > + PPC_FEATURE_POWER5 | PPC_FEATURE_POWER4;
> > + else if (hwcap & PPC_FEATURE_ARCH_2_05)
> > + hwcap |= PPC_FEATURE_POWER5_PLUS | PPC_FEATURE_POWER5 | PPC_FEATURE_POWER4;
> > + else if (hwcap & PPC_FEATURE_POWER5_PLUS)
> > + hwcap |= PPC_FEATURE_POWER5 | PPC_FEATURE_POWER4;
> > + else if (hwcap & PPC_FEATURE_POWER5)
> > + hwcap |= PPC_FEATURE_POWER4;
> > +
> > + h2 = (hwcap << 32) + hwcap2;
> > +
> > + if ( h1 != h2 )
> > + {
> > + printf ("Fail: __ppc_get_hwcap() - HWCAP is %" PRIx64 ". Should be %"
> > + PRIx64 " for thread %ld.\n", h1, h2, tid);
> > + return 1;
>
> Not OK. You should run all tests not quit at the first failure.
>
> Please use "FAIL:" not "Fail:"
>
> > + }
> > +
> > + /* Same test for the platform number. */
> > + a1 = __ppc_get_at_platform ();
> > + at_platform_string = (const char *) getauxval (AT_PLATFORM);
> > + at_platform = _dl_string_platform (at_platform_string);
> > +
> > + if ( a1 != at_platform )
> > + {
> > + printf ("Fail: __ppc_get_at_platform() - AT_PLATFORM is %x. Should be %x"
> > + " for thread %ld\n", a1, at_platform, tid);
> > + return 1;
> > + }
> > +
> > + return 0;
>
> This should be `return err`, where err is set to one if any of the
> tests failed, or zero if no tests failed.
>
> > +}
> > +
> > +void *t1 (void *tid)
> > +{
> > + if (check_tcbhwcap ((long) tid))
> > + {
> > + pthread_exit (tid);
> > + }
> > +
> > + pthread_exit (NULL);
> > +
> > +}
>
> OK.
>
> > +
> > +static int
> > +do_test (void)
> > +{
> > +
> > + pthread_t threads[2];
> > + pthread_attr_t attr;
> > + pthread_attr_init (&attr);
> > + pthread_attr_setdetachstate (&attr, PTHREAD_CREATE_JOINABLE);
> > + void *status;
> > +
> > + long i = 0;
> > +
> > + /* Check for main. */
> > + if (check_tcbhwcap (i))
> > + {
> > + return 1;
> > + }
> > +
> > + /* Check for other thread. */
> > + i++;
> > + if (pthread_create (&threads[i], &attr, t1, (void *)i))
> > + {
> > + printf ("Fail: error creating thread %ld.\n", i);
> > + return 1;
> > + }
>
> OK, thanks for using threads.
>
> Again, note the use of "Fail:" should be "FAIL:", like the POSIX
> conforming test framework.
>
> See:
> http://www.gnu.org/software/dejagnu/manual/x47.html#posix
>
> > +
> > + pthread_attr_destroy (&attr);
> > + if (pthread_join (threads[i], &status))
> > + {
> > + printf ("Fail: error joining thread %ld.\n", i);
> > + return 1;
> > + }
> > + if (status)
> > + {
> > + return 1;
> > + }
> > +
> > + printf("Pass: HWCAP, HWCAP2 and AT_PLATFORM are correctly set in the TCB for"
> > + " all threads.\n");
>
> Similarly "PASS"
>
> > +
> > + pthread_exit (NULL);
> > +
> > +}
> > +
> > +#define TEST_FUNCTION do_test ()
> > +#include "../test-skeleton.c"
> > +
> > 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 0000000..a672eca
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/powerpc/dl-support.c
> > @@ -0,0 +1,25 @@
> > +/* Operating system support for dynamic linking code in static glibc.
> > + Linux/PPC version.
> > + Copyright (C) 2015 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
> > + <http://www.gnu.org/licenses/>. */
> > +
> > +#define DL_PLATFORM_AUXV \
> > + case AT_PLATFORM: \
> > + GLRO (dl_platform) = (void *) av->a_un.a_val; \
> > + break;
>
> This is not quite right. If you can use GLRO (dl_platform) during a static
> link, then every other platform should be able to do so also? Why can't
> you just add AT_PLATFORM directly into the case statement in _dl_aux_init
> like it is already listed in _dl_sysdep_start (shared version)?
>
> Note that cleaner is:
>
> dl-auxv.h [DL_PLATFORM_AUXV]: Define.
> dl-support.c: Include dl-auxv.h and elf/dl-support.c
>
> Like alpha does it, but that's not all that important. Here I'm looking
> to fix this across architectures if possible.
>
> > +
> > +#include <elf/dl-support.c>
> > diff --git a/sysdeps/unix/sysv/linux/powerpc/libc-start.c b/sysdeps/unix/sysv/linux/powerpc/libc-start.c
> > index a9364c7..209a16d 100644
> > --- a/sysdeps/unix/sysv/linux/powerpc/libc-start.c
> > +++ b/sysdeps/unix/sysv/linux/powerpc/libc-start.c
> > @@ -20,6 +20,9 @@
> > #include <ldsodefs.h>
> > #include <sysdep.h>
> >
> > +#ifndef SHARED
> > +#include <hwcapinfo.h>
> > +#endif
> >
> > int __cache_line_size attribute_hidden;
> > /* The main work is done in the generic function. */
> > @@ -68,15 +71,34 @@ __libc_start_main (int argc, char **argv,
> > rtld_fini = NULL;
> > }
> >
> > - /* Initialize the __cache_line_size variable from the aux vector. */
> > + /* 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;
> > +#ifndef SHARED
> > + case AT_HWCAP:
> > + _dl_hwcap = (unsigned long int) av->a_un.a_val;
> > + break;
> > + case AT_HWCAP2:
> > + _dl_hwcap2 = (unsigned long int) av->a_un.a_val;
> > + break;
> > + case AT_PLATFORM:
> > + _dl_platform = (void *) av->a_un.a_val;
> > + break;
> > +#endif
>
> Why do you need this?
>
> In the static case this calls generic_libc_start, which calls
> _dl_aux_init, which does this:
>
> 247 case AT_HWCAP:
> 248 GLRO(dl_hwcap) = (unsigned long int) av->a_un.a_val;
> 249 break;
> 250 case AT_HWCAP2:
> 251 GLRO(dl_hwcap2) = (unsigned long int) av->a_un.a_val;
> 252 break;
>
> ... but doesn't does AT_PLATFORM, which is part of the discussion above.
>
> It seems to me like you're doing this work twice?
>
> You could also follow up with a cleanup patch that moves AT_DCACHEBSIZE
> processing into the power-specific dl-auxv.h/dl-support.c, and remove
> it from libc-start.c, where it's out of place.
>
> > }
> >
> > + /* Initialize hwcap/hwcap2 and platform data so it can be copied to
> > + the TCB later in __libc_setup_tls (). (static case only). */
> > +#ifndef SHARED
> > + __tcb_parse_hwcap_and_convert_at_platform ();
> > +#endif
> > +
>
> OK.
>
> > return generic_start_main (stinfo->main, argc, argv, auxvec,
> > stinfo->init, stinfo->fini, rtld_fini,
> > stack_on_entry);
> > diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/ld.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/ld.abilist
> > index 7d24961..3fd3802 100644
> > --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/ld.abilist
> > +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/ld.abilist
> > @@ -13,6 +13,9 @@ GLIBC_2.1
> > GLIBC_2.22
> > GLIBC_2.22 A
> > __tls_get_addr_opt F
> > +GLIBC_2.23
> > + GLIBC_2.23 A
> > + __parse_hwcap_and_convert_at_platform F
>
> OK.
>
> > GLIBC_2.3
> > GLIBC_2.3 A
> > __tls_get_addr F
> > diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/ld-le.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/ld-le.abilist
> > index 3174e21..3b3717c 100644
> > --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/ld-le.abilist
> > +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/ld-le.abilist
> > @@ -12,3 +12,6 @@ GLIBC_2.17
> > GLIBC_2.22
> > GLIBC_2.22 A
> > __tls_get_addr_opt F
> > +GLIBC_2.23
> > + GLIBC_2.23 A
> > + __parse_hwcap_and_convert_at_platform F
>
> OK.
>
> > diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/ld.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/ld.abilist
> > index d8c4201..2823d13 100644
> > --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/ld.abilist
> > +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/ld.abilist
> > @@ -1,6 +1,9 @@
> > GLIBC_2.22
> > GLIBC_2.22 A
> > __tls_get_addr_opt F
> > +GLIBC_2.23
> > + GLIBC_2.23 A
> > + __parse_hwcap_and_convert_at_platform F
>
> OK.
>
> > GLIBC_2.3
> > GLIBC_2.3 A
> > __libc_memalign F
> > -- 2.3.8 (Apple Git-58)
>
> Cheers,
> Carlos.
>