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

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

[Jim Quinlan]

[-- Attachment #1: Type: text/plain, Size: 305 bytes --]

v2: Add more description text in the new Kconfig settings (Bjorn).

v1: Original

Jim Quinlan (1):
  PCI: pcie_bus_config can be set at build time

 drivers/pci/Kconfig | 56 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.c   | 12 ++++++++++
 2 files changed, 68 insertions(+)

-- 
2.17.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4167 bytes --]

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

[Jim Quinlan]

[-- Attachment #1: Type: text/plain, Size: 3822 bytes --]

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(+)

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
-- 
2.17.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4167 bytes --]

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

[Bjorn Helgaas]

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

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

[Jim Quinlan]

[-- Attachment #1: Type: text/plain, Size: 5721 bytes --]

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

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4167 bytes --]

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

[Bjorn Helgaas]

[+cc Hans, Niklas]

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>

We merged this as b0e85c3c8554 ("PCI: Add Kconfig options for MPS/MRRS
strategy"), which appeared in v5.10.

In retrospect, I think this might have been a mistake because it
forces a build-time configuration for something that may not be known
at build time and can be set via command-line parameter.

But I can't find any discussion about it.  Did you have a use case
where command line parameters weren't usable?

If there's a platform that requires one of these settings, maybe
there's a way to do that programmatically in the host bridge driver
rather than using a Kconfig symbol.

> ---
>  drivers/pci/Kconfig | 56 +++++++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/pci.c   | 12 ++++++++++
>  2 files changed, 68 insertions(+)
> 
> 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
> -- 
> 2.17.1
> 

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

[Jim Quinlan]

[-- Attachment #1: Type: text/plain, Size: 5991 bytes --]

On Fri, Feb 20, 2026 at 5:11 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> [+cc Hans, Niklas]
>
> 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>
>
> We merged this as b0e85c3c8554 ("PCI: Add Kconfig options for MPS/MRRS
> strategy"), which appeared in v5.10.
>
> In retrospect, I think this might have been a mistake because it
> forces a build-time configuration for something that may not be known
> at build time and can be set via command-line parameter.

Hi Bjorn,

I don't see much difference between setting the default at build time
versus setting it via the boot command line --  both methods are
performed before runtime and make assumptions.

>
> But I can't find any discussion about it.  Did you have a use case
> where command line parameters weren't usable?

Yes.  We have a Cable Modem (CM) customer who can update their Linux
version but cannot update their boot loader or its default bootline.
FWIW, they only want the option to set pcie_bus_config to
PCIE_BUS_SAFE via the .config.

>
> If there's a platform that requires one of these settings, maybe
> there's a way to do that programmatically in the host bridge driver
> rather than using a Kconfig symbol.

It's a CM product which uses a particular Wifi chip.  Recognizing this
scenario would have the RC trying to grab the EP's vendor-id (and
possibly more info) , and that seems awkward at best.

Regards,
Jim Quinlan
Broadcom STB/CM

>
> > ---
> >  drivers/pci/Kconfig | 56 +++++++++++++++++++++++++++++++++++++++++++++
> >  drivers/pci/pci.c   | 12 ++++++++++
> >  2 files changed, 68 insertions(+)
> >
> > 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
> > --
> > 2.17.1
> >
>
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5471 bytes --]

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

[Bjorn Helgaas]

On Tue, Feb 24, 2026 at 04:10:17PM -0500, Jim Quinlan wrote:
> On Fri, Feb 20, 2026 at 5:11 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>
> >
> > We merged this as b0e85c3c8554 ("PCI: Add Kconfig options for MPS/MRRS
> > strategy"), which appeared in v5.10.
> >
> > In retrospect, I think this might have been a mistake because it
> > forces a build-time configuration for something that may not be known
> > at build time and can be set via command-line parameter.
> 
> I don't see much difference between setting the default at build time
> versus setting it via the boot command line --  both methods are
> performed before runtime and make assumptions.

True.  I don't like baking unnecessary decisions into the kernel
binary.  This one seems useful for a point product but at the cost of
a small burden on everybody who builds a kernel.

> > But I can't find any discussion about it.  Did you have a use case
> > where command line parameters weren't usable?
> 
> Yes.  We have a Cable Modem (CM) customer who can update their Linux
> version but cannot update their boot loader or its default bootline.
> FWIW, they only want the option to set pcie_bus_config to
> PCIE_BUS_SAFE via the .config.
> 
> > If there's a platform that requires one of these settings, maybe
> > there's a way to do that programmatically in the host bridge driver
> > rather than using a Kconfig symbol.
> 
> It's a CM product which uses a particular Wifi chip.  Recognizing this
> scenario would have the RC trying to grab the EP's vendor-id (and
> possibly more info), and that seems awkward at best.

I assume a closed product like a CM doesn't run a plain vanilla
upstream kernel, so removing the Kconfig option would mean carrying a
one-line patch to set pcie_bus_config.  There's some trade-off between
carrying a source patch and tracking Kconfig requirements, but I don't
know where that would be in your case.

Could this be decided in pcie-brcmstb.c based on DT or something
similar?  I guess you said it only applies to CMs with a particular
WiFi chip, so maybe that's not visible in DT.  But I wonder why it's
specific to the WiFi -- I would think the important thing is that it's
a closed system and every closed system would want PCIE_BUS_SAFE.

> > > ---
> > >  drivers/pci/Kconfig | 56 +++++++++++++++++++++++++++++++++++++++++++++
> > >  drivers/pci/pci.c   | 12 ++++++++++
> > >  2 files changed, 68 insertions(+)
> > >
> > > 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
> > > --
> > > 2.17.1
> > >
> >
> >

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

[Niklas Cassel]

On Tue, Feb 24, 2026 at 04:10:17PM -0500, Jim Quinlan wrote:
> On Fri, Feb 20, 2026 at 5:11 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.

Hello Jim,

Could you perhaps test this series:
https://lore.kernel.org/linux-pci/20251127170908.14850-1-18255117159@163.com/


Since the series above only defaults by setting MPS to MPSS for the
Root Port (and no other devices), for kernels with PCIE_BUS_SAFE=y
pcie_bus_configure_settings() will still be called for each downstream
device, so it should not affect your CM product.

But, it would be nice if you could test the series anyway, just to
verify.


Kind regards,
Niklas

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

[Jim Quinlan]

[-- Attachment #1: Type: text/plain, Size: 8374 bytes --]

On Tue, Feb 24, 2026 at 6:05 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Tue, Feb 24, 2026 at 04:10:17PM -0500, Jim Quinlan wrote:
> > On Fri, Feb 20, 2026 at 5:11 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>
> > >
> > > We merged this as b0e85c3c8554 ("PCI: Add Kconfig options for MPS/MRRS
> > > strategy"), which appeared in v5.10.
> > >
> > > In retrospect, I think this might have been a mistake because it
> > > forces a build-time configuration for something that may not be known
> > > at build time and can be set via command-line parameter.
> >
> > I don't see much difference between setting the default at build time
> > versus setting it via the boot command line --  both methods are
> > performed before runtime and make assumptions.
>
> True.  I don't like baking unnecessary decisions into the kernel
> binary.  This one seems useful for a point product but at the cost of
> a small burden on everybody who builds a kernel.
>
> > > But I can't find any discussion about it.  Did you have a use case
> > > where command line parameters weren't usable?
> >
> > Yes.  We have a Cable Modem (CM) customer who can update their Linux
> > version but cannot update their boot loader or its default bootline.
> > FWIW, they only want the option to set pcie_bus_config to
> > PCIE_BUS_SAFE via the .config.
> >
> > > If there's a platform that requires one of these settings, maybe
> > > there's a way to do that programmatically in the host bridge driver
> > > rather than using a Kconfig symbol.
> >
> > It's a CM product which uses a particular Wifi chip.  Recognizing this
> > scenario would have the RC trying to grab the EP's vendor-id (and
> > possibly more info), and that seems awkward at best.
>
> I assume a closed product like a CM doesn't run a plain vanilla
> upstream kernel, so removing the Kconfig option would mean carrying a
> one-line patch to set pcie_bus_config.  There's some trade-off between
> carrying a source patch and tracking Kconfig requirements, but I don't
> know where that would be in your case.

Our goal is to run a plain vanilla kernel with -- if needed -- BRCM
modules, but we've got a ways to go.
>
>
> Could this be decided in pcie-brcmstb.c based on DT or something
> similar?  I guess you said it only applies to CMs with a particular
> WiFi chip, so maybe that's not visible in DT.  But I wonder why it's
> specific to the WiFi -- I would think the important thing is that it's
> a closed system and every closed system would want PCIE_BUS_SAFE.
We only needed it for a specific Wifi chip with a CM board.  All other
boards were fine with the default action.

Going forward, there are a number of ways for them to handle this if
the CONFIG_PCIE_BUS_*  choices are dropped.  One is to use the DT
bootargs with "pci=pcie_bus_safe".  Another is to use
CONFIG_CMDLINE/CMDLINE_FORCE to carry the same param.  And also what
you have suggested above.

If you are just going to modify upstream only, there should be no problem.
If you are going to call this a "Fix" and backport to all of the past
stable versions down to 5.10, there may be some havoc when this
particular product pulls in the fixes.  I've given a heads-up to my CM
colleague, but this info may fall through the cracks.

Regards,
Jim Quinlan
Broadcom CM/STB

>
>
> > > > ---
> > > >  drivers/pci/Kconfig | 56 +++++++++++++++++++++++++++++++++++++++++++++
> > > >  drivers/pci/pci.c   | 12 ++++++++++
> > > >  2 files changed, 68 insertions(+)
> > > >
> > > > 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
> > > > --
> > > > 2.17.1
> > > >
> > >
> > >
>
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5471 bytes --]

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

[Jim Quinlan]

[-- Attachment #1: Type: text/plain, Size: 1395 bytes --]

On Wed, Feb 25, 2026 at 8:23 AM Niklas Cassel <cassel@kernel.org> wrote:
>
> On Tue, Feb 24, 2026 at 04:10:17PM -0500, Jim Quinlan wrote:
> > On Fri, Feb 20, 2026 at 5:11 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.
>
> Hello Jim,
>
> Could you perhaps test this series:
> https://lore.kernel.org/linux-pci/20251127170908.14850-1-18255117159@163.com/
>
>
> Since the series above only defaults by setting MPS to MPSS for the
> Root Port (and no other devices), for kernels with PCIE_BUS_SAFE=y
> pcie_bus_configure_settings() will still be called for each downstream
> device, so it should not affect your CM product.
>
> But, it would be nice if you could test the series anyway, just to
> verify.

Hello Niklas,

Unfortunately I do not have the specific board that required us to use
the PCIE_BUS_SAFE setting, so I really cannot do a proper test of your
series.  I typically only test things on our STB boards using non-Brcm
devices that have stock upstream drivers.

Sorry & regards,
Jim Quinlan
Broadcom CM/STB

>
>
> Kind regards,
> Niklas

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5471 bytes --]

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

[Bjorn Helgaas]

On Tue, Feb 24, 2026 at 04:10:17PM -0500, Jim Quinlan wrote:
> On Fri, Feb 20, 2026 at 5:11 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>
> >
> > We merged this as b0e85c3c8554 ("PCI: Add Kconfig options for MPS/MRRS
> > strategy"), which appeared in v5.10.
> >
> > In retrospect, I think this might have been a mistake because it
> > forces a build-time configuration for something that may not be known
> > at build time and can be set via command-line parameter.
> ...

> > But I can't find any discussion about it.  Did you have a use case
> > where command line parameters weren't usable?
> 
> Yes.  We have a Cable Modem (CM) customer who can update their Linux
> version but cannot update their boot loader or its default bootline.
> FWIW, they only want the option to set pcie_bus_config to
> PCIE_BUS_SAFE via the .config.
> ... 

> It's a CM product which uses a particular Wifi chip.  Recognizing this
> scenario would have the RC trying to grab the EP's vendor-id (and
> possibly more info), and that seems awkward at best.

This is actually kind of weird.  I don't know why a particular device
would have any special MPS or MRRS requirements.

Do you have any more details about this?  Does the WiFi chip actually
not *work* with other MPS settings?  Or is this a performance thing
where other settings don't give acceptable performance?

I assume there's no hotplug in this setup, so we don't't have to worry
about adding new devices at runtime.  Do you know if there's any
P2PDMA usage?

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

[Jim Quinlan]

[-- Attachment #1: Type: text/plain, Size: 2633 bytes --]

On Mon, Mar 2, 2026 at 7:03 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Tue, Feb 24, 2026 at 04:10:17PM -0500, Jim Quinlan wrote:
> > On Fri, Feb 20, 2026 at 5:11 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>
> > >
> > > We merged this as b0e85c3c8554 ("PCI: Add Kconfig options for MPS/MRRS
> > > strategy"), which appeared in v5.10.
> > >
> > > In retrospect, I think this might have been a mistake because it
> > > forces a build-time configuration for something that may not be known
> > > at build time and can be set via command-line parameter.
> > ...
>
> > > But I can't find any discussion about it.  Did you have a use case
> > > where command line parameters weren't usable?
> >
> > Yes.  We have a Cable Modem (CM) customer who can update their Linux
> > version but cannot update their boot loader or its default bootline.
> > FWIW, they only want the option to set pcie_bus_config to
> > PCIE_BUS_SAFE via the .config.
> > ...
>
> > It's a CM product which uses a particular Wifi chip.  Recognizing this
> > scenario would have the RC trying to grab the EP's vendor-id (and
> > possibly more info), and that seems awkward at best.
>
> This is actually kind of weird.  I don't know why a particular device
> would have any special MPS or MRRS requirements.
>
> Do you have any more details about this?  Does the WiFi chip actually
> not *work* with other MPS settings?  Or is this a performance thing
> where other settings don't give acceptable performance?

Hi Bjorn,
I looked up the original request mail thread on this -- it appears
that the device defaulted to an MPS value of 128, and that value is
what it was assigned. Unfortunately, this device has an occasional  HW
bug when MPS=128.  Rather than fix the HW bug, they wanted a SW change
in Linux,  and somehow that request morphed into a request for
PCIE_BUS_XXX config settings.  So the reason for the original
submission is weak and we should have done something different, but
here we are.

>
> I assume there's no hotplug in this setup, so we don't't have to worry
> about adding new devices at runtime.  Do you know if there's any
> P2PDMA usage?

No hotplug, no P2P.

Regards,
Jim Quinlan
Broadcom STB/CM

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5471 bytes --]

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

[Florian Fainelli]

On 3/5/26 13:36, Jim Quinlan wrote:
> On Mon, Mar 2, 2026 at 7:03 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>>
>> On Tue, Feb 24, 2026 at 04:10:17PM -0500, Jim Quinlan wrote:
>>> On Fri, Feb 20, 2026 at 5:11 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>
>>>>
>>>> We merged this as b0e85c3c8554 ("PCI: Add Kconfig options for MPS/MRRS
>>>> strategy"), which appeared in v5.10.
>>>>
>>>> In retrospect, I think this might have been a mistake because it
>>>> forces a build-time configuration for something that may not be known
>>>> at build time and can be set via command-line parameter.
>>> ...
>>
>>>> But I can't find any discussion about it.  Did you have a use case
>>>> where command line parameters weren't usable?
>>>
>>> Yes.  We have a Cable Modem (CM) customer who can update their Linux
>>> version but cannot update their boot loader or its default bootline.
>>> FWIW, they only want the option to set pcie_bus_config to
>>> PCIE_BUS_SAFE via the .config.
>>> ...
>>
>>> It's a CM product which uses a particular Wifi chip.  Recognizing this
>>> scenario would have the RC trying to grab the EP's vendor-id (and
>>> possibly more info), and that seems awkward at best.
>>
>> This is actually kind of weird.  I don't know why a particular device
>> would have any special MPS or MRRS requirements.
>>
>> Do you have any more details about this?  Does the WiFi chip actually
>> not *work* with other MPS settings?  Or is this a performance thing
>> where other settings don't give acceptable performance?
> 
> Hi Bjorn,
> I looked up the original request mail thread on this -- it appears
> that the device defaulted to an MPS value of 128, and that value is
> what it was assigned. Unfortunately, this device has an occasional  HW
> bug when MPS=128.  Rather than fix the HW bug, they wanted a SW change
> in Linux,  and somehow that request morphed into a request for
> PCIE_BUS_XXX config settings.  So the reason for the original
> submission is weak and we should have done something different, but
> here we are.

Late to the party and probably could have suggested back then, but this 
seems like a prime candidate for using a PCI quirk fixup?
-- 
Florian

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

[Bjorn Helgaas]

On Thu, Mar 05, 2026 at 01:38:43PM -0800, Florian Fainelli wrote:
> On 3/5/26 13:36, Jim Quinlan wrote:
> > On Mon, Mar 2, 2026 at 7:03 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > On Tue, Feb 24, 2026 at 04:10:17PM -0500, Jim Quinlan wrote:
> > > > On Fri, Feb 20, 2026 at 5:11 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>
> > > > > 
> > > > > We merged this as b0e85c3c8554 ("PCI: Add Kconfig options for MPS/MRRS
> > > > > strategy"), which appeared in v5.10.
> > > > > 
> > > > > In retrospect, I think this might have been a mistake because it
> > > > > forces a build-time configuration for something that may not be known
> > > > > at build time and can be set via command-line parameter.
> > > > ...
> > > 
> > > > > But I can't find any discussion about it.  Did you have a use case
> > > > > where command line parameters weren't usable?
> > > > 
> > > > Yes.  We have a Cable Modem (CM) customer who can update their Linux
> > > > version but cannot update their boot loader or its default bootline.
> > > > FWIW, they only want the option to set pcie_bus_config to
> > > > PCIE_BUS_SAFE via the .config.
> > > > ...
> > > 
> > > > It's a CM product which uses a particular Wifi chip.  Recognizing this
> > > > scenario would have the RC trying to grab the EP's vendor-id (and
> > > > possibly more info), and that seems awkward at best.
> > > 
> > > This is actually kind of weird.  I don't know why a particular device
> > > would have any special MPS or MRRS requirements.
> > > 
> > > Do you have any more details about this?  Does the WiFi chip actually
> > > not *work* with other MPS settings?  Or is this a performance thing
> > > where other settings don't give acceptable performance?
> > 
> > Hi Bjorn,
> > I looked up the original request mail thread on this -- it appears
> > that the device defaulted to an MPS value of 128, and that value is
> > what it was assigned. Unfortunately, this device has an occasional HW
> > bug when MPS=128.  Rather than fix the HW bug, they wanted a SW change
> > in Linux, and somehow that request morphed into a request for
> > PCIE_BUS_XXX config settings.  So the reason for the original
> > submission is weak and we should have done something different, but
> > here we are.
> 
> Late to the party and probably could have suggested back then, but this
> seems like a prime candidate for using a PCI quirk fixup?

Definitely.  The WiFi devices is likely used in platforms other than
the Cable Modem in question.  The CM avoids the hardware bug by
building with PCIE_BUS_SAFE, which probably sets MPS larger than 128.

But other platforms may use the same device and trip over this
hardware issue if they use different PCIE_BUS_* config or kernel
parameters that result in MPS=128.

What is the WiFi device in question?  Maybe we can look for reports of
problems or try to reproduce the issue if we know where to look.  Then
maybe we can figure out a solution that helps everybody.

Bjorn