[PATCH V4 0/5] Add two-level support for ITS device table

[PATCH V4 0/5] Add two-level support for ITS device table

[Shanker Donthineni]

This patchset contains necessary code changes to support two-level
(Indirection) table walk feature for device table.

Shanker Donthineni (5):
  irqchip/gicv3-its: Introduce two helper functions for accessing BASERn
  irqchip/gicv3-its: Add a new function for parsing device table BASERn
  irqchip/gicv3-its: Split its_alloc_tables() into two functions
  irqchip/gicv3-its: Remove an unused argument 'node_name'
  irqchip/gicv3-its: Implement two-level(indirect) device table support

 drivers/irqchip/irq-gic-v3-its.c   | 414 +++++++++++++++++++++++--------------
 include/linux/irqchip/arm-gic-v3.h |   4 +
 2 files changed, 266 insertions(+), 152 deletions(-)

-- 
This patchset created on top of Marc Zyngier's branch https://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/log/?h=irq/irqchip-4.7-rc1

I have tested the Indirection feature on Qualcomm Technologies QDF2XXX server platform. See the below ITS driver log mesaages.
[    0.000000] GICv3: GIC: Using split EOI/Deactivate mode
[    0.000000] ITS@0x00000000397e0000
[    0.000000] ITS@0x00000000397e0000: allocated 524288 Devices @47c0400000 (indirect, esz 8, psz 64K, shr 1)
[    0.000000] ITS@0x00000000397e0000: allocated 8192 Interrupt Collections @47c0050000 (flat, esz 8, psz 64K, shr 1)
[    0.000000] ITS@0x00000000397e0000: allocated 8192 Virtual CPUs @47c0060000 (flat, esz 8, psz 64K, shr 1)

Changes since v3:
  Added pathces 1/5, 2/5 and 4/5.

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

[PATCH V4 1/5] irqchip/gicv3-its: Introduce two helper functions for accessing BASERn

[Shanker Donthineni]

This patch adds the two handy helper functions for reading and writing
ITS BASERn register.

Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
---
 drivers/irqchip/irq-gic-v3-its.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 5eb1f9e..6392c82 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -56,13 +56,14 @@ struct its_collection {
 };
 
 /*
- * The ITS_BASER structure - contains memory information and cached
- * value of BASER register configuration.
+ * The ITS_BASER structure - contains memory information, cached value
+ * of BASER register configuration and register idx.
  */
 struct its_baser {
 	void		*base;
 	u64		val;
 	u32		order;
+	u32		idx;
 };
 
 /*
@@ -824,6 +825,17 @@ static const char *its_base_type_string[] = {
 	[GITS_BASER_TYPE_RESERVED7] 	= "Reserved (7)",
 };
 
+static u64 its_read_baser(struct its_node *its, struct its_baser *baser)
+{
+	return readq_relaxed(its->base + GITS_BASER + (baser->idx << 3));
+}
+
+static void its_write_baser(struct its_node *its, struct its_baser *baser,
+			    u64 val)
+{
+	writeq_relaxed(val, its->base + GITS_BASER + (baser->idx << 3));
+}
+
 static void its_free_tables(struct its_node *its)
 {
 	int i;
@@ -863,14 +875,20 @@ static int its_alloc_tables(const char *node_name, struct its_node *its)
 	its->device_ids = ids;
 
 	for (i = 0; i < GITS_BASER_NR_REGS; i++) {
-		u64 val = readq_relaxed(its->base + GITS_BASER + i * 8);
-		u64 type = GITS_BASER_TYPE(val);
-		u64 entry_size = GITS_BASER_ENTRY_SIZE(val);
+		struct its_baser *baser = its->tables + i;
 		int order = get_order(psz);
+		u64 val, type, entry_size;
 		int alloc_pages;
 		u64 tmp;
 		void *base;
 
+		/* Record the register index */
+		baser->idx = i;
+
+		val = its_read_baser(its, baser);
+		type = GITS_BASER_TYPE(val);
+		entry_size = GITS_BASER_ENTRY_SIZE(val);
+
 		if (type == GITS_BASER_TYPE_NONE)
 			continue;
 
@@ -939,8 +957,8 @@ retry_baser:
 		val |= alloc_pages - 1;
 		its->tables[i].val = val;
 
-		writeq_relaxed(val, its->base + GITS_BASER + i * 8);
-		tmp = readq_relaxed(its->base + GITS_BASER + i * 8);
+		its_write_baser(its, baser, val);
+		tmp = its_read_baser(its, baser);
 
 		if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) {
 			/*
-- 
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project

[PATCH V4 2/5] irqchip/gicv3-its: Add a new function for parsing device table BASERn

[Shanker Donthineni]

Only the device table BASERn needs to be handled differently as
compared to all other tables. So, adding a separate function for
easy code maintenance and improved code readability.

Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
---
 drivers/irqchip/irq-gic-v3-its.c | 49 +++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 6392c82..4bd9381 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -836,6 +836,30 @@ static void its_write_baser(struct its_node *its, struct its_baser *baser,
 	writeq_relaxed(val, its->base + GITS_BASER + (baser->idx << 3));
 }
 
+static void its_parse_baser_device(struct its_node *its, struct its_baser *baser,
+				   u32 *order)
+{
+	u64 esz = GITS_BASER_ENTRY_SIZE(its_read_baser(its, baser));
+	u32 ids = its->device_ids;
+	u32 new_order = *order;
+
+	/*
+	 * Allocate as many entries as required to fit the
+	 * range of device IDs that the ITS can grok... The ID
+	 * space being incredibly sparse, this results in a
+	 * massive waste of memory.
+	 */
+	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) / esz);
+		pr_warn("ITS@%pa: Device Table too large, reduce ids %u->%u\n",
+			&its->phys_base, its->device_ids, ids);
+	}
+
+	*order = new_order;
+}
+
 static void its_free_tables(struct its_node *its)
 {
 	int i;
@@ -892,29 +916,8 @@ static int its_alloc_tables(const char *node_name, struct its_node *its)
 		if (type == GITS_BASER_TYPE_NONE)
 			continue;
 
-		/*
-		 * Allocate as many entries as required to fit the
-		 * range of device IDs that the ITS can grok... The ID
-		 * space being incredibly sparse, this results in a
-		 * massive waste of memory.
-		 *
-		 * For other tables, only allocate a single page.
-		 */
-		if (type == GITS_BASER_TYPE_DEVICE) {
-			/*
-			 * 'order' was initialized earlier to the default page
-			 * granule of the the ITS.  We can't have an allocation
-			 * smaller than that.  If the requested allocation
-			 * is smaller, round up to the default page granule.
-			 */
-			order = max(get_order((1UL << ids) * entry_size),
-				    order);
-			if (order >= MAX_ORDER) {
-				order = MAX_ORDER - 1;
-				pr_warn("%s: Device Table too large, reduce its page order to %u\n",
-					node_name, order);
-			}
-		}
+		if (type == GITS_BASER_TYPE_DEVICE)
+			its_parse_baser_device(its, baser, &order);
 
 retry_alloc_baser:
 		alloc_pages = (PAGE_ORDER_TO_SIZE(order) / psz);
-- 
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project

[PATCH V4 3/5] irqchip/gicv3-its: Split its_alloc_tables() into two functions

[Shanker Donthineni]

The function is getting out of control, it has too many goto
statements and would be too complicated for adding a feature
two-level device table. So, it is time for us to cleanup and
move some of the logic to a separate function without affecting
the existing functionality.

Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
---
Changes since v3:
   Moved all changes that are related to Indirection to patch 5/5.

 drivers/irqchip/irq-gic-v3-its.c   | 250 ++++++++++++++++++++-----------------
 include/linux/irqchip/arm-gic-v3.h |   1 +
 2 files changed, 134 insertions(+), 117 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 4bd9381..9c6ecc8 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -57,13 +57,14 @@ struct its_collection {
 
 /*
  * The ITS_BASER structure - contains memory information, cached value
- * of BASER register configuration and register idx.
+ * of BASER register configuration, register idx and page size.
  */
 struct its_baser {
 	void		*base;
 	u64		val;
 	u32		order;
 	u32		idx;
+	u32		psz;
 };
 
 /*
@@ -836,6 +837,116 @@ static void its_write_baser(struct its_node *its, struct its_baser *baser,
 	writeq_relaxed(val, its->base + GITS_BASER + (baser->idx << 3));
 }
 
+static int its_setup_baser(struct its_node *its, struct its_baser *baser,
+			   u32 order)
+{
+	u64 val = its_read_baser(its, baser);
+	u64 esz = GITS_BASER_ENTRY_SIZE(val);
+	u64 type = GITS_BASER_TYPE(val);
+	int psz, alloc_pages;
+	u64 cache, shr, tmp;
+	void *base;
+
+	/* Do first attempt with the requested attributes */
+	cache = baser->val & GITS_BASER_CACHEABILITY_MASK;
+	shr = baser->val & GITS_BASER_SHAREABILITY_MASK;
+	psz = baser->psz;
+
+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);
+		alloc_pages = GITS_BASER_PAGES_MAX;
+		order = get_order(GITS_BASER_PAGES_MAX * psz);
+	}
+
+	base = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, order);
+	if (!base)
+		return -ENOMEM;
+
+retry_baser:
+	val = (virt_to_phys(base)				 |
+		(type << GITS_BASER_TYPE_SHIFT)			 |
+		((esz - 1) << GITS_BASER_ENTRY_SIZE_SHIFT)	 |
+		((alloc_pages - 1) << GITS_BASER_PAGES_SHIFT)	 |
+		cache						 |
+		shr						 |
+		GITS_BASER_VALID);
+
+	switch (psz) {
+	case SZ_4K:
+		val |= GITS_BASER_PAGE_SIZE_4K;
+		break;
+	case SZ_16K:
+		val |= GITS_BASER_PAGE_SIZE_16K;
+		break;
+	case SZ_64K:
+		val |= GITS_BASER_PAGE_SIZE_64K;
+		break;
+	}
+
+	its_write_baser(its, baser, val);
+	tmp = its_read_baser(its, baser);
+
+	if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) {
+		/*
+		 * Shareability didn't stick. Just use
+		 * whatever the read reported, which is likely
+		 * to be the only thing this redistributor
+		 * supports. If that's zero, make it
+		 * non-cacheable as well.
+		 */
+		shr = tmp & GITS_BASER_SHAREABILITY_MASK;
+		if (!shr) {
+			cache = GITS_BASER_nC;
+			__flush_dcache_area(base, PAGE_ORDER_TO_SIZE(order));
+		}
+		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...
+		 */
+		free_pages((unsigned long)base, order);
+		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) {
+		pr_err("ITS@%pa: %s doesn't stick: %lx %lx\n",
+		       &its->phys_base, its_base_type_string[type],
+		       (unsigned long) val, (unsigned long) tmp);
+		free_pages((unsigned long)base, order);
+		return -ENXIO;
+	}
+
+	baser->order = order;
+	baser->base = base;
+	baser->psz = psz;
+	baser->val = val;
+
+	pr_info("ITS@%pa: allocated %d %s @%lx (psz %dK, shr %d)\n",
+		&its->phys_base, (int)(PAGE_ORDER_TO_SIZE(order) / esz),
+		its_base_type_string[type],
+		(unsigned long)virt_to_phys(base),
+		psz / SZ_1K, (int)shr >> GITS_BASER_SHAREABILITY_SHIFT);
+
+	return 0;
+}
+
 static void its_parse_baser_device(struct its_node *its, struct its_baser *baser,
 				   u32 *order)
 {
@@ -875,43 +986,35 @@ static void its_free_tables(struct its_node *its)
 
 static int its_alloc_tables(const char *node_name, struct its_node *its)
 {
-	int err;
-	int i;
-	int psz = SZ_64K;
+	u64 typer = readq_relaxed(its->base + GITS_TYPER);
+	u32 ids = GITS_TYPER_DEVBITS(typer);
 	u64 shr = GITS_BASER_InnerShareable;
-	u64 cache;
-	u64 typer;
-	u32 ids;
+	u64 cache = GITS_BASER_WaWb;
+	int psz = SZ_64K;
+	int err, i;
 
 	if (its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_22375) {
 		/*
 		 * erratum 22375: only alloc 8MB table size
 		 * erratum 24313: ignore memory access type
 		 */
-		cache	= 0;
-		ids	= 0x14;			/* 20 bits, 8MB */
-	} else {
-		cache	= GITS_BASER_WaWb;
-		typer	= readq_relaxed(its->base + GITS_TYPER);
-		ids	= GITS_TYPER_DEVBITS(typer);
+		cache   = GITS_BASER_nCnB;
+		ids     = 0x14;                 /* 20 bits, 8MB */
 	}
 
 	its->device_ids = ids;
 
 	for (i = 0; i < GITS_BASER_NR_REGS; i++) {
 		struct its_baser *baser = its->tables + i;
-		int order = get_order(psz);
-		u64 val, type, entry_size;
-		int alloc_pages;
-		u64 tmp;
-		void *base;
+		u32 order = get_order(psz);
+		u64 type;
 
-		/* Record the register index */
+		/* Record the register index and set preferred settings */
 		baser->idx = i;
+		baser->val = cache | shr;
+		baser->psz = psz;
 
-		val = its_read_baser(its, baser);
-		type = GITS_BASER_TYPE(val);
-		entry_size = GITS_BASER_ENTRY_SIZE(val);
+		type = GITS_BASER_TYPE(its_read_baser(its, baser));
 
 		if (type == GITS_BASER_TYPE_NONE)
 			continue;
@@ -919,106 +1022,19 @@ static int its_alloc_tables(const char *node_name, struct its_node *its)
 		if (type == GITS_BASER_TYPE_DEVICE)
 			its_parse_baser_device(its, baser, &order);
 
-retry_alloc_baser:
-		alloc_pages = (PAGE_ORDER_TO_SIZE(order) / psz);
-		if (alloc_pages > GITS_BASER_PAGES_MAX) {
-			alloc_pages = GITS_BASER_PAGES_MAX;
-			order = get_order(GITS_BASER_PAGES_MAX * psz);
-			pr_warn("%s: Device Table too large, reduce its page order to %u (%u pages)\n",
-				node_name, order, alloc_pages);
-		}
-
-		base = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, order);
-		if (!base) {
-			err = -ENOMEM;
-			goto out_free;
-		}
-
-		its->tables[i].base = base;
-		its->tables[i].order = order;
-
-retry_baser:
-		val = (virt_to_phys(base) 				 |
-		       (type << GITS_BASER_TYPE_SHIFT)			 |
-		       ((entry_size - 1) << GITS_BASER_ENTRY_SIZE_SHIFT) |
-		       cache						 |
-		       shr						 |
-		       GITS_BASER_VALID);
-
-		switch (psz) {
-		case SZ_4K:
-			val |= GITS_BASER_PAGE_SIZE_4K;
-			break;
-		case SZ_16K:
-			val |= GITS_BASER_PAGE_SIZE_16K;
-			break;
-		case SZ_64K:
-			val |= GITS_BASER_PAGE_SIZE_64K;
-			break;
-		}
-
-		val |= alloc_pages - 1;
-		its->tables[i].val = val;
-
-		its_write_baser(its, baser, val);
-		tmp = its_read_baser(its, baser);
-
-		if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) {
-			/*
-			 * Shareability didn't stick. Just use
-			 * whatever the read reported, which is likely
-			 * to be the only thing this redistributor
-			 * supports. If that's zero, make it
-			 * non-cacheable as well.
-			 */
-			shr = tmp & GITS_BASER_SHAREABILITY_MASK;
-			if (!shr) {
-				cache = GITS_BASER_nC;
-				__flush_dcache_area(base, PAGE_ORDER_TO_SIZE(order));
-			}
-			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...
-			 */
-			free_pages((unsigned long)base, order);
-			its->tables[i].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) {
-			pr_err("ITS: %s: GITS_BASER%d doesn't stick: %lx %lx\n",
-			       node_name, i,
-			       (unsigned long) val, (unsigned long) tmp);
-			err = -ENXIO;
-			goto out_free;
+		err = its_setup_baser(its, baser, order);
+		if (err < 0) {
+			its_free_tables(its);
+			return err;
 		}
 
-		pr_info("ITS: allocated %d %s @%lx (psz %dK, shr %d)\n",
-			(int)(PAGE_ORDER_TO_SIZE(order) / entry_size),
-			its_base_type_string[type],
-			(unsigned long)virt_to_phys(base),
-			psz / SZ_1K, (int)shr >> GITS_BASER_SHAREABILITY_SHIFT);
+		/* Update settings which will be used for next BASERn */
+		psz = baser->psz;
+		cache = baser->val & GITS_BASER_CACHEABILITY_MASK;
+		shr = baser->val & GITS_BASER_SHAREABILITY_MASK;
 	}
 
 	return 0;
-
-out_free:
-	its_free_tables(its);
-
-	return err;
 }
 
 static int its_alloc_collections(struct its_node *its)
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index dc493e0..01cf171 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -228,6 +228,7 @@
 #define GITS_BASER_PAGE_SIZE_64K	(2UL << GITS_BASER_PAGE_SIZE_SHIFT)
 #define GITS_BASER_PAGE_SIZE_MASK	(3UL << GITS_BASER_PAGE_SIZE_SHIFT)
 #define GITS_BASER_PAGES_MAX		256
+#define GITS_BASER_PAGES_SHIFT		(0)
 
 #define GITS_BASER_TYPE_NONE		0
 #define GITS_BASER_TYPE_DEVICE		1
-- 
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project

[PATCH V4 4/5] irqchip/gicv3-its: Remove an unused argument 'node_name'

[Shanker Donthineni]

No references to argument 'node_name' after modifying pr_xxx()
messages to include ITS base address instead of 'node_name'.

Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
---
 drivers/irqchip/irq-gic-v3-its.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 9c6ecc8..4f9e288 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -984,7 +984,7 @@ static void its_free_tables(struct its_node *its)
 	}
 }
 
-static int its_alloc_tables(const char *node_name, struct its_node *its)
+static int its_alloc_tables(struct its_node *its)
 {
 	u64 typer = readq_relaxed(its->base + GITS_TYPER);
 	u32 ids = GITS_TYPER_DEVBITS(typer);
@@ -1606,7 +1606,7 @@ static int __init its_probe(struct device_node *node,
 
 	its_enable_quirks(its);
 
-	err = its_alloc_tables(node->full_name, its);
+	err = its_alloc_tables(its);
 	if (err)
 		goto out_free_cmd;
 
-- 
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project

[PATCH V4 5/5] irqchip/gicv3-its: Implement two-level(indirect) device table support

[Shanker Donthineni]

Since device IDs are extremely sparse, the single, a.k.a flat table is
not sufficient for the following two reasons.

1) According to ARM-GIC spec, ITS hw can access maximum of 256(pages)*
   64K(pageszie) bytes. In the best case, it supports upto DEVid=21
   sparse with minimum device table entry size 8bytes.

2) The maximum memory size that is possible without memblock depends on
   MAX_ORDER. 4MB on 4K page size kernel with default MAX_ORDER, so it
   supports DEVid range 19bits.

The two-level device table feature brings us two advantages, the first
is a very high possibility of supporting upto 32bit sparse, and the
second one is the best utilization of memory allocation.

The feature is enabled automatically during driver probe if the memory
requirement is more than 2*ITS-pages and the hardware is capable of
two-level table walk.

Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
---
Changes since v3:
  Changed level-one table pointer type from 'u64 *' to '__le64 *'
  Addressed Marc's review omments.

Changes since v2:
  Fixed a porting bug device 'id' validation check in its_alloc_device_table()

Changes since v1:
  Most of this patch has been rewritten after refactoring its_alloc_tables().
  Always enable device two-level if the memory requirement is more than PAGE_SIZE.
  Fixed the coding bug that breaks on the BE machine.
  Edited the commit text.

 drivers/irqchip/irq-gic-v3-its.c   | 105 +++++++++++++++++++++++++++++++------
 include/linux/irqchip/arm-gic-v3.h |   3 ++
 2 files changed, 92 insertions(+), 16 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 4f9e288..271c7f3 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -838,7 +838,7 @@ static void its_write_baser(struct its_node *its, struct its_baser *baser,
 }
 
 static int its_setup_baser(struct its_node *its, struct its_baser *baser,
-			   u32 order)
+			   u32 order, bool indirect)
 {
 	u64 val = its_read_baser(its, baser);
 	u64 esz = GITS_BASER_ENTRY_SIZE(val);
@@ -875,6 +875,8 @@ retry_baser:
 		shr						 |
 		GITS_BASER_VALID);
 
+	val |=	indirect ? GITS_BASER_INDIRECT : 0x0;
+
 	switch (psz) {
 	case SZ_4K:
 		val |= GITS_BASER_PAGE_SIZE_4K;
@@ -937,28 +939,56 @@ retry_baser:
 	baser->base = base;
 	baser->psz = psz;
 	baser->val = val;
+	tmp = indirect ? GITS_LVL1_ENTRY_SIZE : esz;
 
-	pr_info("ITS@%pa: allocated %d %s @%lx (psz %dK, shr %d)\n",
-		&its->phys_base, (int)(PAGE_ORDER_TO_SIZE(order) / 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) / tmp),
 		its_base_type_string[type],
 		(unsigned long)virt_to_phys(base),
+		indirect ? "indirect" : "flat", (int)esz,
 		psz / SZ_1K, (int)shr >> GITS_BASER_SHAREABILITY_SHIFT);
 
 	return 0;
 }
 
-static void its_parse_baser_device(struct its_node *its, struct its_baser *baser,
+static bool its_parse_baser_device(struct its_node *its, struct its_baser *baser,
 				   u32 *order)
 {
 	u64 esz = GITS_BASER_ENTRY_SIZE(its_read_baser(its, baser));
+	u64 val = GITS_BASER_InnerShareable | GITS_BASER_WaWb;
 	u32 ids = its->device_ids;
 	u32 new_order = *order;
+	bool indirect = false;
+
+	/* No need to enable Indirection if memory requirement < (psz*2)bytes */
+	if ((esz << ids) > (baser->psz * 2)) {
+		/*
+		 * Find out whether hw supports a single or two-level table by
+		 * table by reading bit at offset '62' after writing '1' to it.
+		 */
+		its_write_baser(its, baser, val | GITS_BASER_INDIRECT);
+		val = its_read_baser(its, baser);
+		indirect = !!(val & GITS_BASER_INDIRECT);
+
+		if (indirect) {
+			/*
+			 * The size of the lvl2 table is equal to ITS page size
+			 * which is 'psz'. For computing lvl1 table size,
+			 * subtract ID bits that sparse lvl2 table from 'ids'
+			 * which is reported by ITS hardware times lvl1 table
+			 * entry size.
+			 */
+			ids -= ilog2(baser->psz / esz);
+			esz = GITS_LVL1_ENTRY_SIZE;
+		}
+	}
 
 	/*
 	 * Allocate as many entries as required to fit the
 	 * range of device IDs that the ITS can grok... The ID
 	 * space being incredibly sparse, this results in a
-	 * massive waste of memory.
+	 * massive waste of memory if two-level device table
+	 * feature is not supported by hardware.
 	 */
 	new_order = max_t(u32, get_order(esz << ids), new_order);
 	if (new_order >= MAX_ORDER) {
@@ -969,6 +999,8 @@ static void its_parse_baser_device(struct its_node *its, struct its_baser *baser
 	}
 
 	*order = new_order;
+
+	return indirect;
 }
 
 static void its_free_tables(struct its_node *its)
@@ -1007,6 +1039,7 @@ static int its_alloc_tables(struct its_node *its)
 	for (i = 0; i < GITS_BASER_NR_REGS; i++) {
 		struct its_baser *baser = its->tables + i;
 		u32 order = get_order(psz);
+		bool indirect = false;
 		u64 type;
 
 		/* Record the register index and set preferred settings */
@@ -1020,9 +1053,9 @@ static int its_alloc_tables(struct its_node *its)
 			continue;
 
 		if (type == GITS_BASER_TYPE_DEVICE)
-			its_parse_baser_device(its, baser, &order);
+			indirect = its_parse_baser_device(its, baser, &order);
 
-		err = its_setup_baser(its, baser, order);
+		err = its_setup_baser(its, baser, order, indirect);
 		if (err < 0) {
 			its_free_tables(its);
 			return err;
@@ -1222,10 +1255,57 @@ static struct its_baser *its_get_baser(struct its_node *its, u32 type)
 	return NULL;
 }
 
+static bool its_alloc_device_table(struct its_node *its, u32 dev_id)
+{
+	struct its_baser *baser;
+	struct page *page;
+	u32 esz, idx;
+	__le64 *table;
+
+	baser = its_get_baser(its, GITS_BASER_TYPE_DEVICE);
+
+	/* Don't allow device id that exceeds ITS hardware limit */
+	if (!baser)
+		return (ilog2(dev_id) < its->device_ids);
+
+	/* Don't allow device id that exceeds single, flat table limit */
+	esz = GITS_BASER_ENTRY_SIZE(baser->val);
+	if (!(baser->val & GITS_BASER_INDIRECT))
+		return (dev_id < (PAGE_ORDER_TO_SIZE(baser->order) / esz));
+
+	/* Compute 1st level table index & check if that exceeds table limit */
+	idx = dev_id >> ilog2(baser->psz / esz);
+	if (idx >= (PAGE_ORDER_TO_SIZE(baser->order) / GITS_LVL1_ENTRY_SIZE))
+		return false;
+
+	table = baser->base;
+
+	/* Allocate memory for 2nd level table */
+	if (!table[idx]) {
+		page = alloc_pages(GFP_KERNEL | __GFP_ZERO, get_order(baser->psz));
+		if (!page)
+			return false;
+
+		/* Flush Lvl2 table to PoC if hw doesn't support coherency */
+		if (!(baser->val & GITS_BASER_SHAREABILITY_MASK))
+			__flush_dcache_area(page_address(page), baser->psz);
+
+		table[idx] = cpu_to_le64(page_to_phys(page) | GITS_BASER_VALID);
+
+		/* Flush Lvl1 entry to PoC if hw doesn't support coherency */
+		if (!(baser->val & GITS_BASER_SHAREABILITY_MASK))
+			__flush_dcache_area(table + idx, GITS_LVL1_ENTRY_SIZE);
+
+		/* Ensure updated table contents are visible to ITS hardware */
+		dsb(sy);
+	}
+
+	return true;
+}
+
 static struct its_device *its_create_device(struct its_node *its, u32 dev_id,
 					    int nvecs)
 {
-	struct its_baser *baser;
 	struct its_device *dev;
 	unsigned long *lpi_map;
 	unsigned long flags;
@@ -1236,14 +1316,7 @@ static struct its_device *its_create_device(struct its_node *its, u32 dev_id,
 	int nr_ites;
 	int sz;
 
-	baser = its_get_baser(its, GITS_BASER_TYPE_DEVICE);
-
-	/* Don't allow 'dev_id' that exceeds single, flat table limit */
-	if (baser) {
-		if (dev_id >= (PAGE_ORDER_TO_SIZE(baser->order) /
-			      GITS_BASER_ENTRY_SIZE(baser->val)))
-			return NULL;
-	} else if (ilog2(dev_id) >= its->device_ids)
+	if (!its_alloc_device_table(its, dev_id))
 		return NULL;
 
 	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index 01cf171..107eed4 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -204,6 +204,7 @@
 #define GITS_BASER_NR_REGS		8
 
 #define GITS_BASER_VALID		(1UL << 63)
+#define GITS_BASER_INDIRECT		(1UL << 62)
 #define GITS_BASER_nCnB			(0UL << 59)
 #define GITS_BASER_nC			(1UL << 59)
 #define GITS_BASER_RaWt			(2UL << 59)
@@ -239,6 +240,8 @@
 #define GITS_BASER_TYPE_RESERVED6	6
 #define GITS_BASER_TYPE_RESERVED7	7
 
+#define GITS_LVL1_ENTRY_SIZE           (8UL)
+
 /*
  * ITS commands
  */
-- 
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project

Re: [PATCH V4 1/5] irqchip/gicv3-its: Introduce two helper functions for accessing BASERn

[Marc Zyngier]

Hi Shanker,

On 06/06/16 06:40, Shanker Donthineni wrote:
> This patch adds the two handy helper functions for reading and writing
> ITS BASERn register.
> 
> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 32 +++++++++++++++++++++++++-------
>  1 file changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 5eb1f9e..6392c82 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -56,13 +56,14 @@ struct its_collection {
>  };
>  
>  /*
> - * The ITS_BASER structure - contains memory information and cached
> - * value of BASER register configuration.
> + * The ITS_BASER structure - contains memory information, cached value
> + * of BASER register configuration and register idx.
>   */
>  struct its_baser {
>  	void		*base;
>  	u64		val;
>  	u32		order;
> +	u32		idx;
>  };
>  
>  /*
> @@ -824,6 +825,17 @@ static const char *its_base_type_string[] = {
>  	[GITS_BASER_TYPE_RESERVED7] 	= "Reserved (7)",
>  };
>  
> +static u64 its_read_baser(struct its_node *its, struct its_baser *baser)
> +{
> +	return readq_relaxed(its->base + GITS_BASER + (baser->idx << 3));
> +}
> +
> +static void its_write_baser(struct its_node *its, struct its_baser *baser,
> +			    u64 val)
> +{
> +	writeq_relaxed(val, its->base + GITS_BASER + (baser->idx << 3));
> +}
> +
>  static void its_free_tables(struct its_node *its)
>  {
>  	int i;
> @@ -863,14 +875,20 @@ static int its_alloc_tables(const char *node_name, struct its_node *its)
>  	its->device_ids = ids;
>  
>  	for (i = 0; i < GITS_BASER_NR_REGS; i++) {
> -		u64 val = readq_relaxed(its->base + GITS_BASER + i * 8);
> -		u64 type = GITS_BASER_TYPE(val);
> -		u64 entry_size = GITS_BASER_ENTRY_SIZE(val);
> +		struct its_baser *baser = its->tables + i;
>  		int order = get_order(psz);
> +		u64 val, type, entry_size;
>  		int alloc_pages;
>  		u64 tmp;
>  		void *base;
>  
> +		/* Record the register index */
> +		baser->idx = i;
> +
> +		val = its_read_baser(its, baser);
> +		type = GITS_BASER_TYPE(val);
> +		entry_size = GITS_BASER_ENTRY_SIZE(val);
> +
>  		if (type == GITS_BASER_TYPE_NONE)
>  			continue;
>  
> @@ -939,8 +957,8 @@ retry_baser:
>  		val |= alloc_pages - 1;
>  		its->tables[i].val = val;
>  
> -		writeq_relaxed(val, its->base + GITS_BASER + i * 8);
> -		tmp = readq_relaxed(its->base + GITS_BASER + i * 8);
> +		its_write_baser(its, baser, val);
> +		tmp = its_read_baser(its, baser);
>  
>  		if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) {
>  			/*
> 

It is worth realizing that we always end-up with baser being a pointer 
to one of the its->tables entries. We can easily simplify the whole 
thing and drop the idx field:

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 271c7f3..520d171 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -63,7 +63,6 @@ struct its_baser {
 	void		*base;
 	u64		val;
 	u32		order;
-	u32		idx;
 	u32		psz;
 };
 
@@ -828,13 +827,15 @@ static const char *its_base_type_string[] = {
 
 static u64 its_read_baser(struct its_node *its, struct its_baser *baser)
 {
-	return readq_relaxed(its->base + GITS_BASER + (baser->idx << 3));
+	int idx = baser - its->tables;
+	return readq_relaxed(its->base + GITS_BASER + (idx << 3));
 }
 
 static void its_write_baser(struct its_node *its, struct its_baser *baser,
 			    u64 val)
 {
-	writeq_relaxed(val, its->base + GITS_BASER + (baser->idx << 3));
+	int idx = baser - its->tables;
+	writeq_relaxed(val, its->base + GITS_BASER + (idx << 3));
 }
 
 static int its_setup_baser(struct its_node *its, struct its_baser *baser,
@@ -1042,8 +1043,7 @@ static int its_alloc_tables(struct its_node *its)
 		bool indirect = false;
 		u64 type;
 
-		/* Record the register index and set preferred settings */
-		baser->idx = i;
+		/* Record the preferred settings */
 		baser->val = cache | shr;
 		baser->psz = psz;
 

Thanks,

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

Re: [PATCH V4 1/5] irqchip/gicv3-its: Introduce two helper functions for accessing BASERn

[Marc Zyngier]

On 06/06/16 06:40, Shanker Donthineni wrote:
> This patch adds the two handy helper functions for reading and writing
> ITS BASERn register.
> 
> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 32 +++++++++++++++++++++++++-------
>  1 file changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 5eb1f9e..6392c82 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -56,13 +56,14 @@ struct its_collection {
>  };
>  
>  /*
> - * The ITS_BASER structure - contains memory information and cached
> - * value of BASER register configuration.
> + * The ITS_BASER structure - contains memory information, cached value
> + * of BASER register configuration and register idx.
>   */
>  struct its_baser {
>  	void		*base;
>  	u64		val;
>  	u32		order;
> +	u32		idx;
>  };
>  
>  /*
> @@ -824,6 +825,17 @@ static const char *its_base_type_string[] = {
>  	[GITS_BASER_TYPE_RESERVED7] 	= "Reserved (7)",
>  };
>  
> +static u64 its_read_baser(struct its_node *its, struct its_baser *baser)
> +{
> +	return readq_relaxed(its->base + GITS_BASER + (baser->idx << 3));
> +}
> +
> +static void its_write_baser(struct its_node *its, struct its_baser *baser,
> +			    u64 val)
> +{
> +	writeq_relaxed(val, its->base + GITS_BASER + (baser->idx << 3));
> +}
> +
>  static void its_free_tables(struct its_node *its)
>  {
>  	int i;
> @@ -863,14 +875,20 @@ static int its_alloc_tables(const char *node_name, struct its_node *its)
>  	its->device_ids = ids;
>  
>  	for (i = 0; i < GITS_BASER_NR_REGS; i++) {
> -		u64 val = readq_relaxed(its->base + GITS_BASER + i * 8);
> -		u64 type = GITS_BASER_TYPE(val);
> -		u64 entry_size = GITS_BASER_ENTRY_SIZE(val);
> +		struct its_baser *baser = its->tables + i;
>  		int order = get_order(psz);
> +		u64 val, type, entry_size;
>  		int alloc_pages;
>  		u64 tmp;
>  		void *base;
>  
> +		/* Record the register index */
> +		baser->idx = i;
> +
> +		val = its_read_baser(its, baser);
> +		type = GITS_BASER_TYPE(val);
> +		entry_size = GITS_BASER_ENTRY_SIZE(val);
> +
>  		if (type == GITS_BASER_TYPE_NONE)
>  			continue;
>  
> @@ -939,8 +957,8 @@ retry_baser:
>  		val |= alloc_pages - 1;
>  		its->tables[i].val = val;
>  
> -		writeq_relaxed(val, its->base + GITS_BASER + i * 8);
> -		tmp = readq_relaxed(its->base + GITS_BASER + i * 8);
> +		its_write_baser(its, baser, val);
> +		tmp = its_read_baser(its, baser);
>  
>  		if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) {
>  			/*
> 

Thanks to this abstraction, I now realize what I really hate about this
idea of caching stuff in the its_baser structure: you end up with
inconsistent data: 

- You store something in baser->val
- You write something else using its_write_baser
- You read something else using its_read_baser

We have 3 sets of inconsistent data, and what they mean depends on
the context.

What I'm proposing is that its_write_baser always performs a read back, 
and stores that read in baser->val. This gives us a more consistent 
view of what's going on (the value we wanted, and what the HW is 
actually capable of).

How about something like this, on top of my previous suggestion:

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 520d171..5591441 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -836,6 +836,7 @@ static void its_write_baser(struct its_node *its, struct its_baser *baser,
 {
 	int idx = baser - its->tables;
 	writeq_relaxed(val, its->base + GITS_BASER + (idx << 3));
+	baser->val = its_read_baser(its, baser);
 }
 
 static int its_setup_baser(struct its_node *its, struct its_baser *baser,
@@ -891,7 +892,7 @@ retry_baser:
 	}
 
 	its_write_baser(its, baser, val);
-	tmp = its_read_baser(its, baser);
+	tmp = baser->val;
 
 	if ((val ^ tmp) & GITS_BASER_SHAREABILITY_MASK) {
 		/*
@@ -939,7 +940,6 @@ retry_baser:
 	baser->order = order;
 	baser->base = base;
 	baser->psz = psz;
-	baser->val = val;
 	tmp = indirect ? GITS_LVL1_ENTRY_SIZE : esz;
 
 	pr_info("ITS@%pa: allocated %d %s @%lx (%s, esz %d, psz %dK, shr %d)\n",
@@ -968,7 +968,7 @@ static bool its_parse_baser_device(struct its_node *its, struct its_baser *baser
 		 * table by reading bit at offset '62' after writing '1' to it.
 		 */
 		its_write_baser(its, baser, val | GITS_BASER_INDIRECT);
-		val = its_read_baser(its, baser);
+		val = baser->val;
 		indirect = !!(val & GITS_BASER_INDIRECT);
 
 		if (indirect) {

Thanks,

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