| Message ID | 20240809160430.1144805-1-peter.maydell@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | target/arm: Fix EL3-is-AArch32 mmu indexes | expand |
Am 9. August 2024 16:04:28 UTC schrieb Peter Maydell <peter.maydell@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-4325F7AB7629@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 <shentey@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(-) >