Re: [PATCH 12/13] target/arm: Do memory type alignment check when translation disabled

[Philippe Mathieu-Daudé]

On 23/2/23 21:43, Richard Henderson wrote:

If translation is disabled, the default memory type is Device,
which requires alignment checking.  This is more optimially done

"optimally"?

early via the MemOp given to the TCG memory operation.

Reported-by: Idan Horowitz <idan.horow...@gmail.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1204
Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
---
  target/arm/helper.c | 36 ++++++++++++++++++++++++++++++++++--
  1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 07d4100365..b1b664e0ad 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -11867,6 +11867,37 @@ static inline bool fgt_svc(CPUARMState *env, int el)
          FIELD_EX64(env->cp15.fgt_exec[FGTREG_HFGITR], HFGITR_EL2, SVC_EL1);
  }

+/*

+ * Return true if memory alignment should be enforced.
+ */
+static bool aprofile_require_alignment(CPUARMState *env, int el, uint64_t 
sctlr)
+{
+#ifdef CONFIG_USER_ONLY
+    return false;
+#else
+    /* Check the alignment enable bit. */
+    if (sctlr & SCTLR_A) {
+        return true;
+    }
+
+    /*
+     * If translation is disabled, then the default memory type is
+     * Device(-nGnRnE) instead of Normal, which requires that alignment
+     * be enforced.  Since this affects all ram, it is most efficient
+     * to handle this during translation.
+     */
+    if (sctlr & SCTLR_M) {
+        /* Translation enabled: memory type in PTE via MAIR_ELx. */
+        return false;
+    }

I haven't checked <...

+    if (el < 2 && (arm_hcr_el2_eff(env) & (HCR_DC | HCR_VM))) {
+        /* Stage 2 translation enabled: memory type in PTE. */
+        return false;
+    }

...> this part, but for the rest:
Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>

+    return true;
+#endif
+}
+
  static CPUARMTBFlags rebuild_hflags_common(CPUARMState *env, int fp_el,
                                             ARMMMUIdx mmu_idx,
                                             CPUARMTBFlags flags)
@@ -11936,8 +11967,9 @@ static CPUARMTBFlags rebuild_hflags_a32(CPUARMState 
*env, int fp_el,
  {
      CPUARMTBFlags flags = {};
      int el = arm_current_el(env);
+    uint64_t sctlr = arm_sctlr(env, el);

- if (arm_sctlr(env, el) & SCTLR_A) {

+    if (aprofile_require_alignment(env, el, sctlr)) {
          DP_TBFLAG_ANY(flags, ALIGN_MEM, 1);
      }

@@ -12037,7 +12069,7 @@ static CPUARMTBFlags rebuild_hflags_a64(CPUARMState *env, int el, int fp_el, sctlr = regime_sctlr(env, stage1); - if (sctlr & SCTLR_A) {

+    if (aprofile_require_alignment(env, el, sctlr)) {
          DP_TBFLAG_ANY(flags, ALIGN_MEM, 1);
      }