Re: [Xen-devel] [PATCH 3/3] x86/emul: Improvements to internal users of decode_register()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.01.18 at 14:16, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -1980,9 +1980,8 @@ load_seg(
>      return rc;
>  }
>  
> -void *
> -decode_register(
> -    uint8_t modrm_reg, struct cpu_user_regs *regs, int highbyte_regs)
> +static unsigned long *decode_high_gpr(
> +    struct cpu_user_regs *regs, unsigned int modrm_reg, bool highbyte_regs)

Same remark as made earlier for the array, here regarding the
function name: You're decoding legacy byte registers, not high
byte ones. As to the return type - wasn't it you who objected
to "unsigned long *" when the pointer really just points at a
byte (which I've already reflected in my v4 of the patch which
I assume triggered your attempt to do things differently)? I.e.
either void * or uint8_t *, please.

> @@ -3066,8 +3063,7 @@ x86_emulate(
>      generate_exception_if(state->not_64bit && mode_64bit(), EXC_UD);
>  
>      if ( ea.type == OP_REG )
> -        ea.reg = decode_register(modrm_rm, &_regs,
> -                                 (d & ByteOp) && !rex_prefix);
> +        ea.reg = decode_high_gpr(&_regs, modrm_rm, (d & ByteOp) && 
> !rex_prefix);
>  
>      memset(mmvalp, 0xaa /* arbitrary */, sizeof(*mmvalp));
>  
> @@ -3081,13 +3077,13 @@ x86_emulate(
>          src.type = OP_REG;
>          if ( d & ByteOp )
>          {
> -            src.reg = decode_register(modrm_reg, &_regs, (rex_prefix == 0));
> +            src.reg = decode_high_gpr(&_regs, modrm_reg, (rex_prefix == 0));

Along the lines of the earlier hunk "!rex_prefix"? Same in I think two
more cases further down.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel