Re: [PATCH v2] PCI/AER: Consolidate CXL, ACPI GHES and native AER reporting paths

[Karolina Stolarek <karolina.stolarek@oracle.com>]

On 01/04/2025 03:47, Jon Pan-Doh wrote:
> On Tue, Mar 25, 2025 at 8:08 AM Karolina Stolarek
> <karolina.stolarek@oracle.com> wrote:
>>
>> +static void populate_aer_err_info(struct aer_err_info *info, int severity,
>> +                                 struct aer_capability_regs *aer_regs)
>>   {
>> [...]
>> +       if (severity == AER_CORRECTABLE) {
>> +               info->id = aer_regs->cor_err_source;
>> +               info->status = aer_regs->cor_status;
>> +               info->mask = aer_regs->cor_mask;
>> +       } else {
>> +               info->id = aer_regs->uncor_err_source;
> 
> info->id isn't filled in pci_print_aer(). Is the addition due to
> aer_print_error() having a conditional/log for info->id == id? From a
> brief look at the code, FW-first (e.g. errors from GHES), I think it
> will always be true. However, that doesn't always seem to be the case
> for CXL (e.g. when cxl_dev_state.rcd == true).

I just went with what aer_print_error() expected to get from the 
aer_err_info struct. It's possible that this id check is not be needed 
for GHES, but I thought that as the values are already in the regs, it 
wouldn't hurt to extract them.

> 
> Disclaimer: not a CXL/GHES expert though.
> 
>> +void aer_print_platform_error(struct pci_dev *pdev, int severity,
>> +                             struct aer_capability_regs *aer_regs)
> 
> I like the encapsulation.
> 
> Reviewed-by: Jon Pan-Doh <pandoh@google.com>

Thanks a lot!

All the best,
Karolina

> 
> Thanks,
> Jon