* [Qemu-devel] [PATCH] target-i386: Fix aflag logic for CODE64 and the 0x67 prefix
@ 2013-05-29 19:30 Richard Henderson
2013-05-31 15:03 ` Richard Henderson
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Richard Henderson @ 2013-05-29 19:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini
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;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] target-i386: Fix aflag logic for CODE64 and the 0x67 prefix
2013-05-29 19:30 [Qemu-devel] [PATCH] target-i386: Fix aflag logic for CODE64 and the 0x67 prefix Richard Henderson
@ 2013-05-31 15:03 ` Richard Henderson
2013-05-31 15:04 ` Paolo Bonzini
2013-06-01 0:19 ` Laszlo Ersek
2013-06-03 15:43 ` Anthony Liguori
2 siblings, 1 reply; 5+ messages in thread
From: Richard Henderson @ 2013-05-31 15:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Anthony Liguori
Ping.
On 05/29/2013 12:30 PM, 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;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] target-i386: Fix aflag logic for CODE64 and the 0x67 prefix
2013-05-31 15:03 ` Richard Henderson
@ 2013-05-31 15:04 ` Paolo Bonzini
0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2013-05-31 15:04 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, Anthony Liguori, 1.5.x
Il 31/05/2013 17:03, Richard Henderson ha scritto:
> Ping.
>
> On 05/29/2013 12:30 PM, 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;
>> }
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-stable@nongnu.org
>> }
>> -#endif
>>
>> s->prefix = prefixes;
>> s->aflag = aflag;
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] target-i386: Fix aflag logic for CODE64 and the 0x67 prefix
2013-05-29 19:30 [Qemu-devel] [PATCH] target-i386: Fix aflag logic for CODE64 and the 0x67 prefix Richard Henderson
2013-05-31 15:03 ` Richard Henderson
@ 2013-06-01 0:19 ` Laszlo Ersek
2013-06-03 15:43 ` Anthony Liguori
2 siblings, 0 replies; 5+ messages in thread
From: Laszlo Ersek @ 2013-06-01 0:19 UTC (permalink / raw)
To: Richard Henderson; +Cc: Paolo Bonzini, qemu-devel
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>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] target-i386: Fix aflag logic for CODE64 and the 0x67 prefix
2013-05-29 19:30 [Qemu-devel] [PATCH] target-i386: Fix aflag logic for CODE64 and the 0x67 prefix Richard Henderson
2013-05-31 15:03 ` Richard Henderson
2013-06-01 0:19 ` Laszlo Ersek
@ 2013-06-03 15:43 ` Anthony Liguori
2 siblings, 0 replies; 5+ messages in thread
From: Anthony Liguori @ 2013-06-03 15:43 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: Paolo Bonzini, Anthony Liguori, 1.5.x
Applied. Thanks.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-06-03 18:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-29 19:30 [Qemu-devel] [PATCH] target-i386: Fix aflag logic for CODE64 and the 0x67 prefix Richard Henderson
2013-05-31 15:03 ` Richard Henderson
2013-05-31 15:04 ` Paolo Bonzini
2013-06-01 0:19 ` Laszlo Ersek
2013-06-03 15:43 ` Anthony Liguori
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).