Re: [PATCH V11 06/10] arm64/perf: Enable branch stack events via FEAT_BRBE

From: Anshuman Khandual
Date: Fri Jun 09 2023 - 00:30:30 EST




On 6/5/23 19:13, Mark Rutland wrote:
>> +static u64 branch_type_to_brbcr(int branch_type)
>> +{
>> + u64 brbcr = BRBCR_EL1_DEFAULT_TS;
>> +
>> + /*
>> + * BRBE need not be paused on PMU interrupt while tracing only
>> + * the user space, bcause it will automatically be inside the
>> + * prohibited region. But even after PMU overflow occurs, the
>> + * interrupt could still take much more cycles, before it can
>> + * be taken and by that time BRBE will have been overwritten.
>> + * Let's enable pause on PMU interrupt mechanism even for user
>> + * only traces.
>> + */
>> + brbcr |= BRBCR_EL1_FZP;
> I think this is trying to say that we *should* use FZP when sampling the
> kernel (due to IRQ latency), and *can* safely use it when sampling userspace,
> so it would be good to explain it that way around.

Agreed, following updated comment explains why we should enable FZP
when sampling kernel, otherwise BRBE will capture unwanted records.
It also explains why we should enable FZP even when sampling user
space due to IRQ latency.

/*
* BRBE should be paused on PMU interrupt while tracing kernel
* space to stop capturing further branch records. Otherwise
* interrupt handler branch records might get into the samples
* which is not desired.
*
* BRBE need not be paused on PMU interrupt while tracing only
* the user space, because it will automatically be inside the
* prohibited region. But even after PMU overflow occurs, the
* interrupt could still take much more cycles, before it can
* be taken and by that time BRBE will have been overwritten.
* Hence enable pause on PMU interrupt mechanism even for user
* only traces as well.
*/
brbcr |= BRBCR_EL1_FZP;

>
> It's a bit unfortunate, because where this matters we'll always be losing some
> branches either way, but I guess we don't have much say in the matter.