2018-11-28 14:44:18

by Robert Richter

[permalink] [raw]
Subject: [PATCH v2 00/10] irqdomain, gic-v3-its: Implement late irq domain initialization

This patch series is a rework of the gic-v3-its initialization. It is
in preparation of a further series that uses CMA memory allocation for
ITS tables. For this the CMA framework must be available and thus ITS
needs to be initialized after the arch_initcalls. This requires two
major reworks.

First we must remove all irq domain init order dependencies (patches
#1-#5), second the ITS initialization must be split into an early
probe and a later init part (patches #6-#10).

Patch #1 introduces a new interface to request an irq domain, see the
patch description for details. In patches #2-#5 all ITS related irq
domains are converted to use the new interface. There are no order
dependencies for parent and client irq domain initialization anymore
which allows us to initialize all ITS irq domains in the same initcall
(moving to the later subsys_initcall). Note that I do not have fsl
hardware available, the code should work, but is only carefully
reviewed and compile tested, please test on that hardware.

The remaining patches #6-#10 are an incremental rework of the ITS
initialization, basically splitting probe and init (patch #7) and
separating it from gic-v3 code (patch #8). Patches #9 and #10 change
initcall levels for various drivers.

Patches have been tested with Cavium ThunderX and ThunderX2. I have an
implementation of CMA ITS table allocation on top of this series
available, I will send out patches for this in a couple of days.

v2:
* fix check for domain match in irq_domain_handle_requests()
* add comment that describes this check in irq_domain_handle_
requests()

Robert Richter (10):
irqdomain: Add interface to request an irq domain
irqchip/gic-v3-its-platform-msi: Remove domain init order dependencies
irqchip/irq-gic-v3-its-pci-msi: Remove domain init order dependencies
irqchip/irq-gic-v3-its-fsl-mc-msi: Remove domain init order
dependencies
fsl-mc/dprc-driver: Remove domain init order dependencies
irqchip/gic-v3-its: Initialize its nodes in probe order
irqchip/gic-v3-its: Split probing from its node initialization
irqchip/gic-v3-its: Decouple its initialization from gic
irqchip/gic-v3-its: Initialize its nodes later
irqchip/gic-v3-its: Initialize MSIs with subsys_initcalls

drivers/bus/fsl-mc/dprc-driver.c | 41 +++++++
drivers/bus/fsl-mc/fsl-mc-msi.c | 6 +-
drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 49 +++++---
drivers/irqchip/irq-gic-v3-its-pci-msi.c | 68 ++++++-----
drivers/irqchip/irq-gic-v3-its-platform-msi.c | 56 +++++++--
drivers/irqchip/irq-gic-v3-its.c | 160 ++++++++++++++++---------
drivers/irqchip/irq-gic-v3.c | 12 +-
include/linux/cpuhotplug.h | 1 +
include/linux/irqchip/arm-gic-v3.h | 5 +-
include/linux/irqdomain.h | 15 +++
kernel/irq/irqdomain.c | 163 ++++++++++++++++++++++++++
11 files changed, 446 insertions(+), 130 deletions(-)

--
2.11.0


2018-11-28 14:44:33

by Robert Richter

[permalink] [raw]
Subject: [PATCH v2 04/10] irqchip/irq-gic-v3-its-fsl-mc-msi: Remove domain init order dependencies

Use new irq_domain_request_host_*() interface which allows independent
parent and child initialization using an irq domain request handler.
This makes it possible to move its initialization to a later point
during boot. All domains can be initialized in the same initcall level
then.

Signed-off-by: Robert Richter <[email protected]>
---
drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 47 ++++++++++++++++++-----------
1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c b/drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c
index 606efa64adff..81dfc534ded8 100644
--- a/drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c
@@ -66,11 +66,34 @@ static const struct of_device_id its_device_id[] = {
{},
};

+static int __init its_fsl_create_irq_domain(struct irq_domain *parent,
+ void *priv)
+{
+ struct device_node *np = irq_domain_get_of_node(parent);
+ struct irq_domain *mc_msi_domain;
+
+ if (!msi_get_domain_info(parent)) {
+ pr_err("%pOF: unable to locate ITS domain\n", np);
+ return -ENXIO;
+ }
+
+ mc_msi_domain = fsl_mc_msi_create_irq_domain(parent->fwnode,
+ &its_fsl_mc_msi_domain_info,
+ parent);
+ if (!mc_msi_domain) {
+ pr_err("%pOF: unable to create fsl-mc domain\n", np);
+ return -ENOMEM;
+ }
+
+ pr_info("fsl-mc MSI: %pOF domain created\n", np);
+
+ return 0;
+}
+
static int __init its_fsl_mc_msi_init(void)
{
struct device_node *np;
- struct irq_domain *parent;
- struct irq_domain *mc_msi_domain;
+ int ret = 0;

for (np = of_find_matching_node(NULL, its_device_id); np;
np = of_find_matching_node(np, its_device_id)) {
@@ -79,22 +102,10 @@ static int __init its_fsl_mc_msi_init(void)
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("%pOF: unable to locate ITS domain\n", np);
- continue;
- }
-
- mc_msi_domain = fsl_mc_msi_create_irq_domain(
- of_node_to_fwnode(np),
- &its_fsl_mc_msi_domain_info,
- parent);
- if (!mc_msi_domain) {
- pr_err("%pOF: unable to create fsl-mc domain\n", np);
- continue;
- }
-
- pr_info("fsl-mc MSI: %pOF domain created\n", np);
+ ret = irq_domain_request_host(np, DOMAIN_BUS_NEXUS,
+ its_fsl_create_irq_domain, NULL);
+ if (ret)
+ pr_err("%pOF: unable to register ITS domain\n", np);
}

return 0;
--
2.11.0


2018-11-28 14:44:34

by Robert Richter

[permalink] [raw]
Subject: [PATCH v2 05/10] fsl-mc/dprc-driver: Remove domain init order dependencies

Use new irq_domain_request_host_*() interface which allows independent
parent and child initialization using an irq domain request handler.
This makes it possible to move its initialization to a later point
during boot. All domains can be initialized in the same initcall level
then.

Signed-off-by: Robert Richter <[email protected]>
---
drivers/bus/fsl-mc/dprc-driver.c | 41 ++++++++++++++++++++++++++++++++++++++++
drivers/bus/fsl-mc/fsl-mc-msi.c | 6 +-----
2 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/drivers/bus/fsl-mc/dprc-driver.c b/drivers/bus/fsl-mc/dprc-driver.c
index 52c7e15143d6..2f41886c8e00 100644
--- a/drivers/bus/fsl-mc/dprc-driver.c
+++ b/drivers/bus/fsl-mc/dprc-driver.c
@@ -10,7 +10,9 @@
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/interrupt.h>
+#include <linux/irqdomain.h>
#include <linux/msi.h>
+#include <linux/of.h>
#include <linux/fsl/mc.h>

#include "fsl-mc-private.h"
@@ -575,6 +577,40 @@ static int dprc_setup_irq(struct fsl_mc_device *mc_dev)
return error;
}

+static int dprc_probe(struct fsl_mc_device *mc_dev);
+
+static int dprc_probe_late(struct irq_domain *parent, void *priv)
+{
+ struct fsl_mc_device *mc_dev = priv;
+
+ of_node_put(mc_dev->dev.parent->of_node);
+ of_node_put(irq_domain_get_of_node(parent));
+
+ return dprc_probe(mc_dev);
+}
+
+static int dprc_register_msi_domain(struct fsl_mc_device *mc_dev)
+{
+ struct device_node *mc_of_node, *msi_np;
+ int err = -EINVAL;
+
+ mc_of_node = of_node_get(mc_dev->dev.parent->of_node);
+
+ msi_np = of_parse_phandle(mc_of_node, "msi-parent", 0);
+ if (msi_np && !of_property_read_bool(msi_np, "#msi-cells"))
+ err = irq_domain_request_host(msi_np, DOMAIN_BUS_FSL_MC_MSI,
+ dprc_probe_late, mc_dev);
+
+ if (err) {
+ pr_err("Unable to find fsl-mc MSI domain for %pOF\n",
+ mc_of_node);
+ of_node_put(msi_np);
+ of_node_put(mc_of_node);
+ }
+
+ return err;
+}
+
/**
* dprc_probe - callback invoked when a DPRC is being bound to this driver
*
@@ -641,6 +677,11 @@ static int dprc_probe(struct fsl_mc_device *mc_dev)

error = fsl_mc_find_msi_domain(parent_dev,
&mc_msi_domain);
+
+ if (error == -ENOENT)
+ /* initialize later */
+ return dprc_register_msi_domain(mc_dev);
+
if (error < 0) {
dev_warn(&mc_dev->dev,
"WARNING: MC bus without interrupt support\n");
diff --git a/drivers/bus/fsl-mc/fsl-mc-msi.c b/drivers/bus/fsl-mc/fsl-mc-msi.c
index 8b9c66d7c4ff..550d2ed07f69 100644
--- a/drivers/bus/fsl-mc/fsl-mc-msi.c
+++ b/drivers/bus/fsl-mc/fsl-mc-msi.c
@@ -185,12 +185,8 @@ int fsl_mc_find_msi_domain(struct device *mc_platform_dev,

msi_domain = of_msi_get_domain(mc_platform_dev, mc_of_node,
DOMAIN_BUS_FSL_MC_MSI);
- if (!msi_domain) {
- pr_err("Unable to find fsl-mc MSI domain for %pOF\n",
- mc_of_node);
-
+ if (!msi_domain)
return -ENOENT;
- }

*mc_msi_domain = msi_domain;
return 0;
--
2.11.0


2018-11-28 14:44:35

by Robert Richter

[permalink] [raw]
Subject: [PATCH v2 06/10] irqchip/gic-v3-its: Initialize its nodes in probe order

ATM the last discovered node is initialized first. Though this order
should work too, change the initialization of nodes to probe order as
one would expect it.

Signed-off-by: Robert Richter <[email protected]>
---
drivers/irqchip/irq-gic-v3-its.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index db20e992a40f..4033f71f5181 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3602,7 +3602,7 @@ static int __init its_probe_one(struct resource *res,
goto out_free_tables;

raw_spin_lock(&its_lock);
- list_add(&its->entry, &its_nodes);
+ list_add_tail(&its->entry, &its_nodes);
raw_spin_unlock(&its_lock);

return 0;
--
2.11.0


2018-11-28 14:44:40

by Robert Richter

[permalink] [raw]
Subject: [PATCH v2 07/10] irqchip/gic-v3-its: Split probing from its node initialization

To initialize the its nodes at a later point during boot, we need to
split probing from initialization. Collect all information required
for initialization in struct its_node. We can then use the its node
list for initialization.

Signed-off-by: Robert Richter <[email protected]>
---
drivers/irqchip/irq-gic-v3-its.c | 135 +++++++++++++++++++++++--------------
drivers/irqchip/irq-gic-v3.c | 2 +-
include/linux/irqchip/arm-gic-v3.h | 4 +-
3 files changed, 87 insertions(+), 54 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 4033f71f5181..c28f4158ff70 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -103,6 +103,7 @@ struct its_node {
struct list_head entry;
void __iomem *base;
phys_addr_t phys_base;
+ phys_addr_t phys_size;
struct its_cmd_block *cmd_base;
struct its_cmd_block *cmd_write;
struct its_baser tables[GITS_BASER_NR_REGS];
@@ -3375,7 +3376,7 @@ static struct syscore_ops its_syscore_ops = {
.resume = its_restore_enable,
};

-static int its_init_domain(struct fwnode_handle *handle, struct its_node *its)
+static int its_init_domain(struct its_node *its)
{
struct irq_domain *inner_domain;
struct msi_domain_info *info;
@@ -3384,7 +3385,8 @@ static int its_init_domain(struct fwnode_handle *handle, struct its_node *its)
if (!info)
return -ENOMEM;

- inner_domain = irq_domain_create_tree(handle, &its_domain_ops, its);
+ inner_domain = irq_domain_create_tree(its->fwnode_handle,
+ &its_domain_ops, its);
if (!inner_domain) {
kfree(info);
return -ENOMEM;
@@ -3441,8 +3443,7 @@ static int its_init_vpe_domain(void)
return 0;
}

-static int __init its_compute_its_list_map(struct resource *res,
- void __iomem *its_base)
+static int __init its_compute_its_list_map(struct its_node *its)
{
int its_number;
u32 ctlr;
@@ -3456,15 +3457,15 @@ static int __init its_compute_its_list_map(struct resource *res,
its_number = find_first_zero_bit(&its_list_map, GICv4_ITS_LIST_MAX);
if (its_number >= GICv4_ITS_LIST_MAX) {
pr_err("ITS@%pa: No ITSList entry available!\n",
- &res->start);
+ &its->phys_base);
return -EINVAL;
}

- ctlr = readl_relaxed(its_base + GITS_CTLR);
+ ctlr = readl_relaxed(its->base + GITS_CTLR);
ctlr &= ~GITS_CTLR_ITS_NUMBER;
ctlr |= its_number << GITS_CTLR_ITS_NUMBER_SHIFT;
- writel_relaxed(ctlr, its_base + GITS_CTLR);
- ctlr = readl_relaxed(its_base + GITS_CTLR);
+ writel_relaxed(ctlr, its->base + GITS_CTLR);
+ ctlr = readl_relaxed(its->base + GITS_CTLR);
if ((ctlr & GITS_CTLR_ITS_NUMBER) != (its_number << GITS_CTLR_ITS_NUMBER_SHIFT)) {
its_number = ctlr & GITS_CTLR_ITS_NUMBER;
its_number >>= GITS_CTLR_ITS_NUMBER_SHIFT;
@@ -3472,83 +3473,110 @@ static int __init its_compute_its_list_map(struct resource *res,

if (test_and_set_bit(its_number, &its_list_map)) {
pr_err("ITS@%pa: Duplicate ITSList entry %d\n",
- &res->start, its_number);
+ &its->phys_base, its_number);
return -EINVAL;
}

return its_number;
}

+static void its_free(struct its_node *its)
+{
+ raw_spin_lock(&its_lock);
+ list_del(&its->entry);
+ raw_spin_unlock(&its_lock);
+
+ kfree(its);
+}
+
+static int __init its_init_one(struct its_node *its);
+
static int __init its_probe_one(struct resource *res,
struct fwnode_handle *handle, int numa_node)
{
struct its_node *its;
+ int err;
+
+ its = kzalloc(sizeof(*its), GFP_KERNEL);
+ if (!its)
+ return -ENOMEM;
+
+ raw_spin_lock_init(&its->lock);
+ INIT_LIST_HEAD(&its->entry);
+ INIT_LIST_HEAD(&its->its_device_list);
+ its->fwnode_handle = handle;
+ its->phys_base = res->start;
+ its->phys_size = resource_size(res);
+ its->numa_node = numa_node;
+
+ raw_spin_lock(&its_lock);
+ list_add_tail(&its->entry, &its_nodes);
+ raw_spin_unlock(&its_lock);
+
+ pr_info("ITS %pR\n", res);
+
+ err = its_init_one(its);
+ if (err)
+ its_free(its);
+
+ return err;
+}
+
+static int __init its_init_one(struct its_node *its)
+{
void __iomem *its_base;
u32 val, ctlr;
u64 baser, tmp, typer;
int err;

- its_base = ioremap(res->start, resource_size(res));
+ its_base = ioremap(its->phys_base, its->phys_size);
if (!its_base) {
- pr_warn("ITS@%pa: Unable to map ITS registers\n", &res->start);
- return -ENOMEM;
+ pr_warn("ITS@%pa: Unable to map ITS registers\n", &its->phys_base);
+ err = -ENOMEM;
+ goto fail;
}

val = readl_relaxed(its_base + GITS_PIDR2) & GIC_PIDR2_ARCH_MASK;
if (val != 0x30 && val != 0x40) {
- pr_warn("ITS@%pa: No ITS detected, giving up\n", &res->start);
+ pr_warn("ITS@%pa: No ITS detected, giving up\n", &its->phys_base);
err = -ENODEV;
goto out_unmap;
}

err = its_force_quiescent(its_base);
if (err) {
- pr_warn("ITS@%pa: Failed to quiesce, giving up\n", &res->start);
+ pr_warn("ITS@%pa: Failed to quiesce, giving up\n", &its->phys_base);
goto out_unmap;
}

- pr_info("ITS %pR\n", res);
-
- its = kzalloc(sizeof(*its), GFP_KERNEL);
- if (!its) {
- err = -ENOMEM;
- goto out_unmap;
- }
-
- raw_spin_lock_init(&its->lock);
- INIT_LIST_HEAD(&its->entry);
- INIT_LIST_HEAD(&its->its_device_list);
typer = gic_read_typer(its_base + GITS_TYPER);
its->base = its_base;
- its->phys_base = res->start;
its->ite_size = GITS_TYPER_ITT_ENTRY_SIZE(typer);
its->device_ids = GITS_TYPER_DEVBITS(typer);
its->is_v4 = !!(typer & GITS_TYPER_VLPIS);
if (its->is_v4) {
if (!(typer & GITS_TYPER_VMOVP)) {
- err = its_compute_its_list_map(res, its_base);
+ err = its_compute_its_list_map(its);
if (err < 0)
- goto out_free_its;
+ goto out_unmap;

its->list_nr = err;

pr_info("ITS@%pa: Using ITS number %d\n",
- &res->start, err);
+ &its->phys_base, err);
} else {
- pr_info("ITS@%pa: Single VMOVP capable\n", &res->start);
+ pr_info("ITS@%pa: Single VMOVP capable\n",
+ &its->phys_base);
}
}

- its->numa_node = numa_node;
-
its->cmd_base = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
get_order(ITS_CMD_QUEUE_SZ));
if (!its->cmd_base) {
err = -ENOMEM;
- goto out_free_its;
+ goto out_unmap;
}
its->cmd_write = its->cmd_base;
- its->fwnode_handle = handle;
its->get_msi_base = its_irq_get_msi_base;
its->msi_domain_flags = IRQ_DOMAIN_FLAG_MSI_REMAP;

@@ -3597,13 +3625,11 @@ static int __init its_probe_one(struct resource *res,
if (GITS_TYPER_HCC(typer))
its->flags |= ITS_FLAGS_SAVE_SUSPEND_STATE;

- err = its_init_domain(handle, its);
+ err = its_init_domain(its);
if (err)
goto out_free_tables;

- raw_spin_lock(&its_lock);
- list_add_tail(&its->entry, &its_nodes);
- raw_spin_unlock(&its_lock);
+ pr_info("ITS@%pa: ITS node added\n", &its->phys_base);

return 0;

@@ -3611,11 +3637,10 @@ static int __init its_probe_one(struct resource *res,
its_free_tables(its);
out_free_cmd:
free_pages((unsigned long)its->cmd_base, get_order(ITS_CMD_QUEUE_SZ));
-out_free_its:
- kfree(its);
out_unmap:
iounmap(its_base);
- pr_err("ITS@%pa: failed probing (%d)\n", &res->start, err);
+fail:
+ pr_err("ITS@%pa: failed probing (%d)\n", &its->phys_base, err);
return err;
}

@@ -3888,13 +3913,12 @@ static void __init its_acpi_probe(void)
static void __init its_acpi_probe(void) { }
#endif

-int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
- struct irq_domain *parent_domain)
+static int __init its_init(void);
+
+int __init its_probe(struct fwnode_handle *handle, struct rdists *rdists,
+ struct irq_domain *parent_domain)
{
struct device_node *of_node;
- struct its_node *its;
- bool has_v4 = false;
- int err;

its_parent = parent_domain;
of_node = to_of_node(handle);
@@ -3903,13 +3927,22 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
else
its_acpi_probe();

+ gic_rdists = rdists;
+
+ return its_init();
+}
+
+static int __init its_init(void)
+{
+ struct its_node *its;
+ bool has_v4 = false;
+ int err;
+
if (list_empty(&its_nodes)) {
pr_warn("ITS: No ITS available, not enabling LPIs\n");
return -ENXIO;
}

- gic_rdists = rdists;
-
err = allocate_lpi_tables();
if (err)
return err;
@@ -3917,10 +3950,10 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
list_for_each_entry(its, &its_nodes, entry)
has_v4 |= its->is_v4;

- if (has_v4 & rdists->has_vlpis) {
+ if (has_v4 & gic_rdists->has_vlpis) {
if (its_init_vpe_domain() ||
- its_init_v4(parent_domain, &its_vpe_domain_ops)) {
- rdists->has_vlpis = false;
+ its_init_v4(its_parent, &its_vpe_domain_ops)) {
+ gic_rdists->has_vlpis = false;
pr_err("ITS: Disabling GICv4 support\n");
}
}
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 8f87f40c9460..e04108b7c6b7 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1132,7 +1132,7 @@ static int __init gic_init_bases(void __iomem *dist_base,
gic_cpu_pm_init();

if (gic_dist_supports_lpis()) {
- its_init(handle, &gic_data.rdists, gic_data.domain);
+ its_probe(handle, &gic_data.rdists, gic_data.domain);
its_cpu_init();
}

diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index 071b4cbdf010..a6fdb2910f73 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -603,8 +603,8 @@ struct rdists {
struct irq_domain;
struct fwnode_handle;
int its_cpu_init(void);
-int its_init(struct fwnode_handle *handle, struct rdists *rdists,
- struct irq_domain *domain);
+int its_probe(struct fwnode_handle *handle, struct rdists *rdists,
+ struct irq_domain *domain);
int mbi_init(struct fwnode_handle *fwnode, struct irq_domain *parent);

static inline bool gic_enable_sre(void)
--
2.11.0


2018-11-28 14:44:42

by Robert Richter

[permalink] [raw]
Subject: [PATCH v2 08/10] irqchip/gic-v3-its: Decouple its initialization from gic

This patch separates its initialization from the gic. Probing and
initialization of its nodes is separate now. There is an own cpu
notifier for its now.

Signed-off-by: Robert Richter <[email protected]>
---
drivers/irqchip/irq-gic-v3-its.c | 58 +++++++++++++++++++++++++-------------
drivers/irqchip/irq-gic-v3.c | 14 ++++-----
include/linux/cpuhotplug.h | 1 +
include/linux/irqchip/arm-gic-v3.h | 2 +-
4 files changed, 48 insertions(+), 27 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index c28f4158ff70..fd8561fcfdf3 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -167,6 +167,7 @@ static struct {
} vpe_proxy;

static LIST_HEAD(its_nodes);
+static LIST_HEAD(its_probed);
static DEFINE_RAW_SPINLOCK(its_lock);
static struct rdists *gic_rdists;
static struct irq_domain *its_parent;
@@ -3482,20 +3483,13 @@ static int __init its_compute_its_list_map(struct its_node *its)

static void its_free(struct its_node *its)
{
- raw_spin_lock(&its_lock);
- list_del(&its->entry);
- raw_spin_unlock(&its_lock);
-
kfree(its);
}

-static int __init its_init_one(struct its_node *its);
-
static int __init its_probe_one(struct resource *res,
struct fwnode_handle *handle, int numa_node)
{
struct its_node *its;
- int err;

its = kzalloc(sizeof(*its), GFP_KERNEL);
if (!its)
@@ -3510,16 +3504,12 @@ static int __init its_probe_one(struct resource *res,
its->numa_node = numa_node;

raw_spin_lock(&its_lock);
- list_add_tail(&its->entry, &its_nodes);
+ list_add_tail(&its->entry, &its_probed);
raw_spin_unlock(&its_lock);

pr_info("ITS %pR\n", res);

- err = its_init_one(its);
- if (err)
- its_free(its);
-
- return err;
+ return 0;
}

static int __init its_init_one(struct its_node *its)
@@ -3717,7 +3707,7 @@ static int redist_disable_lpis(void)
return 0;
}

-int its_cpu_init(void)
+static int its_cpu_init(unsigned int cpu)
{
if (!list_empty(&its_nodes)) {
int ret;
@@ -3913,8 +3903,6 @@ static void __init its_acpi_probe(void)
static void __init its_acpi_probe(void) { }
#endif

-static int __init its_init(void);
-
int __init its_probe(struct fwnode_handle *handle, struct rdists *rdists,
struct irq_domain *parent_domain)
{
@@ -3929,23 +3917,51 @@ int __init its_probe(struct fwnode_handle *handle, struct rdists *rdists,

gic_rdists = rdists;

- return its_init();
+ return 0;
}

-static int __init its_init(void)
+int __init its_init(void)
{
struct its_node *its;
bool has_v4 = false;
int err;

+ if (list_empty(&its_probed))
+ return 0;
+
+ raw_spin_lock(&its_lock);
+redo:
+ list_for_each_entry(its, &its_probed, entry) {
+ list_del_init(&its->entry);
+
+ raw_spin_unlock(&its_lock);
+
+ /* Needs to be called in non-atomic context */
+ err = its_init_one(its);
+ if (err)
+ its_free(its);
+
+ raw_spin_lock(&its_lock);
+
+ if (!err)
+ list_add_tail(&its->entry, &its_nodes);
+
+ goto redo;
+ }
+
+ raw_spin_unlock(&its_lock);
+
if (list_empty(&its_nodes)) {
pr_warn("ITS: No ITS available, not enabling LPIs\n");
return -ENXIO;
}

err = allocate_lpi_tables();
- if (err)
+ if (err) {
+ pr_warn("ITS: Failed to initialize (%d), not enabling LPIs\n",
+ err);
return err;
+ }

list_for_each_entry(its, &its_nodes, entry)
has_v4 |= its->is_v4;
@@ -3960,5 +3976,9 @@ static int __init its_init(void)

register_syscore_ops(&its_syscore_ops);

+ cpuhp_setup_state(CPUHP_AP_IRQ_GIC_ITS_STARTING,
+ "irqchip/arm/gicv3-its:starting",
+ its_cpu_init, NULL);
+
return 0;
}
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index e04108b7c6b7..d2942efdb6d5 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -685,9 +685,6 @@ static int gic_starting_cpu(unsigned int cpu)
{
gic_cpu_init();

- if (gic_dist_supports_lpis())
- its_cpu_init();
-
return 0;
}

@@ -815,7 +812,7 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
#else
#define gic_set_affinity NULL
#define gic_smp_init() do { } while(0)
-#endif
+#endif /* CONFIG_SMP */

#ifdef CONFIG_CPU_PM
/* Check whether it's single security state view */
@@ -1131,10 +1128,8 @@ static int __init gic_init_bases(void __iomem *dist_base,
gic_cpu_init();
gic_cpu_pm_init();

- if (gic_dist_supports_lpis()) {
+ if (gic_dist_supports_lpis())
its_probe(handle, &gic_data.rdists, gic_data.domain);
- its_cpu_init();
- }

return 0;

@@ -1327,6 +1322,9 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare

if (static_branch_likely(&supports_deactivate_key))
gic_of_setup_kvm_info(node);
+
+ its_init();
+
return 0;

out_unmap_rdist:
@@ -1630,6 +1628,8 @@ gic_acpi_init(struct acpi_subtable_header *header, const unsigned long end)
if (static_branch_likely(&supports_deactivate_key))
gic_acpi_setup_kvm_info();

+ its_init();
+
return 0;

out_fwhandle_free:
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index e0cd2baa8380..584f73585142 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -96,6 +96,7 @@ enum cpuhp_state {
CPUHP_AP_SCHED_STARTING,
CPUHP_AP_RCUTREE_DYING,
CPUHP_AP_IRQ_GIC_STARTING,
+ CPUHP_AP_IRQ_GIC_ITS_STARTING,
CPUHP_AP_IRQ_HIP04_STARTING,
CPUHP_AP_IRQ_ARMADA_XP_STARTING,
CPUHP_AP_IRQ_BCM2836_STARTING,
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index a6fdb2910f73..f4348fa4260a 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -602,9 +602,9 @@ struct rdists {

struct irq_domain;
struct fwnode_handle;
-int its_cpu_init(void);
int its_probe(struct fwnode_handle *handle, struct rdists *rdists,
struct irq_domain *domain);
+int its_init(void);
int mbi_init(struct fwnode_handle *fwnode, struct irq_domain *parent);

static inline bool gic_enable_sre(void)
--
2.11.0


2018-11-28 14:44:45

by Robert Richter

[permalink] [raw]
Subject: [PATCH v2 09/10] irqchip/gic-v3-its: Initialize its nodes later

Use an initcall to initialize its. This allows us to use the device
framework during initialization that is up at this point. We use
subsys_initcall() here since we need the arch to be initialized
first. It is before pci and platform device probe where devices are
bound to msi interrupts.

Signed-off-by: Robert Richter <[email protected]>
---
drivers/irqchip/irq-gic-v3-its.c | 3 ++-
drivers/irqchip/irq-gic-v3.c | 4 ----
include/linux/irqchip/arm-gic-v3.h | 1 -
3 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index fd8561fcfdf3..13cf56c66483 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3920,7 +3920,7 @@ int __init its_probe(struct fwnode_handle *handle, struct rdists *rdists,
return 0;
}

-int __init its_init(void)
+static int __init its_init(void)
{
struct its_node *its;
bool has_v4 = false;
@@ -3982,3 +3982,4 @@ int __init its_init(void)

return 0;
}
+subsys_initcall(its_init);
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index d2942efdb6d5..01538876ad15 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1323,8 +1323,6 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare
if (static_branch_likely(&supports_deactivate_key))
gic_of_setup_kvm_info(node);

- its_init();
-
return 0;

out_unmap_rdist:
@@ -1628,8 +1626,6 @@ gic_acpi_init(struct acpi_subtable_header *header, const unsigned long end)
if (static_branch_likely(&supports_deactivate_key))
gic_acpi_setup_kvm_info();

- its_init();
-
return 0;

out_fwhandle_free:
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index f4348fa4260a..885d5a4e239a 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -604,7 +604,6 @@ struct irq_domain;
struct fwnode_handle;
int its_probe(struct fwnode_handle *handle, struct rdists *rdists,
struct irq_domain *domain);
-int its_init(void);
int mbi_init(struct fwnode_handle *fwnode, struct irq_domain *parent);

static inline bool gic_enable_sre(void)
--
2.11.0


2018-11-28 14:44:48

by Robert Richter

[permalink] [raw]
Subject: [PATCH v2 10/10] irqchip/gic-v3-its: Initialize MSIs with subsys_initcalls

Since ITS is initialized with with the subsys_initcall now, we don't
need to enable ITS children earlier. Due to the use of irq_domain_
request_host_*() there are no order dependencies when initializing irq
domains.

Signed-off-by: Robert Richter <[email protected]>
---
drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 2 +-
drivers/irqchip/irq-gic-v3-its-pci-msi.c | 2 +-
drivers/irqchip/irq-gic-v3-its-platform-msi.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c b/drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c
index 81dfc534ded8..f6df5ea16aef 100644
--- a/drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c
@@ -111,4 +111,4 @@ static int __init its_fsl_mc_msi_init(void)
return 0;
}

-early_initcall(its_fsl_mc_msi_init);
+subsys_initcall(its_fsl_mc_msi_init);
diff --git a/drivers/irqchip/irq-gic-v3-its-pci-msi.c b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
index 7d7366d55d34..9c4a0ebdab0b 100644
--- a/drivers/irqchip/irq-gic-v3-its-pci-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
@@ -215,4 +215,4 @@ static int __init its_pci_msi_init(void)
its_pci_acpi_msi_init();
return 0;
}
-early_initcall(its_pci_msi_init);
+subsys_initcall(its_pci_msi_init);
diff --git a/drivers/irqchip/irq-gic-v3-its-platform-msi.c b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
index 1f2849bc58c4..76f8a2e85375 100644
--- a/drivers/irqchip/irq-gic-v3-its-platform-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
@@ -201,4 +201,4 @@ static int __init its_pmsi_init(void)
its_pmsi_acpi_init();
return 0;
}
-early_initcall(its_pmsi_init);
+subsys_initcall(its_pmsi_init);
--
2.11.0


2018-11-28 14:45:37

by Robert Richter

[permalink] [raw]
Subject: [PATCH v2 03/10] irqchip/irq-gic-v3-its-pci-msi: Remove domain init order dependencies

Use new irq_domain_request_host_*() interface which allows independent
parent and child initialization using an irq domain request handler.
This makes it possible to move its initialization to a later point
during boot. All domains can be initialized in the same initcall level
then.

Signed-off-by: Robert Richter <[email protected]>
---
drivers/irqchip/irq-gic-v3-its-pci-msi.c | 66 ++++++++++++++++++--------------
1 file changed, 38 insertions(+), 28 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its-pci-msi.c b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
index 8d6d009d1d58..7d7366d55d34 100644
--- a/drivers/irqchip/irq-gic-v3-its-pci-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
@@ -116,27 +116,50 @@ static struct of_device_id its_device_id[] = {
{},
};

-static int __init its_pci_msi_init_one(struct fwnode_handle *handle,
- const char *name)
+static int __init its_pci_create_irq_domain(struct irq_domain *parent,
+ void *priv)
{
- struct irq_domain *parent;
+ char *name = priv;
+ int err = 0;

- 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 (!msi_get_domain_info(parent)) {
+ err = -ENODEV;
+ goto out;
}

- if (!pci_msi_create_irq_domain(handle, &its_pci_msi_domain_info,
+ if (!pci_msi_create_irq_domain(parent->fwnode, &its_pci_msi_domain_info,
parent)) {
- pr_err("%s: Unable to create PCI domain\n", name);
- return -ENOMEM;
+ err = -ENOMEM;
+ goto out;
}

- return 0;
+ pr_info("PCI/MSI: %s domain created\n", name);
+out:
+ if (err)
+ pr_err("PCI/MSI: Failed to create %s domain: %d\n", name, err);
+
+ kfree(name);
+ return err;
+}
+
+static int __init its_pci_msi_init_one(struct fwnode_handle *handle,
+ const char *name)
+{
+ void *priv = kstrdup(name, GFP_KERNEL);
+ int err;
+
+ err = irq_domain_request_fwnode(handle, DOMAIN_BUS_NEXUS,
+ its_pci_create_irq_domain, name, priv);
+ if (err) {
+ pr_err("PCI/MSI: Failed to register %s domain: %d\n",
+ name, err);
+ kfree(priv);
+ }
+
+ return err;
}

-static int __init its_pci_of_msi_init(void)
+static void __init its_pci_of_msi_init(void)
{
struct device_node *np;

@@ -147,13 +170,8 @@ static int __init its_pci_of_msi_init(void)
if (!of_property_read_bool(np, "msi-controller"))
continue;

- if (its_pci_msi_init_one(of_node_to_fwnode(np), np->full_name))
- continue;
-
- pr_info("PCI/MSI: %pOF domain created\n", np);
+ its_pci_msi_init_one(of_node_to_fwnode(np), np->full_name);
}
-
- return 0;
}

#ifdef CONFIG_ACPI
@@ -177,32 +195,24 @@ its_pci_msi_parse_madt(struct acpi_subtable_header *header,
}

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)
+static void __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;
-}
+static void __init its_pci_acpi_msi_init(void) { }
#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);
--
2.11.0


2018-11-28 14:45:54

by Robert Richter

[permalink] [raw]
Subject: [PATCH v2 01/10] irqdomain: Add interface to request an irq domain

This patch introduces a new interface to allow irq domain
initialization regardless of order dependencies. This is done by
requesting a domain and registering a callback function that is called
as soon as a domain becomes available.

A typical irq domain initialization code is the following:

Parent initialization:

...
domain = msi_create_irq_domain(fwnode, info, parent);
if (domain)
irq_domain_update_bus_token(domain, bus_token);
...

Child initialization:

...
parent = irq_find_matching_fwnode(fwnode, bus_token);
if (!parent)
...
create_irq_domain(parent, ...);
...

In case the parent is not yet available, the child initialization
fails. Thus, the current implementation requires the parent domain
being initialized before the child domain. With a complex irq domain
hierarchy it becomes more and more difficult to grant that order as
irq domains are enabled in separate subsystems. Care must be taken
when initializing parent and child domains in the same initcall
level. E.g. Arm's gic-v3-its implementation might have the following
tree and dependencies:

gic-v3
├── its-node-0
│ ├── pci-host-0
│ ├── platform-bus
│ ...
├── its-node-1
│ ├── pci-host-1
...

All domains must be initialized in top-down order of the tree.

This patch introduces an interface that allows domain initialization
without any order requirements, e.g. to be able to initialize the irq
domains in the same initcall level. The following functions have been
introduced to allow the registration of a callback handler:

irq_domain_request_fwnode()
irq_domain_request_host()

Instead of using the irq_find_matching_fwnode() function and it's
variants the child code replaces them with the new functions and
looks e.g. like the following:

...
irq_domain_request_fwnode(fwnode, bus_token,
create_irq_domain, name, priv);
...

Here, the callback function create_irq_domain() is called as soon as
the parent becomes available. All registered handlers are stored in a
list. With each update of the bus token using irq_domain_update_bus_
token(), the list is checked if that domain is requested by a handler
and if that is the case it's callback function is called and the
request removed from the list.

With a late_initcall all requests from the list should already have
been handled, otherwise all remaining requests are removed with an
error reported.

Signed-off-by: Robert Richter <[email protected]>
---
include/linux/irqdomain.h | 15 +++++
kernel/irq/irqdomain.c | 163 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 178 insertions(+)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 068aa46f0d55..27e83803627d 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -311,6 +311,21 @@ static inline struct irq_domain *irq_find_host(struct device_node *node)
return d;
}

+typedef int (*irq_domain_callback_t)(struct irq_domain *, void *);
+int irq_domain_request_fwnode(struct fwnode_handle *fwnode,
+ enum irq_domain_bus_token bus_token,
+ irq_domain_callback_t callback,
+ const char *name, void *priv);
+
+static inline int irq_domain_request_host(struct device_node *node,
+ enum irq_domain_bus_token bus_token,
+ irq_domain_callback_t callback,
+ void *priv)
+{
+ return irq_domain_request_fwnode(of_node_to_fwnode(node), bus_token,
+ callback, node->full_name, priv);
+}
+
/**
* irq_domain_add_linear() - Allocate and register a linear revmap irq_domain.
* @of_node: pointer to interrupt controller's device tree node.
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 3366d11c3e02..ebe628dad568 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -21,6 +21,7 @@
#include <linux/fs.h>

static LIST_HEAD(irq_domain_list);
+static LIST_HEAD(irq_domain_requests);
static DEFINE_MUTEX(irq_domain_mutex);

static struct irq_domain *irq_default_domain;
@@ -45,6 +46,111 @@ static inline void debugfs_remove_domain_dir(struct irq_domain *d) { }
const struct fwnode_operations irqchip_fwnode_ops;
EXPORT_SYMBOL_GPL(irqchip_fwnode_ops);

+struct irq_domain_request {
+ struct list_head list;
+ struct fwnode_handle *fwnode;
+ enum irq_domain_bus_token bus_token;
+ irq_domain_callback_t callback;
+ char *name;
+ void *priv;
+};
+
+static void irq_domain_call_handler(struct irq_domain *domain,
+ irq_domain_callback_t callback, const char *name, void *priv)
+{
+ int ret;
+
+ ret = callback(domain, priv);
+ if (ret)
+ pr_err("%s: Domain request handler failed: %d\n",
+ name, ret);
+
+ of_node_put(irq_domain_get_of_node(domain));
+}
+
+static void irq_domain_free_request(struct irq_domain_request *request)
+{
+ kfree(request->name);
+ kfree(request);
+}
+
+static void irq_domain_handle_requests(struct fwnode_handle *fwnode,
+ enum irq_domain_bus_token bus_token)
+{
+ struct irq_domain *domain;
+ struct irq_domain_request *request;
+
+ if (!fwnode)
+ return;
+redo:
+ domain = irq_find_matching_fwnode(fwnode, bus_token);
+ if (!domain)
+ return;
+
+ mutex_lock(&irq_domain_mutex);
+
+ /*
+ * For serialization of irq domain updates make sure to handle
+ * (and remove) the request only if the domain still matches
+ * the request.
+ */
+ if ((domain->fwnode != fwnode) || (domain->bus_token != bus_token)) {
+ mutex_unlock(&irq_domain_mutex);
+ goto redo;
+ }
+
+ list_for_each_entry(request, &irq_domain_requests, list) {
+ if (request->fwnode != fwnode ||
+ request->bus_token != bus_token)
+ continue;
+
+ list_del(&request->list);
+ mutex_unlock(&irq_domain_mutex);
+
+ irq_domain_call_handler(domain, request->callback,
+ request->name, request->priv);
+ irq_domain_free_request(request);
+
+ goto redo;
+ }
+
+ mutex_unlock(&irq_domain_mutex);
+}
+
+static int __init irq_domain_drain_requests(void)
+{
+ struct irq_domain_request *request;
+ struct irq_domain *domain;
+ int ret = 0;
+redo:
+ mutex_lock(&irq_domain_mutex);
+
+ list_for_each_entry(request, &irq_domain_requests, list) {
+ list_del(&request->list);
+ mutex_unlock(&irq_domain_mutex);
+
+ domain = irq_find_matching_fwnode(request->fwnode,
+ request->bus_token);
+ if (domain) {
+ irq_domain_call_handler(domain, request->callback,
+ request->name, request->priv);
+ } else {
+ ret = -ENODEV;
+ pr_err("%s-%d: Unhandled domain request\n",
+ request->name, request->bus_token);
+ }
+
+ irq_domain_free_request(request);
+
+ goto redo;
+ }
+
+ mutex_unlock(&irq_domain_mutex);
+
+ return ret;
+}
+late_initcall(irq_domain_drain_requests);
+
/**
* irq_domain_alloc_fwnode - Allocate a fwnode_handle suitable for
* identifying an irq domain
@@ -293,6 +399,8 @@ void irq_domain_update_bus_token(struct irq_domain *domain,
debugfs_add_domain_dir(domain);

mutex_unlock(&irq_domain_mutex);
+
+ irq_domain_handle_requests(domain->fwnode, bus_token);
}

/**
@@ -417,6 +525,61 @@ struct irq_domain *irq_find_matching_fwspec(struct irq_fwspec *fwspec,
EXPORT_SYMBOL_GPL(irq_find_matching_fwspec);

/**
+ * irq_domain_request_fwnode() - Requests a domain for a given fwspec
+ * @fwspec: FW specifier for an interrupt
+ * @bus_token: domain-specific data
+ * @callback: function to be called once domain becomes available
+ * @name: name to be used for fwnode
+ * @priv: private data to be passed to callback
+ *
+ * The callback function is called as soon as the domain is available.
+ */
+int irq_domain_request_fwnode(struct fwnode_handle *fwnode,
+ enum irq_domain_bus_token bus_token,
+ irq_domain_callback_t callback,
+ const char *name, void *priv)
+{
+ struct irq_domain *parent;
+ struct irq_domain_request *request;
+
+ if (!fwnode || bus_token == DOMAIN_BUS_ANY || !callback || !name)
+ return -EINVAL;
+
+ parent = irq_find_matching_fwnode(fwnode, bus_token);
+ if (parent) {
+ irq_domain_call_handler(parent, callback, name, priv);
+ return 0;
+ }
+
+ request = kzalloc(sizeof(*request), GFP_KERNEL);
+ if (!request)
+ return -ENOMEM;
+
+ request->fwnode = fwnode;
+ request->bus_token = bus_token;
+ request->callback = callback;
+ request->name = kstrdup(name, GFP_KERNEL);
+ request->priv = priv;
+ INIT_LIST_HEAD(&request->list);
+
+ if (!request->name) {
+ kfree(request);
+ return -ENOMEM;
+ }
+
+ of_node_get(to_of_node(fwnode));
+
+ mutex_lock(&irq_domain_mutex);
+ list_add_tail(&request->list, &irq_domain_requests);
+ mutex_unlock(&irq_domain_mutex);
+
+ /* recheck in case list changed */
+ irq_domain_handle_requests(fwnode, bus_token);
+
+ return 0;
+}
+
+/**
* irq_domain_check_msi_remap - Check whether all MSI irq domains implement
* IRQ remapping
*
--
2.11.0

2018-11-28 14:47:06

by Robert Richter

[permalink] [raw]
Subject: [PATCH v2 02/10] irqchip/gic-v3-its-platform-msi: Remove domain init order dependencies

Use new irq_domain_request_host_*() interface which allows independent
parent and child initialization using an irq domain request handler.
This makes it possible to move its initialization to a later point
during boot. All domains can be initialized in the same initcall level
then.

Signed-off-by: Robert Richter <[email protected]>
---
drivers/irqchip/irq-gic-v3-its-platform-msi.c | 54 +++++++++++++++++++++------
1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its-platform-msi.c b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
index 7b8e87b493fe..1f2849bc58c4 100644
--- a/drivers/irqchip/irq-gic-v3-its-platform-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
@@ -94,25 +94,56 @@ static const struct of_device_id its_device_id[] = {
{},
};

-static int __init its_pmsi_init_one(struct fwnode_handle *fwnode,
- const char *name)
+static int __init its_pmsi_create_irq_domain(struct irq_domain *parent,
+ void *priv)
{
- struct irq_domain *parent;
+ const char *name = priv;
+ int err = 0;

- parent = irq_find_matching_fwnode(fwnode, DOMAIN_BUS_NEXUS);
- if (!parent || !msi_get_domain_info(parent)) {
- pr_err("%s: unable to locate ITS domain\n", name);
- return -ENXIO;
+ if (!msi_get_domain_info(parent)) {
+ err = -ENODEV;
+ goto out;
}

- if (!platform_msi_create_irq_domain(fwnode, &its_pmsi_domain_info,
+ if (!platform_msi_create_irq_domain(parent->fwnode, &its_pmsi_domain_info,
parent)) {
- pr_err("%s: unable to create platform domain\n", name);
- return -ENXIO;
+ err = -ENXIO;
+ goto out;
}

pr_info("Platform MSI: %s domain created\n", name);
- return 0;
+out:
+ if (err)
+ pr_err("Platform MSI: Failed to create %s domain\n", name);
+
+ kfree(name);
+ return err;
+}
+
+static int __init its_pmsi_init_one(struct fwnode_handle *fwnode,
+ const char *name)
+{
+ void *priv = kstrdup(name, GFP_KERNEL);
+ int err;
+
+ if (!name) {
+ err = -EINVAL;
+ goto fail;
+ }
+
+ if (!priv) {
+ err = -ENOMEM;
+ goto fail;
+ }
+
+ err = irq_domain_request_fwnode(fwnode, DOMAIN_BUS_NEXUS,
+ its_pmsi_create_irq_domain, name, priv);
+ if (!err)
+ return 0;
+fail:
+ pr_err("Platform MSI: Failed to register %s domain\n", name);
+ kfree(priv);
+ return err;
}

#ifdef CONFIG_ACPI
@@ -135,7 +166,6 @@ its_pmsi_parse_madt(struct acpi_subtable_header *header,
}

err = its_pmsi_init_one(domain_handle, node_name);
-
out:
kfree(node_name);
return err;
--
2.11.0


2018-12-05 12:52:48

by Richter, Robert

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] irqdomain, gic-v3-its: Implement late irq domain initialization

Marc,

do you have any comments on this series? Please take a look at it.

There is also this one on top:

https://patchwork.kernel.org/cover/10685025/

Both series fix ITS table allocation for 4k page size and make the
upstream kernel working without the need of additional patches.

Thanks,

-Robert


On 28.11.18 15:43:02, Richter, Robert wrote:
> This patch series is a rework of the gic-v3-its initialization. It is
> in preparation of a further series that uses CMA memory allocation for
> ITS tables. For this the CMA framework must be available and thus ITS
> needs to be initialized after the arch_initcalls. This requires two
> major reworks.
>
> First we must remove all irq domain init order dependencies (patches
> #1-#5), second the ITS initialization must be split into an early
> probe and a later init part (patches #6-#10).
>
> Patch #1 introduces a new interface to request an irq domain, see the
> patch description for details. In patches #2-#5 all ITS related irq
> domains are converted to use the new interface. There are no order
> dependencies for parent and client irq domain initialization anymore
> which allows us to initialize all ITS irq domains in the same initcall
> (moving to the later subsys_initcall). Note that I do not have fsl
> hardware available, the code should work, but is only carefully
> reviewed and compile tested, please test on that hardware.
>
> The remaining patches #6-#10 are an incremental rework of the ITS
> initialization, basically splitting probe and init (patch #7) and
> separating it from gic-v3 code (patch #8). Patches #9 and #10 change
> initcall levels for various drivers.
>
> Patches have been tested with Cavium ThunderX and ThunderX2. I have an
> implementation of CMA ITS table allocation on top of this series
> available, I will send out patches for this in a couple of days.
>
> v2:
> * fix check for domain match in irq_domain_handle_requests()
> * add comment that describes this check in irq_domain_handle_
> requests()
>
> Robert Richter (10):
> irqdomain: Add interface to request an irq domain
> irqchip/gic-v3-its-platform-msi: Remove domain init order dependencies
> irqchip/irq-gic-v3-its-pci-msi: Remove domain init order dependencies
> irqchip/irq-gic-v3-its-fsl-mc-msi: Remove domain init order
> dependencies
> fsl-mc/dprc-driver: Remove domain init order dependencies
> irqchip/gic-v3-its: Initialize its nodes in probe order
> irqchip/gic-v3-its: Split probing from its node initialization
> irqchip/gic-v3-its: Decouple its initialization from gic
> irqchip/gic-v3-its: Initialize its nodes later
> irqchip/gic-v3-its: Initialize MSIs with subsys_initcalls
>
> drivers/bus/fsl-mc/dprc-driver.c | 41 +++++++
> drivers/bus/fsl-mc/fsl-mc-msi.c | 6 +-
> drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 49 +++++---
> drivers/irqchip/irq-gic-v3-its-pci-msi.c | 68 ++++++-----
> drivers/irqchip/irq-gic-v3-its-platform-msi.c | 56 +++++++--
> drivers/irqchip/irq-gic-v3-its.c | 160 ++++++++++++++++---------
> drivers/irqchip/irq-gic-v3.c | 12 +-
> include/linux/cpuhotplug.h | 1 +
> include/linux/irqchip/arm-gic-v3.h | 5 +-
> include/linux/irqdomain.h | 15 +++
> kernel/irq/irqdomain.c | 163 ++++++++++++++++++++++++++
> 11 files changed, 446 insertions(+), 130 deletions(-)
>
> --
> 2.11.0
>

2018-12-05 13:28:57

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] irqdomain, gic-v3-its: Implement late irq domain initialization

On 05/12/2018 12:50, Richter, Robert wrote:
> Marc,
>
> do you have any comments on this series? Please take a look at it.

It is on my list of stuff to review. Slowly getting there.

Thanks,

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

2018-12-13 14:23:56

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v2 01/10] irqdomain: Add interface to request an irq domain

Hi Robert,

On 28/11/2018 14:43, Robert Richter wrote:
> This patch introduces a new interface to allow irq domain
> initialization regardless of order dependencies. This is done by
> requesting a domain and registering a callback function that is called
> as soon as a domain becomes available.
>
> A typical irq domain initialization code is the following:
>
> Parent initialization:
>
> ...
> domain = msi_create_irq_domain(fwnode, info, parent);
> if (domain)
> irq_domain_update_bus_token(domain, bus_token);
> ...
>
> Child initialization:
>
> ...
> parent = irq_find_matching_fwnode(fwnode, bus_token);
> if (!parent)
> ...
> create_irq_domain(parent, ...);
> ...
>
> In case the parent is not yet available, the child initialization
> fails. Thus, the current implementation requires the parent domain
> being initialized before the child domain. With a complex irq domain
> hierarchy it becomes more and more difficult to grant that order as
> irq domains are enabled in separate subsystems. Care must be taken
> when initializing parent and child domains in the same initcall
> level. E.g. Arm's gic-v3-its implementation might have the following
> tree and dependencies:
>
> gic-v3
> ├── its-node-0
> │ ├── pci-host-0
> │ ├── platform-bus
> │ ...
> ├── its-node-1
> │ ├── pci-host-1
> ...
>
> All domains must be initialized in top-down order of the tree.
>
> This patch introduces an interface that allows domain initialization
> without any order requirements, e.g. to be able to initialize the irq
> domains in the same initcall level. The following functions have been
> introduced to allow the registration of a callback handler:
>
> irq_domain_request_fwnode()
> irq_domain_request_host()
>
> Instead of using the irq_find_matching_fwnode() function and it's
> variants the child code replaces them with the new functions and
> looks e.g. like the following:
>
> ...
> irq_domain_request_fwnode(fwnode, bus_token,
> create_irq_domain, name, priv);
> ...
>
> Here, the callback function create_irq_domain() is called as soon as
> the parent becomes available. All registered handlers are stored in a
> list. With each update of the bus token using irq_domain_update_bus_
> token(), the list is checked if that domain is requested by a handler
> and if that is the case it's callback function is called and the
> request removed from the list.
>
> With a late_initcall all requests from the list should already have
> been handled, otherwise all remaining requests are removed with an
> error reported.
>
> Signed-off-by: Robert Richter <[email protected]>
> ---
> include/linux/irqdomain.h | 15 +++++
> kernel/irq/irqdomain.c | 163 ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 178 insertions(+)
>
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index 068aa46f0d55..27e83803627d 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -311,6 +311,21 @@ static inline struct irq_domain *irq_find_host(struct device_node *node)
> return d;
> }
>
> +typedef int (*irq_domain_callback_t)(struct irq_domain *, void *);
> +int irq_domain_request_fwnode(struct fwnode_handle *fwnode,
> + enum irq_domain_bus_token bus_token,
> + irq_domain_callback_t callback,
> + const char *name, void *priv);
> +
> +static inline int irq_domain_request_host(struct device_node *node,
> + enum irq_domain_bus_token bus_token,
> + irq_domain_callback_t callback,
> + void *priv)
> +{
> + return irq_domain_request_fwnode(of_node_to_fwnode(node), bus_token,
> + callback, node->full_name, priv);
> +}

I don't think you need this helper. There is no backward compat to
preserve in this case, and I believe all the potential users would be
using the fwnode variant.

I'm not keen at all on the name thing though. An OF node already has
one, and an irqchip fwnode caries one as part of the irqchip_fwid structure.

> +
> /**
> * irq_domain_add_linear() - Allocate and register a linear revmap irq_domain.
> * @of_node: pointer to interrupt controller's device tree node.
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 3366d11c3e02..ebe628dad568 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -21,6 +21,7 @@
> #include <linux/fs.h>
>
> static LIST_HEAD(irq_domain_list);
> +static LIST_HEAD(irq_domain_requests);
> static DEFINE_MUTEX(irq_domain_mutex);
>
> static struct irq_domain *irq_default_domain;
> @@ -45,6 +46,111 @@ static inline void debugfs_remove_domain_dir(struct irq_domain *d) { }
> const struct fwnode_operations irqchip_fwnode_ops;
> EXPORT_SYMBOL_GPL(irqchip_fwnode_ops);
>
> +struct irq_domain_request {
> + struct list_head list;
> + struct fwnode_handle *fwnode;
> + enum irq_domain_bus_token bus_token;
> + irq_domain_callback_t callback;
> + char *name;
> + void *priv;
> +};
> +
> +static void irq_domain_call_handler(struct irq_domain *domain,
> + irq_domain_callback_t callback, const char *name, void *priv)
> +{
> + int ret;
> +
> + ret = callback(domain, priv);
> + if (ret)
> + pr_err("%s: Domain request handler failed: %d\n",
> + name, ret);
> +
> + of_node_put(irq_domain_get_of_node(domain));
> +}
> +
> +static void irq_domain_free_request(struct irq_domain_request *request)
> +{
> + kfree(request->name);
> + kfree(request);
> +}
> +
> +static void irq_domain_handle_requests(struct fwnode_handle *fwnode,
> + enum irq_domain_bus_token bus_token)
> +{
> + struct irq_domain *domain;
> + struct irq_domain_request *request;
> +
> + if (!fwnode)
> + return;
> +redo:
> + domain = irq_find_matching_fwnode(fwnode, bus_token);
> + if (!domain)
> + return;
> +
> + mutex_lock(&irq_domain_mutex);
> +
> + /*
> + * For serialization of irq domain updates make sure to handle
> + * (and remove) the request only if the domain still matches
> + * the request.
> + */
> + if ((domain->fwnode != fwnode) || (domain->bus_token != bus_token)) {
> + mutex_unlock(&irq_domain_mutex);
> + goto redo;
> + }
> +
> + list_for_each_entry(request, &irq_domain_requests, list) {
> + if (request->fwnode != fwnode ||
> + request->bus_token != bus_token)
> + continue;
> +
> + list_del(&request->list);
> + mutex_unlock(&irq_domain_mutex);
> +
> + irq_domain_call_handler(domain, request->callback,
> + request->name, request->priv);
> + irq_domain_free_request(request);
> +
> + goto redo;
> + }

This feels a bit convoluted. Pulling the various calls out of the loop
would be better.

> +
> + mutex_unlock(&irq_domain_mutex);
> +}
> +
> +static int __init irq_domain_drain_requests(void)
> +{
> + struct irq_domain_request *request;
> + struct irq_domain *domain;
> + int ret = 0;
> +redo:
> + mutex_lock(&irq_domain_mutex);
> +
> + list_for_each_entry(request, &irq_domain_requests, list) {
> + list_del(&request->list);
> + mutex_unlock(&irq_domain_mutex);
> +
> + domain = irq_find_matching_fwnode(request->fwnode,
> + request->bus_token);
> + if (domain) {
> + irq_domain_call_handler(domain, request->callback,
> + request->name, request->priv);
> + } else {
> + ret = -ENODEV;
> + pr_err("%s-%d: Unhandled domain request\n",
> + request->name, request->bus_token);
> + }
> +
> + irq_domain_free_request(request);
> +
> + goto redo;
> + }

Why isn't this written in terms of irq_domain_handle_requests instead?

> +
> + mutex_unlock(&irq_domain_mutex);
> +
> + return ret;
> +}
> +late_initcall(irq_domain_drain_requests);

Why do you have to do all matching in this late initcall? I'd have hoped
for requests to be satisfied as domains get added, not as a last ditch
effort.

> +
> /**
> * irq_domain_alloc_fwnode - Allocate a fwnode_handle suitable for
> * identifying an irq domain
> @@ -293,6 +399,8 @@ void irq_domain_update_bus_token(struct irq_domain *domain,
> debugfs_add_domain_dir(domain);
>
> mutex_unlock(&irq_domain_mutex);
> +
> + irq_domain_handle_requests(domain->fwnode, bus_token);
> }
>
> /**
> @@ -417,6 +525,61 @@ struct irq_domain *irq_find_matching_fwspec(struct irq_fwspec *fwspec,
> EXPORT_SYMBOL_GPL(irq_find_matching_fwspec);
>
> /**
> + * irq_domain_request_fwnode() - Requests a domain for a given fwspec
> + * @fwspec: FW specifier for an interrupt
> + * @bus_token: domain-specific data
> + * @callback: function to be called once domain becomes available
> + * @name: name to be used for fwnode
> + * @priv: private data to be passed to callback
> + *
> + * The callback function is called as soon as the domain is available.
> + */
> +int irq_domain_request_fwnode(struct fwnode_handle *fwnode,
> + enum irq_domain_bus_token bus_token,
> + irq_domain_callback_t callback,
> + const char *name, void *priv)
> +{
> + struct irq_domain *parent;
> + struct irq_domain_request *request;
> +
> + if (!fwnode || bus_token == DOMAIN_BUS_ANY || !callback || !name)

Why are you refusing DOMAIN_BUS_ANY? If that's what the caller wants, we
should satisfy it.

> + return -EINVAL;
> +
> + parent = irq_find_matching_fwnode(fwnode, bus_token);
> + if (parent) {
> + irq_domain_call_handler(parent, callback, name, priv);
> + return 0;
> + }
> +
> + request = kzalloc(sizeof(*request), GFP_KERNEL);
> + if (!request)
> + return -ENOMEM;
> +
> + request->fwnode = fwnode;
> + request->bus_token = bus_token;
> + request->callback = callback;
> + request->name = kstrdup(name, GFP_KERNEL);
> + request->priv = priv;
> + INIT_LIST_HEAD(&request->list);
> +
> + if (!request->name) {
> + kfree(request);
> + return -ENOMEM;
> + }
> +
> + of_node_get(to_of_node(fwnode));
> +
> + mutex_lock(&irq_domain_mutex);
> + list_add_tail(&request->list, &irq_domain_requests);
> + mutex_unlock(&irq_domain_mutex);
> +
> + /* recheck in case list changed */
> + irq_domain_handle_requests(fwnode, bus_token);

Why do you need this? The whole thing is racy anyway, and you already
have a catch-all as a late initcall...

> +
> + return 0;
> +}
> +
> +/**
> * irq_domain_check_msi_remap - Check whether all MSI irq domains implement
> * IRQ remapping
> *
>

Overall, this is a bit hackish. Why don't you resolve the deferred
requests as part of a domain being added instead? It would make things
much more straightforward.

Thanks,

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

2019-02-27 16:26:43

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] irqdomain, gic-v3-its: Implement late irq domain initialization

Hi Marc,

On 05/12/2018 14:27, Marc Zyngier wrote:
> On 05/12/2018 12:50, Richter, Robert wrote:
>> Marc,
>>
>> do you have any comments on this series? Please take a look at it.
>
> It is on my list of stuff to review. Slowly getting there.
>

Did you have time to review this patches?
This affects production ready hardware, so we would like to see a upstream
solution for that.

Regards,
Matthias

2019-02-27 16:58:11

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] irqdomain, gic-v3-its: Implement late irq domain initialization

On 27/02/2019 16:24, Matthias Brugger wrote:
> Hi Marc,
>
> On 05/12/2018 14:27, Marc Zyngier wrote:
>> On 05/12/2018 12:50, Richter, Robert wrote:
>>> Marc,
>>>
>>> do you have any comments on this series? Please take a look at it.
>>
>> It is on my list of stuff to review. Slowly getting there.
>>
>
> Did you have time to review this patches?
> This affects production ready hardware, so we would like to see a upstream
> solution for that.

I did: https://lkml.org/lkml/2018/12/13/459

As I said, it is a bit of a hack, but I'm not opposed to the idea. I'd
like it to be driven differently though. Instead of requiring a new
interface, I'd like it to be resolved on demand as domains get
requested. This would allow for some of the more exotic stuff to be
never created (platform MSI anyone?), because no device needs it.

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