From 9cf8a952d57b422d3ff8a9a0163f8adf694f4b2b Mon Sep 17 00:00:00 2001 From: Tushar Dave Date: Thu, 6 Feb 2025 19:03:38 -0800 Subject: [PATCH] PCI/ACS: Fix 'pci=config_acs=' parameter Commit 47c8846a49ba ("PCI: Extend ACS configurability") introduced bugs that fail to configure ACS ctrl to the value specified by the kernel parameter. Essentially there are two bugs: 1) When ACS is configured for multiple PCI devices using 'config_acs' kernel parameter, it results into error "PCI: Can't parse ACS command line parameter". This is due to a bug that doesn't preserve the ACS mask, but instead overwrites the mask with value 0. For example, using 'config_acs' to configure ACS ctrl for multiple BDFs fails: Kernel command line: pci=config_acs=1111011@0020:02:00.0;101xxxx@0039:00:00.0 "dyndbg=file drivers/pci/pci.c +p" PCI: Can't parse ACS command line parameter pci 0020:02:00.0: ACS mask = 0x007f pci 0020:02:00.0: ACS flags = 0x007b pci 0020:02:00.0: Configured ACS to 0x007b After this fix: Kernel command line: pci=config_acs=1111011@0020:02:00.0;101xxxx@0039:00:00.0 "dyndbg=file drivers/pci/pci.c +p" pci 0020:02:00.0: ACS mask = 0x007f pci 0020:02:00.0: ACS flags = 0x007b pci 0020:02:00.0: ACS control = 0x005f pci 0020:02:00.0: ACS fw_ctrl = 0x0053 pci 0020:02:00.0: Configured ACS to 0x007b pci 0039:00:00.0: ACS mask = 0x0070 pci 0039:00:00.0: ACS flags = 0x0050 pci 0039:00:00.0: ACS control = 0x001d pci 0039:00:00.0: ACS fw_ctrl = 0x0000 pci 0039:00:00.0: Configured ACS to 0x0050 2) In the bit manipulation logic, we copy the bit from the firmware settings when mask bit 0. For example, 'disable_acs_redir' fails to clear all three ACS P2P redir bits due to the wrong bit fiddling: 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" pci 0020:02:00.0: ACS mask = 0x002c pci 0020:02:00.0: ACS flags = 0xffd3 pci 0020:02:00.0: Configured ACS to 0xfffb pci 0030:02:00.0: ACS mask = 0x002c pci 0030:02:00.0: ACS flags = 0xffd3 pci 0030:02:00.0: Configured ACS to 0xffdf pci 0039:00:00.0: ACS mask = 0x002c pci 0039:00:00.0: ACS flags = 0xffd3 pci 0039:00:00.0: Configured ACS to 0xffd3 After this fix: 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" pci 0020:02:00.0: ACS mask = 0x002c pci 0020:02:00.0: ACS flags = 0xffd3 pci 0020:02:00.0: ACS control = 0x007f pci 0020:02:00.0: ACS fw_ctrl = 0x007b pci 0020:02:00.0: Configured ACS to 0x0053 pci 0030:02:00.0: ACS mask = 0x002c pci 0030:02:00.0: ACS flags = 0xffd3 pci 0030:02:00.0: ACS control = 0x005f pci 0030:02:00.0: ACS fw_ctrl = 0x005f pci 0030:02:00.0: Configured ACS to 0x0053 pci 0039:00:00.0: ACS mask = 0x002c pci 0039:00:00.0: ACS flags = 0xffd3 pci 0039:00:00.0: ACS control = 0x001d pci 0039:00:00.0: ACS fw_ctrl = 0x0000 pci 0039:00:00.0: Configured ACS to 0x0000 Link: https://lore.kernel.org/r/20250207030338.456887-1-tdave@nvidia.com Fixes: 47c8846a49ba ("PCI: Extend ACS configurability") Signed-off-by: Tushar Dave Signed-off-by: Bjorn Helgaas Reviewed-by: Jason Gunthorpe Reviewed-by: Kuppuswamy Sathyanarayanan --- drivers/pci/pci.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 869d204a70a3..23609dd123f9 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -954,8 +954,10 @@ struct pci_acs { }; 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; @@ -963,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) { @@ -971,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') { @@ -1027,10 +1031,14 @@ 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); + pci_dbg(dev, "ACS fw_ctrl = %#06x\n", caps->fw_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; + /* + * For mask bits that are 0, copy them from the firmware setting + * and apply flags for all the mask bits that are 1. + */ + caps->ctrl = (caps->fw_ctrl & ~mask) | (flags & mask); pci_info(dev, "Configured ACS to %#06x\n", caps->ctrl); } -- 2.25.1