Benjamin Herrenschmidt <b...@kernel.crashing.org> wrote:
> On Sun, 2012-09-09 at 04:37 -0700, Haren Myneni wrote:
> > enable_ppr kernel parameter is used to enable PPR save and restore.
> > Supported on Power7 and later processors.
> >
> > By default, CPU_FTR_HAS_PPR is set for POWER7. If this parameter is not
> > passed, disable CPU_FTR_HAS_PPR.
>
> What is the point ? Obscure / magic kernel command line options to turn
> on a feature are pointless. Nobody knows about them, nobody enables
> them.
>
> What you are doing is guarantee that nobody's ever going to enable your
> code, so your whole patch series is thus irrelevant :-)
>
> If there's a good reason to *avoid* your option, then maybe consider
> adding an option to *disable* the PPR save/restore, though of course
> that would bring the argument that if it needs to be disabled maybe we
> shouldn't do it in the first place, or you might want to think of a
> reasonable way to intuit what the option should be.
IIRC, Haren was saying there's a 6% hit on null syscall for this. So we
suggested having a cmdline option to disable it for distros.
Haren, is my recollection correct? If so, can you add this info the
change log and change the sex of the option.
Mikey
>
> Ben.
>
> > Signed-off-by: Haren Myneni <ha...@us.ibm.com>
> >
> > ---
> > Documentation/kernel-parameters.txt | 4 ++++
> > arch/powerpc/include/asm/cputable.h | 6 ++++--
> > arch/powerpc/kernel/setup_64.c | 14 ++++++++++++++
> > 3 files changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/kernel-parameters.txt
> > b/Documentation/kernel-parameters.txt
> > index ad7e2e5..2881e5f 100644
> > --- a/Documentation/kernel-parameters.txt
> > +++ b/Documentation/kernel-parameters.txt
> > @@ -809,6 +809,10 @@ bytes respectively. Such letter suffixes can also be
> > entirely omitted.
> > to discrete, to make X server driver able to add WB
> > entry later. This parameter enables that.
> >
> > + enable_ppr [PPC/PSERIES]
> > + Saves user defined PPR when process enters to kernel
> > + and restores PPR at exit. But it impacts performance.
> > +
> > enable_timer_pin_1 [X86]
> > Enable PIN 1 of APIC timer
> > Can be useful to work around chipset bugs
> > diff --git a/arch/powerpc/include/asm/cputable.h
> > b/arch/powerpc/include/asm/cputable.h
> > index b3c083d..880c469 100644
> > --- a/arch/powerpc/include/asm/cputable.h
> > +++ b/arch/powerpc/include/asm/cputable.h
> > @@ -203,6 +203,7 @@ extern const char *powerpc_base_platform;
> > #define CPU_FTR_POPCNTD
> > LONG_ASM_CONST(0x0800000000000000)
> > #define CPU_FTR_ICSWX
> > LONG_ASM_CONST(0x1000000000000000)
> > #define CPU_FTR_VMX_COPY LONG_ASM_CONST(0x2000000000000000)
> > +#define CPU_FTR_HAS_PPR
> > LONG_ASM_CONST(0x4000000000000000)
> >
> > #ifndef __ASSEMBLY__
> >
> > @@ -432,7 +433,8 @@ extern const char *powerpc_base_platform;
> > CPU_FTR_PURR | CPU_FTR_SPURR | CPU_FTR_REAL_LE | \
> > CPU_FTR_DSCR | CPU_FTR_SAO | CPU_FTR_ASYM_SMT | \
> > CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
> > - CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | CPU_FTR_VMX_COPY)
> > + CPU_FTR_ICSWX | CPU_FTR_CFAR | CPU_FTR_HVMODE | \
> > + CPU_FTR_VMX_COPY | CPU_FTR_HAS_PPR)
> > #define CPU_FTRS_CELL (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> > CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
> > CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
> > @@ -454,7 +456,7 @@ extern const char *powerpc_base_platform;
> > (CPU_FTRS_POWER3 | CPU_FTRS_RS64 | CPU_FTRS_POWER4 | \
> > CPU_FTRS_PPC970 | CPU_FTRS_POWER5 | CPU_FTRS_POWER6 | \
> > CPU_FTRS_POWER7 | CPU_FTRS_CELL | CPU_FTRS_PA6T | \
> > - CPU_FTR_VSX)
> > + CPU_FTR_VSX | CPU_FTR_HAS_PPR)
> > #endif
> > #else
> > enum {
> > diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> > index 389bd4f..e4c1945 100644
> > --- a/arch/powerpc/kernel/setup_64.c
> > +++ b/arch/powerpc/kernel/setup_64.c
> > @@ -98,6 +98,8 @@ int dcache_bsize;
> > int icache_bsize;
> > int ucache_bsize;
> >
> > +static u32 enable_ppr = 0;
> > +
> > #ifdef CONFIG_SMP
> >
> > static char *smt_enabled_cmdline;
> > @@ -357,6 +359,9 @@ void __init setup_system(void)
> > {
> > DBG(" -> setup_system()\n");
> >
> > + if (cpu_has_feature(CPU_FTR_HAS_PPR) && !enable_ppr)
> > + cur_cpu_spec->cpu_features &= ~CPU_FTR_HAS_PPR;
> > +
> > /* Apply the CPUs-specific and firmware specific fixups to kernel
> > * text (nop out sections not relevant to this CPU or this firmware)
> > */
> > @@ -683,6 +688,15 @@ void __init setup_per_cpu_areas(void)
> > }
> > #endif
> >
> > +/* early_ppr kernel parameter to save/restore PPR register */
> > +static int __init early_ppr_enabled(char *str)
> > +{
> > + enable_ppr = 1;
> > +
> > + return 0;
> > +}
> > +
> > +early_param("enable_ppr", early_ppr_enabled);
> >
> > #ifdef CONFIG_PPC_INDIRECT_IO
> > struct ppc_pci_io ppc_pci_io;
>
>
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
