From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56686) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4VLQ-0005MM-Np for qemu-devel@nongnu.org; Thu, 03 Dec 2015 09:58:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a4VLP-0005v7-Oq for qemu-devel@nongnu.org; Thu, 03 Dec 2015 09:58:52 -0500 MIME-Version: 1.0 In-Reply-To: References: <1448922238-5696-1-git-send-email-Andrew.Baumann@microsoft.com> Date: Thu, 3 Dec 2015 15:58:51 +0100 Message-ID: From: Laurent Desnogues Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH] target-arm: raise exception on misaligned LDREX operands List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: qemu-arm , QEMU Developers , Andrew Baumann On Thu, Dec 3, 2015 at 3:36 PM, Peter Maydell wrote: > On 30 November 2015 at 22:23, Andrew Baumann > 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 >> --- >> 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