Re: [PATCH 3/4] perf arm_spe: Decode Arm N1 IMPDEF events

[Namhyung Kim <namhyung@kernel.org>]

On Wed, Apr 08, 2026 at 09:47:41AM +0100, James Clark wrote:
> 
> 
> On 07/04/2026 7:26 pm, Ian Rogers wrote:
> > On Tue, Apr 7, 2026 at 5:35 AM James Clark <james.clark@linaro.org> wrote:
> > > 
> > > 
> > > 
> > > On 02/04/2026 4:26 pm, Ian Rogers wrote:
> > > > On Wed, Apr 1, 2026 at 7:26 AM James Clark <james.clark@linaro.org> wrote:
> > > > > 
> > > > >   From the TRM [1], N1 has one IMPDEF event which isn't covered by the
> > > > > common list. Add a framework so that more cores can be added in the
> > > > > future and that the N1 IMPDEF event can be decoded. Also increase the
> > > > > size of the buffer because we're adding more strings and if it gets
> > > > > truncated it falls back to a hex dump only.
> > > > > 
> > > > > [1]: https://developer.arm.com/documentation/100616/0401/Statistical-Profiling-Extension/implementation-defined-features-of-SPE
> > > > > Suggested-by: Al Grant <al.grant@arm.com>
> > > > > Signed-off-by: James Clark <james.clark@linaro.org>
> > > > > ---
> > > > >    tools/perf/util/arm-spe-decoder/Build              |  2 +
> > > > >    .../util/arm-spe-decoder/arm-spe-pkt-decoder.c     | 45 ++++++++++++++++++++--
> > > > >    .../util/arm-spe-decoder/arm-spe-pkt-decoder.h     |  5 ++-
> > > > >    tools/perf/util/arm-spe.c                          | 13 ++++---
> > > > >    4 files changed, 54 insertions(+), 11 deletions(-)
> > > > > 
> > > > > diff --git a/tools/perf/util/arm-spe-decoder/Build b/tools/perf/util/arm-spe-decoder/Build
> > > > > index ab500e0efe24..97a298d1e279 100644
> > > > > --- a/tools/perf/util/arm-spe-decoder/Build
> > > > > +++ b/tools/perf/util/arm-spe-decoder/Build
> > > > > @@ -1 +1,3 @@
> > > > >    perf-util-y += arm-spe-pkt-decoder.o arm-spe-decoder.o
> > > > > +
> > > > > +CFLAGS_arm-spe-pkt-decoder.o += -I$(srctree)/tools/arch/arm64/include/ -I$(OUTPUT)arch/arm64/include/generated/
> > > > > diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> > > > > index c880b0dec3a1..42a7501d4dfe 100644
> > > > > --- a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> > > > > +++ b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> > > > > @@ -15,6 +15,8 @@
> > > > > 
> > > > >    #include "arm-spe-pkt-decoder.h"
> > > > > 
> > > > > +#include "../../arm64/include/asm/cputype.h"
> > > > 
> > > > Sashiko spotted:
> > > > https://sashiko.dev/#/patchset/20260401-james-spe-impdef-decode-v1-0-ad0d372c220c%40linaro.org
> > > > """
> > > > This isn't a bug, but does this include directive rely on accidental
> > > > path normalization?
> > > > 
> > > > The relative path ../../arm64/include/asm/cputype.h does not exist relative
> > > > to arm-spe-pkt-decoder.c. It only compiles because the Build file adds
> > > > -I$(srctree)/tools/arch/arm64/include/ to CFLAGS.
> > > > 
> > > > Would it be cleaner to use #include <asm/cputype.h> to explicitly rely on
> > > > the include path?
> > > > [ ... ]
> > > > """
> > > > I wouldn't use <asm/cputype.h> due to cross-compilation and the like,
> > > > instead just add the extra "../" into the include path.
> > > > 
> > > 
> > > Do you mean change the #include to this?
> > > 
> > >     #include "../../../arm64/include/asm/cputype.h"
> > > 
> > > I still need to add:
> > > 
> > >     CFLAGS_arm-spe-pkt-decoder.o += -I$(srctree)/tools/arch/arm64/include/
> > > 
> > > To make the this include in cputype.h work:
> > > 
> > >     #include <asm/sysreg.h>
> > > 
> > > Which probably only works because there isn't a sysreg.h on other
> > > architectures. But I'm not sure what the significance of ../../ vs
> > > ../../../ is if either compile? arm-spe.c already does it with ../../
> > > which is what I copied.
> > 
> > Hmm.. maybe the path should be
> > "../../../arch/arm64/include/asm/cputype.h". The include preference is
> > for a path relative to the source file and
> > ../../arm64/include/asm/cputype.h doesn't exist. It is kind of horrid
> 
> Up 3 dirs from arm-spe-pkt-decoder.c would be
> "tools/arm64/include/asm/cputype.h" which doesn't exist either unless I'm
> missing something?
> 
> If get the compiler to print the path it uses with 3 then it would use the
> x86 uapi include path which doesn't seem any less weird to me:
> 
>  ...
>  In file included from util/arm-spe-decoder/arm-spe-pkt-decoder.c:19:
> 
>  linux/tools/arch/x86/include/uapi/../../../arm64/include/asm/cputype.h:254:10:
> 
> 
> > to add an include path and then use a relative path to escape into a
> > higher-level directory. arm-spe.c is a little different as it is one
> > directory higher in the directory layout.
> > 
> 
> It is one folder higher, but arm-spe.c still relies on adding a special
> include path to CFLAGS_arm-spe.o to make it work. It's not including a real
> path relative to the source either.
> 
> Yeah it's a bit horrible but I don't think the asm/ thing combined with
> copying headers from the kernel to tools expected to handle the case where
> we would want to use asm/ stuff for a different arch than the compile one.
> It might not be normal to use relative include paths to escape to higher
> directories, but it's not a normal situation either. I think it's a special
> case for a special scenario. I'm not sure of a better way, but this is
> working for now.

I hope we can cleanup the header inclusion path someday.  My idea is
that it always starts from the root of the perf directory.  So it'd
include "util/xxx.h" even if the source file is already in util/.

Thanks,
Namhyung