On 03/03/2015 17:47, Mark Burton wrote: > +inline void arm_exclusive_lock(void) > +{ > + if (!cpu_have_exclusive_lock) { > + qemu_mutex_lock(&cpu_exclusive_lock); > + cpu_have_exclusive_lock = true; > + } > +} > + > +inline void arm_exclusive_unlock(void) > +{ > + if (cpu_have_exclusive_lock) { > + cpu_have_exclusive_lock = false; > + qemu_mutex_unlock(&cpu_exclusive_lock); > + } > +} > +
This is almost but not quite a recursive mutex. What about changing it like this: - arm_exclusive_lock just takes the lock and sets the flag; no "if" - arm_exclusive_unlock does the opposite, again no "if" - raise_exception checks the flag and skips "arm_exclusive_lock()" if already set The only other question I have is this: > + gen_helper_atomic_check(success, cpu_env, addr); > + tcg_gen_brcondi_i32(TCG_COND_EQ, success, 0, done_label); Are you setting cpu_R[rd] correctly in this case? That is, should you be jumping to fail_label instead? That could case a failure to be reported as a success. Paolo > + tcg_temp_free_i32(success); > + > + /* Store shoudl be OK lets check the value */ > + tmp = load_reg(s, rt); > + TCGv_i64 val64=tcg_temp_new_i64(); > switch (size) { > case 0: > gen_aa32_ld8u(tmp, addr, get_mem_index(s)); > @@ -7450,21 +7447,19 @@ static void gen_store_exclusive(DisasContext *s, > int rd, int rt, int rt2, > abort(); > } > > - val64 = tcg_temp_new_i64(); > if (size == 3) { > TCGv_i32 tmp2 = tcg_temp_new_i32(); > TCGv_i32 tmp3 = tcg_temp_new_i32(); > + > tcg_gen_addi_i32(tmp2, addr, 4); > gen_aa32_ld32u(tmp3, tmp2, get_mem_index(s)); > - tcg_temp_free_i32(tmp2); > tcg_gen_concat_i32_i64(val64, tmp, tmp3); > - tcg_temp_free_i32(tmp3); > + tcg_temp_free_i32(tmp2); > } else { > - tcg_gen_extu_i32_i64(val64, tmp); > + tcg_gen_extu_i32_i64(val64, tmp); > } > tcg_temp_free_i32(tmp); > - > - tcg_gen_brcond_i64(TCG_COND_NE, val64, cpu_exclusive_val, fail_label); > + tcg_gen_brcond_i64(TCG_COND_NE, cpu_exclusive_val, val64, fail_label); > tcg_temp_free_i64(val64); > > tmp = load_reg(s, rt); > @@ -7489,12 +7484,16 @@ static void gen_store_exclusive(DisasContext *s, > int rd, int rt, int rt2, > gen_aa32_st32(tmp, addr, get_mem_index(s)); > tcg_temp_free_i32(tmp); > } > + > + gen_helper_atomic_release(cpu_env); > tcg_gen_movi_i32(cpu_R[rd], 0); > tcg_gen_br(done_label); > + > gen_set_label(fail_label); > + gen_helper_atomic_release(cpu_env); > tcg_gen_movi_i32(cpu_R[rd], 1); > + > gen_set_label(done_label); > - tcg_gen_movi_i64(cpu_exclusive_addr, -1);