Re: [Xen-devel] [PATCH 3/5] x86emul: support RTM instructions

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.09.16 at 14:37, <andrew.cooper3@xxxxxxxxxx> 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


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