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

[Eric Richter]

On 5/11/20 10:49 PM, 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.
> 
> The BMC is supposed to detect the host bootlooping and stop powering it
> on after 5 or so.
> 
>> 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?
> 

Admittedly, I took the off-list suggestion directly, and didn't look deep into the terminate() behavior. Is there a better method to guaranteed halt the machine that could be used instead? There is no reason to attempt a reboot/MPIPL if a secureboot trap is hit.

> Vasant, do you have anything to add?
> 
>