[PATCH v2 0/8] irqchip/gic-v3-its: Implement ITS nodes as kernel devices and use CMA

[PATCH v2 0/8] irqchip/gic-v3-its: Implement ITS nodes as kernel devices and use CMA

[Robert Richter]

This patch series implements ITS nodes as kernel devices. The
advantage of that is that CMA can be used to allocate large ITS
tables, where standard memory and page allocation fails (above
MAX_ORDER - 1). This approach was suggested first here:

 https://marc.info/?i=56D44812.6000309%40arm.com

Another advantage is that all memory resources of the device are now
device managed, thus the code for releasing the device becomes much
easier.

The following is required to implement this:

 * each ITS node must be assigned to a struct device each,

 * the ITS initialization must be moved to a later point during boot,

 * the ITS enablement must be separated from the GIC code (the GIC
   still needs to be enabled early),

 * the early ITS node probing by DT or ACPI must be separated from the
   ITS initialization,

 * ITS table allocation must be changed to use dma_alloc_coherent()
   which uses CMA for allocation of large memory ranges.

V2:
 * rebased onto v4.11-rc1,
 * fixed syntax error in its_init() function (split probing patch),
 * added comment in its_create_device(),
 * fixed GITS_BASER_PAGE_SIZE_MASK usage in its_setup_baser()


Robert Richter (8):
  irqchip/gic-v3-its: Initialize its nodes in probe order
  irqchip/gic-v3-its: Initialize MSIs with subsys_initcalls
  irqchip/gic-v3-its: Split probing from its node initialization
  irqchip/gic-v3-its: Decouple its initialization from gic
  irqchip/gic-v3-its: Prevent its init ordering dependencies
  irqchip/gic-v3-its: Initialize its nodes later
  irqchip/gic-v3-its: Handle its nodes as kernel devices
  irqchip, gicv3-its, cma: Use CMA for allocation of large device tables

 drivers/irqchip/irq-gic-v3-its-pci-msi.c      |   6 +-
 drivers/irqchip/irq-gic-v3-its-platform-msi.c |   6 +-
 drivers/irqchip/irq-gic-v3-its.c              | 276 ++++++++++++++++----------
 drivers/irqchip/irq-gic-v3.c                  |   8 +-
 include/linux/cpuhotplug.h                    |   1 +
 include/linux/irqchip/arm-gic-v3.h            |   5 +-
 6 files changed, 186 insertions(+), 116 deletions(-)

-- 
2.11.0

[PATCH v2 0/8] irqchip/gic-v3-its: Implement ITS nodes as kernel devices and use CMA

[Robert Richter]

This patch series implements ITS nodes as kernel devices. The
advantage of that is that CMA can be used to allocate large ITS
tables, where standard memory and page allocation fails (above
MAX_ORDER - 1). This approach was suggested first here:

 https://marc.info/?i=56D44812.6000309%40arm.com

Another advantage is that all memory resources of the device are now
device managed, thus the code for releasing the device becomes much
easier.

The following is required to implement this:

 * each ITS node must be assigned to a struct device each,

 * the ITS initialization must be moved to a later point during boot,

 * the ITS enablement must be separated from the GIC code (the GIC
   still needs to be enabled early),

 * the early ITS node probing by DT or ACPI must be separated from the
   ITS initialization,

 * ITS table allocation must be changed to use dma_alloc_coherent()
   which uses CMA for allocation of large memory ranges.

V2:
 * rebased onto v4.11-rc1,
 * fixed syntax error in its_init() function (split probing patch),
 * added comment in its_create_device(),
 * fixed GITS_BASER_PAGE_SIZE_MASK usage in its_setup_baser()


Robert Richter (8):
  irqchip/gic-v3-its: Initialize its nodes in probe order
  irqchip/gic-v3-its: Initialize MSIs with subsys_initcalls
  irqchip/gic-v3-its: Split probing from its node initialization
  irqchip/gic-v3-its: Decouple its initialization from gic
  irqchip/gic-v3-its: Prevent its init ordering dependencies
  irqchip/gic-v3-its: Initialize its nodes later
  irqchip/gic-v3-its: Handle its nodes as kernel devices
  irqchip, gicv3-its, cma: Use CMA for allocation of large device tables

 drivers/irqchip/irq-gic-v3-its-pci-msi.c      |   6 +-
 drivers/irqchip/irq-gic-v3-its-platform-msi.c |   6 +-
 drivers/irqchip/irq-gic-v3-its.c              | 276 ++++++++++++++++----------
 drivers/irqchip/irq-gic-v3.c                  |   8 +-
 include/linux/cpuhotplug.h                    |   1 +
 include/linux/irqchip/arm-gic-v3.h            |   5 +-
 6 files changed, 186 insertions(+), 116 deletions(-)

-- 
2.11.0

[PATCH v2 1/8] irqchip/gic-v3-its: Initialize its nodes in probe order

[Robert Richter]

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 <rrichter@cavium.com>
---
 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 23201004fd7a..ef00ffa92915 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1749,7 +1749,7 @@ static int __init its_probe_one(struct resource *res,
 		goto out_free_tables;
 
 	spin_lock(&its_lock);
-	list_add(&its->entry, &its_nodes);
+	list_add_tail(&its->entry, &its_nodes);
 	spin_unlock(&its_lock);
 
 	return 0;
-- 
2.11.0

[PATCH v2 1/8] irqchip/gic-v3-its: Initialize its nodes in probe order

[Robert Richter]

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 <rrichter@cavium.com>
---
 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 23201004fd7a..ef00ffa92915 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1749,7 +1749,7 @@ static int __init its_probe_one(struct resource *res,
 		goto out_free_tables;
 
 	spin_lock(&its_lock);
-	list_add(&its->entry, &its_nodes);
+	list_add_tail(&its->entry, &its_nodes);
 	spin_unlock(&its_lock);
 
 	return 0;
-- 
2.11.0

[PATCH v2 2/8] irqchip/gic-v3-its: Initialize MSIs with subsys_initcalls

[Robert Richter]

This allows us to use kernel core functionality (e.g. cma) for ITS
initialization. MSIs must be up before the device_initcalls (pci and
platform device probe) and after arch_initcalls (dma init), so
subsys_initcall is fine.

Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 drivers/irqchip/irq-gic-v3-its-pci-msi.c      | 2 +-
 drivers/irqchip/irq-gic-v3-its-platform-msi.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its-pci-msi.c b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
index aee1c60d7ab5..dace9bc4ef8d 100644
--- a/drivers/irqchip/irq-gic-v3-its-pci-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
@@ -194,4 +194,4 @@ static int __init its_pci_msi_init(void)
 
 	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 470b4aa7d62c..7d8c19973766 100644
--- a/drivers/irqchip/irq-gic-v3-its-platform-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
@@ -103,4 +103,4 @@ static int __init its_pmsi_init(void)
 
 	return 0;
 }
-early_initcall(its_pmsi_init);
+subsys_initcall(its_pmsi_init);
-- 
2.11.0

[PATCH v2 2/8] irqchip/gic-v3-its: Initialize MSIs with subsys_initcalls

[Robert Richter]

This allows us to use kernel core functionality (e.g. cma) for ITS
initialization. MSIs must be up before the device_initcalls (pci and
platform device probe) and after arch_initcalls (dma init), so
subsys_initcall is fine.

Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 drivers/irqchip/irq-gic-v3-its-pci-msi.c      | 2 +-
 drivers/irqchip/irq-gic-v3-its-platform-msi.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its-pci-msi.c b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
index aee1c60d7ab5..dace9bc4ef8d 100644
--- a/drivers/irqchip/irq-gic-v3-its-pci-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
@@ -194,4 +194,4 @@ static int __init its_pci_msi_init(void)
 
 	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 470b4aa7d62c..7d8c19973766 100644
--- a/drivers/irqchip/irq-gic-v3-its-platform-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
@@ -103,4 +103,4 @@ static int __init its_pmsi_init(void)
 
 	return 0;
 }
-early_initcall(its_pmsi_init);
+subsys_initcall(its_pmsi_init);
-- 
2.11.0

[PATCH v2 3/8] irqchip/gic-v3-its: Split probing from its node initialization

[Robert Richter]

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 <rrichter@cavium.com>
---
 drivers/irqchip/irq-gic-v3-its.c   | 97 ++++++++++++++++++++++++++------------
 drivers/irqchip/irq-gic-v3.c       |  2 +-
 include/linux/irqchip/arm-gic-v3.h |  4 +-
 3 files changed, 69 insertions(+), 34 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index ef00ffa92915..a0ab1b83c548 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -75,10 +75,12 @@ struct its_baser {
  * list of devices writing to it.
  */
 struct its_node {
+	struct fwnode_handle	*fwnode;
 	raw_spinlock_t		lock;
 	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];
@@ -1629,7 +1631,7 @@ static void its_enable_quirks(struct its_node *its)
 	gic_enable_quirks(iidr, its_quirks, its);
 }
 
-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;
@@ -1638,7 +1640,7 @@ 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, &its_domain_ops, its);
 	if (!inner_domain) {
 		kfree(info);
 		return -ENOMEM;
@@ -1654,55 +1656,83 @@ static int its_init_domain(struct fwnode_handle *handle, struct its_node *its)
 	return 0;
 }
 
+static void its_free(struct its_node *its)
+{
+	spin_lock(&its_lock);
+	list_del(&its->entry);
+	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;
+	its->phys_base = res->start;
+	its->phys_size = resource_size(res);
+	its->numa_node = numa_node;
+
+	spin_lock(&its_lock);
+	list_add_tail(&its->entry, &its_nodes);
+	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;
 	u64 baser, tmp;
 	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);
-		goto out_unmap;
-	}
-
-	pr_info("ITS %pR\n", res);
-
-	its = kzalloc(sizeof(*its), GFP_KERNEL);
-	if (!its) {
-		err = -ENOMEM;
+		pr_warn("ITS@%pa: Failed to quiesce, giving up\n", &its->phys_base);
 		goto out_unmap;
 	}
 
-	raw_spin_lock_init(&its->lock);
-	INIT_LIST_HEAD(&its->entry);
-	INIT_LIST_HEAD(&its->its_device_list);
 	its->base = its_base;
-	its->phys_base = res->start;
 	its->ite_size = ((gic_read_typer(its_base + GITS_TYPER) >> 4) & 0xf) + 1;
-	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;
 
@@ -1744,13 +1774,11 @@ static int __init its_probe_one(struct resource *res,
 	gits_write_cwriter(0, its->base + GITS_CWRITER);
 	writel_relaxed(GITS_CTLR_ENABLE, its->base + GITS_CTLR);
 
-	err = its_init_domain(handle, its);
+	err = its_init_domain(its);
 	if (err)
 		goto out_free_tables;
 
-	spin_lock(&its_lock);
-	list_add_tail(&its->entry, &its_nodes);
-	spin_unlock(&its_lock);
+	pr_info("ITS@%pa: ITS node added\n", &its->phys_base);
 
 	return 0;
 
@@ -1758,11 +1786,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;
 }
 
@@ -1864,8 +1891,10 @@ 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;
 
@@ -1882,8 +1911,14 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
 	}
 
 	gic_rdists = rdists;
+
+	return its_init();
+}
+
+static int __init its_init(void)
+{
 	its_alloc_lpi_tables();
-	its_lpi_init(rdists->id_bits);
+	its_lpi_init(gic_rdists->id_bits);
 
 	return 0;
 }
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index c132f29322cc..b46dc9396331 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -949,7 +949,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())
-		its_init(handle, &gic_data.rdists, gic_data.domain);
+		its_probe(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 672cfef72fc8..a7dc1e3030ef 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -466,8 +466,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);
 
 static inline bool gic_enable_sre(void)
 {
-- 
2.11.0

[PATCH v2 3/8] irqchip/gic-v3-its: Split probing from its node initialization

[Robert Richter]

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 <rrichter@cavium.com>
---
 drivers/irqchip/irq-gic-v3-its.c   | 97 ++++++++++++++++++++++++++------------
 drivers/irqchip/irq-gic-v3.c       |  2 +-
 include/linux/irqchip/arm-gic-v3.h |  4 +-
 3 files changed, 69 insertions(+), 34 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index ef00ffa92915..a0ab1b83c548 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -75,10 +75,12 @@ struct its_baser {
  * list of devices writing to it.
  */
 struct its_node {
+	struct fwnode_handle	*fwnode;
 	raw_spinlock_t		lock;
 	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];
@@ -1629,7 +1631,7 @@ static void its_enable_quirks(struct its_node *its)
 	gic_enable_quirks(iidr, its_quirks, its);
 }
 
-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;
@@ -1638,7 +1640,7 @@ 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, &its_domain_ops, its);
 	if (!inner_domain) {
 		kfree(info);
 		return -ENOMEM;
@@ -1654,55 +1656,83 @@ static int its_init_domain(struct fwnode_handle *handle, struct its_node *its)
 	return 0;
 }
 
+static void its_free(struct its_node *its)
+{
+	spin_lock(&its_lock);
+	list_del(&its->entry);
+	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;
+	its->phys_base = res->start;
+	its->phys_size = resource_size(res);
+	its->numa_node = numa_node;
+
+	spin_lock(&its_lock);
+	list_add_tail(&its->entry, &its_nodes);
+	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;
 	u64 baser, tmp;
 	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);
-		goto out_unmap;
-	}
-
-	pr_info("ITS %pR\n", res);
-
-	its = kzalloc(sizeof(*its), GFP_KERNEL);
-	if (!its) {
-		err = -ENOMEM;
+		pr_warn("ITS@%pa: Failed to quiesce, giving up\n", &its->phys_base);
 		goto out_unmap;
 	}
 
-	raw_spin_lock_init(&its->lock);
-	INIT_LIST_HEAD(&its->entry);
-	INIT_LIST_HEAD(&its->its_device_list);
 	its->base = its_base;
-	its->phys_base = res->start;
 	its->ite_size = ((gic_read_typer(its_base + GITS_TYPER) >> 4) & 0xf) + 1;
-	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;
 
@@ -1744,13 +1774,11 @@ static int __init its_probe_one(struct resource *res,
 	gits_write_cwriter(0, its->base + GITS_CWRITER);
 	writel_relaxed(GITS_CTLR_ENABLE, its->base + GITS_CTLR);
 
-	err = its_init_domain(handle, its);
+	err = its_init_domain(its);
 	if (err)
 		goto out_free_tables;
 
-	spin_lock(&its_lock);
-	list_add_tail(&its->entry, &its_nodes);
-	spin_unlock(&its_lock);
+	pr_info("ITS@%pa: ITS node added\n", &its->phys_base);
 
 	return 0;
 
@@ -1758,11 +1786,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;
 }
 
@@ -1864,8 +1891,10 @@ 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;
 
@@ -1882,8 +1911,14 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
 	}
 
 	gic_rdists = rdists;
+
+	return its_init();
+}
+
+static int __init its_init(void)
+{
 	its_alloc_lpi_tables();
-	its_lpi_init(rdists->id_bits);
+	its_lpi_init(gic_rdists->id_bits);
 
 	return 0;
 }
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index c132f29322cc..b46dc9396331 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -949,7 +949,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())
-		its_init(handle, &gic_data.rdists, gic_data.domain);
+		its_probe(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 672cfef72fc8..a7dc1e3030ef 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -466,8 +466,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);
 
 static inline bool gic_enable_sre(void)
 {
-- 
2.11.0

[PATCH v2 4/8] irqchip/gic-v3-its: Decouple its initialization from gic

[Robert Richter]

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 <rrichter@cavium.com>
---
 drivers/irqchip/irq-gic-v3-its.c   | 55 ++++++++++++++++++++++++++------------
 drivers/irqchip/irq-gic-v3.c       | 11 ++++----
 include/linux/cpuhotplug.h         |  1 +
 include/linux/irqchip/arm-gic-v3.h |  2 +-
 4 files changed, 46 insertions(+), 23 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index a0ab1b83c548..8badc230af4b 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1658,20 +1658,13 @@ static int its_init_domain(struct its_node *its)
 
 static void its_free(struct its_node *its)
 {
-	spin_lock(&its_lock);
-	list_del(&its->entry);
-	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)
@@ -1691,11 +1684,7 @@ static int __init its_probe_one(struct resource *res,
 
 	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)
@@ -1798,7 +1787,7 @@ static bool gic_rdists_supports_plpis(void)
 	return !!(gic_read_typer(gic_data_rdist_rd_base() + GICR_TYPER) & GICR_TYPER_PLPIS);
 }
 
-int its_cpu_init(void)
+static int its_cpu_init(unsigned int cpu)
 {
 	if (!list_empty(&its_nodes)) {
 		if (!gic_rdists_supports_plpis()) {
@@ -1891,8 +1880,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)
 {
@@ -1912,13 +1899,47 @@ 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, *tmp;
+	int err = 0, err2;
+
+	if (list_empty(&its_nodes))
+		return 0;
+
+	spin_lock(&its_lock);
+
+	list_for_each_entry(its, &its_nodes, entry) {
+		err2 = its_init_one(its);
+		if (!err && err2)
+			err = err2;
+	}
+
+	if (!err)
+		goto unlock;
+
+	list_for_each_entry_safe(its, tmp, &its_nodes, entry) {
+		list_del(&its->entry);
+		its_free(its);
+	}
+unlock:
+	spin_unlock(&its_lock);
+
+	if (err) {
+		pr_warn("ITS: Failed to initialize (%d), not enabling LPIs\n",
+			err);
+		return err;
+	}
+
 	its_alloc_lpi_tables();
 	its_lpi_init(gic_rdists->id_bits);
 
+	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 b46dc9396331..2b9be256a9ec 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -536,10 +536,6 @@ static void gic_cpu_init(void)
 
 	gic_cpu_config(rbase, gic_redist_wait_for_rwp);
 
-	/* Give LPIs a spin */
-	if (IS_ENABLED(CONFIG_ARM_GIC_V3_ITS) && gic_dist_supports_lpis())
-		its_cpu_init();
-
 	/* initialise system registers */
 	gic_cpu_sys_reg_init();
 }
@@ -672,7 +668,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 */
@@ -1168,6 +1164,9 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare
 
 	gic_populate_ppi_partitions(node);
 	gic_of_setup_kvm_info(node);
+
+	its_init();
+
 	return 0;
 
 out_unmap_rdist:
@@ -1457,6 +1456,8 @@ gic_acpi_init(struct acpi_subtable_header *header, const unsigned long end)
 	acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, domain_handle);
 	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 62d240e962f0..649122ceff81 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -79,6 +79,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 a7dc1e3030ef..bf256ce1715d 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -465,9 +465,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);
 
 static inline bool gic_enable_sre(void)
 {
-- 
2.11.0

[PATCH v2 4/8] irqchip/gic-v3-its: Decouple its initialization from gic

[Robert Richter]

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 <rrichter@cavium.com>
---
 drivers/irqchip/irq-gic-v3-its.c   | 55 ++++++++++++++++++++++++++------------
 drivers/irqchip/irq-gic-v3.c       | 11 ++++----
 include/linux/cpuhotplug.h         |  1 +
 include/linux/irqchip/arm-gic-v3.h |  2 +-
 4 files changed, 46 insertions(+), 23 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index a0ab1b83c548..8badc230af4b 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1658,20 +1658,13 @@ static int its_init_domain(struct its_node *its)
 
 static void its_free(struct its_node *its)
 {
-	spin_lock(&its_lock);
-	list_del(&its->entry);
-	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)
@@ -1691,11 +1684,7 @@ static int __init its_probe_one(struct resource *res,
 
 	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)
@@ -1798,7 +1787,7 @@ static bool gic_rdists_supports_plpis(void)
 	return !!(gic_read_typer(gic_data_rdist_rd_base() + GICR_TYPER) & GICR_TYPER_PLPIS);
 }
 
-int its_cpu_init(void)
+static int its_cpu_init(unsigned int cpu)
 {
 	if (!list_empty(&its_nodes)) {
 		if (!gic_rdists_supports_plpis()) {
@@ -1891,8 +1880,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)
 {
@@ -1912,13 +1899,47 @@ 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, *tmp;
+	int err = 0, err2;
+
+	if (list_empty(&its_nodes))
+		return 0;
+
+	spin_lock(&its_lock);
+
+	list_for_each_entry(its, &its_nodes, entry) {
+		err2 = its_init_one(its);
+		if (!err && err2)
+			err = err2;
+	}
+
+	if (!err)
+		goto unlock;
+
+	list_for_each_entry_safe(its, tmp, &its_nodes, entry) {
+		list_del(&its->entry);
+		its_free(its);
+	}
+unlock:
+	spin_unlock(&its_lock);
+
+	if (err) {
+		pr_warn("ITS: Failed to initialize (%d), not enabling LPIs\n",
+			err);
+		return err;
+	}
+
 	its_alloc_lpi_tables();
 	its_lpi_init(gic_rdists->id_bits);
 
+	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 b46dc9396331..2b9be256a9ec 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -536,10 +536,6 @@ static void gic_cpu_init(void)
 
 	gic_cpu_config(rbase, gic_redist_wait_for_rwp);
 
-	/* Give LPIs a spin */
-	if (IS_ENABLED(CONFIG_ARM_GIC_V3_ITS) && gic_dist_supports_lpis())
-		its_cpu_init();
-
 	/* initialise system registers */
 	gic_cpu_sys_reg_init();
 }
@@ -672,7 +668,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 */
@@ -1168,6 +1164,9 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare
 
 	gic_populate_ppi_partitions(node);
 	gic_of_setup_kvm_info(node);
+
+	its_init();
+
 	return 0;
 
 out_unmap_rdist:
@@ -1457,6 +1456,8 @@ gic_acpi_init(struct acpi_subtable_header *header, const unsigned long end)
 	acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, domain_handle);
 	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 62d240e962f0..649122ceff81 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -79,6 +79,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 a7dc1e3030ef..bf256ce1715d 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -465,9 +465,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);
 
 static inline bool gic_enable_sre(void)
 {
-- 
2.11.0

[PATCH v2 5/8] irqchip/gic-v3-its: Prevent its init ordering dependencies

[Robert Richter]

Right now its_init() must be called before pci and platform init.
Remove ordering dependencies to allow all initialization functions
being called with the same initcall type.

Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 drivers/irqchip/irq-gic-v3-its-pci-msi.c      | 4 +++-
 drivers/irqchip/irq-gic-v3-its-platform-msi.c | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its-pci-msi.c b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
index dace9bc4ef8d..afe78f224181 100644
--- a/drivers/irqchip/irq-gic-v3-its-pci-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
@@ -77,6 +77,8 @@ static int its_pci_msi_prepare(struct irq_domain *domain, struct device *dev,
 		return -EINVAL;
 
 	msi_info = msi_get_domain_info(domain->parent);
+	if (!msi_info)
+		return -ENODEV;
 
 	pdev = to_pci_dev(dev);
 	dev_alias.pdev = pdev;
@@ -113,7 +115,7 @@ static int __init its_pci_msi_init_one(struct fwnode_handle *handle,
 	struct irq_domain *parent;
 
 	parent = irq_find_matching_fwnode(handle, DOMAIN_BUS_NEXUS);
-	if (!parent || !msi_get_domain_info(parent)) {
+	if (!parent) {
 		pr_err("%s: Unable to locate ITS domain\n", name);
 		return -ENXIO;
 	}
diff --git a/drivers/irqchip/irq-gic-v3-its-platform-msi.c b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
index 7d8c19973766..22ba1dfed8ba 100644
--- a/drivers/irqchip/irq-gic-v3-its-platform-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
@@ -32,6 +32,8 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
 	int ret, index = 0;
 
 	msi_info = msi_get_domain_info(domain->parent);
+	if (!msi_info)
+		return -ENODEV;
 
 	/* Suck the DeviceID out of the msi-parent property */
 	do {
@@ -84,7 +86,7 @@ static int __init its_pmsi_init(void)
 			continue;
 
 		parent = irq_find_matching_host(np, DOMAIN_BUS_NEXUS);
-		if (!parent || !msi_get_domain_info(parent)) {
+		if (!parent) {
 			pr_err("%s: unable to locate ITS domain\n",
 			       np->full_name);
 			continue;
-- 
2.11.0

[PATCH v2 5/8] irqchip/gic-v3-its: Prevent its init ordering dependencies

[Robert Richter]

Right now its_init() must be called before pci and platform init.
Remove ordering dependencies to allow all initialization functions
being called with the same initcall type.

Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 drivers/irqchip/irq-gic-v3-its-pci-msi.c      | 4 +++-
 drivers/irqchip/irq-gic-v3-its-platform-msi.c | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its-pci-msi.c b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
index dace9bc4ef8d..afe78f224181 100644
--- a/drivers/irqchip/irq-gic-v3-its-pci-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-pci-msi.c
@@ -77,6 +77,8 @@ static int its_pci_msi_prepare(struct irq_domain *domain, struct device *dev,
 		return -EINVAL;
 
 	msi_info = msi_get_domain_info(domain->parent);
+	if (!msi_info)
+		return -ENODEV;
 
 	pdev = to_pci_dev(dev);
 	dev_alias.pdev = pdev;
@@ -113,7 +115,7 @@ static int __init its_pci_msi_init_one(struct fwnode_handle *handle,
 	struct irq_domain *parent;
 
 	parent = irq_find_matching_fwnode(handle, DOMAIN_BUS_NEXUS);
-	if (!parent || !msi_get_domain_info(parent)) {
+	if (!parent) {
 		pr_err("%s: Unable to locate ITS domain\n", name);
 		return -ENXIO;
 	}
diff --git a/drivers/irqchip/irq-gic-v3-its-platform-msi.c b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
index 7d8c19973766..22ba1dfed8ba 100644
--- a/drivers/irqchip/irq-gic-v3-its-platform-msi.c
+++ b/drivers/irqchip/irq-gic-v3-its-platform-msi.c
@@ -32,6 +32,8 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
 	int ret, index = 0;
 
 	msi_info = msi_get_domain_info(domain->parent);
+	if (!msi_info)
+		return -ENODEV;
 
 	/* Suck the DeviceID out of the msi-parent property */
 	do {
@@ -84,7 +86,7 @@ static int __init its_pmsi_init(void)
 			continue;
 
 		parent = irq_find_matching_host(np, DOMAIN_BUS_NEXUS);
-		if (!parent || !msi_get_domain_info(parent)) {
+		if (!parent) {
 			pr_err("%s: unable to locate ITS domain\n",
 			       np->full_name);
 			continue;
-- 
2.11.0

[PATCH v2 6/8] irqchip/gic-v3-its: Initialize its nodes later

[Robert Richter]

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 <rrichter@cavium.com>
---
 drivers/irqchip/irq-gic-v3-its.c   | 3 ++-
 drivers/irqchip/irq-gic-v3.c       | 5 -----
 include/linux/irqchip/arm-gic-v3.h | 1 -
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 8badc230af4b..c0eaae35c631 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1902,7 +1902,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, *tmp;
 	int err = 0, err2;
@@ -1943,3 +1943,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 2b9be256a9ec..69da7c1ccbf2 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1164,9 +1164,6 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare
 
 	gic_populate_ppi_partitions(node);
 	gic_of_setup_kvm_info(node);
-
-	its_init();
-
 	return 0;
 
 out_unmap_rdist:
@@ -1456,8 +1453,6 @@ gic_acpi_init(struct acpi_subtable_header *header, const unsigned long end)
 	acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, domain_handle);
 	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 bf256ce1715d..41b7a5e96b80 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -467,7 +467,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);
 
 static inline bool gic_enable_sre(void)
 {
-- 
2.11.0

[PATCH v2 6/8] irqchip/gic-v3-its: Initialize its nodes later

[Robert Richter]

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 <rrichter@cavium.com>
---
 drivers/irqchip/irq-gic-v3-its.c   | 3 ++-
 drivers/irqchip/irq-gic-v3.c       | 5 -----
 include/linux/irqchip/arm-gic-v3.h | 1 -
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 8badc230af4b..c0eaae35c631 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1902,7 +1902,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, *tmp;
 	int err = 0, err2;
@@ -1943,3 +1943,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 2b9be256a9ec..69da7c1ccbf2 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1164,9 +1164,6 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare
 
 	gic_populate_ppi_partitions(node);
 	gic_of_setup_kvm_info(node);
-
-	its_init();
-
 	return 0;
 
 out_unmap_rdist:
@@ -1456,8 +1453,6 @@ gic_acpi_init(struct acpi_subtable_header *header, const unsigned long end)
 	acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, domain_handle);
 	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 bf256ce1715d..41b7a5e96b80 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -467,7 +467,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);
 
 static inline bool gic_enable_sre(void)
 {
-- 
2.11.0

[PATCH v2 7/8] irqchip/gic-v3-its: Handle its nodes as kernel devices

[Robert Richter]

Manage its nodes as kernel devices. We can then use the kernel's
device resource management for memory allocation. Freeing memory
becomes much easier now. This also allows us to use CMA for the
allocation of large its tables.

Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 118 ++++++++++++++++++++-------------------
 1 file changed, 60 insertions(+), 58 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index c0eaae35c631..6625b3a505f0 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -19,6 +19,7 @@
 #include <linux/bitmap.h>
 #include <linux/cpu.h>
 #include <linux/delay.h>
+#include <linux/device.h>
 #include <linux/dma-iommu.h>
 #include <linux/interrupt.h>
 #include <linux/irqdomain.h>
@@ -75,6 +76,7 @@ struct its_baser {
  * list of devices writing to it.
  */
 struct its_node {
+	struct device		dev;
 	struct fwnode_handle	*fwnode;
 	raw_spinlock_t		lock;
 	struct list_head	entry;
@@ -400,7 +402,7 @@ static struct its_cmd_block *its_allocate_entry(struct its_node *its)
 	while (its_queue_full(its)) {
 		count--;
 		if (!count) {
-			pr_err_ratelimited("ITS queue not draining\n");
+			dev_err_ratelimited(&its->dev, "ITS queue not draining\n");
 			return NULL;
 		}
 		cpu_relax();
@@ -460,7 +462,7 @@ static void its_wait_for_range_completion(struct its_node *its,
 
 		count--;
 		if (!count) {
-			pr_err_ratelimited("ITS queue timeout\n");
+			dev_err_ratelimited(&its->dev, "ITS queue timeout\n");
 			return;
 		}
 		cpu_relax();
@@ -480,7 +482,7 @@ static void its_send_single_command(struct its_node *its,
 
 	cmd = its_allocate_entry(its);
 	if (!cmd) {		/* We're soooooo screewed... */
-		pr_err_ratelimited("ITS can't allocate, dropping command\n");
+		dev_err_ratelimited(&its->dev, "ITS can't allocate, dropping command\n");
 		raw_spin_unlock_irqrestore(&its->lock, flags);
 		return;
 	}
@@ -490,7 +492,7 @@ static void its_send_single_command(struct its_node *its,
 	if (sync_col) {
 		sync_cmd = its_allocate_entry(its);
 		if (!sync_cmd) {
-			pr_err_ratelimited("ITS can't SYNC, skipping\n");
+			dev_err_ratelimited(&its->dev, "ITS can't SYNC, skipping\n");
 			goto post;
 		}
 		its_encode_cmd(sync_cmd, GITS_CMD_SYNC);
@@ -867,14 +869,15 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
 retry_alloc_baser:
 	alloc_pages = (PAGE_ORDER_TO_SIZE(order) / psz);
 	if (alloc_pages > GITS_BASER_PAGES_MAX) {
-		pr_warn("ITS@%pa: %s too large, reduce ITS pages %u->%u\n",
-			&its->phys_base, its_base_type_string[type],
-			alloc_pages, GITS_BASER_PAGES_MAX);
+		dev_warn(&its->dev, "%s too large, reduce ITS pages %u->%u\n",
+			its_base_type_string[type], alloc_pages,
+			GITS_BASER_PAGES_MAX);
 		alloc_pages = GITS_BASER_PAGES_MAX;
 		order = get_order(GITS_BASER_PAGES_MAX * psz);
 	}
 
-	base = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, order);
+	base = (void *)devm_get_free_pages(&its->dev, GFP_KERNEL | __GFP_ZERO,
+					   order);
 	if (!base)
 		return -ENOMEM;
 
@@ -926,7 +929,7 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
 		 * size and retry. If we reach 4K, then
 		 * something is horribly wrong...
 		 */
-		free_pages((unsigned long)base, order);
+		devm_free_pages(&its->dev, (unsigned long)base);
 		baser->base = NULL;
 
 		switch (psz) {
@@ -940,10 +943,9 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
 	}
 
 	if (val != tmp) {
-		pr_err("ITS@%pa: %s doesn't stick: %llx %llx\n",
-		       &its->phys_base, its_base_type_string[type],
-		       val, tmp);
-		free_pages((unsigned long)base, order);
+		dev_err(&its->dev, "%s doesn't stick: %llx %llx\n",
+		       its_base_type_string[type], val, tmp);
+		devm_free_pages(&its->dev, (unsigned long)base);
 		return -ENXIO;
 	}
 
@@ -952,8 +954,8 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
 	baser->psz = psz;
 	tmp = indirect ? GITS_LVL1_ENTRY_SIZE : esz;
 
-	pr_info("ITS@%pa: allocated %d %s @%lx (%s, esz %d, psz %dK, shr %d)\n",
-		&its->phys_base, (int)(PAGE_ORDER_TO_SIZE(order) / (int)tmp),
+	dev_info(&its->dev, "allocated %d %s @%lx (%s, esz %d, psz %dK, shr %d)\n",
+		(int)(PAGE_ORDER_TO_SIZE(order) / (int)tmp),
 		its_base_type_string[type],
 		(unsigned long)virt_to_phys(base),
 		indirect ? "indirect" : "flat", (int)esz,
@@ -1004,8 +1006,8 @@ static bool its_parse_baser_device(struct its_node *its, struct its_baser *baser
 	if (new_order >= MAX_ORDER) {
 		new_order = MAX_ORDER - 1;
 		ids = ilog2(PAGE_ORDER_TO_SIZE(new_order) / (int)esz);
-		pr_warn("ITS@%pa: Device Table too large, reduce ids %u->%u\n",
-			&its->phys_base, its->device_ids, ids);
+		dev_warn(&its->dev, "Device Table too large, reduce ids %u->%u\n",
+			its->device_ids, ids);
 	}
 
 	*order = new_order;
@@ -1013,19 +1015,6 @@ static bool its_parse_baser_device(struct its_node *its, struct its_baser *baser
 	return indirect;
 }
 
-static void its_free_tables(struct its_node *its)
-{
-	int i;
-
-	for (i = 0; i < GITS_BASER_NR_REGS; i++) {
-		if (its->tables[i].base) {
-			free_pages((unsigned long)its->tables[i].base,
-				   its->tables[i].order);
-			its->tables[i].base = NULL;
-		}
-	}
-}
-
 static int its_alloc_tables(struct its_node *its)
 {
 	u64 typer = gic_read_typer(its->base + GITS_TYPER);
@@ -1060,10 +1049,8 @@ static int its_alloc_tables(struct its_node *its)
 			indirect = its_parse_baser_device(its, baser, psz, &order);
 
 		err = its_setup_baser(its, baser, cache, shr, psz, order, indirect);
-		if (err < 0) {
-			its_free_tables(its);
+		if (err < 0)
 			return err;
-		}
 
 		/* Update settings which will be used for next BASERn */
 		psz = baser->psz;
@@ -1347,6 +1334,9 @@ static struct its_device *its_create_device(struct its_node *its, u32 dev_id,
 
 	gic_flush_dcache_to_poc(itt, sz);
 
+	/* prevent its from being released */
+	get_device(&its->dev);
+
 	dev->its = its;
 	dev->itt = itt;
 	dev->nr_ites = nr_ites;
@@ -1656,8 +1646,9 @@ static int its_init_domain(struct its_node *its)
 	return 0;
 }
 
-static void its_free(struct its_node *its)
-{
+static void its_device_release(struct device *dev) {
+	struct its_node *its = container_of(dev, struct its_node, dev);
+
 	kfree(its);
 }
 
@@ -1694,34 +1685,47 @@ static int __init its_init_one(struct its_node *its)
 	u64 baser, tmp;
 	int err;
 
-	its_base = ioremap(its->phys_base, its->phys_size);
+	/* On error always use put_device() to free devices */
+	device_initialize(&its->dev);
+	its->dev.release = its_device_release;
+
+	err = dev_set_name(&its->dev, "its@%pa", &its->phys_base);
+	if (!err)
+		err = device_add(&its->dev);
+
+	if (err) {
+		pr_warn("ITS@%pa: Unable to register device\n", &its->phys_base);
+		return err;
+	}
+
+	its_base = devm_ioremap(&its->dev, its->phys_base, its->phys_size);
 	if (!its_base) {
-		pr_warn("ITS@%pa: Unable to map ITS registers\n", &its->phys_base);
+		dev_warn(&its->dev, "Unable to map ITS registers\n");
 		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", &its->phys_base);
+		dev_warn(&its->dev, "No ITS detected, giving up\n");
 		err = -ENODEV;
-		goto out_unmap;
+		goto fail;
 	}
 
 	err = its_force_quiescent(its_base);
 	if (err) {
-		pr_warn("ITS@%pa: Failed to quiesce, giving up\n", &its->phys_base);
-		goto out_unmap;
+		dev_warn(&its->dev, "Failed to quiesce, giving up\n");
+		goto fail;
 	}
 
 	its->base = its_base;
 	its->ite_size = ((gic_read_typer(its_base + GITS_TYPER) >> 4) & 0xf) + 1;
 
-	its->cmd_base = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
-						get_order(ITS_CMD_QUEUE_SZ));
+	its->cmd_base = (void *)devm_get_free_pages(&its->dev,
+		GFP_KERNEL | __GFP_ZERO, get_order(ITS_CMD_QUEUE_SZ));
 	if (!its->cmd_base) {
 		err = -ENOMEM;
-		goto out_unmap;
+		goto fail;
 	}
 	its->cmd_write = its->cmd_base;
 
@@ -1729,11 +1733,11 @@ static int __init its_init_one(struct its_node *its)
 
 	err = its_alloc_tables(its);
 	if (err)
-		goto out_free_cmd;
+		goto fail;
 
 	err = its_alloc_collections(its);
 	if (err)
-		goto out_free_tables;
+		goto fail;
 
 	baser = (virt_to_phys(its->cmd_base)	|
 		 GITS_CBASER_RaWaWb		|
@@ -1756,7 +1760,7 @@ static int __init its_init_one(struct its_node *its)
 			baser |= GITS_CBASER_nC;
 			gits_write_cbaser(baser, its->base + GITS_CBASER);
 		}
-		pr_info("ITS: using cache flushing for cmd queue\n");
+		dev_info(&its->dev, "using cache flushing for cmd queue\n");
 		its->flags |= ITS_FLAGS_CMDQ_NEEDS_FLUSHING;
 	}
 
@@ -1765,20 +1769,14 @@ static int __init its_init_one(struct its_node *its)
 
 	err = its_init_domain(its);
 	if (err)
-		goto out_free_tables;
+		goto fail;
 
-	pr_info("ITS@%pa: ITS node added\n", &its->phys_base);
+	dev_info(&its->dev, "ITS node added\n");
 
 	return 0;
-
-out_free_tables:
-	its_free_tables(its);
-out_free_cmd:
-	free_pages((unsigned long)its->cmd_base, get_order(ITS_CMD_QUEUE_SZ));
-out_unmap:
-	iounmap(its_base);
 fail:
-	pr_err("ITS@%pa: failed probing (%d)\n", &its->phys_base, err);
+	device_del(&its->dev);
+	dev_err(&its->dev, "failed probing (%d)\n", err);
 	return err;
 }
 
@@ -1912,6 +1910,10 @@ static int __init its_init(void)
 
 	spin_lock(&its_lock);
 
+	/*
+	 * Call this for all entries. We can then use put_device() to
+	 * release the nodes on error.
+	 */
 	list_for_each_entry(its, &its_nodes, entry) {
 		err2 = its_init_one(its);
 		if (!err && err2)
@@ -1923,7 +1925,7 @@ static int __init its_init(void)
 
 	list_for_each_entry_safe(its, tmp, &its_nodes, entry) {
 		list_del(&its->entry);
-		its_free(its);
+		put_device(&its->dev);
 	}
 unlock:
 	spin_unlock(&its_lock);
-- 
2.11.0

[PATCH v2 7/8] irqchip/gic-v3-its: Handle its nodes as kernel devices

[Robert Richter]

Manage its nodes as kernel devices. We can then use the kernel's
device resource management for memory allocation. Freeing memory
becomes much easier now. This also allows us to use CMA for the
allocation of large its tables.

Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 118 ++++++++++++++++++++-------------------
 1 file changed, 60 insertions(+), 58 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index c0eaae35c631..6625b3a505f0 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -19,6 +19,7 @@
 #include <linux/bitmap.h>
 #include <linux/cpu.h>
 #include <linux/delay.h>
+#include <linux/device.h>
 #include <linux/dma-iommu.h>
 #include <linux/interrupt.h>
 #include <linux/irqdomain.h>
@@ -75,6 +76,7 @@ struct its_baser {
  * list of devices writing to it.
  */
 struct its_node {
+	struct device		dev;
 	struct fwnode_handle	*fwnode;
 	raw_spinlock_t		lock;
 	struct list_head	entry;
@@ -400,7 +402,7 @@ static struct its_cmd_block *its_allocate_entry(struct its_node *its)
 	while (its_queue_full(its)) {
 		count--;
 		if (!count) {
-			pr_err_ratelimited("ITS queue not draining\n");
+			dev_err_ratelimited(&its->dev, "ITS queue not draining\n");
 			return NULL;
 		}
 		cpu_relax();
@@ -460,7 +462,7 @@ static void its_wait_for_range_completion(struct its_node *its,
 
 		count--;
 		if (!count) {
-			pr_err_ratelimited("ITS queue timeout\n");
+			dev_err_ratelimited(&its->dev, "ITS queue timeout\n");
 			return;
 		}
 		cpu_relax();
@@ -480,7 +482,7 @@ static void its_send_single_command(struct its_node *its,
 
 	cmd = its_allocate_entry(its);
 	if (!cmd) {		/* We're soooooo screewed... */
-		pr_err_ratelimited("ITS can't allocate, dropping command\n");
+		dev_err_ratelimited(&its->dev, "ITS can't allocate, dropping command\n");
 		raw_spin_unlock_irqrestore(&its->lock, flags);
 		return;
 	}
@@ -490,7 +492,7 @@ static void its_send_single_command(struct its_node *its,
 	if (sync_col) {
 		sync_cmd = its_allocate_entry(its);
 		if (!sync_cmd) {
-			pr_err_ratelimited("ITS can't SYNC, skipping\n");
+			dev_err_ratelimited(&its->dev, "ITS can't SYNC, skipping\n");
 			goto post;
 		}
 		its_encode_cmd(sync_cmd, GITS_CMD_SYNC);
@@ -867,14 +869,15 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
 retry_alloc_baser:
 	alloc_pages = (PAGE_ORDER_TO_SIZE(order) / psz);
 	if (alloc_pages > GITS_BASER_PAGES_MAX) {
-		pr_warn("ITS@%pa: %s too large, reduce ITS pages %u->%u\n",
-			&its->phys_base, its_base_type_string[type],
-			alloc_pages, GITS_BASER_PAGES_MAX);
+		dev_warn(&its->dev, "%s too large, reduce ITS pages %u->%u\n",
+			its_base_type_string[type], alloc_pages,
+			GITS_BASER_PAGES_MAX);
 		alloc_pages = GITS_BASER_PAGES_MAX;
 		order = get_order(GITS_BASER_PAGES_MAX * psz);
 	}
 
-	base = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, order);
+	base = (void *)devm_get_free_pages(&its->dev, GFP_KERNEL | __GFP_ZERO,
+					   order);
 	if (!base)
 		return -ENOMEM;
 
@@ -926,7 +929,7 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
 		 * size and retry. If we reach 4K, then
 		 * something is horribly wrong...
 		 */
-		free_pages((unsigned long)base, order);
+		devm_free_pages(&its->dev, (unsigned long)base);
 		baser->base = NULL;
 
 		switch (psz) {
@@ -940,10 +943,9 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
 	}
 
 	if (val != tmp) {
-		pr_err("ITS@%pa: %s doesn't stick: %llx %llx\n",
-		       &its->phys_base, its_base_type_string[type],
-		       val, tmp);
-		free_pages((unsigned long)base, order);
+		dev_err(&its->dev, "%s doesn't stick: %llx %llx\n",
+		       its_base_type_string[type], val, tmp);
+		devm_free_pages(&its->dev, (unsigned long)base);
 		return -ENXIO;
 	}
 
@@ -952,8 +954,8 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
 	baser->psz = psz;
 	tmp = indirect ? GITS_LVL1_ENTRY_SIZE : esz;
 
-	pr_info("ITS@%pa: allocated %d %s @%lx (%s, esz %d, psz %dK, shr %d)\n",
-		&its->phys_base, (int)(PAGE_ORDER_TO_SIZE(order) / (int)tmp),
+	dev_info(&its->dev, "allocated %d %s @%lx (%s, esz %d, psz %dK, shr %d)\n",
+		(int)(PAGE_ORDER_TO_SIZE(order) / (int)tmp),
 		its_base_type_string[type],
 		(unsigned long)virt_to_phys(base),
 		indirect ? "indirect" : "flat", (int)esz,
@@ -1004,8 +1006,8 @@ static bool its_parse_baser_device(struct its_node *its, struct its_baser *baser
 	if (new_order >= MAX_ORDER) {
 		new_order = MAX_ORDER - 1;
 		ids = ilog2(PAGE_ORDER_TO_SIZE(new_order) / (int)esz);
-		pr_warn("ITS@%pa: Device Table too large, reduce ids %u->%u\n",
-			&its->phys_base, its->device_ids, ids);
+		dev_warn(&its->dev, "Device Table too large, reduce ids %u->%u\n",
+			its->device_ids, ids);
 	}
 
 	*order = new_order;
@@ -1013,19 +1015,6 @@ static bool its_parse_baser_device(struct its_node *its, struct its_baser *baser
 	return indirect;
 }
 
-static void its_free_tables(struct its_node *its)
-{
-	int i;
-
-	for (i = 0; i < GITS_BASER_NR_REGS; i++) {
-		if (its->tables[i].base) {
-			free_pages((unsigned long)its->tables[i].base,
-				   its->tables[i].order);
-			its->tables[i].base = NULL;
-		}
-	}
-}
-
 static int its_alloc_tables(struct its_node *its)
 {
 	u64 typer = gic_read_typer(its->base + GITS_TYPER);
@@ -1060,10 +1049,8 @@ static int its_alloc_tables(struct its_node *its)
 			indirect = its_parse_baser_device(its, baser, psz, &order);
 
 		err = its_setup_baser(its, baser, cache, shr, psz, order, indirect);
-		if (err < 0) {
-			its_free_tables(its);
+		if (err < 0)
 			return err;
-		}
 
 		/* Update settings which will be used for next BASERn */
 		psz = baser->psz;
@@ -1347,6 +1334,9 @@ static struct its_device *its_create_device(struct its_node *its, u32 dev_id,
 
 	gic_flush_dcache_to_poc(itt, sz);
 
+	/* prevent its from being released */
+	get_device(&its->dev);
+
 	dev->its = its;
 	dev->itt = itt;
 	dev->nr_ites = nr_ites;
@@ -1656,8 +1646,9 @@ static int its_init_domain(struct its_node *its)
 	return 0;
 }
 
-static void its_free(struct its_node *its)
-{
+static void its_device_release(struct device *dev) {
+	struct its_node *its = container_of(dev, struct its_node, dev);
+
 	kfree(its);
 }
 
@@ -1694,34 +1685,47 @@ static int __init its_init_one(struct its_node *its)
 	u64 baser, tmp;
 	int err;
 
-	its_base = ioremap(its->phys_base, its->phys_size);
+	/* On error always use put_device() to free devices */
+	device_initialize(&its->dev);
+	its->dev.release = its_device_release;
+
+	err = dev_set_name(&its->dev, "its@%pa", &its->phys_base);
+	if (!err)
+		err = device_add(&its->dev);
+
+	if (err) {
+		pr_warn("ITS@%pa: Unable to register device\n", &its->phys_base);
+		return err;
+	}
+
+	its_base = devm_ioremap(&its->dev, its->phys_base, its->phys_size);
 	if (!its_base) {
-		pr_warn("ITS@%pa: Unable to map ITS registers\n", &its->phys_base);
+		dev_warn(&its->dev, "Unable to map ITS registers\n");
 		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", &its->phys_base);
+		dev_warn(&its->dev, "No ITS detected, giving up\n");
 		err = -ENODEV;
-		goto out_unmap;
+		goto fail;
 	}
 
 	err = its_force_quiescent(its_base);
 	if (err) {
-		pr_warn("ITS@%pa: Failed to quiesce, giving up\n", &its->phys_base);
-		goto out_unmap;
+		dev_warn(&its->dev, "Failed to quiesce, giving up\n");
+		goto fail;
 	}
 
 	its->base = its_base;
 	its->ite_size = ((gic_read_typer(its_base + GITS_TYPER) >> 4) & 0xf) + 1;
 
-	its->cmd_base = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
-						get_order(ITS_CMD_QUEUE_SZ));
+	its->cmd_base = (void *)devm_get_free_pages(&its->dev,
+		GFP_KERNEL | __GFP_ZERO, get_order(ITS_CMD_QUEUE_SZ));
 	if (!its->cmd_base) {
 		err = -ENOMEM;
-		goto out_unmap;
+		goto fail;
 	}
 	its->cmd_write = its->cmd_base;
 
@@ -1729,11 +1733,11 @@ static int __init its_init_one(struct its_node *its)
 
 	err = its_alloc_tables(its);
 	if (err)
-		goto out_free_cmd;
+		goto fail;
 
 	err = its_alloc_collections(its);
 	if (err)
-		goto out_free_tables;
+		goto fail;
 
 	baser = (virt_to_phys(its->cmd_base)	|
 		 GITS_CBASER_RaWaWb		|
@@ -1756,7 +1760,7 @@ static int __init its_init_one(struct its_node *its)
 			baser |= GITS_CBASER_nC;
 			gits_write_cbaser(baser, its->base + GITS_CBASER);
 		}
-		pr_info("ITS: using cache flushing for cmd queue\n");
+		dev_info(&its->dev, "using cache flushing for cmd queue\n");
 		its->flags |= ITS_FLAGS_CMDQ_NEEDS_FLUSHING;
 	}
 
@@ -1765,20 +1769,14 @@ static int __init its_init_one(struct its_node *its)
 
 	err = its_init_domain(its);
 	if (err)
-		goto out_free_tables;
+		goto fail;
 
-	pr_info("ITS@%pa: ITS node added\n", &its->phys_base);
+	dev_info(&its->dev, "ITS node added\n");
 
 	return 0;
-
-out_free_tables:
-	its_free_tables(its);
-out_free_cmd:
-	free_pages((unsigned long)its->cmd_base, get_order(ITS_CMD_QUEUE_SZ));
-out_unmap:
-	iounmap(its_base);
 fail:
-	pr_err("ITS@%pa: failed probing (%d)\n", &its->phys_base, err);
+	device_del(&its->dev);
+	dev_err(&its->dev, "failed probing (%d)\n", err);
 	return err;
 }
 
@@ -1912,6 +1910,10 @@ static int __init its_init(void)
 
 	spin_lock(&its_lock);
 
+	/*
+	 * Call this for all entries. We can then use put_device() to
+	 * release the nodes on error.
+	 */
 	list_for_each_entry(its, &its_nodes, entry) {
 		err2 = its_init_one(its);
 		if (!err && err2)
@@ -1923,7 +1925,7 @@ static int __init its_init(void)
 
 	list_for_each_entry_safe(its, tmp, &its_nodes, entry) {
 		list_del(&its->entry);
-		its_free(its);
+		put_device(&its->dev);
 	}
 unlock:
 	spin_unlock(&its_lock);
-- 
2.11.0

[PATCH v2 8/8] irqchip, gicv3-its, cma: Use CMA for allocation of large device tables

[Robert Richter]

The gicv3-its device table may have a size of up to 16MB. With 4k
pagesize the maximum size of memory allocation is 4MB. Use CMA for
allocation of large tables.

We use the device managed version of dma_alloc_coherent(). Thus, we
don't need to release it manually on device removal.

Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 69 +++++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 29 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 6625b3a505f0..6d293a0165b0 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -21,6 +21,7 @@
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/dma-iommu.h>
+#include <linux/dma-mapping.h>
 #include <linux/interrupt.h>
 #include <linux/irqdomain.h>
 #include <linux/acpi_iort.h>
@@ -864,6 +865,7 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
 	u64 type = GITS_BASER_TYPE(val);
 	u32 alloc_pages;
 	void *base;
+	dma_addr_t dma_handle;
 	u64 tmp;
 
 retry_alloc_baser:
@@ -876,13 +878,26 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
 		order = get_order(GITS_BASER_PAGES_MAX * psz);
 	}
 
-	base = (void *)devm_get_free_pages(&its->dev, GFP_KERNEL | __GFP_ZERO,
-					   order);
-	if (!base)
+	base = dmam_alloc_coherent(&its->dev,
+				PAGE_ORDER_TO_SIZE(order),
+				&dma_handle,
+				GFP_KERNEL | __GFP_ZERO);
+
+	if (!base && order >= MAX_ORDER) {
+		order = MAX_ORDER - 1;
+		dev_warn(&its->dev, "Device Table too large, reduce ids %u->%u, no CMA memory available\n",
+			its->device_ids,
+			ilog2(PAGE_ORDER_TO_SIZE(order) / (int)esz));
+		goto retry_alloc_baser;
+	}
+
+	if (!base) {
+		dev_err(&its->dev, "Failed to allocate device table\n");
 		return -ENOMEM;
+	}
 
 retry_baser:
-	val = (virt_to_phys(base)				 |
+	val = (dma_handle					 |
 		(type << GITS_BASER_TYPE_SHIFT)			 |
 		((esz - 1) << GITS_BASER_ENTRY_SIZE_SHIFT)	 |
 		((alloc_pages - 1) << GITS_BASER_PAGES_SHIFT)	 |
@@ -923,29 +938,28 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
 		goto retry_baser;
 	}
 
-	if ((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK) {
-		/*
-		 * Page size didn't stick. Let's try a smaller
-		 * size and retry. If we reach 4K, then
-		 * something is horribly wrong...
-		 */
-		devm_free_pages(&its->dev, (unsigned long)base);
-		baser->base = NULL;
-
-		switch (psz) {
-		case SZ_16K:
-			psz = SZ_4K;
-			goto retry_alloc_baser;
-		case SZ_64K:
-			psz = SZ_16K;
-			goto retry_alloc_baser;
+	if (val != tmp) {
+		dmam_free_coherent(&its->dev, PAGE_ORDER_TO_SIZE(order),
+				base, dma_handle);
+
+		if ((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK) {
+			/*
+			 * Page size didn't stick. Let's try a smaller
+			 * size and retry. If we reach 4K, then
+			 * something is horribly wrong...
+			 */
+			switch (psz) {
+			case SZ_16K:
+				psz = SZ_4K;
+				goto retry_alloc_baser;
+			case SZ_64K:
+				psz = SZ_16K;
+				goto retry_alloc_baser;
+			}
 		}
-	}
 
-	if (val != tmp) {
 		dev_err(&its->dev, "%s doesn't stick: %llx %llx\n",
 		       its_base_type_string[type], val, tmp);
-		devm_free_pages(&its->dev, (unsigned long)base);
 		return -ENXIO;
 	}
 
@@ -1003,12 +1017,6 @@ static bool its_parse_baser_device(struct its_node *its, struct its_baser *baser
 	 * feature is not supported by hardware.
 	 */
 	new_order = max_t(u32, get_order(esz << ids), new_order);
-	if (new_order >= MAX_ORDER) {
-		new_order = MAX_ORDER - 1;
-		ids = ilog2(PAGE_ORDER_TO_SIZE(new_order) / (int)esz);
-		dev_warn(&its->dev, "Device Table too large, reduce ids %u->%u\n",
-			its->device_ids, ids);
-	}
 
 	*order = new_order;
 
@@ -1698,6 +1706,9 @@ static int __init its_init_one(struct its_node *its)
 		return err;
 	}
 
+	/* Setup dma_ops for dmam_alloc_coherent() */
+	arch_setup_dma_ops(&its->dev, 0, 0, NULL, true);
+
 	its_base = devm_ioremap(&its->dev, its->phys_base, its->phys_size);
 	if (!its_base) {
 		dev_warn(&its->dev, "Unable to map ITS registers\n");
-- 
2.11.0

[PATCH v2 8/8] irqchip, gicv3-its, cma: Use CMA for allocation of large device tables

[Robert Richter]

The gicv3-its device table may have a size of up to 16MB. With 4k
pagesize the maximum size of memory allocation is 4MB. Use CMA for
allocation of large tables.

We use the device managed version of dma_alloc_coherent(). Thus, we
don't need to release it manually on device removal.

Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 69 +++++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 29 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 6625b3a505f0..6d293a0165b0 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -21,6 +21,7 @@
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/dma-iommu.h>
+#include <linux/dma-mapping.h>
 #include <linux/interrupt.h>
 #include <linux/irqdomain.h>
 #include <linux/acpi_iort.h>
@@ -864,6 +865,7 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
 	u64 type = GITS_BASER_TYPE(val);
 	u32 alloc_pages;
 	void *base;
+	dma_addr_t dma_handle;
 	u64 tmp;
 
 retry_alloc_baser:
@@ -876,13 +878,26 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
 		order = get_order(GITS_BASER_PAGES_MAX * psz);
 	}
 
-	base = (void *)devm_get_free_pages(&its->dev, GFP_KERNEL | __GFP_ZERO,
-					   order);
-	if (!base)
+	base = dmam_alloc_coherent(&its->dev,
+				PAGE_ORDER_TO_SIZE(order),
+				&dma_handle,
+				GFP_KERNEL | __GFP_ZERO);
+
+	if (!base && order >= MAX_ORDER) {
+		order = MAX_ORDER - 1;
+		dev_warn(&its->dev, "Device Table too large, reduce ids %u->%u, no CMA memory available\n",
+			its->device_ids,
+			ilog2(PAGE_ORDER_TO_SIZE(order) / (int)esz));
+		goto retry_alloc_baser;
+	}
+
+	if (!base) {
+		dev_err(&its->dev, "Failed to allocate device table\n");
 		return -ENOMEM;
+	}
 
 retry_baser:
-	val = (virt_to_phys(base)				 |
+	val = (dma_handle					 |
 		(type << GITS_BASER_TYPE_SHIFT)			 |
 		((esz - 1) << GITS_BASER_ENTRY_SIZE_SHIFT)	 |
 		((alloc_pages - 1) << GITS_BASER_PAGES_SHIFT)	 |
@@ -923,29 +938,28 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
 		goto retry_baser;
 	}
 
-	if ((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK) {
-		/*
-		 * Page size didn't stick. Let's try a smaller
-		 * size and retry. If we reach 4K, then
-		 * something is horribly wrong...
-		 */
-		devm_free_pages(&its->dev, (unsigned long)base);
-		baser->base = NULL;
-
-		switch (psz) {
-		case SZ_16K:
-			psz = SZ_4K;
-			goto retry_alloc_baser;
-		case SZ_64K:
-			psz = SZ_16K;
-			goto retry_alloc_baser;
+	if (val != tmp) {
+		dmam_free_coherent(&its->dev, PAGE_ORDER_TO_SIZE(order),
+				base, dma_handle);
+
+		if ((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK) {
+			/*
+			 * Page size didn't stick. Let's try a smaller
+			 * size and retry. If we reach 4K, then
+			 * something is horribly wrong...
+			 */
+			switch (psz) {
+			case SZ_16K:
+				psz = SZ_4K;
+				goto retry_alloc_baser;
+			case SZ_64K:
+				psz = SZ_16K;
+				goto retry_alloc_baser;
+			}
 		}
-	}
 
-	if (val != tmp) {
 		dev_err(&its->dev, "%s doesn't stick: %llx %llx\n",
 		       its_base_type_string[type], val, tmp);
-		devm_free_pages(&its->dev, (unsigned long)base);
 		return -ENXIO;
 	}
 
@@ -1003,12 +1017,6 @@ static bool its_parse_baser_device(struct its_node *its, struct its_baser *baser
 	 * feature is not supported by hardware.
 	 */
 	new_order = max_t(u32, get_order(esz << ids), new_order);
-	if (new_order >= MAX_ORDER) {
-		new_order = MAX_ORDER - 1;
-		ids = ilog2(PAGE_ORDER_TO_SIZE(new_order) / (int)esz);
-		dev_warn(&its->dev, "Device Table too large, reduce ids %u->%u\n",
-			its->device_ids, ids);
-	}
 
 	*order = new_order;
 
@@ -1698,6 +1706,9 @@ static int __init its_init_one(struct its_node *its)
 		return err;
 	}
 
+	/* Setup dma_ops for dmam_alloc_coherent() */
+	arch_setup_dma_ops(&its->dev, 0, 0, NULL, true);
+
 	its_base = devm_ioremap(&its->dev, its->phys_base, its->phys_size);
 	if (!its_base) {
 		dev_warn(&its->dev, "Unable to map ITS registers\n");
-- 
2.11.0

Re: [PATCH v2 8/8] irqchip, gicv3-its, cma: Use CMA for allocation of large device tables

[Shanker Donthineni]

Hi Robert,

I don't see anywhere in this patch, code calls explicitly CMA API to allocate memory for device table.  The CMA feature is an optional in kernel, and will be handled transparently inside the the DMA layer. It would be nicer to not mention CMA word in the commit subject.


On 03/06/2017 06:57 AM, Robert Richter wrote:
> The gicv3-its device table may have a size of up to 16MB. With 4k
> pagesize the maximum size of memory allocation is 4MB. Use CMA for
> allocation of large tables.
Just say use devm_alloc_coherent() to allocate memory.

> We use the device managed version of dma_alloc_coherent(). Thus, we
> don't need to release it manually on device removal.
>
> Signed-off-by: Robert Richter <rrichter@cavium.com>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 69 +++++++++++++++++++++++-----------------
>  1 file changed, 40 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 6625b3a505f0..6d293a0165b0 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -21,6 +21,7 @@
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/dma-iommu.h>
> +#include <linux/dma-mapping.h>
>  #include <linux/interrupt.h>
>  #include <linux/irqdomain.h>
>  #include <linux/acpi_iort.h>
> @@ -864,6 +865,7 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
>  	u64 type = GITS_BASER_TYPE(val);
>  	u32 alloc_pages;
>  	void *base;
> +	dma_addr_t dma_handle;
>  	u64 tmp;
>  
>  retry_alloc_baser:
> @@ -876,13 +878,26 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
>  		order = get_order(GITS_BASER_PAGES_MAX * psz);
>  	}
>  
> -	base = (void *)devm_get_free_pages(&its->dev, GFP_KERNEL | __GFP_ZERO,
> -					   order);
> -	if (!base)
> +	base = dmam_alloc_coherent(&its->dev,
> +				PAGE_ORDER_TO_SIZE(order),
> +				&dma_handle,
> +				GFP_KERNEL | __GFP_ZERO);
Not just for 1st level device table, you have do a similar code change when allocating memory for 2nd level device table.
> +
> +	if (!base && order >= MAX_ORDER) {
> +		order = MAX_ORDER - 1;
> +		dev_warn(&its->dev, "Device Table too large, reduce ids %u->%u, no CMA memory available\n",
> +			its->device_ids,
> +			ilog2(PAGE_ORDER_TO_SIZE(order) / (int)esz));
> +		goto retry_alloc_baser;
> +	}
> +
> +	if (!base) {
> +		dev_err(&its->dev, "Failed to allocate device table\n");
>  		return -ENOMEM;
> +	}
>  
>  retry_baser:
> -	val = (virt_to_phys(base)				 |
> +	val = (dma_handle					 |
>  		(type << GITS_BASER_TYPE_SHIFT)			 |
>  		((esz - 1) << GITS_BASER_ENTRY_SIZE_SHIFT)	 |
>  		((alloc_pages - 1) << GITS_BASER_PAGES_SHIFT)	 |
> @@ -923,29 +938,28 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
>  		goto retry_baser;
>  	}
>  
> -	if ((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK) {
> -		/*
> -		 * Page size didn't stick. Let's try a smaller
> -		 * size and retry. If we reach 4K, then
> -		 * something is horribly wrong...
> -		 */
> -		devm_free_pages(&its->dev, (unsigned long)base);
> -		baser->base = NULL;
> -
> -		switch (psz) {
> -		case SZ_16K:
> -			psz = SZ_4K;
> -			goto retry_alloc_baser;
> -		case SZ_64K:
> -			psz = SZ_16K;
> -			goto retry_alloc_baser;
> +	if (val != tmp) {
> +		dmam_free_coherent(&its->dev, PAGE_ORDER_TO_SIZE(order),
> +				base, dma_handle);
> +
> +		if ((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK) {
> +			/*
> +			 * Page size didn't stick. Let's try a smaller
> +			 * size and retry. If we reach 4K, then
> +			 * something is horribly wrong...
> +			 */
> +			switch (psz) {
> +			case SZ_16K:
> +				psz = SZ_4K;
> +				goto retry_alloc_baser;
> +			case SZ_64K:
> +				psz = SZ_16K;
> +				goto retry_alloc_baser;
> +			}
>  		}
> -	}
>  
> -	if (val != tmp) {
>  		dev_err(&its->dev, "%s doesn't stick: %llx %llx\n",
>  		       its_base_type_string[type], val, tmp);
> -		devm_free_pages(&its->dev, (unsigned long)base);
>  		return -ENXIO;
>  	}
>  
> @@ -1003,12 +1017,6 @@ static bool its_parse_baser_device(struct its_node *its, struct its_baser *baser
>  	 * feature is not supported by hardware.
>  	 */
>  	new_order = max_t(u32, get_order(esz << ids), new_order);
> -	if (new_order >= MAX_ORDER) {
> -		new_order = MAX_ORDER - 1;
> -		ids = ilog2(PAGE_ORDER_TO_SIZE(new_order) / (int)esz);
> -		dev_warn(&its->dev, "Device Table too large, reduce ids %u->%u\n",
> -			its->device_ids, ids);
> -	}
>  
>  	*order = new_order;
>  
> @@ -1698,6 +1706,9 @@ static int __init its_init_one(struct its_node *its)
>  		return err;
>  	}
>  
> +	/* Setup dma_ops for dmam_alloc_coherent() */
> +	arch_setup_dma_ops(&its->dev, 0, 0, NULL, true);
> +
Why you are hard-coding DMA coherent property to true here ? It breaks the MSI(x) functionally on systems where ITS hardware doesn't support coherency.
>  	its_base = devm_ioremap(&its->dev, its->phys_base, its->phys_size);
>  	if (!its_base) {
>  		dev_warn(&its->dev, "Unable to map ITS registers\n");

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

[PATCH v2 8/8] irqchip, gicv3-its, cma: Use CMA for allocation of large device tables

[Shanker Donthineni]

Hi Robert,

I don't see anywhere in this patch, code calls explicitly CMA API to allocate memory for device table.  The CMA feature is an optional in kernel, and will be handled transparently inside the the DMA layer. It would be nicer to not mention CMA word in the commit subject.


On 03/06/2017 06:57 AM, Robert Richter wrote:
> The gicv3-its device table may have a size of up to 16MB. With 4k
> pagesize the maximum size of memory allocation is 4MB. Use CMA for
> allocation of large tables.
Just say use devm_alloc_coherent() to allocate memory.

> We use the device managed version of dma_alloc_coherent(). Thus, we
> don't need to release it manually on device removal.
>
> Signed-off-by: Robert Richter <rrichter@cavium.com>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 69 +++++++++++++++++++++++-----------------
>  1 file changed, 40 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 6625b3a505f0..6d293a0165b0 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -21,6 +21,7 @@
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/dma-iommu.h>
> +#include <linux/dma-mapping.h>
>  #include <linux/interrupt.h>
>  #include <linux/irqdomain.h>
>  #include <linux/acpi_iort.h>
> @@ -864,6 +865,7 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
>  	u64 type = GITS_BASER_TYPE(val);
>  	u32 alloc_pages;
>  	void *base;
> +	dma_addr_t dma_handle;
>  	u64 tmp;
>  
>  retry_alloc_baser:
> @@ -876,13 +878,26 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
>  		order = get_order(GITS_BASER_PAGES_MAX * psz);
>  	}
>  
> -	base = (void *)devm_get_free_pages(&its->dev, GFP_KERNEL | __GFP_ZERO,
> -					   order);
> -	if (!base)
> +	base = dmam_alloc_coherent(&its->dev,
> +				PAGE_ORDER_TO_SIZE(order),
> +				&dma_handle,
> +				GFP_KERNEL | __GFP_ZERO);
Not just for 1st level device table, you have do a similar code change when allocating memory for 2nd level device table.
> +
> +	if (!base && order >= MAX_ORDER) {
> +		order = MAX_ORDER - 1;
> +		dev_warn(&its->dev, "Device Table too large, reduce ids %u->%u, no CMA memory available\n",
> +			its->device_ids,
> +			ilog2(PAGE_ORDER_TO_SIZE(order) / (int)esz));
> +		goto retry_alloc_baser;
> +	}
> +
> +	if (!base) {
> +		dev_err(&its->dev, "Failed to allocate device table\n");
>  		return -ENOMEM;
> +	}
>  
>  retry_baser:
> -	val = (virt_to_phys(base)				 |
> +	val = (dma_handle					 |
>  		(type << GITS_BASER_TYPE_SHIFT)			 |
>  		((esz - 1) << GITS_BASER_ENTRY_SIZE_SHIFT)	 |
>  		((alloc_pages - 1) << GITS_BASER_PAGES_SHIFT)	 |
> @@ -923,29 +938,28 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
>  		goto retry_baser;
>  	}
>  
> -	if ((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK) {
> -		/*
> -		 * Page size didn't stick. Let's try a smaller
> -		 * size and retry. If we reach 4K, then
> -		 * something is horribly wrong...
> -		 */
> -		devm_free_pages(&its->dev, (unsigned long)base);
> -		baser->base = NULL;
> -
> -		switch (psz) {
> -		case SZ_16K:
> -			psz = SZ_4K;
> -			goto retry_alloc_baser;
> -		case SZ_64K:
> -			psz = SZ_16K;
> -			goto retry_alloc_baser;
> +	if (val != tmp) {
> +		dmam_free_coherent(&its->dev, PAGE_ORDER_TO_SIZE(order),
> +				base, dma_handle);
> +
> +		if ((val ^ tmp) & GITS_BASER_PAGE_SIZE_MASK) {
> +			/*
> +			 * Page size didn't stick. Let's try a smaller
> +			 * size and retry. If we reach 4K, then
> +			 * something is horribly wrong...
> +			 */
> +			switch (psz) {
> +			case SZ_16K:
> +				psz = SZ_4K;
> +				goto retry_alloc_baser;
> +			case SZ_64K:
> +				psz = SZ_16K;
> +				goto retry_alloc_baser;
> +			}
>  		}
> -	}
>  
> -	if (val != tmp) {
>  		dev_err(&its->dev, "%s doesn't stick: %llx %llx\n",
>  		       its_base_type_string[type], val, tmp);
> -		devm_free_pages(&its->dev, (unsigned long)base);
>  		return -ENXIO;
>  	}
>  
> @@ -1003,12 +1017,6 @@ static bool its_parse_baser_device(struct its_node *its, struct its_baser *baser
>  	 * feature is not supported by hardware.
>  	 */
>  	new_order = max_t(u32, get_order(esz << ids), new_order);
> -	if (new_order >= MAX_ORDER) {
> -		new_order = MAX_ORDER - 1;
> -		ids = ilog2(PAGE_ORDER_TO_SIZE(new_order) / (int)esz);
> -		dev_warn(&its->dev, "Device Table too large, reduce ids %u->%u\n",
> -			its->device_ids, ids);
> -	}
>  
>  	*order = new_order;
>  
> @@ -1698,6 +1706,9 @@ static int __init its_init_one(struct its_node *its)
>  		return err;
>  	}
>  
> +	/* Setup dma_ops for dmam_alloc_coherent() */
> +	arch_setup_dma_ops(&its->dev, 0, 0, NULL, true);
> +
Why you are hard-coding DMA coherent property to true here ? It breaks the MSI(x) functionally on systems where ITS hardware doesn't support coherency.
>  	its_base = devm_ioremap(&its->dev, its->phys_base, its->phys_size);
>  	if (!its_base) {
>  		dev_warn(&its->dev, "Unable to map ITS registers\n");

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

Re: [PATCH v2 8/8] irqchip, gicv3-its, cma: Use CMA for allocation of large device tables

[Robert Richter]

On 14.03.17 12:40:45, Shanker Donthineni wrote:

> I don't see anywhere in this patch, code calls explicitly CMA API to
> allocate memory for device table.  The CMA feature is an optional in
> kernel, and will be handled transparently inside the the DMA
> layer. It would be nicer to not mention CMA word in the commit
> subject.

Still CMA is *essential* and used for large tables. IMO this needs to
be emphasized. That's the reason for using devm_alloc_coherent().

> On 03/06/2017 06:57 AM, Robert Richter wrote:
> > The gicv3-its device table may have a size of up to 16MB. With 4k
> > pagesize the maximum size of memory allocation is 4MB. Use CMA for
> > allocation of large tables.
> Just say use devm_alloc_coherent() to allocate memory.
> 
> > We use the device managed version of dma_alloc_coherent(). Thus, we
> > don't need to release it manually on device removal.
> >
> > Signed-off-by: Robert Richter <rrichter@cavium.com>
> > ---
> >  drivers/irqchip/irq-gic-v3-its.c | 69 +++++++++++++++++++++++-----------------
> >  1 file changed, 40 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> > index 6625b3a505f0..6d293a0165b0 100644
> > --- a/drivers/irqchip/irq-gic-v3-its.c
> > +++ b/drivers/irqchip/irq-gic-v3-its.c

> > @@ -876,13 +878,26 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
> >  		order = get_order(GITS_BASER_PAGES_MAX * psz);
> >  	}
> >  
> > -	base = (void *)devm_get_free_pages(&its->dev, GFP_KERNEL | __GFP_ZERO,
> > -					   order);
> > -	if (!base)
> > +	base = dmam_alloc_coherent(&its->dev,
> > +				PAGE_ORDER_TO_SIZE(order),
> > +				&dma_handle,
> > +				GFP_KERNEL | __GFP_ZERO);

> Not just for 1st level device table, you have do a similar code
> change when allocating memory for 2nd level device table.

The 2nd level tables are much smaller, so no need for
dmam_alloc_coherent() there.

Though, we could use device managed devm_get_free_pages() there too.

> > +
> > +	if (!base && order >= MAX_ORDER) {
> > +		order = MAX_ORDER - 1;
> > +		dev_warn(&its->dev, "Device Table too large, reduce ids %u->%u, no CMA memory available\n",
> > +			its->device_ids,
> > +			ilog2(PAGE_ORDER_TO_SIZE(order) / (int)esz));
> > +		goto retry_alloc_baser;
> > +	}
> > +
> > +	if (!base) {
> > +		dev_err(&its->dev, "Failed to allocate device table\n");
> >  		return -ENOMEM;
> > +	}

> > @@ -1698,6 +1706,9 @@ static int __init its_init_one(struct its_node *its)
> >  		return err;
> >  	}
> >  
> > +	/* Setup dma_ops for dmam_alloc_coherent() */
> > +	arch_setup_dma_ops(&its->dev, 0, 0, NULL, true);
> > +

> Why you are hard-coding DMA coherent property to true here ? It
> breaks the MSI(x) functionally on systems where ITS hardware doesn't
> support coherency.

Aren't current ITS tables coherent only?

Thanks,

-Robert

[PATCH v2 8/8] irqchip, gicv3-its, cma: Use CMA for allocation of large device tables

[Robert Richter]

On 14.03.17 12:40:45, Shanker Donthineni wrote:

> I don't see anywhere in this patch, code calls explicitly CMA API to
> allocate memory for device table.  The CMA feature is an optional in
> kernel, and will be handled transparently inside the the DMA
> layer. It would be nicer to not mention CMA word in the commit
> subject.

Still CMA is *essential* and used for large tables. IMO this needs to
be emphasized. That's the reason for using devm_alloc_coherent().

> On 03/06/2017 06:57 AM, Robert Richter wrote:
> > The gicv3-its device table may have a size of up to 16MB. With 4k
> > pagesize the maximum size of memory allocation is 4MB. Use CMA for
> > allocation of large tables.
> Just say use devm_alloc_coherent() to allocate memory.
> 
> > We use the device managed version of dma_alloc_coherent(). Thus, we
> > don't need to release it manually on device removal.
> >
> > Signed-off-by: Robert Richter <rrichter@cavium.com>
> > ---
> >  drivers/irqchip/irq-gic-v3-its.c | 69 +++++++++++++++++++++++-----------------
> >  1 file changed, 40 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> > index 6625b3a505f0..6d293a0165b0 100644
> > --- a/drivers/irqchip/irq-gic-v3-its.c
> > +++ b/drivers/irqchip/irq-gic-v3-its.c

> > @@ -876,13 +878,26 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
> >  		order = get_order(GITS_BASER_PAGES_MAX * psz);
> >  	}
> >  
> > -	base = (void *)devm_get_free_pages(&its->dev, GFP_KERNEL | __GFP_ZERO,
> > -					   order);
> > -	if (!base)
> > +	base = dmam_alloc_coherent(&its->dev,
> > +				PAGE_ORDER_TO_SIZE(order),
> > +				&dma_handle,
> > +				GFP_KERNEL | __GFP_ZERO);

> Not just for 1st level device table, you have do a similar code
> change when allocating memory for 2nd level device table.

The 2nd level tables are much smaller, so no need for
dmam_alloc_coherent() there.

Though, we could use device managed devm_get_free_pages() there too.

> > +
> > +	if (!base && order >= MAX_ORDER) {
> > +		order = MAX_ORDER - 1;
> > +		dev_warn(&its->dev, "Device Table too large, reduce ids %u->%u, no CMA memory available\n",
> > +			its->device_ids,
> > +			ilog2(PAGE_ORDER_TO_SIZE(order) / (int)esz));
> > +		goto retry_alloc_baser;
> > +	}
> > +
> > +	if (!base) {
> > +		dev_err(&its->dev, "Failed to allocate device table\n");
> >  		return -ENOMEM;
> > +	}

> > @@ -1698,6 +1706,9 @@ static int __init its_init_one(struct its_node *its)
> >  		return err;
> >  	}
> >  
> > +	/* Setup dma_ops for dmam_alloc_coherent() */
> > +	arch_setup_dma_ops(&its->dev, 0, 0, NULL, true);
> > +

> Why you are hard-coding DMA coherent property to true here ? It
> breaks the MSI(x) functionally on systems where ITS hardware doesn't
> support coherency.

Aren't current ITS tables coherent only?

Thanks,

-Robert

Re: [PATCH v2 8/8] irqchip, gicv3-its, cma: Use CMA for allocation of large device tables

[Marc Zyngier]

On 15/03/17 18:37, Robert Richter wrote:
> On 14.03.17 12:40:45, Shanker Donthineni wrote:
> 
>> I don't see anywhere in this patch, code calls explicitly CMA API to
>> allocate memory for device table.  The CMA feature is an optional in
>> kernel, and will be handled transparently inside the the DMA
>> layer. It would be nicer to not mention CMA word in the commit
>> subject.
> 
> Still CMA is *essential* and used for large tables. IMO this needs to
> be emphasized. That's the reason for using devm_alloc_coherent().
> 
>> On 03/06/2017 06:57 AM, Robert Richter wrote:
>>> The gicv3-its device table may have a size of up to 16MB. With 4k
>>> pagesize the maximum size of memory allocation is 4MB. Use CMA for
>>> allocation of large tables.
>> Just say use devm_alloc_coherent() to allocate memory.
>>
>>> We use the device managed version of dma_alloc_coherent(). Thus, we
>>> don't need to release it manually on device removal.
>>>
>>> Signed-off-by: Robert Richter <rrichter@cavium.com>
>>> ---
>>>  drivers/irqchip/irq-gic-v3-its.c | 69 +++++++++++++++++++++++-----------------
>>>  1 file changed, 40 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
>>> index 6625b3a505f0..6d293a0165b0 100644
>>> --- a/drivers/irqchip/irq-gic-v3-its.c
>>> +++ b/drivers/irqchip/irq-gic-v3-its.c
> 
>>> @@ -876,13 +878,26 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
>>>  		order = get_order(GITS_BASER_PAGES_MAX * psz);
>>>  	}
>>>  
>>> -	base = (void *)devm_get_free_pages(&its->dev, GFP_KERNEL | __GFP_ZERO,
>>> -					   order);
>>> -	if (!base)
>>> +	base = dmam_alloc_coherent(&its->dev,
>>> +				PAGE_ORDER_TO_SIZE(order),
>>> +				&dma_handle,
>>> +				GFP_KERNEL | __GFP_ZERO);
> 
>> Not just for 1st level device table, you have do a similar code
>> change when allocating memory for 2nd level device table.
> 
> The 2nd level tables are much smaller, so no need for
> dmam_alloc_coherent() there.
> 
> Though, we could use device managed devm_get_free_pages() there too.
> 
>>> +
>>> +	if (!base && order >= MAX_ORDER) {
>>> +		order = MAX_ORDER - 1;
>>> +		dev_warn(&its->dev, "Device Table too large, reduce ids %u->%u, no CMA memory available\n",
>>> +			its->device_ids,
>>> +			ilog2(PAGE_ORDER_TO_SIZE(order) / (int)esz));
>>> +		goto retry_alloc_baser;
>>> +	}
>>> +
>>> +	if (!base) {
>>> +		dev_err(&its->dev, "Failed to allocate device table\n");
>>>  		return -ENOMEM;
>>> +	}
> 
>>> @@ -1698,6 +1706,9 @@ static int __init its_init_one(struct its_node *its)
>>>  		return err;
>>>  	}
>>>  
>>> +	/* Setup dma_ops for dmam_alloc_coherent() */
>>> +	arch_setup_dma_ops(&its->dev, 0, 0, NULL, true);
>>> +
> 
>> Why you are hard-coding DMA coherent property to true here ? It
>> breaks the MSI(x) functionally on systems where ITS hardware doesn't
>> support coherency.
> 
> Aren't current ITS tables coherent only?

No, there is no such guarantee. Actually, there is strictly no need for
coherency, as the ITS tables are only written by the ITS itself, for its
own purpose.

The only things that may benefit from coherency are the property table
and the command queue.

Thanks,

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

[PATCH v2 8/8] irqchip, gicv3-its, cma: Use CMA for allocation of large device tables

[Marc Zyngier]

On 15/03/17 18:37, Robert Richter wrote:
> On 14.03.17 12:40:45, Shanker Donthineni wrote:
> 
>> I don't see anywhere in this patch, code calls explicitly CMA API to
>> allocate memory for device table.  The CMA feature is an optional in
>> kernel, and will be handled transparently inside the the DMA
>> layer. It would be nicer to not mention CMA word in the commit
>> subject.
> 
> Still CMA is *essential* and used for large tables. IMO this needs to
> be emphasized. That's the reason for using devm_alloc_coherent().
> 
>> On 03/06/2017 06:57 AM, Robert Richter wrote:
>>> The gicv3-its device table may have a size of up to 16MB. With 4k
>>> pagesize the maximum size of memory allocation is 4MB. Use CMA for
>>> allocation of large tables.
>> Just say use devm_alloc_coherent() to allocate memory.
>>
>>> We use the device managed version of dma_alloc_coherent(). Thus, we
>>> don't need to release it manually on device removal.
>>>
>>> Signed-off-by: Robert Richter <rrichter@cavium.com>
>>> ---
>>>  drivers/irqchip/irq-gic-v3-its.c | 69 +++++++++++++++++++++++-----------------
>>>  1 file changed, 40 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
>>> index 6625b3a505f0..6d293a0165b0 100644
>>> --- a/drivers/irqchip/irq-gic-v3-its.c
>>> +++ b/drivers/irqchip/irq-gic-v3-its.c
> 
>>> @@ -876,13 +878,26 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
>>>  		order = get_order(GITS_BASER_PAGES_MAX * psz);
>>>  	}
>>>  
>>> -	base = (void *)devm_get_free_pages(&its->dev, GFP_KERNEL | __GFP_ZERO,
>>> -					   order);
>>> -	if (!base)
>>> +	base = dmam_alloc_coherent(&its->dev,
>>> +				PAGE_ORDER_TO_SIZE(order),
>>> +				&dma_handle,
>>> +				GFP_KERNEL | __GFP_ZERO);
> 
>> Not just for 1st level device table, you have do a similar code
>> change when allocating memory for 2nd level device table.
> 
> The 2nd level tables are much smaller, so no need for
> dmam_alloc_coherent() there.
> 
> Though, we could use device managed devm_get_free_pages() there too.
> 
>>> +
>>> +	if (!base && order >= MAX_ORDER) {
>>> +		order = MAX_ORDER - 1;
>>> +		dev_warn(&its->dev, "Device Table too large, reduce ids %u->%u, no CMA memory available\n",
>>> +			its->device_ids,
>>> +			ilog2(PAGE_ORDER_TO_SIZE(order) / (int)esz));
>>> +		goto retry_alloc_baser;
>>> +	}
>>> +
>>> +	if (!base) {
>>> +		dev_err(&its->dev, "Failed to allocate device table\n");
>>>  		return -ENOMEM;
>>> +	}
> 
>>> @@ -1698,6 +1706,9 @@ static int __init its_init_one(struct its_node *its)
>>>  		return err;
>>>  	}
>>>  
>>> +	/* Setup dma_ops for dmam_alloc_coherent() */
>>> +	arch_setup_dma_ops(&its->dev, 0, 0, NULL, true);
>>> +
> 
>> Why you are hard-coding DMA coherent property to true here ? It
>> breaks the MSI(x) functionally on systems where ITS hardware doesn't
>> support coherency.
> 
> Aren't current ITS tables coherent only?

No, there is no such guarantee. Actually, there is strictly no need for
coherency, as the ITS tables are only written by the ITS itself, for its
own purpose.

The only things that may benefit from coherency are the property table
and the command queue.

Thanks,

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

Re: [PATCH v2 8/8] irqchip, gicv3-its, cma: Use CMA for allocation of large device tables

[Robert Richter]

On 15.03.17 18:46:22, Marc Zyngier wrote:
> On 15/03/17 18:37, Robert Richter wrote:
> > On 14.03.17 12:40:45, Shanker Donthineni wrote:

> >>> @@ -1698,6 +1706,9 @@ static int __init its_init_one(struct its_node *its)
> >>>  		return err;
> >>>  	}
> >>>  
> >>> +	/* Setup dma_ops for dmam_alloc_coherent() */
> >>> +	arch_setup_dma_ops(&its->dev, 0, 0, NULL, true);
> >>> +
> > 
> >> Why you are hard-coding DMA coherent property to true here ? It
> >> breaks the MSI(x) functionally on systems where ITS hardware doesn't
> >> support coherency.
> > 
> > Aren't current ITS tables coherent only?
> 
> No, there is no such guarantee. Actually, there is strictly no need for
> coherency, as the ITS tables are only written by the ITS itself, for its
> own purpose.

So no need to change that, right?

Thanks,

-Robert

[PATCH v2 8/8] irqchip, gicv3-its, cma: Use CMA for allocation of large device tables

[Robert Richter]

On 15.03.17 18:46:22, Marc Zyngier wrote:
> On 15/03/17 18:37, Robert Richter wrote:
> > On 14.03.17 12:40:45, Shanker Donthineni wrote:

> >>> @@ -1698,6 +1706,9 @@ static int __init its_init_one(struct its_node *its)
> >>>  		return err;
> >>>  	}
> >>>  
> >>> +	/* Setup dma_ops for dmam_alloc_coherent() */
> >>> +	arch_setup_dma_ops(&its->dev, 0, 0, NULL, true);
> >>> +
> > 
> >> Why you are hard-coding DMA coherent property to true here ? It
> >> breaks the MSI(x) functionally on systems where ITS hardware doesn't
> >> support coherency.
> > 
> > Aren't current ITS tables coherent only?
> 
> No, there is no such guarantee. Actually, there is strictly no need for
> coherency, as the ITS tables are only written by the ITS itself, for its
> own purpose.

So no need to change that, right?

Thanks,

-Robert

Re: [PATCH v2 8/8] irqchip, gicv3-its, cma: Use CMA for allocation of large device tables

[Marc Zyngier]

On 16/03/17 13:31, Robert Richter wrote:
> On 15.03.17 18:46:22, Marc Zyngier wrote:
>> On 15/03/17 18:37, Robert Richter wrote:
>>> On 14.03.17 12:40:45, Shanker Donthineni wrote:
> 
>>>>> @@ -1698,6 +1706,9 @@ static int __init its_init_one(struct its_node *its)
>>>>>  		return err;
>>>>>  	}
>>>>>  
>>>>> +	/* Setup dma_ops for dmam_alloc_coherent() */
>>>>> +	arch_setup_dma_ops(&its->dev, 0, 0, NULL, true);
>>>>> +
>>>
>>>> Why you are hard-coding DMA coherent property to true here ? It
>>>> breaks the MSI(x) functionally on systems where ITS hardware doesn't
>>>> support coherency.
>>>
>>> Aren't current ITS tables coherent only?
>>
>> No, there is no such guarantee. Actually, there is strictly no need for
>> coherency, as the ITS tables are only written by the ITS itself, for its
>> own purpose.
> 
> So no need to change that, right?

I don't think there is any. We just need to allocate memory with the
relevant constraints (alignment and zeroing, mostly), and make sure we
never access it directly. Of course, property tables and command queues
would benefit from being allocated as DMA buffers, which would allow the
cache flush to be dealt with at the DMA level.

Thanks,

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

[PATCH v2 8/8] irqchip, gicv3-its, cma: Use CMA for allocation of large device tables

[Marc Zyngier]

On 16/03/17 13:31, Robert Richter wrote:
> On 15.03.17 18:46:22, Marc Zyngier wrote:
>> On 15/03/17 18:37, Robert Richter wrote:
>>> On 14.03.17 12:40:45, Shanker Donthineni wrote:
> 
>>>>> @@ -1698,6 +1706,9 @@ static int __init its_init_one(struct its_node *its)
>>>>>  		return err;
>>>>>  	}
>>>>>  
>>>>> +	/* Setup dma_ops for dmam_alloc_coherent() */
>>>>> +	arch_setup_dma_ops(&its->dev, 0, 0, NULL, true);
>>>>> +
>>>
>>>> Why you are hard-coding DMA coherent property to true here ? It
>>>> breaks the MSI(x) functionally on systems where ITS hardware doesn't
>>>> support coherency.
>>>
>>> Aren't current ITS tables coherent only?
>>
>> No, there is no such guarantee. Actually, there is strictly no need for
>> coherency, as the ITS tables are only written by the ITS itself, for its
>> own purpose.
> 
> So no need to change that, right?

I don't think there is any. We just need to allocate memory with the
relevant constraints (alignment and zeroing, mostly), and make sure we
never access it directly. Of course, property tables and command queues
would benefit from being allocated as DMA buffers, which would allow the
cache flush to be dealt with at the DMA level.

Thanks,

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

Re: [PATCH v2 8/8] irqchip, gicv3-its, cma: Use CMA for allocation of large device tables

[Shanker Donthineni]

Hi Marc/Robert,

On 03/16/2017 08:41 AM, Marc Zyngier wrote:
> On 16/03/17 13:31, Robert Richter wrote:
>> On 15.03.17 18:46:22, Marc Zyngier wrote:
>>> On 15/03/17 18:37, Robert Richter wrote:
>>>> On 14.03.17 12:40:45, Shanker Donthineni wrote:
>>>>>> @@ -1698,6 +1706,9 @@ static int __init its_init_one(struct its_node *its)
>>>>>>  		return err;
>>>>>>  	}
>>>>>>  
>>>>>> +	/* Setup dma_ops for dmam_alloc_coherent() */
>>>>>> +	arch_setup_dma_ops(&its->dev, 0, 0, NULL, true);
>>>>>> +
>>>>> Why you are hard-coding DMA coherent property to true here ? It
>>>>> breaks the MSI(x) functionally on systems where ITS hardware doesn't
>>>>> support coherency.
>>>> Aren't current ITS tables coherent only?
>>> No, there is no such guarantee. Actually, there is strictly no need for
>>> coherency, as the ITS tables are only written by the ITS itself, for its
>>> own purpose.
>> So no need to change that, right?
> I don't think there is any. We just need to allocate memory with the
> relevant constraints (alignment and zeroing, mostly), and make sure we
> never access it directly. Of course, property tables and command queues
> would benefit from being allocated as DMA buffers, which would allow the
> cache flush to be dealt with at the DMA level.

Agree with Marc, only PROP tables and CMD queue buffers are touched by CPU during runtime. That means DMA coherent property set to true only when both the CMD_BASE (ITS_FLAGS_CMDQ_NEEDS_FLUSHING) and PROP_BASE (RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING) tables support coherency.

> Thanks,
>
> 	M.

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

[PATCH v2 8/8] irqchip, gicv3-its, cma: Use CMA for allocation of large device tables

[Shanker Donthineni]

Hi Marc/Robert,

On 03/16/2017 08:41 AM, Marc Zyngier wrote:
> On 16/03/17 13:31, Robert Richter wrote:
>> On 15.03.17 18:46:22, Marc Zyngier wrote:
>>> On 15/03/17 18:37, Robert Richter wrote:
>>>> On 14.03.17 12:40:45, Shanker Donthineni wrote:
>>>>>> @@ -1698,6 +1706,9 @@ static int __init its_init_one(struct its_node *its)
>>>>>>  		return err;
>>>>>>  	}
>>>>>>  
>>>>>> +	/* Setup dma_ops for dmam_alloc_coherent() */
>>>>>> +	arch_setup_dma_ops(&its->dev, 0, 0, NULL, true);
>>>>>> +
>>>>> Why you are hard-coding DMA coherent property to true here ? It
>>>>> breaks the MSI(x) functionally on systems where ITS hardware doesn't
>>>>> support coherency.
>>>> Aren't current ITS tables coherent only?
>>> No, there is no such guarantee. Actually, there is strictly no need for
>>> coherency, as the ITS tables are only written by the ITS itself, for its
>>> own purpose.
>> So no need to change that, right?
> I don't think there is any. We just need to allocate memory with the
> relevant constraints (alignment and zeroing, mostly), and make sure we
> never access it directly. Of course, property tables and command queues
> would benefit from being allocated as DMA buffers, which would allow the
> cache flush to be dealt with at the DMA level.

Agree with Marc, only PROP tables and CMD queue buffers are touched by CPU during runtime. That means DMA coherent property set to true only when both the CMD_BASE (ITS_FLAGS_CMDQ_NEEDS_FLUSHING) and PROP_BASE (RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING) tables support coherency.

> Thanks,
>
> 	M.

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