Re: [PATCH v4 18/25] powernv/fadump: process architected register state data provided by firmware

[Hari Bathini <hbathini@linux.ibm.com>]

On 14/08/19 10:45 PM, Mahesh J Salgaonkar wrote:
> On 2019-07-16 17:04:08 Tue, Hari Bathini wrote:
>> From: Hari Bathini <hbathini@linux.vnet.ibm.com>
>>
>> Firmware provides architected register state data at the time of crash.
>> Process this data and build CPU notes to append to ELF core.
>>
>> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
>> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/kernel/fadump-common.h          |    4 +
>>  arch/powerpc/platforms/powernv/opal-fadump.c |  197 ++++++++++++++++++++++++--
>>  arch/powerpc/platforms/powernv/opal-fadump.h |   39 +++++
>>  3 files changed, 228 insertions(+), 12 deletions(-)
>>
> [...]
>> @@ -430,6 +577,32 @@ int __init opal_fadump_dt_scan(struct fw_dump *fadump_conf, ulong node)
>>  			return 1;
>>  		}
>>  
>> +		ret = opal_mpipl_query_tag(OPAL_MPIPL_TAG_CPU, &addr);
>> +		if ((ret != OPAL_SUCCESS) || !addr) {
>> +			pr_err("Failed to get CPU metadata (%lld)\n", ret);
>> +			return 1;
>> +		}
>> +
>> +		addr = be64_to_cpu(addr);
>> +		pr_debug("CPU metadata addr: %llx\n", addr);
>> +
>> +		opal_cpu_metadata = __va(addr);
>> +		r_opal_cpu_metadata = (void *)addr;
>> +		fadump_conf->cpu_state_data_version =
>> +			be32_to_cpu(r_opal_cpu_metadata->cpu_data_version);
>> +		if (fadump_conf->cpu_state_data_version !=
>> +		    HDAT_FADUMP_CPU_DATA_VERSION) {
>> +			pr_err("CPU data format version (%lu) mismatch!\n",
>> +			       fadump_conf->cpu_state_data_version);
>> +			return 1;
>> +		}

I think cpu data version mismatch check should still be done early on?

>> +		fadump_conf->cpu_state_entry_size =
>> +			be32_to_cpu(r_opal_cpu_metadata->cpu_data_size);
>> +		fadump_conf->cpu_state_destination_addr =
>> +			be64_to_cpu(r_opal_cpu_metadata->region[0].dest);
>> +		fadump_conf->cpu_state_data_size =
>> +			be64_to_cpu(r_opal_cpu_metadata->region[0].size);
>> +
> 
> opal_fadump_dt_scan isn't the right place to do this. Can you please move above
> cpu related data processing to opal_fadump_build_cpu_notes() ?

I will move the above cpu related data processing to opal_fadump_build_cpu_notes().

Thanks
Hari