diff mbox

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

Message ID CAFEAcA922pjJBa7otZxZBazO2P09MpQSnwyywAbfMdNwCoqYbA@mail.gmail.com
State Accepted
Headers show

Commit Message

Peter Maydell Dec. 15, 2015, 5:31 p.m. UTC 
On 3 December 2015 at 18:36, Andrew Baumann
<Andrew.Baumann@microsoft.com> 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 plumbing to enable alignment checks on loads using

> MO_ALIGN, a do_unaligned_access hook to raise the exception (data

> abort), and uses the new aligned loads in LDREX (for all but

> single-byte loads).

>

> Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>

> ---

> Thanks for the feedback on v1! I wish I had known about (or gone

> looking for) MO_ALIGN sooner...

>

> 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?


The way you have it seems OK to me.

> +/* Raise a data fault alignment exception for the specified virtual address */

> +void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, int is_write,

> +                                 int is_user, uintptr_t retaddr)

> +{

> +    ARMCPU *cpu = ARM_CPU(cs);

> +    CPUARMState *env = &cpu->env;

> +    int target_el;

> +    bool same_el;

> +

> +    if (retaddr) {

> +        /* now we have a real cpu fault */

> +        cpu_restore_state(cs, retaddr);

> +    }

> +

> +    target_el = exception_target_el(env);

> +    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);

> +}


This isn't propagating the 'read or write' information
from is_write into the syndrome and DFSR. You need this minor
tweak:


(compare the similar code in tlb_fill()).

I'm just going to squash that in when I apply this to target-arm.next,
to save you having to respin.

thanks
-- PMM
diff mbox

Patch

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index c6995ca..3e5e0d3 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -154,8 +154,12 @@  void arm_cpu_do_unaligned_access(CPUState *cs,
vaddr vaddr, int is_write,
         env->exception.fsr = 0x1;
     }

+    if (is_write == 1 && arm_feature(env, ARM_FEATURE_V6)) {
+        env->exception.fsr |= (1 << 11);
+    }
+
     raise_exception(env, EXCP_DATA_ABORT,
-                    syn_data_abort(same_el, 0, 0, 0, 0, 0x21),
+                    syn_data_abort(same_el, 0, 0, 0, is_write == 1, 0x21),
                     target_el);
 }