Our current usage of MMU indexes when EL3 is AArch32 is confused.
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.
We could fix this in one of two ways:
* The most straightforward is to add new MMU indexes EL30_0,
EL30_3, EL30_3_PAN to correspond to "Secure PL1&0 at PL0",
"Secure PL1&0 at PL1", and "Secure PL1&0 at PL1 with PAN".
This matches how we use indexes for the AArch64 regimes, and
preserves propirties like being able to determine the privilege
level from an MMU index without any other information. However
it would add two MMU indexes (we can share one with ARMMMUIdx_EL3),
and we are already using 14 of the 16 the core TLB code permits.
* The more complicated approach is the one we take here. We use
the same MMU indexes (E10_0, E10_1, E10_1_PAN) for Secure PL1&0
than we do for NonSecure PL1&0. This saves on MMU indexes, but
means we need to check in some places whether we're in the
Secure PL1&0 regime or not before we interpret an MMU index.
The changes in this commit were created by auditing all the places
where we use specific ARMMMUIdx_ values, and checking whether they
needed to be changed to handle the new index value usage.
Note for potential stable backports: taking also the previous
(comment-change-only) commit might make the backport easier.
Cc:qemu-stable@nongnu.org
Resolves:https://gitlab.com/qemu-project/qemu/-/issues/2326
Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
---
This fixes the bug with the repro case in the bug report, and it
also passes "make check", but I don't have a huge range of
Secure AArch32 test images to test with. I guess it ought to go
into 9.1 as a bugfix, but the nature of the patch means it's
not very easy to be confident it doesn't introduce any new bugs...
---
target/arm/cpu.h | 31 ++++++++++++++++++-------------
target/arm/internals.h | 27 +++++++++++++++++++++++----
target/arm/tcg/translate.h | 2 ++
target/arm/helper.c | 34 +++++++++++++++++++++++-----------
target/arm/ptw.c | 6 +++++-
target/arm/tcg/hflags.c | 4 ++++
target/arm/tcg/translate-a64.c | 2 +-
target/arm/tcg/translate.c | 9 +++++----
8 files changed, 81 insertions(+), 34 deletions(-)