diff mbox series

[v2,RESEND,1/1] PCI: pcie_bus_config can be set at build time

Message ID 20200928194651.5393-2-james.quinlan@broadcom.com
State New
Headers show
Series PCI: pcie_bus_config can be set at build time | expand

Commit Message

Jim Quinlan Sept. 28, 2020, 7:46 p.m. UTC
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.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 drivers/pci/Kconfig | 56 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.c   | 12 ++++++++++
 2 files changed, 68 insertions(+)

Comments

Bjorn Helgaas Sept. 29, 2020, 9:31 p.m. UTC | #1
On Mon, Sep 28, 2020 at 03:46:51PM -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.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>

Applied to pci/enumeration for v5.10, thanks!

I tweaked the help texts and made the Kconfig stuff depend on
CONFIG_EXPERT.  Will that still be enough for what you need?  I'm
worried that users will get themselves in trouble if they fiddle with
things without really understanding what's going on.

Here's what I applied:


commit 2a87f534d198 ("PCI: Add Kconfig options for pcie_bus_config")
Author: Jim Quinlan <james.quinlan@broadcom.com>
Date:   Mon Sep 28 15:46:51 2020 -0400

    PCI: Add Kconfig options for pcie_bus_config
    
    Add Kconfig options for changing the default pcie_bus_config in the same
    manner as the CONFIG_PCIEASPM_XXXX choice.  The pci_bus_config setting may
    still be overridden by kernel command-line parameters, e.g.,
    "pci=pcie_bus_tune_off".
    
    [bhelgaas: depend on EXPERT, tweak help texts]
    Link: https://lore.kernel.org/r/20200928194651.5393-2-james.quinlan@broadcom.com
    Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 4bef5c2bae9f..d323b25ae27e 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -187,6 +187,68 @@ 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 "PCI Express hierarchy optimization setting"
+	default PCIE_BUS_DEFAULT
+	depends on PCI && EXPERT
+	help
+	  MPS (Max Payload Size) and MRRS (Max Read Request Size) are PCIe
+	  device parameters that affect performance and the ability to
+	  support hotplug and peer-to-peer DMA.
+
+	  The following choices set the MPS and MRRS optimization strategy
+	  at compile-time.  The choices are the same as those offered for
+	  the kernel command-line parameter 'pci', i.e.,
+	  'pci=pcie_bus_tune_off', 'pci=pcie_bus_safe',
+	  'pci=pcie_bus_perf', and 'pci=pcie_bus_peer2peer'.
+
+	  This is a compile-time setting and can be overridden by the above
+	  command-line parameters.  If unsure, choose PCIE_BUS_DEFAULT.
+
+config PCIE_BUS_TUNE_OFF
+	bool "Tune Off"
+	depends on PCI
+	help
+	  Use the BIOS defaults; don't touch MPS at all.  This is the same
+	  as booting with 'pci=pcie_bus_tune_off'.
+
+config PCIE_BUS_DEFAULT
+	bool "Default"
+	depends on PCI
+	help
+	  Default choice; ensure that the MPS matches upstream bridge.
+
+config PCIE_BUS_SAFE
+	bool "Safe"
+	depends on PCI
+	help
+	  Use largest MPS that boot-time devices support.  If you have a
+	  closed system with no possibility of adding new devices, this
+	  will use the largest MPS that's supported by all devices.  This
+	  is the same as booting with 'pci=pcie_bus_safe'.
+
+config PCIE_BUS_PERFORMANCE
+	bool "Performance"
+	depends on PCI
+	help
+	  Use MPS and MRRS for best performance.  Ensure that a given
+	  device's MPS is no larger than its parent MPS, which allows us to
+	  keep all switches/bridges to the max MPS supported by their
+	  parent.  This is the same as booting with 'pci=pcie_bus_perf'.
+
+config PCIE_BUS_PEER2PEER
+	bool "Peer2peer"
+	depends on PCI
+	help
+	  Set MPS = 128 for all devices.  MPS configuration effected by the
+	  other options could cause the MPS on one root port to be
+	  different than that of the MPS on another, which may cause
+	  hot-added devices or peer-to-peer DMA to fail.  Set MPS to the
+	  smallest possible value (128B) system-wide to avoid these issues.
+	  This is the same as booting with 'pci=pcie_bus_peer2peer'.
+
+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 a458c46d7e39..49b66ba7c874 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
Jim Quinlan Sept. 30, 2020, 8:57 p.m. UTC | #2
On Tue, Sep 29, 2020 at 5:31 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Mon, Sep 28, 2020 at 03:46:51PM -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.
> >
> > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
>
> Applied to pci/enumeration for v5.10, thanks!
>
> I tweaked the help texts and made the Kconfig stuff depend on
> CONFIG_EXPERT.  Will that still be enough for what you need?  I'm
> worried that users will get themselves in trouble if they fiddle with
> things without really understanding what's going on.

Hi Bjorn,

I didn't even know about CONFIG_EXPERT -- makes sense and looks good to me!

Thanks much,
Jim Quinlan
Broadcom STB

>
> Here's what I applied:
>
>
> commit 2a87f534d198 ("PCI: Add Kconfig options for pcie_bus_config")
> Author: Jim Quinlan <james.quinlan@broadcom.com>
> Date:   Mon Sep 28 15:46:51 2020 -0400
>
>     PCI: Add Kconfig options for pcie_bus_config
>
>     Add Kconfig options for changing the default pcie_bus_config in the same
>     manner as the CONFIG_PCIEASPM_XXXX choice.  The pci_bus_config setting may
>     still be overridden by kernel command-line parameters, e.g.,
>     "pci=pcie_bus_tune_off".
>
>     [bhelgaas: depend on EXPERT, tweak help texts]
>     Link: https://lore.kernel.org/r/20200928194651.5393-2-james.quinlan@broadcom.com
>     Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 4bef5c2bae9f..d323b25ae27e 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -187,6 +187,68 @@ 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 "PCI Express hierarchy optimization setting"
> +       default PCIE_BUS_DEFAULT
> +       depends on PCI && EXPERT
> +       help
> +         MPS (Max Payload Size) and MRRS (Max Read Request Size) are PCIe
> +         device parameters that affect performance and the ability to
> +         support hotplug and peer-to-peer DMA.
> +
> +         The following choices set the MPS and MRRS optimization strategy
> +         at compile-time.  The choices are the same as those offered for
> +         the kernel command-line parameter 'pci', i.e.,
> +         'pci=pcie_bus_tune_off', 'pci=pcie_bus_safe',
> +         'pci=pcie_bus_perf', and 'pci=pcie_bus_peer2peer'.
> +
> +         This is a compile-time setting and can be overridden by the above
> +         command-line parameters.  If unsure, choose PCIE_BUS_DEFAULT.
> +
> +config PCIE_BUS_TUNE_OFF
> +       bool "Tune Off"
> +       depends on PCI
> +       help
> +         Use the BIOS defaults; don't touch MPS at all.  This is the same
> +         as booting with 'pci=pcie_bus_tune_off'.
> +
> +config PCIE_BUS_DEFAULT
> +       bool "Default"
> +       depends on PCI
> +       help
> +         Default choice; ensure that the MPS matches upstream bridge.
> +
> +config PCIE_BUS_SAFE
> +       bool "Safe"
> +       depends on PCI
> +       help
> +         Use largest MPS that boot-time devices support.  If you have a
> +         closed system with no possibility of adding new devices, this
> +         will use the largest MPS that's supported by all devices.  This
> +         is the same as booting with 'pci=pcie_bus_safe'.
> +
> +config PCIE_BUS_PERFORMANCE
> +       bool "Performance"
> +       depends on PCI
> +       help
> +         Use MPS and MRRS for best performance.  Ensure that a given
> +         device's MPS is no larger than its parent MPS, which allows us to
> +         keep all switches/bridges to the max MPS supported by their
> +         parent.  This is the same as booting with 'pci=pcie_bus_perf'.
> +
> +config PCIE_BUS_PEER2PEER
> +       bool "Peer2peer"
> +       depends on PCI
> +       help
> +         Set MPS = 128 for all devices.  MPS configuration effected by the
> +         other options could cause the MPS on one root port to be
> +         different than that of the MPS on another, which may cause
> +         hot-added devices or peer-to-peer DMA to fail.  Set MPS to the
> +         smallest possible value (128B) system-wide to avoid these issues.
> +         This is the same as booting with 'pci=pcie_bus_peer2peer'.
> +
> +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 a458c46d7e39..49b66ba7c874 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
diff mbox series

Patch

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 4bef5c2bae9f..15ce948858fb 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -187,6 +187,62 @@  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.  The choices offered are the same as those offered
+	  for the bootline parameter 'pci'; i.e. 'pci=pcie_bus_tune_off',
+	  'pci=pcie_bus_safe', 'pci=pcie_bus_perf', and 'pci=pcie_bus_peer2peer'.
+	  This is a compile-time setting and is still be overridden by the
+	  above bootline parameters, if present.  If unsure, chose PCIE_BUS_DEFAULT.
+
+config PCIE_BUS_TUNE_OFF
+	bool "Tune Off"
+	depends on PCI
+	help
+	  Use the BIOS defaults; doesn't touch MPS at all.  This is the same
+	  as booting with 'pci=pcie_bus_tune_off'.
+
+config PCIE_BUS_DEFAULT
+	bool "Default"
+	depends on PCI
+	help
+	  Default choice; ensures that the MPS matches upstream bridge.
+
+config PCIE_BUS_SAFE
+	bool "Safe"
+	depends on PCI
+	help
+	  Use largest MPS that boot-time devices support.  If you have a
+	  closed system with no possibility of adding new devices,
+	  this will use the largest MPS that's supported by all devices.
+	  This is the same as booting with 'pci=pcie_bus_safe'.
+
+config PCIE_BUS_PERFORMANCE
+	bool "Performance"
+	depends on PCI
+	help
+	  Use MPS and MRRS for best performance.  This setting ensures
+	  that a given device's MPS is no larger than its parent MPS,
+	  which allows us to keep all switches/bridges to the max MPS supported
+	  by their parent and eventually the PHB.  This is the same as
+	  booting with 'pci=pcie_bus_perf'.
+
+config PCIE_BUS_PEER2PEER
+	bool "Peer2peer"
+	depends on PCI
+	help
+	  Set MPS = 128 for all devices.  MPS configuration effected by
+	  the other options could cause the MPS on one root port to be
+	  different than that of the MPS on another.  Simply making the system
+	  wide MPS be set to the smallest possible value (128B) solves
+	  this issue.  This is the same as booting with 'pci=pcie_bus_peer2peer'.
+
+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