| Message ID | 20220126071853.1940111-1-kai.heng.feng@canonical.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [1/2] PCI/AER: Disable AER when link is in L2/L3 ready, L2 and L3 state | expand |
Hi, On Wed, Jan 26, 2022 at 03:18:51PM +0800, Kai-Heng Feng wrote: > Commit 50310600ebda ("iommu/vt-d: Enable PCI ACS for platform opt in > hint") enables ACS, and some platforms lose its NVMe after resume from > S3: > [ 50.947816] pcieport 0000:00:1b.0: DPC: containment event, status:0x1f01 source:0x0000 > [ 50.947817] pcieport 0000:00:1b.0: DPC: unmasked uncorrectable error detected > [ 50.947829] pcieport 0000:00:1b.0: PCIe Bus Error: severity=Uncorrected (Non-Fatal), type=Transaction Layer, (Receiver ID) > [ 50.947830] pcieport 0000:00:1b.0: device [8086:06ac] error status/mask=00200000/00010000 > [ 50.947831] pcieport 0000:00:1b.0: [21] ACSViol (First) > [ 50.947841] pcieport 0000:00:1b.0: AER: broadcast error_detected message > [ 50.947843] nvme nvme0: frozen state error detected, reset controller > > It happens right after ACS gets enabled during resume. Is this really because of the above commit of due the fact that AER "service" never implemented the PM hooks in the first place ;-) > > There's another case, when Thunderbolt reaches D3cold: > [ 30.100211] pcieport 0000:00:1d.0: AER: Uncorrected (Non-Fatal) error received: 0000:00:1d.0 > [ 30.100251] pcieport 0000:00:1d.0: PCIe Bus Error: severity=Uncorrected (Non-Fatal), type=Transaction Layer, (Requester ID) > [ 30.100256] pcieport 0000:00:1d.0: device [8086:7ab0] error status/mask=00100000/00004000 > [ 30.100262] pcieport 0000:00:1d.0: [20] UnsupReq (First) > [ 30.100267] pcieport 0000:00:1d.0: AER: TLP Header: 34000000 08000052 00000000 00000000 > [ 30.100372] thunderbolt 0000:0a:00.0: AER: can't recover (no error_detected callback) > [ 30.100401] xhci_hcd 0000:3e:00.0: AER: can't recover (no error_detected callback) > [ 30.100427] pcieport 0000:00:1d.0: AER: device recovery failed > > Since PCIe spec "5.2 Link State Power Management" states that TLP and DLLP > transmission is disabled for a Link in L2/L3 Ready (D3hot), L2 (D3cold with aux > power) and L3 (D3cold), so disable AER to avoid the noises from turning power > rails on/off. I think more accurate here is to say when the topology behind the root port enters low power states. Reason here is that you can't really tell from the OS standpoint whether the link went into L1 or L2/3 before the ACPI power resource is turned off. > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=209149 > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=215453 > Fixes: 50310600ebda ("iommu/vt-d: Enable PCI ACS for platform opt in hint") > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> Thanks for fixing this! Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
On Wed, Jan 26, 2022 at 7:03 PM Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > > Hi, > > On Wed, Jan 26, 2022 at 03:18:51PM +0800, Kai-Heng Feng wrote: > > Commit 50310600ebda ("iommu/vt-d: Enable PCI ACS for platform opt in > > hint") enables ACS, and some platforms lose its NVMe after resume from > > S3: > > [ 50.947816] pcieport 0000:00:1b.0: DPC: containment event, status:0x1f01 source:0x0000 > > [ 50.947817] pcieport 0000:00:1b.0: DPC: unmasked uncorrectable error detected > > [ 50.947829] pcieport 0000:00:1b.0: PCIe Bus Error: severity=Uncorrected (Non-Fatal), type=Transaction Layer, (Receiver ID) > > [ 50.947830] pcieport 0000:00:1b.0: device [8086:06ac] error status/mask=00200000/00010000 > > [ 50.947831] pcieport 0000:00:1b.0: [21] ACSViol (First) > > [ 50.947841] pcieport 0000:00:1b.0: AER: broadcast error_detected message > > [ 50.947843] nvme nvme0: frozen state error detected, reset controller > > > > It happens right after ACS gets enabled during resume. > > Is this really because of the above commit of due the fact that AER > "service" never implemented the PM hooks in the first place ;-) From what I can understand, all services other than PME should be disabled during suspend. For example, should we convert commit a697f072f5da8 ("PCI: Disable PTM during suspend to save power") to PM hooks in PTM service? > > > > There's another case, when Thunderbolt reaches D3cold: > > [ 30.100211] pcieport 0000:00:1d.0: AER: Uncorrected (Non-Fatal) error received: 0000:00:1d.0 > > [ 30.100251] pcieport 0000:00:1d.0: PCIe Bus Error: severity=Uncorrected (Non-Fatal), type=Transaction Layer, (Requester ID) > > [ 30.100256] pcieport 0000:00:1d.0: device [8086:7ab0] error status/mask=00100000/00004000 > > [ 30.100262] pcieport 0000:00:1d.0: [20] UnsupReq (First) > > [ 30.100267] pcieport 0000:00:1d.0: AER: TLP Header: 34000000 08000052 00000000 00000000 > > [ 30.100372] thunderbolt 0000:0a:00.0: AER: can't recover (no error_detected callback) > > [ 30.100401] xhci_hcd 0000:3e:00.0: AER: can't recover (no error_detected callback) > > [ 30.100427] pcieport 0000:00:1d.0: AER: device recovery failed > > > > Since PCIe spec "5.2 Link State Power Management" states that TLP and DLLP > > transmission is disabled for a Link in L2/L3 Ready (D3hot), L2 (D3cold with aux > > power) and L3 (D3cold), so disable AER to avoid the noises from turning power > > rails on/off. > > I think more accurate here is to say when the topology behind the root > port enters low power states. Reason here is that you can't really tell > from the OS standpoint whether the link went into L1 or L2/3 before the > ACPI power resource is turned off. OK, let me change the commit message a bit. My intention is to state "transmission is disabled" in those Link states. Kai-Heng > > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=209149 > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=215453 > > Fixes: 50310600ebda ("iommu/vt-d: Enable PCI ACS for platform opt in hint") > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > > Thanks for fixing this! > > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Hi, On Thu, Jan 27, 2022 at 10:21:35AM +0800, Kai-Heng Feng wrote: > On Wed, Jan 26, 2022 at 7:03 PM Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: > > > > Hi, > > > > On Wed, Jan 26, 2022 at 03:18:51PM +0800, Kai-Heng Feng wrote: > > > Commit 50310600ebda ("iommu/vt-d: Enable PCI ACS for platform opt in > > > hint") enables ACS, and some platforms lose its NVMe after resume from > > > S3: > > > [ 50.947816] pcieport 0000:00:1b.0: DPC: containment event, status:0x1f01 source:0x0000 > > > [ 50.947817] pcieport 0000:00:1b.0: DPC: unmasked uncorrectable error detected > > > [ 50.947829] pcieport 0000:00:1b.0: PCIe Bus Error: severity=Uncorrected (Non-Fatal), type=Transaction Layer, (Receiver ID) > > > [ 50.947830] pcieport 0000:00:1b.0: device [8086:06ac] error status/mask=00200000/00010000 > > > [ 50.947831] pcieport 0000:00:1b.0: [21] ACSViol (First) > > > [ 50.947841] pcieport 0000:00:1b.0: AER: broadcast error_detected message > > > [ 50.947843] nvme nvme0: frozen state error detected, reset controller > > > > > > It happens right after ACS gets enabled during resume. > > > > Is this really because of the above commit of due the fact that AER > > "service" never implemented the PM hooks in the first place ;-) > > >From what I can understand, all services other than PME should be > disabled during suspend. > > For example, should we convert commit a697f072f5da8 ("PCI: Disable PTM > during suspend to save power") to PM hooks in PTM service? Yes, I think that's the right thing to do. I wonder how it was not using the PM hooks in the first place.
On Thu, Jan 27, 2022 at 08:29:22AM +0200, Mika Westerberg wrote: > > For example, should we convert commit a697f072f5da8 ("PCI: Disable PTM > > during suspend to save power") to PM hooks in PTM service? > > Yes, I think that's the right thing to do. I wonder how it was not using > the PM hooks in the first place. Actually no. The reason it is not using PM hooks is that PTM is not a port "service" so it needs to be dealt in the core.
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c index 9fa1f97e5b270..e4e9d4a3098d7 100644 --- a/drivers/pci/pcie/aer.c +++ b/drivers/pci/pcie/aer.c @@ -1367,6 +1367,22 @@ static int aer_probe(struct pcie_device *dev) return 0; } +static int aer_suspend(struct pcie_device *dev) +{ + struct aer_rpc *rpc = get_service_data(dev); + + aer_disable_rootport(rpc); + return 0; +} + +static int aer_resume(struct pcie_device *dev) +{ + struct aer_rpc *rpc = get_service_data(dev); + + aer_enable_rootport(rpc); + return 0; +} + /** * aer_root_reset - reset Root Port hierarchy, RCEC, or RCiEP * @dev: pointer to Root Port, RCEC, or RCiEP @@ -1433,12 +1449,15 @@ static pci_ers_result_t aer_root_reset(struct pci_dev *dev) } static struct pcie_port_service_driver aerdriver = { - .name = "aer", - .port_type = PCIE_ANY_PORT, - .service = PCIE_PORT_SERVICE_AER, - - .probe = aer_probe, - .remove = aer_remove, + .name = "aer", + .port_type = PCIE_ANY_PORT, + .service = PCIE_PORT_SERVICE_AER, + .probe = aer_probe, + .suspend = aer_suspend, + .resume = aer_resume, + .runtime_suspend = aer_suspend, + .runtime_resume = aer_resume, + .remove = aer_remove, }; /**
Commit 50310600ebda ("iommu/vt-d: Enable PCI ACS for platform opt in hint") enables ACS, and some platforms lose its NVMe after resume from S3: [ 50.947816] pcieport 0000:00:1b.0: DPC: containment event, status:0x1f01 source:0x0000 [ 50.947817] pcieport 0000:00:1b.0: DPC: unmasked uncorrectable error detected [ 50.947829] pcieport 0000:00:1b.0: PCIe Bus Error: severity=Uncorrected (Non-Fatal), type=Transaction Layer, (Receiver ID) [ 50.947830] pcieport 0000:00:1b.0: device [8086:06ac] error status/mask=00200000/00010000 [ 50.947831] pcieport 0000:00:1b.0: [21] ACSViol (First) [ 50.947841] pcieport 0000:00:1b.0: AER: broadcast error_detected message [ 50.947843] nvme nvme0: frozen state error detected, reset controller It happens right after ACS gets enabled during resume. There's another case, when Thunderbolt reaches D3cold: [ 30.100211] pcieport 0000:00:1d.0: AER: Uncorrected (Non-Fatal) error received: 0000:00:1d.0 [ 30.100251] pcieport 0000:00:1d.0: PCIe Bus Error: severity=Uncorrected (Non-Fatal), type=Transaction Layer, (Requester ID) [ 30.100256] pcieport 0000:00:1d.0: device [8086:7ab0] error status/mask=00100000/00004000 [ 30.100262] pcieport 0000:00:1d.0: [20] UnsupReq (First) [ 30.100267] pcieport 0000:00:1d.0: AER: TLP Header: 34000000 08000052 00000000 00000000 [ 30.100372] thunderbolt 0000:0a:00.0: AER: can't recover (no error_detected callback) [ 30.100401] xhci_hcd 0000:3e:00.0: AER: can't recover (no error_detected callback) [ 30.100427] pcieport 0000:00:1d.0: AER: device recovery failed Since PCIe spec "5.2 Link State Power Management" states that TLP and DLLP transmission is disabled for a Link in L2/L3 Ready (D3hot), L2 (D3cold with aux power) and L3 (D3cold), so disable AER to avoid the noises from turning power rails on/off. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=209149 Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=215453 Fixes: 50310600ebda ("iommu/vt-d: Enable PCI ACS for platform opt in hint") Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> --- drivers/pci/pcie/aer.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-)