[PATCH v2] powerpc: Add POWER9 architected mode to cputable

Subsystems: linux for powerpc (32-bit and 64-bit), the rest

STALE3453d REVIEWED: 1 (0M)

1 review trailer.

8 messages, 4 authors, 2017-02-19 · open the first message on its own page

[PATCH v2] powerpc: Add POWER9 architected mode to cputable

From: Russell Currey <hidden>
Date: 2017-02-17 02:01:47

PVR value of 0x0F000005 means we are arch v3.00 compliant (i.e. POWER9).

Acked-by: Michael Neuling <redacted>
Signed-off-by: Russell Currey <redacted>
---
 arch/powerpc/kernel/cputable.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
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,
-- 
2.11.1

Re: [PATCH v2] powerpc: Add POWER9 architected mode to cputable

From: Daniel Axtens <hidden>
Date: 2017-02-17 08:05:55

Hi Russell,

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

Regards,
Daniel
 		.platform		= "power8",
...
+		.platform		= "power9",
...
 	{	/* Power7 */

Re: [PATCH v2] powerpc: Add POWER9 architected mode to cputable

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2017-02-17 10:11:17

Daniel Axtens [off-list ref] writes:
Hi Russell,

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

Regards,
Daniel
quoted
 		.platform		= "power8",
...
quoted
+		.platform		= "power9",
...
quoted
 	{	/* 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

Re: [PATCH v2] powerpc: Add POWER9 architected mode to cputable

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2017-02-17 10:26:36

Russell Currey [off-list ref] writes:
quoted hunk
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

Re: [PATCH v2] powerpc: Add POWER9 architected mode to cputable

From: Russell Currey <hidden>
Date: 2017-02-17 12:06:10

On Fri, 2017-02-17 at 21:26 +1100, Michael Ellerman wrote:
Russell Currey [off-list ref] writes:
quoted
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

Re: [PATCH v2] powerpc: Add POWER9 architected mode to cputable

From: Russell Currey <hidden>
Date: 2017-02-17 12:07:09

On Fri, 2017-02-17 at 21:26 +1100, Michael Ellerman wrote:
Russell Currey [off-list ref] writes:
quoted
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

Re: [PATCH v2] powerpc: Add POWER9 architected mode to cputable

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2017-02-18 07:54:52

Russell Currey [off-list ref] writes:
On Fri, 2017-02-17 at 21:26 +1100, Michael Ellerman wrote:
quoted
Russell Currey [off-list ref] writes:
quoted
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cput=
able.c
quoted
quoted
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[] =3D=
 {
quoted
quoted
=C2=A0		.machine_check_early	=3D
__machine_check_early_realmode_p8,
=C2=A0		.platform		=3D "power8",
=C2=A0	},
+	{	/* 3.00-compliant processor, i.e. Power9 "architected"
mode */
+		.pvr_mask		=3D 0xffffffff,
+		.pvr_value		=3D 0x0f000005,
+		.cpu_name		=3D "POWER9 (architected)",
+		.cpu_features		=3D CPU_FTRS_POWER9,
+		.cpu_user_features	=3D COMMON_USER_POWER9,
+		.cpu_user_features2	=3D COMMON_USER2_POWER9,
+		.mmu_features		=3D MMU_FTRS_POWER9,
+		.icache_bsize		=3D 128,
+		.dcache_bsize		=3D 128,
+		.num_pmcs		=3D 6,
=20
It's important *not* to set num_pmcs for the architected PVRs.
...
Oh and also, do you want me to respin or are you happy to drop it on your=
 end?

I fixed it up:

commit 6ae3f8ad2017079292cb49c8959b527bcbcbefed
Author:     Russell Currey [off-list ref]
AuthorDate: Fri Feb 17 13:01:35 2017 +1100
Commit:     Michael Ellerman [off-list ref]
CommitDate: Fri Feb 17 21:48:56 2017 +1100

    powerpc: Add POWER9 architected mode to cputable
=20=20=20=20
    PVR value of 0x0F000005 means we are arch v3.00 compliant (i.e. POWER9).
=20=20=20=20
    Acked-by: Michael Neuling [off-list ref]
    Signed-off-by: Russell Currey [off-list ref]
    [mpe: Don't set num_pmcs, so we keep the PMU fields from the raw entry]
    Signed-off-by: Michael Ellerman [off-list ref]
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 6a82ef039c50..bb7a1890aeb7 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -386,6 +386,23 @@ static struct cpu_spec __initdata cpu_specs[] =3D {
 		.machine_check_early	=3D __machine_check_early_realmode_p8,
 		.platform		=3D "power8",
 	},
+	{	/* 3.00-compliant processor, i.e. Power9 "architected" mode */
+		.pvr_mask		=3D 0xffffffff,
+		.pvr_value		=3D 0x0f000005,
+		.cpu_name		=3D "POWER9 (architected)",
+		.cpu_features		=3D CPU_FTRS_POWER9,
+		.cpu_user_features	=3D COMMON_USER_POWER9,
+		.cpu_user_features2	=3D COMMON_USER2_POWER9,
+		.mmu_features		=3D MMU_FTRS_POWER9,
+		.icache_bsize		=3D 128,
+		.dcache_bsize		=3D 128,
+		.oprofile_type		=3D PPC_OPROFILE_INVALID,
+		.oprofile_cpu_type	=3D "ppc64/ibm-compat-v1",
+		.cpu_setup		=3D __setup_cpu_power9,
+		.cpu_restore		=3D __restore_cpu_power9,
+		.flush_tlb		=3D __flush_tlb_power9,
+		.platform		=3D "power9",
+	},
 	{	/* Power7 */
 		.pvr_mask		=3D 0xffff0000,
 		.pvr_value		=3D 0x003f0000,

cheers

Re: [v2] powerpc: Add POWER9 architected mode to cputable

From: Michael Ellerman <hidden>
Date: 2017-02-19 11:33:44

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 <redacted>
Signed-off-by: Russell Currey <redacted>
Applied to powerpc next, thanks.

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

cheers
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help