[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240723224102.GA779599@bhelgaas>
Date: Tue, 23 Jul 2024 17:41:02 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Wei Huang <wei.huang2@....com>
Cc: linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-doc@...r.kernel.org, netdev@...r.kernel.org,
Jonathan.Cameron@...wei.com, corbet@....net, davem@...emloft.net,
edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
alex.williamson@...hat.com, gospo@...adcom.com,
michael.chan@...adcom.com, ajit.khaparde@...adcom.com,
somnath.kotur@...adcom.com, andrew.gospodarek@...adcom.com,
manoj.panicker2@....com, Eric.VanTassell@....com,
vadim.fedorenko@...ux.dev, horms@...nel.org, bagasdotme@...il.com,
bhelgaas@...gle.com
Subject: Re: [PATCH V3 03/10] PCI/TPH: Add pci=notph to prevent use of TPH
On Wed, Jul 17, 2024 at 03:55:04PM -0500, Wei Huang wrote:
> TLP headers with incorrect steering tags (e.g. caused by buggy driver)
> can potentially cause issues when the system hardware consumes the tags.
Hmm. What kind of issues? Crash? Data corruption? Poor
performance?
> Provide a kernel option, with related helper functions, to completely
> prevent TPH from being enabled.
Also would be nice to have a hint about the difference between "notph"
and "nostmode". Maybe that goes in the "nostmode" patch? I'm not
super clear on all the differences here.
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4655,6 +4655,7 @@
> nomio [S390] Do not use MIO instructions.
> norid [S390] ignore the RID field and force use of
> one PCI domain per PCI function
> + notph [PCIE] Do not use PCIe TPH
Expand acronym here since there's no helpful context. Can also
include "(TPH)" if that's useful.
> @@ -322,8 +323,12 @@ static long local_pci_probe(void *_ddi)
> pm_runtime_get_sync(dev);
> pci_dev->driver = pci_drv;
> rc = pci_drv->probe(pci_dev, ddi->id);
> - if (!rc)
> + if (!rc) {
> + if (pci_tph_disabled())
> + pcie_tph_disable(pci_dev);
I'm not really a fan of cluttering probe() like this. Can't we
disable it in pcie_tph_init() so all devices start off with TPH
disabled, and then check pci_tph_disabled() in whatever interface
drivers use to enable TPH?
> +bool pci_tph_disabled(void)
> +{
> + return pcie_tph_disabled;
> +}
> +EXPORT_SYMBOL_GPL(pci_tph_disabled);
Other related interfaces use "pcie" prefix; I think this should match.
Do drivers need this? Would be nice not to export it unless they do.
Bjorn
Powered by blists - more mailing lists