2016-11-09 14:19:03

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v7 00/16] ACPI IORT ARM SMMU support

This patch series is v7 of a previous posting:

https://lkml.org/lkml/2016/10/18/506

v6 -> v7
- Rebased against v4.9-rc4
- Fixed IORT probing on ACPI systems with missing IORT table
- Fixed SMMUv1/v2 global interrupt detection
- Updated iommu_ops firmware look-up

v5 -> v6
- Rebased against v4.9-rc1
- Changed FWNODE_IOMMU to FWNODE_ACPI_STATIC
- Moved platform devices creation into IORT code
- Updated fwnode handling
- Added default dma masks initialization

v4 -> v5
- Added SMMUv1/v2 support
- Rebased against v4.8-rc5 and dependencies series
- Consolidated IORT platform devices creation

v3 -> v4
- Added single mapping API (for IORT named components)
- Fixed arm_smmu_iort_xlate() return value
- Reworked fwnode registration and platform device creation
ordering to fix probe ordering dependencies
- Added code to keep device_node ref count with new iommu
fwspec API
- Added patch to make iommu_fwspec arch agnostic
- Dropped RFC status
- Rebased against v4.8-rc2

v2 -> v3
- Rebased on top of dependencies series [1][2][3](v4.7-rc3)
- Added back reliance on ACPI early probing infrastructure
- Patch[1-3] merged through other dependent series
- Added back IOMMU fwnode generalization
- Move SMMU v3 static functions configuration to IORT code
- Implemented generic IOMMU fwspec API
- Added code to implement fwnode platform device look-up

v1 -> v2:
- Rebased on top of dependencies series [1][2][3](v4.7-rc1)
- Removed IOMMU fwnode generalization
- Implemented ARM SMMU v3 ACPI probing instead of ARM SMMU v2
owing to patch series dependencies [1]
- Moved platform device creation logic to IORT code to
generalize its usage for ARM SMMU v1-v2-v3 components
- Removed reliance on ACPI early device probing
- Created IORT specific iommu_xlate() translation hook leaving
OF code unchanged according to v1 reviews

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, this patchset enables ARM SMMUs 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 future ARM SMMU components easier to integrate.

PATCH (1) adds a FWNODE_ACPI_STATIC type to the struct fwnode_handle type.
It is required to attach a fwnode identifier to platform
devices allocated/detected through static ACPI table entries
(ie 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).

PATCH (2) makes use of the ACPI early probing API to add a linker script
section for probing devices via IORT ACPI kernel code.

PATCH (3) provides IORT support for registering IOMMU IORT node through
their fwnode handle.

PATCH (4) make of_iommu_{set/get}_ops() functions DT agnostic.

PATCH (5) convert ARM SMMU driver to use fwnode instead of of_node as
look-up and iommu_ops retrieval token.

PATCH (6) convert ARM SMMU v3 driver to use fwnode instead of of_node as
look-up and iommu_ops retrieval token.

PATCH (7) 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 (8) provides an IORT function to detect existence of specific type
of IORT components.

PATCH (9) creates the kernel infrastructure required to create ARM SMMU
platform devices for IORT nodes.

PATCH (10) refactors the ARM SMMU v3 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 (11) adds ARM SMMU v3 IORT IOMMU operations to create and probe
ARM SMMU v3 components.

PATCH (12) refactors the ARM SMMU v1/v2 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 (13) adds ARM SMMU v1/v2 IORT IOMMU operations to create and
probe ARM SMMU v1/v2 components.

PATCH (14) Extend the IORT iort_node_map_rid() to work on a type mask
instead of a single type so that the translation API can
be used on a range of components.

PATCH (15) Add IORT API to carry out id mappings for components that do
do not have an input identifier/RIDs (ie named components).

PATCH (16) provides IORT infrastructure to carry out IOMMU configuration
for devices and hook it up to the previously introduced ACPI
DMA configure API.

This patchset is provided for review/testing purposes here:

git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git acpi/iort-smmu-v7

Tested on Juno and FVP models for ARM SMMU v1 and v3 probing path.

Lorenzo Pieralisi (16):
drivers: acpi: add FWNODE_ACPI_STATIC fwnode type
drivers: acpi: iort: introduce linker section for IORT entries probing
drivers: acpi: iort: add support for IOMMU fwnode registration
drivers: iommu: make of_iommu_set/get_ops() DT agnostic
drivers: iommu: arm-smmu: convert struct device of_node to fwnode
usage
drivers: iommu: arm-smmu-v3: convert struct device of_node to fwnode
usage
drivers: acpi: implement acpi_dma_configure
drivers: acpi: iort: add node match function
drivers: acpi: iort: add support for ARM SMMU platform devices
creation
drivers: iommu: arm-smmu-v3: split probe functions into DT/generic
portions
drivers: iommu: arm-smmu-v3: add IORT configuration
drivers: iommu: arm-smmu: split probe functions into DT/generic
portions
drivers: iommu: arm-smmu: add IORT configuration
drivers: acpi: iort: replace rid map type with type mask
drivers: acpi: iort: add single mapping function
drivers: acpi: iort: introduce iort_iommu_configure

drivers/acpi/arm64/iort.c | 596 +++++++++++++++++++++++++++++++++++++-
drivers/acpi/glue.c | 4 +-
drivers/acpi/scan.c | 45 +++
drivers/iommu/arm-smmu-v3.c | 105 +++++--
drivers/iommu/arm-smmu.c | 154 ++++++++--
drivers/iommu/iommu.c | 42 +++
drivers/iommu/of_iommu.c | 39 ---
drivers/pci/probe.c | 3 +-
include/acpi/acpi_bus.h | 2 +
include/asm-generic/vmlinux.lds.h | 1 +
include/linux/acpi.h | 26 ++
include/linux/acpi_iort.h | 14 +
include/linux/fwnode.h | 3 +-
include/linux/iommu.h | 14 +
include/linux/of_iommu.h | 12 +-
15 files changed, 956 insertions(+), 104 deletions(-)

--
2.10.0


2016-11-09 14:19:10

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v7 02/16] 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]>
Reviewed-by: Hanjun Guo <[email protected]>
Tested-by: Hanjun Guo <[email protected]>
Tested-by: Tomasz Nowicki <[email protected]>
Cc: Tomasz Nowicki <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Marc Zyngier <[email protected]>
---
drivers/acpi/arm64/iort.c | 13 ++++++++++---
include/asm-generic/vmlinux.lds.h | 1 +
include/linux/acpi_iort.h | 3 +++
3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 6b81746..2c46ebc 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -361,8 +361,15 @@ void __init acpi_iort_init(void)
acpi_status status;

status = acpi_get_table(ACPI_SIG_IORT, 0, &iort_table);
- if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
- const char *msg = acpi_format_exception(status);
- pr_err("Failed to get table, %s\n", msg);
+ if (ACPI_FAILURE(status)) {
+ if (status != AE_NOT_FOUND) {
+ const char *msg = acpi_format_exception(status);
+
+ pr_err("Failed to get table, %s\n", msg);
+ }
+
+ return;
}
+
+ acpi_probe_device_table(iort);
}
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 3074796..f9c9f3c 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -563,6 +563,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/acpi_iort.h b/include/linux/acpi_iort.h
index 0e32dac..d16fdda 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -39,4 +39,7 @@ static inline struct irq_domain *iort_get_device_domain(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)
+
#endif /* __ACPI_IORT_H__ */
--
2.10.0

2016-11-09 14:19:07

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v7 01/16] drivers: acpi: add FWNODE_ACPI_STATIC fwnode type

On systems booting with a device tree, every struct device is associated
with a struct device_node, that provides its DT firmware 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 operations 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.

The struct acpi_device, that represents firmware objects in the
ACPI namespace already includes a struct fwnode_handle of
type FWNODE_ACPI as their member; the same abstraction is missing
though for devices that are instantiated out of static ACPI tables
entries (eg ARM SMMU devices).

Add a new fwnode_handle type to associate devices created out
of static ACPI table entries to the respective firmware components
and create a simple ACPI core layer interface to dynamically allocate
and free the corresponding firmware nodes so that kernel subsystems
can use it to instantiate the nodes and associate them with the
respective devices.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Reviewed-by: Hanjun Guo <[email protected]>
Tested-by: Hanjun Guo <[email protected]>
Tested-by: Tomasz Nowicki <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
---
include/linux/acpi.h | 21 +++++++++++++++++++++
include/linux/fwnode.h | 3 ++-
2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 689a8b9..6efb13c 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -56,6 +56,27 @@ static inline acpi_handle acpi_device_handle(struct acpi_device *adev)
acpi_fwnode_handle(adev) : NULL)
#define ACPI_HANDLE(dev) acpi_device_handle(ACPI_COMPANION(dev))

+static inline struct fwnode_handle *acpi_alloc_fwnode_static(void)
+{
+ struct fwnode_handle *fwnode;
+
+ fwnode = kzalloc(sizeof(struct fwnode_handle), GFP_KERNEL);
+ if (!fwnode)
+ return NULL;
+
+ fwnode->type = FWNODE_ACPI_STATIC;
+
+ return fwnode;
+}
+
+static inline void acpi_free_fwnode_static(struct fwnode_handle *fwnode)
+{
+ if (WARN_ON(!fwnode || fwnode->type != FWNODE_ACPI_STATIC))
+ return;
+
+ kfree(fwnode);
+}
+
/**
* ACPI_DEVICE_CLASS - macro used to describe an ACPI device with
* the PCI-defined class-code information
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 8516717..8bd28ce 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -17,8 +17,9 @@ enum fwnode_type {
FWNODE_OF,
FWNODE_ACPI,
FWNODE_ACPI_DATA,
+ FWNODE_ACPI_STATIC,
FWNODE_PDATA,
- FWNODE_IRQCHIP,
+ FWNODE_IRQCHIP
};

struct fwnode_handle {
--
2.10.0

2016-11-09 14:19:28

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v7 08/16] drivers: acpi: iort: add node match function

Device drivers (eg ARM SMMU) need to know if a specific component
is part of the IORT table, so that kernel data structures are not
initialized at initcalls time if the respective component is not
part of the IORT table.

To this end, this patch adds a trivial function that allows detecting
if a given IORT node type is present or not in the ACPI table, providing
an ACPI IORT equivalent for of_find_matching_node().

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

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 1ac2720..4bb6acb 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -227,6 +227,21 @@ static struct acpi_iort_node *iort_scan_node(enum acpi_iort_node_type type,
return NULL;
}

+static acpi_status
+iort_match_type_callback(struct acpi_iort_node *node, void *context)
+{
+ return AE_OK;
+}
+
+bool iort_node_match(u8 type)
+{
+ struct acpi_iort_node *node;
+
+ node = iort_scan_node(type, iort_match_type_callback, NULL);
+
+ return node != NULL;
+}
+
static acpi_status iort_match_node_callback(struct acpi_iort_node *node,
void *context)
{
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index d16fdda..17bb078 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -28,10 +28,12 @@ void iort_deregister_domain_token(int trans_id);
struct fwnode_handle *iort_find_domain_token(int trans_id);
#ifdef CONFIG_ACPI_IORT
void acpi_iort_init(void);
+bool iort_node_match(u8 type);
u32 iort_msi_map_rid(struct device *dev, u32 req_id);
struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id);
#else
static inline void acpi_iort_init(void) { }
+static inline bool iort_node_match(u8 type) { return false; }
static inline u32 iort_msi_map_rid(struct device *dev, u32 req_id)
{ return req_id; }
static inline struct irq_domain *iort_get_device_domain(struct device *dev,
--
2.10.0

2016-11-09 14:19:20

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic

The of_iommu_{set/get}_ops() API is used to associate a device
tree node with a specific set of IOMMU operations. The same
kernel interface is required on systems booting with ACPI, where
devices are not associated with a device tree node, therefore
the interface requires generalization.

The struct device fwnode member represents the fwnode token
associated with the device and the struct it points at is firmware
specific; regardless, it is initialized on both ACPI and DT systems
and makes an ideal candidate to use it to associate a set of IOMMU
operations to a given device, through its struct device.fwnode member
pointer.

Convert the DT specific of_iommu_{set/get}_ops() interface to
use struct device.fwnode as a look-up token, making the interface
usable on ACPI systems.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Tested-by: Hanjun Guo <[email protected]>
Tested-by: Tomasz Nowicki <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Hanjun Guo <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Joerg Roedel <[email protected]>
---
drivers/iommu/iommu.c | 42 ++++++++++++++++++++++++++++++++++++++++++
drivers/iommu/of_iommu.c | 39 ---------------------------------------
include/linux/iommu.h | 14 ++++++++++++++
include/linux/of_iommu.h | 12 ++++++++++--
4 files changed, 66 insertions(+), 41 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 9a2f196..5c97c01 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1615,6 +1615,48 @@ int iommu_request_dm_for_dev(struct device *dev)
return ret;
}

+struct iommu_fwentry {
+ struct list_head list;
+ struct fwnode_handle *fwnode;
+ const struct iommu_ops *ops;
+};
+static LIST_HEAD(iommu_fwentry_list);
+static DEFINE_SPINLOCK(iommu_fwentry_lock);
+
+void fwnode_iommu_set_ops(struct fwnode_handle *fwnode,
+ const struct iommu_ops *ops)
+{
+ struct iommu_fwentry *iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
+
+ if (WARN_ON(!iommu))
+ return;
+
+ if (is_of_node(fwnode))
+ of_node_get(to_of_node(fwnode));
+
+ INIT_LIST_HEAD(&iommu->list);
+ iommu->fwnode = fwnode;
+ iommu->ops = ops;
+ spin_lock(&iommu_fwentry_lock);
+ list_add_tail(&iommu->list, &iommu_fwentry_list);
+ spin_unlock(&iommu_fwentry_lock);
+}
+
+const struct iommu_ops *fwnode_iommu_get_ops(struct fwnode_handle *fwnode)
+{
+ struct iommu_fwentry *node;
+ const struct iommu_ops *ops = NULL;
+
+ spin_lock(&iommu_fwentry_lock);
+ list_for_each_entry(node, &iommu_fwentry_list, list)
+ if (node->fwnode == fwnode) {
+ ops = node->ops;
+ break;
+ }
+ spin_unlock(&iommu_fwentry_lock);
+ return ops;
+}
+
int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
const struct iommu_ops *ops)
{
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 5b82862..0f57ddc 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -96,45 +96,6 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
}
EXPORT_SYMBOL_GPL(of_get_dma_window);

-struct of_iommu_node {
- struct list_head list;
- struct device_node *np;
- const struct iommu_ops *ops;
-};
-static LIST_HEAD(of_iommu_list);
-static DEFINE_SPINLOCK(of_iommu_lock);
-
-void of_iommu_set_ops(struct device_node *np, const struct iommu_ops *ops)
-{
- struct of_iommu_node *iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
-
- if (WARN_ON(!iommu))
- return;
-
- of_node_get(np);
- INIT_LIST_HEAD(&iommu->list);
- iommu->np = np;
- iommu->ops = ops;
- spin_lock(&of_iommu_lock);
- list_add_tail(&iommu->list, &of_iommu_list);
- spin_unlock(&of_iommu_lock);
-}
-
-const struct iommu_ops *of_iommu_get_ops(struct device_node *np)
-{
- struct of_iommu_node *node;
- const struct iommu_ops *ops = NULL;
-
- spin_lock(&of_iommu_lock);
- list_for_each_entry(node, &of_iommu_list, list)
- if (node->np == np) {
- ops = node->ops;
- break;
- }
- spin_unlock(&of_iommu_lock);
- return ops;
-}
-
static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
{
struct of_phandle_args *iommu_spec = data;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 436dc21..15d5478 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -351,6 +351,9 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
const struct iommu_ops *ops);
void iommu_fwspec_free(struct device *dev);
int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids);
+void fwnode_iommu_set_ops(struct fwnode_handle *fwnode,
+ const struct iommu_ops *ops);
+const struct iommu_ops *fwnode_iommu_get_ops(struct fwnode_handle *fwnode);

#else /* CONFIG_IOMMU_API */

@@ -580,6 +583,17 @@ static inline int iommu_fwspec_add_ids(struct device *dev, u32 *ids,
return -ENODEV;
}

+static inline void fwnode_iommu_set_ops(struct fwnode_handle *fwnode,
+ const struct iommu_ops *ops)
+{
+}
+
+static inline
+const struct iommu_ops *fwnode_iommu_get_ops(struct fwnode_handle *fwnode)
+{
+ return NULL;
+}
+
#endif /* CONFIG_IOMMU_API */

#endif /* __LINUX_IOMMU_H */
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index e80b9c7..7681007 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -31,8 +31,16 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,

#endif /* CONFIG_OF_IOMMU */

-void of_iommu_set_ops(struct device_node *np, const struct iommu_ops *ops);
-const struct iommu_ops *of_iommu_get_ops(struct device_node *np);
+static inline void of_iommu_set_ops(struct device_node *np,
+ const struct iommu_ops *ops)
+{
+ fwnode_iommu_set_ops(&np->fwnode, ops);
+}
+
+static inline const struct iommu_ops *of_iommu_get_ops(struct device_node *np)
+{
+ return fwnode_iommu_get_ops(&np->fwnode);
+}

extern struct of_device_id __iommu_of_table;

--
2.10.0

2016-11-09 14:19:37

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v7 11/16] drivers: iommu: arm-smmu-v3: add IORT configuration

In ACPI bases systems, in order to be able to create platform
devices and initialize them for ARM SMMU v3 components, the IORT
kernel implementation requires a set of static functions to be
used by the IORT kernel layer to configure platform devices for
ARM SMMU v3 components.

Add static configuration functions to the IORT kernel layer for
the ARM SMMU v3 components, so that the ARM SMMU v3 driver can
initialize its respective platform device by relying on the IORT
kernel infrastructure and by adding a corresponding ACPI device
early probe section entry.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Tested-by: Hanjun Guo <[email protected]>
Tested-by: Tomasz Nowicki <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Joerg Roedel <[email protected]>
---
drivers/acpi/arm64/iort.c | 103 +++++++++++++++++++++++++++++++++++++++++++-
drivers/iommu/arm-smmu-v3.c | 49 ++++++++++++++++++++-
2 files changed, 150 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index ddf83b5..fd52e4c 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -459,6 +459,95 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id)
return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
}

+static void __init acpi_iort_register_irq(int hwirq, const char *name,
+ int trigger,
+ struct resource *res)
+{
+ int irq = acpi_register_gsi(NULL, hwirq, trigger,
+ ACPI_ACTIVE_HIGH);
+
+ if (irq <= 0) {
+ pr_err("could not register gsi hwirq %d name [%s]\n", hwirq,
+ name);
+ return;
+ }
+
+ res->start = irq;
+ res->end = irq;
+ res->flags = IORESOURCE_IRQ;
+ res->name = name;
+}
+
+static int __init arm_smmu_v3_count_resources(struct acpi_iort_node *node)
+{
+ struct acpi_iort_smmu_v3 *smmu;
+ /* Always present mem resource */
+ int num_res = 1;
+
+ /* Retrieve SMMUv3 specific data */
+ smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
+
+ if (smmu->event_gsiv)
+ num_res++;
+
+ if (smmu->pri_gsiv)
+ num_res++;
+
+ if (smmu->gerr_gsiv)
+ num_res++;
+
+ if (smmu->sync_gsiv)
+ num_res++;
+
+ return num_res;
+}
+
+static void __init arm_smmu_v3_init_resources(struct resource *res,
+ struct acpi_iort_node *node)
+{
+ struct acpi_iort_smmu_v3 *smmu;
+ int num_res = 0;
+
+ /* Retrieve SMMUv3 specific data */
+ smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
+
+ res[num_res].start = smmu->base_address;
+ res[num_res].end = smmu->base_address + SZ_128K - 1;
+ res[num_res].flags = IORESOURCE_MEM;
+
+ num_res++;
+
+ if (smmu->event_gsiv)
+ acpi_iort_register_irq(smmu->event_gsiv, "eventq",
+ ACPI_EDGE_SENSITIVE,
+ &res[num_res++]);
+
+ if (smmu->pri_gsiv)
+ acpi_iort_register_irq(smmu->pri_gsiv, "priq",
+ ACPI_EDGE_SENSITIVE,
+ &res[num_res++]);
+
+ if (smmu->gerr_gsiv)
+ acpi_iort_register_irq(smmu->gerr_gsiv, "gerror",
+ ACPI_EDGE_SENSITIVE,
+ &res[num_res++]);
+
+ if (smmu->sync_gsiv)
+ acpi_iort_register_irq(smmu->sync_gsiv, "cmdq-sync",
+ ACPI_EDGE_SENSITIVE,
+ &res[num_res++]);
+}
+
+static bool __init arm_smmu_v3_is_coherent(struct acpi_iort_node *node)
+{
+ struct acpi_iort_smmu_v3 *smmu;
+
+ /* Retrieve SMMUv3 specific data */
+ smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
+
+ return smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE;
+}
+
struct iort_iommu_config {
const char *name;
int (*iommu_init)(struct acpi_iort_node *node);
@@ -468,10 +557,22 @@ struct iort_iommu_config {
struct acpi_iort_node *node);
};

+static const struct iort_iommu_config iort_arm_smmu_v3_cfg __initconst = {
+ .name = "arm-smmu-v3",
+ .iommu_is_coherent = arm_smmu_v3_is_coherent,
+ .iommu_count_resources = arm_smmu_v3_count_resources,
+ .iommu_init_resources = arm_smmu_v3_init_resources
+};
+
static __init
const struct iort_iommu_config *iort_get_iommu_cfg(struct acpi_iort_node *node)
{
- return NULL;
+ switch (node->type) {
+ case ACPI_IORT_NODE_SMMU_V3:
+ return &iort_arm_smmu_v3_cfg;
+ default:
+ return NULL;
+ }
}

/**
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index f970acd..f2401a05 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -20,6 +20,8 @@
* This driver is powered by bad coffee and bombay mix.
*/

+#include <linux/acpi.h>
+#include <linux/acpi_iort.h>
#include <linux/delay.h>
#include <linux/dma-iommu.h>
#include <linux/err.h>
@@ -2559,6 +2561,36 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
return 0;
}

+#ifdef CONFIG_ACPI
+static int arm_smmu_device_acpi_probe(struct platform_device *pdev,
+ struct arm_smmu_device *smmu,
+ bool *bypass)
+{
+ struct acpi_iort_smmu_v3 *iort_smmu;
+ struct device *dev = smmu->dev;
+ struct acpi_iort_node *node;
+
+ node = *(struct acpi_iort_node **)dev_get_platdata(dev);
+
+ /* Retrieve SMMUv3 specific data */
+ iort_smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
+
+ if (iort_smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE)
+ smmu->features |= ARM_SMMU_FEAT_COHERENCY;
+
+ *bypass = false;
+
+ return 0;
+}
+#else
+static inline int arm_smmu_device_acpi_probe(struct platform_device *pdev,
+ struct arm_smmu_device *smmu,
+ bool *bypass)
+{
+ return -ENODEV;
+}
+#endif
+
static int arm_smmu_device_dt_probe(struct platform_device *pdev,
struct arm_smmu_device *smmu,
bool *bypass)
@@ -2626,7 +2658,11 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
if (irq > 0)
smmu->gerr_irq = irq;

- ret = arm_smmu_device_dt_probe(pdev, smmu, &bypass);
+ if (dev->of_node)
+ ret = arm_smmu_device_dt_probe(pdev, smmu, &bypass);
+ else
+ ret = arm_smmu_device_acpi_probe(pdev, smmu, &bypass);
+
if (ret)
return ret;

@@ -2731,6 +2767,17 @@ static int __init arm_smmu_of_init(struct device_node *np)
}
IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3", arm_smmu_of_init);

+#ifdef CONFIG_ACPI
+static int __init acpi_smmu_v3_init(struct acpi_table_header *table)
+{
+ if (iort_node_match(ACPI_IORT_NODE_SMMU_V3))
+ return arm_smmu_init();
+
+ return 0;
+}
+IORT_ACPI_DECLARE(arm_smmu_v3, ACPI_SIG_IORT, acpi_smmu_v3_init);
+#endif
+
MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
MODULE_AUTHOR("Will Deacon <[email protected]>");
MODULE_LICENSE("GPL v2");
--
2.10.0

2016-11-09 14:19:41

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v7 13/16] drivers: iommu: arm-smmu: add IORT configuration

In ACPI bases systems, in order to be able to create platform
devices and initialize them for ARM SMMU components, the IORT
kernel implementation requires a set of static functions to be
used by the IORT kernel layer to configure platform devices for
ARM SMMU components.

Add static configuration functions to the IORT kernel layer for
the ARM SMMU components, so that the ARM SMMU driver can
initialize its respective platform device by relying on the IORT
kernel infrastructure and by adding a corresponding ACPI device
early probe section entry.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Tested-by: Hanjun Guo <[email protected]>
Tested-by: Tomasz Nowicki <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Joerg Roedel <[email protected]>
---
drivers/acpi/arm64/iort.c | 81 +++++++++++++++++++++++++++++++++++++++++++++
drivers/iommu/arm-smmu.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++-
include/linux/acpi_iort.h | 3 ++
3 files changed, 166 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index fd52e4c..4708806 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -548,6 +548,78 @@ static bool __init arm_smmu_v3_is_coherent(struct acpi_iort_node *node)
return smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE;
}

