[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [Qemu-devel] [PATCH 8/9] target/arm: Implement CAS and CA
From: |
Peter Maydell |
Subject: |
Re: [Qemu-arm] [Qemu-devel] [PATCH 8/9] target/arm: Implement CAS and CASP |
Date: |
Fri, 4 May 2018 17:06:12 +0100 |
On 3 May 2018 at 18:32, Richard Henderson <address@hidden> 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 <address@hidden>
thanks
-- PMM