On Sat, 1 Oct 2022 at 17:42, Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> Hoist the computation of the mmu_idx for the ptw up to
> get_phys_addr_with_secure_debug and get_phys_addr_twostage.
> This removes the duplicate check for stage2 disabled
> from the middle of the walk, performing it only once.
>
> Pass ptw_idx through get_phys_addr_{v5,v6,lpae} and arm_{ldl,ldq}_ptw.
>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> /* Translate a S1 pagetable walk through S2 if needed. */
> -static bool S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, hwaddr
> addr,
> +static bool S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
> + ARMMMUIdx s2_mmu_idx, hwaddr addr,
> bool *is_secure_ptr, void **hphys, hwaddr
> *gphys,
> bool debug, ARMMMUFaultInfo *fi)
> {
> bool is_secure = *is_secure_ptr;
> - ARMMMUIdx s2_mmu_idx = is_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2;
> - bool s2_phys = false;
I don't think this works, because the s2_mmu_idx is not necessarily
the same through the whole of a page table walk. See the comment in
get_phys_addr_lpae():
/*
* Secure accesses start with the page table in secure memory and
* can be downgraded to non-secure at any step. Non-secure accesses
* remain non-secure. We implement this by just ORing in the NSTable/NS
* bits at each step.
*/
Currently get_phys_addr_lpae() updates the nstable bit in tableattrs and
passes that to arm_ldq_ptw() for each level of the page tables, which in
turn causes S1_ptw_translate() to select ARMMMUIdx_Stage2_S or ARMMMUIdx_Stage2.
Alternatively, maybe our existing behaviour is a bug -- but then we need
to separate out the bug fix from the refactoring patch.
> @@ -2604,18 +2643,17 @@ static bool
> get_phys_addr_with_secure_debug(CPUARMState *env,
> /* Definitely a real MMU, not an MPU */
>
> if (regime_translation_disabled(env, mmu_idx, is_secure)) {
> - return get_phys_addr_disabled(env, address, access_type, mmu_idx,
> - is_secure, result, fi);
> + goto do_disabled;
> }
I'd prefer to avoid this goto back up into the middle of an unrelated
switch statement.
thanks
-- PMM
