Re: [Xen-devel] [PATCH v4 05/17] x86emul: support MMX/SSE{, 2, 4a} insns with only register operands

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 01/03/17 14:43, Jan Beulich wrote:
>>>> On 01.03.17 at 15:36, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 28/02/17 12:52, Jan Beulich wrote:
>>> @@ -2505,12 +2506,21 @@ x86_decode(
>>>  
>>>                  opcode |= b | MASK_INSR(vex.pfx, X86EMUL_OPC_PFX_MASK);
>>>  
>>> +                if ( !(d & ModRM) )
>>> +                {
>>> +                    modrm_reg = modrm_rm = modrm_mod = modrm = 0;
>>> +                    break;
>>> +                }
>>> +
>>>                  modrm = insn_fetch_type(uint8_t);
>>>                  modrm_mod = (modrm & 0xc0) >> 6;
>>>  
>>>                  break;
>>>              }
>>> +    }
>>>  
>>> +    if ( d & ModRM )
>>> +    {
>>>          modrm_reg = ((rex_prefix & 4) << 1) | ((modrm & 0x38) >> 3);
>>>          modrm_rm  = modrm & 0x07;
>>>  
>> Doesn't this hunk want splitting out into its own patch and
>> backporting?  Xen 4.8's x86_decode_insn() was supposedly able to provide
>> the correct length.
> Well, if this was affecting instructions we could even remotely
> expect to make it here, I would have done it in a separate
> patch, but vmzero{all,upper} just seem to unlikely to warrant
> a backport.
>
>> If so, both patches Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> So I'd prefer to keep it as one patch, but if you make your R-b
> dependent on the split, then I'll do so. Let me know.

It really depends on what are the chances that anyone is making use of
the enhancements.  I'd personally err on the side of backporting, but I
also accept that it is probably very unlikely.

At least it should be obvious to diagnose and easy to backport if anyone
comes across the problem.

~Andrew

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