Re: [PATCH net-next 1/3] pds_core: add simple AER handler

[Brett Creeley <bcreeley@amd.com>]

On 8/5/2025 8:01 AM, Lukas Wunner wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Fri, Feb 16, 2024 at 02:29:50PM -0800, Shannon Nelson wrote:
>> Set up the pci_error_handlers error_detected and resume to be
>> useful in handling AER events.
> 
> The above was committed as d740f4be7cf0 ("pds_core: add simple
> AER handler").
> 
> Just noticed the following while inspecting the pci_error_handlers
> of this driver:
> 
>> +static pci_ers_result_t pdsc_pci_error_detected(struct pci_dev *pdev,
>> +                                             pci_channel_state_t error)
>> +{
>> +     if (error == pci_channel_io_frozen) {
>> +             pdsc_reset_prepare(pdev);
>> +             return PCI_ERS_RESULT_NEED_RESET;
>> +     }
>> +
>> +     return PCI_ERS_RESULT_NONE;
>> +}
> 
> The ->error_detected() callback of this driver invokes
> pdsc_reset_prepare(), which unmaps BARs and calls pci_disable_device(),
> but there is no corresponding ->slot_reset() callback which would invoke
> pdsc_reset_done() to re-enable the device after reset recovery.
> 
> I don't have this hardware available for testing, hence do not feel
> comfortable submitting a fix.  But this definitely looks broken.

Hi Lukas,

Thanks for the note. It's been a bit since I have looked at this, but I 
believe that it's working in the following way:

1. pds_core's pci_error_handlers.error_detected callback returns 
PCI_ERS_RESULT_NEED_RESET
2. status is initialized to PCI_ERS_RESULT_RECOVERED in the pci core and 
since pds_core doesn't have a slot_reset callback then status remains 
PCI_ERS_RESULT_RECOVERED
3. pds_core's pci_error_handlers.resume callback is called, which will 
attempt reset/recover the device to a functional state

I also know that, at the very least, Shannon tested this when adding it 
to the driver.

Let me know if you still think otherwise.

Thanks again for the feedback,

Brett