From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 2F959C77B7A for ; Tue, 30 May 2023 09:43:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=diMrUoZUc54UcgwRZdTwma0a+3xGcqvJZHLeGRtn4D4=; b=ahTn4AqkWBrjMA TeDWdJciAgRl7lN7ANF9oIBa3ilodzviO5Ox9+G3ewiHvGhC9i979yPydSyzC9/2wuy1vBNbZChIY AMjcz+l265QnaVbz+8xmRcChS4BiI0J95K+Mv4Z2toNrSvtIVW5qq5idiKKO3zktmfCtKf6FfZ5e7 Zm5Z31HnjJYAOA/7HwKW77iKs6tOtirRkL6y9jNxpXpc4sO9JkrgcJPtHap4yFrC+BoI7HiHZA6bf mlVTQGPU/mM7VBIsA3fgWQjlqomMUeklWzBQylFbdTLOY3bGxZ8TDzJ07W94tAWVggwoeqorctaiG SWQ3z5p0trlJC4kJLkxg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q3vst-00DDqi-12; Tue, 30 May 2023 09:43:19 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q3vsp-00DDpd-20 for linux-arm-kernel@lists.infradead.org; Tue, 30 May 2023 09:43:17 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id A39F02F4; Tue, 30 May 2023 02:43:59 -0700 (PDT) Received: from [192.168.1.3] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1BBC63F663; Tue, 30 May 2023 02:43:12 -0700 (PDT) Message-ID: <431dc9d7-fab4-6935-51f9-3bd80c206271@arm.com> Date: Tue, 30 May 2023 10:43:15 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 Subject: Re: [PATCH 3/4] perf cs-etm: Track exception level Content-Language: en-US To: Leo Yan , Mike Leach Cc: coresight@lists.linaro.org, denik@chromium.org, Suzuki K Poulose , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Ian Rogers , Adrian Hunter , John Garry , Will Deacon , linux-arm-kernel@lists.infradead.org, linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org References: <20230524131958.2139331-1-james.clark@arm.com> <20230524131958.2139331-4-james.clark@arm.com> <20230528110516.GC886420@leoy-yangtze.lan> From: James Clark In-Reply-To: <20230528110516.GC886420@leoy-yangtze.lan> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230530_024315_753351_3201E78A X-CRM114-Status: GOOD ( 35.68 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 28/05/2023 12:05, Leo Yan wrote: > On Wed, May 24, 2023 at 02:19:57PM +0100, James Clark wrote: >> Currently we assume all trace belongs to the host machine so when >> the decoder should be looking at the guest kernel maps it can crash >> because it looks at the host ones instead. >> >> Avoid one scenario (guest kernel running at EL1) by assigning the >> default guest machine to this trace. For userspace trace it's still not >> possible to determine guest vs host, but the PIDs should help in this >> case. >> >> Signed-off-by: James Clark >> --- >> .../perf/util/cs-etm-decoder/cs-etm-decoder.c | 7 +- >> tools/perf/util/cs-etm.c | 64 ++++++++++++++----- >> tools/perf/util/cs-etm.h | 5 +- >> 3 files changed, 56 insertions(+), 20 deletions(-) >> >> diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c >> index 82a27ab90c8b..ac227cd03eb0 100644 >> --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c >> +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c >> @@ -573,12 +573,13 @@ cs_etm_decoder__set_tid(struct cs_etm_queue *etmq, >> break; >> } >> >> + if (cs_etm__etmq_set_tid_el(etmq, tid, trace_chan_id, >> + elem->context.exception_level)) >> + return OCSD_RESP_FATAL_SYS_ERR; >> + >> if (tid == -1) >> return OCSD_RESP_CONT; >> >> - if (cs_etm__etmq_set_tid(etmq, tid, trace_chan_id)) >> - return OCSD_RESP_FATAL_SYS_ERR; >> - >> /* >> * A timestamp is generated after a PE_CONTEXT element so make sure >> * to rely on that coming one. >> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c >> index a997fe79d458..b9ba19327f26 100644 >> --- a/tools/perf/util/cs-etm.c >> +++ b/tools/perf/util/cs-etm.c >> @@ -14,7 +14,6 @@ >> #include >> #include >> >> -#include >> #include >> >> #include "auxtrace.h" >> @@ -87,6 +86,8 @@ struct cs_etm_traceid_queue { >> union perf_event *event_buf; >> struct thread *thread; >> struct thread *prev_thread; >> + ocsd_ex_level prev_el; >> + ocsd_ex_level el; >> struct branch_stack *last_branch; >> struct branch_stack *last_branch_rb; >> struct cs_etm_packet *prev_packet; >> @@ -479,6 +480,7 @@ static int cs_etm__init_traceid_queue(struct cs_etm_queue *etmq, >> >> queue = &etmq->etm->queues.queue_array[etmq->queue_nr]; >> tidq->trace_chan_id = trace_chan_id; >> + tidq->el = tidq->prev_el = ocsd_EL_unknown; >> tidq->thread = machine__findnew_thread(&etm->session->machines.host, -1, >> queue->tid); >> tidq->prev_thread = machine__idle_thread(&etm->session->machines.host); >> @@ -618,6 +620,7 @@ static void cs_etm__packet_swap(struct cs_etm_auxtrace *etm, >> tmp = tidq->packet; >> tidq->packet = tidq->prev_packet; >> tidq->prev_packet = tmp; >> + tidq->prev_el = tidq->el; >> thread__put(tidq->prev_thread); >> tidq->prev_thread = thread__get(tidq->thread); >> } >> @@ -879,11 +882,34 @@ static bool cs_etm__evsel_is_auxtrace(struct perf_session *session, >> return evsel->core.attr.type == aux->pmu_type; >> } >> >> -static u8 cs_etm__cpu_mode(struct cs_etm_queue *etmq, u64 address) >> +static struct machine *cs_etm__get_machine(struct cs_etm_auxtrace *etm, >> + ocsd_ex_level el) >> { >> - struct machine *machine; >> + /* >> + * Not perfect, but assume anything in EL1 is the default guest, and >> + * everything else is the host. nHVE and pKVM may not work with this > > s/nHVE/nVHE ? > > And it's good to say "if any virtulisation (e.g. pKVM) based on nVHE may > not work with this ....". > >> + * assumption. And distinguishing between guest and host userspaces >> + * isn't currently supported either. Neither is multiple guest support. >> + * All this does is reduce the likeliness of decode errors where we look >> + * into the host kernel maps when it should have been the guest maps. >> + */ >> + switch (el) { >> + case ocsd_EL1: >> + return machines__find_guest(&etm->session->machines, >> + DEFAULT_GUEST_KERNEL_ID); > > I share the same concern with Mike that this will break perf for any > Armv8 CPUs with nVHE (like Cortex-CA53, etc). > I agree, I replied to Mike's comment with some thoughts. > You could see CoreSight has trace data "elem->context.vmid", when a > guest kernel runs in EL1, can we use "elem->context.vmid" as a guest > kernel ID and finally retrieve the corresponding machine pointer? > Do you mean something like this? static struct machine *cs_etm__get_machine(struct cs_etm_auxtrace *etm, ocsd_ex_level el) { u64 pid_fmt; cs_etm__get_pid_fmt(trace_chan_id, &pid_fmt); if (pid_fmt == BIT(ETM_OPT_CTXTID)) return return &etm->session->machines.host; switch (el) { case ocsd_EL1: return machines__find_guest(&etm->session->machines, DEFAULT_GUEST_KERNEL_ID); case ocsd_EL3: case ocsd_EL2: case ocsd_EL0: case ocsd_EL_unknown: default: return &etm->session->machines.host; } } I think this might work, maybe it's not as bad as I thought and we can make it work out of the box. > Thanks, > Leo > >> + case ocsd_EL3: >> + case ocsd_EL2: >> + case ocsd_EL0: >> + case ocsd_EL_unknown: >> + default: >> + return &etm->session->machines.host; >> + } >> +} >> >> - machine = &etmq->etm->session->machines.host; >> +static u8 cs_etm__cpu_mode(struct cs_etm_queue *etmq, u64 address, >> + ocsd_ex_level el) >> +{ >> + struct machine *machine = cs_etm__get_machine(etmq->etm, el); >> >> if (address >= machine__kernel_start(machine)) { >> if (machine__is_host(machine)) >> @@ -893,10 +919,14 @@ static u8 cs_etm__cpu_mode(struct cs_etm_queue *etmq, u64 address) >> } else { >> if (machine__is_host(machine)) >> return PERF_RECORD_MISC_USER; >> - else if (perf_guest) >> + else { >> + /* >> + * Can't really happen at the moment because >> + * cs_etm__get_machine() will always return >> + * machines.host for any non EL1 trace. >> + */ >> return PERF_RECORD_MISC_GUEST_USER; >> - else >> - return PERF_RECORD_MISC_HYPERVISOR; >> + } >> } >> } >> >> @@ -913,11 +943,12 @@ static u32 cs_etm__mem_access(struct cs_etm_queue *etmq, u8 trace_chan_id, >> if (!etmq) >> return 0; >> >> - cpumode = cs_etm__cpu_mode(etmq, address); >> tidq = cs_etm__etmq_get_traceid_queue(etmq, trace_chan_id); >> if (!tidq) >> return 0; >> >> + cpumode = cs_etm__cpu_mode(etmq, address, tidq->el); >> + >> if (!thread__find_map(tidq->thread, cpumode, address, &al)) >> return 0; >> >> @@ -1296,9 +1327,10 @@ cs_etm__get_trace(struct cs_etm_queue *etmq) >> } >> >> static void cs_etm__set_thread(struct cs_etm_auxtrace *etm, >> - struct cs_etm_traceid_queue *tidq, pid_t tid) >> + struct cs_etm_traceid_queue *tidq, pid_t tid, >> + ocsd_ex_level el) >> { >> - struct machine *machine = &etm->session->machines.host; >> + struct machine *machine = cs_etm__get_machine(etm, el); >> >> if (tid != -1) { >> thread__zput(tidq->thread); >> @@ -1308,10 +1340,12 @@ static void cs_etm__set_thread(struct cs_etm_auxtrace *etm, >> /* Couldn't find a known thread */ >> if (!tidq->thread) >> tidq->thread = machine__idle_thread(machine); >> + >> + tidq->el = el; >> } >> >> -int cs_etm__etmq_set_tid(struct cs_etm_queue *etmq, >> - pid_t tid, u8 trace_chan_id) >> +int cs_etm__etmq_set_tid_el(struct cs_etm_queue *etmq, pid_t tid, u8 trace_chan_id, >> + ocsd_ex_level el) >> { >> struct cs_etm_traceid_queue *tidq; >> >> @@ -1319,7 +1353,7 @@ int cs_etm__etmq_set_tid(struct cs_etm_queue *etmq, >> if (!tidq) >> return -EINVAL; >> >> - cs_etm__set_thread(etmq->etm, tidq, tid); >> + cs_etm__set_thread(etmq->etm, tidq, tid, el); >> return 0; >> } >> >> @@ -1389,7 +1423,7 @@ static int cs_etm__synth_instruction_sample(struct cs_etm_queue *etmq, >> struct perf_sample sample = {.ip = 0,}; >> >> event->sample.header.type = PERF_RECORD_SAMPLE; >> - event->sample.header.misc = cs_etm__cpu_mode(etmq, addr); >> + event->sample.header.misc = cs_etm__cpu_mode(etmq, addr, tidq->el); >> event->sample.header.size = sizeof(struct perf_event_header); >> >> /* Set time field based on etm auxtrace config. */ >> @@ -1448,7 +1482,7 @@ static int cs_etm__synth_branch_sample(struct cs_etm_queue *etmq, >> ip = cs_etm__last_executed_instr(tidq->prev_packet); >> >> event->sample.header.type = PERF_RECORD_SAMPLE; >> - event->sample.header.misc = cs_etm__cpu_mode(etmq, ip); >> + event->sample.header.misc = cs_etm__cpu_mode(etmq, ip, tidq->prev_el); >> event->sample.header.size = sizeof(struct perf_event_header); >> >> /* Set time field based on etm auxtrace config. */ >> diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h >> index 70cac0375b34..88e9b25a8a9f 100644 >> --- a/tools/perf/util/cs-etm.h >> +++ b/tools/perf/util/cs-etm.h >> @@ -232,10 +232,11 @@ int cs_etm__process_auxtrace_info(union perf_event *event, >> struct perf_event_attr *cs_etm_get_default_config(struct perf_pmu *pmu); >> >> #ifdef HAVE_CSTRACE_SUPPORT >> +#include >> int cs_etm__get_cpu(u8 trace_chan_id, int *cpu); >> int cs_etm__get_pid_fmt(u8 trace_chan_id, u64 *pid_fmt); >> -int cs_etm__etmq_set_tid(struct cs_etm_queue *etmq, >> - pid_t tid, u8 trace_chan_id); >> +int cs_etm__etmq_set_tid_el(struct cs_etm_queue *etmq, pid_t tid, u8 trace_chan_id, >> + ocsd_ex_level el); >> bool cs_etm__etmq_is_timeless(struct cs_etm_queue *etmq); >> void cs_etm__etmq_set_traceid_queue_timestamp(struct cs_etm_queue *etmq, >> u8 trace_chan_id); >> -- >> 2.34.1 >> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel