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 46BAFC77B7A for ; Tue, 30 May 2023 09:13:29 +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=8qpcTSaE3ae7MzJ5TSe7ldqNR0aNJ8Bnl1otI0OhjOU=; b=evBGUNvj44UuRO 8pH9BPkVc5nfncw+AFeU+Mpw2Lxm56dJ+eD9aBJVKfArbmOdjdYB4LdJC91Dr+drygX10fdR1zfMx +DYohlDsGhAwSftUMmKxWobquMHFvl3ljagKr9lLA6GNJOlo17PlcityJXTyHMgIRMPYAUDVSz20l Vc4QRYjATxF9cpIiIOelM3zFh+h22Ql/SP/Ud63BiwCqRmnyL+vGk5h7iecMDJNvn6DsaGHeAlSbG uGMRSO7FujuLOsc5qHAcO47939LJSB3OxUaeLCM1D3iIBihFZNinmcJgICI0cnHm9nRru01gXs6U4 46g9meNhFJ4+XCYs3oPg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q3vPV-00D6ea-0e; Tue, 30 May 2023 09:12:57 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q3vPS-00D6cG-02 for linux-arm-kernel@lists.infradead.org; Tue, 30 May 2023 09:12:56 +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 2FE412F4; Tue, 30 May 2023 02:13:32 -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 9B1D73F663; Tue, 30 May 2023 02:12:44 -0700 (PDT) Message-ID: <612885c1-b58f-5bce-59dc-bf84b057048d@arm.com> Date: Tue, 30 May 2023 10:12:47 +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 4/4] perf cs-etm: Add exception level consistency check To: Mike Leach , Leo Yan Cc: coresight@lists.linaro.org, denik@chromium.org, Suzuki K Poulose , Leo Yan , 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-5-james.clark@arm.com> Content-Language: en-US From: James Clark In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230530_021254_169524_A10C4AA9 X-CRM114-Status: GOOD ( 27.91 ) 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 25/05/2023 12:39, Mike Leach wrote: > Hi James, > > My concern here is that for etmv3 trace, OpenCSD will only provide > memory spaces as either secure or non-secure, The ETMv3 does not > trace, and hence OpenCSD cannot provide the different ELs. > The memory callback will be either OCSD_MEM_SPACE_S or OCSD_MEM_SPACE_N. > As long as none of the bits are set for EL1-EL3 then no validation will be done so it should be fine. But I will try to test ETMv3. > Can this patch - and the set handle this. (assuming perf supports our > ETMv3 coresight kernel driver) For the whole set, the tracked EL will stay as ocsd_EL_unknown and cs_etm__get_machine() will return host so the behavior will be unchanged from before. This is assuming OpenCSD will set the EL to unknown in elem->context.exception_level in this case. > > Regards > > Mike > > On Wed, 24 May 2023 at 14:20, James Clark wrote: >> >> Assert that our own tracking of the exception level matches what >> OpenCSD provides. OpenCSD doesn't distinguish between EL0 and EL1 in the >> memory access callback so the extra tracking was required. But a rough >> assert can still be done. >> >> Signed-off-by: James Clark >> --- >> .../perf/util/cs-etm-decoder/cs-etm-decoder.c | 6 +-- >> .../perf/util/cs-etm-decoder/cs-etm-decoder.h | 4 +- >> tools/perf/util/cs-etm.c | 37 +++++++++++++------ >> 3 files changed, 32 insertions(+), 15 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 ac227cd03eb0..50b3c248d1e5 100644 >> --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c >> +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c >> @@ -52,15 +52,15 @@ struct cs_etm_decoder { >> static u32 >> cs_etm_decoder__mem_access(const void *context, >> const ocsd_vaddr_t address, >> - const ocsd_mem_space_acc_t mem_space __maybe_unused, >> + const ocsd_mem_space_acc_t mem_space, >> const u8 trace_chan_id, >> const u32 req_size, >> u8 *buffer) >> { >> struct cs_etm_decoder *decoder = (struct cs_etm_decoder *) context; >> >> - return decoder->mem_access(decoder->data, trace_chan_id, >> - address, req_size, buffer); >> + return decoder->mem_access(decoder->data, trace_chan_id, address, >> + req_size, buffer, mem_space); >> } >> >> int cs_etm_decoder__add_mem_access_cb(struct cs_etm_decoder *decoder, >> diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h >> index 21d403f55d96..272c2efe78ee 100644 >> --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h >> +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.h >> @@ -11,6 +11,7 @@ >> #define INCLUDE__CS_ETM_DECODER_H__ >> >> #include >> +#include >> #include >> >> struct cs_etm_decoder; >> @@ -19,7 +20,8 @@ struct cs_etm_packet_queue; >> >> struct cs_etm_queue; >> >> -typedef u32 (*cs_etm_mem_cb_type)(struct cs_etm_queue *, u8, u64, size_t, u8 *); >> +typedef u32 (*cs_etm_mem_cb_type)(struct cs_etm_queue *, u8, u64, size_t, u8 *, >> + const ocsd_mem_space_acc_t); >> >> struct cs_etmv3_trace_params { >> u32 reg_ctrl; >> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c >> index b9ba19327f26..ccf34ed8ddf2 100644 >> --- a/tools/perf/util/cs-etm.c >> +++ b/tools/perf/util/cs-etm.c >> @@ -931,7 +931,8 @@ static u8 cs_etm__cpu_mode(struct cs_etm_queue *etmq, u64 address, >> } >> >> static u32 cs_etm__mem_access(struct cs_etm_queue *etmq, u8 trace_chan_id, >> - u64 address, size_t size, u8 *buffer) >> + u64 address, size_t size, u8 *buffer, >> + const ocsd_mem_space_acc_t mem_space) >> { >> u8 cpumode; >> u64 offset; >> @@ -947,6 +948,20 @@ static u32 cs_etm__mem_access(struct cs_etm_queue *etmq, u8 trace_chan_id, >> if (!tidq) >> return 0; >> >> + /* >> + * We've already tracked EL along side the PID in cs_etm__set_thread() >> + * so double check that it matches what OpenCSD thinks as well. It >> + * doesn't distinguish between EL0 and EL1 for this mem access callback >> + * so we had to do the extra tracking. >> + */ >> + if (mem_space & OCSD_MEM_SPACE_EL1N) { >> + /* Includes both non secure EL1 and EL0 */ >> + assert(tidq->el == ocsd_EL1 || tidq->el == ocsd_EL0); >> + } else if (mem_space & OCSD_MEM_SPACE_EL2) >> + assert(tidq->el == ocsd_EL2); >> + else if (mem_space & OCSD_MEM_SPACE_EL3) >> + assert(tidq->el == ocsd_EL3); >> + >> cpumode = cs_etm__cpu_mode(etmq, address, tidq->el); >> >> if (!thread__find_map(tidq->thread, cpumode, address, &al)) >> @@ -1195,8 +1210,8 @@ static inline int cs_etm__t32_instr_size(struct cs_etm_queue *etmq, >> { >> u8 instrBytes[2]; >> >> - cs_etm__mem_access(etmq, trace_chan_id, addr, >> - ARRAY_SIZE(instrBytes), instrBytes); >> + cs_etm__mem_access(etmq, trace_chan_id, addr, ARRAY_SIZE(instrBytes), >> + instrBytes, 0); >> /* >> * T32 instruction size is indicated by bits[15:11] of the first >> * 16-bit word of the instruction: 0b11101, 0b11110 and 0b11111 >> @@ -1387,8 +1402,8 @@ static void cs_etm__copy_insn(struct cs_etm_queue *etmq, >> else >> sample->insn_len = 4; >> >> - cs_etm__mem_access(etmq, trace_chan_id, sample->ip, >> - sample->insn_len, (void *)sample->insn); >> + cs_etm__mem_access(etmq, trace_chan_id, sample->ip, sample->insn_len, >> + (void *)sample->insn, 0); >> } >> >> u64 cs_etm__convert_sample_time(struct cs_etm_queue *etmq, u64 cs_timestamp) >> @@ -1940,8 +1955,8 @@ static bool cs_etm__is_svc_instr(struct cs_etm_queue *etmq, u8 trace_chan_id, >> * so below only read 2 bytes as instruction size for T32. >> */ >> addr = end_addr - 2; >> - cs_etm__mem_access(etmq, trace_chan_id, addr, >> - sizeof(instr16), (u8 *)&instr16); >> + cs_etm__mem_access(etmq, trace_chan_id, addr, sizeof(instr16), >> + (u8 *)&instr16, 0); >> if ((instr16 & 0xFF00) == 0xDF00) >> return true; >> >> @@ -1956,8 +1971,8 @@ static bool cs_etm__is_svc_instr(struct cs_etm_queue *etmq, u8 trace_chan_id, >> * +---------+---------+-------------------------+ >> */ >> addr = end_addr - 4; >> - cs_etm__mem_access(etmq, trace_chan_id, addr, >> - sizeof(instr32), (u8 *)&instr32); >> + cs_etm__mem_access(etmq, trace_chan_id, addr, sizeof(instr32), >> + (u8 *)&instr32, 0); >> if ((instr32 & 0x0F000000) == 0x0F000000 && >> (instr32 & 0xF0000000) != 0xF0000000) >> return true; >> @@ -1973,8 +1988,8 @@ static bool cs_etm__is_svc_instr(struct cs_etm_queue *etmq, u8 trace_chan_id, >> * +-----------------------+---------+-----------+ >> */ >> addr = end_addr - 4; >> - cs_etm__mem_access(etmq, trace_chan_id, addr, >> - sizeof(instr32), (u8 *)&instr32); >> + cs_etm__mem_access(etmq, trace_chan_id, addr, sizeof(instr32), >> + (u8 *)&instr32, 0); >> if ((instr32 & 0xFFE0001F) == 0xd4000001) >> return true; >> >> -- >> 2.34.1 >> > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel