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

From: Jim Quinlan
Date: Wed Sep 30 2020 - 16:58:02 EST


On Tue, Sep 29, 2020 at 5:31 PM Bjorn Helgaas <helgaas@xxxxxxxxxx> 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@xxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxx>
> 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@xxxxxxxxxxxx
> Signed-off-by: Jim Quinlan <james.quinlan@xxxxxxxxxxxx>
> Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
>
> 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: smime.p7s
Description: S/MIME Cryptographic Signature