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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <652f10660f09bd608b825233713f775a@kernel.org>
Date:   Thu, 17 Sep 2020 11:21:15 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     Leo Yan <leo.yan@...aro.org>
Cc:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Peter Zijlstra <peterz@...radead.org>,
        Will Deacon <will@...nel.org>,
        John Garry <john.garry@...wei.com>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Suleiman Souhlal <suleiman@...gle.com>,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCHv3] perf kvm: add kvm-stat for arm64

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.

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