[Skiboot] [PATCH 09/12] phb4: make endian-clean

[Nicholas Piggin]

Oliver O'Halloran's on October 1, 2019 3:47 pm:
> On Sun, 2019-09-29 at 17:46 +1000, Nicholas Piggin wrote:
>> Convert phb4 dt construction and in-memory hardware tables to use
>> explicit endian conversions.
>> 
>> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
>> ---
>>  hw/phb4.c      | 220 ++++++++++++++++++++++++++-----------------------
>>  include/phb4.h |   2 +-
>>  2 files changed, 116 insertions(+), 106 deletions(-)
>> 
>> diff --git a/hw/phb4.c b/hw/phb4.c
>> index 3c71427ae..f94a9995e 100644
>> --- a/hw/phb4.c
>> +++ b/hw/phb4.c
>> @@ -273,7 +273,7 @@ static int64_t phb4_pcicfg_check(struct phb4 *p, uint32_t bdfn,
>>  		return OPAL_HARDWARE;
>>  
>>  	/* Fetch the PE# from cache */
>> -	*pe = p->tbl_rtt[bdfn];
>> +	*pe = be16_to_cpu(p->tbl_rtt[bdfn]);
>>  
>>  	return OPAL_SUCCESS;
>>  }
>> @@ -923,7 +923,7 @@ static void phb4_init_ioda_cache(struct phb4 *p)
>>  	 * and this occurs before PEs have been assigned.
>>  	 */
>>  	for (i = 0; i < RTT_TABLE_ENTRIES; i++)
>> -		p->tbl_rtt[i] = PHB4_RESERVED_PE_NUM(p);
>> +		p->tbl_rtt[i] = cpu_to_be16(PHB4_RESERVED_PE_NUM(p));
>>  	memset(p->tbl_peltv, 0x0, p->tbl_peltv_size);
>>  	memset(p->tve_cache, 0x0, sizeof(p->tve_cache));
>>  
>> @@ -1748,110 +1748,123 @@ static void phb4_err_clear(struct phb4 *p)
>>  static void phb4_read_phb_status(struct phb4 *p,
>>  				 struct OpalIoPhb4ErrorData *stat)
>>  {
>> -	uint16_t val = 0;
>>  	uint32_t i;
>>  	uint64_t *pPEST;
>> +	uint16_t __16;
>> +	uint32_t __32;
>> +	uint64_t __64;
>>  *snip*
> 
> We consume the data that's read in phb4_read_phb_status() inside of
> skiboot (see phb4_read_phb_status()) so we probably want to handle any
> endian conversions elsewhere. Doing that in phb4_get_diag_data() would
> make sense since that's the entry point used by the OPAL API. That puts
> a bit of distance between where we read it and where we convert it
> though so... dunno.

Disagree, because OpalIoPhb4ErrorData is the OPAL API data type.

It's actually not used in skiboot except eeh_dump_regs (which
admittedly my patch missed converting :P).

>> @@ -5906,6 +5915,7 @@ static void phb4_probe_stack(struct dt_node *stk_node, uint32_t pec_index,
>>  		const void *leq = dt_prop_get_def_size(stk_node, "ibm,lane-eq",
>>  						       NULL, &leq_size);
>>  		if (leq != NULL && leq_size >= 6 * 8)
>> +			/* XXX */
>>  			dt_add_property(np, "ibm,lane-eq", leq, leq_size);
> 
> We're just copying the DT property so conversions shouldn't be needed.

Yeah I think you're right, I went through and added those comments
early on, looks like a stray.

Thanks,
Nick