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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <48ffde5c-084f-4ad6-8be7-314afb14b2ac@amd.com>
Date: Tue, 5 Aug 2025 15:10:19 -0700
From: Brett Creeley <bcreeley@....com>
To: Lukas Wunner <lukas@...ner.de>, Shannon Nelson <shannon.nelson@....com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
 edumazet@...gle.com, pabeni@...hat.com, brett.creeley@....com,
 drivers@...sando.io
Subject: Re: [PATCH net-next 1/3] pds_core: add simple AER handler


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