Re: [PATCH] PowerPC - Add a faster way to read the Time Base register

["Ryan S. Arnold" <ryan dot arnold at gmail dot com>]

On Tue, Feb 14, 2012 at 9:30 AM, Tulio Magno Quites Machado Filho
<tuliom@linux.vnet.ibm.com> wrote:
> Add macro __PPC_GETTIMEBASE() to directly read the Time Base register.
> This is required for applications that measure time at high frequencies
> with high precision that can't afford a syscall.
>
> My FSF copyright assignment is on file.
>
> 2012-02-06 ÂTulio Magno Quites Machado Filho Â<tuliom@linux.vnet.ibm.com>
>
> Â Â Â Â* sysdeps/unix/sysv/linux/powerpc/sys/user.h: New macro definition.

This should be:
        * sysdeps/unix/sysv/linux/powerpc/sys/user.h (__PPC_GETTIMEBASE): New
        macro definition.

> Â Â Â Â* sysdeps/unix/sysv/linux/powerpc/Makefile (tests): Add
> Â Â Â Âtest-gettimebase.
> Â Â Â Â* sysdeps/unix/sysv/linux/powerpc/test-gettimebase.c: Test for
> Â Â Â Â__PPC_GETTIMEBASE() to catch future ISA opcode/insn changes.
> ---
> Âsysdeps/unix/sysv/linux/powerpc/Makefile      |  Â2 +
> Âsysdeps/unix/sysv/linux/powerpc/sys/user.h     |  40 +++++++++++++++++++-
> Âsysdeps/unix/sysv/linux/powerpc/test-gettimebase.c | Â 35 +++++++++++++++++
> Â3 files changed, 76 insertions(+), 1 deletions(-)
> Âcreate mode 100644 sysdeps/unix/sysv/linux/powerpc/test-gettimebase.c
>
> diff --git a/sysdeps/unix/sysv/linux/powerpc/Makefile b/sysdeps/unix/sysv/linux/powerpc/Makefile
> index 55311a4..a2ab09a 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/Makefile
> +++ b/sysdeps/unix/sysv/linux/powerpc/Makefile
> @@ -15,3 +15,5 @@ endif
> Âifeq ($(subdir),elf)
> Âsysdep_routines += dl-vdso
> Âendif
> +
> +tests += test-gettimebase
> diff --git a/sysdeps/unix/sysv/linux/powerpc/sys/user.h b/sysdeps/unix/sysv/linux/powerpc/sys/user.h
> index 5fa3745..301e64a 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/sys/user.h
> +++ b/sysdeps/unix/sysv/linux/powerpc/sys/user.h
> @@ -1,4 +1,4 @@
> -/* Copyright (C) 1998 Free Software Foundation, Inc.
> +/* Copyright (C) 1998, 2012 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
> @@ -36,4 +36,42 @@ struct user {
>    Âchar      Âu_comm[32];       /* user command name */
> Â};
>
> +typedef unsigned long long int __ppc_timebase;
> +

Since the timebase is only held in bits 0:59 you need to add a #define
for __PPC_TIMEBASE_MAX__ that is equivalent to when the timebase rolls
over to zero, i.e., 2^59 - 1 (per the Power ISA 2.06), since
__LONG_LONG_MAX__ is too large.  Don't forget the LL suffix on the
value.

> +#define __STRINGIFY(...) #__VA_ARGS__
> +#define STRINGIFY(...) __STRINGIFY(__VA_ARGS__)
> +
> +#define SPRN_TBRL 268 /* Time Base Read Lower Register */

Please indicate with a comment, where in the Power ISA the timebase
register is mentioned (ISA version, section, etc.) and how this is to
be used.

> +#ifdef __powerpc64__
> +# define __PPC_GETTIMEBASE() Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> + Â({ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> + Â Â__ppc_timebase __tb; Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> + Â Â__asm__ volatile ( Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> + Â Â Â Â Â Â Â Â Â Â "mfspr %[tb], " STRINGIFY(SPRN_TBRL) "\n" \
> + Â Â Â Â Â Â Â Â Â Â : [tb]"=r" (__tb) Â Â Â Â Â Â Â Â Â Â Â Â \
> + Â Â Â Â Â Â Â Â Â Â : ); Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> + Â Â__tb; Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> + Â})
> +#else Â/* __powerpc64__ */
> +# define __PPC_GETTIMEBASE() Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> + Â({ Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> + Â Â__ppc_timebase __tb; Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> + Â Âregister unsigned long __tbu, __tbl, __tmp; Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> + Â Â__asm__ volatile ( Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> + Â Â Â Â Â Â Â Â Â Â "0:\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> + Â Â Â Â Â Â Â Â Â Â "mftbu %[tbu]\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> + Â Â Â Â Â Â Â Â Â Â "mftbl %[tbl]\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> + Â Â Â Â Â Â Â Â Â Â "mftbu %[tmp]\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> + Â Â Â Â Â Â Â Â Â Â "cmpw %[tbu], %[tmp]\n" Â Â Â Â Â Â Â Â Â Â Â Â Â \
> + Â Â Â Â Â Â Â Â Â Â "bne- 0b\n" Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> + Â Â Â Â Â Â Â Â Â Â : [tbu]"=r" (__tbu), [tbl]"=r" (__tbl), Â Â Â Â Â \
> + Â Â Â Â Â Â Â Â Â Â Â [tmp]"=r" (__tmp) Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> + Â Â Â Â Â Â Â Â Â Â : ); Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> + Â Â__tb = ( (__ppc_timebase) __tbu << 32) | __tbl; Â Â Â Â Â Â Â Â Â Â\
> + Â Â__tb; Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â\
> + Â})

I don't think the last __tb; is necessary.  Test it without. This is a
GCC 'statement expression'.  I believe that it returns the value of
the last expression in a statement block.

> +#endif Â/* __powerpc64__ */
> +
> Â#endif Â/* sys/user.h */
> diff --git a/sysdeps/unix/sysv/linux/powerpc/test-gettimebase.c b/sysdeps/unix/sysv/linux/powerpc/test-gettimebase.c
> new file mode 100644
> index 0000000..cb18aab
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/powerpc/test-gettimebase.c
> @@ -0,0 +1,35 @@
> +/* Copyright (C) 2012 Free Software Foundation, Inc.
> + Â This file is part of the GNU C Library.
> + Â Contributed by Tulio Magno Quites Machado <tuliom@linux.vnet.ibm.com>, 2012.
> +
> + Â 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, write to the Free
> + Â Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
> + Â 02111-1307 USA. Â*/
> +
> +/* Test if __PPC_GETTIMEBASE() is compatible with the current processor */

Perhaps add that this is an early notification of a future Power ISA
and binutils change.

> +#include <stdio.h>
> +
> +#include <sys/user.h>
> +
> +static int
> +do_test(void)
> +{
> + Â__ppc_timebase tb = __PPC_GETTIMEBASE();
> + Âprintf("Time Base = %llx\n", tb);
> + Âreturn 0;
> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"

The testcase looks fine.

Something to think about for a future addition to user.h and the
testcase would be to generate valid timestamps from the queried
timebase values. I believe Power ISA Book II, Chapter 5. Timebase
(page 706) demonstrates how to do this if we know the update
frequency.

Do we know on which Power Architecture platforms the update frequency
of the timebase is constant?  How is the timebase update frequency
queried (the ISA says this is possible).

How often/quickly does the timebase roll-over on POWER systems?  As an
example the ISA shows that, at 1Ghz CPU frequency, with a timebase
update frequency of clock-frequency/32 the period of the timebase is
about 18,700 years.  This would presume that the timebase is
initialized to zero on system start.

If setting the timebase register isn't privileged perhaps you can
fabricate a test that sets it near the max and generates valid
timestamps across the rollover.

Ryan