[PATCH v7 4/9] powerpc/pseries: Define MCE error event section.

[Michael Ellerman]

Mahesh Jagannath Salgaonkar <mahesh at 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
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;

cheers