Am 5. März 2024 13:52:34 UTC schrieb Peter Maydell <peter.mayd...@linaro.org>:
>From: Richard Henderson <richard.hender...@linaro.org>
>
>If translation is disabled, the default memory type is Device, which
>requires alignment checking.  This is more optimally done early via
>the MemOp given to the TCG memory operation.
>
>Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
>Reported-by: Idan Horowitz <idan.horow...@gmail.com>
>Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
>Message-id: 20240301204110.656742-6-richard.hender...@linaro.org
>Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1204
>Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
>Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>

Hi,

This change causes an old 4.14.40 Linux kernel to panic on boot using the 
sabrelite machine:

[snip]
Alignment trap: init (1) PC=0x76f1e3d4 Instr=0x14913004 Address=0x76f34f3e FSR 
0x001
Alignment trap: init (1) PC=0x76f1e3d8 Instr=0x148c3004 Address=0x7e8492bd FSR 
0x801
Alignment trap: init (1) PC=0x76f0dab0 Instr=0x6823 Address=0x7e849fbb FSR 0x001
Alignment trap: init (1) PC=0x76f0dab2 Instr=0x6864 Address=0x7e849fbf FSR 0x001
scsi 0:0:0:0: Direct-Access     QEMU     QEMU HARDDISK    2.5+ PQ: 0 ANSI: 5
fsl-asoc-card sound: ASoC: CODEC DAI sgtl5000 not registered
imx-sgtl5000 sound: ASoC: CODEC DAI sgtl5000 not registered
imx-sgtl5000 sound: snd_soc_register_card failed (-517)
Alignment trap: init (1) PC=0x76eac95a Instr=0xf8dd5015 Address=0x7e849b05 FSR 
0x001
Alignment trap: not handling instruction f8dd5015 at [<76eac95a>]
Unhandled fault: alignment exception (0x001) at 0x7e849b05
pgd = 9c59c000
[7e849b05] *pgd=2c552831, *pte=109eb34f, *ppte=109eb83f
Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000007

---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000007

As you can see, some alignment exceptions are handled by the kernel, the last 
one isn't. I added some additional printk()'s and traced it down to this 
location in the kernel: 
<https://github.com/torvalds/linux/blob/v4.14/arch/arm/mm/alignment.c#L762> 
which claims that ARMv6++ CPUs can handle up to word-sized unaligned accesses, 
thus no fixup is needed.

I hope that this will be sufficient for a fix. Let me know if you need any 
additional information.

Best regards,
Bernhard

>---
> target/arm/tcg/hflags.c | 34 ++++++++++++++++++++++++++++++++--
> 1 file changed, 32 insertions(+), 2 deletions(-)
>
>diff --git a/target/arm/tcg/hflags.c b/target/arm/tcg/hflags.c
>index 8e5d35d9227..5da1b0fc1d4 100644
>--- a/target/arm/tcg/hflags.c
>+++ b/target/arm/tcg/hflags.c
>@@ -26,6 +26,35 @@ 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)
>@@ -121,8 +150,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);
>     }
> 
>@@ -223,7 +253,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);
>     }
> 

Reply via email to