Re: [PATCH v6 2/3] perf/arm64: Add BRBE support for bpf_get_branch_snapshot()
| Newsgroups | org.kernel.vger.linux-perf-users,org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Missing `isb()` context synchronization barrier after writing to the `PMCR_EL0` system register in `brbe_snapshot_branch_stack`. -- --- Patch [2]: [PATCH v6 2/3] perf/arm64: Add BRBE support for bpf_get_branch_snapshot() --- commit a7708fa08fe97675c17bb6e7a2098acd91d49bfb Author: Puranjay Mohan <[email protected]> perf/arm64: Add BRBE support for bpf_get_branch_snapshot() This commit implements the perf_snapshot_branch_stack static call for BRBE, allowing bpf_get_branch_snapshot() to capture hardware branch records. It manages BRBE state by pausing it during the snapshot, freezing the counters, and handling buffer invalidation to avoid data corruption. > diff --git a/drivers/perf/arm_brbe.c b/drivers/perf/arm_brbe.c > index ba554e0c846c4..9e7e69c43b17d 100644 > --- a/drivers/perf/arm_brbe.c > +++ b/drivers/perf/arm_brbe.c [ ... ] > +int brbe_snapshot_branch_stack(struct perf_branch_entry *entries, unsigned int cnt) > +{ [ ... ] > + if (paused_here) { > + /* Branches were missed, so discard rather than leave a hole. */ > + brbe_invalidate(); > + > + /* Unpause first; a paused BRBE cannot freeze on overflow. */ > + write_sysreg_s(brbfcr, SYS_BRBFCR_EL1); > + isb(); > + write_pmcr(pmcr); [Severity: Medium] Is a context synchronization barrier missing after this write to PMCR_EL0? Architecture invariants for ARM64 usually require writes to control-plane system registers to be immediately followed by an isb() to ensure the CPU pipeline observes the new configuration. Could the lack of an isb() here allow the CPU to continue operating under a stale PMU configuration for several cycles, potentially missing performance counter and BRBE branch events immediately following the unpause? > + } > + > + local_daif_restore(flags); > + > + return nr_copied; > +} -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2