| Message ID | 57D18744020000780010D26C@prv-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/09/16 14:44, Jan Beulich wrote: > Minimal emulation: XBEGIN aborts right away, hence > - XABORT is just a no-op, > - XEND always raises #GP, > - XTEST always signals neither RTM nor HLE are active. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/arch/x86/x86_emulate/x86_emulate.c > +++ b/xen/arch/x86/x86_emulate/x86_emulate.c > @@ -1172,6 +1172,8 @@ static bool_t vcpu_has( > #define vcpu_has_clflush() vcpu_has( 1, EDX, 19, ctxt, ops) > #define vcpu_has_lzcnt() vcpu_has(0x80000001, ECX, 5, ctxt, ops) > #define vcpu_has_bmi1() vcpu_has(0x00000007, EBX, 3, ctxt, ops) > +#define vcpu_has_hle() vcpu_has(0x00000007, EBX, 4, ctxt, ops) > +#define vcpu_has_rtm() vcpu_has(0x00000007, EBX, 11, ctxt, ops) > > #define vcpu_must_have(leaf, reg, bit) \ > generate_exception_if(!vcpu_has(leaf, reg, bit, ctxt, ops), EXC_UD, -1) > @@ -2852,7 +2854,18 @@ x86_emulate( > lock_prefix = 1; > break; > > - case 0xc6 ... 0xc7: /* mov (sole member of Grp11) */ > + case 0xc6: /* Grp11: mov / xabort */ > + case 0xc7: /* Grp11: mov / xbegin */ > + if ( modrm == 0xf8 && vcpu_has_rtm() ) > + { > + if ( b & 1 ) > + { > + jmp_rel((int32_t)src.val); This should be based on op_bytes. There are two forms, one with a rel16 jump and one with rel32, and I don't see this being accounted for anywhere else. > + _regs.eax = 0; > + } > + dst.type = OP_NONE; The XABORT instruction should explicitly set bit. Incidentally, what is supposed to happen if we branch into the middle of an RTM region? > + break; > + } > generate_exception_if((modrm_reg & 7) != 0, EXC_UD, -1); > case 0x88 ... 0x8b: /* mov */ > case 0xa0 ... 0xa1: /* mov mem.offs,{%al,%ax,%eax,%rax} */ > @@ -4246,6 +4259,17 @@ x86_emulate( > goto done; > goto no_writeback; > > + case 0xd5: /* xend */ > + generate_exception_if(vcpu_has_rtm() && !vex.pfx, EXC_GP, 0); > + break; > + > + case 0xd6: /* xtest */ > + if ( (!vcpu_has_rtm() && !vcpu_has_hle()) || vex.pfx ) > + break; Shouldn't this raise #UD explicitly? I can't spot anything which does if we break out. ~Andrew > + /* Neither HLE nor RTM can be active when we get here. */ > + _regs.eflags |= EFLG_ZF; > + goto no_writeback; > + > case 0xdf: /* invlpga */ > generate_exception_if(!in_protmode(ctxt, ops), EXC_UD, -1); > generate_exception_if(!mode_ring0(), EXC_GP, 0); > > >
>>> On 30.09.16 at 14:37, <andrew.cooper3@citrix.com> wrote: > On 08/09/16 14:44, Jan Beulich wrote: >> @@ -2852,7 +2854,18 @@ x86_emulate( >> lock_prefix = 1; >> break; >> >> - case 0xc6 ... 0xc7: /* mov (sole member of Grp11) */ >> + case 0xc6: /* Grp11: mov / xabort */ >> + case 0xc7: /* Grp11: mov / xbegin */ >> + if ( modrm == 0xf8 && vcpu_has_rtm() ) >> + { >> + if ( b & 1 ) >> + { >> + jmp_rel((int32_t)src.val); > > This should be based on op_bytes. There are two forms, one with a rel16 > jump and one with rel32, and I don't see this being accounted for > anywhere else. Just like for other branches (as well as any instructions with immediate operands) this gets taken care of when the immediate gets fetched. >> + _regs.eax = 0; >> + } >> + dst.type = OP_NONE; > > The XABORT instruction should explicitly set bit. ??? Since we abort upon XBEGIN, XABORT is supposed to be a NOP. > Incidentally, what is supposed to happen if we branch into the middle of > an RTM region? Sooner or later the code would reach an XEND, which is defined to #GP with no prior XBEGIN. >> @@ -4246,6 +4259,17 @@ x86_emulate( >> goto done; >> goto no_writeback; >> >> + case 0xd5: /* xend */ >> + generate_exception_if(vcpu_has_rtm() && !vex.pfx, EXC_GP, 0); >> + break; >> + >> + case 0xd6: /* xtest */ >> + if ( (!vcpu_has_rtm() && !vcpu_has_hle()) || vex.pfx ) >> + break; > > Shouldn't this raise #UD explicitly? I can't spot anything which does > if we break out. As mentioned on IRC I already made this explicit for v2, but even without it's being taken care of by generate_exception_if(ea.type != OP_MEM, EXC_UD, -1); in the second switch() statement. Here's how v2 is going to look like: + case 0xd5: /* xend */ + generate_exception_if(vex.pfx, EXC_UD, -1); + generate_exception_if(!vcpu_has_rtm(), EXC_UD, -1); + generate_exception_if(vcpu_has_rtm(), EXC_GP, 0); + break; + + case 0xd6: /* xtest */ + generate_exception_if(vex.pfx, EXC_UD, -1); + generate_exception_if(!vcpu_has_rtm() && !vcpu_has_hle(), + EXC_UD, -1); + /* Neither HLE nor RTM can be active when we get here. */ + _regs.eflags |= EFLG_ZF; + goto no_writeback; Jan
--- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -1172,6 +1172,8 @@ static bool_t vcpu_has( #define vcpu_has_clflush() vcpu_has( 1, EDX, 19, ctxt, ops) #define vcpu_has_lzcnt() vcpu_has(0x80000001, ECX, 5, ctxt, ops) #define vcpu_has_bmi1() vcpu_has(0x00000007, EBX, 3, ctxt, ops) +#define vcpu_has_hle() vcpu_has(0x00000007, EBX, 4, ctxt, ops) +#define vcpu_has_rtm() vcpu_has(0x00000007, EBX, 11, ctxt, ops) #define vcpu_must_have(leaf, reg, bit) \ generate_exception_if(!vcpu_has(leaf, reg, bit, ctxt, ops), EXC_UD, -1) @@ -2852,7 +2854,18 @@ x86_emulate( lock_prefix = 1; break; - case 0xc6 ... 0xc7: /* mov (sole member of Grp11) */ + case 0xc6: /* Grp11: mov / xabort */ + case 0xc7: /* Grp11: mov / xbegin */ + if ( modrm == 0xf8 && vcpu_has_rtm() ) + { + if ( b & 1 ) + { + jmp_rel((int32_t)src.val); + _regs.eax = 0; + } + dst.type = OP_NONE; + break; + } generate_exception_if((modrm_reg & 7) != 0, EXC_UD, -1); case 0x88 ... 0x8b: /* mov */ case 0xa0 ... 0xa1: /* mov mem.offs,{%al,%ax,%eax,%rax} */ @@ -4246,6 +4259,17 @@ x86_emulate( goto done; goto no_writeback; + case 0xd5: /* xend */ + generate_exception_if(vcpu_has_rtm() && !vex.pfx, EXC_GP, 0); + break; + + case 0xd6: /* xtest */ + if ( (!vcpu_has_rtm() && !vcpu_has_hle()) || vex.pfx ) + break; + /* Neither HLE nor RTM can be active when we get here. */ + _regs.eflags |= EFLG_ZF; + goto no_writeback; + case 0xdf: /* invlpga */ generate_exception_if(!in_protmode(ctxt, ops), EXC_UD, -1); generate_exception_if(!mode_ring0(), EXC_GP, 0);