Re: [PATCH v5 09/14] drivers: iommu: arm-smmu-v3: add IORT configuration

From: Robin Murphy
Date: Tue Sep 13 2016 - 13:30:30 EST


On 09/09/16 15:23, Lorenzo Pieralisi wrote:
> In ACPI bases systems, in order to be able to create platform
> devices and initialize them for ARM SMMU v3 components, the IORT
> kernel implementation requires a set of static functions to be
> used by the IORT kernel layer to configure platform devices for
> ARM SMMU v3 components.
>
> Add static configuration functions to the IORT kernel layer for
> the ARM SMMU v3 components, so that the ARM SMMU v3 driver can
> initialize its respective platform device by relying on the IORT
> kernel infrastructure and by adding a corresponding ACPI device
> early probe section entry.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
> Cc: Will Deacon <will.deacon@xxxxxxx>
> Cc: Robin Murphy <robin.murphy@xxxxxxx>
> Cc: Joerg Roedel <joro@xxxxxxxxxx>
> ---
> drivers/acpi/arm64/iort.c | 103 +++++++++++++++++++++++++++++++++++++++++++-
> drivers/iommu/arm-smmu-v3.c | 95 +++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 195 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index e0a9b16..a2ad102 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -425,6 +425,95 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id)
> return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
> }
>
> +static void __init acpi_iort_register_irq(int hwirq, const char *name,
> + int trigger,
> + struct resource *res)
> +{
> + int irq = acpi_register_gsi(NULL, hwirq, trigger,
> + ACPI_ACTIVE_HIGH);
> +
> + if (irq < 0) {

irq <= 0 ?

> + pr_err("could not register gsi hwirq %d name [%s]\n", hwirq,
> + name);
> + return;
> + }
> +
> + res->start = irq;
> + res->end = irq;
> + res->flags = IORESOURCE_IRQ;
> + res->name = name;
> +}
> +
> +static int __init arm_smmu_v3_count_resources(struct acpi_iort_node *node)
> +{
> + struct acpi_iort_smmu_v3 *smmu;
> + /* Always present mem resource */
> + int num_res = 1;
> +
> + /* Retrieve SMMUv3 specific data */
> + smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> +
> + if (smmu->event_gsiv)
> + num_res++;
> +
> + if (smmu->pri_gsiv)
> + num_res++;
> +
> + if (smmu->gerr_gsiv)
> + num_res++;
> +
> + if (smmu->sync_gsiv)
> + num_res++;
> +
> + return num_res;
> +}
> +
> +static void __init arm_smmu_v3_init_resources(struct resource *res,
> + struct acpi_iort_node *node)
> +{
> + struct acpi_iort_smmu_v3 *smmu;
> + int num_res = 0;
> +
> + /* Retrieve SMMUv3 specific data */
> + smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> +
> + res[num_res].start = smmu->base_address;
> + res[num_res].end = smmu->base_address + SZ_128K - 1;
> + res[num_res].flags = IORESOURCE_MEM;
> +
> + num_res++;
> +
> + if (smmu->event_gsiv)
> + acpi_iort_register_irq(smmu->event_gsiv, "eventq",
> + ACPI_EDGE_SENSITIVE,
> + &res[num_res++]);
> +
> + if (smmu->pri_gsiv)
> + acpi_iort_register_irq(smmu->pri_gsiv, "priq",
> + ACPI_EDGE_SENSITIVE,
> + &res[num_res++]);
> +
> + if (smmu->gerr_gsiv)
> + acpi_iort_register_irq(smmu->gerr_gsiv, "gerror",
> + ACPI_EDGE_SENSITIVE,
> + &res[num_res++]);
> +
> + if (smmu->sync_gsiv)
> + acpi_iort_register_irq(smmu->sync_gsiv, "cmdq-sync",
> + ACPI_EDGE_SENSITIVE,
> + &res[num_res++]);
> +}
> +
> +static bool __init arm_smmu_v3_is_coherent(struct acpi_iort_node *node)
> +{
> + struct acpi_iort_smmu_v3 *smmu;
> +
> + /* Retrieve SMMUv3 specific data */
> + smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> +
> + return smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE;
> +}
> +
> struct iort_iommu_config {
> const char *name;
> int (*iommu_init)(struct acpi_iort_node *node);
> @@ -434,10 +523,22 @@ struct iort_iommu_config {
> struct acpi_iort_node *node);
> };
>
> +static const struct iort_iommu_config iort_arm_smmu_v3_cfg __initconst = {
> + .name = "arm-smmu-v3",
> + .iommu_is_coherent = arm_smmu_v3_is_coherent,
> + .iommu_count_resources = arm_smmu_v3_count_resources,
> + .iommu_init_resources = arm_smmu_v3_init_resources
> +};
> +
> static __init
> const struct iort_iommu_config *iort_get_iommu_cfg(struct acpi_iort_node *node)
> {
> - return NULL;
> + switch (node->type) {
> + case ACPI_IORT_NODE_SMMU_V3:
> + return &iort_arm_smmu_v3_cfg;
> + default:
> + return NULL;
> + }
> }
>
> /**
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index dbc21e3..9463f3f 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -20,6 +20,8 @@
> * This driver is powered by bad coffee and bombay mix.
> */
>
> +#include <linux/acpi.h>
> +#include <linux/acpi_iort.h>
> #include <linux/delay.h>
> #include <linux/dma-iommu.h>
> #include <linux/err.h>
> @@ -2539,6 +2541,32 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
> return 0;
> }
>
> +#ifdef CONFIG_ACPI
> +static int arm_smmu_device_acpi_probe(struct platform_device *pdev,
> + struct arm_smmu_device *smmu)
> +{
> + struct acpi_iort_smmu_v3 *iort_smmu;
> + struct device *dev = smmu->dev;
> + struct acpi_iort_node *node;
> +
> + node = *(struct acpi_iort_node **)dev_get_platdata(dev);
> +
> + /* Retrieve SMMUv3 specific data */
> + iort_smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
> +
> + if (iort_smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE)
> + smmu->features |= ARM_SMMU_FEAT_COHERENCY;
> +
> + return 0;
> +}
> +#else
> +static int arm_smmu_device_acpi_probe(struct platform_device *pdev,
> + struct arm_smmu_device *smmu)
> +{
> + return -ENODEV;
> +}
> +#endif
> +
> static int arm_smmu_device_dt_probe(struct platform_device *pdev,
> struct arm_smmu_device *smmu)
> {
> @@ -2556,6 +2584,7 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
> struct resource *res;
> struct arm_smmu_device *smmu;
> struct device *dev = &pdev->dev;
> + struct fwnode_handle *fwnode;
>
> smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
> if (!smmu) {
> @@ -2592,7 +2621,10 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
> if (irq > 0)
> smmu->gerr_irq = irq;
>
> - ret = arm_smmu_device_dt_probe(pdev, smmu);
> + if (dev->of_node)
> + ret = arm_smmu_device_dt_probe(pdev, smmu);
> + else
> + ret = arm_smmu_device_acpi_probe(pdev, smmu);
>
> if (ret)
> return ret;
> @@ -2615,8 +2647,12 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> + /* FIXME: DT code path does not set up dev->fwnode pointer */
> + fwnode = dev->of_node ? &dev->of_node->fwnode : dev->fwnode;
> +

You're right, this is getting annoying. For a sample of n=1, I've just
booted a Juno with the below and nothing blew up - I'll do a bit more
homework and probably spin it into a proper patch:
---8<---
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index f39ccd5aa701..f811d2796437 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -142,6 +142,7 @@ struct platform_device *of_device_alloc(struct
device_node *np,
}

dev->dev.of_node = of_node_get(np);
+ dev->dev.fwnode = &np->fwnode;
dev->dev.parent = parent ? : &platform_bus;

if (bus_id)
@@ -241,6 +242,7 @@ static struct amba_device
*of_amba_device_create(struct device_node *node,

/* setup generic device info */
dev->dev.of_node = of_node_get(node);
+ dev->dev.fwnode = &node->fwnode;
dev->dev.parent = parent ? : &platform_bus;
dev->dev.platform_data = platform_data;
if (bus_id)
--->8---

Robin.

> /* And we're up. Go go go! */
> - fwspec_iommu_set_ops(&dev->of_node->fwnode, &arm_smmu_ops);
> + fwspec_iommu_set_ops(fwnode, &arm_smmu_ops);
> +
> #ifdef CONFIG_PCI
> pci_request_acs();
> ret = bus_set_iommu(&pci_bus_type, &arm_smmu_ops);
> @@ -2688,6 +2724,61 @@ static int __init arm_smmu_of_init(struct device_node *np)
> }
> IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3", arm_smmu_of_init);
>
> +#ifdef CONFIG_ACPI
> +static int __init acpi_smmu_v3_init(struct acpi_table_header *table)
> +{
> + struct acpi_iort_node *iort_node, *iort_end;
> + struct acpi_table_iort *iort;
> + struct fwnode_handle *fwnode;
> + int i, ret;
> +
> + /*
> + * table and iort will both point to the start of IORT table, but
> + * have different struct types
> + */
> + iort = (struct acpi_table_iort *)table;
> +
> + /* Get the first IORT node */
> + iort_node = ACPI_ADD_PTR(struct acpi_iort_node, table,
> + iort->node_offset);
> + iort_end = ACPI_ADD_PTR(struct acpi_iort_node, table,
> + table->length);
> +
> + for (i = 0; i < iort->node_count; i++) {
> +
> + if (iort_node >= iort_end) {
> + pr_err("iort node pointer overflows, bad table\n");
> + return -EINVAL;
> + }
> +
> + if (iort_node->type == ACPI_IORT_NODE_SMMU_V3) {
> + ret = arm_smmu_init();
> + if (ret)
> + return ret;
> +
> + fwnode = iommu_alloc_fwnode();
> +
> + if (!fwnode)
> + return -ENOMEM;
> +
> + ret = iort_set_fwnode(iort_node, fwnode);
> + if (ret)
> + goto free;
> + }
> +
> + iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
> + iort_node->length);
> + }
> +
> + return 0;
> +free:
> + iommu_free_fwnode(fwnode);
> + return ret;
> +
> +}
> +IORT_ACPI_DECLARE(arm_smmu_v3, ACPI_SIG_IORT, acpi_smmu_v3_init);
> +#endif
> +
> MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
> MODULE_AUTHOR("Will Deacon <will.deacon@xxxxxxx>");
> MODULE_LICENSE("GPL v2");
>