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