On 08/16/2018 09:44 AM, Michael Ellerman wrote:
> Mahesh Jagannath Salgaonkar <mah...@linux.vnet.ibm.com> writes:
>> On 08/08/2018 08:12 PM, Michael Ellerman wrote:
> ...
>>>
>>>> + union {
>>>> + struct {
>>>> + uint8_t ue_err_type;
>>>> + /* XXXXXXXX
>>>> + * X 1: Permanent or Transient UE.
>>>> + * X 1: Effective address provided.
>>>> + * X 1: Logical address provided.
>>>> + * XX 2: Reserved.
>>>> + * XXX 3: Type of UE error.
>>>> + */
>>>
>>> But which bit is bit 0? And is that the LSB or MSB?
>>
>> RTAS errorlog data in BE format, the leftmost bit is MSB 0 (1: Permanent
>> or Transient UE.). I Will update the comment above that properly points
>> out which one is MSB 0.
>>
>>>
>>>
>>>> + uint8_t reserved_1[6];
>>>> + __be64 effective_address;
>>>> + __be64 logical_address;
>>>> + } ue_error;
>>>> + struct {
>>>> + uint8_t soft_err_type;
>>>> + /* XXXXXXXX
>>>> + * X 1: Effective address provided.
>>>> + * XXXXX 5: Reserved.
>>>> + * XX 2: Type of SLB/ERAT/TLB error.
>>>> + */
>>>> + uint8_t reserved_1[6];
>>>> + __be64 effective_address;
>>>> + uint8_t reserved_2[8];
>>>> + } soft_error;
>>>> + } u;
>>>> +};
>>>> +#pragma pack(pop)
>>>
>>> Why not __packed ?
>>
>> Because when used __packed it added 1 byte extra padding between
>> reserved_1[6] and effective_address. That caused wrong effective address
>> to be printed on the console. Hence I switched to #pragma pack to force
>> 1 byte alignment for this structure alone.
>
> OK, that's weird.
>
> Do we really need to bother with all the union stuff? The only
> difference is the field names, and whether logical address has a value
Also the bit fields for UE and other sub errors differ. Yeah but we can
do away with union stuff.
> or not. What about:
>
> struct pseries_mc_errorlog {
> __be32 fru_id;
> __be32 proc_id;
> u8 error_type;
> u8 sub_error_type;
> u8 reserved_1[6];
> __be64 effective_address;
> __be64 logical_address;
> } __packed;
Sure will do.
Thanks
-Mahesh.
>
> cheers
>
