Re: [PATCH] Documentation: Fix config_acs= example

[Bjorn Helgaas]

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

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@xxxxxxxxxx>
>