Re: [Xen-devel] [PATCH 09/17] SVM: use generic instruction decoding

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/09/16 07:55, Jan Beulich wrote:
>>>> On 14.09.16 at 19:56, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 08/09/16 14:14, Jan Beulich wrote:
>>>  int __get_instruction_length_from_list(struct vcpu *v,
>>>          const enum instruction_index *list, unsigned int list_count)
>>>  {
>>>      struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
>>> -    unsigned int i, j, inst_len = 0;
>>> -    enum instruction_index instr = 0;
>>> -    u8 buf[MAX_INST_LEN];
>>> -    const u8 *opcode = NULL;
>>> -    unsigned long fetch_addr, fetch_limit;
>>> -    unsigned int fetch_len, max_len;
>>> +    struct hvm_emulate_ctxt ctxt;
>>> +    struct x86_emulate_state *state;
>>> +    unsigned int inst_len, j, modrm_rm, modrm_reg;
>>> +    int modrm_mod;
>>>  
>>> +#ifdef NDEBUG
>> Presumably this is just for your testing?
> No, I actually meant it to stay that way. Along the lines of the extra
> debugging code we have in map_domain_page().

I was never very happy with the older version of this debugging.  Surely
in a case like this, we should use the intercept information when
available, and check it against the emulator in a debug build.

That way, we don't entirely change the underlying logic in this function
between a debug and non debug build.

>>> @@ -1658,6 +1662,11 @@ x86_decode_base(
>>>  
>>>      switch ( ctxt->opcode )
>>>      {
>>> +    case 0x90: /* nop / pause */
>>> +        if ( repe_prefix() )
>>> +            ctxt->opcode |= X86EMUL_OPC_F3(0, 0);
>>> +        break;
>> Why is it necessary to special case the rep prefix handling in this case?
> Because SVM's pause intercept should not mistakenly also accept a
> plain NOP.

How about a comment to this effect:

/* Note: Prefixes such as rep/repne are only encoded when semantically
meaningful to the instruction, to reduce the complexity of interpreting
this opcode representation. */

~Andrew

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