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

From: Leo Yan <leo.yan@linaro.org>
To: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Marc Zyngier <maz@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Will Deacon <will@kernel.org>, John Garry <john.garry@huawei.com>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Suleiman Souhlal <suleiman@google.com>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCHv3] perf kvm: add kvm-stat for arm64
Date: Thu, 17 Sep 2020 18:12:19 +0800	[thread overview]
Message-ID: <20200917101219.GD12548@leoy-ThinkPad-X240s> (raw)
In-Reply-To: <20200917100950.GC12548@leoy-ThinkPad-X240s>

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?
> 
> Thanks,
> Leo