[Skiboot] [PATCH] dts: spl_wakeup: Remove all workarounds in the spl wakeup logic

Vaidyanathan Srinivasan svaidy at linux.vnet.ibm.com
Fri Mar 9 17:56:49 AEDT 2018


* Shilpa Bhat <shilpa.bhat at linux.vnet.ibm.com> [2018-03-09 10:47:32]:

> We coded few workarounds in special wakeup logic to handle the
> buggy firmware. Now that is fixed remove them as they break the
> special wakeup protocol. As per the spec we should not de-assert
> beofre assert is complete. So follow this protocol.
> 
> Signed-off-by: Shilpasri G Bhat <shilpa.bhat at linux.vnet.ibm.com>
Reviewed-by: Vaidyanathan Srinivasan <svaidy at linux.vnet.ibm.com>
Tested-by: Vaidyanathan Srinivasan <svaidy at linux.vnet.ibm.com>



> ---
>  core/direct-controls.c | 59 +++++++++++++++++++++++++-------------------------
>  hw/dts.c               | 30 +------------------------
>  2 files changed, 30 insertions(+), 59 deletions(-)
> 
> diff --git a/core/direct-controls.c b/core/direct-controls.c
> index 45a4008..c7b9c9b 100644
> --- a/core/direct-controls.c
> +++ b/core/direct-controls.c
> @@ -311,7 +311,7 @@ static int p9_core_set_special_wakeup(struct cpu_thread *cpu)
>  		prlog(PR_ERR, "Could not set special wakeup on %u:%u:"
>  				" Unable to write PPM_SPECIAL_WKUP_HYP.\n",
>  				chip_id, core_id);
> -		return OPAL_HARDWARE;
> +		goto out_fail;
>  	}
>  
>  	for (i = 0; i < P9_SPWKUP_TIMEOUT / P9_SPWKUP_POLL_INTERVAL; i++) {
> @@ -321,9 +321,22 @@ static int p9_core_set_special_wakeup(struct cpu_thread *cpu)
>  					chip_id, core_id);
>  			goto out_fail;
>  		}
> -		if (val & P9_SPECIAL_WKUP_DONE)
> -			return 0;
> -
> +		if (val & P9_SPECIAL_WKUP_DONE) {
> +			/*
> +			 * CORE_GATED will be unset on a successful special
> +			 * wakeup of the core which indicates that the core is
> +			 * out of stop state. If CORE_GATED is still set then
> +			 * raise error.
> +			 */
> +			if (dctl_core_is_gated(cpu)) {
> +				prlog(PR_ERR, "Failed special wakeup on %u:%u"
> +						" as CORE_GATED is set\n",
> +						chip_id, core_id);
> +				goto out_fail;
> +			} else {
> +				return 0;
> +			}
> +		}
>  		time_wait_us(P9_SPWKUP_POLL_INTERVAL);
>  	}
>  
> @@ -332,10 +345,11 @@ static int p9_core_set_special_wakeup(struct cpu_thread *cpu)
>  			chip_id, core_id);
>  
>  out_fail:
> -	/* De-assert special wakeup after a small delay. */
> -	time_wait_us(1);
> -	xscom_write(chip_id, swake_addr, 0);
> -
> +	/*
> +	 * As per the special wakeup protocol we should not de-assert
> +	 * the special wakeup on the core until WAKEUP_DONE is set.
> +	 * So even on error do not de-assert.
> +	 */
>  	return OPAL_HARDWARE;


Recent learning around this issue suggest that software should not
clear special wakeup until it is set.  Otherwise the special wakeup
state machine gets into more errors and blocks the original failing
condition.

In ideal cases this should never happen.  There are fixes in microcode
to harden this area.  Hence as requested by hardware and microcode
team, leaving the special wakeup state machine as is on timeout.

Next attempt may succeed or fail, but will continue to leave the state
machine at the first error/failing state for debug by hardware team.

--Vaidy



More information about the Skiboot mailing list