Re: [PATCH V3 4/7] driver/perf/arm_pmu_platform: Add support for BRBE attributes detection

From: Anshuman Khandual
Date: Wed Oct 12 2022 - 03:50:28 EST




On 10/11/22 14:51, Anshuman Khandual wrote:
>
> On 10/10/22 19:47, James Clark wrote:
>>
>> On 06/10/2022 14:37, James Clark wrote:
>>>
>>> On 29/09/2022 08:58, Anshuman Khandual wrote:
>>>> This adds arm pmu infrastrure to probe BRBE implementation's attributes via
>>>> driver exported callbacks later. The actual BRBE feature detection will be
>>>> added by the driver itself.
>>>>
>>>> CPU specific BRBE entries, cycle count, format support gets detected during
>>>> PMU init. This information gets saved in per-cpu struct pmu_hw_events which
>>>> later helps in operating BRBE during a perf event context.
>>>>
>>>> Cc: Will Deacon <will@xxxxxxxxxx>
>>>> Cc: Mark Rutland <mark.rutland@xxxxxxx>
>>>> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
>>>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>
>>>> ---
>>>> drivers/perf/arm_pmu_platform.c | 34 +++++++++++++++++++++++++++++++++
>>>> 1 file changed, 34 insertions(+)
>>>>
>>>> diff --git a/drivers/perf/arm_pmu_platform.c b/drivers/perf/arm_pmu_platform.c
>>>> index 933b96e243b8..acdc445081aa 100644
>>>> --- a/drivers/perf/arm_pmu_platform.c
>>>> +++ b/drivers/perf/arm_pmu_platform.c
>>>> @@ -172,6 +172,36 @@ static int armpmu_request_irqs(struct arm_pmu *armpmu)
>>>> return err;
>>>> }
>>>>
>>>> +static void arm_brbe_probe_cpu(void *info)
>>>> +{
>>>> + struct pmu_hw_events *hw_events;
>>>> + struct arm_pmu *armpmu = info;
>>>> +
>>>> + /*
>>>> + * Return from here, if BRBE driver has not been
>>>> + * implemented for this PMU. This helps prevent
>>>> + * kernel crash later when brbe_probe() will be
>>>> + * called on the PMU.
>>>> + */
>>>> + if (!armpmu->brbe_probe)
>>>> + return;
>>>> +
>>>> + hw_events = per_cpu_ptr(armpmu->hw_events, smp_processor_id());
>>>> + armpmu->brbe_probe(hw_events);
>>>> +}
>>>> +
>>>> +static int armpmu_request_brbe(struct arm_pmu *armpmu)
>>>> +{
>>>> + int cpu, err = 0;
>>>> +
>>>> + for_each_cpu(cpu, &armpmu->supported_cpus) {
>>>> + err = smp_call_function_single(cpu, arm_brbe_probe_cpu, armpmu, 1);
>>> Hi Anshuman,
>>>
>>> I have LOCKDEP on and the patchset applied to perf/core (82aad7ff7) on
>>> git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git and I get
>> Can you confirm if this is currently the correct place to apply this to?
> This series applied on v6.0-rc5 after the perf ABI changes, both in kernel
> and in user space tools.
>
>> I'm only getting 0 length branch stacks now. Seems like it could be
>> something to do with the layout of perf samples because I know that was
>> done in separate commits:
> Right, might be.
>
>> sudo ./perf record -j any_call -- ls
>> ./perf report -D | grep "branch stack"
>> ... branch stack: nr:0
>> ... branch stack: nr:0
>> ... branch stack: nr:0
>> ... branch stack: nr:0
> I am planning to respin the series on 6.1-rc1 next week which should solve
> these multiple moving parts problem

There are some recent changes which require PMU driver to set data.sample_flags
indicating what kind of records are being filled in there. Here are the commits

a9a931e2666878343 ("perf: Use sample_flags for branch stack")
3aac580d5cc3001ca ("perf: Add sample_flags to indicate the PMU-filled sample data")

Following fix solves the problem for BRBE driver.

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 98e9a615d3cb..85a3aaefc0fb 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -877,6 +877,7 @@ static irqreturn_t armv8pmu_handle_irq(struct arm_pmu *cpu_pmu)
if (has_branch_stack(event)) {
cpu_pmu->brbe_read(cpuc, event);
data.br_stack = &cpuc->branches->brbe_stack;
+ data.sample_flags |= PERF_SAMPLE_BRANCH_STACK;
cpu_pmu->brbe_reset(cpuc);
}