diff mbox

[v2] powerpc: Add POWER9 architected mode to cputable

Message ID 20170217020135.6361-1-ruscur@russell.cc (mailing list archive)
State Accepted
Headers show

Commit Message

Russell Currey Feb. 17, 2017, 2:01 a.m. UTC 
PVR value of 0x0F000005 means we are arch v3.00 compliant (i.e. POWER9).

Acked-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: Russell Currey <ruscur@russell.cc>
---
 arch/powerpc/kernel/cputable.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Daniel Axtens Feb. 17, 2017, 8:05 a.m. UTC | #1 
Hi Russell,

This seems to go Power8, Power9, Power7 - is that intentional?

Regards,
Daniel

>  		.platform		= "power8",
...
> +		.platform		= "power9",
...
>  	{	/* Power7 */
Michael Ellerman Feb. 17, 2017, 10:11 a.m. UTC | #2 
Daniel Axtens <dja@axtens.net> writes:

> Hi Russell,
>
> This seems to go Power8, Power9, Power7 - is that intentional?
>
> Regards,
> Daniel
>
>>  		.platform		= "power8",
> ...
>> +		.platform		= "power9",
> ...
>>  	{	/* Power7 */

It's because we have the architected PVRs and the raw ones, and the
ordering is a bit odd:

$ grep -n -e '(architected)' -e '(raw)' arch/powerpc/kernel/cputable.c
323:		.cpu_name		= "POWER6 (raw)",
343:		.cpu_name		= "POWER6 (architected)",
356:		.cpu_name		= "POWER7 (architected)",
374:		.cpu_name		= "POWER8 (architected)",
392:		.cpu_name		= "POWER9 (architected)",
411:		.cpu_name		= "POWER7 (raw)",
431:		.cpu_name		= "POWER7+ (raw)",
451:		.cpu_name		= "POWER8E (raw)",
471:		.cpu_name		= "POWER8NVL (raw)",
491:		.cpu_name		= "POWER8 (raw)",
511:		.cpu_name		= "POWER8 (raw)",
531:		.cpu_name		= "POWER9 (raw)",
550:		.cpu_name		= "POWER9 (raw)",

But I don't think it matters in practice. The architected entries use a
pvr_mask of 0xffffffff so it has to be an exact match.

cheers
Michael Ellerman Feb. 17, 2017, 10:26 a.m. UTC | #3 
Russell Currey <ruscur@russell.cc> writes:

> diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
> index 6a82ef039c50..d23a54b09436 100644
> --- a/arch/powerpc/kernel/cputable.c
> +++ b/arch/powerpc/kernel/cputable.c
> @@ -386,6 +386,25 @@ static struct cpu_spec __initdata cpu_specs[] = {
>  		.machine_check_early	= __machine_check_early_realmode_p8,
>  		.platform		= "power8",
>  	},
> +	{	/* 3.00-compliant processor, i.e. Power9 "architected" mode */
> +		.pvr_mask		= 0xffffffff,
> +		.pvr_value		= 0x0f000005,
> +		.cpu_name		= "POWER9 (architected)",
> +		.cpu_features		= CPU_FTRS_POWER9,
> +		.cpu_user_features	= COMMON_USER_POWER9,
> +		.cpu_user_features2	= COMMON_USER2_POWER9,
> +		.mmu_features		= MMU_FTRS_POWER9,
> +		.icache_bsize		= 128,
> +		.dcache_bsize		= 128,
> +		.num_pmcs		= 6,

It's important *not* to set num_pmcs for the architected PVRs.

See the comment in setup_cpu_spec():

	/*
	 * If we are overriding a previous value derived from the real
	 * PVR with a new value obtained using a logical PVR value,
	 * don't modify the performance monitor fields.
	 */
	if (old.num_pmcs && !s->num_pmcs) {
		t->num_pmcs = old.num_pmcs;
		t->pmc_type = old.pmc_type;

I realise that having that requirement in the code is serious foot gun
material on our part, but c'est la vie.

The reason we do that is there's no "compat mode" for the PMU. So if you
boot on a Power9, and then the logical PVR says "actually pretend you're
on a Power8", we flip most of the cpu_spec to have the Power8 values,
but *not* the PMU fields. That way the Power9 PMU code will still detect
that it's on a Power9 and work correctly.

Possibly now that oprofile is more or less dead we can rip all that crap
out, and have perf just look at the PVR directly.

cheers
Russell Currey Feb. 17, 2017, 12:05 p.m. UTC | #4 
On Fri, 2017-02-17 at 21:26 +1100, Michael Ellerman wrote:
> Russell Currey <ruscur@russell.cc> writes:
> 
> > diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
> > index 6a82ef039c50..d23a54b09436 100644
> > --- a/arch/powerpc/kernel/cputable.c
> > +++ b/arch/powerpc/kernel/cputable.c
> > @@ -386,6 +386,25 @@ static struct cpu_spec __initdata cpu_specs[] = {
> >  		.machine_check_early	=
> > __machine_check_early_realmode_p8,
> >  		.platform		= "power8",
> >  	},
> > +	{	/* 3.00-compliant processor, i.e. Power9 "architected"
> > mode */
> > +		.pvr_mask		= 0xffffffff,
> > +		.pvr_value		= 0x0f000005,
> > +		.cpu_name		= "POWER9 (architected)",
> > +		.cpu_features		= CPU_FTRS_POWER9,
> > +		.cpu_user_features	= COMMON_USER_POWER9,
> > +		.cpu_user_features2	= COMMON_USER2_POWER9,
> > +		.mmu_features		= MMU_FTRS_POWER9,
> > +		.icache_bsize		= 128,
> > +		.dcache_bsize		= 128,
> > +		.num_pmcs		= 6,
> 
> It's important *not* to set num_pmcs for the architected PVRs.
> 
> See the comment in setup_cpu_spec():
> 
> 	/*
> 	 * If we are overriding a previous value derived from the real
> 	 * PVR with a new value obtained using a logical PVR value,
> 	 * don't modify the performance monitor fields.
> 	 */
> 	if (old.num_pmcs && !s->num_pmcs) {
> 		t->num_pmcs = old.num_pmcs;
> 		t->pmc_type = old.pmc_type;
> 
> I realise that having that requirement in the code is serious foot gun
> material on our part, but c'est la vie.
> 
> The reason we do that is there's no "compat mode" for the PMU. So if you
> boot on a Power9, and then the logical PVR says "actually pretend you're
> on a Power8", we flip most of the cpu_spec to have the Power8 values,
> but *not* the PMU fields. That way the Power9 PMU code will still detect
> that it's on a Power9 and work correctly.
> 
> Possibly now that oprofile is more or less dead we can rip all that crap
> out, and have perf just look at the PVR directly.

Thanks a lot for explaining, that's interesting.  I thought it might just have
been an accidental omission in the architected entries but I should've dug
deeper.

> 
> cheers
Russell Currey Feb. 17, 2017, 12:06 p.m. UTC | #5 
On Fri, 2017-02-17 at 21:26 +1100, Michael Ellerman wrote:
> Russell Currey <ruscur@russell.cc> writes:
> 
> > diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
> > index 6a82ef039c50..d23a54b09436 100644
> > --- a/arch/powerpc/kernel/cputable.c
> > +++ b/arch/powerpc/kernel/cputable.c
> > @@ -386,6 +386,25 @@ static struct cpu_spec __initdata cpu_specs[] = {
> >  		.machine_check_early	=
> > __machine_check_early_realmode_p8,
> >  		.platform		= "power8",
> >  	},
> > +	{	/* 3.00-compliant processor, i.e. Power9 "architected"
> > mode */
> > +		.pvr_mask		= 0xffffffff,
> > +		.pvr_value		= 0x0f000005,
> > +		.cpu_name		= "POWER9 (architected)",
> > +		.cpu_features		= CPU_FTRS_POWER9,
> > +		.cpu_user_features	= COMMON_USER_POWER9,
> > +		.cpu_user_features2	= COMMON_USER2_POWER9,
> > +		.mmu_features		= MMU_FTRS_POWER9,
> > +		.icache_bsize		= 128,
> > +		.dcache_bsize		= 128,
> > +		.num_pmcs		= 6,
> 
> It's important *not* to set num_pmcs for the architected PVRs.
> 
> See the comment in setup_cpu_spec():
> 
> 	/*
> 	 * If we are overriding a previous value derived from the real
> 	 * PVR with a new value obtained using a logical PVR value,
> 	 * don't modify the performance monitor fields.
> 	 */
> 	if (old.num_pmcs && !s->num_pmcs) {
> 		t->num_pmcs = old.num_pmcs;
> 		t->pmc_type = old.pmc_type;
> 
> I realise that having that requirement in the code is serious foot gun
> material on our part, but c'est la vie.
> 
> The reason we do that is there's no "compat mode" for the PMU. So if you
> boot on a Power9, and then the logical PVR says "actually pretend you're
> on a Power8", we flip most of the cpu_spec to have the Power8 values,
> but *not* the PMU fields. That way the Power9 PMU code will still detect
> that it's on a Power9 and work correctly.
> 
> Possibly now that oprofile is more or less dead we can rip all that crap
> out, and have perf just look at the PVR directly.

Oh and also, do you want me to respin or are you happy to drop it on your end?

- Russell

> 
> cheers
Michael Ellerman Feb. 19, 2017, 11:33 a.m. UTC | #6 
On Fri, 2017-02-17 at 02:01:35 UTC, Russell Currey wrote:
> PVR value of 0x0F000005 means we are arch v3.00 compliant (i.e. POWER9).
> 
> Acked-by: Michael Neuling <mikey@neuling.org>
> Signed-off-by: Russell Currey <ruscur@russell.cc>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/6ae3f8ad2017079292cb49c8959b52

cheers
diff mbox

Patch

diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 6a82ef039c50..d23a54b09436 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -386,6 +386,25 @@  static struct cpu_spec __initdata cpu_specs[] = {
 		.machine_check_early	= __machine_check_early_realmode_p8,
 		.platform		= "power8",
 	},
+	{	/* 3.00-compliant processor, i.e. Power9 "architected" mode */
+		.pvr_mask		= 0xffffffff,
+		.pvr_value		= 0x0f000005,
+		.cpu_name		= "POWER9 (architected)",
+		.cpu_features		= CPU_FTRS_POWER9,
+		.cpu_user_features	= COMMON_USER_POWER9,
+		.cpu_user_features2	= COMMON_USER2_POWER9,
+		.mmu_features		= MMU_FTRS_POWER9,
+		.icache_bsize		= 128,
+		.dcache_bsize		= 128,
+		.num_pmcs		= 6,
+		.pmc_type		= PPC_PMC_IBM,
+		.oprofile_cpu_type	= "ppc64/ibm-compat-v1",
+		.oprofile_type		= PPC_OPROFILE_INVALID,
+		.cpu_setup		= __setup_cpu_power9,
+		.cpu_restore		= __restore_cpu_power9,
+		.flush_tlb		= __flush_tlb_power9,
+		.platform		= "power9",
+	},
 	{	/* Power7 */
 		.pvr_mask		= 0xffff0000,
 		.pvr_value		= 0x003f0000,