Re: [PATCHv3] perf kvm: add kvm-stat for arm64

From: Marc Zyngier
Date: Thu Sep 17 2020 - 06:21:39 EST

On 2020-09-17 11:12, Leo Yan wrote:
Add Marc at this time, sorry for spamming.

On Thu, Sep 17, 2020 at 06:09:50PM +0800, Leo Yan wrote:
[ + Marc ]

On Thu, Sep 17, 2020 at 09:36:45AM +0900, Sergey Senozhatsky wrote:


> diff --git a/tools/perf/arch/arm64/util/kvm-stat.c b/tools/perf/arch/arm64/util/kvm-stat.c
> new file mode 100644
> index 000000000000..32e58576f186
> --- /dev/null
> +++ b/tools/perf/arch/arm64/util/kvm-stat.c
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <errno.h>
> +#include <memory.h>
> +#include "../../util/evsel.h"
> +#include "../../util/kvm-stat.h"
> +#include "arm64_exception_types.h"
> +#include "debug.h"
> +
> +define_exit_reasons_table(arm64_exit_reasons, kvm_arm_exception_type);
> +define_exit_reasons_table(arm64_trap_exit_reasons, kvm_arm_exception_class);
> +
> +const char *kvm_trap_exit_reason = "esr_ec";
> +const char *vcpu_id_str = "id";

On Arm64, ftrace tracepoint "kvm_entry" doesn't contain the field "id"
or field "vcpu_id", thus it always reads out the "id" is 0 and it is
recorded into Perf's structure vcpu_event_record::vcpu_id and assigned
to perf thread's private data "thread::private".

With current code, it will not mess up different vcpus' samples because
now the samples are analyzed based on thread context, but since all
threads' "vcpu_id" is zero, thus all samples are accounted for
"vcpu_id=0" and cannot print out correct result with option "--vcpu":

$ perf kvm stat report --vcpu 4

Analyze events for all VMs, VCPU 4:

VM-EXIT Samples Samples% Time% Min Time Max Time Avg time

Total Samples:0, Total events handled time:0.00us.

This is an issue I observed, if we want to support option "--vcpu",
seems we need to change ftrace event for "kvm_entry", but this will
break ABI.

Essentially, this issue is caused by different archs using different
format for ftrace event "kvm_entry", on x86 it contains feild
"vcpu_id" but arm64 only just records "vcpu_pc".

@Marc, @Will, do you have any suggestion for this? Do you think it's
feasible to add a new field "vcpu_id" into the tracepoint "kvm_entry"
for Arm64's version?

The question really is: how will you handle the ABI breackage?
I don't see a good solution for it, apart from having a *separate*
tracepoint that collects all the information you need. And even that is
really ugly.

Jazz is not dead. It just smells funny...