linux-kernel - Re: [PATCH v2 1/2] PCI: Track Flit Mode Status & print it with link status

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]

Message-ID: <20250221233635.GA372807@bhelgaas>
Date: Fri, 21 Feb 2025 17:36:35 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Yazen Ghannam <yazen.ghannam@....com>
Cc: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org,
	Lukas Wunner <lukas@...ner.de>,
	Jonathan Cameron <Jonathan.Cameron@...wei.com>,
	linux-kernel@...r.kernel.org, Borislav Petkov <bp@...en8.de>,
	linux-edac@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
	Mahesh J Salgaonkar <mahesh@...ux.ibm.com>,
	Oliver O'Halloran <oohall@...il.com>,
	Tony Luck <tony.luck@...el.com>
Subject: Re: [PATCH v2 1/2] PCI: Track Flit Mode Status & print it with link
 status

On Fri, Feb 21, 2025 at 10:29:48AM -0500, Yazen Ghannam wrote:
> On Fri, Feb 07, 2025 at 06:18:35PM +0200, Ilpo Järvinen wrote:
> > PCIe r6.0 added Flit mode that mainly alters HW behavior but some OS
> > visible changes are also because of it. The OS visible changes include
> 
> The first sentence reads oddly. Maybe a slight change?
> 
> "...but there are some OS visible changes because of it."

Updated locally.

> > +	if (dev->bus && dev->bus->flit_mode)
> > +		flit_mode = ", in Flit mode";
> > +
> >  	if (bw_avail >= bw_cap && verbose)
> > -		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth (%s x%d link)\n",
> > +		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth (%s x%d link)%s\n",
> >  			 bw_cap / 1000, bw_cap % 1000,
> > -			 pci_speed_string(speed_cap), width_cap);
> > +			 pci_speed_string(speed_cap), width_cap, flit_mode);
> >  	else if (bw_avail < bw_cap)
> > -		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth, limited by %s x%d link at %s (capable of %u.%03u Gb/s with %s x%d link)\n",
> > +		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth, limited by %s x%d link at %s (capable of %u.%03u Gb/s with %s x%d link)%s\n",
> >  			 bw_avail / 1000, bw_avail % 1000,
> >  			 pci_speed_string(speed), width,
> >  			 limiting_dev ? pci_name(limiting_dev) : "<unknown>",
> >  			 bw_cap / 1000, bw_cap % 1000,
> > -			 pci_speed_string(speed_cap), width_cap);
> > +			 pci_speed_string(speed_cap), width_cap, flit_mode);
> 
> Does the "Flit mode" message *need* to go into these lines? Could it be
> its own message?

I suppose it doesn't need to be there, and these bandwidth lines are
already pretty long (my fault, open to suggestions to shorten them),
but I do think it's useful to have related info all on the same line.

>  +#include <linux/string_choices.h>
> 
>  @@ -6190,21 +6190,25 @@ void __pcie_print_link_status(struct pci_dev *dev, bool verbose)
>   	enum pci_bus_speed speed, speed_cap;
>   	struct pci_dev *limiting_dev = NULL;
>   	u32 bw_avail, bw_cap;
>   
>   	bw_cap = pcie_bandwidth_capable(dev, &speed_cap, &width_cap);
>   	bw_avail = pcie_bandwidth_available(dev, &limiting_dev, &speed, &width);
>   
>  +	if (dev->bus)
>  +		pci_info(dev, "Flit mode: %s\n", str_enabled_disabled(dev->bus->flit_mode);
>  +
>   	if (bw_avail >= bw_cap && verbose)
>  		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth (%s x%d link)\n",
>   			 bw_cap / 1000, bw_cap % 1000,
>  			 pci_speed_string(speed_cap), width_cap);
>   	else if (bw_avail < bw_cap)
>  		pci_info(dev, "%u.%03u Gb/s available PCIe bandwidth, limited by %s x%d link at %s (capable of %u.%03u Gb/s with %s x%d link)\n",
>   			 bw_avail / 1000, bw_avail % 1000,
>   			 pci_speed_string(speed), width,
>   			 limiting_dev ? pci_name(limiting_dev) : "<unknown>",
>   			 bw_cap / 1000, bw_cap % 1000,
>  			 pci_speed_string(speed_cap), width_cap);
> 
> >  }