linux-kernel - Re: [PATCH 1/1] PCI: Fix Extend ACS configurability

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: <c7b7316c-127a-4c10-8e94-bcdefd2188d4@nvidia.com>
Date: Tue, 7 Jan 2025 18:34:35 -0800
From: Tushar Dave <tdave@...dia.com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: Jason Gunthorpe <jgg@...dia.com>, corbet@....net, bhelgaas@...gle.com,
 paulmck@...nel.org, akpm@...ux-foundation.org, thuth@...hat.com,
 rostedt@...dmis.org, xiongwei.song@...driver.com, vidyas@...dia.com,
 linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-pci@...r.kernel.org, vsethi@...dia.com, sdonthineni@...dia.com
Subject: Re: [PATCH 1/1] PCI: Fix Extend ACS configurability



On 1/6/25 12:53, Bjorn Helgaas wrote:
> On Mon, Jan 06, 2025 at 12:34:00PM -0800, Tushar Dave wrote:
>> On 1/2/25 10:40, Jason Gunthorpe wrote:
>>> On Fri, Dec 13, 2024 at 12:29:42PM -0800, Tushar Dave wrote:
>>>
>>>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>>>> index dc663c0ca670..fc1c37910d1c 100644
>>>> --- a/Documentation/admin-guide/kernel-parameters.txt
>>>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>>>> @@ -4654,11 +4654,10 @@
>>>>    				Format:
>>>>    				<ACS flags>@<pci_dev>[; ...]
>>>>    				Specify one or more PCI devices (in the format
>>>> -				specified above) optionally prepended with flags
>>>> -				and separated by semicolons. The respective
>>>> -				capabilities will be enabled, disabled or
>>>> -				unchanged based on what is specified in
>>>> -				flags.
>>>> +				specified above) prepended with flags and separated
>>>> +				by semicolons. The respective capabilities will be
>>>> +				enabled, disabled or unchanged based on what is
>>>> +				specified in flags.
>>>>    				ACS Flags is defined as follows:
>>>>    				  bit-0 : ACS Source Validation
>>>> @@ -4673,7 +4672,7 @@
>>>>    				  '1' – force enabled
>>>>    				  'x' – unchanged
>>>>    				For example,
>>>> -				  pci=config_acs=10x
>>>> +				  pci=config_acs=10x@pci:0:0
>>>>    				would configure all devices that support
>>>>    				ACS to enable P2P Request Redirect, disable
>>>>    				Translation Blocking, and leave Source
>>>
>>> Is this an unrelated change? The format of the command line shouldn't
>>> be changed to fix the described bug, why is the documentation changed?
>>
>> The documentation as it is (i.e. without my patch), is not correct.
>>
>> IOW, config_acs parameter does require flags and it is not optional. Without
>> flags it results into "ACS Flags missing". Therefore I remove word
>> "optionally" from the documentation text.
>>
>> Secondly, the syntax in the example 'pci=config_acs=10x' is incorrect. The
>> correct syntax should be 'pci=config_acs=10x@pci:0:0' that would configure
>> all devices that support ACS to enable P2P Request Redirect, disable
>> Translation Blocking, and leave Source Validation unchanged from whatever
>> power-up or firmware set it to.
> 
> I'd suggest a separate patch to fix the documentation so we don't try
> to relate the doc changes with the code changes.

Sure thing. I will send a separate patch for the doc changes.

-Tushar