Re: [PATCH] target/arm: Two fixes for secure ptw

[Peter Maydell]

On Thu, 3 Nov 2022 at 20:30, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 11/4/22 00:19, Peter Maydell wrote:
> >> @@ -2671,6 +2671,13 @@ static bool get_phys_addr_with_struct(CPUARMState 
> >> *env, S1Translate *ptw,
> >>       bool is_secure = ptw->in_secure;
> >>       ARMMMUIdx s1_mmu_idx;
> >>
> >> +    /*
> >> +     * The page table entries may downgrade secure to non-secure, but
> >> +     * cannot upgrade an non-secure translation regime's attributes
> >> +     * to secure.
> >> +     */
> >> +    result->f.attrs.secure = is_secure;
> >> +
> >>       switch (mmu_idx) {
> >>       case ARMMMUIdx_Phys_S:
> >>       case ARMMMUIdx_Phys_NS:
> >> @@ -2712,12 +2719,6 @@ static bool get_phys_addr_with_struct(CPUARMState 
> >> *env, S1Translate *ptw,
> >>           break;
> >>       }
> >>
> >> -    /*
> >> -     * The page table entries may downgrade secure to non-secure, but
> >> -     * cannot upgrade an non-secure translation regime's attributes
> >> -     * to secure.
> >> -     */
> >> -    result->f.attrs.secure = is_secure;
> >>       result->f.attrs.user = regime_is_user(env, mmu_idx);
> >
> > Do we also need to move this setting of attrs.user ?
> > get_phys_addr_disabled() doesn't set that either.
>
> I don't think so.  The only cases which don't pass through the setting of 
> attrs.user are
> the two Phys mmu_idx.  Which was by design per the comment up there about 
> artificially
> deciding which EL regime they belong to.

OK. Do we ever do anything with the attrs for a phys tlb lookup
except use them internally for details of the stage 2 tlb walk ?
I guess they get used for the memory transaction to do the walk.
That matches the old code that just had a local MemTxAttrs in
arm_ldq_ptw() to do the walk which therefore implicitly got
user == false.

-- PMM