From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BE5E6C433FE for ; Wed, 23 Nov 2022 21:44:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235503AbiKWVoN (ORCPT ); Wed, 23 Nov 2022 16:44:13 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55704 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235320AbiKWVoK (ORCPT ); Wed, 23 Nov 2022 16:44:10 -0500 Received: from mail-io1-xd2b.google.com (mail-io1-xd2b.google.com [IPv6:2607:f8b0:4864:20::d2b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 40FABC688A for ; Wed, 23 Nov 2022 13:44:08 -0800 (PST) Received: by mail-io1-xd2b.google.com with SMTP id p141so64941iod.6 for ; Wed, 23 Nov 2022 13:44:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=ndLvFvzrQi4KY9EBB6F51rhBs2HHxiUpuZy5HeAcnK8=; b=WB83wweHpSOKzdvrfMDqsPohu84YJwjVwYe0JjsmgMD61XXgzbZz/AJCePGfqLF7NJ UPmaGZxnTP7xcdHU5KGo1FL1j5vMVqF6ak9c+XTnP8uEWjtDodFLHTlF+IOtp/BDa42i 3/wu56dVh9GRWhP4+idyAQNXRWvRRhBH0aUlQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=ndLvFvzrQi4KY9EBB6F51rhBs2HHxiUpuZy5HeAcnK8=; b=5zKk9oSiJ+KjpiLa4xLOzbxriVOTe6HrsySDSTMKS0UacD3k+Ob9Qs6xws6iDyWiWR 4ETV95cMbqmwpg95jS0YzJB3Pd5KazX2MwRXIl5t/hdPCsRY8SlRw+5OxkTRruspSJp8 XfRbwT/N6YFTTuVPmUm5vkofAEWdxRMLeToX2Lg+uZn20j7jnoI7IyBB2cXLDWEkNr5t E9WmIOxzvAEbkiGRA7ytHff2eM1sCp/CyX+tr+1FprEaz09cfhYHJmyJn5lJAwvjzghL aCqcHDTYhY3gxU4Ke7IlyA6S8XZbXooYagnGs5oOMZdfwzQ/04TstKEu/pDCaotG2lhx cG0Q== X-Gm-Message-State: ANoB5plaJo7H1iB8nbMAbqjsSk5EHZa4stoz6s30Ry75ar+7uOclmc96 YB1JMWkgDwycCnywsLCrI1rARg== X-Google-Smtp-Source: AA0mqf7X1+Ruvcb2iSjxHFr57YzynUkGHWv69JWvWxrRk38xKZnb7qJys+EEwK7cOSnFR1NjO8SK8Q== X-Received: by 2002:a05:6638:4395:b0:36d:9ec0:14e3 with SMTP id bo21-20020a056638439500b0036d9ec014e3mr8247144jab.44.1669239847503; Wed, 23 Nov 2022 13:44:07 -0800 (PST) Received: from localhost (30.23.70.34.bc.googleusercontent.com. [34.70.23.30]) by smtp.gmail.com with UTF8SMTPSA id g2-20020a92d7c2000000b003024b62725dsm6016102ilq.22.2022.11.23.13.44.06 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 23 Nov 2022 13:44:07 -0800 (PST) Date: Wed, 23 Nov 2022 21:44:06 +0000 From: Matthias Kaehlcke To: Bjorn Helgaas Cc: Vidya Sagar , "Kenneth R. Crudup" , bhelgaas@google.com, lorenzo.pieralisi@arm.com, hkallweit1@gmail.com, wangxiongfeng2@huawei.com, mika.westerberg@linux.intel.com, kai.heng.feng@canonical.com, chris.packham@alliedtelesis.co.nz, yangyicong@hisilicon.com, treding@nvidia.com, jonathanh@nvidia.com, abhsahu@nvidia.com, sagupta@nvidia.com, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, kthota@nvidia.com, mmaddireddy@nvidia.com, sagar.tv@gmail.com, Ricky Wu , Rajat Jain , Prasad Malisetty , Victor Ding Subject: Re: [PATCH V1] PCI/ASPM: Save/restore L1SS Capability for suspend/resume Message-ID: References: <0d8cc8c0-31a1-0290-5aa5-0c7b16db1edb@nvidia.com> <20220412225047.GA627910@bhelgaas> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20220412225047.GA627910@bhelgaas> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, not sure this is the best thread to reply to, but we are also observing suspend issues with the same Kioxia NVMe on a platform with a Qualcomm sc7280 SoC. The system runs a v5.15 downstream kernel which includes most (post v5.15) ASPM patches from upstream. There are two issues with the Kioxia NVMe: 1. Power consumption is high unless a LTR_L1.2_THRESHOLD of 0ns is configured (related dubious patch: [1]) 2. The system often hangs on resume unless a longer delay is added in the suspend pass. QC engineers say that the NVMe is taking so much time to settle in L1ss. Other NVMe models don't exhibit power or suspend issues on this platform, except for one model which also needs a shorter delay during suspend, otherwise the system will hang occasionally upon resume. The second issue could possibly be 'fixed' with a quirk for the Kioxia NVMe model, though it seems the issue is not seen on all platforms, apparently the delay is not needed on Kenny's system. I'm currently a bit at a loss with the first issue. The patch mentioned above claims that the (no-)snoop latencies are involved, which may or may not be true. In this thread I saw Kenny posting 'lspci' output from his (now) working system. I noticed max (no-)snoop values of 3145728ns, which seems to be some sort of default (programmed) max. On my system these values are 0ns, which is the default value of the registers. I tried to set these to 3146us from the kernel to see if that makes a difference, but could only successfully update the max snoop latency, but not non-snoop (maybe this can be only done at early initialization time?). With just the max snoop latency set to 3146us power consumption of the NVMe remains high. The output of lspci from my system is attached. In this thread it was mentioned that possibly a BIOS update fixed the issue Kenny was seeing. What kind of values is the BIOS supposed to adjust (I'm a PCI n00b)? Any suggestions about what else to try? Thanks m. [1] https://patchwork.kernel.org/project/linux-arm-msm/patch/1663315719-21563-1-git-send-email-quic_krichai@quicinc.com/ --- 0001:00:00.0 PCI bridge: Qualcomm Device 010b (prog-if 00 [Normal decode]) Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- TAbort- Reset- FastB2B- PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- Capabilities: [40] Power Management version 3 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+) Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- Capabilities: [50] MSI: Enable+ Count=1/32 Maskable+ 64bit+ Address: 00000000fffff000 Data: 0000 Masking: fffffffe Pending: 00000000 Capabilities: [70] Express (v2) Root Port (Slot+), MSI 00 DevCap: MaxPayload 128 bytes, PhantFunc 0 ExtTag- RBE+ DevCtl: CorrErr- NonFatalErr- FatalErr- UnsupReq- RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+ MaxPayload 128 bytes, MaxReadReq 512 bytes DevSta: CorrErr- NonFatalErr- FatalErr- UnsupReq- AuxPwr+ TransPend- LnkCap: Port #0, Speed 8GT/s, Width x2, ASPM L0s L1, Exit Latency L0s <1us, L1 <64us ClockPM- Surprise- LLActRep+ BwNot- ASPMOptComp+ LnkCtl: ASPM L0s L1 Enabled; RCB 128 bytes, Disabled- CommClk+ ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt- LnkSta: Speed 8GT/s (ok), Width x2 (ok) TrErr- Train- SlotClk+ DLActive+ BWMgmt- ABWMgmt- SltCap: AttnBtn+ PwrCtrl+ MRL+ AttnInd+ PwrInd+ HotPlug+ Surprise+ Slot #0, PowerLimit 0.000W; Interlock+ NoCompl- SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet- CmdCplt- HPIrq- LinkChg- Control: AttnInd Off, PwrInd Off, Power- Interlock- SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet- Interlock- Changed: MRL- PresDet- LinkState- RootCap: CRSVisible- RootCtl: ErrCorrectable- ErrNon-Fatal- ErrFatal- PMEIntEna+ CRSVisible- RootSta: PME ReqID 0000, PMEStatus- PMEPending- DevCap2: Completion Timeout: Range ABCD, TimeoutDis+ NROPrPrP+ LTR+ 10BitTagComp- 10BitTagReq- OBFF Not Supported, ExtFmt- EETLPPrefix- EmergencyPowerReduction Not Supported, EmergencyPowerReductionInit- FRS- LN System CLS Not Supported, TPHComp+ ExtTPHComp- ARIFwd- AtomicOpsCap: Routing- 32bit- 64bit- 128bitCAS- DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis- LTR+ OBFF Disabled, ARIFwd- AtomicOpsCtl: ReqEn- EgressBlck- LnkCap2: Supported Link Speeds: 2.5-8GT/s, Crosslink- Retimer- 2Retimers- DRS- LnkCtl2: Target Link Speed: 8GT/s, EnterCompliance- SpeedDis- Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS- Compliance De-emphasis: -6dB LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete+ EqualizationPhase1+ EqualizationPhase2+ EqualizationPhase3+ LinkEqualizationRequest- Retimer- 2Retimers- CrosslinkRes: unsupported Capabilities: [100 v2] Advanced Error Reporting UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UESvrt: DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol- CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr- CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr+ AERCap: First Error Pointer: 00, ECRCGenCap+ ECRCGenEn- ECRCChkCap+ ECRCChkEn- MultHdrRecCap- MultHdrRecEn- TLPPfxPres- HdrLogCap- HeaderLog: 00000000 00000000 00000000 00000000 RootCmd: CERptEn- NFERptEn- FERptEn- RootSta: CERcvd- MultCERcvd- UERcvd- MultUERcvd- FirstFatal- NonFatalMsg- FatalMsg- IntMsg 0 ErrorSrc: ERR_COR: 0000 ERR_FATAL/NONFATAL: 0000 Capabilities: [148 v1] Secondary PCI Express LnkCtl3: LnkEquIntrruptEn- PerformEqu- LaneErrStat: 0 Capabilities: [168 v1] Transaction Processing Hints No steering table available Capabilities: [1fc v1] L1 PM Substates L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+ L1_PM_Substates+ PortCommonModeRestoreTime=70us PortTPowerOnTime=0us L1SubCtl1: PCI-PM_L1.2+ PCI-PM_L1.1- ASPM_L1.2+ ASPM_L1.1- T_CommonMode=70us LTR1.2_Threshold=86016ns L1SubCtl2: T_PwrOn=10us Kernel driver in use: pcieport 0001:01:00.0 Non-Volatile memory controller: KIOXIA Corporation Device 0001 (prog-if 02 [NVM Express]) Subsystem: KIOXIA Corporation Device 0001 Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- [+cc Ricky for rtsx_pci ASPM behavior, Rajat, Prasad for L1 SS stuff, > Victor for interest in disabling ASPM during save/restore] > > On Wed, Feb 16, 2022 at 06:41:39PM +0530, Vidya Sagar wrote: > > On 2/16/2022 11:30 AM, Kenneth R. Crudup wrote: > > > On Wed, 16 Feb 2022, Vidya Sagar wrote: > > > > > > > I see that the ASPM-L1 state of Realtek NIC which was in > > > > disabled state before hibernate got enabled after hibernate. > > > > > > That's actually my SD-Card reader; there's a good chance the BIOS > > > does "something" to it at boot time, as it's possible to boot from > > > SD-Card on my laptop. > > > > > > > This patch doesn't do anything to LnkCtl register which has > > > > control for ASPM L1 state. > > > > > > > Could you please check why ASPM L1 got enabled post hibernation? > > > > > > I wouldn't know how to do that; if you're still interested in that > > > let me know what to do to determine that. > > > I would like Bjorn to take a call on it. > > At this point, there are contradictions in observations. > > Remind me what contradictions you see? I know Kenny saw NVMe errors > on a kernel that included 4257f7e008ea ("PCI/ASPM: Save/restore L1SS > Capability for suspend/resume") in December 2020 [1], and that he did > *not* see those errors on 4257f7e008ea in February 2022 [2]. Is that > what you mean? > > > Just to summarize, > > - The root ports in your laptop don't have support for L1SS > > - With the same old code base with which the errors were observed plus my > > patch on top of it, I see that ASPM-L1 state getting enabled for one of the > > endpoints (Realtek SD-Card reader) after system comes out of hibernation > > even though ASPM-L1 was disabled before the system enter into hibernation. > > No errors are reported now. > > I assume you refer to [2], where on 4257f7e008ea ("PCI/ASPM: > Save/restore L1SS Capability for suspend/resume"), Kenny saw ASPM L1 > disabled before hibernate and enabled afterwards: > > --- pre-hibernate > +++ post-hibernate > 00:1d.7 PCI bridge [0604]: Intel [8086:34b7] > Bus: primary=00, secondary=58, subordinate=58 > LnkCtl: ASPM Disabled; RCB 64 bytes, Disabled- CommClk+ > 58:00.0 RTS525A PCI Express Card Reader [10ec:525a] > - LnkCtl: ASPM Disabled; RCB 64 bytes, Disabled- CommClk- > - ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt- > + LnkCtl: ASPM L1 Enabled; RCB 64 bytes, Disabled- CommClk- > + ExtSynch- ClockPM+ AutWidDis- BWInt- AutBWInt- > > Per PCIe r6.0, sec 7.5.3.7, "ASPM L1 must be enabled by software in > the Upstream component on a Link prior to enabling ASPM L1 in the > Downstream component on that Link," so this definitely seems broken, > but wouldn't explain the NVMe issue. > > The PCI core (pcie_config_aspm_link()) always enables L1 in the > upstream component before the downstream one, but 58:00.0 uses the > rtsx_pci driver, which does a lot of its own ASPM fiddling, so my > guess is that it's doing something wrong here. > > > - With the linux-next top of the tree plus my patch, no change in the ASPM > > states and no errors also reported. > > I don't know which report this refers to. > > > This points to BIOS being buggy (both old and new with new one being less > > problematic) > > I agree that a BIOS change between [1] and [2] seems plausible, but I > don't think we can prove that yet. I'm slightly queasy because while > Kenny may have updated his BIOS, most people will not have. > > I think we should try this patch again with some changes and maybe > some debug logging: > > - I wonder if we should integrate the LTR, L1 SS, and Link Control > ASPM restore instead of having them spread around through > pci_restore_ltr_state(), pci_restore_aspm_l1ss_state(), and > pci_restore_pcie_state(). Maybe a new pci_restore_aspm() that > would be called from pci_restore_pcie_state()? > > - For L1 PM Substates configuration, sec 5.5.4 says that both ports > must be configured while ASPM L1 is disabled, but I don't think we > currently guarantee this: we restore all the upstream component > state first, and we don't know the ASPM state of the downstream > one. Maybe we need to: > > * When restoring upstream component, > + disable its ASPM > > * When restoring downstream component, > + disable its ASPM > + restore upstream component's LTR, L1SS > + restore downstream component's LTR, L1SS > + restore upstream component's ASPM > + restore downstream component's ASPM > > This seems pretty messy, but seems like what the spec requires. > > - Add some pci_dbg() logging of all these save/restore values to > help debug any issues. > > Bjorn > > [1] https://lore.kernel.org/r/20201228040513.GA611645@bjorn-Precision-5520 > [2] https://lore.kernel.org/r/3ca14a7-b726-8430-fe61-a3ac183a1088@panix.com >