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 X-Spam-Level: X-Spam-Status: No, score=-11.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C33D0C5517A for ; Wed, 11 Nov 2020 11:41:33 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4AAF82065E for ; Wed, 11 Nov 2020 11:41:33 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="T1T1mXt1" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4AAF82065E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=nWqQt/rLiDpar166w0jRk0JmmimQ0TdRrgB/mn4a17k=; b=T1T1mXt1Gc/i8+Sxxz6M6F+ll VgNyM88IyA0Xs+EgM6RX+Rru4gGOErcZVzFKZcarXHh255WGtLBHnmyhpz7vjbA1PNhtBzlRBk/cy 2QyaetRIoNyYqfHQ/hdu3thbjQhb8WCRkO0WJL16gZNYIVLT7DIu+MpfQ2sNNcYg4GwlV5AtLT06N sFfZbU6gs53PQDTi46LbTxoOStxAZfhtCeFVNS1VRKF/idvTBCtECl75sssnimYW7yi3lzl3aDe45 Ch3IxlgTHtOhtTNNEVd4stGhwd0aj6VqhL68MVzCiK2BUBXB06Ql5rQzq/l1dzQUEiAoJWSvBT769 as6eFc5Ww==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kcoU6-0000WB-Ks; Wed, 11 Nov 2020 11:40:18 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kcoU4-0000VT-2D for linux-arm-kernel@lists.infradead.org; Wed, 11 Nov 2020 11:40: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 CA6C8101E; Wed, 11 Nov 2020 03:40:11 -0800 (PST) Received: from [10.57.23.123] (unknown [10.57.23.123]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A78F13F7BB; Wed, 11 Nov 2020 03:40:10 -0800 (PST) Subject: Re: [PATCH 3/3] rfc: perf: cs_etm: Detect pid in VMID for kernel running at EL2 To: Al Grant , "linux-arm-kernel@lists.infradead.org" References: <20201110183313.1823760-1-suzuki.poulose@arm.com> <20201110183313.1823760-4-suzuki.poulose@arm.com> From: Suzuki K Poulose Message-ID: Date: Wed, 11 Nov 2020 11:40:03 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.1 MIME-Version: 1.0 In-Reply-To: Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201111_064016_232414_828432C3 X-CRM114-Status: GOOD ( 38.89 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "coresight@lists.linaro.org" , "leo.yan@linaro.org" , Anshuman Khandual , "mathieu.poirier@linaro.org" , "mike.leach@linaro.org" Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 11/11/20 11:03 AM, Al Grant wrote: > From: Suzuki K Poulose >> The pid of the task could be traced as VMID when the kernel is running at EL2. >> Teach the decoder to look for vmid when the context_id is invalid but we have a >> valid VMID. >> >> Cc: Mike Leach >> Cc: Leo Yan >> Cc: Mathieu Poirier >> Signed-off-by: Suzuki K Poulose >> --- >> >> Ideally, we should also confirm that the VMID_OPT is also set in the trcconfigr >> for choosing the VMID. Hence the RFC. May be that is something we could >> cache in the "decoder" instance ? >> --- >> .../perf/util/cs-etm-decoder/cs-etm-decoder.c | 28 +++++++++++-------- >> 1 file changed, 17 insertions(+), 11 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 cd007cc9c283..31ba7ff57914 100644 >> --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c >> +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c >> @@ -502,19 +502,25 @@ cs_etm_decoder__set_tid(struct cs_etm_queue >> *etmq, { >> pid_t tid; >> >> - /* Ignore PE_CONTEXT packets that don't have a valid contextID */ >> - if (!elem->context.ctxt_id_valid) >> - return OCSD_RESP_CONT; >> - >> - tid = elem->context.context_id; >> - 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. >> + * Process the PE_CONTEXT packets if we have a valid >> + * contextID or VMID. >> + * If the kernel is running at EL2, the PID is traced >> + * in contextidr_el2 as VMID. >> */ >> - cs_etm_decoder__reset_timestamp(packet_queue); >> + if (elem->context.ctxt_id_valid || elem->context.vmid_valid) { >> + if (elem->context.ctxt_id_valid) >> + tid = elem->context.context_id; >> + else >> + tid = elem->context.vmid; >> + 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. >> + */ >> + cs_etm_decoder__reset_timestamp(packet_queue); >> + } >> >> return OCSD_RESP_CONT; >> } > > So if both CONTEXTID and VMID fields are valid, it will take the TID from > CONTEXTID? That seems to make the assumption that... Please see my comment under the patch. This is precisely why it is an RFC. As I said, unless we know that VMIDOPT == 1, we cant assume VMID is tid. > > - it is valid to have VMID in trace from an EL1 kernel (and it should be > ignored in favour of getting TID from CONTEXID) Yes, and it is ignored if CONTEXTID is valid. > > - for an EL2 kernel, where we need to get TID from VMID, we are guaranteed > to not have valid CONTEXTID Do you mean the CID is invalid in the trace or CID == 0 ? For a VM case, it may still be valid (when we get to virtualization). > > This seems the wrong way round. Why ? > An EL2 kernel might want to trace its > EL1 guests' CONTEXTID to disambiguate guest processes - right now I don't > believe we could make use of this in perf, but it's information that is > meaningful in an EL2 kernel's trace. On the other hand, an EL1 kernel's This is still possible. The perf session could set : contextid and this would trace the PID of the guest applications in the CID (as the guest kernel is at EL1). But if you want to trace guest applications of multiple VMs, that becomes tricky (because we now have VMID and CID and the perf tool must be explicitly taught how to decode the data) and I would rather have separate perf sessions attached to the VMs. This is precisely why we set "two" separate config bits for the contextid and contextid_in_vmid. See my kernel patch description and the cover letter. > trace doesn't have any business with VMID (which would be the actual > VM id) - if an EL1 kernel was capturing trace from other VMs in a trace > buffer, that's a big security hole. Yes, this is already part of the kernel patch. See : + /* Do not enable VMID tracing if we are not running in EL2 */ + if (attr->config & BIT(ETM_OPT_CTXTID_IN_VMID)) { + if (!is_kernel_in_hyp_mode()) { + ret = -EINVAL; + goto out; + } + config->cfg |= BIT(ETM4_CFG_BIT_VMID) | BIT(ETM4_CFG_BIT_VMID_OPT); + } > > It's safer if perf.data has an explicit indication of which field the TID is in - > that indicator should be either somewhere in AUXTRACE_INFO or in one > of the config fields in the attribute, or a bit in the PERF_RECORD_AUX This is what the perf tool patch does now. Except for the decoder, which doesn't have the info about VMID_OPT. I didn't want to hack it terribly bad to incorporate this. I guess Mike understands this code better than I do and could do it neater. > record. But if you must do it heuristically at decode time based on what > you see in packets, then it would be safer to say that if you see both > VMID and CONTEXIDR, the TID will be in VMID. This may not be entirely correct, unless we correlate the VMID_OPT. Because, when we add Virtualization support (vmid == actual vmid). So we should do: if (VMID && VMIDOPT) { tid = pid; } else if (CID) { } Suzuki > > Al > > > >> -- >> 2.24.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel