[Skiboot] [PATCH v7 21/26] opal-dump: MPIPL endan conversions

[Nicholas Piggin]

Vasant Hegde's on November 13, 2019 2:08 am:
> On 11/11/19 11:19 AM, Nicholas Piggin wrote:
>> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
>> ---
>>   core/opal-dump.c   | 80 +++++++++++++++++++++++++---------------------
>>   include/opal-api.h | 14 ++++----
>>   2 files changed, 51 insertions(+), 43 deletions(-)
>> 
> 
> 
> .../...
> 
>> @@ -426,13 +434,13 @@ static void post_mpipl_arch_regs_data(void)
>> 
>>   	/* Fill CPU register details */
>>   	opal_mpipl_cpu_data->version = OPAL_MPIPL_VERSION;
>> -	opal_mpipl_cpu_data->cpu_data_version = proc_dump->version;
>> +	opal_mpipl_cpu_data->cpu_data_version = cpu_to_be32((u32)proc_dump->version);
>>   	opal_mpipl_cpu_data->cpu_data_size = proc_dump->thread_size;
>>   	opal_mpipl_cpu_data->region_cnt = cpu_to_be32(1);
>> 
>> -	opal_mpipl_cpu_data->region[0].src  = proc_dump->dest_addr & ~(HRMOR_BIT);
>> -	opal_mpipl_cpu_data->region[0].dest = proc_dump->dest_addr & ~(HRMOR_BIT);
>> -	opal_mpipl_cpu_data->region[0].size = proc_dump->act_size;
>> +	opal_mpipl_cpu_data->region[0].src  = proc_dump->dest_addr & ~(cpu_to_be64(HRMOR_BIT));
>> +	opal_mpipl_cpu_data->region[0].dest = proc_dump->dest_addr & ~(cpu_to_be64(HRMOR_BIT));
>> +	opal_mpipl_cpu_data->region[0].size = cpu_to_be64(be32_to_cpu(proc_dump->act_size));
>> 
>>   	/* Update tag */
>>   	opal_mpipl_tags[OPAL_MPIPL_TAG_CPU] = (u64)opal_mpipl_cpu_data;
> 
> We should convert opal_mpipl_cpu_data to BE here.

Ah, missed that because I didn't annotate the opal call types.

What about just doing the conversion in the opal calls? That tends to
be the usual pattern is we keep our internal data structures in native
endian, and then do the conversion at the point where we need to load/
store hardware and OS memory.

> 
>> @@ -442,7 +450,7 @@ static void post_mpipl_get_opal_data(void)
>>   {
>>   	struct mdrt_table *mdrt = (void *)(MDRT_TABLE_BASE);
>>   	int i, j = 0, count = 0;
>> -	u32 mdrt_cnt = ntuple_mdrt->act_cnt;
>> +	int mdrt_cnt = be16_to_cpu(ntuple_mdrt->act_cnt);
>>   	struct opal_mpipl_region *region;
>> 
>>   	/* Count OPAL dump regions */
>> @@ -466,8 +474,8 @@ static void post_mpipl_get_opal_data(void)
>> 
>>   	/* Fill OPAL dump details */
>>   	opal_mpipl_data->version = OPAL_MPIPL_VERSION;
>> -	opal_mpipl_data->crashing_pir = mpipl_metadata->crashing_pir;
>> -	opal_mpipl_data->region_cnt = count;
>> +	opal_mpipl_data->crashing_pir = cpu_to_be32(mpipl_metadata->crashing_pir);
>> +	opal_mpipl_data->region_cnt = cpu_to_be32(count);
>>   	region = opal_mpipl_data->region;
>> 
>>   	mdrt = (void *)(MDRT_TABLE_BASE);
>> @@ -477,9 +485,9 @@ static void post_mpipl_get_opal_data(void)
>>   			continue;
>>   		}
>> 
>> -		region[j].src  = mdrt->src_addr  & ~(HRMOR_BIT);
>> -		region[j].dest = mdrt->dest_addr & ~(HRMOR_BIT);
>> -		region[j].size = mdrt->size;
>> +		region[j].src  = mdrt->src_addr  & ~(cpu_to_be64(HRMOR_BIT));
>> +		region[j].dest = mdrt->dest_addr & ~(cpu_to_be64(HRMOR_BIT));
>> +		region[j].size = cpu_to_be64(be32_to_cpu(mdrt->size));
>> 
>>   		prlog(PR_NOTICE, "OPAL reserved region %d - src : 0x%llx, "
>>   		      "dest : 0x%llx, size : 0x%llx\n", j, region[j].src,
> 
> Can you fix above prlog as well?
> 
> Also we have to convert `opal_mpipl_data` to BE before adding that to 
> `opal_mpipl_tags`.

Yeah I'll take another look at these. I'll do another tree-wide pass
over the opal call APIs to make sure I haven't missed any other type
annotations.

Thanks,
Nick