[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] i386/translate: ignore 0x67 (PREFIX_ADR) on TAR
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH] i386/translate: ignore 0x67 (PREFIX_ADR) on TARGET_X86_64 && CODE64() |
Date: |
Sun, 26 May 2013 10:33:35 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6 |
Il 26/05/2013 01:23, Richard Henderson ha scritto:
> On 2013-05-24 14:37, Laszlo Ersek wrote:
>> @@ -4813,7 +4813,11 @@ static target_ulong disas_insn(CPUX86State
>> *env, DisasContext *s,
>> /* 0x66 is ignored if rex.w is set */
>> dflag = 2;
>> }
>> - if (!(prefixes & PREFIX_ADR)) {
>> + if (prefixes & PREFIX_ADR) {
>> + /* flip it back, 0x67 should have no effect */
>> + aflag ^= 1;
>> + }
>> + else {
>> aflag = 2;
>> }
>> }
>>
>
> Agreed that there's a bug here. I'm thinking it would be clearer to not
> write this as yet another flip, but understand that unlike dflag, aflag
> can only be either 1 or 2 in 64-bit mode.
>
> I'm thinking of something more like this:
>
> diff --git a/target-i386/translate.c b/target-i386/translate.c
> index 0aeccdb..bf772aa 100644
> --- a/target-i386/translate.c
> +++ b/target-i386/translate.c
> @@ -4813,9 +4813,8 @@ static target_ulong disas_insn(CPUX86State *env,
> DisasContext *s,
> /* 0x66 is ignored if rex.w is set */
> dflag = 2;
> }
> - if (!(prefixes & PREFIX_ADR)) {
> - aflag = 2;
> - }
> + /* 0x67 toggles between 64-bit and 32-bit addressing. */
> + aflag = (prefixes & PREFIX_ADR ? 1 : 2);
Isn't that just "aflag++"? Needs a comment of course ("toggle between
32- and 64-bit, not 16- and 32-bit.").
Paolo