linux-kernel - Re: [PATCH v7 4/5] PCI/ASPM: Add sysfs attributes for controlling ASPM link states

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-next>] [day] [month] [year] [list]

Message-ID: <20191121204924.GA81030@google.com>
Date:   Thu, 21 Nov 2019 14:49:24 -0600
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Heiner Kallweit <hkallweit1@...il.com>
Cc:     Frederick Lawler <fred@...dlawl.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Rajat Jain <rajatja@...gle.com>, linux-pci@...r.kernel.org,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Wong Vee Khee <vee.khee.wong@...com>,
        Hui Chun Ong <hui.chun.ong@...com>,
        Keith Busch <keith.busch@...el.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 4/5] PCI/ASPM: Add sysfs attributes for controlling
 ASPM link states

[+cc Rafael, Mika, Wong, Hui, Rajat, Keith, LKML, original patch at [5]]

On Sat, Oct 05, 2019 at 02:07:56PM +0200, Heiner Kallweit wrote:

> +What:		/sys/bus/pci/devices/.../link_pm/clkpm
> +		/sys/bus/pci/devices/.../link_pm/l0s_aspm
> +		/sys/bus/pci/devices/.../link_pm/l1_aspm
> +		/sys/bus/pci/devices/.../link_pm/l1_1_aspm
> +		/sys/bus/pci/devices/.../link_pm/l1_2_aspm
> +		/sys/bus/pci/devices/.../link_pm/l1_1_pcipm
> +		/sys/bus/pci/devices/.../link_pm/l1_2_pcipm
> +Date:		October 2019
> +Contact:	Heiner Kallweit <hkallweit1@...il.com>
> +Description:	If ASPM is supported for an endpoint, then these files
> +		can be used to disable or enable the individual
> +		power management states. Write y/1/on to enable,
> +		n/0/off to disable.

This is queued up for the v5.5 merge window, so if we want to tweak
anything (path names or otherwise), now is the time.

I think I might be inclined to change the directory from "link_pm" to
"link", e.g.,

  - /sys/bus/pci/devices/0000:00:1c.0/link_pm/clkpm
  + /sys/bus/pci/devices/0000:00:1c.0/link/clkpm

because there are other things that haven't been merged yet that could
go in link/ as well:

  * Mika's "link disable" control [1]
  * Dilip's link width/speed controls [2,3]

The max_link_speed, max_link_width, current_link_speed,
current_link_width files could also logically be in link/, although
they've already been merged at the top level.

Rajat's AER statistics change [4] is also coming.  Those stats aren't
link-related, so they wouldn't go in link/.  The current strawman is
an "aer_stats" directory, but I wonder if we should make a more
generic directory like "errors" that could be used for both AER and
DPC and potentially other error-related things.

For example, we could have these link-related things:

  /sys/.../0000:00:1c.0/link/clkpm            # RW ASPM stuff
  /sys/.../0000:00:1c.0/link/l0s_aspm
  /sys/.../0000:00:1c.0/link/...
  /sys/.../0000:00:1c.0/link/disable          # RW Mika
  /sys/.../0000:00:1c.0/link/speed            # RW Dilip's control
  /sys/.../0000:00:1c.0/link/width            # RW Dilip's control
  /sys/.../0000:00:1c.0/link/max_speed        # RO possible rework
  /sys/.../0000:00:1c.0/link/max_width        # RO possible rework

With these backwards compatibility symlinks:

  /sys/.../0000:00:1c.0/max_link_speed     -> link/max_speed
  /sys/.../0000:00:1c.0/current_link_speed -> link/speed

Rajat's current patch puts the AER stats here at the top level:

  /sys/.../0000:00:1c.0/aer_stats/fatal_bit4_DLP

But maybe we could push them down like this:

  /sys/.../0000:00:1c.0/errors/aer/stats/unc_04_dlp
  /sys/.../0000:00:1c.0/errors/aer/stats/unc_26_poison_tlb_blocked
  /sys/.../0000:00:1c.0/errors/aer/stats/cor_00_rx_err
  /sys/.../0000:00:1c.0/errors/aer/stats/cor_15_hdr_log_overflow

There are some AER-related things we don't have at all today that
could go here:

  /sys/.../0000:00:1c.0/errors/aer/ecrc_gen
  /sys/.../0000:00:1c.0/errors/aer/ecrc_check
  /sys/.../0000:00:1c.0/errors/aer/unc_err_status
  /sys/.../0000:00:1c.0/errors/aer/unc_err_mask
  /sys/.../0000:00:1c.0/errors/aer/unc_err_sev

And we might someday want DPC knobs like this:

  /sys/.../0000:00:1c.0/errors/dpc/status
  /sys/.../0000:00:1c.0/errors/dpc/error_source

Any thoughts?

Bjorn

[1] https://lore.kernel.org/r/20190529104942.74991-1-mika.westerberg@linux.intel.com
[2] https://lore.kernel.org/r/d8574605f8e70f41ce1e88ccfb56b63c8f85e4df.1571638827.git.eswara.kota@linux.intel.com
[3] https://lore.kernel.org/r/20191030221436.GA261632@google.com/
[4] https://lore.kernel.org/r/20190827222145.32642-2-rajatja@google.com
[5] https://lore.kernel.org/r/b1c83f8a-9bf6-eac5-82d0-cf5b90128fbf@gmail.com