[PATCH] RFC: PowerPC: Add Program Priority Register support

Ryan S. Arnold ryan.arnold@gmail.com
Fri Mar 16 18:57:00 GMT 2012


On Mon, Feb 27, 2012 at 6:13 PM, Adhemerval Zanella
<azanella@linux.vnet.ibm.com> wrote:
> This patch adds save/restore for the Priority Program Register (PPR) for ISA 2.03+
> for POWER machines. The PPR register is a special register that controls the program
> priority. It is documented on latest ISA 2.06 (Book II Chapter 3.1) and it can be
> used to improve lock performance by setting an higher priority on critical sections
> and an lower priority on lock holding.
>
> Although a program can easily set its priority using the 'or %1,%1,%1' the kernel
> does not save neither restore its value on syscalls. This patch addresses this
> limitation.

The kernel will eventually save/restore the PPR on context switch.  I
presume, that at that time you'll have an assume kernel check which
will nop the save/restore PPR macros that you've defined here?

> --
>
> 2012-02-27  Adhemerval Zanella  <azanella@linux.vnet.ibm.com>
>
>        * elf/stackguard-macros.h: Updated stack_guard offset value.
>        * nptl/sysdeps/powerpc/tls.h: Added PPR value to TCB.
>        * sysdeps/unix/sysv/linux/powerpc/Makefile: Added sys/platform.h install.
>        * nptl/sysdeps/powerpc/tcb-offsets.sym: Calculating PPR offset on TCB.
>        * sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h: Added PPR support
>        for ISA 2.03+.
>        * sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h: Likewise.
>        * sysdeps/unix/sysv/linux/powerpc/syscall.S: Likewise.
>        * sysdeps/unix/sysv/linux/powerpc/sys/platform.h: New file: platform
>        specific macros for PPR set.
>        * sysdeps/unix/sysv/linux/powerpc/Makefile: Added tst-saveppr test.
>        * sysdeps/unix/sysv/linux/powerpc/tst-saveppr.c: New file: test if PPR is
>        set/retored in syscalls.
>
>
> diff --git a/elf/stackguard-macros.h b/elf/stackguard-macros.h
> index a9889cf..5763870 100644
> --- a/elf/stackguard-macros.h
> +++ b/elf/stackguard-macros.h
> @@ -11,7 +11,7 @@
>   ({ uintptr_t x; asm ("ld %0,-28688(13)" : "=r" (x)); x; })
>  #elif defined __powerpc__
>  # define STACK_CHK_GUARD \
> -  ({ uintptr_t x; asm ("lwz %0,-28680(2)" : "=r" (x)); x; })
> +  ({ uintptr_t x; asm ("lwz %0,-28684(2)" : "=r" (x)); x; })
>  #elif defined __sparc__ && defined __arch64__
>  # define STACK_CHK_GUARD \
>   ({ uintptr_t x; asm ("ldx [%%g7+0x28], %0" : "=r" (x)); x; })
> diff --git a/nptl/sysdeps/powerpc/tcb-offsets.sym b/nptl/sysdeps/powerpc/tcb-offsets.sym
> index 8ac133d..3f65249 100644
> --- a/nptl/sysdeps/powerpc/tcb-offsets.sym
> +++ b/nptl/sysdeps/powerpc/tcb-offsets.sym
> @@ -14,7 +14,9 @@ MULTIPLE_THREADS_OFFSET               thread_offsetof (header.multiple_threads)
>  #endif
>  PID                            thread_offsetof (pid)
>  TID                            thread_offsetof (tid)
> +PPR_OFFSET                     (offsetof (tcbhead_t, ppr) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
>  POINTER_GUARD                  (offsetof (tcbhead_t, pointer_guard) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
>  #ifndef __ASSUME_PRIVATE_FUTEX
>  PRIVATE_FUTEX_OFFSET           thread_offsetof (header.private_futex)
>  #endif
> +
> diff --git a/nptl/sysdeps/powerpc/tls.h b/nptl/sysdeps/powerpc/tls.h
> index 4c09eec..6ed6e65 100644
> --- a/nptl/sysdeps/powerpc/tls.h
> +++ b/nptl/sysdeps/powerpc/tls.h
> @@ -1,5 +1,5 @@
>  /* Definition for thread-local data handling.  NPTL/PowerPC version.
> -   Copyright (C) 2003, 2005, 2006, 2007, 2011 Free Software Foundation, Inc.
> +   Copyright (C) 2003-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
> @@ -61,6 +61,8 @@ typedef union dtv
>    are private.  */
>  typedef struct
>  {
> +  /* Program Priority Register saved value.  */
> +  uint64_t  ppr;
>   uintptr_t pointer_guard;
>   uintptr_t stack_guard;
>   dtv_t *dtv;
> diff --git a/sysdeps/unix/sysv/linux/powerpc/Makefile b/sysdeps/unix/sysv/linux/powerpc/Makefile
> index 55311a4..101eaba 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/Makefile
> +++ b/sysdeps/unix/sysv/linux/powerpc/Makefile
> @@ -4,6 +4,8 @@ syscall-list-32bit-condition := __WORDSIZE == 32
>  syscall-list-64bit-options := -D__powerpc64__
>  syscall-list-64bit-condition := __WORDSIZE == 64
>
> +sysdep_headers += sys/platform.h
> +
>  ifeq ($(subdir),rt)
>  librt-routines += rt-sysdep
>  endif
> @@ -15,3 +17,7 @@ endif
>  ifeq ($(subdir),elf)
>  sysdep_routines += dl-vdso
>  endif
> +
> +ifeq ($(subdir),nptl)
> +tests += tst-saveppr
> +endif
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h b/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
> index b0fa372..b10df5d 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/sysdep.h
> @@ -1,4 +1,4 @@
> -/* Copyright (C) 1992,1997-2006,2012 Free Software Foundation, Inc.
> +/* Copyright (C) 1992-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
> @@ -20,6 +20,7 @@
>
>  #include <sysdeps/unix/powerpc/sysdep.h>
>  #include <tls.h>
> +#include <tcb-offsets.h>
>
>  /* Some systen calls got renamed over time, but retained the same semantics.
>    Handle them here so they can be catched by both C and assembler stubs in
> @@ -177,6 +178,30 @@
>  # undef INTERNAL_SYSCALL_DECL
>  # define INTERNAL_SYSCALL_DECL(err) long int err
>
> +/* The Program Priority Register was added in ISA 2.03 */
> +#if defined _ARCH_PWR5X
> +# define SETPPR_BEFORE                                                 \
> +  __asm__ __volatile__(                                                \
> +    "cmpdi cr5,2,0\n\t"                                                        \
> +    "beq- cr5,12\n\t"                                                  \
> +    "mfspr 0,896\n\t"                                                  \
> +    "std 0,%[ppr](2)\n\t" : : [ppr]"i"(PPR_OFFSET) : "r0", "cr5")
> +# define SETPPR_AFTER                                                  \
> +    "cmpdi cr5,2,0\n\t"                                                        \
> +    "beq- cr5,12\n\t"                                                  \
> +    "ld 0,%[ppr](2)\n\t"                                               \
> +    "mtspr 896,0\n\t"
> +# define SETPPR_INPUT                                                  \
> +    , [ppr]"i"(PPR_OFFSET)
> +# define SETPPR_CLOBREG                                                        \
> +    , "cr5"
> +#else
> +# define SETPPR_BEFORE
> +# define SETPPR_AFTER
> +# define SETPPR_INPUT
> +# define SETPPR_CLOBREG
> +#endif
> +
>  # undef INTERNAL_SYSCALL
>  # define INTERNAL_SYSCALL_NCS(name, err, nr, args...)                  \
>   ({                                                                   \
> @@ -191,15 +216,17 @@
>     register long int r10 __asm__ ("r10");                             \
>     register long int r11 __asm__ ("r11");                             \
>     register long int r12 __asm__ ("r12");                             \
> +    SETPPR_BEFORE;                                                     \
>     LOADARGS_##nr(name, args);                                 \
>     __asm__ __volatile__                                               \
>       ("sc   \n\t"                                                     \
> +       SETPPR_AFTER                                                    \
>        "mfcr %0"                                                       \
>        : "=&r" (r0),                                                   \
>         "=&r" (r3), "=&r" (r4), "=&r" (r5),  "=&r" (r6),  "=&r" (r7),  \
>         "=&r" (r8), "=&r" (r9), "=&r" (r10), "=&r" (r11), "=&r" (r12)  \
> -       : ASM_INPUT_##nr                                                        \
> -       : "cr0", "ctr", "memory");                                      \
> +       : ASM_INPUT_##nr SETPPR_INPUT                                   \
> +       : "cr0", "ctr", "memory" SETPPR_CLOBREG);                       \
>     err = r0;                                                          \
>     (int) r3;                                                          \
>   })
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h b/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
> index 0f4fe53..73cb00a 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h
> @@ -1,5 +1,4 @@
> -/* Copyright (C) 1992,1997,1998,1999,2000,2001,2002,2003,2004,2005,2006,2012
> -       Free Software Foundation, Inc.
> +/* Copyright (C) 1992-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
> @@ -23,6 +22,7 @@
>
>  #include <sysdeps/unix/powerpc/sysdep.h>
>  #include <tls.h>
> +#include <tcb-offsets.h>
>
>  /* Define __set_errno() for INLINE_SYSCALL macro below.  */
>  #ifndef __ASSEMBLER__
> @@ -192,6 +192,29 @@
>    gave back in the non-error (CR0.SO cleared) case, otherwise (CR0.SO set)
>    the negation of the return value in the kernel gets reverted.  */
>
> +#if defined _ARCH_PWR5X
> +# define SETPPR_BEFORE                                                 \
> +  __asm__ __volatile__(                                                        \
> +    "cmpdi cr5,13,0\n\t"                                               \
> +    "beq- cr5,12\n\t"                                                  \
> +    "mfspr 0,896\n\t"                                                  \
> +    "std 0,%[ppr](13)\n\t" : : [ppr]"i"(PPR_OFFSET) : "r0", "cr5")
> +# define SETPPR_AFTER                                                  \
> +    "cmpdi cr5,13,0\n\t"                                               \
> +    "beq- cr5,12\n\t"                                                  \
> +    "ld 0,%[ppr](13)\n\t"                                              \
> +    "mtspr 896,0\n\t"
> +# define SETPPR_INPUT                                                  \
> +    ,[ppr]"i"(PPR_OFFSET)
> +# define SETPPR_CLOBREG                                                        \
> +    ,"cr5"
> +#else
> +# define SETPPR_BEFORE
> +# define SETPPR_AFTER
> +# define SETPPR_INPUT
> +# define SETPPR_CLOBREG
> +#endif
> +
>  #undef INTERNAL_SYSCALL
>  #define INTERNAL_SYSCALL_NCS(name, err, nr, args...) \
>   ({                                                                   \
> @@ -202,17 +225,19 @@
>     register long int r6  __asm__ ("r6");                              \
>     register long int r7  __asm__ ("r7");                              \
>     register long int r8  __asm__ ("r8");                              \
> +    SETPPR_BEFORE;                                                     \
>     LOADARGS_##nr (name, ##args);                                      \
>     __asm__ __volatile__                                               \
>       ("sc\n\t"                                                                \
> +       SETPPR_AFTER                                                    \
>        "mfcr  %0\n\t"                                                  \
>        "0:"                                                            \
>        : "=&r" (r0),                                                   \
>          "=&r" (r3), "=&r" (r4), "=&r" (r5),                           \
>          "=&r" (r6), "=&r" (r7), "=&r" (r8)                            \
> -       : ASM_INPUT_##nr                                                        \
> +       : ASM_INPUT_##nr        SETPPR_INPUT                                    \
>        : "r9", "r10", "r11", "r12",                                    \
> -         "cr0", "ctr", "memory");                                      \
> +         "cr0", "ctr", "memory" SETPPR_CLOBREG);                       \
>          err = r0;  \
>     r3;  \
>   })
> diff --git a/sysdeps/unix/sysv/linux/powerpc/sys/platform.h b/sysdeps/unix/sysv/linux/powerpc/sys/platform.h
> new file mode 100644
> index 0000000..52e6bf3
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/powerpc/sys/platform.h
> @@ -0,0 +1,47 @@
> +/* Copyright (C) 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
> +   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/>.  */
> +
> +/* This file contains platform specific macros and function definitions */
> +
> +#ifndef _PLATFORM_H
> +#define _PLATFORM_H    1
> +
> +/*
> + * ISA 2.05 and beyond support the Program Priority Register (PPR) to adjust
> + * the thread priorities based on lock acquisition, wait and release. The ISA
> + * defines the use of form 'or Rx,Rx,Rx' as the way to modify the PRI field.
> + * The priorities are defined as:
> + * Rx = 2 (medium), Rx = 6 (medium-low/normal), Rx = 1 (low)
> + * This 'or' instruction form is a nop in previous hardware, so it's safe to
> + * use unguarded.
> + */
> +#define PPR_PRIO_MED() \
> +   ({ \
> +      __asm __volatile("or 2,2,2\n"); \
> +   })
> +#define PPR_PRIO_MED_LOW() \
> +   ({ \
> +      __asm __volatile("or 6,6,6\n"); \
> +   })
> +#define PPR_PRIO_LOW() \
> +   ({ \
> +      __asm __volatile("or 1,1,1\n"); \
> +   })
> +#define PPR_PRIO_NORMAL() \
> +   PPR_PRIO_MED_LOW()
> +
> +#endif

Considering the recent discussion about timebase, these macros
probably belong as GCC built-in functions.  In lieu of their
availability as GCC built-ins, I believe the current consensus is that
these should indeed go in into sysdeps/powerpc/sys/platform/ppc.h.

When GCC support is available you can add a conditional guard based on
built-in support and have the implementation use the built-in instead
of your version.

These should probably be static inlines for the same reason as
described in the timebase discussion.

> diff --git a/sysdeps/unix/sysv/linux/powerpc/syscall.S b/sysdeps/unix/sysv/linux/powerpc/syscall.S
> index 880c05b..1563dd6 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/syscall.S
> +++ b/sysdeps/unix/sysv/linux/powerpc/syscall.S
> @@ -1,4 +1,4 @@
> -/* Copyright (C) 1991, 1992, 1997, 1999 Free Software Foundation, Inc.
> +/* Copyright (C) 1991-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
> @@ -18,6 +18,10 @@
>  #include <sysdep.h>
>
>  ENTRY (syscall)
> +#if defined _ARCH_PWR5X
> +       mfspr r0,896
> +       std  r0,PPR_OFFSET(r13)
> +#endif
>        mr   r0,r3
>        mr   r3,r4
>        mr   r4,r5
> @@ -26,5 +30,9 @@ ENTRY (syscall)
>        mr   r7,r8
>        mr   r8,r9
>        sc
> +#if defined _ARCH_PWR5X
> +       ld   r0,PPR_OFFSET(r13)
> +       mtspr 896,r0

Is there a reason to use mtspr instead of mtppr (here and in other places)?

> +#endif
>        PSEUDO_RET
>  PSEUDO_END (syscall)
> diff --git a/sysdeps/unix/sysv/linux/powerpc/tst-saveppr.c b/sysdeps/unix/sysv/linux/powerpc/tst-saveppr.c
> new file mode 100644
> index 0000000..d5e59f1
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/powerpc/tst-saveppr.c
> @@ -0,0 +1,63 @@
> +/* Test for saving and restoring the PPR register on syscalls.
> +   Copyright (C) 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
> +   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.  */
> +
> +#include <unistd.h>
> +#include <sys/platform.h>
> +
> +/* This program works both on 32 bit and 64 bit environments,
> +   so we read the 64 bit PPR value word by word.  */
> +
> +void
> +get_ppr (unsigned int *low_word, unsigned int *high_word)
> +{
> +  unsigned int ppr, word1, word2;
> +
> +  asm ("mfspr  %0,896\n"
> +       "andi.  %2,%0,0xffff\n"
> +       "andis. %1,%0,0xffff\n"
> +       "or     %1,%1,%2\n"
> +       "sradi  %0,%0,32" :
> +       "=r" (word1), "=r" (word2), "=r" (ppr) : : "cr0");
> +
> +  *low_word = word1;
> +  *high_word = word2;
> +}
> +
> +static int
> +do_test (void)
> +{
> +  unsigned int low_word_before, high_word_before;
> +  unsigned int low_word_after, high_word_after;
> +
> +  PPR_PRIO_MED_LOW();
> +
> +  get_ppr (&low_word_before, &high_word_before);
> +
> +  // force a syscall
> +  getpid();
> +

Add a comment about why you added the syscall, i.e. you want to
demonstrate that the PPR save/restore is working properly after a
context switch.

> +  get_ppr (&low_word_after, &high_word_after);
> +
> +  return !(low_word_before == low_word_after &&
> +          high_word_before == high_word_after);

Looking at the ISA, only the PRI field of the PPR (PPR[11:13]) is
modifiable via the "or" instruction shortcuts used to set the program
priority.  The PPR[26:31] and PPR[44:63] fields are marekd
"implementation-specific".

Does the "mtspr 896, Rx" instruction only restore the bits in the PRI
field of the PPR?

I can see how the following check might fail if the non-PRI fields
have changed across the context switch.  Do we know anything about how
these other fields might be used, if at all?

Ryan


More information about the Libc-alpha mailing list