[Qemu-devel] [RISU v3 00/11] Support for i386/x86_64 with vector extensions

[Qemu-devel] [RISU v3 00/11] Support for i386/x86_64 with vector extensions

[Jan Bobek]

This patch series adds support for i386 and x86_64 architectures to
RISU. Notably, vector registers (SSE, AVX, AVX-512) are supported for
verification of the apprentice. This is V3 of the series posted in [1]
and [2].

Changes is V3:
  - fix a matching bug caused by misplaced index multiplication [3]
  - print an error and exit when parse of the --xfeatures option fails [4]

References:
  1. https://lists.nongnu.org/archive/html/qemu-devel/2019-04/msg01294.html
  2. https://lists.nongnu.org/archive/html/qemu-devel/2019-05/msg04089.html
  3. https://lists.nongnu.org/archive/html/qemu-devel/2019-05/msg04922.html
  4. https://lists.nongnu.org/archive/html/qemu-devel/2019-05/msg04903.html

Jan Bobek (10):
  Makefile: undefine the arch name symbol
  risu_i386: move reginfo_t and related defines to risu_reginfo_i386.h
  risu_i386: move reginfo-related code to risu_reginfo_i386.c
  risu_reginfo_i386: implement arch-specific reginfo interface
  risu_i386: implement missing CPU-specific functions
  risu_i386: remove old unused code
  test_i386: change syntax from nasm to gas
  configure: add i386/x86_64 architectures
  risu_reginfo_i386: replace xfeature constants with symbolic names
  risu_reginfo_i386: rework --xfeatures value parsing

Richard Henderson (1):
  i386: Add avx512 state to reginfo_t

 configure           |  10 +-
 Makefile            |   5 +-
 risu_reginfo_i386.h |  49 ++++++
 risu_i386.c         | 142 ++--------------
 risu_reginfo_i386.c | 400 ++++++++++++++++++++++++++++++++++++++++++++
 test_i386.S         |  80 +++++++++
 test_i386.s         |  27 ---
 7 files changed, 556 insertions(+), 157 deletions(-)
 create mode 100644 risu_reginfo_i386.h
 create mode 100644 risu_reginfo_i386.c
 create mode 100644 test_i386.S
 delete mode 100644 test_i386.s

-- 
2.20.1

[Qemu-devel] [RISU v3 01/11] Makefile: undefine the arch name symbol

[Jan Bobek]

At least GCC defines the symbol "i386" to 1 to signal the target
platform. We need to use "i386" as an undefined symbol in order to
correctly include risu_reginfo_i386.h from risu.h. Add an -U option to
the build command to make sure the symbol remains undefined.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Jan Bobek <jan.bobek@gmail.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 4aad448..b362dbe 100644
--- a/Makefile
+++ b/Makefile
@@ -17,7 +17,7 @@ VPATH=$(SRCDIR)
 
 CFLAGS ?= -g
 
-ALL_CFLAGS = -Wall -D_GNU_SOURCE -DARCH=$(ARCH) $(BUILD_INC) $(CFLAGS) $(EXTRA_CFLAGS)
+ALL_CFLAGS = -Wall -D_GNU_SOURCE -DARCH=$(ARCH) -U$(ARCH) $(BUILD_INC) $(CFLAGS) $(EXTRA_CFLAGS)
 
 PROG=risu
 SRCS=risu.c comms.c reginfo.c risu_$(ARCH).c risu_reginfo_$(ARCH).c
-- 
2.20.1

[Qemu-devel] [RISU v3 02/11] risu_i386: move reginfo_t and related defines to risu_reginfo_i386.h

[Jan Bobek]

In order to build risu successfully for i386, we need files
risu_reginfo_i386.{h,c}; this patch adds the former by extracting the
relevant code from risu_i386.c.

This patch is pure code motion; no functional changes were made.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Jan Bobek <jan.bobek@gmail.com>
---
 risu_reginfo_i386.h | 37 +++++++++++++++++++++++++++++++++++++
 risu_i386.c         | 23 +----------------------
 2 files changed, 38 insertions(+), 22 deletions(-)
 create mode 100644 risu_reginfo_i386.h

diff --git a/risu_reginfo_i386.h b/risu_reginfo_i386.h
new file mode 100644
index 0000000..5bba439
--- /dev/null
+++ b/risu_reginfo_i386.h
@@ -0,0 +1,37 @@
+/*******************************************************************************
+ * Copyright (c) 2010 Linaro Limited
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Peter Maydell (Linaro) - initial implementation
+ ******************************************************************************/
+
+#ifndef RISU_REGINFO_I386_H
+#define RISU_REGINFO_I386_H
+
+/* This is the data structure we pass over the socket.
+ * It is a simplified and reduced subset of what can
+ * be obtained with a ucontext_t*
+ */
+struct reginfo {
+    uint32_t faulting_insn;
+    gregset_t gregs;
+};
+
+#ifndef REG_GS
+/* Assume that either we get all these defines or none */
+#   define REG_GS      0
+#   define REG_FS      1
+#   define REG_ES      2
+#   define REG_DS      3
+#   define REG_ESP     7
+#   define REG_TRAPNO 12
+#   define REG_EIP    14
+#   define REG_EFL    16
+#   define REG_UESP   17
+#endif /* !defined(REG_GS) */
+
+#endif /* RISU_REGINFO_I386_H */
diff --git a/risu_i386.c b/risu_i386.c
index 5e7e01d..6798a78 100644
--- a/risu_i386.c
+++ b/risu_i386.c
@@ -14,28 +14,7 @@
 #include <string.h>
 
 #include "risu.h"
-
-/* This is the data structure we pass over the socket.
- * It is a simplified and reduced subset of what can
- * be obtained with a ucontext_t*
- */
-struct reginfo {
-    uint32_t faulting_insn;
-    gregset_t gregs;
-};
-
-#ifndef REG_GS
-/* Assume that either we get all these defines or none */
-#define REG_GS 0
-#define REG_FS 1
-#define REG_ES 2
-#define REG_DS 3
-#define REG_ESP 7
-#define REG_TRAPNO 12
-#define REG_EIP 14
-#define REG_EFL 16
-#define REG_UESP 17
-#endif
+#include "risu_reginfo_i386.h"
 
 struct reginfo master_ri, apprentice_ri;
 
-- 
2.20.1

[Qemu-devel] [RISU v3 03/11] risu_i386: move reginfo-related code to risu_reginfo_i386.c

[Jan Bobek]

In order to build risu successfully for i386, we need files
risu_reginfo_i386.{h,c}; this patch adds the latter by extracting the
relevant code from risu_i386.c.

This patch is pure code motion; no functional changes were made.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Jan Bobek <jan.bobek@gmail.com>
---
 risu_i386.c         | 54 -----------------------------------
 risu_reginfo_i386.c | 68 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 54 deletions(-)
 create mode 100644 risu_reginfo_i386.c

diff --git a/risu_i386.c b/risu_i386.c
index 6798a78..2d2f325 100644
--- a/risu_i386.c
+++ b/risu_i386.c
@@ -33,43 +33,6 @@ void advance_pc(void *vuc)
     uc->uc_mcontext.gregs[REG_EIP] += 2;
 }
 
-static void fill_reginfo(struct reginfo *ri, ucontext_t * uc)
-{
-    int i;
-    for (i = 0; i < NGREG; i++) {
-        switch (i) {
-        case REG_ESP:
-        case REG_UESP:
-        case REG_GS:
-        case REG_FS:
-        case REG_ES:
-        case REG_DS:
-        case REG_TRAPNO:
-        case REG_EFL:
-            /* Don't store these registers as it results in mismatches.
-             * In particular valgrind has different values for some
-             * segment registers, and they're boring anyway.
-             * We really shouldn't be ignoring EFL but valgrind doesn't
-             * seem to set it right and I don't care to investigate.
-             */
-            ri->gregs[i] = 0xDEADBEEF;
-            break;
-        case REG_EIP:
-            /* Store the offset from the start of the test image */
-            ri->gregs[i] = uc->uc_mcontext.gregs[i] - image_start_address;
-            break;
-        default:
-            ri->gregs[i] = uc->uc_mcontext.gregs[i];
-            break;
-        }
-    }
-    /* x86 insns aren't 32 bit but we're not really testing x86 so
-     * this is just to distinguish 'do compare' from 'stop'
-     */
-    ri->faulting_insn = *((uint32_t *) uc->uc_mcontext.gregs[REG_EIP]);
-}
-
-
 int send_register_info(int sock, void *uc)
 {
     struct reginfo ri;
@@ -100,23 +63,6 @@ int recv_and_compare_register_info(int sock, void *uc)
     return resp;
 }
 
-static char *regname[] = {
-    "GS", "FS", "ES", "DS", "EDI", "ESI", "EBP", "ESP",
-    "EBX", "EDX", "ECX", "EAX", "TRAPNO", "ERR", "EIP",
-    "CS", "EFL", "UESP", "SS", 0
-};
-
-static void dump_reginfo(struct reginfo *ri)
-{
-    int i;
-    fprintf(stderr, "  faulting insn %x\n", ri->faulting_insn);
-    for (i = 0; i < NGREG; i++) {
-        fprintf(stderr, "  %s: %x\n", regname[i] ? regname[i] : "???",
-                ri->gregs[i]);
-    }
-}
-
-
 /* Print a useful report on the status of the last comparison
  * done in recv_and_compare_register_info(). This is called on
  * exit, so need not restrict itself to signal-safe functions.
diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
new file mode 100644
index 0000000..e8d671f
--- /dev/null
+++ b/risu_reginfo_i386.c
@@ -0,0 +1,68 @@
+/*******************************************************************************
+ * Copyright (c) 2010 Linaro Limited
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Peter Maydell (Linaro) - initial implementation
+ ******************************************************************************/
+
+#include <stdio.h>
+#include <ucontext.h>
+
+#include "risu.h"
+#include "risu_reginfo_i386.h"
+
+static void fill_reginfo(struct reginfo *ri, ucontext_t * uc)
+{
+    int i;
+    for (i = 0; i < NGREG; i++) {
+        switch (i) {
+        case REG_ESP:
+        case REG_UESP:
+        case REG_GS:
+        case REG_FS:
+        case REG_ES:
+        case REG_DS:
+        case REG_TRAPNO:
+        case REG_EFL:
+            /* Don't store these registers as it results in mismatches.
+             * In particular valgrind has different values for some
+             * segment registers, and they're boring anyway.
+             * We really shouldn't be ignoring EFL but valgrind doesn't
+             * seem to set it right and I don't care to investigate.
+             */
+            ri->gregs[i] = 0xDEADBEEF;
+            break;
+        case REG_EIP:
+            /* Store the offset from the start of the test image */
+            ri->gregs[i] = uc->uc_mcontext.gregs[i] - image_start_address;
+            break;
+        default:
+            ri->gregs[i] = uc->uc_mcontext.gregs[i];
+            break;
+        }
+    }
+    /* x86 insns aren't 32 bit but we're not really testing x86 so
+     * this is just to distinguish 'do compare' from 'stop'
+     */
+    ri->faulting_insn = *((uint32_t *) uc->uc_mcontext.gregs[REG_EIP]);
+}
+
+static char *regname[] = {
+    "GS", "FS", "ES", "DS", "EDI", "ESI", "EBP", "ESP",
+    "EBX", "EDX", "ECX", "EAX", "TRAPNO", "ERR", "EIP",
+    "CS", "EFL", "UESP", "SS", 0
+};
+
+static void dump_reginfo(struct reginfo *ri)
+{
+    int i;
+    fprintf(stderr, "  faulting insn %x\n", ri->faulting_insn);
+    for (i = 0; i < NGREG; i++) {
+        fprintf(stderr, "  %s: %x\n", regname[i] ? regname[i] : "???",
+                ri->gregs[i]);
+    }
+}
-- 
2.20.1

[Qemu-devel] [RISU v3 04/11] risu_reginfo_i386: implement arch-specific reginfo interface

[Jan Bobek]

CPU-specific code in risu_reginfo_* is expected to define and export
the following symbols:

- arch_long_opts, arch_extra_help, process_arch_opt
- reginfo_size
- reginfo_init
- reginfo_is_eq
- reginfo_dump, reginfo_dump_mismatch

Make risu_reginfo_i386.c implement this interface; and while we're at
it, expand the support to x86_64 as well.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Jan Bobek <jan.bobek@gmail.com>
---
 risu_reginfo_i386.h |  24 ++++----
 risu_reginfo_i386.c | 147 ++++++++++++++++++++++++++++++++++----------
 2 files changed, 127 insertions(+), 44 deletions(-)

diff --git a/risu_reginfo_i386.h b/risu_reginfo_i386.h
index 5bba439..e350f01 100644
--- a/risu_reginfo_i386.h
+++ b/risu_reginfo_i386.h
@@ -12,7 +12,8 @@
 #ifndef RISU_REGINFO_I386_H
 #define RISU_REGINFO_I386_H
 
-/* This is the data structure we pass over the socket.
+/*
+ * This is the data structure we pass over the socket.
  * It is a simplified and reduced subset of what can
  * be obtained with a ucontext_t*
  */
@@ -21,17 +22,14 @@ struct reginfo {
     gregset_t gregs;
 };
 
-#ifndef REG_GS
-/* Assume that either we get all these defines or none */
-#   define REG_GS      0
-#   define REG_FS      1
-#   define REG_ES      2
-#   define REG_DS      3
-#   define REG_ESP     7
-#   define REG_TRAPNO 12
-#   define REG_EIP    14
-#   define REG_EFL    16
-#   define REG_UESP   17
-#endif /* !defined(REG_GS) */
+/*
+ * For i386, the defines are named REG_EAX, etc.
+ * For x86_64, the defines are named REG_RAX, etc.
+ */
+#ifdef __x86_64__
+# define REG_E(X)   REG_R##X
+#else
+# define REG_E(X)   REG_E##X
+#endif
 
 #endif /* RISU_REGINFO_I386_H */
diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
index e8d671f..c4dc14a 100644
--- a/risu_reginfo_i386.c
+++ b/risu_reginfo_i386.c
@@ -10,59 +10,144 @@
  ******************************************************************************/
 
 #include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
 #include <ucontext.h>
+#include <assert.h>
 
 #include "risu.h"
 #include "risu_reginfo_i386.h"
 
-static void fill_reginfo(struct reginfo *ri, ucontext_t * uc)
+const struct option * const arch_long_opts;
+const char * const arch_extra_help;
+
+void process_arch_opt(int opt, const char *arg)
+{
+    abort();
+}
+
+const int reginfo_size(void)
+{
+    return sizeof(struct reginfo);
+}
+
+/* reginfo_init: initialize with a ucontext */
+void reginfo_init(struct reginfo *ri, ucontext_t *uc)
 {
     int i;
+
+    memset(ri, 0, sizeof(*ri));
+
     for (i = 0; i < NGREG; i++) {
         switch (i) {
-        case REG_ESP:
-        case REG_UESP:
-        case REG_GS:
-        case REG_FS:
-        case REG_ES:
-        case REG_DS:
-        case REG_TRAPNO:
-        case REG_EFL:
-            /* Don't store these registers as it results in mismatches.
-             * In particular valgrind has different values for some
-             * segment registers, and they're boring anyway.
-             * We really shouldn't be ignoring EFL but valgrind doesn't
-             * seem to set it right and I don't care to investigate.
-             */
-            ri->gregs[i] = 0xDEADBEEF;
-            break;
-        case REG_EIP:
-            /* Store the offset from the start of the test image */
+        case REG_E(IP):
+            /* Store the offset from the start of the test image.  */
             ri->gregs[i] = uc->uc_mcontext.gregs[i] - image_start_address;
             break;
-        default:
+        case REG_EFL:
+            /* Store only the "flaggy" bits: SF, ZF, AF, PF, CF.  */
+            ri->gregs[i] = uc->uc_mcontext.gregs[i] & 0xd5;
+            break;
+        case REG_E(SP):
+            /* Ignore the stack.  */
+            ri->gregs[i] = 0xdeadbeef;
+            break;
+        case REG_E(AX):
+        case REG_E(BX):
+        case REG_E(CX):
+        case REG_E(DX):
+        case REG_E(DI):
+        case REG_E(SI):
+        case REG_E(BP):
+#ifdef __x86_64__
+        case REG_R8:
+        case REG_R9:
+        case REG_R10:
+        case REG_R11:
+        case REG_R12:
+        case REG_R13:
+        case REG_R14:
+        case REG_R15:
+#endif
             ri->gregs[i] = uc->uc_mcontext.gregs[i];
             break;
         }
     }
-    /* x86 insns aren't 32 bit but we're not really testing x86 so
-     * this is just to distinguish 'do compare' from 'stop'
+
+    /*
+     * x86 insns aren't 32 bit but 3 bytes are sufficient to
+     * distinguish 'do compare' from 'stop'.
      */
-    ri->faulting_insn = *((uint32_t *) uc->uc_mcontext.gregs[REG_EIP]);
+    ri->faulting_insn = *(uint32_t *)uc->uc_mcontext.gregs[REG_E(IP)];
 }
 
-static char *regname[] = {
-    "GS", "FS", "ES", "DS", "EDI", "ESI", "EBP", "ESP",
-    "EBX", "EDX", "ECX", "EAX", "TRAPNO", "ERR", "EIP",
-    "CS", "EFL", "UESP", "SS", 0
+/* reginfo_is_eq: compare the reginfo structs, returns nonzero if equal */
+int reginfo_is_eq(struct reginfo *m, struct reginfo *a)
+{
+    return 0 == memcmp(m, a, sizeof(*m));
+}
+
+static const char *const regname[NGREG] = {
+    [REG_EFL] = "eflags",
+#ifdef __x86_64__
+    [REG_RIP] = "rip",
+    [REG_RAX] = "rax",
+    [REG_RBX] = "rbx",
+    [REG_RCX] = "rcx",
+    [REG_RDX] = "rdx",
+    [REG_RDI] = "rdi",
+    [REG_RSI] = "rsi",
+    [REG_RBP] = "rbp",
+    [REG_RSP] = "rsp",
+    [REG_R8]  = "r8",
+    [REG_R9]  = "r9",
+    [REG_R10] = "r10",
+    [REG_R11] = "r11",
+    [REG_R12] = "r12",
+    [REG_R13] = "r13",
+    [REG_R14] = "r14",
+    [REG_R15] = "r15",
+#else
+    [REG_EIP] = "eip",
+    [REG_EAX] = "eax",
+    [REG_EBX] = "ebx",
+    [REG_ECX] = "ecx",
+    [REG_EDX] = "edx",
+    [REG_EDI] = "edi",
+    [REG_ESI] = "esi",
+    [REG_EBP] = "ebp",
+    [REG_ESP] = "esp",
+#endif
 };
 
-static void dump_reginfo(struct reginfo *ri)
+#ifdef __x86_64__
+# define PRIxREG   "%016llx"
+#else
+# define PRIxREG   "%08x"
+#endif
+
+/* reginfo_dump: print state to a stream, returns nonzero on success */
+int reginfo_dump(struct reginfo *ri, FILE *f)
 {
     int i;
-    fprintf(stderr, "  faulting insn %x\n", ri->faulting_insn);
+    fprintf(f, "  faulting insn %x\n", ri->faulting_insn);
     for (i = 0; i < NGREG; i++) {
-        fprintf(stderr, "  %s: %x\n", regname[i] ? regname[i] : "???",
-                ri->gregs[i]);
+        if (regname[i]) {
+            fprintf(f, "  %-6s: " PRIxREG "\n", regname[i], ri->gregs[i]);
+        }
     }
+    return !ferror(f);
+}
+
+int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
+{
+    int i;
+    for (i = 0; i < NGREG; i++) {
+        if (m->gregs[i] != a->gregs[i]) {
+            assert(regname[i]);
+            fprintf(f, "Mismatch: %s: " PRIxREG " v " PRIxREG "\n",
+                    regname[i], m->gregs[i], a->gregs[i]);
+        }
+    }
+    return !ferror(f);
 }
-- 
2.20.1

[Qemu-devel] [RISU v3 05/11] risu_i386: implement missing CPU-specific functions

[Jan Bobek]

risu_i386.c is expected to implement the following functions:

- advance_pc
- get_reginfo_paramreg, set_ucontext_paramreg
- get_risuop
- get_pc

This patch adds the necessary code. We use EAX as the parameter
register and opcode "UD1 %xxx,%eax" for triggering RISU actions.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Jan Bobek <jan.bobek@gmail.com>
---
 risu_i386.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/risu_i386.c b/risu_i386.c
index 2d2f325..06d95e5 100644
--- a/risu_i386.c
+++ b/risu_i386.c
@@ -25,12 +25,37 @@ static int insn_is_ud2(uint32_t insn)
 
 void advance_pc(void *vuc)
 {
-    /* We assume that this is either UD1 or UD2.
-     * This would need tweaking if we want to test
-     * expected undefs on x86.
+    ucontext_t *uc = (ucontext_t *) vuc;
+
+    /*
+     * We assume that this is UD1 as per get_risuop below.
+     * This would need tweaking if we want to test expected undefs.
      */
-    ucontext_t *uc = vuc;
-    uc->uc_mcontext.gregs[REG_EIP] += 2;
+    uc->uc_mcontext.gregs[REG_E(IP)] += 3;
+}
+
+void set_ucontext_paramreg(void *vuc, uint64_t value)
+{
+    ucontext_t *uc = (ucontext_t *) vuc;
+    uc->uc_mcontext.gregs[REG_E(AX)] = value;
+}
+
+uint64_t get_reginfo_paramreg(struct reginfo *ri)
+{
+    return ri->gregs[REG_E(AX)];
+}
+
+int get_risuop(struct reginfo *ri)
+{
+    if ((ri->faulting_insn & 0xf8ffff) == 0xc0b90f) { /* UD1 %xxx,%eax */
+        return (ri->faulting_insn >> 16) & 7;
+    }
+    return -1;
+}
+
+uintptr_t get_pc(struct reginfo *ri)
+{
+    return ri->gregs[REG_E(IP)];
 }
 
 int send_register_info(int sock, void *uc)
-- 
2.20.1

[Qemu-devel] [RISU v3 06/11] risu_i386: remove old unused code

[Jan Bobek]

The code being removed is a remnant of the past implementation; it has
since been replaced by its more powerful, architecture-independent
counterpart in reginfo.c.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Jan Bobek <jan.bobek@gmail.com>
---
 risu_i386.c | 58 -----------------------------------------------------
 1 file changed, 58 deletions(-)

diff --git a/risu_i386.c b/risu_i386.c
index 06d95e5..9962b8f 100644
--- a/risu_i386.c
+++ b/risu_i386.c
@@ -16,13 +16,6 @@
 #include "risu.h"
 #include "risu_reginfo_i386.h"
 
-struct reginfo master_ri, apprentice_ri;
-
-static int insn_is_ud2(uint32_t insn)
-{
-    return ((insn & 0xffff) == 0x0b0f);
-}
-
 void advance_pc(void *vuc)
 {
     ucontext_t *uc = (ucontext_t *) vuc;
@@ -57,54 +50,3 @@ uintptr_t get_pc(struct reginfo *ri)
 {
     return ri->gregs[REG_E(IP)];
 }
-
-int send_register_info(int sock, void *uc)
-{
-    struct reginfo ri;
-    fill_reginfo(&ri, uc);
-    return send_data_pkt(sock, &ri, sizeof(ri));
-}
-
-/* Read register info from the socket and compare it with that from the
- * ucontext. Return 0 for match, 1 for end-of-test, 2 for mismatch.
- * NB: called from a signal handler.
- */
-int recv_and_compare_register_info(int sock, void *uc)
-{
-    int resp;
-    fill_reginfo(&master_ri, uc);
-    recv_data_pkt(sock, &apprentice_ri, sizeof(apprentice_ri));
-    if (memcmp(&master_ri, &apprentice_ri, sizeof(master_ri)) != 0) {
-        /* mismatch */
-        resp = 2;
-    } else if (insn_is_ud2(master_ri.faulting_insn)) {
-        /* end of test */
-        resp = 1;
-    } else {
-        /* either successful match or expected undef */
-        resp = 0;
-    }
-    send_response_byte(sock, resp);
-    return resp;
-}
-
-/* Print a useful report on the status of the last comparison
- * done in recv_and_compare_register_info(). This is called on
- * exit, so need not restrict itself to signal-safe functions.
- * Should return 0 if it was a good match (ie end of test)
- * and 1 for a mismatch.
- */
-int report_match_status(void)
-{
-    fprintf(stderr, "match status...\n");
-    fprintf(stderr, "master reginfo:\n");
-    dump_reginfo(&master_ri);
-    fprintf(stderr, "apprentice reginfo:\n");
-    dump_reginfo(&apprentice_ri);
-    if (memcmp(&master_ri, &apprentice_ri, sizeof(master_ri)) == 0) {
-        fprintf(stderr, "match!\n");
-        return 0;
-    }
-    fprintf(stderr, "mismatch!\n");
-    return 1;
-}
-- 
2.20.1

[Qemu-devel] [RISU v3 07/11] test_i386: change syntax from nasm to gas

[Jan Bobek]

This allows us to drop dependency on NASM and build the test image
with GCC only. Adds support for x86_64, too.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Jan Bobek <jan.bobek@gmail.com>
---
 Makefile    |  3 +++
 test_i386.S | 41 +++++++++++++++++++++++++++++++++++++++++
 test_i386.s | 27 ---------------------------
 3 files changed, 44 insertions(+), 27 deletions(-)
 create mode 100644 test_i386.S
 delete mode 100644 test_i386.s

diff --git a/Makefile b/Makefile
index b362dbe..6ab014a 100644
--- a/Makefile
+++ b/Makefile
@@ -49,6 +49,9 @@ $(PROG): $(OBJS)
 %_$(ARCH).elf: %_$(ARCH).s
 	$(AS) -o $@ $<
 
+%_$(ARCH).elf: %_$(ARCH).S
+	$(CC) $(CPPFLAGS) -o $@ -c $<
+
 clean:
 	rm -f $(PROG) $(OBJS) $(BINS)
 
diff --git a/test_i386.S b/test_i386.S
new file mode 100644
index 0000000..456b99c
--- /dev/null
+++ b/test_i386.S
@@ -0,0 +1,41 @@
+/*#############################################################################
+ * Copyright (c) 2010 Linaro Limited
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Peter Maydell (Linaro) - initial implementation
+ *###########################################################################*/
+
+/* A trivial test image for x86 */
+
+/* Initialise the registers to avoid spurious mismatches */
+	xor	%eax, %eax
+	sahf				/* init eflags */
+
+	mov	$0x12345678, %eax
+	mov	$0x9abcdef0, %ebx
+	mov	$0x97361234, %ecx
+	mov	$0x84310284, %edx
+	mov	$0x83624173, %edi
+	mov	$0xfaebfaeb, %esi
+	mov	$0x84610123, %ebp
+
+#ifdef __x86_64__
+	movq	$0x123456789abcdef0, %r8
+	movq	$0xaaaabbbbccccdddd, %r9
+	movq	$0x1010101010101010, %r10
+	movq	$0x1111111111111111, %r11
+	movq	$0x1212121212121212, %r12
+	movq	$0x1313131313131313, %r13
+	movq	$0x1414141414141414, %r14
+	movq	$0x1515151515151515, %r15
+#endif
+
+/* do compare */
+	ud1	%eax, %eax
+
+/* exit test */
+	ud1	%ecx, %eax
diff --git a/test_i386.s b/test_i386.s
deleted file mode 100644
index a2140a0..0000000
--- a/test_i386.s
+++ /dev/null
@@ -1,27 +0,0 @@
-;###############################################################################
-;# Copyright (c) 2010 Linaro Limited
-;# All rights reserved. This program and the accompanying materials
-;# are made available under the terms of the Eclipse Public License v1.0
-;# which accompanies this distribution, and is available at
-;# http://www.eclipse.org/legal/epl-v10.html
-;#
-;# Contributors:
-;#     Peter Maydell (Linaro) - initial implementation
-;###############################################################################
-
-; A trivial test image for x86
-
-BITS 32
-; Initialise the registers to avoid spurious mismatches
-mov eax, 0x12345678
-mov ebx, 0x9abcdef0
-mov ecx, 0x97361234
-mov edx, 0x84310284
-mov edi, 0x83624173
-mov esi, 0xfaebfaeb
-mov ebp, 0x84610123
-; UD1 : do compare
-UD1
-
-; UD2 : exit test
-UD2
-- 
2.20.1

[Qemu-devel] [RISU v3 08/11] configure: add i386/x86_64 architectures

[Jan Bobek]

Now that i386 and x86_64 architectures are supported by RISU, we want
to detect them and build RISU for them automatically.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Jan Bobek <jan.bobek@gmail.com>
---
 configure | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index 65e1819..ca2d7db 100755
--- a/configure
+++ b/configure
@@ -48,12 +48,14 @@ EOF
 }
 
 guess_arch() {
-    if check_define __m68k__ ; then
-        ARCH="m68k"
+    if check_define __aarch64__ ; then
+        ARCH="aarch64"
     elif check_define __arm__ ; then
         ARCH="arm"
-    elif check_define __aarch64__ ; then
-        ARCH="aarch64"
+    elif check_define __i386__ || check_define __x86_64__ ; then
+        ARCH="i386"
+    elif check_define __m68k__ ; then
+        ARCH="m68k"
     elif check_define __powerpc64__ ; then
         ARCH="ppc64"
     else
-- 
2.20.1

[Qemu-devel] [RISU v3 09/11] i386: Add avx512 state to reginfo_t

[Jan Bobek]

From: Richard Henderson <richard.henderson@linaro.org>

The state expected for a given test must be specifically requested
with the --xfeatures=mask command-line argument.  This is recorded
with the saved state so that it is obvious if the apprentice is given
a different argument.  Any features beyond what are present on the
running cpu will read as zero.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 risu_reginfo_i386.h |  14 +++
 risu_reginfo_i386.c | 228 ++++++++++++++++++++++++++++++++++++++++++--
 test_i386.S         |  39 ++++++++
 3 files changed, 273 insertions(+), 8 deletions(-)

diff --git a/risu_reginfo_i386.h b/risu_reginfo_i386.h
index e350f01..b468f79 100644
--- a/risu_reginfo_i386.h
+++ b/risu_reginfo_i386.h
@@ -12,6 +12,10 @@
 #ifndef RISU_REGINFO_I386_H
 #define RISU_REGINFO_I386_H
 
+struct avx512_reg {
+    uint64_t q[8];
+};
+
 /*
  * This is the data structure we pass over the socket.
  * It is a simplified and reduced subset of what can
@@ -19,7 +23,17 @@
  */
 struct reginfo {
     uint32_t faulting_insn;
+    uint32_t mxcsr;
+    uint64_t xfeatures;
+
     gregset_t gregs;
+
+#ifdef __x86_64__
+    struct avx512_reg vregs[32];
+#else
+    struct avx512_reg vregs[8];
+#endif
+    uint64_t kregs[8];
 };
 
 /*
diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
index c4dc14a..83f9541 100644
--- a/risu_reginfo_i386.c
+++ b/risu_reginfo_i386.c
@@ -11,19 +11,32 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <stddef.h>
 #include <string.h>
 #include <ucontext.h>
 #include <assert.h>
+#include <cpuid.h>
 
 #include "risu.h"
 #include "risu_reginfo_i386.h"
 
-const struct option * const arch_long_opts;
-const char * const arch_extra_help;
+#include <asm/sigcontext.h>
+
+static uint64_t xfeatures = 3;  /* SSE */
+
+static const struct option extra_ops[] = {
+    {"xfeatures", required_argument, NULL, FIRST_ARCH_OPT },
+    {0, 0, 0, 0}
+};
+
+const struct option * const arch_long_opts = extra_ops;
+const char * const arch_extra_help
+    = "  --xfeatures=<mask>  Use features in mask for XSAVE\n";
 
 void process_arch_opt(int opt, const char *arg)
 {
-    abort();
+    assert(opt == FIRST_ARCH_OPT);
+    xfeatures = strtoull(arg, 0, 0);
 }
 
 const int reginfo_size(void)
@@ -31,13 +44,37 @@ const int reginfo_size(void)
     return sizeof(struct reginfo);
 }
 
+static void *xsave_feature_buf(struct _xstate *xs, int feature)
+{
+    unsigned int eax, ebx, ecx, edx;
+    int ok;
+
+    /*
+     * Get the location of the XSAVE feature from the cpuid leaf.
+     * Given that we know the xfeature bit is set, this must succeed.
+     */
+    ok = __get_cpuid_count(0xd, feature, &eax, &ebx, &ecx, &edx);
+    assert(ok);
+
+    /* Sanity check that the frame stored by the kernel contains the data. */
+    assert(xs->fpstate.sw_reserved.extended_size >= eax + ebx);
+
+    return (void *)xs + ebx;
+}
+
 /* reginfo_init: initialize with a ucontext */
 void reginfo_init(struct reginfo *ri, ucontext_t *uc)
 {
-    int i;
+    int i, nvecregs;
+    struct _fpstate *fp;
+    struct _xstate *xs;
+    uint64_t features;
 
     memset(ri, 0, sizeof(*ri));
 
+    /* Require master and apprentice to be given the same arguments.  */
+    ri->xfeatures = xfeatures;
+
     for (i = 0; i < NGREG; i++) {
         switch (i) {
         case REG_E(IP):
@@ -79,12 +116,89 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
      * distinguish 'do compare' from 'stop'.
      */
     ri->faulting_insn = *(uint32_t *)uc->uc_mcontext.gregs[REG_E(IP)];
+
+    /*
+     * FP state is omitted if unused (aka in init state).
+     * Use the <asm/sigcontext.h> struct for access to AVX state.
+     */
+
+    fp = (struct _fpstate *)uc->uc_mcontext.fpregs;
+    if (fp == NULL) {
+        return;
+    }
+
+#ifdef __x86_64__
+    nvecregs = 16;
+#else
+    /* We don't (currently) care about the 80387 state, only SSE+.  */
+    if (fp->magic != X86_FXSR_MAGIC) {
+        return;
+    }
+    nvecregs = 8;
+#endif
+
+    /*
+     * Now we know that _fpstate contains FXSAVE data.
+     */
+    ri->mxcsr = fp->mxcsr;
+
+    for (i = 0; i < nvecregs; ++i) {
+#ifdef __x86_64__
+        memcpy(&ri->vregs[i], &fp->xmm_space[i * 4], 16);
+#else
+        memcpy(&ri->vregs[i], &fp->_xmm[i], 16);
+#endif
+    }
+
+    if (fp->sw_reserved.magic1 != FP_XSTATE_MAGIC1) {
+        return;
+    }
+    xs = (struct _xstate *)fp;
+    features = xfeatures & xs->xstate_hdr.xfeatures;
+
+    /*
+     * Now we know that _fpstate contains XSAVE data.
+     */
+
+    if (features & (1 << 2)) {
+        /* YMM_Hi128 state */
+        void *buf = xsave_feature_buf(xs, 2);
+        for (i = 0; i < nvecregs; ++i) {
+            memcpy(&ri->vregs[i].q[2], buf + 16 * i, 16);
+        }
+    }
+
+    if (features & (1 << 5)) {
+        /* Opmask state */
+        uint64_t *buf = xsave_feature_buf(xs, 5);
+        for (i = 0; i < 8; ++i) {
+            ri->kregs[i] = buf[i];
+        }
+    }
+
+    if (features & (1 << 6)) {
+        /* ZMM_Hi256 state */
+        void *buf = xsave_feature_buf(xs, 6);
+        for (i = 0; i < nvecregs; ++i) {
+            memcpy(&ri->vregs[i].q[4], buf + 32 * i, 32);
+        }
+    }
+
+#ifdef __x86_64__
+    if (features & (1 << 7)) {
+        /* Hi16_ZMM state */
+        void *buf = xsave_feature_buf(xs, 7);
+        for (i = 0; i < 16; ++i) {
+            memcpy(&ri->vregs[i + 16], buf + 64 * i, 64);
+        }
+    }
+#endif
 }
 
 /* reginfo_is_eq: compare the reginfo structs, returns nonzero if equal */
 int reginfo_is_eq(struct reginfo *m, struct reginfo *a)
 {
-    return 0 == memcmp(m, a, sizeof(*m));
+    return !memcmp(m, a, sizeof(*m));
 }
 
 static const char *const regname[NGREG] = {
@@ -126,28 +240,126 @@ static const char *const regname[NGREG] = {
 # define PRIxREG   "%08x"
 #endif
 
+static int get_nvecregs(uint64_t features)
+{
+#ifdef __x86_64__
+    return features & (1 << 7) ? 32 : 16;
+#else
+    return 8;
+#endif
+}
+
+static int get_nvecquads(uint64_t features)
+{
+    if (features & (1 << 6)) {
+        return 8;
+    } else if (features & (1 << 2)) {
+        return 4;
+    } else {
+        return 2;
+    }
+}
+
+static char get_vecletter(uint64_t features)
+{
+    if (features & (1 << 6 | 1 << 7)) {
+        return 'z';
+    } else if (features & (1 << 2)) {
+        return 'y';
+    } else {
+        return 'x';
+    }
+}
+
 /* reginfo_dump: print state to a stream, returns nonzero on success */
 int reginfo_dump(struct reginfo *ri, FILE *f)
 {
-    int i;
+    uint64_t features;
+    int i, j, n, w;
+    char r;
+
     fprintf(f, "  faulting insn %x\n", ri->faulting_insn);
     for (i = 0; i < NGREG; i++) {
         if (regname[i]) {
             fprintf(f, "  %-6s: " PRIxREG "\n", regname[i], ri->gregs[i]);
         }
     }
+
+    fprintf(f, "  mxcsr : %x\n", ri->mxcsr);
+    fprintf(f, "  xfeat : %" PRIx64 "\n", ri->xfeatures);
+
+    features = ri->xfeatures;
+    n = get_nvecregs(features);
+    w = get_nvecquads(features);
+    r = get_vecletter(features);
+
+    for (i = 0; i < n; i++) {
+        fprintf(f, "  %cmm%-3d: ", r, i);
+        for (j = w - 1; j >= 0; j--) {
+            fprintf(f, "%016" PRIx64 "%c",
+                    ri->vregs[i].q[j], j == 0 ? '\n' : ' ');
+        }
+    }
+
+    if (features & (1 << 5)) {
+        for (i = 0; i < 8; i++) {
+            fprintf(f, "  k%-5d: %016" PRIx64 "\n", i, ri->kregs[i]);
+        }
+    }
+
     return !ferror(f);
 }
 
 int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
 {
-    int i;
+    int i, j, n, w;
+    uint64_t features;
+    char r;
+
+    fprintf(f, "Mismatch (master v apprentice):\n");
+
     for (i = 0; i < NGREG; i++) {
         if (m->gregs[i] != a->gregs[i]) {
             assert(regname[i]);
-            fprintf(f, "Mismatch: %s: " PRIxREG " v " PRIxREG "\n",
+            fprintf(f, "  %-6s: " PRIxREG " v " PRIxREG "\n",
                     regname[i], m->gregs[i], a->gregs[i]);
         }
     }
+
+    if (m->mxcsr != a->mxcsr) {
+        fprintf(f, "  mxcsr : %x v %x\n", m->mxcsr, a->mxcsr);
+    }
+    if (m->xfeatures != a->xfeatures) {
+        fprintf(f, "  xfeat : %" PRIx64 " v %" PRIx64 "\n",
+                m->xfeatures, a->xfeatures);
+    }
+
+    features = m->xfeatures;
+    n = get_nvecregs(features);
+    w = get_nvecquads(features);
+    r = get_vecletter(features);
+
+    for (i = 0; i < n; i++) {
+        if (memcmp(&m->vregs[i], &a->vregs[i], w * 8)) {
+            fprintf(f, "  %cmm%-3d: ", r, i);
+            for (j = w - 1; j >= 0; j--) {
+                fprintf(f, "%016" PRIx64 "%c",
+                        m->vregs[i].q[j], j == 0 ? '\n' : ' ');
+            }
+            fprintf(f, "       v: ");
+            for (j = w - 1; j >= 0; j--) {
+                fprintf(f, "%016" PRIx64 "%c",
+                        a->vregs[i].q[j], j == 0 ? '\n' : ' ');
+            }
+        }
+    }
+
+    for (i = 0; i < 8; i++) {
+        if (m->kregs[i] != a->kregs[i]) {
+            fprintf(f, "  k%-5d: %016" PRIx64 " v %016" PRIx64 "\n",
+                    i, m->kregs[i], a->kregs[i]);
+        }
+    }
+
     return !ferror(f);
 }
diff --git a/test_i386.S b/test_i386.S
index 456b99c..05344d7 100644
--- a/test_i386.S
+++ b/test_i386.S
@@ -12,6 +12,37 @@
 /* A trivial test image for x86 */
 
 /* Initialise the registers to avoid spurious mismatches */
+
+#ifdef __x86_64__
+#define BASE	%rax
+	lea	2f(%rip), BASE
+#else
+#define BASE	%eax
+	call	1f
+1:	pop	BASE
+	add	$2f-1b, BASE
+#endif
+
+	movdqa	0(BASE), %xmm0
+	movdqa	1*16(BASE), %xmm1
+	movdqa	2*16(BASE), %xmm2
+	movdqa	3*16(BASE), %xmm3
+	movdqa	4*16(BASE), %xmm4
+	movdqa	5*16(BASE), %xmm5
+	movdqa	6*16(BASE), %xmm6
+	movdqa	7*16(BASE), %xmm7
+
+#ifdef __x86_64__
+	movdqa	8*16(BASE), %xmm8
+	movdqa	9*16(BASE), %xmm9
+	movdqa	10*16(BASE), %xmm10
+	movdqa	11*16(BASE), %xmm11
+	movdqa	12*16(BASE), %xmm12
+	movdqa	13*16(BASE), %xmm13
+	movdqa	14*16(BASE), %xmm14
+	movdqa	15*16(BASE), %xmm15
+#endif
+
 	xor	%eax, %eax
 	sahf				/* init eflags */
 
@@ -39,3 +70,11 @@
 
 /* exit test */
 	ud1	%ecx, %eax
+
+	.p2align 16
+2:
+	.set	i, 0
+	.rept	256
+	.byte	i
+	.set	i, i + 1
+	.endr
-- 
2.20.1

[Qemu-devel] [RISU v3 10/11] risu_reginfo_i386: replace xfeature constants with symbolic names

[Jan Bobek]

The original code used "magic numbers", which made it unclear in
some places. Include a reference to the Intel manual where the
constants' meaning is discussed.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Jan Bobek <jan.bobek@gmail.com>
---
 risu_reginfo_i386.c | 48 +++++++++++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 15 deletions(-)

diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
index 83f9541..01ea179 100644
--- a/risu_reginfo_i386.c
+++ b/risu_reginfo_i386.c
@@ -22,7 +22,25 @@
 
 #include <asm/sigcontext.h>
 
-static uint64_t xfeatures = 3;  /* SSE */
+/*
+ * Refer to "Intel(R) 64 and IA-32 Architectures Software Developer's
+ * Manual", Volume 1, Section 13.1 "XSAVE-Supported Features and
+ * State-Component Bitmaps" for detailed discussion of these constants
+ * and their meaning.
+ */
+enum {
+    XFEAT_X87              = 1 << 0,
+    XFEAT_SSE              = 1 << 1,
+    XFEAT_AVX              = 1 << 2,
+    XFEAT_AVX512_OPMASK    = 1 << 5,
+    XFEAT_AVX512_ZMM_HI256 = 1 << 6,
+    XFEAT_AVX512_HI16_ZMM  = 1 << 7,
+    XFEAT_AVX512           = XFEAT_AVX512_OPMASK
+                           | XFEAT_AVX512_ZMM_HI256
+                           | XFEAT_AVX512_HI16_ZMM
+};
+
+static uint64_t xfeatures = XFEAT_X87 | XFEAT_SSE;
 
 static const struct option extra_ops[] = {
     {"xfeatures", required_argument, NULL, FIRST_ARCH_OPT },
@@ -160,34 +178,34 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
      * Now we know that _fpstate contains XSAVE data.
      */
 
-    if (features & (1 << 2)) {
+    if (features & XFEAT_AVX) {
         /* YMM_Hi128 state */
-        void *buf = xsave_feature_buf(xs, 2);
+        void *buf = xsave_feature_buf(xs, XFEAT_AVX);
         for (i = 0; i < nvecregs; ++i) {
             memcpy(&ri->vregs[i].q[2], buf + 16 * i, 16);
         }
     }
 
-    if (features & (1 << 5)) {
+    if (features & XFEAT_AVX512_OPMASK) {
         /* Opmask state */
-        uint64_t *buf = xsave_feature_buf(xs, 5);
+        uint64_t *buf = xsave_feature_buf(xs, XFEAT_AVX512_OPMASK);
         for (i = 0; i < 8; ++i) {
             ri->kregs[i] = buf[i];
         }
     }
 
-    if (features & (1 << 6)) {
+    if (features & XFEAT_AVX512_ZMM_HI256) {
         /* ZMM_Hi256 state */
-        void *buf = xsave_feature_buf(xs, 6);
+        void *buf = xsave_feature_buf(xs, XFEAT_AVX512_ZMM_HI256);
         for (i = 0; i < nvecregs; ++i) {
             memcpy(&ri->vregs[i].q[4], buf + 32 * i, 32);
         }
     }
 
 #ifdef __x86_64__
-    if (features & (1 << 7)) {
+    if (features & XFEAT_AVX512_HI16_ZMM) {
         /* Hi16_ZMM state */
-        void *buf = xsave_feature_buf(xs, 7);
+        void *buf = xsave_feature_buf(xs, XFEAT_AVX512_HI16_ZMM);
         for (i = 0; i < 16; ++i) {
             memcpy(&ri->vregs[i + 16], buf + 64 * i, 64);
         }
@@ -243,7 +261,7 @@ static const char *const regname[NGREG] = {
 static int get_nvecregs(uint64_t features)
 {
 #ifdef __x86_64__
-    return features & (1 << 7) ? 32 : 16;
+    return features & XFEAT_AVX512_HI16_ZMM ? 32 : 16;
 #else
     return 8;
 #endif
@@ -251,9 +269,9 @@ static int get_nvecregs(uint64_t features)
 
 static int get_nvecquads(uint64_t features)
 {
-    if (features & (1 << 6)) {
+    if (features & XFEAT_AVX512_ZMM_HI256) {
         return 8;
-    } else if (features & (1 << 2)) {
+    } else if (features & XFEAT_AVX) {
         return 4;
     } else {
         return 2;
@@ -262,9 +280,9 @@ static int get_nvecquads(uint64_t features)
 
 static char get_vecletter(uint64_t features)
 {
-    if (features & (1 << 6 | 1 << 7)) {
+    if (features & (XFEAT_AVX512_ZMM_HI256 | XFEAT_AVX512_HI16_ZMM)) {
         return 'z';
-    } else if (features & (1 << 2)) {
+    } else if (features & XFEAT_AVX) {
         return 'y';
     } else {
         return 'x';
@@ -301,7 +319,7 @@ int reginfo_dump(struct reginfo *ri, FILE *f)
         }
     }
 
-    if (features & (1 << 5)) {
+    if (features & XFEAT_AVX512_OPMASK) {
         for (i = 0; i < 8; i++) {
             fprintf(f, "  k%-5d: %016" PRIx64 "\n", i, ri->kregs[i]);
         }
-- 
2.20.1

[Qemu-devel] [RISU v3 11/11] risu_reginfo_i386: rework --xfeatures value parsing

[Jan Bobek]

Have the --xfeatures option accept "sse", "avx" and "avx512" in
addition to a plain numerical value, purely for users' convenience.
Don't fail silently when an incorrect value is specified, to avoid
confusion.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Jan Bobek <jan.bobek@gmail.com>
---
 risu_reginfo_i386.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
index 01ea179..194e0ad 100644
--- a/risu_reginfo_i386.c
+++ b/risu_reginfo_i386.c
@@ -53,8 +53,25 @@ const char * const arch_extra_help
 
 void process_arch_opt(int opt, const char *arg)
 {
+    char *endptr;
+
     assert(opt == FIRST_ARCH_OPT);
-    xfeatures = strtoull(arg, 0, 0);
+
+    if (!strcmp(arg, "sse")) {
+        xfeatures = XFEAT_X87 | XFEAT_SSE;
+    } else if (!strcmp(arg, "avx")) {
+        xfeatures = XFEAT_X87 | XFEAT_SSE | XFEAT_AVX;
+    } else if (!strcmp(arg, "avx512")) {
+        xfeatures = XFEAT_X87 | XFEAT_SSE | XFEAT_AVX | XFEAT_AVX512;
+    } else {
+        xfeatures = strtoull(arg, &endptr, 0);
+        if (*endptr) {
+            fprintf(stderr,
+                    "Unable to parse '%s' in '%s' into an xfeatures integer mask\n",
+                    endptr, arg);
+            exit(1);
+        }
+    }
 }
 
 const int reginfo_size(void)
-- 
2.20.1

[Qemu-devel] [RISU PATCH] build-all-arches: include x86 triplets in the build

[Alex Bennée]

There are a couple of minor warts:

  - 32 bit x86 can be either i386-linux-gnu or i686-linux-gnu
  - skip looking for x86_64-linux-gnu-gcc in docker cross envs

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 build-all-archs | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/build-all-archs b/build-all-archs
index a7cd7c2..e5dcfc8 100755
--- a/build-all-archs
+++ b/build-all-archs
@@ -39,7 +39,7 @@ while [[ "$1" = -* ]]; do
             ;;
         --use-docker)
             if [ -z "$arg" ]; then
-                default_tags=$(docker images qemu --format "{{.Repository}}:{{.Tag}}" | grep "\(arm\|ppc64el\|m68k\).*cross$")
+                default_tags=$(docker images qemu --format "{{.Repository}}:{{.Tag}}" | grep "\(arm\|ppc64el\|m68k\|i386\).*cross$")
                 docker_tags=$(echo $default_tags | sed 's/\n/\s/g' )
             else
                 docker_tags="$arg"
@@ -74,7 +74,7 @@ fi
 DOCKER_RUN="docker run --rm -u $(id -u) -v $(pwd):$(pwd) -w $(pwd)"
 
 program_exists() {
-    if [ ! -z "$docker_tags" ]; then
+    if [[ ! -z "$docker_tags" && ! "$1" == "x86_64-linux-gnu-gcc" ]]; then
         use_docker_tag=""
         for tag in $docker_tags; do
             if ${DOCKER_RUN} ${tag} /bin/bash -c "command -v $1 >/dev/null"; then
@@ -88,8 +88,10 @@ program_exists() {
 }
 
 # powerpc64-linux-gnu doesn't work at the moment, so not yet listed.
-for triplet in aarch64-linux-gnu arm-linux-gnueabihf m68k-linux-gnu \
-    powerpc64le-linux-gnu powerpc64-linux-gnu ; do
+for triplet in i386-linux-gnu i686-linux-gnu x86_64-linux-gnu \
+                   aarch64-linux-gnu arm-linux-gnueabihf \
+                   m68k-linux-gnu \
+                   powerpc64le-linux-gnu powerpc64-linux-gnu ; do
 
     if ! program_exists "${triplet}-gcc"; then
         echo "Skipping ${triplet}: no compiler found"
-- 
2.20.1

Re: [Qemu-devel] [RISU v3 07/11] test_i386: change syntax from nasm to gas

[Alex Bennée]

Jan Bobek <jan.bobek@gmail.com> writes:

> This allows us to drop dependency on NASM and build the test image
> with GCC only. Adds support for x86_64, too.
>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Jan Bobek <jan.bobek@gmail.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  Makefile    |  3 +++
>  test_i386.S | 41 +++++++++++++++++++++++++++++++++++++++++
>  test_i386.s | 27 ---------------------------
>  3 files changed, 44 insertions(+), 27 deletions(-)
>  create mode 100644 test_i386.S
>  delete mode 100644 test_i386.s
>
> diff --git a/Makefile b/Makefile
> index b362dbe..6ab014a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -49,6 +49,9 @@ $(PROG): $(OBJS)
>  %_$(ARCH).elf: %_$(ARCH).s
>  	$(AS) -o $@ $<
>
> +%_$(ARCH).elf: %_$(ARCH).S
> +	$(CC) $(CPPFLAGS) -o $@ -c $<
> +
>  clean:
>  	rm -f $(PROG) $(OBJS) $(BINS)
>
> diff --git a/test_i386.S b/test_i386.S
> new file mode 100644
> index 0000000..456b99c
> --- /dev/null
> +++ b/test_i386.S
> @@ -0,0 +1,41 @@
> +/*#############################################################################
> + * Copyright (c) 2010 Linaro Limited
> + * All rights reserved. This program and the accompanying materials
> + * are made available under the terms of the Eclipse Public License v1.0
> + * which accompanies this distribution, and is available at
> + * http://www.eclipse.org/legal/epl-v10.html
> + *
> + * Contributors:
> + *     Peter Maydell (Linaro) - initial implementation
> + *###########################################################################*/
> +
> +/* A trivial test image for x86 */
> +
> +/* Initialise the registers to avoid spurious mismatches */
> +	xor	%eax, %eax
> +	sahf				/* init eflags */
> +
> +	mov	$0x12345678, %eax
> +	mov	$0x9abcdef0, %ebx
> +	mov	$0x97361234, %ecx
> +	mov	$0x84310284, %edx
> +	mov	$0x83624173, %edi
> +	mov	$0xfaebfaeb, %esi
> +	mov	$0x84610123, %ebp
> +
> +#ifdef __x86_64__
> +	movq	$0x123456789abcdef0, %r8
> +	movq	$0xaaaabbbbccccdddd, %r9
> +	movq	$0x1010101010101010, %r10
> +	movq	$0x1111111111111111, %r11
> +	movq	$0x1212121212121212, %r12
> +	movq	$0x1313131313131313, %r13
> +	movq	$0x1414141414141414, %r14
> +	movq	$0x1515151515151515, %r15
> +#endif
> +
> +/* do compare */
> +	ud1	%eax, %eax
> +
> +/* exit test */
> +	ud1	%ecx, %eax
> diff --git a/test_i386.s b/test_i386.s
> deleted file mode 100644
> index a2140a0..0000000
> --- a/test_i386.s
> +++ /dev/null
> @@ -1,27 +0,0 @@
> -;###############################################################################
> -;# Copyright (c) 2010 Linaro Limited
> -;# All rights reserved. This program and the accompanying materials
> -;# are made available under the terms of the Eclipse Public License v1.0
> -;# which accompanies this distribution, and is available at
> -;# http://www.eclipse.org/legal/epl-v10.html
> -;#
> -;# Contributors:
> -;#     Peter Maydell (Linaro) - initial implementation
> -;###############################################################################
> -
> -; A trivial test image for x86
> -
> -BITS 32
> -; Initialise the registers to avoid spurious mismatches
> -mov eax, 0x12345678
> -mov ebx, 0x9abcdef0
> -mov ecx, 0x97361234
> -mov edx, 0x84310284
> -mov edi, 0x83624173
> -mov esi, 0xfaebfaeb
> -mov ebp, 0x84610123
> -; UD1 : do compare
> -UD1
> -
> -; UD2 : exit test
> -UD2


--
Alex Bennée

Re: [Qemu-devel] [RISU v3 09/11] i386: Add avx512 state to reginfo_t

[Alex Bennée]

Jan Bobek <jan.bobek@gmail.com> writes:

> From: Richard Henderson <richard.henderson@linaro.org>
>
> The state expected for a given test must be specifically requested
> with the --xfeatures=mask command-line argument.  This is recorded
> with the saved state so that it is obvious if the apprentice is given
> a different argument.  Any features beyond what are present on the
> running cpu will read as zero.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


> ---
>  risu_reginfo_i386.h |  14 +++
>  risu_reginfo_i386.c | 228 ++++++++++++++++++++++++++++++++++++++++++--
>  test_i386.S         |  39 ++++++++
>  3 files changed, 273 insertions(+), 8 deletions(-)
>
> diff --git a/risu_reginfo_i386.h b/risu_reginfo_i386.h
> index e350f01..b468f79 100644
> --- a/risu_reginfo_i386.h
> +++ b/risu_reginfo_i386.h
> @@ -12,6 +12,10 @@
>  #ifndef RISU_REGINFO_I386_H
>  #define RISU_REGINFO_I386_H
>
> +struct avx512_reg {
> +    uint64_t q[8];
> +};
> +
>  /*
>   * This is the data structure we pass over the socket.
>   * It is a simplified and reduced subset of what can
> @@ -19,7 +23,17 @@
>   */
>  struct reginfo {
>      uint32_t faulting_insn;
> +    uint32_t mxcsr;
> +    uint64_t xfeatures;
> +
>      gregset_t gregs;
> +
> +#ifdef __x86_64__
> +    struct avx512_reg vregs[32];
> +#else
> +    struct avx512_reg vregs[8];
> +#endif
> +    uint64_t kregs[8];
>  };
>
>  /*
> diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
> index c4dc14a..83f9541 100644
> --- a/risu_reginfo_i386.c
> +++ b/risu_reginfo_i386.c
> @@ -11,19 +11,32 @@
>
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <stddef.h>
>  #include <string.h>
>  #include <ucontext.h>
>  #include <assert.h>
> +#include <cpuid.h>
>
>  #include "risu.h"
>  #include "risu_reginfo_i386.h"
>
> -const struct option * const arch_long_opts;
> -const char * const arch_extra_help;
> +#include <asm/sigcontext.h>
> +
> +static uint64_t xfeatures = 3;  /* SSE */
> +
> +static const struct option extra_ops[] = {
> +    {"xfeatures", required_argument, NULL, FIRST_ARCH_OPT },
> +    {0, 0, 0, 0}
> +};
> +
> +const struct option * const arch_long_opts = extra_ops;
> +const char * const arch_extra_help
> +    = "  --xfeatures=<mask>  Use features in mask for XSAVE\n";
>
>  void process_arch_opt(int opt, const char *arg)
>  {
> -    abort();
> +    assert(opt == FIRST_ARCH_OPT);
> +    xfeatures = strtoull(arg, 0, 0);
>  }
>
>  const int reginfo_size(void)
> @@ -31,13 +44,37 @@ const int reginfo_size(void)
>      return sizeof(struct reginfo);
>  }
>
> +static void *xsave_feature_buf(struct _xstate *xs, int feature)
> +{
> +    unsigned int eax, ebx, ecx, edx;
> +    int ok;
> +
> +    /*
> +     * Get the location of the XSAVE feature from the cpuid leaf.
> +     * Given that we know the xfeature bit is set, this must succeed.
> +     */
> +    ok = __get_cpuid_count(0xd, feature, &eax, &ebx, &ecx, &edx);
> +    assert(ok);
> +
> +    /* Sanity check that the frame stored by the kernel contains the data. */
> +    assert(xs->fpstate.sw_reserved.extended_size >= eax + ebx);
> +
> +    return (void *)xs + ebx;
> +}
> +
>  /* reginfo_init: initialize with a ucontext */
>  void reginfo_init(struct reginfo *ri, ucontext_t *uc)
>  {
> -    int i;
> +    int i, nvecregs;
> +    struct _fpstate *fp;
> +    struct _xstate *xs;
> +    uint64_t features;
>
>      memset(ri, 0, sizeof(*ri));
>
> +    /* Require master and apprentice to be given the same arguments.  */
> +    ri->xfeatures = xfeatures;
> +
>      for (i = 0; i < NGREG; i++) {
>          switch (i) {
>          case REG_E(IP):
> @@ -79,12 +116,89 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
>       * distinguish 'do compare' from 'stop'.
>       */
>      ri->faulting_insn = *(uint32_t *)uc->uc_mcontext.gregs[REG_E(IP)];
> +
> +    /*
> +     * FP state is omitted if unused (aka in init state).
> +     * Use the <asm/sigcontext.h> struct for access to AVX state.
> +     */
> +
> +    fp = (struct _fpstate *)uc->uc_mcontext.fpregs;
> +    if (fp == NULL) {
> +        return;
> +    }
> +
> +#ifdef __x86_64__
> +    nvecregs = 16;
> +#else
> +    /* We don't (currently) care about the 80387 state, only SSE+.  */
> +    if (fp->magic != X86_FXSR_MAGIC) {
> +        return;
> +    }
> +    nvecregs = 8;
> +#endif
> +
> +    /*
> +     * Now we know that _fpstate contains FXSAVE data.
> +     */
> +    ri->mxcsr = fp->mxcsr;
> +
> +    for (i = 0; i < nvecregs; ++i) {
> +#ifdef __x86_64__
> +        memcpy(&ri->vregs[i], &fp->xmm_space[i * 4], 16);
> +#else
> +        memcpy(&ri->vregs[i], &fp->_xmm[i], 16);
> +#endif
> +    }
> +
> +    if (fp->sw_reserved.magic1 != FP_XSTATE_MAGIC1) {
> +        return;
> +    }
> +    xs = (struct _xstate *)fp;
> +    features = xfeatures & xs->xstate_hdr.xfeatures;
> +
> +    /*
> +     * Now we know that _fpstate contains XSAVE data.
> +     */
> +
> +    if (features & (1 << 2)) {
> +        /* YMM_Hi128 state */
> +        void *buf = xsave_feature_buf(xs, 2);
> +        for (i = 0; i < nvecregs; ++i) {
> +            memcpy(&ri->vregs[i].q[2], buf + 16 * i, 16);
> +        }
> +    }
> +
> +    if (features & (1 << 5)) {
> +        /* Opmask state */
> +        uint64_t *buf = xsave_feature_buf(xs, 5);
> +        for (i = 0; i < 8; ++i) {
> +            ri->kregs[i] = buf[i];
> +        }
> +    }
> +
> +    if (features & (1 << 6)) {
> +        /* ZMM_Hi256 state */
> +        void *buf = xsave_feature_buf(xs, 6);
> +        for (i = 0; i < nvecregs; ++i) {
> +            memcpy(&ri->vregs[i].q[4], buf + 32 * i, 32);
> +        }
> +    }
> +
> +#ifdef __x86_64__
> +    if (features & (1 << 7)) {
> +        /* Hi16_ZMM state */
> +        void *buf = xsave_feature_buf(xs, 7);
> +        for (i = 0; i < 16; ++i) {
> +            memcpy(&ri->vregs[i + 16], buf + 64 * i, 64);
> +        }
> +    }
> +#endif
>  }
>
>  /* reginfo_is_eq: compare the reginfo structs, returns nonzero if equal */
>  int reginfo_is_eq(struct reginfo *m, struct reginfo *a)
>  {
> -    return 0 == memcmp(m, a, sizeof(*m));
> +    return !memcmp(m, a, sizeof(*m));
>  }
>
>  static const char *const regname[NGREG] = {
> @@ -126,28 +240,126 @@ static const char *const regname[NGREG] = {
>  # define PRIxREG   "%08x"
>  #endif
>
> +static int get_nvecregs(uint64_t features)
> +{
> +#ifdef __x86_64__
> +    return features & (1 << 7) ? 32 : 16;
> +#else
> +    return 8;
> +#endif
> +}
> +
> +static int get_nvecquads(uint64_t features)
> +{
> +    if (features & (1 << 6)) {
> +        return 8;
> +    } else if (features & (1 << 2)) {
> +        return 4;
> +    } else {
> +        return 2;
> +    }
> +}
> +
> +static char get_vecletter(uint64_t features)
> +{
> +    if (features & (1 << 6 | 1 << 7)) {
> +        return 'z';
> +    } else if (features & (1 << 2)) {
> +        return 'y';
> +    } else {
> +        return 'x';
> +    }
> +}
> +
>  /* reginfo_dump: print state to a stream, returns nonzero on success */
>  int reginfo_dump(struct reginfo *ri, FILE *f)
>  {
> -    int i;
> +    uint64_t features;
> +    int i, j, n, w;
> +    char r;
> +
>      fprintf(f, "  faulting insn %x\n", ri->faulting_insn);
>      for (i = 0; i < NGREG; i++) {
>          if (regname[i]) {
>              fprintf(f, "  %-6s: " PRIxREG "\n", regname[i], ri->gregs[i]);
>          }
>      }
> +
> +    fprintf(f, "  mxcsr : %x\n", ri->mxcsr);
> +    fprintf(f, "  xfeat : %" PRIx64 "\n", ri->xfeatures);
> +
> +    features = ri->xfeatures;
> +    n = get_nvecregs(features);
> +    w = get_nvecquads(features);
> +    r = get_vecletter(features);
> +
> +    for (i = 0; i < n; i++) {
> +        fprintf(f, "  %cmm%-3d: ", r, i);
> +        for (j = w - 1; j >= 0; j--) {
> +            fprintf(f, "%016" PRIx64 "%c",
> +                    ri->vregs[i].q[j], j == 0 ? '\n' : ' ');
> +        }
> +    }
> +
> +    if (features & (1 << 5)) {
> +        for (i = 0; i < 8; i++) {
> +            fprintf(f, "  k%-5d: %016" PRIx64 "\n", i, ri->kregs[i]);
> +        }
> +    }
> +
>      return !ferror(f);
>  }
>
>  int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
>  {
> -    int i;
> +    int i, j, n, w;
> +    uint64_t features;
> +    char r;
> +
> +    fprintf(f, "Mismatch (master v apprentice):\n");
> +
>      for (i = 0; i < NGREG; i++) {
>          if (m->gregs[i] != a->gregs[i]) {
>              assert(regname[i]);
> -            fprintf(f, "Mismatch: %s: " PRIxREG " v " PRIxREG "\n",
> +            fprintf(f, "  %-6s: " PRIxREG " v " PRIxREG "\n",
>                      regname[i], m->gregs[i], a->gregs[i]);
>          }
>      }
> +
> +    if (m->mxcsr != a->mxcsr) {
> +        fprintf(f, "  mxcsr : %x v %x\n", m->mxcsr, a->mxcsr);
> +    }
> +    if (m->xfeatures != a->xfeatures) {
> +        fprintf(f, "  xfeat : %" PRIx64 " v %" PRIx64 "\n",
> +                m->xfeatures, a->xfeatures);
> +    }
> +
> +    features = m->xfeatures;
> +    n = get_nvecregs(features);
> +    w = get_nvecquads(features);
> +    r = get_vecletter(features);
> +
> +    for (i = 0; i < n; i++) {
> +        if (memcmp(&m->vregs[i], &a->vregs[i], w * 8)) {
> +            fprintf(f, "  %cmm%-3d: ", r, i);
> +            for (j = w - 1; j >= 0; j--) {
> +                fprintf(f, "%016" PRIx64 "%c",
> +                        m->vregs[i].q[j], j == 0 ? '\n' : ' ');
> +            }
> +            fprintf(f, "       v: ");
> +            for (j = w - 1; j >= 0; j--) {
> +                fprintf(f, "%016" PRIx64 "%c",
> +                        a->vregs[i].q[j], j == 0 ? '\n' : ' ');
> +            }
> +        }
> +    }
> +
> +    for (i = 0; i < 8; i++) {
> +        if (m->kregs[i] != a->kregs[i]) {
> +            fprintf(f, "  k%-5d: %016" PRIx64 " v %016" PRIx64 "\n",
> +                    i, m->kregs[i], a->kregs[i]);
> +        }
> +    }
> +
>      return !ferror(f);
>  }
> diff --git a/test_i386.S b/test_i386.S
> index 456b99c..05344d7 100644
> --- a/test_i386.S
> +++ b/test_i386.S
> @@ -12,6 +12,37 @@
>  /* A trivial test image for x86 */
>
>  /* Initialise the registers to avoid spurious mismatches */
> +
> +#ifdef __x86_64__
> +#define BASE	%rax
> +	lea	2f(%rip), BASE
> +#else
> +#define BASE	%eax
> +	call	1f
> +1:	pop	BASE
> +	add	$2f-1b, BASE
> +#endif
> +
> +	movdqa	0(BASE), %xmm0
> +	movdqa	1*16(BASE), %xmm1
> +	movdqa	2*16(BASE), %xmm2
> +	movdqa	3*16(BASE), %xmm3
> +	movdqa	4*16(BASE), %xmm4
> +	movdqa	5*16(BASE), %xmm5
> +	movdqa	6*16(BASE), %xmm6
> +	movdqa	7*16(BASE), %xmm7
> +
> +#ifdef __x86_64__
> +	movdqa	8*16(BASE), %xmm8
> +	movdqa	9*16(BASE), %xmm9
> +	movdqa	10*16(BASE), %xmm10
> +	movdqa	11*16(BASE), %xmm11
> +	movdqa	12*16(BASE), %xmm12
> +	movdqa	13*16(BASE), %xmm13
> +	movdqa	14*16(BASE), %xmm14
> +	movdqa	15*16(BASE), %xmm15
> +#endif
> +
>  	xor	%eax, %eax
>  	sahf				/* init eflags */
>
> @@ -39,3 +70,11 @@
>
>  /* exit test */
>  	ud1	%ecx, %eax
> +
> +	.p2align 16
> +2:
> +	.set	i, 0
> +	.rept	256
> +	.byte	i
> +	.set	i, i + 1
> +	.endr


--
Alex Bennée

Re: [Qemu-devel] [RISU v3 11/11] risu_reginfo_i386: rework --xfeatures value parsing

[Alex Bennée]

Jan Bobek <jan.bobek@gmail.com> writes:

> Have the --xfeatures option accept "sse", "avx" and "avx512" in
> addition to a plain numerical value, purely for users' convenience.
> Don't fail silently when an incorrect value is specified, to avoid
> confusion.
>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Jan Bobek <jan.bobek@gmail.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>


> ---
>  risu_reginfo_i386.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/risu_reginfo_i386.c b/risu_reginfo_i386.c
> index 01ea179..194e0ad 100644
> --- a/risu_reginfo_i386.c
> +++ b/risu_reginfo_i386.c
> @@ -53,8 +53,25 @@ const char * const arch_extra_help
>
>  void process_arch_opt(int opt, const char *arg)
>  {
> +    char *endptr;
> +
>      assert(opt == FIRST_ARCH_OPT);
> -    xfeatures = strtoull(arg, 0, 0);
> +
> +    if (!strcmp(arg, "sse")) {
> +        xfeatures = XFEAT_X87 | XFEAT_SSE;
> +    } else if (!strcmp(arg, "avx")) {
> +        xfeatures = XFEAT_X87 | XFEAT_SSE | XFEAT_AVX;
> +    } else if (!strcmp(arg, "avx512")) {
> +        xfeatures = XFEAT_X87 | XFEAT_SSE | XFEAT_AVX | XFEAT_AVX512;
> +    } else {
> +        xfeatures = strtoull(arg, &endptr, 0);
> +        if (*endptr) {
> +            fprintf(stderr,
> +                    "Unable to parse '%s' in '%s' into an xfeatures integer mask\n",
> +                    endptr, arg);
> +            exit(1);
> +        }
> +    }
>  }
>
>  const int reginfo_size(void)


--
Alex Bennée

Re: [Qemu-devel] [RISU v3 00/11] Support for i386/x86_64 with vector extensions

[Alex Bennée]

Jan Bobek <jan.bobek@gmail.com> writes:

> This patch series adds support for i386 and x86_64 architectures to
> RISU. Notably, vector registers (SSE, AVX, AVX-512) are supported for
> verification of the apprentice. This is V3 of the series posted in [1]
> and [2].

I've sent a patch to enable x86 in the build-all-arches tests script but
otherwise I think this series looks good to merge.

>
> Changes is V3:
>   - fix a matching bug caused by misplaced index multiplication [3]
>   - print an error and exit when parse of the --xfeatures option fails [4]
>
> References:
>   1. https://lists.nongnu.org/archive/html/qemu-devel/2019-04/msg01294.html
>   2. https://lists.nongnu.org/archive/html/qemu-devel/2019-05/msg04089.html
>   3. https://lists.nongnu.org/archive/html/qemu-devel/2019-05/msg04922.html
>   4. https://lists.nongnu.org/archive/html/qemu-devel/2019-05/msg04903.html
>
> Jan Bobek (10):
>   Makefile: undefine the arch name symbol
>   risu_i386: move reginfo_t and related defines to risu_reginfo_i386.h
>   risu_i386: move reginfo-related code to risu_reginfo_i386.c
>   risu_reginfo_i386: implement arch-specific reginfo interface
>   risu_i386: implement missing CPU-specific functions
>   risu_i386: remove old unused code
>   test_i386: change syntax from nasm to gas
>   configure: add i386/x86_64 architectures
>   risu_reginfo_i386: replace xfeature constants with symbolic names
>   risu_reginfo_i386: rework --xfeatures value parsing
>
> Richard Henderson (1):
>   i386: Add avx512 state to reginfo_t
>
>  configure           |  10 +-
>  Makefile            |   5 +-
>  risu_reginfo_i386.h |  49 ++++++
>  risu_i386.c         | 142 ++--------------
>  risu_reginfo_i386.c | 400 ++++++++++++++++++++++++++++++++++++++++++++
>  test_i386.S         |  80 +++++++++
>  test_i386.s         |  27 ---
>  7 files changed, 556 insertions(+), 157 deletions(-)
>  create mode 100644 risu_reginfo_i386.h
>  create mode 100644 risu_reginfo_i386.c
>  create mode 100644 test_i386.S
>  delete mode 100644 test_i386.s


--
Alex Bennée

Re: [Qemu-devel] [RISU v3 00/11] Support for i386/x86_64 with vector extensions

[Peter Maydell]

On Fri, 24 May 2019 at 10:42, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Jan Bobek <jan.bobek@gmail.com> writes:
>
> > This patch series adds support for i386 and x86_64 architectures to
> > RISU. Notably, vector registers (SSE, AVX, AVX-512) are supported for
> > verification of the apprentice. This is V3 of the series posted in [1]
> > and [2].
>
> I've sent a patch to enable x86 in the build-all-arches tests script but
> otherwise I think this series looks good to merge.

Alex: So should I merge this series, or does it need a respin ?

thanks
-- PMM

Re: [Qemu-devel] [RISU v3 00/11] Support for i386/x86_64 with vector extensions

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Fri, 24 May 2019 at 10:42, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>>
>> Jan Bobek <jan.bobek@gmail.com> writes:
>>
>> > This patch series adds support for i386 and x86_64 architectures to
>> > RISU. Notably, vector registers (SSE, AVX, AVX-512) are supported for
>> > verification of the apprentice. This is V3 of the series posted in [1]
>> > and [2].
>>
>> I've sent a patch to enable x86 in the build-all-arches tests script but
>> otherwise I think this series looks good to merge.
>
> Alex: So should I merge this series, or does it need a respin ?

I think this can be merged. I assume there is more to come to actually
generate the patterns but it doesn't break any of the existing stuff and
has at least one test case.

--
Alex Bennée

Re: [Qemu-devel] [RISU v3 00/11] Support for i386/x86_64 with vector extensions

[Peter Maydell]

On Fri, 7 Jun 2019 at 12:58, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Fri, 24 May 2019 at 10:42, Alex Bennée <alex.bennee@linaro.org> wrote:
> >>
> >>
> >> Jan Bobek <jan.bobek@gmail.com> writes:
> >>
> >> > This patch series adds support for i386 and x86_64 architectures to
> >> > RISU. Notably, vector registers (SSE, AVX, AVX-512) are supported for
> >> > verification of the apprentice. This is V3 of the series posted in [1]
> >> > and [2].
> >>
> >> I've sent a patch to enable x86 in the build-all-arches tests script but
> >> otherwise I think this series looks good to merge.
> >
> > Alex: So should I merge this series, or does it need a respin ?
>
> I think this can be merged. I assume there is more to come to actually
> generate the patterns but it doesn't break any of the existing stuff and
> has at least one test case.

Thanks; pushed this series and your patch to add it to the
build-all-archs script to risu master.

-- PMM