[PATCH V6 0/7] Introduce ACPI world to ITS irqchip

[PATCH V6 0/7] 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

Patches were built on top of accepted ARM64 ACPI PCI support and can be found
all together here:
https://github.com/semihalf-nowicki-tomasz/linux.git (its-acpi-v6)

Tested on Cavium ThunderX server.

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 (7):
  ACPI: I/O Remapping Table (IORT) initial support
  PCI/MSI: Setup MSI domain on a per-devices basis using IORT ACPI table
  irqchip/gicv3-its: Cleanup for ITS domain initialization
  irqchip/gicv3-its: Refator 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/iort.c                      | 386 +++++++++++++++++++++++++++++++
 drivers/irqchip/Kconfig                  |   1 +
 drivers/irqchip/irq-gic-v3-its-pci-msi.c |  88 +++++--
 drivers/irqchip/irq-gic-v3-its.c         | 185 ++++++++++-----
 drivers/irqchip/irq-gic-v3.c             |   7 +-
 drivers/pci/msi.c                        |  11 +-
 include/linux/iort.h                     |  38 +++
 include/linux/irqchip/arm-gic-v3.h       |   4 +-
 10 files changed, 646 insertions(+), 78 deletions(-)
 create mode 100644 drivers/acpi/iort.c
 create mode 100644 include/linux/iort.h

-- 
1.9.1

[PATCH V6 1/7] 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:
- register ITS MSI chip along with ITS translation ID and domain token
- deregister ITS MSI chip based on ITS translation ID
- find registered domain token based on ITS translation ID
- map MSI RID for a device
- find domain token for a device

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
 drivers/acpi/Kconfig  |   3 +
 drivers/acpi/Makefile |   1 +
 drivers/acpi/iort.c   | 386 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/iort.h  |  38 +++++
 4 files changed, 428 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 f98c328..111dd50 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 632e81f..0390f27 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -83,6 +83,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/iort.c b/drivers/acpi/iort.c
new file mode 100644
index 0000000..5bccbc8
--- /dev/null
+++ b/drivers/acpi/iort.c
@@ -0,0 +1,386 @@
+/*
+ * 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/export.h>
+#include <linux/iort.h>
+#include <linux/irqdomain.h>
+#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.
+ * @translation_id: ITS ID.
+ * @token: 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
+ * @translation_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
+ * @translation_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)
+{
+	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 struct acpi_iort_node *
+iort_node_map_rid(struct acpi_iort_node *node, u32 rid_in,
+		  u32 *rid_out, u8 type)
+{
+
+	if (!node)
+		goto out;
+
+	/* Go upstream */
+	while (node->type != type) {
+		struct acpi_iort_id_mapping *id;
+		int i, found = 0;
+
+		/* Exit when no mapping array */
+		if (!node->mapping_offset || !node->mapping_count)
+			return NULL;
+
+		id = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
+				  node->mapping_offset);
+
+		for (i = 0, found = 0; i < node->mapping_count; i++, id++) {
+			/*
+			 * Single mapping is not translation rule,
+			 * lets move on for this case
+			 */
+			if (id->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
+				if (node->type != ACPI_IORT_NODE_SMMU) {
+					rid_in = id->output_base;
+					found = 1;
+					break;
+				}
+
+				pr_warn(FW_BUG "[node %p type %d] SINGLE MAPPING flag not allowed for SMMU node, skipping ID map\n",
+					node, node->type);
+				continue;
+			}
+
+			if (rid_in < id->input_base ||
+			    (rid_in > id->input_base + id->id_count))
+				continue;
+
+			rid_in = id->output_base + (rid_in - id->input_base);
+			found = 1;
+			break;
+		}
+
+		if (!found)
+			return NULL;
+
+		/* Firmware bug! */
+		if (!id->output_reference) {
+			pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n",
+			       node, node->type);
+			return NULL;
+		}
+
+		node = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
+				    id->output_reference);
+	}
+
+out:
+	if (rid_out)
+		*rid_out = 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);
+}
+
+/**
+ * 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;
+	}
+
+	if (!iort_node_map_rid(node, req_id, &dev_id,
+			       ACPI_IORT_NODE_ITS_GROUP))
+		return req_id;
+
+	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);
+}
+
+static int __init iort_table_detect(void)
+{
+	acpi_status status;
+
+	if (acpi_disabled)
+		return -ENODEV;
+
+	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);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+arch_initcall(iort_table_detect);
diff --git a/include/linux/iort.h b/include/linux/iort.h
new file mode 100644
index 0000000..1bcf2fc
--- /dev/null
+++ b/include/linux/iort.h
@@ -0,0 +1,38 @@
+/*
+ * 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>
+
+struct fwnode_handle;
+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
+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 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 V6 1/7] ACPI: I/O Remapping Table (IORT) initial support

[Marc Zyngier]

On Mon, 13 Jun 2016 16:41:07 +0200
Tomasz Nowicki <tn@semihalf.com> 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:
> - register ITS MSI chip along with ITS translation ID and domain token
> - deregister ITS MSI chip based on ITS translation ID
> - find registered domain token based on ITS translation ID
> - map MSI RID for a device
> - find domain token for a device
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
>  drivers/acpi/Kconfig  |   3 +
>  drivers/acpi/Makefile |   1 +
>  drivers/acpi/iort.c   | 386 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/iort.h  |  38 +++++
>  4 files changed, 428 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 f98c328..111dd50 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 632e81f..0390f27 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -83,6 +83,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/iort.c b/drivers/acpi/iort.c
> new file mode 100644
> index 0000000..5bccbc8
> --- /dev/null
> +++ b/drivers/acpi/iort.c
> @@ -0,0 +1,386 @@
> +/*
> + * 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/export.h>
> +#include <linux/iort.h>
> +#include <linux/irqdomain.h>
> +#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.
> + * @translation_id: ITS ID.
> + * @token: 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
> + * @translation_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
> + * @translation_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)
> +{
> +	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 struct acpi_iort_node *
> +iort_node_map_rid(struct acpi_iort_node *node, u32 rid_in,
> +		  u32 *rid_out, u8 type)
> +{
> +
> +	if (!node)
> +		goto out;
> +
> +	/* Go upstream */
> +	while (node->type != type) {
> +		struct acpi_iort_id_mapping *id;
> +		int i, found = 0;
> +
> +		/* Exit when no mapping array */
> +		if (!node->mapping_offset || !node->mapping_count)
> +			return NULL;
> +
> +		id = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
> +				  node->mapping_offset);
> +
> +		for (i = 0, found = 0; i < node->mapping_count; i++, id++) {
> +			/*
> +			 * Single mapping is not translation rule,
> +			 * lets move on for this case
> +			 */
> +			if (id->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
> +				if (node->type != ACPI_IORT_NODE_SMMU) {
> +					rid_in = id->output_base;
> +					found = 1;
> +					break;
> +				}
> +
> +				pr_warn(FW_BUG "[node %p type %d] SINGLE MAPPING flag not allowed for SMMU node, skipping ID map\n",
> +					node, node->type);
> +				continue;
> +			}
> +
> +			if (rid_in < id->input_base ||
> +			    (rid_in > id->input_base + id->id_count))
> +				continue;
> +
> +			rid_in = id->output_base + (rid_in - id->input_base);
> +			found = 1;
> +			break;
> +		}
> +
> +		if (!found)
> +			return NULL;

Why this special case? It would make more sense to use the normal
epilogue, and update rid_out. Unless not finding a translation for a
given rid is illegal?

> +
> +		/* Firmware bug! */
> +		if (!id->output_reference) {
> +			pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n",
> +			       node, node->type);
> +			return NULL;
> +		}
> +
> +		node = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> +				    id->output_reference);
> +	}
> +
> +out:
> +	if (rid_out)
> +		*rid_out = 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);
> +}
> +
> +/**
> + * 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;
> +	}
> +
> +	if (!iort_node_map_rid(node, req_id, &dev_id,
> +			       ACPI_IORT_NODE_ITS_GROUP))
> +		return req_id;

And once you've fixed the special case in iort_node_map_rid, you can
unconditionally return dev_id.

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

Can this actually happen? I can't see how, unless you have a race
between iort_dev_find_its_id and iort_find_domain_token. And given that
both these functions are only called from here, maybe you're better off
having a single function:

struct fwnode_handle *iort_dev_find_its_domain_token(struct device *dev,
						     u32 rid);

which returns the atomic lookup of the ITS handle. Or is there any
constraints preventing us from holding the lock?

> +
> +	return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
> +}
> +
> +static int __init iort_table_detect(void)
> +{
> +	acpi_status status;
> +
> +	if (acpi_disabled)
> +		return -ENODEV;
> +
> +	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);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +arch_initcall(iort_table_detect);

Instead of an initcall, can't this be directly called from whatever
ACPI init we already have?

> diff --git a/include/linux/iort.h b/include/linux/iort.h
> new file mode 100644
> index 0000000..1bcf2fc
> --- /dev/null
> +++ b/include/linux/iort.h
> @@ -0,0 +1,38 @@
> +/*
> + * 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>
> +
> +struct fwnode_handle;
> +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
> +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 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__ */

Other than the above, this is finally starting to look like something
I can work with, provided that you address these small comments. I'd
also expect the ACPI folks (Rafael, Lorenzo) to chime in and comment on
this.

Thanks,

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

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

[Tomasz Nowicki]

On 15.06.2016 10:31, Marc Zyngier wrote:
> On Mon, 13 Jun 2016 16:41:07 +0200
> Tomasz Nowicki <tn@semihalf.com> 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:
>> - register ITS MSI chip along with ITS translation ID and domain token
>> - deregister ITS MSI chip based on ITS translation ID
>> - find registered domain token based on ITS translation ID
>> - map MSI RID for a device
>> - find domain token for a device
>>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> ---
>>   drivers/acpi/Kconfig  |   3 +
>>   drivers/acpi/Makefile |   1 +
>>   drivers/acpi/iort.c   | 386 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/iort.h  |  38 +++++
>>   4 files changed, 428 insertions(+)
>>   create mode 100644 drivers/acpi/iort.c
>>   create mode 100644 include/linux/iort.h
>>

[...]

>> +
>> +static struct acpi_iort_node *
>> +iort_node_map_rid(struct acpi_iort_node *node, u32 rid_in,
>> +		  u32 *rid_out, u8 type)
>> +{
>> +
>> +	if (!node)
>> +		goto out;
>> +
>> +	/* Go upstream */
>> +	while (node->type != type) {
>> +		struct acpi_iort_id_mapping *id;
>> +		int i, found = 0;
>> +
>> +		/* Exit when no mapping array */
>> +		if (!node->mapping_offset || !node->mapping_count)
>> +			return NULL;
>> +
>> +		id = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
>> +				  node->mapping_offset);
>> +
>> +		for (i = 0, found = 0; i < node->mapping_count; i++, id++) {
>> +			/*
>> +			 * Single mapping is not translation rule,
>> +			 * lets move on for this case
>> +			 */
>> +			if (id->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
>> +				if (node->type != ACPI_IORT_NODE_SMMU) {
>> +					rid_in = id->output_base;
>> +					found = 1;
>> +					break;
>> +				}
>> +
>> +				pr_warn(FW_BUG "[node %p type %d] SINGLE MAPPING flag not allowed for SMMU node, skipping ID map\n",
>> +					node, node->type);
>> +				continue;
>> +			}
>> +
>> +			if (rid_in < id->input_base ||
>> +			    (rid_in > id->input_base + id->id_count))
>> +				continue;
>> +
>> +			rid_in = id->output_base + (rid_in - id->input_base);
>> +			found = 1;
>> +			break;
>> +		}
>> +
>> +		if (!found)
>> +			return NULL;
>
> Why this special case? It would make more sense to use the normal
> epilogue, and update rid_out. Unless not finding a translation for a
> given rid is illegal?

We can use the same strategy as __of_msi_map_rid() which means we simply 
use rid_in in case of any error. I will update accordingly.

>
>> +
>> +		/* Firmware bug! */
>> +		if (!id->output_reference) {
>> +			pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n",
>> +			       node, node->type);
>> +			return NULL;
>> +		}
>> +
>> +		node = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
>> +				    id->output_reference);
>> +	}
>> +
>> +out:
>> +	if (rid_out)
>> +		*rid_out = 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);
>> +}
>> +
>> +/**
>> + * 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;
>> +	}
>> +
>> +	if (!iort_node_map_rid(node, req_id, &dev_id,
>> +			       ACPI_IORT_NODE_ITS_GROUP))
>> +		return req_id;
>
> And once you've fixed the special case in iort_node_map_rid, you can
> unconditionally return dev_id.

Right.

>
>> +
>> +	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;
>
> Can this actually happen? I can't see how, unless you have a race
> between iort_dev_find_its_id and iort_find_domain_token. And given that
> both these functions are only called from here, maybe you're better off
> having a single function:
>
> struct fwnode_handle *iort_dev_find_its_domain_token(struct device *dev,
> 						     u32 rid);
>
> which returns the atomic lookup of the ITS handle. Or is there any
> constraints preventing us from holding the lock?

Yes this may happen, let's say we have one ITS with ID = 0:
1. iort_register_domain_token() fails because of lack of memory (-ENOMEM)
2. iort_dev_find_its_id() would point us to ITS with ID = 0
3. iort_find_domain_token() return NULL due to no element on the list 
for ITS ID = 0

Actually iort_dev_find_its_id() finds out ITS ID related to a given 
device, it only interact with IORT content but not with 
iort_msi_chip_list list. iort_find_domain_token() has its own lock for 
iort_msi_chip_list so I am not sure why we need lock.

Thanks,
Tomasz

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

[Lorenzo Pieralisi]

On Mon, Jun 13, 2016 at 04:41:07PM +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:
> - register ITS MSI chip along with ITS translation ID and domain token
> - deregister ITS MSI chip based on ITS translation ID
> - find registered domain token based on ITS translation ID
> - map MSI RID for a device
> - find domain token for a device
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
>  drivers/acpi/Kconfig  |   3 +
>  drivers/acpi/Makefile |   1 +
>  drivers/acpi/iort.c   | 386 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/iort.h  |  38 +++++
>  4 files changed, 428 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 f98c328..111dd50 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 632e81f..0390f27 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -83,6 +83,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/iort.c b/drivers/acpi/iort.c
> new file mode 100644
> index 0000000..5bccbc8
> --- /dev/null
> +++ b/drivers/acpi/iort.c
> @@ -0,0 +1,386 @@
> +/*
> + * 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/export.h>
> +#include <linux/iort.h>
> +#include <linux/irqdomain.h>
> +#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;

A question to be sorted out:

We assume we can rely on the iort_table pointer, obtained through
acpi_get_table(), since we assume acpi_glb_permanent_mmap is set (?),
correct ?

x86 DMAR code seems to rely on that (without even checking
acpi_gbl_permanent_mmap) and this has consequences on when
we can really start parsing IORT entries through this patch
(because if acpi_gbl_permanent_mmap is not set while using
IORT nodes we would dereference unmapped pointers).

@Rafael: can you confirm that's the right approach ?

> +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.
> + * @translation_id: ITS ID.
> + * @token: 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
> + * @translation_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
> + * @translation_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;
> +}

You are lumping irq_domain/MSI/ITS code and basic IORT (core) support
in one patch, I would split them in two.

> +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 struct acpi_iort_node *
> +iort_node_map_rid(struct acpi_iort_node *node, u32 rid_in,
> +		  u32 *rid_out, u8 type)
> +{
> +
> +	if (!node)
> +		goto out;

Mmmm..can you explain to me what's the logic here ?

> +	/* Go upstream */
> +	while (node->type != type) {
> +		struct acpi_iort_id_mapping *id;
> +		int i, found = 0;
> +
> +		/* Exit when no mapping array */
> +		if (!node->mapping_offset || !node->mapping_count)
> +			return NULL;
> +
> +		id = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
> +				  node->mapping_offset);
> +
> +		for (i = 0, found = 0; i < node->mapping_count; i++, id++) {
> +			/*
> +			 * Single mapping is not translation rule,
> +			 * lets move on for this case
> +			 */
> +			if (id->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
> +				if (node->type != ACPI_IORT_NODE_SMMU) {

This is wrong (ie node can be an SMMU v3 and an ITS group).

> +					rid_in = id->output_base;
> +					found = 1;
> +					break;
> +				}
> +
> +				pr_warn(FW_BUG "[node %p type %d] SINGLE MAPPING flag not allowed for SMMU node, skipping ID map\n",
> +					node, node->type);
> +				continue;
> +			}
> +
> +			if (rid_in < id->input_base ||
> +			    (rid_in > id->input_base + id->id_count))
> +				continue;
> +
> +			rid_in = id->output_base + (rid_in - id->input_base);
> +			found = 1;
> +			break;
> +		}
> +

This inner loop is getting too complicated (and this function with
it) to my taste. Is it reasonable to factor it out in a separate
function ?

> +		if (!found)
> +			return NULL;
> +
> +		/* Firmware bug! */
> +		if (!id->output_reference) {
> +			pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n",
> +			       node, node->type);
> +			return NULL;
> +		}
> +
> +		node = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> +				    id->output_reference);
> +	}
> +
> +out:
> +	if (rid_out)
> +		*rid_out = 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);
> +}
> +
> +/**
> + * 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;
> +	}
> +
> +	if (!iort_node_map_rid(node, req_id, &dev_id,
> +			       ACPI_IORT_NODE_ITS_GROUP))
> +		return req_id;
> +
> +	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))
                                              ^
This is supposed to be an index in the ITS identifiers list and it is
always 0 (I *guess* that's because _any_ identifier in that group
would do but I want to undestand why), please explain :)

One reason more why I think you should split this patch in two
so that it becomes easier for Marc to review the ITS specific
bits:

- IORT core
- ITS/MSI IORT handling

> +		return NULL;
> +
> +	handle = iort_find_domain_token(its_id);
> +	if (!handle)
> +		return NULL;
> +
> +	return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
> +}
> +
> +static int __init iort_table_detect(void)
> +{
> +	acpi_status status;
> +
> +	if (acpi_disabled)
> +		return -ENODEV;
> +
> +	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);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +arch_initcall(iort_table_detect);

To prevent calling it from an initcall we can call it from arch
code (but careful about the iort_table pointer validity, see above).

We should settle the iort_table pointer validity first, everything
else depends on it.

Lorenzo

> diff --git a/include/linux/iort.h b/include/linux/iort.h
> new file mode 100644
> index 0000000..1bcf2fc
> --- /dev/null
> +++ b/include/linux/iort.h
> @@ -0,0 +1,38 @@
> +/*
> + * 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>
> +
> +struct fwnode_handle;
> +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
> +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 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 V6 1/7] ACPI: I/O Remapping Table (IORT) initial support

[Tomasz Nowicki]

On 15.06.2016 13:04, Lorenzo Pieralisi wrote:
> On Mon, Jun 13, 2016 at 04:41:07PM +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:
>> - register ITS MSI chip along with ITS translation ID and domain token
>> - deregister ITS MSI chip based on ITS translation ID
>> - find registered domain token based on ITS translation ID
>> - map MSI RID for a device
>> - find domain token for a device
>>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> ---
>>   drivers/acpi/Kconfig  |   3 +
>>   drivers/acpi/Makefile |   1 +
>>   drivers/acpi/iort.c   | 386 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/iort.h  |  38 +++++
>>   4 files changed, 428 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 f98c328..111dd50 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 632e81f..0390f27 100644
>> --- a/drivers/acpi/Makefile
>> +++ b/drivers/acpi/Makefile
>> @@ -83,6 +83,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/iort.c b/drivers/acpi/iort.c
>> new file mode 100644
>> index 0000000..5bccbc8
>> --- /dev/null
>> +++ b/drivers/acpi/iort.c
>> @@ -0,0 +1,386 @@
>> +/*
>> + * 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/export.h>
>> +#include <linux/iort.h>
>> +#include <linux/irqdomain.h>
>> +#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;
>
> A question to be sorted out:
>
> We assume we can rely on the iort_table pointer, obtained through
> acpi_get_table(), since we assume acpi_glb_permanent_mmap is set (?),
> correct ?
>
> x86 DMAR code seems to rely on that (without even checking
> acpi_gbl_permanent_mmap) and this has consequences on when
> we can really start parsing IORT entries through this patch
> (because if acpi_gbl_permanent_mmap is not set while using
> IORT nodes we would dereference unmapped pointers).
>
> @Rafael: can you confirm that's the right approach ?
>
>> +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.
>> + * @translation_id: ITS ID.
>> + * @token: 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
>> + * @translation_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
>> + * @translation_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;
>> +}
>
> You are lumping irq_domain/MSI/ITS code and basic IORT (core) support
> in one patch, I would split them in two.

OK

>
>> +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 struct acpi_iort_node *
>> +iort_node_map_rid(struct acpi_iort_node *node, u32 rid_in,
>> +		  u32 *rid_out, u8 type)
>> +{
>> +
>> +	if (!node)
>> +		goto out;
>
> Mmmm..can you explain to me what's the logic here ?

As Marc pointed out, the logic is not consistent now.

iort_node_map_rid IMO should map rid and return parent node which 
provide final translation e.g. IORT or SMMU node. In case of any error 
it should return NULL and provide 1:1 RID mapping (rid_out = rid_in).

>
>> +	/* Go upstream */
>> +	while (node->type != type) {
>> +		struct acpi_iort_id_mapping *id;
>> +		int i, found = 0;
>> +
>> +		/* Exit when no mapping array */
>> +		if (!node->mapping_offset || !node->mapping_count)
>> +			return NULL;
>> +
>> +		id = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
>> +				  node->mapping_offset);
>> +
>> +		for (i = 0, found = 0; i < node->mapping_count; i++, id++) {
>> +			/*
>> +			 * Single mapping is not translation rule,
>> +			 * lets move on for this case
>> +			 */
>> +			if (id->flags & ACPI_IORT_ID_SINGLE_MAPPING) {
>> +				if (node->type != ACPI_IORT_NODE_SMMU) {
>
> This is wrong (ie node can be an SMMU v3 and an ITS group).

Right, ITS will never get to this point but SMMU v3 can. I will invert 
condition to:
	if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT ||
	    node->type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
	[...]
	}

>
>> +					rid_in = id->output_base;
>> +					found = 1;
>> +					break;
>> +				}
>> +
>> +				pr_warn(FW_BUG "[node %p type %d] SINGLE MAPPING flag not allowed for SMMU node, skipping ID map\n",
>> +					node, node->type);
>> +				continue;
>> +			}
>> +
>> +			if (rid_in < id->input_base ||
>> +			    (rid_in > id->input_base + id->id_count))
>> +				continue;
>> +
>> +			rid_in = id->output_base + (rid_in - id->input_base);
>> +			found = 1;
>> +			break;
>> +		}
>> +
>
> This inner loop is getting too complicated (and this function with
> it) to my taste. Is it reasonable to factor it out in a separate
> function ?

I will try to put it to another fundtion.

>
>> +		if (!found)
>> +			return NULL;
>> +
>> +		/* Firmware bug! */
>> +		if (!id->output_reference) {
>> +			pr_err(FW_BUG "[node %p type %d] ID map has NULL parent reference\n",
>> +			       node, node->type);
>> +			return NULL;
>> +		}
>> +
>> +		node = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
>> +				    id->output_reference);
>> +	}
>> +
>> +out:
>> +	if (rid_out)
>> +		*rid_out = 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);
>> +}
>> +
>> +/**
>> + * 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;
>> +	}
>> +
>> +	if (!iort_node_map_rid(node, req_id, &dev_id,
>> +			       ACPI_IORT_NODE_ITS_GROUP))
>> +		return req_id;
>> +
>> +	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))
>                                                ^
> This is supposed to be an index in the ITS identifiers list and it is
> always 0 (I *guess* that's because _any_ identifier in that group
> would do but I want to undestand why), please explain :)

Well, we do not have infrastructure to decide which index would be 
better and since any index is fine from the iort_get_device_domain() 
perspecitive, I just used 0 here.

>
> One reason more why I think you should split this patch in two
> so that it becomes easier for Marc to review the ITS specific
> bits:
>
> - IORT core
> - ITS/MSI IORT handling
>
>> +		return NULL;
>> +
>> +	handle = iort_find_domain_token(its_id);
>> +	if (!handle)
>> +		return NULL;
>> +
>> +	return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
>> +}
>> +
>> +static int __init iort_table_detect(void)
>> +{
>> +	acpi_status status;
>> +
>> +	if (acpi_disabled)
>> +		return -ENODEV;
>> +
>> +	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);
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +arch_initcall(iort_table_detect);
>
> To prevent calling it from an initcall we can call it from arch
> code (but careful about the iort_table pointer validity, see above).
>
> We should settle the iort_table pointer validity first, everything
> else depends on it.

Yes.

Thanks,
Tomasz

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

[Tomasz Nowicki]

On 06/15/2016 01:04 PM, Lorenzo Pieralisi wrote:
> On Mon, Jun 13, 2016 at 04:41:07PM +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:
>> - register ITS MSI chip along with ITS translation ID and domain token
>> - deregister ITS MSI chip based on ITS translation ID
>> - find registered domain token based on ITS translation ID
>> - map MSI RID for a device
>> - find domain token for a device
>>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> ---
>>   drivers/acpi/Kconfig  |   3 +
>>   drivers/acpi/Makefile |   1 +
>>   drivers/acpi/iort.c   | 386 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/iort.h  |  38 +++++
>>   4 files changed, 428 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 f98c328..111dd50 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 632e81f..0390f27 100644
>> --- a/drivers/acpi/Makefile
>> +++ b/drivers/acpi/Makefile
>> @@ -83,6 +83,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/iort.c b/drivers/acpi/iort.c
>> new file mode 100644
>> index 0000000..5bccbc8
>> --- /dev/null
>> +++ b/drivers/acpi/iort.c
>> @@ -0,0 +1,386 @@
>> +/*
>> + * 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/export.h>
>> +#include <linux/iort.h>
>> +#include <linux/irqdomain.h>
>> +#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;
> A question to be sorted out:
>
> We assume we can rely on the iort_table pointer, obtained through
> acpi_get_table(), since we assume acpi_glb_permanent_mmap is set (?),
> correct ?

Correct.

>
> x86 DMAR code seems to rely on that (without even checking
> acpi_gbl_permanent_mmap) and this has consequences on when
> we can really start parsing IORT entries through this patch
> (because if acpi_gbl_permanent_mmap is not set while using
> IORT nodes we would dereference unmapped pointers).
>
> @Rafael: can you confirm that's the right approach ?

Thanks,
Tomasz

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

[Sinan Kaya]

Hi Tomasz,

> +static acpi_status
> +iort_match_node_callback(struct acpi_iort_node *node, void *context)
> +{
> +	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;
> +

There is problem with the find_dev_node and callback for PCIe here. It assumes
a one-to-one relationship between an SMMU and root complex. 

Just checked with Charles offline to see if there is anything in the IORT spec that forces
this. And, the answer was no. 

Pasting the IORT requirements for you below.

“The IORT was intended to be flexible enough to define static RID to SID mappings, which should cover 
the following configurations:
-	Dedicated SMMU per RC
-	Multiple RC’s per SMMU (as you described)
-	Multiple SMMU’s per RC (with static RID:SID range per SMMU)

The SMMU instance must be identified by either a device ID *or* a combination of 
segment ID *and* Requestor ID. ”

If a root complex has multiple SMMUs, this code is going to return the first SMMU. This needs
to be corrected.

> +		break;
> +	}
> +	}
> +
> +	return AE_NOT_FOUND;
> +}
> +

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

-- 
Sinan Kaya
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 V6 1/7] ACPI: I/O Remapping Table (IORT) initial support

[Lorenzo Pieralisi]

On Wed, Jun 15, 2016 at 09:19:54AM -0400, Sinan Kaya wrote:
> Hi Tomasz,
> 
> > +static acpi_status
> > +iort_match_node_callback(struct acpi_iort_node *node, void *context)
> > +{
> > +	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;
> > +
> 
> There is problem with the find_dev_node and callback for PCIe here. It assumes
> a one-to-one relationship between an SMMU and root complex. 
> 
> Just checked with Charles offline to see if there is anything in the IORT spec that forces
> this. And, the answer was no. 
> 
> Pasting the IORT requirements for you below.
> 
> ?The IORT was intended to be flexible enough to define static RID to SID mappings, which should cover 
> the following configurations:
> -	Dedicated SMMU per RC
> -	Multiple RC?s per SMMU (as you described)
> -	Multiple SMMU?s per RC (with static RID:SID range per SMMU)
> 
> The SMMU instance must be identified by either a device ID *or* a combination of 
> segment ID *and* Requestor ID. ?
> 
> If a root complex has multiple SMMUs, this code is going to return the
> first SMMU. This needs to be corrected.

What you say above is correct, but the problem is not here. This
callback returns either a named component IORT node or a root complex
IORT node corresponding to a device, the problem you are referring to is
related to detecting which SMMU a given named component or root
complex refers too, which is not done here, I will take care of that
on my SMMU series.

When we look for the SMMU a PCI device is connected to, we must first
retrieve the IORT node of its root complex and walk its list of
mappings and match through RID range instead of picking the first
one, as I assumed, wrongly.

Lorenzo

> 
> > +		break;
> > +	}
> > +	}
> > +
> > +	return AE_NOT_FOUND;
> > +}
> > +
> 
> > +
> > +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);
> > +}
> > +
> 
> -- 
> Sinan Kaya
> 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 V6 1/7] ACPI: I/O Remapping Table (IORT) initial support

[Sinan Kaya]

On 6/15/2016 9:34 AM, Lorenzo Pieralisi wrote:
> On Wed, Jun 15, 2016 at 09:19:54AM -0400, Sinan Kaya wrote:
>> Hi Tomasz,
>>
>>> +static acpi_status
>>> +iort_match_node_callback(struct acpi_iort_node *node, void *context)
>>> +{
>>> +	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;
>>> +
>>
>> There is problem with the find_dev_node and callback for PCIe here. It assumes
>> a one-to-one relationship between an SMMU and root complex. 
>>
>> Just checked with Charles offline to see if there is anything in the IORT spec that forces
>> this. And, the answer was no. 
>>
>> Pasting the IORT requirements for you below.
>>
>> ?The IORT was intended to be flexible enough to define static RID to SID mappings, which should cover 
>> the following configurations:
>> -	Dedicated SMMU per RC
>> -	Multiple RC?s per SMMU (as you described)
>> -	Multiple SMMU?s per RC (with static RID:SID range per SMMU)
>>
>> The SMMU instance must be identified by either a device ID *or* a combination of 
>> segment ID *and* Requestor ID. ?
>>
>> If a root complex has multiple SMMUs, this code is going to return the
>> first SMMU. This needs to be corrected.
> 
> What you say above is correct, but the problem is not here. This
> callback returns either a named component IORT node or a root complex
> IORT node corresponding to a device, the problem you are referring to is
> related to detecting which SMMU a given named component or root
> complex refers too, which is not done here, I will take care of that
> on my SMMU series.
> 
> When we look for the SMMU a PCI device is connected to, we must first
> retrieve the IORT node of its root complex and walk its list of
> mappings and match through RID range instead of picking the first
> one, as I assumed, wrongly.
> 
> Lorenzo
> 

Thanks for posting. I was trying to be more explicit by a follow up email.
You sent before me. 

The summary is that iort_find_dev_node function below will locate the wrong IORT
root complex node in a multiple root port inside the same root complex configuration. 

I wish I could share the picture Harb drew here. Let me put it in text.

You can have a use case where you have two root ports in a single root complex.

Each root port has its own SMMU. Root ports are described in the
MCFG table and in the DSDT table as root bridge with their respective bus start
and end addresses. They both participate in the same root complex with the same 
segment number.

First root port requester id range (0x0-0x3ff) and second root port requester id range 
(0x400-0x7ff).

The IORT table has two root complex entries for each root port. The first entry describes
the requester id range (0x0-0x3ff) and points to first smmu behind id.

The second entry also describes the id range (0x400-0x7ff) and points to second smmu id.

The iort_find_dev_node function tries to locate an IORT node for a given PCIe device id.

If the requester id is 0x400 and segment id is 0, then this function will stop searching
as soon as it finds the first node with segment id 0 as it only uses the segment id
as a qualifier. 

It will locate the PCIe root complex node with requester id range (0x0-0x3ff) and use the 
wrong smmu to do the ITS device id mapping.

"The SMMU instance must be identified by either a device ID *or* a combination of 
  segment ID *and* Requestor ID. ?"


>>
>>> +		break;
>>> +	}
>>> +	}
>>> +
>>> +	return AE_NOT_FOUND;
>>> +}
>>> +
>>
>>> +
>>> +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);
>>> +}
>>> +
>>
>> -- 
>> Sinan Kaya
>> 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
>>


-- 
Sinan Kaya
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 V6 1/7] ACPI: I/O Remapping Table (IORT) initial support

[Lorenzo Pieralisi]

On Wed, Jun 15, 2016 at 09:46:29AM -0400, Sinan Kaya wrote:
> On 6/15/2016 9:34 AM, Lorenzo Pieralisi wrote:
> > On Wed, Jun 15, 2016 at 09:19:54AM -0400, Sinan Kaya wrote:
> >> Hi Tomasz,
> >>
> >>> +static acpi_status
> >>> +iort_match_node_callback(struct acpi_iort_node *node, void *context)
> >>> +{
> >>> +	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;
> >>> +
> >>
> >> There is problem with the find_dev_node and callback for PCIe here. It assumes
> >> a one-to-one relationship between an SMMU and root complex. 
> >>
> >> Just checked with Charles offline to see if there is anything in the IORT spec that forces
> >> this. And, the answer was no. 
> >>
> >> Pasting the IORT requirements for you below.
> >>
> >> ?The IORT was intended to be flexible enough to define static RID to SID mappings, which should cover 
> >> the following configurations:
> >> -	Dedicated SMMU per RC
> >> -	Multiple RC?s per SMMU (as you described)
> >> -	Multiple SMMU?s per RC (with static RID:SID range per SMMU)
> >>
> >> The SMMU instance must be identified by either a device ID *or* a combination of 
> >> segment ID *and* Requestor ID. ?
> >>
> >> If a root complex has multiple SMMUs, this code is going to return the
> >> first SMMU. This needs to be corrected.
> > 
> > What you say above is correct, but the problem is not here. This
> > callback returns either a named component IORT node or a root complex
> > IORT node corresponding to a device, the problem you are referring to is
> > related to detecting which SMMU a given named component or root
> > complex refers too, which is not done here, I will take care of that
> > on my SMMU series.
> > 
> > When we look for the SMMU a PCI device is connected to, we must first
> > retrieve the IORT node of its root complex and walk its list of
> > mappings and match through RID range instead of picking the first
> > one, as I assumed, wrongly.
> > 
> > Lorenzo
> > 
> 
> Thanks for posting. I was trying to be more explicit by a follow up
> email.  You sent before me. 
> 
> The summary is that iort_find_dev_node function below will locate the
> wrong IORT root complex node in a multiple root port inside the same
> root complex configuration. 
> 
> I wish I could share the picture Harb drew here. Let me put it in text.
> 
> You can have a use case where you have two root ports in a single root complex.
> 
> Each root port has its own SMMU. Root ports are described in the MCFG
> table and in the DSDT table as root bridge with their respective bus
> start and end addresses. They both participate in the same root
> complex with the same segment number.
> 
> First root port requester id range (0x0-0x3ff) and second root port
> requester id range (0x400-0x7ff).

Ok, so why a single IORT node root complex entry with multiple node
mappings (with different RID ranges AND SMMU output references)
would not do here ?

Sorry for being blunt but I would like to understand where the
problem is here.

> The IORT table has two root complex entries for each root port. The
> first entry describes the requester id range (0x0-0x3ff) and points to
> first smmu behind id.

I lost you here. Do you mean the IORT table has one root complex IORT
node with two node mappings ?

> The second entry also describes the id range (0x400-0x7ff) and points to second smmu id.
> 
> The iort_find_dev_node function tries to locate an IORT node for a given PCIe device id.
> 
> If the requester id is 0x400 and segment id is 0, then this function
> will stop searching as soon as it finds the first node with segment id
> 0 as it only uses the segment id as a qualifier. 

Well yes. The question is whether we should have two root complexes
IORT nodes with the same segment id or a single root complex IORT node
with multiple mappings.

If we have one PCI root complex IORT node with multiple node mappings,
where is the problem ?

Thanks !
Lorenzo

> 
> It will locate the PCIe root complex node with requester id range (0x0-0x3ff) and use the 
> wrong smmu to do the ITS device id mapping.
> 
> "The SMMU instance must be identified by either a device ID *or* a combination of 
>   segment ID *and* Requestor ID. ?"
> 
> 
> >>
> >>> +		break;
> >>> +	}
> >>> +	}
> >>> +
> >>> +	return AE_NOT_FOUND;
> >>> +}
> >>> +
> >>
> >>> +
> >>> +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);
> >>> +}
> >>> +
> >>
> >> -- 
> >> Sinan Kaya
> >> 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
> >>
> 
> 
> -- 
> Sinan Kaya
> 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 V6 1/7] ACPI: I/O Remapping Table (IORT) initial support

[Sinan Kaya]

On 6/15/2016 10:13 AM, Lorenzo Pieralisi wrote:
> On Wed, Jun 15, 2016 at 09:46:29AM -0400, Sinan Kaya wrote:
>> On 6/15/2016 9:34 AM, Lorenzo Pieralisi wrote:
>>> On Wed, Jun 15, 2016 at 09:19:54AM -0400, Sinan Kaya wrote:
>>>> Hi Tomasz,
>>>>
>>>>> +static acpi_status
>>>>> +iort_match_node_callback(struct acpi_iort_node *node, void *context)
>>>>> +{
>>>>> +	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;
>>>>> +
>>>>
>>>> There is problem with the find_dev_node and callback for PCIe here. It assumes
>>>> a one-to-one relationship between an SMMU and root complex. 
>>>>
>>>> Just checked with Charles offline to see if there is anything in the IORT spec that forces
>>>> this. And, the answer was no. 
>>>>
>>>> Pasting the IORT requirements for you below.
>>>>
>>>> ?The IORT was intended to be flexible enough to define static RID to SID mappings, which should cover 
>>>> the following configurations:
>>>> -	Dedicated SMMU per RC
>>>> -	Multiple RC?s per SMMU (as you described)
>>>> -	Multiple SMMU?s per RC (with static RID:SID range per SMMU)
>>>>
>>>> The SMMU instance must be identified by either a device ID *or* a combination of 
>>>> segment ID *and* Requestor ID. ?
>>>>
>>>> If a root complex has multiple SMMUs, this code is going to return the
>>>> first SMMU. This needs to be corrected.
>>>
>>> What you say above is correct, but the problem is not here. This
>>> callback returns either a named component IORT node or a root complex
>>> IORT node corresponding to a device, the problem you are referring to is
>>> related to detecting which SMMU a given named component or root
>>> complex refers too, which is not done here, I will take care of that
>>> on my SMMU series.
>>>
>>> When we look for the SMMU a PCI device is connected to, we must first
>>> retrieve the IORT node of its root complex and walk its list of
>>> mappings and match through RID range instead of picking the first
>>> one, as I assumed, wrongly.
>>>
>>> Lorenzo
>>>
>>
>> Thanks for posting. I was trying to be more explicit by a follow up
>> email.  You sent before me. 
>>
>> The summary is that iort_find_dev_node function below will locate the
>> wrong IORT root complex node in a multiple root port inside the same
>> root complex configuration. 
>>
>> I wish I could share the picture Harb drew here. Let me put it in text.
>>
>> You can have a use case where you have two root ports in a single root complex.
>>
>> Each root port has its own SMMU. Root ports are described in the MCFG
>> table and in the DSDT table as root bridge with their respective bus
>> start and end addresses. They both participate in the same root
>> complex with the same segment number.
>>
>> First root port requester id range (0x0-0x3ff) and second root port
>> requester id range (0x400-0x7ff).
> 
> Ok, so why a single IORT node root complex entry with multiple node
> mappings (with different RID ranges AND SMMU output references)
> would not do here ?

Just talked to Harb following your recommendation. Your suggestion makes more sense.
The mapping is an array. Spec-wise this is doable. 

We assumed that you could only have one mapping under a PCIe RC node. That's why,
we were thinking of multiple root complex nodes with a single unique mapping.
It was an oversight from my side.

> 
> Sorry for being blunt but I would like to understand where the
> problem is here.
> 
>> The IORT table has two root complex entries for each root port. The
>> first entry describes the requester id range (0x0-0x3ff) and points to
>> first smmu behind id.
> 
> I lost you here. Do you mean the IORT table has one root complex IORT
> node with two node mappings ?

Two root complex entries with one mapping.

> 
>> The second entry also describes the id range (0x400-0x7ff) and points to second smmu id.
>>
>> The iort_find_dev_node function tries to locate an IORT node for a given PCIe device id.
>>
>> If the requester id is 0x400 and segment id is 0, then this function
>> will stop searching as soon as it finds the first node with segment id
>> 0 as it only uses the segment id as a qualifier. 
> 
> Well yes. The question is whether we should have two root complexes
> IORT nodes with the same segment id or a single root complex IORT node
> with multiple mappings.
> 
> If we have one PCI root complex IORT node with multiple node mappings,
> where is the problem ?

No problem. That works. Let's make sure that the ITS and SMMU implementation allows this
though. From the spec point of view and ACPI table point of view, multiple node mappings make
perfect sense.

Thanks

> 
> Thanks !
> Lorenzo
> 
>>
>> It will locate the PCIe root complex node with requester id range (0x0-0x3ff) and use the 
>> wrong smmu to do the ITS device id mapping.
>>
>> "The SMMU instance must be identified by either a device ID *or* a combination of 
>>   segment ID *and* Requestor ID. ?"
>>
>>
>>>>
>>>>> +		break;
>>>>> +	}
>>>>> +	}
>>>>> +
>>>>> +	return AE_NOT_FOUND;
>>>>> +}
>>>>> +
>>>>
>>>>> +
>>>>> +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);
>>>>> +}
>>>>> +
>>>>
>>>> -- 
>>>> Sinan Kaya
>>>> 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
>>>>
>>
>>
>> -- 
>> Sinan Kaya
>> 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
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Sinan Kaya
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

[PATCH V6 2/7] PCI/MSI: Setup MSI domain on a per-devices 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>
---
 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 V6 2/7] PCI/MSI: Setup MSI domain on a per-devices basis using IORT ACPI table

[Marc Zyngier]

On Mon, 13 Jun 2016 16:41:08 +0200
Tomasz Nowicki <tn@semihalf.com> 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>

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

[PATCH V6 3/7] 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 5eb1f9e..43d32fc 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1507,13 +1507,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;
@@ -1605,28 +1629,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);
@@ -1677,6 +1682,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

[PATCH V6 4/7] irqchip/gicv3-its: Refator 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. Fix ITS base register address type (from 'unsigned long' to 'phys_addr_t'),
as a bonus we get nice string formatting.
2. 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   | 79 +++++++++++++++++++++-----------------
 drivers/irqchip/irq-gic-v3.c       |  6 +--
 include/linux/irqchip/arm-gic-v3.h |  4 +-
 3 files changed, 48 insertions(+), 41 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 43d32fc..f033fd8 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -74,7 +74,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];
@@ -114,6 +114,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)
@@ -837,7 +838,7 @@ static void its_free_tables(struct its_node *its)
 	}
 }
 
-static int its_alloc_tables(const char *node_name, struct its_node *its)
+static int its_alloc_tables(struct its_node *its)
 {
 	int err;
 	int i;
@@ -893,8 +894,8 @@ static int its_alloc_tables(const char *node_name, struct its_node *its)
 				    order);
 			if (order >= MAX_ORDER) {
 				order = MAX_ORDER - 1;
-				pr_warn("%s: Device Table too large, reduce its page order to %u\n",
-					node_name, order);
+				pr_warn("ITS@%pa: Device Table too large, reduce its page order to %u\n",
+					&its->phys_base, order);
 			}
 		}
 
@@ -903,8 +904,8 @@ retry_alloc_baser:
 		if (alloc_pages > GITS_BASER_PAGES_MAX) {
 			alloc_pages = GITS_BASER_PAGES_MAX;
 			order = get_order(GITS_BASER_PAGES_MAX * psz);
-			pr_warn("%s: Device Table too large, reduce its page order to %u (%u pages)\n",
-				node_name, order, alloc_pages);
+			pr_warn("ITS@%pa: Device Table too large, reduce its page order to %u (%u pages)\n",
+				&its->phys_base, order, alloc_pages);
 		}
 
 		base = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, order);
@@ -978,8 +979,8 @@ retry_baser:
 		}
 
 		if (val != tmp) {
-			pr_err("ITS: %s: GITS_BASER%d doesn't stick: %lx %lx\n",
-			       node_name, i,
+			pr_err("ITS@%pa: GITS_BASER%d doesn't stick: %lx %lx\n",
+			       &its->phys_base, i,
 			       (unsigned long) val, (unsigned long) tmp);
 			err = -ENXIO;
 			goto out_free;
@@ -1507,8 +1508,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;
@@ -1517,13 +1517,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;
@@ -1532,43 +1532,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) {
@@ -1580,9 +1572,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) {
@@ -1593,7 +1585,7 @@ static int __init its_probe(struct device_node *node,
 
 	its_enable_quirks(its);
 
-	err = its_alloc_tables(node->full_name, its);
+	err = its_alloc_tables(its);
 	if (err)
 		goto out_free_cmd;
 
@@ -1629,7 +1621,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;
 
@@ -1647,7 +1639,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;
 }
 
@@ -1675,10 +1667,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)) {
@@ -1688,8 +1680,25 @@ 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);
 
 	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..019b442 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,9 @@ 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);
+	    to_of_node(handle)) /* Temp hack to prevent ITS init for ACPI */
+		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 dc493e0..2136639 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -334,9 +334,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

Re: [PATCH V6 4/7] irqchip/gicv3-its: Refator ITS DT init code to prepare for ACPI

[Marc Zyngier]

On Mon, 13 Jun 2016 16:41:10 +0200
Tomasz Nowicki <tn@semihalf.com> wrote:

s/refator/refactor/ in the subject line.

> 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. Fix ITS base register address type (from 'unsigned long' to 'phys_addr_t'),
> as a bonus we get nice string formatting.
> 2. 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>

This doesn't apply anymore since I've merged Shanker's 2-level
translation patches. Please rebase it on top of my irq/irqchip-4.8
branch.

> ---
>  drivers/irqchip/irq-gic-v3-its.c   | 79 +++++++++++++++++++++-----------------
>  drivers/irqchip/irq-gic-v3.c       |  6 +--
>  include/linux/irqchip/arm-gic-v3.h |  4 +-
>  3 files changed, 48 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 43d32fc..f033fd8 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -74,7 +74,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];
> @@ -114,6 +114,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)
> @@ -837,7 +838,7 @@ static void its_free_tables(struct its_node *its)
>  	}
>  }
>  
> -static int its_alloc_tables(const char *node_name, struct its_node *its)
> +static int its_alloc_tables(struct its_node *its)
>  {
>  	int err;
>  	int i;
> @@ -893,8 +894,8 @@ static int its_alloc_tables(const char *node_name, struct its_node *its)
>  				    order);
>  			if (order >= MAX_ORDER) {
>  				order = MAX_ORDER - 1;
> -				pr_warn("%s: Device Table too large, reduce its page order to %u\n",
> -					node_name, order);
> +				pr_warn("ITS@%pa: Device Table too large, reduce its page order to %u\n",
> +					&its->phys_base, order);
>  			}
>  		}
>  
> @@ -903,8 +904,8 @@ retry_alloc_baser:
>  		if (alloc_pages > GITS_BASER_PAGES_MAX) {
>  			alloc_pages = GITS_BASER_PAGES_MAX;
>  			order = get_order(GITS_BASER_PAGES_MAX * psz);
> -			pr_warn("%s: Device Table too large, reduce its page order to %u (%u pages)\n",
> -				node_name, order, alloc_pages);
> +			pr_warn("ITS@%pa: Device Table too large, reduce its page order to %u (%u pages)\n",
> +				&its->phys_base, order, alloc_pages);
>  		}
>  
>  		base = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, order);
> @@ -978,8 +979,8 @@ retry_baser:
>  		}
>  
>  		if (val != tmp) {
> -			pr_err("ITS: %s: GITS_BASER%d doesn't stick: %lx %lx\n",
> -			       node_name, i,
> +			pr_err("ITS@%pa: GITS_BASER%d doesn't stick: %lx %lx\n",
> +			       &its->phys_base, i,
>  			       (unsigned long) val, (unsigned long) tmp);
>  			err = -ENXIO;
>  			goto out_free;
> @@ -1507,8 +1508,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;
> @@ -1517,13 +1517,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;
> @@ -1532,43 +1532,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) {
> @@ -1580,9 +1572,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) {
> @@ -1593,7 +1585,7 @@ static int __init its_probe(struct device_node *node,
>  
>  	its_enable_quirks(its);
>  
> -	err = its_alloc_tables(node->full_name, its);
> +	err = its_alloc_tables(its);
>  	if (err)
>  		goto out_free_cmd;
>  
> @@ -1629,7 +1621,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;
>  
> @@ -1647,7 +1639,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;
>  }
>  
> @@ -1675,10 +1667,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)) {
> @@ -1688,8 +1680,25 @@ 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);

If you add an "else return -ENODEV;" here...

>  
>  	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..019b442 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,9 @@ 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);
> +	    to_of_node(handle)) /* Temp hack to prevent ITS init for ACPI */
> +		its_init(handle, &gic_data.rdists, gic_data.domain);

... you nuke this hack in this patch. The sooner the better.

>  
>  	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 dc493e0..2136639 100644
> --- a/include/linux/irqchip/arm-gic-v3.h
> +++ b/include/linux/irqchip/arm-gic-v3.h
> @@ -334,9 +334,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)

Thanks,

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

[PATCH V6 5/7] 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>
---
 drivers/irqchip/Kconfig          |  1 +
 drivers/irqchip/irq-gic-v3-its.c | 59 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index fa33c50..c6e652f 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -32,6 +32,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 f033fd8..7315263 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>
@@ -1336,6 +1339,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;
 	}
@@ -1690,6 +1698,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)
 {
@@ -1699,6 +1756,8 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
 	of_node = to_of_node(handle);
 	if (of_node)
 		its_of_probe(of_node);
+	else
+		its_acpi_probe();
 
 	if (list_empty(&its_nodes)) {
 		pr_warn("ITS: No ITS available, not enabling LPIs\n");
-- 
1.9.1

Re: [PATCH V6 5/7] irqchip/gicv3-its: Probe ITS in the ACPI way

[Marc Zyngier]

On Mon, 13 Jun 2016 16:41:11 +0200
Tomasz Nowicki <tn@semihalf.com> 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>

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

[PATCH V6 6/7] 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>
---
 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..23679ae 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

Re: [PATCH V6 6/7] irqchip/gicv3-its: Factor out code that might be reused for ACPI

[Marc Zyngier]

On Mon, 13 Jun 2016 16:41:12 +0200
Tomasz Nowicki <tn@semihalf.com> wrote:

> 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>

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

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

[Tomasz Nowicki]

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

We have now full PCI MSI domain stack, thus we can enable ITS initialization
from GICv3 core driver for ACPI scenario.

Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
 drivers/irqchip/irq-gic-v3-its-pci-msi.c | 46 +++++++++++++++++++++++++++++++-
 drivers/irqchip/irq-gic-v3.c             |  3 +--
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its-pci-msi.c b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
index 23679ae..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,10 +145,53 @@ 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;
 }
-
 early_initcall(its_pci_msi_init);
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 019b442..2a8b2e1 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -951,8 +951,7 @@ static int __init gic_init_bases(void __iomem *dist_base,
 
 	set_handle_irq(gic_handle_irq);
 
-	if (IS_ENABLED(CONFIG_ARM_GIC_V3_ITS) && gic_dist_supports_lpis() &&
-	    to_of_node(handle)) /* Temp hack to prevent ITS init for ACPI */
+	if (IS_ENABLED(CONFIG_ARM_GIC_V3_ITS) && gic_dist_supports_lpis())
 		its_init(handle, &gic_data.rdists, gic_data.domain);
 
 	gic_smp_init();
-- 
1.9.1

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

[Marc Zyngier]

On Mon, 13 Jun 2016 16:41:13 +0200
Tomasz Nowicki <tn@semihalf.com> wrote:

> Let ACPI build ITS PCI MSI domain. ACPI is responsible for retrieving
> inner domain token and passing it on to its_pci_msi_init_one generic
> init call.
> 
> We have now full PCI MSI domain stack, thus we can enable ITS initialization
> from GICv3 core driver for ACPI scenario.
> 
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

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

Re: [PATCH V6 0/7] Introduce ACPI world to ITS irqchip

[Marc Zyngier]

On Mon, 13 Jun 2016 16:41:06 +0200
Tomasz Nowicki <tn@semihalf.com> 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
> 
> Patches were built on top of accepted ARM64 ACPI PCI support and can be found
> all together here:
> https://github.com/semihalf-nowicki-tomasz/linux.git (its-acpi-v6)
> 
> Tested on Cavium ThunderX server.
> 
> 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 (7):
>   ACPI: I/O Remapping Table (IORT) initial support
>   PCI/MSI: Setup MSI domain on a per-devices basis using IORT ACPI table
>   irqchip/gicv3-its: Cleanup for ITS domain initialization
>   irqchip/gicv3-its: Refator 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/iort.c                      | 386 +++++++++++++++++++++++++++++++
>  drivers/irqchip/Kconfig                  |   1 +
>  drivers/irqchip/irq-gic-v3-its-pci-msi.c |  88 +++++--
>  drivers/irqchip/irq-gic-v3-its.c         | 185 ++++++++++-----
>  drivers/irqchip/irq-gic-v3.c             |   7 +-
>  drivers/pci/msi.c                        |  11 +-
>  include/linux/iort.h                     |  38 +++
>  include/linux/irqchip/arm-gic-v3.h       |   4 +-
>  10 files changed, 646 insertions(+), 78 deletions(-)
>  create mode 100644 drivers/acpi/iort.c
>  create mode 100644 include/linux/iort.h
> 


Once you've addressed the couple of remarks I had, and unless someone
has further objections, this should be in a sane state.

Of course, the following question would be "who merges this thing". I'm
happy to take it (mostly because it makes my life easier on the ITS
side), but it needs to be Acked by the usual suspects (Rafael, Bjorn,
Lorenzo).

Thanks,

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

Re: [PATCH V6 0/7] Introduce ACPI world to ITS irqchip

[Tomasz Nowicki]

On 15.06.2016 11:09, Marc Zyngier wrote:
> On Mon, 13 Jun 2016 16:41:06 +0200
> Tomasz Nowicki <tn@semihalf.com> 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
>>
>> Patches were built on top of accepted ARM64 ACPI PCI support and can be found
>> all together here:
>> https://github.com/semihalf-nowicki-tomasz/linux.git (its-acpi-v6)
>>
>> Tested on Cavium ThunderX server.
>>
>> 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 (7):
>>    ACPI: I/O Remapping Table (IORT) initial support
>>    PCI/MSI: Setup MSI domain on a per-devices basis using IORT ACPI table
>>    irqchip/gicv3-its: Cleanup for ITS domain initialization
>>    irqchip/gicv3-its: Refator 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/iort.c                      | 386 +++++++++++++++++++++++++++++++
>>   drivers/irqchip/Kconfig                  |   1 +
>>   drivers/irqchip/irq-gic-v3-its-pci-msi.c |  88 +++++--
>>   drivers/irqchip/irq-gic-v3-its.c         | 185 ++++++++++-----
>>   drivers/irqchip/irq-gic-v3.c             |   7 +-
>>   drivers/pci/msi.c                        |  11 +-
>>   include/linux/iort.h                     |  38 +++
>>   include/linux/irqchip/arm-gic-v3.h       |   4 +-
>>   10 files changed, 646 insertions(+), 78 deletions(-)
>>   create mode 100644 drivers/acpi/iort.c
>>   create mode 100644 include/linux/iort.h
>>
>
>
> Once you've addressed the couple of remarks I had, and unless someone
> has further objections, this should be in a sane state.

Thanks for having a look !

>
> Of course, the following question would be "who merges this thing". I'm
> happy to take it (mostly because it makes my life easier on the ITS
> side), but it needs to be Acked by the usual suspects (Rafael, Bjorn,
> Lorenzo).
>

As for me, I am fine you to merge it.

Bjorn, Rafael, Lorenzo,

Can you please check on respective patches? Thanks in advance.

Tomasz