Re: [v3 04/15] powerpc/perf: Add support for ISA3.1 PMU SPRs
> On 22-Jul-2020, at 9:48 AM, Jordan Niethe wrote:
>
> On Sat, Jul 18, 2020 at 1:02 AM Athira Rajeev
> mailto:atraj...@linux.vnet.ibm.com>> wrote:
>>
>> From: Madhavan Srinivasan
>>
>> PowerISA v3.1 includes new performance monitoring unit(PMU)
>> special purpose registers (SPRs). They are
>>
>> Monitor Mode Control Register 3 (MMCR3)
>> Sampled Instruction Event Register 2 (SIER2)
>> Sampled Instruction Event Register 3 (SIER3)
>>
>> MMCR3 is added for further sampling related configuration
>> control. SIER2/SIER3 are added to provide additional
>> information about the sampled instruction.
>>
>> Patch adds new PPMU flag called "PPMU_ARCH_310S" to support
>> handling of these new SPRs, updates the struct thread_struct
>> to include these new SPRs, include MMCR3 in struct mmcr_regs.
>> This is needed to support programming of MMCR3 SPR during
>> event_[enable/disable]. Patch also adds the sysfs support
>> for the MMCR3 SPR along with SPRN_ macros for these new pmu sprs.
>>
>> Signed-off-by: Madhavan Srinivasan
>> ---
>> arch/powerpc/include/asm/perf_event_server.h | 2 ++
>> arch/powerpc/include/asm/processor.h | 4
>> arch/powerpc/include/asm/reg.h | 6 ++
>> arch/powerpc/kernel/sysfs.c | 8
>> arch/powerpc/perf/core-book3s.c | 29
>>
>> 5 files changed, 49 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/perf_event_server.h
>> b/arch/powerpc/include/asm/perf_event_server.h
>> index 14b8dc1..832450a 100644
>> --- a/arch/powerpc/include/asm/perf_event_server.h
>> +++ b/arch/powerpc/include/asm/perf_event_server.h
>> @@ -22,6 +22,7 @@ struct mmcr_regs {
>>unsigned long mmcr1;
>>unsigned long mmcr2;
>>unsigned long mmcra;
>> + unsigned long mmcr3;
>> };
>> /*
>> * This struct provides the constants and functions needed to
>> @@ -75,6 +76,7 @@ struct power_pmu {
>> #define PPMU_HAS_SIER 0x0040 /* Has SIER */
>> #define PPMU_ARCH_207S 0x0080 /* PMC is architecture v2.07S */
>> #define PPMU_NO_SIAR 0x0100 /* Do not use SIAR */
>> +#define PPMU_ARCH_310S 0x0200 /* Has MMCR3, SIER2 and SIER3 */
> We elsewhere have CPU_FTR_ARCH_31, so should this be PPMU_ARCH_31S to
> be consistent.
>>
Ok,
This change will need to be done in all places which are currently using
PPMU_ARCH_310S
>> /*
>> * Values for flags to get_alternatives()
>> diff --git a/arch/powerpc/include/asm/processor.h
>> b/arch/powerpc/include/asm/processor.h
>> index 52a6783..a466e94 100644
>> --- a/arch/powerpc/include/asm/processor.h
>> +++ b/arch/powerpc/include/asm/processor.h
>> @@ -272,6 +272,10 @@ struct thread_struct {
>>unsignedmmcr0;
>>
>>unsignedused_ebb;
>> + unsigned long mmcr3;
>> + unsigned long sier2;
>> + unsigned long sier3;
>> +
>> #endif
>> };
>>
>> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
>> index 88e6c78..21a1b2d 100644
>> --- a/arch/powerpc/include/asm/reg.h
>> +++ b/arch/powerpc/include/asm/reg.h
>> @@ -876,7 +876,9 @@
>> #define MMCR0_FCHV 0x0001UL /* freeze conditions in hypervisor mode
>> */
>> #define SPRN_MMCR1 798
>> #define SPRN_MMCR2 785
>> +#define SPRN_MMCR3 754
>> #define SPRN_UMMCR2769
>> +#define SPRN_UMMCR3738
>> #define SPRN_MMCRA 0x312
>> #define MMCRA_SDSYNC 0x8000UL /* SDAR synced with SIAR */
>> #define MMCRA_SDAR_DCACHE_MISS 0x4000UL
>> @@ -918,6 +920,10 @@
>> #define SIER_SIHV0x100 /* Sampled MSR_HV */
>> #define SIER_SIAR_VALID 0x040 /* SIAR contents valid */
>> #define SIER_SDAR_VALID 0x020 /* SDAR contents valid */
>> +#define SPRN_SIER2 752
>> +#define SPRN_SIER3 753
>> +#define SPRN_USIER2736
>> +#define SPRN_USIER3737
>> #define SPRN_SIAR 796
>> #define SPRN_SDAR 797
>> #define SPRN_TACR 888
>> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
>> index 571b325..46b4ebc 100644
>> --- a/arch/powerpc/kernel/sysfs.c
>> +++ b/arch/powerpc/kernel/sysfs.c
>> @@ -622,8 +622,10 @@ void ppc_enable_pmcs(void)
>> SYSFS_PMCSETUP(pmc8, SPRN_PMC8);
>>
>> SYSFS_PMCSETUP(mmcra, SPRN_MMCRA);
>> +SYSFS_PMCSETUP(mmcr3, SPRN_MMCR3);
>>
>> static DEVICE_ATTR(mmcra, 0600, show_mmcra, store_mmcra);
>> +static DEVICE_ATTR(mmcr3, 0600, show_mmcr3, store_mmcr3);
>> #endif /* HAS_PPC_PMC56 */
>>
>>
>> @@ -886,6 +888,9 @@ static int register_cpu_online(unsigned int cpu)
>> #ifdef CONFIG_PMU_SYSFS
>>if (cpu_has_feature(CPU_FTR_MMCRA))
>>device_create_file(s, _attr_mmcra);
>> +
>> + if (cpu_has_feature(CPU_FTR_ARCH_31))
>> + device_create_file(s, _attr_mmcr3);
>> #endif /* CONFIG_PMU_SYSFS */
>>
>>if (cpu_has_feature(CPU_FTR_PURR)) {
>> @@ -980,6 +985,9 @@ static int unregister_cpu_online(unsigned int cpu)
>> #ifdef
Re: [v3 04/15] powerpc/perf: Add support for ISA3.1 PMU SPRs
Jordan Niethe writes:
> On Sat, Jul 18, 2020 at 1:02 AM Athira Rajeev
> wrote:
>> From: Madhavan Srinivasan
>>
>> PowerISA v3.1 includes new performance monitoring unit(PMU)
>> special purpose registers (SPRs). They are
...
>>
>> diff --git a/arch/powerpc/include/asm/perf_event_server.h
>> b/arch/powerpc/include/asm/perf_event_server.h
>> index 14b8dc1..832450a 100644
>> --- a/arch/powerpc/include/asm/perf_event_server.h
>> +++ b/arch/powerpc/include/asm/perf_event_server.h
>> @@ -75,6 +76,7 @@ struct power_pmu {
>> #define PPMU_HAS_SIER 0x0040 /* Has SIER */
>> #define PPMU_ARCH_207S 0x0080 /* PMC is architecture v2.07S */
>> #define PPMU_NO_SIAR 0x0100 /* Do not use SIAR */
>> +#define PPMU_ARCH_310S 0x0200 /* Has MMCR3, SIER2 and SIER3 */
> We elsewhere have CPU_FTR_ARCH_31, so should this be PPMU_ARCH_31S to
> be consistent.
The "S" is no longer needed as there's no Book S vs Book E distinction
in ISA v3.1.
So I changed it to PPMU_ARCH_31.
>> diff --git a/arch/powerpc/perf/core-book3s.c
>> b/arch/powerpc/perf/core-book3s.c
>> index f4d07b5..ca32fc0 100644
>> --- a/arch/powerpc/perf/core-book3s.c
>> +++ b/arch/powerpc/perf/core-book3s.c
>> @@ -581,6 +589,11 @@ static void ebb_switch_out(unsigned long mmcr0)
>> current->thread.sdar = mfspr(SPRN_SDAR);
>> current->thread.mmcr0 = mmcr0 & MMCR0_USER_MASK;
>> current->thread.mmcr2 = mfspr(SPRN_MMCR2) & MMCR2_USER_MASK;
>> + if (ppmu->flags & PPMU_ARCH_310S) {
>> + current->thread.mmcr3 = mfspr(SPRN_MMCR3);
> Like MMCR0_USER_MASK and MMCR2_USER_MASK do we need a MMCR3_USER_MASK
> here, or is there no need?
mmcr0 and mmcr2 are visible via ptrace, so masking them here means we
don't expose any bits to userspace via ptrace that aren't also visible
by reading the register.
So at least while mmcr3 is not exposed via ptrace it's safe to not mask
it, if there are even any sensitive bits in it.
cheers
Re: [v3 04/15] powerpc/perf: Add support for ISA3.1 PMU SPRs
On Wed, Jul 22, 2020 at 6:07 PM Athira Rajeev
wrote:
>
>
>
> On 22-Jul-2020, at 9:48 AM, Jordan Niethe wrote:
>
> On Sat, Jul 18, 2020 at 1:02 AM Athira Rajeev
> wrote:
>
>
> From: Madhavan Srinivasan
>
> PowerISA v3.1 includes new performance monitoring unit(PMU)
> special purpose registers (SPRs). They are
>
> Monitor Mode Control Register 3 (MMCR3)
> Sampled Instruction Event Register 2 (SIER2)
> Sampled Instruction Event Register 3 (SIER3)
>
> MMCR3 is added for further sampling related configuration
> control. SIER2/SIER3 are added to provide additional
> information about the sampled instruction.
>
> Patch adds new PPMU flag called "PPMU_ARCH_310S" to support
> handling of these new SPRs, updates the struct thread_struct
> to include these new SPRs, include MMCR3 in struct mmcr_regs.
> This is needed to support programming of MMCR3 SPR during
> event_[enable/disable]. Patch also adds the sysfs support
> for the MMCR3 SPR along with SPRN_ macros for these new pmu sprs.
>
> Signed-off-by: Madhavan Srinivasan
> ---
> arch/powerpc/include/asm/perf_event_server.h | 2 ++
> arch/powerpc/include/asm/processor.h | 4
> arch/powerpc/include/asm/reg.h | 6 ++
> arch/powerpc/kernel/sysfs.c | 8
> arch/powerpc/perf/core-book3s.c | 29
> 5 files changed, 49 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/perf_event_server.h
> b/arch/powerpc/include/asm/perf_event_server.h
> index 14b8dc1..832450a 100644
> --- a/arch/powerpc/include/asm/perf_event_server.h
> +++ b/arch/powerpc/include/asm/perf_event_server.h
> @@ -22,6 +22,7 @@ struct mmcr_regs {
>unsigned long mmcr1;
>unsigned long mmcr2;
>unsigned long mmcra;
> + unsigned long mmcr3;
> };
> /*
> * This struct provides the constants and functions needed to
> @@ -75,6 +76,7 @@ struct power_pmu {
> #define PPMU_HAS_SIER 0x0040 /* Has SIER */
> #define PPMU_ARCH_207S 0x0080 /* PMC is architecture v2.07S */
> #define PPMU_NO_SIAR 0x0100 /* Do not use SIAR */
> +#define PPMU_ARCH_310S 0x0200 /* Has MMCR3, SIER2 and SIER3 */
>
> We elsewhere have CPU_FTR_ARCH_31, so should this be PPMU_ARCH_31S to
> be consistent.
>
>
>
> Ok,
> This change will need to be done in all places which are currently using
> PPMU_ARCH_310S
>
> /*
> * Values for flags to get_alternatives()
> diff --git a/arch/powerpc/include/asm/processor.h
> b/arch/powerpc/include/asm/processor.h
> index 52a6783..a466e94 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -272,6 +272,10 @@ struct thread_struct {
>unsignedmmcr0;
>
>unsignedused_ebb;
> + unsigned long mmcr3;
> + unsigned long sier2;
> + unsigned long sier3;
> +
> #endif
> };
>
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 88e6c78..21a1b2d 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -876,7 +876,9 @@
> #define MMCR0_FCHV 0x0001UL /* freeze conditions in hypervisor mode */
> #define SPRN_MMCR1 798
> #define SPRN_MMCR2 785
> +#define SPRN_MMCR3 754
> #define SPRN_UMMCR2769
> +#define SPRN_UMMCR3738
> #define SPRN_MMCRA 0x312
> #define MMCRA_SDSYNC 0x8000UL /* SDAR synced with SIAR */
> #define MMCRA_SDAR_DCACHE_MISS 0x4000UL
> @@ -918,6 +920,10 @@
> #define SIER_SIHV0x100 /* Sampled MSR_HV */
> #define SIER_SIAR_VALID 0x040 /* SIAR contents valid */
> #define SIER_SDAR_VALID 0x020 /* SDAR contents valid */
> +#define SPRN_SIER2 752
> +#define SPRN_SIER3 753
> +#define SPRN_USIER2736
> +#define SPRN_USIER3737
> #define SPRN_SIAR 796
> #define SPRN_SDAR 797
> #define SPRN_TACR 888
> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> index 571b325..46b4ebc 100644
> --- a/arch/powerpc/kernel/sysfs.c
> +++ b/arch/powerpc/kernel/sysfs.c
> @@ -622,8 +622,10 @@ void ppc_enable_pmcs(void)
> SYSFS_PMCSETUP(pmc8, SPRN_PMC8);
>
> SYSFS_PMCSETUP(mmcra, SPRN_MMCRA);
> +SYSFS_PMCSETUP(mmcr3, SPRN_MMCR3);
>
> static DEVICE_ATTR(mmcra, 0600, show_mmcra, store_mmcra);
> +static DEVICE_ATTR(mmcr3, 0600, show_mmcr3, store_mmcr3);
> #endif /* HAS_PPC_PMC56 */
>
>
> @@ -886,6 +888,9 @@ static int register_cpu_online(unsigned int cpu)
> #ifdef CONFIG_PMU_SYSFS
>if (cpu_has_feature(CPU_FTR_MMCRA))
>device_create_file(s, _attr_mmcra);
> +
> + if (cpu_has_feature(CPU_FTR_ARCH_31))
> + device_create_file(s, _attr_mmcr3);
> #endif /* CONFIG_PMU_SYSFS */
>
>if (cpu_has_feature(CPU_FTR_PURR)) {
> @@ -980,6 +985,9 @@ static int unregister_cpu_online(unsigned int cpu)
> #ifdef CONFIG_PMU_SYSFS
>if (cpu_has_feature(CPU_FTR_MMCRA))
>device_remove_file(s, _attr_mmcra);
> +
Re: [v3 04/15] powerpc/perf: Add support for ISA3.1 PMU SPRs
On Sat, Jul 18, 2020 at 1:02 AM Athira Rajeev
wrote:
>
> From: Madhavan Srinivasan
>
> PowerISA v3.1 includes new performance monitoring unit(PMU)
> special purpose registers (SPRs). They are
>
> Monitor Mode Control Register 3 (MMCR3)
> Sampled Instruction Event Register 2 (SIER2)
> Sampled Instruction Event Register 3 (SIER3)
>
> MMCR3 is added for further sampling related configuration
> control. SIER2/SIER3 are added to provide additional
> information about the sampled instruction.
>
> Patch adds new PPMU flag called "PPMU_ARCH_310S" to support
> handling of these new SPRs, updates the struct thread_struct
> to include these new SPRs, include MMCR3 in struct mmcr_regs.
> This is needed to support programming of MMCR3 SPR during
> event_[enable/disable]. Patch also adds the sysfs support
> for the MMCR3 SPR along with SPRN_ macros for these new pmu sprs.
>
> Signed-off-by: Madhavan Srinivasan
> ---
> arch/powerpc/include/asm/perf_event_server.h | 2 ++
> arch/powerpc/include/asm/processor.h | 4
> arch/powerpc/include/asm/reg.h | 6 ++
> arch/powerpc/kernel/sysfs.c | 8
> arch/powerpc/perf/core-book3s.c | 29
>
> 5 files changed, 49 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/perf_event_server.h
> b/arch/powerpc/include/asm/perf_event_server.h
> index 14b8dc1..832450a 100644
> --- a/arch/powerpc/include/asm/perf_event_server.h
> +++ b/arch/powerpc/include/asm/perf_event_server.h
> @@ -22,6 +22,7 @@ struct mmcr_regs {
> unsigned long mmcr1;
> unsigned long mmcr2;
> unsigned long mmcra;
> + unsigned long mmcr3;
> };
> /*
> * This struct provides the constants and functions needed to
> @@ -75,6 +76,7 @@ struct power_pmu {
> #define PPMU_HAS_SIER 0x0040 /* Has SIER */
> #define PPMU_ARCH_207S 0x0080 /* PMC is architecture v2.07S */
> #define PPMU_NO_SIAR 0x0100 /* Do not use SIAR */
> +#define PPMU_ARCH_310S 0x0200 /* Has MMCR3, SIER2 and SIER3 */
We elsewhere have CPU_FTR_ARCH_31, so should this be PPMU_ARCH_31S to
be consistent.
>
> /*
> * Values for flags to get_alternatives()
> diff --git a/arch/powerpc/include/asm/processor.h
> b/arch/powerpc/include/asm/processor.h
> index 52a6783..a466e94 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -272,6 +272,10 @@ struct thread_struct {
> unsignedmmcr0;
>
> unsignedused_ebb;
> + unsigned long mmcr3;
> + unsigned long sier2;
> + unsigned long sier3;
> +
> #endif
> };
>
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 88e6c78..21a1b2d 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -876,7 +876,9 @@
> #define MMCR0_FCHV 0x0001UL /* freeze conditions in hypervisor mode
> */
> #define SPRN_MMCR1 798
> #define SPRN_MMCR2 785
> +#define SPRN_MMCR3 754
> #define SPRN_UMMCR2769
> +#define SPRN_UMMCR3738
> #define SPRN_MMCRA 0x312
> #define MMCRA_SDSYNC 0x8000UL /* SDAR synced with SIAR */
> #define MMCRA_SDAR_DCACHE_MISS 0x4000UL
> @@ -918,6 +920,10 @@
> #define SIER_SIHV0x100 /* Sampled MSR_HV */
> #define SIER_SIAR_VALID 0x040 /* SIAR contents valid */
> #define SIER_SDAR_VALID 0x020 /* SDAR contents valid */
> +#define SPRN_SIER2 752
> +#define SPRN_SIER3 753
> +#define SPRN_USIER2736
> +#define SPRN_USIER3737
> #define SPRN_SIAR 796
> #define SPRN_SDAR 797
> #define SPRN_TACR 888
> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> index 571b325..46b4ebc 100644
> --- a/arch/powerpc/kernel/sysfs.c
> +++ b/arch/powerpc/kernel/sysfs.c
> @@ -622,8 +622,10 @@ void ppc_enable_pmcs(void)
> SYSFS_PMCSETUP(pmc8, SPRN_PMC8);
>
> SYSFS_PMCSETUP(mmcra, SPRN_MMCRA);
> +SYSFS_PMCSETUP(mmcr3, SPRN_MMCR3);
>
> static DEVICE_ATTR(mmcra, 0600, show_mmcra, store_mmcra);
> +static DEVICE_ATTR(mmcr3, 0600, show_mmcr3, store_mmcr3);
> #endif /* HAS_PPC_PMC56 */
>
>
> @@ -886,6 +888,9 @@ static int register_cpu_online(unsigned int cpu)
> #ifdef CONFIG_PMU_SYSFS
> if (cpu_has_feature(CPU_FTR_MMCRA))
> device_create_file(s, _attr_mmcra);
> +
> + if (cpu_has_feature(CPU_FTR_ARCH_31))
> + device_create_file(s, _attr_mmcr3);
> #endif /* CONFIG_PMU_SYSFS */
>
> if (cpu_has_feature(CPU_FTR_PURR)) {
> @@ -980,6 +985,9 @@ static int unregister_cpu_online(unsigned int cpu)
> #ifdef CONFIG_PMU_SYSFS
> if (cpu_has_feature(CPU_FTR_MMCRA))
> device_remove_file(s, _attr_mmcra);
> +
> + if (cpu_has_feature(CPU_FTR_ARCH_31))
> + device_remove_file(s, _attr_mmcr3);
> #endif /* CONFIG_PMU_SYSFS */
>
> if
