From: Leo Yan <leo.yan@linaro.org> To: Mike Leach <mike.leach@linaro.org> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>, Arnaldo Carvalho de Melo <acme@kernel.org>, Mathieu Poirier <mathieu.poirier@linaro.org>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, John Garry <john.garry@huawei.com>, Will Deacon <will@kernel.org>, Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, Mark Rutland <mark.rutland@arm.com>, Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>, Daniel Kiss <Daniel.Kiss@arm.com>, Denis Nikitin <denik@chromium.org>, Coresight ML <coresight@lists.linaro.org>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v1 3/7] perf cs-etm: Calculate per CPU metadata array size Date: Mon, 11 Jan 2021 23:06:08 +0800 [thread overview] Message-ID: <20210111150608.GC222747@leoy-ThinkPad-X240s> (raw) In-Reply-To: <CAJ9a7VjtUuRRYBBu63kSXKwrGdB8ZoWJz-bE1g9tMLSbkFVDGg@mail.gmail.com> 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 */
WARNING: multiple messages have this Message-ID (diff)
From: Leo Yan <leo.yan@linaro.org> To: Mike Leach <mike.leach@linaro.org> Cc: Mark Rutland <mark.rutland@arm.com>, Denis Nikitin <denik@chromium.org>, Mathieu Poirier <mathieu.poirier@linaro.org>, Suzuki K Poulose <suzuki.poulose@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@redhat.com>, Coresight ML <coresight@lists.linaro.org>, John Garry <john.garry@huawei.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Arnaldo Carvalho de Melo <acme@kernel.org>, Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, Namhyung Kim <namhyung@kernel.org>, Will Deacon <will@kernel.org>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, Daniel Kiss <Daniel.Kiss@arm.com> Subject: Re: [PATCH v1 3/7] perf cs-etm: Calculate per CPU metadata array size Date: Mon, 11 Jan 2021 23:06:08 +0800 [thread overview] Message-ID: <20210111150608.GC222747@leoy-ThinkPad-X240s> (raw) In-Reply-To: <CAJ9a7VjtUuRRYBBu63kSXKwrGdB8ZoWJz-bE1g9tMLSbkFVDGg@mail.gmail.com> 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
next prev parent reply other threads:[~2021-01-11 15:07 UTC|newest] Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-01-09 7:44 [PATCH v1 0/7] coresight: etm-perf: Fix pid tracing with VHE Leo Yan 2021-01-09 7:44 ` Leo Yan 2021-01-09 7:44 ` [PATCH v1 1/7] coresight: etm-perf: Add support for PID tracing for kernel at EL2 Leo Yan 2021-01-09 7:44 ` Leo Yan 2021-01-09 10:05 ` kernel test robot 2021-01-10 1:24 ` [PATCH] " Suzuki K Poulose 2021-01-10 1:41 ` Leo Yan 2021-01-10 22:34 ` Suzuki K Poulose 2021-01-11 0:05 ` Leo Yan 2021-01-09 11:24 ` [PATCH v1 1/7] " kernel test robot 2021-01-11 16:22 ` Mike Leach 2021-01-11 16:22 ` Mike Leach 2021-01-12 7:22 ` Leo Yan 2021-01-12 7:22 ` Leo Yan 2021-01-12 8:58 ` Leo Yan 2021-01-12 8:58 ` Leo Yan 2021-01-12 11:03 ` Suzuki K Poulose 2021-01-12 11:03 ` Suzuki K Poulose 2021-01-12 11:23 ` Mike Leach 2021-01-12 11:23 ` Mike Leach 2021-01-12 14:14 ` Leo Yan 2021-01-12 14:14 ` Leo Yan 2021-01-12 23:43 ` Mike Leach 2021-01-12 23:43 ` Mike Leach 2021-01-15 22:30 ` Mathieu Poirier 2021-01-15 22:30 ` Mathieu Poirier 2021-01-19 7:05 ` Suzuki K Poulose 2021-01-19 7:05 ` Suzuki K Poulose 2021-01-09 7:44 ` [PATCH v1 2/7] perf cs_etm: Use pid tracing explicitly instead of contextid Leo Yan 2021-01-09 7:44 ` Leo Yan 2021-01-15 22:44 ` Mathieu Poirier 2021-01-15 22:44 ` Mathieu Poirier 2021-01-19 2:32 ` Leo Yan 2021-01-19 2:32 ` Leo Yan 2021-01-09 7:44 ` [PATCH v1 3/7] perf cs-etm: Calculate per CPU metadata array size Leo Yan 2021-01-09 7:44 ` Leo Yan 2021-01-11 7:28 ` Suzuki K Poulose 2021-01-11 7:28 ` Suzuki K Poulose 2021-01-11 12:09 ` Mike Leach 2021-01-11 12:09 ` Mike Leach 2021-01-11 15:06 ` Leo Yan [this message] 2021-01-11 15:06 ` Leo Yan 2021-01-13 0:00 ` Mike Leach 2021-01-13 0:00 ` Mike Leach 2021-01-13 2:27 ` Leo Yan 2021-01-13 2:27 ` Leo Yan 2021-01-15 22:46 ` Mathieu Poirier 2021-01-15 22:46 ` Mathieu Poirier 2021-01-16 0:50 ` Leo Yan 2021-01-16 0:50 ` Leo Yan 2021-01-09 7:44 ` [PATCH v1 4/7] perf cs-etm: Add PID format into metadata Leo Yan 2021-01-09 7:44 ` Leo Yan 2021-01-11 9:45 ` Suzuki K Poulose 2021-01-11 9:45 ` Suzuki K Poulose 2021-01-11 13:12 ` Leo Yan 2021-01-11 13:12 ` Leo Yan 2021-01-09 7:44 ` [PATCH v1 5/7] perf cs-etm: Fixup PID_FMT when it is zero Leo Yan 2021-01-09 7:44 ` Leo Yan 2021-01-11 9:47 ` Suzuki K Poulose 2021-01-11 9:47 ` Suzuki K Poulose 2021-01-09 7:44 ` [PATCH v1 6/7] perf cs-etm: Add helper cs_etm__get_pid_fmt() Leo Yan 2021-01-09 7:44 ` Leo Yan 2021-01-11 9:55 ` Suzuki K Poulose 2021-01-11 9:55 ` Suzuki K Poulose 2021-01-09 7:44 ` [PATCH v1 7/7] perf cs-etm: Detect pid in VMID for kernel running at EL2 Leo Yan 2021-01-09 7:44 ` Leo Yan 2021-01-11 10:07 ` Suzuki K Poulose 2021-01-11 10:07 ` Suzuki K Poulose 2021-01-11 13:10 ` Leo Yan 2021-01-11 13:10 ` Leo Yan 2021-01-11 18:16 ` [PATCH v1 0/7] coresight: etm-perf: Fix pid tracing with VHE Mathieu Poirier 2021-01-11 18:16 ` Mathieu Poirier 2021-01-12 7:23 ` Leo Yan 2021-01-12 7:23 ` Leo Yan
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210111150608.GC222747@leoy-ThinkPad-X240s \ --to=leo.yan@linaro.org \ --cc=Daniel.Kiss@arm.com \ --cc=acme@kernel.org \ --cc=alexander.shishkin@linux.intel.com \ --cc=coresight@lists.linaro.org \ --cc=denik@chromium.org \ --cc=john.garry@huawei.com \ --cc=jolsa@redhat.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=mathieu.poirier@linaro.org \ --cc=mike.leach@linaro.org \ --cc=mingo@redhat.com \ --cc=namhyung@kernel.org \ --cc=peterz@infradead.org \ --cc=suzuki.poulose@arm.com \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.