[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 for-2.10 3/3] target/arm: Require alignment f
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH v2 for-2.10 3/3] target/arm: Require alignment for load exclusive |
Date: |
Tue, 15 Aug 2017 17:14:42 +0100 |
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
Re: [Qemu-devel] [PATCH v2 for-2.10 0/3] Fixup logic for exclusive pair, Peter Maydell, 2017/08/15