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=-13.9 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,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 E8542C433DB for ; Sat, 23 Jan 2021 15:26:17 +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 9082023331 for ; Sat, 23 Jan 2021 15:26:17 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9082023331 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=V3Ml4cIpMNjJE9Kpy1ONF92sVNt1p3lSwJ0THGySpXk=; b=yjnMtBvznX9fyHcnuAEE8/X/e eGFmFAMSgEl18gELthbYPVkaMDksKs2bsJWK6l4Sndu8cqBs3i1ClVuXoP1rma8+cdp3jTK0daW7N bA1LGnk3n9y7qgQZ4sah4JsPr92q1vTk/gvjfXKGUymraCec42onbr61Xlcudro8QWDdkRyWEL2uA ox2nXrQuXueLLmBTHO4AMZ7UQFY/JxTHzUzue0EupfN+pQpJDIjQhqKAslgSwViGNe+DxPsF7rFHi V5CY6rmspYqKnwUxRJkgWbrSRUGHdBZqypugpm1mKDwzraIsrCSOGr3nAglLnbMZbEZ9gzlVhY0Kf y7zOrrCdQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1l3KlL-00019D-WF; Sat, 23 Jan 2021 15:23:44 +0000 Received: from mail-pj1-x1030.google.com ([2607:f8b0:4864:20::1030]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1l3KlI-000181-2f for linux-arm-kernel@lists.infradead.org; Sat, 23 Jan 2021 15:23:41 +0000 Received: by mail-pj1-x1030.google.com with SMTP id j12so5634020pjy.5 for ; Sat, 23 Jan 2021 07:23:35 -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=8lhTAwu1659Pg5CR5zuF6DTK+v2V2OGICDBdx5lHJrY=; b=rAnKuSOSrqaul8G4zb9WpvCT6PcrgnUuQ+o+GXAm7zT2fckrgD5queRDSj+PPGlDLu aDf/NCefVR3DvrrmzIujh6OVodKkqM/gb1lKx0ZOrEtjm1WoI8UhiYRsMJG+6zZPB4QG 90MAXuACbG3cKL9JLV+rzTLpcK+bN9PzwwJuTmD11CjCJO2OiyNbgeeytwKG4SniY02L zMF0Jim2b40YdX0qBlGKEa/mMAtmagDh+rPo1jCYDkdb6OdHVr0OOZSbnlhQKqy9/yOr ZAtZQy1qadFrSc5LM5GsOuAG/62c/K/upfMObRac9YveHpjKzGVcWwTRMtn4wuYOAOk/ t5fw== 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=8lhTAwu1659Pg5CR5zuF6DTK+v2V2OGICDBdx5lHJrY=; b=B54bcmxGVBXnyqLJryf6M7GLRBq0FRYpIt/4F5gTtYr1LhNl9WYE6/PlC25PYbBkRU RRN5Z/fKNPNmp+ov205gCJ/BWOUhcYl5XZQHbmlkxcuDTRVUTxl9JQ/e0rSEj3gY+LUZ VB+7mGyULHzlcVZCAFXXO188GbV2vgwjd3gtquYey9a/bHrK7EfwTaHXeXkCtu3i66qG vjjk9+1AcgMed9v/DH+i6QnPxi3fuAWscSjQIuR+gyNBrtNpNd2womAcfBCWVBAJ+VTl 2Xk5aRha+w0aN7qK8q4oW7uJtLZAXKsbHwlL/VeYlzKWXGfELeZELaYwP/VkUNmqBZLu 6zvw== X-Gm-Message-State: AOAM533MvfQ4yQ6KVbLjWDRanJK2P2p0BSh4kRNxvSHJbI8xVtH9k9GO KEhOvn8s7INQ2j9vmCBTFsiHcw== X-Google-Smtp-Source: ABdhPJylYm48YUIqYGNnCjmxdJnZanI/cWR1BF4OREpnVOXeX6ET7SCXePq87P6e7cHjagxTUZo8kg== X-Received: by 2002:a17:902:b688:b029:dc:240a:2bd7 with SMTP id c8-20020a170902b688b02900dc240a2bd7mr71476pls.50.1611415413439; Sat, 23 Jan 2021 07:23:33 -0800 (PST) Received: from leoy-ThinkPad-X240s ([64.120.119.108]) by smtp.gmail.com with ESMTPSA id c17sm11425254pfi.88.2021.01.23.07.23.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 23 Jan 2021 07:23:32 -0800 (PST) Date: Sat, 23 Jan 2021 23:23:25 +0800 From: Leo Yan To: Mike Leach Subject: Re: [PATCH v3] perf: cs-etm: update ETM metadata format Message-ID: <20210123152325.GA71576@leoy-ThinkPad-X240s> References: <20210119180110.6222-1-mike.leach@linaro.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210119180110.6222-1-mike.leach@linaro.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210123_102340_412519_43D30773 X-CRM114-Status: GOOD ( 40.17 ) 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@arm.com, mathieu.poirier@linaro.org, suzuki.poulose@arm.com, peterz@infradead.org, jolsa@redhat.com, coresight@lists.linaro.org, john.garry@huawei.com, linux-kernel@vger.kernel.org, acme@kernel.org, alexander.shishkin@linux.intel.com, mingo@redhat.com, namhyung@kernel.org, will@kernel.org, linux-arm-kernel@lists.infradead.org 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 19, 2021 at 06:01:10PM +0000, Mike Leach wrote: [...] > +/* > + * Read a single cpu parameter block from the auxtrace_info priv block. > + * > + * For version 1 there is a per cpu nr_params entry. If we are handling > + * version 1 file, then there may be less, the same, or more params > + * indicated by this value than the compile time number we understand. > + * > + * For a version 0 info block, there are a fixed number, and we need to > + * fill out the nr_param value in the metadata we create. > + */ > +static u64 *cs_etm__create_meta_blk(u64 *buff_in, int *buff_in_offset, > + int out_blk_size, int nr_params_v0) > +{ > + u64 *metadata = NULL; > + int hdr_version; > + int nr_in_params, nr_out_params, nr_cmn_params; > + int i, k; > + > + metadata = zalloc(sizeof(*metadata) * out_blk_size); > + if (!metadata) > + return NULL; > + > + /* read block current index & version */ > + i = *buff_in_offset; > + hdr_version = buff_in[CS_HEADER_VERSION]; > + > + if (!hdr_version) { > + /* read version 0 info block into a version 1 metadata block */ > + nr_in_params = nr_params_v0; > + metadata[CS_ETM_MAGIC] = buff_in[i + CS_ETM_MAGIC]; > + metadata[CS_ETM_CPU] = buff_in[i + CS_ETM_CPU]; > + metadata[CS_ETM_NR_TRC_PARAMS] = nr_in_params; > + /* remaining block params at offset +1 from source */ > + for (k = CS_ETM_ETMCR; k < nr_in_params; k++) > + metadata[k+1] = buff_in[i + k]; Here it misses to copy data into metadata[CS_ETM_ETMCR], you could see it starts to copy from offset 3 (k = CS_ETM_ETMCR) from buff_in, actually it's the item for CS_ETM_ETMTRACEIDR in the metadata v0. So need to change the logic as below; please confirm if I misunderstand or not. for (k = CS_ETM_ETMCR - 1; k < nr_in_params; k++) metadata[k+1] = buff_in[i + k]; Except this issue for ETMv3, I didn't find any other logic issue for ETMv4; I tested and confirmed the tool shows the consistent result for metadata v0 before and after applying this patch; and it handles metadata v1 correctly. So after fixed the issue for ETMv3, the change looks good to me: Reviewed-by: Leo Yan Tested-by: Leo Yan Thanks! > + /* version 0 has 2 common params */ > + nr_cmn_params = 2; > + } else { > + /* read version 1 info block - input and output nr_params may differ */ > + /* version 1 has 3 common params */ > + nr_cmn_params = 3; > + nr_in_params = buff_in[i + CS_ETM_NR_TRC_PARAMS]; > + > + /* if input has more params than output - skip excess */ > + nr_out_params = nr_in_params + nr_cmn_params; > + if (nr_out_params > out_blk_size) > + nr_out_params = out_blk_size; > + > + for (k = CS_ETM_MAGIC; k < nr_out_params; k++) > + metadata[k] = buff_in[i + k]; > + > + /* record the actual nr params we copied */ > + metadata[CS_ETM_NR_TRC_PARAMS] = nr_out_params - nr_cmn_params; > + } > + > + /* adjust in offset by number of in params used */ > + i += nr_in_params + nr_cmn_params; > + *buff_in_offset = i; > + return metadata; > +} > + > int cs_etm__process_auxtrace_info(union perf_event *event, > struct perf_session *session) > { > @@ -2492,11 +2635,12 @@ int cs_etm__process_auxtrace_info(union perf_event *event, > int info_header_size; > int total_size = auxtrace_info->header.size; > int priv_size = 0; > - int num_cpu; > - int err = 0, idx = -1; > - int i, j, k; > + int num_cpu, trcidr_idx; > + int err = 0; > + int i, j; > u64 *ptr, *hdr = NULL; > u64 **metadata = NULL; > + u64 hdr_version; > > /* > * sizeof(auxtrace_info_event::type) + > @@ -2512,16 +2656,21 @@ int cs_etm__process_auxtrace_info(union perf_event *event, > /* First the global part */ > ptr = (u64 *) auxtrace_info->priv; > > - /* Look for version '0' of the header */ > - if (ptr[0] != 0) > + /* Look for version of the header */ > + hdr_version = ptr[0]; > + if (hdr_version > CS_HEADER_CURRENT_VERSION) { > + /* print routine will print an error on bad version */ > + if (dump_trace) > + cs_etm__print_auxtrace_info(auxtrace_info->priv, 0); > return -EINVAL; > + } > > - hdr = zalloc(sizeof(*hdr) * CS_HEADER_VERSION_0_MAX); > + hdr = zalloc(sizeof(*hdr) * CS_HEADER_VERSION_MAX); > if (!hdr) > return -ENOMEM; > > /* Extract header information - see cs-etm.h for format */ > - for (i = 0; i < CS_HEADER_VERSION_0_MAX; i++) > + for (i = 0; i < CS_HEADER_VERSION_MAX; i++) > hdr[i] = ptr[i]; > num_cpu = hdr[CS_PMU_TYPE_CPUS] & 0xffffffff; > pmu_type = (unsigned int) ((hdr[CS_PMU_TYPE_CPUS] >> 32) & > @@ -2552,35 +2701,31 @@ int cs_etm__process_auxtrace_info(union perf_event *event, > */ > for (j = 0; j < num_cpu; j++) { > if (ptr[i] == __perf_cs_etmv3_magic) { > - metadata[j] = zalloc(sizeof(*metadata[j]) * > - CS_ETM_PRIV_MAX); > - if (!metadata[j]) { > - err = -ENOMEM; > - goto err_free_metadata; > - } > - for (k = 0; k < CS_ETM_PRIV_MAX; k++) > - metadata[j][k] = ptr[i + k]; > + metadata[j] = > + cs_etm__create_meta_blk(ptr, &i, > + CS_ETM_PRIV_MAX, > + CS_ETM_NR_TRC_PARAMS_V0); > > /* The traceID is our handle */ > - idx = metadata[j][CS_ETM_ETMTRACEIDR]; > - i += CS_ETM_PRIV_MAX; > + trcidr_idx = CS_ETM_ETMTRACEIDR; > + > } else if (ptr[i] == __perf_cs_etmv4_magic) { > - metadata[j] = zalloc(sizeof(*metadata[j]) * > - CS_ETMV4_PRIV_MAX); > - if (!metadata[j]) { > - err = -ENOMEM; > - goto err_free_metadata; > - } > - for (k = 0; k < CS_ETMV4_PRIV_MAX; k++) > - metadata[j][k] = ptr[i + k]; > + metadata[j] = > + cs_etm__create_meta_blk(ptr, &i, > + CS_ETMV4_PRIV_MAX, > + CS_ETMV4_NR_TRC_PARAMS_V0); > > /* The traceID is our handle */ > - idx = metadata[j][CS_ETMV4_TRCTRACEIDR]; > - i += CS_ETMV4_PRIV_MAX; > + trcidr_idx = CS_ETMV4_TRCTRACEIDR; > + } > + > + if (!metadata[j]) { > + err = -ENOMEM; > + goto err_free_metadata; > } > > /* Get an RB node for this CPU */ > - inode = intlist__findnew(traceid_list, idx); > + inode = intlist__findnew(traceid_list, metadata[j][trcidr_idx]); > > /* Something went wrong, no need to continue */ > if (!inode) { > @@ -2601,7 +2746,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event, > } > > /* > - * Each of CS_HEADER_VERSION_0_MAX, CS_ETM_PRIV_MAX and > + * Each of CS_HEADER_VERSION_MAX, CS_ETM_PRIV_MAX and > * CS_ETMV4_PRIV_MAX mark how many double words are in the > * global metadata, and each cpu's metadata respectively. > * The following tests if the correct number of double words was > @@ -2703,6 +2848,12 @@ int cs_etm__process_auxtrace_info(union perf_event *event, > intlist__delete(traceid_list); > err_free_hdr: > zfree(&hdr); > - > + /* > + * At this point, as a minimum we have valid header. Dump the rest of > + * the info section - the print routines will error out on structural > + * issues. > + */ > + if (dump_trace) > + cs_etm__print_auxtrace_info(auxtrace_info->priv, num_cpu); > return err; > } > diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h > index 4ad925d6d799..e153d02df0de 100644 > --- a/tools/perf/util/cs-etm.h > +++ b/tools/perf/util/cs-etm.h > @@ -17,23 +17,37 @@ struct perf_session; > */ > enum { > /* Starting with 0x0 */ > - CS_HEADER_VERSION_0, > + CS_HEADER_VERSION, > /* PMU->type (32 bit), total # of CPUs (32 bit) */ > CS_PMU_TYPE_CPUS, > CS_ETM_SNAPSHOT, > - CS_HEADER_VERSION_0_MAX, > + CS_HEADER_VERSION_MAX, > }; > > +/* > + * Update the version for new format. > + * > + * New version 1 format adds a param count to the per cpu metadata. > + * This allows easy adding of new metadata parameters. > + * Requires that new params always added after current ones. > + * Also allows client reader to handle file versions that are different by > + * checking the number of params in the file vs the number expected. > + */ > +#define CS_HEADER_CURRENT_VERSION 1 > + > /* Beginning of header common to both ETMv3 and V4 */ > enum { > CS_ETM_MAGIC, > CS_ETM_CPU, > + /* Number of trace config params in following ETM specific block */ > + CS_ETM_NR_TRC_PARAMS, > + CS_ETM_COMMON_BLK_MAX_V1, > }; > > /* ETMv3/PTM metadata */ > enum { > /* Dynamic, configurable parameters */ > - CS_ETM_ETMCR = CS_ETM_CPU + 1, > + CS_ETM_ETMCR = CS_ETM_COMMON_BLK_MAX_V1, > CS_ETM_ETMTRACEIDR, > /* RO, taken from sysFS */ > CS_ETM_ETMCCER, > @@ -41,10 +55,13 @@ enum { > CS_ETM_PRIV_MAX, > }; > > +/* define fixed version 0 length - allow new format reader to read old files. */ > +#define CS_ETM_NR_TRC_PARAMS_V0 (CS_ETM_ETMIDR - CS_ETM_ETMCR + 1) > + > /* ETMv4 metadata */ > enum { > /* Dynamic, configurable parameters */ > - CS_ETMV4_TRCCONFIGR = CS_ETM_CPU + 1, > + CS_ETMV4_TRCCONFIGR = CS_ETM_COMMON_BLK_MAX_V1, > CS_ETMV4_TRCTRACEIDR, > /* RO, taken from sysFS */ > CS_ETMV4_TRCIDR0, > @@ -55,6 +72,9 @@ enum { > CS_ETMV4_PRIV_MAX, > }; > > +/* define fixed version 0 length - allow new format reader to read old files. */ > +#define CS_ETMV4_NR_TRC_PARAMS_V0 (CS_ETMV4_TRCAUTHSTATUS - CS_ETMV4_TRCCONFIGR + 1) > + > /* > * ETMv3 exception encoding number: > * See Embedded Trace Macrocell spcification (ARM IHI 0014Q) > @@ -162,7 +182,7 @@ struct cs_etm_packet_queue { > > #define BMVAL(val, lsb, msb) ((val & GENMASK(msb, lsb)) >> lsb) > > -#define CS_ETM_HEADER_SIZE (CS_HEADER_VERSION_0_MAX * sizeof(u64)) > +#define CS_ETM_HEADER_SIZE (CS_HEADER_VERSION_MAX * sizeof(u64)) > > #define __perf_cs_etmv3_magic 0x3030303030303030ULL > #define __perf_cs_etmv4_magic 0x4040404040404040ULL > -- > 2.17.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel