[Skiboot] [PATCH] [v3] Fast reboot for P8

[Stewart Smith]

Benjamin Herrenschmidt <benh at kernel.crashing.org> writes:
> On Tue, 2016-09-27 at 18:00 +1000, Stewart Smith wrote:
>> 
>> what would be your criteria for going to signed-off-by ?
>> 
>> i'm thinking:
>> 1) behind a config switch (I'm thinking
>>    nvram_query("experimental-fast-reboot")=="Danger-is-my-middle-
>> name" or some
>>    such check)
>> 2) CAPI disable ptaches from Andrew
>> 3) Check for checkstops
>> 
>> Although skipping 2 and 3 because there's (1) (and not having it
>> enabled by default) could be a way forward.
>
> Yup, I generally agree. Also more testing.

rebooted for me: SHIP IT! :)

>> Couple of quick thoughts below, not near complete review of course :)
>> 
>> > 
>> > --- a/hw/fsp/fsp-leds.c
>> > +++ b/hw/fsp/fsp-leds.c
>> > @@ -1570,6 +1570,9 @@ void create_led_device_nodes(void)
>> >  	if (!pled)
>> >  		return;
>> >  
>> > +	/* Check if already populated (fast-reboot) */
>> > +	if (dt_has_node_property(pled, "compatible", NULL))
>> > +		return;
>> 
>> Any thoughts on a consistent way to do this that makes it relatively
>> obvious we're in fast reboot rather than IPL?
>
> Not off hand. Maybe a different init function alltogether ?

Yeah... maybe.

>> >  	dt_add_property_strings(pled, "compatible",
>> > DT_PROPERTY_LED_COMPATIBLE);
>> >  
>> >  	led_mode = dt_prop_get(pled, DT_PROPERTY_LED_MODE);
>> > diff --git a/hw/occ.c b/hw/occ.c
>> > index b606a67..3d86f7a 100644
>> > --- a/hw/occ.c
>> > +++ b/hw/occ.c
>> > @@ -517,10 +517,14 @@ void occ_pstates_init(void)
>> >  	struct proc_chip *chip;
>> >  	struct cpu_thread *c;
>> >  	s8 pstate_nom;
>> > +	static bool occ_pstates_initialized;
>> >  
>> >  	/* OCC is P8 only */
>> >  	if (proc_gen != proc_gen_p8)
>> >  		return;
>> > +	/* Handle fast reboots */
>> > +	if (occ_pstates_initialized)
>> > +		return;
>> >  
>> >  	chip = next_chip(NULL);
>> >  	if (!chip->homer_base) {
>> > @@ -558,6 +562,7 @@ void occ_pstates_init(void)
>> >  	for_each_chip(chip)
>> >  		chip->throttle = 0;
>> >  	opal_add_poller(occ_throttle_poll, NULL);
>> > +	occ_pstates_initialized = true;
>> >  }
>> 
>> We could reset the OCCs on fast reboot, as there is a PRD command to
>> disable them.. although nobody should *ever* use them....
>
> We could ... should we ?

Probably? I may have to think about it a bit more.

>> FWIW I've been looking at moving OCC init even on openpower into
>> skiboot
>> for the purposes of speeding up boot, so this could be "easy".
>
> I was under the impression that OCC inits requires going through
> 223648726434 lines of horrible TMGT code in HostBoot (or HBRT) ?

I was thinking of just calling HBRT to do it.

>> > --- a/include/config.h
>> > +++ b/include/config.h
>> > @@ -73,7 +73,7 @@
>> >  //#define FORCE_DUMMY_CONSOLE 1
>> >  
>> >  /* Enable this to do fast resets. Currently unreliable... */
>> > -//#define ENABLE_FAST_RESET	1
>> > +#define ENABLE_FAST_REBOOT	1
>> 
>> I think we could remove the config var and just do it via NVRAM
>> setting.
>
> If we have those, sure.

we do as of f0ef4e2d26a89deff97d50942f40a77edcc40867.

-- 
Stewart Smith
OPAL Architect, IBM.