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=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable 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 3E27CC433DB for ; Tue, 19 Jan 2021 07:08:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E247C23129 for ; Tue, 19 Jan 2021 07:08:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730383AbhASHH5 (ORCPT ); Tue, 19 Jan 2021 02:07:57 -0500 Received: from foss.arm.com ([217.140.110.172]:40744 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729489AbhASHGJ (ORCPT ); Tue, 19 Jan 2021 02:06:09 -0500 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 D13E71FB; Mon, 18 Jan 2021 23:05:21 -0800 (PST) Received: from [10.57.40.145] (unknown [10.57.40.145]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 55B633F66E; Mon, 18 Jan 2021 23:05:18 -0800 (PST) Subject: Re: [PATCH v1 1/7] coresight: etm-perf: Add support for PID tracing for kernel at EL2 To: Mathieu Poirier , Leo Yan Cc: Arnaldo Carvalho de Melo , Mike Leach , Alexander Shishkin , John Garry , Will Deacon , Peter Zijlstra , Ingo Molnar , Mark Rutland , Jiri Olsa , Namhyung Kim , Daniel Kiss , Denis Nikitin , coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Al Grant References: <20210109074435.626855-1-leo.yan@linaro.org> <20210109074435.626855-2-leo.yan@linaro.org> <20210115223043.GA375055@xps15> From: Suzuki K Poulose Message-ID: Date: Tue, 19 Jan 2021 07:05:10 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 MIME-Version: 1.0 In-Reply-To: <20210115223043.GA375055@xps15> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Mathieu On 1/15/21 10:30 PM, Mathieu Poirier wrote: > Hey guys, > > On Sat, Jan 09, 2021 at 03:44:29PM +0800, Leo Yan wrote: >> From: Suzuki K Poulose >> >> When the kernel is running at EL2, the PID is stored in CONTEXTIDR_EL2. >> So, tracing CONTEXTIDR_EL1 doesn't give us the pid of the process. >> Thus we should trace the VMID with VMIDOPT set to trace >> CONTEXTIDR_EL2 instead of CONTEXTIDR_EL1. Given that we have an existing >> config option "contextid" and this will be useful for tracing >> virtual machines (when we get to support virtualization). So instead, >> this patch adds a new option, contextid_in_vmid as a separate config. >> Thus on an EL2 kernel, we will have two options available for >> the perf tool. However, to make it easier for the user to >> do pid tracing, we add a new format which will default to >> "contextid" (on EL1 kernel) or "contextid_in_vmid" (on EL2 >> kernel). So that the user doesn't have to bother which EL the >> kernel is running. >> >> i.e, perf record -e cs_etm/pid/u -- >> >> will always do the "pid" tracing, independent of the kernel EL. >> >> Also, the perf tool will be updated to automatically select >> "pid" config instead of the "contextid" for system wide/CPU wide >> mode. >> >> Cc: Mathieu Poirier >> Cc: Al Grant >> Cc: Mike Leach >> Signed-off-by: Suzuki K Poulose >> Signed-off-by: Leo Yan >> --- >> drivers/hwtracing/coresight/coresight-etm-perf.c | 14 ++++++++++++++ >> drivers/hwtracing/coresight/coresight-etm4x-core.c | 9 +++++++++ >> include/linux/coresight-pmu.h | 11 +++++++---- >> 3 files changed, 30 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c >> index bdc34ca449f7..f763def145e4 100644 >> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c >> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c >> @@ -30,14 +30,28 @@ static DEFINE_PER_CPU(struct coresight_device *, csdev_src); >> /* ETMv3.5/PTM's ETMCR is 'config' */ >> PMU_FORMAT_ATTR(cycacc, "config:" __stringify(ETM_OPT_CYCACC)); >> PMU_FORMAT_ATTR(contextid, "config:" __stringify(ETM_OPT_CTXTID)); >> +PMU_FORMAT_ATTR(contextid_in_vmid, "config:" __stringify(ETM_OPT_CTXTID_IN_VMID)); > > I am not convinced adding this new contextid_in_vmid is the best way forward. > >> PMU_FORMAT_ATTR(timestamp, "config:" __stringify(ETM_OPT_TS)); >> PMU_FORMAT_ATTR(retstack, "config:" __stringify(ETM_OPT_RETSTK)); >> /* Sink ID - same for all ETMs */ >> PMU_FORMAT_ATTR(sinkid, "config2:0-31"); >> >> +static ssize_t format_attr_pid_show(struct device *dev, >> + struct device_attribute *attr, >> + char *page) >> +{ >> + int pid_fmt = is_kernel_in_hyp_mode() ? ETM_OPT_CTXTID_IN_VMID : ETM_OPT_CTXTID; >> + >> + return sprintf(page, "config:%d\n", pid_fmt); >> +} >> + >> +struct device_attribute format_attr_pid = __ATTR(pid, 0444, format_attr_pid_show, NULL); > > The same applies here. PMU format bits are options the PMU supports rather than > a representation of the hardware, making bit numbering arbitrary. A such we > don't explicitly need a contextid_in_vmid option. Making the current contextid > variable, the same it was done for 'pid', should be sufficient. Based on the > value carried by contexid, i.e 14 or 15, the perf tools will know where to get > the contextID. > > With regards to backward functionality, user who hard code 'config' on the perf > command line won't get the results they want when the kernel is at EL2 anyway. > > The kernel, with function is_kernel_in_hyp_mode(), is not an issue. I did think about that. The reason behind using a new alias is vaguely mentioned in the description. If a host perf session wants to trace a VM with the contextid_el1, there is no option for that with "contextid" flipped to trace "contextid_el2". This is precisely why I preferred keeping both the hardware configurations and let the kernel choose the right one for the EL, by having an alias. i.e, perf record -e cs_etm/contextid,contextid_in_vmid/ vm could still trace the VM vcpu threads and the CID changes within the VM. (This is triggered from the host, so VM support is not necessary). If we decide not to do this, or change the meaning of contextid now to mean "pid" and change it in the future back to what it really means for supporting such scenarios above, then we are going to be back where we are with the proposal. Suzuki 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=-15.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, 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 CCB85C433DB for ; Tue, 19 Jan 2021 07:07:04 +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 742B223120 for ; Tue, 19 Jan 2021 07:07:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 742B223120 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=lt5L2MtZ9gDsF6P6hubDxqH3eL0EG3CImVgzy4OK4W8=; b=EtPsqYzupmnN2fnWybq1+j1om f20hCg9dBtAi7YoTUe2KtTZGRkVr1DaELWW0443aMjNtXJb1QuZVyx0FE3qooQXVdplJjXBVMYJa+ vm5yXUrgZyFnUwwXz9JZq6pfnBUKDNanpJkbEKbUXECvLm4dn3t1Xr415S1XzrNC4cdNBVa5wTTZD H0yZI/fTzkiR1rvJEV8IrlX6FkCZmUei7viJVZJRSbuP/i6ZL1VCdTRXclvh+fAqE3dBIMKFZoNhI NFo4e2VINkUzGSew2O2V03h6/LEoxyibL9AGCY5QC5NzlojapwcJ/lDNcf+sowywlh3RxmVYilFVn tF0O5Fddw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1l1l54-00080T-4L; Tue, 19 Jan 2021 07:05:34 +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 1l1l4z-0007ze-Qz for linux-arm-kernel@lists.infradead.org; Tue, 19 Jan 2021 07:05:31 +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 D13E71FB; Mon, 18 Jan 2021 23:05:21 -0800 (PST) Received: from [10.57.40.145] (unknown [10.57.40.145]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 55B633F66E; Mon, 18 Jan 2021 23:05:18 -0800 (PST) Subject: Re: [PATCH v1 1/7] coresight: etm-perf: Add support for PID tracing for kernel at EL2 To: Mathieu Poirier , Leo Yan References: <20210109074435.626855-1-leo.yan@linaro.org> <20210109074435.626855-2-leo.yan@linaro.org> <20210115223043.GA375055@xps15> From: Suzuki K Poulose Message-ID: Date: Tue, 19 Jan 2021 07:05:10 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 MIME-Version: 1.0 In-Reply-To: <20210115223043.GA375055@xps15> Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210119_020529_970609_E76990BA X-CRM114-Status: GOOD ( 29.60 ) 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: Mark Rutland , Al Grant , Denis Nikitin , Alexander Shishkin , Jiri Olsa , coresight@lists.linaro.org, John Garry , linux-kernel@vger.kernel.org, Arnaldo Carvalho de Melo , Peter Zijlstra , Ingo Molnar , Namhyung Kim , Daniel Kiss , Will Deacon , linux-arm-kernel@lists.infradead.org, Mike Leach 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 Hi Mathieu On 1/15/21 10:30 PM, Mathieu Poirier wrote: > Hey guys, > > On Sat, Jan 09, 2021 at 03:44:29PM +0800, Leo Yan wrote: >> From: Suzuki K Poulose >> >> When the kernel is running at EL2, the PID is stored in CONTEXTIDR_EL2. >> So, tracing CONTEXTIDR_EL1 doesn't give us the pid of the process. >> Thus we should trace the VMID with VMIDOPT set to trace >> CONTEXTIDR_EL2 instead of CONTEXTIDR_EL1. Given that we have an existing >> config option "contextid" and this will be useful for tracing >> virtual machines (when we get to support virtualization). So instead, >> this patch adds a new option, contextid_in_vmid as a separate config. >> Thus on an EL2 kernel, we will have two options available for >> the perf tool. However, to make it easier for the user to >> do pid tracing, we add a new format which will default to >> "contextid" (on EL1 kernel) or "contextid_in_vmid" (on EL2 >> kernel). So that the user doesn't have to bother which EL the >> kernel is running. >> >> i.e, perf record -e cs_etm/pid/u -- >> >> will always do the "pid" tracing, independent of the kernel EL. >> >> Also, the perf tool will be updated to automatically select >> "pid" config instead of the "contextid" for system wide/CPU wide >> mode. >> >> Cc: Mathieu Poirier >> Cc: Al Grant >> Cc: Mike Leach >> Signed-off-by: Suzuki K Poulose >> Signed-off-by: Leo Yan >> --- >> drivers/hwtracing/coresight/coresight-etm-perf.c | 14 ++++++++++++++ >> drivers/hwtracing/coresight/coresight-etm4x-core.c | 9 +++++++++ >> include/linux/coresight-pmu.h | 11 +++++++---- >> 3 files changed, 30 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c >> index bdc34ca449f7..f763def145e4 100644 >> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c >> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c >> @@ -30,14 +30,28 @@ static DEFINE_PER_CPU(struct coresight_device *, csdev_src); >> /* ETMv3.5/PTM's ETMCR is 'config' */ >> PMU_FORMAT_ATTR(cycacc, "config:" __stringify(ETM_OPT_CYCACC)); >> PMU_FORMAT_ATTR(contextid, "config:" __stringify(ETM_OPT_CTXTID)); >> +PMU_FORMAT_ATTR(contextid_in_vmid, "config:" __stringify(ETM_OPT_CTXTID_IN_VMID)); > > I am not convinced adding this new contextid_in_vmid is the best way forward. > >> PMU_FORMAT_ATTR(timestamp, "config:" __stringify(ETM_OPT_TS)); >> PMU_FORMAT_ATTR(retstack, "config:" __stringify(ETM_OPT_RETSTK)); >> /* Sink ID - same for all ETMs */ >> PMU_FORMAT_ATTR(sinkid, "config2:0-31"); >> >> +static ssize_t format_attr_pid_show(struct device *dev, >> + struct device_attribute *attr, >> + char *page) >> +{ >> + int pid_fmt = is_kernel_in_hyp_mode() ? ETM_OPT_CTXTID_IN_VMID : ETM_OPT_CTXTID; >> + >> + return sprintf(page, "config:%d\n", pid_fmt); >> +} >> + >> +struct device_attribute format_attr_pid = __ATTR(pid, 0444, format_attr_pid_show, NULL); > > The same applies here. PMU format bits are options the PMU supports rather than > a representation of the hardware, making bit numbering arbitrary. A such we > don't explicitly need a contextid_in_vmid option. Making the current contextid > variable, the same it was done for 'pid', should be sufficient. Based on the > value carried by contexid, i.e 14 or 15, the perf tools will know where to get > the contextID. > > With regards to backward functionality, user who hard code 'config' on the perf > command line won't get the results they want when the kernel is at EL2 anyway. > > The kernel, with function is_kernel_in_hyp_mode(), is not an issue. I did think about that. The reason behind using a new alias is vaguely mentioned in the description. If a host perf session wants to trace a VM with the contextid_el1, there is no option for that with "contextid" flipped to trace "contextid_el2". This is precisely why I preferred keeping both the hardware configurations and let the kernel choose the right one for the EL, by having an alias. i.e, perf record -e cs_etm/contextid,contextid_in_vmid/ vm could still trace the VM vcpu threads and the CID changes within the VM. (This is triggered from the host, so VM support is not necessary). If we decide not to do this, or change the meaning of contextid now to mean "pid" and change it in the future back to what it really means for supporting such scenarios above, then we are going to be back where we are with the proposal. Suzuki _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel