[PATCH] drivers/perf: riscv: Disable PERF_SAMPLE_BRANCH_* while not supported

[Pu Lehui]

On 2024/3/8 1:52, Conor Dooley wrote:
> On Tue, Mar 05, 2024 at 03:52:23PM +0000, Pu Lehui wrote:
>> From: Pu Lehui <pulehui at huawei.com>
>>
>> RISC-V perf does not yet support branch sampling. Two riscv bpf
>> testcases get_branch_snapshot and perf_branches/perf_branches_hw failed
>> due to not disabling such sampling.
>>
>> Signed-off-by: Pu Lehui <pulehui at huawei.com>
> 
> This seems worthy of a fixes tag. For what commit I do not know, but I
> figure it is a problem currently in mainline and a fix should be
> backported?

Thanks for review. We need to ban this type of event, otherwise we will 
get unpredictable results. It should be with a fixes tag.

> 
>> ---
>>   drivers/perf/riscv_pmu.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/perf/riscv_pmu.c b/drivers/perf/riscv_pmu.c
>> index c78a6fd6c57f..bc42cb95a97c 100644
>> --- a/drivers/perf/riscv_pmu.c
>> +++ b/drivers/perf/riscv_pmu.c
>> @@ -313,6 +313,10 @@ static int riscv_pmu_event_init(struct perf_event *event)
>>   	u64 event_config = 0;
>>   	uint64_t cmask;
>>   
>> +	/* does not support taken branch sampling */
> 
> Skimming patchwork, I found this comment confusing.
> Isolated from the commit message, which it will be once committed, it
> was not immediately clear that you were referring to the driver. I think
> it is just matter of making this a complete sentence that starts by
> mentioning /what/ does not support this feature. Is it the driver? Does
> the spec not allow it? etc

I actually just found out that the current implementation of riscv perf 
driver doesn't support it. After reading the relevant spec, I did not 
find any description related to branch record. Maybe I may have missed 
something or it may be supported in the future, but the current driver 
does not support it, so I think it is more appropriate to limit this 
comment to the driver implementation. How about "riscv perf driver does 
not support branch stack sampling"?

> 
> Cheers,
> Conor.
> 
>> +	if (has_branch_stack(event))
>> +		return -EOPNOTSUPP;
>> +
>>   	hwc->flags = 0;
>>   	mapped_event = rvpmu->event_map(event, &event_config);
>>   	if (mapped_event < 0) {
>> -- 
>> 2.34.1
>>