linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] PCI/ATS: Device-tree support and other improvements
@ 2020-02-13 16:50 Jean-Philippe Brucker
  2020-02-13 16:50 ` [PATCH 01/11] dt-bindings: PCI: generic: Add ats-supported property Jean-Philippe Brucker
                   ` (12 more replies)
  0 siblings, 13 replies; 21+ messages in thread
From: Jean-Philippe Brucker @ 2020-02-13 16:50 UTC (permalink / raw)
  To: bhelgaas, will, robh+dt, lorenzo.pieralisi, joro, baolu.lu,
	linux-doc, linux-pci, linux-arm-kernel, devicetree, linux-acpi,
	iommu
  Cc: corbet, mark.rutland, liviu.dudau, sudeep.holla, guohanjun, rjw,
	lenb, robin.murphy, dwmw2, amurray, frowand.list

Enable ATS on device-tree based systems, and factor the common ATS
enablement checks into pci_enable_ats().

ATS support in PCIe endpoints is discovered through the ATS capability,
but there is no common method for discovering whether the host bridge
supports ATS. Each vendor provides their own ACPI method:
* DMAR (Intel) reports ATS support per domain or per root port.
* IVRS (AMD) reports negative ATS support for a range of devices.
* IORT (ARM) reports ATS support for a root complex.

In my opinion it's important that we only enable ATS if the host bridge
supports it, but I don't think a finer granularity is useful. If the
host bridge ignores the Address Translation field of TLP headers, it
could in theory treat a Translation Request as a Memory Read Request,
and a Translated Request as a normal memory access, which could result
in silent memory corruption.

Patches 1-3 add a device-tree property that declares ATS support in host
controllers. We only add it to the generic host, but the property can
easily be reused by other PCI hosts. Alternatively, the host drivers can
directly set the new ats_supported property of the host bridge
introduced in patch 1.

Patch 4 uses the new ats_supported host bridge property for IORT. Patch
9 removes the old method that set a flag in each endpoint's fwspec.

Patches 5-8 put all checks required for enabling ATS in common, along
with the new host bridge check.

Jean-Philippe Brucker (11):
  dt-bindings: PCI: generic: Add ats-supported property
  PCI: Add ats_supported host bridge flag
  PCI: OF: Check whether the host bridge supports ATS
  ACPI/IORT: Check ATS capability in root complex node
  PCI/ATS: Gather checks into pci_ats_supported()
  iommu/amd: Use pci_ats_supported()
  iommu/arm-smmu-v3: Use pci_ats_supported()
  iommu/vt-d: Use pci_ats_supported()
  ACPI/IORT: Drop ATS fwspec flag
  arm64: dts: fast models: Enable PCIe ATS for Base RevC FVP
  Documentation: Generalize the "pci=noats" boot parameter

 .../admin-guide/kernel-parameters.txt         |  4 +-
 .../bindings/pci/host-generic-pci.yaml        |  6 +++
 arch/arm64/boot/dts/arm/fvp-base-revc.dts     |  1 +
 drivers/acpi/arm64/iort.c                     | 38 +++++++++++++------
 drivers/acpi/pci_root.c                       |  3 ++
 drivers/iommu/amd_iommu.c                     | 12 ++----
 drivers/iommu/arm-smmu-v3.c                   | 18 ++-------
 drivers/iommu/intel-iommu.c                   |  9 ++---
 drivers/pci/ats.c                             | 30 ++++++++++++++-
 drivers/pci/controller/pci-host-common.c      |  1 +
 drivers/pci/of.c                              |  9 +++++
 drivers/pci/probe.c                           |  7 ++++
 include/linux/acpi_iort.h                     |  8 ++++
 include/linux/iommu.h                         |  4 --
 include/linux/of_pci.h                        |  3 ++
 include/linux/pci-ats.h                       |  3 ++
 include/linux/pci.h                           |  1 +
 17 files changed, 110 insertions(+), 47 deletions(-)

-- 
2.25.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 01/11] dt-bindings: PCI: generic: Add ats-supported property
  2020-02-13 16:50 [PATCH 00/10] PCI/ATS: Device-tree support and other improvements Jean-Philippe Brucker
@ 2020-02-13 16:50 ` Jean-Philippe Brucker
  2020-02-19 22:24   ` Rob Herring
  2020-02-13 16:50 ` [PATCH 02/11] PCI: Add ats_supported host bridge flag Jean-Philippe Brucker
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 21+ messages in thread
From: Jean-Philippe Brucker @ 2020-02-13 16:50 UTC (permalink / raw)
  To: bhelgaas, will, robh+dt, lorenzo.pieralisi, joro, baolu.lu,
	linux-doc, linux-pci, linux-arm-kernel, devicetree, linux-acpi,
	iommu
  Cc: corbet, mark.rutland, liviu.dudau, sudeep.holla, guohanjun, rjw,
	lenb, robin.murphy, dwmw2, amurray, frowand.list

Add a way for firmware to tell the OS that ATS is supported by the PCI
root complex. An endpoint with ATS enabled may send Translation Requests
and Translated Memory Requests, which look just like Normal Memory
Requests with a non-zero AT field. So a root controller that ignores the
AT field may simply forward the request to the IOMMU as a Normal Memory
Request, which could end badly. In any case, the endpoint will be
unusable.

The ats-supported property allows the OS to only enable ATS in endpoints
if the root controller can handle ATS requests. Only add the property to
pcie-host-ecam-generic for the moment. For non-generic root controllers,
availability of ATS can be inferred from the compatible string.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 Documentation/devicetree/bindings/pci/host-generic-pci.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
index 47353d0cd394..7d40edd7f1ef 100644
--- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
+++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
@@ -107,6 +107,12 @@ properties:
 
   dma-coherent: true
 
+  ats-supported:
+    description:
+      Indicates that a PCIe host controller supports ATS, and can handle Memory
+      Requests with Address Type (AT).
+    type: boolean
+
 required:
   - compatible
   - reg
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 02/11] PCI: Add ats_supported host bridge flag
  2020-02-13 16:50 [PATCH 00/10] PCI/ATS: Device-tree support and other improvements Jean-Philippe Brucker
  2020-02-13 16:50 ` [PATCH 01/11] dt-bindings: PCI: generic: Add ats-supported property Jean-Philippe Brucker
