Re: [Xen-devel] [PATCH v3] SVM: use generic instruction decoding

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.09.16 at 16:37, <boris.ostrovsky@xxxxxxxxxx> wrote:
> On 09/30/2016 05:38 AM, Jan Beulich wrote:
>> +        if ( opc_tab[instr].opcode == ctxt.ctxt.opcode )
>>          {
>> -            if ( (inst_len + i) >= fetch_len ) 
>> -            {
>> -                if ( !fetch(vmcb, buf + fetch_len, fetch_addr + fetch_len,
>> -                            max_len - fetch_len) )
>> -                    return 0;
>> -                fetch_len = max_len;
>> -            }
>> +            if ( !opc_tab[instr].modrm.mod )
>> +                return inst_len;
> 
> Is there a reason why this is not folded into the 'if' below?

To keep things readable.

>> +int
>> +x86_insn_modrm(const struct x86_emulate_state *state,
>> +               unsigned int *rm, unsigned int *reg)
>> +{
>> +    check_state(state);
>> +
>> +    if ( !(state->desc & ModRM) )
>> +        return -EINVAL;
>> +
>> +    if ( rm )
>> +        *rm = state->modrm_rm;
>> +    if ( reg )
>> +        *reg = state->modrm_reg;
>> +
>> +    return state->modrm_mod;
>> +}
> 
> Can this return struct modrm (which would then become visible outside of
> svm.c)? And then x86_emulate_state can include the same struct instead
> of the three separate fields.

I'd prefer not to, to leave it to callers which parts they actually care
about. No need for them to put the whole structure on stack when
all they want is e.g. mod.

Jan


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