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

[Thomas Huth <thuth@redhat.com>]

On 01/11/2024 15.28, Peter Maydell 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.

I can confirm that this fixes the problems with the bpim2u and orangepi 
Ubuntu/Armbian tests, so if that counts:

Tested-by: Thomas Huth <thuth@redhat.com>