diff mbox series

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

Message ID 20220125071820.2247260-4-sr@denx.de (mailing list archive)
State Accepted
Delegated to: Bjorn Helgaas
Headers show
Series Fully enable AER | expand

Commit Message

Stefan Roese Jan. 25, 2022, 7:18 a.m. UTC
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(+)

Comments

Pali Rohár Jan. 25, 2022, 2:53 p.m. UTC | #1
On Tuesday 25 January 2022 08:18:20 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>

Reviewed-by: Pali Rohár <pali@kernel.org>

> ---
> 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);
> +
>  	/* Enable ECRC checking if enabled and configured */
>  	pcie_set_ecrc_checking(dev);
>  }
> -- 
> 2.35.0
>
Jonathan Derrick April 4, 2022, 8:22 p.m. UTC | #2
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?

[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

> +
>   	/* Enable ECRC checking if enabled and configured */
>   	pcie_set_ecrc_checking(dev);
>   }
Stefan Roese April 6, 2022, 5:16 a.m. UTC | #3
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.

Thanks,
Stefan
Jonathan Derrick April 6, 2022, 2:49 p.m. UTC | #4
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
diff mbox series

Patch

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);
+
 	/* Enable ECRC checking if enabled and configured */
 	pcie_set_ecrc_checking(dev);
 }