On 3 December 2015 at 18:36, Andrew Baumann
<andrew.baum...@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.baum...@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:
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);
}
(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
