On 23.11.2015 19:49, Peter Maydell wrote: > Ping? I forgot to mark this for-2.5, and given how long the bug's > been hanging around there's not much urgency to fixing it, but > we might as well put the fix into 2.5 if it gets reviewed. >
Hi, Peter. I'm going to review this carefully in a few days :) For now, I see that the comment for this function should be updated to match new code. Best, Sergey > > On 16 November 2015 at 18:28, Peter Maydell <[email protected]> wrote: >> The checks for the unallocated encodings in the ldst_excl group >> (exclusives and load-acquire/store-release) were not correct. This >> error meant that in turn we ended up with code attempting to handle >> the non-existent case of "non-exclusive load-acquire/store-release >> pair". Delete that broken and now unreachable code. >> >> Reported-by: Laurent Desnogues <[email protected]> >> Signed-off-by: Peter Maydell <[email protected]> >> --- >> The easiest way to validate that we have the unallocated >> conditions correct now is to look at C4.4.6 "load/store exclusive" >> in the v8 ARM ARM rev A.h: our three conditions correspond >> to the three "unallocated" rows in the decode table. >> >> PS: Laurent originally reported this way back in 2014: >> http://lists.gnu.org/archive/html/qemu-devel/2014-10/msg01255.html >> >> target-arm/translate-a64.c | 12 ++---------- >> 1 file changed, 2 insertions(+), 10 deletions(-) >> >> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c >> index fe485a4..890ace4 100644 >> --- a/target-arm/translate-a64.c >> +++ b/target-arm/translate-a64.c >> @@ -1833,7 +1833,8 @@ static void disas_ldst_excl(DisasContext *s, uint32_t >> insn) >> int size = extract32(insn, 30, 2); >> TCGv_i64 tcg_addr; >> >> - if ((!is_excl && !is_lasr) || >> + if ((!is_excl && !is_pair && !is_lasr) || >> + (!is_excl && is_pair) || >> (is_pair && size < 2)) { >> unallocated_encoding(s); >> return; >> @@ -1862,15 +1863,6 @@ static void disas_ldst_excl(DisasContext *s, uint32_t >> insn) >> } else { >> do_gpr_ld(s, tcg_rt, tcg_addr, size, false, false); >> } >> - if (is_pair) { >> - TCGv_i64 tcg_rt2 = cpu_reg(s, rt); >> - tcg_gen_addi_i64(tcg_addr, tcg_addr, 1 << size); >> - if (is_store) { >> - do_gpr_st(s, tcg_rt2, tcg_addr, size); >> - } else { >> - do_gpr_ld(s, tcg_rt2, tcg_addr, size, false, false); >> - } >> - } >> } >> } >> >> -- >> 1.9.1 >>
