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 87DEBC43381 for ; Mon, 11 Jan 2021 15:07:02 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 59B1522A84 for ; Mon, 11 Jan 2021 15:07:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1733047AbhAKPG4 (ORCPT ); Mon, 11 Jan 2021 10:06:56 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45598 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726459AbhAKPGz (ORCPT ); Mon, 11 Jan 2021 10:06:55 -0500 Received: from mail-pg1-x535.google.com (mail-pg1-x535.google.com [IPv6:2607:f8b0:4864:20::535]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 99BE4C061786 for ; Mon, 11 Jan 2021 07:06:15 -0800 (PST) Received: by mail-pg1-x535.google.com with SMTP id 30so12683789pgr.6 for ; Mon, 11 Jan 2021 07:06:15 -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=dOiO62KsX0NSWhPyER9EWahr6YjNU7/wBoB4Mtw/qG8=; b=I26iCUdkJfEx0eL31iUzJKV2NGdBGlmztRk2l8NK8xPkktIFW5IWvNoEP2hsV8zjMx i1xgmbxWlTLia+1PSEw+AJHXzbfB9CEzh5ZKH+62fcbJUj2iZgaVOmkSC9H5Z/I40TEk GZq0G17/BBzM6hFL7POwOT8QdIGBt445DdakOul8CW7fZd9Iq6SfgLLoWlq4m7yMSlT/ l2NBW9RJVqmtPHqJsOaS6/xx+QyjUQNPsPZH8Nn8CGZ30W2oBx6m2/6IsDMpzzJBlMJo 3wG2SGQ2/+YfIFVldeAezNkqg1eHTZ/4jmuV8Ni64dQbFHtGKKrekFIw14+QXiDBRE+v lv4g== 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=dOiO62KsX0NSWhPyER9EWahr6YjNU7/wBoB4Mtw/qG8=; b=TEHJ3Cut4dalQ++a/t27JKEaC/pSGBSyMU6LI8gc21XcGY3aVZqyT+YpmzbiJZygsS f77qs/zXmXgny8tsAnRD27XfY8QDT37uSA9j6bX3YKtrfLbY3iTfYPcnODVquCCljfpt veu6fhDMcOSbDcrBP8gJuVkAlwptpYByd/XeuzXjCJM4UIdRNTQmwzROcweubd9Y+84H BeUqrVdyOcw/+yq0bhJz55FGu2kJJi78ryZbqey/x+JzlLOKYLvVYFQXGZmgFdazzTh/ /auHB7YEKTM2EiVtO1WNYDUp2Ig0aD9POttZVpDsG2FHw1+KtYOwlFs9H6G45ZVDG2lv P4Ww== X-Gm-Message-State: AOAM532evP+Dtpj1gntYroMJglKo+1nqLAGSn12SfNba9VcTfelggefl 37VocRf56zEugptS0Bv/6Vu60Q== X-Google-Smtp-Source: ABdhPJyM+zLWM6rli5NSRUi4NFJ6N14FxaYpUYt+V4qKxrq6RHNrPGzbbWI8N8JhW19Eaf2LTFHNOg== X-Received: by 2002:a63:d917:: with SMTP id r23mr49303pgg.126.1610377574968; Mon, 11 Jan 2021 07:06:14 -0800 (PST) Received: from leoy-ThinkPad-X240s ([64.120.119.108]) by smtp.gmail.com with ESMTPSA id x1sm62153pgj.37.2021.01.11.07.06.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 11 Jan 2021 07:06:14 -0800 (PST) Date: Mon, 11 Jan 2021 23:06:08 +0800 From: Leo Yan To: Mike Leach Cc: Suzuki K Poulose , Arnaldo Carvalho de Melo , Mathieu Poirier , 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 Subject: Re: [PATCH v1 3/7] perf cs-etm: Calculate per CPU metadata array size Message-ID: <20210111150608.GC222747@leoy-ThinkPad-X240s> References: <20210109074435.626855-1-leo.yan@linaro.org> <20210109074435.626855-4-leo.yan@linaro.org> <96ec434e-4103-02ac-a05a-761a9ca8cb0d@arm.com> 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 Mon, Jan 11, 2021 at 12:09:12PM +0000, Mike Leach wrote: > Hi Leo, > > I think there is an issue here in that your modification assumes that > all cpus in the system are of the same ETM type. The original routine > allowed for differing ETM types, thus differing cpu ETM field lengths > between ETMv4 / ETMv3, the field size was used after the relevant > magic number for the cpu ETM was read. > > You have replaced two different sizes - with a single calculated size. Thanks for pointing out this. > Moving forwards we are seeing the newer FEAT_ETE protocol drivers > appearing on the list, which will ultimately need a new metadata > structure. > > We have had discussions within ARM regarding the changing of the > format to be more self describing - which should probably be opened > out to the CS mailing list. I think here have two options. One option is I think we can use __perf_cs_etmv3_magic/__perf_cs_etmv4_magic as indicator for the starting of next metadata array; when copy the metadata, always check the next item in the buffer, if it's __perf_cs_etmv3_magic or __perf_cs_etmv4_magic, will break loop and start copying metadata array for next CPU. The suggested change is pasted in below. Another option is I drop patches 03,05/07 in the series and leave the backward compatibility fixing for a saperate patch series with self describing method. Especially, if you think the first option will introduce trouble for enabling self describing later, then I am happy to drop patches 03,05. How about you think for this? Thanks, Leo ---8<--- diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index a2a369e2fbb6..edaec57362f0 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -2558,12 +2558,19 @@ int cs_etm__process_auxtrace_info(union perf_event *event, err = -ENOMEM; goto err_free_metadata; } - for (k = 0; k < CS_ETM_PRIV_MAX; k++) + for (k = 0; k < CS_ETM_PRIV_MAX; k++) { metadata[j][k] = ptr[i + k]; + if (ptr[i + k + 1] == __perf_cs_etmv3_magic || + ptr[i + k + 1] == __perf_cs_etmv4_magic) { + k++; + break; + } + } + /* The traceID is our handle */ idx = metadata[j][CS_ETM_ETMTRACEIDR]; - i += CS_ETM_PRIV_MAX; + i += k; } else if (ptr[i] == __perf_cs_etmv4_magic) { metadata[j] = zalloc(sizeof(*metadata[j]) * CS_ETMV4_PRIV_MAX); @@ -2571,12 +2578,19 @@ int cs_etm__process_auxtrace_info(union perf_event *event, err = -ENOMEM; goto err_free_metadata; } - for (k = 0; k < CS_ETMV4_PRIV_MAX; k++) + for (k = 0; k < CS_ETMV4_PRIV_MAX; k++) { metadata[j][k] = ptr[i + k]; + if (ptr[i + k + 1] == __perf_cs_etmv3_magic || + ptr[i + k + 1] == __perf_cs_etmv4_magic) { + k++; + break; + } + } + /* The traceID is our handle */ idx = metadata[j][CS_ETMV4_TRCTRACEIDR]; - i += CS_ETMV4_PRIV_MAX; + i += k; } /* Get an RB node for this CPU */ 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 6E5C7C433E0 for ; Mon, 11 Jan 2021 15:08:49 +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 17F0822795 for ; Mon, 11 Jan 2021 15:08:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 17F0822795 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=xaIQgdGjoeS+4rFG46Nv/pDklEqkuNT+zNvZQVEmtqI=; b=kLSHCzVPK5NCtdLt8ImPctKXS W8jToDhOOT2pUNQUATXPBD23VsXV0YiSPw3rwZ8LhSkBvUD9lsnpeNYq6NR5VzB1ix17gxuwt9DeQ lkFHm4q5ZCvVK4+jMhPf4mMR2quhZgJzpuMxHMhAjEmnkMXlNEjdvqQKCjd/oNQ4mB7J1OFaqEi3/ tFBm19FZOj2WJLaQyW8vBUiqRwfN1SMXVPMHltnen+fNCyeebmvirF4EyBGe/skzptEiNz/wjHXgc g39QcmkeDwvn5i37rLTjtb2P1MRLCrUOU9b8Cva7l6cWWFz6ZOdjmS2tjh9K6vJzBslD6nnJHTM/z /KZAUHebw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kyylx-0007NP-ST; Mon, 11 Jan 2021 15:06:21 +0000 Received: from mail-pf1-x42a.google.com ([2607:f8b0:4864:20::42a]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kyylt-0007Lv-Ga for linux-arm-kernel@lists.infradead.org; Mon, 11 Jan 2021 15:06:18 +0000 Received: by mail-pf1-x42a.google.com with SMTP id c12so60457pfo.10 for ; Mon, 11 Jan 2021 07:06:16 -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=dOiO62KsX0NSWhPyER9EWahr6YjNU7/wBoB4Mtw/qG8=; b=I26iCUdkJfEx0eL31iUzJKV2NGdBGlmztRk2l8NK8xPkktIFW5IWvNoEP2hsV8zjMx i1xgmbxWlTLia+1PSEw+AJHXzbfB9CEzh5ZKH+62fcbJUj2iZgaVOmkSC9H5Z/I40TEk GZq0G17/BBzM6hFL7POwOT8QdIGBt445DdakOul8CW7fZd9Iq6SfgLLoWlq4m7yMSlT/ l2NBW9RJVqmtPHqJsOaS6/xx+QyjUQNPsPZH8Nn8CGZ30W2oBx6m2/6IsDMpzzJBlMJo 3wG2SGQ2/+YfIFVldeAezNkqg1eHTZ/4jmuV8Ni64dQbFHtGKKrekFIw14+QXiDBRE+v lv4g== 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=dOiO62KsX0NSWhPyER9EWahr6YjNU7/wBoB4Mtw/qG8=; b=tYw7WsZi2cAosCvtiRnlu9nwOmyz24lKW1/HyR/WDw0IBEXJbt8D3PO03AVA1MAT1K o1vWpBhFArfWoPQYubGxLplISESJmaIlVQg3OpZjHMHn5TsHMasQcu/wmykPbEhqrikw JJDM6csATjgbH1izsAXxcC08wMOhbA18FUIXZcV33xVjiuLXZx2QhQJEnW3r6iyAykCq 96mMLJgQwB0CTOrBmOo1kMT9MGuIQiiFfc93BErgi87lIWAT3/GJYAQ6Clng/gbuGI+G p71zX5CASD917JU5zwgkV0CJQzmF99y62+OdQcm8X0cq6L2ztk8S5atdmAXzOOH73WVi UP9A== X-Gm-Message-State: AOAM530fPeB8cOvsjy7a1GI1xR6eXL2PC++sV2VaMdgEUfLohmVkLPfY 6WYDMQFKiiVQQD2ub8niWDCv3g== X-Google-Smtp-Source: ABdhPJyM+zLWM6rli5NSRUi4NFJ6N14FxaYpUYt+V4qKxrq6RHNrPGzbbWI8N8JhW19Eaf2LTFHNOg== X-Received: by 2002:a63:d917:: with SMTP id r23mr49303pgg.126.1610377574968; Mon, 11 Jan 2021 07:06:14 -0800 (PST) Received: from leoy-ThinkPad-X240s ([64.120.119.108]) by smtp.gmail.com with ESMTPSA id x1sm62153pgj.37.2021.01.11.07.06.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 11 Jan 2021 07:06:14 -0800 (PST) Date: Mon, 11 Jan 2021 23:06:08 +0800 From: Leo Yan To: Mike Leach Subject: Re: [PATCH v1 3/7] perf cs-etm: Calculate per CPU metadata array size Message-ID: <20210111150608.GC222747@leoy-ThinkPad-X240s> References: <20210109074435.626855-1-leo.yan@linaro.org> <20210109074435.626855-4-leo.yan@linaro.org> <96ec434e-4103-02ac-a05a-761a9ca8cb0d@arm.com> 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-20210111_100617_686325_0B106A46 X-CRM114-Status: GOOD ( 26.72 ) 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 , 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 Mon, Jan 11, 2021 at 12:09:12PM +0000, Mike Leach wrote: > Hi Leo, > > I think there is an issue here in that your modification assumes that > all cpus in the system are of the same ETM type. The original routine > allowed for differing ETM types, thus differing cpu ETM field lengths > between ETMv4 / ETMv3, the field size was used after the relevant > magic number for the cpu ETM was read. > > You have replaced two different sizes - with a single calculated size. Thanks for pointing out this. > Moving forwards we are seeing the newer FEAT_ETE protocol drivers > appearing on the list, which will ultimately need a new metadata > structure. > > We have had discussions within ARM regarding the changing of the > format to be more self describing - which should probably be opened > out to the CS mailing list. I think here have two options. One option is I think we can use __perf_cs_etmv3_magic/__perf_cs_etmv4_magic as indicator for the starting of next metadata array; when copy the metadata, always check the next item in the buffer, if it's __perf_cs_etmv3_magic or __perf_cs_etmv4_magic, will break loop and start copying metadata array for next CPU. The suggested change is pasted in below. Another option is I drop patches 03,05/07 in the series and leave the backward compatibility fixing for a saperate patch series with self describing method. Especially, if you think the first option will introduce trouble for enabling self describing later, then I am happy to drop patches 03,05. How about you think for this? Thanks, Leo ---8<--- diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index a2a369e2fbb6..edaec57362f0 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -2558,12 +2558,19 @@ int cs_etm__process_auxtrace_info(union perf_event *event, err = -ENOMEM; goto err_free_metadata; } - for (k = 0; k < CS_ETM_PRIV_MAX; k++) + for (k = 0; k < CS_ETM_PRIV_MAX; k++) { metadata[j][k] = ptr[i + k]; + if (ptr[i + k + 1] == __perf_cs_etmv3_magic || + ptr[i + k + 1] == __perf_cs_etmv4_magic) { + k++; + break; + } + } + /* The traceID is our handle */ idx = metadata[j][CS_ETM_ETMTRACEIDR]; - i += CS_ETM_PRIV_MAX; + i += k; } else if (ptr[i] == __perf_cs_etmv4_magic) { metadata[j] = zalloc(sizeof(*metadata[j]) * CS_ETMV4_PRIV_MAX); @@ -2571,12 +2578,19 @@ int cs_etm__process_auxtrace_info(union perf_event *event, err = -ENOMEM; goto err_free_metadata; } - for (k = 0; k < CS_ETMV4_PRIV_MAX; k++) + for (k = 0; k < CS_ETMV4_PRIV_MAX; k++) { metadata[j][k] = ptr[i + k]; + if (ptr[i + k + 1] == __perf_cs_etmv3_magic || + ptr[i + k + 1] == __perf_cs_etmv4_magic) { + k++; + break; + } + } + /* The traceID is our handle */ idx = metadata[j][CS_ETMV4_TRCTRACEIDR]; - i += CS_ETMV4_PRIV_MAX; + i += k; } /* Get an RB node for this CPU */ _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel