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 D7C02C77B73 for ; Thu, 27 Apr 2023 15:53:19 +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=f2MPP/F6zx+rYGaZXywqhcc73VpgiZTZO5vnBFoBsQU=; b=MDr9pOBFxtctHn AHJ4gDbA8hUvQPboct9k9WbjTU5pu4a4jIoRxg0V2O4/NUtSJ1R305XMTjffscoSmQRzdLSeexF9V /E+7rfjatpVeO4iPjdhNdalJoeOM3S8e6Ric/yvQ8PnWJMkqnG1W4YqIMuM+ip2eEeFf3ul+QmmhR 25leQ5IDBSXrOnzSYIubWxHGVryQRK9UJNQCdqQCLIXvCHOEPiuEGhcxGeth1zZlYcNXxwK2hK3D7 a0RhVvg9Ep98ydHLgVwSMU5Qi7tyoQ2UFQHpb/Fh6duaqjsUJ3mUnHhnD/PkL/1JQqCiV9IlVpDG0 erE62Q/nYJ83uhuTF8pg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1ps3uy-006vkv-0W; Thu, 27 Apr 2023 15:52:24 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1ps3uu-006vj3-06 for linux-arm-kernel@lists.infradead.org; Thu, 27 Apr 2023 15:52:21 +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 976E72F4; Thu, 27 Apr 2023 08:52:55 -0700 (PDT) Received: from [10.57.56.155] (unknown [10.57.56.155]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C42833F64C; Thu, 27 Apr 2023 08:52:08 -0700 (PDT) Message-ID: Date: Thu, 27 Apr 2023 16:52:06 +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/7] perf: cs-etm: Validate options after applying themperf_pmu__format_bits Content-Language: en-US To: Leo Yan 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 References: <20230424134748.228137-1-james.clark@arm.com> <20230424134748.228137-5-james.clark@arm.com> <20230427151228.GA152865@leoy-huanghe> From: James Clark In-Reply-To: <20230427151228.GA152865@leoy-huanghe> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230427_085220_206236_56B8AFDE X-CRM114-Status: GOOD ( 24.31 ) 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 27/04/2023 16:12, Leo Yan wrote: > Hi James, > > On Mon, Apr 24, 2023 at 02:47:44PM +0100, James Clark wrote: >> Currently the cs_etm_set_option() function both validates and applies >> the config options. Because it's only called when they are added >> automatically, there are some paths where the user can apply the option >> on the command line and skip the validation. By moving it to the end it >> covers both cases. >> >> Also, options don't need to be re-applied anyway, Perf handles parsing >> and applying the config terms automatically. >> >> Signed-off-by: James Clark >> --- >> tools/perf/arch/arm/util/cs-etm.c | 152 +++++++++++++----------------- >> 1 file changed, 68 insertions(+), 84 deletions(-) >> >> diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c >> index f9b9ebf7fffc..af0a2400c655 100644 >> --- a/tools/perf/arch/arm/util/cs-etm.c >> +++ b/tools/perf/arch/arm/util/cs-etm.c >> @@ -69,21 +69,29 @@ static const char * const metadata_ete_ro[] = { >> static bool cs_etm_is_etmv4(struct auxtrace_record *itr, int cpu); >> static bool cs_etm_is_ete(struct auxtrace_record *itr, int cpu); >> >> -static int cs_etm_set_context_id(struct auxtrace_record *itr, >> - struct evsel *evsel, int cpu) >> +static int cs_etm_validate_context_id(struct auxtrace_record *itr, >> + struct evsel *evsel, int cpu) >> { >> - struct cs_etm_recording *ptr; >> - struct perf_pmu *cs_etm_pmu; >> + struct cs_etm_recording *ptr = >> + container_of(itr, struct cs_etm_recording, itr); >> + struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu; >> char path[PATH_MAX]; >> - int err = -EINVAL; >> + int err; >> u32 val; >> - u64 contextid; >> + u64 contextid = >> + evsel->core.attr.config & >> + (perf_pmu__format_bits(&cs_etm_pmu->format, "contextid1") | >> + perf_pmu__format_bits(&cs_etm_pmu->format, "contextid2")); > > Seems to me, this would break backward compability. > > The old kernel (before 5.11) doesn't provide 'contextid1' and > 'contextid2', so we always check the entry 'contextid' rather than > 'contextid1' and 'contextid2'. > > With this change, if a kernel doesn't contain 'contextid1' and > 'contextid2' formats, will perf tool never trace for contexid? > No because I changed to to be purely validation, so the format flags would still be applied. But yes I think you are right there is a small issue. Now validation of 'contextid' isn't done on pre 5.11 kernels. But that only checks for ETMv3 anyway. Validation of 'contextid1' and 'contextid2' isn't a problem, because if the kernel doesn't support them they can't be applied on the command line anyway. I can fix it by checking for 'contextid' and ETMv3 first and then doing 'contextid1' and 'contextid2' after. > Thanks, > Leo > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel