All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension
@ 2015-07-09  8:17 Richard Henderson
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 01/14] target-i386: Split fxsave/fxrstor implementation Richard Henderson
                   ` (14 more replies)
  0 siblings, 15 replies; 31+ messages in thread
From: Richard Henderson @ 2015-07-09  8:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, ehabkost

I'm still in the process of testing this, as there's no code
written for it yet and hardware to compare against doesn't
start shipping until (probably) August.

But in the meantime there are a number of holes that I found
in XSAVE support that might affect KVM, and one question wrt
SMM support that affects MPX.  So I thought I'd get some 
feedback on this sooner than later.

This patch set depends on the addressing cleanup patchset that
I just posted.  It ought to depend on Pavel Dovgalyuk's exception
handling cleanup patchset, but I haven't included that in my tree.

Comments?


r~


Richard Henderson (14):
  target-i386: Split fxsave/fxrstor implementation
  target-i386: Rearrange processing of 0F 01
  target-i386: Add XSAVE extension
  target-i386: Implement XSAVEOPT
  target-i386: Enable control registers for MPX
  target-i386: Perform set/reset_inhibit_irq inline
  target-i386: Split up gen_lea_modrm
  target-i386: Implement BNDMK
  target-i386: Implement BNDMOV
  target-i386: Implement BNDCL, BNDCU, BNDCN
  target-i386: Update BNDSTATUS for exceptions raised by BOUND
  target-i386: Implement BNDLDX, BNDSTX
  target-i386: Clear bndregs during legacy near jumps
  target-i386: Enable XCR0 features for user-mode

 target-i386/Makefile.objs |    2 +-
 target-i386/cc_helper.c   |   10 -
 target-i386/cpu.c         |   81 ++--
 target-i386/cpu.h         |   23 +-
 target-i386/fpu_helper.c  |  354 ++++++++++++---
 target-i386/helper.c      |   17 +-
 target-i386/helper.h      |   17 +-
 target-i386/kvm.c         |   21 +-
 target-i386/mem_helper.c  |    6 +
 target-i386/misc_helper.c |    9 +
 target-i386/mpx_helper.c  |  152 +++++++
 target-i386/smm_helper.c  |    4 +
 target-i386/translate.c   | 1063 ++++++++++++++++++++++++++++++---------------
 13 files changed, 1297 insertions(+), 462 deletions(-)
 create mode 100644 target-i386/mpx_helper.c

-- 
2.4.3

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

* [Qemu-devel] [PATCH 01/14] target-i386: Split fxsave/fxrstor implementation
  2015-07-09  8:17 [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Richard Henderson
@ 2015-07-09  8:17 ` Richard Henderson
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 02/14] target-i386: Rearrange processing of 0F 01 Richard Henderson
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2015-07-09  8:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, ehabkost

We will be able to reuse these pieces for XSAVE/XRSTOR.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-i386/fpu_helper.c | 145 ++++++++++++++++++++++++++++-------------------
 target-i386/helper.h     |   4 +-
 target-i386/translate.c  |   4 +-
 3 files changed, 90 insertions(+), 63 deletions(-)

diff --git a/target-i386/fpu_helper.c b/target-i386/fpu_helper.c
index 30d34d5..1d72cd1 100644
--- a/target-i386/fpu_helper.c
+++ b/target-i386/fpu_helper.c
@@ -1101,17 +1101,11 @@ void cpu_x86_frstor(CPUX86State *env, target_ulong ptr, int data32)
 }
 #endif
 
-void helper_fxsave(CPUX86State *env, target_ulong ptr, int data64)
+static void do_xsave_fpu(CPUX86State *env, target_ulong ptr)
 {
-    int fpus, fptag, i, nb_xmm_regs;
-    floatx80 tmp;
+    int fpus, fptag, i;
     target_ulong addr;
 
-    /* The operand must be 16 byte aligned */
-    if (ptr & 0xf) {
-        raise_exception(env, EXCP0D_GPF);
-    }
-
     fpus = (env->fpus & ~0x3800) | (env->fpstt & 0x7) << 11;
     fptag = 0;
     for (i = 0; i < 8; i++) {
@@ -1120,60 +1114,71 @@ void helper_fxsave(CPUX86State *env, target_ulong ptr, int data64)
     cpu_stw_data(env, ptr, env->fpuc);
     cpu_stw_data(env, ptr + 2, fpus);
     cpu_stw_data(env, ptr + 4, fptag ^ 0xff);
-#ifdef TARGET_X86_64
-    if (data64) {
-        cpu_stq_data(env, ptr + 0x08, 0); /* rip */
-        cpu_stq_data(env, ptr + 0x10, 0); /* rdp */
-    } else
-#endif
-    {
-        cpu_stl_data(env, ptr + 0x08, 0); /* eip */
-        cpu_stl_data(env, ptr + 0x0c, 0); /* sel  */
-        cpu_stl_data(env, ptr + 0x10, 0); /* dp */
-        cpu_stl_data(env, ptr + 0x14, 0); /* sel  */
-    }
+
+    /* In 32-bit mode this is eip, sel, dp, sel.
+       In 64-bit mode this is rip, rdp.
+       But in either case we don't write actual data, just zeros.  */
+    cpu_stq_data(env, ptr + 0x08, 0); /* eip+sel; rip */
+    cpu_stq_data(env, ptr + 0x10, 0); /* edp+sel; rdp */
 
     addr = ptr + 0x20;
     for (i = 0; i < 8; i++) {
-        tmp = ST(i);
+        floatx80 tmp = ST(i);
         helper_fstt(env, tmp, addr);
         addr += 16;
     }
+}
+
+static void do_xsave_mxcsr(CPUX86State *env, target_ulong ptr)
+{
+    cpu_stl_data(env, ptr + 0x18, env->mxcsr); /* mxcsr */
+    cpu_stl_data(env, ptr + 0x1c, 0x0000ffff); /* mxcsr_mask */
+}
+
+static void do_xsave_sse(CPUX86State *env, target_ulong ptr)
+{
+    int i, nb_xmm_regs;
+    target_ulong addr;
+
+    if (env->hflags & HF_CS64_MASK) {
+        nb_xmm_regs = 16;
+    } else {
+        nb_xmm_regs = 8;
+    }
+
+    addr = ptr + 0xa0;
+    for (i = 0; i < nb_xmm_regs; i++) {
+        cpu_stq_data(env, addr, env->xmm_regs[i].XMM_Q(0));
+        cpu_stq_data(env, addr + 8, env->xmm_regs[i].XMM_Q(1));
+        addr += 16;
+    }
+}
+
+void helper_fxsave(CPUX86State *env, target_ulong ptr)
+{
+    /* The operand must be 16 byte aligned */
+    if (ptr & 0xf) {
+        raise_exception(env, EXCP0D_GPF);
+    }
+
+    do_xsave_fpu(env, ptr);
 
     if (env->cr[4] & CR4_OSFXSR_MASK) {
-        /* XXX: finish it */
-        cpu_stl_data(env, ptr + 0x18, env->mxcsr); /* mxcsr */
-        cpu_stl_data(env, ptr + 0x1c, 0x0000ffff); /* mxcsr_mask */
-        if (env->hflags & HF_CS64_MASK) {
-            nb_xmm_regs = 16;
-        } else {
-            nb_xmm_regs = 8;
-        }
-        addr = ptr + 0xa0;
+        do_xsave_mxcsr(env, ptr);
         /* Fast FXSAVE leaves out the XMM registers */
         if (!(env->efer & MSR_EFER_FFXSR)
             || (env->hflags & HF_CPL_MASK)
             || !(env->hflags & HF_LMA_MASK)) {
-            for (i = 0; i < nb_xmm_regs; i++) {
-                cpu_stq_data(env, addr, env->xmm_regs[i].XMM_Q(0));
-                cpu_stq_data(env, addr + 8, env->xmm_regs[i].XMM_Q(1));
-                addr += 16;
-            }
+            do_xsave_sse(env, ptr);
         }
     }
 }
 
-void helper_fxrstor(CPUX86State *env, target_ulong ptr, int data64)
+static void do_xrstor_fpu(CPUX86State *env, target_ulong ptr)
 {
-    int i, fpus, fptag, nb_xmm_regs;
-    floatx80 tmp;
+    int i, fpus, fptag;
     target_ulong addr;
 
-    /* The operand must be 16 byte aligned */
-    if (ptr & 0xf) {
-        raise_exception(env, EXCP0D_GPF);
-    }
-
     cpu_set_fpuc(env, cpu_lduw_data(env, ptr));
     fpus = cpu_lduw_data(env, ptr + 2);
     fptag = cpu_lduw_data(env, ptr + 4);
@@ -1186,30 +1191,52 @@ void helper_fxrstor(CPUX86State *env, target_ulong ptr, int data64)
 
     addr = ptr + 0x20;
     for (i = 0; i < 8; i++) {
-        tmp = helper_fldt(env, addr);
+        floatx80 tmp = helper_fldt(env, addr);
         ST(i) = tmp;
         addr += 16;
     }
+}
+
+static void do_xrstor_mxcsr(CPUX86State *env, target_ulong ptr)
+{
+    cpu_set_mxcsr(env, cpu_ldl_data(env, ptr + 0x18));
+}
+
+static void do_xrstor_sse(CPUX86State *env, target_ulong ptr)
+{
+    int i, nb_xmm_regs;
+    target_ulong addr;
+
+    if (env->hflags & HF_CS64_MASK) {
+        nb_xmm_regs = 16;
+    } else {
+        nb_xmm_regs = 8;
+    }
+
+    addr = ptr + 0xa0;
+    for (i = 0; i < nb_xmm_regs; i++) {
+        env->xmm_regs[i].XMM_Q(0) = cpu_ldq_data(env, addr);
+        env->xmm_regs[i].XMM_Q(1) = cpu_ldq_data(env, addr + 8);
+        addr += 16;
+    }
+}
+
+void helper_fxrstor(CPUX86State *env, target_ulong ptr)
+{
+    /* The operand must be 16 byte aligned */
+    if (ptr & 0xf) {
+        raise_exception(env, EXCP0D_GPF);
+    }
+
+    do_xrstor_fpu(env, ptr);
 
     if (env->cr[4] & CR4_OSFXSR_MASK) {
-        /* XXX: finish it */
-        cpu_set_mxcsr(env, cpu_ldl_data(env, ptr + 0x18));
-        /* cpu_ldl_data(env, ptr + 0x1c); */
-        if (env->hflags & HF_CS64_MASK) {
-            nb_xmm_regs = 16;
-        } else {
-            nb_xmm_regs = 8;
-        }
-        addr = ptr + 0xa0;
-        /* Fast FXRESTORE leaves out the XMM registers */
+        do_xrstor_mxcsr(env, ptr);
+        /* Fast FXRSTOR leaves out the XMM registers */
         if (!(env->efer & MSR_EFER_FFXSR)
             || (env->hflags & HF_CPL_MASK)
             || !(env->hflags & HF_LMA_MASK)) {
-            for (i = 0; i < nb_xmm_regs; i++) {
-                env->xmm_regs[i].XMM_Q(0) = cpu_ldq_data(env, addr);
-                env->xmm_regs[i].XMM_Q(1) = cpu_ldq_data(env, addr + 8);
-                addr += 16;
-            }
+            do_xrstor_sse(env, ptr);
         }
     }
 }
diff --git a/target-i386/helper.h b/target-i386/helper.h
index 903197b..7fc8697 100644
--- a/target-i386/helper.h
+++ b/target-i386/helper.h
@@ -183,8 +183,8 @@ DEF_HELPER_3(fstenv, void, env, tl, int)
 DEF_HELPER_3(fldenv, void, env, tl, int)
 DEF_HELPER_3(fsave, void, env, tl, int)
 DEF_HELPER_3(frstor, void, env, tl, int)
-DEF_HELPER_3(fxsave, void, env, tl, int)
-DEF_HELPER_3(fxrstor, void, env, tl, int)
+DEF_HELPER_FLAGS_2(fxsave, TCG_CALL_NO_WG, void, env, tl)
+DEF_HELPER_FLAGS_2(fxrstor, TCG_CALL_NO_WG, void, env, tl)
 
 DEF_HELPER_FLAGS_1(clz, TCG_CALL_NO_RWG_SE, tl, tl)
 DEF_HELPER_FLAGS_1(ctz, TCG_CALL_NO_RWG_SE, tl, tl)
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 72f8df9..4216ecb 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7585,7 +7585,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
             gen_lea_modrm(env, s, modrm);
             gen_update_cc_op(s);
             gen_jmp_im(pc_start - s->cs_base);
-            gen_helper_fxsave(cpu_env, cpu_A0, tcg_const_i32(dflag == MO_64));
+            gen_helper_fxsave(cpu_env, cpu_A0);
             break;
         case 1: /* fxrstor */
             if (mod == 3 || !(s->cpuid_features & CPUID_FXSR) ||
@@ -7598,7 +7598,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
             gen_lea_modrm(env, s, modrm);
             gen_update_cc_op(s);
             gen_jmp_im(pc_start - s->cs_base);
-            gen_helper_fxrstor(cpu_env, cpu_A0, tcg_const_i32(dflag == MO_64));
+            gen_helper_fxrstor(cpu_env, cpu_A0);
             break;
         case 2: /* ldmxcsr */
         case 3: /* stmxcsr */
-- 
2.4.3

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

* [Qemu-devel] [PATCH 02/14] target-i386: Rearrange processing of 0F 01
  2015-07-09  8:17 [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Richard Henderson
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 01/14] target-i386: Split fxsave/fxrstor implementation Richard Henderson
@ 2015-07-09  8:17 ` Richard Henderson
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 03/14] target-i386: Add XSAVE extension Richard Henderson
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2015-07-09  8:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, ehabkost

Rather than nesting tests of OP, MOD, and RM, decode them
all at once with a switch.  Fixes incorrect decoding of
AMD Pacifica extensions (aka vmrun et al) via op==2 path.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-i386/translate.c | 470 +++++++++++++++++++++++++-----------------------
 1 file changed, 247 insertions(+), 223 deletions(-)

diff --git a/target-i386/translate.c b/target-i386/translate.c
index 4216ecb..ebe2cf7 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -60,6 +60,12 @@
 # define clztl  clz32
 #endif
 
+/* For a switch indexed by MODRM, match all memory operands for a given OP.  */
+#define CASE_MEM_OP(OP) \
+    case (0 << 6) | (OP << 3) | 0 ... (0 << 6) | (OP << 3) | 7: \
+    case (1 << 6) | (OP << 3) | 0 ... (1 << 6) | (OP << 3) | 7: \
+    case (2 << 6) | (OP << 3) | 0 ... (2 << 6) | (OP << 3) | 7
+
 //#define MACRO_TEST   1
 
 /* global register indexes */
@@ -7070,15 +7076,11 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
             goto illegal_op;
         }
         break;
+
     case 0x101:
         modrm = cpu_ldub_code(env, s->pc++);
-        mod = (modrm >> 6) & 3;
-        op = (modrm >> 3) & 7;
-        rm = modrm & 7;
-        switch(op) {
-        case 0: /* sgdt */
-            if (mod == 3)
-                goto illegal_op;
+        switch (modrm) {
+        CASE_MEM_OP(0): /* sgdt */
             gen_svm_check_intercept(s, pc_start, SVM_EXIT_GDTR_READ);
             gen_lea_modrm(env, s, modrm);
             tcg_gen_ld32u_tl(cpu_T0,
@@ -7091,178 +7093,200 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
             }
             gen_op_st_v(s, CODE64(s) + MO_32, cpu_T0, cpu_A0);
             break;
-        case 1:
-            if (mod == 3) {
-                switch (rm) {
-                case 0: /* monitor */
-                    if (!(s->cpuid_ext_features & CPUID_EXT_MONITOR) ||
-                        s->cpl != 0)
-                        goto illegal_op;
-                    gen_update_cc_op(s);
-                    gen_jmp_im(pc_start - s->cs_base);
-                    tcg_gen_mov_tl(cpu_A0, cpu_regs[R_EAX]);
-                    gen_extu(s->aflag, cpu_A0);
-                    gen_add_A0_ds_seg(s);
-                    gen_helper_monitor(cpu_env, cpu_A0);
-                    break;
-                case 1: /* mwait */
-                    if (!(s->cpuid_ext_features & CPUID_EXT_MONITOR) ||
-                        s->cpl != 0)
-                        goto illegal_op;
-                    gen_update_cc_op(s);
-                    gen_jmp_im(pc_start - s->cs_base);
-                    gen_helper_mwait(cpu_env, tcg_const_i32(s->pc - pc_start));
-                    gen_eob(s);
-                    break;
-                case 2: /* clac */
-                    if (!(s->cpuid_7_0_ebx_features & CPUID_7_0_EBX_SMAP) ||
-                        s->cpl != 0) {
-                        goto illegal_op;
-                    }
-                    gen_helper_clac(cpu_env);
-                    gen_jmp_im(s->pc - s->cs_base);
-                    gen_eob(s);
-                    break;
-                case 3: /* stac */
-                    if (!(s->cpuid_7_0_ebx_features & CPUID_7_0_EBX_SMAP) ||
-                        s->cpl != 0) {
-                        goto illegal_op;
-                    }
-                    gen_helper_stac(cpu_env);
-                    gen_jmp_im(s->pc - s->cs_base);
-                    gen_eob(s);
-                    break;
-                default:
-                    goto illegal_op;
-                }
-            } else { /* sidt */
-                gen_svm_check_intercept(s, pc_start, SVM_EXIT_IDTR_READ);
-                gen_lea_modrm(env, s, modrm);
-                tcg_gen_ld32u_tl(cpu_T0,
-                                 cpu_env, offsetof(CPUX86State, idt.limit));
-                gen_op_st_v(s, MO_16, cpu_T0, cpu_A0);
-                gen_add_A0_im(s, 2);
-                tcg_gen_ld_tl(cpu_T0,
-                              cpu_env, offsetof(CPUX86State, idt.base));
-                if (dflag == MO_16) {
-                    tcg_gen_andi_tl(cpu_T0, cpu_T0, 0xffffff);
-                }
-                gen_op_st_v(s, CODE64(s) + MO_32, cpu_T0, cpu_A0);
+
+        case 0xc8: /* monitor */
+            if (!(s->cpuid_ext_features & CPUID_EXT_MONITOR) || s->cpl != 0) {
+                goto illegal_op;
             }
+            gen_update_cc_op(s);
+            gen_jmp_im(pc_start - s->cs_base);
+            tcg_gen_mov_tl(cpu_A0, cpu_regs[R_EAX]);
+            gen_extu(s->aflag, cpu_A0);
+            gen_add_A0_ds_seg(s);
+            gen_helper_monitor(cpu_env, cpu_A0);
             break;
-        case 2: /* lgdt */
-        case 3: /* lidt */
-            if (mod == 3) {
-                gen_update_cc_op(s);
-                gen_jmp_im(pc_start - s->cs_base);
-                switch(rm) {
-                case 0: /* VMRUN */
-                    if (!(s->flags & HF_SVME_MASK) || !s->pe)
-                        goto illegal_op;
-                    if (s->cpl != 0) {
-                        gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
-                        break;
-                    } else {
-                        gen_helper_vmrun(cpu_env, tcg_const_i32(s->aflag - 1),
-                                         tcg_const_i32(s->pc - pc_start));
-                        tcg_gen_exit_tb(0);
-                        s->is_jmp = DISAS_TB_JUMP;
-                    }
-                    break;
-                case 1: /* VMMCALL */
-                    if (!(s->flags & HF_SVME_MASK))
-                        goto illegal_op;
-                    gen_helper_vmmcall(cpu_env);
-                    break;
-                case 2: /* VMLOAD */
-                    if (!(s->flags & HF_SVME_MASK) || !s->pe)
-                        goto illegal_op;
-                    if (s->cpl != 0) {
-                        gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
-                        break;
-                    } else {
-                        gen_helper_vmload(cpu_env, tcg_const_i32(s->aflag - 1));
-                    }
-                    break;
-                case 3: /* VMSAVE */
-                    if (!(s->flags & HF_SVME_MASK) || !s->pe)
-                        goto illegal_op;
-                    if (s->cpl != 0) {
-                        gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
-                        break;
-                    } else {
-                        gen_helper_vmsave(cpu_env, tcg_const_i32(s->aflag - 1));
-                    }
-                    break;
-                case 4: /* STGI */
-                    if ((!(s->flags & HF_SVME_MASK) &&
-                         !(s->cpuid_ext3_features & CPUID_EXT3_SKINIT)) || 
-                        !s->pe)
-                        goto illegal_op;
-                    if (s->cpl != 0) {
-                        gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
-                        break;
-                    } else {
-                        gen_helper_stgi(cpu_env);
-                    }
-                    break;
-                case 5: /* CLGI */
-                    if (!(s->flags & HF_SVME_MASK) || !s->pe)
-                        goto illegal_op;
-                    if (s->cpl != 0) {
-                        gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
-                        break;
-                    } else {
-                        gen_helper_clgi(cpu_env);
-                    }
-                    break;
-                case 6: /* SKINIT */
-                    if ((!(s->flags & HF_SVME_MASK) && 
-                         !(s->cpuid_ext3_features & CPUID_EXT3_SKINIT)) || 
-                        !s->pe)
-                        goto illegal_op;
-                    gen_helper_skinit(cpu_env);
-                    break;
-                case 7: /* INVLPGA */
-                    if (!(s->flags & HF_SVME_MASK) || !s->pe)
-                        goto illegal_op;
-                    if (s->cpl != 0) {
-                        gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
-                        break;
-                    } else {
-                        gen_helper_invlpga(cpu_env,
-                                           tcg_const_i32(s->aflag - 1));
-                    }
-                    break;
-                default:
-                    goto illegal_op;
-                }
-            } else if (s->cpl != 0) {
+
+        case 0xc9: /* mwait */
+            if (!(s->cpuid_ext_features & CPUID_EXT_MONITOR) || s->cpl != 0) {
+                goto illegal_op;
+            }
+            gen_update_cc_op(s);
+            gen_jmp_im(pc_start - s->cs_base);
+            gen_helper_mwait(cpu_env, tcg_const_i32(s->pc - pc_start));
+            gen_eob(s);
+            break;
+
+        case 0xca: /* clac */
+            if (!(s->cpuid_7_0_ebx_features & CPUID_7_0_EBX_SMAP)
+                || s->cpl != 0) {
+                goto illegal_op;
+            }
+            gen_helper_clac(cpu_env);
+            gen_jmp_im(s->pc - s->cs_base);
+            gen_eob(s);
+            break;
+
+        case 0xcb: /* stac */
+            if (!(s->cpuid_7_0_ebx_features & CPUID_7_0_EBX_SMAP)
+                || s->cpl != 0) {
+                goto illegal_op;
+            }
+            gen_helper_stac(cpu_env);
+            gen_jmp_im(s->pc - s->cs_base);
+            gen_eob(s);
+            break;
+
+        CASE_MEM_OP(1): /* sidt */
+            gen_svm_check_intercept(s, pc_start, SVM_EXIT_IDTR_READ);
+            gen_lea_modrm(env, s, modrm);
+            tcg_gen_ld32u_tl(cpu_T0, cpu_env, offsetof(CPUX86State, idt.limit));
+            gen_op_st_v(s, MO_16, cpu_T0, cpu_A0);
+            gen_add_A0_im(s, 2);
+            tcg_gen_ld_tl(cpu_T0, cpu_env, offsetof(CPUX86State, idt.base));
+            if (dflag == MO_16) {
+                tcg_gen_andi_tl(cpu_T0, cpu_T0, 0xffffff);
+            }
+            gen_op_st_v(s, CODE64(s) + MO_32, cpu_T0, cpu_A0);
+            break;
+
+        case 0xd8: /* VMRUN */
+            if (!(s->flags & HF_SVME_MASK) || !s->pe) {
+                goto illegal_op;
+            }
+            if (s->cpl != 0) {
                 gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
-            } else {
-                gen_svm_check_intercept(s, pc_start,
-                                        op==2 ? SVM_EXIT_GDTR_WRITE : SVM_EXIT_IDTR_WRITE);
-                gen_lea_modrm(env, s, modrm);
-                gen_op_ld_v(s, MO_16, cpu_T1, cpu_A0);
-                gen_add_A0_im(s, 2);
-                gen_op_ld_v(s, CODE64(s) + MO_32, cpu_T0, cpu_A0);
-                if (dflag == MO_16) {
-                    tcg_gen_andi_tl(cpu_T0, cpu_T0, 0xffffff);
-                }
-                if (op == 2) {
-                    tcg_gen_st_tl(cpu_T0, cpu_env,
-                                  offsetof(CPUX86State, gdt.base));
-                    tcg_gen_st32_tl(cpu_T1, cpu_env,
-                                    offsetof(CPUX86State, gdt.limit));
-                } else {
-                    tcg_gen_st_tl(cpu_T0, cpu_env,
-                                  offsetof(CPUX86State, idt.base));
-                    tcg_gen_st32_tl(cpu_T1, cpu_env,
-                                    offsetof(CPUX86State, idt.limit));
-                }
+                break;
+            }
+            gen_update_cc_op(s);
+            gen_jmp_im(pc_start - s->cs_base);
+            gen_helper_vmrun(cpu_env, tcg_const_i32(s->aflag - 1),
+                             tcg_const_i32(s->pc - pc_start));
+            tcg_gen_exit_tb(0);
+            s->is_jmp = DISAS_TB_JUMP;
+            break;
+
+        case 0xd9: /* VMMCALL */
+            if (!(s->flags & HF_SVME_MASK)) {
+                goto illegal_op;
             }
+            gen_update_cc_op(s);
+            gen_jmp_im(pc_start - s->cs_base);
+            gen_helper_vmmcall(cpu_env);
             break;
-        case 4: /* smsw */
+
+        case 0xda: /* VMLOAD */
+            if (!(s->flags & HF_SVME_MASK) || !s->pe) {
+                goto illegal_op;
+            }
+            if (s->cpl != 0) {
+                gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
+                break;
+            }
+            gen_update_cc_op(s);
+            gen_jmp_im(pc_start - s->cs_base);
+            gen_helper_vmload(cpu_env, tcg_const_i32(s->aflag - 1));
+            break;
+
+        case 0xdb: /* VMSAVE */
+            if (!(s->flags & HF_SVME_MASK) || !s->pe) {
+                goto illegal_op;
+            }
+            if (s->cpl != 0) {
+                gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
+                break;
+            }
+            gen_update_cc_op(s);
+            gen_jmp_im(pc_start - s->cs_base);
+            gen_helper_vmsave(cpu_env, tcg_const_i32(s->aflag - 1));
+            break;
+
+        case 0xdc: /* STGI */
+            if ((!(s->flags & HF_SVME_MASK)
+                   && !(s->cpuid_ext3_features & CPUID_EXT3_SKINIT))
+                || !s->pe) {
+                goto illegal_op;
+            }
+            if (s->cpl != 0) {
+                gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
+                break;
+            }
+            gen_update_cc_op(s);
+            gen_jmp_im(pc_start - s->cs_base);
+            gen_helper_stgi(cpu_env);
+            break;
+
+        case 0xdd: /* CLGI */
+            if (!(s->flags & HF_SVME_MASK) || !s->pe) {
+                goto illegal_op;
+            }
+            if (s->cpl != 0) {
+                gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
+                break;
+            }
+            gen_update_cc_op(s);
+            gen_jmp_im(pc_start - s->cs_base);
+            gen_helper_clgi(cpu_env);
+            break;
+
+        case 0xde: /* SKINIT */
+            if ((!(s->flags & HF_SVME_MASK)
+                 && !(s->cpuid_ext3_features & CPUID_EXT3_SKINIT))
+                || !s->pe) {
+                goto illegal_op;
+            }
+            gen_update_cc_op(s);
+            gen_jmp_im(pc_start - s->cs_base);
+            gen_helper_skinit(cpu_env);
+            break;
+
+        case 0xdf: /* INVLPGA */
+            if (!(s->flags & HF_SVME_MASK) || !s->pe) {
+                goto illegal_op;
+            }
+            if (s->cpl != 0) {
+                gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
+                break;
+            }
+            gen_update_cc_op(s);
+            gen_jmp_im(pc_start - s->cs_base);
+            gen_helper_invlpga(cpu_env, tcg_const_i32(s->aflag - 1));
+            break;
+
+        CASE_MEM_OP(2): /* lgdt */
+            if (s->cpl != 0) {
+                gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
+                break;
+            }
+            gen_svm_check_intercept(s, pc_start, SVM_EXIT_GDTR_WRITE);
+            gen_lea_modrm(env, s, modrm);
+            gen_op_ld_v(s, MO_16, cpu_T1, cpu_A0);
+            gen_add_A0_im(s, 2);
+            gen_op_ld_v(s, CODE64(s) + MO_32, cpu_T0, cpu_A0);
+            if (dflag == MO_16) {
+                tcg_gen_andi_tl(cpu_T0, cpu_T0, 0xffffff);
+            }
+            tcg_gen_st_tl(cpu_T0, cpu_env, offsetof(CPUX86State, gdt.base));
+            tcg_gen_st32_tl(cpu_T1, cpu_env, offsetof(CPUX86State, gdt.limit));
+            break;
+
+        CASE_MEM_OP(3): /* lidt */
+            if (s->cpl != 0) {
+                gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
+                break;
+            }
+            gen_svm_check_intercept(s, pc_start, SVM_EXIT_IDTR_WRITE);
+            gen_lea_modrm(env, s, modrm);
+            gen_op_ld_v(s, MO_16, cpu_T1, cpu_A0);
+            gen_add_A0_im(s, 2);
+            gen_op_ld_v(s, CODE64(s) + MO_32, cpu_T0, cpu_A0);
+            if (dflag == MO_16) {
+                tcg_gen_andi_tl(cpu_T0, cpu_T0, 0xffffff);
+            }
+            tcg_gen_st_tl(cpu_T0, cpu_env, offsetof(CPUX86State, idt.base));
+            tcg_gen_st32_tl(cpu_T1, cpu_env, offsetof(CPUX86State, idt.limit));
+            break;
+
+        CASE_MEM_OP(4): /* smsw */
             gen_svm_check_intercept(s, pc_start, SVM_EXIT_READ_CR0);
 #if defined TARGET_X86_64 && defined HOST_WORDS_BIGENDIAN
             tcg_gen_ld32u_tl(cpu_T0, cpu_env, offsetof(CPUX86State, cr[0]) + 4);
@@ -7271,70 +7295,70 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
 #endif
             gen_ldst_modrm(env, s, modrm, MO_16, OR_TMP0, 1);
             break;
-        case 6: /* lmsw */
+
+        CASE_MEM_OP(6): /* lmsw */
             if (s->cpl != 0) {
                 gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
-            } else {
-                gen_svm_check_intercept(s, pc_start, SVM_EXIT_WRITE_CR0);
-                gen_ldst_modrm(env, s, modrm, MO_16, OR_TMP0, 0);
-                gen_helper_lmsw(cpu_env, cpu_T0);
-                gen_jmp_im(s->pc - s->cs_base);
-                gen_eob(s);
+                break;
+            }
+            gen_svm_check_intercept(s, pc_start, SVM_EXIT_WRITE_CR0);
+            gen_ldst_modrm(env, s, modrm, MO_16, OR_TMP0, 0);
+            gen_helper_lmsw(cpu_env, cpu_T0);
+            gen_jmp_im(s->pc - s->cs_base);
+            gen_eob(s);
+            break;
+
+        CASE_MEM_OP(7): /* invlpg */
+            if (s->cpl != 0) {
+                gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
+                break;
             }
+            gen_update_cc_op(s);
+            gen_jmp_im(pc_start - s->cs_base);
+            gen_lea_modrm(env, s, modrm);
+            gen_helper_invlpg(cpu_env, cpu_A0);
+            gen_jmp_im(s->pc - s->cs_base);
+            gen_eob(s);
             break;
-        case 7:
-            if (mod != 3) { /* invlpg */
+
+        case 0xf8: /* swapgs */
+#ifdef TARGET_X86_64
+            if (CODE64(s)) {
                 if (s->cpl != 0) {
                     gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
                 } else {
-                    gen_update_cc_op(s);
-                    gen_jmp_im(pc_start - s->cs_base);
-                    gen_lea_modrm(env, s, modrm);
-                    gen_helper_invlpg(cpu_env, cpu_A0);
-                    gen_jmp_im(s->pc - s->cs_base);
-                    gen_eob(s);
+                    tcg_gen_mov_tl(cpu_T0, cpu_seg_base[R_GS]);
+                    tcg_gen_ld_tl(cpu_seg_base[R_GS], cpu_env,
+                                  offsetof(CPUX86State, kernelgsbase));
+                    tcg_gen_st_tl(cpu_T0, cpu_env,
+                                  offsetof(CPUX86State, kernelgsbase));
                 }
-            } else {
-                switch (rm) {
-                case 0: /* swapgs */
-#ifdef TARGET_X86_64
-                    if (CODE64(s)) {
-                        if (s->cpl != 0) {
-                            gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
-                        } else {
-                            tcg_gen_mov_tl(cpu_T0, cpu_seg_base[R_GS]);
-                            tcg_gen_ld_tl(cpu_seg_base[R_GS], cpu_env,
-                                          offsetof(CPUX86State, kernelgsbase));
-                            tcg_gen_st_tl(cpu_T0, cpu_env,
-                                          offsetof(CPUX86State, kernelgsbase));
-                        }
-                        break;
-                    }
+                break;
+            }
 #endif
-                    goto illegal_op;
-                case 1: /* rdtscp */
-                    if (!(s->cpuid_ext2_features & CPUID_EXT2_RDTSCP))
-                        goto illegal_op;
-                    gen_update_cc_op(s);
-                    gen_jmp_im(pc_start - s->cs_base);
-                    if (s->tb->cflags & CF_USE_ICOUNT) {
-                        gen_io_start();
-		    }
-                    gen_helper_rdtscp(cpu_env);
-                    if (s->tb->cflags & CF_USE_ICOUNT) {
-                        gen_io_end();
-                        gen_jmp(s, s->pc - s->cs_base);
-                    }
-                    break;
-                default:
-                    goto illegal_op;
-                }
+            goto illegal_op;
+
+        case 0xf9: /* rdtscp */
+            if (!(s->cpuid_ext2_features & CPUID_EXT2_RDTSCP)) {
+                goto illegal_op;
+            }
+            gen_update_cc_op(s);
+            gen_jmp_im(pc_start - s->cs_base);
+            if (s->tb->cflags & CF_USE_ICOUNT) {
+                gen_io_start();
+            }
+            gen_helper_rdtscp(cpu_env);
+            if (s->tb->cflags & CF_USE_ICOUNT) {
+                gen_io_end();
+                gen_jmp(s, s->pc - s->cs_base);
             }
             break;
+
         default:
             goto illegal_op;
         }
         break;
+
     case 0x108: /* invd */
     case 0x109: /* wbinvd */
         if (s->cpl != 0) {
-- 
2.4.3

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

* [Qemu-devel] [PATCH 03/14] target-i386: Add XSAVE extension
  2015-07-09  8:17 [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Richard Henderson
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 01/14] target-i386: Split fxsave/fxrstor implementation Richard Henderson
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 02/14] target-i386: Rearrange processing of 0F 01 Richard Henderson
@ 2015-07-09  8:17 ` Richard Henderson
  2015-07-09 13:16   ` Paolo Bonzini
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 04/14] target-i386: Implement XSAVEOPT Richard Henderson
                   ` (11 subsequent siblings)
  14 siblings, 1 reply; 31+ messages in thread
From: Richard Henderson @ 2015-07-09  8:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, ehabkost

This includes XSAVE, XRSTOR, XGETBV, XSETBV, which are all related,
as well as the associate cpuid bits.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-i386/cpu.c        |  39 +++++++++------
 target-i386/cpu.h        |   2 +
 target-i386/fpu_helper.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++
 target-i386/helper.c     |  15 ++++--
 target-i386/helper.h     |   4 ++
 target-i386/kvm.c        |  16 ++++--
 target-i386/translate.c  |  72 ++++++++++++++++++++++++++-
 7 files changed, 245 insertions(+), 26 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 36b07f9..750eed1 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -310,14 +310,14 @@ static const char *cpuid_xsave_feature_name[] = {
 #define TCG_EXT_FEATURES (CPUID_EXT_SSE3 | CPUID_EXT_PCLMULQDQ | \
           CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 | CPUID_EXT_CX16 | \
           CPUID_EXT_SSE41 | CPUID_EXT_SSE42 | CPUID_EXT_POPCNT | \
+          CPUID_EXT_XSAVE | /* CPUID_EXT_OSXSAVE is dynamic */   \
           CPUID_EXT_MOVBE | CPUID_EXT_AES | CPUID_EXT_HYPERVISOR)
           /* missing:
           CPUID_EXT_DTES64, CPUID_EXT_DSCPL, CPUID_EXT_VMX, CPUID_EXT_SMX,
           CPUID_EXT_EST, CPUID_EXT_TM2, CPUID_EXT_CID, CPUID_EXT_FMA,
           CPUID_EXT_XTPR, CPUID_EXT_PDCM, CPUID_EXT_PCID, CPUID_EXT_DCA,
-          CPUID_EXT_X2APIC, CPUID_EXT_TSC_DEADLINE_TIMER, CPUID_EXT_XSAVE,
-          CPUID_EXT_OSXSAVE, CPUID_EXT_AVX, CPUID_EXT_F16C,
-          CPUID_EXT_RDRAND */
+          CPUID_EXT_X2APIC, CPUID_EXT_TSC_DEADLINE_TIMER, CPUID_EXT_AVX,
+          CPUID_EXT_F16C, CPUID_EXT_RDRAND */
 
 #ifdef TARGET_X86_64
 #define TCG_EXT2_X86_64_FEATURES (CPUID_EXT2_SYSCALL | CPUID_EXT2_LM)
@@ -2276,10 +2276,13 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *ebx = (cpu->apic_id << 24) |
                8 << 8; /* CLFLUSH size in quad words, Linux wants it. */
         *ecx = env->features[FEAT_1_ECX];
+        if ((*ecx & CPUID_EXT_XSAVE) && (env->hflags & HF_OSXSAVE_MASK)) {
+            *ecx |= CPUID_EXT_OSXSAVE;
+        }
         *edx = env->features[FEAT_1_EDX];
         if (cs->nr_cores * cs->nr_threads > 1) {
             *ebx |= (cs->nr_cores * cs->nr_threads) << 16;
-            *edx |= 1 << 28;    /* HTT bit */
+            *edx |= CPUID_HT;
         }
         break;
     case 2:
@@ -2403,7 +2406,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         break;
     case 0xD: {
         KVMState *s = cs->kvm_state;
-        uint64_t kvm_mask;
+        uint64_t ena_mask;
         int i;
 
         /* Processor Extended State */
@@ -2411,35 +2414,39 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *ebx = 0;
         *ecx = 0;
         *edx = 0;
-        if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE) || !kvm_enabled()) {
+        if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) {
             break;
         }
-        kvm_mask =
-            kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX) |
-            ((uint64_t)kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX) << 32);
+        if (kvm_enabled()) {
+            ena_mask = kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX);
+            ena_mask <<= 32;
+            ena_mask |= kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX);
+        } else {
+            ena_mask = -1;
+        }
 
         if (count == 0) {
             *ecx = 0x240;
             for (i = 2; i < ARRAY_SIZE(ext_save_areas); i++) {
                 const ExtSaveArea *esa = &ext_save_areas[i];
-                if ((env->features[esa->feature] & esa->bits) == esa->bits &&
-                    (kvm_mask & (1 << i)) != 0) {
+                if ((env->features[esa->feature] & esa->bits) == esa->bits
+                    && ((ena_mask >> i) & 1) != 0) {
                     if (i < 32) {
-                        *eax |= 1 << i;
+                        *eax |= 1u << i;
                     } else {
-                        *edx |= 1 << (i - 32);
+                        *edx |= 1u << (i - 32);
                     }
                     *ecx = MAX(*ecx, esa->offset + esa->size);
                 }
             }
-            *eax |= kvm_mask & (XSTATE_FP | XSTATE_SSE);
+            *eax |= ena_mask & (XSTATE_FP | XSTATE_SSE);
             *ebx = *ecx;
         } else if (count == 1) {
             *eax = env->features[FEAT_XSAVE];
         } else if (count < ARRAY_SIZE(ext_save_areas)) {
             const ExtSaveArea *esa = &ext_save_areas[count];
-            if ((env->features[esa->feature] & esa->bits) == esa->bits &&
-                (kvm_mask & (1 << count)) != 0) {
+            if ((env->features[esa->feature] & esa->bits) == esa->bits
+                && ((ena_mask >> count) & 1) != 0) {
                 *eax = esa->size;
                 *ebx = esa->offset;
             }
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 603aaf0..a8153c0 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -154,6 +154,7 @@
 #define HF_SVMI_SHIFT       21 /* SVM intercepts are active */
 #define HF_OSFXSR_SHIFT     22 /* CR4.OSFXSR */
 #define HF_SMAP_SHIFT       23 /* CR4.SMAP */
+#define HF_OSXSAVE_SHIFT    24 /* CR4.OSXSAVE */
 
 #define HF_CPL_MASK          (3 << HF_CPL_SHIFT)
 #define HF_SOFTMMU_MASK      (1 << HF_SOFTMMU_SHIFT)
@@ -177,6 +178,7 @@
 #define HF_SVMI_MASK         (1 << HF_SVMI_SHIFT)
 #define HF_OSFXSR_MASK       (1 << HF_OSFXSR_SHIFT)
 #define HF_SMAP_MASK         (1 << HF_SMAP_SHIFT)
+#define HF_OSXSAVE_MASK      (1 << HF_OSXSAVE_SHIFT)
 
 /* hflags2 */
 
diff --git a/target-i386/fpu_helper.c b/target-i386/fpu_helper.c
index 1d72cd1..5e860b7 100644
--- a/target-i386/fpu_helper.c
+++ b/target-i386/fpu_helper.c
@@ -1174,6 +1174,39 @@ void helper_fxsave(CPUX86State *env, target_ulong ptr)
     }
 }
 
+static uint64_t get_xinuse(CPUX86State *env)
+{
+    /* We don't track XINUSE.  We could calculate it here, but it's
+       probably less work to simply indicate all components in use.  */
+    return -1;
+}
+
+void helper_xsave(CPUX86State *env, target_ulong ptr, uint64_t rfbm)
+{
+    uint64_t old_bv, new_bv;
+
+    /* The operand must be 64 byte aligned.  */
+    if (ptr & 63) {
+        raise_exception(env, EXCP0D_GPF);
+    }
+
+    /* Never save anything not enabled by XCR0.  */
+    rfbm &= env->xcr0;
+
+    if (rfbm & XSTATE_FP) {
+        do_xsave_fpu(env, ptr);
+    }
+    if (rfbm & XSTATE_SSE) {
+        do_xsave_mxcsr(env, ptr);
+        do_xsave_sse(env, ptr);
+    }
+
+    /* Update the XSTATE_BV field.  */
+    old_bv = cpu_ldq_data(env, ptr + 512);
+    new_bv = (old_bv & ~rfbm) | (get_xinuse(env) & rfbm);
+    cpu_stq_data(env, ptr + 512, new_bv);
+}
+
 static void do_xrstor_fpu(CPUX86State *env, target_ulong ptr)
 {
     int i, fpus, fptag;
@@ -1241,6 +1274,96 @@ void helper_fxrstor(CPUX86State *env, target_ulong ptr)
     }
 }
 
+void helper_xrstor(CPUX86State *env, target_ulong ptr, uint64_t rfbm)
+{
+    uint64_t xstate_bv, xcomp_bv0, xcomp_bv1;
+
+    rfbm &= env->xcr0;
+
+    /* The operand must be 64 byte aligned.  */
+    if (ptr & 63) {
+        raise_exception(env, EXCP0D_GPF);
+    }
+
+    xstate_bv = cpu_ldq_data(env, ptr + 512);
+
+    if ((int64_t)xstate_bv < 0) {
+        /* FIXME: Compact form.  */
+        raise_exception(env, EXCP0D_GPF);
+    }
+
+    /* Standard form.  */
+
+    /* The XSTATE field must not set bits not present in XCR0.  */
+    if (xstate_bv & ~env->xcr0) {
+        raise_exception(env, EXCP0D_GPF);
+    }
+
+    /* The XCOMP field must be zero.  */
+    xcomp_bv0 = cpu_ldq_data(env, ptr + 520);
+    xcomp_bv1 = cpu_ldq_data(env, ptr + 528);
+    if (xcomp_bv0 || xcomp_bv1) {
+        raise_exception(env, EXCP0D_GPF);
+    }
+
+    if (rfbm & XSTATE_FP) {
+        if (xstate_bv & XSTATE_FP) {
+            do_xrstor_fpu(env, ptr);
+        } else {
+            helper_fninit(env);
+            memset(env->fpregs, 0, sizeof(env->fpregs));
+        }
+    }
+    if (rfbm & XSTATE_SSE) {
+        /* Note that the standard form of XRSTOR loads MXCSR from memory
+           whether or not the XSTATE_BV bit is set.  */
+        do_xrstor_mxcsr(env, ptr);
+        if (xstate_bv & XSTATE_SSE) {
+            do_xrstor_sse(env, ptr);
+        } else {
+            /* ??? When AVX is implemented, we may have to be more
+               selective in the clearing.  */
+            memset(env->xmm_regs, 0, sizeof(env->xmm_regs));
+        }
+    }
+}
+
+uint64_t helper_xgetbv(CPUX86State *env, uint32_t ecx)
+{
+    switch (ecx) {
+    case 0:
+        return env->xcr0;
+    case 1:
+        /* FIXME: #GP if !CPUID.(EAX=0DH,ECX=1):EAX.XG1[bit 2].  */
+        return env->xcr0 & get_xinuse(env);
+    }
+    raise_exception(env, EXCP0D_GPF);
+}
+
+void helper_xsetbv(CPUX86State *env, uint32_t ecx, uint64_t mask)
+{
+    uint32_t dummy, ena_lo, ena_hi;
+    uint64_t ena;
+
+    /* Only XCR0 is defined at present; the FPU may not be disabled.  */
+    if (ecx != 0 || (mask & XSTATE_FP) == 0) {
+        goto do_gpf;
+    }
+
+    /* Disallow enabling unimplemented features.  */
+    cpu_x86_cpuid(env, 0x0d, 0, &ena_lo, &dummy, &dummy, &ena_hi);
+    ena = ((uint64_t)ena_hi << 32) | ena_lo;
+    if (mask & ~ena) {
+        goto do_gpf;
+    }
+
+    env->xcr0 = mask;
+    return;
+
+ do_gpf:
+    raise_exception(env, EXCP0D_GPF);
+}
+
 void cpu_get_fp80(uint64_t *pmant, uint16_t *pexp, floatx80 f)
 {
     CPU_LDoubleU temp;
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 5480a96..5e8e63d 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -455,6 +455,7 @@ void cpu_x86_update_cr3(CPUX86State *env, target_ulong new_cr3)
 void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4)
 {
     X86CPU *cpu = x86_env_get_cpu(env);
+    uint32_t hflags;
 
 #if defined(DEBUG_MMU)
     printf("CR4 update: CR4=%08x\n", (uint32_t)env->cr[4]);
@@ -464,24 +465,30 @@ void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4)
          CR4_SMEP_MASK | CR4_SMAP_MASK)) {
         tlb_flush(CPU(cpu), 1);
     }
+
+    /* Clear bits we're going to recompute.  */
+    hflags = env->hflags & ~(HF_OSFXSR_MASK | HF_OSXSAVE_MASK | HF_SMAP_MASK);
+
     /* SSE handling */
     if (!(env->features[FEAT_1_EDX] & CPUID_SSE)) {
         new_cr4 &= ~CR4_OSFXSR_MASK;
     }
-    env->hflags &= ~HF_OSFXSR_MASK;
     if (new_cr4 & CR4_OSFXSR_MASK) {
-        env->hflags |= HF_OSFXSR_MASK;
+        hflags |= HF_OSFXSR_MASK;
+    }
+    if (new_cr4 & CR4_OSXSAVE_MASK) {
+        hflags |= HF_OSXSAVE_MASK;
     }
 
     if (!(env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_SMAP)) {
         new_cr4 &= ~CR4_SMAP_MASK;
     }
-    env->hflags &= ~HF_SMAP_MASK;
     if (new_cr4 & CR4_SMAP_MASK) {
-        env->hflags |= HF_SMAP_MASK;
+        hflags |= HF_SMAP_MASK;
     }
 
     env->cr[4] = new_cr4;
+    env->hflags = hflags;
 }
 
 #if defined(CONFIG_USER_ONLY)
diff --git a/target-i386/helper.h b/target-i386/helper.h
index 7fc8697..1ea4998 100644
--- a/target-i386/helper.h
+++ b/target-i386/helper.h
@@ -185,6 +185,10 @@ DEF_HELPER_3(fsave, void, env, tl, int)
 DEF_HELPER_3(frstor, void, env, tl, int)
 DEF_HELPER_FLAGS_2(fxsave, TCG_CALL_NO_WG, void, env, tl)
 DEF_HELPER_FLAGS_2(fxrstor, TCG_CALL_NO_WG, void, env, tl)
+DEF_HELPER_FLAGS_3(xsave, TCG_CALL_NO_WG, void, env, tl, i64)
+DEF_HELPER_FLAGS_3(xrstor, TCG_CALL_NO_WG, void, env, tl, i64)
+DEF_HELPER_FLAGS_2(xgetbv, TCG_CALL_NO_WG, i64, env, i32)
+DEF_HELPER_FLAGS_3(xsetbv, TCG_CALL_NO_WG, void, env, i32, i64)
 
 DEF_HELPER_FLAGS_1(clz, TCG_CALL_NO_RWG_SE, tl, tl)
 DEF_HELPER_FLAGS_1(ctz, TCG_CALL_NO_RWG_SE, tl, tl)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index daced5c..f057982 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1541,16 +1541,22 @@ static int kvm_get_sregs(X86CPU *cpu)
 #define HFLAG_COPY_MASK \
     ~( HF_CPL_MASK | HF_PE_MASK | HF_MP_MASK | HF_EM_MASK | \
        HF_TS_MASK | HF_TF_MASK | HF_VM_MASK | HF_IOPL_MASK | \
-       HF_OSFXSR_MASK | HF_LMA_MASK | HF_CS32_MASK | \
+       HF_OSFXSR_MASK | HF_OSXSAVE_MASK | HF_LMA_MASK | HF_CS32_MASK | \
        HF_SS32_MASK | HF_CS64_MASK | HF_ADDSEG_MASK)
 
-    hflags = (env->segs[R_SS].flags >> DESC_DPL_SHIFT) & HF_CPL_MASK;
+    hflags = env->hflags & HFLAG_COPY_MASK;
+    hflags |= (env->segs[R_SS].flags >> DESC_DPL_SHIFT) & HF_CPL_MASK;
     hflags |= (env->cr[0] & CR0_PE_MASK) << (HF_PE_SHIFT - CR0_PE_SHIFT);
     hflags |= (env->cr[0] << (HF_MP_SHIFT - CR0_MP_SHIFT)) &
                 (HF_MP_MASK | HF_EM_MASK | HF_TS_MASK);
     hflags |= (env->eflags & (HF_TF_MASK | HF_VM_MASK | HF_IOPL_MASK));
-    hflags |= (env->cr[4] & CR4_OSFXSR_MASK) <<
-                (HF_OSFXSR_SHIFT - CR4_OSFXSR_SHIFT);
+
+    if (env->cr[4] & CR4_OSFXSR_MASK) {
+        hflags |= HF_OSFXSR_MASK;
+    }
+    if (env->cr[4] & CR4_OSXSAVE_MASK) {
+        hflags |= HF_OSXSAVE_MASK;
+    }
 
     if (env->efer & MSR_EFER_LMA) {
         hflags |= HF_LMA_MASK;
@@ -1571,7 +1577,7 @@ static int kvm_get_sregs(X86CPU *cpu)
                         env->segs[R_SS].base) != 0) << HF_ADDSEG_SHIFT;
         }
     }
-    env->hflags = (env->hflags & HFLAG_COPY_MASK) | hflags;
+    env->hflags = hflags;
 
     return 0;
 }
diff --git a/target-i386/translate.c b/target-i386/translate.c
index ebe2cf7..e9992aa 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7149,6 +7149,42 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
             gen_op_st_v(s, CODE64(s) + MO_32, cpu_T0, cpu_A0);
             break;
 
+        case 0xd0: /* xgetbv */
+            if ((s->cpuid_ext_features & CPUID_EXT_XSAVE) == 0
+                || (s->flags & HF_OSXSAVE_MASK) == 0
+                || (s->prefix & (PREFIX_LOCK | PREFIX_DATA
+                                 | PREFIX_REPZ | PREFIX_REPNZ))) {
+                goto illegal_op;
+            }
+            gen_update_cc_op(s);
+            gen_jmp_im(pc_start - s->cs_base);
+            tcg_gen_trunc_tl_i32(cpu_tmp2_i32, cpu_regs[R_ECX]);
+            gen_helper_xgetbv(cpu_tmp1_i64, cpu_env, cpu_tmp2_i32);
+            tcg_gen_extr_i64_tl(cpu_regs[R_EAX], cpu_regs[R_EDX], cpu_tmp1_i64);
+            break;
+
+        case 0xd1: /* xsetbv */
+            if ((s->cpuid_ext_features & CPUID_EXT_XSAVE) == 0
+                || (s->flags & HF_OSXSAVE_MASK) == 0
+                || (s->prefix & (PREFIX_LOCK | PREFIX_DATA
+                                 | PREFIX_REPZ | PREFIX_REPNZ))) {
+                goto illegal_op;
+            }
+            if (s->cpl != 0) {
+                gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
+                break;
+            }
+            gen_update_cc_op(s);
+            gen_jmp_im(pc_start - s->cs_base);
+            tcg_gen_concat_tl_i64(cpu_tmp1_i64, cpu_regs[R_EAX],
+                                  cpu_regs[R_EDX]);
+            tcg_gen_trunc_tl_i32(cpu_tmp2_i32, cpu_regs[R_ECX]);
+            gen_helper_xsetbv(cpu_env, cpu_tmp2_i32, cpu_tmp1_i64);
+            /* End TB because translation flags may change.  */
+            gen_jmp_im(s->pc - pc_start);
+            gen_eob(s);
+            break;
+
         case 0xd8: /* VMRUN */
             if (!(s->flags & HF_SVME_MASK) || !s->pe) {
                 goto illegal_op;
@@ -7644,7 +7680,41 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
                 gen_op_st_v(s, MO_32, cpu_T0, cpu_A0);
             }
             break;
-        case 5: /* lfence */
+        case 4: /* xsave */
+            if (mod == 3
+                || (s->cpuid_ext_features & CPUID_EXT_XSAVE) == 0
+                || (s->flags & HF_OSXSAVE_MASK) == 0
+                || (s->prefix & (PREFIX_LOCK | PREFIX_DATA
+                                 | PREFIX_REPZ | PREFIX_REPNZ))) {
+                goto illegal_op;
+            }
+            gen_lea_modrm(env, s, modrm);
+            tcg_gen_concat_tl_i64(cpu_tmp1_i64, cpu_regs[R_EAX],
+                                  cpu_regs[R_EDX]);
+            gen_helper_xsave(cpu_env, cpu_A0, cpu_tmp1_i64);
+            break;
+        case 5:
+            if (mod == 3) {
+                /* lfence */
+                if (!(s->cpuid_features & CPUID_SSE2)
+                    || (s->prefix & PREFIX_LOCK)) {
+                    goto illegal_op;
+                }
+                /* no-op */
+            } else {
+                /* xrstor */
+                if ((s->cpuid_ext_features & CPUID_EXT_XSAVE) == 0
+                    || (s->flags & HF_OSXSAVE_MASK) == 0
+                    || (s->prefix & (PREFIX_LOCK | PREFIX_DATA
+                                     | PREFIX_REPZ | PREFIX_REPNZ))) {
+                    goto illegal_op;
+                }
+                gen_lea_modrm(env, s, modrm);
+                tcg_gen_concat_tl_i64(cpu_tmp1_i64, cpu_regs[R_EAX],
+                                      cpu_regs[R_EDX]);
+                gen_helper_xrstor(cpu_env, cpu_A0, cpu_tmp1_i64);
+            }
+            break;
         case 6: /* mfence */
             if ((modrm & 0xc7) != 0xc0 || !(s->cpuid_features & CPUID_SSE2))
                 goto illegal_op;
-- 
2.4.3

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

* [Qemu-devel] [PATCH 04/14] target-i386: Implement XSAVEOPT
  2015-07-09  8:17 [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Richard Henderson
                   ` (2 preceding siblings ...)
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 03/14] target-i386: Add XSAVE extension Richard Henderson
@ 2015-07-09  8:17 ` Richard Henderson
  2015-07-09 13:06   ` Paolo Bonzini
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 05/14] target-i386: Enable control registers for MPX Richard Henderson
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 31+ messages in thread
From: Richard Henderson @ 2015-07-09  8:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, ehabkost

Note the cpu.c change -- don't advertise more XSAVE features
than we implement.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-i386/cpu.c        |  2 +-
 target-i386/fpu_helper.c | 28 +++++++++++++++++++++++-----
 target-i386/helper.h     |  1 +
 target-i386/translate.c  | 27 ++++++++++++++++++++++++---
 4 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 750eed1..b2fc59f 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -405,7 +405,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
     },
     [FEAT_XSAVE] = {
         .feat_names = cpuid_xsave_feature_name,
-        .cpuid_eax = 0xd,
+        .cpuid_eax = CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XGETBV1,
         .cpuid_needs_ecx = true, .cpuid_ecx = 1,
         .cpuid_reg = R_EAX,
         .tcg_features = 0,
diff --git a/target-i386/fpu_helper.c b/target-i386/fpu_helper.c
index 5e860b7..d0b960c 100644
--- a/target-i386/fpu_helper.c
+++ b/target-i386/fpu_helper.c
@@ -1181,7 +1181,8 @@ static uint64_t get_xinuse(CPUX86State *env)
     return -1;
 }
 
-void helper_xsave(CPUX86State *env, target_ulong ptr, uint64_t rfbm)
+static void do_xsave(CPUX86State *env, target_ulong ptr,
+                     uint64_t rfbm, uint64_t inuse, uint64_t opt)
 {
     uint64_t old_bv, new_bv;
 
@@ -1192,21 +1193,36 @@ void helper_xsave(CPUX86State *env, target_ulong ptr, uint64_t rfbm)
 
     /* Never save anything not enabled by XCR0.  */
     rfbm &= env->xcr0;
+    opt &= rfbm;
 
-    if (rfbm & XSTATE_FP) {
+    if (opt & XSTATE_FP) {
         do_xsave_fpu(env, ptr);
     }
     if (rfbm & XSTATE_SSE) {
+        /* Note that saving MXCSR is not suppressed by XSAVEOPT.  */
         do_xsave_mxcsr(env, ptr);
+    }
+    if (opt & XSTATE_SSE) {
         do_xsave_sse(env, ptr);
     }
 
     /* Update the XSTATE_BV field.  */
     old_bv = cpu_ldq_data(env, ptr + 512);
-    new_bv = (old_bv & ~rfbm) | (get_xinuse(env) & rfbm);
+    new_bv = (old_bv & ~rfbm) | (inuse & rfbm);
     cpu_stq_data(env, ptr + 512, new_bv);
 }
 
+void helper_xsave(CPUX86State *env, target_ulong ptr, uint64_t rfbm)
+{
+    do_xsave(env, ptr, rfbm, get_xinuse(env), -1);
+}
+
+void helper_xsaveopt(CPUX86State *env, target_ulong ptr, uint64_t rfbm)
+{
+    uint64_t inuse = get_xinuse(env);
+    do_xsave(env, ptr, rfbm, inuse, inuse);
+}
+
 static void do_xrstor_fpu(CPUX86State *env, target_ulong ptr)
 {
     int i, fpus, fptag;
@@ -1334,8 +1350,10 @@ uint64_t helper_xgetbv(CPUX86State *env, uint32_t ecx)
     case 0:
         return env->xcr0;
     case 1:
-        /* FIXME: #GP if !CPUID.(EAX=0DH,ECX=1):EAX.XG1[bit 2].  */
-        return env->xcr0 & get_xinuse(env);
+        if (env->features[FEAT_XSAVE] & CPUID_XSAVE_XGETBV1) {
+            return env->xcr0 & get_xinuse(env);
+        }
+        break;
     }
     raise_exception(env, EXCP0D_GPF);
 }
diff --git a/target-i386/helper.h b/target-i386/helper.h
index 1ea4998..e6df35c 100644
--- a/target-i386/helper.h
+++ b/target-i386/helper.h
@@ -186,6 +186,7 @@ DEF_HELPER_3(frstor, void, env, tl, int)
 DEF_HELPER_FLAGS_2(fxsave, TCG_CALL_NO_WG, void, env, tl)
 DEF_HELPER_FLAGS_2(fxrstor, TCG_CALL_NO_WG, void, env, tl)
 DEF_HELPER_FLAGS_3(xsave, TCG_CALL_NO_WG, void, env, tl, i64)
+DEF_HELPER_FLAGS_3(xsaveopt, TCG_CALL_NO_WG, void, env, tl, i64)
 DEF_HELPER_FLAGS_3(xrstor, TCG_CALL_NO_WG, void, env, tl, i64)
 DEF_HELPER_FLAGS_2(xgetbv, TCG_CALL_NO_WG, i64, env, i32)
 DEF_HELPER_FLAGS_3(xsetbv, TCG_CALL_NO_WG, void, env, i32, i64)
diff --git a/target-i386/translate.c b/target-i386/translate.c
index e9992aa..a98347f 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -133,6 +133,7 @@ typedef struct DisasContext {
     int cpuid_ext2_features;
     int cpuid_ext3_features;
     int cpuid_7_0_ebx_features;
+    int cpuid_xsave_features;
 } DisasContext;
 
 static void gen_eob(DisasContext *s);
@@ -7715,9 +7716,28 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
                 gen_helper_xrstor(cpu_env, cpu_A0, cpu_tmp1_i64);
             }
             break;
-        case 6: /* mfence */
-            if ((modrm & 0xc7) != 0xc0 || !(s->cpuid_features & CPUID_SSE2))
-                goto illegal_op;
+        case 6:
+            if (mod == 3) {
+                /* mfence */
+                if (!(s->cpuid_features & CPUID_SSE2)
+                    || (s->prefix & PREFIX_LOCK)) {
+                    goto illegal_op;
+                }
+                /* no-op */
+            } else {
+                /* xsaveopt */
+                if ((s->cpuid_ext_features & CPUID_EXT_XSAVE) == 0
+                    || (s->cpuid_xsave_features & CPUID_XSAVE_XSAVEOPT) == 0
+                    || (s->flags & HF_OSXSAVE_MASK) == 0
+                    || (s->prefix & (PREFIX_LOCK | PREFIX_DATA
+                                     | PREFIX_REPZ | PREFIX_REPNZ))) {
+                    goto illegal_op;
+                }
+                gen_lea_modrm(env, s, modrm);
+                tcg_gen_concat_tl_i64(cpu_tmp1_i64, cpu_regs[R_EAX],
+                                      cpu_regs[R_EDX]);
+                gen_helper_xsaveopt(cpu_env, cpu_A0, cpu_tmp1_i64);
+            }
             break;
         case 7: /* sfence / clflush */
             if ((modrm & 0xc7) == 0xc0) {
@@ -7917,6 +7937,7 @@ static inline void gen_intermediate_code_internal(X86CPU *cpu,
     dc->cpuid_ext2_features = env->features[FEAT_8000_0001_EDX];
     dc->cpuid_ext3_features = env->features[FEAT_8000_0001_ECX];
     dc->cpuid_7_0_ebx_features = env->features[FEAT_7_0_EBX];
+    dc->cpuid_xsave_features = env->features[FEAT_XSAVE];
 #ifdef TARGET_X86_64
     dc->lma = (flags >> HF_LMA_SHIFT) & 1;
     dc->code64 = (flags >> HF_CS64_SHIFT) & 1;
-- 
2.4.3

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

* [Qemu-devel] [PATCH 05/14] target-i386: Enable control registers for MPX
  2015-07-09  8:17 [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Richard Henderson
                   ` (3 preceding siblings ...)
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 04/14] target-i386: Implement XSAVEOPT Richard Henderson
@ 2015-07-09  8:17 ` Richard Henderson
  2015-07-09 13:12   ` Paolo Bonzini
                     ` (2 more replies)
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 06/14] target-i386: Perform set/reset_inhibit_irq inline Richard Henderson
                   ` (9 subsequent siblings)
  14 siblings, 3 replies; 31+ messages in thread
From: Richard Henderson @ 2015-07-09  8:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, ehabkost

Enable and disable at CPL changes, MSR changes, and XRSTOR changes.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-i386/Makefile.objs |  2 +-
 target-i386/cpu.c         | 18 +++++------
 target-i386/cpu.h         | 21 ++++++++++++-
 target-i386/fpu_helper.c  | 78 +++++++++++++++++++++++++++++++++++++++++++++--
 target-i386/helper.c      |  2 ++
 target-i386/kvm.c         |  5 +++
 target-i386/misc_helper.c |  9 ++++++
 target-i386/mpx_helper.c  | 51 +++++++++++++++++++++++++++++++
 target-i386/smm_helper.c  |  4 +++
 target-i386/translate.c   |  5 +++
 10 files changed, 179 insertions(+), 16 deletions(-)
 create mode 100644 target-i386/mpx_helper.c

diff --git a/target-i386/Makefile.objs b/target-i386/Makefile.objs
index 7a1df2c..2cb07e1 100644
--- a/target-i386/Makefile.objs
+++ b/target-i386/Makefile.objs
@@ -1,6 +1,6 @@
 obj-y += translate.o helper.o cpu.o
 obj-y += excp_helper.o fpu_helper.o cc_helper.o int_helper.o svm_helper.o
-obj-y += smm_helper.o misc_helper.o mem_helper.o seg_helper.o
+obj-y += smm_helper.o misc_helper.o mem_helper.o seg_helper.o mpx_helper.o
 obj-y += gdbstub.o
 obj-$(CONFIG_SOFTMMU) += machine.o arch_memory_mapping.o arch_dump.o
 obj-$(CONFIG_KVM) += kvm.o
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index b2fc59f..c19ff8a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -335,7 +335,8 @@ static const char *cpuid_xsave_feature_name[] = {
 #define TCG_SVM_FEATURES 0
 #define TCG_KVM_FEATURES 0
 #define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP | \
-          CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ADX)
+          CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ADX | \
+          CPUID_7_0_EBX_MPX)
           /* missing:
           CPUID_7_0_EBX_FSGSBASE, CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2,
           CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM,
@@ -433,12 +434,7 @@ static const X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = {
 };
 #undef REGISTER
 
-typedef struct ExtSaveArea {
-    uint32_t feature, bits;
-    uint32_t offset, size;
-} ExtSaveArea;
-
-static const ExtSaveArea ext_save_areas[] = {
+const ExtSaveArea x86_ext_save_areas[] = {
     [2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX,
             .offset = 0x240, .size = 0x100 },
     [3] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX,
@@ -2427,8 +2423,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
 
         if (count == 0) {
             *ecx = 0x240;
-            for (i = 2; i < ARRAY_SIZE(ext_save_areas); i++) {
-                const ExtSaveArea *esa = &ext_save_areas[i];
+            for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
+                const ExtSaveArea *esa = &x86_ext_save_areas[i];
                 if ((env->features[esa->feature] & esa->bits) == esa->bits
                     && ((ena_mask >> i) & 1) != 0) {
                     if (i < 32) {
@@ -2443,8 +2439,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             *ebx = *ecx;
         } else if (count == 1) {
             *eax = env->features[FEAT_XSAVE];
-        } else if (count < ARRAY_SIZE(ext_save_areas)) {
-            const ExtSaveArea *esa = &ext_save_areas[count];
+        } else if (count < ARRAY_SIZE(x86_ext_save_areas)) {
+            const ExtSaveArea *esa = &x86_ext_save_areas[count];
             if ((env->features[esa->feature] & esa->bits) == esa->bits
                 && ((ena_mask >> count) & 1) != 0) {
                 *eax = esa->size;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index a8153c0..8f0fc35 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -155,6 +155,9 @@
 #define HF_OSFXSR_SHIFT     22 /* CR4.OSFXSR */
 #define HF_SMAP_SHIFT       23 /* CR4.SMAP */
 #define HF_OSXSAVE_SHIFT    24 /* CR4.OSXSAVE */
+#define HF_MPX_EN_SHIFT     25 /* MPX Enabled (CR4+XCR0+BNDCFGx) */
+#define HF_MPX_PR_SHIFT     26 /* BNDCFGx.BNDPRESERVE */
+#define HF_MPX_IU_SHIFT     27 /* BND registers in-use */
 
 #define HF_CPL_MASK          (3 << HF_CPL_SHIFT)
 #define HF_SOFTMMU_MASK      (1 << HF_SOFTMMU_SHIFT)
@@ -179,6 +182,9 @@
 #define HF_OSFXSR_MASK       (1 << HF_OSFXSR_SHIFT)
 #define HF_SMAP_MASK         (1 << HF_SMAP_SHIFT)
 #define HF_OSXSAVE_MASK      (1 << HF_OSXSAVE_SHIFT)
+#define HF_MPX_EN_MASK       (1 << HF_MPX_EN_SHIFT)
+#define HF_MPX_PR_MASK       (1 << HF_MPX_PR_SHIFT)
+#define HF_MPX_IU_MASK       (1 << HF_MPX_IU_SHIFT)
 
 /* hflags2 */
 
@@ -741,6 +747,10 @@ typedef struct BNDCSReg {
     uint64_t sts;
 } BNDCSReg;
 
+#define BNDCFG_ENABLE       1ULL
+#define BNDCFG_BNDPRESERVE  2ULL
+#define BNDCFG_BDIR_MASK    TARGET_PAGE_MASK
+
 #ifdef HOST_WORDS_BIGENDIAN
 #define XMM_B(n) _b[63 - (n)]
 #define XMM_W(n) _w[31 - (n)]
@@ -1096,7 +1106,14 @@ void cpu_x86_frstor(CPUX86State *s, target_ulong ptr, int data32);
 int cpu_x86_signal_handler(int host_signum, void *pinfo,
                            void *puc);
 
-/* cpuid.c */
+/* cpu.c */
+typedef struct ExtSaveArea {
+    uint32_t feature, bits;
+    uint32_t offset, size;
+} ExtSaveArea;
+
+extern const ExtSaveArea x86_ext_save_areas[];
+
 void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
                    uint32_t *eax, uint32_t *ebx,
                    uint32_t *ecx, uint32_t *edx);
@@ -1336,6 +1353,8 @@ void x86_cpu_compat_set_features(const char *cpu_model, FeatureWord w,
 void x86_cpu_compat_kvm_no_autoenable(FeatureWord w, uint32_t features);
 void x86_cpu_compat_kvm_no_autodisable(FeatureWord w, uint32_t features);
 
+/* mpx_helper.c */
+void cpu_sync_bndcs_hf(CPUX86State *env);
 
 /* Return name of 32-bit register, from a R_* constant */
 const char *get_register_name_32(unsigned int reg);
diff --git a/target-i386/fpu_helper.c b/target-i386/fpu_helper.c
index d0b960c..c3cb218 100644
--- a/target-i386/fpu_helper.c
+++ b/target-i386/fpu_helper.c
@@ -1154,6 +1154,22 @@ static void do_xsave_sse(CPUX86State *env, target_ulong ptr)
     }
 }
 
+static void do_xsave_bndregs(CPUX86State *env, target_ulong addr)
+{
+    int i;
+
+    for (i = 0; i < 4; i++, addr += 16) {
+        cpu_stq_data(env, addr, env->bnd_regs[i].lb);
+        cpu_stq_data(env, addr + 8, env->bnd_regs[i].ub);
+    }
+}
+
+static void do_xsave_bndcsr(CPUX86State *env, target_ulong addr)
+{
+    cpu_stq_data(env, addr, env->bndcs_regs.cfgu);
+    cpu_stq_data(env, addr + 8, env->bndcs_regs.sts);
+}
+
 void helper_fxsave(CPUX86State *env, target_ulong ptr)
 {
     /* The operand must be 16 byte aligned */
@@ -1176,9 +1192,16 @@ void helper_fxsave(CPUX86State *env, target_ulong ptr)
 
 static uint64_t get_xinuse(CPUX86State *env)
 {
-    /* We don't track XINUSE.  We could calculate it here, but it's
-       probably less work to simply indicate all components in use.  */
-    return -1;
+    uint64_t inuse = -1;
+
+    /* For the most part, we don't track XINUSE.  We could calculate it
+       here for all components, but it's probably less work to simply
+       indicate in use.  That said, the state of BNDREGS is important
+       enough to track in HFLAGS, so we might as well use that here.  */
+    if ((env->hflags & HF_MPX_IU_MASK) == 0) {
+       inuse &= ~XSTATE_BNDREGS;
+    }
+    return inuse;
 }
 
 static void do_xsave(CPUX86State *env, target_ulong ptr,
@@ -1205,6 +1228,12 @@ static void do_xsave(CPUX86State *env, target_ulong ptr,
     if (opt & XSTATE_SSE) {
         do_xsave_sse(env, ptr);
     }
+    if (opt & XSTATE_BNDREGS) {
+        do_xsave_bndregs(env, ptr + x86_ext_save_areas[XSTATE_BNDREGS].offset);
+    }
+    if (opt & XSTATE_BNDCSR) {
+        do_xsave_bndcsr(env, ptr + x86_ext_save_areas[XSTATE_BNDCSR].offset);
+    }
 
     /* Update the XSTATE_BV field.  */
     old_bv = cpu_ldq_data(env, ptr + 512);
@@ -1270,6 +1299,24 @@ static void do_xrstor_sse(CPUX86State *env, target_ulong ptr)
     }
 }
 
+static void do_xrstor_bndregs(CPUX86State *env, target_ulong addr)
+{
+    int i;
+
+    for (i = 0; i < 4; i++, addr += 16) {
+        env->bnd_regs[i].lb = cpu_ldq_data(env, addr);
+        env->bnd_regs[i].ub = cpu_ldq_data(env, addr + 8);
+    }
+}
+
+static void do_xrstor_bndcsr(CPUX86State *env, target_ulong addr)
+{
+    /* FIXME: Extend highest implemented bit of linear address.  */
+    env->bndcs_regs.cfgu = cpu_ldq_data(env, addr);
+    env->bndcs_regs.sts = cpu_ldq_data(env, addr + 8);
+    cpu_sync_bndcs_hf(env);
+}
+
 void helper_fxrstor(CPUX86State *env, target_ulong ptr)
 {
     /* The operand must be 16 byte aligned */
@@ -1342,6 +1389,25 @@ void helper_xrstor(CPUX86State *env, target_ulong ptr, uint64_t rfbm)
             memset(env->xmm_regs, 0, sizeof(env->xmm_regs));
         }
     }
+    if (rfbm & XSTATE_BNDREGS) {
+        if (xstate_bv & XSTATE_BNDREGS) {
+            do_xrstor_bndregs(env,
+                              ptr + x86_ext_save_areas[XSTATE_BNDREGS].offset);
+            env->hflags |= HF_MPX_IU_MASK;
+        } else {
+            memset(env->bnd_regs, 0, sizeof(env->bnd_regs));
+            env->hflags &= ~HF_MPX_IU_MASK;
+        }
+    }
+    if (rfbm & XSTATE_BNDCSR) {
+        if (xstate_bv & XSTATE_BNDCSR) {
+            do_xrstor_bndcsr(env,
+                             ptr + x86_ext_save_areas[XSTATE_BNDCSR].offset);
+        } else {
+            memset(&env->bndcs_regs, 0, sizeof(env->bndcs_regs));
+        }
+        cpu_sync_bndcs_hf(env);
+    }
 }
 
 uint64_t helper_xgetbv(CPUX86State *env, uint32_t ecx)
@@ -1375,7 +1441,13 @@ void helper_xsetbv(CPUX86State *env, uint32_t ecx, uint64_t mask)
         goto do_gpf;
     }
 
+    /* Disallow enabling only half of MPX.  */
+    if ((mask ^ (mask * (XSTATE_BNDCSR / XSTATE_BNDREGS))) & XSTATE_BNDCSR) {
+        goto do_gpf;
+    }
+
     env->xcr0 = mask;
+    cpu_sync_bndcs_hf(env);
     return;
 
  do_gpf:
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 5e8e63d..c7edaa4 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -489,6 +489,8 @@ void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4)
 
     env->cr[4] = new_cr4;
     env->hflags = hflags;
+
+    cpu_sync_bndcs_hf(env);
 }
 
 #if defined(CONFIG_USER_ONLY)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index f057982..27ae029 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2186,6 +2186,11 @@ int kvm_arch_get_registers(CPUState *cs)
     if (ret < 0) {
         return ret;
     }
+
+    /* ??? HFLAGS may be out of sync if any of the above error out.
+       But there seems little point in recomputing this multiple times.  */
+    cpu_sync_bndcs_hf(&cpu->env);
+
     return 0;
 }
 
diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
index 52c5d65..1e1cc4a 100644
--- a/target-i386/misc_helper.c
+++ b/target-i386/misc_helper.c
@@ -394,6 +394,12 @@ void helper_wrmsr(CPUX86State *env)
     case MSR_IA32_MISC_ENABLE:
         env->msr_ia32_misc_enable = val;
         break;
+    case MSR_IA32_BNDCFGS:
+        /* FIXME: #GP if reserved bits are set.  */
+        /* FIXME: Extend highest implemented bit of linear address.  */
+        env->msr_bndcfgs = val;
+        cpu_sync_bndcs_hf(env);
+        break;
     default:
         if ((uint32_t)env->regs[R_ECX] >= MSR_MC0_CTL
             && (uint32_t)env->regs[R_ECX] < MSR_MC0_CTL +
@@ -539,6 +545,9 @@ void helper_rdmsr(CPUX86State *env)
     case MSR_IA32_MISC_ENABLE:
         val = env->msr_ia32_misc_enable;
         break;
+    case MSR_IA32_BNDCFGS:
+        val = env->msr_bndcfgs;
+        break;
     default:
         if ((uint32_t)env->regs[R_ECX] >= MSR_MC0_CTL
             && (uint32_t)env->regs[R_ECX] < MSR_MC0_CTL +
diff --git a/target-i386/mpx_helper.c b/target-i386/mpx_helper.c
new file mode 100644
index 0000000..decb2ea
--- /dev/null
+++ b/target-i386/mpx_helper.c
@@ -0,0 +1,51 @@
+/*
+ *  x86 MPX helpers
+ *
+ *  Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "cpu.h"
+#include "exec/helper-proto.h"
+#include "exec/cpu_ldst.h"
+
+
+void cpu_sync_bndcs_hf(CPUX86State *env)
+{
+    uint32_t hflags = env->hflags;
+    uint32_t bndcsr;
+
+    if ((hflags & HF_CPL_MASK) == 3) {
+        bndcsr = env->bndcs_regs.cfgu;
+    } else {
+        bndcsr = env->msr_bndcfgs;
+    }
+
+    if ((hflags & HF_OSXSAVE_MASK)
+        && (env->xcr0 & XSTATE_BNDCSR)
+        && (bndcsr & BNDCFG_ENABLE)) {
+        hflags |= HF_MPX_EN_MASK;
+    } else {
+        hflags &= ~HF_MPX_EN_MASK;
+    }
+
+    if (bndcsr & BNDCFG_BNDPRESERVE) {
+        hflags |= HF_MPX_PR_MASK;
+    } else {
+        hflags &= ~HF_MPX_PR_MASK;
+    }
+
+    env->hflags = hflags;
+}
diff --git a/target-i386/smm_helper.c b/target-i386/smm_helper.c
index 02e24b9..77fc1b8 100644
--- a/target-i386/smm_helper.c
+++ b/target-i386/smm_helper.c
@@ -97,6 +97,10 @@ void do_smm_enter(X86CPU *cpu)
     x86_stl_phys(cs, sm_state + 0x7e94, env->tr.limit);
     x86_stw_phys(cs, sm_state + 0x7e92, (env->tr.flags >> 8) & 0xf0ff);
 
+    /* ??? Vol 1, 16.5.6 Intel MPX and SMM says that IA32_BNDCFGS
+       is saved at offset 7ED0.  Vol 3, 34.4.1.1, Table 32-2, has
+       7EA0-7ED7 as "reserved".  What's this, and what's really
+       supposed to happen?  */
     x86_stq_phys(cs, sm_state + 0x7ed0, env->efer);
 
     x86_stq_phys(cs, sm_state + 0x7ff8, env->regs[R_EAX]);
diff --git a/target-i386/translate.c b/target-i386/translate.c
index a98347f..eb30401 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7714,6 +7714,11 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
                 tcg_gen_concat_tl_i64(cpu_tmp1_i64, cpu_regs[R_EAX],
                                       cpu_regs[R_EDX]);
                 gen_helper_xrstor(cpu_env, cpu_A0, cpu_tmp1_i64);
+                /* XRSTOR is how MPX is enabled, which changes how
+                   we translate.  Thus we need to end the TB.  */
+                gen_update_cc_op(s);
+                gen_jmp_im(s->pc - s->cs_base);
+                gen_eob(s);
             }
             break;
         case 6:
-- 
2.4.3

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

* [Qemu-devel] [PATCH 06/14] target-i386: Perform set/reset_inhibit_irq inline
  2015-07-09  8:17 [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Richard Henderson
                   ` (4 preceding siblings ...)
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 05/14] target-i386: Enable control registers for MPX Richard Henderson
@ 2015-07-09  8:17 ` Richard Henderson
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 07/14] target-i386: Split up gen_lea_modrm Richard Henderson
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2015-07-09  8:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, ehabkost

With helpers that can be reused for other things.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-i386/cc_helper.c | 10 ----------
 target-i386/helper.h    |  2 --
 target-i386/translate.c | 37 ++++++++++++++++++++++++++++---------
 3 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/target-i386/cc_helper.c b/target-i386/cc_helper.c
index ecbf0ec..1bf89aa 100644
--- a/target-i386/cc_helper.c
+++ b/target-i386/cc_helper.c
@@ -382,13 +382,3 @@ void helper_sti_vm(CPUX86State *env)
     }
 }
 #endif
-
-void helper_set_inhibit_irq(CPUX86State *env)
-{
-    env->hflags |= HF_INHIBIT_IRQ_MASK;
-}
-
-void helper_reset_inhibit_irq(CPUX86State *env)
-{
-    env->hflags &= ~HF_INHIBIT_IRQ_MASK;
-}
diff --git a/target-i386/helper.h b/target-i386/helper.h
index e6df35c..0c957bf 100644
--- a/target-i386/helper.h
+++ b/target-i386/helper.h
@@ -61,8 +61,6 @@ DEF_HELPER_1(cli, void, env)
 DEF_HELPER_1(sti, void, env)
 DEF_HELPER_1(clac, void, env)
 DEF_HELPER_1(stac, void, env)
-DEF_HELPER_1(set_inhibit_irq, void, env)
-DEF_HELPER_1(reset_inhibit_irq, void, env)
 DEF_HELPER_3(boundw, void, env, tl, int)
 DEF_HELPER_3(boundl, void, env, tl, int)
 DEF_HELPER_1(rsm, void, env)
diff --git a/target-i386/translate.c b/target-i386/translate.c
index eb30401..9303dc0 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -2393,14 +2393,36 @@ static void gen_debug(DisasContext *s, target_ulong cur_eip)
     s->is_jmp = DISAS_TB_JUMP;
 }
 
+static void gen_set_hflag(DisasContext *s, uint32_t mask)
+{
+    if ((s->flags & mask) == 0) {
+        TCGv_i32 t = tcg_temp_new_i32();
+        tcg_gen_ld_i32(t, cpu_env, offsetof(CPUX86State, hflags));
+        tcg_gen_ori_i32(t, t, mask);
+        tcg_gen_st_i32(t, cpu_env, offsetof(CPUX86State, hflags));
+        tcg_temp_free_i32(t);
+        s->flags |= mask;
+    }
+}
+
+static void gen_reset_hflag(DisasContext *s, uint32_t mask)
+{
+    if (s->flags & mask) {
+        TCGv_i32 t = tcg_temp_new_i32();
+        tcg_gen_ld_i32(t, cpu_env, offsetof(CPUX86State, hflags));
+        tcg_gen_andi_i32(t, t, ~mask);
+        tcg_gen_st_i32(t, cpu_env, offsetof(CPUX86State, hflags));
+        tcg_temp_free_i32(t);
+        s->flags &= ~mask;
+    }
+}
+
 /* generate a generic end of block. Trace exception is also generated
    if needed */
 static void gen_eob(DisasContext *s)
 {
     gen_update_cc_op(s);
-    if (s->tb->flags & HF_INHIBIT_IRQ_MASK) {
-        gen_helper_reset_inhibit_irq(cpu_env);
-    }
+    gen_reset_hflag(s, HF_INHIBIT_IRQ_MASK);
     if (s->tb->flags & HF_RF_MASK) {
         gen_helper_reset_rf(cpu_env);
     }
@@ -5190,8 +5212,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
             /* if reg == SS, inhibit interrupts/trace. */
             /* If several instructions disable interrupts, only the
                _first_ does it */
-            if (!(s->tb->flags & HF_INHIBIT_IRQ_MASK))
-                gen_helper_set_inhibit_irq(cpu_env);
+            gen_set_hflag(s, HF_INHIBIT_IRQ_MASK);
             s->tf = 0;
         }
         if (s->is_jmp) {
@@ -5258,8 +5279,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
             /* if reg == SS, inhibit interrupts/trace */
             /* If several instructions disable interrupts, only the
                _first_ does it */
-            if (!(s->tb->flags & HF_INHIBIT_IRQ_MASK))
-                gen_helper_set_inhibit_irq(cpu_env);
+            gen_set_hflag(s, HF_INHIBIT_IRQ_MASK);
             s->tf = 0;
         }
         if (s->is_jmp) {
@@ -6813,8 +6833,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
                 /* interruptions are enabled only the first insn after sti */
                 /* If several instructions disable interrupts, only the
                    _first_ does it */
-                if (!(s->tb->flags & HF_INHIBIT_IRQ_MASK))
-                    gen_helper_set_inhibit_irq(cpu_env);
+                gen_set_hflag(s, HF_INHIBIT_IRQ_MASK);
                 /* give a chance to handle pending irqs */
                 gen_jmp_im(s->pc - s->cs_base);
                 gen_eob(s);
-- 
2.4.3

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

* [Qemu-devel] [PATCH 07/14] target-i386: Split up gen_lea_modrm
  2015-07-09  8:17 [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Richard Henderson
                   ` (5 preceding siblings ...)
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 06/14] target-i386: Perform set/reset_inhibit_irq inline Richard Henderson
@ 2015-07-09  8:17 ` Richard Henderson
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 08/14] target-i386: Implement BNDMK Richard Henderson
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2015-07-09  8:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, ehabkost

This is immediately usable by lea and multi-byte nop,
and will be required to implement parts of the mpx spec.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-i386/translate.c | 199 +++++++++++++++++++++---------------------------
 1 file changed, 85 insertions(+), 114 deletions(-)

diff --git a/target-i386/translate.c b/target-i386/translate.c
index 9303dc0..ada0dec 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -1806,37 +1806,52 @@ static void gen_shifti(DisasContext *s1, int op, TCGMemOp ot, int d, int c)
     }
 }
 
-static void gen_lea_modrm(CPUX86State *env, DisasContext *s, int modrm)
+/* Decompose an address.  */
+
+typedef struct AddressParts {
+    int def_seg;
+    int base;
+    int index;
+    int scale;
+    target_long disp;
+} AddressParts;
+
+static AddressParts gen_lea_modrm_0(CPUX86State *env, DisasContext *s,
+                                    int modrm)
 {
+    int def_seg, base, index, scale, mod, rm;
     target_long disp;
-    int havesib, base, index, scale;
-    int mod, rm, code, def_seg, ovr_seg;
-    TCGv sum;
+    bool havesib;
 
     def_seg = R_DS;
-    ovr_seg = s->override;
+    index = -1;
+    scale = 0;
+    disp = 0;
+
     mod = (modrm >> 6) & 3;
     rm = modrm & 7;
+    base = rm | REX_B(s);
+
+    if (mod == 3) {
+        /* Normally filtered out earlier, but including this path
+           simplifies multi-byte nop, as well as bndcl, bndcu, bndcn.  */
+        goto done;
+    }
 
     switch (s->aflag) {
     case MO_64:
     case MO_32:
         havesib = 0;
-        base = rm;
-        index = -1;
-        scale = 0;
-
-        if (base == 4) {
-            havesib = 1;
-            code = cpu_ldub_code(env, s->pc++);
+        if (rm == 4) {
+            int code = cpu_ldub_code(env, s->pc++);
             scale = (code >> 6) & 3;
             index = ((code >> 3) & 7) | REX_X(s);
             if (index == 4) {
                 index = -1;  /* no index */
             }
-            base = (code & 7);
+            base = (code & 7) | REX_B(s);
+            havesib = 1;
         }
-        base |= REX_B(s);
 
         switch (mod) {
         case 0:
@@ -1845,10 +1860,9 @@ static void gen_lea_modrm(CPUX86State *env, DisasContext *s, int modrm)
                 disp = (int32_t)cpu_ldl_code(env, s->pc);
                 s->pc += 4;
                 if (CODE64(s) && !havesib) {
+                    base = -2;
                     disp += s->pc + s->rip_offset;
                 }
-            } else {
-                disp = 0;
             }
             break;
         case 1:
@@ -1865,46 +1879,19 @@ static void gen_lea_modrm(CPUX86State *env, DisasContext *s, int modrm)
         if (base == R_ESP && s->popl_esp_hack) {
             disp += s->popl_esp_hack;
         }
-
-        /* Compute the address, with a minimum number of TCG ops.  */
-        TCGV_UNUSED(sum);
-        if (index >= 0) {
-            if (scale == 0) {
-                sum = cpu_regs[index];
-            } else {
-                tcg_gen_shli_tl(cpu_A0, cpu_regs[index], scale);
-                sum = cpu_A0;
-            }
-            if (base >= 0) {
-                tcg_gen_add_tl(cpu_A0, sum, cpu_regs[base]);
-                sum = cpu_A0;
-            }
-        } else if (base >= 0) {
-            sum = cpu_regs[base];
-        }
-        if (TCGV_IS_UNUSED(sum)) {
-            tcg_gen_movi_tl(cpu_A0, disp);
-            sum = cpu_A0;
-        } else if (disp != 0) {
-            tcg_gen_addi_tl(cpu_A0, sum, disp);
-            sum = cpu_A0;
-        }
-
         if (base == R_EBP || base == R_ESP) {
             def_seg = R_SS;
         }
         break;
 
     case MO_16:
-        sum = cpu_A0;
         if (mod == 0) {
             if (rm == 6) {
+                base = -1;
                 disp = cpu_lduw_code(env, s->pc);
                 s->pc += 2;
-                tcg_gen_movi_tl(cpu_A0, disp);
                 break;
             }
-            disp = 0;
         } else if (mod == 1) {
             disp = (int8_t)cpu_ldub_code(env, s->pc++);
         } else {
@@ -1914,102 +1901,89 @@ static void gen_lea_modrm(CPUX86State *env, DisasContext *s, int modrm)
 
         switch (rm) {
         case 0:
-            tcg_gen_add_tl(cpu_A0, cpu_regs[R_EBX], cpu_regs[R_ESI]);
+            base = R_EBX;
+            index = R_ESI;
             break;
         case 1:
-            tcg_gen_add_tl(cpu_A0, cpu_regs[R_EBX], cpu_regs[R_EDI]);
+            base = R_EBX;
+            index = R_EDI;
             break;
         case 2:
-            tcg_gen_add_tl(cpu_A0, cpu_regs[R_EBP], cpu_regs[R_ESI]);
+            base = R_EBP;
+            index = R_ESI;
             def_seg = R_SS;
             break;
         case 3:
-            tcg_gen_add_tl(cpu_A0, cpu_regs[R_EBP], cpu_regs[R_EDI]);
+            base = R_EBP;
+            index = R_EDI;
             def_seg = R_SS;
             break;
         case 4:
-            sum = cpu_regs[R_ESI];
+            base = R_ESI;
             break;
         case 5:
-            sum = cpu_regs[R_EDI];
+            base = R_EDI;
             break;
         case 6:
-            sum = cpu_regs[R_EBP];
+            base = R_EBP;
             def_seg = R_SS;
             break;
         default:
         case 7:
-            sum = cpu_regs[R_EBX];
+            base = R_EBX;
             break;
         }
-        if (disp != 0) {
-            tcg_gen_addi_tl(cpu_A0, sum, disp);
-            sum = cpu_A0;
-        }
         break;
 
     default:
         tcg_abort();
     }
 
-    gen_lea_v_seg(s, s->aflag, sum, def_seg, ovr_seg);
+ done:
+    return (AddressParts){ def_seg, base, index, scale, disp };
 }
 
-static void gen_nop_modrm(CPUX86State *env, DisasContext *s, int modrm)
+/* Compute the address, with a minimum number of TCG ops.  */
+static TCGv gen_lea_modrm_1(AddressParts a)
 {
-    int mod, rm, base, code;
-
-    mod = (modrm >> 6) & 3;
-    if (mod == 3)
-        return;
-    rm = modrm & 7;
-
-    switch (s->aflag) {
-    case MO_64:
-    case MO_32:
-        base = rm;
+    TCGv ea;
 
-        if (base == 4) {
-            code = cpu_ldub_code(env, s->pc++);
-            base = (code & 7);
+    TCGV_UNUSED(ea);
+    if (a.index >= 0) {
+        if (a.scale == 0) {
+            ea = cpu_regs[a.index];
+        } else {
+            tcg_gen_shli_tl(cpu_A0, cpu_regs[a.index], a.scale);
+            ea = cpu_A0;
         }
-
-        switch (mod) {
-        case 0:
-            if (base == 5) {
-                s->pc += 4;
-            }
-            break;
-        case 1:
-            s->pc++;
-            break;
-        default:
-        case 2:
-            s->pc += 4;
-            break;
+        if (a.base >= 0) {
+            tcg_gen_add_tl(cpu_A0, ea, cpu_regs[a.base]);
+            ea = cpu_A0;
         }
-        break;
+    } else if (a.base >= 0) {
+        ea = cpu_regs[a.base];
+    }
+    if (TCGV_IS_UNUSED(ea)) {
+        tcg_gen_movi_tl(cpu_A0, a.disp);
+        ea = cpu_A0;
+    } else if (a.disp != 0) {
+        tcg_gen_addi_tl(cpu_A0, ea, a.disp);
+        ea = cpu_A0;
+    }
 
-    case MO_16:
-        switch (mod) {
-        case 0:
-            if (rm == 6) {
-                s->pc += 2;
-            }
-            break;
-        case 1:
-            s->pc++;
-            break;
-        default:
-        case 2:
-            s->pc += 2;
-            break;
-        }
-        break;
+    return ea;
+}
 
-    default:
-        tcg_abort();
-    }
+static void gen_lea_modrm(CPUX86State *env, DisasContext *s, int modrm)
+{
+    AddressParts a = gen_lea_modrm_0(env, s, modrm);
+    TCGv ea = gen_lea_modrm_1(a);
+    gen_lea_v_seg(s, s->aflag, ea, a.def_seg, s->override);
+}
+
+static void gen_nop_modrm(CPUX86State *env, DisasContext *s, int modrm)
+{
+    (void)gen_lea_modrm_0(env, s, modrm);
 }
 
 /* used for LEA and MOV AX, mem */
@@ -5345,19 +5319,16 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
         break;
 
     case 0x8d: /* lea */
-        ot = dflag;
         modrm = cpu_ldub_code(env, s->pc++);
         mod = (modrm >> 6) & 3;
         if (mod == 3)
             goto illegal_op;
         reg = ((modrm >> 3) & 7) | rex_r;
-        /* we must ensure that no segment is added */
-        s->override = -1;
-        val = s->addseg;
-        s->addseg = 0;
-        gen_lea_modrm(env, s, modrm);
-        s->addseg = val;
-        gen_op_mov_reg_v(ot, reg, cpu_A0);
+        {
+            AddressParts a = gen_lea_modrm_0(env, s, modrm);
+            TCGv ea = gen_lea_modrm_1(a);
+            gen_op_mov_reg_v(dflag, reg, ea);
+        }
         break;
 
     case 0xa0: /* mov EAX, Ov */
-- 
2.4.3

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

* [Qemu-devel] [PATCH 08/14] target-i386: Implement BNDMK
  2015-07-09  8:17 [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Richard Henderson
                   ` (6 preceding siblings ...)
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 07/14] target-i386: Split up gen_lea_modrm Richard Henderson
@ 2015-07-09  8:17 ` Richard Henderson
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 09/14] target-i386: Implement BNDMOV Richard Henderson
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2015-07-09  8:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, ehabkost

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-i386/translate.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/target-i386/translate.c b/target-i386/translate.c
index ada0dec..69bb6c6 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -75,6 +75,8 @@ static TCGv cpu_cc_dst, cpu_cc_src, cpu_cc_src2, cpu_cc_srcT;
 static TCGv_i32 cpu_cc_op;
 static TCGv cpu_regs[CPU_NB_REGS];
 static TCGv cpu_seg_base[6];
+static TCGv_i64 cpu_bndl[4];
+static TCGv_i64 cpu_bndu[4];
 /* local temps */
 static TCGv cpu_T0, cpu_T1;
 /* local register indexes (only used inside old micro ops) */
@@ -7514,7 +7516,41 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
             break;
         }
         break;
-    case 0x119 ... 0x11f: /* nop (multi byte) */
+    case 0x11b:
+        modrm = cpu_ldub_code(env, s->pc++);
+        if (s->flags & HF_MPX_EN_MASK) {
+            mod = (modrm >> 6) & 3;
+            reg = ((modrm >> 3) & 7) | rex_r;
+            if (mod != 3 && (prefixes & PREFIX_REPZ)) {
+                /* bndmk */
+                if (reg >= 4
+                    || (prefixes & PREFIX_LOCK)
+                    || s->aflag == MO_16) {
+                    goto illegal_op;
+                }
+                AddressParts a = gen_lea_modrm_0(env, s, modrm);
+                if (a.base >= 0) {
+                    tcg_gen_extu_tl_i64(cpu_bndl[reg], cpu_regs[a.base]);
+                } else if (a.base == -1) {
+                    /* no base register has lower bound of 0 */
+                    tcg_gen_movi_i64(cpu_bndl[reg], 0);
+                } else {
+                    /* rip-relative generates #ud */
+                    goto illegal_op;
+                }
+                tcg_gen_not_tl(cpu_A0, gen_lea_modrm_1(a));
+                if (!CODE64(s)) {
+                    tcg_gen_ext32u_tl(cpu_A0, cpu_A0);
+                }
+                tcg_gen_extu_tl_i64(cpu_bndu[reg], cpu_A0);
+                /* bnd registers are now in-use */
+                gen_set_hflag(s, HF_MPX_IU_MASK);
+                break;
+            }
+        }
+        gen_nop_modrm(env, s, modrm);
+        break;
+    case 0x119: case 0x11a: case 0x11c ... 0x11f: /* nop (multi byte) */
         modrm = cpu_ldub_code(env, s->pc++);
         gen_nop_modrm(env, s, modrm);
         break;
@@ -7857,6 +7893,12 @@ void optimize_flags_init(void)
         [R_GS] = "gs_base",
         [R_SS] = "ss_base",
     };
+    static const char bnd_regl_names[4][8] = {
+        "bnd0_lb", "bnd1_lb", "bnd2_lb", "bnd3_lb"
+    };
+    static const char bnd_regu_names[4][8] = {
+        "bnd0_ub", "bnd1_ub", "bnd2_ub", "bnd3_ub"
+    };
     int i;
 
     cpu_env = tcg_global_reg_new_ptr(TCG_AREG0, "env");
@@ -7881,6 +7923,16 @@ void optimize_flags_init(void)
                                  offsetof(CPUX86State, segs[i].base),
                                  seg_base_names[i]);
     }
