Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210109074435.626855-1-leo.yan@linaro.org> <20210109074435.626855-4-leo.yan@linaro.org>
 <96ec434e-4103-02ac-a05a-761a9ca8cb0d@arm.com>
In-Reply-To: <96ec434e-4103-02ac-a05a-761a9ca8cb0d@arm.com>
From:   Mike Leach <mike.leach@linaro.org>
Date:   Mon, 11 Jan 2021 12:09:12 +0000
Message-ID: <CAJ9a7VjtUuRRYBBu63kSXKwrGdB8ZoWJz-bE1g9tMLSbkFVDGg@mail.gmail.com>
Subject: Re: [PATCH v1 3/7] perf cs-etm: Calculate per CPU metadata array size
To:     Suzuki K Poulose <suzuki.poulose@arm.com>
Cc:     Leo Yan <leo.yan@linaro.org>,
        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>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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.

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.

Regards

Mike


On Mon, 11 Jan 2021 at 07:29, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> On 1/9/21 7:44 AM, Leo Yan wrote:
> > The metadata array can be extended over time and the tool, if using the
> > predefined macro (like CS_ETMV4_PRIV_MAX for ETMv4) as metadata array
> > size to copy data, it can cause compatible issue within different
> > versions of perf tool.
> >
> > E.g. we recorded a data file with an old version tool, afterwards if
> > use the new version perf tool to parse the file, since the metadata
> > array has been extended and the macro CS_ETMV4_PRIV_MAX has been
> > altered, if use it to parse the perf data with old format, this will
> > lead to mismatch.
> >
> > To maintain backward compatibility, this patch calculates per CPU
> > metadata array size on the runtime, the calculation is based on the
> > info stored in the data file so that it's reliable.
> >
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
>
> Looks good to me.
>
> Acked-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>


--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK