On Thu, 2014-10-23 at 10:22 +0200, Hans de Goede wrote:
> Hi Ian,
>
> On 10/22/2014 08:55 PM, Ian Campbell wrote:
> > On Wed, 2014-10-22 at 15:45 +0200, Hans de Goede wrote:
> >> if (!fake) {
> >> #if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
> >> - armv7_init_nonsec();
> >> - secure_ram_addr(_do_nonsec_entry)(kernel_entry,
> >> - 0, machid, r2);
> >> -#else
> >> - kernel_entry(0, machid, r2);
> >> + if (boot_nonsec()) {
> >> + armv7_init_nonsec();
> >> + secure_ram_addr(_do_nonsec_entry)(kernel_entry,
> >> + 0, machid, r2);
> >> + }
> >> #endif
> >> + kernel_entry(0, machid, r2);
> >
> > There's a subtle different here, which is that this final kernel_entry
> > call used to be in the #else clause, and so emitted for the NONSEC ||
> > VIRT case. So if the _do_nonsec_entry call were to fail (not currently
> > possible) and return you'd end up trying again via the sec path.
> >
> > I'm not sure that's a bad thing, but it is a difference so it'd be good
> > to know it was a deliberate choice (or not).
>
> I was under the assumption that do_nonsec_entry would never fail, and would
> not return, which is why I wrote this code the way I wrote it.
AFAICT in practice it can't fail today, but if it were somehow modified
in the future to do so this would expose some slightly surprising
behaviour.
> I'm not sure
> if retrying in secure mode meets the principle of least surprise, so I guess
> the #if .. #endif block should probably get an "else" added before the #endif,
> do you agree?
Yes.
BTW, if you put the #ifdef around boot_nonsec() instead and make the
#else case #define boot_nonsec() (0) then does that end up looking
cleaner here at the caller? Maybe that causes knockons with the
prototypes for the unused functions in that case, in which case I doubt
it is worth it.
Ian.
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
