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

[Benjamin Herrenschmidt]

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.

> 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 ?

> >  	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 ?

> 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) ?

> > diff --git a/hw/psi.c b/hw/psi.c
> > index 3efc177..bb55c10 100644
> > --- a/hw/psi.c
> > +++ b/hw/psi.c
> > @@ -432,34 +432,25 @@ static int64_t psi_p7_get_xive(struct
> > irq_source *is, uint32_t isn __unused,
> >  	return OPAL_SUCCESS;
> >  }
> >  
> > +static const uint32_t psi_p8_irq_to_xivr[P8_IRQ_PSI_ALL_COUNT] = {
> > +	[P8_IRQ_PSI_FSP]	= PSIHB_XIVR_FSP,
> > +	[P8_IRQ_PSI_OCC]	= PSIHB_XIVR_OCC,
> > +	[P8_IRQ_PSI_FSI]	= PSIHB_XIVR_FSI,
> > +	[P8_IRQ_PSI_LPC]	= PSIHB_XIVR_LPC,
> > +	[P8_IRQ_PSI_LOCAL_ERR]	= PSIHB_XIVR_LOCAL_ERR,
> > +	[P8_IRQ_PSI_HOST_ERR]	= PSIHB_XIVR_HOST_ERR,
> > +};
> > +
> 
> This hunk had a conflict with master IIRC.

Yeah not surprising... I can try to look into it later if you don't
beat me to 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.
> > 
> >  /* Enable this to make fast reboot clear memory */
> >  //#define FAST_REBOOT_CLEARS_MEMORY	1
> 
> We might want to do this, although that could come in future..
> getting all threads to clear a part of mem could be an easy way to do
> it quickly.

Yeah maybe... reboot on most machines don't clear RAM but I can see
some people screaming ;-) Even on FSP machines, with MPIPL these days
we may not be clearing it.

Cheers,
Ben.