Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 27 Jan 2022 11:30:00 -0800
From:   Keith Busch <kbusch@kernel.org>
To:     "Rafael J. Wysocki" <rafael@kernel.org>
Cc:     Sagi Grimberg <sagi@grimberg.me>, Christoph Hellwig <hch@lst.de>,
        Len Brown <lenb@kernel.org>,
        Linux PM <linux-pm@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        m.heingbecker@googlemail.com,
        linux-nvme <linux-nvme@lists.infradead.org>
Subject: Re: [Bug] nvme blocks PC10 since v5.15 - bisected
Message-ID: <20220127193000.GC2266713@dhcp-10-100-145-180.wdc.com>
References: <CAJZ5v0hvvYedSn5u-i7sjpoEHU4P65t7i1b2pVn=S1q0nHWgqQ@mail.gmail.com>
 <20220121210905.GA1114868@dhcp-10-100-145-180.wdc.com>
 <CAJZ5v0gybg+Wk+008UBFnm2WqrxowOarhst9Eh+91BN823mqNA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAJZ5v0gybg+Wk+008UBFnm2WqrxowOarhst9Eh+91BN823mqNA@mail.gmail.com>
Precedence: bulk

On Thu, Jan 27, 2022 at 08:02:07PM +0100, Rafael J. Wysocki wrote:
> On Fri, Jan 21, 2022 at 10:09 PM Keith Busch <kbusch@kernel.org> wrote:
> >
> > On Fri, Jan 21, 2022 at 08:00:49PM +0100, Rafael J. Wysocki wrote:
> > > Hi Keith,
> > >
> > > It is reported that the following commit
> > >
> > > commit e5ad96f388b765fe6b52f64f37e910c0ba4f3de7
> > > Author: Keith Busch <kbusch@kernel.org>
> > > Date:   Tue Jul 27 09:40:44 2021 -0700
> > >
> > >    nvme-pci: disable hmb on idle suspend
> > >
> > >    An idle suspend may or may not disable host memory access from devices
> > >    placed in low power mode. Either way, it should always be safe to
> > >    disable the host memory buffer prior to entering the low power mode, and
> > >    this should also always be faster than a full device shutdown.
> > >
> > >    Signed-off-by: Keith Busch <kbusch@kernel.org>
> > >    Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> > >    Signed-off-by: Christoph Hellwig <hch@lst.de>
> > >
> > > is the source of a serious power regression occurring since 5.15
> > > (please see https://bugzilla.kernel.org/show_bug.cgi?id=215467).
> > >
> > > After this commit, the SoC on the affected system cannot enter
> > > C-states deeper than PC2 while suspended to idle which basically
> > > defeats the purpose of suspending.
> > >
> > > What may be happening is that nvme_disable_prepare_reset() that is not
> > > called any more in the ndev->nr_host_mem_descs case somehow causes the
> > > LTR of the device to change to "no requirement" which allows deeper
> > > C-states to be entered.
> > >
> > > Can you have a look at this, please?
> >
> > I thought platforms that wanted full device shutdown  behaviour would
> > always set acpi_storage_d3. Is that not happening here?
> 
> Evidently, it isn't.
> 
> Also that flag is about putting the device into D3, which need not be
> necessary as long as the LTR is set to "don't care".

The only NVMe spec guidance for a driver to initiate a controller
shutdown is to prepare for D3 transition. If this platform wants a full
device shutdown without D3, then I think we may need a quirk.

We did a shutdown before because we didn't know any better and it's the
safeset thing to do. That caused complaints for excessive resume
latency, so now we have a platform indicator to tell us if we should,
and we rely on that. Are you suggesting we instead consult the PCIe LTR
in addition to ACPI storage properties?