Re: [PATCH] target/arm: Make the final stage1+2 write to secure be unconditional

[Richard Henderson]

On 10/7/22 09:20, Peter Maydell wrote:
> > -            /* Check if IPA translates to secure or non-secure PA space. */
> > -            if (is_secure) {
> > -                if (ipa_secure) {
> > -                    result->attrs.secure =
> > -                        !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW));
> > -                } else {
> > -                    result->attrs.secure =
> > -                        !((env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW))
> > -                        || (env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW)));
> > -                }
> > -            }
>
> If:
>   is_secure == true
>   ipa_secure == false
>   (env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW) is non-zero
>   (env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW) is zero
> then the old code sets attrs.secure to true...

No, I think the misalignment of the two lines wrt the !() may have been 
confusing:

  if (true) {
    if (false) {
    } else {
      secure = !((0) || (non-zero))
             = !(1)
             = 0
    }
  }


r~

> > +            /*
> > +             * Check if IPA translates to secure or non-secure PA space.
> > +             * Note that VSTCR overrides VTCR and {N}SW overrides {N}SA.
> > +             */
> > +            result->attrs.secure =
> > +                (is_secure
> > +                 && !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW))
> > +                 && (ipa_secure
> > +                     || !(env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW))));
>
> ...but the new code will set it to false, I think ?
>
> thanks
> -- PMM