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

Bjorn Helgaas
2025-01-06T20:53:29Z

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.