>> --- >> arch/powerpc/include/asm/reg.h | 3 +++ >> arch/powerpc/kernel/cpu_setup_power.S | 8 ++++++++ >> arch/powerpc/kernel/dt_cpu_ftrs.c | 26 ++++++++++++++++++++++++++ >> 3 files changed, 37 insertions(+) >>=20 >> diff --git a/arch/powerpc/include/asm/reg.h = b/arch/powerpc/include/asm/reg.h >> index 21a1b2d..900ada1 100644 >> --- a/arch/powerpc/include/asm/reg.h >> +++ b/arch/powerpc/include/asm/reg.h >> @@ -1068,6 +1068,9 @@ >> #define MMCR0_PMC2_LOADMISSTIME 0x5 >> #endif >>=20 >> +/* BHRB disable bit for PowerISA v3.10 */ >> +#define MMCRA_BHRB_DISABLE 0x0000002000000000 > Shouldn't this go under SPRN_MMCRA with the other MMCRA_*. Hi Jordan Ok, the definition of MMCRA is under #ifdef for 64 bit . if I move = definition of MMCRA_BHRB_DISABLE along with other SPR's, I also need to define this for 32-bit to satisfy core-book3s to compile as = below: diff --git a/arch/powerpc/include/asm/reg.h = b/arch/powerpc/include/asm/reg.h index 900ada10762c..7e271657b412 100644 --- a/arch/powerpc/include/asm/reg.h +++ b/arch/powerpc/include/asm/reg.h @@ -888,6 +888,8 @@ #define MMCRA_SLOT 0x07000000UL /* SLOT bits (37-39) */ #define MMCRA_SLOT_SHIFT 24 #define MMCRA_SAMPLE_ENABLE 0x00000001UL /* enable sampling */ +/* BHRB disable bit for PowerISA v3.10 */ +#define MMCRA_BHRB_DISABLE 0x0000002000000000 #define POWER6_MMCRA_SDSYNC 0x0000080000000000ULL /* SDAR/SIAR = synced */ #define POWER6_MMCRA_SIHV 0x0000040000000000ULL #define POWER6_MMCRA_SIPR 0x0000020000000000ULL @@ -1068,9 +1070,6 @@ #define MMCR0_PMC2_LOADMISSTIME 0x5 #endif =20 -/* BHRB disable bit for PowerISA v3.10 */ -#define MMCRA_BHRB_DISABLE 0x0000002000000000 - /* * SPRG usage: * diff --git a/arch/powerpc/perf/core-book3s.c = b/arch/powerpc/perf/core-book3s.c index 36baae666387..88068f20827c 100644 --- a/arch/powerpc/perf/core-book3s.c +++ b/arch/powerpc/perf/core-book3s.c @@ -94,6 +94,7 @@ static unsigned int freeze_events_kernel =3D = MMCR0_FCS; #define SPRN_SIER2 0 #define SPRN_SIER3 0 #define MMCRA_SAMPLE_ENABLE 0 +#define MMCRA_BHRB_DISABLE 0 =20 static inline unsigned long perf_ip_adjust(struct pt_regs *regs) { >> + >> /* >> * SPRG usage: >> * >> diff --git a/arch/powerpc/kernel/cpu_setup_power.S = b/arch/powerpc/kernel/cpu_setup_power.S >> index efdcfa7..b8e0d1e 100644 >> --- a/arch/powerpc/kernel/cpu_setup_power.S >> +++ b/arch/powerpc/kernel/cpu_setup_power.S >> @@ -94,6 +94,7 @@ _GLOBAL(__restore_cpu_power8) >> _GLOBAL(__setup_cpu_power10) >> mflr r11 >> bl __init_FSCR_power10 >> + bl __init_PMU_ISA31 > So we set MMCRA here but then aren't we still going to call __init_PMU > which will overwrite that? > Would this setting MMCRA also need to be handled in = __restore_cpu_power10? Thanks for this nice catch ! When I rebased code initial phase, we = didn=E2=80=99t had power10 part filled in. It was a miss from my side in adding PMu init functions and thanks for = pointing this out.=20 Below patch will call __init_PMU functions in setup and restore. Please = check if this looks good -- diff --git a/arch/powerpc/kernel/cpu_setup_power.S = b/arch/powerpc/kernel/cpu_setup_power.S index efdcfa714106..e672a6c5fd7c 100644 --- a/arch/powerpc/kernel/cpu_setup_power.S +++ b/arch/powerpc/kernel/cpu_setup_power.S @@ -94,6 +94,9 @@ _GLOBAL(__restore_cpu_power8) _GLOBAL(__setup_cpu_power10) mflr r11 bl __init_FSCR_power10 + bl __init_PMU + bl __init_PMU_ISA31 + bl __init_PMU_HV b 1f =20 _GLOBAL(__setup_cpu_power9) @@ -124,6 +127,9 @@ _GLOBAL(__setup_cpu_power9) _GLOBAL(__restore_cpu_power10) mflr r11 bl __init_FSCR_power10 + bl __init_PMU + bl __init_PMU_ISA31 + bl __init_PMU_HV b 1f =20 _GLOBAL(__restore_cpu_power9) @@ -233,3 +239,10 @@ __init_PMU_ISA207: li r5,0 mtspr SPRN_MMCRS,r5 blr + +__init_PMU_ISA31: + li r5,0 + mtspr SPRN_MMCR3,r5 + LOAD_REG_IMMEDIATE(r5, MMCRA_BHRB_DISABLE) + mtspr SPRN_MMCRA,r5 + blr =E2=80=94 >> b 1f >>=20 >> _GLOBAL(__setup_cpu_power9) >> @@ -233,3 +234,10 @@ __init_PMU_ISA207: >> li r5,0 >> mtspr SPRN_MMCRS,r5 >> blr >> + >> +__init_PMU_ISA31: >> + li r5,0 >> + mtspr SPRN_MMCR3,r5 >> + LOAD_REG_IMMEDIATE(r5, MMCRA_BHRB_DISABLE) >> + mtspr SPRN_MMCRA,r5 >> + blr >> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c = b/arch/powerpc/kernel/dt_cpu_ftrs.c >> index 3a40951..f482286 100644 >> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c >> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c >> @@ -450,6 +450,31 @@ static int __init feat_enable_pmu_power9(struct = dt_cpu_feature *f) >> return 1; >> } >>=20 >> +static void init_pmu_power10(void) >> +{ >> + init_pmu_power9(); >> + >> + mtspr(SPRN_MMCR3, 0); >> + mtspr(SPRN_MMCRA, MMCRA_BHRB_DISABLE); >> +} >> + >> +static int __init feat_enable_pmu_power10(struct dt_cpu_feature *f) >> +{ >> + hfscr_pmu_enable(); >> + >> + init_pmu_power10(); >> + init_pmu_registers =3D init_pmu_power10; >> + >> + cur_cpu_spec->cpu_features |=3D CPU_FTR_MMCRA; >> + cur_cpu_spec->cpu_user_features |=3D = PPC_FEATURE_PSERIES_PERFMON_COMPAT; >> + >> + cur_cpu_spec->num_pmcs =3D 6; >> + cur_cpu_spec->pmc_type =3D PPC_PMC_IBM; >> + cur_cpu_spec->oprofile_cpu_type =3D "ppc64/power10"; >> + >> + return 1; >> +} >> + >> static int __init feat_enable_tm(struct dt_cpu_feature *f) >> { >> #ifdef CONFIG_PPC_TRANSACTIONAL_MEM >> @@ -639,6 +664,7 @@ struct dt_cpu_feature_match { >> {"pc-relative-addressing", feat_enable, 0}, >> {"machine-check-power9", feat_enable_mce_power9, 0}, >> {"performance-monitor-power9", feat_enable_pmu_power9, 0}, >> + {"performance-monitor-power10", feat_enable_pmu_power10, 0}, >> {"event-based-branch-v3", feat_enable, 0}, >> {"random-number-generator", feat_enable, 0}, >> {"system-call-vectored", feat_disable, 0}, >> -- >> 1.8.3.1 --Apple-Mail=_28D83C0F-1F3F-49AA-95FF-7F8F48673511 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8

On 22-Jul-2020, at 10:11 AM, Jordan Niethe <jniethe5@gmail.com> = wrote:

On Sat, Jul 18, 2020 at 1:13 AM = Athira Rajeev
<atrajeev@linux.vnet.ibm.com> wrote:

From: Madhavan = Srinivasan <maddy@linux.ibm.com>

Add = power10 feature function to dt_cpu_ftrs.c along
with a = power10 specific init() to initialize pmu sprs,
sets the = oprofile_cpu_type and cpu_features. This will
enable = performance monitoring unit(PMU) for Power10
in CPU = features with "performance-monitor-power10".

For PowerISA v3.1, BHRB disable is controlled via Monitor = Mode
Control Register A (MMCRA) bit, namely "BHRB = Recording Disable
(BHRBRD)". This patch initializes MMCRA = BHRBRD to disable BHRB
feature at boot for power10.

Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
---
arch/powerpc/include/asm/reg.h =        | 3 +++
arch/powerpc/kernel/cpu_setup_power.S | 8 ++++++++
arch/powerpc/kernel/dt_cpu_ftrs.c     | = 26 ++++++++++++++++++++++++++
3 files changed, 37 = insertions(+)

diff --git = a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 21a1b2d..900ada1 100644
--- = a/arch/powerpc/include/asm/reg.h
+++ = b/arch/powerpc/include/asm/reg.h
@@ -1068,6 +1068,9 @@
#define MMCR0_PMC2_LOADMISSTIME =        0x5
#endif

+/* BHRB disable bit for PowerISA v3.10 */
+#define MMCRA_BHRB_DISABLE =     0x0000002000000000
Shouldn't this go under SPRN_MMCRA with the other = MMCRA_*.

Hi Jordan

Ok, the = definition of MMCRA is under #ifdef for 64 bit . if I move = definition of MMCRA_BHRB_DISABLE along with other SPR's, I = also

need to define this for 32-bit to satisfy core-book3s to = compile as below:

diff --git = a/arch/powerpc/include/asm/reg.h = b/arch/powerpc/include/asm/reg.h

index 900ada10762c..7e271657b412 = 100644

--- a/arch/powerpc/include/asm/reg.h

+++ = b/arch/powerpc/include/asm/reg.h

@@ -888,6 +888,8 @@

#define MMCRA_SLOT = 0x07000000UL /* SLOT bits (37-39) */

#define = MMCRA_SLOT_SHIFT 24

#define MMCRA_SAMPLE_ENABLE = 0x00000001UL /* enable sampling */

+/* BHRB disable bit for PowerISA v3.10 = */

+#define = MMCRA_BHRB_DISABLE 0x0000002000000000

#define = POWER6_MMCRA_SDSYNC 0x0000080000000000ULL /* = SDAR/SIAR synced */

#define POWER6_MMCRA_SIHV = 0x0000040000000000ULL

#define POWER6_MMCRA_SIPR = 0x0000020000000000ULL

@@ -1068,9 +1070,6 @@

#define MMCR0_PMC2_LOADMISSTIME = 0x5

#endif

-/* BHRB disable bit for PowerISA v3.10 = */

-#define = MMCRA_BHRB_DISABLE 0x0000002000000000

* SPRG = usage:

diff --git a/arch/powerpc/perf/core-book3s.c = b/arch/powerpc/perf/core-book3s.c

index 36baae666387..88068f20827c = 100644

--- a/arch/powerpc/perf/core-book3s.c

+++ = b/arch/powerpc/perf/core-book3s.c

@@ -94,6 +94,7 @@ static unsigned int = freeze_events_kernel =3D MMCR0_FCS;

#define SPRN_SIER2 = 0

#define SPRN_SIER3 = 0

#define MMCRA_SAMPLE_ENABLE 0

+#define = MMCRA_BHRB_DISABLE 0

static inline unsigned long = perf_ip_adjust(struct pt_regs *regs)

{

+
/*
* SPRG usage:
*
diff --git a/arch/powerpc/kernel/cpu_setup_power.S = b/arch/powerpc/kernel/cpu_setup_power.S
index = efdcfa7..b8e0d1e 100644
--- = a/arch/powerpc/kernel/cpu_setup_power.S
+++ = b/arch/powerpc/kernel/cpu_setup_power.S
@@ -94,6 +94,7 @@ = _GLOBAL(__restore_cpu_power8)
_GLOBAL(__setup_cpu_power10)
       mflr =    r11
       bl =      __init_FSCR_power10
+ =       bl =      __init_PMU_ISA31
So we set MMCRA here but then aren't we still going to call = __init_PMU
which will = overwrite that?
Would this setting MMCRA also need to be handled in = __restore_cpu_power10?

Thanks for this nice catch ! When I rebased code = initial phase, we didn=E2=80=99t had power10 part filled = in.

It was a miss from my side in adding PMu init functions = and thanks for pointing this out.

Below patch will call = __init_PMU functions in setup and restore. Please check if this looks = good

diff --git = a/arch/powerpc/kernel/cpu_setup_power.S = b/arch/powerpc/kernel/cpu_setup_power.S

index = efdcfa714106..e672a6c5fd7c 100644

--- = a/arch/powerpc/kernel/cpu_setup_power.S

+++ = b/arch/powerpc/kernel/cpu_setup_power.S

@@ -94,6 +94,9 @@ = _GLOBAL(__restore_cpu_power8)

_GLOBAL(__setup_cpu_power10)=

= mflr r11

bl = __init_FSCR_power10

+ bl __init_PMU

+ bl = __init_PMU_ISA31

+ bl = __init_PMU_HV

b = 1f

_GLOBAL(__setup_cpu_power9)

@@ -124,6 +127,9 @@ = _GLOBAL(__setup_cpu_power9)

_GLOBAL(__restore_cpu_power10)=

= mflr r11

bl = __init_FSCR_power10

+ bl __init_PMU

+ bl = __init_PMU_ISA31

+ bl = __init_PMU_HV

b = 1f

_GLOBAL(__restore_cpu_power9)

@@ -233,3 +239,10 @@ __init_PMU_ISA207:

li = r5,0

mtspr = SPRN_MMCRS,r5

= blr

+__init_PMU_ISA31:

+ li = r5,0

+ mtspr SPRN_MMCR3,r5

+ = LOAD_REG_IMMEDIATE(r5, MMCRA_BHRB_DISABLE)

+ = mtspr = SPRN_MMCRA,r5

+ blr

=E2=80=94

       b =       1f

_GLOBAL(__setup_cpu_power9)
@@ -233,3 +234,10 = @@ __init_PMU_ISA207:
       li =      r5,0
       mtspr =   SPRN_MMCRS,r5
       blr
+
+__init_PMU_ISA31:
+ =       li =      r5,0
+ =       mtspr   SPRN_MMCR3,r5
+       LOAD_REG_IMMEDIATE(r5, = MMCRA_BHRB_DISABLE)
+ =       mtspr   SPRN_MMCRA,r5
+       blr
diff = --git a/arch/powerpc/kernel/dt_cpu_ftrs.c = b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 3a40951..f482286 = 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ = -450,6 +450,31 @@ static int __init feat_enable_pmu_power9(struct = dt_cpu_feature *f)
       return 1;
}

+static void = init_pmu_power10(void)
+{
+ =       init_pmu_power9();
+
+       mtspr(SPRN_MMCR3, = 0);
+ =       mtspr(SPRN_MMCRA, = MMCRA_BHRB_DISABLE);
+}
+
+static int __init feat_enable_pmu_power10(struct = dt_cpu_feature *f)
+{
+ =       hfscr_pmu_enable();
++       init_pmu_power10();
+       init_pmu_registers =3D = init_pmu_power10;
+
+ =       cur_cpu_spec->cpu_features |=3D = CPU_FTR_MMCRA;
+ =       cur_cpu_spec->cpu_user_features = |=3D PPC_FEATURE_PSERIES_PERFMON_COMPAT;
+
+ =       cur_cpu_spec->num_pmcs =          =3D 6;
+ =       cur_cpu_spec->pmc_type =          =3D = PPC_PMC_IBM;
+ =       cur_cpu_spec->oprofile_cpu_type =3D= "ppc64/power10";
+
+ =       return 1;
+}
+
static int __init feat_enable_tm(struct = dt_cpu_feature *f)
{
#ifdef = CONFIG_PPC_TRANSACTIONAL_MEM
@@ -639,6 +664,7 @@ struct = dt_cpu_feature_match {
       {"pc-relative-address= ing", feat_enable, 0},
       {"machine-check-power= 9", feat_enable_mce_power9, 0},
       {"performance-monitor= -power9", feat_enable_pmu_power9, 0},
+ =       {"performance-monitor-power10", = feat_enable_pmu_power10, 0},
       {"event-based-branch-= v3", feat_enable, 0},
       {"random-number-gener= ator", feat_enable, 0},
       {"system-call-vectore= d", feat_disable, 0},
--
1.8.3.1

= --Apple-Mail=_28D83C0F-1F3F-49AA-95FF-7F8F48673511--