[PATCH v9 0/6] PCI: Add support for ACS Enhanced Capability

[PATCH v9 0/6] PCI: Add support for ACS Enhanced Capability

[Wei Wang]

This patchset improves the core ACS implementation and adds support for
the Access Control Services (ACS) Enhanced Capability, introduced with
PCIe Gen 5.

Improvements to the core ACS implementation include:
- Validating ACS enable flags against device-specific capabilities rather
  than generic kernel masks. This ensures only supported features are
  enabled while safely ignoring attempts to disable unsupported bits.

- Consolidating delimiter parsing into pci_dev_str_match() and returning
  -ENODEV when no further entries can be parsed. This removes duplicated
  logic in callers.

- Refactoring ACS parameter handling by splitting the intertwined
  disable_acs_redir and config_acs param logic into dedicated functions.
  This improves maintainability and robustness while optimizing parsing
  with better validation and readability.

- Updating the config_acs kernel parameter documentation to include an
  example of multi-device configuration with distinct settings and
  advising users to quote the parameter to avoid bootloader parsing
  issues with the semicolon separator.

Support for the ACS Enhanced Capability is built on top of this improved
implementation. This capability provides additional access control
features that improve device isolation — particularly important in
virtualization scenarios where devices are passed through to different
virtual machines (VMs). Strong isolation is critical to ensure security
between devices assigned to different VMs and the host.

In Linux, device grouping assumes that devices in separate IOMMU groups
are properly isolated. To uphold this assumption, the enhanced ACS
controls are enabled by default on hardware that supports the PCI_ACS_ECAP
capability. As with other basic ACS access controls, these new controls
can be configured via the config_acs= boot parameter.

Support for checking the enhanced ACS controls on Root and Downstream
Ports has been added to pci_acs_enabled(). On devices that support
PCI_ACS_ECAP, these controls must be properly enabled. To maintain
compatibility with legacy devices that lack PCI_ACS_ECAP support,
pci_acs_enabled() simply skips the check.

v8->v9 changes:
Patch 1:
  - strip unsupported bits from both mask and flags rather than aborting
    the entire configuration. This aligns with the existing command-line
    configurations to have the supported bits applied at least (Sashiko)
Patch 4:
  - Preserve pre-refactor behavior for the empty-bitstring form, e.g.
   "@0000:01:00.0" in config_acs, which resets caps->ctrl to caps->fw_ctrl
    for the matched device (undoing pci_std_enable_acs()). Also restore the
   "ACS Flags missing" pci_err() diagnostic on a missing '@' delimiter.
   (Sashiko)
  - Two other comments from Sashiko were marked as non-critical and
    pre-existing; left unchanged for now to preserve the current behavior.
Patch 5:
  - Enable Unclaimed Request Redirect Control and Upstream Port Memory
    Target Access Redirect for Downstream Port only (PCI_ACS_ECAP is set
    in pci_acs_enabled() for root port and downstream port).(Sashiko)
  v8 Link: https://lore.kernel.org/all/SI2PR01MB43932642FC4DD3D075983B1FDC0A2@SI2PR01MB4393.apcprd01.prod.exchangelabs.com/

v7->v8 changes:
Patch 1:
  - Validate against dev->acs_capabilities & GENMASK_U16(6, 0) rather than
    the raw dev->acs_capabilities, as the latter may contain fields (like
    the egress control vector size) that do not correspond to bits in the
    ACS control register. (Sashiko-bot)
  - Gate the new capability check on !acs_mask so it only fires for the
    config_acs parameter path. This avoids breaking legacy
    disable_acs_redir behavior. (Sashiko-bot)
  - Validate both enable and disable bits (the full mask), not just flags.
    Reverts the v7 narrowing to enable-only.
Patch 2:
  - Remove the leading '-' characters in the documentation examples to
    prevent copy-paste confusion for users. (Randy)
  - Recommend using double quotes instead of single quotes for multiple
    devices to avoid potential shell-escaping issues where single quotes
    might be treated as literal characters. (Sashiko-bot)
Patch 3:
  - Move the error reporting back to the callers so that caller context
    is included in the dmesg output. (Krzysztof)
  - In pci_dev_str_match(): enforce stricter boundary checks on successful
    matches (see Patch 3 commit log for full details).
Patch 4:
  - Move the pci_info logging for ACS modifications up to pci_enable_acs()
    so it prints for both disable_acs_redir and config_acs modifications.
    Introduction of kernel_default_ctrl so the print only fires when a
    cmdline param diverges from pci_std_enable_acs()'s output (not on
    every pci_std_enable_acs() modification). (Sashiko-bot)
  - Drop max_shift; bound the parse loop by the u16 width (16) and rely on
    the existing invalid_bits & ~valid_ctrl check to report out-of-range
    bits with their bitmask, which is more informative than reporting the
    parse position.
  - Drop the -ENODEV "no more entries" signal. *endptr is now advanced
    past the delimiter so callers terminate the loop naturally via
    while (*p).
Patch 5:
  - In pci_param_config_acs(): when ECAP is supported by the device add
    the ECAP control bits (GENMASK_U16(12, 7)) to valid_ctrl.
  - Validate DMAC/UMAC reserved encoding against the prospective ctrl
    value rather than enabled_bits alone, so the case where the user
    enables one bit while fw_ctrl already has the other set is also
    caught.
Patch 6:
  - In pci_acs_ecap_enabled(): Add a defensive NULL check for the upstream
    port before dereferencing it. While standard downstream ports should
    always have an upstream port, this guards against kernel panics in
    malformed topologies. (Sashiko-bot)
  - Remove the redundant pdev->acs_capabilities & PCI_ACS_ECAP condition
    in pci_acs_flags_enabled(). The preceding bitwise operation
    (acs_flags &= (pdev->acs_capabilities | PCI_ACS_EC))
    already guarantees that acs_flags will only contain the PCI_ACS_ECAP
    bit if the device actually supports it.
  v7 Link: https://lore.kernel.org/all/SI2PR01MB439385689A32A1DDA9CEABE1DC3F2@SI2PR01MB4393.apcprd01.prod.exchangelabs.com/

v6->v7 changes:
 - Rebased onto next-20260505 (based on v7.1-rc2)
 - Picked up Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
 - No functional changes
 v6 Link: https://lore.kernel.org/all/SEZPR01MB439931D7320F6C476D181C1EDC44A@SEZPR01MB4399.apcprd01.prod.exchangelabs.com/

v5->v6 changes:
- Patch 3: In pci_dev_str_match(), explicitly set `ret = 0` when no
  matching string is found. This resolves a smatch warning that `ret`
  returned from sscanf() may be 2 or 4 even though no matching string
  is found later comparison.
  v5 Link: https://lore.kernel.org/all/SI2PR01MB439326AF08A79D1C5661C29BDC6CA@SI2PR01MB4393.apcprd01.prod.exchangelabs.com/

v4->v5 changes:
- Added significant refactoring of the core ACS implementation (Patches
  1-4) to improve validation, safety, and readability;
- For USP and DSP Memory Target Access Control, added masks and enum
  values for the encodings and explicitly rejected the reserved encoding
  (0b11);
- In pci_acs_ecap_enabled(), removed the use of 'is_dsp' variable.
  v4 Link: https://lore.kernel.org/all/SI2PR01MB43932C799AE9111C7D2C319FDC65A@SI2PR01MB4393.apcprd01.prod.exchangelabs.com/

v3->v4 changes:
- In pci_acs_ecap_enabled(): Check the pcie type for
  PCI_EXP_TYPE_DOWNSTREAM explicitly.
  v3 Link: https://lore.kernel.org/all/SI2PR01MB439325B4E44D5A39F34A4015DC9AA@SI2PR01MB4393.apcprd01.prod.exchangelabs.com/

v2->v3 changes:
- Drop the warning when a device has no support for the enhanced
  capability.
  v2 Link: https://lore.kernel.org/all/SI2PR01MB4393B836EA4FEDD1823483BADC94A@SI2PR01MB4393.apcprd01.prod.exchangelabs.com/

v1->v2 changes:
- Enabled all enhanced ACS controls by default, rather than just Unclaimed
  Request Redirect (which addressed the primary issue we encountered);
- Added checks for enhanced ACS controls on Root and Downstream Ports in
  pci_acs_enabled() to ensure proper enablement when grouping devices or
  enabling features such as IOMMU PASID.
  v1 Link: https://lore.kernel.org/all/SI2PR01MB43931A911357962A5E986FFEDC8CA@SI2PR01MB4393.apcprd01.prod.exchangelabs.com/

Thanks to Jason Gunthorpe, Jonathan Cameron, Dan Carpenter,
Krzysztof Wilczyński, and Randy Dunlap for reviewing the patchset.

Patches on github: https://github.com/wei-w-wang/linux/tree/v9-acs-enhanced-cap-and-refactor

Wei Wang (6):
  PCI: Validate ACS control bits against device-specific ACS
    capabilities
  Documentation/kernel-parameters: Add multi-device config_acs example
  PCI: Consolidate delimiter handling into pci_dev_str_match()
  PCI: Refactor disable_acs_redir and config_acs param handling
  PCI: Enable the enhanced ACS controls introduced by PCI_ACS_ECAP
  PCI: Add the enhanced ACS controls check to pci_acs_flags_enabled()

 .../admin-guide/kernel-parameters.txt         |  30 +-
 drivers/pci/pci.c                             | 313 +++++++++++++-----
 include/uapi/linux/pci_regs.h                 |  13 +
 3 files changed, 257 insertions(+), 99 deletions(-)


base-commit: 32b6ef9a5d0eca44f9cd91f52f4faa89f145a0de
-- 
2.51.0

[PATCH v9 1/6] PCI: Validate ACS control bits against device-specific ACS capabilities

[Wei Wang]

The ACS control validation used the kernel's full set of ACS control bits,
which allowed users to request ACS features that the device does not
support. Because hardware silently ignores these unsupported bits, users
have no indication that their config_acs= request was partially
ineffective.

Validate the requested ACS control bits against dev->acs_capabilities so
that only device-supported ACS controls are accepted. Mask the capability
with GENMASK_U16(6, 0) to select only the currently defined ACS control
bits (0-6). Higher bits in the capability structure (e.g. the egress
control vector size in bits 8-15) do not correspond to control bits in
the ACS control register. Add a debug message to report which unsupported
bits were ignored.

__pci_config_acs() is also called from disable_acs_redir_param(), which
passes a hardcoded acs_mask (PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC) that
does not need validation. Gate the new check on !acs_mask so it only
fires for the config_acs_param path, which always invokes
__pci_config_acs() with acs_mask == 0 and builds the actual mask from
user input.

Also move the check after the device is matched, since the ctrl bits apply
only to the matched device.

Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/pci/pci.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b2879a6be5f8..2af679111a9b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -910,6 +910,7 @@ struct pci_acs {
 static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
 			     const char *p, const u16 acs_mask, const u16 acs_flags)
 {
+	u16 valid_ctrl = dev->acs_capabilities & GENMASK_U16(6, 0);
 	u16 flags = acs_flags;
 	u16 mask = acs_mask;
 	char *delimit;
@@ -955,12 +956,6 @@ static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
 			}
 		}
 
-		if (mask & ~(PCI_ACS_SV | PCI_ACS_TB | PCI_ACS_RR | PCI_ACS_CR |
-			    PCI_ACS_UF | PCI_ACS_EC | PCI_ACS_DT)) {
-			pci_err(dev, "Invalid ACS flags specified\n");
-			return;
-		}
-
 		ret = pci_dev_str_match(dev, p, &p);
 		if (ret < 0) {
 			pr_warn_once("PCI: Can't parse ACS command line parameter\n");
@@ -983,6 +978,13 @@ static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
 	if (!pci_dev_specific_disable_acs_redir(dev))
 		return;
 
+	if (!acs_mask && (mask & ~valid_ctrl)) {
+		pci_dbg(dev, "Ignoring unsupported ACS bits: %#06x\n",
+			mask & ~valid_ctrl);
+		mask &= valid_ctrl;
+		flags &= valid_ctrl;
+	}
+
 	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);
-- 
2.51.0

[PATCH v9 2/6] Documentation/kernel-parameters: Add multi-device config_acs example

[Wei Wang]

The config_acs parameter allows configuring ACS settings for multiple PCI
devices by separating entries with semicolons. The current documentation
only illustrates applying one configuration pattern to all devices, but
does not show how to specify multiple devices with different ACS settings
in a single parameter. Add an example demonstrating multi-device usage
with distinct ACS configurations.

Some bootloaders interpret ';' as a command separator, which can cause
the parameter to be split as multiple commands. Document that the entire
parameter may need to be quoted to avoid bootloader parsing issues. This
avoids confusing bootloader errors such as "can't find command <BDF>"
when passing multiple device entries.

Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 63f0c66c9123..a2563ff69f51 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5345,6 +5345,13 @@ Kernel parameters
 
 				Note: this may remove isolation between devices
 				and may put more devices in an IOMMU group.
+
+				  "pci=config_acs=10x@0000:04:00.0;1x101@0000:84:00.0"
+				configures two devices with different ACS settings.
+				Note: Some bootloaders interpret ';' as a command
+				separator. If so, quote the entire parameter to
+				ensure it is passed to the kernel unchanged.
+
 		force_floating	[S390] Force usage of floating interrupts.
 		nomio		[S390] Do not use MIO instructions.
 		norid		[S390] ignore the RID field and force use of
-- 
2.51.0

[PATCH v9 3/6] PCI: Consolidate delimiter handling into pci_dev_str_match()

[Wei Wang]

Callers of pci_dev_str_match() manually checked for the ';' or ','
delimiter and advanced the parameter pointer past it. Move this common
logic into pci_dev_str_match() so callers no longer need to duplicate it.

As a side effect of this consolidation, pci_dev_str_match() now enforces
stricter boundary checks on successful matches. Previously, if a user
provided a malformed parameter with garbage characters appended to a
valid BDF (e.g., "pci=config_acs=110x@0000:04:00.0foo"), the callers
would silently accept the prefix match and ignore the trailing garbage.
Now, pci_dev_str_match() strictly verifies that the matched token is
immediately followed by a valid delimiter (';', ',') or the null
terminator ('\0'), returning -EINVAL if it encounters invalid trailing
characters.

Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/pci/pci.c | 40 ++++++++++++++++++----------------------
 1 file changed, 18 insertions(+), 22 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 2af679111a9b..587c013f04db 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -336,7 +336,7 @@ static int pci_dev_str_match_path(struct pci_dev *dev, const char *path,
  * pci_dev_str_match - test if a string matches a device
  * @dev: the PCI device to test
  * @p: string to match the device against
- * @endptr: pointer to the string after the match
+ * @endptr: pointer to the string after the match, with the delimiter skipped
  *
  * Test if a string (typically from a kernel parameter) matches a specified
  * PCI device. The string may be of one of the following formats:
@@ -390,8 +390,13 @@ static int pci_dev_str_match(struct pci_dev *dev, const char *p,
 		    (!subsystem_vendor ||
 			    subsystem_vendor == dev->subsystem_vendor) &&
 		    (!subsystem_device ||
-			    subsystem_device == dev->subsystem_device))
-			goto found;
+			    subsystem_device == dev->subsystem_device)) {
+			ret = 1;
+			goto out;
+		}
+
+		/* No matching string found */
+		ret = 0;
 	} else {
 		/*
 		 * PCI Bus, Device, Function IDs are specified
@@ -401,15 +406,18 @@ static int pci_dev_str_match(struct pci_dev *dev, const char *p,
 		if (ret < 0)
 			return ret;
 		else if (ret)
-			goto found;
+			goto out;
 	}
+out:
+	/*
+	 * Whether we matched (ret == 1) or didn't (ret == 0),
+	 * ensure the token ends with a valid boundary.
+	 */
+	if (*p != '\0' && *p != ';' && *p != ',')
+		return -EINVAL;
 
-	*endptr = p;
-	return 0;
-
-found:
-	*endptr = p;
-	return 1;
+	*endptr = *p == '\0' ? p : p + 1;
+	return ret;
 }
 
 static u8 __pci_find_next_cap(struct pci_bus *bus, unsigned int devfn,
@@ -964,12 +972,6 @@ static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
 			/* Found a match */
 			break;
 		}
-
-		if (*p != ';' && *p != ',') {
-			/* End of param or invalid format */
-			break;
-		}
-		p++;
 	}
 
 	if (ret != 1)
@@ -6488,12 +6490,6 @@ static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
 			       p);
 			break;
 		}
-
-		if (*p != ';' && *p != ',') {
-			/* End of param or invalid format */
-			break;
-		}
-		p++;
 	}
 out:
 	spin_unlock(&resource_alignment_lock);
-- 
2.51.0

[PATCH v9 4/6] PCI: Refactor disable_acs_redir and config_acs param handling

[Wei Wang]

The current implementation mixes disable_acs_redir and config_acs param
processing inside __pci_config_acs(), using acs_mask==0 as an implicit
signal to switch into config_acs_param mode. The intertwined logic is
hard to follow and easy to break:

  - The acs_mask==0 special case is non-obvious and obscures which code
    paths apply to which parameter.
  - The interleaved logic is fragile; changes intended for one parameter
    can unintentionally affect the other. For example,
    pci_dev_specific_disable_acs_redir() is invoked on the common path
    even though it should apply only to disable_acs_redir.

Split the two behaviors into dedicated functions,
pci_param_disable_acs_redir() and pci_param_config_acs(), making the
control flow explicit and easier to maintain.

The new pci_param_config_acs() implementation also improves on the
original logic:

  - It searches for the matching device first and parses ACS flags only
    for the matched device, avoiding unnecessary work.
  - A switch statement replaces multiple if/else blocks, improving
    readability.
  - Variable names (enabled_bits, disabled_bits) more clearly express
    their roles compared to the previous mask/flags pair.
  - Error messages are clearer.

This refactor separates concerns, improves robustness, and makes future
extensions to ACS parameter handling safer and easier to review.

Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>
---
 drivers/pci/pci.c | 180 +++++++++++++++++++++++++++-------------------
 1 file changed, 108 insertions(+), 72 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 587c013f04db..6cffdbba99d3 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -915,90 +915,121 @@ struct pci_acs {
 	u16 fw_ctrl;
 };
 
-static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
-			     const char *p, const u16 acs_mask, const u16 acs_flags)
+static bool pci_dev_match_disable_acs_redir(struct pci_dev *dev, const char *p)
 {
-	u16 valid_ctrl = dev->acs_capabilities & GENMASK_U16(6, 0);
-	u16 flags = acs_flags;
-	u16 mask = acs_mask;
-	char *delimit;
 	int ret = 0;
 
-	if (!p)
-		return;
-
 	while (*p) {
-		if (!acs_mask) {
-			/* Check for ACS flags */
-			delimit = strstr(p, "@");
-			if (delimit) {
-				int end;
-				u32 shift = 0;
-
-				end = delimit - p - 1;
-				mask = 0;
-				flags = 0;
-
-				while (end > -1) {
-					if (*(p + end) == '0') {
-						mask |= 1 << shift;
-						shift++;
-						end--;
-					} else if (*(p + end) == '1') {
-						mask |= 1 << shift;
-						flags |= 1 << shift;
-						shift++;
-						end--;
-					} else if ((*(p + end) == 'x') || (*(p + end) == 'X')) {
-						shift++;
-						end--;
-					} else {
-						pci_err(dev, "Invalid ACS flags... Ignoring\n");
-						return;
-					}
-				}
-				p = delimit + 1;
-			} else {
-				pci_err(dev, "ACS Flags missing\n");
-				return;
-			}
-		}
-
 		ret = pci_dev_str_match(dev, p, &p);
-		if (ret < 0) {
-			pr_warn_once("PCI: Can't parse ACS command line parameter\n");
-			break;
-		} else if (ret == 1) {
-			/* Found a match */
+		if (ret == 1) {
+			return true;
+		} else if (ret < 0) {
+			pr_warn_once("PCI: Can't parse disable_acs_redir param\n");
 			break;
 		}
 	}
 
-	if (ret != 1)
+	return false;
+}
+
+static void pci_param_disable_acs_redir(struct pci_dev *dev,
+					struct pci_acs *caps)
+{
+	const u16 acs_redir_mask = PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC;
+
+	if (!disable_acs_redir_param ||
+	    !pci_dev_match_disable_acs_redir(dev, disable_acs_redir_param) ||
+	    !pci_dev_specific_disable_acs_redir(dev))
 		return;
 
-	if (!pci_dev_specific_disable_acs_redir(dev))
-		return;
+	caps->ctrl = caps->fw_ctrl & ~acs_redir_mask;
+}
 
-	if (!acs_mask && (mask & ~valid_ctrl)) {
-		pci_dbg(dev, "Ignoring unsupported ACS bits: %#06x\n",
-			mask & ~valid_ctrl);
-		mask &= valid_ctrl;
-		flags &= valid_ctrl;
-	}
-
-	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);
+static const char *pci_dev_match_config_acs(struct pci_dev *dev, const char *p)
+{
+	const char *seg;
+	int ret;
 
 	/*
-	 * For mask bits that are 0, copy them from the firmware setting
-	 * and apply flags for all the mask bits that are 1.
+	 * Example: 11101010x@0000:01:00.0;101x@0000:02:00.0;1@0000:03:00.0
+	 * Three segments delimited via ';'. @seg always points to the start
+	 * of a segment and p advances to the start of the device id (BDF)
+	 * after '@'. Upon match, return the start of the matching segment.
 	 */
-	caps->ctrl = (caps->fw_ctrl & ~mask) | (flags & mask);
+	while (*p) {
+		seg = p;
+		p = strchr(seg, '@');
+		/* If malformed string, stop parsing. */
+		if (!p) {
+			pci_err(dev, "ACS Flags missing\n");
+			break;
+		}
 
-	pci_info(dev, "Configured ACS to %#06x\n", caps->ctrl);
+		p++;
+		/* pci_dev_str_match() updates p to the next segment. */
+		ret = pci_dev_str_match(dev, p, &p);
+		if (ret == 1)
+			return seg;
+
+		if (ret < 0) {
+			pr_warn_once("PCI: Can't parse config_acs param\n");
+			break;
+		}
+	}
+
+	return NULL;
+}
+
+static void pci_param_config_acs(struct pci_dev *dev, struct pci_acs *caps)
+{
+	u16 shift = 0, valid_ctrl = dev->acs_capabilities & GENMASK_U16(6, 0);
+	u16 invalid_bits, enabled_bits = 0, disabled_bits = 0;
+	const char *p, *seg;
+
+	if (!config_acs_param || !valid_ctrl)
+		return;
+
+	seg = pci_dev_match_config_acs(dev, config_acs_param);
+	if (!seg)
+		return;
+
+	p = strchr(seg, '@');
+	/* Parse bitstring backwards from '@' */
+	while (p > seg) {
+		if (shift >= 16) {
+			pci_err(dev, "ACS bitstring exceeds 16 bits\n");
+			return;
+		}
+		switch (*--p) {
+		case '1':
+			enabled_bits |= BIT(shift);
+			break;
+		case '0':
+			disabled_bits |= BIT(shift);
+			break;
+		case 'x':
+		case 'X':
+			/* Skip this bit */
+			break;
+		default:
+			pci_err(dev, "Invalid ACS flag character '%c'\n", *p);
+			return;
+		}
+		shift++;
+	}
+
+	invalid_bits = (enabled_bits | disabled_bits) & ~valid_ctrl;
+	if (invalid_bits) {
+		pci_dbg(dev, "Ignoring unsupported ACS bits: %#06x\n",
+			invalid_bits);
+		enabled_bits &= valid_ctrl;
+		disabled_bits &= valid_ctrl;
+	}
+
+	pci_dbg(dev, "ACS enabled: %#06x, disabled: %#06x\n",
+		enabled_bits, disabled_bits);
+
+	caps->ctrl = (caps->fw_ctrl | enabled_bits) & ~disabled_bits;
 }
 
 /**
@@ -1033,6 +1064,7 @@ void pci_enable_acs(struct pci_dev *dev)
 {
 	struct pci_acs caps;
 	bool enable_acs = false;
+	u16 kernel_default_ctrl;
 	int pos;
 
 	/* If an iommu is present we start with kernel default caps */
@@ -1051,14 +1083,18 @@ void pci_enable_acs(struct pci_dev *dev)
 	if (enable_acs)
 		pci_std_enable_acs(dev, &caps);
 
+	kernel_default_ctrl = caps.ctrl;
+
 	/*
 	 * Always apply caps from the command line, even if there is no iommu.
 	 * Trust that the admin has a reason to change the ACS settings.
 	 */
-	__pci_config_acs(dev, &caps, disable_acs_redir_param,
-			 PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC,
-			 ~(PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC));
-	__pci_config_acs(dev, &caps, config_acs_param, 0, 0);
+	pci_param_disable_acs_redir(dev, &caps);
+	pci_param_config_acs(dev, &caps);
+
+	if (caps.ctrl != kernel_default_ctrl)
+		pci_info(dev, "User configured ACS to %#06x (default: %#06x)\n",
+			 caps.ctrl, kernel_default_ctrl);
 
 	pci_write_config_word(dev, pos + PCI_ACS_CTRL, caps.ctrl);
 }
