[Skiboot] [PATCH v7 12/22] hdata: Add result table property to "dump" node

[Vasant Hegde]

On 05/09/2019 06:46 AM, Michael Neuling wrote:
> On Sat, 2019-04-13 at 14:45 +0530, Vasant Hegde wrote:
>> During MPIPL hostboot updates MDRT table with details of memory movement
>> (source, destination address, size, etc). Convert these details to fadump
>> structure format and pass it to kernel via 'result-table' property.
>>
>> Device tree property:
>>    /ibm,opal/dump/result-table - follows fadump structure format
>>
>> Signed-off-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
>> ---
>>   hdata/spira.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   hdata/spira.h |  4 +++
>>   2 files changed, 97 insertions(+)
>>
>> diff --git a/hdata/spira.c b/hdata/spira.c
>> index 34e3034ce..fa6149322 100644
>> --- a/hdata/spira.c
>> +++ b/hdata/spira.c
>> @@ -26,6 +26,8 @@
>>   #include <fsp-attn.h>
>>   #include <fsp-leds.h>
>>   #include <skiboot.h>
>> +#include <mem_region-malloc.h>
>> +#include <opal-dump.h>
>>   
>>   #include "hdata.h"
>>   #include "hostservices.h"
>> @@ -1055,6 +1057,94 @@ static void dt_init_secureboot_node(const struct iplparams_sysparams *sysparams)
>>   	dt_add_property_cells(node, "hw-key-hash-size", hw_key_hash_size);
>>   }
>>   
>> +static void opal_dump_add_result_table(const struct iplparams_iplparams *p)
>> +{
>> +	int i, j = 0;
>> +	u32 mdrt_cnt = spira.ntuples.mdump_res.act_cnt;
>> +	u32 mdrt_max_cnt = MDRT_TABLE_SIZE / sizeof(struct mdrt_table);
>> +	size_t prop_size;
>> +	struct mdrt_table *mdrt = (void *)(MDRT_TABLE_BASE);
>> +	struct fadump_section *fadump_section;
>> +	struct fadump *result_table;
>> +	struct dt_node *dump_node;
>> +
>> +	dump_node = dt_find_by_path(opal_node, "dump");
>> +	if (!dump_node)
>> +		return;
>> +
>> +	/* Check boot params to detect MPIPL boot or not */
>> +	if (p->cec_ipl_maj_type != IPLPARAMS_MAJ_TYPE_REIPL)
>> +		return;
>> +	/*
>> +	 * On FSP system we get minor type as post dump IPL and on BMC system
>> +	 * we get platform reboot. Hence lets check for both values
>> +	 */
>> +	if (p->cec_ipl_min_type != IPLPARAMS_MIN_TYPE_POST_DUMP &&
>> +	    p->cec_ipl_min_type != IPLPARAMS_MIN_TYPE_PLAT_REBOOT) {
>> +		prlog(PR_DEBUG, "DUMP: Non MPIPL reboot "
>> +		      "[minor type = 0x%x]\n", p->cec_ipl_min_type);
>> +		return;
>> +	}
>> +	if (p->cec_ipl_attrib != IPLPARAMS_ATTRIB_MEM_PRESERVE) {
>> +		prlog(PR_DEBUG, "DUMP: Memory not preserved\n");
>> +		return;
>> +	}
>> +
>> +	if (mdrt_cnt == 0 || mdrt_cnt >= mdrt_max_cnt) {
>> +		prlog(PR_DEBUG, "DUMP: Invalid MDRT count : %x\n", mdrt_cnt);
>> +		return;
>> +	}
>> +
>> +	prlog(PR_NOTICE, "DUMP: Dump found, MDRT count = 0x%x\n", mdrt_cnt);
>> +
>> +	/* Calculcate property size */
>> +	prop_size = sizeof(struct fadump) +
>> +		(mdrt_cnt * sizeof(struct fadump_section));
>> +	result_table = zalloc(prop_size);
>> +	if (!result_table) {
>> +		prlog(PR_ERR, "DUMP: Failed to allocate memory\n");
>> +		return;
>> +	}
>> +
>> +	/* Copy MDRT entries to result-table */
>> +	result_table->fadump_section_size = sizeof(struct fadump_section);
>> +	for (i = 0; i < mdrt_cnt; i++) {
>> +		/* Skip garbage entry */
>> +		if ((mdrt->dest_addr == 0) || (mdrt->size == 0)) {
>> +			mdrt++;
>> +			continue;
>> +		}
>> +
>> +		if (mdrt->dump_type != DUMP_TYPE_FADUMP) {
>> +			mdrt++;
>> +			continue;
>> +		}
>> +
>> +		fadump_section = &(result_table->section[j++]);
>> +		fadump_section->source_type = mdrt->data_region;
>> +		/* Clear top bit */
>> +		fadump_section->source_addr =
>> +			cpu_to_be64(mdrt->src_addr & ~(HRMOR_BIT));
>> +		fadump_section->dest_addr =
>> +			cpu_to_be64(mdrt->dest_addr & ~(HRMOR_BIT));
>> +		fadump_section->source_size = cpu_to_be64(mdrt->size);
>> +		fadump_section->dest_size = cpu_to_be64(mdrt->size);
>> +		mdrt++;
>> +	}
>> +
>> +	if (j == 0) {
>> +		prlog(PR_DEBUG, "DUMP: MDRT table is empty\n");
>> +		free(result_table);
>> +		return;
>> +	}
>> +
>> +	result_table->section_count = cpu_to_be16(j);
>> +	/* Actual property size */
>> +	prop_size = sizeof(struct fadump) + (j * sizeof(struct fadump_section));
>> +	dt_add_property(dump_node, "result-table", result_table, prop_size);
> 
> This concerns me, especially since it's an API.

We have 'compatible' property inside /ibm,opal/dump node. This will help to identify
if there is any change in interface or structure.

> 
> How is this structure communicated to the kernel? This seems to be a large flat
> structure passed from opal to the kernel here.

Yes. This is common structure used to pass information between OPAL and kernel.
Post MPIPL OPAL passes "result-table" property to kernel. This property uses
"fadump" structure.

Also during fadump registration kernel uses this structure to pass information
to OPAL.

> 
> What happens if we need to make a change to this format in future?  How are we
> going to communicate that to the kernel?  Do we need a compat or version
> property here?

Yes. We have "compatible" property. Kernel will check for this property.

> 
> Why don't we represent the structure more in the device tree, rather than
> flattening it here?


We need structure anyway. So added as single property.


-Vasant