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: <3cdbb79a-7c98-4172-9c74-4ae968054fda@onemain.com>
Date: Tue, 5 Aug 2025 15:28:40 -0700
From: Shannon Nelson <sln@...main.com>
To: Brett Creeley <bcreeley@....com>, 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/25 3:10 PM, Brett Creeley wrote:
> 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

Thanks, Brett - yes this sounds about right by my memory.  The resume 
callback takes care of getting the device reset and re -enabled.

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

Yep, several cycles, several scenarios.

Cheers,
sln

>
> Let me know if you still think otherwise.
>
> Thanks again for the feedback,
>
> Brett
>
>