Re: [PATCH v2 2/4] x86/mce, EDAC/mce_amd: Add support for new MCA_SYND{1,2} registers

[Naik, Avadhut]

On 7/10/2024 04:38, Borislav Petkov wrote:
> On Tue, Jul 09, 2024 at 01:27:25AM -0500, Naik, Avadhut wrote:
> 
>> Userspace error decoding tools like the rasdaemon gather related hardware error
>> information through the tracepoints. As such, its important to have these two
>> registers in the tracepoint so that the tools like rasdaemon can parse them
>> and output the supplemental error information like FRU Text contained in them.
> 
> Put *that* in the commit message - do not explain what the patch does.
> 
Will do.
 
>> With this set, the first two elements of the vendor data dynamic array are SYND 1/2
>> registers while the third element is MCA_CONFIG (added through patch 4 of the set).
>> Now, in rasdaemon, SYND1/2 register contents (i.e. first two fields) are interpreted
>> as FRU Text only if BIT(9) of MCA_CONFIG (third field) is set.
>>
>> Thus, we depend on array's layout for accurate FRU Text decoding in the rasdaemon.
> 
> So it sounds to me like we want to document and thus freeze the
> vendor-specific blob layout because tools are going to be using and parsing
> it. And this will spare us the kernel version checks.
> 
> And new additions to that AMD-specific blob will come at the end and will
> have to be documented too.
> 
> That sounds like an ok compromise to me...
> 
> Thx.
>
Sounds good!
Is it okay to document this where the new wrapper and vendor-specific data
structures are being defined, in arch/x86/include/asm/mce.h?
Similar approach has been taken for struct mce.
Or do you have any other recommendations?

-- 
Thanks,
Avadhut Naik