Re: [Qemu-arm] [Qemu-devel] [PATCH] target-arm: raise exception on misaligned LDREX operands

[Laurent Desnogues]

On Thu, Dec 3, 2015 at 3:36 PM, Peter Maydell <address@hidden> wrote:
> On 30 November 2015 at 22:23, Andrew Baumann
> <address@hidden> wrote:
>> Qemu does not generally perform alignment checks. However, the ARM ARM
>> requires implementation of alignment exceptions for a number of cases
>> including LDREX, and Windows-on-ARM relies on this.
>>
>> This change adds a helper function to raise an alignment exception
>> (data abort), a framework for implementing alignment checks in
>> translated instructions, and adds one such check to the translation of
>> LDREX instruction (for all variants except single-byte loads).
>>
>> Signed-off-by: Andrew Baumann <address@hidden>
>> ---
>> I realise this will need to wait until after 2.5, but wanted to get
>> the review feedback started. If needed, I can resend this later.
>>
>> arm_regime_using_lpae_format() is a no-op wrapper I added to export
>> regime_using_lpae_format (which is a static inline). Would it be
>> preferable to simply export the existing function, and rename it? If
>> so, is this still the correct name to use for the function?
>>
>> CONFIG_ALIGNMENT_EXCEPTIONS shows how the check can be conditionally
>> enabled, but isn't presently hooked up to any configure mechanism. I
>> figured that the overhead of an alignment check in LDREX is not high
>> enough to warrant disabling the feature, but if it gets used more
>> widely it might be.
>>
>> The same change is almost certainly applicable to arm64, but I am not
>> in a position to test it.
>
> TCG supports "this load/store should do an alignment check"
> using the MO_ALIGN TCGMemOp flag (which results in a call to
> the CPU's do_unaligned_access hook if the guest address is not
> aligned). I think we should use this core-code functionality
> rather than rolling our own equivalent (it is more efficient).
> There are some examples in a few of the other targets (eg MIPS)
> of how to do this, but basically you need to arrange that the
> initial loads in gen_load_exclusive get the MO_ALIGN flag
> ORed in, and then wire up the do_unaligned_access hook and
> make it raise a suitable exception.

After quickly looking at the code in softmmu_template.h, I wonder if
MO_ALIGN would correcly handle the ldrexd pair case which requires an
8-byte alignment but does 2 4-byte loads (even if the code is tweaked
to read 8-byte at once, then checking 16-byte alignment of AArch64
ldxp 64-bit could not be handled correctly).

Thanks,

Laurent