@ 2020-02-13 16:50 ` Jean-Philippe Brucker
  2020-02-15 21:10   ` Bjorn Helgaas
  2020-02-13 16:50 ` [PATCH 03/11] PCI: OF: Check whether the host bridge supports ATS Jean-Philippe Brucker
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 21+ messages in thread
From: Jean-Philippe Brucker @ 2020-02-13 16:50 UTC (permalink / raw)
  To: bhelgaas, will, robh+dt, lorenzo.pieralisi, joro, baolu.lu,
	linux-doc, linux-pci, linux-arm-kernel, devicetree, linux-acpi,
	iommu
  Cc: corbet, mark.rutland, liviu.dudau, sudeep.holla, guohanjun, rjw,
	lenb, robin.murphy, dwmw2, amurray, frowand.list

Each vendor has their own way of describing whether a host bridge
supports ATS.  The Intel and AMD ACPI tables selectively enable or
disable ATS per device or sub-tree, while Arm has a single bit for each
host bridge.  For those that need it, add an ats_supported bit to the
host bridge structure.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 drivers/pci/probe.c | 7 +++++++
 include/linux/pci.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 512cb4312ddd..75c0a25af44e 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -598,6 +598,13 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge)
 	bridge->native_shpc_hotplug = 1;
 	bridge->native_pme = 1;
 	bridge->native_ltr = 1;
+
+	/*
+	 * Some systems may disable ATS at the host bridge (ACPI IORT,
+	 * device-tree), other filter it with a smaller granularity (ACPI DMAR
+	 * and IVRS).
+	 */
+	bridge->ats_supported = 1;
 }
 
 struct pci_host_bridge *pci_alloc_host_bridge(size_t priv)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 3840a541a9de..9fe2e84d74d7 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -511,6 +511,7 @@ struct pci_host_bridge {
 	unsigned int	native_pme:1;		/* OS may use PCIe PME */
 	unsigned int	native_ltr:1;		/* OS may use PCIe LTR */
 	unsigned int	preserve_config:1;	/* Preserve FW resource setup */
+	unsigned int	ats_supported:1;
 
 	/* Resource alignment requirements */
 	resource_size_t (*align_resource)(struct pci_dev *dev,
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 03/11] PCI: OF: Check whether the host bridge supports ATS
  2020-02-13 16:50 [PATCH 00/10] PCI/ATS: Device-tree support and other improvements Jean-Philippe Brucker
  2020-02-13 16:50 ` [PATCH 01/11] dt-bindings: PCI: generic: Add ats-supported property Jean-Philippe Brucker
  2020-02-13 16:50 ` [PATCH 02/11] PCI: Add ats_supported host bridge flag Jean-Philippe Brucker
@ 2020-02-13 16:50 ` Jean-Philippe Brucker
  2020-02-13 18:26   ` Rob Herring
  2020-02-13 16:50 ` [PATCH 04/11] ACPI/IORT: Check ATS capability in root complex node Jean-Philippe Brucker
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 21+ messages in thread
From: Jean-Philippe Brucker @ 2020-02-13 16:50 UTC (permalink / raw)
  To: bhelgaas, will, robh+dt, lorenzo.pieralisi, joro, baolu.lu,
	linux-doc, linux-pci, linux-arm-kernel, devicetree, linux-acpi,
	iommu
  Cc: corbet, mark.rutland, liviu.dudau, sudeep.holla, guohanjun, rjw,
	lenb, robin.murphy, dwmw2, amurray, frowand.list

Copy the ats-supported flag into the pci_host_bridge structure.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 drivers/pci/controller/pci-host-common.c | 1 +
 drivers/pci/of.c                         | 9 +++++++++
 include/linux/of_pci.h                   | 3 +++
 3 files changed, 13 insertions(+)

diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
index 250a3fc80ec6..a6ac927be291 100644
--- a/drivers/pci/controller/pci-host-common.c
+++ b/drivers/pci/controller/pci-host-common.c
@@ -92,6 +92,7 @@ int pci_host_common_probe(struct platform_device *pdev,
 		return ret;
 	}
 
+	of_pci_host_check_ats(bridge);
 	platform_set_drvdata(pdev, bridge->bus);
 	return 0;
 }
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 81ceeaa6f1d5..4b8a877f1e9f 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -576,6 +576,15 @@ int pci_parse_request_of_pci_ranges(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(pci_parse_request_of_pci_ranges);
 
+void of_pci_host_check_ats(struct pci_host_bridge *bridge)
+{
+	struct device_node *np = bridge->bus->dev.of_node;
+
+	if (!np)
+		return;
+
+	bridge->ats_supported = of_property_read_bool(np, "ats-supported");
+}
 #endif /* CONFIG_PCI */
 
 /**
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 29658c0ee71f..2d0af410438c 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -7,12 +7,14 @@
 
 struct pci_dev;
 struct device_node;
+struct pci_host_bridge;
 
 #if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_PCI)
 struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn);
 int of_pci_get_devfn(struct device_node *np);
 void of_pci_check_probe_only(void);
+void of_pci_host_check_ats(struct pci_host_bridge *bridge);
 #else
 static inline struct device_node *of_pci_find_child_device(struct device_node *parent,
 					     unsigned int devfn)
@@ -26,6 +28,7 @@ static inline int of_pci_get_devfn(struct device_node *np)
 }
 
 static inline void of_pci_check_probe_only(void) { }
+static inline void of_pci_host_check_ats(struct pci_host_bridge *bridge) { }
 #endif
 
 #if IS_ENABLED(CONFIG_OF_IRQ)
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 04/11] ACPI/IORT: Check ATS capability in root complex node
  2020-02-13 16:50 [PATCH 00/10] PCI/ATS: Device-tree support and other improvements Jean-Philippe Brucker
                   ` (2 preceding siblings ...)
  2020-02-13 16:50 ` [PATCH 03/11] PCI: OF: Check whether the host bridge supports ATS Jean-Philippe Brucker
@ 2020-02-13 16:50 ` Jean-Philippe Brucker
  2020-03-06  9:37   ` Hanjun Guo
  2020-02-13 16:50 ` [PATCH 05/11] PCI/ATS: Gather checks into pci_ats_supported() Jean-Philippe Brucker
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 21+ messages in thread
From: Jean-Philippe Brucker @ 2020-02-13 16:50 UTC (permalink / raw)
  To: bhelgaas, will, robh+dt, lorenzo.pieralisi, joro, baolu.lu,
	linux-doc, linux-pci, linux-arm-kernel, devicetree, linux-acpi,
	iommu
  Cc: corbet, mark.rutland, liviu.dudau, sudeep.holla, guohanjun, rjw,
	lenb, robin.murphy, dwmw2, amurray, frowand.list

When initializing a PCI root bridge, copy its "ATS supported" attribute
into the root bridge.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 drivers/acpi/arm64/iort.c | 27 +++++++++++++++++++++++++++
 drivers/acpi/pci_root.c   |  3 +++
 include/linux/acpi_iort.h |  8 ++++++++
 3 files changed, 38 insertions(+)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index ed3d2d1a7ae9..d99d7f5b51e1 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1633,6 +1633,33 @@ static void __init iort_enable_acs(struct acpi_iort_node *iort_node)
 		}
 	}
 }
+
+static acpi_status iort_match_host_bridge_callback(struct acpi_iort_node *node,
+						   void *context)
+{
+	struct acpi_iort_root_complex *pci_rc;
+	struct pci_host_bridge *host_bridge = context;
+
+	pci_rc = (struct acpi_iort_root_complex *)node->node_data;
+
+	return pci_domain_nr(host_bridge->bus) == pci_rc->pci_segment_number ?
+		AE_OK : AE_NOT_FOUND;
+}
+
+void iort_pci_host_bridge_setup(struct pci_host_bridge *host_bridge)
+{
+	struct acpi_iort_node *node;
+	struct acpi_iort_root_complex *pci_rc;
+
+	node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
+			      iort_match_host_bridge_callback, host_bridge);
+	if (!node)
+		return;
+
+	pci_rc = (struct acpi_iort_root_complex *)node->node_data;
+	host_bridge->ats_supported = !!(pci_rc->ats_attribute &
+					ACPI_IORT_ATS_SUPPORTED);
+}
 #else
 static inline void iort_enable_acs(struct acpi_iort_node *iort_node) { }
 #endif
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index d1e666ef3fcc..eb2fb8f17c0b 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -6,6 +6,7 @@
  *  Copyright (C) 2001, 2002 Paul Diefenbaugh <paul.s.diefenbaugh@intel.com>
  */
 
+#include <linux/acpi_iort.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -917,6 +918,8 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
 	if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL))
 		host_bridge->native_ltr = 0;
 
+	iort_pci_host_bridge_setup(host_bridge);
+
 	/*
 	 * Evaluate the "PCI Boot Configuration" _DSM Function.  If it
 	 * exists and returns 0, we must preserve any PCI resource
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index 8e7e2ec37f1b..7b06871cc3aa 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -10,6 +10,7 @@
 #include <linux/acpi.h>
 #include <linux/fwnode.h>
 #include <linux/irqdomain.h>
+#include <linux/pci.h>
 
 #define IORT_IRQ_MASK(irq)		(irq & 0xffffffffULL)
 #define IORT_IRQ_TRIGGER_MASK(irq)	((irq >> 32) & 0xffffffffULL)
@@ -55,4 +56,11 @@ int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
 { return 0; }
 #endif
 
+#if defined(CONFIG_ACPI_IORT) && defined(CONFIG_PCI)
+void iort_pci_host_bridge_setup(struct pci_host_bridge *host_bridge);
+#else
+static inline
+void iort_pci_host_bridge_setup(struct pci_host_bridge *host_bridge) { }
+#endif
+
 #endif /* __ACPI_IORT_H__ */
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 05/11] PCI/ATS: Gather checks into pci_ats_supported()
  2020-02-13 16:50 [PATCH 00/10] PCI/ATS: Device-tree support and other improvements Jean-Philippe Brucker
                   ` (3 preceding siblings ...)
  2020-02-13 16:50 ` [PATCH 04/11] ACPI/IORT: Check ATS capability in root complex node Jean-Philippe Brucker
@ 2020-02-13 16:50 ` Jean-Philippe Brucker
  2020-02-13 16:50 ` [PATCH 06/11] iommu/amd: Use pci_ats_supported() Jean-Philippe Brucker
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Jean-Philippe Brucker @ 2020-02-13 16:50 UTC (permalink / raw)
  To: bhelgaas, will, robh+dt, lorenzo.pieralisi, joro, baolu.lu,
	linux-doc, linux-pci, linux-arm-kernel, devicetree, linux-acpi,
	iommu
  Cc: corbet, mark.rutland, liviu.dudau, sudeep.holla, guohanjun, rjw,
	lenb, robin.murphy, dwmw2, amurray, frowand.list

IOMMU drivers need to perform several tests when checking if a device
supports ATS.  Move them all into a new function that returns true when
a device and its host bridge support ATS.

Since pci_enable_ats() now calls pci_ats_supported(), the following
new checks are now common:
* whether a device is trusted.  Devices plugged into external-facing
  ports such as thunderbolt are untrusted.
* whether the host bridge supports ATS, which defaults to true unless
  the firmware description states that ATS isn't supported by the host
  bridge.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 drivers/pci/ats.c       | 30 +++++++++++++++++++++++++++++-
 include/linux/pci-ats.h |  3 +++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index 390e92f2d8d1..bbfd0d42b8b9 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -30,6 +30,34 @@ void pci_ats_init(struct pci_dev *dev)
 	dev->ats_cap = pos;
 }
 
+/**
+ * pci_ats_supported - check if the device can use ATS
+ * @dev: the PCI device
+ *
+ * Returns true if the device supports ATS and is allowed to use it, false
+ * otherwise.
+ */
+bool pci_ats_supported(struct pci_dev *dev)
+{
+	struct pci_host_bridge *bridge;
+
+	if (!dev->ats_cap)
+		return false;
+
+	if (dev->untrusted)
+		return false;
+
+	bridge = pci_find_host_bridge(dev->bus);
+	if (!bridge)
+		return false;
+
+	if (!bridge->ats_supported)
+		return false;
+
+	return true;
+}
+EXPORT_SYMBOL_GPL(pci_ats_supported);
+
 /**
  * pci_enable_ats - enable the ATS capability
  * @dev: the PCI device
@@ -42,7 +70,7 @@ int pci_enable_ats(struct pci_dev *dev, int ps)
 	u16 ctrl;
 	struct pci_dev *pdev;
 
-	if (!dev->ats_cap)
+	if (!pci_ats_supported(dev))
 		return -EINVAL;
 
 	if (WARN_ON(dev->ats_enabled))
diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h
index d08f0869f121..f75c307f346d 100644
--- a/include/linux/pci-ats.h
+++ b/include/linux/pci-ats.h
@@ -6,11 +6,14 @@
 
 #ifdef CONFIG_PCI_ATS
 /* Address Translation Service */
+bool pci_ats_supported(struct pci_dev *dev);
 int pci_enable_ats(struct pci_dev *dev, int ps);
 void pci_disable_ats(struct pci_dev *dev);
 int pci_ats_queue_depth(struct pci_dev *dev);
 int pci_ats_page_aligned(struct pci_dev *dev);
 #else /* CONFIG_PCI_ATS */
+static inline bool pci_ats_supported(struct pci_dev *d)
+{ return false; }
 static inline int pci_enable_ats(struct pci_dev *d, int ps)
 { return -ENODEV; }
 static inline void pci_disable_ats(struct pci_dev *d) { }
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 06/11] iommu/amd: Use pci_ats_supported()
  2020-02-13 16:50 [PATCH 00/10] PCI/ATS: Device-tree support and other improvements Jean-Philippe Brucker
                   ` (4 preceding siblings ...)
  2020-02-13 16:50 ` [PATCH 05/11] PCI/ATS: Gather checks into pci_ats_supported() Jean-Philippe Brucker
@ 2020-02-13 16:50 ` Jean-Philippe Brucker
  2020-02-13 16:50 ` [PATCH 07/11] iommu/arm-smmu-v3: " Jean-Philippe Brucker
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Jean-Philippe Brucker @ 2020-02-13 16:50 UTC (permalink / raw)
  To: bhelgaas, will, robh+dt, lorenzo.pieralisi, joro, baolu.lu,
	linux-doc, linux-pci, linux-arm-kernel, devicetree, linux-acpi,
	iommu
  Cc: corbet, mark.rutland, liviu.dudau, sudeep.holla, guohanjun, rjw,
	lenb, robin.murphy, dwmw2, amurray, frowand.list

The pci_ats_supported() function checks if a device supports ATS and is
allowed to use it.  In addition to checking that the device has an ATS
capability and that the global pci=noats is not set
(pci_ats_disabled()), it also checks if a device is untrusted (plugged
into an external-facing port such as thunderbolt).

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 drivers/iommu/amd_iommu.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index aac132bd1ef0..084f0b2e132e 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -291,16 +291,15 @@ static struct iommu_group *acpihid_device_group(struct device *dev)
 static bool pci_iommuv2_capable(struct pci_dev *pdev)
 {
 	static const int caps[] = {
-		PCI_EXT_CAP_ID_ATS,
 		PCI_EXT_CAP_ID_PRI,
 		PCI_EXT_CAP_ID_PASID,
 	};
 	int i, pos;
 
-	if (pci_ats_disabled())
+	if (!pci_ats_supported(pdev))
 		return false;
 
-	for (i = 0; i < 3; ++i) {
+	for (i = 0; i < 2; ++i) {
 		pos = pci_find_ext_capability(pdev, caps[i]);
 		if (pos == 0)
 			return false;
@@ -3040,11 +3039,8 @@ int amd_iommu_device_info(struct pci_dev *pdev,
 
 	memset(info, 0, sizeof(*info));
 
-	if (!pci_ats_disabled()) {
-		pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_ATS);
-		if (pos)
-			info->flags |= AMD_IOMMU_DEVICE_FLAG_ATS_SUP;
-	}
+	if (pci_ats_supported(pdev))
+		info->flags |= AMD_IOMMU_DEVICE_FLAG_ATS_SUP;
 
 	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI);
 	if (pos)
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 07/11] iommu/arm-smmu-v3: Use pci_ats_supported()
  2020-02-13 16:50 [PATCH 00/10] PCI/ATS: Device-tree support and other improvements Jean-Philippe Brucker
                   ` (5 preceding siblings ...)
  2020-02-13 16:50 ` [PATCH 06/11] iommu/amd: Use pci_ats_supported() Jean-Philippe Brucker
@ 2020-02-13 16:50 ` Jean-Philippe Brucker
  2020-02-13 16:50 ` [PATCH 08/11] iommu/vt-d: " Jean-Philippe Brucker
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Jean-Philippe Brucker @ 2020-02-13 16:50 UTC (permalink / raw)
  To: bhelgaas, will, robh+dt, lorenzo.pieralisi, joro, baolu.lu,
	linux-doc, linux-pci, linux-arm-kernel, devicetree, linux-acpi,
	iommu
  Cc: corbet, mark.rutland, liviu.dudau, sudeep.holla, guohanjun, rjw,
	lenb, robin.murphy, dwmw2, amurray, frowand.list

The new pci_ats_supported() function checks if a device supports ATS and
is allowed to use it.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 drivers/iommu/arm-smmu-v3.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 034ad9671b83..bd2cfd946a36 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2577,26 +2577,14 @@ static void arm_smmu_install_ste_for_dev(struct arm_smmu_master *master)
 	}
 }
 
-#ifdef CONFIG_PCI_ATS
 static bool arm_smmu_ats_supported(struct arm_smmu_master *master)
 {
-	struct pci_dev *pdev;
+	struct device *dev = master->dev;
 	struct arm_smmu_device *smmu = master->smmu;
-	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(master->dev);
-
-	if (!(smmu->features & ARM_SMMU_FEAT_ATS) || !dev_is_pci(master->dev) ||
-	    !(fwspec->flags & IOMMU_FWSPEC_PCI_RC_ATS) || pci_ats_disabled())
-		return false;
 
-	pdev = to_pci_dev(master->dev);
-	return !pdev->untrusted && pdev->ats_cap;
+	return (smmu->features & ARM_SMMU_FEAT_ATS) && dev_is_pci(dev) &&
+		pci_ats_supported(to_pci_dev(dev));
 }
-#else
-static bool arm_smmu_ats_supported(struct arm_smmu_master *master)
-{
-	return false;
-}
-#endif
 
 static void arm_smmu_enable_ats(struct arm_smmu_master *master)
 {
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 08/11] iommu/vt-d: Use pci_ats_supported()
  2020-02-13 16:50 [PATCH 00/10] PCI/ATS: Device-tree support and other improvements Jean-Philippe Brucker
                   ` (6 preceding siblings ...)
  2020-02-13 16:50 ` [PATCH 07/11] iommu/arm-smmu-v3: " Jean-Philippe Brucker
@ 2020-02-13 16:50 ` Jean-Philippe Brucker
  2020-02-13 16:50 ` [PATCH 09/11] ACPI/IORT: Drop ATS fwspec flag Jean-Philippe Brucker
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Jean-Philippe Brucker @ 2020-02-13 16:50 UTC (permalink / raw)
  To: bhelgaas, will, robh+dt, lorenzo.pieralisi, joro, baolu.lu,
	linux-doc, linux-pci, linux-arm-kernel, devicetree, linux-acpi,
	iommu
  Cc: corbet, mark.rutland, liviu.dudau, sudeep.holla, guohanjun, rjw,
	lenb, robin.murphy, dwmw2, amurray, frowand.list

The pci_ats_supported() function checks if a device supports ATS and is
allowed to use it.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 drivers/iommu/intel-iommu.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 9dc37672bf89..668f1b99111b 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1449,8 +1449,7 @@ static void iommu_enable_dev_iotlb(struct device_domain_info *info)
 	    !pci_reset_pri(pdev) && !pci_enable_pri(pdev, 32))
 		info->pri_enabled = 1;
 #endif
-	if (!pdev->untrusted && info->ats_supported &&
-	    pci_ats_page_aligned(pdev) &&
+	if (info->ats_supported && pci_ats_page_aligned(pdev) &&
 	    !pci_enable_ats(pdev, VTD_PAGE_SHIFT)) {
 		info->ats_enabled = 1;
 		domain_update_iotlb(info->domain);
@@ -2611,10 +2610,8 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu,
 	if (dev && dev_is_pci(dev)) {
 		struct pci_dev *pdev = to_pci_dev(info->dev);
 
-		if (!pdev->untrusted &&
-		    !pci_ats_disabled() &&
-		    ecap_dev_iotlb_support(iommu->ecap) &&
-		    pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_ATS) &&
+		if (ecap_dev_iotlb_support(iommu->ecap) &&
+		    pci_ats_supported(pdev) &&
 		    dmar_find_matched_atsr_unit(pdev))
 			info->ats_supported = 1;
 
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 09/11] ACPI/IORT: Drop ATS fwspec flag
  2020-02-13 16:50 [PATCH 00/10] PCI/ATS: Device-tree support and other improvements Jean-Philippe Brucker
                   ` (7 preceding siblings ...)
  2020-02-13 16:50 ` [PATCH 08/11] iommu/vt-d: " Jean-Philippe Brucker
@ 2020-02-13 16:50 ` Jean-Philippe Brucker
  2020-03-06  9:42   ` Hanjun Guo
  2020-02-13 16:50 ` [PATCH 10/11] arm64: dts: fast models: Enable PCIe ATS for Base RevC FVP Jean-Philippe Brucker
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 21+ messages in thread
From: Jean-Philippe Brucker @ 2020-02-13 16:50 UTC (permalink / raw)
  To: bhelgaas, will, robh+dt, lorenzo.pieralisi, joro, baolu.lu,
	linux-doc, linux-pci, linux-arm-kernel, devicetree, linux-acpi,
	iommu
  Cc: corbet, mark.rutland, liviu.dudau, sudeep.holla, guohanjun, rjw,
	lenb, robin.murphy, dwmw2, amurray, frowand.list

Now that the ats_supported flag is in the host bridge structure where it
belongs, we can remove it from the per-device fwspec structure.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 drivers/acpi/arm64/iort.c | 11 -----------
 include/linux/iommu.h     |  4 ----
 2 files changed, 15 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index d99d7f5b51e1..f634641b3699 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -924,14 +924,6 @@ static int arm_smmu_iort_xlate(struct device *dev, u32 streamid,
 	return ret;
 }
 
-static bool iort_pci_rc_supports_ats(struct acpi_iort_node *node)
-{
-	struct acpi_iort_root_complex *pci_rc;
-
-	pci_rc = (struct acpi_iort_root_complex *)node->node_data;
-	return pci_rc->ats_attribute & ACPI_IORT_ATS_SUPPORTED;
-}
-
 static int iort_iommu_xlate(struct device *dev, struct acpi_iort_node *node,
 			    u32 streamid)
 {
@@ -1026,9 +1018,6 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev)
 		info.node = node;
 		err = pci_for_each_dma_alias(to_pci_dev(dev),
 					     iort_pci_iommu_init, &info);
-
-		if (!err && iort_pci_rc_supports_ats(node))
-			dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS;
 	} else {
 		int i = 0;
 
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index d1b5f4d98569..1739f8a7a4b4 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -589,15 +589,11 @@ struct iommu_fwspec {
 	const struct iommu_ops	*ops;
 	struct fwnode_handle	*iommu_fwnode;
 	void			*iommu_priv;
-	u32			flags;
 	u32			num_pasid_bits;
 	unsigned int		num_ids;
 	u32			ids[1];
 };
 
-/* ATS is supported */
-#define IOMMU_FWSPEC_PCI_RC_ATS			(1 << 0)
-
 /**
  * struct iommu_sva - handle to a device-mm bond
  */
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 10/11] arm64: dts: fast models: Enable PCIe ATS for Base RevC FVP
  2020-02-13 16:50 [PATCH 00/10] PCI/ATS: Device-tree support and other improvements Jean-Philippe Brucker
                   ` (8 preceding siblings ...)
  2020-02-13 16:50 ` [PATCH 09/11] ACPI/IORT: Drop ATS fwspec flag Jean-Philippe Brucker
@ 2020-02-13 16:50 ` Jean-Philippe Brucker
  2020-02-13 16:50 ` [PATCH 11/11] Documentation: Generalize the "pci=noats" boot parameter Jean-Philippe Brucker
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Jean-Philippe Brucker @ 2020-02-13 16:50 UTC (permalink / raw)
  To: bhelgaas, will, robh+dt, lorenzo.pieralisi, joro, baolu.lu,
	linux-doc, linux-pci, linux-arm-kernel, devicetree, linux-acpi,
	iommu
  Cc: corbet, mark.rutland, liviu.dudau, sudeep.holla, guohanjun, rjw,
	lenb, robin.murphy, dwmw2, amurray, frowand.list

Declare that the host controller supports ATS, so the OS can enable it
for ATS-capable PCIe endpoints.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
All endpoints support ATS provided they have the ats_supported=1 model
parameter. "lspci -vv" shows whether ATS is supported and enabled.
---
 arch/arm64/boot/dts/arm/fvp-base-revc.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/arm/fvp-base-revc.dts b/arch/arm64/boot/dts/arm/fvp-base-revc.dts
index 62ab0d54ff71..6e5bb7bcb4b3 100644
--- a/arch/arm64/boot/dts/arm/fvp-base-revc.dts
+++ b/arch/arm64/boot/dts/arm/fvp-base-revc.dts
@@ -170,6 +170,7 @@ pci: pci@40000000 {
 		iommu-map = <0x0 &smmu 0x0 0x10000>;
 
 		dma-coherent;
+		ats-supported;
 	};
 
 	smmu: smmu@2b400000 {
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 11/11] Documentation: Generalize the "pci=noats" boot parameter
  2020-02-13 16:50 [PATCH 00/10] PCI/ATS: Device-tree support and other improvements Jean-Philippe Brucker
                   ` (9 preceding siblings ...)
  2020-02-13 16:50 ` [PATCH 10/11] arm64: dts: fast models: Enable PCIe ATS for Base RevC FVP Jean-Philippe Brucker
@ 2020-02-13 16:50 ` Jean-Philippe Brucker
  2020-02-19 10:42 ` [PATCH 00/10] PCI/ATS: Device-tree support and other improvements Joerg Roedel
  2020-03-06  9:32 ` Hanjun Guo
  12 siblings, 0 replies; 21+ messages in thread
From: Jean-Philippe Brucker @ 2020-02-13 16:50 UTC (permalink / raw)
  To: bhelgaas, will, robh+dt, lorenzo.pieralisi, joro, baolu.lu,
	linux-doc, linux-pci, linux-arm-kernel, devicetree, linux-acpi,
	iommu
  Cc: corbet, mark.rutland, liviu.dudau, sudeep.holla, guohanjun, rjw,
	lenb, robin.murphy, dwmw2, amurray, frowand.list

The "pci=noats" kernel parameter disables PCIe ATS globally, and affects
any ATS-capable IOMMU driver. So rather than adding Arm SMMUv3, which
recently gained ATS support, to the list of relevant build options,
simplify the noats description.

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 Documentation/admin-guide/kernel-parameters.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index dbc22d684627..e5fa8d057a3c 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3606,8 +3606,8 @@
 				on: Turn realloc on
 		realloc		same as realloc=on
 		noari		do not use PCIe ARI.
-		noats		[PCIE, Intel-IOMMU, AMD-IOMMU]
-				do not use PCIe ATS (and IOMMU device IOTLB).
+		noats		[PCIE] Do not use PCIe ATS (and IOMMU device
+				IOTLB).
 		pcie_scan_all	Scan all possible PCIe devices.  Otherwise we
 				only look for one device below a PCIe downstream
 				port.
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 03/11] PCI: OF: Check whether the host bridge supports ATS
  2020-02-13 16:50 ` [PATCH 03/11] PCI: OF: Check whether the host bridge supports ATS Jean-Philippe Brucker
@ 2020-02-13 18:26   ` Rob Herring
  2020-02-17 12:40     ` Jean-Philippe Brucker
  0 siblings, 1 reply; 21+ messages in thread
From: Rob Herring @ 2020-02-13 18:26 UTC (permalink / raw)
  To: Jean-Philippe Brucker
  Cc: Bjorn Helgaas, Will Deacon, Lorenzo Pieralisi, Joerg Roedel,
	Lu Baolu, Linux Doc Mailing List, PCI,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	devicetree, linux-acpi, Linux IOMMU, Jonathan Corbet,
	Mark Rutland, Liviu Dudau, Sudeep Holla, Hanjun Guo,
	Rafael J. Wysocki, Len Brown, Robin Murphy, David Woodhouse,
	Andrew Murray, Frank Rowand

On Thu, Feb 13, 2020 at 10:52 AM Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:
>
> Copy the ats-supported flag into the pci_host_bridge structure.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  drivers/pci/controller/pci-host-common.c | 1 +
>  drivers/pci/of.c                         | 9 +++++++++
>  include/linux/of_pci.h                   | 3 +++
>  3 files changed, 13 insertions(+)
>
> diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> index 250a3fc80ec6..a6ac927be291 100644
> --- a/drivers/pci/controller/pci-host-common.c
> +++ b/drivers/pci/controller/pci-host-common.c
> @@ -92,6 +92,7 @@ int pci_host_common_probe(struct platform_device *pdev,
>                 return ret;
>         }
>
> +       of_pci_host_check_ats(bridge);
>         platform_set_drvdata(pdev, bridge->bus);
>         return 0;
>  }
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 81ceeaa6f1d5..4b8a877f1e9f 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -576,6 +576,15 @@ int pci_parse_request_of_pci_ranges(struct device *dev,
>  }
>  EXPORT_SYMBOL_GPL(pci_parse_request_of_pci_ranges);
>
> +void of_pci_host_check_ats(struct pci_host_bridge *bridge)
> +{
> +       struct device_node *np = bridge->bus->dev.of_node;
> +
> +       if (!np)
> +               return;
> +
> +       bridge->ats_supported = of_property_read_bool(np, "ats-supported");
> +}

Not really any point in a common function if we expect this to be only
for ECAM hosts which it seems to be based on the binding.

Otherwise, needs an export if not.

Rob

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 02/11] PCI: Add ats_supported host bridge flag
  2020-02-13 16:50 ` [PATCH 02/11] PCI: Add ats_supported host bridge flag Jean-Philippe Brucker
@ 2020-02-15 21:10   ` Bjorn Helgaas
  2020-02-17 15:40     ` Jean-Philippe Brucker
  0 siblings, 1 reply; 21+ messages in thread
From: Bjorn Helgaas @ 2020-02-15 21:10 UTC (permalink / raw)
  To: Jean-Philippe Brucker
  Cc: will, robh+dt, lorenzo.pieralisi, joro, baolu.lu, linux-doc,
	linux-pci, linux-arm-kernel, devicetree, linux-acpi, iommu,
	corbet, mark.rutland, liviu.dudau, sudeep.holla, guohanjun, rjw,
	lenb, robin.murphy, dwmw2, amurray, frowand.list

On Thu, Feb 13, 2020 at 05:50:40PM +0100, Jean-Philippe Brucker wrote:
> Each vendor has their own way of describing whether a host bridge
> supports ATS.  The Intel and AMD ACPI tables selectively enable or
> disable ATS per device or sub-tree, while Arm has a single bit for each
> host bridge.  For those that need it, add an ats_supported bit to the
> host bridge structure.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  drivers/pci/probe.c | 7 +++++++
>  include/linux/pci.h | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 512cb4312ddd..75c0a25af44e 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -598,6 +598,13 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge)
>  	bridge->native_shpc_hotplug = 1;
>  	bridge->native_pme = 1;
>  	bridge->native_ltr = 1;
> +
> +	/*
> +	 * Some systems may disable ATS at the host bridge (ACPI IORT,
> +	 * device-tree), other filter it with a smaller granularity (ACPI DMAR
> +	 * and IVRS).
> +	 */
> +	bridge->ats_supported = 1;

The cover letter says it's important to enable ATS only if the host
bridge supports it.  From the other patches, it looks like we learn if
the host bridge supports ATS from either a DT "ats-supported" property
or an ACPI IORT table.  If that's the case, shouldn't the default here
be "ATS is *not* supported"?

>  }
>  
>  struct pci_host_bridge *pci_alloc_host_bridge(size_t priv)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 3840a541a9de..9fe2e84d74d7 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -511,6 +511,7 @@ struct pci_host_bridge {
>  	unsigned int	native_pme:1;		/* OS may use PCIe PME */
>  	unsigned int	native_ltr:1;		/* OS may use PCIe LTR */
>  	unsigned int	preserve_config:1;	/* Preserve FW resource setup */
> +	unsigned int	ats_supported:1;
>  
>  	/* Resource alignment requirements */
>  	resource_size_t (*align_resource)(struct pci_dev *dev,
> -- 
> 2.25.0
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 03/11] PCI: OF: Check whether the host bridge supports ATS
  2020-02-13 18:26   ` Rob Herring
@ 2020-02-17 12:40     ` Jean-Philippe Brucker
  0 siblings, 0 replies; 21+ messages in thread
From: Jean-Philippe Brucker @ 2020-02-17 12:40 UTC (permalink / raw)
  To: Rob Herring
  Cc: Bjorn Helgaas, Will Deacon, Lorenzo Pieralisi, Joerg Roedel,
	Lu Baolu, Linux Doc Mailing List, PCI,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	devicetree, linux-acpi, Linux IOMMU, Jonathan Corbet,
	Mark Rutland, Liviu Dudau, Sudeep Holla, Hanjun Guo,
	Rafael J. Wysocki, Len Brown, Robin Murphy, David Woodhouse,
	Andrew Murray, Frank Rowand

On Thu, Feb 13, 2020 at 12:26:46PM -0600, Rob Herring wrote:
> On Thu, Feb 13, 2020 at 10:52 AM Jean-Philippe Brucker
> <jean-philippe@linaro.org> wrote:
> >
> > Copy the ats-supported flag into the pci_host_bridge structure.
> >
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > ---
> >  drivers/pci/controller/pci-host-common.c | 1 +
> >  drivers/pci/of.c                         | 9 +++++++++
> >  include/linux/of_pci.h                   | 3 +++
> >  3 files changed, 13 insertions(+)
> >
> > diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> > index 250a3fc80ec6..a6ac927be291 100644
> > --- a/drivers/pci/controller/pci-host-common.c
> > +++ b/drivers/pci/controller/pci-host-common.c
> > @@ -92,6 +92,7 @@ int pci_host_common_probe(struct platform_device *pdev,
> >                 return ret;
> >         }
> >
> > +       of_pci_host_check_ats(bridge);
> >         platform_set_drvdata(pdev, bridge->bus);
> >         return 0;
> >  }
> > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > index 81ceeaa6f1d5..4b8a877f1e9f 100644
> > --- a/drivers/pci/of.c
> > +++ b/drivers/pci/of.c
> > @@ -576,6 +576,15 @@ int pci_parse_request_of_pci_ranges(struct device *dev,
> >  }
> >  EXPORT_SYMBOL_GPL(pci_parse_request_of_pci_ranges);
> >
> > +void of_pci_host_check_ats(struct pci_host_bridge *bridge)
> > +{
> > +       struct device_node *np = bridge->bus->dev.of_node;
> > +
> > +       if (!np)
> > +               return;
> > +
> > +       bridge->ats_supported = of_property_read_bool(np, "ats-supported");
> > +}
> 
> Not really any point in a common function if we expect this to be only
> for ECAM hosts which it seems to be based on the binding.

I'll move this to pci-host-common.c

Thanks,
Jean

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 02/11] PCI: Add ats_supported host bridge flag
  2020-02-15 21:10   ` Bjorn Helgaas
@ 2020-02-17 15:40     ` Jean-Philippe Brucker
  0 siblings, 0 replies; 21+ messages in thread
From: Jean-Philippe Brucker @ 2020-02-17 15:40 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: will, robh+dt, lorenzo.pieralisi, joro, baolu.lu, linux-doc,
	linux-pci, linux-arm-kernel, devicetree, linux-acpi, iommu,
	corbet, mark.rutland, liviu.dudau, sudeep.holla, guohanjun, rjw,
	lenb, robin.murphy, dwmw2, amurray, frowand.list

On Sat, Feb 15, 2020 at 03:10:47PM -0600, Bjorn Helgaas wrote:
> On Thu, Feb 13, 2020 at 05:50:40PM +0100, Jean-Philippe Brucker wrote:
> > Each vendor has their own way of describing whether a host bridge
> > supports ATS.  The Intel and AMD ACPI tables selectively enable or
> > disable ATS per device or sub-tree, while Arm has a single bit for each
> > host bridge.  For those that need it, add an ats_supported bit to the
> > host bridge structure.
> > 
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > ---
> >  drivers/pci/probe.c | 7 +++++++
> >  include/linux/pci.h | 1 +
> >  2 files changed, 8 insertions(+)
> > 
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 512cb4312ddd..75c0a25af44e 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -598,6 +598,13 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge)
> >  	bridge->native_shpc_hotplug = 1;
> >  	bridge->native_pme = 1;
> >  	bridge->native_ltr = 1;
> > +
> > +	/*
> > +	 * Some systems may disable ATS at the host bridge (ACPI IORT,
> > +	 * device-tree), other filter it with a smaller granularity (ACPI DMAR
> > +	 * and IVRS).
> > +	 */
> > +	bridge->ats_supported = 1;
> 
> The cover letter says it's important to enable ATS only if the host
> bridge supports it.  From the other patches, it looks like we learn if
> the host bridge supports ATS from either a DT "ats-supported" property
> or an ACPI IORT table.  If that's the case, shouldn't the default here
> be "ATS is *not* supported"?

The ACPI IVRS table (AMD) doesn't have a property for the host bridge, it
can only deselect ATS for a sub-range of devices. Similarly the DMAR table
(Intel) declares that ATS is supported either by the whole PCIe domain or
for sub-ranges of devices. I selected ats_supported at the bridge by
default since IVRS needs it and DMAR has its own fine-grained ATS support
configuration.

I'm still not sure this is the right approach, given that the
ats_supported bridge property doesn't exactly correspond to a firmware
property on all platforms. Maybe the device-tree implementation should
follow the IORT one where each device carries a fwspec property stating
"root-complex supports ATS". But it isn't nice either so I tried a cleaner
implementation (as discussed with Robin back on the ATS-with-SMMUv3 series
[1]).

Thanks,
Jean

[1] https://lore.kernel.org/linux-iommu/c10c7adb-c7f6-f8c6-05cc-f4f143427a2d@arm.com/

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 00/10] PCI/ATS: Device-tree support and other improvements
  2020-02-13 16:50 [PATCH 00/10] PCI/ATS: Device-tree support and other improvements Jean-Philippe Brucker
                   ` (10 preceding siblings ...)
  2020-02-13 16:50 ` [PATCH 11/11] Documentation: Generalize the "pci=noats" boot parameter Jean-Philippe Brucker
@ 2020-02-19 10:42 ` Joerg Roedel
  2020-03-06  9:32 ` Hanjun Guo
  12 siblings, 0 replies; 21+ messages in thread
From: Joerg Roedel @ 2020-02-19 10:42 UTC (permalink / raw)
  To: Jean-Philippe Brucker
  Cc: bhelgaas, will, robh+dt, lorenzo.pieralisi, baolu.lu, linux-doc,
	linux-pci, linux-arm-kernel, devicetree, linux-acpi, iommu,
	corbet, mark.rutland, liviu.dudau, sudeep.holla, guohanjun, rjw,
	lenb, robin.murphy, dwmw2, amurray, frowand.list

Hi Jean-Philippe,


On Thu, Feb 13, 2020 at 05:50:38PM +0100, Jean-Philippe Brucker wrote:
> Jean-Philippe Brucker (11):
>   dt-bindings: PCI: generic: Add ats-supported property
>   PCI: Add ats_supported host bridge flag
>   PCI: OF: Check whether the host bridge supports ATS
>   ACPI/IORT: Check ATS capability in root complex node
>   PCI/ATS: Gather checks into pci_ats_supported()
>   iommu/amd: Use pci_ats_supported()
>   iommu/arm-smmu-v3: Use pci_ats_supported()
>   iommu/vt-d: Use pci_ats_supported()
>   ACPI/IORT: Drop ATS fwspec flag
>   arm64: dts: fast models: Enable PCIe ATS for Base RevC FVP
>   Documentation: Generalize the "pci=noats" boot parameter 

Nice patch-set! Thanks for the consolidation work. I can take it into
the iommu-tree, given that the non-iommu parts get the necessary Acks.


Regards,

	Joerg

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 01/11] dt-bindings: PCI: generic: Add ats-supported property
  2020-02-13 16:50 ` [PATCH 01/11] dt-bindings: PCI: generic: Add ats-supported property Jean-Philippe Brucker
@ 2020-02-19 22:24   ` Rob Herring
  0 siblings, 0 replies; 21+ messages in thread
From: Rob Herring @ 2020-02-19 22:24 UTC (permalink / raw)
  To: Jean-Philippe Brucker
  Cc: bhelgaas, will, robh+dt, lorenzo.pieralisi, joro, baolu.lu,
	linux-doc, linux-pci, linux-arm-kernel, devicetree, linux-acpi,
	iommu, corbet, mark.rutland, liviu.dudau, sudeep.holla,
	guohanjun, rjw, lenb, robin.murphy, dwmw2, amurray, frowand.list

