On 3 May 2018 at 18:32, Richard Henderson <richard.hender...@linaro.org> wrote: > On 05/03/2018 07:55 AM, Peter Maydell wrote: >>> + /* If compare equal, write back new data, else write back old >>> data. */ >>> + tcg_gen_movcond_i64(TCG_COND_NE, c1, c2, zero, t1, d1); >>> + tcg_gen_movcond_i64(TCG_COND_NE, c2, c2, zero, t2, d2); >>> + tcg_gen_qemu_st_i64(c1, addr, memidx, MO_64 | s->be_data); >>> + tcg_gen_qemu_st_i64(c2, a2, memidx, MO_64 | s->be_data); >> >> I think this has the wrong behaviour if you do a CASP-with-mismatched-value >> to read-only memory -- architecturally this should fail the comparison >> and return the memory value in registers, it's not allowed to do a >> memory write and take a data abort because the memory isn't writable. > > If this is true [...]
Fortunately it turns out that it is not true. In the pseudocode, the CAS accesses are made with an acctype of either AccType_ORDEREDRW or AccType_ATOMICRW, and when this gets down into the translation table walk code AArch64.CheckPermission() handles those access types specially and generates a permission fault on !r || !w, so will fault the read part of the read-modify-write. I wouldn't be too surprised if we get the ESR_ELx WnR bit wrong for this (it is supposed to be "0 if a plain read would fault, otherwise 1"), but let's not worry about that for now... Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> thanks -- PMM
