netdev - Re: [PATCH V3 03/10] PCI/TPH: Add pci=notph to prevent use of TPH

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <28b953c3-ef66-4bd2-a024-ec860399ffbf@amd.com>
Date: Wed, 24 Jul 2024 15:05:59 -0500
From: Wei Huang <wei.huang2@....com>
To: Bjorn Helgaas <helgaas@...nel.org>
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 7/23/24 17:41, Bjorn Helgaas wrote:
> 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?

Not crash or functionality errors. Usually it is QoS related because of
resource competition. AMD has

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

I can combine them. Here is the combination and it meaning based on TPH
Control Register values:

Requestor Enable | ST Mode | Meaning
---------------------------------------------------------------
00               | xx      | TPH disabled (i.e. notph)
01               | 00      | TPH enabled, NO ST Mode (i.e. nostmode)
01 or 11         | 01      | Interrupt Vector mode
01 or 11         | 10      | Device specific mode

If you have any other thoughts on how to approach these modes, please
let me know.




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