Re: [v3 07/15] powerpc/perf: Add power10_feat to dt_cpu_ftrs
> On 22-Jul-2020, at 4:19 PM, Jordan Niethe wrote: > > On Wed, Jul 22, 2020 at 5:55 PM Athira Rajeev > mailto:atraj...@linux.vnet.ibm.com>> wrote: >> >> >> >> 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_LOADMISSTIME0x5 >> #endif >> >> +/* BHRB disable bit for PowerISA v3.10 */ >> +#define MMCRA_BHRB_DISABLE 0x0020 >> >> 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 0x0700UL /* SLOT bits (37-39) */ >> #define MMCRA_SLOT_SHIFT 24 >> #define MMCRA_SAMPLE_ENABLE 0x0001UL /* enable sampling */ >> +/* BHRB disable bit for PowerISA v3.10 */ >> +#define MMCRA_BHRB_DISABLE 0x0020 >> #define POWER6_MMCRA_SDSYNC 0x0800ULL/* SDAR/SIAR synced */ >> #define POWER6_MMCRA_SIHV 0x0400ULL >> #define POWER6_MMCRA_SIPR 0x0200ULL >> @@ -1068,9 +1070,6 @@ >> #define MMCR0_PMC2_LOADMISSTIME0x5 >> #endif >> >> >> >> -/* BHRB disable bit for PowerISA v3.10 */ >> -#define MMCRA_BHRB_DISABLE 0x0020 >> - >> /* >> * 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_ENABLE0 >> +#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) >> mflrr11 >> 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) > > Won't you also need to change where the label 1 is: > --- a/arch/powerpc/kernel/cpu_setup_power.S > +++ b/arch/powerpc/kernel/cpu_setup_power.S > @@ -100,8 +100,8 @@ _GLOBAL(__setup_cpu_power10) > _GLOBAL(__setup_cpu_power9) >mflrr11 >bl __init_FSCR > -1: bl __init_PMU > - bl __init_hvmode
Re: [v3 07/15] powerpc/perf: Add power10_feat to dt_cpu_ftrs
> On 22-Jul-2020, at 10:11 AM, Jordan Niethe wrote: > > On Sat, Jul 18, 2020 at 1:13 AM Athira Rajeev > mailto:atraj...@linux.vnet.ibm.com>> 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_LOADMISSTIME0x5 >> #endif >> >> +/* BHRB disable bit for PowerISA v3.10 */ >> +#define MMCRA_BHRB_DISABLE 0x0020 > 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 0x0700UL /* SLOT bits (37-39) */ #define MMCRA_SLOT_SHIFT 24 #define MMCRA_SAMPLE_ENABLE 0x0001UL /* enable sampling */ +/* BHRB disable bit for PowerISA v3.10 */ +#define MMCRA_BHRB_DISABLE 0x0020 #define POWER6_MMCRA_SDSYNC 0x0800ULL/* SDAR/SIAR synced */ #define POWER6_MMCRA_SIHV 0x0400ULL #define POWER6_MMCRA_SIPR 0x0200ULL @@ -1068,9 +1070,6 @@ #define MMCR0_PMC2_LOADMISSTIME0x5 #endif -/* BHRB disable bit for PowerISA v3.10 */ -#define MMCRA_BHRB_DISABLE 0x0020 - /* * 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_ENABLE0 +#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) >>mflrr11 >>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) mflrr11 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) mflrr11 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 @@
Re: [v3 07/15] powerpc/perf: Add power10_feat to dt_cpu_ftrs
On Wed, Jul 22, 2020 at 5:55 PM Athira Rajeev wrote: > > > > 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_LOADMISSTIME0x5 > #endif > > +/* BHRB disable bit for PowerISA v3.10 */ > +#define MMCRA_BHRB_DISABLE 0x0020 > > 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 0x0700UL /* SLOT bits (37-39) */ > #define MMCRA_SLOT_SHIFT 24 > #define MMCRA_SAMPLE_ENABLE 0x0001UL /* enable sampling */ > +/* BHRB disable bit for PowerISA v3.10 */ > +#define MMCRA_BHRB_DISABLE 0x0020 > #define POWER6_MMCRA_SDSYNC 0x0800ULL/* SDAR/SIAR synced */ > #define POWER6_MMCRA_SIHV 0x0400ULL > #define POWER6_MMCRA_SIPR 0x0200ULL > @@ -1068,9 +1070,6 @@ > #define MMCR0_PMC2_LOADMISSTIME0x5 > #endif > > > > -/* BHRB disable bit for PowerISA v3.10 */ > -#define MMCRA_BHRB_DISABLE 0x0020 > - > /* > * 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_ENABLE0 > +#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) >mflrr11 >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) Won't you also need to change where the label 1 is: --- a/arch/powerpc/kernel/cpu_setup_power.S +++ b/arch/powerpc/kernel/cpu_setup_power.S @@ -100,8 +100,8 @@ _GLOBAL(__setup_cpu_power10) _GLOBAL(__setup_cpu_power9) mflrr11 bl __init_FSCR -1: bl __init_PMU - bl __init_hvmode_206 + bl __init_PMU +1: bl __init_hvmode_206 mtlrr11 beqlr li r0,0 > @@ -124,6 +127,9 @@ _GLOBAL(__setup_cpu_power9) > _GLOBAL(__restore_cpu_power10) > mflr r11 > bl __init_FSCR_power10 > + bl __init
Re: [v3 07/15] powerpc/perf: Add power10_feat to dt_cpu_ftrs
Athira Rajeev writes: >> On 22-Jul-2020, at 10:11 AM, Jordan Niethe wrote: >> >> On Sat, Jul 18, 2020 at 1:13 AM Athira Rajeev >> mailto:atraj...@linux.vnet.ibm.com>> 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_LOADMISSTIME0x5 >>> #endif >>> >>> +/* BHRB disable bit for PowerISA v3.10 */ >>> +#define MMCRA_BHRB_DISABLE 0x0020 >> 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 0x0700UL /* SLOT bits (37-39) */ > #define MMCRA_SLOT_SHIFT 24 > #define MMCRA_SAMPLE_ENABLE 0x0001UL /* enable sampling */ > +/* BHRB disable bit for PowerISA v3.10 */ > +#define MMCRA_BHRB_DISABLE 0x0020 I changed it to: #define MMCRA_BHRB_DISABLE 0x20UL // BHRB disable bit for ISA v3.1 >>> 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) >>>mflrr11 >>>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 Actually those changes should be in a separate patch. This one is wiring up DT CPU features, the cpu setup routines are not used by DT CPU features. So please send a new patch I can insert into the series that adds the cpu_setup_power.S changes. cheers > -- > 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) > mflrr11 > 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) > mflrr11 > 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 >
Re: [v3 07/15] powerpc/perf: Add power10_feat to dt_cpu_ftrs
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_LOADMISSTIME0x5 > #endif > > +/* BHRB disable bit for PowerISA v3.10 */ > +#define MMCRA_BHRB_DISABLE 0x0020 Shouldn't this go under SPRN_MMCRA with the other MMCRA_*. > + > /* > * 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) > mflrr11 > 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? > 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 >
[v3 07/15] powerpc/perf: Add power10_feat to dt_cpu_ftrs
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_LOADMISSTIME0x5 #endif +/* BHRB disable bit for PowerISA v3.10 */ +#define MMCRA_BHRB_DISABLE 0x0020 + /* * 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) mflrr11 bl __init_FSCR_power10 + bl __init_PMU_ISA31 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