Re: [Xen-devel] [PATCH] x86emul: use unambiguous register names

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 03/01/17 13:01, Jan Beulich wrote:
> This is in preparation of eliminating the mis-naming of 64-bit fields
> with 32-bit register names (eflags instead of rflags etc).
>
> Note that the result is not fully consistent until after at least one
> more patch is in place, primarily to limit patch size (by trying to not
> touch the same line twice).
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> --- a/tools/tests/x86_emulator/x86_emulate.c
> +++ b/tools/tests/x86_emulator/x86_emulate.c
> @@ -10,9 +10,11 @@
>  
>  /* For generic assembly code: use macros to define operation/operand sizes. 
> */
>  #ifdef __i386__
> +# define r(name)       e ## name
>  # define __OS          "l"  /* Operation Suffix */
>  # define __OP          "e"  /* Operand Prefix */
>  #else
> +# define r(name)       r ## name
>  # define __OS          "q"  /* Operation Suffix */
>  # define __OP          "r"  /* Operand Prefix */
>  #endif
> --- a/xen/arch/x86/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate.c
> @@ -21,6 +21,8 @@
>  #undef cpuid
>  #undef wbinvd
>  
> +#define r(name) r ## name
> +

Hmm.  I am no overwhelmed with this syntax, but I can't propose an
alternative, so ok.

> @@ -2716,36 +2716,36 @@ x86_emulate(
>          struct segment_register cs, sreg;
>  
>      case 0x00 ... 0x05: add: /* add */
> -        emulate_2op_SrcV("add", src, dst, _regs.eflags);
> +        emulate_2op_SrcV("add", src, dst, _regs.r(flags));
>          break;
>  

All of these types of operations only adjust the arithmetic flags, so
could legitimately use eflags alone.  Is it worth reducing?

> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
> @@ -583,41 +583,9 @@ x86_emulate(
>      const struct x86_emulate_ops *ops);
>  
>  #ifndef NDEBUG
> -/*
> - * In debug builds, wrap x86_emulate() with some assertions about its 
> expected
> - * behaviour.
> - */

I'd leave this comment here as well.

Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

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