| Message ID | 20221011034139.731146-1-kai.heng.feng@canonical.com |
|---|---|
| Headers | show |
| Series | Disable PTM when device is suspended for all PCIe devices | expand |
On 11.10.22 05:41, Kai-Heng Feng wrote: > BugLink: https://bugs.launchpad.net/bugs/1988797 > > [Impact] > Unsupporeted PTM requests can flood kernel dmesg. It can also prevent > the system from suspending because the AER IRQ can never be quiesced. > > [Fix] > Disable PTM when PCIe device is suspended. > > [Test] > No more AER in dmesg, and affected system can suspend correctly. > > [Where problems could occur] > According to PCIe spec, disabling PTM is suggested, but it's possible > that there are devices don't like PTM to be disabled during suspend. What would be the visible effect if this happens? Errors on such devices or potentially they remain suspended? This is a somewhat substantial change and with it just reaching linux-next I would suspect there is not much experience with potential side effects. So it would be good to know what to watch out for. Acked-by: Stefan Bader <stefan.bader@canonical.com> > > Bjorn Helgaas (9): > PCI/PTM: Cache PTM Capability offset > PCI/PTM: Add pci_upstream_ptm() helper > PCI/PTM: Separate configuration and enable > PCI/PTM: Add pci_suspend_ptm() and pci_resume_ptm() > PCI/PTM: Move pci_ptm_info() body into its only caller > PCI/PTM: Preserve RsvdP bits in PTM Control register > PCI/PTM: Reorder functions in logical order > PCI/PTM: Consolidate PTM interface declarations > PCI/PM: Always disable PTM for all devices during suspend > > Rajvi Jingar (1): > PCI/PM: Simplify pci_pm_suspend_noirq() > > drivers/pci/pci-driver.c | 30 ++-- > drivers/pci/pci.c | 28 +--- > drivers/pci/pci.h | 14 +- > drivers/pci/pcie/ptm.c | 300 ++++++++++++++++++++++----------------- > include/linux/pci.h | 3 + > 5 files changed, 199 insertions(+), 176 deletions(-) >
On Tue, Oct 11, 2022 at 3:56 PM Stefan Bader <stefan.bader@canonical.com> wrote: > > On 11.10.22 05:41, Kai-Heng Feng wrote: > > BugLink: https://bugs.launchpad.net/bugs/1988797 > > > > [Impact] > > Unsupporeted PTM requests can flood kernel dmesg. It can also prevent > > the system from suspending because the AER IRQ can never be quiesced. > > > > [Fix] > > Disable PTM when PCIe device is suspended. > > > > [Test] > > No more AER in dmesg, and affected system can suspend correctly. > > > > [Where problems could occur] > > According to PCIe spec, disabling PTM is suggested, but it's possible > > that there are devices don't like PTM to be disabled during suspend. > > What would be the visible effect if this happens? Errors on such devices or > potentially they remain suspended? This is a somewhat substantial change and > with it just reaching linux-next I would suspect there is not much experience > with potential side effects. So it would be good to know what to watch out for. The change appears to be large, but the functional change is actually quite simple - to disable PTM on both ports and endpoints, for both system-wide suspend and runtime suspend. In reality this won't affect too many PCIe devices, AFAIK devices that support PTM are quite limited. I really can't say what's the potential visible effect, as the PCIe spec is very arcane and a single stuff can be scattered to different hundred pages. Kai-Heng > > Acked-by: Stefan Bader <stefan.bader@canonical.com> > > > > Bjorn Helgaas (9): > > PCI/PTM: Cache PTM Capability offset > > PCI/PTM: Add pci_upstream_ptm() helper > > PCI/PTM: Separate configuration and enable > > PCI/PTM: Add pci_suspend_ptm() and pci_resume_ptm() > > PCI/PTM: Move pci_ptm_info() body into its only caller > > PCI/PTM: Preserve RsvdP bits in PTM Control register > > PCI/PTM: Reorder functions in logical order > > PCI/PTM: Consolidate PTM interface declarations > > PCI/PM: Always disable PTM for all devices during suspend > > > > Rajvi Jingar (1): > > PCI/PM: Simplify pci_pm_suspend_noirq() > > > > drivers/pci/pci-driver.c | 30 ++-- > > drivers/pci/pci.c | 28 +--- > > drivers/pci/pci.h | 14 +- > > drivers/pci/pcie/ptm.c | 300 ++++++++++++++++++++++----------------- > > include/linux/pci.h | 3 + > > 5 files changed, 199 insertions(+), 176 deletions(-) > > >
On Tue, Oct 11, 2022 at 11:41:28AM +0800, Kai-Heng Feng wrote: > BugLink: https://bugs.launchpad.net/bugs/1988797 > > [Impact] > Unsupporeted PTM requests can flood kernel dmesg. It can also prevent > the system from suspending because the AER IRQ can never be quiesced. > > [Fix] > Disable PTM when PCIe device is suspended. > > [Test] > No more AER in dmesg, and affected system can suspend correctly. > > [Where problems could occur] > According to PCIe spec, disabling PTM is suggested, but it's possible > that there are devices don't like PTM to be disabled during suspend. Applied to kinetic/linux and kinetic/linux-unstable. Thanks, -Andrea
Kai-Heng Feng kirjoitti 11.10.2022 klo 6.41: > BugLink: https://bugs.launchpad.net/bugs/1988797 > > [Impact] > Unsupporeted PTM requests can flood kernel dmesg. It can also prevent > the system from suspending because the AER IRQ can never be quiesced. > > [Fix] > Disable PTM when PCIe device is suspended. > > [Test] > No more AER in dmesg, and affected system can suspend correctly. > > [Where problems could occur] > According to PCIe spec, disabling PTM is suggested, but it's possible > that there are devices don't like PTM to be disabled during suspend. > > Bjorn Helgaas (9): > PCI/PTM: Cache PTM Capability offset > PCI/PTM: Add pci_upstream_ptm() helper > PCI/PTM: Separate configuration and enable > PCI/PTM: Add pci_suspend_ptm() and pci_resume_ptm() > PCI/PTM: Move pci_ptm_info() body into its only caller > PCI/PTM: Preserve RsvdP bits in PTM Control register > PCI/PTM: Reorder functions in logical order > PCI/PTM: Consolidate PTM interface declarations > PCI/PM: Always disable PTM for all devices during suspend > > Rajvi Jingar (1): > PCI/PM: Simplify pci_pm_suspend_noirq() > > drivers/pci/pci-driver.c | 30 ++-- > drivers/pci/pci.c | 28 +--- > drivers/pci/pci.h | 14 +- > drivers/pci/pcie/ptm.c | 300 ++++++++++++++++++++++----------------- > include/linux/pci.h | 3 + > 5 files changed, 199 insertions(+), 176 deletions(-) > Acked-by: Timo Aaltonen <timo.aaltonen@canonical.com>
On 11.10.22 05:41, Kai-Heng Feng wrote: > BugLink: https://bugs.launchpad.net/bugs/1988797 > > [Impact] > Unsupporeted PTM requests can flood kernel dmesg. It can also prevent > the system from suspending because the AER IRQ can never be quiesced. > > [Fix] > Disable PTM when PCIe device is suspended. > > [Test] > No more AER in dmesg, and affected system can suspend correctly. > > [Where problems could occur] > According to PCIe spec, disabling PTM is suggested, but it's possible > that there are devices don't like PTM to be disabled during suspend. > > Bjorn Helgaas (9): > PCI/PTM: Cache PTM Capability offset > PCI/PTM: Add pci_upstream_ptm() helper > PCI/PTM: Separate configuration and enable > PCI/PTM: Add pci_suspend_ptm() and pci_resume_ptm() > PCI/PTM: Move pci_ptm_info() body into its only caller > PCI/PTM: Preserve RsvdP bits in PTM Control register > PCI/PTM: Reorder functions in logical order > PCI/PTM: Consolidate PTM interface declarations > PCI/PM: Always disable PTM for all devices during suspend > > Rajvi Jingar (1): > PCI/PM: Simplify pci_pm_suspend_noirq() > > drivers/pci/pci-driver.c | 30 ++-- > drivers/pci/pci.c | 28 +--- > drivers/pci/pci.h | 14 +- > drivers/pci/pcie/ptm.c | 300 ++++++++++++++++++++++----------------- > include/linux/pci.h | 3 + > 5 files changed, 199 insertions(+), 176 deletions(-) > Applied to jammy:linux/master-next. Thanks. -Stefan