[PATCH V7 0/8] Introduce ACPI world to ITS irqchip

[PATCH V7 0/8] Introduce ACPI world to ITS irqchip

[Tomasz Nowicki]

The series builds the PCI/MSI domain stack based on initial IORT driver
which is added in first place. As a reference please see IORT spec:
http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf

Tested on Cavium ThunderX server. The patches can be found here:
https://github.com/semihalf-nowicki-tomasz/linux.git (its-acpi-v7)

v6 -> v7
- rebased against irq/irqchip-4.8
- improvements for IORT ID mapping
- call IORT init call from acpi_init() instead of arch_initcall()
- split IORT driver in to two parts: core and ITS/MSI

v5 -> v6
- added locking mechanism for IORT list with domain token
- IORT function name improvements
- extended IORT RID mapping helper
- reworked IORT to be more SMMU friendly
- IORT functions which map RID and find corresponding domain are not
  PCI specific any more (struct pci_dev -> struct device)
- bug fixes

v4 -> v5
- rebased against v4.7-rc1
- drop generic layer and call IORT functions directly
- improve resource abstraction and string formatting

v3 -> v4
- rebased against v4.5
- add ACPI support for IRQ domain handling on a per-device basis
- reorder domain setup step
- improve error handling
- code style improvements

v2 -> v3
- rebased on top of 4.4
- fixes and improvements for redistributor init via GICC structures
- fixes as per kbuild reports

v1 -> v2
- rebased on top of 4.4-rc4
- use pci_msi_domain_get_msi_rid for requester ID to device ID translation

Tomasz Nowicki (8):
  ACPI: I/O Remapping Table (IORT) initial support
  ACPI: Add new IORT functions to support MSI domain handling
  PCI/MSI: Setup MSI domain on a per-device basis using IORT ACPI table
  irqchip/gicv3-its: Cleanup for ITS domain initialization
  irqchip/gicv3-its: Refactor ITS DT init code to prepare for ACPI
  irqchip/gicv3-its: Probe ITS in the ACPI way
  irqchip/gicv3-its: Factor out code that might be reused for ACPI
  irqchip/gicv3-its: Use MADT ITS subtable to do PCI/MSI domain
    initialization

 drivers/acpi/Kconfig                     |   3 +
 drivers/acpi/Makefile                    |   1 +
 drivers/acpi/bus.c                       |   2 +
 drivers/acpi/iort.c                      | 389 +++++++++++++++++++++++++++++++
 drivers/irqchip/Kconfig                  |   1 +
 drivers/irqchip/irq-gic-v3-its-pci-msi.c |  88 +++++--
 drivers/irqchip/irq-gic-v3-its.c         | 169 ++++++++++----
 drivers/irqchip/irq-gic-v3.c             |   7 +-
 drivers/pci/msi.c                        |  11 +-
 include/linux/iort.h                     |  41 ++++
 include/linux/irqchip/arm-gic-v3.h       |   4 +-
 11 files changed, 646 insertions(+), 70 deletions(-)
 create mode 100644 drivers/acpi/iort.c
 create mode 100644 include/linux/iort.h

-- 
1.9.1

[PATCH V7 1/8] ACPI: I/O Remapping Table (IORT) initial support

[Tomasz Nowicki]

IORT shows representation of IO topology for ARM based systems.
It describes how various components are connected together on
parent-child basis e.g. PCI RC -> SMMU -> ITS. Also see IORT spec.

Initial support allows to detect IORT table presence and save its
root pointer obtained through acpi_get_table(). The pointer validity
depends on acpi_gbl_permanent_mmap because if acpi_gbl_permanent_mmap
is not set while using IORT nodes we would dereference unmapped pointers.

For the aforementioned reason call iort_table_detect() from acpi_init()
which guarantees acpi_gbl_permanent_mmap to be set at that point.

Add generic helpers which are helpful for scanning and retrieving
information from IORT table content. List of the most important helpers:
- iort_find_dev_node() finds IORT node for a given device
- iort_node_map_rid() maps device RID and returns IORT node which provides
  final translation

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
 drivers/acpi/Kconfig  |   3 +
 drivers/acpi/Makefile |   1 +
 drivers/acpi/bus.c    |   2 +
 drivers/acpi/iort.c   | 220 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/iort.h  |  30 +++++++
 5 files changed, 256 insertions(+)
 create mode 100644 drivers/acpi/iort.c
 create mode 100644 include/linux/iort.h

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index b7e2e77..848471f 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
 config ACPI_CCA_REQUIRED
 	bool
 
+config IORT_TABLE
+	bool
+
 config ACPI_DEBUGGER
 	bool "AML debugger interface"
 	select ACPI_DEBUG
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 251ce85..c7c9b29 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
 obj-$(CONFIG_ACPI_BGRT)		+= bgrt.o
 obj-$(CONFIG_ACPI_CPPC_LIB)	+= cppc_acpi.o
 obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
+obj-$(CONFIG_IORT_TABLE) 	+= iort.o
 
 # processor has its own "processor." module_param namespace
 processor-y			:= processor_driver.o
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 31e8da6..176c17d 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -33,6 +33,7 @@
 #ifdef CONFIG_X86
 #include <asm/mpspec.h>
 #endif
+#include <linux/iort.h>
 #include <linux/pci.h>
 #include <acpi/apei.h>
 #include <linux/dmi.h>
@@ -1118,6 +1119,7 @@ static int __init acpi_init(void)
 	}
 
 	pci_mmcfg_late_init();
+	iort_table_detect();
 	acpi_scan_init();
 	acpi_ec_init();
 	acpi_debugfs_init();
diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
new file mode 100644
index 0000000..e342bac
--- /dev/null
+++ b/drivers/acpi/iort.c
@@ -0,0 +1,220 @@
+/*
+ * Copyright (C) 2016, Semihalf
+ *	Author: Tomasz Nowicki <tn@semihalf.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * This file implements early detection/parsing of I/O mapping
+ * reported to OS through firmware via I/O Remapping Table (IORT)
+ * IORT document number: ARM DEN 0049A
+ */
+
+#define pr_fmt(fmt)	"ACPI: IORT: " fmt
+
+#include <linux/iort.h>
+#include <linux/kernel.h>
+#include <linux/pci.h>
+
+typedef acpi_status (*iort_find_node_callback)
+	(struct acpi_iort_node *node, void *context);
+
+/* Root pointer to the mapped IORT table */
+static struct acpi_table_header *iort_table;
+
+static struct acpi_iort_node *
+iort_scan_node(enum acpi_iort_node_type type,
+	       iort_find_node_callback callback, void *context)
+{
+	struct acpi_iort_node *iort_node, *iort_end;
+	struct acpi_table_iort *iort;
+	int i;
+
+	/* Get the first IORT node */
+	iort = (struct acpi_table_iort *)iort_table;
+	iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort,
+				 iort->node_offset);
+	iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
+				iort_table->length);
+
+	for (i = 0; i < iort->node_count; i++) {
+		if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND,
+			       "IORT node pointer overflows, bad table!\n"))
+			return NULL;
+
+		if (iort_node->type == type) {
+			if (ACPI_SUCCESS(callback(iort_node, context)))
+				return iort_node;
+		}
+
+		iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
+					 iort_node->length);
+	}
+
+	return NULL;
+}
+
+static acpi_status
+iort_match_node_callback(struct acpi_iort_node *node, void *context)
+{
+	struct device *dev = context;
+
+	switch (node->type) {
+	case ACPI_IORT_NODE_NAMED_COMPONENT: {
+		struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+		struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
+		struct acpi_iort_named_component *ncomp;
+
+		if (!adev)
+			break;
+
+		ncomp = (struct acpi_iort_named_component *)node->node_data;
+
+		if (ACPI_FAILURE(acpi_get_name(adev->handle,
+					       ACPI_FULL_PATHNAME, &buffer))) {
+			dev_warn(dev, "Can't get device full path name\n");
+			break;
+		}
+
+		if (!strcmp(ncomp->device_name, (char *)buffer.pointer))
+			return AE_OK;
+
+		break;
+	}
+	case ACPI_IORT_NODE_PCI_ROOT_COMPLEX: {
+		struct acpi_iort_root_complex *pci_rc;
+		struct pci_bus *bus;
+
+		bus = to_pci_bus(dev);
+		pci_rc = (struct acpi_iort_root_complex *)node->node_data;
+
+		/*
+		 * It is assumed that PCI segment numbers maps one-to-one
+		 * with root complexes. Each segment number can represent only
+		 * one root complex.
+		 */
+		if (pci_rc->pci_segment_number == pci_domain_nr(bus))
+			return AE_OK;
+
+		break;
+	}
+	}
+
+	return AE_NOT_FOUND;
+}
+
+static int
+iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in, u32 *rid_out)
+{
+	if (!rid_out)
+		return -EINVAL;
+
+	/* Single mapping does not care for input id */
+	if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
+		if (type == ACPI_IORT_NODE_NAMED_COMPONENT ||
+		    type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
+			*rid_out = map->output_base;
+			return 0;
+		}
+
+		pr_warn(FW_BUG "[map %p] SINGLE MAPPING flag not allowed for node type %d, skipping ID map\n",
+			map, type);
+		return -ENXIO;
+	}
+
+	if (rid_in < map->input_base ||
+	    (rid_in > map->input_base + map->id_count))
+		return -ENXIO;
+
+	*rid_out = map->output_base + (rid_in - map->input_base);
+	return 0;
+}
+
+static struct acpi_iort_node *
+iort_node_map_rid(struct acpi_iort_node *node, u32 rid_in,
+		  u32 *rid_out, u8 type)
+{
+	u32 rid = rid_in;
+	int found = 0;
+
+	/* Climb up ID mapping tree to find specified node type */
+	while (node && node->type != type) {
+		struct acpi_iort_id_mapping *map;
+		int i;
+
+		if (!node->mapping_offset || !node->mapping_count) {
+			node = NULL;
+			break;
+		}
+
+		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);
+			node = NULL;
+			break;
+		}
+
+		/* Do the RID translation */
+		for (i = 0; i < node->mapping_count; i++, map++) {
+			if(!iort_id_map(map, node->type, rid, &rid)) {
+				found = 1;
+				break;
+			}
+		}
+
+		if (!found) {
+			node = NULL;
+			break;
+		}
+
+		node = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
+				    map->output_reference);
+	}
+
+	/* Take input RID as output RID in case of any error */
+	if (rid_out)
+		*rid_out = node ? rid : rid_in;
+	return node;
+}
+
+static struct acpi_iort_node *
+iort_find_dev_node(struct device *dev)
+{
+	struct pci_bus *pbus;
+
+	if (!dev_is_pci(dev))
+		return iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
+				      iort_match_node_callback, dev);
+
+	/* Find a PCI root bus */
+	pbus = to_pci_dev(dev)->bus;
+	while (!pci_is_root_bus(pbus))
+		pbus = pbus->parent;
+
+	return iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
+			      iort_match_node_callback, &pbus->dev);
+}
+
+void __init iort_table_detect(void)
+{
+	acpi_status status;
+
+	if (acpi_disabled)
+		return;
+
+	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);
+	}
+}
diff --git a/include/linux/iort.h b/include/linux/iort.h
new file mode 100644
index 0000000..cde6809
--- /dev/null
+++ b/include/linux/iort.h
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2016, Semihalf
+ *	Author: Tomasz Nowicki <tn@semihalf.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+ */
+
+#ifndef __IORT_H__
+#define __IORT_H__
+
+#include <linux/acpi.h>
+
+#ifdef CONFIG_IORT_TABLE
+void iort_table_detect(void);
+#else
+static inline void iort_table_detect(void) { }
+#endif
+
+#endif /* __IORT_H__ */
-- 
1.9.1

Re: [PATCH V7 1/8] ACPI: I/O Remapping Table (IORT) initial support

[Hanjun Guo]

Hi Tomasz,

Sorry for jumping out late, just one comment below.

On 2016/6/20 19:02, Tomasz Nowicki wrote:
> IORT shows representation of IO topology for ARM based systems.
> It describes how various components are connected together on
> parent-child basis e.g. PCI RC -> SMMU -> ITS. Also see IORT spec.
>
> Initial support allows to detect IORT table presence and save its
> root pointer obtained through acpi_get_table(). The pointer validity
> depends on acpi_gbl_permanent_mmap because if acpi_gbl_permanent_mmap
> is not set while using IORT nodes we would dereference unmapped pointers.
>
> For the aforementioned reason call iort_table_detect() from acpi_init()
> which guarantees acpi_gbl_permanent_mmap to be set at that point.
>
> Add generic helpers which are helpful for scanning and retrieving
> information from IORT table content. List of the most important helpers:
> - iort_find_dev_node() finds IORT node for a given device
> - iort_node_map_rid() maps device RID and returns IORT node which provides
>   final translation
>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
>  drivers/acpi/Kconfig  |   3 +
>  drivers/acpi/Makefile |   1 +
>  drivers/acpi/bus.c    |   2 +
>  drivers/acpi/iort.c   | 220 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/iort.h  |  30 +++++++

[...]

> +static acpi_status
> +iort_match_node_callback(struct acpi_iort_node *node, void *context)
> +{
> +	struct device *dev = context;
> +
> +	switch (node->type) {
> +	case ACPI_IORT_NODE_NAMED_COMPONENT: {
> +		struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };

ACPI_ALLOCATE_BUFFER is used here, so ...

> +		struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
> +		struct acpi_iort_named_component *ncomp;
> +
> +		if (!adev)
> +			break;
> +
> +		ncomp = (struct acpi_iort_named_component *)node->node_data;
> +
> +		if (ACPI_FAILURE(acpi_get_name(adev->handle,
> +					       ACPI_FULL_PATHNAME, &buffer))) {
> +			dev_warn(dev, "Can't get device full path name\n");
> +			break;
> +		}
> +
> +		if (!strcmp(ncomp->device_name, (char *)buffer.pointer))
> +			return AE_OK;

... we need to kfree(buffer.pointer) before we return or break.

With this fixed,

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

> +
> +		break;
> +	}

Re: [PATCH V7 1/8] ACPI: I/O Remapping Table (IORT) initial support

[Marc Zyngier]

On 21/06/16 08:12, Hanjun Guo wrote:
> Hi Tomasz,
> 
> Sorry for jumping out late, just one comment below.
> 
> On 2016/6/20 19:02, Tomasz Nowicki wrote:
>> IORT shows representation of IO topology for ARM based systems.
>> It describes how various components are connected together on
>> parent-child basis e.g. PCI RC -> SMMU -> ITS. Also see IORT spec.
>>
>> Initial support allows to detect IORT table presence and save its
>> root pointer obtained through acpi_get_table(). The pointer validity
>> depends on acpi_gbl_permanent_mmap because if acpi_gbl_permanent_mmap
>> is not set while using IORT nodes we would dereference unmapped pointers.
>>
>> For the aforementioned reason call iort_table_detect() from acpi_init()
>> which guarantees acpi_gbl_permanent_mmap to be set at that point.
>>
>> Add generic helpers which are helpful for scanning and retrieving
>> information from IORT table content. List of the most important helpers:
>> - iort_find_dev_node() finds IORT node for a given device
>> - iort_node_map_rid() maps device RID and returns IORT node which provides
>>   final translation
>>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> ---
>>  drivers/acpi/Kconfig  |   3 +
>>  drivers/acpi/Makefile |   1 +
>>  drivers/acpi/bus.c    |   2 +
>>  drivers/acpi/iort.c   | 220 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/iort.h  |  30 +++++++
> 
> [...]
> 
>> +static acpi_status
>> +iort_match_node_callback(struct acpi_iort_node *node, void *context)
>> +{
>> +	struct device *dev = context;
>> +
>> +	switch (node->type) {
>> +	case ACPI_IORT_NODE_NAMED_COMPONENT: {
>> +		struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> 
> ACPI_ALLOCATE_BUFFER is used here, so ...
> 
>> +		struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
>> +		struct acpi_iort_named_component *ncomp;
>> +
>> +		if (!adev)
>> +			break;
>> +
>> +		ncomp = (struct acpi_iort_named_component *)node->node_data;
>> +
>> +		if (ACPI_FAILURE(acpi_get_name(adev->handle,
>> +					       ACPI_FULL_PATHNAME, &buffer))) {
>> +			dev_warn(dev, "Can't get device full path name\n");
>> +			break;
>> +		}
>> +
>> +		if (!strcmp(ncomp->device_name, (char *)buffer.pointer))
>> +			return AE_OK;
> 
> ... we need to kfree(buffer.pointer) before we return or break.

For the record, I've queued this patch on top:

diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
index 496dcf6..3b9e55b 100644
--- a/drivers/acpi/iort.c
+++ b/drivers/acpi/iort.c
@@ -158,11 +158,15 @@ iort_match_node_callback(struct acpi_iort_node *node, void *context)
 		if (ACPI_FAILURE(acpi_get_name(adev->handle,
 					       ACPI_FULL_PATHNAME, &buffer))) {
 			dev_warn(dev, "Can't get device full path name\n");
-			break;
-		}
+		} else {
+			int match;
 
-		if (!strcmp(ncomp->device_name, (char *)buffer.pointer))
-			return AE_OK;
+			match = !strcmp(ncomp->device_name, buffer.pointer);
+			acpi_os_free(&buffer);
+
+			if (match)
+				return AE_OK;
+		}
 
 		break;
 	}


assuming that Rafael is OK with the general approach, of course.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH V7 1/8] ACPI: I/O Remapping Table (IORT) initial support

[Tomasz Nowicki]

On 22.06.2016 12:50, Marc Zyngier wrote:
> On 21/06/16 08:12, Hanjun Guo wrote:
>> Hi Tomasz,
>>
>> Sorry for jumping out late, just one comment below.
>>
>> On 2016/6/20 19:02, Tomasz Nowicki wrote:
>>> IORT shows representation of IO topology for ARM based systems.
>>> It describes how various components are connected together on
>>> parent-child basis e.g. PCI RC -> SMMU -> ITS. Also see IORT spec.
>>>
>>> Initial support allows to detect IORT table presence and save its
>>> root pointer obtained through acpi_get_table(). The pointer validity
>>> depends on acpi_gbl_permanent_mmap because if acpi_gbl_permanent_mmap
>>> is not set while using IORT nodes we would dereference unmapped pointers.
>>>
>>> For the aforementioned reason call iort_table_detect() from acpi_init()
>>> which guarantees acpi_gbl_permanent_mmap to be set at that point.
>>>
>>> Add generic helpers which are helpful for scanning and retrieving
>>> information from IORT table content. List of the most important helpers:
>>> - iort_find_dev_node() finds IORT node for a given device
>>> - iort_node_map_rid() maps device RID and returns IORT node which provides
>>>    final translation
>>>
>>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>>> ---
>>>   drivers/acpi/Kconfig  |   3 +
>>>   drivers/acpi/Makefile |   1 +
>>>   drivers/acpi/bus.c    |   2 +
>>>   drivers/acpi/iort.c   | 220 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   include/linux/iort.h  |  30 +++++++
>>
>> [...]
>>
>>> +static acpi_status
>>> +iort_match_node_callback(struct acpi_iort_node *node, void *context)
>>> +{
>>> +	struct device *dev = context;
>>> +
>>> +	switch (node->type) {
>>> +	case ACPI_IORT_NODE_NAMED_COMPONENT: {
>>> +		struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>>
>> ACPI_ALLOCATE_BUFFER is used here, so ...
>>
>>> +		struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
>>> +		struct acpi_iort_named_component *ncomp;
>>> +
>>> +		if (!adev)
>>> +			break;
>>> +
>>> +		ncomp = (struct acpi_iort_named_component *)node->node_data;
>>> +
>>> +		if (ACPI_FAILURE(acpi_get_name(adev->handle,
>>> +					       ACPI_FULL_PATHNAME, &buffer))) {
>>> +			dev_warn(dev, "Can't get device full path name\n");
>>> +			break;
>>> +		}
>>> +
>>> +		if (!strcmp(ncomp->device_name, (char *)buffer.pointer))
>>> +			return AE_OK;
>>
>> ... we need to kfree(buffer.pointer) before we return or break.
>
> For the record, I've queued this patch on top:
>
> diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
> index 496dcf6..3b9e55b 100644
> --- a/drivers/acpi/iort.c
> +++ b/drivers/acpi/iort.c
> @@ -158,11 +158,15 @@ iort_match_node_callback(struct acpi_iort_node *node, void *context)
>   		if (ACPI_FAILURE(acpi_get_name(adev->handle,
>   					       ACPI_FULL_PATHNAME, &buffer))) {
>   			dev_warn(dev, "Can't get device full path name\n");
> -			break;
> -		}
> +		} else {
> +			int match;
>
> -		if (!strcmp(ncomp->device_name, (char *)buffer.pointer))
> -			return AE_OK;
> +			match = !strcmp(ncomp->device_name, buffer.pointer);
> +			acpi_os_free(&buffer);
> +
> +			if (match)
> +				return AE_OK;
> +		}
>
>   		break;
>   	}
>
>
> assuming that Rafael is OK with the general approach, of course.
>

Thanks Marc. As Lorenzo pointed out, we need another fix for 
iort_node_map_rid(). Do you want me to refresh the whole series or this 
patch only?

Tomasz

Re: [PATCH V7 1/8] ACPI: I/O Remapping Table (IORT) initial support

[Marc Zyngier]

On 22/06/16 12:06, Tomasz Nowicki wrote:
> On 22.06.2016 12:50, Marc Zyngier wrote:
>> On 21/06/16 08:12, Hanjun Guo wrote:
>>> Hi Tomasz,
>>>
>>> Sorry for jumping out late, just one comment below.
>>>
>>> On 2016/6/20 19:02, Tomasz Nowicki wrote:
>>>> IORT shows representation of IO topology for ARM based systems.
>>>> It describes how various components are connected together on
>>>> parent-child basis e.g. PCI RC -> SMMU -> ITS. Also see IORT spec.
>>>>
>>>> Initial support allows to detect IORT table presence and save its
>>>> root pointer obtained through acpi_get_table(). The pointer validity
>>>> depends on acpi_gbl_permanent_mmap because if acpi_gbl_permanent_mmap
>>>> is not set while using IORT nodes we would dereference unmapped pointers.
>>>>
>>>> For the aforementioned reason call iort_table_detect() from acpi_init()
>>>> which guarantees acpi_gbl_permanent_mmap to be set at that point.
>>>>
>>>> Add generic helpers which are helpful for scanning and retrieving
>>>> information from IORT table content. List of the most important helpers:
>>>> - iort_find_dev_node() finds IORT node for a given device
>>>> - iort_node_map_rid() maps device RID and returns IORT node which provides
>>>>    final translation
>>>>
>>>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>>>> ---
>>>>   drivers/acpi/Kconfig  |   3 +
>>>>   drivers/acpi/Makefile |   1 +
>>>>   drivers/acpi/bus.c    |   2 +
>>>>   drivers/acpi/iort.c   | 220 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>   include/linux/iort.h  |  30 +++++++
>>>
>>> [...]
>>>
>>>> +static acpi_status
>>>> +iort_match_node_callback(struct acpi_iort_node *node, void *context)
>>>> +{
>>>> +	struct device *dev = context;
>>>> +
>>>> +	switch (node->type) {
>>>> +	case ACPI_IORT_NODE_NAMED_COMPONENT: {
>>>> +		struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>>>
>>> ACPI_ALLOCATE_BUFFER is used here, so ...
>>>
>>>> +		struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
>>>> +		struct acpi_iort_named_component *ncomp;
>>>> +
>>>> +		if (!adev)
>>>> +			break;
>>>> +
>>>> +		ncomp = (struct acpi_iort_named_component *)node->node_data;
>>>> +
>>>> +		if (ACPI_FAILURE(acpi_get_name(adev->handle,
>>>> +					       ACPI_FULL_PATHNAME, &buffer))) {
>>>> +			dev_warn(dev, "Can't get device full path name\n");
>>>> +			break;
>>>> +		}
>>>> +
>>>> +		if (!strcmp(ncomp->device_name, (char *)buffer.pointer))
>>>> +			return AE_OK;
>>>
>>> ... we need to kfree(buffer.pointer) before we return or break.
>>
>> For the record, I've queued this patch on top:
>>
>> diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
>> index 496dcf6..3b9e55b 100644
>> --- a/drivers/acpi/iort.c
>> +++ b/drivers/acpi/iort.c
>> @@ -158,11 +158,15 @@ iort_match_node_callback(struct acpi_iort_node *node, void *context)
>>   		if (ACPI_FAILURE(acpi_get_name(adev->handle,
>>   					       ACPI_FULL_PATHNAME, &buffer))) {
>>   			dev_warn(dev, "Can't get device full path name\n");
>> -			break;
>> -		}
>> +		} else {
>> +			int match;
>>
>> -		if (!strcmp(ncomp->device_name, (char *)buffer.pointer))
>> -			return AE_OK;
>> +			match = !strcmp(ncomp->device_name, buffer.pointer);
>> +			acpi_os_free(&buffer);
>> +
>> +			if (match)
>> +				return AE_OK;
>> +		}
>>
>>   		break;
>>   	}
>>
>>
>> assuming that Rafael is OK with the general approach, of course.
>>
> 
> Thanks Marc. As Lorenzo pointed out, we need another fix for 
> iort_node_map_rid(). Do you want me to refresh the whole series or this 
> patch only?

If the changes are localized enough, just this patch.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH V7 1/8] ACPI: I/O Remapping Table (IORT) initial support

[Lorenzo Pieralisi]

On Mon, Jun 20, 2016 at 01:02:14PM +0200, Tomasz Nowicki wrote:
> IORT shows representation of IO topology for ARM based systems.
> It describes how various components are connected together on
> parent-child basis e.g. PCI RC -> SMMU -> ITS. Also see IORT spec.

Add the spec link if you want a reference to it otherwise "Also see IORT spec"
is pretty much useless.

> Initial support allows to detect IORT table presence and save its
> root pointer obtained through acpi_get_table(). The pointer validity
> depends on acpi_gbl_permanent_mmap because if acpi_gbl_permanent_mmap
> is not set while using IORT nodes we would dereference unmapped pointers.

We still need Rafael's feeback on this.

> For the aforementioned reason call iort_table_detect() from acpi_init()
> which guarantees acpi_gbl_permanent_mmap to be set at that point.

Ditto.

> Add generic helpers which are helpful for scanning and retrieving
> information from IORT table content. List of the most important helpers:
> - iort_find_dev_node() finds IORT node for a given device
> - iort_node_map_rid() maps device RID and returns IORT node which provides
>   final translation
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
>  drivers/acpi/Kconfig  |   3 +
>  drivers/acpi/Makefile |   1 +
>  drivers/acpi/bus.c    |   2 +
>  drivers/acpi/iort.c   | 220 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/iort.h  |  30 +++++++
>  5 files changed, 256 insertions(+)
>  create mode 100644 drivers/acpi/iort.c
>  create mode 100644 include/linux/iort.h
> 
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index b7e2e77..848471f 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
>  config ACPI_CCA_REQUIRED
>  	bool
>  
> +config IORT_TABLE
> +	bool
> +
>  config ACPI_DEBUGGER
>  	bool "AML debugger interface"
>  	select ACPI_DEBUG
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 251ce85..c7c9b29 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
>  obj-$(CONFIG_ACPI_BGRT)		+= bgrt.o
>  obj-$(CONFIG_ACPI_CPPC_LIB)	+= cppc_acpi.o
>  obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
> +obj-$(CONFIG_IORT_TABLE) 	+= iort.o
>  
>  # processor has its own "processor." module_param namespace
>  processor-y			:= processor_driver.o
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 31e8da6..176c17d 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -33,6 +33,7 @@
>  #ifdef CONFIG_X86
>  #include <asm/mpspec.h>
>  #endif
> +#include <linux/iort.h>
>  #include <linux/pci.h>
>  #include <acpi/apei.h>
>  #include <linux/dmi.h>
> @@ -1118,6 +1119,7 @@ static int __init acpi_init(void)
>  	}
>  
>  	pci_mmcfg_late_init();
> +	iort_table_detect();

IORT is arch specific (and empty on any system other than ARM64), this call
here is fine by me because it provides us the ordering we need (ie
initialize the IORT table pointers before we start running scan handlers),
we need Rafael's feeback on this though.

[...]

> +static int
> +iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in, u32 *rid_out)
> +{
> +	if (!rid_out)
> +		return -EINVAL;

Nit: It is an internal function honestly I do not see the need for
this check here, it is close to paranoia.

> +	/* Single mapping does not care for input id */
> +	if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
> +		if (type == ACPI_IORT_NODE_NAMED_COMPONENT ||
> +		    type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
> +			*rid_out = map->output_base;
> +			return 0;
> +		}
> +
> +		pr_warn(FW_BUG "[map %p] SINGLE MAPPING flag not allowed for node type %d, skipping ID map\n",
> +			map, type);
> +		return -ENXIO;
> +	}
> +
> +	if (rid_in < map->input_base ||
> +	    (rid_in > map->input_base + map->id_count))
> +		return -ENXIO;
> +
> +	*rid_out = map->output_base + (rid_in - map->input_base);
> +	return 0;
> +}
> +
> +static struct acpi_iort_node *
> +iort_node_map_rid(struct acpi_iort_node *node, u32 rid_in,
> +		  u32 *rid_out, u8 type)
> +{
> +	u32 rid = rid_in;
> +	int found = 0;
> +
> +	/* Climb up ID mapping tree to find specified node type */

s/Climb/Parse the/

> +	while (node && node->type != type) {
> +		struct acpi_iort_id_mapping *map;
> +		int i;
> +
> +		if (!node->mapping_offset || !node->mapping_count) {
> +			node = NULL;
> +			break;
> +		}
> +
> +		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);
> +			node = NULL;
> +			break;
> +		}
> +
> +		/* Do the RID translation */
> +		for (i = 0; i < node->mapping_count; i++, map++) {
> +			if(!iort_id_map(map, node->type, rid, &rid)) {
> +				found = 1;

You do not reset found to 0, for next iteration. I still think this function
is too complicated.

> +				break;
> +			}
> +		}
> +
> +		if (!found) {
> +			node = NULL;
> +			break;
> +		}

You could even remove "found" altogether:

if (i == node->mapping_count)
	goto fail_map;

 fail_map:
        /* Map input RID to output RID unchanged on mapping failure*/
 	*rid_out = rid_in;
	return NULL;

At least it is clear what you return when.

> +
> +		node = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> +				    map->output_reference);
> +	}
> +
> +	/* Take input RID as output RID in case of any error */
> +	if (rid_out)
> +		*rid_out = node ? rid : rid_in;
> +	return node;
> +}
> +
> +static struct acpi_iort_node *
> +iort_find_dev_node(struct device *dev)
> +{
> +	struct pci_bus *pbus;
> +
> +	if (!dev_is_pci(dev))
> +		return iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
> +				      iort_match_node_callback, dev);
> +
> +	/* Find a PCI root bus */
> +	pbus = to_pci_dev(dev)->bus;
> +	while (!pci_is_root_bus(pbus))
> +		pbus = pbus->parent;
> +
> +	return iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
> +			      iort_match_node_callback, &pbus->dev);
> +}
> +
> +void __init iort_table_detect(void)
> +{
> +	acpi_status status;
> +
> +	if (acpi_disabled)
> +		return;

Nit: if you are here acpi_disabled == false, so this check is
pointless.

> +	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);
> +	}

We really need to know if relying on iort_table to stay mapped
is a valid approach, it seems to me that the answer is yes
given that that's how eg DMAR is handled on x86 but we need
an acknowledgment from Rafael on this.

Other than that and my minor comments above (inclusive of Hanjun's
bug fix for iort_match_node_callback(), I did not reiterate that comment
but we have to fix it) patch seems fine.

Lorenzo

> +}
> diff --git a/include/linux/iort.h b/include/linux/iort.h
> new file mode 100644
> index 0000000..cde6809
> --- /dev/null
> +++ b/include/linux/iort.h
> @@ -0,0 +1,30 @@
> +/*
> + * Copyright (C) 2016, Semihalf
> + *	Author: Tomasz Nowicki <tn@semihalf.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
> + * Place - Suite 330, Boston, MA 02111-1307 USA.
> + */
> +
> +#ifndef __IORT_H__
> +#define __IORT_H__
> +
> +#include <linux/acpi.h>
> +
> +#ifdef CONFIG_IORT_TABLE
> +void iort_table_detect(void);
> +#else
> +static inline void iort_table_detect(void) { }
> +#endif
> +
> +#endif /* __IORT_H__ */
> -- 
> 1.9.1
> 

Re: [PATCH V7 1/8] ACPI: I/O Remapping Table (IORT) initial support

[Lorenzo Pieralisi]

Hi Rafael,

On Mon, Jun 20, 2016 at 01:02:14PM +0200, Tomasz Nowicki wrote:
> IORT shows representation of IO topology for ARM based systems.
> It describes how various components are connected together on
> parent-child basis e.g. PCI RC -> SMMU -> ITS. Also see IORT spec.
> 
> Initial support allows to detect IORT table presence and save its
> root pointer obtained through acpi_get_table(). The pointer validity
> depends on acpi_gbl_permanent_mmap because if acpi_gbl_permanent_mmap
> is not set while using IORT nodes we would dereference unmapped pointers.
> 
> For the aforementioned reason call iort_table_detect() from acpi_init()
> which guarantees acpi_gbl_permanent_mmap to be set at that point.

This moot point has become a blocker for this patchset and we need
your feedback please. I noticed that x86 code (drivers/iommu/dmar.c)
makes the same assumption (ie a table pointer, dmar_tbl, retrieved
through acpi_get_table_with_size() can be stashed in a global variable
and reused in functions like dmar_acpi_dev_scope_init() and
dmar_ir_support() without going through the ACPI API to retrieve
it again.

Is this a valid assumption ? This patch relies on it, since
otherwise it would become quite unwieldy (and unnecessary) to
create a cache of IORT nodes entries, so if we can retrieve the
table pointer through acpi_get_table_size() in iort_table_detect()
and stash it for further use that would make things much simpler
(as they are in the current patch).

I think that's the whole point behind acpi_gbl_permanent_mmap,
if that's set the corresponding tables pointers can be considered
permanently mapped, we really need to know if that's a reliable
assumption to make progress.

Thanks,
Lorenzo

> Add generic helpers which are helpful for scanning and retrieving
> information from IORT table content. List of the most important helpers:
> - iort_find_dev_node() finds IORT node for a given device
> - iort_node_map_rid() maps device RID and returns IORT node which provides
>   final translation
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
>  drivers/acpi/Kconfig  |   3 +
>  drivers/acpi/Makefile |   1 +
>  drivers/acpi/bus.c    |   2 +
>  drivers/acpi/iort.c   | 220 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/iort.h  |  30 +++++++
>  5 files changed, 256 insertions(+)
>  create mode 100644 drivers/acpi/iort.c
>  create mode 100644 include/linux/iort.h
> 
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index b7e2e77..848471f 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
>  config ACPI_CCA_REQUIRED
>  	bool
>  
> +config IORT_TABLE
> +	bool
> +
>  config ACPI_DEBUGGER
>  	bool "AML debugger interface"
>  	select ACPI_DEBUG
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 251ce85..c7c9b29 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
>  obj-$(CONFIG_ACPI_BGRT)		+= bgrt.o
>  obj-$(CONFIG_ACPI_CPPC_LIB)	+= cppc_acpi.o
>  obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
> +obj-$(CONFIG_IORT_TABLE) 	+= iort.o
>  
>  # processor has its own "processor." module_param namespace
>  processor-y			:= processor_driver.o
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 31e8da6..176c17d 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -33,6 +33,7 @@
>  #ifdef CONFIG_X86
>  #include <asm/mpspec.h>
>  #endif
> +#include <linux/iort.h>
>  #include <linux/pci.h>
>  #include <acpi/apei.h>
>  #include <linux/dmi.h>
> @@ -1118,6 +1119,7 @@ static int __init acpi_init(void)
>  	}
>  
>  	pci_mmcfg_late_init();
> +	iort_table_detect();
>  	acpi_scan_init();
>  	acpi_ec_init();
>  	acpi_debugfs_init();
> diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
> new file mode 100644
> index 0000000..e342bac
> --- /dev/null
> +++ b/drivers/acpi/iort.c
> @@ -0,0 +1,220 @@
> +/*
> + * Copyright (C) 2016, Semihalf
> + *	Author: Tomasz Nowicki <tn@semihalf.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * This file implements early detection/parsing of I/O mapping
> + * reported to OS through firmware via I/O Remapping Table (IORT)
> + * IORT document number: ARM DEN 0049A
> + */
> +
> +#define pr_fmt(fmt)	"ACPI: IORT: " fmt
> +
> +#include <linux/iort.h>
> +#include <linux/kernel.h>
> +#include <linux/pci.h>
> +
> +typedef acpi_status (*iort_find_node_callback)
> +	(struct acpi_iort_node *node, void *context);
> +
> +/* Root pointer to the mapped IORT table */
> +static struct acpi_table_header *iort_table;
> +
> +static struct acpi_iort_node *
> +iort_scan_node(enum acpi_iort_node_type type,
> +	       iort_find_node_callback callback, void *context)
> +{
> +	struct acpi_iort_node *iort_node, *iort_end;
> +	struct acpi_table_iort *iort;
> +	int i;
> +
> +	/* Get the first IORT node */
> +	iort = (struct acpi_table_iort *)iort_table;
> +	iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort,
> +				 iort->node_offset);
> +	iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> +				iort_table->length);
> +
> +	for (i = 0; i < iort->node_count; i++) {
> +		if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND,
> +			       "IORT node pointer overflows, bad table!\n"))
> +			return NULL;
> +
> +		if (iort_node->type == type) {
> +			if (ACPI_SUCCESS(callback(iort_node, context)))
> +				return iort_node;
> +		}
> +
> +		iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
> +					 iort_node->length);
> +	}
> +
> +	return NULL;
> +}
> +
> +static acpi_status
> +iort_match_node_callback(struct acpi_iort_node *node, void *context)
> +{
> +	struct device *dev = context;
> +
> +	switch (node->type) {
> +	case ACPI_IORT_NODE_NAMED_COMPONENT: {
> +		struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +		struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
> +		struct acpi_iort_named_component *ncomp;
> +
> +		if (!adev)
> +			break;
> +
> +		ncomp = (struct acpi_iort_named_component *)node->node_data;
> +
> +		if (ACPI_FAILURE(acpi_get_name(adev->handle,
> +					       ACPI_FULL_PATHNAME, &buffer))) {
> +			dev_warn(dev, "Can't get device full path name\n");
> +			break;
> +		}
> +
> +		if (!strcmp(ncomp->device_name, (char *)buffer.pointer))
> +			return AE_OK;
> +
> +		break;
> +	}
> +	case ACPI_IORT_NODE_PCI_ROOT_COMPLEX: {
> +		struct acpi_iort_root_complex *pci_rc;
> +		struct pci_bus *bus;
> +
> +		bus = to_pci_bus(dev);
> +		pci_rc = (struct acpi_iort_root_complex *)node->node_data;
> +
> +		/*
> +		 * It is assumed that PCI segment numbers maps one-to-one
> +		 * with root complexes. Each segment number can represent only
> +		 * one root complex.
> +		 */
> +		if (pci_rc->pci_segment_number == pci_domain_nr(bus))
> +			return AE_OK;
> +
> +		break;
> +	}
> +	}
> +
> +	return AE_NOT_FOUND;
> +}
> +
> +static int
> +iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in, u32 *rid_out)
> +{
> +	if (!rid_out)
> +		return -EINVAL;
> +
> +	/* Single mapping does not care for input id */
> +	if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
> +		if (type == ACPI_IORT_NODE_NAMED_COMPONENT ||
> +		    type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
> +			*rid_out = map->output_base;
> +			return 0;
> +		}
> +
> +		pr_warn(FW_BUG "[map %p] SINGLE MAPPING flag not allowed for node type %d, skipping ID map\n",
> +			map, type);
> +		return -ENXIO;
> +	}
> +
> +	if (rid_in < map->input_base ||
> +	    (rid_in > map->input_base + map->id_count))
> +		return -ENXIO;
> +
> +	*rid_out = map->output_base + (rid_in - map->input_base);
> +	return 0;
> +}
> +
> +static struct acpi_iort_node *
> +iort_node_map_rid(struct acpi_iort_node *node, u32 rid_in,
> +		  u32 *rid_out, u8 type)
> +{
> +	u32 rid = rid_in;
> +	int found = 0;
> +
> +	/* Climb up ID mapping tree to find specified node type */
> +	while (node && node->type != type) {
> +		struct acpi_iort_id_mapping *map;
> +		int i;
> +
> +		if (!node->mapping_offset || !node->mapping_count) {
> +			node = NULL;
> +			break;
> +		}
> +
> +		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);
> +			node = NULL;
> +			break;
> +		}
> +
> +		/* Do the RID translation */
> +		for (i = 0; i < node->mapping_count; i++, map++) {
> +			if(!iort_id_map(map, node->type, rid, &rid)) {
> +				found = 1;
> +				break;
> +			}
> +		}
> +
> +		if (!found) {
> +			node = NULL;
> +			break;
> +		}
> +
> +		node = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> +				    map->output_reference);
> +	}
> +
> +	/* Take input RID as output RID in case of any error */
> +	if (rid_out)
> +		*rid_out = node ? rid : rid_in;
> +	return node;
> +}
> +
> +static struct acpi_iort_node *
> +iort_find_dev_node(struct device *dev)
> +{
> +	struct pci_bus *pbus;
> +
> +	if (!dev_is_pci(dev))
> +		return iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
> +				      iort_match_node_callback, dev);
> +
> +	/* Find a PCI root bus */
> +	pbus = to_pci_dev(dev)->bus;
> +	while (!pci_is_root_bus(pbus))
> +		pbus = pbus->parent;
> +
> +	return iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
> +			      iort_match_node_callback, &pbus->dev);
> +}
> +
> +void __init iort_table_detect(void)
> +{
> +	acpi_status status;
> +
> +	if (acpi_disabled)
> +		return;
> +
> +	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);
> +	}
> +}
> diff --git a/include/linux/iort.h b/include/linux/iort.h
> new file mode 100644
> index 0000000..cde6809
> --- /dev/null
> +++ b/include/linux/iort.h
> @@ -0,0 +1,30 @@
> +/*
> + * Copyright (C) 2016, Semihalf
> + *	Author: Tomasz Nowicki <tn@semihalf.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
> + * Place - Suite 330, Boston, MA 02111-1307 USA.
> + */
> +
> +#ifndef __IORT_H__
> +#define __IORT_H__
> +
> +#include <linux/acpi.h>
> +
> +#ifdef CONFIG_IORT_TABLE
> +void iort_table_detect(void);
> +#else
> +static inline void iort_table_detect(void) { }
> +#endif
> +
> +#endif /* __IORT_H__ */
> -- 
> 1.9.1
> 

[PATCH V7 1/8] ACPI: I/O Remapping Table (IORT) initial support

[Tomasz Nowicki]

IORT shows representation of IO topology for ARM based systems.
It describes how various components are connected together on
parent-child basis e.g. PCI RC -> SMMU -> ITS. Also see IORT spec.
http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf

Initial support allows to detect IORT table presence and save its
root pointer obtained through acpi_get_table(). The pointer validity
depends on acpi_gbl_permanent_mmap because if acpi_gbl_permanent_mmap
is not set while using IORT nodes we would dereference unmapped pointers.

For the aforementioned reason call iort_table_detect() from acpi_init()
which guarantees acpi_gbl_permanent_mmap to be set at that point.

Add generic helpers which are helpful for scanning and retrieving
information from IORT table content. List of the most important helpers:
- iort_find_dev_node() finds IORT node for a given device
- iort_node_map_rid() maps device RID and returns IORT node which provides
  final translation

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
 drivers/acpi/Kconfig  |   3 +
 drivers/acpi/Makefile |   1 +
 drivers/acpi/bus.c    |   2 +
 drivers/acpi/iort.c   | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/iort.h  |  30 +++++++
 5 files changed, 253 insertions(+)
 create mode 100644 drivers/acpi/iort.c
 create mode 100644 include/linux/iort.h

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index b7e2e77..848471f 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
 config ACPI_CCA_REQUIRED
 	bool
 
+config IORT_TABLE
+	bool
+
 config ACPI_DEBUGGER
 	bool "AML debugger interface"
 	select ACPI_DEBUG
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 251ce85..c7c9b29 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
 obj-$(CONFIG_ACPI_BGRT)		+= bgrt.o
 obj-$(CONFIG_ACPI_CPPC_LIB)	+= cppc_acpi.o
 obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
+obj-$(CONFIG_IORT_TABLE) 	+= iort.o
 
 # processor has its own "processor." module_param namespace
 processor-y			:= processor_driver.o
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 31e8da6..176c17d 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -33,6 +33,7 @@
 #ifdef CONFIG_X86
 #include <asm/mpspec.h>
 #endif
+#include <linux/iort.h>
 #include <linux/pci.h>
 #include <acpi/apei.h>
 #include <linux/dmi.h>
@@ -1118,6 +1119,7 @@ static int __init acpi_init(void)
 	}
 
 	pci_mmcfg_late_init();
+	iort_table_detect();
 	acpi_scan_init();
 	acpi_ec_init();
 	acpi_debugfs_init();
diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
new file mode 100644
index 0000000..fcfa008f
--- /dev/null
+++ b/drivers/acpi/iort.c
@@ -0,0 +1,217 @@
+/*
+ * Copyright (C) 2016, Semihalf
+ *	Author: Tomasz Nowicki <tn@semihalf.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * This file implements early detection/parsing of I/O mapping
+ * reported to OS through firmware via I/O Remapping Table (IORT)
+ * IORT document number: ARM DEN 0049A
+ */
+
+#define pr_fmt(fmt)	"ACPI: IORT: " fmt
+
+#include <linux/iort.h>
+#include <linux/kernel.h>
+#include <linux/pci.h>
+
+typedef acpi_status (*iort_find_node_callback)
+	(struct acpi_iort_node *node, void *context);
+
+/* Root pointer to the mapped IORT table */
+static struct acpi_table_header *iort_table;
+
+static struct acpi_iort_node *
+iort_scan_node(enum acpi_iort_node_type type,
+	       iort_find_node_callback callback, void *context)
+{
+	struct acpi_iort_node *iort_node, *iort_end;
+	struct acpi_table_iort *iort;
+	int i;
+
+	/* Get the first IORT node */
+	iort = (struct acpi_table_iort *)iort_table;
+	iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort,
+				 iort->node_offset);
+	iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
+				iort_table->length);
+
+	for (i = 0; i < iort->node_count; i++) {
+		if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND,
+			       "IORT node pointer overflows, bad table!\n"))
+			return NULL;
+
+		if (iort_node->type == type) {
+			if (ACPI_SUCCESS(callback(iort_node, context)))
+				return iort_node;
+		}
+
+		iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
+					 iort_node->length);
+	}
+
+	return NULL;
+}
+
+static acpi_status
+iort_match_node_callback(struct acpi_iort_node *node, void *context)
+{
+	struct device *dev = context;
+
+	switch (node->type) {
+	case ACPI_IORT_NODE_NAMED_COMPONENT: {
+		struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+		struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
+		struct acpi_iort_named_component *ncomp;
+
+		if (!adev)
+			break;
+
+		ncomp = (struct acpi_iort_named_component *)node->node_data;
+
+		if (ACPI_FAILURE(acpi_get_name(adev->handle,
+					       ACPI_FULL_PATHNAME, &buffer))) {
+			dev_warn(dev, "Can't get device full path name\n");
+		} else {
+			int match;
+
+			match = !strcmp(ncomp->device_name, buffer.pointer);
+			kfree(buffer.pointer);
+
+			if (match)
+				return AE_OK;
+		}
+
+		break;
+	}
+	case ACPI_IORT_NODE_PCI_ROOT_COMPLEX: {
+		struct acpi_iort_root_complex *pci_rc;
+		struct pci_bus *bus;
+
+		bus = to_pci_bus(dev);
+		pci_rc = (struct acpi_iort_root_complex *)node->node_data;
+
+		/*
+		 * It is assumed that PCI segment numbers maps one-to-one
+		 * with root complexes. Each segment number can represent only
+		 * one root complex.
+		 */
+		if (pci_rc->pci_segment_number == pci_domain_nr(bus))
+			return AE_OK;
+
+		break;
+	}
+	}
+
+	return AE_NOT_FOUND;
+}
+
+static int
+iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in, u32 *rid_out)
+{
+	/* Single mapping does not care for input id */
+	if (map->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
+		if (type == ACPI_IORT_NODE_NAMED_COMPONENT ||
+		    type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
+			*rid_out = map->output_base;
+			return 0;
+		}
+
+		pr_warn(FW_BUG "[map %p] SINGLE MAPPING flag not allowed for node type %d, skipping ID map\n",
+			map, type);
+		return -ENXIO;
+	}
+
+	if (rid_in < map->input_base ||
+	    (rid_in > map->input_base + map->id_count))
+		return -ENXIO;
+
+	*rid_out = map->output_base + (rid_in - map->input_base);
+	return 0;
+}
+
+static struct acpi_iort_node *
+iort_node_map_rid(struct acpi_iort_node *node, u32 rid_in,
+		  u32 *rid_out, u8 type)
+{
+	u32 rid = rid_in;
+
+	/* Parse the ID mapping tree to find specified node type */
+	while (node) {
+		struct acpi_iort_id_mapping *map;
+		int i;
+
+		if (node->type == type) {
+			if (rid_out)
+				*rid_out = rid;
+			return node;
+		}
+
+		if (!node->mapping_offset || !node->mapping_count)
+			goto fail_map;
+
+		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);
+			goto fail_map;
+		}
+
+		/* Do the RID translation */
+		for (i = 0; i < node->mapping_count; i++, map++) {
+			if(!iort_id_map(map, node->type, rid, &rid))
+				break;
+		}
+
+		if (i == node->mapping_count)
+			goto fail_map;
+
+		node = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
+				    map->output_reference);
+	}
+
+fail_map:
+	/* Map input RID to output RID unchanged on mapping failure*/
+	if (rid_out)
+		*rid_out = rid_in;
+	return NULL;
+}
+
+static struct acpi_iort_node *
+iort_find_dev_node(struct device *dev)
+{
+	struct pci_bus *pbus;
+
+	if (!dev_is_pci(dev))
+		return iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
+				      iort_match_node_callback, dev);
+
+	/* Find a PCI root bus */
+	pbus = to_pci_dev(dev)->bus;
+	while (!pci_is_root_bus(pbus))
+		pbus = pbus->parent;
+
+	return iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
+			      iort_match_node_callback, &pbus->dev);
+}
+
+void __init iort_table_detect(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);
+	}
+}
diff --git a/include/linux/iort.h b/include/linux/iort.h
new file mode 100644
index 0000000..cde6809
--- /dev/null
+++ b/include/linux/iort.h
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2016, Semihalf
+ *	Author: Tomasz Nowicki <tn@semihalf.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+ */
+
+#ifndef __IORT_H__
+#define __IORT_H__
+
+#include <linux/acpi.h>
+
+#ifdef CONFIG_IORT_TABLE
+void iort_table_detect(void);
+#else
+static inline void iort_table_detect(void) { }
+#endif
+
+#endif /* __IORT_H__ */
-- 
1.9.1

Re: [PATCH V7 1/8] ACPI: I/O Remapping Table (IORT) initial support

[Tomasz Nowicki]

On 22.06.2016 14:35, Tomasz Nowicki wrote:
> IORT shows representation of IO topology for ARM based systems.
> It describes how various components are connected together on
> parent-child basis e.g. PCI RC -> SMMU -> ITS. Also see IORT spec.
> http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf
>
> Initial support allows to detect IORT table presence and save its
> root pointer obtained through acpi_get_table(). The pointer validity
> depends on acpi_gbl_permanent_mmap because if acpi_gbl_permanent_mmap
> is not set while using IORT nodes we would dereference unmapped pointers.
>
> For the aforementioned reason call iort_table_detect() from acpi_init()
> which guarantees acpi_gbl_permanent_mmap to be set at that point.
>
> Add generic helpers which are helpful for scanning and retrieving
> information from IORT table content. List of the most important helpers:
> - iort_find_dev_node() finds IORT node for a given device
> - iort_node_map_rid() maps device RID and returns IORT node which provides
>    final translation
>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>

The updated patch incorporates fixes and addresses Lorenzo's comments.
The whole series is here:
https://github.com/semihalf-nowicki-tomasz/linux.git (its-acpi-v7.1)

Thanks,
Tomasz

Re: [PATCH V7 1/8] ACPI: I/O Remapping Table (IORT) initial support

[Marc Zyngier]

On 22/06/16 13:35, Tomasz Nowicki wrote:
> IORT shows representation of IO topology for ARM based systems.
> It describes how various components are connected together on
> parent-child basis e.g. PCI RC -> SMMU -> ITS. Also see IORT spec.
> http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf
> 
> Initial support allows to detect IORT table presence and save its
> root pointer obtained through acpi_get_table(). The pointer validity
> depends on acpi_gbl_permanent_mmap because if acpi_gbl_permanent_mmap
> is not set while using IORT nodes we would dereference unmapped pointers.
> 
> For the aforementioned reason call iort_table_detect() from acpi_init()
> which guarantees acpi_gbl_permanent_mmap to be set at that point.
> 
> Add generic helpers which are helpful for scanning and retrieving
> information from IORT table content. List of the most important helpers:
> - iort_find_dev_node() finds IORT node for a given device
> - iort_node_map_rid() maps device RID and returns IORT node which provides
>   final translation
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
>  drivers/acpi/Kconfig  |   3 +
>  drivers/acpi/Makefile |   1 +
>  drivers/acpi/bus.c    |   2 +
>  drivers/acpi/iort.c   | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/iort.h  |  30 +++++++
>  5 files changed, 253 insertions(+)
>  create mode 100644 drivers/acpi/iort.c
>  create mode 100644 include/linux/iort.h
> 
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index b7e2e77..848471f 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
>  config ACPI_CCA_REQUIRED
>  	bool
>  
> +config IORT_TABLE
> +	bool
> +
>  config ACPI_DEBUGGER
>  	bool "AML debugger interface"
>  	select ACPI_DEBUG
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 251ce85..c7c9b29 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
>  obj-$(CONFIG_ACPI_BGRT)		+= bgrt.o
>  obj-$(CONFIG_ACPI_CPPC_LIB)	+= cppc_acpi.o
>  obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
> +obj-$(CONFIG_IORT_TABLE) 	+= iort.o
>  
>  # processor has its own "processor." module_param namespace
>  processor-y			:= processor_driver.o
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 31e8da6..176c17d 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -33,6 +33,7 @@
>  #ifdef CONFIG_X86
>  #include <asm/mpspec.h>
>  #endif
> +#include <linux/iort.h>
>  #include <linux/pci.h>
>  #include <acpi/apei.h>
>  #include <linux/dmi.h>
> @@ -1118,6 +1119,7 @@ static int __init acpi_init(void)
>  	}
>  
>  	pci_mmcfg_late_init();
> +	iort_table_detect();
>  	acpi_scan_init();
>  	acpi_ec_init();
>  	acpi_debugfs_init();
> diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
> new file mode 100644
> index 0000000..fcfa008f
> --- /dev/null
> +++ b/drivers/acpi/iort.c
> @@ -0,0 +1,217 @@
> +/*
> + * Copyright (C) 2016, Semihalf
> + *	Author: Tomasz Nowicki <tn@semihalf.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * This file implements early detection/parsing of I/O mapping
> + * reported to OS through firmware via I/O Remapping Table (IORT)
> + * IORT document number: ARM DEN 0049A
> + */
> +
> +#define pr_fmt(fmt)	"ACPI: IORT: " fmt
> +
> +#include <linux/iort.h>
> +#include <linux/kernel.h>
> +#include <linux/pci.h>
> +
> +typedef acpi_status (*iort_find_node_callback)
> +	(struct acpi_iort_node *node, void *context);
> +
> +/* Root pointer to the mapped IORT table */
> +static struct acpi_table_header *iort_table;
> +
> +static struct acpi_iort_node *
> +iort_scan_node(enum acpi_iort_node_type type,
> +	       iort_find_node_callback callback, void *context)
> +{
> +	struct acpi_iort_node *iort_node, *iort_end;
> +	struct acpi_table_iort *iort;
> +	int i;
> +
> +	/* Get the first IORT node */
> +	iort = (struct acpi_table_iort *)iort_table;
> +	iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort,
> +				 iort->node_offset);
> +	iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> +				iort_table->length);
> +
> +	for (i = 0; i < iort->node_count; i++) {
> +		if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND,
> +			       "IORT node pointer overflows, bad table!\n"))
> +			return NULL;
> +
> +		if (iort_node->type == type) {
> +			if (ACPI_SUCCESS(callback(iort_node, context)))
> +				return iort_node;
> +		}
> +
> +		iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
> +					 iort_node->length);
> +	}
> +
> +	return NULL;
> +}
> +
> +static acpi_status
> +iort_match_node_callback(struct acpi_iort_node *node, void *context)
> +{
> +	struct device *dev = context;
> +
> +	switch (node->type) {
> +	case ACPI_IORT_NODE_NAMED_COMPONENT: {
> +		struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +		struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
> +		struct acpi_iort_named_component *ncomp;
> +
> +		if (!adev)
> +			break;
> +
> +		ncomp = (struct acpi_iort_named_component *)node->node_data;
> +
> +		if (ACPI_FAILURE(acpi_get_name(adev->handle,
> +					       ACPI_FULL_PATHNAME, &buffer))) {
> +			dev_warn(dev, "Can't get device full path name\n");
> +		} else {
> +			int match;
> +
> +			match = !strcmp(ncomp->device_name, buffer.pointer);
> +			kfree(buffer.pointer);

Why did you change this to a naked kfree? The ACPI code clearly states:

/*
 * Allocate a new buffer. We directectly call acpi_os_allocate here to
 * purposefully bypass the (optionally enabled) internal allocation
 * tracking mechanism since we only want to track internal
 * allocations. Note: The caller should use acpi_os_free to free this
 * buffer created via ACPI_ALLOCATE_BUFFER.
 */

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH V7 1/8] ACPI: I/O Remapping Table (IORT) initial support

[Tomasz Nowicki]

On 22.06.2016 15:25, Marc Zyngier wrote:
> On 22/06/16 13:35, Tomasz Nowicki wrote:
>> IORT shows representation of IO topology for ARM based systems.
>> It describes how various components are connected together on
>> parent-child basis e.g. PCI RC -> SMMU -> ITS. Also see IORT spec.
>> http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf
>>
>> Initial support allows to detect IORT table presence and save its
>> root pointer obtained through acpi_get_table(). The pointer validity
>> depends on acpi_gbl_permanent_mmap because if acpi_gbl_permanent_mmap
>> is not set while using IORT nodes we would dereference unmapped pointers.
>>
>> For the aforementioned reason call iort_table_detect() from acpi_init()
>> which guarantees acpi_gbl_permanent_mmap to be set at that point.
>>
>> Add generic helpers which are helpful for scanning and retrieving
>> information from IORT table content. List of the most important helpers:
>> - iort_find_dev_node() finds IORT node for a given device
>> - iort_node_map_rid() maps device RID and returns IORT node which provides
>>    final translation
>>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> ---
>>   drivers/acpi/Kconfig  |   3 +
>>   drivers/acpi/Makefile |   1 +
>>   drivers/acpi/bus.c    |   2 +
>>   drivers/acpi/iort.c   | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/iort.h  |  30 +++++++
>>   5 files changed, 253 insertions(+)
>>   create mode 100644 drivers/acpi/iort.c
>>   create mode 100644 include/linux/iort.h
>>
>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> index b7e2e77..848471f 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
>>   config ACPI_CCA_REQUIRED
>>   	bool
>>
>> +config IORT_TABLE
>> +	bool
>> +
>>   config ACPI_DEBUGGER
>>   	bool "AML debugger interface"
>>   	select ACPI_DEBUG
>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>> index 251ce85..c7c9b29 100644
>> --- a/drivers/acpi/Makefile
>> +++ b/drivers/acpi/Makefile
>> @@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
>>   obj-$(CONFIG_ACPI_BGRT)		+= bgrt.o
>>   obj-$(CONFIG_ACPI_CPPC_LIB)	+= cppc_acpi.o
>>   obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
>> +obj-$(CONFIG_IORT_TABLE) 	+= iort.o
>>
>>   # processor has its own "processor." module_param namespace
>>   processor-y			:= processor_driver.o
>> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
>> index 31e8da6..176c17d 100644
>> --- a/drivers/acpi/bus.c
>> +++ b/drivers/acpi/bus.c
>> @@ -33,6 +33,7 @@
>>   #ifdef CONFIG_X86
>>   #include <asm/mpspec.h>
>>   #endif
>> +#include <linux/iort.h>
>>   #include <linux/pci.h>
>>   #include <acpi/apei.h>
>>   #include <linux/dmi.h>
>> @@ -1118,6 +1119,7 @@ static int __init acpi_init(void)
>>   	}
>>
>>   	pci_mmcfg_late_init();
>> +	iort_table_detect();
>>   	acpi_scan_init();
>>   	acpi_ec_init();
>>   	acpi_debugfs_init();
>> diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
>> new file mode 100644
>> index 0000000..fcfa008f
>> --- /dev/null
>> +++ b/drivers/acpi/iort.c
>> @@ -0,0 +1,217 @@
>> +/*
>> + * Copyright (C) 2016, Semihalf
>> + *	Author: Tomasz Nowicki <tn@semihalf.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + *
>> + * This file implements early detection/parsing of I/O mapping
>> + * reported to OS through firmware via I/O Remapping Table (IORT)
>> + * IORT document number: ARM DEN 0049A
>> + */
>> +
>> +#define pr_fmt(fmt)	"ACPI: IORT: " fmt
>> +
>> +#include <linux/iort.h>
>> +#include <linux/kernel.h>
>> +#include <linux/pci.h>
>> +
>> +typedef acpi_status (*iort_find_node_callback)
>> +	(struct acpi_iort_node *node, void *context);
>> +
>> +/* Root pointer to the mapped IORT table */
>> +static struct acpi_table_header *iort_table;
>> +
>> +static struct acpi_iort_node *
>> +iort_scan_node(enum acpi_iort_node_type type,
>> +	       iort_find_node_callback callback, void *context)
>> +{
>> +	struct acpi_iort_node *iort_node, *iort_end;
>> +	struct acpi_table_iort *iort;
>> +	int i;
>> +
>> +	/* Get the first IORT node */
>> +	iort = (struct acpi_table_iort *)iort_table;
>> +	iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort,
>> +				 iort->node_offset);
>> +	iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
>> +				iort_table->length);
>> +
>> +	for (i = 0; i < iort->node_count; i++) {
>> +		if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND,
>> +			       "IORT node pointer overflows, bad table!\n"))
>> +			return NULL;
>> +
>> +		if (iort_node->type == type) {
>> +			if (ACPI_SUCCESS(callback(iort_node, context)))
>> +				return iort_node;
>> +		}
>> +
>> +		iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
>> +					 iort_node->length);
>> +	}
>> +
>> +	return NULL;
>> +}
>> +
>> +static acpi_status
>> +iort_match_node_callback(struct acpi_iort_node *node, void *context)
>> +{
>> +	struct device *dev = context;
>> +
>> +	switch (node->type) {
>> +	case ACPI_IORT_NODE_NAMED_COMPONENT: {
>> +		struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>> +		struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
>> +		struct acpi_iort_named_component *ncomp;
>> +
>> +		if (!adev)
>> +			break;
>> +
>> +		ncomp = (struct acpi_iort_named_component *)node->node_data;
>> +
>> +		if (ACPI_FAILURE(acpi_get_name(adev->handle,
>> +					       ACPI_FULL_PATHNAME, &buffer))) {
>> +			dev_warn(dev, "Can't get device full path name\n");
>> +		} else {
>> +			int match;
>> +
>> +			match = !strcmp(ncomp->device_name, buffer.pointer);
>> +			kfree(buffer.pointer);
>
> Why did you change this to a naked kfree? The ACPI code clearly states:
>
> /*
>   * Allocate a new buffer. We directectly call acpi_os_allocate here to
>   * purposefully bypass the (optionally enabled) internal allocation
>   * tracking mechanism since we only want to track internal
>   * allocations. Note: The caller should use acpi_os_free to free this
>   * buffer created via ACPI_ALLOCATE_BUFFER.
>   */

Yes we should use symmetric free function to acpi_os_allocate here, 
which means acpi_os_free should be used here.

My main motivation was to free buffer.pointer instead of &buffer

Would you mind to fix that (s/kfree/acpi_os_free) before merging ?

Thanks,
Tomasz

Re: [PATCH V7 1/8] ACPI: I/O Remapping Table (IORT) initial support

[Marc Zyngier]

On 22/06/16 14:52, Tomasz Nowicki wrote:
> On 22.06.2016 15:25, Marc Zyngier wrote:
>> On 22/06/16 13:35, Tomasz Nowicki wrote:
>>> IORT shows representation of IO topology for ARM based systems.
>>> It describes how various components are connected together on
>>> parent-child basis e.g. PCI RC -> SMMU -> ITS. Also see IORT spec.
>>> http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf
>>>
>>> Initial support allows to detect IORT table presence and save its
>>> root pointer obtained through acpi_get_table(). The pointer validity
>>> depends on acpi_gbl_permanent_mmap because if acpi_gbl_permanent_mmap
>>> is not set while using IORT nodes we would dereference unmapped pointers.
>>>
>>> For the aforementioned reason call iort_table_detect() from acpi_init()
>>> which guarantees acpi_gbl_permanent_mmap to be set at that point.
>>>
>>> Add generic helpers which are helpful for scanning and retrieving
>>> information from IORT table content. List of the most important helpers:
>>> - iort_find_dev_node() finds IORT node for a given device
>>> - iort_node_map_rid() maps device RID and returns IORT node which provides
>>>    final translation
>>>
>>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>>> ---
>>>   drivers/acpi/Kconfig  |   3 +
>>>   drivers/acpi/Makefile |   1 +
>>>   drivers/acpi/bus.c    |   2 +
>>>   drivers/acpi/iort.c   | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   include/linux/iort.h  |  30 +++++++
>>>   5 files changed, 253 insertions(+)
>>>   create mode 100644 drivers/acpi/iort.c
>>>   create mode 100644 include/linux/iort.h
>>>
>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>> index b7e2e77..848471f 100644
>>> --- a/drivers/acpi/Kconfig
>>> +++ b/drivers/acpi/Kconfig
>>> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
>>>   config ACPI_CCA_REQUIRED
>>>   	bool
>>>
>>> +config IORT_TABLE
>>> +	bool
>>> +
>>>   config ACPI_DEBUGGER
>>>   	bool "AML debugger interface"
>>>   	select ACPI_DEBUG
>>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>>> index 251ce85..c7c9b29 100644
>>> --- a/drivers/acpi/Makefile
>>> +++ b/drivers/acpi/Makefile
>>> @@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
>>>   obj-$(CONFIG_ACPI_BGRT)		+= bgrt.o
>>>   obj-$(CONFIG_ACPI_CPPC_LIB)	+= cppc_acpi.o
>>>   obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
>>> +obj-$(CONFIG_IORT_TABLE) 	+= iort.o
>>>
>>>   # processor has its own "processor." module_param namespace
>>>   processor-y			:= processor_driver.o
>>> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
>>> index 31e8da6..176c17d 100644
>>> --- a/drivers/acpi/bus.c
>>> +++ b/drivers/acpi/bus.c
>>> @@ -33,6 +33,7 @@
>>>   #ifdef CONFIG_X86
>>>   #include <asm/mpspec.h>
>>>   #endif
>>> +#include <linux/iort.h>
>>>   #include <linux/pci.h>
>>>   #include <acpi/apei.h>
>>>   #include <linux/dmi.h>
>>> @@ -1118,6 +1119,7 @@ static int __init acpi_init(void)
>>>   	}
>>>
>>>   	pci_mmcfg_late_init();
>>> +	iort_table_detect();
>>>   	acpi_scan_init();
>>>   	acpi_ec_init();
>>>   	acpi_debugfs_init();
>>> diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
>>> new file mode 100644
>>> index 0000000..fcfa008f
>>> --- /dev/null
>>> +++ b/drivers/acpi/iort.c
>>> @@ -0,0 +1,217 @@
>>> +/*
>>> + * Copyright (C) 2016, Semihalf
>>> + *	Author: Tomasz Nowicki <tn@semihalf.com>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms and conditions of the GNU General Public License,
>>> + * version 2, as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope it will be useful, but WITHOUT
>>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>>> + * more details.
>>> + *
>>> + * This file implements early detection/parsing of I/O mapping
>>> + * reported to OS through firmware via I/O Remapping Table (IORT)
>>> + * IORT document number: ARM DEN 0049A
>>> + */
>>> +
>>> +#define pr_fmt(fmt)	"ACPI: IORT: " fmt
>>> +
>>> +#include <linux/iort.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/pci.h>
>>> +
>>> +typedef acpi_status (*iort_find_node_callback)
>>> +	(struct acpi_iort_node *node, void *context);
>>> +
>>> +/* Root pointer to the mapped IORT table */
>>> +static struct acpi_table_header *iort_table;
>>> +
>>> +static struct acpi_iort_node *
>>> +iort_scan_node(enum acpi_iort_node_type type,
>>> +	       iort_find_node_callback callback, void *context)
>>> +{
>>> +	struct acpi_iort_node *iort_node, *iort_end;
>>> +	struct acpi_table_iort *iort;
>>> +	int i;
>>> +
>>> +	/* Get the first IORT node */
>>> +	iort = (struct acpi_table_iort *)iort_table;
>>> +	iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort,
>>> +				 iort->node_offset);
>>> +	iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
>>> +				iort_table->length);
>>> +
>>> +	for (i = 0; i < iort->node_count; i++) {
>>> +		if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND,
>>> +			       "IORT node pointer overflows, bad table!\n"))
>>> +			return NULL;
>>> +
>>> +		if (iort_node->type == type) {
>>> +			if (ACPI_SUCCESS(callback(iort_node, context)))
>>> +				return iort_node;
>>> +		}
>>> +
>>> +		iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
>>> +					 iort_node->length);
>>> +	}
>>> +
>>> +	return NULL;
>>> +}
>>> +
>>> +static acpi_status
>>> +iort_match_node_callback(struct acpi_iort_node *node, void *context)
>>> +{
>>> +	struct device *dev = context;
>>> +
>>> +	switch (node->type) {
>>> +	case ACPI_IORT_NODE_NAMED_COMPONENT: {
>>> +		struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>>> +		struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
>>> +		struct acpi_iort_named_component *ncomp;
>>> +
>>> +		if (!adev)
>>> +			break;
>>> +
>>> +		ncomp = (struct acpi_iort_named_component *)node->node_data;
>>> +
>>> +		if (ACPI_FAILURE(acpi_get_name(adev->handle,
>>> +					       ACPI_FULL_PATHNAME, &buffer))) {
>>> +			dev_warn(dev, "Can't get device full path name\n");
>>> +		} else {
>>> +			int match;
>>> +
>>> +			match = !strcmp(ncomp->device_name, buffer.pointer);
>>> +			kfree(buffer.pointer);
>>
>> Why did you change this to a naked kfree? The ACPI code clearly states:
>>
>> /*
>>   * Allocate a new buffer. We directectly call acpi_os_allocate here to
>>   * purposefully bypass the (optionally enabled) internal allocation
>>   * tracking mechanism since we only want to track internal
>>   * allocations. Note: The caller should use acpi_os_free to free this
>>   * buffer created via ACPI_ALLOCATE_BUFFER.
>>   */
> 
> Yes we should use symmetric free function to acpi_os_allocate here, 
> which means acpi_os_free should be used here.
> 
> My main motivation was to free buffer.pointer instead of &buffer
> 
> Would you mind to fix that (s/kfree/acpi_os_free) before merging ?

Sure.

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH V7 1/8] ACPI: I/O Remapping Table (IORT) initial support

[Hanjun Guo]

On 2016/6/22 22:51, Marc Zyngier wrote:
> On 22/06/16 14:52, Tomasz Nowicki wrote:
>> On 22.06.2016 15:25, Marc Zyngier wrote:
>>> On 22/06/16 13:35, Tomasz Nowicki wrote:
>>>> IORT shows representation of IO topology for ARM based systems.
>>>> It describes how various components are connected together on
>>>> parent-child basis e.g. PCI RC -> SMMU -> ITS. Also see IORT spec.
>>>> http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf
>>>>
>>>> Initial support allows to detect IORT table presence and save its
>>>> root pointer obtained through acpi_get_table(). The pointer validity
>>>> depends on acpi_gbl_permanent_mmap because if acpi_gbl_permanent_mmap
>>>> is not set while using IORT nodes we would dereference unmapped pointers.
>>>>
>>>> For the aforementioned reason call iort_table_detect() from acpi_init()
>>>> which guarantees acpi_gbl_permanent_mmap to be set at that point.
>>>>
>>>> Add generic helpers which are helpful for scanning and retrieving
>>>> information from IORT table content. List of the most important helpers:
>>>> - iort_find_dev_node() finds IORT node for a given device
>>>> - iort_node_map_rid() maps device RID and returns IORT node which provides
>>>>    final translation
>>>>
>>>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>>>> ---
>>>>   drivers/acpi/Kconfig  |   3 +
>>>>   drivers/acpi/Makefile |   1 +
>>>>   drivers/acpi/bus.c    |   2 +
>>>>   drivers/acpi/iort.c   | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>   include/linux/iort.h  |  30 +++++++
>>>>   5 files changed, 253 insertions(+)
>>>>   create mode 100644 drivers/acpi/iort.c
>>>>   create mode 100644 include/linux/iort.h
>>>>
>>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>>> index b7e2e77..848471f 100644
>>>> --- a/drivers/acpi/Kconfig
>>>> +++ b/drivers/acpi/Kconfig
>>>> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
>>>>   config ACPI_CCA_REQUIRED
>>>>   	bool
>>>>
>>>> +config IORT_TABLE
>>>> +	bool
>>>> +
>>>>   config ACPI_DEBUGGER
>>>>   	bool "AML debugger interface"
>>>>   	select ACPI_DEBUG
>>>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>>>> index 251ce85..c7c9b29 100644
>>>> --- a/drivers/acpi/Makefile
>>>> +++ b/drivers/acpi/Makefile
>>>> @@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
>>>>   obj-$(CONFIG_ACPI_BGRT)		+= bgrt.o
>>>>   obj-$(CONFIG_ACPI_CPPC_LIB)	+= cppc_acpi.o
>>>>   obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
>>>> +obj-$(CONFIG_IORT_TABLE) 	+= iort.o
>>>>
>>>>   # processor has its own "processor." module_param namespace
>>>>   processor-y			:= processor_driver.o
>>>> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
>>>> index 31e8da6..176c17d 100644
>>>> --- a/drivers/acpi/bus.c
>>>> +++ b/drivers/acpi/bus.c
>>>> @@ -33,6 +33,7 @@
>>>>   #ifdef CONFIG_X86
>>>>   #include <asm/mpspec.h>
>>>>   #endif
>>>> +#include <linux/iort.h>
>>>>   #include <linux/pci.h>
>>>>   #include <acpi/apei.h>
>>>>   #include <linux/dmi.h>
>>>> @@ -1118,6 +1119,7 @@ static int __init acpi_init(void)
>>>>   	}
>>>>
>>>>   	pci_mmcfg_late_init();
>>>> +	iort_table_detect();
>>>>   	acpi_scan_init();
>>>>   	acpi_ec_init();
>>>>   	acpi_debugfs_init();
>>>> diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
>>>> new file mode 100644
>>>> index 0000000..fcfa008f
>>>> --- /dev/null
>>>> +++ b/drivers/acpi/iort.c
>>>> @@ -0,0 +1,217 @@
>>>> +/*
>>>> + * Copyright (C) 2016, Semihalf
>>>> + *	Author: Tomasz Nowicki <tn@semihalf.com>
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or modify it
>>>> + * under the terms and conditions of the GNU General Public License,
>>>> + * version 2, as published by the Free Software Foundation.
>>>> + *
>>>> + * This program is distributed in the hope it will be useful, but WITHOUT
>>>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>>>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>>>> + * more details.
>>>> + *
>>>> + * This file implements early detection/parsing of I/O mapping
>>>> + * reported to OS through firmware via I/O Remapping Table (IORT)
>>>> + * IORT document number: ARM DEN 0049A
>>>> + */
>>>> +
>>>> +#define pr_fmt(fmt)	"ACPI: IORT: " fmt
>>>> +
>>>> +#include <linux/iort.h>
>>>> +#include <linux/kernel.h>
>>>> +#include <linux/pci.h>
>>>> +
>>>> +typedef acpi_status (*iort_find_node_callback)
>>>> +	(struct acpi_iort_node *node, void *context);
>>>> +
>>>> +/* Root pointer to the mapped IORT table */
>>>> +static struct acpi_table_header *iort_table;
>>>> +
>>>> +static struct acpi_iort_node *
>>>> +iort_scan_node(enum acpi_iort_node_type type,
>>>> +	       iort_find_node_callback callback, void *context)
>>>> +{
>>>> +	struct acpi_iort_node *iort_node, *iort_end;
>>>> +	struct acpi_table_iort *iort;
>>>> +	int i;
>>>> +
>>>> +	/* Get the first IORT node */
>>>> +	iort = (struct acpi_table_iort *)iort_table;
>>>> +	iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort,
>>>> +				 iort->node_offset);
>>>> +	iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
>>>> +				iort_table->length);
>>>> +
>>>> +	for (i = 0; i < iort->node_count; i++) {
>>>> +		if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND,
>>>> +			       "IORT node pointer overflows, bad table!\n"))
>>>> +			return NULL;
>>>> +
>>>> +		if (iort_node->type == type) {
>>>> +			if (ACPI_SUCCESS(callback(iort_node, context)))
>>>> +				return iort_node;
>>>> +		}
>>>> +
>>>> +		iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
>>>> +					 iort_node->length);
>>>> +	}
>>>> +
>>>> +	return NULL;
>>>> +}
>>>> +
>>>> +static acpi_status
>>>> +iort_match_node_callback(struct acpi_iort_node *node, void *context)
>>>> +{
>>>> +	struct device *dev = context;
>>>> +
>>>> +	switch (node->type) {
>>>> +	case ACPI_IORT_NODE_NAMED_COMPONENT: {
>>>> +		struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>>>> +		struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
>>>> +		struct acpi_iort_named_component *ncomp;
>>>> +
>>>> +		if (!adev)
>>>> +			break;
>>>> +
>>>> +		ncomp = (struct acpi_iort_named_component *)node->node_data;
>>>> +
>>>> +		if (ACPI_FAILURE(acpi_get_name(adev->handle,
>>>> +					       ACPI_FULL_PATHNAME, &buffer))) {
>>>> +			dev_warn(dev, "Can't get device full path name\n");
>>>> +		} else {
>>>> +			int match;
>>>> +
>>>> +			match = !strcmp(ncomp->device_name, buffer.pointer);
>>>> +			kfree(buffer.pointer);
>>> Why did you change this to a naked kfree? The ACPI code clearly states:
>>>
>>> /*
>>>   * Allocate a new buffer. We directectly call acpi_os_allocate here to
>>>   * purposefully bypass the (optionally enabled) internal allocation
>>>   * tracking mechanism since we only want to track internal
>>>   * allocations. Note: The caller should use acpi_os_free to free this
>>>   * buffer created via ACPI_ALLOCATE_BUFFER.
>>>   */
>> Yes we should use symmetric free function to acpi_os_allocate here, 
>> which means acpi_os_free should be used here.
>>
>> My main motivation was to free buffer.pointer instead of &buffer

It needs to be free buffer.pointer.

>>
>> Would you mind to fix that (s/kfree/acpi_os_free) before merging ?
> Sure.

I tracked Marc's tree on git.kernel.org, it's  acpi_os_free(buffer.pointer) there so
my worry is gone :)

Thanks
Hanjun

Re: [PATCH V7 1/8] ACPI: I/O Remapping Table (IORT) initial support

[Christopher Covington]

Hi Marc,

On 06/22/2016 09:34 PM, Hanjun Guo wrote:
> On 2016/6/22 22:51, Marc Zyngier wrote:
>> On 22/06/16 14:52, Tomasz Nowicki wrote:
>>> On 22.06.2016 15:25, Marc Zyngier wrote:
>>>> On 22/06/16 13:35, Tomasz Nowicki wrote:
>>>>> IORT shows representation of IO topology for ARM based systems.
>>>>> It describes how various components are connected together on
>>>>> parent-child basis e.g. PCI RC -> SMMU -> ITS. Also see IORT spec.
>>>>> http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf
>>>>>
>>>>> Initial support allows to detect IORT table presence and save its
>>>>> root pointer obtained through acpi_get_table(). The pointer validity
>>>>> depends on acpi_gbl_permanent_mmap because if acpi_gbl_permanent_mmap
>>>>> is not set while using IORT nodes we would dereference unmapped pointers.
>>>>>
>>>>> For the aforementioned reason call iort_table_detect() from acpi_init()
>>>>> which guarantees acpi_gbl_permanent_mmap to be set at that point.
>>>>>
>>>>> Add generic helpers which are helpful for scanning and retrieving
>>>>> information from IORT table content. List of the most important helpers:
>>>>> - iort_find_dev_node() finds IORT node for a given device
>>>>> - iort_node_map_rid() maps device RID and returns IORT node which provides
>>>>>    final translation
>>>>>
>>>>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>>>>> ---
>>>>>   drivers/acpi/Kconfig  |   3 +
>>>>>   drivers/acpi/Makefile |   1 +
>>>>>   drivers/acpi/bus.c    |   2 +
>>>>>   drivers/acpi/iort.c   | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>   include/linux/iort.h  |  30 +++++++
>>>>>   5 files changed, 253 insertions(+)
>>>>>   create mode 100644 drivers/acpi/iort.c
>>>>>   create mode 100644 include/linux/iort.h

> I tracked Marc's tree on git.kernel.org, it's  acpi_os_free(buffer.pointer) there so
> my worry is gone :)

Do you plan on submitting this during the 4.8 merge window?

Thanks,
Cov

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code
Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH V7 1/8] ACPI: I/O Remapping Table (IORT) initial support

[Marc Zyngier]

On Tue, 26 Jul 2016 09:19:15 -0400
Christopher Covington <cov@codeaurora.org> wrote:

Hi Christopher,

> Hi Marc,
> 
> On 06/22/2016 09:34 PM, Hanjun Guo wrote:
> > On 2016/6/22 22:51, Marc Zyngier wrote:  
> >> On 22/06/16 14:52, Tomasz Nowicki wrote:  
> >>> On 22.06.2016 15:25, Marc Zyngier wrote:  
> >>>> On 22/06/16 13:35, Tomasz Nowicki wrote:  
> >>>>> IORT shows representation of IO topology for ARM based systems.
> >>>>> It describes how various components are connected together on
> >>>>> parent-child basis e.g. PCI RC -> SMMU -> ITS. Also see IORT spec.
> >>>>> http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf
> >>>>>
> >>>>> Initial support allows to detect IORT table presence and save its
> >>>>> root pointer obtained through acpi_get_table(). The pointer validity
> >>>>> depends on acpi_gbl_permanent_mmap because if acpi_gbl_permanent_mmap
> >>>>> is not set while using IORT nodes we would dereference unmapped pointers.
> >>>>>
> >>>>> For the aforementioned reason call iort_table_detect() from acpi_init()
> >>>>> which guarantees acpi_gbl_permanent_mmap to be set at that point.
> >>>>>
> >>>>> Add generic helpers which are helpful for scanning and retrieving
> >>>>> information from IORT table content. List of the most important helpers:
> >>>>> - iort_find_dev_node() finds IORT node for a given device
> >>>>> - iort_node_map_rid() maps device RID and returns IORT node which provides
> >>>>>    final translation
> >>>>>
> >>>>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> >>>>> ---
> >>>>>   drivers/acpi/Kconfig  |   3 +
> >>>>>   drivers/acpi/Makefile |   1 +
> >>>>>   drivers/acpi/bus.c    |   2 +
> >>>>>   drivers/acpi/iort.c   | 217 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>>>   include/linux/iort.h  |  30 +++++++
> >>>>>   5 files changed, 253 insertions(+)
> >>>>>   create mode 100644 drivers/acpi/iort.c
> >>>>>   create mode 100644 include/linux/iort.h  
> 
> > I tracked Marc's tree on git.kernel.org, it's  acpi_os_free(buffer.pointer) there so
> > my worry is gone :)  
> 
> Do you plan on submitting this during the 4.8 merge window?

Short of having received the necessary Ack from the ACPI maintainer,
the answer is unfortunately negative. I'm hoping that the new split
ACPI maintenance between generic code and arm64 will help getting
things moving for 4.9.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.

[PATCH V7 2/8] ACPI: Add new IORT functions to support MSI domain handling

[Tomasz Nowicki]

For ITS, MSI functionality consists on building domain stack and
during that process we need to reference to domain stack components
e.g. before we create new DOMAIN_BUS_PCI_MSI domain we need to specify
its DOMAIN_BUS_NEXUS parent domain. In order to manage that process
properly, maintain list which elements contain domain token
(unique for MSI domain stack) and ITS ID: iort_register_domain_token()
and iort_deregister_domain_token(). Then retrieve domain token
any time later with ITS ID being key off: iort_find_domain_token().
With domain token and domain type we are able to find corresponding
IRQ domain.

Since IORT is prepared to describe MSI domain on a per-device basis,
use existing IORT helpers and implement two calls:
1. iort_msi_map_rid() to map MSI RID for a device
2. iort_get_device_domain() to find domain token for a device

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
 drivers/acpi/iort.c  | 169 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/iort.h |  11 ++++
 2 files changed, 180 insertions(+)

diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
index e342bac..496dcf6 100644
--- a/drivers/acpi/iort.c
+++ b/drivers/acpi/iort.c
@@ -22,12 +22,91 @@
 #include <linux/kernel.h>
 #include <linux/pci.h>
 
+struct iort_its_msi_chip {
+	struct list_head	list;
+	struct fwnode_handle	*fw_node;
+	u32			translation_id;
+};
+
 typedef acpi_status (*iort_find_node_callback)
 	(struct acpi_iort_node *node, void *context);
 
 /* Root pointer to the mapped IORT table */
 static struct acpi_table_header *iort_table;
 
+static LIST_HEAD(iort_msi_chip_list);
+static DEFINE_SPINLOCK(iort_msi_chip_lock);
+
+/**
+ * iort_register_domain_token() - register domain token and related ITS ID
+ * to the list from where we can get it back later on.
+ * @trans_id: ITS ID.
+ * @fw_node: Domain token.
+ *
+ * Returns: 0 on success, -ENOMEM if no memory when allocating list element
+ */
+int iort_register_domain_token(int trans_id, struct fwnode_handle *fw_node)
+{
+	struct iort_its_msi_chip *its_msi_chip;
+
+	its_msi_chip = kzalloc(sizeof(*its_msi_chip), GFP_KERNEL);
+	if (!its_msi_chip)
+		return -ENOMEM;
+
+	its_msi_chip->fw_node = fw_node;
+	its_msi_chip->translation_id = trans_id;
+
+	spin_lock(&iort_msi_chip_lock);
+	list_add(&its_msi_chip->list, &iort_msi_chip_list);
+	spin_unlock(&iort_msi_chip_lock);
+
+	return 0;
+}
+
+/**
+ * iort_deregister_domain_token() - Deregister domain token based on ITS ID
+ * @trans_id: ITS ID.
+ *
+ * Returns: none.
+ */
+void iort_deregister_domain_token(int trans_id)
+{
+	struct iort_its_msi_chip *its_msi_chip, *t;
+
+	spin_lock(&iort_msi_chip_lock);
+	list_for_each_entry_safe(its_msi_chip, t, &iort_msi_chip_list, list) {
+		if (its_msi_chip->translation_id == trans_id) {
+			list_del(&its_msi_chip->list);
+			kfree(its_msi_chip);
+			break;
+		}
+	}
+	spin_unlock(&iort_msi_chip_lock);
+}
+
+/**
+ * iort_find_domain_token() - Find domain token based on given ITS ID
+ * @trans_id: ITS ID.
+ *
+ * Returns: domain token when find on the list, NULL otherwise
+ */
+struct fwnode_handle *iort_find_domain_token(int trans_id)
+{
+	struct fwnode_handle *fw_node = NULL;
+	struct iort_its_msi_chip *its_msi_chip;
+
+	spin_lock(&iort_msi_chip_lock);
+	list_for_each_entry(its_msi_chip, &iort_msi_chip_list, list) {
+		if (its_msi_chip->translation_id == trans_id) {
+			fw_node = its_msi_chip->fw_node;
+			break;
+		}
+	}
+	spin_unlock(&iort_msi_chip_lock);
+
+	return fw_node;
+}
+
 static struct acpi_iort_node *
 iort_scan_node(enum acpi_iort_node_type type,
 	       iort_find_node_callback callback, void *context)
@@ -205,6 +284,96 @@ iort_find_dev_node(struct device *dev)
 			      iort_match_node_callback, &pbus->dev);
 }
 
+/**
+ * iort_msi_map_rid() - Map a MSI requester ID for a device
+ * @dev: The device for which the mapping is to be done.
+ * @req_id: The device requester ID.
+ *
+ * Returns: mapped MSI RID on success, input requester ID otherwise
+ */
+u32 iort_msi_map_rid(struct device *dev, u32 req_id)
+{
+	struct acpi_iort_node *node;
+	u32 dev_id;
+
+	if (!iort_table)
+		return req_id;
+
+	node = iort_find_dev_node(dev);
+	if (!node) {
+		dev_err(dev, "can't find related IORT node\n");
+		return req_id;
+	}
+
+	iort_node_map_rid(node, req_id, &dev_id, ACPI_IORT_NODE_ITS_GROUP);
+	return dev_id;
+}
+
+/**
+ * iort_dev_find_its_id() - Find the ITS identifier for a device
+ * @dev: The device.
+ * @idx: Index of the ITS identifier list.
+ * @its_id: ITS identifier.
+ *
+ * Returns: 0 on success, appropriate error value otherwise
+ */
+static int
+iort_dev_find_its_id(struct device *dev, u32 req_id, unsigned int idx,
+		     int *its_id)
+{
+	struct acpi_iort_its_group *its;
+	struct acpi_iort_node *node;
+
+	node = iort_find_dev_node(dev);
+	if (!node) {
+		dev_err(dev, "can't find related IORT node\n");
+		return -ENXIO;
+	}
+
+	node = iort_node_map_rid(node, req_id, NULL, ACPI_IORT_NODE_ITS_GROUP);
+	if (!node) {
+		dev_err(dev, "can't find related ITS node\n");
+		return -ENXIO;
+	}
+
+	/* Move to ITS specific data */
+	its = (struct acpi_iort_its_group *)node->node_data;
+	if (idx > its->its_count) {
+		dev_err(dev, "requested ITS ID index [%d] is greater than available [%d]\n",
+			idx, its->its_count);
+		return -ENXIO;
+	}
+
+	*its_id = its->identifiers[idx];
+	return 0;
+}
+
+/**
+ * iort_get_device_domain() - Find MSI domain related to a device
+ * @dev: The device.
+ * @req_id: Requester ID for the device.
+ *
+ * Returns: the MSI domain for this device, NULL otherwise
+ */
+struct irq_domain *
+iort_get_device_domain(struct device *dev, u32 req_id)
+{
+	static struct fwnode_handle *handle;
+	int its_id;
+
+	if (!iort_table)
+		return NULL;
+
+	if (iort_dev_find_its_id(dev, req_id, 0, &its_id))
+		return NULL;
+
+	handle = iort_find_domain_token(its_id);
+	if (!handle)
+		return NULL;
+
+	return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
+}
+
 void __init iort_table_detect(void)
 {
 	acpi_status status;
diff --git a/include/linux/iort.h b/include/linux/iort.h
index cde6809..d7daba1 100644
--- a/include/linux/iort.h
+++ b/include/linux/iort.h
@@ -20,11 +20,22 @@
 #define __IORT_H__
 
 #include <linux/acpi.h>
+#include <linux/fwnode.h>
+#include <linux/irqdomain.h>
 
+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);
 #ifdef CONFIG_IORT_TABLE
 void iort_table_detect(void);
+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 iort_table_detect(void) { }
+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, u32 req_id) { return NULL; }
 #endif
 
 #endif /* __IORT_H__ */
-- 
1.9.1

Re: [PATCH V7 2/8] ACPI: Add new IORT functions to support MSI domain handling

[Hanjun Guo]

On 2016/6/20 19:02, Tomasz Nowicki wrote:
> For ITS, MSI functionality consists on building domain stack and
> during that process we need to reference to domain stack components
> e.g. before we create new DOMAIN_BUS_PCI_MSI domain we need to specify
> its DOMAIN_BUS_NEXUS parent domain. In order to manage that process
> properly, maintain list which elements contain domain token
> (unique for MSI domain stack) and ITS ID: iort_register_domain_token()
> and iort_deregister_domain_token(). Then retrieve domain token
> any time later with ITS ID being key off: iort_find_domain_token().
> With domain token and domain type we are able to find corresponding
> IRQ domain.
>
> Since IORT is prepared to describe MSI domain on a per-device basis,
> use existing IORT helpers and implement two calls:
> 1. iort_msi_map_rid() to map MSI RID for a device
> 2. iort_get_device_domain() to find domain token for a device
>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
>  drivers/acpi/iort.c  | 169 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/iort.h |  11 ++++
>  2 files changed, 180 insertions(+)
>
> diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
> index e342bac..496dcf6 100644
> --- a/drivers/acpi/iort.c
> +++ b/drivers/acpi/iort.c
> @@ -22,12 +22,91 @@
>  #include <linux/kernel.h>
>  #include <linux/pci.h>
>
> +struct iort_its_msi_chip {
> +	struct list_head	list;
> +	struct fwnode_handle	*fw_node;
> +	u32			translation_id;
> +};
> +
>  typedef acpi_status (*iort_find_node_callback)
>  	(struct acpi_iort_node *node, void *context);
>
>  /* Root pointer to the mapped IORT table */
>  static struct acpi_table_header *iort_table;
>
> +static LIST_HEAD(iort_msi_chip_list);
> +static DEFINE_SPINLOCK(iort_msi_chip_lock);
> +
> +/**
> + * iort_register_domain_token() - register domain token and related ITS ID
> + * to the list from where we can get it back later on.
> + * @trans_id: ITS ID.
> + * @fw_node: Domain token.
> + *
> + * Returns: 0 on success, -ENOMEM if no memory when allocating list element
> + */
> +int iort_register_domain_token(int trans_id, struct fwnode_handle *fw_node)
> +{
> +	struct iort_its_msi_chip *its_msi_chip;
> +
> +	its_msi_chip = kzalloc(sizeof(*its_msi_chip), GFP_KERNEL);
> +	if (!its_msi_chip)
> +		return -ENOMEM;
> +
> +	its_msi_chip->fw_node = fw_node;
> +	its_msi_chip->translation_id = trans_id;
> +
> +	spin_lock(&iort_msi_chip_lock);
> +	list_add(&its_msi_chip->list, &iort_msi_chip_list);
> +	spin_unlock(&iort_msi_chip_lock);
> +
> +	return 0;
> +}
> +
> +/**
> + * iort_deregister_domain_token() - Deregister domain token based on ITS ID
> + * @trans_id: ITS ID.
> + *
> + * Returns: none.
> + */
> +void iort_deregister_domain_token(int trans_id)
> +{
> +	struct iort_its_msi_chip *its_msi_chip, *t;
> +
> +	spin_lock(&iort_msi_chip_lock);
> +	list_for_each_entry_safe(its_msi_chip, t, &iort_msi_chip_list, list) {
> +		if (its_msi_chip->translation_id == trans_id) {
> +			list_del(&its_msi_chip->list);
> +			kfree(its_msi_chip);
> +			break;
> +		}
> +	}
> +	spin_unlock(&iort_msi_chip_lock);
> +}
> +
> +/**
> + * iort_find_domain_token() - Find domain token based on given ITS ID
> + * @trans_id: ITS ID.
> + *
> + * Returns: domain token when find on the list, NULL otherwise
> + */
> +struct fwnode_handle *iort_find_domain_token(int trans_id)
> +{
> +	struct fwnode_handle *fw_node = NULL;
> +	struct iort_its_msi_chip *its_msi_chip;
> +
> +	spin_lock(&iort_msi_chip_lock);
> +	list_for_each_entry(its_msi_chip, &iort_msi_chip_list, list) {
> +		if (its_msi_chip->translation_id == trans_id) {
> +			fw_node = its_msi_chip->fw_node;
> +			break;
> +		}
> +	}
> +	spin_unlock(&iort_msi_chip_lock);
> +
> +	return fw_node;
> +}
> +
>  static struct acpi_iort_node *
>  iort_scan_node(enum acpi_iort_node_type type,
>  	       iort_find_node_callback callback, void *context)
> @@ -205,6 +284,96 @@ iort_find_dev_node(struct device *dev)
>  			      iort_match_node_callback, &pbus->dev);
>  }
>
> +/**
> + * iort_msi_map_rid() - Map a MSI requester ID for a device
> + * @dev: The device for which the mapping is to be done.
> + * @req_id: The device requester ID.
> + *
> + * Returns: mapped MSI RID on success, input requester ID otherwise
> + */
> +u32 iort_msi_map_rid(struct device *dev, u32 req_id)
> +{
> +	struct acpi_iort_node *node;
> +	u32 dev_id;
> +
> +	if (!iort_table)
> +		return req_id;
> +
> +	node = iort_find_dev_node(dev);
> +	if (!node) {
> +		dev_err(dev, "can't find related IORT node\n");
> +		return req_id;
> +	}
> +
> +	iort_node_map_rid(node, req_id, &dev_id, ACPI_IORT_NODE_ITS_GROUP);
> +	return dev_id;
> +}
> +
> +/**
> + * iort_dev_find_its_id() - Find the ITS identifier for a device
> + * @dev: The device.
> + * @idx: Index of the ITS identifier list.
> + * @its_id: ITS identifier.
> + *
> + * Returns: 0 on success, appropriate error value otherwise
> + */
> +static int
> +iort_dev_find_its_id(struct device *dev, u32 req_id, unsigned int idx,
> +		     int *its_id)
> +{
> +	struct acpi_iort_its_group *its;
> +	struct acpi_iort_node *node;
> +
> +	node = iort_find_dev_node(dev);
> +	if (!node) {
> +		dev_err(dev, "can't find related IORT node\n");
> +		return -ENXIO;
> +	}
> +
> +	node = iort_node_map_rid(node, req_id, NULL, ACPI_IORT_NODE_ITS_GROUP);
> +	if (!node) {
> +		dev_err(dev, "can't find related ITS node\n");
> +		return -ENXIO;
> +	}
> +
> +	/* Move to ITS specific data */
> +	its = (struct acpi_iort_its_group *)node->node_data;
> +	if (idx > its->its_count) {
> +		dev_err(dev, "requested ITS ID index [%d] is greater than available [%d]\n",
> +			idx, its->its_count);
> +		return -ENXIO;
> +	}
> +
> +	*its_id = its->identifiers[idx];
> +	return 0;
> +}
> +
> +/**
> + * iort_get_device_domain() - Find MSI domain related to a device
> + * @dev: The device.
> + * @req_id: Requester ID for the device.
> + *
> + * Returns: the MSI domain for this device, NULL otherwise
> + */
> +struct irq_domain *
> +iort_get_device_domain(struct device *dev, u32 req_id)
> +{
> +	static struct fwnode_handle *handle;
> +	int its_id;
> +
> +	if (!iort_table)
> +		return NULL;
> +
> +	if (iort_dev_find_its_id(dev, req_id, 0, &its_id))
> +		return NULL;
> +
> +	handle = iort_find_domain_token(its_id);
> +	if (!handle)
> +		return NULL;
> +
> +	return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
> +}
> +
>  void __init iort_table_detect(void)
>  {
>  	acpi_status status;
> diff --git a/include/linux/iort.h b/include/linux/iort.h
> index cde6809..d7daba1 100644
> --- a/include/linux/iort.h
> +++ b/include/linux/iort.h
> @@ -20,11 +20,22 @@
>  #define __IORT_H__
>
>  #include <linux/acpi.h>
> +#include <linux/fwnode.h>
> +#include <linux/irqdomain.h>
>
> +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);
>  #ifdef CONFIG_IORT_TABLE
>  void iort_table_detect(void);
> +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 iort_table_detect(void) { }
> +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, u32 req_id) { return NULL; }
>  #endif

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

[PATCH V7 3/8] PCI/MSI: Setup MSI domain on a per-device basis using IORT ACPI table

[Tomasz Nowicki]

It is possible to provide information about which MSI controller to
use on a per-device basis for DT. This patch supply this with ACPI support.

Currently, IORT is the only one ACPI table which can provide such mapping.
In order to plug IORT into MSI infrastructure we are adding ACPI
equivalents for finding PCI device domain and its RID translation
(pci_msi_domain_get_msi_rid and pci_msi_domain_get_msi_rid calls).

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/pci/msi.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index a080f44..1d45e81 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -18,6 +18,7 @@
 #include <linux/smp.h>
 #include <linux/errno.h>
 #include <linux/io.h>
+#include <linux/iort.h>
 #include <linux/slab.h>
 #include <linux/irqdomain.h>
 #include <linux/of_irq.h>
@@ -1364,8 +1365,8 @@ u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev)
 	pci_for_each_dma_alias(pdev, get_msi_id_cb, &rid);
 
 	of_node = irq_domain_get_of_node(domain);
-	if (of_node)
-		rid = of_msi_map_rid(&pdev->dev, of_node, rid);
+	rid = of_node ? of_msi_map_rid(&pdev->dev, of_node, rid) :
+			iort_msi_map_rid(&pdev->dev, rid);
 
 	return rid;
 }
@@ -1381,9 +1382,13 @@ u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev)
  */
 struct irq_domain *pci_msi_get_device_domain(struct pci_dev *pdev)
 {
+	struct irq_domain *dom;
 	u32 rid = 0;
 
 	pci_for_each_dma_alias(pdev, get_msi_id_cb, &rid);
-	return of_msi_map_get_device_domain(&pdev->dev, rid);
+	dom = of_msi_map_get_device_domain(&pdev->dev, rid);
+	if (!dom)
+		dom = iort_get_device_domain(&pdev->dev, rid);
+	return dom;
 }
 #endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
-- 
1.9.1

Re: [PATCH V7 3/8] PCI/MSI: Setup MSI domain on a per-device basis using IORT ACPI table

[Hanjun Guo]

On 2016/6/20 19:02, Tomasz Nowicki wrote:
> It is possible to provide information about which MSI controller to
> use on a per-device basis for DT. This patch supply this with ACPI support.
>
> Currently, IORT is the only one ACPI table which can provide such mapping.
> In order to plug IORT into MSI infrastructure we are adding ACPI
> equivalents for finding PCI device domain and its RID translation
> (pci_msi_domain_get_msi_rid and pci_msi_domain_get_msi_rid calls).
>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/pci/msi.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index a080f44..1d45e81 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -18,6 +18,7 @@
>  #include <linux/smp.h>
>  #include <linux/errno.h>
>  #include <linux/io.h>
> +#include <linux/iort.h>
>  #include <linux/slab.h>
>  #include <linux/irqdomain.h>
>  #include <linux/of_irq.h>
> @@ -1364,8 +1365,8 @@ u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev)
>  	pci_for_each_dma_alias(pdev, get_msi_id_cb, &rid);
>
>  	of_node = irq_domain_get_of_node(domain);
> -	if (of_node)
> -		rid = of_msi_map_rid(&pdev->dev, of_node, rid);
> +	rid = of_node ? of_msi_map_rid(&pdev->dev, of_node, rid) :
> +			iort_msi_map_rid(&pdev->dev, rid);
>
>  	return rid;
>  }
> @@ -1381,9 +1382,13 @@ u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev)
>   */
>  struct irq_domain *pci_msi_get_device_domain(struct pci_dev *pdev)
>  {
> +	struct irq_domain *dom;
>  	u32 rid = 0;
>
>  	pci_for_each_dma_alias(pdev, get_msi_id_cb, &rid);
> -	return of_msi_map_get_device_domain(&pdev->dev, rid);
> +	dom = of_msi_map_get_device_domain(&pdev->dev, rid);
> +	if (!dom)
> +		dom = iort_get_device_domain(&pdev->dev, rid);
> +	return dom;
>  }
>  #endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

Re: [PATCH V7 3/8] PCI/MSI: Setup MSI domain on a per-device basis using IORT ACPI table

[Bjorn Helgaas]

On Mon, Jun 20, 2016 at 01:02:16PM +0200, Tomasz Nowicki wrote:
> It is possible to provide information about which MSI controller to
> use on a per-device basis for DT. This patch supply this with ACPI support.
> 
> Currently, IORT is the only one ACPI table which can provide such mapping.
> In order to plug IORT into MSI infrastructure we are adding ACPI
> equivalents for finding PCI device domain and its RID translation
> (pci_msi_domain_get_msi_rid and pci_msi_domain_get_msi_rid calls).
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

I assume this will be merged along with the rest of the series via
some non-PCI tree.

> ---
>  drivers/pci/msi.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index a080f44..1d45e81 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -18,6 +18,7 @@
>  #include <linux/smp.h>
>  #include <linux/errno.h>
>  #include <linux/io.h>
> +#include <linux/iort.h>
>  #include <linux/slab.h>
>  #include <linux/irqdomain.h>
>  #include <linux/of_irq.h>
> @@ -1364,8 +1365,8 @@ u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev)
>  	pci_for_each_dma_alias(pdev, get_msi_id_cb, &rid);
>  
>  	of_node = irq_domain_get_of_node(domain);
> -	if (of_node)
> -		rid = of_msi_map_rid(&pdev->dev, of_node, rid);
> +	rid = of_node ? of_msi_map_rid(&pdev->dev, of_node, rid) :
> +			iort_msi_map_rid(&pdev->dev, rid);
>  
>  	return rid;
>  }
> @@ -1381,9 +1382,13 @@ u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev)
>   */
>  struct irq_domain *pci_msi_get_device_domain(struct pci_dev *pdev)
>  {
> +	struct irq_domain *dom;
>  	u32 rid = 0;
>  
>  	pci_for_each_dma_alias(pdev, get_msi_id_cb, &rid);
> -	return of_msi_map_get_device_domain(&pdev->dev, rid);
> +	dom = of_msi_map_get_device_domain(&pdev->dev, rid);
> +	if (!dom)
> +		dom = iort_get_device_domain(&pdev->dev, rid);
> +	return dom;
>  }
>  #endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V7 4/8] irqchip/gicv3-its: Cleanup for ITS domain initialization

[Tomasz Nowicki]

There is no point to initialize ITS without having msi-controller
property in corresponding DT node. However, its_probe is checking
msi-controller presence at the end, so we can save our time and do that
check prior to its_probe call. Also, for the code clarity purpose,
we put domain initialization to separate function.

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 57 ++++++++++++++++++++++++----------------
 1 file changed, 34 insertions(+), 23 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 7ceaba8..bd43de1 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1609,13 +1609,37 @@ static void its_enable_quirks(struct its_node *its)
 	gic_enable_quirks(iidr, its_quirks, its);
 }
 
+static int its_init_domain(struct device_node *node, struct its_node *its,
+			   struct irq_domain *parent)
+{
+	struct irq_domain *inner_domain;
+	struct msi_domain_info *info;
+
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	inner_domain = irq_domain_add_tree(node, &its_domain_ops, its);
+	if (!inner_domain) {
+		kfree(info);
+		return -ENOMEM;
+	}
+
+	inner_domain->parent = parent;
+	inner_domain->bus_token = DOMAIN_BUS_NEXUS;
+	info->ops = &its_msi_domain_ops;
+	info->data = its;
+	inner_domain->host_data = info;
+
+	return 0;
+}
+
 static int __init its_probe(struct device_node *node,
 			    struct irq_domain *parent)
 {
 	struct resource res;
 	struct its_node *its;
 	void __iomem *its_base;
-	struct irq_domain *inner_domain;
 	u32 val;
 	u64 baser, tmp;
 	int err;
@@ -1707,28 +1731,9 @@ static int __init its_probe(struct device_node *node,
 	writeq_relaxed(0, its->base + GITS_CWRITER);
 	writel_relaxed(GITS_CTLR_ENABLE, its->base + GITS_CTLR);
 
-	if (of_property_read_bool(node, "msi-controller")) {
-		struct msi_domain_info *info;
-
-		info = kzalloc(sizeof(*info), GFP_KERNEL);
-		if (!info) {
-			err = -ENOMEM;
-			goto out_free_tables;
-		}
-
-		inner_domain = irq_domain_add_tree(node, &its_domain_ops, its);
-		if (!inner_domain) {
-			err = -ENOMEM;
-			kfree(info);
-			goto out_free_tables;
-		}
-
-		inner_domain->parent = parent;
-		inner_domain->bus_token = DOMAIN_BUS_NEXUS;
-		info->ops = &its_msi_domain_ops;
-		info->data = its;
-		inner_domain->host_data = info;
-	}
+	err = its_init_domain(node, its, parent);
+	if (err)
+		goto out_free_tables;
 
 	spin_lock(&its_lock);
 	list_add(&its->entry, &its_nodes);
@@ -1779,6 +1784,12 @@ int __init its_init(struct device_node *node, struct rdists *rdists,
 
 	for (np = of_find_matching_node(node, its_device_id); np;
 	     np = of_find_matching_node(np, its_device_id)) {
+		if (!of_property_read_bool(np, "msi-controller")) {
+			pr_warn("%s: no msi-controller property, ITS ignored\n",
+				np->full_name);
+			continue;
+		}
+
 		its_probe(np, parent_domain);
 	}
 
-- 
1.9.1

Re: [PATCH V7 4/8] irqchip/gicv3-its: Cleanup for ITS domain initialization

[Hanjun Guo]

On 2016/6/20 19:02, Tomasz Nowicki wrote:
> There is no point to initialize ITS without having msi-controller
> property in corresponding DT node. However, its_probe is checking
> msi-controller presence at the end, so we can save our time and do that
> check prior to its_probe call. Also, for the code clarity purpose,
> we put domain initialization to separate function.
>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 57 ++++++++++++++++++++++++----------------
>  1 file changed, 34 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 7ceaba8..bd43de1 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -1609,13 +1609,37 @@ static void its_enable_quirks(struct its_node *its)
>  	gic_enable_quirks(iidr, its_quirks, its);
>  }
>
> +static int its_init_domain(struct device_node *node, struct its_node *its,
> +			   struct irq_domain *parent)
> +{
> +	struct irq_domain *inner_domain;
> +	struct msi_domain_info *info;
> +
> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	inner_domain = irq_domain_add_tree(node, &its_domain_ops, its);
> +	if (!inner_domain) {
> +		kfree(info);
> +		return -ENOMEM;
> +	}
> +
> +	inner_domain->parent = parent;
> +	inner_domain->bus_token = DOMAIN_BUS_NEXUS;
> +	info->ops = &its_msi_domain_ops;
> +	info->data = its;
> +	inner_domain->host_data = info;
> +
> +	return 0;
> +}
> +
>  static int __init its_probe(struct device_node *node,
>  			    struct irq_domain *parent)
>  {
>  	struct resource res;
>  	struct its_node *its;
>  	void __iomem *its_base;
> -	struct irq_domain *inner_domain;
>  	u32 val;
>  	u64 baser, tmp;
>  	int err;
> @@ -1707,28 +1731,9 @@ static int __init its_probe(struct device_node *node,
>  	writeq_relaxed(0, its->base + GITS_CWRITER);
>  	writel_relaxed(GITS_CTLR_ENABLE, its->base + GITS_CTLR);
>
> -	if (of_property_read_bool(node, "msi-controller")) {
> -		struct msi_domain_info *info;
> -
> -		info = kzalloc(sizeof(*info), GFP_KERNEL);
> -		if (!info) {
> -			err = -ENOMEM;
> -			goto out_free_tables;
> -		}
> -
> -		inner_domain = irq_domain_add_tree(node, &its_domain_ops, its);
> -		if (!inner_domain) {
> -			err = -ENOMEM;
> -			kfree(info);
> -			goto out_free_tables;
> -		}
> -
> -		inner_domain->parent = parent;
> -		inner_domain->bus_token = DOMAIN_BUS_NEXUS;
> -		info->ops = &its_msi_domain_ops;
> -		info->data = its;
> -		inner_domain->host_data = info;
> -	}
> +	err = its_init_domain(node, its, parent);
> +	if (err)
> +		goto out_free_tables;
>
>  	spin_lock(&its_lock);
>  	list_add(&its->entry, &its_nodes);
> @@ -1779,6 +1784,12 @@ int __init its_init(struct device_node *node, struct rdists *rdists,
>
>  	for (np = of_find_matching_node(node, its_device_id); np;
>  	     np = of_find_matching_node(np, its_device_id)) {
> +		if (!of_property_read_bool(np, "msi-controller")) {
> +			pr_warn("%s: no msi-controller property, ITS ignored\n",
> +				np->full_name);
> +			continue;
> +		}
> +
>  		its_probe(np, parent_domain);
>  	}

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

[PATCH V7 5/8] irqchip/gicv3-its: Refactor ITS DT init code to prepare for ACPI

[Tomasz Nowicki]

In order to add ACPI support we need to isolate ACPI&DT common code and
move DT logic to corresponding functions. To achieve this we are using
firmware agnostic handle which can be unpacked to either DT or ACPI node.

No functional changes other than a very minor one:
1. Terminate its_init call with -ENODEV for non-DT case which allows
to remove hack from its-gic-v3.c.
2. Fix ITS base register address type (from 'unsigned long' to 'phys_addr_t'),
as a bonus we get nice string formatting.
3. Since there is only one of ITS parent domain convert it to static global
variable and drop the parameter from its_probe_one. Users can refer to it
in more convenient way then.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
 drivers/irqchip/irq-gic-v3-its.c   | 65 ++++++++++++++++++++++----------------
 drivers/irqchip/irq-gic-v3.c       |  7 ++--
 include/linux/irqchip/arm-gic-v3.h |  4 +--
 3 files changed, 42 insertions(+), 34 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index bd43de1..1e0888d 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -75,7 +75,7 @@ struct its_node {
 	raw_spinlock_t		lock;
 	struct list_head	entry;
 	void __iomem		*base;
-	unsigned long		phys_base;
+	phys_addr_t		phys_base;
 	struct its_cmd_block	*cmd_base;
 	struct its_cmd_block	*cmd_write;
 	struct its_baser	tables[GITS_BASER_NR_REGS];
@@ -115,6 +115,7 @@ struct its_device {
 static LIST_HEAD(its_nodes);
 static DEFINE_SPINLOCK(its_lock);
 static struct rdists *gic_rdists;
+static struct irq_domain *its_parent;
 
 #define gic_data_rdist()		(raw_cpu_ptr(gic_rdists->rdist))
 #define gic_data_rdist_rd_base()	(gic_data_rdist()->rd_base)
@@ -1609,8 +1610,7 @@ static void its_enable_quirks(struct its_node *its)
 	gic_enable_quirks(iidr, its_quirks, its);
 }
 
-static int its_init_domain(struct device_node *node, struct its_node *its,
-			   struct irq_domain *parent)
+static int its_init_domain(struct fwnode_handle *handle, struct its_node *its)
 {
 	struct irq_domain *inner_domain;
 	struct msi_domain_info *info;
@@ -1619,13 +1619,13 @@ static int its_init_domain(struct device_node *node, struct its_node *its,
 	if (!info)
 		return -ENOMEM;
 
-	inner_domain = irq_domain_add_tree(node, &its_domain_ops, its);
+	inner_domain = irq_domain_create_tree(handle, &its_domain_ops, its);
 	if (!inner_domain) {
 		kfree(info);
 		return -ENOMEM;
 	}
 
-	inner_domain->parent = parent;
+	inner_domain->parent = its_parent;
 	inner_domain->bus_token = DOMAIN_BUS_NEXUS;
 	info->ops = &its_msi_domain_ops;
 	info->data = its;
@@ -1634,43 +1634,35 @@ static int its_init_domain(struct device_node *node, struct its_node *its,
 	return 0;
 }
 
-static int __init its_probe(struct device_node *node,
-			    struct irq_domain *parent)
+static int __init its_probe_one(struct resource *res,
+				struct fwnode_handle *handle, int numa_node)
 {
-	struct resource res;
 	struct its_node *its;
 	void __iomem *its_base;
 	u32 val;
 	u64 baser, tmp;
 	int err;
 
-	err = of_address_to_resource(node, 0, &res);
-	if (err) {
-		pr_warn("%s: no regs?\n", node->full_name);
-		return -ENXIO;
-	}
-
-	its_base = ioremap(res.start, resource_size(&res));
+	its_base = ioremap(res->start, resource_size(res));
 	if (!its_base) {
-		pr_warn("%s: unable to map registers\n", node->full_name);
+		pr_warn("ITS@%pa: Unable to map ITS registers\n", &res->start);
 		return -ENOMEM;
 	}
 
 	val = readl_relaxed(its_base + GITS_PIDR2) & GIC_PIDR2_ARCH_MASK;
 	if (val != 0x30 && val != 0x40) {
-		pr_warn("%s: no ITS detected, giving up\n", node->full_name);
+		pr_warn("ITS@%pa: No ITS detected, giving up\n", &res->start);
 		err = -ENODEV;
 		goto out_unmap;
 	}
 
 	err = its_force_quiescent(its_base);
 	if (err) {
-		pr_warn("%s: failed to quiesce, giving up\n",
-			node->full_name);
+		pr_warn("ITS@%pa: Failed to quiesce, giving up\n", &res->start);
 		goto out_unmap;
 	}
 
-	pr_info("ITS: %s\n", node->full_name);
+	pr_info("ITS@%pa\n", &res->start);
 
 	its = kzalloc(sizeof(*its), GFP_KERNEL);
 	if (!its) {
@@ -1682,9 +1674,9 @@ static int __init its_probe(struct device_node *node,
 	INIT_LIST_HEAD(&its->entry);
 	INIT_LIST_HEAD(&its->its_device_list);
 	its->base = its_base;
-	its->phys_base = res.start;
+	its->phys_base = res->start;
 	its->ite_size = ((readl_relaxed(its_base + GITS_TYPER) >> 4) & 0xf) + 1;
-	its->numa_node = of_node_to_nid(node);
+	its->numa_node = numa_node;
 
 	its->cmd_base = kzalloc(ITS_CMD_QUEUE_SZ, GFP_KERNEL);
 	if (!its->cmd_base) {
@@ -1731,7 +1723,7 @@ static int __init its_probe(struct device_node *node,
 	writeq_relaxed(0, its->base + GITS_CWRITER);
 	writel_relaxed(GITS_CTLR_ENABLE, its->base + GITS_CTLR);
 
-	err = its_init_domain(node, its, parent);
+	err = its_init_domain(handle, its);
 	if (err)
 		goto out_free_tables;
 
@@ -1749,7 +1741,7 @@ out_free_its:
 	kfree(its);
 out_unmap:
 	iounmap(its_base);
-	pr_err("ITS: failed probing %s (%d)\n", node->full_name, err);
+	pr_err("ITS@%pa: failed probing (%d)\n", &res->start, err);
 	return err;
 }
 
@@ -1777,10 +1769,10 @@ static struct of_device_id its_device_id[] = {
 	{},
 };
 
-int __init its_init(struct device_node *node, struct rdists *rdists,
-	     struct irq_domain *parent_domain)
+static int __init its_of_probe(struct device_node *node)
 {
 	struct device_node *np;
+	struct resource res;
 
 	for (np = of_find_matching_node(node, its_device_id); np;
 	     np = of_find_matching_node(np, its_device_id)) {
@@ -1790,8 +1782,27 @@ int __init its_init(struct device_node *node, struct rdists *rdists,
 			continue;
 		}
 
-		its_probe(np, parent_domain);
+		if (of_address_to_resource(np, 0, &res)) {
+			pr_warn("%s: no regs?\n", np->full_name);
+			continue;
+		}
+
+		its_probe_one(&res, &np->fwnode, of_node_to_nid(np));
 	}
+	return 0;
+}
+
+int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
+		    struct irq_domain *parent_domain)
+{
+	struct device_node *of_node;
+
+	its_parent = parent_domain;
+	of_node = to_of_node(handle);
+	if (of_node)
+		its_of_probe(of_node);
+	else
+		return -ENODEV;
 
 	if (list_empty(&its_nodes)) {
 		pr_warn("ITS: No ITS available, not enabling LPIs\n");
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 2c5ba0e..2a8b2e1 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -911,7 +911,6 @@ static int __init gic_init_bases(void __iomem *dist_base,
 				 u64 redist_stride,
 				 struct fwnode_handle *handle)
 {
-	struct device_node *node;
 	u32 typer;
 	int gic_irqs;
 	int err;
@@ -952,10 +951,8 @@ static int __init gic_init_bases(void __iomem *dist_base,
 
 	set_handle_irq(gic_handle_irq);
 
-	node = to_of_node(handle);
-	if (IS_ENABLED(CONFIG_ARM_GIC_V3_ITS) && gic_dist_supports_lpis() &&
-	    node) /* Temp hack to prevent ITS init for ACPI */
-		its_init(node, &gic_data.rdists, gic_data.domain);
+	if (IS_ENABLED(CONFIG_ARM_GIC_V3_ITS) && gic_dist_supports_lpis())
+		its_init(handle, &gic_data.rdists, gic_data.domain);
 
 	gic_smp_init();
 	gic_dist_init();
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index 107eed4..a1ea4e2 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -338,9 +338,9 @@ struct rdists {
 };
 
 struct irq_domain;
-struct device_node;
+struct fwnode_handle;
 int its_cpu_init(void);
-int its_init(struct device_node *node, struct rdists *rdists,
+int its_init(struct fwnode_handle *handle, struct rdists *rdists,
 	     struct irq_domain *domain);
 
 static inline bool gic_enable_sre(void)
-- 
1.9.1

[PATCH V7 6/8] irqchip/gicv3-its: Probe ITS in the ACPI way

[Tomasz Nowicki]

ITS is prepared for being initialized different than DT,
therefore we can initialize it in ACPI way. We collect register base
address from MADT table and pass mandatory info to firmware-agnostic
ITS init call.

Use here IORT lib to register ITS domain which then can be found and
used on to build another PCI MSI domain in hierarchical stack domain.

NOTE: Waiting for proper ITS and NUMA node relation description in IORT
table, we pass around NUMA_NO_NODE to the its_probe_one init call.
This means that Cavium ThunderX erratum 23144 (pass1.1 only)
is not supported for ACPI boot method yet.

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/Kconfig          |  1 +
 drivers/irqchip/irq-gic-v3-its.c | 59 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 5495a5b..fd4b8b8 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -38,6 +38,7 @@ config ARM_GIC_V3
 config ARM_GIC_V3_ITS
 	bool
 	select PCI_MSI_IRQ_DOMAIN
+	select IORT_TABLE if ACPI
 
 config ARM_NVIC
 	bool
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 1e0888d..c5c8616 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -15,10 +15,13 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/acpi.h>
 #include <linux/bitmap.h>
 #include <linux/cpu.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
+#include <linux/irqdomain.h>
+#include <linux/iort.h>
 #include <linux/log2.h>
 #include <linux/mm.h>
 #include <linux/msi.h>
@@ -1438,6 +1441,11 @@ static int its_irq_gic_domain_alloc(struct irq_domain *domain,
 		fwspec.param[0] = GIC_IRQ_TYPE_LPI;
 		fwspec.param[1] = hwirq;
 		fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
+	} else if (is_fwnode_irqchip(domain->parent->fwnode)) {
+		fwspec.fwnode = domain->parent->fwnode;
+		fwspec.param_count = 2;
+		fwspec.param[0] = hwirq;
+		fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
 	} else {
 		return -EINVAL;
 	}
@@ -1792,6 +1800,55 @@ static int __init its_of_probe(struct device_node *node)
 	return 0;
 }
 
+#ifdef CONFIG_ACPI
+
+#define ACPI_GICV3_ITS_MEM_SIZE (SZ_128K)
+
+static int __init gic_acpi_parse_madt_its(struct acpi_subtable_header *header,
+					  const unsigned long end)
+{
+	struct acpi_madt_generic_translator *its_entry;
+	struct fwnode_handle *dom_handle;
+	struct resource res;
+	int err;
+
+	its_entry = (struct acpi_madt_generic_translator *)header;
+	res.start = its_entry->base_address;
+	res.end = its_entry->base_address + ACPI_GICV3_ITS_MEM_SIZE - 1;
+
+	dom_handle = irq_domain_alloc_fwnode((void *)its_entry->base_address);
+	if (!dom_handle) {
+		pr_err("ITS@%pa: Unable to allocate GICv3 ITS domain token\n",
+		       &res.start);
+		return -ENOMEM;
+	}
+
+	err = iort_register_domain_token(its_entry->translation_id, dom_handle);
+	if (err) {
+		pr_err("ITS@%pa: Unable to register GICv3 ITS domain token (ITS ID %d) to IORT\n",
+		       &res.start, its_entry->translation_id);
+		goto dom_err;
+	}
+
+	err = its_probe_one(&res, dom_handle, NUMA_NO_NODE);
+	if (!err)
+		return 0;
+
+	iort_deregister_domain_token(its_entry->translation_id);
+dom_err:
+	irq_domain_free_fwnode(dom_handle);
+	return err;
+}
+
+static void __init its_acpi_probe(void)
+{
+	acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
+			      gic_acpi_parse_madt_its, 0);
+}
+#else
+static void __init its_acpi_probe(void) { }
+#endif
+
 int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
 		    struct irq_domain *parent_domain)
 {
@@ -1802,7 +1859,7 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
 	if (of_node)
 		its_of_probe(of_node);
 	else
-		return -ENODEV;
+		its_acpi_probe();
 
 	if (list_empty(&its_nodes)) {
 		pr_warn("ITS: No ITS available, not enabling LPIs\n");
-- 
1.9.1

Re: [PATCH V7 6/8] irqchip/gicv3-its: Probe ITS in the ACPI way

[Hanjun Guo]

On 2016/6/20 19:02, Tomasz Nowicki wrote:
> ITS is prepared for being initialized different than DT,
> therefore we can initialize it in ACPI way. We collect register base
> address from MADT table and pass mandatory info to firmware-agnostic
> ITS init call.
>
> Use here IORT lib to register ITS domain which then can be found and
> used on to build another PCI MSI domain in hierarchical stack domain.
>
> NOTE: Waiting for proper ITS and NUMA node relation description in IORT
> table, we pass around NUMA_NO_NODE to the its_probe_one init call.
> This means that Cavium ThunderX erratum 23144 (pass1.1 only)
> is not supported for ACPI boot method yet.
>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/irqchip/Kconfig          |  1 +
>  drivers/irqchip/irq-gic-v3-its.c | 59 +++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 59 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 5495a5b..fd4b8b8 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -38,6 +38,7 @@ config ARM_GIC_V3
>  config ARM_GIC_V3_ITS
>  	bool
>  	select PCI_MSI_IRQ_DOMAIN
> +	select IORT_TABLE if ACPI
>
>  config ARM_NVIC
>  	bool
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 1e0888d..c5c8616 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -15,10 +15,13 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>
> +#include <linux/acpi.h>
>  #include <linux/bitmap.h>
>  #include <linux/cpu.h>
>  #include <linux/delay.h>
>  #include <linux/interrupt.h>
> +#include <linux/irqdomain.h>
> +#include <linux/iort.h>
>  #include <linux/log2.h>
>  #include <linux/mm.h>
>  #include <linux/msi.h>
> @@ -1438,6 +1441,11 @@ static int its_irq_gic_domain_alloc(struct irq_domain *domain,
>  		fwspec.param[0] = GIC_IRQ_TYPE_LPI;
>  		fwspec.param[1] = hwirq;
>  		fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
> +	} else if (is_fwnode_irqchip(domain->parent->fwnode)) {
> +		fwspec.fwnode = domain->parent->fwnode;
> +		fwspec.param_count = 2;
> +		fwspec.param[0] = hwirq;
> +		fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
>  	} else {
>  		return -EINVAL;
>  	}
> @@ -1792,6 +1800,55 @@ static int __init its_of_probe(struct device_node *node)
>  	return 0;
>  }
>
> +#ifdef CONFIG_ACPI
> +
> +#define ACPI_GICV3_ITS_MEM_SIZE (SZ_128K)
> +
> +static int __init gic_acpi_parse_madt_its(struct acpi_subtable_header *header,
> +					  const unsigned long end)
> +{
> +	struct acpi_madt_generic_translator *its_entry;
> +	struct fwnode_handle *dom_handle;
> +	struct resource res;
> +	int err;
> +
> +	its_entry = (struct acpi_madt_generic_translator *)header;
> +	res.start = its_entry->base_address;
> +	res.end = its_entry->base_address + ACPI_GICV3_ITS_MEM_SIZE - 1;
> +
> +	dom_handle = irq_domain_alloc_fwnode((void *)its_entry->base_address);
> +	if (!dom_handle) {
> +		pr_err("ITS@%pa: Unable to allocate GICv3 ITS domain token\n",
> +		       &res.start);
> +		return -ENOMEM;
> +	}
> +
> +	err = iort_register_domain_token(its_entry->translation_id, dom_handle);
> +	if (err) {
> +		pr_err("ITS@%pa: Unable to register GICv3 ITS domain token (ITS ID %d) to IORT\n",
> +		       &res.start, its_entry->translation_id);
> +		goto dom_err;
> +	}
> +
> +	err = its_probe_one(&res, dom_handle, NUMA_NO_NODE);
> +	if (!err)
> +		return 0;
> +
> +	iort_deregister_domain_token(its_entry->translation_id);
> +dom_err:
> +	irq_domain_free_fwnode(dom_handle);
> +	return err;
> +}
> +
> +static void __init its_acpi_probe(void)
> +{
> +	acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
> +			      gic_acpi_parse_madt_its, 0);
> +}
> +#else
> +static void __init its_acpi_probe(void) { }
> +#endif
> +
>  int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
>  		    struct irq_domain *parent_domain)
>  {
> @@ -1802,7 +1859,7 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
>  	if (of_node)
>  		its_of_probe(of_node);
>  	else
> -		return -ENODEV;
> +		its_acpi_probe();
>
>  	if (list_empty(&its_nodes)) {
>  		pr_warn("ITS: No ITS available, not enabling LPIs\n");
>

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

[PATCH V7 7/8] irqchip/gicv3-its: Factor out code that might be reused for ACPI

[Tomasz Nowicki]

Firmware agnostic code lands in common functions which do necessary
domain initialization based on unique domain handler. DT specific
code goes to DT specific init call.

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-v3-its-pci-msi.c | 44 +++++++++++++++++++++-----------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its-pci-msi.c b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
index aee60ed..d2c2496 100644
--- a/drivers/irqchip/irq-gic-v3-its-pci-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
@@ -106,34 +106,48 @@ static struct of_device_id its_device_id[] = {
 	{},
 };
 
-static int __init its_pci_msi_init(void)
+static int __init its_pci_msi_init_one(struct fwnode_handle *handle,
+				       const char *name)
 {
-	struct device_node *np;
 	struct irq_domain *parent;
 
+	parent = irq_find_matching_fwnode(handle, DOMAIN_BUS_NEXUS);
+	if (!parent || !msi_get_domain_info(parent)) {
+		pr_err("%s: Unable to locate ITS domain\n", name);
+		return -ENXIO;
+	}
+
+	if (!pci_msi_create_irq_domain(handle, &its_pci_msi_domain_info,
+				       parent)) {
+		pr_err("%s: Unable to create PCI domain\n", name);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static int __init its_pci_of_msi_init(void)
+{
+	struct device_node *np;
+
 	for (np = of_find_matching_node(NULL, its_device_id); np;
 	     np = of_find_matching_node(np, its_device_id)) {
 		if (!of_property_read_bool(np, "msi-controller"))
 			continue;
 
-		parent = irq_find_matching_host(np, DOMAIN_BUS_NEXUS);
-		if (!parent || !msi_get_domain_info(parent)) {
-			pr_err("%s: unable to locate ITS domain\n",
-			       np->full_name);
+		if (its_pci_msi_init_one(of_node_to_fwnode(np), np->full_name))
 			continue;
-		}
-
-		if (!pci_msi_create_irq_domain(of_node_to_fwnode(np),
-					       &its_pci_msi_domain_info,
-					       parent)) {
-			pr_err("%s: unable to create PCI domain\n",
-			       np->full_name);
-			continue;
-		}
 
 		pr_info("PCI/MSI: %s domain created\n", np->full_name);
 	}
 
 	return 0;
 }
+
+static int __init its_pci_msi_init(void)
+{
+	its_pci_of_msi_init();
+
+	return 0;
+}
 early_initcall(its_pci_msi_init);
-- 
1.9.1

[PATCH V7 8/8] irqchip/gicv3-its: Use MADT ITS subtable to do PCI/MSI domain initialization

[Tomasz Nowicki]

Let ACPI build ITS PCI MSI domain. ACPI code is responsible for retrieving
inner domain token and passing it on to its_pci_msi_init_one generic
init call.

IORT maintains list of registered domain tokens and allows to find
corresponding domain based on MADT ITS subtable ID info.

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-v3-its-pci-msi.c | 44 ++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3-its-pci-msi.c b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
index d2c2496..ae7221c 100644
--- a/drivers/irqchip/irq-gic-v3-its-pci-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
@@ -15,6 +15,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/iort.h>
 #include <linux/msi.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
@@ -144,9 +145,52 @@ static int __init its_pci_of_msi_init(void)
 	return 0;
 }
 
+#ifdef CONFIG_ACPI
+
+static int __init
+its_pci_msi_parse_madt(struct acpi_subtable_header *header,
+		       const unsigned long end)
+{
+	struct acpi_madt_generic_translator *its_entry;
+	struct fwnode_handle *dom_handle;
+	const char *node_name;
+	int err = -ENXIO;
+
+	its_entry = (struct acpi_madt_generic_translator *)header;
+	node_name = kasprintf(GFP_KERNEL, "ITS@0x%lx",
+			      (long)its_entry->base_address);
+	dom_handle = iort_find_domain_token(its_entry->translation_id);
+	if (!dom_handle) {
+		pr_err("%s: Unable to locate ITS domain handle\n", node_name);
+		goto out;
+	}
+
+	err = its_pci_msi_init_one(dom_handle, node_name);
+	if (!err)
+		pr_info("PCI/MSI: %s domain created\n", node_name);
+
+out:
+	kfree(node_name);
+	return err;
+}
+
+static int __init its_pci_acpi_msi_init(void)
+{
+	acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
+			      its_pci_msi_parse_madt, 0);
+	return 0;
+}
+#else
+static int __init its_pci_acpi_msi_init(void)
+{
+	return 0;
+}
+#endif
+
 static int __init its_pci_msi_init(void)
 {
 	its_pci_of_msi_init();
+	its_pci_acpi_msi_init();
 
 	return 0;
 }
-- 
1.9.1

Re: [PATCH V7 8/8] irqchip/gicv3-its: Use MADT ITS subtable to do PCI/MSI domain initialization

[Hanjun Guo]

On 2016/6/20 19:02, Tomasz Nowicki wrote:
> Let ACPI build ITS PCI MSI domain. ACPI code is responsible for retrieving
> inner domain token and passing it on to its_pci_msi_init_one generic
> init call.
>
> IORT maintains list of registered domain tokens and allows to find
> corresponding domain based on MADT ITS subtable ID info.
>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/irqchip/irq-gic-v3-its-pci-msi.c | 44 ++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its-pci-msi.c b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
> index d2c2496..ae7221c 100644
> --- a/drivers/irqchip/irq-gic-v3-its-pci-msi.c
> +++ b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
> @@ -15,6 +15,7 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>
> +#include <linux/iort.h>
>  #include <linux/msi.h>
>  #include <linux/of.h>
>  #include <linux/of_irq.h>
> @@ -144,9 +145,52 @@ static int __init its_pci_of_msi_init(void)
>  	return 0;
>  }
>
> +#ifdef CONFIG_ACPI
> +
> +static int __init
> +its_pci_msi_parse_madt(struct acpi_subtable_header *header,
> +		       const unsigned long end)
> +{
> +	struct acpi_madt_generic_translator *its_entry;
> +	struct fwnode_handle *dom_handle;
> +	const char *node_name;
> +	int err = -ENXIO;
> +
> +	its_entry = (struct acpi_madt_generic_translator *)header;
> +	node_name = kasprintf(GFP_KERNEL, "ITS@0x%lx",
> +			      (long)its_entry->base_address);
> +	dom_handle = iort_find_domain_token(its_entry->translation_id);
> +	if (!dom_handle) {
> +		pr_err("%s: Unable to locate ITS domain handle\n", node_name);
> +		goto out;
> +	}
> +
> +	err = its_pci_msi_init_one(dom_handle, node_name);
> +	if (!err)
> +		pr_info("PCI/MSI: %s domain created\n", node_name);
> +
> +out:
> +	kfree(node_name);
> +	return err;
> +}
> +
> +static int __init its_pci_acpi_msi_init(void)
> +{
> +	acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
> +			      its_pci_msi_parse_madt, 0);
> +	return 0;
> +}
> +#else
> +static int __init its_pci_acpi_msi_init(void)
> +{
> +	return 0;
> +}
> +#endif
> +
>  static int __init its_pci_msi_init(void)
>  {
>  	its_pci_of_msi_init();
> +	its_pci_acpi_msi_init();
>
>  	return 0;
>  }

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

Re: [PATCH V7 0/8] Introduce ACPI world to ITS irqchip

[Shanker Donthineni]

Hi Tomasz,

On 06/20/2016 06:02 AM, Tomasz Nowicki wrote:
> The series builds the PCI/MSI domain stack based on initial IORT driver
> which is added in first place. As a reference please see IORT spec:
> http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Rema
> pping_Table.pdf
>
> Tested on Cavium ThunderX server. The patches can be found here:
> https://github.com/semihalf-nowicki-tomasz/linux.git (its-acpi-v7)
>
>

I have tested this patchset on Qualcomm Technologies server platform 
QDF2XXX.

-- 
Shanker Donthineni
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

Re: [PATCH V7 0/8] Introduce ACPI world to ITS irqchip

[Tomasz Nowicki]

On 20.06.2016 13:02, Tomasz Nowicki wrote:
> The series builds the PCI/MSI domain stack based on initial IORT driver
> which is added in first place. As a reference please see IORT spec:
> http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf
>
> Tested on Cavium ThunderX server. The patches can be found here:
> https://github.com/semihalf-nowicki-tomasz/linux.git (its-acpi-v7)
>

Hi Bjorn,

I know it is vacation time :) but would you mind to have a look at 
respective patches ? Your ACK is major thing for PCI part.

Thanks,
Tomasz

Re: [PATCH V7 0/8] Introduce ACPI world to ITS irqchip

[Robert Richter]

On 20.06.16 13:02:13, Tomasz Nowicki wrote:
> The series builds the PCI/MSI domain stack based on initial IORT driver
> which is added in first place. As a reference please see IORT spec:
> http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Remapping_Table.pdf
> 
> Tested on Cavium ThunderX server. The patches can be found here:
> https://github.com/semihalf-nowicki-tomasz/linux.git (its-acpi-v7)
> 
> v6 -> v7
> - rebased against irq/irqchip-4.8
> - improvements for IORT ID mapping
> - call IORT init call from acpi_init() instead of arch_initcall()
> - split IORT driver in to two parts: core and ITS/MSI
> 
> v5 -> v6
> - added locking mechanism for IORT list with domain token
> - IORT function name improvements
> - extended IORT RID mapping helper
> - reworked IORT to be more SMMU friendly
> - IORT functions which map RID and find corresponding domain are not
>   PCI specific any more (struct pci_dev -> struct device)
> - bug fixes
> 
> v4 -> v5
> - rebased against v4.7-rc1
> - drop generic layer and call IORT functions directly
> - improve resource abstraction and string formatting
> 
> v3 -> v4
> - rebased against v4.5
> - add ACPI support for IRQ domain handling on a per-device basis
> - reorder domain setup step
> - improve error handling
> - code style improvements
> 
> v2 -> v3
> - rebased on top of 4.4
> - fixes and improvements for redistributor init via GICC structures
> - fixes as per kbuild reports
> 
> v1 -> v2
> - rebased on top of 4.4-rc4
> - use pci_msi_domain_get_msi_rid for requester ID to device ID translation
> 
> Tomasz Nowicki (8):
>   ACPI: I/O Remapping Table (IORT) initial support
>   ACPI: Add new IORT functions to support MSI domain handling
>   PCI/MSI: Setup MSI domain on a per-device basis using IORT ACPI table
>   irqchip/gicv3-its: Cleanup for ITS domain initialization
>   irqchip/gicv3-its: Refactor ITS DT init code to prepare for ACPI
>   irqchip/gicv3-its: Probe ITS in the ACPI way
>   irqchip/gicv3-its: Factor out code that might be reused for ACPI
>   irqchip/gicv3-its: Use MADT ITS subtable to do PCI/MSI domain
>     initialization
> 
>  drivers/acpi/Kconfig                     |   3 +
>  drivers/acpi/Makefile                    |   1 +
>  drivers/acpi/bus.c                       |   2 +
>  drivers/acpi/iort.c                      | 389 +++++++++++++++++++++++++++++++
>  drivers/irqchip/Kconfig                  |   1 +
>  drivers/irqchip/irq-gic-v3-its-pci-msi.c |  88 +++++--
>  drivers/irqchip/irq-gic-v3-its.c         | 169 ++++++++++----
>  drivers/irqchip/irq-gic-v3.c             |   7 +-
>  drivers/pci/msi.c                        |  11 +-
>  include/linux/iort.h                     |  41 ++++
>  include/linux/irqchip/arm-gic-v3.h       |   4 +-
>  11 files changed, 646 insertions(+), 70 deletions(-)
>  create mode 100644 drivers/acpi/iort.c
>  create mode 100644 include/linux/iort.h

For the whole series:

Tested-by: Robert Richter <rrichter@cavium.com>

Tested on Cavium ThunderX, test included:

 [RFC PATCH V5 0/5] ECAM quirks handling for ARM64 platforms

Thanks Tomasz,

-Robert