-- 
2.51.0

[PATCH v9 5/6] PCI: Enable the enhanced ACS controls introduced by PCI_ACS_ECAP

[Wei Wang]

The ACS Enhanced Capability introduces several new access controls to
improve device isolation. These new controls are particularly important
for device passthrough in virtualization scenarios.

For example, a DMA transaction from a device may target a guest physical
address that lies within the memory aperture of the switch's upstream
port, but not within any memory aperture or BAR space of a downstream
port. In such cases, the switch would generate an Unsupported Request (UR)
response to the device, which is undesirable. Enabling Unclaimed Request
Redirect Control ensures that these DMA requests are forwarded upstream
instead of being rejected.

The ACS DSP and USP Memory Target Access Control and ACS I/O Request
Blocking features similarly enhance device isolation. Device grouping in
Linux assumes that devices are properly isolated. Therefore, enable these
controls by default if PCI_ACS_ECAP is supported by the hardware. As with
other basic ACS access controls, these new controls can be configured via
the "config_acs=" boot parameter.

Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
---
 .../admin-guide/kernel-parameters.txt         | 23 +++++++++----
 drivers/pci/pci.c                             | 34 +++++++++++++++++--
 include/uapi/linux/pci_regs.h                 | 13 +++++++
 3 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a2563ff69f51..9019dac26b17 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5324,13 +5324,22 @@ Kernel parameters
 				flags.
 
 				ACS Flags is defined as follows:
-				  bit-0 : ACS Source Validation
-				  bit-1 : ACS Translation Blocking
-				  bit-2 : ACS P2P Request Redirect
-				  bit-3 : ACS P2P Completion Redirect
-				  bit-4 : ACS Upstream Forwarding
-				  bit-5 : ACS P2P Egress Control
-				  bit-6 : ACS Direct Translated P2P
+				  bit-0     : ACS Source Validation
+				  bit-1     : ACS Translation Blocking
+				  bit-2     : ACS P2P Request Redirect
+				  bit-3     : ACS P2P Completion Redirect
+				  bit-4     : ACS Upstream Forwarding
+				  bit-5     : ACS P2P Egress Control
+				  bit-6     : ACS Direct Translated P2P
+				  bit-7     : ACS I/O Request Blocking
+				  bit-9:8   : ACS DSP Memory Target Access Ctrl
+				      00    : Direct Request access enabled
+				      01    : Request blocking enabled
+				      10    : Request redirect enabled
+				      11    : Reserved
+				  bit-11:10 : ACS USP Memory Target Access Ctrl
+				              Same encoding as bit-9:8
+				  bit-12    : ACS Unclaimed Request Redirect Ctrl
 				Each bit can be marked as:
 				  '0' – force disabled
 				  '1' – force enabled
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 6cffdbba99d3..50a3b61f9898 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -983,9 +983,12 @@ static const char *pci_dev_match_config_acs(struct pci_dev *dev, const char *p)
 static void pci_param_config_acs(struct pci_dev *dev, struct pci_acs *caps)
 {
 	u16 shift = 0, valid_ctrl = dev->acs_capabilities & GENMASK_U16(6, 0);
-	u16 invalid_bits, enabled_bits = 0, disabled_bits = 0;
+	u16 new_ctrl, invalid_bits, enabled_bits = 0, disabled_bits = 0;
 	const char *p, *seg;
 
+	if (dev->acs_capabilities & PCI_ACS_ECAP)
+		valid_ctrl |= GENMASK_U16(12, 7);
+
 	if (!config_acs_param || !valid_ctrl)
 		return;
 
@@ -1026,10 +1029,23 @@ static void pci_param_config_acs(struct pci_dev *dev, struct pci_acs *caps)
 		disabled_bits &= valid_ctrl;
 	}
 
+	/*
+	 * DMAC/UMAC are 2-bit fields where encoding 0b11 is reserved. The
+	 * final value can land on 0b11 not only when the user enables both
+	 * bits, but also when they enable one bit, e.g. "1x", while fw_ctrl
+	 * already has the other set (e.g. "01").
+	 */
+	new_ctrl = (caps->fw_ctrl | enabled_bits) & ~disabled_bits;
+	if (FIELD_GET(PCI_ACS_DMAC_MASK, new_ctrl) == PCI_ACS_MAC_RSVD ||
+	    FIELD_GET(PCI_ACS_UMAC_MASK, new_ctrl) == PCI_ACS_MAC_RSVD) {
+		pci_err(dev, "ACS DMAC/UMAC would be set to reserved value\n");
+		return;
+	}
+
 	pci_dbg(dev, "ACS enabled: %#06x, disabled: %#06x\n",
 		enabled_bits, disabled_bits);
 
-	caps->ctrl = (caps->fw_ctrl | enabled_bits) & ~disabled_bits;
+	caps->ctrl = new_ctrl;
 }
 
 /**
@@ -1051,6 +1067,20 @@ static void pci_std_enable_acs(struct pci_dev *dev, struct pci_acs *caps)
 	/* Upstream Forwarding */
 	caps->ctrl |= (dev->acs_capabilities & PCI_ACS_UF);
 
+	/*
+	 * Enable Unclaimed Request Redirect Control, I/O Request Blocking,
+	 * and Downstream and Upstream Port Memory Target Access Redirect.
+	 */
+	if (dev->acs_capabilities & PCI_ACS_ECAP) {
+		caps->ctrl |= PCI_ACS_IB;
+		FIELD_MODIFY(PCI_ACS_DMAC_MASK, &caps->ctrl, PCI_ACS_MAC_RR);
+		if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM) {
+			caps->ctrl |= PCI_ACS_URRC;
+			FIELD_MODIFY(PCI_ACS_UMAC_MASK,
+				     &caps->ctrl, PCI_ACS_MAC_RR);
+		}
+	}
+
 	/* Enable Translation Blocking for external devices and noats */
 	if (pci_ats_disabled() || dev->external_facing || dev->untrusted)
 		caps->ctrl |= (dev->acs_capabilities & PCI_ACS_TB);
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index facaa324bd86..aeab8e5d54da 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -1016,6 +1016,7 @@
 
 /* Access Control Service */
 #define PCI_ACS_CAP		0x04	/* ACS Capability Register */
+#define  PCI_ACS_ECAP		0x0080  /* ACS Enhanced Capability (CAP reg) */
 #define  PCI_ACS_SV		0x0001	/* Source Validation */
 #define  PCI_ACS_TB		0x0002	/* Translation Blocking */
 #define  PCI_ACS_RR		0x0004	/* P2P Request Redirect */
@@ -1023,10 +1024,22 @@
 #define  PCI_ACS_UF		0x0010	/* Upstream Forwarding */
 #define  PCI_ACS_EC		0x0020	/* P2P Egress Control */
 #define  PCI_ACS_DT		0x0040	/* Direct Translated P2P */
+#define  PCI_ACS_IB		0x0080	/* I/O Request Blocking (CTRL reg) */
+#define  PCI_ACS_DMAC_MASK	0x0300  /* DSP Memory Target Access Control */
+#define  PCI_ACS_UMAC_MASK	0x0C00  /* USP Memory Target Access Control */
+#define  PCI_ACS_URRC		0x1000	/* Unclaimed Request Redirect Ctrl */
 #define PCI_ACS_EGRESS_BITS	0x05	/* ACS Egress Control Vector Size */
 #define PCI_ACS_CTRL		0x06	/* ACS Control Register */
 #define PCI_ACS_EGRESS_CTL_V	0x08	/* ACS Egress Control Vector */
 
+/* Encodings for DSP and USP Memory Target Access Control */
+enum {
+	PCI_ACS_MAC_DA   = 0x0,		/* Direct request access */
+	PCI_ACS_MAC_RB   = 0x1,		/* Request blocking */
+	PCI_ACS_MAC_RR   = 0x2,		/* Request redirect */
+	PCI_ACS_MAC_RSVD = 0x3,		/* Reserved */
+};
+
 /* SATA capability */
 #define PCI_SATA_REGS		4	/* SATA REGs specifier */
 #define  PCI_SATA_REGS_MASK	0xF	/* location - BAR#/inline */
-- 
2.51.0

[PATCH v9 6/6] PCI: Add the enhanced ACS controls check to pci_acs_flags_enabled()

[Wei Wang]

The enhanced ACS controls introduced by PCIe Gen 5 ensures better device
isolation. On devices that support the PCI_ACS_ECAP capability, the
controls are required to be enabled properly:
- ACS I/O Request Blocking needs to be enabled to avoid unintended
  upstream I/O requests.
- ACS DSP and USP Memory Target Access Control needs to be set with
  Request Redirect or Request Blocking to ensure the Downstream and
  Upstream Port memory resource ranges are not accessed by upstream
  memory requests.
- ACS Unclaimed Request Redirect needs to be enabled to ensure accesses to
  areas that lies within a Switch's Upstream Port memory apertures but not
  within any Downstream Port memory apertures get redirected.

To maintain compatibility with legacy devices that lack PCI_ACS_ECAP
support, pci_acs_enabled() skips checking for the capability.

Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/pci/pci.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 50a3b61f9898..b951195d5adc 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3648,6 +3648,57 @@ void pci_configure_ari(struct pci_dev *dev)
 	}
 }
 
+static bool pci_dev_has_memory_bars(struct pci_dev *pdev)
+{
+	int i;
+
+	for (i = 0; i <= PCI_ROM_RESOURCE; i++) {
+		if (pci_resource_flags(pdev, i) & IORESOURCE_MEM)
+			return true;
+	}
+
+	return false;
+}
+
+static bool pci_acs_ecap_enabled(struct pci_dev *pdev, u16 ctrl)
+{
+	struct pci_dev *upstream_pdev;
+
+	/*
+	 * For ACS DSP/USP Memory Target Access Control, either Request
+	 * Redirect or Request Blocking must be enabled to enforce isolation.
+	 * According to PCIe spec 7.0, the DSP Memory Target Access is
+	 * applicable to both Root Ports and Switch Downstream Ports that have
+	 * applicable Memory BAR space to protect. So if the device does not
+	 * have a Memory BAR, it skips the check.
+	 */
+	if (pci_dev_has_memory_bars(pdev) &&
+	    FIELD_GET(PCI_ACS_DMAC_MASK, ctrl) != PCI_ACS_MAC_RB &&
+	    FIELD_GET(PCI_ACS_DMAC_MASK, ctrl) != PCI_ACS_MAC_RR)
+		return false;
+
+	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_DOWNSTREAM) {
+		upstream_pdev = pci_upstream_bridge(pdev);
+
+		/*
+		 * The USP Memory Target Access is only applicable to
+		 * downstream ports that have applicable Memory BAR space in
+		 * the Switch Upstream Port to protect.
+		 */
+		if (upstream_pdev && pci_dev_has_memory_bars(upstream_pdev) &&
+		    FIELD_GET(PCI_ACS_UMAC_MASK, ctrl) != PCI_ACS_MAC_RB &&
+		    FIELD_GET(PCI_ACS_UMAC_MASK, ctrl) != PCI_ACS_MAC_RR)
+			return false;
+
+		/* PCI_ACS_URRC is applicable to Downstream Ports only. */
+		if (!(ctrl & PCI_ACS_URRC))
+			return false;
+	}
+
+	/* PCI_ACS_IB is applicable to both Root and Downstream Ports. */
+	return !!(ctrl & PCI_ACS_IB);
+}
+
 static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags)
 {
 	int pos;
@@ -3665,6 +3716,18 @@ static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags)
 	acs_flags &= (pdev->acs_capabilities | PCI_ACS_EC);
 
 	pci_read_config_word(pdev, pos + PCI_ACS_CTRL, &ctrl);
+
+	if (acs_flags & PCI_ACS_ECAP) {
+		if (!pci_acs_ecap_enabled(pdev, ctrl))
+			return false;
+		/*
+		 * The check for the required controls in PCI_ACS_ECAP has
+		 * passed. Clear the ECAP flag and continue to check the
+		 * basic ACS controls.
+		 */
+		acs_flags &= ~PCI_ACS_ECAP;
+	}
+
 	return (ctrl & acs_flags) == acs_flags;
 }
 
@@ -3723,6 +3786,8 @@ bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags)
 	 */
 	case PCI_EXP_TYPE_DOWNSTREAM:
 	case PCI_EXP_TYPE_ROOT_PORT:
+		/* PCI_ACS_ECAP applies to Root and Downstream Ports only */
+		acs_flags |= PCI_ACS_ECAP;
 		return pci_acs_flags_enabled(pdev, acs_flags);
 	/*
 	 * PCIe 3.0, 6.12.1.2 specifies ACS capabilities that should be
-- 
2.51.0

Re: [PATCH v9 1/6] PCI: Validate ACS control bits against device-specific ACS capabilities

[sashiko-bot]

> The ACS control validation used the kernel's full set of ACS control bits,
> which allowed users to request ACS features that the device does not
> support. Because hardware silently ignores these unsupported bits, users
> have no indication that their config_acs= request was partially
> ineffective.
> 
> Validate the requested ACS control bits against dev->acs_capabilities so
> that only device-supported ACS controls are accepted. Mask the capability
> with GENMASK_U16(6, 0) to select only the currently defined ACS control
> bits (0-6). Higher bits in the capability structure (e.g. the egress
> control vector size in bits 8-15) do not correspond to control bits in
> the ACS control register. Add a debug message to report which unsupported
> bits were ignored.
> 
> __pci_config_acs() is also called from disable_acs_redir_param(), which
> passes a hardcoded acs_mask (PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC) that
> does not need validation. Gate the new check on !acs_mask so it only
> fires for the config_acs_param path, which always invokes
> __pci_config_acs() with acs_mask == 0 and builds the actual mask from
> user input.
> 
> Also move the check after the device is matched, since the ctrl bits apply
> only to the matched device.
> 
> Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/SI2PR01MB4393F337A792CB3258C2E073DCB62@SI2PR01MB4393.apcprd01.prod.exchangelabs.com?part=1

Re: [PATCH v9 2/6] Documentation/kernel-parameters: Add multi-device config_acs example

[sashiko-bot]

> The config_acs parameter allows configuring ACS settings for multiple PCI
> devices by separating entries with semicolons. The current documentation
> only illustrates applying one configuration pattern to all devices, but
> does not show how to specify multiple devices with different ACS settings
> in a single parameter. Add an example demonstrating multi-device usage
> with distinct ACS configurations.
> 
> Some bootloaders interpret ';' as a command separator, which can cause
> the parameter to be split as multiple commands. Document that the entire
> parameter may need to be quoted to avoid bootloader parsing issues. This
> avoids confusing bootloader errors such as "can't find command <BDF>"
> when passing multiple device entries.
> 
> Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/SI2PR01MB4393F337A792CB3258C2E073DCB62@SI2PR01MB4393.apcprd01.prod.exchangelabs.com?part=2

Re: [PATCH v9 3/6] PCI: Consolidate delimiter handling into pci_dev_str_match()

[sashiko-bot]

> Callers of pci_dev_str_match() manually checked for the ';' or ','
> delimiter and advanced the parameter pointer past it. Move this common
> logic into pci_dev_str_match() so callers no longer need to duplicate it.
> 
> As a side effect of this consolidation, pci_dev_str_match() now enforces
> stricter boundary checks on successful matches. Previously, if a user
> provided a malformed parameter with garbage characters appended to a
> valid BDF (e.g., "pci=config_acs=110x@0000:04:00.0foo"), the callers
> would silently accept the prefix match and ignore the trailing garbage.
> Now, pci_dev_str_match() strictly verifies that the matched token is
> immediately followed by a valid delimiter (';', ',') or the null
> terminator ('\0'), returning -EINVAL if it encounters invalid trailing
> characters.
> 
> Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/SI2PR01MB4393F337A792CB3258C2E073DCB62@SI2PR01MB4393.apcprd01.prod.exchangelabs.com?part=3

Re: [PATCH v9 4/6] PCI: Refactor disable_acs_redir and config_acs param handling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but user-provided ACS command line parameters silently wipe out standard IOMMU protections (SV, TB, UF) for unspecified bits by reverting to hardware defaults.
--

commit 62951886e4f26a51d147451d4aa7384b487c2ff6
Author: Wei Wang <wei.w.wang@hotmail.com>

PCI: Refactor disable_acs_redir and config_acs param handling

This commit splits the intertwined disable_acs_redir and config_acs parameter
handling inside __pci_config_acs() into dedicated functions. It also improves
parameter parsing by checking for a matching device before parsing flags and
replacing complex string parsing with cleaner loops and switch statements.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 587c013f04db3..6cffdbba99d34 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c

[ ... ]

> +static void pci_param_disable_acs_redir(struct pci_dev *dev,
> +					struct pci_acs *caps)
> +{
> +	const u16 acs_redir_mask = PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC;
> +
> +	if (!disable_acs_redir_param ||
> +	    !pci_dev_match_disable_acs_redir(dev, disable_acs_redir_param) ||
> +	    !pci_dev_specific_disable_acs_redir(dev))
>  		return;
>  
> +	caps->ctrl = caps->fw_ctrl & ~acs_redir_mask;

[Severity: High]
This isn't a bug introduced by this patch, but does this overwrite silently
discard the standard IOMMU protections (SV, TB, UF) established earlier?

When an administrator uses pci=disable_acs_redir=..., pci_enable_acs()
first enables OS-level protections via pci_std_enable_acs(dev, &caps).

However, because this assignment bases the new control register completely on
caps->fw_ctrl, any previously configured security bits are erased. This breaks
PCIe isolation guarantees for the affected devices.

[ ... ]

> +static void pci_param_config_acs(struct pci_dev *dev, struct pci_acs *caps)
> +{

[ ... ]

> +	invalid_bits = (enabled_bits | disabled_bits) & ~valid_ctrl;
> +	if (invalid_bits) {
> +		pci_dbg(dev, "Ignoring unsupported ACS bits: %#06x\n",
> +			invalid_bits);
> +		enabled_bits &= valid_ctrl;
> +		disabled_bits &= valid_ctrl;
> +	}
>  
> -	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);
> +	pci_dbg(dev, "ACS enabled: %#06x, disabled: %#06x\n",
> +		enabled_bits, disabled_bits);
>  
> -	/*
> -	 * 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);
> +	caps->ctrl = (caps->fw_ctrl | enabled_bits) & ~disabled_bits;

[Severity: High]
This is a pre-existing issue, but does this assignment also inadvertently wipe
out the IOMMU protections set up by pci_std_enable_acs()?

Similar to pci_param_disable_acs_redir(), when pci=config_acs=... is used,
all un-masked bits revert to their fw_ctrl hardware default state rather than
persisting their pci_std_enable_acs() modifications. 

This could potentially allow DMA spoofing or unauthorized memory access on
devices where the admin only intended to tweak specific ACS bits.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/SI2PR01MB4393F337A792CB3258C2E073DCB62@SI2PR01MB4393.apcprd01.prod.exchangelabs.com?part=4

Re: [PATCH v9 5/6] PCI: Enable the enhanced ACS controls introduced by PCI_ACS_ECAP

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unconditional validation of DMAC and UMAC fields rejects valid user ACS configurations on non-ECAP devices and fails to account for kernel-corrected states.
- [Medium] ACS Enhanced Capability access controls are unconditionally applied to unsupported PCIe port types, writing to reserved bits.
--

commit e1302a2bae5cac8f4c54de9a2a416f24d9b8a517
Author: Wei Wang <wei.w.wang@hotmail.com>

PCI: Enable the enhanced ACS controls introduced by PCI_ACS_ECAP

The ACS Enhanced Capability introduces several new access controls to
improve device isolation. These new controls are particularly important
for device passthrough in virtualization scenarios.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 6cffdbba99d34..50a3b61f9898e 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -1026,10 +1029,23 @@ static void pci_param_config_acs(struct pci_dev *dev, struct pci_acs *caps)
>  		disabled_bits &= valid_ctrl;
>  	}
>  
> +	/*
> +	 * DMAC/UMAC are 2-bit fields where encoding 0b11 is reserved. The
> +	 * final value can land on 0b11 not only when the user enables both
> +	 * bits, but also when they enable one bit, e.g. "1x", while fw_ctrl
> +	 * already has the other set (e.g. "01").
> +	 */
> +	new_ctrl = (caps->fw_ctrl | enabled_bits) & ~disabled_bits;
> +	if (FIELD_GET(PCI_ACS_DMAC_MASK, new_ctrl) == PCI_ACS_MAC_RSVD ||
> +	    FIELD_GET(PCI_ACS_UMAC_MASK, new_ctrl) == PCI_ACS_MAC_RSVD) {
> +		pci_err(dev, "ACS DMAC/UMAC would be set to reserved value\n");
> +		return;
> +	}

[Severity: High]
Does this unconditional check against reserved DMAC/UMAC values reject valid
user configurations on non-ECAP devices?

If a user boots with the config_acs parameter to configure a non-ECAP device,
but its firmware leaves bits 8:9 or 10:11 as 0b11, this FIELD_GET check detects
PCI_ACS_MAC_RSVD and aborts the configuration, preventing the user from
applying basic ACS settings.

Additionally, for ECAP devices with buggy firmware defaults (0b11), building
the new control register using caps->fw_ctrl instead of the kernel-corrected
caps->ctrl revives the invalid firmware state, instantly triggering this
validation failure and rejecting unrelated user flags.

[ ... ]

> @@ -1051,6 +1067,20 @@ static void pci_std_enable_acs(struct pci_dev *dev, struct pci_acs *caps)
>  	/* Upstream Forwarding */
>  	caps->ctrl |= (dev->acs_capabilities & PCI_ACS_UF);
>  
> +	/*
> +	 * Enable Unclaimed Request Redirect Control, I/O Request Blocking,
> +	 * and Downstream and Upstream Port Memory Target Access Redirect.
> +	 */
> +	if (dev->acs_capabilities & PCI_ACS_ECAP) {
> +		caps->ctrl |= PCI_ACS_IB;
> +		FIELD_MODIFY(PCI_ACS_DMAC_MASK, &caps->ctrl, PCI_ACS_MAC_RR);

[Severity: Medium]
Can this logic unconditionally apply these access controls to unsupported
port types?

If the kernel enumerates a PCIe Endpoint or Upstream Port that has the
PCI_ACS_ECAP bit set in its ACS capabilities, this unconditionally modifies
PCI_ACS_DMAC_MASK and sets PCI_ACS_IB.

Because ECAP controls like DMAC only apply to Root Ports and Downstream Ports,
applying them to Endpoints or Upstream Ports writes to reserved bits, which
could risk hardware misbehavior.

> +		if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM) {
> +			caps->ctrl |= PCI_ACS_URRC;
> +			FIELD_MODIFY(PCI_ACS_UMAC_MASK,
> +				     &caps->ctrl, PCI_ACS_MAC_RR);
> +		}
> +	}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/SI2PR01MB4393F337A792CB3258C2E073DCB62@SI2PR01MB4393.apcprd01.prod.exchangelabs.com?part=5

Re: [PATCH v9 6/6] PCI: Add the enhanced ACS controls check to pci_acs_flags_enabled()

[sashiko-bot]

> The enhanced ACS controls introduced by PCIe Gen 5 ensures better device
> isolation. On devices that support the PCI_ACS_ECAP capability, the
> controls are required to be enabled properly:
> - ACS I/O Request Blocking needs to be enabled to avoid unintended
>   upstream I/O requests.
> - ACS DSP and USP Memory Target Access Control needs to be set with
>   Request Redirect or Request Blocking to ensure the Downstream and
>   Upstream Port memory resource ranges are not accessed by upstream
>   memory requests.
> - ACS Unclaimed Request Redirect needs to be enabled to ensure accesses to
>   areas that lies within a Switch's Upstream Port memory apertures but not
>   within any Downstream Port memory apertures get redirected.
> 
> To maintain compatibility with legacy devices that lack PCI_ACS_ECAP
> support, pci_acs_enabled() skips checking for the capability.
> 
> Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/SI2PR01MB4393F337A792CB3258C2E073DCB62@SI2PR01MB4393.apcprd01.prod.exchangelabs.com?part=6

Re: [PATCH v9 1/6] PCI: Validate ACS control bits against device-specific ACS capabilities

[Ilpo Järvinen]

On Thu, 3 Sep 2026, Wei Wang wrote:

> The ACS control validation used the kernel's full set of ACS control bits,
> which allowed users to request ACS features that the device does not
> support. Because hardware silently ignores these unsupported bits, users
> have no indication that their config_acs= request was partially
> ineffective.
> 
> Validate the requested ACS control bits against dev->acs_capabilities so
> that only device-supported ACS controls are accepted. Mask the capability
> with GENMASK_U16(6, 0) to select only the currently defined ACS control
> bits (0-6). Higher bits in the capability structure (e.g. the egress
> control vector size in bits 8-15) do not correspond to control bits in
> the ACS control register. Add a debug message to report which unsupported
> bits were ignored.
> 
> __pci_config_acs() is also called from disable_acs_redir_param(), which
> passes a hardcoded acs_mask (PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC) that
> does not need validation. Gate the new check on !acs_mask so it only
> fires for the config_acs_param path, which always invokes
> __pci_config_acs() with acs_mask == 0 and builds the actual mask from
> user input.
> 
> Also move the check after the device is matched, since the ctrl bits apply
> only to the matched device.
> 
> Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/pci/pci.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b2879a6be5f8..2af679111a9b 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -910,6 +910,7 @@ struct pci_acs {
>  static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
>  			     const char *p, const u16 acs_mask, const u16 acs_flags)
>  {
> +	u16 valid_ctrl = dev->acs_capabilities & GENMASK_U16(6, 0);

This looks a step backwards.

These bits are surely named so this mask should be a composite of those 
define names, either done here or through another define (likely the 
latter is better).

>  	u16 flags = acs_flags;
>  	u16 mask = acs_mask;
>  	char *delimit;
> @@ -955,12 +956,6 @@ static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
>  			}
>  		}
>  
> -		if (mask & ~(PCI_ACS_SV | PCI_ACS_TB | PCI_ACS_RR | PCI_ACS_CR |
> -			    PCI_ACS_UF | PCI_ACS_EC | PCI_ACS_DT)) {
> -			pci_err(dev, "Invalid ACS flags specified\n");
> -			return;
> -		}
> -
>  		ret = pci_dev_str_match(dev, p, &p);
>  		if (ret < 0) {
>  			pr_warn_once("PCI: Can't parse ACS command line parameter\n");
> @@ -983,6 +978,13 @@ static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
>  	if (!pci_dev_specific_disable_acs_redir(dev))
>  		return;
>  
> +	if (!acs_mask && (mask & ~valid_ctrl)) {
> +		pci_dbg(dev, "Ignoring unsupported ACS bits: %#06x\n",
> +			mask & ~valid_ctrl);
> +		mask &= valid_ctrl;
> +		flags &= valid_ctrl;
> +	}
> +
>  	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);
> 

-- 
 i.

Re: [PATCH v9 6/6] PCI: Add the enhanced ACS controls check to pci_acs_flags_enabled()

[Ilpo Järvinen]

On Thu, 3 Sep 2026, Wei Wang wrote:

> The enhanced ACS controls introduced by PCIe Gen 5 ensures better device
> isolation. On devices that support the PCI_ACS_ECAP capability, the
> controls are required to be enabled properly:
> - ACS I/O Request Blocking needs to be enabled to avoid unintended
>   upstream I/O requests.
> - ACS DSP and USP Memory Target Access Control needs to be set with
>   Request Redirect or Request Blocking to ensure the Downstream and
>   Upstream Port memory resource ranges are not accessed by upstream
>   memory requests.
> - ACS Unclaimed Request Redirect needs to be enabled to ensure accesses to
>   areas that lies within a Switch's Upstream Port memory apertures but not
>   within any Downstream Port memory apertures get redirected.
> 
> To maintain compatibility with legacy devices that lack PCI_ACS_ECAP
> support, pci_acs_enabled() skips checking for the capability.
> 
> Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/pci/pci.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 50a3b61f9898..b951195d5adc 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3648,6 +3648,57 @@ void pci_configure_ari(struct pci_dev *dev)
>  	}
>  }
>  
> +static bool pci_dev_has_memory_bars(struct pci_dev *pdev)
> +{
> +	int i;
> +
> +	for (i = 0; i <= PCI_ROM_RESOURCE; i++) {
> +		if (pci_resource_flags(pdev, i) & IORESOURCE_MEM)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +static bool pci_acs_ecap_enabled(struct pci_dev *pdev, u16 ctrl)
> +{
> +	struct pci_dev *upstream_pdev;
> +
> +	/*
> +	 * For ACS DSP/USP Memory Target Access Control, either Request
> +	 * Redirect or Request Blocking must be enabled to enforce isolation.
> +	 * According to PCIe spec 7.0, the DSP Memory Target Access is
> +	 * applicable to both Root Ports and Switch Downstream Ports that have
> +	 * applicable Memory BAR space to protect. So if the device does not
> +	 * have a Memory BAR, it skips the check.
> +	 */
> +	if (pci_dev_has_memory_bars(pdev) &&
> +	    FIELD_GET(PCI_ACS_DMAC_MASK, ctrl) != PCI_ACS_MAC_RB &&
> +	    FIELD_GET(PCI_ACS_DMAC_MASK, ctrl) != PCI_ACS_MAC_RR)
> +		return false;
> +
> +	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_DOWNSTREAM) {
> +		upstream_pdev = pci_upstream_bridge(pdev);
> +
> +		/*
> +		 * The USP Memory Target Access is only applicable to
> +		 * downstream ports that have applicable Memory BAR space in
> +		 * the Switch Upstream Port to protect.
> +		 */
> +		if (upstream_pdev && pci_dev_has_memory_bars(upstream_pdev) &&
> +		    FIELD_GET(PCI_ACS_UMAC_MASK, ctrl) != PCI_ACS_MAC_RB &&
> +		    FIELD_GET(PCI_ACS_UMAC_MASK, ctrl) != PCI_ACS_MAC_RR)
> +			return false;
> +
> +		/* PCI_ACS_URRC is applicable to Downstream Ports only. */
> +		if (!(ctrl & PCI_ACS_URRC))
> +			return false;
> +	}
> +
> +	/* PCI_ACS_IB is applicable to both Root and Downstream Ports. */
> +	return !!(ctrl & PCI_ACS_IB);

When the return type is bool, !!() construct is unnecessary.

> +}
> +
>  static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags)
>  {
>  	int pos;
> @@ -3665,6 +3716,18 @@ static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags)
>  	acs_flags &= (pdev->acs_capabilities | PCI_ACS_EC);
>  
>  	pci_read_config_word(pdev, pos + PCI_ACS_CTRL, &ctrl);
> +
> +	if (acs_flags & PCI_ACS_ECAP) {
> +		if (!pci_acs_ecap_enabled(pdev, ctrl))
> +			return false;
> +		/*
> +		 * The check for the required controls in PCI_ACS_ECAP has
> +		 * passed. Clear the ECAP flag and continue to check the
> +		 * basic ACS controls.
> +		 */
> +		acs_flags &= ~PCI_ACS_ECAP;
> +	}
> +
>  	return (ctrl & acs_flags) == acs_flags;
>  }
>  
> @@ -3723,6 +3786,8 @@ bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags)
>  	 */
>  	case PCI_EXP_TYPE_DOWNSTREAM:
>  	case PCI_EXP_TYPE_ROOT_PORT:
> +		/* PCI_ACS_ECAP applies to Root and Downstream Ports only */
> +		acs_flags |= PCI_ACS_ECAP;
>  		return pci_acs_flags_enabled(pdev, acs_flags);
>  	/*
>  	 * PCIe 3.0, 6.12.1.2 specifies ACS capabilities that should be
> 

-- 
 i.

Re: [PATCH v9 4/6] PCI: Refactor disable_acs_redir and config_acs param handling

[Wei Wang]

On 9/3/26 12:26 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] This problem wasn't introduced by this patch, but user-provided ACS command line parameters silently wipe out standard IOMMU protections (SV, TB, UF) for unspecified bits by reverting to hardware defaults.


I plan to keep the patch as is unless someone sees a concrete case that warrants
a behavior change.


> --
> 
> commit 62951886e4f26a51d147451d4aa7384b487c2ff6
> Author: Wei Wang <wei.w.wang@hotmail.com>
> 
> PCI: Refactor disable_acs_redir and config_acs param handling
> 
> This commit splits the intertwined disable_acs_redir and config_acs parameter
> handling inside __pci_config_acs() into dedicated functions. It also improves
> parameter parsing by checking for a matching device before parsing flags and
> replacing complex string parsing with cleaner loops and switch statements.
> 
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 587c013f04db3..6cffdbba99d34 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
> 
> [ ... ]
> 
>> +static void pci_param_disable_acs_redir(struct pci_dev *dev,
>> +					struct pci_acs *caps)
>> +{
>> +	const u16 acs_redir_mask = PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC;
>> +
>> +	if (!disable_acs_redir_param ||
>> +	    !pci_dev_match_disable_acs_redir(dev, disable_acs_redir_param) ||
>> +	    !pci_dev_specific_disable_acs_redir(dev))
>>   		return;
>>   
>> +	caps->ctrl = caps->fw_ctrl & ~acs_redir_mask;
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does this overwrite silently
> discard the standard IOMMU protections (SV, TB, UF) established earlier?
> 
> When an administrator uses pci=disable_acs_redir=..., pci_enable_acs()
> first enables OS-level protections via pci_std_enable_acs(dev, &caps).
> 
> However, because this assignment bases the new control register completely on
> caps->fw_ctrl, any previously configured security bits are erased. This breaks
> PCIe isolation guarantees for the affected devices.
> 

This mainly addresses how to handle the "x" bit.
kernel-parameters.txt mentions:
"
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 Validation unchanged from whatever power-up
or firmware set it to.
"

So the original intent was to leave the state as the firmware left it.

Regarding IOMMU grouping, the impact should be minimal since groups
are made later from the register as actually written (via
pci_acs_enabled()), so there is no false claim of isolation.


> [ ... ]
> 
>> +static void pci_param_config_acs(struct pci_dev *dev, struct pci_acs *caps)
>> +{
> 
> [ ... ]
> 
>> +	invalid_bits = (enabled_bits | disabled_bits) & ~valid_ctrl;
>> +	if (invalid_bits) {
>> +		pci_dbg(dev, "Ignoring unsupported ACS bits: %#06x\n",
>> +			invalid_bits);
>> +		enabled_bits &= valid_ctrl;
>> +		disabled_bits &= valid_ctrl;
>> +	}
>>   
>> -	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);
>> +	pci_dbg(dev, "ACS enabled: %#06x, disabled: %#06x\n",
>> +		enabled_bits, disabled_bits);
>>   
>> -	/*
>> -	 * 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);
>> +	caps->ctrl = (caps->fw_ctrl | enabled_bits) & ~disabled_bits;
> 
> [Severity: High]
> This is a pre-existing issue, but does this assignment also inadvertently wipe
> out the IOMMU protections set up by pci_std_enable_acs()?
> 
> Similar to pci_param_disable_acs_redir(), when pci=config_acs=... is used,
> all un-masked bits revert to their fw_ctrl hardware default state rather than
> persisting their pci_std_enable_acs() modifications.
> 
> This could potentially allow DMA spoofing or unauthorized memory access on
> devices where the admin only intended to tweak specific ACS bits.
> 

Similar to the above, RR/CR are already cleared by the admin's request, so
subsequent device grouping will work as expected.

Re: [PATCH v9 6/6] PCI: Add the enhanced ACS controls check to pci_acs_flags_enabled()

[Wei Wang]

On 9/3/26 7:09 PM, Ilpo Järvinen wrote:
> On Thu, 3 Sep 2026, Wei Wang wrote:
> 
>> The enhanced ACS controls introduced by PCIe Gen 5 ensures better device
>> isolation. On devices that support the PCI_ACS_ECAP capability, the
>> controls are required to be enabled properly:
>> - ACS I/O Request Blocking needs to be enabled to avoid unintended
>>    upstream I/O requests.
>> - ACS DSP and USP Memory Target Access Control needs to be set with
>>    Request Redirect or Request Blocking to ensure the Downstream and
>>    Upstream Port memory resource ranges are not accessed by upstream
>>    memory requests.
>> - ACS Unclaimed Request Redirect needs to be enabled to ensure accesses to
>>    areas that lies within a Switch's Upstream Port memory apertures but not
>>    within any Downstream Port memory apertures get redirected.
>>
>> To maintain compatibility with legacy devices that lack PCI_ACS_ECAP
>> support, pci_acs_enabled() skips checking for the capability.
>>
>> Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>
>> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
>> ---
>>   drivers/pci/pci.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 65 insertions(+)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 50a3b61f9898..b951195d5adc 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -3648,6 +3648,57 @@ void pci_configure_ari(struct pci_dev *dev)
>>   	}
>>   }
>>   
>> +static bool pci_dev_has_memory_bars(struct pci_dev *pdev)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i <= PCI_ROM_RESOURCE; i++) {
>> +		if (pci_resource_flags(pdev, i) & IORESOURCE_MEM)
>> +			return true;
>> +	}
>> +
>> +	return false;
>> +}
>> +
>> +static bool pci_acs_ecap_enabled(struct pci_dev *pdev, u16 ctrl)
>> +{
>> +	struct pci_dev *upstream_pdev;
>> +
>> +	/*
>> +	 * For ACS DSP/USP Memory Target Access Control, either Request
>> +	 * Redirect or Request Blocking must be enabled to enforce isolation.
>> +	 * According to PCIe spec 7.0, the DSP Memory Target Access is
>> +	 * applicable to both Root Ports and Switch Downstream Ports that have
>> +	 * applicable Memory BAR space to protect. So if the device does not
>> +	 * have a Memory BAR, it skips the check.
>> +	 */
>> +	if (pci_dev_has_memory_bars(pdev) &&
>> +	    FIELD_GET(PCI_ACS_DMAC_MASK, ctrl) != PCI_ACS_MAC_RB &&
>> +	    FIELD_GET(PCI_ACS_DMAC_MASK, ctrl) != PCI_ACS_MAC_RR)
>> +		return false;
>> +
>> +	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_DOWNSTREAM) {
>> +		upstream_pdev = pci_upstream_bridge(pdev);
>> +
>> +		/*
>> +		 * The USP Memory Target Access is only applicable to
>> +		 * downstream ports that have applicable Memory BAR space in
>> +		 * the Switch Upstream Port to protect.
>> +		 */
>> +		if (upstream_pdev && pci_dev_has_memory_bars(upstream_pdev) &&
>> +		    FIELD_GET(PCI_ACS_UMAC_MASK, ctrl) != PCI_ACS_MAC_RB &&
>> +		    FIELD_GET(PCI_ACS_UMAC_MASK, ctrl) != PCI_ACS_MAC_RR)
>> +			return false;
>> +
>> +		/* PCI_ACS_URRC is applicable to Downstream Ports only. */
>> +		if (!(ctrl & PCI_ACS_URRC))
>> +			return false;
>> +	}
>> +
>> +	/* PCI_ACS_IB is applicable to both Root and Downstream Ports. */
>> +	return !!(ctrl & PCI_ACS_IB);
> 
> When the return type is bool, !!() construct is unnecessary.

Good catch, thanks. The compiler already does the conversion for bool.

Re: [PATCH v9 1/6] PCI: Validate ACS control bits against device-specific ACS capabilities

[Wei Wang]

On 9/3/26 7:07 PM, Ilpo Järvinen wrote:
> On Thu, 3 Sep 2026, Wei Wang wrote:
> 
>> The ACS control validation used the kernel's full set of ACS control bits,
>> which allowed users to request ACS features that the device does not
>> support. Because hardware silently ignores these unsupported bits, users
>> have no indication that their config_acs= request was partially
>> ineffective.
>>
>> Validate the requested ACS control bits against dev->acs_capabilities so
>> that only device-supported ACS controls are accepted. Mask the capability
>> with GENMASK_U16(6, 0) to select only the currently defined ACS control
>> bits (0-6). Higher bits in the capability structure (e.g. the egress
>> control vector size in bits 8-15) do not correspond to control bits in
>> the ACS control register. Add a debug message to report which unsupported
>> bits were ignored.
>>
>> __pci_config_acs() is also called from disable_acs_redir_param(), which
>> passes a hardcoded acs_mask (PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC) that
>> does not need validation. Gate the new check on !acs_mask so it only
>> fires for the config_acs_param path, which always invokes
>> __pci_config_acs() with acs_mask == 0 and builds the actual mask from
>> user input.
>>
>> Also move the check after the device is matched, since the ctrl bits apply
>> only to the matched device.
>>
>> Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>
>> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
>> ---
>>   drivers/pci/pci.c | 14 ++++++++------
>>   1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index b2879a6be5f8..2af679111a9b 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -910,6 +910,7 @@ struct pci_acs {
>>   static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
>>   			     const char *p, const u16 acs_mask, const u16 acs_flags)
>>   {
>> +	u16 valid_ctrl = dev->acs_capabilities & GENMASK_U16(6, 0);
> 
> This looks a step backwards.
> 
> These bits are surely named so this mask should be a composite of those
> define names, either done here or through another define (likely the
> latter is better).

Yes, will use another define:

#define PCI_ACS_BASIC_CTRL_MASK \
	(PCI_ACS_SV | PCI_ACS_TB | PCI_ACS_RR | PCI_ACS_CR | \
	 PCI_ACS_UF | PCI_ACS_EC | PCI_ACS_DT)