On Fri, 25 Aug 2023 19:05:32 +0100, Sam Edwards <cfswo...@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.