linux-kernel - Re: [PATCH] Documentation: Fix config

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: <20250113174035.GA408797@bhelgaas>
Date: Mon, 13 Jan 2025 11:40:35 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Akihiko Odaki <akihiko.odaki@...nix.com>
Cc: Jonathan Corbet <corbet@....net>, Bjorn Helgaas <bhelgaas@...gle.com>,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-pci@...r.kernel.org,
	Krzysztof Wilczyński <kwilczynski@...nel.org>,
	Tushar Dave <tdave@...dia.com>, Jason Gunthorpe <jgg@...dia.com>,
	Vidya Sagar <vidyas@...dia.com>, Vikram Sethi <vsethi@...dia.com>,
	Shanker Donthineni <sdonthineni@...dia.com>,
	"Paul E. McKenney" <paulmck@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Huth <thuth@...hat.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Xiongwei Song <xiongwei.song@...driver.com>
Subject: Re: [PATCH] Documentation: Fix config_acs= example

[+cc folks from related discussion at
https://lore.kernel.org/r/20241213202942.44585-1-tdave@nvidia.com]

On Sun, Sep 15, 2024 at 10:36:58AM +0900, Akihiko Odaki wrote:
> The documentation used to say:
> > For example,
> >   pci=config_acs=10x
> > 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.
> 
> However, a flag specification always needs to be suffixed with "@" and a
> PCI device string, which is missing in this example. It needs to be
> suffixed with "@pci:0:0" to configure all devices that support ACS in
> particular.

Thanks for the patch.  Krzysztof added a bit to the commit log at:

https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?id=57722057d762

but I think we should also include the flags template:

  config_acs =
                Format:
                <ACS flags>@<pci_dev>[; ...]

so the commit log includes a hint about what "flag specification"
means.

Also, I think the "pci_dev" documentation is poor and should be
improved in a second patch.  The only current mention I see is here:

        pci=option[,option...]  [PCI,EARLY] various PCI subsystem options.

                                Some options herein operate on a specific device
                                or a set of devices (<pci_dev>). These are
                                specified in one of the following formats:

                                [<domain>:]<bus>:<dev>.<func>[/<dev>.<func>]*
                                pci:<vendor>:<device>[:<subvendor>:<subdevice>]

                                Note: the first format specifies a PCI
                                bus/device/function address which may change
                                if new hardware is inserted, if motherboard
                                firmware changes, or due to changes caused
                                by other kernel parameters. If the
                                domain is left unspecified, it is
                                taken to be zero. Optionally, a path
                                to a device through multiple device/function
                                addresses can be specified after the base
                                address (this is more robust against
                                renumbering issues).  The second format
                                selects devices using IDs from the
                                configuration space which may match multiple
                                devices in the system.

where I guess "pci_dev" means the second format:

  pci:<vendor>:<device>[:<subvendor>:<subdevice>]

and apparently "pci:0:0" means all devices with vendor==0 and
device==0, and it's not completely obvious to me that this means "all
devices".

> ---
>  Documentation/admin-guide/kernel-parameters.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index ee2984e46c06..5611903c27a9 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4604,7 +4604,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
> 
> ---
> base-commit: 46a0057a5853cbdb58211c19e89ba7777dc6fd50
> change-id: 20240911-acs-3043a2737cc9
> 
> Best regards,
> -- 
> Akihiko Odaki <akihiko.odaki@...nix.com>
>