Re: [PATCH V7 5/6] arm64/perf: Add branch stack support in ARMV8 PMU

Mark Rutland
2023-02-23T13:57:07Z

On Mon, Feb 13, 2023 at 01:53:56PM +0530, Anshuman Khandual wrote:
>
>
> On 2/9/23 01:06, Mark Rutland wrote:
> > On Fri, Jan 13, 2023 at 10:41:51AM +0530, Anshuman Khandual wrote:
> >>
> >>
> >> On 1/12/23 19:59, Mark Rutland wrote:
> >>> On Thu, Jan 05, 2023 at 08:40:38AM +0530, Anshuman Khandual wrote:
> >>>> @@ -878,6 +890,13 @@ static irqreturn_t armv8pmu_handle_irq(struct arm_pmu *cpu_pmu)
> >>>> if (!armpmu_event_set_period(event))
> >>>> continue;
> >>>>
> >>>> + if (has_branch_stack(event)) {
> >>>> + WARN_ON(!cpuc->branches);
> >>>> + armv8pmu_branch_read(cpuc, event);
> >>>> + data.br_stack = &cpuc->branches->branch_stack;
> >>>> + data.sample_flags |= PERF_SAMPLE_BRANCH_STACK;
> >>>> + }
> >>>
> >>> How do we ensure the data we're getting isn't changed under our feet? Is BRBE
> >>> disabled at this point?
> >>
> >> Right, BRBE is paused after a PMU IRQ. We also ensure the buffer is disabled for
> >> all exception levels, i.e removing BRBCR_EL1_E0BRE/E1BRE from the configuration,
> >> before initiating the actual read, which eventually populates the data.br_stack.
> >
> > Ok; just to confirm, what exactly is the condition that enforces that BRBE is
> > disabled? Is that *while* there's an overflow asserted, or does something else
> > get set at the instant the overflow occurs?
>
> - BRBE can be disabled completely via BRBCR_EL1_E0BRE/E1BRE irrespective of PMU interrupt
> - But with PMU interrupt, it just pauses if BRBCR_EL1_FZP is enabled

IIUC the distinction between "disabled completely" and "just pauses" doesn't
really matter to us, and a pause is sufficient for use to be able to read and
manipulate the records.

I also note that we always set BRBCR_EL1.FZP.

Am I missing something?

Thanks,
Mark.