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

[PATCH v7 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.

v6->v7 changes:
 - Rebased onto next-20260506 (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 and Dan Carpenter for reviewing the patchset.

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

Wei Wang (6):
  PCI: Validate ACS enable flags 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         |  32 +-
 drivers/pci/pci.c                             | 274 ++++++++++++------
 include/uapi/linux/pci_regs.h                 |  13 +
 3 files changed, 224 insertions(+), 95 deletions(-)


base-commit: 735d2f48cadaa9a87e7c7601667878de70c771c5
-- 
2.51.0

[PATCH v7 1/6] PCI: Validate ACS enable flags against device-specific ACS capabilities

[Wei Wang]

The ACS flag validation used the kernel's full set of ACS control bits,
which allowed users to request ACS features that the device does not
support. These unsupported bits would be silently ignored by hardware.
Validate the requested enable flags against dev->acs_capabilities so
that only device-supported ACS bits are accepted. Accordingly, move the
check after the device is matched, since the enable bits apply only to
the matched device.

Also change the validation to apply only to bits being enabled, since
attempting to disable unsupported features does not cause functional
issues.

Finally, improve the error message to report which invalid bits were
requested.

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

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 783e71962cc6..e017200694d8 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -931,12 +931,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");
@@ -959,6 +953,12 @@ static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
 	if (!pci_dev_specific_disable_acs_redir(dev))
 		return;
 
+	if (flags & ~dev->acs_capabilities) {
+		pci_err(dev, "Invalid ACS enable flags specified: %#06x\n",
+			(u16)(flags & ~dev->acs_capabilities));
+		return;
+	}
+
 	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 v7 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 | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 170aae226e2e..0c159ab59e14 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5277,15 +5277,20 @@ Kernel parameters
 				  '1' – force enabled
 				  'x' – unchanged
 				For example,
-				  pci=config_acs=10x@pci:0:0
+				- 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.
-
 				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 v7 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.

When no delimiter is found (end of parameter list or malformed entry),
pci_dev_str_match() now returns -ENODEV to indicate that no further entries
can be parsed, preserving the previous caller behavior of stopping parsing.

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

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index e017200694d8..ebd1e81264d2 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -334,7 +334,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:
@@ -374,8 +374,10 @@ static int pci_dev_str_match(struct pci_dev *dev, const char *p,
 			     &subsystem_vendor, &subsystem_device, &count);
 		if (ret != 4) {
 			ret = sscanf(p, "%hx:%hx%n", &vendor, &device, &count);
-			if (ret != 2)
-				return -EINVAL;
+			if (ret != 2) {
+				ret = -EINVAL;
+				goto not_found;
+			}
 
 			subsystem_vendor = 0;
 			subsystem_device = 0;
@@ -390,6 +392,9 @@ static int pci_dev_str_match(struct pci_dev *dev, const char *p,
 		    (!subsystem_device ||
 			    subsystem_device == dev->subsystem_device))
 			goto found;
+
+		/* No matching string found */
+		ret = 0;
 	} else {
 		/*
 		 * PCI Bus, Device, Function IDs are specified
@@ -397,16 +402,28 @@ static int pci_dev_str_match(struct pci_dev *dev, const char *p,
 		 */
 		ret = pci_dev_str_match_path(dev, p, &p);
 		if (ret < 0)
-			return ret;
+			goto not_found;
 		else if (ret)
 			goto found;
 	}
 
-	*endptr = p;
-	return 0;
+not_found:
+	if (ret < 0)
+		pr_err("PCI: Can't parse parameter: %s\n", p);
+
+	if (*p != ';' && *p != ',') {
+		/*
+		 * End of param or invalid format. Return -ENODEV so the caller
+		 * stops parsing.
+		 */
+		return -ENODEV;
+	}
+
+	*endptr = p + 1;
+	return ret;
 
 found:
-	*endptr = p;
+	*endptr = *p == '\0' ? p : p + 1;
 	return 1;
 }
 
@@ -933,18 +950,11 @@ static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
 
 		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 */
 			break;
 		}
-
-		if (*p != ';' && *p != ',') {
-			/* End of param or invalid format */
-			break;
-		}
-		p++;
 	}
 
 	if (ret != 1)
@@ -6387,16 +6397,8 @@ static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
 			align = 1ULL << align_order;
 			break;
 		} else if (ret < 0) {
-			pr_err("PCI: Can't parse resource_alignment parameter: %s\n",
-			       p);
 			break;
 		}
-
-		if (*p != ';' && *p != ',') {
-			/* End of param or invalid format */
-			break;
-		}
-		p++;
 	}
 out:
 	spin_unlock(&resource_alignment_lock);
-- 
2.51.0

[PATCH v7 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.
  - Bounds checking (max_shift) prevents overrunning the device’s ACS
    capability bits.
  - Variable names (enabled_bits, disabled_bits) more clearly express
    their roles compared to the previous mask/flags pair.
  - Error messages are simplified to reduce dmesg noise.

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 | 168 +++++++++++++++++++++++++++-------------------
 1 file changed, 98 insertions(+), 70 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index ebd1e81264d2..54f702ed4c96 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -900,87 +900,117 @@ 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 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) {
+		if (ret == 1)
+			return true;
+		else if (ret < 0)
 			break;
-		} else if (ret == 1) {
-			/* Found a match */
-			break;
-		}
 	}
 
-	if (ret != 1)
+	return false;
+}
+
+static void pci_param_disable_acs_redir(struct pci_dev *dev,
+					struct pci_acs *caps)
+{
+	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 (flags & ~dev->acs_capabilities) {
-		pci_err(dev, "Invalid ACS enable flags specified: %#06x\n",
-			(u16)(flags & ~dev->acs_capabilities));
-		return;
-	}
-
-	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 || p == seg)
+			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;
+		else if (ret < 0)
+			break;
+	}
+
+	return NULL;
+}
+
+static void pci_param_config_acs(struct pci_dev *dev, struct pci_acs *caps)
+{
+	u16 shift = 0, max_shift = fls(dev->acs_capabilities) - 1;
+	u16 enabled_bits = 0, disabled_bits = 0;
+	const char *p, *seg;
+
+	if (!config_acs_param)
+		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 > max_shift) {
+			pci_err(dev, "ACS flag bit %d exceed range %d\n",
+				shift, max_shift);
+			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++;
+	}
+
+	if (enabled_bits & ~dev->acs_capabilities) {
+		pci_err(dev, "Invalid ACS enable flags specified: %#06x\n",
+			(u16)(enabled_bits & ~dev->acs_capabilities));
+		return;
+	}
+
+	pci_dbg(dev, "ACS enabled: %#06x, disabled: %#06x\n",
+		enabled_bits, disabled_bits);
+
+	caps->ctrl = (caps->fw_ctrl | enabled_bits) & ~disabled_bits;
+
+	pci_info(dev, "Configured ACS to %#06x (FW default: %#06x)\n",
+		 caps->ctrl, caps->fw_ctrl);
 }
 
 /**
@@ -1037,10 +1067,8 @@ void pci_enable_acs(struct pci_dev *dev)
 	 * 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);
 
 	pci_write_config_word(dev, pos + PCI_ACS_CTRL, caps.ctrl);
 }
-- 
2.51.0

[PATCH v7 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                             | 10 ++++++++
 include/uapi/linux/pci_regs.h                 | 13 +++++++++++
 3 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 0c159ab59e14..32e82859089c 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5265,13 +5265,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 54f702ed4c96..b0c15de0aedb 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1032,6 +1032,16 @@ static void pci_std_enable_acs(struct pci_dev *dev, struct pci_acs *caps)
 	/* Upstream Forwarding */
 	caps->ctrl |= (dev->acs_capabilities & PCI_ACS_UF);
 
+	/*
+	 * Redirect 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_URRC | PCI_ACS_IB;
+		FIELD_MODIFY(PCI_ACS_DMAC_MASK, &caps->ctrl, PCI_ACS_MAC_RR);
+		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 14f634ab9350..b8e188eb06db 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 */
 #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 */
