[Skiboot] [PATCH v7 17/22] fadump: Add support to trigger memory preserving IPL on BMC system

[Vasant Hegde]

On 05/09/2019 09:34 AM, Michael Neuling wrote:
> On Sat, 2019-04-13 at 14:45 +0530, Vasant Hegde wrote:
>> On FSP based system we call 'attn' instruction. FSP detects attention and
>> initiates memory preserving IPL. On BMC system we have to call SBE S0
>> interrupt to initiate memory preserving IPL.
>>
>> This patch adds support to call SBE S0 interrupt in assert path.
>> Sequence :
>>    - S0 interrupt on slave chip SBE
>>    - S0 interrupt on master chip SBE
>>
>> Note that this is hooked to ipmi_terminate path. We have HDAT flag for MPIPL
>> support. If MPIPL is not supported then we don't create 'ibm,opal/dump' node
>> and we will fall back to existing termination flow.
>>
>> Note:
>>    - It uses xscom node to detect master and slave chip. So that we can
>>      trigger early MPIPL (even before initializing SBE driver).
>>    - At present we do not have a proper way to detect SBE is alive or not.
>>      So we wait for predefined time and then call normal reboot.
>>
>> Signed-off-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
>> ---
>>   hw/ipmi/ipmi-attn.c |  9 +++++++-
>>   hw/sbe-p9.c         | 63
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   include/sbe-p9.h    | 10 +++++++++
>>   3 files changed, 81 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ipmi/ipmi-attn.c b/hw/ipmi/ipmi-attn.c
>> index c6c1c59b5..ae97e92ab 100644
>> --- a/hw/ipmi/ipmi-attn.c
>> +++ b/hw/ipmi/ipmi-attn.c
>> @@ -1,4 +1,4 @@
>> -/* Copyright 2015 IBM Corp.
>> +/* Copyright 2015-2019 IBM Corp.
>>    *
>>    * Licensed under the Apache License, Version 2.0 (the "License");
>>    * you may not use this file except in compliance with the License.
>> @@ -19,6 +19,7 @@
>>   #include <pel.h>
>>   #include <platform.h>
>>   #include <processor.h>
>> +#include <sbe-p9.h>
>>   #include <skiboot.h>
>>   #include <stack.h>
>>   #include <timebase.h>
>> @@ -65,6 +66,12 @@ static void ipmi_log_terminate_event(const char *msg)
>>   
>>   void __attribute__((noreturn)) ipmi_terminate(const char *msg)
>>   {
>> +	/*
>> +	 * If fadump is supported then trigger SBE interrupt
>> +	 * to initiate fadump
>> +	 */
>> +	p9_sbe_terminate();
>> +
> 
> I'm not sure we want to change every platform do this on all ipmi_terminate.

I have check inside p9_sbe_terminate() to check whether MPIPL is supported or 
not. If
supported it triggers MPIPL. Otherwise it will fall back to normal assert 
path...which is reboot
on BMC system.

Initially I added p9_sbe_terminate -> ipmi_terminate() and then replace platform 
specific
"terminate" hook. But then to enable MPIPL on new platform (say boston) we need OPAL
changes as well.


> 
> Why doesn't this use the OPAL reboot type like FSP systems?

On FSP system, assert is slight different. We call "attn" instruction.

> 
>>   	/* Terminate called before initializing IPMI (early abort) */
>>   	if (!ipmi_present()) {
>>   		if (platform.cec_reboot)
>> diff --git a/hw/sbe-p9.c b/hw/sbe-p9.c
>> index ba7691f50..694375a5a 100644
>> --- a/hw/sbe-p9.c
>> +++ b/hw/sbe-p9.c
>> @@ -948,3 +948,66 @@ void p9_sbe_init(void)
>>   	/* Initiate SBE timeout poller */
>>   	opal_add_poller(p9_sbe_timeout_poll, NULL);
>>   }
>> +
>> +/* Terminate and initiate MPIPL */
>> +void p9_sbe_terminate(void)
>> +{
>> +	uint32_t chip_id;
>> +	uint32_t primary_chip = -1;
>> +	int rc;
>> +	u64 wait_tb;
>> +	struct dt_node *xn;
>> +
>> +	if (proc_gen < proc_gen_p9)
>> +		return;
>> +
>> +	/* Return if MPIPL is not supported */
>> +	if (!dt_find_by_path(opal_node, "dump"))
>> +		return;
>> +
>> +	dt_for_each_compatible(dt_root, xn, "ibm,xscom") {
>> +		chip_id = dt_get_chip_id(xn);
>> +
>> +		if (dt_has_node_property(xn, "primary", NULL)) {
>> +			primary_chip = chip_id;
>> +			continue;
>> +		}
>> +
>> +		rc = xscom_write(chip_id,
>> +				 SBE_CONTROL_REG_RW, SBE_CONTROL_REG_S0);
>> +		/* Initiate normal reboot */
>> +		if (rc) {
>> +			prlog(PR_ERR, "Failed to write S0 interrupt [chip id =
>> %x]\n",
>> +			      chip_id);
>> +			return;
>> +		}
>> +	}
> 
> A comment as to what this above code is doing would be useful.  Finding the
> primary chip?

Yes. Fixed.

> 
>> +
>> +	if (primary_chip == -1) {
>> +		prlog(PR_ERR, "Master chip ID not found.\n");
> 
> Master or primary? Don't change language arbitrarily.

Sure. Fixed.


> 
>> +		return;
>> +	}
>> +
>> +	/* Write S0 interrupt on master SBE */
> 
> No comment needed above. It's clear what it's doing from the code.
> 
>> +	rc = xscom_write(primary_chip,
>> +			 SBE_CONTROL_REG_RW, SBE_CONTROL_REG_S0);
>> +	/* Initiate normal reboot */
> 
> this comment is useful, but should be up one line

Fixed.

-Vasant