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

[Andrew Baumann]

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.

 target-arm/helper.c    |  8 ++++++++
 target-arm/helper.h    |  1 +
 target-arm/internals.h |  3 +++
 target-arm/op_helper.c | 21 +++++++++++++++++++++
 target-arm/translate.c | 29 +++++++++++++++++++++++++++++
 5 files changed, 62 insertions(+)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index afc4163..59d5a41 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -5996,6 +5996,14 @@ static inline bool regime_using_lpae_format(CPUARMState 
*env,
     return false;
 }
 
+/* Returns true if the translation regime is using LPAE format page tables.
+ * Used when raising alignment exceptions, whose FSR changes depending on
+ * whether the long or short descriptor format is in use. */
+bool arm_regime_using_lpae_format(CPUARMState *env, ARMMMUIdx mmu_idx)
+{
+    return regime_using_lpae_format(env, mmu_idx);
+}
+
 static inline bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx)
 {
     switch (mmu_idx) {
diff --git a/target-arm/helper.h b/target-arm/helper.h
index c2a85c7..16d0137 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -48,6 +48,7 @@ DEF_HELPER_FLAGS_3(sel_flags, TCG_CALL_NO_RWG_SE,
                    i32, i32, i32, i32)
 DEF_HELPER_2(exception_internal, void, env, i32)
 DEF_HELPER_4(exception_with_syndrome, void, env, i32, i32, i32)
+DEF_HELPER_2(alignment_exception, void, env, tl)
 DEF_HELPER_1(wfi, void, env)
 DEF_HELPER_1(wfe, void, env)
 DEF_HELPER_1(yield, void, env)
diff --git a/target-arm/internals.h b/target-arm/internals.h
index 347998c..17afe2a 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -441,4 +441,7 @@ struct ARMMMUFaultInfo {
 bool arm_tlb_fill(CPUState *cpu, vaddr address, int rw, int mmu_idx,
                   uint32_t *fsr, ARMMMUFaultInfo *fi);
 
+/* Return true if the translation regime is using LPAE format page tables */
+bool arm_regime_using_lpae_format(CPUARMState *env, ARMMMUIdx mmu_idx);
+
 #endif
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 6cd54c8..5c46210 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -376,6 +376,27 @@ void HELPER(exception_with_syndrome)(CPUARMState *env, 
uint32_t excp,
     raise_exception(env, excp, syndrome, target_el);
 }
 
+/* Raise a data fault alignment exception for the specified virtual address */
+void HELPER(alignment_exception)(CPUARMState *env, target_ulong vaddr)
+{
+    int target_el = exception_target_el(env);
+    bool same_el = (arm_current_el(env) != target_el);
+
+    env->exception.vaddress = vaddr;
+
+    /* the DFSR for an alignment fault depends on whether we're using
+     * the LPAE long descriptor format, or the short descriptor format */
+    if (arm_regime_using_lpae_format(env, cpu_mmu_index(env, false))) {
+        env->exception.fsr = 0x21;
+    } else {
+        env->exception.fsr = 0x1;
+    }
+
+    raise_exception(env, EXCP_DATA_ABORT,
+                    syn_data_abort(same_el, 0, 0, 0, 0, 0x21),
+                    target_el);
+}
+
 uint32_t HELPER(cpsr_read)(CPUARMState *env)
 {
     return cpsr_read(env) & ~(CPSR_EXEC | CPSR_RESERVED);
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 5d22879..c05ea1f 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -37,6 +37,7 @@
 
 #include "trace-tcg.h"
 
+#define CONFIG_ALIGNMENT_EXCEPTIONS 1
 
 #define ENABLE_ARCH_4T    arm_dc_feature(s, ARM_FEATURE_V4T)
 #define ENABLE_ARCH_5     arm_dc_feature(s, ARM_FEATURE_V5)
@@ -1058,6 +1059,28 @@ static void gen_exception_insn(DisasContext *s, int 
offset, int excp,
     s->is_jmp = DISAS_JUMP;
 }
 
+/* Emit an inline alignment check, which raises an exception if the given
+ * address is not aligned according to "size" (which must be a power of 2). */
+static void gen_alignment_check(DisasContext *s, int pc_offset,
+                                target_ulong size, TCGv addr)
+{
+#ifdef CONFIG_ALIGNMENT_EXCEPTIONS
+    TCGLabel *alignok_label = gen_new_label();
+    TCGv tmp = tcg_temp_new();
+
+    /* check alignment, branch to alignok_label if aligned */
+    tcg_gen_andi_tl(tmp, addr, size - 1);
+    tcg_gen_brcondi_tl(TCG_COND_EQ, tmp, 0, alignok_label);
+
+    /* emit alignment exception */
+    gen_set_pc_im(s, s->pc - pc_offset);
+    gen_helper_alignment_exception(cpu_env, addr);
+
+    gen_set_label(alignok_label);
+    tcg_temp_free(tmp);
+#endif
+}
+
 /* Force a TB lookup after an instruction that changes the CPU state.  */
 static inline void gen_lookup_tb(DisasContext *s)
 {
@@ -7430,6 +7453,12 @@ static void gen_load_exclusive(DisasContext *s, int rt, 
int rt2,
 
     s->is_ldex = true;
 
+    /* emit alignment check if needed */
+    if (size != 0) {
+        /* NB: all LDREX variants (incl. thumb) occupy 4 bytes */
+        gen_alignment_check(s, 4, (target_ulong)1 << size, addr);
+    }
+
     switch (size) {
     case 0:
         gen_aa32_ld8u(tmp, addr, get_mem_index(s));
-- 
2.5.3