[Skiboot] [PATCH V2 5/7] hmi: Rework HMI event handling of FIR read failure

[Andrew Donnellan]

On 21/03/16 12:00, Russell Currey wrote:
> If reading the FIR with XSCOM failed, the existing code would not raise
> a HMI under the assumption that the CPU was asleep and nothing is wrong.
> Now that it is possible to check whether or not the CPU was asleep,
> raise an unrecoverable HMI if the read failed for other reasons, and
> skip the CPU if it was asleep.
>
> If the CPU is asleep when it's not meant to be and that is the cause of
> the HMI, an unrecoverable "catchall" HMI will be raised since no other
> components will claim responsibility for it.
>
> Signed-off-by: Russell Currey <ruscur at russell.cc>
> ---
> V2: Raise HMI on XSCOM fail for non-sleep reasons, completely reword
> ---
>   core/hmi.c | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/core/hmi.c b/core/hmi.c
> index 127686f..71fdf48 100644
> --- a/core/hmi.c
> +++ b/core/hmi.c
> @@ -299,6 +299,7 @@ static bool decode_core_fir(struct cpu_thread *cpu,
>   	uint32_t core_id;
>   	int i;
>   	bool found = false;
> +	int64_t ret;

As per comment on patch 1, should this just be "int" to match the 
declaration of xscom_read()?

>
>   	/* Sanity check */
>   	if (!cpu || !hmi_evt)
> @@ -307,9 +308,23 @@ static bool decode_core_fir(struct cpu_thread *cpu,
>   	core_id = pir_to_core_id(cpu->pir);
>
>   	/* Get CORE FIR register value. */
> -	if (xscom_read(cpu->chip_id, XSCOM_ADDR_P8_EX(core_id, CORE_FIR),
> -							&core_fir) != 0) {
> +	ret = xscom_read(cpu->chip_id, XSCOM_ADDR_P8_EX(core_id, CORE_FIR),
> +			 &core_fir);
> +
> +	if (ret == OPAL_HARDWARE) {
>   		prerror("HMI: XSCOM error reading CORE FIR\n");
> +		/* If the FIR can't be read, we should checkstop. */
> +		return true;
> +	} else if (ret == OPAL_WRONG_STATE) {
> +		/*
> +		 * CPU is asleep, so it probably didn't cause the checkstop.
> +		 * If no other HMI cause is found a "catchall" checkstop
> +		 * will be raised, so if this CPU should've been awake the
> +		 * error will be handled appropriately.
> +		 */
> +		prlog(PR_DEBUG,
> +		      "HMI: FIR read failed, chip %d core %d asleep\n",
> +		      cpu->chip_id, core_id);
>   		return false;
>   	}
>
>

Rest doesn't look bad.

-- 
Andrew Donnellan              Software Engineer, OzLabs
andrew.donnellan at au1.ibm.com  Australia Development Lab, Canberra
+61 2 6201 8874 (work)        IBM Australia Limited