Re: [PATCH 0/4] objtool,perf: Use shared x86 insn decoder

From: Arnaldo Carvalho de Melo
Date: Fri Aug 30 2019 - 15:01:15 EST

Next message: David Dai: "[v2] net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate"
Previous message: Krishna Reddy: "RE: [PATCH 3/7] iommu/arm-smmu: Add tlb_sync implementation hook"
In reply to: Arnaldo Carvalho de Melo: "Re: [PATCH 0/4] objtool,perf: Use shared x86 insn decoder"
Next in thread: Josh Poimboeuf: "Re: [PATCH 0/4] objtool,perf: Use shared x86 insn decoder"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Em Fri, Aug 30, 2019 at 03:40:20PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Aug 29, 2019 at 05:41:17PM -0500, Josh Poimboeuf escreveu:
> > It's kind of silly that we have *three* identical copies of the x86 insn
> > decoder in the kernel tree. Make it approximately 50% less silly by
> > reducing that number to two.

> Ok, I fixed up some minor conflicts with my perf/core branch and will
> submit this together with the other things to the container builds and
> then push it upstream, collected Peter and Masami's Acks.

Ok, I quickly take that back (but a fix is provided below), as I noticed
that it is failing in the first cross-builds I tried:

perfbuilder@1317a113fde0:~$ export | grep HOME -B20
declare -x ARCH="arm64"
declare -x CROSS_COMPILE="aarch64-linux-gnu-"
declare -x EXTRA_MAKE_ARGS="CORESIGHT=1"
declare -x HOME="/home/perfbuilder"
perfbuilder@1317a113fde0:~$

I completely removed the build dir and restarted the build just in case,
but got:

CC /tmp/build/perf/util/intel-pt-decoder/intel-pt-insn-decoder.o
util/intel-pt-decoder/intel-pt-insn-decoder.c:12:10: fatal error: asm/insn.h: No such file or directory
#include <asm/insn.h>
^~~~~~~~~~~~
compilation terminated.
mv: cannot stat '/tmp/build/perf/util/intel-pt-decoder/.intel-pt-insn-decoder.o.tmp': No such file or directory
make[5]: *** [util/intel-pt-decoder/Build:14: /tmp/build/perf/util/intel-pt-decoder/intel-pt-insn-decoder.o] Error 1

Also on:

perfbuilder@f3b14e504747:~$ export | grep HOME -B20
declare -x ARCH="s390"
declare -x CROSS_COMPILE="s390x-linux-gnu-"
declare -x HOME="/home/perfbuilder"
perfbuilder@f3b14e504747:~$

CC /tmp/build/perf/util/intel-pt-decoder/intel-pt-insn-decoder.o
CC /tmp/build/perf/util/demangle-rust.o
util/intel-pt-decoder/intel-pt-insn-decoder.c:12:22: fatal error: asm/insn.h: No such file or directory
compilation terminated.
mv: cannot stat '/tmp/build/perf/util/intel-pt-decoder/.intel-pt-insn-decoder.o.tmp': No such file or directory
util/intel-pt-decoder/Build:13: recipe for target '/tmp/build/perf/util/intel-pt-decoder/intel-pt-insn-decoder.o' failed

All built with:

perfbuilder@1317a113fde0:~$ alias m
alias m='make $EXTRA_MAKE_ARGS ARCH=$ARCH CROSS_COMPILE=$CROSS_COMPILE EXTRA_CFLAGS="$EXTRA_CFLAGS" -C /git/perf/tools/perf O=/tmp/build/perf'
perfbuilder@1317a113fde0:~$

I.e. we need to make sure that it always gets the x86 stuff, not
something that is tied to the host arch, with the patch below we get it
to work, please take a look.

Probably this should go to the master copy, i.e. to the kernel sources,
no?

That or we'll have to ask the check-headers.sh and objtool sync-check
(hey, this should be unified, each project could provide just the list
of things it uses, but I digress) to ignore those lines...

I.e. we want to decode intel_PT traces on other arches, ditto for
CoreSight (not affected here, but similar concept).

will kick the full container build process now.

- Arnaldo

diff --git a/tools/arch/x86/include/asm/inat.h b/tools/arch/x86/include/asm/inat.h
index 4cf2ad521f65..877827b7c2c3 100644
--- a/tools/arch/x86/include/asm/inat.h
+++ b/tools/arch/x86/include/asm/inat.h
@@ -6,7 +6,7 @@
*
* Written by Masami Hiramatsu <mhiramat@xxxxxxxxxx>
*/
-#include <asm/inat_types.h>
+#include "inat_types.h"

/*
* Internal bits. Don't use bitmasks directly, because these bits are
diff --git a/tools/arch/x86/include/asm/insn.h b/tools/arch/x86/include/asm/insn.h
index 154f27be8bfc..37a4c390750b 100644
--- a/tools/arch/x86/include/asm/insn.h
+++ b/tools/arch/x86/include/asm/insn.h
@@ -8,7 +8,7 @@
*/

/* insn_attr_t is defined in inat.h */
-#include <asm/inat.h>
+#include "inat.h"

struct insn_field {
union {
diff --git a/tools/arch/x86/lib/inat.c b/tools/arch/x86/lib/inat.c
index 12539fca75c4..4f5ed49e1b4e 100644
--- a/tools/arch/x86/lib/inat.c
+++ b/tools/arch/x86/lib/inat.c
@@ -4,7 +4,7 @@
*
* Written by Masami Hiramatsu <mhiramat@xxxxxxxxxx>
*/
-#include <asm/insn.h>
+#include "../include/asm/insn.h"

/* Attribute tables are generated from opcode map */
#include "inat-tables.c"
diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
index 0b5862ba6a75..79e048f1d902 100644
--- a/tools/arch/x86/lib/insn.c
+++ b/tools/arch/x86/lib/insn.c
@@ -10,8 +10,8 @@
#else
#include <string.h>
#endif
-#include <asm/inat.h>
-#include <asm/insn.h>
+#include "../include/asm/inat.h"
+#include "../include/asm/insn.h"

/* Verify next sizeof(t) bytes can be on the same instruction */
#define validate_next(t, insn, n) \
diff --git a/tools/perf/arch/x86/tests/insn-x86.c b/tools/perf/arch/x86/tests/insn-x86.c
index 824ae7164d01..745f29adb14b 100644
--- a/tools/perf/arch/x86/tests/insn-x86.c
+++ b/tools/perf/arch/x86/tests/insn-x86.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/types.h>
-#include <asm/insn.h>
+#include "../../../../arch/x86/include/asm/insn.h"
#include <string.h>

#include "debug.h"
diff --git a/tools/perf/arch/x86/util/archinsn.c b/tools/perf/arch/x86/util/archinsn.c
index 589213d38c87..9876c7a7ed7c 100644
--- a/tools/perf/arch/x86/util/archinsn.c
+++ b/tools/perf/arch/x86/util/archinsn.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0
-#include <asm/insn.h>
+#include "../../../../arch/x86/include/asm/insn.h"
#include "archinsn.h"
#include "machine.h"
#include "thread.h"
diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
index d2215a54d407..fb8a3558d3d5 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
@@ -9,7 +9,7 @@
#include <string.h>
#include <endian.h>
#include <byteswap.h>
-#include <asm/insn.h>
+#include "../../../arch/x86/include/asm/insn.h"

#include "../../../arch/x86/lib/inat.c"
#include "../../../arch/x86/lib/insn.c"

> - Arnaldo
>
> > Josh Poimboeuf (4):
> > objtool: Move x86 insn decoder to a common location
> > perf: Update .gitignore file
> > perf intel-pt: Remove inat.c from build dependency list
> > perf intel-pt: Use shared x86 insn decoder
> >
> > .../{objtool => }/arch/x86/include/asm/inat.h | 0
> > .../arch/x86/include/asm/inat_types.h | 0
> > .../{objtool => }/arch/x86/include/asm/insn.h | 0
> > .../arch/x86/include/asm/orc_types.h | 0
> > tools/{objtool => }/arch/x86/lib/inat.c | 0
> > tools/{objtool => }/arch/x86/lib/insn.c | 0
> > .../arch/x86/lib/x86-opcode-map.txt | 0
> > .../arch/x86/tools/gen-insn-attr-x86.awk | 0
> > tools/objtool/Makefile | 4 +-
> > tools/objtool/arch/x86/Build | 4 +-
> > tools/objtool/arch/x86/decode.c | 4 +-
> > tools/objtool/sync-check.sh | 12 +-
> > tools/perf/.gitignore | 3 +
> > tools/perf/arch/x86/tests/insn-x86.c | 2 +-
> > tools/perf/arch/x86/util/archinsn.c | 2 +-
> > tools/perf/check-headers.sh | 11 +-
> > tools/perf/util/intel-pt-decoder/Build | 22 +-
> > .../intel-pt-decoder/gen-insn-attr-x86.awk | 392 ------
> > tools/perf/util/intel-pt-decoder/inat.c | 82 --
> > tools/perf/util/intel-pt-decoder/inat.h | 230 ----
> > tools/perf/util/intel-pt-decoder/inat_types.h | 15 -
> > tools/perf/util/intel-pt-decoder/insn.c | 593 ---------
> > tools/perf/util/intel-pt-decoder/insn.h | 216 ----
> > .../intel-pt-decoder/intel-pt-insn-decoder.c | 10 +-
> > .../util/intel-pt-decoder/x86-opcode-map.txt | 1072 -----------------
> > 25 files changed, 34 insertions(+), 2640 deletions(-)
> > rename tools/{objtool => }/arch/x86/include/asm/inat.h (100%)
> > rename tools/{objtool => }/arch/x86/include/asm/inat_types.h (100%)
> > rename tools/{objtool => }/arch/x86/include/asm/insn.h (100%)
> > rename tools/{objtool => }/arch/x86/include/asm/orc_types.h (100%)
> > rename tools/{objtool => }/arch/x86/lib/inat.c (100%)
> > rename tools/{objtool => }/arch/x86/lib/insn.c (100%)
> > rename tools/{objtool => }/arch/x86/lib/x86-opcode-map.txt (100%)
> > rename tools/{objtool => }/arch/x86/tools/gen-insn-attr-x86.awk (100%)
> > delete mode 100644 tools/perf/util/intel-pt-decoder/gen-insn-attr-x86.awk
> > delete mode 100644 tools/perf/util/intel-pt-decoder/inat.c
> > delete mode 100644 tools/perf/util/intel-pt-decoder/inat.h
> > delete mode 100644 tools/perf/util/intel-pt-decoder/inat_types.h
> > delete mode 100644 tools/perf/util/intel-pt-decoder/insn.c
> > delete mode 100644 tools/perf/util/intel-pt-decoder/insn.h
> > delete mode 100644 tools/perf/util/intel-pt-decoder/x86-opcode-map.txt
> >
> > --
> > 2.20.1
>
> --
>
> - Arnaldo

--

- Arnaldo

Next message: David Dai: "[v2] net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate"
Previous message: Krishna Reddy: "RE: [PATCH 3/7] iommu/arm-smmu: Add tlb_sync implementation hook"
In reply to: Arnaldo Carvalho de Melo: "Re: [PATCH 0/4] objtool,perf: Use shared x86 insn decoder"
Next in thread: Josh Poimboeuf: "Re: [PATCH 0/4] objtool,perf: Use shared x86 insn decoder"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]