+    for (i = 0; i < 4; ++i) {
+        cpu_bndl[i]
+            = tcg_global_mem_new_i64(TCG_AREG0,
+                                     offsetof(CPUX86State, bnd_regs[i].lb),
+                                     bnd_regl_names[i]);
+        cpu_bndu[i]
+            = tcg_global_mem_new_i64(TCG_AREG0,
+                                     offsetof(CPUX86State, bnd_regs[i].ub),
+                                     bnd_regu_names[i]);
+    }
 }
 
 /* generate intermediate code in gen_opc_buf and gen_opparam_buf for
-- 
2.4.3

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

* [Qemu-devel] [PATCH 09/14] target-i386: Implement BNDMOV
  2015-07-09  8:17 [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Richard Henderson
                   ` (7 preceding siblings ...)
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 08/14] target-i386: Implement BNDMK Richard Henderson
@ 2015-07-09  8:17 ` Richard Henderson
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 10/14] target-i386: Implement BNDCL, BNDCU, BNDCN Richard Henderson
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2015-07-09  8:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, ehabkost

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-i386/translate.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/target-i386/translate.c b/target-i386/translate.c
index 69bb6c6..fcafa81 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7516,6 +7516,47 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
             break;
         }
         break;
+    case 0x11a:
+        modrm = cpu_ldub_code(env, s->pc++);
+        if (s->flags & HF_MPX_EN_MASK) {
+            mod = (modrm >> 6) & 3;
+            reg = ((modrm >> 3) & 7) | rex_r;
+            if (prefixes & PREFIX_DATA) {
+                /* bndmov -- from reg/mem */
+                if (reg >= 4 || s->aflag == MO_16) {
+                    goto illegal_op;
+                }
+                if (mod == 3) {
+                    int reg2 = (modrm & 7) | REX_B(s);
+                    if (reg2 >= 4 || (prefixes & PREFIX_LOCK)) {
+                        goto illegal_op;
+                    }
+                    if (s->flags & HF_MPX_IU_MASK) {
+                        tcg_gen_mov_i64(cpu_bndl[reg], cpu_bndl[reg2]);
+                        tcg_gen_mov_i64(cpu_bndu[reg], cpu_bndu[reg2]);
+                    }
+                } else {
+                    gen_lea_modrm(env, s, modrm);
+                    if (CODE64(s)) {
+                        tcg_gen_qemu_ld_i64(cpu_bndl[reg], cpu_A0,
+                                            s->mem_index, MO_LEQ);
+                        tcg_gen_addi_tl(cpu_A0, cpu_A0, 8);
+                        tcg_gen_qemu_ld_i64(cpu_bndu[reg], cpu_A0,
+                                            s->mem_index, MO_LEQ);
+                    } else {
+                        tcg_gen_qemu_ld_i64(cpu_bndl[reg], cpu_A0,
+                                            s->mem_index, MO_LEUL);
+                        tcg_gen_addi_tl(cpu_A0, cpu_A0, 4);
+                        tcg_gen_qemu_ld_i64(cpu_bndu[reg], cpu_A0,
+                                            s->mem_index, MO_LEUL);
+                    }
+                    /* bnd registers are now in-use */
+                    gen_set_hflag(s, HF_MPX_IU_MASK);
+                }
+            }
+        }
+        gen_nop_modrm(env, s, modrm);
+        break;
     case 0x11b:
         modrm = cpu_ldub_code(env, s->pc++);
         if (s->flags & HF_MPX_EN_MASK) {
@@ -7546,11 +7587,41 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
                 /* bnd registers are now in-use */
                 gen_set_hflag(s, HF_MPX_IU_MASK);
                 break;
+            } else if (prefixes & PREFIX_DATA) {
+                /* bndmov -- to reg/mem */
+                if (reg >= 4 || s->aflag == MO_16) {
+                    goto illegal_op;
+                }
+                if (mod == 3) {
+                    int reg2 = (modrm & 7) | REX_B(s);
+                    if (reg2 >= 4 || (prefixes & PREFIX_LOCK)) {
+                        goto illegal_op;
+                    }
+                    if (s->flags & HF_MPX_IU_MASK) {
+                        tcg_gen_mov_i64(cpu_bndl[reg2], cpu_bndl[reg]);
+                        tcg_gen_mov_i64(cpu_bndu[reg2], cpu_bndu[reg]);
+                    }
+                } else {
+                    gen_lea_modrm(env, s, modrm);
+                    if (CODE64(s)) {
+                        tcg_gen_qemu_st_i64(cpu_bndl[reg], cpu_A0,
+                                            s->mem_index, MO_LEQ);
+                        tcg_gen_addi_tl(cpu_A0, cpu_A0, 8);
+                        tcg_gen_qemu_st_i64(cpu_bndu[reg], cpu_A0,
+                                            s->mem_index, MO_LEQ);
+                    } else {
+                        tcg_gen_qemu_st_i64(cpu_bndl[reg], cpu_A0,
+                                            s->mem_index, MO_LEUL);
+                        tcg_gen_addi_tl(cpu_A0, cpu_A0, 4);
+                        tcg_gen_qemu_st_i64(cpu_bndu[reg], cpu_A0,
+                                            s->mem_index, MO_LEUL);
+                    }
+                }
             }
         }
         gen_nop_modrm(env, s, modrm);
         break;
-    case 0x119: case 0x11a: case 0x11c ... 0x11f: /* nop (multi byte) */
+    case 0x119: case 0x11c ... 0x11f: /* nop (multi byte) */
         modrm = cpu_ldub_code(env, s->pc++);
         gen_nop_modrm(env, s, modrm);
         break;
-- 
2.4.3

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

* [Qemu-devel] [PATCH 10/14] target-i386: Implement BNDCL, BNDCU, BNDCN
  2015-07-09  8:17 [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Richard Henderson
                   ` (8 preceding siblings ...)
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 09/14] target-i386: Implement BNDMOV Richard Henderson
@ 2015-07-09  8:17 ` Richard Henderson
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 11/14] target-i386: Update BNDSTATUS for exceptions raised by BOUND Richard Henderson
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2015-07-09  8:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, ehabkost

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-i386/helper.h     |  2 ++
 target-i386/mpx_helper.c |  8 ++++++++
 target-i386/translate.c  | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/target-i386/helper.h b/target-i386/helper.h
index 0c957bf..331457f 100644
--- a/target-i386/helper.h
+++ b/target-i386/helper.h
@@ -16,6 +16,8 @@ DEF_HELPER_2(divq_EAX, void, env, tl)
 DEF_HELPER_2(idivq_EAX, void, env, tl)
 #endif
 
+DEF_HELPER_FLAGS_2(bndck, TCG_CALL_NO_WG, void, env, i32)
+
 DEF_HELPER_2(aam, void, env, int)
 DEF_HELPER_2(aad, void, env, int)
 DEF_HELPER_1(aaa, void, env)
diff --git a/target-i386/mpx_helper.c b/target-i386/mpx_helper.c
index decb2ea..172a4d2 100644
--- a/target-i386/mpx_helper.c
+++ b/target-i386/mpx_helper.c
@@ -49,3 +49,11 @@ void cpu_sync_bndcs_hf(CPUX86State *env)
 
     env->hflags = hflags;
 }
+
+void helper_bndck(CPUX86State *env, uint32_t fail)
+{
+    if (unlikely(fail)) {
+        env->bndcs_regs.sts = 1;
+        raise_exception(env, EXCP05_BOUND);
+    }
+}
diff --git a/target-i386/translate.c b/target-i386/translate.c
index fcafa81..05796cc 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -1988,6 +1988,23 @@ static void gen_nop_modrm(CPUX86State *env, DisasContext *s, int modrm)
     (void)gen_lea_modrm_0(env, s, modrm);
 }
 
+/* Used for BNDCL, BNDCU, BNDCN.  */
+static void gen_bndck(CPUX86State *env, DisasContext *s, int modrm,
+                      TCGCond cond, TCGv_i64 bndv, target_ulong pc_start)
+{
+    TCGv ea = gen_lea_modrm_1(gen_lea_modrm_0(env, s, modrm));
+
+    tcg_gen_extu_tl_i64(cpu_tmp1_i64, ea);
+    if (!CODE64(s)) {
+        tcg_gen_ext32u_i64(cpu_tmp1_i64, cpu_tmp1_i64);
+    }
+    tcg_gen_setcond_i64(cond, cpu_tmp1_i64, cpu_tmp1_i64, bndv);
+    tcg_gen_trunc_i64_i32(cpu_tmp2_i32, cpu_tmp1_i64);
+    gen_update_cc_op(s);
+    gen_jmp_im(pc_start - s->cs_base);
+    gen_helper_bndck(cpu_env, cpu_tmp2_i32);
+}
+
 /* used for LEA and MOV AX, mem */
 static void gen_add_A0_ds_seg(DisasContext *s)
 {
@@ -7521,7 +7538,26 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
         if (s->flags & HF_MPX_EN_MASK) {
             mod = (modrm >> 6) & 3;
             reg = ((modrm >> 3) & 7) | rex_r;
-            if (prefixes & PREFIX_DATA) {
+            if (prefixes & PREFIX_REPZ) {
+                /* bndcl */
+                if (reg >= 4
+                    || (prefixes & PREFIX_LOCK)
+                    || s->aflag == MO_16) {
+                    goto illegal_op;
+                }
+                gen_bndck(env, s, modrm, TCG_COND_LTU, cpu_bndl[reg], pc_start);
+            } else if (prefixes & PREFIX_REPNZ) {
+                /* bndcu */
+                if (reg >= 4
+                    || (prefixes & PREFIX_LOCK)
+                    || s->aflag == MO_16) {
+                    goto illegal_op;
+                }
+                TCGv_i64 notu = tcg_temp_new_i64();
+                tcg_gen_not_i64(notu, cpu_bndu[reg]);
+                gen_bndck(env, s, modrm, TCG_COND_GTU, notu, pc_start);
+                tcg_temp_free_i64(notu);
+            } else if (prefixes & PREFIX_DATA) {
                 /* bndmov -- from reg/mem */
                 if (reg >= 4 || s->aflag == MO_16) {
                     goto illegal_op;
@@ -7587,6 +7623,14 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
                 /* bnd registers are now in-use */
                 gen_set_hflag(s, HF_MPX_IU_MASK);
                 break;
+            } else if (prefixes & PREFIX_REPNZ) {
+                /* bndcn */
+                if (reg >= 4
+                    || (prefixes & PREFIX_LOCK)
+                    || s->aflag == MO_16) {
+                    goto illegal_op;
+                }
+                gen_bndck(env, s, modrm, TCG_COND_GTU, cpu_bndu[reg], pc_start);
             } else if (prefixes & PREFIX_DATA) {
                 /* bndmov -- to reg/mem */
                 if (reg >= 4 || s->aflag == MO_16) {
-- 
2.4.3

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

* [Qemu-devel] [PATCH 11/14] target-i386: Update BNDSTATUS for exceptions raised by BOUND
  2015-07-09  8:17 [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Richard Henderson
                   ` (9 preceding siblings ...)
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 10/14] target-i386: Implement BNDCL, BNDCU, BNDCN Richard Henderson
@ 2015-07-09  8:17 ` Richard Henderson
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 12/14] target-i386: Implement BNDLDX, BNDSTX Richard Henderson
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2015-07-09  8:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, ehabkost

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-i386/mem_helper.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target-i386/mem_helper.c b/target-i386/mem_helper.c
index 1aec8a5..c8d5fac 100644
--- a/target-i386/mem_helper.c
+++ b/target-i386/mem_helper.c
@@ -91,6 +91,9 @@ void helper_boundw(CPUX86State *env, target_ulong a0, int v)
     high = cpu_ldsw_data(env, a0 + 2);
     v = (int16_t)v;
     if (v < low || v > high) {
+        if (env->hflags & HF_MPX_EN_MASK) {
+            env->bndcs_regs.sts = 0;
+        }
         raise_exception(env, EXCP05_BOUND);
     }
 }
@@ -102,6 +105,9 @@ void helper_boundl(CPUX86State *env, target_ulong a0, int v)
     low = cpu_ldl_data(env, a0);
     high = cpu_ldl_data(env, a0 + 4);
     if (v < low || v > high) {
+        if (env->hflags & HF_MPX_EN_MASK) {
+            env->bndcs_regs.sts = 0;
+        }
         raise_exception(env, EXCP05_BOUND);
     }
 }
-- 
2.4.3

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

* [Qemu-devel] [PATCH 12/14] target-i386: Implement BNDLDX, BNDSTX
  2015-07-09  8:17 [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Richard Henderson
                   ` (10 preceding siblings ...)
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 11/14] target-i386: Update BNDSTATUS for exceptions raised by BOUND Richard Henderson
@ 2015-07-09  8:17 ` Richard Henderson
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 13/14] target-i386: Clear bndregs during legacy near jumps Richard Henderson
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2015-07-09  8:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, ehabkost

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-i386/helper.h     |  4 +++
 target-i386/mpx_helper.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++
 target-i386/translate.c  | 61 +++++++++++++++++++++++++++++++
 3 files changed, 158 insertions(+)

diff --git a/target-i386/helper.h b/target-i386/helper.h
index 331457f..48c2e23 100644
--- a/target-i386/helper.h
+++ b/target-i386/helper.h
@@ -17,6 +17,10 @@ DEF_HELPER_2(idivq_EAX, void, env, tl)
 #endif
 
 DEF_HELPER_FLAGS_2(bndck, TCG_CALL_NO_WG, void, env, i32)
+DEF_HELPER_FLAGS_3(bndldx32, TCG_CALL_NO_WG, i64, env, tl, tl)
+DEF_HELPER_FLAGS_3(bndldx64, TCG_CALL_NO_WG, i64, env, tl, tl)
+DEF_HELPER_FLAGS_5(bndstx32, TCG_CALL_NO_WG, void, env, tl, tl, i64, i64)
+DEF_HELPER_FLAGS_5(bndstx64, TCG_CALL_NO_WG, void, env, tl, tl, i64, i64)
 
 DEF_HELPER_2(aam, void, env, int)
 DEF_HELPER_2(aad, void, env, int)
diff --git a/target-i386/mpx_helper.c b/target-i386/mpx_helper.c
index 172a4d2..0ac40bd 100644
--- a/target-i386/mpx_helper.c
+++ b/target-i386/mpx_helper.c
@@ -57,3 +57,96 @@ void helper_bndck(CPUX86State *env, uint32_t fail)
         raise_exception(env, EXCP05_BOUND);
     }
 }
+
+static uint64_t lookup_bte64(CPUX86State *env, uint64_t base)
+{
+    uint64_t bndcsr, bde, bt;
+
+    if ((env->hflags & HF_CPL_MASK) == 3) {
+        bndcsr = env->bndcs_regs.cfgu;
+    } else {
+        bndcsr = env->msr_bndcfgs;
+    }
+
+    bde = (extract64(base, 20, 28) << 3) + (extract64(bndcsr, 20, 44) << 12);
+    bt = cpu_ldq_data(env, bde);
+    if ((bt & 1) == 0) {
+        env->bndcs_regs.sts = bde | 2;
+        raise_exception(env, EXCP05_BOUND);
+    }
+
+    return (extract64(base, 3, 17) << 5) + (bt & ~7);
+}
+
+static uint32_t lookup_bte32(CPUX86State *env, uint32_t base)
+{
+    uint32_t bndcsr, bde, bt;
+
+    if ((env->hflags & HF_CPL_MASK) == 3) {
+        bndcsr = env->bndcs_regs.cfgu;
+    } else {
+        bndcsr = env->msr_bndcfgs;
+    }
+
+    bde = (extract32(base, 12, 20) << 2) + (bndcsr & TARGET_PAGE_MASK);
+    bt = cpu_ldl_data(env, bde);
+    if ((bt & 1) == 0) {
+        env->bndcs_regs.sts = bde | 2;
+        raise_exception(env, EXCP05_BOUND);
+    }
+
+    return (extract32(base, 2, 10) << 4) + (bt & ~3);
+}
+
+uint64_t helper_bndldx64(CPUX86State *env, target_ulong base, target_ulong ptr)
+{
+    uint64_t bte, lb, ub, pt;
+
+    bte = lookup_bte64(env, base);
+    lb = cpu_ldq_data(env, bte);
+    ub = cpu_ldq_data(env, bte + 8);
+    pt = cpu_ldq_data(env, bte + 16);
+
+    if (pt != ptr) {
+        lb = ub = 0;
+    }
+    env->mmx_t0.q = ub;
+    return lb;
+}
+
+uint64_t helper_bndldx32(CPUX86State *env, target_ulong base, target_ulong ptr)
+{
+    uint32_t bte, lb, ub, pt;
+
+    bte = lookup_bte32(env, base);
+    lb = cpu_ldl_data(env, bte);
+    ub = cpu_ldl_data(env, bte + 4);
+    pt = cpu_ldl_data(env, bte + 8);
+
+    if (pt != ptr) {
+        lb = ub = 0;
+    }
+    return ((uint64_t)ub << 32) | lb;
+}
+
+void helper_bndstx64(CPUX86State *env, target_ulong base, target_ulong ptr,
+                     uint64_t lb, uint64_t ub)
+{
+    uint64_t bte;
+
+    bte = lookup_bte64(env, base);
+    cpu_stq_data(env, bte, lb);
+    cpu_stq_data(env, bte + 8, ub);
+    cpu_stq_data(env, bte + 16, ptr);
+}
+
+void helper_bndstx32(CPUX86State *env, target_ulong base, target_ulong ptr,
+                     uint64_t lb, uint64_t ub)
+{
+    uint32_t bte;
+
+    bte = lookup_bte32(env, base);
+    cpu_stl_data(env, bte, lb);
+    cpu_stl_data(env, bte + 4, ub);
+    cpu_stl_data(env, bte + 8, ptr);
+}
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 05796cc..7892951 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7589,6 +7589,38 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
                     /* bnd registers are now in-use */
                     gen_set_hflag(s, HF_MPX_IU_MASK);
                 }
+            } else if (mod != 3) {
+                /* bndldx */
+                AddressParts a = gen_lea_modrm_0(env, s, modrm);
+                if (reg >= 4
+                    || (prefixes & PREFIX_LOCK)
+                    || s->aflag == MO_16
+                    || a.base < -1) {
+                    goto illegal_op;
+                }
+                if (a.base >= 0) {
+                    tcg_gen_addi_tl(cpu_A0, cpu_regs[a.base], a.disp);
+                } else {
+                    tcg_gen_movi_tl(cpu_A0, 0);
+                }
+                gen_lea_v_seg(s, s->aflag, cpu_A0, a.def_seg, s->override);
+                if (a.index >= 0) {
+                    tcg_gen_mov_tl(cpu_T0, cpu_regs[a.index]);
+                } else {
+                    tcg_gen_movi_tl(cpu_T0, 0);
+                }
+                gen_update_cc_op(s);
+                gen_jmp_im(pc_start - s->cs_base);
+                if (CODE64(s)) {
+                    gen_helper_bndldx64(cpu_bndl[reg], cpu_env, cpu_A0, cpu_T0);
+                    tcg_gen_ld_i64(cpu_bndu[reg], cpu_env,
+                                   offsetof(CPUX86State, mmx_t0.q));
+                } else {
+                    gen_helper_bndldx32(cpu_bndu[reg], cpu_env, cpu_A0, cpu_T0);
+                    tcg_gen_ext32u_i64(cpu_bndl[reg], cpu_bndu[reg]);
+                    tcg_gen_shri_i64(cpu_bndu[reg], cpu_bndu[reg], 32);
+                }
+                gen_set_hflag(s, HF_MPX_IU_MASK);
             }
         }
         gen_nop_modrm(env, s, modrm);
