Re: [Qemu-devel] [PATCH] target-i386: fix cmpxchg instruction emulation

[malc]

On Sat, 10 Dec 2011, Andreas Gustafsson wrote:

> When the i386 cmpxchg instruction is executed with a memory operand
> and the comparison result is "unequal", do the memory write before
> changing the accumulator instead of the other way around, because
> otherwise the new accumulator value will incorrectly be used in the
> comparison when the instruction is restarted after a page fault.
> 
> This bug was originally reported on 2010-04-25 as
> https://bugs.launchpad.net/qemu/+bug/569760 

I'm starting to lose count on how many times this patch resurfaces here.
Please see http://web.archiveorange.com/archive/v/1XS1vaW9MlyMzguWl5fE
and the links in the thread.

> 
> Signed-off-by: Andreas Gustafsson <address@hidden>
> ---
> --- translate.c.orig  2011-12-09 18:21:28.000000000 +0200
> +++ translate.c       2011-12-09 18:21:24.000000000 +0200
> @@ -4870,20 +4870,23 @@ static target_ulong disas_insn(DisasCont
>              tcg_gen_sub_tl(t2, cpu_regs[R_EAX], t0);
>              gen_extu(ot, t2);
>              tcg_gen_brcondi_tl(TCG_COND_EQ, t2, 0, label1);
> +            label2 = gen_new_label();
>              if (mod == 3) {
> -                label2 = gen_new_label();
>                  gen_op_mov_reg_v(ot, R_EAX, t0);
>                  tcg_gen_br(label2);
>                  gen_set_label(label1);
>                  gen_op_mov_reg_v(ot, rm, t1);
> -                gen_set_label(label2);
>              } else {
> -                tcg_gen_mov_tl(t1, t0);
> +                /* perform no-op 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(ot + s->mem_index, t0, a0);
>                  gen_op_mov_reg_v(ot, R_EAX, t0);
> +                tcg_gen_br(label2);
>                  gen_set_label(label1);
> -                /* always store */
>                  gen_op_st_v(ot + s->mem_index, t1, a0);
>              }
> +            gen_set_label(label2);
>              tcg_gen_mov_tl(cpu_cc_src, t0);
>              tcg_gen_mov_tl(cpu_cc_dst, t2);
>              s->cc_op = CC_OP_SUBB + ot;
> 

-- 
mailto:address@hidden