| Message ID | 20200908163248.14330-1-james.quinlan@broadcom.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [RESEND,v1] PCI: pcie_bus_config can be set at build time | expand |
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@broadcom.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 >
Hi Bjorn, On Wed, Sep 9, 2020 at 10:25 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > 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? Yes, we've been shipping our kernel with the DEFAULT and since we do not have FW it is not configured optimally. Some customers have noticed and I tell them to put 'pci=pcie_bus_safe' on their bootline but I'd rather have this setting work for all customers as it yields the option we want. > > 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. IIRC I just copied the comments that were in the bootline settings. I'm concerned about there being the same comment in two places; sooner or later someone will update one place and not the other. > > Maybe we could mention the corresponding command-line parameters, > e.g., "This is the same as booting with 'pci=pcie_bus_tune_off'"? Will do and resubmit. Thanks, Jim Quinlan Broadcom STB > > > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.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 > >
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
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 | 40 ++++++++++++++++++++++++++++++++++++++++ drivers/pci/pci.c | 12 ++++++++++++ 2 files changed, 52 insertions(+)