Re: [PATCH 1/3] target/arm: Correct LDRD atomicity and fault behaviour

[Richard Henderson <richard.henderson@linaro.org>]

On 2/27/25 09:58, Peter Maydell wrote:
> On Thu, 27 Feb 2025 at 17:41, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> On 2/27/25 06:27, Peter Maydell wrote:
>>> +static void do_ldrd_load(DisasContext *s, TCGv_i32 addr, int rt, int rt2)
>>> +{
>>> +    /*
>>> +     * LDRD is required to be an atomic 64-bit access if the
>>> +     * address is 8-aligned, two atomic 32-bit accesses if
>>> +     * it's only 4-aligned, and to give an alignemnt fault
>>> +     * if it's not 4-aligned.
>>> +     * Rt is always the word from the lower address, and Rt2 the
>>> +     * data from the higher address, regardless of endianness.
>>> +     * So (like gen_load_exclusive) we avoid gen_aa32_ld_i64()
>>> +     * so we don't get its SCTLR_B check, and instead do a 64-bit access
>>> +     * using MO_BE if appropriate and then split the two halves.
>>> +     *
>>> +     * This also gives us the correct behaviour of not updating
>>> +     * rt if the load of rt2 faults; this is required for cases
>>> +     * like "ldrd r2, r3, [r2]" where rt is also the base register.
>>> +     */
>>> +    int mem_idx = get_mem_index(s);
>>> +    MemOp opc = MO_64 | MO_ALIGN_4 | MO_ATOM_SUBALIGN | s->be_data;
>>
>> The 64-bit atomicity begins with armv7 + LPAE, and not present for any m-profile.
>> Worth checking ARM_FEATURE_LPAE, or at least adding to the comment?
>>
>> Getting 2 x 4-byte atomicity, but not require 8-byte atomicity, would use
>> MO_ATOM_IFALIGN_PAIR.
> 
> Definitely worth a comment at minimum. Do we generate better
> code for MO_ATOM_IFALIGN_PAIR ? (If not, then providing higher
> atomicity than the architecture mandates seems harmless.)

We could, but currently do not, generate better code for IFALIGN_PAIR for MO_64. 
Currently the only place we take special care is for MO_128.

> For the comment in memop.h that currently reads
>       * MO_ATOM_SUBALIGN: the operation is single-copy atomic by parts
>       *    by the alignment.  E.g. if the address is 0 mod 4, then each
>       *    4-byte subobject is single-copy atomic.
>       *    This is the atomicity e.g. of IBM Power.
> 
> maybe we could expand the e.g:
> 
>    E.g if an 8-byte value is accessed at an address which is 0 mod 8,
>    then the whole 8-byte access is single-copy atomic; otherwise,
>    if it is accessed at 0 mod 4 then each 4-byte subobject is
>    single-copy atomic; otherwise if it is accessed at 0 mod 2
>    then the four 2-byte subobjects are single-copy atomic.
> 
> ?

Yes, that's correct.

> I wasn't sure when reading what we currently have whether
> it provided the 8-byte-aligned guarantee, rather than merely
> the 4-byte-aligned one.

I was trying to highlight the difference between SUBALIGN and IFALIGN, and perhaps didn't 
do adequate job of it.

r~