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

From: Mark Rutland <mark.rutland@arm.com>
To: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: "linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Catalin Marinas <Catalin.Marinas@arm.com>,
	Will Deacon <will@kernel.org>
Subject: Re: [PATCH V7 5/6] arm64/perf: Add branch stack support in ARMV8 PMU
Date: Thu, 23 Feb 2023 13:47:34 +0000	[thread overview]
Message-ID: <Y/dudjSnIFtHzNRI@FVFF77S0Q05N.cambridge.arm.com> (raw)
In-Reply-To: <f952612b-db37-c21e-4dde-12202768cb44@arm.com>

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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel