Commit 47c8846a49ba ("PCI: Extend ACS configurability") introduced a bug that fails to configure ACS ctrl for multiple PCI devices. It affects both 'config_acs' and 'disable_acs_redir'. For example, using 'config_acs' to configure ACS ctrl for multiple BDFs fails: [ 0.000000] Kernel command line: pci=config_acs=1111011@0020:02:00.0;101xxxx@0039:00:00.0 "dyndbg=file drivers/pci/pci.c +p" earlycon [ 12.349875] PCI: Can't parse ACS command line parameter [ 19.629314] pci 0020:02:00.0: ACS mask = 0x007f [ 19.629315] pci 0020:02:00.0: ACS flags = 0x007b [ 19.629316] pci 0020:02:00.0: Configured ACS to 0x007b After this fix: [ 0.000000] Kernel command line: pci=config_acs=1111011@0020:02:00.0;101xxxx@0039:00:00.0 "dyndbg=file drivers/pci/pci.c +p" earlycon [ 19.583814] pci 0020:02:00.0: ACS mask = 0x007f [ 19.588532] pci 0020:02:00.0: ACS flags = 0x007b [ 19.593249] pci 0020:02:00.0: ACS control = 0x001d [ 19.598143] pci 0020:02:00.0: Configured ACS to 0x007b [ 24.088699] pci 0039:00:00.0: ACS mask = 0x0070 [ 24.093416] pci 0039:00:00.0: ACS flags = 0x0050 [ 24.098136] pci 0039:00:00.0: ACS control = 0x001d [ 24.103031] pci 0039:00:00.0: Configured ACS to 0x005d For example, using 'disable_acs_redire' fails to clear all three ACS P2P redir bits: [ 0.000000] Kernel command line: pci=disable_acs_redir=0020:02:00.0;0030:02:00.0;0039:00:00.0 "dyndbg=file drivers/pci/pci.c +p" earlycon [ 19.615860] pci 0020:02:00.0: ACS mask = 0x002c [ 19.615862] pci 0020:02:00.0: ACS flags = 0xffd3 [ 19.615863] pci 0020:02:00.0: Configured ACS to 0xfffb [ 22.332683] pci 0030:02:00.0: ACS mask = 0x002c [ 22.332685] pci 0030:02:00.0: ACS flags = 0xffd3 [ 22.332686] pci 0030:02:00.0: Configured ACS to 0xffdf [ 24.110278] pci 0039:00:00.0: ACS mask = 0x002c [ 24.110281] pci 0039:00:00.0: ACS flags = 0xffd3 [ 24.110283] pci 0039:00:00.0: Configured ACS to 0xffd3 After this fix: [ 0.000000] Kernel command line: pci=disable_acs_redir=0020:02:00.0;0030:02:00.0;0039:00:00.0 "dyndbg=file drivers/pci/pci.c +p" earlycon [ 19.597909] pci 0020:02:00.0: ACS mask = 0x002c [ 19.597910] pci 0020:02:00.0: ACS flags = 0xffd3 [ 19.597911] pci 0020:02:00.0: ACS control = 0x007f [ 19.597911] pci 0020:02:00.0: Configured ACS to 0x0053 [ 22.314124] pci 0030:02:00.0: ACS mask = 0x002c [ 22.314126] pci 0030:02:00.0: ACS flags = 0xffd3 [ 22.314127] pci 0030:02:00.0: ACS control = 0x005f [ 22.314128] pci 0030:02:00.0: Configured ACS to 0x0053 [ 24.091711] pci 0039:00:00.0: ACS mask = 0x002c [ 24.091712] pci 0039:00:00.0: ACS flags = 0xffd3 [ 24.091714] pci 0039:00:00.0: ACS control = 0x001d [ 24.091715] pci 0039:00:00.0: Configured ACS to 0x0011 Fixes: 47c8846a49ba ("PCI: Extend ACS configurability") Signed-off-by: Tushar Dave <tdave@xxxxxxxxxx> --- Documentation/admin-guide/kernel-parameters.txt | 11 +++++------ drivers/pci/pci.c | 16 +++++++++------- 2 files changed, 14 insertions(+), 13 deletions(-) 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 diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 0b29ec6e8e5e..35ff21b014ac 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -951,12 +951,13 @@ static const char *config_acs_param; struct pci_acs { u16 cap; u16 ctrl; - u16 fw_ctrl; }; static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps, - const char *p, u16 mask, u16 flags) + const char *p, const u16 acs_mask, const u16 acs_flags) { + u16 flags = acs_flags; + u16 mask = acs_mask; char *delimit; int ret = 0; @@ -964,7 +965,7 @@ static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps, return; while (*p) { - if (!mask) { + if (!acs_mask) { /* Check for ACS flags */ delimit = strstr(p, "@"); if (delimit) { @@ -972,6 +973,8 @@ static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps, u32 shift = 0; end = delimit - p - 1; + mask = 0; + flags = 0; while (end > -1) { if (*(p + end) == '0') { @@ -1028,10 +1031,10 @@ static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps, pci_dbg(dev, "ACS mask = %#06x\n", mask); pci_dbg(dev, "ACS flags = %#06x\n", flags); + pci_dbg(dev, "ACS control = %#06x\n", caps->ctrl); - /* If mask is 0 then we copy the bit from the firmware setting. */ - caps->ctrl = (caps->ctrl & ~mask) | (caps->fw_ctrl & mask); - caps->ctrl |= flags; + caps->ctrl &= ~mask; + caps->ctrl |= (flags & mask); pci_info(dev, "Configured ACS to %#06x\n", caps->ctrl); } @@ -1082,7 +1085,6 @@ static void pci_enable_acs(struct pci_dev *dev) pci_read_config_word(dev, pos + PCI_ACS_CAP, &caps.cap); pci_read_config_word(dev, pos + PCI_ACS_CTRL, &caps.ctrl); - caps.fw_ctrl = caps.ctrl; if (enable_acs) pci_std_enable_acs(dev, &caps); -- 2.34.1