+#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 v7 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 | 62 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b0c15de0aedb..b684ae4e92b5 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3551,6 +3551,53 @@ 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)
+{
+	/*
+	 * 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) {
+		/*
+		 * 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 (pci_dev_has_memory_bars(pci_upstream_bridge(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;
@@ -3568,6 +3615,19 @@ 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 &&
+	    pdev->acs_capabilities & 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;
 }
 
@@ -3626,6 +3686,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 v7 3/6] PCI: Consolidate delimiter handling into pci_dev_str_match()

[Wei Wang]

On 5/6/26 10:10 PM, Wei Wang wrote:
> 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.
> 
> When no delimiter is found (end of parameter list or malformed entry),
> pci_dev_str_match() now returns -ENODEV to indicate that no further entries
> can be parsed, preserving the previous caller behavior of stopping parsing.
> 
> Signed-off-by: Wei Wang <wei.w.wang@hotmail.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>   drivers/pci/pci.c | 46 ++++++++++++++++++++++++----------------------
>   1 file changed, 24 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index e017200694d8..ebd1e81264d2 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -334,7 +334,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:
> @@ -374,8 +374,10 @@ static int pci_dev_str_match(struct pci_dev *dev, const char *p,
>   			     &subsystem_vendor, &subsystem_device, &count);
>   		if (ret != 4) {
>   			ret = sscanf(p, "%hx:%hx%n", &vendor, &device, &count);
> -			if (ret != 2)
> -				return -EINVAL;
> +			if (ret != 2) {
> +				ret = -EINVAL;
> +				goto not_found;
> +			}
>   
>   			subsystem_vendor = 0;
>   			subsystem_device = 0;
> @@ -390,6 +392,9 @@ static int pci_dev_str_match(struct pci_dev *dev, const char *p,
>   		    (!subsystem_device ||
>   			    subsystem_device == dev->subsystem_device))
>   			goto found;
> +
> +		/* No matching string found */
> +		ret = 0;
>   	} else {
>   		/*
>   		 * PCI Bus, Device, Function IDs are specified
> @@ -397,16 +402,28 @@ static int pci_dev_str_match(struct pci_dev *dev, const char *p,
>   		 */
>   		ret = pci_dev_str_match_path(dev, p, &p);
>   		if (ret < 0)
> -			return ret;
> +			goto not_found;
>   		else if (ret)
>   			goto found;
>   	}
>   
> -	*endptr = p;
> -	return 0;
> +not_found:
> +	if (ret < 0)
> +		pr_err("PCI: Can't parse parameter: %s\n", p);

Hi Krzysztof,

Apologies, I just saw your feedback here:
https://lore.kernel.org/all/20260504052005.GA2312220@rocinante/#t

For some reason, it bypassed my inbox before I sent out v7.
I'll review your comments and we can continue the discussion on
that thread.

Thanks!

> +
> +	if (*p != ';' && *p != ',') {
> +		/*
> +		 * End of param or invalid format. Return -ENODEV so the caller
> +		 * stops parsing.
> +		 */
> +		return -ENODEV;
> +	}
> +
> +	*endptr = p + 1;
> +	return ret;
>   
>   found:
> -	*endptr = p;
> +	*endptr = *p == '\0' ? p : p + 1;
>   	return 1;
>   }
>   
> @@ -933,18 +950,11 @@ static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
>   
>   		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 */
>   			break;
>   		}
> -
> -		if (*p != ';' && *p != ',') {
> -			/* End of param or invalid format */
> -			break;
> -		}
> -		p++;
>   	}
>   
>   	if (ret != 1)
> @@ -6387,16 +6397,8 @@ static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
>   			align = 1ULL << align_order;
>   			break;
>   		} else if (ret < 0) {
> -			pr_err("PCI: Can't parse resource_alignment parameter: %s\n",
> -			       p);
>   			break;
>   		}
> -
> -		if (*p != ';' && *p != ',') {
> -			/* End of param or invalid format */
> -			break;
> -		}
> -		p++;
>   	}
>   out:
>   	spin_unlock(&resource_alignment_lock);

Re: [PATCH v7 1/6] PCI: Validate ACS enable flags against device-specific ACS capabilities

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The new validation logic unconditionally fails when parsing the `disable_acs_redir` parameter, completely breaking the ability to disable ACS redirection.
- [High] The validation logic incorrectly treats the 'Egress Control Vector Size' field as capability flags, breaking the configuration of Enhanced ACS features and allowing reserved bit corruption.
- [Medium] The complete removal of the `mask` bounds validation allows malformed user strings to clear reserved bits in the ACS Control Register.
--

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

PCI: Validate ACS enable flags against device-specific ACS capabilities

The ACS flag validation is updated to use dev->acs_capabilities instead
of the full set of ACS control bits, ensuring only supported ACS features
are enabled. Note that a regression regarding the disable_acs_redir
parameter was fixed later in the patch series by commit 2b4c7af1c4a60
(PCI: Refactor disable_acs_redir and config_acs param handling).

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 783e71962cc61..e017200694d8a 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -931,12 +931,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;
> -		}
> -

Does the removal of the mask validation allow malformed user strings to
clear reserved bits in the ACS Control Register?

If a user specifies 0 in the command line for an upper bit position, that
bit is set in mask but remains clear in flags. Because flags is 0 at that
position, the new validation checks will trivially pass.

The function later evaluates:

caps->ctrl = (caps->fw_ctrl & ~mask) | (flags & mask);

which would unconditionally clear the corresponding bits in the device's ACS
Control register, potentially violating the PCIe specification that dictates
reserved bits must be preserved or read-only.

>  		ret = pci_dev_str_match(dev, p, &p);
>  		if (ret < 0) {
[ ... ]
> @@ -959,6 +953,12 @@ static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
>  	if (!pci_dev_specific_disable_acs_redir(dev))
>  		return;
>  
> +	if (flags & ~dev->acs_capabilities) {
> +		pci_err(dev, "Invalid ACS enable flags specified: %#06x\n",
> +			(u16)(flags & ~dev->acs_capabilities));
> +		return;
> +	}
> +

Will this validation unconditionally fail when parsing the disable_acs_redir
parameter, breaking the ability to disable ACS redirection?

When the pci=disable_acs_redir parameter is used, __pci_config_acs() is
called with an inverted mask for flags, making acs_flags evaluate to 0xFFD3
as a 16-bit unsigned integer.

Since flags has its upper bits set, and ~dev->acs_capabilities also has its
upper bits set, the bitwise AND will result in a non-zero value. This seems
like it will instantly log an invalid flags error and return early, silently
failing to disable ACS redirection.

Could treating the Egress Control Vector Size field as capability flags break
the configuration of Enhanced ACS features?

dev->acs_capabilities is populated by reading the 16-bit PCI_ACS_CAP register,
where bits 8-15 contain the Egress Control Vector Size integer. Validating
against the full 16-bit value incorrectly uses the vector size as a bitmask
of valid capabilities.

If a user provides a parameter to enable Enhanced ACS controls, the validation
might erroneously reject them unless the device's vector size happens to
have those exact bits set. Conversely, if a device has a non-zero egress
vector size, a user could pass flags corresponding to the set bits in the
vector size, allowing unintended modification of reserved bits in
PCI_ACS_CTRL.

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The documentation incorrectly advises using single quotes around the `pci=config_acs=...` parameter, which the kernel's command-line parser does not strip, potentially causing the parameter to be silently ignored.
--

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

Documentation/kernel-parameters: Add multi-device config_acs example

This commit adds an example to the kernel parameters documentation for
configuring ACS settings for multiple PCI devices. It also adds a note
recommending quoting the parameter to prevent bootloaders from misinterpreting
the semicolon separator.

> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
[ ... ]
>  				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'

Will the kernel's command-line parser correctly handle single quotes here?

Looking at next_arg() in lib/cmdline.c, the kernel explicitly detects and
strips double quotes ("), but it treats single quotes (') as literal text
characters.

If a bootloader passes the quoted string verbatim to the kernel, the parameter
will be extracted as 'pci rather than pci. This will fail the early_param()
check for "pci", causing the ACS configuration to be silently ignored.

Should the documentation recommend using double quotes instead?
(e.g., "- "pci=config_acs=..."")

> +				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.
> +

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] When path-based parsing fails, the error message logs the remaining parameters (or an empty string) instead of the malformed parameter.
- [Medium] Replacing a rate-limited print with an unconditional `pr_err()` causes severe boot log spam for malformed parameters.
--

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

PCI: Consolidate delimiter handling into pci_dev_str_match()

This commit moves the ';' or ',' delimiter checking logic directly into
pci_dev_str_match(), simplifying caller code. It also updates the function
to return -ENODEV when parsing reaches the end of the list or encounters an
invalid format.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index e017200694d8a..ebd1e81264d21 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
[ ... ]
> @@ -397,16 +402,28 @@ static int pci_dev_str_match(struct pci_dev *dev, const char *p,
>  		 */
>  		ret = pci_dev_str_match_path(dev, p, &p);
>  		if (ret < 0)
> -			return ret;
> +			goto not_found;

In pci_dev_str_match_path(), the pointer p is advanced immediately:

static int pci_dev_str_match_path(struct pci_dev *dev, const char *path,
				  const char **endptr)
{
...
	*endptr = strchrnul(path, ';');

Since pci_dev_str_match() calls pci_dev_str_match_path(dev, p, &p), the local
pointer p is updated to the end of the parameter before returning an error.

When we jump to not_found, does this mean the pr_err() will print the
remaining parameters (or an empty string) instead of the malformed parameter?

>  		else if (ret)
>  			goto found;
>  	}
>  
> -	*endptr = p;
> -	return 0;
> +not_found:
> +	if (ret < 0)
> +		pr_err("PCI: Can't parse parameter: %s\n", p);

Because pci_dev_str_match() is evaluated for every PCI device during boot
enumeration (for example, through __pci_config_acs()), it is called
repeatedly.

Could this unconditional pr_err() cause severe boot log spam if a malformed
parameter is passed on the kernel command line?

The previous code in __pci_config_acs() used a rate-limited pr_warn_once(),
which this commit removed:

[ ... ]
> @@ -933,18 +950,11 @@ static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
>  
>  		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) {
[ ... ]

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Bounds checking in pci_param_config_acs() breaks backwards compatibility for padded parameters and contains an integer underflow that leads to Undefined Behavior.
- [Medium] Silenced logging for disable_acs_redir reduces configuration observability.
--

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

PCI: Refactor disable_acs_redir and config_acs param handling

This commit refactors the ACS parameter handling by splitting the old
__pci_config_acs function into dedicated functions for disable_acs_redir
and config_acs parameters.

> --- 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)
> +{
> +	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;
> +}

The previous implementation printed a pci_info message when ACS control was
modified. Is it intentional that pci_param_disable_acs_redir no longer logs
confirmation that the redirect was disabled?

This might make debugging device isolation issues more difficult without the
dmesg confirmation.

[ ... ]

> +static void pci_param_config_acs(struct pci_dev *dev, struct pci_acs *caps)
> +{
> +	u16 shift = 0, max_shift = fls(dev->acs_capabilities) - 1;

If dev->acs_capabilities is 0, fls(0) evaluates to 0. Will 0 - 1 underflow
the u16 max_shift variable to 65535, effectively bypassing the bounds check
below?

> +	u16 enabled_bits = 0, disabled_bits = 0;
> +	const char *p, *seg;

[ ... ]

> +	p = strchr(seg, '@');
> +	/* Parse bitstring backwards from '@' */
> +	while (p > seg) {
> +		if (shift > max_shift) {
> +			pci_err(dev, "ACS flag bit %d exceed range %d\n",
> +				shift, max_shift);
> +			return;
> +		}

The previous implementation gracefully allowed parameter padding that exceeded
the capability mask, only erroring if unsupported bits were actively enabled.

Does aborting here when shift exceeds max_shift break backwards compatibility
for existing working kernel boot parameters (like config_acs=00000000@...)
on devices with fewer capabilities?

> +
> +		switch (*--p) {
> +		case '1':
> +			enabled_bits |= BIT(shift);
> +			break;

If the bounds check is bypassed due to the max_shift underflow mentioned
above, could shift increment without bounds for long user strings?

If shift exceeds the width of an unsigned long, BIT(shift) could trigger
undefined behavior. Additionally, since enabled_bits is a u16, shift amounts
of 16 or greater will silently truncate.

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `config_acs=` boot parameter parser incorrectly validates the newly added ACS Enhanced Capability controls (bits 8-12), unconditionally rejecting valid configurations and bypassing administrator-defined isolation policies.
--

commit 7640f1b4b51e699ac56a5798f5e8dbb9114e4b25
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/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 0c159ab59e14b..32e82859089c8 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5265,13 +5265,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

Does this configuration introduce a regression where valid inputs for bits 8
through 12 are unconditionally rejected?

Looking at the parser in drivers/pci/pci.c:pci_param_config_acs(), it validates
the requested bits against the hardware capabilities:

	u16 shift = 0, max_shift = fls(dev->acs_capabilities) - 1;
	...
		if (shift > max_shift) {
			pci_err(dev, "ACS flag bit %d exceed range %d\n",
				shift, max_shift);
			return;
		}
	...
	if (enabled_bits & ~dev->acs_capabilities) {
		pci_err(dev, "Invalid ACS enable flags specified: %#06x\n",
			(u16)(enabled_bits & ~dev->acs_capabilities));
		return;
	}

Since bits 8 through 15 of the ACS Capability Register represent the Egress
Control Vector Size rather than capability flags, validating bits 8-12 against
dev->acs_capabilities appears to be incorrect.

If the Egress Control Vector Size is 0, max_shift will be 7, and the parser
will reject any attempt to configure bits 8 through 12. If it is non-zero, the
bitwise validation will fail unpredictably depending on the size value.

Will this cause the requested administrator-defined isolation policies to be
bypassed?

[ ... ]

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unchecked NULL pointer dereference in `pci_dev_has_memory_bars()` when evaluating the upstream bridge of a Downstream Port.
--

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

PCI: Add the enhanced ACS controls check to pci_acs_flags_enabled()

This commit adds enhanced ACS controls checks introduced by PCIe Gen 5 to
ensure better device isolation. It verifies specific ACS controls on devices
that support the PCI_ACS_ECAP capability.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b0c15de0aedb2..b684ae4e92b5e 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c

[ ... ]

> @@ -3551,6 +3551,53 @@ 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)
> +{
> +	/*
> +	 * 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) {
> +		/*
> +		 * 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 (pci_dev_has_memory_bars(pci_upstream_bridge(pdev)) &&

Could pci_upstream_bridge() return NULL here?

If the device resides on a root bus, which can sometimes occur in certain
virtualized environments or hardware topologies, pci_upstream_bridge() will
return NULL.

If that happens, would pci_dev_has_memory_bars() dereference the NULL
pointer when evaluating pci_resource_flags()?

> +		    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;
> +	}

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

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

[Randy Dunlap]

Hi,

On 5/6/26 7:10 AM, Wei Wang wrote:
> 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 | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 170aae226e2e..0c159ab59e14 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5277,15 +5277,20 @@ Kernel parameters
>  				  '1' – force enabled
>  				  'x' – unchanged
>  				For example,
> -				  pci=config_acs=10x@pci:0:0
> +				- pci=config_acs=10x@pci:0:0

Are these added hyphens/dashes helpful?
Are they needed for some reason?
I don't see any problem here without the leading '-'.
They (here and below) seem like they could add some confusion IMO.

>  				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.
> -
>  				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

-- 
~Randy

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

[Wei Wang]

On 5/7/26 6:06 AM, Randy Dunlap wrote:
> Hi,
> 
> On 5/6/26 7:10 AM, Wei Wang wrote:
>> 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 | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 170aae226e2e..0c159ab59e14 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -5277,15 +5277,20 @@ Kernel parameters
>>   				  '1' – force enabled
>>   				  'x' – unchanged
>>   				For example,
>> -				  pci=config_acs=10x@pci:0:0
>> +				- pci=config_acs=10x@pci:0:0
> 
> Are these added hyphens/dashes helpful?
> Are they needed for some reason?
> I don't see any problem here without the leading '-'.
> They (here and below) seem like they could add some confusion IMO.

My intention was to use the hyphens as bullet points to separate the
examples. Yes, maybe they could cause copy-paste confusion for users.
I will drop them. Thanks.


> 
>>   				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.
>> -
>>   				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
>