On 2024/3/9 17:06, Atish Patra wrote:
On Fri, Mar 8, 2024 at 10:48 PM Pu Lehui <pulehui@xxxxxxxxxxxxxxx> wrote:
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@xxxxxxxxxx>
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@xxxxxxxxxx>
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
Yes. There was no specification at the time this driver was upstreamed.
The spec for branch record being is on its way for ratification and patches
will be available soon.
https://github.com/riscv/riscv-control-transfer-records
The changes look good to me for now. I will be happy to send a RB tag once you
add the Fixes tag.
Thanks for your addition, looking forward to the upcoming
implementation. Will send new version soon.
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
