linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: <Mario.Limonciello@dell.com>
To: <helgaas@kernel.org>, <rafael@kernel.org>
Cc: <rjw@rjwysocki.net>, <linux-nvme@lists.infradead.org>,
	<kbusch@kernel.org>, <kai.heng.feng@canonical.com>,
	<keith.busch@intel.com>, <hch@lst.de>, <sagi@grimberg.me>,
	<linux-pm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<rajatja@google.com>, <linux-pci@vger.kernel.org>
Subject: RE: [PATCH v2 2/2] nvme-pci: Allow PCI bus-level PM to be used if ASPM is disabled
Date: Thu, 8 Aug 2019 20:05:48 +0000	[thread overview]
Message-ID: <707af7109fee4942a4ba8eebbee6cde8@AUSX13MPC105.AMER.DELL.COM> (raw)
In-Reply-To: <20190808183954.GG151852@google.com>

> This is more meaningful to you than to most people because "applying
> the standard PCI PM" doesn't tell us what that means in terms of the
> device.  Presumably it has something to do with a D-state transition?
> I *assume* a suspend might involve the D0 -> D3hot transition you
> mention below?
> 
> > The reason for doing that was a (reportedly) widespread failure to
> > take the PCIe link down during D0 -> D3hot transitions of NVMe
> > devices,
> 
> I don't know any of the details, but "failure to take the link down
> during D0 -> D3hot transitions" is phrased as though it might be a
> hardware erratum.  If this *is* related to an NVMe erratum, that would
> explain why you only need to patch the nvme driver, and it would be
> useful to mention that in the commit log, since otherwise it sounds
> like something that might be needed in other drivers, too.

NVME is special in this case that there is other logic being put in place
to set the drive's power state explicitly.

I would mention that also this alternate flow is quicker for s0ix
resume since NVME doesn't go through shutdown routine.

Unanimously the feedback from vendors was to avoid NVME shutdown
and to instead use SetFeatures to go into deepest power state instead
over S0ix.

> 
> According to PCIe r5.0 sec 5.3.2, the only legal link states for D3hot
> are L1, L2/L3 Ready.  So if you put a device in D3hot and its link
> stays in L0, that sounds like a defect.  Is that what happens?
> 
> Obviously I'm still confused.  I think it would help if you could
> describe the problem in terms of the specific PCIe states involved
> (D0, D3hot, L0, L1, L2, L3, etc) because then the spec would help
> explain what's happening.

Before that commit, the flow for NVME s0ix was:

* Delete IO SQ/CQ
* Shutdown NVME controller
* Save PCI registers
* Go into D3hot
* Read PMCSR

A functioning drive had the link at L1.2 and NVME power state at PS4
at this point.
Resuming looked like this:

* Restore PCI registers
* Enable NVME controller
* Configure NVME controller (IO queues, features, etc).

After that commit the flow for NVME s0ix is:

* Use NVME SetFeatures to put drive into low power mode (PS3 or PS4)
* Save PCI config register
* ASPM is used to bring link into L1.2

The resume flow is:

* Restore PCI registers

"Non-functioning" drives consumed too much power from the old flow.

The root cause varied from manufacturer to manufacturer.
The two I know off hand:

One instance is that when PM status register is read after the device in L1.2
from D3 it causes link to go to L0 and then stay there.

Another instance I heard drive isn't able to service D3hot request when NVME
was already shut down.



  parent reply	other threads:[~2019-08-08 20:06 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-25  9:51 [Regression] Commit "nvme/pci: Use host managed power state for suspend" has problems Rafael J. Wysocki
2019-07-25 14:02 ` Kai-Heng Feng
2019-07-25 16:23   ` Mario.Limonciello
2019-07-25 17:03     ` Rafael J. Wysocki
2019-07-25 17:23       ` Mario.Limonciello
2019-07-25 18:20       ` Kai-Heng Feng
2019-07-25 19:09         ` Mario.Limonciello
2019-07-30 10:45       ` Rafael J. Wysocki
2019-07-30 14:41         ` Keith Busch
2019-07-30 17:14           ` Mario.Limonciello
2019-07-30 18:50             ` Kai-Heng Feng
2019-07-30 19:19               ` Keith Busch
2019-07-30 21:05                 ` Mario.Limonciello
2019-07-30 21:31                   ` Keith Busch
2019-07-31 21:25                     ` Rafael J. Wysocki
2019-07-31 22:19                       ` Keith Busch
2019-07-31 22:33                         ` Rafael J. Wysocki
2019-08-01  9:05                           ` Kai-Heng Feng
2019-08-01 17:29                             ` Rafael J. Wysocki
2019-08-01 19:05                               ` Mario.Limonciello
2019-08-01 22:26                                 ` Rafael J. Wysocki
2019-08-02 10:55                                   ` Kai-Heng Feng
2019-08-02 11:04                                     ` Rafael J. Wysocki
2019-08-05 19:13                                       ` Kai-Heng Feng
2019-08-05 21:28                                         ` Rafael J. Wysocki
2019-08-06 14:02                                           ` Mario.Limonciello
2019-08-06 15:00                                             ` Rafael J. Wysocki
2019-08-07 10:29                                               ` Rafael J. Wysocki
2019-08-01 20:22                             ` Keith Busch
2019-08-07  9:48                         ` Rafael J. Wysocki
2019-08-07 10:45                           ` Christoph Hellwig
2019-08-07 10:54                             ` Rafael J. Wysocki
2019-08-07  9:53                         ` [PATCH] nvme-pci: Do not prevent PCI bus-level PM from being used Rafael J. Wysocki
2019-08-07 10:14                           ` Rafael J. Wysocki
2019-08-07 10:43                           ` Christoph Hellwig
2019-08-07 14:37                           ` Keith Busch
2019-08-08  8:36                         ` [PATCH] nvme-pci: Allow PCI bus-level PM to be used if ASPM is disabled Rafael J. Wysocki
2019-08-08  8:48                           ` Christoph Hellwig
2019-08-08  9:06                             ` Rafael J. Wysocki
2019-08-08 10:03                         ` [PATCH v2 0/2] " Rafael J. Wysocki
2019-08-08 10:06                           ` [PATCH v2 1/2] PCI: PCIe: ASPM: Introduce pcie_aspm_enabled_mask() Rafael J. Wysocki
2019-08-08 13:15                             ` Bjorn Helgaas
2019-08-08 14:48                               ` Rafael J. Wysocki
2019-08-08 10:10                           ` [PATCH v2 2/2] nvme-pci: Allow PCI bus-level PM to be used if ASPM is disabled Rafael J. Wysocki
2019-08-08 13:43                             ` Bjorn Helgaas
2019-08-08 14:47                               ` Rafael J. Wysocki
2019-08-08 17:06                                 ` Rafael J. Wysocki
2019-08-08 18:39                                 ` Bjorn Helgaas
2019-08-08 20:01                                   ` Keith Busch
2019-08-08 20:05                                   ` Mario.Limonciello [this message]
2019-08-08 20:41                                   ` Rafael J. Wysocki
2019-08-09  4:47                                     ` Bjorn Helgaas
2019-08-09  8:04                                       ` Rafael J. Wysocki
2019-08-08 21:51                         ` [PATCH v3 0/2] " Rafael J. Wysocki
2019-08-08 21:55                           ` [PATCH v3 1/2] PCI: PCIe: ASPM: Introduce pcie_aspm_enabled() Rafael J. Wysocki
2019-08-09  4:50                             ` Bjorn Helgaas
2019-08-09  8:00                               ` Rafael J. Wysocki
2019-10-07 22:34                             ` Bjorn Helgaas
2019-10-08  9:27                               ` Rafael J. Wysocki
2019-10-08 21:16                                 ` Bjorn Helgaas
2019-10-08 22:54                                   ` Rafael J. Wysocki
2019-10-09 12:49                                     ` Bjorn Helgaas
2019-08-08 21:58                           ` [PATCH v3 2/2] nvme-pci: Allow PCI bus-level PM to be used if ASPM is disabled Rafael J. Wysocki
2019-08-08 22:13                           ` [PATCH v3 0/2] " Keith Busch
2019-08-09  8:05                             ` Rafael J. Wysocki
2019-08-09 14:52                               ` Keith Busch
2019-07-25 16:59   ` [Regression] Commit "nvme/pci: Use host managed power state for suspend" has problems Rafael J. Wysocki
2019-07-25 14:52 ` Keith Busch
2019-07-25 19:48   ` Rafael J. Wysocki
2019-07-25 19:52     ` Keith Busch
2019-07-25 20:02       ` Rafael J. Wysocki
2019-07-26 14:02         ` Kai-Heng Feng
2019-07-27 12:55           ` Rafael J. Wysocki
2019-07-29 15:51             ` Mario.Limonciello
2019-07-29 22:05               ` Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=707af7109fee4942a4ba8eebbee6cde8@AUSX13MPC105.AMER.DELL.COM \
    --to=mario.limonciello@dell.com \
    --cc=hch@lst.de \
    --cc=helgaas@kernel.org \
    --cc=kai.heng.feng@canonical.com \
    --cc=kbusch@kernel.org \
    --cc=keith.busch@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=rajatja@google.com \
    --cc=rjw@rjwysocki.net \
    --cc=sagi@grimberg.me \
    --subject='RE: [PATCH v2 2/2] nvme-pci: Allow PCI bus-level PM to be used if ASPM is disabled' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).