[PATCH] pci: Add a acs_disable option for pci kernel parameter

[PATCH] pci: Add a acs_disable option for pci kernel parameter

[sbates]

From: Stephen Bates <sbates@raithlin.com>

On some servers the BIOS sets up ACS on any valid pci_dev in the
system. The kernel has no way of backing this out since the kernel
only turns ACS capabilities on.

This patch adds a new boot option to the pci kernel parameter called
"acs_disable" that will disable ACS. This is useful for PCI peer to
peer communication but can cause problems when IOVA isolation is
required and an IOMMU is enabled. Use with care.

Signed-off-by: Stephen Bates <sbates@raithlin.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  4 ++++
 drivers/pci/pci.c                               | 23 ++++++++++++++++++++---
 drivers/pci/pci.h                               |  2 +-
 drivers/pci/probe.c                             |  4 ++--
 4 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 0549662..695eb12 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2907,6 +2907,10 @@
 		earlydump	[X86] dump PCI config space before the kernel
 			        changes anything
 		off		[X86] don't probe for the PCI bus
+		acs_disable     [PCIE] disable access control services. Note
+				this can interfere with IOVA isolation if an IOMMU
+				is enabled but can be necessary when doing PCI
+				peer to peer communication. Use with care.
 		bios		[X86-32] force use of PCI BIOS, don't access
 				the hardware directly. Use this if your machine
 				has a non-standard PCI host bridge.
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 6078dfc..ce33608 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -110,6 +110,9 @@ unsigned int pcibios_max_latency = 255;
 /* If set, the PCIe ARI capability will not be used. */
 static bool pcie_ari_disabled;
 
+/* If set, the PCIe ACS capability will be disabled. */
+static bool pci_acs_disable;
+
 /* Disable bridge_d3 for all PCIe ports */
 static bool pci_bridge_d3_disable;
 /* Force bridge_d3 for all PCIe ports */
@@ -1182,7 +1185,7 @@ void pci_restore_state(struct pci_dev *dev)
 	pci_restore_msi_state(dev);
 
 	/* Restore ACS and IOV configuration state */
-	pci_enable_acs(dev);
+	pci_config_acs(dev);
 	pci_restore_iov_state(dev);
 
 	dev->state_saved = false;
@@ -2821,11 +2824,23 @@ static void pci_std_enable_acs(struct pci_dev *dev)
 }
 
 /**
- * pci_enable_acs - enable ACS if hardware support it
+ * pci_config_acs - configure ACS
  * @dev: the PCI device
  */
-void pci_enable_acs(struct pci_dev *dev)
+void pci_config_acs(struct pci_dev *dev)
 {
+	int pos;
+
+	if (pci_acs_disable) {
+		pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ACS);
+		if (!pos)
+			return;
+
+		dev_warn_ratelimited(&dev->dev,
+				     "disabling ACS via pci_acs_disable\n");
+		pci_write_config_word(dev, pos + PCI_ACS_CTRL, 0);
+	}
+
 	if (!pci_acs_enable)
 		return;
 
@@ -5471,6 +5486,8 @@ static int __init pci_setup(char *str)
 		if (*str && (str = pcibios_setup(str)) && *str) {
 			if (!strcmp(str, "nomsi")) {
 				pci_no_msi();
+			} else if (!strcmp(str, "acs_disable")) {
+				pci_acs_disable = true;
 			} else if (!strcmp(str, "noaer")) {
 				pci_no_aer();
 			} else if (!strncmp(str, "realloc=", 8)) {
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index a6560c9..16d94d3 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -338,7 +338,7 @@ static inline resource_size_t pci_resource_alignment(struct pci_dev *dev,
 	return resource_alignment(res);
 }
 
-void pci_enable_acs(struct pci_dev *dev);
+void pci_config_acs(struct pci_dev *dev);
 
 #ifdef CONFIG_PCIE_PTM
 void pci_ptm_init(struct pci_dev *dev);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ff94b69..86c3299 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2020,8 +2020,8 @@ static void pci_init_capabilities(struct pci_dev *dev)
 	/* Address Translation Services */
 	pci_ats_init(dev);
 
-	/* Enable ACS P2P upstream forwarding */
-	pci_enable_acs(dev);
+	/* Configure ACS P2P upstream forwarding */
+	pci_config_acs(dev);
 
 	/* Precision Time Measurement */
 	pci_ptm_init(dev);
-- 
2.7.4

Re: [PATCH] pci: Add a acs_disable option for pci kernel parameter

[Christoph Hellwig]

On Thu, Oct 26, 2017 at 08:37:49PM -0600, sbates@raithlin.com wrote:
> From: Stephen Bates <sbates@raithlin.com>
> 
> On some servers the BIOS sets up ACS on any valid pci_dev in the
> system. The kernel has no way of backing this out since the kernel
> only turns ACS capabilities on.
> 
> This patch adds a new boot option to the pci kernel parameter called
> "acs_disable" that will disable ACS. This is useful for PCI peer to
> peer communication but can cause problems when IOVA isolation is
> required and an IOMMU is enabled. Use with care.

Eww.  Can we please add smbios quirks for the systems where you've
observed this? (we probably also want to keep the option just in case).

Re: [PATCH] pci: Add a acs_disable option for pci kernel parameter

[Stephen Bates]

Pj4gVGhpcyBwYXRjaCBhZGRzIGEgbmV3IGJvb3Qgb3B0aW9uIHRvIHRoZSBwY2kga2VybmVsIHBh
cmFtZXRlciBjYWxsZWQNCj4+ICJhY3NfZGlzYWJsZSIgdGhhdCB3aWxsIGRpc2FibGUgQUNTLiBU
aGlzIGlzIHVzZWZ1bCBmb3IgUENJIHBlZXIgdG8NCj4+IHBlZXIgY29tbXVuaWNhdGlvbiBidXQg
Y2FuIGNhdXNlIHByb2JsZW1zIHdoZW4gSU9WQSBpc29sYXRpb24gaXMNCj4+IHJlcXVpcmVkIGFu
ZCBhbiBJT01NVSBpcyBlbmFibGVkLiBVc2Ugd2l0aCBjYXJlLg0KDQo+IEV3dy4NCg0KVGhhbmtz
IGZvciB0aGUgZmVlZGJhY2sgQ2hyaXN0b3BoLiBNeSBzZW50aW1lbnRzIGV4YWN0bHkgOy0pLiAg
DQoNCj4gQ2FuIHdlIHBsZWFzZSBhZGQgc21iaW9zIHF1aXJrcyBmb3IgdGhlIHN5c3RlbXMgd2hl
cmUgeW91J3ZlDQo+IG9ic2VydmVkIHRoaXM/IA0KDQpJIGNhbiBsb29rIGF0IGRvaW5nIHRoaXMu
IFRoZSBpc3N1ZSB3aXRoIHRoaXMgYXBwcm9hY2ggaXMgdGhhdCBpdCB3aWxsIHJlcXVpcmUgYSBr
ZXJuZWwgcGF0Y2ggZm9yIGVhY2ggbmV3IHN5c3RlbSB0aGF0IGlzIGRldGVjdGVkLg0KDQo+IHdl
IHByb2JhYmx5IGFsc28gd2FudCB0byBrZWVwIHRoZSBvcHRpb24ganVzdCBpbiBjYXNlKS4NCg0K
QWdyZWVkLiBBdCBsZWFzdCB0aGVuIG9wZXJhdG9ycyBoYXZlIGEgcGF0aCB0byBBQ1MgZGlzYWJs
ZSBwcmlvciB0byBhIHF1aXJrIGJlaW5nIGFjY2VwdGVkLg0KDQpTdGVwaGVuDQoNCg==