Re: [PATCH V4] PCI: Extend ACS configurability

[Bjorn Helgaas <helgaas@kernel.org>]

On Tue, Jun 25, 2024 at 09:01:50PM +0530, Vidya Sagar wrote:
> PCIe ACS settings control the level of isolation and the possible P2P
> paths between devices. With greater isolation the kernel will create
> smaller iommu_groups and with less isolation there is more HW that
> can achieve P2P transfers. From a virtualization perspective all
> devices in the same iommu_group must be assigned to the same VM as
> they lack security isolation.
> 
> There is no way for the kernel to automatically know the correct
> ACS settings for any given system and workload. Existing command line
> options (ex:- disable_acs_redir) allow only for large scale change,
> disabling all isolation, but this is not sufficient for more complex cases.
> 
> Add a kernel command-line option 'config_acs' to directly control all the
> ACS bits for specific devices, which allows the operator to setup the
> right level of isolation to achieve the desired P2P configuration.
> The definition is future proof, when new ACS bits are added to the spec
> the open syntax can be extended.
> 
> ACS needs to be setup early in the kernel boot as the ACS settings
> effect how iommu_groups are formed. iommu_group formation is a one
> time event during initial device discovery, changing ACS bits after
> kernel boot can result in an inaccurate view of the iommu_groups
> compared to the current isolation configuration.
> 
> ACS applies to PCIe Downstream Ports and multi-function devices.
> The default ACS settings are strict and deny any direct traffic
> between two functions. This results in the smallest iommu_group the
> HW can support. Frequently these values result in slow or
> non-working P2PDMA.
> 
> ACS offers a range of security choices controlling how traffic is
> allowed to go directly between two devices. Some popular choices:
>   - Full prevention
>   - Translated requests can be direct, with various options
>   - Asymmetric direct traffic, A can reach B but not the reverse
>   - All traffic can be direct
> Along with some other less common ones for special topologies.
> 
> The intention is that this option would be used with expert knowledge
> of the HW capability and workload to achieve the desired
> configuration.
> 
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>

Applied with the tweaks below to pci/acs for v6.11, thanks!

I added an example to the doc; please check it to see if I interpreted
the doc correctly.

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 42d0f6fd40d0..b2057241ea6c 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4621,24 +4621,34 @@
 				may put more devices in an IOMMU group.
 		config_acs=
 				Format:
-				=<ACS flags>@<pci_dev>[; ...]
+				<ACS flags>@<pci_dev>[; ...]
 				Specify one or more PCI devices (in the format
 				specified above) optionally prepended with flags
 				and separated by semicolons. The respective
-				capabilities will be enabled, disabled or unchanged
-				based on what is specified in flags.
-				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
-				Each bit can be marked as
-				‘0‘ – force disabled
-				‘1’ – force enabled
-				‘x’ – unchanged.
+				capabilities will be enabled, disabled or
+				unchanged based on what is specified in
+				flags.
+
+				ACS Flags is defined as follows:
+				  bit-0 : ACS Source Validation
+				  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
+				Each bit can be marked as:
+				  '0' – force disabled
+				  '1' – force enabled
+				  'x' – unchanged
+				For example,
+				  pci=config_acs=10x
+				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.
 		force_floating	[S390] Force usage of floating interrupts.
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 1afe650ce338..45d93101a08b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1006,7 +1006,7 @@ 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_info_once("PCI: Can't parse acs command line parameter\n");
+			pr_info_once("PCI: Can't parse ACS command line parameter\n");
 			break;
 		} else if (ret == 1) {
 			/* Found a match */
@@ -1026,14 +1026,14 @@ static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
 	if (!pci_dev_specific_disable_acs_redir(dev))
 		return;
 
-	pci_dbg(dev, "ACS mask  = 0x%X\n", mask);
-	pci_dbg(dev, "ACS flags = 0x%X\n", flags);
+	pci_dbg(dev, "ACS mask  = %#06x\n", mask);
+	pci_dbg(dev, "ACS flags = %#06x\n", flags);
 
 	/* If mask is 0 then we copy the bit from the firmware setting. */
 	caps->ctrl = (caps->ctrl & ~mask) | (caps->fw_ctrl & mask);
 	caps->ctrl |= flags;
 
-	pci_info(dev, "Configured ACS to 0x%x\n", caps->ctrl);
+	pci_info(dev, "Configured ACS to %#06x\n", caps->ctrl);
 }
 
 /**