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

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

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?

>  
> -            if ( buf[inst_len+i] != opcode[i+1] )
> -                goto mismatch;
> +            if ( modrm_mod == opc_tab[instr].modrm.mod &&
> +                 (modrm_rm & 7) == opc_tab[instr].modrm.rm &&
> +                 (modrm_reg & 7) == opc_tab[instr].modrm.reg )
> +                return inst_len;
>          }

...

> +
> +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.


-boris



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