[PATCH] irqchip/gic-v3: Allow 'dma-noncoherent' property for ITS

[PATCH] irqchip/gic-v3: Allow 'dma-noncoherent' property for ITS

[Ben Dai]

Currently the ITS driver expects the hardware to report whether it is
shareable, but according to the description of the GITS_CBASER register
in the GICv3 architecture specification:
 > It is IMPLEMENTATION DEFINED whether this field has a fixed value or
 > can be programmed by software. Implementing this field with a fixed
 > value is deprecated.

It means that the hardware may expect the software to correctly configure
the access attributes of the ITS. In order to support those designs where
ITS and CPU are not in a coherent domain, allow 'dma-noncoherent' property
for ITS.

Signed-off-by: Ben Dai <ben.dai9703@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de> (maintainer:IRQCHIP DRIVERS)
Cc: Marc Zyngier <maz@kernel.org> (maintainer:IRQCHIP DRIVERS)
Cc: linux-kernel@vger.kernel.org (open list:IRQCHIP DRIVERS)
---
 drivers/irqchip/irq-gic-v3-its.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 973ede0197e3..794b14b0a6b4 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -5009,6 +5009,7 @@ static int __init its_probe_one(struct resource *res,
 	void __iomem *its_base;
 	u64 baser, tmp, typer;
 	struct page *page;
+	bool coherent;
 	u32 ctlr;
 	int err;
 
@@ -5087,15 +5088,25 @@ static int __init its_probe_one(struct resource *res,
 		goto out_free_tables;
 
 	baser = (virt_to_phys(its->cmd_base)	|
-		 GITS_CBASER_RaWaWb		|
-		 GITS_CBASER_InnerShareable	|
 		 (ITS_CMD_QUEUE_SZ / SZ_4K - 1)	|
 		 GITS_CBASER_VALID);
 
+	coherent = of_dma_is_coherent(to_of_node(handle));
+	if (coherent) {
+		baser |= (GITS_CBASER_RaWaWb | GITS_CBASER_InnerShareable);
+	} else {
+		baser |= GITS_CBASER_nC;
+		its->flags |= ITS_FLAGS_CMDQ_NEEDS_FLUSHING;
+	}
+
 	gits_write_cbaser(baser, its->base + GITS_CBASER);
-	tmp = gits_read_cbaser(its->base + GITS_CBASER);
 
-	if ((tmp ^ baser) & GITS_CBASER_SHAREABILITY_MASK) {
+	if (coherent) {
+		tmp = gits_read_cbaser(its->base + GITS_CBASER);
+
+		if ((tmp ^ baser) & GITS_CBASER_SHAREABILITY_MASK)
+			its->flags |= ITS_FLAGS_CMDQ_NEEDS_FLUSHING;
+
 		if (!(tmp & GITS_CBASER_SHAREABILITY_MASK)) {
 			/*
 			 * The HW reports non-shareable, we must
@@ -5107,10 +5118,11 @@ static int __init its_probe_one(struct resource *res,
 			baser |= GITS_CBASER_nC;
 			gits_write_cbaser(baser, its->base + GITS_CBASER);
 		}
-		pr_info("ITS: using cache flushing for cmd queue\n");
-		its->flags |= ITS_FLAGS_CMDQ_NEEDS_FLUSHING;
 	}
 
+	if (its->flags & ITS_FLAGS_CMDQ_NEEDS_FLUSHING)
+		pr_info("ITS: using cache flushing for cmd queue\n");
+
 	gits_write_cwriter(0, its->base + GITS_CWRITER);
 	ctlr = readl_relaxed(its->base + GITS_CTLR);
 	ctlr |= GITS_CTLR_ENABLE;
-- 
2.34.1

Re: [PATCH] irqchip/gic-v3: Allow 'dma-noncoherent' property for ITS

[Marc Zyngier]

On Sun, 12 Feb 2023 09:35:05 +0000,
Ben Dai <ben.dai9703@gmail.com> wrote:
> 
> Currently the ITS driver expects the hardware to report whether it is
> shareable, but according to the description of the GITS_CBASER register
> in the GICv3 architecture specification:
>  > It is IMPLEMENTATION DEFINED whether this field has a fixed value or
>  > can be programmed by software. Implementing this field with a fixed
>  > value is deprecated.
> 
> It means that the hardware may expect the software to correctly configure
> the access attributes of the ITS. In order to support those designs where
> ITS and CPU are not in a coherent domain, allow 'dma-noncoherent' property
> for ITS.

No. This is an integration bug, most likely a Rockchip quality
design. I have repeatedly explained how to deal with this, and I'm not
going to do that again. Please search the list archives.

I will not be taking this sort of patch.

	M.

-- 
Without deviation from the norm, progress is not possible.