Subject: Re: [v3 04/15] powerpc/perf: Add support for ISA3.1 PMU SPRs

From: Michael Ellerman <[email-masked]>
Date: Wed, 22 Jul 2020 22:03:19 +1000

Jordan Niethe <[email-masked]> writes:
> On Sat, Jul 18, 2020 at 1:02 AM Athira Rajeev <[email-masked]> wrote:
>> From: Madhavan Srinivasan <[email-masked]>
>>
>> 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          0x00000040 /* Has SIER */
>>  #define PPMU_ARCH_207S         0x00000080 /* PMC is architecture v2.07S */
>>  #define PPMU_NO_SIAR           0x00000100 /* Do not use SIAR */
>> +#define PPMU_ARCH_310S         0x00000200 /* 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