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=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 9680DC43381 for ; Tue, 12 Jan 2021 14:15:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6450A23121 for ; Tue, 12 Jan 2021 14:15:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730676AbhALOPr (ORCPT ); Tue, 12 Jan 2021 09:15:47 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33876 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726241AbhALOPq (ORCPT ); Tue, 12 Jan 2021 09:15:46 -0500 Received: from mail-pj1-x102c.google.com (mail-pj1-x102c.google.com [IPv6:2607:f8b0:4864:20::102c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6882EC061786 for ; Tue, 12 Jan 2021 06:15:06 -0800 (PST) Received: by mail-pj1-x102c.google.com with SMTP id lj6so1671932pjb.0 for ; Tue, 12 Jan 2021 06:15:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=7GtD3YonrmXIET8rBk/Trj9GYaC3dwJx6PaGPk04xNU=; b=p8zPfzTtoi55Jiq3vrboqKOsVERPA/n2bF35M7daR4boTgYxhddvsIyjQxvcVmLooa 0xBRL05l6RWtKiE8TMeMxDtCFYueNjl4dOMSSxCKZ275OfsSAxctnDujd3HkEVCwRJIP 4fcdnMKyLh4GkpH/6avje2iRLTTPNW+QAuTk3TCeBgovAGh73eSgHYs+x3xt0jIRlRhy bSgOrkRMr1eFe/4MVPY7viBZrlANizGeEm7HH1NZmM8T4RXXNbsRQ5YSQ6qaAn5bUgyN 1FNKilRh2T7Cw7XjO5eggmetopm4Bk/epZ/R9T2S0+wGtUhGmpXenG26M6asawwU7O7C a5dg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=7GtD3YonrmXIET8rBk/Trj9GYaC3dwJx6PaGPk04xNU=; b=UYI01V0dRMTyuvyHgaB3Bj4sHcOkWaEbo9FLOkTSbBenJLwlb2NUhhLmKv8+dOeNgM hmmqFFDrKbcKkdl9uJm+SDIjpwUZTXMIBJqOnUFqduIBSd9Eg/m6giXhDNG710jVqJs3 fH2vYhHiV0oc3IkH/sVEXDl7vKWQ/TRBK06wA79ZNnpdZ2o+36n0pLZ3NjvvF/fn/yR5 vO8ZQJ2hA7roTzY3Emv617ztFvHsRR2fhfr2Oxy7o5VENthceOf3hhzx/g76dx8m1Vjo ZWfO2IWV7q5OFMQFHlhAreHqPvCFrIsH56A/mRz42qHF4i4i3P6GAL0Y8xyNQyECXSKi czZA== X-Gm-Message-State: AOAM530rcORanfi7RdHCKyUBme5do5wTYN8jDZ8l6dO96rZbtSXOixyY bRfNve2PrvjJZGZaIIVbOHevCQ== X-Google-Smtp-Source: ABdhPJx3te7nt4Meaz1hm32robPfSSYqRQOAN/H6JsLTOVomFO7bTq5g/J3hhhn7CeMTHLe+h7uBOA== X-Received: by 2002:a17:902:82c8:b029:da:eead:94a0 with SMTP id u8-20020a17090282c8b02900daeead94a0mr4988832plz.42.1610460905559; Tue, 12 Jan 2021 06:15:05 -0800 (PST) Received: from leoy-ThinkPad-X240s ([202.155.204.36]) by smtp.gmail.com with ESMTPSA id b11sm3355025pfr.38.2021.01.12.06.15.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Jan 2021 06:15:04 -0800 (PST) Date: Tue, 12 Jan 2021 22:14:58 +0800 From: Leo Yan To: Mike Leach Cc: Arnaldo Carvalho de Melo , Mathieu Poirier , Suzuki K Poulose , Alexander Shishkin , John Garry , Will Deacon , Peter Zijlstra , Ingo Molnar , Mark Rutland , Jiri Olsa , Namhyung Kim , Daniel Kiss , Denis Nikitin , Coresight ML , linux-arm-kernel , Linux Kernel Mailing List , Al Grant Subject: Re: [PATCH v1 1/7] coresight: etm-perf: Add support for PID tracing for kernel at EL2 Message-ID: <20210112141458.GF18965@leoy-ThinkPad-X240s> References: <20210109074435.626855-1-leo.yan@linaro.org> <20210109074435.626855-2-leo.yan@linaro.org> <20210112085826.GC18965@leoy-ThinkPad-X240s> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Mike, On Tue, Jan 12, 2021 at 11:23:03AM +0000, Mike Leach wrote: > Hi Leo, > > On Tue, 12 Jan 2021 at 08:58, Leo Yan wrote: > > > > Hi Mike, > > > > On Mon, Jan 11, 2021 at 04:22:39PM +0000, Mike Leach wrote: > > > > [...] > > > > > > diff --git a/include/linux/coresight-pmu.h b/include/linux/coresight-pmu.h > > > > index b0e35eec6499..927c6285ce5d 100644 > > > > --- a/include/linux/coresight-pmu.h > > > > +++ b/include/linux/coresight-pmu.h > > > > @@ -11,16 +11,19 @@ > > > > #define CORESIGHT_ETM_PMU_SEED 0x10 > > > > > > > > /* ETMv3.5/PTM's ETMCR config bit */ > > > > -#define ETM_OPT_CYCACC 12 > > > > -#define ETM_OPT_CTXTID 14 > > > > -#define ETM_OPT_TS 28 > > > > -#define ETM_OPT_RETSTK 29 > > > > +#define ETM_OPT_CYCACC 12 > > > > +#define ETM_OPT_CTXTID 14 > > > > +#define ETM_OPT_CTXTID_IN_VMID 15 > > > > > > Minor issue here - ETMv3.x / PTM cannot trace CXTID in VMID so this > > > may better be named ETM4_OPT_CTXTID_IN_VMID, rather than be grouped > > > with the ETM3.5 options? > > > > I looked into this suggestion but found it's complex than I assumed. > > This config bits are not only used for ETMv3.x / PTM, it's also used > > as an configuration interface between user space in Perf and kernel > > drivers. > > > > For example, in the userspace, perf tool sets bit ETM_OPT_TS to enable > > timestamp [1], this is same for ETMv3 and ETMv4. In the kernel side, > > the configuration is directly used ETMv3 (in coresight-etm3x-core.c), > > but the configuration bits are converted for ETMv4 in the function > > etm4_parse_event_config() [2]. > > > > So this is a historical issue, at the early period ETMv3 and ETMv4 can > > be compatible with each other for configurations, but after evoluation, > > some configs only belong to ETMv4 and cannot be applied on ETMv3 > > anymore, but we still use ETMv3.5 config bits as the interface between > > kernel and userspace. > > > > I was aware that etm3/ptm used these bits as both the options and the > bit values for direct writing to the ETMCR register for ETMv3, and > re-mapped to appropriate register values in ETMv4. > In the past we have re-used etmv3.5 bit definitions ETM_xxx when > appropriate, but where unique to ETM4 we have used a ETM4_xxx naming > convention. I am concern this approach is not friendly for extension; for example, let's say IP ETM5 with defined bit 28 as CTXTID, if add a new option for it, we need to define macro as: #define ETM5_OPT_CTXTID 28 This will result in confliction with the existed option ETM_OPT_TS and it is hard for maintenance for options, since there have different prefixes (like ETM_OPT_xxx, ETM4_OPT_xxx, ETM5_OPT_xxx, etc). I'd like to take option as knob to enable or disable hardware feature; the low level drivers should set the appropriate values for registers based on different options. Furthermore, ETM driver should report error when detect any option is not supported, I.e. ETM3 driver should report failure if user wrongly set the option ETM_OPT_CTXTID_IN_VMID. > I am not suggesting re-factoring the options completely, just > re-naming this single option to make it clear it is unique to ETM4+. Here I perfer Suzuki's suggestion to simply refine comments, something like below: /* * Below are bit offsets for perf options, most of them are orignally * coming from ETMv3.5/PTM's ETMCR config bits (so far except * ETM_OPT_CTXTID_IN_VMID is only used for ETMv4). * * ETMv3.5/PTM doesn't define ETMCR config bits with prefix "ETM3_" and * directly use below macros as config bits. */ #define ETM_OPT_CYCACC 12 #define ETM_OPT_CTXTID 14 #define ETM_OPT_CTXTID_IN_VMID 15 #define ETM_OPT_TS 28 #define ETM_OPT_RETSTK 29 > Looking at the etmv3 driver, at present it does not actually appear to > support contextid tracing - and when it does, both bits 14 and 15 will > be required to be used - as ETMCR defines these bits as ContextID > size. > Should this ever get fixed. Good catch! Seems to me, this is a good example that we should distinguish the definition between Perf options and config bits :) > then having an overlapping option bit - > that appears to be valid for ETMv3 will be confusing. I hope the the proposed change can avoid the confusion, if have concern, please let me know. Thanks a lot for suggestions, Leo 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=-9.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 43462C433E0 for ; Tue, 12 Jan 2021 14:17:10 +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 D931B2311D for ; Tue, 12 Jan 2021 14:17:09 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D931B2311D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org 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-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=5TmTPMBZZXXsg1zNnWw+2ehTL92/DX8GCt/+yiYLrqs=; b=g24I+zXBAczxDC5QoSpPzoKOz 0qXhLhGAMoy3VTcT/L9Bp3bBQbAkIsz4GkucLLZPYpTj8BJOxVwKgHpRvt24UE21OWtou7u/g5AgH jOUOiFNHZn1p9SFEkhOaaWVNwaWfAmjvAujXGERbP08kDrXkbm6CvBj52GM0sjRem1zURt77qpy9N fKCRcySc0b308uXuKpsmRW+O0CbE3H09pFxYFSD88leHyZXioipfTfaLTEQNFKeekYwGUH/WwMhtk wzhZD1+HIkjqo1NuqQ5RQkX9L98bvhy9stmIO56ksRNkK28cTEMJ3XPIJlipRn9UsNOPkC1a95Q0S fe4gg84Sw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kzKS4-0004os-Rp; Tue, 12 Jan 2021 14:15:16 +0000 Received: from mail-pj1-x1031.google.com ([2607:f8b0:4864:20::1031]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kzKRw-0004lw-1x for linux-arm-kernel@lists.infradead.org; Tue, 12 Jan 2021 14:15:11 +0000 Received: by mail-pj1-x1031.google.com with SMTP id cq1so1476068pjb.4 for ; Tue, 12 Jan 2021 06:15:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=7GtD3YonrmXIET8rBk/Trj9GYaC3dwJx6PaGPk04xNU=; b=p8zPfzTtoi55Jiq3vrboqKOsVERPA/n2bF35M7daR4boTgYxhddvsIyjQxvcVmLooa 0xBRL05l6RWtKiE8TMeMxDtCFYueNjl4dOMSSxCKZ275OfsSAxctnDujd3HkEVCwRJIP 4fcdnMKyLh4GkpH/6avje2iRLTTPNW+QAuTk3TCeBgovAGh73eSgHYs+x3xt0jIRlRhy bSgOrkRMr1eFe/4MVPY7viBZrlANizGeEm7HH1NZmM8T4RXXNbsRQ5YSQ6qaAn5bUgyN 1FNKilRh2T7Cw7XjO5eggmetopm4Bk/epZ/R9T2S0+wGtUhGmpXenG26M6asawwU7O7C a5dg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=7GtD3YonrmXIET8rBk/Trj9GYaC3dwJx6PaGPk04xNU=; b=TZeGs3uuVRjoFeGKjY7i09+UoAoNIKTHb3xBiH9f5SN7EW0NlgYI+R9cx4NSxVBEBr sbwVCg3V6AjSWPt2xJkGeoQaJdABs2Xeq3E4cVhUYNwutJJ/Je4IDwtY6Ad/xuq2oQzc cWMncyfhEvvNBkn4AthGXWJt+FZPVmec3TL/+dNnBR3NRHlXXJJI0X0eibZ0/hLgfh7G lB2D7RIl6ZTGJ3LxDFmr8dvMlRaxnvTEat78ceNbHlYMTns4NtoTlloyRvVsLryoNIoc /l8An5oCr4b3KljX8ehk66rwXJL3dHOGC2Kf98aaojY1+Ubd/UDdVGLRlPDktMc7ZdaQ EwgQ== X-Gm-Message-State: AOAM531557/ezlDRnR+bEXFYhgtXQfmnbScnX4p46sHCKoKYyKL7Q7ji jpF/VcvsYOpvKR2rOl7Om6zcdw== X-Google-Smtp-Source: ABdhPJx3te7nt4Meaz1hm32robPfSSYqRQOAN/H6JsLTOVomFO7bTq5g/J3hhhn7CeMTHLe+h7uBOA== X-Received: by 2002:a17:902:82c8:b029:da:eead:94a0 with SMTP id u8-20020a17090282c8b02900daeead94a0mr4988832plz.42.1610460905559; Tue, 12 Jan 2021 06:15:05 -0800 (PST) Received: from leoy-ThinkPad-X240s ([202.155.204.36]) by smtp.gmail.com with ESMTPSA id b11sm3355025pfr.38.2021.01.12.06.15.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Jan 2021 06:15:04 -0800 (PST) Date: Tue, 12 Jan 2021 22:14:58 +0800 From: Leo Yan To: Mike Leach Subject: Re: [PATCH v1 1/7] coresight: etm-perf: Add support for PID tracing for kernel at EL2 Message-ID: <20210112141458.GF18965@leoy-ThinkPad-X240s> References: <20210109074435.626855-1-leo.yan@linaro.org> <20210109074435.626855-2-leo.yan@linaro.org> <20210112085826.GC18965@leoy-ThinkPad-X240s> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210112_091508_157590_1C5C35C2 X-CRM114-Status: GOOD ( 36.91 ) 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 , Mathieu Poirier , Suzuki K Poulose , Alexander Shishkin , Jiri Olsa , Coresight ML , John Garry , Linux Kernel Mailing List , Arnaldo Carvalho de Melo , Peter Zijlstra , Ingo Molnar , Namhyung Kim , Will Deacon , linux-arm-kernel , Daniel Kiss 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 Hi Mike, On Tue, Jan 12, 2021 at 11:23:03AM +0000, Mike Leach wrote: > Hi Leo, > > On Tue, 12 Jan 2021 at 08:58, Leo Yan wrote: > > > > Hi Mike, > > > > On Mon, Jan 11, 2021 at 04:22:39PM +0000, Mike Leach wrote: > > > > [...] > > > > > > diff --git a/include/linux/coresight-pmu.h b/include/linux/coresight-pmu.h > > > > index b0e35eec6499..927c6285ce5d 100644 > > > > --- a/include/linux/coresight-pmu.h > > > > +++ b/include/linux/coresight-pmu.h > > > > @@ -11,16 +11,19 @@ > > > > #define CORESIGHT_ETM_PMU_SEED 0x10 > > > > > > > > /* ETMv3.5/PTM's ETMCR config bit */ > > > > -#define ETM_OPT_CYCACC 12 > > > > -#define ETM_OPT_CTXTID 14 > > > > -#define ETM_OPT_TS 28 > > > > -#define ETM_OPT_RETSTK 29 > > > > +#define ETM_OPT_CYCACC 12 > > > > +#define ETM_OPT_CTXTID 14 > > > > +#define ETM_OPT_CTXTID_IN_VMID 15 > > > > > > Minor issue here - ETMv3.x / PTM cannot trace CXTID in VMID so this > > > may better be named ETM4_OPT_CTXTID_IN_VMID, rather than be grouped > > > with the ETM3.5 options? > > > > I looked into this suggestion but found it's complex than I assumed. > > This config bits are not only used for ETMv3.x / PTM, it's also used > > as an configuration interface between user space in Perf and kernel > > drivers. > > > > For example, in the userspace, perf tool sets bit ETM_OPT_TS to enable > > timestamp [1], this is same for ETMv3 and ETMv4. In the kernel side, > > the configuration is directly used ETMv3 (in coresight-etm3x-core.c), > > but the configuration bits are converted for ETMv4 in the function > > etm4_parse_event_config() [2]. > > > > So this is a historical issue, at the early period ETMv3 and ETMv4 can > > be compatible with each other for configurations, but after evoluation, > > some configs only belong to ETMv4 and cannot be applied on ETMv3 > > anymore, but we still use ETMv3.5 config bits as the interface between > > kernel and userspace. > > > > I was aware that etm3/ptm used these bits as both the options and the > bit values for direct writing to the ETMCR register for ETMv3, and > re-mapped to appropriate register values in ETMv4. > In the past we have re-used etmv3.5 bit definitions ETM_xxx when > appropriate, but where unique to ETM4 we have used a ETM4_xxx naming > convention. I am concern this approach is not friendly for extension; for example, let's say IP ETM5 with defined bit 28 as CTXTID, if add a new option for it, we need to define macro as: #define ETM5_OPT_CTXTID 28 This will result in confliction with the existed option ETM_OPT_TS and it is hard for maintenance for options, since there have different prefixes (like ETM_OPT_xxx, ETM4_OPT_xxx, ETM5_OPT_xxx, etc). I'd like to take option as knob to enable or disable hardware feature; the low level drivers should set the appropriate values for registers based on different options. Furthermore, ETM driver should report error when detect any option is not supported, I.e. ETM3 driver should report failure if user wrongly set the option ETM_OPT_CTXTID_IN_VMID. > I am not suggesting re-factoring the options completely, just > re-naming this single option to make it clear it is unique to ETM4+. Here I perfer Suzuki's suggestion to simply refine comments, something like below: /* * Below are bit offsets for perf options, most of them are orignally * coming from ETMv3.5/PTM's ETMCR config bits (so far except * ETM_OPT_CTXTID_IN_VMID is only used for ETMv4). * * ETMv3.5/PTM doesn't define ETMCR config bits with prefix "ETM3_" and * directly use below macros as config bits. */ #define ETM_OPT_CYCACC 12 #define ETM_OPT_CTXTID 14 #define ETM_OPT_CTXTID_IN_VMID 15 #define ETM_OPT_TS 28 #define ETM_OPT_RETSTK 29 > Looking at the etmv3 driver, at present it does not actually appear to > support contextid tracing - and when it does, both bits 14 and 15 will > be required to be used - as ETMCR defines these bits as ContextID > size. > Should this ever get fixed. Good catch! Seems to me, this is a good example that we should distinguish the definition between Perf options and config bits :) > then having an overlapping option bit - > that appears to be valid for ETMv3 will be confusing. I hope the the proposed change can avoid the confusion, if have concern, please let me know. Thanks a lot for suggestions, Leo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel