Re: [PATCH 1/3] powerpc/perf: Fix branch_filter support for multiple filters in powerpc

[kajoljain <kjain@linux.ibm.com>]

On 9/16/22 17:02, Disha Goel wrote:
> 
> On 9/13/22 5:25 PM, Athira Rajeev wrote:
>> For PERF_SAMPLE_BRANCH_STACK sample type, different branch_sample_type
>> ie branch filters are supported. The branch filters are requested via
>> event attribute "branch_sample_type". Multiple branch filters can be
>> passed in event attribute.
>>
>> Example:
>> perf record -b -o- -B --branch-filter any,ind_call true
>>
>> Powerpc does not support having multiple branch filters at
>> sametime. In powerpc, branch filters for branch stack sampling
>> is set via MMCRA IFM bits [32:33]. But currently when requesting
>> for multiple filter types, the "perf record" command does not
>> report any error.
>>
>> Example:
>> perf record -b -o- -B --branch-filter any,save_type true
>> perf record -b -o- -B --branch-filter any,ind_call true
>>
>> The "bhrb_filter_map" function in PMU driver code does the
>> validity check for supported branch filters. But this check
>> is done for single filter. Hence "perf record" will proceed
>> here without reporting any error.
>>
>> Fix power_pmu_event_init to return EOPNOTSUPP when multiple
>> branch filters are requested in the event attr.
>>
>> After the fix:
>> perf record --branch-filter any,ind_call -- ls
>> Error:
>> cycles: PMU Hardware doesn't support sampling/overflow-interrupts.
>> Try 'perf stat'
>>
>> Reported-by: Disha Goel <disgoel@linux.vnet.ibm.com>
>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>> Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>
> 
> Tested-by: Disha Goel <disgoel@linux.vnet.ibm.com>

Patch looks good to me.

Reviewed-By: Kajol Jain <kjain@linux.ibm.com>

Thanks,
Kajol Jain

> 
>> ---
>>  arch/powerpc/perf/core-book3s.c | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
>> index 13919eb96931..2c2d235cb8d8 100644
>> --- a/arch/powerpc/perf/core-book3s.c
>> +++ b/arch/powerpc/perf/core-book3s.c
>> @@ -2131,6 +2131,21 @@ static int power_pmu_event_init(struct perf_event *event)
>>  	if (has_branch_stack(event)) {
>>  		u64 bhrb_filter = -1;
>>
>> +		/*
>> +		 * powerpc does not support having multiple branch filters
>> +		 * at sametime. Branch filters are set via MMCRA IFM[32:33]
>> +		 * bits for power8 and above. Return EOPNOTSUPP when multiple
>> +		 * branch filters are requested in the event attr.
>> +		 *
>> +		 * When opening event via perf_event_open, branch_sample_type
>> +		 * gets adjusted in perf_copy_attr function. Kernel will
>> +		 * automatically adjust the branch_sample_type based on the
>> +		 * event modifier settings to include PERF_SAMPLE_BRANCH_PLM_ALL.
>> +		 * Hence drop the check for PERF_SAMPLE_BRANCH_PLM_ALL.
>> +		 */
>> +		if (hweight64(event->attr.branch_sample_type & ~PERF_SAMPLE_BRANCH_PLM_ALL) > 1)
>> +			return -EOPNOTSUPP;
>> +
>>  		if (ppmu->bhrb_filter_map)
>>  			bhrb_filter = ppmu->bhrb_filter_map(
>>  					event->attr.branch_sample_type);