Re: [Qemu-devel] [PATCH] target-i386: Fix aflag logic for CODE64 and the 0x67 prefix

[Laszlo Ersek <lersek@redhat.com>]

On 05/29/13 21:30, Richard Henderson wrote:
> The code reorganization in commit 4a6fd938 broke handling of PREFIX_ADR.
> While fixing this, tidy and comment the code so that it's more obvious
> what's going on in setting both aflag and dflag.
> 
> The TARGET_X86_64 ifdef can be eliminated because CODE64 expands to the
> constant zero when TARGET_X86_64 is undefined.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  target-i386/translate.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/target-i386/translate.c b/target-i386/translate.c
> index 0aeccdb..14b0298 100644
> --- a/target-i386/translate.c
> +++ b/target-i386/translate.c
> @@ -4677,8 +4677,6 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
>      }
>      s->pc = pc_start;
>      prefixes = 0;
> -    aflag = s->code32;
> -    dflag = s->code32;
>      s->override = -1;
>      rex_w = -1;
>      rex_r = 0;
> @@ -4801,23 +4799,25 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
>      }
>  
>      /* Post-process prefixes.  */
> -    if (prefixes & PREFIX_DATA) {
> -        dflag ^= 1;
> -    }
> -    if (prefixes & PREFIX_ADR) {
> -        aflag ^= 1;
> -    }
> -#ifdef TARGET_X86_64
>      if (CODE64(s)) {
> -        if (rex_w == 1) {
> -            /* 0x66 is ignored if rex.w is set */
> -            dflag = 2;
> +        /* In 64-bit mode, the default data size is 32-bit.  Select 64-bit
> +           data with rex_w, and 16-bit data with 0x66; rex_w takes precedence
> +           over 0x66 if both are present.  */
> +        dflag = (rex_w > 0 ? 2 : prefixes & PREFIX_DATA ? 0 : 1);
> +        /* In 64-bit mode, 0x67 selects 32-bit addressing.  */
> +        aflag = (prefixes & PREFIX_ADR ? 1 : 2);
> +    } else {
> +        /* In 16/32-bit mode, 0x66 selects the opposite data size.  */
> +        dflag = s->code32;
> +        if (prefixes & PREFIX_DATA) {
> +            dflag ^= 1;
>          }
> -        if (!(prefixes & PREFIX_ADR)) {
> -            aflag = 2;
> +        /* In 16/32-bit mode, 0x67 selects the opposite addressing.  */
> +        aflag = s->code32;
> +        if (prefixes & PREFIX_ADR) {
> +            aflag ^= 1;
>          }
>      }
> -#endif
>  
>      s->prefix = prefixes;
>      s->aflag = aflag;
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>