Re: [PATCH 0/2] Fix EL3 AArch32 MMU index usage (again)

[Peter Maydell <peter.maydell@linaro.org>]

On Fri, 1 Nov 2024 at 14:28, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> In commit 4c2c0474693229 I tried to fix a problem with our
> usage of MMU indexes when EL3 is AArch32. The problem we're
> trying to fix is:
>
>     Architecturally, when EL3 is AArch32, all Secure code runs under the
>     Secure PL1&0 translation regime:
>      * code at EL3, which might be Mon, or SVC, or any of the
>        other privileged modes (PL1)
>      * code at EL0 (Secure PL0)
>
>     This is different from when EL3 is AArch64, in which case EL3 is its
>     own translation regime, and EL1 and EL0 (whether AArch32 or AArch64)
>     have their own regime.
>
>     We claimed to be mapping Secure PL1 to our ARMMMUIdx_EL3, but didn't
>     do anything special about Secure PL0, which meant it used the same
>     ARMMMUIdx_EL10_0 that NonSecure PL0 does.  This resulted in a bug
>     where arm_sctlr() incorrectly picked the NonSecure SCTLR as the
>     controlling register when in Secure PL0, which meant we were
>     spuriously generating alignment faults because we were looking at the
>     wrong SCTLR control bits.
>
>     The use of ARMMMUIdx_EL3 for Secure PL1 also resulted in the bug that
>     we wouldn't honour the PAN bit for Secure PL1, because there's no
>     equivalent _PAN mmu index for it.
>
> The "spurious alignment faults" part is
> https://gitlab.com/qemu-project/qemu/-/issues/2326
>
> Commit 4c2c047469322 tried to fix this using what I described in the
> commit message as a "more complicated approach", but didn't get it
> right in several ways. Full detail in the commit message of patch 1,
> but the major visible problem was that regime_el() would return 1
> even when the CPU was in Monitor mode; this meant that page table
> walks in Monitor mode would look at the wrong SCTLR, TCR, etc and
> would generally fault when they should not.
>
> Rather than trying to fix up the multiple problems with the complicated
> approach, this series first reverts that commit and then fixes the
> initial problem with the idea that commit 4c2c047469322 describes
> as "the most straightforward" approach: we add new MMU indexes
> EL30_0 and EL30_3_PAN, and use the EL3 index as EL30_3. These then
> correspond to "Secure PL1&0 at PL0", "Secure PL1&0 at PL1", and
> "Secure PL1&0 at PL1 with PAN", and parallel the NonSecure use
> of EL10_0, EL10_1_PAN and EL10_1.

The submitter tells me that this also fixes
https://gitlab.com/qemu-project/qemu/-/issues/2588
(a different variety of "misbehaviour in Secure Monitor mode").

-- PMM