diff mbox series

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

Message ID 20230223204342.1093632-13-richard.henderson@linaro.org
State Superseded
Headers show
Series {tcg,aarch64}: Add TLB_CHECK_ALIGNED | expand

Commit Message

Richard Henderson Feb. 23, 2023, 8:43 p.m. UTC
If translation is disabled, the default memory type is Device,
which requires alignment checking.  This is more optimially done
early via the MemOp given to the TCG memory operation.

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

Comments

Philippe Mathieu-Daudé Feb. 23, 2023, 9:41 p.m. UTC | #1
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.horowitz@gmail.com>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1204
> Signed-off-by: Richard Henderson <richard.henderson@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é <philmd@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);
>       }
>
diff mbox series

Patch

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