[PATCH -tip -v12 02/11] x86: x86 instruction decoder build-time selftest
Add a user-space selftest of x86 instruction decoder at kernel build time. When CONFIG_X86_DECODER_SELFTEST=y, Kbuild builds a test harness of x86 instruction decoder and performs it after building vmlinux. The test compares the results of objdump and x86 instruction decoder code and check there are no differences. Changes from v10: - Use unsigned int instead of unsigned. Signed-off-by: Masami Hiramatsu mhira...@redhat.com Signed-off-by: Jim Keniston jkeni...@us.ibm.com Cc: H. Peter Anvin h...@zytor.com Cc: Steven Rostedt rost...@goodmis.org Cc: Ananth N Mavinakayanahalli ana...@in.ibm.com Cc: Srikar Dronamraju sri...@linux.vnet.ibm.com Cc: Ingo Molnar mi...@elte.hu Cc: Frederic Weisbecker fweis...@gmail.com Cc: Andi Kleen a...@linux.intel.com Cc: Vegard Nossum vegard.nos...@gmail.com Cc: Avi Kivity a...@redhat.com Cc: Przemysław Pawełczyk przemys...@pawelczyk.it Cc: Sam Ravnborg s...@ravnborg.org --- arch/x86/Kconfig.debug |9 arch/x86/Makefile |3 + arch/x86/include/asm/inat.h |2 + arch/x86/include/asm/insn.h |2 + arch/x86/lib/inat.c |2 + arch/x86/lib/insn.c |2 + arch/x86/scripts/Makefile | 19 +++ arch/x86/scripts/distill.awk| 42 + arch/x86/scripts/test_get_len.c | 99 +++ arch/x86/scripts/user_include.h | 49 +++ 10 files changed, 229 insertions(+), 0 deletions(-) create mode 100644 arch/x86/scripts/Makefile create mode 100644 arch/x86/scripts/distill.awk create mode 100644 arch/x86/scripts/test_get_len.c create mode 100644 arch/x86/scripts/user_include.h diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug index d105f29..7d0b681 100644 --- a/arch/x86/Kconfig.debug +++ b/arch/x86/Kconfig.debug @@ -186,6 +186,15 @@ config X86_DS_SELFTEST config HAVE_MMIOTRACE_SUPPORT def_bool y +config X86_DECODER_SELFTEST + bool x86 instruction decoder selftest + depends on DEBUG_KERNEL + ---help--- +Perform x86 instruction decoder selftests at build time. +This option is useful for checking the sanity of x86 instruction +decoder code. +If unsure, say N. + # # IO delay types: # diff --git a/arch/x86/Makefile b/arch/x86/Makefile index 1b68659..7046556 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -154,6 +154,9 @@ all: bzImage KBUILD_IMAGE := $(boot)/bzImage bzImage: vmlinux +ifeq ($(CONFIG_X86_DECODER_SELFTEST),y) + $(Q)$(MAKE) $(build)=arch/x86/scripts posttest +endif $(Q)$(MAKE) $(build)=$(boot) $(KBUILD_IMAGE) $(Q)mkdir -p $(objtree)/arch/$(UTS_MACHINE)/boot $(Q)ln -fsn ../../x86/boot/bzImage $(objtree)/arch/$(UTS_MACHINE)/boot/$@ diff --git a/arch/x86/include/asm/inat.h b/arch/x86/include/asm/inat.h index 01e079a..9090665 100644 --- a/arch/x86/include/asm/inat.h +++ b/arch/x86/include/asm/inat.h @@ -20,7 +20,9 @@ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. * */ +#ifdef __KERNEL__ #include linux/types.h +#endif /* Instruction attributes */ typedef u32 insn_attr_t; diff --git a/arch/x86/include/asm/insn.h b/arch/x86/include/asm/insn.h index 5b50fa3..5736404 100644 --- a/arch/x86/include/asm/insn.h +++ b/arch/x86/include/asm/insn.h @@ -20,7 +20,9 @@ * Copyright (C) IBM Corporation, 2009 */ +#ifdef __KERNEL__ #include linux/types.h +#endif /* insn_attr_t is defined in inat.h */ #include asm/inat.h diff --git a/arch/x86/lib/inat.c b/arch/x86/lib/inat.c index d6a34be..564ecbd 100644 --- a/arch/x86/lib/inat.c +++ b/arch/x86/lib/inat.c @@ -18,7 +18,9 @@ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. * */ +#ifdef __KERNEL__ #include linux/module.h +#endif #include asm/insn.h /* Attribute tables are generated from opcode map */ diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c index 254c848..3b9451a 100644 --- a/arch/x86/lib/insn.c +++ b/arch/x86/lib/insn.c @@ -18,8 +18,10 @@ * Copyright (C) IBM Corporation, 2002, 2004, 2009 */ +#ifdef __KERNEL__ #include linux/string.h #include linux/module.h +#endif #include asm/inat.h #include asm/insn.h diff --git a/arch/x86/scripts/Makefile b/arch/x86/scripts/Makefile new file mode 100644 index 000..f08859e --- /dev/null +++ b/arch/x86/scripts/Makefile @@ -0,0 +1,19 @@ +PHONY += posttest +quiet_cmd_posttest = TEST$@ + cmd_posttest = objdump -d $(objtree)/vmlinux | awk -f $(srctree)/arch/x86/scripts/distill.awk | $(obj)/test_get_len + +posttest: $(obj)/test_get_len vmlinux + $(call cmd,posttest) + +test_get_len_SRC = $(srctree)/arch/x86/scripts/test_get_len.c $(srctree)/arch/x86/lib/insn.c $(srctree)/arch/x86/lib/inat.c +test_get_len_INC = $(srctree)/arch/x86/include/asm/inat.h $(srctree)/arch/x86/include/asm/insn.h $(objtree)/arch/x86/lib/inat-tables.c + +quiet_cmd_test_get_len = CC $@ + cmd_test_get_len = $(CC) -Wall $(test_get_len_SRC)
Re: [PATCH -tip -v12 02/11] x86: x86 instruction decoder build-time selftest
On Thu, Jul 16, 2009 at 11:57:06AM -0400, Masami Hiramatsu wrote:
Add a user-space selftest of x86 instruction decoder at kernel build time.
When CONFIG_X86_DECODER_SELFTEST=y, Kbuild builds a test harness of x86
instruction decoder and performs it after building vmlinux.
The test compares the results of objdump and x86 instruction decoder
code and check there are no differences.
Long overdue review from my side...
arch/x86/scripts/Makefile | 19 +++
arch/x86/scripts/distill.awk| 42 +
arch/x86/scripts/test_get_len.c | 99
+++
arch/x86/scripts/user_include.h | 49 +++
Hmmm, we have two architectures that uses scripts/ and three that
uses tools/.
I prefer the latter name as what we have ere is beyound what
I generally recognize as a script.
we have scripts/ in top-level and we do not rename this
as we have this hardcoded too many places - but no reason to
use the wrong name here.
diff --git a/arch/x86/include/asm/inat.h b/arch/x86/include/asm/inat.h
index 01e079a..9090665 100644
--- a/arch/x86/include/asm/inat.h
+++ b/arch/x86/include/asm/inat.h
@@ -20,7 +20,9 @@
* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
*
*/
+#ifdef __KERNEL__
#include linux/types.h
+#endif
/* Instruction attributes */
typedef u32 insn_attr_t;
Why this?
If you need this to use this file from userspace then could we do some
other trick to make this OK?
I see it repeated several times below.
[If this has already been discussed I have missed it - sorry].
diff --git a/arch/x86/scripts/Makefile b/arch/x86/scripts/Makefile
new file mode 100644
index 000..f08859e
--- /dev/null
+++ b/arch/x86/scripts/Makefile
@@ -0,0 +1,19 @@
+PHONY += posttest
+quiet_cmd_posttest = TEST$@
+ cmd_posttest = objdump -d $(objtree)/vmlinux | awk -f
$(srctree)/arch/x86/scripts/distill.awk | $(obj)/test_get_len
+
You are using the native objdump here.
But I assume this fails miserably when you build x86 on a powerpc host.
In other words - you broke an allyesconfig build for -next...
We have $(OBJDUMP) for this.
+posttest: $(obj)/test_get_len vmlinux
+ $(call cmd,posttest)
+
+test_get_len_SRC = $(srctree)/arch/x86/scripts/test_get_len.c
$(srctree)/arch/x86/lib/insn.c $(srctree)/arch/x86/lib/inat.c
+test_get_len_INC = $(srctree)/arch/x86/include/asm/inat.h
$(srctree)/arch/x86/include/asm/insn.h $(objtree)/arch/x86/lib/inat-tables.c
+
+quiet_cmd_test_get_len = CC $@
+ cmd_test_get_len = $(CC) -Wall $(test_get_len_SRC)
-I$(objtree)/arch/x86/lib/ -I$(srctree)/arch/x86/include -include
$(srctree)/arch/x86/scripts/user_include.h -o $@
Is there a specific reason why you cannot use the standard hostprogs-y for this?
It will take care of dependency tracking etc.
What you have above is a hopeless incomplete list of dependencies.
You need to use HOST_EXTRACFLAGS to set additional -I options and the -include.
+
+static void usage()
+{
+ fprintf(stderr, usage: %s distilled_disassembly\n, prog);
+ exit(1);
+}
It would be nice to tell the user what the program is supposed to do.
I know this is a bit unusual but no reason to copy bad practice.
index 000..3bdcc55
--- /dev/null
+++ b/arch/x86/scripts/user_include.h
@@ -0,0 +1,49 @@
+#ifndef __USER_TYPES_H
+#define __USER_TYPES_H
+
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright (C) IBM Corporation, 2009
+ */
+
+#include string.h
+
+#ifdef __x86_64__
+#define CONFIG_X86_64
+#else
+#define CONFIG_X86_32
+#endif
+typedef unsigned char u8;
+typedef unsigned short u16;
+typedef unsigned int u32;
+typedef unsigned long long u64;
+
+typedef signed char s8;
+typedef short s16;
+typedef int s32;
+typedef long long s64;
+
+typedef enum bool { false = 0, true } bool;
+
+/* any harmless file-scope decl */
+#define NOP_DECL struct __nop
+#define EXPORT_SYMBOL_GPL(symbol) NOP_DECL
+#define MODULE_LICENSE(gpl) NOP_DECL
+
+#define WARN_ON(cond) do { } while (0)
+#define unlikely(cond) (cond)
+
So this is a file that alows you to include the other files without dragging in
a massive amount a stuff.
Would be nice if you wrote so in the file so it is explicit.
I tried
Re: [PATCH -tip -v12 02/11] x86: x86 instruction decoder build-time selftest
Sam Ravnborg wrote:
On Thu, Jul 16, 2009 at 11:57:06AM -0400, Masami Hiramatsu wrote:
Add a user-space selftest of x86 instruction decoder at kernel build time.
When CONFIG_X86_DECODER_SELFTEST=y, Kbuild builds a test harness of x86
instruction decoder and performs it after building vmlinux.
The test compares the results of objdump and x86 instruction decoder
code and check there are no differences.
Long overdue review from my side...
arch/x86/scripts/Makefile | 19 +++
arch/x86/scripts/distill.awk| 42 +
arch/x86/scripts/test_get_len.c | 99
+++
arch/x86/scripts/user_include.h | 49 +++
Hmmm, we have two architectures that uses scripts/ and three that
uses tools/.
I prefer the latter name as what we have ere is beyound what
I generally recognize as a script.
we have scripts/ in top-level and we do not rename this
as we have this hardcoded too many places - but no reason to
use the wrong name here.
diff --git a/arch/x86/include/asm/inat.h b/arch/x86/include/asm/inat.h
index 01e079a..9090665 100644
--- a/arch/x86/include/asm/inat.h
+++ b/arch/x86/include/asm/inat.h
@@ -20,7 +20,9 @@
* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307,
USA.
*
*/
+#ifdef __KERNEL__
#include linux/types.h
+#endif
/* Instruction attributes */
typedef u32 insn_attr_t;
Why this?
If you need this to use this file from userspace then could we do some
other trick to make this OK?
I see it repeated several times below.
[If this has already been discussed I have missed it - sorry].
diff --git a/arch/x86/scripts/Makefile b/arch/x86/scripts/Makefile
new file mode 100644
index 000..f08859e
--- /dev/null
+++ b/arch/x86/scripts/Makefile
@@ -0,0 +1,19 @@
+PHONY += posttest
+quiet_cmd_posttest = TEST$@
+ cmd_posttest = objdump -d $(objtree)/vmlinux | awk -f
$(srctree)/arch/x86/scripts/distill.awk | $(obj)/test_get_len
+
You are using the native objdump here.
But I assume this fails miserably when you build x86 on a powerpc host.
In other words - you broke an allyesconfig build for -next...
We have $(OBJDUMP) for this.
Ah, I see... Would you know actual name of x86-objdump on the powerpc
(or any other crosscompiling host)? I just set OBJDUMP=objdump is OK?
I'm not so sure about cross-compiling kernel...
+posttest: $(obj)/test_get_len vmlinux
+$(call cmd,posttest)
+
+test_get_len_SRC = $(srctree)/arch/x86/scripts/test_get_len.c
$(srctree)/arch/x86/lib/insn.c $(srctree)/arch/x86/lib/inat.c
+test_get_len_INC = $(srctree)/arch/x86/include/asm/inat.h
$(srctree)/arch/x86/include/asm/insn.h $(objtree)/arch/x86/lib/inat-tables.c
+
+quiet_cmd_test_get_len = CC $@
+ cmd_test_get_len = $(CC) -Wall $(test_get_len_SRC)
-I$(objtree)/arch/x86/lib/ -I$(srctree)/arch/x86/include -include
$(srctree)/arch/x86/scripts/user_include.h -o $@
Is there a specific reason why you cannot use the standard hostprogs-y for
this?
It will take care of dependency tracking etc.
What you have above is a hopeless incomplete list of dependencies.
You need to use HOST_EXTRACFLAGS to set additional -I options and the
-include.
Thank you, I'll try to use hostprogs-y.
+
+static void usage()
+{
+fprintf(stderr, usage: %s distilled_disassembly\n, prog);
+exit(1);
+}
It would be nice to tell the user what the program is supposed to do.
I know this is a bit unusual but no reason to copy bad practice.
Sure, maybe copying usage line in distill.awk is more helpful for user...
Thank you,
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division
e-mail: mhira...@redhat.com
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -tip -v12 02/11] x86: x86 instruction decoder build-time selftest
Masami Hiramatsu wrote: You are using the native objdump here. But I assume this fails miserably when you build x86 on a powerpc host. In other words - you broke an allyesconfig build for -next... We have $(OBJDUMP) for this. Ah, I see... Would you know actual name of x86-objdump on the powerpc (or any other crosscompiling host)? I just set OBJDUMP=objdump is OK? I'm not so sure about cross-compiling kernel... Oops, we already have it. Yes, I'll use $(OBJDUMP). -- Masami Hiramatsu Software Engineer Hitachi Computer Products (America), Inc. Software Solutions Division e-mail: mhira...@redhat.com -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -tip -v12 02/11] x86: x86 instruction decoder build-time selftest
+ cmd_posttest = objdump -d $(objtree)/vmlinux | awk -f $(srctree)/arch/x86/scripts/distill.awk | $(obj)/test_get_len + You are using the native objdump here. But I assume this fails miserably when you build x86 on a powerpc host. In other words - you broke an allyesconfig build for -next... We have $(OBJDUMP) for this. Ah, I see... Would you know actual name of x86-objdump on the powerpc (or any other crosscompiling host)? I just set OBJDUMP=objdump is OK? I'm not so sure about cross-compiling kernel... Replacing objdump with $(OBJDUMP) will do the trick. We set OBJDUMP to the correct value in the top-level makefile. Are there any parts of your user-space program that rely on the host is little-endian? If it does then it would fail on a power-pc target despite using the correct objdump. Sam -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -tip -v12 02/11] x86: x86 instruction decoder build-time selftest
Sam Ravnborg wrote: + cmd_posttest = objdump -d $(objtree)/vmlinux | awk -f $(srctree)/arch/x86/scripts/distill.awk | $(obj)/test_get_len + You are using the native objdump here. But I assume this fails miserably when you build x86 on a powerpc host. In other words - you broke an allyesconfig build for -next... We have $(OBJDUMP) for this. Ah, I see... Would you know actual name of x86-objdump on the powerpc (or any other crosscompiling host)? I just set OBJDUMP=objdump is OK? I'm not so sure about cross-compiling kernel... Replacing objdump with $(OBJDUMP) will do the trick. We set OBJDUMP to the correct value in the top-level makefile. Are there any parts of your user-space program that rely on the host is little-endian? If it does then it would fail on a power-pc target despite using the correct objdump. Hmm, as far as I can see, the result of get_next() macro with the types more than two bytes(s16, s32...) might be effected. But it doesn't effect get_insn_len test because those values are ignored. Thank you, -- Masami Hiramatsu Software Engineer Hitachi Computer Products (America), Inc. Software Solutions Division e-mail: mhira...@redhat.com -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
