Re: [PATCH v5 14/14] drivers: acpi: iort: introduce iort_iommu_configure

From: Nate Watterson
Date: Tue Sep 13 2016 - 04:14:30 EST

On 2016-09-09 10:23, Lorenzo Pieralisi wrote:
DT based systems have a generic kernel API to configure IOMMUs
for devices (ie of_iommu_configure()).

On ARM based ACPI systems, the of_iommu_configure() equivalent can
be implemented atop ACPI IORT kernel API, with the corresponding
functions to map device identifiers to IOMMUs and retrieve the
corresponding IOMMU operations necessary for DMA operations set-up.

By relying on the iommu_fwspec generic kernel infrastructure,
implement the IORT based IOMMU configuration for ARM ACPI systems
and hook it up in the ACPI kernel layer that implements DMA
configuration for a device.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
Cc: Hanjun Guo <hanjun.guo@xxxxxxxxxx>
Cc: Tomasz Nowicki <tn@xxxxxxxxxxxx>
Cc: "Rafael J. Wysocki" <rjw@xxxxxxxxxxxxx>
---
drivers/acpi/arm64/iort.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++
drivers/acpi/scan.c | 7 +++-
include/linux/acpi_iort.h | 6 +++
3 files changed, 108 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 7c68eb4..55a4ae9 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -19,6 +19,7 @@
#define pr_fmt(fmt) "ACPI: IORT: " fmt

#include <linux/acpi_iort.h>
+#include <linux/iommu-fwspec.h>
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/pci.h>
@@ -27,6 +28,8 @@

#define IORT_TYPE_MASK(type) (1 << (type))
#define IORT_MSI_TYPE (1 << ACPI_IORT_NODE_ITS_GROUP)
+#define IORT_IOMMU_TYPE ((1 << ACPI_IORT_NODE_SMMU) | \
+ (1 << ACPI_IORT_NODE_SMMU_V3))

struct iort_its_msi_chip {
struct list_head list;
@@ -467,6 +470,99 @@ struct irq_domain *iort_get_device_domain(struct
device *dev, u32 req_id)
return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
}

+static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
+{
+ u32 *rid = data;
+
+ *rid = alias;
+ return 0;
+}
+
+static int arm_smmu_iort_xlate(struct device *dev, u32 streamid,
+ struct fwnode_handle *fwnode)
+{
+ int ret = iommu_fwspec_init(dev, fwnode);
+
+ if (!ret)
+ ret = iommu_fwspec_add_ids(dev, &streamid, 1);
+
+ return ret;
+}
+
+static const struct iommu_ops *iort_iommu_xlate(struct device *dev,
+ struct acpi_iort_node *node,
+ u32 streamid)
+{
+ struct fwnode_handle *iort_fwnode = NULL;
+ int ret;
+
+ if (node) {
+ iort_fwnode = iort_get_fwnode(node);
+ if (!iort_fwnode)
+ return NULL;
+
+ ret = arm_smmu_iort_xlate(dev, streamid,
+ iort_fwnode);
+ if (!ret)
+ return fwspec_iommu_get_ops(iort_fwnode);
+ }
+
+ return NULL;
+}
+
+/**
+ * iort_iommu_configure - Set-up IOMMU configuration for a device.
+ *
+ * @dev: device to configure
+ *
+ * Returns: iommu_ops pointer on configuration success
+ * NULL on configuration failure
+ */
+const struct iommu_ops *iort_iommu_configure(struct device *dev)
+{
+ struct acpi_iort_node *node, *parent;
+ const struct iommu_ops *ops = NULL;
+ u32 streamid = 0;
+
+ if (dev_is_pci(dev)) {
+ struct pci_bus *bus = to_pci_dev(dev)->bus;
+ u32 rid;
+
+ pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
+ &rid);
+
+ node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
+ iort_match_node_callback, &bus->dev);
+ if (!node)
+ return NULL;
+
+ parent = iort_node_map_rid(node, rid, &streamid,
+ IORT_IOMMU_TYPE);
+
+ ops = iort_iommu_xlate(dev, parent, streamid);
+
+ } else {
+ int i = 0;
+
+ node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
+ iort_match_node_callback, dev);
+ if (!node)
+ return NULL;
+

Nothing wrong with your code here, but wanted to warn you that there
appears to be a bug in iort_match_node_callback() for NAMED_COMPONENTS.

iort_match_node_callback() {
acpi_status status = AE_NOT_FOUND;
...
case ACPI_IORT_NODE_NAMED_COMPONENT: {
...
status = acpi_get_name(adev->handle, ACPI_FULL_PATHNAME, &buf);
if (ACPI_FAILURE(status)) {
dev_warn(dev, "Can't get device full path name\n");
break;
}

ncomp = (struct acpi_iort_named_component *)node->node_data;
if (!strcmp(ncomp->device_name, buf.pointer))
status = AE_OK;

acpi_os_free(buf.pointer);
break;
}
...
return status;
}

Notice how if strcmp() fails, status remains set to the status of the call
to acpi_get_name() which must have been OK since we would have broken out
of the switch statement otherwise. This is causing all manner of platform
devices not even described in the IORT to get hooked up using the IDs of
the first properly iommu-attached NAMED_COMPONENT device found in the IORT.

+ parent = iort_node_get_id(node, &streamid,
+ IORT_IOMMU_TYPE, i++);
+
+ while (parent) {
+ ops = iort_iommu_xlate(dev, parent, streamid);
+
+ parent = iort_node_get_id(node, &streamid,
+ IORT_IOMMU_TYPE, i++);
+ }
+ }
+
+ return ops;
+}
+
static void __init acpi_iort_register_irq(int hwirq, const char *name,
int trigger,
struct resource *res)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 9614232..7e56a85 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -7,6 +7,7 @@
#include <linux/slab.h>
#include <linux/kernel.h>
#include <linux/acpi.h>
+#include <linux/acpi_iort.h>
#include <linux/signal.h>
#include <linux/kthread.h>
#include <linux/dmi.h>
@@ -1377,11 +1378,15 @@ enum dev_dma_attr acpi_get_dma_attr(struct
acpi_device *adev)
*/
void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
{
+ const struct iommu_ops *iommu;
+
+ iommu = iort_iommu_configure(dev);
+
/*
* Assume dma valid range starts at 0 and covers the whole
* coherent_dma_mask.
*/
- arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, NULL,
+ arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, iommu,
attr == DEV_DMA_COHERENT);
}

diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index 1ed4f8f..167649a 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -36,6 +36,8 @@ struct irq_domain *iort_get_device_domain(struct
device *dev, u32 req_id);
int iort_set_fwnode(struct acpi_iort_node *iort_node,
struct fwnode_handle *fwnode);
struct fwnode_handle *iort_get_fwnode(struct acpi_iort_node *node);
+/* IOMMU interface */
+const struct iommu_ops *iort_iommu_configure(struct device *dev);
#else
static inline void acpi_iort_init(void) { }
static inline u32 iort_msi_map_rid(struct device *dev, u32 req_id)
@@ -49,6 +51,10 @@ static inline int iort_set_fwnode(struct
acpi_iort_node *iort_node,
static inline
struct fwnode_handle *iort_get_fwnode(struct acpi_iort_node *node)
{ return NULL; }
+/* IOMMU interface */
+static inline
+const struct iommu_ops *iort_iommu_configure(struct device *dev)
+{ return NULL; }
#endif

#define IORT_ACPI_DECLARE(name, table_id, fn) \

--
Qualcomm Datacenter Technologies, Inc. on behalf of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux
Foundation Collaborative Project.