[Skiboot] [PATCH v7 18/26] fsp: endian conversions

[Vasant Hegde]

On 11/13/19 8:11 AM, Nicholas Piggin wrote:
> Vasant Hegde's on November 12, 2019 8:42 pm:
>> On 11/11/19 11:19 AM, Nicholas Piggin wrote:
>>> Reviewed-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
>>> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
>>> ---
>>>    hw/fsp/fsp-codeupdate.c |  4 +-
>>>    hw/fsp/fsp-codeupdate.h | 86 ++++++++++++++++++++---------------------
>>>    hw/fsp/fsp-sysdump.c    |  2 +-
>>>    hw/fsp/fsp-sysparam.c   |  2 +-
>>>    hw/fsp/fsp.c            | 16 ++++----
>>
>>
>> I tried to look into couple of drivers under hw/fsp/* (not yet looked into all
>> files).
>> Here are few places we need fixes.
>>
>> hw/fsp/fsp-attn.c
>>
>> 109         ti_attn->msg_len = VERSION_LEN + BT_FRAME_LEN +
>> 110                                    strlen(ti_attn->msg.file_info);
>> 111 }
>>
>>
>> hw/fsp/fsp-dump.c
>>
>> 376         *dump_id = record->id;
>> 377         *dump_size = record->size;
>> 378         *dump_type = record->type;
>>
>> Data returned by fsp_get_sys_param() needs to be converted (we do TCE mapping of
>> buffer and FSP
>> will fill BE data).
> 
> Okay, I can do another pass over it and take another look. If we give
> the OPAL APIs and hardware / in-memory structures the right endian
> annotated types, then it mostly becomes a mechanical process of
> following sparse.
> 
>> I tried to boot ZZ with this patch series (both LE and BE mode). its not booting
>> for me. I suspect HDAT related changes causing some issue (haven't looked into
>> it in detail).
> 
> Thanks, I'll see if I can fix it. We can't be breaking BE builds.

I did another round of test with clean boot. System booting fine with BE build, but
interestingly  BE DEBUG build is failing to boot. That's something to debug.

> 
>>
>> fsp_mkmsg() usage needs careful audit .. as fsp_wreg() -> out_be32() converts
>> data from cpu_to_be().
> 
> I don't expect everything to work with LE after this series, but basic
> witherspoon is working so it gives a starting point. I'll have a bit
> more look over the FSP conversions. If it becomes tricky and you'd like
> more time to go over it, I can take it out of the initial series.

I think as long as we don't break BE stuff its fine to pus changes. No need to 
take out
FSP related changes.. Once we have base code merged we can fix rest of the things to
make it work in ZZ.

-Vasant