2016-04-14 17:26:28

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [RFC PATCH 00/11] ACPI IORT ARM SMMU support

The ACPI IORT table provides information that allows instantiating
ARM SMMU devices and carrying out id mappings between components on
ARM based systems (devices, IOMMUs, interrupt controllers).

http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf

Building on basic IORT support, available through this posting:

https://marc.info/?l=linux-acpi&m=145976009630179&w=2

this patchset enables ARM SMMU support on ACPI systems.

Most of the code is aimed at building the required generic ACPI
infrastructure to create and enable IOMMU components and to bring
the IOMMU infrastructure for ACPI on par with DT, which is going to
make ARM SMMUv3 and future IOMMU components easier to integrate.

PATCH [1] fixes a warning caused by a missing forward structure
declaration.

PATCH [2] provides IORT support for registering IOMMU components.

PATCH [3] adds a FWNODE_IOMMU type to the struct fwnode_handle type.
It is required to attach a fwnode identifier to platform
devices allocated/detected through IORT tables entries;
IOMMU devices have to have an identifier to look them up
eg IOMMU core layer carrying out id translation. This can be
done through a fwnode_handle (ie IOMMU platform devices created
out of IORT tables are not ACPI devices hence they can't be
allocated as such, otherwise they would have a fwnode_handle of
type FWNODE_ACPI). This patch requires discussion and it is key
to the RFC.

PATCH [4] makes use of the ACPI early probing API to add a linker script
section for probing devices via IORT ACPI kernel code.

PATCH [5] refactors the ARM SMMU driver so that the init functions are
split in a way that groups together code that probes through DT
and code that carries out HW registers FW agnostic probing, in
preparation for adding the ACPI probing path.

PATCH [6] makes the of_xlate() interface for IOMMUs DT agnostic by
changing its API and making it work on ACPI systems too
through the introduction of a generic IOMMU FW translation
specifier.

PATCH [7] implements ARM SMMU streamid mapping based on the previously
introduced infrastructure.

PATCH [8] enhances IORT kernel code to implement the full set of mappings
allowed through the ACPI IORT table.

PATCH [9] implements the of_dma_configure() API in ACPI world -
acpi_dma_configure() - and patches PCI and ACPI core code to start
making use of it.

PATCH [10] implements and enables code for probing the ARM SMMU with ACPI,
building on top of the previously introduced generic infrastructure.

PATCH [11] is a mechanical conversion of IRQ domain infrastructure to
generalize its translation capabilities for other kernel
subsystems, and it is there to prove that the approach taken
to implement IOMMU translation in a DT agnostic way would lead
to structure data duplication that may be deemed unnecessary
and can therefore be avoided.

This patchset is built on top and depends on these two patch series:

R.Murphy "Generic DT bindings for PCI and ARM SMMU"
https://marc.info/?l=linux-arm-kernel&m=145675372917701&w=2

T.Nowicki "Introduce ACPI world to ITS irqchip"
https://marc.info/?l=linux-acpi&m=145976009630179&w=2

Tested on Juno-r2 board with both DT and ACPI probing paths.

Lorenzo Pieralisi (11):
drivers: acpi: iort: fix struct pci_dev compiler warnings
drivers: acpi: iort: add support for IOMMU registration
drivers: iommu: add FWNODE_IOMMU fwnode type
drivers: acpi: iort: introduce linker section for IORT entries probing
drivers: iommu: arm-smmu: split probe functions into DT/generic
portions
drivers: iommu: make of_xlate() interface DT agnostic
drivers: iommu: arm-smmu: allow ACPI based streamid translation
drivers: acpi: iort: enhance mapping API
drivers: acpi: implement acpi_dma_configure
drivers: iommu: arm-smmu: implement ACPI probing
drivers: irqchip: make struct irq_fwspec generic

Documentation/IRQ-domain.txt | 2 +-
arch/arm/mach-exynos/suspend.c | 6 +-
arch/arm/mach-imx/gpc.c | 6 +-
arch/arm/mach-omap2/omap-wakeupgen.c | 6 +-
drivers/acpi/glue.c | 4 +-
drivers/acpi/gsi.c | 2 +-
drivers/acpi/iort.c | 188 ++++++++++++++++++-
drivers/acpi/scan.c | 29 +++
drivers/gpio/gpio-xgene-sb.c | 8 +-
drivers/iommu/arm-smmu.c | 346 ++++++++++++++++++++++++++++++-----
drivers/iommu/exynos-iommu.c | 11 +-
drivers/iommu/mtk_iommu.c | 13 +-
drivers/iommu/of_iommu.c | 8 +-
drivers/irqchip/irq-alpine-msi.c | 2 +-
drivers/irqchip/irq-crossbar.c | 6 +-
drivers/irqchip/irq-gic-v2m.c | 2 +-
drivers/irqchip/irq-gic-v3-its.c | 2 +-
drivers/irqchip/irq-gic-v3.c | 4 +-
drivers/irqchip/irq-gic.c | 4 +-
drivers/irqchip/irq-imx-gpcv2.c | 6 +-
drivers/irqchip/irq-mbigen.c | 4 +-
drivers/irqchip/irq-mips-gic.c | 2 +-
drivers/irqchip/irq-mtk-sysirq.c | 6 +-
drivers/irqchip/irq-mvebu-odmi.c | 2 +-
drivers/irqchip/irq-nvic.c | 4 +-
drivers/irqchip/irq-tegra.c | 6 +-
drivers/irqchip/irq-vf610-mscm-ir.c | 6 +-
drivers/of/base.c | 24 +++
drivers/pci/probe.c | 3 +-
include/acpi/acpi_bus.h | 2 +
include/asm-generic/vmlinux.lds.h | 1 +
include/linux/acpi.h | 5 +
include/linux/fwnode.h | 21 +++
include/linux/iommu.h | 31 +++-
include/linux/iort.h | 18 ++
include/linux/irqdomain.h | 22 +--
include/linux/of.h | 7 +
kernel/irq/irqdomain.c | 17 +-
38 files changed, 692 insertions(+), 144 deletions(-)

--
2.6.4


2016-04-14 17:23:11

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [RFC PATCH 01/11] drivers: acpi: iort: fix struct pci_dev compiler warnings

When the kernel is configured with no IORT support the compilation
issues the following warnings:

In file included from drivers/irqchip/irq-gic-v3-its-pci-msi.c:19:0:
include/linux/iort.h:28:33: warning: 'struct pci_dev' declared inside
parameter list
u32 iort_pci_get_msi_rid(struct pci_dev *pdev, u32 req_id);
^
include/linux/iort.h:28:33: warning: its scope is only this definition
or declaration, which is probably not what you want
include/linux/iort.h:29:50: warning: 'struct pci_dev' declared inside
parameter list
struct fwnode_handle *iort_pci_get_domain(struct pci_dev *pdev, u32
req_id);

This patch fixes the warnings with a struct pci_dev forward declaration
in the IORT header file.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Tomasz Nowicki <[email protected]>
---
include/linux/iort.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/include/linux/iort.h b/include/linux/iort.h
index 7441941..b15fe1a 100644
--- a/include/linux/iort.h
+++ b/include/linux/iort.h
@@ -21,6 +21,7 @@

#include <linux/acpi.h>

+struct pci_dev;
struct fwnode_handle;
int iort_register_domain_token(int trans_id, struct fwnode_handle *fw_node);
void iort_deregister_domain_token(int trans_id);
--
2.6.4

2016-04-14 17:23:17

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [RFC PATCH 03/11] drivers: iommu: add FWNODE_IOMMU fwnode type

On systems booting with a device tree, every struct device is
associated with a struct device_node, that represents its DT
representation. The device node can be used in generic kernel
contexts (eg IRQ translation, IOMMU streamid mapping), to
retrieve the properties associated with the device and carry
out kernel operation accordingly. Owing to the 1:1 relationship
between the device and its device_node, the device_node can also
be used as a look-up token for the device (eg looking up a device
through its device_node), to retrieve the device in kernel paths
where the device_node is available.

On systems booting with ACPI, the same abstraction provided by
the device_node is required to provide:

- Look-up functionality
- Identify the device in kernel functions that must be firmware
agnostic (eg IOMMU of_xlate() callbacks)

Therefore, mirroring the approach implemented in the IRQ domain
kernel layer, this patch adds an additional fwnode type FWNODE_IOMMU.

This patch also implements a glue kernel layer that allows to
allocate/free FWNODE_IOMMU fwnode_handle structures and associate
them with IOMMU devices.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
---
include/linux/fwnode.h | 1 +
include/linux/iommu.h | 25 +++++++++++++++++++++++++
2 files changed, 26 insertions(+)

diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 8516717..6e10050 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -19,6 +19,7 @@ enum fwnode_type {
FWNODE_ACPI_DATA,
FWNODE_PDATA,
FWNODE_IRQCHIP,
+ FWNODE_IOMMU,
};

struct fwnode_handle {
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index a5c539f..0bba25e 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -37,6 +37,7 @@ struct bus_type;
struct device;
struct iommu_domain;
struct notifier_block;
+struct fwnode_handle;

/* iommu fault flags */
#define IOMMU_FAULT_READ 0x0
@@ -542,4 +543,28 @@ static inline void iommu_device_unlink(struct device *dev, struct device *link)

#endif /* CONFIG_IOMMU_API */

+/* IOMMU fwnode handling */
+static inline bool is_fwnode_iommu(struct fwnode_handle *fwnode)
+{
+ return fwnode && fwnode->type == FWNODE_IOMMU;
+}
+
+static inline struct fwnode_handle *iommu_alloc_fwnode(void)
+{
+ struct fwnode_handle *fwnode;
+
+ fwnode = kzalloc(sizeof(struct fwnode_handle), GFP_KERNEL);
+ fwnode->type = FWNODE_IOMMU;
+
+ return fwnode;
+}
+
+static inline void iommu_free_fwnode(struct fwnode_handle *fwnode)
+{
+ if (WARN_ON(!is_fwnode_iommu(fwnode)))
+ return;
+
+ kfree(fwnode);
+}
+
#endif /* __LINUX_IOMMU_H */
--
2.6.4

2016-04-14 17:23:26

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [RFC PATCH 06/11] drivers: iommu: make of_xlate() interface DT agnostic

On systems booting with ACPI, the IOMMU drivers require the same
kind of id mapping carried out with a DT tree through the of_xlate()
API in order to map devices identifiers to IOMMU ones.

On ACPI systems, since DT nodes are not present (ie struct
device.of_node == NULL), to identify the device requiring the translation
the struct device_node (and the structure used to pass translation
information - struct of_phandle_args - that contains a struct device_node)
cannot be used, so a generic translation structure to be used for IOMMU
mapping should be defined, based on the firmware agnostic fwnode_handle
type.

This patch mechanically refactors/renames the of_xlate API to make
it DT agnostic, by declaring a new type (struct iommu_fwspec), that
allows the kernel to pass a device identifier (fwnode - which can
represent either a DT node or an IOMMU FW node) and by changing the
of_xlate signature so that it does not take anymore the DT specific
of_phandle_args argument and replaces it with the DT agnostic
iommu_fwspec one.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Matthias Brugger <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Hanjun Guo <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Tomasz Nowicki <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Marek Szyprowski <[email protected]>
---
drivers/iommu/arm-smmu.c | 12 +++++++-----
drivers/iommu/exynos-iommu.c | 11 +++++++----
drivers/iommu/mtk_iommu.c | 13 ++++++++-----
drivers/iommu/of_iommu.c | 20 ++++++++++++++++++--
include/linux/iommu.h | 24 ++++++++++++++++++++----
5 files changed, 60 insertions(+), 20 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 6c42770..84bcff7 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1440,18 +1440,20 @@ out_unlock:
return ret;
}

-static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
+static int arm_smmu_fw_xlate(struct device *dev, struct iommu_fwspec *args)
{
struct arm_smmu_device *smmu;
- struct platform_device *smmu_pdev;
+ struct platform_device *smmu_pdev = NULL;
+
+ if (is_of_node(args->fwnode))
+ smmu_pdev = of_find_device_by_node(to_of_node(args->fwnode));

- smmu_pdev = of_find_device_by_node(args->np);
if (!smmu_pdev)
return -ENODEV;

smmu = platform_get_drvdata(smmu_pdev);

- return arm_smmu_add_dev_streamid(smmu, dev, args->args[0]);
+ return arm_smmu_add_dev_streamid(smmu, dev, args->param[0]);
}

static struct iommu_ops arm_smmu_ops = {
@@ -1468,7 +1470,7 @@ static struct iommu_ops arm_smmu_ops = {
.device_group = arm_smmu_device_group,
.domain_get_attr = arm_smmu_domain_get_attr,
.domain_set_attr = arm_smmu_domain_set_attr,
- .of_xlate = arm_smmu_of_xlate,
+ .fw_xlate = arm_smmu_fw_xlate,
.pgsize_bitmap = -1UL, /* Restricted during device attach */
};

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 5ecc86c..84ff5bb 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1250,13 +1250,16 @@ static void exynos_iommu_remove_device(struct device *dev)
iommu_group_remove_device(dev);
}

-static int exynos_iommu_of_xlate(struct device *dev,
- struct of_phandle_args *spec)
+static int exynos_iommu_fw_xlate(struct device *dev,
+ struct iommu_fwspec *args)
{
struct exynos_iommu_owner *owner = dev->archdata.iommu;
- struct platform_device *sysmmu = of_find_device_by_node(spec->np);
+ struct platform_device *sysmmu = NULL;
struct sysmmu_drvdata *data;

+ if (is_of_node(args->fwnode))
+ sysmmu = of_find_device_by_node(to_of_node(args->fwnode));
+
if (!sysmmu)
return -ENODEV;

@@ -1290,7 +1293,7 @@ static struct iommu_ops exynos_iommu_ops = {
.add_device = exynos_iommu_add_device,
.remove_device = exynos_iommu_remove_device,
.pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE,
- .of_xlate = exynos_iommu_of_xlate,
+ .fw_xlate = exynos_iommu_fw_xlate,
};

static bool init_done;
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 929a66a..e08dc0a 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -436,20 +436,23 @@ static struct iommu_group *mtk_iommu_device_group(struct device *dev)
return data->m4u_group;
}

-static int mtk_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
+static int mtk_iommu_fw_xlate(struct device *dev, struct iommu_fwspec *args)
{
struct mtk_iommu_client_priv *head, *priv, *next;
struct platform_device *m4updev;

- if (args->args_count != 1) {
+ if (!is_of_node(args->fwnode))
+ return -ENODEV;
+
+ if (args->param_count != 1) {
dev_err(dev, "invalid #iommu-cells(%d) property for IOMMU\n",
- args->args_count);
+ args->param_count);
return -EINVAL;
}

if (!dev->archdata.iommu) {
/* Get the m4u device */
- m4updev = of_find_device_by_node(args->np);
+ m4updev = of_find_device_by_node(to_of_node(args->fwnode));
of_node_put(args->np);
if (WARN_ON(!m4updev))
return -EINVAL;
@@ -469,7 +472,7 @@ static int mtk_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
if (!priv)
goto err_free_mem;

- priv->mtk_m4u_id = args->args[0];
+ priv->mtk_m4u_id = args->param[0];
list_add_tail(&priv->client, &head->client);

return 0;
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 910826c..331dd78 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -135,6 +135,18 @@ struct iommu_ops *of_iommu_get_ops(struct device_node *np)
return ops;
}

+static void of_phandle_args_to_fwspec(struct of_phandle_args *iommu_data,
+ struct iommu_fwspec *fwspec)
+{
+ int i;
+
+ fwspec->fwnode = iommu_data->np ? &iommu_data->np->fwnode : NULL;
+ fwspec->param_count = iommu_data->args_count;
+
+ for (i = 0; i < iommu_data->args_count; i++)
+ fwspec->param[i] = iommu_data->args[i];
+}
+
static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
{
struct of_phandle_args *iommu_spec = data;
@@ -148,6 +160,7 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
struct device_node *master_np)
{
struct of_phandle_args iommu_spec;
+ struct iommu_fwspec fwspec;
struct device_node *np = NULL;
struct iommu_ops *ops = NULL;
int idx = 0;
@@ -170,8 +183,9 @@ struct iommu_ops *of_iommu_configure(struct device *dev,

iommu_spec.np = np;
iommu_spec.args_count = 1;
+ of_phandle_args_to_fwspec(&iommu_spec, &fwspec);
ops = of_iommu_get_ops(np);
- if (!ops || !ops->of_xlate || ops->of_xlate(dev, &iommu_spec))
+ if (!ops || !ops->fw_xlate || ops->fw_xlate(dev, &fwspec))
goto err_put_node;

of_node_put(np);
@@ -189,7 +203,9 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
np = iommu_spec.np;
ops = of_iommu_get_ops(np);

- if (!ops || !ops->of_xlate || ops->of_xlate(dev, &iommu_spec))
+ of_phandle_args_to_fwspec(&iommu_spec, &fwspec);
+
+ if (!ops || !ops->fw_xlate || ops->fw_xlate(dev, &fwspec))
goto err_put_node;

of_node_put(np);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 0bba25e..5184d81 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -85,6 +85,24 @@ struct iommu_domain {
void *iova_cookie;
};

+#define IOMMU_MAP_SPEC_PARAMS 16
+
+/**
+ * struct iommu_fwspec - generic IOMMU specifier structure
+ *
+ * @fwnode: Pointer to a firmware-specific descriptor
+ * @param_count: Number of device-specific parameters
+ * @param: Device-specific parameters
+ *
+ * This structure, directly modeled after of_phandle_args, is used to
+ * pass a device-specific description of an IOMMU mapping.
+ */
+struct iommu_fwspec {
+ struct fwnode_handle *fwnode;
+ int param_count;
+ u32 param[IOMMU_MAP_SPEC_PARAMS];
+};
+
enum iommu_cap {
IOMMU_CAP_CACHE_COHERENCY, /* IOMMU can enforce cache coherent DMA
transactions */
@@ -155,7 +173,7 @@ struct iommu_dm_region {
* @domain_window_disable: Disable a particular window for a domain
* @domain_set_windows: Set the number of windows for a domain
* @domain_get_windows: Return the number of windows for a domain
- * @of_xlate: add OF master IDs to iommu grouping
+ * @fw_xlate: add FW master IDs to iommu grouping
* @pgsize_bitmap: bitmap of supported page sizes
* @priv: per-instance data private to the iommu driver
*/
@@ -196,9 +214,7 @@ struct iommu_ops {
/* Get the number of windows per domain */
u32 (*domain_get_windows)(struct iommu_domain *domain);

-#ifdef CONFIG_OF_IOMMU
- int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
-#endif
+ int (*fw_xlate)(struct device *dev, struct iommu_fwspec *args);

unsigned long pgsize_bitmap;
void *priv;
--
2.6.4

2016-04-14 17:23:31

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [RFC PATCH 08/11] drivers: acpi: iort: enhance mapping API

The current IORT API works only for mappings that have the GIC ITS
as a sink node and does not allow mapping components other than
PCI root complexes.

This patch enhances the IORT API to allow any mapping permitted by
the IORT specifications, inclusive of IORT translations for named
components.

Based on prior work by Hanjun Guo <[email protected]>.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Hanjun Guo <[email protected]>
Cc: Tomasz Nowicki <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
---
drivers/acpi/iort.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 53 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
index 080888a..2b5ce65 100644
--- a/drivers/acpi/iort.c
+++ b/drivers/acpi/iort.c
@@ -182,15 +182,61 @@ iort_scan_node(enum acpi_iort_node_type type,
return NULL;
}

+static struct acpi_iort_node *
+iort_find_parent_node(struct acpi_iort_node *node, u8 type)
+{
+ struct acpi_iort_id_mapping *id;
+
+ if (!node || !node->mapping_offset || !node->mapping_count)
+ return NULL;
+
+ id = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
+ node->mapping_offset);
+
+ if (!id->output_reference) {
+ pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n",
+ node, node->type);
+ return NULL;
+ }
+
+ node = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
+ id->output_reference);
+
+ return node->type == type ? node : NULL;
+}
+
static acpi_status
iort_find_dev_callback(struct acpi_iort_node *node, void *context)
{
- struct acpi_iort_root_complex *pci_rc;
struct device *dev = context;
- struct pci_bus *bus;

switch (node->type) {
- case ACPI_IORT_NODE_PCI_ROOT_COMPLEX:
+ case ACPI_IORT_NODE_NAMED_COMPONENT: {
+ struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+ struct acpi_iort_named_component *ncomp;
+ struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
+
+ if (!adev)
+ break;
+
+ ncomp = (struct acpi_iort_named_component *)node->node_data;
+
+ if (ACPI_FAILURE(acpi_get_name(adev->handle,
+ ACPI_FULL_PATHNAME, &buffer))) {
+ pr_warn("Can't get device full path name\n");
+ break;
+ }
+
+ if (!strcmp(ncomp->device_name, (char *)buffer.pointer))
+ return AE_OK;
+
+ break;
+ }
+
+ case ACPI_IORT_NODE_PCI_ROOT_COMPLEX: {
+ struct acpi_iort_root_complex *pci_rc;
+ struct pci_bus *bus;
+
bus = to_pci_bus(dev);
pci_rc = (struct acpi_iort_root_complex *)node->node_data;

@@ -204,20 +250,21 @@ iort_find_dev_callback(struct acpi_iort_node *node, void *context)

break;
}
+ }

return AE_NOT_FOUND;
}

static struct acpi_iort_node *
iort_dev_map_rid(struct acpi_iort_node *node, u32 rid_in,
- u32 *rid_out)
+ u32 *rid_out, u8 type)
{

if (!node)
goto out;

/* Go upstream */
- while (node->type != ACPI_IORT_NODE_ITS_GROUP) {
+ while (node->type != type) {
struct acpi_iort_id_mapping *id;
int i, found = 0;

@@ -283,7 +330,7 @@ iort_its_find_node_and_map_rid(struct pci_dev *pdev, u32 req_id, u32 *dev_id)
return NULL;
}

- return iort_dev_map_rid(node, req_id, dev_id);
+ return iort_dev_map_rid(node, req_id, dev_id, ACPI_IORT_NODE_ITS_GROUP);
}

/**
--
2.6.4

2016-04-14 17:23:45

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [RFC PATCH 11/11] drivers: irqchip: make struct irq_fwspec generic

The struct irq_fwspec is used in IRQ domain core code to carry out
IRQ translations (on both DT and ACPI systems) but it has nothing
IRQ specific in it, hence it can be reused by other kernel subsystems
(ie IOMMU) to carry out translation mappings as they deem fit.

This patch mechanically converts the struct irq_fwspec to a generic
struct fwspec available to all kernel subsystems that envisage a use
for it, and generalizes the IRQ domain helper (that is not IRQ domain
specific) to translate struct of_phandle_args to a struct fwspec.

Since the struct fwspec is subsystem agnostic, the struct iommu_fwspec
can be converted to it too in all kernel components currently making
use of it.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Marc Zyngier <[email protected]>
---
Documentation/IRQ-domain.txt | 2 +-
arch/arm/mach-exynos/suspend.c | 6 +++---
arch/arm/mach-imx/gpc.c | 6 +++---
arch/arm/mach-omap2/omap-wakeupgen.c | 6 +++---
drivers/acpi/gsi.c | 2 +-
drivers/acpi/iort.c | 2 +-
drivers/gpio/gpio-xgene-sb.c | 8 ++++----
drivers/iommu/arm-smmu.c | 2 +-
drivers/iommu/exynos-iommu.c | 2 +-
drivers/iommu/mtk_iommu.c | 2 +-
drivers/iommu/of_iommu.c | 14 +-------------
drivers/irqchip/irq-alpine-msi.c | 2 +-
drivers/irqchip/irq-crossbar.c | 6 +++---
drivers/irqchip/irq-gic-v2m.c | 2 +-
drivers/irqchip/irq-gic-v3-its.c | 2 +-
drivers/irqchip/irq-gic-v3.c | 4 ++--
drivers/irqchip/irq-gic.c | 4 ++--
drivers/irqchip/irq-imx-gpcv2.c | 6 +++---
drivers/irqchip/irq-mbigen.c | 4 ++--
drivers/irqchip/irq-mips-gic.c | 2 +-
drivers/irqchip/irq-mtk-sysirq.c | 6 +++---
drivers/irqchip/irq-mvebu-odmi.c | 2 +-
drivers/irqchip/irq-nvic.c | 4 ++--
drivers/irqchip/irq-tegra.c | 6 +++---
drivers/irqchip/irq-vf610-mscm-ir.c | 6 +++---
drivers/of/base.c | 24 ++++++++++++++++++++++++
include/linux/fwnode.h | 20 ++++++++++++++++++++
include/linux/iommu.h | 20 +-------------------
include/linux/irqdomain.h | 22 ++--------------------
include/linux/of.h | 7 +++++++
kernel/irq/irqdomain.c | 17 +++--------------
31 files changed, 105 insertions(+), 113 deletions(-)

diff --git a/Documentation/IRQ-domain.txt b/Documentation/IRQ-domain.txt
index 8d990bd..c9085e5 100644
--- a/Documentation/IRQ-domain.txt
+++ b/Documentation/IRQ-domain.txt
@@ -32,7 +32,7 @@ top of the irq_alloc_desc*() API. An irq_domain to manage mapping is
preferred over interrupt controller drivers open coding their own
reverse mapping scheme.

-irq_domain also implements translation from an abstract irq_fwspec
+irq_domain also implements translation from an abstract fwspec
structure to hwirq numbers (Device Tree and ACPI GSI so far), and can
be easily extended to support other IRQ topology data sources.

diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
index fee2b00..40a48c0 100644
--- a/arch/arm/mach-exynos/suspend.c
+++ b/arch/arm/mach-exynos/suspend.c
@@ -179,7 +179,7 @@ static struct irq_chip exynos_pmu_chip = {
};

static int exynos_pmu_domain_translate(struct irq_domain *d,
- struct irq_fwspec *fwspec,
+ struct fwspec *fwspec,
unsigned long *hwirq,
unsigned int *type)
{
@@ -203,8 +203,8 @@ static int exynos_pmu_domain_alloc(struct irq_domain *domain,
unsigned int virq,
unsigned int nr_irqs, void *data)
{
- struct irq_fwspec *fwspec = data;
- struct irq_fwspec parent_fwspec;
+ struct fwspec *fwspec = data;
+ struct fwspec parent_fwspec;
irq_hw_number_t hwirq;
int i;

diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
index fd87205..5e4aac6 100644
--- a/arch/arm/mach-imx/gpc.c
+++ b/arch/arm/mach-imx/gpc.c
@@ -184,7 +184,7 @@ static struct irq_chip imx_gpc_chip = {
};

static int imx_gpc_domain_translate(struct irq_domain *d,
- struct irq_fwspec *fwspec,
+ struct fwspec *fwspec,
unsigned long *hwirq,
unsigned int *type)
{
@@ -208,8 +208,8 @@ static int imx_gpc_domain_alloc(struct irq_domain *domain,
unsigned int irq,
unsigned int nr_irqs, void *data)
{
- struct irq_fwspec *fwspec = data;
- struct irq_fwspec parent_fwspec;
+ struct fwspec *fwspec = data;
+ struct fwspec parent_fwspec;
irq_hw_number_t hwirq;
int i;

diff --git a/arch/arm/mach-omap2/omap-wakeupgen.c b/arch/arm/mach-omap2/omap-wakeupgen.c
index f397bd6..b74ede2 100644
--- a/arch/arm/mach-omap2/omap-wakeupgen.c
+++ b/arch/arm/mach-omap2/omap-wakeupgen.c
@@ -401,7 +401,7 @@ static struct irq_chip wakeupgen_chip = {
};

static int wakeupgen_domain_translate(struct irq_domain *d,
- struct irq_fwspec *fwspec,
+ struct fwspec *fwspec,
unsigned long *hwirq,
unsigned int *type)
{
@@ -425,8 +425,8 @@ static int wakeupgen_domain_alloc(struct irq_domain *domain,
unsigned int virq,
unsigned int nr_irqs, void *data)
{
- struct irq_fwspec *fwspec = data;
- struct irq_fwspec parent_fwspec;
+ struct fwspec *fwspec = data;
+ struct fwspec parent_fwspec;
irq_hw_number_t hwirq;
int i;

diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
index ee9e0f2..05e8b32 100644
--- a/drivers/acpi/gsi.c
+++ b/drivers/acpi/gsi.c
@@ -54,7 +54,7 @@ EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
int polarity)
{
- struct irq_fwspec fwspec;
+ struct fwspec fwspec;

if (WARN_ON(!acpi_gsi_domain_id)) {
pr_warn("GSI: No registered irqchip, giving up\n");
diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
index b1bb8fb..5151d68 100644
--- a/drivers/acpi/iort.c
+++ b/drivers/acpi/iort.c
@@ -450,7 +450,7 @@ struct iommu_ops *iort_iommu_configure(struct device *dev)
{
struct acpi_iort_node *node, *parent;
struct iommu_ops *ops = NULL;
- struct iommu_fwspec fwspec;
+ struct fwspec fwspec;
struct iort_iommu_node *iommu_node;
u32 rid = 0, devid = 0;

diff --git a/drivers/gpio/gpio-xgene-sb.c b/drivers/gpio/gpio-xgene-sb.c
index 31cbcb8..0c5a2cb 100644
--- a/drivers/gpio/gpio-xgene-sb.c
+++ b/drivers/gpio/gpio-xgene-sb.c
@@ -124,7 +124,7 @@ static struct irq_chip xgene_gpio_sb_irq_chip = {
static int xgene_gpio_sb_to_irq(struct gpio_chip *gc, u32 gpio)
{
struct xgene_gpio_sb *priv = gpiochip_get_data(gc);
- struct irq_fwspec fwspec;
+ struct fwspec fwspec;

if ((gpio < priv->irq_start) ||
(gpio > HWIRQ_TO_GPIO(priv, priv->nirq)))
@@ -169,7 +169,7 @@ static void xgene_gpio_sb_domain_deactivate(struct irq_domain *d,
}

static int xgene_gpio_sb_domain_translate(struct irq_domain *d,
- struct irq_fwspec *fwspec,
+ struct fwspec *fwspec,
unsigned long *hwirq,
unsigned int *type)
{
@@ -187,8 +187,8 @@ static int xgene_gpio_sb_domain_alloc(struct irq_domain *domain,
unsigned int virq,
unsigned int nr_irqs, void *data)
{
- struct irq_fwspec *fwspec = data;
- struct irq_fwspec parent_fwspec;
+ struct fwspec *fwspec = data;
+ struct fwspec parent_fwspec;
struct xgene_gpio_sb *priv = domain->host_data;
irq_hw_number_t hwirq;
unsigned int i;
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 85ab4b7..643a8e3 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1456,7 +1456,7 @@ static struct platform_device *arm_smmu_get_dev(struct fwnode_handle *fwnode)
return dev ? to_platform_device(dev) : NULL;
}

-static int arm_smmu_fw_xlate(struct device *dev, struct iommu_fwspec *args)
+static int arm_smmu_fw_xlate(struct device *dev, struct fwspec *args)
{
struct arm_smmu_device *smmu;
struct platform_device *smmu_pdev = NULL;
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 84ff5bb..b445378 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1251,7 +1251,7 @@ static void exynos_iommu_remove_device(struct device *dev)
}

static int exynos_iommu_fw_xlate(struct device *dev,
- struct iommu_fwspec *args)
+ struct fwspec *args)
{
struct exynos_iommu_owner *owner = dev->archdata.iommu;
struct platform_device *sysmmu = NULL;
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index e08dc0a..459c293 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -436,7 +436,7 @@ static struct iommu_group *mtk_iommu_device_group(struct device *dev)
return data->m4u_group;
}

-static int mtk_iommu_fw_xlate(struct device *dev, struct iommu_fwspec *args)
+static int mtk_iommu_fw_xlate(struct device *dev, struct fwspec *args)
{
struct mtk_iommu_client_priv *head, *priv, *next;
struct platform_device *m4updev;
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 331dd78..bf9757a 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -135,18 +135,6 @@ struct iommu_ops *of_iommu_get_ops(struct device_node *np)
return ops;
}

-static void of_phandle_args_to_fwspec(struct of_phandle_args *iommu_data,
- struct iommu_fwspec *fwspec)
-{
- int i;
-
- fwspec->fwnode = iommu_data->np ? &iommu_data->np->fwnode : NULL;
- fwspec->param_count = iommu_data->args_count;
-
- for (i = 0; i < iommu_data->args_count; i++)
- fwspec->param[i] = iommu_data->args[i];
-}
-
static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
{
struct of_phandle_args *iommu_spec = data;
@@ -160,7 +148,7 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
struct device_node *master_np)
{
struct of_phandle_args iommu_spec;
- struct iommu_fwspec fwspec;
+ struct fwspec fwspec;
struct device_node *np = NULL;
struct iommu_ops *ops = NULL;
int idx = 0;
diff --git a/drivers/irqchip/irq-alpine-msi.c b/drivers/irqchip/irq-alpine-msi.c
index 2538425..1514155 100644
--- a/drivers/irqchip/irq-alpine-msi.c
+++ b/drivers/irqchip/irq-alpine-msi.c
@@ -119,7 +119,7 @@ static struct irq_chip middle_irq_chip = {
static int alpine_msix_gic_domain_alloc(struct irq_domain *domain,
unsigned int virq, int sgi)
{
- struct irq_fwspec fwspec;
+ struct fwspec fwspec;
struct irq_data *d;
int ret;

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 75573fa..377ad04 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -78,7 +78,7 @@ static struct irq_chip crossbar_chip = {
static int allocate_gic_irq(struct irq_domain *domain, unsigned virq,
irq_hw_number_t hwirq)
{
- struct irq_fwspec fwspec;
+ struct fwspec fwspec;
int i;
int err;

@@ -115,7 +115,7 @@ static int allocate_gic_irq(struct irq_domain *domain, unsigned virq,
static int crossbar_domain_alloc(struct irq_domain *d, unsigned int virq,
unsigned int nr_irqs, void *data)
{
- struct irq_fwspec *fwspec = data;
+ struct fwspec *fwspec = data;
irq_hw_number_t hwirq;
int i;

@@ -170,7 +170,7 @@ static void crossbar_domain_free(struct irq_domain *domain, unsigned int virq,
}

static int crossbar_domain_translate(struct irq_domain *d,
- struct irq_fwspec *fwspec,
+ struct fwspec *fwspec,
unsigned long *hwirq,
unsigned int *type)
{
diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index 28f047c..bd202f7 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -118,7 +118,7 @@ static int gicv2m_irq_gic_domain_alloc(struct irq_domain *domain,
unsigned int virq,
irq_hw_number_t hwirq)
{
- struct irq_fwspec fwspec;
+ struct fwspec fwspec;
struct irq_data *d;
int err;

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index a277c9b..320cd4b 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1272,7 +1272,7 @@ static int its_irq_gic_domain_alloc(struct irq_domain *domain,
unsigned int virq,
irq_hw_number_t hwirq)
{
- struct irq_fwspec fwspec;
+ struct fwspec fwspec;

if (irq_domain_get_of_node(domain->parent)) {
fwspec.fwnode = domain->parent->fwnode;
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 7708941..1ed02d8 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -744,7 +744,7 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
}

static int gic_irq_domain_translate(struct irq_domain *d,
- struct irq_fwspec *fwspec,
+ struct fwspec *fwspec,
unsigned long *hwirq,
unsigned int *type)
{
@@ -788,7 +788,7 @@ static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
int i, ret;
irq_hw_number_t hwirq;
unsigned int type = IRQ_TYPE_NONE;
- struct irq_fwspec *fwspec = arg;
+ struct fwspec *fwspec = arg;

ret = gic_irq_domain_translate(domain, fwspec, &hwirq, &type);
if (ret)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 282344b..e59862f 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -929,7 +929,7 @@ static void gic_irq_domain_unmap(struct irq_domain *d, unsigned int irq)
}

static int gic_irq_domain_translate(struct irq_domain *d,
- struct irq_fwspec *fwspec,
+ struct fwspec *fwspec,
unsigned long *hwirq,
unsigned int *type)
{
@@ -988,7 +988,7 @@ static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
int i, ret;
irq_hw_number_t hwirq;
unsigned int type = IRQ_TYPE_NONE;
- struct irq_fwspec *fwspec = arg;
+ struct fwspec *fwspec = arg;

ret = gic_irq_domain_translate(domain, fwspec, &hwirq, &type);
if (ret)
diff --git a/drivers/irqchip/irq-imx-gpcv2.c b/drivers/irqchip/irq-imx-gpcv2.c
index 15af9a9..cf080c2 100644
--- a/drivers/irqchip/irq-imx-gpcv2.c
+++ b/drivers/irqchip/irq-imx-gpcv2.c
@@ -151,7 +151,7 @@ static struct irq_chip gpcv2_irqchip_data_chip = {
};

static int imx_gpcv2_domain_translate(struct irq_domain *d,
- struct irq_fwspec *fwspec,
+ struct fwspec *fwspec,
unsigned long *hwirq,
unsigned int *type)
{
@@ -175,8 +175,8 @@ static int imx_gpcv2_domain_alloc(struct irq_domain *domain,
unsigned int irq, unsigned int nr_irqs,
void *data)
{
- struct irq_fwspec *fwspec = data;
- struct irq_fwspec parent_fwspec;
+ struct fwspec *fwspec = data;
+ struct fwspec parent_fwspec;
irq_hw_number_t hwirq;
unsigned int type;
int err;
diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index d67baa2..4f7010d 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -176,7 +176,7 @@ static void mbigen_write_msg(struct msi_desc *desc, struct msi_msg *msg)
}

static int mbigen_domain_translate(struct irq_domain *d,
- struct irq_fwspec *fwspec,
+ struct fwspec *fwspec,
unsigned long *hwirq,
unsigned int *type)
{
@@ -207,7 +207,7 @@ static int mbigen_irq_domain_alloc(struct irq_domain *domain,
unsigned int nr_irqs,
void *args)
{
- struct irq_fwspec *fwspec = args;
+ struct fwspec *fwspec = args;
irq_hw_number_t hwirq;
unsigned int type;
struct mbigen_device *mgn_chip;
diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index 94a30da..a6990be 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -828,7 +828,7 @@ static int gic_dev_domain_xlate(struct irq_domain *d, struct device_node *ctrlr,
static int gic_dev_domain_alloc(struct irq_domain *d, unsigned int virq,
unsigned int nr_irqs, void *arg)
{
- struct irq_fwspec *fwspec = arg;
+ struct fwspec *fwspec = arg;
struct gic_irq_spec spec = {
.type = GIC_DEVICE,
.hwirq = fwspec->param[1],
diff --git a/drivers/irqchip/irq-mtk-sysirq.c b/drivers/irqchip/irq-mtk-sysirq.c
index 63ac73b..affc5f6 100644
--- a/drivers/irqchip/irq-mtk-sysirq.c
+++ b/drivers/irqchip/irq-mtk-sysirq.c
@@ -68,7 +68,7 @@ static struct irq_chip mtk_sysirq_chip = {
};

static int mtk_sysirq_domain_translate(struct irq_domain *d,
- struct irq_fwspec *fwspec,
+ struct fwspec *fwspec,
unsigned long *hwirq,
unsigned int *type)
{
@@ -93,8 +93,8 @@ static int mtk_sysirq_domain_alloc(struct irq_domain *domain, unsigned int virq,
{
int i;
irq_hw_number_t hwirq;
- struct irq_fwspec *fwspec = arg;
- struct irq_fwspec gic_fwspec = *fwspec;
+ struct fwspec *fwspec = arg;
+ struct fwspec gic_fwspec = *fwspec;

if (fwspec->param_count != 3)
return -EINVAL;
diff --git a/drivers/irqchip/irq-mvebu-odmi.c b/drivers/irqchip/irq-mvebu-odmi.c
index b4d3678..e8ad004 100644
--- a/drivers/irqchip/irq-mvebu-odmi.c
+++ b/drivers/irqchip/irq-mvebu-odmi.c
@@ -79,7 +79,7 @@ static int odmi_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
unsigned int nr_irqs, void *args)
{
struct odmi_data *odmi = NULL;
- struct irq_fwspec fwspec;
+ struct fwspec fwspec;
struct irq_data *d;
unsigned int hwirq, odmin;
int ret;
diff --git a/drivers/irqchip/irq-nvic.c b/drivers/irqchip/irq-nvic.c
index b177710..174b1ed 100644
--- a/drivers/irqchip/irq-nvic.c
+++ b/drivers/irqchip/irq-nvic.c
@@ -49,7 +49,7 @@ nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
}

static int nvic_irq_domain_translate(struct irq_domain *d,
- struct irq_fwspec *fwspec,
+ struct fwspec *fwspec,
unsigned long *hwirq, unsigned int *type)
{
if (WARN_ON(fwspec->param_count < 1))
@@ -65,7 +65,7 @@ static int nvic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
int i, ret;
irq_hw_number_t hwirq;
unsigned int type = IRQ_TYPE_NONE;
- struct irq_fwspec *fwspec = arg;
+ struct fwspec *fwspec = arg;

ret = nvic_irq_domain_translate(domain, fwspec, &hwirq, &type);
if (ret)
diff --git a/drivers/irqchip/irq-tegra.c b/drivers/irqchip/irq-tegra.c
index 50be963..c231ca2 100644
--- a/drivers/irqchip/irq-tegra.c
+++ b/drivers/irqchip/irq-tegra.c
@@ -222,7 +222,7 @@ static struct irq_chip tegra_ictlr_chip = {
};

static int tegra_ictlr_domain_translate(struct irq_domain *d,
- struct irq_fwspec *fwspec,
+ struct fwspec *fwspec,
unsigned long *hwirq,
unsigned int *type)
{
@@ -246,8 +246,8 @@ static int tegra_ictlr_domain_alloc(struct irq_domain *domain,
unsigned int virq,
unsigned int nr_irqs, void *data)
{
- struct irq_fwspec *fwspec = data;
- struct irq_fwspec parent_fwspec;
+ struct fwspec *fwspec = data;
+ struct fwspec parent_fwspec;
struct tegra_ictlr_info *info = domain->host_data;
irq_hw_number_t hwirq;
unsigned int i;
diff --git a/drivers/irqchip/irq-vf610-mscm-ir.c b/drivers/irqchip/irq-vf610-mscm-ir.c
index 56b5e3c..abdd406 100644
--- a/drivers/irqchip/irq-vf610-mscm-ir.c
+++ b/drivers/irqchip/irq-vf610-mscm-ir.c
@@ -130,8 +130,8 @@ static int vf610_mscm_ir_domain_alloc(struct irq_domain *domain, unsigned int vi
{
int i;
irq_hw_number_t hwirq;
- struct irq_fwspec *fwspec = arg;
- struct irq_fwspec parent_fwspec;
+ struct fwspec *fwspec = arg;
+ struct fwspec parent_fwspec;

if (!irq_domain_get_of_node(domain->parent))
return -EINVAL;
@@ -162,7 +162,7 @@ static int vf610_mscm_ir_domain_alloc(struct irq_domain *domain, unsigned int vi
}

static int vf610_mscm_ir_domain_translate(struct irq_domain *d,
- struct irq_fwspec *fwspec,
+ struct fwspec *fwspec,
unsigned long *hwirq,
unsigned int *type)
{
diff --git a/drivers/of/base.c b/drivers/of/base.c
index b299de2..943dd6e 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1690,6 +1690,30 @@ int of_count_phandle_with_args(const struct device_node *np, const char *list_na
EXPORT_SYMBOL(of_count_phandle_with_args);

/**
+ * of_phandle_args_to_fwspec - Convert a struct of_phandle_args to struct
+ * fwspec
+ * @args: of_phandle_args source structure
+ * @fwspec: fwspec specifier destination structure
+ */
+void of_phandle_args_to_fwspec(struct of_phandle_args *args,
+ struct fwspec *fwspec)
+{
+ int i;
+
+ if (args->args_count >= FWSPEC_PARAMS) {
+ pr_warn("phandle arguments count exceeding fwspec params\n");
+ return;
+ }
+
+ fwspec->fwnode = args->np ? &args->np->fwnode : NULL;
+ fwspec->param_count = args->args_count;
+
+ for (i = 0; i < args->args_count; i++)
+ fwspec->param[i] = args->args[i];
+}
+EXPORT_SYMBOL(of_phandle_args_to_fwspec);
+
+/**
* __of_add_property - Add a property to a node without lock operations
*/
int __of_add_property(struct device_node *np, struct property *prop)
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 6e10050..64621bd 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -12,6 +12,8 @@
#ifndef _LINUX_FWNODE_H_
#define _LINUX_FWNODE_H_

+#include <linux/types.h>
+
enum fwnode_type {
FWNODE_INVALID = 0,
FWNODE_OF,
@@ -27,4 +29,22 @@ struct fwnode_handle {
struct fwnode_handle *secondary;
};

+#define FWSPEC_PARAMS 16
+
+/**
+ * struct fwspec - generic FW node specifier structure
+ *
+ * @fwnode: Pointer to a firmware-specific descriptor
+ * @param_count: Number of device-specific parameters
+ * @param: Device-specific parameters
+ *
+ * This structure, directly modeled after of_phandle_args, is used to
+ * pass a fwnode specific description of translation parameters.
+ */
+struct fwspec {
+ struct fwnode_handle *fwnode;
+ int param_count;
+ u32 param[FWSPEC_PARAMS];
+};
+
#endif
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 5184d81..8904ef8 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -85,24 +85,6 @@ struct iommu_domain {
void *iova_cookie;
};

-#define IOMMU_MAP_SPEC_PARAMS 16
-
-/**
- * struct iommu_fwspec - generic IOMMU specifier structure
- *
- * @fwnode: Pointer to a firmware-specific descriptor
- * @param_count: Number of device-specific parameters
- * @param: Device-specific parameters
- *
- * This structure, directly modeled after of_phandle_args, is used to
- * pass a device-specific description of an IOMMU mapping.
- */
-struct iommu_fwspec {
- struct fwnode_handle *fwnode;
- int param_count;
- u32 param[IOMMU_MAP_SPEC_PARAMS];
-};
-
enum iommu_cap {
IOMMU_CAP_CACHE_COHERENCY, /* IOMMU can enforce cache coherent DMA
transactions */
@@ -214,7 +196,7 @@ struct iommu_ops {
/* Get the number of windows per domain */
u32 (*domain_get_windows)(struct iommu_domain *domain);

- int (*fw_xlate)(struct device *dev, struct iommu_fwspec *args);
+ int (*fw_xlate)(struct device *dev, struct fwspec *args);

unsigned long pgsize_bitmap;
void *priv;
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 2aed043..1f240b6 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -43,24 +43,6 @@ struct irq_data;
/* Number of irqs reserved for a legacy isa controller */
#define NUM_ISA_INTERRUPTS 16

-#define IRQ_DOMAIN_IRQ_SPEC_PARAMS 16
-
-/**
- * struct irq_fwspec - generic IRQ specifier structure
- *
- * @fwnode: Pointer to a firmware-specific descriptor
- * @param_count: Number of device-specific parameters
- * @param: Device-specific parameters
- *
- * This structure, directly modeled after of_phandle_args, is used to
- * pass a device-specific description of an interrupt.
- */
-struct irq_fwspec {
- struct fwnode_handle *fwnode;
- int param_count;
- u32 param[IRQ_DOMAIN_IRQ_SPEC_PARAMS];
-};
-
/*
* Should several domains have the same device node, but serve
* different purposes (for example one domain is for PCI/MSI, and the
@@ -110,7 +92,7 @@ struct irq_domain_ops {
unsigned int nr_irqs);
void (*activate)(struct irq_domain *d, struct irq_data *irq_data);
void (*deactivate)(struct irq_domain *d, struct irq_data *irq_data);
- int (*translate)(struct irq_domain *d, struct irq_fwspec *fwspec,
+ int (*translate)(struct irq_domain *d, struct fwspec *fwspec,
unsigned long *out_hwirq, unsigned int *out_type);
#endif
};
@@ -301,7 +283,7 @@ extern void irq_domain_disassociate(struct irq_domain *domain,

extern unsigned int irq_create_mapping(struct irq_domain *host,
irq_hw_number_t hwirq);
-extern unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec);
+extern unsigned int irq_create_fwspec_mapping(struct fwspec *fwspec);
extern void irq_dispose_mapping(unsigned int virq);

/**
diff --git a/include/linux/of.h b/include/linux/of.h
index 7fcb681..451f6e2 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -333,6 +333,8 @@ extern int of_parse_phandle_with_fixed_args(const struct device_node *np,
struct of_phandle_args *out_args);
extern int of_count_phandle_with_args(const struct device_node *np,
const char *list_name, const char *cells_name);
+extern void of_phandle_args_to_fwspec(struct of_phandle_args *irq_data,
+ struct fwspec *fwspec);

extern void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align));
extern int of_alias_get_id(struct device_node *np, const char *stem);
@@ -608,6 +610,11 @@ static inline int of_count_phandle_with_args(struct device_node *np,
return -ENOSYS;
}

+static inline void of_phandle_args_to_fwspec(struct of_phandle_args *irq_data,
+ struct fwspec *fwspec)
+{
+}
+
static inline int of_alias_get_id(struct device_node *np, const char *stem)
{
return -ENOSYS;
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 3a519a0..6e59384 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -537,7 +537,7 @@ int irq_create_strict_mappings(struct irq_domain *domain, unsigned int irq_base,
EXPORT_SYMBOL_GPL(irq_create_strict_mappings);

static int irq_domain_translate(struct irq_domain *d,
- struct irq_fwspec *fwspec,
+ struct fwspec *fwspec,
irq_hw_number_t *hwirq, unsigned int *type)
{
#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
@@ -554,19 +554,8 @@ static int irq_domain_translate(struct irq_domain *d,
return 0;
}

-static void of_phandle_args_to_fwspec(struct of_phandle_args *irq_data,
- struct irq_fwspec *fwspec)
-{
- int i;
-
- fwspec->fwnode = irq_data->np ? &irq_data->np->fwnode : NULL;
- fwspec->param_count = irq_data->args_count;
-
- for (i = 0; i < irq_data->args_count; i++)
- fwspec->param[i] = irq_data->args[i];
-}

-unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
+unsigned int irq_create_fwspec_mapping(struct fwspec *fwspec)
{
struct irq_domain *domain;
irq_hw_number_t hwirq;
@@ -621,7 +610,7 @@ EXPORT_SYMBOL_GPL(irq_create_fwspec_mapping);

unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data)
{
- struct irq_fwspec fwspec;
+ struct fwspec fwspec;

of_phandle_args_to_fwspec(irq_data, &fwspec);
return irq_create_fwspec_mapping(&fwspec);
--
2.6.4

2016-04-14 17:23:37

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [RFC PATCH 10/11] drivers: iommu: arm-smmu: implement ACPI probing

In ACPI world ARM SMMU components are described through IORT table
entries, that contain SMMU parameters and provide the kernel with
data to create the corresponding kernel abstractions and allow
SMMU probing and initialization.

Currently, the arm-smmu driver probe routines are DT based, hence,
to enable the driver to probe through ACPI they should be augmented
so that the DT and ACPI probing paths can actually share the common
code that probes and initializes the ARM SMMU allowing them to co-exist
seamlessly.

This patch refactors the ARM SMMU probing path to introduce ACPI
probing effectively enabling the ARM SMMU on ACPI based systems.

To create the required platform devices representing ARM SMMU components
and initialize them with the required data, this patch also implements
ARM SMMU ACPI probing by adding a hook into the ACPI IORT linker section,
that is executed automatically by the kernel on boot and allows
to detect and configure the ARM SMMU devices present in the system.

Based on prior work by Hanjun Guo <[email protected]>.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Hanjun Guo <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Tomasz Nowicki <[email protected]>
---
drivers/iommu/arm-smmu.c | 240 +++++++++++++++++++++++++++++++++++++++++++++--
include/linux/iort.h | 3 +
2 files changed, 233 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 0f1e784..85ab4b7 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -28,6 +28,7 @@

#define pr_fmt(fmt) "arm-smmu: " fmt

+#include <linux/acpi.h>
#include <linux/delay.h>
#include <linux/dma-iommu.h>
#include <linux/dma-mapping.h>
@@ -36,6 +37,7 @@
#include <linux/io.h>
#include <linux/iommu.h>
#include <linux/iopoll.h>
+#include <linux/iort.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_address.h>
@@ -1729,6 +1731,103 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
return 0;
}

+#ifdef CONFIG_ACPI
+static int __init acpi_smmu_version(u32 model, u32 *version)
+{
+ switch (model) {
+ case ACPI_IORT_SMMU_V1:
+ case ACPI_IORT_SMMU_CORELINK_MMU400:
+ *version = ARM_SMMU_V1;
+ return 0;
+ case ACPI_IORT_SMMU_V2:
+ case ACPI_IORT_SMMU_CORELINK_MMU500:
+ *version = ARM_SMMU_V2;
+ return 0;
+ default:
+ break;
+ }
+
+ return -ENODEV;
+}
+
+static int __init arm_smmu_acpi_probe(struct platform_device *pdev,
+ struct arm_smmu_device *smmu)
+{
+ struct device *dev = smmu->dev;
+ struct acpi_iort_node *node =
+ *(struct acpi_iort_node **)dev_get_platdata(dev);
+ struct acpi_iort_smmu *iort_smmu;
+ int num_irqs, i, err, trigger, ret, irq_idx;
+ u64 *ctx_irq, *glb_irq;
+
+ /* Retrieve SMMU1/2 specific data */
+ iort_smmu = (struct acpi_iort_smmu *)node->node_data;
+
+ ret = acpi_smmu_version(iort_smmu->model, &smmu->version);
+ if (ret < 0)
+ return ret;
+
+ glb_irq = ACPI_ADD_PTR(u64, node, iort_smmu->global_interrupt_offset);
+ if (!IORT_IRQ_MASK(glb_irq[1])) /* 0 means not implemented */
+ smmu->num_global_irqs = 1;
+ else
+ smmu->num_global_irqs = 2;
+
+ smmu->num_context_irqs = iort_smmu->context_interrupt_count;
+ num_irqs = smmu->num_context_irqs + smmu->num_global_irqs;
+
+ if (!smmu->num_context_irqs) {
+ dev_err(dev, "found %d interrupts but expected at least %d\n",
+ num_irqs, smmu->num_global_irqs + 1);
+ return -ENODEV;
+ }
+
+ smmu->irqs = devm_kzalloc(dev, sizeof(*smmu->irqs) * num_irqs,
+ GFP_KERNEL);
+ if (!smmu->irqs) {
+ dev_err(dev, "failed to allocate %d irqs\n", num_irqs);
+ return -ENOMEM;
+ }
+
+ /* Global IRQs */
+ for (i = irq_idx = 0; i < smmu->num_global_irqs; i++, irq_idx++) {
+ int hw_irq = IORT_IRQ_MASK(glb_irq[i]);
+
+ trigger = IORT_IRQ_TRIGGER_MASK(glb_irq[i]);
+ smmu->irqs[irq_idx] = acpi_register_gsi(NULL, hw_irq, trigger,
+ ACPI_ACTIVE_HIGH);
+ }
+
+ /* Context IRQs */
+ ctx_irq = ACPI_ADD_PTR(u64, node, iort_smmu->context_interrupt_offset);
+ for (i = 0; i < smmu->num_context_irqs; i++, irq_idx++) {
+ int hw_irq = IORT_IRQ_MASK(ctx_irq[i]);
+
+ trigger = IORT_IRQ_TRIGGER_MASK(ctx_irq[i]);
+ smmu->irqs[irq_idx] = acpi_register_gsi(NULL, hw_irq, trigger,
+ ACPI_ACTIVE_HIGH);
+ }
+
+ if (iort_smmu->flags & ACPI_IORT_SMMU_COHERENT_WALK)
+ smmu->features |= ARM_SMMU_FEAT_COHERENT_WALK;
+
+ err = arm_smmu_device_cfg_probe(smmu);
+ if (err)
+ return err;
+
+ iort_iommu_set_node(&arm_smmu_ops, node, pdev->dev.fwnode);
+
+ return 0;
+}
+
+#else
+static inline int arm_smmu_acpi_probe(struct platform_device *pdev,
+ struct arm_smmu_device *smmu)
+{
+ return -ENODEV;
+}
+#endif
+
static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,smmu-v1", .data = (void *)ARM_SMMU_V1 },
{ .compatible = "arm,smmu-v2", .data = (void *)ARM_SMMU_V2 },
@@ -1933,7 +2032,10 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
return PTR_ERR(smmu->base);
smmu->size = resource_size(res);

- err = arm_smmu_dt_probe(pdev, smmu);
+ if (acpi_disabled)
+ err = arm_smmu_dt_probe(pdev, smmu);
+ else
+ err = arm_smmu_acpi_probe(pdev, smmu);

if (err)
return err;
@@ -2007,16 +2109,20 @@ static int __init arm_smmu_init(void)

if (done)
return 0;
- /*
- * Play nice with systems that don't have an ARM SMMU by checking that
- * an ARM SMMU exists in the system before proceeding with the driver
- * and IOMMU bus operation registration.
- */
- np = of_find_matching_node(NULL, arm_smmu_of_match);
- if (!np)
- return 0;

- of_node_put(np);
+ if (acpi_disabled) {
+ /*
+ * Play nice with systems that don't have an ARM SMMU by
+ * checking that an ARM SMMU exists in the system before
+ * proceeding with the driver and IOMMU bus operation
+ * registration.
+ */
+ np = of_find_matching_node(NULL, arm_smmu_of_match);
+ if (!np)
+ return 0;
+
+ of_node_put(np);
+ }

ret = platform_driver_register(&arm_smmu_driver);
if (ret)
@@ -2048,6 +2154,120 @@ static void __exit arm_smmu_exit(void)
subsys_initcall(arm_smmu_init);
module_exit(arm_smmu_exit);

+#ifdef CONFIG_ACPI
+static int __init add_smmu_platform_device(struct acpi_iort_node *node)
+{
+ struct acpi_iort_smmu *smmu;
+ struct platform_device *pdev = NULL;
+ struct resource resources;
+ enum dev_dma_attr attr;
+ int ret;
+
+ /* Retrieve SMMU1/2 specific data */
+ smmu = (struct acpi_iort_smmu *)node->node_data;
+
+ memset(&resources, 0, sizeof(resources));
+ resources.start = smmu->base_address;
+ resources.end = smmu->base_address + smmu->span - 1;
+ resources.flags = IORESOURCE_MEM;
+
+ pdev = platform_device_alloc("arm-smmu", PLATFORM_DEVID_AUTO);
+ if (!pdev)
+ return PTR_ERR(pdev);
+
+ pdev->dev.fwnode = iommu_alloc_fwnode();
+
+ if (!pdev->dev.fwnode) {
+ ret = -ENOMEM;
+ goto dev_put;
+ }
+
+ ret = platform_device_add_resources(pdev, &resources, 1);
+ if (ret)
+ goto free_node;
+
+ ret = platform_device_add_data(pdev, &node, sizeof(node));
+ if (ret)
+ goto free_node;
+
+ pdev->dev.dma_mask = kmalloc(sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
+ if (!pdev->dev.dma_mask) {
+ ret = -ENOMEM;
+ goto free_node;
+ }
+
+ /*
+ * Set default dma mask value for the table walker,
+ * to be overridden on probing with correct value.
+ */
+ *pdev->dev.dma_mask = DMA_BIT_MASK(32);
+ pdev->dev.coherent_dma_mask = *pdev->dev.dma_mask;
+
+ attr = smmu->flags & ACPI_IORT_SMMU_COHERENT_WALK ?
+ DEV_DMA_COHERENT : DEV_DMA_NON_COHERENT;
+
+ /* Configure DMA for the page table walker */
+ acpi_dma_configure(&pdev->dev, attr);
+
+ ret = platform_device_add(pdev);
+ if (ret)
+ goto dma_deconfigure;
+
+ return 0;
+
+dma_deconfigure:
+ acpi_dma_deconfigure(&pdev->dev);
+ kfree(pdev->dev.dma_mask);
+free_node:
+ iommu_free_fwnode(pdev->dev.fwnode);
+dev_put:
+ platform_device_put(pdev);
+
+ return ret;
+}
+
+static int __init arm_smmu_acpi_init(struct acpi_table_header *table)
+{
+ struct acpi_iort_node *iort_node, *iort_end;
+ struct acpi_table_iort *iort;
+ int i, ret = arm_smmu_init();
+
+ if (ret)
+ return ret;
+ /*
+ * iort_table and iort 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, iort,
+ iort->node_offset);
+ iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort,
+ 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) {
+ ret = add_smmu_platform_device(iort_node);
+ if (ret)
+ return ret;
+ }
+
+ iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
+ iort_node->length);
+ }
+
+ return 0;
+}
+
+IORT_ACPI_DECLARE(arm_smmu, ACPI_SIG_IORT, arm_smmu_acpi_init);
+#endif
+
static int __init arm_smmu_of_init(struct device_node *np)
{
struct arm_smmu_device *smmu;
diff --git a/include/linux/iort.h b/include/linux/iort.h
index 7a7af40..958b236 100644
--- a/include/linux/iort.h
+++ b/include/linux/iort.h
@@ -21,6 +21,9 @@

#include <linux/acpi.h>

+#define IORT_IRQ_MASK(irq) (irq & 0xffffffffULL)
+#define IORT_IRQ_TRIGGER_MASK(irq) ((irq >> 32) & 0xffffffffULL)
+
struct pci_dev;
struct fwnode_handle;
int iort_register_domain_token(int trans_id, struct fwnode_handle *fw_node);
--
2.6.4

2016-04-14 17:24:13

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [RFC PATCH 09/11] drivers: acpi: implement acpi_dma_configure

On DT based systems, the of_dma_configure() API implements DMA configuration
for a given device. On ACPI systems an API equivalent to of_dma_configure()
is missing which implies that it is currently not possible to set-up DMA
operations for devices through the ACPI generic kernel layer.

This patch fills the gap by introducing acpi_dma_configure/deconfigure()
calls, that carry out IOMMU configuration through IORT (on systems where
it is present) and call arch_setup_dma_ops(...) with the retrieved
parameters.

The DMA range size passed to arch_setup_dma_ops() is sized according
to the device coherent_dma_mask (starting at address 0x0), mirroring the
DT probing path behaviour when a dma-ranges property is not provided
for the device being probed; this changes the current arch_setup_dma_ops()
call parameters in the ACPI probing case, but since arch_setup_dma_ops()
is a NOP on all architectures but ARM/ARM64 this patch does not change
the current kernel behaviour on them.

This patch updates ACPI and PCI core code to use the newly introduced
acpi_dma_configure function, providing the same functionality
as of_dma_configure on ARM systems and leaving behaviour unchanged
for all other arches.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Tomasz Nowicki <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
---
drivers/acpi/glue.c | 4 +--
drivers/acpi/iort.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
drivers/acpi/scan.c | 29 +++++++++++++++++
drivers/pci/probe.c | 3 +-
include/acpi/acpi_bus.h | 2 ++
include/linux/acpi.h | 5 +++
include/linux/iort.h | 9 ++++++
7 files changed, 133 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 5ea5dc2..f8d6564 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -227,8 +227,7 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)

attr = acpi_get_dma_attr(acpi_dev);
if (attr != DEV_DMA_NOT_SUPPORTED)
- arch_setup_dma_ops(dev, 0, 0, NULL,
- attr == DEV_DMA_COHERENT);
+ acpi_dma_configure(dev, attr);

acpi_physnode_link_name(physical_node_name, node_id);
retval = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
@@ -251,6 +250,7 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
return 0;

err:
+ acpi_dma_deconfigure(dev);
ACPI_COMPANION_SET(dev, NULL);
put_device(dev);
put_device(&acpi_dev->dev);
diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
index 2b5ce65..b1bb8fb 100644
--- a/drivers/acpi/iort.c
+++ b/drivers/acpi/iort.c
@@ -72,6 +72,31 @@ int iort_iommu_set_node(struct iommu_ops *ops, struct acpi_iort_node *node,
return 0;
}

+/**
+ * iort_iommu_get_node - Retrieve iort_iommu_node associated with an IORT node.
+ *
+ * @node: IORT table node to be looked-up
+ *
+ * Returns: iort_iommu_node pointer on success
+ * NULL on failure
+ */
+static struct iort_iommu_node *iort_iommu_get_node(struct acpi_iort_node *node)
+{
+ struct iort_iommu_node *iommu_node;
+
+ spin_lock(&iort_iommu_lock);
+ list_for_each_entry(iommu_node, &iort_iommu_list, list) {
+ if (iommu_node->node == node)
+ goto found;
+ }
+
+ iommu_node = NULL;
+found:
+ spin_unlock(&iort_iommu_lock);
+
+ return iommu_node;
+}
+
typedef acpi_status (*iort_find_node_callback)
(struct acpi_iort_node *node, void *context);

@@ -405,6 +430,66 @@ iort_pci_get_domain(struct pci_dev *pdev, u32 req_id)
return domain_handle;
}

+static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
+{
+ u32 *rid = data;
+
+ *rid = alias;
+ return 0;
+}
+
+/**
+ * iort_iommu_configure - Set-up IOMMU configuration for a device.
+ *
+ * @dev: device that requires IOMMU set-up
+ *
+ * Returns: iommu_ops pointer on configuration success
+ * NULL on configuration failure
+ */
+struct iommu_ops *iort_iommu_configure(struct device *dev)
+{
+ struct acpi_iort_node *node, *parent;
+ struct iommu_ops *ops = NULL;
+ struct iommu_fwspec fwspec;
+ struct iort_iommu_node *iommu_node;
+ u32 rid = 0, devid = 0;
+
+ if (dev_is_pci(dev)) {
+ struct pci_bus *bus = to_pci_dev(dev)->bus;
+
+ pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
+ &rid);
+
+ node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
+ iort_find_dev_callback, &bus->dev);
+ } else
+ node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
+ iort_find_dev_callback, dev);
+
+ if (!node)
+ return NULL;
+
+ iort_dev_map_rid(node, rid, &devid, ACPI_IORT_NODE_SMMU);
+
+ parent = iort_find_parent_node(node, ACPI_IORT_NODE_SMMU);
+
+ if (!parent)
+ return NULL;
+
+ iommu_node = iort_iommu_get_node(parent);
+ ops = iommu_node->ops;
+
+ fwspec.fwnode = iommu_node->fwnode;
+ fwspec.param_count = 1;
+ fwspec.param[0] = devid;
+
+ if (!ops || !ops->fw_xlate || ops->fw_xlate(dev, &fwspec))
+ return NULL;
+
+ return ops;
+}
+
+
static int __init iort_table_detect(void)
{
acpi_status status;
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 5f28cf7..e0fd5e3 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/iort.h>
#include <linux/signal.h>
#include <linux/kthread.h>
#include <linux/dmi.h>
@@ -1358,6 +1359,34 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
return DEV_DMA_NON_COHERENT;
}

+/**
+ * acpi_dma_configure - Set-up DMA configuration for the device.
+ * @dev: The pointer to the device
+ * @attr: device dma attributes
+ */
+void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
+{
+ 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, iommu,
+ attr == DEV_DMA_COHERENT);
+}
+
+/**
+ * acpi_dma_deconfigure - Tear-down DMA configuration for the device.
+ * @dev: The pointer to the device
+ */
+void acpi_dma_deconfigure(struct device *dev)
+{
+ arch_teardown_dma_ops(dev);
+}
+
static void acpi_init_coherency(struct acpi_device *adev)
{
unsigned long long cca = 0;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ef569e8..9cf90b8 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1725,8 +1725,7 @@ static void pci_dma_configure(struct pci_dev *dev)
if (attr == DEV_DMA_NOT_SUPPORTED)
dev_warn(&dev->dev, "DMA not supported.\n");
else
- arch_setup_dma_ops(&dev->dev, 0, 0, NULL,
- attr == DEV_DMA_COHERENT);
+ acpi_dma_configure(&dev->dev, attr);
}

pci_put_host_bridge_device(bridge);
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 14362a8..212eff2 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -566,6 +566,8 @@ struct acpi_pci_root {

bool acpi_dma_supported(struct acpi_device *adev);
enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev);
+void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr);
+void acpi_dma_deconfigure(struct device *dev);

struct acpi_device *acpi_find_child_device(struct acpi_device *parent,
u64 address, bool check_children);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 06ed7e5..69b9041 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -683,6 +683,11 @@ static inline enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
return DEV_DMA_NOT_SUPPORTED;
}

+static inline void acpi_dma_configure(struct device *dev,
+ enum dev_dma_attr attr) { }
+
+static inline void acpi_dma_deconfigure(struct device *dev) { }
+
#define ACPI_PTR(_ptr) (NULL)

#endif /* !CONFIG_ACPI */
diff --git a/include/linux/iort.h b/include/linux/iort.h
index 766adda..7a7af40 100644
--- a/include/linux/iort.h
+++ b/include/linux/iort.h
@@ -31,6 +31,15 @@ struct fwnode_handle *iort_pci_get_domain(struct pci_dev *pdev, u32 req_id);
int iort_iommu_set_node(struct iommu_ops *ops, struct acpi_iort_node *node,
struct fwnode_handle *fwnode);

+#ifdef CONFIG_IORT_TABLE
+struct iommu_ops *iort_iommu_configure(struct device *dev);
+#else
+static inline struct iommu_ops *iort_iommu_configure(struct device *dev)
+{
+ return NULL;
+}
+#endif
+
#define IORT_ACPI_DECLARE(name, table_id, fn) \
ACPI_DECLARE_PROBE_ENTRY(iort, name, table_id, 0, NULL, 0, fn)

--
2.6.4

2016-04-14 17:24:33

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [RFC PATCH 07/11] drivers: iommu: arm-smmu: allow ACPI based streamid translation

The ACPI IORT table provides data to ARM IOMMU drivers to carry out
streamid mappings and the kernel has the infrastructure to implement
it through the fw_xlate() struct iommu_ops hook.

By relying on the DT agnostic struct iommu_fwspec fields, this
patch adds code in the ARM SMMU fw_xlate() callback that allows
streamid translation on ACPI based ARM system.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Robin Murphy <[email protected]>
---
drivers/iommu/arm-smmu.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 84bcff7..0f1e784 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1440,6 +1440,20 @@ out_unlock:
return ret;
}

+static int arm_smmu_dev_node_match(struct device *dev, void *data)
+{
+ return is_fwnode_iommu(dev->fwnode) && dev->fwnode == data;
+}
+
+static struct platform_device *arm_smmu_get_dev(struct fwnode_handle *fwnode)
+{
+ struct device *dev;
+
+ dev = bus_find_device(&platform_bus_type, NULL, fwnode,
+ arm_smmu_dev_node_match);
+ return dev ? to_platform_device(dev) : NULL;
+}
+
static int arm_smmu_fw_xlate(struct device *dev, struct iommu_fwspec *args)
{
struct arm_smmu_device *smmu;
@@ -1447,6 +1461,8 @@ static int arm_smmu_fw_xlate(struct device *dev, struct iommu_fwspec *args)

if (is_of_node(args->fwnode))
smmu_pdev = of_find_device_by_node(to_of_node(args->fwnode));
+ else if (is_fwnode_iommu(args->fwnode))
+ smmu_pdev = arm_smmu_get_dev(args->fwnode);

if (!smmu_pdev)
return -ENODEV;
--
2.6.4

2016-04-14 17:25:00

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [RFC PATCH 05/11] drivers: iommu: arm-smmu: split probe functions into DT/generic portions

Current ARM SMMU probe functions intermingle HW and DT probing
in the initialization functions to detect and programme the ARM SMMU
driver features. In order to allow probing the ARM SMMU with other
firmwares than DT, this patch splits the ARM SMMU init functions into
DT and HW specific portions so that other FW interfaces (ie ACPI) can
reuse the HW probing functions and skip the DT portion accordingly.

This patch implements no functional change, only code reshuffling.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Hanjun Guo <[email protected]>
Cc: Robin Murphy <[email protected]>
---
drivers/iommu/arm-smmu.c | 80 +++++++++++++++++++++++++++++++-----------------
1 file changed, 52 insertions(+), 28 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 2d65de4..6c42770 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1551,7 +1551,7 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
unsigned long size;
void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
u32 id;
- bool cttw_dt, cttw_reg;
+ bool cttw_reg, cttw_fw = smmu->features & ARM_SMMU_FEAT_COHERENT_WALK;

dev_notice(smmu->dev, "probing hardware configuration...\n");
dev_notice(smmu->dev, "SMMUv%d with:\n", smmu->version);
@@ -1593,20 +1593,18 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)

/*
* In order for DMA API calls to work properly, we must defer to what
- * the DT says about coherency, regardless of what the hardware claims.
+ * the FW says about coherency, regardless of what the hardware
+ * claims.
* Fortunately, this also opens up a workaround for systems where the
* ID register value has ended up configured incorrectly.
*/
- cttw_dt = of_dma_is_coherent(smmu->dev->of_node);
cttw_reg = !!(id & ID0_CTTW);
- if (cttw_dt)
- smmu->features |= ARM_SMMU_FEAT_COHERENT_WALK;
- if (cttw_dt || cttw_reg)
+ if (cttw_fw || cttw_reg)
dev_notice(smmu->dev, "\t%scoherent table walk\n",
- cttw_dt ? "" : "non-");
- if (cttw_dt != cttw_reg)
+ cttw_fw ? "" : "non-");
+ if (cttw_fw != cttw_reg)
dev_notice(smmu->dev,
- "\t(IDR0.CTTW overridden by dma-coherent property)\n");
+ "\t(IDR0.CTTW overridden by FW configuration)\n");

if (id & ID0_SMS) {
u32 smr, sid, mask;
@@ -1827,30 +1825,17 @@ static int arm_smmu_probe_mmu_masters(struct arm_smmu_device *smmu)
return err;
}

-static int arm_smmu_device_dt_probe(struct platform_device *pdev)
+static int arm_smmu_dt_probe(struct platform_device *pdev,
+ struct arm_smmu_device *smmu)
{
const struct of_device_id *of_id;
struct resource *res;
- struct arm_smmu_device *smmu;
struct device *dev = &pdev->dev;
int num_irqs, i, err;

- smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
- if (!smmu) {
- dev_err(dev, "failed to allocate arm_smmu_device\n");
- return -ENOMEM;
- }
- smmu->dev = dev;
-
of_id = of_match_node(arm_smmu_of_match, dev->of_node);
smmu->version = (enum arm_smmu_arch_version)of_id->data;

- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- smmu->base = devm_ioremap_resource(dev, res);
- if (IS_ERR(smmu->base))
- return PTR_ERR(smmu->base);
- smmu->size = resource_size(res);
-
if (of_property_read_u32(dev->of_node, "#global-interrupts",
&smmu->num_global_irqs)) {
dev_err(dev, "missing #global-interrupts property\n");
@@ -1887,12 +1872,54 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
smmu->irqs[i] = irq;
}

+ /*
+ * In order for DMA API calls to work properly, we must defer to what
+ * the DT says about coherency, regardless of what the hardware detect
+ * through probing in arm_smmu_device_cfg_probe(). Check the DT dma
+ * coherent property and initialize the corresponding SMMU feature
+ * so that arm_smmu_device_cfg_probe() can check the FW reported value.
+ */
+ if (of_dma_is_coherent(smmu->dev->of_node))
+ smmu->features |= ARM_SMMU_FEAT_COHERENT_WALK;
+
err = arm_smmu_device_cfg_probe(smmu);
if (err)
return err;

parse_driver_options(smmu);

+ /* Check first to avoid of_parse_phandle_with_args complaining */
+ if (of_property_read_bool(dev->of_node, "mmu-masters"))
+ arm_smmu_probe_mmu_masters(smmu);
+
+ return 0;
+}
+
+static int arm_smmu_device_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ struct arm_smmu_device *smmu;
+ struct device *dev = &pdev->dev;
+ int i, err;
+
+ smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
+ if (!smmu) {
+ dev_err(dev, "failed to allocate arm_smmu_device\n");
+ return -ENOMEM;
+ }
+ smmu->dev = dev;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ smmu->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(smmu->base))
+ return PTR_ERR(smmu->base);
+ smmu->size = resource_size(res);
+
+ err = arm_smmu_dt_probe(pdev, smmu);
+
+ if (err)
+ return err;
+
if (smmu->version > ARM_SMMU_V1 &&
smmu->num_context_banks != smmu->num_context_irqs) {
dev_err(dev,
@@ -1915,9 +1942,6 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
}

platform_set_drvdata(pdev, smmu);
- /* Check first to avoid of_parse_phandle_with_args complaining */
- if (of_property_read_bool(dev->of_node, "mmu-masters"))
- arm_smmu_probe_mmu_masters(smmu);
arm_smmu_device_reset(smmu);
return 0;

@@ -1953,7 +1977,7 @@ static struct platform_driver arm_smmu_driver = {
.name = "arm-smmu",
.of_match_table = of_match_ptr(arm_smmu_of_match),
},
- .probe = arm_smmu_device_dt_probe,
+ .probe = arm_smmu_device_probe,
.remove = arm_smmu_device_remove,
};

--
2.6.4

2016-04-14 17:25:29

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [RFC PATCH 04/11] drivers: acpi: iort: introduce linker section for IORT entries probing

Since commit e647b532275b ("ACPI: Add early device probing
infrastructure") the kernel has gained the infrastructure that allows
adding linker script section entries to execute ACPI driver callbacks
(ie probe routines) for all subsystems that register a table entry
in the respective kernel section (eg clocksource, irqchip).

Since ARM IOMMU devices data is described through IORT tables when
booting with ACPI, the ARM IOMMU drivers must be made able to hook ACPI
callback routines that are called to probe IORT entries and initialize
the respective IOMMU devices.

To avoid adding driver specific hooks into IORT table initialization
code (breaking therefore code modularity - ie ACPI IORT code must be made
aware of ARM SMMU drivers ACPI init callbacks), this patch adds code
that allows ARM SMMU drivers to take advantage of the ACPI early probing
infrastructure, so that they can add linker script section entries
containing drivers callback to be executed on IORT tables detection.

Since IORT nodes are differentiated by a type, the callback routines
can easily parse the IORT table entries, check the IORT nodes and
carry out some actions whenever the IORT node type associated with
the driver specific callback is matched.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Tomasz Nowicki <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Marc Zyngier <[email protected]>
---
drivers/acpi/iort.c | 2 ++
include/asm-generic/vmlinux.lds.h | 1 +
include/linux/iort.h | 3 +++
3 files changed, 6 insertions(+)

diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
index 98db580..080888a 100644
--- a/drivers/acpi/iort.c
+++ b/drivers/acpi/iort.c
@@ -372,6 +372,8 @@ static int __init iort_table_detect(void)
return -EINVAL;
}

+ acpi_probe_device_table(iort);
+
return 0;
}
arch_initcall(iort_table_detect);
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 339125b..0aae448 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -536,6 +536,7 @@
IRQCHIP_OF_MATCH_TABLE() \
ACPI_PROBE_TABLE(irqchip) \
ACPI_PROBE_TABLE(clksrc) \
+ ACPI_PROBE_TABLE(iort) \
EARLYCON_TABLE()

#define INIT_TEXT \
diff --git a/include/linux/iort.h b/include/linux/iort.h
index 148d9a1..766adda 100644
--- a/include/linux/iort.h
+++ b/include/linux/iort.h
@@ -31,4 +31,7 @@ struct fwnode_handle *iort_pci_get_domain(struct pci_dev *pdev, u32 req_id);
int iort_iommu_set_node(struct iommu_ops *ops, struct acpi_iort_node *node,
struct fwnode_handle *fwnode);

+#define IORT_ACPI_DECLARE(name, table_id, fn) \
+ ACPI_DECLARE_PROBE_ENTRY(iort, name, table_id, 0, NULL, 0, fn)
+
#endif /* __IORT_H__ */
--
2.6.4

2016-04-14 17:26:00

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [RFC PATCH 02/11] drivers: acpi: iort: add support for IOMMU registration

The ACPI IORT table provide entries for IOMMU (aka SMMU in ARM world)
components that allow creating the kernel data structures required to
probe and initialize the IOMMU devices.

This patch provides support in the IORT kernel code to register IOMMU
components.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Cc: Hanjun Guo <[email protected]>
Cc: Tomasz Nowicki <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
---
drivers/acpi/iort.c | 42 ++++++++++++++++++++++++++++++++++++++++++
include/linux/iort.h | 2 ++
2 files changed, 44 insertions(+)

diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
index 3119683..98db580 100644
--- a/drivers/acpi/iort.c
+++ b/drivers/acpi/iort.c
@@ -19,10 +19,13 @@
#define pr_fmt(fmt) "ACPI: IORT: " fmt

#include <linux/export.h>
+#include <linux/iommu.h>
#include <linux/iort.h>
#include <linux/irqdomain.h>
#include <linux/kernel.h>
+#include <linux/list.h>
#include <linux/pci.h>
+#include <linux/slab.h>

struct iort_its_msi_chip {
struct list_head list;
@@ -30,6 +33,45 @@ struct iort_its_msi_chip {
u32 translation_id;
};

+struct iort_iommu_node {
+ struct list_head list;
+ struct iommu_ops *ops;
+ struct acpi_iort_node *node;
+ struct fwnode_handle *fwnode;
+};
+static LIST_HEAD(iort_iommu_list);
+static DEFINE_SPINLOCK(iort_iommu_lock);
+
+/**
+ * iort_iommu_set_node - Create iort_mmu_node and use it to register
+ * iommu structures on iort_iommu_list.
+ *
+ * @ops: IOMMU operations
+ * @node: IORT table node associated with the IOMMU
+ * @fwnode: fwnode_handle associated with the IOMMU
+ *
+ * Returns: 0 on success
+ * -ENOMEM on failure
+ */
+int iort_iommu_set_node(struct iommu_ops *ops, struct acpi_iort_node *node,
+ struct fwnode_handle *fwnode)
+{
+ struct iort_iommu_node *iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
+
+ if (WARN_ON(!iommu))
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(&iommu->list);
+ iommu->ops = ops;
+ iommu->node = node;
+ iommu->fwnode = fwnode;
+ spin_lock(&iort_iommu_lock);
+ list_add_tail(&iommu->list, &iort_iommu_list);
+ spin_unlock(&iort_iommu_lock);
+
+ return 0;
+}
+
typedef acpi_status (*iort_find_node_callback)
(struct acpi_iort_node *node, void *context);

diff --git a/include/linux/iort.h b/include/linux/iort.h
index b15fe1a..148d9a1 100644
--- a/include/linux/iort.h
+++ b/include/linux/iort.h
@@ -28,5 +28,7 @@ void iort_deregister_domain_token(int trans_id);
struct fwnode_handle *iort_its_find_domain_token(int trans_id);
u32 iort_pci_get_msi_rid(struct pci_dev *pdev, u32 req_id);
struct fwnode_handle *iort_pci_get_domain(struct pci_dev *pdev, u32 req_id);
+int iort_iommu_set_node(struct iommu_ops *ops, struct acpi_iort_node *node,
+ struct fwnode_handle *fwnode);

#endif /* __IORT_H__ */
--
2.6.4

2016-04-15 16:14:09

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [RFC PATCH 09/11] drivers: acpi: implement acpi_dma_configure

On Thu, Apr 14, 2016 at 06:25:41PM +0100, Lorenzo Pieralisi wrote:
> On DT based systems, the of_dma_configure() API implements DMA configuration
> for a given device. On ACPI systems an API equivalent to of_dma_configure()
> is missing which implies that it is currently not possible to set-up DMA
> operations for devices through the ACPI generic kernel layer.
>
> This patch fills the gap by introducing acpi_dma_configure/deconfigure()
> calls, that carry out IOMMU configuration through IORT (on systems where
> it is present) and call arch_setup_dma_ops(...) with the retrieved
> parameters.
>
> The DMA range size passed to arch_setup_dma_ops() is sized according
> to the device coherent_dma_mask (starting at address 0x0), mirroring the
> DT probing path behaviour when a dma-ranges property is not provided
> for the device being probed; this changes the current arch_setup_dma_ops()
> call parameters in the ACPI probing case, but since arch_setup_dma_ops()
> is a NOP on all architectures but ARM/ARM64 this patch does not change
> the current kernel behaviour on them.
>
> This patch updates ACPI and PCI core code to use the newly introduced
> acpi_dma_configure function, providing the same functionality
> as of_dma_configure on ARM systems and leaving behaviour unchanged
> for all other arches.
>
> Signed-off-by: Lorenzo Pieralisi <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Tomasz Nowicki <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>

There's only a tiny PCI change in this series, so I assume somebody
else will merge all this. Here's my ack for the PCI part:

Acked-by: Bjorn Helgaas <[email protected]> # for drivers/pci/probe.c change

One question on use of pci_for_each_dma_alias() below.

> +static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
> +{
> + u32 *rid = data;
> +
> + *rid = alias;
> + return 0;
> +}
> +
> +/**
> + * iort_iommu_configure - Set-up IOMMU configuration for a device.
> + *
> + * @dev: device that requires IOMMU set-up
> + *
> + * Returns: iommu_ops pointer on configuration success
> + * NULL on configuration failure
> + */
> +struct iommu_ops *iort_iommu_configure(struct device *dev)
> +{
> + struct acpi_iort_node *node, *parent;
> + struct iommu_ops *ops = NULL;
> + struct iommu_fwspec fwspec;
> + struct iort_iommu_node *iommu_node;
> + u32 rid = 0, devid = 0;
> +
> + if (dev_is_pci(dev)) {
> + struct pci_bus *bus = to_pci_dev(dev)->bus;
> +
> + pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
> + &rid);

You end up with only the last DMA alias in "rid". Is it really true
that you only need to call iort_dev_map_rid() for one of the aliases?

> + node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
> + iort_find_dev_callback, &bus->dev);
> + } else
> + node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
> + iort_find_dev_callback, dev);
> +
> + if (!node)
> + return NULL;
> +
> + iort_dev_map_rid(node, rid, &devid, ACPI_IORT_NODE_SMMU);
> +
> + parent = iort_find_parent_node(node, ACPI_IORT_NODE_SMMU);
> +
> + if (!parent)
> + return NULL;
> +
> + iommu_node = iort_iommu_get_node(parent);
> + ops = iommu_node->ops;
> +
> + fwspec.fwnode = iommu_node->fwnode;
> + fwspec.param_count = 1;
> + fwspec.param[0] = devid;
> +
> + if (!ops || !ops->fw_xlate || ops->fw_xlate(dev, &fwspec))
> + return NULL;
> +
> + return ops;
> +}

2016-04-15 16:32:05

by Robin Murphy

[permalink] [raw]
Subject: Re: [RFC PATCH 09/11] drivers: acpi: implement acpi_dma_configure

Hi Bjorn,

On 15/04/16 17:14, Bjorn Helgaas wrote:
> On Thu, Apr 14, 2016 at 06:25:41PM +0100, Lorenzo Pieralisi wrote:
>> On DT based systems, the of_dma_configure() API implements DMA configuration
>> for a given device. On ACPI systems an API equivalent to of_dma_configure()
>> is missing which implies that it is currently not possible to set-up DMA
>> operations for devices through the ACPI generic kernel layer.
>>
>> This patch fills the gap by introducing acpi_dma_configure/deconfigure()
>> calls, that carry out IOMMU configuration through IORT (on systems where
>> it is present) and call arch_setup_dma_ops(...) with the retrieved
>> parameters.
>>
>> The DMA range size passed to arch_setup_dma_ops() is sized according
>> to the device coherent_dma_mask (starting at address 0x0), mirroring the
>> DT probing path behaviour when a dma-ranges property is not provided
>> for the device being probed; this changes the current arch_setup_dma_ops()
>> call parameters in the ACPI probing case, but since arch_setup_dma_ops()
>> is a NOP on all architectures but ARM/ARM64 this patch does not change
>> the current kernel behaviour on them.
>>
>> This patch updates ACPI and PCI core code to use the newly introduced
>> acpi_dma_configure function, providing the same functionality
>> as of_dma_configure on ARM systems and leaving behaviour unchanged
>> for all other arches.
>>
>> Signed-off-by: Lorenzo Pieralisi <[email protected]>
>> Cc: Bjorn Helgaas <[email protected]>
>> Cc: Robin Murphy <[email protected]>
>> Cc: Tomasz Nowicki <[email protected]>
>> Cc: Joerg Roedel <[email protected]>
>> Cc: "Rafael J. Wysocki" <[email protected]>
>
> There's only a tiny PCI change in this series, so I assume somebody
> else will merge all this. Here's my ack for the PCI part:
>
> Acked-by: Bjorn Helgaas <[email protected]> # for drivers/pci/probe.c change
>
> One question on use of pci_for_each_dma_alias() below.
>
>> +static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
>> +{
>> + u32 *rid = data;
>> +
>> + *rid = alias;
>> + return 0;
>> +}
>> +
>> +/**
>> + * iort_iommu_configure - Set-up IOMMU configuration for a device.
>> + *
>> + * @dev: device that requires IOMMU set-up
>> + *
>> + * Returns: iommu_ops pointer on configuration success
>> + * NULL on configuration failure
>> + */
>> +struct iommu_ops *iort_iommu_configure(struct device *dev)
>> +{
>> + struct acpi_iort_node *node, *parent;
>> + struct iommu_ops *ops = NULL;
>> + struct iommu_fwspec fwspec;
>> + struct iort_iommu_node *iommu_node;
>> + u32 rid = 0, devid = 0;
>> +
>> + if (dev_is_pci(dev)) {
>> + struct pci_bus *bus = to_pci_dev(dev)->bus;
>> +
>> + pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
>> + &rid);
>
> You end up with only the last DMA alias in "rid". Is it really true
> that you only need to call iort_dev_map_rid() for one of the aliases?

Indeed - all we care about is what things look like by the time they
come out of the root complex on their way to the the IOMMU, so whatever
intermediate aliasing _within_ the PCI bus might happen along the way
doesn't actually matter.

Robin.

>> + node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
>> + iort_find_dev_callback, &bus->dev);
>> + } else
>> + node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
>> + iort_find_dev_callback, dev);
>> +
>> + if (!node)
>> + return NULL;
>> +
>> + iort_dev_map_rid(node, rid, &devid, ACPI_IORT_NODE_SMMU);
>> +
>> + parent = iort_find_parent_node(node, ACPI_IORT_NODE_SMMU);
>> +
>> + if (!parent)
>> + return NULL;
>> +
>> + iommu_node = iort_iommu_get_node(parent);
>> + ops = iommu_node->ops;
>> +
>> + fwspec.fwnode = iommu_node->fwnode;
>> + fwspec.param_count = 1;
>> + fwspec.param[0] = devid;
>> +
>> + if (!ops || !ops->fw_xlate || ops->fw_xlate(dev, &fwspec))
>> + return NULL;
>> +
>> + return ops;
>> +}
> _______________________________________________
> iommu mailing list
> [email protected]
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>

2016-04-15 18:29:19

by Timur Tabi

[permalink] [raw]
Subject: Re: [RFC PATCH 09/11] drivers: acpi: implement acpi_dma_configure

On Thu, Apr 14, 2016 at 12:25 PM, Lorenzo Pieralisi
<[email protected]> wrote:
> +void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
> +{
> + 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, iommu,
> + attr == DEV_DMA_COHERENT);
> +}

I have a network driver that is impacted by this code, so thank you
for posting this. (See
https://www.mail-archive.com/[email protected]/msg106249.html).

One one SOC, the driver needs to set the mask to 32 bits. On another
SOC, it needs to set it to 64 bits. On device tree, the driver will
use dma-ranges.

In your patches, where is coherent_dma_mask initialized? I found this
code in add_smmu_platform_device(), but I think this is setting the
mask for the IOMMU driver, not the individual devices. Either way, I
don't understand where the correct value is going to be overridden.

+ /*
+ * Set default dma mask value for the table walker,
+ * to be overridden on probing with correct value.
+ */
+ *pdev->dev.dma_mask = DMA_BIT_MASK(32);
+ pdev->dev.coherent_dma_mask = *pdev->dev.dma_mask;

--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

2016-04-18 10:30:16

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [RFC PATCH 09/11] drivers: acpi: implement acpi_dma_configure

On Fri, Apr 15, 2016 at 01:29:14PM -0500, Timur Tabi wrote:
> On Thu, Apr 14, 2016 at 12:25 PM, Lorenzo Pieralisi
> <[email protected]> wrote:
> > +void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
> > +{
> > + 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, iommu,
> > + attr == DEV_DMA_COHERENT);
> > +}
>
> I have a network driver that is impacted by this code, so thank you
> for posting this. (See
> https://www.mail-archive.com/[email protected]/msg106249.html).
>
> One one SOC, the driver needs to set the mask to 32 bits. On another
> SOC, it needs to set it to 64 bits. On device tree, the driver will
> use dma-ranges.

First off I think we agree this patch does not change current behaviour
as far as the devices default dma_mask are concerned. They are
initialized in PCI/ACPI core code:

- pci_setup_device()
- acpi_create_platform_device()

As for ACPI DT-dma-ranges equivalent I have to check if I can use
the _DMA method for that so that we can put in place the same
mechanism as DT to override the default masks, other than that it is
up to the drivers to set-up the dma mask accordingly, that's not
something this patchset is changing anyway.

> In your patches, where is coherent_dma_mask initialized? I found this
> code in add_smmu_platform_device(), but I think this is setting the
> mask for the IOMMU driver, not the individual devices. Either way, I
> don't understand where the correct value is going to be overridden.

For the ARM SMMU table walker:

arm_smmu_device_cfg_probe() - dma_set_mask_and_coherent()

For other devices see above.

Thanks,
Lorenzo

>
> + /*
> + * Set default dma mask value for the table walker,
> + * to be overridden on probing with correct value.
> + */
> + *pdev->dev.dma_mask = DMA_BIT_MASK(32);
> + pdev->dev.coherent_dma_mask = *pdev->dev.dma_mask;
>
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project.
>

2016-04-18 10:43:49

by Robin Murphy

[permalink] [raw]
Subject: Re: [RFC PATCH 09/11] drivers: acpi: implement acpi_dma_configure

On 15/04/16 19:29, Timur Tabi wrote:
> On Thu, Apr 14, 2016 at 12:25 PM, Lorenzo Pieralisi
> <[email protected]> wrote:
>> +void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
>> +{
>> + 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, iommu,
>> + attr == DEV_DMA_COHERENT);
>> +}
>
> I have a network driver that is impacted by this code, so thank you
> for posting this. (See
> https://www.mail-archive.com/[email protected]/msg106249.html).
>
> One one SOC, the driver needs to set the mask to 32 bits. On another
> SOC, it needs to set it to 64 bits. On device tree, the driver will
> use dma-ranges.

That's the wrong way to look at it - the driver isn't _using_
dma-ranges, you're merely relying on the OF code setting the _default_
DMA mask differently based on the property. If your driver is in the
minority of those which actually care about DMA masks, then it should be
calling dma_set_mask_and_coherent() appropriately and not relying on the
default.

> In your patches, where is coherent_dma_mask initialized? I found this
> code in add_smmu_platform_device(), but I think this is setting the
> mask for the IOMMU driver, not the individual devices.

Yes, that's for the SMMU itself as a device (i.e. the page table walker)
- as a handy example of "drivers which actually care about DMA masks",
it specifically needs to avoid a too-small DMA mask because
bounce-buffering the page tables tends to make things go horribly wrong.

Robin.

> Either way, I
> don't understand where the correct value is going to be overridden.
>
> + /*
> + * Set default dma mask value for the table walker,
> + * to be overridden on probing with correct value.
> + */
> + *pdev->dev.dma_mask = DMA_BIT_MASK(32);
> + pdev->dev.coherent_dma_mask = *pdev->dev.dma_mask;
>

2016-04-19 08:28:14

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [RFC PATCH 06/11] drivers: iommu: make of_xlate() interface DT agnostic

Hello,

On 2016-04-14 19:25, Lorenzo Pieralisi wrote:
> On systems booting with ACPI, the IOMMU drivers require the same
> kind of id mapping carried out with a DT tree through the of_xlate()
> API in order to map devices identifiers to IOMMU ones.
>
> On ACPI systems, since DT nodes are not present (ie struct
> device.of_node == NULL), to identify the device requiring the translation
> the struct device_node (and the structure used to pass translation
> information - struct of_phandle_args - that contains a struct device_node)
> cannot be used, so a generic translation structure to be used for IOMMU
> mapping should be defined, based on the firmware agnostic fwnode_handle
> type.
>
> This patch mechanically refactors/renames the of_xlate API to make
> it DT agnostic, by declaring a new type (struct iommu_fwspec), that
> allows the kernel to pass a device identifier (fwnode - which can
> represent either a DT node or an IOMMU FW node) and by changing the
> of_xlate signature so that it does not take anymore the DT specific
> of_phandle_args argument and replaces it with the DT agnostic
> iommu_fwspec one.
>
> Signed-off-by: Lorenzo Pieralisi <[email protected]>
> Cc: Matthias Brugger <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Hanjun Guo <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Tomasz Nowicki <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Marek Szyprowski <[email protected]>

I'm not sure if this is the right approach, although I have not enough
knowledge on ACPI firmware. Do you plan to rewrite all subsystems to the
new "fwspec" based interface? Right now of_xlate is rather common
interface used by various subsystems. Maybe it will be much easier to
just add acpi_xlate callback and plumb it to the generic code? Maybe later
when similar code will be in other subsystems and drivers, it can be
unified, having much more real use cases?

> ---
> drivers/iommu/arm-smmu.c | 12 +++++++-----
> drivers/iommu/exynos-iommu.c | 11 +++++++----
> drivers/iommu/mtk_iommu.c | 13 ++++++++-----
> drivers/iommu/of_iommu.c | 20 ++++++++++++++++++--
> include/linux/iommu.h | 24 ++++++++++++++++++++----
> 5 files changed, 60 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 6c42770..84bcff7 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -1440,18 +1440,20 @@ out_unlock:
> return ret;
> }
>
> -static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
> +static int arm_smmu_fw_xlate(struct device *dev, struct iommu_fwspec *args)
> {
> struct arm_smmu_device *smmu;
> - struct platform_device *smmu_pdev;
> + struct platform_device *smmu_pdev = NULL;
> +
> + if (is_of_node(args->fwnode))
> + smmu_pdev = of_find_device_by_node(to_of_node(args->fwnode));
>
> - smmu_pdev = of_find_device_by_node(args->np);
> if (!smmu_pdev)
> return -ENODEV;
>
> smmu = platform_get_drvdata(smmu_pdev);
>
> - return arm_smmu_add_dev_streamid(smmu, dev, args->args[0]);
> + return arm_smmu_add_dev_streamid(smmu, dev, args->param[0]);
> }
>
> static struct iommu_ops arm_smmu_ops = {
> @@ -1468,7 +1470,7 @@ static struct iommu_ops arm_smmu_ops = {
> .device_group = arm_smmu_device_group,
> .domain_get_attr = arm_smmu_domain_get_attr,
> .domain_set_attr = arm_smmu_domain_set_attr,
> - .of_xlate = arm_smmu_of_xlate,
> + .fw_xlate = arm_smmu_fw_xlate,
> .pgsize_bitmap = -1UL, /* Restricted during device attach */
> };
>
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 5ecc86c..84ff5bb 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -1250,13 +1250,16 @@ static void exynos_iommu_remove_device(struct device *dev)
> iommu_group_remove_device(dev);
> }
>
> -static int exynos_iommu_of_xlate(struct device *dev,
> - struct of_phandle_args *spec)
> +static int exynos_iommu_fw_xlate(struct device *dev,
> + struct iommu_fwspec *args)
> {
> struct exynos_iommu_owner *owner = dev->archdata.iommu;
> - struct platform_device *sysmmu = of_find_device_by_node(spec->np);
> + struct platform_device *sysmmu = NULL;
> struct sysmmu_drvdata *data;
>
> + if (is_of_node(args->fwnode))
> + sysmmu = of_find_device_by_node(to_of_node(args->fwnode));
> +
> if (!sysmmu)
> return -ENODEV;
>
> @@ -1290,7 +1293,7 @@ static struct iommu_ops exynos_iommu_ops = {
> .add_device = exynos_iommu_add_device,
> .remove_device = exynos_iommu_remove_device,
> .pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE,
> - .of_xlate = exynos_iommu_of_xlate,
> + .fw_xlate = exynos_iommu_fw_xlate,
> };
>
> static bool init_done;
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 929a66a..e08dc0a 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -436,20 +436,23 @@ static struct iommu_group *mtk_iommu_device_group(struct device *dev)
> return data->m4u_group;
> }
>
> -static int mtk_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
> +static int mtk_iommu_fw_xlate(struct device *dev, struct iommu_fwspec *args)
> {
> struct mtk_iommu_client_priv *head, *priv, *next;
> struct platform_device *m4updev;
>
> - if (args->args_count != 1) {
> + if (!is_of_node(args->fwnode))
> + return -ENODEV;
> +
> + if (args->param_count != 1) {
> dev_err(dev, "invalid #iommu-cells(%d) property for IOMMU\n",
> - args->args_count);
> + args->param_count);
> return -EINVAL;
> }
>
> if (!dev->archdata.iommu) {
> /* Get the m4u device */
> - m4updev = of_find_device_by_node(args->np);
> + m4updev = of_find_device_by_node(to_of_node(args->fwnode));
> of_node_put(args->np);
> if (WARN_ON(!m4updev))
> return -EINVAL;
> @@ -469,7 +472,7 @@ static int mtk_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
> if (!priv)
> goto err_free_mem;
>
> - priv->mtk_m4u_id = args->args[0];
> + priv->mtk_m4u_id = args->param[0];
> list_add_tail(&priv->client, &head->client);
>
> return 0;
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 910826c..331dd78 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -135,6 +135,18 @@ struct iommu_ops *of_iommu_get_ops(struct device_node *np)
> return ops;
> }
>
> +static void of_phandle_args_to_fwspec(struct of_phandle_args *iommu_data,
> + struct iommu_fwspec *fwspec)
> +{
> + int i;
> +
> + fwspec->fwnode = iommu_data->np ? &iommu_data->np->fwnode : NULL;
> + fwspec->param_count = iommu_data->args_count;
> +
> + for (i = 0; i < iommu_data->args_count; i++)
> + fwspec->param[i] = iommu_data->args[i];
> +}
> +
> static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
> {
> struct of_phandle_args *iommu_spec = data;
> @@ -148,6 +160,7 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
> struct device_node *master_np)
> {
> struct of_phandle_args iommu_spec;
> + struct iommu_fwspec fwspec;
> struct device_node *np = NULL;
> struct iommu_ops *ops = NULL;
> int idx = 0;
> @@ -170,8 +183,9 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
>
> iommu_spec.np = np;
> iommu_spec.args_count = 1;
> + of_phandle_args_to_fwspec(&iommu_spec, &fwspec);
> ops = of_iommu_get_ops(np);
> - if (!ops || !ops->of_xlate || ops->of_xlate(dev, &iommu_spec))
> + if (!ops || !ops->fw_xlate || ops->fw_xlate(dev, &fwspec))
> goto err_put_node;
>
> of_node_put(np);
> @@ -189,7 +203,9 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
> np = iommu_spec.np;
> ops = of_iommu_get_ops(np);
>
> - if (!ops || !ops->of_xlate || ops->of_xlate(dev, &iommu_spec))
> + of_phandle_args_to_fwspec(&iommu_spec, &fwspec);
> +
> + if (!ops || !ops->fw_xlate || ops->fw_xlate(dev, &fwspec))
> goto err_put_node;
>
> of_node_put(np);
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 0bba25e..5184d81 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -85,6 +85,24 @@ struct iommu_domain {
> void *iova_cookie;
> };
>
> +#define IOMMU_MAP_SPEC_PARAMS 16
> +
> +/**
> + * struct iommu_fwspec - generic IOMMU specifier structure
> + *
> + * @fwnode: Pointer to a firmware-specific descriptor
> + * @param_count: Number of device-specific parameters
> + * @param: Device-specific parameters
> + *
> + * This structure, directly modeled after of_phandle_args, is used to
> + * pass a device-specific description of an IOMMU mapping.
> + */
> +struct iommu_fwspec {
> + struct fwnode_handle *fwnode;
> + int param_count;
> + u32 param[IOMMU_MAP_SPEC_PARAMS];
> +};
> +
> enum iommu_cap {
> IOMMU_CAP_CACHE_COHERENCY, /* IOMMU can enforce cache coherent DMA
> transactions */
> @@ -155,7 +173,7 @@ struct iommu_dm_region {
> * @domain_window_disable: Disable a particular window for a domain
> * @domain_set_windows: Set the number of windows for a domain
> * @domain_get_windows: Return the number of windows for a domain
> - * @of_xlate: add OF master IDs to iommu grouping
> + * @fw_xlate: add FW master IDs to iommu grouping
> * @pgsize_bitmap: bitmap of supported page sizes
> * @priv: per-instance data private to the iommu driver
> */
> @@ -196,9 +214,7 @@ struct iommu_ops {
> /* Get the number of windows per domain */
> u32 (*domain_get_windows)(struct iommu_domain *domain);
>
> -#ifdef CONFIG_OF_IOMMU
> - int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
> -#endif
> + int (*fw_xlate)(struct device *dev, struct iommu_fwspec *args);
>
> unsigned long pgsize_bitmap;
> void *priv;

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

2016-04-19 11:30:58

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [RFC PATCH 06/11] drivers: iommu: make of_xlate() interface DT agnostic

Hi Marek,

On Tue, Apr 19, 2016 at 10:28:02AM +0200, Marek Szyprowski wrote:
> Hello,
>
> On 2016-04-14 19:25, Lorenzo Pieralisi wrote:
> >On systems booting with ACPI, the IOMMU drivers require the same
> >kind of id mapping carried out with a DT tree through the of_xlate()
> >API in order to map devices identifiers to IOMMU ones.
> >
> >On ACPI systems, since DT nodes are not present (ie struct
> >device.of_node == NULL), to identify the device requiring the translation
> >the struct device_node (and the structure used to pass translation
> >information - struct of_phandle_args - that contains a struct device_node)
> >cannot be used, so a generic translation structure to be used for IOMMU
> >mapping should be defined, based on the firmware agnostic fwnode_handle
> >type.
> >
> >This patch mechanically refactors/renames the of_xlate API to make
> >it DT agnostic, by declaring a new type (struct iommu_fwspec), that
> >allows the kernel to pass a device identifier (fwnode - which can
> >represent either a DT node or an IOMMU FW node) and by changing the
> >of_xlate signature so that it does not take anymore the DT specific
> >of_phandle_args argument and replaces it with the DT agnostic
> >iommu_fwspec one.
> >
> >Signed-off-by: Lorenzo Pieralisi <[email protected]>
> >Cc: Matthias Brugger <[email protected]>
> >Cc: Will Deacon <[email protected]>
> >Cc: Hanjun Guo <[email protected]>
> >Cc: Rob Herring <[email protected]>
> >Cc: Krzysztof Kozlowski <[email protected]>
> >Cc: Robin Murphy <[email protected]>
> >Cc: Tomasz Nowicki <[email protected]>
> >Cc: Joerg Roedel <[email protected]>
> >Cc: Marek Szyprowski <[email protected]>
>
> I'm not sure if this is the right approach, although I have not enough
> knowledge on ACPI firmware. Do you plan to rewrite all subsystems to the
> new "fwspec" based interface? Right now of_xlate is rather common
> interface used by various subsystems. Maybe it will be much easier to

Yes, that's a valid concern, when you say "it is rather common" though,
it seems to me that the of_xlate footprint is still subsystem specific,
so this patch should be self-contained anyway (granted, doing this
conversion for a specific subsystem is questionable, I guess that what
you are asking is, if you do it for IOMMU, why would not you do it for
other subsystems ?).

It is an RFC for this specific reason.

> just add acpi_xlate callback and plumb it to the generic code? Maybe later
> when similar code will be in other subsystems and drivers, it can be
> unified, having much more real use cases?

Yes, that's a possibility, it means adding yet another hook into
the IOMMU drivers, probably a simpler change than this one, I
posted this code as and RFC to see which direction we want to take
so further feedback is welcome we can then choose the best approach.

Thanks,
Lorenzo

>
> >---
> > drivers/iommu/arm-smmu.c | 12 +++++++-----
> > drivers/iommu/exynos-iommu.c | 11 +++++++----
> > drivers/iommu/mtk_iommu.c | 13 ++++++++-----
> > drivers/iommu/of_iommu.c | 20 ++++++++++++++++++--
> > include/linux/iommu.h | 24 ++++++++++++++++++++----
> > 5 files changed, 60 insertions(+), 20 deletions(-)
> >
> >diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> >index 6c42770..84bcff7 100644
> >--- a/drivers/iommu/arm-smmu.c
> >+++ b/drivers/iommu/arm-smmu.c
> >@@ -1440,18 +1440,20 @@ out_unlock:
> > return ret;
> > }
> >-static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
> >+static int arm_smmu_fw_xlate(struct device *dev, struct iommu_fwspec *args)
> > {
> > struct arm_smmu_device *smmu;
> >- struct platform_device *smmu_pdev;
> >+ struct platform_device *smmu_pdev = NULL;
> >+
> >+ if (is_of_node(args->fwnode))
> >+ smmu_pdev = of_find_device_by_node(to_of_node(args->fwnode));
> >- smmu_pdev = of_find_device_by_node(args->np);
> > if (!smmu_pdev)
> > return -ENODEV;
> > smmu = platform_get_drvdata(smmu_pdev);
> >- return arm_smmu_add_dev_streamid(smmu, dev, args->args[0]);
> >+ return arm_smmu_add_dev_streamid(smmu, dev, args->param[0]);
> > }
> > static struct iommu_ops arm_smmu_ops = {
> >@@ -1468,7 +1470,7 @@ static struct iommu_ops arm_smmu_ops = {
> > .device_group = arm_smmu_device_group,
> > .domain_get_attr = arm_smmu_domain_get_attr,
> > .domain_set_attr = arm_smmu_domain_set_attr,
> >- .of_xlate = arm_smmu_of_xlate,
> >+ .fw_xlate = arm_smmu_fw_xlate,
> > .pgsize_bitmap = -1UL, /* Restricted during device attach */
> > };
> >diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> >index 5ecc86c..84ff5bb 100644
> >--- a/drivers/iommu/exynos-iommu.c
> >+++ b/drivers/iommu/exynos-iommu.c
> >@@ -1250,13 +1250,16 @@ static void exynos_iommu_remove_device(struct device *dev)
> > iommu_group_remove_device(dev);
> > }
> >-static int exynos_iommu_of_xlate(struct device *dev,
> >- struct of_phandle_args *spec)
> >+static int exynos_iommu_fw_xlate(struct device *dev,
> >+ struct iommu_fwspec *args)
> > {
> > struct exynos_iommu_owner *owner = dev->archdata.iommu;
> >- struct platform_device *sysmmu = of_find_device_by_node(spec->np);
> >+ struct platform_device *sysmmu = NULL;
> > struct sysmmu_drvdata *data;
> >+ if (is_of_node(args->fwnode))
> >+ sysmmu = of_find_device_by_node(to_of_node(args->fwnode));
> >+
> > if (!sysmmu)
> > return -ENODEV;
> >@@ -1290,7 +1293,7 @@ static struct iommu_ops exynos_iommu_ops = {
> > .add_device = exynos_iommu_add_device,
> > .remove_device = exynos_iommu_remove_device,
> > .pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE,
> >- .of_xlate = exynos_iommu_of_xlate,
> >+ .fw_xlate = exynos_iommu_fw_xlate,
> > };
> > static bool init_done;
> >diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> >index 929a66a..e08dc0a 100644
> >--- a/drivers/iommu/mtk_iommu.c
> >+++ b/drivers/iommu/mtk_iommu.c
> >@@ -436,20 +436,23 @@ static struct iommu_group *mtk_iommu_device_group(struct device *dev)
> > return data->m4u_group;
> > }
> >-static int mtk_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
> >+static int mtk_iommu_fw_xlate(struct device *dev, struct iommu_fwspec *args)
> > {
> > struct mtk_iommu_client_priv *head, *priv, *next;
> > struct platform_device *m4updev;
> >- if (args->args_count != 1) {
> >+ if (!is_of_node(args->fwnode))
> >+ return -ENODEV;
> >+
> >+ if (args->param_count != 1) {
> > dev_err(dev, "invalid #iommu-cells(%d) property for IOMMU\n",
> >- args->args_count);
> >+ args->param_count);
> > return -EINVAL;
> > }
> > if (!dev->archdata.iommu) {
> > /* Get the m4u device */
> >- m4updev = of_find_device_by_node(args->np);
> >+ m4updev = of_find_device_by_node(to_of_node(args->fwnode));
> > of_node_put(args->np);
> > if (WARN_ON(!m4updev))
> > return -EINVAL;
> >@@ -469,7 +472,7 @@ static int mtk_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
> > if (!priv)
> > goto err_free_mem;
> >- priv->mtk_m4u_id = args->args[0];
> >+ priv->mtk_m4u_id = args->param[0];
> > list_add_tail(&priv->client, &head->client);
> > return 0;
> >diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> >index 910826c..331dd78 100644
> >--- a/drivers/iommu/of_iommu.c
> >+++ b/drivers/iommu/of_iommu.c
> >@@ -135,6 +135,18 @@ struct iommu_ops *of_iommu_get_ops(struct device_node *np)
> > return ops;
> > }
> >+static void of_phandle_args_to_fwspec(struct of_phandle_args *iommu_data,
> >+ struct iommu_fwspec *fwspec)
> >+{
> >+ int i;
> >+
> >+ fwspec->fwnode = iommu_data->np ? &iommu_data->np->fwnode : NULL;
> >+ fwspec->param_count = iommu_data->args_count;
> >+
> >+ for (i = 0; i < iommu_data->args_count; i++)
> >+ fwspec->param[i] = iommu_data->args[i];
> >+}
> >+
> > static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
> > {
> > struct of_phandle_args *iommu_spec = data;
> >@@ -148,6 +160,7 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
> > struct device_node *master_np)
> > {
> > struct of_phandle_args iommu_spec;
> >+ struct iommu_fwspec fwspec;
> > struct device_node *np = NULL;
> > struct iommu_ops *ops = NULL;
> > int idx = 0;
> >@@ -170,8 +183,9 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
> > iommu_spec.np = np;
> > iommu_spec.args_count = 1;
> >+ of_phandle_args_to_fwspec(&iommu_spec, &fwspec);
> > ops = of_iommu_get_ops(np);
> >- if (!ops || !ops->of_xlate || ops->of_xlate(dev, &iommu_spec))
> >+ if (!ops || !ops->fw_xlate || ops->fw_xlate(dev, &fwspec))
> > goto err_put_node;
> > of_node_put(np);
> >@@ -189,7 +203,9 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
> > np = iommu_spec.np;
> > ops = of_iommu_get_ops(np);
> >- if (!ops || !ops->of_xlate || ops->of_xlate(dev, &iommu_spec))
> >+ of_phandle_args_to_fwspec(&iommu_spec, &fwspec);
> >+
> >+ if (!ops || !ops->fw_xlate || ops->fw_xlate(dev, &fwspec))
> > goto err_put_node;
> > of_node_put(np);
> >diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> >index 0bba25e..5184d81 100644
> >--- a/include/linux/iommu.h
> >+++ b/include/linux/iommu.h
> >@@ -85,6 +85,24 @@ struct iommu_domain {
> > void *iova_cookie;
> > };
> >+#define IOMMU_MAP_SPEC_PARAMS 16
> >+
> >+/**
> >+ * struct iommu_fwspec - generic IOMMU specifier structure
> >+ *
> >+ * @fwnode: Pointer to a firmware-specific descriptor
> >+ * @param_count: Number of device-specific parameters
> >+ * @param: Device-specific parameters
> >+ *
> >+ * This structure, directly modeled after of_phandle_args, is used to
> >+ * pass a device-specific description of an IOMMU mapping.
> >+ */
> >+struct iommu_fwspec {
> >+ struct fwnode_handle *fwnode;
> >+ int param_count;
> >+ u32 param[IOMMU_MAP_SPEC_PARAMS];
> >+};
> >+
> > enum iommu_cap {
> > IOMMU_CAP_CACHE_COHERENCY, /* IOMMU can enforce cache coherent DMA
> > transactions */
> >@@ -155,7 +173,7 @@ struct iommu_dm_region {
> > * @domain_window_disable: Disable a particular window for a domain
> > * @domain_set_windows: Set the number of windows for a domain
> > * @domain_get_windows: Return the number of windows for a domain
> >- * @of_xlate: add OF master IDs to iommu grouping
> >+ * @fw_xlate: add FW master IDs to iommu grouping
> > * @pgsize_bitmap: bitmap of supported page sizes
> > * @priv: per-instance data private to the iommu driver
> > */
> >@@ -196,9 +214,7 @@ struct iommu_ops {
> > /* Get the number of windows per domain */
> > u32 (*domain_get_windows)(struct iommu_domain *domain);
> >-#ifdef CONFIG_OF_IOMMU
> >- int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
> >-#endif
> >+ int (*fw_xlate)(struct device *dev, struct iommu_fwspec *args);
> > unsigned long pgsize_bitmap;
> > void *priv;
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>

2016-04-20 07:14:55

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [RFC PATCH 06/11] drivers: iommu: make of_xlate() interface DT agnostic

Hi Lorenzo,

On 2016-04-19 13:30, Lorenzo Pieralisi wrote:
> Hi Marek,
>
> On Tue, Apr 19, 2016 at 10:28:02AM +0200, Marek Szyprowski wrote:
>> Hello,
>>
>> On 2016-04-14 19:25, Lorenzo Pieralisi wrote:
>>> On systems booting with ACPI, the IOMMU drivers require the same
>>> kind of id mapping carried out with a DT tree through the of_xlate()
>>> API in order to map devices identifiers to IOMMU ones.
>>>
>>> On ACPI systems, since DT nodes are not present (ie struct
>>> device.of_node == NULL), to identify the device requiring the translation
>>> the struct device_node (and the structure used to pass translation
>>> information - struct of_phandle_args - that contains a struct device_node)
>>> cannot be used, so a generic translation structure to be used for IOMMU
>>> mapping should be defined, based on the firmware agnostic fwnode_handle
>>> type.
>>>
>>> This patch mechanically refactors/renames the of_xlate API to make
>>> it DT agnostic, by declaring a new type (struct iommu_fwspec), that
>>> allows the kernel to pass a device identifier (fwnode - which can
>>> represent either a DT node or an IOMMU FW node) and by changing the
>>> of_xlate signature so that it does not take anymore the DT specific
>>> of_phandle_args argument and replaces it with the DT agnostic
>>> iommu_fwspec one.
>>>
>>> Signed-off-by: Lorenzo Pieralisi <[email protected]>
>>> Cc: Matthias Brugger <[email protected]>
>>> Cc: Will Deacon <[email protected]>
>>> Cc: Hanjun Guo <[email protected]>
>>> Cc: Rob Herring <[email protected]>
>>> Cc: Krzysztof Kozlowski <[email protected]>
>>> Cc: Robin Murphy <[email protected]>
>>> Cc: Tomasz Nowicki <[email protected]>
>>> Cc: Joerg Roedel <[email protected]>
>>> Cc: Marek Szyprowski <[email protected]>
>> I'm not sure if this is the right approach, although I have not enough
>> knowledge on ACPI firmware. Do you plan to rewrite all subsystems to the
>> new "fwspec" based interface? Right now of_xlate is rather common
>> interface used by various subsystems. Maybe it will be much easier to
> Yes, that's a valid concern, when you say "it is rather common" though,
> it seems to me that the of_xlate footprint is still subsystem specific,
> so this patch should be self-contained anyway (granted, doing this
> conversion for a specific subsystem is questionable, I guess that what
> you are asking is, if you do it for IOMMU, why would not you do it for
> other subsystems ?).

I was curious if you want to replace of_xlate() interface in other
subsystems
like clocks, power domains, regulators, etc. Each of_xlate interface is
specific to particular subsystem, but they all more or less follows the same
style, what makes it easier to understand the code.

> It is an RFC for this specific reason.
>
>> just add acpi_xlate callback and plumb it to the generic code? Maybe later
>> when similar code will be in other subsystems and drivers, it can be
>> unified, having much more real use cases?
> Yes, that's a possibility, it means adding yet another hook into
> the IOMMU drivers, probably a simpler change than this one, I
> posted this code as and RFC to see which direction we want to take
> so further feedback is welcome we can then choose the best approach.
>
> Thanks,
> Lorenzo
>
>>> ---
>>> drivers/iommu/arm-smmu.c | 12 +++++++-----
>>> drivers/iommu/exynos-iommu.c | 11 +++++++----
>>> drivers/iommu/mtk_iommu.c | 13 ++++++++-----
>>> drivers/iommu/of_iommu.c | 20 ++++++++++++++++++--
>>> include/linux/iommu.h | 24 ++++++++++++++++++++----
>>> 5 files changed, 60 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>>> index 6c42770..84bcff7 100644
>>> --- a/drivers/iommu/arm-smmu.c
>>> +++ b/drivers/iommu/arm-smmu.c
>>> @@ -1440,18 +1440,20 @@ out_unlock:
>>> return ret;
>>> }
>>> -static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
>>> +static int arm_smmu_fw_xlate(struct device *dev, struct iommu_fwspec *args)
>>> {
>>> struct arm_smmu_device *smmu;
>>> - struct platform_device *smmu_pdev;
>>> + struct platform_device *smmu_pdev = NULL;
>>> +
>>> + if (is_of_node(args->fwnode))
>>> + smmu_pdev = of_find_device_by_node(to_of_node(args->fwnode));
>>> - smmu_pdev = of_find_device_by_node(args->np);
>>> if (!smmu_pdev)
>>> return -ENODEV;
>>> smmu = platform_get_drvdata(smmu_pdev);
>>> - return arm_smmu_add_dev_streamid(smmu, dev, args->args[0]);
>>> + return arm_smmu_add_dev_streamid(smmu, dev, args->param[0]);
>>> }
>>> static struct iommu_ops arm_smmu_ops = {
>>> @@ -1468,7 +1470,7 @@ static struct iommu_ops arm_smmu_ops = {
>>> .device_group = arm_smmu_device_group,
>>> .domain_get_attr = arm_smmu_domain_get_attr,
>>> .domain_set_attr = arm_smmu_domain_set_attr,
>>> - .of_xlate = arm_smmu_of_xlate,
>>> + .fw_xlate = arm_smmu_fw_xlate,
>>> .pgsize_bitmap = -1UL, /* Restricted during device attach */
>>> };
>>> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
>>> index 5ecc86c..84ff5bb 100644
>>> --- a/drivers/iommu/exynos-iommu.c
>>> +++ b/drivers/iommu/exynos-iommu.c
>>> @@ -1250,13 +1250,16 @@ static void exynos_iommu_remove_device(struct device *dev)
>>> iommu_group_remove_device(dev);
>>> }
>>> -static int exynos_iommu_of_xlate(struct device *dev,
>>> - struct of_phandle_args *spec)
>>> +static int exynos_iommu_fw_xlate(struct device *dev,
>>> + struct iommu_fwspec *args)
>>> {
>>> struct exynos_iommu_owner *owner = dev->archdata.iommu;
>>> - struct platform_device *sysmmu = of_find_device_by_node(spec->np);
>>> + struct platform_device *sysmmu = NULL;
>>> struct sysmmu_drvdata *data;
>>> + if (is_of_node(args->fwnode))
>>> + sysmmu = of_find_device_by_node(to_of_node(args->fwnode));
>>> +
>>> if (!sysmmu)
>>> return -ENODEV;
>>> @@ -1290,7 +1293,7 @@ static struct iommu_ops exynos_iommu_ops = {
>>> .add_device = exynos_iommu_add_device,
>>> .remove_device = exynos_iommu_remove_device,
>>> .pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE,
>>> - .of_xlate = exynos_iommu_of_xlate,
>>> + .fw_xlate = exynos_iommu_fw_xlate,
>>> };
>>> static bool init_done;
>>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
>>> index 929a66a..e08dc0a 100644
>>> --- a/drivers/iommu/mtk_iommu.c
>>> +++ b/drivers/iommu/mtk_iommu.c
>>> @@ -436,20 +436,23 @@ static struct iommu_group *mtk_iommu_device_group(struct device *dev)
>>> return data->m4u_group;
>>> }
>>> -static int mtk_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
>>> +static int mtk_iommu_fw_xlate(struct device *dev, struct iommu_fwspec *args)
>>> {
>>> struct mtk_iommu_client_priv *head, *priv, *next;
>>> struct platform_device *m4updev;
>>> - if (args->args_count != 1) {
>>> + if (!is_of_node(args->fwnode))
>>> + return -ENODEV;
>>> +
>>> + if (args->param_count != 1) {
>>> dev_err(dev, "invalid #iommu-cells(%d) property for IOMMU\n",
>>> - args->args_count);
>>> + args->param_count);
>>> return -EINVAL;
>>> }
>>> if (!dev->archdata.iommu) {
>>> /* Get the m4u device */
>>> - m4updev = of_find_device_by_node(args->np);
>>> + m4updev = of_find_device_by_node(to_of_node(args->fwnode));
>>> of_node_put(args->np);
>>> if (WARN_ON(!m4updev))
>>> return -EINVAL;
>>> @@ -469,7 +472,7 @@ static int mtk_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
>>> if (!priv)
>>> goto err_free_mem;
>>> - priv->mtk_m4u_id = args->args[0];
>>> + priv->mtk_m4u_id = args->param[0];
>>> list_add_tail(&priv->client, &head->client);
>>> return 0;
>>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>>> index 910826c..331dd78 100644
>>> --- a/drivers/iommu/of_iommu.c
>>> +++ b/drivers/iommu/of_iommu.c
>>> @@ -135,6 +135,18 @@ struct iommu_ops *of_iommu_get_ops(struct device_node *np)
>>> return ops;
>>> }
>>> +static void of_phandle_args_to_fwspec(struct of_phandle_args *iommu_data,
>>> + struct iommu_fwspec *fwspec)
>>> +{
>>> + int i;
>>> +
>>> + fwspec->fwnode = iommu_data->np ? &iommu_data->np->fwnode : NULL;
>>> + fwspec->param_count = iommu_data->args_count;
>>> +
>>> + for (i = 0; i < iommu_data->args_count; i++)
>>> + fwspec->param[i] = iommu_data->args[i];
>>> +}
>>> +
>>> static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
>>> {
>>> struct of_phandle_args *iommu_spec = data;
>>> @@ -148,6 +160,7 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
>>> struct device_node *master_np)
>>> {
>>> struct of_phandle_args iommu_spec;
>>> + struct iommu_fwspec fwspec;
>>> struct device_node *np = NULL;
>>> struct iommu_ops *ops = NULL;
>>> int idx = 0;
>>> @@ -170,8 +183,9 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
>>> iommu_spec.np = np;
>>> iommu_spec.args_count = 1;
>>> + of_phandle_args_to_fwspec(&iommu_spec, &fwspec);
>>> ops = of_iommu_get_ops(np);
>>> - if (!ops || !ops->of_xlate || ops->of_xlate(dev, &iommu_spec))
>>> + if (!ops || !ops->fw_xlate || ops->fw_xlate(dev, &fwspec))
>>> goto err_put_node;
>>> of_node_put(np);
>>> @@ -189,7 +203,9 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
>>> np = iommu_spec.np;
>>> ops = of_iommu_get_ops(np);
>>> - if (!ops || !ops->of_xlate || ops->of_xlate(dev, &iommu_spec))
>>> + of_phandle_args_to_fwspec(&iommu_spec, &fwspec);
>>> +
>>> + if (!ops || !ops->fw_xlate || ops->fw_xlate(dev, &fwspec))
>>> goto err_put_node;
>>> of_node_put(np);
>>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>>> index 0bba25e..5184d81 100644
>>> --- a/include/linux/iommu.h
>>> +++ b/include/linux/iommu.h
>>> @@ -85,6 +85,24 @@ struct iommu_domain {
>>> void *iova_cookie;
>>> };
>>> +#define IOMMU_MAP_SPEC_PARAMS 16
>>> +
>>> +/**
>>> + * struct iommu_fwspec - generic IOMMU specifier structure
>>> + *
>>> + * @fwnode: Pointer to a firmware-specific descriptor
>>> + * @param_count: Number of device-specific parameters
>>> + * @param: Device-specific parameters
>>> + *
>>> + * This structure, directly modeled after of_phandle_args, is used to
>>> + * pass a device-specific description of an IOMMU mapping.
>>> + */
>>> +struct iommu_fwspec {
>>> + struct fwnode_handle *fwnode;
>>> + int param_count;
>>> + u32 param[IOMMU_MAP_SPEC_PARAMS];
>>> +};
>>> +
>>> enum iommu_cap {
>>> IOMMU_CAP_CACHE_COHERENCY, /* IOMMU can enforce cache coherent DMA
>>> transactions */
>>> @@ -155,7 +173,7 @@ struct iommu_dm_region {
>>> * @domain_window_disable: Disable a particular window for a domain
>>> * @domain_set_windows: Set the number of windows for a domain
>>> * @domain_get_windows: Return the number of windows for a domain
>>> - * @of_xlate: add OF master IDs to iommu grouping
>>> + * @fw_xlate: add FW master IDs to iommu grouping
>>> * @pgsize_bitmap: bitmap of supported page sizes
>>> * @priv: per-instance data private to the iommu driver
>>> */
>>> @@ -196,9 +214,7 @@ struct iommu_ops {
>>> /* Get the number of windows per domain */
>>> u32 (*domain_get_windows)(struct iommu_domain *domain);
>>> -#ifdef CONFIG_OF_IOMMU
>>> - int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
>>> -#endif
>>> + int (*fw_xlate)(struct device *dev, struct iommu_fwspec *args);
>>> unsigned long pgsize_bitmap;
>>> void *priv;

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

2016-04-21 22:45:41

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [RFC PATCH 09/11] drivers: acpi: implement acpi_dma_configure

On Thu, Apr 14, 2016 at 8:25 PM, Lorenzo Pieralisi
<[email protected]> wrote:
> On DT based systems, the of_dma_configure() API implements DMA configuration
> for a given device. On ACPI systems an API equivalent to of_dma_configure()
> is missing which implies that it is currently not possible to set-up DMA
> operations for devices through the ACPI generic kernel layer.
>
> This patch fills the gap by introducing acpi_dma_configure/deconfigure()
> calls, that carry out IOMMU configuration through IORT (on systems where
> it is present) and call arch_setup_dma_ops(...) with the retrieved
> parameters.
>
> The DMA range size passed to arch_setup_dma_ops() is sized according
> to the device coherent_dma_mask (starting at address 0x0), mirroring the
> DT probing path behaviour when a dma-ranges property is not provided
> for the device being probed; this changes the current arch_setup_dma_ops()
> call parameters in the ACPI probing case, but since arch_setup_dma_ops()
> is a NOP on all architectures but ARM/ARM64 this patch does not change
> the current kernel behaviour on them.
>
> This patch updates ACPI and PCI core code to use the newly introduced
> acpi_dma_configure function, providing the same functionality
> as of_dma_configure on ARM systems and leaving behaviour unchanged
> for all other arches.
>

Nitpicks below.

> --- a/drivers/acpi/iort.c
> +++ b/drivers/acpi/iort.c
> @@ -72,6 +72,31 @@ int iort_iommu_set_node(struct iommu_ops *ops, struct acpi_iort_node *node,
> return 0;
> }
>
> +/**
> + * iort_iommu_get_node - Retrieve iort_iommu_node associated with an IORT node.
> + *
> + * @node: IORT table node to be looked-up
> + *
> + * Returns: iort_iommu_node pointer on success
> + * NULL on failure
> + */
> +static struct iort_iommu_node *iort_iommu_get_node(struct acpi_iort_node *node)
> +{
> + struct iort_iommu_node *iommu_node;
> +
> + spin_lock(&iort_iommu_lock);
> + list_for_each_entry(iommu_node, &iort_iommu_list, list) {
> + if (iommu_node->node == node)
> + goto found;
> + }
> +
> + iommu_node = NULL;
> +found:
> + spin_unlock(&iort_iommu_lock);
> +
> + return iommu_node;

Ouch, and why not to

strut iommu_node = NULL;

lock
list for each() {
if ()
break;
}
unlock

return iommu_node;

?

> +}


> +/**
> + * iort_iommu_configure - Set-up IOMMU configuration for a device.
> + *
> + * @dev: device that requires IOMMU set-up
> + *
> + * Returns: iommu_ops pointer on configuration success
> + * NULL on configuration failure
> + */
> +struct iommu_ops *iort_iommu_configure(struct device *dev)
> +{
> + struct acpi_iort_node *node, *parent;
> + struct iommu_ops *ops = NULL;
> + struct iommu_fwspec fwspec;
> + struct iort_iommu_node *iommu_node;
> + u32 rid = 0, devid = 0;
> +
> + if (dev_is_pci(dev)) {
> + struct pci_bus *bus = to_pci_dev(dev)->bus;
> +
> + pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
> + &rid);
> +
> + node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
> + iort_find_dev_callback, &bus->dev);

> + } else

checkpatch.pl ?

> + node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
> + iort_find_dev_callback, dev);
> +
> + if (!node)
> + return NULL;
> +
> + iort_dev_map_rid(node, rid, &devid, ACPI_IORT_NODE_SMMU);
> +
> + parent = iort_find_parent_node(node, ACPI_IORT_NODE_SMMU);

> +

Redundant.

> + if (!parent)
> + return NULL;
> +
> + iommu_node = iort_iommu_get_node(parent);
> + ops = iommu_node->ops;
> +
> + fwspec.fwnode = iommu_node->fwnode;
> + fwspec.param_count = 1;
> + fwspec.param[0] = devid;
> +
> + if (!ops || !ops->fw_xlate || ops->fw_xlate(dev, &fwspec))
> + return NULL;
> +
> + return ops;
> +}
> +

--
With Best Regards,
Andy Shevchenko

2016-04-22 10:58:00

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [RFC PATCH 09/11] drivers: acpi: implement acpi_dma_configure

Hi Andy,

On Fri, Apr 22, 2016 at 01:45:38AM +0300, Andy Shevchenko wrote:
> On Thu, Apr 14, 2016 at 8:25 PM, Lorenzo Pieralisi
> <[email protected]> wrote:
> > On DT based systems, the of_dma_configure() API implements DMA configuration
> > for a given device. On ACPI systems an API equivalent to of_dma_configure()
> > is missing which implies that it is currently not possible to set-up DMA
> > operations for devices through the ACPI generic kernel layer.
> >
> > This patch fills the gap by introducing acpi_dma_configure/deconfigure()
> > calls, that carry out IOMMU configuration through IORT (on systems where
> > it is present) and call arch_setup_dma_ops(...) with the retrieved
> > parameters.
> >
> > The DMA range size passed to arch_setup_dma_ops() is sized according
> > to the device coherent_dma_mask (starting at address 0x0), mirroring the
> > DT probing path behaviour when a dma-ranges property is not provided
> > for the device being probed; this changes the current arch_setup_dma_ops()
> > call parameters in the ACPI probing case, but since arch_setup_dma_ops()
> > is a NOP on all architectures but ARM/ARM64 this patch does not change
> > the current kernel behaviour on them.
> >
> > This patch updates ACPI and PCI core code to use the newly introduced
> > acpi_dma_configure function, providing the same functionality
> > as of_dma_configure on ARM systems and leaving behaviour unchanged
> > for all other arches.
> >
>
> Nitpicks below.

Thanks for having a look.

> > --- a/drivers/acpi/iort.c
> > +++ b/drivers/acpi/iort.c
> > @@ -72,6 +72,31 @@ int iort_iommu_set_node(struct iommu_ops *ops, struct acpi_iort_node *node,
> > return 0;
> > }
> >
> > +/**
> > + * iort_iommu_get_node - Retrieve iort_iommu_node associated with an IORT node.
> > + *
> > + * @node: IORT table node to be looked-up
> > + *
> > + * Returns: iort_iommu_node pointer on success
> > + * NULL on failure
> > + */
> > +static struct iort_iommu_node *iort_iommu_get_node(struct acpi_iort_node *node)
> > +{
> > + struct iort_iommu_node *iommu_node;
> > +
> > + spin_lock(&iort_iommu_lock);
> > + list_for_each_entry(iommu_node, &iort_iommu_list, list) {
> > + if (iommu_node->node == node)
> > + goto found;
> > + }
> > +
> > + iommu_node = NULL;
> > +found:
> > + spin_unlock(&iort_iommu_lock);
> > +
> > + return iommu_node;
>
> Ouch, and why not to
>
> strut iommu_node = NULL;
>
> lock
> list for each() {
> if ()
> break;
> }
> unlock
>
> return iommu_node;

To make sure iommu_node is NULL if no node is found, but this list handling
function needs updating anyway (both locking and list handling), so I will
rework it and take your suggestion into account, I agree it is not that
readable (or safe to begin with).

> ?
>
> > +}
>
>
> > +/**
> > + * iort_iommu_configure - Set-up IOMMU configuration for a device.
> > + *
> > + * @dev: device that requires IOMMU set-up
> > + *
> > + * Returns: iommu_ops pointer on configuration success
> > + * NULL on configuration failure
> > + */
> > +struct iommu_ops *iort_iommu_configure(struct device *dev)
> > +{
> > + struct acpi_iort_node *node, *parent;
> > + struct iommu_ops *ops = NULL;
> > + struct iommu_fwspec fwspec;
> > + struct iort_iommu_node *iommu_node;
> > + u32 rid = 0, devid = 0;
> > +
> > + if (dev_is_pci(dev)) {
> > + struct pci_bus *bus = to_pci_dev(dev)->bus;
> > +
> > + pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
> > + &rid);
> > +
> > + node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
> > + iort_find_dev_callback, &bus->dev);
>
> > + } else
>
> checkpatch.pl ?

checkpatch.pl --strict does not even barf at it. I will add the braces
and go check why checkpatch.pl is quiet :)

> > + node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
> > + iort_find_dev_callback, dev);
> > +
> > + if (!node)
> > + return NULL;
> > +
> > + iort_dev_map_rid(node, rid, &devid, ACPI_IORT_NODE_SMMU);
> > +
> > + parent = iort_find_parent_node(node, ACPI_IORT_NODE_SMMU);
>
> > +
>
> Redundant.

Ok.

Thanks,
Lorenzo

>

> > + if (!parent)
> > + return NULL;
> > +
> > + iommu_node = iort_iommu_get_node(parent);
> > + ops = iommu_node->ops;
> > +
> > + fwspec.fwnode = iommu_node->fwnode;
> > + fwspec.param_count = 1;
> > + fwspec.param[0] = devid;
> > +
> > + if (!ops || !ops->fw_xlate || ops->fw_xlate(dev, &fwspec))
> > + return NULL;
> > +
> > + return ops;
> > +}
> > +
>
> --
> With Best Regards,
> Andy Shevchenko
>