On Thu, 13 Feb 2020 17:50:39 +0100, Jean-Philippe Brucker wrote:
> Add a way for firmware to tell the OS that ATS is supported by the PCI
> root complex. An endpoint with ATS enabled may send Translation Requests
> and Translated Memory Requests, which look just like Normal Memory
> Requests with a non-zero AT field. So a root controller that ignores the
> AT field may simply forward the request to the IOMMU as a Normal Memory
> Request, which could end badly. In any case, the endpoint will be
> unusable.
> 
> The ats-supported property allows the OS to only enable ATS in endpoints
> if the root controller can handle ATS requests. Only add the property to
> pcie-host-ecam-generic for the moment. For non-generic root controllers,
> availability of ATS can be inferred from the compatible string.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  Documentation/devicetree/bindings/pci/host-generic-pci.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 00/10] PCI/ATS: Device-tree support and other improvements
  2020-02-13 16:50 [PATCH 00/10] PCI/ATS: Device-tree support and other improvements Jean-Philippe Brucker
                   ` (11 preceding siblings ...)
  2020-02-19 10:42 ` [PATCH 00/10] PCI/ATS: Device-tree support and other improvements Joerg Roedel
@ 2020-03-06  9:32 ` Hanjun Guo
  12 siblings, 0 replies; 21+ messages in thread
From: Hanjun Guo @ 2020-03-06  9:32 UTC (permalink / raw)
  To: Jean-Philippe Brucker, bhelgaas, will, robh+dt,
	lorenzo.pieralisi, joro, baolu.lu, linux-doc, linux-pci,
	linux-arm-kernel, devicetree, linux-acpi, iommu
  Cc: corbet, mark.rutland, liviu.dudau, sudeep.holla, rjw, lenb,
	robin.murphy, dwmw2, amurray, frowand.list, Linuxarm

On 2020/2/14 0:50, Jean-Philippe Brucker wrote:
> Enable ATS on device-tree based systems, and factor the common ATS
> enablement checks into pci_enable_ats().
> 
> ATS support in PCIe endpoints is discovered through the ATS capability,
> but there is no common method for discovering whether the host bridge
> supports ATS. Each vendor provides their own ACPI method:
> * DMAR (Intel) reports ATS support per domain or per root port.
> * IVRS (AMD) reports negative ATS support for a range of devices.
> * IORT (ARM) reports ATS support for a root complex.

Tested this patch set against 5.6-rc2 on a Kunpeng920 ARM server,
I just confirmed that this patch set didn't break anything in
my box with ACPI booting, PCI devices work as expected, FWIW,

Tested-by: Hanjun Guo <guohanjun@huawei.com>

Thanks
Hanjun


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 04/11] ACPI/IORT: Check ATS capability in root complex node
  2020-02-13 16:50 ` [PATCH 04/11] ACPI/IORT: Check ATS capability in root complex node Jean-Philippe Brucker
@ 2020-03-06  9:37   ` Hanjun Guo
  0 siblings, 0 replies; 21+ messages in thread
From: Hanjun Guo @ 2020-03-06  9:37 UTC (permalink / raw)
  To: Jean-Philippe Brucker, bhelgaas, will, robh+dt,
	lorenzo.pieralisi, joro, baolu.lu, linux-doc, linux-pci,
	linux-arm-kernel, devicetree, linux-acpi, iommu
  Cc: corbet, mark.rutland, liviu.dudau, sudeep.holla, rjw, lenb,
	robin.murphy, dwmw2, amurray, frowand.list

On 2020/2/14 0:50, Jean-Philippe Brucker wrote:
> When initializing a PCI root bridge, copy its "ATS supported" attribute
> into the root bridge.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  drivers/acpi/arm64/iort.c | 27 +++++++++++++++++++++++++++
>  drivers/acpi/pci_root.c   |  3 +++
>  include/linux/acpi_iort.h |  8 ++++++++

Acked-by: Hanjun Guo <guohanjun@huawei.com>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 09/11] ACPI/IORT: Drop ATS fwspec flag
  2020-02-13 16:50 ` [PATCH 09/11] ACPI/IORT: Drop ATS fwspec flag Jean-Philippe Brucker
@ 2020-03-06  9:42   ` Hanjun Guo
  0 siblings, 0 replies; 21+ messages in thread
From: Hanjun Guo @ 2020-03-06  9:42 UTC (permalink / raw)
  To: Jean-Philippe Brucker, bhelgaas, will, robh+dt,
	lorenzo.pieralisi, joro, baolu.lu, linux-doc, linux-pci,
	linux-arm-kernel, devicetree, linux-acpi, iommu
  Cc: corbet, mark.rutland, liviu.dudau, sudeep.holla, rjw, lenb,
	robin.murphy, dwmw2, amurray, frowand.list

On 2020/2/14 0:50, Jean-Philippe Brucker wrote:
> Now that the ats_supported flag is in the host bridge structure where it
> belongs, we can remove it from the per-device fwspec structure.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  drivers/acpi/arm64/iort.c | 11 -----------
>  include/linux/iommu.h     |  4 ----
>  2 files changed, 15 deletions(-)

Acked-by: Hanjun Guo <guohanjun@huawei.com>


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2020-03-06  9:42 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-13 16:50 [PATCH 00/10] PCI/ATS: Device-tree support and other improvements Jean-Philippe Brucker
2020-02-13 16:50 ` [PATCH 01/11] dt-bindings: PCI: generic: Add ats-supported property Jean-Philippe Brucker
2020-02-19 22:24   ` Rob Herring
2020-02-13 16:50 ` [PATCH 02/11] PCI: Add ats_supported host bridge flag Jean-Philippe Brucker
2020-02-15 21:10   ` Bjorn Helgaas
2020-02-17 15:40     ` Jean-Philippe Brucker
2020-02-13 16:50 ` [PATCH 03/11] PCI: OF: Check whether the host bridge supports ATS Jean-Philippe Brucker
2020-02-13 18:26   ` Rob Herring
2020-02-17 12:40     ` Jean-Philippe Brucker
2020-02-13 16:50 ` [PATCH 04/11] ACPI/IORT: Check ATS capability in root complex node Jean-Philippe Brucker
2020-03-06  9:37   ` Hanjun Guo
2020-02-13 16:50 ` [PATCH 05/11] PCI/ATS: Gather checks into pci_ats_supported() Jean-Philippe Brucker
2020-02-13 16:50 ` [PATCH 06/11] iommu/amd: Use pci_ats_supported() Jean-Philippe Brucker
2020-02-13 16:50 ` [PATCH 07/11] iommu/arm-smmu-v3: " Jean-Philippe Brucker
2020-02-13 16:50 ` [PATCH 08/11] iommu/vt-d: " Jean-Philippe Brucker
2020-02-13 16:50 ` [PATCH 09/11] ACPI/IORT: Drop ATS fwspec flag Jean-Philippe Brucker
2020-03-06  9:42   ` Hanjun Guo
2020-02-13 16:50 ` [PATCH 10/11] arm64: dts: fast models: Enable PCIe ATS for Base RevC FVP Jean-Philippe Brucker
2020-02-13 16:50 ` [PATCH 11/11] Documentation: Generalize the "pci=noats" boot parameter Jean-Philippe Brucker
2020-02-19 10:42 ` [PATCH 00/10] PCI/ATS: Device-tree support and other improvements Joerg Roedel
2020-03-06  9:32 ` Hanjun Guo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).