On Thu, 6 Oct 2022 at 16:10, Richard Henderson <richard.hender...@linaro.org> wrote: > > On 10/6/22 07:27, Peter Maydell wrote: > > On Sat, 1 Oct 2022 at 17:24, Richard Henderson > > <richard.hender...@linaro.org> wrote: > >> > >> The starting security state comes with the translation regime, > >> not the current state of arm_is_secure_below_el3(). > >> > >> Create a new local variable, s2walk_secure, which does not need > >> to be written back to result->attrs.secure -- we compute that > >> value later, after the S2 walk is complete. > >> > >> Signed-off-by: Richard Henderson <richard.hender...@linaro.org> > >> --- > >> v3: Do not modify ipa_secure, per review. > >> --- > > > > Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> > > > > I did find myself wondering if we should explicitly set > > result->attrs.secure = false; > > in an else-branch of the last "if (is_secure)", though. > > At the moment we rely on get_phys_addr_lpae() for the stage2 > > doing that for us, I think. Having the code here always set > > result->attrs.secure before the 'return 0' avoids having to think > > about that... > > Yes, we're currently (and predating this patch set) relying on the attrs > structure being > cleared to start. But I can certainly add the assignment if you like.
Yeah, cleared-at-start is fine. But here we're also relying on the stage 2 call to get_phys_addr_lpae() not setting it to 1, because we pass that the same 'result' pointer, not a fresh one. -- PMM
