On Mon, Mar 14, 2022 at 09:21:46AM -0700, Eric Badger wrote: > On Sun, Mar 13, 2022 at 02:43:14PM -0700, Raj, Ashok wrote: > > On Sun, Mar 13, 2022 at 02:52:20PM -0500, Bjorn Helgaas wrote: > > > On Fri, Mar 11, 2022 at 02:58:07AM +0000, Kuppuswamy Sathyanarayanan wrote: > > > > Currently the aer_irq() handler returns IRQ_NONE for cases without bits > > > > PCI_ERR_ROOT_UNCOR_RCV or PCI_ERR_ROOT_COR_RCV are set. But this > > > > assumption is incorrect. > > > > > > > > Consider a scenario where aer_irq() is triggered for a correctable > > > > error, and while we process the error and before we clear the error > > > > status in "Root Error Status" register, if the same kind of error > > > > is triggered again, since aer_irq() only clears events it saw, the > > > > multi-bit error is left in tact. This will cause the interrupt to fire > > > > again, resulting in entering aer_irq() with just the multi-bit error > > > > logged in the "Root Error Status" register. > > > > > > > > Repeated AER recovery test has revealed this condition does happen > > > > and this prevents any new interrupt from being triggered. Allow to > > > > process interrupt even if only multi-correctable (BIT 1) or > > > > multi-uncorrectable bit (BIT 3) is set. > > > > > > > > Reported-by: Eric Badger <ebadger@xxxxxxxxxxxxxxx> > > > > > > Is there a bug report with any concrete details (dmesg, lspci, etc) > > > that we can include here? > > > > Eric might have more details to add when he collected numerous logs to get > > to the timeline of the problem. The test was to stress the links with an > > automated power off, this will result in some eDPC UC error followed by > > link down. The recovery worked fine for several cycles and suddenly there > > were no more interrupts. A manual rescan on pci would probe and device is > > operational again. > > The problem was originally discovered while performing a looping hot plug > test. At hot remove time, one or more corrected errors usually appeared: > > [256236.078151] pcieport 0000:89:02.0: AER: Corrected error received: 0000:89:02.0 > [256236.078154] pcieport 0000:89:02.0: AER: PCIe Bus Error: severity=Corrected, type=Physical Layer, (Receiver ID) > [256236.088606] pcieport 0000:89:02.0: AER: device [8086:347a] error status/mask=00000001/00000000 > [256236.097857] pcieport 0000:89:02.0: AER: [ 0] RxErr > [256236.152622] pcieport 0000:89:02.0: pciehp: Slot(400): Link Down > [256236.152623] pcieport 0000:89:02.0: pciehp: Slot(400): Card not present > [256236.152631] pcieport 0000:89:02.0: DPC: containment event, status:0x1f01 source:0x0000 > [256236.152632] pcieport 0000:89:02.0: DPC: unmasked uncorrectable error detected reason 0 ext_reason 0 > [256236.152634] pcieport 0000:89:02.0: AER: PCIe Bus Error: severity=Uncorrected (Fatal), type=Transaction Layer, (Receiver ID) > [256236.164207] pcieport 0000:89:02.0: AER: device [8086:347a] error status/mask=00000020/00100000 > [256236.173464] pcieport 0000:89:02.0: AER: [ 5] SDES (First) > [256236.278407] pci 0000:8a:00.0: Removing from iommu group 32 > [256237.500837] pcieport 0000:89:02.0: Data Link Layer Link Active not set in 1000 msec > [256237.500842] pcieport 0000:89:02.0: link reset at upstream device 0000:89:02.0 failed > [256237.500865] pcieport 0000:89:02.0: AER: Device recovery failed > > The problematic case arose when 2 corrected errors arrived in a sequence like this: > > 1. Correctable error triggered, bit 0 (ERR_COR) set in Root Error Status, > which now has value 0x1. > 2. aer_irq() triggered, reads Root Error Status, finds value 0x1. > 3. Second correctable error triggered, bit 1 (multiple ERR_COR) set in Root > Error Status, which now has value 0x3. > 4. aer_irq() writes back 0x1 to Root Error Status, which now has value 0x2. > 5. aer_irq() triggered again due to the second error, but, finding value 0x2 > in Root Error Status, takes no action. Future interrupts are now inhibited. Thanks for the additional details! After this patch, I guess aer_irq() still reads 0x2 (PCI_ERR_ROOT_MULTI_COR_RCV), but now it writes 0x2 back which clears PCI_ERR_ROOT_MULTI_COR_RCV. In addition, aer_irq() will continue on to read PCI_ERR_ROOT_ERR_SRC, which probably contains either 0 or junk left over from being captured when PCI_ERR_ROOT_COR_RCV was set. And aer_irq() will queue an e_src record with status == PCI_ERR_ROOT_MULTI_COR_RCV. But since PCI_ERR_ROOT_COR_RCV is not set in status, aer_isr_one_error() will do nothing, right? That might not be *terrible* and is definitely better than not being able to handle future interrupts. But we basically threw away the information that multiple errors occurred, and we queued an e_src record that occupies space without being used for anything. Bjorn