@@ -7661,6 +7693,35 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
                                             s->mem_index, MO_LEUL);
                     }
                 }
+            } else if (mod != 3) {
+                /* bndstx */
+                AddressParts a = gen_lea_modrm_0(env, s, modrm);
+                if (reg >= 4
+                    || (prefixes & PREFIX_LOCK)
+                    || s->aflag == MO_16
+                    || a.base < -1) {
+                    goto illegal_op;
+                }
+                if (a.base >= 0) {
+                    tcg_gen_addi_tl(cpu_A0, cpu_regs[a.base], a.disp);
+                } else {
+                    tcg_gen_movi_tl(cpu_A0, 0);
+                }
+                gen_lea_v_seg(s, s->aflag, cpu_A0, a.def_seg, s->override);
+                if (a.index >= 0) {
+                    tcg_gen_mov_tl(cpu_T0, cpu_regs[a.index]);
+                } else {
+                    tcg_gen_movi_tl(cpu_T0, 0);
+                }
+                gen_update_cc_op(s);
+                gen_jmp_im(pc_start - s->cs_base);
+                if (CODE64(s)) {
+                    gen_helper_bndstx64(cpu_env, cpu_A0, cpu_T0,
+                                        cpu_bndl[reg], cpu_bndu[reg]);
+                } else {
+                    gen_helper_bndstx32(cpu_env, cpu_A0, cpu_T0,
+                                        cpu_bndl[reg], cpu_bndu[reg]);
+                }
             }
         }
         gen_nop_modrm(env, s, modrm);
-- 
2.4.3

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

* [Qemu-devel] [PATCH 13/14] target-i386: Clear bndregs during legacy near jumps
  2015-07-09  8:17 [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Richard Henderson
                   ` (11 preceding siblings ...)
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 12/14] target-i386: Implement BNDLDX, BNDSTX Richard Henderson
@ 2015-07-09  8:17 ` Richard Henderson
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 14/14] target-i386: Enable XCR0 features for user-mode Richard Henderson
  2015-11-17 17:43 ` [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Paolo Bonzini
  14 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2015-07-09  8:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, ehabkost

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-i386/translate.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/target-i386/translate.c b/target-i386/translate.c
index 7892951..f067d98 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -2410,6 +2410,24 @@ static void gen_reset_hflag(DisasContext *s, uint32_t mask)
     }
 }
 
+/* Clear BND registers during legacy branches.  */
+static void gen_bnd_jmp(DisasContext *s)
+{
+    /* Do nothing if BND prefix present, MPX is disabled, BNDPRESERVE is set,
+       or if the BNDREGs are known to be in INIT state already.  */
+    if ((s->prefix & PREFIX_REPNZ) == 0
+        && (s->flags & HF_MPX_EN_MASK) == 0
+        && (s->flags & HF_MPX_PR_MASK) == 0
+        && (s->flags & HF_MPX_IU_MASK) == 0) {
+        int i;
+        for (i = 0; i < 4; ++i) {
+            tcg_gen_movi_i64(cpu_bndl[i], 0);
+            tcg_gen_movi_i64(cpu_bndu[i], 0);
+        }
+        gen_reset_hflag(s, HF_MPX_IU_MASK);
+    }
+}
+
 /* generate a generic end of block. Trace exception is also generated
    if needed */
 static void gen_eob(DisasContext *s)
@@ -4832,6 +4850,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
             tcg_gen_movi_tl(cpu_T1, next_eip);
             gen_push_v(s, cpu_T1);
             gen_op_jmp_v(cpu_T0);
+            gen_bnd_jmp(s);
             gen_eob(s);
             break;
         case 3: /* lcall Ev */
@@ -4859,6 +4878,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
                 tcg_gen_ext16u_tl(cpu_T0, cpu_T0);
             }
             gen_op_jmp_v(cpu_T0);
+            gen_bnd_jmp(s);
             gen_eob(s);
             break;
         case 5: /* ljmp Ev */
@@ -6260,6 +6280,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
         gen_stack_update(s, val + (1 << ot));
         /* Note that gen_pop_T0 uses a zero-extending load.  */
         gen_op_jmp_v(cpu_T0);
+        gen_bnd_jmp(s);
         gen_eob(s);
         break;
     case 0xc3: /* ret */
@@ -6267,6 +6288,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
         gen_pop_update(s, ot);
         /* Note that gen_pop_T0 uses a zero-extending load.  */
         gen_op_jmp_v(cpu_T0);
+        gen_bnd_jmp(s);
         gen_eob(s);
         break;
     case 0xca: /* lret im */
@@ -6335,6 +6357,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
             }
             tcg_gen_movi_tl(cpu_T0, next_eip);
             gen_push_v(s, cpu_T0);
+            gen_bnd_jmp(s);
             gen_jmp(s, tval);
         }
         break;
@@ -6364,6 +6387,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
         } else if (!CODE64(s)) {
             tval &= 0xffffffff;
         }
+        gen_bnd_jmp(s);
         gen_jmp(s, tval);
         break;
     case 0xea: /* ljmp im */
@@ -6403,6 +6427,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
         if (dflag == MO_16) {
             tval &= 0xffff;
         }
+        gen_bnd_jmp(s);
         gen_jcc(s, b, tval, next_eip);
         break;
 
-- 
2.4.3

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

* [Qemu-devel] [PATCH 14/14] target-i386: Enable XCR0 features for user-mode
  2015-07-09  8:17 [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Richard Henderson
                   ` (12 preceding siblings ...)
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 13/14] target-i386: Clear bndregs during legacy near jumps Richard Henderson
@ 2015-07-09  8:17 ` Richard Henderson
  2015-07-09 13:15   ` Paolo Bonzini
  2015-11-17 17:43 ` [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Paolo Bonzini
  14 siblings, 1 reply; 31+ messages in thread
From: Richard Henderson @ 2015-07-09  8:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, ehabkost

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-i386/cpu.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index c19ff8a..2402c2a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2654,7 +2654,17 @@ static void x86_cpu_reset(CPUState *s)
     cpu_set_fpuc(env, 0x37f);
 
     env->mxcsr = 0x1f80;
-    env->xstate_bv = XSTATE_FP | XSTATE_SSE;
+
+    /* ??? This variable is somewhat silly.  Methinks KVM should be
+       using XCR0 to store into the XSTATE_BV field.  Either that or
+       there's more missing information, e.g. the AVX bits.  */
+    env->xstate_bv = XSTATE_FP;
+    if (env->features[FEAT_1_EDX] & CPUID_SSE) {
+        env->xstate_bv |= XSTATE_SSE;
+    }
+    if (env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_MPX) {
+        env->xstate_bv |= XSTATE_BNDREGS | XSTATE_BNDCSR;
+    }
 
     env->pat = 0x0007040600070406ULL;
     env->msr_ia32_misc_enable = MSR_IA32_MISC_ENABLE_DEFAULT;
@@ -2665,7 +2675,15 @@ static void x86_cpu_reset(CPUState *s)
     cpu_breakpoint_remove_all(s, BP_CPU);
     cpu_watchpoint_remove_all(s, BP_CPU);
 
-    env->xcr0 = 1;
+    env->xcr0 = XSTATE_FP;
+
+#ifdef CONFIG_USER_ONLY
+    /* Enable all the features for user-mode.  */
+    env->xcr0 = env->xstate_bv;
+    if (env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE) {
+        cpu_x86_update_cr4(env, CR4_OSFXSR_MASK | CR4_OSXSAVE_MASK);
+    }
+#endif
 
     /*
      * SDM 11.11.5 requires:
-- 
2.4.3

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

* Re: [Qemu-devel] [PATCH 04/14] target-i386: Implement XSAVEOPT
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 04/14] target-i386: Implement XSAVEOPT Richard Henderson
@ 2015-07-09 13:06   ` Paolo Bonzini
  2015-07-10  7:00     ` Richard Henderson
  0 siblings, 1 reply; 31+ messages in thread
From: Paolo Bonzini @ 2015-07-09 13:06 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: ehabkost



On 09/07/2015 10:17, Richard Henderson wrote:
> @@ -405,7 +405,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>      },
>      [FEAT_XSAVE] = {
>          .feat_names = cpuid_xsave_feature_name,
> -        .cpuid_eax = 0xd,
> +        .cpuid_eax = CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XGETBV1,

KVM does implements CPUID_XSAVE_XSAVEC though.  Should you set
.tcg_features instead?

Paolo

>          .cpuid_needs_ecx = true, .cpuid_ecx = 1,
>          .cpuid_reg = R_EAX,
>          .tcg_features = 0,

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

* Re: [Qemu-devel] [PATCH 05/14] target-i386: Enable control registers for MPX
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 05/14] target-i386: Enable control registers for MPX Richard Henderson
@ 2015-07-09 13:12   ` Paolo Bonzini
  2015-07-09 13:18   ` Paolo Bonzini
  2016-02-09 13:28   ` Paolo Bonzini
  2 siblings, 0 replies; 31+ messages in thread
From: Paolo Bonzini @ 2015-07-09 13:12 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: ehabkost



On 09/07/2015 10:17, Richard Henderson wrote:
> +    /* ??? Vol 1, 16.5.6 Intel MPX and SMM says that IA32_BNDCFGS
> +       is saved at offset 7ED0.  Vol 3, 34.4.1.1, Table 32-2, has
> +       7EA0-7ED7 as "reserved".  What's this, and what's really
> +       supposed to happen?  */
>      x86_stq_phys(cs, sm_state + 0x7ed0, env->efer);

The format that QEMU (and KVM) are using is based on AMD's format for
the state save area.  In my copy of the AMD manual this is Table 10-1.

Reserved areas in there are 0xfef0-0xfefb and 0xff04-0xff1f.

I definitely should update KVM to save/restore BNDCFGS.  Thanks for the
heads up!

Paolo

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

* Re: [Qemu-devel] [PATCH 14/14] target-i386: Enable XCR0 features for user-mode
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 14/14] target-i386: Enable XCR0 features for user-mode Richard Henderson
@ 2015-07-09 13:15   ` Paolo Bonzini
  2015-07-10  7:24     ` Richard Henderson
  0 siblings, 1 reply; 31+ messages in thread
From: Paolo Bonzini @ 2015-07-09 13:15 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: ehabkost



On 09/07/2015 10:17, Richard Henderson wrote:
> +
> +    /* ??? This variable is somewhat silly.  Methinks KVM should be
> +       using XCR0 to store into the XSTATE_BV field.  Either that or
> +       there's more missing information, e.g. the AVX bits.  */
> +    env->xstate_bv = XSTATE_FP;
> +    if (env->features[FEAT_1_EDX] & CPUID_SSE) {
> +        env->xstate_bv |= XSTATE_SSE;
> +    }
> +    if (env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_MPX) {
> +        env->xstate_bv |= XSTATE_BNDREGS | XSTATE_BNDCSR;
> +    }

xstate_bv != xcr0 if the kernel is using XSAVEOPT and some of the values
were in the initial state.  Legacy state is never optimized, hence the
value of env->xstate_bv after reset.  So I think this hunk is wrong,
while the other is correct.

Paolo

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

* Re: [Qemu-devel] [PATCH 03/14] target-i386: Add XSAVE extension
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 03/14] target-i386: Add XSAVE extension Richard Henderson
@ 2015-07-09 13:16   ` Paolo Bonzini
  0 siblings, 0 replies; 31+ messages in thread
From: Paolo Bonzini @ 2015-07-09 13:16 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: ehabkost



On 09/07/2015 10:17, Richard Henderson wrote:
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index daced5c..f057982 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -1541,16 +1541,22 @@ static int kvm_get_sregs(X86CPU *cpu)
>  #define HFLAG_COPY_MASK \
>      ~( HF_CPL_MASK | HF_PE_MASK | HF_MP_MASK | HF_EM_MASK | \
>         HF_TS_MASK | HF_TF_MASK | HF_VM_MASK | HF_IOPL_MASK | \
> -       HF_OSFXSR_MASK | HF_LMA_MASK | HF_CS32_MASK | \
> +       HF_OSFXSR_MASK | HF_OSXSAVE_MASK | HF_LMA_MASK | HF_CS32_MASK | \
>         HF_SS32_MASK | HF_CS64_MASK | HF_ADDSEG_MASK)
>  
> -    hflags = (env->segs[R_SS].flags >> DESC_DPL_SHIFT) & HF_CPL_MASK;
> +    hflags = env->hflags & HFLAG_COPY_MASK;
> +    hflags |= (env->segs[R_SS].flags >> DESC_DPL_SHIFT) & HF_CPL_MASK;
>      hflags |= (env->cr[0] & CR0_PE_MASK) << (HF_PE_SHIFT - CR0_PE_SHIFT);
>      hflags |= (env->cr[0] << (HF_MP_SHIFT - CR0_MP_SHIFT)) &
>                  (HF_MP_MASK | HF_EM_MASK | HF_TS_MASK);
>      hflags |= (env->eflags & (HF_TF_MASK | HF_VM_MASK | HF_IOPL_MASK));
> -    hflags |= (env->cr[4] & CR4_OSFXSR_MASK) <<
> -                (HF_OSFXSR_SHIFT - CR4_OSFXSR_SHIFT);
> +
> +    if (env->cr[4] & CR4_OSFXSR_MASK) {
> +        hflags |= HF_OSFXSR_MASK;
> +    }
> +    if (env->cr[4] & CR4_OSXSAVE_MASK) {
> +        hflags |= HF_OSXSAVE_MASK;
> +    }
>  
>      if (env->efer & MSR_EFER_LMA) {
>          hflags |= HF_LMA_MASK;
> @@ -1571,7 +1577,7 @@ static int kvm_get_sregs(X86CPU *cpu)
>                          env->segs[R_SS].base) != 0) << HF_ADDSEG_SHIFT;
>          }
>      }
> -    env->hflags = (env->hflags & HFLAG_COPY_MASK) | hflags;
> +    env->hflags = hflags;
>  
>      return 0;
>  }

These KVM bits look good.

Paolo

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

* Re: [Qemu-devel] [PATCH 05/14] target-i386: Enable control registers for MPX
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 05/14] target-i386: Enable control registers for MPX Richard Henderson
  2015-07-09 13:12   ` Paolo Bonzini
@ 2015-07-09 13:18   ` Paolo Bonzini
  2015-07-10  7:44     ` Richard Henderson
  2016-02-09 13:28   ` Paolo Bonzini
  2 siblings, 1 reply; 31+ messages in thread
From: Paolo Bonzini @ 2015-07-09 13:18 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: ehabkost



On 09/07/2015 10:17, Richard Henderson wrote:
> 
> +void cpu_sync_bndcs_hf(CPUX86State *env)

s/hf/hflags/ :)

> +{
> +    uint32_t hflags = env->hflags;
> +    uint32_t bndcsr;
> +
> +    if ((hflags & HF_CPL_MASK) == 3) {
> +        bndcsr = env->bndcs_regs.cfgu;
> +    } else {
> +        bndcsr = env->msr_bndcfgs;
> +    }
> +
> +    if ((hflags & HF_OSXSAVE_MASK)
> +        && (env->xcr0 & XSTATE_BNDCSR)
> +        && (bndcsr & BNDCFG_ENABLE)) {
> +        hflags |= HF_MPX_EN_MASK;
> +    } else {
> +        hflags &= ~HF_MPX_EN_MASK;
> +    }
> +
> +    if (bndcsr & BNDCFG_BNDPRESERVE) {
> +        hflags |= HF_MPX_PR_MASK;
> +    } else {
> +        hflags &= ~HF_MPX_PR_MASK;
> +    }
> +
> +    env->hflags = hflags;
> +}

> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index f057982..27ae029 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -2186,6 +2186,11 @@ int kvm_arch_get_registers(CPUState *cs)
>      if (ret < 0) {
>          return ret;
>      }
> +
> +    /* ??? HFLAGS may be out of sync if any of the above error out.
> +       But there seems little point in recomputing this multiple times.  */
> +    cpu_sync_bndcs_hf(&cpu->env);

Why aren't you just using a goto, like

    if (ret < 0) {
        goto out;
    }
    ret = 0;
out:
    cpu_sync_bndcs_hf(&cpu->env);
    return ret;

>      return 0;

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

* Re: [Qemu-devel] [PATCH 04/14] target-i386: Implement XSAVEOPT
  2015-07-09 13:06   ` Paolo Bonzini
@ 2015-07-10  7:00     ` Richard Henderson
  0 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2015-07-10  7:00 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: ehabkost

On 07/09/2015 02:06 PM, Paolo Bonzini wrote:
>
>
> On 09/07/2015 10:17, Richard Henderson wrote:
>> @@ -405,7 +405,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>>       },
>>       [FEAT_XSAVE] = {
>>           .feat_names = cpuid_xsave_feature_name,
>> -        .cpuid_eax = 0xd,
>> +        .cpuid_eax = CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XGETBV1,
>
> KVM does implements CPUID_XSAVE_XSAVEC though.  Should you set
> .tcg_features instead?

Oops, yes of course -- 0xd is the selector here.


r~

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

* Re: [Qemu-devel] [PATCH 14/14] target-i386: Enable XCR0 features for user-mode
  2015-07-09 13:15   ` Paolo Bonzini
@ 2015-07-10  7:24     ` Richard Henderson
  2015-07-10  9:36       ` Paolo Bonzini
  0 siblings, 1 reply; 31+ messages in thread
From: Richard Henderson @ 2015-07-10  7:24 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: ehabkost

On 07/09/2015 02:15 PM, Paolo Bonzini wrote:
> On 09/07/2015 10:17, Richard Henderson wrote:
>> +
>> +    /* ??? This variable is somewhat silly.  Methinks KVM should be
>> +       using XCR0 to store into the XSTATE_BV field.  Either that or
>> +       there's more missing information, e.g. the AVX bits.  */
>> +    env->xstate_bv = XSTATE_FP;
>> +    if (env->features[FEAT_1_EDX] & CPUID_SSE) {
>> +        env->xstate_bv |= XSTATE_SSE;
>> +    }
>> +    if (env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_MPX) {
>> +        env->xstate_bv |= XSTATE_BNDREGS | XSTATE_BNDCSR;
>> +    }
>
> xstate_bv != xcr0 if the kernel is using XSAVEOPT and some of the values
> were in the initial state.  Legacy state is never optimized, hence the
> value of env->xstate_bv after reset.  So I think this hunk is wrong,
> while the other is correct.

Yes, it's a copy of the field of the same name from the xsave format.

Have we stopped using tcg entirely when kvm is enabled?  I guess so, since I 
seem to recall an effort to build qemu without tcg support.  So any fears about 
tcg corrupting kvm state would be unfounded, right?

If so, I can see how this variable aids initial xsave construction as well as 
copying the xsave block during migration.

It does beg the question of why xstate_bv isn't zero at reset.  Surely all of 
the xmm and fpu registers are in INIT state at this time, and that's what the 
XRSTOR that will consume this block is going to care about.


r~

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

* Re: [Qemu-devel] [PATCH 05/14] target-i386: Enable control registers for MPX
  2015-07-09 13:18   ` Paolo Bonzini
@ 2015-07-10  7:44     ` Richard Henderson
  0 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2015-07-10  7:44 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: ehabkost

On 07/09/2015 02:18 PM, Paolo Bonzini wrote:
>
>
> On 09/07/2015 10:17, Richard Henderson wrote:
>>
>> +void cpu_sync_bndcs_hf(CPUX86State *env)
>
> s/hf/hflags/ :)

Heh.  Done.

> Why aren't you just using a goto, like
>
>      if (ret < 0) {
>          goto out;
>      }
>      ret = 0;
> out:
>      cpu_sync_bndcs_hf(&cpu->env);
>      return ret;

Doh.  Done.  Thanks,


r~

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

* Re: [Qemu-devel] [PATCH 14/14] target-i386: Enable XCR0 features for user-mode
  2015-07-10  7:24     ` Richard Henderson
@ 2015-07-10  9:36       ` Paolo Bonzini
  0 siblings, 0 replies; 31+ messages in thread
From: Paolo Bonzini @ 2015-07-10  9:36 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: ehabkost



On 10/07/2015 09:24, Richard Henderson wrote:
> On 07/09/2015 02:15 PM, Paolo Bonzini wrote:
>> On 09/07/2015 10:17, Richard Henderson wrote:
>>> +
>>> +    /* ??? This variable is somewhat silly.  Methinks KVM should be
>>> +       using XCR0 to store into the XSTATE_BV field.  Either that or
>>> +       there's more missing information, e.g. the AVX bits.  */
>>> +    env->xstate_bv = XSTATE_FP;
>>> +    if (env->features[FEAT_1_EDX] & CPUID_SSE) {
>>> +        env->xstate_bv |= XSTATE_SSE;
>>> +    }
>>> +    if (env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_MPX) {
>>> +        env->xstate_bv |= XSTATE_BNDREGS | XSTATE_BNDCSR;
>>> +    }
>>
>> xstate_bv != xcr0 if the kernel is using XSAVEOPT and some of the values
>> were in the initial state.  Legacy state is never optimized, hence the
>> value of env->xstate_bv after reset.  So I think this hunk is wrong,
>> while the other is correct.
> 
> Yes, it's a copy of the field of the same name from the xsave format.
> 
> Have we stopped using tcg entirely when kvm is enabled?

Yes, for about 8 years. :)

> I guess so,
> since I seem to recall an effort to build qemu without tcg support.  So
> any fears about tcg corrupting kvm state would be unfounded, right?
> 
> If so, I can see how this variable aids initial xsave construction as
> well as copying the xsave block during migration.
> 
> It does beg the question of why xstate_bv isn't zero at reset.  Surely
> all of the xmm and fpu registers are in INIT state at this time, and
> that's what the XRSTOR that will consume this block is going to care about.

That's a bug.  I was somehow convinced that XSAVEOPT never optimized the
FP and SSE state, but that's nonsense.

Paolo

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

* Re: [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension
  2015-07-09  8:17 [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Richard Henderson
                   ` (13 preceding siblings ...)
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 14/14] target-i386: Enable XCR0 features for user-mode Richard Henderson
@ 2015-11-17 17:43 ` Paolo Bonzini
  2015-11-18  9:43   ` Richard Henderson
  14 siblings, 1 reply; 31+ messages in thread
From: Paolo Bonzini @ 2015-11-17 17:43 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: ehabkost



On 09/07/2015 10:17, Richard Henderson wrote:
> I'm still in the process of testing this, as there's no code
> written for it yet and hardware to compare against doesn't
> start shipping until (probably) August.
> 
> But in the meantime there are a number of holes that I found
> in XSAVE support that might affect KVM, and one question wrt
> SMM support that affects MPX.  So I thought I'd get some 
> feedback on this sooner than later.
> 
> This patch set depends on the addressing cleanup patchset that
> I just posted.  It ought to depend on Pavel Dovgalyuk's exception
> handling cleanup patchset, but I haven't included that in my tree.
> 
> Comments?

Hi Richard, it would be nice to have these patches---or at least the
XSAVE support---in 2.6.  I also have a PKRU implementation for TCG, but
currently I'm only implementing RDPKRU/WRPKRU because I would like to
build the XSAVE support on top of your patches.

Regarding SMM support, there are three ways to go:

1) pester Intel some more so that they disclose the format of the SMM
state save area;

2) just place BNDCFGS at a random offset that is left as reserved in
AMD's manual;

3) do not save BNDCFGS at all since no one uses it anyway. *shrug*

The holes in the computation of KVM's hflags are probably harmless, but
nice to have anyway.  Thanks for fixing them.  Are there others that I
missed?

Paolo

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

* Re: [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension
  2015-11-17 17:43 ` [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Paolo Bonzini
@ 2015-11-18  9:43   ` Richard Henderson
  2015-11-18 10:13     ` Paolo Bonzini
  0 siblings, 1 reply; 31+ messages in thread
From: Richard Henderson @ 2015-11-18  9:43 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: ehabkost

On 11/17/2015 06:43 PM, Paolo Bonzini wrote:
> Hi Richard, it would be nice to have these patches---or at least the
> XSAVE support---in 2.6.  I also have a PKRU implementation for TCG, but
> currently I'm only implementing RDPKRU/WRPKRU because I would like to
> build the XSAVE support on top of your patches.

Sure.  I'll see about updating that branch this weekend.

> Regarding SMM support, there are three ways to go:
>
> 1) pester Intel some more so that they disclose the format of the SMM
> state save area;

They have done so, and relatively well.  Section 34.4.1.1 of the software 
developer's manual (I'm looking at 325462-055, June 2015).

The issue, perhaps, is that the Intel and AMD layouts are totally different. 
Now, given that we've been using the AMD layout with Intel emulations maybe 
that means that it really doesn't matter what layout we use, so long as we're 
self-consistent.

Is there anything besides BIOS code that runs in SMM anyway?  Do we have to be 
compatible with anything besides SeaBIOS in this area?

> 2) just place BNDCFGS at a random offset that is left as reserved in
> AMD's manual;
>
> 3) do not save BNDCFGS at all since no one uses it anyway. *shrug*

I'm not a fan of 3 simply because it means that one can't experiment with it, 
since turning it on means either that SMM produces weird results or kernel 
state gets corrupted.

> The holes in the computation of KVM's hflags are probably harmless, but
> nice to have anyway.  Thanks for fixing them.  Are there others that I
> missed?

Not that I saw.


r~

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

* Re: [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension
  2015-11-18  9:43   ` Richard Henderson
@ 2015-11-18 10:13     ` Paolo Bonzini
  0 siblings, 0 replies; 31+ messages in thread
From: Paolo Bonzini @ 2015-11-18 10:13 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: ehabkost



On 18/11/2015 10:43, Richard Henderson wrote:
>> 1) pester Intel some more so that they disclose the format of the SMM
>> state save area;
> 
> They have done so, and relatively well.  Section 34.4.1.1 of the
> software developer's manual (I'm looking at 325462-055, June 2015).

Relatively well unfortunately is not enough.  Unlike AMD, they do not
document where the descriptor cache is, which we need to implement SMM
save and restore.

> The issue, perhaps, is that the Intel and AMD layouts are totally
> different. Now, given that we've been using the AMD layout with Intel
> emulations maybe that means that it really doesn't matter what layout we
> use, so long as we're self-consistent.
> 
> Is there anything besides BIOS code that runs in SMM anyway?  Do we have
> to be compatible with anything besides SeaBIOS in this area?

There's OVMF, whose maintainers would really like the SMM state save
area to be a superset of the documented format.  They have grudgingly
accepted that we used AMD's format, which is completely different.  But
if we used Intel's format and did not put the descriptor cache at the
right place, then the next field Intel adds may overlap our descriptor
cache fields; we would be back with the same problem.

I would just place BNDCFGS at a random offset that is left as reserved
in AMD's manual.  Since we are at it, we might also find a home for EFER
in the 32-bit case, because it is used when LM is 0 but NX is 1.

Paolo

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

* Re: [Qemu-devel] [PATCH 05/14] target-i386: Enable control registers for MPX
  2015-07-09  8:17 ` [Qemu-devel] [PATCH 05/14] target-i386: Enable control registers for MPX Richard Henderson
  2015-07-09 13:12   ` Paolo Bonzini
  2015-07-09 13:18   ` Paolo Bonzini
@ 2016-02-09 13:28   ` Paolo Bonzini
  2016-02-09 15:50     ` Eric Blake
  2016-02-09 19:08     ` Richard Henderson
  2 siblings, 2 replies; 31+ messages in thread
From: Paolo Bonzini @ 2016-02-09 13:28 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: ehabkost

On 09/07/2015 10:17, Richard Henderson wrote:
> +    /* Disallow enabling only half of MPX.  */
> +    if ((mask ^ (mask * (XSTATE_BNDCSR / XSTATE_BNDREGS))) & XSTATE_BNDCSR) {

I'm refreshing patches 1-4 to add PKE support, and this caught my eye...

What about just

	if (!!(mask & XSTATE_BNDCSR) != !!(mask & XSTATE_BNDREGS))

?

Paolo

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

* Re: [Qemu-devel] [PATCH 05/14] target-i386: Enable control registers for MPX
  2016-02-09 13:28   ` Paolo Bonzini
@ 2016-02-09 15:50     ` Eric Blake
  2016-02-09 15:50       ` Paolo Bonzini
  2016-02-09 19:08     ` Richard Henderson
  1 sibling, 1 reply; 31+ messages in thread
From: Eric Blake @ 2016-02-09 15:50 UTC (permalink / raw)
  To: Paolo Bonzini, Richard Henderson, qemu-devel; +Cc: ehabkost

[-- Attachment #1: Type: text/plain, Size: 589 bytes --]

On 02/09/2016 06:28 AM, Paolo Bonzini wrote:
> On 09/07/2015 10:17, Richard Henderson wrote:
>> +    /* Disallow enabling only half of MPX.  */
>> +    if ((mask ^ (mask * (XSTATE_BNDCSR / XSTATE_BNDREGS))) & XSTATE_BNDCSR) {
> 
> I'm refreshing patches 1-4 to add PKE support, and this caught my eye...
> 
> What about just
> 
> 	if (!!(mask & XSTATE_BNDCSR) != !!(mask & XSTATE_BNDREGS))

Or even:

    if (!(mask & XSTATE_BNDCSR) != !(mask & XSTATE_BNDREGS))


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

* Re: [Qemu-devel] [PATCH 05/14] target-i386: Enable control registers for MPX
  2016-02-09 15:50     ` Eric Blake
@ 2016-02-09 15:50       ` Paolo Bonzini
  0 siblings, 0 replies; 31+ messages in thread
From: Paolo Bonzini @ 2016-02-09 15:50 UTC (permalink / raw)
  To: Eric Blake, Richard Henderson, qemu-devel; +Cc: ehabkost

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256



On 09/02/2016 16:50, Eric Blake wrote:
>>> What about just
>>> 
>>> if (!!(mask & XSTATE_BNDCSR) != !!(mask & XSTATE_BNDREGS))
> Or even:
> 
> if (!(mask & XSTATE_BNDCSR) != !(mask & XSTATE_BNDREGS))
> 

This is more mysterious. :)

Paolo
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJWugrcAAoJEL/70l94x66DYRMH/3+xoIVWaY4x36J4KCO97Qbt
enRqt+3LHVf8c8lDPmlQoVeGv6AUV8UEztrHEE61lL9S11zsqLTIJ7VWuhfS4K8B
BzJa9PZ+JFLmLj7p0qMU0HyqQ6j8Db5AyaqUUWqCALy2GFspX8vGkY6mvtjyKJP2
xxZ4nb0ESiEBsDeyuEff0sC7s5N2TlPmLVn9Vp2WdlTDPIsPKchRy3XaLGZ07mkh
LT+ymbyqbq20v7rbLGZoxBnGmsE2RbR+OfkAAa+fqHWZo3kfkMusRCeRCRKTWpix
6jhZJjc1X9+mCAEsobuxOMRwZkZP+4kZv3JYCod7zSuORgK4b+RbMwTfM5Row6U=
=FIiX
-----END PGP SIGNATURE-----

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

* Re: [Qemu-devel] [PATCH 05/14] target-i386: Enable control registers for MPX
  2016-02-09 13:28   ` Paolo Bonzini
  2016-02-09 15:50     ` Eric Blake
@ 2016-02-09 19:08     ` Richard Henderson
  1 sibling, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2016-02-09 19:08 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: ehabkost

On 02/10/2016 12:28 AM, Paolo Bonzini wrote:
> On 09/07/2015 10:17, Richard Henderson wrote:
>> +    /* Disallow enabling only half of MPX.  */
>> +    if ((mask ^ (mask * (XSTATE_BNDCSR / XSTATE_BNDREGS))) & XSTATE_BNDCSR) {
>
> I'm refreshing patches 1-4 to add PKE support, and this caught my eye...
>
> What about just
>
> 	if (!!(mask & XSTATE_BNDCSR) != !!(mask & XSTATE_BNDREGS))
>
> ?

Hmph.  Missed optimization for your version.
It is a bit more readable though, I admit...


r~



void bar();
void foo(unsigned mask)
{
   if ((mask ^ (mask * 4)) & 8) {
     bar();
   }
}
void baz(unsigned mask)
{
   if (!!(mask & 8) != !!(mask & 2)) {
     bar();
   }
}


foo:
         leal    0(,%rdi,4), %eax
         xorl    %eax, %edi
         andl    $8, %edi
         jne     .L4
	...
baz:
         movl    %edi, %eax
         shrl    %edi
         shrl    $3, %eax
         andl    $1, %edi
         andl    $1, %eax
         cmpb    %dil, %al
         je      .L5
	...
clang_baz:
         movl    %edi, %eax
         shrl    $3, %eax
         shrl    %edi
         xorl    %eax, %edi
         testb   $1, %dil
         jne     .LBB1_2

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

end of thread, other threads:[~2016-02-09 19:09 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-09  8:17 [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Richard Henderson
2015-07-09  8:17 ` [Qemu-devel] [PATCH 01/14] target-i386: Split fxsave/fxrstor implementation Richard Henderson
2015-07-09  8:17 ` [Qemu-devel] [PATCH 02/14] target-i386: Rearrange processing of 0F 01 Richard Henderson
2015-07-09  8:17 ` [Qemu-devel] [PATCH 03/14] target-i386: Add XSAVE extension Richard Henderson
2015-07-09 13:16   ` Paolo Bonzini
2015-07-09  8:17 ` [Qemu-devel] [PATCH 04/14] target-i386: Implement XSAVEOPT Richard Henderson
2015-07-09 13:06   ` Paolo Bonzini
2015-07-10  7:00     ` Richard Henderson
2015-07-09  8:17 ` [Qemu-devel] [PATCH 05/14] target-i386: Enable control registers for MPX Richard Henderson
2015-07-09 13:12   ` Paolo Bonzini
2015-07-09 13:18   ` Paolo Bonzini
2015-07-10  7:44     ` Richard Henderson
2016-02-09 13:28   ` Paolo Bonzini
2016-02-09 15:50     ` Eric Blake
2016-02-09 15:50       ` Paolo Bonzini
2016-02-09 19:08     ` Richard Henderson
2015-07-09  8:17 ` [Qemu-devel] [PATCH 06/14] target-i386: Perform set/reset_inhibit_irq inline Richard Henderson
2015-07-09  8:17 ` [Qemu-devel] [PATCH 07/14] target-i386: Split up gen_lea_modrm Richard Henderson
2015-07-09  8:17 ` [Qemu-devel] [PATCH 08/14] target-i386: Implement BNDMK Richard Henderson
2015-07-09  8:17 ` [Qemu-devel] [PATCH 09/14] target-i386: Implement BNDMOV Richard Henderson
2015-07-09  8:17 ` [Qemu-devel] [PATCH 10/14] target-i386: Implement BNDCL, BNDCU, BNDCN Richard Henderson
2015-07-09  8:17 ` [Qemu-devel] [PATCH 11/14] target-i386: Update BNDSTATUS for exceptions raised by BOUND Richard Henderson
2015-07-09  8:17 ` [Qemu-devel] [PATCH 12/14] target-i386: Implement BNDLDX, BNDSTX Richard Henderson
2015-07-09  8:17 ` [Qemu-devel] [PATCH 13/14] target-i386: Clear bndregs during legacy near jumps Richard Henderson
2015-07-09  8:17 ` [Qemu-devel] [PATCH 14/14] target-i386: Enable XCR0 features for user-mode Richard Henderson
2015-07-09 13:15   ` Paolo Bonzini
2015-07-10  7:24     ` Richard Henderson
2015-07-10  9:36       ` Paolo Bonzini
2015-11-17 17:43 ` [Qemu-devel] [PATCH 00/14] target-i386: Implement MPX extension Paolo Bonzini
2015-11-18  9:43   ` Richard Henderson
2015-11-18 10:13     ` Paolo Bonzini

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.