From 26ba00cf58e9f21b08fff4c691ce7e9bb21dd123 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 21 Nov 2022 21:24:04 +0000 Subject: [PATCH] target/arm: Don't do two-stage lookup if stage 2 is disabled In get_phys_addr_with_struct(), we call get_phys_addr_twostage() if the CPU supports EL2. However, we don't check here that stage 2 is actually enabled. Instead we only check that inside get_phys_addr_twostage() to skip stage 2 translation. This means that even if stage 2 is disabled we still tell the stage 1 lookup to do its page table walks via stage 2. This works by luck for normal CPU accesses, but it breaks for debug accesses, which are used by the disassembler and also by semihosting file reads and writes, because the debug case takes a different code path inside S1_ptw_translate(). This means that setups that use semihosting for file loads are broken (a regression since 7.1, introduced in recent ptw refactoring), and that sometimes disassembly in debug logs reports "unable to read memory" rather than showing the guest insns. Fix the bug by hoisting the "is stage 2 enabled?" check up to get_phys_addr_with_struct(), so that we handle S2 disabled the same way we do the "no EL2" case, with a simple single stage lookup. Reported-by: Jens Wiklander Reviewed-by: Richard Henderson Signed-off-by: Peter Maydell Message-id: 20221121212404.1450382-1-peter.maydell@linaro.org --- target/arm/ptw.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/target/arm/ptw.c b/target/arm/ptw.c index 9a6277d862..8ca468d65b 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -2612,8 +2612,8 @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, ret = get_phys_addr_with_struct(env, ptw, address, access_type, result, fi); - /* If S1 fails or S2 is disabled, return early. */ - if (ret || regime_translation_disabled(env, ARMMMUIdx_Stage2, is_secure)) { + /* If S1 fails, return early. */ + if (ret) { return ret; } @@ -2739,7 +2739,8 @@ static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw, * Otherwise, a stage1+stage2 translation is just stage 1. */ ptw->in_mmu_idx = mmu_idx = s1_mmu_idx; - if (arm_feature(env, ARM_FEATURE_EL2)) { + if (arm_feature(env, ARM_FEATURE_EL2) && + !regime_translation_disabled(env, ARMMMUIdx_Stage2, is_secure)) { return get_phys_addr_twostage(env, ptw, address, access_type, result, fi); }