[Skiboot] [PATCH v3 4/7] SLW: Add opal_slw_set_reg support for power9

[Stewart Smith]

Akshay Adiga <akshay.adiga at linux.vnet.ibm.com> writes:
> This OPAL call is made from Linux to OPAL to configure values in
> various SPRs after wakeup from a deep idle state.
>
> Signed-off-by: Akshay Adiga <akshay.adiga at linux.vnet.ibm.com>
> ---
>  hw/slw.c | 52 ++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 34 insertions(+), 18 deletions(-)

I know you're not the one who introduced the OPAL call, but it'd be
great if you could add in some documentation in doc/opal-api/ to cover
exactly what the behaviour/use of OPAL_SLW_SET_REG is.

A bit strange is how firmware is the only thing that's allowed to touch
HID0 (as all the bits move around and we need special incantations to
change the register), but we expect an OS to know that it needs to
explicitly set HID0 (and on some processors more) to be restored.

I gather we don't have any real written down rules about what registers
may/may not need restoring on future processors/stop states?

Maybe Vaidy knows the future direction here? Would be be making a safe
bet if we introduced some DT properties of exactly what registers should
be set to be restored from a firmware perspective?

> diff --git a/hw/slw.c b/hw/slw.c
> index c0ab9de..4600279 100644
> --- a/hw/slw.c
> +++ b/hw/slw.c
> @@ -30,6 +30,7 @@
>  #include <libfdt/libfdt.h>
>  #include <opal-api.h>
>
> +#include <p9_stop_api.H>
>  #include <p8_pore_table_gen_api.H>
>  #include <sbe_xip_image.h>
>
> @@ -1301,33 +1302,48 @@ int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val)
>  	assert(c);
>  	chip = get_chip(c->chip_id);
>  	assert(chip);
> -	image = (void *) chip->slw_base;
>
> -	/* Check of the SPR is supported by libpore */
> -	for ( i=0; i < SLW_SPR_REGS_SIZE ; i++)  {
> -		if (sprn == SLW_SPR_REGS[i].value)  {
> -			spr_is_supported = 1;
> -			break;
> +	if (chip->type == PROC_CHIP_P9_NIMBUS ||
> +	    chip->type == PROC_CHIP_P9_CUMULUS) {
> +		if (!chip->homer_base) {
> +			log_simple_error(&e_info(OPAL_RC_SLW_REG),
> +					 "SLW: HOMER base not set %x\n",
> +					 chip->id);
> +			return OPAL_INTERNAL_ERROR;
>  		}
> -	}
> -	if (!spr_is_supported) {
> -		log_simple_error(&e_info(OPAL_RC_SLW_REG),
> +		rc = p9_stop_save_cpureg((void *)chip->homer_base,
> +					 sprn, val, cpu_pir);
> +
> +	} else { /* Assuming its P8 */

I'd prefer we do an explicit check.

> +
> +		/* Check of the SPR is supported by libpore */
> +		for (i = 0; i < SLW_SPR_REGS_SIZE ; i++)  {
> +			if (sprn == SLW_SPR_REGS[i].value)  {
> +				spr_is_supported = 1;
> +				break;
> +			}
> +		}
> +		if (!spr_is_supported) {
> +			log_simple_error(&e_info(OPAL_RC_SLW_REG),
>  			"SLW: Trying to set unsupported spr for CPU %x\n",
> -			c->pir);
> -		return OPAL_UNSUPPORTED;
> +				c->pir);
> +			return OPAL_UNSUPPORTED;
> +		}
> +		image = (void *)chip->slw_base;
> +		rc = p8_pore_gen_cpureg_fixed(image, P8_SLW_MODEBUILD_SRAM,
> +					      sprn, val,
> +					      cpu_get_core_index(c),
> +					      cpu_get_thread_index(c));
>  	}
>
> -	rc = p8_pore_gen_cpureg_fixed(image, P8_SLW_MODEBUILD_SRAM, sprn,
> -						val, cpu_get_core_index(c),
> -						cpu_get_thread_index(c));
> -
>  	if (rc) {
>  		log_simple_error(&e_info(OPAL_RC_SLW_REG),
> -			"SLW: Failed to set spr for CPU %x\n",
> -			c->pir);
> +			"SLW: Failed to set spr %llx for CPU %x, RC=0x%x\n",
> +			sprn, c->pir, rc);
>  		return OPAL_INTERNAL_ERROR;
>  	}
> -
> +	prlog(PR_DEBUG, "SLW: restore spr:0x%llx on c:0x%x with 0x%llx\n",
> +	      sprn, c->pir, val);
>  	return OPAL_SUCCESS;
>
>  }
> -- 
> 2.5.5
>

-- 
Stewart Smith
OPAL Architect, IBM.