* [PATCH V5 1/7] ARM64, ACPI, PCI: I/O Remapping Table (IORT) initial support.
2016-05-31 11:19 [PATCH V5 0/7] Introduce ACPI world to ITS irqchip Tomasz Nowicki
@ 2016-05-31 11:19 ` Tomasz Nowicki
2016-06-04 11:15 ` Marc Zyngier
2016-05-31 11:19 ` [PATCH V5 2/7] acpi, pci: Setup MSI domain on a per-devices basis Tomasz Nowicki
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Tomasz Nowicki @ 2016-05-31 11:19 UTC (permalink / raw)
To: marc.zyngier, tglx, jason, rjw, lorenzo.pieralisi,
robert.richter, shijie.huang, Suravee.Suthikulpanit, hanjun.guo
Cc: al.stone, mw, graeme.gregory, Catalin.Marinas, will.deacon,
linux-kernel, linux-acpi, linux-arm-kernel, ddaney.cavm, okaya,
andrea.gallo, 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 based on PCI device and requester ID
- find domain token based on PCI device and requester ID
Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
drivers/acpi/Kconfig | 3 +
drivers/acpi/Makefile | 1 +
drivers/acpi/iort.c | 344 ++++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/iort.h | 38 ++++++
4 files changed, 386 insertions(+)
create mode 100644 drivers/acpi/iort.c
create mode 100644 include/linux/iort.h
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index b7e2e77..848471f 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
config ACPI_CCA_REQUIRED
bool
+config IORT_TABLE
+ bool
+
config ACPI_DEBUGGER
bool "AML debugger interface"
select ACPI_DEBUG
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 251ce85..c7c9b29 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
obj-$(CONFIG_ACPI_BGRT) += bgrt.o
obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o
obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
+obj-$(CONFIG_IORT_TABLE) += iort.o
# processor has its own "processor." module_param namespace
processor-y := processor_driver.o
diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
new file mode 100644
index 0000000..226eb6d
--- /dev/null
+++ b/drivers/acpi/iort.c
@@ -0,0 +1,344 @@
+/*
+ * 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);
+
+/**
+ * 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 not 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;
+
+ list_add(&its_msi_chip->list, &iort_msi_chip_list);
+ return 0;
+}
+
+/**
+ * iort_unregister_domain_token() - unregister 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;
+
+ 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;
+ }
+ }
+}
+
+/**
+ * iort_find_its_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_its_find_domain_token(int trans_id)
+{
+ struct iort_its_msi_chip *its_msi_chip;
+
+ list_for_each_entry(its_msi_chip, &iort_msi_chip_list, list) {
+ if (its_msi_chip->translation_id == trans_id)
+ return its_msi_chip->fw_node;
+ }
+
+ return NULL;
+}
+
+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;
+
+ if (!iort_table)
+ return NULL;
+
+ /*
+ * iort_table and iort both point to the start of IORT table, but
+ * have different struct types
+ */
+ iort = (struct acpi_table_iort *)iort_table;
+
+ /* Get the first IORT node */
+ iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort,
+ iort->node_offset);
+ iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
+ iort_table->length);
+
+ for (i = 0; i < iort->node_count; i++) {
+ if (iort_node >= iort_end) {
+ pr_err("iort node pointer overflows, bad table\n");
+ return 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_find_dev_callback(struct acpi_iort_node *node, void *context)
+{
+ struct acpi_iort_root_complex *pci_rc;
+ struct device *dev = context;
+ struct pci_bus *bus;
+
+ switch (node->type) {
+ case ACPI_IORT_NODE_PCI_ROOT_COMPLEX:
+ 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_dev_map_rid(struct acpi_iort_node *node, u32 rid_in,
+ u32 *rid_out)
+{
+
+ if (!node)
+ goto out;
+
+ /* Go upstream */
+ while (node->type != ACPI_IORT_NODE_ITS_GROUP) {
+ 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_its_find_node_and_map_rid(struct pci_dev *pdev, u32 req_id, u32 *dev_id)
+{
+ struct pci_bus *pbus = pdev->bus;
+ struct acpi_iort_node *node;
+
+ /* Find a PCI root bus */
+ while (!pci_is_root_bus(pbus))
+ pbus = pbus->parent;
+
+
+ node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
+ iort_find_dev_callback, &pbus->dev);
+ if (!node) {
+ dev_err(&pdev->dev, "can't find related IORT node\n");
+ return NULL;
+ }
+
+ return iort_dev_map_rid(node, req_id, dev_id);
+}
+
+/**
+ * iort_pci_domain_get_msi_rid() - find MSI RID based on PCI device ID
+ * @pdev: The PCI device
+ * @req_id: The PCI device requester ID
+ *
+ * Returns: MSI RID on success, input requester ID otherwise
+ */
+u32 iort_pci_get_msi_rid(struct pci_dev *pdev, u32 req_id)
+{
+ u32 dev_id;
+
+ if (!iort_its_find_node_and_map_rid(pdev, req_id, &dev_id))
+ return req_id;
+
+ return dev_id;
+}
+
+/**
+ * iort_pci_find_its_id() - find the ITS identifier based on specified device.
+ * @pdev: The PCI device
+ * @idx: Index of the ITS identifier list
+ * @its_id: ITS identifier
+ *
+ * Returns: 0 on success, appropriate error value otherwise
+ */
+static int
+iort_pci_find_its_id(struct pci_dev *pdev, u32 req_id, unsigned int idx,
+ int *its_id)
+{
+ struct acpi_iort_its_group *its;
+ struct acpi_iort_node *node;
+
+ node = iort_its_find_node_and_map_rid(pdev, req_id, NULL);
+ if (!node)
+ return -ENXIO;
+
+ /* Move to ITS specific data */
+ its = (struct acpi_iort_its_group *)node->node_data;
+ if (idx > its->its_count) {
+ dev_err(&pdev->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_pci_get_msi_domain_handle() - find registered domain token related to
+ * PCI device
+ * @pdev: The PCI device
+ * @req_id: The PCI device requester ID
+ *
+ * Returns: the MSI domain for this device, NULL otherwise
+ */
+struct irq_domain *
+iort_pci_get_domain(struct pci_dev *pdev, u32 req_id)
+{
+ static struct fwnode_handle *handle;
+ int its_id;
+
+ if (iort_pci_find_its_id(pdev, req_id, 0, &its_id))
+ return NULL;
+
+ handle = iort_its_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)) {
+ 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..490ff4d
--- /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_its_find_domain_token(int trans_id);
+#ifdef CONFIG_IORT_TABLE
+u32 iort_pci_get_msi_rid(struct pci_dev *pdev, u32 req_id);
+struct irq_domain *iort_pci_get_domain(struct pci_dev *pdev, u32 req_id);
+#else
+static inline u32 iort_pci_get_msi_rid(struct pci_dev *pdev, u32 req_id)
+{ return req_id; }
+static inline struct irq_domain *
+iort_pci_get_domain(struct pci_dev *pdev, u32 req_id) { return NULL; }
+#endif
+
+#endif /* __IORT_H__ */
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH V5 1/7] ARM64, ACPI, PCI: I/O Remapping Table (IORT) initial support.
2016-05-31 11:19 ` [PATCH V5 1/7] ARM64, ACPI, PCI: I/O Remapping Table (IORT) initial support Tomasz Nowicki
@ 2016-06-04 11:15 ` Marc Zyngier
2016-06-07 14:34 ` Tomasz Nowicki
0 siblings, 1 reply; 15+ messages in thread
From: Marc Zyngier @ 2016-06-04 11:15 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: tglx, jason, rjw, lorenzo.pieralisi, robert.richter,
shijie.huang, Suravee.Suthikulpanit, hanjun.guo, al.stone, mw,
graeme.gregory, Catalin.Marinas, will.deacon, linux-kernel,
linux-acpi, linux-arm-kernel, ddaney.cavm, okaya, andrea.gallo
On Tue, 31 May 2016 13:19:38 +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 based on PCI device and requester ID
> - find domain token based on PCI device and requester ID
>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
> drivers/acpi/Kconfig | 3 +
> drivers/acpi/Makefile | 1 +
> drivers/acpi/iort.c | 344 ++++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/iort.h | 38 ++++++
> 4 files changed, 386 insertions(+)
> create mode 100644 drivers/acpi/iort.c
> create mode 100644 include/linux/iort.h
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index b7e2e77..848471f 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
> config ACPI_CCA_REQUIRED
> bool
>
> +config IORT_TABLE
> + bool
> +
> config ACPI_DEBUGGER
> bool "AML debugger interface"
> select ACPI_DEBUG
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 251ce85..c7c9b29 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
> obj-$(CONFIG_ACPI_BGRT) += bgrt.o
> obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o
> obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
> +obj-$(CONFIG_IORT_TABLE) += iort.o
>
> # processor has its own "processor." module_param namespace
> processor-y := processor_driver.o
> diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
> new file mode 100644
> index 0000000..226eb6d
> --- /dev/null
> +++ b/drivers/acpi/iort.c
> @@ -0,0 +1,344 @@
> +/*
> + * 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);
> +
> +/**
> + * 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 not 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;
> +
> + list_add(&its_msi_chip->list, &iort_msi_chip_list);
No locking? How do you handle concurrent accesses?
> + return 0;
> +}
> +
> +/**
> + * iort_unregister_domain_token() - unregister 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;
> +
> + 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;
> + }
> + }
> +}
Same here.
> +
> +/**
> + * iort_find_its_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_its_find_domain_token(int trans_id)
> +{
> + struct iort_its_msi_chip *its_msi_chip;
> +
> + list_for_each_entry(its_msi_chip, &iort_msi_chip_list, list) {
> + if (its_msi_chip->translation_id == trans_id)
> + return its_msi_chip->fw_node;
> + }
> +
> + return NULL;
> +}
Same here.
> +
> +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;
> +
> + if (!iort_table)
> + return NULL;
> +
> + /*
> + * iort_table and iort both point to the start of IORT table, but
> + * have different struct types
> + */
> + iort = (struct acpi_table_iort *)iort_table;
> +
> + /* Get the first IORT node */
> + iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort,
> + iort->node_offset);
> + iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> + iort_table->length);
> +
> + for (i = 0; i < iort->node_count; i++) {
> + if (iort_node >= iort_end) {
> + pr_err("iort node pointer overflows, bad table\n");
This probably deserves a WARN_ON, a TAINT_FIRMWARE_WORKAROUND and maybe
a TAINT_CRAP as an added injury.
> + 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_find_dev_callback(struct acpi_iort_node *node, void *context)
This is actually trying to match a node to a given device. How about
calling it iort_match_node_callback?
> +{
> + struct acpi_iort_root_complex *pci_rc;
> + struct device *dev = context;
> + struct pci_bus *bus;
> +
> + switch (node->type) {
> + case ACPI_IORT_NODE_PCI_ROOT_COMPLEX:
> + 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_dev_map_rid(struct acpi_iort_node *node, u32 rid_in,
> + u32 *rid_out)
Given that there is no "dev" involved in this functions, but only
nodes, consider renaming this to iort_node_map_rid.
> +{
> +
> + if (!node)
> + goto out;
> +
> + /* Go upstream */
> + while (node->type != ACPI_IORT_NODE_ITS_GROUP) {
> + 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);
> + }
Do we always want to resolve an ID from the device down to the last
possible transformation? While this works fine for the ITS (which is
supposed to be the last user of the RID), this may not work that well
for intermediate remapping elements (IOMMU, for example).
So I'm wondering if what we actually want is something that would say
iort_node_map_rid(from_node, to_node, rid_in, &rid_out)?
> +
> +out:
> + if (rid_out)
> + *rid_out = rid_in;
> + return node;
> +}
> +
> +static struct acpi_iort_node *
> +iort_its_find_node_and_map_rid(struct pci_dev *pdev, u32 req_id, u32 *dev_id)
Does this only applies to a PCI device?
> +{
> + struct pci_bus *pbus = pdev->bus;
> + struct acpi_iort_node *node;
> +
> + /* Find a PCI root bus */
> + while (!pci_is_root_bus(pbus))
> + pbus = pbus->parent;
> +
> +
> + node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
> + iort_find_dev_callback, &pbus->dev);
> + if (!node) {
> + dev_err(&pdev->dev, "can't find related IORT node\n");
> + return NULL;
> + }
> +
> + return iort_dev_map_rid(node, req_id, dev_id);
> +}
> +
> +/**
> + * iort_pci_domain_get_msi_rid() - find MSI RID based on PCI device ID
This doesn't match the name of the function.
> + * @pdev: The PCI device
> + * @req_id: The PCI device requester ID
> + *
> + * Returns: MSI RID on success, input requester ID otherwise
> + */
> +u32 iort_pci_get_msi_rid(struct pci_dev *pdev, u32 req_id)
> +{
> + u32 dev_id;
> +
> + if (!iort_its_find_node_and_map_rid(pdev, req_id, &dev_id))
> + return req_id;
> +
> + return dev_id;
> +}
> +
> +/**
> + * iort_pci_find_its_id() - find the ITS identifier based on specified device.
> + * @pdev: The PCI device
> + * @idx: Index of the ITS identifier list
> + * @its_id: ITS identifier
> + *
> + * Returns: 0 on success, appropriate error value otherwise
> + */
> +static int
> +iort_pci_find_its_id(struct pci_dev *pdev, u32 req_id, unsigned int idx,
> + int *its_id)
> +{
> + struct acpi_iort_its_group *its;
> + struct acpi_iort_node *node;
> +
> + node = iort_its_find_node_and_map_rid(pdev, req_id, NULL);
> + if (!node)
> + return -ENXIO;
> +
> + /* Move to ITS specific data */
> + its = (struct acpi_iort_its_group *)node->node_data;
> + if (idx > its->its_count) {
> + dev_err(&pdev->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_pci_get_msi_domain_handle() - find registered domain token related to
> + * PCI device
Broken comment.
> + * @pdev: The PCI device
> + * @req_id: The PCI device requester ID
> + *
> + * Returns: the MSI domain for this device, NULL otherwise
> + */
> +struct irq_domain *
> +iort_pci_get_domain(struct pci_dev *pdev, u32 req_id)
Again: does this have to be PCI specific?
> +{
> + static struct fwnode_handle *handle;
> + int its_id;
> +
> + if (iort_pci_find_its_id(pdev, req_id, 0, &its_id))
> + return NULL;
> +
> + handle = iort_its_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)) {
> + 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..490ff4d
> --- /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_its_find_domain_token(int trans_id);
> +#ifdef CONFIG_IORT_TABLE
> +u32 iort_pci_get_msi_rid(struct pci_dev *pdev, u32 req_id);
> +struct irq_domain *iort_pci_get_domain(struct pci_dev *pdev, u32 req_id);
> +#else
> +static inline u32 iort_pci_get_msi_rid(struct pci_dev *pdev, u32 req_id)
> +{ return req_id; }
> +static inline struct irq_domain *
> +iort_pci_get_domain(struct pci_dev *pdev, u32 req_id) { return NULL; }
> +#endif
> +
> +#endif /* __IORT_H__ */
Thanks,
M.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH V5 1/7] ARM64, ACPI, PCI: I/O Remapping Table (IORT) initial support.
2016-06-04 11:15 ` Marc Zyngier
@ 2016-06-07 14:34 ` Tomasz Nowicki
2016-06-07 16:25 ` Marc Zyngier
0 siblings, 1 reply; 15+ messages in thread
From: Tomasz Nowicki @ 2016-06-07 14:34 UTC (permalink / raw)
To: Marc Zyngier
Cc: tglx, jason, rjw, lorenzo.pieralisi, robert.richter,
shijie.huang, Suravee.Suthikulpanit, hanjun.guo, al.stone, mw,
graeme.gregory, Catalin.Marinas, will.deacon, linux-kernel,
linux-acpi, linux-arm-kernel, ddaney.cavm, okaya, andrea.gallo
On 04.06.2016 13:15, Marc Zyngier wrote:
> On Tue, 31 May 2016 13:19:38 +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 based on PCI device and requester ID
>> - find domain token based on PCI device and requester ID
>>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> ---
>> drivers/acpi/Kconfig | 3 +
>> drivers/acpi/Makefile | 1 +
>> drivers/acpi/iort.c | 344 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/iort.h | 38 ++++++
>> 4 files changed, 386 insertions(+)
>> create mode 100644 drivers/acpi/iort.c
>> create mode 100644 include/linux/iort.h
>>
>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> index b7e2e77..848471f 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
>> config ACPI_CCA_REQUIRED
>> bool
>>
>> +config IORT_TABLE
>> + bool
>> +
>> config ACPI_DEBUGGER
>> bool "AML debugger interface"
>> select ACPI_DEBUG
>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>> index 251ce85..c7c9b29 100644
>> --- a/drivers/acpi/Makefile
>> +++ b/drivers/acpi/Makefile
>> @@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
>> obj-$(CONFIG_ACPI_BGRT) += bgrt.o
>> obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o
>> obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
>> +obj-$(CONFIG_IORT_TABLE) += iort.o
>>
>> # processor has its own "processor." module_param namespace
>> processor-y := processor_driver.o
>> diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
>> new file mode 100644
>> index 0000000..226eb6d
>> --- /dev/null
>> +++ b/drivers/acpi/iort.c
>> @@ -0,0 +1,344 @@
>> +/*
>> + * 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);
>> +
>> +/**
>> + * 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 not 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;
>> +
>> + list_add(&its_msi_chip->list, &iort_msi_chip_list);
>
> No locking? How do you handle concurrent accesses?
I wandered if we need locking here but at the end I did not find
worst-case scenario.
1. Adding elements to list is done in first place here (later on list is
not modified):
start_kernel -> init_IRQ -> [...] - > gic_acpi_parse_madt_its ->
iort_register_domain_token
2. Then we only retrieving elements form list:
start_kernel -> rest_init -> kernel_init -> [...] -> do_initcalls ->
its_pci_msi_init -> its_pci_acpi_msi_init -> iort_its_find_domain_token
pci_set_msi_domain -> iort_get_device_domain -> iort_its_find_domain_token
Do you mean some specific case?
>
>> + return 0;
>> +}
>> +
>> +/**
>> + * iort_unregister_domain_token() - unregister 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;
>> +
>> + 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;
>> + }
>> + }
>> +}
>
> Same here.
>
>> +
>> +/**
>> + * iort_find_its_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_its_find_domain_token(int trans_id)
>> +{
>> + struct iort_its_msi_chip *its_msi_chip;
>> +
>> + list_for_each_entry(its_msi_chip, &iort_msi_chip_list, list) {
>> + if (its_msi_chip->translation_id == trans_id)
>> + return its_msi_chip->fw_node;
>> + }
>> +
>> + return NULL;
>> +}
>
> Same here.
>
>> +
>> +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;
>> +
>> + if (!iort_table)
>> + return NULL;
>> +
>> + /*
>> + * iort_table and iort both point to the start of IORT table, but
>> + * have different struct types
>> + */
>> + iort = (struct acpi_table_iort *)iort_table;
>> +
>> + /* Get the first IORT node */
>> + iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort,
>> + iort->node_offset);
>> + iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
>> + iort_table->length);
>> +
>> + for (i = 0; i < iort->node_count; i++) {
>> + if (iort_node >= iort_end) {
>> + pr_err("iort node pointer overflows, bad table\n");
>
> This probably deserves a WARN_ON, a TAINT_FIRMWARE_WORKAROUND and maybe
> a TAINT_CRAP as an added injury.
I think we can use similar solution as in dmar.c
if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND,
"IORT node pointer overflows, bad table\n"))
return NULL;
>
>> + 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_find_dev_callback(struct acpi_iort_node *node, void *context)
>
> This is actually trying to match a node to a given device. How about
> calling it iort_match_node_callback?
Yeah, sounds good to me.
>
>> +{
>> + struct acpi_iort_root_complex *pci_rc;
>> + struct device *dev = context;
>> + struct pci_bus *bus;
>> +
>> + switch (node->type) {
>> + case ACPI_IORT_NODE_PCI_ROOT_COMPLEX:
>> + 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_dev_map_rid(struct acpi_iort_node *node, u32 rid_in,
>> + u32 *rid_out)
>
> Given that there is no "dev" involved in this functions, but only
> nodes, consider renaming this to iort_node_map_rid.
+1
>
>> +{
>> +
>> + if (!node)
>> + goto out;
>> +
>> + /* Go upstream */
>> + while (node->type != ACPI_IORT_NODE_ITS_GROUP) {
>> + 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);
>> + }
>
> Do we always want to resolve an ID from the device down to the last
> possible transformation? While this works fine for the ITS (which is
> supposed to be the last user of the RID), this may not work that well
> for intermediate remapping elements (IOMMU, for example).
>
> So I'm wondering if what we actually want is something that would say
> iort_node_map_rid(from_node, to_node, rid_in, &rid_out)?
Good point. Actually Lorenzo improved that function in his SMMU ACPI
series addressing your comment. So we can make it more generic from day one.
>
>> +
>> +out:
>> + if (rid_out)
>> + *rid_out = rid_in;
>> + return node;
>> +}
>> +
>> +static struct acpi_iort_node *
>> +iort_its_find_node_and_map_rid(struct pci_dev *pdev, u32 req_id, u32 *dev_id)
>
> Does this only applies to a PCI device?
No, it does not. I will use struct device instead.
>
>> +{
>> + struct pci_bus *pbus = pdev->bus;
>> + struct acpi_iort_node *node;
>> +
>> + /* Find a PCI root bus */
>> + while (!pci_is_root_bus(pbus))
>> + pbus = pbus->parent;
>> +
>> +
>> + node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
>> + iort_find_dev_callback, &pbus->dev);
>> + if (!node) {
>> + dev_err(&pdev->dev, "can't find related IORT node\n");
>> + return NULL;
>> + }
>> +
>> + return iort_dev_map_rid(node, req_id, dev_id);
>> +}
>> +
>> +/**
>> + * iort_pci_domain_get_msi_rid() - find MSI RID based on PCI device ID
>
> This doesn't match the name of the function.
>
>> + * @pdev: The PCI device
>> + * @req_id: The PCI device requester ID
>> + *
>> + * Returns: MSI RID on success, input requester ID otherwise
>> + */
>> +u32 iort_pci_get_msi_rid(struct pci_dev *pdev, u32 req_id)
>> +{
>> + u32 dev_id;
>> +
>> + if (!iort_its_find_node_and_map_rid(pdev, req_id, &dev_id))
>> + return req_id;
>> +
>> + return dev_id;
>> +}
>> +
>> +/**
>> + * iort_pci_find_its_id() - find the ITS identifier based on specified device.
>> + * @pdev: The PCI device
>> + * @idx: Index of the ITS identifier list
>> + * @its_id: ITS identifier
>> + *
>> + * Returns: 0 on success, appropriate error value otherwise
>> + */
>> +static int
>> +iort_pci_find_its_id(struct pci_dev *pdev, u32 req_id, unsigned int idx,
>> + int *its_id)
>> +{
>> + struct acpi_iort_its_group *its;
>> + struct acpi_iort_node *node;
>> +
>> + node = iort_its_find_node_and_map_rid(pdev, req_id, NULL);
>> + if (!node)
>> + return -ENXIO;
>> +
>> + /* Move to ITS specific data */
>> + its = (struct acpi_iort_its_group *)node->node_data;
>> + if (idx > its->its_count) {
>> + dev_err(&pdev->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_pci_get_msi_domain_handle() - find registered domain token related to
>> + * PCI device
>
> Broken comment.
Will fix.
>
>> + * @pdev: The PCI device
>> + * @req_id: The PCI device requester ID
>> + *
>> + * Returns: the MSI domain for this device, NULL otherwise
>> + */
>> +struct irq_domain *
>> +iort_pci_get_domain(struct pci_dev *pdev, u32 req_id)
>
> Again: does this have to be PCI specific?
Here struct device can be used as well.
Thanks,
Tomasz
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH V5 1/7] ARM64, ACPI, PCI: I/O Remapping Table (IORT) initial support.
2016-06-07 14:34 ` Tomasz Nowicki
@ 2016-06-07 16:25 ` Marc Zyngier
0 siblings, 0 replies; 15+ messages in thread
From: Marc Zyngier @ 2016-06-07 16:25 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: tglx, jason, rjw, lorenzo.pieralisi, robert.richter,
shijie.huang, Suravee.Suthikulpanit, hanjun.guo, al.stone, mw,
graeme.gregory, Catalin.Marinas, will.deacon, linux-kernel,
linux-acpi, linux-arm-kernel, ddaney.cavm, okaya, andrea.gallo
On 07/06/16 15:34, Tomasz Nowicki wrote:
> On 04.06.2016 13:15, Marc Zyngier wrote:
>> On Tue, 31 May 2016 13:19:38 +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 based on PCI device and requester ID
>>> - find domain token based on PCI device and requester ID
>>>
>>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>>> ---
>>> drivers/acpi/Kconfig | 3 +
>>> drivers/acpi/Makefile | 1 +
>>> drivers/acpi/iort.c | 344 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/iort.h | 38 ++++++
>>> 4 files changed, 386 insertions(+)
>>> create mode 100644 drivers/acpi/iort.c
>>> create mode 100644 include/linux/iort.h
>>>
>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>> index b7e2e77..848471f 100644
>>> --- a/drivers/acpi/Kconfig
>>> +++ b/drivers/acpi/Kconfig
>>> @@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
>>> config ACPI_CCA_REQUIRED
>>> bool
>>>
>>> +config IORT_TABLE
>>> + bool
>>> +
>>> config ACPI_DEBUGGER
>>> bool "AML debugger interface"
>>> select ACPI_DEBUG
>>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>>> index 251ce85..c7c9b29 100644
>>> --- a/drivers/acpi/Makefile
>>> +++ b/drivers/acpi/Makefile
>>> @@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
>>> obj-$(CONFIG_ACPI_BGRT) += bgrt.o
>>> obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o
>>> obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
>>> +obj-$(CONFIG_IORT_TABLE) += iort.o
>>>
>>> # processor has its own "processor." module_param namespace
>>> processor-y := processor_driver.o
>>> diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
>>> new file mode 100644
>>> index 0000000..226eb6d
>>> --- /dev/null
>>> +++ b/drivers/acpi/iort.c
>>> @@ -0,0 +1,344 @@
>>> +/*
>>> + * 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);
>>> +
>>> +/**
>>> + * 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 not 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;
>>> +
>>> + list_add(&its_msi_chip->list, &iort_msi_chip_list);
>>
>> No locking? How do you handle concurrent accesses?
>
> I wandered if we need locking here but at the end I did not find
> worst-case scenario.
>
> 1. Adding elements to list is done in first place here (later on list is
> not modified):
> start_kernel -> init_IRQ -> [...] - > gic_acpi_parse_madt_its ->
> iort_register_domain_token
>
> 2. Then we only retrieving elements form list:
>
> start_kernel -> rest_init -> kernel_init -> [...] -> do_initcalls ->
> its_pci_msi_init -> its_pci_acpi_msi_init -> iort_its_find_domain_token
>
> pci_set_msi_domain -> iort_get_device_domain -> iort_its_find_domain_token
>
> Do you mean some specific case?
Not right now, but as a matter of principle, shared data structures
should be protected (law of minimal surprise). And that will still work
if someone comes up with a fancy hot-pluggable socket that has an ITS on it.
[...]
>>> +static struct acpi_iort_node *
>>> +iort_dev_map_rid(struct acpi_iort_node *node, u32 rid_in,
>>> + u32 *rid_out)
>>
>> Given that there is no "dev" involved in this functions, but only
>> nodes, consider renaming this to iort_node_map_rid.
>
> +1
>
>>
>>> +{
>>> +
>>> + if (!node)
>>> + goto out;
>>> +
>>> + /* Go upstream */
>>> + while (node->type != ACPI_IORT_NODE_ITS_GROUP) {
>>> + 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);
>>> + }
>>
>> Do we always want to resolve an ID from the device down to the last
>> possible transformation? While this works fine for the ITS (which is
>> supposed to be the last user of the RID), this may not work that well
>> for intermediate remapping elements (IOMMU, for example).
>>
>> So I'm wondering if what we actually want is something that would say
>> iort_node_map_rid(from_node, to_node, rid_in, &rid_out)?
>
> Good point. Actually Lorenzo improved that function in his SMMU ACPI
> series addressing your comment. So we can make it more generic from day one.
Indeed. He also has a couple of fixes that you could directly include in
the next drop.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH V5 2/7] acpi, pci: Setup MSI domain on a per-devices basis.
2016-05-31 11:19 [PATCH V5 0/7] Introduce ACPI world to ITS irqchip Tomasz Nowicki
2016-05-31 11:19 ` [PATCH V5 1/7] ARM64, ACPI, PCI: I/O Remapping Table (IORT) initial support Tomasz Nowicki
@ 2016-05-31 11:19 ` Tomasz Nowicki
2016-05-31 11:19 ` [PATCH V5 3/7] irqchip, GICv3, ITS: Cleanup for ITS domain initialization Tomasz Nowicki
` (4 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Tomasz Nowicki @ 2016-05-31 11:19 UTC (permalink / raw)
To: marc.zyngier, tglx, jason, rjw, lorenzo.pieralisi,
robert.richter, shijie.huang, Suravee.Suthikulpanit, hanjun.guo
Cc: al.stone, mw, graeme.gregory, Catalin.Marinas, will.deacon,
linux-kernel, linux-acpi, linux-arm-kernel, ddaney.cavm, okaya,
andrea.gallo, 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..ba49df7 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_pci_get_msi_rid(pdev, 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_pci_get_domain(pdev, rid);
+ return dom;
}
#endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH V5 3/7] irqchip, GICv3, ITS: Cleanup for ITS domain initialization.
2016-05-31 11:19 [PATCH V5 0/7] Introduce ACPI world to ITS irqchip Tomasz Nowicki
2016-05-31 11:19 ` [PATCH V5 1/7] ARM64, ACPI, PCI: I/O Remapping Table (IORT) initial support Tomasz Nowicki
2016-05-31 11:19 ` [PATCH V5 2/7] acpi, pci: Setup MSI domain on a per-devices basis Tomasz Nowicki
@ 2016-05-31 11:19 ` Tomasz Nowicki
2016-05-31 11:19 ` [PATCH V5 4/7] irqchip, GICv3, ITS: Refator ITS DT init code to prepare for ACPI Tomasz Nowicki
` (3 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Tomasz Nowicki @ 2016-05-31 11:19 UTC (permalink / raw)
To: marc.zyngier, tglx, jason, rjw, lorenzo.pieralisi,
robert.richter, shijie.huang, Suravee.Suthikulpanit, hanjun.guo
Cc: al.stone, mw, graeme.gregory, Catalin.Marinas, will.deacon,
linux-kernel, linux-acpi, linux-arm-kernel, ddaney.cavm, okaya,
andrea.gallo, 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 6bd881b..cb33c92 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1463,13 +1463,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;
@@ -1560,28 +1584,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);
@@ -1632,6 +1637,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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH V5 4/7] irqchip, GICv3, ITS: Refator ITS DT init code to prepare for ACPI.
2016-05-31 11:19 [PATCH V5 0/7] Introduce ACPI world to ITS irqchip Tomasz Nowicki
` (2 preceding siblings ...)
2016-05-31 11:19 ` [PATCH V5 3/7] irqchip, GICv3, ITS: Cleanup for ITS domain initialization Tomasz Nowicki
@ 2016-05-31 11:19 ` Tomasz Nowicki
2016-05-31 11:19 ` [PATCH V5 5/7] irqchip, gicv3, its: Probe ITS in the ACPI way Tomasz Nowicki
` (2 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Tomasz Nowicki @ 2016-05-31 11:19 UTC (permalink / raw)
To: marc.zyngier, tglx, jason, rjw, lorenzo.pieralisi,
robert.richter, shijie.huang, Suravee.Suthikulpanit, hanjun.guo
Cc: al.stone, mw, graeme.gregory, Catalin.Marinas, will.deacon,
linux-kernel, linux-acpi, linux-arm-kernel, ddaney.cavm, okaya,
andrea.gallo, 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 | 77 +++++++++++++++++++++-----------------
drivers/irqchip/irq-gic-v3.c | 6 +--
include/linux/irqchip/arm-gic-v3.h | 4 +-
3 files changed, 47 insertions(+), 40 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index cb33c92..6a97ee5 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -73,7 +73,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];
@@ -112,6 +112,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)
@@ -823,7 +824,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;
@@ -879,8 +880,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);
}
}
@@ -889,8 +890,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);
@@ -964,8 +965,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;
@@ -1463,8 +1464,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;
@@ -1473,13 +1473,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;
@@ -1488,43 +1488,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)
{
- 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) {
@@ -1536,7 +1528,7 @@ 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->cmd_base = kzalloc(ITS_CMD_QUEUE_SZ, GFP_KERNEL);
@@ -1548,7 +1540,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;
@@ -1584,7 +1576,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;
@@ -1602,7 +1594,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;
}
@@ -1630,10 +1622,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)) {
@@ -1643,8 +1635,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);
}
+ 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 fb042ba..cee8296 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 bfbd707..0e75cb7 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH V5 5/7] irqchip, gicv3, its: Probe ITS in the ACPI way.
2016-05-31 11:19 [PATCH V5 0/7] Introduce ACPI world to ITS irqchip Tomasz Nowicki
` (3 preceding siblings ...)
2016-05-31 11:19 ` [PATCH V5 4/7] irqchip, GICv3, ITS: Refator ITS DT init code to prepare for ACPI Tomasz Nowicki
@ 2016-05-31 11:19 ` Tomasz Nowicki
2016-06-04 11:55 ` Marc Zyngier
2016-05-31 11:19 ` [PATCH V5 6/7] its, pci, msi: Factor out code that might be reused for ACPI Tomasz Nowicki
2016-05-31 11:19 ` [PATCH V5 7/7] acpi, gicv3, its: Use MADT ITS subtable to do PCI/MSI domain initialization Tomasz Nowicki
6 siblings, 1 reply; 15+ messages in thread
From: Tomasz Nowicki @ 2016-05-31 11:19 UTC (permalink / raw)
To: marc.zyngier, tglx, jason, rjw, lorenzo.pieralisi,
robert.richter, shijie.huang, Suravee.Suthikulpanit, hanjun.guo
Cc: al.stone, mw, graeme.gregory, Catalin.Marinas, will.deacon,
linux-kernel, linux-acpi, linux-arm-kernel, ddaney.cavm, okaya,
andrea.gallo, 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.
Note that we are using 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.
Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
---
drivers/irqchip/Kconfig | 1 +
drivers/irqchip/irq-gic-v3-its.c | 60 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 61 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 6a97ee5..c8f36a5 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>
@@ -1312,6 +1315,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;
}
@@ -1645,6 +1653,56 @@ static int __init its_of_probe(struct device_node *node)
return 0;
}
+#ifdef CONFIG_ACPI
+
+#define ACPI_GICV3_ITS_MEM_SIZE (2 * SZ_64K)
+
+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);
+ if (!err)
+ return 0;
+
+ iort_deregister_domain_token(its_entry->translation_id);
+dom_err:
+ irq_domain_free_fwnode(dom_handle);
+ return err;
+}
+
+void __init its_acpi_probe(void)
+{
+ if (acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
+ gic_acpi_parse_madt_its, 0) <= 0)
+ pr_info("No valid GIC ITS entries exist\n");
+}
+#else
+static inline void __init its_acpi_probe(struct irq_domain *parent_domain) { }
+#endif
+
int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
struct irq_domain *parent_domain)
{
@@ -1654,6 +1712,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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH V5 5/7] irqchip, gicv3, its: Probe ITS in the ACPI way.
2016-05-31 11:19 ` [PATCH V5 5/7] irqchip, gicv3, its: Probe ITS in the ACPI way Tomasz Nowicki
@ 2016-06-04 11:55 ` Marc Zyngier
2016-06-07 16:07 ` Tomasz Nowicki
0 siblings, 1 reply; 15+ messages in thread
From: Marc Zyngier @ 2016-06-04 11:55 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: tglx, jason, rjw, lorenzo.pieralisi, robert.richter,
shijie.huang, Suravee.Suthikulpanit, hanjun.guo, al.stone, mw,
graeme.gregory, Catalin.Marinas, will.deacon, linux-kernel,
linux-acpi, linux-arm-kernel, ddaney.cavm, okaya, andrea.gallo
On Tue, 31 May 2016 13:19:42 +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.
>
> Note that we are using 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.
>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> ---
> drivers/irqchip/Kconfig | 1 +
> drivers/irqchip/irq-gic-v3-its.c | 60 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 61 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 6a97ee5..c8f36a5 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>
> @@ -1312,6 +1315,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;
> }
> @@ -1645,6 +1653,56 @@ static int __init its_of_probe(struct device_node *node)
> return 0;
> }
>
> +#ifdef CONFIG_ACPI
> +
> +#define ACPI_GICV3_ITS_MEM_SIZE (2 * SZ_64K)
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);
> + if (!err)
> + return 0;
> +
> + iort_deregister_domain_token(its_entry->translation_id);
> +dom_err:
> + irq_domain_free_fwnode(dom_handle);
> + return err;
> +}
> +
> +void __init its_acpi_probe(void)
Should be static.
> +{
> + if (acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
> + gic_acpi_parse_madt_its, 0) <= 0)
> + pr_info("No valid GIC ITS entries exist\n");
We already have a message further down, no need to shout twice.
> +}
> +#else
> +static inline void __init its_acpi_probe(struct irq_domain *parent_domain) { }
Drop the inline, the compiler can figure out that by itself.
> +#endif
> +
> int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
> struct irq_domain *parent_domain)
> {
> @@ -1654,6 +1712,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");
Thanks,
M.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH V5 5/7] irqchip, gicv3, its: Probe ITS in the ACPI way.
2016-06-04 11:55 ` Marc Zyngier
@ 2016-06-07 16:07 ` Tomasz Nowicki
0 siblings, 0 replies; 15+ messages in thread
From: Tomasz Nowicki @ 2016-06-07 16:07 UTC (permalink / raw)
To: Marc Zyngier
Cc: tglx, jason, rjw, lorenzo.pieralisi, robert.richter,
shijie.huang, Suravee.Suthikulpanit, hanjun.guo, al.stone, mw,
graeme.gregory, Catalin.Marinas, will.deacon, linux-kernel,
linux-acpi, linux-arm-kernel, ddaney.cavm, okaya, andrea.gallo
On 04.06.2016 13:55, Marc Zyngier wrote:
> On Tue, 31 May 2016 13:19:42 +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.
>>
>> Note that we are using 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.
>>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
[...]
>> }
>>
>> +#ifdef CONFIG_ACPI
>> +
>> +#define ACPI_GICV3_ITS_MEM_SIZE (2 * SZ_64K)
>
> 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);
>> + if (!err)
>> + return 0;
>> +
>> + iort_deregister_domain_token(its_entry->translation_id);
>> +dom_err:
>> + irq_domain_free_fwnode(dom_handle);
>> + return err;
>> +}
>> +
>> +void __init its_acpi_probe(void)
>
> Should be static.
>
>> +{
>> + if (acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_TRANSLATOR,
>> + gic_acpi_parse_madt_its, 0) <= 0)
>> + pr_info("No valid GIC ITS entries exist\n");
>
> We already have a message further down, no need to shout twice.
>
>> +}
>> +#else
>> +static inline void __init its_acpi_probe(struct irq_domain *parent_domain) { }
>
> Drop the inline, the compiler can figure out that by itself.
>
I will address all above comments in next series.
Tomasz
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH V5 6/7] its, pci, msi: Factor out code that might be reused for ACPI.
2016-05-31 11:19 [PATCH V5 0/7] Introduce ACPI world to ITS irqchip Tomasz Nowicki
` (4 preceding siblings ...)
2016-05-31 11:19 ` [PATCH V5 5/7] irqchip, gicv3, its: Probe ITS in the ACPI way Tomasz Nowicki
@ 2016-05-31 11:19 ` Tomasz Nowicki
2016-05-31 11:19 ` [PATCH V5 7/7] acpi, gicv3, its: Use MADT ITS subtable to do PCI/MSI domain initialization Tomasz Nowicki
6 siblings, 0 replies; 15+ messages in thread
From: Tomasz Nowicki @ 2016-05-31 11:19 UTC (permalink / raw)
To: marc.zyngier, tglx, jason, rjw, lorenzo.pieralisi,
robert.richter, shijie.huang, Suravee.Suthikulpanit, hanjun.guo
Cc: al.stone, mw, graeme.gregory, Catalin.Marinas, will.deacon,
linux-kernel, linux-acpi, linux-arm-kernel, ddaney.cavm, okaya,
andrea.gallo, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH V5 7/7] acpi, gicv3, its: Use MADT ITS subtable to do PCI/MSI domain initialization.
2016-05-31 11:19 [PATCH V5 0/7] Introduce ACPI world to ITS irqchip Tomasz Nowicki
` (5 preceding siblings ...)
2016-05-31 11:19 ` [PATCH V5 6/7] its, pci, msi: Factor out code that might be reused for ACPI Tomasz Nowicki
@ 2016-05-31 11:19 ` Tomasz Nowicki
2016-06-04 12:07 ` Marc Zyngier
6 siblings, 1 reply; 15+ messages in thread
From: Tomasz Nowicki @ 2016-05-31 11:19 UTC (permalink / raw)
To: marc.zyngier, tglx, jason, rjw, lorenzo.pieralisi,
robert.richter, shijie.huang, Suravee.Suthikulpanit, hanjun.guo
Cc: al.stone, mw, graeme.gregory, Catalin.Marinas, will.deacon,
linux-kernel, linux-acpi, linux-arm-kernel, ddaney.cavm, okaya,
andrea.gallo, 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 | 42 +++++++++++++++++++++++++++++++-
drivers/irqchip/irq-gic-v3.c | 3 +--
2 files changed, 42 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..985f5d9 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,49 @@ 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;
+ char node_name[6 + 16 + 1]; // ITS identification name
+
+ its_entry = (struct acpi_madt_generic_translator *)header;
+ snprintf(node_name, 23, "ITS@0x%lx", (long)its_entry->base_address);
+ dom_handle = iort_its_find_domain_token(its_entry->translation_id);
+ if (!dom_handle) {
+ pr_err("%s: Unable to locate ITS domain handle\n", node_name);
+ return 0;
+ }
+
+ if (its_pci_msi_init_one(dom_handle, node_name))
+ return 0;
+
+ pr_info("PCI/MSI: %s domain created\n", node_name);
+ return 0;
+}
+
+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
+inline 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 cee8296..41c1cda 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH V5 7/7] acpi, gicv3, its: Use MADT ITS subtable to do PCI/MSI domain initialization.
2016-05-31 11:19 ` [PATCH V5 7/7] acpi, gicv3, its: Use MADT ITS subtable to do PCI/MSI domain initialization Tomasz Nowicki
@ 2016-06-04 12:07 ` Marc Zyngier
2016-06-07 16:12 ` Tomasz Nowicki
0 siblings, 1 reply; 15+ messages in thread
From: Marc Zyngier @ 2016-06-04 12:07 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: tglx, jason, rjw, lorenzo.pieralisi, robert.richter,
shijie.huang, Suravee.Suthikulpanit, hanjun.guo, al.stone, mw,
graeme.gregory, Catalin.Marinas, will.deacon, linux-kernel,
linux-acpi, linux-arm-kernel, ddaney.cavm, okaya, andrea.gallo
On Tue, 31 May 2016 13:19:44 +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>
> ---
> drivers/irqchip/irq-gic-v3-its-pci-msi.c | 42 +++++++++++++++++++++++++++++++-
> drivers/irqchip/irq-gic-v3.c | 3 +--
> 2 files changed, 42 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..985f5d9 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,49 @@ 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;
> + char node_name[6 + 16 + 1]; // ITS identification name
Jeezus... Where is that coming from? Is that the size of "ITS@0x"+16
hex digits and a final NUL? For the sake of keeping things
maintainable/readable, please use kasprintf/kfree.
> +
> + its_entry = (struct acpi_madt_generic_translator *)header;
> + snprintf(node_name, 23, "ITS@0x%lx", (long)its_entry->base_address);
> + dom_handle = iort_its_find_domain_token(its_entry->translation_id);
> + if (!dom_handle) {
> + pr_err("%s: Unable to locate ITS domain handle\n", node_name);
> + return 0;
return 0? not even a slap on the wrist? Hum...
> + }
> +
> + if (its_pci_msi_init_one(dom_handle, node_name))
> + return 0;
Same here.
> +
> + pr_info("PCI/MSI: %s domain created\n", node_name);
> + return 0;
> +}
> +
> +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
> +inline static int __init its_pci_acpi_msi_init(void)
static only.
> +{
> + 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 cee8296..41c1cda 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();
Thanks,
M.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH V5 7/7] acpi, gicv3, its: Use MADT ITS subtable to do PCI/MSI domain initialization.
2016-06-04 12:07 ` Marc Zyngier
@ 2016-06-07 16:12 ` Tomasz Nowicki
0 siblings, 0 replies; 15+ messages in thread
From: Tomasz Nowicki @ 2016-06-07 16:12 UTC (permalink / raw)
To: Marc Zyngier
Cc: tglx, jason, rjw, lorenzo.pieralisi, robert.richter,
shijie.huang, Suravee.Suthikulpanit, hanjun.guo, al.stone, mw,
graeme.gregory, Catalin.Marinas, will.deacon, linux-kernel,
linux-acpi, linux-arm-kernel, ddaney.cavm, okaya, andrea.gallo
On 04.06.2016 14:07, Marc Zyngier wrote:
> On Tue, 31 May 2016 13:19:44 +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>
>> ---
>> drivers/irqchip/irq-gic-v3-its-pci-msi.c | 42 +++++++++++++++++++++++++++++++-
>> drivers/irqchip/irq-gic-v3.c | 3 +--
>> 2 files changed, 42 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..985f5d9 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,49 @@ 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;
>> + char node_name[6 + 16 + 1]; // ITS identification name
>
> Jeezus... Where is that coming from? Is that the size of "ITS@0x"+16
> hex digits and a final NUL? For the sake of keeping things
> maintainable/readable, please use kasprintf/kfree.
kasprintf/kfree will be used here.
>
>> +
>> + its_entry = (struct acpi_madt_generic_translator *)header;
>> + snprintf(node_name, 23, "ITS@0x%lx", (long)its_entry->base_address);
>> + dom_handle = iort_its_find_domain_token(its_entry->translation_id);
>> + if (!dom_handle) {
>> + pr_err("%s: Unable to locate ITS domain handle\n", node_name);
>> + return 0;
>
> return 0? not even a slap on the wrist? Hum...
>
>> + }
>> +
>> + if (its_pci_msi_init_one(dom_handle, node_name))
>> + return 0;
>
> Same here.
Yeah, decent error code are needed here.
>
>> +
>> + pr_info("PCI/MSI: %s domain created\n", node_name);
>> + return 0;
>> +}
>> +
>> +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
>> +inline static int __init its_pci_acpi_msi_init(void)
>
> static only.
OK
>
>> +{
>> + 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 cee8296..41c1cda 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();
>
Thanks for review.
Tomasz
^ permalink raw reply [flat|nested] 15+ messages in thread