All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/11] fuzz: update x86emul fuzzer
@ 2017-02-01 12:02 Wei Liu
  2017-02-01 12:02 ` [PATCH v3 01/11] x86emul/test: add missing dependency for x86_emulate.o Wei Liu
                   ` (10 more replies)
  0 siblings, 11 replies; 28+ messages in thread
From: Wei Liu @ 2017-02-01 12:02 UTC (permalink / raw)
  To: Xen-devel; +Cc: George Dunlap, Ian Jackson, Wei Liu, Jan Beulich, Andrew Cooper

The first few patches refactor x86emul code so that more code can be shared
between xen and userspace tools.

I have run XTF suite (tests subject to availability on the testbox I use, and
xsa-195 was skipped because qemu segfault -- a known issue) against this
series, no issue is found.

Please see individual patch for changelog.

Wei.
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>

Wei Liu (11):
  x86emul/test: add missing dependency for x86_emulate.o
  x86: extract macros to x86-defns.h
  x86: extract vendor numeric id to x86-vendors.h
  x86emul/test: use x86-vendors.h
  x86emul: use eflags definitions in x86-defns.h
  x86emul: use msr definitions in msr-index.h
  x86: add UMIP CR4 bit
  x86emul: use CR definitions in x86-defns.h
  fuzz/x86emul: update fuzzer
  fuzz/x86emul: print out minimal input size
  fuzz: update README.afl example

 tools/fuzz/README.afl                              |   5 +-
 tools/fuzz/x86_instruction_emulator/Makefile       |   9 +-
 .../afl-x86-insn-emulator-fuzzer.c                 |   8 +
 .../x86-insn-emulator-fuzzer.c                     | 666 +++++++++++++++++++--
 tools/tests/x86_emulator/Makefile                  |  11 +-
 tools/tests/x86_emulator/test_x86_emulator.c       |   9 -
 tools/tests/x86_emulator/x86_emulate.c             |   3 -
 tools/tests/x86_emulator/x86_emulate.h             |   9 +-
 xen/arch/x86/x86_emulate/x86_emulate.c             | 403 ++++++-------
 xen/include/asm-x86/processor.h                    |  73 +--
 xen/include/asm-x86/x86-defns.h                    |  69 +++
 xen/include/asm-x86/x86-vendors.h                  |  13 +
 12 files changed, 900 insertions(+), 378 deletions(-)
 create mode 100644 xen/include/asm-x86/x86-defns.h
 create mode 100644 xen/include/asm-x86/x86-vendors.h

-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3 01/11] x86emul/test: add missing dependency for x86_emulate.o
  2017-02-01 12:02 [PATCH v3 00/11] fuzz: update x86emul fuzzer Wei Liu
@ 2017-02-01 12:02 ` Wei Liu
  2017-02-01 13:03   ` Jan Beulich
  2017-02-01 12:02 ` [PATCH v3 02/11] x86: extract macros to x86-defns.h Wei Liu
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 28+ messages in thread
From: Wei Liu @ 2017-02-01 12:02 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

f4497d6b74 added x86_emulate.h private header but didn't add dependency
for it.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tools/tests/x86_emulator/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/tests/x86_emulator/Makefile b/tools/tests/x86_emulator/Makefile
index 0b5baff67c..883daf30a3 100644
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -45,7 +45,7 @@ x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
 
 HOSTCFLAGS += $(CFLAGS_xeninclude)
 
-x86_emulate.o: x86_emulate.c x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
+x86_emulate.o: x86_emulate.c x86_emulate.h x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
 	$(HOSTCC) $(HOSTCFLAGS) -D__XEN_TOOLS__ -c -g -o $@ $<
 
 test_x86_emulator.o: test_x86_emulator.c blowfish.h x86_emulate/x86_emulate.h
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 02/11] x86: extract macros to x86-defns.h
  2017-02-01 12:02 [PATCH v3 00/11] fuzz: update x86emul fuzzer Wei Liu
  2017-02-01 12:02 ` [PATCH v3 01/11] x86emul/test: add missing dependency for x86_emulate.o Wei Liu
@ 2017-02-01 12:02 ` Wei Liu
  2017-02-01 13:05   ` Jan Beulich
  2017-02-01 12:02 ` [PATCH v3 03/11] x86: extract vendor numeric id to x86-vendors.h Wei Liu
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 28+ messages in thread
From: Wei Liu @ 2017-02-01 12:02 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

... so that they can be used by userspace x86 instruction emulator test
program and fuzzer as well.

No functional change.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

v3: Don't move TRAP_*
---
 xen/include/asm-x86/processor.h | 65 ++-------------------------------------
 xen/include/asm-x86/x86-defns.h | 68 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 63 deletions(-)
 create mode 100644 xen/include/asm-x86/x86-defns.h

diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 4da9c193e0..0b9191befa 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -16,6 +16,8 @@
 #include <asm/desc.h>
 #endif
 
+#include "x86-defns.h"
+
 /*
  * CPU vendor IDs
  */
@@ -25,69 +27,6 @@
 #define X86_VENDOR_NUM 3
 #define X86_VENDOR_UNKNOWN 0xff
 
-/*
- * EFLAGS bits
- */
-#define X86_EFLAGS_CF	0x00000001 /* Carry Flag */
-#define X86_EFLAGS_MBS	0x00000002 /* Resvd bit */
-#define X86_EFLAGS_PF	0x00000004 /* Parity Flag */
-#define X86_EFLAGS_AF	0x00000010 /* Auxillary carry Flag */
-#define X86_EFLAGS_ZF	0x00000040 /* Zero Flag */
-#define X86_EFLAGS_SF	0x00000080 /* Sign Flag */
-#define X86_EFLAGS_TF	0x00000100 /* Trap Flag */
-#define X86_EFLAGS_IF	0x00000200 /* Interrupt Flag */
-#define X86_EFLAGS_DF	0x00000400 /* Direction Flag */
-#define X86_EFLAGS_OF	0x00000800 /* Overflow Flag */
-#define X86_EFLAGS_IOPL	0x00003000 /* IOPL mask */
-#define X86_EFLAGS_NT	0x00004000 /* Nested Task */
-#define X86_EFLAGS_RF	0x00010000 /* Resume Flag */
-#define X86_EFLAGS_VM	0x00020000 /* Virtual Mode */
-#define X86_EFLAGS_AC	0x00040000 /* Alignment Check */
-#define X86_EFLAGS_VIF	0x00080000 /* Virtual Interrupt Flag */
-#define X86_EFLAGS_VIP	0x00100000 /* Virtual Interrupt Pending */
-#define X86_EFLAGS_ID	0x00200000 /* CPUID detection flag */
-
-#define X86_EFLAGS_ARITH_MASK                          \
-    (X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF |   \
-     X86_EFLAGS_ZF | X86_EFLAGS_SF | X86_EFLAGS_OF)
-
-/*
- * Intel CPU flags in CR0
- */
-#define X86_CR0_PE              0x00000001 /* Enable Protected Mode    (RW) */
-#define X86_CR0_MP              0x00000002 /* Monitor Coprocessor      (RW) */
-#define X86_CR0_EM              0x00000004 /* Require FPU Emulation    (RO) */
-#define X86_CR0_TS              0x00000008 /* Task Switched            (RW) */
-#define X86_CR0_ET              0x00000010 /* Extension type           (RO) */
-#define X86_CR0_NE              0x00000020 /* Numeric Error Reporting  (RW) */
-#define X86_CR0_WP              0x00010000 /* Supervisor Write Protect (RW) */
-#define X86_CR0_AM              0x00040000 /* Alignment Checking       (RW) */
-#define X86_CR0_NW              0x20000000 /* Not Write-Through        (RW) */
-#define X86_CR0_CD              0x40000000 /* Cache Disable            (RW) */
-#define X86_CR0_PG              0x80000000 /* Paging                   (RW) */
-
-/*
- * Intel CPU features in CR4
- */
-#define X86_CR4_VME        0x00000001 /* enable vm86 extensions */
-#define X86_CR4_PVI        0x00000002 /* virtual interrupts flag enable */
-#define X86_CR4_TSD        0x00000004 /* disable time stamp at ipl 3 */
-#define X86_CR4_DE         0x00000008 /* enable debugging extensions */
-#define X86_CR4_PSE        0x00000010 /* enable page size extensions */
-#define X86_CR4_PAE        0x00000020 /* enable physical address extensions */
-#define X86_CR4_MCE        0x00000040 /* Machine check enable */
-#define X86_CR4_PGE        0x00000080 /* enable global pages */
-#define X86_CR4_PCE        0x00000100 /* enable performance counters at ipl 3 */
-#define X86_CR4_OSFXSR     0x00000200 /* enable fast FPU save and restore */
-#define X86_CR4_OSXMMEXCPT 0x00000400 /* enable unmasked SSE exceptions */
-#define X86_CR4_VMXE       0x00002000 /* enable VMX */
-#define X86_CR4_SMXE       0x00004000 /* enable SMX */
-#define X86_CR4_FSGSBASE   0x00010000 /* enable {rd,wr}{fs,gs}base */
-#define X86_CR4_PCIDE      0x00020000 /* enable PCID */
-#define X86_CR4_OSXSAVE    0x00040000 /* enable XSAVE/XRSTOR */
-#define X86_CR4_SMEP       0x00100000 /* enable SMEP */
-#define X86_CR4_SMAP       0x00200000 /* enable SMAP */
-#define X86_CR4_PKE        0x00400000 /* enable PKE */
 
 /*
  * Trap/fault mnemonics.
diff --git a/xen/include/asm-x86/x86-defns.h b/xen/include/asm-x86/x86-defns.h
new file mode 100644
index 0000000000..48ea5ea03b
--- /dev/null
+++ b/xen/include/asm-x86/x86-defns.h
@@ -0,0 +1,68 @@
+#ifndef __XEN_X86_DEFNS_H__
+#define __XEN_X86_DEFNS_H__
+
+/*
+ * EFLAGS bits
+ */
+#define X86_EFLAGS_CF	0x00000001 /* Carry Flag */
+#define X86_EFLAGS_MBS	0x00000002 /* Resvd bit */
+#define X86_EFLAGS_PF	0x00000004 /* Parity Flag */
+#define X86_EFLAGS_AF	0x00000010 /* Auxillary carry Flag */
+#define X86_EFLAGS_ZF	0x00000040 /* Zero Flag */
+#define X86_EFLAGS_SF	0x00000080 /* Sign Flag */
+#define X86_EFLAGS_TF	0x00000100 /* Trap Flag */
+#define X86_EFLAGS_IF	0x00000200 /* Interrupt Flag */
+#define X86_EFLAGS_DF	0x00000400 /* Direction Flag */
+#define X86_EFLAGS_OF	0x00000800 /* Overflow Flag */
+#define X86_EFLAGS_IOPL	0x00003000 /* IOPL mask */
+#define X86_EFLAGS_NT	0x00004000 /* Nested Task */
+#define X86_EFLAGS_RF	0x00010000 /* Resume Flag */
+#define X86_EFLAGS_VM	0x00020000 /* Virtual Mode */
+#define X86_EFLAGS_AC	0x00040000 /* Alignment Check */
+#define X86_EFLAGS_VIF	0x00080000 /* Virtual Interrupt Flag */
+#define X86_EFLAGS_VIP	0x00100000 /* Virtual Interrupt Pending */
+#define X86_EFLAGS_ID	0x00200000 /* CPUID detection flag */
+
+#define X86_EFLAGS_ARITH_MASK                          \
+    (X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF |   \
+     X86_EFLAGS_ZF | X86_EFLAGS_SF | X86_EFLAGS_OF)
+
+/*
+ * Intel CPU flags in CR0
+ */
+#define X86_CR0_PE              0x00000001 /* Enable Protected Mode    (RW) */
+#define X86_CR0_MP              0x00000002 /* Monitor Coprocessor      (RW) */
+#define X86_CR0_EM              0x00000004 /* Require FPU Emulation    (RO) */
+#define X86_CR0_TS              0x00000008 /* Task Switched            (RW) */
+#define X86_CR0_ET              0x00000010 /* Extension type           (RO) */
+#define X86_CR0_NE              0x00000020 /* Numeric Error Reporting  (RW) */
+#define X86_CR0_WP              0x00010000 /* Supervisor Write Protect (RW) */
+#define X86_CR0_AM              0x00040000 /* Alignment Checking       (RW) */
+#define X86_CR0_NW              0x20000000 /* Not Write-Through        (RW) */
+#define X86_CR0_CD              0x40000000 /* Cache Disable            (RW) */
+#define X86_CR0_PG              0x80000000 /* Paging                   (RW) */
+
+/*
+ * Intel CPU features in CR4
+ */
+#define X86_CR4_VME        0x00000001 /* enable vm86 extensions */
+#define X86_CR4_PVI        0x00000002 /* virtual interrupts flag enable */
+#define X86_CR4_TSD        0x00000004 /* disable time stamp at ipl 3 */
+#define X86_CR4_DE         0x00000008 /* enable debugging extensions */
+#define X86_CR4_PSE        0x00000010 /* enable page size extensions */
+#define X86_CR4_PAE        0x00000020 /* enable physical address extensions */
+#define X86_CR4_MCE        0x00000040 /* Machine check enable */
+#define X86_CR4_PGE        0x00000080 /* enable global pages */
+#define X86_CR4_PCE        0x00000100 /* enable performance counters at ipl 3 */
+#define X86_CR4_OSFXSR     0x00000200 /* enable fast FPU save and restore */
+#define X86_CR4_OSXMMEXCPT 0x00000400 /* enable unmasked SSE exceptions */
+#define X86_CR4_VMXE       0x00002000 /* enable VMX */
+#define X86_CR4_SMXE       0x00004000 /* enable SMX */
+#define X86_CR4_FSGSBASE   0x00010000 /* enable {rd,wr}{fs,gs}base */
+#define X86_CR4_PCIDE      0x00020000 /* enable PCID */
+#define X86_CR4_OSXSAVE    0x00040000 /* enable XSAVE/XRSTOR */
+#define X86_CR4_SMEP       0x00100000 /* enable SMEP */
+#define X86_CR4_SMAP       0x00200000 /* enable SMAP */
+#define X86_CR4_PKE        0x00400000 /* enable PKE */
+
+#endif	/* __XEN_X86_DEFNS_H__ */
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 03/11] x86: extract vendor numeric id to x86-vendors.h
  2017-02-01 12:02 [PATCH v3 00/11] fuzz: update x86emul fuzzer Wei Liu
  2017-02-01 12:02 ` [PATCH v3 01/11] x86emul/test: add missing dependency for x86_emulate.o Wei Liu
  2017-02-01 12:02 ` [PATCH v3 02/11] x86: extract macros to x86-defns.h Wei Liu
@ 2017-02-01 12:02 ` Wei Liu
  2017-02-01 12:02 ` [PATCH v3 04/11] x86emul/test: use x86-vendors.h Wei Liu
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 28+ messages in thread
From: Wei Liu @ 2017-02-01 12:02 UTC (permalink / raw)
  To: Xen-devel; +Cc: Wei Liu

They will be shared between xen and userspace programs.

This is not strictly necessary, but it helps reduce overall code size.

No functional change.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 xen/include/asm-x86/processor.h   | 10 +---------
 xen/include/asm-x86/x86-vendors.h | 13 +++++++++++++
 2 files changed, 14 insertions(+), 9 deletions(-)
 create mode 100644 xen/include/asm-x86/x86-vendors.h

diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 0b9191befa..ebe4e37aad 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -17,15 +17,7 @@
 #endif
 
 #include "x86-defns.h"
-
-/*
- * CPU vendor IDs
- */
-#define X86_VENDOR_INTEL 0
-#define X86_VENDOR_AMD 1
-#define X86_VENDOR_CENTAUR 2
-#define X86_VENDOR_NUM 3
-#define X86_VENDOR_UNKNOWN 0xff
+#include "x86-vendors.h"
 
 
 /*
diff --git a/xen/include/asm-x86/x86-vendors.h b/xen/include/asm-x86/x86-vendors.h
new file mode 100644
index 0000000000..cae5507bd0
--- /dev/null
+++ b/xen/include/asm-x86/x86-vendors.h
@@ -0,0 +1,13 @@
+#ifndef __XEN_X86_VENDORS_H__
+#define __XEN_X86_VENDORS_H__
+
+/*
+ * CPU vendor IDs
+ */
+#define X86_VENDOR_INTEL 0
+#define X86_VENDOR_AMD 1
+#define X86_VENDOR_CENTAUR 2
+#define X86_VENDOR_NUM 3
+#define X86_VENDOR_UNKNOWN 0xff
+
+#endif	/* __XEN_X86_VENDORS_H__ */
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 04/11] x86emul/test: use x86-vendors.h
  2017-02-01 12:02 [PATCH v3 00/11] fuzz: update x86emul fuzzer Wei Liu
                   ` (2 preceding siblings ...)
  2017-02-01 12:02 ` [PATCH v3 03/11] x86: extract vendor numeric id to x86-vendors.h Wei Liu
@ 2017-02-01 12:02 ` Wei Liu
  2017-02-01 13:09   ` Jan Beulich
  2017-02-01 12:02 ` [PATCH v3 05/11] x86emul: use eflags definitions in x86-defns.h Wei Liu
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 28+ messages in thread
From: Wei Liu @ 2017-02-01 12:02 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

No functional change.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

V3: link asm-x86 to working directory
---
 tools/fuzz/x86_instruction_emulator/Makefile | 9 +++++++--
 tools/tests/x86_emulator/Makefile            | 9 +++++++--
 tools/tests/x86_emulator/x86_emulate.h       | 7 ++-----
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile
index f2bb12e871..d5c5bba0b0 100644
--- a/tools/fuzz/x86_instruction_emulator/Makefile
+++ b/tools/fuzz/x86_instruction_emulator/Makefile
@@ -11,10 +11,15 @@ endif
 x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
 	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
 
+asm/x86-vendors.h:
+	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm
+
 x86_emulate.c x86_emulate.h: %:
 	[ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$*
 
-CFLAGS += $(CFLAGS_xeninclude) -D__XEN_TOOLS__
+CFLAGS += $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -I.
+
+x86_emulate.h: asm/x86-vendors.h
 
 x86_emulate.o: x86_emulate.c x86_emulate.h x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
 
@@ -31,7 +36,7 @@ all: x86-instruction-emulator-fuzzer-all
 
 .PHONY: distclean
 distclean: clean
-	rm -f x86_emulate x86_emulate.c x86_emulate.h
+	rm -f x86_emulate x86_emulate.c x86_emulate.h asm
 
 .PHONY: clean
 clean:
diff --git a/tools/tests/x86_emulator/Makefile b/tools/tests/x86_emulator/Makefile
index 883daf30a3..bd1c4d664e 100644
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -32,7 +32,7 @@ $(TARGET): x86_emulate.o test_x86_emulator.o
 
 .PHONY: clean
 clean:
-	rm -rf $(TARGET) *.o *~ core blowfish.h blowfish.bin x86_emulate
+	rm -rf $(TARGET) *.o *~ core blowfish.h blowfish.bin x86_emulate asm
 
 .PHONY: distclean
 distclean: clean
@@ -43,7 +43,12 @@ install:
 x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
 	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
 
-HOSTCFLAGS += $(CFLAGS_xeninclude)
+asm/x86-vendors.h:
+	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm
+
+x86_emulate.h: asm/x86-vendors.h
+
+HOSTCFLAGS += $(CFLAGS_xeninclude) -I.
 
 x86_emulate.o: x86_emulate.c x86_emulate.h x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
 	$(HOSTCC) $(HOSTCFLAGS) -D__XEN_TOOLS__ -c -g -o $@ $<
diff --git a/tools/tests/x86_emulator/x86_emulate.h b/tools/tests/x86_emulator/x86_emulate.h
index 3a6badee46..db287004e6 100644
--- a/tools/tests/x86_emulator/x86_emulate.h
+++ b/tools/tests/x86_emulator/x86_emulate.h
@@ -6,6 +6,8 @@
 #include <string.h>
 #include <xen/xen.h>
 
+#include <asm/x86-vendors.h>
+
 #define BUG() abort()
 #define ASSERT assert
 #define ASSERT_UNREACHABLE() assert(!__LINE__)
@@ -38,11 +40,6 @@
 
 #define is_canonical_address(x) (((int64_t)(x) >> 47) == ((int64_t)(x) >> 63))
 
-/* There's no strict need for these to be in sync with processor.h. */
-#define X86_VENDOR_INTEL   0
-#define X86_VENDOR_AMD     2
-#define X86_VENDOR_UNKNOWN 0xff
-
 #define MMAP_SZ 16384
 bool emul_test_make_stack_executable(void);
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 05/11] x86emul: use eflags definitions in x86-defns.h
  2017-02-01 12:02 [PATCH v3 00/11] fuzz: update x86emul fuzzer Wei Liu
                   ` (3 preceding siblings ...)
  2017-02-01 12:02 ` [PATCH v3 04/11] x86emul/test: use x86-vendors.h Wei Liu
@ 2017-02-01 12:02 ` Wei Liu
  2017-02-01 12:02 ` [PATCH v3 06/11] x86emul: use msr definitions in msr-index.h Wei Liu
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 28+ messages in thread
From: Wei Liu @ 2017-02-01 12:02 UTC (permalink / raw)
  To: Xen-devel; +Cc: Wei Liu

Basically this patch does 's/EFLG_/X86_EFLAGS_/g' and with indentation
fixed up. And remove the duplicates in x86_emualte.c.  This in turn
requires userspace test harness to include x86-defns.h. Also remove a
few duplicates in userspace harness program.

No functional change.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
v3:
1. Adapt build system to previous patch
2. Add spaces around "|"
---
 tools/fuzz/x86_instruction_emulator/Makefile |   4 +-
 tools/tests/x86_emulator/Makefile            |   4 +-
 tools/tests/x86_emulator/test_x86_emulator.c |   9 -
 tools/tests/x86_emulator/x86_emulate.h       |   1 +
 xen/arch/x86/x86_emulate/x86_emulate.c       | 321 +++++++++++++--------------
 5 files changed, 160 insertions(+), 179 deletions(-)

diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile
index d5c5bba0b0..240bc58b06 100644
--- a/tools/fuzz/x86_instruction_emulator/Makefile
+++ b/tools/fuzz/x86_instruction_emulator/Makefile
@@ -11,7 +11,7 @@ endif
 x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
 	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
 
-asm/x86-vendors.h:
+asm/x86-vendors.h asm/x86-defns.h:
 	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm
 
 x86_emulate.c x86_emulate.h: %:
@@ -19,7 +19,7 @@ x86_emulate.c x86_emulate.h: %:
 
 CFLAGS += $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -I.
 
-x86_emulate.h: asm/x86-vendors.h
+x86_emulate.h: asm/x86-vendors.h asm/x86-defns.h
 
 x86_emulate.o: x86_emulate.c x86_emulate.h x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
 
diff --git a/tools/tests/x86_emulator/Makefile b/tools/tests/x86_emulator/Makefile
index bd1c4d664e..6978288294 100644
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -43,10 +43,10 @@ install:
 x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
 	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
 
-asm/x86-vendors.h:
+asm/x86-vendors.h asm/x86-defns.h:
 	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm
 
-x86_emulate.h: asm/x86-vendors.h
+x86_emulate.h: asm/x86-vendors.h asm/x86-defns.h
 
 HOSTCFLAGS += $(CFLAGS_xeninclude) -I.
 
diff --git a/tools/tests/x86_emulator/test_x86_emulator.c b/tools/tests/x86_emulator/test_x86_emulator.c
index 924fd36f18..27200e0283 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -23,15 +23,6 @@ static const struct {
 #endif
 };
 
-/* EFLAGS bit definitions. */
-#define EFLG_OF (1<<11)
-#define EFLG_DF (1<<10)
-#define EFLG_SF (1<<7)
-#define EFLG_ZF (1<<6)
-#define EFLG_AF (1<<4)
-#define EFLG_PF (1<<2)
-#define EFLG_CF (1<<0)
-
 static unsigned int bytes_read;
 
 static int read(
diff --git a/tools/tests/x86_emulator/x86_emulate.h b/tools/tests/x86_emulator/x86_emulate.h
index db287004e6..e064deaeea 100644
--- a/tools/tests/x86_emulator/x86_emulate.h
+++ b/tools/tests/x86_emulator/x86_emulate.h
@@ -6,6 +6,7 @@
 #include <string.h>
 #include <xen/xen.h>
 
+#include <asm/x86-defns.h>
 #include <asm/x86-vendors.h>
 
 #define BUG() abort()
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index 21dd98cebc..6e8221d9d8 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -447,26 +447,6 @@ typedef union {
 #define CR4_FSGSBASE   (1<<16)
 #define CR4_OSXSAVE    (1<<18)
 
-/* EFLAGS bit definitions. */
-#define EFLG_ID   (1<<21)
-#define EFLG_VIP  (1<<20)
-#define EFLG_VIF  (1<<19)
-#define EFLG_AC   (1<<18)
-#define EFLG_VM   (1<<17)
-#define EFLG_RF   (1<<16)
-#define EFLG_NT   (1<<14)
-#define EFLG_IOPL (3<<12)
-#define EFLG_OF   (1<<11)
-#define EFLG_DF   (1<<10)
-#define EFLG_IF   (1<<9)
-#define EFLG_TF   (1<<8)
-#define EFLG_SF   (1<<7)
-#define EFLG_ZF   (1<<6)
-#define EFLG_AF   (1<<4)
-#define EFLG_PF   (1<<2)
-#define EFLG_MBS  (1<<1)
-#define EFLG_CF   (1<<0)
-
 /* Floating point status word definitions. */
 #define FSW_ES    (1U << 7)
 
@@ -521,14 +501,16 @@ typedef union {
  * These EFLAGS bits are restored from saved value during emulation, and
  * any changes are written back to the saved value after emulation.
  */
-#define EFLAGS_MASK (EFLG_OF|EFLG_SF|EFLG_ZF|EFLG_AF|EFLG_PF|EFLG_CF)
+#define EFLAGS_MASK (X86_EFLAGS_OF | X86_EFLAGS_SF | X86_EFLAGS_ZF | \
+                     X86_EFLAGS_AF | X86_EFLAGS_PF | X86_EFLAGS_CF)
 
 /*
  * These EFLAGS bits are modifiable (by POPF and IRET), possibly subject
  * to further CPL and IOPL constraints.
  */
-#define EFLAGS_MODIFIABLE (EFLG_ID|EFLG_AC|EFLG_RF|EFLG_NT|EFLG_IOPL| \
-                           EFLG_DF|EFLG_IF|EFLG_TF|EFLAGS_MASK)
+#define EFLAGS_MODIFIABLE (X86_EFLAGS_ID | X86_EFLAGS_AC | X86_EFLAGS_RF | \
+                           X86_EFLAGS_NT | X86_EFLAGS_IOPL | X86_EFLAGS_DF | \
+                           X86_EFLAGS_IF | X86_EFLAGS_TF | EFLAGS_MASK)
 
 /* Before executing instruction: restore necessary bits in EFLAGS. */
 #define _PRE_EFLAGS(_sav, _msk, _tmp)                           \
@@ -761,7 +743,8 @@ do {                                                                    \
 } while (0)
 #define register_address_adjust(reg, adj)                               \
     _register_address_increment(reg,                                    \
-                                _regs._eflags & EFLG_DF ? -(adj) : (adj), \
+                                _regs._eflags & X86_EFLAGS_DF ?         \
+                                -(adj) : (adj),                         \
                                 ad_bytes)
 
 #define sp_pre_dec(dec) ({                                              \
@@ -784,7 +767,7 @@ do {                                                                    \
     rc = ops->insn_fetch(x86_seg_cs, ip, NULL, 0, ctxt);                \
     if ( rc ) goto done;                                                \
     _regs.r(ip) = ip;                                                   \
-    singlestep = _regs._eflags & EFLG_TF;                               \
+    singlestep = _regs._eflags & X86_EFLAGS_TF;                         \
 } while (0)
 
 #define validate_far_branch(cs, ip) ({                                  \
@@ -801,7 +784,7 @@ do {                                                                    \
 #define commit_far_branch(cs, newip) ({                                 \
     validate_far_branch(cs, newip);                                     \
     _regs.r(ip) = (newip);                                              \
-    singlestep = _regs._eflags & EFLG_TF;                               \
+    singlestep = _regs._eflags & X86_EFLAGS_TF;                         \
     ops->write_segment(x86_seg_cs, cs, ctxt);                           \
 })
 
@@ -854,7 +837,7 @@ static int _get_fpu(
         if ( type >= X86EMUL_FPU_ymm )
         {
             /* Should be unreachable if VEX decoding is working correctly. */
-            ASSERT((cr0 & CR0_PE) && !(ctxt->regs->_eflags & EFLG_VM));
+            ASSERT((cr0 & CR0_PE) && !(ctxt->regs->_eflags & X86_EFLAGS_VM));
         }
         if ( cr0 & CR0_EM )
         {
@@ -946,7 +929,8 @@ do {                                                                    \
                    : [eflags] "+g" (_regs._eflags),                     \
                      [tmp] "=&r" (tmp_), "+m" (fic)                     \
                    : [func] "rm" (stub.func),                           \
-                     [mask] "i" (EFLG_ZF|EFLG_PF|EFLG_CF) );            \
+                     [mask] "i" (X86_EFLAGS_ZF|X86_EFLAGS_PF|           \
+                                 X86_EFLAGS_CF) );                      \
     put_stub(stub);                                                     \
 } while (0)
 
@@ -988,7 +972,7 @@ static inline void put_loop_count(
         }                                                               \
         goto complete_insn;                                             \
     }                                                                   \
-    if ( max_reps > 1 && (_regs._eflags & EFLG_TF) &&                   \
+    if ( max_reps > 1 && (_regs._eflags & X86_EFLAGS_TF) &&             \
          !is_branch_step(ctxt, ops) )                                   \
         max_reps = 1;                                                   \
     max_reps;                                                           \
@@ -1022,7 +1006,7 @@ static void __put_rep_prefix(
 /* Clip maximum repetitions so that the index register at most just wraps. */
 #define truncate_ea_and_reps(ea, reps, bytes_per_rep) ({                  \
     unsigned long todo__, ea__ = truncate_word(ea, ad_bytes);             \
-    if ( !(_regs._eflags & EFLG_DF) )                                     \
+    if ( !(_regs._eflags & X86_EFLAGS_DF) )                               \
         todo__ = truncate_word(-(ea), ad_bytes) / (bytes_per_rep);        \
     else if ( truncate_word((ea) + (bytes_per_rep) - 1, ad_bytes) < ea__ )\
         todo__ = 1;                                                       \
@@ -1143,28 +1127,28 @@ test_cc(
     switch ( (condition & 15) >> 1 )
     {
     case 0: /* o */
-        rc |= (flags & EFLG_OF);
+        rc |= (flags & X86_EFLAGS_OF);
         break;
     case 1: /* b/c/nae */
-        rc |= (flags & EFLG_CF);
+        rc |= (flags & X86_EFLAGS_CF);
         break;
     case 2: /* z/e */
-        rc |= (flags & EFLG_ZF);
+        rc |= (flags & X86_EFLAGS_ZF);
         break;
     case 3: /* be/na */
-        rc |= (flags & (EFLG_CF|EFLG_ZF));
+        rc |= (flags & (X86_EFLAGS_CF | X86_EFLAGS_ZF));
         break;
     case 4: /* s */
-        rc |= (flags & EFLG_SF);
+        rc |= (flags & X86_EFLAGS_SF);
         break;
     case 5: /* p/pe */
-        rc |= (flags & EFLG_PF);
+        rc |= (flags & X86_EFLAGS_PF);
         break;
     case 7: /* le/ng */
-        rc |= (flags & EFLG_ZF);
+        rc |= (flags & X86_EFLAGS_ZF);
         /* fall through */
     case 6: /* l/nge */
-        rc |= (!(flags & EFLG_SF) != !(flags & EFLG_OF));
+        rc |= (!(flags & X86_EFLAGS_SF) != !(flags & X86_EFLAGS_OF));
         break;
     }
 
@@ -1179,7 +1163,7 @@ get_cpl(
 {
     struct segment_register reg;
 
-    if ( ctxt->regs->_eflags & EFLG_VM )
+    if ( ctxt->regs->_eflags & X86_EFLAGS_VM )
         return 3;
 
     if ( (ops->read_segment == NULL) ||
@@ -1197,7 +1181,7 @@ _mode_iopl(
     int cpl = get_cpl(ctxt, ops);
     if ( cpl == -1 )
         return -1;
-    return cpl <= MASK_EXTR(ctxt->regs->_eflags, EFLG_IOPL);
+    return cpl <= MASK_EXTR(ctxt->regs->_eflags, X86_EFLAGS_IOPL);
 }
 
 #define mode_ring0() ({                         \
@@ -1217,7 +1201,7 @@ _mode_iopl(
         rc = ops->read_cr(4, &cr4, ctxt);                    \
         if ( rc != X86EMUL_OKAY ) goto done;                 \
     }                                                        \
-    !!(cr4 & (_regs._eflags & EFLG_VM ? CR4_VME : CR4_PVI)); \
+    !!(cr4 & (_regs._eflags & X86_EFLAGS_VM ? CR4_VME : CR4_PVI)); \
 })
 
 static int ioport_access_check(
@@ -1230,7 +1214,7 @@ static int ioport_access_check(
     struct segment_register tr;
     int rc = X86EMUL_OKAY;
 
-    if ( !(ctxt->regs->_eflags & EFLG_VM) && mode_iopl() )
+    if ( !(ctxt->regs->_eflags & X86_EFLAGS_VM) && mode_iopl() )
         return X86EMUL_OKAY;
 
     fail_if(ops->read_segment == NULL);
@@ -1299,7 +1283,7 @@ in_protmode(
     struct x86_emulate_ctxt *ctxt,
     const struct x86_emulate_ops  *ops)
 {
-    return !(in_realmode(ctxt, ops) || (ctxt->regs->_eflags & EFLG_VM));
+    return !(in_realmode(ctxt, ops) || (ctxt->regs->_eflags & X86_EFLAGS_VM));
 }
 
 #define EAX 0
@@ -1822,8 +1806,8 @@ static int inject_swint(enum x86_swint_type type,
              * a 32bit OS.  Someone with many TUITs can see about reading the
              * TSS Software Interrupt Redirection bitmap.
              */
-            if ( (ctxt->regs->_eflags & EFLG_VM) &&
-                 ((ctxt->regs->_eflags & EFLG_IOPL) != EFLG_IOPL) )
+            if ( (ctxt->regs->_eflags & X86_EFLAGS_VM) &&
+                 ((ctxt->regs->_eflags & X86_EFLAGS_IOPL) != X86_EFLAGS_IOPL) )
                 goto raise_exn;
 
             /*
@@ -2313,7 +2297,7 @@ x86_decode(
             default:
                 BUG(); /* Shouldn't be possible. */
             case 2:
-                if ( state->regs->_eflags & EFLG_VM )
+                if ( state->regs->_eflags & X86_EFLAGS_VM )
                     break;
                 /* fall through */
             case 4:
@@ -2676,7 +2660,8 @@ x86_emulate(
     struct x86_emulate_state state;
     int rc;
     uint8_t b, d;
-    bool singlestep = (_regs._eflags & EFLG_TF) && !is_branch_step(ctxt, ops);
+    bool singlestep = (_regs._eflags & X86_EFLAGS_TF) &&
+	    !is_branch_step(ctxt, ops);
     struct operand src = { .reg = PTR_POISON };
     struct operand dst = { .reg = PTR_POISON };
     enum x86_swint_type swint_type;
@@ -2954,33 +2939,34 @@ x86_emulate(
         uint8_t al = _regs.al;
         unsigned int eflags = _regs._eflags;
 
-        _regs._eflags &= ~(EFLG_CF|EFLG_AF|EFLG_SF|EFLG_ZF|EFLG_PF);
-        if ( ((al & 0x0f) > 9) || (eflags & EFLG_AF) )
+        _regs._eflags &= ~(X86_EFLAGS_CF | X86_EFLAGS_AF | X86_EFLAGS_SF |
+                           X86_EFLAGS_ZF | X86_EFLAGS_PF);
+        if ( ((al & 0x0f) > 9) || (eflags & X86_EFLAGS_AF) )
         {
-            _regs._eflags |= EFLG_AF;
-            if ( b == 0x2f && (al < 6 || (eflags & EFLG_CF)) )
-                _regs._eflags |= EFLG_CF;
+            _regs._eflags |= X86_EFLAGS_AF;
+            if ( b == 0x2f && (al < 6 || (eflags & X86_EFLAGS_CF)) )
+                _regs._eflags |= X86_EFLAGS_CF;
             _regs.al += (b == 0x27) ? 6 : -6;
         }
-        if ( (al > 0x99) || (eflags & EFLG_CF) )
+        if ( (al > 0x99) || (eflags & X86_EFLAGS_CF) )
         {
             _regs.al += (b == 0x27) ? 0x60 : -0x60;
-            _regs._eflags |= EFLG_CF;
+            _regs._eflags |= X86_EFLAGS_CF;
         }
-        _regs._eflags |= !_regs.al ? EFLG_ZF : 0;
-        _regs._eflags |= ((int8_t)_regs.al < 0) ? EFLG_SF : 0;
-        _regs._eflags |= even_parity(_regs.al) ? EFLG_PF : 0;
+        _regs._eflags |= !_regs.al ? X86_EFLAGS_ZF : 0;
+        _regs._eflags |= ((int8_t)_regs.al < 0) ? X86_EFLAGS_SF : 0;
+        _regs._eflags |= even_parity(_regs.al) ? X86_EFLAGS_PF : 0;
         break;
     }
 
     case 0x37: /* aaa */
     case 0x3f: /* aas */
-        _regs._eflags &= ~EFLG_CF;
-        if ( (_regs.al > 9) || (_regs._eflags & EFLG_AF) )
+        _regs._eflags &= ~X86_EFLAGS_CF;
+        if ( (_regs.al > 9) || (_regs._eflags & X86_EFLAGS_AF) )
         {
             _regs.al += (b == 0x37) ? 6 : -6;
             _regs.ah += (b == 0x37) ? 1 : -1;
-            _regs._eflags |= EFLG_CF | EFLG_AF;
+            _regs._eflags |= X86_EFLAGS_CF | X86_EFLAGS_AF;
         }
         _regs.al &= 0x0f;
         break;
@@ -3086,12 +3072,12 @@ x86_emulate(
                 goto done;
             if ( src_rpl > (dst.val & 3) )
             {
-                _regs._eflags |= EFLG_ZF;
+                _regs._eflags |= X86_EFLAGS_ZF;
                 dst.val = (dst.val & ~3) | src_rpl;
             }
             else
             {
-                _regs._eflags &= ~EFLG_ZF;
+                _regs._eflags &= ~X86_EFLAGS_ZF;
                 dst.type = OP_NONE;
             }
             generate_exception_if(!in_protmode(ctxt, ops), EXC_UD);
@@ -3354,7 +3340,7 @@ x86_emulate(
             goto done;
 
         _regs.r(ip) = imm1;
-        singlestep = _regs._eflags & EFLG_TF;
+        singlestep = _regs._eflags & X86_EFLAGS_TF;
         break;
 
     case 0x9b:  /* wait/fwait */
@@ -3366,8 +3352,8 @@ x86_emulate(
         break;
 
     case 0x9c: /* pushf */
-        if ( (_regs._eflags & EFLG_VM) &&
-             MASK_EXTR(_regs._eflags, EFLG_IOPL) != 3 )
+        if ( (_regs._eflags & X86_EFLAGS_VM) &&
+             MASK_EXTR(_regs._eflags, X86_EFLAGS_IOPL) != 3 )
         {
             cr4 = 0;
             if ( op_bytes == 2 && ops->read_cr )
@@ -3377,21 +3363,21 @@ x86_emulate(
                     goto done;
             }
             generate_exception_if(!(cr4 & CR4_VME), EXC_GP, 0);
-            src.val = (_regs.flags & ~EFLG_IF) | EFLG_IOPL;
-            if ( _regs._eflags & EFLG_VIF )
-                src.val |= EFLG_IF;
+            src.val = (_regs.flags & ~X86_EFLAGS_IF) | X86_EFLAGS_IOPL;
+            if ( _regs._eflags & X86_EFLAGS_VIF )
+                src.val |= X86_EFLAGS_IF;
         }
         else
-            src.val = _regs.r(flags) & ~(EFLG_VM | EFLG_RF);
+            src.val = _regs.r(flags) & ~(X86_EFLAGS_VM | X86_EFLAGS_RF);
         goto push;
 
     case 0x9d: /* popf */ {
-        uint32_t mask = EFLG_VIP | EFLG_VIF | EFLG_VM;
+        uint32_t mask = X86_EFLAGS_VIP | X86_EFLAGS_VIF | X86_EFLAGS_VM;
 
         cr4 = 0;
         if ( !mode_ring0() )
         {
-            if ( _regs._eflags & EFLG_VM )
+            if ( _regs._eflags & X86_EFLAGS_VM )
             {
                 if ( op_bytes == 2 && ops->read_cr )
                 {
@@ -3400,12 +3386,12 @@ x86_emulate(
                         goto done;
                 }
                 generate_exception_if(!(cr4 & CR4_VME) &&
-                                      MASK_EXTR(_regs._eflags, EFLG_IOPL) != 3,
+                                      MASK_EXTR(_regs._eflags, X86_EFLAGS_IOPL) != 3,
                                       EXC_GP, 0);
             }
-            mask |= EFLG_IOPL;
+            mask |= X86_EFLAGS_IOPL;
             if ( !mode_iopl() )
-                mask |= EFLG_IF;
+                mask |= X86_EFLAGS_IF;
         }
         /* 64-bit mode: POP defaults to a 64-bit operand. */
         if ( mode_64bit() && (op_bytes == 4) )
@@ -3418,32 +3404,33 @@ x86_emulate(
             dst.val = (uint16_t)dst.val | (_regs._eflags & 0xffff0000u);
             if ( cr4 & CR4_VME )
             {
-                if ( dst.val & EFLG_IF )
+                if ( dst.val & X86_EFLAGS_IF )
                 {
-                    generate_exception_if(_regs._eflags & EFLG_VIP, EXC_GP, 0);
-                    dst.val |= EFLG_VIF;
+                    generate_exception_if(_regs._eflags & X86_EFLAGS_VIP,
+                                          EXC_GP, 0);
+                    dst.val |= X86_EFLAGS_VIF;
                 }
                 else
-                    dst.val &= ~EFLG_VIF;
-                mask &= ~EFLG_VIF;
+                    dst.val &= ~X86_EFLAGS_VIF;
+                mask &= ~X86_EFLAGS_VIF;
             }
         }
         dst.val &= EFLAGS_MODIFIABLE;
         _regs._eflags &= mask;
-        _regs._eflags |= (dst.val & ~mask) | EFLG_MBS;
+        _regs._eflags |= (dst.val & ~mask) | X86_EFLAGS_MBS;
         break;
     }
 
     case 0x9e: /* sahf */
         if ( mode_64bit() )
             vcpu_must_have(lahf_lm);
-        *(uint8_t *)&_regs._eflags = (_regs.ah & EFLAGS_MASK) | EFLG_MBS;
+        *(uint8_t *)&_regs._eflags = (_regs.ah & EFLAGS_MASK) | X86_EFLAGS_MBS;
         break;
 
     case 0x9f: /* lahf */
         if ( mode_64bit() )
             vcpu_must_have(lahf_lm);
-        _regs.ah = (_regs._eflags & EFLAGS_MASK) | EFLG_MBS;
+        _regs.ah = (_regs._eflags & EFLAGS_MASK) | X86_EFLAGS_MBS;
         break;
 
     case 0xa4 ... 0xa5: /* movs */ {
@@ -3487,8 +3474,8 @@ x86_emulate(
         put_rep_prefix(1);
         /* cmp: dst - src ==> src=*%%edi,dst=*%%esi ==> *%%esi - *%%edi */
         emulate_2op_SrcV("cmp", src, dst, _regs._eflags);
-        if ( (repe_prefix() && !(_regs._eflags & EFLG_ZF)) ||
-             (repne_prefix() && (_regs._eflags & EFLG_ZF)) )
+        if ( (repe_prefix() && !(_regs._eflags & X86_EFLAGS_ZF)) ||
+             (repne_prefix() && (_regs._eflags & X86_EFLAGS_ZF)) )
             _regs.r(ip) = next_eip;
         break;
     }
@@ -3537,8 +3524,8 @@ x86_emulate(
         /* cmp: %%eax - *%%edi ==> src=%%eax,dst=*%%edi ==> src - dst */
         dst.bytes = src.bytes;
         emulate_2op_SrcV("cmp", dst, src, _regs._eflags);
-        if ( (repe_prefix() && !(_regs._eflags & EFLG_ZF)) ||
-             (repne_prefix() && (_regs._eflags & EFLG_ZF)) )
+        if ( (repe_prefix() && !(_regs._eflags & X86_EFLAGS_ZF)) ||
+             (repne_prefix() && (_regs._eflags & X86_EFLAGS_ZF)) )
             _regs.r(ip) = next_eip;
         break;
     }
@@ -3683,7 +3670,7 @@ x86_emulate(
         goto done;
 
     case 0xce: /* into */
-        if ( !(_regs._eflags & EFLG_OF) )
+        if ( !(_regs._eflags & X86_EFLAGS_OF) )
             break;
         src.val = EXC_OF;
         swint_type = x86_swint_into;
@@ -3691,7 +3678,7 @@ x86_emulate(
 
     case 0xcf: /* iret */ {
         unsigned long sel, eip, eflags;
-        uint32_t mask = EFLG_VIP | EFLG_VIF | EFLG_VM;
+        uint32_t mask = X86_EFLAGS_VIP | X86_EFLAGS_VIF | X86_EFLAGS_VM;
 
         fail_if(!in_realmode(ctxt, ops));
         if ( (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes),
@@ -3705,7 +3692,7 @@ x86_emulate(
             eflags = (uint16_t)eflags | (_regs._eflags & 0xffff0000u);
         eflags &= EFLAGS_MODIFIABLE;
         _regs._eflags &= mask;
-        _regs._eflags |= (eflags & ~mask) | EFLG_MBS;
+        _regs._eflags |= (eflags & ~mask) | X86_EFLAGS_MBS;
         if ( (rc = load_seg(x86_seg_cs, sel, 1, &cs, ctxt, ops)) ||
              (rc = commit_far_branch(&cs, (uint32_t)eip)) )
             goto done;
@@ -3737,15 +3724,15 @@ x86_emulate(
             generate_exception_if(!base, EXC_DE);
             _regs.ax = ((al / base) << 8) | (al % base);
         }
-        _regs._eflags &= ~(EFLG_SF|EFLG_ZF|EFLG_PF);
-        _regs._eflags |= !_regs.al ? EFLG_ZF : 0;
-        _regs._eflags |= ((int8_t)_regs.al < 0) ? EFLG_SF : 0;
-        _regs._eflags |= even_parity(_regs.al) ? EFLG_PF : 0;
+        _regs._eflags &= ~(X86_EFLAGS_SF | X86_EFLAGS_ZF | X86_EFLAGS_PF);
+        _regs._eflags |= !_regs.al ? X86_EFLAGS_ZF : 0;
+        _regs._eflags |= ((int8_t)_regs.al < 0) ? X86_EFLAGS_SF : 0;
+        _regs._eflags |= even_parity(_regs.al) ? X86_EFLAGS_PF : 0;
         break;
     }
 
     case 0xd6: /* salc */
-        _regs.al = (_regs._eflags & EFLG_CF) ? 0xff : 0x00;
+        _regs.al = (_regs._eflags & X86_EFLAGS_CF) ? 0xff : 0x00;
         break;
 
     case 0xd7: /* xlat */ {
@@ -4263,7 +4250,7 @@ x86_emulate(
 
     case 0xe0 ... 0xe2: /* loop{,z,nz} */ {
         unsigned long count = get_loop_count(&_regs, ad_bytes);
-        int do_jmp = !(_regs._eflags & EFLG_ZF); /* loopnz */
+        int do_jmp = !(_regs._eflags & X86_EFLAGS_ZF); /* loopnz */
 
         if ( b == 0xe1 )
             do_jmp = !do_jmp; /* loopz */
@@ -4351,7 +4338,7 @@ x86_emulate(
         break;
 
     case 0xf5: /* cmc */
-        _regs._eflags ^= EFLG_CF;
+        _regs._eflags ^= X86_EFLAGS_CF;
         break;
 
     case 0xf6 ... 0xf7: /* Grp3 */
@@ -4371,21 +4358,21 @@ x86_emulate(
             emulate_1op("neg", dst, _regs._eflags);
             break;
         case 4: /* mul */
-            _regs._eflags &= ~(EFLG_OF|EFLG_CF);
+            _regs._eflags &= ~(X86_EFLAGS_OF | X86_EFLAGS_CF);
             switch ( dst.bytes )
             {
             case 1:
                 dst.val = _regs.al;
                 dst.val *= src.val;
                 if ( (uint8_t)dst.val != (uint16_t)dst.val )
-                    _regs._eflags |= EFLG_OF|EFLG_CF;
+                    _regs._eflags |= X86_EFLAGS_OF | X86_EFLAGS_CF;
                 dst.bytes = 2;
                 break;
             case 2:
                 dst.val = _regs.ax;
                 dst.val *= src.val;
                 if ( (uint16_t)dst.val != (uint32_t)dst.val )
-                    _regs._eflags |= EFLG_OF|EFLG_CF;
+                    _regs._eflags |= X86_EFLAGS_OF | X86_EFLAGS_CF;
                 _regs.dx = dst.val >> 16;
                 break;
 #ifdef __x86_64__
@@ -4393,7 +4380,7 @@ x86_emulate(
                 dst.val = _regs._eax;
                 dst.val *= src.val;
                 if ( (uint32_t)dst.val != dst.val )
-                    _regs._eflags |= EFLG_OF|EFLG_CF;
+                    _regs._eflags |= X86_EFLAGS_OF | X86_EFLAGS_CF;
                 _regs.rdx = dst.val >> 32;
                 break;
 #endif
@@ -4401,7 +4388,7 @@ x86_emulate(
                 u[0] = src.val;
                 u[1] = _regs.r(ax);
                 if ( mul_dbl(u) )
-                    _regs._eflags |= EFLG_OF|EFLG_CF;
+                    _regs._eflags |= X86_EFLAGS_OF | X86_EFLAGS_CF;
                 _regs.r(dx) = u[1];
                 dst.val = u[0];
                 break;
@@ -4409,13 +4396,13 @@ x86_emulate(
             break;
         case 5: /* imul */
         imul:
-            _regs._eflags &= ~(EFLG_OF|EFLG_CF);
+            _regs._eflags &= ~(X86_EFLAGS_OF | X86_EFLAGS_CF);
             switch ( dst.bytes )
             {
             case 1:
                 dst.val = (int8_t)src.val * (int8_t)_regs.al;
                 if ( (int8_t)dst.val != (int16_t)dst.val )
-                    _regs._eflags |= EFLG_OF|EFLG_CF;
+                    _regs._eflags |= X86_EFLAGS_OF | X86_EFLAGS_CF;
                 ASSERT(b > 0x6b);
                 dst.bytes = 2;
                 break;
@@ -4423,7 +4410,7 @@ x86_emulate(
                 dst.val = ((uint32_t)(int16_t)src.val *
                            (uint32_t)(int16_t)_regs.ax);
                 if ( (int16_t)dst.val != (int32_t)dst.val )
-                    _regs._eflags |= EFLG_OF|EFLG_CF;
+                    _regs._eflags |= X86_EFLAGS_OF | X86_EFLAGS_CF;
                 if ( b > 0x6b )
                     _regs.dx = dst.val >> 16;
                 break;
@@ -4432,7 +4419,7 @@ x86_emulate(
                 dst.val = ((uint64_t)(int32_t)src.val *
                            (uint64_t)(int32_t)_regs._eax);
                 if ( (int32_t)dst.val != dst.val )
-                    _regs._eflags |= EFLG_OF|EFLG_CF;
+                    _regs._eflags |= X86_EFLAGS_OF | X86_EFLAGS_CF;
                 if ( b > 0x6b )
                     _regs.rdx = dst.val >> 32;
                 break;
@@ -4441,7 +4428,7 @@ x86_emulate(
                 u[0] = src.val;
                 u[1] = _regs.r(ax);
                 if ( imul_dbl(u) )
-                    _regs._eflags |= EFLG_OF|EFLG_CF;
+                    _regs._eflags |= X86_EFLAGS_OF | X86_EFLAGS_CF;
                 if ( b > 0x6b )
                     _regs.r(dx) = u[1];
                 dst.val = u[0];
@@ -4542,46 +4529,47 @@ x86_emulate(
         break;
 
     case 0xf8: /* clc */
-        _regs._eflags &= ~EFLG_CF;
+        _regs._eflags &= ~X86_EFLAGS_CF;
         break;
 
     case 0xf9: /* stc */
-        _regs._eflags |= EFLG_CF;
+        _regs._eflags |= X86_EFLAGS_CF;
         break;
 
     case 0xfa: /* cli */
         if ( mode_iopl() )
-            _regs._eflags &= ~EFLG_IF;
+            _regs._eflags &= ~X86_EFLAGS_IF;
         else
         {
             generate_exception_if(!mode_vif(), EXC_GP, 0);
-            _regs._eflags &= ~EFLG_VIF;
+            _regs._eflags &= ~X86_EFLAGS_VIF;
         }
         break;
 
     case 0xfb: /* sti */
         if ( mode_iopl() )
         {
-            if ( !(_regs._eflags & EFLG_IF) )
+            if ( !(_regs._eflags & X86_EFLAGS_IF) )
                 ctxt->retire.sti = true;
-            _regs._eflags |= EFLG_IF;
+            _regs._eflags |= X86_EFLAGS_IF;
         }
         else
         {
-            generate_exception_if((_regs._eflags & EFLG_VIP) || !mode_vif(),
+            generate_exception_if((_regs._eflags & X86_EFLAGS_VIP) ||
+				  !mode_vif(),
                                   EXC_GP, 0);
-            if ( !(_regs._eflags & EFLG_VIF) )
+            if ( !(_regs._eflags & X86_EFLAGS_VIF) )
                 ctxt->retire.sti = true;
-            _regs._eflags |= EFLG_VIF;
+            _regs._eflags |= X86_EFLAGS_VIF;
         }
         break;
 
     case 0xfc: /* cld */
-        _regs._eflags &= ~EFLG_DF;
+        _regs._eflags &= ~X86_EFLAGS_DF;
         break;
 
     case 0xfd: /* std */
-        _regs._eflags |= EFLG_DF;
+        _regs._eflags |= X86_EFLAGS_DF;
         break;
 
     case 0xfe: /* Grp4 */
@@ -4643,7 +4631,7 @@ x86_emulate(
                 goto done;
             break;
         case 4: /* verr / verw */
-            _regs._eflags &= ~EFLG_ZF;
+            _regs._eflags &= ~X86_EFLAGS_ZF;
             switch ( rc = protmode_load_seg(x86_seg_none, src.val, false,
                                             &sreg, ctxt, ops) )
             {
@@ -4651,7 +4639,7 @@ x86_emulate(
                 if ( sreg.attr.fields.s &&
                      ((modrm_reg & 1) ? ((sreg.attr.fields.type & 0xa) == 0x2)
                                       : ((sreg.attr.fields.type & 0xa) != 0x8)) )
-                    _regs._eflags |= EFLG_ZF;
+                    _regs._eflags |= X86_EFLAGS_ZF;
                 break;
             case X86EMUL_EXCEPTION:
                 if ( ctxt->event_pending )
@@ -4681,9 +4669,9 @@ x86_emulate(
             vcpu_must_have(smap);
             generate_exception_if(vex.pfx || !mode_ring0(), EXC_UD);
 
-            _regs._eflags &= ~EFLG_AC;
+            _regs._eflags &= ~X86_EFLAGS_AC;
             if ( modrm == 0xcb )
-                _regs._eflags |= EFLG_AC;
+                _regs._eflags |= X86_EFLAGS_AC;
             goto complete_insn;
 
 #ifdef __XEN__
@@ -4717,7 +4705,7 @@ x86_emulate(
             generate_exception_if(!vcpu_has_rtm() && !vcpu_has_hle(),
                                   EXC_UD);
             /* Neither HLE nor RTM can be active when we get here. */
-            _regs._eflags |= EFLG_ZF;
+            _regs._eflags |= X86_EFLAGS_ZF;
             goto complete_insn;
 
         case 0xdf: /* invlpga */
@@ -4868,7 +4856,7 @@ x86_emulate(
 
     case X86EMUL_OPC(0x0f, 0x02): /* lar */
         generate_exception_if(!in_protmode(ctxt, ops), EXC_UD);
-        _regs._eflags &= ~EFLG_ZF;
+        _regs._eflags &= ~X86_EFLAGS_ZF;
         switch ( rc = protmode_load_seg(x86_seg_none, src.val, false, &sreg,
                                         ctxt, ops) )
         {
@@ -4888,12 +4876,12 @@ x86_emulate(
                 case 0x09: /* available 32/64-bit TSS */
                 case 0x0b: /* busy 32/64-bit TSS */
                 case 0x0c: /* 32/64-bit call gate */
-                    _regs._eflags |= EFLG_ZF;
+                    _regs._eflags |= X86_EFLAGS_ZF;
                     break;
                 }
             }
             else
-                _regs._eflags |= EFLG_ZF;
+                _regs._eflags |= X86_EFLAGS_ZF;
             break;
         case X86EMUL_EXCEPTION:
             if ( ctxt->event_pending )
@@ -4906,7 +4894,7 @@ x86_emulate(
             rc = X86EMUL_OKAY;
             break;
         }
-        if ( _regs._eflags & EFLG_ZF )
+        if ( _regs._eflags & X86_EFLAGS_ZF )
             dst.val = ((sreg.attr.bytes & 0xff) << 8) |
                       ((sreg.limit >> (sreg.attr.fields.g ? 12 : 0)) &
                        0xf0000) |
@@ -4917,7 +4905,7 @@ x86_emulate(
 
     case X86EMUL_OPC(0x0f, 0x03): /* lsl */
         generate_exception_if(!in_protmode(ctxt, ops), EXC_UD);
-        _regs._eflags &= ~EFLG_ZF;
+        _regs._eflags &= ~X86_EFLAGS_ZF;
         switch ( rc = protmode_load_seg(x86_seg_none, src.val, false, &sreg,
                                         ctxt, ops) )
         {
@@ -4934,12 +4922,12 @@ x86_emulate(
                 case 0x02: /* LDT */
                 case 0x09: /* available 32/64-bit TSS */
                 case 0x0b: /* busy 32/64-bit TSS */
-                    _regs._eflags |= EFLG_ZF;
+                    _regs._eflags |= X86_EFLAGS_ZF;
                     break;
                 }
             }
             else
-                _regs._eflags |= EFLG_ZF;
+                _regs._eflags |= X86_EFLAGS_ZF;
             break;
         case X86EMUL_EXCEPTION:
             if ( ctxt->event_pending )
@@ -4952,7 +4940,7 @@ x86_emulate(
             rc = X86EMUL_OKAY;
             break;
         }
-        if ( _regs._eflags & EFLG_ZF )
+        if ( _regs._eflags & X86_EFLAGS_ZF )
             dst.val = sreg.limit;
         else
             dst.type = OP_NONE;
@@ -4988,7 +4976,7 @@ x86_emulate(
             cs.attr.bytes = 0xa9b; /* L+DB+P+S+Code */
 
             _regs.rcx = _regs.rip;
-            _regs.r11 = _regs._eflags & ~EFLG_RF;
+            _regs.r11 = _regs._eflags & ~X86_EFLAGS_RF;
 
             if ( (rc = ops->read_msr(mode_64bit() ? MSR_LSTAR : MSR_CSTAR,
                                      &msr_content, ctxt)) != 0 )
@@ -4997,7 +4985,7 @@ x86_emulate(
 
             if ( (rc = ops->read_msr(MSR_FMASK, &msr_content, ctxt)) != 0 )
                 goto done;
-            _regs._eflags &= ~(msr_content | EFLG_RF);
+            _regs._eflags &= ~(msr_content | X86_EFLAGS_RF);
         }
         else
 #endif
@@ -5006,7 +4994,7 @@ x86_emulate(
 
             _regs.r(cx) = _regs._eip;
             _regs._eip = msr_content;
-            _regs._eflags &= ~(EFLG_VM | EFLG_IF | EFLG_RF);
+            _regs._eflags &= ~(X86_EFLAGS_VM | X86_EFLAGS_IF | X86_EFLAGS_RF);
         }
 
         fail_if(ops->write_segment == NULL);
@@ -5016,20 +5004,20 @@ x86_emulate(
 
         /*
          * SYSCALL (unlike most instructions) evaluates its singlestep action
-         * based on the resulting EFLG_TF, not the starting EFLG_TF.
+         * based on the resulting EFLAGS.TF, not the starting EFLAGS.TF.
          *
          * As the #DB is raised after the CPL change and before the OS can
          * switch stack, it is a large risk for privilege escalation.
          *
-         * 64bit kernels should mask EFLG_TF in MSR_FMASK to avoid any
+         * 64bit kernels should mask EFLAGS.TF in MSR_FMASK to avoid any
          * vulnerability.  Running the #DB handler on an IST stack is also a
          * mitigation.
          *
-         * 32bit kernels have no ability to mask EFLG_TF at all.  Their only
-         * mitigation is to use a task gate for handling #DB (or to not use
-         * enable EFER.SCE to start with).
+         * 32bit kernels have no ability to mask EFLAGS.TF at all.
+         * Their only mitigation is to use a task gate for handling
+         * #DB (or to not use enable EFER.SCE to start with).
          */
-        singlestep = _regs._eflags & EFLG_TF;
+        singlestep = _regs._eflags & X86_EFLAGS_TF;
 
         break;
     }
@@ -5260,7 +5248,7 @@ x86_emulate(
         if ( lm < 0 )
             goto cannot_emulate;
 
-        _regs._eflags &= ~(EFLG_VM | EFLG_IF | EFLG_RF);
+        _regs._eflags &= ~(X86_EFLAGS_VM | X86_EFLAGS_IF | X86_EFLAGS_RF);
 
         cs.sel = msr_content & ~3; /* SELECTOR_RPL_MASK */
         cs.base = 0;   /* flat segment */
@@ -5286,7 +5274,7 @@ x86_emulate(
             goto done;
         _regs.r(sp) = lm ? msr_content : (uint32_t)msr_content;
 
-        singlestep = _regs._eflags & EFLG_TF;
+        singlestep = _regs._eflags & X86_EFLAGS_TF;
         break;
     }
 
@@ -5328,7 +5316,7 @@ x86_emulate(
         _regs.r(ip) = op_bytes == 8 ? _regs.r(dx) : _regs._edx;
         _regs.r(sp) = op_bytes == 8 ? _regs.r(cx) : _regs._ecx;
 
-        singlestep = _regs._eflags & EFLG_TF;
+        singlestep = _regs._eflags & X86_EFLAGS_TF;
         break;
     }
 
@@ -5517,14 +5505,15 @@ x86_emulate(
                    ((dst.orig_val << shift) |
                     ((src.val >> (width - shift)) & ((1ull << shift) - 1))));
         dst.val = truncate_word(dst.val, dst.bytes);
-        _regs._eflags &= ~(EFLG_OF|EFLG_SF|EFLG_ZF|EFLG_PF|EFLG_CF);
+        _regs._eflags &= ~(X86_EFLAGS_OF | X86_EFLAGS_SF | X86_EFLAGS_ZF |
+                           X86_EFLAGS_PF | X86_EFLAGS_CF);
         if ( (dst.val >> ((b & 8) ? (shift - 1) : (width - shift))) & 1 )
-            _regs._eflags |= EFLG_CF;
+            _regs._eflags |= X86_EFLAGS_CF;
         if ( ((dst.val ^ dst.orig_val) >> (width - 1)) & 1 )
-            _regs._eflags |= EFLG_OF;
-        _regs._eflags |= ((dst.val >> (width - 1)) & 1) ? EFLG_SF : 0;
-        _regs._eflags |= (dst.val == 0) ? EFLG_ZF : 0;
-        _regs._eflags |= even_parity(dst.val) ? EFLG_PF : 0;
+            _regs._eflags |= X86_EFLAGS_OF;
+        _regs._eflags |= ((dst.val >> (width - 1)) & 1) ? X86_EFLAGS_SF : 0;
+        _regs._eflags |= (dst.val == 0) ? X86_EFLAGS_ZF : 0;
+        _regs._eflags |= even_parity(dst.val) ? X86_EFLAGS_PF : 0;
         break;
     }
 
@@ -5624,7 +5613,7 @@ x86_emulate(
         src.val = _regs.r(ax);
         /* cmp: %%eax - dst ==> dst and src swapped for macro invocation */
         emulate_2op_SrcV("cmp", dst, src, _regs._eflags);
-        if ( _regs._eflags & EFLG_ZF )
+        if ( _regs._eflags & X86_EFLAGS_ZF )
         {
             /* Success: write back to memory. */
             dst.val = src.orig_val;
@@ -5663,7 +5652,7 @@ x86_emulate(
         asm ( "popcnt %1,%0" : "=r" (dst.val) : "rm" (src.val) );
         _regs._eflags &= ~EFLAGS_MASK;
         if ( !dst.val )
-            _regs._eflags |= EFLG_ZF;
+            _regs._eflags |= X86_EFLAGS_ZF;
         break;
 
     case X86EMUL_OPC(0x0f, 0xba): /* Grp8 */
@@ -5688,21 +5677,21 @@ x86_emulate(
         asm ( "bsf %2,%0" ASM_FLAG_OUT(, "; setz %1")
               : "=r" (dst.val), ASM_FLAG_OUT("=@ccz", "=qm") (zf)
               : "rm" (src.val) );
-        _regs._eflags &= ~EFLG_ZF;
+        _regs._eflags &= ~X86_EFLAGS_ZF;
         if ( (vex.pfx == vex_f3) && vcpu_has_bmi1() )
         {
-            _regs._eflags &= ~EFLG_CF;
+            _regs._eflags &= ~X86_EFLAGS_CF;
             if ( zf )
             {
-                _regs._eflags |= EFLG_CF;
+                _regs._eflags |= X86_EFLAGS_CF;
                 dst.val = op_bytes * 8;
             }
             else if ( !dst.val )
-                _regs._eflags |= EFLG_ZF;
+                _regs._eflags |= X86_EFLAGS_ZF;
         }
         else if ( zf )
         {
-            _regs._eflags |= EFLG_ZF;
+            _regs._eflags |= X86_EFLAGS_ZF;
             dst.type = OP_NONE;
         }
         break;
@@ -5715,25 +5704,25 @@ x86_emulate(
         asm ( "bsr %2,%0" ASM_FLAG_OUT(, "; setz %1")
               : "=r" (dst.val), ASM_FLAG_OUT("=@ccz", "=qm") (zf)
               : "rm" (src.val) );
-        _regs._eflags &= ~EFLG_ZF;
+        _regs._eflags &= ~X86_EFLAGS_ZF;
         if ( (vex.pfx == vex_f3) && vcpu_has_lzcnt() )
         {
-            _regs._eflags &= ~EFLG_CF;
+            _regs._eflags &= ~X86_EFLAGS_CF;
             if ( zf )
             {
-                _regs._eflags |= EFLG_CF;
+                _regs._eflags |= X86_EFLAGS_CF;
                 dst.val = op_bytes * 8;
             }
             else
             {
                 dst.val = op_bytes * 8 - 1 - dst.val;
                 if ( !dst.val )
-                    _regs._eflags |= EFLG_ZF;
+                    _regs._eflags |= X86_EFLAGS_ZF;
             }
         }
         else if ( zf )
         {
-            _regs._eflags |= EFLG_ZF;
+            _regs._eflags |= X86_EFLAGS_ZF;
             dst.type = OP_NONE;
         }
         break;
@@ -5807,7 +5796,7 @@ x86_emulate(
                 }
                 _regs._eflags &= ~EFLAGS_MASK;
                 if ( carry )
-                    _regs._eflags |= EFLG_CF;
+                    _regs._eflags |= X86_EFLAGS_CF;
                 break;
 #endif
 
@@ -5850,7 +5839,7 @@ x86_emulate(
                 }
                 _regs._eflags &= ~EFLAGS_MASK;
                 if ( carry )
-                    _regs._eflags |= EFLG_CF;
+                    _regs._eflags |= X86_EFLAGS_CF;
                 break;
 #endif
             }
@@ -5899,7 +5888,7 @@ x86_emulate(
             /* Expected != actual: store actual to rDX:rAX and clear ZF. */
             _regs.r(ax) = !(rex_prefix & REX_W) ? old->u32[0] : old->u64[0];
             _regs.r(dx) = !(rex_prefix & REX_W) ? old->u32[1] : old->u64[1];
-            _regs._eflags &= ~EFLG_ZF;
+            _regs._eflags &= ~X86_EFLAGS_ZF;
         }
         else
         {
@@ -5921,7 +5910,7 @@ x86_emulate(
             if ( (rc = ops->cmpxchg(ea.mem.seg, ea.mem.off, old, aux,
                                     op_bytes, ctxt)) != X86EMUL_OKAY )
                 goto done;
-            _regs._eflags |= EFLG_ZF;
+            _regs._eflags |= X86_EFLAGS_ZF;
         }
         break;
     }
@@ -6070,7 +6059,7 @@ x86_emulate(
     case X86EMUL_OPC_66(0x0f38, 0xf6): /* adcx r/m,r */
     case X86EMUL_OPC_F3(0x0f38, 0xf6): /* adox r/m,r */
     {
-        unsigned int mask = rep_prefix() ? EFLG_OF : EFLG_CF;
+        unsigned int mask = rep_prefix() ? X86_EFLAGS_OF : X86_EFLAGS_CF;
         unsigned int aux = _regs._eflags & mask ? ~0 : 0;
         bool carry;
 
@@ -6263,7 +6252,7 @@ x86_emulate(
         rc = X86EMUL_OKAY;
     }
 
-    ctxt->regs->_eflags &= ~EFLG_RF;
+    ctxt->regs->_eflags &= ~X86_EFLAGS_RF;
 
  done:
     _put_fpu();
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 06/11] x86emul: use msr definitions in msr-index.h
  2017-02-01 12:02 [PATCH v3 00/11] fuzz: update x86emul fuzzer Wei Liu
                   ` (4 preceding siblings ...)
  2017-02-01 12:02 ` [PATCH v3 05/11] x86emul: use eflags definitions in x86-defns.h Wei Liu
@ 2017-02-01 12:02 ` Wei Liu
  2017-02-01 12:02 ` [PATCH v3 07/11] x86: add UMIP CR4 bit Wei Liu
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 28+ messages in thread
From: Wei Liu @ 2017-02-01 12:02 UTC (permalink / raw)
  To: Xen-devel; +Cc: Wei Liu

Change the names used in code according to numeric values. Remove the
now unused macros in x86_emualte.c and fix indentation. This in turns
requires including msr-index.h  and removing duplicates in userspace
x86_emulate.c in userspace harness program.

No functional change.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
v3:
1. Adapt build system to previous patch
---
 tools/fuzz/x86_instruction_emulator/Makefile |  4 +--
 tools/tests/x86_emulator/Makefile            |  4 +--
 tools/tests/x86_emulator/x86_emulate.c       |  3 --
 tools/tests/x86_emulator/x86_emulate.h       |  1 +
 xen/arch/x86/x86_emulate/x86_emulate.c       | 43 ++++++++++------------------
 5 files changed, 20 insertions(+), 35 deletions(-)

diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile
index 240bc58b06..1dfacf7da4 100644
--- a/tools/fuzz/x86_instruction_emulator/Makefile
+++ b/tools/fuzz/x86_instruction_emulator/Makefile
@@ -11,7 +11,7 @@ endif
 x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
 	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
 
-asm/x86-vendors.h asm/x86-defns.h:
+asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h:
 	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm
 
 x86_emulate.c x86_emulate.h: %:
@@ -19,7 +19,7 @@ x86_emulate.c x86_emulate.h: %:
 
 CFLAGS += $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -I.
 
-x86_emulate.h: asm/x86-vendors.h asm/x86-defns.h
+x86_emulate.h: asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h
 
 x86_emulate.o: x86_emulate.c x86_emulate.h x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
 
diff --git a/tools/tests/x86_emulator/Makefile b/tools/tests/x86_emulator/Makefile
index 6978288294..1fbcd8bd71 100644
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -43,10 +43,10 @@ install:
 x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
 	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
 
-asm/x86-vendors.h asm/x86-defns.h:
+asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h:
 	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm
 
-x86_emulate.h: asm/x86-vendors.h asm/x86-defns.h
+x86_emulate.h: asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h
 
 HOSTCFLAGS += $(CFLAGS_xeninclude) -I.
 
diff --git a/tools/tests/x86_emulator/x86_emulate.c b/tools/tests/x86_emulator/x86_emulate.c
index 615326259b..cda0fd8ee1 100644
--- a/tools/tests/x86_emulator/x86_emulate.c
+++ b/tools/tests/x86_emulator/x86_emulate.c
@@ -2,9 +2,6 @@
 
 #include <sys/mman.h>
 
-#define EFER_SCE       (1 << 0)
-#define EFER_LMA       (1 << 10)
-
 #define cpu_has_amd_erratum(nr) 0
 #define mark_regs_dirty(r) ((void)(r))
 #define cpu_has_mpx false
diff --git a/tools/tests/x86_emulator/x86_emulate.h b/tools/tests/x86_emulator/x86_emulate.h
index e064deaeea..6d6f5125ca 100644
--- a/tools/tests/x86_emulator/x86_emulate.h
+++ b/tools/tests/x86_emulator/x86_emulate.h
@@ -6,6 +6,7 @@
 #include <string.h>
 #include <xen/xen.h>
 
+#include <asm/msr-index.h>
 #include <asm/x86-defns.h>
 #include <asm/x86-vendors.h>
 
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index 6e8221d9d8..e649205848 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -415,23 +415,6 @@ typedef union {
 # define ASM_FLAG_OUT(yes, no) no
 #endif
 
-/* MSRs. */
-#define MSR_TSC          0x00000010
-#define MSR_SYSENTER_CS  0x00000174
-#define MSR_SYSENTER_ESP 0x00000175
-#define MSR_SYSENTER_EIP 0x00000176
-#define MSR_DEBUGCTL     0x000001d9
-#define DEBUGCTL_BTF     (1 << 1)
-#define MSR_BNDCFGS      0x00000d90
-#define BNDCFG_ENABLE    (1 << 0)
-#define BNDCFG_PRESERVE  (1 << 1)
-#define MSR_EFER         0xc0000080
-#define MSR_STAR         0xc0000081
-#define MSR_LSTAR        0xc0000082
-#define MSR_CSTAR        0xc0000083
-#define MSR_FMASK        0xc0000084
-#define MSR_TSC_AUX      0xc0000103
-
 /* Control register flags. */
 #define CR0_PE    (1<<0)
 #define CR0_MP    (1<<1)
@@ -1731,8 +1714,8 @@ static bool is_branch_step(struct x86_emulate_ctxt *ctxt,
     uint64_t debugctl;
 
     return ops->read_msr &&
-           ops->read_msr(MSR_DEBUGCTL, &debugctl, ctxt) == X86EMUL_OKAY &&
-           (debugctl & DEBUGCTL_BTF);
+           ops->read_msr(MSR_IA32_DEBUGCTLMSR, &debugctl, ctxt) == X86EMUL_OKAY &&
+           (debugctl & IA32_DEBUGCTLMSR_BTF);
 }
 
 static bool umip_active(struct x86_emulate_ctxt *ctxt,
@@ -1894,9 +1877,9 @@ static void adjust_bnd(struct x86_emulate_ctxt *ctxt,
     if ( !mode_ring0() )
         bndcfg = read_bndcfgu();
     else if ( !ops->read_msr ||
-              ops->read_msr(MSR_BNDCFGS, &bndcfg, ctxt) != X86EMUL_OKAY )
+              ops->read_msr(MSR_IA32_BNDCFGS, &bndcfg, ctxt) != X86EMUL_OKAY )
         return;
-    if ( (bndcfg & BNDCFG_ENABLE) && !(bndcfg & BNDCFG_PRESERVE) )
+    if ( (bndcfg & IA32_BNDCFGS_ENABLE) && !(bndcfg & IA32_BNDCFGS_PRESERVE) )
     {
         /*
          * Using BNDMK or any other MPX instruction here is pointless, as
@@ -4983,7 +4966,7 @@ x86_emulate(
                 goto done;
             _regs.rip = msr_content;
 
-            if ( (rc = ops->read_msr(MSR_FMASK, &msr_content, ctxt)) != 0 )
+            if ( (rc = ops->read_msr(MSR_SYSCALL_MASK, &msr_content, ctxt)) != 0 )
                 goto done;
             _regs._eflags &= ~(msr_content | X86_EFLAGS_RF);
         }
@@ -5009,7 +4992,7 @@ x86_emulate(
          * As the #DB is raised after the CPL change and before the OS can
          * switch stack, it is a large risk for privilege escalation.
          *
-         * 64bit kernels should mask EFLAGS.TF in MSR_FMASK to avoid any
+         * 64bit kernels should mask EFLAGS.TF in MSR_SYSCALL_MASK to avoid any
          * vulnerability.  Running the #DB handler on an IST stack is also a
          * mitigation.
          *
@@ -5207,7 +5190,7 @@ x86_emulate(
             generate_exception_if(cr4 & CR4_TSD, EXC_GP, 0);
         }
         fail_if(ops->read_msr == NULL);
-        if ( (rc = ops->read_msr(MSR_TSC, &val, ctxt)) != 0 )
+        if ( (rc = ops->read_msr(MSR_IA32_TSC, &val, ctxt)) != 0 )
             goto done;
         _regs.r(dx) = val >> 32;
         _regs.r(ax) = (uint32_t)val;
@@ -5240,7 +5223,8 @@ x86_emulate(
         generate_exception_if(!in_protmode(ctxt, ops), EXC_GP, 0);
 
         fail_if(ops->read_msr == NULL);
-        if ( (rc = ops->read_msr(MSR_SYSENTER_CS, &msr_content, ctxt)) != 0 )
+        if ( (rc = ops->read_msr(MSR_IA32_SYSENTER_CS, &msr_content, ctxt))
+             != 0 )
             goto done;
 
         generate_exception_if(!(msr_content & 0xfffc), EXC_GP, 0);
@@ -5266,11 +5250,13 @@ x86_emulate(
              (rc = ops->write_segment(x86_seg_ss, &sreg, ctxt)) != 0 )
             goto done;
 
-        if ( (rc = ops->read_msr(MSR_SYSENTER_EIP, &msr_content, ctxt)) != 0 )
+        if ( (rc = ops->read_msr(MSR_IA32_SYSENTER_EIP, &msr_content, ctxt))
+             != 0 )
             goto done;
         _regs.r(ip) = lm ? msr_content : (uint32_t)msr_content;
 
-        if ( (rc = ops->read_msr(MSR_SYSENTER_ESP, &msr_content, ctxt)) != 0 )
+        if ( (rc = ops->read_msr(MSR_IA32_SYSENTER_ESP, &msr_content, ctxt))
+             != 0 )
             goto done;
         _regs.r(sp) = lm ? msr_content : (uint32_t)msr_content;
 
@@ -5287,7 +5273,8 @@ x86_emulate(
         generate_exception_if(!in_protmode(ctxt, ops), EXC_GP, 0);
 
         fail_if(ops->read_msr == NULL);
-        if ( (rc = ops->read_msr(MSR_SYSENTER_CS, &msr_content, ctxt)) != 0 )
+        if ( (rc = ops->read_msr(MSR_IA32_SYSENTER_CS, &msr_content, ctxt))
+             != 0 )
             goto done;
 
         generate_exception_if(!(msr_content & 0xfffc), EXC_GP, 0);
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 07/11] x86: add UMIP CR4 bit
  2017-02-01 12:02 [PATCH v3 00/11] fuzz: update x86emul fuzzer Wei Liu
                   ` (5 preceding siblings ...)
  2017-02-01 12:02 ` [PATCH v3 06/11] x86emul: use msr definitions in msr-index.h Wei Liu
@ 2017-02-01 12:02 ` Wei Liu
  2017-02-01 12:02 ` [PATCH v3 08/11] x86emul: use CR definitions in x86-defns.h Wei Liu
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 28+ messages in thread
From: Wei Liu @ 2017-02-01 12:02 UTC (permalink / raw)
  To: Xen-devel; +Cc: Wei Liu

It will be used later to remove duplicates in x86emul.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
 xen/include/asm-x86/x86-defns.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/include/asm-x86/x86-defns.h b/xen/include/asm-x86/x86-defns.h
index 48ea5ea03b..70453e8dfb 100644
--- a/xen/include/asm-x86/x86-defns.h
+++ b/xen/include/asm-x86/x86-defns.h
@@ -56,6 +56,7 @@
 #define X86_CR4_PCE        0x00000100 /* enable performance counters at ipl 3 */
 #define X86_CR4_OSFXSR     0x00000200 /* enable fast FPU save and restore */
 #define X86_CR4_OSXMMEXCPT 0x00000400 /* enable unmasked SSE exceptions */
+#define X86_CR4_UMIP       0x00000800 /* enable UMIP */
 #define X86_CR4_VMXE       0x00002000 /* enable VMX */
 #define X86_CR4_SMXE       0x00004000 /* enable SMX */
 #define X86_CR4_FSGSBASE   0x00010000 /* enable {rd,wr}{fs,gs}base */
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 08/11] x86emul: use CR definitions in x86-defns.h
  2017-02-01 12:02 [PATCH v3 00/11] fuzz: update x86emul fuzzer Wei Liu
                   ` (6 preceding siblings ...)
  2017-02-01 12:02 ` [PATCH v3 07/11] x86: add UMIP CR4 bit Wei Liu
@ 2017-02-01 12:02 ` Wei Liu
  2017-02-01 12:02 ` [PATCH v3 09/11] fuzz/x86emul: update fuzzer Wei Liu
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 28+ messages in thread
From: Wei Liu @ 2017-02-01 12:02 UTC (permalink / raw)
  To: Xen-devel; +Cc: Wei Liu

And remove the duplicates.

No functional change.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/x86_emulate/x86_emulate.c | 45 ++++++++++++----------------------
 1 file changed, 15 insertions(+), 30 deletions(-)

diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index e649205848..77b9de4a1e 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -415,21 +415,6 @@ typedef union {
 # define ASM_FLAG_OUT(yes, no) no
 #endif
 
-/* Control register flags. */
-#define CR0_PE    (1<<0)
-#define CR0_MP    (1<<1)
-#define CR0_EM    (1<<2)
-#define CR0_TS    (1<<3)
-
-#define CR4_VME        (1<<0)
-#define CR4_PVI        (1<<1)
-#define CR4_TSD        (1<<2)
-#define CR4_OSFXSR     (1<<9)
-#define CR4_OSXMMEXCPT (1<<10)
-#define CR4_UMIP       (1<<11)
-#define CR4_FSGSBASE   (1<<16)
-#define CR4_OSXSAVE    (1<<18)
-
 /* Floating point status word definitions. */
 #define FSW_ES    (1U << 7)
 
@@ -810,7 +795,7 @@ static int _get_fpu(
             if ( rc != X86EMUL_OKAY )
                 return rc;
             generate_exception_if(!(cr4 & ((type == X86EMUL_FPU_xmm)
-                                           ? CR4_OSFXSR : CR4_OSXSAVE)),
+                                           ? X86_CR4_OSFXSR : X86_CR4_OSXSAVE)),
                                   EXC_UD);
         }
 
@@ -820,16 +805,16 @@ static int _get_fpu(
         if ( type >= X86EMUL_FPU_ymm )
         {
             /* Should be unreachable if VEX decoding is working correctly. */
-            ASSERT((cr0 & CR0_PE) && !(ctxt->regs->_eflags & X86_EFLAGS_VM));
+            ASSERT((cr0 & X86_CR0_PE) && !(ctxt->regs->_eflags & X86_EFLAGS_VM));
         }
-        if ( cr0 & CR0_EM )
+        if ( cr0 & X86_CR0_EM )
         {
             generate_exception_if(type == X86EMUL_FPU_fpu, EXC_NM);
             generate_exception_if(type == X86EMUL_FPU_mmx, EXC_UD);
             generate_exception_if(type == X86EMUL_FPU_xmm, EXC_UD);
         }
-        generate_exception_if((cr0 & CR0_TS) &&
-                              (type != X86EMUL_FPU_wait || (cr0 & CR0_MP)),
+        generate_exception_if((cr0 & X86_CR0_TS) &&
+                              (type != X86EMUL_FPU_wait || (cr0 & X86_CR0_MP)),
                               EXC_NM);
     }
 
@@ -852,7 +837,7 @@ do {                                                            \
     _put_fpu();                                                 \
     if ( (_fic)->exn_raised == EXC_XM && ops->read_cr &&        \
          ops->read_cr(4, &cr4, ctxt) == X86EMUL_OKAY &&         \
-         !(cr4 & CR4_OSXMMEXCPT) )                              \
+         !(cr4 & X86_CR4_OSXMMEXCPT) )				\
         (_fic)->exn_raised = EXC_UD;                            \
     generate_exception_if((_fic)->exn_raised >= 0,              \
                           (_fic)->exn_raised);                  \
@@ -1184,7 +1169,7 @@ _mode_iopl(
         rc = ops->read_cr(4, &cr4, ctxt);                    \
         if ( rc != X86EMUL_OKAY ) goto done;                 \
     }                                                        \
-    !!(cr4 & (_regs._eflags & X86_EFLAGS_VM ? CR4_VME : CR4_PVI)); \
+    !!(cr4 & (_regs._eflags & X86_EFLAGS_VM ? X86_CR4_VME : X86_CR4_PVI)); \
 })
 
 static int ioport_access_check(
@@ -1258,7 +1243,7 @@ in_realmode(
         return 0;
 
     rc = ops->read_cr(0, &cr0, ctxt);
-    return (!rc && !(cr0 & CR0_PE));
+    return (!rc && !(cr0 & X86_CR0_PE));
 }
 
 static bool
@@ -1726,7 +1711,7 @@ static bool umip_active(struct x86_emulate_ctxt *ctxt,
     /* Intentionally not using mode_ring0() here to avoid its fail_if(). */
     return get_cpl(ctxt, ops) > 0 &&
            ops->read_cr && ops->read_cr(4, &cr4, ctxt) == X86EMUL_OKAY &&
-           (cr4 & CR4_UMIP);
+           (cr4 & X86_CR4_UMIP);
 }
 
 /* Inject a software interrupt/exception, emulating if needed. */
@@ -3345,7 +3330,7 @@ x86_emulate(
                 if ( rc != X86EMUL_OKAY )
                     goto done;
             }
-            generate_exception_if(!(cr4 & CR4_VME), EXC_GP, 0);
+            generate_exception_if(!(cr4 & X86_CR4_VME), EXC_GP, 0);
             src.val = (_regs.flags & ~X86_EFLAGS_IF) | X86_EFLAGS_IOPL;
             if ( _regs._eflags & X86_EFLAGS_VIF )
                 src.val |= X86_EFLAGS_IF;
@@ -3368,7 +3353,7 @@ x86_emulate(
                     if ( rc != X86EMUL_OKAY )
                         goto done;
                 }
-                generate_exception_if(!(cr4 & CR4_VME) &&
+                generate_exception_if(!(cr4 & X86_CR4_VME) &&
                                       MASK_EXTR(_regs._eflags, X86_EFLAGS_IOPL) != 3,
                                       EXC_GP, 0);
             }
@@ -3385,7 +3370,7 @@ x86_emulate(
         if ( op_bytes == 2 )
         {
             dst.val = (uint16_t)dst.val | (_regs._eflags & 0xffff0000u);
-            if ( cr4 & CR4_VME )
+            if ( cr4 & X86_CR4_VME )
             {
                 if ( dst.val & X86_EFLAGS_IF )
                 {
@@ -5009,7 +4994,7 @@ x86_emulate(
         generate_exception_if(!mode_ring0(), EXC_GP, 0);
         fail_if((ops->read_cr == NULL) || (ops->write_cr == NULL));
         if ( (rc = ops->read_cr(0, &dst.val, ctxt)) != X86EMUL_OKAY ||
-             (rc = ops->write_cr(0, dst.val & ~CR0_TS, ctxt)) != X86EMUL_OKAY )
+             (rc = ops->write_cr(0, dst.val & ~X86_CR0_TS, ctxt)) != X86EMUL_OKAY )
             goto done;
         break;
 
@@ -5187,7 +5172,7 @@ x86_emulate(
             fail_if(ops->read_cr == NULL);
             if ( (rc = ops->read_cr(4, &cr4, ctxt)) )
                 goto done;
-            generate_exception_if(cr4 & CR4_TSD, EXC_GP, 0);
+            generate_exception_if(cr4 & X86_CR4_TSD, EXC_GP, 0);
         }
         fail_if(ops->read_msr == NULL);
         if ( (rc = ops->read_msr(MSR_IA32_TSC, &val, ctxt)) != 0 )
@@ -5560,7 +5545,7 @@ x86_emulate(
         fail_if(!ops->read_cr);
         if ( (rc = ops->read_cr(4, &cr4, ctxt)) != X86EMUL_OKAY )
             goto done;
-        generate_exception_if(!(cr4 & CR4_FSGSBASE), EXC_UD);
+        generate_exception_if(!(cr4 & X86_CR4_FSGSBASE), EXC_UD);
         seg = modrm_reg & 1 ? x86_seg_gs : x86_seg_fs;
         fail_if(!ops->read_segment);
         if ( (rc = ops->read_segment(seg, &sreg, ctxt)) != X86EMUL_OKAY )
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 09/11] fuzz/x86emul: update fuzzer
  2017-02-01 12:02 [PATCH v3 00/11] fuzz: update x86emul fuzzer Wei Liu
                   ` (7 preceding siblings ...)
  2017-02-01 12:02 ` [PATCH v3 08/11] x86emul: use CR definitions in x86-defns.h Wei Liu
@ 2017-02-01 12:02 ` Wei Liu
  2017-02-01 16:57   ` Wei Liu
  2017-02-02 12:20   ` Jan Beulich
  2017-02-01 12:02 ` [PATCH v3 10/11] fuzz/x86emul: print out minimal input size Wei Liu
  2017-02-01 12:02 ` [PATCH v3 11/11] fuzz: update README.afl example Wei Liu
  10 siblings, 2 replies; 28+ messages in thread
From: Wei Liu @ 2017-02-01 12:02 UTC (permalink / raw)
  To: Xen-devel
  Cc: Wei Liu, George Dunlap, Andrew Cooper, Ian Jackson,
	George Dunlap, Jan Beulich

Provide the fuzzer with more ops, and more sophisticated input
structure.

Based on a patch originally written by Andrew and George.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>

v3:
1. Address comments from Jan

v2:
1. Address comments from Jan
2. Provide architecturally correct behaviour for rep stub
---
 .../x86-insn-emulator-fuzzer.c                     | 661 +++++++++++++++++++--
 1 file changed, 599 insertions(+), 62 deletions(-)

diff --git a/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c
index 7d7f731677..4c404ceb81 100644
--- a/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c
+++ b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c
@@ -4,6 +4,7 @@
 #include <inttypes.h>
 #include <limits.h>
 #include <stdbool.h>
+#include <stddef.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -16,26 +17,78 @@
 
 #include "x86_emulate.h"
 
-static unsigned char data[4096];
+#define MSR_INDEX_MAX 16
+
+#define SEG_NUM x86_seg_none
+
+struct input_struct {
+    unsigned long cr[5];
+    uint64_t msr[MSR_INDEX_MAX];
+    struct cpu_user_regs regs;
+    struct segment_register segments[SEG_NUM];
+    unsigned long options;
+    unsigned char data[4096];
+} input;
+#define DATA_OFFSET offsetof(struct input_struct, data)
 static unsigned int data_index;
-static unsigned int data_max;
+static unsigned int data_num;
+
+/*
+ * Randomly return success or failure when processing data.  If
+ * `exception` is false, this function turns _EXCEPTION to _OKAY.
+ */
+int maybe_fail(const char *why, bool exception)
+{
+    int rc;
+
+    if ( data_index >= data_num )
+        rc = X86EMUL_EXCEPTION;
+    else
+    {
+        /* Randomly returns value:
+         * 50% okay
+         * 25% unhandlable
+         * 25% exception
+         */
+        if ( input.data[data_index] > 0xc0 )
+            rc = X86EMUL_EXCEPTION;
+        else if ( input.data[data_index] > 0x80 )
+            rc = X86EMUL_UNHANDLEABLE;
+        else
+            rc = X86EMUL_OKAY;
+        data_index++;
+    }
+
+    if ( rc == X86EMUL_EXCEPTION && !exception )
+        rc = X86EMUL_OKAY;
+
+    printf("maybe_fail %s: %d\n", why, rc);
+
+    return rc;
+}
 
 static int data_read(const char *why, void *dst, unsigned int bytes)
 {
     unsigned int i;
+    int rc;
 
-    if ( data_index + bytes > data_max )
-        return X86EMUL_EXCEPTION;
+    if ( data_index + bytes > data_num )
+        rc = X86EMUL_EXCEPTION;
+    else
+        rc = maybe_fail(why, true);
 
-    memcpy(dst,  data + data_index, bytes);
-    data_index += bytes;
+    if ( rc == X86EMUL_OKAY )
+    {
+        memcpy(dst,  input.data + data_index, bytes);
+        data_index += bytes;
 
-    printf("%s: ", why);
-    for ( i = 0; i < bytes; i++ )
-        printf(" %02x", *(unsigned char *)(dst + i));
-    printf("\n");
+        printf("%s: ", why);
+        for ( i = 0; i < bytes; i++ )
+            printf(" %02x", *(unsigned char *)(dst + i));
+        printf("\n");
+    }
 
-    return X86EMUL_OKAY;
+    return rc;
 }
 
 static int fuzz_read(
@@ -48,14 +101,112 @@ static int fuzz_read(
     return data_read("read", p_data, bytes);
 }
 
-static int fuzz_fetch(
+static int fuzz_read_io(
+    unsigned int port,
+    unsigned int bytes,
+    unsigned long *val,
+    struct x86_emulate_ctxt *ctxt)
+{
+    return data_read("read_io", val, bytes);
+}
+
+static int fuzz_insn_fetch(
     unsigned int seg,
     unsigned long offset,
     void *p_data,
     unsigned int bytes,
     struct x86_emulate_ctxt *ctxt)
 {
-    return data_read("fetch", p_data, bytes);
+    return data_read("insn_fetch", p_data, bytes);
+}
+
+static int _fuzz_rep_read(const char *why, unsigned long *reps)
+{
+    int rc;
+    unsigned long bytes_read = 0;
+
+    rc = data_read(why, &bytes_read, sizeof(bytes_read));
+
+    if ( bytes_read <= *reps )
+        *reps = bytes_read;
+
+    switch ( rc )
+    {
+    case X86EMUL_UNHANDLEABLE:
+        /* No work is done in this case */
+        *reps = 0;
+        break;
+    case X86EMUL_EXCEPTION:
+    case X86EMUL_RETRY:
+        /* Halve the amount in this case */
+        *reps /= 2;
+    }
+
+    return rc;
+}
+
+static int _fuzz_rep_write(const char *why, unsigned long *reps)
+{
+    int rc = maybe_fail(why, true);
+
+    switch ( rc )
+    {
+    case X86EMUL_UNHANDLEABLE:
+        /* No work is done in this case */
+        *reps = 0;
+        break;
+    case X86EMUL_EXCEPTION:
+    case X86EMUL_RETRY:
+        /* Halve the amount in this case */
+        *reps /= 2;
+    }
+
+    return rc;
+}
+
+static int fuzz_rep_ins(
+    uint16_t src_port,
+    enum x86_segment dst_seg,
+    unsigned long dst_offset,
+    unsigned int bytes_per_rep,
+    unsigned long *reps,
+    struct x86_emulate_ctxt *ctxt)
+{
+    return _fuzz_rep_read("rep_ins", reps);
+}
+
+static int fuzz_rep_movs(
+    enum x86_segment src_seg,
+    unsigned long src_offset,
+    enum x86_segment dst_seg,
+    unsigned long dst_offset,
+    unsigned int bytes_per_rep,
+    unsigned long *reps,
+    struct x86_emulate_ctxt *ctxt)
+{
+    return _fuzz_rep_read("rep_movs", reps);
+}
+
+static int fuzz_rep_outs(
+    enum x86_segment src_seg,
+    unsigned long src_offset,
+    uint16_t dst_port,
+    unsigned int bytes_per_rep,
+    unsigned long *reps,
+    struct x86_emulate_ctxt *ctxt)
+{
+    return _fuzz_rep_write("rep_outs", reps);
+}
+
+static int fuzz_rep_stos(
+    void *p_data,
+    enum x86_segment seg,
+    unsigned long offset,
+    unsigned int bytes_per_rep,
+    unsigned long *reps,
+    struct x86_emulate_ctxt *ctxt)
+{
+    return _fuzz_rep_write("rep_stos", reps);
 }
 
 static int fuzz_write(
@@ -65,7 +216,7 @@ static int fuzz_write(
     unsigned int bytes,
     struct x86_emulate_ctxt *ctxt)
 {
-    return X86EMUL_OKAY;
+    return maybe_fail("write", true);
 }
 
 static int fuzz_cmpxchg(
@@ -76,18 +227,285 @@ static int fuzz_cmpxchg(
     unsigned int bytes,
     struct x86_emulate_ctxt *ctxt)
 {
-    return X86EMUL_OKAY;
+    return maybe_fail("cmpxchg", true);
+}
+
+static int fuzz_invlpg(
+    enum x86_segment seg,
+    unsigned long offset,
+    struct x86_emulate_ctxt *ctxt)
+{
+    return maybe_fail("invlpg", false);
+}
+
+static int fuzz_wbinvd(
+    struct x86_emulate_ctxt *ctxt)
+{
+    return maybe_fail("wbinvd", true);
+}
+
+static int fuzz_write_io(
+    unsigned int port,
+    unsigned int bytes,
+    unsigned long val,
+    struct x86_emulate_ctxt *ctxt)
+{
+    return maybe_fail("write_io", true);
+}
+
+static int fuzz_read_segment(
+    enum x86_segment seg,
+    struct segment_register *reg,
+    struct x86_emulate_ctxt *ctxt)
+{
+    int rc;
+
+    if ( seg >= SEG_NUM )
+        return X86EMUL_UNHANDLEABLE;
+
+    rc = maybe_fail("read_segment", true);
+
+    if ( rc == X86EMUL_OKAY )
+        *reg = input.segments[seg];
+
+    return rc;
+}
+
+static int fuzz_write_segment(
+    enum x86_segment seg,
+    const struct segment_register *reg,
+    struct x86_emulate_ctxt *ctxt)
+{
+    int rc;
+
+    if ( seg >= SEG_NUM )
+        return X86EMUL_UNHANDLEABLE;
+
+    rc = maybe_fail("write_segment", true);
+
+    if ( rc == X86EMUL_OKAY )
+        input.segments[seg] = *reg;
+
+    return rc;
+}
+
+static int fuzz_read_cr(
+    unsigned int reg,
+    unsigned long *val,
+    struct x86_emulate_ctxt *ctxt)
+{
+    int rc;
+
+    if ( reg >= ARRAY_SIZE(input.cr) )
+        return X86EMUL_UNHANDLEABLE;
+
+    rc = maybe_fail("read_cr", true);
+
+    if ( rc == X86EMUL_OKAY )
+        *val = input.cr[reg];
+
+    return rc;
+}
+
+static int fuzz_write_cr(
+    unsigned int reg,
+    unsigned long val,
+    struct x86_emulate_ctxt *ctxt)
+{
+    int rc;
+
+    if ( reg >= ARRAY_SIZE(input.cr) )
+        return X86EMUL_UNHANDLEABLE;
+
+    rc = maybe_fail("write_cr", true);
+
+    if ( rc == X86EMUL_OKAY )
+        input.cr[reg] = val;
+
+    return rc;
+}
+
+enum {
+    MSRI_IA32_SYSENTER_CS,
+    MSRI_IA32_SYSENTER_ESP,
+    MSRI_IA32_SYSENTER_EIP,
+    MSRI_EFER,
+    MSRI_STAR,
+    MSRI_LSTAR,
+    MSRI_CSTAR,
+    MSRI_SYSCALL_MASK
+};
+
+static const unsigned int msr_index[MSR_INDEX_MAX] = {
+    [MSRI_IA32_SYSENTER_CS]  = MSR_IA32_SYSENTER_CS,
+    [MSRI_IA32_SYSENTER_ESP] = MSR_IA32_SYSENTER_ESP,
+    [MSRI_IA32_SYSENTER_EIP] = MSR_IA32_SYSENTER_EIP,
+    [MSRI_EFER]              = MSR_EFER,
+    [MSRI_STAR]              = MSR_STAR,
+    [MSRI_LSTAR]             = MSR_LSTAR,
+    [MSRI_CSTAR]             = MSR_CSTAR,
+    [MSRI_SYSCALL_MASK]      = MSR_SYSCALL_MASK
+};
+
+static int _fuzz_read_msr(
+    unsigned int reg,
+    uint64_t *val,
+    struct x86_emulate_ctxt *ctxt)
+{
+    unsigned int idx;
+
+    switch ( reg )
+    {
+    case MSR_TSC_AUX:
+    case MSR_IA32_TSC:
+        return data_read("read_msr", val, sizeof(*val));
+    case MSR_EFER:
+        *val = input.msr[MSRI_EFER];
+        *val &= ~EFER_LMA;
+        if ( (*val & EFER_LME) && (input.cr[4] & X86_CR4_PAE) &&
+             (input.cr[0] & X86_CR0_PG) )
+        {
+            printf("Setting EFER_LMA\n");
+            *val |= EFER_LMA;
+        }
+        return X86EMUL_OKAY;
+    }
+
+    for ( idx = 0; idx < MSR_INDEX_MAX; idx++ )
+    {
+        if ( msr_index[idx] == reg )
+        {
+            *val = input.msr[idx];
+            return X86EMUL_OKAY;
+        }
+    }
+
+    return X86EMUL_EXCEPTION;
 }
 
+static int fuzz_read_msr(
+    unsigned int reg,
+    uint64_t *val,
+    struct x86_emulate_ctxt *ctxt) {
+    int rc;
+
+    rc = maybe_fail("read_msr", true);
+    if ( rc != X86EMUL_OKAY )
+        return rc;
+
+    return _fuzz_read_msr(reg, val, ctxt);
+}
+
+static int fuzz_write_msr(
+    unsigned int reg,
+    uint64_t val,
+    struct x86_emulate_ctxt *ctxt)
+{
+    unsigned int idx;
+    int rc;
+
+    rc = maybe_fail("write_ms", true);
+    if ( rc != X86EMUL_OKAY )
+        return rc;
+
+    switch ( reg )
+    {
+    case MSR_TSC_AUX:
+    case MSR_IA32_TSC:
+        return X86EMUL_OKAY;
+    }
+
+    for ( idx = 0; idx < MSR_INDEX_MAX; idx++ )
+    {
+        if ( msr_index[idx] == reg )
+        {
+            input.msr[idx] = val;
+            return X86EMUL_OKAY;
+        }
+    }
+
+    return X86EMUL_EXCEPTION;
+}
+
+#define SET(h) .h = fuzz_##h
 static struct x86_emulate_ops fuzz_emulops = {
-    .read       = fuzz_read,
-    .insn_fetch = fuzz_fetch,
-    .write      = fuzz_write,
-    .cmpxchg    = fuzz_cmpxchg,
-    .cpuid      = emul_test_cpuid,
-    .read_cr    = emul_test_read_cr,
+    SET(read),
+    SET(insn_fetch),
+    SET(write),
+    SET(cmpxchg),
+    SET(rep_ins),
+    SET(rep_outs),
+    SET(rep_movs),
+    SET(rep_stos),
+    SET(read_segment),
+    SET(write_segment),
+    SET(read_io),
+    SET(write_io),
+    SET(read_cr),
+    SET(write_cr),
+    SET(read_msr),
+    SET(write_msr),
+    SET(wbinvd),
+    SET(invlpg),
     .get_fpu    = emul_test_get_fpu,
+    .cpuid      = emul_test_cpuid,
 };
+#undef SET
+
+static void setup_fpu_exception_handler(void)
+{
+    /* FIXME - just disable exceptions for now */
+    unsigned long a;
+
+    asm volatile ( "fnclex");
+    a = 0x37f; /* FCW_DEFAULT in Xen */
+    asm volatile ( "fldcw %0" :: "m" (a));
+    a = 0x1f80; /* MXCSR_DEFAULT in Xen */
+    asm volatile ( "ldmxcsr %0" :: "m" (a) );
+}
+
+static void dump_state(struct x86_emulate_ctxt *ctxt)
+{
+    struct cpu_user_regs *regs = ctxt->regs;
+    uint64_t val;
+
+    printf(" -- State -- \n");
+    printf("addr / sp size: %d / %d\n", ctxt->addr_size, ctxt->sp_size);
+    printf(" cr0: %lx\n", input.cr[0]);
+    printf(" cr3: %lx\n", input.cr[3]);
+    printf(" cr4: %lx\n", input.cr[4]);
+
+    printf(" rip: %"PRIx64"\n", regs->rip);
+
+    _fuzz_read_msr(MSR_EFER, &val, ctxt);
+    printf("EFER: %"PRIx64"\n", val);
+}
+
+static bool long_mode_active(struct x86_emulate_ctxt *ctxt)
+{
+    uint64_t val;
+
+    if ( _fuzz_read_msr(MSR_EFER, &val, ctxt) != X86EMUL_OKAY )
+        return false;
+
+    return val & EFER_LMA;
+}
+
+static bool in_longmode(struct x86_emulate_ctxt *ctxt)
+{
+    return long_mode_active(ctxt) && input.segments[x86_seg_cs].attr.fields.l;
+}
+
+static void set_sizes(struct x86_emulate_ctxt *ctxt)
+{
+    if ( in_longmode(ctxt) )
+        ctxt->addr_size = ctxt->sp_size = 64;
+    else
+    {
+        ctxt->addr_size = input.segments[x86_seg_cs].attr.fields.db ? 32 : 16;
+        ctxt->sp_size   = input.segments[x86_seg_ss].attr.fields.db ? 32 : 16;
+    }
+}
 
 #define CANONICALIZE(x)                                   \
     do {                                                  \
@@ -100,10 +518,146 @@ static struct x86_emulate_ops fuzz_emulops = {
         (x) = _y;                                       \
     } while( 0 )
 
-#define ADDR_SIZE_SHIFT 60
-#define ADDR_SIZE_64 (2ULL << ADDR_SIZE_SHIFT)
-#define ADDR_SIZE_32 (1ULL << ADDR_SIZE_SHIFT)
-#define ADDR_SIZE_16 (0)
+/* Expects bitmap and regs to be defined */
+#define CANONICALIZE_MAYBE(reg)                       \
+    if ( !(bitmap & (1 << CANONICALIZE_##reg)) )      \
+        CANONICALIZE(regs->reg);                      \
+
+enum {
+    HOOK_read,
+    HOOK_insn_fetch,
+    HOOK_write,
+    HOOK_cmpxchg,
+    HOOK_rep_ins,
+    HOOK_rep_outs,
+    HOOK_rep_movs,
+    HOOK_rep_stos,
+    HOOK_read_segment,
+    HOOK_write_segment,
+    HOOK_read_io,
+    HOOK_write_io,
+    HOOK_read_cr,
+    HOOK_write_cr,
+    HOOK_read_dr,
+    HOOK_write_dr,
+    HOOK_read_msr,
+    HOOK_write_msr,
+    HOOK_wbinvd,
+    HOOK_cpuid,
+    HOOK_inject_hw_exception,
+    HOOK_inject_sw_interrupt,
+    HOOK_get_fpu,
+    HOOK_put_fpu,
+    HOOK_invlpg,
+    HOOK_vmfunc,
+    OPTION_swint_emulation, /* Two bits */
+    CANONICALIZE_rip = OPTION_swint_emulation + 2,
+    CANONICALIZE_rsp,
+    CANONICALIZE_rbp
+};
+
+/* Expects bitmap to be defined */
+#define MAYBE_DISABLE_HOOK(h)                          \
+    if ( bitmap & (1 << HOOK_##h) )                    \
+    {                                                  \
+        fuzz_emulops.h = NULL;                         \
+        printf("Disabling hook "#h"\n");               \
+    }
+
+static void disable_hooks(void)
+{
+    unsigned long bitmap = input.options;
+
+    /* See also sanitize_input, some hooks can't be disabled. */
+    MAYBE_DISABLE_HOOK(read);
+    MAYBE_DISABLE_HOOK(insn_fetch);
+    MAYBE_DISABLE_HOOK(write);
+    MAYBE_DISABLE_HOOK(cmpxchg);
+    MAYBE_DISABLE_HOOK(rep_ins);
+    MAYBE_DISABLE_HOOK(rep_outs);
+    MAYBE_DISABLE_HOOK(rep_movs);
+    MAYBE_DISABLE_HOOK(rep_stos);
+    MAYBE_DISABLE_HOOK(read_segment);
+    MAYBE_DISABLE_HOOK(write_segment);
+    MAYBE_DISABLE_HOOK(read_io);
+    MAYBE_DISABLE_HOOK(write_io);
+    MAYBE_DISABLE_HOOK(read_cr);
+    MAYBE_DISABLE_HOOK(write_cr);
+    MAYBE_DISABLE_HOOK(read_msr);
+    MAYBE_DISABLE_HOOK(write_msr);
+    MAYBE_DISABLE_HOOK(wbinvd);
+    MAYBE_DISABLE_HOOK(cpuid);
+    MAYBE_DISABLE_HOOK(get_fpu);
+    MAYBE_DISABLE_HOOK(invlpg);
+}
+
+static void set_swint_support(struct x86_emulate_ctxt *ctxt)
+{
+    unsigned int swint_opt = (input.options >> OPTION_swint_emulation) & 3;
+
+    static const enum x86_swint_emulation map[4] = {
+        x86_swint_emulate_none,
+        x86_swint_emulate_none,
+        x86_swint_emulate_icebp,
+        x86_swint_emulate_all };
+
+    ctxt->swint_emulate = map[swint_opt];
+}
+
+/*
+ * Constrain input to architecturally-possible states where
+ * the emulator relies on these
+ *
+ * In general we want the emulator to be as absolutely robust as
+ * possible; which means that we want to minimize the number of things
+ * it assumes about the input state.  Tesing this means minimizing and
+ * removing as much of the input constraints as possible.
+ *
+ * So we only add constraints that (in general) have been proven to
+ * cause crashes in the emulator.
+ *
+ * For future reference: other constraints which might be necessary at
+ * some point:
+ *
+ * - EFER.LMA => !EFLAGS.NT
+ * - In VM86 mode, force segment...
+ *  - ...access rights to 0xf3
+ *  - ...limits to 0xffff
+ *  - ...bases to below 1Mb, 16-byte aligned
+ *  - ...selectors to (base >> 4)
+ */
+void sanitize_input(struct x86_emulate_ctxt *ctxt) {
+    struct cpu_user_regs *regs = &input.regs;
+    unsigned long bitmap = input.options;
+
+    /* Some hooks can't be disabled. */
+    input.options &= ~((1<<HOOK_read)|(1<<HOOK_insn_fetch));
+
+    /* Zero 'private' entries */
+    regs->error_code = 0;
+    regs->entry_vector = 0;
+
+    CANONICALIZE_MAYBE(rip);
+    CANONICALIZE_MAYBE(rsp);
+    CANONICALIZE_MAYBE(rbp);
+
+    /*
+     * CR0.PG can't be set if CR0.PE isn't set.  Set is more interesting, so
+     * set PE if PG is set.
+     */
+    if ( input.cr[0] & X86_CR0_PG )
+        input.cr[0] |= X86_CR0_PE;
+
+    /* EFLAGS.VM not available in long mode */
+    if ( long_mode_active(ctxt) )
+        regs->rflags &= ~X86_EFLAGS_VM;
+
+    /* EFLAGS.VM implies 16-bit mode */
+    if ( regs->rflags & X86_EFLAGS_VM ) {
+        input.segments[x86_seg_cs].attr.fields.db = 0;
+        input.segments[x86_seg_ss].attr.fields.db = 0;
+    }
+}
 
 int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size)
 {
@@ -114,10 +668,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size)
         .addr_size = 8 * sizeof(void *),
         .sp_size = 8 * sizeof(void *),
     };
-    unsigned int nr = 0;
     int rc;
-    unsigned int x;
-    const uint8_t *p = data_p;
 
     stack_exec = emul_test_make_stack_executable();
     if ( !stack_exec )
@@ -127,52 +678,38 @@ int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size)
     }
 
     /* Reset all global state variables */
-    memset(data, 0, sizeof(data));
+    memset(&input, 0, sizeof(input));
     data_index = 0;
-    data_max = 0;
-
-    nr = size < sizeof(regs) ? size : sizeof(regs);
+    data_num = 0;
 
-    memcpy(&regs, p, nr);
-    p += sizeof(regs);
+    if ( size <= DATA_OFFSET )
+    {
+        printf("Input too small\n");
+        return 1;
+    }
 
-    if ( nr < size )
+    if ( size > sizeof(input) )
     {
-        memcpy(data, p, size - nr);
-        data_max = size - nr;
+        printf("Input too large\n");
+        return 1;
     }
 
-    ctxt.force_writeback = false;
+    memcpy(&input, data_p, size);
 
-    /* Zero 'private' fields */
-    regs.error_code = 0;
-    regs.entry_vector = 0;
+    data_num = size - DATA_OFFSET;
 
-    /* Use the upper bits of regs.eip to determine addr_size */
-    x = (regs.rip >> ADDR_SIZE_SHIFT) & 0x3;
-    if ( x == 3 )
-        x = 2;
-    ctxt.addr_size = 16 << x;
-    printf("addr_size: %d\n", ctxt.addr_size);
+    sanitize_input(&ctxt);
 
-    /* Use the upper bit of regs.rsp to determine sp_size (if appropriate) */
-    if ( ctxt.addr_size == 64 )
-        ctxt.sp_size = 64;
-    else
-    {
-        /* If addr_size isn't 64-bits, sp_size can only be 16 or 32 bits */
-        x = (regs.rsp >> ADDR_SIZE_SHIFT) & 0x1;
-        ctxt.sp_size = 16 << x;
-    }
-    printf("sp_size: %d\n", ctxt.sp_size);
-    CANONICALIZE(regs.rip);
-    CANONICALIZE(regs.rsp);
-    CANONICALIZE(regs.rbp);
+    disable_hooks();
 
-    /* Zero all segments for now */
-    regs.cs = regs.ss = regs.es = regs.ds = regs.fs = regs.gs = 0;
+    set_swint_support(&ctxt);
 
     do {
+        setup_fpu_exception_handler();
+
+        set_sizes(&ctxt);
+        dump_state(&ctxt);
+
         rc = x86_emulate(&ctxt, &fuzz_emulops);
         printf("Emulation result: %d\n", rc);
     } while ( rc == X86EMUL_OKAY );
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 10/11] fuzz/x86emul: print out minimal input size
  2017-02-01 12:02 [PATCH v3 00/11] fuzz: update x86emul fuzzer Wei Liu
                   ` (8 preceding siblings ...)
  2017-02-01 12:02 ` [PATCH v3 09/11] fuzz/x86emul: update fuzzer Wei Liu
@ 2017-02-01 12:02 ` Wei Liu
  2017-02-01 12:02 ` [PATCH v3 11/11] fuzz: update README.afl example Wei Liu
  10 siblings, 0 replies; 28+ messages in thread
From: Wei Liu @ 2017-02-01 12:02 UTC (permalink / raw)
  To: Xen-devel; +Cc: Wei Liu

... so that users can know how big the initial input should be.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
 .../fuzz/x86_instruction_emulator/afl-x86-insn-emulator-fuzzer.c  | 8 ++++++++
 tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c    | 5 +++++
 2 files changed, 13 insertions(+)

diff --git a/tools/fuzz/x86_instruction_emulator/afl-x86-insn-emulator-fuzzer.c b/tools/fuzz/x86_instruction_emulator/afl-x86-insn-emulator-fuzzer.c
index 494c23ba2e..16edbd6bab 100644
--- a/tools/fuzz/x86_instruction_emulator/afl-x86-insn-emulator-fuzzer.c
+++ b/tools/fuzz/x86_instruction_emulator/afl-x86-insn-emulator-fuzzer.c
@@ -2,8 +2,10 @@
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 
 extern int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size);
+extern unsigned int fuzz_minimal_input_size(void);
 
 #define INPUT_SIZE  4096
 static uint8_t input[INPUT_SIZE];
@@ -21,6 +23,12 @@ int main(int argc, char **argv)
         exit(-1);
     }
 
+    if ( !strcmp(argv[1], "--min-input-size") )
+    {
+        printf("%u\n", fuzz_minimal_input_size());
+        exit(0);
+    }
+
     fp = fopen(argv[1], "rb");
     if ( fp == NULL )
     {
diff --git a/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c
index 4c404ceb81..edc5130b3b 100644
--- a/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c
+++ b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c
@@ -717,6 +717,11 @@ int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size)
     return 0;
 }
 
+unsigned int fuzz_minimal_input_size(void)
+{
+    return DATA_OFFSET + 1;
+}
+
 /*
  * Local variables:
  * mode: C
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 11/11] fuzz: update README.afl example
  2017-02-01 12:02 [PATCH v3 00/11] fuzz: update x86emul fuzzer Wei Liu
                   ` (9 preceding siblings ...)
  2017-02-01 12:02 ` [PATCH v3 10/11] fuzz/x86emul: print out minimal input size Wei Liu
@ 2017-02-01 12:02 ` Wei Liu
  10 siblings, 0 replies; 28+ messages in thread
From: Wei Liu @ 2017-02-01 12:02 UTC (permalink / raw)
  To: Xen-devel; +Cc: Wei Liu

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 tools/fuzz/README.afl | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/fuzz/README.afl b/tools/fuzz/README.afl
index 431b4a8cbf..68e0fa396f 100644
--- a/tools/fuzz/README.afl
+++ b/tools/fuzz/README.afl
@@ -20,9 +20,10 @@ Use the x86 instruction emulator fuzzer as an example.
    $ make distclean
    $ make CC=$AFLPATH/afl-gcc afl # produces afl-x86-insn-emulator-fuzzer
 
-3. provide initial test case:
+3. provide initial test case (fuzzer dependent, see afl-*.c):
    $ mkdir testcase_dir
-   $ echo -n -e '\xc3' > testcase_dir/ret.bin
+   $ dd if=/dev/urandom of=testcase_dir/rand.bin \
+       bs=`./afl-x86-insn-emulator-fuzzer --min-input-size` count=1
 
 4. run the fuzzer with AFL:
    $ $AFLPATH/afl-fuzz -m none -t 1000 -i testcase_dir -o findings_dir -- \
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 01/11] x86emul/test: add missing dependency for x86_emulate.o
  2017-02-01 12:02 ` [PATCH v3 01/11] x86emul/test: add missing dependency for x86_emulate.o Wei Liu
@ 2017-02-01 13:03   ` Jan Beulich
  2017-02-02 17:43     ` Wei Liu
  0 siblings, 1 reply; 28+ messages in thread
From: Jan Beulich @ 2017-02-01 13:03 UTC (permalink / raw)
  To: Wei Liu; +Cc: AndrewCooper, Xen-devel

>>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote:
> --- a/tools/tests/x86_emulator/Makefile
> +++ b/tools/tests/x86_emulator/Makefile
> @@ -45,7 +45,7 @@ x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
>  
>  HOSTCFLAGS += $(CFLAGS_xeninclude)
>  
> -x86_emulate.o: x86_emulate.c x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
> +x86_emulate.o: x86_emulate.c x86_emulate.h x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
>  	$(HOSTCC) $(HOSTCFLAGS) -D__XEN_TOOLS__ -c -g -o $@ $<
>  
>  test_x86_emulator.o: test_x86_emulator.c blowfish.h x86_emulate/x86_emulate.h

I think this latter one should be updated too. Which then calls for
macroizing:

x86_emulate.h := x86_emulate.h x86_emulate/x86_emulate.h

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 02/11] x86: extract macros to x86-defns.h
  2017-02-01 12:02 ` [PATCH v3 02/11] x86: extract macros to x86-defns.h Wei Liu
@ 2017-02-01 13:05   ` Jan Beulich
  2017-02-02 16:58     ` Wei Liu
  0 siblings, 1 reply; 28+ messages in thread
From: Jan Beulich @ 2017-02-01 13:05 UTC (permalink / raw)
  To: Wei Liu; +Cc: AndrewCooper, Xen-devel

>>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote:
> --- a/xen/include/asm-x86/processor.h
> +++ b/xen/include/asm-x86/processor.h
> @@ -16,6 +16,8 @@
>  #include <asm/desc.h>
>  #endif
>  
> +#include "x86-defns.h"

This should be <asm/x86-defns.h>.

With that adjusted,
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 04/11] x86emul/test: use x86-vendors.h
  2017-02-01 12:02 ` [PATCH v3 04/11] x86emul/test: use x86-vendors.h Wei Liu
@ 2017-02-01 13:09   ` Jan Beulich
  2017-02-01 13:13     ` Andrew Cooper
  0 siblings, 1 reply; 28+ messages in thread
From: Jan Beulich @ 2017-02-01 13:09 UTC (permalink / raw)
  To: AndrewCooper, Wei Liu; +Cc: Xen-devel

>>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote:
> --- a/tools/fuzz/x86_instruction_emulator/Makefile
> +++ b/tools/fuzz/x86_instruction_emulator/Makefile
> @@ -11,10 +11,15 @@ endif
>  x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
>  	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
>  
> +asm/x86-vendors.h:
> +	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm

Hmm, that's making all headers available. I don't really like it to
be that broad. Andrew, what do you think

> -CFLAGS += $(CFLAGS_xeninclude) -D__XEN_TOOLS__
> +CFLAGS += $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -I.
> +
> +x86_emulate.h: asm/x86-vendors.h

That's not a true dependency; I'd rather see this build on the
suggested macroization (i.e. introduce it here too, and do the
same for the non-fuzzing harness).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 04/11] x86emul/test: use x86-vendors.h
  2017-02-01 13:09   ` Jan Beulich
@ 2017-02-01 13:13     ` Andrew Cooper
  2017-02-01 13:22       ` Wei Liu
  2017-02-01 13:26       ` Jan Beulich
  0 siblings, 2 replies; 28+ messages in thread
From: Andrew Cooper @ 2017-02-01 13:13 UTC (permalink / raw)
  To: Jan Beulich, Wei Liu; +Cc: Xen-devel

On 01/02/17 13:09, Jan Beulich wrote:
>>>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote:
>> --- a/tools/fuzz/x86_instruction_emulator/Makefile
>> +++ b/tools/fuzz/x86_instruction_emulator/Makefile
>> @@ -11,10 +11,15 @@ endif
>>  x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
>>  	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
>>  
>> +asm/x86-vendors.h:
>> +	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm
> Hmm, that's making all headers available. I don't really like it to
> be that broad. Andrew, what do you think

This is just a test utility.  I think we can trust ourselves to only
include appropriate headers.

This is simpler and easier than keeping a whitelist of permitted
headers, which doesn't require any modification when we add a new header.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 04/11] x86emul/test: use x86-vendors.h
  2017-02-01 13:13     ` Andrew Cooper
@ 2017-02-01 13:22       ` Wei Liu
  2017-02-01 13:26       ` Jan Beulich
  1 sibling, 0 replies; 28+ messages in thread
From: Wei Liu @ 2017-02-01 13:22 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Jan Beulich

On Wed, Feb 01, 2017 at 01:13:44PM +0000, Andrew Cooper wrote:
> On 01/02/17 13:09, Jan Beulich wrote:
> >>>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote:
> >> --- a/tools/fuzz/x86_instruction_emulator/Makefile
> >> +++ b/tools/fuzz/x86_instruction_emulator/Makefile
> >> @@ -11,10 +11,15 @@ endif
> >>  x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
> >>  	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
> >>  
> >> +asm/x86-vendors.h:
> >> +	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm
> > Hmm, that's making all headers available. I don't really like it to
> > be that broad. Andrew, what do you think
> 
> This is just a test utility.  I think we can trust ourselves to only
> include appropriate headers.
> 
> This is simpler and easier than keeping a whitelist of permitted
> headers, which doesn't require any modification when we add a new header.
> 

Yes, this is exactly the reason why I did it like this.

Wei.

> ~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 04/11] x86emul/test: use x86-vendors.h
  2017-02-01 13:13     ` Andrew Cooper
  2017-02-01 13:22       ` Wei Liu
@ 2017-02-01 13:26       ` Jan Beulich
  1 sibling, 0 replies; 28+ messages in thread
From: Jan Beulich @ 2017-02-01 13:26 UTC (permalink / raw)
  To: Andrew Cooper, Wei Liu; +Cc: Xen-devel

>>> On 01.02.17 at 14:13, <andrew.cooper3@citrix.com> wrote:
> On 01/02/17 13:09, Jan Beulich wrote:
>>>>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote:
>>> --- a/tools/fuzz/x86_instruction_emulator/Makefile
>>> +++ b/tools/fuzz/x86_instruction_emulator/Makefile
>>> @@ -11,10 +11,15 @@ endif
>>>  x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
>>>  	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
>>>  
>>> +asm/x86-vendors.h:
>>> +	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm
>> Hmm, that's making all headers available. I don't really like it to
>> be that broad. Andrew, what do you think
> 
> This is just a test utility.  I think we can trust ourselves to only
> include appropriate headers.
> 
> This is simpler and easier than keeping a whitelist of permitted
> headers, which doesn't require any modification when we add a new header.

Well, okay then (albeit this "doesn't require any modification"
doesn't hold with the dependency the patch also adds; perhaps
midterm we should arrange for the compiler to generate
dependencies for us).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 09/11] fuzz/x86emul: update fuzzer
  2017-02-01 12:02 ` [PATCH v3 09/11] fuzz/x86emul: update fuzzer Wei Liu
@ 2017-02-01 16:57   ` Wei Liu
  2017-02-02 12:20   ` Jan Beulich
  1 sibling, 0 replies; 28+ messages in thread
From: Wei Liu @ 2017-02-01 16:57 UTC (permalink / raw)
  To: Xen-devel
  Cc: Wei Liu, George Dunlap, Andrew Cooper, Ian Jackson,
	George Dunlap, Jan Beulich

This patch will be squashed in.

From 4990d760f900223e32cb800e7374ee45d357081b Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@citrix.com>
Date: Wed, 1 Feb 2017 16:54:58 +0000
Subject: [PATCH] fixup! fuzz/x86emul: update fuzzer

---
 .../x86-insn-emulator-fuzzer.c                         | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c
index edc5130b3b..ed43935600 100644
--- a/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c
+++ b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c
@@ -294,17 +294,12 @@ static int fuzz_read_cr(
     unsigned long *val,
     struct x86_emulate_ctxt *ctxt)
 {
-    int rc;
-
     if ( reg >= ARRAY_SIZE(input.cr) )
         return X86EMUL_UNHANDLEABLE;
 
-    rc = maybe_fail("read_cr", true);
+    *val = input.cr[reg];
 
-    if ( rc == X86EMUL_OKAY )
-        *val = input.cr[reg];
-
-    return rc;
+    return X86EMUL_OKAY;
 }
 
 static int fuzz_write_cr(
@@ -312,17 +307,12 @@ static int fuzz_write_cr(
     unsigned long val,
     struct x86_emulate_ctxt *ctxt)
 {
-    int rc;
-
     if ( reg >= ARRAY_SIZE(input.cr) )
         return X86EMUL_UNHANDLEABLE;
 
-    rc = maybe_fail("write_cr", true);
+    input.cr[reg] = val;
 
-    if ( rc == X86EMUL_OKAY )
-        input.cr[reg] = val;
-
-    return rc;
+    return X86EMUL_OKAY;
 }
 
 enum {
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 09/11] fuzz/x86emul: update fuzzer
  2017-02-01 12:02 ` [PATCH v3 09/11] fuzz/x86emul: update fuzzer Wei Liu
  2017-02-01 16:57   ` Wei Liu
@ 2017-02-02 12:20   ` Jan Beulich
  2017-02-02 16:50     ` Wei Liu
  1 sibling, 1 reply; 28+ messages in thread
From: Jan Beulich @ 2017-02-02 12:20 UTC (permalink / raw)
  To: Wei Liu
  Cc: George Dunlap, Andrew Cooper, Ian Jackson, George Dunlap, Xen-devel

>>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote:
> @@ -16,26 +17,78 @@
>  
>  #include "x86_emulate.h"
>  
> -static unsigned char data[4096];
> +#define MSR_INDEX_MAX 16
> +
> +#define SEG_NUM x86_seg_none
> +
> +struct input_struct {
> +    unsigned long cr[5];
> +    uint64_t msr[MSR_INDEX_MAX];
> +    struct cpu_user_regs regs;
> +    struct segment_register segments[SEG_NUM];
> +    unsigned long options;
> +    unsigned char data[4096];
> +} input;
> +#define DATA_OFFSET offsetof(struct input_struct, data)
>  static unsigned int data_index;
> -static unsigned int data_max;
> +static unsigned int data_num;
> +
> +/*
> + * Randomly return success or failure when processing data.  If
> + * `exception` is false, this function turns _EXCEPTION to _OKAY.
> + */
> +int maybe_fail(const char *why, bool exception)

static?

> +static int _fuzz_rep_read(const char *why, unsigned long *reps)
> +{
> +    int rc;
> +    unsigned long bytes_read = 0;
> +
> +    rc = data_read(why, &bytes_read, sizeof(bytes_read));
> +
> +    if ( bytes_read <= *reps )
> +        *reps = bytes_read;
> +
> +    switch ( rc )
> +    {
> +    case X86EMUL_UNHANDLEABLE:
> +        /* No work is done in this case */
> +        *reps = 0;
> +        break;
> +    case X86EMUL_EXCEPTION:
> +    case X86EMUL_RETRY:
> +        /* Halve the amount in this case */
> +        *reps /= 2;
> +    }

Even if not strictly needed at this point in time, adding a break
statement here would be nice.

> +static int fuzz_invlpg(
> +    enum x86_segment seg,
> +    unsigned long offset,
> +    struct x86_emulate_ctxt *ctxt)
> +{
> +    return maybe_fail("invlpg", false);
> +}
> +
> +static int fuzz_wbinvd(
> +    struct x86_emulate_ctxt *ctxt)
> +{
> +    return maybe_fail("wbinvd", true);
> +}

Can these two reasonably fail? I wonder if we shouldn't perhaps make
the hooks return void.

> +static int fuzz_read_segment(
> +    enum x86_segment seg,
> +    struct segment_register *reg,
> +    struct x86_emulate_ctxt *ctxt)
> +{
> +    int rc;
> +
> +    if ( seg >= SEG_NUM )
> +        return X86EMUL_UNHANDLEABLE;
> +
> +    rc = maybe_fail("read_segment", true);
> +
> +    if ( rc == X86EMUL_OKAY )
> +        *reg = input.segments[seg];
> +
> +    return rc;
> +}

Just like with ->read_cr(), this must not vary in returned state
between multiple invocations.

> +static int _fuzz_read_msr(
> +    unsigned int reg,
> +    uint64_t *val,
> +    struct x86_emulate_ctxt *ctxt)
> +{
> +    unsigned int idx;
> +
> +    switch ( reg )
> +    {
> +    case MSR_TSC_AUX:
> +    case MSR_IA32_TSC:
> +        return data_read("read_msr", val, sizeof(*val));

Strictly speaking the above applies to TSC_AUX too. And TSC should
return monotonically increasing values. I don't think though that
producing random output here matters right now. A comment may
be worthwhile.

> +static int fuzz_read_msr(
> +    unsigned int reg,
> +    uint64_t *val,
> +    struct x86_emulate_ctxt *ctxt) {
> +    int rc;
> +
> +    rc = maybe_fail("read_msr", true);
> +    if ( rc != X86EMUL_OKAY )
> +        return rc;

This, otoh, again needs to strictly follow the revised read_cr() model.

> +static void set_swint_support(struct x86_emulate_ctxt *ctxt)
> +{
> +    unsigned int swint_opt = (input.options >> OPTION_swint_emulation) & 3;
> +
> +    static const enum x86_swint_emulation map[4] = {

As (I think) said before - please don't put blank lines between
declarations; they're supposed to separate declarations from
statements.

> +/*
> + * Constrain input to architecturally-possible states where
> + * the emulator relies on these
> + *
> + * In general we want the emulator to be as absolutely robust as
> + * possible; which means that we want to minimize the number of things
> + * it assumes about the input state.  Tesing this means minimizing and
> + * removing as much of the input constraints as possible.
> + *
> + * So we only add constraints that (in general) have been proven to
> + * cause crashes in the emulator.
> + *
> + * For future reference: other constraints which might be necessary at
> + * some point:
> + *
> + * - EFER.LMA => !EFLAGS.NT
> + * - In VM86 mode, force segment...
> + *  - ...access rights to 0xf3
> + *  - ...limits to 0xffff
> + *  - ...bases to below 1Mb, 16-byte aligned
> + *  - ...selectors to (base >> 4)
> + */
> +void sanitize_input(struct x86_emulate_ctxt *ctxt) {

static? And { on its own line.

>      do {
> +        setup_fpu_exception_handler();

I'm sorry for having mislead you regarding the comment which was
here: As said elsewhere, I was - based on the function's name -
assuming this to be a one time action, while in fact this is being done
before every emulated insn. Once this becomes a one time thing, it
indeed needs moving out of the loop, so leaving a comment here is
warranted.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 09/11] fuzz/x86emul: update fuzzer
  2017-02-02 12:20   ` Jan Beulich
@ 2017-02-02 16:50     ` Wei Liu
  2017-02-02 17:01       ` Jan Beulich
  0 siblings, 1 reply; 28+ messages in thread
From: Wei Liu @ 2017-02-02 16:50 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Wei Liu, George Dunlap, Andrew Cooper, Ian Jackson,
	George Dunlap, Xen-devel

On Thu, Feb 02, 2017 at 05:20:56AM -0700, Jan Beulich wrote:
> >>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote:
> > @@ -16,26 +17,78 @@
> >  
> >  #include "x86_emulate.h"
> >  
> > -static unsigned char data[4096];
> > +#define MSR_INDEX_MAX 16
> > +
> > +#define SEG_NUM x86_seg_none
> > +
> > +struct input_struct {
> > +    unsigned long cr[5];
> > +    uint64_t msr[MSR_INDEX_MAX];
> > +    struct cpu_user_regs regs;
> > +    struct segment_register segments[SEG_NUM];
> > +    unsigned long options;
> > +    unsigned char data[4096];
> > +} input;
> > +#define DATA_OFFSET offsetof(struct input_struct, data)
> >  static unsigned int data_index;
> > -static unsigned int data_max;
> > +static unsigned int data_num;
> > +
> > +/*
> > + * Randomly return success or failure when processing data.  If
> > + * `exception` is false, this function turns _EXCEPTION to _OKAY.
> > + */
> > +int maybe_fail(const char *why, bool exception)
> 
> static?

Done.

> 
> > +static int _fuzz_rep_read(const char *why, unsigned long *reps)
> > +{
> > +    int rc;
> > +    unsigned long bytes_read = 0;
> > +
> > +    rc = data_read(why, &bytes_read, sizeof(bytes_read));
> > +
> > +    if ( bytes_read <= *reps )
> > +        *reps = bytes_read;
> > +
> > +    switch ( rc )
> > +    {
> > +    case X86EMUL_UNHANDLEABLE:
> > +        /* No work is done in this case */
> > +        *reps = 0;
> > +        break;
> > +    case X86EMUL_EXCEPTION:
> > +    case X86EMUL_RETRY:
> > +        /* Halve the amount in this case */
> > +        *reps /= 2;
> > +    }
> 
> Even if not strictly needed at this point in time, adding a break
> statement here would be nice.
> 

Done.

> > +static int fuzz_invlpg(
> > +    enum x86_segment seg,
> > +    unsigned long offset,
> > +    struct x86_emulate_ctxt *ctxt)
> > +{
> > +    return maybe_fail("invlpg", false);
> > +}
> > +
> > +static int fuzz_wbinvd(
> > +    struct x86_emulate_ctxt *ctxt)
> > +{
> > +    return maybe_fail("wbinvd", true);
> > +}
> 
> Can these two reasonably fail? I wonder if we shouldn't perhaps make
> the hooks return void.
> 

Both can generate exceptions in some cases according to manual, so I
think it makes sense to leave them as-is, provided they fit in the model
you want to use (like in_realmode).

Of course if you end up changing the hooks to return void, I'm fine with
just removing these two hooks from fuzzer altogether.

> > +static int fuzz_read_segment(
> > +    enum x86_segment seg,
> > +    struct segment_register *reg,
> > +    struct x86_emulate_ctxt *ctxt)
> > +{
> > +    int rc;
> > +
> > +    if ( seg >= SEG_NUM )
> > +        return X86EMUL_UNHANDLEABLE;
> > +
> > +    rc = maybe_fail("read_segment", true);
> > +
> > +    if ( rc == X86EMUL_OKAY )
> > +        *reg = input.segments[seg];
> > +
> > +    return rc;
> > +}
> 
> Just like with ->read_cr(), this must not vary in returned state
> between multiple invocations.
> 

Fixed for both read_segment and write_segment.

> > +static int _fuzz_read_msr(
> > +    unsigned int reg,
> > +    uint64_t *val,
> > +    struct x86_emulate_ctxt *ctxt)
> > +{
> > +    unsigned int idx;
> > +
> > +    switch ( reg )
> > +    {
> > +    case MSR_TSC_AUX:
> > +    case MSR_IA32_TSC:
> > +        return data_read("read_msr", val, sizeof(*val));
> 
> Strictly speaking the above applies to TSC_AUX too. And TSC should
> return monotonically increasing values. I don't think though that
> producing random output here matters right now. A comment may
> be worthwhile.
> 

Right, I will add the following:

        /*                                                                                               
         * TSC should return monotonically increasing values, but                                        
         * returning random values is fine in fuzzer.                                                    
         */

> > +static int fuzz_read_msr(
> > +    unsigned int reg,
> > +    uint64_t *val,
> > +    struct x86_emulate_ctxt *ctxt) {
> > +    int rc;
> > +
> > +    rc = maybe_fail("read_msr", true);
> > +    if ( rc != X86EMUL_OKAY )
> > +        return rc;
> 
> This, otoh, again needs to strictly follow the revised read_cr() model.

Fixed. And then merge _fuzz_read_msr and fuzz_read_msr.

> 
> > +static void set_swint_support(struct x86_emulate_ctxt *ctxt)
> > +{
> > +    unsigned int swint_opt = (input.options >> OPTION_swint_emulation) & 3;
> > +
> > +    static const enum x86_swint_emulation map[4] = {
> 
> As (I think) said before - please don't put blank lines between
> declarations; they're supposed to separate declarations from
> statements.
> 

Fixed.

> > +/*
> > + * Constrain input to architecturally-possible states where
> > + * the emulator relies on these
> > + *
> > + * In general we want the emulator to be as absolutely robust as
> > + * possible; which means that we want to minimize the number of things
> > + * it assumes about the input state.  Tesing this means minimizing and
> > + * removing as much of the input constraints as possible.
> > + *
> > + * So we only add constraints that (in general) have been proven to
> > + * cause crashes in the emulator.
> > + *
> > + * For future reference: other constraints which might be necessary at
> > + * some point:
> > + *
> > + * - EFER.LMA => !EFLAGS.NT
> > + * - In VM86 mode, force segment...
> > + *  - ...access rights to 0xf3
> > + *  - ...limits to 0xffff
> > + *  - ...bases to below 1Mb, 16-byte aligned
> > + *  - ...selectors to (base >> 4)
> > + */
> > +void sanitize_input(struct x86_emulate_ctxt *ctxt) {
> 
> static? And { on its own line.
> 

Done.

> >      do {
> > +        setup_fpu_exception_handler();
> 
> I'm sorry for having mislead you regarding the comment which was
> here: As said elsewhere, I was - based on the function's name -
> assuming this to be a one time action, while in fact this is being done
> before every emulated insn. Once this becomes a one time thing, it
> indeed needs moving out of the loop, so leaving a comment here is
> warranted.
> 

No problem, I add back the original comment.

Wei.

> Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 02/11] x86: extract macros to x86-defns.h
  2017-02-01 13:05   ` Jan Beulich
@ 2017-02-02 16:58     ` Wei Liu
  0 siblings, 0 replies; 28+ messages in thread
From: Wei Liu @ 2017-02-02 16:58 UTC (permalink / raw)
  To: Jan Beulich; +Cc: AndrewCooper, Wei Liu, Xen-devel

On Wed, Feb 01, 2017 at 06:05:23AM -0700, Jan Beulich wrote:
> >>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote:
> > --- a/xen/include/asm-x86/processor.h
> > +++ b/xen/include/asm-x86/processor.h
> > @@ -16,6 +16,8 @@
> >  #include <asm/desc.h>
> >  #endif
> >  
> > +#include "x86-defns.h"
> 
> This should be <asm/x86-defns.h>.

Right. I will fix the inclusion of x86-vendors.h as well.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 09/11] fuzz/x86emul: update fuzzer
  2017-02-02 16:50     ` Wei Liu
@ 2017-02-02 17:01       ` Jan Beulich
  2017-02-02 17:12         ` Wei Liu
  0 siblings, 1 reply; 28+ messages in thread
From: Jan Beulich @ 2017-02-02 17:01 UTC (permalink / raw)
  To: Wei Liu
  Cc: George Dunlap, Andrew Cooper, Ian Jackson, George Dunlap, Xen-devel

>>> On 02.02.17 at 17:50, <wei.liu2@citrix.com> wrote:
> On Thu, Feb 02, 2017 at 05:20:56AM -0700, Jan Beulich wrote:
>> >>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote:
>> > +static int fuzz_read_segment(
>> > +    enum x86_segment seg,
>> > +    struct segment_register *reg,
>> > +    struct x86_emulate_ctxt *ctxt)
>> > +{
>> > +    int rc;
>> > +
>> > +    if ( seg >= SEG_NUM )
>> > +        return X86EMUL_UNHANDLEABLE;
>> > +
>> > +    rc = maybe_fail("read_segment", true);
>> > +
>> > +    if ( rc == X86EMUL_OKAY )
>> > +        *reg = input.segments[seg];
>> > +
>> > +    return rc;
>> > +}
>> 
>> Just like with ->read_cr(), this must not vary in returned state
>> between multiple invocations.
> 
> Fixed for both read_segment and write_segment.

Why for write_segment? That one may fail at any time (and wouldn't
normally be invoked more than once for a given segment anyway).

>> > +static int _fuzz_read_msr(
>> > +    unsigned int reg,
>> > +    uint64_t *val,
>> > +    struct x86_emulate_ctxt *ctxt)
>> > +{
>> > +    unsigned int idx;
>> > +
>> > +    switch ( reg )
>> > +    {
>> > +    case MSR_TSC_AUX:
>> > +    case MSR_IA32_TSC:
>> > +        return data_read("read_msr", val, sizeof(*val));
>> 
>> Strictly speaking the above applies to TSC_AUX too. And TSC should
>> return monotonically increasing values. I don't think though that
>> producing random output here matters right now. A comment may
>> be worthwhile.
>> 
> 
> Right, I will add the following:
> 
>         /*
>          * TSC should return monotonically increasing values, but
>          * returning random values is fine in fuzzer.
>          */

What about TSC_AUX then?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 09/11] fuzz/x86emul: update fuzzer
  2017-02-02 17:01       ` Jan Beulich
@ 2017-02-02 17:12         ` Wei Liu
  2017-02-03  7:04           ` Jan Beulich
  0 siblings, 1 reply; 28+ messages in thread
From: Wei Liu @ 2017-02-02 17:12 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Wei Liu, George Dunlap, Andrew Cooper, Ian Jackson,
	George Dunlap, Xen-devel

On Thu, Feb 02, 2017 at 10:01:46AM -0700, Jan Beulich wrote:
> >>> On 02.02.17 at 17:50, <wei.liu2@citrix.com> wrote:
> > On Thu, Feb 02, 2017 at 05:20:56AM -0700, Jan Beulich wrote:
> >> >>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote:
> >> > +static int fuzz_read_segment(
> >> > +    enum x86_segment seg,
> >> > +    struct segment_register *reg,
> >> > +    struct x86_emulate_ctxt *ctxt)
> >> > +{
> >> > +    int rc;
> >> > +
> >> > +    if ( seg >= SEG_NUM )
> >> > +        return X86EMUL_UNHANDLEABLE;
> >> > +
> >> > +    rc = maybe_fail("read_segment", true);
> >> > +
> >> > +    if ( rc == X86EMUL_OKAY )
> >> > +        *reg = input.segments[seg];
> >> > +
> >> > +    return rc;
> >> > +}
> >> 
> >> Just like with ->read_cr(), this must not vary in returned state
> >> between multiple invocations.
> > 
> > Fixed for both read_segment and write_segment.
> 
> Why for write_segment? That one may fail at any time (and wouldn't
> normally be invoked more than once for a given segment anyway).

OK, that makes sense. I will revert it back.

> 
> >> > +static int _fuzz_read_msr(
> >> > +    unsigned int reg,
> >> > +    uint64_t *val,
> >> > +    struct x86_emulate_ctxt *ctxt)
> >> > +{
> >> > +    unsigned int idx;
> >> > +
> >> > +    switch ( reg )
> >> > +    {
> >> > +    case MSR_TSC_AUX:
> >> > +    case MSR_IA32_TSC:
> >> > +        return data_read("read_msr", val, sizeof(*val));
> >> 
> >> Strictly speaking the above applies to TSC_AUX too. And TSC should
> >> return monotonically increasing values. I don't think though that
> >> producing random output here matters right now. A comment may
> >> be worthwhile.
> >> 
> > 
> > Right, I will add the following:
> > 
> >         /*
> >          * TSC should return monotonically increasing values, but
> >          * returning random values is fine in fuzzer.
> >          */
> 
> What about TSC_AUX then?
> 

What model would you like it to follow? I suppose returning random value
is also fine? I.e. I should just add TSC_AUX to the comment as well.

Wei.

> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 01/11] x86emul/test: add missing dependency for x86_emulate.o
  2017-02-01 13:03   ` Jan Beulich
@ 2017-02-02 17:43     ` Wei Liu
  2017-02-03  7:01       ` Jan Beulich
  0 siblings, 1 reply; 28+ messages in thread
From: Wei Liu @ 2017-02-02 17:43 UTC (permalink / raw)
  To: Jan Beulich; +Cc: AndrewCooper, Wei Liu, Xen-devel

On Wed, Feb 01, 2017 at 06:03:20AM -0700, Jan Beulich wrote:
> >>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote:
> > --- a/tools/tests/x86_emulator/Makefile
> > +++ b/tools/tests/x86_emulator/Makefile
> > @@ -45,7 +45,7 @@ x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
> >  
> >  HOSTCFLAGS += $(CFLAGS_xeninclude)
> >  
> > -x86_emulate.o: x86_emulate.c x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
> > +x86_emulate.o: x86_emulate.c x86_emulate.h x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
> >  	$(HOSTCC) $(HOSTCFLAGS) -D__XEN_TOOLS__ -c -g -o $@ $<
> >  
> >  test_x86_emulator.o: test_x86_emulator.c blowfish.h x86_emulate/x86_emulate.h
> 
> I think this latter one should be updated too. Which then calls for
> macroizing:
> 
> x86_emulate.h := x86_emulate.h x86_emulate/x86_emulate.h
> 
> Jan
> 

Not sure I understand what you want, but using x86_emulate.h as variable
name looks a bit weird. Anyway, this is what I come up with:


From 4bd9c4cbe3ca6197d38355eac24023ccf7b24335 Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@citrix.com>
Date: Wed, 1 Feb 2017 11:49:41 +0000
Subject: [PATCH] x86emul/test: add missing dependency for x86_emulate.o

f4497d6b74 added x86_emulate.h private header but didn't add dependency
for it.

Use macro to reduce repetition.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/tests/x86_emulator/Makefile | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/tests/x86_emulator/Makefile b/tools/tests/x86_emulator/Makefile
index 0b5baff67c..b489959d1d 100644
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -45,8 +45,10 @@ x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
 
 HOSTCFLAGS += $(CFLAGS_xeninclude)
 
-x86_emulate.o: x86_emulate.c x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
+x86_emulate.h := x86_emulate.h x86_emulate/x86_emulate.h
+
+x86_emulate.o: x86_emulate.c x86_emulate/x86_emulate.c $(x86_emulate.h)
 	$(HOSTCC) $(HOSTCFLAGS) -D__XEN_TOOLS__ -c -g -o $@ $<
 
-test_x86_emulator.o: test_x86_emulator.c blowfish.h x86_emulate/x86_emulate.h
+test_x86_emulator.o: test_x86_emulator.c blowfish.h $(x86_emulate.h)
 	$(HOSTCC) $(HOSTCFLAGS) -c -g -o $@ $<
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 01/11] x86emul/test: add missing dependency for x86_emulate.o
  2017-02-02 17:43     ` Wei Liu
@ 2017-02-03  7:01       ` Jan Beulich
  0 siblings, 0 replies; 28+ messages in thread
From: Jan Beulich @ 2017-02-03  7:01 UTC (permalink / raw)
  To: Wei Liu; +Cc: AndrewCooper, Xen-devel

>>> On 02.02.17 at 18:43, <wei.liu2@citrix.com> wrote:
> On Wed, Feb 01, 2017 at 06:03:20AM -0700, Jan Beulich wrote:
>> >>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote:
>> > --- a/tools/tests/x86_emulator/Makefile
>> > +++ b/tools/tests/x86_emulator/Makefile
>> > @@ -45,7 +45,7 @@ x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
>> >  
>> >  HOSTCFLAGS += $(CFLAGS_xeninclude)
>> >  
>> > -x86_emulate.o: x86_emulate.c x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
>> > +x86_emulate.o: x86_emulate.c x86_emulate.h x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
>> >  	$(HOSTCC) $(HOSTCFLAGS) -D__XEN_TOOLS__ -c -g -o $@ $<
>> >  
>> >  test_x86_emulator.o: test_x86_emulator.c blowfish.h x86_emulate/x86_emulate.h
>> 
>> I think this latter one should be updated too. Which then calls for
>> macroizing:
>> 
>> x86_emulate.h := x86_emulate.h x86_emulate/x86_emulate.h
> 
> Not sure I understand what you want, but using x86_emulate.h as variable
> name looks a bit weird.

Well, that name very clearly identifies what the variable is to
represent. If the dot in the name is really disturbing, I could live
with x86_emulate-h as the name. Your proposed adjustment
looks exactly like I was hoping for.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 09/11] fuzz/x86emul: update fuzzer
  2017-02-02 17:12         ` Wei Liu
@ 2017-02-03  7:04           ` Jan Beulich
  2017-02-03 11:22             ` Wei Liu
  0 siblings, 1 reply; 28+ messages in thread
From: Jan Beulich @ 2017-02-03  7:04 UTC (permalink / raw)
  To: Wei Liu
  Cc: George Dunlap, Andrew Cooper, Ian Jackson, George Dunlap, Xen-devel

>>> On 02.02.17 at 18:12, <wei.liu2@citrix.com> wrote:
> On Thu, Feb 02, 2017 at 10:01:46AM -0700, Jan Beulich wrote:
>> >>> On 02.02.17 at 17:50, <wei.liu2@citrix.com> wrote:
>> > On Thu, Feb 02, 2017 at 05:20:56AM -0700, Jan Beulich wrote:
>> >> >>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote:
>> >> > +static int _fuzz_read_msr(
>> >> > +    unsigned int reg,
>> >> > +    uint64_t *val,
>> >> > +    struct x86_emulate_ctxt *ctxt)
>> >> > +{
>> >> > +    unsigned int idx;
>> >> > +
>> >> > +    switch ( reg )
>> >> > +    {
>> >> > +    case MSR_TSC_AUX:
>> >> > +    case MSR_IA32_TSC:
>> >> > +        return data_read("read_msr", val, sizeof(*val));
>> >> 
>> >> Strictly speaking the above applies to TSC_AUX too. And TSC should
>> >> return monotonically increasing values. I don't think though that
>> >> producing random output here matters right now. A comment may
>> >> be worthwhile.
>> >> 
>> > 
>> > Right, I will add the following:
>> > 
>> >         /*
>> >          * TSC should return monotonically increasing values, but
>> >          * returning random values is fine in fuzzer.
>> >          */
>> 
>> What about TSC_AUX then?
>> 
> 
> What model would you like it to follow? I suppose returning random value
> is also fine? I.e. I should just add TSC_AUX to the comment as well.

That's one option. Even better would be to consistently return
the same (possibly random) value. But the value itself isn't being
looked at by the emulator, and iirc the MSR also isn't ever being
read multiple times, so it really doesn't matter as long as the
caller(s) of x86_emulate() do(es)n't care.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 09/11] fuzz/x86emul: update fuzzer
  2017-02-03  7:04           ` Jan Beulich
@ 2017-02-03 11:22             ` Wei Liu
  0 siblings, 0 replies; 28+ messages in thread
From: Wei Liu @ 2017-02-03 11:22 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Wei Liu, George Dunlap, Andrew Cooper, Ian Jackson,
	George Dunlap, Xen-devel

On Fri, Feb 03, 2017 at 12:04:04AM -0700, Jan Beulich wrote:
> >>> On 02.02.17 at 18:12, <wei.liu2@citrix.com> wrote:
> > On Thu, Feb 02, 2017 at 10:01:46AM -0700, Jan Beulich wrote:
> >> >>> On 02.02.17 at 17:50, <wei.liu2@citrix.com> wrote:
> >> > On Thu, Feb 02, 2017 at 05:20:56AM -0700, Jan Beulich wrote:
> >> >> >>> On 01.02.17 at 13:02, <wei.liu2@citrix.com> wrote:
> >> >> > +static int _fuzz_read_msr(
> >> >> > +    unsigned int reg,
> >> >> > +    uint64_t *val,
> >> >> > +    struct x86_emulate_ctxt *ctxt)
> >> >> > +{
> >> >> > +    unsigned int idx;
> >> >> > +
> >> >> > +    switch ( reg )
> >> >> > +    {
> >> >> > +    case MSR_TSC_AUX:
> >> >> > +    case MSR_IA32_TSC:
> >> >> > +        return data_read("read_msr", val, sizeof(*val));
> >> >> 
> >> >> Strictly speaking the above applies to TSC_AUX too. And TSC should
> >> >> return monotonically increasing values. I don't think though that
> >> >> producing random output here matters right now. A comment may
> >> >> be worthwhile.
> >> >> 
> >> > 
> >> > Right, I will add the following:
> >> > 
> >> >         /*
> >> >          * TSC should return monotonically increasing values, but
> >> >          * returning random values is fine in fuzzer.
> >> >          */
> >> 
> >> What about TSC_AUX then?
> >> 
> > 
> > What model would you like it to follow? I suppose returning random value
> > is also fine? I.e. I should just add TSC_AUX to the comment as well.
> 
> That's one option. Even better would be to consistently return
> the same (possibly random) value. But the value itself isn't being
> looked at by the emulator, and iirc the MSR also isn't ever being
> read multiple times, so it really doesn't matter as long as the
> caller(s) of x86_emulate() do(es)n't care.

Returning consistent doable but that would involve rewriting a
significant portion of the fuzzer. Let's leave it for another day.

I will modify the comment as follows:

    /*
     * TSC should return monotonically increasing values, TSC_AUX should
     * preferably return consistent value, but returning random values
     * is fine in fuzzer.
     */

> 
> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2017-02-03 11:22 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-01 12:02 [PATCH v3 00/11] fuzz: update x86emul fuzzer Wei Liu
2017-02-01 12:02 ` [PATCH v3 01/11] x86emul/test: add missing dependency for x86_emulate.o Wei Liu
2017-02-01 13:03   ` Jan Beulich
2017-02-02 17:43     ` Wei Liu
2017-02-03  7:01       ` Jan Beulich
2017-02-01 12:02 ` [PATCH v3 02/11] x86: extract macros to x86-defns.h Wei Liu
2017-02-01 13:05   ` Jan Beulich
2017-02-02 16:58     ` Wei Liu
2017-02-01 12:02 ` [PATCH v3 03/11] x86: extract vendor numeric id to x86-vendors.h Wei Liu
2017-02-01 12:02 ` [PATCH v3 04/11] x86emul/test: use x86-vendors.h Wei Liu
2017-02-01 13:09   ` Jan Beulich
2017-02-01 13:13     ` Andrew Cooper
2017-02-01 13:22       ` Wei Liu
2017-02-01 13:26       ` Jan Beulich
2017-02-01 12:02 ` [PATCH v3 05/11] x86emul: use eflags definitions in x86-defns.h Wei Liu
2017-02-01 12:02 ` [PATCH v3 06/11] x86emul: use msr definitions in msr-index.h Wei Liu
2017-02-01 12:02 ` [PATCH v3 07/11] x86: add UMIP CR4 bit Wei Liu
2017-02-01 12:02 ` [PATCH v3 08/11] x86emul: use CR definitions in x86-defns.h Wei Liu
2017-02-01 12:02 ` [PATCH v3 09/11] fuzz/x86emul: update fuzzer Wei Liu
2017-02-01 16:57   ` Wei Liu
2017-02-02 12:20   ` Jan Beulich
2017-02-02 16:50     ` Wei Liu
2017-02-02 17:01       ` Jan Beulich
2017-02-02 17:12         ` Wei Liu
2017-02-03  7:04           ` Jan Beulich
2017-02-03 11:22             ` Wei Liu
2017-02-01 12:02 ` [PATCH v3 10/11] fuzz/x86emul: print out minimal input size Wei Liu
2017-02-01 12:02 ` [PATCH v3 11/11] fuzz: update README.afl example Wei Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.