Linux Perf Users
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Leo Yan <leo.yan@arm.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
	Besar Wicaksono <bwicaksono@nvidia.com>,
	Will Deacon <will@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Ian Rogers <irogers@google.com>, Jiri Olsa <jolsa@kernel.org>,
	Kan Liang <kan.liang@linux.intel.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-perf-users@vger.kernel.org,
	James Clark <james.clark@arm.com>
Subject: Re: [RFC/PATCH 1/1] tools headers arm64: Sync arm64's cputype.h with the kernel sources
Date: Tue, 4 Jun 2024 19:55:42 +0100	[thread overview]
Message-ID: <Zl9jLtiFagBcH7oH@J2N7QTR9R3> (raw)
In-Reply-To: <a1990b2e-2c10-42b6-93e9-fef324cb91b2@arm.com>

On Tue, Jun 04, 2024 at 06:14:22PM +0100, Leo Yan wrote:
> On 6/4/24 10:11, Mark Rutland wrote:
> > Hi Arnaldo,
> > 
> > On Mon, Jun 03, 2024 at 03:33:07PM -0300, Arnaldo Carvalho de Melo wrote:
> > > To get the changes in:
> > > 
> > >    0ce85db6c2141b7f ("arm64: cputype: Add Neoverse-V3 definitions")
> > >    02a0a04676fa7796 ("arm64: cputype: Add Cortex-X4 definitions")
> > >    f4d9d9dcc70b96b5 ("arm64: Add Neoverse-V2 part")
> > 
> > As a heads-up, there are likely to be a couple more updates here
> > shortly:
> > 
> >    https://lore.kernel.org/linux-arm-kernel/20240603111812.1514101-1-mark.rutland@arm.com/
> > 
> > > That makes this perf source code to be rebuilt:
> > > 
> > >    CC      /tmp/build/perf-tools/util/arm-spe.o
> > > 
> > > The changes in the above patch add MIDR_NEOVERSE_V[23] and
> > > MIDR_NEOVERSE_V1 is used in arm-spe.c, so probably we need to add those
> > > and perhaps MIDR_CORTEX_X4 to that array? Or maybe we need to leave this
> > > for later when this is all tested on those machines?
> > 
> > Hmm... looking at where that was added this is somewhat misnamed, this
> > is really saying that these cores use the same IMPLEMENTATION DEFINED
> > encoding of the source field. That's not really a property of Neoverse
> > specifically, and I'm not sure what Arm's policy is here going forwards.
> > 
> > We should probably rename that to something like
> > common_data_source_encoding, with a big comment about exactly what it
> > implies.
> 
> Agreed.
> 
> I went through Neoverse-V2/V3/V4, Cortex-X4, Cortex-X4, Cortex-A720, and
> Cortex-X925, all of them have the same definition for data source packet
> format. It makes sense to change name to Neoverse specific to a more general
> name.

Cool.

> > I would not touch this for now -- someone would have to go audit the
> > TRMs to check that those other cores have the same encoding, and I think
> > it'd be better to do that as a follow-up.
> 
> So far, it's fine to not change the file util/arm-spe.c.
> 
> Now more and more Arm CPUs support the data source in SPE and share the same
> data source format. It's not scalable for us to adding every CPU variant
> into the file util/arm-spe.c.
> 
> I would like to expose the PMSIDR_EL1.LDS bit (Data source indicator for
> sampled load instructions) via the 'cap' folder, and then we can save this
> info into the perf meta data during record phase.

I'd be happy to expose fields from PMSIDR_EL1.

> In the perf report, we can parse the meta data and if the
> PMSIDR_EL1.LDS bit is set, the tool will parse the data source packet
> based on the common format.

I don't believe that's right.

PMSIDR_EL1.LDS indicates that the loaded data source field is
implemented, but even when it is implemented, the format is
IMPLEMENTATION DEFINED.

Today, Arm Ltd implementations happen to share a format, but that isn't
implied by PMSIDR_EL1.LDS, and there's no guarantee that future CPUs
will all use the same format.

For the moment we'll have to keep adding to this list.

Mark.

  reply	other threads:[~2024-06-04 18:55 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-03 18:33 [RFC/PATCH 1/1] tools headers arm64: Sync arm64's cputype.h with the kernel sources Arnaldo Carvalho de Melo
2024-06-04  9:11 ` Mark Rutland
2024-06-04 13:53   ` Arnaldo Carvalho de Melo
2024-06-04 14:34     ` Mark Rutland
2024-06-04 18:55       ` Arnaldo Carvalho de Melo
2024-06-04 17:14   ` Leo Yan
2024-06-04 18:55     ` Mark Rutland [this message]
2024-06-04 20:01       ` Leo Yan
2024-06-05  9:32         ` Mark Rutland
2024-08-08  4:37           ` Besar Wicaksono
2024-08-08  8:35             ` 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=Zl9jLtiFagBcH7oH@J2N7QTR9R3 \
    --to=mark.rutland@arm.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=bwicaksono@nvidia.com \
    --cc=irogers@google.com \
    --cc=james.clark@arm.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=leo.yan@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=namhyung@kernel.org \
    --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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox