[PATCH 0/4] GICv3 ITS fixes for v4.0-rc6

[PATCH 0/4] GICv3 ITS fixes for v4.0-rc6

[Marc Zyngier]

The folowing patches fix a number of minor issues, some which have
been discovered when writing the KVM ITS emulation.

The first two patches fix some command encoding bugs, the third is
purely cosmetic, and the fourth fixes an issue that could occur on
configurations where the ITS geenrates cacheable accesses, but isn't
in the same shareability domain as the CPU.

It would be good if they could make it into 4.0-rc6.

Thanks,

	M.

Andre Przywara (1):
  irqchip: gicv3-its: Fix device ID encoding

Marc Zyngier (3):
  irqchip: gicv3-its: Fix encoding of collection's target redistributor
  irqchip: gicv3-its: Fix PROP/PEND and BASE/CBASE confusion
  irqchip: gicv3-its: Use non-cacheable accesses when no shareability

 drivers/irqchip/irq-gic-v3-its.c   | 57 ++++++++++++++++++++++++++++++++------
 include/linux/irqchip/arm-gic-v3.h | 17 ++++++++++++
 2 files changed, 65 insertions(+), 9 deletions(-)

-- 
2.1.4

[PATCH 1/4] irqchip: gicv3-its: Fix encoding of collection's target redistributor

[Marc Zyngier]

With a monolithic GICv3, redistributors are addressed using a linear
number, while a distributed implementation uses physical addresses.

When encoding a target address into a command, we strip the lower
16 bits, as redistributors are always 64kB aligned. This works
perfectly well with a distributed implementation, but has the
silly effect of always encoding target 0 in the monolithic case
(unless you have more than 64k CPUs, of course).

The obvious fix is to shift the linear target number by 16 when
computing the target address, so that we don't loose any precious
bit.

Reported-by: Andre Przywara <andre.przywara@arm.com>
Tested-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.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 596b0a9..7318dba 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1026,7 +1026,7 @@ static void its_cpu_init_collection(void)
 			 * This ITS wants a linear CPU number.
 			 */
 			target = readq_relaxed(gic_data_rdist_rd_base() + GICR_TYPER);
-			target = GICR_TYPER_CPU_NUMBER(target);
+			target = GICR_TYPER_CPU_NUMBER(target) << 16;
 		}
 
 		/* Perform collection mapping */
-- 
2.1.4

[PATCH 2/4] irqchip: gicv3-its: Fix device ID encoding

[Marc Zyngier]

From: Andre Przywara <andre.przywara@arm.com>

When building ITS commands which have the device ID in it, we
should mask off the whole upper 32 bits of the first command word
before inserting the new value in there.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.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 7318dba..fa0c436 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -169,7 +169,7 @@ static void its_encode_cmd(struct its_cmd_block *cmd, u8 cmd_nr)
 
 static void its_encode_devid(struct its_cmd_block *cmd, u32 devid)
 {
-	cmd->raw_cmd[0] &= ~(0xffffUL << 32);
+	cmd->raw_cmd[0] &= BIT_ULL(32) - 1;
 	cmd->raw_cmd[0] |= ((u64)devid) << 32;
 }
 
-- 
2.1.4

[PATCH 3/4] irqchip: gicv3-its: Fix PROP/PEND and BASE/CBASE confusion

[Marc Zyngier]

The ITS driver sometime mixes up the use of GICR_PROPBASE bitfields
for the GICR_PENDBASE register, and GITS_BASER for GICR_CBASE.

This does not lead to any observable bug because similar bits are
at the same location, but this just make the code even harder to
understand...

This patch provides the required #defines and fixes the mixup.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-v3-its.c   |  6 +++---
 include/linux/irqchip/arm-gic-v3.h | 13 +++++++++++++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index fa0c436..56353f6 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -986,8 +986,8 @@ static void its_cpu_init_lpis(void)
 
 	/* set PENDBASE */
 	val = (page_to_phys(pend_page) |
-	       GICR_PROPBASER_InnerShareable |
-	       GICR_PROPBASER_WaWb);
+	       GICR_PENDBASER_InnerShareable |
+	       GICR_PENDBASER_WaWb);
 
 	writeq_relaxed(val, rbase + GICR_PENDBASER);
 
@@ -1425,7 +1425,7 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
 	writeq_relaxed(0, its->base + GITS_CWRITER);
 	writel_relaxed(GITS_CTLR_ENABLE, its->base + GITS_CTLR);
 
-	if ((tmp ^ baser) & GITS_BASER_SHAREABILITY_MASK) {
+	if ((tmp ^ baser) & GITS_CBASER_SHAREABILITY_MASK) {
 		pr_info("ITS: using cache flushing for cmd queue\n");
 		its->flags |= ITS_FLAGS_CMDQ_NEEDS_FLUSHING;
 	}
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index 781974a..826a4bd 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -128,6 +128,19 @@
 #define GICR_PROPBASER_RaWaWb		(7U << 7)
 #define GICR_PROPBASER_IDBITS_MASK	(0x1f)
 
+#define GICR_PENDBASER_NonShareable	(0U << 10)
+#define GICR_PENDBASER_InnerShareable	(1U << 10)
+#define GICR_PENDBASER_OuterShareable	(2U << 10)
+#define GICR_PENDBASER_SHAREABILITY_MASK (3UL << 10)
+#define GICR_PENDBASER_nCnB		(0U << 7)
+#define GICR_PENDBASER_nC		(1U << 7)
+#define GICR_PENDBASER_RaWt		(2U << 7)
+#define GICR_PENDBASER_RaWb		(3U << 7)
+#define GICR_PENDBASER_WaWt		(4U << 7)
+#define GICR_PENDBASER_WaWb		(5U << 7)
+#define GICR_PENDBASER_RaWaWt		(6U << 7)
+#define GICR_PENDBASER_RaWaWb		(7U << 7)
+
 /*
  * Re-Distributor registers, offsets from SGI_base
  */
-- 
2.1.4

[PATCH 4/4] irqchip: gicv3-its: Use non-cacheable accesses when no shareability

[Marc Zyngier]

If the ITS or the redistributors report their shareability as zero,
then it is important to make sure they will no generate any cacheable
traffic, as this is unlikely to produce the expected result.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-v3-its.c   | 47 ++++++++++++++++++++++++++++++++++----
 include/linux/irqchip/arm-gic-v3.h |  4 ++++
 2 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 56353f6..9687f8a 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -802,6 +802,7 @@ static int its_alloc_tables(struct its_node *its)
 	int i;
 	int psz = SZ_64K;
 	u64 shr = GITS_BASER_InnerShareable;
+	u64 cache = GITS_BASER_WaWb;
 
 	for (i = 0; i < GITS_BASER_NR_REGS; i++) {
 		u64 val = readq_relaxed(its->base + GITS_BASER + i * 8);
@@ -848,7 +849,7 @@ retry_baser:
 		val = (virt_to_phys(base) 				 |
 		       (type << GITS_BASER_TYPE_SHIFT)			 |
 		       ((entry_size - 1) << GITS_BASER_ENTRY_SIZE_SHIFT) |
-		       GITS_BASER_WaWb					 |
+		       cache						 |
 		       shr						 |
 		       GITS_BASER_VALID);
 
@@ -874,9 +875,12 @@ retry_baser:
 			 * Shareability didn't stick. Just use
 			 * whatever the read reported, which is likely
 			 * to be the only thing this redistributor
-			 * supports.
+			 * supports. If that's zero, make it
+			 * non-cacheable as well.
 			 */
 			shr = tmp & GITS_BASER_SHAREABILITY_MASK;
+			if (!shr)
+				cache = GITS_BASER_nC;
 			goto retry_baser;
 		}
 
@@ -980,6 +984,17 @@ static void its_cpu_init_lpis(void)
 	tmp = readq_relaxed(rbase + GICR_PROPBASER);
 
 	if ((tmp ^ val) & GICR_PROPBASER_SHAREABILITY_MASK) {
+		if (!(tmp & GICR_PROPBASER_SHAREABILITY_MASK)) {
+			/*
+			 * The HW reports non-shareable, we must
+			 * remove the cacheability attributes as
+			 * well.
+			 */
+			val &= ~(GICR_PROPBASER_SHAREABILITY_MASK |
+				 GICR_PROPBASER_CACHEABILITY_MASK);
+			val |= GICR_PROPBASER_nC;
+			writeq_relaxed(val, rbase + GICR_PROPBASER);
+		}
 		pr_info_once("GIC: using cache flushing for LPI property table\n");
 		gic_rdists->flags |= RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING;
 	}
@@ -990,6 +1005,18 @@ static void its_cpu_init_lpis(void)
 	       GICR_PENDBASER_WaWb);
 
 	writeq_relaxed(val, rbase + GICR_PENDBASER);
+	tmp = readq_relaxed(rbase + GICR_PENDBASER);
+
+	if (!(tmp & GICR_PENDBASER_SHAREABILITY_MASK)) {
+		/*
+		 * The HW reports non-shareable, we must remove the
+		 * cacheability attributes as well.
+		 */
+		val &= ~(GICR_PENDBASER_SHAREABILITY_MASK |
+			 GICR_PENDBASER_CACHEABILITY_MASK);
+		val |= GICR_PENDBASER_nC;
+		writeq_relaxed(val, rbase + GICR_PENDBASER);
+	}
 
 	/* Enable LPIs */
 	val = readl_relaxed(rbase + GICR_CTLR);
@@ -1422,14 +1449,26 @@ static int its_probe(struct device_node *node, struct irq_domain *parent)
 
 	writeq_relaxed(baser, its->base + GITS_CBASER);
 	tmp = readq_relaxed(its->base + GITS_CBASER);
-	writeq_relaxed(0, its->base + GITS_CWRITER);
-	writel_relaxed(GITS_CTLR_ENABLE, its->base + GITS_CTLR);
 
 	if ((tmp ^ baser) & GITS_CBASER_SHAREABILITY_MASK) {
+		if (!(tmp & GITS_CBASER_SHAREABILITY_MASK)) {
+			/*
+			 * The HW reports non-shareable, we must
+			 * remove the cacheability attributes as
+			 * well.
+			 */
+			baser &= ~(GITS_CBASER_SHAREABILITY_MASK |
+				   GITS_CBASER_CACHEABILITY_MASK);
+			baser |= GITS_CBASER_nC;
+			writeq_relaxed(baser, its->base + GITS_CBASER);
+		}
 		pr_info("ITS: using cache flushing for cmd queue\n");
 		its->flags |= ITS_FLAGS_CMDQ_NEEDS_FLUSHING;
 	}
 
+	writeq_relaxed(0, its->base + GITS_CWRITER);
+	writel_relaxed(GITS_CTLR_ENABLE, its->base + GITS_CTLR);
+
 	if (of_property_read_bool(its->msi_chip.of_node, "msi-controller")) {
 		its->domain = irq_domain_add_tree(NULL, &its_domain_ops, its);
 		if (!its->domain) {
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index 826a4bd..ffbc034 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -126,6 +126,7 @@
 #define GICR_PROPBASER_WaWb		(5U << 7)
 #define GICR_PROPBASER_RaWaWt		(6U << 7)
 #define GICR_PROPBASER_RaWaWb		(7U << 7)
+#define GICR_PROPBASER_CACHEABILITY_MASK (7U << 7)
 #define GICR_PROPBASER_IDBITS_MASK	(0x1f)
 
 #define GICR_PENDBASER_NonShareable	(0U << 10)
@@ -140,6 +141,7 @@
 #define GICR_PENDBASER_WaWb		(5U << 7)
 #define GICR_PENDBASER_RaWaWt		(6U << 7)
 #define GICR_PENDBASER_RaWaWb		(7U << 7)
+#define GICR_PENDBASER_CACHEABILITY_MASK (7U << 7)
 
 /*
  * Re-Distributor registers, offsets from SGI_base
@@ -195,6 +197,7 @@
 #define GITS_CBASER_WaWb		(5UL << 59)
 #define GITS_CBASER_RaWaWt		(6UL << 59)
 #define GITS_CBASER_RaWaWb		(7UL << 59)
+#define GITS_CBASER_CACHEABILITY_MASK	(7UL << 59)
 #define GITS_CBASER_NonShareable	(0UL << 10)
 #define GITS_CBASER_InnerShareable	(1UL << 10)
 #define GITS_CBASER_OuterShareable	(2UL << 10)
@@ -211,6 +214,7 @@
 #define GITS_BASER_WaWb			(5UL << 59)
 #define GITS_BASER_RaWaWt		(6UL << 59)
 #define GITS_BASER_RaWaWb		(7UL << 59)
+#define GITS_BASER_CACHEABILITY_MASK	(7UL << 59)
 #define GITS_BASER_TYPE_SHIFT		(56)
 #define GITS_BASER_TYPE(r)		(((r) >> GITS_BASER_TYPE_SHIFT) & 7)
 #define GITS_BASER_ENTRY_SIZE_SHIFT	(48)
-- 
2.1.4

[PATCH 0/4] GICv3 ITS fixes for v4.0-rc6

[Jason Cooper]

Marc,

On Fri, Mar 27, 2015 at 02:15:01PM +0000, Marc Zyngier wrote:
> The folowing patches fix a number of minor issues, some which have
> been discovered when writing the KVM ITS emulation.
> 
> The first two patches fix some command encoding bugs, the third is
> purely cosmetic, and the fourth fixes an issue that could occur on
> configurations where the ITS geenrates cacheable accesses, but isn't
> in the same shareability domain as the CPU.
> 
> It would be good if they could make it into 4.0-rc6.
> 
> Thanks,
> 
> 	M.
> 
> Andre Przywara (1):
>   irqchip: gicv3-its: Fix device ID encoding
> 
> Marc Zyngier (3):
>   irqchip: gicv3-its: Fix encoding of collection's target redistributor
>   irqchip: gicv3-its: Fix PROP/PEND and BASE/CBASE confusion
>   irqchip: gicv3-its: Use non-cacheable accesses when no shareability
> 
>  drivers/irqchip/irq-gic-v3-its.c   | 57 ++++++++++++++++++++++++++++++++------
>  include/linux/irqchip/arm-gic-v3.h | 17 ++++++++++++
>  2 files changed, 65 insertions(+), 9 deletions(-)

Applied to irqchip/urgent-gic and merged into irqchip/urgent.  I'll send a PR
mid-week or so.

thx,

Jason.

[PATCH 0/4] GICv3 ITS fixes for v4.0-rc6

[Marc Zyngier]

On 29/03/15 20:57, Jason Cooper wrote:
> Marc,
> 
> On Fri, Mar 27, 2015 at 02:15:01PM +0000, Marc Zyngier wrote:
>> The folowing patches fix a number of minor issues, some which have
>> been discovered when writing the KVM ITS emulation.
>>
>> The first two patches fix some command encoding bugs, the third is
>> purely cosmetic, and the fourth fixes an issue that could occur on
>> configurations where the ITS geenrates cacheable accesses, but isn't
>> in the same shareability domain as the CPU.
>>
>> It would be good if they could make it into 4.0-rc6.
>>
>> Thanks,
>>
>> 	M.
>>
>> Andre Przywara (1):
>>   irqchip: gicv3-its: Fix device ID encoding
>>
>> Marc Zyngier (3):
>>   irqchip: gicv3-its: Fix encoding of collection's target redistributor
>>   irqchip: gicv3-its: Fix PROP/PEND and BASE/CBASE confusion
>>   irqchip: gicv3-its: Use non-cacheable accesses when no shareability
>>
>>  drivers/irqchip/irq-gic-v3-its.c   | 57 ++++++++++++++++++++++++++++++++------
>>  include/linux/irqchip/arm-gic-v3.h | 17 ++++++++++++
>>  2 files changed, 65 insertions(+), 9 deletions(-)
> 
> Applied to irqchip/urgent-gic and merged into irqchip/urgent.  I'll send a PR
> mid-week or so.

Awesome, thanks Jason.

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