Re: [PATCH] sunxi: psci: remove redundant initialization from psci_arch_init

[Marc Zyngier <maz@kernel.org>]

On Fri, 25 Aug 2023 19:05:32 +0100,
Sam Edwards <cfsworks@gmail.com> wrote:
> 
> On 8/25/23 00:20, Chen-Yu Tsai wrote:
> 
> Hi Chen-Yu,
> 
> > IIRC the GIC manual says that the secure bit is set or cleared to select
> > which bank of registers is accessed.
> 
> Which secure bit are we talking about here? Do we mean the
> *configured* secure bit (SCR.NS, what the code is attempting to clear)
> or the *effective* secure bit (AWPROT[1], et al)? The distinction is
> important in monitor mode (where this function runs) since there (and
> only there) the CPU core ignores the configured setting and runs in
> the secure world unconditionally.
> 
> I'm guessing it's most likely the latter since the former isn't
> exposed outside of the CPU core, unless the GIC has some special
> signal going to it...

The GIC definitely has the NS bit routed to it. Otherwise, the secure
configuration would just be an utter joke. Just try it.

> 
> > And I suppose it is here to be more robust.
> 
> ...but if it is the former (i.e. SCR.NS is significant in this
> function) the code should be retained, but moved *before* the GIC
> register accesses, and the old value of SCR.NS should be restored
> *after*.
> 
> Either way: I don't think this line should be kept in its current
> form, because it's written in a way that strongly suggests that we
> want to run in secure mode after exiting monitor mode, which is flatly
> not the case.

Well, history is unfortunately against you on that front. Running on
the secure side definitely was a requirement when this code was
initially written, as the AW BSP *required* to run on the secure side.

If that requirement is no more, great. But I don't think you can
decide that unilaterally.

	M.

-- 
Without deviation from the norm, progress is not possible.