[Skiboot] [PATCH v4 01/18] libstb/secureboot: use platform.terminate instead of hard abort

[Vasant Hegde]

On 5/12/20 9:19 AM, Oliver O'Halloran wrote:
> On Mon, 2020-05-11 at 16:31 -0500, Eric Richter wrote:
>> Halting the boot via an abort() call will cause the BMC to keep
>> restarting the machine indefinitely.

Sorry for delay. I missed to respond to couple of emails.

If MPIPL is enabled its resulting in calling MPIPL, not reboot. So BMC will not 
be aware of MPIPL boots. Hence we endup in continuous loop.

> 
> The BMC is supposed to detect the host bootlooping and stop powering it
> on after 5 or so.
> 

Correct. I think its 3 continuous boot failures.

>> Ending via platform.terminate()
>> should be cleaner and prevent needless bootloops.
> 
> abort() is a #defined as assert(0) which results in a call to
> assert_fail():
> 
> void __noreturn assert_fail(const char *msg, const char *file,
>                                  unsigned int line, const char *function)
> {
>          static bool in_abort = false;
> 
>          (void)function;
>          if (in_abort)
>                  for (;;) ;
>          in_abort = true;
> 
>          /* snip comment */
>          prlog(PR_EMERG, "assert failed at %s:%u: %s\n", file, line, msg);
>          backtrace();
> 
>          /* Save crashing CPU details */
>          opal_mpipl_save_crashing_pir();
> 
>          if (platform.terminate)
>                  platform.terminate(msg);
> 
>          for (;;) ;
> }
> 
> So I'm not really seeing how this is an improvement. The terminate
> callback is usually ipmi_terminate() which will also trigger an MPIPL
> on the systems which support it. That might be the cause of your boot
> loop?
> 
> Vasant, do you have anything to add?

Problem today is, it trigger MPIPL if it find `dump` node in DT. I think we 
should defer MPIPL enablement until our init is complete. I will fix this and 
send patch soon.

-Vasant