Re: [PATCH v4 3/3] PCI/AER: Enable AER on all PCIe devices supporting it

[Jonathan Derrick <jonathan.derrick@linux.dev>]

On 4/5/2022 11:16 PM, Stefan Roese wrote:
> On 4/4/22 22:22, Jonathan Derrick wrote:
>>
>>
>> On 1/25/2022 12:18 AM, Stefan Roese wrote:
>>> With this change, AER is now enabled on all PCIe devices, also when the
>>> PCIe device is hot-plugged.
>>>
>>> Please note that this change is quite invasive, as with this patch
>>> applied, AER now will be enabled in the Device Control registers of all
>>> available PCIe Endpoints, which currently is not the case.
>>>
>>> When "pci=noaer" is selected, AER stays disabled of course.
>>>
>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>> Cc: Bjorn Helgaas <helgaas@kernel.org>
>>> Cc: Pali Rohár <pali@kernel.org>
>>> Cc: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>> Cc: Yao Hongbo <yaohongbo@linux.alibaba.com>
>>> Cc: Naveen Naidu <naveennaidu479@gmail.com>
>>> ---
>>> v4:
>>> - No change
>>>
>>> v3:
>>> - New patch, replacing the "old" 2/2 patch
>>>    Now enabling of AER for each PCIe device is done in pci_aer_init(),
>>>    which also makes sure that AER is enabled in each PCIe device even 
>>> when
>>>    it's hot-plugged.
>>>
>>>   drivers/pci/pcie/aer.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
>>> index 5585fefc4d0e..10b2f7db8adb 100644
>>> --- a/drivers/pci/pcie/aer.c
>>> +++ b/drivers/pci/pcie/aer.c
>>> @@ -388,6 +388,10 @@ void pci_aer_init(struct pci_dev *dev)
>>>       pci_aer_clear_status(dev);
>>> +    /* Enable AER if requested */
>>> +    if (pci_aer_available())
>>> +        pci_enable_pcie_error_reporting(dev);
>> There are a lot of devices that do this explicitly [1]
>> May suggest a cleanup patch to follow-up?
> 
> Yes, good idea. I can try to work on this once this patchset is merged.
> 
>> [1] 
>> https://elixir.bootlin.com/linux/v5.18-rc1/A/ident/pci_enable_pcie_error_reporting 
>>
>>
>> ... Also a quirk list in the future for broken devices
> 
> IMHO this should only be done, when such devices are detected.
Yep; I'm just anticipating

> 
> Thanks,
> Stefan