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

[Mark Rutland]

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.