Re: [Qemu-devel] [PATCH v2 for-2.10 3/3] target/arm: Require alignment for load exclusive

[Peter Maydell]

On 15 August 2017 at 16:56, Philippe Mathieu-Daudé <address@hidden> wrote:
> Hi Richard,
>
> On 08/15/2017 11:57 AM, Richard Henderson wrote:
>>
>> From: Alistair Francis <address@hidden>
>>
>> Acording to the ARM ARM exclusive loads require the same allignment as
>> exclusive stores. Let's update the memops used for the load to match
>> that of the store. This adds the alignment requirement to the memops.
>>
>> Reviewed-by: Edgar E. Iglesias <address@hidden>
>> Signed-off-by: Alistair Francis <address@hidden>
>> [rth: Require 16-byte alignment for 64-bit LDXP.]
>> Signed-off-by: Richard Henderson <address@hidden>
>> ---
>>   target/arm/translate-a64.c | 11 ++++++-----
>>   1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
>> index eac545e4f2..2200e25be0 100644
>> --- a/target/arm/translate-a64.c
>> +++ b/target/arm/translate-a64.c
>> @@ -1861,7 +1861,7 @@ static void gen_load_exclusive(DisasContext *s, int
>> rt, int rt2,
>>           g_assert(size >= 2);
>>           if (size == 2) {
>>               /* The pair must be single-copy atomic for the doubleword.
>> */
>> -            memop |= MO_64;
>> +            memop |= MO_64 | MO_ALIGN;
>
>
> isn't MO_ALIGN_8 enough?

MO_ALIGN is "align to size of access", ie to 8 bytes in this case.
MO_ALIGN_8 is "align to a specified size (8 bytes) which isn't the
 size of the access".
In this case the access size is 8 bytes so we don't need MO_ALIGN_8.

Alignments to something other than the access size are the oddball
case, so I think it makes sense to stick with MO_ALIGN for the
common case of "just aligned to the access size" so you can
spot the odd cases when you're reading the source.

thanks
-- PMM