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