[Skiboot] [PATCH] console: Completely flush output buffer before power down and reboot

[Stewart Smith]

Alistair Popple <alistair at popple.id.au> writes:
> On Mon, 9 Nov 2015 13:10:29 Stewart Smith wrote:
>> Russell Currey <ruscur at russell.cc> writes:
>> > Completely flush the output buffer of the console driver before
>> > power down and reboot.  Implements the flushing function for uart
>> > consoles, which includes the astbmc and rhesus platforms.
>> >
>> > Adds a new function, flush(), to the con_ops struct that allows
>> > each console driver to specify how their output buffers are flushed.
>> >
>> > In the cec_power_down and cec_reboot functions, the flush function
>> > of the driver is called if it exists.
>> >
>> > This fixes an issue where some console output is sometimes lost before
>> > power down or reboot in uart consoles. If this issue is also prevalent
>> > in other console types then it can be fixed later by adding a .flush
>> > to that driver's con_ops.
>> 
>> I guess this is specifically problematic in some panic() situations,
>> especially when the panic reboot timeout is set to 0 (i.e. that Linux
>> does *not* reboot after panic) as we then spin in a (for lack of a
>> better term) death-loop and never call into OPAL to do something like,
>> say, flush the panic message?
>
> Actually it's the opposite. If the reboot timeout is set we reboot but not 
> always before flushing the lpc uart buffer and hence we miss the
> output.

Ahh, that's right. I blame ENOCOFFEE or EMONDAY.

> We still have a bug when the reboot timeout is 0. I believe when the kernel 
> panics opal_poll_event() stops getting called. Hence the lpc uart buffer gets 
> filled but nothing is called to flush it out.

Yep, that's what happens.

Arguably I should go write a kernel patch that does something kind of
sensible there.

Interestingly though, I guess on FSP systems this means we'd still be
replying to heartbeat messages from FSP, so the machine would change
behaviour as it'd stay at the panic forever rather than current
behaviour of eventually rebooting.

I'd argue that the current behaviour of timing out on the heartbeat is a
bug though, as the intended behaviour is obviously *not* to reboot,
otherwise you wouldn't have that kernel parameter set.

>> > Signed-off-by: Russell Currey <ruscur at russell.cc>
>> 
>> should this also head towards stable? I think we've had a bug open for
>> it... or at least there's a possible case to make for including it in
>> stable....
>
> Yeah, probably a good idea.

Done, cherry-picked to stable as of dd7980e6d9ff