Re: [PATCH v3 30/42] target/arm: Add ptw_idx argument to S1_ptw_translate

[Richard Henderson]

On 10/7/22 02:19, Peter Maydell wrote:
> 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.

Ouch.  I had missed this subtlety.

We could play lsb games with the mmu_idx itself, knowing that we have either 
ARMMMUIdx_{Stage2,Phys}_S and generate ARMMMUIdx_{Stage2,Phys}.  I'll have another good 
long look at this.


> >       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.

Oops, I guess I missed this one when I went back through to eliminate the gotos.


r~