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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Nate,

On 2016/9/13 16:14, Nate Watterson wrote:
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.

As I said in previous email, please use the new version of IORT patch
set :)

Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux