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 17660C77B61 for ; Mon, 1 May 2023 07:35:58 +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:MIME-Version:References: Message-ID:Subject:Cc: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=v1HVhsfjiIGrbIs90OdPW23qnI8jjg5eBjTKmLKdmf4=; b=eWeKaxb8fp5eLR /itBcy3yXanMxgcgLUWlSHendzRoq20ZwNjk2M6Bw0PLjGovb1VcGropbvHRluC5a+XWQEEqgOcD2 GEn9/4hBKVB2MHWRCReR0iXpUJ7yVGXhXoi6LHtbGTHG3sFcggYlzfUuqe/AogP4XLQHEBQgDQzfe PfZCDpCF7DfxcAFbMV5LCA0yfUpCOHe4JqD6Mxn4unHEzXa5SVptib6KZaGpJk1L2tMQegBvSK5gY eoi2+rDX7eotVP+Fb9k47kGn8oU8eIXHjqgGoFoaNxUsoBNPOq1JF/rlkSMBF9kYpjTIHcRM+SoHd TiGlaQJcqRAF5bHw1HZg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1ptO3w-00FPKc-2t; Mon, 01 May 2023 07:35:08 +0000 Received: from mail-pj1-x1031.google.com ([2607:f8b0:4864:20::1031]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1ptO3t-00FPJu-29 for linux-arm-kernel@lists.infradead.org; Mon, 01 May 2023 07:35:07 +0000 Received: by mail-pj1-x1031.google.com with SMTP id 98e67ed59e1d1-24df4ecdb87so621642a91.0 for ; Mon, 01 May 2023 00:35:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1682926504; x=1685518504; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=1wQrzKEZuX3/5Eef7KYCRXhJKEAjq0AaAjZ6GxmmUQA=; b=pz6OmxP6Fg7Vb4oJk0JZFeQ+MTDUHyPTrS/XaaO/HIdZRNFns/MsY2sRHWMXintVHn t+M3tkRSn55imwLUUniv4dxD9C5df5uptXLGHHknGuyGPyqnMHkX6Ws9QBlEwU+rV2Wl p8RSHV9DpzPkzAqVjEIp8WJbPrTlBCuNI7gU/RMvw9VkWuBxmQxZFg16NYgJ4TGTxJ00 DbDh2d1OH2GrvL0uEa+VoeXxKImM4LwPVv3CFiRmPO6Naii89aEQYJNWlMX13BQjVonl WUg1jYj7mdR3gcelvz+B7MddNbvBwS9JSCcXca2RiPTE9PfwimO2Waq52C1RjGAeVH54 ZBRA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682926504; x=1685518504; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=1wQrzKEZuX3/5Eef7KYCRXhJKEAjq0AaAjZ6GxmmUQA=; b=PC3y/aufYvcZkD6Gb/iOap7ypKuO12WJ9Bi224oeNYhuDzkV/dlkd39vIiknG/OcuH T3IbVzyDVMyF7Fex+IVz2KY/1+Md/X3bdvv6lfQiJXq1XG8+9UiECPWgkQRUYWFBUCwc 7DE46U/gLcbUnrSdoE2QKy+7VZPLUz2VG36fHgWv69AiJjG5p9PjGFvUIYlu77tXNZmP zG3ZEBD6pnBFtt0uG9MMiPQ/lMD57f5NSCg5HTPIQsa4lmHhT7EMr9GBg34B1naTNyhA tLoqxz5JoiGP3A4nWnzQmqLQZL9SSe+SYO2epDja9tFdmX1Pdp73W9dp5dpEfElIoEgL +xBw== X-Gm-Message-State: AC+VfDzQLWf4FWfLsjUmm6UyPni4kem7V6o7xUeKh/hQjHPvkjQe3ure I6qLX6+SPugkDPyXrv9e+N/Fy59WUf4x3LftsDZf97Jb X-Google-Smtp-Source: ACHHUZ5t9aChF+EmTsZGawIyA2f+eNOqT6Sz8TCl4+O9Y7OcJ6pgRQb4NvOOcYnrIyx8drmrmh/Wnw== X-Received: by 2002:a17:90b:954:b0:24d:fb2c:1ae0 with SMTP id dw20-20020a17090b095400b0024dfb2c1ae0mr2722814pjb.17.1682926503957; Mon, 01 May 2023 00:35:03 -0700 (PDT) Received: from leoy-yangtze.lan ([107.151.177.135]) by smtp.gmail.com with ESMTPSA id h24-20020a17090adb9800b0024df0e14479sm2013816pjv.5.2023.05.01.00.34.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 01 May 2023 00:35:03 -0700 (PDT) Date: Mon, 1 May 2023 15:34:52 +0800 From: Leo Yan To: James Clark Cc: linux-perf-users@vger.kernel.org, coresight@lists.linaro.org, shy828301@gmail.com, denik@google.com, Mathieu Poirier , Suzuki K Poulose , Mike Leach , John Garry , Will Deacon , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Ian Rogers , Adrian Hunter , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 4/7] perf: cs-etm: Validate options after applying themperf_pmu__format_bits Message-ID: <20230501073452.GA4660@leoy-yangtze.lan> References: <20230424134748.228137-1-james.clark@arm.com> <20230424134748.228137-5-james.clark@arm.com> <20230427151228.GA152865@leoy-huanghe> <20230427221047.GA174797@leoy-huanghe> <15217b55-8a80-fbe9-7b7f-997939f42d0d@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <15217b55-8a80-fbe9-7b7f-997939f42d0d@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230501_003505_712576_4FA7E2A5 X-CRM114-Status: GOOD ( 37.55 ) 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 Fri, Apr 28, 2023 at 01:33:16PM +0100, James Clark wrote: [...] > > ETMv4 uses 'contextid' as well, since the user space needs to know which > > exception level's PID should be traced, e.g. when CPU runs in EL2 > > 'contextid' is set as ETM_OPT_CTXTID2, the perf tool will set 'contextid2' > > to tell driver to trace CONTEXTIDR_EL2. > > > > That's still working because it reads the config term in the setup > function rather than setting any one bit manually: > > if (!perf_cpu_map__empty(cpus)) { > evsel__set_config_if_unset(cs_etm_pmu, cs_etm_evsel, > "timestamp", 1); > evsel__set_config_if_unset(cs_etm_pmu, cs_etm_evsel, > "contextid", 1); > } Okay, yeah, this change can set CTXTID or CTXTID2 bit if user doesn't set anything. > > We can only verify 'contextid', and set 'contextid1' or 'contextid2' based > > on CPU running exception level, finally driver knows how to trace PID. > > > > Thanks, > > Leo > > > > I'm not 100% sure what you mean by this. But previously the validation > was looking at both contextid1 and contextid2 options and checking if > either were supported if either were set. > > I have the following change in mind, it fixes the backwards > compatibility issue. And the validation should be exactly the same as it > was before. Except for one bug that I found when both bits are requested > which I've also fixed here: > > From f1b9f56df29dfb4f2a7be25f009c79c86335587a Mon Sep 17 00:00:00 2001 > From: James Clark > Date: Fri, 28 Apr 2023 10:29:52 +0100 > Subject: [PATCH] perf cs-etm: Fix contextid validation > > Pre 5.11 kernels don't support 'contextid1' and 'contextid2' so > validation would be skipped. By adding an additional check for > 'contextid', old kernels will still have validation done even though > contextid would either be contextid1 or contextid2. > > Additionally now that it's possible to override options, an existing bug > in the validation is revealed. 'val' is overwritten by the contextid1 > validation, and re-used for contextid2 validation causing it to always > fail. '!val || val != 0x4' is the same as 'val != 0x4' because 0 is also > != 4, so that expression can be simplified and the temp variable not > overwritten. > > Fixes: 35c51f83dd1e ("perf cs-etm: Validate options after applying them") > Signed-off-by: James Clark LGTM: Reviewed-by: Leo Yan --- > tools/perf/arch/arm/util/cs-etm.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c > index 77cb03e6ff87..9ca040bfb1aa 100644 > --- a/tools/perf/arch/arm/util/cs-etm.c > +++ b/tools/perf/arch/arm/util/cs-etm.c > @@ -78,9 +78,9 @@ static int cs_etm_validate_context_id(struct auxtrace_record *itr, > char path[PATH_MAX]; > int err; > u32 val; > - u64 contextid = > - evsel->core.attr.config & > - (perf_pmu__format_bits(&cs_etm_pmu->format, "contextid1") | > + u64 contextid = evsel->core.attr.config & > + (perf_pmu__format_bits(&cs_etm_pmu->format, "contextid") | > + perf_pmu__format_bits(&cs_etm_pmu->format, "contextid1") | > perf_pmu__format_bits(&cs_etm_pmu->format, "contextid2")); > > if (!contextid) > @@ -114,8 +114,7 @@ static int cs_etm_validate_context_id(struct auxtrace_record *itr, > * 0b00100 Maximum of 32-bit Context ID size. > * All other values are reserved. > */ > - val = BMVAL(val, 5, 9); > - if (!val || val != 0x4) { > + if (BMVAL(val, 5, 9) != 0x4) { > pr_err("%s: CONTEXTIDR_EL1 isn't supported, disable with %s/contextid1=0/\n", > CORESIGHT_ETM_PMU_NAME, CORESIGHT_ETM_PMU_NAME); > return -EINVAL; > -- > 2.34.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel