linux-kernel - RE: [PATCH v1 0/2] PCI/ASPM: Refine L1 PM Substates support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 6 Dec 2017 01:55:37 +0000
From:   "Chen, Kenji" <kenji.chen@...el.com>
To:     Bjorn Helgaas <helgaas@...nel.org>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>
CC:     Thierry Reding <treding@...dia.com>,
        Manikanta Maddireddy <mmaddireddy@...dia.com>,
        David Daney <david.daney@...ium.com>,
        "Krishna Kishore" <kthota@...dia.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Vidya Sagar <vidyas@...dia.com>,
        Julia Lawall <Julia.Lawall@...6.fr>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        Patrick Georgi <pgeorgi@...omium.org>,
        "Rajat Jain" <rajatja@...gle.com>, Yinghai Lu <yinghai@...nel.org>,
        Stefan Reinauer <stefan.reinauer@...eboot.org>,
        Duncan Laurie <dlaurie@...omium.org>
Subject: RE: [PATCH v1 0/2] PCI/ASPM: Refine L1 PM Substates support

https://pcisig.com/sites/default/files/specification_documents/ECN_L1_PM_Substates_with_CLKREQ_31_May_2013_Rev10a.pdf
Pls also check My Intel Doc# 545845 for the bits fields description.

-----Original Message-----
From: Bjorn Helgaas [mailto:helgaas@...nel.org] 
Sent: Wednesday, December 6, 2017 3:41 AM
To: linux-pci@...r.kernel.org
Cc: Chen, Kenji <kenji.chen@...el.com>; Thierry Reding <treding@...dia.com>; Manikanta Maddireddy <mmaddireddy@...dia.com>; David Daney <david.daney@...ium.com>; Krishna Kishore <kthota@...dia.com>; linux-kernel@...r.kernel.org; Vidya Sagar <vidyas@...dia.com>; Julia Lawall <Julia.Lawall@...6.fr>; linux-tegra@...r.kernel.org; Patrick Georgi <pgeorgi@...omium.org>; Rajat Jain <rajatja@...gle.com>; Yinghai Lu <yinghai@...nel.org>; Stefan Reinauer <stefan.reinauer@...eboot.org>; Duncan Laurie <dlaurie@...omium.org>
Subject: Re: [PATCH v1 0/2] PCI/ASPM: Refine L1 PM Substates support

On Sat, Dec 02, 2017 at 03:45:38PM -0600, Bjorn Helgaas wrote:
> The PCIe active link power state is L0.  ASPM defines two low-power
> states: L0s and L1.  The L1 PM Substates feature defines two 
> additional low-power states: L1.1 and L2.2.
> 
> The L1.2 state may have substantial entry/exit latency.  Downstream 
> devices can use the Latency Tolerance Reporting (LTR) feature to 
> report how much latency they can tolerate.  The L1 PM Substates 
> capability includes a programmable L1.2 threshold register.  If the 
> downstream devices can tolerate the latency indicated by the 
> threshold, L1.2 may be used.
> 
> If LTR is not enabled, the L1.2 threshold is ignored and L1.2 can be 
> used whenever it is enabled and CLKREQ# is de-asserted.  Linux 
> currently never enables LTR, but firmware may have done so.
> 
> The current L1 PM substates support in Linux sets the L1.2 threshold 
> to a fixed value (163.84us) copied from coreboot [1].  I don't know 
> where that value came from, and it is incorrect for some platforms, 
> e.g., Tegra [2].
> 
> These patches change that so we calculate the L1.2 threshold based on 
> values reported by the hardware, and we enable LTR whenever possible.
> 
> This is all just my understanding from reading the spec.  I'd be glad 
> to be corrected if I'm going wrong.
> 
> I'm particularly puzzled about the coreboot code in
> pciexp_L1_substate_commit() that sets LTR_L1.2_THRESHOLD:
> 
> +	pcie_update_cfg(root, root_cap + 0x08, ~0xe3ff0000,
> +		(1 << 21) | (1 << 23) | (1 << 30));
> 
> This is writing the L1 PM Substates Control 1 register, but the shifts 
> don't match up with any of the fields in the register, so this is an 
> awfully funny way to write the threshold.
> 
> [1] 
> https://mail.coreboot.org/pipermail/coreboot-gerrit/2015-March/021134.
> html [2] 
> https://lkml.kernel.org/r/1510492674-12786-1-git-send-email-vidyas@nvi
> dia.com
> 
> ---
> 
> Bjorn Helgaas (2):
>       PCI/ASPM: Calculate LTR_L1.2_THRESHOLD from device characteristics
>       PCI/ASPM: Enable Latency Tolerance Reporting when supported
> 
> 
>  drivers/pci/pcie/aspm.c |   71 +++++++++++++++++++++++++++++++----------------
>  drivers/pci/probe.c     |   33 ++++++++++++++++++++++
>  include/linux/pci.h     |    2 +
>  3 files changed, 82 insertions(+), 24 deletions(-)

I applied these with Vidya's Reviewed-by to pci/aspm for v4.16.

I'd still really like to hear from the coreboot folks about the rationale for the current coreboot code.  You folks are in a much better position to actually understand the hardware details.

Bjorn