linux-kernel - Re: [RESEND PATCH v1] PCI: pcie_bus

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20200910022555.GA749829@bjorn-Precision-5520>
Date:   Wed, 9 Sep 2020 21:25:55 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Jim Quinlan <james.quinlan@...adcom.com>
Cc:     linux-pci@...r.kernel.org, bcm-kernel-feedback-list@...adcom.com,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [RESEND PATCH v1] PCI: pcie_bus_config can be set at build time

On Tue, Sep 08, 2020 at 12:32:48PM -0400, Jim Quinlan wrote:
> The Kconfig is modified so that the pcie_bus_config setting can be done at
> build time in the same manner as the CONFIG_PCIEASPM_XXXX choice.  The
> pci_bus_config setting may still be overridden by the bootline param.

I guess...  I really hate these build-time config settings for both
ASPM and MPS/MRRS.  But Linux just isn't smart or flexible enough to
do the right thing at run-time, so I guess we're kind of stuck.

I guess you have systems where you need a different default?

It'd be nice if we could put a little more detail in the Kconfig to
help users choose the correct one.  "Ensure MPS matches upstream
bridge" is *accurate*, but it doesn't really tell me why I would
choose this rather than a different one.

Maybe we could mention the corresponding command-line parameters,
e.g., "This is the same as booting with 'pci=pcie_bus_tune_off'"?

> Signed-off-by: Jim Quinlan <james.quinlan@...adcom.com>
> ---
>  drivers/pci/Kconfig | 40 ++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/pci.c   | 12 ++++++++++++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 4bef5c2bae9f..efe69b0d9f7f 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -187,6 +187,46 @@ config PCI_HYPERV
>  	  The PCI device frontend driver allows the kernel to import arbitrary
>  	  PCI devices from a PCI backend to support PCI driver domains.
>  
> +choice
> +	prompt "PCIE default bus config setting"
> +	default PCIE_BUS_DEFAULT
> +	depends on PCI
> +	help
> +	  One of the following choices will set the pci_bus_config at
> +	  compile time.  This will still be overridden by the appropriate
> +	  pci bootline parameter.
> +
> +config PCIE_BUS_TUNE_OFF
> +	bool "Tune Off"
> +	depends on PCI
> +	help
> +	  Use the BIOS defaults; doesn't touch MPS at all.
> +
> +config PCIE_BUS_DEFAULT
> +	bool "Default"
> +	depends on PCI
> +	help
> +	  Ensure MPS matches upstream bridge.
> +
> +config PCIE_BUS_SAFE
> +	bool "Safe"
> +	depends on PCI
> +	help
> +	  Use largest MPS boot-time devices support.
> +
> +config PCIE_BUS_PERFORMANCE
> +	bool "Performance"
> +	depends on PCI
> +	help
> +	  Use MPS and MRRS for best performance.
> +
> +config PCIE_BUS_PEER2PEER
> +	bool "Peer2peer"
> +	depends on PCI
> +	help
> +	  Set MPS = 128 for all devices.
> +endchoice
> +
>  source "drivers/pci/hotplug/Kconfig"
>  source "drivers/pci/controller/Kconfig"
>  source "drivers/pci/endpoint/Kconfig"
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index e39c5499770f..dfb52ed4a931 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -101,7 +101,19 @@ unsigned long pci_hotplug_mmio_pref_size = DEFAULT_HOTPLUG_MMIO_PREF_SIZE;
>  #define DEFAULT_HOTPLUG_BUS_SIZE	1
>  unsigned long pci_hotplug_bus_size = DEFAULT_HOTPLUG_BUS_SIZE;
>  
> +
> +/* PCIE bus config, can be overridden by bootline param */
> +#ifdef CONFIG_PCIE_BUS_TUNE_OFF
> +enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_TUNE_OFF;
> +#elif defined CONFIG_PCIE_BUS_SAFE
> +enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_SAFE;
> +#elif defined CONFIG_PCIE_BUS_PERFORMANCE
> +enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_PERFORMANCE;
> +#elif defined CONFIG_PCIE_BUS_PEER2PEER
> +enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_PEER2PEER;
> +#else
>  enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_DEFAULT;
> +#endif
>  
>  /*
>   * The default CLS is used if arch didn't set CLS explicitly and not
> -- 
> 2.17.1
>