diff mbox series

target/i386: cmpxchg should not touch accumulator

Message ID 20210731190507.9007-1-amdeniulari@protonmail.com (mailing list archive)
State New, archived
Headers show
Series target/i386: cmpxchg should not touch accumulator | expand

Commit Message

yqwfh July 31, 2021, 7:06 p.m. UTC
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.com>
---
 target/i386/tcg/translate.c | 9 ---------
 1 file changed, 9 deletions(-)

Comments

Richard Henderson Aug. 2, 2021, 6:46 p.m. UTC | #1
On 7/31/21 9:06 AM, yqwfh wrote:
> Signed-off-by: Daniele Ahmed <ahmeddan@amazon.com>
> ---
>   target/i386/tcg/translate.c | 9 ---------
>   1 file changed, 9 deletions(-)
> 
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index aacb605..41386dd 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -5366,7 +5366,6 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
>                   gen_lea_modrm(env, s, modrm);
>                   tcg_gen_atomic_cmpxchg_tl(oldv, s->A0, cmpv, newv,
>                                             s->mem_index, ot | MO_LE);
> -                gen_op_mov_reg_v(s, ot, R_EAX, oldv);
>               } else {
>                   if (mod == 3) {
>                       rm = (modrm & 7) | REX_B(s);
> @@ -5381,15 +5380,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
>                   /* store value = (old == cmp ? new : old);  */
>                   tcg_gen_movcond_tl(TCG_COND_EQ, newv, oldv, cmpv, newv, oldv);
>                   if (mod == 3) {
> -                    gen_op_mov_reg_v(s, ot, R_EAX, oldv);
>                       gen_op_mov_reg_v(s, ot, rm, newv);
> -                } else {
> -                    /* Perform an unconditional store cycle like physical cpu;
> -                       must be before changing accumulator to ensure
> -                       idempotency if the store faults and the instruction
> -                       is restarted */
> -                    gen_op_st_v(s, ot, newv, s->A0);
> -                    gen_op_mov_reg_v(s, ot, R_EAX, oldv);
>                   }

Obviously incorrect.  You can't *not* store to EAX.  You must conditionally store to EAX.


r~
diff mbox series

Patch

diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index aacb605..41386dd 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -5366,7 +5366,6 @@  static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
                 gen_lea_modrm(env, s, modrm);
                 tcg_gen_atomic_cmpxchg_tl(oldv, s->A0, cmpv, newv,
                                           s->mem_index, ot | MO_LE);
-                gen_op_mov_reg_v(s, ot, R_EAX, oldv);
             } else {
                 if (mod == 3) {
                     rm = (modrm & 7) | REX_B(s);
@@ -5381,15 +5380,7 @@  static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
                 /* store value = (old == cmp ? new : old);  */
                 tcg_gen_movcond_tl(TCG_COND_EQ, newv, oldv, cmpv, newv, oldv);
                 if (mod == 3) {
-                    gen_op_mov_reg_v(s, ot, R_EAX, oldv);
                     gen_op_mov_reg_v(s, ot, rm, newv);
-                } else {
-                    /* Perform an unconditional store cycle like physical cpu;
-                       must be before changing accumulator to ensure
-                       idempotency if the store faults and the instruction
-                       is restarted */
-                    gen_op_st_v(s, ot, newv, s->A0);
-                    gen_op_mov_reg_v(s, ot, R_EAX, oldv);
                 }
             }
             tcg_gen_mov_tl(cpu_cc_src, oldv);