> On 22-Jul-2020, at 10:11 AM, Jordan Niethe wrote: > > On Sat, Jul 18, 2020 at 1:13 AM Athira Rajeev > > wrote: >> >> From: Madhavan Srinivasan >> >> 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 >> --- >> 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 = 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’t 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 — >> 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 = init_pmu_power10; >> + >> + cur_cpu_spec->cpu_features |= CPU_FTR_MMCRA; >> + cur_cpu_spec->cpu_user_features |= PPC_FEATURE_PSERIES_PERFMON_COMPAT; >> + >> + cur_cpu_spec->num_pmcs = 6; >> + cur_cpu_spec->pmc_type = PPC_PMC_IBM; >> + cur_cpu_spec->oprofile_cpu_type = "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