Am 9. August 2024 16:04:28 UTC schrieb Peter Maydell <peter.mayd...@linaro.org>:
>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.
>
>Patch 1 cleans up an out of date comment about MMU index
>usage; patch 2 is the actual bug fix.
>
>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...
>
>Bernhard: I suspect this is the same bug you reported a few months
>back in this thread:
>https://lore.kernel.org/qemu-devel/c875173e-4b5b-4f71-8cf4-4325f7ab7...@gmail.com/
> -- if you're able to test that this patchset fixes your test
>case as well, that would be great.
Hi Peter,
indeed this fixes my guest, too! Thanks for keeping me updated.
Series:
Tested-by: Bernhard Beschow <shen...@gmail.com>
>
>thanks
>-- PMM
>
>Peter Maydell (2):
> target/arm: Update translation regime comment for new features
> target/arm: Fix usage of MMU indexes when EL3 is AArch32
>
> target/arm/cpu.h | 50 ++++++++++++++++++++++------------
> 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, 95 insertions(+), 39 deletions(-)
>
