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

[Richard Henderson]

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.


r~