+static int __init arm_smmu_count_resources(struct acpi_iort_node *node)
+{
+ struct acpi_iort_smmu *smmu;
+ int num_irqs;
+ u64 *glb_irq;
+
+ /* Retrieve SMMU specific data */
+ smmu = (struct acpi_iort_smmu *)node->node_data;
+
+ glb_irq = ACPI_ADD_PTR(u64, node, smmu->global_interrupt_offset);
+ if (!IORT_IRQ_MASK(glb_irq[1])) /* 0 means not implemented */
+ num_irqs = 1;
+ else
+ num_irqs = 2;
+
+ num_irqs += smmu->context_interrupt_count;
+
+ return num_irqs + 1;
+}
+
+static void __init arm_smmu_init_resources(struct resource *res,
+ struct acpi_iort_node *node)
+{
+ struct acpi_iort_smmu *smmu;
+ int i, hw_irq, trigger, num_res = 0;
+ u64 *ctx_irq, *glb_irq;
+
+ /* Retrieve SMMU specific data */
+ smmu = (struct acpi_iort_smmu *)node->node_data;
+
+ res[num_res].start = smmu->base_address;
+ res[num_res].end = smmu->base_address + smmu->span - 1;
+ res[num_res].flags = IORESOURCE_MEM;
+ num_res++;
+
+ glb_irq = ACPI_ADD_PTR(u64, node, smmu->global_interrupt_offset);
+ /* Global IRQs */
+ hw_irq = IORT_IRQ_MASK(glb_irq[0]);
+ trigger = IORT_IRQ_TRIGGER_MASK(glb_irq[0]);
+
+ acpi_iort_register_irq(hw_irq, "arm-smmu-global", trigger,
+ &res[num_res++]);
+
+ /* Global IRQs */
+ hw_irq = IORT_IRQ_MASK(glb_irq[1]);
+ if (hw_irq) {
+ trigger = IORT_IRQ_TRIGGER_MASK(glb_irq[1]);
+ acpi_iort_register_irq(hw_irq, "arm-smmu-global", trigger,
+ &res[num_res++]);
+ }
+
+ /* Context IRQs */
+ ctx_irq = ACPI_ADD_PTR(u64, node, smmu->context_interrupt_offset);
+ for (i = 0; i < smmu->context_interrupt_count; i++) {
+ hw_irq = IORT_IRQ_MASK(ctx_irq[i]);
+ trigger = IORT_IRQ_TRIGGER_MASK(ctx_irq[i]);
+
+ acpi_iort_register_irq(hw_irq, "arm-smmu-context", trigger,
+ &res[num_res++]);
+ }
+}
+
+static bool __init arm_smmu_is_coherent(struct acpi_iort_node *node)
+{
+ struct acpi_iort_smmu *smmu;
+
+ /* Retrieve SMMU specific data */
+ smmu = (struct acpi_iort_smmu *)node->node_data;
+
+ return smmu->flags & ACPI_IORT_SMMU_COHERENT_WALK;
+}
+
struct iort_iommu_config {
const char *name;
int (*iommu_init)(struct acpi_iort_node *node);
@@ -564,12 +636,21 @@ static const struct iort_iommu_config iort_arm_smmu_v3_cfg __initconst = {
.iommu_init_resources = arm_smmu_v3_init_resources
};

+static const struct iort_iommu_config iort_arm_smmu_cfg __initconst = {
+ .name = "arm-smmu",
+ .iommu_is_coherent = arm_smmu_is_coherent,
+ .iommu_count_resources = arm_smmu_count_resources,
+ .iommu_init_resources = arm_smmu_init_resources
+};
+
static __init
const struct iort_iommu_config *iort_get_iommu_cfg(struct acpi_iort_node *node)
{
switch (node->type) {
case ACPI_IORT_NODE_SMMU_V3:
return &iort_arm_smmu_v3_cfg;
+ case ACPI_IORT_NODE_SMMU:
+ return &iort_arm_smmu_cfg;
default:
return NULL;
}
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index e1b6951..2e86cbf 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -28,6 +28,8 @@

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

+#include <linux/acpi.h>
+#include <linux/acpi_iort.h>
#include <linux/atomic.h>
#include <linux/delay.h>
#include <linux/dma-iommu.h>
@@ -1904,6 +1906,70 @@ static const struct of_device_id arm_smmu_of_match[] = {
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);

+#ifdef CONFIG_ACPI
+static int acpi_smmu_get_data(u32 model, u32 *version, u32 *impl)
+{
+ int ret = 0;
+
+ switch (model) {
+ case ACPI_IORT_SMMU_V1:
+ case ACPI_IORT_SMMU_CORELINK_MMU400:
+ *version = ARM_SMMU_V1;
+ *impl = GENERIC_SMMU;
+ break;
+ case ACPI_IORT_SMMU_V2:
+ *version = ARM_SMMU_V2;
+ *impl = GENERIC_SMMU;
+ break;
+ case ACPI_IORT_SMMU_CORELINK_MMU500:
+ *version = ARM_SMMU_V2;
+ *impl = ARM_MMU500;
+ break;
+ default:
+ ret = -ENODEV;
+ }
+
+ return ret;
+}
+
+static int arm_smmu_device_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;
+ u64 *glb_irq;
+ int ret;
+
+ /* Retrieve SMMU1/2 specific data */
+ iort_smmu = (struct acpi_iort_smmu *)node->node_data;
+
+ ret = acpi_smmu_get_data(iort_smmu->model, &smmu->version,
+ &smmu->model);
+ 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;
+
+ if (iort_smmu->flags & ACPI_IORT_SMMU_COHERENT_WALK)
+ smmu->features |= ARM_SMMU_FEAT_COHERENT_WALK;
+
+ return 0;
+}
+#else
+static inline int arm_smmu_device_acpi_probe(struct platform_device *pdev,
+ struct arm_smmu_device *smmu)
+{
+ return -ENODEV;
+}
+#endif
+
static int arm_smmu_device_dt_probe(struct platform_device *pdev,
struct arm_smmu_device *smmu)
{
@@ -1955,7 +2021,11 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
}
smmu->dev = dev;

- err = arm_smmu_device_dt_probe(pdev, smmu);
+ if (dev->of_node)
+ err = arm_smmu_device_dt_probe(pdev, smmu);
+ else
+ err = arm_smmu_device_acpi_probe(pdev, smmu);
+
if (err)
return err;

@@ -2103,6 +2173,17 @@ IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401", arm_smmu_of_init);
IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500", arm_smmu_of_init);
IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2", arm_smmu_of_init);

+#ifdef CONFIG_ACPI
+static int __init arm_smmu_acpi_init(struct acpi_table_header *table)
+{
+ if (iort_node_match(ACPI_IORT_NODE_SMMU))
+ return arm_smmu_init();
+
+ return 0;
+}
+IORT_ACPI_DECLARE(arm_smmu, ACPI_SIG_IORT, arm_smmu_acpi_init);
+#endif
+
MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
MODULE_AUTHOR("Will Deacon <[email protected]>");
MODULE_LICENSE("GPL v2");
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index 17bb078..79ba1bb 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -23,6 +23,9 @@
#include <linux/fwnode.h>
#include <linux/irqdomain.h>

+#define IORT_IRQ_MASK(irq) (irq & 0xffffffffULL)
+#define IORT_IRQ_TRIGGER_MASK(irq) ((irq >> 32) & 0xffffffffULL)
+
int iort_register_domain_token(int trans_id, struct fwnode_handle *fw_node);
void iort_deregister_domain_token(int trans_id);
struct fwnode_handle *iort_find_domain_token(int trans_id);
--
2.10.0

2016-11-09 14:19:52

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v7 16/16] drivers: acpi: iort: introduce iort_iommu_configure

DT based systems have a generic kernel API to configure IOMMUs
for devices (ie of_iommu_configure()).

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

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

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Tested-by: Hanjun Guo <[email protected]>
Tested-by: Tomasz Nowicki <[email protected]>
Cc: Hanjun Guo <[email protected]>
Cc: Tomasz Nowicki <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
---
drivers/acpi/arm64/iort.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++
drivers/acpi/scan.c | 7 +++-
include/linux/acpi_iort.h | 6 +++
3 files changed, 111 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 7d30605..c129c60 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -28,6 +28,8 @@

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

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

+static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
+{
+ u32 *rid = data;
+
+ *rid = alias;
+ return 0;
+}
+
+static int arm_smmu_iort_xlate(struct device *dev, u32 streamid,
+ struct fwnode_handle *fwnode,
+ const struct iommu_ops *ops)
+{
+ int ret = iommu_fwspec_init(dev, fwnode, ops);
+
+ if (!ret)
+ ret = iommu_fwspec_add_ids(dev, &streamid, 1);
+
+ return ret;
+}
+
+static const struct iommu_ops *iort_iommu_xlate(struct device *dev,
+ struct acpi_iort_node *node,
+ u32 streamid)
+{
+ struct fwnode_handle *iort_fwnode = NULL;
+ const struct iommu_ops *ops = NULL;
+ int ret = -ENODEV;
+
+ if (node) {
+ iort_fwnode = iort_get_fwnode(node);
+ if (!iort_fwnode)
+ return NULL;
+
+ ops = fwnode_iommu_get_ops(iort_fwnode);
+ if (!ops)
+ return NULL;
+
+ ret = arm_smmu_iort_xlate(dev, streamid,
+ iort_fwnode, ops);
+ }
+
+ return ret ? NULL : ops;
+}
+
+/**
+ * iort_iommu_configure - Set-up IOMMU configuration for a device.
+ *
+ * @dev: device to configure
+ *
+ * Returns: iommu_ops pointer on configuration success
+ * NULL on configuration failure
+ */
+const struct iommu_ops *iort_iommu_configure(struct device *dev)
+{
+ struct acpi_iort_node *node, *parent;
+ const struct iommu_ops *ops = NULL;
+ u32 streamid = 0;
+
+ if (dev_is_pci(dev)) {
+ struct pci_bus *bus = to_pci_dev(dev)->bus;
+ u32 rid;
+
+ pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
+ &rid);
+
+ node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
+ iort_match_node_callback, &bus->dev);
+ if (!node)
+ return NULL;
+
+ parent = iort_node_map_rid(node, rid, &streamid,
+ IORT_IOMMU_TYPE);
+
+ ops = iort_iommu_xlate(dev, parent, streamid);
+
+ } else {
+ int i = 0;
+
+ node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
+ iort_match_node_callback, dev);
+ if (!node)
+ return NULL;
+
+ parent = iort_node_get_id(node, &streamid,
+ IORT_IOMMU_TYPE, i++);
+
+ while (parent) {
+ ops = iort_iommu_xlate(dev, parent, streamid);
+
+ parent = iort_node_get_id(node, &streamid,
+ IORT_IOMMU_TYPE, i++);
+ }
+ }
+
+ return ops;
+}
+
static void __init acpi_iort_register_irq(int hwirq, const char *name,
int trigger,
struct resource *res)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 694e0b6..e5f7004 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -7,6 +7,7 @@
#include <linux/slab.h>
#include <linux/kernel.h>
#include <linux/acpi.h>
+#include <linux/acpi_iort.h>
#include <linux/signal.h>
#include <linux/kthread.h>
#include <linux/dmi.h>
@@ -1377,6 +1378,8 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
*/
void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
{
+ const struct iommu_ops *iommu;
+
/*
* Set default coherent_dma_mask to 32 bit. Drivers are expected to
* setup the correct supported mask.
@@ -1391,11 +1394,13 @@ void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
if (!dev->dma_mask)
dev->dma_mask = &dev->coherent_dma_mask;

+ iommu = iort_iommu_configure(dev);
+
/*
* Assume dma valid range starts at 0 and covers the whole
* coherent_dma_mask.
*/
- arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, NULL,
+ arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, iommu,
attr == DEV_DMA_COHERENT);
}
EXPORT_SYMBOL_GPL(acpi_dma_configure);
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index 79ba1bb..dcb2b60 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -34,6 +34,8 @@ void acpi_iort_init(void);
bool iort_node_match(u8 type);
u32 iort_msi_map_rid(struct device *dev, u32 req_id);
struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id);
+/* IOMMU interface */
+const struct iommu_ops *iort_iommu_configure(struct device *dev);
#else
static inline void acpi_iort_init(void) { }
static inline bool iort_node_match(u8 type) { return false; }
@@ -42,6 +44,10 @@ static inline u32 iort_msi_map_rid(struct device *dev, u32 req_id)
static inline struct irq_domain *iort_get_device_domain(struct device *dev,
u32 req_id)
{ return NULL; }
+/* IOMMU interface */
+static inline
+const struct iommu_ops *iort_iommu_configure(struct device *dev)
+{ return NULL; }
#endif

#define IORT_ACPI_DECLARE(name, table_id, fn) \
--
2.10.0

2016-11-09 14:20:10

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v7 15/16] drivers: acpi: iort: add single mapping function

The current IORT id mapping API requires components to provide
an input requester ID (a Bus-Device-Function (BDF) identifier for
PCI devices) to translate an input identifier to an output
identifier through an IORT range mapping.

Named components do not have an identifiable source ID therefore
their respective input/output mapping can only be defined in
IORT tables through single mappings, that provide a translation
that does not require any input identifier.

Current IORT interface for requester id mappings (iort_node_map_rid())
is not suitable for components that do not provide a requester id,
so it cannot be used for IORT named components.

Add an interface to the IORT API to enable retrieval of id
by allowing an indexed walk of the single mappings array for
a given component, therefore completing the IORT mapping API.

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

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 62057c6..7d30605 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -318,6 +318,45 @@ static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in,
return 0;
}

+static
+struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
+ u32 *id_out, u8 type_mask,
+ int index)
+{
+ struct acpi_iort_node *parent;
+ struct acpi_iort_id_mapping *map;
+
+ if (!node->mapping_offset || !node->mapping_count ||
+ index >= node->mapping_count)
+ return NULL;
+
+ map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
+ node->mapping_offset);
+
+ /* Firmware bug! */
+ if (!map->output_reference) {
+ pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n",
+ node, node->type);
+ return NULL;
+ }
+
+ parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
+ map->output_reference);
+
+ if (!(IORT_TYPE_MASK(parent->type) & type_mask))
+ return NULL;
+
+ if (map[index].flags & ACPI_IORT_ID_SINGLE_MAPPING) {
+ if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT ||
+ node->type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
+ *id_out = map[index].output_base;
+ return parent;
+ }
+ }
+
+ return NULL;
+}
+
static struct acpi_iort_node *iort_node_map_rid(struct acpi_iort_node *node,
u32 rid_in, u32 *rid_out,
u8 type_mask)
--
2.10.0

2016-11-09 14:20:34

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v7 14/16] drivers: acpi: iort: replace rid map type with type mask

IORT tables provide data that allow the kernel to carry out
device ID mappings between endpoints and system components
(eg interrupt controllers, IOMMUs). When the mapping for a
given device ID is carried out, the translation mechanism
is done on a per-subsystem basis rather than a component
subtype (ie the IOMMU kernel layer will look for mappings
from a device to all IORT node types corresponding to IOMMU
components), therefore the corresponding mapping API should
work on a range (ie mask) of IORT node types corresponding
to a common set of components (eg IOMMUs) rather than a
specific node type.

Upgrade the IORT iort_node_map_rid() API to work with a
type mask instead of a single node type so that it can
be used for mappings that span multiple components types
(ie IOMMUs).

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

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 4708806..62057c6 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -26,6 +26,9 @@
#include <linux/platform_device.h>
#include <linux/slab.h>

+#define IORT_TYPE_MASK(type) (1 << (type))
+#define IORT_MSI_TYPE (1 << ACPI_IORT_NODE_ITS_GROUP)
+
struct iort_its_msi_chip {
struct list_head list;
struct fwnode_handle *fw_node;
@@ -317,7 +320,7 @@ static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in,

static struct acpi_iort_node *iort_node_map_rid(struct acpi_iort_node *node,
u32 rid_in, u32 *rid_out,
- u8 type)
+ u8 type_mask)
{
u32 rid = rid_in;

@@ -326,7 +329,7 @@ static struct acpi_iort_node *iort_node_map_rid(struct acpi_iort_node *node,
struct acpi_iort_id_mapping *map;
int i;

- if (node->type == type) {
+ if (IORT_TYPE_MASK(node->type) & type_mask) {
if (rid_out)
*rid_out = rid;
return node;
@@ -399,7 +402,7 @@ u32 iort_msi_map_rid(struct device *dev, u32 req_id)
if (!node)
return req_id;

- iort_node_map_rid(node, req_id, &dev_id, ACPI_IORT_NODE_ITS_GROUP);
+ iort_node_map_rid(node, req_id, &dev_id, IORT_MSI_TYPE);
return dev_id;
}

@@ -421,7 +424,7 @@ static int iort_dev_find_its_id(struct device *dev, u32 req_id,
if (!node)
return -ENXIO;

- node = iort_node_map_rid(node, req_id, NULL, ACPI_IORT_NODE_ITS_GROUP);
+ node = iort_node_map_rid(node, req_id, NULL, IORT_MSI_TYPE);
if (!node)
return -ENXIO;

--
2.10.0

2016-11-09 14:21:00

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v7 12/16] 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]>
Tested-by: Hanjun Guo <[email protected]>
Tested-by: Tomasz Nowicki <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Hanjun Guo <[email protected]>
Cc: Robin Murphy <[email protected]>
---
drivers/iommu/arm-smmu.c | 62 +++++++++++++++++++++++++++++-------------------
1 file changed, 37 insertions(+), 25 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 8a3c2a4..e1b6951 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1668,7 +1668,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;
int i;

dev_notice(smmu->dev, "probing hardware configuration...\n");
@@ -1713,20 +1713,17 @@ 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");

/* Max. number of entries we have for stream matching/indexing */
size = 1 << ((id >> ID0_NUMSIDB_SHIFT) & ID0_NUMSIDB_MASK);
@@ -1907,15 +1904,25 @@ static const struct of_device_id arm_smmu_of_match[] = {
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);

-static int arm_smmu_device_dt_probe(struct platform_device *pdev)
+static int arm_smmu_device_dt_probe(struct platform_device *pdev,
+ struct arm_smmu_device *smmu)
{
const struct arm_smmu_match_data *data;
- struct resource *res;
- struct arm_smmu_device *smmu;
struct device *dev = &pdev->dev;
- int num_irqs, i, err;
bool legacy_binding;

+ if (of_property_read_u32(dev->of_node, "#global-interrupts",
+ &smmu->num_global_irqs)) {
+ dev_err(dev, "missing #global-interrupts property\n");
+ return -ENODEV;
+ }
+
+ data = of_device_get_match_data(dev);
+ smmu->version = data->version;
+ smmu->model = data->model;
+
+ parse_driver_options(smmu);
+
legacy_binding = of_find_property(dev->of_node, "mmu-masters", NULL);
if (legacy_binding && !using_generic_binding) {
if (!using_legacy_binding)
@@ -1928,6 +1935,19 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
return -ENODEV;
}

+ if (of_dma_is_coherent(smmu->dev->of_node))
+ smmu->features |= ARM_SMMU_FEAT_COHERENT_WALK;
+
+ 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 num_irqs, i, err;
+
smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
if (!smmu) {
dev_err(dev, "failed to allocate arm_smmu_device\n");
@@ -1935,9 +1955,9 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
}
smmu->dev = dev;

- data = of_device_get_match_data(dev);
- smmu->version = data->version;
- smmu->model = data->model;
+ err = arm_smmu_device_dt_probe(pdev, smmu);
+ if (err)
+ return err;

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
smmu->base = devm_ioremap_resource(dev, res);
@@ -1945,12 +1965,6 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
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");
- return -ENODEV;
- }
-
num_irqs = 0;
while ((res = platform_get_resource(pdev, IORESOURCE_IRQ, num_irqs))) {
num_irqs++;
@@ -1985,8 +1999,6 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
if (err)
return err;

- parse_driver_options(smmu);
-
if (smmu->version == ARM_SMMU_V2 &&
smmu->num_context_banks != smmu->num_context_irqs) {
dev_err(dev,
@@ -2048,7 +2060,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.10.0

2016-11-09 14:21:16

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v7 10/16] drivers: iommu: arm-smmu-v3: split probe functions into DT/generic portions

Current ARM SMMUv3 probe functions intermingle HW and DT probing in the
initialization functions to detect and programme the ARM SMMU v3 driver
features. In order to allow probing the ARM SMMUv3 with other firmwares
than DT, this patch splits the ARM SMMUv3 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]>
Acked-by: Will Deacon <[email protected]>
Tested-by: Hanjun Guo <[email protected]>
Tested-by: Tomasz Nowicki <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Hanjun Guo <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Joerg Roedel <[email protected]>
---
drivers/iommu/arm-smmu-v3.c | 46 +++++++++++++++++++++++++++++----------------
1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index fef9f60..f970acd 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2381,10 +2381,10 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)
return 0;
}

-static int arm_smmu_device_probe(struct arm_smmu_device *smmu)
+static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
{
u32 reg;
- bool coherent;
+ bool coherent = smmu->features & ARM_SMMU_FEAT_COHERENCY;

/* IDR0 */
reg = readl_relaxed(smmu->base + ARM_SMMU_IDR0);
@@ -2436,13 +2436,9 @@ static int arm_smmu_device_probe(struct arm_smmu_device *smmu)
smmu->features |= ARM_SMMU_FEAT_HYP;

/*
- * The dma-coherent property is used in preference to the ID
+ * The coherency feature as set by FW is used in preference to the ID
* register, but warn on mismatch.
*/
- coherent = of_dma_is_coherent(smmu->dev->of_node);
- if (coherent)
- smmu->features |= ARM_SMMU_FEAT_COHERENCY;
-
if (!!(reg & IDR0_COHACC) != coherent)
dev_warn(smmu->dev, "IDR0.COHACC overridden by dma-coherent property (%s)\n",
coherent ? "true" : "false");
@@ -2563,21 +2559,37 @@ static int arm_smmu_device_probe(struct arm_smmu_device *smmu)
return 0;
}

-static int arm_smmu_device_dt_probe(struct platform_device *pdev)
+static int arm_smmu_device_dt_probe(struct platform_device *pdev,
+ struct arm_smmu_device *smmu,
+ bool *bypass)
{
- int irq, ret;
- struct resource *res;
- struct arm_smmu_device *smmu;
struct device *dev = &pdev->dev;
- bool bypass = true;
u32 cells;

+ *bypass = true;
+
if (of_property_read_u32(dev->of_node, "#iommu-cells", &cells))
dev_err(dev, "missing #iommu-cells property\n");
else if (cells != 1)
dev_err(dev, "invalid #iommu-cells value (%d)\n", cells);
else
- bypass = false;
+ *bypass = false;
+
+ parse_driver_options(smmu);
+
+ if (of_dma_is_coherent(smmu->dev->of_node))
+ smmu->features |= ARM_SMMU_FEAT_COHERENCY;
+
+ return 0;
+}
+
+static int arm_smmu_device_probe(struct platform_device *pdev)
+{
+ int irq, ret;
+ struct resource *res;
+ struct arm_smmu_device *smmu;
+ struct device *dev = &pdev->dev;
+ bool bypass;

smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
if (!smmu) {
@@ -2614,10 +2626,12 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
if (irq > 0)
smmu->gerr_irq = irq;

- parse_driver_options(smmu);
+ ret = arm_smmu_device_dt_probe(pdev, smmu, &bypass);
+ if (ret)
+ return ret;

/* Probe the h/w */
- ret = arm_smmu_device_probe(smmu);
+ ret = arm_smmu_device_hw_probe(smmu);
if (ret)
return ret;

@@ -2679,7 +2693,7 @@ static struct platform_driver arm_smmu_driver = {
.name = "arm-smmu-v3",
.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.10.0

2016-11-09 14:21:33

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v7 09/16] drivers: acpi: iort: add support for ARM SMMU platform devices creation

In ARM ACPI systems, IOMMU components are specified through static
IORT table entries. In order to create platform devices for the
corresponding ARM SMMU components, IORT kernel code should be made
able to parse IORT table entries and create platform devices
dynamically.

This patch adds the generic IORT infrastructure required to create
platform devices for ARM SMMUs.

ARM SMMU versions have different resources requirement therefore this
patch also introduces an IORT specific structure (ie iort_iommu_config)
that contains hooks (to be defined when the corresponding ARM SMMU
driver support is added to the kernel) to be used to define the
platform devices names, init the IOMMUs, count their resources and
finally initialize them.

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

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 4bb6acb..ddf83b5 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -19,9 +19,11 @@
#define pr_fmt(fmt) "ACPI: IORT: " fmt

#include <linux/acpi_iort.h>
+#include <linux/iommu.h>
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/pci.h>
+#include <linux/platform_device.h>
#include <linux/slab.h>

struct iort_its_msi_chip {
@@ -457,6 +459,153 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id)
return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
}

+struct iort_iommu_config {
+ const char *name;
+ int (*iommu_init)(struct acpi_iort_node *node);
+ bool (*iommu_is_coherent)(struct acpi_iort_node *node);
+ int (*iommu_count_resources)(struct acpi_iort_node *node);
+ void (*iommu_init_resources)(struct resource *res,
+ struct acpi_iort_node *node);
+};
+
+static __init
+const struct iort_iommu_config *iort_get_iommu_cfg(struct acpi_iort_node *node)
+{
+ return NULL;
+}
+
+/**
+ * iort_add_smmu_platform_device() - Allocate a platform device for SMMU
+ * @node: Pointer to SMMU ACPI IORT node
+ *
+ * Returns: 0 on success, <0 failure
+ */
+static int __init iort_add_smmu_platform_device(struct acpi_iort_node *node)
+{
+ struct fwnode_handle *fwnode;
+ struct platform_device *pdev;
+ struct resource *r;
+ enum dev_dma_attr attr;
+ int ret, count;
+ const struct iort_iommu_config *ops = iort_get_iommu_cfg(node);
+
+ if (!ops)
+ return -ENODEV;
+
+ pdev = platform_device_alloc(ops->name, PLATFORM_DEVID_AUTO);
+ if (!pdev)
+ return PTR_ERR(pdev);
+
+ count = ops->iommu_count_resources(node);
+
+ r = kcalloc(count, sizeof(*r), GFP_KERNEL);
+ if (!r) {
+ ret = -ENOMEM;
+ goto dev_put;
+ }
+
+ ops->iommu_init_resources(r, node);
+
+ ret = platform_device_add_resources(pdev, r, count);
+ /*
+ * Resources are duplicated in platform_device_add_resources,
+ * free their allocated memory
+ */
+ kfree(r);
+
+ if (ret)
+ goto dev_put;
+
+ /*
+ * Add a copy of IORT node pointer to platform_data to
+ * be used to retrieve IORT data information.
+ */
+ ret = platform_device_add_data(pdev, &node, sizeof(node));
+ if (ret)
+ goto dev_put;
+
+ /*
+ * We expect the dma masks to be equivalent for
+ * all SMMUs set-ups
+ */
+ pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
+
+ fwnode = iort_get_fwnode(node);
+
+ if (!fwnode) {
+ ret = -ENODEV;
+ goto dev_put;
+ }
+
+ pdev->dev.fwnode = fwnode;
+
+ attr = ops->iommu_is_coherent(node) ?
+ 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);
+dev_put:
+ platform_device_put(pdev);
+
+ return ret;
+}
+
+static void __init iort_init_platform_devices(void)
+{
+ struct acpi_iort_node *iort_node, *iort_end;
+ struct acpi_table_iort *iort;
+ struct fwnode_handle *fwnode;
+ int i, ret;
+
+ /*
+ * iort_table and iort both point to the start of IORT table, but
+ * have different struct types
+ */
+ iort = (struct acpi_table_iort *)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,
+ 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;
+ }
+
+ if ((iort_node->type == ACPI_IORT_NODE_SMMU) ||
+ (iort_node->type == ACPI_IORT_NODE_SMMU_V3)) {
+
+ fwnode = acpi_alloc_fwnode_static();
+ if (!fwnode)
+ return;
+
+ iort_set_fwnode(iort_node, fwnode);
+
+ ret = iort_add_smmu_platform_device(iort_node);
+ if (ret) {
+ iort_delete_fwnode(iort_node);
+ acpi_free_fwnode_static(fwnode);
+ return;
+ }
+ }
+
+ iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
+ iort_node->length);
+ }
+}
+
void __init acpi_iort_init(void)
{
acpi_status status;
@@ -472,5 +621,7 @@ void __init acpi_iort_init(void)
return;
}

+ iort_init_platform_devices();
+
acpi_probe_device_table(iort);
}
--
2.10.0

2016-11-09 14:22:07

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v7 07/16] 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 for now are just wrappers around arch_setup_dma_ops() and
arch_teardown_dma_ops() and also updates ACPI and PCI core code to use
the newly introduced acpi_dma_configure/acpi_dma_deconfigure functions.

Since acpi_dma_configure() is used to configure DMA operations, the
function initializes the dma/coherent_dma masks to sane default values
if the current masks are uninitialized (also to keep the default values
consistent with DT systems) to make sure the device has a complete
default DMA set-up.

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.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]> [pci]
Tested-by: Hanjun Guo <[email protected]>
Tested-by: Tomasz Nowicki <[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/scan.c | 40 ++++++++++++++++++++++++++++++++++++++++
drivers/pci/probe.c | 3 +--
include/acpi/acpi_bus.h | 2 ++
include/linux/acpi.h | 5 +++++
5 files changed, 50 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/scan.c b/drivers/acpi/scan.c
index 035ac64..694e0b6 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1370,6 +1370,46 @@ 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)
+{
+ /*
+ * Set default coherent_dma_mask to 32 bit. Drivers are expected to
+ * setup the correct supported mask.
+ */
+ if (!dev->coherent_dma_mask)
+ dev->coherent_dma_mask = DMA_BIT_MASK(32);
+
+ /*
+ * Set it to coherent_dma_mask by default if the architecture
+ * code has not set it.
+ */
+ if (!dev->dma_mask)
+ dev->dma_mask = &dev->coherent_dma_mask;
+
+ /*
+ * Assume dma valid range starts at 0 and covers the whole
+ * coherent_dma_mask.
+ */
+ arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, NULL,
+ attr == DEV_DMA_COHERENT);
+}
+EXPORT_SYMBOL_GPL(acpi_dma_configure);
+
+/**
+ * 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);
+}
+EXPORT_SYMBOL_GPL(acpi_dma_deconfigure);
+
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 ab00267..c29e07a 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1738,8 +1738,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 c1a524d..4242c31 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -573,6 +573,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 6efb13c..df961f4 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -764,6 +764,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)

static inline void acpi_device_set_enumerated(struct acpi_device *adev)
--
2.10.0

2016-11-09 14:22:25

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v7 06/16] drivers: iommu: arm-smmu-v3: convert struct device of_node to fwnode usage

Current ARM SMMU v3 driver rely on the struct device.of_node pointer for
device look-up and iommu_ops retrieval.

In preparation for ACPI probing enablement, convert the driver to use
the struct device.fwnode member for device and iommu_ops look-up so that
the driver infrastructure can be used also on systems that do not
associate an of_node pointer to a struct device (eg ACPI), making the
device look-up and iommu_ops retrieval firmware agnostic.

Signed-off-by: Lorenzo Pieralisi <[email protected]>
Tested-by: Hanjun Guo <[email protected]>
Tested-by: Tomasz Nowicki <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Hanjun Guo <[email protected]>
Cc: Robin Murphy <[email protected]>
---
drivers/iommu/arm-smmu-v3.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index e6f9b2d..fef9f60 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1723,13 +1723,14 @@ static struct platform_driver arm_smmu_driver;

static int arm_smmu_match_node(struct device *dev, void *data)
{
- return dev->of_node == data;
+ return dev->fwnode == data;
}

-static struct arm_smmu_device *arm_smmu_get_by_node(struct device_node *np)
+static
+struct arm_smmu_device *arm_smmu_get_by_fwnode(struct fwnode_handle *fwnode)
{
struct device *dev = driver_find_device(&arm_smmu_driver.driver, NULL,
- np, arm_smmu_match_node);
+ fwnode, arm_smmu_match_node);
put_device(dev);
return dev ? dev_get_drvdata(dev) : NULL;
}
@@ -1765,7 +1766,7 @@ static int arm_smmu_add_device(struct device *dev)
master = fwspec->iommu_priv;
smmu = master->smmu;
} else {
- smmu = arm_smmu_get_by_node(to_of_node(fwspec->iommu_fwnode));
+ smmu = arm_smmu_get_by_fwnode(fwspec->iommu_fwnode);
if (!smmu)
return -ENODEV;
master = kzalloc(sizeof(*master), GFP_KERNEL);
@@ -2634,7 +2635,8 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
return ret;

/* And we're up. Go go go! */
- of_iommu_set_ops(dev->of_node, &arm_smmu_ops);
+ fwnode_iommu_set_ops(dev->fwnode, &arm_smmu_ops);
+
#ifdef CONFIG_PCI
if (pci_bus_type.iommu_ops != &arm_smmu_ops) {
pci_request_acs();
--
2.10.0

2016-11-09 14:23:01

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v7 05/16] drivers: iommu: arm-smmu: convert struct device of_node to fwnode usage

Current ARM SMMU driver rely on the struct device.of_node pointer for
device look-up and iommu_ops retrieval.

In preparation for ACPI probing enablement, convert the driver to use
the struct device.fwnode member for device and iommu_ops look-up so that
the driver infrastructure can be used also on systems that do not
associate an of_node pointer to a struct device (eg ACPI), making the
device look-up and iommu_ops retrieval firmware agnostic.

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

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 8f72814..8a3c2a4 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1379,13 +1379,14 @@ static bool arm_smmu_capable(enum iommu_cap cap)

static int arm_smmu_match_node(struct device *dev, void *data)
{
- return dev->of_node == data;
+ return dev->fwnode == data;
}

-static struct arm_smmu_device *arm_smmu_get_by_node(struct device_node *np)
+static
+struct arm_smmu_device *arm_smmu_get_by_fwnode(struct fwnode_handle *fwnode)
{
struct device *dev = driver_find_device(&arm_smmu_driver.driver, NULL,
- np, arm_smmu_match_node);
+ fwnode, arm_smmu_match_node);
put_device(dev);
return dev ? dev_get_drvdata(dev) : NULL;
}
@@ -1403,7 +1404,7 @@ static int arm_smmu_add_device(struct device *dev)
if (ret)
goto out_free;
} else if (fwspec && fwspec->ops == &arm_smmu_ops) {
- smmu = arm_smmu_get_by_node(to_of_node(fwspec->iommu_fwnode));
+ smmu = arm_smmu_get_by_fwnode(fwspec->iommu_fwnode);
} else {
return -ENODEV;
}
@@ -2007,7 +2008,7 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
}
}

- of_iommu_set_ops(dev->of_node, &arm_smmu_ops);
+ fwnode_iommu_set_ops(dev->fwnode, &arm_smmu_ops);
platform_set_drvdata(pdev, smmu);
arm_smmu_device_reset(smmu);

--
2.10.0

2016-11-09 14:23:32

by Lorenzo Pieralisi

[permalink] [raw]
Subject: [PATCH v7 03/16] drivers: acpi: iort: add support for IOMMU fwnode 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 and their respective fwnode.

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

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 2c46ebc..1ac2720 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -20,7 +20,9 @@

#include <linux/acpi_iort.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;
@@ -28,6 +30,90 @@ struct iort_its_msi_chip {
u32 translation_id;
};

+struct iort_fwnode {
+ struct list_head list;
+ struct acpi_iort_node *iort_node;
+ struct fwnode_handle *fwnode;
+};
+static LIST_HEAD(iort_fwnode_list);
+static DEFINE_SPINLOCK(iort_fwnode_lock);
+
+/**
+ * iort_set_fwnode() - Create iort_fwnode and use it to register
+ * iommu data in the iort_fwnode_list
+ *
+ * @node: IORT table node associated with the IOMMU
+ * @fwnode: fwnode associated with the IORT node
+ *
+ * Returns: 0 on success
+ * <0 on failure
+ */
+static inline int iort_set_fwnode(struct acpi_iort_node *iort_node,
+ struct fwnode_handle *fwnode)
+{
+ struct iort_fwnode *np;
+
+ np = kzalloc(sizeof(struct iort_fwnode), GFP_ATOMIC);
+
+ if (WARN_ON(!np))
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(&np->list);
+ np->iort_node = iort_node;
+ np->fwnode = fwnode;
+
+ spin_lock(&iort_fwnode_lock);
+ list_add_tail(&np->list, &iort_fwnode_list);
+ spin_unlock(&iort_fwnode_lock);
+
+ return 0;
+}
+
+/**
+ * iort_get_fwnode() - Retrieve fwnode associated with an IORT node
+ *
+ * @node: IORT table node to be looked-up
+ *
+ * Returns: fwnode_handle pointer on success, NULL on failure
+ */
+static inline
+struct fwnode_handle *iort_get_fwnode(struct acpi_iort_node *node)
+{
+ struct iort_fwnode *curr;
+ struct fwnode_handle *fwnode = NULL;
+
+ spin_lock(&iort_fwnode_lock);
+ list_for_each_entry(curr, &iort_fwnode_list, list) {
+ if (curr->iort_node == node) {
+ fwnode = curr->fwnode;
+ break;
+ }
+ }
+ spin_unlock(&iort_fwnode_lock);
+
+ return fwnode;
+}
+
+/**
+ * iort_delete_fwnode() - Delete fwnode associated with an IORT node
+ *
+ * @node: IORT table node associated with fwnode to delete
+ */
+static inline void iort_delete_fwnode(struct acpi_iort_node *node)
+{
+ struct iort_fwnode *curr, *tmp;
+
+ spin_lock(&iort_fwnode_lock);
+ list_for_each_entry_safe(curr, tmp, &iort_fwnode_list, list) {
+ if (curr->iort_node == node) {
+ list_del(&curr->list);
+ kfree(curr);
+ break;
+ }
+ }
+ spin_unlock(&iort_fwnode_lock);
+}
+
typedef acpi_status (*iort_find_node_callback)
(struct acpi_iort_node *node, void *context);

--
2.10.0

2016-11-09 14:40:16

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic

On 09/11/16 14:19, Lorenzo Pieralisi wrote:
> The of_iommu_{set/get}_ops() API is used to associate a device
> tree node with a specific set of IOMMU operations. The same
> kernel interface is required on systems booting with ACPI, where
> devices are not associated with a device tree node, therefore
> the interface requires generalization.
>
> The struct device fwnode member represents the fwnode token
> associated with the device and the struct it points at is firmware
> specific; regardless, it is initialized on both ACPI and DT systems
> and makes an ideal candidate to use it to associate a set of IOMMU
> operations to a given device, through its struct device.fwnode member
> pointer.
>
> Convert the DT specific of_iommu_{set/get}_ops() interface to
> use struct device.fwnode as a look-up token, making the interface
> usable on ACPI systems.
>
> Signed-off-by: Lorenzo Pieralisi <[email protected]>
> Tested-by: Hanjun Guo <[email protected]>
> Tested-by: Tomasz Nowicki <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Hanjun Guo <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> ---
> drivers/iommu/iommu.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> drivers/iommu/of_iommu.c | 39 ---------------------------------------
> include/linux/iommu.h | 14 ++++++++++++++
> include/linux/of_iommu.h | 12 ++++++++++--
> 4 files changed, 66 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 9a2f196..5c97c01 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1615,6 +1615,48 @@ int iommu_request_dm_for_dev(struct device *dev)
> return ret;
> }
>
> +struct iommu_fwentry {

:)

> + struct list_head list;
> + struct fwnode_handle *fwnode;
> + const struct iommu_ops *ops;
> +};
> +static LIST_HEAD(iommu_fwentry_list);
> +static DEFINE_SPINLOCK(iommu_fwentry_lock);
> +
> +void fwnode_iommu_set_ops(struct fwnode_handle *fwnode,
> + const struct iommu_ops *ops)
> +{
> + struct iommu_fwentry *iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
> +
> + if (WARN_ON(!iommu))
> + return;
> +
> + if (is_of_node(fwnode))

Nit: this check is actually redundant, since to_of_node() already does
the right thing and of_node_get() is NULL-safe - iommu_fwspec_init()
already works that way. On the other hand, though, it is perhaps more
intuitive to see it explicitly, and since to_of_node() is inline it
ought to result in the same object code (I've not checked, though).
Either way:

Reviewed-by: Robin Murphy <[email protected]>

> + of_node_get(to_of_node(fwnode));
> +
> + INIT_LIST_HEAD(&iommu->list);
> + iommu->fwnode = fwnode;
> + iommu->ops = ops;
> + spin_lock(&iommu_fwentry_lock);
> + list_add_tail(&iommu->list, &iommu_fwentry_list);
> + spin_unlock(&iommu_fwentry_lock);
> +}
> +
> +const struct iommu_ops *fwnode_iommu_get_ops(struct fwnode_handle *fwnode)
> +{
> + struct iommu_fwentry *node;
> + const struct iommu_ops *ops = NULL;
> +
> + spin_lock(&iommu_fwentry_lock);
> + list_for_each_entry(node, &iommu_fwentry_list, list)
> + if (node->fwnode == fwnode) {
> + ops = node->ops;
> + break;
> + }
> + spin_unlock(&iommu_fwentry_lock);
> + return ops;
> +}
> +
> int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
> const struct iommu_ops *ops)
> {
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 5b82862..0f57ddc 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -96,45 +96,6 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
> }
> EXPORT_SYMBOL_GPL(of_get_dma_window);
>
> -struct of_iommu_node {
> - struct list_head list;
> - struct device_node *np;
> - const struct iommu_ops *ops;
> -};
> -static LIST_HEAD(of_iommu_list);
> -static DEFINE_SPINLOCK(of_iommu_lock);
> -
> -void of_iommu_set_ops(struct device_node *np, const struct iommu_ops *ops)
> -{
> - struct of_iommu_node *iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
> -
> - if (WARN_ON(!iommu))
> - return;
> -
> - of_node_get(np);
> - INIT_LIST_HEAD(&iommu->list);
> - iommu->np = np;
> - iommu->ops = ops;
> - spin_lock(&of_iommu_lock);
> - list_add_tail(&iommu->list, &of_iommu_list);
> - spin_unlock(&of_iommu_lock);
> -}
> -
> -const struct iommu_ops *of_iommu_get_ops(struct device_node *np)
> -{
> - struct of_iommu_node *node;
> - const struct iommu_ops *ops = NULL;
> -
> - spin_lock(&of_iommu_lock);
> - list_for_each_entry(node, &of_iommu_list, list)
> - if (node->np == np) {
> - ops = node->ops;
> - break;
> - }
> - spin_unlock(&of_iommu_lock);
> - return ops;
> -}
> -
> static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
> {
> struct of_phandle_args *iommu_spec = data;
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 436dc21..15d5478 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -351,6 +351,9 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
> const struct iommu_ops *ops);
> void iommu_fwspec_free(struct device *dev);
> int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids);
> +void fwnode_iommu_set_ops(struct fwnode_handle *fwnode,
> + const struct iommu_ops *ops);
> +const struct iommu_ops *fwnode_iommu_get_ops(struct fwnode_handle *fwnode);
>
> #else /* CONFIG_IOMMU_API */
>
> @@ -580,6 +583,17 @@ static inline int iommu_fwspec_add_ids(struct device *dev, u32 *ids,
> return -ENODEV;
> }
>
> +static inline void fwnode_iommu_set_ops(struct fwnode_handle *fwnode,
> + const struct iommu_ops *ops)
> +{
> +}
> +
> +static inline
> +const struct iommu_ops *fwnode_iommu_get_ops(struct fwnode_handle *fwnode)
> +{
> + return NULL;
> +}
> +
> #endif /* CONFIG_IOMMU_API */
>
> #endif /* __LINUX_IOMMU_H */
> diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
> index e80b9c7..7681007 100644
> --- a/include/linux/of_iommu.h
> +++ b/include/linux/of_iommu.h
> @@ -31,8 +31,16 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
>
> #endif /* CONFIG_OF_IOMMU */
>
> -void of_iommu_set_ops(struct device_node *np, const struct iommu_ops *ops);
> -const struct iommu_ops *of_iommu_get_ops(struct device_node *np);
> +static inline void of_iommu_set_ops(struct device_node *np,
> + const struct iommu_ops *ops)
> +{
> + fwnode_iommu_set_ops(&np->fwnode, ops);
> +}
> +
> +static inline const struct iommu_ops *of_iommu_get_ops(struct device_node *np)
> +{
> + return fwnode_iommu_get_ops(&np->fwnode);
> +}
>
> extern struct of_device_id __iommu_of_table;
>
>

2016-11-09 14:43:25

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH v7 05/16] drivers: iommu: arm-smmu: convert struct device of_node to fwnode usage

On 09/11/16 14:19, Lorenzo Pieralisi wrote:
> Current ARM SMMU driver rely on the struct device.of_node pointer for
> device look-up and iommu_ops retrieval.
>
> In preparation for ACPI probing enablement, convert the driver to use
> the struct device.fwnode member for device and iommu_ops look-up so that
> the driver infrastructure can be used also on systems that do not
> associate an of_node pointer to a struct device (eg ACPI), making the
> device look-up and iommu_ops retrieval firmware agnostic.
>
> Signed-off-by: Lorenzo Pieralisi <[email protected]>
> Tested-by: Hanjun Guo <[email protected]>
> Tested-by: Tomasz Nowicki <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Hanjun Guo <[email protected]>
> Cc: Robin Murphy <[email protected]>

Reviewed-by: Robin Murphy <[email protected]>

> ---
> drivers/iommu/arm-smmu.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 8f72814..8a3c2a4 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -1379,13 +1379,14 @@ static bool arm_smmu_capable(enum iommu_cap cap)
>
> static int arm_smmu_match_node(struct device *dev, void *data)
> {
> - return dev->of_node == data;
> + return dev->fwnode == data;
> }
>
> -static struct arm_smmu_device *arm_smmu_get_by_node(struct device_node *np)
> +static
> +struct arm_smmu_device *arm_smmu_get_by_fwnode(struct fwnode_handle *fwnode)
> {
> struct device *dev = driver_find_device(&arm_smmu_driver.driver, NULL,
> - np, arm_smmu_match_node);
> + fwnode, arm_smmu_match_node);
> put_device(dev);
> return dev ? dev_get_drvdata(dev) : NULL;
> }
> @@ -1403,7 +1404,7 @@ static int arm_smmu_add_device(struct device *dev)
> if (ret)
> goto out_free;
> } else if (fwspec && fwspec->ops == &arm_smmu_ops) {
> - smmu = arm_smmu_get_by_node(to_of_node(fwspec->iommu_fwnode));
> + smmu = arm_smmu_get_by_fwnode(fwspec->iommu_fwnode);
> } else {
> return -ENODEV;
> }
> @@ -2007,7 +2008,7 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
> }
> }
>
> - of_iommu_set_ops(dev->of_node, &arm_smmu_ops);
> + fwnode_iommu_set_ops(dev->fwnode, &arm_smmu_ops);
> platform_set_drvdata(pdev, smmu);
> arm_smmu_device_reset(smmu);
>
>

2016-11-09 14:44:15

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH v7 06/16] drivers: iommu: arm-smmu-v3: convert struct device of_node to fwnode usage

On 09/11/16 14:19, Lorenzo Pieralisi wrote:
> Current ARM SMMU v3 driver rely on the struct device.of_node pointer for
> device look-up and iommu_ops retrieval.
>
> In preparation for ACPI probing enablement, convert the driver to use
> the struct device.fwnode member for device and iommu_ops look-up so that
> the driver infrastructure can be used also on systems that do not
> associate an of_node pointer to a struct device (eg ACPI), making the
> device look-up and iommu_ops retrieval firmware agnostic.
>
> Signed-off-by: Lorenzo Pieralisi <[email protected]>
> Tested-by: Hanjun Guo <[email protected]>
> Tested-by: Tomasz Nowicki <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Hanjun Guo <[email protected]>
> Cc: Robin Murphy <[email protected]>

Reviewed-by: Robin Murphy <[email protected]>

> ---
> drivers/iommu/arm-smmu-v3.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index e6f9b2d..fef9f60 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -1723,13 +1723,14 @@ static struct platform_driver arm_smmu_driver;
>
> static int arm_smmu_match_node(struct device *dev, void *data)
> {
> - return dev->of_node == data;
> + return dev->fwnode == data;
> }
>
> -static struct arm_smmu_device *arm_smmu_get_by_node(struct device_node *np)
> +static
> +struct arm_smmu_device *arm_smmu_get_by_fwnode(struct fwnode_handle *fwnode)
> {
> struct device *dev = driver_find_device(&arm_smmu_driver.driver, NULL,
> - np, arm_smmu_match_node);
> + fwnode, arm_smmu_match_node);
> put_device(dev);
> return dev ? dev_get_drvdata(dev) : NULL;
> }
> @@ -1765,7 +1766,7 @@ static int arm_smmu_add_device(struct device *dev)
> master = fwspec->iommu_priv;
> smmu = master->smmu;
> } else {
> - smmu = arm_smmu_get_by_node(to_of_node(fwspec->iommu_fwnode));
> + smmu = arm_smmu_get_by_fwnode(fwspec->iommu_fwnode);
> if (!smmu)
> return -ENODEV;
> master = kzalloc(sizeof(*master), GFP_KERNEL);
> @@ -2634,7 +2635,8 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev)
> return ret;
>
> /* And we're up. Go go go! */
> - of_iommu_set_ops(dev->of_node, &arm_smmu_ops);
> + fwnode_iommu_set_ops(dev->fwnode, &arm_smmu_ops);
> +
> #ifdef CONFIG_PCI
> if (pci_bus_type.iommu_ops != &arm_smmu_ops) {
> pci_request_acs();
>

2016-11-09 15:41:43

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH v7 07/16] drivers: acpi: implement acpi_dma_configure

On 09/11/16 14:19, 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 for now are just wrappers around arch_setup_dma_ops() and
> arch_teardown_dma_ops() and also updates ACPI and PCI core code to use
> the newly introduced acpi_dma_configure/acpi_dma_deconfigure functions.
>
> Since acpi_dma_configure() is used to configure DMA operations, the
> function initializes the dma/coherent_dma masks to sane default values
> if the current masks are uninitialized (also to keep the default values
> consistent with DT systems) to make sure the device has a complete
> default DMA set-up.
>
> 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.
>
> Signed-off-by: Lorenzo Pieralisi <[email protected]>
> Acked-by: Bjorn Helgaas <[email protected]> [pci]
> Tested-by: Hanjun Guo <[email protected]>
> Tested-by: Tomasz Nowicki <[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/scan.c | 40 ++++++++++++++++++++++++++++++++++++++++
> drivers/pci/probe.c | 3 +--
> include/acpi/acpi_bus.h | 2 ++
> include/linux/acpi.h | 5 +++++
> 5 files changed, 50 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/scan.c b/drivers/acpi/scan.c
> index 035ac64..694e0b6 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1370,6 +1370,46 @@ 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)
> +{
> + /*
> + * Set default coherent_dma_mask to 32 bit. Drivers are expected to
> + * setup the correct supported mask.
> + */
> + if (!dev->coherent_dma_mask)
> + dev->coherent_dma_mask = DMA_BIT_MASK(32);
> +
> + /*
> + * Set it to coherent_dma_mask by default if the architecture
> + * code has not set it.
> + */
> + if (!dev->dma_mask)
> + dev->dma_mask = &dev->coherent_dma_mask;
> +
> + /*
> + * Assume dma valid range starts at 0 and covers the whole
> + * coherent_dma_mask.
> + */
> + arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, NULL,
> + attr == DEV_DMA_COHERENT);
> +}
> +EXPORT_SYMBOL_GPL(acpi_dma_configure);
> +
> +/**
> + * 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);
> +}
> +EXPORT_SYMBOL_GPL(acpi_dma_deconfigure);
> +
> 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 ab00267..c29e07a 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1738,8 +1738,7 @@ static void pci_dma_configure(struct pci_dev *dev)
> if (attr == DEV_DMA_NOT_SUPPORTED)
> dev_warn(&dev->dev, "DMA not supported.\n");

If we're going to check attr for DMA support before each call like this,
it seems like we may as well put the check at the top of
acpi_dma_configure() instead. I think it should be viable to predicate
the warning on dev_is_pci() at this point, if that's the appropriate
behaviour.

Still, we're likely to be touching this again soon with the
IOMMU-probe-ordering work, so there's plenty of room for further
cleanups then:

Reviewed-by: Robin Murphy <[email protected]>

> 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 c1a524d..4242c31 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -573,6 +573,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 6efb13c..df961f4 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -764,6 +764,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)
>
> static inline void acpi_device_set_enumerated(struct acpi_device *adev)
>

2016-11-09 23:36:16

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v7 00/16] ACPI IORT ARM SMMU support

Hi Lorenzo,

On Wed, Nov 9, 2016 at 3:19 PM, Lorenzo Pieralisi
<[email protected]> wrote:
> This patch series is v7 of a previous posting:
>
> https://lkml.org/lkml/2016/10/18/506

I don't see anything objectionable in this series.

Please let me know which patches in particular to look at in detail.

Thanks,
Rafael

2016-11-09 23:37:09

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v7 01/16] drivers: acpi: add FWNODE_ACPI_STATIC fwnode type

On Wed, Nov 9, 2016 at 3:19 PM, Lorenzo Pieralisi
<[email protected]> wrote:
> On systems booting with a device tree, every struct device is associated
> with a struct device_node, that provides its DT firmware 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 operations 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.
>
> The struct acpi_device, that represents firmware objects in the
> ACPI namespace already includes a struct fwnode_handle of
> type FWNODE_ACPI as their member; the same abstraction is missing
> though for devices that are instantiated out of static ACPI tables
> entries (eg ARM SMMU devices).
>
> Add a new fwnode_handle type to associate devices created out
> of static ACPI table entries to the respective firmware components
> and create a simple ACPI core layer interface to dynamically allocate
> and free the corresponding firmware nodes so that kernel subsystems
> can use it to instantiate the nodes and associate them with the
> respective devices.
>
> Signed-off-by: Lorenzo Pieralisi <[email protected]>
> Reviewed-by: Hanjun Guo <[email protected]>
> Tested-by: Hanjun Guo <[email protected]>
> Tested-by: Tomasz Nowicki <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>

Acked-by: Rafael J. Wysocki <[email protected]>

Thanks!

2016-11-10 10:57:13

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v7 00/16] ACPI IORT ARM SMMU support

Hi Rafael,

On Thu, Nov 10, 2016 at 12:36:12AM +0100, Rafael J. Wysocki wrote:
> Hi Lorenzo,
>
> On Wed, Nov 9, 2016 at 3:19 PM, Lorenzo Pieralisi
> <[email protected]> wrote:
> > This patch series is v7 of a previous posting:
> >
> > https://lkml.org/lkml/2016/10/18/506
>
> I don't see anything objectionable in this series.

Thank you very much !

> Please let me know which patches in particular to look at in detail.

Apart from patch 1, patch 7 (and patch 16 that adds on top of it)
requires an ack from an ACPI core perspective. acpi_dma_configure()
should not affect x86/ia64 systems at all so those patches should
not really be controversial (patch 16 adds an IORT hook into
acpi_dma_configure() that is a nop by design on anything other than
ARM64), please let me know if they are ok.

Thank you again for your time,
Lorenzo

2016-11-10 11:11:54

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic

On Wed, Nov 09, 2016 at 02:40:08PM +0000, Robin Murphy wrote:

[...]

> > +void fwnode_iommu_set_ops(struct fwnode_handle *fwnode,
> > + const struct iommu_ops *ops)
> > +{
> > + struct iommu_fwentry *iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
> > +
> > + if (WARN_ON(!iommu))
> > + return;
> > +
> > + if (is_of_node(fwnode))
>
> Nit: this check is actually redundant, since to_of_node() already does
> the right thing and of_node_get() is NULL-safe - iommu_fwspec_init()
> already works that way. On the other hand, though, it is perhaps more
> intuitive to see it explicitly, and since to_of_node() is inline it
> ought to result in the same object code (I've not checked, though).

I can easily fold this change in the final code and I think we
should keep this consistent so I am happy to change my code and
make the is_of_node() check implicit.

> Either way:
>
> Reviewed-by: Robin Murphy <[email protected]>

Thank you !
Lorenzo

> > + of_node_get(to_of_node(fwnode));
> > +
> > + INIT_LIST_HEAD(&iommu->list);
> > + iommu->fwnode = fwnode;
> > + iommu->ops = ops;
> > + spin_lock(&iommu_fwentry_lock);
> > + list_add_tail(&iommu->list, &iommu_fwentry_list);
> > + spin_unlock(&iommu_fwentry_lock);
> > +}
> > +
> > +const struct iommu_ops *fwnode_iommu_get_ops(struct fwnode_handle *fwnode)
> > +{
> > + struct iommu_fwentry *node;
> > + const struct iommu_ops *ops = NULL;
> > +
> > + spin_lock(&iommu_fwentry_lock);
> > + list_for_each_entry(node, &iommu_fwentry_list, list)
> > + if (node->fwnode == fwnode) {
> > + ops = node->ops;
> > + break;
> > + }
> > + spin_unlock(&iommu_fwentry_lock);
> > + return ops;
> > +}
> > +
> > int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
> > const struct iommu_ops *ops)
> > {
> > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > index 5b82862..0f57ddc 100644
> > --- a/drivers/iommu/of_iommu.c
> > +++ b/drivers/iommu/of_iommu.c
> > @@ -96,45 +96,6 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
> > }
> > EXPORT_SYMBOL_GPL(of_get_dma_window);
> >
> > -struct of_iommu_node {
> > - struct list_head list;
> > - struct device_node *np;
> > - const struct iommu_ops *ops;
> > -};
> > -static LIST_HEAD(of_iommu_list);
> > -static DEFINE_SPINLOCK(of_iommu_lock);
> > -
> > -void of_iommu_set_ops(struct device_node *np, const struct iommu_ops *ops)
> > -{
> > - struct of_iommu_node *iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
> > -
> > - if (WARN_ON(!iommu))
> > - return;
> > -
> > - of_node_get(np);
> > - INIT_LIST_HEAD(&iommu->list);
> > - iommu->np = np;
> > - iommu->ops = ops;
> > - spin_lock(&of_iommu_lock);
> > - list_add_tail(&iommu->list, &of_iommu_list);
> > - spin_unlock(&of_iommu_lock);
> > -}
> > -
> > -const struct iommu_ops *of_iommu_get_ops(struct device_node *np)
> > -{
> > - struct of_iommu_node *node;
> > - const struct iommu_ops *ops = NULL;
> > -
> > - spin_lock(&of_iommu_lock);
> > - list_for_each_entry(node, &of_iommu_list, list)
> > - if (node->np == np) {
> > - ops = node->ops;
> > - break;
> > - }
> > - spin_unlock(&of_iommu_lock);
> > - return ops;
> > -}
> > -
> > static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
> > {
> > struct of_phandle_args *iommu_spec = data;
> > diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > index 436dc21..15d5478 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -351,6 +351,9 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
> > const struct iommu_ops *ops);
> > void iommu_fwspec_free(struct device *dev);
> > int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids);
> > +void fwnode_iommu_set_ops(struct fwnode_handle *fwnode,
> > + const struct iommu_ops *ops);
> > +const struct iommu_ops *fwnode_iommu_get_ops(struct fwnode_handle *fwnode);
> >
> > #else /* CONFIG_IOMMU_API */
> >
> > @@ -580,6 +583,17 @@ static inline int iommu_fwspec_add_ids(struct device *dev, u32 *ids,
> > return -ENODEV;
> > }
> >
> > +static inline void fwnode_iommu_set_ops(struct fwnode_handle *fwnode,
> > + const struct iommu_ops *ops)
> > +{
> > +}
> > +
> > +static inline
> > +const struct iommu_ops *fwnode_iommu_get_ops(struct fwnode_handle *fwnode)
> > +{
> > + return NULL;
> > +}
> > +
> > #endif /* CONFIG_IOMMU_API */
> >
> > #endif /* __LINUX_IOMMU_H */
> > diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
> > index e80b9c7..7681007 100644
> > --- a/include/linux/of_iommu.h
> > +++ b/include/linux/of_iommu.h
> > @@ -31,8 +31,16 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
> >
> > #endif /* CONFIG_OF_IOMMU */
> >
> > -void of_iommu_set_ops(struct device_node *np, const struct iommu_ops *ops);
> > -const struct iommu_ops *of_iommu_get_ops(struct device_node *np);
> > +static inline void of_iommu_set_ops(struct device_node *np,
> > + const struct iommu_ops *ops)
> > +{
> > + fwnode_iommu_set_ops(&np->fwnode, ops);
> > +}
> > +
> > +static inline const struct iommu_ops *of_iommu_get_ops(struct device_node *np)
> > +{
> > + return fwnode_iommu_get_ops(&np->fwnode);
> > +}
> >
> > extern struct of_device_id __iommu_of_table;
> >
> >
>

2016-11-11 12:44:58

by Hanjun Guo

[permalink] [raw]
Subject: Re: [PATCH v7 00/16] ACPI IORT ARM SMMU support

On 11/09/2016 10:19 PM, Lorenzo Pieralisi wrote:
> This patch series is v7 of a previous posting:
>
> https://lkml.org/lkml/2016/10/18/506
>
> v6 -> v7
> - Rebased against v4.9-rc4
> - Fixed IORT probing on ACPI systems with missing IORT table
> - Fixed SMMUv1/v2 global interrupt detection
> - Updated iommu_ops firmware look-up

Although no major update for the new version, I tested
this patchset again on hisilicon d03, and the platform
device works as before with SMMUv3.

Thanks
Hanjun

2016-11-11 15:22:54

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic

On Wed, Nov 09, 2016 at 02:19:36PM +0000, Lorenzo Pieralisi wrote:
> +struct iommu_fwentry {
> + struct list_head list;
> + struct fwnode_handle *fwnode;
> + const struct iommu_ops *ops;
> +};

Is there a reason the iommu_ops need to be stored there? Currently it
seems that the ops are only needed to get the of_xlate fn-ptr later. And
the place where it is called the iommu-ops should also be available
through pdev->dev->bus->iommu_ops.


Joerg

2016-11-11 16:17:45

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic

On 11/11/16 15:22, Joerg Roedel wrote:
> On Wed, Nov 09, 2016 at 02:19:36PM +0000, Lorenzo Pieralisi wrote:
>> +struct iommu_fwentry {
>> + struct list_head list;
>> + struct fwnode_handle *fwnode;
>> + const struct iommu_ops *ops;
>> +};
>
> Is there a reason the iommu_ops need to be stored there? Currently it
> seems that the ops are only needed to get the of_xlate fn-ptr later. And
> the place where it is called the iommu-ops should also be available
> through pdev->dev->bus->iommu_ops.

In the original of_iommu_configure design, the thought was that an ops
structure could be IOMMU-instance-specific (hence the later-removed
"priv" member), so I suppose right now it is mostly a hangover from
that. However, it's also what we initialise a device's fwspec with, so
becomes important again if we're ever going to get past the limitations
of buses-which-are-not-actually-buses[1].

Robin.

[1]:http://www.mail-archive.com/[email protected]/msg14576.html

2016-11-11 16:27:40

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic

On Fri, Nov 11, 2016 at 04:17:37PM +0000, Robin Murphy wrote:
> In the original of_iommu_configure design, the thought was that an ops
> structure could be IOMMU-instance-specific (hence the later-removed
> "priv" member), so I suppose right now it is mostly a hangover from
> that. However, it's also what we initialise a device's fwspec with, so
> becomes important again if we're ever going to get past the limitations
> of buses-which-are-not-actually-buses[1].

Yeah, I discussed this with a few others at LPC. My current idea is to
tell the iommu-core which hardware-iommus exist in the system and a
seperate iommu_ops ptr for each of them. Then every struct device can
link to the iommu-instance it is translated by.

We are not there yet, but this will give you the same per-device
iommu-ops as implemented here.



Joerg

2016-11-11 17:43:46

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic

On 11/11/16 16:27, Joerg Roedel wrote:
> On Fri, Nov 11, 2016 at 04:17:37PM +0000, Robin Murphy wrote:
>> In the original of_iommu_configure design, the thought was that an ops
>> structure could be IOMMU-instance-specific (hence the later-removed
>> "priv" member), so I suppose right now it is mostly a hangover from
>> that. However, it's also what we initialise a device's fwspec with, so
>> becomes important again if we're ever going to get past the limitations
>> of buses-which-are-not-actually-buses[1].
>
> Yeah, I discussed this with a few others at LPC. My current idea is to
> tell the iommu-core which hardware-iommus exist in the system and a
> seperate iommu_ops ptr for each of them. Then every struct device can
> link to the iommu-instance it is translated by.

Er, that sounds very much like a description of what we already have in
4.9-rc. Every struct device now has an iommu_fwspec which encapsulates
both an iommu_ops pointer (which can perfectly well be per-instance if
the IOMMU driver wants) and a place for the IOMMU-private data to
replace the mess of archdata.iommu and driver-internal globals.

> We are not there yet, but this will give you the same per-device
> iommu-ops as implemented here.

With those two patches I linked to, which make the bulk of the IOMMU
core code per-device-ops-aware off the bat, I'd say we *are* already
pretty much there. It's only iommu_domain_alloc() which needs a
device-based alternative, and the non-of_xlate-based IOMMU drivers to
either call iommu_fwspec_init() for themselves, or perhaps for x86
plumbing in DMAR/IVRS equivalents of the IORT parsing to the
infrastructure provided by this series.

Robin.

2016-11-14 10:26:01

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic

Hi Robin, Joerg,

On Fri, Nov 11, 2016 at 05:43:39PM +0000, Robin Murphy wrote:
> On 11/11/16 16:27, Joerg Roedel wrote:
> > On Fri, Nov 11, 2016 at 04:17:37PM +0000, Robin Murphy wrote:
> >> In the original of_iommu_configure design, the thought was that an ops
> >> structure could be IOMMU-instance-specific (hence the later-removed
> >> "priv" member), so I suppose right now it is mostly a hangover from
> >> that. However, it's also what we initialise a device's fwspec with, so
> >> becomes important again if we're ever going to get past the limitations
> >> of buses-which-are-not-actually-buses[1].
> >
> > Yeah, I discussed this with a few others at LPC. My current idea is to
> > tell the iommu-core which hardware-iommus exist in the system and a
> > seperate iommu_ops ptr for each of them. Then every struct device can
> > link to the iommu-instance it is translated by.
>
> Er, that sounds very much like a description of what we already have in
> 4.9-rc. Every struct device now has an iommu_fwspec which encapsulates
> both an iommu_ops pointer (which can perfectly well be per-instance if
> the IOMMU driver wants) and a place for the IOMMU-private data to
> replace the mess of archdata.iommu and driver-internal globals.
>
> > We are not there yet, but this will give you the same per-device
> > iommu-ops as implemented here.
>
> With those two patches I linked to, which make the bulk of the IOMMU
> core code per-device-ops-aware off the bat, I'd say we *are* already
> pretty much there. It's only iommu_domain_alloc() which needs a
> device-based alternative, and the non-of_xlate-based IOMMU drivers to
> either call iommu_fwspec_init() for themselves, or perhaps for x86
> plumbing in DMAR/IVRS equivalents of the IORT parsing to the
> infrastructure provided by this series.

I think it all boils down to how we end up implementing the per-device
iommu_ops look-up/binding, question is what do you want me to do with
this patch, it should be fine to drop it and use dev->bus->iommu_ops
for the look-up but I should know sooner rather than later to make
sure the series get another good round of testing.

Please let me know, thank you very much.

Lorenzo

2016-11-14 12:00:58

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic

On 14/11/16 10:26, Lorenzo Pieralisi wrote:
> Hi Robin, Joerg,
>
> On Fri, Nov 11, 2016 at 05:43:39PM +0000, Robin Murphy wrote:
>> On 11/11/16 16:27, Joerg Roedel wrote:
>>> On Fri, Nov 11, 2016 at 04:17:37PM +0000, Robin Murphy wrote:
>>>> In the original of_iommu_configure design, the thought was that an ops
>>>> structure could be IOMMU-instance-specific (hence the later-removed
>>>> "priv" member), so I suppose right now it is mostly a hangover from
>>>> that. However, it's also what we initialise a device's fwspec with, so
>>>> becomes important again if we're ever going to get past the limitations
>>>> of buses-which-are-not-actually-buses[1].
>>>
>>> Yeah, I discussed this with a few others at LPC. My current idea is to
>>> tell the iommu-core which hardware-iommus exist in the system and a
>>> seperate iommu_ops ptr for each of them. Then every struct device can
>>> link to the iommu-instance it is translated by.
>>
>> Er, that sounds very much like a description of what we already have in
>> 4.9-rc. Every struct device now has an iommu_fwspec which encapsulates
>> both an iommu_ops pointer (which can perfectly well be per-instance if
>> the IOMMU driver wants) and a place for the IOMMU-private data to
>> replace the mess of archdata.iommu and driver-internal globals.
>>
>>> We are not there yet, but this will give you the same per-device
>>> iommu-ops as implemented here.
>>
>> With those two patches I linked to, which make the bulk of the IOMMU
>> core code per-device-ops-aware off the bat, I'd say we *are* already
>> pretty much there. It's only iommu_domain_alloc() which needs a
>> device-based alternative, and the non-of_xlate-based IOMMU drivers to
>> either call iommu_fwspec_init() for themselves, or perhaps for x86
>> plumbing in DMAR/IVRS equivalents of the IORT parsing to the
>> infrastructure provided by this series.
>
> I think it all boils down to how we end up implementing the per-device
> iommu_ops look-up/binding, question is what do you want me to do with
> this patch, it should be fine to drop it and use dev->bus->iommu_ops
> for the look-up but I should know sooner rather than later to make
> sure the series get another good round of testing.

If we've already made the decision to move away from bus ops, I don't
see that it makes sense to deliberately introduce new dependencies on
them. Besides, as it stands, this patch literally implements "tell the
iommu-core which hardware-iommus exist in the system and a seperate
iommu_ops ptr for each of them" straight off.

Robin.

>
> Please let me know, thank you very much.
>
> Lorenzo
>

2016-11-14 15:52:32

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic

On Mon, Nov 14, 2016 at 12:00:47PM +0000, Robin Murphy wrote:
> If we've already made the decision to move away from bus ops, I don't
> see that it makes sense to deliberately introduce new dependencies on
> them. Besides, as it stands, this patch literally implements "tell the
> iommu-core which hardware-iommus exist in the system and a seperate
> iommu_ops ptr for each of them" straight off.

Not sure which code you are looking at, but as I see it we have only
per-device iommu-ops now (with this patch). That is different from
having core-visible hardware-iommu instances where devices could link
to.

Also the rest of iommu-core code still makes use of the per-bus ops. The
per-device ops are only used for the of_xlate fn-ptr.



Joerg

2016-11-14 16:41:35

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic

On Mon, Nov 14, 2016 at 04:52:23PM +0100, Joerg Roedel wrote:
> On Mon, Nov 14, 2016 at 12:00:47PM +0000, Robin Murphy wrote:
> > If we've already made the decision to move away from bus ops, I don't
> > see that it makes sense to deliberately introduce new dependencies on
> > them. Besides, as it stands, this patch literally implements "tell the
> > iommu-core which hardware-iommus exist in the system and a seperate
> > iommu_ops ptr for each of them" straight off.
>
> Not sure which code you are looking at, but as I see it we have only
> per-device iommu-ops now (with this patch). That is different from
> having core-visible hardware-iommu instances where devices could link
> to.

This patch enables the IOMMU-OF-node<->device look-up on non-OF (ie
ACPI) systems by "converting" the of_node to a generic fwnode_handle,
that's all it does (and move the related look-up code from
drivers/iommu/of_iommu.c to drivers/iommu/iommu.c so that it does
not depend on OF_IOMMU any longer).

> Also the rest of iommu-core code still makes use of the per-bus ops. The
> per-device ops are only used for the of_xlate fn-ptr.

I can put this patch on the backburner and retrieve the iommu_ops
through the dev->bus path in the IORT xlate function (iort_iommu_xlate()
introduced in the last patch), the change is trivial and should work
just fine but it deserves a v8 to give everyone a chance to test it.

We would end-up handling the device->iommu_ops look-up differently in DT
and ACPI for streamid translations though, I am not sure I see a reason
why.

Thanks,
Lorenzo

2016-11-14 18:25:26

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic

On 14/11/16 15:52, Joerg Roedel wrote:
> On Mon, Nov 14, 2016 at 12:00:47PM +0000, Robin Murphy wrote:
>> If we've already made the decision to move away from bus ops, I don't
>> see that it makes sense to deliberately introduce new dependencies on
>> them. Besides, as it stands, this patch literally implements "tell the
>> iommu-core which hardware-iommus exist in the system and a seperate
>> iommu_ops ptr for each of them" straight off.
>
> Not sure which code you are looking at, but as I see it we have only
> per-device iommu-ops now (with this patch). That is different from
> having core-visible hardware-iommu instances where devices could link
> to.

The per-device IOMMU ops are already there since 57f98d2f61e1. This
patch generalises the other end, moving the "registering an IOMMU
instance" (i.e. iommu_fwentry) bit into the IOMMU core, from being
OF-specific. I'd be perfectly happy if we rename iommu_fwentry to
iommu_instance, fwnode_iommu_set_ops() to iommu_register_instance(), and
such if that makes the design intent clearer.

If you'd also prefer to replace iommu_fwspec::ops with an opaque
iommu_fwspec::iommu_instance pointer so that things are a bit more
centralised (and users are forced to go through the API rather then call
ops directly), I'd have no major objection either. My main point is that
we've been deliberately putting the relevant building blocks in place -
the of_iommu_{get,set}_ops stuff was designed from the start to
accommodate per-instance ops, via the ops pointer *being* the instance
token; the iommu_fwspec stuff is deliberately intended to provide
per-device ops on top of that. The raw functionality is either there in
iommu.c already, or moving there in patches already written, so if it
doesn't look right all we need to focus on is making it look right.

> Also the rest of iommu-core code still makes use of the per-bus ops. The
> per-device ops are only used for the of_xlate fn-ptr.

Hence my aforementioned patches intended for 4.10, directly following on
from introducing iommu_fwspec in 4.9:

http://www.mail-archive.com/[email protected]/msg14576.html

...the purpose being to provide a smooth transition from per-bus ops to
per-device, per-instance ops. Apply those and we're 90% of the way there
for OF-based IOMMU drivers (not that any of those actually need
per-instance ops, admittedly; I did prototype it for the ARM SMMU ages
ago, but it didn't seem worth the bother). Lorenzo's series broadens the
scope to ACPI-based systems and moves the generically-useful parts into
the core where we can easily build on them further if necessary. The
major remaining work is to convert external callers of the current
bus-dependent functions like iommu_domain_alloc(), iommu_present(), etc.
to device-based alternatives.

Robin.

2016-11-15 10:06:05

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic

On Mon, Nov 14, 2016 at 06:25:16PM +0000, Robin Murphy wrote:
> On 14/11/16 15:52, Joerg Roedel wrote:
> > On Mon, Nov 14, 2016 at 12:00:47PM +0000, Robin Murphy wrote:
> >> If we've already made the decision to move away from bus ops, I don't
> >> see that it makes sense to deliberately introduce new dependencies on
> >> them. Besides, as it stands, this patch literally implements "tell the
> >> iommu-core which hardware-iommus exist in the system and a seperate
> >> iommu_ops ptr for each of them" straight off.
> >
> > Not sure which code you are looking at, but as I see it we have only
> > per-device iommu-ops now (with this patch). That is different from
> > having core-visible hardware-iommu instances where devices could link
> > to.
>
> The per-device IOMMU ops are already there since 57f98d2f61e1. This
> patch generalises the other end, moving the "registering an IOMMU
> instance" (i.e. iommu_fwentry) bit into the IOMMU core, from being
> OF-specific. I'd be perfectly happy if we rename iommu_fwentry to
> iommu_instance, fwnode_iommu_set_ops() to iommu_register_instance(), and
> such if that makes the design intent clearer.

I second that and I need to know what to do with this patch sooner
rather than later so it is time we make a decision please.

Joerg, what's your opinion ?

Thanks,
Lorenzo

> If you'd also prefer to replace iommu_fwspec::ops with an opaque
> iommu_fwspec::iommu_instance pointer so that things are a bit more
> centralised (and users are forced to go through the API rather then call
> ops directly), I'd have no major objection either. My main point is that
> we've been deliberately putting the relevant building blocks in place -
> the of_iommu_{get,set}_ops stuff was designed from the start to
> accommodate per-instance ops, via the ops pointer *being* the instance
> token; the iommu_fwspec stuff is deliberately intended to provide
> per-device ops on top of that. The raw functionality is either there in
> iommu.c already, or moving there in patches already written, so if it
> doesn't look right all we need to focus on is making it look right.
>
> > Also the rest of iommu-core code still makes use of the per-bus ops. The
> > per-device ops are only used for the of_xlate fn-ptr.
>
> Hence my aforementioned patches intended for 4.10, directly following on
> from introducing iommu_fwspec in 4.9:
>
> http://www.mail-archive.com/[email protected]/msg14576.html
>
> ...the purpose being to provide a smooth transition from per-bus ops to
> per-device, per-instance ops. Apply those and we're 90% of the way there
> for OF-based IOMMU drivers (not that any of those actually need
> per-instance ops, admittedly; I did prototype it for the ARM SMMU ages
> ago, but it didn't seem worth the bother). Lorenzo's series broadens the
> scope to ACPI-based systems and moves the generically-useful parts into
> the core where we can easily build on them further if necessary. The
> major remaining work is to convert external callers of the current
> bus-dependent functions like iommu_domain_alloc(), iommu_present(), etc.
> to device-based alternatives.
>
> Robin.

2016-11-15 10:12:15

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v7 00/16] ACPI IORT ARM SMMU support

Hi Rafael,

On Thu, Nov 10, 2016 at 12:36:12AM +0100, Rafael J. Wysocki wrote:
> Hi Lorenzo,
>
> On Wed, Nov 9, 2016 at 3:19 PM, Lorenzo Pieralisi
> <[email protected]> wrote:
> > This patch series is v7 of a previous posting:
> >
> > https://lkml.org/lkml/2016/10/18/506
>
> I don't see anything objectionable in this series.
>
> Please let me know which patches in particular to look at in detail.

Are patches 7 and consequently 16 (that builds on top of 7) ok with
you ? They should be equivalent to nop on anything other than ARM64
but they touch ACPI core so they require an ACK if they are ok please.

Thank you very much !
Lorenzo

2016-11-15 13:04:15

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v7 00/16] ACPI IORT ARM SMMU support

On Tue, Nov 15, 2016 at 11:12 AM, Lorenzo Pieralisi
<[email protected]> wrote:
> Hi Rafael,
>
> On Thu, Nov 10, 2016 at 12:36:12AM +0100, Rafael J. Wysocki wrote:
>> Hi Lorenzo,
>>
>> On Wed, Nov 9, 2016 at 3:19 PM, Lorenzo Pieralisi
>> <[email protected]> wrote:
>> > This patch series is v7 of a previous posting:
>> >
>> > https://lkml.org/lkml/2016/10/18/506
>>
>> I don't see anything objectionable in this series.
>>
>> Please let me know which patches in particular to look at in detail.
>
> Are patches 7 and consequently 16 (that builds on top of 7) ok with
> you ? They should be equivalent to nop on anything other than ARM64
> but they touch ACPI core so they require an ACK if they are ok please.

Yes, they are. Please feel free to add my ACKs to those or I will
send them separately later today.

Thanks,
Rafael

2016-11-15 14:06:54

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v7 00/16] ACPI IORT ARM SMMU support

On Tue, Nov 15, 2016 at 02:04:09PM +0100, Rafael J. Wysocki wrote:
> On Tue, Nov 15, 2016 at 11:12 AM, Lorenzo Pieralisi
> <[email protected]> wrote:
> > Hi Rafael,
> >
> > On Thu, Nov 10, 2016 at 12:36:12AM +0100, Rafael J. Wysocki wrote:
> >> Hi Lorenzo,
> >>
> >> On Wed, Nov 9, 2016 at 3:19 PM, Lorenzo Pieralisi
> >> <[email protected]> wrote:
> >> > This patch series is v7 of a previous posting:
> >> >
> >> > https://lkml.org/lkml/2016/10/18/506
> >>
> >> I don't see anything objectionable in this series.
> >>
> >> Please let me know which patches in particular to look at in detail.
> >
> > Are patches 7 and consequently 16 (that builds on top of 7) ok with
> > you ? They should be equivalent to nop on anything other than ARM64
> > but they touch ACPI core so they require an ACK if they are ok please.
>
> Yes, they are. Please feel free to add my ACKs to those or I will
> send them separately later today.

Thank you very much, I will add them to v8 that should be final,
I will send it when the dust has settled on patch 4, which looks
like the last bit to sort out (we may not even need a v8 at all).

Thanks !
Lorenzo

2016-11-16 00:55:34

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v7 07/16] drivers: acpi: implement acpi_dma_configure

On Wed, Nov 9, 2016 at 3:19 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 for now are just wrappers around arch_setup_dma_ops() and
> arch_teardown_dma_ops() and also updates ACPI and PCI core code to use
> the newly introduced acpi_dma_configure/acpi_dma_deconfigure functions.
>
> Since acpi_dma_configure() is used to configure DMA operations, the
> function initializes the dma/coherent_dma masks to sane default values
> if the current masks are uninitialized (also to keep the default values
> consistent with DT systems) to make sure the device has a complete
> default DMA set-up.
>
> 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.
>
> Signed-off-by: Lorenzo Pieralisi <[email protected]>
> Acked-by: Bjorn Helgaas <[email protected]> [pci]
> Tested-by: Hanjun Guo <[email protected]>
> Tested-by: Tomasz Nowicki <[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]>

LGTM

Acked-by: Rafael J. Wysocki <[email protected]>

2016-11-16 00:56:40

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v7 16/16] drivers: acpi: iort: introduce iort_iommu_configure

On Wed, Nov 9, 2016 at 3:19 PM, Lorenzo Pieralisi
<[email protected]> wrote:
> DT based systems have a generic kernel API to configure IOMMUs
> for devices (ie of_iommu_configure()).
>
> On ARM based ACPI systems, the of_iommu_configure() equivalent can
> be implemented atop ACPI IORT kernel API, with the corresponding
> functions to map device identifiers to IOMMUs and retrieve the
> corresponding IOMMU operations necessary for DMA operations set-up.
>
> By relying on the iommu_fwspec generic kernel infrastructure,
> implement the IORT based IOMMU configuration for ARM ACPI systems
> and hook it up in the ACPI kernel layer that implements DMA
> configuration for a device.
>
> Signed-off-by: Lorenzo Pieralisi <[email protected]>
> Tested-by: Hanjun Guo <[email protected]>
> Tested-by: Tomasz Nowicki <[email protected]>
> Cc: Hanjun Guo <[email protected]>
> Cc: Tomasz Nowicki <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>

For the ACPI core part:

Acked-by: Rafael J. Wysocki <[email protected]>

2016-11-16 09:55:21

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic

Hi Joerg,

On Mon, Nov 14, 2016 at 06:25:16PM +0000, Robin Murphy wrote:
> On 14/11/16 15:52, Joerg Roedel wrote:
> > On Mon, Nov 14, 2016 at 12:00:47PM +0000, Robin Murphy wrote:
> >> If we've already made the decision to move away from bus ops, I don't
> >> see that it makes sense to deliberately introduce new dependencies on
> >> them. Besides, as it stands, this patch literally implements "tell the
> >> iommu-core which hardware-iommus exist in the system and a seperate
> >> iommu_ops ptr for each of them" straight off.
> >
> > Not sure which code you are looking at, but as I see it we have only
> > per-device iommu-ops now (with this patch). That is different from
> > having core-visible hardware-iommu instances where devices could link
> > to.
>
> The per-device IOMMU ops are already there since 57f98d2f61e1. This
> patch generalises the other end, moving the "registering an IOMMU
> instance" (i.e. iommu_fwentry) bit into the IOMMU core, from being
> OF-specific. I'd be perfectly happy if we rename iommu_fwentry to
> iommu_instance, fwnode_iommu_set_ops() to iommu_register_instance(), and
> such if that makes the design intent clearer.

I can easily make the changes Robin suggests above, I need to know
what to do with this patch it is the last blocking point for this
series and time is running out I can revert to using dev->bus to
retrieve iommu_ops (even though I do not think it makes sense given
what Robin outlines below) but I need to know please, we can't gate
an entire series for this patch that is just syntactic sugar.

Thanks !
Lorenzo

> If you'd also prefer to replace iommu_fwspec::ops with an opaque
> iommu_fwspec::iommu_instance pointer so that things are a bit more
> centralised (and users are forced to go through the API rather then call
> ops directly), I'd have no major objection either. My main point is that
> we've been deliberately putting the relevant building blocks in place -
> the of_iommu_{get,set}_ops stuff was designed from the start to
> accommodate per-instance ops, via the ops pointer *being* the instance
> token; the iommu_fwspec stuff is deliberately intended to provide
> per-device ops on top of that. The raw functionality is either there in
> iommu.c already, or moving there in patches already written, so if it
> doesn't look right all we need to focus on is making it look right.
>
> > Also the rest of iommu-core code still makes use of the per-bus ops. The
> > per-device ops are only used for the of_xlate fn-ptr.
>
> Hence my aforementioned patches intended for 4.10, directly following on
> from introducing iommu_fwspec in 4.9:
>
> http://www.mail-archive.com/[email protected]/msg14576.html
>
> ...the purpose being to provide a smooth transition from per-bus ops to
> per-device, per-instance ops. Apply those and we're 90% of the way there
> for OF-based IOMMU drivers (not that any of those actually need
> per-instance ops, admittedly; I did prototype it for the ARM SMMU ages
> ago, but it didn't seem worth the bother). Lorenzo's series broadens the
> scope to ACPI-based systems and moves the generically-useful parts into
> the core where we can easily build on them further if necessary. The
> major remaining work is to convert external callers of the current
> bus-dependent functions like iommu_domain_alloc(), iommu_present(), etc.
> to device-based alternatives.
>
> Robin.

2016-11-16 13:28:08

by Tomasz Nowicki

[permalink] [raw]
Subject: Re: [PATCH v7 00/16] ACPI IORT ARM SMMU support

On 09.11.2016 15:19, Lorenzo Pieralisi wrote:
> This patch series is v7 of a previous posting:
>
> https://lkml.org/lkml/2016/10/18/506
>

[...]

>
> 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, this patchset enables ARM SMMUs 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 future ARM SMMU components easier to integrate.
>

[...]

>
> This patchset is provided for review/testing purposes here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git acpi/iort-smmu-v7
>
> Tested on Juno and FVP models for ARM SMMU v1 and v3 probing path.
>

For all series:
Reviewed-by: Tomasz Nowicki <[email protected]>

Thanks,
Tomasz

2016-11-29 16:06:09

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic

On Wed, Nov 16, 2016 at 09:56:15AM +0000, Lorenzo Pieralisi wrote:
> I can easily make the changes Robin suggests above, I need to know
> what to do with this patch it is the last blocking point for this
> series and time is running out I can revert to using dev->bus to
> retrieve iommu_ops (even though I do not think it makes sense given
> what Robin outlines below) but I need to know please, we can't gate
> an entire series for this patch that is just syntactic sugar.

Well, I didn't really object to the approach per-se, I just wanted to
know the rationale behind the need for the iommu-ops pointer. So through
which tree should this series be merged?
I think I can live with the pointer for now, we can later convert it to
an iommu-instance pointer.


Joerg

2016-11-29 17:07:00

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v7 04/16] drivers: iommu: make of_iommu_set/get_ops() DT agnostic

On Tue, Nov 29, 2016 at 05:05:49PM +0100, Joerg Roedel wrote:
> On Wed, Nov 16, 2016 at 09:56:15AM +0000, Lorenzo Pieralisi wrote:
> > I can easily make the changes Robin suggests above, I need to know
> > what to do with this patch it is the last blocking point for this
> > series and time is running out I can revert to using dev->bus to
> > retrieve iommu_ops (even though I do not think it makes sense given
> > what Robin outlines below) but I need to know please, we can't gate
> > an entire series for this patch that is just syntactic sugar.
>
> Well, I didn't really object to the approach per-se, I just wanted to
> know the rationale behind the need for the iommu-ops pointer. So through
> which tree should this series be merged?

I was just about to send a pull request to you, since it conflicts with
my ARM SMMU patches and the PCI/ACPI bits are acked by Rafael and Bjorn.

My for-joerg/arm-smmu/updates is ready to go; just need to write the
pull request.

Will